[PATCH] app/testpmd: fix IPv6 tunnel packet checksum error

[PATCH] app/testpmd: fix IPv6 tunnel packet checksum error

[Shiyang He]

In checksum forwarding mode, the checksum of tunnel packet calculated
incorrectly when outer header is IPv6.

This pathch fixes the issue by setting L4 checksum flag.

Fixes: daa02b5cddbb ("mbuf: add namespace to offload flags")
Cc: stable@dpdk.org

Signed-off-by: Shiyang He <shiyangx.he@intel.com>
---
 app/test-pmd/csumonly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index fc85c22a77..bd2fccc458 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -582,7 +582,7 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 		else
 			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
 	} else
-		ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6;
+		ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6 | RTE_MBUF_F_TX_L4_MASK;
 
 	if (info->outer_l4_proto != IPPROTO_UDP)
 		return ol_flags;
-- 
2.37.2

RE: [PATCH] app/testpmd: fix IPv6 tunnel packet checksum error

[He, ShiyangX]

>-----Original Message-----
>From: Ferruh Yigit <ferruh.yigit@amd.com>
>Sent: Friday, June 2, 2023 6:10 AM
>To: He, ShiyangX <shiyangx.he@intel.com>; dev@dpdk.org
>Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org; Singh, Aman
>Deep <aman.deep.singh@intel.com>; Zhang, Yuying
><yuying.zhang@intel.com>; Somnath Kotur
><somnath.kotur@broadcom.com>; Andrew Rybchenko
><andrew.rybchenko@oktetlabs.ru>; Matz, Olivier <olivier.matz@6wind.com>;
>Ajit Khaparde <ajit.khaparde@broadcom.com>
>Subject: Re: [PATCH] app/testpmd: fix IPv6 tunnel packet checksum error
>
>On 5/30/2023 4:36 PM, Shiyang He wrote:
>> In checksum forwarding mode, the checksum of tunnel packet calculated
>> incorrectly when outer header is IPv6.
>>
>> This patch fixes the issue by setting L4 checksum flag.
>>
>> Fixes: daa02b5cddbb ("mbuf: add namespace to offload flags")
>
>This commit just updates the flag name, actual commit that introduce the
>commit should be something else.
>
>Probably:
>Fixes: c14236f210d8 ("mbuf: replace inner fields by outer fields semantic")
>Cc: jijiang.liu@intel.com
>
>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Shiyang He <shiyangx.he@intel.com>
>> ---
>>  app/test-pmd/csumonly.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
>> fc85c22a77..bd2fccc458 100644
>> --- a/app/test-pmd/csumonly.c
>> +++ b/app/test-pmd/csumonly.c
>> @@ -582,7 +582,7 @@ process_outer_cksums(void *outer_l3_hdr, struct
>testpmd_offload_info *info,
>>  		else
>>  			ipv4_hdr->hdr_checksum =
>rte_ipv4_cksum(ipv4_hdr);
>>  	} else
>> -		ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6;
>> +		ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6 |
>RTE_MBUF_F_TX_L4_MASK;
>>
>
>When 'or' with MASK, it is same as requesting UDP checksum.
>```
>#define RTE_MBUF_F_TX_UDP_CKSUM     (3ULL << 52)
>#define RTE_MBUF_F_TX_L4_MASK       (3ULL << 52)
>```
>
>Can you please describe more the problem, is it UDP checksum missing (since
>ipv6 itself doesn't have checksum)?
>If so, shouldn't code check first that if UDP checksum offload is requested?
>Can you please describe why/how above code change is fixing the issue?

Sorry for the late reply!
I re-debugged the issue again and found that it was caused by PMD.
I will resubmit the v2 patch!

Re: [PATCH] app/testpmd: fix IPv6 tunnel packet checksum error

[Ferruh Yigit]

On 5/30/2023 4:36 PM, Shiyang He wrote:
> In checksum forwarding mode, the checksum of tunnel packet calculated
> incorrectly when outer header is IPv6.
> 
> This patch fixes the issue by setting L4 checksum flag.
> 
> Fixes: daa02b5cddbb ("mbuf: add namespace to offload flags")

This commit just updates the flag name, actual commit that introduce the
commit should be something else.

Probably:
Fixes: c14236f210d8 ("mbuf: replace inner fields by outer fields semantic")
Cc: jijiang.liu@intel.com


> Cc: stable@dpdk.org
> 
> Signed-off-by: Shiyang He <shiyangx.he@intel.com>
> ---
>  app/test-pmd/csumonly.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index fc85c22a77..bd2fccc458 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -582,7 +582,7 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
>  		else
>  			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
>  	} else
> -		ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6;
> +		ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6 | RTE_MBUF_F_TX_L4_MASK;
>  

When 'or' with MASK, it is same as requesting UDP checksum.
```
#define RTE_MBUF_F_TX_UDP_CKSUM     (3ULL << 52)
#define RTE_MBUF_F_TX_L4_MASK       (3ULL << 52)
```

Can you please describe more the problem, is it UDP checksum missing
(since ipv6 itself doesn't have checksum)?
If so, shouldn't code check first that if UDP checksum offload is requested?
Can you please describe why/how above code change is fixing the issue?

[PATCH] app/testpmd: fix IPv6 tunnel packet checksum error

[Shiyang He]

In checksum forwarding mode, the checksum of tunnel packet calculated
incorrectly when outer header is IPv6.

This patch fixes the issue by setting L4 checksum flag.

Fixes: daa02b5cddbb ("mbuf: add namespace to offload flags")
Cc: stable@dpdk.org

Signed-off-by: Shiyang He <shiyangx.he@intel.com>
---
 app/test-pmd/csumonly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index fc85c22a77..bd2fccc458 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -582,7 +582,7 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 		else
 			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
 	} else
-		ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6;
+		ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6 | RTE_MBUF_F_TX_L4_MASK;
 
 	if (info->outer_l4_proto != IPPROTO_UDP)
 		return ol_flags;
-- 
2.37.2