From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A112AA04EF; Mon, 1 Jun 2020 04:02:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 99CAF1D406; Mon, 1 Jun 2020 04:02:18 +0200 (CEST) Received: from huawei.com (szxga02-in.huawei.com [45.249.212.188]) by dpdk.org (Postfix) with ESMTP id BB86A1BF3C; Mon, 1 Jun 2020 04:02:15 +0200 (CEST) Received: from DGGEMM401-HUB.china.huawei.com (unknown [172.30.72.55]) by Forcepoint Email with ESMTP id B7C57EA98C3E9DD79B52; Mon, 1 Jun 2020 10:02:13 +0800 (CST) Received: from DGGEMM513-MBX.china.huawei.com ([169.254.1.135]) by DGGEMM401-HUB.china.huawei.com ([10.3.20.209]) with mapi id 14.03.0487.000; Mon, 1 Jun 2020 10:02:02 +0800 From: wangyunjian To: Stephen Hemminger CC: "dev@dpdk.org" , "sthemmin@microsoft.com" , "Lilijun (Jerry)" , xudingke , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name Thread-Index: AQHWNOf+nvfj03Fhn0u+a/jtvEN/Gai+0oEAgAQzZxA= Date: Mon, 1 Jun 2020 02:02:02 +0000 Message-ID: <34EFBCA9F01B0748BEB6B629CE643AE60D02D5FD@dggemm513-mbx.china.huawei.com> References: <768c74d06680b93b2ce6bbf0813d1910666888dc.1590666521.git.wangyunjian@huawei.com> <20200529104726.0a76d7bf@hermes.lan> In-Reply-To: <20200529104726.0a76d7bf@hermes.lan> Accept-Language: en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.173.251.152] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for device.name X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Saturday, May 30, 2020 1:47 AM > To: wangyunjian > Cc: dev@dpdk.org; sthemmin@microsoft.com; Lilijun (Jerry) > ; xudingke ; > stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] bus/vmbus: fix wrong allocation for > device.name >=20 > On Thu, 28 May 2020 20:03:07 +0800 > wangyunjian wrote: >=20 > > From: Yunjian Wang > > > > 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 >=20 > 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. >=20 > Here is a very rough idea what that would look like: >=20 Looks good , this patch is a more complete fix. Thanks, Yunjian > From edf90357a99c7970546ef00941a9593bca46d08e Mon Sep 17 00:00:00 > 2001 > From: Stephen Hemminger > Date: Fri, 29 May 2020 10:45:26 -0700 > Subject: [PATCH] vmbus: support whitelist/blacklist >=20 > This makes VMBus work like PCI and support blacklist and whitelist comman= d > line arguments. >=20 > It also moves the vmbus device name into the internal structure. >=20 > This is compile tested only. >=20 > Signed-off-by: Stephen Hemminger > --- > 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(-) >=20 > 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 > #include > #include > -#include > #include > #include > #include > @@ -230,7 +229,7 @@ rte_vmbus_unmap_device(struct rte_vmbus_device > *dev) >=20 > /* Scan one vmbus sysfs entry, and fill the devices list from it. */ st= atic 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; >=20 > dev->device.bus =3D &rte_vmbus_bus.bus; > - dev->device.name =3D strdup(name); > - if (!dev->device.name) > - goto error; > + rte_uuid_copy(dev->device_id, id); >=20 > /* 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; > } >=20 > - /* 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 =3D SOCKET_ID_ANY; > } >=20 > - dev->device.devargs =3D vmbus_devargs_lookup(dev); > - > - /* device is valid, add in list (sorted) */ > VMBUS_LOG(DEBUG, "Adding vmbus device %s", name); >=20 > TAILQ_FOREACH(dev2, &rte_vmbus_bus.device_list, next) { @@ -346,10 > +333,21 @@ rte_vmbus_scan(void) > } >=20 > while ((e =3D readdir(dir)) !=3D NULL) { > + rte_uuid_t id; > + > if (e->d_name[0] =3D=3D '.') > continue; >=20 > - if (vmbus_scan_one(e->d_name) < 0) > + if (rte_uuid_parse(e->d_name, id) !=3D 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 in= dex > 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 >=20 > 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); >=20 > 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); >=20 > +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); > } >=20 > +void > +vmbus_name_set(struct rte_vmbus_device *dev) { > + struct rte_devargs *devargs; > + > + devargs =3D 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 =3D dev->device.devargs->name; > + else > + dev->device.name =3D dev->name; > +} > + > /** > * Match the VMBUS driver and device using UUID table > * > @@ -85,6 +100,7 @@ vmbus_match(const struct rte_vmbus_driver *dr, >=20 > 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 */ >=20 > 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); >=20 > - /* TODO add blacklisted */ > + if (dev->device.devargs !=3D NULL && > + dev->device.devargs->policy =3D=3D 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 =3D 0; >=20 > /* map resources for device */ > ret =3D rte_vmbus_map_device(dev); > @@ -210,15 +234,16 @@ vmbus_parse(const char *name, void *addr) > * -w 'vmbus:635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=3D20' > */ > 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; >=20 > RTE_EAL_DEVARGS_FOREACH("vmbus", devargs) { > + rte_uuid_t addr; > + > vmbus_parse(devargs->name, &addr); >=20 > - if (rte_uuid_compare(dev->device_id, addr) =3D=3D 0) > + if (rte_uuid_compare(device_id, addr) =3D=3D 0) > return devargs; > } > return NULL; > @@ -285,6 +310,24 @@ vmbus_find_device(const struct rte_device *start, > rte_dev_cmp_t cmp, > return NULL; > } >=20 > +bool > +vmbus_ignore_device(rte_uuid_t device_id) { > + struct rte_devargs *devargs =3D vmbus_devargs_lookup(device_id); > + > + switch (rte_vmbus_bus.bus.conf.scan_mode) { > + case RTE_BUS_SCAN_WHITELIST: > + if (devargs && devargs->policy =3D=3D RTE_DEV_WHITELISTED) > + return false; > + break; > + case RTE_BUS_SCAN_UNDEFINED: > + case RTE_BUS_SCAN_BLACKLIST: > + if (devargs =3D=3D NULL || devargs->policy !=3D RTE_DEV_BLACKLISTED) > + return false; > + break; > + } > + return true; > +} >=20 > struct rte_vmbus_bus rte_vmbus_bus =3D { > .bus =3D { > -- > 2.26.2