DPDK patches and discussions
 help / color / mirror / Atom feed
* [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
@ 2024-09-06 11:47 Anatoly Burakov
  2024-09-06 11:47 ` [RFC PATCH v1 1/5] eal: update socket ID API documentation Anatoly Burakov
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Anatoly Burakov @ 2024-09-06 11:47 UTC (permalink / raw)
  To: dev

While initially, DPDK has used the term "socket ID" to refer to physical package
ID, the last time DPDK read "physical_package_id" for socket ID was ~9 years
ago, so it's been a while since we've actually switched over to using the term
"socket" to mean "NUMA node".

This wasn't a problem before, as most systems had one NUMA node per physical
socket. However, in the last few years, more and more systems have multiple NUMA
nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
transition was pretty seamless, however now we're faced with a situation when
most of our documentation still uses outdated terms, and our API is ripe with
references to "sockets" when in actuality we mean "NUMA nodes". This could be a
source of confusion.

While completely renaming all of our API's would be a huge effort, will take a
long time and arguably wouldn't even be worth the API breakages (given that this
mismatch between terminology and reality is implicitly understood by most people
working on DPDK, and so this isn't so much of a problem in practice), we can do
some tweaks around the edges and at least document this unfortunate reality.

This patchset suggests the following changes:

- Update rte_socket/rte_lcore documentation to refer to NUMA nodes rather than
sockets - Rename internal structures' fields to better reflect this intention -
Rename --socket-mem/--socket-limit flags to refer to NUMA rather than sockets -
Add internal API to get physical package ID [1]

The documentation is updated to refer to new EAL flags, but is otherwise left
untouched, and instead the entry in "glossary" is amended to indicate that when
DPDK documentation refers to "sockets", it actually means "NUMA ID's". As next
steps, we could rename all API parameters to refer to NUMA ID rather than socket
ID - this would not break neither API nor ABI, and instead would be a
documentation change in practice.

[1] This could be used to group lcores by physical package, see e.g. discussion
    under this patch: https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-vipin.varghese@amd.com/

Anatoly Burakov (5):
  eal: update socket ID API documentation
  lcore: rename socket ID to NUMA ID
  eal: rename socket ID to NUMA ID in internal config
  eal: rename --socket-mem/--socket-limit
  lcore: store physical package ID internally

 doc/guides/faq/faq.rst                        |  4 +--
 doc/guides/howto/lm_bond_virtio_sriov.rst     |  2 +-
 doc/guides/howto/lm_virtio_vhost_user.rst     |  2 +-
 doc/guides/howto/pvp_reference_benchmark.rst  |  4 +--
 .../virtio_user_for_container_networking.rst  |  2 +-
 doc/guides/linux_gsg/build_sample_apps.rst    | 20 +++++------
 doc/guides/linux_gsg/linux_eal_parameters.rst | 16 ++++-----
 doc/guides/nics/mlx4.rst                      |  2 +-
 doc/guides/nics/mlx5.rst                      |  2 +-
 .../prog_guide/env_abstraction_layer.rst      | 12 +++----
 doc/guides/prog_guide/glossary.rst            |  5 ++-
 doc/guides/prog_guide/multi_proc_support.rst  |  2 +-
 doc/guides/sample_app_ug/bbdev_app.rst        |  6 ++--
 doc/guides/sample_app_ug/ipsec_secgw.rst      |  6 ++--
 doc/guides/sample_app_ug/vdpa.rst             |  2 +-
 doc/guides/sample_app_ug/vhost.rst            |  4 +--
 lib/eal/common/eal_common_dynmem.c            | 14 ++++----
 lib/eal/common/eal_common_lcore.c             | 28 +++++++++++++---
 lib/eal/common/eal_common_options.c           | 33 ++++++++++---------
 lib/eal/common/eal_common_thread.c            | 12 +++----
 lib/eal/common/eal_internal_cfg.h             | 10 +++---
 lib/eal/common/eal_options.h                  |  8 +++--
 lib/eal/common/eal_private.h                  |  5 ++-
 lib/eal/common/eal_thread.h                   | 11 +++++++
 lib/eal/common/malloc_heap.c                  |  2 +-
 lib/eal/freebsd/eal.c                         |  2 +-
 lib/eal/freebsd/eal_lcore.c                   |  6 ++++
 lib/eal/include/rte_lcore.h                   | 25 +++++++-------
 lib/eal/linux/eal.c                           | 22 ++++++-------
 lib/eal/linux/eal_lcore.c                     | 28 ++++++++++++++++
 lib/eal/linux/eal_memory.c                    | 22 ++++++-------
 lib/eal/windows/eal.c                         |  2 +-
 lib/eal/windows/eal_lcore.c                   |  7 ++++
 33 files changed, 204 insertions(+), 124 deletions(-)

-- 
2.43.5


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

* [RFC PATCH v1 1/5] eal: update socket ID API documentation
  2024-09-06 11:47 [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Anatoly Burakov
@ 2024-09-06 11:47 ` Anatoly Burakov
  2024-09-06 11:47 ` [RFC PATCH v1 2/5] lcore: rename socket ID to NUMA ID Anatoly Burakov
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Anatoly Burakov @ 2024-09-06 11:47 UTC (permalink / raw)
  To: dev, Tyler Retzlaff

Currently, even though through out DPDK we refer to "socket ID's", in
actuality we are referring to NUMA node ID's, which do not necessarily
correspond to physical sockets.

This is not an API change nor a semantics change, it is merely an update
of API documentation to match what is already the case (the semantics
have changed back when systems started reporting multiple NUMA nodes per
physical socket).

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 doc/guides/prog_guide/glossary.rst |  5 ++++-
 lib/eal/include/rte_lcore.h        | 25 ++++++++++++-------------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/doc/guides/prog_guide/glossary.rst b/doc/guides/prog_guide/glossary.rst
index 8d6349701e..d09d7bf5f6 100644
--- a/doc/guides/prog_guide/glossary.rst
+++ b/doc/guides/prog_guide/glossary.rst
@@ -191,7 +191,10 @@ Slave lcore
    Deprecated name for *worker lcore*. No longer used.
 
 Socket
-   A physical CPU, that includes several *cores*.
+   For historical reasons, the term "socket" is used in the DPDK to refer to
+   both physical sockets, as well as NUMA nodes. As a general rule, the term
+   should be understood to mean "NUMA node" unless it is clear from context
+   that it is referring to physical CPU sockets.
 
 SLA
    Service Level Agreement
diff --git a/lib/eal/include/rte_lcore.h b/lib/eal/include/rte_lcore.h
index 7deae47af3..de9e940b76 100644
--- a/lib/eal/include/rte_lcore.h
+++ b/lib/eal/include/rte_lcore.h
@@ -113,22 +113,21 @@ unsigned int rte_lcore_count(void);
 int rte_lcore_index(int lcore_id);
 
 /**
- * Return the ID of the physical socket of the logical core we are
- * running on.
+ * Return the ID of NUMA node of the logical core we are running on.
  * @return
- *   the ID of current lcoreid's physical socket
+ *   the ID of current lcoreid's NUMA node
  */
 unsigned int rte_socket_id(void);
 
 /**
- * Return number of physical sockets detected on the system.
+ * Return number of NUMA nodes detected on the system.
  *
- * Note that number of nodes may not be correspondent to their physical id's:
- * for example, a system may report two socket id's, but the actual socket id's
+ * Note that number of nodes may not be correspondent to their NUMA ID's:
+ * for example, a system may report two NUMA ID's, but the actual NUMA ID's
  * may be 0 and 8.
  *
  * @return
- *   the number of physical sockets as recognized by EAL
+ *   the number of NUMA ID's as recognized by EAL
  */
 unsigned int
 rte_socket_count(void);
@@ -137,26 +136,26 @@ rte_socket_count(void);
  * Return socket id with a particular index.
  *
  * This will return socket id at a particular position in list of all detected
- * physical socket id's. For example, on a machine with sockets [0, 8], passing
- * 1 as a parameter will return 8.
+ * NUMA node ID's. For example, on a machine with NUMA nodes [0, 8], passing 1
+ * as a parameter will return 8.
  *
  * @param idx
- *   index of physical socket id to return
+ *   index of NUMA node ID to return
  *
  * @return
- *   - physical socket id as recognized by EAL
+ *   - NUMA node ID as recognized by EAL
  *   - -1 on error, with errno set to EINVAL
  */
 int
 rte_socket_id_by_idx(unsigned int idx);
 
 /**
- * Get the ID of the physical socket of the specified lcore
+ * Get the ID of the NUMA node of the specified lcore
  *
  * @param lcore_id
  *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
  * @return
- *   the ID of lcoreid's physical socket
+ *   the ID of lcoreid's NUMA node
  */
 unsigned int
 rte_lcore_to_socket_id(unsigned int lcore_id);
-- 
2.43.5


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

* [RFC PATCH v1 2/5] lcore: rename socket ID to NUMA ID
  2024-09-06 11:47 [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Anatoly Burakov
  2024-09-06 11:47 ` [RFC PATCH v1 1/5] eal: update socket ID API documentation Anatoly Burakov
@ 2024-09-06 11:47 ` Anatoly Burakov
  2024-09-06 11:47 ` [RFC PATCH v1 3/5] eal: rename socket ID to NUMA ID in internal config Anatoly Burakov
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Anatoly Burakov @ 2024-09-06 11:47 UTC (permalink / raw)
  To: dev, Tyler Retzlaff

Rename socket ID to NUMA ID in internal lcore structure. This does not
change any user facing API's, although it does alter a couple of log
messages.

In particular, telemetry API and lcore dump API changes have been omitted
as there may be consumers of these API that depend on specifics of messages
generated by these API's.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/eal_common_lcore.c  | 10 +++++-----
 lib/eal/common/eal_common_thread.c | 12 ++++++------
 lib/eal/common/eal_private.h       |  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_common_lcore.c
index 2ff9252c52..ba8fce6607 100644
--- a/lib/eal/common/eal_common_lcore.c
+++ b/lib/eal/common/eal_common_lcore.c
@@ -115,7 +115,7 @@ unsigned int rte_get_next_lcore(unsigned int i, int skip_main, int wrap)
 unsigned int
 rte_lcore_to_socket_id(unsigned int lcore_id)
 {
-	return lcore_config[lcore_id].socket_id;
+	return lcore_config[lcore_id].numa_id;
 }
 
 static int
@@ -173,17 +173,17 @@ rte_eal_cpu_init(void)
 		config->lcore_role[lcore_id] = ROLE_RTE;
 		lcore_config[lcore_id].core_role = ROLE_RTE;
 		lcore_config[lcore_id].core_id = eal_cpu_core_id(lcore_id);
-		lcore_config[lcore_id].socket_id = socket_id;
+		lcore_config[lcore_id].numa_id = socket_id;
 		EAL_LOG(DEBUG, "Detected lcore %u as "
-				"core %u on socket %u",
+				"core %u on NUMA node %u",
 				lcore_id, lcore_config[lcore_id].core_id,
-				lcore_config[lcore_id].socket_id);
+				lcore_config[lcore_id].numa_id);
 		count++;
 	}
 	for (; lcore_id < CPU_SETSIZE; lcore_id++) {
 		if (eal_cpu_detected(lcore_id) == 0)
 			continue;
-		EAL_LOG(DEBUG, "Skipped lcore %u as core %u on socket %u",
+		EAL_LOG(DEBUG, "Skipped lcore %u as core %u on NUMA node %u",
 			lcore_id, eal_cpu_core_id(lcore_id),
 			eal_cpu_socket_id(lcore_id));
 	}
diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
index a53bc639ae..aa98bdc3ff 100644
--- a/lib/eal/common/eal_common_thread.c
+++ b/lib/eal/common/eal_common_thread.c
@@ -24,13 +24,13 @@
 
 RTE_DEFINE_PER_LCORE(unsigned int, _lcore_id) = LCORE_ID_ANY;
 RTE_DEFINE_PER_LCORE(int, _thread_id) = -1;
-static RTE_DEFINE_PER_LCORE(unsigned int, _socket_id) =
+static RTE_DEFINE_PER_LCORE(unsigned int, _numa_id) =
 	(unsigned int)SOCKET_ID_ANY;
 static RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
 
 unsigned rte_socket_id(void)
 {
-	return RTE_PER_LCORE(_socket_id);
+	return RTE_PER_LCORE(_numa_id);
 }
 
 static int
@@ -66,8 +66,8 @@ thread_update_affinity(rte_cpuset_t *cpusetp)
 {
 	unsigned int lcore_id = rte_lcore_id();
 
-	/* store socket_id in TLS for quick access */
-	RTE_PER_LCORE(_socket_id) =
+	/* store numa_id in TLS for quick access */
+	RTE_PER_LCORE(_numa_id) =
 		eal_cpuset_socket_id(cpusetp);
 
 	/* store cpuset in TLS for quick access */
@@ -76,7 +76,7 @@ thread_update_affinity(rte_cpuset_t *cpusetp)
 
 	if (lcore_id != (unsigned)LCORE_ID_ANY) {
 		/* EAL thread will update lcore_config */
-		lcore_config[lcore_id].socket_id = RTE_PER_LCORE(_socket_id);
+		lcore_config[lcore_id].numa_id = RTE_PER_LCORE(_numa_id);
 		memmove(&lcore_config[lcore_id].cpuset, cpusetp,
 			sizeof(rte_cpuset_t));
 	}
@@ -256,7 +256,7 @@ static int control_thread_init(void *arg)
 	/* Set control thread socket ID to SOCKET_ID_ANY
 	 * as control threads may be scheduled on any NUMA node.
 	 */
-	RTE_PER_LCORE(_socket_id) = SOCKET_ID_ANY;
+	RTE_PER_LCORE(_numa_id) = SOCKET_ID_ANY;
 	params->ret = rte_thread_set_affinity_by_id(rte_thread_self(), cpuset);
 	if (params->ret != 0) {
 		rte_atomic_store_explicit(&params->status,
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index af09620426..196dadc8a2 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -30,7 +30,7 @@ struct lcore_config {
 	volatile int ret;          /**< return value of function */
 
 	volatile RTE_ATOMIC(enum rte_lcore_state_t) state; /**< lcore state */
-	unsigned int socket_id;    /**< physical socket id for this lcore */
+	unsigned int numa_id;    /**< NUMA node ID for this lcore */
 	unsigned int core_id;      /**< core number on socket for this lcore */
 	int core_index;            /**< relative index, starting from 0 */
 	uint8_t core_role;         /**< role of core eg: OFF, RTE, SERVICE */
-- 
2.43.5


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

* [RFC PATCH v1 3/5] eal: rename socket ID to NUMA ID in internal config
  2024-09-06 11:47 [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Anatoly Burakov
  2024-09-06 11:47 ` [RFC PATCH v1 1/5] eal: update socket ID API documentation Anatoly Burakov
  2024-09-06 11:47 ` [RFC PATCH v1 2/5] lcore: rename socket ID to NUMA ID Anatoly Burakov
@ 2024-09-06 11:47 ` Anatoly Burakov
  2024-09-06 11:47 ` [RFC PATCH v1 4/5] eal: rename --socket-mem/--socket-limit Anatoly Burakov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Anatoly Burakov @ 2024-09-06 11:47 UTC (permalink / raw)
  To: dev, Tyler Retzlaff, Bruce Richardson, Dmitry Kozlyuk, Pallavi Kadam

This patch renames socket ID-related fields in internal EAL config
structure to refer to NUMA ID instead. No user-facing API's are changed.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/eal_common_dynmem.c  | 14 +++++++-------
 lib/eal/common/eal_common_options.c | 16 ++++++++--------
 lib/eal/common/eal_internal_cfg.h   | 10 +++++-----
 lib/eal/common/malloc_heap.c        |  2 +-
 lib/eal/freebsd/eal.c               |  2 +-
 lib/eal/linux/eal.c                 | 10 +++++-----
 lib/eal/linux/eal_memory.c          | 22 +++++++++++-----------
 lib/eal/windows/eal.c               |  2 +-
 8 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/lib/eal/common/eal_common_dynmem.c b/lib/eal/common/eal_common_dynmem.c
index b4dc231940..4377af5aab 100644
--- a/lib/eal/common/eal_common_dynmem.c
+++ b/lib/eal/common/eal_common_dynmem.c
@@ -264,9 +264,9 @@ eal_dynmem_hugepage_init(void)
 #endif
 	}
 
-	/* make a copy of socket_mem, needed for balanced allocation. */
+	/* make a copy of numa_mem, needed for balanced allocation. */
 	for (hp_sz_idx = 0; hp_sz_idx < RTE_MAX_NUMA_NODES; hp_sz_idx++)
-		memory[hp_sz_idx] = internal_conf->socket_mem[hp_sz_idx];
+		memory[hp_sz_idx] = internal_conf->numa_mem[hp_sz_idx];
 
 	/* calculate final number of pages */
 	if (eal_dynmem_calc_num_pages_per_socket(memory,
@@ -334,10 +334,10 @@ eal_dynmem_hugepage_init(void)
 	}
 
 	/* if socket limits were specified, set them */
-	if (internal_conf->force_socket_limits) {
+	if (internal_conf->force_numa_limits) {
 		unsigned int i;
 		for (i = 0; i < RTE_MAX_NUMA_NODES; i++) {
-			uint64_t limit = internal_conf->socket_limit[i];
+			uint64_t limit = internal_conf->numa_limit[i];
 			if (limit == 0)
 				continue;
 			if (rte_mem_alloc_validator_register("socket-limit",
@@ -382,7 +382,7 @@ eal_dynmem_calc_num_pages_per_socket(
 		return -1;
 
 	/* if specific memory amounts per socket weren't requested */
-	if (internal_conf->force_sockets == 0) {
+	if (internal_conf->force_numa == 0) {
 		size_t total_size;
 #ifdef RTE_ARCH_64
 		int cpu_per_socket[RTE_MAX_NUMA_NODES];
@@ -509,10 +509,10 @@ eal_dynmem_calc_num_pages_per_socket(
 
 		/* if we didn't satisfy all memory requirements per socket */
 		if (memory[socket] > 0 &&
-				internal_conf->socket_mem[socket] != 0) {
+				internal_conf->numa_mem[socket] != 0) {
 			/* to prevent icc errors */
 			requested = (unsigned int)(
-				internal_conf->socket_mem[socket] / 0x100000);
+				internal_conf->numa_mem[socket] / 0x100000);
 			available = requested -
 				((unsigned int)(memory[socket] / 0x100000));
 			EAL_LOG(ERR, "Not enough memory available on "
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index f1a5e329a5..73fbb8587b 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -333,14 +333,14 @@ eal_reset_internal_config(struct internal_config *internal_cfg)
 	internal_cfg->hugepage_dir = NULL;
 	internal_cfg->hugepage_file.unlink_before_mapping = false;
 	internal_cfg->hugepage_file.unlink_existing = true;
-	internal_cfg->force_sockets = 0;
+	internal_cfg->force_numa = 0;
 	/* zero out the NUMA config */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		internal_cfg->socket_mem[i] = 0;
-	internal_cfg->force_socket_limits = 0;
+		internal_cfg->numa_mem[i] = 0;
+	internal_cfg->force_numa_limits = 0;
 	/* zero out the NUMA limits config */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		internal_cfg->socket_limit[i] = 0;
+		internal_cfg->numa_limit[i] = 0;
 	/* zero out hugedir descriptors */
 	for (i = 0; i < MAX_HUGEPAGE_SIZES; i++) {
 		memset(&internal_cfg->hugepage_info[i], 0,
@@ -2041,7 +2041,7 @@ eal_adjust_config(struct internal_config *internal_cfg)
 	/* if no memory amounts were requested, this will result in 0 and
 	 * will be overridden later, right after eal_hugepage_info_init() */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		internal_cfg->memory += internal_cfg->socket_mem[i];
+		internal_cfg->memory += internal_cfg->numa_mem[i];
 
 	return 0;
 }
@@ -2082,12 +2082,12 @@ eal_check_common_options(struct internal_config *internal_cfg)
 			"option");
 		return -1;
 	}
-	if (mem_parsed && internal_cfg->force_sockets == 1) {
+	if (mem_parsed && internal_cfg->force_numa == 1) {
 		EAL_LOG(ERR, "Options -m and --"OPT_SOCKET_MEM" cannot "
 			"be specified at the same time");
 		return -1;
 	}
-	if (internal_cfg->no_hugetlbfs && internal_cfg->force_sockets == 1) {
+	if (internal_cfg->no_hugetlbfs && internal_cfg->force_numa == 1) {
 		EAL_LOG(ERR, "Option --"OPT_SOCKET_MEM" cannot "
 			"be specified together with --"OPT_NO_HUGE);
 		return -1;
@@ -2105,7 +2105,7 @@ eal_check_common_options(struct internal_config *internal_cfg)
 			"be specified together with --"OPT_NO_HUGE);
 		return -1;
 	}
-	if (internal_conf->force_socket_limits && internal_conf->legacy_mem) {
+	if (internal_conf->force_numa_limits && internal_conf->legacy_mem) {
 		EAL_LOG(ERR, "Option --"OPT_SOCKET_LIMIT
 			" is only supported in non-legacy memory mode");
 	}
diff --git a/lib/eal/common/eal_internal_cfg.h b/lib/eal/common/eal_internal_cfg.h
index 167ec501fa..c34a2c3fe7 100644
--- a/lib/eal/common/eal_internal_cfg.h
+++ b/lib/eal/common/eal_internal_cfg.h
@@ -68,11 +68,11 @@ struct internal_config {
 	 */
 	volatile unsigned create_uio_dev; /**< true to create /dev/uioX devices */
 	volatile enum rte_proc_type_t process_type; /**< multi-process proc type */
-	/** true to try allocating memory on specific sockets */
-	volatile unsigned force_sockets;
-	volatile uint64_t socket_mem[RTE_MAX_NUMA_NODES]; /**< amount of memory per socket */
-	volatile unsigned force_socket_limits;
-	volatile uint64_t socket_limit[RTE_MAX_NUMA_NODES]; /**< limit amount of memory per socket */
+	/** true to try allocating memory on specific NUMA nodes */
+	volatile unsigned force_numa;
+	volatile uint64_t numa_mem[RTE_MAX_NUMA_NODES]; /**< amount of memory per socket */
+	volatile unsigned force_numa_limits;
+	volatile uint64_t numa_limit[RTE_MAX_NUMA_NODES]; /**< limit amount of memory per socket */
 	uintptr_t base_virtaddr;          /**< base address to try and reserve memory from */
 	volatile unsigned legacy_mem;
 	/**< true to enable legacy memory behavior (no dynamic allocation,
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 058aaf4209..f87a947a42 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -711,7 +711,7 @@ malloc_get_numa_socket(void)
 	/* for control threads, return first socket where memory is available */
 	for (idx = 0; idx < rte_socket_count(); idx++) {
 		socket_id = rte_socket_id_by_idx(idx);
-		if (conf->socket_mem[socket_id] != 0)
+		if (conf->numa_mem[socket_id] != 0)
 			return socket_id;
 	}
 	/* We couldn't quickly find a NUMA node where memory was available,
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 1229230063..8c7981645b 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -749,7 +749,7 @@ rte_eal_init(int argc, char **argv)
 		}
 	}
 
-	if (internal_conf->memory == 0 && internal_conf->force_sockets == 0) {
+	if (internal_conf->memory == 0 && internal_conf->force_numa == 0) {
 		if (internal_conf->no_hugetlbfs)
 			internal_conf->memory = MEMSIZE_IF_NO_HUGE_PAGE;
 		else
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index d742cc98e2..cf174aeaa3 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -695,26 +695,26 @@ eal_parse_args(int argc, char **argv)
 		}
 		case OPT_SOCKET_MEM_NUM:
 			if (eal_parse_socket_arg(optarg,
-					internal_conf->socket_mem) < 0) {
+					internal_conf->numa_mem) < 0) {
 				EAL_LOG(ERR, "invalid parameters for --"
 						OPT_SOCKET_MEM);
 				eal_usage(prgname);
 				ret = -1;
 				goto out;
 			}
-			internal_conf->force_sockets = 1;
+			internal_conf->force_numa = 1;
 			break;
 
 		case OPT_SOCKET_LIMIT_NUM:
 			if (eal_parse_socket_arg(optarg,
-					internal_conf->socket_limit) < 0) {
+					internal_conf->numa_limit) < 0) {
 				EAL_LOG(ERR, "invalid parameters for --"
 						OPT_SOCKET_LIMIT);
 				eal_usage(prgname);
 				ret = -1;
 				goto out;
 			}
-			internal_conf->force_socket_limits = 1;
+			internal_conf->force_numa_limits = 1;
 			break;
 
 		case OPT_VFIO_INTR_NUM:
@@ -1137,7 +1137,7 @@ rte_eal_init(int argc, char **argv)
 		}
 	}
 
-	if (internal_conf->memory == 0 && internal_conf->force_sockets == 0) {
+	if (internal_conf->memory == 0 && internal_conf->force_numa == 0) {
 		if (internal_conf->no_hugetlbfs)
 			internal_conf->memory = MEMSIZE_IF_NO_HUGE_PAGE;
 	}
diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
index 45879ca743..45776ed33d 100644
--- a/lib/eal/linux/eal_memory.c
+++ b/lib/eal/linux/eal_memory.c
@@ -282,7 +282,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi,
 			oldpolicy = MPOL_DEFAULT;
 		}
 		for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-			if (internal_conf->socket_mem[i])
+			if (internal_conf->numa_mem[i])
 				maxnode = i + 1;
 	}
 #endif
@@ -301,7 +301,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi,
 
 			if (j == maxnode) {
 				node_id = (node_id + 1) % maxnode;
-				while (!internal_conf->socket_mem[node_id]) {
+				while (!internal_conf->numa_mem[node_id]) {
 					node_id++;
 					node_id %= maxnode;
 				}
@@ -1269,9 +1269,9 @@ eal_legacy_hugepage_init(void)
 
 	huge_register_sigbus();
 
-	/* make a copy of socket_mem, needed for balanced allocation. */
+	/* make a copy of numa_mem, needed for balanced allocation. */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		memory[i] = internal_conf->socket_mem[i];
+		memory[i] = internal_conf->numa_mem[i];
 
 	/* map all hugepages and sort them */
 	for (i = 0; i < (int)internal_conf->num_hugepage_sizes; i++) {
@@ -1339,7 +1339,7 @@ eal_legacy_hugepage_init(void)
 
 	huge_recover_sigbus();
 
-	if (internal_conf->memory == 0 && internal_conf->force_sockets == 0)
+	if (internal_conf->memory == 0 && internal_conf->force_numa == 0)
 		internal_conf->memory = eal_get_hugepage_mem_size();
 
 	nr_hugefiles = nr_hugepages;
@@ -1365,9 +1365,9 @@ eal_legacy_hugepage_init(void)
 		}
 	}
 
-	/* make a copy of socket_mem, needed for number of pages calculation */
+	/* make a copy of numa_mem, needed for number of pages calculation */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
-		memory[i] = internal_conf->socket_mem[i];
+		memory[i] = internal_conf->numa_mem[i];
 
 	/* calculate final number of pages */
 	nr_hugepages = eal_dynmem_calc_num_pages_per_socket(memory,
@@ -1722,12 +1722,12 @@ memseg_primary_init_32(void)
 	 */
 	active_sockets = 0;
 	total_requested_mem = 0;
-	if (internal_conf->force_sockets)
+	if (internal_conf->force_numa)
 		for (i = 0; i < rte_socket_count(); i++) {
 			uint64_t mem;
 
 			socket_id = rte_socket_id_by_idx(i);
-			mem = internal_conf->socket_mem[socket_id];
+			mem = internal_conf->numa_mem[socket_id];
 
 			if (mem == 0)
 				continue;
@@ -1779,7 +1779,7 @@ memseg_primary_init_32(void)
 
 		/* if we didn't specifically request memory on this socket */
 		skip = active_sockets != 0 &&
-				internal_conf->socket_mem[socket_id] == 0;
+				internal_conf->numa_mem[socket_id] == 0;
 		/* ...or if we didn't specifically request memory on *any*
 		 * socket, and this is not main lcore
 		 */
@@ -1794,7 +1794,7 @@ memseg_primary_init_32(void)
 
 		/* max amount of memory on this socket */
 		max_socket_mem = (active_sockets != 0 ?
-					internal_conf->socket_mem[socket_id] :
+					internal_conf->numa_mem[socket_id] :
 					internal_conf->memory) +
 					extra_mem_per_socket;
 		cur_socket_mem = 0;
diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
index 28b78a95a6..34bceeed4e 100644
--- a/lib/eal/windows/eal.c
+++ b/lib/eal/windows/eal.c
@@ -331,7 +331,7 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	if (internal_conf->memory == 0 && !internal_conf->force_sockets) {
+	if (internal_conf->memory == 0 && !internal_conf->force_numa) {
 		if (internal_conf->no_hugetlbfs)
 			internal_conf->memory = MEMSIZE_IF_NO_HUGE_PAGE;
 	}
-- 
2.43.5


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

* [RFC PATCH v1 4/5] eal: rename --socket-mem/--socket-limit
  2024-09-06 11:47 [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Anatoly Burakov
                   ` (2 preceding siblings ...)
  2024-09-06 11:47 ` [RFC PATCH v1 3/5] eal: rename socket ID to NUMA ID in internal config Anatoly Burakov
@ 2024-09-06 11:47 ` Anatoly Burakov
  2024-09-09  7:42   ` fengchengwen
  2024-09-06 11:47 ` [RFC PATCH v1 5/5] lcore: store physical package ID internally Anatoly Burakov
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Anatoly Burakov @ 2024-09-06 11:47 UTC (permalink / raw)
  To: dev, Matan Azrad, Viacheslav Ovsiienko, Dariusz Sosnowski,
	Bing Zhao, Ori Kam, Suanming Mou, Tyler Retzlaff,
	Nicolas Chautru, Radu Nicolau, Akhil Goyal, Maxime Coquelin,
	Chenbo Xia

Currently, --socket-mem and --socket-limit EAL flags effectively refer to
NUMA nodes, not CPU sockets. Update the flag names to reflect this. Old
flag names are still supported for backward compatibility.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    Technically, this is a user-facing change and so would require a
    deprecation notice. We can do it the other way around, and add
    support for --numa-mem/--numa-limit but do not expose it in
    documentation yet, and instead add a deprecation notice for next
    release. However, since old flags are kept for compatibility,
    nothing will break as a result of merging this series even if we
    didn't announce this change in advance. I'm open to feedback on
    how to best do this change.

 doc/guides/faq/faq.rst                        |  4 ++--
 doc/guides/howto/lm_bond_virtio_sriov.rst     |  2 +-
 doc/guides/howto/lm_virtio_vhost_user.rst     |  2 +-
 doc/guides/howto/pvp_reference_benchmark.rst  |  4 ++--
 .../virtio_user_for_container_networking.rst  |  2 +-
 doc/guides/linux_gsg/build_sample_apps.rst    | 20 +++++++++----------
 doc/guides/linux_gsg/linux_eal_parameters.rst | 16 +++++++--------
 doc/guides/nics/mlx4.rst                      |  2 +-
 doc/guides/nics/mlx5.rst                      |  2 +-
 .../prog_guide/env_abstraction_layer.rst      | 12 +++++------
 doc/guides/prog_guide/multi_proc_support.rst  |  2 +-
 doc/guides/sample_app_ug/bbdev_app.rst        |  6 +++---
 doc/guides/sample_app_ug/ipsec_secgw.rst      |  6 +++---
 doc/guides/sample_app_ug/vdpa.rst             |  2 +-
 doc/guides/sample_app_ug/vhost.rst            |  4 ++--
 lib/eal/common/eal_common_options.c           | 17 +++++++++-------
 lib/eal/common/eal_options.h                  |  8 +++++---
 lib/eal/linux/eal.c                           | 12 +++++------
 18 files changed, 64 insertions(+), 59 deletions(-)

diff --git a/doc/guides/faq/faq.rst b/doc/guides/faq/faq.rst
index 2aec432d75..8557d9daf9 100644
--- a/doc/guides/faq/faq.rst
+++ b/doc/guides/faq/faq.rst
@@ -31,7 +31,7 @@ If I execute "l2fwd -l 0-3 -m 64 -n 3 -- -p 3", I get the following output, indi
 I have set up a total of 1024 Hugepages (that is, allocated 512 2M pages to each NUMA node).
 
 The -m command line parameter does not guarantee that huge pages will be reserved on specific sockets. Therefore, allocated huge pages may not be on socket 0.
-To request memory to be reserved on a specific socket, please use the --socket-mem command-line parameter instead of -m.
+To request memory to be reserved on a specific socket, please use the --numa-mem command-line parameter instead of -m.
 
 
 I am running a 32-bit DPDK application on a NUMA system, and sometimes the application initializes fine but cannot allocate memory. Why is that happening?
@@ -54,7 +54,7 @@ For example, if your EAL coremask is 0xff0, the main core will usually be the fi
 .. Note: Instead of '-c 0xff0' use the '-l 4-11' as a cleaner way to define lcores.
 
 In this way, the hugepages have a greater chance of being allocated to the correct socket.
-Additionally, a ``--socket-mem`` option could be used to ensure the availability of memory for each socket, so that if hugepages were allocated on
+Additionally, a ``--numa-mem`` option could be used to ensure the availability of memory for each socket, so that if hugepages were allocated on
 the wrong socket, the application simply will not start.
 
 
diff --git a/doc/guides/howto/lm_bond_virtio_sriov.rst b/doc/guides/howto/lm_bond_virtio_sriov.rst
index 60b4462c2c..1859508559 100644
--- a/doc/guides/howto/lm_bond_virtio_sriov.rst
+++ b/doc/guides/howto/lm_bond_virtio_sriov.rst
@@ -614,7 +614,7 @@ Run testpmd in the Virtual Machine.
    # use for bonding of virtio and vf tests in VM
 
    /root/dpdk/<build_dir>/app/dpdk-testpmd \
-   -l 0-3 -n 4 --socket-mem 350 --  --i --port-topology=chained
+   -l 0-3 -n 4 --numa-mem 350 --  --i --port-topology=chained
 
 .. _lm_bond_virtio_sriov_switch_conf:
 
diff --git a/doc/guides/howto/lm_virtio_vhost_user.rst b/doc/guides/howto/lm_virtio_vhost_user.rst
index c5c48f10a9..b84ef0dc29 100644
--- a/doc/guides/howto/lm_virtio_vhost_user.rst
+++ b/doc/guides/howto/lm_virtio_vhost_user.rst
@@ -438,4 +438,4 @@ run_testpmd_in_vm.sh
    # test system has 8 cpus (0-7), use cpus 2-7 for VM
 
    /root/dpdk/<build_dir>/app/dpdk-testpmd \
-   -l 0-5 -n 4 --socket-mem 350 -- --burst=64 --i
+   -l 0-5 -n 4 --numa-mem 350 -- --burst=64 --i
diff --git a/doc/guides/howto/pvp_reference_benchmark.rst b/doc/guides/howto/pvp_reference_benchmark.rst
index 1043356b3d..073e72ea6f 100644
--- a/doc/guides/howto/pvp_reference_benchmark.rst
+++ b/doc/guides/howto/pvp_reference_benchmark.rst
@@ -122,7 +122,7 @@ Testpmd launch
 
    .. code-block:: console
 
-      <build_dir>/app/dpdk-testpmd -l 0,2,3,4,5 --socket-mem=1024 -n 4 \
+      <build_dir>/app/dpdk-testpmd -l 0,2,3,4,5 --numa-mem=1024 -n 4 \
           --vdev 'net_vhost0,iface=/tmp/vhost-user1' \
           --vdev 'net_vhost1,iface=/tmp/vhost-user2' -- \
           --portmask=f -i --rxq=1 --txq=1 \
@@ -332,7 +332,7 @@ Start testpmd:
 
    .. code-block:: console
 
-      <build_dir>/app/dpdk-testpmd -l 0,1,2 --socket-mem 1024 -n 4 \
+      <build_dir>/app/dpdk-testpmd -l 0,1,2 --numa-mem 1024 -n 4 \
           --proc-type auto --file-prefix pg -- \
           --portmask=3 --forward-mode=macswap --port-topology=chained \
           --disable-rss -i --rxq=1 --txq=1 \
diff --git a/doc/guides/howto/virtio_user_for_container_networking.rst b/doc/guides/howto/virtio_user_for_container_networking.rst
index 5eab360a1c..a07dfd45b0 100644
--- a/doc/guides/howto/virtio_user_for_container_networking.rst
+++ b/doc/guides/howto/virtio_user_for_container_networking.rst
@@ -77,7 +77,7 @@ some minor changes.
 
     .. code-block:: console
 
-        $(testpmd) -l 0-1 -n 4 --socket-mem 1024,1024 \
+        $(testpmd) -l 0-1 -n 4 --numa-mem 1024,1024 \
             --vdev 'eth_vhost0,iface=/tmp/sock0' \
             --file-prefix=host --no-pci -- -i
 
diff --git a/doc/guides/linux_gsg/build_sample_apps.rst b/doc/guides/linux_gsg/build_sample_apps.rst
index 4f99617233..eceba2a7b7 100644
--- a/doc/guides/linux_gsg/build_sample_apps.rst
+++ b/doc/guides/linux_gsg/build_sample_apps.rst
@@ -34,7 +34,7 @@ The following is the list of options that can be given to the EAL:
 .. code-block:: console
 
     ./rte-app [-c COREMASK | -l CORELIST] [-n NUM] [-b <domain:bus:devid.func>] \
-              [--socket-mem=MB,...] [-d LIB.so|DIR] [-m MB] [-r NUM] [-v] [--file-prefix] \
+              [--numa-mem=MB,...] [-d LIB.so|DIR] [-m MB] [-r NUM] [-v] [--file-prefix] \
 	      [--proc-type <primary|secondary|auto>]
 
 The EAL options are as follows:
@@ -55,12 +55,12 @@ The EAL options are as follows:
   use the specified Ethernet device(s) only. Use comma-separate
   ``[domain:]bus:devid.func`` values. Cannot be used with ``-b`` option.
 
-* ``--socket-mem``:
+* ``--numa-mem``:
   Memory to allocate from hugepages on specific sockets. In dynamic memory mode,
   this memory will also be pinned (i.e. not released back to the system until
   application closes).
 
-* ``--socket-limit``:
+* ``--numa-limit``:
   Limit maximum memory available for allocation on each socket. Does not support
   legacy memory mode.
 
@@ -71,7 +71,7 @@ The EAL options are as follows:
 
 * ``-m MB``:
   Memory to allocate from hugepages, regardless of processor socket. It is
-  recommended that ``--socket-mem`` be used instead of this option.
+  recommended that ``--numa-mem`` be used instead of this option.
 
 * ``-r NUM``:
   Number of memory ranks.
@@ -158,9 +158,9 @@ Hugepage Memory Use by Applications
 
 When running an application, it is recommended to use the same amount of memory as that allocated for hugepages.
 This is done automatically by the DPDK application at startup,
-if no ``-m`` or ``--socket-mem`` parameter is passed to it when run.
+if no ``-m`` or ``--numa-mem`` parameter is passed to it when run.
 
-If more memory is requested by explicitly passing a ``-m`` or ``--socket-mem`` value, the application fails.
+If more memory is requested by explicitly passing a ``-m`` or ``--numa-mem`` value, the application fails.
 However, the application itself can also fail if the user requests less memory than the reserved amount of hugepage-memory, particularly if using the ``-m`` option.
 The reason is as follows.
 Suppose the system has 1024 reserved 2 MB pages in socket 0 and 1024 in socket 1.
@@ -168,15 +168,15 @@ If the user requests 128 MB of memory, the 64 pages may not match the constraint
 
 *   The hugepage memory by be given to the application by the kernel in socket 1 only.
     In this case, if the application attempts to create an object, such as a ring or memory pool in socket 0, it fails.
-    To avoid this issue, it is recommended that the ``--socket-mem`` option be used instead of the ``-m`` option.
+    To avoid this issue, it is recommended that the ``--numa-mem`` option be used instead of the ``-m`` option.
 
 *   These pages can be located anywhere in physical memory, and, although the DPDK EAL will attempt to allocate memory in contiguous blocks,
     it is possible that the pages will not be contiguous. In this case, the application is not able to allocate big memory pools.
 
 The socket-mem option can be used to request specific amounts of memory for specific sockets.
-This is accomplished by supplying the ``--socket-mem`` flag followed by amounts of memory requested on each socket,
-for example, supply ``--socket-mem=0,512`` to try and reserve 512 MB for socket 1 only.
-Similarly, on a four socket system, to allocate 1 GB memory on each of sockets 0 and 2 only, the parameter ``--socket-mem=1024,0,1024`` can be used.
+This is accomplished by supplying the ``--numa-mem`` flag followed by amounts of memory requested on each socket,
+for example, supply ``--numa-mem=0,512`` to try and reserve 512 MB for socket 1 only.
+Similarly, on a four socket system, to allocate 1 GB memory on each of sockets 0 and 2 only, the parameter ``--numa-mem=1024,0,1024`` can be used.
 No memory will be reserved on any CPU socket that is not explicitly referenced, for example, socket 3 in this case.
 If the DPDK cannot allocate enough memory on each socket, the EAL initialization fails.
 
diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
index ea8f381391..7c5b26ce26 100644
--- a/doc/guides/linux_gsg/linux_eal_parameters.rst
+++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
@@ -60,20 +60,20 @@ Memory-related options
 
     Use legacy DPDK memory allocation mode.
 
-*   ``--socket-mem <amounts of memory per socket>``
+*   ``--numa-mem <amounts of memory per NUMA node>``
 
-    Preallocate specified amounts of memory per socket. The parameter is a
+    Preallocate specified amounts of memory per NUMA node. The parameter is a
     comma-separated list of values. For example::
 
-        --socket-mem 1024,2048
+        --numa-mem 1024,2048
 
-    This will allocate 1 gigabyte of memory on socket 0, and 2048 megabytes of
-    memory on socket 1.
+    This will allocate 1 gigabyte of memory on NUMA node 0, and 2048 megabytes of
+    memory on NUMA node 1.
 
-*   ``--socket-limit <amounts of memory per socket>``
+*   ``--numa-limit <amounts of memory per NUMA node>``
 
-    Place a per-socket upper limit on memory use (non-legacy memory mode only).
-    0 will disable the limit for a particular socket.
+    Place a per-NUMA node upper limit on memory use (non-legacy memory mode only).
+    0 will disable the limit for a particular NUMA node.
 
 *   ``--single-file-segments``
 
diff --git a/doc/guides/nics/mlx4.rst b/doc/guides/nics/mlx4.rst
index ecc5a32913..d79d5b1661 100644
--- a/doc/guides/nics/mlx4.rst
+++ b/doc/guides/nics/mlx4.rst
@@ -364,7 +364,7 @@ Performance tuning
 
 #. To minimize overhead of searching Memory Regions:
 
-   - '--socket-mem' is recommended to pin memory by predictable amount.
+   - '--numa-mem' is recommended to pin memory by predictable amount.
    - Configure per-lcore cache when creating Mempools for packet buffer.
    - Refrain from dynamically allocating/freeing memory in run-time.
 
diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 1dccdaad50..2b0613b259 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -1763,7 +1763,7 @@ Performance tuning
 
 #. To minimize overhead of searching Memory Regions:
 
-   - '--socket-mem' is recommended to pin memory by predictable amount.
+   - '--numa-mem' is recommended to pin memory by predictable amount.
    - Configure per-lcore cache when creating Mempools for packet buffer.
    - Refrain from dynamically allocating/freeing memory in run-time.
 
diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst
index b9fac1839d..6b958d67c0 100644
--- a/doc/guides/prog_guide/env_abstraction_layer.rst
+++ b/doc/guides/prog_guide/env_abstraction_layer.rst
@@ -119,11 +119,11 @@ in use, either reserved memory will satisfy the requirements, or the allocation
 will fail.
 
 There is no need to preallocate any memory at startup using ``-m`` or
-``--socket-mem`` command-line parameters, however it is still possible to do so,
+``--numa-mem`` command-line parameters, however it is still possible to do so,
 in which case preallocate memory will be "pinned" (i.e. will never be released
 by the application back to the system). It will be possible to allocate more
 hugepages, and deallocate those, but any preallocated pages will not be freed.
-If neither ``-m`` nor ``--socket-mem`` were specified, no memory will be
+If neither ``-m`` nor ``--numa-mem`` were specified, no memory will be
 preallocated, and all memory will be allocated at runtime, as needed.
 
 Another available option to use in dynamic memory mode is
@@ -143,7 +143,7 @@ to deny them), allocation validator callbacks are also available via
 ``rte_mem_alloc_validator_callback_register()`` function.
 
 A default validator callback is provided by EAL, which can be enabled with a
-``--socket-limit`` command-line option, for a simple way to limit maximum amount
+``--numa-limit`` command-line option, for a simple way to limit maximum amount
 of memory that can be used by DPDK application.
 
 .. warning::
@@ -164,7 +164,7 @@ This mode mimics historical behavior of EAL. That is, EAL will reserve all
 memory at startup, sort all memory into large IOVA-contiguous chunks, and will
 not allow acquiring or releasing hugepages from the system at runtime.
 
-If neither ``-m`` nor ``--socket-mem`` were specified, the entire available
+If neither ``-m`` nor ``--numa-mem`` were specified, the entire available
 hugepage memory will be preallocated.
 
 Hugepage Allocation Matching
@@ -187,7 +187,7 @@ very dependent on the memory allocation patterns of the application.
 
 Additional restrictions are present when running in 32-bit mode. In dynamic
 memory mode, by default maximum of 2 gigabytes of VA space will be preallocated,
-and all of it will be on main lcore NUMA node unless ``--socket-mem`` flag is
+and all of it will be on main lcore NUMA node unless ``--numa-mem`` flag is
 used.
 
 In legacy mode, VA space will only be preallocated for segments that were
@@ -1183,7 +1183,7 @@ into a single block.
 If deallocating pages at runtime is supported, and the free element encloses
 one or more pages, those pages can be deallocated and be removed from the heap.
 If DPDK was started with command-line parameters for preallocating memory
-(``-m`` or ``--socket-mem``), then those pages that were allocated at startup
+(``-m`` or ``--numa-mem``), then those pages that were allocated at startup
 will not be deallocated.
 
 Any successful deallocation event will trigger a callback, for which user
diff --git a/doc/guides/prog_guide/multi_proc_support.rst b/doc/guides/prog_guide/multi_proc_support.rst
index 0c57145470..3082ffe183 100644
--- a/doc/guides/prog_guide/multi_proc_support.rst
+++ b/doc/guides/prog_guide/multi_proc_support.rst
@@ -127,7 +127,7 @@ allocate more memory than they need. However if ``--legacy-mem`` is used, DPDK
 will attempt to preallocate all memory it can get to, and memory use must be
 explicitly limited. This is done by passing the ``-m`` flag to each process to
 specify how much hugepage memory, in megabytes, each process can use (or passing
-``--socket-mem`` to specify how much hugepage memory on each socket each process
+``--numa-mem`` to specify how much hugepage memory on each socket each process
 can use).
 
 .. note::
diff --git a/doc/guides/sample_app_ug/bbdev_app.rst b/doc/guides/sample_app_ug/bbdev_app.rst
index 7f02f0ed90..bd1387cb5c 100644
--- a/doc/guides/sample_app_ug/bbdev_app.rst
+++ b/doc/guides/sample_app_ug/bbdev_app.rst
@@ -67,7 +67,7 @@ issue the command:
 .. code-block:: console
 
     $ ./<build_dir>/examples/dpdk-bbdev --vdev='baseband_turbo_sw' -a <NIC0PCIADDR> \
-    -c 0x38 --socket-mem=2,2 --file-prefix=bbdev -- -e 0x10 -d 0x20
+    -c 0x38 --numa-mem=2,2 --file-prefix=bbdev -- -e 0x10 -d 0x20
 
 where, NIC0PCIADDR is the PCI address of the Rx port
 
@@ -99,12 +99,12 @@ ports.
 .. code-block:: console
 
     $ ./pktgen-3.4.0/app/x86_64-native-linux-gcc/pktgen -c 0x3 \
-    --socket-mem=1,1 --file-prefix=pg -a <NIC1PCIADDR> -- -m 1.0 -P
+    --numa-mem=1,1 --file-prefix=pg -a <NIC1PCIADDR> -- -m 1.0 -P
 
 where:
 
 * ``-c COREMASK``: A hexadecimal bitmask of cores to run on
-* ``--socket-mem``: Memory to allocate on specific sockets (use comma separated values)
+* ``--numa-mem``: Memory to allocate on specific sockets (use comma separated values)
 * ``--file-prefix``: Prefix for hugepage filenames
 * ``-a <NIC1PCIADDR>``: Add a PCI device in allow list. The argument format is <[domain:]bus:devid.func>.
 * ``-m <string>``: Matrix for mapping ports to logical cores.
diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst b/doc/guides/sample_app_ug/ipsec_secgw.rst
index cada8c375f..7dce577de9 100644
--- a/doc/guides/sample_app_ug/ipsec_secgw.rst
+++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
@@ -270,7 +270,7 @@ The mapping of lcores to port/queues is similar to other l3fwd applications.
 
 For example, given the following command line to run application in poll mode::
 
-    ./<build_dir>/examples/dpdk-ipsec-secgw -l 20,21 -n 4 --socket-mem 0,2048       \
+    ./<build_dir>/examples/dpdk-ipsec-secgw -l 20,21 -n 4 --numa-mem 0,2048       \
            --vdev "crypto_null" -- -p 0xf -P -u 0x3             \
            --config="(0,0,20),(1,0,20),(2,0,21),(3,0,21)"       \
            -f /path/to/config_file --transfer-mode poll         \
@@ -281,7 +281,7 @@ where each option means:
 
 *   The ``-n`` option sets memory 4 channels.
 
-*   The ``--socket-mem`` to use 2GB on socket 1.
+*   The ``--numa-mem`` to use 2GB on socket 1.
 
 *   The ``--vdev "crypto_null"`` option creates virtual NULL cryptodev PMD.
 
@@ -368,7 +368,7 @@ For example, something like the following command line:
 
 .. code-block:: console
 
-    ./<build_dir>/examples/dpdk-ipsec-secgw -l 20,21 -n 4 --socket-mem 0,2048 \
+    ./<build_dir>/examples/dpdk-ipsec-secgw -l 20,21 -n 4 --numa-mem 0,2048 \
             -a 81:00.0 -a 81:00.1 -a 81:00.2 -a 81:00.3 \
             --vdev "crypto_aesni_mb" --vdev "crypto_null" \
 	    -- \
diff --git a/doc/guides/sample_app_ug/vdpa.rst b/doc/guides/sample_app_ug/vdpa.rst
index bc11242d03..f3e96f36d7 100644
--- a/doc/guides/sample_app_ug/vdpa.rst
+++ b/doc/guides/sample_app_ug/vdpa.rst
@@ -54,7 +54,7 @@ Take IFCVF driver for example:
 
 .. code-block:: console
 
-        ./dpdk-vdpa -c 0x2 -n 4 --socket-mem 1024,1024 \
+        ./dpdk-vdpa -c 0x2 -n 4 --numa-mem 1024,1024 \
                 -a 0000:06:00.3,vdpa=1 -a 0000:06:00.4,vdpa=1 \
                 -- --interactive
 
diff --git a/doc/guides/sample_app_ug/vhost.rst b/doc/guides/sample_app_ug/vhost.rst
index 982e19214d..ec2805a0c9 100644
--- a/doc/guides/sample_app_ug/vhost.rst
+++ b/doc/guides/sample_app_ug/vhost.rst
@@ -60,7 +60,7 @@ Start the vswitch example
 
 .. code-block:: console
 
-        ./dpdk-vhost -l 0-3 -n 4 --socket-mem 1024  \
+        ./dpdk-vhost -l 0-3 -n 4 --numa-mem 1024  \
              -- --socket-file /tmp/sock0 --client \
              ...
 
@@ -218,5 +218,5 @@ Common Issues
   pre-allocation
 
   The builtin example doesn't support dynamic memory allocation. When vhost
-  backend enables "builtin-net-driver", "--socket-mem" option should be
+  backend enables "builtin-net-driver", "--numa-mem" option should be
   added at virtio-user PMD side as a startup item.
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 73fbb8587b..f29e7a585c 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -91,8 +91,11 @@ eal_long_options[] = {
 	{OPT_DEV_BLOCK,         1, NULL, OPT_DEV_BLOCK_NUM        },
 	{OPT_DEV_ALLOW,		1, NULL, OPT_DEV_ALLOW_NUM	  },
 	{OPT_PROC_TYPE,         1, NULL, OPT_PROC_TYPE_NUM        },
-	{OPT_SOCKET_MEM,        1, NULL, OPT_SOCKET_MEM_NUM       },
-	{OPT_SOCKET_LIMIT,      1, NULL, OPT_SOCKET_LIMIT_NUM     },
+	/* socket-mem/socket-limit are kept for backwards compatiblity */
+	{OPT_SOCKET_MEM,        1, NULL, OPT_NUMA_MEM_NUM         },
+	{OPT_SOCKET_LIMIT,      1, NULL, OPT_NUMA_LIMIT_NUM       },
+	{OPT_NUMA_MEM,          1, NULL, OPT_NUMA_MEM_NUM         },
+	{OPT_NUMA_LIMIT,        1, NULL, OPT_NUMA_LIMIT_NUM       },
 	{OPT_SYSLOG,            1, NULL, OPT_SYSLOG_NUM           },
 	{OPT_VDEV,              1, NULL, OPT_VDEV_NUM             },
 	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
@@ -2083,12 +2086,12 @@ eal_check_common_options(struct internal_config *internal_cfg)
 		return -1;
 	}
 	if (mem_parsed && internal_cfg->force_numa == 1) {
-		EAL_LOG(ERR, "Options -m and --"OPT_SOCKET_MEM" cannot "
+		EAL_LOG(ERR, "Options -m and --"OPT_NUMA_MEM" cannot "
 			"be specified at the same time");
 		return -1;
 	}
 	if (internal_cfg->no_hugetlbfs && internal_cfg->force_numa == 1) {
-		EAL_LOG(ERR, "Option --"OPT_SOCKET_MEM" cannot "
+		EAL_LOG(ERR, "Option --"OPT_NUMA_MEM" cannot "
 			"be specified together with --"OPT_NO_HUGE);
 		return -1;
 	}
@@ -2106,7 +2109,7 @@ eal_check_common_options(struct internal_config *internal_cfg)
 		return -1;
 	}
 	if (internal_conf->force_numa_limits && internal_conf->legacy_mem) {
-		EAL_LOG(ERR, "Option --"OPT_SOCKET_LIMIT
+		EAL_LOG(ERR, "Option --"OPT_NUMA_LIMIT
 			" is only supported in non-legacy memory mode");
 	}
 	if (internal_cfg->single_file_segments &&
@@ -2141,7 +2144,7 @@ eal_check_common_options(struct internal_config *internal_cfg)
 	if (internal_cfg->legacy_mem && internal_cfg->memory == 0) {
 		EAL_LOG(NOTICE, "Static memory layout is selected, "
 			"amount of reserved memory can be adjusted with "
-			"-m or --"OPT_SOCKET_MEM);
+			"-m or --"OPT_NUMA_MEM);
 	}
 
 	return 0;
@@ -2194,7 +2197,7 @@ eal_common_usage(void)
 	       "  --"OPT_MAIN_LCORE" ID     Core ID that is used as main\n"
 	       "  --"OPT_MBUF_POOL_OPS_NAME" Pool ops name for mbuf to use\n"
 	       "  -n CHANNELS         Number of memory channels\n"
-	       "  -m MB               Memory to allocate (see also --"OPT_SOCKET_MEM")\n"
+	       "  -m MB               Memory to allocate (see also --"OPT_NUMA_MEM")\n"
 	       "  -r RANKS            Force number of memory ranks (don't detect)\n"
 	       "  -b, --block         Add a device to the blocked list.\n"
 	       "                      Prevent EAL from using this device. The argument\n"
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 3cc9cb6412..597cd3f655 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -60,9 +60,11 @@ enum {
 #define OPT_IN_MEMORY         "in-memory"
 	OPT_IN_MEMORY_NUM,
 #define OPT_SOCKET_MEM        "socket-mem"
-	OPT_SOCKET_MEM_NUM,
-#define OPT_SOCKET_LIMIT        "socket-limit"
-	OPT_SOCKET_LIMIT_NUM,
+#define OPT_NUMA_MEM          "numa-mem"
+	OPT_NUMA_MEM_NUM,
+#define OPT_SOCKET_LIMIT      "socket-limit"
+#define OPT_NUMA_LIMIT        "numa-limit"
+	OPT_NUMA_LIMIT_NUM,
 #define OPT_SYSLOG            "syslog"
 	OPT_SYSLOG_NUM,
 #define OPT_VDEV              "vdev"
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index cf174aeaa3..591028f641 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -443,8 +443,8 @@ eal_usage(const char *prgname)
 	printf("\nUsage: %s ", prgname);
 	eal_common_usage();
 	printf("EAL Linux options:\n"
-	       "  --"OPT_SOCKET_MEM"        Memory to allocate on sockets (comma separated values)\n"
-	       "  --"OPT_SOCKET_LIMIT"      Limit memory allocation on sockets (comma separated values)\n"
+	       "  --"OPT_NUMA_MEM"        Memory to allocate on NUMA nodes (comma separated values)\n"
+	       "  --"OPT_NUMA_LIMIT"      Limit memory allocation on NUMA nodes (comma separated values)\n"
 	       "  --"OPT_HUGE_DIR"          Directory where hugetlbfs is mounted\n"
 	       "  --"OPT_FILE_PREFIX"       Prefix for hugepage filenames\n"
 	       "  --"OPT_CREATE_UIO_DEV"    Create /dev/uioX (usually done by hotplug)\n"
@@ -693,11 +693,11 @@ eal_parse_args(int argc, char **argv)
 			}
 			break;
 		}
-		case OPT_SOCKET_MEM_NUM:
+		case OPT_NUMA_MEM_NUM:
 			if (eal_parse_socket_arg(optarg,
 					internal_conf->numa_mem) < 0) {
 				EAL_LOG(ERR, "invalid parameters for --"
-						OPT_SOCKET_MEM);
+						OPT_NUMA_MEM);
 				eal_usage(prgname);
 				ret = -1;
 				goto out;
@@ -705,11 +705,11 @@ eal_parse_args(int argc, char **argv)
 			internal_conf->force_numa = 1;
 			break;
 
-		case OPT_SOCKET_LIMIT_NUM:
+		case OPT_NUMA_LIMIT_NUM:
 			if (eal_parse_socket_arg(optarg,
 					internal_conf->numa_limit) < 0) {
 				EAL_LOG(ERR, "invalid parameters for --"
-						OPT_SOCKET_LIMIT);
+						OPT_NUMA_LIMIT);
 				eal_usage(prgname);
 				ret = -1;
 				goto out;
-- 
2.43.5


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

* [RFC PATCH v1 5/5] lcore: store physical package ID internally
  2024-09-06 11:47 [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Anatoly Burakov
                   ` (3 preceding siblings ...)
  2024-09-06 11:47 ` [RFC PATCH v1 4/5] eal: rename --socket-mem/--socket-limit Anatoly Burakov
@ 2024-09-06 11:47 ` Anatoly Burakov
  2024-09-09  7:49   ` fengchengwen
  2024-10-14 17:31   ` Stephen Hemminger
  2024-09-06 12:37 ` [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Morten Brørup
  2024-09-09  7:51 ` fengchengwen
  6 siblings, 2 replies; 16+ messages in thread
From: Anatoly Burakov @ 2024-09-06 11:47 UTC (permalink / raw)
  To: dev, Tyler Retzlaff, Bruce Richardson, Dmitry Kozlyuk, Pallavi Kadam

This patch introduces a new field in the lcore structure that stores the
physical package ID of the core. This field is populated during EAL init.
It is not exposed through any external API's for now.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/common/eal_common_lcore.c | 18 ++++++++++++++++++
 lib/eal/common/eal_private.h      |  3 +++
 lib/eal/common/eal_thread.h       | 11 +++++++++++
 lib/eal/freebsd/eal_lcore.c       |  6 ++++++
 lib/eal/linux/eal_lcore.c         | 28 ++++++++++++++++++++++++++++
 lib/eal/windows/eal_lcore.c       |  7 +++++++
 6 files changed, 73 insertions(+)

diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_common_lcore.c
index ba8fce6607..9e937c2d6a 100644
--- a/lib/eal/common/eal_common_lcore.c
+++ b/lib/eal/common/eal_common_lcore.c
@@ -144,7 +144,9 @@ rte_eal_cpu_init(void)
 	unsigned lcore_id;
 	unsigned count = 0;
 	unsigned int socket_id, prev_socket_id;
+	unsigned int package_id, prev_package_id;
 	int lcore_to_socket_id[RTE_MAX_LCORE];
+	int lcore_to_package_id[RTE_MAX_LCORE];
 
 	/*
 	 * Parse the maximum set of logical cores, detect the subset of running
@@ -160,6 +162,10 @@ rte_eal_cpu_init(void)
 		socket_id = eal_cpu_socket_id(lcore_id);
 		lcore_to_socket_id[lcore_id] = socket_id;
 
+		/* find physical package ID */
+		package_id = eal_cpu_package_id(lcore_id);
+		lcore_to_package_id[lcore_id] = package_id;
+
 		if (eal_cpu_detected(lcore_id) == 0) {
 			config->lcore_role[lcore_id] = ROLE_OFF;
 			lcore_config[lcore_id].core_index = -1;
@@ -174,6 +180,7 @@ rte_eal_cpu_init(void)
 		lcore_config[lcore_id].core_role = ROLE_RTE;
 		lcore_config[lcore_id].core_id = eal_cpu_core_id(lcore_id);
 		lcore_config[lcore_id].numa_id = socket_id;
+		lcore_config[lcore_id].package_id = package_id;
 		EAL_LOG(DEBUG, "Detected lcore %u as "
 				"core %u on NUMA node %u",
 				lcore_id, lcore_config[lcore_id].core_id,
@@ -199,14 +206,25 @@ rte_eal_cpu_init(void)
 	qsort(lcore_to_socket_id, RTE_DIM(lcore_to_socket_id),
 			sizeof(lcore_to_socket_id[0]), socket_id_cmp);
 
+	/* sort all package id's in ascending order */
+	qsort(lcore_to_package_id, RTE_DIM(lcore_to_package_id),
+			sizeof(lcore_to_package_id[0]), socket_id_cmp);
+
 	prev_socket_id = -1;
+	prev_package_id = -1;
 	config->numa_node_count = 0;
+	config->package_count = 0;
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		socket_id = lcore_to_socket_id[lcore_id];
+		package_id = lcore_to_package_id[lcore_id];
 		if (socket_id != prev_socket_id)
 			config->numa_nodes[config->numa_node_count++] =
 					socket_id;
+		if (package_id != prev_package_id)
+			config->packages[config->package_count++] =
+					package_id;
 		prev_socket_id = socket_id;
+		prev_package_id = package_id;
 	}
 	EAL_LOG(INFO, "Detected NUMA nodes: %u", config->numa_node_count);
 
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 196dadc8a2..611c0de640 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -31,6 +31,7 @@ struct lcore_config {
 
 	volatile RTE_ATOMIC(enum rte_lcore_state_t) state; /**< lcore state */
 	unsigned int numa_id;    /**< NUMA node ID for this lcore */
+	unsigned int package_id;   /**< Physical package ID for this lcore */
 	unsigned int core_id;      /**< core number on socket for this lcore */
 	int core_index;            /**< relative index, starting from 0 */
 	uint8_t core_role;         /**< role of core eg: OFF, RTE, SERVICE */
@@ -48,6 +49,8 @@ struct rte_config {
 	uint32_t lcore_count;        /**< Number of available logical cores. */
 	uint32_t numa_node_count;    /**< Number of detected NUMA nodes. */
 	uint32_t numa_nodes[RTE_MAX_NUMA_NODES]; /**< List of detected NUMA nodes. */
+	uint32_t package_count;      /**< Number of detected physical packages. */
+	uint32_t packages[RTE_MAX_NUMA_NODES]; /**< List of detected physical packages. */
 	uint32_t service_lcore_count;/**< Number of available service cores. */
 	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
 
diff --git a/lib/eal/common/eal_thread.h b/lib/eal/common/eal_thread.h
index 1c3c3442d3..32ba36589e 100644
--- a/lib/eal/common/eal_thread.h
+++ b/lib/eal/common/eal_thread.h
@@ -27,6 +27,17 @@ __rte_noreturn uint32_t eal_thread_loop(void *arg);
  */
 unsigned eal_cpu_socket_id(unsigned cpu_id);
 
+/**
+ * Get the package id from cpu id.
+ * This function is private to EAL.
+ *
+ * @param cpu_id
+ *   The logical process id.
+ * @return
+ *   socket_id or SOCKET_ID_ANY
+ */
+unsigned eal_cpu_package_id(unsigned cpu_id);
+
 /**
  * Default buffer size to use with eal_thread_dump_affinity()
  */
diff --git a/lib/eal/freebsd/eal_lcore.c b/lib/eal/freebsd/eal_lcore.c
index 1d3d1b67b9..26284bf7e3 100644
--- a/lib/eal/freebsd/eal_lcore.c
+++ b/lib/eal/freebsd/eal_lcore.c
@@ -41,6 +41,12 @@ eal_cpu_socket_id(__rte_unused unsigned cpu_id)
 	return 0;
 }
 
+unsigned
+eal_cpu_package_id(__rte_unused unsigned cpu_id)
+{
+	return 0;
+}
+
 /* Check if a cpu is present by the presence of the
  * cpu information for it.
  */
diff --git a/lib/eal/linux/eal_lcore.c b/lib/eal/linux/eal_lcore.c
index 29b36dd610..c436686e1b 100644
--- a/lib/eal/linux/eal_lcore.c
+++ b/lib/eal/linux/eal_lcore.c
@@ -13,6 +13,7 @@
 
 #define SYS_CPU_DIR "/sys/devices/system/cpu/cpu%u"
 #define CORE_ID_FILE "topology/core_id"
+#define PACKAGE_ID_FILE "topology/physical_package_id"
 #define NUMA_NODE_PATH "/sys/devices/system/node"
 
 /* Check if a cpu is present by the presence of the cpu information for it */
@@ -53,6 +54,33 @@ eal_cpu_socket_id(unsigned lcore_id)
 	return 0;
 }
 
+/*
+ * Get CPU package ID for a logical core.
+ *
+ * This searches each nodeX directories in /sys for the symlink for the given
+ * lcore_id and returns the numa node where the lcore is found. If lcore is not
+ * found on any numa node, returns zero.
+ */
+unsigned
+eal_cpu_package_id(unsigned lcore_id)
+{
+	char path[PATH_MAX];
+	unsigned long id;
+
+	int len = snprintf(path, sizeof(path),
+			SYS_CPU_DIR "/%s", lcore_id, PACKAGE_ID_FILE);
+	if (len <= 0 || (unsigned)len >= sizeof(path))
+		goto err;
+	if (eal_parse_sysfs_value(path, &id) != 0)
+		goto err;
+	return (unsigned)id;
+
+err:
+	EAL_LOG(ERR, "Error reading package id value from %s "
+			"for lcore %u - assuming package 0", SYS_CPU_DIR, lcore_id);
+	return 0;
+}
+
 /* Get the cpu core id value from the /sys/.../cpuX core_id value */
 unsigned
 eal_cpu_core_id(unsigned lcore_id)
diff --git a/lib/eal/windows/eal_lcore.c b/lib/eal/windows/eal_lcore.c
index a498044620..cca954b689 100644
--- a/lib/eal/windows/eal_lcore.c
+++ b/lib/eal/windows/eal_lcore.c
@@ -233,6 +233,13 @@ eal_cpu_socket_id(unsigned int lcore_id)
 	return cpu_map.lcores[lcore_id].socket_id;
 }
 
+unsigned
+eal_cpu_package_id(unsigned int lcore_id)
+{
+	/* FIXME: Windows does support reporting Package ID */
+	return cpu_map.lcores[lcore_id].socket_id;
+}
+
 unsigned
 eal_cpu_core_id(unsigned int lcore_id)
 {
-- 
2.43.5


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

* RE: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
  2024-09-06 11:47 [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Anatoly Burakov
                   ` (4 preceding siblings ...)
  2024-09-06 11:47 ` [RFC PATCH v1 5/5] lcore: store physical package ID internally Anatoly Burakov
@ 2024-09-06 12:37 ` Morten Brørup
  2024-09-06 12:46   ` Burakov, Anatoly
  2024-09-09  7:51 ` fengchengwen
  6 siblings, 1 reply; 16+ messages in thread
From: Morten Brørup @ 2024-09-06 12:37 UTC (permalink / raw)
  To: Anatoly Burakov, dev

> From: Anatoly Burakov [mailto:anatoly.burakov@intel.com]
> Sent: Friday, 6 September 2024 13.47
> To: dev@dpdk.org
> Subject: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
> 
> While initially, DPDK has used the term "socket ID" to refer to physical
> package
> ID, the last time DPDK read "physical_package_id" for socket ID was ~9 years
> ago, so it's been a while since we've actually switched over to using the term
> "socket" to mean "NUMA node".
> 
> This wasn't a problem before, as most systems had one NUMA node per physical
> socket. However, in the last few years, more and more systems have multiple
> NUMA
> nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
> transition was pretty seamless, however now we're faced with a situation when
> most of our documentation still uses outdated terms, and our API is ripe with
> references to "sockets" when in actuality we mean "NUMA nodes". This could be
> a
> source of confusion.
> 
> While completely renaming all of our API's would be a huge effort, will take a
> long time and arguably wouldn't even be worth the API breakages (given that
> this
> mismatch between terminology and reality is implicitly understood by most
> people
> working on DPDK, and so this isn't so much of a problem in practice), we can
> do
> some tweaks around the edges and at least document this unfortunate reality.
> 
> This patchset suggests the following changes:
> 
> - Update rte_socket/rte_lcore documentation to refer to NUMA nodes rather than
> sockets - Rename internal structures' fields to better reflect this intention
> -
> Rename --socket-mem/--socket-limit flags to refer to NUMA rather than sockets
> -
> Add internal API to get physical package ID [1]
> 
> The documentation is updated to refer to new EAL flags, but is otherwise left
> untouched, and instead the entry in "glossary" is amended to indicate that
> when
> DPDK documentation refers to "sockets", it actually means "NUMA ID's". As next
> steps, we could rename all API parameters to refer to NUMA ID rather than
> socket
> ID - this would not break neither API nor ABI, and instead would be a
> documentation change in practice.
> 
> [1] This could be used to group lcores by physical package, see e.g.
> discussion
>     under this patch:
> https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-
> vipin.varghese@amd.com/

Thank you for cleaning this up, Anatoly.

I would prefer to take one more step and also rename functions and parameters, e.g. rte_socket_id() -> rte_numa_id().

For backwards compatibility, macros/functions with the old names can be added.


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

* Re: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
  2024-09-06 12:37 ` [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Morten Brørup
@ 2024-09-06 12:46   ` Burakov, Anatoly
  2024-09-06 13:02     ` Morten Brørup
  0 siblings, 1 reply; 16+ messages in thread
From: Burakov, Anatoly @ 2024-09-06 12:46 UTC (permalink / raw)
  To: Morten Brørup, dev

On 9/6/2024 2:37 PM, Morten Brørup wrote:
>> From: Anatoly Burakov [mailto:anatoly.burakov@intel.com]
>> Sent: Friday, 6 September 2024 13.47
>> To: dev@dpdk.org
>> Subject: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
>>
>> While initially, DPDK has used the term "socket ID" to refer to physical
>> package
>> ID, the last time DPDK read "physical_package_id" for socket ID was ~9 years
>> ago, so it's been a while since we've actually switched over to using the term
>> "socket" to mean "NUMA node".
>>
>> This wasn't a problem before, as most systems had one NUMA node per physical
>> socket. However, in the last few years, more and more systems have multiple
>> NUMA
>> nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
>> transition was pretty seamless, however now we're faced with a situation when
>> most of our documentation still uses outdated terms, and our API is ripe with
>> references to "sockets" when in actuality we mean "NUMA nodes". This could be
>> a
>> source of confusion.
>>
>> While completely renaming all of our API's would be a huge effort, will take a
>> long time and arguably wouldn't even be worth the API breakages (given that
>> this
>> mismatch between terminology and reality is implicitly understood by most
>> people
>> working on DPDK, and so this isn't so much of a problem in practice), we can
>> do
>> some tweaks around the edges and at least document this unfortunate reality.
>>
>> This patchset suggests the following changes:
>>
>> - Update rte_socket/rte_lcore documentation to refer to NUMA nodes rather than
>> sockets - Rename internal structures' fields to better reflect this intention
>> -
>> Rename --socket-mem/--socket-limit flags to refer to NUMA rather than sockets
>> -
>> Add internal API to get physical package ID [1]
>>
>> The documentation is updated to refer to new EAL flags, but is otherwise left
>> untouched, and instead the entry in "glossary" is amended to indicate that
>> when
>> DPDK documentation refers to "sockets", it actually means "NUMA ID's". As next
>> steps, we could rename all API parameters to refer to NUMA ID rather than
>> socket
>> ID - this would not break neither API nor ABI, and instead would be a
>> documentation change in practice.
>>
>> [1] This could be used to group lcores by physical package, see e.g.
>> discussion
>>      under this patch:
>> https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-
>> vipin.varghese@amd.com/
> 
> Thank you for cleaning this up, Anatoly.
> 
> I would prefer to take one more step and also rename functions and parameters, e.g. rte_socket_id() -> rte_numa_id().
> 
> For backwards compatibility, macros/functions with the old names can be added.
> 

I don't think we can do such changes without deprecation notices, but 
it's a good candidate for next release.

I have thought about including parameter renames in this patchset, but 
for now I decided against doing so. I can certainly include this in the 
next revision if that's something community is willing to accept.

-- 
Thanks,
Anatoly


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

* RE: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
  2024-09-06 12:46   ` Burakov, Anatoly
@ 2024-09-06 13:02     ` Morten Brørup
  2024-09-06 13:07       ` Bruce Richardson
  0 siblings, 1 reply; 16+ messages in thread
From: Morten Brørup @ 2024-09-06 13:02 UTC (permalink / raw)
  To: Burakov, Anatoly, dev

> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> Sent: Friday, 6 September 2024 14.46
> 
> On 9/6/2024 2:37 PM, Morten Brørup wrote:
> >> From: Anatoly Burakov [mailto:anatoly.burakov@intel.com]
> >> Sent: Friday, 6 September 2024 13.47
> >> To: dev@dpdk.org
> >> Subject: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
> >>
> >> While initially, DPDK has used the term "socket ID" to refer to physical
> >> package
> >> ID, the last time DPDK read "physical_package_id" for socket ID was ~9
> years
> >> ago, so it's been a while since we've actually switched over to using the
> term
> >> "socket" to mean "NUMA node".
> >>
> >> This wasn't a problem before, as most systems had one NUMA node per
> physical
> >> socket. However, in the last few years, more and more systems have multiple
> >> NUMA
> >> nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
> >> transition was pretty seamless, however now we're faced with a situation
> when
> >> most of our documentation still uses outdated terms, and our API is ripe
> with
> >> references to "sockets" when in actuality we mean "NUMA nodes". This could
> be
> >> a
> >> source of confusion.
> >>
> >> While completely renaming all of our API's would be a huge effort, will
> take a
> >> long time and arguably wouldn't even be worth the API breakages (given that
> >> this
> >> mismatch between terminology and reality is implicitly understood by most
> >> people
> >> working on DPDK, and so this isn't so much of a problem in practice), we
> can
> >> do
> >> some tweaks around the edges and at least document this unfortunate
> reality.
> >>
> >> This patchset suggests the following changes:
> >>
> >> - Update rte_socket/rte_lcore documentation to refer to NUMA nodes rather
> than
> >> sockets - Rename internal structures' fields to better reflect this
> intention
> >> -
> >> Rename --socket-mem/--socket-limit flags to refer to NUMA rather than
> sockets
> >> -
> >> Add internal API to get physical package ID [1]
> >>
> >> The documentation is updated to refer to new EAL flags, but is otherwise
> left
> >> untouched, and instead the entry in "glossary" is amended to indicate that
> >> when
> >> DPDK documentation refers to "sockets", it actually means "NUMA ID's". As
> next
> >> steps, we could rename all API parameters to refer to NUMA ID rather than
> >> socket
> >> ID - this would not break neither API nor ABI, and instead would be a
> >> documentation change in practice.
> >>
> >> [1] This could be used to group lcores by physical package, see e.g.
> >> discussion
> >>      under this patch:
> >> https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-
> >> vipin.varghese@amd.com/
> >
> > Thank you for cleaning this up, Anatoly.
> >
> > I would prefer to take one more step and also rename functions and
> parameters, e.g. rte_socket_id() -> rte_numa_id().
> >
> > For backwards compatibility, macros/functions with the old names can be
> added.
> >
> 
> I don't think we can do such changes without deprecation notices, but
> it's a good candidate for next release.

Perhaps we can keep ABI compatibility by adding wrapper functions with the old names/parameters, which simply call the same functions with the new names/parameters.

The Devil is in the details, and I haven't looked deeply into this. So take with a grain of salt.

> 
> I have thought about including parameter renames in this patchset, but
> for now I decided against doing so. I can certainly include this in the
> next revision if that's something community is willing to accept.

I agree with your decision on this. Renaming the parameters without renaming the functions could be confusing.

If we cannot take the additional step to rename the functions, let's also not rename their parameters.

> 
> --
> Thanks,
> Anatoly


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

* Re: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
  2024-09-06 13:02     ` Morten Brørup
@ 2024-09-06 13:07       ` Bruce Richardson
  2024-09-06 13:17         ` Burakov, Anatoly
  0 siblings, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2024-09-06 13:07 UTC (permalink / raw)
  To: Morten Brørup; +Cc: Burakov, Anatoly, dev

On Fri, Sep 06, 2024 at 03:02:53PM +0200, Morten Brørup wrote:
> > From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> > Sent: Friday, 6 September 2024 14.46
> > 
> > On 9/6/2024 2:37 PM, Morten Brørup wrote:
> > >> From: Anatoly Burakov [mailto:anatoly.burakov@intel.com]
> > >> Sent: Friday, 6 September 2024 13.47
> > >> To: dev@dpdk.org
> > >> Subject: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
> > >>
> > >> While initially, DPDK has used the term "socket ID" to refer to physical
> > >> package
> > >> ID, the last time DPDK read "physical_package_id" for socket ID was ~9
> > years
> > >> ago, so it's been a while since we've actually switched over to using the
> > term
> > >> "socket" to mean "NUMA node".
> > >>
> > >> This wasn't a problem before, as most systems had one NUMA node per
> > physical
> > >> socket. However, in the last few years, more and more systems have multiple
> > >> NUMA
> > >> nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
> > >> transition was pretty seamless, however now we're faced with a situation
> > when
> > >> most of our documentation still uses outdated terms, and our API is ripe
> > with
> > >> references to "sockets" when in actuality we mean "NUMA nodes". This could
> > be
> > >> a
> > >> source of confusion.
> > >>
> > >> While completely renaming all of our API's would be a huge effort, will
> > take a
> > >> long time and arguably wouldn't even be worth the API breakages (given that
> > >> this
> > >> mismatch between terminology and reality is implicitly understood by most
> > >> people
> > >> working on DPDK, and so this isn't so much of a problem in practice), we
> > can
> > >> do
> > >> some tweaks around the edges and at least document this unfortunate
> > reality.
> > >>
> > >> This patchset suggests the following changes:
> > >>
> > >> - Update rte_socket/rte_lcore documentation to refer to NUMA nodes rather
> > than
> > >> sockets - Rename internal structures' fields to better reflect this
> > intention
> > >> -
> > >> Rename --socket-mem/--socket-limit flags to refer to NUMA rather than
> > sockets
> > >> -
> > >> Add internal API to get physical package ID [1]
> > >>
> > >> The documentation is updated to refer to new EAL flags, but is otherwise
> > left
> > >> untouched, and instead the entry in "glossary" is amended to indicate that
> > >> when
> > >> DPDK documentation refers to "sockets", it actually means "NUMA ID's". As
> > next
> > >> steps, we could rename all API parameters to refer to NUMA ID rather than
> > >> socket
> > >> ID - this would not break neither API nor ABI, and instead would be a
> > >> documentation change in practice.
> > >>
> > >> [1] This could be used to group lcores by physical package, see e.g.
> > >> discussion
> > >>      under this patch:
> > >> https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-
> > >> vipin.varghese@amd.com/
> > >
> > > Thank you for cleaning this up, Anatoly.
> > >
> > > I would prefer to take one more step and also rename functions and
> > parameters, e.g. rte_socket_id() -> rte_numa_id().
> > >
> > > For backwards compatibility, macros/functions with the old names can be
> > added.
> > >
> > 
> > I don't think we can do such changes without deprecation notices, but
> > it's a good candidate for next release.
> 
> Perhaps we can keep ABI compatibility by adding wrapper functions with the old names/parameters, which simply call the same functions with the new names/parameters.
> 
> The Devil is in the details, and I haven't looked deeply into this. So take with a grain of salt.
> 
> > 
> > I have thought about including parameter renames in this patchset, but
> > for now I decided against doing so. I can certainly include this in the
> > next revision if that's something community is willing to accept.
> 
> I agree with your decision on this. Renaming the parameters without renaming the functions could be confusing.
> 

I actually wonder if that is true. If we are simply renaming the parameters
without:
a) changing their types
b) changing the function behaviour
then it is neither an API nor an ABI break. If we were to do so, it would
be like changing a comment, since the actual parameter name is purely a
convenience to hint to the user what the value being passed actually does.

That only applies for function parameters though. For any defines or macros
that need renaming, then we are into API break territory and we would want
backward compatible versions of same.

/Bruce

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

* Re: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
  2024-09-06 13:07       ` Bruce Richardson
@ 2024-09-06 13:17         ` Burakov, Anatoly
  2024-09-06 13:58           ` Morten Brørup
  0 siblings, 1 reply; 16+ messages in thread
From: Burakov, Anatoly @ 2024-09-06 13:17 UTC (permalink / raw)
  To: Bruce Richardson, Morten Brørup; +Cc: dev

On 9/6/2024 3:07 PM, Bruce Richardson wrote:
> On Fri, Sep 06, 2024 at 03:02:53PM +0200, Morten Brørup wrote:
>>> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
>>> Sent: Friday, 6 September 2024 14.46
>>>
>>> On 9/6/2024 2:37 PM, Morten Brørup wrote:
>>>>> From: Anatoly Burakov [mailto:anatoly.burakov@intel.com]
>>>>> Sent: Friday, 6 September 2024 13.47
>>>>> To: dev@dpdk.org
>>>>> Subject: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
>>>>>
>>>>> While initially, DPDK has used the term "socket ID" to refer to physical
>>>>> package
>>>>> ID, the last time DPDK read "physical_package_id" for socket ID was ~9
>>> years
>>>>> ago, so it's been a while since we've actually switched over to using the
>>> term
>>>>> "socket" to mean "NUMA node".
>>>>>
>>>>> This wasn't a problem before, as most systems had one NUMA node per
>>> physical
>>>>> socket. However, in the last few years, more and more systems have multiple
>>>>> NUMA
>>>>> nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
>>>>> transition was pretty seamless, however now we're faced with a situation
>>> when
>>>>> most of our documentation still uses outdated terms, and our API is ripe
>>> with
>>>>> references to "sockets" when in actuality we mean "NUMA nodes". This could
>>> be
>>>>> a
>>>>> source of confusion.
>>>>>
>>>>> While completely renaming all of our API's would be a huge effort, will
>>> take a
>>>>> long time and arguably wouldn't even be worth the API breakages (given that
>>>>> this
>>>>> mismatch between terminology and reality is implicitly understood by most
>>>>> people
>>>>> working on DPDK, and so this isn't so much of a problem in practice), we
>>> can
>>>>> do
>>>>> some tweaks around the edges and at least document this unfortunate
>>> reality.
>>>>>
>>>>> This patchset suggests the following changes:
>>>>>
>>>>> - Update rte_socket/rte_lcore documentation to refer to NUMA nodes rather
>>> than
>>>>> sockets - Rename internal structures' fields to better reflect this
>>> intention
>>>>> -
>>>>> Rename --socket-mem/--socket-limit flags to refer to NUMA rather than
>>> sockets
>>>>> -
>>>>> Add internal API to get physical package ID [1]
>>>>>
>>>>> The documentation is updated to refer to new EAL flags, but is otherwise
>>> left
>>>>> untouched, and instead the entry in "glossary" is amended to indicate that
>>>>> when
>>>>> DPDK documentation refers to "sockets", it actually means "NUMA ID's". As
>>> next
>>>>> steps, we could rename all API parameters to refer to NUMA ID rather than
>>>>> socket
>>>>> ID - this would not break neither API nor ABI, and instead would be a
>>>>> documentation change in practice.
>>>>>
>>>>> [1] This could be used to group lcores by physical package, see e.g.
>>>>> discussion
>>>>>       under this patch:
>>>>> https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-
>>>>> vipin.varghese@amd.com/
>>>>
>>>> Thank you for cleaning this up, Anatoly.
>>>>
>>>> I would prefer to take one more step and also rename functions and
>>> parameters, e.g. rte_socket_id() -> rte_numa_id().
>>>>
>>>> For backwards compatibility, macros/functions with the old names can be
>>> added.
>>>>
>>>
>>> I don't think we can do such changes without deprecation notices, but
>>> it's a good candidate for next release.
>>
>> Perhaps we can keep ABI compatibility by adding wrapper functions with the old names/parameters, which simply call the same functions with the new names/parameters.
>>
>> The Devil is in the details, and I haven't looked deeply into this. So take with a grain of salt.
>>
>>>
>>> I have thought about including parameter renames in this patchset, but
>>> for now I decided against doing so. I can certainly include this in the
>>> next revision if that's something community is willing to accept.
>>
>> I agree with your decision on this. Renaming the parameters without renaming the functions could be confusing.
>>
> 
> I actually wonder if that is true. If we are simply renaming the parameters
> without:
> a) changing their types
> b) changing the function behaviour
> then it is neither an API nor an ABI break. If we were to do so, it would
> be like changing a comment, since the actual parameter name is purely a
> convenience to hint to the user what the value being passed actually does.
> 
> That only applies for function parameters though. For any defines or macros
> that need renaming, then we are into API break territory and we would want
> backward compatible versions of same.
> 

To be clear, I was referring to the former rather than the latter; 
renaming public API function parameters/structure fields can be done 
relatively easily and won't break anything. If there is consensus on 
going further than I have with this patchset, I can certainly do so.

-- 
Thanks,
Anatoly


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

* RE: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
  2024-09-06 13:17         ` Burakov, Anatoly
@ 2024-09-06 13:58           ` Morten Brørup
  0 siblings, 0 replies; 16+ messages in thread
From: Morten Brørup @ 2024-09-06 13:58 UTC (permalink / raw)
  To: Burakov, Anatoly, Bruce Richardson; +Cc: dev

> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> Sent: Friday, 6 September 2024 15.18
> Subject: Re: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
> 
> On 9/6/2024 3:07 PM, Bruce Richardson wrote:
> > On Fri, Sep 06, 2024 at 03:02:53PM +0200, Morten Brørup wrote:
> >>> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> >>> Sent: Friday, 6 September 2024 14.46
> >>>
> >>> On 9/6/2024 2:37 PM, Morten Brørup wrote:
> >>>>> From: Anatoly Burakov [mailto:anatoly.burakov@intel.com]
> >>>>> Sent: Friday, 6 September 2024 13.47
> >>>>> To: dev@dpdk.org
> >>>>> Subject: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in
> DPDK
> >>>>>
> >>>>> While initially, DPDK has used the term "socket ID" to refer to physical
> >>>>> package
> >>>>> ID, the last time DPDK read "physical_package_id" for socket ID was ~9
> >>> years
> >>>>> ago, so it's been a while since we've actually switched over to using
> the
> >>> term
> >>>>> "socket" to mean "NUMA node".
> >>>>>
> >>>>> This wasn't a problem before, as most systems had one NUMA node per
> >>> physical
> >>>>> socket. However, in the last few years, more and more systems have
> multiple
> >>>>> NUMA
> >>>>> nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
> >>>>> transition was pretty seamless, however now we're faced with a situation
> >>> when
> >>>>> most of our documentation still uses outdated terms, and our API is ripe
> >>> with
> >>>>> references to "sockets" when in actuality we mean "NUMA nodes". This
> could
> >>> be
> >>>>> a
> >>>>> source of confusion.
> >>>>>
> >>>>> While completely renaming all of our API's would be a huge effort, will
> >>> take a
> >>>>> long time and arguably wouldn't even be worth the API breakages (given
> that
> >>>>> this
> >>>>> mismatch between terminology and reality is implicitly understood by
> most
> >>>>> people
> >>>>> working on DPDK, and so this isn't so much of a problem in practice), we
> >>> can
> >>>>> do
> >>>>> some tweaks around the edges and at least document this unfortunate
> >>> reality.
> >>>>>
> >>>>> This patchset suggests the following changes:
> >>>>>
> >>>>> - Update rte_socket/rte_lcore documentation to refer to NUMA nodes
> rather
> >>> than
> >>>>> sockets - Rename internal structures' fields to better reflect this
> >>> intention
> >>>>> -
> >>>>> Rename --socket-mem/--socket-limit flags to refer to NUMA rather than
> >>> sockets
> >>>>> -
> >>>>> Add internal API to get physical package ID [1]
> >>>>>
> >>>>> The documentation is updated to refer to new EAL flags, but is otherwise
> >>> left
> >>>>> untouched, and instead the entry in "glossary" is amended to indicate
> that
> >>>>> when
> >>>>> DPDK documentation refers to "sockets", it actually means "NUMA ID's".
> As
> >>> next
> >>>>> steps, we could rename all API parameters to refer to NUMA ID rather
> than
> >>>>> socket
> >>>>> ID - this would not break neither API nor ABI, and instead would be a
> >>>>> documentation change in practice.
> >>>>>
> >>>>> [1] This could be used to group lcores by physical package, see e.g.
> >>>>> discussion
> >>>>>       under this patch:
> >>>>> https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-
> >>>>> vipin.varghese@amd.com/
> >>>>
> >>>> Thank you for cleaning this up, Anatoly.
> >>>>
> >>>> I would prefer to take one more step and also rename functions and
> >>> parameters, e.g. rte_socket_id() -> rte_numa_id().
> >>>>
> >>>> For backwards compatibility, macros/functions with the old names can be
> >>> added.
> >>>>
> >>>
> >>> I don't think we can do such changes without deprecation notices, but
> >>> it's a good candidate for next release.
> >>
> >> Perhaps we can keep ABI compatibility by adding wrapper functions with the
> old names/parameters, which simply call the same functions with the new
> names/parameters.
> >>
> >> The Devil is in the details, and I haven't looked deeply into this. So take
> with a grain of salt.
> >>
> >>>
> >>> I have thought about including parameter renames in this patchset, but
> >>> for now I decided against doing so. I can certainly include this in the
> >>> next revision if that's something community is willing to accept.
> >>
> >> I agree with your decision on this. Renaming the parameters without
> renaming the functions could be confusing.
> >>
> >
> > I actually wonder if that is true. If we are simply renaming the parameters
> > without:
> > a) changing their types
> > b) changing the function behaviour
> > then it is neither an API nor an ABI break. If we were to do so, it would
> > be like changing a comment, since the actual parameter name is purely a
> > convenience to hint to the user what the value being passed actually does.
> >
> > That only applies for function parameters though. For any defines or macros
> > that need renaming, then we are into API break territory and we would want
> > backward compatible versions of same.
> >
> 
> To be clear, I was referring to the former rather than the latter;
> renaming public API function parameters/structure fields can be done
> relatively easily and won't break anything.

Agree.
By "confusing" I was referring to source code readability; e.g. an application calls some rte_socket_id() and passes the returned value on as a numa_id parameter to some other rte_socket_xyz() function.

I have no strong opinion on this.

But I would prefer going all the way, and also fix the API, i.e. rename the functions etc.
And I'm hoping ABI/API backwards compatibility can be preserved by adding wrappers with the previous names.

> If there is consensus on
> going further than I have with this patchset, I can certainly do so.



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

* Re: [RFC PATCH v1 4/5] eal: rename --socket-mem/--socket-limit
  2024-09-06 11:47 ` [RFC PATCH v1 4/5] eal: rename --socket-mem/--socket-limit Anatoly Burakov
@ 2024-09-09  7:42   ` fengchengwen
  0 siblings, 0 replies; 16+ messages in thread
From: fengchengwen @ 2024-09-09  7:42 UTC (permalink / raw)
  To: Anatoly Burakov, dev, Matan Azrad, Viacheslav Ovsiienko,
	Dariusz Sosnowski, Bing Zhao, Ori Kam, Suanming Mou,
	Tyler Retzlaff, Nicolas Chautru, Radu Nicolau, Akhil Goyal,
	Maxime Coquelin, Chenbo Xia

On 2024/9/6 19:47, Anatoly Burakov wrote:
> Currently, --socket-mem and --socket-limit EAL flags effectively refer to
> NUMA nodes, not CPU sockets. Update the flag names to reflect this. Old
> flag names are still supported for backward compatibility.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>     Technically, this is a user-facing change and so would require a
>     deprecation notice. We can do it the other way around, and add
>     support for --numa-mem/--numa-limit but do not expose it in
>     documentation yet, and instead add a deprecation notice for next
>     release. However, since old flags are kept for compatibility,
>     nothing will break as a result of merging this series even if we
>     didn't announce this change in advance. I'm open to feedback on
>     how to best do this change.
> 

...

> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index cf174aeaa3..591028f641 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -443,8 +443,8 @@ eal_usage(const char *prgname)
>  	printf("\nUsage: %s ", prgname);
>  	eal_common_usage();
>  	printf("EAL Linux options:\n"
> -	       "  --"OPT_SOCKET_MEM"        Memory to allocate on sockets (comma separated values)\n"
> -	       "  --"OPT_SOCKET_LIMIT"      Limit memory allocation on sockets (comma separated values)\n"
> +	       "  --"OPT_NUMA_MEM"        Memory to allocate on NUMA nodes (comma separated values)\n"
> +	       "  --"OPT_NUMA_LIMIT"      Limit memory allocation on NUMA nodes (comma separated values)\n"
>  	       "  --"OPT_HUGE_DIR"          Directory where hugetlbfs is mounted\n"
>  	       "  --"OPT_FILE_PREFIX"       Prefix for hugepage filenames\n"
>  	       "  --"OPT_CREATE_UIO_DEV"    Create /dev/uioX (usually done by hotplug)\n"
> @@ -693,11 +693,11 @@ eal_parse_args(int argc, char **argv)
>  			}
>  			break;
>  		}
> -		case OPT_SOCKET_MEM_NUM:
> +		case OPT_NUMA_MEM_NUM:
>  			if (eal_parse_socket_arg(optarg,
>  					internal_conf->numa_mem) < 0) {
>  				EAL_LOG(ERR, "invalid parameters for --"
> -						OPT_SOCKET_MEM);
> +						OPT_NUMA_MEM);

This may confuse user, how about "invalid parameters for --numa-mem (aka --socket-mem)"

>  				eal_usage(prgname);
>  				ret = -1;
>  				goto out;
> @@ -705,11 +705,11 @@ eal_parse_args(int argc, char **argv)
>  			internal_conf->force_numa = 1;
>  			break;
>  
> -		case OPT_SOCKET_LIMIT_NUM:
> +		case OPT_NUMA_LIMIT_NUM:
>  			if (eal_parse_socket_arg(optarg,
>  					internal_conf->numa_limit) < 0) {
>  				EAL_LOG(ERR, "invalid parameters for --"
> -						OPT_SOCKET_LIMIT);
> +						OPT_NUMA_LIMIT);

Same to above

>  				eal_usage(prgname);
>  				ret = -1;
>  				goto out;
> 

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

* Re: [RFC PATCH v1 5/5] lcore: store physical package ID internally
  2024-09-06 11:47 ` [RFC PATCH v1 5/5] lcore: store physical package ID internally Anatoly Burakov
@ 2024-09-09  7:49   ` fengchengwen
  2024-10-14 17:31   ` Stephen Hemminger
  1 sibling, 0 replies; 16+ messages in thread
From: fengchengwen @ 2024-09-09  7:49 UTC (permalink / raw)
  To: Anatoly Burakov, dev, Tyler Retzlaff, Bruce Richardson,
	Dmitry Kozlyuk, Pallavi Kadam

It seemed there is no usage scenario of the new field, suggest add it when we indeed need it.

On 2024/9/6 19:47, Anatoly Burakov wrote:
> This patch introduces a new field in the lcore structure that stores the
> physical package ID of the core. This field is populated during EAL init.
> It is not exposed through any external API's for now.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

...

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

* Re: [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK
  2024-09-06 11:47 [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Anatoly Burakov
                   ` (5 preceding siblings ...)
  2024-09-06 12:37 ` [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Morten Brørup
@ 2024-09-09  7:51 ` fengchengwen
  6 siblings, 0 replies; 16+ messages in thread
From: fengchengwen @ 2024-09-09  7:51 UTC (permalink / raw)
  To: Anatoly Burakov, dev

for commit 1~4,
Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2024/9/6 19:47, Anatoly Burakov wrote:
> While initially, DPDK has used the term "socket ID" to refer to physical package
> ID, the last time DPDK read "physical_package_id" for socket ID was ~9 years
> ago, so it's been a while since we've actually switched over to using the term
> "socket" to mean "NUMA node".
> 
> This wasn't a problem before, as most systems had one NUMA node per physical
> socket. However, in the last few years, more and more systems have multiple NUMA
> nodes per physical CPU socket. Since DPDK used NUMA nodes already, the
> transition was pretty seamless, however now we're faced with a situation when
> most of our documentation still uses outdated terms, and our API is ripe with
> references to "sockets" when in actuality we mean "NUMA nodes". This could be a
> source of confusion.
> 
> While completely renaming all of our API's would be a huge effort, will take a
> long time and arguably wouldn't even be worth the API breakages (given that this
> mismatch between terminology and reality is implicitly understood by most people
> working on DPDK, and so this isn't so much of a problem in practice), we can do
> some tweaks around the edges and at least document this unfortunate reality.
> 
> This patchset suggests the following changes:
> 
> - Update rte_socket/rte_lcore documentation to refer to NUMA nodes rather than
> sockets - Rename internal structures' fields to better reflect this intention -
> Rename --socket-mem/--socket-limit flags to refer to NUMA rather than sockets -
> Add internal API to get physical package ID [1]
> 
> The documentation is updated to refer to new EAL flags, but is otherwise left
> untouched, and instead the entry in "glossary" is amended to indicate that when
> DPDK documentation refers to "sockets", it actually means "NUMA ID's". As next
> steps, we could rename all API parameters to refer to NUMA ID rather than socket
> ID - this would not break neither API nor ABI, and instead would be a
> documentation change in practice.
> 
> [1] This could be used to group lcores by physical package, see e.g. discussion
>     under this patch: https://patches.dpdk.org/project/dpdk/cover/20240827151014.201-1-vipin.varghese@amd.com/
> 
> Anatoly Burakov (5):
>   eal: update socket ID API documentation
>   lcore: rename socket ID to NUMA ID
>   eal: rename socket ID to NUMA ID in internal config
>   eal: rename --socket-mem/--socket-limit
>   lcore: store physical package ID internally
> 
>  doc/guides/faq/faq.rst                        |  4 +--
>  doc/guides/howto/lm_bond_virtio_sriov.rst     |  2 +-
>  doc/guides/howto/lm_virtio_vhost_user.rst     |  2 +-
>  doc/guides/howto/pvp_reference_benchmark.rst  |  4 +--
>  .../virtio_user_for_container_networking.rst  |  2 +-
>  doc/guides/linux_gsg/build_sample_apps.rst    | 20 +++++------
>  doc/guides/linux_gsg/linux_eal_parameters.rst | 16 ++++-----
>  doc/guides/nics/mlx4.rst                      |  2 +-
>  doc/guides/nics/mlx5.rst                      |  2 +-
>  .../prog_guide/env_abstraction_layer.rst      | 12 +++----
>  doc/guides/prog_guide/glossary.rst            |  5 ++-
>  doc/guides/prog_guide/multi_proc_support.rst  |  2 +-
>  doc/guides/sample_app_ug/bbdev_app.rst        |  6 ++--
>  doc/guides/sample_app_ug/ipsec_secgw.rst      |  6 ++--
>  doc/guides/sample_app_ug/vdpa.rst             |  2 +-
>  doc/guides/sample_app_ug/vhost.rst            |  4 +--
>  lib/eal/common/eal_common_dynmem.c            | 14 ++++----
>  lib/eal/common/eal_common_lcore.c             | 28 +++++++++++++---
>  lib/eal/common/eal_common_options.c           | 33 ++++++++++---------
>  lib/eal/common/eal_common_thread.c            | 12 +++----
>  lib/eal/common/eal_internal_cfg.h             | 10 +++---
>  lib/eal/common/eal_options.h                  |  8 +++--
>  lib/eal/common/eal_private.h                  |  5 ++-
>  lib/eal/common/eal_thread.h                   | 11 +++++++
>  lib/eal/common/malloc_heap.c                  |  2 +-
>  lib/eal/freebsd/eal.c                         |  2 +-
>  lib/eal/freebsd/eal_lcore.c                   |  6 ++++
>  lib/eal/include/rte_lcore.h                   | 25 +++++++-------
>  lib/eal/linux/eal.c                           | 22 ++++++-------
>  lib/eal/linux/eal_lcore.c                     | 28 ++++++++++++++++
>  lib/eal/linux/eal_memory.c                    | 22 ++++++-------
>  lib/eal/windows/eal.c                         |  2 +-
>  lib/eal/windows/eal_lcore.c                   |  7 ++++
>  33 files changed, 204 insertions(+), 124 deletions(-)
> 

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

* Re: [RFC PATCH v1 5/5] lcore: store physical package ID internally
  2024-09-06 11:47 ` [RFC PATCH v1 5/5] lcore: store physical package ID internally Anatoly Burakov
  2024-09-09  7:49   ` fengchengwen
@ 2024-10-14 17:31   ` Stephen Hemminger
  1 sibling, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-14 17:31 UTC (permalink / raw)
  To: Anatoly Burakov
  Cc: dev, Tyler Retzlaff, Bruce Richardson, Dmitry Kozlyuk, Pallavi Kadam

On Fri,  6 Sep 2024 12:47:31 +0100
Anatoly Burakov <anatoly.burakov@intel.com> wrote:

> +/*
> + * Get CPU package ID for a logical core.
> + *
> + * This searches each nodeX directories in /sys for the symlink for the given
> + * lcore_id and returns the numa node where the lcore is found. If lcore is not
> + * found on any numa node, returns zero.
> + */
> +unsigned
> +eal_cpu_package_id(unsigned lcore_id)
> +{
> +	char path[PATH_MAX];
> +	unsigned long id;
> +
> +	int len = snprintf(path, sizeof(path),
> +			SYS_CPU_DIR "/%s", lcore_id, PACKAGE_ID_FILE);
> +	if (len <= 0 || (unsigned)len >= sizeof(path))
> +		goto err;
> +	if (eal_parse_sysfs_value(path, &id) != 0)
> +		goto err;
> +	return (unsigned)id;
> +
> +err:
> +	EAL_LOG(ERR, "Error reading package id value from %s "
> +			"for lcore %u - assuming package 0", SYS_CPU_DIR, lcore_id);
> +	return 0;
> +}

This is wrong. Lcore Id is not always the same as the CPU number in the system.

Also, it is not clear while package != numa node is something we want DPDK
to start supporting. Adding yet another layer adds complexity.

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

end of thread, other threads:[~2024-10-14 17:31 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-06 11:47 [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Anatoly Burakov
2024-09-06 11:47 ` [RFC PATCH v1 1/5] eal: update socket ID API documentation Anatoly Burakov
2024-09-06 11:47 ` [RFC PATCH v1 2/5] lcore: rename socket ID to NUMA ID Anatoly Burakov
2024-09-06 11:47 ` [RFC PATCH v1 3/5] eal: rename socket ID to NUMA ID in internal config Anatoly Burakov
2024-09-06 11:47 ` [RFC PATCH v1 4/5] eal: rename --socket-mem/--socket-limit Anatoly Burakov
2024-09-09  7:42   ` fengchengwen
2024-09-06 11:47 ` [RFC PATCH v1 5/5] lcore: store physical package ID internally Anatoly Burakov
2024-09-09  7:49   ` fengchengwen
2024-10-14 17:31   ` Stephen Hemminger
2024-09-06 12:37 ` [RFC PATCH v1 0/5] Adjust wording for NUMA vs. socket ID in DPDK Morten Brørup
2024-09-06 12:46   ` Burakov, Anatoly
2024-09-06 13:02     ` Morten Brørup
2024-09-06 13:07       ` Bruce Richardson
2024-09-06 13:17         ` Burakov, Anatoly
2024-09-06 13:58           ` Morten Brørup
2024-09-09  7:51 ` fengchengwen

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).