From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0052.outbound.protection.outlook.com [104.47.1.52]) by dpdk.org (Postfix) with ESMTP id CF5DC14E8; Thu, 14 Dec 2017 15:43:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=dUkVKqLwOd/BFIxo41o0ibZCHefI4vyivFjBhvRt0Bc=; b=frOi1oq66Q5xI6ye5Fwrqbj9nN5215sIXk+g0nD7HXXb4JWTzn9CtC2yK7V8rejXmxydCcktQQd9WpzzTSAh0lPAEm3khYOojAn3zl1v1RnEKR75TfHWL6v0GuQjbmxlSWErybfqOrQ1cVsZUZyIi0WirYDIat6VtepZoECe3no= Received: from HE1PR0502MB3659.eurprd05.prod.outlook.com (10.167.127.17) by HE1PR0502MB3660.eurprd05.prod.outlook.com (10.167.127.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.282.5; Thu, 14 Dec 2017 14:43:04 +0000 Received: from HE1PR0502MB3659.eurprd05.prod.outlook.com ([fe80::982e:2dce:9449:6891]) by HE1PR0502MB3659.eurprd05.prod.outlook.com ([fe80::982e:2dce:9449:6891%13]) with mapi id 15.20.0282.012; Thu, 14 Dec 2017 14:43:04 +0000 From: Matan Azrad To: =?iso-8859-1?Q?Ga=EBtan_Rivet?= CC: Adrien Mazarguil , Thomas Monjalon , "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [PATCH v2 4/4] net/failsafe: fix removed device handling Thread-Index: AQHTdCVqeFZYAzJ0Kk2VOrfpqCAdSKNBZw+wgAAKGACAAGDPgIAA0g4ggAAF+ACAACKyYIAACXmAgAAS5TA= Date: Thu, 14 Dec 2017 14:43:03 +0000 Message-ID: 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> <20171214132701.rwlyymuzvrl3tgsu@bidouze.vm.6wind.com> In-Reply-To: <20171214132701.rwlyymuzvrl3tgsu@bidouze.vm.6wind.com> Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; HE1PR0502MB3660; 6:YsN64qT5TowPal0a0rhMT2oTNVu+wDAGxDm19TOkkNITvUWdfEtkCIJfI+g9hl2jgLMoDvWv3tf6W6VyS0AIfhe+LcERrpbK5p8sa6kVd5qnrR7wMu44RpzVTYwhMA5PmXZuuUuu+vj6E9eMzj+Kc7LoM9jKuOHEDwbBahzhWRGi2T6WL9rik+1xV6kBH0FsYajqlOHXvcv1d723CsP7i9sQyayXbJh9jY+wsRwE7USpjgU1ZHm8LAaSKDP87QlMlYOP5jF8ssazFZL7dfI3YyRtdjICwOadUUqc0NMq+5Il9+QMTY1VDaSJCEURfX40yx+cNhzZt3c6f3NQNbo3nlcHIfzqJ9pg1bKuwtH5kkQ=; 5:R67lxXE2y1Zx44YcJuW5UARIaGr/+OmS3B16aD34DzE4/9XhiH0h0NpHr7YFxcgYEGCd4NSKJ/SZ1gZpUHXVpHSegPtiXx6hA3qqKV5hdOGj1/z09lz1mgKQRUjHYenjX0mFD+7iLOvNBmwhu+ktPsOi40CxCmVHGGa5hor+7AY=; 24:He8gxVQlu9QP+yskdxoy3YIYdxVcqzjYwFbzb+4NtJDWGKmR1fXnATCjlBEWFoTTtdNCvq4m0y1eKe//7PNcGbdJFfgeCJayzsG7Ml6gjkQ=; 7:2Omc9CWliPIhbQDFnYiphxvZjmsK+I1Wg/MWyWk5NYHZu+1blM8ZRTxB+pOtor7gPw/XKQzUSe0wxFUHzCWFqGi8aV1BenM56zag4H0QYPaXxhS4PyUh3HOqmfD+jrPjSFR1QYYMOqB3Ba7L157gWMkowW+Kaf+sIxTdl0tGi1ph0uHrj8A88ean2yVtyL5s6aIZd2MIUF8oFRRkw5S4apVx2PrfUiMSx3xJvzFuBVEjNzUiIXOx8WddLV7+T2sv x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: 3a96e438-b02f-48e5-f9dd-08d54300fb70 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(5600026)(4604075)(48565401081)(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(2017052603307); SRVR:HE1PR0502MB3660; x-ms-traffictypediagnostic: HE1PR0502MB3660: authentication-results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040450)(2401047)(5005006)(8121501046)(3231023)(3002001)(10201501046)(93006095)(93001095)(6055026)(6041248)(20161123564025)(20161123558100)(20161123555025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123562025)(6072148)(201708071742011); SRVR:HE1PR0502MB3660; BCL:0; PCL:0; RULEID:(100000803101)(100110400095); SRVR:HE1PR0502MB3660; x-forefront-prvs: 05214FD68E x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(346002)(376002)(396003)(366004)(76104003)(199004)(189003)(24454002)(51444003)(13464003)(6916009)(93886005)(2950100002)(3660700001)(33656002)(54906003)(5660300001)(59450400001)(76176011)(5250100002)(4326008)(74316002)(2900100001)(6506007)(81156014)(66066001)(2906002)(55016002)(8676002)(316002)(106356001)(81166006)(105586002)(8936002)(9686003)(68736007)(53936002)(3280700002)(86362001)(14454004)(229853002)(478600001)(7696005)(99286004)(305945005)(25786009)(6116002)(7736002)(97736004)(102836003)(3846002)(53546011)(6436002)(6246003); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR0502MB3660; H:HE1PR0502MB3659.eurprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3a96e438-b02f-48e5-f9dd-08d54300fb70 X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Dec 2017 14:43:03.8933 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0502MB3660 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 14:43:06 -0000 Hi > -----Original Message----- > From: Ga=EBtan Rivet [mailto:gaetan.rivet@6wind.com] > Sent: Thursday, December 14, 2017 3:27 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 >=20 > On Thu, Dec 14, 2017 at 01:07:31PM +0000, Matan Azrad wrote: > > Hi Gaetan > > > > > -----Original Message----- > > > From: Ga=EBtan 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 > > > > > > > >=20 > >=20 > > > > > Ok, actually you were right here to do it this way. The "is_remov= ed" > > > > > 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. >=20 > I'm surprised it doesn't already. > Either the rte_eth* function was introduced after the failsafe, or be war= y of > potential issues. I don't see a problem right now though. >=20 > > 2. maybe -EIO is preferred because -ENODEV is used for no port > error? >=20 > 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. >=20 > Something like: >=20 > ---8<--- >=20 > static int > eth_error(pid, int original_ret) > { > int ret; >=20 > if (original_ret =3D=3D 0) > return original_ret; > ret =3D rte_eth_is_removed(pid); > if (ret =3D=3D 0 || ret =3D=3D -ENOTSUP) > return original_ret; > return -EIO; > } >=20 > int > rte_eth_ops_xyz(pid) > { > int ret; > ret =3D eth_dev(pid).ops_xyz(); > return eth_error(pid, ret); > } >=20 > --->8--- >=20 > This way you would be able to change it easily and the logic would be > insulated. >=20 Nice. > > 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? > > >=20 > Agreed otherwise. >=20 Will create V3, thanks! > Thanks, >=20 > > > -- > > > Ga=EBtan Rivet > > > 6WIND >=20 > -- > Ga=EBtan Rivet > 6WIND