DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name
@ 2020-05-28 12:03 wangyunjian
  2020-05-28 16:12 ` Stephen Hemminger
  2020-05-29 17:47 ` Stephen Hemminger
  0 siblings, 2 replies; 5+ messages in thread
From: wangyunjian @ 2020-05-28 12:03 UTC (permalink / raw)
  To: dev; +Cc: sthemmin, jerry.lilijun, xudingke, Yunjian Wang, stable

From: Yunjian Wang <wangyunjian@huawei.com>

We do not need and should not allocate memory for device.name.
The device.name should be set point to the devargs->name.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Cc: stable@dpdk.org

Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 drivers/bus/vmbus/linux/vmbus_bus.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 3c924ee..31d0dd3 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -242,9 +242,6 @@
 		return -1;
 
 	dev->device.bus = &rte_vmbus_bus.bus;
-	dev->device.name = strdup(name);
-	if (!dev->device.name)
-		goto error;
 
 	/* sysfs base directory
 	 *   /sys/bus/vmbus/devices/7a08391f-f5a0-4ac0-9802-d13fd964f8df
@@ -296,6 +293,7 @@
 	}
 
 	dev->device.devargs = vmbus_devargs_lookup(dev);
+	dev->device.name = dev->device.devargs->name;
 
 	/* device is valid, add in list (sorted) */
 	VMBUS_LOG(DEBUG, "Adding vmbus device %s", name);
-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name
  2020-05-28 12:03 [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name wangyunjian
@ 2020-05-28 16:12 ` Stephen Hemminger
  2020-05-29  7:24   ` wangyunjian
  2020-05-29 17:47 ` Stephen Hemminger
  1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2020-05-28 16:12 UTC (permalink / raw)
  To: wangyunjian; +Cc: dev, sthemmin, jerry.lilijun, xudingke, stable

On Thu, 28 May 2020 20:03:07 +0800
wangyunjian <wangyunjian@huawei.com> wrote:

> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> We do not need and should not allocate memory for device.name.
> The device.name should be set point to the devargs->name.
> 
> Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
>  drivers/bus/vmbus/linux/vmbus_bus.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
> index 3c924ee..31d0dd3 100644
> --- a/drivers/bus/vmbus/linux/vmbus_bus.c
> +++ b/drivers/bus/vmbus/linux/vmbus_bus.c
> @@ -242,9 +242,6 @@
>  		return -1;
>  
>  	dev->device.bus = &rte_vmbus_bus.bus;
> -	dev->device.name = strdup(name);
> -	if (!dev->device.name)
> -		goto error;
>  
>  	/* sysfs base directory
>  	 *   /sys/bus/vmbus/devices/7a08391f-f5a0-4ac0-9802-d13fd964f8df
> @@ -296,6 +293,7 @@
>  	}
>  
>  	dev->device.devargs = vmbus_devargs_lookup(dev);
> +	dev->device.name = dev->device.devargs->name;
>  
>  	/* device is valid, add in list (sorted) */
>  	VMBUS_LOG(DEBUG, "Adding vmbus device %s", name);

This doesn't seem right. devargs is not filled in unless devargs is used.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name
  2020-05-28 16:12 ` Stephen Hemminger
@ 2020-05-29  7:24   ` wangyunjian
  0 siblings, 0 replies; 5+ messages in thread
From: wangyunjian @ 2020-05-29  7:24 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, sthemmin, Lilijun (Jerry), xudingke, stable



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, May 29, 2020 12:13 AM
> To: wangyunjian <wangyunjian@huawei.com>
> Cc: dev@dpdk.org; sthemmin@microsoft.com; Lilijun (Jerry)
> <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for
> device.name
> 
> On Thu, 28 May 2020 20:03:07 +0800
> wangyunjian <wangyunjian@huawei.com> wrote:
> 
> > From: Yunjian Wang <wangyunjian@huawei.com>
> >
> > We do not need and should not allocate memory for device.name.
> > The device.name should be set point to the devargs->name.
> >
> > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > ---
> >  drivers/bus/vmbus/linux/vmbus_bus.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c
> b/drivers/bus/vmbus/linux/vmbus_bus.c
> > index 3c924ee..31d0dd3 100644
> > --- a/drivers/bus/vmbus/linux/vmbus_bus.c
> > +++ b/drivers/bus/vmbus/linux/vmbus_bus.c
> > @@ -242,9 +242,6 @@
> >  		return -1;
> >
> >  	dev->device.bus = &rte_vmbus_bus.bus;
> > -	dev->device.name = strdup(name);
> > -	if (!dev->device.name)
> > -		goto error;
> >
> >  	/* sysfs base directory
> >  	 *   /sys/bus/vmbus/devices/7a08391f-f5a0-4ac0-9802-d13fd964f8df
> > @@ -296,6 +293,7 @@
> >  	}
> >
> >  	dev->device.devargs = vmbus_devargs_lookup(dev);
> > +	dev->device.name = dev->device.devargs->name;
> >
> >  	/* device is valid, add in list (sorted) */
> >  	VMBUS_LOG(DEBUG, "Adding vmbus device %s", name);
> 
> This doesn't seem right. devargs is not filled in unless devargs is used.

At present, the memory allocated for the device.name is not released
in the error handling code. I have not found the relevant code to release
the vmbus device, so I am not sure how to release it corrently.

Generally, the pointer of device.name should be set to another pointer.
However, it was defined as "const" pointer and could not be released directly.

Thanks,
Yunjian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name
  2020-05-28 12:03 [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name wangyunjian
  2020-05-28 16:12 ` Stephen Hemminger
@ 2020-05-29 17:47 ` Stephen Hemminger
  2020-06-01  2:02   ` wangyunjian
  1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2020-05-29 17:47 UTC (permalink / raw)
  To: wangyunjian; +Cc: dev, sthemmin, jerry.lilijun, xudingke, stable

On Thu, 28 May 2020 20:03:07 +0800
wangyunjian <wangyunjian@huawei.com> wrote:

> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> We do not need and should not allocate memory for device.name.
> The device.name should be set point to the devargs->name.
> 
> Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>

The correct fix is more complex. The code needs to act more like PCI.
The name should be in the vmbus_device structure (like PCI).
And the devargs logic needs to be more involved and also handle
whitelist/blacklisting.

Here is a very rough idea what that would look like:

From edf90357a99c7970546ef00941a9593bca46d08e Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 29 May 2020 10:45:26 -0700
Subject: [PATCH] vmbus: support whitelist/blacklist

This makes VMBus work like PCI and support blacklist and
whitelist command line arguments.

It also moves the vmbus device name into the internal
structure.

This is compile tested only.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/bus/vmbus/linux/vmbus_bus.c | 30 ++++++++--------
 drivers/bus/vmbus/private.h         |  5 ++-
 drivers/bus/vmbus/rte_bus_vmbus.h   |  2 +-
 drivers/bus/vmbus/vmbus_common.c    | 53 ++++++++++++++++++++++++++---
 4 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 3c924eee1412..9162b30bb46c 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -14,7 +14,6 @@
 #include <rte_uuid.h>
 #include <rte_tailq.h>
 #include <rte_log.h>
-#include <rte_devargs.h>
 #include <rte_memory.h>
 #include <rte_malloc.h>
 #include <rte_bus_vmbus.h>
@@ -230,7 +229,7 @@ rte_vmbus_unmap_device(struct rte_vmbus_device *dev)
 
 /* Scan one vmbus sysfs entry, and fill the devices list from it. */
 static int
-vmbus_scan_one(const char *name)
+vmbus_scan_one(rte_uuid_t id, const char *name)
 {
 	struct rte_vmbus_device *dev, *dev2;
 	char filename[PATH_MAX];
@@ -242,14 +241,10 @@ vmbus_scan_one(const char *name)
 		return -1;
 
 	dev->device.bus = &rte_vmbus_bus.bus;
-	dev->device.name = strdup(name);
-	if (!dev->device.name)
-		goto error;
+	rte_uuid_copy(dev->device_id, id);
 
 	/* sysfs base directory
 	 *   /sys/bus/vmbus/devices/7a08391f-f5a0-4ac0-9802-d13fd964f8df
-	 * or on older kernel
-	 *   /sys/bus/vmbus/devices/vmbus_1
 	 */
 	snprintf(dirname, sizeof(dirname), "%s/%s",
 		 SYSFS_VMBUS_DEVICES, name);
@@ -265,11 +260,6 @@ vmbus_scan_one(const char *name)
 		return 0;
 	}
 
-	/* get device id */
-	snprintf(filename, sizeof(filename), "%s/device_id", dirname);
-	if (parse_sysfs_uuid(filename, dev->device_id) < 0)
-		goto error;
-
 	/* get relid */
 	snprintf(filename, sizeof(filename), "%s/id", dirname);
 	if (eal_parse_sysfs_value(filename, &tmp) < 0)
@@ -295,9 +285,6 @@ vmbus_scan_one(const char *name)
 		dev->device.numa_node = SOCKET_ID_ANY;
 	}
 
-	dev->device.devargs = vmbus_devargs_lookup(dev);
-
-	/* device is valid, add in list (sorted) */
 	VMBUS_LOG(DEBUG, "Adding vmbus device %s", name);
 
 	TAILQ_FOREACH(dev2, &rte_vmbus_bus.device_list, next) {
@@ -346,10 +333,21 @@ rte_vmbus_scan(void)
 	}
 
 	while ((e = readdir(dir)) != NULL) {
+		rte_uuid_t id;
+
 		if (e->d_name[0] == '.')
 			continue;
 
-		if (vmbus_scan_one(e->d_name) < 0)
+		if (rte_uuid_parse(e->d_name, id) != 0) {
+			VMBUS_LOG(DEBUG, "ignore '%s' non-uuid in vmbus/devices",
+				  e->d_name);
+			continue;
+		}
+
+		if (vmbus_ignore_device(id))
+			continue;
+
+		if (vmbus_scan_one(id, e->d_name) < 0)
 			goto error;
 	}
 	closedir(dir);
diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h
index f19b14e4a657..dd6a17c7238c 100644
--- a/drivers/bus/vmbus/private.h
+++ b/drivers/bus/vmbus/private.h
@@ -71,12 +71,15 @@ struct vmbus_channel {
 #define VMBUS_MAX_CHANNELS	64
 
 struct rte_devargs *
-vmbus_devargs_lookup(struct rte_vmbus_device *dev);
+vmbus_devargs_lookup(rte_uuid_t device_id);
+
+void vmbus_name_set(struct rte_vmbus_device *dev);
 
 int vmbus_chan_create(const struct rte_vmbus_device *device,
 		      uint16_t relid, uint16_t subid, uint8_t monitor_id,
 		      struct vmbus_channel **new_chan);
 
+bool vmbus_ignore_device(rte_uuid_t device_id);
 void vmbus_add_device(struct rte_vmbus_device *vmbus_dev);
 void vmbus_insert_device(struct rte_vmbus_device *exist_vmbus_dev,
 			 struct rte_vmbus_device *new_vmbus_dev);
diff --git a/drivers/bus/vmbus/rte_bus_vmbus.h b/drivers/bus/vmbus/rte_bus_vmbus.h
index 4cf73ce81513..62d067f19179 100644
--- a/drivers/bus/vmbus/rte_bus_vmbus.h
+++ b/drivers/bus/vmbus/rte_bus_vmbus.h
@@ -73,7 +73,7 @@ struct rte_vmbus_device {
 	uint32_t *int_page;		       /**< VMBUS interrupt page */
 	struct vmbus_channel *primary;	       /**< VMBUS primary channel */
 	struct vmbus_mon_page *monitor_page;   /**< VMBUS monitor page */
-
+	char name[RTE_UUID_STRLEN];	       /**< VMBUS uuid (ASCII) */
 	struct rte_intr_handle intr_handle;    /**< Interrupt handle */
 	struct rte_mem_resource resource[VMBUS_MAX_RESOURCE];
 };
diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c
index 3adef01c95de..ee2180f1d10c 100644
--- a/drivers/bus/vmbus/vmbus_common.c
+++ b/drivers/bus/vmbus/vmbus_common.c
@@ -61,6 +61,21 @@ vmbus_unmap_resource(void *requested_addr, size_t size)
 			  requested_addr);
 }
 
+void
+vmbus_name_set(struct rte_vmbus_device *dev)
+{
+	struct rte_devargs *devargs;
+
+	devargs = vmbus_devargs_lookup(dev->device_id);
+	rte_uuid_unparse(dev->device_id, dev->name, sizeof(dev->name));
+
+	/* if the device is not blacklisted, no rte_devargs exists for it. */
+	if (devargs)
+		dev->device.name = dev->device.devargs->name;
+	else
+		dev->device.name = dev->name;
+}
+
 /**
  * Match the VMBUS driver and device using UUID table
  *
@@ -85,6 +100,7 @@ vmbus_match(const struct rte_vmbus_driver *dr,
 
 	return false;
 }
+
 /*
  * If device ID match, call the devinit() function of the driver.
  */
@@ -99,10 +115,18 @@ vmbus_probe_one_driver(struct rte_vmbus_driver *dr,
 		return 1;	 /* not supported */
 
 	rte_uuid_unparse(dev->device_id, guid, sizeof(guid));
-	VMBUS_LOG(INFO, "VMBUS device %s on NUMA socket %i",
+	VMBUS_LOG(DEBUG, "VMBUS device %s on NUMA socket %i",
 		  guid, dev->device.numa_node);
 
-	/* TODO add blacklisted */
+	if (dev->device.devargs != NULL &&
+	    dev->device.devargs->policy == RTE_DEV_BLACKLISTED) {
+		VMBUS_LOG(INFO, "  Device is blacklisted, not initializing\n");
+		return 1;
+	}
+
+	/* Older kernel versions do not report NUMA node for vmbus */
+	if (dev->device.numa_node < 0)
+		dev->device.numa_node = 0;
 
 	/* map resources for device */
 	ret = rte_vmbus_map_device(dev);
@@ -210,15 +234,16 @@ vmbus_parse(const char *name, void *addr)
  *	-w 'vmbus:635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20'
  */
 struct rte_devargs *
-vmbus_devargs_lookup(struct rte_vmbus_device *dev)
+vmbus_devargs_lookup(rte_uuid_t device_id)
 {
 	struct rte_devargs *devargs;
-	rte_uuid_t addr;
 
 	RTE_EAL_DEVARGS_FOREACH("vmbus", devargs) {
+		rte_uuid_t addr;
+
 		vmbus_parse(devargs->name, &addr);
 
-		if (rte_uuid_compare(dev->device_id, addr) == 0)
+		if (rte_uuid_compare(device_id, addr) == 0)
 			return devargs;
 	}
 	return NULL;
@@ -285,6 +310,24 @@ vmbus_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
 	return NULL;
 }
 
+bool
+vmbus_ignore_device(rte_uuid_t device_id)
+{
+	struct rte_devargs *devargs = vmbus_devargs_lookup(device_id);
+
+	switch (rte_vmbus_bus.bus.conf.scan_mode) {
+	case RTE_BUS_SCAN_WHITELIST:
+		if (devargs && devargs->policy == RTE_DEV_WHITELISTED)
+			return false;
+		break;
+	case RTE_BUS_SCAN_UNDEFINED:
+	case RTE_BUS_SCAN_BLACKLIST:
+		if (devargs == NULL || devargs->policy != RTE_DEV_BLACKLISTED)
+			return false;
+		break;
+	}
+	return true;
+}
 
 struct rte_vmbus_bus rte_vmbus_bus = {
 	.bus = {
-- 
2.26.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name
  2020-05-29 17:47 ` Stephen Hemminger
@ 2020-06-01  2:02   ` wangyunjian
  0 siblings, 0 replies; 5+ messages in thread
From: wangyunjian @ 2020-06-01  2:02 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, sthemmin, Lilijun (Jerry), xudingke, stable

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Saturday, May 30, 2020 1:47 AM
> To: wangyunjian <wangyunjian@huawei.com>
> Cc: dev@dpdk.org; sthemmin@microsoft.com; Lilijun (Jerry)
> <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for
> device.name
> 
> On Thu, 28 May 2020 20:03:07 +0800
> wangyunjian <wangyunjian@huawei.com> wrote:
> 
> > From: Yunjian Wang <wangyunjian@huawei.com>
> >
> > We do not need and should not allocate memory for device.name.
> > The device.name should be set point to the devargs->name.
> >
> > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> 
> The correct fix is more complex. The code needs to act more like PCI.
> The name should be in the vmbus_device structure (like PCI).
> And the devargs logic needs to be more involved and also handle
> whitelist/blacklisting.
> 
> Here is a very rough idea what that would look like:
> 

Looks good , this patch is a more complete fix.

Thanks,
Yunjian

> From edf90357a99c7970546ef00941a9593bca46d08e Mon Sep 17 00:00:00
> 2001
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Fri, 29 May 2020 10:45:26 -0700
> Subject: [PATCH] vmbus: support whitelist/blacklist
> 
> This makes VMBus work like PCI and support blacklist and whitelist command
> line arguments.
> 
> It also moves the vmbus device name into the internal structure.
> 
> This is compile tested only.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/bus/vmbus/linux/vmbus_bus.c | 30 ++++++++--------
>  drivers/bus/vmbus/private.h         |  5 ++-
>  drivers/bus/vmbus/rte_bus_vmbus.h   |  2 +-
>  drivers/bus/vmbus/vmbus_common.c    | 53
> ++++++++++++++++++++++++++---
>  4 files changed, 67 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c
> b/drivers/bus/vmbus/linux/vmbus_bus.c
> index 3c924eee1412..9162b30bb46c 100644
> --- a/drivers/bus/vmbus/linux/vmbus_bus.c
> +++ b/drivers/bus/vmbus/linux/vmbus_bus.c
> @@ -14,7 +14,6 @@
>  #include <rte_uuid.h>
>  #include <rte_tailq.h>
>  #include <rte_log.h>
> -#include <rte_devargs.h>
>  #include <rte_memory.h>
>  #include <rte_malloc.h>
>  #include <rte_bus_vmbus.h>
> @@ -230,7 +229,7 @@ rte_vmbus_unmap_device(struct rte_vmbus_device
> *dev)
> 
>  /* Scan one vmbus sysfs entry, and fill the devices list from it. */  static int
> -vmbus_scan_one(const char *name)
> +vmbus_scan_one(rte_uuid_t id, const char *name)
>  {
>  	struct rte_vmbus_device *dev, *dev2;
>  	char filename[PATH_MAX];
> @@ -242,14 +241,10 @@ vmbus_scan_one(const char *name)
>  		return -1;
> 
>  	dev->device.bus = &rte_vmbus_bus.bus;
> -	dev->device.name = strdup(name);
> -	if (!dev->device.name)
> -		goto error;
> +	rte_uuid_copy(dev->device_id, id);
> 
>  	/* sysfs base directory
>  	 *   /sys/bus/vmbus/devices/7a08391f-f5a0-4ac0-9802-d13fd964f8df
> -	 * or on older kernel
> -	 *   /sys/bus/vmbus/devices/vmbus_1
>  	 */
>  	snprintf(dirname, sizeof(dirname), "%s/%s",
>  		 SYSFS_VMBUS_DEVICES, name);
> @@ -265,11 +260,6 @@ vmbus_scan_one(const char *name)
>  		return 0;
>  	}
> 
> -	/* get device id */
> -	snprintf(filename, sizeof(filename), "%s/device_id", dirname);
> -	if (parse_sysfs_uuid(filename, dev->device_id) < 0)
> -		goto error;
> -
>  	/* get relid */
>  	snprintf(filename, sizeof(filename), "%s/id", dirname);
>  	if (eal_parse_sysfs_value(filename, &tmp) < 0) @@ -295,9 +285,6 @@
> vmbus_scan_one(const char *name)
>  		dev->device.numa_node = SOCKET_ID_ANY;
>  	}
> 
> -	dev->device.devargs = vmbus_devargs_lookup(dev);
> -
> -	/* device is valid, add in list (sorted) */
>  	VMBUS_LOG(DEBUG, "Adding vmbus device %s", name);
> 
>  	TAILQ_FOREACH(dev2, &rte_vmbus_bus.device_list, next) { @@ -346,10
> +333,21 @@ rte_vmbus_scan(void)
>  	}
> 
>  	while ((e = readdir(dir)) != NULL) {
> +		rte_uuid_t id;
> +
>  		if (e->d_name[0] == '.')
>  			continue;
> 
> -		if (vmbus_scan_one(e->d_name) < 0)
> +		if (rte_uuid_parse(e->d_name, id) != 0) {
> +			VMBUS_LOG(DEBUG, "ignore '%s' non-uuid in vmbus/devices",
> +				  e->d_name);
> +			continue;
> +		}
> +
> +		if (vmbus_ignore_device(id))
> +			continue;
> +
> +		if (vmbus_scan_one(id, e->d_name) < 0)
>  			goto error;
>  	}
>  	closedir(dir);
> diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h index
> f19b14e4a657..dd6a17c7238c 100644
> --- a/drivers/bus/vmbus/private.h
> +++ b/drivers/bus/vmbus/private.h
> @@ -71,12 +71,15 @@ struct vmbus_channel {
>  #define VMBUS_MAX_CHANNELS	64
> 
>  struct rte_devargs *
> -vmbus_devargs_lookup(struct rte_vmbus_device *dev);
> +vmbus_devargs_lookup(rte_uuid_t device_id);
> +
> +void vmbus_name_set(struct rte_vmbus_device *dev);
> 
>  int vmbus_chan_create(const struct rte_vmbus_device *device,
>  		      uint16_t relid, uint16_t subid, uint8_t monitor_id,
>  		      struct vmbus_channel **new_chan);
> 
> +bool vmbus_ignore_device(rte_uuid_t device_id);
>  void vmbus_add_device(struct rte_vmbus_device *vmbus_dev);  void
> vmbus_insert_device(struct rte_vmbus_device *exist_vmbus_dev,
>  			 struct rte_vmbus_device *new_vmbus_dev); diff --git
> a/drivers/bus/vmbus/rte_bus_vmbus.h b/drivers/bus/vmbus/rte_bus_vmbus.h
> index 4cf73ce81513..62d067f19179 100644
> --- a/drivers/bus/vmbus/rte_bus_vmbus.h
> +++ b/drivers/bus/vmbus/rte_bus_vmbus.h
> @@ -73,7 +73,7 @@ struct rte_vmbus_device {
>  	uint32_t *int_page;		       /**< VMBUS interrupt page */
>  	struct vmbus_channel *primary;	       /**< VMBUS primary channel
> */
>  	struct vmbus_mon_page *monitor_page;   /**< VMBUS monitor page
> */
> -
> +	char name[RTE_UUID_STRLEN];	       /**< VMBUS uuid (ASCII) */
>  	struct rte_intr_handle intr_handle;    /**< Interrupt handle */
>  	struct rte_mem_resource resource[VMBUS_MAX_RESOURCE];  }; diff
> --git a/drivers/bus/vmbus/vmbus_common.c
> b/drivers/bus/vmbus/vmbus_common.c
> index 3adef01c95de..ee2180f1d10c 100644
> --- a/drivers/bus/vmbus/vmbus_common.c
> +++ b/drivers/bus/vmbus/vmbus_common.c
> @@ -61,6 +61,21 @@ vmbus_unmap_resource(void *requested_addr, size_t
> size)
>  			  requested_addr);
>  }
> 
> +void
> +vmbus_name_set(struct rte_vmbus_device *dev) {
> +	struct rte_devargs *devargs;
> +
> +	devargs = vmbus_devargs_lookup(dev->device_id);
> +	rte_uuid_unparse(dev->device_id, dev->name, sizeof(dev->name));
> +
> +	/* if the device is not blacklisted, no rte_devargs exists for it. */
> +	if (devargs)
> +		dev->device.name = dev->device.devargs->name;
> +	else
> +		dev->device.name = dev->name;
> +}
> +
>  /**
>   * Match the VMBUS driver and device using UUID table
>   *
> @@ -85,6 +100,7 @@ vmbus_match(const struct rte_vmbus_driver *dr,
> 
>  	return false;
>  }
> +
>  /*
>   * If device ID match, call the devinit() function of the driver.
>   */
> @@ -99,10 +115,18 @@ vmbus_probe_one_driver(struct rte_vmbus_driver
> *dr,
>  		return 1;	 /* not supported */
> 
>  	rte_uuid_unparse(dev->device_id, guid, sizeof(guid));
> -	VMBUS_LOG(INFO, "VMBUS device %s on NUMA socket %i",
> +	VMBUS_LOG(DEBUG, "VMBUS device %s on NUMA socket %i",
>  		  guid, dev->device.numa_node);
> 
> -	/* TODO add blacklisted */
> +	if (dev->device.devargs != NULL &&
> +	    dev->device.devargs->policy == RTE_DEV_BLACKLISTED) {
> +		VMBUS_LOG(INFO, "  Device is blacklisted, not initializing\n");
> +		return 1;
> +	}
> +
> +	/* Older kernel versions do not report NUMA node for vmbus */
> +	if (dev->device.numa_node < 0)
> +		dev->device.numa_node = 0;
> 
>  	/* map resources for device */
>  	ret = rte_vmbus_map_device(dev);
> @@ -210,15 +234,16 @@ vmbus_parse(const char *name, void *addr)
>   *	-w 'vmbus:635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20'
>   */
>  struct rte_devargs *
> -vmbus_devargs_lookup(struct rte_vmbus_device *dev)
> +vmbus_devargs_lookup(rte_uuid_t device_id)
>  {
>  	struct rte_devargs *devargs;
> -	rte_uuid_t addr;
> 
>  	RTE_EAL_DEVARGS_FOREACH("vmbus", devargs) {
> +		rte_uuid_t addr;
> +
>  		vmbus_parse(devargs->name, &addr);
> 
> -		if (rte_uuid_compare(dev->device_id, addr) == 0)
> +		if (rte_uuid_compare(device_id, addr) == 0)
>  			return devargs;
>  	}
>  	return NULL;
> @@ -285,6 +310,24 @@ vmbus_find_device(const struct rte_device *start,
> rte_dev_cmp_t cmp,
>  	return NULL;
>  }
> 
> +bool
> +vmbus_ignore_device(rte_uuid_t device_id) {
> +	struct rte_devargs *devargs = vmbus_devargs_lookup(device_id);
> +
> +	switch (rte_vmbus_bus.bus.conf.scan_mode) {
> +	case RTE_BUS_SCAN_WHITELIST:
> +		if (devargs && devargs->policy == RTE_DEV_WHITELISTED)
> +			return false;
> +		break;
> +	case RTE_BUS_SCAN_UNDEFINED:
> +	case RTE_BUS_SCAN_BLACKLIST:
> +		if (devargs == NULL || devargs->policy != RTE_DEV_BLACKLISTED)
> +			return false;
> +		break;
> +	}
> +	return true;
> +}
> 
>  struct rte_vmbus_bus rte_vmbus_bus = {
>  	.bus = {
> --
> 2.26.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-06-01  2:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-28 12:03 [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name wangyunjian
2020-05-28 16:12 ` Stephen Hemminger
2020-05-29  7:24   ` wangyunjian
2020-05-29 17:47 ` Stephen Hemminger
2020-06-01  2:02   ` wangyunjian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).