From a9af466702fa12ad48c3f44180a891b7761d348e Mon Sep 17 00:00:00 2001 From: d00f Date: Fri, 14 Aug 2026 04:49:33 +0200 Subject: [PATCH] stp: drop the management failsafe 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. --- doc/stp.md | 39 ------------------------------------ html/config.js | 3 +-- html/stp.html | 3 +-- html/stp.js | 7 +------ httpd/httpd.c | 3 --- httpd/page_impl.c | 4 ---- rtl837x_stp.c | 50 +---------------------------------------------- rtl837x_stp.h | 2 -- rtlplayground.c | 1 - 9 files changed, 4 insertions(+), 108 deletions(-) diff --git a/doc/stp.md b/doc/stp.md index f96fb2b..61bb5e5 100644 --- a/doc/stp.md +++ b/doc/stp.md @@ -9,10 +9,6 @@ silent, and blocks a port on which it sees its own BPDU. STP can be enabled and controlled via the web interface or the command line, as follows: -> **Before you enable it on a switch you reach over the network**: read the -> [management failsafe](#management-failsafe) section. The management VLAN -> rides a port that STP can block. - ## Quick start ``` @@ -123,41 +119,6 @@ claims a better priority. on the far side reacts badly to them (some unmanaged switches with loop prevention cut the link) but you still want STP on the rest of the ports. -## Management failsafe - -Enabling STP on a switch you administer over the network is a genuine risk: the -management VLAN rides a port that STP may put into blocking, and once that -happens the way back is a power cycle. - -The firmware therefore runs a commit-confirm watchdog. Enabling STP, by hand or -from the startup config, arms a one-shot window of `stp failsafe ` -(default 180). One HTTP request inside the window confirms that management -survived the new tree and disarms the watchdog until the next enable; a window -with no management activity disables STP and restores forwarding. After the -confirmation STP runs unsupervised, so a quiet network no longer loses its -tree to three minutes of nobody looking at the web UI. - -``` -stp failsafe 180 # length of the armed window after enabling (0 = never armed) -``` - -Any later event that newly takes a port out of forwarding arms the window -again: a port rejoining via `stp port on`, root guard firing, the loop -latch. If management traffic keeps flowing past the new block, the very next -request confirms and disarms; if the block cut it, the silent window restores -forwarding as above. A stable network with nothing newly blocked never re-arms. - -A command executed on the serial console also confirms, on the grounds that an -operator with out-of-band access does not need the automatic restore; the -command that enabled STP does not count, only activity after it. - -A headless switch that nobody confirms over HTTP should set `stp failsafe 0`, -otherwise a reboot with STP in the startup config disables it again three -minutes later. Setting a new value while STP runs arms a fresh window. - -The status page shows whether the failsafe has tripped since STP was last -enabled. - ## Status The Spanning Tree page shows the elected root (priority and MAC), the path cost diff --git a/html/config.js b/html/config.js index e44652c..505f0f3 100644 --- a/html/config.js +++ b/html/config.js @@ -23,7 +23,6 @@ const conf_cmds = [ /^isolate\s+\d{1,2}(\s+(off|\d{1,2}))+$/, /^stp\s+(on|off)$/, /^stp\s+(prio|hello|maxage|fwd|txhold)\s+\d{1,2}$/, - /^stp\s+failsafe\s+\d{1,3}$/, /^stp\s+version\s+(rstp|stp)$/, /^stp\s+port\s+\d{1,2}\s+(on|off)$/, /^stp\s+port\s+\d{1,2}\s+edge\s+(on|off|auto)$/, @@ -56,7 +55,7 @@ const conf_overwrite = [ /^lag\s+\d+\b/, /^laghash\b/, /^isolate\s+\d{1,2}\b/, - /^stp\s+(prio|hello|maxage|fwd|txhold|version|failsafe)\b/, + /^stp\s+(prio|hello|maxage|fwd|txhold|version)\b/, /^stp\s+port\s+\d{1,2}\s+(edge|cost|prio|guard|filter|p2p)\b/, /^igmp\b/, /^mtu\s+\d{1,2}\b/, diff --git a/html/stp.html b/html/stp.html index 5bdbeb5..59a98e4 100644 --- a/html/stp.html +++ b/html/stp.html @@ -16,7 +16,7 @@

Bridge settings

- + @@ -25,7 +25,6 @@ -
PriorityVersionHello [s]Max age [s]Fwd delay [s]Tx holdMgmt failsafe [s]PriorityVersionHello [s]Max age [s]Fwd delay [s]Tx hold

Changes apply immediately. Edge ports skip the listen period; guard/filter act on received BPDUs.

diff --git a/html/stp.js b/html/stp.js index ec73ee7..acb59e9 100644 --- a/html/stp.js +++ b/html/stp.js @@ -99,9 +99,7 @@ function fetchStp() { const s = JSON.parse(xhttp.responseText); if (!stpRows) buildPortsTable(s.ports); - document.getElementById("stpStat").textContent = s.fsT - ? "\u26a0 STP was disabled by the management failsafe (ports were blocked while management was unreachable). Review the topology before re-enabling." - : s.on + document.getElementById("stpStat").textContent = s.on ? (s.weRoot ? "This switch (" + bridgeSelf(s) + ") is the root bridge — topology changes: " + parseInt(s.tc, 16) @@ -133,7 +131,6 @@ function fetchStp() { document.getElementById("bMaxage").value = s.maxage; document.getElementById("bFwd").value = s.fwd; document.getElementById("bTxhold").value = s.txhold; - document.getElementById("bFailsafe").value = s.fs; for (const p of s.ports) { document.getElementById("en_" + p.p).value = (p.f & PF_ENABLED) ? "on" : "off"; document.getElementById("edge_" + p.p).value = @@ -179,8 +176,6 @@ window.addEventListener("load", function() { .addEventListener("change", e => stpCmd("stp fwd " + e.target.value)); document.getElementById("bTxhold") .addEventListener("change", e => stpCmd("stp txhold " + e.target.value)); - document.getElementById("bFailsafe") - .addEventListener("change", e => stpCmd("stp failsafe " + e.target.value)); document.getElementById("stpMode") .addEventListener("change", () => { stpDirty = true; }); diff --git a/httpd/httpd.c b/httpd/httpd.c index bc98013..9681caf 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -22,7 +22,6 @@ extern volatile __xdata uint8_t sfr_data[4]; extern volatile __xdata uint32_t ticks; /* 200 Hz free-running tick, owned by rtlplayground.c */ -volatile __xdata uint8_t mgmt_alive; /* consumed by the STP management failsafe */ extern __code uint8_t * __code hex; extern __code struct f_data f_data[]; extern __code char * __code mime_strings[]; @@ -551,8 +550,6 @@ void httpd_appcall(void) __xdata struct httpd_state * __xdata s = &(uip_conn->appstate); dbg_char('P'); - if (uip_newdata()) - mgmt_alive = 1; /* any HTTP activity proves management still works (STP failsafe) */ #ifdef DEBUG if (uip_newdata()) write_char('N'); diff --git a/httpd/page_impl.c b/httpd/page_impl.c index ac389e8..a53362e 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -582,10 +582,6 @@ void send_stp(void) itoa_html(stp_fwddelay_s); slen += strtox(outbuf + slen, ",\"txhold\":"); itoa_html(stp_txhold); - slen += strtox(outbuf + slen, ",\"fs\":"); - itoa_html(stp_failsafe_s); - slen += strtox(outbuf + slen, ",\"fsT\":"); - itoa_html(stp_failsafe_tripped); slen += strtox(outbuf + slen, ",\"rootPrio\":\""); byte_to_html(root_bridge.prio); byte_to_html(root_bridge.ext); diff --git a/rtl837x_stp.c b/rtl837x_stp.c index 40e5bd4..1416553 100644 --- a/rtl837x_stp.c +++ b/rtl837x_stp.c @@ -46,11 +46,6 @@ __xdata uint8_t stp_fwddelay_s; __xdata uint8_t stp_rstp; __xdata uint8_t stp_txhold; -__xdata uint8_t stp_failsafe_s; -__xdata uint8_t stp_failsafe_cnt; /* seconds left of the armed window */ -__xdata uint8_t stp_failsafe_armed; -__xdata uint8_t stp_failsafe_tripped; -extern volatile __xdata uint8_t mgmt_alive; /* set by httpd on any request */ __xdata uint8_t stp_pflags[10]; __xdata uint32_t stp_pcost[10]; @@ -180,9 +175,7 @@ static void stp_status(void) } print_string("changes "); print_short(stp_tc_count); - print_string(" failsafe "); - itoa(stp_failsafe_s); - print_string(stp_failsafe_tripped ? "s TRIPPED\n" : "s\n"); + write_char('\n'); print_string("port state role edge\n"); reg_read_m(RTL837X_MSTP_STATES); for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) { @@ -563,29 +556,6 @@ void stp_timers(void) __banked for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) stp_tx_budget[stp_i] = stp_txhold; - /* Management failsafe: armed as a one-shot window by "stp on". - * The first HTTP request inside the window proves management - * survived the new tree and disarms it; a silent window disables - * STP. Deliberately NOT conditioned on our own MSTP states: - * hardware incident 2026-07-21 showed a NEIGHBOR (TP-Link Easy - * Smart loop prevention) cutting our uplink in reaction to our - * BPDUs while our ASIC was all-forwarding - only going fully - * quiet (no BPDU TX) lets such a neighbor recover. */ - if (stp_failsafe_armed) { - if (mgmt_alive) { - stp_failsafe_armed = 0; - print_string("STP failsafe: management confirmed - disarmed\n"); - } else if (--stp_failsafe_cnt == 0) { - print_string("STP failsafe: no management activity - disabling STP\n"); - stp_failsafe_armed = 0; - stp_off(); - stpEnabled = 0; - stp_failsafe_tripped = 1; - return; - } - } - mgmt_alive = 0; - /* Link supervision. Without this the state machine never learns * that a port lost carrier: it keeps the port in forwarding, keeps * announcing on it, and never flushes what was learned behind it - @@ -685,8 +655,6 @@ void stp_defaults(void) __banked stp_fwddelay_s = 15; stp_rstp = 1; stp_txhold = 6; - stp_failsafe_s = 180; - stp_failsafe_tripped = 0; for (stp_i = 0; stp_i < 10; stp_i++) { /* enabled, auto-edge on: host-facing ports go forwarding after * 3 s of BPDU silence instead of the full forward delay */ @@ -798,10 +766,6 @@ void stp_parse(void) __banked __reentrant { if (cmd_compare(1, "on")) { print_string("STP enabled\n"); - stp_failsafe_tripped = 0; - stp_failsafe_cnt = stp_failsafe_s; - stp_failsafe_armed = (stp_failsafe_s && save_cmd) ? 1 : 0; - mgmt_alive = 0; stpEnabled = 1; stp_setup(); return; @@ -810,7 +774,6 @@ void stp_parse(void) __banked __reentrant print_string("STP disabled\n"); stp_off(); stpEnabled = 0; - stp_failsafe_armed = 0; return; } if (cmd_compare(1, "status")) { @@ -838,11 +801,6 @@ void stp_parse(void) __banked __reentrant if (stpEnabled) { /* (re)join: listen first */ stp_state_set(port, 0b01); port_timers[port] = (uint16_t)stp_fwddelay_s * STP_HZ; - if (stp_failsafe_s && save_cmd) { - stp_failsafe_armed = 1; - stp_failsafe_cnt = stp_failsafe_s; - mgmt_alive = 0; - } } } else if (cmd_compare(3, "off")) { stp_pflags[port] &= ~STP_PF_ENABLED; @@ -944,12 +902,6 @@ void stp_parse(void) __banked __reentrant if (stp_scratch < 1 || stp_scratch > 10) goto err; stp_txhold = stp_scratch; - } else if (cmd_compare(1, "failsafe")) { - /* 0 never arms; otherwise the length of the armed window */ - stp_failsafe_s = stp_scratch; - stp_failsafe_cnt = stp_scratch; - stp_failsafe_armed = (stp_scratch && stpEnabled && save_cmd) ? 1 : 0; - mgmt_alive = 0; } else { goto err; } diff --git a/rtl837x_stp.h b/rtl837x_stp.h index fb534ae..a8e68a8 100644 --- a/rtl837x_stp.h +++ b/rtl837x_stp.h @@ -27,8 +27,6 @@ extern __xdata uint8_t stp_maxage_s; /* max age, 6-40 s (default 20) */ extern __xdata uint8_t stp_fwddelay_s; /* forward delay, 4-30 s (default 15); our listen period */ extern __xdata uint8_t stp_rstp; /* 1 = RSTP BPDUs (v2), 0 = STP-compatible Config BPDUs (v0) */ extern __xdata uint8_t stp_txhold; /* max BPDUs per port per second (default 6) */ -extern __xdata uint8_t stp_failsafe_s; /* mgmt watchdog, seconds (0 = off) */ -extern __xdata uint8_t stp_failsafe_tripped; /* Per-port config/status flags (stp_pflags[]) */ #define STP_PF_ENABLED 0x01 /* port participates in STP (default on) */ diff --git a/rtlplayground.c b/rtlplayground.c index b24f8f5..6e2d9df 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -122,7 +122,6 @@ __xdata uint8_t tx_seq; __xdata uint8_t stpEnabled; __xdata uint8_t igmpEnabled; -extern __xdata uint8_t stp_failsafe_armed; __xdata char hostname[24]; /* device hostname, default set at boot, see rtl837x_common.h */ __code uint16_t bit_mask[16] = {