* [PATCH 1/2] net/mlx5: fix invalid entry in shared RXQs list
2022-02-14 9:00 [PATCH 0/2] fix shared RxQ creation Michael Baum
@ 2022-02-14 9:00 ` Michael Baum
2022-02-14 9:00 ` [PATCH 2/2] net/mlx5: optimize RxQ creation Michael Baum
2022-02-23 8:52 ` [PATCH 0/2] fix shared " Raslan Darawsheh
2 siblings, 0 replies; 4+ messages in thread
From: Michael Baum @ 2022-02-14 9:00 UTC (permalink / raw)
To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Viacheslav Ovsiienko, stable
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
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] net/mlx5: optimize RxQ creation
2022-02-14 9:00 [PATCH 0/2] fix shared RxQ creation Michael Baum
2022-02-14 9:00 ` [PATCH 1/2] net/mlx5: fix invalid entry in shared RXQs list Michael Baum
@ 2022-02-14 9:00 ` Michael Baum
2022-02-23 8:52 ` [PATCH 0/2] fix shared " Raslan Darawsheh
2 siblings, 0 replies; 4+ messages in thread
From: Michael Baum @ 2022-02-14 9:00 UTC (permalink / raw)
To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Viacheslav Ovsiienko
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
^ permalink raw reply [flat|nested] 4+ messages in thread