Message-ID: <20220124084649.0918ba5c@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Mon, 24 Jan 2022 08:46:49 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     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>,
        "f.fainelli@...il.com" <f.fainelli@...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 17:31:47 +0200 Vladimir Oltean wrote:
> > I checked DSA doc again and it says:
> > 
> > "Since tagging protocols in category 1 and 2 break software (and most
> > often also hardware) packet dissection on the DSA master, features
> > such as RPS (Receive Packet Steering) on the DSA master would be
> > broken. The DSA framework deals with this by hooking into the flow
> > dissector and shifting the offset at which the IP header is to be
> > found in the tagged frame as seen by the DSA master. This behavior is
> > automatic based on the overhead value of the tagging protocol. If not
> > all packets are of equal size, the tagger can implement the
> > flow_dissect method of the struct dsa_device_ops and override this
> > default behavior by specifying the correct offset incurred by each
> > individual RX packet. Tail taggers do not cause issues to the flow
> > dissector."
> > 
> > It makes me think that it is the master network driver that does not
> > implement that IP header location shift. Anyway, I believe it also
> > depends on HW capabilities to inform that shift, right?
> > 
> > I'm trying to think as a DSA newbie (which is exactly what I am).
> > Differently from an isolated ethernet driver, with DSA, the system
> > does have control of "something" after the offload should be applied:
> > the dsa switch. Can't we have a generic way to send a packet to the
> > switch and make it bounce back to the CPU (passing through the offload
> > engine)? Would it work if I set the destination port as the CPU port?
> > This way, we could simply detect if the offload worked and disable
> > those features that did not work. It could work with a generic
> > implementation or, if needed, a specialized optional ds_switch_ops
> > function just to setup that temporary lookback forwarding rule.  
> 
> To be clear, do you consider this simpler than an ndo operation that
> returns true or false if a certain DSA master can offload a certain
> netdev feature in the presence of a certain DSA tag?
> 
> Ignoring the fact that there are subtly different ways in which various
> hardware manufacturers implement packet injection from the CPU (and this
> is reflected in the various struct dsa_device_ops :: xmit operation),
> plus the fact that dsa_device_ops :: xmit takes a slave net_device as
> argument, for which there is none to represent the CPU port. These
> points mean that you'd need to implement a separate, hardware-specific
> loopback xmit for each tagging protocol. But again, ignoring that for a
> second.
> 
> When would be a good time to probe for DSA master features? The DSA
> master might be down when DSA switches probe. What should we do with
> packets sent on a DSA port until we've finished probing for DSA master
> capabilities?

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).

Powered by blists - more mailing lists