[PATCH 0/2] fix shared RxQ creation

[PATCH 0/2] fix shared RxQ creation

[Michael Baum]

Fix error flow in RxQ creation and optimize it.

Michael Baum (2):
  net/mlx5: fix invalid entry in shared RXQs list
  net/mlx5: optimize RxQ creation

 drivers/net/mlx5/mlx5_rx.h  |  3 +--
 drivers/net/mlx5/mlx5_rxq.c | 40 ++++++++++++++++---------------------
 2 files changed, 18 insertions(+), 25 deletions(-)

-- 
2.25.1

[PATCH 1/2] net/mlx5: fix invalid entry in shared RXQs list

[Michael Baum]

The mlx5_rxq_new function creates control structure and if it from
shared group, it is inserted into the shared RXQs list.

After that, there are some validations which in case they fail, RxQ
control object is released.
In these cases, invalid pointer to the object still in the list, and
access it may cause a crash.

Move the list insertion to the end of the function where the RxQ control
object is surely valid.

Fixes: 09c2555303be ("net/mlx5: support shared Rx queue")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx5/mlx5_rxq.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 580d7ae868..fe72cf49d3 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1719,12 +1719,6 @@ mlx5_rxq_new(struct rte_eth_dev *dev, struct mlx5_rxq_priv *rxq,
 		return NULL;
 	}
 	LIST_INIT(&tmpl->owners);
-	if (conf->share_group > 0) {
-		tmpl->rxq.shared = 1;
-		tmpl->share_group = conf->share_group;
-		tmpl->share_qid = conf->share_qid;
-		LIST_INSERT_HEAD(&priv->sh->shared_rxqs, tmpl, share_entry);
-	}
 	rxq->ctrl = tmpl;
 	LIST_INSERT_HEAD(&tmpl->owners, rxq, owner_entry);
 	MLX5_ASSERT(n_seg && n_seg <= MLX5_MAX_RXQ_NSEG);
@@ -1933,6 +1927,12 @@ mlx5_rxq_new(struct rte_eth_dev *dev, struct mlx5_rxq_priv *rxq,
 	tmpl->rxq.mprq_bufs =
 		(struct mlx5_mprq_buf *(*)[desc])(*tmpl->rxq.elts + desc_n);
 	tmpl->rxq.idx = idx;
+	if (conf->share_group > 0) {
+		tmpl->rxq.shared = 1;
+		tmpl->share_group = conf->share_group;
+		tmpl->share_qid = conf->share_qid;
+		LIST_INSERT_HEAD(&priv->sh->shared_rxqs, tmpl, share_entry);
+	}
 	LIST_INSERT_HEAD(&priv->rxqsctrl, tmpl, next);
 	return tmpl;
 error:
-- 
2.25.1

[PATCH 2/2] net/mlx5: optimize RxQ creation

[Michael Baum]

Recently shared RxQ has been introduced. All shared Rx queues with same
group and queue ID share the same rxq_ctrl, but each one has
mlx5_rxq_priv structure.
The mlx5_rx_queue_setup generates a new rxq_priv structure, and looks
for a rxq_ctrl structure to refer to. If there is already a compatible
rxq_ctrl structure it refers it, otherwise it calls the mlx5_rxq_new
function that generates a new one.

This patch makes mlx5_rxq_new function "standalone", it generates a
rxq_ctrl structure regardless to specific rxq_priv structure. All
operations on the rxq_ctrl structure that depend on the new rxq_priv
structure are performed in the mlx5_rx_queue_setup function, at the same
place for either a new rxq_ctrl structure or an existing rxq_ctrl
structure.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx5/mlx5_rx.h  |  3 +--
 drivers/net/mlx5/mlx5_rxq.c | 28 +++++++++++-----------------
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rx.h b/drivers/net/mlx5/mlx5_rx.h
index 7e417819f7..38335fd744 100644
--- a/drivers/net/mlx5/mlx5_rx.h
+++ b/drivers/net/mlx5/mlx5_rx.h
@@ -204,8 +204,7 @@ void mlx5_rx_intr_vec_disable(struct rte_eth_dev *dev);
 int mlx5_rx_intr_enable(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int mlx5_rx_intr_disable(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int mlx5_rxq_obj_verify(struct rte_eth_dev *dev);
-struct mlx5_rxq_ctrl *mlx5_rxq_new(struct rte_eth_dev *dev,
-				   struct mlx5_rxq_priv *rxq,
+struct mlx5_rxq_ctrl *mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t idx,
 				   uint16_t desc, unsigned int socket,
 				   const struct rte_eth_rxconf *conf,
 				   const struct rte_eth_rxseg_split *rx_seg,
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index fe72cf49d3..eaa48487cc 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -910,25 +910,23 @@ mlx5_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 		rte_errno = ENOMEM;
 		return -rte_errno;
 	}
-	rxq->priv = priv;
-	rxq->idx = idx;
-	(*priv->rxq_privs)[idx] = rxq;
-	if (rxq_ctrl != NULL) {
-		/* Join owner list. */
-		LIST_INSERT_HEAD(&rxq_ctrl->owners, rxq, owner_entry);
-		rxq->ctrl = rxq_ctrl;
-	} else {
-		rxq_ctrl = mlx5_rxq_new(dev, rxq, desc, socket, conf, rx_seg,
+	if (rxq_ctrl == NULL) {
+		rxq_ctrl = mlx5_rxq_new(dev, idx, desc, socket, conf, rx_seg,
 					n_seg);
 		if (rxq_ctrl == NULL) {
 			DRV_LOG(ERR, "port %u unable to allocate rx queue index %u",
 				dev->data->port_id, idx);
 			mlx5_free(rxq);
-			(*priv->rxq_privs)[idx] = NULL;
 			rte_errno = ENOMEM;
 			return -rte_errno;
 		}
 	}
+	rxq->priv = priv;
+	rxq->idx = idx;
+	(*priv->rxq_privs)[idx] = rxq;
+	/* Join owner list. */
+	LIST_INSERT_HEAD(&rxq_ctrl->owners, rxq, owner_entry);
+	rxq->ctrl = rxq_ctrl;
 	mlx5_rxq_ref(dev, idx);
 	DRV_LOG(DEBUG, "port %u adding Rx queue %u to list",
 		dev->data->port_id, idx);
@@ -1660,8 +1658,8 @@ mlx5_mprq_prepare(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
  *
  * @param dev
  *   Pointer to Ethernet device.
- * @param rxq
- *   RX queue private data.
+ * @param idx
+ *   RX queue index.
  * @param desc
  *   Number of descriptors to configure in queue.
  * @param socket
@@ -1671,12 +1669,10 @@ mlx5_mprq_prepare(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
  *   A DPDK queue object on success, NULL otherwise and rte_errno is set.
  */
 struct mlx5_rxq_ctrl *
-mlx5_rxq_new(struct rte_eth_dev *dev, struct mlx5_rxq_priv *rxq,
-	     uint16_t desc,
+mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 	     unsigned int socket, const struct rte_eth_rxconf *conf,
 	     const struct rte_eth_rxseg_split *rx_seg, uint16_t n_seg)
 {
-	uint16_t idx = rxq->idx;
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_rxq_ctrl *tmpl;
 	unsigned int mb_len = rte_pktmbuf_data_room_size(rx_seg[0].mp);
@@ -1719,8 +1715,6 @@ mlx5_rxq_new(struct rte_eth_dev *dev, struct mlx5_rxq_priv *rxq,
 		return NULL;
 	}
 	LIST_INIT(&tmpl->owners);
-	rxq->ctrl = tmpl;
-	LIST_INSERT_HEAD(&tmpl->owners, rxq, owner_entry);
 	MLX5_ASSERT(n_seg && n_seg <= MLX5_MAX_RXQ_NSEG);
 	/*
 	 * Save the original segment configuration in the shared queue
-- 
2.25.1

RE: [PATCH 0/2] fix shared RxQ creation

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Monday, February 14, 2022 11:00 AM
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Subject: [PATCH 0/2] fix shared RxQ creation
> 
> Fix error flow in RxQ creation and optimize it.
> 
> Michael Baum (2):
>   net/mlx5: fix invalid entry in shared RXQs list
>   net/mlx5: optimize RxQ creation
> 
>  drivers/net/mlx5/mlx5_rx.h  |  3 +--
>  drivers/net/mlx5/mlx5_rxq.c | 40 ++++++++++++++++---------------------
>  2 files changed, 18 insertions(+), 25 deletions(-)
> 
> --
> 2.25.1

Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh