[Patch v2 0/3] net/netvsc fixes for handling PCI hotplug with multiple devices

[Patch v2 0/3] net/netvsc fixes for handling PCI hotplug with multiple devices

[longli]

From: Long Li <longli@microsoft.com>

On some systems, one PCI device may have muiltple NET devices. This
patchset fixes issues on those systems when such PCI device is doing hot 
add/remove.

Change in v2: added cover letter, drop the 4th patch that will be implemented in EAL instead for device args caching

Long Li (3):
  net/netvsc: scan all net devices under the PCI device
  net/netvsc: remove RTE device if all its net devices are removed
  net/netvsc: log error on failure to switch data path

 drivers/net/netvsc/hn_ethdev.c | 29 ++++++++++++-----------------
 drivers/net/netvsc/hn_vf.c     | 18 +++++++++++++++---
 2 files changed, 27 insertions(+), 20 deletions(-)

-- 
2.34.1

[Patch v2 1/3] net/netvsc: scan all net devices under the PCI device

[longli]

From: Long Li <longli@microsoft.com>

The current code has the wrong assumption that a PCI device can have only
one Ethernet device. This is not correct as a PCI device can be multi
functional and have multiple Ethernet devices.

Fix this by scanning all the devices under a PCI device.

Fixes: a2a23a794b3a ("net/netvsc: support VF device hot add/remove")
Cc: stable@dpdk.org
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/netvsc/hn_ethdev.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 1736cb5d07..f848157b49 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -570,7 +570,7 @@ static void netvsc_hotplug_retry(void *args)
 	struct rte_devargs *d = &hot_ctx->da;
 	char buf[256];
 
-	DIR *di;
+	DIR *di = NULL;
 	struct dirent *dir;
 	struct ifreq req;
 	struct rte_ether_addr eth_addr;
@@ -590,7 +590,9 @@ static void netvsc_hotplug_retry(void *args)
 	if (!di) {
 		PMD_DRV_LOG(DEBUG, "%s: can't open directory %s, "
 			    "retrying in 1 second", __func__, buf);
-		goto retry;
+		/* The device is still being initialized, retry after 1 second */
+		rte_eal_alarm_set(1000000, netvsc_hotplug_retry, hot_ctx);
+		return;
 	}
 
 	while ((dir = readdir(di))) {
@@ -614,10 +616,9 @@ static void netvsc_hotplug_retry(void *args)
 				    dir->d_name);
 			break;
 		}
-		if (req.ifr_hwaddr.sa_family != ARPHRD_ETHER) {
-			closedir(di);
-			goto free_hotadd_ctx;
-		}
+		if (req.ifr_hwaddr.sa_family != ARPHRD_ETHER)
+			continue;
+
 		memcpy(eth_addr.addr_bytes, req.ifr_hwaddr.sa_data,
 		       RTE_DIM(eth_addr.addr_bytes));
 
@@ -636,22 +637,16 @@ static void netvsc_hotplug_retry(void *args)
 				PMD_DRV_LOG(ERR,
 					    "Failed to add PCI device %s",
 					    d->name);
-				break;
 			}
+
+			break;
 		}
-		/* When the code reaches here, we either have already added
-		 * the device, or its MAC address did not match.
-		 */
-		closedir(di);
-		goto free_hotadd_ctx;
 	}
-	closedir(di);
-retry:
-	/* The device is still being initialized, retry after 1 second */
-	rte_eal_alarm_set(1000000, netvsc_hotplug_retry, hot_ctx);
-	return;
 
 free_hotadd_ctx:
+	if (di)
+		closedir(di);
+
 	rte_spinlock_lock(&hv->hotadd_lock);
 	LIST_REMOVE(hot_ctx, list);
 	rte_spinlock_unlock(&hv->hotadd_lock);
-- 
2.34.1

[Patch v2 2/3] net/netvsc: remove RTE device if all its net devices are removed

[longli]

From: Long Li <longli@microsoft.com>

An RTE device can have multiple Ethernet devices. On hot plug events, it
can't be removed until all its Ethernet devices have been removed.

Fixes: a2a23a794b3a ("net/netvsc: support VF device hot add/remove")
Cc: stable@dpdk.org
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/netvsc/hn_vf.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index b664beaa5d..5d8058774d 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -102,6 +102,7 @@ static void hn_remove_delayed(void *args)
 	uint16_t port_id = hv->vf_ctx.vf_port;
 	struct rte_device *dev = rte_eth_devices[port_id].device;
 	int ret;
+	bool all_eth_removed;
 
 	/* Tell VSP to switch data path to synthetic */
 	hn_vf_remove(hv);
@@ -138,7 +139,17 @@ static void hn_remove_delayed(void *args)
 		PMD_DRV_LOG(ERR, "rte_eth_dev_close failed port_id=%u ret=%d",
 			    port_id, ret);
 
-	ret = rte_dev_remove(dev);
+	/* Remove the rte device when all its eth devices are removed */
+	all_eth_removed = true;
+	RTE_ETH_FOREACH_DEV_OF(port_id, dev) {
+		if (rte_eth_devices[port_id].state != RTE_ETH_DEV_UNUSED) {
+			all_eth_removed = false;
+			break;
+		}
+	}
+	if (all_eth_removed)
+		ret = rte_dev_remove(dev);
+
 	hv->vf_ctx.vf_state = vf_removed;
 
 	rte_rwlock_write_unlock(&hv->vf_lock);
-- 
2.34.1

[Patch v2 3/3] net/netvsc: log error on failure to switch data path

[longli]

From: Long Li <longli@microsoft.com>

There is no recovery code path if netvsc failed to switch data path.

Log an error in this case for troubleshooting.

Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/netvsc/hn_vf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index 5d8058774d..4ff766ec8b 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -316,8 +316,9 @@ static void hn_vf_remove(struct hn_data *hv)
 	} else {
 		/* Stop incoming packets from arriving on VF */
 		ret = hn_nvs_set_datapath(hv, NVS_DATAPATH_SYNTHETIC);
-		if (ret == 0)
-			hv->vf_ctx.vf_vsc_switched = false;
+		if (ret)
+			PMD_DRV_LOG(ERR, "Failed to switch to synthetic");
+		hv->vf_ctx.vf_vsc_switched = false;
 	}
 	rte_rwlock_write_unlock(&hv->vf_lock);
 }
-- 
2.34.1