Re: [PATCH v2 net-next 1/2] net: dsa: tag_dsa: Unify regular and ethertype DSA taggers

["Tobias Waldekranz" <tobias@waldekranz.com>]

On Sat Nov 14, 2020 at 5:44 PM CET, Andrew Lunn wrote:
> > > > + *
> > > > + * A 3-bit code is used to relay why a particular frame was sent to
> > > > + * the CPU. We only use this to determine if the packet was mirrored
> > > > + * or trapped, i.e. whether the packet has been forwarded by hardware
> > > > + * or not.
> > >
> > > Maybe add that, not all generations support all codes.
> > 
> > Not sure I have that information.
>
> I'm not asking you list per code which switches support it. I'm just
> think we should add a warning, it cannot be assumed all switches
> support all codes. I just looked at the 6161 for example, and it is
> missing 5 from its list.

I see, yeah sure I can do that.

> > That leaves us with DSA_CODE_IGMP_MLD_TRAP. Here is the problem:
> > 
> > Currenly, tag_dsa.c will set skb->offload_fwd_mark for IGMP/MLD
> > packets, whereas tag_edsa.c will exempt them. So we can not unify the
> > two without changing the behavior of one.
>
> I'm not saying that this change is wrong. I'm just afraid as a
> behaviour change, it might break something. If something does break,
> it will be easier to track down, if it is a change on its own. So
> please look if we can add a simple patch to tag_dsa.c which removes
> the marking of such frames. And then the next patch can combine the
> two into one driver. If it does break, git bisect will then tell us
> which patch broke it.

Ahh, I think I see what you are saying now. So I would copy the
IGMP/MLD exemption from tag_edsa.c to tag_dsa.c first, and then apply
the unify patch. Yeah that makes alot of sense, will do!