Re: [dpdk-dev] about rx checksum flags

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, May 31, 2016 11:03 PM
> To: Olivier MATZ
> Cc: Yuanhan Liu; dev@dpdk.org; Ananyev, Konstantin; Richardson, Bruce; Adrien Mazarguil; Tan, Jianfeng
> Subject: Re: [dpdk-dev] about rx checksum flags
> 
> 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.

Hmm, I don't see the point here.
If the HW clearly reports that checksum is invalid (not unknown),
why SW has to assume it is ' undetermined' and recheck it?
To me that means just wasted cycles.
In general, it sounds like really strange approach to me:
write your SW with assumption that all HW you are going to use
will not work correctly. 

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

It might be a good feature to have, but if most HW vendors don't support it
why to bother?

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

I think that what Olivier proposed is good enough and
definitely a step forward from what we have right now.

Konstantin

> >
> > Thanks for your feedback.
> > Olivier