A Bridge Identifier is two priority octets followed by the MAC, compared
as one unsigned number. The test here read the first priority octet and
then went straight to the MAC, so the system ID extension in between was
never looked at and two bridges differing only in it were ranked by MAC
instead. The field is stored, sent and printed, just not compared.
Ordinary single instance RSTP leaves the extension zero on both sides,
which is why this has not shown up. Where it is not zero the ranking is
simply wrong: same priority octet, extension 0x0a against 0x00, and the
worse bridge wins if its MAC happens to be lower.
cmpMAC becomes cmpBytes with a length, since the identifier is eight
contiguous bytes in both the packet overlay and root_bridge, and the
loop was already doing the right thing for six of them. sdcc lays the
struct out with no padding, checked, so the eight byte compare is the
standard's rule written directly.
802.1D has a port leave the edge state when a BPDU arrives on it. Here
the flag was only ever cleared by the loop latch, root guard, a link
coming back, "stp on", "stp off" and the edge command itself, so a port
that auto-edged during the three seconds of silence after link-up kept
the flag for as long as it stayed up, whatever the neighbour sent.
Two things read that flag. The status page prints it, so a port talking
to a bridge reported edge 1 and there was no way to tell from the output
whether a BPDU had ever arrived. More quietly, stp_topology_change()
returns early for an edge port, which is right for a real one and wrong
for this: a topology change on such a port was neither counted nor
propagated, and port_l2_forget_port() never ran, so what was learned
behind it stayed in the table.
Only the flag is cleared. The port is not pushed back through the listen
period, which would take a working link out of forwarding for a forward
delay the first time a neighbour speaks.
"stp status" has always worked, but the line printed on a bad command
listed only on and off, so the one subcommand that shows what the bridge
thinks was the one you had to already know about.
The window could be armed from the serial console but only ever disarmed
by an HTTP request. save_cmd, which gates arming, is cleared only while
execute_config() replays the startup config, so every interactive command
armed it wherever it was typed, while mgmt_alive, which disarms it, was
written in exactly one place, on HTTP traffic. An operator working
entirely on the serial console therefore lost STP 180 seconds after
enabling it however much they typed, which is what makes the mechanism
impossible to test from a console.
The documentation described the behaviour that was intended rather than
the one that was built, and in both directions: it said a command on the
serial console also confirms, and it said a reboot with STP in the
startup config disables it again three minutes later. Neither held. The
replay path never armed the window at all.
Repairing the asymmetry would have kept a mechanism whose premise is
contested anyway. A watchdog that switches the protection off in response
to silence adds a second failure mode on top of the first: where the
network is misconfigured and STP is the thing holding a storm back,
restoring forwarding removes the last reason management still answers.
Gone with it: the stp failsafe command, the fs and fsT fields of
/stp.json, the input and the tripped banner on the Spanning Tree page,
the two persistence patterns in config.js, the documentation section, and
mgmt_alive itself, which had no other reader.
550 bytes back, 145 of BANK1 and 405 of BANK2, and five of xdata, which
is the four counters and mgmt_alive and nothing else. Built for
SWTGW218AS and KP_9000_6XHML_X2 on sdcc 4.5.0.
Your console session shows the shape of this better than I could have. STP found
a loop, blocked the port, unblocked the other side of the pair, and then the
failsafe turned STP off. It had been disarmed by your first console command and
re-armed by the loop detection itself, so a mechanism that exists to protect
against a lockout ended up removing loop protection while a loop was physically
present. That is the part that did not make sense, and it wasn't the console.
Two changes, both narrowing.
Loop detection and root guard no longer arm the window. Those are the protocol
doing its job on evidence off the wire. Nothing an operator did needs undoing
there, and nobody is waiting to confirm anything.
Typing on the serial console no longer disarms it. The failsafe asks one
question, whether the operator can still reach management over the network, and
serial activity doesn't answer it. It proves somebody is standing at the box,
which is the one case where a lockout doesn't matter, and it took the safety net
away from a remote operator on behalf of someone not using it. HTTP activity
still confirms, because that is the path being measured, and the console in the
web interface counts for the same reason.
What is left arms on stp on, stp port N on and stp failsafe, each of them an
operator choosing something whose outcome the protocol then decides.
Gives back 61 bytes of BANK2, 31 of the common area and a byte of xdata.
Two things from the review, both about the console being where you end up when
the tree is not what you expected.
The six messages that name a port were printing the internal index. On a board
whose map is not the identity that is a different number from the one written
next to the socket, which is worse than no number at all. They go through
machine.log_to_phys_port now, in a small helper that also swallows the newline
each of them repeated.
"stp status" prints the bridge and root IDs, the root port and path cost, the
topology change count, the failsafe setting, and a line per port with state,
role and operational edge. Everything it shows is state the module already
keeps, apart from the port states, which come from one read of MSTP_STATES.
660 bytes of BANK2, which leaves 3366 free. No internal RAM, no xdata.
The report on the PR is that it will not link for KP_9000_6XHML_X2, with
"?ASlink-Error-Could not get N consecutive bytes in internal RAM for area OSEG"
five times over. It builds here on sdcc 4.2.0 and 4.5.0 for that same machine
and the same commit, so something in the toolchain differs, but the pressure it
is complaining about is mine and it costs little to give back.
stp_in() is __banked, so its temporaries get exclusive DSEG instead of
overlaying with anything else. Recording the designated bridge put four more
live values across a memcpy in the middle of it and the register allocator
answered with five spill locations. The module went from 5 bytes of DSEG to 12,
and from 17 sloc references to 49.
Moving that block into a __reentrant helper puts its temporaries on the stack
instead. The module now claims no DSEG at all, 5 bytes better than before the
recording was added, and the image sits at 95 bytes of DSEG against 101 on main.
It costs 170 bytes of BANK2, where there is room.
The failsafe is a commit confirm window for an interactive change: turn STP on,
and if management goes quiet for stp_failsafe_s seconds the switch undoes it.
The three places that arm it sit in the command parser, and execute_config()
drives that same parser at boot, so a saved "stp on" arms the window too. A
switch that reboots with nobody watching then turns its own STP back off.
Measured on a SWTGW218AS with "stp failsafe 180" in the saved config: cold boot,
no HTTP and no console for four minutes, and "STP failsafe: disabling" arrives
on time, with stp.json reporting on:0 and fsT:1.
execute_config() already clears save_cmd while it replays and sets it again at
the end, so the three parser sites can just test it. The two on the protocol
side, the loop latch and the root guard, stay unconditional. They react to what
arrived on the wire, which is the case the failsafe exists for, and they only
run once the replay is long finished.
BANK2 grows 24 bytes. Nothing else moves.
stp_dbridge, stp_dpid and stp_dcost were declared and read by the status page,
but nothing ever wrote them, so they stayed zero for the life of the firmware.
The page's validity test then always failed, and the Designated Bridge,
Designated Port ID and Designated Cost columns reported our own values on every
port, including the root port where the answer is the upstream neighbour. The
three arrays reserved 140 bytes of xdata and never used any of it.
They are filled now, right after the loop check, so a frame that came back from
one of our own ports is not mistaken for a neighbour.
The validity test moves from the last byte of the stored MAC to the stored Port
ID. A Port ID is 1-based on the wire and cannot be zero, while a neighbour whose
MAC happens to end in 0x00 would have failed the old test.
The root path cost byte swap happens once now, and the root port branch reuses
the value instead of repeating the shifts.
BANK2 grows 148 bytes, BANK1 loses 9, and xdata does not move.
Three things from the page feedback.
The rows came out in logical order while carrying the physical port number,
so on the six-port boards the first row is labelled 5. Sorting the rows by
that number in JS puts every board back into front-panel order. I walked all
25 machine definitions and each one now yields a clean 1..N.
The Designated Bridge column is hard to read without knowing this switch's
own bridge ID, so the status line shows it in the same priority and MAC shape
as the cells use. On the test switch that reads 61440-06:05:16:1E:F9:24 and
matches the Designated Bridge of every locally designated port, which is the
comparison that was missing. The root bridge and the path cost now use the
same formatting as the columns instead of raw hex.
Enabling STP printed nothing until the next poll, and because the ports start
blocked, management can stay quiet for the whole listening and learning
period, so the page had no chance to say anything later. It now writes what
is about to happen before the command goes out, and how long the ports need.
Page data only. Both banks, xdata and the common bank are unchanged.
The review asked for these to come out and the reasoning holds. A guard that
gives up mid-transaction lets the code carry on with whatever the engine left
behind, and that is what cost me an SPI clip twice. An unbounded wait on a
wedged engine still hangs, but it hangs in a known place instead of writing
garbage into the L2 table.
BANK1 loses 45 bytes and xdata one. BANK2 and the common bank do not move.
Review asked for this across the other commits too. Gone are the blocks
that restate what doc/stp.md already says, the ones that explain what an
embedded programmer already knows, and one that had gone stale inside this
very branch: the CLI summary above stp_parse still described "cost <0-255>
(x1000)" while the parser has taken the raw 0 to 200000000 for some time,
and it never learned about p2p or trk at all. A usage list next to the
parser is the kind of thing that rots first, so it is out rather than
updated.
The review flagged one comment saying a variable is in xdata because the
internal RAM overlay is full, on the grounds that it may stop being true.
Four more of the same kind were in these files and are out as well, one of
them pointing at a file that does not exist in this branch at all. The
declarations still say __xdata, which is the part a reader needs.
Also out: the note on why three helpers are __reentrant, which was really
a paragraph about two bytes of DSEG, and the measurement story behind the
tick divider, which belongs with the other timer numbers in doc/stp.md.
One comment was not stale but simply wrong. "max BPDUs per port per second"
sat on stp_failsafe_tripped, having slid down two lines when the two
failsafe variables were inserted above it. It describes stp_txhold and is
back there now.
Short factual labels stay: they sit next to the magic number they explain
and the codebase uses them throughout. The generated code is byte for byte
what it was before this commit, both banks and xdata unchanged.
Review asked for this directly: the hardware layout above port_l2mc_set()
would be better as documentation than as a comment, keeping only the two
lines that say what the function does.
doc/l2.md gains a section on static multicast entries, why delivery uses
the forward action rather than the trap, and the SMI layout of the entry.
doc/CpuPort.md gains the layout of the tag's flags and pmask words, with
the byte order trap that cost an afternoon: writing the flags constant raw
instead of through HTONS puts 0x0020 on the wire as 0x2000, which is EFID
rather than LEARN_DIS, and the ASIC then leaves the 0x8899 header on the
frame.
The comments those paragraphs came from are replaced by a pointer to the
file that now holds them.
The note said a blocked port drops frames the CPU injects into it and so
cannot send BPDUs of its own. Hardware says otherwise, and it matters,
because that sentence is the reason one would go looking for a way to let
control frames out of a blocked port when there is nothing to fix there.
Held a two port group in blocking and watched from the neighbour. Our
BPDUs kept leaving it, 27 of them with a largest gap of 2.00 s, which is
the hello interval with nothing missed. Pings across the same port stopped
dead for 11.63 s in one unbroken gap, so data really is held. In the same
window the neighbour sent 74 frames with a largest gap of 1.04 s while our
receive counter for them moved by 2, and the port stayed in its trunk
throughout, so nothing in the aggregation code was discarding them.
The failsafe used to watch management traffic for as long as STP ran, so
three minutes of nobody looking at the web UI took the tree down on any
quiet network. That made a standing STP config impractical, which is the
problem raised in the review of the original PR.
Enabling STP arms a window of stp_failsafe_s seconds. One HTTP request
inside it confirms that management survived the new tree and disarms the
watchdog; a silent window disables STP and restores forwarding. Both
outcomes print to the console and the syslog.
The window re-arms on any later event that newly takes a port out of
forwarding: a port rejoining via "stp port N on", root guard firing, the
loop latch. Those were covered by the old always-on surveillance and a
disarmed window would have left them able to cut management off for good.
If management traffic keeps flowing past the new block, the next request
confirms straight away, which is the correct verdict, the block did not
cut it. The arming deliberately does not refresh an already armed window:
root guard can re-fire on every hello, and refreshing the countdown on
each one would keep a cut-off window from ever expiring. A stable network
with nothing newly blocked never re-arms, which is the reviewed-for
behaviour.
The request or console command that causes the arming never counts as its
own confirmation: mgmt_alive is cleared when a command arms, and the
console hook only disarms when the window predates the command. Without
that, enabling from the web UI or the console would confirm the window
before the new tree had any chance to cut management off.
A command on the serial console confirms like HTTP does. An operator at
the console has out-of-band access that no tree can cut, so the automatic
restore only takes STP away from someone equipped to deal with the
situation. The hook sits on the interactive console path only, identified
by cmd_available, so neither the config replay at boot nor HTTP commands
pass through it.
After a confirmation STP runs unsupervised until something new blocks.
Headless installs where nobody will confirm should set stp failsafe 0;
doc/stp.md says so.
Costs two bytes of XDATA, the armed flag and the console-path snapshot;
stp.rel and rtlplayground.rel keep their segment sizes.
A port set to admit tagged frames only will never see a BPDU, because
delivery rides the forward action and the ingress pipeline drops untagged
frames before the L2 lookup. The failure is silent and looks like a dead
receive path: the port turns edge after three seconds, the bridge elects
itself root, and nothing hints at the ingress setting. Diagnosing exactly
that cost most of a day on a live switch, with the neighbour provably
transmitting the whole time.
stp_setup() now prints one line per affected port, so the hint lands at
"stp on" and at every config replay on boot. The check runs in its own
loop after the MSTP write: port_ingress_filter_get() reads a register
into sfr_data, which the state-building loop above is still using. The
port number in the message is physical, matching what the ingress
command takes.
doc/stp.md explains why this can happen here and not on a normal bridge,
where BPDUs are consumed before any VLAN classification.
stp.rel stays at DSEG 5 with no OSEG and the image at 10498 bytes of
XDATA.
Every RST BPDU we sent carried flags 0x3c - designated, learning, forwarding -
whatever the port was actually doing. A blocked port kept announcing itself as
forwarding, and the root port would have called itself designated. Nothing on
this bench acted on it, but it is a lie in the protocol frame and the kind
that surfaces in somebody else's mixed network.
Derive the flags instead: the root port reports the root role, every other
transmitting port is designated (alternates do not transmit at all), and the
learning and forwarding bits mirror the ASIC state, so a listening port now
sends 0x0c. TC and TCA stay dynamic as before. Legacy Config BPDUs are
unchanged - their flags only ever carried TC and TCA.
Costs nothing in internal RAM; the state comes from the register scratch that
is already there.
Two of our own ports on one segment blocked each other in turn instead
of settling. The guard on the loop path only acted when port_timers[]
had already run out, so a BPDU arriving while the port was blocked did
nothing: the timer expired, the port went forwarding, the loop reopened
and the pair started over. The comment above the code claimed the
opposite - "if the loop persists the BPDUs keep arriving and the port
stays blocked" - but nothing implemented it.
Measured on a SWTGW218AS with a patch cord between two free ports: both
ports blocked, both returned to forwarding one forward delay later, and
the topology-change counter reached 0x51 in 5.5 minutes - 15.6 changes
per minute for as long as the cable was in.
Let the better Port ID decide for both. That port is forwarding by
construction, so it goes on hearing the loop and re-arms the other
port's timer on every BPDU, which is what turns the block into a latch;
the held port only has to keep transmitting, which the send path already
allows in any MSTP state. Nothing here depends on a blocked port still
receiving - that was never established. Having one writer also removes a
race: while both ends decided for themselves, the winner's re-arm could
land in the loser's port_timers[] first, the loser read it as "already
blocked" and skipped its own state change, and the loop stayed open.
802.1D compares the priority before the number and stp_cnf_send() puts
stp_pprio[] on the wire next to it, so compare that first - otherwise
"stp port N prio" would quietly not influence which end of a looped pair
keeps forwarding.
The port number arrives in a frame and our bridge MAC is public in every
BPDU we send, so bound it to the ports this module manages before
indexing anything. Outside that range nothing would release the block
either: stp_timers() walks min_port..max_port and skips ports that are
not STP-enabled, so their port_timers[] never counts down.
Equal Port IDs mean the frame came back on the port it left - a loop
further out, behind an unmanaged switch. There is no pair to choose
from, so that port holds itself down; since it can only re-arm while it
is receiving, that case stays the forward-delay pulse it was before
rather than becoming a real latch.
The work sits in a __reentrant helper on purpose, like the two functions
above it: parameters and locals then live on the stack. Inlined into
stp_in(), which is __banked and whose temporaries cannot be overlaid,
the same code costs two more bytes of DSEG - enough to stop an image
that also carries LACP from linking at all.
Verified on hardware: with the loop in place for 1 h 36 min exactly one
port blocked, the other kept forwarding, and the topology-change counter
moved four times in total - three of them the link event and the
promotion in the first minute.
The wording read as a claim about the CPU interface in general, which is
wrong and misleading: the 8051 sits behind an ordinary port of the internal
switch and is an ordinary member of a forwarding mask - which is exactly
what this implementation relies on.
Say what is actually broken instead: the trap action, a separate mechanism
whose destination is an external CPU port these boards do not populate.
Record the measurements behind it, including the widened CPU_PMSK and both
external-CPU destinations, and add the ACL trap result - a rule matching the
group intercepts frames but does not deliver them either, which is a second,
independent path to the same conclusion.
Reported-by: vDorst
We only recognised RST BPDUs when the Protocol Version Identifier was
exactly 2, which silently drops every MST BPDU: 802.1s uses version 3
with type 2 and a prefix deliberately laid out to be identical to an RST
BPDU, precisely so that an RSTP bridge can parse it.
802.1D-2004 14.4 spells the rule out - a bridge shall accept a version
identifier of 2 or greater and treat the BPDU as RST, ignoring anything
beyond what it understands. Compare with >= instead of ==. The receive
path already length-checks before touching the body and only reads the
fields common to both formats, so a longer MST body needs no other care.
The two fields are deliberately asymmetric: the Protocol Identifier must
be exactly zero (it is a sanity check), while the version is an extension
point that has to tolerate the future.
The state machine never looked at link state, so a port whose cable was
pulled stayed in forwarding: it kept being announced, kept its learned
entries, and the most ordinary topology change there is went unnoticed.
Observed on hardware - a non-edge port with the link administratively
down still reported forwarding and left the topology-change counter at
zero for the whole observation window.
Sample the carrier bitmap once per second, alongside the tx-budget refill
(the 50 Hz tick has no business doing register reads). On carrier loss put
the port back to blocking and run the normal topology-change path, which
flushes just that port's entries. On carrier return re-run the listen
period rather than forwarding immediately - the segment may have been
rewired while we were down - and clear the operational edge flag so a port
that was auto-edged has to earn it again.
stp_setup() seeds the bitmap from the hardware so enabling STP does not
report every already-down port as a fresh topology change.
Covers enabling it, the bridge and per-port settings, how BPDUs are
delivered on this hardware (RMA forward constrained by a CPU-only static
L2 entry, since trap-to-CPU targets an external CPU these boards do not
have), the timer base, and the management failsafe - enabling STP over
the network can block the very port the management VLAN rides on, so
that part is spelled out rather than left to be discovered.
"stp port N edge off" cleared only the admin and auto flags, not the
operational one - and that is the flag the engine actually consults: it
exempts the port from topology changes and lets it skip the listen
period. A port therefore stayed an edge port until the next "stp off" /
"stp on", silently ignoring the new setting. Clear it with the others,
and mark an admin edge operational right away, as stp_setup() does.
The timers assumed stp_timers() runs at 64 Hz. It does not: the main loop
idles on the 200 Hz system tick and calls us every fourth pass, i.e.
50 Hz. Measured on hardware - with hello configured to 2 s the BPDUs left
the port 2.560 s apart, exactly the 28 % overshoot the wrong constant
implies, and every other timer (forward delay, max age, tx-hold refill)
was stretched the same way.
Move the constant to the header with the arithmetic spelled out, and use
it in the status page too, which had the 64 hardcoded and therefore aged
the same counters differently than the engine.
The Topology Change Acknowledgment is staged in a one-shot variable and
consumed when the BPDU is built - but stp_cnf_send() can return before
that, when the port is filtered/tripped or its tx-hold budget for this
second is spent. The flag then survived and was OR-ed into the next BPDU
this switch sent, on whatever port that happened to be. Clear it with
the frame it belonged to.
A port entering forwarding, or being blocked because its own BPDU came
back, changes where MAC addresses live - but the counter was bumped and
nothing else happened: our forwarding table kept the stale entries and
the neighbours were never told.
Flush the affected port's dynamic entries (bounded single-port flush)
and set the Topology Change flag in our BPDUs for max age + forward
delay, so neighbours age their tables out as well. Edge ports are
exempt: a host coming or going is not a topology change.
port_l2_forget() flushes the whole table and polls the flush engine
without a bound. A topology change only needs to age out the port that
changed, and the STP tick cannot afford an unbounded poll: add
port_l2_forget_port() with a single-port mask and the same bounded wait
the static-entry helper uses.
Two protocol-correctness fixes on the information we advertise:
Only designated ports announce periodically. The root port is where our
root information arrives; sending it back there feeds the upstream
bridge its own data and makes us look like a competing designated
bridge on that segment.
Relay the message age instead of always claiming zero. A bridge
increments the received age by one second per hop, so downstream
neighbours can age the information out; advertising 0 forever made our
BPDUs look eternally fresh no matter how stale the root information was.
Age stays 0 while we are the root ourselves.
Accept legacy Topology Change Notification BPDUs (v0, type 0x80): reply
on the ingress port with a Config BPDU carrying Topology Change
Acknowledgment so the sender stops repeating, and count the change.
Also stop reading fields past the end of short frames: require the
header through bpdu_type (33 bytes with the CPU/VLAN prefix) before
classifying, and the full 35-byte body before the election logic -
truncated or fuzzed BPDUs are dropped instead of parsed as garbage.
An RST BPDU body is 36 bytes: the Config-BPDU fields plus a trailing
version-1 length octet (zero - there is no version-1 information).
Ours was 35 - strict 802.1w parsers treat such a BPDU as malformed and
drop it. Add the field, keep legacy Config BPDUs at 35 bytes, and set
the 802.3 length accordingly (0x27 with LLC for RST, 0x26 for Config).
Bring the Spanning Tree page in line with a typical managed switch's
per-port panel. Configuration gains the full-range path cost (raw
0..200000000, 0 = auto, replacing the old 1000x-scaled byte), a
point-to-point admin control (auto/on/off), and the priority is now a
0..240 step-16 dropdown. A new status table shows, per port, the Port
State, Role, Designated Bridge / Port ID / Cost (learned from received
BPDUs, kept per port and aged via the BPDU age), Operational Edge and
Operational Point-to-Point.
The designated fields fall back to presenting this switch as the
segment's designated bridge when no fresh BPDU has been heard (so a
quiet port shows our own bridge-id, as the vendor UIs do). /stp.json
carries the packed hex fields plus our own MAC for that fallback.
Space: reclaim BANK2 for the above by moving rtl837x_pins to HOME and
compacting leds_dump into a register-address table (~800B); bandwidth
returns to BANK1. No BANK3 - hardware-verified that PSBANK > 2 crashes
this SoC at boot (a bricked unit and an SPI-programmer recovery earlier
today); a warning to that effect is now in rtl837x_lldp.c.
Hardware-verified: cost 200000000 and p2p off round-trip through the CLI
and JSON, the status table populates correctly with STP enabled (all
ports Forwarding/Designated, oper-edge and oper-p2p True), LACP 3f/3f
and the LAN unaffected.
(cherry picked from commit 2ec62072f061dc9e78bc821ba1c297cb6819e206)
Enabling STP on a bridge whose management rides an in-band VLAN can cut
off that very management - and not only by our own blocking: on this
network the upstream TP-Link Easy Smart switch's "loop prevention"
reacted to our BPDU hellos by blocking ITS port towards us while our
ASIC was all-forwarding, isolating the whole segment until a power
cycle. Recoverable only by going quiet.
Add a commit-confirm watchdog: while STP is enabled, any HTTP request
re-arms a countdown ("stp failsafe <seconds>", default 180, 0 disables);
if management stays silent for the whole window, STP disables itself,
which also stops BPDU TX so a neighbour's loop protection can release
its block. The web UI polls /stp.json every 2 s, so an open browser
naturally keeps the watchdog re-armed. The trip is reported via
/stp.json (fs, fsT) and as a warning on the Spanning Tree page.
Deliberately not conditioned on our own MSTP port states - the incident
above proves the uplink can be dead while every local port forwards.
Also bound the NIC DMA busy-waits (nic_tx_packet, nic_rx_header,
nic_rx_packet): an unbounded spin on SFR_NIC_CTRL freezes the entire
main loop (timers, HTTP, ARP) if the ASIC ever fails to consume a
transfer; give up after ~65k polls and drop the frame instead.
Hardware-verified end to end: with priority 15 against a live RSTP
bridge the uplink died 6 s after "stp on" and the network recovered BY
ITSELF 66 s later (trip at 45 s + neighbour release), fsT=1, LACP and
LAN intact. Telemetry via syslog-to-edge-port host confirmed the full
chain: countdown 44->4, trip, hello TX stopping at the trip.
(cherry picked from commit 1fa9775156fd6d7ebfdda2382f73430b86601230)
Two TX bugs meant our BPDUs NEVER left the switch as valid STP frames -
on the wire they appeared as ethertype 0x8899 (the raw Realtek CPU tag)
and were flooded to all ports instead of directed. Every earlier root
election was a solo act: no other bridge ever saw us. Both are the same
bug classes fixed for LACP earlier:
- rtl_tag.flags was written raw (0x0020); like every other tag field it
must go through HTONS, otherwise the bits land in the wrong byte
(0x2000 = EFID), the ASIC fails to parse the tag and floods the frame
with the 0x8899 header still attached.
- With a management VLAN set, tcpip_output() splices an 802.1Q tag after
the SA, again shifting the CPU tag out of the parsed position. BPDUs are
link-local and must egress untagged: suppress the VLAN insert per frame,
exactly as lacp_send() does.
Hardware note discovered while fixing this: RTL_TAG_KEEP on an LLC/802.3
(length-field) frame makes the ASIC drop it entirely - the same flag works
fine on ethertype frames (LACP). So BPDUs use LEARN_DIS only.
Verified on the wire (tcpdump on the peer): clean "802.3 ... LLC, dsap STP
0x42 ... Rapid STP, bridge-id 8000.<our mac>" at the hello interval, sent
directed (no flood), management HTTP unaffected, LAN at 0% loss throughout.
(cherry picked from commit 4a41a292a9ab88d4fb05a8481ad28f8ffcfd9bc4)
"stp port 7 edge" (no value) passed the cmd_words_len < 4 check and then
cmd_compare(4, ...) read a stale word left over from the PREVIOUS command
line - cmd_words_b is not cleared between commands - so the sub-command
could randomly match whatever was typed before. Require 5 words for every
per-port sub-command that carries an argument (everything except on/off).
(cherry picked from commit 1210f4f9257b14c31ad653fc7616ef403a494d28)
Implements the standard 802.1D-2004/802.1w configuration surface:
Bridge: priority (0-15 x4096), hello time, max age, forward delay,
force-version (RSTP v2 / STP-compatible v0 Config BPDUs), tx hold
count (per-port per-second BPDU budget).
Per port: enable, admin edge (forwarding immediately - no listen gap),
auto edge (forwarding after 3 s of BPDU silence; DEFAULT, so
host-facing ports no longer take the full forward delay),
path cost (0=auto/20000), port priority, BPDU guard (port disabled
on BPDU receipt), root guard (never accept a better root on the
port), BPDU filter (no BPDUs in or out).
Engine additions: root max-age expiry (reclaim the tree when the root goes
silent), root path cost accounting (rx cost + root-port cost, advertised in
our BPDUs), loop detection (our own BPDU coming back blocks the port for a
listen period), topology-change counter, approximated per-port roles
(Root/Designated/Alternate) for diagnostics.
CLI: "stp prio|hello|maxage|fwd|txhold|version ..." and
"stp port <n> on|off|edge|cost|prio|guard|filter ..." (stp_parse, delegated
from cmd_parser); all forms accepted by the startup-config validator so the
whole configuration persists. /stp.json now reports config + status; the
Spanning Tree page exposes everything with immediate-apply controls and live
state/role columns (edit-in-flight guard against the 2 s refresh).
8051 memory: the module moves to code BANK2; internal-RAM pressure from
cross-bank calls resolved by xdata loop iterators/scratch, __reentrant on
the small helpers, and moving httpd's header-pointer globals to xdata.
Verified on hardware (SWTGW218AS): defaults land per standard; priority and
hello change live; admin-edge ports (the LACP bond uplinks) keep the LAN at
0% loss THROUGH "stp on"; auto-edge ports forward after 3 s; a port that
heard real BPDUs (a VM bridge behind physical port 6) correctly declined
auto-edge, sat out the full listen period and became Designated; tc counts
promotions; we win the root election at priority 16384 vs 32768.
(cherry picked from commit 09a34dc6acdc81ab9cab0727d2f4a59c68131a3e)
Add a Spanning Tree page: an on/off toggle driving the existing "stp"
command over /cmd, and a live status section fed by a new /stp.json
endpoint - the elected root bridge (priority + MAC), our path cost,
whether we are the root, and the per-port STP state read live from the
ASIC's MSTP register (same 2-bit encoding stp_setup() writes). Ports are
reported by their physical numbers.
Recovered-from: 3132319, 9365c86
The "do not block the CPU port" mask 0x0f covers bits 3:0 of MSTP_STATES
byte 1, which is ports 8 AND 9 - so stp_setup unconditionally forced
port 8 (a real front port, the SFP uplink on SWTGW218AS) into forwarding
and it could never be blocked. The CPU port alone is bits 3:2 = 0x0c.
"stp on" put every port into blocking (stp_setup, port_timers = "10 s") but
nothing ever counted those timers down: stp_timers() only sent hello BPDUs.
On a network with no other (R)STP bridge - i.e. nobody sends us BPDUs - every
port therefore stayed blocking FOREVER and enabling STP took the whole
network down until "stp off".
- stp_timers(): count port_timers down; when a port's listen period expires
with no better root heard, promote it to forwarding in MSTP_STATES (we are
the designated bridge on that port).
- Calibrate the tick constants to the real stp_timers() rate (~64 Hz: main
loop ~256 Hz / (STP_TICK_DIVIDER+1)): TIME_HELLO 0x200->0x80 is an actual
2 s hello, port_timers 0xa00->0x280 an actual 10 s listen period. Measured
before the fix, ports converged only after ~40 s.
- Move struct bridge into rtl837x_stp.h and export root_bridge/-_cost for
the web UI status endpoint.
Verified on hardware: "stp on" -> ports report Blocking, after the 10 s
listen period all ports promote to Forwarding and LAN connectivity returns;
"stp off" restores forwarding immediately. We elect ourselves root (weRoot)
with no other bridge present.
(cherry picked from commit 8537a15ca254b2122272b20bec7a66426e86df4b)
The always-mapped common area is nearly full (349 bytes free before this
change), and the STP state machine that follows does not fit there. Move
the module to BANK2 next to the other protocol code; its public entry
points are already __banked, and cmpMAC/stp_cnf_send have no callers
outside the file.
The rtl_tag `flags` word (LEARN_DIS, KEEP) and the `pmask` ALLOW-bit
semantics are properties of the RTL8_4 CPU tag, not of any one protocol:
STP injects BPDUs with LEARN_DIS set and LACP emits slow-protocol frames
the same way. Define them once in the shared header, with the HTONS
byte-order caveat documented, so every feature that hand-builds a CPU
tag frame uses the same constants.
(cherry picked from commit 7f905b3e90f9bc5df586a5138723d97edf3d6aaf)
With STP enabled the switch is a participating bridge, so BPDUs must be
consumed, not relayed - yet the reserved group 01:80:C2:00:00:00 was
flooded across the VLAN just like any multicast, leaking every BPDU to
all ports (the same defect class as the LACPDU flood addressed in the
LACP branch, PR #299).
On stp on, write a CPU-only static L2 multicast entry for the BPDU group
per VLAN: BPDUs can arrive VLAN-tagged and classify into the tag's VID,
so cover every VLAN present in the VLAN table plus every port's PVID for
the untagged case.
On stp off the same entries are retargeted to all ports + CPU, restoring
the previous flood behaviour: an unmanaged switch is expected to be
transparent to BPDUs so the surrounding spanning tree can span through
it, and dropping them instead would partition that topology.
Note: with STP enabled the ports start out blocking, which also stops
egress of CPU-originated LACPDUs, so an active LACP aggregate drops
until the ports reach forwarding - a pre-existing interaction, not
changed here.
port_l2mc_set() writes a static L2 multicast entry for a reserved group
01:80:C2:00:00:<last> in a given VLAN with a given member portmask.
Slow-protocol frames must reach the management CPU without being flooded
to other ports, but the RMA "trap" action cannot deliver to the internal
NIC on this hardware - its destination is an external CPU attached to a
physical port. The working alternative is to keep the RMA action at
"forward" and constrain the egress with a static entry: the forward
lookup then hits the entry's member mask instead of the VLAN flood mask.
Hardware-verified on a SWTGW218AS in both directions: a mask without the
CPU bit stops delivery to the CPU, a CPU-only mask delivers with no port
egress. Lookups are IVL, so callers add one entry per VID they care
about; rewriting the same MAC+VID replaces the entry in place.
Used by the BPDU containment in the next commit; the pending LACP branch
adopts it for 01:80:C2:00:00:02 the same way.
The single-digit arm of the ingress parser guards with p - '1' > 9,
which no digit can satisfy: the largest, '9', gives 8. The digit that
needed rejecting is '0', which gives -1 and indexes one byte before
phys_to_log_port, so "ingress 0 t" reads out of bounds and applies the
ingress mode to whatever port number that byte happens to contain.
Ports are 1-based, so reject anything below '1'; values above '9' are
already excluded by the isnumber check before this.
The MAC table listing tests bit 0 of byte 2 of the third table data
word for the static flag, but the flag lives in bit 0 of byte 1: an
entry written with byte 1 bit 0 set survives the aging engine
indefinitely where an identical entry without it ages out, and reads
back with exactly that bit set through both the address and the
next-entry read methods. Byte 2 of that word reads zero for learned and
static entries alike, so every entry has always been listed as learned
and a static entry has never been visible as such in the table listing.
parse_isolate() accepted a two digit port and mapped it to logical port 9,
the CPU port, while port_isolate() and port_isolation_get() both refuse
anything above machine.max_port. Setting the isolation of the CPU port was
therefore declined without a word and reading it always answered no
members, whatever the hardware held.
Bound the port to the front panel, so the command says what it does. The
digit is checked before it indexes phys_to_log_port[], which a non numeric
argument used to read past.
d00f
logicog
René van Dorst
bloqaudio
d00f