trap: deliver reserved multicast to the CPU by trap, STP first

A reserved-multicast trap goes to the port named in EXT_CPU_CTRL, which
names no port out of reset, so trapped frames were dropped at ingress.
trap_init() points it at the CPU port once at startup, and
trap_rma_set() changes only the action field of an RMA register.

A trapped frame arrives without the VLAN tag a forwarded one gets, so
TRAP_RX_BODY() gives the offset of the payload for both forms.

STP now traps its address while it runs instead of steering it with a
static L2 entry per VLAN, so turning it on no longer walks the VLAN
table and a pvid change no longer needs stp off and on. With STP off
the flooding entries are restored as before.

A trapped BPDU passes the tagged-only ingress filter that drops a
forwarded one, so the stp_setup() warning about such ports goes.

Makefile

@@ -89,7 +89,8 @@ SRCS += \
 	rtl837x_port.c \
 	rtl837x_stp.c \
 	rtl837x_lldp.c \
-	rtl837x_storm.c
+	rtl837x_storm.c \
+	rtl837x_trap.c
 SRCS += \
 	httpd/httpd.c \
 	httpd/page_impl.c

doc/l2.md

@@ -73,13 +73,14 @@ without being flooded to the other ports. No bridge relays these frames:
 their addresses are in the set that 802.1D-2004 clause 7.12.6 forbids a
 bridge to forward, and what travels the network is the information, with
 every bridge regenerating BPDUs of its own on its designated ports. The reserved-multicast *trap* action
-cannot do that on this hardware, because its destination is an external CPU
+does that: it delivers to the port named in `EXT_CPU_CTRL` (0x6724), which
-attached to a physical port, which these boards do not populate. The protocol
+`trap_init()` sets to 9, the CPU port, at startup; with the reset value 0xf it
-modules therefore leave the reserved-multicast action at *forward* and constrain
+names no port and the trapped frame is dropped. STP traps its address while it
-the egress with a static L2 multicast entry instead: the lookup hits the entry's
+runs (see stp.md). Where the action is left at *forward*, the egress can be
-own port mask rather than the VLAN flood mask. Verified on a SWTGW218AS both
+constrained with a static L2 multicast entry instead: the lookup hits the
-ways, with the CPU bit cleared, where delivery stops, and with the CPU bit alone,
+entry's own port mask rather than the VLAN flood mask. Verified on a SWTGW218AS
-where nothing egresses.
+both ways, with the CPU bit cleared, where delivery stops, and with the CPU bit
+alone, where nothing egresses.
 
 `port_l2mc_set()` writes one such entry. The SMI layout is the L2 multicast
 variant of the table entry:

doc/stp.md

@@ -35,28 +35,34 @@ BPDUs are addressed to `01:80:C2:00:00:00`, a reserved link-local group. The
 ASIC's Reserved-Multicast action for that address decides what happens to the
 frame.
 
-Forwarding to the CPU port works normally: the 8051 sits behind an ordinary
+While STP runs, the action is *trap*. A trap delivers to the port named in
-port of the internal switch and is an ordinary member of a forwarding mask.
+`EXT_CPU_CTRL` (0x6724), whose reset value 0xf names no port, and with that
-The *trap* action does not deliver to it. Its destination is an external CPU
+value the trapped frame is dropped at ingress. `trap_init()` sets it to 9, the
-attached to a physical port (`cpuTag_externalCpuPort_set`, `EXT_CPU_CTRL` in
+port the 8051 sits behind, once at startup. The trap needs no per-VLAN L2
-the vendor SDK), which these boards do not populate. The ACL trap and
+entries, so it does not depend on which VLANs exist or on the PVIDs, and turning
-redirect actions do not deliver to the 8051 either.
+STP on no longer walks the whole VLAN table. BPDUs reach the CPU on a port that
+STP holds in blocking with either action: measured on a SWTGW218AS by sending
+BPDUs into a blocked port with forward and with trap, both ports stopped being
+edge and waited out the forward delay. The chip exempts this address from the
+ingress state check; the slow-protocols address used by LACP is not exempt.
 
-Delivery therefore uses the *forward* action, constrained to the CPU port
+A trapped frame reaches the CPU in the format it came in with: unlike a
-by a static L2 multicast entry (`port_l2mc_set()`), one per VLAN in use:
+forwarded one it has no VLAN tag inserted after the RTL tag, and the RTL tag
+carries the trap reason. `stp_in()` reads the BPDU from the offset
+`TRAP_RX_BODY()` returns, which works for both.
 
-* while STP runs, the entry's member mask is the CPU port only — BPDUs reach
+With STP off, the action goes back to *forward* and a static L2 multicast
-  the CPU and are not flooded to other ports, as a participating bridge
+entry per VLAN in use (`port_l2mc_set()`) targets all ports, restoring the
-  requires;
-* with STP off, the same entries are retargeted to all ports, restoring the
 transparency an unmanaged switch is expected to have, so a surrounding
 spanning tree can span *through* this device.
 
-A BPDU delivered this way is an ordinary frame to the port's ingress logic
+The port's acceptable-frame-type filter treats the two actions differently.
-and passes through its acceptable-frame-type filter. BPDUs are untagged, so a
+BPDUs are untagged, so a port set to admit tagged frames only
-port set to admit tagged frames only (`ingress <port>t`) never delivers one
+(`ingress <port>t`) drops a forwarded BPDU before it reaches the CPU, while a
-to the CPU. `stp_setup()` prints a warning for every STP-enabled port in that
+trapped one passes. Both were measured on a SWTGW218AS with `ingress 3t`:
-state.
+LACPDUs, delivered by forward, stopped arriving on port 3, and BPDUs sent into
+the same port, delivered by trap, kept arriving. A port that admits tagged
+frames only therefore takes part in STP.
 
 ## Link aggregation
 

rtl837x_regs.h

@@ -285,6 +285,12 @@
  */
 #define RTL8373_RLDP_TIMER		0x1074
 #define RTL837X_RMA0_CONF		0x4ecc
+#define RTL837X_RMA2_CONF		0x4ed4
+#define RTL837X_RMA_ACT_MASK		0x30
+#define RTL837X_RMA_ACT_FORWARD		0x00
+#define RTL837X_RMA_ACT_TRAP		0x10
+#define RTL837X_RMA_ACT_DROP		0x20
+#define RTL837X_EXT_CPU_CTRL		0x6724
 #define RTL837X_IMR_INT_PORT_LINK_STS_CHG	0x5f34
 #define RTL837X_ISR_SW_INT_MODE		0x5f84
 #define RTL837X_ISR_INT_PORT_LINK_CHG	0x5f88

rtl837x_stp.c

@@ -12,6 +12,7 @@
 #include "rtl837x_regs.h"
 #include "rtl837x_stp.h"
 #include "rtl837x_port.h"
+#include "rtl837x_trap.h"
 #include "uip.h"
 #include "machine.h"
 
@@ -85,6 +86,7 @@ __xdata uint16_t stp_link_now;
 
 __xdata uint8_t  stp_scratch;
 __xdata uint8_t  stp_tx_flags_extra;	/* one-shot flags OR-ed into the next BPDU (TCA) */
+__xdata uint8_t  stp_rxbody;
 __xdata uint16_t stp_rxlen;		/* received frame length, saved before uip_len is consumed */
 __xdata uint8_t  stp_msg_age;		/* message age of the root info we hold, seconds */
 __xdata uint16_t stp_tc_while;		/* ticks left to set the TC flag in our BPDUs */
@@ -121,11 +123,13 @@ struct stp_pkt {
 	uint8_t version1_length;	/* RST BPDU only: length of the (empty) v1 part */
 };
 
-struct stp_pkt_in {
+struct stp_pkt_in_hdr {
 	uint8_t stp_addr[6];
 	uint8_t src_addr[6];
 	struct rtl_tag rtl_tag;
-	struct vlan_tag vlan_tag;
+};
+
+struct stp_pkt_in {
 	uint16_t msg_len;
 	uint8_t dsap;
 	uint8_t ssap;
@@ -147,7 +151,8 @@ struct stp_pkt_in {
 };
 
 #define STP_O ((__xdata struct stp_pkt *)&uip_buf[RTL_FRAME_DESC_SIZE])
-#define STP_I ((__xdata struct stp_pkt_in *)&uip_buf[0])
+#define STP_H ((__xdata struct stp_pkt_in_hdr *)&uip_buf[0])
+#define STP_I ((__xdata struct stp_pkt_in *)&uip_buf[stp_rxbody])
 
 #define BPDU_VER_STP		0x00
 #define BPDU_VER_RSTP		0x02
@@ -158,7 +163,8 @@ struct stp_pkt_in {
 
 #define BPDU_LEN_CONFIG		0x26	// LLC and a 35 byte body
 #define BPDU_LEN_RST		0x27	// LLC and a 36 byte body
-#define BPDU_LEN_MIN_HEADER	33	// addresses through bpdu_type
+#define BPDU_LEN_MIN_BODY	9
+#define BPDU_LEN_FULL_BODY	40
 
 #define BPDU_FLAG_TC		0x01
 #define BPDU_FLAG_LEARNING	0x10
@@ -547,7 +553,8 @@ void stp_in(void) __banked
 {
 	uint8_t port;
 
-	if (uip_len < BPDU_LEN_MIN_HEADER) {
+	stp_rxbody = TRAP_RX_BODY();
+	if (uip_len < stp_rxbody + BPDU_LEN_MIN_BODY) {
 		uip_len = 0;
 		return;
 	}
@@ -557,7 +564,7 @@ void stp_in(void) __banked
 	uip_len = 0;
 
 	/* Ingress port: low nibble of the CPU tag's pmask on RX */
-	stp_scratch = ((uint8_t)HTONS(STP_I->rtl_tag.pmask)) & 0x0f;
+	stp_scratch = ((uint8_t)HTONS(STP_H->rtl_tag.pmask)) & 0x0f;
 	if (stp_scratch < machine.min_port || stp_scratch > machine.max_port)
 		return;
 	port = stp_ent_of[stp_scratch];
@@ -607,7 +614,7 @@ void stp_in(void) __banked
 	}
 
 	/* Everything below reads the full Config/RST body. */
-	if (stp_rxlen < 64)
+	if (stp_rxlen < stp_rxbody + BPDU_LEN_FULL_BODY)
 		return;
 
 	/* Our own BPDU coming back: two of our ports sit on one segment. Only
@@ -865,8 +872,7 @@ void stp_defaults(void) __banked
 
 
 /*
- * Steer BPDUs while STP runs, and restore flooding when it stops.
+ * Restore BPDU flooding when STP stops.
- * Changing a port's PVID while STP runs needs "stp off" then "stp on".
  */
 static void stp_fdb_update(__xdata uint16_t pmask)
 {
@@ -928,16 +934,6 @@ void stp_setup(void) __banked
 
 	print_reg(RTL837X_MSTP_STATES); write_char('\n');
 
-	for (stp_i = machine.min_port; stp_i <= machine.max_port; stp_i++) {
-		if (!(stp_pflags[stp_i] & STP_PF_ENABLED))
-			continue;
-		if (port_ingress_filter_get(stp_i) != VLAN_TAGGED)
-			continue;
-		print_string("STP: port ");
-		write_char('0' + machine.log_to_phys_port[stp_i]);
-		print_string(" admits tagged frames only - BPDUs are untagged and will not arrive\n");
-	}
-
 	/* 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);
@@ -949,8 +945,7 @@ void stp_setup(void) __banked
 
 	stp_claim_root();
 
-	/* Take BPDUs to the CPU only - we are a participating bridge now. */
+	trap_rma_set(RTL837X_RMA0_CONF, RTL837X_RMA_ACT_TRAP);
-	stp_fdb_update(PMASK_CPU);
 }
 
 
@@ -972,6 +967,7 @@ void stp_off(void) __banked
 
 	/* Restore BPDU transparency: flood them again like an unmanaged switch. */
 	stp_fdb_update(PMASK_CPU | (machine_detected.isRTL8373 ? PMASK_9 : PMASK_6));
+	trap_rma_set(RTL837X_RMA0_CONF, RTL837X_RMA_ACT_FORWARD);
 }
 
 

rtl837x_trap.c

@@ -0,0 +1,21 @@
+#include "rtl837x_common.h"
+#include "rtl837x_sfr.h"
+#include "rtl837x_regs.h"
+#include "rtl837x_trap.h"
+
+#pragma codeseg BANK2
+#pragma constseg BANK2
+
+extern __xdata uint8_t sfr_data[4];
+
+void trap_init(void) __banked
+{
+	REG_SET(RTL837X_EXT_CPU_CTRL, CPU_PORT);
+}
+
+void trap_rma_set(uint16_t reg, uint8_t act) __banked
+{
+	reg_read_m(reg);
+	sfr_data[3] = (sfr_data[3] & ~RTL837X_RMA_ACT_MASK) | act;
+	reg_write_m(reg);
+}

rtl837x_trap.h

@@ -0,0 +1,16 @@
+#ifndef _RTL837X_TRAP_H_
+#define _RTL837X_TRAP_H_
+
+#include <stdint.h>
+#include "rtl837x_common.h"
+
+#define TRAP_RX_HDR_SIZE	(12 + RTL_TAG_SIZE)
+
+#define TRAP_RX_BODY() \
+	((uip_buf[TRAP_RX_HDR_SIZE] == 0x81 && uip_buf[TRAP_RX_HDR_SIZE + 1] == 0x00) \
+	 ? TRAP_RX_HDR_SIZE + VLAN_TAG_SIZE : TRAP_RX_HDR_SIZE)
+
+void trap_init(void) __banked;
+void trap_rma_set(uint16_t reg, uint8_t act) __banked;
+
+#endif

rtlplayground.c

@@ -11,6 +11,7 @@
 #include "rtl837x_pins.h"
 #include "rtl837x_phy.h"
 #include "rtl837x_port.h"
+#include "rtl837x_trap.h"
 #include "rtl837x_stp.h"
 #include "rtl837x_igmp.h"
 #include "rtl837x_leds.h"
@@ -1793,6 +1794,7 @@ void main(void)
 	stp_defaults();		/* 802.1D/w default config before any "stp ..." replay */
 	nic_setup();
 	REG_SET(RTL837X_NIC_INT_MSK, NIC_INT_RXIE);
+	trap_init();
 	vlan_setup();
 	port_l2_setup();
 	igmp_setup();