[dpdk-dev] [PATCH v9 13/14] eal/pci: Add rte_eal_dev_attach/detach() functions
On 2015/02/19 22:30, Tetsuya Mukawa wrote:
> On 2015/02/19 21:10, Thomas Monjalon wrote:
>> 2015-02-19 11:49, Tetsuya Mukawa:
>>> +/* 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;
>> Could you explain why you store devargs in a list?
> I try to do same behavior when rte_eal_init() is called.
Sorry for lack of explanation.
"vdevargs" of rte_eal_dev_attach_vdev() will be same format of "--vdev"
option.
And when rte_eal_init() is called, such a "--vdev" option value will be
stored in devargs_list.
So I try to same thing here.
> If only hotplug doesn't do this, someone may be confused when they try
> to realize devargs_list.
>
>>> + /* 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_init(args))
>> TODO: get port_id from init.
> Yes, I will.
> I also add comment about it.
>
>>> + goto err2;
>>> + /* get port_id enabled by above procedures */
>>> + if (rte_eth_dev_get_changed_port(devs, _port_id))
>>> + goto err2;
>> [...]
>>> --- a/lib/librte_eal/common/include/rte_dev.h
>>> +++ b/lib/librte_eal/common/include/rte_dev.h
>>> @@ -47,6 +47,7 @@ extern "C" {
>>> #endif
>>>
>>> #include
>>> +#include
>>>
>>> /** Double linked list of device drivers. */
>>> TAILQ_HEAD(rte_driver_list, rte_driver);
>>> @@ -57,6 +58,11 @@ TAILQ_HEAD(rte_driver_list, rte_driver);
>>> typedef int (rte_dev_init_t)(const char *name, const char *args);
>>>
>>> /**
>>> + * Uninitilization function called for each device driver once.
>>> + */
>>> +typedef int (rte_dev_uninit_t)(const char *name);
>> Why using name as parameter and not port_id?
> This function pointer will be implemented in PMDs.
> For example, in pcap PMD, rte_pmd_pcap_devuninit() is the function.
>
> static struct rte_driver pmd_pcap_drv = {
> .name = "eth_pcap",
> .type = PMD_VDEV,
> .init = rte_pmd_pcap_devinit,
> .uninit = rte_pmd_pcap_devuninit,
> };
>
> "port_id" isn't needed in PMD.
>
>> [...]
>>> --- a/lib/librte_eal/linuxapp/eal/Makefile
>>> +++ b/lib/librte_eal/linuxapp/eal/Makefile
>>> @@ -45,6 +45,7 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
>>> CFLAGS += -I$(RTE_SDK)/lib/librte_ring
>>> CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
>>> CFLAGS += -I$(RTE_SDK)/lib/librte_malloc
>>> +CFLAGS += -I$(RTE_SDK)/lib/librte_mbuf
>> Why do you need mbuf?
> To include rte_ethdev.h in this code, rte_mbuf.h is also needed.
>
>> [...]
>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> @@ -21,6 +21,8 @@ DPDK_2.0 {
>>> rte_eal_alarm_cancel;
>>> rte_eal_alarm_set;
>>> rte_eal_dev_init;
>>> + rte_eal_dev_attach;
>>> + rte_eal_dev_detach;
>> Please keep alphabetical order.
>>
> Sure, I will.
>
> Thanks,
> Tetsuya
>
[dpdk-dev] [PATCH v9 13/14] eal/pci: Add rte_eal_dev_attach/detach() functions
2015-02-19 11:49, Tetsuya Mukawa:
> +/* 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;
Could you explain why you store devargs in a list?
> + /* 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_init(args))
TODO: get port_id from init.
> + goto err2;
> + /* get port_id enabled by above procedures */
> + if (rte_eth_dev_get_changed_port(devs, _port_id))
> + goto err2;
[...]
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -47,6 +47,7 @@ extern "C" {
> #endif
>
> #include
> +#include
>
> /** Double linked list of device drivers. */
> TAILQ_HEAD(rte_driver_list, rte_driver);
> @@ -57,6 +58,11 @@ TAILQ_HEAD(rte_driver_list, rte_driver);
> typedef int (rte_dev_init_t)(const char *name, const char *args);
>
> /**
> + * Uninitilization function called for each device driver once.
> + */
> +typedef int (rte_dev_uninit_t)(const char *name);
Why using name as parameter and not port_id?
[...]
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -45,6 +45,7 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
> CFLAGS += -I$(RTE_SDK)/lib/librte_ring
> CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
> CFLAGS += -I$(RTE_SDK)/lib/librte_malloc
> +CFLAGS += -I$(RTE_SDK)/lib/librte_mbuf
Why do you need mbuf?
[...]
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -21,6 +21,8 @@ DPDK_2.0 {
> rte_eal_alarm_cancel;
> rte_eal_alarm_set;
> rte_eal_dev_init;
> + rte_eal_dev_attach;
> + rte_eal_dev_detach;
Please keep alphabetical order.
[dpdk-dev] [PATCH v9 13/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.
v9:
- Fix comments.
- Use strcmp() instead of strncmp().
- Remove RTE_EAL_INVOKE_TYPE_PROBE/CLOSE.
- Change definition of rte_dev_uninit_t.
(Thanks to Thomas Monjalon and Maxime Leroy)
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 | 307
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, 357 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..4976bb9 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,307 @@ rte_eal_dev_init(void)
}
return 0;
}
+
+/* So far, DPDK hotplug function only supports linux */
+#ifdef RTE_LIBRTE_EAL_HOTPLUG
+static int
+rte_eal_vdev_find_and_init(const char *name)
+{
+ struct rte_devargs *devargs;
+ struct rte_driver *driver;
+
+ if (name == NULL)
+ return -EINVAL;
+
+ /* find the specified device and call init function */
+ TAILQ_FOREACH(devargs, _list, next) {
+
+ if (devargs->type != RTE_DEVTYPE_VIRTUAL)
+ continue;
+
+ if (strcmp(name, devargs->virtual.drv_name))
+ continue;
+
+ TAILQ_FOREACH(driver, _driver_list, next) {
+ if (driver->type != PMD_VDEV)
+ continue;
+
+ /*
+* search a driver prefix in virtual device name.
+* For example, if the driver is pcap PMD, driver->name
+* will be "eth_pcap", but devargs->virtual.drv_name
+* will be "eth_pcapN". So use strncmp to compare.
+*/
+ if (!strncmp(driver->name, devargs->virtual.drv_name,
+ strlen(driver->name))) {
+ driver->init(devargs->virtual.drv_name,
+ devargs->args);
+ break;
+ }
+ }
+
+ if (driver == NULL) {
+ RTE_LOG(WARNING, EAL, "no driver found for %s\n",
+ devargs->virtual.drv_name);
+ }
+ return 0;
+ }
+ return 1;
+}
+
+static int
+rte_eal_vdev_find_and_uninit(const char *name)
+{
+ struct rte_devargs *devargs;
+ struct rte_driver *driver;
+
+ if (name == NULL)
+ return -EINVAL;
+
+ /* find the specified device and call uninit function */
+ TAILQ_FOREACH(devargs, _list, next) {
+
+ if (devargs->type != RTE_DEVTYPE_VIRTUAL)
+ continue;
+
+ if (strcmp(name, devargs->virtual.drv_name))
+ continue;
+
+ TAILQ_FOREACH(driver, _driver_list, next) {
+ if (driver->type != PMD_VDEV)
+ continue;
+
+ /*
+* search a driver prefix in virtual device name.
+* For example, if the driver is pcap PMD, driver->name
+* will be "eth_pcap", but devargs->virtual.drv_name
+* will be "eth_pcapN". So use strncmp to compare.
+*/
+ if (!strncmp(driver->name, devargs->virtual.drv_name,
+ strlen(driver->name))) {
+
