Re: [PATCH v2 08/12] bus/vdev: scan and probe vdev in secondary processes

From: Jan Blunck <jblunck@infradead.org>
To: Jianfeng Tan <jianfeng.tan@intel.com>
Cc: dev <dev@dpdk.org>, Bruce Richardson <bruce.richardson@intel.com>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	yliu@fridaylinux.org,
	Maxime Coquelin <maxime.coquelin@redhat.com>,
	Tetsuya Mukawa <mtetsuyah@gmail.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v2 08/12] bus/vdev: scan and probe vdev in secondary processes
Date: Thu, 5 Oct 2017 15:04:31 +0200	[thread overview]
Message-ID: <CALe+Z02ecbFQF+SPbCrpXA-pnXopkwQy0PZSAKPZFnb-moKAbQ@mail.gmail.com> (raw)
In-Reply-To: <1506606959-76230-9-git-send-email-jianfeng.tan@intel.com>

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
>