DPDK patches and discussions
 help / color / mirror / Atom feed
* Is it correct to report checksum good when there is no checksum?
@ 2022-11-10  9:25 Andrew Rybchenko
  2022-11-10  9:55 ` Morten Brørup
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Rybchenko @ 2022-11-10  9:25 UTC (permalink / raw)
  To: dev
  Cc: Olivier Matz, Ananyev, Konstantin, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand, Morten Brørup

Hi all,

some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
For me it looks strange, but I see some technical reasons behind.
Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
Should UNKNOWN or NONE be used instead?

Thanks,
Andrew.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: Is it correct to report checksum good when there is no checksum?
  2022-11-10  9:25 Is it correct to report checksum good when there is no checksum? Andrew Rybchenko
@ 2022-11-10  9:55 ` Morten Brørup
  2022-11-10 10:08   ` Andrew Rybchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Morten Brørup @ 2022-11-10  9:55 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
  Cc: Olivier Matz, Ananyev, Konstantin, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand

> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Thursday, 10 November 2022 10.26
> 
> Hi all,
> 
> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> For me it looks strange, but I see some technical reasons behind.

Please note: IPv6 packets by definition have no IP checksum.

> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> Should UNKNOWN or NONE be used instead?

Certainly not NONE. Its description says: "the IP checksum is *not* correct in the packet [...]". But there is no incorrect IP checksum in the packet.

I will argue against UNKNOWN. Its description says: "no information about the RX IP checksum". But we do have information about it! We know that the IP checksum is not there (the value is "NULL"), and that it is not supposed to be there (the value is supposed to be "NULL").

So I consider GOOD the correct response here.

GOOD also means that the application can proceed processing the packet normally without further IP header checksum checking, so it's good for performance.

It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD that IPv6 packets always return this value, because IPv6 packets have no IP header checksum, and that is what is expected of them.

-Morten


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Is it correct to report checksum good when there is no checksum?
  2022-11-10  9:55 ` Morten Brørup
@ 2022-11-10 10:08   ` Andrew Rybchenko
  2022-11-10 10:29     ` Thomas Monjalon
  2022-11-10 10:29     ` Morten Brørup
  0 siblings, 2 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2022-11-10 10:08 UTC (permalink / raw)
  To: Morten Brørup, dev
  Cc: Olivier Matz, Ananyev, Konstantin, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand

On 11/10/22 12:55, Morten Brørup wrote:
>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>> Sent: Thursday, 10 November 2022 10.26
>>
>> Hi all,
>>
>> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
>> For me it looks strange, but I see some technical reasons behind.
> 
> Please note: IPv6 packets by definition have no IP checksum.
> 
>> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
>> Should UNKNOWN or NONE be used instead?
> 
> Certainly not NONE. Its description says: "the IP checksum is *not* correct in the packet [...]". But there is no incorrect IP checksum in the packet.
> 

Thanks, I should read the definition of none more careful.

> I will argue against UNKNOWN. Its description says: "no information about the RX IP checksum". But we do have information about it! We know that the IP checksum is not there (the value is "NULL"), and that it is not supposed to be there (the value is supposed to be "NULL").
> 

I thought that "no checksum" => "no information" => UNKNOWN

> So I consider GOOD the correct response here.
> 
> GOOD also means that the application can proceed processing the packet normally without further IP header checksum checking, so it's good for performance.
> 

It is very important point and would be nice to have in GOOD
case definition (both IP and L4 cases). It is the right
motivation why GOOD makes sense for IPv6.

> It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD that IPv6 packets always return this value, because IPv6 packets have no IP header checksum, and that is what is expected of them.
> 

Could you make a patch?

Bonus question is UDP checksum 0 case. GOOD as well?
(just want to clarify the documentation while we're on it).

Thanks a lot,
Andrew.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Is it correct to report checksum good when there is no checksum?
  2022-11-10 10:08   ` Andrew Rybchenko
@ 2022-11-10 10:29     ` Thomas Monjalon
  2022-11-10 10:29     ` Morten Brørup
  1 sibling, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2022-11-10 10:29 UTC (permalink / raw)
  To: Morten Brørup, Andrew Rybchenko
  Cc: dev, Olivier Matz, Ananyev, Konstantin, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, David Marchand

10/11/2022 11:08, Andrew Rybchenko:
> On 11/10/22 12:55, Morten Brørup wrote:
> >> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> >> Sent: Thursday, 10 November 2022 10.26
> >>
> >> Hi all,
> >>
> >> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> >> For me it looks strange, but I see some technical reasons behind.
> > 
> > Please note: IPv6 packets by definition have no IP checksum.
> > 
> >> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> >> Should UNKNOWN or NONE be used instead?
> > 
> > Certainly not NONE. Its description says: "the IP checksum is *not* correct in the packet [...]". But there is no incorrect IP checksum in the packet.
> > 
> 
> Thanks, I should read the definition of none more careful.
> 
> > I will argue against UNKNOWN. Its description says: "no information about the RX IP checksum". But we do have information about it! We know that the IP checksum is not there (the value is "NULL"), and that it is not supposed to be there (the value is supposed to be "NULL").
> > 
> 
> I thought that "no checksum" => "no information" => UNKNOWN
> 
> > So I consider GOOD the correct response here.
> > 
> > GOOD also means that the application can proceed processing the packet normally without further IP header checksum checking, so it's good for performance.
> > 
> 
> It is very important point and would be nice to have in GOOD
> case definition (both IP and L4 cases). It is the right
> motivation why GOOD makes sense for IPv6.
> 
> > It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD that IPv6 packets always return this value, because IPv6 packets have no IP header checksum, and that is what is expected of them.
> 
> Could you make a patch?

That would be perfect. I agree to use GOOD for IPv6 checksum.

> Bonus question is UDP checksum 0 case. GOOD as well?
> (just want to clarify the documentation while we're on it).

Good question :)



^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: Is it correct to report checksum good when there is no checksum?
  2022-11-10 10:08   ` Andrew Rybchenko
  2022-11-10 10:29     ` Thomas Monjalon
@ 2022-11-10 10:29     ` Morten Brørup
  2022-11-10 10:34       ` Andrew Rybchenko
  1 sibling, 1 reply; 11+ messages in thread
From: Morten Brørup @ 2022-11-10 10:29 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
  Cc: Olivier Matz, Konstantin Ananyev, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand

> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Thursday, 10 November 2022 11.09
> 
> On 11/10/22 12:55, Morten Brørup wrote:
> >> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> >> Sent: Thursday, 10 November 2022 10.26
> >>
> >> Hi all,
> >>
> >> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> >> For me it looks strange, but I see some technical reasons behind.
> >
> > Please note: IPv6 packets by definition have no IP checksum.
> >
> >> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> >> Should UNKNOWN or NONE be used instead?
> >
> > Certainly not NONE. Its description says: "the IP checksum is *not*
> correct in the packet [...]". But there is no incorrect IP checksum in
> the packet.
> >
> 
> Thanks, I should read the definition of none more careful.
> 
> > I will argue against UNKNOWN. Its description says: "no information
> about the RX IP checksum". But we do have information about it! We know
> that the IP checksum is not there (the value is "NULL"), and that it is
> not supposed to be there (the value is supposed to be "NULL").
> >
> 
> I thought that "no checksum" => "no information" => UNKNOWN

That was my initial interpretation too, and it stuck with me for a while.

But then I tried hard to read it differently, tweaking it to support the conclusion I was looking for.

> 
> > So I consider GOOD the correct response here.
> >
> > GOOD also means that the application can proceed processing the
> packet normally without further IP header checksum checking, so it's
> good for performance.
> >
> 
> It is very important point and would be nice to have in GOOD
> case definition (both IP and L4 cases). It is the right
> motivation why GOOD makes sense for IPv6.
> 
> > It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD
> that IPv6 packets always return this value, because IPv6 packets have
> no IP header checksum, and that is what is expected of them.
> >
> 
> Could you make a patch?

Too busy right now, but I'll put it on my todo list. :-)

> 
> Bonus question is UDP checksum 0 case. GOOD as well?
> (just want to clarify the documentation while we're on it).

No. The UDP checksum is not optional in IPv6.

RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets are originated by an IPv6 node, the UDP checksum is not optional. [...] IPv6 receivers must discard UDP packets containing a zero checksum, and should log the error."

> 
> Thanks a lot,
> Andrew.
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Is it correct to report checksum good when there is no checksum?
  2022-11-10 10:29     ` Morten Brørup
@ 2022-11-10 10:34       ` Andrew Rybchenko
  2022-11-10 11:02         ` Morten Brørup
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Rybchenko @ 2022-11-10 10:34 UTC (permalink / raw)
  To: Morten Brørup, dev
  Cc: Olivier Matz, Konstantin Ananyev, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand

On 11/10/22 13:29, Morten Brørup wrote:
>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>> Sent: Thursday, 10 November 2022 11.09
>>
>> On 11/10/22 12:55, Morten Brørup wrote:
>>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>>>> Sent: Thursday, 10 November 2022 10.26
>>>>
>>>> Hi all,
>>>>
>>>> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
>>>> For me it looks strange, but I see some technical reasons behind.
>>>
>>> Please note: IPv6 packets by definition have no IP checksum.
>>>
>>>> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
>>>> Should UNKNOWN or NONE be used instead?
>>>
>>> Certainly not NONE. Its description says: "the IP checksum is *not*
>> correct in the packet [...]". But there is no incorrect IP checksum in
>> the packet.
>>>
>>
>> Thanks, I should read the definition of none more careful.
>>
>>> I will argue against UNKNOWN. Its description says: "no information
>> about the RX IP checksum". But we do have information about it! We know
>> that the IP checksum is not there (the value is "NULL"), and that it is
>> not supposed to be there (the value is supposed to be "NULL").
>>>
>>
>> I thought that "no checksum" => "no information" => UNKNOWN
> 
> That was my initial interpretation too, and it stuck with me for a while.
> 
> But then I tried hard to read it differently, tweaking it to support the conclusion I was looking for.
> 
>>
>>> So I consider GOOD the correct response here.
>>>
>>> GOOD also means that the application can proceed processing the
>> packet normally without further IP header checksum checking, so it's
>> good for performance.
>>>
>>
>> It is very important point and would be nice to have in GOOD
>> case definition (both IP and L4 cases). It is the right
>> motivation why GOOD makes sense for IPv6.
>>
>>> It should be added to the description of RTE_MBUF_F_RX_IP_CKSUM_GOOD
>> that IPv6 packets always return this value, because IPv6 packets have
>> no IP header checksum, and that is what is expected of them.
>>>
>>
>> Could you make a patch?
> 
> Too busy right now, but I'll put it on my todo list. :-)
> 
>>
>> Bonus question is UDP checksum 0 case. GOOD as well?
>> (just want to clarify the documentation while we're on it).
> 
> No. The UDP checksum is not optional in IPv6.
> 
> RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets are originated by an IPv6 node, the UDP checksum is not optional. [...] IPv6 receivers must discard UDP packets containing a zero checksum, and should log the error."
> 

Yes I know, but I'm asking about IPv4 case with UDP checksum 0.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: Is it correct to report checksum good when there is no checksum?
  2022-11-10 10:34       ` Andrew Rybchenko
@ 2022-11-10 11:02         ` Morten Brørup
  2022-11-10 11:11           ` Bruce Richardson
  2022-11-10 11:23           ` Andrew Rybchenko
  0 siblings, 2 replies; 11+ messages in thread
From: Morten Brørup @ 2022-11-10 11:02 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
  Cc: Olivier Matz, Konstantin Ananyev, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand

> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Thursday, 10 November 2022 11.34
> 
> On 11/10/22 13:29, Morten Brørup wrote:
> >> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> >> Sent: Thursday, 10 November 2022 11.09
> >>
> >> On 11/10/22 12:55, Morten Brørup wrote:
> >>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> >>>> Sent: Thursday, 10 November 2022 10.26
> >>>>
> >>>> Hi all,
> >>>>
> >>>> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> >>>> For me it looks strange, but I see some technical reasons behind.
> >>>
> >>> Please note: IPv6 packets by definition have no IP checksum.
> >>>
> >>>> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> >>>> Should UNKNOWN or NONE be used instead?
> >>>
> >>> Certainly not NONE. Its description says: "the IP checksum is *not*
> >> correct in the packet [...]". But there is no incorrect IP checksum
> in
> >> the packet.
> >>>
> >>
> >> Thanks, I should read the definition of none more careful.
> >>
> >>> I will argue against UNKNOWN. Its description says: "no information
> >> about the RX IP checksum". But we do have information about it! We
> know
> >> that the IP checksum is not there (the value is "NULL"), and that it
> is
> >> not supposed to be there (the value is supposed to be "NULL").
> >>>
> >>
> >> I thought that "no checksum" => "no information" => UNKNOWN
> >
> > That was my initial interpretation too, and it stuck with me for a
> while.
> >
> > But then I tried hard to read it differently, tweaking it to support
> the conclusion I was looking for.
> >
> >>
> >>> So I consider GOOD the correct response here.
> >>>
> >>> GOOD also means that the application can proceed processing the
> >> packet normally without further IP header checksum checking, so it's
> >> good for performance.
> >>>
> >>
> >> It is very important point and would be nice to have in GOOD
> >> case definition (both IP and L4 cases). It is the right
> >> motivation why GOOD makes sense for IPv6.
> >>
> >>> It should be added to the description of
> RTE_MBUF_F_RX_IP_CKSUM_GOOD
> >> that IPv6 packets always return this value, because IPv6 packets
> have
> >> no IP header checksum, and that is what is expected of them.
> >>>
> >>
> >> Could you make a patch?
> >
> > Too busy right now, but I'll put it on my todo list. :-)
> >
> >>
> >> Bonus question is UDP checksum 0 case. GOOD as well?
> >> (just want to clarify the documentation while we're on it).
> >
> > No. The UDP checksum is not optional in IPv6.
> >
> > RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets
> are originated by an IPv6 node, the UDP checksum is not optional. [...]
> IPv6 receivers must discard UDP packets containing a zero checksum, and
> should log the error."
> >
> 
> Yes I know, but I'm asking about IPv4 case with UDP checksum 0.

It cannot be UNKNOWN, because we do have information: The checksum was intentionally omitted.

I would prefer GOOD, using the same logic as for the IPv6 header checksum.

Trying very hard to tweak the meaning of NONE's description ("the L4 checksum is not correct in the packet data, but the integrity of the L4 data is verified."), we could argue that "not correct" != "intentionally omitted" (and an intentional omission is absolutely correct), and conclude that it cannot be NONE. A seasoned politician would say this without blinking, but it is up to individual interpretation.

We should settle on either GOOD or NONE, and write it in the documentation.

In a perfect world, the PMD DPDK compliance tests should also check things like this.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Is it correct to report checksum good when there is no checksum?
  2022-11-10 11:02         ` Morten Brørup
@ 2022-11-10 11:11           ` Bruce Richardson
  2022-11-10 11:26             ` Andrew Rybchenko
  2022-11-10 11:23           ` Andrew Rybchenko
  1 sibling, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2022-11-10 11:11 UTC (permalink / raw)
  To: Morten Brørup
  Cc: Andrew Rybchenko, dev, Olivier Matz, Konstantin Ananyev,
	Stephen Hemminger, Jerin Jacob Kollanukkaran, Ferruh Yigit,
	Thomas Monjalon, David Marchand

On Thu, Nov 10, 2022 at 12:02:48PM +0100, Morten Brørup wrote:
> > From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> > Sent: Thursday, 10 November 2022 11.34
> > 
> > On 11/10/22 13:29, Morten Brørup wrote:
> > >> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> > >> Sent: Thursday, 10 November 2022 11.09
> > >>
> > >> On 11/10/22 12:55, Morten Brørup wrote:
> > >>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> > >>>> Sent: Thursday, 10 November 2022 10.26
> > >>>>
> > >>>> Hi all,
> > >>>>
> > >>>> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
> > >>>> For me it looks strange, but I see some technical reasons behind.
> > >>>
> > >>> Please note: IPv6 packets by definition have no IP checksum.
> > >>>
> > >>>> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> > >>>> Should UNKNOWN or NONE be used instead?
> > >>>
> > >>> Certainly not NONE. Its description says: "the IP checksum is *not*
> > >> correct in the packet [...]". But there is no incorrect IP checksum
> > in
> > >> the packet.
> > >>>
> > >>
> > >> Thanks, I should read the definition of none more careful.
> > >>
> > >>> I will argue against UNKNOWN. Its description says: "no information
> > >> about the RX IP checksum". But we do have information about it! We
> > know
> > >> that the IP checksum is not there (the value is "NULL"), and that it
> > is
> > >> not supposed to be there (the value is supposed to be "NULL").
> > >>>
> > >>
> > >> I thought that "no checksum" => "no information" => UNKNOWN
> > >
> > > That was my initial interpretation too, and it stuck with me for a
> > while.
> > >
> > > But then I tried hard to read it differently, tweaking it to support
> > the conclusion I was looking for.
> > >
> > >>
> > >>> So I consider GOOD the correct response here.
> > >>>
> > >>> GOOD also means that the application can proceed processing the
> > >> packet normally without further IP header checksum checking, so it's
> > >> good for performance.
> > >>>
> > >>
> > >> It is very important point and would be nice to have in GOOD
> > >> case definition (both IP and L4 cases). It is the right
> > >> motivation why GOOD makes sense for IPv6.
> > >>
> > >>> It should be added to the description of
> > RTE_MBUF_F_RX_IP_CKSUM_GOOD
> > >> that IPv6 packets always return this value, because IPv6 packets
> > have
> > >> no IP header checksum, and that is what is expected of them.
> > >>>
> > >>
> > >> Could you make a patch?
> > >
> > > Too busy right now, but I'll put it on my todo list. :-)
> > >
> > >>
> > >> Bonus question is UDP checksum 0 case. GOOD as well?
> > >> (just want to clarify the documentation while we're on it).
> > >
> > > No. The UDP checksum is not optional in IPv6.
> > >
> > > RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets
> > are originated by an IPv6 node, the UDP checksum is not optional. [...]
> > IPv6 receivers must discard UDP packets containing a zero checksum, and
> > should log the error."
> > >
> > 
> > Yes I know, but I'm asking about IPv4 case with UDP checksum 0.
> 
> It cannot be UNKNOWN, because we do have information: The checksum was intentionally omitted.
> 
> I would prefer GOOD, using the same logic as for the IPv6 header checksum.
> 
> Trying very hard to tweak the meaning of NONE's description ("the L4 checksum is not correct in the packet data, but the integrity of the L4 data is verified."), we could argue that "not correct" != "intentionally omitted" (and an intentional omission is absolutely correct), and conclude that it cannot be NONE. A seasoned politician would say this without blinking, but it is up to individual interpretation.
> 
> We should settle on either GOOD or NONE, and write it in the documentation.
> 
> In a perfect world, the PMD DPDK compliance tests should also check things like this.
> 

I would think that for cases where the checksum is intentionally omitted we
either add a new flag for "not applicable" or else just go with "good" as
you suggest. I think for simplicity to go with the latter.

Can we redefine "GOOD" to just mean "does not need to be checked by
software", rather than trying to define it in terms of what was done by
hardware?

/Bruce

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Is it correct to report checksum good when there is no checksum?
  2022-11-10 11:02         ` Morten Brørup
  2022-11-10 11:11           ` Bruce Richardson
@ 2022-11-10 11:23           ` Andrew Rybchenko
  1 sibling, 0 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2022-11-10 11:23 UTC (permalink / raw)
  To: Morten Brørup, dev
  Cc: Olivier Matz, Konstantin Ananyev, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand

On 11/10/22 14:02, Morten Brørup wrote:
>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>> Sent: Thursday, 10 November 2022 11.34
>>
>> On 11/10/22 13:29, Morten Brørup wrote:
>>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>>>> Sent: Thursday, 10 November 2022 11.09
>>>>
>>>> On 11/10/22 12:55, Morten Brørup wrote:
>>>>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>>>>>> Sent: Thursday, 10 November 2022 10.26
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
>>>>>> For me it looks strange, but I see some technical reasons behind.
>>>>>
>>>>> Please note: IPv6 packets by definition have no IP checksum.
>>>>>
>>>>>> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
>>>>>> Should UNKNOWN or NONE be used instead?
>>>>>
>>>>> Certainly not NONE. Its description says: "the IP checksum is *not*
>>>> correct in the packet [...]". But there is no incorrect IP checksum
>> in
>>>> the packet.
>>>>>
>>>>
>>>> Thanks, I should read the definition of none more careful.
>>>>
>>>>> I will argue against UNKNOWN. Its description says: "no information
>>>> about the RX IP checksum". But we do have information about it! We
>> know
>>>> that the IP checksum is not there (the value is "NULL"), and that it
>> is
>>>> not supposed to be there (the value is supposed to be "NULL").
>>>>>
>>>>
>>>> I thought that "no checksum" => "no information" => UNKNOWN
>>>
>>> That was my initial interpretation too, and it stuck with me for a
>> while.
>>>
>>> But then I tried hard to read it differently, tweaking it to support
>> the conclusion I was looking for.
>>>
>>>>
>>>>> So I consider GOOD the correct response here.
>>>>>
>>>>> GOOD also means that the application can proceed processing the
>>>> packet normally without further IP header checksum checking, so it's
>>>> good for performance.
>>>>>
>>>>
>>>> It is very important point and would be nice to have in GOOD
>>>> case definition (both IP and L4 cases). It is the right
>>>> motivation why GOOD makes sense for IPv6.
>>>>
>>>>> It should be added to the description of
>> RTE_MBUF_F_RX_IP_CKSUM_GOOD
>>>> that IPv6 packets always return this value, because IPv6 packets
>> have
>>>> no IP header checksum, and that is what is expected of them.
>>>>>
>>>>
>>>> Could you make a patch?
>>>
>>> Too busy right now, but I'll put it on my todo list. :-)
>>>
>>>>
>>>> Bonus question is UDP checksum 0 case. GOOD as well?
>>>> (just want to clarify the documentation while we're on it).
>>>
>>> No. The UDP checksum is not optional in IPv6.
>>>
>>> RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets
>> are originated by an IPv6 node, the UDP checksum is not optional. [...]
>> IPv6 receivers must discard UDP packets containing a zero checksum, and
>> should log the error."
>>>
>>
>> Yes I know, but I'm asking about IPv4 case with UDP checksum 0.
> 
> It cannot be UNKNOWN, because we do have information: The checksum was intentionally omitted.
> 

I think that UNKNOWN definition should be updated to say that
it means that checksum is present and could be verified, but
NIC has not done it and application should do it itself.

> I would prefer GOOD, using the same logic as for the IPv6 header checksum.
> 

Yes, since it correct checksum from UDP over IPv4 protocol
definition. Application simply has no information to verify
checksum, so it cannot be UNKNOWN.

Since application gets entry packet in DPDK case, in GOOD case
it could check if checksum is 0 or not itself and do extra
checks in 0 case if it is possible (higher layer checksums etc)
and required.

> Trying very hard to tweak the meaning of NONE's description ("the L4 checksum is not correct in the packet data, but the integrity of the L4 data is verified."), we could argue that "not correct" != "intentionally omitted" (and an intentional omission is absolutely correct), and conclude that it cannot be NONE. A seasoned politician would say this without blinking, but it is up to individual interpretation.
> 
> We should settle on either GOOD or NONE, and write it in the documentation.
> 

NONE requires "but the integrity of the L4 data is verified".
Who said that NIC has verified L4 data integrity?

> In a perfect world, the PMD DPDK compliance tests should also check things like this.
> 

JFYI The initial IPv6 question comes from my attempt to
classify [1]. Now I understand that the test should be
fixed to expect GOOD in IPv6 case, not UNKNOWN.

[1] 
https://ts-factory.io/bublik/v2/log/163204?focusId=164090&mode=treeAndinfoAndlog

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Is it correct to report checksum good when there is no checksum?
  2022-11-10 11:11           ` Bruce Richardson
@ 2022-11-10 11:26             ` Andrew Rybchenko
  2022-11-10 11:57               ` Morten Brørup
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Rybchenko @ 2022-11-10 11:26 UTC (permalink / raw)
  To: Bruce Richardson, Morten Brørup
  Cc: dev, Olivier Matz, Konstantin Ananyev, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand

On 11/10/22 14:11, Bruce Richardson wrote:
> On Thu, Nov 10, 2022 at 12:02:48PM +0100, Morten Brørup wrote:
>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>>> Sent: Thursday, 10 November 2022 11.34
>>>
>>> On 11/10/22 13:29, Morten Brørup wrote:
>>>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>>>>> Sent: Thursday, 10 November 2022 11.09
>>>>>
>>>>> On 11/10/22 12:55, Morten Brørup wrote:
>>>>>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>>>>>>> Sent: Thursday, 10 November 2022 10.26
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6 packets.
>>>>>>> For me it looks strange, but I see some technical reasons behind.
>>>>>>
>>>>>> Please note: IPv6 packets by definition have no IP checksum.
>>>>>>
>>>>>>> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
>>>>>>> Should UNKNOWN or NONE be used instead?
>>>>>>
>>>>>> Certainly not NONE. Its description says: "the IP checksum is *not*
>>>>> correct in the packet [...]". But there is no incorrect IP checksum
>>> in
>>>>> the packet.
>>>>>>
>>>>>
>>>>> Thanks, I should read the definition of none more careful.
>>>>>
>>>>>> I will argue against UNKNOWN. Its description says: "no information
>>>>> about the RX IP checksum". But we do have information about it! We
>>> know
>>>>> that the IP checksum is not there (the value is "NULL"), and that it
>>> is
>>>>> not supposed to be there (the value is supposed to be "NULL").
>>>>>>
>>>>>
>>>>> I thought that "no checksum" => "no information" => UNKNOWN
>>>>
>>>> That was my initial interpretation too, and it stuck with me for a
>>> while.
>>>>
>>>> But then I tried hard to read it differently, tweaking it to support
>>> the conclusion I was looking for.
>>>>
>>>>>
>>>>>> So I consider GOOD the correct response here.
>>>>>>
>>>>>> GOOD also means that the application can proceed processing the
>>>>> packet normally without further IP header checksum checking, so it's
>>>>> good for performance.
>>>>>>
>>>>>
>>>>> It is very important point and would be nice to have in GOOD
>>>>> case definition (both IP and L4 cases). It is the right
>>>>> motivation why GOOD makes sense for IPv6.
>>>>>
>>>>>> It should be added to the description of
>>> RTE_MBUF_F_RX_IP_CKSUM_GOOD
>>>>> that IPv6 packets always return this value, because IPv6 packets
>>> have
>>>>> no IP header checksum, and that is what is expected of them.
>>>>>>
>>>>>
>>>>> Could you make a patch?
>>>>
>>>> Too busy right now, but I'll put it on my todo list. :-)
>>>>
>>>>>
>>>>> Bonus question is UDP checksum 0 case. GOOD as well?
>>>>> (just want to clarify the documentation while we're on it).
>>>>
>>>> No. The UDP checksum is not optional in IPv6.
>>>>
>>>> RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets
>>> are originated by an IPv6 node, the UDP checksum is not optional. [...]
>>> IPv6 receivers must discard UDP packets containing a zero checksum, and
>>> should log the error."
>>>>
>>>
>>> Yes I know, but I'm asking about IPv4 case with UDP checksum 0.
>>
>> It cannot be UNKNOWN, because we do have information: The checksum was intentionally omitted.
>>
>> I would prefer GOOD, using the same logic as for the IPv6 header checksum.
>>
>> Trying very hard to tweak the meaning of NONE's description ("the L4 checksum is not correct in the packet data, but the integrity of the L4 data is verified."), we could argue that "not correct" != "intentionally omitted" (and an intentional omission is absolutely correct), and conclude that it cannot be NONE. A seasoned politician would say this without blinking, but it is up to individual interpretation.
>>
>> We should settle on either GOOD or NONE, and write it in the documentation.
>>
>> In a perfect world, the PMD DPDK compliance tests should also check things like this.
>>
> 
> I would think that for cases where the checksum is intentionally omitted we
> either add a new flag for "not applicable" or else just go with "good" as
> you suggest. I think for simplicity to go with the latter.
> 
> Can we redefine "GOOD" to just mean "does not need to be checked by
> software", rather than trying to define it in terms of what was done by
> hardware?

Yes, it is very good idea since we are providing the
information to application and make it clear what the
application should do.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: Is it correct to report checksum good when there is no checksum?
  2022-11-10 11:26             ` Andrew Rybchenko
@ 2022-11-10 11:57               ` Morten Brørup
  0 siblings, 0 replies; 11+ messages in thread
From: Morten Brørup @ 2022-11-10 11:57 UTC (permalink / raw)
  To: Andrew Rybchenko, Bruce Richardson
  Cc: dev, Olivier Matz, Konstantin Ananyev, Stephen Hemminger,
	Jerin Jacob Kollanukkaran, Ferruh Yigit, Thomas Monjalon,
	David Marchand

> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Thursday, 10 November 2022 12.26
> 
> On 11/10/22 14:11, Bruce Richardson wrote:
> > On Thu, Nov 10, 2022 at 12:02:48PM +0100, Morten Brørup wrote:
> >>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> >>> Sent: Thursday, 10 November 2022 11.34
> >>>
> >>> On 11/10/22 13:29, Morten Brørup wrote:
> >>>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> >>>>> Sent: Thursday, 10 November 2022 11.09
> >>>>>
> >>>>> On 11/10/22 12:55, Morten Brørup wrote:
> >>>>>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> >>>>>>> Sent: Thursday, 10 November 2022 10.26
> >>>>>>>
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> some drivers report RTE_MBUF_F_RX_IP_CKSUM_GOOD for IPv6
> packets.
> >>>>>>> For me it looks strange, but I see some technical reasons
> behind.
> >>>>>>
> >>>>>> Please note: IPv6 packets by definition have no IP checksum.
> >>>>>>
> >>>>>>> Documentation in lib/mbuf/rte_mbuf_core.h is a bit vague.
> >>>>>>> Should UNKNOWN or NONE be used instead?
> >>>>>>
> >>>>>> Certainly not NONE. Its description says: "the IP checksum is
> *not*
> >>>>> correct in the packet [...]". But there is no incorrect IP
> checksum
> >>> in
> >>>>> the packet.
> >>>>>>
> >>>>>
> >>>>> Thanks, I should read the definition of none more careful.
> >>>>>
> >>>>>> I will argue against UNKNOWN. Its description says: "no
> information
> >>>>> about the RX IP checksum". But we do have information about it!
> We
> >>> know
> >>>>> that the IP checksum is not there (the value is "NULL"), and that
> it
> >>> is
> >>>>> not supposed to be there (the value is supposed to be "NULL").
> >>>>>>
> >>>>>
> >>>>> I thought that "no checksum" => "no information" => UNKNOWN
> >>>>
> >>>> That was my initial interpretation too, and it stuck with me for a
> >>> while.
> >>>>
> >>>> But then I tried hard to read it differently, tweaking it to
> support
> >>> the conclusion I was looking for.
> >>>>
> >>>>>
> >>>>>> So I consider GOOD the correct response here.
> >>>>>>
> >>>>>> GOOD also means that the application can proceed processing the
> >>>>> packet normally without further IP header checksum checking, so
> it's
> >>>>> good for performance.
> >>>>>>
> >>>>>
> >>>>> It is very important point and would be nice to have in GOOD
> >>>>> case definition (both IP and L4 cases). It is the right
> >>>>> motivation why GOOD makes sense for IPv6.
> >>>>>
> >>>>>> It should be added to the description of
> >>> RTE_MBUF_F_RX_IP_CKSUM_GOOD
> >>>>> that IPv6 packets always return this value, because IPv6 packets
> >>> have
> >>>>> no IP header checksum, and that is what is expected of them.
> >>>>>>
> >>>>>
> >>>>> Could you make a patch?
> >>>>
> >>>> Too busy right now, but I'll put it on my todo list. :-)
> >>>>
> >>>>>
> >>>>> Bonus question is UDP checksum 0 case. GOOD as well?
> >>>>> (just want to clarify the documentation while we're on it).
> >>>>
> >>>> No. The UDP checksum is not optional in IPv6.
> >>>>
> >>>> RFC 2460 section 8.1 bullet 4 says: "Unlike IPv4, when UDP packets
> >>> are originated by an IPv6 node, the UDP checksum is not optional.
> [...]
> >>> IPv6 receivers must discard UDP packets containing a zero checksum,
> and
> >>> should log the error."
> >>>>
> >>>
> >>> Yes I know, but I'm asking about IPv4 case with UDP checksum 0.
> >>
> >> It cannot be UNKNOWN, because we do have information: The checksum
> was intentionally omitted.
> >>
> >> I would prefer GOOD, using the same logic as for the IPv6 header
> checksum.
> >>
> >> Trying very hard to tweak the meaning of NONE's description ("the L4
> checksum is not correct in the packet data, but the integrity of the L4
> data is verified."), we could argue that "not correct" !=
> "intentionally omitted" (and an intentional omission is absolutely
> correct), and conclude that it cannot be NONE. A seasoned politician
> would say this without blinking, but it is up to individual
> interpretation.
> >>
> >> We should settle on either GOOD or NONE, and write it in the
> documentation.
> >>
> >> In a perfect world, the PMD DPDK compliance tests should also check
> things like this.
> >>
> >
> > I would think that for cases where the checksum is intentionally
> omitted we
> > either add a new flag for "not applicable" or else just go with
> "good" as
> > you suggest. I think for simplicity to go with the latter.
> >
> > Can we redefine "GOOD" to just mean "does not need to be checked by
> > software", rather than trying to define it in terms of what was done
> by
> > hardware?
> 
> Yes, it is very good idea since we are providing the
> information to application and make it clear what the
> application should do.
> 

We need to think this all the way through:

Eventually, this information will be carried over to the TX side, so we also need to consider TX handling.

E.g. GOOD means that the packet is good to go, and does not need the checksum to be updated on the way out.


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-11-10 11:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-10  9:25 Is it correct to report checksum good when there is no checksum? Andrew Rybchenko
2022-11-10  9:55 ` Morten Brørup
2022-11-10 10:08   ` Andrew Rybchenko
2022-11-10 10:29     ` Thomas Monjalon
2022-11-10 10:29     ` Morten Brørup
2022-11-10 10:34       ` Andrew Rybchenko
2022-11-10 11:02         ` Morten Brørup
2022-11-10 11:11           ` Bruce Richardson
2022-11-10 11:26             ` Andrew Rybchenko
2022-11-10 11:57               ` Morten Brørup
2022-11-10 11:23           ` Andrew Rybchenko

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