diff mbox series

[1/4] net/tap: fix Rx cksum flags on IP options packets

Message ID 20210427135755.927-2-olivier.matz@6wind.com (mailing list archive)
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers
Series net/tap: fix Rx cksum |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Olivier Matz April 27, 2021, 1:57 p.m. UTC 
  When packet type is IPV4_EXT, the checksum is always marked as good in
the mbuf offload flags.

Since we know the header lengths, we can easily call
rte_ipv4_udptcp_cksum() in this case too.

Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
Cc: stable@dpdk.org

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/tap/rte_eth_tap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ferruh Yigit April 30, 2021, 2:48 p.m. UTC | #1 
On 4/27/2021 2:57 PM, Olivier Matz wrote:
> When packet type is IPV4_EXT, the checksum is always marked as good in
> the mbuf offload flags.
> 
> Since we know the header lengths, we can easily call
> rte_ipv4_udptcp_cksum() in this case too.
> 
> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 68baa18523..e7b185a4b5 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -350,7 +350,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
>  		/* Don't verify checksum for multi-segment packets. */
>  		if (mbuf->nb_segs > 1)
>  			return;
> -		if (l3 == RTE_PTYPE_L3_IPV4) {
> +		if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {

Should we take 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' into account?

>  			if (l4 == RTE_PTYPE_L4_UDP) {
>  				udp_hdr = (struct rte_udp_hdr *)l4_hdr;
>  				if (udp_hdr->dgram_cksum == 0) {
> @@ -364,7 +364,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
>  				}
>  			}
>  			cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> -		} else if (l3 == RTE_PTYPE_L3_IPV6) {
> +		} else { /* l3 == RTE_PTYPE_L3_IPV6, checked above */
>  			cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
>  		}
>  		mbuf->ol_flags |= cksum ?
>
Andrew Rybchenko June 8, 2021, 10:13 a.m. UTC | #2 
On 4/30/21 5:48 PM, Ferruh Yigit wrote:
> On 4/27/2021 2:57 PM, Olivier Matz wrote:
>> When packet type is IPV4_EXT, the checksum is always marked as good in
>> the mbuf offload flags.
>>
>> Since we know the header lengths, we can easily call
>> rte_ipv4_udptcp_cksum() in this case too.
>>
>> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>> ---
>>  drivers/net/tap/rte_eth_tap.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index 68baa18523..e7b185a4b5 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -350,7 +350,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
>>  		/* Don't verify checksum for multi-segment packets. */
>>  		if (mbuf->nb_segs > 1)
>>  			return;
>> -		if (l3 == RTE_PTYPE_L3_IPV4) {
>> +		if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
> 
> Should we take 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' into account?

I think we should.

> 
>>  			if (l4 == RTE_PTYPE_L4_UDP) {
>>  				udp_hdr = (struct rte_udp_hdr *)l4_hdr;
>>  				if (udp_hdr->dgram_cksum == 0) {
>> @@ -364,7 +364,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
>>  				}
>>  			}
>>  			cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
>> -		} else if (l3 == RTE_PTYPE_L3_IPV6) {
>> +		} else { /* l3 == RTE_PTYPE_L3_IPV6, checked above */
>>  			cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
>>  		}
>>  		mbuf->ol_flags |= cksum ?
>>
Olivier Matz June 8, 2021, 12:29 p.m. UTC | #3 
Hi Ferruh, Andrew,

On Tue, Jun 08, 2021 at 01:13:59PM +0300, Andrew Rybchenko wrote:
> On 4/30/21 5:48 PM, Ferruh Yigit wrote:
> > On 4/27/2021 2:57 PM, Olivier Matz wrote:
> >> When packet type is IPV4_EXT, the checksum is always marked as good in
> >> the mbuf offload flags.
> >>
> >> Since we know the header lengths, we can easily call
> >> rte_ipv4_udptcp_cksum() in this case too.
> >>
> >> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >> ---
> >>  drivers/net/tap/rte_eth_tap.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> >> index 68baa18523..e7b185a4b5 100644
> >> --- a/drivers/net/tap/rte_eth_tap.c
> >> +++ b/drivers/net/tap/rte_eth_tap.c
> >> @@ -350,7 +350,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
> >>  		/* Don't verify checksum for multi-segment packets. */
> >>  		if (mbuf->nb_segs > 1)
> >>  			return;
> >> -		if (l3 == RTE_PTYPE_L3_IPV4) {
> >> +		if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
> > 
> > Should we take 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' into account?
> 
> I think we should.

I think 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' cannot happen here:

- mbuf->packet_type is generated by rte_net_get_ptype(), which cannot
  return 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN'
- right above this code, we already returned if l3 is not in
  (RTE_PTYPE_L3_IPV4, RTE_PTYPE_L3_IPV4_EXT, RTE_PTYPE_L3_IPV6)

> > 
> >>  			if (l4 == RTE_PTYPE_L4_UDP) {
> >>  				udp_hdr = (struct rte_udp_hdr *)l4_hdr;
> >>  				if (udp_hdr->dgram_cksum == 0) {
> >> @@ -364,7 +364,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
> >>  				}
> >>  			}
> >>  			cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> >> -		} else if (l3 == RTE_PTYPE_L3_IPV6) {
> >> +		} else { /* l3 == RTE_PTYPE_L3_IPV6, checked above */
> >>  			cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> >>  		}
> >>  		mbuf->ol_flags |= cksum ?
> >>
>
Andrew Rybchenko June 8, 2021, 12:34 p.m. UTC | #4 
On 6/8/21 3:29 PM, Olivier Matz wrote:
> Hi Ferruh, Andrew,
> 
> On Tue, Jun 08, 2021 at 01:13:59PM +0300, Andrew Rybchenko wrote:
>> On 4/30/21 5:48 PM, Ferruh Yigit wrote:
>>> On 4/27/2021 2:57 PM, Olivier Matz wrote:
>>>> When packet type is IPV4_EXT, the checksum is always marked as good in
>>>> the mbuf offload flags.
>>>>
>>>> Since we know the header lengths, we can easily call
>>>> rte_ipv4_udptcp_cksum() in this case too.
>>>>
>>>> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>>> ---
>>>>  drivers/net/tap/rte_eth_tap.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>>>> index 68baa18523..e7b185a4b5 100644
>>>> --- a/drivers/net/tap/rte_eth_tap.c
>>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>>> @@ -350,7 +350,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
>>>>  		/* Don't verify checksum for multi-segment packets. */
>>>>  		if (mbuf->nb_segs > 1)
>>>>  			return;
>>>> -		if (l3 == RTE_PTYPE_L3_IPV4) {
>>>> +		if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
>>>
>>> Should we take 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' into account?
>>
>> I think we should.
> 
> I think 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' cannot happen here:
> 
> - mbuf->packet_type is generated by 

(), which cannot
>   return 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN'

My question if it is guaranteed and the only possible branch.
Can application set packet_type itself and do not call
rte_net_get_ptype(). Yes, typically application knows
if it has IPv4 options in the header or not, but theoretically
could be unaware as well.

> - right above this code, we already returned if l3 is not in
>   (RTE_PTYPE_L3_IPV4, RTE_PTYPE_L3_IPV4_EXT, RTE_PTYPE_L3_IPV6)

If so, it sounds like it should be allowed above as well.

> 
>>>
>>>>  			if (l4 == RTE_PTYPE_L4_UDP) {
>>>>  				udp_hdr = (struct rte_udp_hdr *)l4_hdr;
>>>>  				if (udp_hdr->dgram_cksum == 0) {
>>>> @@ -364,7 +364,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
>>>>  				}
>>>>  			}
>>>>  			cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
>>>> -		} else if (l3 == RTE_PTYPE_L3_IPV6) {
>>>> +		} else { /* l3 == RTE_PTYPE_L3_IPV6, checked above */
>>>>  			cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
>>>>  		}
>>>>  		mbuf->ol_flags |= cksum ?
>>>>
>>
Olivier Matz June 8, 2021, 12:49 p.m. UTC | #5 
On Tue, Jun 08, 2021 at 03:34:36PM +0300, Andrew Rybchenko wrote:
> On 6/8/21 3:29 PM, Olivier Matz wrote:
> > Hi Ferruh, Andrew,
> > 
> > On Tue, Jun 08, 2021 at 01:13:59PM +0300, Andrew Rybchenko wrote:
> >> On 4/30/21 5:48 PM, Ferruh Yigit wrote:
> >>> On 4/27/2021 2:57 PM, Olivier Matz wrote:
> >>>> When packet type is IPV4_EXT, the checksum is always marked as good in
> >>>> the mbuf offload flags.
> >>>>
> >>>> Since we know the header lengths, we can easily call
> >>>> rte_ipv4_udptcp_cksum() in this case too.
> >>>>
> >>>> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
> >>>> Cc: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >>>> ---
> >>>>  drivers/net/tap/rte_eth_tap.c | 4 ++--
> >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> >>>> index 68baa18523..e7b185a4b5 100644
> >>>> --- a/drivers/net/tap/rte_eth_tap.c
> >>>> +++ b/drivers/net/tap/rte_eth_tap.c
> >>>> @@ -350,7 +350,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
> >>>>  		/* Don't verify checksum for multi-segment packets. */
> >>>>  		if (mbuf->nb_segs > 1)
> >>>>  			return;
> >>>> -		if (l3 == RTE_PTYPE_L3_IPV4) {
> >>>> +		if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
> >>>
> >>> Should we take 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' into account?
> >>
> >> I think we should.
> > 
> > I think 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' cannot happen here:
> > 
> > - mbuf->packet_type is generated by 
> 
> (), which cannot
> >   return 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN'
> 
> My question if it is guaranteed and the only possible branch.
> Can application set packet_type itself and do not call
> rte_net_get_ptype(). Yes, typically application knows
> if it has IPv4 options in the header or not, but theoretically
> could be unaware as well.

This function is called on the Rx path from pmd_rx_burst(), so
the application does not have access to the mbuf.

The software parser that sets the packet type returns either
RTE_PTYPE_L3_IPV4 if there is no option, or RTE_PTYPE_L3_IPV4_EXT
else. The value RTE_PTYPE_L3_IPV4_EXT_UNKNOWN is used by PMDs that don't
know if there are options.

> > - right above this code, we already returned if l3 is not in
> >   (RTE_PTYPE_L3_IPV4, RTE_PTYPE_L3_IPV4_EXT, RTE_PTYPE_L3_IPV6)
> 
> If so, it sounds like it should be allowed above as well.
> 
> > 
> >>>
> >>>>  			if (l4 == RTE_PTYPE_L4_UDP) {
> >>>>  				udp_hdr = (struct rte_udp_hdr *)l4_hdr;
> >>>>  				if (udp_hdr->dgram_cksum == 0) {
> >>>> @@ -364,7 +364,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
> >>>>  				}
> >>>>  			}
> >>>>  			cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> >>>> -		} else if (l3 == RTE_PTYPE_L3_IPV6) {
> >>>> +		} else { /* l3 == RTE_PTYPE_L3_IPV6, checked above */
> >>>>  			cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> >>>>  		}
> >>>>  		mbuf->ol_flags |= cksum ?
> >>>>
> >>
>
Andrew Rybchenko June 8, 2021, 1:57 p.m. UTC | #6 
On 6/8/21 3:49 PM, Olivier Matz wrote:
> On Tue, Jun 08, 2021 at 03:34:36PM +0300, Andrew Rybchenko wrote:
>> On 6/8/21 3:29 PM, Olivier Matz wrote:
>>> Hi Ferruh, Andrew,
>>>
>>> On Tue, Jun 08, 2021 at 01:13:59PM +0300, Andrew Rybchenko wrote:
>>>> On 4/30/21 5:48 PM, Ferruh Yigit wrote:
>>>>> On 4/27/2021 2:57 PM, Olivier Matz wrote:
>>>>>> When packet type is IPV4_EXT, the checksum is always marked as good in
>>>>>> the mbuf offload flags.
>>>>>>
>>>>>> Since we know the header lengths, we can easily call
>>>>>> rte_ipv4_udptcp_cksum() in this case too.
>>>>>>
>>>>>> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>>>>> ---
>>>>>>  drivers/net/tap/rte_eth_tap.c | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>>>>>> index 68baa18523..e7b185a4b5 100644
>>>>>> --- a/drivers/net/tap/rte_eth_tap.c
>>>>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>>>>> @@ -350,7 +350,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
>>>>>>  		/* Don't verify checksum for multi-segment packets. */
>>>>>>  		if (mbuf->nb_segs > 1)
>>>>>>  			return;
>>>>>> -		if (l3 == RTE_PTYPE_L3_IPV4) {
>>>>>> +		if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
>>>>>
>>>>> Should we take 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' into account?
>>>>
>>>> I think we should.
>>>
>>> I think 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' cannot happen here:
>>>
>>> - mbuf->packet_type is generated by 
>>
>> (), which cannot
>>>   return 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN'
>>
>> My question if it is guaranteed and the only possible branch.
>> Can application set packet_type itself and do not call
>> rte_net_get_ptype(). Yes, typically application knows
>> if it has IPv4 options in the header or not, but theoretically
>> could be unaware as well.
> 
> This function is called on the Rx path from pmd_rx_burst(), so
> the application does not have access to the mbuf.
> 
> The software parser that sets the packet type returns either
> RTE_PTYPE_L3_IPV4 if there is no option, or RTE_PTYPE_L3_IPV4_EXT
> else. The value RTE_PTYPE_L3_IPV4_EXT_UNKNOWN is used by PMDs that don't
> know if there are options.

I see. What I'm trying to say that there are non
obvious assumptions here on rte_net_get_ptype()
behaviour which can be changed. May be it makes
sense to add comments here to highlight it.

> 
>>> - right above this code, we already returned if l3 is not in
>>>   (RTE_PTYPE_L3_IPV4, RTE_PTYPE_L3_IPV4_EXT, RTE_PTYPE_L3_IPV6)
>>
>> If so, it sounds like it should be allowed above as well.
>>
>>>
>>>>>
>>>>>>  			if (l4 == RTE_PTYPE_L4_UDP) {
>>>>>>  				udp_hdr = (struct rte_udp_hdr *)l4_hdr;
>>>>>>  				if (udp_hdr->dgram_cksum == 0) {
>>>>>> @@ -364,7 +364,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
>>>>>>  				}
>>>>>>  			}
>>>>>>  			cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
>>>>>> -		} else if (l3 == RTE_PTYPE_L3_IPV6) {
>>>>>> +		} else { /* l3 == RTE_PTYPE_L3_IPV6, checked above */
>>>>>>  			cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
>>>>>>  		}
>>>>>>  		mbuf->ol_flags |= cksum ?
>>>>>>
>>>>
>>
Olivier Matz June 8, 2021, 2:30 p.m. UTC | #7 
On Tue, Jun 08, 2021 at 04:57:00PM +0300, Andrew Rybchenko wrote:
> On 6/8/21 3:49 PM, Olivier Matz wrote:
> > On Tue, Jun 08, 2021 at 03:34:36PM +0300, Andrew Rybchenko wrote:
> >> On 6/8/21 3:29 PM, Olivier Matz wrote:
> >>> Hi Ferruh, Andrew,
> >>>
> >>> On Tue, Jun 08, 2021 at 01:13:59PM +0300, Andrew Rybchenko wrote:
> >>>> On 4/30/21 5:48 PM, Ferruh Yigit wrote:
> >>>>> On 4/27/2021 2:57 PM, Olivier Matz wrote:
> >>>>>> When packet type is IPV4_EXT, the checksum is always marked as good in
> >>>>>> the mbuf offload flags.
> >>>>>>
> >>>>>> Since we know the header lengths, we can easily call
> >>>>>> rte_ipv4_udptcp_cksum() in this case too.
> >>>>>>
> >>>>>> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
> >>>>>> Cc: stable@dpdk.org
> >>>>>>
> >>>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >>>>>> ---
> >>>>>>  drivers/net/tap/rte_eth_tap.c | 4 ++--
> >>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> >>>>>> index 68baa18523..e7b185a4b5 100644
> >>>>>> --- a/drivers/net/tap/rte_eth_tap.c
> >>>>>> +++ b/drivers/net/tap/rte_eth_tap.c
> >>>>>> @@ -350,7 +350,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
> >>>>>>  		/* Don't verify checksum for multi-segment packets. */
> >>>>>>  		if (mbuf->nb_segs > 1)
> >>>>>>  			return;
> >>>>>> -		if (l3 == RTE_PTYPE_L3_IPV4) {
> >>>>>> +		if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
> >>>>>
> >>>>> Should we take 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' into account?
> >>>>
> >>>> I think we should.
> >>>
> >>> I think 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN' cannot happen here:
> >>>
> >>> - mbuf->packet_type is generated by 
> >>
> >> (), which cannot
> >>>   return 'RTE_PTYPE_L3_IPV4_EXT_UNKNOWN'
> >>
> >> My question if it is guaranteed and the only possible branch.
> >> Can application set packet_type itself and do not call
> >> rte_net_get_ptype(). Yes, typically application knows
> >> if it has IPv4 options in the header or not, but theoretically
> >> could be unaware as well.
> > 
> > This function is called on the Rx path from pmd_rx_burst(), so
> > the application does not have access to the mbuf.
> > 
> > The software parser that sets the packet type returns either
> > RTE_PTYPE_L3_IPV4 if there is no option, or RTE_PTYPE_L3_IPV4_EXT
> > else. The value RTE_PTYPE_L3_IPV4_EXT_UNKNOWN is used by PMDs that don't
> > know if there are options.
> 
> I see. What I'm trying to say that there are non
> obvious assumptions here on rte_net_get_ptype()
> behaviour which can be changed. May be it makes
> sense to add comments here to highlight it.

Ok, I'll add some words about it.

Thanks!

> 
> > 
> >>> - right above this code, we already returned if l3 is not in
> >>>   (RTE_PTYPE_L3_IPV4, RTE_PTYPE_L3_IPV4_EXT, RTE_PTYPE_L3_IPV6)
> >>
> >> If so, it sounds like it should be allowed above as well.
> >>
> >>>
> >>>>>
> >>>>>>  			if (l4 == RTE_PTYPE_L4_UDP) {
> >>>>>>  				udp_hdr = (struct rte_udp_hdr *)l4_hdr;
> >>>>>>  				if (udp_hdr->dgram_cksum == 0) {
> >>>>>> @@ -364,7 +364,7 @@ tap_verify_csum(struct rte_mbuf *mbuf)
> >>>>>>  				}
> >>>>>>  			}
> >>>>>>  			cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
> >>>>>> -		} else if (l3 == RTE_PTYPE_L3_IPV6) {
> >>>>>> +		} else { /* l3 == RTE_PTYPE_L3_IPV6, checked above */
> >>>>>>  			cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
> >>>>>>  		}
> >>>>>>  		mbuf->ol_flags |= cksum ?
> >>>>>>
> >>>>
> >>
>
diff mbox series

Patch

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 68baa18523..e7b185a4b5 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -350,7 +350,7 @@  tap_verify_csum(struct rte_mbuf *mbuf)
 		/* Don't verify checksum for multi-segment packets. */
 		if (mbuf->nb_segs > 1)
 			return;
-		if (l3 == RTE_PTYPE_L3_IPV4) {
+		if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
 			if (l4 == RTE_PTYPE_L4_UDP) {
 				udp_hdr = (struct rte_udp_hdr *)l4_hdr;
 				if (udp_hdr->dgram_cksum == 0) {
@@ -364,7 +364,7 @@  tap_verify_csum(struct rte_mbuf *mbuf)
 				}
 			}
 			cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr);
-		} else if (l3 == RTE_PTYPE_L3_IPV6) {
+		} else { /* l3 == RTE_PTYPE_L3_IPV6, checked above */
 			cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr);
 		}
 		mbuf->ol_flags |= cksum ?