diff mbox series

net/ice: fix error forwarding IPv6 VXLAN packet

Message ID 20211208095626.85026-1-kevinx.liu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series net/ice: fix error forwarding IPv6 VXLAN packet |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Kevin Liu Dec. 8, 2021, 9:56 a.m. UTC 
  In ice_txd_enable_offload(), when set tunnel packet Tx checksum
offload enable, td_offset should be set with outer l2/l3 len instead
of inner l2/l3 len.

This patch fix the bug that the checksum engine can forward tunnle
packets.

Fixes: 28f9002ab67f ("net/ice: add Tx AVX512 offload path")

Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 drivers/net/ice/ice_rxtx_vec_common.h | 52 +++++++++++++++++++--------
 1 file changed, 37 insertions(+), 15 deletions(-)

Comments

Qi Zhang Jan. 2, 2022, 8:46 a.m. UTC | #1 
> -----Original Message-----
> From: Kevin Liu <kevinx.liu@intel.com>
> Sent: Wednesday, December 8, 2021 5:56 PM
> To: dev@dpdk.org
> Cc: Zhang, RobinX <robinx.zhang@intel.com>; Wang, Jie1X
> <jie1x.wang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: [PATCH] net/ice: fix error forwarding IPv6 VXLAN packet
> 
> In ice_txd_enable_offload(), when set tunnel packet Tx checksum offload
> enable, td_offset should be set with outer l2/l3 len instead of inner l2/l3 len.
> 
> This patch fix the bug that the checksum engine can forward tunnle packets.

s/tunnle/tunnel

> 
> Fixes: 28f9002ab67f ("net/ice: add Tx AVX512 offload path")
> 
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---
>  drivers/net/ice/ice_rxtx_vec_common.h | 52 +++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_rxtx_vec_common.h
> b/drivers/net/ice/ice_rxtx_vec_common.h
> index dfe60c81d9..8ff01046e1 100644
> --- a/drivers/net/ice/ice_rxtx_vec_common.h
> +++ b/drivers/net/ice/ice_rxtx_vec_common.h
> @@ -364,23 +364,45 @@ ice_txd_enable_offload(struct rte_mbuf *tx_pkt,
>  	uint32_t td_offset = 0;
> 
>  	/* Tx Checksum Offload */
> -	/* SET MACLEN */
> -	td_offset |= (tx_pkt->l2_len >> 1) <<
> +	/*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->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 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;

Is this fix also cover IPv4 Tx checksum offload? 
Please refine the title to reflect what exactly the patch does if necessary.

Btw, could you also check if any fix needed in ice_txd_enable_checksum?
I saw inconsistent offset configure on l3_len between scaler path and vector path.
Its better to always keep them identical.
Kevin Liu Jan. 4, 2022, 8:35 a.m. UTC | #2 
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: 2022年1月2日 16:46
> To: Liu, KevinX <kevinx.liu@intel.com>; dev@dpdk.org
> Cc: Zhang, RobinX <robinx.zhang@intel.com>; Wang, Jie1X
> <jie1x.wang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: RE: [PATCH] net/ice: fix error forwarding IPv6 VXLAN packet
> 
> 
> 
> > -----Original Message-----
> > From: Kevin Liu <kevinx.liu@intel.com>
> > Sent: Wednesday, December 8, 2021 5:56 PM
> > To: dev@dpdk.org
> > Cc: Zhang, RobinX <robinx.zhang@intel.com>; Wang, Jie1X
> > <jie1x.wang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> > Subject: [PATCH] net/ice: fix error forwarding IPv6 VXLAN packet
> >
> > In ice_txd_enable_offload(), when set tunnel packet Tx checksum
> > offload enable, td_offset should be set with outer l2/l3 len instead of inner
> l2/l3 len.
> >
> > This patch fix the bug that the checksum engine can forward tunnle packets.
> 
> s/tunnle/tunnel
> 
> >
> > Fixes: 28f9002ab67f ("net/ice: add Tx AVX512 offload path")
> >
> > Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> > ---
> >  drivers/net/ice/ice_rxtx_vec_common.h | 52
> > +++++++++++++++++++--------
> >  1 file changed, 37 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_rxtx_vec_common.h
> > b/drivers/net/ice/ice_rxtx_vec_common.h
> > index dfe60c81d9..8ff01046e1 100644
> > --- a/drivers/net/ice/ice_rxtx_vec_common.h
> > +++ b/drivers/net/ice/ice_rxtx_vec_common.h
> > @@ -364,23 +364,45 @@ ice_txd_enable_offload(struct rte_mbuf *tx_pkt,
> > uint32_t td_offset = 0;
> >
> >  /* Tx Checksum Offload */
> > -/* SET MACLEN */
> > -td_offset |= (tx_pkt->l2_len >> 1) <<
> > +/*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->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 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;
> 
> Is this fix also cover IPv4 Tx checksum offload?
Yes, cover it.
> Please refine the title to reflect what exactly the patch does if necessary.
> 
> Btw, could you also check if any fix needed in ice_txd_enable_checksum?
> I saw inconsistent offset configure on l3_len between scaler path and vector
> path.
> Its better to always keep them identical.
> 
> 
I checked code in ice_txd_enable_checksum, and it should also be fixed here. I will send a new patch of v2.

Thanks
Kevin
diff mbox series

Patch

diff --git a/drivers/net/ice/ice_rxtx_vec_common.h b/drivers/net/ice/ice_rxtx_vec_common.h
index dfe60c81d9..8ff01046e1 100644
--- a/drivers/net/ice/ice_rxtx_vec_common.h
+++ b/drivers/net/ice/ice_rxtx_vec_common.h
@@ -364,23 +364,45 @@  ice_txd_enable_offload(struct rte_mbuf *tx_pkt,
 	uint32_t td_offset = 0;
 
 	/* Tx Checksum Offload */
-	/* SET MACLEN */
-	td_offset |= (tx_pkt->l2_len >> 1) <<
+	/*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->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 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;
+		}
 	}
 
 	/* Enable L4 checksum offloads */