DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eal: add function to return number of detected sockets
@ 2017-12-22 11:58 Anatoly Burakov
  2017-12-22 12:41 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
  0 siblings, 1 reply; 42+ messages in thread
From: Anatoly Burakov @ 2017-12-22 11:58 UTC (permalink / raw)
  To: dev

During lcore scan, find maximum socket ID and store it.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_lcore.c  | 12 ++++++++++++
 lib/librte_eal/common/include/rte_eal.h   |  1 +
 lib/librte_eal/common/include/rte_lcore.h |  8 ++++++++
 lib/librte_eal/rte_eal_version.map        |  6 ++++++
 4 files changed, 27 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 0db1555..546c802 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -57,6 +57,7 @@ rte_eal_cpu_init(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	unsigned lcore_id;
 	unsigned count = 0;
+	unsigned max_socket_id = 0;
 
 	/*
 	 * Parse the maximum set of logical cores, detect the subset of running
@@ -100,6 +101,8 @@ rte_eal_cpu_init(void)
 				lcore_id, lcore_config[lcore_id].core_id,
 				lcore_config[lcore_id].socket_id);
 		count++;
+		max_socket_id = RTE_MAX(max_socket_id,
+					lcore_config[lcore_id].socket_id);
 	}
 	/* Set the count of enabled logical cores of the EAL configuration */
 	config->lcore_count = count;
@@ -108,5 +111,14 @@ rte_eal_cpu_init(void)
 		RTE_MAX_LCORE);
 	RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
 
+	config->numa_node_count = max_socket_id + 1;
+	RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", config->numa_node_count);
+
 	return 0;
 }
+
+unsigned rte_num_sockets(void)
+{
+	const struct rte_config *config = rte_eal_get_configuration();
+	return config->numa_node_count;
+}
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 8e4e71c..5b12914 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -83,6 +83,7 @@ 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 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 c89e6ba..6a75c9b 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -148,6 +148,14 @@ rte_lcore_index(int lcore_id)
 unsigned rte_socket_id(void);
 
 /**
+ * Return number of physical sockets on the system.
+ * @return
+ *   the number of physical sockets as recognized by EAL
+ *
+ */
+unsigned rte_num_sockets(void);
+
+/**
  * Get the ID of the physical socket of the specified lcore
  *
  * @param lcore_id
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index f4f46c1..e086c6e 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -200,6 +200,12 @@ DPDK_17.11 {
 
 } DPDK_17.08;
 
+DPDK_18.02 {
+	global:
+
+	rte_num_sockets;
+} DPDK_17.11;
+
 EXPERIMENTAL {
 	global:
 
-- 
2.7.4

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

* [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2017-12-22 11:58 [dpdk-dev] [PATCH] eal: add function to return number of detected sockets Anatoly Burakov
@ 2017-12-22 12:41 ` Anatoly Burakov
  2018-01-11 22:20   ` Thomas Monjalon
                     ` (2 more replies)
  0 siblings, 3 replies; 42+ messages in thread
From: Anatoly Burakov @ 2017-12-22 12:41 UTC (permalink / raw)
  To: dev

During lcore scan, find maximum socket ID and store it.

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

Notes:
    v2:
    - checkpatch changes
    - check socket before deciding if the core is not to be used

 lib/librte_eal/common/eal_common_lcore.c  | 37 +++++++++++++++++++++----------
 lib/librte_eal/common/include/rte_eal.h   |  1 +
 lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
 lib/librte_eal/rte_eal_version.map        |  6 +++++
 4 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 0db1555..c9729a0 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -57,6 +57,7 @@ rte_eal_cpu_init(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	unsigned lcore_id;
 	unsigned count = 0;
+	unsigned int socket_id, max_socket_id = 0;
 
 	/*
 	 * Parse the maximum set of logical cores, detect the subset of running
@@ -68,6 +69,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
+		}
+		max_socket_id = RTE_MAX(max_socket_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) {
@@ -83,18 +97,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,
@@ -108,5 +111,15 @@ rte_eal_cpu_init(void)
 		RTE_MAX_LCORE);
 	RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
 
+	config->numa_node_count = max_socket_id + 1;
+	RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", config->numa_node_count);
+
 	return 0;
 }
+
+unsigned int
+rte_num_sockets(void)
+{
+	const struct rte_config *config = rte_eal_get_configuration();
+	return config->numa_node_count;
+}
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 8e4e71c..5b12914 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -83,6 +83,7 @@ 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 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 c89e6ba..7c72c9e 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -148,6 +148,14 @@ rte_lcore_index(int lcore_id)
 unsigned rte_socket_id(void);
 
 /**
+ * Return number of physical sockets on the system.
+ * @return
+ *   the number of physical sockets as recognized by EAL
+ *
+ */
+unsigned int rte_num_sockets(void);
+
+/**
  * Get the ID of the physical socket of the specified lcore
  *
  * @param lcore_id
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index f4f46c1..e086c6e 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -200,6 +200,12 @@ DPDK_17.11 {
 
 } DPDK_17.08;
 
+DPDK_18.02 {
+	global:
+
+	rte_num_sockets;
+} DPDK_17.11;
+
 EXPERIMENTAL {
 	global:
 
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2017-12-22 12:41 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
@ 2018-01-11 22:20   ` Thomas Monjalon
  2018-01-12 11:44     ` Burakov, Anatoly
  2018-01-16 17:53   ` [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal Anatoly Burakov
       [not found]   ` <cover.1517848624.git.anatoly.burakov@intel.com>
  2 siblings, 1 reply; 42+ messages in thread
From: Thomas Monjalon @ 2018-01-11 22:20 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev

22/12/2017 13:41, Anatoly Burakov:
> During lcore scan, find maximum socket ID and store it.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
>  	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */

isn't it breaking the ABI?

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2018-01-11 22:20   ` Thomas Monjalon
@ 2018-01-12 11:44     ` Burakov, Anatoly
  2018-01-12 11:50       ` Thomas Monjalon
  0 siblings, 1 reply; 42+ messages in thread
From: Burakov, Anatoly @ 2018-01-12 11:44 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> 22/12/2017 13:41, Anatoly Burakov:
>> During lcore scan, find maximum socket ID and store it.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>> --- a/lib/librte_eal/common/include/rte_eal.h
>> +++ b/lib/librte_eal/common/include/rte_eal.h
>> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
>>   	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> 
> isn't it breaking the ABI?
> 
> 

Yep, you're right, forgot to add that. I didn't expect this to get 
merged in 18.02 anyway, so v2 will follow.

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2018-01-12 11:44     ` Burakov, Anatoly
@ 2018-01-12 11:50       ` Thomas Monjalon
  2018-01-16 11:56         ` Burakov, Anatoly
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Monjalon @ 2018-01-12 11:50 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

12/01/2018 12:44, Burakov, Anatoly:
> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> > 22/12/2017 13:41, Anatoly Burakov:
> >> During lcore scan, find maximum socket ID and store it.
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> ---
> >> --- a/lib/librte_eal/common/include/rte_eal.h
> >> +++ b/lib/librte_eal/common/include/rte_eal.h
> >> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
> >>   	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> > 
> > isn't it breaking the ABI?
> > 
> > 
> 
> Yep, you're right, forgot to add that. I didn't expect this to get 
> merged in 18.02 anyway, so v2 will follow.

Please write 18.05 in the subject to show your expectation.
Thanks

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2018-01-12 11:50       ` Thomas Monjalon
@ 2018-01-16 11:56         ` Burakov, Anatoly
  2018-01-16 12:20           ` Thomas Monjalon
  0 siblings, 1 reply; 42+ messages in thread
From: Burakov, Anatoly @ 2018-01-16 11:56 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> 12/01/2018 12:44, Burakov, Anatoly:
>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
>>> 22/12/2017 13:41, Anatoly Burakov:
>>>> During lcore scan, find maximum socket ID and store it.
>>>>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> ---
>>>> --- a/lib/librte_eal/common/include/rte_eal.h
>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
>>>> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
>>>>    	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
>>>
>>> isn't it breaking the ABI?
>>>
>>>
>>
>> Yep, you're right, forgot to add that. I didn't expect this to get
>> merged in 18.02 anyway, so v2 will follow.
> 
> Please write 18.05 in the subject to show your expectation.
> Thanks
> 

Does it have to be an ABI change though? We can put numa_node_count 
after pointer to mem_config, in which case it won't be an ABI break. 
Would that be better?

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2018-01-16 11:56         ` Burakov, Anatoly
@ 2018-01-16 12:20           ` Thomas Monjalon
  2018-01-16 15:05             ` Burakov, Anatoly
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Monjalon @ 2018-01-16 12:20 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

16/01/2018 12:56, Burakov, Anatoly:
> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> > 12/01/2018 12:44, Burakov, Anatoly:
> >> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> >>> 22/12/2017 13:41, Anatoly Burakov:
> >>>> During lcore scan, find maximum socket ID and store it.
> >>>>
> >>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>>> ---
> >>>> --- a/lib/librte_eal/common/include/rte_eal.h
> >>>> +++ b/lib/librte_eal/common/include/rte_eal.h
> >>>> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
> >>>>    	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> >>>
> >>> isn't it breaking the ABI?
> >>>
> >>>
> >>
> >> Yep, you're right, forgot to add that. I didn't expect this to get
> >> merged in 18.02 anyway, so v2 will follow.
> > 
> > Please write 18.05 in the subject to show your expectation.
> > Thanks
> > 
> 
> Does it have to be an ABI change though? We can put numa_node_count 
> after pointer to mem_config, in which case it won't be an ABI break. 
> Would that be better?

Changing the size of a struct which is allocated by the app,
is an ABI break.
Is your solution changing the size?

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2018-01-16 12:20           ` Thomas Monjalon
@ 2018-01-16 15:05             ` Burakov, Anatoly
  2018-01-16 17:34               ` Thomas Monjalon
  0 siblings, 1 reply; 42+ messages in thread
From: Burakov, Anatoly @ 2018-01-16 15:05 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 16-Jan-18 12:20 PM, Thomas Monjalon wrote:
> 16/01/2018 12:56, Burakov, Anatoly:
>> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
>>> 12/01/2018 12:44, Burakov, Anatoly:
>>>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
>>>>> 22/12/2017 13:41, Anatoly Burakov:
>>>>>> During lcore scan, find maximum socket ID and store it.
>>>>>>
>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>>>> ---
>>>>>> --- a/lib/librte_eal/common/include/rte_eal.h
>>>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
>>>>>> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
>>>>>>     	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
>>>>>
>>>>> isn't it breaking the ABI?
>>>>>
>>>>>
>>>>
>>>> Yep, you're right, forgot to add that. I didn't expect this to get
>>>> merged in 18.02 anyway, so v2 will follow.
>>>
>>> Please write 18.05 in the subject to show your expectation.
>>> Thanks
>>>
>>
>> Does it have to be an ABI change though? We can put numa_node_count
>> after pointer to mem_config, in which case it won't be an ABI break.
>> Would that be better?
> 
> Changing the size of a struct which is allocated by the app,
> is an ABI break.
> Is your solution changing the size?
> 

It's not really allocated as such. rte_config is a global static 
variable, and we only ever get pointers to it from the user code. If we 
add the new value at the end, all of the old data layout would be intact 
and work as before, so nothing would change as far as old code is concerned.

However, if that's still considered an ABI break, then OK, break it is.

Some background for why this is needed - for the memory hotplug, we need 
to know how many sockets we can allocate memory at, to distinguish 
between socket that doesn't exist, and socket that exists but has no 
memory allocated on it. I'm OK with trying other approaches (such as 
storing numa nodes in a static variable somewhere) if breaking ABI for 
this is too much to ask for such a minute change.

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2018-01-16 15:05             ` Burakov, Anatoly
@ 2018-01-16 17:34               ` Thomas Monjalon
  2018-01-16 17:38                 ` Burakov, Anatoly
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Monjalon @ 2018-01-16 17:34 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

16/01/2018 16:05, Burakov, Anatoly:
> On 16-Jan-18 12:20 PM, Thomas Monjalon wrote:
> > 16/01/2018 12:56, Burakov, Anatoly:
> >> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> >>> 12/01/2018 12:44, Burakov, Anatoly:
> >>>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> >>>>> 22/12/2017 13:41, Anatoly Burakov:
> >>>>>> During lcore scan, find maximum socket ID and store it.
> >>>>>>
> >>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>>>>> ---
> >>>>>> --- a/lib/librte_eal/common/include/rte_eal.h
> >>>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
> >>>>>> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
> >>>>>>     	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> >>>>>
> >>>>> isn't it breaking the ABI?
> >>>>>
> >>>>>
> >>>>
> >>>> Yep, you're right, forgot to add that. I didn't expect this to get
> >>>> merged in 18.02 anyway, so v2 will follow.
> >>>
> >>> Please write 18.05 in the subject to show your expectation.
> >>> Thanks
> >>>
> >>
> >> Does it have to be an ABI change though? We can put numa_node_count
> >> after pointer to mem_config, in which case it won't be an ABI break.
> >> Would that be better?
> > 
> > Changing the size of a struct which is allocated by the app,
> > is an ABI break.
> > Is your solution changing the size?
> > 
> 
> It's not really allocated as such. rte_config is a global static 
> variable, and we only ever get pointers to it from the user code. If we 
> add the new value at the end, all of the old data layout would be intact 
> and work as before, so nothing would change as far as old code is concerned.
> 
> However, if that's still considered an ABI break, then OK, break it is.

Maybe that assuming it is never allocated (not copied for instance)
we could consider it is not an ABI break.

> Some background for why this is needed - for the memory hotplug, we need 
> to know how many sockets we can allocate memory at, to distinguish 
> between socket that doesn't exist, and socket that exists but has no 
> memory allocated on it. I'm OK with trying other approaches (such as 
> storing numa nodes in a static variable somewhere) if breaking ABI for 
> this is too much to ask for such a minute change.

Why is it important for 18.02?
Memory hotplug will be integrated only in 18.05.
I think it is better to just wait (and announce the deprecation).

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2018-01-16 17:34               ` Thomas Monjalon
@ 2018-01-16 17:38                 ` Burakov, Anatoly
  2018-01-16 18:26                   ` Thomas Monjalon
  0 siblings, 1 reply; 42+ messages in thread
From: Burakov, Anatoly @ 2018-01-16 17:38 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 16-Jan-18 5:34 PM, Thomas Monjalon wrote:
> 16/01/2018 16:05, Burakov, Anatoly:
>> On 16-Jan-18 12:20 PM, Thomas Monjalon wrote:
>>> 16/01/2018 12:56, Burakov, Anatoly:
>>>> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
>>>>> 12/01/2018 12:44, Burakov, Anatoly:
>>>>>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
>>>>>>> 22/12/2017 13:41, Anatoly Burakov:
>>>>>>>> During lcore scan, find maximum socket ID and store it.
>>>>>>>>
>>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>>>>>> ---
>>>>>>>> --- a/lib/librte_eal/common/include/rte_eal.h
>>>>>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
>>>>>>>> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
>>>>>>>>      	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
>>>>>>>
>>>>>>> isn't it breaking the ABI?
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Yep, you're right, forgot to add that. I didn't expect this to get
>>>>>> merged in 18.02 anyway, so v2 will follow.
>>>>>
>>>>> Please write 18.05 in the subject to show your expectation.
>>>>> Thanks
>>>>>
>>>>
>>>> Does it have to be an ABI change though? We can put numa_node_count
>>>> after pointer to mem_config, in which case it won't be an ABI break.
>>>> Would that be better?
>>>
>>> Changing the size of a struct which is allocated by the app,
>>> is an ABI break.
>>> Is your solution changing the size?
>>>
>>
>> It's not really allocated as such. rte_config is a global static
>> variable, and we only ever get pointers to it from the user code. If we
>> add the new value at the end, all of the old data layout would be intact
>> and work as before, so nothing would change as far as old code is concerned.
>>
>> However, if that's still considered an ABI break, then OK, break it is.
> 
> Maybe that assuming it is never allocated (not copied for instance)
> we could consider it is not an ABI break.
> 
>> Some background for why this is needed - for the memory hotplug, we need
>> to know how many sockets we can allocate memory at, to distinguish
>> between socket that doesn't exist, and socket that exists but has no
>> memory allocated on it. I'm OK with trying other approaches (such as
>> storing numa nodes in a static variable somewhere) if breaking ABI for
>> this is too much to ask for such a minute change.
> 
> Why is it important for 18.02?
> Memory hotplug will be integrated only in 18.05.
> I think it is better to just wait (and announce the deprecation).
> 

It isn't, i've already marked this patch as deferred. However, we'll 
have to have this discussion anyway :)

-- 
Thanks,
Anatoly

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

* [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal
  2017-12-22 12:41 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
  2018-01-11 22:20   ` Thomas Monjalon
@ 2018-01-16 17:53   ` Anatoly Burakov
  2018-01-23 10:39     ` Mcnamara, John
  2018-02-12 16:00     ` Jonas Pfefferle
       [not found]   ` <cover.1517848624.git.anatoly.burakov@intel.com>
  2 siblings, 2 replies; 42+ messages in thread
From: Anatoly Burakov @ 2018-01-16 17:53 UTC (permalink / raw)
  To: dev; +Cc: Neil Horman, John McNamara, Marko Kovacevic

There will be a new function added in v18.05 that will return
number of detected sockets, which will change the ABI.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 doc/guides/rel_notes/deprecation.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 13e8543..9662150 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -8,6 +8,8 @@ API and ABI deprecation notices are to be posted here.
 Deprecation Notices
 -------------------
 
+* eal: new ``numa_node_count`` member will be added to ``rte_config`` structure
+  in v18.05.
 * eal: several API and ABI changes are planned for ``rte_devargs`` in v18.02.
   The format of device command line parameters will change. The bus will need
   to be explicitly stated in the device declaration. The enum ``rte_devtype``
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH v2] eal: add function to return number of detected sockets
  2018-01-16 17:38                 ` Burakov, Anatoly
@ 2018-01-16 18:26                   ` Thomas Monjalon
  0 siblings, 0 replies; 42+ messages in thread
From: Thomas Monjalon @ 2018-01-16 18:26 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

16/01/2018 18:38, Burakov, Anatoly:
> On 16-Jan-18 5:34 PM, Thomas Monjalon wrote:
> > 16/01/2018 16:05, Burakov, Anatoly:
> >> On 16-Jan-18 12:20 PM, Thomas Monjalon wrote:
> >>> 16/01/2018 12:56, Burakov, Anatoly:
> >>>> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> >>>>> 12/01/2018 12:44, Burakov, Anatoly:
> >>>>>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> >>>>>>> 22/12/2017 13:41, Anatoly Burakov:
> >>>>>>>> During lcore scan, find maximum socket ID and store it.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>>>>>>> ---
> >>>>>>>> --- a/lib/librte_eal/common/include/rte_eal.h
> >>>>>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
> >>>>>>>> @@ -83,6 +83,7 @@ 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 service_lcore_count;/**< Number of available service cores. */
> >>>>>>>>      	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> >>>>>>>
> >>>>>>> isn't it breaking the ABI?
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>> Yep, you're right, forgot to add that. I didn't expect this to get
> >>>>>> merged in 18.02 anyway, so v2 will follow.
> >>>>>
> >>>>> Please write 18.05 in the subject to show your expectation.
> >>>>> Thanks
> >>>>>
> >>>>
> >>>> Does it have to be an ABI change though? We can put numa_node_count
> >>>> after pointer to mem_config, in which case it won't be an ABI break.
> >>>> Would that be better?
> >>>
> >>> Changing the size of a struct which is allocated by the app,
> >>> is an ABI break.
> >>> Is your solution changing the size?
> >>>
> >>
> >> It's not really allocated as such. rte_config is a global static
> >> variable, and we only ever get pointers to it from the user code. If we
> >> add the new value at the end, all of the old data layout would be intact
> >> and work as before, so nothing would change as far as old code is concerned.
> >>
> >> However, if that's still considered an ABI break, then OK, break it is.
> > 
> > Maybe that assuming it is never allocated (not copied for instance)
> > we could consider it is not an ABI break.
> > 
> >> Some background for why this is needed - for the memory hotplug, we need
> >> to know how many sockets we can allocate memory at, to distinguish
> >> between socket that doesn't exist, and socket that exists but has no
> >> memory allocated on it. I'm OK with trying other approaches (such as
> >> storing numa nodes in a static variable somewhere) if breaking ABI for
> >> this is too much to ask for such a minute change.
> > 
> > Why is it important for 18.02?
> > Memory hotplug will be integrated only in 18.05.
> > I think it is better to just wait (and announce the deprecation).
> > 
> 
> It isn't, i've already marked this patch as deferred. However, we'll 
> have to have this discussion anyway :)

To be on the safe side, you announce a deprecation.
And there will be no debate in 18.05 (except if someone has a better idea).

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

* Re: [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal
  2018-01-16 17:53   ` [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal Anatoly Burakov
@ 2018-01-23 10:39     ` Mcnamara, John
  2018-02-07 10:10       ` Jerin Jacob
  2018-02-12 16:00     ` Jonas Pfefferle
  1 sibling, 1 reply; 42+ messages in thread
From: Mcnamara, John @ 2018-01-23 10:39 UTC (permalink / raw)
  To: Burakov, Anatoly, dev; +Cc: Neil Horman, Kovacevic, Marko



> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Tuesday, January 16, 2018 5:54 PM
> To: dev@dpdk.org
> Cc: Neil Horman <nhorman@tuxdriver.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko <marko.kovacevic@intel.com>
> Subject: [PATCH] doc: add ABI change notice for numa_node_count in eal
> 
> There will be a new function added in v18.05 that will return number of
> detected sockets, which will change the ABI.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 13e8543..9662150 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -8,6 +8,8 @@ API and ABI deprecation notices are to be posted here.
>  Deprecation Notices
>  -------------------
> 
> +* eal: new ``numa_node_count`` member will be added to ``rte_config``
> +structure in v18.05.
>  * eal: several API and ABI changes are planned for ``rte_devargs`` in  v18.02.

In general it is best to leave a blank line between the bullet points. But that
doesn't affect the rendering so:

Acked-by: John McNamara <john.mcnamara@intel.com>

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

* [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets
       [not found]   ` <cover.1517848624.git.anatoly.burakov@intel.com>
@ 2018-02-05 16:37     ` Anatoly Burakov
  2018-02-05 17:39       ` Burakov, Anatoly
                         ` (3 more replies)
  0 siblings, 4 replies; 42+ messages in thread
From: Anatoly Burakov @ 2018-02-05 16:37 UTC (permalink / raw)
  To: dev

During lcore scan, find maximum socket ID and store it.

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

Notes:
    v3:
    - Added ABI backwards compatibility
    
    v2:
    - checkpatch changes
    - check socket before deciding if the core is not to be used

 lib/librte_eal/common/eal_common_lcore.c  | 37 +++++++++++++++++++++----------
 lib/librte_eal/common/include/rte_eal.h   | 25 +++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
 lib/librte_eal/linuxapp/eal/eal.c         | 27 +++++++++++++++++++++-
 lib/librte_eal/rte_eal_version.map        |  9 +++++++-
 5 files changed, 92 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 7724fa4..827ddeb 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -28,6 +28,7 @@ rte_eal_cpu_init(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	unsigned lcore_id;
 	unsigned count = 0;
+	unsigned int socket_id, max_socket_id = 0;
 
 	/*
 	 * Parse the maximum set of logical cores, detect the subset of running
@@ -39,6 +40,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
+		}
+		max_socket_id = RTE_MAX(max_socket_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 +68,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 +82,15 @@ rte_eal_cpu_init(void)
 		RTE_MAX_LCORE);
 	RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
 
+	config->numa_node_count = max_socket_id + 1;
+	RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", config->numa_node_count);
+
 	return 0;
 }
+
+unsigned int
+rte_num_sockets(void)
+{
+	const struct rte_config *config = rte_eal_get_configuration();
+	return config->numa_node_count;
+}
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 08c6637..bbf54e2 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -57,6 +57,29 @@ 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 service_lcore_count;/**< Number of available service cores. */
+	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
+
+	/** Primary or secondary configuration */
+	enum rte_proc_type_t process_type;
+
+	/** PA or VA mapping mode */
+	enum rte_iova_mode iova_mode;
+
+	/**
+	 * Pointer to memory configuration, which may be shared across multiple
+	 * DPDK instances
+	 */
+	struct rte_mem_config *mem_config;
+} __attribute__((__packed__));
+
+/**
+ * The global RTE configuration structure - 18.02 ABI version.
+ */
+struct rte_config_v1802 {
+	uint32_t master_lcore;       /**< Id of the master lcore */
+	uint32_t lcore_count;        /**< Number of available logical cores. */
 	uint32_t service_lcore_count;/**< Number of available service cores. */
 	enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
 
@@ -80,6 +103,8 @@ struct rte_config {
  *   A pointer to the global configuration structure.
  */
 struct rte_config *rte_eal_get_configuration(void);
+struct rte_config_v1802 *rte_eal_get_configuration_v1802(void);
+struct rte_config *rte_eal_get_configuration_v1805(void);
 
 /**
  * Get a lcore's role.
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index d84bcff..ddf4c64 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -120,6 +120,14 @@ rte_lcore_index(int lcore_id)
 unsigned rte_socket_id(void);
 
 /**
+ * Return number of physical sockets on the system.
+ * @return
+ *   the number of physical sockets as recognized by EAL
+ *
+ */
+unsigned int rte_num_sockets(void);
+
+/**
  * Get the ID of the physical socket of the specified lcore
  *
  * @param lcore_id
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 451fdaf..757f404 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -67,6 +67,9 @@ static rte_usage_hook_t	rte_application_usage_hook = NULL;
 /* early configuration structure, when memory config is not mmapped */
 static struct rte_mem_config early_mem_config;
 
+/* compatibility structure to return to old ABI calls */
+static struct rte_config_v1802 v1802_config;
+
 /* define fd variable here, because file needs to be kept open for the
  * duration of the program, as we hold a write lock on it in the primary proc */
 static int mem_cfg_fd = -1;
@@ -103,11 +106,33 @@ rte_eal_mbuf_default_mempool_ops(void)
 }
 
 /* Return a pointer to the configuration structure */
+struct rte_config_v1802 *
+rte_eal_get_configuration_v1802(void)
+{
+	/* copy everything to old config so that it's up to date */
+	v1802_config.iova_mode = rte_config.iova_mode;
+	v1802_config.lcore_count = rte_config.lcore_count;
+	memcpy(v1802_config.lcore_role, rte_config.lcore_role,
+			sizeof(rte_config.lcore_role));
+	v1802_config.master_lcore = rte_config.master_lcore;
+	v1802_config.mem_config = rte_config.mem_config;
+	v1802_config.process_type = rte_config.process_type;
+	v1802_config.service_lcore_count = rte_config.service_lcore_count;
+
+	return &v1802_config;
+}
+VERSION_SYMBOL(rte_eal_get_configuration, _v1802, 18.02);
+
+/* Return a pointer to the configuration structure */
 struct rte_config *
-rte_eal_get_configuration(void)
+rte_eal_get_configuration_v1805(void)
 {
 	return &rte_config;
 }
+VERSION_SYMBOL(rte_eal_get_configuration, _v1805, 18.05);
+BIND_DEFAULT_SYMBOL(rte_eal_get_configuration, _v1805, 18.05);
+MAP_STATIC_SYMBOL(struct rte_config *rte_eal_get_configuration(void),
+		rte_eal_get_configuration_v1805);
 
 enum rte_iova_mode
 rte_eal_iova_mode(void)
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 4146907..fc83e74 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -211,6 +211,13 @@ DPDK_18.02 {
 
 }  DPDK_17.11;
 
+DPDK_18.05 {
+	global:
+
+	rte_num_sockets;
+
+} DPDK_18.02;
+
 EXPERIMENTAL {
 	global:
 
@@ -255,4 +262,4 @@ EXPERIMENTAL {
 	rte_service_set_stats_enable;
 	rte_service_start_with_defaults;
 
-} DPDK_18.02;
+} DPDK_18.05;
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets
  2018-02-05 16:37     ` [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets Anatoly Burakov
@ 2018-02-05 17:39       ` Burakov, Anatoly
  2018-02-05 22:45         ` Thomas Monjalon
  2018-02-07  9:58       ` [dpdk-dev] [PATCH 18.05 v4] Add " Anatoly Burakov
                         ` (2 subsequent siblings)
  3 siblings, 1 reply; 42+ messages in thread
From: Burakov, Anatoly @ 2018-02-05 17:39 UTC (permalink / raw)
  To: dev

On 05-Feb-18 4:37 PM, Anatoly Burakov wrote:
> During lcore scan, find maximum socket ID and store it.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>      v3:
>      - Added ABI backwards compatibility
>      
>      v2:
>      - checkpatch changes
>      - check socket before deciding if the core is not to be used
> 
>   lib/librte_eal/common/eal_common_lcore.c  | 37 +++++++++++++++++++++----------
>   lib/librte_eal/common/include/rte_eal.h   | 25 +++++++++++++++++++++
>   lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
>   lib/librte_eal/linuxapp/eal/eal.c         | 27 +++++++++++++++++++++-
>   lib/librte_eal/rte_eal_version.map        |  9 +++++++-
>   5 files changed, 92 insertions(+), 14 deletions(-)
> 

This patch does not break ABI, but does it in a very ugly way. Is it 
worth it?

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets
  2018-02-05 17:39       ` Burakov, Anatoly
@ 2018-02-05 22:45         ` Thomas Monjalon
  2018-02-06  9:28           ` Burakov, Anatoly
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Monjalon @ 2018-02-05 22:45 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

05/02/2018 18:39, Burakov, Anatoly:
> On 05-Feb-18 4:37 PM, Anatoly Burakov wrote:
> > During lcore scan, find maximum socket ID and store it.
> > 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
> > 
> > Notes:
> >      v3:
> >      - Added ABI backwards compatibility
> >      
> >      v2:
> >      - checkpatch changes
> >      - check socket before deciding if the core is not to be used
> > 
> >   lib/librte_eal/common/eal_common_lcore.c  | 37 +++++++++++++++++++++----------
> >   lib/librte_eal/common/include/rte_eal.h   | 25 +++++++++++++++++++++
> >   lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
> >   lib/librte_eal/linuxapp/eal/eal.c         | 27 +++++++++++++++++++++-
> >   lib/librte_eal/rte_eal_version.map        |  9 +++++++-
> >   5 files changed, 92 insertions(+), 14 deletions(-)
> > 
> 
> This patch does not break ABI, but does it in a very ugly way. Is it 
> worth it?

I think we agreed to not get this patch in 18.02.
Did you change your mind?

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

* Re: [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets
  2018-02-05 22:45         ` Thomas Monjalon
@ 2018-02-06  9:28           ` Burakov, Anatoly
  2018-02-06  9:47             ` Thomas Monjalon
  0 siblings, 1 reply; 42+ messages in thread
From: Burakov, Anatoly @ 2018-02-06  9:28 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 05-Feb-18 10:45 PM, Thomas Monjalon wrote:
> 05/02/2018 18:39, Burakov, Anatoly:
>> On 05-Feb-18 4:37 PM, Anatoly Burakov wrote:
>>> During lcore scan, find maximum socket ID and store it.
>>>
>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>> ---
>>>
>>> Notes:
>>>       v3:
>>>       - Added ABI backwards compatibility
>>>       
>>>       v2:
>>>       - checkpatch changes
>>>       - check socket before deciding if the core is not to be used
>>>
>>>    lib/librte_eal/common/eal_common_lcore.c  | 37 +++++++++++++++++++++----------
>>>    lib/librte_eal/common/include/rte_eal.h   | 25 +++++++++++++++++++++
>>>    lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
>>>    lib/librte_eal/linuxapp/eal/eal.c         | 27 +++++++++++++++++++++-
>>>    lib/librte_eal/rte_eal_version.map        |  9 +++++++-
>>>    5 files changed, 92 insertions(+), 14 deletions(-)
>>>
>>
>> This patch does not break ABI, but does it in a very ugly way. Is it
>> worth it?
> 
> I think we agreed to not get this patch in 18.02.
> Did you change your mind?
> 

Sorry, how do i mark this patch as for 18.05? Is it a patch header?

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets
  2018-02-06  9:28           ` Burakov, Anatoly
@ 2018-02-06  9:47             ` Thomas Monjalon
  0 siblings, 0 replies; 42+ messages in thread
From: Thomas Monjalon @ 2018-02-06  9:47 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

06/02/2018 10:28, Burakov, Anatoly:
> On 05-Feb-18 10:45 PM, Thomas Monjalon wrote:
> > 05/02/2018 18:39, Burakov, Anatoly:
> >> On 05-Feb-18 4:37 PM, Anatoly Burakov wrote:
> >>> During lcore scan, find maximum socket ID and store it.
> >>>
> >>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>> ---
> >>>
> >>> Notes:
> >>>       v3:
> >>>       - Added ABI backwards compatibility
> >>>       
> >>>       v2:
> >>>       - checkpatch changes
> >>>       - check socket before deciding if the core is not to be used
> >>>
> >>>    lib/librte_eal/common/eal_common_lcore.c  | 37 +++++++++++++++++++++----------
> >>>    lib/librte_eal/common/include/rte_eal.h   | 25 +++++++++++++++++++++
> >>>    lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
> >>>    lib/librte_eal/linuxapp/eal/eal.c         | 27 +++++++++++++++++++++-
> >>>    lib/librte_eal/rte_eal_version.map        |  9 +++++++-
> >>>    5 files changed, 92 insertions(+), 14 deletions(-)
> >>>
> >>
> >> This patch does not break ABI, but does it in a very ugly way. Is it
> >> worth it?
> > 
> > I think we agreed to not get this patch in 18.02.
> > Did you change your mind?
> > 
> 
> Sorry, how do i mark this patch as for 18.05? Is it a patch header?

So your answer is "yes, it is for 18.05" :)

Next time, you could add 18.05 near "PATCH v3", or say it in annotations.

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

* [dpdk-dev] [PATCH 18.05 v4] Add function to return number of detected sockets
  2018-02-05 16:37     ` [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets Anatoly Burakov
  2018-02-05 17:39       ` Burakov, Anatoly
@ 2018-02-07  9:58       ` Anatoly Burakov
  2018-02-07  9:58       ` [dpdk-dev] [PATCH 18.05 v4] eal: add " Anatoly Burakov
  2018-03-22 10:58       ` [dpdk-dev] [PATCH v5] eal: provide API for querying valid socket id's Anatoly Burakov
  3 siblings, 0 replies; 42+ messages in thread
From: Anatoly Burakov @ 2018-02-07  9:58 UTC (permalink / raw)
  To: dev

This patch is for 18.05 and implements changes referenced
in the deprecation notice[1]. (not yet merged as of this
writing)

This patchset breaks the EAL ABI and bumps its version. This
is arguably OK as memory changes will change a lot in EAL and
thus likely break ABI anyway. However, two other alternative
implementations are possible:

1) local static variable recording number of detected
   sockets. This is arguably less clean approach, but will not
   break the ABI and will have relatively little impact on the
   codebase.

2) keeping ABI compatibility, as shown in v3 of this patch [2].

My preference would be to keep this one.

[1] http://dpdk.org/dev/patchwork/patch/33853/
[2] http://dpdk.org/dev/patchwork/patch/34994/

Anatoly Burakov (1):
  eal: add function to return number of detected sockets

 lib/librte_eal/bsdapp/eal/Makefile        |  2 +-
 lib/librte_eal/common/eal_common_lcore.c  | 37 +++++++++++++++++++++----------
 lib/librte_eal/common/include/rte_eal.h   |  1 +
 lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
 lib/librte_eal/linuxapp/eal/Makefile      |  2 +-
 lib/librte_eal/rte_eal_version.map        |  9 +++++++-
 6 files changed, 44 insertions(+), 15 deletions(-)

-- 
2.7.4

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

* [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-02-05 16:37     ` [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets Anatoly Burakov
  2018-02-05 17:39       ` Burakov, Anatoly
  2018-02-07  9:58       ` [dpdk-dev] [PATCH 18.05 v4] Add " Anatoly Burakov
@ 2018-02-07  9:58       ` Anatoly Burakov
  2018-03-08 12:12         ` Bruce Richardson
  2018-03-21  4:59         ` gowrishankar muthukrishnan
  2018-03-22 10:58       ` [dpdk-dev] [PATCH v5] eal: provide API for querying valid socket id's Anatoly Burakov
  3 siblings, 2 replies; 42+ messages in thread
From: Anatoly Burakov @ 2018-02-07  9:58 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson

During lcore scan, find maximum socket ID and store it. This will
break the ABI, so bump ABI version.

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

Notes:
    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  | 37 +++++++++++++++++++++----------
 lib/librte_eal/common/include/rte_eal.h   |  1 +
 lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
 lib/librte_eal/linuxapp/eal/Makefile      |  2 +-
 lib/librte_eal/rte_eal_version.map        |  9 +++++++-
 6 files changed, 44 insertions(+), 15 deletions(-)

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..827ddeb 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -28,6 +28,7 @@ rte_eal_cpu_init(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	unsigned lcore_id;
 	unsigned count = 0;
+	unsigned int socket_id, max_socket_id = 0;
 
 	/*
 	 * Parse the maximum set of logical cores, detect the subset of running
@@ -39,6 +40,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
+		}
+		max_socket_id = RTE_MAX(max_socket_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 +68,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 +82,15 @@ rte_eal_cpu_init(void)
 		RTE_MAX_LCORE);
 	RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
 
+	config->numa_node_count = max_socket_id + 1;
+	RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", config->numa_node_count);
+
 	return 0;
 }
+
+unsigned int
+rte_num_sockets(void)
+{
+	const struct rte_config *config = rte_eal_get_configuration();
+	return config->numa_node_count;
+}
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 08c6637..63fcc2e 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -57,6 +57,7 @@ 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 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 d84bcff..ddf4c64 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -120,6 +120,14 @@ rte_lcore_index(int lcore_id)
 unsigned rte_socket_id(void);
 
 /**
+ * Return number of physical sockets on the system.
+ * @return
+ *   the number of physical sockets as recognized by EAL
+ *
+ */
+unsigned int rte_num_sockets(void);
+
+/**
  * 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/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 4146907..fc83e74 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -211,6 +211,13 @@ DPDK_18.02 {
 
 }  DPDK_17.11;
 
+DPDK_18.05 {
+	global:
+
+	rte_num_sockets;
+
+} DPDK_18.02;
+
 EXPERIMENTAL {
 	global:
 
@@ -255,4 +262,4 @@ EXPERIMENTAL {
 	rte_service_set_stats_enable;
 	rte_service_start_with_defaults;
 
-} DPDK_18.02;
+} DPDK_18.05;
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal
  2018-01-23 10:39     ` Mcnamara, John
@ 2018-02-07 10:10       ` Jerin Jacob
  2018-02-09 14:42         ` Bruce Richardson
  0 siblings, 1 reply; 42+ messages in thread
From: Jerin Jacob @ 2018-02-07 10:10 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: Burakov, Anatoly, dev, Neil Horman, Kovacevic, Marko

-----Original Message-----
> Date: Tue, 23 Jan 2018 10:39:58 +0000
> From: "Mcnamara, John" <john.mcnamara@intel.com>
> To: "Burakov, Anatoly" <anatoly.burakov@intel.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> CC: Neil Horman <nhorman@tuxdriver.com>, "Kovacevic, Marko"
>  <marko.kovacevic@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] doc: add ABI change notice for
>  numa_node_count in eal
> 
> 
> 
> > -----Original Message-----
> > From: Burakov, Anatoly
> > Sent: Tuesday, January 16, 2018 5:54 PM
> > To: dev@dpdk.org
> > Cc: Neil Horman <nhorman@tuxdriver.com>; Mcnamara, John
> > <john.mcnamara@intel.com>; Kovacevic, Marko <marko.kovacevic@intel.com>
> > Subject: [PATCH] doc: add ABI change notice for numa_node_count in eal
> > 
> > There will be a new function added in v18.05 that will return number of
> > detected sockets, which will change the ABI.
> > 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index 13e8543..9662150 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -8,6 +8,8 @@ API and ABI deprecation notices are to be posted here.
> >  Deprecation Notices
> >  -------------------
> > 
> > +* eal: new ``numa_node_count`` member will be added to ``rte_config``
> > +structure in v18.05.
> >  * eal: several API and ABI changes are planned for ``rte_devargs`` in  v18.02.
> 
> In general it is best to leave a blank line between the bullet points. But that
> doesn't affect the rendering so:
> 
> Acked-by: John McNamara <john.mcnamara@intel.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>


> 
> 

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

* Re: [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal
  2018-02-07 10:10       ` Jerin Jacob
@ 2018-02-09 14:42         ` Bruce Richardson
  2018-02-14  0:04           ` Thomas Monjalon
  0 siblings, 1 reply; 42+ messages in thread
From: Bruce Richardson @ 2018-02-09 14:42 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Mcnamara, John, Burakov, Anatoly, dev, Neil Horman, Kovacevic, Marko

On Wed, Feb 07, 2018 at 03:40:20PM +0530, Jerin Jacob wrote:
> -----Original Message-----
> > Date: Tue, 23 Jan 2018 10:39:58 +0000
> > From: "Mcnamara, John" <john.mcnamara@intel.com>
> > To: "Burakov, Anatoly" <anatoly.burakov@intel.com>, "dev@dpdk.org"
> >  <dev@dpdk.org>
> > CC: Neil Horman <nhorman@tuxdriver.com>, "Kovacevic, Marko"
> >  <marko.kovacevic@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH] doc: add ABI change notice for
> >  numa_node_count in eal
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Burakov, Anatoly
> > > Sent: Tuesday, January 16, 2018 5:54 PM
> > > To: dev@dpdk.org
> > > Cc: Neil Horman <nhorman@tuxdriver.com>; Mcnamara, John
> > > <john.mcnamara@intel.com>; Kovacevic, Marko <marko.kovacevic@intel.com>
> > > Subject: [PATCH] doc: add ABI change notice for numa_node_count in eal
> > > 
> > > There will be a new function added in v18.05 that will return number of
> > > detected sockets, which will change the ABI.
> > > 
> > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > ---
> > >  doc/guides/rel_notes/deprecation.rst | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > b/doc/guides/rel_notes/deprecation.rst
> > > index 13e8543..9662150 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -8,6 +8,8 @@ API and ABI deprecation notices are to be posted here.
> > >  Deprecation Notices
> > >  -------------------
> > > 
> > > +* eal: new ``numa_node_count`` member will be added to ``rte_config``
> > > +structure in v18.05.
> > >  * eal: several API and ABI changes are planned for ``rte_devargs`` in  v18.02.
> > 
> > In general it is best to leave a blank line between the bullet points. But that
> > doesn't affect the rendering so:
> > 
> > Acked-by: John McNamara <john.mcnamara@intel.com>
> 
> Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

* Re: [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal
  2018-01-16 17:53   ` [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal Anatoly Burakov
  2018-01-23 10:39     ` Mcnamara, John
@ 2018-02-12 16:00     ` Jonas Pfefferle
  1 sibling, 0 replies; 42+ messages in thread
From: Jonas Pfefferle @ 2018-02-12 16:00 UTC (permalink / raw)
  To: Anatoly Burakov, dev; +Cc: Neil Horman, John McNamara, Marko Kovacevic

On Tue, 16 Jan 2018 17:53:40 +0000
  Anatoly Burakov <anatoly.burakov@intel.com> wrote:
> There will be a new function added in v18.05 that will return
> number of detected sockets, which will change the ABI.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> doc/guides/rel_notes/deprecation.rst | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst 
>b/doc/guides/rel_notes/deprecation.rst
> index 13e8543..9662150 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -8,6 +8,8 @@ API and ABI deprecation notices are to be posted 
>here.
> Deprecation Notices
> -------------------
> 
> +* eal: new ``numa_node_count`` member will be added to 
>``rte_config`` structure
> +  in v18.05.
> * eal: several API and ABI changes are planned for ``rte_devargs`` 
>in v18.02.
>   The format of device command line parameters will change. The bus 
>will need
>   to be explicitly stated in the device declaration. The enum 
>``rte_devtype``
> -- 
> 2.7.4

Acked-by: Jonas Pfefferle <pepperjo@japf.ch>

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

* Re: [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal
  2018-02-09 14:42         ` Bruce Richardson
@ 2018-02-14  0:04           ` Thomas Monjalon
  2018-02-14 14:25             ` Thomas Monjalon
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Monjalon @ 2018-02-14  0:04 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: dev, Bruce Richardson, Jerin Jacob, Mcnamara, John, Neil Horman,
	Kovacevic, Marko

> > > > There will be a new function added in v18.05 that will return number of
> > > > detected sockets, which will change the ABI.
> > > > 
> > > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > > ---
> > > > +* eal: new ``numa_node_count`` member will be added to ``rte_config``
> > > > +structure in v18.05.
> > > 
> > > Acked-by: John McNamara <john.mcnamara@intel.com>
> > 
> > Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Acked-by: Thomas Monjalon <thomas@monjalon.net>

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

* Re: [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal
  2018-02-14  0:04           ` Thomas Monjalon
@ 2018-02-14 14:25             ` Thomas Monjalon
  0 siblings, 0 replies; 42+ messages in thread
From: Thomas Monjalon @ 2018-02-14 14:25 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: dev, Bruce Richardson, Jerin Jacob, Mcnamara, John, Neil Horman,
	Kovacevic, Marko

14/02/2018 01:04, Thomas Monjalon:
> > > > > There will be a new function added in v18.05 that will return number of
> > > > > detected sockets, which will change the ABI.
> > > > > 
> > > > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > > > ---
> > > > > +* eal: new ``numa_node_count`` member will be added to ``rte_config``
> > > > > +structure in v18.05.
> > > > 
> > > > Acked-by: John McNamara <john.mcnamara@intel.com>
> > > 
> > > Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Applied

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

* Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-02-07  9:58       ` [dpdk-dev] [PATCH 18.05 v4] eal: add " Anatoly Burakov
@ 2018-03-08 12:12         ` Bruce Richardson
  2018-03-08 14:38           ` Burakov, Anatoly
  2018-03-21  4:59         ` gowrishankar muthukrishnan
  1 sibling, 1 reply; 42+ messages in thread
From: Bruce Richardson @ 2018-03-08 12:12 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev

On Wed, Feb 07, 2018 at 09:58:36AM +0000, Anatoly Burakov wrote:
> During lcore scan, find maximum socket ID and store it. This will
> break the ABI, so bump ABI version.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>     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  | 37 +++++++++++++++++++++----------
>  lib/librte_eal/common/include/rte_eal.h   |  1 +
>  lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
>  lib/librte_eal/linuxapp/eal/Makefile      |  2 +-
>  lib/librte_eal/rte_eal_version.map        |  9 +++++++-
>  6 files changed, 44 insertions(+), 15 deletions(-)
> 
Breaking the ABI is the best way to implement this change, and given the
deprecation was previously announced I'm ok with that.

Question: we are ok assuming that the socket numbers are sequential, or
nearly so, and knowing the maximum socket number seen is a good
approximation of the actual physical sockets? I know in terms of cores
on a system, the core id's often jump - are there systems where the
socket numbers do too?

/Bruce

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

* Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-03-08 12:12         ` Bruce Richardson
@ 2018-03-08 14:38           ` Burakov, Anatoly
  2018-03-09 16:32             ` Bruce Richardson
  2018-03-20 22:43             ` Thomas Monjalon
  0 siblings, 2 replies; 42+ messages in thread
From: Burakov, Anatoly @ 2018-03-08 14:38 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On 08-Mar-18 12:12 PM, Bruce Richardson wrote:
> On Wed, Feb 07, 2018 at 09:58:36AM +0000, Anatoly Burakov wrote:
>> During lcore scan, find maximum socket ID and store it. This will
>> break the ABI, so bump ABI version.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>
>> Notes:
>>      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  | 37 +++++++++++++++++++++----------
>>   lib/librte_eal/common/include/rte_eal.h   |  1 +
>>   lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
>>   lib/librte_eal/linuxapp/eal/Makefile      |  2 +-
>>   lib/librte_eal/rte_eal_version.map        |  9 +++++++-
>>   6 files changed, 44 insertions(+), 15 deletions(-)
>>
> Breaking the ABI is the best way to implement this change, and given the
> deprecation was previously announced I'm ok with that.
> 
> Question: we are ok assuming that the socket numbers are sequential, or
> nearly so, and knowing the maximum socket number seen is a good
> approximation of the actual physical sockets? I know in terms of cores
> on a system, the core id's often jump - are there systems where the
> socket numbers do too?
> 
> /Bruce
> 

I am not aware of any system that would jump sockets like that. I'm open 
to corrections, however :)

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-03-08 14:38           ` Burakov, Anatoly
@ 2018-03-09 16:32             ` Bruce Richardson
  2018-03-20 22:43             ` Thomas Monjalon
  1 sibling, 0 replies; 42+ messages in thread
From: Bruce Richardson @ 2018-03-09 16:32 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

On Thu, Mar 08, 2018 at 02:38:37PM +0000, Burakov, Anatoly wrote:
> On 08-Mar-18 12:12 PM, Bruce Richardson wrote:
> > On Wed, Feb 07, 2018 at 09:58:36AM +0000, Anatoly Burakov wrote:
> > > During lcore scan, find maximum socket ID and store it. This will
> > > break the ABI, so bump ABI version.
> > > 
> > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > ---
> > > 
> > > Notes:
> > >      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  | 37 +++++++++++++++++++++----------
> > >   lib/librte_eal/common/include/rte_eal.h   |  1 +
> > >   lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
> > >   lib/librte_eal/linuxapp/eal/Makefile      |  2 +-
> > >   lib/librte_eal/rte_eal_version.map        |  9 +++++++-
> > >   6 files changed, 44 insertions(+), 15 deletions(-)
> > > 
> > Breaking the ABI is the best way to implement this change, and given the
> > deprecation was previously announced I'm ok with that.
> > 
> > Question: we are ok assuming that the socket numbers are sequential, or
> > nearly so, and knowing the maximum socket number seen is a good
> > approximation of the actual physical sockets? I know in terms of cores
> > on a system, the core id's often jump - are there systems where the
> > socket numbers do too?
> > 
> > /Bruce
> > 
> 
> I am not aware of any system that would jump sockets like that. I'm open to
> corrections, however :)
> 
> -- 
In the absense of any corrections, I think this is fine to have.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

* Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-03-08 14:38           ` Burakov, Anatoly
  2018-03-09 16:32             ` Bruce Richardson
@ 2018-03-20 22:43             ` Thomas Monjalon
  1 sibling, 0 replies; 42+ messages in thread
From: Thomas Monjalon @ 2018-03-20 22:43 UTC (permalink / raw)
  To: Burakov, Anatoly, Bruce Richardson, Chao Zhu; +Cc: dev, gowrishankar.m

08/03/2018 15:38, Burakov, Anatoly:
> On 08-Mar-18 12:12 PM, Bruce Richardson wrote:
> > Question: we are ok assuming that the socket numbers are sequential, or
> > nearly so, and knowing the maximum socket number seen is a good
> > approximation of the actual physical sockets? I know in terms of cores
> > on a system, the core id's often jump - are there systems where the
> > socket numbers do too?
> 
> I am not aware of any system that would jump sockets like that. I'm open 
> to corrections, however :)

I think some IBM CPUs had this kind of jump in socket numbering.
Chao?

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

* Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-02-07  9:58       ` [dpdk-dev] [PATCH 18.05 v4] eal: add " Anatoly Burakov
  2018-03-08 12:12         ` Bruce Richardson
@ 2018-03-21  4:59         ` gowrishankar muthukrishnan
  2018-03-21 10:24           ` Burakov, Anatoly
  1 sibling, 1 reply; 42+ messages in thread
From: gowrishankar muthukrishnan @ 2018-03-21  4:59 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Bruce Richardson, Chao Zhu

On Wednesday 07 February 2018 03:28 PM, Anatoly Burakov wrote:
> During lcore scan, find maximum socket ID and store it. This will
> break the ABI, so bump ABI version.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>      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  | 37 +++++++++++++++++++++----------
>   lib/librte_eal/common/include/rte_eal.h   |  1 +
>   lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
>   lib/librte_eal/linuxapp/eal/Makefile      |  2 +-
>   lib/librte_eal/rte_eal_version.map        |  9 +++++++-
>   6 files changed, 44 insertions(+), 15 deletions(-)
>
> 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..827ddeb 100644
> --- a/lib/librte_eal/common/eal_common_lcore.c
> +++ b/lib/librte_eal/common/eal_common_lcore.c
> @@ -28,6 +28,7 @@ rte_eal_cpu_init(void)
>   	struct rte_config *config = rte_eal_get_configuration();
>   	unsigned lcore_id;
>   	unsigned count = 0;
> +	unsigned int socket_id, max_socket_id = 0;
>
>   	/*
>   	 * Parse the maximum set of logical cores, detect the subset of running
> @@ -39,6 +40,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
> +		}
> +		max_socket_id = RTE_MAX(max_socket_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 +68,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 +82,15 @@ rte_eal_cpu_init(void)
>   		RTE_MAX_LCORE);
>   	RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
>
> +	config->numa_node_count = max_socket_id + 1;

In some IBM servers, socket ID number does not seem to be in sequence. 
For an instance, 0 and 8 for a 2 node server.

In this case, numa_node_count would mislead users if wrongly understood 
by its variable name IMO (see below)
> +	RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", config->numa_node_count);

For an instance, reading above message would tell 'EAL detected 8 nodes' 
in my server, but actually there are only two nodes.

Could its name better be 'numa_node_id_max' ?. Also, we store in actual 
count of numa nodes in _count variable.

Also, there could be a case when there is no local memory available to a 
numa node too.

Thanks,
Gowrishankar
> +
>   	return 0;
>   }
> +
> +unsigned int
> +rte_num_sockets(void)
> +{
> +	const struct rte_config *config = rte_eal_get_configuration();
> +	return config->numa_node_count;
> +}
> diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
> index 08c6637..63fcc2e 100644
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -57,6 +57,7 @@ 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 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 d84bcff..ddf4c64 100644
> --- a/lib/librte_eal/common/include/rte_lcore.h
> +++ b/lib/librte_eal/common/include/rte_lcore.h
> @@ -120,6 +120,14 @@ rte_lcore_index(int lcore_id)
>   unsigned rte_socket_id(void);
>
>   /**
> + * Return number of physical sockets on the system.
> + * @return
> + *   the number of physical sockets as recognized by EAL
> + *
> + */
> +unsigned int rte_num_sockets(void);
> +
> +/**
>    * 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/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 4146907..fc83e74 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -211,6 +211,13 @@ DPDK_18.02 {
>
>   }  DPDK_17.11;
>
> +DPDK_18.05 {
> +	global:
> +
> +	rte_num_sockets;
> +
> +} DPDK_18.02;
> +
>   EXPERIMENTAL {
>   	global:
>
> @@ -255,4 +262,4 @@ EXPERIMENTAL {
>   	rte_service_set_stats_enable;
>   	rte_service_start_with_defaults;
>
> -} DPDK_18.02;
> +} DPDK_18.05;

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

* Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-03-21  4:59         ` gowrishankar muthukrishnan
@ 2018-03-21 10:24           ` Burakov, Anatoly
  2018-03-22  5:16             ` gowrishankar muthukrishnan
  0 siblings, 1 reply; 42+ messages in thread
From: Burakov, Anatoly @ 2018-03-21 10:24 UTC (permalink / raw)
  To: gowrishankar muthukrishnan; +Cc: dev, Bruce Richardson, Chao Zhu

On 21-Mar-18 4:59 AM, gowrishankar muthukrishnan wrote:
> On Wednesday 07 February 2018 03:28 PM, Anatoly Burakov wrote:
>> During lcore scan, find maximum socket ID and store it. This will
>> break the ABI, so bump ABI version.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>
>> Notes:
>>      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  | 37 
>> +++++++++++++++++++++----------
>>   lib/librte_eal/common/include/rte_eal.h   |  1 +
>>   lib/librte_eal/common/include/rte_lcore.h |  8 +++++++
>>   lib/librte_eal/linuxapp/eal/Makefile      |  2 +-
>>   lib/librte_eal/rte_eal_version.map        |  9 +++++++-
>>   6 files changed, 44 insertions(+), 15 deletions(-)
>>
>> 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..827ddeb 100644
>> --- a/lib/librte_eal/common/eal_common_lcore.c
>> +++ b/lib/librte_eal/common/eal_common_lcore.c
>> @@ -28,6 +28,7 @@ rte_eal_cpu_init(void)
>>       struct rte_config *config = rte_eal_get_configuration();
>>       unsigned lcore_id;
>>       unsigned count = 0;
>> +    unsigned int socket_id, max_socket_id = 0;
>>
>>       /*
>>        * Parse the maximum set of logical cores, detect the subset of 
>> running
>> @@ -39,6 +40,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
>> +        }
>> +        max_socket_id = RTE_MAX(max_socket_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 +68,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 +82,15 @@ rte_eal_cpu_init(void)
>>           RTE_MAX_LCORE);
>>       RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
>>
>> +    config->numa_node_count = max_socket_id + 1;
> 
> In some IBM servers, socket ID number does not seem to be in sequence. 
> For an instance, 0 and 8 for a 2 node server.
> 
> In this case, numa_node_count would mislead users if wrongly understood 
> by its variable name IMO (see below)
>> +    RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", 
>> config->numa_node_count);
> 
> For an instance, reading above message would tell 'EAL detected 8 nodes' 
> in my server, but actually there are only two nodes.
> 
> Could its name better be 'numa_node_id_max' ?. Also, we store in actual 
> count of numa nodes in _count variable.
> 
> Also, there could be a case when there is no local memory available to a 
> numa node too.
> 
> Thanks,
> Gowrishankar

The point of this patchset is to (pre)allocate memory only on existing 
sockets.

If we don't know how many sockets there are, we are forced to 
preallocate VA space per each *possible* NUMA node - that is, reserve 
e.g. 8x128G of memory, 6 of which will go unused on a 2-socket system. 
We can't know if there is no memory on socket in advance, but we can at 
least avoid preallocating VA space for sockets that don't exist in the 
first place.

How about we store all possible socket id's instead? e.g. something like:

static int numa_node_ids[MAX_NUMA_NODES];
<...>
int rte_eal_cpu_init() {
	int sockets[RTE_MAX_LCORE];
	<...>
	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
		core_to_socket[lcore_id] = socket;
	}
	<...>
	qsort(sockets);
	<...>
	// store all unique sockets in numa_node_ids in ascending order
}
<...>

on a 2 socket system we then get:

rte_num_sockets() => return 2
rte_get_socket_id(int idx) => return numa_node_ids[idx]

Would that be suitable?

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-03-21 10:24           ` Burakov, Anatoly
@ 2018-03-22  5:16             ` gowrishankar muthukrishnan
  2018-03-22  9:04               ` Burakov, Anatoly
  0 siblings, 1 reply; 42+ messages in thread
From: gowrishankar muthukrishnan @ 2018-03-22  5:16 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Bruce Richardson, Chao Zhu

On Wednesday 21 March 2018 03:54 PM, Burakov, Anatoly wrote:
>
>>> +    config->numa_node_count = max_socket_id + 1;
>>
>> In some IBM servers, socket ID number does not seem to be in 
>> sequence. For an instance, 0 and 8 for a 2 node server.
>>
>> In this case, numa_node_count would mislead users if wrongly 
>> understood by its variable name IMO (see below)
>>> +    RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", 
>>> config->numa_node_count);
>>
>> For an instance, reading above message would tell 'EAL detected 8 
>> nodes' in my server, but actually there are only two nodes.
>>
>> Could its name better be 'numa_node_id_max' ?. Also, we store in 
>> actual count of numa nodes in _count variable.
>>
>> Also, there could be a case when there is no local memory available 
>> to a numa node too.
>>
>> Thanks,
>> Gowrishankar
>
> The point of this patchset is to (pre)allocate memory only on existing 
> sockets.
>
> If we don't know how many sockets there are, we are forced to 
> preallocate VA space per each *possible* NUMA node - that is, reserve 
> e.g. 8x128G of memory, 6 of which will go unused on a 2-socket system. 
> We can't know if there is no memory on socket in advance, but we can 
> at least avoid preallocating VA space for sockets that don't exist in 
> the first place.
>

Sounds good Anatoly.
May be, sysfs/ might help to confirm if a numa node has local memory ?.

Anyway, for the context of this particular patch (return numa nodes), 
below approach you mentioned is good.

> How about we store all possible socket id's instead? e.g. something like:
>
> static int numa_node_ids[MAX_NUMA_NODES];
> <...>
> int rte_eal_cpu_init() {
>     int sockets[RTE_MAX_LCORE];
>     <...>
>     for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
>         core_to_socket[lcore_id] = socket;
sockets[lcore_id] = eal_cpu_socket_id(lcore_id);
>     }
>     <...>
>     qsort(sockets);
>     <...>
>     // store all unique sockets in numa_node_ids in ascending order

Just thinking that, is there a purpose of retaining a numa ID which does 
not have local memory attached ?
but sockets[] is suppose to reflect all available nodes though (and 
assuming, its calling place to ensure
for the existence of numa local memory).


> }
> <...>
>
> on a 2 socket system we then get:
>
> rte_num_sockets() => return 2
> rte_get_socket_id(int idx) => return numa_node_ids[idx]
rte_get_socket_mem(idx) might help to validate for local memory existence ?

>
> Would that be suitable?
>

Thanks,
Gowrishankar

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

* Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets
  2018-03-22  5:16             ` gowrishankar muthukrishnan
@ 2018-03-22  9:04               ` Burakov, Anatoly
  0 siblings, 0 replies; 42+ messages in thread
From: Burakov, Anatoly @ 2018-03-22  9:04 UTC (permalink / raw)
  To: gowrishankar muthukrishnan; +Cc: dev, Bruce Richardson, Chao Zhu

On 22-Mar-18 5:16 AM, gowrishankar muthukrishnan wrote:
> On Wednesday 21 March 2018 03:54 PM, Burakov, Anatoly wrote:
>>
>>>> +    config->numa_node_count = max_socket_id + 1;
>>>
>>> In some IBM servers, socket ID number does not seem to be in 
>>> sequence. For an instance, 0 and 8 for a 2 node server.
>>>
>>> In this case, numa_node_count would mislead users if wrongly 
>>> understood by its variable name IMO (see below)
>>>> +    RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", 
>>>> config->numa_node_count);
>>>
>>> For an instance, reading above message would tell 'EAL detected 8 
>>> nodes' in my server, but actually there are only two nodes.
>>>
>>> Could its name better be 'numa_node_id_max' ?. Also, we store in 
>>> actual count of numa nodes in _count variable.
>>>
>>> Also, there could be a case when there is no local memory available 
>>> to a numa node too.
>>>
>>> Thanks,
>>> Gowrishankar
>>
>> The point of this patchset is to (pre)allocate memory only on existing 
>> sockets.
>>
>> If we don't know how many sockets there are, we are forced to 
>> preallocate VA space per each *possible* NUMA node - that is, reserve 
>> e.g. 8x128G of memory, 6 of which will go unused on a 2-socket system. 
>> We can't know if there is no memory on socket in advance, but we can 
>> at least avoid preallocating VA space for sockets that don't exist in 
>> the first place.
>>
> 
> Sounds good Anatoly.
> May be, sysfs/ might help to confirm if a numa node has local memory ?.

We can't go to sysfs every time we want to allocate memory, and we can't 
really depend on what sysfs tells us about availability of hugepages on 
a particular socket (assuming that's what you meant by "confirm if a 
numa node has local memory"). User may modify hugepage numbers for each 
socket at runtime, and suddenly we do (or don't) have memory on local 
socket.

Therefore i think a better approach would be - if a socket exists (that 
is, if we can find lcores on that socket, even if they're not active), 
assume it has/had/will have memory, and store it as a valid socket id.

I'll respin a v5 with changes outlined below then. Thanks!

> 
> Anyway, for the context of this particular patch (return numa nodes), 
> below approach you mentioned is good.
> 
>> How about we store all possible socket id's instead? e.g. something like:
>>
>> static int numa_node_ids[MAX_NUMA_NODES];
>> <...>
>> int rte_eal_cpu_init() {
>>     int sockets[RTE_MAX_LCORE];
>>     <...>
>>     for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
>>         core_to_socket[lcore_id] = socket;
> sockets[lcore_id] = eal_cpu_socket_id(lcore_id);
>>     }
>>     <...>
>>     qsort(sockets);
>>     <...>
>>     // store all unique sockets in numa_node_ids in ascending order
> 
> Just thinking that, is there a purpose of retaining a numa ID which does 
> not have local memory attached ?
> but sockets[] is suppose to reflect all available nodes though (and 
> assuming, its calling place to ensure
> for the existence of numa local memory).
> 
> 
>> }
>> <...>
>>
>> on a 2 socket system we then get:
>>
>> rte_num_sockets() => return 2
>> rte_get_socket_id(int idx) => return numa_node_ids[idx]
> rte_get_socket_mem(idx) might help to validate for local memory existence ?
> 
>>
>> Would that be suitable?
>>
> 
> Thanks,
> Gowrishankar
> 
> 


-- 
Thanks,
Anatoly

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

* [dpdk-dev] [PATCH v5] eal: provide API for querying valid socket id's
  2018-02-05 16:37     ` [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets Anatoly Burakov
                         ` (2 preceding siblings ...)
  2018-02-07  9:58       ` [dpdk-dev] [PATCH 18.05 v4] eal: add " Anatoly Burakov
@ 2018-03-22 10:58       ` Anatoly Burakov
  2018-03-22 11:45         ` Burakov, Anatoly
  2018-03-22 12:36         ` [dpdk-dev] [PATCH v6] " Anatoly Burakov
  3 siblings, 2 replies; 42+ messages in thread
From: Anatoly Burakov @ 2018-03-22 10:58 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, thomas, chaozhu, gowrishankar.m

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:
    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/rte_eal_version.map        |  2 +
 6 files changed, 100 insertions(+), 14 deletions(-)

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/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;
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH v5] eal: provide API for querying valid socket id's
  2018-03-22 10:58       ` [dpdk-dev] [PATCH v5] eal: provide API for querying valid socket id's Anatoly Burakov
@ 2018-03-22 11:45         ` Burakov, Anatoly
  2018-03-22 12:36         ` [dpdk-dev] [PATCH v6] " Anatoly Burakov
  1 sibling, 0 replies; 42+ messages in thread
From: Burakov, Anatoly @ 2018-03-22 11:45 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, thomas, chaozhu, gowrishankar.m

On 22-Mar-18 10:58 AM, 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>
> ---

Left out meson ABI version, will respin.

-- 
Thanks,
Anatoly

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

* [dpdk-dev] [PATCH v6] eal: provide API for querying valid socket id's
  2018-03-22 10:58       ` [dpdk-dev] [PATCH v5] eal: provide API for querying valid socket id's Anatoly Burakov
  2018-03-22 11:45         ` Burakov, Anatoly
@ 2018-03-22 12:36         ` Anatoly Burakov
  2018-03-22 17:07           ` gowrishankar muthukrishnan
                             ` (2 more replies)
  1 sibling, 3 replies; 42+ messages in thread
From: Anatoly Burakov @ 2018-03-22 12:36 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, thomas, chaozhu, gowrishankar.m

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

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;
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH v6] eal: provide API for querying valid socket id's
  2018-03-22 12:36         ` [dpdk-dev] [PATCH v6] " Anatoly Burakov
@ 2018-03-22 17:07           ` gowrishankar muthukrishnan
  2018-03-27 16:24           ` Thomas Monjalon
  2018-03-31 17:08           ` [dpdk-dev] [PATCH v7] " Anatoly Burakov
  2 siblings, 0 replies; 42+ messages in thread
From: gowrishankar muthukrishnan @ 2018-03-22 17:07 UTC (permalink / raw)
  To: Anatoly Burakov, dev; +Cc: Bruce Richardson, thomas, chaozhu

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

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

* Re: [dpdk-dev] [PATCH v6] eal: provide API for querying valid socket id's
  2018-03-22 12:36         ` [dpdk-dev] [PATCH v6] " Anatoly Burakov
  2018-03-22 17:07           ` gowrishankar muthukrishnan
@ 2018-03-27 16:24           ` Thomas Monjalon
  2018-03-31 13:35             ` Burakov, Anatoly
  2018-03-31 17:08           ` [dpdk-dev] [PATCH v7] " Anatoly Burakov
  2 siblings, 1 reply; 42+ messages in thread
From: Thomas Monjalon @ 2018-03-27 16:24 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Bruce Richardson, chaozhu, gowrishankar.m

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;

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

* Re: [dpdk-dev] [PATCH v6] eal: provide API for querying valid socket id's
  2018-03-27 16:24           ` Thomas Monjalon
@ 2018-03-31 13:35             ` Burakov, Anatoly
  2018-04-02 15:27               ` Thomas Monjalon
  0 siblings, 1 reply; 42+ messages in thread
From: Burakov, Anatoly @ 2018-03-31 13:35 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Bruce Richardson, chaozhu, gowrishankar.m

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?

-- 
Thanks,
Anatoly

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

* [dpdk-dev] [PATCH v7] eal: provide API for querying valid socket id's
  2018-03-22 12:36         ` [dpdk-dev] [PATCH v6] " Anatoly Burakov
  2018-03-22 17:07           ` gowrishankar muthukrishnan
  2018-03-27 16:24           ` Thomas Monjalon
@ 2018-03-31 17:08           ` Anatoly Burakov
  2018-04-04 22:31             ` Thomas Monjalon
  2 siblings, 1 reply; 42+ messages in thread
From: Anatoly Burakov @ 2018-03-31 17:08 UTC (permalink / raw)
  To: dev
  Cc: Neil Horman, John McNamara, Marko Kovacevic, Bruce Richardson,
	thomas, chaozhu, gowrishankar.m

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.

Also, remove deprecation notice corresponding to this change.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
---

Notes:
    v7:
    - Renamed rte_num_socket_ids() to rte_socket_count()
    - Removed deprecation notice associated with this change
    - Addressed review comments
    
    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

 doc/guides/rel_notes/deprecation.rst      |  3 --
 lib/librte_eal/bsdapp/eal/Makefile        |  2 +-
 lib/librte_eal/common/eal_common_lcore.c  | 75 ++++++++++++++++++++++++++-----
 lib/librte_eal/common/include/rte_eal.h   |  2 +
 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 +
 8 files changed, 100 insertions(+), 18 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 74c18ed..80472f5 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -38,9 +38,6 @@ Deprecation Notices
   success and failure, respectively.  This will change to 1 and 0 for true and
   false, respectively, to make use of the function more intuitive.
 
-* eal: new ``numa_node_count`` member will be added to ``rte_config`` structure
-  in v18.05.
-
 * eal: due to internal data layout reorganization, there will be changes to
   several structures and functions as a result of coming changes to support
   memory hotplug in v18.05.
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..3167e9d 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_socket_count(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..991cbe0 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -57,6 +57,8 @@ 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..7312975 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_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.
+ *
+ * @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 15d1c6a..4aa63e3 100644
--- a/lib/librte_eal/meson.build
+++ b/lib/librte_eal/meson.build
@@ -21,7 +21,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..30ec1fc 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -257,5 +257,7 @@ EXPERIMENTAL {
 	rte_service_set_runstate_mapped_check;
 	rte_service_set_stats_enable;
 	rte_service_start_with_defaults;
+	rte_socket_count;
+	rte_socket_id_by_idx;
 
 } DPDK_18.02;
-- 
2.7.4

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

* Re: [dpdk-dev] [PATCH v6] eal: provide API for querying valid socket id's
  2018-03-31 13:35             ` Burakov, Anatoly
@ 2018-04-02 15:27               ` Thomas Monjalon
  0 siblings, 0 replies; 42+ messages in thread
From: Thomas Monjalon @ 2018-04-02 15:27 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Bruce Richardson, chaozhu, gowrishankar.m

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.

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

* Re: [dpdk-dev] [PATCH v7] eal: provide API for querying valid socket id's
  2018-03-31 17:08           ` [dpdk-dev] [PATCH v7] " Anatoly Burakov
@ 2018-04-04 22:31             ` Thomas Monjalon
  0 siblings, 0 replies; 42+ messages in thread
From: Thomas Monjalon @ 2018-04-04 22:31 UTC (permalink / raw)
  To: Anatoly Burakov
  Cc: dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson, chaozhu, gowrishankar.m

31/03/2018 19:08, Anatoly Burakov:
> 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.
> 
> Also, remove deprecation notice corresponding to this change.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> ---
> 
> Notes:
>     v7:
>     - Renamed rte_num_socket_ids() to rte_socket_count()
>     - Removed deprecation notice associated with this change
>     - Addressed review comments

You forgot the release notes for the ABI version (from my previous review).

Applied and fixed.

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

end of thread, other threads:[~2018-04-04 22:31 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-22 11:58 [dpdk-dev] [PATCH] eal: add function to return number of detected sockets Anatoly Burakov
2017-12-22 12:41 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
2018-01-11 22:20   ` Thomas Monjalon
2018-01-12 11:44     ` Burakov, Anatoly
2018-01-12 11:50       ` Thomas Monjalon
2018-01-16 11:56         ` Burakov, Anatoly
2018-01-16 12:20           ` Thomas Monjalon
2018-01-16 15:05             ` Burakov, Anatoly
2018-01-16 17:34               ` Thomas Monjalon
2018-01-16 17:38                 ` Burakov, Anatoly
2018-01-16 18:26                   ` Thomas Monjalon
2018-01-16 17:53   ` [dpdk-dev] [PATCH] doc: add ABI change notice for numa_node_count in eal Anatoly Burakov
2018-01-23 10:39     ` Mcnamara, John
2018-02-07 10:10       ` Jerin Jacob
2018-02-09 14:42         ` Bruce Richardson
2018-02-14  0:04           ` Thomas Monjalon
2018-02-14 14:25             ` Thomas Monjalon
2018-02-12 16:00     ` Jonas Pfefferle
     [not found]   ` <cover.1517848624.git.anatoly.burakov@intel.com>
2018-02-05 16:37     ` [dpdk-dev] [PATCH v3] eal: add function to return number of detected sockets Anatoly Burakov
2018-02-05 17:39       ` Burakov, Anatoly
2018-02-05 22:45         ` Thomas Monjalon
2018-02-06  9:28           ` Burakov, Anatoly
2018-02-06  9:47             ` Thomas Monjalon
2018-02-07  9:58       ` [dpdk-dev] [PATCH 18.05 v4] Add " Anatoly Burakov
2018-02-07  9:58       ` [dpdk-dev] [PATCH 18.05 v4] eal: add " Anatoly Burakov
2018-03-08 12:12         ` Bruce Richardson
2018-03-08 14:38           ` Burakov, Anatoly
2018-03-09 16:32             ` Bruce Richardson
2018-03-20 22:43             ` Thomas Monjalon
2018-03-21  4:59         ` gowrishankar muthukrishnan
2018-03-21 10:24           ` Burakov, Anatoly
2018-03-22  5:16             ` gowrishankar muthukrishnan
2018-03-22  9:04               ` Burakov, Anatoly
2018-03-22 10:58       ` [dpdk-dev] [PATCH v5] eal: provide API for querying valid socket id's Anatoly Burakov
2018-03-22 11:45         ` Burakov, Anatoly
2018-03-22 12:36         ` [dpdk-dev] [PATCH v6] " Anatoly Burakov
2018-03-22 17:07           ` gowrishankar muthukrishnan
2018-03-27 16:24           ` Thomas Monjalon
2018-03-31 13:35             ` Burakov, Anatoly
2018-04-02 15:27               ` Thomas Monjalon
2018-03-31 17:08           ` [dpdk-dev] [PATCH v7] " Anatoly Burakov
2018-04-04 22:31             ` Thomas Monjalon

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