DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: Lance Richardson <lance.richardson@broadcom.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] question regarding rx checksum offload flags
Date: Fri, 18 Dec 2020 10:27:25 +0100	[thread overview]
Message-ID: <20201218092725.GA2631@platinum> (raw)
In-Reply-To: <CADyeNEAJQf9_g2CiiTNY+oXvmXuhT4FgEmNXc_6zPWscHjaUvg@mail.gmail.com>

Hi Lance,

On Mon, Dec 14, 2020 at 12:41:34PM -0500, Lance Richardson wrote:
> On Thu, Sep 17, 2020 at 10:05 AM Olivier Matz <olivier.matz@6wind.com> wrote:
> >
> > Hi Lance,
> >
> > On Mon, Aug 24, 2020 at 04:11:45PM -0400, Lance Richardson wrote:
> > > I was looking for some clarification regarding how rx checksum
> > > flags should be set for tunnel packets having both inner and outer
> > > IP/L4 headers.
> > >
> > > Based on comments in rte_mbuf_core.h, it seems to me. that the
> > > inner (encapsulated) IP header checksum status should determine
> > > which of these goes into ol_flags:
> > >     PKT_RX_IP_CKSUM_UNKNOWN
> > >     PKT_RX_IP_CKSUM_BAD
> > >     PKT_RX_IP_CKSUM_GOOD
> > >     PKT_RX_IP_CKSUM_NONE
> > >
> > > Similarly, the L4 checksum status should determine which of these
> > > goes into ol_flags:
> > >    PKT_RX_L4_CKSUM_UNKNOWN
> > >    PKT_RX_L4_CKSUM_BAD
> > >    PKT_RX_L4_CKSUM_GOOD
> > >    PKT_RX_L4_CKSUM_NONE
> > >
> > > The IP header checksum status for the outer IP header should determine
> > > whether this flag is set in ol_flags:
> > >     PKT_RX_EIP_CKSUM_BAD
> > >
> > > And for UDP-based tunnel encapsulations, the outer L4 checksum status
> > > should determine which of these goes into ol_flags:
> > >     PKT_RX_OUTER_L4_CKSUM_UNKNOWN
> > >     PKT_RX_OUTER_L4_CKSUM_BAD
> > >     PKT_RX_OUTER_L4_CKSUM_GOOD
> > >     PKT_RX_OUTER_L4_CKSUM_INVALID
> > >
> > > Finally, the checksum status of inner headers should have no influence
> > > on PKT_RX_EIP_CKSUM_BAD or PKT_RX_OUTER_L4_CKSUM_*, and
> > > likewise the checksum status of outer headers should have no influence
> > > on PKT_RX_L4_CKSUM_* or PKT_RX_IP_CKSUM_*.
> > >
> > > Is this correct? Apologies for such a basic question, but I'm having trouble
> > > correlating the above with implementations.
> > >
> > > Thanks and regards,
> > >     Lance
> >
> > The PKT_RX_EIP_CKSUM_BAD flag was added by these commits:
> >
> > https://git.dpdk.org/dpdk/commit/?id=c22265f6fd4cdcac9ee1b4970e4af8459d267516
> > https://git.dpdk.org/dpdk/commit/?id=d909af8f72ca3f8ab4fe1942abfb4f53e15ff8bc
> >
> > First, to be honnest, I don't think this API is the right one. From a software
> > stack point of view, it would have been more logical to have PKT_RX_INNER_*
> > flags instead of outer.
> >
> > That said, your understanding looks correct to me. I think this is the
> > expected behavior when the DEV_RX_OFFLOAD_OUTER* capability is enabled.
> >
> > If the capability is not set, only the PKT_RX_IP_CKSUM* and
> > PKT_RX_L4_CKSUM* flags may be set, and they reference the first layer.
> >
> > Regards,
> > Olivier
> 
> Hi Olivier,
> 
> I've been thinking about how to address this for the bnxt PMD which always
> reports PKT_RX_OUTER_L4_CKSUM_* and PKT_RX_EIP_CKSUM_BAD
> regardless of whether DEV_RX_OFFLOAD_OUTER_UDP_CKSUM or
> DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM are enabled.
> 
> One option would be to modify the PMD to respect the outer UDP and
> outer IPv4 checksum offload configuration. Since the mapping of hardware
> checksum status to mbuf checksum status is table-based, this would add
> very little overhead to the packet handling path, but it would add some
> (a larger table would be required, and the index would need to include
> the tunnel/non-tunnel status of the packet).
> 
> Another option would be to modify the PMD to force the outer UDP and
> outer IPv4 checksum offloads to always be enabled (there is at least
> one existing PMD that currently does this).
> 
> The second option should be the least disruptive to existing users, and
> would require the least effort to implement, but will it be an acceptable
> approach going forward? (If not, it seems the first option would be the
> right one to choose.)

I think the first option is the right one. It matches the behavior of
other drivers.

Regards,
Olivier

      parent reply	other threads:[~2020-12-18  9:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-24 20:11 Lance Richardson
2020-08-25 14:39 ` Lance Richardson
2020-08-26 13:39   ` Lance Richardson
2020-09-17 14:05 ` Olivier Matz
2020-12-14 17:41   ` Lance Richardson
2020-12-15 22:05     ` Lance Richardson
2020-12-18  9:27     ` Olivier Matz [this message]

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=20201218092725.GA2631@platinum \
    --to=olivier.matz@6wind.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=lance.richardson@broadcom.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).