diff mbox series

net/ice: fix Tx offload path choice

Message ID 20211224150925.3296471-1-kevinx.liu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series net/ice: fix Tx offload path choice |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Kevin Liu Dec. 24, 2021, 3:09 p.m. UTC 
  Testpmd forwards packets in checksum mode that it needs to calculate
the checksum of each layer's protocol.

When setting the hardware calculates the outer UDP checksum and the
software calculates the outer IP checksum, the dev->tx_pkt_burst in
ice_set_tx_function is set to ice_xmit_pkts_vec_avx2.
The inner and outer UDP checksum of the tunnel packet after forwarding
is wrong.The dev->tx_pkt_burst should be set to ice_xmit_pkts.

The patch adds RTE_ETH_TX_OFFLOAD_OUTER_UDP_CKSUM to
ICE_TX_NO_VECTOR_FLAGS,set dev->tx_pkt_burst to ice_xmit_pkts.After
the tunnel packet is forwarded, the inner and outer UDP checksum is
correct.

At the same time, the patch of "net/ice: fix Tx Checksum offload" will
cause interrupt errors in a special case that only inner IP and inner
UDP checksum are set for hardware calculation.The patch is updating
ICE_TX_NO_VECTOR_FLAGS, the problem can be solved, so I will restore the
code modification of that patch.

Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-segments")
Fixes: 28f9002ab67f ("net/ice: add Tx AVX512 offload path")
Fixes: 295968d17407 ("ethdev: add namespace")
Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
Cc: stable@dpdk.org

Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 app/test-pmd/csumonly.c               |  6 +--
 drivers/net/ice/ice_rxtx.c            | 41 ++++++-------------
 drivers/net/ice/ice_rxtx_vec_common.h | 59 +++++++++------------------
 3 files changed, 34 insertions(+), 72 deletions(-)

Comments

Ling, WeiX Feb. 24, 2022, 2:32 a.m. UTC | #1 
> -----Original Message-----
> From: Kevin Liu <kevinx.liu@intel.com>
> Sent: Friday, December 24, 2021 11:09 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] net/ice: fix Tx offload path choice
> 
> Testpmd forwards packets in checksum mode that it needs to calculate the
> checksum of each layer's protocol.
> 
> When setting the hardware calculates the outer UDP checksum and the
> software calculates the outer IP checksum, the dev->tx_pkt_burst in
> ice_set_tx_function is set to ice_xmit_pkts_vec_avx2.
> The inner and outer UDP checksum of the tunnel packet after forwarding is
> wrong.The dev->tx_pkt_burst should be set to ice_xmit_pkts.
> 
> The patch adds RTE_ETH_TX_OFFLOAD_OUTER_UDP_CKSUM to
> ICE_TX_NO_VECTOR_FLAGS,set dev->tx_pkt_burst to ice_xmit_pkts.After
> the tunnel packet is forwarded, the inner and outer UDP checksum is correct.
> 
> At the same time, the patch of "net/ice: fix Tx Checksum offload" will cause
> interrupt errors in a special case that only inner IP and inner UDP checksum
> are set for hardware calculation.The patch is updating
> ICE_TX_NO_VECTOR_FLAGS, the problem can be solved, so I will restore the
> code modification of that patch.
> 
> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
> segments")
> Fixes: 28f9002ab67f ("net/ice: add Tx AVX512 offload path")
> Fixes: 295968d17407 ("ethdev: add namespace")
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---

Tested-by: Wei Ling <weix.ling@intel.com>
Qi Zhang Feb. 27, 2022, 6:19 a.m. UTC | #2 
> -----Original Message-----
> From: Liu, KevinX <kevinx.liu@intel.com>
> Sent: Friday, December 24, 2021 11:09 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>;
> Liu, KevinX <kevinx.liu@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/ice: fix Tx offload path choice
> 
> Testpmd forwards packets in checksum mode that it needs to calculate the
> checksum of each layer's protocol.
> 
> When setting the hardware calculates the outer UDP checksum and the
> software calculates the outer IP checksum, the dev->tx_pkt_burst in
> ice_set_tx_function is set to ice_xmit_pkts_vec_avx2.
> The inner and outer UDP checksum of the tunnel packet after forwarding is
> wrong.The dev->tx_pkt_burst should be set to ice_xmit_pkts.
> 
> The patch adds RTE_ETH_TX_OFFLOAD_OUTER_UDP_CKSUM to
> ICE_TX_NO_VECTOR_FLAGS,set dev->tx_pkt_burst to ice_xmit_pkts.After the
> tunnel packet is forwarded, the inner and outer UDP checksum is correct.
> 
> At the same time, the patch of "net/ice: fix Tx Checksum offload" will cause
> interrupt errors in a special case that only inner IP and inner UDP checksum are
> set for hardware calculation.The patch is updating ICE_TX_NO_VECTOR_FLAGS,
> the problem can be solved, so I will restore the code modification of that patch.
> 
> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-segments")
> Fixes: 28f9002ab67f ("net/ice: add Tx AVX512 offload path")
> Fixes: 295968d17407 ("ethdev: add namespace")
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---
>  app/test-pmd/csumonly.c               |  6 +--
>  drivers/net/ice/ice_rxtx.c            | 41 ++++++-------------
>  drivers/net/ice/ice_rxtx_vec_common.h | 59 +++++++++------------------
>  3 files changed, 34 insertions(+), 72 deletions(-)

Please separate testpmd fix and pmd fix into two patches.
diff mbox series

Patch

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 02bc3929c7..c235456e58 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -513,7 +513,7 @@  process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 				ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;
 			} else {
 				if (info->is_tunnel)
-					l4_off = info->l2_len +
+					l4_off = info->outer_l2_len +
 						 info->outer_l3_len +
 						 info->l2_len + info->l3_len;
 				else
@@ -536,7 +536,7 @@  process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 			ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
 		} else {
 			if (info->is_tunnel)
-				l4_off = info->l2_len + info->outer_l3_len +
+				l4_off = info->outer_l2_len + info->outer_l3_len +
 					 info->l2_len + info->l3_len;
 			else
 				l4_off = info->l2_len + info->l3_len;
@@ -625,7 +625,7 @@  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
 		udp_hdr->dgram_cksum = get_udptcp_checksum(m, outer_l3_hdr,
-					info->l2_len + info->outer_l3_len,
+					info->outer_l2_len + info->outer_l3_len,
 					info->outer_ethertype);
 	}
 
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 4f218bcd0d..041f4bc91f 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2501,35 +2501,18 @@  ice_txd_enable_checksum(uint64_t ol_flags,
 			<< ICE_TX_DESC_LEN_MACLEN_S;
 
 	/* Enable L3 checksum offloads */
-	/*Tunnel package usage outer len enable L3 checksum offload*/
-	if (ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) {
-		if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) {
-			*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4_CSUM;
-			*td_offset |= (tx_offload.outer_l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		} else if (ol_flags & RTE_MBUF_F_TX_IPV4) {
-			*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4;
-			*td_offset |= (tx_offload.outer_l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		} else if (ol_flags & RTE_MBUF_F_TX_IPV6) {
-			*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV6;
-			*td_offset |= (tx_offload.outer_l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		}
-	} else {
-		if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) {
-			*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4_CSUM;
-			*td_offset |= (tx_offload.l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		} else if (ol_flags & RTE_MBUF_F_TX_IPV4) {
-			*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4;
-			*td_offset |= (tx_offload.l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		} else if (ol_flags & RTE_MBUF_F_TX_IPV6) {
-			*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV6;
-			*td_offset |= (tx_offload.l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		}
+	if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) {
+		*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4_CSUM;
+		*td_offset |= (tx_offload.l3_len >> 2) <<
+			ICE_TX_DESC_LEN_IPLEN_S;
+	} else if (ol_flags & RTE_MBUF_F_TX_IPV4) {
+		*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4;
+		*td_offset |= (tx_offload.l3_len >> 2) <<
+			ICE_TX_DESC_LEN_IPLEN_S;
+	} else if (ol_flags & RTE_MBUF_F_TX_IPV6) {
+		*td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV6;
+		*td_offset |= (tx_offload.l3_len >> 2) <<
+			ICE_TX_DESC_LEN_IPLEN_S;
 	}
 
 	if (ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
diff --git a/drivers/net/ice/ice_rxtx_vec_common.h b/drivers/net/ice/ice_rxtx_vec_common.h
index 8ff01046e1..2dd2d83650 100644
--- a/drivers/net/ice/ice_rxtx_vec_common.h
+++ b/drivers/net/ice/ice_rxtx_vec_common.h
@@ -250,7 +250,8 @@  ice_rxq_vec_setup_default(struct ice_rx_queue *rxq)
 #define ICE_TX_NO_VECTOR_FLAGS (			\
 		RTE_ETH_TX_OFFLOAD_MULTI_SEGS |		\
 		RTE_ETH_TX_OFFLOAD_OUTER_IPV4_CKSUM |	\
-		RTE_ETH_TX_OFFLOAD_TCP_TSO)
+		RTE_ETH_TX_OFFLOAD_TCP_TSO |	\
+		RTE_ETH_TX_OFFLOAD_OUTER_UDP_CKSUM)
 
 #define ICE_TX_VECTOR_OFFLOAD (				\
 		RTE_ETH_TX_OFFLOAD_VLAN_INSERT |		\
@@ -364,45 +365,23 @@  ice_txd_enable_offload(struct rte_mbuf *tx_pkt,
 	uint32_t td_offset = 0;
 
 	/* Tx Checksum Offload */
-	/*Tunnel package usage outer len enable L2/L3 checksum offload*/
-	if (ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) {
-		/* SET MACLEN */
-		td_offset |= (tx_pkt->outer_l2_len >> 1) <<
-			ICE_TX_DESC_LEN_MACLEN_S;
-
-		/* Enable L3 checksum offload */
-		if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) {
-			td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4_CSUM;
-			td_offset |= (tx_pkt->outer_l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		} else if (ol_flags & RTE_MBUF_F_TX_IPV4) {
-			td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4;
-			td_offset |= (tx_pkt->outer_l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		} else if (ol_flags & RTE_MBUF_F_TX_IPV6) {
-			td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV6;
-			td_offset |= (tx_pkt->outer_l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		}
-	} else {
-		/* SET MACLEN */
-		td_offset |= (tx_pkt->l2_len >> 1) <<
-			ICE_TX_DESC_LEN_MACLEN_S;
-
-		/* Enable L3 checksum offload */
-		if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) {
-			td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4_CSUM;
-			td_offset |= (tx_pkt->l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		} else if (ol_flags & RTE_MBUF_F_TX_IPV4) {
-			td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4;
-			td_offset |= (tx_pkt->l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		} else if (ol_flags & RTE_MBUF_F_TX_IPV6) {
-			td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV6;
-			td_offset |= (tx_pkt->l3_len >> 2) <<
-				ICE_TX_DESC_LEN_IPLEN_S;
-		}
+	/* SET MACLEN */
+	td_offset |= (tx_pkt->l2_len >> 1) <<
+		ICE_TX_DESC_LEN_MACLEN_S;
+
+	/* Enable L3 checksum offload */
+	if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) {
+		td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4_CSUM;
+		td_offset |= (tx_pkt->l3_len >> 2) <<
+			ICE_TX_DESC_LEN_IPLEN_S;
+	} else if (ol_flags & RTE_MBUF_F_TX_IPV4) {
+		td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4;
+		td_offset |= (tx_pkt->l3_len >> 2) <<
+			ICE_TX_DESC_LEN_IPLEN_S;
+	} else if (ol_flags & RTE_MBUF_F_TX_IPV6) {
+		td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV6;
+		td_offset |= (tx_pkt->l3_len >> 2) <<
+			ICE_TX_DESC_LEN_IPLEN_S;
 	}
 
 	/* Enable L4 checksum offloads */