net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port
PATCH 1/1 net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port
A BCM531x5 can be connected to the host through its IMP port (8) or through port 5. With port 5 as the CPU port, b53_brcm_hdr_setup() puts the switch in dual-IMP mode, in which frames trapped for the CPU, BPDUs among them, go to port 8 when they come from a LAN port. b53 sets up no WAN ports, so that is every user port. On boards that use only port 5 and leave port 8 unconnected, BPDUs are lost: STP never sees its peers, and a loop between two user ports takes the host off the network.
Mirror frames sent to the bridge group address from the user ports to port 5 instead. The multiport address registers cannot do this, since the RoboSwitch data sheets resolve reserved multicast before them, and marking the user ports as WAN ports stopped forwarding between them when tried. Other frames trapped for the CPU still go to port 8, as the mirror matches a single destination address. The switch has one mirror, so tc mirroring is refused on these boards.
This was observed on a BCM53125 board in real use, when both user ports were cabled into the same LAN. Tested on that board with a host sending BPDUs into a user port: before, none reached the CPU and the bridge stayed its own root; after, all of them did, the bridge took the sender as root, and forwarding between the user ports kept working. No selftests were run.
---
Sent to net-next rather than net because it takes the mirror away from tc on these boards. I can resend to net with Fixes: 8fab459e69ab ("net:
dsa: b53: Enable Broadcom tags for 531x5/539x families") if you prefer.
Only checked on a BCM53125. BCM5301x boards with port 5 as the only CPU port may have the same problem, but I could not confirm it.
--- drivers/net/dsa/b53/b53_common.c | 24 ++++++++++++++++++++++++ drivers/net/dsa/b53/b53_priv.h | 1 + 2 files changed, 25 insertions(+)
drivers/net/dsa/b53/b53_common.c +24 -0
@@ -797,6 +797,24 @@ void b53_brcm_hdr_setup(struct dsa_switch *ds, int port) hdr_ctl |=GC_FRM_MGMT_PORT_M ;b53_write8 (dev,B53_MGMT_PAGE ,B53_GLOBAL_CONFIG , hdr_ctl); +/* With port 5 as the CPU port, a BCM531x5 runs in dual-IMP mode and +* traps BPDUs from LAN ports to port 8. When port 8 is not a CPU +* port, mirror them to port 5 instead. This takes the switch's only +* mirror, so tc mirroring is refused. +*/ +if (tag_en && port ==5 &&is531x5 (dev) && + !dsa_is_cpu_port (ds,B53_CPU_PORT )) { +b53_write48 (dev,B53_MGMT_PAGE ,B53_IG_MIR_MAC , +ether_addr_to_u64 (eth_stp_addr)); +b53_write16 (dev,B53_MGMT_PAGE ,B53_IG_MIR_CTL , + (MIRROR_DA <<MIRROR_FILTER_SHIFT ) | + (dsa_user_ports (ds) &MIRROR_MASK )); +b53_write16 (dev,B53_MGMT_PAGE ,B53_EG_MIR_CTL ,0 ); +b53_write16 (dev,B53_MGMT_PAGE ,B53_MIR_CAP_CTL , +MIRROR_EN | port); + dev->bpdu_mirror = true; + } +/* B53_BRCM_HDR not present on devices with legacy tags */ if (dev->tag_protocol ==DSA_TAG_PROTO_BRCM_LEGACY || dev->tag_protocol ==DSA_TAG_PROTO_BRCM_LEGACY_FCS )
@@ -2604,6 +2622,12 @@ int b53_mirror_add(struct dsa_switch *ds, int port,struct b53_device *dev = ds->priv;u16 reg, loc; +if (dev->bpdu_mirror) { +NL_SET_ERR_MSG_MOD (extack, +"The mirror carries BPDUs to CPU port 5" ); +return -EBUSY ; + } +if (ingress) loc =B53_IG_MIR_CTL ;else
drivers/net/dsa/b53/b53_priv.h +1 -0
@@ -170,6 +170,7 @@ struct b53_device {/* run time configuration */ bool enable_jumbo; +bool bpdu_mirror;unsigned int num_vlans;struct b53_vlan *vlans;
---
base-commit: d26955e57475434b59d43ba372b2a50d33d5e0ed
change-id: 20261109-b53-bpdu-mirror-d3f66c7a7fd2
Best regards,
Signature · 1 line
Joseph Telaak <[email protected]>
Jonas Gorski
maintainerHi,
Quoted 17 lines · Joseph Telaak
A BCM531x5 can be connected to the host through its IMP port (8) or through port 5. With port 5 as the CPU port,
b53_brcm_hdr_setup()puts the switch in dual-IMP mode, in which frames trapped for the CPU, BPDUs among them, go to port 8 when they come from a LAN port. b53 sets up no WAN ports, so that is every user port. On boards that use only port 5 and leave port 8 unconnected, BPDUs are lost: STP never sees its peers, and a loop between two user ports takes the host off the network.Mirror frames sent to the bridge group address from the user ports to port 5 instead. The multiport address registers cannot do this, since the RoboSwitch data sheets resolve reserved multicast before them, and marking the user ports as WAN ports stopped forwarding between them when tried. Other frames trapped for the CPU still go to port 8, as the mirror matches a single destination address. The switch has one mirror, so tc mirroring is refused on these boards.
Quoted 6 lines
This was observed on a BCM53125 board in real use, when both user ports were cabled into the same LAN. Tested on that board with a host sending BPDUs into a user port: before, none reached the CPU and the bridge stayed its own root; after, all of them did, the bridge took the sender as root, and forwarding between the user ports kept working. No selftests were run.
So on your board only port 5 is wired up, and port 8 is unpopulated and no way to make it work?
Be aware that more than BPDUs are broken by this, as anything trapped to CPU is not working. This also affects traffic on ports outside of the bridge, other trapped packets in the 01:80:c2:00:00:0* range, or IGMP/MLD snooped packets (or any other L2 protocol snooping).
Quoted 9 lines
Signed-off-by Joseph Telaak <[email protected]>---
Sent to net-next rather than net because it takes the mirror away from tc on these boards. I can resend to net with Fixes:8fab459e69ab("net:
dsa: b53: Enable Broadcom tags for 531x5/539x families") if you prefer.Only checked on a BCM53125. BCM5301x boards with port 5 as the only CPU port may have the same problem, but I could not confirm it.
They do, though there it's "easy" to fix by switching to port 8 in the device tree, since all three ports are AFAIK always hardwired in the SoC.
Quoted 10 lines
--- drivers/net/dsa/b53/b53_common.c | 24 ++++++++++++++++++++++++ drivers/net/dsa/b53/b53_priv.h | 1 + 2 files changed, 25 insertions(+)drivers/net/dsa/b53/b53_common.c +0
-0
drivers/net/dsa/b53/b53_common.c void b53_brcm_hdr_setup(struct dsa_switch *ds, int port) · line 806
Quoted 10 lines
drivers/net/dsa/b53/b53_common.c +7
-0@@ -797,6 +797,24 @@ void b53_brcm_hdr_setup(struct dsa_switch *ds, int port) hdr_ctl |=GC_FRM_MGMT_PORT_M ;b53_write8 (dev,B53_MGMT_PAGE ,B53_GLOBAL_CONFIG , hdr_ctl); +/* With port 5 as the CPU port, a BCM531x5 runs in dual-IMP mode and +* traps BPDUs from LAN ports to port 8. When port 8 is not a CPU +* port, mirror them to port 5 instead. This takes the switch's only +* mirror, so tc mirroring is refused. +*/ +if (tag_en && port ==5 &&is531x5 (dev) && + !dsa_is_cpu_port (ds,B53_CPU_PORT )) {
So this should be extended to all dual IMP capable chips, because they are all equally affected.
drivers/net/dsa/b53/b53_common.c void b53_brcm_hdr_setup(struct dsa_switch *ds, int port) · line 814
Quoted 8 lines
drivers/net/dsa/b53/b53_common.c +8
-0+b53_write48 (dev,B53_MGMT_PAGE ,B53_IG_MIR_MAC , +ether_addr_to_u64 (eth_stp_addr)); +b53_write16 (dev,B53_MGMT_PAGE ,B53_IG_MIR_CTL , + (MIRROR_DA <<MIRROR_FILTER_SHIFT ) | + (dsa_user_ports (ds) &MIRROR_MASK )); +b53_write16 (dev,B53_MGMT_PAGE ,B53_EG_MIR_CTL ,0 ); +b53_write16 (dev,B53_MGMT_PAGE ,B53_MIR_CAP_CTL , +MIRROR_EN | port);
Have you tried a catch-all port 8 egress to port 5 mirror? If that happens to work, it should catch any trapped traffic. May not work while the port is disabled in software though.
Quoted 7 lines
drivers/net/dsa/b53/b53_common.c +3
-0+ dev->bpdu_mirror = true; + } +/* B53_BRCM_HDR not present on devices with legacy tags */ if (dev->tag_protocol ==DSA_TAG_PROTO_BRCM_LEGACY || dev->tag_protocol ==DSA_TAG_PROTO_BRCM_LEGACY_FCS )
drivers/net/dsa/b53/b53_common.c int b53_mirror_add(struct dsa_switch *ds, int port, · line 2628
Quoted 7 lines
drivers/net/dsa/b53/b53_common.c +4
-0@@ -2604,6 +2622,12 @@ int b53_mirror_add(struct dsa_switch *ds, int port,struct b53_device *dev = ds->priv;u16 reg, loc; +if (dev->bpdu_mirror) { +NL_SET_ERR_MSG_MOD (extack, +"The mirror carries BPDUs to CPU port 5" ); +return -EBUSY ;
I think this should be rather -EOPNOTSUPP, as with this setup we will never be able to support it; -EBUSY feels more like a temporary state.
Best regards,
Jonas