netdev - Re: [PATCH net-next v4 11/11] net: dsa: realtek: rtl8365mb: multiple cpu ports, non cpu extint

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:   Mon, 24 Jan 2022 19:21:58 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Luiz Angelo Daros de Luca <luizluca@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Frank Wunderlich <frank-w@...lic-files.de>,
        Alvin Šipraga <ALSI@...g-olufsen.dk>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "arinc.unal@...nc9.com" <arinc.unal@...nc9.com>
Subject: Re: [PATCH net-next v4 11/11] net: dsa: realtek: rtl8365mb: multiple
 cpu ports, non cpu extint

On Mon, Jan 24, 2022 at 09:01:20AM -0800, Florian Fainelli wrote:
> On 1/24/2022 8:55 AM, Vladimir Oltean wrote:
> > On Mon, Jan 24, 2022 at 08:46:49AM -0800, Jakub Kicinski wrote:
> > > I thought for drivers setting the legacy NETIF_F_IP*_CSUM feature
> > > it's driver's responsibility to validate the geometry of the packet
> > > will work with the parser the device has. Or at least I think that's
> > > what Tom was pushing for when he was cleaning up the checksumming last
> > > (and wrote the long comment on the subject in skbuff.h).
> > 
> > Sorry Jakub, I don't understand what you mean to say when applied to the
> > context discussed here?
> 
> I believe what Jakub meant to say is that if a DSA conduit device driver
> advertises any of the NETIF_F_IP*_CSUM feature bits, then the driver's
> transmit path has the responsibility of checking that the payload being
> transmitted has a chance of being checksummed properly by the hardware. The
> problem here is not so much the geometry itself (linear or not, number/size
> of fragments, etc.) as much as the placement of the L2/L3 headers usually.
> 
> DSA conduit network device drivers do not have the ability today to
> determine what type of DSA tagging is being applied onto the DSA master but
> they do know whether DSA tagging is in use or not which may be enough to be
> overly compatible.
> 
> It is not clear to me whether we can solve this generically within the DSA
> framework or even if this is desirable, but once we have identified a
> problematic association of DSA tagger and DSA conduit, we can always have
> the DSA conduit driver do something like:
> 
> if (netdev_uses_dsa(dev))
> 	skb_checksum_help()
> 
> or have a fix_features callback which does reject the enabling of
> NETIF_F_IP*_CSUM if netdev_uses_dsa() becomes true.

Yes, but as you point out, the DSA master driver doesn't know what
header/trailer format it's dealing with. We could use netdev_uses_dsa()
as a very rough approximation, and that might work when we know that the
particular Ethernet controller is used only in conjunction with a single
type of DSA switch [from the same vendor], but I think we're just
delaying the inevitable, which is to treat the case where an Ethernet
controller can be a DSA master for more than one switch type, and it
understands some protocols but not others.
Also, scattering "if (netdev_uses_dsa(dev)) skb_checksum_help()" in
DSA-unaware drivers (the common case) seems like the improper approach.
We might end up seeing this pattern quite a lot, so DSA-unaware drivers
won't be DSA-unaware any longer.
It's still possible I'm misunderstanding something...