DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/iavf: fix core dump when exiting testpmd
@ 2023-10-26  8:54 Kaiwen Deng
  2023-10-31  8:15 ` Zhang, Qi Z
  2023-11-01  1:34 ` [PATCH v2] net/iavf: fix coredump " Kaiwen Deng
  0 siblings, 2 replies; 12+ messages in thread
From: Kaiwen Deng @ 2023-10-26  8:54 UTC (permalink / raw)
  To: dev
  Cc: qiming.yang, yidingx.zhou, Kaiwen Deng, Jingjing Wu, Beilei Xing,
	Wenzhuo Lu, Bruce Richardson

release null mbuf results coredump.
This commit adding mbuf check before releasing.

Fixes: 12016895fcf3 ("net/iavf: fix buffer leak on Tx queue stop")

Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
---
 drivers/net/iavf/iavf_rxtx_vec_common.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h b/drivers/net/iavf/iavf_rxtx_vec_common.h
index e18cdc3f11..f50a500536 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_common.h
+++ b/drivers/net/iavf/iavf_rxtx_vec_common.h
@@ -187,8 +187,11 @@ _iavf_tx_queue_release_mbufs_vec(struct iavf_tx_queue *txq)
 
 	i = txq->next_dd - txq->rs_thresh + 1;
 	while (i != txq->tx_tail) {
-		rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf);
-		txq->sw_ring[i].mbuf = NULL;
+		if (txq->sw_ring[i].mbuf) {
+			rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf);
+			txq->sw_ring[i].mbuf = NULL;
+		}
+
 		if (++i == txq->nb_tx_desc)
 			i = 0;
 	}
-- 
2.34.1


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

* RE: [PATCH] net/iavf: fix core dump when exiting testpmd
  2023-10-26  8:54 [PATCH] net/iavf: fix core dump when exiting testpmd Kaiwen Deng
@ 2023-10-31  8:15 ` Zhang, Qi Z
  2023-11-01  1:34 ` [PATCH v2] net/iavf: fix coredump " Kaiwen Deng
  1 sibling, 0 replies; 12+ messages in thread
From: Zhang, Qi Z @ 2023-10-31  8:15 UTC (permalink / raw)
  To: Deng, KaiwenX, dev
  Cc: Yang, Qiming, Zhou, YidingX, Deng, KaiwenX, Wu, Jingjing, Xing,
	Beilei, Lu, Wenzhuo, Richardson, Bruce



> -----Original Message-----
> From: Kaiwen Deng <kaiwenx.deng@intel.com>
> Sent: Thursday, October 26, 2023 4:55 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: [PATCH] net/iavf: fix core dump when exiting testpmd
> 
> release null mbuf results coredump.
> This commit adding mbuf check before releasing.
> 
> Fixes: 12016895fcf3 ("net/iavf: fix buffer leak on Tx queue stop")
> 
> Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> ---
>  drivers/net/iavf/iavf_rxtx_vec_common.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h
> b/drivers/net/iavf/iavf_rxtx_vec_common.h
> index e18cdc3f11..f50a500536 100644
> --- a/drivers/net/iavf/iavf_rxtx_vec_common.h
> +++ b/drivers/net/iavf/iavf_rxtx_vec_common.h
> @@ -187,8 +187,11 @@ _iavf_tx_queue_release_mbufs_vec(struct
> iavf_tx_queue *txq)
> 
>  	i = txq->next_dd - txq->rs_thresh + 1;
>  	while (i != txq->tx_tail) {
> -		rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf);
> -		txq->sw_ring[i].mbuf = NULL;
> +		if (txq->sw_ring[i].mbuf) {
> +			rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf);
> +			txq->sw_ring[i].mbuf = NULL;
> +		}

Could you explain how to reproduce this issue in the commit log.

Looks like it is to fix the scenario that queue stop and start again and some mbuf will be double free at some time?

> +
>  		if (++i == txq->nb_tx_desc)
>  			i = 0;
>  	}
> --
> 2.34.1


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

* [PATCH v2] net/iavf: fix coredump when exiting testpmd
  2023-10-26  8:54 [PATCH] net/iavf: fix core dump when exiting testpmd Kaiwen Deng
  2023-10-31  8:15 ` Zhang, Qi Z
@ 2023-11-01  1:34 ` Kaiwen Deng
  2023-11-01  7:53   ` Lu, Wenzhuo
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Kaiwen Deng @ 2023-11-01  1:34 UTC (permalink / raw)
  To: dev
  Cc: stable, qiming.yang, yidingx.zhou, Kaiwen Deng, Jingjing Wu,
	Beilei Xing, Zhichao Zeng

Avf releasing mbuf using the vector path release API 
causes a coredump when the basic Tx path is selected.
This commit changes to use the basic path release API
when selecting the basic Tx path.

Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
Cc: stable@dpdk.org

Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
---
 drivers/net/iavf/iavf_rxtx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index 610912f635..a16e03d88c 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
 				PMD_DRV_LOG(DEBUG,
 					"AVX2 does not support outer checksum offload, using Basic Tx (port %d).",
 					dev->data->port_id);
+				return;
 			} else {
 				dev->tx_pkt_burst = iavf_xmit_pkts_vec_avx2_offload;
 				dev->tx_pkt_prepare = iavf_prep_pkts;
-- 
2.34.1


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

* RE: [PATCH v2] net/iavf: fix coredump when exiting testpmd
  2023-11-01  1:34 ` [PATCH v2] net/iavf: fix coredump " Kaiwen Deng
@ 2023-11-01  7:53   ` Lu, Wenzhuo
  2023-11-01  9:32     ` Deng, KaiwenX
  2023-11-01 11:00   ` Zhang, Qi Z
  2023-11-02  4:43   ` [PATCH v3] net/iavf: fix mbuf release API selection Kaiwen Deng
  2 siblings, 1 reply; 12+ messages in thread
From: Lu, Wenzhuo @ 2023-11-01  7:53 UTC (permalink / raw)
  To: Deng, KaiwenX, dev
  Cc: stable, Yang, Qiming, Zhou, YidingX, Deng, KaiwenX, Wu, Jingjing,
	Xing, Beilei, Zeng, ZhichaoX

Hi Kaiwen,

> -----Original Message-----
> From: Kaiwen Deng <kaiwenx.deng@intel.com>
> Sent: Wednesday, November 1, 2023 9:35 AM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: [PATCH v2] net/iavf: fix coredump when exiting testpmd
> 
> Avf releasing mbuf using the vector path release API causes a coredump
> when the basic Tx path is selected.
> This commit changes to use the basic path release API when selecting the
> basic Tx path.
Sorry, don't catch the point. 
I see you changed the code when selecting AVX2 non-offload path. Confused about what's " the vector path release API " and what's " the basic path release API ".

> 
> Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> ---
>  drivers/net/iavf/iavf_rxtx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index
> 610912f635..a16e03d88c 100644
> --- a/drivers/net/iavf/iavf_rxtx.c
> +++ b/drivers/net/iavf/iavf_rxtx.c
> @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
>  				PMD_DRV_LOG(DEBUG,
>  					"AVX2 does not support outer checksum offload,
> using Basic Tx (port %d).",
>  					dev->data->port_id);
> +				return;
>  			} else {
>  				dev->tx_pkt_burst = iavf_xmit_pkts_vec_avx2_offload;
>  				dev->tx_pkt_prepare = iavf_prep_pkts;
> --
> 2.34.1


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

* RE: [PATCH v2] net/iavf: fix coredump when exiting testpmd
  2023-11-01  7:53   ` Lu, Wenzhuo
@ 2023-11-01  9:32     ` Deng, KaiwenX
  0 siblings, 0 replies; 12+ messages in thread
From: Deng, KaiwenX @ 2023-11-01  9:32 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev
  Cc: stable, Yang, Qiming, Zhou, YidingX, Wu, Jingjing, Xing, Beilei,
	Zeng, ZhichaoX



> -----Original Message-----
> From: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Sent: Wednesday, November 1, 2023 3:53 PM
> To: Deng, KaiwenX <kaiwenx.deng@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: RE: [PATCH v2] net/iavf: fix coredump when exiting testpmd
> 
> Hi Kaiwen,
> 
> > -----Original Message-----
> > From: Kaiwen Deng <kaiwenx.deng@intel.com>
> > Sent: Wednesday, November 1, 2023 9:35 AM
> > To: dev@dpdk.org
> > Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Deng, KaiwenX
> > <kaiwenx.deng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> > Beilei <beilei.xing@intel.com>; Zeng, ZhichaoX
> > <zhichaox.zeng@intel.com>
> > Subject: [PATCH v2] net/iavf: fix coredump when exiting testpmd
> >
> > Avf releasing mbuf using the vector path release API causes a coredump
> > when the basic Tx path is selected.
> > This commit changes to use the basic path release API when selecting
> > the basic Tx path.
> Sorry, don't catch the point.
> I see you changed the code when selecting AVX2 non-offload path. Confused
> about what's " the vector path release API " and what's " the basic path
> release API ".
> 
Hi Wenzhuo:

Thanks for your review.
According to the code below.
"the vector path release API" is iavf_tx_queue_release_mbufs_sse.
"the basic path release API" is release_txq_mbufs.

static const
struct iavf_txq_ops iavf_txq_release_mbufs_ops[] = {
	[IAVF_REL_MBUFS_DEFAULT].release_mbufs = release_txq_mbufs,
#ifdef RTE_ARCH_X86
	[IAVF_REL_MBUFS_SSE_VEC].release_mbufs = iavf_tx_queue_release_mbufs_sse,
#ifdef CC_AVX512_SUPPORT
	[IAVF_REL_MBUFS_AVX512_VEC].release_mbufs = iavf_tx_queue_release_mbufs_avx512,
#endif
> >
> > Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> > ---
> >  drivers/net/iavf/iavf_rxtx.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/iavf/iavf_rxtx.c
> > b/drivers/net/iavf/iavf_rxtx.c index 610912f635..a16e03d88c 100644
> > --- a/drivers/net/iavf/iavf_rxtx.c
> > +++ b/drivers/net/iavf/iavf_rxtx.c
> > @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
> >  				PMD_DRV_LOG(DEBUG,
> >  					"AVX2 does not support outer
> checksum offload, using Basic Tx
> > (port %d).",
> >  					dev->data->port_id);
> > +				return;
> >  			} else {
> >  				dev->tx_pkt_burst =
> iavf_xmit_pkts_vec_avx2_offload;
> >  				dev->tx_pkt_prepare = iavf_prep_pkts;
> > --
> > 2.34.1


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

* RE: [PATCH v2] net/iavf: fix coredump when exiting testpmd
  2023-11-01  1:34 ` [PATCH v2] net/iavf: fix coredump " Kaiwen Deng
  2023-11-01  7:53   ` Lu, Wenzhuo
@ 2023-11-01 11:00   ` Zhang, Qi Z
  2023-11-02  4:43   ` [PATCH v3] net/iavf: fix mbuf release API selection Kaiwen Deng
  2 siblings, 0 replies; 12+ messages in thread
From: Zhang, Qi Z @ 2023-11-01 11:00 UTC (permalink / raw)
  To: Deng, KaiwenX, dev
  Cc: stable, Yang, Qiming, Zhou, YidingX, Deng, KaiwenX, Wu, Jingjing,
	Xing, Beilei, Zeng, ZhichaoX



> -----Original Message-----
> From: Kaiwen Deng <kaiwenx.deng@intel.com>
> Sent: Wednesday, November 1, 2023 9:35 AM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: [PATCH v2] net/iavf: fix coredump when exiting testpmd

please remove testpmd from the title if this issue is not specific to testpmd.

You can always add reproduce step with testpmd in commit log.

> 
> Avf releasing mbuf using the vector path release API causes a coredump
> when the basic Tx path is selected.
> This commit changes to use the basic path release API when selecting the
> basic Tx path.
> 
> Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> ---
>  drivers/net/iavf/iavf_rxtx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index
> 610912f635..a16e03d88c 100644
> --- a/drivers/net/iavf/iavf_rxtx.c
> +++ b/drivers/net/iavf/iavf_rxtx.c
> @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
>  				PMD_DRV_LOG(DEBUG,
>  					"AVX2 does not support outer
> checksum offload, using Basic Tx (port %d).",
>  					dev->data->port_id);
> +				return;
>  			} else {
>  				dev->tx_pkt_burst =
> iavf_xmit_pkts_vec_avx2_offload;
>  				dev->tx_pkt_prepare = iavf_prep_pkts;
> --
> 2.34.1


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

* [PATCH v3] net/iavf: fix mbuf release API selection
  2023-11-01  1:34 ` [PATCH v2] net/iavf: fix coredump " Kaiwen Deng
  2023-11-01  7:53   ` Lu, Wenzhuo
  2023-11-01 11:00   ` Zhang, Qi Z
@ 2023-11-02  4:43   ` Kaiwen Deng
  2023-11-08  9:33     ` Zhang, Qi Z
  2023-11-09  4:58     ` [PATCH v4] " Kaiwen Deng
  2 siblings, 2 replies; 12+ messages in thread
From: Kaiwen Deng @ 2023-11-02  4:43 UTC (permalink / raw)
  To: dev
  Cc: stable, qiming.yang, yidingx.zhou, Kaiwen Deng, Jingjing Wu,
	Beilei Xing, Zhichao Zeng

When AVX2 is forcibly selected and outer checksum
offload is configured, the basic Tx path will be
selected. Also, the txq mbuf release API is incorrectly
set to iavf_tx_queue_release_mbufs_sse. This causes
coredump.

This commit selects release_txq_mbufs to releasing
txq mbufs when selecting the basic Tx path.

Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
Cc: stable@dpdk.org

Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
---
 drivers/net/iavf/iavf_rxtx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index 610912f635..a16e03d88c 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
 				PMD_DRV_LOG(DEBUG,
 					"AVX2 does not support outer checksum offload, using Basic Tx (port %d).",
 					dev->data->port_id);
+				return;
 			} else {
 				dev->tx_pkt_burst = iavf_xmit_pkts_vec_avx2_offload;
 				dev->tx_pkt_prepare = iavf_prep_pkts;
-- 
2.34.1


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

* RE: [PATCH v3] net/iavf: fix mbuf release API selection
  2023-11-02  4:43   ` [PATCH v3] net/iavf: fix mbuf release API selection Kaiwen Deng
@ 2023-11-08  9:33     ` Zhang, Qi Z
  2023-11-08 12:02       ` Zhang, Qi Z
  2023-11-09  4:58     ` [PATCH v4] " Kaiwen Deng
  1 sibling, 1 reply; 12+ messages in thread
From: Zhang, Qi Z @ 2023-11-08  9:33 UTC (permalink / raw)
  To: Deng, KaiwenX, dev
  Cc: stable, Yang, Qiming, Zhou, YidingX, Deng, KaiwenX, Wu, Jingjing,
	Xing, Beilei, Zeng, ZhichaoX



> -----Original Message-----
> From: Kaiwen Deng <kaiwenx.deng@intel.com>
> Sent: Thursday, November 2, 2023 12:43 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: [PATCH v3] net/iavf: fix mbuf release API selection
> 
> When AVX2 is forcibly selected and outer checksum offload is configured, the
> basic Tx path will be selected. Also, the txq mbuf release API is incorrectly set
> to iavf_tx_queue_release_mbufs_sse. This causes coredump.
> 
> This commit selects release_txq_mbufs to releasing txq mbufs when selecting
> the basic Tx path.
> 
> Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> ---
>  drivers/net/iavf/iavf_rxtx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index
> 610912f635..a16e03d88c 100644
> --- a/drivers/net/iavf/iavf_rxtx.c
> +++ b/drivers/net/iavf/iavf_rxtx.c
> @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
>  				PMD_DRV_LOG(DEBUG,
>  					"AVX2 does not support outer
> checksum offload, using Basic Tx (port %d).",
>  					dev->data->port_id);
> +				return;

This make the execution routing not consistent between avx2 and avx512.
I think it will be a better solution if we reset the use_avx2 flag here, and use this flag to decide if need to overwrite the release function later.

>  			} else {
>  				dev->tx_pkt_burst =
> iavf_xmit_pkts_vec_avx2_offload;
>  				dev->tx_pkt_prepare = iavf_prep_pkts;
> --
> 2.34.1


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

* RE: [PATCH v3] net/iavf: fix mbuf release API selection
  2023-11-08  9:33     ` Zhang, Qi Z
@ 2023-11-08 12:02       ` Zhang, Qi Z
  2023-11-09  1:42         ` Deng, KaiwenX
  0 siblings, 1 reply; 12+ messages in thread
From: Zhang, Qi Z @ 2023-11-08 12:02 UTC (permalink / raw)
  To: Zhang, Qi Z, Deng, KaiwenX, dev
  Cc: stable, Yang, Qiming, Zhou, YidingX, Deng, KaiwenX, Wu, Jingjing,
	Xing, Beilei, Zeng, ZhichaoX



> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Wednesday, November 8, 2023 5:33 PM
> To: Deng, KaiwenX <kaiwenx.deng@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: RE: [PATCH v3] net/iavf: fix mbuf release API selection
> 
> 
> 
> > -----Original Message-----
> > From: Kaiwen Deng <kaiwenx.deng@intel.com>
> > Sent: Thursday, November 2, 2023 12:43 PM
> > To: dev@dpdk.org
> > Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Deng, KaiwenX
> > <kaiwenx.deng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> > Beilei <beilei.xing@intel.com>; Zeng, ZhichaoX
> > <zhichaox.zeng@intel.com>
> > Subject: [PATCH v3] net/iavf: fix mbuf release API selection
> >
> > When AVX2 is forcibly selected and outer checksum offload is
> > configured, the basic Tx path will be selected. Also, the txq mbuf
> > release API is incorrectly set to iavf_tx_queue_release_mbufs_sse. This
> causes coredump.
> >
> > This commit selects release_txq_mbufs to releasing txq mbufs when
> > selecting the basic Tx path.
> >
> > Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> > ---
> >  drivers/net/iavf/iavf_rxtx.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/iavf/iavf_rxtx.c
> > b/drivers/net/iavf/iavf_rxtx.c index 610912f635..a16e03d88c 100644
> > --- a/drivers/net/iavf/iavf_rxtx.c
> > +++ b/drivers/net/iavf/iavf_rxtx.c
> > @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
> >  				PMD_DRV_LOG(DEBUG,
> >  					"AVX2 does not support outer
> > checksum offload, using Basic Tx (port %d).",
> >  					dev->data->port_id);
> > +				return;
> 
> This make the execution routing not consistent between avx2 and avx512.
> I think it will be a better solution if we reset the use_avx2 flag here, and use
> this flag to decide if need to overwrite the release function later.

Or you can just "goto normal" and removing function call assignment.

> 
> >  			} else {
> >  				dev->tx_pkt_burst =
> > iavf_xmit_pkts_vec_avx2_offload;
> >  				dev->tx_pkt_prepare = iavf_prep_pkts;
> > --
> > 2.34.1


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

* RE: [PATCH v3] net/iavf: fix mbuf release API selection
  2023-11-08 12:02       ` Zhang, Qi Z
@ 2023-11-09  1:42         ` Deng, KaiwenX
  0 siblings, 0 replies; 12+ messages in thread
From: Deng, KaiwenX @ 2023-11-09  1:42 UTC (permalink / raw)
  To: Zhang, Qi Z, dev
  Cc: stable, Yang, Qiming, Zhou, YidingX, Wu, Jingjing, Xing, Beilei,
	Zeng, ZhichaoX



> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Wednesday, November 8, 2023 8:03 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Deng, KaiwenX
> <kaiwenx.deng@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: RE: [PATCH v3] net/iavf: fix mbuf release API selection
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: Wednesday, November 8, 2023 5:33 PM
> > To: Deng, KaiwenX <kaiwenx.deng@intel.com>; dev@dpdk.org
> > Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Deng, KaiwenX
> > <kaiwenx.deng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> > Beilei <beilei.xing@intel.com>; Zeng, ZhichaoX
> > <zhichaox.zeng@intel.com>
> > Subject: RE: [PATCH v3] net/iavf: fix mbuf release API selection
> >
> >
> >
> > > -----Original Message-----
> > > From: Kaiwen Deng <kaiwenx.deng@intel.com>
> > > Sent: Thursday, November 2, 2023 12:43 PM
> > > To: dev@dpdk.org
> > > Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> > > YidingX <yidingx.zhou@intel.com>; Deng, KaiwenX
> > > <kaiwenx.deng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > > Xing, Beilei <beilei.xing@intel.com>; Zeng, ZhichaoX
> > > <zhichaox.zeng@intel.com>
> > > Subject: [PATCH v3] net/iavf: fix mbuf release API selection
> > >
> > > When AVX2 is forcibly selected and outer checksum offload is
> > > configured, the basic Tx path will be selected. Also, the txq mbuf
> > > release API is incorrectly set to iavf_tx_queue_release_mbufs_sse.
> > > This
> > causes coredump.
> > >
> > > This commit selects release_txq_mbufs to releasing txq mbufs when
> > > selecting the basic Tx path.
> > >
> > > Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> > > ---
> > >  drivers/net/iavf/iavf_rxtx.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/iavf/iavf_rxtx.c
> > > b/drivers/net/iavf/iavf_rxtx.c index 610912f635..a16e03d88c 100644
> > > --- a/drivers/net/iavf/iavf_rxtx.c
> > > +++ b/drivers/net/iavf/iavf_rxtx.c
> > > @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
> > >  				PMD_DRV_LOG(DEBUG,
> > >  					"AVX2 does not support outer
> > > checksum offload, using Basic Tx (port %d).",
> > >  					dev->data->port_id);
> > > +				return;
> >
> > This make the execution routing not consistent between avx2 and avx512.
> > I think it will be a better solution if we reset the use_avx2 flag
> > here, and use this flag to decide if need to overwrite the release function
> later.
> 
> Or you can just "goto normal" and removing function call assignment.
I will prepare a new patch.
Thanks.
> 
> >
> > >  			} else {
> > >  				dev->tx_pkt_burst =
> > > iavf_xmit_pkts_vec_avx2_offload;
> > >  				dev->tx_pkt_prepare = iavf_prep_pkts;
> > > --
> > > 2.34.1


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

* [PATCH v4] net/iavf: fix mbuf release API selection
  2023-11-02  4:43   ` [PATCH v3] net/iavf: fix mbuf release API selection Kaiwen Deng
  2023-11-08  9:33     ` Zhang, Qi Z
@ 2023-11-09  4:58     ` Kaiwen Deng
  2023-11-09 12:23       ` Zhang, Qi Z
  1 sibling, 1 reply; 12+ messages in thread
From: Kaiwen Deng @ 2023-11-09  4:58 UTC (permalink / raw)
  To: dev
  Cc: stable, qiming.yang, yidingx.zhou, Kaiwen Deng, Jingjing Wu,
	Beilei Xing, Zhichao Zeng

When AVX2 is forcibly selected and outer checksum
offload is configured, the basic Tx path will be
selected. Also, the txq mbuf release API is incorrectly
set to iavf_tx_queue_release_mbufs_sse. This causes
coredump.

This commit selects release_txq_mbufs to releasing
txq mbufs when selecting the basic Tx path.

Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
Cc: stable@dpdk.org

Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
---
 drivers/net/iavf/iavf_rxtx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index 45f638c1d2..f19aa14646 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -4018,11 +4018,9 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
 				PMD_DRV_LOG(DEBUG, "Using AVX2 Vector Tx (port %d).",
 					    dev->data->port_id);
 			} else if (check_ret == IAVF_VECTOR_CTX_OFFLOAD_PATH) {
-				dev->tx_pkt_burst = iavf_xmit_pkts;
-				dev->tx_pkt_prepare = iavf_prep_pkts;
 				PMD_DRV_LOG(DEBUG,
-					"AVX2 does not support outer checksum offload, using Basic Tx (port %d).",
-					dev->data->port_id);
+					"AVX2 does not support outer checksum offload.");
+				goto normal;
 			} else {
 				dev->tx_pkt_burst = iavf_xmit_pkts_vec_avx2_offload;
 				dev->tx_pkt_prepare = iavf_prep_pkts;
-- 
2.34.1


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

* RE: [PATCH v4] net/iavf: fix mbuf release API selection
  2023-11-09  4:58     ` [PATCH v4] " Kaiwen Deng
@ 2023-11-09 12:23       ` Zhang, Qi Z
  0 siblings, 0 replies; 12+ messages in thread
From: Zhang, Qi Z @ 2023-11-09 12:23 UTC (permalink / raw)
  To: Deng, KaiwenX, dev
  Cc: stable, Yang, Qiming, Zhou, YidingX, Deng, KaiwenX, Wu, Jingjing,
	Xing, Beilei, Zeng, ZhichaoX



> -----Original Message-----
> From: Kaiwen Deng <kaiwenx.deng@intel.com>
> Sent: Thursday, November 9, 2023 12:59 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: [PATCH v4] net/iavf: fix mbuf release API selection
> 
> When AVX2 is forcibly selected and outer checksum offload is configured, the
> basic Tx path will be selected. Also, the txq mbuf release API is incorrectly set
> to iavf_tx_queue_release_mbufs_sse. This causes coredump.
> 
> This commit selects release_txq_mbufs to releasing txq mbufs when selecting
> the basic Tx path.
> 
> Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>

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

Applied to dpdk-next-net-intel.

Thanks
Qi


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

end of thread, other threads:[~2023-11-09 12:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-26  8:54 [PATCH] net/iavf: fix core dump when exiting testpmd Kaiwen Deng
2023-10-31  8:15 ` Zhang, Qi Z
2023-11-01  1:34 ` [PATCH v2] net/iavf: fix coredump " Kaiwen Deng
2023-11-01  7:53   ` Lu, Wenzhuo
2023-11-01  9:32     ` Deng, KaiwenX
2023-11-01 11:00   ` Zhang, Qi Z
2023-11-02  4:43   ` [PATCH v3] net/iavf: fix mbuf release API selection Kaiwen Deng
2023-11-08  9:33     ` Zhang, Qi Z
2023-11-08 12:02       ` Zhang, Qi Z
2023-11-09  1:42         ` Deng, KaiwenX
2023-11-09  4:58     ` [PATCH v4] " Kaiwen Deng
2023-11-09 12:23       ` Zhang, Qi Z

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).