netdev - Re: [PATCH 2/2] net: dsa: tag_ksz: dont let the hardware process the layer 4 checksum

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]

Message-ID: <20210715143648.yutq6vceoblnhhfp@skbuf>
Date:   Thu, 15 Jul 2021 17:36:48 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Lino Sanfilippo <LinoSanfilippo@....de>, woojung.huh@...rochip.com,
        UNGLinuxDriver@...rochip.com, vivien.didelot@...il.com,
        f.fainelli@...il.com, davem@...emloft.net, kuba@...nel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] net: dsa: tag_ksz: dont let the hardware process the
 layer 4 checksum

On Thu, Jul 15, 2021 at 03:08:53PM +0200, Andrew Lunn wrote:
> > - If we inherit NETIF_F_HW_CSUM from the master for tail taggers, it is
> >   actively detrimential to keep this feature enabled, as proven my Lino.
> >   As for header taggers, I fail to see how this would be helpful, since
> >   the DSA master would always fail to see the real IP header (it has
> >   been pushed to the right by the DSA tag), and therefore, the DSA
> >   master offload would be effectively bypassed.
>
> The Marvell MACs know about DSA and should be able to perform hardware
> checksumming. It is a long time since i looked at how this works, but
> i think there is a field in the descriptor which gets set with the
> offset to the IP header, so it work for DSA as well as EDSA.
>
> I _think_ Broadcom MACs also know about Broadcom tags and can do the
> right thing.
>
> So we need to be a bit careful here to prevent performance regressions
> for same vendor MAC+Switch combinations.

Tell me more (show me some code). Do Marvell Ethernet controllers which
support TX checksumming with Marvell switches do different things
depending on whether DSA or EDSA is used? Because we can currently
toggle between DSA and EDSA at runtime.

This new information means we can only accept Lino's patch 2/2 as-is for
the "net" tree, otherwise we will introduce regressions one way or
another. It will only be a partial fix for the particular case of KSZ
switches which probably have no DSA master counterpart to support TX
checksumming.

I expect Marvell switches to be equally broken on the Broadcom genet
controller? No one will provide the TX checksum in that case. And that
is not even "fixable" without devising a system where custom code can be
run per {tagger, DSA master} pair, and this includes the case where the
tagging protocol changes at runtime.