* [PATCH 1/4] net/i40e: fix buffer leak on Rx reconfiguration [not found] <20230830155919.592390-1-bruce.richardson@intel.com> @ 2023-08-30 15:59 ` Bruce Richardson 2023-08-30 15:59 ` [PATCH 2/4] net/iavf: fix buffer leak on Tx queue stop Bruce Richardson ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Bruce Richardson @ 2023-08-30 15:59 UTC (permalink / raw) To: dev Cc: Yuying Zhang, Beilei Xing, Jingjing Wu, Bruce Richardson, qi.z.zhang, stable When reconfiguring a single queue on a device, the mbuf initializer value was not getting set, and remained at zero. This lead to mbuf leaks as the refcount was incorrect (0), so on free it wrapped around to UINT16_MAX. When setting up the mbuf initializer, also ensure that the queue is explicitly marked as using a vector function by setting the "rx_using_sse" flag. Fixes: a3c83a2527e1 ("net/i40e: enable runtime queue setup") Cc: qi.z.zhang@intel.com Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/net/i40e/i40e_rxtx.c | 6 ++++++ drivers/net/i40e/i40e_rxtx_vec_common.h | 1 + 2 files changed, 7 insertions(+) diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index b4f65b58fa..d96bbbb677 100644 --- a/drivers/net/i40e/i40e_rxtx.c +++ b/drivers/net/i40e/i40e_rxtx.c @@ -1918,6 +1918,12 @@ i40e_dev_rx_queue_setup_runtime(struct rte_eth_dev *dev, if (use_def_burst_func) ad->rx_bulk_alloc_allowed = false; i40e_set_rx_function(dev); + + if (ad->rx_vec_allowed && i40e_rxq_vec_setup(rxq)) { + PMD_DRV_LOG(ERR, "Failed vector rx setup."); + return -EINVAL; + } + return 0; } else if (ad->rx_vec_allowed && !rte_is_power_of_2(rxq->nb_rx_desc)) { PMD_DRV_LOG(ERR, "Vector mode is allowed, but descriptor" diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h index fe1a6ec75e..8b745630e4 100644 --- a/drivers/net/i40e/i40e_rxtx_vec_common.h +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h @@ -201,6 +201,7 @@ i40e_rxq_vec_setup_default(struct i40e_rx_queue *rxq) rte_compiler_barrier(); p = (uintptr_t)&mb_def.rearm_data; rxq->mbuf_initializer = *(uint64_t *)p; + rxq->rx_using_sse = 1; return 0; } -- 2.39.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] net/iavf: fix buffer leak on Tx queue stop [not found] <20230830155919.592390-1-bruce.richardson@intel.com> 2023-08-30 15:59 ` [PATCH 1/4] net/i40e: fix buffer leak on Rx reconfiguration Bruce Richardson @ 2023-08-30 15:59 ` Bruce Richardson 2023-08-30 15:59 ` [PATCH 3/4] net/iavf: fix restart of Rx queue on reconfigure Bruce Richardson [not found] ` <20230831123337.871496-1-bruce.richardson@intel.com> 3 siblings, 0 replies; 13+ messages in thread From: Bruce Richardson @ 2023-08-30 15:59 UTC (permalink / raw) To: dev Cc: Yuying Zhang, Beilei Xing, Jingjing Wu, Bruce Richardson, wenzhuo.lu, stable When a queue is stopped all buffers are meant to released from it, and then a new set of buffers reallocated on start. For the iavf code when using vector Tx, some buffers were left in the ring, and so those buffers were leaked. The buffers were missed as the release code only handled one side of a wrap-around case in the ring. Fix the issue by doing a single iteration of the ring freeing all buffers in it, handling wraparound through a simple post-increment check. Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") Fixes: 9ab9514c150e ("net/iavf: enable AVX512 for Tx") Cc: wenzhuo.lu@intel.com Cc: jingjing.wu@intel.com Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/net/iavf/iavf_rxtx_vec_avx512.c | 17 ++++++++--------- drivers/net/iavf/iavf_rxtx_vec_common.h | 11 +++++------ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/net/iavf/iavf_rxtx_vec_avx512.c b/drivers/net/iavf/iavf_rxtx_vec_avx512.c index 3e66df5341..48337d5e03 100644 --- a/drivers/net/iavf/iavf_rxtx_vec_avx512.c +++ b/drivers/net/iavf/iavf_rxtx_vec_avx512.c @@ -2460,20 +2460,19 @@ iavf_tx_queue_release_mbufs_avx512(struct iavf_tx_queue *txq) { unsigned int i; const uint16_t max_desc = (uint16_t)(txq->nb_tx_desc - 1); + const uint16_t end_desc = txq->tx_tail >> txq->use_ctx; /* next empty slot */ + const uint16_t wrap_point = txq->nb_tx_desc >> txq->use_ctx; /* end of SW ring */ struct iavf_tx_vec_entry *swr = (void *)txq->sw_ring; if (!txq->sw_ring || txq->nb_free == max_desc) return; - i = (txq->next_dd >> txq->use_ctx) + 1 - - (txq->rs_thresh >> txq->use_ctx); - - if (txq->tx_tail < i) { - for (; i < (unsigned int)(txq->nb_tx_desc >> txq->use_ctx); i++) { - rte_pktmbuf_free_seg(swr[i].mbuf); - swr[i].mbuf = NULL; - } - i = 0; + i = (txq->next_dd - txq->rs_thresh + 1) >> txq->use_ctx; + while (i != end_desc) { + rte_pktmbuf_free_seg(swr[i].mbuf); + swr[i].mbuf = NULL; + if (++i == wrap_point) + i = 0; } } diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h b/drivers/net/iavf/iavf_rxtx_vec_common.h index ddb13ce8c3..1fac957fe1 100644 --- a/drivers/net/iavf/iavf_rxtx_vec_common.h +++ b/drivers/net/iavf/iavf_rxtx_vec_common.h @@ -186,12 +186,11 @@ _iavf_tx_queue_release_mbufs_vec(struct iavf_tx_queue *txq) return; i = txq->next_dd - txq->rs_thresh + 1; - if (txq->tx_tail < i) { - for (; i < txq->nb_tx_desc; i++) { - rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf); - txq->sw_ring[i].mbuf = NULL; - } - i = 0; + while (i != txq->tx_tail) { + rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf); + txq->sw_ring[i].mbuf = NULL; + if (++i == txq->nb_tx_desc) + i = 0; } } -- 2.39.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] net/iavf: fix restart of Rx queue on reconfigure [not found] <20230830155919.592390-1-bruce.richardson@intel.com> 2023-08-30 15:59 ` [PATCH 1/4] net/i40e: fix buffer leak on Rx reconfiguration Bruce Richardson 2023-08-30 15:59 ` [PATCH 2/4] net/iavf: fix buffer leak on Tx queue stop Bruce Richardson @ 2023-08-30 15:59 ` Bruce Richardson [not found] ` <20230831123337.871496-1-bruce.richardson@intel.com> 3 siblings, 0 replies; 13+ messages in thread From: Bruce Richardson @ 2023-08-30 15:59 UTC (permalink / raw) To: dev; +Cc: Yuying Zhang, Beilei Xing, Jingjing Wu, Bruce Richardson, stable After reconfiguring an RX queue the mbuf_initializer value was not being correctly set. Fix this by calling the appropriate function if vector processing is enabled. This mirrors the behaviour by the i40e driver. Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") Cc: jingjing.wu@intel.com Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/net/iavf/iavf_rxtx.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index f7df4665d1..347f686831 100644 --- a/drivers/net/iavf/iavf_rxtx.c +++ b/drivers/net/iavf/iavf_rxtx.c @@ -755,6 +755,12 @@ iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, if (check_rx_vec_allow(rxq) == false) ad->rx_vec_allowed = false; + /* check vector conflict */ + if (ad->rx_vec_allowed && iavf_rxq_vec_setup(rxq)) { + PMD_DRV_LOG(ERR, "Failed vector rx setup."); + return -EINVAL; + } + return 0; } -- 2.39.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <20230831123337.871496-1-bruce.richardson@intel.com>]
* [PATCH v2 1/4] net/i40e: fix buffer leak on Rx reconfiguration [not found] ` <20230831123337.871496-1-bruce.richardson@intel.com> @ 2023-08-31 12:33 ` Bruce Richardson 2023-09-01 7:12 ` Zhang, Qi Z 2023-08-31 12:33 ` [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop Bruce Richardson 2023-08-31 12:33 ` [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure Bruce Richardson 2 siblings, 1 reply; 13+ messages in thread From: Bruce Richardson @ 2023-08-31 12:33 UTC (permalink / raw) To: dev; +Cc: Bruce Richardson, qi.z.zhang, stable When reconfiguring a single queue on a device, the mbuf initializer value was not getting set, and remained at zero. This lead to mbuf leaks as the refcount was incorrect (0), so on free it wrapped around to UINT16_MAX. When setting up the mbuf initializer, also ensure that the queue is explicitly marked as using a vector function by setting the "rx_using_sse" flag. Fixes: a3c83a2527e1 ("net/i40e: enable runtime queue setup") Cc: qi.z.zhang@intel.com Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/net/i40e/i40e_rxtx.c | 6 ++++++ drivers/net/i40e/i40e_rxtx_vec_common.h | 1 + 2 files changed, 7 insertions(+) diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index b4f65b58fa..d96bbbb677 100644 --- a/drivers/net/i40e/i40e_rxtx.c +++ b/drivers/net/i40e/i40e_rxtx.c @@ -1918,6 +1918,12 @@ i40e_dev_rx_queue_setup_runtime(struct rte_eth_dev *dev, if (use_def_burst_func) ad->rx_bulk_alloc_allowed = false; i40e_set_rx_function(dev); + + if (ad->rx_vec_allowed && i40e_rxq_vec_setup(rxq)) { + PMD_DRV_LOG(ERR, "Failed vector rx setup."); + return -EINVAL; + } + return 0; } else if (ad->rx_vec_allowed && !rte_is_power_of_2(rxq->nb_rx_desc)) { PMD_DRV_LOG(ERR, "Vector mode is allowed, but descriptor" diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h index fe1a6ec75e..8b745630e4 100644 --- a/drivers/net/i40e/i40e_rxtx_vec_common.h +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h @@ -201,6 +201,7 @@ i40e_rxq_vec_setup_default(struct i40e_rx_queue *rxq) rte_compiler_barrier(); p = (uintptr_t)&mb_def.rearm_data; rxq->mbuf_initializer = *(uint64_t *)p; + rxq->rx_using_sse = 1; return 0; } -- 2.39.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 1/4] net/i40e: fix buffer leak on Rx reconfiguration 2023-08-31 12:33 ` [PATCH v2 1/4] net/i40e: fix buffer leak on Rx reconfiguration Bruce Richardson @ 2023-09-01 7:12 ` Zhang, Qi Z 0 siblings, 0 replies; 13+ messages in thread From: Zhang, Qi Z @ 2023-09-01 7:12 UTC (permalink / raw) To: Richardson, Bruce, dev; +Cc: stable > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Thursday, August 31, 2023 8:34 PM > To: dev@dpdk.org > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Zhang, Qi Z > <qi.z.zhang@intel.com>; stable@dpdk.org > Subject: [PATCH v2 1/4] net/i40e: fix buffer leak on Rx reconfiguration > > When reconfiguring a single queue on a device, the mbuf initializer value was > not getting set, and remained at zero. This lead to mbuf leaks as the refcount > was incorrect (0), so on free it wrapped around to UINT16_MAX. When > setting up the mbuf initializer, also ensure that the queue is explicitly marked > as using a vector function by setting the "rx_using_sse" flag. > > Fixes: a3c83a2527e1 ("net/i40e: enable runtime queue setup") > Cc: qi.z.zhang@intel.com > Cc: stable@dpdk.org > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/net/i40e/i40e_rxtx.c | 6 ++++++ > drivers/net/i40e/i40e_rxtx_vec_common.h | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index > b4f65b58fa..d96bbbb677 100644 > --- a/drivers/net/i40e/i40e_rxtx.c > +++ b/drivers/net/i40e/i40e_rxtx.c > @@ -1918,6 +1918,12 @@ i40e_dev_rx_queue_setup_runtime(struct > rte_eth_dev *dev, > if (use_def_burst_func) > ad->rx_bulk_alloc_allowed = false; > i40e_set_rx_function(dev); > + > + if (ad->rx_vec_allowed && i40e_rxq_vec_setup(rxq)) { > + PMD_DRV_LOG(ERR, "Failed vector rx setup."); > + return -EINVAL; > + } > + > return 0; > } else if (ad->rx_vec_allowed && !rte_is_power_of_2(rxq- > >nb_rx_desc)) { > PMD_DRV_LOG(ERR, "Vector mode is allowed, but > descriptor" > diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h > b/drivers/net/i40e/i40e_rxtx_vec_common.h > index fe1a6ec75e..8b745630e4 100644 > --- a/drivers/net/i40e/i40e_rxtx_vec_common.h > +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h > @@ -201,6 +201,7 @@ i40e_rxq_vec_setup_default(struct i40e_rx_queue > *rxq) > rte_compiler_barrier(); > p = (uintptr_t)&mb_def.rearm_data; > rxq->mbuf_initializer = *(uint64_t *)p; > + rxq->rx_using_sse = 1; I saw some further code clean can be done by leveraging this change. > return 0; > } > > -- > 2.39.2 Acked-by: Qi Zhang <qi.z.zhang@intel.com> Applied to dpdk-next-net-intel. Thanks Qi ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop [not found] ` <20230831123337.871496-1-bruce.richardson@intel.com> 2023-08-31 12:33 ` [PATCH v2 1/4] net/i40e: fix buffer leak on Rx reconfiguration Bruce Richardson @ 2023-08-31 12:33 ` Bruce Richardson 2023-09-04 2:17 ` Lu, Wenzhuo 2023-08-31 12:33 ` [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure Bruce Richardson 2 siblings, 1 reply; 13+ messages in thread From: Bruce Richardson @ 2023-08-31 12:33 UTC (permalink / raw) To: dev; +Cc: Bruce Richardson, wenzhuo.lu, jingjing.wu, stable When a queue is stopped all buffers are meant to released from it, and then a new set of buffers reallocated on start. For the iavf code when using vector Tx, some buffers were left in the ring, and so those buffers were leaked. The buffers were missed as the release code only handled one side of a wrap-around case in the ring. Fix the issue by doing a single iteration of the ring freeing all buffers in it, handling wraparound through a simple post-increment check. Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") Fixes: 9ab9514c150e ("net/iavf: enable AVX512 for Tx") Cc: wenzhuo.lu@intel.com Cc: jingjing.wu@intel.com Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/net/iavf/iavf_rxtx_vec_avx512.c | 17 ++++++++--------- drivers/net/iavf/iavf_rxtx_vec_common.h | 11 +++++------ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/net/iavf/iavf_rxtx_vec_avx512.c b/drivers/net/iavf/iavf_rxtx_vec_avx512.c index 3e66df5341..48337d5e03 100644 --- a/drivers/net/iavf/iavf_rxtx_vec_avx512.c +++ b/drivers/net/iavf/iavf_rxtx_vec_avx512.c @@ -2460,20 +2460,19 @@ iavf_tx_queue_release_mbufs_avx512(struct iavf_tx_queue *txq) { unsigned int i; const uint16_t max_desc = (uint16_t)(txq->nb_tx_desc - 1); + const uint16_t end_desc = txq->tx_tail >> txq->use_ctx; /* next empty slot */ + const uint16_t wrap_point = txq->nb_tx_desc >> txq->use_ctx; /* end of SW ring */ struct iavf_tx_vec_entry *swr = (void *)txq->sw_ring; if (!txq->sw_ring || txq->nb_free == max_desc) return; - i = (txq->next_dd >> txq->use_ctx) + 1 - - (txq->rs_thresh >> txq->use_ctx); - - if (txq->tx_tail < i) { - for (; i < (unsigned int)(txq->nb_tx_desc >> txq->use_ctx); i++) { - rte_pktmbuf_free_seg(swr[i].mbuf); - swr[i].mbuf = NULL; - } - i = 0; + i = (txq->next_dd - txq->rs_thresh + 1) >> txq->use_ctx; + while (i != end_desc) { + rte_pktmbuf_free_seg(swr[i].mbuf); + swr[i].mbuf = NULL; + if (++i == wrap_point) + i = 0; } } diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h b/drivers/net/iavf/iavf_rxtx_vec_common.h index ddb13ce8c3..1fac957fe1 100644 --- a/drivers/net/iavf/iavf_rxtx_vec_common.h +++ b/drivers/net/iavf/iavf_rxtx_vec_common.h @@ -186,12 +186,11 @@ _iavf_tx_queue_release_mbufs_vec(struct iavf_tx_queue *txq) return; i = txq->next_dd - txq->rs_thresh + 1; - if (txq->tx_tail < i) { - for (; i < txq->nb_tx_desc; i++) { - rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf); - txq->sw_ring[i].mbuf = NULL; - } - i = 0; + while (i != txq->tx_tail) { + rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf); + txq->sw_ring[i].mbuf = NULL; + if (++i == txq->nb_tx_desc) + i = 0; } } -- 2.39.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop 2023-08-31 12:33 ` [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop Bruce Richardson @ 2023-09-04 2:17 ` Lu, Wenzhuo 2023-09-04 2:30 ` Zhang, Qi Z 0 siblings, 1 reply; 13+ messages in thread From: Lu, Wenzhuo @ 2023-09-04 2:17 UTC (permalink / raw) To: Richardson, Bruce, dev; +Cc: Wu, Jingjing, stable > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Thursday, August 31, 2023 8:34 PM > To: dev@dpdk.org > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Lu, Wenzhuo > <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; > stable@dpdk.org > Subject: [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop > > When a queue is stopped all buffers are meant to released from it, and then a > new set of buffers reallocated on start. For the iavf code when using vector Tx, > some buffers were left in the ring, and so those buffers were leaked. The > buffers were missed as the release code only handled one side of a > wrap-around case in the ring. > > Fix the issue by doing a single iteration of the ring freeing all buffers in it, > handling wraparound through a simple post-increment check. > > Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") > Fixes: 9ab9514c150e ("net/iavf: enable AVX512 for Tx") > Cc: wenzhuo.lu@intel.com > Cc: jingjing.wu@intel.com > Cc: stable@dpdk.org > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop 2023-09-04 2:17 ` Lu, Wenzhuo @ 2023-09-04 2:30 ` Zhang, Qi Z 0 siblings, 0 replies; 13+ messages in thread From: Zhang, Qi Z @ 2023-09-04 2:30 UTC (permalink / raw) To: Lu, Wenzhuo, Richardson, Bruce, dev; +Cc: Wu, Jingjing, stable > -----Original Message----- > From: Lu, Wenzhuo <wenzhuo.lu@intel.com> > Sent: Monday, September 4, 2023 10:18 AM > To: Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org > Cc: Wu, Jingjing <jingjing.wu@intel.com>; stable@dpdk.org > Subject: RE: [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop > > > > > -----Original Message----- > > From: Richardson, Bruce <bruce.richardson@intel.com> > > Sent: Thursday, August 31, 2023 8:34 PM > > To: dev@dpdk.org > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Lu, Wenzhuo > > <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; > > stable@dpdk.org > > Subject: [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop > > > > When a queue is stopped all buffers are meant to released from it, and > > then a new set of buffers reallocated on start. For the iavf code when > > using vector Tx, some buffers were left in the ring, and so those > > buffers were leaked. The buffers were missed as the release code only > > handled one side of a wrap-around case in the ring. > > > > Fix the issue by doing a single iteration of the ring freeing all > > buffers in it, handling wraparound through a simple post-increment check. > > > > Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") > > Fixes: 9ab9514c150e ("net/iavf: enable AVX512 for Tx") > > Cc: wenzhuo.lu@intel.com > > Cc: jingjing.wu@intel.com > > Cc: stable@dpdk.org > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com> Applied to dpdk-next-net-intel. Thanks Qi ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure [not found] ` <20230831123337.871496-1-bruce.richardson@intel.com> 2023-08-31 12:33 ` [PATCH v2 1/4] net/i40e: fix buffer leak on Rx reconfiguration Bruce Richardson 2023-08-31 12:33 ` [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop Bruce Richardson @ 2023-08-31 12:33 ` Bruce Richardson 2023-09-04 1:15 ` Zhang, Qi Z 2 siblings, 1 reply; 13+ messages in thread From: Bruce Richardson @ 2023-08-31 12:33 UTC (permalink / raw) To: dev; +Cc: Bruce Richardson, jingjing.wu, stable After reconfiguring an RX queue the mbuf_initializer value was not being correctly set. Fix this by calling the appropriate function if vector processing is enabled. This mirrors the behaviour by the i40e driver. Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") Cc: jingjing.wu@intel.com Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/net/iavf/iavf_rxtx.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index f7df4665d1..797cdda4b2 100644 --- a/drivers/net/iavf/iavf_rxtx.c +++ b/drivers/net/iavf/iavf_rxtx.c @@ -755,6 +755,13 @@ iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, if (check_rx_vec_allow(rxq) == false) ad->rx_vec_allowed = false; +#if defined RTE_ARCH_X86 || defined RTE_ARCH_ARM + /* check vector conflict */ + if (ad->rx_vec_allowed && iavf_rxq_vec_setup(rxq)) { + PMD_DRV_LOG(ERR, "Failed vector rx setup."); + return -EINVAL; + } +#endif return 0; } -- 2.39.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure 2023-08-31 12:33 ` [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure Bruce Richardson @ 2023-09-04 1:15 ` Zhang, Qi Z 2023-09-04 1:30 ` Zhang, Qi Z 0 siblings, 1 reply; 13+ messages in thread From: Zhang, Qi Z @ 2023-09-04 1:15 UTC (permalink / raw) To: Richardson, Bruce, dev; +Cc: Richardson, Bruce, Wu, Jingjing, stable > -----Original Message----- > From: Bruce Richardson <bruce.richardson@intel.com> > Sent: Thursday, August 31, 2023 8:34 PM > To: dev@dpdk.org > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Wu, Jingjing > <jingjing.wu@intel.com>; stable@dpdk.org > Subject: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure > > After reconfiguring an RX queue the mbuf_initializer value was not being > correctly set. Fix this by calling the appropriate function if vector processing is > enabled. This mirrors the behaviour by the i40e driver. > > Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") > Cc: jingjing.wu@intel.com > Cc: stable@dpdk.org > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/net/iavf/iavf_rxtx.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index > f7df4665d1..797cdda4b2 100644 > --- a/drivers/net/iavf/iavf_rxtx.c > +++ b/drivers/net/iavf/iavf_rxtx.c > @@ -755,6 +755,13 @@ iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, > uint16_t queue_idx, > if (check_rx_vec_allow(rxq) == false) > ad->rx_vec_allowed = false; > > +#if defined RTE_ARCH_X86 || defined RTE_ARCH_ARM > + /* check vector conflict */ > + if (ad->rx_vec_allowed && iavf_rxq_vec_setup(rxq)) { > + PMD_DRV_LOG(ERR, "Failed vector rx setup."); > + return -EINVAL; > + } > +#endif Bruce: May I know more details about how to reproduce this issue? As the iavf PMD does not support RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP (i40e does) Which indicates before we call rte_eth_rx_queue_setup, the device must be stopped (the flag will be checked in rte_eth_rx_queue_setup) So if we do below steps rte_eth_dev_stop .. rte_eth_rx_queue_setup rte_eth_dev_start the iavf_rxq_vec_setup should be invoked in rte_eth_dev_start -> iavf_set_rx_function anything I missed? Thanks Qi > return 0; > } > > -- > 2.39.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure 2023-09-04 1:15 ` Zhang, Qi Z @ 2023-09-04 1:30 ` Zhang, Qi Z 2023-09-04 7:54 ` Bruce Richardson 0 siblings, 1 reply; 13+ messages in thread From: Zhang, Qi Z @ 2023-09-04 1:30 UTC (permalink / raw) To: Richardson, Bruce, dev; +Cc: Richardson, Bruce, Wu, Jingjing, stable > -----Original Message----- > From: Zhang, Qi Z > Sent: Monday, September 4, 2023 9:15 AM > To: Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Wu, Jingjing > <jingjing.wu@intel.com>; stable@dpdk.org > Subject: RE: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure > > > > > -----Original Message----- > > From: Bruce Richardson <bruce.richardson@intel.com> > > Sent: Thursday, August 31, 2023 8:34 PM > > To: dev@dpdk.org > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Wu, Jingjing > > <jingjing.wu@intel.com>; stable@dpdk.org > > Subject: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on > > reconfigure > > > > After reconfiguring an RX queue the mbuf_initializer value was not > > being correctly set. Fix this by calling the appropriate function if > > vector processing is enabled. This mirrors the behaviour by the i40e driver. > > > > Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") > > Cc: jingjing.wu@intel.com > > Cc: stable@dpdk.org > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > --- > > drivers/net/iavf/iavf_rxtx.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/net/iavf/iavf_rxtx.c > > b/drivers/net/iavf/iavf_rxtx.c index > > f7df4665d1..797cdda4b2 100644 > > --- a/drivers/net/iavf/iavf_rxtx.c > > +++ b/drivers/net/iavf/iavf_rxtx.c > > @@ -755,6 +755,13 @@ iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, > > uint16_t queue_idx, > > if (check_rx_vec_allow(rxq) == false) > > ad->rx_vec_allowed = false; > > > > +#if defined RTE_ARCH_X86 || defined RTE_ARCH_ARM > > + /* check vector conflict */ > > + if (ad->rx_vec_allowed && iavf_rxq_vec_setup(rxq)) { > > + PMD_DRV_LOG(ERR, "Failed vector rx setup."); > > + return -EINVAL; > > + } > > +#endif > > Bruce: > > May I know more details about how to reproduce this issue? > As the iavf PMD does not support > RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP (i40e does) OK, not sure if the patch 4/4 answered my question 😊 should I squash patch 3, 4 into one? , for my understanding patch 3 doesn't appear to be a bug fix unless we announce RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP. > Which indicates before we call rte_eth_rx_queue_setup, the device > must be stopped (the flag will be checked in rte_eth_rx_queue_setup) > > So if we do below steps > > rte_eth_dev_stop .. > rte_eth_rx_queue_setup > rte_eth_dev_start > > the iavf_rxq_vec_setup should be invoked in rte_eth_dev_start -> > iavf_set_rx_function > > anything I missed? > > Thanks > Qi > > > return 0; > > } > > > > -- > > 2.39.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure 2023-09-04 1:30 ` Zhang, Qi Z @ 2023-09-04 7:54 ` Bruce Richardson 2023-09-05 0:02 ` Zhang, Qi Z 0 siblings, 1 reply; 13+ messages in thread From: Bruce Richardson @ 2023-09-04 7:54 UTC (permalink / raw) To: Zhang, Qi Z; +Cc: dev, Wu, Jingjing, stable On Mon, Sep 04, 2023 at 02:30:32AM +0100, Zhang, Qi Z wrote: > > > > -----Original Message----- > > From: Zhang, Qi Z > > Sent: Monday, September 4, 2023 9:15 AM > > To: Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Wu, Jingjing > > <jingjing.wu@intel.com>; stable@dpdk.org > > Subject: RE: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure > > > > > > > > > -----Original Message----- > > > From: Bruce Richardson <bruce.richardson@intel.com> > > > Sent: Thursday, August 31, 2023 8:34 PM > > > To: dev@dpdk.org > > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Wu, Jingjing > > > <jingjing.wu@intel.com>; stable@dpdk.org > > > Subject: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on > > > reconfigure > > > > > > After reconfiguring an RX queue the mbuf_initializer value was not > > > being correctly set. Fix this by calling the appropriate function if > > > vector processing is enabled. This mirrors the behaviour by the i40e driver. > > > > > > Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") > > > Cc: jingjing.wu@intel.com > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > > --- > > > drivers/net/iavf/iavf_rxtx.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/drivers/net/iavf/iavf_rxtx.c > > > b/drivers/net/iavf/iavf_rxtx.c index > > > f7df4665d1..797cdda4b2 100644 > > > --- a/drivers/net/iavf/iavf_rxtx.c > > > +++ b/drivers/net/iavf/iavf_rxtx.c > > > @@ -755,6 +755,13 @@ iavf_dev_rx_queue_setup(struct rte_eth_dev *dev, > > > uint16_t queue_idx, > > > if (check_rx_vec_allow(rxq) == false) > > > ad->rx_vec_allowed = false; > > > > > > +#if defined RTE_ARCH_X86 || defined RTE_ARCH_ARM > > > + /* check vector conflict */ > > > + if (ad->rx_vec_allowed && iavf_rxq_vec_setup(rxq)) { > > > + PMD_DRV_LOG(ERR, "Failed vector rx setup."); > > > + return -EINVAL; > > > + } > > > +#endif > > > > Bruce: > > > > May I know more details about how to reproduce this issue? > > As the iavf PMD does not support > > RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP (i40e does) > > OK, not sure if the patch 4/4 answered my question 😊 > > should I squash patch 3, 4 into one? , for my understanding patch 3 doesn't appear to be a bug fix unless we announce RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP. > You may have a point. I was experimenting with queue reconfiguration which is where I hit this issue. However, even without queue reconfig support, the device still needs to support queue-stop followed by queue-start, I think, and there may still be an issue there - I'll have to check. /Bruce ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure 2023-09-04 7:54 ` Bruce Richardson @ 2023-09-05 0:02 ` Zhang, Qi Z 0 siblings, 0 replies; 13+ messages in thread From: Zhang, Qi Z @ 2023-09-05 0:02 UTC (permalink / raw) To: Richardson, Bruce; +Cc: dev, Wu, Jingjing, stable > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Monday, September 4, 2023 3:54 PM > To: Zhang, Qi Z <qi.z.zhang@intel.com> > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; stable@dpdk.org > Subject: Re: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure > > On Mon, Sep 04, 2023 at 02:30:32AM +0100, Zhang, Qi Z wrote: > > > > > > > -----Original Message----- > > > From: Zhang, Qi Z > > > Sent: Monday, September 4, 2023 9:15 AM > > > To: Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org > > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Wu, Jingjing > > > <jingjing.wu@intel.com>; stable@dpdk.org > > > Subject: RE: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on > > > reconfigure > > > > > > > > > > > > > -----Original Message----- > > > > From: Bruce Richardson <bruce.richardson@intel.com> > > > > Sent: Thursday, August 31, 2023 8:34 PM > > > > To: dev@dpdk.org > > > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Wu, Jingjing > > > > <jingjing.wu@intel.com>; stable@dpdk.org > > > > Subject: [PATCH v2 3/4] net/iavf: fix restart of Rx queue on > > > > reconfigure > > > > > > > > After reconfiguring an RX queue the mbuf_initializer value was not > > > > being correctly set. Fix this by calling the appropriate function > > > > if vector processing is enabled. This mirrors the behaviour by the i40e > driver. > > > > > > > > Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx") > > > > Cc: jingjing.wu@intel.com > > > > Cc: stable@dpdk.org > > > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > > > --- > > > > drivers/net/iavf/iavf_rxtx.c | 7 +++++++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/drivers/net/iavf/iavf_rxtx.c > > > > b/drivers/net/iavf/iavf_rxtx.c index > > > > f7df4665d1..797cdda4b2 100644 > > > > --- a/drivers/net/iavf/iavf_rxtx.c > > > > +++ b/drivers/net/iavf/iavf_rxtx.c > > > > @@ -755,6 +755,13 @@ iavf_dev_rx_queue_setup(struct rte_eth_dev > > > > *dev, uint16_t queue_idx, > > > > if (check_rx_vec_allow(rxq) == false) > > > > ad->rx_vec_allowed = false; > > > > > > > > +#if defined RTE_ARCH_X86 || defined RTE_ARCH_ARM > > > > + /* check vector conflict */ > > > > + if (ad->rx_vec_allowed && iavf_rxq_vec_setup(rxq)) { > > > > + PMD_DRV_LOG(ERR, "Failed vector rx setup."); > > > > + return -EINVAL; > > > > + } > > > > +#endif > > > > > > Bruce: > > > > > > May I know more details about how to reproduce this issue? > > > As the iavf PMD does not support > > > RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP (i40e does) > > > > OK, not sure if the patch 4/4 answered my question 😊 > > > > should I squash patch 3, 4 into one? , for my understanding patch 3 doesn't > appear to be a bug fix unless we announce > RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP. > > > You may have a point. I was experimenting with queue reconfiguration which > is where I hit this issue. > However, even without queue reconfig support, the device still needs to > support queue-stop followed by queue-start, I think, and there may still be an > issue there - I'll have to check. Yes, queue start / stop should be supported. Btw, I didn't see mbuf_initializer has been reset during queue stop / start, it might be a different issue. > /Bruce ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-09-05 0:02 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20230830155919.592390-1-bruce.richardson@intel.com>
2023-08-30 15:59 ` [PATCH 1/4] net/i40e: fix buffer leak on Rx reconfiguration Bruce Richardson
2023-08-30 15:59 ` [PATCH 2/4] net/iavf: fix buffer leak on Tx queue stop Bruce Richardson
2023-08-30 15:59 ` [PATCH 3/4] net/iavf: fix restart of Rx queue on reconfigure Bruce Richardson
[not found] ` <20230831123337.871496-1-bruce.richardson@intel.com>
2023-08-31 12:33 ` [PATCH v2 1/4] net/i40e: fix buffer leak on Rx reconfiguration Bruce Richardson
2023-09-01 7:12 ` Zhang, Qi Z
2023-08-31 12:33 ` [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop Bruce Richardson
2023-09-04 2:17 ` Lu, Wenzhuo
2023-09-04 2:30 ` Zhang, Qi Z
2023-08-31 12:33 ` [PATCH v2 3/4] net/iavf: fix restart of Rx queue on reconfigure Bruce Richardson
2023-09-04 1:15 ` Zhang, Qi Z
2023-09-04 1:30 ` Zhang, Qi Z
2023-09-04 7:54 ` Bruce Richardson
2023-09-05 0:02 ` 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).