fix shared RxQ creation

PATCH In review Michael Baum 2 participants 4 messages Feb 14, 2022 – Feb 23, 2022 Network drivers

lore

Share & tools

lore

Michael Baum

author

2022-02-14 09:00 UTC #0 lore raw

cover 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(-)

Signature · 1 line

2.25.1

Michael Baum

author

2022-02-14 09:00 UTC #1 lore raw

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

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 [email protected]

Signed-off-by Michael Baum <[email protected]>

Acked-by Viacheslav Ovsiienko <[email protected]>

---
 drivers/net/mlx5/mlx5_rxq.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/net/mlx5/mlx5_rxq.c +6 -6

index 580d7ae868..fe72cf49d3 100644
@@ -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:

Signature · 1 line

2.25.1

Michael Baum

author

2022-02-14 09:00 UTC #2 lore raw

PATCH 2/2 net/mlx5: optimize RxQ creation

In reply to 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 <[email protected]>

Acked-by Viacheslav Ovsiienko <[email protected]>

---
 drivers/net/mlx5/mlx5_rx.h  |  3 +--
 drivers/net/mlx5/mlx5_rxq.c | 28 +++++++++++-----------------
 2 files changed, 12 insertions(+), 19 deletions(-)

drivers/net/mlx5/mlx5_rx.h +1 -2

index 7e417819f7..38335fd744 100644
@@ -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,

drivers/net/mlx5/mlx5_rxq.c +11 ~~-17~~

index fe72cf49d3..eaa48487cc 100644
@@ -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

Signature · 1 line

2.25.1

Raslan Darawsheh

2022-02-23 08:52 UTC #3 lore raw

In reply to Michael Baum

Hi,

Quoted 14 lines

-----Original Message-----
From: Michael Baum <[email protected]>
Sent: Monday, February 14, 2022 11:00 AM
To: [email protected]
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Slava Ovsiienko <[email protected]>
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

Quoted 6 lines

 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