Re: [PATCH net-next 1/2] dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()

[Daniel Golle <daniel@makrotopia.org>]

On Mon, Mar 09, 2026 at 09:07:07AM +0100, Jonas Gorski wrote:
> On Mon, Mar 9, 2026 at 1:02 AM Daniel Golle <daniel@makrotopia.org> wrote:
> >
> > On Sun, Mar 08, 2026 at 09:15:42PM +0100, Jonas Gorski wrote:
> > > On Sun, Mar 8, 2026 at 4:19 PM Daniel Golle <daniel@makrotopia.org> wrote:
> > > >
> > > > On Sat, Mar 07, 2026 at 10:57:56AM +0100, Jonas Gorski wrote:
> > > > > Hi,
> > > > >
> > > > > On 07/03/2026 04:30, Daniel Golle wrote:
> > > > > > The MxL862xx offloads forwarding between bridged ports to the
> > > > > > hardware, so set dsa_default_offload_fwd_mark() to avoid duplicate
> > > > > > forwarding of packets of (eg. flooded) frames arriving at the CPU
> > > > > > port.
> > > > > >
> > > > > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > > > > ---
> > > > > >  net/dsa/tag_mxl862xx.c | 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/net/dsa/tag_mxl862xx.c b/net/dsa/tag_mxl862xx.c
> > > > > > index 01f2158682718..c02f69de61cbb 100644
> > > > > > --- a/net/dsa/tag_mxl862xx.c
> > > > > > +++ b/net/dsa/tag_mxl862xx.c
> > > > > > @@ -92,6 +92,8 @@ static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,
> > > > > >     skb_pull_rcsum(skb, MXL862_HEADER_LEN);
> > > > > >     dsa_strip_etype_header(skb, MXL862_HEADER_LEN);
> > > > > >
> > > > > > +   dsa_default_offload_fwd_mark(skb);
> > > > >
> > > > > Does the switch (by default) also flood link local traffic (e.g. STP, LACP,
> > > > > etc)? If not, you should not mark these as fwd offloaded.
> > > >
> > > > No, it doesn't. Testing revealed that apparently it silently consumes them.
> > > > I assume we'll need something like
> > > >
> > > > if (likely(!is_link_local_ether_addr(eth_hdr(skb)->h_dest)))
> > > >     dsa_default_offload_fwd_mark(skb);
> > > >
> > > > similar to tag_brcm.c, right?
> > >
> > > Yepp, that would have been my suggestion :-)
> >
> > Ok, I'll do that then.
> >
> > >
> > > > > Is there is a bit in the header that says whether a packet was flooded or
> > > > > trapped that you can check?
> > > >
> > > > Earlier testing showed that frames arriving at the microcontroller under
> > > > some conditions are relayed to the DSA CPU port -- and are destinguishable
> > > > as such. However, there is not bit directly indicating whether the packet
> > > > was (also) flooded already.
> > >
> > > So you know if they were copied to the cpu port due to some rules, but
> > > not if they were snooped or trapped?
> >
> > I haven't found any details about that documented, and all I can say
> > is that in some cases (some of the IEEE1588v2 frames generated by the
> > local_termination.sh selftest, for example) the microcontroller seems
> > to send an additional copy of the frame to the DSA CPU port using
> > fields of the 8-byte tag which usually aren't used -- and that's why I
> > even noticed them. The initial driver now merged upstream disabled the
> > microcontroller SDMA and FMDA, effectively disconnecting it from the
> > switch datapath, so it would shut up, and this effect can now longer
> > be observed.
> >
> > Now, when testing if link-local frames would be forwarded I noticed
> > that there is a set of default rules instatiated after reset which
> > trap link-local frames to the microcontroller port. I suppose it could
> > also be setup to snoop instead of trap by manually erasing these PCE
> > rules and setting new ones instead. In this case, however, there would
> > be no way to tell from the frame received on the DSA CPU port if the
> > frame had also been sent to the microcontroller.
> 
> For dynamic configuration you would probably need to then check the
> destination address, check if snooping or trapping is configured for
> it, and then set (not) mark offloaded accordingly. While linux allows
> you do dynamically configure some of the groups between
> flooding/trapping, I'm not sure if this is propagated via dsa or even
> switchdev.
> 
> I suggest sticking to the default of trapping and not marking as
> offloaded. It's much easier to flood in software if needed than the
> other way round.

Yeah, I'll do that and right now I'm trying to come up with a set of
PCE rules which allow for link-local frames to even arrive at the
CPU port at all (which would allow software flooding or even doing
anything with them on the CPU)