[dpdk-dev,v6,3/8] pci: split match and probe function

Message ID 1484581107-2025-4-git-send-email-shreyansh.jain@nxp.com
State Superseded, archived
Headers show

Checks

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

Commit Message

Shreyansh Jain Jan. 16, 2017, 3:38 p.m.
Matching of PCI device address and driver ID table is being done at two
discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
and rte_eal_pci_detach_dev).

Splitting the matching function into a public fn rte_pci_match.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   1 +
 lib/librte_eal/common/eal_common_pci.c          | 189 +++++++++++++-----------
 lib/librte_eal/common/include/rte_pci.h         |  15 ++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   1 +
 4 files changed, 121 insertions(+), 85 deletions(-)

Comments

Stephen Hemminger Jan. 16, 2017, 6:24 p.m. | #1
On Mon, 16 Jan 2017 21:08:22 +0530
Shreyansh Jain <shreyansh.jain@nxp.com> wrote:

> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
> +int
> +rte_pci_match(struct rte_pci_driver *pci_drv,
> +		  struct rte_pci_device *pci_dev)

Use const?
Ferruh Yigit Jan. 16, 2017, 7:53 p.m. | #2
On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
> Matching of PCI device address and driver ID table is being done at two
> discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
> and rte_eal_pci_detach_dev).
> 
> Splitting the matching function into a public fn rte_pci_match.
> 
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>

<...>

>  /*
> - * If vendor/device ID match, call the remove() function of the
> + * If vendor/device ID match, call the probe() function of the
>   * driver.
>   */
>  static int
> -rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
> -		struct rte_pci_device *dev)
> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
> +			     struct rte_pci_device *dev)
>  {
> -	const struct rte_pci_id *id_table;
> +	int ret;
> +	struct rte_pci_addr *loc;
>  
>  	if ((dr == NULL) || (dev == NULL))
>  		return -EINVAL;
>  
> -	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
> +	loc = &dev->addr;
>  
> -		/* check if device's identifiers match the driver's ones */
> -		if (id_table->vendor_id != dev->id.vendor_id &&
> -				id_table->vendor_id != PCI_ANY_ID)
> -			continue;
> -		if (id_table->device_id != dev->id.device_id &&
> -				id_table->device_id != PCI_ANY_ID)
> -			continue;
> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
> -			continue;
> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
> -				id_table->subsystem_device_id != PCI_ANY_ID)
> -			continue;
> +	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> +			loc->domain, loc->bus, loc->devid, loc->function,
> +			dev->device.numa_node);

This cause bunch of log printed during app startup, what about printing
this log when probed device found?

>  
> -		struct rte_pci_addr *loc = &dev->addr;
> +	/* The device is not blacklisted; Check if driver supports it */
> +	ret = rte_pci_match(dr, dev);
> +	if (ret) {
> +		/* Match of device and driver failed */
> +		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
> +			dr->driver.name);
> +		return 1;
> +	}
> +
> +	/* no initialization when blacklisted, return without error */
> +	if (dev->device.devargs != NULL &&
> +		dev->device.devargs->type ==
> +			RTE_DEVTYPE_BLACKLISTED_PCI) {
> +		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
> +			" initializing\n");
> +		return 1;
> +	}
>  
> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> -				loc->domain, loc->bus, loc->devid,
> -				loc->function, dev->device.numa_node);
> +	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
> +		dev->id.device_id, dr->driver.name);

Same for this one, this line cause printing all registered drivers for
each device during app initialization, only matched one can be logged.

>  
> -		RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
> -				dev->id.device_id, dr->driver.name);
> +	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> +		/* map resources for devices that use igb_uio */
> +		ret = rte_eal_pci_map_device(dev);
> +		if (ret != 0)
> +			return ret;
> +	}
>  
> -		if (dr->remove && (dr->remove(dev) < 0))
> -			return -1;	/* negative value is an error */
> +	/* reference driver structure */
> +	dev->driver = dr;
>  
> -		/* clear driver structure */
> +	/* call the driver probe() function */
> +	ret = dr->probe(dr, dev);
> +	if (ret) {
>  		dev->driver = NULL;
> -
>  		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> -			/* unmap resources for devices that use igb_uio */
>  			rte_eal_pci_unmap_device(dev);
> +	}
>  
> -		return 0;
> +	return ret;
> +}

<...>

> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index b553b13..5ed2589 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -186,5 +186,6 @@ DPDK_17.02 {
>  	rte_bus_dump;
>  	rte_bus_register;
>  	rte_bus_unregister;
> +	rte_pci_match;

I think this is internal API, should library expose this API?

>  
>  } DPDK_16.11;
>
Shreyansh Jain Jan. 17, 2017, 4:54 a.m. | #3
Hello Ferruh,

On Tuesday 17 January 2017 01:23 AM, Ferruh Yigit wrote:
> On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
>> Matching of PCI device address and driver ID table is being done at two
>> discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
>> and rte_eal_pci_detach_dev).
>>
>> Splitting the matching function into a public fn rte_pci_match.
>>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>
> <...>
>
>>  /*
>> - * If vendor/device ID match, call the remove() function of the
>> + * If vendor/device ID match, call the probe() function of the
>>   * driver.
>>   */
>>  static int
>> -rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
>> -		struct rte_pci_device *dev)
>> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
>> +			     struct rte_pci_device *dev)
>>  {
>> -	const struct rte_pci_id *id_table;
>> +	int ret;
>> +	struct rte_pci_addr *loc;
>>
>>  	if ((dr == NULL) || (dev == NULL))
>>  		return -EINVAL;
>>
>> -	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
>> +	loc = &dev->addr;
>>
>> -		/* check if device's identifiers match the driver's ones */
>> -		if (id_table->vendor_id != dev->id.vendor_id &&
>> -				id_table->vendor_id != PCI_ANY_ID)
>> -			continue;
>> -		if (id_table->device_id != dev->id.device_id &&
>> -				id_table->device_id != PCI_ANY_ID)
>> -			continue;
>> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
>> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
>> -			continue;
>> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
>> -				id_table->subsystem_device_id != PCI_ANY_ID)
>> -			continue;
>> +	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> +			loc->domain, loc->bus, loc->devid, loc->function,
>> +			dev->device.numa_node);
>
> This cause bunch of log printed during app startup, what about printing
> this log when probed device found?

Only thing I did was move around this log message without adding 
anything new. Maybe earlier it was in some conditional (match) and now 
it isn't. I will check again and move to match-only case.

>
>>
>> -		struct rte_pci_addr *loc = &dev->addr;
>> +	/* The device is not blacklisted; Check if driver supports it */
>> +	ret = rte_pci_match(dr, dev);
>> +	if (ret) {
>> +		/* Match of device and driver failed */
>> +		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
>> +			dr->driver.name);
>> +		return 1;
>> +	}
>> +
>> +	/* no initialization when blacklisted, return without error */
>> +	if (dev->device.devargs != NULL &&
>> +		dev->device.devargs->type ==
>> +			RTE_DEVTYPE_BLACKLISTED_PCI) {
>> +		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
>> +			" initializing\n");
>> +		return 1;
>> +	}
>>
>> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> -				loc->domain, loc->bus, loc->devid,
>> -				loc->function, dev->device.numa_node);
>> +	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> +		dev->id.device_id, dr->driver.name);
>
> Same for this one, this line cause printing all registered drivers for
> each device during app initialization, only matched one can be logged.

Same. Will post v7 shortly with only match case printing.
What about DEBUG for all cases?

>
>>
>> -		RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
>> -				dev->id.device_id, dr->driver.name);
>> +	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
>> +		/* map resources for devices that use igb_uio */
>> +		ret = rte_eal_pci_map_device(dev);
>> +		if (ret != 0)
>> +			return ret;
>> +	}
>>
>> -		if (dr->remove && (dr->remove(dev) < 0))
>> -			return -1;	/* negative value is an error */
>> +	/* reference driver structure */
>> +	dev->driver = dr;
>>
>> -		/* clear driver structure */
>> +	/* call the driver probe() function */
>> +	ret = dr->probe(dr, dev);
>> +	if (ret) {
>>  		dev->driver = NULL;
>> -
>>  		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
>> -			/* unmap resources for devices that use igb_uio */
>>  			rte_eal_pci_unmap_device(dev);
>> +	}
>>
>> -		return 0;
>> +	return ret;
>> +}
>
> <...>
>
>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> index b553b13..5ed2589 100644
>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> @@ -186,5 +186,6 @@ DPDK_17.02 {
>>  	rte_bus_dump;
>>  	rte_bus_register;
>>  	rte_bus_unregister;
>> +	rte_pci_match;
>
> I think this is internal API, should library expose this API?

Idea is that pci_match be useable outside of PCI for any other PCI-like 
bus (BDF compliant). For example, one of NXP's devices are very close to 
PCI (but not exactly PCI) and they too rely on BDF for addressing/naming.

>
>>
>>  } DPDK_16.11;
>>
>
>

-
Shreyansh
Ferruh Yigit Jan. 17, 2017, 9:58 a.m. | #4
On 1/17/2017 4:54 AM, Shreyansh Jain wrote:
> Hello Ferruh,
> 
> On Tuesday 17 January 2017 01:23 AM, Ferruh Yigit wrote:
>> On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
>>> Matching of PCI device address and driver ID table is being done at two
>>> discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
>>> and rte_eal_pci_detach_dev).
>>>
>>> Splitting the matching function into a public fn rte_pci_match.
>>>
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>
>> <...>
>>
>>>  /*
>>> - * If vendor/device ID match, call the remove() function of the
>>> + * If vendor/device ID match, call the probe() function of the
>>>   * driver.
>>>   */
>>>  static int
>>> -rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
>>> -		struct rte_pci_device *dev)
>>> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
>>> +			     struct rte_pci_device *dev)
>>>  {
>>> -	const struct rte_pci_id *id_table;
>>> +	int ret;
>>> +	struct rte_pci_addr *loc;
>>>
>>>  	if ((dr == NULL) || (dev == NULL))
>>>  		return -EINVAL;
>>>
>>> -	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
>>> +	loc = &dev->addr;
>>>
>>> -		/* check if device's identifiers match the driver's ones */
>>> -		if (id_table->vendor_id != dev->id.vendor_id &&
>>> -				id_table->vendor_id != PCI_ANY_ID)
>>> -			continue;
>>> -		if (id_table->device_id != dev->id.device_id &&
>>> -				id_table->device_id != PCI_ANY_ID)
>>> -			continue;
>>> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
>>> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
>>> -			continue;
>>> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
>>> -				id_table->subsystem_device_id != PCI_ANY_ID)
>>> -			continue;
>>> +	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>>> +			loc->domain, loc->bus, loc->devid, loc->function,
>>> +			dev->device.numa_node);
>>
>> This cause bunch of log printed during app startup, what about printing
>> this log when probed device found?
> 
> Only thing I did was move around this log message without adding 
> anything new. Maybe earlier it was in some conditional (match) and now 
> it isn't. I will check again and move to match-only case.
> 
>>
>>>
>>> -		struct rte_pci_addr *loc = &dev->addr;
>>> +	/* The device is not blacklisted; Check if driver supports it */
>>> +	ret = rte_pci_match(dr, dev);
>>> +	if (ret) {
>>> +		/* Match of device and driver failed */
>>> +		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
>>> +			dr->driver.name);
>>> +		return 1;
>>> +	}
>>> +
>>> +	/* no initialization when blacklisted, return without error */
>>> +	if (dev->device.devargs != NULL &&
>>> +		dev->device.devargs->type ==
>>> +			RTE_DEVTYPE_BLACKLISTED_PCI) {
>>> +		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
>>> +			" initializing\n");
>>> +		return 1;
>>> +	}
>>>
>>> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>>> -				loc->domain, loc->bus, loc->devid,
>>> -				loc->function, dev->device.numa_node);
>>> +	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>>> +		dev->id.device_id, dr->driver.name);
>>
>> Same for this one, this line cause printing all registered drivers for
>> each device during app initialization, only matched one can be logged.
> 
> Same. Will post v7 shortly with only match case printing.
> What about DEBUG for all cases?

I would prefer existing behavior, INFO level for successfully probed
device and driver, but no strong opinion.

> 
>>
>>>
>>> -		RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
>>> -				dev->id.device_id, dr->driver.name);
>>> +	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
>>> +		/* map resources for devices that use igb_uio */
>>> +		ret = rte_eal_pci_map_device(dev);
>>> +		if (ret != 0)
>>> +			return ret;
>>> +	}
>>>
>>> -		if (dr->remove && (dr->remove(dev) < 0))
>>> -			return -1;	/* negative value is an error */
>>> +	/* reference driver structure */
>>> +	dev->driver = dr;
>>>
>>> -		/* clear driver structure */
>>> +	/* call the driver probe() function */
>>> +	ret = dr->probe(dr, dev);
>>> +	if (ret) {
>>>  		dev->driver = NULL;
>>> -
>>>  		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
>>> -			/* unmap resources for devices that use igb_uio */
>>>  			rte_eal_pci_unmap_device(dev);
>>> +	}
>>>
>>> -		return 0;
>>> +	return ret;
>>> +}
>>
>> <...>
>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> index b553b13..5ed2589 100644
>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> @@ -186,5 +186,6 @@ DPDK_17.02 {
>>>  	rte_bus_dump;
>>>  	rte_bus_register;
>>>  	rte_bus_unregister;
>>> +	rte_pci_match;
>>
>> I think this is internal API, should library expose this API?
> 
> Idea is that pci_match be useable outside of PCI for any other PCI-like 
> bus (BDF compliant). For example, one of NXP's devices are very close to 
> PCI (but not exactly PCI) and they too rely on BDF for addressing/naming.

OK.

> 
>>
>>>
>>>  } DPDK_16.11;
>>>
>>
>>
> 
> -
> Shreyansh
>
Shreyansh Jain Jan. 17, 2017, 10:10 a.m. | #5
On Monday 16 January 2017 11:54 PM, Stephen Hemminger wrote:
> On Mon, 16 Jan 2017 21:08:22 +0530
> Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>
>> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
>> +int
>> +rte_pci_match(struct rte_pci_driver *pci_drv,
>> +		  struct rte_pci_device *pci_dev)
>
> Use const?
>

Done in v7. Thanks
Shreyansh Jain Jan. 17, 2017, 10:14 a.m. | #6
On Tuesday 17 January 2017 03:28 PM, Ferruh Yigit wrote:
> On 1/17/2017 4:54 AM, Shreyansh Jain wrote:
>> Hello Ferruh,
>>
>> On Tuesday 17 January 2017 01:23 AM, Ferruh Yigit wrote:
>>> On 1/16/2017 3:38 PM, Shreyansh Jain wrote:
>>>> Matching of PCI device address and driver ID table is being done at two
>>>> discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
>>>> and rte_eal_pci_detach_dev).
>>>>
>>>> Splitting the matching function into a public fn rte_pci_match.
>>>>
>>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>>
>>> <...>
>>>
>>>>  /*
>>>> - * If vendor/device ID match, call the remove() function of the
>>>> + * If vendor/device ID match, call the probe() function of the
>>>>   * driver.
>>>>   */
>>>>  static int
>>>> -rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
>>>> -		struct rte_pci_device *dev)
>>>> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
>>>> +			     struct rte_pci_device *dev)
>>>>  {
>>>> -	const struct rte_pci_id *id_table;
>>>> +	int ret;
>>>> +	struct rte_pci_addr *loc;
>>>>
>>>>  	if ((dr == NULL) || (dev == NULL))
>>>>  		return -EINVAL;
>>>>
>>>> -	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
>>>> +	loc = &dev->addr;
>>>>
>>>> -		/* check if device's identifiers match the driver's ones */
>>>> -		if (id_table->vendor_id != dev->id.vendor_id &&
>>>> -				id_table->vendor_id != PCI_ANY_ID)
>>>> -			continue;
>>>> -		if (id_table->device_id != dev->id.device_id &&
>>>> -				id_table->device_id != PCI_ANY_ID)
>>>> -			continue;
>>>> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
>>>> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
>>>> -			continue;
>>>> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
>>>> -				id_table->subsystem_device_id != PCI_ANY_ID)
>>>> -			continue;
>>>> +	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>>>> +			loc->domain, loc->bus, loc->devid, loc->function,
>>>> +			dev->device.numa_node);
>>>
>>> This cause bunch of log printed during app startup, what about printing
>>> this log when probed device found?
>>
>> Only thing I did was move around this log message without adding
>> anything new. Maybe earlier it was in some conditional (match) and now
>> it isn't. I will check again and move to match-only case.
>>
>>>
>>>>
>>>> -		struct rte_pci_addr *loc = &dev->addr;
>>>> +	/* The device is not blacklisted; Check if driver supports it */
>>>> +	ret = rte_pci_match(dr, dev);
>>>> +	if (ret) {
>>>> +		/* Match of device and driver failed */
>>>> +		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
>>>> +			dr->driver.name);
>>>> +		return 1;
>>>> +	}
>>>> +
>>>> +	/* no initialization when blacklisted, return without error */
>>>> +	if (dev->device.devargs != NULL &&
>>>> +		dev->device.devargs->type ==
>>>> +			RTE_DEVTYPE_BLACKLISTED_PCI) {
>>>> +		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
>>>> +			" initializing\n");
>>>> +		return 1;
>>>> +	}
>>>>
>>>> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>>>> -				loc->domain, loc->bus, loc->devid,
>>>> -				loc->function, dev->device.numa_node);
>>>> +	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>>>> +		dev->id.device_id, dr->driver.name);
>>>
>>> Same for this one, this line cause printing all registered drivers for
>>> each device during app initialization, only matched one can be logged.
>>
>> Same. Will post v7 shortly with only match case printing.
>> What about DEBUG for all cases?
>
> I would prefer existing behavior, INFO level for successfully probed
> device and driver, but no strong opinion.

Reverted to existing behavior. It was a miss from my side. I had moved
this log message _before_ matching unlike the original code.
Pushed v7 to ML.

>
>>
>>>
>>>>
>>>> -		RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
>>>> -				dev->id.device_id, dr->driver.name);
>>>> +	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
>>>> +		/* map resources for devices that use igb_uio */
>>>> +		ret = rte_eal_pci_map_device(dev);
>>>> +		if (ret != 0)
>>>> +			return ret;
>>>> +	}
>>>>
>>>> -		if (dr->remove && (dr->remove(dev) < 0))
>>>> -			return -1;	/* negative value is an error */
>>>> +	/* reference driver structure */
>>>> +	dev->driver = dr;
>>>>
>>>> -		/* clear driver structure */
>>>> +	/* call the driver probe() function */
>>>> +	ret = dr->probe(dr, dev);
>>>> +	if (ret) {
>>>>  		dev->driver = NULL;
>>>> -
>>>>  		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
>>>> -			/* unmap resources for devices that use igb_uio */
>>>>  			rte_eal_pci_unmap_device(dev);
>>>> +	}
>>>>
>>>> -		return 0;
>>>> +	return ret;
>>>> +}
>>>
>>> <...>
>>>
>>>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> index b553b13..5ed2589 100644
>>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> @@ -186,5 +186,6 @@ DPDK_17.02 {
>>>>  	rte_bus_dump;
>>>>  	rte_bus_register;
>>>>  	rte_bus_unregister;
>>>> +	rte_pci_match;
>>>
>>> I think this is internal API, should library expose this API?
>>
>> Idea is that pci_match be useable outside of PCI for any other PCI-like
>> bus (BDF compliant). For example, one of NXP's devices are very close to
>> PCI (but not exactly PCI) and they too rely on BDF for addressing/naming.
>
> OK.
>
>>
>>>
>>>>
>>>>  } DPDK_16.11;
>>>>
>>>
>>>
>>
>> -
>> Shreyansh
>>
>
>

Patch

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 4dcf653..c015889 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -182,5 +182,6 @@  DPDK_17.02 {
 	rte_bus_dump;
 	rte_bus_register;
 	rte_bus_unregister;
+	rte_pci_match;
 
 } DPDK_16.11;
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 72547bd..0d799be 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -152,129 +152,148 @@  pci_unmap_resource(void *requested_addr, size_t size)
 				requested_addr);
 }
 
-/*
- * If vendor/device ID match, call the probe() function of the
- * driver.
- */
-static int
-rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
+int
+rte_pci_match(struct rte_pci_driver *pci_drv,
+		  struct rte_pci_device *pci_dev)
 {
-	int ret;
+	int match = 1;
 	const struct rte_pci_id *id_table;
 
-	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
+	if (!pci_drv || !pci_dev || !pci_drv->id_table) {
+		RTE_LOG(DEBUG, EAL, "Invalid PCI Driver object\n");
+		return -1;
+	}
 
+	for (id_table = pci_drv->id_table; id_table->vendor_id != 0;
+	     id_table++) {
 		/* check if device's identifiers match the driver's ones */
-		if (id_table->vendor_id != dev->id.vendor_id &&
+		if (id_table->vendor_id != pci_dev->id.vendor_id &&
 				id_table->vendor_id != PCI_ANY_ID)
 			continue;
-		if (id_table->device_id != dev->id.device_id &&
+		if (id_table->device_id != pci_dev->id.device_id &&
 				id_table->device_id != PCI_ANY_ID)
 			continue;
-		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
-				id_table->subsystem_vendor_id != PCI_ANY_ID)
+		if (id_table->subsystem_vendor_id !=
+		    pci_dev->id.subsystem_vendor_id &&
+		    id_table->subsystem_vendor_id != PCI_ANY_ID)
 			continue;
-		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
-				id_table->subsystem_device_id != PCI_ANY_ID)
+		if (id_table->subsystem_device_id !=
+		    pci_dev->id.subsystem_device_id &&
+		    id_table->subsystem_device_id != PCI_ANY_ID)
 			continue;
-		if (id_table->class_id != dev->id.class_id &&
+		if (id_table->class_id != pci_dev->id.class_id &&
 				id_table->class_id != RTE_CLASS_ANY_ID)
 			continue;
 
-		struct rte_pci_addr *loc = &dev->addr;
-
-		RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
-				loc->domain, loc->bus, loc->devid, loc->function,
-				dev->device.numa_node);
-
-		/* no initialization when blacklisted, return without error */
-		if (dev->device.devargs != NULL &&
-			dev->device.devargs->type ==
-				RTE_DEVTYPE_BLACKLISTED_PCI) {
-			RTE_LOG(INFO, EAL, "  Device is blacklisted, not initializing\n");
-			return 1;
-		}
-
-		RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->driver.name);
-
-		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
-			/* map resources for devices that use igb_uio */
-			ret = rte_eal_pci_map_device(dev);
-			if (ret != 0)
-				return ret;
-		}
-
-		/* reference driver structure */
-		dev->driver = dr;
-
-		/* call the driver probe() function */
-		ret = dr->probe(dr, dev);
-		if (ret) {
-			dev->driver = NULL;
-			if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
-				rte_eal_pci_unmap_device(dev);
-		}
-
-		return ret;
+		match = 0;
+		break;
 	}
-	/* return positive value if driver doesn't support this device */
-	return 1;
+
+	return match;
 }
 
 /*
- * If vendor/device ID match, call the remove() function of the
+ * If vendor/device ID match, call the probe() function of the
  * driver.
  */
 static int
-rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
-		struct rte_pci_device *dev)
+rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
+			     struct rte_pci_device *dev)
 {
-	const struct rte_pci_id *id_table;
+	int ret;
+	struct rte_pci_addr *loc;
 
 	if ((dr == NULL) || (dev == NULL))
 		return -EINVAL;
 
-	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
+	loc = &dev->addr;
 
-		/* check if device's identifiers match the driver's ones */
-		if (id_table->vendor_id != dev->id.vendor_id &&
-				id_table->vendor_id != PCI_ANY_ID)
-			continue;
-		if (id_table->device_id != dev->id.device_id &&
-				id_table->device_id != PCI_ANY_ID)
-			continue;
-		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
-				id_table->subsystem_vendor_id != PCI_ANY_ID)
-			continue;
-		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
-				id_table->subsystem_device_id != PCI_ANY_ID)
-			continue;
+	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			loc->domain, loc->bus, loc->devid, loc->function,
+			dev->device.numa_node);
 
-		struct rte_pci_addr *loc = &dev->addr;
+	/* The device is not blacklisted; Check if driver supports it */
+	ret = rte_pci_match(dr, dev);
+	if (ret) {
+		/* Match of device and driver failed */
+		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
+			dr->driver.name);
+		return 1;
+	}
+
+	/* no initialization when blacklisted, return without error */
+	if (dev->device.devargs != NULL &&
+		dev->device.devargs->type ==
+			RTE_DEVTYPE_BLACKLISTED_PCI) {
+		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
+			" initializing\n");
+		return 1;
+	}
 
-		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
-				loc->domain, loc->bus, loc->devid,
-				loc->function, dev->device.numa_node);
+	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
+		dev->id.device_id, dr->driver.name);
 
-		RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->driver.name);
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
+		/* map resources for devices that use igb_uio */
+		ret = rte_eal_pci_map_device(dev);
+		if (ret != 0)
+			return ret;
+	}
 
-		if (dr->remove && (dr->remove(dev) < 0))
-			return -1;	/* negative value is an error */
+	/* reference driver structure */
+	dev->driver = dr;
 
-		/* clear driver structure */
+	/* call the driver probe() function */
+	ret = dr->probe(dr, dev);
+	if (ret) {
 		dev->driver = NULL;
-
 		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
-			/* unmap resources for devices that use igb_uio */
 			rte_eal_pci_unmap_device(dev);
+	}
 
-		return 0;
+	return ret;
+}
+
+/*
+ * If vendor/device ID match, call the remove() function of the
+ * driver.
+ */
+static int
+rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
+		struct rte_pci_device *dev)
+{
+	int ret;
+	struct rte_pci_addr *loc;
+
+	if ((dr == NULL) || (dev == NULL))
+		return -EINVAL;
+
+	ret = rte_pci_match(dr, dev);
+	if (ret) {
+		/* Device and driver don't match */
+		return 1;
 	}
 
-	/* return positive value if driver doesn't support this device */
-	return 1;
+	loc = &dev->addr;
+
+	RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			loc->domain, loc->bus, loc->devid,
+			loc->function, dev->device.numa_node);
+
+	RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
+			dev->id.device_id, dr->driver.name);
+
+	if (dr->remove && (dr->remove(dev) < 0))
+		return -1;	/* negative value is an error */
+
+	/* clear driver structure */
+	dev->driver = NULL;
+
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+		/* unmap resources for devices that use igb_uio */
+		rte_eal_pci_unmap_device(dev);
+
+	return 0;
 }
 
 /*
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 8557e47..6c9ec39 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -371,6 +371,21 @@  rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
 int rte_eal_pci_scan(void);
 
 /**
+ * Match the PCI Driver and Device using the ID Table
+ *
+ * @param pci_drv
+ *	PCI driver from which ID table would be extracted
+ * @param pci_dev
+ *	PCI device to match against the driver
+ * @return
+ *	0 for successful match
+ *	!0 for unsuccessful match
+ */
+int
+rte_pci_match(struct rte_pci_driver *pci_drv,
+	      struct rte_pci_device *pci_dev);
+
+/**
  * Probe the PCI bus for registered drivers.
  *
  * Scan the content of the PCI bus, and call the probe() function for
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index b553b13..5ed2589 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -186,5 +186,6 @@  DPDK_17.02 {
 	rte_bus_dump;
 	rte_bus_register;
 	rte_bus_unregister;
+	rte_pci_match;
 
 } DPDK_16.11;