From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <matan@mellanox.com>
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 <matan@mellanox.com>
To: =?iso-8859-1?Q?Ga=EBtan_Rivet?= <gaetan.rivet@6wind.com>
CC: Adrien Mazarguil <adrien.mazarguil@6wind.com>, Thomas Monjalon
<thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>, "stable@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: <HE1PR0502MB3659BBD4B2681DB8F12516FBD20A0@HE1PR0502MB3659.eurprd05.prod.outlook.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>
<HE1PR0502MB3659B4A6797E3A7A2FC78834D2350@HE1PR0502MB3659.eurprd05.prod.outlook.com>
<20171213160916.e3rmxmhfhqz72wco@bidouze.vm.6wind.com>
<20171213215545.kywwximn2g5xm5x5@bidouze.vm.6wind.com>
<HE1PR0502MB36596023F82792ABB3D84C7CD20A0@HE1PR0502MB3659.eurprd05.prod.outlook.com>
<20171214104856.d5qgnawuzb54l36z@bidouze.vm.6wind.com>
<HE1PR0502MB365936804A133CBE6BEAA5D7D20A0@HE1PR0502MB3659.eurprd05.prod.outlook.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: <HE1PR0502MB3660B1B93D4869BC3D86FB04D20A0@HE1PR0502MB3660.eurprd05.prod.outlook.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <matan@mellanox.com>
> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon
> <thomas@monjalon.net>; 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 <matan@mellanox.com>
> > > Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; 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
> <snip>
>=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