[dpdk-dev,v6] eal: provide API for querying valid socket id's

Message ID 9ff09ae09b3e9e1b149f73a693b552f8c94a01a2.1521722141.git.anatoly.burakov@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Anatoly Burakov March 22, 2018, 12:36 p.m. UTC
  During lcore scan, find all socket ID's and store them, and
provide public API to query valid socket id's. This will break
the ABI, so bump ABI version.

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

Notes:
    v6:
    - Fixed meson ABI version header
    
    v5:
    - Move API to experimental
    - Store list of valid socket id's instead of simply
      recording the biggest one
    
    v4:
    - Remove backwards ABI compatibility, bump ABI instead
    
    v3:
    - Added ABI compatibility
    
    v2:
    - checkpatch changes
    - check socket before deciding if the core is not to be used

 lib/librte_eal/bsdapp/eal/Makefile        |  2 +-
 lib/librte_eal/common/eal_common_lcore.c  | 75 ++++++++++++++++++++++++++-----
 lib/librte_eal/common/include/rte_eal.h   |  3 ++
 lib/librte_eal/common/include/rte_lcore.h | 30 +++++++++++++
 lib/librte_eal/linuxapp/eal/Makefile      |  2 +-
 lib/librte_eal/meson.build                |  2 +-
 lib/librte_eal/rte_eal_version.map        |  2 +
 7 files changed, 101 insertions(+), 15 deletions(-)
  

Comments

Gowrishankar March 22, 2018, 5:07 p.m. UTC | #1
On Thursday 22 March 2018 06:06 PM, Anatoly Burakov wrote:
> During lcore scan, find all socket ID's and store them, and
> provide public API to query valid socket id's. This will break
> the ABI, so bump ABI version.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>

Thanks,
Gowrishankar
  
Thomas Monjalon March 27, 2018, 4:24 p.m. UTC | #2
22/03/2018 13:36, Anatoly Burakov:
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -57,6 +57,9 @@ enum rte_proc_type_t {
>  struct rte_config {
>  	uint32_t master_lcore;       /**< Id of the master lcore */
>  	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. */

Please keep this comment on the same line if it's below 99 chars.


> --- a/lib/librte_eal/common/include/rte_lcore.h
> +++ b/lib/librte_eal/common/include/rte_lcore.h
> @@ -132,6 +132,36 @@ rte_lcore_index(int lcore_id)
>  unsigned rte_socket_id(void);
>  
>  /**
> + * Return number of physical sockets 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
> + * may be 0 and 8.
> + *
> + * @return
> + *   the number of physical sockets as recognized by EAL
> + */
> +unsigned int __rte_experimental
> +rte_num_socket_ids(void);

I suggest rte_socket_count() as function name.


> +/**
> + * 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.
> + *
> + * @param idx
> + *   index of physical socket id to return
> + *
> + * @return
> + *   - physical socket id as recognized by EAL
> + *   - -1 on error, with errno set to EINVAL
> + */
> +int __rte_experimental
> +rte_socket_id_by_idx(unsigned int idx);

OK for this function.


> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> -LIBABIVER := 6
> +LIBABIVER := 7

When changing the ABI version, you need to update the release notes.

There is also a deprecation notice to remove.


> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -229,6 +229,8 @@ EXPERIMENTAL {
>  	rte_mp_request;
>  	rte_mp_request_async;
>  	rte_mp_reply;
> +	rte_num_socket_ids;
> +	rte_socket_id_by_idx;

This one is not in the alphabetical order.

>  	rte_service_attr_get;
>  	rte_service_attr_reset_all;
>  	rte_service_component_register;
  
Anatoly Burakov March 31, 2018, 1:35 p.m. UTC | #3
On 27-Mar-18 5:24 PM, Thomas Monjalon wrote:
> 22/03/2018 13:36, Anatoly Burakov:
>> --- a/lib/librte_eal/common/include/rte_eal.h
>> +++ b/lib/librte_eal/common/include/rte_eal.h
>> @@ -57,6 +57,9 @@ enum rte_proc_type_t {
>>   struct rte_config {
>>   	uint32_t master_lcore;       /**< Id of the master lcore */
>>   	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. */
> 
> Please keep this comment on the same line if it's below 99 chars.

If this is allowed, can we fix checkpatch to not report error in these 
cases?
  
Thomas Monjalon April 2, 2018, 3:27 p.m. UTC | #4
31/03/2018 15:35, Burakov, Anatoly:
> On 27-Mar-18 5:24 PM, Thomas Monjalon wrote:
> > 22/03/2018 13:36, Anatoly Burakov:
> >> --- a/lib/librte_eal/common/include/rte_eal.h
> >> +++ b/lib/librte_eal/common/include/rte_eal.h
> >> @@ -57,6 +57,9 @@ enum rte_proc_type_t {
> >>   struct rte_config {
> >>   	uint32_t master_lcore;       /**< Id of the master lcore */
> >>   	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. */
> > 
> > Please keep this comment on the same line if it's below 99 chars.
> 
> If this is allowed, can we fix checkpatch to not report error in these 
> cases?

Checkpatch is just a tool, not an authority.
In this case, given all other lines are formatted with comments
at the end of lines, it is better to do the same if the line size
is reasonnable.
  

Patch

diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index dd455e6..ed1d17b 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -21,7 +21,7 @@  LDLIBS += -lgcc_s
 
 EXPORT_MAP := ../../rte_eal_version.map
 
-LIBABIVER := 6
+LIBABIVER := 7
 
 # specific to bsdapp exec-env
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) := eal.c
diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 7724fa4..50d9f82 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -7,6 +7,7 @@ 
 #include <string.h>
 #include <dirent.h>
 
+#include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_eal.h>
 #include <rte_lcore.h>
@@ -16,6 +17,19 @@ 
 #include "eal_private.h"
 #include "eal_thread.h"
 
+static int
+socket_id_cmp(const void *a, const void *b)
+{
+	const int *lcore_id_a = a;
+	const int *lcore_id_b = b;
+
+	if (*lcore_id_a < *lcore_id_b)
+		return -1;
+	if (*lcore_id_a > *lcore_id_b)
+		return 1;
+	return 0;
+}
+
 /*
  * Parse /sys/devices/system/cpu to get the number of physical and logical
  * processors on the machine. The function will fill the cpu_info
@@ -28,6 +42,8 @@  rte_eal_cpu_init(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	unsigned lcore_id;
 	unsigned count = 0;
+	unsigned int socket_id, prev_socket_id;
+	int lcore_to_socket_id[RTE_MAX_LCORE];
 
 	/*
 	 * Parse the maximum set of logical cores, detect the subset of running
@@ -39,6 +55,19 @@  rte_eal_cpu_init(void)
 		/* init cpuset for per lcore config */
 		CPU_ZERO(&lcore_config[lcore_id].cpuset);
 
+		/* find socket first */
+		socket_id = eal_cpu_socket_id(lcore_id);
+		if (socket_id >= RTE_MAX_NUMA_NODES) {
+#ifdef RTE_EAL_ALLOW_INV_SOCKET_ID
+			socket_id = 0;
+#else
+			RTE_LOG(ERR, EAL, "Socket ID (%u) is greater than RTE_MAX_NUMA_NODES (%d)\n",
+					socket_id, RTE_MAX_NUMA_NODES);
+			return -1;
+#endif
+		}
+		lcore_to_socket_id[lcore_id] = socket_id;
+
 		/* in 1:1 mapping, record related cpu detected state */
 		lcore_config[lcore_id].detected = eal_cpu_detected(lcore_id);
 		if (lcore_config[lcore_id].detected == 0) {
@@ -54,18 +83,7 @@  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 = eal_cpu_socket_id(lcore_id);
-		if (lcore_config[lcore_id].socket_id >= RTE_MAX_NUMA_NODES) {
-#ifdef RTE_EAL_ALLOW_INV_SOCKET_ID
-			lcore_config[lcore_id].socket_id = 0;
-#else
-			RTE_LOG(ERR, EAL, "Socket ID (%u) is greater than "
-				"RTE_MAX_NUMA_NODES (%d)\n",
-				lcore_config[lcore_id].socket_id,
-				RTE_MAX_NUMA_NODES);
-			return -1;
-#endif
-		}
+		lcore_config[lcore_id].socket_id = socket_id;
 		RTE_LOG(DEBUG, EAL, "Detected lcore %u as "
 				"core %u on socket %u\n",
 				lcore_id, lcore_config[lcore_id].core_id,
@@ -79,5 +97,38 @@  rte_eal_cpu_init(void)
 		RTE_MAX_LCORE);
 	RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
 
+	/* sort all socket id's in ascending order */
+	qsort(lcore_to_socket_id, RTE_DIM(lcore_to_socket_id),
+			sizeof(lcore_to_socket_id[0]), socket_id_cmp);
+
+	prev_socket_id = -1;
+	config->numa_node_count = 0;
+	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+		socket_id = lcore_to_socket_id[lcore_id];
+		if (socket_id != prev_socket_id)
+			config->numa_nodes[config->numa_node_count++] =
+					socket_id;
+		prev_socket_id = socket_id;
+	}
+	RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", config->numa_node_count);
+
 	return 0;
 }
+
+unsigned int __rte_experimental
+rte_num_socket_ids(void)
+{
+	const struct rte_config *config = rte_eal_get_configuration();
+	return config->numa_node_count;
+}
+
+int __rte_experimental
+rte_socket_id_by_idx(unsigned int idx)
+{
+	const struct rte_config *config = rte_eal_get_configuration();
+	if (idx >= config->numa_node_count) {
+		rte_errno = EINVAL;
+		return -1;
+	}
+	return config->numa_nodes[idx];
+}
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 93ca4cc..6109472 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -57,6 +57,9 @@  enum rte_proc_type_t {
 struct rte_config {
 	uint32_t master_lcore;       /**< Id of the master lcore */
 	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 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/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 0472220..c6511a9 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -132,6 +132,36 @@  rte_lcore_index(int lcore_id)
 unsigned rte_socket_id(void);
 
 /**
+ * Return number of physical sockets 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
+ * may be 0 and 8.
+ *
+ * @return
+ *   the number of physical sockets as recognized by EAL
+ */
+unsigned int __rte_experimental
+rte_num_socket_ids(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.
+ *
+ * @param idx
+ *   index of physical socket id to return
+ *
+ * @return
+ *   - physical socket id as recognized by EAL
+ *   - -1 on error, with errno set to EINVAL
+ */
+int __rte_experimental
+rte_socket_id_by_idx(unsigned int idx);
+
+/**
  * Get the ID of the physical socket of the specified lcore
  *
  * @param lcore_id
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 7e5bbe8..b9c7727 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -10,7 +10,7 @@  ARCH_DIR ?= $(RTE_ARCH)
 EXPORT_MAP := ../../rte_eal_version.map
 VPATH += $(RTE_SDK)/lib/librte_eal/common/arch/$(ARCH_DIR)
 
-LIBABIVER := 6
+LIBABIVER := 7
 
 VPATH += $(RTE_SDK)/lib/librte_eal/common
 
diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
index d9ba385..8f0ce1b 100644
--- a/lib/librte_eal/meson.build
+++ b/lib/librte_eal/meson.build
@@ -43,7 +43,7 @@  else
 	error('unsupported system type @0@'.format(hostmachine.system()))
 endif
 
-version = 6  # the version of the EAL API
+version = 7  # the version of the EAL API
 allow_experimental_apis = true
 deps += 'compat'
 cflags += '-D_GNU_SOURCE'
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 1d88437..6a4c355 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -229,6 +229,8 @@  EXPERIMENTAL {
 	rte_mp_request;
 	rte_mp_request_async;
 	rte_mp_reply;
+	rte_num_socket_ids;
+	rte_socket_id_by_idx;
 	rte_service_attr_get;
 	rte_service_attr_reset_all;
 	rte_service_component_register;