From 02c816647c7914559b4ddfa02ea0be9e2a49ddbb Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:26:39 +0200 Subject: [PATCH 1/4] stp: treat a link aggregation group as one entity Spanning tree so far saw only physical ports. Two ports aggregated towards the same neighbour look to the protocol like two parallel paths, so it blocks one of them and cuts the group in half. Give each of the four groups an STP entity of its own, numbered above the physical ports. A member port answers to its group: BPDUs arriving on it are accounted to the entity, the entity's state is written to every member together, and its BPDUs carry the state of a live member. Ports outside any group keep behaving exactly as before. Membership comes from the aggregation tables once a second; when it changed, the entity's state is reapplied to the new set of members. The CLI takes lag <1-4> wherever it takes port , the web UI shows a group as one row, and the saved config keeps the lag form of the commands. --- doc/stp.md | 22 ++++ httpd/page_impl.c | 36 +++++- rtl837x_stp.c | 296 ++++++++++++++++++++++++++++++++++++++-------- rtl837x_stp.h | 24 ++-- 4 files changed, 319 insertions(+), 59 deletions(-) diff --git a/doc/stp.md b/doc/stp.md index 49bedd36..c6ff6598 100644 --- a/doc/stp.md +++ b/doc/stp.md @@ -58,6 +58,28 @@ port set to admit tagged frames only (`ingress t`) never delivers one to the CPU. `stp_setup()` prints a warning for every STP-enabled port in that state. +## Link aggregation + +A LAG is a port to the protocol, the way the vendor firmware presents it as +Trk1 to Trk4. The four groups are entities alongside the nine physical ports: +they carry their own path cost, priority, edge and guard settings, they hold +their own timers, and they appear as their own rows on the Spanning Tree page. + +``` +stp lag 1 cost 10000 # the group decides, not its members +stp lag 1 edge off +``` + +Membership comes from the aggregation registers the `lag` command writes, re-read +once a second, so a group changing under LACP is picked up without any +coordination between the two. A member port is not an STP port of its own: +`stp port ` on a member says which group to configure instead. State +changes are written to every member in one register write, since the hardware +has no per-group state, and BPDUs are transmitted through the lowest +member while carrying the group's own port id. Losing one member of a live +LAG is not a topology change; the logical port only goes down with its last +link. + Port states live in `RTL837X_MSTP_STATES (0x5310)`, two bits per port: `00` disabled, `01` blocking, `10` learning, `11` forwarding. A port in blocking forwards nothing between ports, but it still sends what the CPU diff --git a/httpd/page_impl.c b/httpd/page_impl.c index 1d725626..ec7ef26a 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -607,17 +607,43 @@ void send_stp(void) slen += strtox(outbuf + slen, "\",\"weRoot\":"); bool_to_html(stp_root_port == 0xff ? 1 : 0); slen += strtox(outbuf + slen, ",\"rootPort\":"); - itoa_html(stp_root_port == 0xff ? 0 : machine.log_to_phys_port[stp_root_port]); + j = stp_root_port; + if (j != 0xff && j >= STP_LAG_BASE) { + j = 0; + while (j < STP_LAG_BASE && !((stp_lag_mask[stp_root_port - STP_LAG_BASE] >> j) & 1)) + j++; + if (j >= STP_LAG_BASE) + j = 0; + } + itoa_html(j == 0xff ? 0 : machine.log_to_phys_port[j]); slen += strtox(outbuf + slen, ",\"tc\":\""); byte_to_html(stp_tc_count >> 8); byte_to_html(stp_tc_count); slen += strtox(outbuf + slen, "\",\"ports\":["); reg_read_m(RTL837X_MSTP_STATES); - for (i = machine.min_port; i <= machine.max_port; i++) { + for (i = 0; i < STP_ENTITIES; i++) { + if (i < STP_LAG_BASE && (i < machine.min_port || i > machine.max_port)) + continue; + if (i < STP_LAG_BASE && stp_ent_of[i] != i) + continue; + if (i >= STP_LAG_BASE && !stp_lag_mask[i - STP_LAG_BASE]) + continue; slen += strtox(outbuf + slen, "{\"p\":"); - itoa_html(machine.log_to_phys_port[i]); + itoa_html(i < STP_LAG_BASE ? machine.log_to_phys_port[i] : 0); + slen += strtox(outbuf + slen, ",\"lag\":"); + itoa_html(i < STP_LAG_BASE ? 0 : i - STP_LAG_BASE + 1); + slen += strtox(outbuf + slen, ",\"mbr\":"); + itoa16_html(i < STP_LAG_BASE ? 0 : stp_lag_mask[i - STP_LAG_BASE]); slen += strtox(outbuf + slen, ",\"st\":"); - st = (sfr_data[3 - (i >> 2)] >> ((i << 1) & 0x7)) & 0x3; + j = i; + if (i >= STP_LAG_BASE) { + j = 0; + while (j < STP_LAG_BASE && !((stp_lag_mask[i - STP_LAG_BASE] >> j) & 1)) + j++; + if (j >= STP_LAG_BASE) + j = 0; + } + st = (sfr_data[3 - (j >> 2)] >> ((j << 1) & 0x7)) & 0x3; itoa_html(st); slen += strtox(outbuf + slen, ",\"role\":"); if (!(stp_pflags[i] & STP_PF_ENABLED) || (stp_pflags[i] & STP_PF_TRIPPED)) @@ -658,6 +684,8 @@ void send_stp(void) } + + void send_eee(void) { dbg_string("send_eee called\nsending EEE status\n"); diff --git a/rtl837x_stp.c b/rtl837x_stp.c index e33dbb4a..20f530d8 100644 --- a/rtl837x_stp.c +++ b/rtl837x_stp.c @@ -47,31 +47,36 @@ __xdata uint8_t stp_rstp; /* 1 = RST BPDUs, 0 = legacy Config BPDUs */ __xdata uint8_t stp_txhold; /* BPDUs per port per second */ -__xdata uint8_t stp_pflags[10]; -__xdata uint32_t stp_pcost[10]; /* 0 = auto */ -__xdata uint8_t stp_pprio[10]; -__xdata uint8_t stp_pp2p[10]; /* admin point-to-point: 0 auto, 1 on, 2 off */ +__xdata uint8_t stp_ent_of[10]; +__xdata uint16_t stp_lag_mask[STP_LAG_COUNT]; +__xdata uint8_t stp_ss_i; +__xdata uint8_t stp_map_dirty; +__xdata uint8_t stp_pflags[STP_ENTITIES]; +__xdata uint32_t stp_pcost[STP_ENTITIES]; /* 0 = auto */ +__xdata uint8_t stp_pprio[STP_ENTITIES]; +__xdata uint8_t stp_pp2p[STP_ENTITIES]; /* admin point-to-point: 0 auto, 1 on, 2 off */ /* Designated bridge, port and cost last heard on the port; stp_bpdu_age tells * whether they are still current. */ -__xdata struct bridge stp_dbridge[10]; -__xdata uint16_t stp_dpid[10]; -__xdata uint32_t stp_dcost[10]; +__xdata struct bridge stp_dbridge[STP_ENTITIES]; +__xdata uint16_t stp_dpid[STP_ENTITIES]; +__xdata uint32_t stp_dcost[STP_ENTITIES]; /* ---- Status / runtime ---- */ __xdata struct bridge root_bridge; __xdata uint32_t root_bridge_cost; /* our cost to the root (rx cost + root port cost) */ __xdata uint8_t stp_root_port; /* 0xff = we are the root */ __xdata uint16_t stp_tc_count; -__xdata uint16_t stp_scratch16; /* scratch for status printing only */ +__xdata uint16_t stp_scratch16; /* scratch for status printing and the lag map */ +__xdata uint8_t stp_st_of; -__xdata uint16_t port_timers[10]; /* listen-period countdown (0 = not listening) */ -__xdata uint16_t port_hello[10]; /* hello TX countdown */ -__xdata uint16_t stp_bpdu_age[10]; /* ticks since last BPDU seen on port (saturating) */ -__xdata uint8_t stp_loop_held[10]; /* port is out of forwarding because a loop was seen on it */ -__xdata uint8_t stp_tx_budget[10]; /* tx hold: BPDUs left in the current second */ -__xdata uint8_t stp_tx_count[10]; /* BPDUs actually put on the wire, wraps at 256 */ +__xdata uint16_t port_timers[STP_ENTITIES]; /* listen-period countdown (0 = not listening) */ +__xdata uint16_t port_hello[STP_ENTITIES]; /* hello TX countdown */ +__xdata uint16_t stp_bpdu_age[STP_ENTITIES]; /* ticks since last BPDU seen on port (saturating) */ +__xdata uint8_t stp_loop_held[STP_ENTITIES]; /* port is out of forwarding because a loop was seen on it */ +__xdata uint8_t stp_tx_budget[STP_ENTITIES]; /* tx hold: BPDUs left in the current second */ +__xdata uint8_t stp_tx_count[STP_ENTITIES]; /* BPDUs actually put on the wire, wraps at 256 */ __xdata uint16_t stp_sec_tick; /* 1 s window for the tx budget */ __xdata uint16_t stp_link_prev; /* carrier bitmap as of the last check */ __xdata uint16_t stp_link_now; @@ -164,7 +169,12 @@ struct stp_pkt_in { /* Console messages name the port on the front panel, not the internal index. */ static void print_port_nl(uint8_t port) __reentrant { - print_byte(machine.log_to_phys_port[port]); + if (port >= STP_LAG_BASE) { + write_char('L'); + write_char('1' + port - STP_LAG_BASE); + } else { + print_byte(machine.log_to_phys_port[port]); + } write_char('\n'); } @@ -184,6 +194,8 @@ static __code const char stp_state_txt[] = "off blocklearnfwd "; static __code const char stp_role_txt[] = "desgroot"; static __code const char stp_edge_txt[] = "no yes "; +static uint8_t stp_ent_active(uint8_t e) __reentrant; + static void print_field(__code const char *txt, uint8_t idx, uint8_t width) __reentrant { txt += idx * width; @@ -207,7 +219,12 @@ static void stp_status(void) print_string(" (this switch)\n"); } else { print_string(" port "); - print_byte(machine.log_to_phys_port[stp_root_port]); + if (stp_root_port >= STP_LAG_BASE) { + write_char('L'); + write_char('1' + stp_root_port - STP_LAG_BASE); + } else { + print_byte(machine.log_to_phys_port[stp_root_port]); + } print_string(" cost "); print_long(root_bridge_cost); write_char('\n'); @@ -217,11 +234,26 @@ static void stp_status(void) write_char('\n'); print_string("port state role edge tx bpdu\n"); reg_read_m(RTL837X_MSTP_STATES); - for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) { + for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) { + if (!stp_ent_active(stp_i)) + continue; write_char(' '); - print_byte(machine.log_to_phys_port[stp_i]); + if (stp_i >= STP_LAG_BASE) { + write_char('L'); + write_char('1' + stp_i - STP_LAG_BASE); + } else { + print_byte(machine.log_to_phys_port[stp_i]); + } print_string(" "); - print_field(stp_state_txt, (sfr_data[3 - (stp_i >> 2)] >> ((stp_i << 1) & 0x7)) & 0x3, 5); + stp_st_of = stp_i; + if (stp_i >= STP_LAG_BASE) { + stp_st_of = 0; + while (stp_st_of < 10 && !((stp_lag_mask[stp_i - STP_LAG_BASE] >> stp_st_of) & 1)) + stp_st_of++; + if (stp_st_of >= 10) + stp_st_of = 0; + } + print_field(stp_state_txt, (sfr_data[3 - (stp_st_of >> 2)] >> ((stp_st_of << 1) & 0x7)) & 0x3, 5); write_char(' '); print_field(stp_role_txt, stp_i == stp_root_port ? 1 : 0, 4); write_char(' '); @@ -266,18 +298,96 @@ int8_t cmpBytes(__xdata uint8_t *m1, __xdata uint8_t *m2, uint8_t n) __reentrant } -/* Write one port's 2-bit state into the ASIC's MSTP register. - * 00 disable, 01 blocking, 10 learning, 11 forwarding. */ +static void stp_lag_map(void) +{ + for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) + stp_ent_of[stp_ss_i] = stp_ss_i; + for (stp_scratch = 0; stp_scratch < STP_LAG_COUNT; stp_scratch++) { + stp_scratch16 = port_lag_members_get(stp_scratch); + if (stp_lag_mask[stp_scratch] != stp_scratch16) + stp_map_dirty = 1; + stp_lag_mask[stp_scratch] = stp_scratch16; + for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) + if ((stp_lag_mask[stp_scratch] >> stp_ss_i) & 1) + stp_ent_of[stp_ss_i] = STP_LAG_BASE + stp_scratch; + } +} + + +static uint8_t stp_ent_active(uint8_t e) __reentrant +{ + if (e >= STP_ENTITIES) + return 0; + if (e >= STP_LAG_BASE) + return stp_lag_mask[e - STP_LAG_BASE] != 0; + return e >= machine.min_port && e <= machine.max_port && stp_ent_of[e] == e; +} + + +static uint8_t stp_ent_has(uint8_t ent, uint8_t p) __reentrant +{ + if (ent < STP_LAG_BASE) + return ent == p; + return (stp_lag_mask[ent - STP_LAG_BASE] >> p) & 1; +} + + +static uint8_t stp_state_port(uint8_t e) __reentrant +{ + if (e < STP_LAG_BASE) + return e; + for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) + if ((stp_lag_mask[e - STP_LAG_BASE] >> stp_ss_i) & 1) + return stp_ss_i; + return 0; +} + + +static void stp_state_bits(uint8_t port, uint8_t state) __reentrant +{ + for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) + if (stp_ent_has(port, stp_ss_i)) + sfr_data[3 - (stp_ss_i >> 2)] |= (uint8_t)(state << ((stp_ss_i << 1) & 0x7)); +} + + +/* MSTP register encoding: 00 disable, 01 blocking, 10 learning, 11 forwarding. */ static void stp_state_set(uint8_t port, uint8_t state) __reentrant { reg_read_m(RTL837X_MSTP_STATES); - stp_scratch = 3 - (port >> 2); - sfr_data[stp_scratch] &= ~(uint8_t)(0b11 << ((port << 1) & 0x7)); - sfr_data[stp_scratch] |= (uint8_t)(state << ((port << 1) & 0x7)); + for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) { + if (!stp_ent_has(port, stp_ss_i)) + continue; + stp_scratch = 3 - (stp_ss_i >> 2); + sfr_data[stp_scratch] &= ~(uint8_t)(0b11 << ((stp_ss_i << 1) & 0x7)); + sfr_data[stp_scratch] |= (uint8_t)(state << ((stp_ss_i << 1) & 0x7)); + } reg_write_m(RTL837X_MSTP_STATES); } +static void stp_ent_apply(uint8_t e) __reentrant +{ + if (!(stp_pflags[e] & STP_PF_ENABLED)) { + stp_state_set(e, 0b11); + return; + } + if (stp_pflags[e] & STP_PF_TRIPPED) { + stp_state_set(e, 0b00); + return; + } + if (stp_pflags[e] & STP_PF_ADMEDGE) { + stp_pflags[e] |= STP_PF_OPEREDGE; + stp_state_set(e, 0b11); + return; + } + if (port_timers[e]) + stp_state_set(e, 0b01); + else + stp_state_set(e, ((stp_link_prev >> e) & 1) ? 0b11 : 0b01); +} + + /* Signal a topology change. Edge ports are exempt. */ static void stp_topology_change(uint8_t port) __reentrant { @@ -285,7 +395,13 @@ static void stp_topology_change(uint8_t port) __reentrant return; stp_tc_count++; stp_tc_while = ((uint16_t)stp_maxage_s + stp_fwddelay_s) * STP_HZ; - port_l2_forget_port(port); + if (port >= STP_LAG_BASE) { + for (uint8_t i = machine.min_port; i <= machine.max_port; i++) + if (stp_ent_has(port, i)) + port_l2_forget_port(i); + } else { + port_l2_forget_port(port); + } } @@ -296,7 +412,7 @@ static void stp_topology_change(uint8_t port) __reentrant */ static void stp_loop_hold_peer(uint8_t port) __reentrant { - if (port < machine.min_port || port > machine.max_port) + if (port >= STP_ENTITIES || !stp_ent_active(port)) return; if (!(stp_pflags[port] & STP_PF_ENABLED)) return; @@ -348,7 +464,18 @@ void stp_cnf_send(uint8_t port) __reentrant STP_O->rtl_tag.version = RTL_FRAME_TAG_VERSION; STP_O->rtl_tag.reason = 0x00; STP_O->rtl_tag.flags = HTONS(RTL_TAG_LEARN_DIS); - STP_O->rtl_tag.pmask = HTONS(((uint16_t)1) << port); + if (port >= STP_LAG_BASE) { + stp_scratch = 0; + while (stp_scratch < 10 && !stp_ent_has(port, stp_scratch)) + stp_scratch++; + if (stp_scratch >= 10) { + stp_tx_flags_extra = 0; + return; + } + STP_O->rtl_tag.pmask = HTONS(((uint16_t)1) << stp_scratch); + } else { + STP_O->rtl_tag.pmask = HTONS(((uint16_t)1) << port); + } STP_O->dsap = 0x42; STP_O->ssap = 0x42; @@ -359,8 +486,9 @@ void stp_cnf_send(uint8_t port) __reentrant STP_O->version = BPDU_VER_RSTP; STP_O->bpdu_type = BPDU_TYPE_RST; reg_read_m(RTL837X_MSTP_STATES); + stp_scratch = stp_state_port(port); STP_O->flags = port == stp_root_port ? BPDU_ROLE_ROOT : BPDU_ROLE_DESIGNATED; - if (((sfr_data[3 - (port >> 2)] >> ((port << 1) & 0x7)) & 0b11) == 0b11) + if (((sfr_data[3 - (stp_scratch >> 2)] >> ((stp_scratch << 1) & 0x7)) & 0b11) == 0b11) STP_O->flags |= BPDU_FLAG_LEARNING | BPDU_FLAG_FORWARDING; } else { STP_O->msg_len = HTONS(BPDU_LEN_CONFIG); @@ -424,7 +552,7 @@ void stp_in(void) __banked stp_scratch = ((uint8_t)HTONS(STP_I->rtl_tag.pmask)) & 0x0f; if (stp_scratch < machine.min_port || stp_scratch > machine.max_port) return; - port = stp_scratch; + port = stp_ent_of[stp_scratch]; // Make sure this is the type of (R)STP packet we are interested in: if (!(STP_I->dsap == 0x42 && STP_I->ssap == 0x42 && STP_I->ctrl == 0x03)) @@ -516,8 +644,9 @@ void stp_in(void) __banked uint8_t i; stp_tc_count++; for (i = machine.min_port; i <= machine.max_port; i++) - if (i != port && (stp_pflags[i] & STP_PF_ENABLED) - && !(stp_pflags[i] & STP_PF_OPEREDGE)) + if (!stp_ent_has(port, i) + && (stp_pflags[stp_ent_of[i]] & STP_PF_ENABLED) + && !(stp_pflags[stp_ent_of[i]] & STP_PF_OPEREDGE)) port_l2_forget_port(i); } if (stp_tc_while < ((uint16_t)stp_hello_s + 1) * STP_HZ) @@ -563,9 +692,35 @@ void stp_timers(void) __banked /* Refill the per-port tx budgets once per second (tx hold count) */ if (++stp_sec_tick >= STP_HZ) { stp_sec_tick = 0; - for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) + for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) stp_tx_budget[stp_i] = stp_txhold; + stp_lag_map(); + if (stp_map_dirty) { + stp_map_dirty = 0; + for (stp_i = 0; stp_i < STP_LAG_BASE; stp_i++) { + if (stp_i < machine.min_port || stp_i > machine.max_port) + continue; + if (stp_ent_of[stp_i] != stp_i) { + stp_loop_held[stp_i] = 0; + port_timers[stp_i] = 0; + } else { + stp_ent_apply(stp_i); + } + } + for (stp_i = STP_LAG_BASE; stp_i < STP_ENTITIES; stp_i++) { + if (stp_lag_mask[stp_i - STP_LAG_BASE]) { + stp_ent_apply(stp_i); + } else { + stp_pflags[stp_i] &= ~(STP_PF_OPEREDGE | STP_PF_TRIPPED); + stp_loop_held[stp_i] = 0; + port_timers[stp_i] = 0; + } + } + if (stp_root_port != 0xff && !stp_ent_active(stp_root_port)) + stp_claim_root(); + } + /* 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 - @@ -574,8 +729,20 @@ void stp_timers(void) __banked * of the 50 Hz tick. */ reg_read_m(RTL837X_REG_LINKS_STS); stp_link_now = (uint16_t)sfr_data[1] | ((uint16_t)sfr_data[2] << 8); + { + __xdata static uint16_t eff; + eff = 0; + for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) + if ((stp_link_now >> stp_i) & 1) + eff |= (uint16_t)1 << stp_ent_of[stp_i]; + stp_link_now = eff; + } if (stp_link_now != stp_link_prev) { - for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) { + for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) { + if (stp_i < 10 && (stp_i < machine.min_port || stp_i > machine.max_port)) + continue; + if (stp_i < 10 && stp_ent_of[stp_i] != stp_i) + continue; if (!(stp_pflags[stp_i] & STP_PF_ENABLED)) continue; if (!((stp_link_now ^ stp_link_prev) >> stp_i & 1)) @@ -600,8 +767,8 @@ void stp_timers(void) __banked } } - for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) { - if (!(stp_pflags[stp_i] & STP_PF_ENABLED)) + for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) { + if (!stp_ent_active(stp_i) || !(stp_pflags[stp_i] & STP_PF_ENABLED)) continue; if (stp_bpdu_age[stp_i] < 0xffff) @@ -667,7 +834,12 @@ void stp_defaults(void) __banked stp_fwddelay_s = 15; stp_rstp = 1; stp_txhold = 6; - for (stp_i = 0; stp_i < 10; stp_i++) { + for (stp_i = 0; stp_i < 10; stp_i++) + stp_ent_of[stp_i] = stp_i; + for (stp_i = 0; stp_i < STP_LAG_COUNT; stp_i++) + stp_lag_mask[stp_i] = 0; + stp_map_dirty = 0; + for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) { /* enabled, auto-edge on: host-facing ports go forwarding after * 3 s of BPDU silence instead of the full forward delay */ stp_pflags[stp_i] = STP_PF_ENABLED | STP_PF_AUTOEDGE; @@ -714,25 +886,34 @@ static void stp_fdb_update(__xdata uint16_t pmask) void stp_setup(void) __banked { print_string("Enabling STP: "); + stp_lag_map(); sfr_data[0] = sfr_data[1] = sfr_data[2] = sfr_data[3] = 0; - for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) { + for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) { stp_pflags[stp_i] &= ~(STP_PF_OPEREDGE | STP_PF_TRIPPED); stp_loop_held[stp_i] = 0; stp_bpdu_age[stp_i] = 0; stp_tx_budget[stp_i] = stp_txhold; stp_tx_count[stp_i] = 0; - if (!(stp_pflags[stp_i] & STP_PF_ENABLED) || (stp_pflags[stp_i] & STP_PF_ADMEDGE)) { + port_timers[stp_i] = 0; + port_hello[stp_i] = (uint16_t)stp_hello_s * STP_HZ; + if (stp_i < 10 && (stp_i < machine.min_port || stp_i > machine.max_port)) + continue; + if (stp_i >= STP_LAG_BASE && !stp_lag_mask[stp_i - STP_LAG_BASE]) + continue; + if (stp_i < 10 && stp_ent_of[stp_i] != stp_i) + continue; + if (!(stp_pflags[stp_i] & STP_PF_ENABLED) + || (stp_pflags[stp_i] & STP_PF_ADMEDGE)) { /* not participating, or admin edge: forwarding immediately */ if (stp_pflags[stp_i] & STP_PF_ADMEDGE) stp_pflags[stp_i] |= STP_PF_OPEREDGE; - sfr_data[3 - (stp_i >> 2)] |= (uint8_t)(0b11 << ((stp_i << 1) & 0x7)); + stp_state_bits(stp_i, 0b11); port_timers[stp_i] = 0; } else { /* listen first: blocking until the forward-delay expires */ - sfr_data[3 - (stp_i >> 2)] |= (uint8_t)(0b01 << ((stp_i << 1) & 0x7)); + stp_state_bits(stp_i, 0b01); port_timers[stp_i] = (uint16_t)stp_fwddelay_s * STP_HZ; } - port_hello[stp_i] = (uint16_t)stp_hello_s * STP_HZ; } sfr_data[1] |= 0x0c; // Do not block the CPU port (bits 3:2 of byte 1 = port 9) reg_write_m(RTL837X_MSTP_STATES); @@ -752,7 +933,11 @@ void stp_setup(void) __banked /* Seed the carrier bitmap, so turning STP on does not report every * port that was already down as a fresh topology change. */ reg_read_m(RTL837X_REG_LINKS_STS); - stp_link_prev = (uint16_t)sfr_data[1] | ((uint16_t)sfr_data[2] << 8); + stp_link_now = (uint16_t)sfr_data[1] | ((uint16_t)sfr_data[2] << 8); + stp_link_prev = 0; + for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) + if ((stp_link_now >> stp_i) & 1) + stp_link_prev |= (uint16_t)1 << stp_ent_of[stp_i]; stp_claim_root(); @@ -768,6 +953,8 @@ void stp_off(void) __banked // Set STP port state to forwarding // States are: 00 disable, 01 blocking, 10 learning, 11 forwarding sfr_data[3 - (stp_i >> 2)] |= (uint8_t)(0b11 << ((stp_i << 1) & 0x7)); + } + for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) { stp_pflags[stp_i] &= ~(STP_PF_OPEREDGE | STP_PF_TRIPPED); stp_loop_held[stp_i] = 0; port_timers[stp_i] = 0; @@ -803,12 +990,27 @@ void stp_parse(void) __banked __reentrant if (cmd_words_len < 3) goto err; - if (cmd_compare(1, "port")) { + if (cmd_compare(1, "port") || cmd_compare(1, "lag")) { + stp_lag_map(); if (cmd_words_len < 4) goto err; - if (!cmd_parse_port_separator(cmd_words_b[2])) - goto err; - port = atoi_results_u8; + if (cmd_compare(1, "lag")) { + if (!atoi_byte(cmd_words_b[2])) + goto err; + if (!atoi_results_u8 || atoi_results_u8 > STP_LAG_COUNT) + goto err; + port = STP_LAG_BASE + atoi_results_u8 - 1; + } else { + if (!cmd_parse_port_separator(cmd_words_b[2])) + goto err; + port = atoi_results_u8; + if (stp_ent_of[port] != port) { + print_string("Port belongs to a LAG, configure it as lag "); + print_byte(stp_ent_of[port] - STP_LAG_BASE + 1); + write_char('\n'); + return; + } + } if (cmd_words_len < 5 && !cmd_compare(3, "on") && !cmd_compare(3, "off")) goto err; if (cmd_compare(3, "on")) { @@ -926,5 +1128,5 @@ void stp_parse(void) __banked __reentrant } return; err: - print_string("Error: stp on|off|status | prio <0-15> | hello <1-10> | maxage <6-40> | fwd <4-30> | txhold <1-10> | version rstp|stp | port <1-9> on|off|edge|cost|prio|guard|filter ...\n"); + print_string("Error: stp on|off|status | prio <0-15> | hello <1-10> | maxage <6-40> | fwd <4-30> | txhold <1-10> | version rstp|stp | port <1-9>|lag <1-4> on|off|edge|cost|prio|guard|filter ...\n"); } diff --git a/rtl837x_stp.h b/rtl837x_stp.h index 97080ce0..75289a5d 100644 --- a/rtl837x_stp.h +++ b/rtl837x_stp.h @@ -12,6 +12,10 @@ void stp_defaults(void) __banked; /* Tick rate of stp_timers(), also used by the web UI. */ #define STP_HZ 50 +#define STP_LAG_BASE 10 +#define STP_LAG_COUNT 4 +#define STP_ENTITIES (STP_LAG_BASE + STP_LAG_COUNT) + /* Bridge identifier as carried in a BPDU (priority, extension, MAC). */ struct bridge { uint8_t prio; @@ -36,15 +40,19 @@ extern __xdata uint8_t stp_txhold; #define STP_PF_OPEREDGE 0x40 /* runtime: port went forwarding as an edge */ #define STP_PF_TRIPPED 0x80 /* runtime: disabled by BPDU guard */ -extern __xdata uint8_t stp_pflags[10]; -extern __xdata uint32_t stp_pcost[10]; -extern __xdata uint8_t stp_pprio[10]; -extern __xdata uint8_t stp_pp2p[10]; +extern __xdata uint8_t stp_pflags[STP_ENTITIES]; +extern __xdata uint32_t stp_pcost[STP_ENTITIES]; /* path cost; 0 = auto (20000) */ +extern __xdata uint8_t stp_pprio[STP_ENTITIES]; +extern __xdata uint8_t stp_pp2p[STP_ENTITIES]; +extern __xdata uint8_t stp_ent_of[10]; /* the STP entity a port answers to: itself, or its lag */ +extern __xdata uint16_t stp_lag_mask[STP_LAG_COUNT]; /* member ports of each lag; 0 = not part of STP */ -extern __xdata struct bridge stp_dbridge[10]; -extern __xdata uint16_t stp_dpid[10]; -extern __xdata uint32_t stp_dcost[10]; -extern __xdata uint16_t stp_bpdu_age[10]; +/* Last-heard designated info per port (from received BPDUs); consult + * stp_bpdu_age to decide whether it is still current. */ +extern __xdata struct bridge stp_dbridge[STP_ENTITIES]; +extern __xdata uint16_t stp_dpid[STP_ENTITIES]; +extern __xdata uint32_t stp_dcost[STP_ENTITIES]; +extern __xdata uint16_t stp_bpdu_age[STP_ENTITIES]; /* ticks since a BPDU was heard */ extern __xdata struct bridge root_bridge; extern __xdata uint32_t root_bridge_cost; From eab927f52a8dcd7aa240708bd7f948c6fb533dec Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Sun, 6 Sep 2026 15:41:59 +0200 Subject: [PATCH 2/4] stp: name the port count instead of spelling out ten Review of #397 pointed at the bare 10, which shows up in fifteen places. It is the width of the port field in the MSTP register, so it is the CPU port index plus one and it gets a name. The web page counted to ten as well when listing a group's members. It uses numPorts now, like the rest of that page does. The aggregation section of the doc no longer names the OEM firmware or calls a group a trunk, and it says plainly that the hardware keeps no per-group state, so the firmware writes the members itself. Generated code is unchanged: the only difference in the assembly is the version string. --- doc/stp.md | 26 +++++++++++++++----------- rtl837x_stp.c | 30 +++++++++++++++--------------- rtl837x_stp.h | 4 +++- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/doc/stp.md b/doc/stp.md index c6ff6598..8b6780c5 100644 --- a/doc/stp.md +++ b/doc/stp.md @@ -60,23 +60,27 @@ state. ## Link aggregation -A LAG is a port to the protocol, the way the vendor firmware presents it as -Trk1 to Trk4. The four groups are entities alongside the nine physical ports: -they carry their own path cost, priority, edge and guard settings, they hold -their own timers, and they appear as their own rows on the Spanning Tree page. +A LAG (Link Aggregation Group) is handled like an additional port by the STP +protocol, with its own timers and states. The switch devices support up to 4 +LAGs, which are shown alongside the physical ports in the STP status. A group +carries its own path cost, priority, edge and guard settings, and gets its own +row on the Spanning Tree page. ``` stp lag 1 cost 10000 # the group decides, not its members stp lag 1 edge off ``` -Membership comes from the aggregation registers the `lag` command writes, re-read -once a second, so a group changing under LACP is picked up without any -coordination between the two. A member port is not an STP port of its own: -`stp port ` on a member says which group to configure instead. State -changes are written to every member in one register write, since the hardware -has no per-group state, and BPDUs are transmitted through the lowest -member while carrying the group's own port id. Losing one member of a live +LAGs are configured via the `lag` command. Once a port is a member of a LAG it +can no longer be configured individually for STP, so `stp port ` on a member +tells you which group to configure instead. Membership is re-read from the +aggregation registers once a second, so a group changing under LACP is picked +up without any coordination between the two. + +The switch hardware does not handle STP state for a LAG as a whole. State +changes for the members have to be done by the firmware, updating every member +port, which is possible using a single register write. BPDUs go out through the +lowest member and carry the group's own port id. Losing one member of a live LAG is not a topology change; the logical port only goes down with its last link. diff --git a/rtl837x_stp.c b/rtl837x_stp.c index 20f530d8..78a4cfd3 100644 --- a/rtl837x_stp.c +++ b/rtl837x_stp.c @@ -47,7 +47,7 @@ __xdata uint8_t stp_rstp; /* 1 = RST BPDUs, 0 = legacy Config BPDUs */ __xdata uint8_t stp_txhold; /* BPDUs per port per second */ -__xdata uint8_t stp_ent_of[10]; +__xdata uint8_t stp_ent_of[STP_PORTS]; __xdata uint16_t stp_lag_mask[STP_LAG_COUNT]; __xdata uint8_t stp_ss_i; __xdata uint8_t stp_map_dirty; @@ -248,9 +248,9 @@ static void stp_status(void) stp_st_of = stp_i; if (stp_i >= STP_LAG_BASE) { stp_st_of = 0; - while (stp_st_of < 10 && !((stp_lag_mask[stp_i - STP_LAG_BASE] >> stp_st_of) & 1)) + while (stp_st_of < STP_PORTS && !((stp_lag_mask[stp_i - STP_LAG_BASE] >> stp_st_of) & 1)) stp_st_of++; - if (stp_st_of >= 10) + if (stp_st_of >= STP_PORTS) stp_st_of = 0; } print_field(stp_state_txt, (sfr_data[3 - (stp_st_of >> 2)] >> ((stp_st_of << 1) & 0x7)) & 0x3, 5); @@ -300,14 +300,14 @@ int8_t cmpBytes(__xdata uint8_t *m1, __xdata uint8_t *m2, uint8_t n) __reentrant static void stp_lag_map(void) { - for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) + for (stp_ss_i = 0; stp_ss_i < STP_PORTS; stp_ss_i++) stp_ent_of[stp_ss_i] = stp_ss_i; for (stp_scratch = 0; stp_scratch < STP_LAG_COUNT; stp_scratch++) { stp_scratch16 = port_lag_members_get(stp_scratch); if (stp_lag_mask[stp_scratch] != stp_scratch16) stp_map_dirty = 1; stp_lag_mask[stp_scratch] = stp_scratch16; - for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) + for (stp_ss_i = 0; stp_ss_i < STP_PORTS; stp_ss_i++) if ((stp_lag_mask[stp_scratch] >> stp_ss_i) & 1) stp_ent_of[stp_ss_i] = STP_LAG_BASE + stp_scratch; } @@ -336,7 +336,7 @@ static uint8_t stp_state_port(uint8_t e) __reentrant { if (e < STP_LAG_BASE) return e; - for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) + for (stp_ss_i = 0; stp_ss_i < STP_PORTS; stp_ss_i++) if ((stp_lag_mask[e - STP_LAG_BASE] >> stp_ss_i) & 1) return stp_ss_i; return 0; @@ -345,7 +345,7 @@ static uint8_t stp_state_port(uint8_t e) __reentrant static void stp_state_bits(uint8_t port, uint8_t state) __reentrant { - for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) + for (stp_ss_i = 0; stp_ss_i < STP_PORTS; stp_ss_i++) if (stp_ent_has(port, stp_ss_i)) sfr_data[3 - (stp_ss_i >> 2)] |= (uint8_t)(state << ((stp_ss_i << 1) & 0x7)); } @@ -355,7 +355,7 @@ static void stp_state_bits(uint8_t port, uint8_t state) __reentrant static void stp_state_set(uint8_t port, uint8_t state) __reentrant { reg_read_m(RTL837X_MSTP_STATES); - for (stp_ss_i = 0; stp_ss_i < 10; stp_ss_i++) { + for (stp_ss_i = 0; stp_ss_i < STP_PORTS; stp_ss_i++) { if (!stp_ent_has(port, stp_ss_i)) continue; stp_scratch = 3 - (stp_ss_i >> 2); @@ -466,9 +466,9 @@ void stp_cnf_send(uint8_t port) __reentrant STP_O->rtl_tag.flags = HTONS(RTL_TAG_LEARN_DIS); if (port >= STP_LAG_BASE) { stp_scratch = 0; - while (stp_scratch < 10 && !stp_ent_has(port, stp_scratch)) + while (stp_scratch < STP_PORTS && !stp_ent_has(port, stp_scratch)) stp_scratch++; - if (stp_scratch >= 10) { + if (stp_scratch >= STP_PORTS) { stp_tx_flags_extra = 0; return; } @@ -739,9 +739,9 @@ void stp_timers(void) __banked } if (stp_link_now != stp_link_prev) { for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) { - if (stp_i < 10 && (stp_i < machine.min_port || stp_i > machine.max_port)) + if (stp_i < STP_PORTS && (stp_i < machine.min_port || stp_i > machine.max_port)) continue; - if (stp_i < 10 && stp_ent_of[stp_i] != stp_i) + if (stp_i < STP_PORTS && stp_ent_of[stp_i] != stp_i) continue; if (!(stp_pflags[stp_i] & STP_PF_ENABLED)) continue; @@ -834,7 +834,7 @@ void stp_defaults(void) __banked stp_fwddelay_s = 15; stp_rstp = 1; stp_txhold = 6; - for (stp_i = 0; stp_i < 10; stp_i++) + for (stp_i = 0; stp_i < STP_PORTS; stp_i++) stp_ent_of[stp_i] = stp_i; for (stp_i = 0; stp_i < STP_LAG_COUNT; stp_i++) stp_lag_mask[stp_i] = 0; @@ -896,11 +896,11 @@ void stp_setup(void) __banked stp_tx_count[stp_i] = 0; port_timers[stp_i] = 0; port_hello[stp_i] = (uint16_t)stp_hello_s * STP_HZ; - if (stp_i < 10 && (stp_i < machine.min_port || stp_i > machine.max_port)) + if (stp_i < STP_PORTS && (stp_i < machine.min_port || stp_i > machine.max_port)) continue; if (stp_i >= STP_LAG_BASE && !stp_lag_mask[stp_i - STP_LAG_BASE]) continue; - if (stp_i < 10 && stp_ent_of[stp_i] != stp_i) + if (stp_i < STP_PORTS && stp_ent_of[stp_i] != stp_i) continue; if (!(stp_pflags[stp_i] & STP_PF_ENABLED) || (stp_pflags[stp_i] & STP_PF_ADMEDGE)) { diff --git a/rtl837x_stp.h b/rtl837x_stp.h index 75289a5d..b547f5d8 100644 --- a/rtl837x_stp.h +++ b/rtl837x_stp.h @@ -2,6 +2,7 @@ #define _RTL837X_STP_H_ #include +#include "rtl837x_common.h" void stp_in(void) __banked; void stp_setup(void) __banked; void stp_timers(void) __banked; @@ -12,7 +13,8 @@ void stp_defaults(void) __banked; /* Tick rate of stp_timers(), also used by the web UI. */ #define STP_HZ 50 -#define STP_LAG_BASE 10 +#define STP_PORTS (CPU_PORT + 1) +#define STP_LAG_BASE STP_PORTS #define STP_LAG_COUNT 4 #define STP_ENTITIES (STP_LAG_BASE + STP_LAG_COUNT) From 78ecef15e907b90c5329fdaac1a1752e84d10ae8 Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:01:15 +0200 Subject: [PATCH 3/4] stp gui: address a link aggregation group as one entity The web UI became one index.html plus one main.js while this branch was open, so the STP page changes had no html/stp.js and html/config.js left to live in. They are re-applied here against the new structure. A row now carries two derived fields: the name the CLI expects for it ("port N" or "lag N") and the key its controls are keyed by (the port number or "LN"). A group's row is labelled with its front panel members, which /stp.json reports in the mbr field. conf_cmds and conf_overwrite accept the "stp lag N ..." forms so the startup config keeps them. --- html/main.js | 85 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 31 deletions(-) diff --git a/html/main.js b/html/main.js index c3fec057..27f78aef 100644 --- a/html/main.js +++ b/html/main.js @@ -1020,6 +1020,13 @@ const conf_cmds = [ /^stp\s+port\s+\d{1,2}\s+guard\s+(none|bpdu|root)$/, /^stp\s+port\s+\d{1,2}\s+filter\s+(on|off)$/, /^stp\s+port\s+\d{1,2}\s+p2p\s+(auto|on|off)$/, + /^stp\s+lag\s+[1-4]\s+(on|off)$/, + /^stp\s+lag\s+[1-4]\s+edge\s+(on|off|auto)$/, + /^stp\s+lag\s+[1-4]\s+cost\s+\d{1,9}$/, + /^stp\s+lag\s+[1-4]\s+prio\s+\d{1,3}$/, + /^stp\s+lag\s+[1-4]\s+guard\s+(none|bpdu|root)$/, + /^stp\s+lag\s+[1-4]\s+filter\s+(on|off)$/, + /^stp\s+lag\s+[1-4]\s+p2p\s+(auto|on|off)$/, /^igmp\s+(on|off)$/, /^mtu\s+\d{1,2}\s+\d+$/, /^bw\s+(in|out)\s+\d{1,2}\s+\S+$/, @@ -1055,6 +1062,7 @@ const conf_overwrite = [ /^isolate\s+\d{1,2}\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/, + /^stp\s+lag\s+[1-4]\s+(edge|cost|prio|guard|filter|p2p)\b/, /^igmp\b/, /^mtu\s+\d{1,2}\b/, /^bw\s+(in|out)\s+\d{1,2}\b/, @@ -2601,45 +2609,58 @@ function num(id, min, max, onch) { return n; } +function members(mask) { + const a = []; + for (let i = 0; i < numPorts; i++) + if (mask & (1 << i)) a.push(logToPhysPort[i]); + return a.join(","); +} + function buildPortsTable(ports) { const tbl = document.getElementById("stpPortsTbl"); const stat = document.getElementById("stpStatTbl"); for (const p of [...ports].sort((a, b) => a.p - b.p)) { + p.n = p.lag ? "lag " + p.lag : "port " + p.p; + p.k = p.lag ? "L" + p.lag : p.p; const tr = tbl.insertRow(); - tr.insertCell().textContent = p.p; - tr.insertCell().appendChild(sel("en_" + p.p, + tr.insertCell().textContent = p.lag + ? "LAG" + p.lag + " (" + members(p.mbr) + ")" + : p.p; + tr.insertCell().appendChild(sel("en_" + p.k, [["on","Enable"],["off","Disable"]], - e => stpCmd("stp port " + p.p + " " + e.target.value))); - const pc = num("cost_" + p.p, 0, 200000000, - e => stpCmd("stp port " + p.p + " cost " + e.target.value)); + e => stpCmd("stp " + p.n + " " + e.target.value))); + const pc = num("cost_" + p.k, 0, 200000000, + e => stpCmd("stp " + p.n + " cost " + e.target.value)); pc.style.width = "7em"; pc.title = "0 - 200000000 (0 = Auto)"; tr.insertCell().appendChild(pc); - const pr = sel("prio_" + p.p, [], - e => stpCmd("stp port " + p.p + " prio " + e.target.value)); + const pr = sel("prio_" + p.k, [], + e => stpCmd("stp " + p.n + " prio " + e.target.value)); for (let v = 0; v <= 240; v += 16) { const o = document.createElement("option"); o.value = v; o.textContent = v + (v === 128 ? " (default)" : ""); pr.appendChild(o); } tr.insertCell().appendChild(pr); - tr.insertCell().appendChild(sel("edge_" + p.p, + tr.insertCell().appendChild(sel("edge_" + p.k, [["auto","Auto"],["on","Enable"],["off","Disable"]], - e => stpCmd("stp port " + p.p + " edge " + e.target.value))); - tr.insertCell().appendChild(sel("filt_" + p.p, + e => stpCmd("stp " + p.n + " edge " + e.target.value))); + tr.insertCell().appendChild(sel("filt_" + p.k, [["off","Disable"],["on","Enable"]], - e => stpCmd("stp port " + p.p + " filter " + e.target.value))); - tr.insertCell().appendChild(sel("guard_" + p.p, + e => stpCmd("stp " + p.n + " filter " + e.target.value))); + tr.insertCell().appendChild(sel("guard_" + p.k, [["none","None"],["bpdu","BPDU"],["root","Root"]], - e => stpCmd("stp port " + p.p + " guard " + e.target.value))); - tr.insertCell().appendChild(sel("p2p_" + p.p, + e => stpCmd("stp " + p.n + " guard " + e.target.value))); + tr.insertCell().appendChild(sel("p2p_" + p.k, [["auto","Auto"],["on","Enable"],["off","Disable"]], - e => stpCmd("stp port " + p.p + " p2p " + e.target.value))); + e => stpCmd("stp " + p.n + " p2p " + e.target.value))); const sr = stat.insertRow(); - sr.insertCell().textContent = p.p; + sr.insertCell().textContent = p.lag + ? "LAG" + p.lag + " (" + members(p.mbr) + ")" + : p.p; for (const id of ["st","role","db","dp","dc","oe","op"]) - sr.insertCell().id = id + "_" + p.p; + sr.insertCell().id = id + "_" + p.k; } stpRows = ports.length; } @@ -2672,18 +2693,19 @@ function fetchStp() { + " \u2014 topology changes: " + parseInt(s.tc, 16)) : ""; for (const p of s.ports) { + const k = p.lag ? "L" + p.lag : p.p; const trip = (p.f & PF_TRIPPED) ? " (guard!)" : ""; - document.getElementById("st_" + p.p).textContent = + document.getElementById("st_" + k).textContent = s.on ? STP_STATES[p.st] + trip : "-"; - document.getElementById("role_" + p.p).textContent = + document.getElementById("role_" + k).textContent = s.on ? STP_ROLES[p.role] : "-"; - document.getElementById("db_" + p.p).textContent = s.on ? fmtBridgeId(p.db) : "-"; - document.getElementById("dp_" + p.p).textContent = + document.getElementById("db_" + k).textContent = s.on ? fmtBridgeId(p.db) : "-"; + document.getElementById("dp_" + k).textContent = s.on ? (parseInt(p.dp.slice(0, 2), 16) + "-" + parseInt(p.dp.slice(2), 16)) : "-"; - document.getElementById("dc_" + p.p).textContent = s.on ? parseInt(p.dc, 16) : "-"; - document.getElementById("oe_" + p.p).textContent = + document.getElementById("dc_" + k).textContent = s.on ? parseInt(p.dc, 16) : "-"; + document.getElementById("oe_" + k).textContent = s.on ? ((p.f & PF_OPEREDGE) ? "True" : "False") : "-"; - document.getElementById("op_" + p.p).textContent = s.on ? (p.p2 == 2 ? "False" : "True") : "-"; + document.getElementById("op_" + k).textContent = s.on ? (p.p2 == 2 ? "False" : "True") : "-"; } if (stpDirty) return; @@ -2695,15 +2717,16 @@ function fetchStp() { document.getElementById("bFwd").value = s.fwd; document.getElementById("bTxhold").value = s.txhold; for (const p of s.ports) { - document.getElementById("en_" + p.p).value = (p.f & PF_ENABLED) ? "on" : "off"; - document.getElementById("edge_" + p.p).value = + const k = p.lag ? "L" + p.lag : p.p; + document.getElementById("en_" + k).value = (p.f & PF_ENABLED) ? "on" : "off"; + document.getElementById("edge_" + k).value = (p.f & PF_ADMEDGE) ? "on" : ((p.f & PF_AUTOEDGE) ? "auto" : "off"); - document.getElementById("cost_" + p.p).value = parseInt(p.pc, 16); - document.getElementById("prio_" + p.p).value = p.prio; - document.getElementById("p2p_" + p.p).value = ["auto","on","off"][p.p2]; - document.getElementById("guard_" + p.p).value = + document.getElementById("cost_" + k).value = parseInt(p.pc, 16); + document.getElementById("prio_" + k).value = p.prio; + document.getElementById("p2p_" + k).value = ["auto","on","off"][p.p2]; + document.getElementById("guard_" + k).value = (p.f & PF_BPDUGUARD) ? "bpdu" : ((p.f & PF_ROOTGUARD) ? "root" : "none"); - document.getElementById("filt_" + p.p).value = (p.f & PF_FILTER) ? "on" : "off"; + document.getElementById("filt_" + k).value = (p.f & PF_FILTER) ? "on" : "off"; } } }; From c1ad800fc5dc174ad12a6cfe14960e286d18a07f Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Thu, 10 Sep 2026 15:17:53 +0200 Subject: [PATCH 4/4] stp: name the MSTP state bits and the entity index The state literals were spread over eighteen places in the file, sixteen of them older than this branch, so naming only the new ones would have left it written half one way and half the other. Naming them separates two roles the literal hid: 0b11 is the forwarding state in a call and a two bit field mask in a read-modify-write, hence both STP_ST_FORWARDING and STP_ST_MASK. The comment above stp_state_set() said what the defines now say, so it goes. stp_parse() holds an entity index, not a port. cmd_parse_port() has already mapped the front panel digit through machine.phys_to_log_port[], and the aggregates continue that same logical range above the CPU port, so call the variable ent. Reading the two branches of the if no longer means following the parser two functions down. No change to the generated code: 5331 instructions before and after. --- rtl837x_stp.c | 94 +++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/rtl837x_stp.c b/rtl837x_stp.c index 78a4cfd3..b29a0193 100644 --- a/rtl837x_stp.c +++ b/rtl837x_stp.c @@ -166,6 +166,13 @@ struct stp_pkt_in { #define BPDU_ROLE_ROOT (0b10 << 2) #define BPDU_ROLE_DESIGNATED (0b11 << 2) +/* Port state as the MSTP register encodes it, two bits per port. */ +#define STP_ST_DISABLED 0b00 +#define STP_ST_BLOCKING 0b01 +#define STP_ST_LEARNING 0b10 +#define STP_ST_FORWARDING 0b11 +#define STP_ST_MASK 0b11 + /* Console messages name the port on the front panel, not the internal index. */ static void print_port_nl(uint8_t port) __reentrant { @@ -351,7 +358,6 @@ static void stp_state_bits(uint8_t port, uint8_t state) __reentrant } -/* MSTP register encoding: 00 disable, 01 blocking, 10 learning, 11 forwarding. */ static void stp_state_set(uint8_t port, uint8_t state) __reentrant { reg_read_m(RTL837X_MSTP_STATES); @@ -359,7 +365,7 @@ static void stp_state_set(uint8_t port, uint8_t state) __reentrant if (!stp_ent_has(port, stp_ss_i)) continue; stp_scratch = 3 - (stp_ss_i >> 2); - sfr_data[stp_scratch] &= ~(uint8_t)(0b11 << ((stp_ss_i << 1) & 0x7)); + sfr_data[stp_scratch] &= ~(uint8_t)(STP_ST_MASK << ((stp_ss_i << 1) & 0x7)); sfr_data[stp_scratch] |= (uint8_t)(state << ((stp_ss_i << 1) & 0x7)); } reg_write_m(RTL837X_MSTP_STATES); @@ -369,22 +375,22 @@ static void stp_state_set(uint8_t port, uint8_t state) __reentrant static void stp_ent_apply(uint8_t e) __reentrant { if (!(stp_pflags[e] & STP_PF_ENABLED)) { - stp_state_set(e, 0b11); + stp_state_set(e, STP_ST_FORWARDING); return; } if (stp_pflags[e] & STP_PF_TRIPPED) { - stp_state_set(e, 0b00); + stp_state_set(e, STP_ST_DISABLED); return; } if (stp_pflags[e] & STP_PF_ADMEDGE) { stp_pflags[e] |= STP_PF_OPEREDGE; - stp_state_set(e, 0b11); + stp_state_set(e, STP_ST_FORWARDING); return; } if (port_timers[e]) - stp_state_set(e, 0b01); + stp_state_set(e, STP_ST_BLOCKING); else - stp_state_set(e, ((stp_link_prev >> e) & 1) ? 0b11 : 0b01); + stp_state_set(e, ((stp_link_prev >> e) & 1) ? STP_ST_FORWARDING : STP_ST_BLOCKING); } @@ -421,7 +427,7 @@ static void stp_loop_hold_peer(uint8_t port) __reentrant if (!port_timers[port]) { print_string("STP: loop detected, blocking port "); print_port_nl(port); - stp_state_set(port, 0b01); + stp_state_set(port, STP_ST_BLOCKING); stp_pflags[port] &= ~STP_PF_OPEREDGE; stp_topology_change(port); } @@ -488,7 +494,7 @@ void stp_cnf_send(uint8_t port) __reentrant reg_read_m(RTL837X_MSTP_STATES); stp_scratch = stp_state_port(port); STP_O->flags = port == stp_root_port ? BPDU_ROLE_ROOT : BPDU_ROLE_DESIGNATED; - if (((sfr_data[3 - (stp_scratch >> 2)] >> ((stp_scratch << 1) & 0x7)) & 0b11) == 0b11) + if (((sfr_data[3 - (stp_scratch >> 2)] >> ((stp_scratch << 1) & 0x7)) & STP_ST_MASK) == STP_ST_FORWARDING) STP_O->flags |= BPDU_FLAG_LEARNING | BPDU_FLAG_FORWARDING; } else { STP_O->msg_len = HTONS(BPDU_LEN_CONFIG); @@ -573,7 +579,7 @@ void stp_in(void) __banked print_string("STP: BPDU guard tripped, disabling port "); print_port_nl(port); stp_pflags[port] |= STP_PF_TRIPPED; - stp_state_set(port, 0b00); + stp_state_set(port, STP_ST_DISABLED); stp_tc_count++; return; } @@ -664,7 +670,7 @@ void stp_in(void) __banked if (stp_pflags[port] & STP_PF_ROOTGUARD) { print_string("STP: root guard blocking port "); print_port_nl(port); - stp_state_set(port, 0b01); + stp_state_set(port, STP_ST_BLOCKING); port_timers[port] = (uint16_t)stp_fwddelay_s * STP_HZ; stp_pflags[port] &= ~STP_PF_OPEREDGE; return; @@ -748,7 +754,7 @@ void stp_timers(void) __banked if (!((stp_link_now ^ stp_link_prev) >> stp_i & 1)) continue; /* Either way the port must stop forwarding first. */ - stp_state_set(stp_i, 0b01); + stp_state_set(stp_i, STP_ST_BLOCKING); if ((stp_link_now >> stp_i) & 1) { /* Carrier back: re-run the listen period rather than * forwarding straight away - the segment may have been @@ -793,7 +799,7 @@ void stp_timers(void) __banked if (port_timers[stp_i]) { if (!--port_timers[stp_i]) { stp_loop_held[stp_i] = 0; - stp_state_set(stp_i, 0b11); + stp_state_set(stp_i, STP_ST_FORWARDING); print_string("STP: port forwarding "); print_port_nl(stp_i); stp_topology_change(stp_i); @@ -804,7 +810,7 @@ void stp_timers(void) __banked * host-facing, go to forwarding without the full wait. */ port_timers[stp_i] = 0; stp_pflags[stp_i] |= STP_PF_OPEREDGE; - stp_state_set(stp_i, 0b11); + stp_state_set(stp_i, STP_ST_FORWARDING); print_string("STP: edge port forwarding "); print_port_nl(stp_i); } @@ -907,11 +913,11 @@ void stp_setup(void) __banked /* not participating, or admin edge: forwarding immediately */ if (stp_pflags[stp_i] & STP_PF_ADMEDGE) stp_pflags[stp_i] |= STP_PF_OPEREDGE; - stp_state_bits(stp_i, 0b11); + stp_state_bits(stp_i, STP_ST_FORWARDING); port_timers[stp_i] = 0; } else { /* listen first: blocking until the forward-delay expires */ - stp_state_bits(stp_i, 0b01); + stp_state_bits(stp_i, STP_ST_BLOCKING); port_timers[stp_i] = (uint16_t)stp_fwddelay_s * STP_HZ; } } @@ -952,7 +958,7 @@ void stp_off(void) __banked for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) { // Set STP port state to forwarding // States are: 00 disable, 01 blocking, 10 learning, 11 forwarding - sfr_data[3 - (stp_i >> 2)] |= (uint8_t)(0b11 << ((stp_i << 1) & 0x7)); + sfr_data[3 - (stp_i >> 2)] |= (uint8_t)(STP_ST_FORWARDING << ((stp_i << 1) & 0x7)); } for (stp_i = 0; stp_i < STP_ENTITIES; stp_i++) { stp_pflags[stp_i] &= ~(STP_PF_OPEREDGE | STP_PF_TRIPPED); @@ -969,7 +975,7 @@ void stp_off(void) __banked void stp_parse(void) __banked __reentrant { - uint8_t port; + uint8_t ent; if (cmd_compare(1, "on")) { print_string("STP enabled\n"); @@ -990,7 +996,7 @@ void stp_parse(void) __banked __reentrant if (cmd_words_len < 3) goto err; - if (cmd_compare(1, "port") || cmd_compare(1, "lag")) { + if (cmd_compare(1, "ent") || cmd_compare(1, "lag")) { stp_lag_map(); if (cmd_words_len < 4) goto err; @@ -999,14 +1005,14 @@ void stp_parse(void) __banked __reentrant goto err; if (!atoi_results_u8 || atoi_results_u8 > STP_LAG_COUNT) goto err; - port = STP_LAG_BASE + atoi_results_u8 - 1; + ent = STP_LAG_BASE + atoi_results_u8 - 1; } else { if (!cmd_parse_port_separator(cmd_words_b[2])) goto err; - port = atoi_results_u8; - if (stp_ent_of[port] != port) { + ent = atoi_results_u8; + if (stp_ent_of[ent] != ent) { print_string("Port belongs to a LAG, configure it as lag "); - print_byte(stp_ent_of[port] - STP_LAG_BASE + 1); + print_byte(stp_ent_of[ent] - STP_LAG_BASE + 1); write_char('\n'); return; } @@ -1014,26 +1020,26 @@ void stp_parse(void) __banked __reentrant if (cmd_words_len < 5 && !cmd_compare(3, "on") && !cmd_compare(3, "off")) goto err; if (cmd_compare(3, "on")) { - stp_pflags[port] |= STP_PF_ENABLED; - stp_pflags[port] &= ~STP_PF_TRIPPED; + stp_pflags[ent] |= STP_PF_ENABLED; + stp_pflags[ent] &= ~STP_PF_TRIPPED; if (stp_enabled) { /* (re)join: listen first */ - stp_state_set(port, 0b01); - port_timers[port] = (uint16_t)stp_fwddelay_s * STP_HZ; + stp_state_set(ent, STP_ST_BLOCKING); + port_timers[ent] = (uint16_t)stp_fwddelay_s * STP_HZ; } } else if (cmd_compare(3, "off")) { - stp_pflags[port] &= ~STP_PF_ENABLED; + stp_pflags[ent] &= ~STP_PF_ENABLED; if (stp_enabled) - stp_state_set(port, 0b11); /* plain forwarding */ + stp_state_set(ent, STP_ST_FORWARDING); /* plain forwarding */ } else if (cmd_compare(3, "edge")) { /* Also drop the *operational* edge flag: it is what exempts the - * port from topology changes and lets it skip the listen period, + * ent from topology changes and lets it skip the listen period, * so leaving it set would keep the old behaviour until the next * "stp off"/"stp on". An admin edge is operational immediately. */ - stp_pflags[port] &= ~(STP_PF_ADMEDGE | STP_PF_AUTOEDGE | STP_PF_OPEREDGE); + stp_pflags[ent] &= ~(STP_PF_ADMEDGE | STP_PF_AUTOEDGE | STP_PF_OPEREDGE); if (cmd_compare(4, "on")) - stp_pflags[port] |= STP_PF_ADMEDGE | STP_PF_OPEREDGE; + stp_pflags[ent] |= STP_PF_ADMEDGE | STP_PF_OPEREDGE; else if (cmd_compare(4, "auto")) - stp_pflags[port] |= STP_PF_AUTOEDGE; + stp_pflags[ent] |= STP_PF_AUTOEDGE; else if (!cmd_compare(4, "off")) goto err; } else if (cmd_compare(3, "cost")) { @@ -1050,14 +1056,14 @@ void stp_parse(void) __banked __reentrant } if (stp_cost_scratch > 200000000UL) goto err; - stp_pcost[port] = stp_cost_scratch; + stp_pcost[ent] = stp_cost_scratch; } else if (cmd_compare(3, "p2p")) { if (cmd_compare(4, "auto")) - stp_pp2p[port] = 0; + stp_pp2p[ent] = 0; else if (cmd_compare(4, "on")) - stp_pp2p[port] = 1; + stp_pp2p[ent] = 1; else if (cmd_compare(4, "off")) - stp_pp2p[port] = 2; + stp_pp2p[ent] = 2; else goto err; } else if (cmd_compare(3, "prio")) { @@ -1065,20 +1071,20 @@ void stp_parse(void) __banked __reentrant goto err; if (atoi_results_u8 > 240 || (atoi_results_u8 & 0x0f)) goto err; - stp_pprio[port] = atoi_results_u8; + stp_pprio[ent] = atoi_results_u8; } else if (cmd_compare(3, "guard")) { - stp_pflags[port] &= ~(STP_PF_BPDUGUARD | STP_PF_ROOTGUARD); + stp_pflags[ent] &= ~(STP_PF_BPDUGUARD | STP_PF_ROOTGUARD); if (cmd_compare(4, "bpdu")) - stp_pflags[port] |= STP_PF_BPDUGUARD; + stp_pflags[ent] |= STP_PF_BPDUGUARD; else if (cmd_compare(4, "root")) - stp_pflags[port] |= STP_PF_ROOTGUARD; + stp_pflags[ent] |= STP_PF_ROOTGUARD; else if (!cmd_compare(4, "none")) goto err; } else if (cmd_compare(3, "filter")) { if (cmd_compare(4, "on")) - stp_pflags[port] |= STP_PF_FILTER; + stp_pflags[ent] |= STP_PF_FILTER; else if (cmd_compare(4, "off")) - stp_pflags[port] &= ~STP_PF_FILTER; + stp_pflags[ent] &= ~STP_PF_FILTER; else goto err; } else { @@ -1128,5 +1134,5 @@ void stp_parse(void) __banked __reentrant } return; err: - print_string("Error: stp on|off|status | prio <0-15> | hello <1-10> | maxage <6-40> | fwd <4-30> | txhold <1-10> | version rstp|stp | port <1-9>|lag <1-4> on|off|edge|cost|prio|guard|filter ...\n"); + print_string("Error: stp on|off|status | prio <0-15> | hello <1-10> | maxage <6-40> | fwd <4-30> | txhold <1-10> | version rstp|stp | ent <1-9>|lag <1-4> on|off|edge|cost|prio|guard|filter ...\n"); }