LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Lino Sanfilippo <LinoSanfilippo@gmx.de>,
	woojung.huh@microchip.com, UNGLinuxDriver@microchip.com,
	vivien.didelot@gmail.com, f.fainelli@gmail.com,
	davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] net: dsa: tag_ksz: dont let the hardware process the layer 4 checksum
Date: Thu, 15 Jul 2021 17:36:48 +0300	[thread overview]
Message-ID: <20210715143648.yutq6vceoblnhhfp@skbuf> (raw)
In-Reply-To: <YPAzZXaC/En3s4ly@lunn.ch>

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.

  reply	other threads:[~2021-07-15 14:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14 19:17 [PATCH 0/2] Fixes for KSZ DSA switch Lino Sanfilippo
2021-07-14 19:17 ` [PATCH 1/2] net: dsa: tag_ksz: linearize SKB before adding DSA tag Lino Sanfilippo
2021-07-14 19:17 ` [PATCH 2/2] net: dsa: tag_ksz: dont let the hardware process the layer 4 checksum Lino Sanfilippo
2021-07-14 19:48   ` Vladimir Oltean
2021-07-14 20:15     ` Andrew Lunn
2021-07-15  6:54       ` Vladimir Oltean
2021-07-15 11:16         ` Aw: " Lino Sanfilippo
2021-07-15 11:49           ` Vladimir Oltean
2021-07-15 13:04             ` Aw: " Lino Sanfilippo
2021-07-15 13:12               ` Vladimir Oltean
2021-07-15 13:34                 ` Aw: " Lino Sanfilippo
2021-07-15 13:08         ` Andrew Lunn
2021-07-15 14:36           ` Vladimir Oltean [this message]
2021-07-15 15:50             ` Andrew Lunn
2021-07-19  8:20             ` Aw: " Lino Sanfilippo
2021-07-19  8:24               ` Vladimir Oltean
2021-07-15 16:24           ` Florian Fainelli
2021-07-15 23:23 ` [PATCH 0/2] Fixes for KSZ DSA switch Florian Fainelli
2021-07-16  8:49   ` Aw: " Lino Sanfilippo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210715143648.yutq6vceoblnhhfp@skbuf \
    --to=olteanv@gmail.com \
    --cc=LinoSanfilippo@gmx.de \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@gmail.com \
    --cc=woojung.huh@microchip.com \
    --subject='Re: [PATCH 2/2] net: dsa: tag_ksz: dont let the hardware process the layer 4 checksum' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).