[PATCH 0/3] clean-up on virtual PMDs

[PATCH 0/3] clean-up on virtual PMDs

[Ferruh Yigit]

This is a clean-up patch, no defect fixed, no functional difference 
expected.

Patch mainly removes duplicated fields between data->dev_private
and data (struct rte_eth_dev_data).

There are a few minor cleanups that:
pcap: move a common code into a function
ring: remove duplicated data->rx/tx_queues allocation and assignment


Ferruh Yigit (3):
  pcap: remove duplicate fields in internal data struct
  ring: remove duplicate fields in internal data struct
  null: remove duplicate fields in internal data struct

 drivers/net/null/rte_eth_null.c |  36 ++++-------
 drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
 drivers/net/ring/rte_eth_ring.c |  52 +++++-----------
 3 files changed, 87 insertions(+), 131 deletions(-)

-- 
2.5.0

[PATCH 1/3] pcap: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- Remove duplicate nb_rx/tx_queues fields from internals
2- Move duplicate code into a common function

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
 1 file changed, 61 insertions(+), 69 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index f9230eb..c8b7dbd 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -103,8 +103,6 @@ struct tx_pcaps {
 struct pmd_internals {
 	struct pcap_rx_queue rx_queue[RTE_PMD_RING_MAX_RX_RINGS];
 	struct pcap_tx_queue tx_queue[RTE_PMD_RING_MAX_TX_RINGS];
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
 	int if_index;
 	int single_iface;
 };
@@ -396,7 +394,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* If not open already, open tx pcaps/dumpers */
-	for (i = 0; i < internals->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		tx = &internals->tx_queue[i];
 
 		if (!tx->dumper && strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) {
@@ -411,7 +409,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* If not open already, open rx pcaps */
-	for (i = 0; i < internals->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		rx = &internals->rx_queue[i];
 
 		if (rx->pcap != NULL)
@@ -457,7 +455,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 		goto status_down;
 	}
 
-	for (i = 0; i < internals->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		tx = &internals->tx_queue[i];
 
 		if (tx->dumper != NULL) {
@@ -471,7 +469,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 		}
 	}
 
-	for (i = 0; i < internals->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		rx = &internals->rx_queue[i];
 
 		if (rx->pcap != NULL) {
@@ -499,8 +497,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->if_index = internals->if_index;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_pktlen = (uint32_t) -1;
-	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
+	dev_info->max_rx_queues = dev->data->nb_rx_queues;
+	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
 }
@@ -515,16 +513,16 @@ eth_stats_get(struct rte_eth_dev *dev,
 	unsigned long tx_packets_err_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
-	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_rx_queues;
-			i++) {
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < dev->data->nb_rx_queues; i++) {
 		igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
 		igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
 		rx_packets_total += igb_stats->q_ipackets[i];
 		rx_bytes_total += igb_stats->q_ibytes[i];
 	}
 
-	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_tx_queues;
-			i++) {
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < dev->data->nb_tx_queues; i++) {
 		igb_stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
 		igb_stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
 		igb_stats->q_errors[i] = internal->tx_queue[i].err_pkts;
@@ -545,11 +543,11 @@ eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		internal->rx_queue[i].rx_pkts = 0;
 		internal->rx_queue[i].rx_bytes = 0;
 	}
-	for (i = 0; i < internal->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		internal->tx_queue[i].tx_pkts = 0;
 		internal->tx_queue[i].tx_bytes = 0;
 		internal->tx_queue[i].err_pkts = 0;
@@ -840,9 +838,6 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
-	(*internals)->nb_rx_queues = nb_rx_queues;
-	(*internals)->nb_tx_queues = nb_tx_queues;
-
 	if (pair == NULL)
 		(*internals)->if_index = 0;
 	else
@@ -860,11 +855,11 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
 
 	(*eth_dev)->data = data;
 	(*eth_dev)->dev_ops = &ops;
-	(*eth_dev)->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	(*eth_dev)->driver = NULL;
-	(*eth_dev)->data->kdrv = RTE_KDRV_NONE;
-	(*eth_dev)->data->drv_name = drivername;
-	(*eth_dev)->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	return 0;
 
@@ -876,16 +871,12 @@ error:
 }
 
 static int
-rte_eth_from_pcaps_n_dumpers(const char *name,
-		struct rx_pcaps *rx_queues,
-		const unsigned nb_rx_queues,
-		struct tx_pcaps *tx_queues,
-		const unsigned nb_tx_queues,
-		const unsigned numa_node,
-		struct rte_kvargs *kvlist)
+rte_eth_from_pcaps_common(const char *name, struct rx_pcaps *rx_queues,
+		const unsigned nb_rx_queues, struct tx_pcaps *tx_queues,
+		const unsigned nb_tx_queues, const unsigned numa_node,
+		struct rte_kvargs *kvlist, struct pmd_internals **internals,
+		struct rte_eth_dev **eth_dev)
 {
-	struct pmd_internals *internals = NULL;
-	struct rte_eth_dev *eth_dev = NULL;
 	unsigned i;
 
 	/* do some parameter checking */
@@ -895,28 +886,51 @@ rte_eth_from_pcaps_n_dumpers(const char *name,
 		return -1;
 
 	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
-			&internals, &eth_dev, kvlist) < 0)
+			internals, eth_dev, kvlist) < 0)
 		return -1;
 
 	for (i = 0; i < nb_rx_queues; i++) {
-		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
-		snprintf(internals->rx_queue[i].name,
-			sizeof(internals->rx_queue[i].name), "%s",
+		(*internals)->rx_queue[i].pcap = rx_queues->pcaps[i];
+		snprintf((*internals)->rx_queue[i].name,
+			sizeof((*internals)->rx_queue[i].name), "%s",
 			rx_queues->names[i]);
-		snprintf(internals->rx_queue[i].type,
-			sizeof(internals->rx_queue[i].type), "%s",
+		snprintf((*internals)->rx_queue[i].type,
+			sizeof((*internals)->rx_queue[i].type), "%s",
 			rx_queues->types[i]);
 	}
 	for (i = 0; i < nb_tx_queues; i++) {
-		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
-		snprintf(internals->tx_queue[i].name,
-			sizeof(internals->tx_queue[i].name), "%s",
+		(*internals)->tx_queue[i].dumper = tx_queues->dumpers[i];
+		snprintf((*internals)->tx_queue[i].name,
+			sizeof((*internals)->tx_queue[i].name), "%s",
 			tx_queues->names[i]);
-		snprintf(internals->tx_queue[i].type,
-			sizeof(internals->tx_queue[i].type), "%s",
+		snprintf((*internals)->tx_queue[i].type,
+			sizeof((*internals)->tx_queue[i].type), "%s",
 			tx_queues->types[i]);
 	}
 
+	return 0;
+}
+
+static int
+rte_eth_from_pcaps_n_dumpers(const char *name,
+		struct rx_pcaps *rx_queues,
+		const unsigned nb_rx_queues,
+		struct tx_pcaps *tx_queues,
+		const unsigned nb_tx_queues,
+		const unsigned numa_node,
+		struct rte_kvargs *kvlist)
+{
+	struct pmd_internals *internals = NULL;
+	struct rte_eth_dev *eth_dev = NULL;
+	int ret;
+
+	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+			tx_queues, nb_tx_queues, numa_node, kvlist,
+			&internals, &eth_dev);
+
+	if (ret < 0)
+		return ret;
+
 	/* using multiple pcaps/interfaces */
 	internals->single_iface = 0;
 
@@ -938,36 +952,14 @@ rte_eth_from_pcaps(const char *name,
 {
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
-	unsigned i;
-
-	/* do some parameter checking */
-	if (rx_queues == NULL && nb_rx_queues > 0)
-		return -1;
-	if (tx_queues == NULL && nb_tx_queues > 0)
-		return -1;
+	int ret;
 
-	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
-			&internals, &eth_dev, kvlist) < 0)
-		return -1;
+	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+			tx_queues, nb_tx_queues, numa_node, kvlist,
+			&internals, &eth_dev);
 
-	for (i = 0; i < nb_rx_queues; i++) {
-		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
-		snprintf(internals->rx_queue[i].name,
-			sizeof(internals->rx_queue[i].name), "%s",
-			rx_queues->names[i]);
-		snprintf(internals->rx_queue[i].type,
-			sizeof(internals->rx_queue[i].type), "%s",
-			rx_queues->types[i]);
-	}
-	for (i = 0; i < nb_tx_queues; i++) {
-		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
-		snprintf(internals->tx_queue[i].name,
-			sizeof(internals->tx_queue[i].name), "%s",
-			tx_queues->names[i]);
-		snprintf(internals->tx_queue[i].type,
-			sizeof(internals->tx_queue[i].type), "%s",
-			tx_queues->types[i]);
-	}
+	if (ret < 0)
+		return ret;
 
 	/* store wether we are using a single interface for rx/tx or not */
 	internals->single_iface = single_iface;
-- 
2.5.0

[PATCH 2/3] ring: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- Remove duplicate nb_rx/tx_queues fields from internals
2- Remove data->rx/tx_queues allocation, whose auto allocated by
libether
3- Remove duplicate data->rx/tx_queues[i] assignments

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/ring/rte_eth_ring.c | 52 +++++++++++------------------------------
 1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index d92b088..47dd5f2 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -59,9 +59,6 @@ struct ring_queue {
 };
 
 struct pmd_internals {
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
-
 	struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
 	struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
 
@@ -138,7 +135,7 @@ eth_dev_set_link_up(struct rte_eth_dev *dev)
 }
 
 static int
-eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
+eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 				    uint16_t nb_rx_desc __rte_unused,
 				    unsigned int socket_id __rte_unused,
 				    const struct rte_eth_rxconf *rx_conf __rte_unused,
@@ -165,12 +162,11 @@ static void
 eth_dev_info(struct rte_eth_dev *dev,
 		struct rte_eth_dev_info *dev_info)
 {
-	struct pmd_internals *internals = dev->data->dev_private;
 	dev_info->driver_name = drivername;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_pktlen = (uint32_t)-1;
-	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
+	dev_info->max_rx_queues = dev->data->nb_rx_queues;
+	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
 }
@@ -183,13 +179,13 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 	const struct pmd_internals *internal = dev->data->dev_private;
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
-			i < internal->nb_rx_queues; i++) {
+			i < dev->data->nb_rx_queues; i++) {
 		igb_stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
 		rx_total += igb_stats->q_ipackets[i];
 	}
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
-			i < internal->nb_tx_queues; i++) {
+			i < dev->data->nb_tx_queues; i++) {
 		igb_stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
 		igb_stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
 		tx_total += igb_stats->q_opackets[i];
@@ -206,9 +202,9 @@ eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++)
+	for (i = 0; i < dev->data->nb_rx_queues; i++)
 		internal->rx_ring_queues[i].rx_pkts.cnt = 0;
-	for (i = 0; i < internal->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		internal->tx_ring_queues[i].tx_pkts.cnt = 0;
 		internal->tx_ring_queues[i].err_pkts.cnt = 0;
 	}
@@ -262,7 +258,6 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	struct rte_eth_dev_data *data = NULL;
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
-
 	unsigned i;
 
 	/* do some parameter checking */
@@ -291,20 +286,6 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 		goto error;
 	}
 
-	data->rx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_rx_queues,
-			0, numa_node);
-	if (data->rx_queues == NULL) {
-		rte_errno = ENOMEM;
-		goto error;
-	}
-
-	data->tx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_tx_queues,
-			0, numa_node);
-	if (data->tx_queues == NULL) {
-		rte_errno = ENOMEM;
-		goto error;
-	}
-
 	internals = rte_zmalloc_socket(name, sizeof(*internals), 0, numa_node);
 	if (internals == NULL) {
 		rte_errno = ENOMEM;
@@ -327,16 +308,11 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
-	internals->nb_rx_queues = nb_rx_queues;
-	internals->nb_tx_queues = nb_tx_queues;
-	for (i = 0; i < nb_rx_queues; i++) {
+	for (i = 0; i < nb_rx_queues; i++)
 		internals->rx_ring_queues[i].rng = rx_queues[i];
-		data->rx_queues[i] = &internals->rx_ring_queues[i];
-	}
-	for (i = 0; i < nb_tx_queues; i++) {
+
+	for (i = 0; i < nb_tx_queues; i++)
 		internals->tx_ring_queues[i].rng = tx_queues[i];
-		data->tx_queues[i] = &internals->tx_ring_queues[i];
-	}
 
 	data->dev_private = internals;
 	data->port_id = eth_dev->data->port_id;
@@ -349,10 +325,10 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	eth_dev->data = data;
 	eth_dev->driver = NULL;
 	eth_dev->dev_ops = &ops;
-	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-	eth_dev->data->kdrv = RTE_KDRV_NONE;
-	eth_dev->data->drv_name = drivername;
-	eth_dev->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	TAILQ_INIT(&(eth_dev->link_intr_cbs));
 
-- 
2.5.0

Re: [PATCH 2/3] ring: remove duplicate fields in internal data struct

[Bruce Richardson]

On Fri, Jan 29, 2016 at 05:16:21PM +0000, Ferruh Yigit wrote:
> 1- Remove duplicate nb_rx/tx_queues fields from internals
> 2- Remove data->rx/tx_queues allocation, whose auto allocated by
> libether
> 3- Remove duplicate data->rx/tx_queues[i] assignments
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  drivers/net/ring/rte_eth_ring.c | 52 +++++++++++------------------------------
>  1 file changed, 14 insertions(+), 38 deletions(-)
> 

Hi Ferruh,

does this patch not break rte_eth_from_ring() and rte_eth_from_rings() since
the "auto allocation" you refer to is only performed on eth_dev_configure, I
believe. Currently, you can create an rte_ring and then use it as though it
were an ethdev by calling:
	port_id = rte_eth_from_ring(r)

With this change, I believe the user instead has to now call eth_from_ring,
then do a series of configure and queue setup calls just to make the new
ethdev usable.

Regards.
/Bruce

Re: [PATCH 2/3] ring: remove duplicate fields in internal data struct

[Ferruh Yigit]

On Wed, Feb 17, 2016 at 05:36:55PM +0000, Bruce Richardson wrote:
> On Fri, Jan 29, 2016 at 05:16:21PM +0000, Ferruh Yigit wrote:
> > 1- Remove duplicate nb_rx/tx_queues fields from internals
> > 2- Remove data->rx/tx_queues allocation, whose auto allocated by
> > libether
> > 3- Remove duplicate data->rx/tx_queues[i] assignments
> > 
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> >  drivers/net/ring/rte_eth_ring.c | 52 +++++++++++------------------------------
> >  1 file changed, 14 insertions(+), 38 deletions(-)
> > 
> 
> Hi Ferruh,
Hi Bruce,
> 
> does this patch not break rte_eth_from_ring() and rte_eth_from_rings() since
> the "auto allocation" you refer to is only performed on eth_dev_configure, I
> believe. Currently, you can create an rte_ring and then use it as though it
> were an ethdev by calling:
> 	port_id = rte_eth_from_ring(r)
> 
> With this change, I believe the user instead has to now call eth_from_ring,
> then do a series of configure and queue setup calls just to make the new
> ethdev usable.
> 
I wasn't aware requirement to use without config and setup calls, yes this patch breaks it. I will update the patch to recover this kind of usage.

Thanks,
ferruh

[PATCH 3/3] null: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- remove duplicate nb_rx/tx_queues fields from internals
2- remove duplicate numa_node field from internals

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/null/rte_eth_null.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 77fc988..1c354ad 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -69,10 +69,6 @@ struct null_queue {
 struct pmd_internals {
 	unsigned packet_size;
 	unsigned packet_copy;
-	unsigned numa_node;
-
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
 
 	struct null_queue rx_null_queues[RTE_MAX_QUEUES_PER_PORT];
 	struct null_queue tx_null_queues[RTE_MAX_QUEUES_PER_PORT];
@@ -192,13 +188,8 @@ eth_null_copy_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 }
 
 static int
-eth_dev_configure(struct rte_eth_dev *dev) {
-	struct pmd_internals *internals;
-
-	internals = dev->data->dev_private;
-	internals->nb_rx_queues = dev->data->nb_rx_queues;
-	internals->nb_tx_queues = dev->data->nb_tx_queues;
-
+eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
+{
 	return 0;
 }
 
@@ -237,7 +228,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 
 	internals = dev->data->dev_private;
 
-	if (rx_queue_id >= internals->nb_rx_queues)
+	if (rx_queue_id >= dev->data->nb_rx_queues)
 		return -ENODEV;
 
 	packet_size = internals->packet_size;
@@ -246,7 +237,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 	dev->data->rx_queues[rx_queue_id] =
 		&internals->rx_null_queues[rx_queue_id];
 	dummy_packet = rte_zmalloc_socket(NULL,
-			packet_size, 0, internals->numa_node);
+			packet_size, 0, dev->data->numa_node);
 	if (dummy_packet == NULL)
 		return -ENOMEM;
 
@@ -271,7 +262,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 
 	internals = dev->data->dev_private;
 
-	if (tx_queue_id >= internals->nb_tx_queues)
+	if (tx_queue_id >= dev->data->nb_tx_queues)
 		return -ENODEV;
 
 	packet_size = internals->packet_size;
@@ -279,7 +270,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 	dev->data->tx_queues[tx_queue_id] =
 		&internals->tx_null_queues[tx_queue_id];
 	dummy_packet = rte_zmalloc_socket(NULL,
-			packet_size, 0, internals->numa_node);
+			packet_size, 0, dev->data->numa_node);
 	if (dummy_packet == NULL)
 		return -ENOMEM;
 
@@ -323,7 +314,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 
 	internal = dev->data->dev_private;
 	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
-			RTE_MIN(internal->nb_rx_queues,
+			RTE_MIN(dev->data->nb_rx_queues,
 				RTE_DIM(internal->rx_null_queues)));
 	for (i = 0; i < num_stats; i++) {
 		igb_stats->q_ipackets[i] =
@@ -332,7 +323,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 	}
 
 	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
-			RTE_MIN(internal->nb_tx_queues,
+			RTE_MIN(dev->data->nb_tx_queues,
 				RTE_DIM(internal->tx_null_queues)));
 	for (i = 0; i < num_stats; i++) {
 		igb_stats->q_opackets[i] =
@@ -535,11 +526,8 @@ eth_dev_null_create(const char *name,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the nulls are local per-process */
 
-	internals->nb_rx_queues = nb_rx_queues;
-	internals->nb_tx_queues = nb_tx_queues;
 	internals->packet_size = packet_size;
 	internals->packet_copy = packet_copy;
-	internals->numa_node = numa_node;
 
 	internals->flow_type_rss_offloads =  ETH_RSS_PROTO_MASK;
 	internals->reta_size = RTE_DIM(internals->reta_conf) * RTE_RETA_GROUP_SIZE;
@@ -560,10 +548,10 @@ eth_dev_null_create(const char *name,
 	TAILQ_INIT(&eth_dev->link_intr_cbs);
 
 	eth_dev->driver = NULL;
-	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-	eth_dev->data->kdrv = RTE_KDRV_NONE;
-	eth_dev->data->drv_name = drivername;
-	eth_dev->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	/* finally assign rx and tx ops */
 	if (packet_copy) {
-- 
2.5.0

Re: [PATCH 3/3] null: remove duplicate fields in internal data struct

[Tetsuya Mukawa]

On 2016/01/30 2:16, Ferruh Yigit wrote:
> 1- remove duplicate nb_rx/tx_queues fields from internals
> 2- remove duplicate numa_node field from internals
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  drivers/net/null/rte_eth_null.c | 36 ++++++++++++------------------------
>  1 file changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> index 77fc988..1c354ad 100644
> --- a/drivers/net/null/rte_eth_null.c
> +++ b/drivers/net/null/rte_eth_null.c
> @@ -69,10 +69,6 @@ struct null_queue {
>  struct pmd_internals {
>  	unsigned packet_size;
>  	unsigned packet_copy;
> -	unsigned numa_node;
> -
> -	unsigned nb_rx_queues;
> -	unsigned nb_tx_queues;
>  
>  	struct null_queue rx_null_queues[RTE_MAX_QUEUES_PER_PORT];
>  	struct null_queue tx_null_queues[RTE_MAX_QUEUES_PER_PORT];
> @@ -192,13 +188,8 @@ eth_null_copy_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>  }
>  
>  static int
> -eth_dev_configure(struct rte_eth_dev *dev) {
> -	struct pmd_internals *internals;
> -
> -	internals = dev->data->dev_private;
> -	internals->nb_rx_queues = dev->data->nb_rx_queues;
> -	internals->nb_tx_queues = dev->data->nb_tx_queues;
> -
> +eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
> +{
>  	return 0;
>  }
>  
> @@ -237,7 +228,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
>  
>  	internals = dev->data->dev_private;
>  
> -	if (rx_queue_id >= internals->nb_rx_queues)
> +	if (rx_queue_id >= dev->data->nb_rx_queues)
>  		return -ENODEV;
>  
>  	packet_size = internals->packet_size;
> @@ -246,7 +237,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
>  	dev->data->rx_queues[rx_queue_id] =
>  		&internals->rx_null_queues[rx_queue_id];
>  	dummy_packet = rte_zmalloc_socket(NULL,
> -			packet_size, 0, internals->numa_node);
> +			packet_size, 0, dev->data->numa_node);
>  	if (dummy_packet == NULL)
>  		return -ENOMEM;
>  
> @@ -271,7 +262,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
>  
>  	internals = dev->data->dev_private;
>  
> -	if (tx_queue_id >= internals->nb_tx_queues)
> +	if (tx_queue_id >= dev->data->nb_tx_queues)
>  		return -ENODEV;
>  
>  	packet_size = internals->packet_size;
> @@ -279,7 +270,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
>  	dev->data->tx_queues[tx_queue_id] =
>  		&internals->tx_null_queues[tx_queue_id];
>  	dummy_packet = rte_zmalloc_socket(NULL,
> -			packet_size, 0, internals->numa_node);
> +			packet_size, 0, dev->data->numa_node);
>  	if (dummy_packet == NULL)
>  		return -ENOMEM;
>  
> @@ -323,7 +314,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
>  
>  	internal = dev->data->dev_private;
>  	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
> -			RTE_MIN(internal->nb_rx_queues,
> +			RTE_MIN(dev->data->nb_rx_queues,
>  				RTE_DIM(internal->rx_null_queues)));
>  	for (i = 0; i < num_stats; i++) {
>  		igb_stats->q_ipackets[i] =
> @@ -332,7 +323,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
>  	}
>  
>  	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
> -			RTE_MIN(internal->nb_tx_queues,
> +			RTE_MIN(dev->data->nb_tx_queues,
>  				RTE_DIM(internal->tx_null_queues)));
>  	for (i = 0; i < num_stats; i++) {
>  		igb_stats->q_opackets[i] =
> @@ -535,11 +526,8 @@ eth_dev_null_create(const char *name,
>  	/* NOTE: we'll replace the data element, of originally allocated eth_dev
>  	 * so the nulls are local per-process */
>  
> -	internals->nb_rx_queues = nb_rx_queues;
> -	internals->nb_tx_queues = nb_tx_queues;
>  	internals->packet_size = packet_size;
>  	internals->packet_copy = packet_copy;
> -	internals->numa_node = numa_node;
>  
>  	internals->flow_type_rss_offloads =  ETH_RSS_PROTO_MASK;
>  	internals->reta_size = RTE_DIM(internals->reta_conf) * RTE_RETA_GROUP_SIZE;
> @@ -560,10 +548,10 @@ eth_dev_null_create(const char *name,
>  	TAILQ_INIT(&eth_dev->link_intr_cbs);
>  
>  	eth_dev->driver = NULL;
> -	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
> -	eth_dev->data->kdrv = RTE_KDRV_NONE;
> -	eth_dev->data->drv_name = drivername;
> -	eth_dev->data->numa_node = numa_node;
> +	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
> +	data->kdrv = RTE_KDRV_NONE;
> +	data->drv_name = drivername;
> +	data->numa_node = numa_node;
>  
>  	/* finally assign rx and tx ops */
>  	if (packet_copy) {
Tested-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Acked-by: Tetsuya Mukawa <mukawa@igel.co.jp>

[PATCH v2 0/3] clean-up on virtual PMDs

[Ferruh Yigit]

This is a clean-up patch, no defect fixed, no functional difference 
expected.

Patch mainly removes duplicated fields between data->dev_private
and data (struct rte_eth_dev_data).

Also pcap has minor update:
pcap: move a common code into a function

Ferruh Yigit (3):
  pcap: remove duplicate fields in internal data struct
  ring: remove duplicate fields in internal data struct
  null: remove duplicate fields in internal data struct

 drivers/net/null/rte_eth_null.c |  36 ++++-------
 drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
 drivers/net/ring/rte_eth_ring.c |  57 ++++++++----------
 3 files changed, 98 insertions(+), 125 deletions(-)

-- 
2.5.0

[PATCH v2 1/3] pcap: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- Remove duplicate nb_rx/tx_queues fields from internals
2- Move duplicate code into a common function

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
 1 file changed, 61 insertions(+), 69 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index f9230eb..c8b7dbd 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -103,8 +103,6 @@ struct tx_pcaps {
 struct pmd_internals {
 	struct pcap_rx_queue rx_queue[RTE_PMD_RING_MAX_RX_RINGS];
 	struct pcap_tx_queue tx_queue[RTE_PMD_RING_MAX_TX_RINGS];
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
 	int if_index;
 	int single_iface;
 };
@@ -396,7 +394,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* If not open already, open tx pcaps/dumpers */
-	for (i = 0; i < internals->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		tx = &internals->tx_queue[i];
 
 		if (!tx->dumper && strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) {
@@ -411,7 +409,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* If not open already, open rx pcaps */
-	for (i = 0; i < internals->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		rx = &internals->rx_queue[i];
 
 		if (rx->pcap != NULL)
@@ -457,7 +455,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 		goto status_down;
 	}
 
-	for (i = 0; i < internals->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		tx = &internals->tx_queue[i];
 
 		if (tx->dumper != NULL) {
@@ -471,7 +469,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 		}
 	}
 
-	for (i = 0; i < internals->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		rx = &internals->rx_queue[i];
 
 		if (rx->pcap != NULL) {
@@ -499,8 +497,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->if_index = internals->if_index;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_pktlen = (uint32_t) -1;
-	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
+	dev_info->max_rx_queues = dev->data->nb_rx_queues;
+	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
 }
@@ -515,16 +513,16 @@ eth_stats_get(struct rte_eth_dev *dev,
 	unsigned long tx_packets_err_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
-	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_rx_queues;
-			i++) {
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < dev->data->nb_rx_queues; i++) {
 		igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
 		igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
 		rx_packets_total += igb_stats->q_ipackets[i];
 		rx_bytes_total += igb_stats->q_ibytes[i];
 	}
 
-	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_tx_queues;
-			i++) {
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < dev->data->nb_tx_queues; i++) {
 		igb_stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
 		igb_stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
 		igb_stats->q_errors[i] = internal->tx_queue[i].err_pkts;
@@ -545,11 +543,11 @@ eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		internal->rx_queue[i].rx_pkts = 0;
 		internal->rx_queue[i].rx_bytes = 0;
 	}
-	for (i = 0; i < internal->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		internal->tx_queue[i].tx_pkts = 0;
 		internal->tx_queue[i].tx_bytes = 0;
 		internal->tx_queue[i].err_pkts = 0;
@@ -840,9 +838,6 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
-	(*internals)->nb_rx_queues = nb_rx_queues;
-	(*internals)->nb_tx_queues = nb_tx_queues;
-
 	if (pair == NULL)
 		(*internals)->if_index = 0;
 	else
@@ -860,11 +855,11 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
 
 	(*eth_dev)->data = data;
 	(*eth_dev)->dev_ops = &ops;
-	(*eth_dev)->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	(*eth_dev)->driver = NULL;
-	(*eth_dev)->data->kdrv = RTE_KDRV_NONE;
-	(*eth_dev)->data->drv_name = drivername;
-	(*eth_dev)->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	return 0;
 
@@ -876,16 +871,12 @@ error:
 }
 
 static int
-rte_eth_from_pcaps_n_dumpers(const char *name,
-		struct rx_pcaps *rx_queues,
-		const unsigned nb_rx_queues,
-		struct tx_pcaps *tx_queues,
-		const unsigned nb_tx_queues,
-		const unsigned numa_node,
-		struct rte_kvargs *kvlist)
+rte_eth_from_pcaps_common(const char *name, struct rx_pcaps *rx_queues,
+		const unsigned nb_rx_queues, struct tx_pcaps *tx_queues,
+		const unsigned nb_tx_queues, const unsigned numa_node,
+		struct rte_kvargs *kvlist, struct pmd_internals **internals,
+		struct rte_eth_dev **eth_dev)
 {
-	struct pmd_internals *internals = NULL;
-	struct rte_eth_dev *eth_dev = NULL;
 	unsigned i;
 
 	/* do some parameter checking */
@@ -895,28 +886,51 @@ rte_eth_from_pcaps_n_dumpers(const char *name,
 		return -1;
 
 	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
-			&internals, &eth_dev, kvlist) < 0)
+			internals, eth_dev, kvlist) < 0)
 		return -1;
 
 	for (i = 0; i < nb_rx_queues; i++) {
-		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
-		snprintf(internals->rx_queue[i].name,
-			sizeof(internals->rx_queue[i].name), "%s",
+		(*internals)->rx_queue[i].pcap = rx_queues->pcaps[i];
+		snprintf((*internals)->rx_queue[i].name,
+			sizeof((*internals)->rx_queue[i].name), "%s",
 			rx_queues->names[i]);
-		snprintf(internals->rx_queue[i].type,
-			sizeof(internals->rx_queue[i].type), "%s",
+		snprintf((*internals)->rx_queue[i].type,
+			sizeof((*internals)->rx_queue[i].type), "%s",
 			rx_queues->types[i]);
 	}
 	for (i = 0; i < nb_tx_queues; i++) {
-		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
-		snprintf(internals->tx_queue[i].name,
-			sizeof(internals->tx_queue[i].name), "%s",
+		(*internals)->tx_queue[i].dumper = tx_queues->dumpers[i];
+		snprintf((*internals)->tx_queue[i].name,
+			sizeof((*internals)->tx_queue[i].name), "%s",
 			tx_queues->names[i]);
-		snprintf(internals->tx_queue[i].type,
-			sizeof(internals->tx_queue[i].type), "%s",
+		snprintf((*internals)->tx_queue[i].type,
+			sizeof((*internals)->tx_queue[i].type), "%s",
 			tx_queues->types[i]);
 	}
 
+	return 0;
+}
+
+static int
+rte_eth_from_pcaps_n_dumpers(const char *name,
+		struct rx_pcaps *rx_queues,
+		const unsigned nb_rx_queues,
+		struct tx_pcaps *tx_queues,
+		const unsigned nb_tx_queues,
+		const unsigned numa_node,
+		struct rte_kvargs *kvlist)
+{
+	struct pmd_internals *internals = NULL;
+	struct rte_eth_dev *eth_dev = NULL;
+	int ret;
+
+	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+			tx_queues, nb_tx_queues, numa_node, kvlist,
+			&internals, &eth_dev);
+
+	if (ret < 0)
+		return ret;
+
 	/* using multiple pcaps/interfaces */
 	internals->single_iface = 0;
 
@@ -938,36 +952,14 @@ rte_eth_from_pcaps(const char *name,
 {
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
-	unsigned i;
-
-	/* do some parameter checking */
-	if (rx_queues == NULL && nb_rx_queues > 0)
-		return -1;
-	if (tx_queues == NULL && nb_tx_queues > 0)
-		return -1;
+	int ret;
 
-	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
-			&internals, &eth_dev, kvlist) < 0)
-		return -1;
+	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+			tx_queues, nb_tx_queues, numa_node, kvlist,
+			&internals, &eth_dev);
 
-	for (i = 0; i < nb_rx_queues; i++) {
-		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
-		snprintf(internals->rx_queue[i].name,
-			sizeof(internals->rx_queue[i].name), "%s",
-			rx_queues->names[i]);
-		snprintf(internals->rx_queue[i].type,
-			sizeof(internals->rx_queue[i].type), "%s",
-			rx_queues->types[i]);
-	}
-	for (i = 0; i < nb_tx_queues; i++) {
-		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
-		snprintf(internals->tx_queue[i].name,
-			sizeof(internals->tx_queue[i].name), "%s",
-			tx_queues->names[i]);
-		snprintf(internals->tx_queue[i].type,
-			sizeof(internals->tx_queue[i].type), "%s",
-			tx_queues->types[i]);
-	}
+	if (ret < 0)
+		return ret;
 
 	/* store wether we are using a single interface for rx/tx or not */
 	internals->single_iface = single_iface;
-- 
2.5.0

Re: [PATCH v2 1/3] pcap: remove duplicate fields in internal data struct

[Nicolas Pernas Maradei]

Hi,

Patch looks good to me although I haven't tested it. Adding Acked-by line.

Nico.

On 18/02/16 11:26, Ferruh Yigit wrote:
> 1- Remove duplicate nb_rx/tx_queues fields from internals
> 2- Move duplicate code into a common function
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Nicolas Pernas Maradei <nicolas.pernas.maradei@emutex.com>
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
>  1 file changed, 61 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index f9230eb..c8b7dbd 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -103,8 +103,6 @@ struct tx_pcaps {
>  struct pmd_internals {
>  	struct pcap_rx_queue rx_queue[RTE_PMD_RING_MAX_RX_RINGS];
>  	struct pcap_tx_queue tx_queue[RTE_PMD_RING_MAX_TX_RINGS];
> -	unsigned nb_rx_queues;
> -	unsigned nb_tx_queues;
>  	int if_index;
>  	int single_iface;
>  };
> @@ -396,7 +394,7 @@ eth_dev_start(struct rte_eth_dev *dev)
>  	}
>  
>  	/* If not open already, open tx pcaps/dumpers */
> -	for (i = 0; i < internals->nb_tx_queues; i++) {
> +	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>  		tx = &internals->tx_queue[i];
>  
>  		if (!tx->dumper && strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) {
> @@ -411,7 +409,7 @@ eth_dev_start(struct rte_eth_dev *dev)
>  	}
>  
>  	/* If not open already, open rx pcaps */
> -	for (i = 0; i < internals->nb_rx_queues; i++) {
> +	for (i = 0; i < dev->data->nb_rx_queues; i++) {
>  		rx = &internals->rx_queue[i];
>  
>  		if (rx->pcap != NULL)
> @@ -457,7 +455,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
>  		goto status_down;
>  	}
>  
> -	for (i = 0; i < internals->nb_tx_queues; i++) {
> +	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>  		tx = &internals->tx_queue[i];
>  
>  		if (tx->dumper != NULL) {
> @@ -471,7 +469,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
>  		}
>  	}
>  
> -	for (i = 0; i < internals->nb_rx_queues; i++) {
> +	for (i = 0; i < dev->data->nb_rx_queues; i++) {
>  		rx = &internals->rx_queue[i];
>  
>  		if (rx->pcap != NULL) {
> @@ -499,8 +497,8 @@ eth_dev_info(struct rte_eth_dev *dev,
>  	dev_info->if_index = internals->if_index;
>  	dev_info->max_mac_addrs = 1;
>  	dev_info->max_rx_pktlen = (uint32_t) -1;
> -	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
> -	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
> +	dev_info->max_rx_queues = dev->data->nb_rx_queues;
> +	dev_info->max_tx_queues = dev->data->nb_tx_queues;
>  	dev_info->min_rx_bufsize = 0;
>  	dev_info->pci_dev = NULL;
>  }
> @@ -515,16 +513,16 @@ eth_stats_get(struct rte_eth_dev *dev,
>  	unsigned long tx_packets_err_total = 0;
>  	const struct pmd_internals *internal = dev->data->dev_private;
>  
> -	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_rx_queues;
> -			i++) {
> +	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
> +			i < dev->data->nb_rx_queues; i++) {
>  		igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
>  		igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
>  		rx_packets_total += igb_stats->q_ipackets[i];
>  		rx_bytes_total += igb_stats->q_ibytes[i];
>  	}
>  
> -	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_tx_queues;
> -			i++) {
> +	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
> +			i < dev->data->nb_tx_queues; i++) {
>  		igb_stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
>  		igb_stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
>  		igb_stats->q_errors[i] = internal->tx_queue[i].err_pkts;
> @@ -545,11 +543,11 @@ eth_stats_reset(struct rte_eth_dev *dev)
>  {
>  	unsigned i;
>  	struct pmd_internals *internal = dev->data->dev_private;
> -	for (i = 0; i < internal->nb_rx_queues; i++) {
> +	for (i = 0; i < dev->data->nb_rx_queues; i++) {
>  		internal->rx_queue[i].rx_pkts = 0;
>  		internal->rx_queue[i].rx_bytes = 0;
>  	}
> -	for (i = 0; i < internal->nb_tx_queues; i++) {
> +	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>  		internal->tx_queue[i].tx_pkts = 0;
>  		internal->tx_queue[i].tx_bytes = 0;
>  		internal->tx_queue[i].err_pkts = 0;
> @@ -840,9 +838,6 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
>  	/* NOTE: we'll replace the data element, of originally allocated eth_dev
>  	 * so the rings are local per-process */
>  
> -	(*internals)->nb_rx_queues = nb_rx_queues;
> -	(*internals)->nb_tx_queues = nb_tx_queues;
> -
>  	if (pair == NULL)
>  		(*internals)->if_index = 0;
>  	else
> @@ -860,11 +855,11 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
>  
>  	(*eth_dev)->data = data;
>  	(*eth_dev)->dev_ops = &ops;
> -	(*eth_dev)->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
>  	(*eth_dev)->driver = NULL;
> -	(*eth_dev)->data->kdrv = RTE_KDRV_NONE;
> -	(*eth_dev)->data->drv_name = drivername;
> -	(*eth_dev)->data->numa_node = numa_node;
> +	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
> +	data->kdrv = RTE_KDRV_NONE;
> +	data->drv_name = drivername;
> +	data->numa_node = numa_node;
>  
>  	return 0;
>  
> @@ -876,16 +871,12 @@ error:
>  }
>  
>  static int
> -rte_eth_from_pcaps_n_dumpers(const char *name,
> -		struct rx_pcaps *rx_queues,
> -		const unsigned nb_rx_queues,
> -		struct tx_pcaps *tx_queues,
> -		const unsigned nb_tx_queues,
> -		const unsigned numa_node,
> -		struct rte_kvargs *kvlist)
> +rte_eth_from_pcaps_common(const char *name, struct rx_pcaps *rx_queues,
> +		const unsigned nb_rx_queues, struct tx_pcaps *tx_queues,
> +		const unsigned nb_tx_queues, const unsigned numa_node,
> +		struct rte_kvargs *kvlist, struct pmd_internals **internals,
> +		struct rte_eth_dev **eth_dev)
>  {
> -	struct pmd_internals *internals = NULL;
> -	struct rte_eth_dev *eth_dev = NULL;
>  	unsigned i;
>  
>  	/* do some parameter checking */
> @@ -895,28 +886,51 @@ rte_eth_from_pcaps_n_dumpers(const char *name,
>  		return -1;
>  
>  	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
> -			&internals, &eth_dev, kvlist) < 0)
> +			internals, eth_dev, kvlist) < 0)
>  		return -1;
>  
>  	for (i = 0; i < nb_rx_queues; i++) {
> -		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
> -		snprintf(internals->rx_queue[i].name,
> -			sizeof(internals->rx_queue[i].name), "%s",
> +		(*internals)->rx_queue[i].pcap = rx_queues->pcaps[i];
> +		snprintf((*internals)->rx_queue[i].name,
> +			sizeof((*internals)->rx_queue[i].name), "%s",
>  			rx_queues->names[i]);
> -		snprintf(internals->rx_queue[i].type,
> -			sizeof(internals->rx_queue[i].type), "%s",
> +		snprintf((*internals)->rx_queue[i].type,
> +			sizeof((*internals)->rx_queue[i].type), "%s",
>  			rx_queues->types[i]);
>  	}
>  	for (i = 0; i < nb_tx_queues; i++) {
> -		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
> -		snprintf(internals->tx_queue[i].name,
> -			sizeof(internals->tx_queue[i].name), "%s",
> +		(*internals)->tx_queue[i].dumper = tx_queues->dumpers[i];
> +		snprintf((*internals)->tx_queue[i].name,
> +			sizeof((*internals)->tx_queue[i].name), "%s",
>  			tx_queues->names[i]);
> -		snprintf(internals->tx_queue[i].type,
> -			sizeof(internals->tx_queue[i].type), "%s",
> +		snprintf((*internals)->tx_queue[i].type,
> +			sizeof((*internals)->tx_queue[i].type), "%s",
>  			tx_queues->types[i]);
>  	}
>  
> +	return 0;
> +}
> +
> +static int
> +rte_eth_from_pcaps_n_dumpers(const char *name,
> +		struct rx_pcaps *rx_queues,
> +		const unsigned nb_rx_queues,
> +		struct tx_pcaps *tx_queues,
> +		const unsigned nb_tx_queues,
> +		const unsigned numa_node,
> +		struct rte_kvargs *kvlist)
> +{
> +	struct pmd_internals *internals = NULL;
> +	struct rte_eth_dev *eth_dev = NULL;
> +	int ret;
> +
> +	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
> +			tx_queues, nb_tx_queues, numa_node, kvlist,
> +			&internals, &eth_dev);
> +
> +	if (ret < 0)
> +		return ret;
> +
>  	/* using multiple pcaps/interfaces */
>  	internals->single_iface = 0;
>  
> @@ -938,36 +952,14 @@ rte_eth_from_pcaps(const char *name,
>  {
>  	struct pmd_internals *internals = NULL;
>  	struct rte_eth_dev *eth_dev = NULL;
> -	unsigned i;
> -
> -	/* do some parameter checking */
> -	if (rx_queues == NULL && nb_rx_queues > 0)
> -		return -1;
> -	if (tx_queues == NULL && nb_tx_queues > 0)
> -		return -1;
> +	int ret;
>  
> -	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
> -			&internals, &eth_dev, kvlist) < 0)
> -		return -1;
> +	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
> +			tx_queues, nb_tx_queues, numa_node, kvlist,
> +			&internals, &eth_dev);
>  
> -	for (i = 0; i < nb_rx_queues; i++) {
> -		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
> -		snprintf(internals->rx_queue[i].name,
> -			sizeof(internals->rx_queue[i].name), "%s",
> -			rx_queues->names[i]);
> -		snprintf(internals->rx_queue[i].type,
> -			sizeof(internals->rx_queue[i].type), "%s",
> -			rx_queues->types[i]);
> -	}
> -	for (i = 0; i < nb_tx_queues; i++) {
> -		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
> -		snprintf(internals->tx_queue[i].name,
> -			sizeof(internals->tx_queue[i].name), "%s",
> -			tx_queues->names[i]);
> -		snprintf(internals->tx_queue[i].type,
> -			sizeof(internals->tx_queue[i].type), "%s",
> -			tx_queues->types[i]);
> -	}
> +	if (ret < 0)
> +		return ret;
>  
>  	/* store wether we are using a single interface for rx/tx or not */
>  	internals->single_iface = single_iface;

[PATCH v2 2/3] ring: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- Remove duplicate nb_rx/tx_queues fields from internals

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/ring/rte_eth_ring.c | 57 ++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index d92b088..fd87999 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -59,9 +59,6 @@ struct ring_queue {
 };
 
 struct pmd_internals {
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
-
 	struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
 	struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
 
@@ -138,7 +135,7 @@ eth_dev_set_link_up(struct rte_eth_dev *dev)
 }
 
 static int
-eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
+eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 				    uint16_t nb_rx_desc __rte_unused,
 				    unsigned int socket_id __rte_unused,
 				    const struct rte_eth_rxconf *rx_conf __rte_unused,
@@ -165,40 +162,39 @@ static void
 eth_dev_info(struct rte_eth_dev *dev,
 		struct rte_eth_dev_info *dev_info)
 {
-	struct pmd_internals *internals = dev->data->dev_private;
 	dev_info->driver_name = drivername;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_pktlen = (uint32_t)-1;
-	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
+	dev_info->max_rx_queues = dev->data->nb_rx_queues;
+	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
 }
 
 static void
-eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
+eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned i;
 	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
-			i < internal->nb_rx_queues; i++) {
-		igb_stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
-		rx_total += igb_stats->q_ipackets[i];
+			i < dev->data->nb_rx_queues; i++) {
+		stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
+		rx_total += stats->q_ipackets[i];
 	}
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
-			i < internal->nb_tx_queues; i++) {
-		igb_stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
-		igb_stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
-		tx_total += igb_stats->q_opackets[i];
-		tx_err_total += igb_stats->q_errors[i];
+			i < dev->data->nb_tx_queues; i++) {
+		stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
+		stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
+		tx_total += stats->q_opackets[i];
+		tx_err_total += stats->q_errors[i];
 	}
 
-	igb_stats->ipackets = rx_total;
-	igb_stats->opackets = tx_total;
-	igb_stats->oerrors = tx_err_total;
+	stats->ipackets = rx_total;
+	stats->opackets = tx_total;
+	stats->oerrors = tx_err_total;
 }
 
 static void
@@ -206,9 +202,9 @@ eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++)
+	for (i = 0; i < dev->data->nb_rx_queues; i++)
 		internal->rx_ring_queues[i].rx_pkts.cnt = 0;
-	for (i = 0; i < internal->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		internal->tx_ring_queues[i].tx_pkts.cnt = 0;
 		internal->tx_ring_queues[i].err_pkts.cnt = 0;
 	}
@@ -262,7 +258,6 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	struct rte_eth_dev_data *data = NULL;
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
-
 	unsigned i;
 
 	/* do some parameter checking */
@@ -291,15 +286,15 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 		goto error;
 	}
 
-	data->rx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_rx_queues,
-			0, numa_node);
+	data->rx_queues = rte_zmalloc_socket(name,
+			sizeof(void *) * nb_rx_queues, 0, numa_node);
 	if (data->rx_queues == NULL) {
 		rte_errno = ENOMEM;
 		goto error;
 	}
 
-	data->tx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_tx_queues,
-			0, numa_node);
+	data->tx_queues = rte_zmalloc_socket(name,
+			sizeof(void *) * nb_tx_queues, 0, numa_node);
 	if (data->tx_queues == NULL) {
 		rte_errno = ENOMEM;
 		goto error;
@@ -327,8 +322,6 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
-	internals->nb_rx_queues = nb_rx_queues;
-	internals->nb_tx_queues = nb_tx_queues;
 	for (i = 0; i < nb_rx_queues; i++) {
 		internals->rx_ring_queues[i].rng = rx_queues[i];
 		data->rx_queues[i] = &internals->rx_ring_queues[i];
@@ -349,10 +342,10 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	eth_dev->data = data;
 	eth_dev->driver = NULL;
 	eth_dev->dev_ops = &ops;
-	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-	eth_dev->data->kdrv = RTE_KDRV_NONE;
-	eth_dev->data->drv_name = drivername;
-	eth_dev->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	TAILQ_INIT(&(eth_dev->link_intr_cbs));
 
-- 
2.5.0

Re: [PATCH v2 2/3] ring: remove duplicate fields in internal data struct

[Nicolas Pernas Maradei]

Adding Acked-by line.

On 18/02/16 11:26, Ferruh Yigit wrote:
> 1- Remove duplicate nb_rx/tx_queues fields from internals
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Nicolas Pernas Maradei <nicolas.pernas.maradei@emutex.com>
> ---
>  drivers/net/ring/rte_eth_ring.c | 57 ++++++++++++++++++-----------------------
>  1 file changed, 25 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
> index d92b088..fd87999 100644
> --- a/drivers/net/ring/rte_eth_ring.c
> +++ b/drivers/net/ring/rte_eth_ring.c
> @@ -59,9 +59,6 @@ struct ring_queue {
>  };
>  
>  struct pmd_internals {
> -	unsigned nb_rx_queues;
> -	unsigned nb_tx_queues;
> -
>  	struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
>  	struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
>  
> @@ -138,7 +135,7 @@ eth_dev_set_link_up(struct rte_eth_dev *dev)
>  }
>  
>  static int
> -eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
> +eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
>  				    uint16_t nb_rx_desc __rte_unused,
>  				    unsigned int socket_id __rte_unused,
>  				    const struct rte_eth_rxconf *rx_conf __rte_unused,
> @@ -165,40 +162,39 @@ static void
>  eth_dev_info(struct rte_eth_dev *dev,
>  		struct rte_eth_dev_info *dev_info)
>  {
> -	struct pmd_internals *internals = dev->data->dev_private;
>  	dev_info->driver_name = drivername;
>  	dev_info->max_mac_addrs = 1;
>  	dev_info->max_rx_pktlen = (uint32_t)-1;
> -	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
> -	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
> +	dev_info->max_rx_queues = dev->data->nb_rx_queues;
> +	dev_info->max_tx_queues = dev->data->nb_tx_queues;
>  	dev_info->min_rx_bufsize = 0;
>  	dev_info->pci_dev = NULL;
>  }
>  
>  static void
> -eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
> +eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  {
>  	unsigned i;
>  	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
>  	const struct pmd_internals *internal = dev->data->dev_private;
>  
>  	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
> -			i < internal->nb_rx_queues; i++) {
> -		igb_stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
> -		rx_total += igb_stats->q_ipackets[i];
> +			i < dev->data->nb_rx_queues; i++) {
> +		stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
> +		rx_total += stats->q_ipackets[i];
>  	}
>  
>  	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
> -			i < internal->nb_tx_queues; i++) {
> -		igb_stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
> -		igb_stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
> -		tx_total += igb_stats->q_opackets[i];
> -		tx_err_total += igb_stats->q_errors[i];
> +			i < dev->data->nb_tx_queues; i++) {
> +		stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
> +		stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
> +		tx_total += stats->q_opackets[i];
> +		tx_err_total += stats->q_errors[i];
>  	}
>  
> -	igb_stats->ipackets = rx_total;
> -	igb_stats->opackets = tx_total;
> -	igb_stats->oerrors = tx_err_total;
> +	stats->ipackets = rx_total;
> +	stats->opackets = tx_total;
> +	stats->oerrors = tx_err_total;
>  }
>  
>  static void
> @@ -206,9 +202,9 @@ eth_stats_reset(struct rte_eth_dev *dev)
>  {
>  	unsigned i;
>  	struct pmd_internals *internal = dev->data->dev_private;
> -	for (i = 0; i < internal->nb_rx_queues; i++)
> +	for (i = 0; i < dev->data->nb_rx_queues; i++)
>  		internal->rx_ring_queues[i].rx_pkts.cnt = 0;
> -	for (i = 0; i < internal->nb_tx_queues; i++) {
> +	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>  		internal->tx_ring_queues[i].tx_pkts.cnt = 0;
>  		internal->tx_ring_queues[i].err_pkts.cnt = 0;
>  	}
> @@ -262,7 +258,6 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
>  	struct rte_eth_dev_data *data = NULL;
>  	struct pmd_internals *internals = NULL;
>  	struct rte_eth_dev *eth_dev = NULL;
> -
>  	unsigned i;
>  
>  	/* do some parameter checking */
> @@ -291,15 +286,15 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
>  		goto error;
>  	}
>  
> -	data->rx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_rx_queues,
> -			0, numa_node);
> +	data->rx_queues = rte_zmalloc_socket(name,
> +			sizeof(void *) * nb_rx_queues, 0, numa_node);
>  	if (data->rx_queues == NULL) {
>  		rte_errno = ENOMEM;
>  		goto error;
>  	}
>  
> -	data->tx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_tx_queues,
> -			0, numa_node);
> +	data->tx_queues = rte_zmalloc_socket(name,
> +			sizeof(void *) * nb_tx_queues, 0, numa_node);
>  	if (data->tx_queues == NULL) {
>  		rte_errno = ENOMEM;
>  		goto error;
> @@ -327,8 +322,6 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
>  	/* NOTE: we'll replace the data element, of originally allocated eth_dev
>  	 * so the rings are local per-process */
>  
> -	internals->nb_rx_queues = nb_rx_queues;
> -	internals->nb_tx_queues = nb_tx_queues;
>  	for (i = 0; i < nb_rx_queues; i++) {
>  		internals->rx_ring_queues[i].rng = rx_queues[i];
>  		data->rx_queues[i] = &internals->rx_ring_queues[i];
> @@ -349,10 +342,10 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
>  	eth_dev->data = data;
>  	eth_dev->driver = NULL;
>  	eth_dev->dev_ops = &ops;
> -	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
> -	eth_dev->data->kdrv = RTE_KDRV_NONE;
> -	eth_dev->data->drv_name = drivername;
> -	eth_dev->data->numa_node = numa_node;
> +	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
> +	data->kdrv = RTE_KDRV_NONE;
> +	data->drv_name = drivername;
> +	data->numa_node = numa_node;
>  
>  	TAILQ_INIT(&(eth_dev->link_intr_cbs));
>  

Re: [PATCH v2 2/3] ring: remove duplicate fields in internal data struct

[Bruce Richardson]

On Thu, Feb 18, 2016 at 11:26:42AM +0000, Ferruh Yigit wrote:
> 1- Remove duplicate nb_rx/tx_queues fields from internals
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  drivers/net/ring/rte_eth_ring.c | 57 ++++++++++++++++++-----------------------
>  1 file changed, 25 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
> index d92b088..fd87999 100644
> --- a/drivers/net/ring/rte_eth_ring.c
> +++ b/drivers/net/ring/rte_eth_ring.c
> @@ -59,9 +59,6 @@ struct ring_queue {
>  };
>  
>  struct pmd_internals {
> -	unsigned nb_rx_queues;
> -	unsigned nb_tx_queues;
> -
>  	struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
>  	struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
>  
> @@ -138,7 +135,7 @@ eth_dev_set_link_up(struct rte_eth_dev *dev)
>  }
>  
>  static int
> -eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
> +eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
>  				    uint16_t nb_rx_desc __rte_unused,
>  				    unsigned int socket_id __rte_unused,
>  				    const struct rte_eth_rxconf *rx_conf __rte_unused,
> @@ -165,40 +162,39 @@ static void
>  eth_dev_info(struct rte_eth_dev *dev,
>  		struct rte_eth_dev_info *dev_info)
>  {
> -	struct pmd_internals *internals = dev->data->dev_private;
>  	dev_info->driver_name = drivername;
>  	dev_info->max_mac_addrs = 1;
>  	dev_info->max_rx_pktlen = (uint32_t)-1;
> -	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
> -	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
> +	dev_info->max_rx_queues = dev->data->nb_rx_queues;
> +	dev_info->max_tx_queues = dev->data->nb_tx_queues;

I'm still not convined this is correct. What happens if a ring PMD is created
with 16 queues (i.e. backed by 16 rings), and then the user uses
rte_eth_dev_configure to only actually use 4 queues. The fact that the internal
array still has 16 queues will be lost, and the device will only ever report
4 as the max number it can support.

Regards,
/Bruce

Re: [PATCH v2 2/3] ring: remove duplicate fields in internal data struct

[Ferruh Yigit]

On 2/23/2016 3:26 PM, Bruce Richardson wrote:
> On Thu, Feb 18, 2016 at 11:26:42AM +0000, Ferruh Yigit wrote:
>> 1- Remove duplicate nb_rx/tx_queues fields from internals
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>  drivers/net/ring/rte_eth_ring.c | 57 ++++++++++++++++++-----------------------
>>  1 file changed, 25 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
>> index d92b088..fd87999 100644
>> --- a/drivers/net/ring/rte_eth_ring.c
>> +++ b/drivers/net/ring/rte_eth_ring.c
>> @@ -59,9 +59,6 @@ struct ring_queue {
>>  };
>>  
>>  struct pmd_internals {
>> -	unsigned nb_rx_queues;
>> -	unsigned nb_tx_queues;
>> -
>>  	struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
>>  	struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
>>  
>> @@ -138,7 +135,7 @@ eth_dev_set_link_up(struct rte_eth_dev *dev)
>>  }
>>  
>>  static int
>> -eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
>> +eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
>>  				    uint16_t nb_rx_desc __rte_unused,
>>  				    unsigned int socket_id __rte_unused,
>>  				    const struct rte_eth_rxconf *rx_conf __rte_unused,
>> @@ -165,40 +162,39 @@ static void
>>  eth_dev_info(struct rte_eth_dev *dev,
>>  		struct rte_eth_dev_info *dev_info)
>>  {
>> -	struct pmd_internals *internals = dev->data->dev_private;
>>  	dev_info->driver_name = drivername;
>>  	dev_info->max_mac_addrs = 1;
>>  	dev_info->max_rx_pktlen = (uint32_t)-1;
>> -	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
>> -	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
>> +	dev_info->max_rx_queues = dev->data->nb_rx_queues;
>> +	dev_info->max_tx_queues = dev->data->nb_tx_queues;
> 
> I'm still not convined this is correct. What happens if a ring PMD is created
> with 16 queues (i.e. backed by 16 rings), and then the user uses
> rte_eth_dev_configure to only actually use 4 queues.

Right, since user explicitly set 4 queues.

> The fact that the internal
> array still has 16 queues will be lost, 

Not lost exactly, app can re-configure with rte_eth_dev_configure() to
use 16 queses back and it will work fine.

> and the device will only ever report
> 4 as the max number it can support.

I think this is same for all PMDs, and data->nb_xx_queues reports the
number of the configured queues, not max number; and indeed for ring PMD
max queue number is hardcoded in the config file.

I guess you what you refer is "number of queues used in first
configuration", is there a use case to save this value? And if there is
does it make sense to application save it instead of PMD, because for
your sample case application creates ring with rte_eth_from_ring() API,
so app already knows the initial configured queue number.

Thanks,
ferruh

Re: [PATCH v2 2/3] ring: remove duplicate fields in internal data struct

[Bruce Richardson]

On Tue, Feb 23, 2016 at 03:58:50PM +0000, Ferruh Yigit wrote:
> On 2/23/2016 3:26 PM, Bruce Richardson wrote:
> > On Thu, Feb 18, 2016 at 11:26:42AM +0000, Ferruh Yigit wrote:
> >> 1- Remove duplicate nb_rx/tx_queues fields from internals
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >>  drivers/net/ring/rte_eth_ring.c | 57 ++++++++++++++++++-----------------------
> >>  1 file changed, 25 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
> >> index d92b088..fd87999 100644
> >> --- a/drivers/net/ring/rte_eth_ring.c
> >> +++ b/drivers/net/ring/rte_eth_ring.c
> >> @@ -59,9 +59,6 @@ struct ring_queue {
> >>  };
> >>  
> >>  struct pmd_internals {
> >> -	unsigned nb_rx_queues;
> >> -	unsigned nb_tx_queues;
> >> -
> >>  	struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
> >>  	struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
> >>  
> >> @@ -138,7 +135,7 @@ eth_dev_set_link_up(struct rte_eth_dev *dev)
> >>  }
> >>  
> >>  static int
> >> -eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
> >> +eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
> >>  				    uint16_t nb_rx_desc __rte_unused,
> >>  				    unsigned int socket_id __rte_unused,
> >>  				    const struct rte_eth_rxconf *rx_conf __rte_unused,
> >> @@ -165,40 +162,39 @@ static void
> >>  eth_dev_info(struct rte_eth_dev *dev,
> >>  		struct rte_eth_dev_info *dev_info)
> >>  {
> >> -	struct pmd_internals *internals = dev->data->dev_private;
> >>  	dev_info->driver_name = drivername;
> >>  	dev_info->max_mac_addrs = 1;
> >>  	dev_info->max_rx_pktlen = (uint32_t)-1;
> >> -	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
> >> -	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
> >> +	dev_info->max_rx_queues = dev->data->nb_rx_queues;
> >> +	dev_info->max_tx_queues = dev->data->nb_tx_queues;
> > 
> > I'm still not convined this is correct. What happens if a ring PMD is created
> > with 16 queues (i.e. backed by 16 rings), and then the user uses
> > rte_eth_dev_configure to only actually use 4 queues.
> 
> Right, since user explicitly set 4 queues.
> 
> > The fact that the internal
> > array still has 16 queues will be lost, 
> 
> Not lost exactly, app can re-configure with rte_eth_dev_configure() to
> use 16 queses back and it will work fine.
> 
No, it can't do that, because the vNIC is reporting that the max number of queues
supported is now 4, since you now set max_rx_queues = nb_rx_queues.

> > and the device will only ever report
> > 4 as the max number it can support.
> 
> I think this is same for all PMDs, and data->nb_xx_queues reports the
> number of the configured queues, not max number; and indeed for ring PMD
> max queue number is hardcoded in the config file.
Yes, it is consistent with other PMDs as it is. The variables you think are
duplicate and want to remove are the ones that track the max number of queues
to be reported out, so that the app can know how many it can use in a call
to reconfigure.

/Bruce
> 
> I guess you what you refer is "number of queues used in first
> configuration", is there a use case to save this value? And if there is
> does it make sense to application save it instead of PMD, because for
> your sample case application creates ring with rte_eth_from_ring() API,
> so app already knows the initial configured queue number.
> 
> Thanks,
> ferruh

[PATCH v2 3/3] null: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- remove duplicate nb_rx/tx_queues fields from internals
2- remove duplicate numa_node field from internals

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Acked-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 drivers/net/null/rte_eth_null.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 77fc988..1c354ad 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -69,10 +69,6 @@ struct null_queue {
 struct pmd_internals {
 	unsigned packet_size;
 	unsigned packet_copy;
-	unsigned numa_node;
-
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
 
 	struct null_queue rx_null_queues[RTE_MAX_QUEUES_PER_PORT];
 	struct null_queue tx_null_queues[RTE_MAX_QUEUES_PER_PORT];
@@ -192,13 +188,8 @@ eth_null_copy_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 }
 
 static int
-eth_dev_configure(struct rte_eth_dev *dev) {
-	struct pmd_internals *internals;
-
-	internals = dev->data->dev_private;
-	internals->nb_rx_queues = dev->data->nb_rx_queues;
-	internals->nb_tx_queues = dev->data->nb_tx_queues;
-
+eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
+{
 	return 0;
 }
 
@@ -237,7 +228,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 
 	internals = dev->data->dev_private;
 
-	if (rx_queue_id >= internals->nb_rx_queues)
+	if (rx_queue_id >= dev->data->nb_rx_queues)
 		return -ENODEV;
 
 	packet_size = internals->packet_size;
@@ -246,7 +237,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 	dev->data->rx_queues[rx_queue_id] =
 		&internals->rx_null_queues[rx_queue_id];
 	dummy_packet = rte_zmalloc_socket(NULL,
-			packet_size, 0, internals->numa_node);
+			packet_size, 0, dev->data->numa_node);
 	if (dummy_packet == NULL)
 		return -ENOMEM;
 
@@ -271,7 +262,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 
 	internals = dev->data->dev_private;
 
-	if (tx_queue_id >= internals->nb_tx_queues)
+	if (tx_queue_id >= dev->data->nb_tx_queues)
 		return -ENODEV;
 
 	packet_size = internals->packet_size;
@@ -279,7 +270,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 	dev->data->tx_queues[tx_queue_id] =
 		&internals->tx_null_queues[tx_queue_id];
 	dummy_packet = rte_zmalloc_socket(NULL,
-			packet_size, 0, internals->numa_node);
+			packet_size, 0, dev->data->numa_node);
 	if (dummy_packet == NULL)
 		return -ENOMEM;
 
@@ -323,7 +314,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 
 	internal = dev->data->dev_private;
 	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
-			RTE_MIN(internal->nb_rx_queues,
+			RTE_MIN(dev->data->nb_rx_queues,
 				RTE_DIM(internal->rx_null_queues)));
 	for (i = 0; i < num_stats; i++) {
 		igb_stats->q_ipackets[i] =
@@ -332,7 +323,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 	}
 
 	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
-			RTE_MIN(internal->nb_tx_queues,
+			RTE_MIN(dev->data->nb_tx_queues,
 				RTE_DIM(internal->tx_null_queues)));
 	for (i = 0; i < num_stats; i++) {
 		igb_stats->q_opackets[i] =
@@ -535,11 +526,8 @@ eth_dev_null_create(const char *name,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the nulls are local per-process */
 
-	internals->nb_rx_queues = nb_rx_queues;
-	internals->nb_tx_queues = nb_tx_queues;
 	internals->packet_size = packet_size;
 	internals->packet_copy = packet_copy;
-	internals->numa_node = numa_node;
 
 	internals->flow_type_rss_offloads =  ETH_RSS_PROTO_MASK;
 	internals->reta_size = RTE_DIM(internals->reta_conf) * RTE_RETA_GROUP_SIZE;
@@ -560,10 +548,10 @@ eth_dev_null_create(const char *name,
 	TAILQ_INIT(&eth_dev->link_intr_cbs);
 
 	eth_dev->driver = NULL;
-	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-	eth_dev->data->kdrv = RTE_KDRV_NONE;
-	eth_dev->data->drv_name = drivername;
-	eth_dev->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	/* finally assign rx and tx ops */
 	if (packet_copy) {
-- 
2.5.0

Re: [PATCH v2 3/3] null: remove duplicate fields in internal data struct

[Nicolas Pernas Maradei]

Adding Acked-by line

On 18/02/16 11:26, Ferruh Yigit wrote:
> 1- remove duplicate nb_rx/tx_queues fields from internals
> 2- remove duplicate numa_node field from internals
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Tested-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> Acked-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> Acked-by: Nicolas Pernas Maradei <nicolas.pernas.maradei@emutex.com>
> ---
>  drivers/net/null/rte_eth_null.c | 36 ++++++++++++------------------------
>  1 file changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> index 77fc988..1c354ad 100644
> --- a/drivers/net/null/rte_eth_null.c
> +++ b/drivers/net/null/rte_eth_null.c
> @@ -69,10 +69,6 @@ struct null_queue {
>  struct pmd_internals {
>  	unsigned packet_size;
>  	unsigned packet_copy;
> -	unsigned numa_node;
> -
> -	unsigned nb_rx_queues;
> -	unsigned nb_tx_queues;
>  
>  	struct null_queue rx_null_queues[RTE_MAX_QUEUES_PER_PORT];
>  	struct null_queue tx_null_queues[RTE_MAX_QUEUES_PER_PORT];
> @@ -192,13 +188,8 @@ eth_null_copy_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>  }
>  
>  static int
> -eth_dev_configure(struct rte_eth_dev *dev) {
> -	struct pmd_internals *internals;
> -
> -	internals = dev->data->dev_private;
> -	internals->nb_rx_queues = dev->data->nb_rx_queues;
> -	internals->nb_tx_queues = dev->data->nb_tx_queues;
> -
> +eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
> +{
>  	return 0;
>  }
>  
> @@ -237,7 +228,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
>  
>  	internals = dev->data->dev_private;
>  
> -	if (rx_queue_id >= internals->nb_rx_queues)
> +	if (rx_queue_id >= dev->data->nb_rx_queues)
>  		return -ENODEV;
>  
>  	packet_size = internals->packet_size;
> @@ -246,7 +237,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
>  	dev->data->rx_queues[rx_queue_id] =
>  		&internals->rx_null_queues[rx_queue_id];
>  	dummy_packet = rte_zmalloc_socket(NULL,
> -			packet_size, 0, internals->numa_node);
> +			packet_size, 0, dev->data->numa_node);
>  	if (dummy_packet == NULL)
>  		return -ENOMEM;
>  
> @@ -271,7 +262,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
>  
>  	internals = dev->data->dev_private;
>  
> -	if (tx_queue_id >= internals->nb_tx_queues)
> +	if (tx_queue_id >= dev->data->nb_tx_queues)
>  		return -ENODEV;
>  
>  	packet_size = internals->packet_size;
> @@ -279,7 +270,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
>  	dev->data->tx_queues[tx_queue_id] =
>  		&internals->tx_null_queues[tx_queue_id];
>  	dummy_packet = rte_zmalloc_socket(NULL,
> -			packet_size, 0, internals->numa_node);
> +			packet_size, 0, dev->data->numa_node);
>  	if (dummy_packet == NULL)
>  		return -ENOMEM;
>  
> @@ -323,7 +314,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
>  
>  	internal = dev->data->dev_private;
>  	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
> -			RTE_MIN(internal->nb_rx_queues,
> +			RTE_MIN(dev->data->nb_rx_queues,
>  				RTE_DIM(internal->rx_null_queues)));
>  	for (i = 0; i < num_stats; i++) {
>  		igb_stats->q_ipackets[i] =
> @@ -332,7 +323,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
>  	}
>  
>  	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
> -			RTE_MIN(internal->nb_tx_queues,
> +			RTE_MIN(dev->data->nb_tx_queues,
>  				RTE_DIM(internal->tx_null_queues)));
>  	for (i = 0; i < num_stats; i++) {
>  		igb_stats->q_opackets[i] =
> @@ -535,11 +526,8 @@ eth_dev_null_create(const char *name,
>  	/* NOTE: we'll replace the data element, of originally allocated eth_dev
>  	 * so the nulls are local per-process */
>  
> -	internals->nb_rx_queues = nb_rx_queues;
> -	internals->nb_tx_queues = nb_tx_queues;
>  	internals->packet_size = packet_size;
>  	internals->packet_copy = packet_copy;
> -	internals->numa_node = numa_node;
>  
>  	internals->flow_type_rss_offloads =  ETH_RSS_PROTO_MASK;
>  	internals->reta_size = RTE_DIM(internals->reta_conf) * RTE_RETA_GROUP_SIZE;
> @@ -560,10 +548,10 @@ eth_dev_null_create(const char *name,
>  	TAILQ_INIT(&eth_dev->link_intr_cbs);
>  
>  	eth_dev->driver = NULL;
> -	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
> -	eth_dev->data->kdrv = RTE_KDRV_NONE;
> -	eth_dev->data->drv_name = drivername;
> -	eth_dev->data->numa_node = numa_node;
> +	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
> +	data->kdrv = RTE_KDRV_NONE;
> +	data->drv_name = drivername;
> +	data->numa_node = numa_node;
>  
>  	/* finally assign rx and tx ops */
>  	if (packet_copy) {

[PATCH v3 0/3] clean-up on virtual PMDs

[Ferruh Yigit]

This is a clean-up patch, no defect fixed, no functional difference 
expected.

Patch removes duplicated fields between data->dev_private
and data (struct rte_eth_dev_data) for pcap and null PMDs.
For ring, renames some fields in private structure.

Also for pcap: move a common code into a function

v3:
* ring: Add fields in internal data struct back, rename them
these fields required to keep max configured queue number

v2:
* ring: Add memory allocation and queue assignment back, 
these are to make rte_eth_from_ring() work without 
rte_eth_dev_configure() or rte_eth_rx/tx_queue_setup() calls


Ferruh Yigit (3):
  pcap: remove duplicate fields in internal data struct
  ring: rename fields in internal data struct
  null: remove duplicate fields in internal data struct

 drivers/net/null/rte_eth_null.c |  36 ++++-------
 drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
 drivers/net/ring/rte_eth_ring.c |  59 +++++++++---------
 3 files changed, 102 insertions(+), 123 deletions(-)

-- 
2.5.0

[PATCH v3 1/3] pcap: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- Remove duplicate nb_rx/tx_queues fields from internals
2- Move duplicate code into a common function

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Nicolas Pernas Maradei <nicolas.pernas.maradei@emutex.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
 1 file changed, 61 insertions(+), 69 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index f9230eb..c8b7dbd 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -103,8 +103,6 @@ struct tx_pcaps {
 struct pmd_internals {
 	struct pcap_rx_queue rx_queue[RTE_PMD_RING_MAX_RX_RINGS];
 	struct pcap_tx_queue tx_queue[RTE_PMD_RING_MAX_TX_RINGS];
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
 	int if_index;
 	int single_iface;
 };
@@ -396,7 +394,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* If not open already, open tx pcaps/dumpers */
-	for (i = 0; i < internals->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		tx = &internals->tx_queue[i];
 
 		if (!tx->dumper && strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) {
@@ -411,7 +409,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* If not open already, open rx pcaps */
-	for (i = 0; i < internals->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		rx = &internals->rx_queue[i];
 
 		if (rx->pcap != NULL)
@@ -457,7 +455,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 		goto status_down;
 	}
 
-	for (i = 0; i < internals->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		tx = &internals->tx_queue[i];
 
 		if (tx->dumper != NULL) {
@@ -471,7 +469,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 		}
 	}
 
-	for (i = 0; i < internals->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		rx = &internals->rx_queue[i];
 
 		if (rx->pcap != NULL) {
@@ -499,8 +497,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->if_index = internals->if_index;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_pktlen = (uint32_t) -1;
-	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
+	dev_info->max_rx_queues = dev->data->nb_rx_queues;
+	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
 }
@@ -515,16 +513,16 @@ eth_stats_get(struct rte_eth_dev *dev,
 	unsigned long tx_packets_err_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
-	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_rx_queues;
-			i++) {
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < dev->data->nb_rx_queues; i++) {
 		igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
 		igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
 		rx_packets_total += igb_stats->q_ipackets[i];
 		rx_bytes_total += igb_stats->q_ibytes[i];
 	}
 
-	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_tx_queues;
-			i++) {
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < dev->data->nb_tx_queues; i++) {
 		igb_stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
 		igb_stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
 		igb_stats->q_errors[i] = internal->tx_queue[i].err_pkts;
@@ -545,11 +543,11 @@ eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		internal->rx_queue[i].rx_pkts = 0;
 		internal->rx_queue[i].rx_bytes = 0;
 	}
-	for (i = 0; i < internal->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		internal->tx_queue[i].tx_pkts = 0;
 		internal->tx_queue[i].tx_bytes = 0;
 		internal->tx_queue[i].err_pkts = 0;
@@ -840,9 +838,6 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
-	(*internals)->nb_rx_queues = nb_rx_queues;
-	(*internals)->nb_tx_queues = nb_tx_queues;
-
 	if (pair == NULL)
 		(*internals)->if_index = 0;
 	else
@@ -860,11 +855,11 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
 
 	(*eth_dev)->data = data;
 	(*eth_dev)->dev_ops = &ops;
-	(*eth_dev)->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	(*eth_dev)->driver = NULL;
-	(*eth_dev)->data->kdrv = RTE_KDRV_NONE;
-	(*eth_dev)->data->drv_name = drivername;
-	(*eth_dev)->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	return 0;
 
@@ -876,16 +871,12 @@ error:
 }
 
 static int
-rte_eth_from_pcaps_n_dumpers(const char *name,
-		struct rx_pcaps *rx_queues,
-		const unsigned nb_rx_queues,
-		struct tx_pcaps *tx_queues,
-		const unsigned nb_tx_queues,
-		const unsigned numa_node,
-		struct rte_kvargs *kvlist)
+rte_eth_from_pcaps_common(const char *name, struct rx_pcaps *rx_queues,
+		const unsigned nb_rx_queues, struct tx_pcaps *tx_queues,
+		const unsigned nb_tx_queues, const unsigned numa_node,
+		struct rte_kvargs *kvlist, struct pmd_internals **internals,
+		struct rte_eth_dev **eth_dev)
 {
-	struct pmd_internals *internals = NULL;
-	struct rte_eth_dev *eth_dev = NULL;
 	unsigned i;
 
 	/* do some parameter checking */
@@ -895,28 +886,51 @@ rte_eth_from_pcaps_n_dumpers(const char *name,
 		return -1;
 
 	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
-			&internals, &eth_dev, kvlist) < 0)
+			internals, eth_dev, kvlist) < 0)
 		return -1;
 
 	for (i = 0; i < nb_rx_queues; i++) {
-		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
-		snprintf(internals->rx_queue[i].name,
-			sizeof(internals->rx_queue[i].name), "%s",
+		(*internals)->rx_queue[i].pcap = rx_queues->pcaps[i];
+		snprintf((*internals)->rx_queue[i].name,
+			sizeof((*internals)->rx_queue[i].name), "%s",
 			rx_queues->names[i]);
-		snprintf(internals->rx_queue[i].type,
-			sizeof(internals->rx_queue[i].type), "%s",
+		snprintf((*internals)->rx_queue[i].type,
+			sizeof((*internals)->rx_queue[i].type), "%s",
 			rx_queues->types[i]);
 	}
 	for (i = 0; i < nb_tx_queues; i++) {
-		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
-		snprintf(internals->tx_queue[i].name,
-			sizeof(internals->tx_queue[i].name), "%s",
+		(*internals)->tx_queue[i].dumper = tx_queues->dumpers[i];
+		snprintf((*internals)->tx_queue[i].name,
+			sizeof((*internals)->tx_queue[i].name), "%s",
 			tx_queues->names[i]);
-		snprintf(internals->tx_queue[i].type,
-			sizeof(internals->tx_queue[i].type), "%s",
+		snprintf((*internals)->tx_queue[i].type,
+			sizeof((*internals)->tx_queue[i].type), "%s",
 			tx_queues->types[i]);
 	}
 
+	return 0;
+}
+
+static int
+rte_eth_from_pcaps_n_dumpers(const char *name,
+		struct rx_pcaps *rx_queues,
+		const unsigned nb_rx_queues,
+		struct tx_pcaps *tx_queues,
+		const unsigned nb_tx_queues,
+		const unsigned numa_node,
+		struct rte_kvargs *kvlist)
+{
+	struct pmd_internals *internals = NULL;
+	struct rte_eth_dev *eth_dev = NULL;
+	int ret;
+
+	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+			tx_queues, nb_tx_queues, numa_node, kvlist,
+			&internals, &eth_dev);
+
+	if (ret < 0)
+		return ret;
+
 	/* using multiple pcaps/interfaces */
 	internals->single_iface = 0;
 
@@ -938,36 +952,14 @@ rte_eth_from_pcaps(const char *name,
 {
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
-	unsigned i;
-
-	/* do some parameter checking */
-	if (rx_queues == NULL && nb_rx_queues > 0)
-		return -1;
-	if (tx_queues == NULL && nb_tx_queues > 0)
-		return -1;
+	int ret;
 
-	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
-			&internals, &eth_dev, kvlist) < 0)
-		return -1;
+	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+			tx_queues, nb_tx_queues, numa_node, kvlist,
+			&internals, &eth_dev);
 
-	for (i = 0; i < nb_rx_queues; i++) {
-		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
-		snprintf(internals->rx_queue[i].name,
-			sizeof(internals->rx_queue[i].name), "%s",
-			rx_queues->names[i]);
-		snprintf(internals->rx_queue[i].type,
-			sizeof(internals->rx_queue[i].type), "%s",
-			rx_queues->types[i]);
-	}
-	for (i = 0; i < nb_tx_queues; i++) {
-		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
-		snprintf(internals->tx_queue[i].name,
-			sizeof(internals->tx_queue[i].name), "%s",
-			tx_queues->names[i]);
-		snprintf(internals->tx_queue[i].type,
-			sizeof(internals->tx_queue[i].type), "%s",
-			tx_queues->types[i]);
-	}
+	if (ret < 0)
+		return ret;
 
 	/* store wether we are using a single interface for rx/tx or not */
 	internals->single_iface = single_iface;
-- 
2.5.0

[PATCH v3 2/3] ring: rename fields in internal data struct

[Ferruh Yigit]

Rename nb_rx/tx_queues fields in internals struct to max_rx/tx_queues

This patch just does variable renaming to help understanding code better,
nothing changed in functionality of the code.

Updated fields required to keep max queue numbers configured. For current
queue number requirements data->nb_rx/tx_queues fields used.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/ring/rte_eth_ring.c | 59 ++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index d92b088..c9ecb0e 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -59,8 +59,8 @@ struct ring_queue {
 };
 
 struct pmd_internals {
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
+	unsigned max_rx_queues;
+	unsigned max_tx_queues;
 
 	struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
 	struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
@@ -138,7 +138,7 @@ eth_dev_set_link_up(struct rte_eth_dev *dev)
 }
 
 static int
-eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
+eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 				    uint16_t nb_rx_desc __rte_unused,
 				    unsigned int socket_id __rte_unused,
 				    const struct rte_eth_rxconf *rx_conf __rte_unused,
@@ -169,36 +169,36 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->driver_name = drivername;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_pktlen = (uint32_t)-1;
-	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
+	dev_info->max_rx_queues = (uint16_t)internals->max_rx_queues;
+	dev_info->max_tx_queues = (uint16_t)internals->max_tx_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
 }
 
 static void
-eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
+eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned i;
 	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
-			i < internal->nb_rx_queues; i++) {
-		igb_stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
-		rx_total += igb_stats->q_ipackets[i];
+			i < dev->data->nb_rx_queues; i++) {
+		stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
+		rx_total += stats->q_ipackets[i];
 	}
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
-			i < internal->nb_tx_queues; i++) {
-		igb_stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
-		igb_stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
-		tx_total += igb_stats->q_opackets[i];
-		tx_err_total += igb_stats->q_errors[i];
+			i < dev->data->nb_tx_queues; i++) {
+		stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
+		stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
+		tx_total += stats->q_opackets[i];
+		tx_err_total += stats->q_errors[i];
 	}
 
-	igb_stats->ipackets = rx_total;
-	igb_stats->opackets = tx_total;
-	igb_stats->oerrors = tx_err_total;
+	stats->ipackets = rx_total;
+	stats->opackets = tx_total;
+	stats->oerrors = tx_err_total;
 }
 
 static void
@@ -206,9 +206,9 @@ eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++)
+	for (i = 0; i < dev->data->nb_rx_queues; i++)
 		internal->rx_ring_queues[i].rx_pkts.cnt = 0;
-	for (i = 0; i < internal->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		internal->tx_ring_queues[i].tx_pkts.cnt = 0;
 		internal->tx_ring_queues[i].err_pkts.cnt = 0;
 	}
@@ -262,7 +262,6 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	struct rte_eth_dev_data *data = NULL;
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
-
 	unsigned i;
 
 	/* do some parameter checking */
@@ -291,15 +290,15 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 		goto error;
 	}
 
-	data->rx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_rx_queues,
-			0, numa_node);
+	data->rx_queues = rte_zmalloc_socket(name,
+			sizeof(void *) * nb_rx_queues, 0, numa_node);
 	if (data->rx_queues == NULL) {
 		rte_errno = ENOMEM;
 		goto error;
 	}
 
-	data->tx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_tx_queues,
-			0, numa_node);
+	data->tx_queues = rte_zmalloc_socket(name,
+			sizeof(void *) * nb_tx_queues, 0, numa_node);
 	if (data->tx_queues == NULL) {
 		rte_errno = ENOMEM;
 		goto error;
@@ -327,8 +326,8 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
-	internals->nb_rx_queues = nb_rx_queues;
-	internals->nb_tx_queues = nb_tx_queues;
+	internals->max_rx_queues = nb_rx_queues;
+	internals->max_tx_queues = nb_tx_queues;
 	for (i = 0; i < nb_rx_queues; i++) {
 		internals->rx_ring_queues[i].rng = rx_queues[i];
 		data->rx_queues[i] = &internals->rx_ring_queues[i];
@@ -349,10 +348,10 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	eth_dev->data = data;
 	eth_dev->driver = NULL;
 	eth_dev->dev_ops = &ops;
-	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-	eth_dev->data->kdrv = RTE_KDRV_NONE;
-	eth_dev->data->drv_name = drivername;
-	eth_dev->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	TAILQ_INIT(&(eth_dev->link_intr_cbs));
 
-- 
2.5.0

Re: [PATCH v3 2/3] ring: rename fields in internal data struct

[Bruce Richardson]

On Fri, Feb 26, 2016 at 03:26:54PM +0000, Ferruh Yigit wrote:
> Rename nb_rx/tx_queues fields in internals struct to max_rx/tx_queues
> 
> This patch just does variable renaming to help understanding code better,
> nothing changed in functionality of the code.
> 
> Updated fields required to keep max queue numbers configured. For current
> queue number requirements data->nb_rx/tx_queues fields used.
> 
There are other changes in this patchset other than renaming the rx/tx queue
fields. They should be called out in the commit message, I think. The whitespace
cleanup change should probably be omitted from this patch as not related to
renaming.

/Bruce

[PATCH v3 3/3] null: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- remove duplicate nb_rx/tx_queues fields from internals
2- remove duplicate numa_node field from internals

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Acked-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Acked-by: Nicolas Pernas Maradei <nicolas.pernas.maradei@emutex.com>
---
 drivers/net/null/rte_eth_null.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 77fc988..1c354ad 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -69,10 +69,6 @@ struct null_queue {
 struct pmd_internals {
 	unsigned packet_size;
 	unsigned packet_copy;
-	unsigned numa_node;
-
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
 
 	struct null_queue rx_null_queues[RTE_MAX_QUEUES_PER_PORT];
 	struct null_queue tx_null_queues[RTE_MAX_QUEUES_PER_PORT];
@@ -192,13 +188,8 @@ eth_null_copy_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 }
 
 static int
-eth_dev_configure(struct rte_eth_dev *dev) {
-	struct pmd_internals *internals;
-
-	internals = dev->data->dev_private;
-	internals->nb_rx_queues = dev->data->nb_rx_queues;
-	internals->nb_tx_queues = dev->data->nb_tx_queues;
-
+eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
+{
 	return 0;
 }
 
@@ -237,7 +228,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 
 	internals = dev->data->dev_private;
 
-	if (rx_queue_id >= internals->nb_rx_queues)
+	if (rx_queue_id >= dev->data->nb_rx_queues)
 		return -ENODEV;
 
 	packet_size = internals->packet_size;
@@ -246,7 +237,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 	dev->data->rx_queues[rx_queue_id] =
 		&internals->rx_null_queues[rx_queue_id];
 	dummy_packet = rte_zmalloc_socket(NULL,
-			packet_size, 0, internals->numa_node);
+			packet_size, 0, dev->data->numa_node);
 	if (dummy_packet == NULL)
 		return -ENOMEM;
 
@@ -271,7 +262,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 
 	internals = dev->data->dev_private;
 
-	if (tx_queue_id >= internals->nb_tx_queues)
+	if (tx_queue_id >= dev->data->nb_tx_queues)
 		return -ENODEV;
 
 	packet_size = internals->packet_size;
@@ -279,7 +270,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 	dev->data->tx_queues[tx_queue_id] =
 		&internals->tx_null_queues[tx_queue_id];
 	dummy_packet = rte_zmalloc_socket(NULL,
-			packet_size, 0, internals->numa_node);
+			packet_size, 0, dev->data->numa_node);
 	if (dummy_packet == NULL)
 		return -ENOMEM;
 
@@ -323,7 +314,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 
 	internal = dev->data->dev_private;
 	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
-			RTE_MIN(internal->nb_rx_queues,
+			RTE_MIN(dev->data->nb_rx_queues,
 				RTE_DIM(internal->rx_null_queues)));
 	for (i = 0; i < num_stats; i++) {
 		igb_stats->q_ipackets[i] =
@@ -332,7 +323,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 	}
 
 	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
-			RTE_MIN(internal->nb_tx_queues,
+			RTE_MIN(dev->data->nb_tx_queues,
 				RTE_DIM(internal->tx_null_queues)));
 	for (i = 0; i < num_stats; i++) {
 		igb_stats->q_opackets[i] =
@@ -535,11 +526,8 @@ eth_dev_null_create(const char *name,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the nulls are local per-process */
 
-	internals->nb_rx_queues = nb_rx_queues;
-	internals->nb_tx_queues = nb_tx_queues;
 	internals->packet_size = packet_size;
 	internals->packet_copy = packet_copy;
-	internals->numa_node = numa_node;
 
 	internals->flow_type_rss_offloads =  ETH_RSS_PROTO_MASK;
 	internals->reta_size = RTE_DIM(internals->reta_conf) * RTE_RETA_GROUP_SIZE;
@@ -560,10 +548,10 @@ eth_dev_null_create(const char *name,
 	TAILQ_INIT(&eth_dev->link_intr_cbs);
 
 	eth_dev->driver = NULL;
-	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-	eth_dev->data->kdrv = RTE_KDRV_NONE;
-	eth_dev->data->drv_name = drivername;
-	eth_dev->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	/* finally assign rx and tx ops */
 	if (packet_copy) {
-- 
2.5.0

[PATCH v4 0/3] clean-up on virtual PMDs

[Ferruh Yigit]

This is a clean-up patch, no defect fixed, no functional difference 
expected.

Patch removes duplicated fields between data->dev_private
and data (struct rte_eth_dev_data) for pcap and null PMDs.
For ring, renames some variables and does code cleanup.

Also for pcap: move a common code into a function

v4:
* ring: Commit message updated to mention from code cleanup,
no code update.

v3:
* ring: Add fields in internal data struct back, rename them
these fields required to keep max configured queue number

v2:
* ring: Add memory allocation and queue assignment back, 
these are to make rte_eth_from_ring() work without 
rte_eth_dev_configure() or rte_eth_rx/tx_queue_setup() calls


Ferruh Yigit (3):
  pcap: remove duplicate fields in internal data struct
  ring: variable rename and code cleanup
  null: remove duplicate fields in internal data struct

 drivers/net/null/rte_eth_null.c |  36 ++++-------
 drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
 drivers/net/ring/rte_eth_ring.c |  59 +++++++++---------
 3 files changed, 102 insertions(+), 123 deletions(-)

-- 
2.5.0

[PATCH v4 1/3] pcap: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- Remove duplicate nb_rx/tx_queues fields from internals
2- Move duplicate code into a common function

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Nicolas Pernas Maradei <nicolas.pernas.maradei@emutex.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 130 +++++++++++++++++++---------------------
 1 file changed, 61 insertions(+), 69 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index f9230eb..c8b7dbd 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -103,8 +103,6 @@ struct tx_pcaps {
 struct pmd_internals {
 	struct pcap_rx_queue rx_queue[RTE_PMD_RING_MAX_RX_RINGS];
 	struct pcap_tx_queue tx_queue[RTE_PMD_RING_MAX_TX_RINGS];
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
 	int if_index;
 	int single_iface;
 };
@@ -396,7 +394,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* If not open already, open tx pcaps/dumpers */
-	for (i = 0; i < internals->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		tx = &internals->tx_queue[i];
 
 		if (!tx->dumper && strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) {
@@ -411,7 +409,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 	}
 
 	/* If not open already, open rx pcaps */
-	for (i = 0; i < internals->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		rx = &internals->rx_queue[i];
 
 		if (rx->pcap != NULL)
@@ -457,7 +455,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 		goto status_down;
 	}
 
-	for (i = 0; i < internals->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		tx = &internals->tx_queue[i];
 
 		if (tx->dumper != NULL) {
@@ -471,7 +469,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
 		}
 	}
 
-	for (i = 0; i < internals->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		rx = &internals->rx_queue[i];
 
 		if (rx->pcap != NULL) {
@@ -499,8 +497,8 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->if_index = internals->if_index;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_pktlen = (uint32_t) -1;
-	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
+	dev_info->max_rx_queues = dev->data->nb_rx_queues;
+	dev_info->max_tx_queues = dev->data->nb_tx_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
 }
@@ -515,16 +513,16 @@ eth_stats_get(struct rte_eth_dev *dev,
 	unsigned long tx_packets_err_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
-	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_rx_queues;
-			i++) {
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < dev->data->nb_rx_queues; i++) {
 		igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
 		igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
 		rx_packets_total += igb_stats->q_ipackets[i];
 		rx_bytes_total += igb_stats->q_ibytes[i];
 	}
 
-	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_tx_queues;
-			i++) {
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < dev->data->nb_tx_queues; i++) {
 		igb_stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
 		igb_stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
 		igb_stats->q_errors[i] = internal->tx_queue[i].err_pkts;
@@ -545,11 +543,11 @@ eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++) {
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		internal->rx_queue[i].rx_pkts = 0;
 		internal->rx_queue[i].rx_bytes = 0;
 	}
-	for (i = 0; i < internal->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		internal->tx_queue[i].tx_pkts = 0;
 		internal->tx_queue[i].tx_bytes = 0;
 		internal->tx_queue[i].err_pkts = 0;
@@ -840,9 +838,6 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
-	(*internals)->nb_rx_queues = nb_rx_queues;
-	(*internals)->nb_tx_queues = nb_tx_queues;
-
 	if (pair == NULL)
 		(*internals)->if_index = 0;
 	else
@@ -860,11 +855,11 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
 
 	(*eth_dev)->data = data;
 	(*eth_dev)->dev_ops = &ops;
-	(*eth_dev)->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
 	(*eth_dev)->driver = NULL;
-	(*eth_dev)->data->kdrv = RTE_KDRV_NONE;
-	(*eth_dev)->data->drv_name = drivername;
-	(*eth_dev)->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	return 0;
 
@@ -876,16 +871,12 @@ error:
 }
 
 static int
-rte_eth_from_pcaps_n_dumpers(const char *name,
-		struct rx_pcaps *rx_queues,
-		const unsigned nb_rx_queues,
-		struct tx_pcaps *tx_queues,
-		const unsigned nb_tx_queues,
-		const unsigned numa_node,
-		struct rte_kvargs *kvlist)
+rte_eth_from_pcaps_common(const char *name, struct rx_pcaps *rx_queues,
+		const unsigned nb_rx_queues, struct tx_pcaps *tx_queues,
+		const unsigned nb_tx_queues, const unsigned numa_node,
+		struct rte_kvargs *kvlist, struct pmd_internals **internals,
+		struct rte_eth_dev **eth_dev)
 {
-	struct pmd_internals *internals = NULL;
-	struct rte_eth_dev *eth_dev = NULL;
 	unsigned i;
 
 	/* do some parameter checking */
@@ -895,28 +886,51 @@ rte_eth_from_pcaps_n_dumpers(const char *name,
 		return -1;
 
 	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
-			&internals, &eth_dev, kvlist) < 0)
+			internals, eth_dev, kvlist) < 0)
 		return -1;
 
 	for (i = 0; i < nb_rx_queues; i++) {
-		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
-		snprintf(internals->rx_queue[i].name,
-			sizeof(internals->rx_queue[i].name), "%s",
+		(*internals)->rx_queue[i].pcap = rx_queues->pcaps[i];
+		snprintf((*internals)->rx_queue[i].name,
+			sizeof((*internals)->rx_queue[i].name), "%s",
 			rx_queues->names[i]);
-		snprintf(internals->rx_queue[i].type,
-			sizeof(internals->rx_queue[i].type), "%s",
+		snprintf((*internals)->rx_queue[i].type,
+			sizeof((*internals)->rx_queue[i].type), "%s",
 			rx_queues->types[i]);
 	}
 	for (i = 0; i < nb_tx_queues; i++) {
-		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
-		snprintf(internals->tx_queue[i].name,
-			sizeof(internals->tx_queue[i].name), "%s",
+		(*internals)->tx_queue[i].dumper = tx_queues->dumpers[i];
+		snprintf((*internals)->tx_queue[i].name,
+			sizeof((*internals)->tx_queue[i].name), "%s",
 			tx_queues->names[i]);
-		snprintf(internals->tx_queue[i].type,
-			sizeof(internals->tx_queue[i].type), "%s",
+		snprintf((*internals)->tx_queue[i].type,
+			sizeof((*internals)->tx_queue[i].type), "%s",
 			tx_queues->types[i]);
 	}
 
+	return 0;
+}
+
+static int
+rte_eth_from_pcaps_n_dumpers(const char *name,
+		struct rx_pcaps *rx_queues,
+		const unsigned nb_rx_queues,
+		struct tx_pcaps *tx_queues,
+		const unsigned nb_tx_queues,
+		const unsigned numa_node,
+		struct rte_kvargs *kvlist)
+{
+	struct pmd_internals *internals = NULL;
+	struct rte_eth_dev *eth_dev = NULL;
+	int ret;
+
+	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+			tx_queues, nb_tx_queues, numa_node, kvlist,
+			&internals, &eth_dev);
+
+	if (ret < 0)
+		return ret;
+
 	/* using multiple pcaps/interfaces */
 	internals->single_iface = 0;
 
@@ -938,36 +952,14 @@ rte_eth_from_pcaps(const char *name,
 {
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
-	unsigned i;
-
-	/* do some parameter checking */
-	if (rx_queues == NULL && nb_rx_queues > 0)
-		return -1;
-	if (tx_queues == NULL && nb_tx_queues > 0)
-		return -1;
+	int ret;
 
-	if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
-			&internals, &eth_dev, kvlist) < 0)
-		return -1;
+	ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+			tx_queues, nb_tx_queues, numa_node, kvlist,
+			&internals, &eth_dev);
 
-	for (i = 0; i < nb_rx_queues; i++) {
-		internals->rx_queue[i].pcap = rx_queues->pcaps[i];
-		snprintf(internals->rx_queue[i].name,
-			sizeof(internals->rx_queue[i].name), "%s",
-			rx_queues->names[i]);
-		snprintf(internals->rx_queue[i].type,
-			sizeof(internals->rx_queue[i].type), "%s",
-			rx_queues->types[i]);
-	}
-	for (i = 0; i < nb_tx_queues; i++) {
-		internals->tx_queue[i].dumper = tx_queues->dumpers[i];
-		snprintf(internals->tx_queue[i].name,
-			sizeof(internals->tx_queue[i].name), "%s",
-			tx_queues->names[i]);
-		snprintf(internals->tx_queue[i].type,
-			sizeof(internals->tx_queue[i].type), "%s",
-			tx_queues->types[i]);
-	}
+	if (ret < 0)
+		return ret;
 
 	/* store wether we are using a single interface for rx/tx or not */
 	internals->single_iface = single_iface;
-- 
2.5.0

[PATCH v4 2/3] ring: variable rename and code cleanup

[Ferruh Yigit]

Rename nb_rx/tx_queues fields in internals struct to max_rx/tx_queues
Updated fields required to keep max queue numbers configured. For current
queue number requirements data->nb_rx/tx_queues fields used.

Some checkpatch corrections and code clenaup.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/ring/rte_eth_ring.c | 59 ++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index d92b088..c9ecb0e 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -59,8 +59,8 @@ struct ring_queue {
 };
 
 struct pmd_internals {
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
+	unsigned max_rx_queues;
+	unsigned max_tx_queues;
 
 	struct ring_queue rx_ring_queues[RTE_PMD_RING_MAX_RX_RINGS];
 	struct ring_queue tx_ring_queues[RTE_PMD_RING_MAX_TX_RINGS];
@@ -138,7 +138,7 @@ eth_dev_set_link_up(struct rte_eth_dev *dev)
 }
 
 static int
-eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
+eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 				    uint16_t nb_rx_desc __rte_unused,
 				    unsigned int socket_id __rte_unused,
 				    const struct rte_eth_rxconf *rx_conf __rte_unused,
@@ -169,36 +169,36 @@ eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->driver_name = drivername;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_pktlen = (uint32_t)-1;
-	dev_info->max_rx_queues = (uint16_t)internals->nb_rx_queues;
-	dev_info->max_tx_queues = (uint16_t)internals->nb_tx_queues;
+	dev_info->max_rx_queues = (uint16_t)internals->max_rx_queues;
+	dev_info->max_tx_queues = (uint16_t)internals->max_tx_queues;
 	dev_info->min_rx_bufsize = 0;
 	dev_info->pci_dev = NULL;
 }
 
 static void
-eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
+eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned i;
 	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
-			i < internal->nb_rx_queues; i++) {
-		igb_stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
-		rx_total += igb_stats->q_ipackets[i];
+			i < dev->data->nb_rx_queues; i++) {
+		stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
+		rx_total += stats->q_ipackets[i];
 	}
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
-			i < internal->nb_tx_queues; i++) {
-		igb_stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
-		igb_stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
-		tx_total += igb_stats->q_opackets[i];
-		tx_err_total += igb_stats->q_errors[i];
+			i < dev->data->nb_tx_queues; i++) {
+		stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
+		stats->q_errors[i] = internal->tx_ring_queues[i].err_pkts.cnt;
+		tx_total += stats->q_opackets[i];
+		tx_err_total += stats->q_errors[i];
 	}
 
-	igb_stats->ipackets = rx_total;
-	igb_stats->opackets = tx_total;
-	igb_stats->oerrors = tx_err_total;
+	stats->ipackets = rx_total;
+	stats->opackets = tx_total;
+	stats->oerrors = tx_err_total;
 }
 
 static void
@@ -206,9 +206,9 @@ eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++)
+	for (i = 0; i < dev->data->nb_rx_queues; i++)
 		internal->rx_ring_queues[i].rx_pkts.cnt = 0;
-	for (i = 0; i < internal->nb_tx_queues; i++) {
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		internal->tx_ring_queues[i].tx_pkts.cnt = 0;
 		internal->tx_ring_queues[i].err_pkts.cnt = 0;
 	}
@@ -262,7 +262,6 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	struct rte_eth_dev_data *data = NULL;
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
-
 	unsigned i;
 
 	/* do some parameter checking */
@@ -291,15 +290,15 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 		goto error;
 	}
 
-	data->rx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_rx_queues,
-			0, numa_node);
+	data->rx_queues = rte_zmalloc_socket(name,
+			sizeof(void *) * nb_rx_queues, 0, numa_node);
 	if (data->rx_queues == NULL) {
 		rte_errno = ENOMEM;
 		goto error;
 	}
 
-	data->tx_queues = rte_zmalloc_socket(name, sizeof(void *) * nb_tx_queues,
-			0, numa_node);
+	data->tx_queues = rte_zmalloc_socket(name,
+			sizeof(void *) * nb_tx_queues, 0, numa_node);
 	if (data->tx_queues == NULL) {
 		rte_errno = ENOMEM;
 		goto error;
@@ -327,8 +326,8 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
-	internals->nb_rx_queues = nb_rx_queues;
-	internals->nb_tx_queues = nb_tx_queues;
+	internals->max_rx_queues = nb_rx_queues;
+	internals->max_tx_queues = nb_tx_queues;
 	for (i = 0; i < nb_rx_queues; i++) {
 		internals->rx_ring_queues[i].rng = rx_queues[i];
 		data->rx_queues[i] = &internals->rx_ring_queues[i];
@@ -349,10 +348,10 @@ rte_eth_from_rings(const char *name, struct rte_ring *const rx_queues[],
 	eth_dev->data = data;
 	eth_dev->driver = NULL;
 	eth_dev->dev_ops = &ops;
-	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-	eth_dev->data->kdrv = RTE_KDRV_NONE;
-	eth_dev->data->drv_name = drivername;
-	eth_dev->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	TAILQ_INIT(&(eth_dev->link_intr_cbs));
 
-- 
2.5.0

Re: [PATCH v4 2/3] ring: variable rename and code cleanup

[Bruce Richardson]

On Fri, Feb 26, 2016 at 04:58:08PM +0000, Ferruh Yigit wrote:
> Rename nb_rx/tx_queues fields in internals struct to max_rx/tx_queues
> Updated fields required to keep max queue numbers configured. For current
> queue number requirements data->nb_rx/tx_queues fields used.
> 
> Some checkpatch corrections and code clenaup.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v4 3/3] null: remove duplicate fields in internal data struct

[Ferruh Yigit]

1- remove duplicate nb_rx/tx_queues fields from internals
2- remove duplicate numa_node field from internals

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Acked-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Acked-by: Nicolas Pernas Maradei <nicolas.pernas.maradei@emutex.com>
---
 drivers/net/null/rte_eth_null.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 77fc988..1c354ad 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -69,10 +69,6 @@ struct null_queue {
 struct pmd_internals {
 	unsigned packet_size;
 	unsigned packet_copy;
-	unsigned numa_node;
-
-	unsigned nb_rx_queues;
-	unsigned nb_tx_queues;
 
 	struct null_queue rx_null_queues[RTE_MAX_QUEUES_PER_PORT];
 	struct null_queue tx_null_queues[RTE_MAX_QUEUES_PER_PORT];
@@ -192,13 +188,8 @@ eth_null_copy_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 }
 
 static int
-eth_dev_configure(struct rte_eth_dev *dev) {
-	struct pmd_internals *internals;
-
-	internals = dev->data->dev_private;
-	internals->nb_rx_queues = dev->data->nb_rx_queues;
-	internals->nb_tx_queues = dev->data->nb_tx_queues;
-
+eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
+{
 	return 0;
 }
 
@@ -237,7 +228,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 
 	internals = dev->data->dev_private;
 
-	if (rx_queue_id >= internals->nb_rx_queues)
+	if (rx_queue_id >= dev->data->nb_rx_queues)
 		return -ENODEV;
 
 	packet_size = internals->packet_size;
@@ -246,7 +237,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 	dev->data->rx_queues[rx_queue_id] =
 		&internals->rx_null_queues[rx_queue_id];
 	dummy_packet = rte_zmalloc_socket(NULL,
-			packet_size, 0, internals->numa_node);
+			packet_size, 0, dev->data->numa_node);
 	if (dummy_packet == NULL)
 		return -ENOMEM;
 
@@ -271,7 +262,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 
 	internals = dev->data->dev_private;
 
-	if (tx_queue_id >= internals->nb_tx_queues)
+	if (tx_queue_id >= dev->data->nb_tx_queues)
 		return -ENODEV;
 
 	packet_size = internals->packet_size;
@@ -279,7 +270,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 	dev->data->tx_queues[tx_queue_id] =
 		&internals->tx_null_queues[tx_queue_id];
 	dummy_packet = rte_zmalloc_socket(NULL,
-			packet_size, 0, internals->numa_node);
+			packet_size, 0, dev->data->numa_node);
 	if (dummy_packet == NULL)
 		return -ENOMEM;
 
@@ -323,7 +314,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 
 	internal = dev->data->dev_private;
 	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
-			RTE_MIN(internal->nb_rx_queues,
+			RTE_MIN(dev->data->nb_rx_queues,
 				RTE_DIM(internal->rx_null_queues)));
 	for (i = 0; i < num_stats; i++) {
 		igb_stats->q_ipackets[i] =
@@ -332,7 +323,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 	}
 
 	num_stats = RTE_MIN((unsigned)RTE_ETHDEV_QUEUE_STAT_CNTRS,
-			RTE_MIN(internal->nb_tx_queues,
+			RTE_MIN(dev->data->nb_tx_queues,
 				RTE_DIM(internal->tx_null_queues)));
 	for (i = 0; i < num_stats; i++) {
 		igb_stats->q_opackets[i] =
@@ -535,11 +526,8 @@ eth_dev_null_create(const char *name,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the nulls are local per-process */
 
-	internals->nb_rx_queues = nb_rx_queues;
-	internals->nb_tx_queues = nb_tx_queues;
 	internals->packet_size = packet_size;
 	internals->packet_copy = packet_copy;
-	internals->numa_node = numa_node;
 
 	internals->flow_type_rss_offloads =  ETH_RSS_PROTO_MASK;
 	internals->reta_size = RTE_DIM(internals->reta_conf) * RTE_RETA_GROUP_SIZE;
@@ -560,10 +548,10 @@ eth_dev_null_create(const char *name,
 	TAILQ_INIT(&eth_dev->link_intr_cbs);
 
 	eth_dev->driver = NULL;
-	eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
-	eth_dev->data->kdrv = RTE_KDRV_NONE;
-	eth_dev->data->drv_name = drivername;
-	eth_dev->data->numa_node = numa_node;
+	data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+	data->kdrv = RTE_KDRV_NONE;
+	data->drv_name = drivername;
+	data->numa_node = numa_node;
 
 	/* finally assign rx and tx ops */
 	if (packet_copy) {
-- 
2.5.0

Re: [PATCH v4 0/3] clean-up on virtual PMDs

[Bruce Richardson]

On Fri, Feb 26, 2016 at 04:58:06PM +0000, Ferruh Yigit wrote:
> This is a clean-up patch, no defect fixed, no functional difference 
> expected.
> 
> Patch removes duplicated fields between data->dev_private
> and data (struct rte_eth_dev_data) for pcap and null PMDs.
> For ring, renames some variables and does code cleanup.
> 
> Also for pcap: move a common code into a function
> 
> v4:
> * ring: Commit message updated to mention from code cleanup,
> no code update.
> 
> v3:
> * ring: Add fields in internal data struct back, rename them
> these fields required to keep max configured queue number
> 
> v2:
> * ring: Add memory allocation and queue assignment back, 
> these are to make rte_eth_from_ring() work without 
> rte_eth_dev_configure() or rte_eth_rx/tx_queue_setup() calls
> 
> 
> Ferruh Yigit (3):
>   pcap: remove duplicate fields in internal data struct
>   ring: variable rename and code cleanup
>   null: remove duplicate fields in internal data struct
>
Applied to dpdk-next-net/rel_16_04

/Bruce