patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH] net/ice: fix scalar Rx and Tx path segment
@ 2022-11-03 17:20 Mingjin Ye
  2022-11-04  6:57 ` Xu, Ke1
  2022-11-09 12:56 ` [PATCH v2] " Mingjin Ye
  0 siblings, 2 replies; 22+ messages in thread
From: Mingjin Ye @ 2022-11-03 17:20 UTC (permalink / raw)
  To: dev
  Cc: stable, yidingx.zhou, Mingjin Ye, Qiming Yang, Qi Zhang,
	Jingjing Wu, Wenzhuo Lu, Ferruh Yigit, Xiaoyun Li

CRC is stripped by the hardware in the scattered Rx path. If the last
buffer packet length is '0', the scalar Tx path would send empty buffer
that causes the Tx queue to overflow.

This patch adds a judgment for the last buffer length to fix this issue,
so that it would free the mbuf associated to the last one if the last
buffer is empty.

Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 drivers/net/ice/ice_rxtx.c | 53 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 0a2b0376ac..4f9c29eaf6 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
 			} else
 				rxm->data_len = (uint16_t)(rx_packet_len -
 							   RTE_ETHER_CRC_LEN);
+		} else if (rx_packet_len == 0) {
+			rte_pktmbuf_free_seg(rxm);
+			first_seg->nb_segs--;
+			last_seg->next = NULL;
 		}
 
 		first_seg->port = rxq->port_id;
@@ -2903,6 +2907,35 @@ ice_calc_pkt_desc(struct rte_mbuf *tx_pkt)
 	return count;
 }
 
+/*Check the number of valid mbufs and free the invalid mbufs*/
+static inline uint16_t
+ice_check_mbuf(struct rte_mbuf *tx_pkt)
+{
+	struct rte_mbuf *txd = tx_pkt;
+	struct rte_mbuf *txd_removal = NULL;
+	struct rte_mbuf *txd_pre = NULL;
+	uint16_t count = 0;
+	uint16_t removal = 0;
+
+	while (txd != NULL) {
+		if (removal == 1 || txd->data_len == 0) {
+			txd_removal = txd;
+			txd = txd->next;
+			if (removal == 0) {
+				removal = 1;
+				txd_pre->next = NULL;
+			}
+			rte_pktmbuf_free_seg(txd_removal);
+		} else {
+			++count;
+			txd_pre = txd;
+			txd = txd->next;
+		}
+	}
+
+	return count;
+}
+
 uint16_t
 ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 {
@@ -2960,11 +2993,27 @@ ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		 * the mbuf data size exceeds max data size that hw allows
 		 * per tx desc.
 		 */
-		if (ol_flags & RTE_MBUF_F_TX_TCP_SEG)
+		if (ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
 			nb_used = (uint16_t)(ice_calc_pkt_desc(tx_pkt) +
 					     nb_ctx);
-		else
+		} else {
+			nb_used = ice_check_mbuf(tx_pkt);
+			if (nb_used == 0) {
+				PMD_TX_LOG(ERR,
+				"Check packets is empty "
+				"(port=%d queue=%d)\n",
+				txq->port_id, txq->queue_id);
+				continue;
+			} else if (nb_used < tx_pkt->nb_segs) {
+				PMD_TX_LOG(WRINING,
+				"Check packets valid num ="
+				"%4u total num = %4u (port=%d queue=%d)\n",
+				nb_used, tx_pkt->nb_segs, txq->port_id, txq->queue_id);
+				tx_pkt->nb_segs = nb_used;
+			}
 			nb_used = (uint16_t)(tx_pkt->nb_segs + nb_ctx);
+		}
+
 		tx_last = (uint16_t)(tx_id + nb_used - 1);
 
 		/* Circular ring */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH] net/ice: fix scalar Rx and Tx path segment
  2022-11-03 17:20 [PATCH] net/ice: fix scalar Rx and Tx path segment Mingjin Ye
@ 2022-11-04  6:57 ` Xu, Ke1
  2022-11-09 12:56 ` [PATCH v2] " Mingjin Ye
  1 sibling, 0 replies; 22+ messages in thread
From: Xu, Ke1 @ 2022-11-04  6:57 UTC (permalink / raw)
  To: Ye, MingjinX, dev
  Cc: stable, Zhou, YidingX, Ye, MingjinX, Yang, Qiming, Zhang, Qi Z,
	Wu, Jingjing, Lu, Wenzhuo, Ferruh Yigit, Li, Xiaoyun


> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Friday, November 4, 2022 1:21 AM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>; Ye, MingjinX
> <mingjinx.ye@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH] net/ice: fix scalar Rx and Tx path segment
> 
> CRC is stripped by the hardware in the scattered Rx path. If the last buffer
> packet length is '0', the scalar Tx path would send empty buffer that causes
> the Tx queue to overflow.
> 
> This patch adds a judgment for the last buffer length to fix this issue, so that
> it would free the mbuf associated to the last one if the last buffer is empty.
> 
> Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>


V1 patch verified and passed. Remains some compile errors to fix for redhat.

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


> ---
>  drivers/net/ice/ice_rxtx.c | 53 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2] net/ice: fix scalar Rx and Tx path segment
  2022-11-03 17:20 [PATCH] net/ice: fix scalar Rx and Tx path segment Mingjin Ye
  2022-11-04  6:57 ` Xu, Ke1
@ 2022-11-09 12:56 ` Mingjin Ye
  2022-11-10  2:01   ` Xu, Ke1
                     ` (2 more replies)
  1 sibling, 3 replies; 22+ messages in thread
From: Mingjin Ye @ 2022-11-09 12:56 UTC (permalink / raw)
  To: dev
  Cc: qiming.yang, stable, yidingx.zhou, Mingjin Ye, Qi Zhang,
	Wenzhuo Lu, Jingjing Wu, Xiaoyun Li, Ferruh Yigit

CRC is stripped by the hardware in the scattered Rx path. If the last
buffer packet length is '0', the scalar Tx path would send empty buffer
that causes the Tx queue to overflow.

This patch adds a judgment for the last buffer length to fix this issue,
so that it would free the mbuf associated to the last one if the last
buffer is empty.

Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>

v2:
	* Fix log level in ice_rxtx.c source file.
---
 drivers/net/ice/ice_rxtx.c | 53 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 0a2b0376ac..b181f66aad 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
 			} else
 				rxm->data_len = (uint16_t)(rx_packet_len -
 							   RTE_ETHER_CRC_LEN);
+		} else if (rx_packet_len == 0) {
+			rte_pktmbuf_free_seg(rxm);
+			first_seg->nb_segs--;
+			last_seg->next = NULL;
 		}
 
 		first_seg->port = rxq->port_id;
@@ -2903,6 +2907,35 @@ ice_calc_pkt_desc(struct rte_mbuf *tx_pkt)
 	return count;
 }
 
+/*Check the number of valid mbufs and free the invalid mbufs*/
+static inline uint16_t
+ice_check_mbuf(struct rte_mbuf *tx_pkt)
+{
+	struct rte_mbuf *txd = tx_pkt;
+	struct rte_mbuf *txd_removal = NULL;
+	struct rte_mbuf *txd_pre = NULL;
+	uint16_t count = 0;
+	uint16_t removal = 0;
+
+	while (txd != NULL) {
+		if (removal == 1 || txd->data_len == 0) {
+			txd_removal = txd;
+			txd = txd->next;
+			if (removal == 0) {
+				removal = 1;
+				txd_pre->next = NULL;
+			}
+			rte_pktmbuf_free_seg(txd_removal);
+		} else {
+			++count;
+			txd_pre = txd;
+			txd = txd->next;
+		}
+	}
+
+	return count;
+}
+
 uint16_t
 ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 {
@@ -2960,11 +2993,27 @@ ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		 * the mbuf data size exceeds max data size that hw allows
 		 * per tx desc.
 		 */
-		if (ol_flags & RTE_MBUF_F_TX_TCP_SEG)
+		if (ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
 			nb_used = (uint16_t)(ice_calc_pkt_desc(tx_pkt) +
 					     nb_ctx);
-		else
+		} else {
+			nb_used = ice_check_mbuf(tx_pkt);
+			if (nb_used == 0) {
+				PMD_TX_LOG(ERR,
+				"Check packets is empty "
+				"(port=%d queue=%d)\n",
+				txq->port_id, txq->queue_id);
+				continue;
+			} else if (nb_used < tx_pkt->nb_segs) {
+				PMD_TX_LOG(DEBUG,
+				"Check packets valid num ="
+				"%4u total num = %4u (port=%d queue=%d)\n",
+				nb_used, tx_pkt->nb_segs, txq->port_id, txq->queue_id);
+				tx_pkt->nb_segs = nb_used;
+			}
 			nb_used = (uint16_t)(tx_pkt->nb_segs + nb_ctx);
+		}
+
 		tx_last = (uint16_t)(tx_id + nb_used - 1);
 
 		/* Circular ring */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
  2022-11-09 12:56 ` [PATCH v2] " Mingjin Ye
@ 2022-11-10  2:01   ` Xu, Ke1
  2022-11-10 10:37   ` Zhang, Qi Z
  2022-11-11 12:04   ` [PATCH v3 1/2] net/ice: fix scalar Rx " Mingjin Ye
  2 siblings, 0 replies; 22+ messages in thread
From: Xu, Ke1 @ 2022-11-10  2:01 UTC (permalink / raw)
  To: Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Ye, MingjinX, Zhang, Qi Z,
	Lu, Wenzhuo, Wu, Jingjing, Li, Xiaoyun, Ferruh Yigit

> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Wednesday, November 9, 2022 8:56 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Ferruh
> Yigit <ferruh.yigit@intel.com>
> Subject: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
> 
> CRC is stripped by the hardware in the scattered Rx path. If the last buffer
> packet length is '0', the scalar Tx path would send empty buffer that causes
> the Tx queue to overflow.
> 
> This patch adds a judgment for the last buffer length to fix this issue, so that
> it would free the mbuf associated to the last one if the last buffer is empty.
> 
> Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>

Tested and passed.

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


> 
> v2:
> 	* Fix log level in ice_rxtx.c source file.
> ---
>  drivers/net/ice/ice_rxtx.c | 53 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
  2022-11-09 12:56 ` [PATCH v2] " Mingjin Ye
  2022-11-10  2:01   ` Xu, Ke1
@ 2022-11-10 10:37   ` Zhang, Qi Z
  2022-11-11  3:12     ` Ye, MingjinX
  2022-11-11 12:04   ` [PATCH v3 1/2] net/ice: fix scalar Rx " Mingjin Ye
  2 siblings, 1 reply; 22+ messages in thread
From: Zhang, Qi Z @ 2022-11-10 10:37 UTC (permalink / raw)
  To: Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Lu, Wenzhuo, Wu, Jingjing,
	Li, Xiaoyun, Ferruh Yigit



> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Wednesday, November 9, 2022 8:56 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Ferruh
> Yigit <ferruh.yigit@intel.com>
> Subject: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
> 
> CRC is stripped by the hardware in the scattered Rx path. If the last buffer
> packet length is '0', the scalar Tx path would send empty buffer that causes
> the Tx queue to overflow.

Please separate this patch into two, one for Rx and one for Tx, they are independent.

For the Tx implementation, I think we can move them into tx_prepare where is place to check Tx violation.
 
> 
> This patch adds a judgment for the last buffer length to fix this issue, so that
> it would free the mbuf associated to the last one if the last buffer is empty.
> 
> Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> 
> v2:
> 	* Fix log level in ice_rxtx.c source file.
> ---
>  drivers/net/ice/ice_rxtx.c | 53 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c index
> 0a2b0376ac..b181f66aad 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
>  			} else
>  				rxm->data_len = (uint16_t)(rx_packet_len -
> 
> RTE_ETHER_CRC_LEN);
> +		} else if (rx_packet_len == 0) {
> +			rte_pktmbuf_free_seg(rxm);
> +			first_seg->nb_segs--;
> +			last_seg->next = NULL;
>  		}
> 
>  		first_seg->port = rxq->port_id;
> @@ -2903,6 +2907,35 @@ ice_calc_pkt_desc(struct rte_mbuf *tx_pkt)
>  	return count;
>  }
> 
> +/*Check the number of valid mbufs and free the invalid mbufs*/ static
> +inline uint16_t ice_check_mbuf(struct rte_mbuf *tx_pkt) {
> +	struct rte_mbuf *txd = tx_pkt;
> +	struct rte_mbuf *txd_removal = NULL;
> +	struct rte_mbuf *txd_pre = NULL;
> +	uint16_t count = 0;
> +	uint16_t removal = 0;
> +
> +	while (txd != NULL) {
> +		if (removal == 1 || txd->data_len == 0) {
> +			txd_removal = txd;
> +			txd = txd->next;
> +			if (removal == 0) {
> +				removal = 1;
> +				txd_pre->next = NULL;
> +			}
> +			rte_pktmbuf_free_seg(txd_removal);
> +		} else {
> +			++count;
> +			txd_pre = txd;
> +			txd = txd->next;
> +		}
> +	}
> +
> +	return count;
> +}
> +
>  uint16_t
>  ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> { @@ -2960,11 +2993,27 @@ ice_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts, uint16_t nb_pkts)
>  		 * the mbuf data size exceeds max data size that hw allows
>  		 * per tx desc.
>  		 */
> -		if (ol_flags & RTE_MBUF_F_TX_TCP_SEG)
> +		if (ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
>  			nb_used = (uint16_t)(ice_calc_pkt_desc(tx_pkt) +
>  					     nb_ctx);
> -		else
> +		} else {
> +			nb_used = ice_check_mbuf(tx_pkt);
> +			if (nb_used == 0) {
> +				PMD_TX_LOG(ERR,
> +				"Check packets is empty "
> +				"(port=%d queue=%d)\n",
> +				txq->port_id, txq->queue_id);
> +				continue;
> +			} else if (nb_used < tx_pkt->nb_segs) {
> +				PMD_TX_LOG(DEBUG,
> +				"Check packets valid num ="
> +				"%4u total num = %4u (port=%d
> queue=%d)\n",
> +				nb_used, tx_pkt->nb_segs, txq->port_id, txq-
> >queue_id);
> +				tx_pkt->nb_segs = nb_used;
> +			}
>  			nb_used = (uint16_t)(tx_pkt->nb_segs + nb_ctx);
> +		}
> +
>  		tx_last = (uint16_t)(tx_id + nb_used - 1);
> 
>  		/* Circular ring */
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
  2022-11-10 10:37   ` Zhang, Qi Z
@ 2022-11-11  3:12     ` Ye, MingjinX
  0 siblings, 0 replies; 22+ messages in thread
From: Ye, MingjinX @ 2022-11-11  3:12 UTC (permalink / raw)
  To: Zhang, Qi Z, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Lu, Wenzhuo, Wu, Jingjing,
	Li, Xiaoyun, Ferruh Yigit



> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: 2022年11月10日 18:37
> To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Ferruh
> Yigit <ferruh.yigit@intel.com>
> Subject: RE: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
> 
> 
> 
> > -----Original Message-----
> > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > Sent: Wednesday, November 9, 2022 8:56 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Ye, MingjinX
> > <mingjinx.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Lu,
> > Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > Li, Xiaoyun <xiaoyun.li@intel.com>; Ferruh Yigit
> > <ferruh.yigit@intel.com>
> > Subject: [PATCH v2] net/ice: fix scalar Rx and Tx path segment
> >
> > CRC is stripped by the hardware in the scattered Rx path. If the last
> > buffer packet length is '0', the scalar Tx path would send empty
> > buffer that causes the Tx queue to overflow.
> 
> Please separate this patch into two, one for Rx and one for Tx, they are
> independent.
> 
> For the Tx implementation, I think we can move them into tx_prepare where
> is place to check Tx violation.
Thanks for your suggestion, I will provide 2 new patches according to the new
scheme and promote it to the community.
> 
> >
> > This patch adds a judgment for the last buffer length to fix this
> > issue, so that it would free the mbuf associated to the last one if the last
> buffer is empty.
> >
> > Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> >
> > v2:
> > 	* Fix log level in ice_rxtx.c source file.
> > ---
> >  drivers/net/ice/ice_rxtx.c | 53
> > ++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > index 0a2b0376ac..b181f66aad 100644
> > --- a/drivers/net/ice/ice_rxtx.c
> > +++ b/drivers/net/ice/ice_rxtx.c
> > @@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
> >  			} else
> >  				rxm->data_len = (uint16_t)(rx_packet_len -
> >
> > RTE_ETHER_CRC_LEN);
> > +		} else if (rx_packet_len == 0) {
> > +			rte_pktmbuf_free_seg(rxm);
> > +			first_seg->nb_segs--;
> > +			last_seg->next = NULL;
> >  		}
> >
> >  		first_seg->port = rxq->port_id;
> > @@ -2903,6 +2907,35 @@ ice_calc_pkt_desc(struct rte_mbuf *tx_pkt)
> >  	return count;
> >  }
> >
> > +/*Check the number of valid mbufs and free the invalid mbufs*/ static
> > +inline uint16_t ice_check_mbuf(struct rte_mbuf *tx_pkt) {
> > +	struct rte_mbuf *txd = tx_pkt;
> > +	struct rte_mbuf *txd_removal = NULL;
> > +	struct rte_mbuf *txd_pre = NULL;
> > +	uint16_t count = 0;
> > +	uint16_t removal = 0;
> > +
> > +	while (txd != NULL) {
> > +		if (removal == 1 || txd->data_len == 0) {
> > +			txd_removal = txd;
> > +			txd = txd->next;
> > +			if (removal == 0) {
> > +				removal = 1;
> > +				txd_pre->next = NULL;
> > +			}
> > +			rte_pktmbuf_free_seg(txd_removal);
> > +		} else {
> > +			++count;
> > +			txd_pre = txd;
> > +			txd = txd->next;
> > +		}
> > +	}
> > +
> > +	return count;
> > +}
> > +
> >  uint16_t
> >  ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t
> > nb_pkts) { @@ -2960,11 +2993,27 @@ ice_xmit_pkts(void *tx_queue,
> > struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> >  		 * the mbuf data size exceeds max data size that hw allows
> >  		 * per tx desc.
> >  		 */
> > -		if (ol_flags & RTE_MBUF_F_TX_TCP_SEG)
> > +		if (ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
> >  			nb_used = (uint16_t)(ice_calc_pkt_desc(tx_pkt) +
> >  					     nb_ctx);
> > -		else
> > +		} else {
> > +			nb_used = ice_check_mbuf(tx_pkt);
> > +			if (nb_used == 0) {
> > +				PMD_TX_LOG(ERR,
> > +				"Check packets is empty "
> > +				"(port=%d queue=%d)\n",
> > +				txq->port_id, txq->queue_id);
> > +				continue;
> > +			} else if (nb_used < tx_pkt->nb_segs) {
> > +				PMD_TX_LOG(DEBUG,
> > +				"Check packets valid num ="
> > +				"%4u total num = %4u (port=%d
> > queue=%d)\n",
> > +				nb_used, tx_pkt->nb_segs, txq->port_id,
> txq-
> > >queue_id);
> > +				tx_pkt->nb_segs = nb_used;
> > +			}
> >  			nb_used = (uint16_t)(tx_pkt->nb_segs + nb_ctx);
> > +		}
> > +
> >  		tx_last = (uint16_t)(tx_id + nb_used - 1);
> >
> >  		/* Circular ring */
> > --
> > 2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v3 1/2] net/ice: fix scalar Rx path segment
  2022-11-11 12:04   ` [PATCH v3 1/2] net/ice: fix scalar Rx " Mingjin Ye
@ 2022-11-11  4:59     ` Zhang, Qi Z
  2022-11-11 12:04     ` [PATCH v3 2/2] net/ice: fix scalar Tx " Mingjin Ye
  1 sibling, 0 replies; 22+ messages in thread
From: Zhang, Qi Z @ 2022-11-11  4:59 UTC (permalink / raw)
  To: Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Ferruh Yigit, Wu, Jingjing,
	Lu, Wenzhuo, Li, Xiaoyun



> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Friday, November 11, 2022 8:04 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li, Xiaoyun
> <xiaoyun.li@intel.com>
> Subject: [PATCH v3 1/2] net/ice: fix scalar Rx path segment
> 
> CRC is stripped by the hardware in the scattered Rx path. The last buffer is
> invalid if it's packet length is zero.
> 
> This patch adds a judgment for the last buffer length to fix this issue, it would
> free the mbuf associated to the last one if the last buffer is empty.
> 
> Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>

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

Applied to dpdk-next-net-intel.

Thanks
Qi


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
  2022-11-11 12:04     ` [PATCH v3 2/2] net/ice: fix scalar Tx " Mingjin Ye
@ 2022-11-11  5:09       ` Zhang, Qi Z
  2022-11-11  8:30         ` Ye, MingjinX
  2022-11-11 16:01       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
  2022-11-11 16:12       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
  2 siblings, 1 reply; 22+ messages in thread
From: Zhang, Qi Z @ 2022-11-11  5:09 UTC (permalink / raw)
  To: Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Wu, Jingjing, Lu, Wenzhuo,
	Ferruh Yigit, Li, Xiaoyun, Liu, KevinX



> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Friday, November 11, 2022 8:04 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> 
> The scalar Tx path would send empty buffer that causes the Tx queue to
> overflow.
> 
> This patch adds the last buffer length judgment in tx_prepare to fix this issue,
> rte_errno will be set to EINVAL and returned if the last buffer is empty.
> 
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
>  drivers/net/ice/ice_rxtx.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c index
> e6ddd2513d..69358f6a3a 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev,
> struct ice_tx_queue *txq)
>  #define ICE_MIN_TSO_MSS            64
>  #define ICE_MAX_TSO_MSS            9728
>  #define ICE_MAX_TSO_FRAME_SIZE     262144
> +
> +/*Check for invalid mbuf*/
> +static inline uint16_t
> +ice_check_mbuf(struct rte_mbuf *tx_pkt) {

Better to name the function to exactly match what it does. 
e.g.: ice_check_emtpy_mbuf
and also declare it as inline.

> +	struct rte_mbuf *txd = tx_pkt;
> +
> +	while (txd != NULL) {
> +		if (txd->data_len == 0)
> +			return -1;
> +		txd = txd->next;
> +	}
> +
> +	return 0;
> +}
> +
>  uint16_t
>  ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
>  	      uint16_t nb_pkts)
> @@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> struct rte_mbuf **tx_pkts,
>  	struct ice_tx_queue *txq = tx_queue;
>  	struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
>  	uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
> +	uint16_t nb_used;
> 
>  	for (i = 0; i < nb_pkts; i++) {
>  		m = tx_pkts[i];
> @@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> struct rte_mbuf **tx_pkts,
>  			rte_errno = -ret;
>  			return i;
>  		}
> +
> +		if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
> +			ice_check_mbuf(m)) {

Why "!(ol_flags & RTE_MBUF_F_TX_TCP_SEG)" is needed here?
A empty mbuf with TSO enabled is still acceptable?

> +			rte_errno = EINVAL;
> +			PMD_DRV_LOG(ERR, "INVALID mbuf:	last mbuf
> data_len=[0]");
> +			return i;
> +		}
>  	}
>  	return i;
>  }
> --
> 2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
  2022-11-11  5:09       ` Zhang, Qi Z
@ 2022-11-11  8:30         ` Ye, MingjinX
  2022-11-11  8:45           ` Zhang, Qi Z
  0 siblings, 1 reply; 22+ messages in thread
From: Ye, MingjinX @ 2022-11-11  8:30 UTC (permalink / raw)
  To: Zhang, Qi Z, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Wu, Jingjing, Lu, Wenzhuo,
	Ferruh Yigit, Li, Xiaoyun, Liu, KevinX



> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: 2022年11月11日 13:09
> To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> 
> 
> 
> > -----Original Message-----
> > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > Sent: Friday, November 11, 2022 8:04 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Ye, MingjinX
> > <mingjinx.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> > Jingjing <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> > Ferruh Yigit <ferruh.yigit@intel.com>; Li, Xiaoyun
> > <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> > Subject: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> >
> > The scalar Tx path would send empty buffer that causes the Tx queue to
> > overflow.
> >
> > This patch adds the last buffer length judgment in tx_prepare to fix
> > this issue, rte_errno will be set to EINVAL and returned if the last buffer is
> empty.
> >
> > Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> > Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > ---
> >  drivers/net/ice/ice_rxtx.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > index e6ddd2513d..69358f6a3a 100644
> > --- a/drivers/net/ice/ice_rxtx.c
> > +++ b/drivers/net/ice/ice_rxtx.c
> > @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev
> > *dev, struct ice_tx_queue *txq)
> >  #define ICE_MIN_TSO_MSS            64
> >  #define ICE_MAX_TSO_MSS            9728
> >  #define ICE_MAX_TSO_FRAME_SIZE     262144
> > +
> > +/*Check for invalid mbuf*/
> > +static inline uint16_t
> > +ice_check_mbuf(struct rte_mbuf *tx_pkt) {
> 
> Better to name the function to exactly match what it does.
> e.g.: ice_check_emtpy_mbuf
> and also declare it as inline.
will optimize.
> 
> > +	struct rte_mbuf *txd = tx_pkt;
> > +
> > +	while (txd != NULL) {
> > +		if (txd->data_len == 0)
> > +			return -1;
> > +		txd = txd->next;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  uint16_t
> >  ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> >  	      uint16_t nb_pkts)
> > @@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> >  	struct ice_tx_queue *txq = tx_queue;
> >  	struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
> >  	uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
> > +	uint16_t nb_used;
> >
> >  	for (i = 0; i < nb_pkts; i++) {
> >  		m = tx_pkts[i];
> > @@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> >  			rte_errno = -ret;
> >  			return i;
> >  		}
> > +
> > +		if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
> > +			ice_check_mbuf(m)) {
> 
> Why "!(ol_flags & RTE_MBUF_F_TX_TCP_SEG)" is needed here?
> A empty mbuf with TSO enabled is still acceptable?
Should not be expected to be implemented in test-pmd, should detect.
> 
> > +			rte_errno = EINVAL;
> > +			PMD_DRV_LOG(ERR, "INVALID mbuf:	last mbuf
> > data_len=[0]");
> > +			return i;
> > +		}
> >  	}
> >  	return i;
> >  }
> > --
> > 2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
  2022-11-11  8:30         ` Ye, MingjinX
@ 2022-11-11  8:45           ` Zhang, Qi Z
  0 siblings, 0 replies; 22+ messages in thread
From: Zhang, Qi Z @ 2022-11-11  8:45 UTC (permalink / raw)
  To: Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Wu, Jingjing, Lu, Wenzhuo,
	Ferruh Yigit, Li, Xiaoyun, Liu, KevinX



> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Friday, November 11, 2022 4:31 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: 2022年11月11日 13:09
> > To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ferruh
> > Yigit <ferruh.yigit@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> > Liu, KevinX <kevinx.liu@intel.com>
> > Subject: RE: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> >
> >
> >
> > > -----Original Message-----
> > > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > > Sent: Friday, November 11, 2022 8:04 PM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > > YidingX <yidingx.zhou@intel.com>; Ye, MingjinX
> > > <mingjinx.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> > > Jingjing <jingjing.wu@intel.com>; Lu, Wenzhuo
> > > <wenzhuo.lu@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>; Li,
> > > Xiaoyun <xiaoyun.li@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> > > Subject: [PATCH v3 2/2] net/ice: fix scalar Tx path segment
> > >
> > > The scalar Tx path would send empty buffer that causes the Tx queue
> > > to overflow.
> > >
> > > This patch adds the last buffer length judgment in tx_prepare to fix
> > > this issue, rte_errno will be set to EINVAL and returned if the last
> > > buffer is
> > empty.
> > >
> > > Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> > > Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > > ---
> > >  drivers/net/ice/ice_rxtx.c | 24 ++++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > > index e6ddd2513d..69358f6a3a 100644
> > > --- a/drivers/net/ice/ice_rxtx.c
> > > +++ b/drivers/net/ice/ice_rxtx.c
> > > @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev
> > > *dev, struct ice_tx_queue *txq)
> > >  #define ICE_MIN_TSO_MSS            64
> > >  #define ICE_MAX_TSO_MSS            9728
> > >  #define ICE_MAX_TSO_FRAME_SIZE     262144
> > > +
> > > +/*Check for invalid mbuf*/
> > > +static inline uint16_t
> > > +ice_check_mbuf(struct rte_mbuf *tx_pkt) {
> >
> > Better to name the function to exactly match what it does.
> > e.g.: ice_check_emtpy_mbuf
> > and also declare it as inline.
> will optimize.
> >
> > > +	struct rte_mbuf *txd = tx_pkt;
> > > +
> > > +	while (txd != NULL) {
> > > +		if (txd->data_len == 0)
> > > +			return -1;
> > > +		txd = txd->next;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  uint16_t
> > >  ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> > >  	      uint16_t nb_pkts)
> > > @@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > > struct rte_mbuf **tx_pkts,
> > >  	struct ice_tx_queue *txq = tx_queue;
> > >  	struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
> > >  	uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
> > > +	uint16_t nb_used;
> > >
> > >  	for (i = 0; i < nb_pkts; i++) {
> > >  		m = tx_pkts[i];
> > > @@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > > struct rte_mbuf **tx_pkts,
> > >  			rte_errno = -ret;
> > >  			return i;
> > >  		}
> > > +
> > > +		if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
> > > +			ice_check_mbuf(m)) {
> >
> > Why "!(ol_flags & RTE_MBUF_F_TX_TCP_SEG)" is needed here?
> > A empty mbuf with TSO enabled is still acceptable?
> Should not be expected to be implemented in test-pmd, should detect.

Why related with testpmd?, testpmd is not the only application, this is the question for how to identify a violated mbuf.

> >
> > > +			rte_errno = EINVAL;
> > > +			PMD_DRV_LOG(ERR, "INVALID mbuf:	last mbuf
> > > data_len=[0]");
> > > +			return i;
> > > +		}
> > >  	}
> > >  	return i;
> > >  }
> > > --
> > > 2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v4 2/2] net/ice: fix scalar Tx path segment
  2022-11-11 16:12         ` [PATCH v4 2/2] net/ice: fix scalar Tx " Mingjin Ye
@ 2022-11-11  9:01           ` Zhang, Qi Z
  2022-11-11  9:14             ` Xu, Ke1
  2023-09-19  8:15           ` David Marchand
  1 sibling, 1 reply; 22+ messages in thread
From: Zhang, Qi Z @ 2022-11-11  9:01 UTC (permalink / raw)
  To: Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Wu, Jingjing, Ferruh Yigit,
	Lu, Wenzhuo, Liu, KevinX



> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Saturday, November 12, 2022 12:13 AM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Liu,
> KevinX <kevinx.liu@intel.com>
> Subject: [PATCH v4 2/2] net/ice: fix scalar Tx path segment
> 
> The scalar Tx path would send empty buffer that causes the Tx queue to
> overflow.
> 
> This patch adds the last buffer length judgment in tx_prepare to fix this issue,
> rte_errno will be set to EINVAL and returned if the last buffer is empty.
> 
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>

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

Applied to dpdk-next-net-intel.

Thanks
Qi


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v4 1/2] net/ice: fix scalar Rx path segment
  2022-11-11 16:12       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
@ 2022-11-11  9:03         ` Zhang, Qi Z
  2022-11-11  9:13           ` Xu, Ke1
  2022-11-11 16:12         ` [PATCH v4 2/2] net/ice: fix scalar Tx " Mingjin Ye
  1 sibling, 1 reply; 22+ messages in thread
From: Zhang, Qi Z @ 2022-11-11  9:03 UTC (permalink / raw)
  To: Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Wu, Jingjing, Ferruh Yigit,
	Lu, Wenzhuo, Li, Xiaoyun



> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Saturday, November 12, 2022 12:13 AM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH v4 1/2] net/ice: fix scalar Rx path segment
> 
> CRC is stripped by the hardware in the scattered Rx path. The last buffer is
> invalid if it's packet length is zero.
> 
> This patch adds a judgment for the last buffer length to fix this issue, it would
> free the mbuf associated to the last one if the last buffer is empty.
> 
> Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>

v3 already be applied, no new change on v4, will ignore this patch.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v4 1/2] net/ice: fix scalar Rx path segment
  2022-11-11  9:03         ` Zhang, Qi Z
@ 2022-11-11  9:13           ` Xu, Ke1
  0 siblings, 0 replies; 22+ messages in thread
From: Xu, Ke1 @ 2022-11-11  9:13 UTC (permalink / raw)
  To: Zhang, Qi Z, Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Wu, Jingjing, Ferruh Yigit,
	Lu, Wenzhuo, Li, Xiaoyun



> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Friday, November 11, 2022 5:03 PM
> To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Ferruh
> Yigit <ferruh.yigit@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>
> Subject: RE: [PATCH v4 1/2] net/ice: fix scalar Rx path segment
> 
> 
> 
> > -----Original Message-----
> > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > Sent: Saturday, November 12, 2022 12:13 AM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Ye, MingjinX
> > <mingjinx.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> > Jingjing <jingjing.wu@intel.com>; Ferruh Yigit
> > <ferruh.yigit@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li,
> > Xiaoyun <xiaoyun.li@intel.com>
> > Subject: [PATCH v4 1/2] net/ice: fix scalar Rx path segment
> >
> > CRC is stripped by the hardware in the scattered Rx path. The last
> > buffer is invalid if it's packet length is zero.
> >
> > This patch adds a judgment for the last buffer length to fix this
> > issue, it would free the mbuf associated to the last one if the last buffer is
> empty.
> >
> > Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> 
> v3 already be applied, no new change on v4, will ignore this patch.

Tested with v4 patch, passed.

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH v4 2/2] net/ice: fix scalar Tx path segment
  2022-11-11  9:01           ` Zhang, Qi Z
@ 2022-11-11  9:14             ` Xu, Ke1
  0 siblings, 0 replies; 22+ messages in thread
From: Xu, Ke1 @ 2022-11-11  9:14 UTC (permalink / raw)
  To: Zhang, Qi Z, Ye, MingjinX, dev
  Cc: Yang, Qiming, stable, Zhou, YidingX, Wu, Jingjing, Ferruh Yigit,
	Lu, Wenzhuo, Liu, KevinX



> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Friday, November 11, 2022 5:02 PM
> To: Ye, MingjinX <mingjinx.ye@intel.com>; dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Ferruh
> Yigit <ferruh.yigit@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Liu,
> KevinX <kevinx.liu@intel.com>
> Subject: RE: [PATCH v4 2/2] net/ice: fix scalar Tx path segment
> 
> 
> 
> > -----Original Message-----
> > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > Sent: Saturday, November 12, 2022 12:13 AM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; stable@dpdk.org; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Ye, MingjinX
> > <mingjinx.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> > Jingjing <jingjing.wu@intel.com>; Ferruh Yigit
> > <ferruh.yigit@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Liu,
> > KevinX <kevinx.liu@intel.com>
> > Subject: [PATCH v4 2/2] net/ice: fix scalar Tx path segment
> >
> > The scalar Tx path would send empty buffer that causes the Tx queue to
> > overflow.
> >
> > This patch adds the last buffer length judgment in tx_prepare to fix
> > this issue, rte_errno will be set to EINVAL and returned if the last buffer is
> empty.
> >
> > Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> > Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Applied to dpdk-next-net-intel.
> 
> Thanks
> Qi

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 1/2] net/ice: fix scalar Rx path segment
  2022-11-09 12:56 ` [PATCH v2] " Mingjin Ye
  2022-11-10  2:01   ` Xu, Ke1
  2022-11-10 10:37   ` Zhang, Qi Z
@ 2022-11-11 12:04   ` Mingjin Ye
  2022-11-11  4:59     ` Zhang, Qi Z
  2022-11-11 12:04     ` [PATCH v3 2/2] net/ice: fix scalar Tx " Mingjin Ye
  2 siblings, 2 replies; 22+ messages in thread
From: Mingjin Ye @ 2022-11-11 12:04 UTC (permalink / raw)
  To: dev
  Cc: qiming.yang, stable, yidingx.zhou, Mingjin Ye, Qi Zhang,
	Ferruh Yigit, Jingjing Wu, Wenzhuo Lu, Xiaoyun Li

CRC is stripped by the hardware in the scattered Rx path. The last buffer
is invalid if it's packet length is zero.

This patch adds a judgment for the last buffer length to fix this issue,
it would free the mbuf associated to the last one if the last buffer is
empty.

Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 drivers/net/ice/ice_rxtx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 0a2b0376ac..e6ddd2513d 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
 			} else
 				rxm->data_len = (uint16_t)(rx_packet_len -
 							   RTE_ETHER_CRC_LEN);
+		} else if (rx_packet_len == 0) {
+			rte_pktmbuf_free_seg(rxm);
+			first_seg->nb_segs--;
+			last_seg->next = NULL;
 		}
 
 		first_seg->port = rxq->port_id;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 2/2] net/ice: fix scalar Tx path segment
  2022-11-11 12:04   ` [PATCH v3 1/2] net/ice: fix scalar Rx " Mingjin Ye
  2022-11-11  4:59     ` Zhang, Qi Z
@ 2022-11-11 12:04     ` Mingjin Ye
  2022-11-11  5:09       ` Zhang, Qi Z
                         ` (2 more replies)
  1 sibling, 3 replies; 22+ messages in thread
From: Mingjin Ye @ 2022-11-11 12:04 UTC (permalink / raw)
  To: dev
  Cc: qiming.yang, stable, yidingx.zhou, Mingjin Ye, Qi Zhang,
	Jingjing Wu, Wenzhuo Lu, Ferruh Yigit, Xiaoyun Li, Kevin Liu

The scalar Tx path would send empty buffer that causes the Tx queue to
overflow.

This patch adds the last buffer length judgment in tx_prepare to fix this
issue, rte_errno will be set to EINVAL and returned if the last buffer is
empty.

Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 drivers/net/ice/ice_rxtx.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index e6ddd2513d..69358f6a3a 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev, struct ice_tx_queue *txq)
 #define ICE_MIN_TSO_MSS            64
 #define ICE_MAX_TSO_MSS            9728
 #define ICE_MAX_TSO_FRAME_SIZE     262144
+
+/*Check for invalid mbuf*/
+static inline uint16_t
+ice_check_mbuf(struct rte_mbuf *tx_pkt)
+{
+	struct rte_mbuf *txd = tx_pkt;
+
+	while (txd != NULL) {
+		if (txd->data_len == 0)
+			return -1;
+		txd = txd->next;
+	}
+
+	return 0;
+}
+
 uint16_t
 ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 	      uint16_t nb_pkts)
@@ -3653,6 +3669,7 @@ ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 	struct ice_tx_queue *txq = tx_queue;
 	struct rte_eth_dev *dev = &rte_eth_devices[txq->port_id];
 	uint16_t max_frame_size = dev->data->mtu + ICE_ETH_OVERHEAD;
+	uint16_t nb_used;
 
 	for (i = 0; i < nb_pkts; i++) {
 		m = tx_pkts[i];
@@ -3689,6 +3706,13 @@ ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 			rte_errno = -ret;
 			return i;
 		}
+
+		if (!(ol_flags & RTE_MBUF_F_TX_TCP_SEG) &&
+			ice_check_mbuf(m)) {
+			rte_errno = EINVAL;
+			PMD_DRV_LOG(ERR, "INVALID mbuf:	last mbuf data_len=[0]");
+			return i;
+		}
 	}
 	return i;
 }
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 1/2] net/ice: fix scalar Rx path segment
  2022-11-11 12:04     ` [PATCH v3 2/2] net/ice: fix scalar Tx " Mingjin Ye
  2022-11-11  5:09       ` Zhang, Qi Z
@ 2022-11-11 16:01       ` Mingjin Ye
  2022-11-11 16:01         ` [PATCH v4 2/2] net/ice: fix scalar Tx " Mingjin Ye
  2022-11-11 16:12       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
  2 siblings, 1 reply; 22+ messages in thread
From: Mingjin Ye @ 2022-11-11 16:01 UTC (permalink / raw)
  To: dev
  Cc: qiming.yang, stable, yidingx.zhou, Mingjin Ye, Qi Zhang,
	Xiaoyun Li, Jingjing Wu, Wenzhuo Lu, Ferruh Yigit

CRC is stripped by the hardware in the scattered Rx path. The last buffer
is invalid if it's packet length is zero.

This patch adds a judgment for the last buffer length to fix this issue,
it would free the mbuf associated to the last one if the last buffer is
empty.

Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 drivers/net/ice/ice_rxtx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 0a2b0376ac..e6ddd2513d 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
 			} else
 				rxm->data_len = (uint16_t)(rx_packet_len -
 							   RTE_ETHER_CRC_LEN);
+		} else if (rx_packet_len == 0) {
+			rte_pktmbuf_free_seg(rxm);
+			first_seg->nb_segs--;
+			last_seg->next = NULL;
 		}
 
 		first_seg->port = rxq->port_id;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 2/2] net/ice: fix scalar Tx path segment
  2022-11-11 16:01       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
@ 2022-11-11 16:01         ` Mingjin Ye
  0 siblings, 0 replies; 22+ messages in thread
From: Mingjin Ye @ 2022-11-11 16:01 UTC (permalink / raw)
  To: dev
  Cc: qiming.yang, stable, yidingx.zhou, Mingjin Ye, Qi Zhang,
	Xiaoyun Li, Jingjing Wu, Ferruh Yigit, Wenzhuo Lu, Kevin Liu

The scalar Tx path would send empty buffer that causes the Tx queue to
overflow.

This patch adds the last buffer length judgment in tx_prepare to fix this
issue, rte_errno will be set to EINVAL and returned if the last buffer is
empty.

Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
v3:
	* delete unused variable.
---
 drivers/net/ice/ice_rxtx.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index e6ddd2513d..c1cde65a39 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev, struct ice_tx_queue *txq)
 #define ICE_MIN_TSO_MSS            64
 #define ICE_MAX_TSO_MSS            9728
 #define ICE_MAX_TSO_FRAME_SIZE     262144
+
+/*Check for emtpy mbuf*/
+static inline uint16_t
+ice_check_emtpy_mbuf(struct rte_mbuf *tx_pkt)
+{
+	struct rte_mbuf *txd = tx_pkt;
+
+	while (txd != NULL) {
+		if (txd->data_len == 0)
+			return -1;
+		txd = txd->next;
+	}
+
+	return 0;
+}
+
 uint16_t
 ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 	      uint16_t nb_pkts)
@@ -3689,6 +3705,12 @@ ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 			rte_errno = -ret;
 			return i;
 		}
+
+		if (ice_check_emtpy_mbuf(m) != 0) {
+			rte_errno = EINVAL;
+			PMD_DRV_LOG(ERR, "INVALID mbuf:	last mbuf data_len=[0]");
+			return i;
+		}
 	}
 	return i;
 }
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 1/2] net/ice: fix scalar Rx path segment
  2022-11-11 12:04     ` [PATCH v3 2/2] net/ice: fix scalar Tx " Mingjin Ye
  2022-11-11  5:09       ` Zhang, Qi Z
  2022-11-11 16:01       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
@ 2022-11-11 16:12       ` Mingjin Ye
  2022-11-11  9:03         ` Zhang, Qi Z
  2022-11-11 16:12         ` [PATCH v4 2/2] net/ice: fix scalar Tx " Mingjin Ye
  2 siblings, 2 replies; 22+ messages in thread
From: Mingjin Ye @ 2022-11-11 16:12 UTC (permalink / raw)
  To: dev
  Cc: qiming.yang, stable, yidingx.zhou, Mingjin Ye, Qi Zhang,
	Jingjing Wu, Ferruh Yigit, Wenzhuo Lu, Xiaoyun Li

CRC is stripped by the hardware in the scattered Rx path. The last buffer
is invalid if it's packet length is zero.

This patch adds a judgment for the last buffer length to fix this issue,
it would free the mbuf associated to the last one if the last buffer is
empty.

Fixes: 6eac0b7fde95 ("net/ice: support advance Rx/Tx")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 drivers/net/ice/ice_rxtx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 0a2b0376ac..e6ddd2513d 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2111,6 +2111,10 @@ ice_recv_scattered_pkts(void *rx_queue,
 			} else
 				rxm->data_len = (uint16_t)(rx_packet_len -
 							   RTE_ETHER_CRC_LEN);
+		} else if (rx_packet_len == 0) {
+			rte_pktmbuf_free_seg(rxm);
+			first_seg->nb_segs--;
+			last_seg->next = NULL;
 		}
 
 		first_seg->port = rxq->port_id;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 2/2] net/ice: fix scalar Tx path segment
  2022-11-11 16:12       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
  2022-11-11  9:03         ` Zhang, Qi Z
@ 2022-11-11 16:12         ` Mingjin Ye
  2022-11-11  9:01           ` Zhang, Qi Z
  2023-09-19  8:15           ` David Marchand
  1 sibling, 2 replies; 22+ messages in thread
From: Mingjin Ye @ 2022-11-11 16:12 UTC (permalink / raw)
  To: dev
  Cc: qiming.yang, stable, yidingx.zhou, Mingjin Ye, Qi Zhang,
	Jingjing Wu, Ferruh Yigit, Wenzhuo Lu, Kevin Liu

The scalar Tx path would send empty buffer that causes the Tx queue to
overflow.

This patch adds the last buffer length judgment in tx_prepare to fix this
issue, rte_errno will be set to EINVAL and returned if the last buffer is
empty.

Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>

v3:
	* delete unused variable.
	* Full detection of mbufs.
---
 drivers/net/ice/ice_rxtx.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index e6ddd2513d..9017353943 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev, struct ice_tx_queue *txq)
 #define ICE_MIN_TSO_MSS            64
 #define ICE_MAX_TSO_MSS            9728
 #define ICE_MAX_TSO_FRAME_SIZE     262144
+
+/*Check for empty mbuf*/
+static inline uint16_t
+ice_check_empty_mbuf(struct rte_mbuf *tx_pkt)
+{
+	struct rte_mbuf *txd = tx_pkt;
+
+	while (txd != NULL) {
+		if (txd->data_len == 0)
+			return -1;
+		txd = txd->next;
+	}
+
+	return 0;
+}
+
 uint16_t
 ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 	      uint16_t nb_pkts)
@@ -3689,6 +3705,12 @@ ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 			rte_errno = -ret;
 			return i;
 		}
+
+		if (ice_check_empty_mbuf(m) != 0) {
+			rte_errno = EINVAL;
+			PMD_DRV_LOG(ERR, "INVALID mbuf:	last mbuf data_len=[0]");
+			return i;
+		}
 	}
 	return i;
 }
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/2] net/ice: fix scalar Tx path segment
  2022-11-11 16:12         ` [PATCH v4 2/2] net/ice: fix scalar Tx " Mingjin Ye
  2022-11-11  9:01           ` Zhang, Qi Z
@ 2023-09-19  8:15           ` David Marchand
  2023-09-27  8:44             ` David Marchand
  1 sibling, 1 reply; 22+ messages in thread
From: David Marchand @ 2023-09-19  8:15 UTC (permalink / raw)
  To: Mingjin Ye, qiming.yang, Qi Zhang
  Cc: dev, stable, yidingx.zhou, Jingjing Wu, Ferruh Yigit, Wenzhuo Lu,
	Kevin Liu

Hello,

Replying on this old thread, as it seems no in depth review was done
but I need more info before fixing a bug added by ccf33dccf7aa
("net/ice: check illegal packet sizes").


On Fri, Nov 11, 2022 at 9:26 AM Mingjin Ye <mingjinx.ye@intel.com> wrote:
>
> The scalar Tx path would send empty buffer that causes the Tx queue to
> overflow.

This is too vague.
Please describe the exact issue.
Is it a sw (read net/ice driver) bug? a hw bug?


>
> This patch adds the last buffer length judgment in tx_prepare to fix this

This reads odd.

The added code will stop and return an error when the last segment
*that is evaluated by the code* is empty.
But on the other hand, it is easier to understand that the check is to
catch the *first* empty segment of a packet.


> issue, rte_errno will be set to EINVAL and returned if the last buffer is
> empty.
>
> Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
>
> v3:
>         * delete unused variable.
>         * Full detection of mbufs.
> ---
>  drivers/net/ice/ice_rxtx.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> index e6ddd2513d..9017353943 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev, struct ice_tx_queue *txq)
>  #define ICE_MIN_TSO_MSS            64
>  #define ICE_MAX_TSO_MSS            9728
>  #define ICE_MAX_TSO_FRAME_SIZE     262144
> +
> +/*Check for empty mbuf*/

Missing spaces.


> +static inline uint16_t
> +ice_check_empty_mbuf(struct rte_mbuf *tx_pkt)

This function name is confusing.

In dpdk, a mbuf can be both a segment descriptor (part of a packet) or
a packet descriptor (in which case, it overlaps the first segment
descriptor).
So it is better to be explicit with "segment" or "packet" when
referring to a mbuf.


> +{
> +       struct rte_mbuf *txd = tx_pkt;
> +
> +       while (txd != NULL) {
> +               if (txd->data_len == 0)
> +                       return -1;
> +               txd = txd->next;
> +       }

There is nothing in the API that says that an empty segment is faulty.

With the current description, I understand that any empty segment
could lead to a bug, regardless of asking offloads (which
ice_prep_pkts is all about).
So a more valid fix is probably to skip empty segments in the tx
handler function(s) and not reject packets in ice_prep_pkts.
Another option could be to fix this "overflow" mentionned in the commitlog.

Looking fwd to explanations.


> +
> +       return 0;
> +}
> +
>  uint16_t
>  ice_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
>               uint16_t nb_pkts)


-- 
David Marchand


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/2] net/ice: fix scalar Tx path segment
  2023-09-19  8:15           ` David Marchand
@ 2023-09-27  8:44             ` David Marchand
  0 siblings, 0 replies; 22+ messages in thread
From: David Marchand @ 2023-09-27  8:44 UTC (permalink / raw)
  To: Mingjin Ye, qiming.yang, Qi Zhang
  Cc: dev, stable, yidingx.zhou, Jingjing Wu, Ferruh Yigit, Wenzhuo Lu,
	Kevin Liu

On Tue, Sep 19, 2023 at 10:15 AM David Marchand
<david.marchand@redhat.com> wrote:
> Replying on this old thread, as it seems no in depth review was done
> but I need more info before fixing a bug added by ccf33dccf7aa
> ("net/ice: check illegal packet sizes").
>
>
> On Fri, Nov 11, 2022 at 9:26 AM Mingjin Ye <mingjinx.ye@intel.com> wrote:
> >
> > The scalar Tx path would send empty buffer that causes the Tx queue to
> > overflow.
>
> This is too vague.
> Please describe the exact issue.
> Is it a sw (read net/ice driver) bug? a hw bug?
>
>
> >
> > This patch adds the last buffer length judgment in tx_prepare to fix this
>
> This reads odd.
>
> The added code will stop and return an error when the last segment
> *that is evaluated by the code* is empty.
> But on the other hand, it is easier to understand that the check is to
> catch the *first* empty segment of a packet.
>
>
> > issue, rte_errno will be set to EINVAL and returned if the last buffer is
> > empty.
> >
> > Fixes: 17c7d0f9d6a4 ("net/ice: support basic Rx/Tx")
> > Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> >
> > v3:
> >         * delete unused variable.
> >         * Full detection of mbufs.
> > ---
> >  drivers/net/ice/ice_rxtx.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > index e6ddd2513d..9017353943 100644
> > --- a/drivers/net/ice/ice_rxtx.c
> > +++ b/drivers/net/ice/ice_rxtx.c
> > @@ -3643,6 +3643,22 @@ ice_set_tx_function_flag(struct rte_eth_dev *dev, struct ice_tx_queue *txq)
> >  #define ICE_MIN_TSO_MSS            64
> >  #define ICE_MAX_TSO_MSS            9728
> >  #define ICE_MAX_TSO_FRAME_SIZE     262144
> > +
> > +/*Check for empty mbuf*/
>
> Missing spaces.
>
>
> > +static inline uint16_t
> > +ice_check_empty_mbuf(struct rte_mbuf *tx_pkt)
>
> This function name is confusing.
>
> In dpdk, a mbuf can be both a segment descriptor (part of a packet) or
> a packet descriptor (in which case, it overlaps the first segment
> descriptor).
> So it is better to be explicit with "segment" or "packet" when
> referring to a mbuf.
>
>
> > +{
> > +       struct rte_mbuf *txd = tx_pkt;
> > +
> > +       while (txd != NULL) {
> > +               if (txd->data_len == 0)
> > +                       return -1;
> > +               txd = txd->next;
> > +       }
>
> There is nothing in the API that says that an empty segment is faulty.
>
> With the current description, I understand that any empty segment
> could lead to a bug, regardless of asking offloads (which
> ice_prep_pkts is all about).
> So a more valid fix is probably to skip empty segments in the tx
> handler function(s) and not reject packets in ice_prep_pkts.
> Another option could be to fix this "overflow" mentionned in the commitlog.
>
> Looking fwd to explanations.

Ping.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-09-27  8:45 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03 17:20 [PATCH] net/ice: fix scalar Rx and Tx path segment Mingjin Ye
2022-11-04  6:57 ` Xu, Ke1
2022-11-09 12:56 ` [PATCH v2] " Mingjin Ye
2022-11-10  2:01   ` Xu, Ke1
2022-11-10 10:37   ` Zhang, Qi Z
2022-11-11  3:12     ` Ye, MingjinX
2022-11-11 12:04   ` [PATCH v3 1/2] net/ice: fix scalar Rx " Mingjin Ye
2022-11-11  4:59     ` Zhang, Qi Z
2022-11-11 12:04     ` [PATCH v3 2/2] net/ice: fix scalar Tx " Mingjin Ye
2022-11-11  5:09       ` Zhang, Qi Z
2022-11-11  8:30         ` Ye, MingjinX
2022-11-11  8:45           ` Zhang, Qi Z
2022-11-11 16:01       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
2022-11-11 16:01         ` [PATCH v4 2/2] net/ice: fix scalar Tx " Mingjin Ye
2022-11-11 16:12       ` [PATCH v4 1/2] net/ice: fix scalar Rx " Mingjin Ye
2022-11-11  9:03         ` Zhang, Qi Z
2022-11-11  9:13           ` Xu, Ke1
2022-11-11 16:12         ` [PATCH v4 2/2] net/ice: fix scalar Tx " Mingjin Ye
2022-11-11  9:01           ` Zhang, Qi Z
2022-11-11  9:14             ` Xu, Ke1
2023-09-19  8:15           ` David Marchand
2023-09-27  8:44             ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).