[dpdk-dev] [PATCH v6 15/17] eal: add hotplug operations for pci and vdev

[Shreyansh jain]

On Thursday 14 July 2016 10:20 PM, Jan Viktorin wrote:
> On Tue, 12 Jul 2016 11:31:20 +0530
> Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
> 
>> Hotplug which deals with resources should come from the layer that already
>> handles them, i.e. EAL.
>>
>> For both attach and detach operations, 'name' is used to select the bus
>> that will handle the request.
>>
>> Signed-off-by: David Marchand <david.marchand at 6wind.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>
>> ---
>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 ++
>>  lib/librte_eal/common/eal_common_dev.c          | 47 +++++++++++++++++++++++++
>>  lib/librte_eal/common/include/rte_dev.h         | 25 +++++++++++++
>>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 ++
>>  4 files changed, 76 insertions(+)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> index 1852c4a..6f9324f 100644
>> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> @@ -159,5 +159,7 @@ DPDK_16.07 {
>>  	rte_keepalive_mark_sleep;
>>  	rte_keepalive_register_relay_callback;
>>  	rte_thread_setname;
>> +	rte_eal_dev_attach;
>> +	rte_eal_dev_detach;
>>  
>>  } DPDK_16.04;
>> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
>> index a8a4146..14c6cf1 100644
>> --- a/lib/librte_eal/common/eal_common_dev.c
>> +++ b/lib/librte_eal/common/eal_common_dev.c
>> @@ -150,3 +150,50 @@ rte_eal_vdev_uninit(const char *name)
>>  	RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
>>  	return -EINVAL;
>>  }
>> +
>> +int rte_eal_dev_attach(const char *name, const char *devargs)
>> +{
>> +	struct rte_pci_addr addr;
>> +	int ret = -1;
>> +
>> +	if (name == NULL || devargs == NULL) {
>> +		RTE_LOG(ERR, EAL, "Invalid device arguments provided\n");
>> +		return ret;
>> +	}
>> +
>> +	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
>> +		if (rte_eal_pci_probe_one(&addr) < 0)
>> +			goto err;
>> +
>> +	} else {
>> +		if (rte_eal_vdev_init(name, devargs))
>> +			goto err;
>> +	}
>> +
>> +	return 0;
>> +
>> +err:
>> +	RTE_LOG(ERR, EAL, "Driver cannot attach the device\n");
> 
> I think this log can be more specific. We have the name of the device.
> It might be confusing to the user when the attach fails and there is
> no simple way how to determine which one.

Agree. I will update with 'name'.

> 
>> +	return ret;
>> +}
>> +
>> +int rte_eal_dev_detach(const char *name)
>> +{
>> +	struct rte_pci_addr addr;
>> +
>> +	if (name == NULL)
>> +		goto err;
>> +
>> +	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
>> +		if (rte_eal_pci_detach(&addr) < 0)
>> +			goto err;
>> +	} else {
>> +		if (rte_eal_vdev_uninit(name))
>> +			goto err;
>> +	}
>> +	return 0;
>> +
>> +err:
>> +	RTE_LOG(ERR, EAL, "Driver cannot detach the device\n");
> 
> Same as for attach.

OK.

> 
>> +	return -1;
>> +}
>> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
>> index 994650b..2f0579c 100644
>> --- a/lib/librte_eal/common/include/rte_dev.h
>> +++ b/lib/librte_eal/common/include/rte_dev.h
>> @@ -178,6 +178,31 @@ int rte_eal_vdev_init(const char *name, const char *args);
>>   */
>>  int rte_eal_vdev_uninit(const char *name);
>>  
>> +/**
>> + * Attach a resource to a registered driver.
> 
> From my POV, the term "resource" is not good here. We have
> rte_pci_resource but we refer to a device here. The function is called
> rte_eal_DEV_attach. No idea why to call it a resource.

Your point of 'resource' for 'rte_eal_*dev*_attach' makes sense.
Ideally I wouldn't have given much heed to this - for me 'resource' is just a device representation within EAL.
But, now that you have highlighted, name<=>comment is certainly mismatch.

I will change.

> 
>> + *
>> + * @param name
>> + *   The resource name, that refers to a pci resource or some private
>> + *   way of designating a resource for vdev drivers. Based on this
>> + *   resource name, eal will identify a driver capable of handling
>> + *   this resource and pass this resource to the driver probing
>> + *   function.
>> + * @param devargs
>> + *   Device arguments to be passed to the driver.
>> + * @return
>> + *   0 on success, negative on error.
>> + */
>> +int rte_eal_dev_attach(const char *name, const char *devargs);
>> +
>> +/**
>> + * Detach a resource from its driver.
> 
> Same here for resource.

I will change to "Detach a device from its driver".

> 
> Jan
> 
>> + *
>> + * @param name
>> + *   Same description as for rte_eal_dev_attach().
>> + *   Here, eal will call the driver detaching function.
>> + */
>> +int rte_eal_dev_detach(const char *name);
>> +
>>  #define DRIVER_EXPORT_NAME_ARRAY(n, idx) n##idx[]
>>  
>>  #define DRIVER_EXPORT_NAME(name, idx) \
>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> index a617b9e..db866b8 100644
>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> @@ -163,5 +163,7 @@ DPDK_16.07 {
>>  	rte_keepalive_mark_sleep;
>>  	rte_keepalive_register_relay_callback;
>>  	rte_thread_setname;
>> +	rte_eal_dev_attach;
>> +	rte_eal_dev_detach;
>>  
>>  } DPDK_16.04;
>