DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Olivier MATZ <olivier.matz@6wind.com>
Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	"Tan, Jianfeng" <jianfeng.tan@intel.com>
Subject: Re: [dpdk-dev] about rx checksum flags
Date: Tue, 31 May 2016 15:02:47 -0700	[thread overview]
Message-ID: <20160531150247.15819f1d@xeon-e3> (raw)
In-Reply-To: <574DFB11.5020701@6wind.com>

On Tue, 31 May 2016 22:58:57 +0200
Olivier MATZ <olivier.matz@6wind.com> wrote:

> Hi Stephen,
> 
> On 05/31/2016 10:28 PM, Stephen Hemminger wrote:
> > On Tue, 31 May 2016 21:11:59 +0200
> > Olivier MATZ <olivier.matz@6wind.com> wrote:
> > 
> >>
> >>
> >> On 05/31/2016 10:09 AM, Yuanhan Liu wrote:
> >>> On Mon, May 30, 2016 at 05:26:21PM +0200, Olivier Matz wrote:
> >>>>  PKT_RX_L4_CKSUM_NONE: the L4 checksum is not correct in the packet
> >>>>  data, but the integrity of the L4 header is verified.
> >>>>   -> the application can process the packet but must not verify the
> >>>>      checksum by sw. It has to take care to recalculate the cksum
> >>>>      if the packet is transmitted (either by sw or using tx offload)
> >>>
> >>> I like the explanation you made at [1] better :)
> >>>
> >>> So in general, I think this proposal is good to have.
> >>
> >> Thanks everyone for your feedback.
> >>
> >> I'll try to send a first patch proposition soon.
> >>
> >> Regards,
> >> Olivier
> > 
> > I think it is time to ditch the old definitions of Rx checksum and instead
> > use something more compatiable with virtio (and Linux). I.e have three values
> >   1) checksum is know good for packet contents
> >   2) checksum value one's complement for packet contents
> >   3) checksum is undetermined
> > The original definition seems to be Intel HW centric and applies to a limited
> > range of devices making it unusable by general application.
> > 
> > Break the ABI, and ditch the old values (ok mark PKT_RX_L4_CKSUM_BAD as __deprecated
> > and remove all usage).
> > 
> 
> Don't you think knowing that a checksum is bad could be useful?

Not really. They should be mark as undetermined, then software can recheck
for the possibly buggy hardware.

> In that case the application can drop/log the packet without any
> additional cpu cost.
> 
> What do you mean by beeing unusable by general application?

Right now application can only see "known bad" or "indeterminate"
there is no way to no which packets are good. Since good is the desired/expected
case, software has to checksum every packet.

> 
> I think the "2)" also requires a csum_start + csum_offset in
> mbuf structure, right?

Not really, it would mean having a way to get the raw one's complement sum
out of the hardware. This is a good way to support the tunnel protocol du jour
without having to have firmware support. Unfortunately, most hardware vendors
don't believe in doing it that way.


> Do you also suggest to drop IP checksum flags?

IP checksum offload is mostly useless. If application needs to look
at IP, it can do whole checksum in very few instructions, the whole header
is in the same cache line as src/dst so the HW offload is really no help.

> 
> Will it be possible to manage tunnel checksums?
> 
> I think this would be a pretty big change. If there is no additional
> argument than beeing more compatible with virtio/linux, I'm wondering
> if it's worth breaking the API. Let's wait for other opinions.
> 
> Thanks for your feedback.
> Olivier

  reply	other threads:[~2016-05-31 22:02 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-30 15:26 Olivier Matz
2016-05-30 16:07 ` Adrien Mazarguil
2016-05-31  2:43 ` Tan, Jianfeng
2016-05-31 10:08   ` Adrien Mazarguil
2016-05-31 19:11     ` Olivier MATZ
2016-05-31  8:09 ` Yuanhan Liu
2016-05-31 19:11   ` Olivier MATZ
2016-05-31 20:28     ` Stephen Hemminger
2016-05-31 20:58       ` Olivier MATZ
2016-05-31 22:02         ` Stephen Hemminger [this message]
2016-06-01  9:06           ` Ananyev, Konstantin
2016-06-02  7:42             ` Chandran, Sugesh
2016-06-03 12:43               ` Olivier Matz
2016-06-08  8:22                 ` Chandran, Sugesh
2016-06-08 13:02                   ` Olivier Matz
2016-06-10 16:15                     ` Chandran, Sugesh
2016-07-06 12:52                       ` Chandran, Sugesh
2016-07-06 13:18                         ` Olivier MATZ

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=20160531150247.15819f1d@xeon-e3 \
    --to=stephen@networkplumber.org \
    --cc=adrien.mazarguil@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=jianfeng.tan@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=yuanhan.liu@linux.intel.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).