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

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?

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!

[PATCH v2] net/ice:fix tunnel packet TX descriptor error

[Shiyang He]

The TX descriptor of tunnel packet filled incorrectly due to the MACLEN
is not set.

This patch fixes this issue by setting MACLEN to correctly fill the
TX descriptor.

Fixes: bd70c451532c ("net/ice: support Tx checksum offload for tunnel")
Cc: stable@dpdk.org

Signed-off-by: Shiyang He <shiyangx.he@intel.com>
---
 drivers/net/ice/ice_rxtx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 0ea0045836..3af552f3e1 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2738,10 +2738,7 @@ ice_txd_enable_checksum(uint64_t ol_flags,
 			union ice_tx_offload tx_offload)
 {
 	/* Set MACLEN */
-	if (ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
-		*td_offset |= (tx_offload.outer_l2_len >> 1)
-			<< ICE_TX_DESC_LEN_MACLEN_S;
-	else
+	if (!(ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK))
 		*td_offset |= (tx_offload.l2_len >> 1)
 			<< ICE_TX_DESC_LEN_MACLEN_S;
 
@@ -3002,9 +2999,12 @@ ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 
 		/* Fill in tunneling parameters if necessary */
 		cd_tunneling_params = 0;
-		if (ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
+		if (ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) {
+			td_offset |= (tx_offload.outer_l2_len >> 1)
+				<< ICE_TX_DESC_LEN_MACLEN_S;
 			ice_parse_tunneling_params(ol_flags, tx_offload,
 						   &cd_tunneling_params);
+		}
 
 		/* Enable checksum offloading */
 		if (ol_flags & ICE_TX_CKSUM_OFFLOAD_MASK)
-- 
2.37.2

RE: [PATCH v2] net/ice:fix tunnel packet TX descriptor error

[Zhang, Qi Z]

> -----Original Message-----
> From: He, ShiyangX <shiyangx.he@intel.com>
> Sent: Thursday, June 8, 2023 6:19 PM
> To: dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; He, ShiyangX
> <shiyangx.he@intel.com>; stable@dpdk.org; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Subject: [PATCH v2] net/ice:fix tunnel packet TX descriptor error

Please check the title format next time.

> 
> The TX descriptor of tunnel packet filled incorrectly due to the MACLEN is
> not set.
> 
> This patch fixes this issue by setting MACLEN to correctly fill the TX
> descriptor.
> 
> Fixes: bd70c451532c ("net/ice: support Tx checksum offload for tunnel")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shiyang He <shiyangx.he@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

RE: [PATCH v2] net/ice:fix tunnel packet TX descriptor error

[Xu, Ke1]

> From: Shiyang He <shiyangx.he@intel.com>
> Sent: Thursday, June 8, 2023 6:19 PM
> To: dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; He, ShiyangX
> <shiyangx.he@intel.com>; stable@dpdk.org; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Subject: [PATCH v2] net/ice:fix tunnel packet TX descriptor error
>
> The TX descriptor of tunnel packet filled incorrectly due to the MACLEN is not
> set.
>
> This patch fixes this issue by setting MACLEN to correctly fill the TX descriptor.
>
> Fixes: bd70c451532c ("net/ice: support Tx checksum offload for tunnel")
> Cc: stable@dpdk.org
>
> Signed-off-by: Shiyang He <shiyangx.he@intel.com>

Verified passed.

Tested-by: Ke Xu <ke1.xu@intel.com>

> ---
>  drivers/net/ice/ice_rxtx.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>