From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f195.google.com (mail-wr0-f195.google.com [209.85.128.195]) by dpdk.org (Postfix) with ESMTP id 7488A2C1A for ; Wed, 12 Jul 2017 10:44:29 +0200 (CEST) Received: by mail-wr0-f195.google.com with SMTP id 77so4134133wrb.3 for ; Wed, 12 Jul 2017 01:44:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=3RjyTCXeDW9kH+NKEoFeAhQUGmhczsvaBQ+v0vv0lI8=; b=guba33ZtAxg0iORHRXDJ1EcO4c8Kp3gvR1WAUh4U6U7CFiT6Jw3naV5S6EapSsvSJg vFKdq8kvuDdbZ6THXcFcx7DXhPD20Hkw0Q8zCy1hsh22kfMAIDXf2I6KGNbLPMzA49og hNgqNwqJi7dFHZ//7z8NKKBWQPAvHQlihEgde7f7dE7k/7QOMBEeUAcTVvzNvBPQShh2 Laiou5XtwvyIlOlQcJweVBU2o7lJyuVYLkLTLiYI0GdQu2Mwh1NF6gGeBwqQrfKfdson VeN4A5XTaqJU3YDJc6FbvfUu2FS0Sg9mfSCdC+EWMR63dWh5UgeFtF86m7euVYXlN657 uoZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=3RjyTCXeDW9kH+NKEoFeAhQUGmhczsvaBQ+v0vv0lI8=; b=BBIIj91kLMO6CsnaUL1WU6x7vvNbsAJgd5k1zS+R8htJbUawNJ4/dlCS4iMLZ0zA8Z deVmPFB0Qtq/T0En7jguZh7w4hyAbMU+Jcb+M/9vYxTUHOIh71aMcSVFxigtAX7PI99W 8agJe7rRESxdVW6RJpe0rpXtT/wAjmsAmL0DcVIZ6r5DHZ77wRugsFY4AkTX7qGgqZJ2 j1V7etgQoJWOP9NJqlviT/hk30eG0hMrvDM5vZEMGhJ0oucpvzdAUe0aG4tq1LMtvRkJ e11jnlrpTpUxOQNrHciS9tihKvYaXGvYeveO26FUjtVQzx98vG7D14m3xoJeMMlFpf9h 0URQ== X-Gm-Message-State: AIVw112FHNjppcPLnZSOjeUFxwRIZWnPWqVAys32pLxALnaO/Dx1EB+w awCpf+aQheuKT/cISgD5ejFOiCYvsg== X-Received: by 10.223.142.143 with SMTP id q15mr2065925wrb.180.1499849069066; Wed, 12 Jul 2017 01:44:29 -0700 (PDT) MIME-Version: 1.0 Sender: jblunck@gmail.com Received: by 10.28.45.210 with HTTP; Wed, 12 Jul 2017 01:44:28 -0700 (PDT) In-Reply-To: <1eb5a98019675d97547ec3965738d23b350302ab.1499814957.git.gaetan.rivet@6wind.com> References: <1eb5a98019675d97547ec3965738d23b350302ab.1499814957.git.gaetan.rivet@6wind.com> From: Jan Blunck Date: Wed, 12 Jul 2017 04:44:28 -0400 X-Google-Sender-Auth: emOBhJfk_1sx6J-zGy00kUGKn8k Message-ID: To: Gaetan Rivet Cc: dev Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v3 4/8] eal: fix hotplug add / remove X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Jul 2017 08:44:29 -0000 On Tue, Jul 11, 2017 at 7:25 PM, Gaetan Rivet wrote: > The hotplug API requires a few properties that were not previously > explicitly enforced: > > - Idempotency, two consecutive scans should result in the same state. > - Upon returning, internal devices are now allocated and available > through the new `find_device` operator, meaning that they should be > identifiable. > > The current rte_eal_hotplug_add implementation identifies devices by > their names, as it is readily available and easy to define. > > The device name must be passed to the internal rte_device handle in > order to be available during scan, when it is then assigned to the > device. The current way of passing down this information from the device > declaration is through the global rte_devargs list. This only true for the virtual bus (vdev). > > Furthermore, the rte_device cannot take a bus-specific generated name, > as it is then not identifiable by the `find_device` operator. The device > must take the user-defined name. Ideally, an rte_device name should not > change during its existence. > > This commit generates a new rte_devargs associated with the plugged > device and inserts it in the global rte_devargs list. It consequently > releases it upon device removal. So unplugging a device which devargs have been passed via the command line is implicitly removing the command line parameter? This is a surprising side-effect and it sound wrong to do. What happens if the device is getting replugged? It seems to me that you want to make the hotplug add functionality behave like a force attach. You can achieve this by calling rte_eal_devargs_add/parse before calling into hotplug add. > Fixes: a3ee360f4440 ("eal: add hotplug add/remove device") > > Signed-off-by: Gaetan Rivet > --- > lib/librte_eal/common/eal_common_dev.c | 57 ++++++++++++++++++++++++++++++++-- > 1 file changed, 54 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c > index 32e12b5..f5566a6 100644 > --- a/lib/librte_eal/common/eal_common_dev.c > +++ b/lib/librte_eal/common/eal_common_dev.c > @@ -118,11 +118,32 @@ int rte_eal_dev_detach(struct rte_device *dev) > return ret; > } > > +static char * > +full_dev_name(const char *bus, const char *dev, const char *args) > +{ > + char *name; > + size_t len; > + > + len = strlen(bus) + 1 + > + strlen(dev) + 1 + > + strlen(args) + 1; > + name = calloc(1, len); > + if (name == NULL) { > + RTE_LOG(ERR, EAL, "Could not allocate full device name\n"); > + return NULL; > + } > + snprintf(name, len, "%s:%s,%s", bus, dev, > + args ? args : ""); > + return name; > +} > + > int rte_eal_hotplug_add(const char *busname, const char *devname, > const char *devargs) > { > struct rte_bus *bus; > struct rte_device *dev; > + struct rte_devargs *da; > + char *name; > int ret; > > bus = rte_bus_find_by_name(busname); > @@ -137,21 +158,49 @@ int rte_eal_hotplug_add(const char *busname, const char *devname, > return -ENOTSUP; > } > > + name = full_dev_name(busname, devname, devargs); > + if (name == NULL) > + return -ENOMEM; > + > + da = calloc(1, sizeof(*da)); > + if (da == NULL) { > + ret = -ENOMEM; > + goto err_name; > + } > + > + ret = rte_eal_devargs_parse(name, da); > + if (ret) > + goto err_devarg; > + > + ret = rte_eal_devargs_insert(da); > + if (ret) > + goto err_devarg; > + > ret = bus->scan(); > if (ret) > - return ret; > + goto err_devarg; > > dev = bus->find_device(NULL, cmp_detached_dev_name, devname); > if (dev == NULL) { > RTE_LOG(ERR, EAL, "Cannot find unplugged device (%s)\n", > devname); > - return -EINVAL; > + ret = -ENODEV; > + goto err_devarg; > } > > ret = bus->plug(dev, devargs); > - if (ret) > + if (ret) { > RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", > dev->name); > + goto err_devarg; > + } > + free(name); > + return 0; > + > +err_devarg: > + rte_eal_devargs_remove(busname, devname); > +err_name: > + free(name); > return ret; > } > > @@ -179,6 +228,8 @@ int rte_eal_hotplug_remove(const char *busname, const char *devname) > return -EINVAL; > } > > + rte_eal_devargs_remove(busname, devname); > + dev->devargs = NULL; > ret = bus->unplug(dev); > if (ret) > RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n", > -- > 2.1.4 >