Subject: Re: [PATCH v2 net 4/4] net: dsa: mv88e6xxx: Limit rsvd2cpu policy to user ports on 6393X

From: Tobias Waldekranz <[email-masked]>
Date: Thu, 19 Dec 2024 15:34:43 +0100

On tor, dec 19, 2024 at 16:05, Vladimir Oltean <[email-masked]> wrote:
> On Thu, Dec 19, 2024 at 01:30:43PM +0100, Tobias Waldekranz wrote:
>> For packets with a DA in the IEEE reserved L2 group range, originating
>> from a CPU, forward it as normal, rather than classifying it as
>> management.
>
> Doesn't this break STP? Must be able to inject into ports with an STP
> state other than FORWARDING. I expect that you need a DSA_CMD_FROM_CPU
> tag for that, can't do it with DSA_CMD_FORWARD.

You need a FROM_CPU to force a packet out through a blocking port,
yes. But I don't see how this could apply to STP.

If STP is enabled on the bridge, we will never allow BPDUs to be
forwarded. Locally originating STP BPDUs are always injected directly on
the lower interface, so OFM is never set on those.

If STP is disabled on the bridge, then we will forward incoming BPDUs
(br_handle_frame()). In that case OFM could be set if the BPDU came in
on a foreign port. But since STP is disabled, no port will be blocked in
this case, so it would not matter.

>> Example use-case:
>> 
>>      bridge (group_fwd_mask 0x4000)
>>      / |  \
>>  swp1 swp2 tap0
>>    \   /
>> (mv88e6xxx)
>> 
>> We've created a bridge with a non-zero group_fwd_mask (allowing LLDP
>> in this example) containing a set of ports managed by mv88e6xxx and
>> some foreign interface (e.g. an L2 VPN tunnel).
>> 
>> Since an LLDP packet coming in to the bridge from the other side of
>> tap0 is eligable for tx forward offloading, a FORWARD frame destined
>> for swp1 and swp2 would be send to the conduit interface.
>> 
>> Before this change, due to rsvd2cpu being enabled on the CPU port, the
>> switch would try to trap it back to the CPU. Given that the CPU is
>> trusted, instead assume that it indeed meant for the packet to be
>> forwarded like any other.
>
> It looks like an oversight in the switchdev tx_fwd_offload scheme. Can't
> we teach nbp_switchdev_frame_mark_tx_fwd_offload() to make an exception
> for is_link_local_ether_addr() packets, and not set skb->offload_fwd_mark?

That sounds like a better option if it is acceptible to the broader
community. I thought that this might be a quirk of mv88e6xxx's rsvd2cpu
bits. But if more devices behave in the same way, then it would be
better to just exempt this whole class from offloading.

Do you know how any other ASICs behave from this perspective?