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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <C7391HRJGAV2.28OAVQBIGDRAT@wkz-x280>
Date:   Sat, 14 Nov 2020 20:55:42 +0100
From:   "Tobias Waldekranz" <tobias@...dekranz.com>
To:     "Andrew Lunn" <andrew@...n.ch>
Cc:     <davem@...emloft.net>, <kuba@...nel.org>,
        <vivien.didelot@...il.com>, <f.fainelli@...il.com>,
        <olteanv@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net-next 1/2] net: dsa: tag_dsa: Unify regular and
 ethertype DSA taggers

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!