From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00088.outbound.protection.outlook.com [40.107.0.88]) by dpdk.org (Postfix) with ESMTP id 14D39D0C9; Wed, 9 May 2018 15:02:01 +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=Z1ATMrmVcqksYAyJSZJvoSRSh95SmPn7R25jW5MU9u0=; b=apVdeHSDGfPtnN+4pXDYWm8fyWPCvUUfktApBKX4XuFnoYkk932rdnZs7KYvPml/3HOYJgvkY4Xo7JSftzdt2wKK3LB5oz0+k8VF5/9swAUcU12Wf4j/HfNRHVEwwhvlbVkOhO/SH3mpm7jIUC0G+pdYllEn1euT6lcDiPpWPUs= Received: from VI1PR0501MB2608.eurprd05.prod.outlook.com (10.168.137.20) by VI1PR0501MB2016.eurprd05.prod.outlook.com (10.167.195.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.755.16; Wed, 9 May 2018 13:01:59 +0000 Received: from VI1PR0501MB2608.eurprd05.prod.outlook.com ([fe80::49ca:b540:8f36:c063]) by VI1PR0501MB2608.eurprd05.prod.outlook.com ([fe80::49ca:b540:8f36:c063%17]) with mapi id 15.20.0755.012; Wed, 9 May 2018 13:01:59 +0000 From: Matan Azrad To: Thomas Monjalon , =?iso-8859-1?Q?Ga=EBtan_Rivet?= CC: "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 10/11] net/failsafe: fix sub-device ownership race Thread-Index: AQHT53o/LYGsz+5LfEOdedAQczqcXqQnVx8AgAAFwLc= Date: Wed, 9 May 2018 13:01:58 +0000 Message-ID: References: <20180509094337.26112-1-thomas@monjalon.net> <20180509094337.26112-11-thomas@monjalon.net>, <20180509124123.un67tmsh75kxwrir@bidouze.vm.6wind.com> In-Reply-To: <20180509124123.un67tmsh75kxwrir@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: [52.164.245.112] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; VI1PR0501MB2016; 7:zGrjwF4p3EajclVk7uhI7VMnUqkpWfEzHPrIOpW6F/zLTOghIQdIOtMNDT+bK+bUGQVDfmdw2vPC9QjQS0v7Mhj2NZ3QrJ37dewoBDkHbJW9mAhzmu5z/Q4uHtKIvHON8QLujkci5xg3w6JAMgTCiVQCO42At9LzTSoKcsitFdBgQgqOkaqAJEAisKZku2jD01f/MFN1JVjIeasCzNvtykkryPsgeowI9RnPFX/U7IMpnNqG+lvbgC2wURCFo5PH x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(4534165)(4627221)(201703031133081)(201702281549075)(5600026)(48565401081)(2017052603328)(7153060)(7193020); SRVR:VI1PR0501MB2016; x-ms-traffictypediagnostic: VI1PR0501MB2016: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(60795455431006)(278428928389397); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3231254)(944501410)(52105095)(93006095)(93001095)(10201501046)(3002001)(6055026)(149027)(150027)(6041310)(20161123562045)(20161123558120)(20161123564045)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(6072148)(201708071742011); SRVR:VI1PR0501MB2016; BCL:0; PCL:0; RULEID:; SRVR:VI1PR0501MB2016; x-forefront-prvs: 0667289FF8 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39380400002)(39860400002)(376002)(346002)(366004)(396003)(189003)(199004)(4326008)(229853002)(97736004)(53936002)(6246003)(316002)(55016002)(7736002)(9686003)(54896002)(14454004)(186003)(6506007)(102836004)(99286004)(7696005)(76176011)(110136005)(81166006)(54906003)(8676002)(81156014)(26005)(8936002)(66066001)(6116002)(3846002)(3280700002)(2900100001)(3660700001)(2906002)(68736007)(74316002)(476003)(446003)(11346002)(25786009)(478600001)(486006)(33656002)(5250100002)(6436002)(86362001)(105586002)(106356001)(5660300001); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR0501MB2016; H:VI1PR0501MB2608.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: SCvkXYeKLN3w0GW3OSv95aVK2+nWsKA0DTR5UDCeTxLKo+oavP5ULly00DVoZjy/FilCvNwGz1RScb8xImMtUTjKFzvM8W4JEMmNygjuOHSwEMEx6jDw7RxxhCExPqv4/7WJUrLGxVwnPsxTiON4wUBel4rZ6J+96JnM2GUHV4LpcwDIKbDWUN4cLQGHvo7I spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 230494f4-3679-4a9b-75c3-08d5b5ad0cd6 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 230494f4-3679-4a9b-75c3-08d5b5ad0cd6 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 May 2018 13:01:58.9644 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0501MB2016 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 10/11] net/failsafe: fix sub-device ownership race 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: Wed, 09 May 2018 13:02:01 -0000 Hi Gaetan Regarding backporting. This version should be bacported for 18.02.1. There we have the new event. Regarding uint32 The maximum port id number can be 0xffff. In this case the loop will be infinite if we use uint16 to iterate over all= the ports. Hello Matan, Two nitpicks below: On Wed, May 09, 2018 at 11:43:36AM +0200, = Thomas Monjalon wrote: > From: Matan Azrad > > There is time between the su= b-device port probing by the sub-device PMD > to the sub-device port owners= hip taking by a fail-safe port. > > In this time, the port is available for= the application usage. For > example, the port will be exposed to the appl= ications which use > RTE_ETH_FOREACH_DEV iterator. > > Thus, ownership unaw= are applications may manage the port in this time > what may cause a lot of= problematic behaviors in the fail-safe > sub-device initialization. > > Re= gister to the ethdev NEW event to take the sub-device port ownership > befo= re it becomes exposed to the application. > > Fixes: a46f8d584eb8 ("net/fai= lsafe: add fail-safe PMD") This fix is relying on the RTE_ETH_EVENT_NEW, an= API that I think is not meant to be backported in the stable release that = would be targetted by this commit id. I think this fix is useless without t= he rest of this series, so I don't know what is exactly planned about the r= est (whether it is backported, and where), but I would only CC stable if th= is is planned, and only as soon as the relevant APIs are introduced. > Cc: = stable@dpdk.org > > Signed-off-by: Matan Azrad > --- > drivers/net/failsafe= /failsafe.c | 22 ++++++++++--- > drivers/net/failsafe/failsafe_eal.c | 58 += ++++++++++++++++++++------------ > drivers/net/failsafe/failsafe_ether.c | = 23 +++++++++++++ > drivers/net/failsafe/failsafe_private.h | 4 +++ > 4 file= s changed, 83 insertions(+), 24 deletions(-) > > diff --git a/drivers/net/f= ailsafe/failsafe.c b/drivers/net/failsafe/failsafe.c > index fc989c4f5..c9d= 128de3 100644 > --- a/drivers/net/failsafe/failsafe.c > +++ b/drivers/net/f= ailsafe/failsafe.c > @@ -204,16 +204,25 @@ fs_eth_dev_create(struct rte_vde= v_device *vdev) > } > snprintf(priv->my_owner.name, sizeof(priv->my_owner.n= ame), > FAILSAFE_OWNER_NAME); > + DEBUG("Failsafe port %u owner info: %s_%0= 16"PRIX64, dev->data->port_id, > + priv->my_owner.name, priv->my_owner.id);= > + ret =3D rte_eth_dev_callback_register(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, = > + failsafe_eth_new_event_callback, > + dev); > + if (ret) { > + ERROR("Fa= iled to register NEW callback"); > + goto free_args; > + } > ret =3D failsa= fe_eal_init(dev); > if (ret) > - goto free_args; > + goto unregister_new_ca= llback; > ret =3D fs_mutex_init(priv); > if (ret) > - goto free_args; > + g= oto unregister_new_callback; > ret =3D failsafe_hotplug_alarm_install(dev);= > if (ret) { > ERROR("Could not set up plug-in event detection"); > - goto= free_args; > + goto unregister_new_callback; > } > mac =3D &dev->data->mac= _addrs[0]; > if (mac_from_arg) { > @@ -226,7 +235,7 @@ fs_eth_dev_create(st= ruct rte_vdev_device *vdev) > mac); > if (ret) { > ERROR("Failed to set def= ault MAC address"); > - goto free_args; > + goto unregister_new_callback; >= } > } > } else { > @@ -261,6 +270,9 @@ fs_eth_dev_create(struct rte_vdev_d= evice *vdev) > }; > rte_eth_dev_probing_finish(dev); > return 0; > +unregis= ter_new_callback: > + rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_= EVENT_NEW, > + failsafe_eth_new_event_callback, dev); > free_args: > failsa= fe_args_free(dev); > free_subs: > @@ -280,6 +292,8 @@ fs_rte_eth_free(const= char *name) > dev =3D rte_eth_dev_allocated(name); > if (dev =3D=3D NULL) = > return -ENODEV; > + rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_= EVENT_NEW, > + failsafe_eth_new_event_callback, dev); > ret =3D failsafe_ea= l_uninit(dev); > if (ret) > ERROR("Error while uninitializing sub-EAL"); > = diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/fai= lsafe_eal.c > index ce767703f..8f1b9d845 100644 > --- a/drivers/net/failsaf= e/failsafe_eal.c > +++ b/drivers/net/failsafe/failsafe_eal.c > @@ -10,7 +10= ,7 @@ > static int > fs_ethdev_portid_get(const char *name, uint16_t *port_= id) > { > - uint16_t pid; > + uint32_t pid; I do not see why the port_id is= made uint32_t? Is there a reason? Otherwise all seems fine. With the prope= r justification or with uin16_t pid, and with a second pass on the backport= tagging, Acked-by: Gaetan Rivet Thanks, -- Ga=EBtan Rivet 6WIND