[dpdk-dev] [PATCH] dev: fix virtual dev attach

[dpdk-dev] [PATCH] dev: fix virtual dev attach

[Gaetan Rivet]

If the device cannot be parsed as a PCI device, the rte_devargs
function returns -EFAULT. This error code signifies that the address
given to the PCI bus is invalid. When it happens, the best course of
action is to try to plug the device using the vdev bus.

Fixes: 1c35f666df07 ("dev: fix attach proceeding with vdev on PCI
success")

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index d74f978..97b063e 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -74,7 +74,7 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
 	}
 
 	ret = rte_eal_hotplug_add("pci", name, devargs);
-	if (ret != -EINVAL)
+	if (ret != -EINVAL && ret != -EFAULT)
 		return ret;
 
 	/*
-- 
2.1.4

[dpdk-dev] [PATCH v2] dev: fix virtual dev attach

[Gaetan Rivet]

Checking against error values returned by rte_eal_hotplug_add is
inelegant and prone to mistakes. Additionally, the failed PCI probe
prints a useless error that would throw off unsuspecting users.

Use the relevant functions to infer the intended bus. The limitation to
PCI or vdev device is kept for strict API compatibility.

Fixes: 1c35f666df07 ("dev: fix attach proceeding with vdev on PCI
success")

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---

v2:

  - Avoid the PCI parsing error when attaching a vdev.

 lib/librte_eal/common/eal_common_dev.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index d74f978..f98302d 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -66,6 +66,7 @@ static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 
 int rte_eal_dev_attach(const char *name, const char *devargs)
 {
+	struct rte_bus *bus;
 	int ret;
 
 	if (name == NULL || devargs == NULL) {
@@ -73,9 +74,18 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
 		return -EINVAL;
 	}
 
-	ret = rte_eal_hotplug_add("pci", name, devargs);
-	if (ret != -EINVAL)
-		return ret;
+	bus = rte_bus_find_by_device_name(name);
+	if (bus == NULL) {
+		RTE_LOG(ERR, EAL, "Unable to find a bus for the device '%s'\n",
+			name);
+		return -EINVAL;
+	}
+	if (strcmp(bus->name, "pci") == 0)
+		return rte_eal_hotplug_add("pci", name, devargs);
+	if (strcmp(bus->name, "vdev") != 0) {
+		RTE_LOG(ERR, EAL, "Device attach is only supported for PCI and vdev devices.\n");
+		return -ENOTSUP;
+	}
 
 	/*
 	 * If we haven't found a bus device the user meant to "hotplug" a
-- 
2.1.4

[dpdk-dev] [PATCH v3] dev: fix virtual dev attach

[Gaetan Rivet]

Checking against error values returned by rte_eal_hotplug_add is
inelegant and prone to mistakes. Additionally, the failed PCI probe
prints a useless error that would throw off unsuspecting users:

   ERROR: failed to parse device "pci:net_ring0"

This error is printed when attempting to probe a virtual device first
with the PCI bus (here, a net_ring0 device), which will inevitably fail.

Use the relevant functions to infer the intended bus. The limitation to
PCI or vdev device is kept for strict API compatibility. Thus the PCI
probe attempt is avoided and the right function is directly called.

Fixes: 1c35f666df07 ("dev: fix attach proceeding with vdev on PCI
success")

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---

v2:

  - Avoid the PCI parsing error when attaching a vdev.

v3:

  - Explain the parsing error previously thrown that the current version
    of the patch avoids.

 lib/librte_eal/common/eal_common_dev.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index d74f978..f98302d 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -66,6 +66,7 @@ static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 
 int rte_eal_dev_attach(const char *name, const char *devargs)
 {
+	struct rte_bus *bus;
 	int ret;
 
 	if (name == NULL || devargs == NULL) {
@@ -73,9 +74,18 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
 		return -EINVAL;
 	}
 
-	ret = rte_eal_hotplug_add("pci", name, devargs);
-	if (ret != -EINVAL)
-		return ret;
+	bus = rte_bus_find_by_device_name(name);
+	if (bus == NULL) {
+		RTE_LOG(ERR, EAL, "Unable to find a bus for the device '%s'\n",
+			name);
+		return -EINVAL;
+	}
+	if (strcmp(bus->name, "pci") == 0)
+		return rte_eal_hotplug_add("pci", name, devargs);
+	if (strcmp(bus->name, "vdev") != 0) {
+		RTE_LOG(ERR, EAL, "Device attach is only supported for PCI and vdev devices.\n");
+		return -ENOTSUP;
+	}
 
 	/*
 	 * If we haven't found a bus device the user meant to "hotplug" a
-- 
2.1.4

Re: [dpdk-dev] [PATCH v3] dev: fix virtual dev attach

[Thomas Monjalon]

31/07/2017 14:58, Gaetan Rivet:
> Checking against error values returned by rte_eal_hotplug_add is
> inelegant and prone to mistakes. Additionally, the failed PCI probe
> prints a useless error that would throw off unsuspecting users:
> 
>    ERROR: failed to parse device "pci:net_ring0"
> 
> This error is printed when attempting to probe a virtual device first
> with the PCI bus (here, a net_ring0 device), which will inevitably fail.
> 
> Use the relevant functions to infer the intended bus. The limitation to
> PCI or vdev device is kept for strict API compatibility. Thus the PCI
> probe attempt is avoided and the right function is directly called.
> 
> Fixes: 1c35f666df07 ("dev: fix attach proceeding with vdev on PCI
> success")
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Applied with additional explanation from v1, thanks:

dev: fix vdev attach after PCI mismatch

If the device is a vdev, the parsing for PCI will fail with -EFAULT,
and will not try to check for a vdev.