This branch moved rtl837x_pins to the common window in eaff953 to reclaim
BANK2 for the per-port status work, which was free at the time because the
module was only the I2C and GPIO pin helpers.
main has since put the SFP EEPROM transfer there, so the module now weighs
934 bytes and the common window is 156 bytes short. BANK2 has room for it
again, and that is where main keeps it.
Common segment +934 bytes, BANK2 -934.
Only httpd.c conflicted: main added the buffered configuration upload
next to the pointers this branch had moved into xdata to free internal
RAM. Both belong, so the new globals sit above declarations that keep
their storage class.
The I2C controller transfers up to sixteen bytes per transaction and
page_impl.c already used that for sfp_send_data(), while sfp_read_reg()
asked for one byte and every caller looped. Reading a module therefore
cost one address phase per byte: 87 transactions when a module is
inserted, 52 for the sfp command, 36 for the vendor block in status.json.
sfp_read_block() replaces sfp_read_reg() and the callers that already
wanted a run of registers ask for it once: the vendor fields as three
16 byte pages, the diagnostics as one transfer, rate and encoding
together. That drops the three paths above to 8, 6 and 3 transactions,
and sfp_send_data() loses its copy of the transfer.
The vendor loops now run over 16..63 rather than 20..59 so the page base
is a multiple of sixteen and the index into the buffer is a single AND.
The four extra bytes at each end are read and discarded. The diagnostics
read asks for 16 bytes rather than the 15 it uses, because 16 is a width
the shipped firmware already exercises and 15 is not.
The device address, the bus selection and the start bit go into the
control register in one write now that the memory address is written
first, so a transfer touches that register once instead of three times.
The register reads take their result from the SFRs directly rather than
through the sfr_data mirror. The result is a bool and the destination is
sfp_buf, so a caller that cares about a failed transfer looks at the
return value instead of a flag.
Every caller gives up on the first failed read rather than carrying a
flag to the end, which is why the module read moved out of handle_sfp
into a function of its own. A module whose read fails is left marked as
absent, so the next poll retries it instead of configuring the SerDes
from bytes that never arrived.
BANK1 -194 bytes, BANK2 +382, common segment +44, xdata +15 for the
buffer, and one byte more of internal RAM free than before the series.
Built for all 25 machine definitions on sdcc 4.5.0; the tightest common
segment is 98 bytes free on SWTG024AS_V2_0, against 54 before this
series.
CONFIG_UPLOAD_BUF at 2560 capped a configuration upload at about 2.2K
while the config sector holds 4K. Size the buffer as CONFIG_LEN plus
room for the multipart framing so the whole sector is usable.
config_take() wrote the payload without checking it against the sector:
safe before only because the buffer could not hold an oversized one.
Reject a payload that does not fit CONFIG_LEN, terminator included,
instead of writing past the erased sector.
send_config() streams a configuration larger than the TCP output buffer
through the cont_addr/cont_len continuation, but set cont_addr to the
offset within the config instead of a flash address, so the tail was read
from code space. The file server sets cont_addr absolute; do the same by
adding CONFIG_START.
The bug was unreachable while CONFIG_UPLOAD_BUF capped uploads below
TCP_OUTBUF_SIZE, but a stored config near the full sector exposes it.
sfp_read_reg() sat in rtlplayground.c, so it occupied the common 16 KB
window that every bank shares, even though nothing outside the SFP paths
calls it. That window is the tightest resource in the image:
SWTG024AS_V2_0 and SWTG024AS_A_2_0_1_5C_1SFP had 54 bytes left in it.
rtl837x_pins.c is already in BANK2 and already holds the I2C bus helpers
this function calls, so the transfer belongs there. The function moves
verbatim and becomes __banked; the prototype in rtl837x_common.h says so,
which is what keeps the callers in BANK1 and BANK2 honest.
No behaviour change. The common segment gains 200 bytes on every machine:
159 to 359 free on SWTGW218AS, 54 to 254 on the two variants above.
BANK1 +6 bytes, BANK2 +336. Built on sdcc 4.5.0.
Saving the configuration works in Chrome and fails in Firefox, and the
difference is only how each browser splits the request. The handler
erased the config sector as soon as the request started and then parsed
the multipart body one TCP segment at a time, which requires every
boundary and every part header to fall inside a single segment. Firefox
splits inside a part header, so the parser lost its place and the
erased sector was left holding a truncated body or nothing at all. A
single-burst scripted post lost the whole body the same way.
The configuration is limited to two kilobytes, so the whole request body
now accumulates in xdata and is parsed only after the closing boundary
has arrived. The parts are walked in one pass, the part carrying a
filename is written to a freshly erased sector, and the client receives
a 200 instead of the previous silent close. No segmentation can confuse
this, since the parser only ever sees a complete body.
Locating the closing boundary first also bounds the walk over the parts,
since none can lie beyond it, so the length of the buffer is no longer
the bound and the test for the two trailing dashes is unnecessary.
The walk matches at offsets inside a buffer that is not terminated, so
neither existing helper fits: strcmp() goes on to compare the byte after
the match and is_word_x() demands a separator there. Add strstart() and
strstart_x() for that case, and use strlen_x() for the boundary length.
The firmware upload path still streams, since a megabyte cannot be
buffered, and is untouched.
The bounded waits were silent: on timeout the code went straight back to
its caller, and handle_rx() then read a frame the DMA may never have
delivered, which is worse than waiting longer.
Each of the three transfers now says so on the console, the two RX ones
report failure to handle_rx(), and handle_rx() acknowledges the packet
and gives up on it rather than parsing whatever is in the buffer. The
guard variable moved to the top of its function, so the block that held
it and its indentation are gone.
The comment said the ASIC never completes a TX when the egress port is in a
non-forwarding MSTP state. The guard is worth keeping either way, since an
unbounded spin in the DMA wait takes the whole main loop down, but the
mechanism is more than the evidence supports and the DMA into the TX ring
has no business knowing the egress port at all.
The variable lives in rtlplayground.c, so it is declared in rtl837x_common.h
with the others there, and it follows the naming of the rest.
The header carried comments on the externs that the definitions in
rtl837x_stp.c repeat, sometimes differently, which is one place too many to
keep in step. What only the header had, the value ranges and what the
designated arrays hold, moved to the definitions; the rest is gone. Function
declarations lost their comments too.
The status printer only prints, so its running commentary went. A define
replaces the bare 33 in stp_in(), and the note on the loop check is down to
what applies at that line.
A blocked port does pass a received BPDU up to the CPU. The evidence is in
logicog's capture of a looped pair: the port the loop check had already
blocked kept reporting a BPDU age of zero seconds across dumps taken more
than a forward delay apart, and that counter is only cleared in stp_in().
That is also what makes the loop latch work, since the port that stays
forwarding has to go on hearing the blocked one.
The tag flag comment said the source address is not learned on the egress
port. doc/CpuPort.md defines it as not learning the source address from the
frame at all, which is the narrower claim to make. The dangling heading
above it described a field that is documented there too.
802.1D puts the port's own address in the source field and the bridge
address only in the Bridge Identifier. We used the bridge address for
both, and on this hardware that costs the management path.
Measured on a SWTGW218AS: the ASIC learns the source address of a frame
addressed to 01:80:c2:00:00:00, and the bridge's own address is not
exempt. A BPDU that leaves a blocked port and comes back on a forwarding
one therefore moves the management address off the CPU port, and frames
for it are then sent down that port instead of to the CPU. Traffic
between other stations is unaffected, which is what makes it look like
the CPU port has been blocked.
The derived address keeps the bridge address and sets the locally
administered bit, so it differs from the bridge address in the first
octet for any globally assigned OUI, with the port number in the low
nibble of the last octet. Nothing here reads the source address of a
received BPDU; the loop check compares the Bridge Identifier.
Declaring port at the top of stp_in() and stp_parse() does the same job
without a block that is not indented like one. The static xdata copy in
stp_in() went with it, it was only ever written.
The argument count check in stp_parse() that lost its comment guards
against cmd_compare(4, ..) reading a stale word from the previous command
line, because cmd_words_b is not cleared between commands.
tcpip_output() splices the 802.1Q tag in right behind the source address,
which is exactly where the ASIC expects the RTL tag of a frame the CPU
addressed to a port itself. The tag then ends up behind the VLAN tag, the
ASIC does not find it, and the frame goes out flooded with the 0x8899
header still on it instead of being sent to the port that was asked for.
Whether a frame is CPU-tagged is a property of the frame, so decide it
here from the ether-type rather than having every sender of such a frame
clear management_vlan around its tcpip_output() call. stp_cnf_send() did
that, and no longer has to.
The comments they replace are gone with them. Two things the comments
carried that the names do not:
Accepting version >= 2 rather than == 2 is deliberate. 802.1D-2004 14.4
has an RSTP bridge accept a higher Protocol Version and treat it as RST,
and MSTP sends version 3 type 2 with a prefix identical to an RST BPDU
for exactly that reason, so insisting on == 2 would make us blind to
every MST bridge on the segment.
In the TCN branch stp_cnf_send() transmits by itself, so uip_len is
cleared afterwards to keep handle_rx() from sending the frame twice.
Worth keeping out of the code but on record: RTL_TAG_KEEP is deliberately
not set here. On an LLC/802.3 frame the ASIC drops the frame outright with
that flag, while on ethertype frames such as LACP it works fine.
The port number comes out of a received BPDU, so the range check is
there to keep a forged frame from naming a port this module does not
manage - including the CPU port, which would cost us the management
path. Nothing outside min_port..max_port would ever release the block
either, because stp_timers() only counts down the ports it walks.
The three xdata bytes and the reused stp_we_root scratch are gone; the
compiler needs a register for an index either way. Frees four bytes of
xdata and 29 bytes of code.
The tree structure already crossed the switch by regeneration, but the
topology-change information did not: a received TC flag was ignored and
a TCN only acknowledged, so bridges behind this one kept stale entries
until normal aging.
A TC flag in a received BPDU now flushes the other non-edge ports once
and arms the transmit window our BPDUs already copy the flag from,
refreshed to hello+1 seconds by every further flagged frame so it ends
one hello after the neighbour stops, without shortening the longer
window a local change arms. A TCN is acknowledged as before and then
treated like a local change on that port.
The table could say a port was designated and had heard nothing, which is
two different situations wearing the same face: either we are not
announcing on that segment, or we are and nobody is answering. Telling them
apart needed a capture on the far side.
port state role edge tx bpdu
05 fwd desg yes 2a 255
01 block desg no 2a 21
03 fwd root no 00 0
The tx column counts BPDUs actually handed to the hardware, so it moves
only past the enable, filter and tx hold checks in stp_cnf_send(). A
designated port has to show it climbing once per hello time. The root port
never does, because we do not announce back towards the root, so a
neighbour that has taken us as root falls silent in both directions on that
link and the two columns together say exactly that rather than looking like
a fault.
The counter is a byte and wraps at 256. It is meant to be watched moving,
not summed, and it starts again when STP is enabled.
67 bytes of BANK2 and 10 of xdata, nothing in BANK1 or internal RAM. Built
for SWTGW218AS and KP_9000_6XHML_X2 on sdcc 4.5.0.
The table printed the ASIC's raw two bit state, a 1 or a 2 for the role and
a 1 or a 0 for the edge flag, so reading it meant having the source open
next to the console. The columns carry the words now:
port state role edge bpdu
05 fwd desg yes 255
01 block desg no 21
02 learn desg no 5
03 fwd root no 0
They come from fixed width tables indexed by the same values as before, so
nothing about how any of the three is derived changes, and the columns line
up under the header without a formatter.
The role column still only tells the root port from everything else,
because that is all the state machine tracks. A port sitting in blocking
because a better BPDU arrived on it reads as designated here. Naming the
column makes that visible rather than introducing it.
154 bytes of BANK2, nothing in BANK1, xdata or internal RAM. Built for
SWTGW218AS and KP_9000_6XHML_X2 on sdcc 4.5.0.
The status output named state, role and edge, none of which separates a
port nobody is speaking (R)STP to from a port whose BPDUs we are
dropping. Both look the same: forwarding, designated, edge, and the tree
rooted at ourselves. stp_in() leaves on eight different conditions, from
a short frame through an unexpected LLC header to a disabled port, and
none of them says anything.
stp_bpdu_age was already maintained for the ageing rules, so this only
prints it, in seconds and capped at 255. A column that counts up means
nothing is arriving; a column that stays near zero means frames are
arriving and any disagreement about the tree is ours.
Eighty two bytes of BANK2 and two of xdata, most of it the sixteen bit
divide by the tick rate. It comes out of a branch that gives back three
hundred and twenty eight, so it is affordable, and printing raw ticks to
save it would put the reader back to converting in their head.
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.
d00f
logicog
feelfree69
René van Dorst
bloqaudio
d00f