Merge pull request #737 from DrDoof/fix/stp-port-id-panel

stp: use the front panel number as the port ID

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 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;
 	}
 

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;