[dpdk-dev] ethdev: revert use device name from device structure

Message ID 20170922113007.43385-1-ferruh.yigit@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit Sept. 22, 2017, 11:30 a.m. UTC
  This reverts commit a1e7c17555e8f77d520ba5f06ed26c00e77a2bd1.

Original commit assumes there is 1:1 mapping between physical device and
ethdev port, so that device name can be used per port instead of ethdev
name field.

But one physical device may have multiple ethdev ports and each port
needs its own unique name.

One issue reported here:
http://dpdk.org/ml/archives/users/2017-September/002484.html

So reverting back the commit to continue using ethdev name field per
port.

Reported-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 lib/librte_ether/rte_ethdev.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)
  

Comments

Adrien Mazarguil Sept. 22, 2017, 2:57 p.m. UTC | #1
On Fri, Sep 22, 2017 at 12:30:07PM +0100, Ferruh Yigit wrote:
> This reverts commit a1e7c17555e8f77d520ba5f06ed26c00e77a2bd1.
> 
> Original commit assumes there is 1:1 mapping between physical device and
> ethdev port, so that device name can be used per port instead of ethdev
> name field.
> 
> But one physical device may have multiple ethdev ports and each port
> needs its own unique name.
> 
> One issue reported here:
> http://dpdk.org/ml/archives/users/2017-September/002484.html
> 
> So reverting back the commit to continue using ethdev name field per
> port.
> 
> Reported-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
  
Thomas Monjalon Oct. 11, 2017, 9:56 p.m. UTC | #2
22/09/2017 16:57, Adrien Mazarguil:
> On Fri, Sep 22, 2017 at 12:30:07PM +0100, Ferruh Yigit wrote:
> > This reverts commit a1e7c17555e8f77d520ba5f06ed26c00e77a2bd1.
> > 
> > Original commit assumes there is 1:1 mapping between physical device and
> > ethdev port, so that device name can be used per port instead of ethdev
> > name field.
> > 
> > But one physical device may have multiple ethdev ports and each port
> > needs its own unique name.
> > 
> > One issue reported here:
> > http://dpdk.org/ml/archives/users/2017-September/002484.html
> > 
> > So reverting back the commit to continue using ethdev name field per
> > port.
> > 
> > Reported-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Acked-by: Thomas Monjalon <thomas@monjalon.net>

I agree we must keep separate the device name and the ethdev port name.

About the OVS issue (Cc'ing people), it would be valuable to work
in DPDK to establish a reliable syntax to describe a device
or a port. This is what we call devargs ; it is still a work in progress.
  
Ferruh Yigit Oct. 11, 2017, 10:44 p.m. UTC | #3
On 10/11/2017 10:56 PM, Thomas Monjalon wrote:
> 22/09/2017 16:57, Adrien Mazarguil:
>> On Fri, Sep 22, 2017 at 12:30:07PM +0100, Ferruh Yigit wrote:
>>> This reverts commit a1e7c17555e8f77d520ba5f06ed26c00e77a2bd1.
>>>
>>> Original commit assumes there is 1:1 mapping between physical device and
>>> ethdev port, so that device name can be used per port instead of ethdev
>>> name field.
>>>
>>> But one physical device may have multiple ethdev ports and each port
>>> needs its own unique name.
>>>
>>> One issue reported here:
>>> http://dpdk.org/ml/archives/users/2017-September/002484.html
>>>
>>> So reverting back the commit to continue using ethdev name field per
>>> port.
>>>
>>> Reported-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 6016fd9be..c944b9fb1 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -177,11 +177,9 @@  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 &&
-				rte_eth_devices[i].device) {
-			if (!strcmp(rte_eth_devices[i].device->name, name))
-				return &rte_eth_devices[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];
 	}
 	return NULL;
 }
@@ -318,7 +316,7 @@  rte_eth_dev_count(void)
 int
 rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
 {
-	const char *tmp;
+	char *tmp;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
@@ -329,7 +327,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_devices[port_id].device->name;
+	tmp = rte_eth_dev_data[port_id].name;
 	strcpy(name, tmp);
 	return 0;
 }
@@ -337,7 +335,6 @@  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) {
@@ -346,13 +343,11 @@  rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id)
 	}
 
 	RTE_ETH_FOREACH_DEV(i) {
-		if (!rte_eth_devices[i].device)
-			continue;
+		if (!strncmp(name,
+			rte_eth_dev_data[i].name, strlen(name))) {
 
-		ret = strncmp(name, rte_eth_devices[i].device->name,
-				strlen(name));
-		if (ret == 0) {
 			*port_id = i;
+
 			return 0;
 		}
 	}
@@ -435,8 +430,8 @@  rte_eth_dev_detach(uint8_t port_id, char *name)
 	if (rte_eth_dev_is_detachable(port_id))
 		goto err;
 
-	snprintf(name, RTE_DEV_NAME_MAX_LEN, "%s",
-		 rte_eth_devices[port_id].device->name);
+	snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
+		 "%s", rte_eth_devices[port_id].data->name);
 
 	ret = rte_eal_dev_detach(rte_eth_devices[port_id].device);
 	if (ret < 0)