[dpdk-stable] patch 'vfio: do not needlessly setup device in secondary process' has been queued to LTS release 17.11.5

[Yongseok Koh]

Hi,

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 at dpdk.org" tag in the commit message.

If you think this patch is still needed for stable/17.11, please let me know.
Then I'll take it back.


Thanks,
Yongseok


> On Nov 29, 2018, at 3:12 PM, Yongseok Koh <yskoh at mellanox.com> 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=http%3A%2F%2Fdpdk.org%2Fbrowse%2Fdpdk-stable&data=02%7C01%7Cyskoh%40mellanox.com%7C984a5148ee3f4d0911c508d65650a735%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636791301732433081&sdata=xSYO3rfdcp080xZsdFughJwuHl10CW%2B6njLe8SLNR5o%3D&reserved=0 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 <dariusz.stojaczyk at intel.com>
> 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 <dariusz.stojaczyk at intel.com>
> Acked-by: Anatoly Burakov <anatoly.burakov at intel.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> ---
> 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 = 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 == NULL) {
> 		RTE_LOG(ERR, EAL, "  %s cannot find TAILQ entry for PCI device!\n",
> 				pci_addr);
> -		goto err_vfio_dev_fd;
> +		return -1;
> 	}
> 
> +	ret = rte_vfio_setup_device(rte_pci_get_sysfs_path(), pci_addr,
> +					&vfio_dev_fd, &device_info);
> +	if (ret)
> +		return ret;
> +
> 	/* map BARs */
> 	maps = vfio_res->maps;
> 
> -- 
> 2.11.0
> 
> ---
>  Diff of the applied patch vs upstream commit (please double-check if non-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 <dariusz.stojaczyk at intel.com>
> 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 == NULL) {
>  		RTE_LOG(ERR, EAL, "  %s cannot find TAILQ entry for PCI device!\n",
>  				pci_addr);