* [PATCH] drivers/bus: set device NUMA node to unknown by default @ 2022-09-29 12:05 Olivier Matz 2022-09-30 7:10 ` David Marchand 0 siblings, 1 reply; 8+ messages in thread From: Olivier Matz @ 2022-09-29 12:05 UTC (permalink / raw) To: dev Cc: Ray Kinsella, Parav Pandit, Xueming Li, Hemant Agrawal, Sachin Saxena, Stephen Hemminger, Long Li The dev->device.numa_node field is set by each bus driver for every device it manages to indicate on which NUMA node this device lies. When this information is unknown, the assigned value is not consistent across the bus drivers. Set the default value to SOCKET_ID_ANY (-1) by all bus drivers when the NUMA information is unavailable. Signed-off-by: Olivier Matz <olivier.matz@6wind.com> --- doc/guides/rel_notes/deprecation.rst | 7 ------- doc/guides/rel_notes/release_22_11.rst | 6 ++++++ drivers/bus/auxiliary/auxiliary_common.c | 8 ++------ drivers/bus/auxiliary/linux/auxiliary.c | 13 +++++-------- drivers/bus/dpaa/dpaa_bus.c | 1 + drivers/bus/fslmc/fslmc_bus.c | 1 + drivers/bus/pci/bsd/pci.c | 2 +- drivers/bus/pci/linux/pci.c | 16 ++++++---------- drivers/bus/pci/pci_common.c | 8 ++------ drivers/bus/pci/windows/pci.c | 1 - drivers/bus/vmbus/linux/vmbus_bus.c | 1 - drivers/bus/vmbus/vmbus_common.c | 8 ++------ 12 files changed, 26 insertions(+), 46 deletions(-) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index be231ff8d2..75645f0a9a 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -36,13 +36,6 @@ Deprecation Notices ``__atomic_thread_fence`` must be used for patches that need to be merged in 20.08 onwards. This change will not introduce any performance degradation. -* bus: The ``dev->device.numa_node`` field is set by each bus driver for - every device it manages to indicate on which NUMA node this device lies. - When this information is unknown, the assigned value is not consistent - across the bus drivers. - In DPDK 22.11, the default value will be set to -1 by all bus drivers - when the NUMA information is unavailable. - * kni: The KNI kernel module and library are not recommended for use by new applications - other technologies such as virtio-user are recommended instead. Following the DPDK technical board diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst index 0231959874..5989e4a04f 100644 --- a/doc/guides/rel_notes/release_22_11.rst +++ b/doc/guides/rel_notes/release_22_11.rst @@ -266,6 +266,12 @@ ABI Changes * eventdev: Added ``weight`` and ``affinity`` fields to ``rte_event_queue_conf`` structure. +* bus: Changed the device numa node to -1 when NUMA information is unavailable. + The ``dev->device.numa_node`` field is set by each bus driver for + every device it manages to indicate on which NUMA node this device lies. + When this information is unknown, the assigned value was not consistent + across the bus drivers. + Known Issues ------------ diff --git a/drivers/bus/auxiliary/auxiliary_common.c b/drivers/bus/auxiliary/auxiliary_common.c index 259ff152c4..6bb1fe7c96 100644 --- a/drivers/bus/auxiliary/auxiliary_common.c +++ b/drivers/bus/auxiliary/auxiliary_common.c @@ -105,12 +105,8 @@ rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver *drv, return -1; } - if (dev->device.numa_node < 0) { - if (rte_socket_count() > 1) - AUXILIARY_LOG(INFO, "Device %s is not NUMA-aware, defaulting socket to 0", - dev->name); - dev->device.numa_node = 0; - } + if (dev->device.numa_node < 0 && rte_socket_count() > 1) + RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware\n", dev->name); iova_mode = rte_eal_iova_mode(); if ((drv->drv_flags & RTE_AUXILIARY_DRV_NEED_IOVA_AS_VA) > 0 && diff --git a/drivers/bus/auxiliary/linux/auxiliary.c b/drivers/bus/auxiliary/linux/auxiliary.c index d4c564cd78..02fc9285dc 100644 --- a/drivers/bus/auxiliary/linux/auxiliary.c +++ b/drivers/bus/auxiliary/linux/auxiliary.c @@ -40,14 +40,11 @@ auxiliary_scan_one(const char *dirname, const char *name) /* Get NUMA node, default to 0 if not present */ snprintf(filename, sizeof(filename), "%s/%s/numa_node", dirname, name); - if (access(filename, F_OK) != -1) { - if (eal_parse_sysfs_value(filename, &tmp) == 0) - dev->device.numa_node = tmp; - else - dev->device.numa_node = -1; - } else { - dev->device.numa_node = 0; - } + if (access(filename, F_OK) == 0 && + eal_parse_sysfs_value(filename, &tmp) == 0) + dev->device.numa_node = tmp; + else + dev->device.numa_node = SOCKET_ID_ANY; auxiliary_on_scan(dev); diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c index 682427ba2c..447b222a76 100644 --- a/drivers/bus/dpaa/dpaa_bus.c +++ b/drivers/bus/dpaa/dpaa_bus.c @@ -179,6 +179,7 @@ dpaa_create_device_list(void) } dev->device.bus = &rte_dpaa_bus.bus; + dev->device.numa_node = SOCKET_ID_ANY; /* Allocate interrupt handle instance */ dev->intr_handle = diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c index 8503004e3d..57bfb5111a 100644 --- a/drivers/bus/fslmc/fslmc_bus.c +++ b/drivers/bus/fslmc/fslmc_bus.c @@ -156,6 +156,7 @@ scan_one_fslmc_device(char *dev_name) } dev->device.bus = &rte_fslmc_bus.bus; + dev->device.numa_node = SOCKET_ID_ANY; /* Allocate interrupt instance */ dev->intr_handle = diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c index 844d145fed..7459d15c7e 100644 --- a/drivers/bus/pci/bsd/pci.c +++ b/drivers/bus/pci/bsd/pci.c @@ -246,7 +246,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf) dev->max_vfs = 0; /* FreeBSD has no NUMA support (yet) */ - dev->device.numa_node = 0; + dev->device.numa_node = SOCKET_ID_ANY; pci_common_set(dev); diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c index c8703d52f3..ade17079fc 100644 --- a/drivers/bus/pci/linux/pci.c +++ b/drivers/bus/pci/linux/pci.c @@ -283,17 +283,13 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr) } /* get numa node, default to 0 if not present */ - snprintf(filename, sizeof(filename), "%s/numa_node", - dirname); + snprintf(filename, sizeof(filename), "%s/numa_node", dirname); - if (access(filename, F_OK) != -1) { - if (eal_parse_sysfs_value(filename, &tmp) == 0) - dev->device.numa_node = tmp; - else - dev->device.numa_node = -1; - } else { - dev->device.numa_node = 0; - } + if (access(filename, F_OK) == 0 && + eal_parse_sysfs_value(filename, &tmp) == 0) + dev->device.numa_node = tmp; + else + dev->device.numa_node = SOCKET_ID_ANY; pci_common_set(dev); diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c index 5ea72bcf23..a59c5b4286 100644 --- a/drivers/bus/pci/pci_common.c +++ b/drivers/bus/pci/pci_common.c @@ -235,12 +235,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr, return 1; } - if (dev->device.numa_node < 0) { - if (rte_socket_count() > 1) - RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware, defaulting socket to 0\n", - dev->name); - dev->device.numa_node = 0; - } + if (dev->device.numa_node < 0 && rte_socket_count() > 1) + RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware\n", dev->name); already_probed = rte_dev_is_probed(&dev->device); if (already_probed && !(dr->drv_flags & RTE_PCI_DRV_PROBE_AGAIN)) { diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c index 3f7a8b9432..5cf05ce1a0 100644 --- a/drivers/bus/pci/windows/pci.c +++ b/drivers/bus/pci/windows/pci.c @@ -249,7 +249,6 @@ get_device_resource_info(HDEVINFO dev_info, DWORD error = GetLastError(); if (error == ERROR_NOT_FOUND) { /* On older CPUs, NUMA is not bound to PCIe locality. */ - dev->device.numa_node = 0; return ERROR_SUCCESS; } RTE_LOG_WIN32_ERR("SetupDiGetDevicePropertyW" diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c index f502783f7a..01d8111b85 100644 --- a/drivers/bus/vmbus/linux/vmbus_bus.c +++ b/drivers/bus/vmbus/linux/vmbus_bus.c @@ -293,7 +293,6 @@ vmbus_scan_one(const char *name) goto error; dev->device.numa_node = tmp; } else { - /* if no NUMA support, set default to 0 */ dev->device.numa_node = SOCKET_ID_ANY; } diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c index 03b39c82b7..8d32d66504 100644 --- a/drivers/bus/vmbus/vmbus_common.c +++ b/drivers/bus/vmbus/vmbus_common.c @@ -111,12 +111,8 @@ vmbus_probe_one_driver(struct rte_vmbus_driver *dr, /* reference driver structure */ dev->driver = dr; - if (dev->device.numa_node < 0) { - if (rte_socket_count() > 1) - VMBUS_LOG(INFO, "Device %s is not NUMA-aware, defaulting socket to 0", - guid); - dev->device.numa_node = 0; - } + if (dev->device.numa_node < 0 && rte_socket_count() > 1) + VMBUS_LOG(INFO, "Device %s is not NUMA-aware", guid); /* call the driver probe() function */ VMBUS_LOG(INFO, " probe driver: %s", dr->driver.name); -- 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/bus: set device NUMA node to unknown by default 2022-09-29 12:05 [PATCH] drivers/bus: set device NUMA node to unknown by default Olivier Matz @ 2022-09-30 7:10 ` David Marchand 2022-09-30 8:11 ` Olivier Matz 2022-10-04 14:58 ` [PATCH v2] " Olivier Matz 0 siblings, 2 replies; 8+ messages in thread From: David Marchand @ 2022-09-30 7:10 UTC (permalink / raw) To: Olivier Matz Cc: dev, Ray Kinsella, Parav Pandit, Xueming Li, Hemant Agrawal, Sachin Saxena, Stephen Hemminger, Long Li On Thu, Sep 29, 2022 at 2:05 PM Olivier Matz <olivier.matz@6wind.com> wrote: > > The dev->device.numa_node field is set by each bus driver for > every device it manages to indicate on which NUMA node this device lies. > > When this information is unknown, the assigned value is not consistent > across the bus drivers. > > Set the default value to SOCKET_ID_ANY (-1) by all bus drivers > when the NUMA information is unavailable. > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com> - The dma/idxd driver, which has its own bus, seems ok, though I would align its code for readability. @@ -322,7 +322,7 @@ dsa_scan(void) while ((wq = readdir(dev_dir)) != NULL) { struct rte_dsa_device *dev; - int numa_node = -1; + int numa_node = SOCKET_ID_ANY; if (strncmp(wq->d_name, "wq", 2) != 0) continue; - There is an impact on upper classes of devices. For ethdev, a port inherits the numa_node value from the rte_device object. Yet, rte_eth_dev_socket_id() is described as: * @return * The NUMA socket ID to which the Ethernet device is connected or * a default of zero if the socket could not be determined. * -1 is returned is the port_id value is out of range. -- David marchand ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/bus: set device NUMA node to unknown by default 2022-09-30 7:10 ` David Marchand @ 2022-09-30 8:11 ` Olivier Matz 2022-10-04 14:58 ` [PATCH v2] " Olivier Matz 1 sibling, 0 replies; 8+ messages in thread From: Olivier Matz @ 2022-09-30 8:11 UTC (permalink / raw) To: David Marchand Cc: dev, Ray Kinsella, Parav Pandit, Xueming Li, Hemant Agrawal, Sachin Saxena, Stephen Hemminger, Long Li On Fri, Sep 30, 2022 at 09:10:55AM +0200, David Marchand wrote: > On Thu, Sep 29, 2022 at 2:05 PM Olivier Matz <olivier.matz@6wind.com> wrote: > > > > The dev->device.numa_node field is set by each bus driver for > > every device it manages to indicate on which NUMA node this device lies. > > > > When this information is unknown, the assigned value is not consistent > > across the bus drivers. > > > > Set the default value to SOCKET_ID_ANY (-1) by all bus drivers > > when the NUMA information is unavailable. > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com> > > - The dma/idxd driver, which has its own bus, seems ok, though I > would align its code for readability. > > @@ -322,7 +322,7 @@ dsa_scan(void) > > while ((wq = readdir(dev_dir)) != NULL) { > struct rte_dsa_device *dev; > - int numa_node = -1; > + int numa_node = SOCKET_ID_ANY; > > if (strncmp(wq->d_name, "wq", 2) != 0) > continue; > > > - There is an impact on upper classes of devices. > > For ethdev, a port inherits the numa_node value from the rte_device object. > Yet, rte_eth_dev_socket_id() is described as: > * @return > * The NUMA socket ID to which the Ethernet device is connected or > * a default of zero if the socket could not be determined. > * -1 is returned is the port_id value is out of range. Good catches, I'll fix them in v2. Thanks, Olivier ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] drivers/bus: set device NUMA node to unknown by default 2022-09-30 7:10 ` David Marchand 2022-09-30 8:11 ` Olivier Matz @ 2022-10-04 14:58 ` Olivier Matz 2022-10-05 8:52 ` David Marchand 2022-10-06 19:31 ` David Marchand 1 sibling, 2 replies; 8+ messages in thread From: Olivier Matz @ 2022-10-04 14:58 UTC (permalink / raw) To: dev Cc: Ray Kinsella, Parav Pandit, Xueming Li, Hemant Agrawal, Sachin Saxena, Stephen Hemminger, Long Li, david.marchand The dev->device.numa_node field is set by each bus driver for every device it manages to indicate on which NUMA node this device lies. When this information is unknown, the assigned value is not consistent across the bus drivers. Set the default value to SOCKET_ID_ANY (-1) by all bus drivers when the NUMA information is unavailable. This change impacts rte_eth_dev_socket_id() in the same manner. Signed-off-by: Olivier Matz <olivier.matz@6wind.com> --- v2 * use SOCKET_ID_ANY instead of -1 in drivers/dma/idxd (David) * document the behavior change of rte_eth_dev_socket_id() * fix few examples where rte_eth_dev_socket_id() was expected to return 0 on unknown socket doc/guides/rel_notes/deprecation.rst | 7 ------- doc/guides/rel_notes/release_22_11.rst | 6 ++++++ drivers/bus/auxiliary/auxiliary_common.c | 8 ++------ drivers/bus/auxiliary/linux/auxiliary.c | 13 +++++-------- drivers/bus/dpaa/dpaa_bus.c | 1 + drivers/bus/fslmc/fslmc_bus.c | 1 + drivers/bus/pci/bsd/pci.c | 2 +- drivers/bus/pci/linux/pci.c | 16 ++++++---------- drivers/bus/pci/pci_common.c | 8 ++------ drivers/bus/pci/windows/pci.c | 1 - drivers/bus/vmbus/linux/vmbus_bus.c | 1 - drivers/bus/vmbus/vmbus_common.c | 8 ++------ drivers/dma/idxd/idxd_bus.c | 3 ++- examples/distributor/main.c | 4 ++-- examples/flow_classify/flow_classify.c | 2 ++ examples/rxtx_callbacks/main.c | 2 +- lib/ethdev/rte_ethdev.h | 4 ++-- 17 files changed, 35 insertions(+), 52 deletions(-) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index a991fa14de..2a1a6ff899 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -33,13 +33,6 @@ Deprecation Notices ``__atomic_thread_fence`` must be used for patches that need to be merged in 20.08 onwards. This change will not introduce any performance degradation. -* bus: The ``dev->device.numa_node`` field is set by each bus driver for - every device it manages to indicate on which NUMA node this device lies. - When this information is unknown, the assigned value is not consistent - across the bus drivers. - In DPDK 22.11, the default value will be set to -1 by all bus drivers - when the NUMA information is unavailable. - * kni: The KNI kernel module and library are not recommended for use by new applications - other technologies such as virtio-user are recommended instead. Following the DPDK technical board diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst index 53fe21453c..d52f823694 100644 --- a/doc/guides/rel_notes/release_22_11.rst +++ b/doc/guides/rel_notes/release_22_11.rst @@ -317,6 +317,12 @@ ABI Changes * eventdev: Added ``weight`` and ``affinity`` fields to ``rte_event_queue_conf`` structure. +* bus: Changed the device numa node to -1 when NUMA information is unavailable. + The ``dev->device.numa_node`` field is set by each bus driver for + every device it manages to indicate on which NUMA node this device lies. + When this information is unknown, the assigned value was not consistent + across the bus drivers. This similarly impacts ``rte_eth_dev_socket_id()``. + Known Issues ------------ diff --git a/drivers/bus/auxiliary/auxiliary_common.c b/drivers/bus/auxiliary/auxiliary_common.c index 259ff152c4..6bb1fe7c96 100644 --- a/drivers/bus/auxiliary/auxiliary_common.c +++ b/drivers/bus/auxiliary/auxiliary_common.c @@ -105,12 +105,8 @@ rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver *drv, return -1; } - if (dev->device.numa_node < 0) { - if (rte_socket_count() > 1) - AUXILIARY_LOG(INFO, "Device %s is not NUMA-aware, defaulting socket to 0", - dev->name); - dev->device.numa_node = 0; - } + if (dev->device.numa_node < 0 && rte_socket_count() > 1) + RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware\n", dev->name); iova_mode = rte_eal_iova_mode(); if ((drv->drv_flags & RTE_AUXILIARY_DRV_NEED_IOVA_AS_VA) > 0 && diff --git a/drivers/bus/auxiliary/linux/auxiliary.c b/drivers/bus/auxiliary/linux/auxiliary.c index d4c564cd78..02fc9285dc 100644 --- a/drivers/bus/auxiliary/linux/auxiliary.c +++ b/drivers/bus/auxiliary/linux/auxiliary.c @@ -40,14 +40,11 @@ auxiliary_scan_one(const char *dirname, const char *name) /* Get NUMA node, default to 0 if not present */ snprintf(filename, sizeof(filename), "%s/%s/numa_node", dirname, name); - if (access(filename, F_OK) != -1) { - if (eal_parse_sysfs_value(filename, &tmp) == 0) - dev->device.numa_node = tmp; - else - dev->device.numa_node = -1; - } else { - dev->device.numa_node = 0; - } + if (access(filename, F_OK) == 0 && + eal_parse_sysfs_value(filename, &tmp) == 0) + dev->device.numa_node = tmp; + else + dev->device.numa_node = SOCKET_ID_ANY; auxiliary_on_scan(dev); diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c index 682427ba2c..447b222a76 100644 --- a/drivers/bus/dpaa/dpaa_bus.c +++ b/drivers/bus/dpaa/dpaa_bus.c @@ -179,6 +179,7 @@ dpaa_create_device_list(void) } dev->device.bus = &rte_dpaa_bus.bus; + dev->device.numa_node = SOCKET_ID_ANY; /* Allocate interrupt handle instance */ dev->intr_handle = diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c index 8503004e3d..57bfb5111a 100644 --- a/drivers/bus/fslmc/fslmc_bus.c +++ b/drivers/bus/fslmc/fslmc_bus.c @@ -156,6 +156,7 @@ scan_one_fslmc_device(char *dev_name) } dev->device.bus = &rte_fslmc_bus.bus; + dev->device.numa_node = SOCKET_ID_ANY; /* Allocate interrupt instance */ dev->intr_handle = diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c index 844d145fed..7459d15c7e 100644 --- a/drivers/bus/pci/bsd/pci.c +++ b/drivers/bus/pci/bsd/pci.c @@ -246,7 +246,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf) dev->max_vfs = 0; /* FreeBSD has no NUMA support (yet) */ - dev->device.numa_node = 0; + dev->device.numa_node = SOCKET_ID_ANY; pci_common_set(dev); diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c index c8703d52f3..ade17079fc 100644 --- a/drivers/bus/pci/linux/pci.c +++ b/drivers/bus/pci/linux/pci.c @@ -283,17 +283,13 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr) } /* get numa node, default to 0 if not present */ - snprintf(filename, sizeof(filename), "%s/numa_node", - dirname); + snprintf(filename, sizeof(filename), "%s/numa_node", dirname); - if (access(filename, F_OK) != -1) { - if (eal_parse_sysfs_value(filename, &tmp) == 0) - dev->device.numa_node = tmp; - else - dev->device.numa_node = -1; - } else { - dev->device.numa_node = 0; - } + if (access(filename, F_OK) == 0 && + eal_parse_sysfs_value(filename, &tmp) == 0) + dev->device.numa_node = tmp; + else + dev->device.numa_node = SOCKET_ID_ANY; pci_common_set(dev); diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c index 5ea72bcf23..a59c5b4286 100644 --- a/drivers/bus/pci/pci_common.c +++ b/drivers/bus/pci/pci_common.c @@ -235,12 +235,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr, return 1; } - if (dev->device.numa_node < 0) { - if (rte_socket_count() > 1) - RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware, defaulting socket to 0\n", - dev->name); - dev->device.numa_node = 0; - } + if (dev->device.numa_node < 0 && rte_socket_count() > 1) + RTE_LOG(INFO, EAL, "Device %s is not NUMA-aware\n", dev->name); already_probed = rte_dev_is_probed(&dev->device); if (already_probed && !(dr->drv_flags & RTE_PCI_DRV_PROBE_AGAIN)) { diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c index 3f7a8b9432..5cf05ce1a0 100644 --- a/drivers/bus/pci/windows/pci.c +++ b/drivers/bus/pci/windows/pci.c @@ -249,7 +249,6 @@ get_device_resource_info(HDEVINFO dev_info, DWORD error = GetLastError(); if (error == ERROR_NOT_FOUND) { /* On older CPUs, NUMA is not bound to PCIe locality. */ - dev->device.numa_node = 0; return ERROR_SUCCESS; } RTE_LOG_WIN32_ERR("SetupDiGetDevicePropertyW" diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c index f502783f7a..01d8111b85 100644 --- a/drivers/bus/vmbus/linux/vmbus_bus.c +++ b/drivers/bus/vmbus/linux/vmbus_bus.c @@ -293,7 +293,6 @@ vmbus_scan_one(const char *name) goto error; dev->device.numa_node = tmp; } else { - /* if no NUMA support, set default to 0 */ dev->device.numa_node = SOCKET_ID_ANY; } diff --git a/drivers/bus/vmbus/vmbus_common.c b/drivers/bus/vmbus/vmbus_common.c index 03b39c82b7..8d32d66504 100644 --- a/drivers/bus/vmbus/vmbus_common.c +++ b/drivers/bus/vmbus/vmbus_common.c @@ -111,12 +111,8 @@ vmbus_probe_one_driver(struct rte_vmbus_driver *dr, /* reference driver structure */ dev->driver = dr; - if (dev->device.numa_node < 0) { - if (rte_socket_count() > 1) - VMBUS_LOG(INFO, "Device %s is not NUMA-aware, defaulting socket to 0", - guid); - dev->device.numa_node = 0; - } + if (dev->device.numa_node < 0 && rte_socket_count() > 1) + VMBUS_LOG(INFO, "Device %s is not NUMA-aware", guid); /* call the driver probe() function */ VMBUS_LOG(INFO, " probe driver: %s", dr->driver.name); diff --git a/drivers/dma/idxd/idxd_bus.c b/drivers/dma/idxd/idxd_bus.c index 9b7b16c6e3..bbbfd3f648 100644 --- a/drivers/dma/idxd/idxd_bus.c +++ b/drivers/dma/idxd/idxd_bus.c @@ -12,6 +12,7 @@ #include <dev_driver.h> #include <rte_devargs.h> #include <rte_eal.h> +#include <rte_memory.h> #include <rte_log.h> #include <rte_dmadev_pmd.h> #include <rte_string_fns.h> @@ -322,7 +323,7 @@ dsa_scan(void) while ((wq = readdir(dev_dir)) != NULL) { struct rte_dsa_device *dev; - int numa_node = -1; + int numa_node = SOCKET_ID_ANY; if (strncmp(wq->d_name, "wq", 2) != 0) continue; diff --git a/examples/distributor/main.c b/examples/distributor/main.c index 68f07cc7fb..d41c3bdb14 100644 --- a/examples/distributor/main.c +++ b/examples/distributor/main.c @@ -231,7 +231,7 @@ lcore_rx(struct lcore_params *p) if ((enabled_port_mask & (1 << port)) == 0) continue; - if (rte_eth_dev_socket_id(port) > 0 && + if (rte_eth_dev_socket_id(port) >= 0 && rte_eth_dev_socket_id(port) != socket_id) printf("WARNING, port %u is on remote NUMA node to " "RX thread.\n\tPerformance will not " @@ -406,7 +406,7 @@ lcore_tx(struct rte_ring *in_r) if ((enabled_port_mask & (1 << port)) == 0) continue; - if (rte_eth_dev_socket_id(port) > 0 && + if (rte_eth_dev_socket_id(port) >= 0 && rte_eth_dev_socket_id(port) != socket_id) printf("WARNING, port %u is on remote NUMA node to " "TX thread.\n\tPerformance will not " diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c index 97708b7084..cdd51b2476 100644 --- a/examples/flow_classify/flow_classify.c +++ b/examples/flow_classify/flow_classify.c @@ -818,6 +818,8 @@ main(int argc, char *argv[]) printf("\nWARNING: Too many lcores enabled. Only 1 used.\n"); socket_id = rte_eth_dev_socket_id(0); + if (socket_id == SOCKET_ID_ANY) + socket_id = rte_lcore_to_socket_id(rte_get_next_lcore(-1, 0, 0)); /* Memory allocation. 8< */ size = RTE_CACHE_LINE_ROUNDUP(sizeof(struct flow_classifier_acl)); diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c index edf0ab9b08..59eee49208 100644 --- a/examples/rxtx_callbacks/main.c +++ b/examples/rxtx_callbacks/main.c @@ -249,7 +249,7 @@ lcore_main(void) uint16_t port; RTE_ETH_FOREACH_DEV(port) - if (rte_eth_dev_socket_id(port) > 0 && + if (rte_eth_dev_socket_id(port) >= 0 && rte_eth_dev_socket_id(port) != (int)rte_socket_id()) printf("WARNING, port %u is on remote NUMA node to " diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index a21f58b9cd..dd8d25d6d4 100644 --- a/lib/ethdev/rte_ethdev.h +++ b/lib/ethdev/rte_ethdev.h @@ -2445,8 +2445,8 @@ int rte_eth_hairpin_unbind(uint16_t tx_port, uint16_t rx_port); * The port identifier of the Ethernet device * @return * The NUMA socket ID to which the Ethernet device is connected or - * a default of zero if the socket could not be determined. - * -1 is returned is the port_id value is out of range. + * a default of -1 (SOCKET_ID_ANY) if the socket could not be determined. + * -1 is also returned if the port_id is invalid. */ int rte_eth_dev_socket_id(uint16_t port_id); -- 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drivers/bus: set device NUMA node to unknown by default 2022-10-04 14:58 ` [PATCH v2] " Olivier Matz @ 2022-10-05 8:52 ` David Marchand 2022-10-05 9:04 ` Olivier Matz 2022-10-06 19:31 ` David Marchand 1 sibling, 1 reply; 8+ messages in thread From: David Marchand @ 2022-10-05 8:52 UTC (permalink / raw) To: Olivier Matz Cc: dev, Ray Kinsella, Parav Pandit, Xueming Li, Hemant Agrawal, Sachin Saxena, Stephen Hemminger, Long Li, Ferruh Yigit, Andrew Rybchenko, Thomas Monjalon On Tue, Oct 4, 2022 at 4:59 PM Olivier Matz <olivier.matz@6wind.com> wrote: > > The dev->device.numa_node field is set by each bus driver for > every device it manages to indicate on which NUMA node this device lies. > > When this information is unknown, the assigned value is not consistent > across the bus drivers. > > Set the default value to SOCKET_ID_ANY (-1) by all bus drivers > when the NUMA information is unavailable. This change impacts > rte_eth_dev_socket_id() in the same manner. > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com> > --- > > v2 > * use SOCKET_ID_ANY instead of -1 in drivers/dma/idxd (David) > * document the behavior change of rte_eth_dev_socket_id() > * fix few examples where rte_eth_dev_socket_id() was expected to > return 0 on unknown socket Cc: ethdev maintainers. [snip] > diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst > index 53fe21453c..d52f823694 100644 > --- a/doc/guides/rel_notes/release_22_11.rst > +++ b/doc/guides/rel_notes/release_22_11.rst > @@ -317,6 +317,12 @@ ABI Changes > * eventdev: Added ``weight`` and ``affinity`` fields > to ``rte_event_queue_conf`` structure. > > +* bus: Changed the device numa node to -1 when NUMA information is unavailable. > + The ``dev->device.numa_node`` field is set by each bus driver for > + every device it manages to indicate on which NUMA node this device lies. > + When this information is unknown, the assigned value was not consistent > + across the bus drivers. This similarly impacts ``rte_eth_dev_socket_id()``. > + > > Known Issues > ------------ [snip] > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > index a21f58b9cd..dd8d25d6d4 100644 > --- a/lib/ethdev/rte_ethdev.h > +++ b/lib/ethdev/rte_ethdev.h > @@ -2445,8 +2445,8 @@ int rte_eth_hairpin_unbind(uint16_t tx_port, uint16_t rx_port); > * The port identifier of the Ethernet device > * @return > * The NUMA socket ID to which the Ethernet device is connected or > - * a default of zero if the socket could not be determined. > - * -1 is returned is the port_id value is out of range. > + * a default of -1 (SOCKET_ID_ANY) if the socket could not be determined. > + * -1 is also returned if the port_id is invalid. > */ > int rte_eth_dev_socket_id(uint16_t port_id); It would be better to distinguish the two cases, using rte_errno. Something like: diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index 2821770e2d..1baf302804 100644 --- a/lib/ethdev/rte_ethdev.c +++ b/lib/ethdev/rte_ethdev.c @@ -562,8 +562,16 @@ rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner) int rte_eth_dev_socket_id(uint16_t port_id) { - RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1); - return rte_eth_devices[port_id].data->numa_node; + int socket_id = SOCKET_ID_ANY; + + if (!rte_eth_dev_is_valid_port(port_id)) + rte_errno = EINVAL; + } else { + socket_id = rte_eth_devices[port_id].data->numa_node; + if (socket_id == SOCKET_ID_ANY) + rte_errno = 0; + } + return socket_id; } void * diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index dd8d25d6d4..03456b2dbb 100644 --- a/lib/ethdev/rte_ethdev.h +++ b/lib/ethdev/rte_ethdev.h @@ -2444,9 +2444,11 @@ int rte_eth_hairpin_unbind(uint16_t tx_port, uint16_t rx_port); * @param port_id * The port identifier of the Ethernet device * @return - * The NUMA socket ID to which the Ethernet device is connected or - * a default of -1 (SOCKET_ID_ANY) if the socket could not be determined. - * -1 is also returned if the port_id is invalid. + * - The NUMA socket ID which the Ethernet device is connected to. + * - -1 (which translates to SOCKET_ID_ANY) if the socket could not be + * determined. rte_errno is then set to: + * - EINVAL is the port_id is invalid, + * - 0 is the socket could not be determined, */ int rte_eth_dev_socket_id(uint16_t port_id); WDYT? -- David Marchand ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drivers/bus: set device NUMA node to unknown by default 2022-10-05 8:52 ` David Marchand @ 2022-10-05 9:04 ` Olivier Matz 2022-10-05 9:32 ` Thomas Monjalon 0 siblings, 1 reply; 8+ messages in thread From: Olivier Matz @ 2022-10-05 9:04 UTC (permalink / raw) To: David Marchand Cc: dev, Ray Kinsella, Parav Pandit, Xueming Li, Hemant Agrawal, Sachin Saxena, Stephen Hemminger, Long Li, Ferruh Yigit, Andrew Rybchenko, Thomas Monjalon Hi David, On Wed, Oct 05, 2022 at 10:52:49AM +0200, David Marchand wrote: > On Tue, Oct 4, 2022 at 4:59 PM Olivier Matz <olivier.matz@6wind.com> wrote: > > > > The dev->device.numa_node field is set by each bus driver for > > every device it manages to indicate on which NUMA node this device lies. > > > > When this information is unknown, the assigned value is not consistent > > across the bus drivers. > > > > Set the default value to SOCKET_ID_ANY (-1) by all bus drivers > > when the NUMA information is unavailable. This change impacts > > rte_eth_dev_socket_id() in the same manner. > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com> > > --- > > > > v2 > > * use SOCKET_ID_ANY instead of -1 in drivers/dma/idxd (David) > > * document the behavior change of rte_eth_dev_socket_id() > > * fix few examples where rte_eth_dev_socket_id() was expected to > > return 0 on unknown socket > > Cc: ethdev maintainers. > > [snip] > > > diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst > > index 53fe21453c..d52f823694 100644 > > --- a/doc/guides/rel_notes/release_22_11.rst > > +++ b/doc/guides/rel_notes/release_22_11.rst > > @@ -317,6 +317,12 @@ ABI Changes > > * eventdev: Added ``weight`` and ``affinity`` fields > > to ``rte_event_queue_conf`` structure. > > > > +* bus: Changed the device numa node to -1 when NUMA information is unavailable. > > + The ``dev->device.numa_node`` field is set by each bus driver for > > + every device it manages to indicate on which NUMA node this device lies. > > + When this information is unknown, the assigned value was not consistent > > + across the bus drivers. This similarly impacts ``rte_eth_dev_socket_id()``. > > + > > > > Known Issues > > ------------ > > [snip] > > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > > index a21f58b9cd..dd8d25d6d4 100644 > > --- a/lib/ethdev/rte_ethdev.h > > +++ b/lib/ethdev/rte_ethdev.h > > @@ -2445,8 +2445,8 @@ int rte_eth_hairpin_unbind(uint16_t tx_port, uint16_t rx_port); > > * The port identifier of the Ethernet device > > * @return > > * The NUMA socket ID to which the Ethernet device is connected or > > - * a default of zero if the socket could not be determined. > > - * -1 is returned is the port_id value is out of range. > > + * a default of -1 (SOCKET_ID_ANY) if the socket could not be determined. > > + * -1 is also returned if the port_id is invalid. > > */ > > int rte_eth_dev_socket_id(uint16_t port_id); > > It would be better to distinguish the two cases, using rte_errno. > Something like: > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > index 2821770e2d..1baf302804 100644 > --- a/lib/ethdev/rte_ethdev.c > +++ b/lib/ethdev/rte_ethdev.c > @@ -562,8 +562,16 @@ rte_eth_dev_owner_get(const uint16_t port_id, > struct rte_eth_dev_owner *owner) > int > rte_eth_dev_socket_id(uint16_t port_id) > { > - RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1); > - return rte_eth_devices[port_id].data->numa_node; > + int socket_id = SOCKET_ID_ANY; > + > + if (!rte_eth_dev_is_valid_port(port_id)) > + rte_errno = EINVAL; > + } else { > + socket_id = rte_eth_devices[port_id].data->numa_node; > + if (socket_id == SOCKET_ID_ANY) > + rte_errno = 0; > + } > + return socket_id; > } > > void * > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > index dd8d25d6d4..03456b2dbb 100644 > --- a/lib/ethdev/rte_ethdev.h > +++ b/lib/ethdev/rte_ethdev.h > @@ -2444,9 +2444,11 @@ int rte_eth_hairpin_unbind(uint16_t tx_port, > uint16_t rx_port); > * @param port_id > * The port identifier of the Ethernet device > * @return > - * The NUMA socket ID to which the Ethernet device is connected or > - * a default of -1 (SOCKET_ID_ANY) if the socket could not be determined. > - * -1 is also returned if the port_id is invalid. > + * - The NUMA socket ID which the Ethernet device is connected to. > + * - -1 (which translates to SOCKET_ID_ANY) if the socket could not be > + * determined. rte_errno is then set to: > + * - EINVAL is the port_id is invalid, > + * - 0 is the socket could not be determined, > */ > int rte_eth_dev_socket_id(uint16_t port_id); > > WDYT? As discussed off-list, it is indeed better. Thanks for the review Olivier ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drivers/bus: set device NUMA node to unknown by default 2022-10-05 9:04 ` Olivier Matz @ 2022-10-05 9:32 ` Thomas Monjalon 0 siblings, 0 replies; 8+ messages in thread From: Thomas Monjalon @ 2022-10-05 9:32 UTC (permalink / raw) To: David Marchand, Olivier Matz Cc: dev, Ray Kinsella, Parav Pandit, Xueming Li, Hemant Agrawal, Sachin Saxena, Stephen Hemminger, Long Li, Ferruh Yigit, Andrew Rybchenko 05/10/2022 11:04, Olivier Matz: > Hi David, > > On Wed, Oct 05, 2022 at 10:52:49AM +0200, David Marchand wrote: > > On Tue, Oct 4, 2022 at 4:59 PM Olivier Matz <olivier.matz@6wind.com> wrote: > > > > > > The dev->device.numa_node field is set by each bus driver for > > > every device it manages to indicate on which NUMA node this device lies. > > > > > > When this information is unknown, the assigned value is not consistent > > > across the bus drivers. > > > > > > Set the default value to SOCKET_ID_ANY (-1) by all bus drivers > > > when the NUMA information is unavailable. This change impacts > > > rte_eth_dev_socket_id() in the same manner. > > > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com> > > > --- > > > > > > v2 > > > * use SOCKET_ID_ANY instead of -1 in drivers/dma/idxd (David) > > > * document the behavior change of rte_eth_dev_socket_id() > > > * fix few examples where rte_eth_dev_socket_id() was expected to > > > return 0 on unknown socket > > > > Cc: ethdev maintainers. > > > > [snip] > > > > > diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst > > > index 53fe21453c..d52f823694 100644 > > > --- a/doc/guides/rel_notes/release_22_11.rst > > > +++ b/doc/guides/rel_notes/release_22_11.rst > > > @@ -317,6 +317,12 @@ ABI Changes > > > * eventdev: Added ``weight`` and ``affinity`` fields > > > to ``rte_event_queue_conf`` structure. > > > > > > +* bus: Changed the device numa node to -1 when NUMA information is unavailable. > > > + The ``dev->device.numa_node`` field is set by each bus driver for > > > + every device it manages to indicate on which NUMA node this device lies. > > > + When this information is unknown, the assigned value was not consistent > > > + across the bus drivers. This similarly impacts ``rte_eth_dev_socket_id()``. > > > + > > > > > > Known Issues > > > ------------ > > > > [snip] > > > > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > > > index a21f58b9cd..dd8d25d6d4 100644 > > > --- a/lib/ethdev/rte_ethdev.h > > > +++ b/lib/ethdev/rte_ethdev.h > > > @@ -2445,8 +2445,8 @@ int rte_eth_hairpin_unbind(uint16_t tx_port, uint16_t rx_port); > > > * The port identifier of the Ethernet device > > > * @return > > > * The NUMA socket ID to which the Ethernet device is connected or > > > - * a default of zero if the socket could not be determined. > > > - * -1 is returned is the port_id value is out of range. > > > + * a default of -1 (SOCKET_ID_ANY) if the socket could not be determined. > > > + * -1 is also returned if the port_id is invalid. > > > */ > > > int rte_eth_dev_socket_id(uint16_t port_id); > > > > It would be better to distinguish the two cases, using rte_errno. > > Something like: > > > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > > index 2821770e2d..1baf302804 100644 > > --- a/lib/ethdev/rte_ethdev.c > > +++ b/lib/ethdev/rte_ethdev.c > > @@ -562,8 +562,16 @@ rte_eth_dev_owner_get(const uint16_t port_id, > > struct rte_eth_dev_owner *owner) > > int > > rte_eth_dev_socket_id(uint16_t port_id) > > { > > - RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1); > > - return rte_eth_devices[port_id].data->numa_node; > > + int socket_id = SOCKET_ID_ANY; > > + > > + if (!rte_eth_dev_is_valid_port(port_id)) > > + rte_errno = EINVAL; > > + } else { > > + socket_id = rte_eth_devices[port_id].data->numa_node; > > + if (socket_id == SOCKET_ID_ANY) > > + rte_errno = 0; > > + } > > + return socket_id; > > } > > > > void * > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > > index dd8d25d6d4..03456b2dbb 100644 > > --- a/lib/ethdev/rte_ethdev.h > > +++ b/lib/ethdev/rte_ethdev.h > > @@ -2444,9 +2444,11 @@ int rte_eth_hairpin_unbind(uint16_t tx_port, > > uint16_t rx_port); > > * @param port_id > > * The port identifier of the Ethernet device > > * @return > > - * The NUMA socket ID to which the Ethernet device is connected or > > - * a default of -1 (SOCKET_ID_ANY) if the socket could not be determined. > > - * -1 is also returned if the port_id is invalid. > > + * - The NUMA socket ID which the Ethernet device is connected to. > > + * - -1 (which translates to SOCKET_ID_ANY) if the socket could not be > > + * determined. rte_errno is then set to: > > + * - EINVAL is the port_id is invalid, > > + * - 0 is the socket could not be determined, > > */ > > int rte_eth_dev_socket_id(uint16_t port_id); > > > > WDYT? > > As discussed off-list, it is indeed better. +1 Note that the decision to take in case of EINVAL requires some work for each call to rte_eth_dev_socket_id(). I suppose we can leave it as a future exercise. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drivers/bus: set device NUMA node to unknown by default 2022-10-04 14:58 ` [PATCH v2] " Olivier Matz 2022-10-05 8:52 ` David Marchand @ 2022-10-06 19:31 ` David Marchand 1 sibling, 0 replies; 8+ messages in thread From: David Marchand @ 2022-10-06 19:31 UTC (permalink / raw) To: Olivier Matz Cc: dev, Ray Kinsella, Parav Pandit, Xueming Li, Hemant Agrawal, Sachin Saxena, Stephen Hemminger, Long Li, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko On Tue, Oct 4, 2022 at 4:59 PM Olivier Matz <olivier.matz@6wind.com> wrote: > > The dev->device.numa_node field is set by each bus driver for > every device it manages to indicate on which NUMA node this device lies. > > When this information is unknown, the assigned value is not consistent > across the bus drivers. > > Set the default value to SOCKET_ID_ANY (-1) by all bus drivers > when the NUMA information is unavailable. This change impacts > rte_eth_dev_socket_id() in the same manner. > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com> > --- > > v2 > * use SOCKET_ID_ANY instead of -1 in drivers/dma/idxd (David) > * document the behavior change of rte_eth_dev_socket_id() > * fix few examples where rte_eth_dev_socket_id() was expected to > return 0 on unknown socket Applied with suggestion on setting rte_errno for rte_eth_dev_socket_id(). Thanks Olivier! -- David Marchand ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-06 19:32 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-29 12:05 [PATCH] drivers/bus: set device NUMA node to unknown by default Olivier Matz 2022-09-30 7:10 ` David Marchand 2022-09-30 8:11 ` Olivier Matz 2022-10-04 14:58 ` [PATCH v2] " Olivier Matz 2022-10-05 8:52 ` David Marchand 2022-10-05 9:04 ` Olivier Matz 2022-10-05 9:32 ` Thomas Monjalon 2022-10-06 19:31 ` David Marchand
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).