From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 47C47A0C46 for ; Mon, 16 Aug 2021 08:24:32 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A47444113E; Mon, 16 Aug 2021 08:24:31 +0200 (CEST) Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2100.outbound.protection.outlook.com [40.107.236.100]) by mails.dpdk.org (Postfix) with ESMTP id 678B240C35; Wed, 11 Aug 2021 17:25:42 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OnCKWW0QlDt7gzPmBkOACO3MI4cgD62RrT8piLOMC/u7si3d5w1Kt90W4nCcdpg4dxT+s83qx8lCQLcZCMVa+4KGHb2GLdHmiurHLcicHMfG39mdJJEIphb9YBSBwVlPbSAI9+46fUD6QL+aHJPgWczdnSTw9jH/b/J8M2weUz5EWqgk1dD4xEDW9oKl+58RN8hgv2t6vQagZ64NwW9KFqhHoVs84W2w5LjGVLdbsbPPg/UbsikE3Y3/x4X5kzAwFCDhPPtGjoGMvRfTocIFUsrGkdwxmlAfstNyqVh1pyuyYj+69t2Ojb6jO63l/6RNMrxY4TYUQd9cumppL7EBmQ== 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=rcUhAoaL0EJlosS/wuUmc/iN10kX5Mx2Xl2unxC6vfk=; b=hE6Axwg3TGoAAHhZqtrXK7ZkT4XG1c8ZszNjWxwa3B6fHo9WpCxhqlHUM3lrNTamv+V2MbZz4pG4MPTaEdW96vwGsEFT0jg94n/7iIO+ojSIXUypy/SmEoH3XyRylqbW2F81KWCbRwvGECWFgAYZlLrc/NSx5M3iPJVwo6p+3bPMHXlAuWoUhhWjMjJJhmEyAL7pedpRxRKHtzwEpFDPlQ9k9RwlmKOB8JUm3TsaCvU4i8YgcfYHOgESoOizVbw4S8vSosfLAitKxhozRJfqiQAULkj83S9rkhLQAa1aCiX9eZQFv0Ddf8cxkrTeO1XPB2BBSlm0Rv5jbmHPXPlYmQ== 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=rcUhAoaL0EJlosS/wuUmc/iN10kX5Mx2Xl2unxC6vfk=; b=dtX4EtrdR+slb+Y8J71n6j03ollZoQR1swff2TbZ93ENlSVXhgT5/aLFDMQWtIEYghun51NvbNcqqzkGm4LOGlGtWtqVqTQb8DSMWAlACx12ih2/qJOQ4GdLIV4whdvQ4LqLvzpVHIxCjA9/QrBqz4XUweD7FupURiq6wimHc6I= Received: from MW4PR21MB1907.namprd21.prod.outlook.com (2603:10b6:303:71::8) by MWHPR2101MB0812.namprd21.prod.outlook.com (2603:10b6:301:7c::39) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4436.4; Wed, 11 Aug 2021 15:25:39 +0000 Received: from MW4PR21MB1907.namprd21.prod.outlook.com ([fe80::203b:4b22:735f:bdc2]) by MW4PR21MB1907.namprd21.prod.outlook.com ([fe80::203b:4b22:735f:bdc2%9]) with mapi id 15.20.4436.009; Wed, 11 Aug 2021 15:25:39 +0000 From: Stephen Hemminger To: Jonathan Erb , Long Li CC: "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [PATCH v2] bus/vmbus: Fix crash when handling packets in secondary process Thread-Index: AQHXgj/PzmblBmjXy06N0dgoiHkju6tV01wAgBWZYYCAAxh9gA== Date: Wed, 11 Aug 2021 15:25:39 +0000 Message-ID: References: <20210726170040.25155-1-jonathan.erb@banduracyber.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=218d6f35-e65d-48a4-ac71-434402406f06; 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=2021-08-11T15:23:40Z; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47; authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=microsoft.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 016c8332-c3ab-440e-b691-08d95cdc467c x-ms-traffictypediagnostic: MWHPR2101MB0812: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:2657; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: GDaMWcP0o/wbFiAnXpK5rkD1C7dCsbjCN+jWqLp/72og3FV54ABfgZHIFrWxp6hBPKquOQAnwrIKZFlRqhDFCyjNraf1aqi5MCREVjRe1Imk999tA8NdyMB60PjYvMzYR5oKGDjLulLS1lxfj7fyGT2vFhNMpiAIcZZPuDxaTerdVqt1P914HlsDIkLxCNeHPIrD+UHJohox58D/zYP8LQyytgSPuk2CvcIfCxxSHRdcqinJcaExPKCimQ/7vgV8b6vHqGK/qJUuZQ/zcY7BtgsfRp4T0IAg5Pa0xVq6IJLb5uTHynMjGrRfK/+pg4wdwXkIO2ZquLC4eZu7uUA7t6+/qkFnYKo1sjEro9us8ojtgp9cm41g5stiLzT/of1pfFODdwLLJsoJcI4ID+fXuptDUr46Q86q0EW+oCc0oGHKMSpjjjaBc8SH6prBI9gTPl/xkYn8uLs23xQEdw6vzM3A+Lo/YmEiEr5FMRvjZ0P24lkSt+2wcwYGZkoLBH1eapZTYGz7tZktfL6g3PU/xyvKpV/YIsyzoZZpWleQqSEOUT6+hN9zAdLMvSC8c9u7YWuP691D+7V1YVy8atEGCug+1aX+vWR8OhxuL2jS1YXDgpEi8aZF8otMtcJcLLRdyL5IJxMdWW+UzfHmL9zfI1OdQfsfXz5GowUCV9e8GI6iksHK0NwDNz7r93YUXlKMHVau9wKe1VFh/52mka2gf3lIyKk+Ll4Pk+k0yQiM3cV6W3NvIZaxSGQfcqGx4MXE x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MW4PR21MB1907.namprd21.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(2906002)(53546011)(8990500004)(33656002)(8676002)(9686003)(186003)(76116006)(508600001)(55016002)(52536014)(316002)(8936002)(7696005)(38070700005)(82960400001)(82950400001)(122000001)(54906003)(83380400001)(66446008)(71200400001)(66476007)(66946007)(110136005)(64756008)(66556008)(4326008)(26005)(6506007)(5660300002)(6636002)(166002)(86362001)(10290500003)(38100700002)(10090945008); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?i4g04W7pfFGof3cUBc71YDNMFhvZ/ibqzqNUsffmNLlvEu+IAoCGLhm36b9j?= =?us-ascii?Q?WrOWUJYlVGOMvHfKHFClYuvOgMCfnhd79WRCPFvT6XFqILw4vFBCyzpFO5oe?= =?us-ascii?Q?9ObBiC8y6Du2bQ2XJz7ExOZZe5z5aokQXXP3Y34ePfS2Dn9PvbtkdEnIeeMi?= =?us-ascii?Q?WV9Y55lQtcusD1fVA6UrQirJ2yGiujvzAwQMGcSH+yq1dJECI3XRMuoRtFsX?= =?us-ascii?Q?O9mHtjSTHmGzuCcZVgfzNEGBwjFjx3c3z61DuNnQLtWLQRpBI2RB1OuLJyda?= =?us-ascii?Q?ntn4I+jR2Lk3FDSDsI050DDfsP7pYImY7lXXNpq7PIR2VV2fMDtve/nb+E3c?= =?us-ascii?Q?67ndDjBpUN8iM3uGApQy3PEKFyWj/vKwFv0ywvhgPH85EXLmGzp8Be53qkgU?= =?us-ascii?Q?tUt8xqMiL/7RP4nPj58Pw/RMGf8q9JH3TwIJyZrWl1HKJ7GKcRocbP3QdMWh?= =?us-ascii?Q?cpAJQ34scbq/O7g1WisMikeODCfseJD4UW62XcqskREdTOTBZ1wV0iEMucL9?= =?us-ascii?Q?89DLf2+bVJ+FUvOljepld73BrOtmLkEoF4Aa4LItJsqcQMZOwXVqrJ+sMVjf?= =?us-ascii?Q?6pybFmi/HkEyI3JQXNmMCzCky8zBMdMzacdcgeFcJrJiJypVaw+74z/yGcqs?= =?us-ascii?Q?ZNPxaXB9Bxdl4EMcmSqZRDrdrqZreLzhV5HBqABs6FM5pKM3RO+A4pG/wrbR?= =?us-ascii?Q?uoRXj0h9ad50nMArqmUkJS+zDLgyyT7yZJNDiPDc9DqwtCG40UKynWn/xP/i?= =?us-ascii?Q?pxJ9bHSYF09oKwuqU1A6WUD/uzltNfkGc0U8hpY0KccBNYhZlOoniZNHEMCa?= =?us-ascii?Q?JVBVFyzrA9bW3arGSRtN7M7UALpButtL/A1K98BbqGl3mv1UtWJwiipOHzxt?= =?us-ascii?Q?nGXNf8KRDjdkjI3pH1jGSWu3J1L9cEZ3fqA7PPRfovfUG02BViPyHwr/Rzw7?= =?us-ascii?Q?My+HBAaepAaa49j9+hxjX7A15o3Z0v59e9Bh1bjDtwj21fmIxu1xm7HUkYCq?= =?us-ascii?Q?t4L+c1H+W2j1Xn4Iu+A5/szMhFNAVYPUMlVncBd0nNd+rvwWd/rQ0qzMhSb3?= =?us-ascii?Q?zqnJOvLFJzu8XVOxcRykoP1zH2JaAYJ1sPQ8zO2V8g8FDdTDIOe5KLYfzAdX?= =?us-ascii?Q?KMia7oiM+MZkfEeZ+oXurdJH+TxYgsSiPEC+gKdnP8XWzPbCZV0mhIYbxgMF?= =?us-ascii?Q?+XrZwVrVpQlvhNosTG1OnLTgQut3SR9bhfb15l+HBGILIS7SfO2yJcIyVp0T?= =?us-ascii?Q?1ndEDq/0YFKmjBAqHxp4NpUhyzu49LO0pm7K4xK/25Ij9fq8VMjl0YBScO0I?= =?us-ascii?Q?CN/NxTQvfIfE/J6qFEBb04RR?= MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR21MB1907.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 016c8332-c3ab-440e-b691-08d95cdc467c X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Aug 2021 15:25:39.4396 (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: njNdzAtYz6gkMz0UNrh3VeNjqLqkQv4t9TNeUsMk/AONBeWnrG5ZqYPteiD67Mv0UkE4/+Bu4PVfPUF+9tNeQA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR2101MB0812 X-Mailman-Approved-At: Mon, 16 Aug 2021 08:24:29 +0200 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [dpdk-stable] [PATCH v2] bus/vmbus: Fix crash when handling packets in secondary process X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Looks fine, only comments would be to keep to original style and add check = if primary channel not found? From: Jonathan Erb Sent: Monday, August 9, 2021 9:07 AM To: Long Li ; Stephen Hemminger Cc: dev@dpdk.org; stable@dpdk.org Subject: Re: [PATCH v2] bus/vmbus: Fix crash when handling packets in secon= dary process You don't often get email from jonathan.erb@banduracyber.com. Learn why this is important Would it be possible to resolve the lack of subchannels in secondary proces= ses as follows: 1. Create the following function in vmbus_uio.c to obtain the count of subc= hannels created by the primary: int vmbus_uio_get_num_subchan(struct vmbus_channel *primary) { const struct rte_vmbus_device *dev =3D primary->device; char chan_path[PATH_MAX]; struct dirent *ent; DIR *chan_dir; int err; int subchan_cnt =3D 0; snprintf(chan_path, sizeof(chan_path), "%s/%s/channels", SYSFS_VMBUS_DEVICES, dev->device.name); chan_dir =3D opendir(chan_path); if (!chan_dir) { VMBUS_LOG(ERR, "cannot open %s: %s", chan_path, strerror(errno)); return 0; } while ((ent =3D readdir(chan_dir))) { unsigned long relid, subid, monid; char *endp; if (ent->d_name[0] =3D=3D '.') continue; errno =3D 0; relid =3D strtoul(ent->d_name, &endp, 0); if (*endp || errno !=3D 0 || relid > UINT16_MAX) { VMBUS_LOG(NOTICE, "not a valid channel relid: %s", ent->d_name); continue; } subchan_cnt++; } closedir(chan_dir); //Less one for primary channel return subchan_cnt - 1; } 2. Then change the bottom of vmbus_uio_map_secondary() to be simply: /* fd is not needed in secondary process, close it */ close(fd); if (vmbus_chan_create(dev, dev->relid, 0, dev->monitor_id, &dev->primary)) { VMBUS_LOG(ERR, "cannot create primary channel"); return -1; } int subchannels_cnt =3D vmbus_uio_get_num_subchan(dev->primary); for (i =3D 0; i < subchannels_cnt; i++) { if(vmbus_uio_get_subchan(dev->primary, &chan)) return -1; STAILQ_INSERT_TAIL(&dev->primary->subchannel_list, chan, next); } return 0; } In this way the secondary processes detect the number of subchannels create= d by the primary, then perform their own mappings as needed. All this can o= ccur before rte_eal_init has completed. Jonathan On 7/26/21 6:16 PM, Long Li wrote: Subject: [PATCH v2] bus/vmbus: Fix crash when handling packets in secondary process Have secondary processes construct their own copy of primary channel with own mappings. Remove vmbus_channel primary ptr from struct mapped_vmbus_resource as its not used. Populate virtual memory address "addr" in struct rte_mem_resource for secondary processes as netvsc will attempt to reference it thus causing a crash. It was initialized for primary processes but not for secondary. Cc: stable@dpdk.org Signed-off-by: Jonathan Erb --- v2: * Remove unnecessary check for NULL pointer before call to rte_free() per reviwer comment. drivers/bus/vmbus/private.h | 1 - drivers/bus/vmbus/vmbus_channel.c | 4 +--- drivers/bus/vmbus/vmbus_common_uio.c | 14 +++++++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h index 528d60a42f..746212bd5f 100644 --- a/drivers/bus/vmbus/private.h +++ b/drivers/bus/vmbus/private.h @@ -42,7 +42,6 @@ struct mapped_vmbus_resource { rte_uuid_t id; int nb_maps; - struct vmbus_channel *primary; struct vmbus_map maps[VMBUS_MAX_RESOURCE]; char path[PATH_MAX]; }; diff --git a/drivers/bus/vmbus/vmbus_channel.c b/drivers/bus/vmbus/vmbus_channel.c index f67f1c438a..119b9b367e 100644 --- a/drivers/bus/vmbus/vmbus_channel.c +++ b/drivers/bus/vmbus/vmbus_channel.c @@ -351,10 +351,8 @@ int rte_vmbus_chan_open(struct rte_vmbus_device *device, err =3D vmbus_chan_create(device, device->relid, 0, device->monitor_id, new_chan); - if (!err) { + if (!err) device->primary =3D *new_chan; - uio_res->primary =3D *new_chan; - } return err; } diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c index 8582e32c1d..83c56b6fa2 100644 --- a/drivers/bus/vmbus/vmbus_common_uio.c +++ b/drivers/bus/vmbus/vmbus_common_uio.c @@ -69,8 +69,10 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev) fd, offset, uio_res->maps[i].size, 0); - if (mapaddr =3D=3D uio_res->maps[i].addr) + if (mapaddr =3D=3D uio_res->maps[i].addr) { + dev->resource[i].addr =3D mapaddr; continue; /* successful map */ + } if (mapaddr =3D=3D MAP_FAILED) VMBUS_LOG(ERR, @@ -88,9 +90,9 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev) /* fd is not needed in secondary process, close it */ close(fd); - dev->primary =3D uio_res->primary; - if (!dev->primary) { - VMBUS_LOG(ERR, "missing primary channel"); + if (vmbus_chan_create(dev, dev->relid, 0, + dev->monitor_id, &dev->primary)) { + VMBUS_LOG(ERR, "cannot create primary channel"); return -1; } Looking at this closer, I don't think it will work for subchannels in the s= econdary process. The code after is: STAILQ_FOREACH(chan, &dev->primary->subchannel_list, next) { if (vmbus_uio_map_secondary_subchan(dev, chan) !=3D 0) { VMBUS_LOG(ERR, "cannot map secondary subchan"); return -1; } } Because at this time, the primary channel is just created, "subchannel_list= " should be NULL. The secondary process ends up running without subchannels= . In the primary process, "subchannel_list" are populated when it calls hn_= dev_configure(), so it is good. Sorry I didn't spot this earlier. The best way to fix this is to use rte me= mory functions for allocating vmbus device. In this way they can be proper= ly mapped to the secondary process. But rte memory functions are not availa= ble at the time vmbus device is probed. I haven't been able to find a good = way to fix this. Will keep looking. Thanks, Long @@ -211,8 +213,10 @@ vmbus_uio_unmap_resource(struct rte_vmbus_device *dev) return; /* secondary processes - just free maps */ - if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) { + rte_free(dev->primary); return vmbus_uio_unmap(uio_res); + } TAILQ_REMOVE(uio_res_list, uio_res, next); -- 2.17.1