[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Tetsuya Mukawa]

On 2015/02/18 10:17, Thomas Monjalon wrote:
> 2015-02-17 19:26, Tetsuya Mukawa:
>> On 2015/02/17 18:23, Thomas Monjalon wrote:
>>> 2015-02-17 17:51, Tetsuya Mukawa:
 On 2015/02/17 10:48, Thomas Monjalon wrote:
> 2015-02-16 13:14, Tetsuya Mukawa:
>> +/* attach the new physical device, then store port_id of the device */
>> +static int
>> +rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id)
>> +{
>> +uint8_t new_port_id;
>> +struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
>> +
>> +if ((addr == NULL) || (port_id == NULL))
>> +goto err;
>> +
>> +/* save current port status */
>> +if (rte_eth_dev_save(devs, sizeof(devs)))
>> +goto err;
>> +/* re-construct pci_device_list */
>> +if (rte_eal_pci_scan())
>> +goto err;
>> +/* invoke probe func of the driver can handle the new device */
>> +if (rte_eal_pci_probe_one(addr))
>> +goto err;
> You should get the port_id from the previous function instead of 
> searching it.
 I agree this will beautify this code, but actually to do like above
 changes current DPDK code much more, and it will not be clear, and not
 beautiful.

 Could I explain it more.
 Problem is initialization sequence of virtual device and physical device
 are completely different.

 (1) Attaching a physical device case
 - To return port id, pci_invoke_all_drivers() needs to return port id.
 - It means "devinit" of "struct rte_pci_driver" needs to return port_id.
 - "devinit" will be rte_eth_dev_init(). But if the device is virtual,
 this function is not implemented.

 (2) Attaching virtual device case
 - To return port id from rte_eal_pci_probe_one(),
 pci_invoke_all_drivers() needs to return port id.
 - It means "init" of "struct rte_driver" needs to return port_id.
 - "init" will be implemented in PMD. But this function has different
 usage in physical device and virtual device.
 - Especially, In the case of physical device, "init" doesn't allocate
 eth device, so cannot return port id.

 As a result, to remove  rte_eth_dev_save() and
 rte_eth_dev_get_changed_port(), below different functions needs to
 return port id.
  - "devinit" of "struct rte_pci_driver".
  - "init" of "struct rte_driver".
>>> Yes, exactly,
>>> I think you shouldn't hesitate to improve existing EAL code.
>>> And I also think we should try to remove differences between virtual and
>>> pci devices.
>> I strongly agree with it. But I haven't investigated how to remove it so
>> far.
>> To be honest, I want to submit hotplug patches to DPDK-2.0.
>> Is above functionality needed to merge the hotplug patches?
>> I guess I will not be able to remove it by 23rd.
> Obviously, it would be better to have it in dpdk-2.0.0-rc1.
> If not possible to fix it, would it be possible to work on other comments
> and keep this cleanup for post-rc1 integration?
> I feel this cleanup is important to get the right design but it wouldn't be
> fair to block this (old) patchset for this reason.
>

I appreciate for your suggestion. I will keep working on it for post-rc1.

Thanks,
Tetsuya

[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Thomas Monjalon]

2015-02-17 19:26, Tetsuya Mukawa:
> On 2015/02/17 18:23, Thomas Monjalon wrote:
> > 2015-02-17 17:51, Tetsuya Mukawa:
> >> On 2015/02/17 10:48, Thomas Monjalon wrote:
> >>> 2015-02-16 13:14, Tetsuya Mukawa:
>  +/* attach the new physical device, then store port_id of the device */
>  +static int
>  +rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id)
>  +{
>  +uint8_t new_port_id;
>  +struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
>  +
>  +if ((addr == NULL) || (port_id == NULL))
>  +goto err;
>  +
>  +/* save current port status */
>  +if (rte_eth_dev_save(devs, sizeof(devs)))
>  +goto err;
>  +/* re-construct pci_device_list */
>  +if (rte_eal_pci_scan())
>  +goto err;
>  +/* invoke probe func of the driver can handle the new device */
>  +if (rte_eal_pci_probe_one(addr))
>  +goto err;
> >>> You should get the port_id from the previous function instead of 
> >>> searching it.
> >> I agree this will beautify this code, but actually to do like above
> >> changes current DPDK code much more, and it will not be clear, and not
> >> beautiful.
> >>
> >> Could I explain it more.
> >> Problem is initialization sequence of virtual device and physical device
> >> are completely different.
> >>
> >> (1) Attaching a physical device case
> >> - To return port id, pci_invoke_all_drivers() needs to return port id.
> >> - It means "devinit" of "struct rte_pci_driver" needs to return port_id.
> >> - "devinit" will be rte_eth_dev_init(). But if the device is virtual,
> >> this function is not implemented.
> >>
> >> (2) Attaching virtual device case
> >> - To return port id from rte_eal_pci_probe_one(),
> >> pci_invoke_all_drivers() needs to return port id.
> >> - It means "init" of "struct rte_driver" needs to return port_id.
> >> - "init" will be implemented in PMD. But this function has different
> >> usage in physical device and virtual device.
> >> - Especially, In the case of physical device, "init" doesn't allocate
> >> eth device, so cannot return port id.
> >>
> >> As a result, to remove  rte_eth_dev_save() and
> >> rte_eth_dev_get_changed_port(), below different functions needs to
> >> return port id.
> >>  - "devinit" of "struct rte_pci_driver".
> >>  - "init" of "struct rte_driver".
> > Yes, exactly,
> > I think you shouldn't hesitate to improve existing EAL code.
> > And I also think we should try to remove differences between virtual and
> > pci devices.
> 
> I strongly agree with it. But I haven't investigated how to remove it so
> far.
> To be honest, I want to submit hotplug patches to DPDK-2.0.
> Is above functionality needed to merge the hotplug patches?
> I guess I will not be able to remove it by 23rd.

Obviously, it would be better to have it in dpdk-2.0.0-rc1.
If not possible to fix it, would it be possible to work on other comments
and keep this cleanup for post-rc1 integration?
I feel this cleanup is important to get the right design but it wouldn't be
fair to block this (old) patchset for this reason.

[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Tetsuya Mukawa]

On 2015/02/17 18:23, Thomas Monjalon wrote:
> 2015-02-17 17:51, Tetsuya Mukawa:
>> On 2015/02/17 10:48, Thomas Monjalon wrote:
>>> 2015-02-16 13:14, Tetsuya Mukawa:
 +/* attach the new physical device, then store port_id of the device */
 +static int
 +rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id)
 +{
 +  uint8_t new_port_id;
 +  struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
 +
 +  if ((addr == NULL) || (port_id == NULL))
 +  goto err;
 +
 +  /* save current port status */
 +  if (rte_eth_dev_save(devs, sizeof(devs)))
 +  goto err;
 +  /* re-construct pci_device_list */
 +  if (rte_eal_pci_scan())
 +  goto err;
 +  /* invoke probe func of the driver can handle the new device */
 +  if (rte_eal_pci_probe_one(addr))
 +  goto err;
>>> You should get the port_id from the previous function instead of searching 
>>> it.
>> I agree this will beautify this code, but actually to do like above
>> changes current DPDK code much more, and it will not be clear, and not
>> beautiful.
>>
>> Could I explain it more.
>> Problem is initialization sequence of virtual device and physical device
>> are completely different.
>>
>> (1) Attaching a physical device case
>> - To return port id, pci_invoke_all_drivers() needs to return port id.
>> - It means "devinit" of "struct rte_pci_driver" needs to return port_id.
>> - "devinit" will be rte_eth_dev_init(). But if the device is virtual,
>> this function is not implemented.
>>
>> (2) Attaching virtual device case
>> - To return port id from rte_eal_pci_probe_one(),
>> pci_invoke_all_drivers() needs to return port id.
>> - It means "init" of "struct rte_driver" needs to return port_id.
>> - "init" will be implemented in PMD. But this function has different
>> usage in physical device and virtual device.
>> - Especially, In the case of physical device, "init" doesn't allocate
>> eth device, so cannot return port id.
>>
>> As a result, to remove  rte_eth_dev_save() and
>> rte_eth_dev_get_changed_port(), below different functions needs to
>> return port id.
>>  - "devinit" of "struct rte_pci_driver".
>>  - "init" of "struct rte_driver".
> Yes, exactly,
> I think you shouldn't hesitate to improve existing EAL code.
> And I also think we should try to remove differences between virtual and
> pci devices.

I strongly agree with it. But I haven't investigated how to remove it so
far.
To be honest, I want to submit hotplug patches to DPDK-2.0.
Is above functionality needed to merge the hotplug patches?
I guess I will not be able to remove it by 23rd.

Regards,
Tetsuya

>> That is why I implement like above.
>>
 +  /* get port_id enabled by above procedures */
 +  if (rte_eth_dev_get_changed_port(devs, _port_id))
 +  goto err;
 +
 +  *port_id = new_port_id;
 +  return 0;
 +err:
 +  RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n");
 +  return -1;
 +}
>>> [...]
>>>
 +/* attach the new virtual device, then store port_id of the device */
 +static int
 +rte_eal_dev_attach_vdev(const char *vdevargs, uint8_t *port_id)
 +{
 +  char *args;
 +  uint8_t new_port_id;
 +  struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
 +
 +  if ((vdevargs == NULL) || (port_id == NULL))
 +  goto err0;
 +
 +  args = strdup(vdevargs);
 +  if (args == NULL)
 +  goto err0;
 +
 +  /* save current port status */
 +  if (rte_eth_dev_save(devs, sizeof(devs)))
 +  goto err1;
 +  /* add the vdevargs to devargs_list */
 +  if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, args))
 +  goto err1;
 +  /* parse vdevargs, then retrieve device name */
 +  get_vdev_name(args);
 +  /* walk around dev_driver_list to find the driver of the device,
 +   * then invoke probe function o the driver */
 +  if (rte_eal_vdev_find_and_invoke(args, RTE_EAL_INVOKE_TYPE_PROBE))
 +  goto err2;
>>> Again, you should get port_id from the attach procedure.
>>>
 +  /* get port_id enabled by above procedures */
 +  if (rte_eth_dev_get_changed_port(devs, _port_id))
 +  goto err2;
>>> [...]
>>>
  /**
 + * Uninitilization function called for each device driver once.
 + */
 +typedef int (rte_dev_uninit_t)(const char *name, const char *args);
>>> Why do you need args for uninit?
>>>
>> I just added for the case that finalization code of PMD needs it.
>> But, probably "args" parameter can be removed.
> Yes I think
>
>

[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Tetsuya Mukawa]

On 2015/02/17 10:48, Thomas Monjalon wrote:
> 2015-02-16 13:14, Tetsuya Mukawa:
>> These functions are used for attaching or detaching a port.
>> When rte_eal_dev_attach() is called, the function tries to realize the
>> device name as pci address. If this is done successfully,
>> rte_eal_dev_attach() will attach physical device port. If not, attaches
>> virtual devive port.
>> When rte_eal_dev_detach() is called, the function gets the device type
>> of this port to know whether the port is come from physical or virtual.
>> And then specific detaching function will be called.
>>
>> v8:
>> - Add missing symbol in version map.
>>   (Thanks to Qiu, Michael and Iremonger, Bernard)
>> v7:
>> - Fix typo of warning messages.
>>   (Thanks to Qiu, Michael)
>> v5:
>> - Change function names like below.
>>   rte_eal_dev_find_and_invoke() to rte_eal_vdev_find_and_invoke().
>>   rte_eal_dev_invoke() to rte_eal_vdev_invoke().
>> - Add code to handle a return value of rte_eal_devargs_remove().
>> - Fix pci address format in rte_eal_dev_detach().
>> v4:
>> - Fix comment.
>> - Add error checking.
>> - Fix indent of 'if' statement.
>> - Change function name.
>>
>> Signed-off-by: Tetsuya Mukawa 
>> ---
>>  lib/librte_eal/common/eal_common_dev.c  | 276 
>> 
>>  lib/librte_eal/common/eal_private.h |  11 +
>>  lib/librte_eal/common/include/rte_dev.h |  33 +++
>>  lib/librte_eal/linuxapp/eal/Makefile|   1 +
>>  lib/librte_eal/linuxapp/eal/eal_pci.c   |   6 +-
>>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   2 +
>>  6 files changed, 326 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_dev.c 
>> b/lib/librte_eal/common/eal_common_dev.c
>> index eae5656..3d169a4 100644
>> --- a/lib/librte_eal/common/eal_common_dev.c
>> +++ b/lib/librte_eal/common/eal_common_dev.c
>> @@ -32,10 +32,13 @@
>>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>   */
>>  
>> +#include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>>  
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -107,3 +110,276 @@ rte_eal_dev_init(void)
>>  }
>>  return 0;
>>  }
>> +
>> +/* So far, DPDK hotplug function only supports linux */
> This comment should be in the configuration.

Sure I will.

>
>> +#ifdef ENABLE_HOTPLUG
>> +static void
>> +rte_eal_vdev_invoke(struct rte_driver *driver,
>> +struct rte_devargs *devargs, enum rte_eal_invoke_type type)
>> +{
>> +if ((driver == NULL) || (devargs == NULL))
>> +return;
>> +
>> +switch (type) {
>> +case RTE_EAL_INVOKE_TYPE_PROBE:
>> +driver->init(devargs->virtual.drv_name, devargs->args);
>> +break;
>> +case RTE_EAL_INVOKE_TYPE_CLOSE:
>> +driver->uninit(devargs->virtual.drv_name, devargs->args);
>> +break;
>> +default:
>> +break;
>> +}
>> +}
> It would be clearer to directly call init and uninit instead of using this
> "invoke" method.

Sure, I will change it.

>> +
>> +static int
>> +rte_eal_vdev_find_and_invoke(const char *name, int type)
>> +{
>> +struct rte_devargs *devargs;
>> +struct rte_driver *driver;
>> +
>> +if (name == NULL)
>> +return -EINVAL;
>> +
>> +/* call the init function for each virtual device */
> This comment is wrong.

Thanks, I will fix it.

>> +TAILQ_FOREACH(devargs, _list, next) {
>> +
>> +if (devargs->type != RTE_DEVTYPE_VIRTUAL)
>> +continue;
>> +
>> +if (strncmp(name, devargs->virtual.drv_name, strlen(name)))
>> +continue;
>> +
>> +TAILQ_FOREACH(driver, _driver_list, next) {
>> +if (driver->type != PMD_VDEV)
>> +continue;
>> +
>> +/* search a driver prefix in virtual device name */
>> +if (!strncmp(driver->name, devargs->virtual.drv_name,
>> +strlen(driver->name))) {
> Why not use strcmp?

I will replace it.

>> +rte_eal_vdev_invoke(driver, devargs, type);
>> +break;
>> +}
>> +}
>> +
>> +if (driver == NULL) {
>> +RTE_LOG(WARNING, EAL, "no driver found for %s\n",
>> +  devargs->virtual.drv_name);
>> +}
>> +return 0;
>> +}
>> +return 1;
>> +}
>> +
>> +/* attach the new physical device, then store port_id of the device */
>> +static int
>> +rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id)
>> +{
>> +uint8_t new_port_id;
>> +struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
>> +
>> +if ((addr == NULL) || (port_id == NULL))
>> +goto err;
>> +
>> +/* save current port status */
>> +if (rte_eth_dev_save(devs, sizeof(devs)))
>> +goto err;
>> +/* re-construct pci_device_list */
>> +

[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Maxime Leroy]

Hi Tetsuya,

On Tue, Feb 17, 2015 at 9:51 AM, Tetsuya Mukawa  wrote:
>
>
> >> +/* get port_id enabled by above procedures */
> >> +if (rte_eth_dev_get_changed_port(devs, _port_id))
> >> +goto err2;
> > [...]
> >
> >>  /**
> >> + * Uninitilization function called for each device driver once.
> >> + */
> >> +typedef int (rte_dev_uninit_t)(const char *name, const char *args);
> > Why do you need args for uninit?
> >
>
> I just added for the case that finalization code of PMD needs it.
> But, probably "args" parameter can be removed.
>
>

I think there are no needs to have any args in the uninit function:
1) You librte_pmd_null doesn't use it
2) You give exactly the same argument that was used by the init
function. A driver should have already stored these parameters in an
internal private structure at initialization. So it's not needed to
give me back for uninit method.

>From my understanding devargs_list is only needed at the init to store
the arguments when we parse the command line. Then, at initialization,
rte_eal_dev_init  creates the devices from this list .

By removing args from uninit function, you doesn't need to add and
remove anymore devargs in devargs_list to (de)attach a new device.

What do you think ?

Maxime

[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Thomas Monjalon]

2015-02-17 17:51, Tetsuya Mukawa:
> On 2015/02/17 10:48, Thomas Monjalon wrote:
> > 2015-02-16 13:14, Tetsuya Mukawa:
> >> +/* attach the new physical device, then store port_id of the device */
> >> +static int
> >> +rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id)
> >> +{
> >> +  uint8_t new_port_id;
> >> +  struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
> >> +
> >> +  if ((addr == NULL) || (port_id == NULL))
> >> +  goto err;
> >> +
> >> +  /* save current port status */
> >> +  if (rte_eth_dev_save(devs, sizeof(devs)))
> >> +  goto err;
> >> +  /* re-construct pci_device_list */
> >> +  if (rte_eal_pci_scan())
> >> +  goto err;
> >> +  /* invoke probe func of the driver can handle the new device */
> >> +  if (rte_eal_pci_probe_one(addr))
> >> +  goto err;
> > You should get the port_id from the previous function instead of searching 
> > it.
> 
> I agree this will beautify this code, but actually to do like above
> changes current DPDK code much more, and it will not be clear, and not
> beautiful.
> 
> Could I explain it more.
> Problem is initialization sequence of virtual device and physical device
> are completely different.
> 
> (1) Attaching a physical device case
> - To return port id, pci_invoke_all_drivers() needs to return port id.
> - It means "devinit" of "struct rte_pci_driver" needs to return port_id.
> - "devinit" will be rte_eth_dev_init(). But if the device is virtual,
> this function is not implemented.
> 
> (2) Attaching virtual device case
> - To return port id from rte_eal_pci_probe_one(),
> pci_invoke_all_drivers() needs to return port id.
> - It means "init" of "struct rte_driver" needs to return port_id.
> - "init" will be implemented in PMD. But this function has different
> usage in physical device and virtual device.
> - Especially, In the case of physical device, "init" doesn't allocate
> eth device, so cannot return port id.
> 
> As a result, to remove  rte_eth_dev_save() and
> rte_eth_dev_get_changed_port(), below different functions needs to
> return port id.
>  - "devinit" of "struct rte_pci_driver".
>  - "init" of "struct rte_driver".

Yes, exactly,
I think you shouldn't hesitate to improve existing EAL code.
And I also think we should try to remove differences between virtual and
pci devices.

> That is why I implement like above.
> 
> >> +  /* get port_id enabled by above procedures */
> >> +  if (rte_eth_dev_get_changed_port(devs, _port_id))
> >> +  goto err;
> >> +
> >> +  *port_id = new_port_id;
> >> +  return 0;
> >> +err:
> >> +  RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n");
> >> +  return -1;
> >> +}
> > [...]
> >
> >> +/* attach the new virtual device, then store port_id of the device */
> >> +static int
> >> +rte_eal_dev_attach_vdev(const char *vdevargs, uint8_t *port_id)
> >> +{
> >> +  char *args;
> >> +  uint8_t new_port_id;
> >> +  struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
> >> +
> >> +  if ((vdevargs == NULL) || (port_id == NULL))
> >> +  goto err0;
> >> +
> >> +  args = strdup(vdevargs);
> >> +  if (args == NULL)
> >> +  goto err0;
> >> +
> >> +  /* save current port status */
> >> +  if (rte_eth_dev_save(devs, sizeof(devs)))
> >> +  goto err1;
> >> +  /* add the vdevargs to devargs_list */
> >> +  if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, args))
> >> +  goto err1;
> >> +  /* parse vdevargs, then retrieve device name */
> >> +  get_vdev_name(args);
> >> +  /* walk around dev_driver_list to find the driver of the device,
> >> +   * then invoke probe function o the driver */
> >> +  if (rte_eal_vdev_find_and_invoke(args, RTE_EAL_INVOKE_TYPE_PROBE))
> >> +  goto err2;
> > Again, you should get port_id from the attach procedure.
> >
> >> +  /* get port_id enabled by above procedures */
> >> +  if (rte_eth_dev_get_changed_port(devs, _port_id))
> >> +  goto err2;
> > [...]
> >
> >>  /**
> >> + * Uninitilization function called for each device driver once.
> >> + */
> >> +typedef int (rte_dev_uninit_t)(const char *name, const char *args);
> > Why do you need args for uninit?
> >
> 
> I just added for the case that finalization code of PMD needs it.
> But, probably "args" parameter can be removed.

Yes I think

[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Thomas Monjalon]

2015-02-16 13:14, Tetsuya Mukawa:
> These functions are used for attaching or detaching a port.
> When rte_eal_dev_attach() is called, the function tries to realize the
> device name as pci address. If this is done successfully,
> rte_eal_dev_attach() will attach physical device port. If not, attaches
> virtual devive port.
> When rte_eal_dev_detach() is called, the function gets the device type
> of this port to know whether the port is come from physical or virtual.
> And then specific detaching function will be called.
> 
> v8:
> - Add missing symbol in version map.
>   (Thanks to Qiu, Michael and Iremonger, Bernard)
> v7:
> - Fix typo of warning messages.
>   (Thanks to Qiu, Michael)
> v5:
> - Change function names like below.
>   rte_eal_dev_find_and_invoke() to rte_eal_vdev_find_and_invoke().
>   rte_eal_dev_invoke() to rte_eal_vdev_invoke().
> - Add code to handle a return value of rte_eal_devargs_remove().
> - Fix pci address format in rte_eal_dev_detach().
> v4:
> - Fix comment.
> - Add error checking.
> - Fix indent of 'if' statement.
> - Change function name.
> 
> Signed-off-by: Tetsuya Mukawa 
> ---
>  lib/librte_eal/common/eal_common_dev.c  | 276 
> 
>  lib/librte_eal/common/eal_private.h |  11 +
>  lib/librte_eal/common/include/rte_dev.h |  33 +++
>  lib/librte_eal/linuxapp/eal/Makefile|   1 +
>  lib/librte_eal/linuxapp/eal/eal_pci.c   |   6 +-
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   2 +
>  6 files changed, 326 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_dev.c 
> b/lib/librte_eal/common/eal_common_dev.c
> index eae5656..3d169a4 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -32,10 +32,13 @@
>   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -107,3 +110,276 @@ rte_eal_dev_init(void)
>   }
>   return 0;
>  }
> +
> +/* So far, DPDK hotplug function only supports linux */

This comment should be in the configuration.

> +#ifdef ENABLE_HOTPLUG
> +static void
> +rte_eal_vdev_invoke(struct rte_driver *driver,
> + struct rte_devargs *devargs, enum rte_eal_invoke_type type)
> +{
> + if ((driver == NULL) || (devargs == NULL))
> + return;
> +
> + switch (type) {
> + case RTE_EAL_INVOKE_TYPE_PROBE:
> + driver->init(devargs->virtual.drv_name, devargs->args);
> + break;
> + case RTE_EAL_INVOKE_TYPE_CLOSE:
> + driver->uninit(devargs->virtual.drv_name, devargs->args);
> + break;
> + default:
> + break;
> + }
> +}

It would be clearer to directly call init and uninit instead of using this
"invoke" method.

> +
> +static int
> +rte_eal_vdev_find_and_invoke(const char *name, int type)
> +{
> + struct rte_devargs *devargs;
> + struct rte_driver *driver;
> +
> + if (name == NULL)
> + return -EINVAL;
> +
> + /* call the init function for each virtual device */

This comment is wrong.

> + TAILQ_FOREACH(devargs, _list, next) {
> +
> + if (devargs->type != RTE_DEVTYPE_VIRTUAL)
> + continue;
> +
> + if (strncmp(name, devargs->virtual.drv_name, strlen(name)))
> + continue;
> +
> + TAILQ_FOREACH(driver, _driver_list, next) {
> + if (driver->type != PMD_VDEV)
> + continue;
> +
> + /* search a driver prefix in virtual device name */
> + if (!strncmp(driver->name, devargs->virtual.drv_name,
> + strlen(driver->name))) {

Why not use strcmp?

> + rte_eal_vdev_invoke(driver, devargs, type);
> + break;
> + }
> + }
> +
> + if (driver == NULL) {
> + RTE_LOG(WARNING, EAL, "no driver found for %s\n",
> +   devargs->virtual.drv_name);
> + }
> + return 0;
> + }
> + return 1;
> +}
> +
> +/* attach the new physical device, then store port_id of the device */
> +static int
> +rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id)
> +{
> + uint8_t new_port_id;
> + struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
> +
> + if ((addr == NULL) || (port_id == NULL))
> + goto err;
> +
> + /* save current port status */
> + if (rte_eth_dev_save(devs, sizeof(devs)))
> + goto err;
> + /* re-construct pci_device_list */
> + if (rte_eal_pci_scan())
> + goto err;
> + /* invoke probe func of the driver can handle the new device */
> + if (rte_eal_pci_probe_one(addr))
> + goto err;

You should get the port_id from 

[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Iremonger, Bernard]

> -Original Message-
> From: Tetsuya Mukawa [mailto:mukawa at igel.co.jp]
> Sent: Monday, February 16, 2015 4:15 AM
> To: dev at dpdk.org
> Cc: Qiu, Michael; Iremonger, Bernard; Tetsuya Mukawa
> Subject: [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions
> 
> These functions are used for attaching or detaching a port.
> When rte_eal_dev_attach() is called, the function tries to realize the device 
> name as pci address. If
> this is done successfully,
> rte_eal_dev_attach() will attach physical device port. If not, attaches 
> virtual devive port.
> When rte_eal_dev_detach() is called, the function gets the device type of 
> this port to know whether
> the port is come from physical or virtual.
> And then specific detaching function will be called.
> 
> v8:
> - Add missing symbol in version map.
>   (Thanks to Qiu, Michael and Iremonger, Bernard)
> v7:
> - Fix typo of warning messages.
>   (Thanks to Qiu, Michael)
> v5:
> - Change function names like below.
>   rte_eal_dev_find_and_invoke() to rte_eal_vdev_find_and_invoke().
>   rte_eal_dev_invoke() to rte_eal_vdev_invoke().
> - Add code to handle a return value of rte_eal_devargs_remove().
> - Fix pci address format in rte_eal_dev_detach().
> v4:
> - Fix comment.
> - Add error checking.
> - Fix indent of 'if' statement.
> - Change function name.
> 
> Signed-off-by: Tetsuya Mukawa 

Acked-by: Bernard Iremonger 

[dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions

[Tetsuya Mukawa]

These functions are used for attaching or detaching a port.
When rte_eal_dev_attach() is called, the function tries to realize the
device name as pci address. If this is done successfully,
rte_eal_dev_attach() will attach physical device port. If not, attaches
virtual devive port.
When rte_eal_dev_detach() is called, the function gets the device type
of this port to know whether the port is come from physical or virtual.
And then specific detaching function will be called.

v8:
- Add missing symbol in version map.
  (Thanks to Qiu, Michael and Iremonger, Bernard)
v7:
- Fix typo of warning messages.
  (Thanks to Qiu, Michael)
v5:
- Change function names like below.
  rte_eal_dev_find_and_invoke() to rte_eal_vdev_find_and_invoke().
  rte_eal_dev_invoke() to rte_eal_vdev_invoke().
- Add code to handle a return value of rte_eal_devargs_remove().
- Fix pci address format in rte_eal_dev_detach().
v4:
- Fix comment.
- Add error checking.
- Fix indent of 'if' statement.
- Change function name.

Signed-off-by: Tetsuya Mukawa 
---
 lib/librte_eal/common/eal_common_dev.c  | 276 
 lib/librte_eal/common/eal_private.h |  11 +
 lib/librte_eal/common/include/rte_dev.h |  33 +++
 lib/librte_eal/linuxapp/eal/Makefile|   1 +
 lib/librte_eal/linuxapp/eal/eal_pci.c   |   6 +-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   2 +
 6 files changed, 326 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c 
b/lib/librte_eal/common/eal_common_dev.c
index eae5656..3d169a4 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -32,10 +32,13 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */

+#include 
+#include 
 #include 
 #include 
 #include 

+#include 
 #include 
 #include 
 #include 
@@ -107,3 +110,276 @@ rte_eal_dev_init(void)
}
return 0;
 }
+
+/* So far, DPDK hotplug function only supports linux */
+#ifdef ENABLE_HOTPLUG
+static void
+rte_eal_vdev_invoke(struct rte_driver *driver,
+   struct rte_devargs *devargs, enum rte_eal_invoke_type type)
+{
+   if ((driver == NULL) || (devargs == NULL))
+   return;
+
+   switch (type) {
+   case RTE_EAL_INVOKE_TYPE_PROBE:
+   driver->init(devargs->virtual.drv_name, devargs->args);
+   break;
+   case RTE_EAL_INVOKE_TYPE_CLOSE:
+   driver->uninit(devargs->virtual.drv_name, devargs->args);
+   break;
+   default:
+   break;
+   }
+}
+
+static int
+rte_eal_vdev_find_and_invoke(const char *name, int type)
+{
+   struct rte_devargs *devargs;
+   struct rte_driver *driver;
+
+   if (name == NULL)
+   return -EINVAL;
+
+   /* call the init function for each virtual device */
+   TAILQ_FOREACH(devargs, _list, next) {
+
+   if (devargs->type != RTE_DEVTYPE_VIRTUAL)
+   continue;
+
+   if (strncmp(name, devargs->virtual.drv_name, strlen(name)))
+   continue;
+
+   TAILQ_FOREACH(driver, _driver_list, next) {
+   if (driver->type != PMD_VDEV)
+   continue;
+
+   /* search a driver prefix in virtual device name */
+   if (!strncmp(driver->name, devargs->virtual.drv_name,
+   strlen(driver->name))) {
+   rte_eal_vdev_invoke(driver, devargs, type);
+   break;
+   }
+   }
+
+   if (driver == NULL) {
+   RTE_LOG(WARNING, EAL, "no driver found for %s\n",
+ devargs->virtual.drv_name);
+   }
+   return 0;
+   }
+   return 1;
+}
+
+/* attach the new physical device, then store port_id of the device */
+static int
+rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id)
+{
+   uint8_t new_port_id;
+   struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
+
+   if ((addr == NULL) || (port_id == NULL))
+   goto err;
+
+   /* save current port status */
+   if (rte_eth_dev_save(devs, sizeof(devs)))
+   goto err;
+   /* re-construct pci_device_list */
+   if (rte_eal_pci_scan())
+   goto err;
+   /* invoke probe func of the driver can handle the new device */
+   if (rte_eal_pci_probe_one(addr))
+   goto err;
+   /* get port_id enabled by above procedures */
+   if (rte_eth_dev_get_changed_port(devs, _port_id))
+   goto err;
+
+   *port_id = new_port_id;
+   return 0;
+err:
+   RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n");
+   return -1;
+}
+
+/* detach the new physical device, then store pci_addr of the device */
+static int
+rte_eal_dev_detach_pdev(uint8_t port_id, struct