DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix device reference in secondary process
@ 2018-05-02  6:13 Yongseok Koh
  2018-05-02  6:53 ` Nélio Laranjeiro
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Yongseok Koh @ 2018-05-02  6:13 UTC (permalink / raw)
  To: adrien.mazarguil, nelio.laranjeiro; +Cc: dev, Yongseok Koh, stable

rte_eth_devices[] is not shared between primary and secondary process, but
a static array to each process. The backward pointer of device (priv->dev)
must be reset when a secondary process attaches to a device.

Fixes: f8b9a3bad467 ("net/mlx5: install a socket to exchange a file descriptor")
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 8f983061a..f606e3dd4 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -799,6 +799,12 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			}
 			eth_dev->device = &pci_dev->device;
 			eth_dev->dev_ops = &mlx5_dev_sec_ops;
+			/*
+			 * rte_eth_devices[] is not shared but static to each
+			 * process. The backward pointer should be reset.
+			 */
+			priv = eth_dev->data->dev_private;
+			priv->dev = eth_dev;
 			err = mlx5_uar_init_secondary(eth_dev);
 			if (err)
 				goto error;
-- 
2.11.0

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

* Re: [dpdk-dev] [PATCH] net/mlx5: fix device reference in secondary process
  2018-05-02  6:13 [dpdk-dev] [PATCH] net/mlx5: fix device reference in secondary process Yongseok Koh
@ 2018-05-02  6:53 ` Nélio Laranjeiro
  2018-05-02  7:12   ` Yongseok Koh
  2018-05-02 23:10 ` [dpdk-dev] [PATCH v2] net/mlx5: change device reference for " Yongseok Koh
  2018-05-09 11:04 ` [dpdk-dev] [PATCH v3] " Yongseok Koh
  2 siblings, 1 reply; 8+ messages in thread
From: Nélio Laranjeiro @ 2018-05-02  6:53 UTC (permalink / raw)
  To: Yongseok Koh; +Cc: adrien.mazarguil, dev, stable

On Tue, May 01, 2018 at 11:13:20PM -0700, Yongseok Koh wrote:
> rte_eth_devices[] is not shared between primary and secondary process, but
> a static array to each process. The backward pointer of device (priv->dev)
> must be reset when a secondary process attaches to a device.
> 
> Fixes: f8b9a3bad467 ("net/mlx5: install a socket to exchange a file descriptor")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  drivers/net/mlx5/mlx5.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 8f983061a..f606e3dd4 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -799,6 +799,12 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>  			}
>  			eth_dev->device = &pci_dev->device;
>  			eth_dev->dev_ops = &mlx5_dev_sec_ops;
> +			/*
> +			 * rte_eth_devices[] is not shared but static to each
> +			 * process. The backward pointer should be reset.
> +			 */
> +			priv = eth_dev->data->dev_private;
> +			priv->dev = eth_dev;
>  			err = mlx5_uar_init_secondary(eth_dev);
>  			if (err)
>  				goto error;
> -- 
> 2.11.0
> 

-- 
Nélio Laranjeiro
6WIND

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

* Re: [dpdk-dev] [PATCH] net/mlx5: fix device reference in secondary process
  2018-05-02  6:53 ` Nélio Laranjeiro
@ 2018-05-02  7:12   ` Yongseok Koh
  0 siblings, 0 replies; 8+ messages in thread
From: Yongseok Koh @ 2018-05-02  7:12 UTC (permalink / raw)
  To: Nélio Laranjeiro; +Cc: Adrien Mazarguil, dev, stable

> On May 1, 2018, at 11:53 PM, Nélio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:
> 
> On Tue, May 01, 2018 at 11:13:20PM -0700, Yongseok Koh wrote:
>> rte_eth_devices[] is not shared between primary and secondary process, but
>> a static array to each process. The backward pointer of device (priv->dev)
>> must be reset when a secondary process attaches to a device.
>> 
>> Fixes: f8b9a3bad467 ("net/mlx5: install a socket to exchange a file descriptor")
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Sorry, self NACK.
priv->dev is shared, so if it is changed by secondary process, primary will get
affected. Will come up with a different idea...

Thanks,
Yongseok


>> ---
>> drivers/net/mlx5/mlx5.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>> 
>> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
>> index 8f983061a..f606e3dd4 100644
>> --- a/drivers/net/mlx5/mlx5.c
>> +++ b/drivers/net/mlx5/mlx5.c
>> @@ -799,6 +799,12 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>> 			}
>> 			eth_dev->device = &pci_dev->device;
>> 			eth_dev->dev_ops = &mlx5_dev_sec_ops;
>> +			/*
>> +			 * rte_eth_devices[] is not shared but static to each
>> +			 * process. The backward pointer should be reset.
>> +			 */
>> +			priv = eth_dev->data->dev_private;
>> +			priv->dev = eth_dev;
>> 			err = mlx5_uar_init_secondary(eth_dev);
>> 			if (err)
>> 				goto error;
>> -- 
>> 2.11.0
>> 
> 
> -- 
> Nélio Laranjeiro
> 6WIND


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

* [dpdk-dev] [PATCH v2] net/mlx5: change device reference for secondary process
  2018-05-02  6:13 [dpdk-dev] [PATCH] net/mlx5: fix device reference in secondary process Yongseok Koh
  2018-05-02  6:53 ` Nélio Laranjeiro
@ 2018-05-02 23:10 ` Yongseok Koh
  2018-05-06  6:25   ` Shahaf Shuler
  2018-05-09 11:04 ` [dpdk-dev] [PATCH v3] " Yongseok Koh
  2 siblings, 1 reply; 8+ messages in thread
From: Yongseok Koh @ 2018-05-02 23:10 UTC (permalink / raw)
  To: adrien.mazarguil, nelio.laranjeiro; +Cc: dev, Yongseok Koh

rte_eth_devices[] is not shared between primary and secondary process, but
a static array to each process. The reverse pointer of device (priv->dev)
is invalid. Instead, priv has the pointer to shared data of the device,
  struct rte_eth_dev_data *dev_data;

Two macros are added,
  #define port_id(priv) ((priv)->dev_data->port_id)
  #define eth_dev(priv) (&rte_eth_devices[(priv)->dev_data->port_id])

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5.c      |  2 +-
 drivers/net/mlx5/mlx5.h      |  5 ++++-
 drivers/net/mlx5/mlx5_flow.c | 21 +++++++++------------
 drivers/net/mlx5/mlx5_mr.c   | 19 +++++++++----------
 drivers/net/mlx5/mlx5_rxq.c  | 30 +++++++++++++++---------------
 drivers/net/mlx5/mlx5_txq.c  | 15 +++++++--------
 6 files changed, 45 insertions(+), 47 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 8f983061a..6c4a571ab 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -946,7 +946,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			goto port_error;
 		}
 		eth_dev->data->dev_private = priv;
-		priv->dev = eth_dev;
+		priv->dev_data = eth_dev->data;
 		eth_dev->data->mac_addrs = priv->mac;
 		eth_dev->device = &pci_dev->device;
 		rte_eth_copy_pci_info(eth_dev, pci_dev);
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index c4d1d456b..3ab16bfa2 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -121,7 +121,7 @@ struct mlx5_verbs_alloc_ctx {
 };
 
 struct priv {
-	struct rte_eth_dev *dev; /* Ethernet device of master process. */
+	struct rte_eth_dev_data *dev_data;  /* Pointer to device data. */
 	struct ibv_context *ctx; /* Verbs context. */
 	struct ibv_device_attr_ex device_attr; /* Device properties. */
 	struct ibv_pd *pd; /* Protection Domain. */
@@ -168,6 +168,9 @@ struct priv {
 	uint32_t nl_sn; /* Netlink message sequence number. */
 };
 
+#define port_id(priv) ((priv)->dev_data->port_id)
+#define eth_dev(priv) (&rte_eth_devices[(priv)->dev_data->port_id])
+
 /* mlx5.c */
 
 int mlx5_getenv_int(const char *);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 129311d50..2e4bfac58 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2061,7 +2061,7 @@ mlx5_flow_create_action_queue_drop(struct rte_eth_dev *dev,
 		parser->queue[HASH_RXQ_ETH].ibv_attr;
 	if (parser->count)
 		flow->cs = parser->cs;
-	if (!priv->dev->data->dev_started)
+	if (!dev->data->dev_started)
 		return 0;
 	parser->queue[HASH_RXQ_ETH].ibv_attr = NULL;
 	flow->frxq[HASH_RXQ_ETH].ibv_flow =
@@ -2113,7 +2113,6 @@ mlx5_flow_create_action_queue_rss(struct rte_eth_dev *dev,
 				  struct rte_flow *flow,
 				  struct rte_flow_error *error)
 {
-	struct priv *priv = dev->data->dev_private;
 	unsigned int i;
 
 	for (i = 0; i != hash_rxq_init_n; ++i) {
@@ -2122,7 +2121,7 @@ mlx5_flow_create_action_queue_rss(struct rte_eth_dev *dev,
 		flow->frxq[i].ibv_attr = parser->queue[i].ibv_attr;
 		parser->queue[i].ibv_attr = NULL;
 		flow->frxq[i].hash_fields = parser->queue[i].hash_fields;
-		if (!priv->dev->data->dev_started)
+		if (!dev->data->dev_started)
 			continue;
 		flow->frxq[i].hrxq =
 			mlx5_hrxq_get(dev,
@@ -2268,7 +2267,7 @@ mlx5_flow_create_action_queue(struct rte_eth_dev *dev,
 			      struct rte_flow *flow,
 			      struct rte_flow_error *error)
 {
-	struct priv *priv = dev->data->dev_private;
+	struct priv *priv __rte_unused = dev->data->dev_private;
 	int ret;
 	unsigned int i;
 	unsigned int flows_n = 0;
@@ -2281,7 +2280,7 @@ mlx5_flow_create_action_queue(struct rte_eth_dev *dev,
 		goto error;
 	if (parser->count)
 		flow->cs = parser->cs;
-	if (!priv->dev->data->dev_started)
+	if (!dev->data->dev_started)
 		return 0;
 	for (i = 0; i != hash_rxq_init_n; ++i) {
 		if (!flow->frxq[i].hrxq)
@@ -3124,9 +3123,9 @@ mlx5_flow_isolate(struct rte_eth_dev *dev,
 	}
 	priv->isolated = !!enable;
 	if (enable)
-		priv->dev->dev_ops = &mlx5_dev_ops_isolate;
+		dev->dev_ops = &mlx5_dev_ops_isolate;
 	else
-		priv->dev->dev_ops = &mlx5_dev_ops;
+		dev->dev_ops = &mlx5_dev_ops;
 	return 0;
 }
 
@@ -3514,11 +3513,10 @@ mlx5_fdir_filter_flush(struct rte_eth_dev *dev)
 static void
 mlx5_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info)
 {
-	struct priv *priv = dev->data->dev_private;
 	struct rte_eth_fdir_masks *mask =
-		&priv->dev->data->dev_conf.fdir_conf.mask;
+		&dev->data->dev_conf.fdir_conf.mask;
 
-	fdir_info->mode = priv->dev->data->dev_conf.fdir_conf.mode;
+	fdir_info->mode = dev->data->dev_conf.fdir_conf.mode;
 	fdir_info->guarant_spc = 0;
 	rte_memcpy(&fdir_info->mask, mask, sizeof(fdir_info->mask));
 	fdir_info->max_flexpayload = 0;
@@ -3546,9 +3544,8 @@ static int
 mlx5_fdir_ctrl_func(struct rte_eth_dev *dev, enum rte_filter_op filter_op,
 		    void *arg)
 {
-	struct priv *priv = dev->data->dev_private;
 	enum rte_fdir_mode fdir_mode =
-		priv->dev->data->dev_conf.fdir_conf.mode;
+		dev->data->dev_conf.fdir_conf.mode;
 
 	if (filter_op == RTE_ETH_FILTER_NOP)
 		return 0;
diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
index 1b4c7ec5d..7a337d0c3 100644
--- a/drivers/net/mlx5/mlx5_mr.c
+++ b/drivers/net/mlx5/mlx5_mr.c
@@ -105,8 +105,8 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 	rte_spinlock_lock(&txq_ctrl->priv->mr_lock);
 	/* Add a new entry, register MR first. */
 	DRV_LOG(DEBUG, "port %u discovered new memory pool \"%s\" (%p)",
-		txq_ctrl->priv->dev->data->port_id, mp->name, (void *)mp);
-	dev = txq_ctrl->priv->dev;
+		port_id(txq_ctrl->priv), mp->name, (void *)mp);
+	dev = eth_dev(txq_ctrl->priv);
 	mr = mlx5_mr_get(dev, mp);
 	if (mr == NULL) {
 		if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
@@ -114,8 +114,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 				"port %u using unregistered mempool 0x%p(%s)"
 				" in secondary process, please create mempool"
 				" before rte_eth_dev_start()",
-				txq_ctrl->priv->dev->data->port_id,
-				(void *)mp, mp->name);
+				port_id(txq_ctrl->priv), (void *)mp, mp->name);
 			rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
 			rte_errno = ENOTSUP;
 			return NULL;
@@ -126,7 +125,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 		DRV_LOG(DEBUG,
 			"port %u unable to configure memory region,"
 			" ibv_reg_mr() failed.",
-			txq_ctrl->priv->dev->data->port_id);
+			port_id(txq_ctrl->priv));
 		rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
 		return NULL;
 	}
@@ -135,7 +134,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 		DRV_LOG(DEBUG,
 			"port %u memory region <-> memory pool table full, "
 			" dropping oldest entry",
-			txq_ctrl->priv->dev->data->port_id);
+			port_id(txq_ctrl->priv));
 		--idx;
 		mlx5_mr_release(txq->mp2mr[0]);
 		memmove(&txq->mp2mr[0], &txq->mp2mr[1],
@@ -146,7 +145,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 	DRV_LOG(DEBUG,
 		"port %u new memory region lkey for MP \"%s\" (%p): 0x%08"
 		PRIu32,
-		txq_ctrl->priv->dev->data->port_id, mp->name, (void *)mp,
+		port_id(txq_ctrl->priv), mp->name, (void *)mp,
 		txq_ctrl->txq.mp2mr[idx]->lkey);
 	rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
 	return mr;
@@ -207,15 +206,15 @@ mlx5_mp2mr_iter(struct rte_mempool *mp, void *arg)
 	if (rte_mempool_obj_iter(mp, txq_mp2mr_mbuf_check, &data) == 0 ||
 			data.ret == -1)
 		return;
-	mr = mlx5_mr_get(priv->dev, mp);
+	mr = mlx5_mr_get(eth_dev(priv), mp);
 	if (mr) {
 		mlx5_mr_release(mr);
 		return;
 	}
-	mr = mlx5_mr_new(priv->dev, mp);
+	mr = mlx5_mr_new(eth_dev(priv), mp);
 	if (!mr)
 		DRV_LOG(ERR, "port %u cannot create memory region: %s",
-			priv->dev->data->port_id, strerror(rte_errno));
+			port_id(priv), strerror(rte_errno));
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 126412ddd..a85b628fe 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -78,7 +78,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
 		buf = rte_pktmbuf_alloc(rxq_ctrl->rxq.mp);
 		if (buf == NULL) {
 			DRV_LOG(ERR, "port %u empty mbuf pool",
-				rxq_ctrl->priv->dev->data->port_id);
+				port_id(rxq_ctrl->priv));
 			rte_errno = ENOMEM;
 			goto error;
 		}
@@ -122,7 +122,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
 	DRV_LOG(DEBUG,
 		"port %u Rx queue %u allocated and configured %u segments"
 		" (max %u packets)",
-		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx, elts_n,
+		port_id(rxq_ctrl->priv), rxq_ctrl->idx, elts_n,
 		elts_n / (1 << rxq_ctrl->rxq.sges_n));
 	return 0;
 error:
@@ -134,7 +134,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
 		(*rxq_ctrl->rxq.elts)[i] = NULL;
 	}
 	DRV_LOG(DEBUG, "port %u Rx queue %u failed, freed everything",
-		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
+		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
 	rte_errno = err; /* Restore rte_errno. */
 	return -rte_errno;
 }
@@ -155,7 +155,7 @@ rxq_free_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
 	uint16_t i;
 
 	DRV_LOG(DEBUG, "port %u Rx queue %u freeing WRs",
-		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
+		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
 	if (rxq->elts == NULL)
 		return;
 	/**
@@ -186,7 +186,7 @@ void
 mlx5_rxq_cleanup(struct mlx5_rxq_ctrl *rxq_ctrl)
 {
 	DRV_LOG(DEBUG, "port %u cleaning up Rx queue %u",
-		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
+		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
 	if (rxq_ctrl->ibv)
 		mlx5_rxq_ibv_release(rxq_ctrl->ibv);
 	memset(rxq_ctrl, 0, sizeof(*rxq_ctrl));
@@ -354,11 +354,11 @@ mlx5_rx_queue_release(void *dpdk_rxq)
 		return;
 	rxq_ctrl = container_of(rxq, struct mlx5_rxq_ctrl, rxq);
 	priv = rxq_ctrl->priv;
-	if (!mlx5_rxq_releasable(priv->dev, rxq_ctrl->rxq.stats.idx))
+	if (!mlx5_rxq_releasable(eth_dev(priv), rxq_ctrl->rxq.stats.idx))
 		rte_panic("port %u Rx queue %u is still used by a flow and"
-			  " cannot be removed\n", priv->dev->data->port_id,
-			  rxq_ctrl->idx);
-	mlx5_rxq_release(priv->dev, rxq_ctrl->rxq.stats.idx);
+			  " cannot be removed\n",
+			  port_id(priv), rxq_ctrl->idx);
+	mlx5_rxq_release(eth_dev(priv), rxq_ctrl->rxq.stats.idx);
 }
 
 /**
@@ -378,9 +378,9 @@ mlx5_rx_intr_vec_enable(struct rte_eth_dev *dev)
 	unsigned int rxqs_n = priv->rxqs_n;
 	unsigned int n = RTE_MIN(rxqs_n, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
 	unsigned int count = 0;
-	struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
+	struct rte_intr_handle *intr_handle = dev->intr_handle;
 
-	if (!priv->dev->data->dev_conf.intr_conf.rxq)
+	if (!dev->data->dev_conf.intr_conf.rxq)
 		return 0;
 	mlx5_rx_intr_vec_disable(dev);
 	intr_handle->intr_vec = malloc(n * sizeof(intr_handle->intr_vec[0]));
@@ -452,12 +452,12 @@ void
 mlx5_rx_intr_vec_disable(struct rte_eth_dev *dev)
 {
 	struct priv *priv = dev->data->dev_private;
-	struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
+	struct rte_intr_handle *intr_handle = dev->intr_handle;
 	unsigned int i;
 	unsigned int rxqs_n = priv->rxqs_n;
 	unsigned int n = RTE_MIN(rxqs_n, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
 
-	if (!priv->dev->data->dev_conf.intr_conf.rxq)
+	if (!dev->data->dev_conf.intr_conf.rxq)
 		return;
 	if (!intr_handle->intr_vec)
 		goto free;
@@ -897,7 +897,7 @@ mlx5_rxq_ibv_release(struct mlx5_rxq_ibv *rxq_ibv)
 	if (!ret)
 		rxq_ibv->mr = NULL;
 	DRV_LOG(DEBUG, "port %u Verbs Rx queue %u: refcnt %d",
-		rxq_ibv->rxq_ctrl->priv->dev->data->port_id,
+		port_id(rxq_ibv->rxq_ctrl->priv),
 		rxq_ibv->rxq_ctrl->idx, rte_atomic32_read(&rxq_ibv->refcnt));
 	if (rte_atomic32_dec_and_test(&rxq_ibv->refcnt)) {
 		rxq_free_elts(rxq_ibv->rxq_ctrl);
@@ -990,7 +990,7 @@ mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 		return NULL;
 	}
 	tmpl->socket = socket;
-	if (priv->dev->data->dev_conf.intr_conf.rxq)
+	if (dev->data->dev_conf.intr_conf.rxq)
 		tmpl->irq = 1;
 	/* Enable scattered packets support for this queue if necessary. */
 	assert(mb_len >= RTE_PKTMBUF_HEADROOM);
diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index 44358744d..29959b4c7 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -48,7 +48,7 @@ txq_alloc_elts(struct mlx5_txq_ctrl *txq_ctrl)
 	for (i = 0; (i != elts_n); ++i)
 		(*txq_ctrl->txq.elts)[i] = NULL;
 	DRV_LOG(DEBUG, "port %u Tx queue %u allocated and configured %u WRs",
-		txq_ctrl->priv->dev->data->port_id, txq_ctrl->idx, elts_n);
+		port_id(txq_ctrl->priv), txq_ctrl->idx, elts_n);
 	txq_ctrl->txq.elts_head = 0;
 	txq_ctrl->txq.elts_tail = 0;
 	txq_ctrl->txq.elts_comp = 0;
@@ -70,7 +70,7 @@ txq_free_elts(struct mlx5_txq_ctrl *txq_ctrl)
 	struct rte_mbuf *(*elts)[elts_n] = txq_ctrl->txq.elts;
 
 	DRV_LOG(DEBUG, "port %u Tx queue %u freeing WRs",
-		txq_ctrl->priv->dev->data->port_id, txq_ctrl->idx);
+		port_id(txq_ctrl->priv), txq_ctrl->idx);
 	txq_ctrl->txq.elts_head = 0;
 	txq_ctrl->txq.elts_tail = 0;
 	txq_ctrl->txq.elts_comp = 0;
@@ -255,9 +255,9 @@ mlx5_tx_queue_release(void *dpdk_txq)
 	priv = txq_ctrl->priv;
 	for (i = 0; (i != priv->txqs_n); ++i)
 		if ((*priv->txqs)[i] == txq) {
-			mlx5_txq_release(priv->dev, i);
+			mlx5_txq_release(eth_dev(priv), i);
 			DRV_LOG(DEBUG, "port %u removing Tx queue %u from list",
-				priv->dev->data->port_id, txq_ctrl->idx);
+				port_id(priv), txq_ctrl->idx);
 			break;
 		}
 }
@@ -618,7 +618,7 @@ mlx5_txq_ibv_release(struct mlx5_txq_ibv *txq_ibv)
 {
 	assert(txq_ibv);
 	DRV_LOG(DEBUG, "port %u Verbs Tx queue %u: refcnt %d",
-		txq_ibv->txq_ctrl->priv->dev->data->port_id,
+		port_id(txq_ibv->txq_ctrl->priv),
 		txq_ibv->txq_ctrl->idx, rte_atomic32_read(&txq_ibv->refcnt));
 	if (rte_atomic32_dec_and_test(&txq_ibv->refcnt)) {
 		claim_zero(mlx5_glue->destroy_qp(txq_ibv->qp));
@@ -685,7 +685,7 @@ txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)
 	unsigned int txqs_inline;
 	unsigned int inline_max_packet_sz;
 	eth_tx_burst_t tx_pkt_burst =
-		mlx5_select_tx_function(txq_ctrl->priv->dev);
+		mlx5_select_tx_function(eth_dev(priv));
 	int is_empw_func = is_empw_burst_func(tx_pkt_burst);
 	int tso = !!(txq_ctrl->txq.offloads & (DEV_TX_OFFLOAD_TCP_TSO |
 					       DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
@@ -759,8 +759,7 @@ txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)
 			DRV_LOG(WARNING,
 				"port %u txq inline is too large (%d) setting"
 				" it to the maximum possible: %d\n",
-				priv->dev->data->port_id, txq_inline,
-				max_inline);
+				port_id(priv), txq_inline, max_inline);
 			txq_ctrl->txq.max_inline = max_inline /
 						   RTE_CACHE_LINE_SIZE;
 		}
-- 
2.11.0

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

* Re: [dpdk-dev] [PATCH v2] net/mlx5: change device reference for secondary process
  2018-05-02 23:10 ` [dpdk-dev] [PATCH v2] net/mlx5: change device reference for " Yongseok Koh
@ 2018-05-06  6:25   ` Shahaf Shuler
  2018-05-07  6:46     ` Yongseok Koh
  0 siblings, 1 reply; 8+ messages in thread
From: Shahaf Shuler @ 2018-05-06  6:25 UTC (permalink / raw)
  To: Yongseok Koh, Adrien Mazarguil, Nélio Laranjeiro; +Cc: dev, Yongseok Koh

Hi Koh,

See some small comments. 

Thursday, May 3, 2018 2:11 AM, Yongseok Koh:
> Subject: [dpdk-dev] [PATCH v2] net/mlx5: change device reference for
> secondary process
> 
> rte_eth_devices[] is not shared between primary and secondary process,
> but a static array to each process. The reverse pointer of device (priv->dev) is
> invalid. Instead, priv has the pointer to shared data of the device,
>   struct rte_eth_dev_data *dev_data;
> 
> Two macros are added,
>   #define port_id(priv) ((priv)->dev_data->port_id)
>   #define eth_dev(priv) (&rte_eth_devices[(priv)->dev_data->port_id])
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5.c      |  2 +-
>  drivers/net/mlx5/mlx5.h      |  5 ++++-
>  drivers/net/mlx5/mlx5_flow.c | 21 +++++++++------------
>  drivers/net/mlx5/mlx5_mr.c   | 19 +++++++++----------
>  drivers/net/mlx5/mlx5_rxq.c  | 30 +++++++++++++++---------------
> drivers/net/mlx5/mlx5_txq.c  | 15 +++++++--------
>  6 files changed, 45 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 8f983061a..6c4a571ab 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -946,7 +946,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv
> __rte_unused,
>  			goto port_error;
>  		}
>  		eth_dev->data->dev_private = priv;
> -		priv->dev = eth_dev;
> +		priv->dev_data = eth_dev->data;
>  		eth_dev->data->mac_addrs = priv->mac;
>  		eth_dev->device = &pci_dev->device;
>  		rte_eth_copy_pci_info(eth_dev, pci_dev); diff --git
> a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> c4d1d456b..3ab16bfa2 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -121,7 +121,7 @@ struct mlx5_verbs_alloc_ctx {  };
> 
>  struct priv {
> -	struct rte_eth_dev *dev; /* Ethernet device of master process. */
> +	struct rte_eth_dev_data *dev_data;  /* Pointer to device data. */
>  	struct ibv_context *ctx; /* Verbs context. */
>  	struct ibv_device_attr_ex device_attr; /* Device properties. */
>  	struct ibv_pd *pd; /* Protection Domain. */ @@ -168,6 +168,9 @@
> struct priv {
>  	uint32_t nl_sn; /* Netlink message sequence number. */  };
> 
> +#define port_id(priv) ((priv)->dev_data->port_id) #define eth_dev(priv)
> +(&rte_eth_devices[(priv)->dev_data->port_id])

MACRO should with capital letters and prefixed with MLX5 to align with the files consistency. 
Also suggest to use the port_id macro inside the eth_dev one. 

> +
>  /* mlx5.c */
> 
>  int mlx5_getenv_int(const char *);
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 129311d50..2e4bfac58 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -2061,7 +2061,7 @@ mlx5_flow_create_action_queue_drop(struct
> rte_eth_dev *dev,
>  		parser->queue[HASH_RXQ_ETH].ibv_attr;
>  	if (parser->count)
>  		flow->cs = parser->cs;
> -	if (!priv->dev->data->dev_started)
> +	if (!dev->data->dev_started)
>  		return 0;
>  	parser->queue[HASH_RXQ_ETH].ibv_attr = NULL;
>  	flow->frxq[HASH_RXQ_ETH].ibv_flow =
> @@ -2113,7 +2113,6 @@ mlx5_flow_create_action_queue_rss(struct
> rte_eth_dev *dev,
>  				  struct rte_flow *flow,
>  				  struct rte_flow_error *error)
>  {
> -	struct priv *priv = dev->data->dev_private;
>  	unsigned int i;
> 
>  	for (i = 0; i != hash_rxq_init_n; ++i) { @@ -2122,7 +2121,7 @@
> mlx5_flow_create_action_queue_rss(struct rte_eth_dev *dev,
>  		flow->frxq[i].ibv_attr = parser->queue[i].ibv_attr;
>  		parser->queue[i].ibv_attr = NULL;
>  		flow->frxq[i].hash_fields = parser->queue[i].hash_fields;
> -		if (!priv->dev->data->dev_started)
> +		if (!dev->data->dev_started)
>  			continue;
>  		flow->frxq[i].hrxq =
>  			mlx5_hrxq_get(dev,
> @@ -2268,7 +2267,7 @@ mlx5_flow_create_action_queue(struct
> rte_eth_dev *dev,
>  			      struct rte_flow *flow,
>  			      struct rte_flow_error *error)
>  {
> -	struct priv *priv = dev->data->dev_private;
> +	struct priv *priv __rte_unused = dev->data->dev_private;

Why not removing it? 

>  	int ret;
>  	unsigned int i;
>  	unsigned int flows_n = 0;
> @@ -2281,7 +2280,7 @@ mlx5_flow_create_action_queue(struct
> rte_eth_dev *dev,
>  		goto error;
>  	if (parser->count)
>  		flow->cs = parser->cs;
> -	if (!priv->dev->data->dev_started)
> +	if (!dev->data->dev_started)
>  		return 0;
>  	for (i = 0; i != hash_rxq_init_n; ++i) {
>  		if (!flow->frxq[i].hrxq)
> @@ -3124,9 +3123,9 @@ mlx5_flow_isolate(struct rte_eth_dev *dev,
>  	}
>  	priv->isolated = !!enable;
>  	if (enable)
> -		priv->dev->dev_ops = &mlx5_dev_ops_isolate;
> +		dev->dev_ops = &mlx5_dev_ops_isolate;
>  	else
> -		priv->dev->dev_ops = &mlx5_dev_ops;
> +		dev->dev_ops = &mlx5_dev_ops;
>  	return 0;
>  }
> 
> @@ -3514,11 +3513,10 @@ mlx5_fdir_filter_flush(struct rte_eth_dev *dev)
> static void  mlx5_fdir_info_get(struct rte_eth_dev *dev, struct
> rte_eth_fdir_info *fdir_info)  {
> -	struct priv *priv = dev->data->dev_private;
>  	struct rte_eth_fdir_masks *mask =
> -		&priv->dev->data->dev_conf.fdir_conf.mask;
> +		&dev->data->dev_conf.fdir_conf.mask;
> 
> -	fdir_info->mode = priv->dev->data->dev_conf.fdir_conf.mode;
> +	fdir_info->mode = dev->data->dev_conf.fdir_conf.mode;
>  	fdir_info->guarant_spc = 0;
>  	rte_memcpy(&fdir_info->mask, mask, sizeof(fdir_info->mask));
>  	fdir_info->max_flexpayload = 0;
> @@ -3546,9 +3544,8 @@ static int
>  mlx5_fdir_ctrl_func(struct rte_eth_dev *dev, enum rte_filter_op filter_op,
>  		    void *arg)
>  {
> -	struct priv *priv = dev->data->dev_private;
>  	enum rte_fdir_mode fdir_mode =
> -		priv->dev->data->dev_conf.fdir_conf.mode;
> +		dev->data->dev_conf.fdir_conf.mode;
> 
>  	if (filter_op == RTE_ETH_FILTER_NOP)
>  		return 0;
> diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c index
> 1b4c7ec5d..7a337d0c3 100644
> --- a/drivers/net/mlx5/mlx5_mr.c
> +++ b/drivers/net/mlx5/mlx5_mr.c
> @@ -105,8 +105,8 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq,
> struct rte_mempool *mp,
>  	rte_spinlock_lock(&txq_ctrl->priv->mr_lock);
>  	/* Add a new entry, register MR first. */
>  	DRV_LOG(DEBUG, "port %u discovered new memory pool \"%s\"
> (%p)",
> -		txq_ctrl->priv->dev->data->port_id, mp->name, (void
> *)mp);
> -	dev = txq_ctrl->priv->dev;
> +		port_id(txq_ctrl->priv), mp->name, (void *)mp);
> +	dev = eth_dev(txq_ctrl->priv);
>  	mr = mlx5_mr_get(dev, mp);
>  	if (mr == NULL) {
>  		if (rte_eal_process_type() != RTE_PROC_PRIMARY) { @@ -
> 114,8 +114,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct
> rte_mempool *mp,
>  				"port %u using unregistered mempool
> 0x%p(%s)"
>  				" in secondary process, please create
> mempool"
>  				" before rte_eth_dev_start()",
> -				txq_ctrl->priv->dev->data->port_id,
> -				(void *)mp, mp->name);
> +				port_id(txq_ctrl->priv), (void *)mp, mp-
> >name);
>  			rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
>  			rte_errno = ENOTSUP;
>  			return NULL;
> @@ -126,7 +125,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq,
> struct rte_mempool *mp,
>  		DRV_LOG(DEBUG,
>  			"port %u unable to configure memory region,"
>  			" ibv_reg_mr() failed.",
> -			txq_ctrl->priv->dev->data->port_id);
> +			port_id(txq_ctrl->priv));
>  		rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
>  		return NULL;
>  	}
> @@ -135,7 +134,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq,
> struct rte_mempool *mp,
>  		DRV_LOG(DEBUG,
>  			"port %u memory region <-> memory pool table full,
> "
>  			" dropping oldest entry",
> -			txq_ctrl->priv->dev->data->port_id);
> +			port_id(txq_ctrl->priv));
>  		--idx;
>  		mlx5_mr_release(txq->mp2mr[0]);
>  		memmove(&txq->mp2mr[0], &txq->mp2mr[1], @@ -146,7
> +145,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct
> rte_mempool *mp,
>  	DRV_LOG(DEBUG,
>  		"port %u new memory region lkey for MP \"%s\" (%p):
> 0x%08"
>  		PRIu32,
> -		txq_ctrl->priv->dev->data->port_id, mp->name, (void *)mp,
> +		port_id(txq_ctrl->priv), mp->name, (void *)mp,
>  		txq_ctrl->txq.mp2mr[idx]->lkey);
>  	rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
>  	return mr;
> @@ -207,15 +206,15 @@ mlx5_mp2mr_iter(struct rte_mempool *mp, void
> *arg)
>  	if (rte_mempool_obj_iter(mp, txq_mp2mr_mbuf_check, &data) ==
> 0 ||
>  			data.ret == -1)
>  		return;
> -	mr = mlx5_mr_get(priv->dev, mp);
> +	mr = mlx5_mr_get(eth_dev(priv), mp);
>  	if (mr) {
>  		mlx5_mr_release(mr);
>  		return;
>  	}
> -	mr = mlx5_mr_new(priv->dev, mp);
> +	mr = mlx5_mr_new(eth_dev(priv), mp);
>  	if (!mr)
>  		DRV_LOG(ERR, "port %u cannot create memory region: %s",
> -			priv->dev->data->port_id, strerror(rte_errno));
> +			port_id(priv), strerror(rte_errno));
>  }
> 
>  /**
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index 126412ddd..a85b628fe 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -78,7 +78,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
>  		buf = rte_pktmbuf_alloc(rxq_ctrl->rxq.mp);
>  		if (buf == NULL) {
>  			DRV_LOG(ERR, "port %u empty mbuf pool",
> -				rxq_ctrl->priv->dev->data->port_id);
> +				port_id(rxq_ctrl->priv));
>  			rte_errno = ENOMEM;
>  			goto error;
>  		}
> @@ -122,7 +122,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
>  	DRV_LOG(DEBUG,
>  		"port %u Rx queue %u allocated and configured %u
> segments"
>  		" (max %u packets)",
> -		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx, elts_n,
> +		port_id(rxq_ctrl->priv), rxq_ctrl->idx, elts_n,
>  		elts_n / (1 << rxq_ctrl->rxq.sges_n));
>  	return 0;
>  error:
> @@ -134,7 +134,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
>  		(*rxq_ctrl->rxq.elts)[i] = NULL;
>  	}
>  	DRV_LOG(DEBUG, "port %u Rx queue %u failed, freed everything",
> -		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
> +		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
>  	rte_errno = err; /* Restore rte_errno. */
>  	return -rte_errno;
>  }
> @@ -155,7 +155,7 @@ rxq_free_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
>  	uint16_t i;
> 
>  	DRV_LOG(DEBUG, "port %u Rx queue %u freeing WRs",
> -		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
> +		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
>  	if (rxq->elts == NULL)
>  		return;
>  	/**
> @@ -186,7 +186,7 @@ void
>  mlx5_rxq_cleanup(struct mlx5_rxq_ctrl *rxq_ctrl)  {
>  	DRV_LOG(DEBUG, "port %u cleaning up Rx queue %u",
> -		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
> +		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
>  	if (rxq_ctrl->ibv)
>  		mlx5_rxq_ibv_release(rxq_ctrl->ibv);
>  	memset(rxq_ctrl, 0, sizeof(*rxq_ctrl)); @@ -354,11 +354,11 @@
> mlx5_rx_queue_release(void *dpdk_rxq)
>  		return;
>  	rxq_ctrl = container_of(rxq, struct mlx5_rxq_ctrl, rxq);
>  	priv = rxq_ctrl->priv;
> -	if (!mlx5_rxq_releasable(priv->dev, rxq_ctrl->rxq.stats.idx))
> +	if (!mlx5_rxq_releasable(eth_dev(priv), rxq_ctrl->rxq.stats.idx))
>  		rte_panic("port %u Rx queue %u is still used by a flow and"
> -			  " cannot be removed\n", priv->dev->data->port_id,
> -			  rxq_ctrl->idx);
> -	mlx5_rxq_release(priv->dev, rxq_ctrl->rxq.stats.idx);
> +			  " cannot be removed\n",
> +			  port_id(priv), rxq_ctrl->idx);
> +	mlx5_rxq_release(eth_dev(priv), rxq_ctrl->rxq.stats.idx);
>  }
> 
>  /**
> @@ -378,9 +378,9 @@ mlx5_rx_intr_vec_enable(struct rte_eth_dev *dev)
>  	unsigned int rxqs_n = priv->rxqs_n;
>  	unsigned int n = RTE_MIN(rxqs_n,
> (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
>  	unsigned int count = 0;
> -	struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
> +	struct rte_intr_handle *intr_handle = dev->intr_handle;
> 
> -	if (!priv->dev->data->dev_conf.intr_conf.rxq)
> +	if (!dev->data->dev_conf.intr_conf.rxq)
>  		return 0;
>  	mlx5_rx_intr_vec_disable(dev);
>  	intr_handle->intr_vec = malloc(n * sizeof(intr_handle->intr_vec[0]));
> @@ -452,12 +452,12 @@ void
>  mlx5_rx_intr_vec_disable(struct rte_eth_dev *dev)  {
>  	struct priv *priv = dev->data->dev_private;
> -	struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
> +	struct rte_intr_handle *intr_handle = dev->intr_handle;
>  	unsigned int i;
>  	unsigned int rxqs_n = priv->rxqs_n;
>  	unsigned int n = RTE_MIN(rxqs_n,
> (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
> 
> -	if (!priv->dev->data->dev_conf.intr_conf.rxq)
> +	if (!dev->data->dev_conf.intr_conf.rxq)
>  		return;
>  	if (!intr_handle->intr_vec)
>  		goto free;
> @@ -897,7 +897,7 @@ mlx5_rxq_ibv_release(struct mlx5_rxq_ibv *rxq_ibv)
>  	if (!ret)
>  		rxq_ibv->mr = NULL;
>  	DRV_LOG(DEBUG, "port %u Verbs Rx queue %u: refcnt %d",
> -		rxq_ibv->rxq_ctrl->priv->dev->data->port_id,
> +		port_id(rxq_ibv->rxq_ctrl->priv),
>  		rxq_ibv->rxq_ctrl->idx, rte_atomic32_read(&rxq_ibv-
> >refcnt));
>  	if (rte_atomic32_dec_and_test(&rxq_ibv->refcnt)) {
>  		rxq_free_elts(rxq_ibv->rxq_ctrl);
> @@ -990,7 +990,7 @@ mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t
> idx, uint16_t desc,
>  		return NULL;
>  	}
>  	tmpl->socket = socket;
> -	if (priv->dev->data->dev_conf.intr_conf.rxq)
> +	if (dev->data->dev_conf.intr_conf.rxq)
>  		tmpl->irq = 1;
>  	/* Enable scattered packets support for this queue if necessary. */
>  	assert(mb_len >= RTE_PKTMBUF_HEADROOM); diff --git
> a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c index
> 44358744d..29959b4c7 100644
> --- a/drivers/net/mlx5/mlx5_txq.c
> +++ b/drivers/net/mlx5/mlx5_txq.c
> @@ -48,7 +48,7 @@ txq_alloc_elts(struct mlx5_txq_ctrl *txq_ctrl)
>  	for (i = 0; (i != elts_n); ++i)
>  		(*txq_ctrl->txq.elts)[i] = NULL;
>  	DRV_LOG(DEBUG, "port %u Tx queue %u allocated and configured
> %u WRs",
> -		txq_ctrl->priv->dev->data->port_id, txq_ctrl->idx, elts_n);
> +		port_id(txq_ctrl->priv), txq_ctrl->idx, elts_n);
>  	txq_ctrl->txq.elts_head = 0;
>  	txq_ctrl->txq.elts_tail = 0;
>  	txq_ctrl->txq.elts_comp = 0;
> @@ -70,7 +70,7 @@ txq_free_elts(struct mlx5_txq_ctrl *txq_ctrl)
>  	struct rte_mbuf *(*elts)[elts_n] = txq_ctrl->txq.elts;
> 
>  	DRV_LOG(DEBUG, "port %u Tx queue %u freeing WRs",
> -		txq_ctrl->priv->dev->data->port_id, txq_ctrl->idx);
> +		port_id(txq_ctrl->priv), txq_ctrl->idx);
>  	txq_ctrl->txq.elts_head = 0;
>  	txq_ctrl->txq.elts_tail = 0;
>  	txq_ctrl->txq.elts_comp = 0;
> @@ -255,9 +255,9 @@ mlx5_tx_queue_release(void *dpdk_txq)
>  	priv = txq_ctrl->priv;
>  	for (i = 0; (i != priv->txqs_n); ++i)
>  		if ((*priv->txqs)[i] == txq) {
> -			mlx5_txq_release(priv->dev, i);
> +			mlx5_txq_release(eth_dev(priv), i);
>  			DRV_LOG(DEBUG, "port %u removing Tx queue %u
> from list",
> -				priv->dev->data->port_id, txq_ctrl->idx);
> +				port_id(priv), txq_ctrl->idx);
>  			break;
>  		}
>  }
> @@ -618,7 +618,7 @@ mlx5_txq_ibv_release(struct mlx5_txq_ibv *txq_ibv)
> {
>  	assert(txq_ibv);
>  	DRV_LOG(DEBUG, "port %u Verbs Tx queue %u: refcnt %d",
> -		txq_ibv->txq_ctrl->priv->dev->data->port_id,
> +		port_id(txq_ibv->txq_ctrl->priv),
>  		txq_ibv->txq_ctrl->idx, rte_atomic32_read(&txq_ibv-
> >refcnt));
>  	if (rte_atomic32_dec_and_test(&txq_ibv->refcnt)) {
>  		claim_zero(mlx5_glue->destroy_qp(txq_ibv->qp));
> @@ -685,7 +685,7 @@ txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)
>  	unsigned int txqs_inline;
>  	unsigned int inline_max_packet_sz;
>  	eth_tx_burst_t tx_pkt_burst =
> -		mlx5_select_tx_function(txq_ctrl->priv->dev);
> +		mlx5_select_tx_function(eth_dev(priv));
>  	int is_empw_func = is_empw_burst_func(tx_pkt_burst);
>  	int tso = !!(txq_ctrl->txq.offloads & (DEV_TX_OFFLOAD_TCP_TSO |
> 
> DEV_TX_OFFLOAD_VXLAN_TNL_TSO | @@ -759,8 +759,7 @@
> txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)
>  			DRV_LOG(WARNING,
>  				"port %u txq inline is too large (%d) setting"
>  				" it to the maximum possible: %d\n",
> -				priv->dev->data->port_id, txq_inline,
> -				max_inline);
> +				port_id(priv), txq_inline, max_inline);
>  			txq_ctrl->txq.max_inline = max_inline /
>  						   RTE_CACHE_LINE_SIZE;
>  		}
> --
> 2.11.0

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

* Re: [dpdk-dev] [PATCH v2] net/mlx5: change device reference for secondary process
  2018-05-06  6:25   ` Shahaf Shuler
@ 2018-05-07  6:46     ` Yongseok Koh
  0 siblings, 0 replies; 8+ messages in thread
From: Yongseok Koh @ 2018-05-07  6:46 UTC (permalink / raw)
  To: Shahaf Shuler, Adrien Mazarguil; +Cc: Nélio Laranjeiro, dev

> On May 5, 2018, at 11:25 PM, Shahaf Shuler <shahafs@mellanox.com> wrote:
> 
> Hi Koh,
> 
> See some small comments. 
> 
> Thursday, May 3, 2018 2:11 AM, Yongseok Koh:
>> Subject: [dpdk-dev] [PATCH v2] net/mlx5: change device reference for
>> secondary process
>> 
>> rte_eth_devices[] is not shared between primary and secondary process,
>> but a static array to each process. The reverse pointer of device (priv->dev) is
>> invalid. Instead, priv has the pointer to shared data of the device,
>>  struct rte_eth_dev_data *dev_data;
>> 
>> Two macros are added,
>>  #define port_id(priv) ((priv)->dev_data->port_id)
>>  #define eth_dev(priv) (&rte_eth_devices[(priv)->dev_data->port_id])
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> drivers/net/mlx5/mlx5.c      |  2 +-
>> drivers/net/mlx5/mlx5.h      |  5 ++++-
>> drivers/net/mlx5/mlx5_flow.c | 21 +++++++++------------
>> drivers/net/mlx5/mlx5_mr.c   | 19 +++++++++----------
>> drivers/net/mlx5/mlx5_rxq.c  | 30 +++++++++++++++---------------
>> drivers/net/mlx5/mlx5_txq.c  | 15 +++++++--------
>> 6 files changed, 45 insertions(+), 47 deletions(-)
>> 
>> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
>> 8f983061a..6c4a571ab 100644
>> --- a/drivers/net/mlx5/mlx5.c
>> +++ b/drivers/net/mlx5/mlx5.c
>> @@ -946,7 +946,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv
>> __rte_unused,
>> 			goto port_error;
>> 		}
>> 		eth_dev->data->dev_private = priv;
>> -		priv->dev = eth_dev;
>> +		priv->dev_data = eth_dev->data;
>> 		eth_dev->data->mac_addrs = priv->mac;
>> 		eth_dev->device = &pci_dev->device;
>> 		rte_eth_copy_pci_info(eth_dev, pci_dev); diff --git
>> a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
>> c4d1d456b..3ab16bfa2 100644
>> --- a/drivers/net/mlx5/mlx5.h
>> +++ b/drivers/net/mlx5/mlx5.h
>> @@ -121,7 +121,7 @@ struct mlx5_verbs_alloc_ctx {  };
>> 
>> struct priv {
>> -	struct rte_eth_dev *dev; /* Ethernet device of master process. */
>> +	struct rte_eth_dev_data *dev_data;  /* Pointer to device data. */
>> 	struct ibv_context *ctx; /* Verbs context. */
>> 	struct ibv_device_attr_ex device_attr; /* Device properties. */
>> 	struct ibv_pd *pd; /* Protection Domain. */ @@ -168,6 +168,9 @@
>> struct priv {
>> 	uint32_t nl_sn; /* Netlink message sequence number. */  };
>> 
>> +#define port_id(priv) ((priv)->dev_data->port_id)
>> +#define eth_dev(priv) (&rte_eth_devices[(priv)->dev_data->port_id])
> 
> MACRO should with capital letters and prefixed with MLX5 to align with the files consistency. 
> Also suggest to use the port_id macro inside the eth_dev one. 

The reason was, I wanted to make it naturally mingle with other code because
it is neither a functional macro nor a representation of a value. Either way
is okay to me anyway. As Nelio is OOO, let's hear from Adrien.

Majority vote :-) Adrien??

>> +
>> /* mlx5.c */
>> 
>> int mlx5_getenv_int(const char *);
>> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
>> index 129311d50..2e4bfac58 100644
>> --- a/drivers/net/mlx5/mlx5_flow.c
>> +++ b/drivers/net/mlx5/mlx5_flow.c
>> @@ -2061,7 +2061,7 @@ mlx5_flow_create_action_queue_drop(struct
>> rte_eth_dev *dev,
>> 		parser->queue[HASH_RXQ_ETH].ibv_attr;
>> 	if (parser->count)
>> 		flow->cs = parser->cs;
>> -	if (!priv->dev->data->dev_started)
>> +	if (!dev->data->dev_started)
>> 		return 0;
>> 	parser->queue[HASH_RXQ_ETH].ibv_attr = NULL;
>> 	flow->frxq[HASH_RXQ_ETH].ibv_flow =
>> @@ -2113,7 +2113,6 @@ mlx5_flow_create_action_queue_rss(struct
>> rte_eth_dev *dev,
>> 				  struct rte_flow *flow,
>> 				  struct rte_flow_error *error)
>> {
>> -	struct priv *priv = dev->data->dev_private;
>> 	unsigned int i;
>> 
>> 	for (i = 0; i != hash_rxq_init_n; ++i) { @@ -2122,7 +2121,7 @@
>> mlx5_flow_create_action_queue_rss(struct rte_eth_dev *dev,
>> 		flow->frxq[i].ibv_attr = parser->queue[i].ibv_attr;
>> 		parser->queue[i].ibv_attr = NULL;
>> 		flow->frxq[i].hash_fields = parser->queue[i].hash_fields;
>> -		if (!priv->dev->data->dev_started)
>> +		if (!dev->data->dev_started)
>> 			continue;
>> 		flow->frxq[i].hrxq =
>> 			mlx5_hrxq_get(dev,
>> @@ -2268,7 +2267,7 @@ mlx5_flow_create_action_queue(struct
>> rte_eth_dev *dev,
>> 			      struct rte_flow *flow,
>> 			      struct rte_flow_error *error)
>> {
>> -	struct priv *priv = dev->data->dev_private;
>> +	struct priv *priv __rte_unused = dev->data->dev_private;
> 
> Why not removing it? 

It is used in assert() below when NDEBUG is off. I could have made the assert()
longer by deleting it but I preferred this way.

>> 	int ret;
>> 	unsigned int i;
>> 	unsigned int flows_n = 0;
>> @@ -2281,7 +2280,7 @@ mlx5_flow_create_action_queue(struct
>> rte_eth_dev *dev,
>> 		goto error;
>> 	if (parser->count)
>> 		flow->cs = parser->cs;
>> -	if (!priv->dev->data->dev_started)
>> +	if (!dev->data->dev_started)
>> 		return 0;
>> 	for (i = 0; i != hash_rxq_init_n; ++i) {
>> 		if (!flow->frxq[i].hrxq)
>> @@ -3124,9 +3123,9 @@ mlx5_flow_isolate(struct rte_eth_dev *dev,
>> 	}
>> 	priv->isolated = !!enable;
>> 	if (enable)
>> -		priv->dev->dev_ops = &mlx5_dev_ops_isolate;
>> +		dev->dev_ops = &mlx5_dev_ops_isolate;
>> 	else
>> -		priv->dev->dev_ops = &mlx5_dev_ops;
>> +		dev->dev_ops = &mlx5_dev_ops;
>> 	return 0;
>> }
>> 
>> @@ -3514,11 +3513,10 @@ mlx5_fdir_filter_flush(struct rte_eth_dev *dev)
>> static void  mlx5_fdir_info_get(struct rte_eth_dev *dev, struct
>> rte_eth_fdir_info *fdir_info)  {
>> -	struct priv *priv = dev->data->dev_private;
>> 	struct rte_eth_fdir_masks *mask =
>> -		&priv->dev->data->dev_conf.fdir_conf.mask;
>> +		&dev->data->dev_conf.fdir_conf.mask;
>> 
>> -	fdir_info->mode = priv->dev->data->dev_conf.fdir_conf.mode;
>> +	fdir_info->mode = dev->data->dev_conf.fdir_conf.mode;
>> 	fdir_info->guarant_spc = 0;
>> 	rte_memcpy(&fdir_info->mask, mask, sizeof(fdir_info->mask));
>> 	fdir_info->max_flexpayload = 0;
>> @@ -3546,9 +3544,8 @@ static int
>> mlx5_fdir_ctrl_func(struct rte_eth_dev *dev, enum rte_filter_op filter_op,
>> 		    void *arg)
>> {
>> -	struct priv *priv = dev->data->dev_private;
>> 	enum rte_fdir_mode fdir_mode =
>> -		priv->dev->data->dev_conf.fdir_conf.mode;
>> +		dev->data->dev_conf.fdir_conf.mode;
>> 
>> 	if (filter_op == RTE_ETH_FILTER_NOP)
>> 		return 0;
>> diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c index
>> 1b4c7ec5d..7a337d0c3 100644
>> --- a/drivers/net/mlx5/mlx5_mr.c
>> +++ b/drivers/net/mlx5/mlx5_mr.c
>> @@ -105,8 +105,8 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq,
>> struct rte_mempool *mp,
>> 	rte_spinlock_lock(&txq_ctrl->priv->mr_lock);
>> 	/* Add a new entry, register MR first. */
>> 	DRV_LOG(DEBUG, "port %u discovered new memory pool \"%s\"
>> (%p)",
>> -		txq_ctrl->priv->dev->data->port_id, mp->name, (void
>> *)mp);
>> -	dev = txq_ctrl->priv->dev;
>> +		port_id(txq_ctrl->priv), mp->name, (void *)mp);
>> +	dev = eth_dev(txq_ctrl->priv);
>> 	mr = mlx5_mr_get(dev, mp);
>> 	if (mr == NULL) {
>> 		if (rte_eal_process_type() != RTE_PROC_PRIMARY) { @@ -
>> 114,8 +114,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct
>> rte_mempool *mp,
>> 				"port %u using unregistered mempool
>> 0x%p(%s)"
>> 				" in secondary process, please create
>> mempool"
>> 				" before rte_eth_dev_start()",
>> -				txq_ctrl->priv->dev->data->port_id,
>> -				(void *)mp, mp->name);
>> +				port_id(txq_ctrl->priv), (void *)mp, mp-
>>> name);
>> 			rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
>> 			rte_errno = ENOTSUP;
>> 			return NULL;
>> @@ -126,7 +125,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq,
>> struct rte_mempool *mp,
>> 		DRV_LOG(DEBUG,
>> 			"port %u unable to configure memory region,"
>> 			" ibv_reg_mr() failed.",
>> -			txq_ctrl->priv->dev->data->port_id);
>> +			port_id(txq_ctrl->priv));
>> 		rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
>> 		return NULL;
>> 	}
>> @@ -135,7 +134,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq,
>> struct rte_mempool *mp,
>> 		DRV_LOG(DEBUG,
>> 			"port %u memory region <-> memory pool table full,
>> "
>> 			" dropping oldest entry",
>> -			txq_ctrl->priv->dev->data->port_id);
>> +			port_id(txq_ctrl->priv));
>> 		--idx;
>> 		mlx5_mr_release(txq->mp2mr[0]);
>> 		memmove(&txq->mp2mr[0], &txq->mp2mr[1], @@ -146,7
>> +145,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct
>> rte_mempool *mp,
>> 	DRV_LOG(DEBUG,
>> 		"port %u new memory region lkey for MP \"%s\" (%p):
>> 0x%08"
>> 		PRIu32,
>> -		txq_ctrl->priv->dev->data->port_id, mp->name, (void *)mp,
>> +		port_id(txq_ctrl->priv), mp->name, (void *)mp,
>> 		txq_ctrl->txq.mp2mr[idx]->lkey);
>> 	rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
>> 	return mr;
>> @@ -207,15 +206,15 @@ mlx5_mp2mr_iter(struct rte_mempool *mp, void
>> *arg)
>> 	if (rte_mempool_obj_iter(mp, txq_mp2mr_mbuf_check, &data) ==
>> 0 ||
>> 			data.ret == -1)
>> 		return;
>> -	mr = mlx5_mr_get(priv->dev, mp);
>> +	mr = mlx5_mr_get(eth_dev(priv), mp);
>> 	if (mr) {
>> 		mlx5_mr_release(mr);
>> 		return;
>> 	}
>> -	mr = mlx5_mr_new(priv->dev, mp);
>> +	mr = mlx5_mr_new(eth_dev(priv), mp);
>> 	if (!mr)
>> 		DRV_LOG(ERR, "port %u cannot create memory region: %s",
>> -			priv->dev->data->port_id, strerror(rte_errno));
>> +			port_id(priv), strerror(rte_errno));
>> }
>> 
>> /**
>> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
>> index 126412ddd..a85b628fe 100644
>> --- a/drivers/net/mlx5/mlx5_rxq.c
>> +++ b/drivers/net/mlx5/mlx5_rxq.c
>> @@ -78,7 +78,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
>> 		buf = rte_pktmbuf_alloc(rxq_ctrl->rxq.mp);
>> 		if (buf == NULL) {
>> 			DRV_LOG(ERR, "port %u empty mbuf pool",
>> -				rxq_ctrl->priv->dev->data->port_id);
>> +				port_id(rxq_ctrl->priv));
>> 			rte_errno = ENOMEM;
>> 			goto error;
>> 		}
>> @@ -122,7 +122,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
>> 	DRV_LOG(DEBUG,
>> 		"port %u Rx queue %u allocated and configured %u
>> segments"
>> 		" (max %u packets)",
>> -		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx, elts_n,
>> +		port_id(rxq_ctrl->priv), rxq_ctrl->idx, elts_n,
>> 		elts_n / (1 << rxq_ctrl->rxq.sges_n));
>> 	return 0;
>> error:
>> @@ -134,7 +134,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
>> 		(*rxq_ctrl->rxq.elts)[i] = NULL;
>> 	}
>> 	DRV_LOG(DEBUG, "port %u Rx queue %u failed, freed everything",
>> -		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
>> +		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
>> 	rte_errno = err; /* Restore rte_errno. */
>> 	return -rte_errno;
>> }
>> @@ -155,7 +155,7 @@ rxq_free_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
>> 	uint16_t i;
>> 
>> 	DRV_LOG(DEBUG, "port %u Rx queue %u freeing WRs",
>> -		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
>> +		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
>> 	if (rxq->elts == NULL)
>> 		return;
>> 	/**
>> @@ -186,7 +186,7 @@ void
>> mlx5_rxq_cleanup(struct mlx5_rxq_ctrl *rxq_ctrl)  {
>> 	DRV_LOG(DEBUG, "port %u cleaning up Rx queue %u",
>> -		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
>> +		port_id(rxq_ctrl->priv), rxq_ctrl->idx);
>> 	if (rxq_ctrl->ibv)
>> 		mlx5_rxq_ibv_release(rxq_ctrl->ibv);
>> 	memset(rxq_ctrl, 0, sizeof(*rxq_ctrl)); @@ -354,11 +354,11 @@
>> mlx5_rx_queue_release(void *dpdk_rxq)
>> 		return;
>> 	rxq_ctrl = container_of(rxq, struct mlx5_rxq_ctrl, rxq);
>> 	priv = rxq_ctrl->priv;
>> -	if (!mlx5_rxq_releasable(priv->dev, rxq_ctrl->rxq.stats.idx))
>> +	if (!mlx5_rxq_releasable(eth_dev(priv), rxq_ctrl->rxq.stats.idx))
>> 		rte_panic("port %u Rx queue %u is still used by a flow and"
>> -			  " cannot be removed\n", priv->dev->data->port_id,
>> -			  rxq_ctrl->idx);
>> -	mlx5_rxq_release(priv->dev, rxq_ctrl->rxq.stats.idx);
>> +			  " cannot be removed\n",
>> +			  port_id(priv), rxq_ctrl->idx);
>> +	mlx5_rxq_release(eth_dev(priv), rxq_ctrl->rxq.stats.idx);
>> }
>> 
>> /**
>> @@ -378,9 +378,9 @@ mlx5_rx_intr_vec_enable(struct rte_eth_dev *dev)
>> 	unsigned int rxqs_n = priv->rxqs_n;
>> 	unsigned int n = RTE_MIN(rxqs_n,
>> (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
>> 	unsigned int count = 0;
>> -	struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
>> +	struct rte_intr_handle *intr_handle = dev->intr_handle;
>> 
>> -	if (!priv->dev->data->dev_conf.intr_conf.rxq)
>> +	if (!dev->data->dev_conf.intr_conf.rxq)
>> 		return 0;
>> 	mlx5_rx_intr_vec_disable(dev);
>> 	intr_handle->intr_vec = malloc(n * sizeof(intr_handle->intr_vec[0]));
>> @@ -452,12 +452,12 @@ void
>> mlx5_rx_intr_vec_disable(struct rte_eth_dev *dev)  {
>> 	struct priv *priv = dev->data->dev_private;
>> -	struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
>> +	struct rte_intr_handle *intr_handle = dev->intr_handle;
>> 	unsigned int i;
>> 	unsigned int rxqs_n = priv->rxqs_n;
>> 	unsigned int n = RTE_MIN(rxqs_n,
>> (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
>> 
>> -	if (!priv->dev->data->dev_conf.intr_conf.rxq)
>> +	if (!dev->data->dev_conf.intr_conf.rxq)
>> 		return;
>> 	if (!intr_handle->intr_vec)
>> 		goto free;
>> @@ -897,7 +897,7 @@ mlx5_rxq_ibv_release(struct mlx5_rxq_ibv *rxq_ibv)
>> 	if (!ret)
>> 		rxq_ibv->mr = NULL;
>> 	DRV_LOG(DEBUG, "port %u Verbs Rx queue %u: refcnt %d",
>> -		rxq_ibv->rxq_ctrl->priv->dev->data->port_id,
>> +		port_id(rxq_ibv->rxq_ctrl->priv),
>> 		rxq_ibv->rxq_ctrl->idx, rte_atomic32_read(&rxq_ibv-
>>> refcnt));
>> 	if (rte_atomic32_dec_and_test(&rxq_ibv->refcnt)) {
>> 		rxq_free_elts(rxq_ibv->rxq_ctrl);
>> @@ -990,7 +990,7 @@ mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t
>> idx, uint16_t desc,
>> 		return NULL;
>> 	}
>> 	tmpl->socket = socket;
>> -	if (priv->dev->data->dev_conf.intr_conf.rxq)
>> +	if (dev->data->dev_conf.intr_conf.rxq)
>> 		tmpl->irq = 1;
>> 	/* Enable scattered packets support for this queue if necessary. */
>> 	assert(mb_len >= RTE_PKTMBUF_HEADROOM); diff --git
>> a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c index
>> 44358744d..29959b4c7 100644
>> --- a/drivers/net/mlx5/mlx5_txq.c
>> +++ b/drivers/net/mlx5/mlx5_txq.c
>> @@ -48,7 +48,7 @@ txq_alloc_elts(struct mlx5_txq_ctrl *txq_ctrl)
>> 	for (i = 0; (i != elts_n); ++i)
>> 		(*txq_ctrl->txq.elts)[i] = NULL;
>> 	DRV_LOG(DEBUG, "port %u Tx queue %u allocated and configured
>> %u WRs",
>> -		txq_ctrl->priv->dev->data->port_id, txq_ctrl->idx, elts_n);
>> +		port_id(txq_ctrl->priv), txq_ctrl->idx, elts_n);
>> 	txq_ctrl->txq.elts_head = 0;
>> 	txq_ctrl->txq.elts_tail = 0;
>> 	txq_ctrl->txq.elts_comp = 0;
>> @@ -70,7 +70,7 @@ txq_free_elts(struct mlx5_txq_ctrl *txq_ctrl)
>> 	struct rte_mbuf *(*elts)[elts_n] = txq_ctrl->txq.elts;
>> 
>> 	DRV_LOG(DEBUG, "port %u Tx queue %u freeing WRs",
>> -		txq_ctrl->priv->dev->data->port_id, txq_ctrl->idx);
>> +		port_id(txq_ctrl->priv), txq_ctrl->idx);
>> 	txq_ctrl->txq.elts_head = 0;
>> 	txq_ctrl->txq.elts_tail = 0;
>> 	txq_ctrl->txq.elts_comp = 0;
>> @@ -255,9 +255,9 @@ mlx5_tx_queue_release(void *dpdk_txq)
>> 	priv = txq_ctrl->priv;
>> 	for (i = 0; (i != priv->txqs_n); ++i)
>> 		if ((*priv->txqs)[i] == txq) {
>> -			mlx5_txq_release(priv->dev, i);
>> +			mlx5_txq_release(eth_dev(priv), i);
>> 			DRV_LOG(DEBUG, "port %u removing Tx queue %u
>> from list",
>> -				priv->dev->data->port_id, txq_ctrl->idx);
>> +				port_id(priv), txq_ctrl->idx);
>> 			break;
>> 		}
>> }
>> @@ -618,7 +618,7 @@ mlx5_txq_ibv_release(struct mlx5_txq_ibv *txq_ibv)
>> {
>> 	assert(txq_ibv);
>> 	DRV_LOG(DEBUG, "port %u Verbs Tx queue %u: refcnt %d",
>> -		txq_ibv->txq_ctrl->priv->dev->data->port_id,
>> +		port_id(txq_ibv->txq_ctrl->priv),
>> 		txq_ibv->txq_ctrl->idx, rte_atomic32_read(&txq_ibv-
>>> refcnt));
>> 	if (rte_atomic32_dec_and_test(&txq_ibv->refcnt)) {
>> 		claim_zero(mlx5_glue->destroy_qp(txq_ibv->qp));
>> @@ -685,7 +685,7 @@ txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)
>> 	unsigned int txqs_inline;
>> 	unsigned int inline_max_packet_sz;
>> 	eth_tx_burst_t tx_pkt_burst =
>> -		mlx5_select_tx_function(txq_ctrl->priv->dev);
>> +		mlx5_select_tx_function(eth_dev(priv));
>> 	int is_empw_func = is_empw_burst_func(tx_pkt_burst);
>> 	int tso = !!(txq_ctrl->txq.offloads & (DEV_TX_OFFLOAD_TCP_TSO |
>> 
>> DEV_TX_OFFLOAD_VXLAN_TNL_TSO | @@ -759,8 +759,7 @@
>> txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)
>> 			DRV_LOG(WARNING,
>> 				"port %u txq inline is too large (%d) setting"
>> 				" it to the maximum possible: %d\n",
>> -				priv->dev->data->port_id, txq_inline,
>> -				max_inline);
>> +				port_id(priv), txq_inline, max_inline);
>> 			txq_ctrl->txq.max_inline = max_inline /
>> 						   RTE_CACHE_LINE_SIZE;
>> 		}
>> --
>> 2.11.0

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

* [dpdk-dev] [PATCH v3] net/mlx5: change device reference for secondary process
  2018-05-02  6:13 [dpdk-dev] [PATCH] net/mlx5: fix device reference in secondary process Yongseok Koh
  2018-05-02  6:53 ` Nélio Laranjeiro
  2018-05-02 23:10 ` [dpdk-dev] [PATCH v2] net/mlx5: change device reference for " Yongseok Koh
@ 2018-05-09 11:04 ` Yongseok Koh
  2018-05-09 11:30   ` Shahaf Shuler
  2 siblings, 1 reply; 8+ messages in thread
From: Yongseok Koh @ 2018-05-09 11:04 UTC (permalink / raw)
  To: adrien.mazarguil, nelio.laranjeiro; +Cc: dev, Yongseok Koh

rte_eth_devices[] is not shared between primary and secondary process, but
a static array to each process. The reverse pointer of device (priv->dev)
is invalid. Instead, priv has the pointer to shared data of the device,
  struct rte_eth_dev_data *dev_data;

Two macros are added,
  #define PORT_ID(priv) ((priv)->dev_data->port_id)
  #define ETH_DEV(priv) (&rte_eth_devices[PORT_ID(priv)])

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

v3:
* eth_dev() -> ETH_DEV()
* port_id() -> PORT_ID()

 drivers/net/mlx5/mlx5.c      |  2 +-
 drivers/net/mlx5/mlx5.h      |  5 ++++-
 drivers/net/mlx5/mlx5_flow.c | 21 +++++++++------------
 drivers/net/mlx5/mlx5_mr.c   | 19 +++++++++----------
 drivers/net/mlx5/mlx5_rxq.c  | 38 +++++++++++++++++++-------------------
 drivers/net/mlx5/mlx5_txq.c  | 15 +++++++--------
 6 files changed, 49 insertions(+), 51 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index fcb14fe1a..efc6313fe 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -954,7 +954,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			goto port_error;
 		}
 		eth_dev->data->dev_private = priv;
-		priv->dev = eth_dev;
+		priv->dev_data = eth_dev->data;
 		eth_dev->data->mac_addrs = priv->mac;
 		eth_dev->device = &pci_dev->device;
 		rte_eth_copy_pci_info(eth_dev, pci_dev);
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index c4d1d456b..f1294c54b 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -121,7 +121,7 @@ struct mlx5_verbs_alloc_ctx {
 };
 
 struct priv {
-	struct rte_eth_dev *dev; /* Ethernet device of master process. */
+	struct rte_eth_dev_data *dev_data;  /* Pointer to device data. */
 	struct ibv_context *ctx; /* Verbs context. */
 	struct ibv_device_attr_ex device_attr; /* Device properties. */
 	struct ibv_pd *pd; /* Protection Domain. */
@@ -168,6 +168,9 @@ struct priv {
 	uint32_t nl_sn; /* Netlink message sequence number. */
 };
 
+#define PORT_ID(priv) ((priv)->dev_data->port_id)
+#define ETH_DEV(priv) (&rte_eth_devices[PORT_ID(priv)])
+
 /* mlx5.c */
 
 int mlx5_getenv_int(const char *);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 38811bbce..ec6d00f21 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2034,7 +2034,7 @@ mlx5_flow_create_action_queue_drop(struct rte_eth_dev *dev,
 		parser->queue[HASH_RXQ_ETH].ibv_attr;
 	if (parser->count)
 		flow->cs = parser->cs;
-	if (!priv->dev->data->dev_started)
+	if (!dev->data->dev_started)
 		return 0;
 	parser->queue[HASH_RXQ_ETH].ibv_attr = NULL;
 	flow->frxq[HASH_RXQ_ETH].ibv_flow =
@@ -2086,7 +2086,6 @@ mlx5_flow_create_action_queue_rss(struct rte_eth_dev *dev,
 				  struct rte_flow *flow,
 				  struct rte_flow_error *error)
 {
-	struct priv *priv = dev->data->dev_private;
 	unsigned int i;
 
 	for (i = 0; i != hash_rxq_init_n; ++i) {
@@ -2095,7 +2094,7 @@ mlx5_flow_create_action_queue_rss(struct rte_eth_dev *dev,
 		flow->frxq[i].ibv_attr = parser->queue[i].ibv_attr;
 		parser->queue[i].ibv_attr = NULL;
 		flow->frxq[i].hash_fields = parser->queue[i].hash_fields;
-		if (!priv->dev->data->dev_started)
+		if (!dev->data->dev_started)
 			continue;
 		flow->frxq[i].hrxq =
 			mlx5_hrxq_get(dev,
@@ -2241,7 +2240,7 @@ mlx5_flow_create_action_queue(struct rte_eth_dev *dev,
 			      struct rte_flow *flow,
 			      struct rte_flow_error *error)
 {
-	struct priv *priv = dev->data->dev_private;
+	struct priv *priv __rte_unused = dev->data->dev_private;
 	int ret;
 	unsigned int i;
 	unsigned int flows_n = 0;
@@ -2254,7 +2253,7 @@ mlx5_flow_create_action_queue(struct rte_eth_dev *dev,
 		goto error;
 	if (parser->count)
 		flow->cs = parser->cs;
-	if (!priv->dev->data->dev_started)
+	if (!dev->data->dev_started)
 		return 0;
 	for (i = 0; i != hash_rxq_init_n; ++i) {
 		if (!flow->frxq[i].hrxq)
@@ -3097,9 +3096,9 @@ mlx5_flow_isolate(struct rte_eth_dev *dev,
 	}
 	priv->isolated = !!enable;
 	if (enable)
-		priv->dev->dev_ops = &mlx5_dev_ops_isolate;
+		dev->dev_ops = &mlx5_dev_ops_isolate;
 	else
-		priv->dev->dev_ops = &mlx5_dev_ops;
+		dev->dev_ops = &mlx5_dev_ops;
 	return 0;
 }
 
@@ -3487,11 +3486,10 @@ mlx5_fdir_filter_flush(struct rte_eth_dev *dev)
 static void
 mlx5_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info)
 {
-	struct priv *priv = dev->data->dev_private;
 	struct rte_eth_fdir_masks *mask =
-		&priv->dev->data->dev_conf.fdir_conf.mask;
+		&dev->data->dev_conf.fdir_conf.mask;
 
-	fdir_info->mode = priv->dev->data->dev_conf.fdir_conf.mode;
+	fdir_info->mode = dev->data->dev_conf.fdir_conf.mode;
 	fdir_info->guarant_spc = 0;
 	rte_memcpy(&fdir_info->mask, mask, sizeof(fdir_info->mask));
 	fdir_info->max_flexpayload = 0;
@@ -3519,9 +3517,8 @@ static int
 mlx5_fdir_ctrl_func(struct rte_eth_dev *dev, enum rte_filter_op filter_op,
 		    void *arg)
 {
-	struct priv *priv = dev->data->dev_private;
 	enum rte_fdir_mode fdir_mode =
-		priv->dev->data->dev_conf.fdir_conf.mode;
+		dev->data->dev_conf.fdir_conf.mode;
 
 	if (filter_op == RTE_ETH_FILTER_NOP)
 		return 0;
diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
index 1b4c7ec5d..48ac84bc8 100644
--- a/drivers/net/mlx5/mlx5_mr.c
+++ b/drivers/net/mlx5/mlx5_mr.c
@@ -105,8 +105,8 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 	rte_spinlock_lock(&txq_ctrl->priv->mr_lock);
 	/* Add a new entry, register MR first. */
 	DRV_LOG(DEBUG, "port %u discovered new memory pool \"%s\" (%p)",
-		txq_ctrl->priv->dev->data->port_id, mp->name, (void *)mp);
-	dev = txq_ctrl->priv->dev;
+		PORT_ID(txq_ctrl->priv), mp->name, (void *)mp);
+	dev = ETH_DEV(txq_ctrl->priv);
 	mr = mlx5_mr_get(dev, mp);
 	if (mr == NULL) {
 		if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
@@ -114,8 +114,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 				"port %u using unregistered mempool 0x%p(%s)"
 				" in secondary process, please create mempool"
 				" before rte_eth_dev_start()",
-				txq_ctrl->priv->dev->data->port_id,
-				(void *)mp, mp->name);
+				PORT_ID(txq_ctrl->priv), (void *)mp, mp->name);
 			rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
 			rte_errno = ENOTSUP;
 			return NULL;
@@ -126,7 +125,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 		DRV_LOG(DEBUG,
 			"port %u unable to configure memory region,"
 			" ibv_reg_mr() failed.",
-			txq_ctrl->priv->dev->data->port_id);
+			PORT_ID(txq_ctrl->priv));
 		rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
 		return NULL;
 	}
@@ -135,7 +134,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 		DRV_LOG(DEBUG,
 			"port %u memory region <-> memory pool table full, "
 			" dropping oldest entry",
-			txq_ctrl->priv->dev->data->port_id);
+			PORT_ID(txq_ctrl->priv));
 		--idx;
 		mlx5_mr_release(txq->mp2mr[0]);
 		memmove(&txq->mp2mr[0], &txq->mp2mr[1],
@@ -146,7 +145,7 @@ mlx5_txq_mp2mr_reg(struct mlx5_txq_data *txq, struct rte_mempool *mp,
 	DRV_LOG(DEBUG,
 		"port %u new memory region lkey for MP \"%s\" (%p): 0x%08"
 		PRIu32,
-		txq_ctrl->priv->dev->data->port_id, mp->name, (void *)mp,
+		PORT_ID(txq_ctrl->priv), mp->name, (void *)mp,
 		txq_ctrl->txq.mp2mr[idx]->lkey);
 	rte_spinlock_unlock(&txq_ctrl->priv->mr_lock);
 	return mr;
@@ -207,15 +206,15 @@ mlx5_mp2mr_iter(struct rte_mempool *mp, void *arg)
 	if (rte_mempool_obj_iter(mp, txq_mp2mr_mbuf_check, &data) == 0 ||
 			data.ret == -1)
 		return;
-	mr = mlx5_mr_get(priv->dev, mp);
+	mr = mlx5_mr_get(ETH_DEV(priv), mp);
 	if (mr) {
 		mlx5_mr_release(mr);
 		return;
 	}
-	mr = mlx5_mr_new(priv->dev, mp);
+	mr = mlx5_mr_new(ETH_DEV(priv), mp);
 	if (!mr)
 		DRV_LOG(ERR, "port %u cannot create memory region: %s",
-			priv->dev->data->port_id, strerror(rte_errno));
+			PORT_ID(priv), strerror(rte_errno));
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 126412ddd..aa1ddd0b6 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -78,7 +78,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
 		buf = rte_pktmbuf_alloc(rxq_ctrl->rxq.mp);
 		if (buf == NULL) {
 			DRV_LOG(ERR, "port %u empty mbuf pool",
-				rxq_ctrl->priv->dev->data->port_id);
+				PORT_ID(rxq_ctrl->priv));
 			rte_errno = ENOMEM;
 			goto error;
 		}
@@ -122,7 +122,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
 	DRV_LOG(DEBUG,
 		"port %u Rx queue %u allocated and configured %u segments"
 		" (max %u packets)",
-		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx, elts_n,
+		PORT_ID(rxq_ctrl->priv), rxq_ctrl->idx, elts_n,
 		elts_n / (1 << rxq_ctrl->rxq.sges_n));
 	return 0;
 error:
@@ -134,7 +134,7 @@ rxq_alloc_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
 		(*rxq_ctrl->rxq.elts)[i] = NULL;
 	}
 	DRV_LOG(DEBUG, "port %u Rx queue %u failed, freed everything",
-		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
+		PORT_ID(rxq_ctrl->priv), rxq_ctrl->idx);
 	rte_errno = err; /* Restore rte_errno. */
 	return -rte_errno;
 }
@@ -155,7 +155,7 @@ rxq_free_elts(struct mlx5_rxq_ctrl *rxq_ctrl)
 	uint16_t i;
 
 	DRV_LOG(DEBUG, "port %u Rx queue %u freeing WRs",
-		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
+		PORT_ID(rxq_ctrl->priv), rxq_ctrl->idx);
 	if (rxq->elts == NULL)
 		return;
 	/**
@@ -186,7 +186,7 @@ void
 mlx5_rxq_cleanup(struct mlx5_rxq_ctrl *rxq_ctrl)
 {
 	DRV_LOG(DEBUG, "port %u cleaning up Rx queue %u",
-		rxq_ctrl->priv->dev->data->port_id, rxq_ctrl->idx);
+		PORT_ID(rxq_ctrl->priv), rxq_ctrl->idx);
 	if (rxq_ctrl->ibv)
 		mlx5_rxq_ibv_release(rxq_ctrl->ibv);
 	memset(rxq_ctrl, 0, sizeof(*rxq_ctrl));
@@ -354,11 +354,11 @@ mlx5_rx_queue_release(void *dpdk_rxq)
 		return;
 	rxq_ctrl = container_of(rxq, struct mlx5_rxq_ctrl, rxq);
 	priv = rxq_ctrl->priv;
-	if (!mlx5_rxq_releasable(priv->dev, rxq_ctrl->rxq.stats.idx))
+	if (!mlx5_rxq_releasable(ETH_DEV(priv), rxq_ctrl->rxq.stats.idx))
 		rte_panic("port %u Rx queue %u is still used by a flow and"
-			  " cannot be removed\n", priv->dev->data->port_id,
-			  rxq_ctrl->idx);
-	mlx5_rxq_release(priv->dev, rxq_ctrl->rxq.stats.idx);
+			  " cannot be removed\n",
+			  PORT_ID(priv), rxq_ctrl->idx);
+	mlx5_rxq_release(ETH_DEV(priv), rxq_ctrl->rxq.stats.idx);
 }
 
 /**
@@ -378,9 +378,9 @@ mlx5_rx_intr_vec_enable(struct rte_eth_dev *dev)
 	unsigned int rxqs_n = priv->rxqs_n;
 	unsigned int n = RTE_MIN(rxqs_n, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
 	unsigned int count = 0;
-	struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
+	struct rte_intr_handle *intr_handle = dev->intr_handle;
 
-	if (!priv->dev->data->dev_conf.intr_conf.rxq)
+	if (!dev->data->dev_conf.intr_conf.rxq)
 		return 0;
 	mlx5_rx_intr_vec_disable(dev);
 	intr_handle->intr_vec = malloc(n * sizeof(intr_handle->intr_vec[0]));
@@ -452,12 +452,12 @@ void
 mlx5_rx_intr_vec_disable(struct rte_eth_dev *dev)
 {
 	struct priv *priv = dev->data->dev_private;
-	struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
+	struct rte_intr_handle *intr_handle = dev->intr_handle;
 	unsigned int i;
 	unsigned int rxqs_n = priv->rxqs_n;
 	unsigned int n = RTE_MIN(rxqs_n, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
 
-	if (!priv->dev->data->dev_conf.intr_conf.rxq)
+	if (!dev->data->dev_conf.intr_conf.rxq)
 		return;
 	if (!intr_handle->intr_vec)
 		goto free;
@@ -897,7 +897,7 @@ mlx5_rxq_ibv_release(struct mlx5_rxq_ibv *rxq_ibv)
 	if (!ret)
 		rxq_ibv->mr = NULL;
 	DRV_LOG(DEBUG, "port %u Verbs Rx queue %u: refcnt %d",
-		rxq_ibv->rxq_ctrl->priv->dev->data->port_id,
+		PORT_ID(rxq_ibv->rxq_ctrl->priv),
 		rxq_ibv->rxq_ctrl->idx, rte_atomic32_read(&rxq_ibv->refcnt));
 	if (rte_atomic32_dec_and_test(&rxq_ibv->refcnt)) {
 		rxq_free_elts(rxq_ibv->rxq_ctrl);
@@ -990,7 +990,7 @@ mlx5_rxq_new(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc,
 		return NULL;
 	}
 	tmpl->socket = socket;
-	if (priv->dev->data->dev_conf.intr_conf.rxq)
+	if (dev->data->dev_conf.intr_conf.rxq)
 		tmpl->irq = 1;
 	/* Enable scattered packets support for this queue if necessary. */
 	assert(mb_len >= RTE_PKTMBUF_HEADROOM);
@@ -1328,8 +1328,8 @@ mlx5_ind_table_ibv_release(struct rte_eth_dev *dev,
 	unsigned int i;
 
 	DRV_LOG(DEBUG, "port %u indirection table %p: refcnt %d",
-		((struct priv *)dev->data->dev_private)->port,
-		(void *)ind_tbl, rte_atomic32_read(&ind_tbl->refcnt));
+		dev->data->port_id, (void *)ind_tbl,
+		rte_atomic32_read(&ind_tbl->refcnt));
 	if (rte_atomic32_dec_and_test(&ind_tbl->refcnt)) {
 		claim_zero(mlx5_glue->destroy_rwq_ind_table
 			   (ind_tbl->ind_table));
@@ -1589,8 +1589,8 @@ int
 mlx5_hrxq_release(struct rte_eth_dev *dev, struct mlx5_hrxq *hrxq)
 {
 	DRV_LOG(DEBUG, "port %u hash Rx queue %p: refcnt %d",
-		((struct priv *)dev->data->dev_private)->port,
-		(void *)hrxq, rte_atomic32_read(&hrxq->refcnt));
+		dev->data->port_id, (void *)hrxq,
+		rte_atomic32_read(&hrxq->refcnt));
 	if (rte_atomic32_dec_and_test(&hrxq->refcnt)) {
 		claim_zero(mlx5_glue->destroy_qp(hrxq->qp));
 		DEBUG("port %u delete QP %p: hash: 0x%" PRIx64 ", tunnel:"
diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index e05d1a0e8..af2537379 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -48,7 +48,7 @@ txq_alloc_elts(struct mlx5_txq_ctrl *txq_ctrl)
 	for (i = 0; (i != elts_n); ++i)
 		(*txq_ctrl->txq.elts)[i] = NULL;
 	DRV_LOG(DEBUG, "port %u Tx queue %u allocated and configured %u WRs",
-		txq_ctrl->priv->dev->data->port_id, txq_ctrl->idx, elts_n);
+		PORT_ID(txq_ctrl->priv), txq_ctrl->idx, elts_n);
 	txq_ctrl->txq.elts_head = 0;
 	txq_ctrl->txq.elts_tail = 0;
 	txq_ctrl->txq.elts_comp = 0;
@@ -70,7 +70,7 @@ txq_free_elts(struct mlx5_txq_ctrl *txq_ctrl)
 	struct rte_mbuf *(*elts)[elts_n] = txq_ctrl->txq.elts;
 
 	DRV_LOG(DEBUG, "port %u Tx queue %u freeing WRs",
-		txq_ctrl->priv->dev->data->port_id, txq_ctrl->idx);
+		PORT_ID(txq_ctrl->priv), txq_ctrl->idx);
 	txq_ctrl->txq.elts_head = 0;
 	txq_ctrl->txq.elts_tail = 0;
 	txq_ctrl->txq.elts_comp = 0;
@@ -255,9 +255,9 @@ mlx5_tx_queue_release(void *dpdk_txq)
 	priv = txq_ctrl->priv;
 	for (i = 0; (i != priv->txqs_n); ++i)
 		if ((*priv->txqs)[i] == txq) {
-			mlx5_txq_release(priv->dev, i);
+			mlx5_txq_release(ETH_DEV(priv), i);
 			DRV_LOG(DEBUG, "port %u removing Tx queue %u from list",
-				priv->dev->data->port_id, txq_ctrl->idx);
+				PORT_ID(priv), txq_ctrl->idx);
 			break;
 		}
 }
@@ -618,7 +618,7 @@ mlx5_txq_ibv_release(struct mlx5_txq_ibv *txq_ibv)
 {
 	assert(txq_ibv);
 	DRV_LOG(DEBUG, "port %u Verbs Tx queue %u: refcnt %d",
-		txq_ibv->txq_ctrl->priv->dev->data->port_id,
+		PORT_ID(txq_ibv->txq_ctrl->priv),
 		txq_ibv->txq_ctrl->idx, rte_atomic32_read(&txq_ibv->refcnt));
 	if (rte_atomic32_dec_and_test(&txq_ibv->refcnt)) {
 		claim_zero(mlx5_glue->destroy_qp(txq_ibv->qp));
@@ -685,7 +685,7 @@ txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)
 	unsigned int txqs_inline;
 	unsigned int inline_max_packet_sz;
 	eth_tx_burst_t tx_pkt_burst =
-		mlx5_select_tx_function(txq_ctrl->priv->dev);
+		mlx5_select_tx_function(ETH_DEV(priv));
 	int is_empw_func = is_empw_burst_func(tx_pkt_burst);
 	int tso = !!(txq_ctrl->txq.offloads & (DEV_TX_OFFLOAD_TCP_TSO |
 					       DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
@@ -747,8 +747,7 @@ txq_set_params(struct mlx5_txq_ctrl *txq_ctrl)
 			DRV_LOG(WARNING,
 				"port %u txq inline is too large (%d) setting"
 				" it to the maximum possible: %d\n",
-				priv->dev->data->port_id, txq_inline,
-				max_inline);
+				PORT_ID(priv), txq_inline, max_inline);
 			txq_ctrl->txq.max_inline = max_inline /
 						   RTE_CACHE_LINE_SIZE;
 		}
-- 
2.11.0

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

* Re: [dpdk-dev] [PATCH v3] net/mlx5: change device reference for secondary process
  2018-05-09 11:04 ` [dpdk-dev] [PATCH v3] " Yongseok Koh
@ 2018-05-09 11:30   ` Shahaf Shuler
  0 siblings, 0 replies; 8+ messages in thread
From: Shahaf Shuler @ 2018-05-09 11:30 UTC (permalink / raw)
  To: Yongseok Koh, Adrien Mazarguil, Nélio Laranjeiro; +Cc: dev, Yongseok Koh

Wednesday, May 9, 2018 2:05 PM, Yongseok Koh:
> Subject: [dpdk-dev] [PATCH v3] net/mlx5: change device reference for
> secondary process
> 
> rte_eth_devices[] is not shared between primary and secondary process,
> but a static array to each process. The reverse pointer of device (priv->dev) is
> invalid. Instead, priv has the pointer to shared data of the device,
>   struct rte_eth_dev_data *dev_data;
> 
> Two macros are added,
>   #define PORT_ID(priv) ((priv)->dev_data->port_id)
>   #define ETH_DEV(priv) (&rte_eth_devices[PORT_ID(priv)])
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

applied to next-net-mlx. thanks

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

end of thread, other threads:[~2018-05-09 11:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-02  6:13 [dpdk-dev] [PATCH] net/mlx5: fix device reference in secondary process Yongseok Koh
2018-05-02  6:53 ` Nélio Laranjeiro
2018-05-02  7:12   ` Yongseok Koh
2018-05-02 23:10 ` [dpdk-dev] [PATCH v2] net/mlx5: change device reference for " Yongseok Koh
2018-05-06  6:25   ` Shahaf Shuler
2018-05-07  6:46     ` Yongseok Koh
2018-05-09 11:04 ` [dpdk-dev] [PATCH v3] " Yongseok Koh
2018-05-09 11:30   ` Shahaf Shuler

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