[dpdk-dev] about rx checksum flags

[dpdk-dev] about rx checksum flags

[Olivier Matz]

Hi,

I'm planning to add the support for offloads in virtio-net pmd.
It appears that the current rx flags in mbuf are not sufficient to
describe the state of a packet received from a virtual driver.
I think we need a way to say "the checksum in the packet data is
not calculated, but the integrity of the data is verified".

Currently, we have one flag for L4 (same for IP):

  PKT_RX_L4_CKSUM_BAD: L4 cksum of RX pkt. is not OK.

This has also another problem that has already been discussed [1]:
if no flag is set, it is expected that the checksum is verified by
hw, but there is no way to say "the hw does not know if the cksum
is correct".

I would like to extend this flag to a 4-state value (2 bits):

 PKT_RX_L4_CKSUM_UNKNOWN: no information about the RX L4 checksum
  -> the application should verify the checksum by sw

 PKT_RX_L4_CKSUM_BAD: the L4 checksum in the packet is wrong
  -> the application can drop the packet without additional check

 PKT_RX_L4_CKSUM_GOOD: the L4 checksum in the packet is valid
  -> the application can accept the packet without verifying the
     checksum by sw

 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)

To keep the compatibility with application, the old flag is kept at the
same value, and a new flag is added. It is assumed that the behavior
of applications was:

  PKT_RX_L4_CKSUM_BAD = 0 -> packet is accepted
  PKT_RX_L4_CKSUM_BAD = 1 -> packet is dropped

The new checksum states for L4 (same for IP) would be:

  old flag   new flag   meaning
  0          0          PKT_RX_L4_CKSUM_UNKNOWN
  1          0          PKT_RX_L4_CKSUM_BAD
  0          1          PKT_RX_L4_CKSUM_GOOD
  1          1          PKT_RX_L4_CKSUM_NONE

With this, an old application that only checks the old flag, and
running using a dpdk having this modification would accept GOOD and
UNKNOWN packets (like today), drop BAD packets (like today) and
drop NONE packets (this is a new feature that has to be explicitly
enabled by the application).


Any comment?

Olivier


[1] http://dpdk.org/ml/archives/dev/2015-January/011550.html

Re: [dpdk-dev] about rx checksum flags

[Adrien Mazarguil]

On Mon, May 30, 2016 at 05:26:21PM +0200, Olivier Matz wrote:
> Hi,
> 
> I'm planning to add the support for offloads in virtio-net pmd.
> It appears that the current rx flags in mbuf are not sufficient to
> describe the state of a packet received from a virtual driver.
> I think we need a way to say "the checksum in the packet data is
> not calculated, but the integrity of the data is verified".
> 
> Currently, we have one flag for L4 (same for IP):
> 
>   PKT_RX_L4_CKSUM_BAD: L4 cksum of RX pkt. is not OK.
> 
> This has also another problem that has already been discussed [1]:
> if no flag is set, it is expected that the checksum is verified by
> hw, but there is no way to say "the hw does not know if the cksum
> is correct".
> 
> I would like to extend this flag to a 4-state value (2 bits):
> 
>  PKT_RX_L4_CKSUM_UNKNOWN: no information about the RX L4 checksum
>   -> the application should verify the checksum by sw
> 
>  PKT_RX_L4_CKSUM_BAD: the L4 checksum in the packet is wrong
>   -> the application can drop the packet without additional check
> 
>  PKT_RX_L4_CKSUM_GOOD: the L4 checksum in the packet is valid
>   -> the application can accept the packet without verifying the
>      checksum by sw
> 
>  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)

This API makes and lot of sense for both mlx4 and mlx5 PMDs as well, for
which hardware only returns information about "good" headers ("unknown"
should be assumed otherwise). It does not map well with the current API that
only provides info about bad checksums.

For instance, non-IP packets do not have valid L3/L4 checksums and thus are
not returned as "good" by hardware. These PMDs perform the "not good means
bad" conversion for DPDK which is wrong in such cases. Same for unknown
packet types.

> To keep the compatibility with application, the old flag is kept at the
> same value, and a new flag is added. It is assumed that the behavior
> of applications was:
> 
>   PKT_RX_L4_CKSUM_BAD = 0 -> packet is accepted
>   PKT_RX_L4_CKSUM_BAD = 1 -> packet is dropped
> 
> The new checksum states for L4 (same for IP) would be:
> 
>   old flag   new flag   meaning
>   0          0          PKT_RX_L4_CKSUM_UNKNOWN
>   1          0          PKT_RX_L4_CKSUM_BAD
>   0          1          PKT_RX_L4_CKSUM_GOOD
>   1          1          PKT_RX_L4_CKSUM_NONE
> 
> With this, an old application that only checks the old flag, and
> running using a dpdk having this modification would accept GOOD and
> UNKNOWN packets (like today), drop BAD packets (like today) and
> drop NONE packets (this is a new feature that has to be explicitly
> enabled by the application).
> 
> 
> Any comment?

Considering mlx4 and mlx5 can only return "good" or "unknown" checksums, I'm
all for updating the API as suggested.

> Olivier
> 
> 
> [1] http://dpdk.org/ml/archives/dev/2015-January/011550.html

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] about rx checksum flags

[Tan, Jianfeng]

Hi Oliver,


On 5/30/2016 11:26 PM, Olivier Matz wrote:
> Hi,
>
> I'm planning to add the support for offloads in virtio-net pmd.
> It appears that the current rx flags in mbuf are not sufficient to
> describe the state of a packet received from a virtual driver.
> I think we need a way to say "the checksum in the packet data is
> not calculated, but the integrity of the data is verified".

I also met this problem :-). Glad to see you raise it up in the mail list.

>
> Currently, we have one flag for L4 (same for IP):
>
>    PKT_RX_L4_CKSUM_BAD: L4 cksum of RX pkt. is not OK.
>
> This has also another problem that has already been discussed [1]:
> if no flag is set, it is expected that the checksum is verified by
> hw, but there is no way to say "the hw does not know if the cksum
> is correct".
>
> I would like to extend this flag to a 4-state value (2 bits):
>
>   PKT_RX_L4_CKSUM_UNKNOWN: no information about the RX L4 checksum
>    -> the application should verify the checksum by sw
>
>   PKT_RX_L4_CKSUM_BAD: the L4 checksum in the packet is wrong
>    -> the application can drop the packet without additional check
>
>   PKT_RX_L4_CKSUM_GOOD: the L4 checksum in the packet is valid
>    -> the application can accept the packet without verifying the
>       checksum by sw
>
>   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)
>
> To keep the compatibility with application, the old flag is kept at the
> same value, and a new flag is added. It is assumed that the behavior
> of applications was:
>
>    PKT_RX_L4_CKSUM_BAD = 0 -> packet is accepted
>    PKT_RX_L4_CKSUM_BAD = 1 -> packet is dropped
>
> The new checksum states for L4 (same for IP) would be:
>
>    old flag   new flag   meaning
>    0          0          PKT_RX_L4_CKSUM_UNKNOWN
>    1          0          PKT_RX_L4_CKSUM_BAD
>    0          1          PKT_RX_L4_CKSUM_GOOD
>    1          1          PKT_RX_L4_CKSUM_NONE
>
> With this, an old application that only checks the old flag, and
> running using a dpdk having this modification would accept GOOD and
> UNKNOWN packets (like today), drop BAD packets (like today) and
> drop NONE packets (this is a new feature that has to be explicitly
> enabled by the application).
>
>
> Any comment?

Why not take care of PKT_RX_IP_CKSUM_BAD? Is it too easy for sw to handle?

For virtio, there's only one bit, VIRTIO_NET_HDR_F_DATA_VALID, to 
indicate that checksum is valid. Shall we differentiate L3 checksum and 
L4 checksum in rte_mbuf.ol_flags?

Thanks,
Jianfeng

>
> Olivier
>
>
> [1] http://dpdk.org/ml/archives/dev/2015-January/011550.html

Re: [dpdk-dev] about rx checksum flags

[Adrien Mazarguil]

On Tue, May 31, 2016 at 10:43:29AM +0800, Tan, Jianfeng wrote:
> Hi Oliver,
> 
> 
> On 5/30/2016 11:26 PM, Olivier Matz wrote:
> >Hi,
> >
> >I'm planning to add the support for offloads in virtio-net pmd.
> >It appears that the current rx flags in mbuf are not sufficient to
> >describe the state of a packet received from a virtual driver.
> >I think we need a way to say "the checksum in the packet data is
> >not calculated, but the integrity of the data is verified".
> 
> I also met this problem :-). Glad to see you raise it up in the mail list.
> 
> >
> >Currently, we have one flag for L4 (same for IP):
> >
> >   PKT_RX_L4_CKSUM_BAD: L4 cksum of RX pkt. is not OK.
> >
> >This has also another problem that has already been discussed [1]:
> >if no flag is set, it is expected that the checksum is verified by
> >hw, but there is no way to say "the hw does not know if the cksum
> >is correct".
> >
> >I would like to extend this flag to a 4-state value (2 bits):
> >
> >  PKT_RX_L4_CKSUM_UNKNOWN: no information about the RX L4 checksum
> >   -> the application should verify the checksum by sw
> >
> >  PKT_RX_L4_CKSUM_BAD: the L4 checksum in the packet is wrong
> >   -> the application can drop the packet without additional check
> >
> >  PKT_RX_L4_CKSUM_GOOD: the L4 checksum in the packet is valid
> >   -> the application can accept the packet without verifying the
> >      checksum by sw
> >
> >  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)
> >
> >To keep the compatibility with application, the old flag is kept at the
> >same value, and a new flag is added. It is assumed that the behavior
> >of applications was:
> >
> >   PKT_RX_L4_CKSUM_BAD = 0 -> packet is accepted
> >   PKT_RX_L4_CKSUM_BAD = 1 -> packet is dropped
> >
> >The new checksum states for L4 (same for IP) would be:
> >
> >   old flag   new flag   meaning
> >   0          0          PKT_RX_L4_CKSUM_UNKNOWN
> >   1          0          PKT_RX_L4_CKSUM_BAD
> >   0          1          PKT_RX_L4_CKSUM_GOOD
> >   1          1          PKT_RX_L4_CKSUM_NONE
> >
> >With this, an old application that only checks the old flag, and
> >running using a dpdk having this modification would accept GOOD and
> >UNKNOWN packets (like today), drop BAD packets (like today) and
> >drop NONE packets (this is a new feature that has to be explicitly
> >enabled by the application).
> >
> >
> >Any comment?
> 
> Why not take care of PKT_RX_IP_CKSUM_BAD? Is it too easy for sw to handle?

I thought PKT_RX_IP_CKSUM_BAD was to be modified in a similar fashion, but
since you raise the issue, mlx4/mlx5 need this as well. These boards only
report "good" checksums for L3 and L4.

> For virtio, there's only one bit, VIRTIO_NET_HDR_F_DATA_VALID, to indicate
> that checksum is valid. Shall we differentiate L3 checksum and L4 checksum
> in rte_mbuf.ol_flags?
> 
> Thanks,
> Jianfeng
> 
> >
> >Olivier
> >
> >
> >[1] http://dpdk.org/ml/archives/dev/2015-January/011550.html
> 

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] about rx checksum flags

[Olivier MATZ]

Hi,

On 05/31/2016 12:08 PM, Adrien Mazarguil wrote:
> On Tue, May 31, 2016 at 10:43:29AM +0800, Tan, Jianfeng wrote:
>> Why not take care of PKT_RX_IP_CKSUM_BAD? Is it too easy for sw to handle?
> 
> I thought PKT_RX_IP_CKSUM_BAD was to be modified in a similar fashion, but
> since you raise the issue, mlx4/mlx5 need this as well. These boards only
> report "good" checksums for L3 and L4.

Yep, maybe it was not so clear in my initial mail, but I think the L4
example should apply to IP as well.

>> For virtio, there's only one bit, VIRTIO_NET_HDR_F_DATA_VALID, to indicate
>> that checksum is valid. Shall we differentiate L3 checksum and L4 checksum
>> in rte_mbuf.ol_flags?

>From what I understand from the specification, when the driver
(virtio-net) receives a packet with the VIRTIO_NET_HDR_F_NEEDS_CSUM
flag, it can be assumed that all checksums before hdr->csum_start are
valid. Here we would set PKT_RX_IP_CKSUM_GOOD and PKT_RX_L4_CKSUM_NONE.

In case of VIRTIO_NET_HDR_F_DATA_VALID, I understand that both L3 and
L4 of the outer header is valid. Here we would set PKT_RX_IP_CKSUM_GOOD
and PKT_RX_L4_CKSUM_GOOD.

Regards,
Olivier

Re: [dpdk-dev] about rx checksum flags

[Yuanhan Liu]

On Mon, May 30, 2016 at 05:26:21PM +0200, Olivier Matz wrote:
> Hi,
> 
> I'm planning to add the support for offloads in virtio-net pmd.

Good to know, and thanks!

> It appears that the current rx flags in mbuf are not sufficient to
> describe the state of a packet received from a virtual driver.
> I think we need a way to say "the checksum in the packet data is
> not calculated, but the integrity of the data is verified".
> 
> Currently, we have one flag for L4 (same for IP):
> 
>   PKT_RX_L4_CKSUM_BAD: L4 cksum of RX pkt. is not OK.
> 
> This has also another problem that has already been discussed [1]:
> if no flag is set, it is expected that the checksum is verified by
> hw, but there is no way to say "the hw does not know if the cksum
> is correct".
> 
> I would like to extend this flag to a 4-state value (2 bits):
> 
>  PKT_RX_L4_CKSUM_UNKNOWN: no information about the RX L4 checksum
>   -> the application should verify the checksum by sw
> 
>  PKT_RX_L4_CKSUM_BAD: the L4 checksum in the packet is wrong
>   -> the application can drop the packet without additional check
> 
>  PKT_RX_L4_CKSUM_GOOD: the L4 checksum in the packet is valid
>   -> the application can accept the packet without verifying the
>      checksum by sw

This is good to have, which could save some burderns of cksum
validation when using kernel virtio-net (that has a TCP/IP stack
on top of it) and vhost-user combo.

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

	--yliu

> To keep the compatibility with application, the old flag is kept at the
> same value, and a new flag is added. It is assumed that the behavior
> of applications was:
> 
>   PKT_RX_L4_CKSUM_BAD = 0 -> packet is accepted
>   PKT_RX_L4_CKSUM_BAD = 1 -> packet is dropped
> 
> The new checksum states for L4 (same for IP) would be:
> 
>   old flag   new flag   meaning
>   0          0          PKT_RX_L4_CKSUM_UNKNOWN
>   1          0          PKT_RX_L4_CKSUM_BAD
>   0          1          PKT_RX_L4_CKSUM_GOOD
>   1          1          PKT_RX_L4_CKSUM_NONE
> 
> With this, an old application that only checks the old flag, and
> running using a dpdk having this modification would accept GOOD and
> UNKNOWN packets (like today), drop BAD packets (like today) and
> drop NONE packets (this is a new feature that has to be explicitly
> enabled by the application).
> 
> 
> Any comment?
> 
> Olivier
> 
> 
> [1] http://dpdk.org/ml/archives/dev/2015-January/011550.html

Re: [dpdk-dev] about rx checksum flags

[Olivier MATZ]

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

Re: [dpdk-dev] about rx checksum flags

[Stephen Hemminger]

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

Re: [dpdk-dev] about rx checksum flags

[Olivier MATZ]

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

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

Do you also suggest to drop IP checksum flags?

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

Re: [dpdk-dev] about rx checksum flags

[Stephen Hemminger]

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

Re: [dpdk-dev] about rx checksum flags

[Ananyev, Konstantin]

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

Re: [dpdk-dev] about rx checksum flags

[Chandran, Sugesh]

Hi Olivier,

Thank you for working on this..
A comment on the proposal is given below,
 

Regards
_Sugesh

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev,
> Konstantin
> Sent: Wednesday, June 1, 2016 10:07 AM
> To: Stephen Hemminger <stephen@networkplumber.org>; Olivier MATZ
> <olivier.matz@6wind.com>
> Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>; dev@dpdk.org; 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
> 
> 
> 
> > -----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.
> >
[Sugesh] The checksum offload can boost the tunneling performance in OVS.
I guess the IP checksum also important as L4. In some cases, UDP checksum is
zero and no need to validate it. But Ip checksum is present on all the packets and that must be
validated all  the time. At higher packet rate, the ip checksum offload can offer slight 
performance improvement. What do you think??

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

Re: [dpdk-dev] about rx checksum flags

[Olivier Matz]

Hi,

On 06/02/2016 09:42 AM, Chandran, Sugesh wrote:
>>>> 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.
>>> > >
> [Sugesh] The checksum offload can boost the tunneling performance in OVS.
> I guess the IP checksum also important as L4. In some cases, UDP checksum is
> zero and no need to validate it. But Ip checksum is present on all the packets and that must be
> validated all  the time. At higher packet rate, the ip checksum offload can offer slight 
> performance improvement. What do you think??
> 

Agree, in some situations (and this is even more true with packet
types / smartnics), the application could process without accessing
the packet data if we keep the IP cksum flags.

Regards,
Olivier

Re: [dpdk-dev] about rx checksum flags

[Chandran, Sugesh]

Regards
_Sugesh


> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Friday, June 3, 2016 1:43 PM
> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>; dev@dpdk.org; 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
> 
> Hi,
> 
> On 06/02/2016 09:42 AM, Chandran, Sugesh wrote:
> >>>> 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.
> >>> > >
> > [Sugesh] The checksum offload can boost the tunneling performance in
> OVS.
> > I guess the IP checksum also important as L4. In some cases, UDP
> > checksum is zero and no need to validate it. But Ip checksum is
> > present on all the packets and that must be validated all  the time.
> > At higher packet rate, the ip checksum offload can offer slight performance
> improvement. What do you think??
> >
> 
> Agree, in some situations (and this is even more true with packet types /
> smartnics), the application could process without accessing the packet data if
> we keep the IP cksum flags.
[Sugesh] True, If that's the case, Will you considering to implement IP
checksum flags as well along with L4?
As you said , this will be useful when we offload packet lookup itself into the NICs(May be
when using Flow director) ? 



> 
> Regards,
> Olivier

Re: [dpdk-dev] about rx checksum flags

[Olivier Matz]

Hi,

On 06/08/2016 10:22 AM, Chandran, Sugesh wrote:
>>> I guess the IP checksum also important as L4. In some cases, UDP
>>> checksum is zero and no need to validate it. But Ip checksum is
>>> present on all the packets and that must be validated all  the time.
>>> At higher packet rate, the ip checksum offload can offer slight performance
>> improvement. What do you think??
>>>
>>
>> Agree, in some situations (and this is even more true with packet types /
>> smartnics), the application could process without accessing the packet data if
>> we keep the IP cksum flags.
> [Sugesh] True, If that's the case, Will you considering to implement IP
> checksum flags as well along with L4?
> As you said , this will be useful when we offload packet lookup itself into the NICs(May be
> when using Flow director) ? 

Yes, I plan to implement the same rx status flags (good, bad, unknown,
none) for rx IP checksum too.

Regards,
Olivier

Re: [dpdk-dev] about rx checksum flags

[Chandran, Sugesh]

Regards
_Sugesh

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, June 8, 2016 2:02 PM
> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>; dev@dpdk.org; 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
> 
> Hi,
> 
> On 06/08/2016 10:22 AM, Chandran, Sugesh wrote:
> >>> I guess the IP checksum also important as L4. In some cases, UDP
> >>> checksum is zero and no need to validate it. But Ip checksum is
> >>> present on all the packets and that must be validated all  the time.
> >>> At higher packet rate, the ip checksum offload can offer slight
> >>> performance
> >> improvement. What do you think??
> >>>
> >>
> >> Agree, in some situations (and this is even more true with packet
> >> types / smartnics), the application could process without accessing
> >> the packet data if we keep the IP cksum flags.
> > [Sugesh] True, If that's the case, Will you considering to implement
> > IP checksum flags as well along with L4?
> > As you said , this will be useful when we offload packet lookup itself
> > into the NICs(May be when using Flow director) ?
> 
> Yes, I plan to implement the same rx status flags (good, bad, unknown,
> none) for rx IP checksum too.
[Sugesh] That's great!, Thank you Olivier.
> 
> Regards,
> Olivier

Re: [dpdk-dev] about rx checksum flags

[Chandran, Sugesh]

Hi Olivier,

Just to confirm , is this rx checksum patch already submitted in the DPDK ML?
We would like to use these flags to speed up the tunneling in OVS.



Regards
_Sugesh


> -----Original Message-----
> From: Chandran, Sugesh
> Sent: Friday, June 10, 2016 5:16 PM
> To: 'Olivier Matz' <olivier.matz@6wind.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>; dev@dpdk.org; 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
> 
> 
> 
> Regards
> _Sugesh
> 
> > -----Original Message-----
> > From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > Sent: Wednesday, June 8, 2016 2:02 PM
> > To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Stephen Hemminger
> > <stephen@networkplumber.org>
> > Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>; dev@dpdk.org;
> > 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
> >
> > Hi,
> >
> > On 06/08/2016 10:22 AM, Chandran, Sugesh wrote:
> > >>> I guess the IP checksum also important as L4. In some cases, UDP
> > >>> checksum is zero and no need to validate it. But Ip checksum is
> > >>> present on all the packets and that must be validated all  the time.
> > >>> At higher packet rate, the ip checksum offload can offer slight
> > >>> performance
> > >> improvement. What do you think??
> > >>>
> > >>
> > >> Agree, in some situations (and this is even more true with packet
> > >> types / smartnics), the application could process without accessing
> > >> the packet data if we keep the IP cksum flags.
> > > [Sugesh] True, If that's the case, Will you considering to implement
> > > IP checksum flags as well along with L4?
> > > As you said , this will be useful when we offload packet lookup
> > > itself into the NICs(May be when using Flow director) ?
> >
> > Yes, I plan to implement the same rx status flags (good, bad, unknown,
> > none) for rx IP checksum too.
> [Sugesh] That's great!, Thank you Olivier.
> >
> > Regards,
> > Olivier

Re: [dpdk-dev] about rx checksum flags

[Olivier MATZ]

Hi Sugesh

On 07/06/2016 02:52 PM, Chandran, Sugesh wrote:
> Hi Olivier,
> 
> Just to confirm , is this rx checksum patch already submitted in the DPDK ML?
> We would like to use these flags to speed up the tunneling in OVS.

No it is not submitted yet.
I plan to send it in the coming days, it will apply on top of the sw
packet_type patchset I've sent yesterday.

Regards,
Olivier