diff --git a/doc/stp.md b/doc/stp.md index 543f8928..e21a891c 100644 --- a/doc/stp.md +++ b/doc/stp.md @@ -142,6 +142,8 @@ Among ports offering the same root, the lowest root path cost wins, then the designated bridge, the designated port ID, and finally this switch's own port ID. A worse BPDU from a different bridge does not replace what a port holds; a worse one from the same designated bridge and port does. +The number in a port ID is the one on the front panel, and 101 and up for +the LAGs, so it matches the port labels on both ends of a link. ## Topology changes diff --git a/httpd/page_stp.c b/httpd/page_stp.c index fbc2ce50..fb9898e8 100644 --- a/httpd/page_stp.c +++ b/httpd/page_stp.c @@ -153,7 +153,7 @@ void send_stp(void) __banked bridge_to_html(); slen += strtox(outbuf + slen, "\",\"dp\":\""); stp_byte_html(dsg ? (stp_dpid[i] >> 8) : stp_pprio[i]); - stp_byte_html(dsg ? stp_dpid[i] : (i + 1)); + stp_byte_html(dsg ? stp_dpid[i] : j); slen += strtox(outbuf + slen, "\",\"dc\":\""); pi_u32 = dsg ? stp_dcost[i] : root_bridge_cost; u32hex_html(); slen += strtox(outbuf + slen, "\"},"); diff --git a/rtl837x_stp.c b/rtl837x_stp.c index 1e41235f..4c04acd4 100644 --- a/rtl837x_stp.c +++ b/rtl837x_stp.c @@ -413,13 +413,28 @@ static uint8_t stp_ent_active(uint8_t e) __reentrant } +static uint8_t stp_pnum(uint8_t e) __reentrant +{ + if (e >= STP_LAG_BASE) + return e + 101 - STP_LAG_BASE; + return machine.log_to_phys_port[e]; +} + + +static uint8_t stp_ent_by_pnum(uint8_t n) __reentrant +{ + for (stp_ss_i = 0; stp_ss_i < STP_ENTITIES; stp_ss_i++) + if (stp_ent_active(stp_ss_i) && stp_pnum(stp_ss_i) == n) + return stp_ss_i; + return 0xff; +} + + uint8_t stp_ent_id(uint8_t e) __banked { if (!stp_ent_active(e)) return 0; - if (e >= STP_LAG_BASE) - return e + 101 - STP_LAG_BASE; - return machine.log_to_phys_port[e]; + return stp_pnum(e); } @@ -624,9 +639,9 @@ static int8_t stp_cmp_designated(uint8_t port) __reentrant stp_cmp = cmpBytes((__xdata uint8_t *)&stp_dbridge[port], (__xdata uint8_t *)&stp_self, 8); if (stp_cmp) return stp_cmp; - if (stp_dpid[port] == (((uint16_t)stp_pprio[port] << 8) | (port + 1))) + if (stp_dpid[port] == (((uint16_t)stp_pprio[port] << 8) | stp_pnum(port))) return 0; - return stp_dpid[port] < (((uint16_t)stp_pprio[port] << 8) | (port + 1)) ? -1 : 1; + return stp_dpid[port] < (((uint16_t)stp_pprio[port] << 8) | stp_pnum(port)) ? -1 : 1; } @@ -814,7 +829,7 @@ void stp_cnf_send(uint8_t port) __reentrant STP_O->bridge.ext = 0x00; STP_O->port_prio = stp_pprio[port]; - STP_O->port_id = port + 1; + STP_O->port_id = stp_pnum(port); /* Message age, incremented by one second per bridge we relay through. * The timer fields are in 1/256 s on the wire, and sdcc stores uint16 * little-endian, so assigning the plain second count lands the value in @@ -898,8 +913,8 @@ static int8_t stp_msg_vs_ours(uint8_t port) __reentrant return stp_cmp; if (STP_I->port_prio != stp_pprio[port]) return STP_I->port_prio < stp_pprio[port] ? -1 : 1; - if (STP_I->port_id != port + 1) - return STP_I->port_id < port + 1 ? -1 : 1; + if (STP_I->port_id != stp_pnum(port)) + return STP_I->port_id < stp_pnum(port) ? -1 : 1; return 0; } @@ -1047,7 +1062,6 @@ void stp_in(void) __banked stp_loop_peer = STP_I->port_id; /* 1-based, as we send it */ if (!stp_loop_peer) return; - stp_loop_peer--; /* A Port ID is (priority, number) and priority is compared * first - stp_cnf_send() puts stp_pprio[] on the wire next to * the number, so "stp port N prio" has to be able to decide @@ -1056,10 +1070,10 @@ void stp_in(void) __banked if (STP_I->port_prio != stp_pprio[port]) { if (STP_I->port_prio < stp_pprio[port]) return; /* peer is better: it decides */ - } else if (stp_loop_peer < port) { + } else if (stp_loop_peer < stp_pnum(port)) { return; } - stp_loop_hold_peer(stp_loop_peer); + stp_loop_hold_peer(stp_ent_by_pnum(stp_loop_peer)); return; } diff --git a/test/stp/stp_sim.c b/test/stp/stp_sim.c index 12f93853..916e5104 100644 --- a/test/stp/stp_sim.c +++ b/test/stp/stp_sim.c @@ -17,7 +17,7 @@ uint8_t uip_buf[UIP_CONF_BUFFER_ALLOC_SIZE]; uint16_t uip_len; struct uip_eth_addr uip_ethaddr = { .addr = { 0x1c, 0x2a, 0xa3, 0x1a, 0x72, 0x4e } }; const struct machine machine = { - .log_to_phys_port = { 1, 2, 3, 4, 5, 6, 7, 8, 9 } + .log_to_phys_port = { 1, 2, 3, 4, 5, 6, 10, 8, 9 } }; struct machine_runtime machine_detected = { .isRTL8373 = 1 }; uint8_t sfr_data[4]; @@ -2049,7 +2049,7 @@ static void scen_lag(void) t.legacy = 2; bpdu_in(&t); check(tx_frames[1] == 1 && tx_frames[2] == 0, "a reply leaves through the lowest member"); - check(last_tx[1].port_id == STP_LAG_BASE + 1, "with the group's own port id"); + check(last_tx[1].port_id == 101, "with the group's own port id"); memset(flush_count, 0, sizeof(flush_count)); links_set((1 << 2) | (1 << 8)); @@ -2292,6 +2292,24 @@ static void scen_mikrotik_wire_capture(void) check(attributed, "both VLAN layouts select the captured root and change RX and edge status only on the ingress port"); } +static void scen_port_id_panel(void) +{ + struct sim_bpdu own = { + .port = 0, .root_prio = 0x40, .root_cost = 20000, + .br_prio = 0x80, .port_id = 10 + }; + + printf("66. the port ID is the front panel number, not the chip port\n"); + reset_all(); + links_set((1 << 0) | (1 << 6) | (1 << 8)); + follow_root(5); + check(last_tx[6].port_id == 10, "chip port 6 sends front panel number 10"); + memcpy(own.root_mac, ROOT_MAC, 6); + memcpy(own.br_mac, uip_ethaddr.addr, 6); + bpdu_in(&own); + check_state(6, 1, "its BPDU coming back on another port blocks chip port 6"); +} + int main(int argc, char **argv) { verbose = argc > 1 && argv[1][0] == '-' ? (argv[1][1] == 'd' ? 2 : 1) : 0; @@ -2353,6 +2371,7 @@ int main(int argc, char **argv) scen_loop_peer_heard_bpdu(); scen_loop_peer_autoedge_off(); scen_mikrotik_wire_capture(); + scen_port_id_panel(); if (failures) { printf("\n%d check(s) failed\n", failures); return 1;