From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by dpdk.org (Postfix) with ESMTP id 50BDA14E8 for ; Thu, 14 Dec 2017 14:27:15 +0100 (CET) Received: by mail-wm0-f52.google.com with SMTP id f140so11399456wmd.2 for ; Thu, 14 Dec 2017 05:27:15 -0800 (PST) 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=W/a0yQU4xOLpCUOkSt70SsL+r1RWUAmuaLXnWIBybRg=; b=fHKMRolbK7gFZkgYIwfWv71G6TOVpFlQGvQ+aJvlCCh6RrTTshC+bfzSDTIsWSkfXs QChJAlgMgl3D70xSGLOO2SGJk25HHgs4L0xmMpR1L7B4/RSy1yjBNfaOMDV2Fuc0d+4q 0ZMvSu4oCPMz2IgeHghEa4mk7pHYFQX55NdBqjzpAIJyMc4hi6RUU0GW+oR9Yvle/kVe Sxl/6ki+GbZnwolvLl8plWrNss8JQ6T25qGuNFzYXz/VCENtNfi8PDRCIfGIbwqK1CeG Bq9IKfacg06N6LGDR1g/CUo/L3c1dm+q3OVpexAys7ZfOJ7A7MYL3O9EagoaKfwy71HR Fz7g== 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=W/a0yQU4xOLpCUOkSt70SsL+r1RWUAmuaLXnWIBybRg=; b=VpdNuZp3wzsInsRDz79NJZyicEWwzhu0fF0S5PiIg/O8AVzac1b/BmsAd0HF38UhmF iG/WLOn67pmOYvHaOPZDu1T1PHYun1gXKHjcTv8fvgzbbKOBR48a6MbE2ix9PLWjT/yu 247xAyiyHLcJSW+T1iLn9HUM+VEXndXEmRzFI8BB8qVbS9y6bjv/x0/6tUDh9lFyPFU0 HnSX2r8h0k0qmCF4QOworfkYyyIOhghVooEk+mL4hH18DaUk2iDBoPRfO7INWEw8Cu0I ID1wsk+TAR8XyOWd+LO865DT8nPhJ3P4h5aOXyZgTzngkmtWq7Fb0vNis09dv2zvOIuk KEkw== X-Gm-Message-State: AKGB3mL8P9haD7FLHHwo0J1gtsz5m4Sm9PWLlL7zH5FEIMeFTCok/7hq oojqXrwnBrMl+ripkaP3iY4uNA== X-Google-Smtp-Source: ACJfBotxsG7SzhetfC94vNcc81+2vOf9anRRHg1DUVksSt3YZCmu0mXqCYQqtCr50SA62teA0eNCyw== X-Received: by 10.80.149.141 with SMTP id w13mr12222606eda.79.1513258034882; Thu, 14 Dec 2017 05:27:14 -0800 (PST) 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 r8sm3097382edm.22.2017.12.14.05.27.13 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 14 Dec 2017 05:27:13 -0800 (PST) Date: Thu, 14 Dec 2017 14:27:01 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Matan Azrad Cc: Adrien Mazarguil , Thomas Monjalon , "dev@dpdk.org" , "stable@dpdk.org" Message-ID: <20171214132701.rwlyymuzvrl3tgsu@bidouze.vm.6wind.com> References: <1509637324-13525-1-git-send-email-matan@mellanox.com> <1513175370-16583-1-git-send-email-matan@mellanox.com> <1513175370-16583-5-git-send-email-matan@mellanox.com> <20171213151641.g42zr7zupbsdgxsv@bidouze.vm.6wind.com> <20171213160916.e3rmxmhfhqz72wco@bidouze.vm.6wind.com> <20171213215545.kywwximn2g5xm5x5@bidouze.vm.6wind.com> <20171214104856.d5qgnawuzb54l36z@bidouze.vm.6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v2 4/4] net/failsafe: fix removed device handling 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: Thu, 14 Dec 2017 13:27:15 -0000 On Thu, Dec 14, 2017 at 01:07:31PM +0000, Matan Azrad wrote: > Hi Gaetan > > > -----Original Message----- > > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com] > > Sent: Thursday, December 14, 2017 12:49 PM > > To: Matan Azrad > > Cc: Adrien Mazarguil ; Thomas Monjalon > > ; dev@dpdk.org; stable@dpdk.org > > Subject: Re: [PATCH v2 4/4] net/failsafe: fix removed device handling > > > > On Thu, Dec 14, 2017 at 10:40:22AM +0000, Matan Azrad wrote: > > > Hi Gaetan > > > > > > > > > Ok, actually you were right here to do it this way. The "is_removed" > > > > check needs to happen after the operation attempt to effectively > > > > mitigate the possible race. Checking before attempting the call will > > > > be much less effective. > > > > > > > > That being said, would it be cleaner to have eth_dev ops return > > > > -ENODEV directly, and check against it within fail-safe? > > > > > > > > > > I think that according to "is_removed" semantic we must return a Boolean > > value (Each value different from '0' means that the device is removed) like > > other functions in c library (for example isspace()). > > > > > > > Sure, I wasn't discussing the interface proposed by > > rte_eth_dev_is_removed(). > > > > What I meant was to ask whether checking rte_eth_dev_is_removed() > > would be more interesting in the ethdev layer, making the eth_dev_ops > > return -ENODEV regardless of the previous error if this check is supported by > > the driver and signal that the port is removed. > > > > I think this information could be interesting to other systems, not just fail- > > safe. > > > > Ok. Got you now. > Interesting approach - plan: > 1. update fs_link_update to use rte_eth* functions. I'm surprised it doesn't already. Either the rte_eth* function was introduced after the failsafe, or be wary of potential issues. I don't see a problem right now though. > 2. maybe -EIO is preferred because -ENODEV is used for no port error? Good point, didn't think about it. Prepare yourself maybe to some arguments about the most relevant error code. -EIO seems fine to me, but maybe use a wrapper for all this. Something like: ---8<--- static int eth_error(pid, int original_ret) { int ret; if (original_ret == 0) return original_ret; ret = rte_eth_is_removed(pid); if (ret == 0 || ret == -ENOTSUP) return original_ret; return -EIO; } int rte_eth_ops_xyz(pid) { int ret; ret = eth_dev(pid).ops_xyz(); return eth_error(pid, ret); } --->8--- This way you would be able to change it easily and the logic would be insulated. > 3. update all relevant rte_eth* to use "is_removed" in error flows(1 patch for flow APIs and 1 for the others). > 4. Change fs checks in error flows to check rte_eth* return values. > 5. Remove CC stable from commit massage. > > What do you think? > Agreed otherwise. Thanks, > > -- > > Gaëtan Rivet > > 6WIND -- Gaëtan Rivet 6WIND