linux-kernel - Re: [PATCH 08/14] net: vlan: remove invalid VLAN protocol warning

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Nov 2022 11:42:09 +0100
From:   Felix Fietkau <nbd@....name>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 08/14] net: vlan: remove invalid VLAN protocol warning

On 08.11.22 11:33, Vladimir Oltean wrote:
> On Tue, Nov 08, 2022 at 11:24:51AM +0100, Felix Fietkau wrote:
>> On 08.11.22 11:08, Vladimir Oltean wrote:
>> > On Tue, Nov 08, 2022 at 10:46:54AM +0100, Felix Fietkau wrote:
>> > > I think it depends on the hardware. If you can rely on the hardware always
>> > > reporting the port out-of-band, a generic "oob" tagger would be fine.
>> > > In my case, it depends on whether a second non-DSA ethernet MAC is active on
>> > > the same device, so I do need to continue using the "mtk" tag driver.
>> > 
>> > It's not only about the time dimension (always OOB, vs sometimes OOB),
>> > but also about what is conveyed through the OOB tag. I can see 2 vendors
>> > agreeing today on a common "oob" tagger only to diverge in the future
>> > when they'll need to convey more information than just port id. What do
>> > you do with the tagging protocol string names then. Gotta make them
>> > unique from the get go, can't export "oob" to user space IMO.
>> > 
>> > > The flow dissector part is already solved: I simply used the existing
>> > > .flow_dissect() tag op.
>> > 
>> > Yes, well this seems like a generic enough case (DSA offload tag present
>> > => don't shift network header because it's where it should be) to treat
>> > it in the generic flow dissector and not have to invoke any tagger-specific
>> > fixup method.
>> 
>> In that case I think we shouldn't use METADATA_HW_PORT_MUX, since it is
>> already used for other purposes. I will add a new metadata type METADATA_DSA
>> instead.
> 
> Which case, flow dissector figuring out that DSA offload tag is present?
> Well, the skb can only carry one dst pointer ATM, so if it's METADATA_HW_PORT_MUX
> but it belongs to SR-IOV on the DSA master, we have bigger problems anyway.
> So, proto == ETH_P_XDSA && have METADATA_HW_PORT_MUX should be pretty
> good indication that DSA offload tag is present.
> 
> Anyway I raised this concern as well to Jakub as well. Seems to be
> theoretical at the moment. Using METADATA_HW_PORT_MUX seems to be fine
> right now. Can be changed later if needed; it's not ABI.
Okay, I will stick with METADATA_HW_PORT_MUX for now. If we use it in 
the flow dissector to avoid the tagger specific fixup, we might as well 
use it in DSA to skip the tag proto receive call. What do you think?

- Felix