[PATCH net-next] net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

[PATCH net-next] net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

[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.

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.

Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
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(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a2a5754c413c..6b33775c7598 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -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
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index c4d2a41c714f..6269c8b3c95a 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -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,
--  
Joseph Telaak <joseph@telaak.dev>

Re: [PATCH net-next] net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

[Jonas Gorski]

Hi,

On Wed, Nov 11, 2026 at 7:31 AM Joseph Telaak <joseph@telaak.dev> wrote:
>
> 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.

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).

>
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>
> ---
> 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.

> ---
>  drivers/net/dsa/b53/b53_common.c | 24 ++++++++++++++++++++++++
>  drivers/net/dsa/b53/b53_priv.h   |  1 +
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index a2a5754c413c..6b33775c7598 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -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.

> +               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.

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

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

Re: [PATCH net-next] net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

[Florian Fainelli]

On 11/11/26 02:11, Jonas Gorski wrote:
> Hi,
> 
> On Wed, Nov 11, 2026 at 7:31 AM Joseph Telaak <joseph@telaak.dev> wrote:
>>
>> 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.
> 
> 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).
> 
>>
>> Signed-off-by: Joseph Telaak <joseph@telaak.dev>
>> ---
>> 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.

Yes, I have a series of patches doing that, because using port 5 is just 
not working for everything.

> 
>> ---
>>   drivers/net/dsa/b53/b53_common.c | 24 ++++++++++++++++++++++++
>>   drivers/net/dsa/b53/b53_priv.h   |  1 +
>>   2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
>> index a2a5754c413c..6b33775c7598 100644
>> --- a/drivers/net/dsa/b53/b53_common.c
>> +++ b/drivers/net/dsa/b53/b53_common.c
>> @@ -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.
> 
>> +               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.

We could also try to use the CFP to perform a full redirection to port 8.

> 
>> +               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;
> 
> 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.

Agreed.
-- 
Florian

Re: [PATCH net-next] net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

[Jonas Gorski]

On Wed, Nov 11, 2026 at 6:22 PM Florian Fainelli
<florian.fainelli@broadcom.com> wrote:
>
> On 11/11/26 02:11, Jonas Gorski wrote:
> > Hi,
> >
> > On Wed, Nov 11, 2026 at 7:31 AM Joseph Telaak <joseph@telaak.dev> wrote:
> >>
> >> 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.
> >
> > 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).
> >
> >>
> >> Signed-off-by: Joseph Telaak <joseph@telaak.dev>
> >> ---
> >> 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.
>
> Yes, I have a series of patches doing that, because using port 5 is just
> not working for everything.
>
> >
> >> ---
> >>   drivers/net/dsa/b53/b53_common.c | 24 ++++++++++++++++++++++++
> >>   drivers/net/dsa/b53/b53_priv.h   |  1 +
> >>   2 files changed, 25 insertions(+)
> >>
> >> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> >> index a2a5754c413c..6b33775c7598 100644
> >> --- a/drivers/net/dsa/b53/b53_common.c
> >> +++ b/drivers/net/dsa/b53/b53_common.c
> >> @@ -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.
> >
> >> +               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.
>
> We could also try to use the CFP to perform a full redirection to port 8.

CFP isn't supported in all bcm531x5 chip variants. E.g BCM53115M and
BCM53125 do, wile BCM53115S and BCM53125S don't. And likely the
presumably lower cost S variants will be found most (at least the
board I use for testing has one).

So we could at most use it as an alternative path where supported to
free up mirroring. Checking if we have CFP can be done via the Bonding
Pad Status Register.

Best regards,
Jonas

Re: [PATCH net-next] net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

[Joseph Telaak]

Hi Jonas,

> So on your board only port 5 is wired up, and port 8 is unpopulated
> and no way to make it work?

Yes. The SoC has a single GbE MAC, wired to port 5 over RGMII, and
port 8 is not connected.

> 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.

I tried it on the BCM53125 with a host sending test frames into a user
port:

- Standalone port: without a mirror nothing reaches the CPU, not even
  broadcast. With the port 8 egress mirror everything does except
  01:80:c2:00:00:03, but only with Rx/Tx enabled on port 8 and its
  link forced up in the IMP override register. EAPOL sent to the
  host's own MAC does get through.
- Bridged port: the egress mirror does not catch BPDUs, whatever the
  state of port 8. The DA mirror from v1 does. 01:80:c2:00:00:02/03/0d
  don't reach the CPU with either.
- IGMP: with the CPU copy bits from 3362cb822ea7 set by hand, reports
  for a group the bridge has offloaded reach the CPU only with the
  egress mirror. I couldn't test MLD, as the board's kernel has no
  IPv6.
- Both mirrors can run at once. Bridged BPDUs, IGMP reports and
  standalone traffic then reach the CPU, each frame once, and
  forwarding keeps working.

So for v2 I would set up both mirrors, enable port 8 and force its
link, on all dual IMP chips, and refuse tc mirroring with -EOPNOTSUPP.
Does that sound reasonable?

Joseph

Re: [PATCH net-next] net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

[Joseph Telaak]

> > We could also try to use the CFP to perform a full redirection to port 8.
>
> CFP isn't supported in all bcm531x5 chip variants. E.g BCM53115M and
> BCM53125 do, wile BCM53115S and BCM53125S don't. And likely the
> presumably lower cost S variants will be found most (at least the
> board I use for testing has one).
>
> So we could at most use it as an alternative path where supported to
> free up mirroring. Checking if we have CFP can be done via the Bonding
> Pad Status Register.

The CFP redirect would be a nice fit where it exists, since it leaves
the mirror free for tc. This board has a BCM53125M, so I can try it
there. For v2 I'd start with the mirrors, since they work on both
variants, and add the CFP path for the M parts as a follow-up if you
think it's worth it.

Joseph

Re: [PATCH net-next] net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The new condition in b53_brcm_hdr_setup() calls
  `!dsa_is_cpu_port(ds, B53_CPU_PORT)` (port 8) without checking that…
- [Low] The commit fixes a user-visible bug but has no Fixes: tag.

--

net: dsa: b53: mirror BPDUs to CPU port 5 when port 8 is not a CPU port

On BCM531x5 switches where port 5 is the CPU port and port 8 is not,
this patch uses the ingress mirror to copy frames sent to the bridge
group address from the user ports to port 5. It also refuses tc
mirroring on those boards, because the switch has only one mirror.

> Signed-off-by: Joseph Telaak <joseph@telaak.dev>

[Severity: Low]
Should this commit carry a Fixes: tag?

The notes below the --- line name 8fab459e69ab ("net: dsa: b53: Enable
Broadcom tags for 531x5/539x families"), but those notes are dropped
when the patch is applied.

That commit added the dual-IMP selection for port 5 in
b53_brcm_hdr_setup():

	else if (port == 5)
		hdr_ctl |= GC_FRM_MGMT_PORT_M;

Without the tag, nothing in the applied commit records which kernels
lose BPDUs on these boards.

> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index a2a5754c413c6..6b33775c75985 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -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)) {

[Severity: High]
Can dsa_is_cpu_port(ds, B53_CPU_PORT) dereference a NULL pointer when
port 8 has no dsa_port?

DSA creates dsa_port objects only for indices below ds->num_ports
(dsa_switch_touch_ports()). b53_switch_init() derives that count from
enabled_ports:

	dev->num_ports = fls(dev->enabled_ports);

	dev->ds->num_ports = min_t(unsigned int, dev->num_ports, DSA_MAX_PORTS);

When platform data is supplied (for example, b53_spi_probe() takes it
from spi->dev.platform_data), b53_switch_register() replaces the chip
table mask:

		dev->enabled_ports = dev->pdata->enabled_ports;

Take a BCM531x5 described this way, using port 5 as the CPU port and
with BIT(8) cleared in enabled_ports (for example 0x3f). That leaves
ds->num_ports at 6, and the call chain becomes:

b53_setup()
  b53_enable_cpu_port(dev, 5)
    b53_brcm_hdr_setup(ds, 5)
      dsa_is_cpu_port(ds, 8)
        dsa_to_port(ds, 8)    <- no index 8 on dst->ports, returns NULL
        ->type                <- NULL dereference

dsa_to_port() returns NULL when no port matches, and dsa_is_cpu_port()
uses the result without checking it. Before this patch, b53 did not look
up the dsa_port for B53_CPU_PORT.

Boards probed from DT get the chip table mask, which includes port 8,
so only boards using b53_platform_data can hit this.

Could a missing port 8 count as not a CPU port, so the BPDU mirror is
still installed? Something like:

	(B53_CPU_PORT >= ds->num_ports || !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));

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111-b53-bpdu-mirror-v1-1-f9f32e9d25db%40telaak.dev