[dpdk-dev,v2,08/12] bus/vdev: scan and probe vdev in secondary processes

Message ID 1506606959-76230-9-git-send-email-jianfeng.tan@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

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

Commit Message

Jianfeng Tan Sept. 28, 2017, 1:55 p.m. UTC
  Base on primary/secondary communication channel, we add vdev action
to scan virtual devices in secondary processes.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/bus/vdev/vdev.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 4 deletions(-)
  

Comments

Jan Blunck Oct. 5, 2017, 1:04 p.m. UTC | #1
On Thu, Sep 28, 2017 at 3:55 PM, Jianfeng Tan <jianfeng.tan@intel.com> wrote:
> Base on primary/secondary communication channel, we add vdev action
> to scan virtual devices in secondary processes.
>

This doesn't need to be in vdev. You application could just hotplug
the drivers with exactly the same parameters as the primary process.
The driver need to be aware of the primary/secondary process split
though.


> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>  drivers/bus/vdev/vdev.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 100 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index e0ba0da..1d1690f 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -104,7 +104,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
>
>         name = rte_vdev_device_name(dev);
>
> -       VDEV_LOG(DEBUG, "Search driver %s to probe device %s\n", name,
> +       VDEV_LOG(DEBUG, "Search driver %s to probe device %s", name,
>                 rte_vdev_device_name(dev));
>
>         if (vdev_parse(name, &driver))
> @@ -190,7 +190,7 @@ rte_vdev_init(const char *name, const char *args)
>         ret = vdev_probe_all_drivers(dev);
>         if (ret) {
>                 if (ret > 0)
> -                       VDEV_LOG(ERR, "no driver found for %s\n", name);
> +                       VDEV_LOG(ERR, "no driver found for %s", name);
>                 goto fail;
>         }
>
> @@ -213,7 +213,7 @@ vdev_remove_driver(struct rte_vdev_device *dev)
>         const struct rte_vdev_driver *driver;
>
>         if (!dev->device.driver) {
> -               VDEV_LOG(DEBUG, "no driver attach to device %s\n", name);
> +               VDEV_LOG(DEBUG, "no driver attach to device %s", name);
>                 return 1;
>         }
>
> @@ -252,12 +252,105 @@ rte_vdev_uninit(const char *name)
>         return 0;
>  }
>
> +struct vdev_action_params {
> +#define VDEV_SCAN_REQUEST      1
> +#define VDEV_SCAN_RESPONSE     2
> +       int type;
> +       char name[32];
> +};
> +
> +static int vdev_plug(struct rte_device *dev);
> +
> +static int
> +vdev_action(const void *params, int len,
> +           int fds[] __rte_unused,
> +           int fds_num __rte_unused)
> +{
> +       struct rte_vdev_device *dev;
> +       struct rte_devargs *devargs;
> +       struct vdev_action_params ou_params;
> +       const struct vdev_action_params *in_params = params;
> +
> +       switch (in_params->type) {
> +       case VDEV_SCAN_REQUEST:
> +               ou_params.type = VDEV_SCAN_RESPONSE;
> +               TAILQ_FOREACH(dev, &vdev_device_list, next) {
> +                       VDEV_LOG(INFO, "push vdev, %s", dev->device.name);
> +                       strncpy(ou_params.name, dev->device.name, 32);
> +                       rte_eal_mp_sendmsg("vdev", &ou_params,
> +                                                         len, NULL, 0);
> +               }
> +               break;
> +       case VDEV_SCAN_RESPONSE:
> +               VDEV_LOG(INFO, "get vdev, %s", in_params->name);
> +
> +               if (strlen(in_params->name) == 0) {
> +                       VDEV_LOG(ERR, "invalid name was passed");
> +                       break;
> +               }
> +
> +               dev = find_vdev(in_params->name);
> +               if (dev) {
> +                       VDEV_LOG(ERR, "vdev already exists: %s", in_params->name);
> +                       break;
> +               }
> +
> +               devargs = alloc_devargs(in_params->name, NULL);
> +               if (!devargs) {
> +                       VDEV_LOG(ERR, "failed to allocate memory");
> +                       break;
> +               }
> +
> +               dev = calloc(1, sizeof(*dev));
> +               if (!dev) {
> +                       VDEV_LOG(ERR, "failed to allocate memory");
> +                       free(devargs);
> +                       break;
> +               }
> +
> +               dev->device.devargs = devargs;
> +               dev->device.numa_node = 0; /* to be corrected in probe() */
> +               dev->device.name = devargs->name;
> +
> +               TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
> +               TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
> +
> +               if (vdev_plug(&dev->device) < 0) {
> +                       VDEV_LOG(ERR, "failed to plug device %s", in_params->name);
> +                       free(devargs);
> +                       free(dev);
> +               } else
> +                       VDEV_LOG(INFO, "plug in device: %s", in_params->name);
> +
> +               break;
> +       default:
> +               VDEV_LOG(ERR, "vdev cannot recognize this message");
> +       }
> +
> +       return 0;
> +}
> +
>  static int
>  vdev_scan(void)
>  {
>         struct rte_vdev_device *dev;
>         struct rte_devargs *devargs;
>
> +       if (rte_eal_mp_action_register("vdev", vdev_action) < 0) {
> +               VDEV_LOG(ERR, "vdev fails to add action");
> +               return -1;
> +       }
> +
> +       if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> +               struct vdev_action_params params;
> +
> +               params.type = VDEV_SCAN_REQUEST;
> +               rte_eal_mp_sendmsg("vdev", &params,
> +                                                 sizeof(params), NULL, 0);
> +
> +               return 0;
> +       }
> +
>         /* for virtual devices we scan the devargs_list populated via cmdline */
>         TAILQ_FOREACH(devargs, &devargs_list, next) {
>
> @@ -287,6 +380,9 @@ vdev_probe(void)
>  {
>         struct rte_vdev_device *dev;
>
> +       if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> +               return 0;
> +
>         /* call the init function for each virtual device */
>         TAILQ_FOREACH(dev, &vdev_device_list, next) {
>
> @@ -294,7 +390,7 @@ vdev_probe(void)
>                         continue;
>
>                 if (vdev_probe_all_drivers(dev)) {
> -                       VDEV_LOG(ERR, "failed to initialize %s device\n",
> +                       VDEV_LOG(ERR, "failed to initialize %s device",
>                                 rte_vdev_device_name(dev));
>                         return -1;
>                 }
> --
> 2.7.4
>
  
Jianfeng Tan Oct. 9, 2017, 1:08 a.m. UTC | #2
Hi Jan,

> -----Original Message-----

> From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of Jan

> Blunck

> Sent: Thursday, October 5, 2017 9:05 PM

> To: Tan, Jianfeng

> Cc: dev; Richardson, Bruce; Ananyev, Konstantin; De Lara Guarch, Pablo;

> Thomas Monjalon; yliu@fridaylinux.org; Maxime Coquelin; Tetsuya Mukawa;

> Yigit, Ferruh

> Subject: Re: [dpdk-dev] [PATCH v2 08/12] bus/vdev: scan and probe vdev in

> secondary processes

> 

> On Thu, Sep 28, 2017 at 3:55 PM, Jianfeng Tan <jianfeng.tan@intel.com>

> wrote:

> > Base on primary/secondary communication channel, we add vdev action

> > to scan virtual devices in secondary processes.

> >

> 

> This doesn't need to be in vdev. You application could just hotplug

> the drivers with exactly the same parameters as the primary process.

> The driver need to be aware of the primary/secondary process split

> though.

> 


This patch can make sure the virtual devices can be discovered and attached automatically by the secondary process. Of course, we can also add parameter to indicate if a vdev expects to be discovered and initialized automatically.
How do you think?

Thanks,
Jianfeng
  

Patch

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index e0ba0da..1d1690f 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -104,7 +104,7 @@  vdev_probe_all_drivers(struct rte_vdev_device *dev)
 
 	name = rte_vdev_device_name(dev);
 
-	VDEV_LOG(DEBUG, "Search driver %s to probe device %s\n", name,
+	VDEV_LOG(DEBUG, "Search driver %s to probe device %s", name,
 		rte_vdev_device_name(dev));
 
 	if (vdev_parse(name, &driver))
@@ -190,7 +190,7 @@  rte_vdev_init(const char *name, const char *args)
 	ret = vdev_probe_all_drivers(dev);
 	if (ret) {
 		if (ret > 0)
-			VDEV_LOG(ERR, "no driver found for %s\n", name);
+			VDEV_LOG(ERR, "no driver found for %s", name);
 		goto fail;
 	}
 
@@ -213,7 +213,7 @@  vdev_remove_driver(struct rte_vdev_device *dev)
 	const struct rte_vdev_driver *driver;
 
 	if (!dev->device.driver) {
-		VDEV_LOG(DEBUG, "no driver attach to device %s\n", name);
+		VDEV_LOG(DEBUG, "no driver attach to device %s", name);
 		return 1;
 	}
 
@@ -252,12 +252,105 @@  rte_vdev_uninit(const char *name)
 	return 0;
 }
 
+struct vdev_action_params {
+#define VDEV_SCAN_REQUEST	1
+#define VDEV_SCAN_RESPONSE	2
+	int type;
+	char name[32];
+};
+
+static int vdev_plug(struct rte_device *dev);
+
+static int
+vdev_action(const void *params, int len,
+	    int fds[] __rte_unused,
+	    int fds_num __rte_unused)
+{
+	struct rte_vdev_device *dev;
+	struct rte_devargs *devargs;
+	struct vdev_action_params ou_params;
+	const struct vdev_action_params *in_params = params;
+
+	switch (in_params->type) {
+	case VDEV_SCAN_REQUEST:
+		ou_params.type = VDEV_SCAN_RESPONSE;
+		TAILQ_FOREACH(dev, &vdev_device_list, next) {
+			VDEV_LOG(INFO, "push vdev, %s", dev->device.name);
+			strncpy(ou_params.name, dev->device.name, 32);
+			rte_eal_mp_sendmsg("vdev", &ou_params,
+							  len, NULL, 0);
+		}
+		break;
+	case VDEV_SCAN_RESPONSE:
+		VDEV_LOG(INFO, "get vdev, %s", in_params->name);
+
+		if (strlen(in_params->name) == 0) {
+			VDEV_LOG(ERR, "invalid name was passed");
+			break;
+		}
+
+		dev = find_vdev(in_params->name);
+		if (dev) {
+			VDEV_LOG(ERR, "vdev already exists: %s", in_params->name);
+			break;
+		}
+
+		devargs = alloc_devargs(in_params->name, NULL);
+		if (!devargs) {
+			VDEV_LOG(ERR, "failed to allocate memory");
+			break;
+		}
+
+		dev = calloc(1, sizeof(*dev));
+		if (!dev) {
+			VDEV_LOG(ERR, "failed to allocate memory");
+			free(devargs);
+			break;
+		}
+
+		dev->device.devargs = devargs;
+		dev->device.numa_node = 0; /* to be corrected in probe() */
+		dev->device.name = devargs->name;
+
+		TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
+		TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
+
+		if (vdev_plug(&dev->device) < 0) {
+			VDEV_LOG(ERR, "failed to plug device %s", in_params->name);
+			free(devargs);
+			free(dev);
+		} else
+			VDEV_LOG(INFO, "plug in device: %s", in_params->name);
+
+		break;
+	default:
+		VDEV_LOG(ERR, "vdev cannot recognize this message");
+	}
+
+	return 0;
+}
+
 static int
 vdev_scan(void)
 {
 	struct rte_vdev_device *dev;
 	struct rte_devargs *devargs;
 
+	if (rte_eal_mp_action_register("vdev", vdev_action) < 0) {
+		VDEV_LOG(ERR, "vdev fails to add action");
+		return -1;
+	}
+
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		struct vdev_action_params params;
+
+		params.type = VDEV_SCAN_REQUEST;
+		rte_eal_mp_sendmsg("vdev", &params,
+						  sizeof(params), NULL, 0);
+
+		return 0;
+	}
+
 	/* for virtual devices we scan the devargs_list populated via cmdline */
 	TAILQ_FOREACH(devargs, &devargs_list, next) {
 
@@ -287,6 +380,9 @@  vdev_probe(void)
 {
 	struct rte_vdev_device *dev;
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		return 0;
+
 	/* call the init function for each virtual device */
 	TAILQ_FOREACH(dev, &vdev_device_list, next) {
 
@@ -294,7 +390,7 @@  vdev_probe(void)
 			continue;
 
 		if (vdev_probe_all_drivers(dev)) {
-			VDEV_LOG(ERR, "failed to initialize %s device\n",
+			VDEV_LOG(ERR, "failed to initialize %s device",
 				rte_vdev_device_name(dev));
 			return -1;
 		}