[dpdk-dev] [PATCH 0/3] vdev_netvsc: improve the device args parameters meaning

[dpdk-dev] [PATCH 0/3] vdev_netvsc: improve the device args parameters meaning

[Matan Azrad]

After some experience with the device running, it is better to change the meaning of the "force" parameter in whitelist mode.

Matan Azrad (3):
  net/vdev_netvsc: remove specified devices IP check
  net/vdev_netvsc: add check for specifying by 1 way
  net/vdev_netvsc: prefer netvsc devices in scan

 doc/guides/nics/vdev_netvsc.rst       |  10 ++-
 drivers/net/vdev_netvsc/vdev_netvsc.c | 129 +++++++++++++++++++---------------
 2 files changed, 78 insertions(+), 61 deletions(-)

-- 
1.9.5

[dpdk-dev] [PATCH 1/3] net/vdev_netvsc: remove specified devices IP check

[Matan Azrad]

If the netvsc driver starts in blacklist mode, it does not
automatically probe IP associated netvsc devices. Therefore, the only
way to probe them is to specify them by the EAL command line, using the
"force" parameter to skip the IP check in the driver.

>From now on, the user does not need to add the "force" parameter if he
specifies an IP associated netvsc device by the EAL command line, and the
responsibility of the IP check is now in the user's hands.

However, in the absence of any specification, the driver still skips IP
associated netvsc devices.

Cc: stable@dpdk.org

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 doc/guides/nics/vdev_netvsc.rst       | 10 +++++++---
 drivers/net/vdev_netvsc/vdev_netvsc.c |  2 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/vdev_netvsc.rst b/doc/guides/nics/vdev_netvsc.rst
index 55d130a..a2d49f4 100644
--- a/doc/guides/nics/vdev_netvsc.rst
+++ b/doc/guides/nics/vdev_netvsc.rst
@@ -89,12 +89,16 @@ The following device parameters are supported:
 - ``force`` [int]
 
   If nonzero, forces the use of specified interfaces even if not detected as
-  NetVSC or detected as routed NETVSC.
+  NetVSC.
 
 - ``ignore`` [int]
 
   If nonzero, ignores the driver runnig (actually used to disable the
   auto-detection in Hyper-V VM).
 
-Not specifying either ``iface`` or ``mac`` makes this driver attach itself to
-all unrouted NetVSC interfaces found on the system.
+.. note::
+
+   Not specifying either ``iface`` or ``mac`` makes this driver attach itself to
+   all unrouted NetVSC interfaces found on the system.
+   Specifying the device makes this driver attach itself to the device
+   regardless the device routes.
diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index db0080a..d2c41a5 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -569,7 +569,7 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 	/* Routed NetVSC should not be probed. */
 	if (vdev_netvsc_has_route(iface, AF_INET) ||
 	    vdev_netvsc_has_route(iface, AF_INET6)) {
-		if (!specified || !force)
+		if (!specified)
 			return 0;
 		DRV_LOG(WARNING, "probably using routed NetVSC interface \"%s\""
 			" (index %u)", iface->if_name, iface->if_index);
-- 
1.9.5

[dpdk-dev] [PATCH 2/3] net/vdev_netvsc: add check for specifying by 1 way

[Matan Azrad]

There are now 2 ways to specify a netvsc device by the EAL command
line - either by the interface name or by the MAC address.

The user should not specify a netvsc device using more than 1 way,
Thus, if a device is specified in more than 1 way, the driver stops
to probe it.

Validate it in the driver initialization.

Cc: stable@dpdk.org

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/vdev_netvsc/vdev_netvsc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index d2c41a5..018ffae 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -702,6 +702,11 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 			rte_kvargs_free(kvargs);
 		return 0;
 	}
+	if (specified > 1) {
+		DRV_LOG(ERR, "More than one way used to specify the netvsc"
+			" device.");
+		goto error;
+	}
 	rte_eal_alarm_cancel(vdev_netvsc_alarm, NULL);
 	/* Gather interfaces. */
 	ret = vdev_netvsc_foreach_iface(vdev_netvsc_netvsc_probe, name, kvargs,
-- 
1.9.5

[dpdk-dev] [PATCH 3/3] net/vdev_netvsc: prefer netvsc devices in scan

[Matan Azrad]

There is an option to run a non-netvsc device as a netvsc device only
when the "force" parameter is set to 1 in the EAL command line.
Consequently, more than one device may be found to be matching the
"mac" parameter specifying the device.

Prefer netvsc devices to be scanned before any non-netvsc device, even
when the "force" parameter is set.

Cc: stable@dpdk.org

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/vdev_netvsc/vdev_netvsc.c | 122 ++++++++++++++++++----------------
 1 file changed, 65 insertions(+), 57 deletions(-)

diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index 018ffae..75aec89 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -98,6 +98,43 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 }
 
 /**
+ * Determine if a network interface is NetVSC.
+ *
+ * @param[in] iface
+ *   Pointer to netdevice description structure (name and index).
+ *
+ * @return
+ *   A nonzero value when interface is detected as NetVSC. In case of error,
+ *   rte_errno is updated and 0 returned.
+ */
+static int
+vdev_netvsc_iface_is_netvsc(const struct if_nameindex *iface)
+{
+	static const char temp[] = "/sys/class/net/%s/device/class_id";
+	char path[sizeof(temp) + IF_NAMESIZE];
+	FILE *f;
+	int ret;
+	int len = 0;
+
+	ret = snprintf(path, sizeof(path), temp, iface->if_name);
+	if (ret == -1 || (size_t)ret >= sizeof(path)) {
+		rte_errno = ENOBUFS;
+		return 0;
+	}
+	f = fopen(path, "r");
+	if (!f) {
+		rte_errno = errno;
+		return 0;
+	}
+	ret = fscanf(f, NETVSC_CLASS_ID "%n", &len);
+	if (ret == EOF)
+		rte_errno = errno;
+	ret = len == (int)strlen(NETVSC_CLASS_ID);
+	fclose(f);
+	return ret;
+}
+
+/**
  * Iterate over system network interfaces.
  *
  * This function runs a given callback function for each netdevice found on
@@ -106,6 +143,8 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
  * @param func
  *   Callback function pointer. List traversal is aborted when this function
  *   returns a nonzero value.
+ * @param is_netvsc
+ *   Indicates the device type to iterate - netvsc or non-netvsc.
  * @param ...
  *   Variable parameter list passed as @p va_list to @p func.
  *
@@ -117,7 +156,7 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 static int
 vdev_netvsc_foreach_iface(int (*func)(const struct if_nameindex *iface,
 				      const struct ether_addr *eth_addr,
-				      va_list ap), ...)
+				      va_list ap), int is_netvsc, ...)
 {
 	struct if_nameindex *iface = if_nameindex();
 	int s = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP);
@@ -135,10 +174,14 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 		goto error;
 	}
 	for (i = 0; iface[i].if_name; ++i) {
+		int is_netvsc_ret;
 		struct ifreq req;
 		struct ether_addr eth_addr;
 		va_list ap;
 
+		is_netvsc_ret = vdev_netvsc_iface_is_netvsc(&iface[i]) ? 1 : 0;
+		if (is_netvsc ^ is_netvsc_ret)
+			continue;
 		strncpy(req.ifr_name, iface[i].if_name, sizeof(req.ifr_name));
 		if (ioctl(s, SIOCGIFHWADDR, &req) == -1) {
 			DRV_LOG(WARNING, "cannot retrieve information about"
@@ -153,7 +196,7 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 		}
 		memcpy(eth_addr.addr_bytes, req.ifr_hwaddr.sa_data,
 		       RTE_DIM(eth_addr.addr_bytes));
-		va_start(ap, func);
+		va_start(ap, is_netvsc);
 		ret = func(&iface[i], &eth_addr, ap);
 		va_end(ap);
 		if (ret)
@@ -168,43 +211,6 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 }
 
 /**
- * Determine if a network interface is NetVSC.
- *
- * @param[in] iface
- *   Pointer to netdevice description structure (name and index).
- *
- * @return
- *   A nonzero value when interface is detected as NetVSC. In case of error,
- *   rte_errno is updated and 0 returned.
- */
-static int
-vdev_netvsc_iface_is_netvsc(const struct if_nameindex *iface)
-{
-	static const char temp[] = "/sys/class/net/%s/device/class_id";
-	char path[sizeof(temp) + IF_NAMESIZE];
-	FILE *f;
-	int ret;
-	int len = 0;
-
-	ret = snprintf(path, sizeof(path), temp, iface->if_name);
-	if (ret == -1 || (size_t)ret >= sizeof(path)) {
-		rte_errno = ENOBUFS;
-		return 0;
-	}
-	f = fopen(path, "r");
-	if (!f) {
-		rte_errno = errno;
-		return 0;
-	}
-	ret = fscanf(f, NETVSC_CLASS_ID "%n", &len);
-	if (ret == EOF)
-		rte_errno = errno;
-	ret = len == (int)strlen(NETVSC_CLASS_ID);
-	fclose(f);
-	return ret;
-}
-
-/**
  * Determine if a network interface has a route.
  *
  * @param[in] name
@@ -379,8 +385,6 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 		strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
 		return 0;
 	}
-	if (vdev_netvsc_iface_is_netvsc(iface))
-		return 0;
 	if (!is_same_ether_addr(eth_addr, &ctx->if_addr))
 		return 0;
 	/* Look for associated PCI device. */
@@ -449,7 +453,8 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 	int ret;
 
 	LIST_FOREACH(ctx, &vdev_netvsc_ctx_list, entry) {
-		ret = vdev_netvsc_foreach_iface(vdev_netvsc_device_probe, ctx);
+		ret = vdev_netvsc_foreach_iface(vdev_netvsc_device_probe, 0,
+		      ctx);
 		if (ret < 0)
 			break;
 	}
@@ -505,7 +510,6 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 {
 	const char *name = va_arg(ap, const char *);
 	struct rte_kvargs *kvargs = va_arg(ap, struct rte_kvargs *);
-	int force = va_arg(ap, int);
 	unsigned int specified = va_arg(ap, unsigned int);
 	unsigned int *matched = va_arg(ap, unsigned int *);
 	unsigned int i;
@@ -559,13 +563,6 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 			iface->if_name, iface->if_index);
 		return 0;
 	}
-	if (!vdev_netvsc_iface_is_netvsc(iface)) {
-		if (!specified || !force)
-			return 0;
-		DRV_LOG(WARNING,
-			"using non-NetVSC interface \"%s\" (index %u)",
-			iface->if_name, iface->if_index);
-	}
 	/* Routed NetVSC should not be probed. */
 	if (vdev_netvsc_has_route(iface, AF_INET) ||
 	    vdev_netvsc_has_route(iface, AF_INET6)) {
@@ -632,7 +629,7 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 	/* Request virtual device generation. */
 	DRV_LOG(DEBUG, "generating virtual device \"%s\" with arguments \"%s\"",
 		ctx->devname, ctx->devargs);
-	vdev_netvsc_foreach_iface(vdev_netvsc_device_probe, ctx);
+	vdev_netvsc_foreach_iface(vdev_netvsc_device_probe, 0, ctx);
 	ret = rte_eal_hotplug_add("vdev", ctx->devname, ctx->devargs);
 	if (ret)
 		goto error;
@@ -709,14 +706,25 @@ static LIST_HEAD(, vdev_netvsc_ctx) vdev_netvsc_ctx_list =
 	}
 	rte_eal_alarm_cancel(vdev_netvsc_alarm, NULL);
 	/* Gather interfaces. */
-	ret = vdev_netvsc_foreach_iface(vdev_netvsc_netvsc_probe, name, kvargs,
-					force, specified, &matched);
+	ret = vdev_netvsc_foreach_iface(vdev_netvsc_netvsc_probe, 1, name,
+					kvargs, specified, &matched);
 	if (ret < 0)
 		goto error;
-	if (matched < specified)
-		DRV_LOG(WARNING,
-			"some of the specified parameters did not match"
-			" recognized network interfaces");
+	if (specified && matched < specified) {
+		if (!force) {
+			DRV_LOG(ERR, "Cannot find the specified netvsc device");
+			goto error;
+		}
+		/* Try to force probing on non-netvsc specified device. */
+		if (vdev_netvsc_foreach_iface(vdev_netvsc_netvsc_probe, 0, name,
+					      kvargs, specified, &matched) < 0)
+			goto error;
+		if (matched < specified) {
+			DRV_LOG(ERR, "Cannot find the specified device");
+			goto error;
+		}
+		DRV_LOG(WARNING, "non-netvsc device was probed as netvsc");
+	}
 	ret = rte_eal_alarm_set(VDEV_NETVSC_PROBE_MS * 1000,
 				vdev_netvsc_alarm, NULL);
 	if (ret < 0) {
-- 
1.9.5

Re: [dpdk-dev] [PATCH 0/3] vdev_netvsc: improve the device args parameters meaning

[Ferruh Yigit]

On 4/10/2018 2:32 PM, Matan Azrad wrote:
> After some experience with the device running, it is better to change the meaning of the "force" parameter in whitelist mode.
> 
> Matan Azrad (3):
>   net/vdev_netvsc: remove specified devices IP check
>   net/vdev_netvsc: add check for specifying by 1 way
>   net/vdev_netvsc: prefer netvsc devices in scan

Series applied to dpdk-next-net/master, thanks.