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]

Message-ID: <20220124093556.50fe39a3@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Mon, 24 Jan 2022 09:35:56 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        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, 24 Jan 2022 19:21:58 +0200 Vladimir Oltean wrote:
> On Mon, Jan 24, 2022 at 09:01:20AM -0800, Florian Fainelli wrote:
> > On 1/24/2022 8:55 AM, Vladimir Oltean wrote:  
> > > 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.

Sorry I used "geometry" loosely.

What I meant is simply that if the driver uses NETIF_F_IP*_CSUM 
it should parse the packet before it hands it off to the HW.

There is infinity of protocols users can come up with, while the device
parser is very much finite, so it's only practical to check compliance
with the HW parser in the driver. The reverse approach of adding
per-protocol caps is a dead end IMO. And we should not bloat the stack
when NETIF_F_HW_CSUM exists and the memo that parsing packets on Tx is
bad b/c of protocol ossification went out a decade ago.

> > 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 not about DSA. The driver should not check

if (dsa())
	blah;

it should check 

if (!(eth [-> vlan] -> ip -> tcp/udp))
	csum_help();

> It's still possible I'm misunderstanding something...