From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f43.google.com (mail-wm0-f43.google.com [74.125.82.43]) by dpdk.org (Postfix) with ESMTP id 5507E137C for ; Tue, 29 Aug 2017 10:18:28 +0200 (CEST) Received: by mail-wm0-f43.google.com with SMTP id y71so16066794wmd.0 for ; Tue, 29 Aug 2017 01:18:28 -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=Ic7V6Fpgr1WvTb9DkjxBVs5o0PAccKao4ACBOXif3O4=; b=QaYc8nKTmXQ7WTtb5C8AhShIGVbCOYQtg/J1JBBt1bjzMTzpUHMiU+R10Zpgx11jNF 3pH4co8Ff0Y+qSVhsJSTFqTtNgAbD2UNNE73VviScFfYk6lmMtfl1bkhzuPnxvrBKa9g QTRnJtTym+5UChzZSVUpwzHePFHz21ChrFG5+XFn5vYaGU+z+3epF3gSNs/ji27zKYD1 hTYkfmDtnEZUtP1yDVxYZrw4v5GbMr3TPlZLYzddno2XjEiLAOa32DKfIAP0OAg0BJSI EMqMrquCRVusSrnFkqmxsC+FNwKlkc/DZdqXRbrz7MkXDVLP3zBmoymn9TvU8BqIj+3I kFLw== 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=Ic7V6Fpgr1WvTb9DkjxBVs5o0PAccKao4ACBOXif3O4=; b=jxeAEGAxThwMRztBjdEc7gQ6n2/PRitQRgiqcpGyHpzuxG2G4uiO9wqNN6t1emCjYX 9QxlnvHOwuVVtv+tW9Tqf2K7cQ0+mIgtraq8Pnbbi6SEdWddXZy4XNmRtv2GLHbbZe2+ lKJ08pgtAOyq5Fffrx6XEMXw2Kto6cR0/ifPyVK9UxKAeLWcDKpw9+HeQ9ZIQtMSMpE6 1gzfV1vQwMsgpZBpUGYzLp7GlcZ84JcB442GGxOCDp5Ii9zN6dvFT8fD9m1fk4YB5J42 Eg/cAFaeferC58+Bk/cFq3DI5soIf874fXUOoFbxEA8Ng8Ow66H/od8RLY66ZkbW3ZOU VnZw== X-Gm-Message-State: AHYfb5ggIcXlUUGfnjX9OaKqzx3FkMWXoZtMGH0xTE9O51D+GwS3knUA WPVGJc5EN5K23F5Z X-Received: by 10.28.238.197 with SMTP id j66mr1583608wmi.170.1503994707710; Tue, 29 Aug 2017 01:18:27 -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 v60sm3381424wrc.71.2017.08.29.01.18.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 29 Aug 2017 01:18:26 -0700 (PDT) Date: Tue, 29 Aug 2017 10:18:17 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Raslan Darawsheh Cc: thomas@monjalon.net, dev@dpdk.org Message-ID: <20170829081817.GI8124@bidouze.vm.6wind.com> References: <1503993089-55121-1-git-send-email-rasland@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1503993089-55121-1-git-send-email-rasland@mellanox.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] net/failsafe: fix failsafe bus uninit return value 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: Tue, 29 Aug 2017 08:18:28 -0000 Hi Raslan, On Tue, Aug 29, 2017 at 10:51:29AM +0300, Raslan Darawsheh wrote: > fs_bus_uninit is always returning 0 no matter what was the status > of each sub device bus_uninit value. An error on fs_bus_uninit should not stop failsafe_eal_uninit from finishing its operation. So for the sake of internal consistency for this scope, it might be acceptable to yield an error. However, if you do so, you then need to ignore the error within failsafe_eal_uninit, so that the fail-safe device state is also properly set. something like: diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c index c8f4318..d144719 100644 --- a/drivers/net/failsafe/failsafe_eal.c +++ b/drivers/net/failsafe/failsafe_eal.c @@ -111,8 +111,6 @@ failsafe_eal_uninit(struct rte_eth_dev *dev) int ret; ret = fs_bus_uninit(dev); - if (ret) - return ret; PRIV(dev)->state = DEV_PROBED - 1; - return 0; + return ret; } > > Will now return the first sub device fail value in case it fails. > > Fixes: a46f8d58 ("net/failsafe: add fail-safe PMD") > > Signed-off-by: Raslan Darawsheh > --- > drivers/net/failsafe/failsafe_eal.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c > index c8f4318..4295347 100644 > --- a/drivers/net/failsafe/failsafe_eal.c > +++ b/drivers/net/failsafe/failsafe_eal.c > @@ -90,19 +90,20 @@ fs_bus_uninit(struct rte_eth_dev *dev) > { > struct sub_device *sdev = NULL; > uint8_t i; > - int ret; > + int ret = 0, sdev_ret; No second declaration on the same line if one is initialized please. > > FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) { > - ret = rte_eal_hotplug_remove(sdev->bus->name, > + sdev_ret = rte_eal_hotplug_remove(sdev->bus->name, > sdev->dev->name); > - if (ret) { > + if (sdev_ret) { > ERROR("Failed to remove requested device %s", > sdev->dev->name); I think printing the error code might be useful, at least it would not be completely lost. > + ret = (ret ? ret : sdev_ret); ret is meaningless anyway, as it might represent any sub_device removal error. I'd set it to -1 if sdev_ret is displayed to avoid any unnecessary confusion. > continue; > } > sdev->state = DEV_PROBED - 1; > } > - return 0; > + return ret; > } > > int > -- > 2.7.4 > Best regards, -- Gaëtan Rivet 6WIND