[dpdk-dev,v7,6/6] devargs: parse bus info

Message ID cd5734d73c2a4908523b88ae5e04b7eec4aa9928.1499211317.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

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

Commit Message

Gaëtan Rivet July 4, 2017, 11:55 p.m. UTC
  Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_devargs.c  | 42 ++++++++++++++++++++++++-----
 lib/librte_eal/common/eal_common_vdev.c     |  6 +++--
 lib/librte_eal/common/include/rte_devargs.h |  3 +++
 3 files changed, 42 insertions(+), 9 deletions(-)
  

Comments

Stephen Hemminger July 5, 2017, 6:03 p.m. UTC | #1
On Wed,  5 Jul 2017 01:55:23 +0200
Gaetan Rivet <gaetan.rivet@6wind.com> wrote:

>  	case RTE_DEVTYPE_WHITELISTED_PCI:
>  	case RTE_DEVTYPE_BLACKLISTED_PCI:
>  		/* try to parse pci identifier */
> -		if (eal_parse_pci_BDF(buf, &devargs->pci.addr) != 0 &&
> -		    eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0)
> +		if (bus->parse(devname, &devargs->pci.addr) != 0)
>  			goto fail;

Shouldn't these go under bus args for PCI?
It would be good to get all the bus specific args out of generic code.
  
Shreyansh Jain July 6, 2017, 9:07 a.m. UTC | #2
Hello Gaetan,

On Wednesday 05 July 2017 05:25 AM, Gaetan Rivet wrote:
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>   lib/librte_eal/common/eal_common_devargs.c  | 42 ++++++++++++++++++++++++-----
>   lib/librte_eal/common/eal_common_vdev.c     |  6 +++--
>   lib/librte_eal/common/include/rte_devargs.h |  3 +++
>   3 files changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index ffa8ad9..102bd8d 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,
>   	return 0;
>   }
>   
> +static int
> +bus_name_cmp(const struct rte_bus *bus, const void *_name)
> +{
> +	const char *name = _name;
> +
> +	return strncmp(bus->name, name,
> +		       strlen(bus->name));

Trivial: Any specific reason why this is split across multiple lines 
even though it is less than 80 chars in total?

> +}
> +
>   /* store a whitelist parameter for later parsing */
>   int
>   rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>   {
>   	struct rte_devargs *devargs = NULL;
> -	char *buf = NULL;
> +	struct rte_bus *bus = NULL;
> +	char *dev = NULL;
> +	const char *devname;
>   	int ret;
>   
>   	/* use malloc instead of rte_malloc as it's called early at init */
> @@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>   	memset(devargs, 0, sizeof(*devargs));
>   	devargs->type = devtype;
>   
> -	if (rte_eal_parse_devargs_str(devargs_str, &buf, &devargs->args))
> +	if (rte_eal_parse_devargs_str(devargs_str, &dev, &devargs->args))

A very basic (and possibly incorrect) question:

here, for example "'net_pcap0,rx_pcap=input.pcap,tx_pcap=output.pcap'" 
was passed to application, which would mean;

dev = "net_pcap0" (name of the device)
devargs = "rx_pcap=input.pcap,tx_pcap=output.pcap"

>   		goto fail;
> +	devname = dev;
> +	do {
> +		bus = rte_bus_find(bus, bus_name_cmp, dev);
> +		if (bus == NULL)
> +			break;
> +		devname = dev + strlen(bus->name) + 1;

Assuming "vdev" bus:
                       "net_pcap0"
                             |
devname points here --------' (4+1) chars from start of "net_pcap0".
Is that the expectation?

Probably I am missing something here (maybe the input already has a bus 
name.)

Or, if this is not the case, then we will have to change the test 
application (test_devargs.c) which passes such strings to 
"rte_eal_devargs_add".

> +		if (rte_bus_find_by_device_name(devname) == bus)

Obviously, if my assumption is correct, this fails. Or, maybe I am 
completely off the mark.

> +			break;
> +		devname = dev;
> +	} while (1);
> +	if (bus == NULL) {
> +		bus = rte_bus_find_by_device_name(devname);
> +		if (bus == NULL) {
> +			fprintf(stderr, "ERROR: failed to parse bus info from device declaration\n");
> +			goto fail;
> +		}
> +	}
> +	devargs->bus = bus;
   >   	switch (devargs->type) {
>   	case RTE_DEVTYPE_WHITELISTED_PCI:
>   	case RTE_DEVTYPE_BLACKLISTED_PCI:
>   		/* try to parse pci identifier */
> -		if (eal_parse_pci_BDF(buf, &devargs->pci.addr) != 0 &&
> -		    eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0)
> +		if (bus->parse(devname, &devargs->pci.addr) != 0)
>   			goto fail;
>   
>   		break;
>   	case RTE_DEVTYPE_VIRTUAL:
>   		/* save driver name */
>   		ret = snprintf(devargs->virt.drv_name,
> -			       sizeof(devargs->virt.drv_name), "%s", buf);
> +			       sizeof(devargs->virt.drv_name), "%s", devname);
>   		if (ret < 0 || ret >= (int)sizeof(devargs->virt.drv_name))
>   			goto fail;
>   
>   		break;
>   	}

I think all this will change in the devargs series.

>   
> -	free(buf);
> +	free(dev);
>   	TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
>   	return 0;
>   
>   fail:
> -	free(buf);
> +	free(dev);
>   	if (devargs) {
>   		free(devargs->args);
>   		free(devargs);
> diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
> index 6ecd1b5..8fd1ef7 100644
> --- a/lib/librte_eal/common/eal_common_vdev.c
> +++ b/lib/librte_eal/common/eal_common_vdev.c
> @@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)
>   		return NULL;
>   
>   	devargs->type = RTE_DEVTYPE_VIRTUAL;
> +	devargs->bus = rte_bus_find_by_name("vdev");
>   	if (args)
>   		devargs->args = strdup(args);
>   
> @@ -289,12 +290,13 @@ vdev_scan(void)
>   {
>   	struct rte_vdev_device *dev;
>   	struct rte_devargs *devargs;
> +	struct rte_bus *vbus;
>   
>   	/* for virtual devices we scan the devargs_list populated via cmdline */
> -
> +	vbus = rte_bus_find_by_name("vdev");
>   	TAILQ_FOREACH(devargs, &devargs_list, next) {
>   
> -		if (devargs->type != RTE_DEVTYPE_VIRTUAL)
> +		if (devargs->bus != vbus)
>   			continue;
>   
>   		dev = find_vdev(devargs->virt.drv_name);
> diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> index 88120a1..1f50a24 100644
> --- a/lib/librte_eal/common/include/rte_devargs.h
> +++ b/lib/librte_eal/common/include/rte_devargs.h
> @@ -51,6 +51,7 @@ extern "C" {
>   #include <stdio.h>
>   #include <sys/queue.h>
>   #include <rte_pci.h>
> +#include <rte_bus.h>
>   
>   /**
>    * Type of generic device
> @@ -89,6 +90,8 @@ struct rte_devargs {
>   			char drv_name[32];
>   		} virt;
>   	};
> +	/** Bus handle for the device. */
> +	struct rte_bus *bus;
>   	/** Arguments string as given by user or "" for no argument. */
>   	char *args;
>   };
> 
-
Shreyansh
  
Gaëtan Rivet July 6, 2017, 10 a.m. UTC | #3
On Thu, Jul 06, 2017 at 02:37:16PM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
> 
> On Wednesday 05 July 2017 05:25 AM, Gaetan Rivet wrote:
> >Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> >---
> >  lib/librte_eal/common/eal_common_devargs.c  | 42 ++++++++++++++++++++++++-----
> >  lib/librte_eal/common/eal_common_vdev.c     |  6 +++--
> >  lib/librte_eal/common/include/rte_devargs.h |  3 +++
> >  3 files changed, 42 insertions(+), 9 deletions(-)
> >
> >diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> >index ffa8ad9..102bd8d 100644
> >--- a/lib/librte_eal/common/eal_common_devargs.c
> >+++ b/lib/librte_eal/common/eal_common_devargs.c
> >@@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,
> >  	return 0;
> >  }
> >+static int
> >+bus_name_cmp(const struct rte_bus *bus, const void *_name)
> >+{
> >+	const char *name = _name;
> >+
> >+	return strncmp(bus->name, name,
> >+		       strlen(bus->name));
> 
> Trivial: Any specific reason why this is split across multiple lines even
> though it is less than 80 chars in total?
> 

Not really, it's only a matter of taste.
If is mostly to hightlight that we are limiting the comparison to
bus->name length, by putting it on its own line.

> >+}
> >+
> >  /* store a whitelist parameter for later parsing */
> >  int
> >  rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
> >  {
> >  	struct rte_devargs *devargs = NULL;
> >-	char *buf = NULL;
> >+	struct rte_bus *bus = NULL;
> >+	char *dev = NULL;
> >+	const char *devname;
> >  	int ret;
> >  	/* use malloc instead of rte_malloc as it's called early at init */
> >@@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
> >  	memset(devargs, 0, sizeof(*devargs));
> >  	devargs->type = devtype;
> >-	if (rte_eal_parse_devargs_str(devargs_str, &buf, &devargs->args))
> >+	if (rte_eal_parse_devargs_str(devargs_str, &dev, &devargs->args))
> 
> A very basic (and possibly incorrect) question:
> 
> here, for example "'net_pcap0,rx_pcap=input.pcap,tx_pcap=output.pcap'" was
> passed to application, which would mean;
> 
> dev = "net_pcap0" (name of the device)
> devargs = "rx_pcap=input.pcap,tx_pcap=output.pcap"
> 
> >  		goto fail;
> >+	devname = dev;
> >+	do {
> >+		bus = rte_bus_find(bus, bus_name_cmp, dev);
> >+		if (bus == NULL)
> >+			break;
> >+		devname = dev + strlen(bus->name) + 1;
> 
> Assuming "vdev" bus:
>                       "net_pcap0"
>                             |
> devname points here --------' (4+1) chars from start of "net_pcap0".
> Is that the expectation?
> 

Yes :)
Well, actually, almost. The lines

> >+		bus = rte_bus_find(bus, bus_name_cmp, dev);
> >+		if (bus == NULL)
> >+			break;

Mean that at this point, we would already have broken out of the loop.
But to continue with your example, assuming that we have a bus that
is named "net_p":

> Probably I am missing something here (maybe the input already has a bus
> name.)
> 
> Or, if this is not the case, then we will have to change the test
> application (test_devargs.c) which passes such strings to
> "rte_eal_devargs_add".
> 
> >+		if (rte_bus_find_by_device_name(devname) == bus)
> 
> Obviously, if my assumption is correct, this fails. Or, maybe I am
> completely off the mark.
> 

It should not be necessary to update the tests (yet).
This bit of code tries to recognize bus names at the start of the
declaration. However, some device names might be ambiguous and start
like bus names for any reason. Device names have no specification beside
not having commas within, bus names have no specification at all.

Thus, we first try here to recognize a bus name within dev, but we do
not stop here. We also verify afterward that the resulting device would
be correct, and that its bus handler is actually the same as the bus we
first recognized.

In your example, the line


> >+        if (rte_bus_find_by_device_name(devname) == bus)

Fails, as the device is incorrect and rte_bus_find_by_device_name
returns NULL as no bus is able to handle a device named

> "cap0,rx_pcap=input.pcap,tx_pcap=output.pcap"

Considering this, we should break from this loop with no recognized bus.
As such, we enter afterward in the branch:

> >+			break;
> >+		devname = dev;

Note here that devname is set back to the start of dev.

> >+	} while (1);
> >+	if (bus == NULL) {
> >+		bus = rte_bus_find_by_device_name(devname);

Which will try to recognize a bus from the device name ("net_pcap0"),
which should succeed in recognizing vdev.

It is a little contrived, but I wanted this parsing to both be flexible
and perform as many checks as possible.

I am developing a new bus that have a high probability of name conflicts
with other device names, so I am extra careful on this side.

> >+		if (bus == NULL) {
> >+			fprintf(stderr, "ERROR: failed to parse bus info from device declaration\n");
> >+			goto fail;
> >+		}
> >+	}
> >+	devargs->bus = bus;
>   >   	switch (devargs->type) {
> >  	case RTE_DEVTYPE_WHITELISTED_PCI:
> >  	case RTE_DEVTYPE_BLACKLISTED_PCI:
> >  		/* try to parse pci identifier */
> >-		if (eal_parse_pci_BDF(buf, &devargs->pci.addr) != 0 &&
> >-		    eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0)
> >+		if (bus->parse(devname, &devargs->pci.addr) != 0)
> >  			goto fail;
> >  		break;
> >  	case RTE_DEVTYPE_VIRTUAL:
> >  		/* save driver name */
> >  		ret = snprintf(devargs->virt.drv_name,
> >-			       sizeof(devargs->virt.drv_name), "%s", buf);
> >+			       sizeof(devargs->virt.drv_name), "%s", devname);
> >  		if (ret < 0 || ret >= (int)sizeof(devargs->virt.drv_name))
> >  			goto fail;
> >  		break;
> >  	}
> 
> I think all this will change in the devargs series.
> 

Indeed :)

> >-	free(buf);
> >+	free(dev);
> >  	TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
> >  	return 0;
> >  fail:
> >-	free(buf);
> >+	free(dev);
> >  	if (devargs) {
> >  		free(devargs->args);
> >  		free(devargs);
> >diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
> >index 6ecd1b5..8fd1ef7 100644
> >--- a/lib/librte_eal/common/eal_common_vdev.c
> >+++ b/lib/librte_eal/common/eal_common_vdev.c
> >@@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)
> >  		return NULL;
> >  	devargs->type = RTE_DEVTYPE_VIRTUAL;
> >+	devargs->bus = rte_bus_find_by_name("vdev");
> >  	if (args)
> >  		devargs->args = strdup(args);
> >@@ -289,12 +290,13 @@ vdev_scan(void)
> >  {
> >  	struct rte_vdev_device *dev;
> >  	struct rte_devargs *devargs;
> >+	struct rte_bus *vbus;
> >  	/* for virtual devices we scan the devargs_list populated via cmdline */
> >-
> >+	vbus = rte_bus_find_by_name("vdev");
> >  	TAILQ_FOREACH(devargs, &devargs_list, next) {
> >-		if (devargs->type != RTE_DEVTYPE_VIRTUAL)
> >+		if (devargs->bus != vbus)
> >  			continue;
> >  		dev = find_vdev(devargs->virt.drv_name);
> >diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> >index 88120a1..1f50a24 100644
> >--- a/lib/librte_eal/common/include/rte_devargs.h
> >+++ b/lib/librte_eal/common/include/rte_devargs.h
> >@@ -51,6 +51,7 @@ extern "C" {
> >  #include <stdio.h>
> >  #include <sys/queue.h>
> >  #include <rte_pci.h>
> >+#include <rte_bus.h>
> >  /**
> >   * Type of generic device
> >@@ -89,6 +90,8 @@ struct rte_devargs {
> >  			char drv_name[32];
> >  		} virt;
> >  	};
> >+	/** Bus handle for the device. */
> >+	struct rte_bus *bus;
> >  	/** Arguments string as given by user or "" for no argument. */
> >  	char *args;
> >  };
> >
> -
> Shreyansh
  
Shreyansh Jain July 6, 2017, 1:17 p.m. UTC | #4
Hello Gaetan,

On Thursday 06 July 2017 03:30 PM, Gaëtan Rivet wrote:
> On Thu, Jul 06, 2017 at 02:37:16PM +0530, Shreyansh Jain wrote:
>> Hello Gaetan,
>>
>> On Wednesday 05 July 2017 05:25 AM, Gaetan Rivet wrote:
>>> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>>> ---
>>>   lib/librte_eal/common/eal_common_devargs.c  | 42 ++++++++++++++++++++++++-----
>>>   lib/librte_eal/common/eal_common_vdev.c     |  6 +++--
>>>   lib/librte_eal/common/include/rte_devargs.h |  3 +++
>>>   3 files changed, 42 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
>>> index ffa8ad9..102bd8d 100644
>>> --- a/lib/librte_eal/common/eal_common_devargs.c
>>> +++ b/lib/librte_eal/common/eal_common_devargs.c
>>> @@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,
>>>   	return 0;
>>>   }
>>> +static int
>>> +bus_name_cmp(const struct rte_bus *bus, const void *_name)
>>> +{
>>> +	const char *name = _name;
>>> +
>>> +	return strncmp(bus->name, name,
>>> +		       strlen(bus->name));
>>
>> Trivial: Any specific reason why this is split across multiple lines even
>> though it is less than 80 chars in total?
>>
> 
> Not really, it's only a matter of taste.
> If is mostly to hightlight that we are limiting the comparison to
> bus->name length, by putting it on its own line.

Ok. I am ok with this.

> 
>>> +}
>>> +
>>>   /* store a whitelist parameter for later parsing */
>>>   int
>>>   rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>>>   {
>>>   	struct rte_devargs *devargs = NULL;
>>> -	char *buf = NULL;
>>> +	struct rte_bus *bus = NULL;
>>> +	char *dev = NULL;
>>> +	const char *devname;
>>>   	int ret;
>>>   	/* use malloc instead of rte_malloc as it's called early at init */
>>> @@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>>>   	memset(devargs, 0, sizeof(*devargs));
>>>   	devargs->type = devtype;
>>> -	if (rte_eal_parse_devargs_str(devargs_str, &buf, &devargs->args))
>>> +	if (rte_eal_parse_devargs_str(devargs_str, &dev, &devargs->args))
>>
>> A very basic (and possibly incorrect) question:
>>
>> here, for example "'net_pcap0,rx_pcap=input.pcap,tx_pcap=output.pcap'" was
>> passed to application, which would mean;
>>
>> dev = "net_pcap0" (name of the device)
>> devargs = "rx_pcap=input.pcap,tx_pcap=output.pcap"
>>
>>>   		goto fail;
>>> +	devname = dev;
>>> +	do {
>>> +		bus = rte_bus_find(bus, bus_name_cmp, dev);
>>> +		if (bus == NULL)
>>> +			break;
>>> +		devname = dev + strlen(bus->name) + 1;
>>
>> Assuming "vdev" bus:
>>                        "net_pcap0"
>>                              |
>> devname points here --------' (4+1) chars from start of "net_pcap0".
>> Is that the expectation?
>>
> 
> Yes :)
> Well, actually, almost. The lines
> 
>>> +		bus = rte_bus_find(bus, bus_name_cmp, dev);
>>> +		if (bus == NULL)
>>> +			break;
> 
> Mean that at this point, we would already have broken out of the loop.
> But to continue with your example, assuming that we have a bus that
> is named "net_p":
> 
>> Probably I am missing something here (maybe the input already has a bus
>> name.)
>>
>> Or, if this is not the case, then we will have to change the test
>> application (test_devargs.c) which passes such strings to
>> "rte_eal_devargs_add".
>>
>>> +		if (rte_bus_find_by_device_name(devname) == bus)
>>
>> Obviously, if my assumption is correct, this fails. Or, maybe I am
>> completely off the mark.
>>
> 
> It should not be necessary to update the tests (yet).
> This bit of code tries to recognize bus names at the start of the
> declaration. However, some device names might be ambiguous and start
> like bus names for any reason. Device names have no specification beside
> not having commas within, bus names have no specification at all.
> 
> Thus, we first try here to recognize a bus name within dev, but we do
> not stop here. We also verify afterward that the resulting device would
> be correct, and that its bus handler is actually the same as the bus we
> first recognized.
> 
> In your example, the line
> 
> 
>>> +        if (rte_bus_find_by_device_name(devname) == bus)
> 
> Fails, as the device is incorrect and rte_bus_find_by_device_name
> returns NULL as no bus is able to handle a device named
> 
>> "cap0,rx_pcap=input.pcap,tx_pcap=output.pcap"
> 
> Considering this, we should break from this loop with no recognized bus.
> As such, we enter afterward in the branch:
> 
>>> +			break;
>>> +		devname = dev;
> 
> Note here that devname is set back to the start of dev.
> 
>>> +	} while (1);
>>> +	if (bus == NULL) {
>>> +		bus = rte_bus_find_by_device_name(devname);
> 
> Which will try to recognize a bus from the device name ("net_pcap0"),
> which should succeed in recognizing vdev.
> 
> It is a little contrived, but I wanted this parsing to both be flexible
> and perform as many checks as possible.
> 
> I am developing a new bus that have a high probability of name conflicts
> with other device names, so I am extra careful on this side.

Ok. I completely missed this logic.
Thanks.

> 
>>> +		if (bus == NULL) {
>>> +			fprintf(stderr, "ERROR: failed to parse bus info from device declaration\n");
>>> +			goto fail;
>>> +		}
>>> +	}
>>> +	devargs->bus = bus;
>>    >   	switch (devargs->type) {
>>>   	case RTE_DEVTYPE_WHITELISTED_PCI:
>>>   	case RTE_DEVTYPE_BLACKLISTED_PCI:
>>>   		/* try to parse pci identifier */
>>> -		if (eal_parse_pci_BDF(buf, &devargs->pci.addr) != 0 &&
>>> -		    eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0)
>>> +		if (bus->parse(devname, &devargs->pci.addr) != 0)
>>>   			goto fail;
>>>   		break;
>>>   	case RTE_DEVTYPE_VIRTUAL:
>>>   		/* save driver name */
>>>   		ret = snprintf(devargs->virt.drv_name,
>>> -			       sizeof(devargs->virt.drv_name), "%s", buf);
>>> +			       sizeof(devargs->virt.drv_name), "%s", devname);
>>>   		if (ret < 0 || ret >= (int)sizeof(devargs->virt.drv_name))
>>>   			goto fail;
>>>   		break;
>>>   	}
>>
>> I think all this will change in the devargs series.
>>
> 
> Indeed :)
> 
>>> -	free(buf);
>>> +	free(dev);
>>>   	TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
>>>   	return 0;
>>>   fail:
>>> -	free(buf);
>>> +	free(dev);
>>>   	if (devargs) {
>>>   		free(devargs->args);
>>>   		free(devargs);
>>> diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
>>> index 6ecd1b5..8fd1ef7 100644
>>> --- a/lib/librte_eal/common/eal_common_vdev.c
>>> +++ b/lib/librte_eal/common/eal_common_vdev.c
>>> @@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)
>>>   		return NULL;
>>>   	devargs->type = RTE_DEVTYPE_VIRTUAL;
>>> +	devargs->bus = rte_bus_find_by_name("vdev");
>>>   	if (args)
>>>   		devargs->args = strdup(args);
>>> @@ -289,12 +290,13 @@ vdev_scan(void)
>>>   {
>>>   	struct rte_vdev_device *dev;
>>>   	struct rte_devargs *devargs;
>>> +	struct rte_bus *vbus;
>>>   	/* for virtual devices we scan the devargs_list populated via cmdline */
>>> -
>>> +	vbus = rte_bus_find_by_name("vdev");
>>>   	TAILQ_FOREACH(devargs, &devargs_list, next) {
>>> -		if (devargs->type != RTE_DEVTYPE_VIRTUAL)
>>> +		if (devargs->bus != vbus)
>>>   			continue;
>>>   		dev = find_vdev(devargs->virt.drv_name);
>>> diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
>>> index 88120a1..1f50a24 100644
>>> --- a/lib/librte_eal/common/include/rte_devargs.h
>>> +++ b/lib/librte_eal/common/include/rte_devargs.h
>>> @@ -51,6 +51,7 @@ extern "C" {
>>>   #include <stdio.h>
>>>   #include <sys/queue.h>
>>>   #include <rte_pci.h>
>>> +#include <rte_bus.h>
>>>   /**
>>>    * Type of generic device
>>> @@ -89,6 +90,8 @@ struct rte_devargs {
>>>   			char drv_name[32];
>>>   		} virt;
>>>   	};
>>> +	/** Bus handle for the device. */
>>> +	struct rte_bus *bus;
>>>   	/** Arguments string as given by user or "" for no argument. */
>>>   	char *args;
>>>   };
>>>
>> -
>> Shreyansh
> 

And, for this patch as well:

Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
  

Patch

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index ffa8ad9..102bd8d 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -78,12 +78,23 @@  rte_eal_parse_devargs_str(const char *devargs_str,
 	return 0;
 }
 
+static int
+bus_name_cmp(const struct rte_bus *bus, const void *_name)
+{
+	const char *name = _name;
+
+	return strncmp(bus->name, name,
+		       strlen(bus->name));
+}
+
 /* store a whitelist parameter for later parsing */
 int
 rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 {
 	struct rte_devargs *devargs = NULL;
-	char *buf = NULL;
+	struct rte_bus *bus = NULL;
+	char *dev = NULL;
+	const char *devname;
 	int ret;
 
 	/* use malloc instead of rte_malloc as it's called early at init */
@@ -94,34 +105,51 @@  rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 	memset(devargs, 0, sizeof(*devargs));
 	devargs->type = devtype;
 
-	if (rte_eal_parse_devargs_str(devargs_str, &buf, &devargs->args))
+	if (rte_eal_parse_devargs_str(devargs_str, &dev, &devargs->args))
 		goto fail;
+	devname = dev;
+	do {
+		bus = rte_bus_find(bus, bus_name_cmp, dev);
+		if (bus == NULL)
+			break;
+		devname = dev + strlen(bus->name) + 1;
+		if (rte_bus_find_by_device_name(devname) == bus)
+			break;
+		devname = dev;
+	} while (1);
+	if (bus == NULL) {
+		bus = rte_bus_find_by_device_name(devname);
+		if (bus == NULL) {
+			fprintf(stderr, "ERROR: failed to parse bus info from device declaration\n");
+			goto fail;
+		}
+	}
+	devargs->bus = bus;
 
 	switch (devargs->type) {
 	case RTE_DEVTYPE_WHITELISTED_PCI:
 	case RTE_DEVTYPE_BLACKLISTED_PCI:
 		/* try to parse pci identifier */
-		if (eal_parse_pci_BDF(buf, &devargs->pci.addr) != 0 &&
-		    eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0)
+		if (bus->parse(devname, &devargs->pci.addr) != 0)
 			goto fail;
 
 		break;
 	case RTE_DEVTYPE_VIRTUAL:
 		/* save driver name */
 		ret = snprintf(devargs->virt.drv_name,
-			       sizeof(devargs->virt.drv_name), "%s", buf);
+			       sizeof(devargs->virt.drv_name), "%s", devname);
 		if (ret < 0 || ret >= (int)sizeof(devargs->virt.drv_name))
 			goto fail;
 
 		break;
 	}
 
-	free(buf);
+	free(dev);
 	TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
 	return 0;
 
 fail:
-	free(buf);
+	free(dev);
 	if (devargs) {
 		free(devargs->args);
 		free(devargs);
diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 6ecd1b5..8fd1ef7 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -177,6 +177,7 @@  alloc_devargs(const char *name, const char *args)
 		return NULL;
 
 	devargs->type = RTE_DEVTYPE_VIRTUAL;
+	devargs->bus = rte_bus_find_by_name("vdev");
 	if (args)
 		devargs->args = strdup(args);
 
@@ -289,12 +290,13 @@  vdev_scan(void)
 {
 	struct rte_vdev_device *dev;
 	struct rte_devargs *devargs;
+	struct rte_bus *vbus;
 
 	/* for virtual devices we scan the devargs_list populated via cmdline */
-
+	vbus = rte_bus_find_by_name("vdev");
 	TAILQ_FOREACH(devargs, &devargs_list, next) {
 
-		if (devargs->type != RTE_DEVTYPE_VIRTUAL)
+		if (devargs->bus != vbus)
 			continue;
 
 		dev = find_vdev(devargs->virt.drv_name);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 88120a1..1f50a24 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -51,6 +51,7 @@  extern "C" {
 #include <stdio.h>
 #include <sys/queue.h>
 #include <rte_pci.h>
+#include <rte_bus.h>
 
 /**
  * Type of generic device
@@ -89,6 +90,8 @@  struct rte_devargs {
 			char drv_name[32];
 		} virt;
 	};
+	/** Bus handle for the device. */
+	struct rte_bus *bus;
 	/** Arguments string as given by user or "" for no argument. */
 	char *args;
 };