From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f174.google.com (mail-wr0-f174.google.com [209.85.128.174]) by dpdk.org (Postfix) with ESMTP id 80B6C2BF5 for ; Wed, 12 Jul 2017 22:40:07 +0200 (CEST) Received: by mail-wr0-f174.google.com with SMTP id 77so43782097wrb.1 for ; Wed, 12 Jul 2017 13:40:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=W7WWT+LbGw5vDkrOx/H50Pki36pbq4vFtAgwD9j92yY=; b=vfjclntXPE8gHvQD3j4O1k+dShEHb7OUZtfjMeYpTsfhimgnQ5L8mbBPM0xh0lXh6n +1x15kOVPgtpb8OkOYaPdvEPMCTrRdqelaQXEMSGdGOKKaNgKDm1jQvJpt+emLHZtFbL WO0Vmff9EhZar791oVU6UbDAeOSFxo+sFkxENGl7wTMLO2bND1e58tWkg9oHnnE02PL7 YmSMFiXgVwSdWP4rQsSVGvXh2m6KGYeU8yApi1OCb5I8Fy5wO9XUrQKrJNNHFx6ZoUEw pJoPiz48HV2NVoyc6JRGJ1IZi+no1VHechYRKwxZQJzCsHGAvMKaXHD7OXJyK9xWubhl Tt2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=W7WWT+LbGw5vDkrOx/H50Pki36pbq4vFtAgwD9j92yY=; b=qmWx0ojQCD1kNDbcFY28LtEaB1hvgVbKIcpYcK+xCzgP9FY7un2RhRphpu57wGlVV0 r9MeC6VYpochXkaPvYL1y/duw4cfD/YRGCQFydRYSFyuZqewmiFSISNAeqiU5IQXpcKg fTR02wYrVk4/4xm/6W/ipRH26+WH7L+keYbrZOlWCFecTyAyBobCKu7I5fF/gr39KLWL PRt5nbba0BKJB+rlf9u7QHriuYgWY1+Y0KIfn2cn2VB4Md9N1XRd+P5nhKZOzslwahyj p4irFQTUOtObvabQYjKFmn4sbe+o8FzHc3XHsRxEPKqMnD++L81Un+cn5EYnu0vY0b93 DGxw== X-Gm-Message-State: AIVw110VZE7tFqJf2G/cKIWR5z4ayH+KLRpjQC43wUxOVZqiC9WyMKl4 tO9BVg1GBM1yZn7J+1g= X-Received: by 10.223.177.201 with SMTP id r9mr188309wra.36.1499892006941; Wed, 12 Jul 2017 13:40:06 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 55sm4420048wrt.36.2017.07.12.13.40.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Jul 2017 13:40:05 -0700 (PDT) Date: Wed, 12 Jul 2017 22:39:57 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Thomas Monjalon Cc: dev@dpdk.org Message-ID: <20170712203957.GG11154@bidouze.vm.6wind.com> References: <20170712182812.18404-1-thomas@monjalon.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170712182812.18404-1-thomas@monjalon.net> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] net/failsafe: do not probe device if plugged out 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 20:40:07 -0000 Hi Thomas, Nice idea. A few remarks below: On Wed, Jul 12, 2017 at 08:28:12PM +0200, Thomas Monjalon wrote: > When probing a sub-devices which does not exist in the system, > some errors are logged: > EAL: Cannot find unplugged device (0002:00:02.0) > PMD: net_failsafe: ERROR: sub_device 0 probe failed > > It is normal to have these errors when initializing the failsafe > device and its sub-devices. > But we do not need to log them at each probe. > And it is even less relevant if the sub-device has been plugged out. > > The unavailable devices are skipped after the first probe, considering > two exceptions: > - The vdevs are always probed because they do not really exist on the > virtual bus before probing them. > - The sub-device list given by an executed command line may change > from one call to the next one, therefore it is considered to be always > the first one. Anyway, such external command should check the > device availability before passing it to the failsafe PMD. > > Signed-off-by: Thomas Monjalon > --- > This patch is on top of the failsafe series which is > pending for integration in 17.08-rc2. > --- > drivers/net/failsafe/failsafe_eal.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c > index 3321dda71..5181066ed 100644 > --- a/drivers/net/failsafe/failsafe_eal.c > +++ b/drivers/net/failsafe/failsafe_eal.c > @@ -49,6 +49,11 @@ fs_find_ethdev(const struct rte_device *dev) This needs to be rebased on the latest version. Sorry I forgot to supersede the previous one. > return NULL; > } > > +static int cmp_dev_name(const struct rte_device *dev, const void *name) > +{ > + return strcmp(dev->name, name); > +} > + > static int > fs_bus_init(struct rte_eth_dev *dev) > { > @@ -56,12 +61,24 @@ fs_bus_init(struct rte_eth_dev *dev) > struct rte_device *rdev; > struct rte_devargs *da; > uint8_t i; > + static int first_init = 1; I did not do that before within this PMD, but a boolean would be better there. > int ret; > > FOREACH_SUBDEV(sdev, i, dev) { > if (sdev->state != DEV_PARSED) > continue; > da = &sdev->devargs; > + Superfluous line. > + /* skip plugged out devices */ > + if (! first_init > + && sdev->cmdline == NULL > + && strcmp(da->bus->name, "vdev") != 0) { Use first_init == false instead of negation. && should be at the end of the line instead of the start of the next one. Indentation is wrong. > + da->bus->scan(); > + if (bus->find_device(NULL, cmp_dev_name, da->name) == NULL) > + continue; /* device not found */ da->bus->find_device instead of bus->find_device. This function cannot find the device back currently on the PCI bus, blocking the plugging of VF. The PCI bus will scan the VF while no rte_devargs exists to describe it within the global list. If the device exists, it will detect it, allocate it and then set its name. Without any rte_devargs, the name of a PCI device falls back to its canonical name (DomBDF instead of BDF). The name comparison with da->name can only succeed if the slave was declared using the DomBDF format. The fix is to do a deep copy of the rte_devargs (the API has been sent previously with the rte_devargs rework but I have since removed it) and insert it using rte_eal_devargs_insert(). This is essentially the solution I used for the rte_eal_hotplug_add() fix[1]. The alternative fix is to propose an API for buses to transform device names into their canonical form on demand... And it would certainly only be useful for the PCI bus. The issue is discussed there: [1]: http://dpdk.org/ml/archives/dev/2017-July/071155.html > + } > + > + /* probe device */ > rdev = rte_eal_hotplug_add(da->bus->name, > da->name, > da->args); > @@ -84,6 +101,10 @@ fs_bus_init(struct rte_eth_dev *dev) > ETH(sdev)->state = RTE_ETH_DEV_DEFERRED; > sdev->state = DEV_PROBED; > } > + Superfluous line. > + if (first_init) > + first_init = 0; first_init = false; > + > return 0; > } > > -- > 2.13.2 > -- Gaëtan Rivet 6WIND