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

[Vladimir Oltean <olteanv@gmail.com>]

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