From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on0065.outbound.protection.outlook.com [104.47.2.65]) by dpdk.org (Postfix) with ESMTP id EC692914D; Wed, 16 Aug 2017 11:02:32 +0200 (CEST) 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=3gGH2j1PDb8LQmezjuwwzdKyICKYlWDmVreY273FgR0=; b=RIhh8r6BoWvDRTrh4d51hb+yq6Xd4KXu3CY54vb3TsXeblgOvtNRl7wtjQDGXiusGGc94T2M5kLX1BtfN8dniZuWZNsx90bESWgwYHCx9MizmdgIfoOZiIA+KIKCHzR9UcS2eOQVBVTx/3k8KYtR5uRGfXQFBfr7Ry+4eFO4TL0= Received: from DB6PR0502MB3048.eurprd05.prod.outlook.com (10.172.250.136) by DB6PR0502MB3046.eurprd05.prod.outlook.com (10.172.247.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.1.1341.21; Wed, 16 Aug 2017 09:02:32 +0000 Received: from DB6PR0502MB3048.eurprd05.prod.outlook.com ([fe80::e938:efe9:effc:7ef8]) by DB6PR0502MB3048.eurprd05.prod.outlook.com ([fe80::e938:efe9:effc:7ef8%14]) with mapi id 15.01.1341.020; Wed, 16 Aug 2017 09:02:31 +0000 From: Matan Azrad To: =?iso-8859-1?Q?Ga=EBtan_Rivet?= CC: "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [PATCH] net/failsafe: fix tx sub device deactivating Thread-Index: AQHTFmwvljSnE/QXPEKxYcpWuzWhjKKGrWFg Date: Wed, 16 Aug 2017 09:02:31 +0000 Message-ID: References: <1502780359-60553-1-git-send-email-matan@mellanox.com> <20170816084629.GK8124@bidouze.vm.6wind.com> In-Reply-To: <20170816084629.GK8124@bidouze.vm.6wind.com> Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB6PR0502MB3046; 6:4viSyKGKQvxXa+i4nvuaiP0at+tSNTK09Mpl84VAm3Nfju/mFgB1mjnykr8+3Bn61Sa9rJaUh29OflqHT7jAVZxKyOnSzTEK3BDuPIXy2CNWzvkoqHBeDXl6htLmlHmhYRgFeZAX0/t/xRIK/PaFq/GZFqCVoblT0l66vwgFGXWInyoujNlPrEICpLwMMJ4/dzttulpoOEHGuHq6TFfRnufBrUmeVULboVzi4CltJ3mZxoqsK/4ZLr6RtbKjXykbwBl9fF6Q2xqwDTIv565qMrAKG9qoS3M3oMlqfEPG8P+EWalWei2oiBao8HQct/VLFbL099qBYGMPLnRe1EPMoA==; 5:K8kB+kmtTD5d/11QlPbp9JjEi2pj2DHTqe7Sba7MmvZv5I4n/6O6I1u+vIabvgIEF3joa2CNmlyUhtVfpsuZGufU3b582H6dSud0tdL7dfmDPeJvvg/EDuGz9P3SN31/3MoPG+sWNmpmBCJzTiXwlA==; 24:+rz+knxi3qPKmDD3UpV6r/C9/lLJvOzdhuxWCugqDh0lC/qdT1unLSagVx64AgXna2pT553jkDL7FSuNpYY61kl1/aQr7V+CJFg6x4EKQUI=; 7:3vVzfrvDcEnR9WJ+qboP71UxcnSqKW1OVul0r5uWqYY34E8eey9OVFnze0pnC40FfM+tu5QnrXTuBPguNjJNTFewiZ0yvvYzrGU7jLiYX1bshiGTWtM0gqLqrwfUeTW1xyCek5CGqfKr0r4Ud5ixaFMNYcjFuuGZ6OGkEtttAmUgUhTcX0Pc+k/CEGbFQWCYAw8WJTtQaIebzg8r2lX2hoxGQBuC0ExMNI4W+tGfLlA= x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: 5b776e0a-daf3-4e40-449a-08d4e485876c x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(48565401081)(2017052603031)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:DB6PR0502MB3046; x-ms-traffictypediagnostic: DB6PR0502MB3046: x-exchange-antispam-report-test: UriScan:(189930954265078)(45079756050767); x-microsoft-antispam-prvs: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(8121501046)(5005006)(10201501046)(100000703101)(100105400095)(93006095)(93001095)(3002001)(6055026)(6041248)(20161123564025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123560025)(20161123555025)(20161123558100)(20161123562025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:DB6PR0502MB3046; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:DB6PR0502MB3046; x-forefront-prvs: 0401647B7F x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(979002)(6009001)(39860400002)(189002)(377454003)(199003)(13464003)(24454002)(6436002)(966005)(45080400002)(2900100001)(2950100002)(229853002)(6916009)(5250100002)(74316002)(3846002)(54906002)(99286003)(9686003)(53936002)(6116002)(97736004)(102836003)(86362001)(55016002)(6306002)(6506006)(3280700002)(3660700001)(66066001)(8936002)(101416001)(5660300001)(110136004)(14454004)(53546010)(2906002)(6246003)(50986999)(189998001)(54356999)(478600001)(76176999)(106356001)(68736007)(105586002)(25786009)(81156014)(7736002)(305945005)(33656002)(8676002)(7696004)(4326008)(81166006)(969003)(989001)(999001)(1009001)(1019001); DIR:OUT; SFP:1101; SCL:1; SRVR:DB6PR0502MB3046; H:DB6PR0502MB3048.eurprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A: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-originalarrivaltime: 16 Aug 2017 09:02:31.8947 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0502MB3046 Subject: Re: [dpdk-stable] [PATCH] net/failsafe: fix tx sub device deactivating X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Aug 2017 09:02:33 -0000 Hi Gaetan > -----Original Message----- > From: Ga=EBtan Rivet [mailto:gaetan.rivet@6wind.com] > Sent: Wednesday, August 16, 2017 11:47 AM > To: Matan Azrad > Cc: dev@dpdk.org; stable@dpdk.org > Subject: Re: [PATCH] net/failsafe: fix tx sub device deactivating >=20 > Hi Matan, >=20 > Thanks for spotting this, a few nits below. >=20 > On Tue, Aug 15, 2017 at 09:59:19AM +0300, Matan Azrad wrote: > > The corrupted code couldn't recognize that all sub devices were not > > ready for tx traffic when failsafe PMD was trying to switch device > > because of an unreachable condition using. > > > > Hence, the current tx sub device variable was not updated correctly. > > > > The fix removed the unreachable condition and adds condition in the > > right place to handle non tx device ready scenario. > > >=20 > It should be reworded as >=20 > Make the condition reachable by moving it in the right place to > handle the scenario when no TX device is ready. >=20 > If the condition is removed and then added, I find it clearer to say that= it was > moved. But the two conditions are different, The old condition can't handle the scenario we want. =09 >=20 > > Fixes: ebea83f899d8 ("net/failsafe: add plug-in support") > > Fixes: 598fb8aec6f6 ("net/failsafe: support device removal") > > >=20 > The root commit introducing the issue is the first one, but this fix only= applies > to the second. > So I don't know which commit is actually fixed by this, but I find peculi= ar to > have two commits targeted by a fix. >=20 > In doubt, probably leave both, but maybe someone has a better idea about > it? I also thought about it, and found the two are necessary for future review.= =20 =20 >=20 > > Signed-off-by: Matan Azrad > > Cc: stable@dpdk.org >=20 > The Cc: stable line should immediately follow the Fixes: line. >=20 Will be fixed. > > --- > > drivers/net/failsafe/failsafe_private.h | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > Hi Gaetan > > I didn't find any real scenario which cause to problematic behavior > > because of the previous code. > > But it may be. > > > > diff --git a/drivers/net/failsafe/failsafe_private.h > > b/drivers/net/failsafe/failsafe_private.h > > index 0361cf4..dc97aec 100644 > > --- a/drivers/net/failsafe/failsafe_private.h > > +++ b/drivers/net/failsafe/failsafe_private.h > > @@ -346,9 +346,10 @@ fs_switch_dev(struct rte_eth_dev *dev, > > PRIV(dev)->subs_tx =3D i; > > break; > > } > > - } else if (txd && txd->state < req_state) { > > - DEBUG("No device ready, deactivating tx_dev"); > > - PRIV(dev)->subs_tx =3D PRIV(dev)->subs_tail; > > + if (i >=3D PRIV(dev)->subs_tail || !sdev) { >=20 > `!sdev` should be `sdev =3D=3D NULL`, see [1]. OK, will be fixed. >=20 > > + DEBUG("No device ready, deactivating tx_dev"); > > + PRIV(dev)->subs_tx =3D PRIV(dev)->subs_tail; > > + } > > } else { > > return; > > } > > -- > > 2.7.4 > > >=20 > With these changes, >=20 > Acked-by: Gaetan Rivet >=20 > [1]: > https://emea01.safelinks.protection.outlook.com/?url=3Dhttp%3A%2F%2Fdpd > k.org%2Fdoc%2Fguides%2Fcontributing%2Fcoding_style.html%23c- > statement-style-and- > conventions&data=3D02%7C01%7Cmatan%40mellanox.com%7C6283c71dcc2b4 > ebe5f6608d4e48350cd%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0% > 7C636384700025293702&sdata=3DnNMTElzhe3RlEMc3vB67QlwAYYYQ%2ByNNp > 9ebXgSsMM8%3D&reserved=3D0 > -- > Ga=EBtan Rivet > 6WIND Thanks=20 Matan Azrad