From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 310F7A04DD; Tue, 20 Oct 2020 23:39:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8D446AC6C; Tue, 20 Oct 2020 23:39:31 +0200 (CEST) Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-eopbgr750108.outbound.protection.outlook.com [40.107.75.108]) by dpdk.org (Postfix) with ESMTP id A8149A9D9; Tue, 20 Oct 2020 23:39:29 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=I2Nqc1ien1/6rwtc+9d/m50ETPadoHj1DP1hCsHOlQl/cw95O0Ugy/9f5tw8GFARHWI0BFhJsbo7C6XBnDRcnITsEv6WKPg+Zl+y3T5Buu9Qg74s24aeg+KGj4LlG92WRseFd2WHEVd0SJYJSYs0cEM4EI24AXSHOrE4EJEKMY6B07qrsfrw1N6zpVD3JuCrgmz+ONLGnA8pHkOGp7W8JZxckCHZ2Ugwz1iczyoeOgabm7c6v8cTPU8tdyEceEXYKbKBjnx1cqdc3yNSfq2+0NkPAiU5JqUfBkjW5nb0fcA00Qdjio7StJ32rVvd+tzoas8t7+U2P7DRJ5B5h82UKA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uYKV27cBB1a6LMFLI5LJMnddxiWhaTDaDvriWRRjUnI=; b=VMjRW7AEdZtkR+ddIPMm9vJC1/AzmOaAesavZB8brOUQJDrHBXZjv0nWS0G1WkCHuTnpWYVDOQ8tn8aBCZ9Wl95A6WJ6NMnu0CTk2dAHOuDlmm9ZfUWUEnJCk/EapbTSh1dF2GFCXrLmkanNeWeca0GYpbbxIOjBQzrLBENz4wvdKakVe3d3m8KdW4DcEegBVCzsdsiY7gIr5xZxrl5TyNLCsey4hb48IsnS7dLoKcnyphQhiafln0/VIVSsd0sx8V5nb27Fdz/IifTvuXwNnpgV9q9rnJGU8TiJ/pRbHnqgm6Jit7Di5HF2rO5E+20DMlRUbh61v7XPnjikuZjcJQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uYKV27cBB1a6LMFLI5LJMnddxiWhaTDaDvriWRRjUnI=; b=e1y23ywHSDKEhP5HcwuktlI90yfyp79ra1eW4cHK46axTRec2bccT+UGNXk1e4MeBDHxnVS0lnIrl85oE0e490GDJMHVAgLjiT+q+RcaEAeArJbY7buQkLjGd3hreVtg6M/FdG5Q+YKa70whbBmFU/K6X8aLW735CSCsNGduT20= Received: from BN8PR21MB1155.namprd21.prod.outlook.com (2603:10b6:408:73::10) by BN6PR21MB0132.namprd21.prod.outlook.com (2603:10b6:404:93::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.3; Tue, 20 Oct 2020 21:39:27 +0000 Received: from BN8PR21MB1155.namprd21.prod.outlook.com ([fe80::ccf4:7bc9:ac7a:1a19]) by BN8PR21MB1155.namprd21.prod.outlook.com ([fe80::ccf4:7bc9:ac7a:1a19%3]) with mapi id 15.20.3499.003; Tue, 20 Oct 2020 21:39:27 +0000 From: Long Li To: Matan Azrad , Long Li , Matan Azrad CC: "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [PATCH] net/vdev_netvsc: Prevent alarm lost on failed device probe Thread-Index: AQHWoyoSSzzOBT3KK0CKZ8fVsWTK46mc+vKAgAJshoCAAJwjAIABCN0w Date: Tue, 20 Oct 2020 21:39:26 +0000 Message-ID: References: <1602790385-13032-1-git-send-email-longli@linuxonhyperv.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ActionId=99fd5064-0836-40ac-9406-14ab91a2ce9f; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=true; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=Internal; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2020-10-19T20:32:01Z; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47; authentication-results: nvidia.com; dkim=none (message not signed) header.d=none;nvidia.com; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [67.168.111.68] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 6af0b294-28c4-473f-f927-08d875409e8b x-ms-traffictypediagnostic: BN6PR21MB0132: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:6108; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Zoyig0YYL11ShWgJcu123h4wPaieKxQKLSOaySspb1hAfdL2gmow72O0WnXtiUGuZHofEZfuUcfHhUyp6OKsDMqUWe2xaJhIcDzZ8925nedl5SxKGRhRVIxcJYMp2ENtrrOcXlVac7vAp8ECeIw6dqZ+82f7WoAYCch5kbaVr3lJIIQWQJO/ABnhVa902FTDpD4FlDxbfB8HsJhPLCF10fs95RHxbzxNScaiGFTwriCj1mJBaxpj3hOODroDCu3s7d7fnoRwuuWfx+FWW5NRfd02XBJ1XFv8ywpHN3FW23Hs1c5RQlRoUUV1FPsPKR6Jjz8V4xXFVvnBxH0WEhI2fQ== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BN8PR21MB1155.namprd21.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(39860400002)(396003)(376002)(346002)(136003)(5660300002)(8936002)(66446008)(8990500004)(478600001)(110136005)(66476007)(6506007)(2906002)(64756008)(54906003)(76116006)(66556008)(82960400001)(8676002)(10290500003)(7696005)(82950400001)(86362001)(186003)(52536014)(33656002)(316002)(4326008)(66946007)(55016002)(83380400001)(9686003)(26005)(71200400001); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: FYKcjFjhx/nw6KXzy2+rCZ2Gh/3zzcdPUi+TFx1pabRpjkHjxA1ZxOc5GHN0O60GVm0R2f3WA2J6pm7noROZnM9MXNUESmoMcw8S39ZGU1dbPvj1dvSwVGQP5D6Q0Wkt8DBHUptMCqQ+ZnIWXJvDN0kJf41fFHyq7fUrhZeKJ1YB8snRAunGpEWNgegRPGLHVZWDVqCw0CREPiVcdIFhVGEQ4XMDFsvTPAX9+Oaj7CkcLC0txdSGMqHFRLcL94+M2ktgb2XNwIdCQV6cH3yyK768cSLDKJ32hzsu1eAjkq4raVN0xxSZDc1nZg9CgLiVTpS25BfJhb138FH+Am10dY2rLbxrjiL8/OPqP+fERME1bWmrF6o/gjjJxRo/ltnvFf22hnQa7hByugoqu/7iSRNHok8aH0Pl4Zx988FX6vXEltDYEFf5hZvoh2XuxTWg6acAcnIDsp4SaRNDnNP8iHy+2Zc4fFZfc0HCRVOEe88t0Afb0Wg8etJXVsSzqOUsVT2+6avRA8s1kKGjCGKC+WBUTvYPt68FPMMuWrlRZm3ScUEKDyyatjDsMROV6I120uQPR89oQTaCW1ohohSkIL3/Z2xAc4+lbQ9uzW9d/FCVDrXwqzZEgUBEv829aS4IRrw7mv2LOTlg7burvCWMAA== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BN8PR21MB1155.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6af0b294-28c4-473f-f927-08d875409e8b X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Oct 2020 21:39:27.0225 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 4fjiWx/xZuP1tjyF6WhpBzRJnWevYIvyKHn/f2akalMH1ARKp6OrBJG90c/Vmjzqw/MhVf+SFuo+Xtwc7v06/A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR21MB0132 Subject: Re: [dpdk-dev] [PATCH] net/vdev_netvsc: Prevent alarm lost on failed device probe 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > Subject: RE: [PATCH] net/vdev_netvsc: Prevent alarm lost on failed device > probe >=20 >=20 >=20 > From: Long Li > > > Subject: RE: [PATCH] net/vdev_netvsc: Prevent alarm lost on failed > > > device probe > > > > > > > > > > > > From: Long Li > > > > If a device probe fails, the alarm is canceled and will no longer > > > > work for previously probed devices. > > > > > > > > Fix this by introducing a flag to track if alarm has been set. > > > > Because it's possible that an alarm is triggered while probing is > > > > in progress that may modify vdev_netvsc_ctx_list, introduce a lock > > > > to > > protect it. > > > > > > > > Cc: stable@dpdk.org > > > > Signed-off-by: Long Li > > > > --- > > > > drivers/net/vdev_netvsc/vdev_netvsc.c | 41 > > > > +++++++++++++++++++++++++---------- > > > > 1 file changed, 30 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c > > > > b/drivers/net/vdev_netvsc/vdev_netvsc.c > > > > index be8f19c..bd7e308 100644 > > > > --- a/drivers/net/vdev_netvsc/vdev_netvsc.c > > > > +++ b/drivers/net/vdev_netvsc/vdev_netvsc.c > > > > @@ -76,6 +76,11 @@ struct vdev_netvsc_ctx { > > > > /** Context list is common to all driver instances. */ static > > > > LIST_HEAD(, > > > > vdev_netvsc_ctx) vdev_netvsc_ctx_list =3D > > > > LIST_HEAD_INITIALIZER(vdev_netvsc_ctx_list); > > > > +/* Lock to protect concurrent accesses to vdev_netvsc_ctx_list */ > > > > +static rte_rwlock_t vdev_netvsc_ctx_list_lock; > > > > + > > > > +/* Flag to track if alarm has been set */ static int > > > > +vdev_netvsc_alarm_set; > > > > > > > > /** Number of entries in context list. */ static unsigned int > > > > vdev_netvsc_ctx_count; @@ -454,19 +459,26 @@ static LIST_HEAD(, > > > > vdev_netvsc_ctx) vdev_netvsc_ctx_list =3D > > > > struct vdev_netvsc_ctx *ctx; > > > > int ret; > > > > > > > > + rte_rwlock_write_lock(&vdev_netvsc_ctx_list_lock); > > > > LIST_FOREACH(ctx, &vdev_netvsc_ctx_list, entry) { > > > > ret =3D vdev_netvsc_foreach_iface(vdev_netvsc_devic= e_probe, > 0, > > > > ctx); > > > > if (ret < 0) > > > > break; > > > > } > > > > - if (!vdev_netvsc_ctx_count) > > > > + rte_rwlock_write_unlock(&vdev_netvsc_ctx_list_lock); > > > > + > > > > + if (!vdev_netvsc_ctx_count) { > > > > + vdev_netvsc_alarm_set =3D 0; > > > > return; > > > > + } > > > > + > > > > ret =3D rte_eal_alarm_set(VDEV_NETVSC_PROBE_MS * 1000, > > > > vdev_netvsc_alarm, NULL); > > > > if (ret < 0) { > > > > DRV_LOG(ERR, "unable to reschedule alarm callback: = %s", > > > > rte_strerror(-ret)); > > > > + vdev_netvsc_alarm_set =3D 0; > > > > } > > > > } > > > > > > > > @@ -698,34 +710,41 @@ static LIST_HEAD(, vdev_netvsc_ctx) > > > > vdev_netvsc_ctx_list =3D > > > > " device."); > > > > goto error; > > > > } > > > > - rte_eal_alarm_cancel(vdev_netvsc_alarm, NULL); > > > > + > > > > + rte_rwlock_write_lock(&vdev_netvsc_ctx_list_lock); > > > > /* Gather interfaces. */ > > > > ret =3D vdev_netvsc_foreach_iface(vdev_netvsc_netvsc_probe, > > > > 1, > > > name, > > > > kvargs, specified, &matched= ); > > > > if (ret < 0) > > > > - goto error; > > > > + goto error_unlock; > > > > if (specified && matched < specified) { > > > > if (!force) { > > > > DRV_LOG(ERR, "Cannot find the specified net= vsc device"); > > > > - goto error; > > > > + goto error_unlock; > > > > } > > > > /* Try to force probing on non-netvsc specified dev= ice. */ > > > > if > > > > (vdev_netvsc_foreach_iface(vdev_netvsc_netvsc_probe, 0, > > > name, > > > > kvargs, specified, &m= atched) < 0) > > > > - goto error; > > > > + goto error_unlock; > > > > if (matched < specified) { > > > > DRV_LOG(ERR, "Cannot find the specified dev= ice"); > > > > - goto error; > > > > + goto error_unlock; > > > > } > > > > DRV_LOG(WARNING, "non-netvsc device was probed as > > netvsc"); > > > > } > > > > - ret =3D rte_eal_alarm_set(VDEV_NETVSC_PROBE_MS * 1000, > > > > + if (!vdev_netvsc_alarm_set) { > > > > + ret =3D rte_eal_alarm_set(VDEV_NETVSC_PROBE_MS * > > > > + 1000, > > > > vdev_netvsc_alarm, NULL); > > > > - if (ret < 0) { > > > > - DRV_LOG(ERR, "unable to schedule alarm callback: %s= ", > > > > - rte_strerror(-ret)); > > > > - goto error; > > > > + if (ret < 0) > > > > + DRV_LOG(ERR, "unable to schedule alarm call= back: %s", > > > > + rte_strerror(-ret)); > > > > + else > > > > + vdev_netvsc_alarm_set =3D 1; > > > > } > > > > + > > > > +error_unlock: > > > > + rte_rwlock_write_unlock(&vdev_netvsc_ctx_list_lock); > > > > + > > > > error: > > > > if (kvargs) > > > > rte_kvargs_free(kvargs); > > > > -- > > > > > > Hi > > > > > > Nice direction. > > > > > > As I understand from your patch it looks like you want to add > > > support for the vdev_netvsc driver itself to be hot-plug aware - so > > > it can be probed\removed in run-time (even after EAL initialization). > > > So I think the title and commit log should be converted to this: > > > net/vdev_netvsc: support driver instance hot-plug ... > > > > Hi Matan, > > > > Actually I was not trying to add support for hot-plug (although it > > made half way there). > > > > The problem I'm trying to solve is on a system with multiple > > vdev_netvsc interfaces. For example, if the system has eth1, eth2 .. > > eth10. What if the probing on eth1 to eth9 succeed, but on eth10 > > fails? Current behavior is that we can still have DPDK run on eth1 to > > eth9, but without any alarms. Because the alarm is canceled while > > trying to probing eth10. Then once eth1 to eth9 lost VF, they won't get= VF > back because we don't have any alarms. > > > > Long >=20 > Ok, got you. >=20 > So, why to manage critical sections in the code, better, to stop alarm be= fore > adding a new device, and apply it back if the list of devices is not empt= y. > It will make it simpler, no? Okay, I will send v2 based on your suggestion. Thanks. >=20 >=20 >=20 > > > > > > More things: > > > Look on convention of the file - blank line only after variables > > > declarations inside the blocks. > > > > > > Need to adjust also vdev_netvsc_vdev_remove to the feature. > > > > > > Matan