Re: [PATCH net 1/1] net: stmmac: Prevent DSA tags from breaking COE

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi Linus,

On Tue, 19 Dec 2023 15:19:45 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Tue, Dec 19, 2023 at 2:07 PM Maxime Chevallier
> <maxime.chevallier@bootlin.com> wrote:
> 
> > So it looks like an acceptable solution would be something along the
> > lines of what Linus is suggesting here :
> >
> > https://lore.kernel.org/netdev/20231216-new-gemini-ethernet-regression-v2-2-64c269413dfa@linaro.org/
> >
> > If so, maybe it's worth adding a new helper for that check ?  
> 
> Yeah it's a bit annoying when skb->protocol is not == ethertype of buffer.
> 
> I can certainly add a helper such as skb_eth_raw_ethertype()
> to <linux/if_ether.h> that will inspect the actual ethertype in
> skb->data.
> 
> It's the most straight-forward approach.

Agreed :)

> We could also add something like bool custom_ethertype; to
> struct sk_buff and set that to true if the tagger adds a custom
> ethertype. But I don't know how the network developers feel about
> that.

I don't think this would be OK, first because sk_buff is pretty
sensitive when it comes to cache alignment, adding things for this kind
of use-cases isn't necessarily a good idea. Moreover, populating this
flag isn't going to be straightforward as well. I guess some ethertype
would be compatible with checksum engines, while other wouldn't, so
probably what 'custom_ethertype' means will depend on the MAC driver.

From my point of view the first approach would indeed be better.

Thanks,

Maxime