[PATCH 1/2] ethdev: ensure same name size for device and ethdev

[PATCH 1/2] ethdev: ensure same name size for device and ethdev

[Ferruh Yigit]

rte_device->name copied into eth_dev->name, right now size is same for
both but the requirement is not clear.

This patch highlights the relation without changing actual sizes.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 lib/librte_eal/common/include/rte_dev.h     | 2 ++
 lib/librte_eal/common/include/rte_devargs.h | 2 +-
 lib/librte_ether/rte_ethdev.h               | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index de20c06..fdd5b11 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -132,6 +132,8 @@ struct rte_driver {
 	const char *alias;              /**< Driver alias. */
 };
 
+#define RTE_DEV_NAME_MAX_LEN (32)
+
 /**
  * A structure describing a generic device.
  */
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 88120a1..b11cbfc 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -86,7 +86,7 @@ struct rte_devargs {
 		/** Used if type is RTE_DEVTYPE_VIRTUAL. */
 		struct {
 			/** Driver name. */
-			char drv_name[32];
+			char drv_name[RTE_DEV_NAME_MAX_LEN];
 		} virt;
 	};
 	/** Arguments string as given by user or "" for no argument. */
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 97c3b8e..d1da369 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1689,7 +1689,7 @@ struct rte_eth_dev_sriov {
 };
 #define RTE_ETH_DEV_SRIOV(dev)         ((dev)->data->sriov)
 
-#define RTE_ETH_NAME_MAX_LEN (32)
+#define RTE_ETH_NAME_MAX_LEN RTE_DEV_NAME_MAX_LEN
 
 /**
  * @internal
-- 
2.9.3

[PATCH 2/2] drivers/net: use device name from device structure

[Ferruh Yigit]

Device name resides in two different locations, in rte_device->name and
in ethernet device private data.

For now, the copy in the ethernet device private data is required for
multi process support, the name is the how secondary process finds about
primary process device.

But for drivers there is no reason to use the copy in the ethernet
device private data.

This patch updates PMDs to use only rte_device->name.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Gaetan Rivet <gaetan.rivet@6wind.com>
Cc: Jan Blunck <jblunck@infradead.org>
---
 drivers/net/af_packet/rte_eth_af_packet.c |  2 +-
 drivers/net/bnxt/bnxt_irq.c               |  2 +-
 drivers/net/bonding/rte_eth_bond_alb.c    |  5 +++--
 drivers/net/bonding/rte_eth_bond_args.c   |  2 +-
 drivers/net/bonding/rte_eth_bond_pmd.c    |  6 +++---
 drivers/net/i40e/i40e_ethdev.c            |  6 +++---
 drivers/net/ixgbe/ixgbe_ethdev.c          |  4 ++--
 drivers/net/kni/rte_eth_kni.c             |  2 +-
 drivers/net/tap/rte_eth_tap.c             | 12 ++++++------
 9 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 5620acd..9a47852 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -413,7 +413,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
 	if (data_size > buf_size) {
 		RTE_LOG(ERR, PMD,
 			"%s: %d bytes will not fit in mbuf (%d bytes)\n",
-			dev->data->name, data_size, buf_size);
+			dev->device->name, data_size, buf_size);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/net/bnxt/bnxt_irq.c b/drivers/net/bnxt/bnxt_irq.c
index 20e17ff..4d9eec1 100644
--- a/drivers/net/bnxt/bnxt_irq.c
+++ b/drivers/net/bnxt/bnxt_irq.c
@@ -126,7 +126,7 @@ int bnxt_setup_int(struct bnxt *bp)
 		for (i = 0; i < total_vecs; i++) {
 			bp->irq_tbl[i].vector = i;
 			snprintf(bp->irq_tbl[i].name, len,
-				 "%s-%d", bp->eth_dev->data->name, i);
+				 "%s-%d", bp->eth_dev->device->name, i);
 			bp->irq_tbl[i].handler = bnxt_int_handler;
 		}
 	} else {
diff --git a/drivers/net/bonding/rte_eth_bond_alb.c b/drivers/net/bonding/rte_eth_bond_alb.c
index 38f5c4d..7f6884d 100644
--- a/drivers/net/bonding/rte_eth_bond_alb.c
+++ b/drivers/net/bonding/rte_eth_bond_alb.c
@@ -80,7 +80,8 @@ bond_mode_alb_enable(struct rte_eth_dev *bond_dev)
 		 * The value is chosen to be cache aligned.
 		 */
 		data_size = 256 + RTE_PKTMBUF_HEADROOM;
-		snprintf(mem_name, sizeof(mem_name), "%s_MODE6", bond_dev->data->name);
+		snprintf(mem_name, sizeof(mem_name), "%s_MODE6",
+				bond_dev->device->name);
 		internals->mode6.mempool = rte_pktmbuf_pool_create(mem_name,
 			512 * RTE_MAX_ETHPORTS,
 			RTE_MEMPOOL_CACHE_MAX_SIZE >= 32 ?
@@ -89,7 +90,7 @@ bond_mode_alb_enable(struct rte_eth_dev *bond_dev)
 
 		if (internals->mode6.mempool == NULL) {
 			RTE_LOG(ERR, PMD, "%s: Failed to initialize ALB mempool.\n",
-					bond_dev->data->name);
+					bond_dev->device->name);
 			goto mempool_alloc_error;
 		}
 	}
diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
index dc468d2..bc764f6 100644
--- a/drivers/net/bonding/rte_eth_bond_args.c
+++ b/drivers/net/bonding/rte_eth_bond_args.c
@@ -90,7 +90,7 @@ find_port_id_by_dev_name(const char *name)
 		if (rte_eth_devices[i].data == NULL)
 			continue;
 
-		if (strcmp(rte_eth_devices[i].data->name, name) == 0)
+		if (strcmp(rte_eth_devices[i].device->name, name) == 0)
 			return i;
 	}
 	return -1;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 82959ab..92aed45 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1666,7 +1666,7 @@ bond_ethdev_close(struct rte_eth_dev *dev)
 	uint8_t bond_port_id = internals->port_id;
 	int skipped = 0;
 
-	RTE_LOG(INFO, EAL, "Closing bonded device %s\n", dev->data->name);
+	RTE_LOG(INFO, EAL, "Closing bonded device %s\n", dev->device->name);
 	while (internals->slave_count != skipped) {
 		uint8_t port_id = internals->slaves[skipped].port_id;
 
@@ -1675,7 +1675,7 @@ bond_ethdev_close(struct rte_eth_dev *dev)
 		if (rte_eth_bond_slave_remove(bond_port_id, port_id) != 0) {
 			RTE_LOG(ERR, EAL,
 				"Failed to remove port %d from bonded device "
-				"%s\n", port_id, dev->data->name);
+				"%s\n", port_id, dev->device->name);
 			skipped++;
 		}
 	}
@@ -2498,7 +2498,7 @@ bond_remove(struct rte_vdev_device *dev)
 static int
 bond_ethdev_configure(struct rte_eth_dev *dev)
 {
-	char *name = dev->data->name;
+	const char *name = dev->device->name;
 	struct bond_dev_private *internals = dev->data->dev_private;
 	struct rte_kvargs *kvlist = internals->kvlist;
 	int arg_count;
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 0ac85f0..7b350b6 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -919,7 +919,7 @@ i40e_init_ethtype_filter_list(struct rte_eth_dev *dev)
 	/* Initialize ethertype filter rule list and hash */
 	TAILQ_INIT(&ethertype_rule->ethertype_list);
 	snprintf(ethertype_hash_name, RTE_HASH_NAMESIZE,
-		 "ethertype_%s", dev->data->name);
+		 "ethertype_%s", dev->device->name);
 	ethertype_rule->hash_table = rte_hash_create(&ethertype_hash_params);
 	if (!ethertype_rule->hash_table) {
 		PMD_INIT_LOG(ERR, "Failed to create ethertype hash table!");
@@ -964,7 +964,7 @@ i40e_init_tunnel_filter_list(struct rte_eth_dev *dev)
 	/* Initialize tunnel filter rule list and hash */
 	TAILQ_INIT(&tunnel_rule->tunnel_list);
 	snprintf(tunnel_hash_name, RTE_HASH_NAMESIZE,
-		 "tunnel_%s", dev->data->name);
+		 "tunnel_%s", dev->device->name);
 	tunnel_rule->hash_table = rte_hash_create(&tunnel_hash_params);
 	if (!tunnel_rule->hash_table) {
 		PMD_INIT_LOG(ERR, "Failed to create tunnel hash table!");
@@ -1009,7 +1009,7 @@ i40e_init_fdir_filter_list(struct rte_eth_dev *dev)
 	/* Initialize flow director filter rule list and hash */
 	TAILQ_INIT(&fdir_info->fdir_list);
 	snprintf(fdir_hash_name, RTE_HASH_NAMESIZE,
-		 "fdir_%s", dev->data->name);
+		 "fdir_%s", dev->device->name);
 	fdir_info->hash_table = rte_hash_create(&fdir_hash_params);
 	if (!fdir_info->hash_table) {
 		PMD_INIT_LOG(ERR, "Failed to create fdir hash table!");
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 026178b..a8cd883 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1491,7 +1491,7 @@ static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev)
 
 	TAILQ_INIT(&fdir_info->fdir_list);
 	snprintf(fdir_hash_name, RTE_HASH_NAMESIZE,
-		 "fdir_%s", eth_dev->data->name);
+		 "fdir_%s", eth_dev->device->name);
 	fdir_info->hash_handle = rte_hash_create(&fdir_hash_params);
 	if (!fdir_info->hash_handle) {
 		PMD_INIT_LOG(ERR, "Failed to create fdir hash table!");
@@ -1527,7 +1527,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 
 	TAILQ_INIT(&l2_tn_info->l2_tn_list);
 	snprintf(l2_tn_hash_name, RTE_HASH_NAMESIZE,
-		 "l2_tn_%s", eth_dev->data->name);
+		 "l2_tn_%s", eth_dev->device->name);
 	l2_tn_info->hash_handle = rte_hash_create(&l2_tn_hash_params);
 	if (!l2_tn_info->hash_handle) {
 		PMD_INIT_LOG(ERR, "Failed to create L2 TN hash table!");
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index f688d91..72a2733 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -145,7 +145,7 @@ eth_kni_start(struct rte_eth_dev *dev)
 	uint16_t port_id = dev->data->port_id;
 	struct rte_mempool *mb_pool;
 	struct rte_kni_conf conf;
-	const char *name = dev->data->name + 4; /* remove net_ */
+	const char *name = dev->device->name + 4; /* remove net_ */
 
 	snprintf(conf.name, RTE_KNI_NAMESIZE, "%s", name);
 	conf.force_bind = 0;
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 305f018..1314e30 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -843,7 +843,7 @@ tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 
 	if (is_zero_ether_addr(mac_addr)) {
 		RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n",
-			dev->data->name);
+			dev->device->name);
 		return;
 	}
 	/* Check the actual current MAC address on the tap netdevice */
@@ -868,13 +868,13 @@ tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 		if (tap_flow_implicit_destroy(pmd, TAP_REMOTE_LOCAL_MAC) < 0) {
 			RTE_LOG(ERR, PMD,
 				"%s: Couldn't delete MAC redirection rule\n",
-				dev->data->name);
+				dev->device->name);
 			return;
 		}
 		if (tap_flow_implicit_create(pmd, TAP_REMOTE_LOCAL_MAC) < 0)
 			RTE_LOG(ERR, PMD,
 				"%s: Couldn't add MAC redirection rule\n",
-				dev->data->name);
+				dev->device->name);
 	}
 }
 
@@ -937,12 +937,12 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->trigger_seen = 1; /* force initial burst */
 	rxq->in_port = dev->data->port_id;
 	rxq->nb_rx_desc = nb_desc;
-	iovecs = rte_zmalloc_socket(dev->data->name, sizeof(*iovecs), 0,
+	iovecs = rte_zmalloc_socket(dev->device->name, sizeof(*iovecs), 0,
 				    socket_id);
 	if (!iovecs) {
 		RTE_LOG(WARNING, PMD,
 			"%s: Couldn't allocate %d RX descriptors\n",
-			dev->data->name, nb_desc);
+			dev->device->name, nb_desc);
 		return -ENOMEM;
 	}
 	rxq->iovecs = iovecs;
@@ -962,7 +962,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 		if (!*tmp) {
 			RTE_LOG(WARNING, PMD,
 				"%s: couldn't allocate memory for queue %d\n",
-				dev->data->name, rx_queue_id);
+				dev->device->name, rx_queue_id);
 			ret = -ENOMEM;
 			goto error;
 		}
-- 
2.9.3

Re: [PATCH 2/2] drivers/net: use device name from device structure

[Thomas Monjalon]

26/05/2017 18:11, Ferruh Yigit:
> Device name resides in two different locations, in rte_device->name and
> in ethernet device private data.

Yes would be nice to remove the name from rte_eth_dev_data.

> For now, the copy in the ethernet device private data is required for
> multi process support, the name is the how secondary process finds about
> primary process device.

Yes it is in rte_eth_dev_attach_secondary().
This secondary process forces us to write ugly data structures.

> But for drivers there is no reason to use the copy in the ethernet
> device private data.

Yes I agree.
There are probably other places where we can avoid using this field.
I see rte_eth_dev_get_name_by_port() and rte_eth_dev_get_port_by_name()
using rte_eth_dev_data[port].name.

> This patch updates PMDs to use only rte_device->name.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [PATCH 2/2] drivers/net: use device name from device structure

[Ferruh Yigit]

On 6/9/2017 2:52 PM, Thomas Monjalon wrote:
> 26/05/2017 18:11, Ferruh Yigit:
>> Device name resides in two different locations, in rte_device->name and
>> in ethernet device private data.
> 
> Yes would be nice to remove the name from rte_eth_dev_data.
> 
>> For now, the copy in the ethernet device private data is required for
>> multi process support, the name is the how secondary process finds about
>> primary process device.
> 
> Yes it is in rte_eth_dev_attach_secondary().
> This secondary process forces us to write ugly data structures.
> 
>> But for drivers there is no reason to use the copy in the ethernet
>> device private data.
> 
> Yes I agree.
> There are probably other places where we can avoid using this field.
> I see rte_eth_dev_get_name_by_port() and rte_eth_dev_get_port_by_name()
> using rte_eth_dev_data[port].name.

I will scan librte_ether for the cases private name usage can be
eliminated and will send a new version of the patch.

> 
>> This patch updates PMDs to use only rte_device->name.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

[PATCH v2 1/3] ethdev: ensure same name size for device and ethdev

[Ferruh Yigit]

rte_device->name copied into eth_dev->name, right now size is same for
both but the requirement is not clear.

This patch highlights the relation without changing actual sizes.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 lib/librte_eal/common/include/rte_dev.h     | 2 ++
 lib/librte_eal/common/include/rte_devargs.h | 2 +-
 lib/librte_ether/rte_ethdev.h               | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index de20c06..fdd5b11 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -132,6 +132,8 @@ struct rte_driver {
 	const char *alias;              /**< Driver alias. */
 };
 
+#define RTE_DEV_NAME_MAX_LEN (32)
+
 /**
  * A structure describing a generic device.
  */
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 88120a1..b11cbfc 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -86,7 +86,7 @@ struct rte_devargs {
 		/** Used if type is RTE_DEVTYPE_VIRTUAL. */
 		struct {
 			/** Driver name. */
-			char drv_name[32];
+			char drv_name[RTE_DEV_NAME_MAX_LEN];
 		} virt;
 	};
 	/** Arguments string as given by user or "" for no argument. */
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index d1297a5..e8ac546 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1624,7 +1624,7 @@ struct rte_eth_dev_sriov {
 };
 #define RTE_ETH_DEV_SRIOV(dev)         ((dev)->data->sriov)
 
-#define RTE_ETH_NAME_MAX_LEN (32)
+#define RTE_ETH_NAME_MAX_LEN RTE_DEV_NAME_MAX_LEN
 
 /**
  * @internal
-- 
2.9.4

[PATCH v2 2/3] drivers/net: use device name from device structure

[Ferruh Yigit]

Device name resides in two different locations, in rte_device->name and
in ethernet device private data.

For now, the copy in the ethernet device private data is required for
multi process support, the name is the how secondary process finds about
primary process device.

But for drivers there is no reason to use the copy in the ethernet
device private data.

This patch updates PMDs to use only rte_device->name.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Gaetan Rivet <gaetan.rivet@6wind.com>
Cc: Jan Blunck <jblunck@infradead.org>
---
 drivers/net/af_packet/rte_eth_af_packet.c |  2 +-
 drivers/net/bnxt/bnxt_irq.c               |  2 +-
 drivers/net/bonding/rte_eth_bond_alb.c    |  5 +++--
 drivers/net/bonding/rte_eth_bond_args.c   |  2 +-
 drivers/net/bonding/rte_eth_bond_pmd.c    |  6 +++---
 drivers/net/i40e/i40e_ethdev.c            |  6 +++---
 drivers/net/ixgbe/ixgbe_ethdev.c          |  4 ++--
 drivers/net/kni/rte_eth_kni.c             |  2 +-
 drivers/net/ring/rte_eth_ring.c           | 22 ++++++++++++----------
 drivers/net/tap/rte_eth_tap.c             | 12 ++++++------
 10 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 5620acd..9a47852 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -413,7 +413,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
 	if (data_size > buf_size) {
 		RTE_LOG(ERR, PMD,
 			"%s: %d bytes will not fit in mbuf (%d bytes)\n",
-			dev->data->name, data_size, buf_size);
+			dev->device->name, data_size, buf_size);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/net/bnxt/bnxt_irq.c b/drivers/net/bnxt/bnxt_irq.c
index a3d235d..47cda7e 100644
--- a/drivers/net/bnxt/bnxt_irq.c
+++ b/drivers/net/bnxt/bnxt_irq.c
@@ -137,7 +137,7 @@ int bnxt_setup_int(struct bnxt *bp)
 		for (i = 0; i < total_vecs; i++) {
 			bp->irq_tbl[i].vector = i;
 			snprintf(bp->irq_tbl[i].name, len,
-				 "%s-%d", bp->eth_dev->data->name, i);
+				 "%s-%d", bp->eth_dev->device->name, i);
 			bp->irq_tbl[i].handler = bnxt_int_handler;
 		}
 	} else {
diff --git a/drivers/net/bonding/rte_eth_bond_alb.c b/drivers/net/bonding/rte_eth_bond_alb.c
index 38f5c4d..7f6884d 100644
--- a/drivers/net/bonding/rte_eth_bond_alb.c
+++ b/drivers/net/bonding/rte_eth_bond_alb.c
@@ -80,7 +80,8 @@ bond_mode_alb_enable(struct rte_eth_dev *bond_dev)
 		 * The value is chosen to be cache aligned.
 		 */
 		data_size = 256 + RTE_PKTMBUF_HEADROOM;
-		snprintf(mem_name, sizeof(mem_name), "%s_MODE6", bond_dev->data->name);
+		snprintf(mem_name, sizeof(mem_name), "%s_MODE6",
+				bond_dev->device->name);
 		internals->mode6.mempool = rte_pktmbuf_pool_create(mem_name,
 			512 * RTE_MAX_ETHPORTS,
 			RTE_MEMPOOL_CACHE_MAX_SIZE >= 32 ?
@@ -89,7 +90,7 @@ bond_mode_alb_enable(struct rte_eth_dev *bond_dev)
 
 		if (internals->mode6.mempool == NULL) {
 			RTE_LOG(ERR, PMD, "%s: Failed to initialize ALB mempool.\n",
-					bond_dev->data->name);
+					bond_dev->device->name);
 			goto mempool_alloc_error;
 		}
 	}
diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
index dc468d2..bc764f6 100644
--- a/drivers/net/bonding/rte_eth_bond_args.c
+++ b/drivers/net/bonding/rte_eth_bond_args.c
@@ -90,7 +90,7 @@ find_port_id_by_dev_name(const char *name)
 		if (rte_eth_devices[i].data == NULL)
 			continue;
 
-		if (strcmp(rte_eth_devices[i].data->name, name) == 0)
+		if (strcmp(rte_eth_devices[i].device->name, name) == 0)
 			return i;
 	}
 	return -1;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 82959ab..92aed45 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1666,7 +1666,7 @@ bond_ethdev_close(struct rte_eth_dev *dev)
 	uint8_t bond_port_id = internals->port_id;
 	int skipped = 0;
 
-	RTE_LOG(INFO, EAL, "Closing bonded device %s\n", dev->data->name);
+	RTE_LOG(INFO, EAL, "Closing bonded device %s\n", dev->device->name);
 	while (internals->slave_count != skipped) {
 		uint8_t port_id = internals->slaves[skipped].port_id;
 
@@ -1675,7 +1675,7 @@ bond_ethdev_close(struct rte_eth_dev *dev)
 		if (rte_eth_bond_slave_remove(bond_port_id, port_id) != 0) {
 			RTE_LOG(ERR, EAL,
 				"Failed to remove port %d from bonded device "
-				"%s\n", port_id, dev->data->name);
+				"%s\n", port_id, dev->device->name);
 			skipped++;
 		}
 	}
@@ -2498,7 +2498,7 @@ bond_remove(struct rte_vdev_device *dev)
 static int
 bond_ethdev_configure(struct rte_eth_dev *dev)
 {
-	char *name = dev->data->name;
+	const char *name = dev->device->name;
 	struct bond_dev_private *internals = dev->data->dev_private;
 	struct rte_kvargs *kvlist = internals->kvlist;
 	int arg_count;
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index f614949..8b61c04 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -919,7 +919,7 @@ i40e_init_ethtype_filter_list(struct rte_eth_dev *dev)
 	/* Initialize ethertype filter rule list and hash */
 	TAILQ_INIT(&ethertype_rule->ethertype_list);
 	snprintf(ethertype_hash_name, RTE_HASH_NAMESIZE,
-		 "ethertype_%s", dev->data->name);
+		 "ethertype_%s", dev->device->name);
 	ethertype_rule->hash_table = rte_hash_create(&ethertype_hash_params);
 	if (!ethertype_rule->hash_table) {
 		PMD_INIT_LOG(ERR, "Failed to create ethertype hash table!");
@@ -964,7 +964,7 @@ i40e_init_tunnel_filter_list(struct rte_eth_dev *dev)
 	/* Initialize tunnel filter rule list and hash */
 	TAILQ_INIT(&tunnel_rule->tunnel_list);
 	snprintf(tunnel_hash_name, RTE_HASH_NAMESIZE,
-		 "tunnel_%s", dev->data->name);
+		 "tunnel_%s", dev->device->name);
 	tunnel_rule->hash_table = rte_hash_create(&tunnel_hash_params);
 	if (!tunnel_rule->hash_table) {
 		PMD_INIT_LOG(ERR, "Failed to create tunnel hash table!");
@@ -1009,7 +1009,7 @@ i40e_init_fdir_filter_list(struct rte_eth_dev *dev)
 	/* Initialize flow director filter rule list and hash */
 	TAILQ_INIT(&fdir_info->fdir_list);
 	snprintf(fdir_hash_name, RTE_HASH_NAMESIZE,
-		 "fdir_%s", dev->data->name);
+		 "fdir_%s", dev->device->name);
 	fdir_info->hash_table = rte_hash_create(&fdir_hash_params);
 	if (!fdir_info->hash_table) {
 		PMD_INIT_LOG(ERR, "Failed to create fdir hash table!");
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 504e951..0e04bd4 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1480,7 +1480,7 @@ static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev)
 
 	TAILQ_INIT(&fdir_info->fdir_list);
 	snprintf(fdir_hash_name, RTE_HASH_NAMESIZE,
-		 "fdir_%s", eth_dev->data->name);
+		 "fdir_%s", eth_dev->device->name);
 	fdir_info->hash_handle = rte_hash_create(&fdir_hash_params);
 	if (!fdir_info->hash_handle) {
 		PMD_INIT_LOG(ERR, "Failed to create fdir hash table!");
@@ -1516,7 +1516,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 
 	TAILQ_INIT(&l2_tn_info->l2_tn_list);
 	snprintf(l2_tn_hash_name, RTE_HASH_NAMESIZE,
-		 "l2_tn_%s", eth_dev->data->name);
+		 "l2_tn_%s", eth_dev->device->name);
 	l2_tn_info->hash_handle = rte_hash_create(&l2_tn_hash_params);
 	if (!l2_tn_info->hash_handle) {
 		PMD_INIT_LOG(ERR, "Failed to create L2 TN hash table!");
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index f688d91..72a2733 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -145,7 +145,7 @@ eth_kni_start(struct rte_eth_dev *dev)
 	uint16_t port_id = dev->data->port_id;
 	struct rte_mempool *mb_pool;
 	struct rte_kni_conf conf;
-	const char *name = dev->data->name + 4; /* remove net_ */
+	const char *name = dev->device->name + 4; /* remove net_ */
 
 	snprintf(conf.name, RTE_KNI_NAMESIZE, "%s", name);
 	conf.force_bind = 0;
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index d4dce95..6e3f9a3 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -268,6 +268,8 @@ do_eth_dev_ring_create(const char *name,
 	struct rte_eth_dev_data *data = NULL;
 	struct pmd_internals *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
+	void **rx_queues_local = NULL;
+	void **tx_queues_local = NULL;
 	unsigned i;
 
 	RTE_LOG(INFO, PMD, "Creating rings-backed ethdev on numa socket %u\n",
@@ -282,16 +284,16 @@ do_eth_dev_ring_create(const char *name,
 		goto error;
 	}
 
-	data->rx_queues = rte_zmalloc_socket(name,
+	rx_queues_local = rte_zmalloc_socket(name,
 			sizeof(void *) * nb_rx_queues, 0, numa_node);
-	if (data->rx_queues == NULL) {
+	if (rx_queues_local == NULL) {
 		rte_errno = ENOMEM;
 		goto error;
 	}
 
-	data->tx_queues = rte_zmalloc_socket(name,
+	tx_queues_local = rte_zmalloc_socket(name,
 			sizeof(void *) * nb_tx_queues, 0, numa_node);
-	if (data->tx_queues == NULL) {
+	if (tx_queues_local == NULL) {
 		rte_errno = ENOMEM;
 		goto error;
 	}
@@ -318,6 +320,10 @@ do_eth_dev_ring_create(const char *name,
 	/* NOTE: we'll replace the data element, of originally allocated eth_dev
 	 * so the rings are local per-process */
 
+	rte_memcpy(data, eth_dev->data, sizeof(*data));
+	data->rx_queues = rx_queues_local;
+	data->tx_queues = tx_queues_local;
+
 	internals->action = action;
 	internals->max_rx_queues = nb_rx_queues;
 	internals->max_tx_queues = nb_tx_queues;
@@ -331,8 +337,6 @@ do_eth_dev_ring_create(const char *name,
 	}
 
 	data->dev_private = internals;
-	data->port_id = eth_dev->data->port_id;
-	memmove(data->name, eth_dev->data->name, sizeof(data->name));
 	data->nb_rx_queues = (uint16_t)nb_rx_queues;
 	data->nb_tx_queues = (uint16_t)nb_tx_queues;
 	data->dev_link = pmd_link;
@@ -352,10 +356,8 @@ do_eth_dev_ring_create(const char *name,
 	return data->port_id;
 
 error:
-	if (data) {
-		rte_free(data->rx_queues);
-		rte_free(data->tx_queues);
-	}
+	rte_free(rx_queues_local);
+	rte_free(tx_queues_local);
 	rte_free(data);
 	rte_free(internals);
 
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 1bb660e..9d00a81 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -843,7 +843,7 @@ tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 
 	if (is_zero_ether_addr(mac_addr)) {
 		RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n",
-			dev->data->name);
+			dev->device->name);
 		return;
 	}
 	/* Check the actual current MAC address on the tap netdevice */
@@ -868,13 +868,13 @@ tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 		if (tap_flow_implicit_destroy(pmd, TAP_REMOTE_LOCAL_MAC) < 0) {
 			RTE_LOG(ERR, PMD,
 				"%s: Couldn't delete MAC redirection rule\n",
-				dev->data->name);
+				dev->device->name);
 			return;
 		}
 		if (tap_flow_implicit_create(pmd, TAP_REMOTE_LOCAL_MAC) < 0)
 			RTE_LOG(ERR, PMD,
 				"%s: Couldn't add MAC redirection rule\n",
-				dev->data->name);
+				dev->device->name);
 	}
 }
 
@@ -937,12 +937,12 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->trigger_seen = 1; /* force initial burst */
 	rxq->in_port = dev->data->port_id;
 	rxq->nb_rx_desc = nb_desc;
-	iovecs = rte_zmalloc_socket(dev->data->name, sizeof(*iovecs), 0,
+	iovecs = rte_zmalloc_socket(dev->device->name, sizeof(*iovecs), 0,
 				    socket_id);
 	if (!iovecs) {
 		RTE_LOG(WARNING, PMD,
 			"%s: Couldn't allocate %d RX descriptors\n",
-			dev->data->name, nb_desc);
+			dev->device->name, nb_desc);
 		return -ENOMEM;
 	}
 	rxq->iovecs = iovecs;
@@ -962,7 +962,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
 		if (!*tmp) {
 			RTE_LOG(WARNING, PMD,
 				"%s: couldn't allocate memory for queue %d\n",
-				dev->data->name, rx_queue_id);
+				dev->device->name, rx_queue_id);
 			ret = -ENOMEM;
 			goto error;
 		}
-- 
2.9.4

[PATCH v2 3/3] ethdev: use device name from device structure

[Ferruh Yigit]

Device name resides in two different locations, in rte_device->name and
in ethernet device private data.

For now, the copy in the ethernet device private data is required for
multi process support, the name is the how secondary process finds about
primary process device.

But in the ethdev library some eth_dev->data->name usage can be
converted to rte_device->name.

This patch updates ethdev to use rte_device->name when possible.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Gaetan Rivet <gaetan.rivet@6wind.com>
Cc: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_ether/rte_ethdev.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index d257406..1cbe9b9 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -178,9 +178,11 @@ rte_eth_dev_allocated(const char *name)
 	unsigned i;
 
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
-		if ((rte_eth_devices[i].state == RTE_ETH_DEV_ATTACHED) &&
-		    strcmp(rte_eth_devices[i].data->name, name) == 0)
-			return &rte_eth_devices[i];
+		if (rte_eth_devices[i].state == RTE_ETH_DEV_ATTACHED &&
+				rte_eth_devices[i].device) {
+			if (!strcmp(rte_eth_devices[i].device->name, name))
+				return &rte_eth_devices[i];
+		}
 	}
 	return NULL;
 }
@@ -310,7 +312,7 @@ rte_eth_dev_count(void)
 int
 rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
 {
-	char *tmp;
+	const char *tmp;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
@@ -321,7 +323,7 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
 
 	/* shouldn't check 'rte_eth_devices[i].data',
 	 * because it might be overwritten by VDEV PMD */
-	tmp = rte_eth_dev_data[port_id].name;
+	tmp = rte_eth_devices[port_id].device->name;
 	strcpy(name, tmp);
 	return 0;
 }
@@ -329,6 +331,7 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
 int
 rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id)
 {
+	int ret;
 	int i;
 
 	if (name == NULL) {
@@ -341,11 +344,14 @@ rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id)
 
 	*port_id = RTE_MAX_ETHPORTS;
 	RTE_ETH_FOREACH_DEV(i) {
-		if (!strncmp(name,
-			rte_eth_dev_data[i].name, strlen(name))) {
+		if (!rte_eth_devices[i].device)
+			continue;
 
-			*port_id = i;
+		ret = strncmp(name, rte_eth_devices[i].device->name,
+			strlen(name));
 
+		if (!ret) {
+			*port_id = i;
 			return 0;
 		}
 	}
@@ -438,8 +444,8 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 	if (rte_eth_dev_is_detachable(port_id))
 		goto err;
 
-	snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
-		 "%s", rte_eth_devices[port_id].data->name);
+	snprintf(name, sizeof(rte_eth_devices[port_id].device->name),
+		 "%s", rte_eth_devices[port_id].device->name);
 	ret = rte_eal_dev_detach(name);
 	if (ret < 0)
 		goto err;
-- 
2.9.4

Re: [PATCH 2/2] drivers/net: use device name from device structure

[Jan Blunck]

On Fri, Jun 9, 2017 at 3:52 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 26/05/2017 18:11, Ferruh Yigit:
>> Device name resides in two different locations, in rte_device->name and
>> in ethernet device private data.
>
> Yes would be nice to remove the name from rte_eth_dev_data.
>

I wonder if this is really the right thing to do. The name in the
eth_dev data is the eth_dev device name and it might be different from
the low-level device name. Some busses might use UUID as the device
identifier and I don't believe that this is a user friendly name.

>> For now, the copy in the ethernet device private data is required for
>> multi process support, the name is the how secondary process finds about
>> primary process device.
>
> Yes it is in rte_eth_dev_attach_secondary().
> This secondary process forces us to write ugly data structures.
>
>> But for drivers there is no reason to use the copy in the ethernet
>> device private data.
>
> Yes I agree.

Probably. But it also depends on at what stage the driver is using the
name and what information is printed. During probing I would expect
the low-level device name to be printed. After probing the eth_dev PMD
should use the user friendly device name.

> There are probably other places where we can avoid using this field.
> I see rte_eth_dev_get_name_by_port() and rte_eth_dev_get_port_by_name()
> using rte_eth_dev_data[port].name.
>
>> This patch updates PMDs to use only rte_device->name.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>

Re: [PATCH 2/2] drivers/net: use device name from device structure

[Ferruh Yigit]

On 6/10/2017 8:35 AM, Jan Blunck wrote:
> On Fri, Jun 9, 2017 at 3:52 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>> 26/05/2017 18:11, Ferruh Yigit:
>>> Device name resides in two different locations, in rte_device->name and
>>> in ethernet device private data.
>>
>> Yes would be nice to remove the name from rte_eth_dev_data.
>>
> 
> I wonder if this is really the right thing to do. The name in the
> eth_dev data is the eth_dev device name and it might be different from
> the low-level device name. Some busses might use UUID as the device
> identifier and I don't believe that this is a user friendly name.

Right now eth_dev->data->name is same with with rte_dev->device->name.
And there is an assumption that they will be same [1].

But if you think they can be different in the future, I think we can:
1- Keep as it is.
2- Reduce to single variable as much as possible (this patch), when
different naming implemented, update relevant parts according. Since
this is internal structure, I believe won't cause an ABI issue.


[1]
rte_eth_dev_pci_allocate() and rte_eth_vdev_allocate() use
rte_dev->device->name to call rte_eth_dev_allocate(), which inside calls
rte_eth_dev_allocated() with same name. Which assumes any previously
created eth_dev, created with rte_dev->device->name.

> 
>>> For now, the copy in the ethernet device private data is required for
>>> multi process support, the name is the how secondary process finds about
>>> primary process device.
>>
>> Yes it is in rte_eth_dev_attach_secondary().
>> This secondary process forces us to write ugly data structures.
>>
>>> But for drivers there is no reason to use the copy in the ethernet
>>> device private data.
>>
>> Yes I agree.
> 
> Probably. But it also depends on at what stage the driver is using the
> name and what information is printed. During probing I would expect
> the low-level device name to be printed. After probing the eth_dev PMD
> should use the user friendly device name.

This makes sense when different naming used for core device and eth_dev,
but this is not the case for now.

> 
>> There are probably other places where we can avoid using this field.
>> I see rte_eth_dev_get_name_by_port() and rte_eth_dev_get_port_by_name()
>> using rte_eth_dev_data[port].name.
>>
>>> This patch updates PMDs to use only rte_device->name.
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>

Re: [PATCH v2 3/3] ethdev: use device name from device structure

[Thomas Monjalon]

09/06/2017 20:36, Ferruh Yigit:
> Device name resides in two different locations, in rte_device->name and
> in ethernet device private data.
> 
> For now, the copy in the ethernet device private data is required for
> multi process support, the name is the how secondary process finds about
> primary process device.
> 
> But in the ethdev library some eth_dev->data->name usage can be
> converted to rte_device->name.
> 
> This patch updates ethdev to use rte_device->name when possible.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Series applied, thanks