stp: use the front panel number as the port ID

The port ID in the BPDUs was the chip port plus one, so on most boards
it did not match the port label, neither on the Spanning Tree page nor
on the neighbour. Send the front panel number instead, and 101 and up
for the LAGs, the same numbers the page already uses for the Port
column. A returned BPDU of our own is mapped back to the port by that
number.

Scenario 66 gives one port a front panel number that differs from its
chip port.

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
 

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, "\"},");

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 stp_pnum(e);
-		return e + 101 - STP_LAG_BASE;
-	return machine.log_to_phys_port[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)
+	if (STP_I->port_id != stp_pnum(port))
-		return STP_I->port_id < port + 1 ? -1 : 1;
+		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;
 	}
 

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;