netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ioana Ciornei <ioana.ciornei@nxp.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH net-next 1/2] net: enetc: declare NETIF_F_IP_CSUM and do it in software
Date: Thu, 7 Oct 2021 12:12:46 +0000	[thread overview]
Message-ID: <20211007121246.pscw27lc4agpcgpj@skbuf> (raw)
In-Reply-To: <20211007064720.envusypxkazx6gz2@skbuf>

On Thu, Oct 07, 2021 at 09:47:20AM +0300, Ioana Ciornei wrote:
> On Wed, Oct 06, 2021 at 05:24:18PM -0700, Jakub Kicinski wrote:
> > On Wed,  6 Oct 2021 23:13:07 +0300 Ioana Ciornei wrote:
> > > This is just a preparation patch for software TSO in the enetc driver.
> > > Unfortunately, ENETC does not support Tx checksum offload which would
> > > normally render TSO, even software, impossible.
> > > 
> > > Declare NETIF_F_IP_CSUM as part of the feature set and do it at driver
> > > level using skb_csum_hwoffload_help() so that we can move forward and
> > > also add support for TSO in the next patch.
> > > 
> > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > 
> > Did you choose NETIF_F_IP_CSUM intentionally?
> > It'll only support IPv4, and since you always fall back to SW
> > I'd think NETIF_F_HW_CSUM makes more sense.
> 
> Somewhat intentionally, yes.
> 
> If I would use NETIF_F_HW_CSUM, as I understand it, the GSO path, added
> in the next patch, would have to compute the checksum not only for IPv6
> but also for any other protocols other than UDP and TCP (which currently
> it supports).
> I just didn't look into that at the moment.
> 

Now that I think of it, you have a point with declaring NETIF_F_HW_CSUM.

In the non-GSO case, skb_checksum_help() will be able to handle anything
that we throw at it.

On the GSO case, only skbs with TCP over IPv4 or IPv6 (depending on what
we declare in the features - TSO/TSO6) will be received - which the csum
code can handle.

Anyhow, I'll change this to use NETIF_F_HW_CSUM instead of
NETIF_F_IP_CSUM and I'll also look into IPv6 for the TSO part.

Thanks,
Ioana

  reply	other threads:[~2021-10-07 12:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-06 20:13 [PATCH net-next 0/2] net: enetc: add support for software TSO Ioana Ciornei
2021-10-06 20:13 ` [PATCH net-next 1/2] net: enetc: declare NETIF_F_IP_CSUM and do it in software Ioana Ciornei
2021-10-07  0:24   ` Jakub Kicinski
2021-10-07  6:47     ` Ioana Ciornei
2021-10-07 12:12       ` Ioana Ciornei [this message]
2021-10-06 20:13 ` [PATCH net-next 2/2] net: enetc: add support for software TSO Ioana Ciornei
2021-10-07  0:30   ` Jakub Kicinski
2021-10-07  6:48     ` Ioana Ciornei
2021-10-07  7:59   ` Claudiu Manoil
2021-10-07  8:33     ` Ioana Ciornei
2021-10-07  9:06       ` Claudiu Manoil
2021-10-07  9:26         ` Ioana Ciornei

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=20211007121246.pscw27lc4agpcgpj@skbuf \
    --to=ioana.ciornei@nxp.com \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vladimir.oltean@nxp.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).