From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id A22871B550 for ; Tue, 8 Jan 2019 17:47:59 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Jan 2019 08:47:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,454,1539673200"; d="scan'208";a="108271587" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by orsmga008.jf.intel.com with ESMTP; 08 Jan 2019 08:47:44 -0800 Received: from irsmsx107.ger.corp.intel.com ([169.254.10.127]) by IRSMSX152.ger.corp.intel.com ([169.254.6.47]) with mapi id 14.03.0415.000; Tue, 8 Jan 2019 16:47:43 +0000 From: "Burakov, Anatoly" To: Yongseok Koh , "Stojaczyk, Dariusz" CC: Maxime Coquelin , dpdk stable Thread-Topic: [dpdk-stable] patch 'vfio: do not needlessly setup device in secondary process' has been queued to LTS release 17.11.5 Thread-Index: AQHUl/pd8APcDsNkLE2BXO+cK+8VxqWls7PA Date: Tue, 8 Jan 2019 16:47:43 +0000 Message-ID: References: <20181129231202.30436-1-yskoh@mellanox.com> <20181129231202.30436-127-yskoh@mellanox.com> <96077BDF-660B-4817-81AD-05A2C22A73E8@mellanox.com> In-Reply-To: <96077BDF-660B-4817-81AD-05A2C22A73E8@mellanox.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOWQ2ZDBlYWMtMWYyMy00MDAwLWI5MmItODhmZGYyMTZkYmY3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoic05HZk53aGsxVktqbkRYVk5zQnlidG9LTkxFNUZEUDhSZlNGNGUreEZNK1hZakRrbHN0enB5NkRRYlVNWmxsNCJ9 dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-stable] patch 'vfio: do not needlessly setup device in secondary process' has been queued to LTS release 17.11.5 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: Tue, 08 Jan 2019 16:48:00 -0000 Hi, What's the reason for "mistakenly" merging this patch? This patch fixes a r= eal bug. Why revert? Thanks, Anatoly > -----Original Message----- > From: Yongseok Koh [mailto:yskoh@mellanox.com] > Sent: Thursday, December 20, 2018 12:22 AM > To: Stojaczyk, Dariusz > Cc: Burakov, Anatoly ; Maxime Coquelin > ; dpdk stable > Subject: Re: [dpdk-stable] patch 'vfio: do not needlessly setup device in > secondary process' has been queued to LTS release 17.11.5 >=20 > Hi, >=20 > This patch is being removed from stable/17.11 as it was mistakenly merged= . > Patches having 'fix' keyword in the title were merged even though those > don't have "Cc: stable@dpdk.org" tag in the commit message. >=20 > If you think this patch is still needed for stable/17.11, please let me k= now. > Then I'll take it back. >=20 >=20 > Thanks, > Yongseok >=20 >=20 > > On Nov 29, 2018, at 3:12 PM, Yongseok Koh > wrote: > > > > Hi, > > > > FYI, your patch has been queued to LTS release 17.11.5 > > > > Note it hasn't been pushed to > https://emea01.safelinks.protection.outlook.com/?url=3Dhttp%3A%2F%2Fdpd > k.org%2Fbrowse%2Fdpdk- > stable&data=3D02%7C01%7Cyskoh%40mellanox.com%7C984a5148ee3f4d > 0911c508d65650a735%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7 > C636791301732433081&sdata=3DxSYO3rfdcp080xZsdFughJwuHl10CW%2B6 > njLe8SLNR5o%3D&reserved=3D0 yet. > > It will be pushed if I get no objections before 12/01/18. So please > > shout if anyone has objections. > > > > Also note that after the patch there's a diff of the upstream commit > > vs the patch applied to the branch. If the code is different (ie: not > > only metadata diffs), due for example to a change in context or macro > names, please double check it. > > > > Thanks. > > > > Yongseok > > > > --- > > From ff44ba8002e2db8c02eb769d547bfd7659af14e1 Mon Sep 17 00:00:00 > 2001 > > From: Darek Stojaczyk > > Date: Wed, 21 Nov 2018 19:41:32 +0100 > > Subject: [PATCH] vfio: do not needlessly setup device in secondary > > process > > > > [ upstream commit 047e3f9f2a4a4b73da86b707af8a32039ba1cad7 ] > > > > Setting up a device that wasn't setup in the primary process will > > possibly break the primary process. That's because the IPC message to > > retrieve the group fd in the primary will also *open* that group if it > > wasn't opened before. Even though the secondary process closes that fd > > soon after as a part of its error handling path, the primary process > > leaks it. > > > > What's worse, opening that fd on the primary will increment the > > process-local counter of opened groups. > > If it was 0 before, then the group will never be added to the vfio > > container, nor dpdk memory will be ever mapped. > > > > This patch moves the proper error checks earlier in the code to fully > > prevent setting up devices in secondary processes that weren't setup > > in the primary process. > > > > Fixes: 2f4adfad0a69 ("vfio: add multiprocess support") > > > > Signed-off-by: Darek Stojaczyk > > Acked-by: Anatoly Burakov > > Reviewed-by: Maxime Coquelin > > --- > > drivers/bus/pci/linux/pci_vfio.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/bus/pci/linux/pci_vfio.c > > b/drivers/bus/pci/linux/pci_vfio.c > > index 745db260c..654897284 100644 > > --- a/drivers/bus/pci/linux/pci_vfio.c > > +++ b/drivers/bus/pci/linux/pci_vfio.c > > @@ -580,11 +580,6 @@ pci_vfio_map_resource_secondary(struct > rte_pci_device *dev) > > snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT, > > loc->domain, loc->bus, loc->devid, loc->function); > > > > - ret =3D rte_vfio_setup_device(rte_pci_get_sysfs_path(), pci_addr, > > - &vfio_dev_fd, &device_info); > > - if (ret) > > - return ret; > > - > > /* if we're in a secondary process, just find our tailq entry */ > > TAILQ_FOREACH(vfio_res, vfio_res_list, next) { > > if (rte_pci_addr_cmp(&vfio_res->pci_addr, > > @@ -596,9 +591,14 @@ pci_vfio_map_resource_secondary(struct > rte_pci_device *dev) > > if (vfio_res =3D=3D NULL) { > > RTE_LOG(ERR, EAL, " %s cannot find TAILQ entry for PCI > device!\n", > > pci_addr); > > - goto err_vfio_dev_fd; > > + return -1; > > } > > > > + ret =3D rte_vfio_setup_device(rte_pci_get_sysfs_path(), pci_addr, > > + &vfio_dev_fd, &device_info); > > + if (ret) > > + return ret; > > + > > /* map BARs */ > > maps =3D vfio_res->maps; > > > > -- > > 2.11.0 > > > > --- > > Diff of the applied patch vs upstream commit (please double-check if n= on- > empty: > > --- > > --- - 2018-11-29 15:01:50.745484864 -0800 > > +++ 0127-vfio-do-not-needlessly-setup-device-in-secondary-pro.patch > 2018-11-29 15:01:45.335961000 -0800 > > @@ -1,8 +1,10 @@ > > -From 047e3f9f2a4a4b73da86b707af8a32039ba1cad7 Mon Sep 17 00:00:00 > > 2001 > > +From ff44ba8002e2db8c02eb769d547bfd7659af14e1 Mon Sep 17 00:00:00 > > +2001 > > From: Darek Stojaczyk > > Date: Wed, 21 Nov 2018 19:41:32 +0100 > > Subject: [PATCH] vfio: do not needlessly setup device in secondary > > process > > > > +[ upstream commit 047e3f9f2a4a4b73da86b707af8a32039ba1cad7 ] > > + > > Setting up a device that wasn't setup in the primary process will > > possibly break the primary process. That's because the IPC message to > > retrieve the group fd in the @@ -31,10 +33,10 @@ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/bus/pci/linux/pci_vfio.c > > b/drivers/bus/pci/linux/pci_vfio.c > > -index ffd26f195..54a4c959e 100644 > > +index 745db260c..654897284 100644 > > --- a/drivers/bus/pci/linux/pci_vfio.c > > +++ b/drivers/bus/pci/linux/pci_vfio.c > > -@@ -794,11 +794,6 @@ pci_vfio_map_resource_secondary(struct > > rte_pci_device *dev) > > +@@ -580,11 +580,6 @@ pci_vfio_map_resource_secondary(struct > > +rte_pci_device *dev) > > snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT, > > loc->domain, loc->bus, loc->devid, loc->function); > > > > @@ -46,7 +48,7 @@ > > /* if we're in a secondary process, just find our tailq entry */ > > TAILQ_FOREACH(vfio_res, vfio_res_list, next) { > > if (rte_pci_addr_cmp(&vfio_res->pci_addr, > > -@@ -810,9 +805,14 @@ pci_vfio_map_resource_secondary(struct > > rte_pci_device *dev) > > +@@ -596,9 +591,14 @@ pci_vfio_map_resource_secondary(struct > > +rte_pci_device *dev) > > if (vfio_res =3D=3D NULL) { > > RTE_LOG(ERR, EAL, " %s cannot find TAILQ entry for PCI > device!\n", > > pci_addr);