[dpdk-dev] [PATCH v1 0/5] make lcore_config internal

[dpdk-dev] [PATCH v1 0/5] make lcore_config internal

[Stephen Hemminger]

This set of patches makes the lcore_config structure
internal to EAL and not part of the ABI. It supersedes
earlier RFC.

The first 4 would go into current release, and the
last would go into later release to make it fully
internal.

The same should be done for rte_config, but that
is more difficult.

Stephen Hemminger (5):
  eal: add accessor functions for lcore_config
  bus: use lcore accessor functions
  examples/bond: use lcore accessor
  app/test: use lcore accessor functions
  eal: make lcore_config private

 app/test/test_cryptodev.c                 |  2 +-
 app/test/test_hash_readwrite_lf.c         | 14 ++---
 app/test/test_ring_perf.c                 | 22 ++++---
 app/test/test_stack_perf.c                | 20 +++---
 doc/guides/rel_notes/release_19_05.rst    |  7 +++
 drivers/bus/dpaa/dpaa_bus.c               |  6 +-
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c  |  4 +-
 examples/bond/main.c                      |  4 +-
 lib/librte_eal/common/eal_common_launch.c |  2 +
 lib/librte_eal/common/eal_common_lcore.c  | 39 ++++++++++++
 lib/librte_eal/common/eal_private.h       | 22 +++++++
 lib/librte_eal/common/include/rte_lcore.h | 76 +++++++++++------------
 lib/librte_eal/common/rte_service.c       |  2 +
 lib/librte_eal/rte_eal_version.map        | 12 +++-
 14 files changed, 159 insertions(+), 73 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v1 1/5] eal: add accessor functions for lcore_config

[Stephen Hemminger]

The fields of the internal EAL core configuration are currently
laid bare as part of the API. This is not good practice and limits
fixing issues with layout and sizes.

Make new accessor functions for the fields used by current drivers
and examples. Mark the state and return code functions as experimental
since these values might change in the future and probably shouldn't
have been used by non EAL code anyway.

Most of these functions are not marked as experimental since
we want applications to convert over asap. The one exception is
the return_code, which is only used by some tests now and not
clear that it is even that useful.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_19_05.rst    |  7 +++
 lib/librte_eal/common/eal_common_lcore.c  | 39 +++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 52 ++++++++++++++++-------
 lib/librte_eal/rte_eal_version.map        | 11 +++++
 4 files changed, 94 insertions(+), 15 deletions(-)

diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst
index dbdf07a0c05b..4beea5705be7 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -222,6 +222,13 @@ ABI Changes
   alignment for ``rte_crypto_asym_op`` to restore expected ``rte_crypto_op``
   layout and alignment.
 
+* eal: the lcore config structure ``struct lcore_config`` will be made
+  internal to the EAL in a future release. This will allow the structure to
+  change without impacting API or ABI. All accesses to fields of this
+  structure should be done by the corresponding accessor functions.
+  For example, instead of using ``lcore_config[lcore_id].socket_id``
+  the function ``rte_lcore_socket_id(lcore_id)`` should be used instead.
+
 
 Shared Library Versions
 -----------------------
diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 1cbac42286ba..c3cf5a06269d 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -16,6 +16,45 @@
 #include "eal_private.h"
 #include "eal_thread.h"
 
+int rte_lcore_index(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_index;
+}
+
+int rte_lcore_to_cpu_id(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_id;
+}
+
+rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].cpuset;
+}
+
+unsigned
+rte_lcore_to_socket_id(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].socket_id;
+}
+
+int
+rte_lcore_return_code(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].ret;
+}
+
 static int
 socket_id_cmp(const void *a, const void *b)
 {
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index dea17f500065..7687fe650f64 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -121,15 +121,8 @@ rte_lcore_count(void)
  * @return
  *   The relative index, or -1 if not enabled.
  */
-static inline int
-rte_lcore_index(int lcore_id)
-{
-	if (lcore_id >= RTE_MAX_LCORE)
-		return -1;
-	if (lcore_id < 0)
-		lcore_id = (int)rte_lcore_id();
-	return lcore_config[lcore_id].core_index;
-}
+int
+rte_lcore_index(int lcore_id);
 
 /**
  * Return the ID of the physical socket of the logical core we are
@@ -177,11 +170,40 @@ rte_socket_id_by_idx(unsigned int idx);
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned
-rte_lcore_to_socket_id(unsigned lcore_id)
-{
-	return lcore_config[lcore_id].socket_id;
-}
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id);
+
+/**
+ * Return the id of the lcore on a socket starting from zero.
+ *
+ * @param lcore_id
+ *   The targeted lcore, or -1 for the current one.
+ * @return
+ *   The relative index, or -1 if not enabled.
+ */
+int
+rte_lcore_to_cpu_id(int lcore_id);
+
+/**
+ * Return the cpuset for a given lcore.
+ * @param lcore_id
+ *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
+ * @return
+ *   The cpuset of that lcore
+ */
+rte_cpuset_t
+rte_lcore_cpuset(unsigned int lcore_id);
+
+/**
+ * Get the return code from a lcore thread.
+ * @param lcore_id
+ *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1
+ *   and finished
+ * @return
+ *   the return code from the lcore thread
+ */
+int __rte_experimental
+rte_lcore_return_code(unsigned int lcore_id);
 
 /**
  * Test if an lcore is enabled.
@@ -193,7 +215,7 @@ rte_lcore_to_socket_id(unsigned lcore_id)
  *   True if the given lcore is enabled; false otherwise.
  */
 static inline int
-rte_lcore_is_enabled(unsigned lcore_id)
+rte_lcore_is_enabled(unsigned int lcore_id)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
 	if (lcore_id >= RTE_MAX_LCORE)
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index d6e375135ad1..f6688327cad3 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -268,6 +268,16 @@ DPDK_18.11 {
 
 } DPDK_18.08;
 
+DPDK_19.05 {
+	global:
+
+	rte_lcore_cpuset;
+	rte_lcore_index;
+	rte_lcore_to_cpu_id;
+	rte_lcore_to_socket_id;
+
+} DPDK_18.08;
+
 EXPERIMENTAL {
 	global:
 
@@ -329,6 +339,7 @@ EXPERIMENTAL {
 	rte_fbarray_set_free;
 	rte_fbarray_set_used;
 	rte_intr_callback_unregister_pending;
+	rte_lcore_return_code;
 	rte_log_register_type_and_pick_level;
 	rte_malloc_dump_heaps;
 	rte_malloc_heap_create;
-- 
2.17.1

[dpdk-dev] [PATCH v1 2/5] bus: use lcore accessor functions

[Stephen Hemminger]

Fix the bus logic in fslmc and dpaa drivers to use the new
accessor functions.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/bus/dpaa/dpaa_bus.c              | 6 ++++--
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index ac20eccd53c1..08c822781d9d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -254,6 +254,7 @@ int rte_dpaa_portal_init(void *arg)
 	unsigned int cpu, lcore = rte_lcore_id();
 	int ret;
 	struct dpaa_portal *dpaa_io_portal;
+	rte_cpuset_t cpuset;
 
 	BUS_INIT_FUNC_TRACE();
 
@@ -263,12 +264,13 @@ int rte_dpaa_portal_init(void *arg)
 		if (lcore >= RTE_MAX_LCORE)
 			return -1;
 
-	cpu = lcore_config[lcore].core_id;
+	cpu = rte_lcore_to_cpu_id(lcore);
 
 	/* Set CPU affinity for this thread.*/
 	id = pthread_self();
+	cpuset = rte_lcore_cpuset(lcore);
 	ret = pthread_setaffinity_np(id, sizeof(cpu_set_t),
-			&lcore_config[lcore].cpuset);
+				     &cpuset);
 	if (ret) {
 		DPAA_BUS_LOG(ERR, "pthread_setaffinity_np failed on core :%u"
 			     " (lcore=%u) with ret: %d", cpu, lcore, ret);
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index 7bcbde840e63..8efb24af5c6a 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -366,7 +366,9 @@ dpaa2_check_lcore_cpuset(void)
 
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		for (i = 0; i < RTE_MAX_LCORE; i++) {
-			if (CPU_ISSET(i, &lcore_config[lcore_id].cpuset)) {
+			rte_cpuset_t cpuset = rte_lcore_cpuset(lcore_id);
+
+			if (CPU_ISSET(i, &cpuset)) {
 				RTE_LOG(DEBUG, EAL, "lcore id = %u cpu=%u\n",
 					lcore_id, i);
 				if (dpaa2_cpu[lcore_id] != 0xffffffff) {
-- 
2.17.1

[dpdk-dev] [PATCH v1 3/5] examples/bond: use lcore accessor

[Stephen Hemminger]

Use the accessor function to look at lcore state.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/bond/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/bond/main.c b/examples/bond/main.c
index ef86194fff4a..48e6755ea294 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -526,8 +526,8 @@ static void cmd_start_parsed(__attribute__((unused)) void *parsed_result,
 	int slave_core_id = rte_lcore_id();
 
 	rte_spinlock_trylock(&global_flag_stru_p->lock);
-	if (global_flag_stru_p->LcoreMainIsRunning == 0)	{
-		if (lcore_config[global_flag_stru_p->LcoreMainCore].state != WAIT)	{
+	if (global_flag_stru_p->LcoreMainIsRunning == 0) {
+		if (rte_lcore_state(global_flag_stru_p->LcoreMainCore) != WAIT) {
 			rte_spinlock_unlock(&global_flag_stru_p->lock);
 			return;
 		}
-- 
2.17.1

[dpdk-dev] [PATCH v1 4/5] app/test: use lcore accessor functions

[Stephen Hemminger]

Use the new accessor functions.  Don't refer to lcore_config directly.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_cryptodev.c         |  2 +-
 app/test/test_hash_readwrite_lf.c | 14 +++++++-------
 app/test/test_ring_perf.c         | 22 ++++++++++++----------
 app/test/test_stack_perf.c        | 20 ++++++++++----------
 4 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9f31aaa7e6b2..eca6d3db16a5 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -378,7 +378,7 @@ testsuite_setup(void)
 			strcpy(temp_str, vdev_args);
 			strlcat(temp_str, ";", sizeof(temp_str));
 			slave_core_count++;
-			socket_id = lcore_config[i].socket_id;
+			socket_id = rte_lcore_to_socket_id(i);
 		}
 		if (slave_core_count != 2) {
 			RTE_LOG(ERR, USER1,
diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 4ab4c8ee64cf..1361a8aa4c9c 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -741,7 +741,7 @@ test_hash_add_no_ks_lookup_hit(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			rte_eal_mp_wait_lcore();
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -813,7 +813,7 @@ test_hash_add_no_ks_lookup_miss(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -889,7 +889,7 @@ test_hash_add_ks_lookup_hit_non_sp(struct rwc_perf *rwc_perf_results,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -965,7 +965,7 @@ test_hash_add_ks_lookup_hit_sp(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1040,7 +1040,7 @@ test_hash_add_ks_lookup_miss(struct rwc_perf *rwc_perf_results, int rwc_lf, int
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1141,7 +1141,7 @@ test_hash_multi_add_lookup(struct rwc_perf *rwc_perf_results, int rwc_lf,
 				rte_eal_mp_wait_lcore();
 
 				for (i = 1; i <= rwc_core_cnt[n]; i++)
-					if (lcore_config[i].ret < 0)
+					if (rte_lcore_return_code(i) < 0)
 						goto err;
 
 				unsigned long long cycles_per_lookup =
@@ -1225,7 +1225,7 @@ test_hash_add_ks_lookup_hit_extbkt(struct rwc_perf *rwc_perf_results,
 			rte_eal_mp_wait_lcore();
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
index ebb3939f51d0..6eccccfe93b4 100644
--- a/app/test/test_ring_perf.c
+++ b/app/test/test_ring_perf.c
@@ -52,10 +52,11 @@ get_two_hyperthreads(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 == c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -75,10 +76,11 @@ get_two_cores(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 != c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -98,8 +100,8 @@ get_two_sockets(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if (s1 != s2){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
diff --git a/app/test/test_stack_perf.c b/app/test/test_stack_perf.c
index ba27fbf7076d..70561fecda0d 100644
--- a/app/test/test_stack_perf.c
+++ b/app/test/test_stack_perf.c
@@ -44,10 +44,10 @@ get_two_hyperthreads(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] == core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -70,10 +70,10 @@ get_two_cores(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] != core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -95,8 +95,8 @@ get_two_sockets(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if (socket[0] != socket[1]) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
-- 
2.17.1

[dpdk-dev] [PATCH v1 5/5] eal: make lcore_config private

[Stephen Hemminger]

The internal structure of lcore_config should not be part of
visible API/ABI. Make it private to EAL.

Rearrange and resize the fields in the structure so it takes
less memory (and cache footprint).

THIS BREAKS ABI OF PREVIOUS RELEASES.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_launch.c |  2 ++
 lib/librte_eal/common/eal_private.h       | 22 +++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 24 -----------------------
 lib/librte_eal/common/rte_service.c       |  2 ++
 lib/librte_eal/rte_eal_version.map        |  1 -
 5 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_launch.c b/lib/librte_eal/common/eal_common_launch.c
index fe0ba3f0d617..cf52d717f68e 100644
--- a/lib/librte_eal/common/eal_common_launch.c
+++ b/lib/librte_eal/common/eal_common_launch.c
@@ -15,6 +15,8 @@
 #include <rte_per_lcore.h>
 #include <rte_lcore.h>
 
+#include "eal_private.h"
+
 /*
  * Wait until a lcore finished its job.
  */
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 798ede553b21..25e80547904f 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -10,6 +10,28 @@
 #include <stdio.h>
 
 #include <rte_dev.h>
+#include <rte_lcore.h>
+
+/**
+ * Structure storing internal configuration (per-lcore)
+ */
+struct lcore_config {
+	uint32_t core_id;      /**< core number on socket for this lcore */
+	uint32_t core_index;   /**< relative index, starting from 0 */
+	uint16_t socket_id;    /**< physical socket id for this lcore */
+	uint8_t core_role;         /**< role of core eg: OFF, RTE, SERVICE */
+	uint8_t detected;          /**< true if lcore was detected */
+	volatile enum rte_lcore_state_t state; /**< lcore state */
+	rte_cpuset_t cpuset;       /**< cpu set which the lcore affinity to */
+	pthread_t thread_id;       /**< pthread identifier */
+	int pipe_master2slave[2];  /**< communication pipe with master */
+	int pipe_slave2master[2];  /**< communication pipe with master */
+	lcore_function_t * volatile f;         /**< function to call */
+	void * volatile arg;       /**< argument of function */
+	volatile int ret;          /**< return value of function */
+};
+
+extern struct lcore_config lcore_config[RTE_MAX_LCORE];
 
 /**
  * Initialize the memzone subsystem (private to eal).
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 7687fe650f64..bc416bc8c19f 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -37,30 +37,6 @@ typedef cpuset_t rte_cpuset_t;
 } while (0)
 #endif
 
-/**
- * Structure storing internal configuration (per-lcore)
- */
-struct lcore_config {
-	unsigned detected;         /**< true if lcore was detected */
-	pthread_t thread_id;       /**< pthread identifier */
-	int pipe_master2slave[2];  /**< communication pipe with master */
-	int pipe_slave2master[2];  /**< communication pipe with master */
-	lcore_function_t * volatile f;         /**< function to call */
-	void * volatile arg;       /**< argument of function */
-	volatile int ret;          /**< return value of function */
-	volatile enum rte_lcore_state_t state; /**< lcore state */
-	unsigned socket_id;        /**< physical socket id for this lcore */
-	unsigned core_id;          /**< core number on socket for this lcore */
-	int core_index;            /**< relative index, starting from 0 */
-	rte_cpuset_t cpuset;       /**< cpu set which the lcore affinity to */
-	uint8_t core_role;         /**< role of core eg: OFF, RTE, SERVICE */
-};
-
-/**
- * Internal configuration (per-lcore)
- */
-extern struct lcore_config lcore_config[RTE_MAX_LCORE];
-
 RTE_DECLARE_PER_LCORE(unsigned, _lcore_id);  /**< Per thread "lcore id". */
 RTE_DECLARE_PER_LCORE(rte_cpuset_t, _cpuset); /**< Per thread "cpuset". */
 
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 5f75e5a53fbf..8d53d966446a 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -21,6 +21,8 @@
 #include <rte_memory.h>
 #include <rte_malloc.h>
 
+#include "eal_private.h"
+
 #define RTE_SERVICE_NUM_MAX 64
 
 #define SERVICE_F_REGISTERED    (1 << 0)
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index f6688327cad3..2f175f58ed42 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -4,7 +4,6 @@ DPDK_2.0 {
 	__rte_panic;
 	eal_parse_sysfs_value;
 	eal_timer_source;
-	lcore_config;
 	per_lcore__lcore_id;
 	per_lcore__rte_errno;
 	rte_calloc;
-- 
2.17.1

Re: [dpdk-dev] [PATCH v1 1/5] eal: add accessor functions for lcore_config

[David Marchand]

On Mon, Apr 8, 2019 at 8:25 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> The fields of the internal EAL core configuration are currently
> laid bare as part of the API. This is not good practice and limits
> fixing issues with layout and sizes.
>
> Make new accessor functions for the fields used by current drivers
> and examples. Mark the state and return code functions as experimental
> since these values might change in the future and probably shouldn't
> have been used by non EAL code anyway.
>

You removed the state function, no need to mention it.

Since we already expose the state, exposing the ret code seems fair.
Applications can wait themselves and check the return code the way they
want.

Just want to point that the current test app could do without it, since it
is quite simple, it waits for all, then checks all return codes.



> Most of these functions are not marked as experimental since
> we want applications to convert over asap. The one exception is
> the return_code, which is only used by some tests now and not
> clear that it is even that useful.
>

+1


diff --git a/lib/librte_eal/common/eal_common_lcore.c
> b/lib/librte_eal/common/eal_common_lcore.c
> index 1cbac42286ba..c3cf5a06269d 100644
> --- a/lib/librte_eal/common/eal_common_lcore.c
> +++ b/lib/librte_eal/common/eal_common_lcore.c
> @@ -16,6 +16,45 @@
>  #include "eal_private.h"
>  #include "eal_thread.h"
>
> +int rte_lcore_index(int lcore_id)
> +{
> +       if (unlikely(lcore_id >= RTE_MAX_LCORE))
> +               return -1;
> +
> +       if (lcore_id < 0)
> +               lcore_id = (int)rte_lcore_id();
> +
> +       return lcore_config[lcore_id].core_index;
> +}
> +
> +int rte_lcore_to_cpu_id(int lcore_id)
> +{
> +       if (unlikely(lcore_id >= RTE_MAX_LCORE))
> +               return -1;
> +
> +       if (lcore_id < 0)
> +               lcore_id = (int)rte_lcore_id();
> +
> +       return lcore_config[lcore_id].core_id;
> +}
> +
> +rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id)
> +{
> +       return lcore_config[lcore_id].cpuset;
> +}
> +
> +unsigned
>

Ah, checkpatch does not complain for me, I would have expected a warning on
"unsigned int".
Can you fix it so that it matches the declaration from the header ?

+rte_lcore_to_socket_id(unsigned int lcore_id)
> +{
> +       return lcore_config[lcore_id].socket_id;
> +}
> +
> +int
> +rte_lcore_return_code(unsigned int lcore_id)
> +{
> +       return lcore_config[lcore_id].ret;
> +}
> +
>  static int
>  socket_id_cmp(const void *a, const void *b)
>  {



The rest looks fine.
Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

Re: [dpdk-dev] [PATCH v1 2/5] bus: use lcore accessor functions

[David Marchand]

On Mon, Apr 8, 2019 at 8:25 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> Fix the bus logic in fslmc and dpaa drivers to use the new
> accessor functions.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/bus/dpaa/dpaa_bus.c              | 6 ++++--
>  drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 4 +++-
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
> index ac20eccd53c1..08c822781d9d 100644
> --- a/drivers/bus/dpaa/dpaa_bus.c
> +++ b/drivers/bus/dpaa/dpaa_bus.c
> @@ -254,6 +254,7 @@ int rte_dpaa_portal_init(void *arg)
>         unsigned int cpu, lcore = rte_lcore_id();
>         int ret;
>         struct dpaa_portal *dpaa_io_portal;
> +       rte_cpuset_t cpuset;
>
>         BUS_INIT_FUNC_TRACE();
>
> @@ -263,12 +264,13 @@ int rte_dpaa_portal_init(void *arg)
>                 if (lcore >= RTE_MAX_LCORE)
>                         return -1;
>
> -       cpu = lcore_config[lcore].core_id;
> +       cpu = rte_lcore_to_cpu_id(lcore);
>
>         /* Set CPU affinity for this thread.*/
>         id = pthread_self();
> +       cpuset = rte_lcore_cpuset(lcore);
>         ret = pthread_setaffinity_np(id, sizeof(cpu_set_t),
> -                       &lcore_config[lcore].cpuset);
> +                                    &cpuset);
>         if (ret) {
>                 DPAA_BUS_LOG(ERR, "pthread_setaffinity_np failed on core
> :%u"
>                              " (lcore=%u) with ret: %d", cpu, lcore, ret);
> diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
> b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
> index 7bcbde840e63..8efb24af5c6a 100644
> --- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
> +++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
> @@ -366,7 +366,9 @@ dpaa2_check_lcore_cpuset(void)
>
>         for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
>                 for (i = 0; i < RTE_MAX_LCORE; i++) {
> -                       if (CPU_ISSET(i, &lcore_config[lcore_id].cpuset)) {
> +                       rte_cpuset_t cpuset = rte_lcore_cpuset(lcore_id);
> +
> +                       if (CPU_ISSET(i, &cpuset)) {
>                                 RTE_LOG(DEBUG, EAL, "lcore id = %u
> cpu=%u\n",
>                                         lcore_id, i);
>                                 if (dpaa2_cpu[lcore_id] != 0xffffffff) {
> --
> 2.17.1
>
>
Reviewed-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand

Re: [dpdk-dev] [PATCH v1 3/5] examples/bond: use lcore accessor

[David Marchand]

On Mon, Apr 8, 2019 at 8:25 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> Use the accessor function to look at lcore state.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  examples/bond/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/examples/bond/main.c b/examples/bond/main.c
> index ef86194fff4a..48e6755ea294 100644
> --- a/examples/bond/main.c
> +++ b/examples/bond/main.c
> @@ -526,8 +526,8 @@ static void cmd_start_parsed(__attribute__((unused))
> void *parsed_result,
>         int slave_core_id = rte_lcore_id();
>
>         rte_spinlock_trylock(&global_flag_stru_p->lock);
> -       if (global_flag_stru_p->LcoreMainIsRunning == 0)        {
> -               if (lcore_config[global_flag_stru_p->LcoreMainCore].state
> != WAIT)      {
> +       if (global_flag_stru_p->LcoreMainIsRunning == 0) {
> +               if (rte_lcore_state(global_flag_stru_p->LcoreMainCore) !=
> WAIT) {
>

s/rte_lcore_state/rte_eal_get_lcore_state/

                        rte_spinlock_unlock(&global_flag_stru_p->lock);
>                         return;
>                 }
> --
> 2.17.1
>
>
And you missed another reference in this file:

diff --git a/examples/bond/main.c b/examples/bond/main.c
index 48e6755..982b7ea 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -527,7 +527,7 @@ static void cmd_start_parsed(__attribute__((unused))
void *parsed_result,

        rte_spinlock_trylock(&global_flag_stru_p->lock);
        if (global_flag_stru_p->LcoreMainIsRunning == 0) {
-               if (rte_lcore_state(global_flag_stru_p->LcoreMainCore) !=
WAIT) {
+               if
(rte_eal_get_lcore_state(global_flag_stru_p->LcoreMainCore) != WAIT) {
                        rte_spinlock_unlock(&global_flag_stru_p->lock);
                        return;
                }
@@ -796,7 +796,7 @@ static void prompt(__attribute__((unused)) void *arg1)

        /* check state of lcores */
        RTE_LCORE_FOREACH_SLAVE(slave_core_id) {
-       if (lcore_config[slave_core_id].state != WAIT)
+       if (rte_eal_get_lcore_state(slave_core_id) != WAIT)
                return -EBUSY;
        }
        /* start lcore main on core != master_core - ARP response thread */


-- 
David Marchand

Re: [dpdk-dev] [PATCH v1 4/5] app/test: use lcore accessor functions

[David Marchand]

On Mon, Apr 8, 2019 at 8:26 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> Use the new accessor functions.  Don't refer to lcore_config directly.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  app/test/test_cryptodev.c         |  2 +-
>  app/test/test_hash_readwrite_lf.c | 14 +++++++-------
>  app/test/test_ring_perf.c         | 22 ++++++++++++----------
>  app/test/test_stack_perf.c        | 20 ++++++++++----------
>  4 files changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index 9f31aaa7e6b2..eca6d3db16a5 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -378,7 +378,7 @@ testsuite_setup(void)
>                         strcpy(temp_str, vdev_args);
>                         strlcat(temp_str, ";", sizeof(temp_str));
>                         slave_core_count++;
> -                       socket_id = lcore_config[i].socket_id;
> +                       socket_id = rte_lcore_to_socket_id(i);
>                 }
>                 if (slave_core_count != 2) {
>                         RTE_LOG(ERR, USER1,
> diff --git a/app/test/test_hash_readwrite_lf.c
> b/app/test/test_hash_readwrite_lf.c
> index 4ab4c8ee64cf..1361a8aa4c9c 100644
> --- a/app/test/test_hash_readwrite_lf.c
> +++ b/app/test/test_hash_readwrite_lf.c
> @@ -741,7 +741,7 @@ test_hash_add_no_ks_lookup_hit(struct rwc_perf
> *rwc_perf_results, int rwc_lf,
>                         rte_eal_mp_wait_lcore();
>
>                         for (i = 1; i <= rwc_core_cnt[n]; i++)
> -                               if (lcore_config[i].ret < 0)
> +                               if (rte_lcore_return_code(i) < 0)
>                                         goto err;
>
>                         unsigned long long cycles_per_lookup =
> @@ -813,7 +813,7 @@ test_hash_add_no_ks_lookup_miss(struct rwc_perf
> *rwc_perf_results, int rwc_lf,
>                         if (ret < 0)
>                                 goto err;
>                         for (i = 1; i <= rwc_core_cnt[n]; i++)
> -                               if (lcore_config[i].ret < 0)
> +                               if (rte_lcore_return_code(i) < 0)
>                                         goto err;
>
>                         unsigned long long cycles_per_lookup =
> @@ -889,7 +889,7 @@ test_hash_add_ks_lookup_hit_non_sp(struct rwc_perf
> *rwc_perf_results,
>                         if (ret < 0)
>                                 goto err;
>                         for (i = 1; i <= rwc_core_cnt[n]; i++)
> -                               if (lcore_config[i].ret < 0)
> +                               if (rte_lcore_return_code(i) < 0)
>                                         goto err;
>
>                         unsigned long long cycles_per_lookup =
> @@ -965,7 +965,7 @@ test_hash_add_ks_lookup_hit_sp(struct rwc_perf
> *rwc_perf_results, int rwc_lf,
>                         if (ret < 0)
>                                 goto err;
>                         for (i = 1; i <= rwc_core_cnt[n]; i++)
> -                               if (lcore_config[i].ret < 0)
> +                               if (rte_lcore_return_code(i) < 0)
>                                         goto err;
>
>                         unsigned long long cycles_per_lookup =
> @@ -1040,7 +1040,7 @@ test_hash_add_ks_lookup_miss(struct rwc_perf
> *rwc_perf_results, int rwc_lf, int
>                         if (ret < 0)
>                                 goto err;
>                         for (i = 1; i <= rwc_core_cnt[n]; i++)
> -                               if (lcore_config[i].ret < 0)
> +                               if (rte_lcore_return_code(i) < 0)
>                                         goto err;
>
>                         unsigned long long cycles_per_lookup =
> @@ -1141,7 +1141,7 @@ test_hash_multi_add_lookup(struct rwc_perf
> *rwc_perf_results, int rwc_lf,
>                                 rte_eal_mp_wait_lcore();
>
>                                 for (i = 1; i <= rwc_core_cnt[n]; i++)
> -                                       if (lcore_config[i].ret < 0)
> +                                       if (rte_lcore_return_code(i) < 0)
>                                                 goto err;
>
>                                 unsigned long long cycles_per_lookup =
> @@ -1225,7 +1225,7 @@ test_hash_add_ks_lookup_hit_extbkt(struct rwc_perf
> *rwc_perf_results,
>                         rte_eal_mp_wait_lcore();
>
>                         for (i = 1; i <= rwc_core_cnt[n]; i++)
> -                               if (lcore_config[i].ret < 0)
> +                               if (rte_lcore_return_code(i) < 0)
>                                         goto err;
>
>                         unsigned long long cycles_per_lookup =
> diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
> index ebb3939f51d0..6eccccfe93b4 100644
> --- a/app/test/test_ring_perf.c
> +++ b/app/test/test_ring_perf.c
> @@ -52,10 +52,11 @@ get_two_hyperthreads(struct lcore_pair *lcp)
>                 RTE_LCORE_FOREACH(id2) {
>                         if (id1 == id2)
>                                 continue;
> -                       c1 = lcore_config[id1].core_id;
> -                       c2 = lcore_config[id2].core_id;
> -                       s1 = lcore_config[id1].socket_id;
> -                       s2 = lcore_config[id2].socket_id;
> +
> +                       c1 = rte_lcore_to_cpu_id(id1);
> +                       c2 = rte_lcore_to_cpu_id(id2);
> +                       s1 = rte_lcore_to_socket_id(id1);
> +                       s2 = rte_lcore_to_socket_id(id2);
>                         if ((c1 == c2) && (s1 == s2)){
>                                 lcp->c1 = id1;
>                                 lcp->c2 = id2;
> @@ -75,10 +76,11 @@ get_two_cores(struct lcore_pair *lcp)
>                 RTE_LCORE_FOREACH(id2) {
>                         if (id1 == id2)
>                                 continue;
> -                       c1 = lcore_config[id1].core_id;
> -                       c2 = lcore_config[id2].core_id;
> -                       s1 = lcore_config[id1].socket_id;
> -                       s2 = lcore_config[id2].socket_id;
> +
> +                       c1 = rte_lcore_to_cpu_id(id1);
> +                       c2 = rte_lcore_to_cpu_id(id2);
> +                       s1 = rte_lcore_to_socket_id(id1);
> +                       s2 = rte_lcore_to_socket_id(id2);
>                         if ((c1 != c2) && (s1 == s2)){
>                                 lcp->c1 = id1;
>                                 lcp->c2 = id2;
> @@ -98,8 +100,8 @@ get_two_sockets(struct lcore_pair *lcp)
>                 RTE_LCORE_FOREACH(id2) {
>                         if (id1 == id2)
>                                 continue;
> -                       s1 = lcore_config[id1].socket_id;
> -                       s2 = lcore_config[id2].socket_id;
> +                       s1 = rte_lcore_to_socket_id(id1);
> +                       s2 = rte_lcore_to_socket_id(id2);
>                         if (s1 != s2){
>                                 lcp->c1 = id1;
>                                 lcp->c2 = id2;
> diff --git a/app/test/test_stack_perf.c b/app/test/test_stack_perf.c
> index ba27fbf7076d..70561fecda0d 100644
> --- a/app/test/test_stack_perf.c
> +++ b/app/test/test_stack_perf.c
> @@ -44,10 +44,10 @@ get_two_hyperthreads(struct lcore_pair *lcp)
>                 RTE_LCORE_FOREACH(id[1]) {
>                         if (id[0] == id[1])
>                                 continue;
> -                       core[0] = lcore_config[id[0]].core_id;
> -                       core[1] = lcore_config[id[1]].core_id;
> -                       socket[0] = lcore_config[id[0]].socket_id;
> -                       socket[1] = lcore_config[id[1]].socket_id;
> +                       core[0] = rte_lcore_to_cpu_id(id[0]);
> +                       core[1] = rte_lcore_to_cpu_id(id[1]);
> +                       socket[0] = rte_lcore_to_socket_id(id[0]);
> +                       socket[1] = rte_lcore_to_socket_id(id[1]);
>                         if ((core[0] == core[1]) && (socket[0] ==
> socket[1])) {
>                                 lcp->c1 = id[0];
>                                 lcp->c2 = id[1];
> @@ -70,10 +70,10 @@ get_two_cores(struct lcore_pair *lcp)
>                 RTE_LCORE_FOREACH(id[1]) {
>                         if (id[0] == id[1])
>                                 continue;
> -                       core[0] = lcore_config[id[0]].core_id;
> -                       core[1] = lcore_config[id[1]].core_id;
> -                       socket[0] = lcore_config[id[0]].socket_id;
> -                       socket[1] = lcore_config[id[1]].socket_id;
> +                       core[0] = rte_lcore_to_cpu_id(id[0]);
> +                       core[1] = rte_lcore_to_cpu_id(id[1]);
> +                       socket[0] = rte_lcore_to_socket_id(id[0]);
> +                       socket[1] = rte_lcore_to_socket_id(id[1]);
>                         if ((core[0] != core[1]) && (socket[0] ==
> socket[1])) {
>                                 lcp->c1 = id[0];
>                                 lcp->c2 = id[1];
> @@ -95,8 +95,8 @@ get_two_sockets(struct lcore_pair *lcp)
>                 RTE_LCORE_FOREACH(id[1]) {
>                         if (id[0] == id[1])
>                                 continue;
> -                       socket[0] = lcore_config[id[0]].socket_id;
> -                       socket[1] = lcore_config[id[1]].socket_id;
> +                       socket[0] = rte_lcore_to_socket_id(id[0]);
> +                       socket[1] = rte_lcore_to_socket_id(id[1]);
>                         if (socket[0] != socket[1]) {
>                                 lcp->c1 = id[0];
>                                 lcp->c2 = id[1];
> --
> 2.17.1
>

Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

[dpdk-dev] [PATCH v2 0/5] make lcore_config internal

[Stephen Hemminger]

This set of patches makes the lcore_config structure less visible
as part of the ABI.  This version does not break the ABI (yet)
follow on patch moves lcore_config into eal_private.h

The changes in v2 is to:
 - new patch to use unsigned int in lcore.h first
 - incorporate feedback from David
 - don't include last patch to make it private
	(to avoid accidental early merge)
	
Stephen Hemminger (5):
  eal: use unsigned int in rte_lcore.h functions
  eal: add accessor functions for lcore_config
  bus: use lcore accessor functions
  examples/bond: use lcore accessor
  app/test: use lcore accessor functions

 app/test/test_cryptodev.c                 |  2 +-
 app/test/test_hash_readwrite_lf.c         | 14 +++---
 app/test/test_ring_perf.c                 | 22 +++++----
 app/test/test_stack_perf.c                | 20 ++++----
 doc/guides/rel_notes/release_19_05.rst    |  6 +++
 drivers/bus/dpaa/dpaa_bus.c               |  6 ++-
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c  |  4 +-
 examples/bond/main.c                      |  5 +-
 lib/librte_eal/common/eal_common_lcore.c  | 39 +++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 58 ++++++++++++++++-------
 lib/librte_eal/rte_eal_version.map        | 11 +++++
 11 files changed, 136 insertions(+), 51 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v2 1/5] eal: use unsigned int in rte_lcore.h functions

[Stephen Hemminger]

Purely cosmetic change, use unsigned int instead of unsigned alone.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/include/rte_lcore.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index dea17f500065..959ef9ece4b2 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -137,7 +137,7 @@ rte_lcore_index(int lcore_id)
  * @return
  *   the ID of current lcoreid's physical socket
  */
-unsigned rte_socket_id(void);
+unsigned int rte_socket_id(void);
 
 /**
  * Return number of physical sockets detected on the system.
@@ -177,8 +177,8 @@ rte_socket_id_by_idx(unsigned int idx);
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned
-rte_lcore_to_socket_id(unsigned lcore_id)
+static inline unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
 {
 	return lcore_config[lcore_id].socket_id;
 }
@@ -193,7 +193,7 @@ rte_lcore_to_socket_id(unsigned lcore_id)
  *   True if the given lcore is enabled; false otherwise.
  */
 static inline int
-rte_lcore_is_enabled(unsigned lcore_id)
+rte_lcore_is_enabled(unsigned int lcore_id)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
 	if (lcore_id >= RTE_MAX_LCORE)
@@ -214,8 +214,8 @@ rte_lcore_is_enabled(unsigned lcore_id)
  * @return
  *   The next lcore_id or RTE_MAX_LCORE if not found.
  */
-static inline unsigned
-rte_get_next_lcore(unsigned i, int skip_master, int wrap)
+static inline unsigned int
+rte_get_next_lcore(unsigned int i, int skip_master, int wrap)
 {
 	i++;
 	if (wrap)
-- 
2.17.1

[dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for lcore_config

[Stephen Hemminger]

The fields of the internal EAL core configuration are currently
laid bare as part of the API. This is not good practice and limits
fixing issues with layout and sizes.

Make new accessor functions for the fields used by current drivers
and examples. Mark return code functions as experimental
since this value might change in the future and probably shouldn't
have been used by non EAL code anyway.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 doc/guides/rel_notes/release_19_05.rst    |  6 +++
 lib/librte_eal/common/eal_common_lcore.c  | 39 ++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 50 ++++++++++++++++-------
 lib/librte_eal/rte_eal_version.map        | 11 +++++
 4 files changed, 92 insertions(+), 14 deletions(-)

diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst
index dbdf07a0c05b..32aae5d3bcfa 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -222,6 +222,12 @@ ABI Changes
   alignment for ``rte_crypto_asym_op`` to restore expected ``rte_crypto_op``
   layout and alignment.
 
+* eal: the lcore config structure ``struct lcore_config`` will be made
+  internal to the EAL in a future release. This will allow the structure to
+  change without impacting API or ABI. All accesses to fields of this
+  structure should be done by the corresponding accessor functions.
+  For example, instead of using ``lcore_config[lcore_id].socket_id``
+  the function ``rte_lcore_socket_id(lcore_id)`` should be used instead.
 
 Shared Library Versions
 -----------------------
diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 1cbac42286ba..6cf4d7abb0bd 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -16,6 +16,45 @@
 #include "eal_private.h"
 #include "eal_thread.h"
 
+int rte_lcore_index(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_index;
+}
+
+int rte_lcore_to_cpu_id(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_id;
+}
+
+rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].cpuset;
+}
+
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].socket_id;
+}
+
+int
+rte_lcore_return_code(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].ret;
+}
+
 static int
 socket_id_cmp(const void *a, const void *b)
 {
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 959ef9ece4b2..dc9f3dc0843d 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -121,15 +121,8 @@ rte_lcore_count(void)
  * @return
  *   The relative index, or -1 if not enabled.
  */
-static inline int
-rte_lcore_index(int lcore_id)
-{
-	if (lcore_id >= RTE_MAX_LCORE)
-		return -1;
-	if (lcore_id < 0)
-		lcore_id = (int)rte_lcore_id();
-	return lcore_config[lcore_id].core_index;
-}
+int __rte_experimental
+rte_lcore_index(int lcore_id);
 
 /**
  * Return the ID of the physical socket of the logical core we are
@@ -177,11 +170,40 @@ rte_socket_id_by_idx(unsigned int idx);
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned int
-rte_lcore_to_socket_id(unsigned int lcore_id)
-{
-	return lcore_config[lcore_id].socket_id;
-}
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id);
+
+/**
+ * Return the id of the lcore on a socket starting from zero.
+ *
+ * @param lcore_id
+ *   The targeted lcore, or -1 for the current one.
+ * @return
+ *   The relative index, or -1 if not enabled.
+ */
+int
+rte_lcore_to_cpu_id(int lcore_id);
+
+/**
+ * Return the cpuset for a given lcore.
+ * @param lcore_id
+ *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
+ * @return
+ *   The cpuset of that lcore
+ */
+rte_cpuset_t
+rte_lcore_cpuset(unsigned int lcore_id);
+
+/**
+ * Get the return code from a lcore thread.
+ * @param lcore_id
+ *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1
+ *   and finished
+ * @return
+ *   the return code from the lcore thread
+ */
+int __rte_experimental
+rte_lcore_return_code(unsigned int lcore_id);
 
 /**
  * Test if an lcore is enabled.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index d6e375135ad1..f6688327cad3 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -268,6 +268,16 @@ DPDK_18.11 {
 
 } DPDK_18.08;
 
+DPDK_19.05 {
+	global:
+
+	rte_lcore_cpuset;
+	rte_lcore_index;
+	rte_lcore_to_cpu_id;
+	rte_lcore_to_socket_id;
+
+} DPDK_18.08;
+
 EXPERIMENTAL {
 	global:
 
@@ -329,6 +339,7 @@ EXPERIMENTAL {
 	rte_fbarray_set_free;
 	rte_fbarray_set_used;
 	rte_intr_callback_unregister_pending;
+	rte_lcore_return_code;
 	rte_log_register_type_and_pick_level;
 	rte_malloc_dump_heaps;
 	rte_malloc_heap_create;
-- 
2.17.1

[dpdk-dev] [PATCH v2 3/5] bus: use lcore accessor functions

[Stephen Hemminger]

The lcore_config structure will be hidden in future release.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/dpaa/dpaa_bus.c              | 6 ++++--
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index ac20eccd53c1..08c822781d9d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -254,6 +254,7 @@ int rte_dpaa_portal_init(void *arg)
 	unsigned int cpu, lcore = rte_lcore_id();
 	int ret;
 	struct dpaa_portal *dpaa_io_portal;
+	rte_cpuset_t cpuset;
 
 	BUS_INIT_FUNC_TRACE();
 
@@ -263,12 +264,13 @@ int rte_dpaa_portal_init(void *arg)
 		if (lcore >= RTE_MAX_LCORE)
 			return -1;
 
-	cpu = lcore_config[lcore].core_id;
+	cpu = rte_lcore_to_cpu_id(lcore);
 
 	/* Set CPU affinity for this thread.*/
 	id = pthread_self();
+	cpuset = rte_lcore_cpuset(lcore);
 	ret = pthread_setaffinity_np(id, sizeof(cpu_set_t),
-			&lcore_config[lcore].cpuset);
+				     &cpuset);
 	if (ret) {
 		DPAA_BUS_LOG(ERR, "pthread_setaffinity_np failed on core :%u"
 			     " (lcore=%u) with ret: %d", cpu, lcore, ret);
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index 7bcbde840e63..8efb24af5c6a 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -366,7 +366,9 @@ dpaa2_check_lcore_cpuset(void)
 
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		for (i = 0; i < RTE_MAX_LCORE; i++) {
-			if (CPU_ISSET(i, &lcore_config[lcore_id].cpuset)) {
+			rte_cpuset_t cpuset = rte_lcore_cpuset(lcore_id);
+
+			if (CPU_ISSET(i, &cpuset)) {
 				RTE_LOG(DEBUG, EAL, "lcore id = %u cpu=%u\n",
 					lcore_id, i);
 				if (dpaa2_cpu[lcore_id] != 0xffffffff) {
-- 
2.17.1

[dpdk-dev] [PATCH v2 4/5] examples/bond: use lcore accessor

[Stephen Hemminger]

Referring to lcore_config directly is no longer recommended.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/bond/main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/examples/bond/main.c b/examples/bond/main.c
index ef86194fff4a..b047ab23f01d 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -526,8 +526,9 @@ static void cmd_start_parsed(__attribute__((unused)) void *parsed_result,
 	int slave_core_id = rte_lcore_id();
 
 	rte_spinlock_trylock(&global_flag_stru_p->lock);
-	if (global_flag_stru_p->LcoreMainIsRunning == 0)	{
-		if (lcore_config[global_flag_stru_p->LcoreMainCore].state != WAIT)	{
+	if (global_flag_stru_p->LcoreMainIsRunning == 0) {
+		if (rte_eal_get_lcore_state(global_flag_stru_p->LcoreMainCore)
+		    != WAIT) {
 			rte_spinlock_unlock(&global_flag_stru_p->lock);
 			return;
 		}
-- 
2.17.1

[dpdk-dev] [PATCH v2 5/5] app/test: use lcore accessor functions

[Stephen Hemminger]

Don't refer to lcore_config directly.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_cryptodev.c         |  2 +-
 app/test/test_hash_readwrite_lf.c | 14 +++++++-------
 app/test/test_ring_perf.c         | 22 ++++++++++++----------
 app/test/test_stack_perf.c        | 20 ++++++++++----------
 4 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9f31aaa7e6b2..eca6d3db16a5 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -378,7 +378,7 @@ testsuite_setup(void)
 			strcpy(temp_str, vdev_args);
 			strlcat(temp_str, ";", sizeof(temp_str));
 			slave_core_count++;
-			socket_id = lcore_config[i].socket_id;
+			socket_id = rte_lcore_to_socket_id(i);
 		}
 		if (slave_core_count != 2) {
 			RTE_LOG(ERR, USER1,
diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 4ab4c8ee64cf..1361a8aa4c9c 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -741,7 +741,7 @@ test_hash_add_no_ks_lookup_hit(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			rte_eal_mp_wait_lcore();
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -813,7 +813,7 @@ test_hash_add_no_ks_lookup_miss(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -889,7 +889,7 @@ test_hash_add_ks_lookup_hit_non_sp(struct rwc_perf *rwc_perf_results,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -965,7 +965,7 @@ test_hash_add_ks_lookup_hit_sp(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1040,7 +1040,7 @@ test_hash_add_ks_lookup_miss(struct rwc_perf *rwc_perf_results, int rwc_lf, int
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1141,7 +1141,7 @@ test_hash_multi_add_lookup(struct rwc_perf *rwc_perf_results, int rwc_lf,
 				rte_eal_mp_wait_lcore();
 
 				for (i = 1; i <= rwc_core_cnt[n]; i++)
-					if (lcore_config[i].ret < 0)
+					if (rte_lcore_return_code(i) < 0)
 						goto err;
 
 				unsigned long long cycles_per_lookup =
@@ -1225,7 +1225,7 @@ test_hash_add_ks_lookup_hit_extbkt(struct rwc_perf *rwc_perf_results,
 			rte_eal_mp_wait_lcore();
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
index ebb3939f51d0..6eccccfe93b4 100644
--- a/app/test/test_ring_perf.c
+++ b/app/test/test_ring_perf.c
@@ -52,10 +52,11 @@ get_two_hyperthreads(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 == c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -75,10 +76,11 @@ get_two_cores(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 != c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -98,8 +100,8 @@ get_two_sockets(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if (s1 != s2){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
diff --git a/app/test/test_stack_perf.c b/app/test/test_stack_perf.c
index ba27fbf7076d..70561fecda0d 100644
--- a/app/test/test_stack_perf.c
+++ b/app/test/test_stack_perf.c
@@ -44,10 +44,10 @@ get_two_hyperthreads(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] == core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -70,10 +70,10 @@ get_two_cores(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] != core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -95,8 +95,8 @@ get_two_sockets(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if (socket[0] != socket[1]) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for lcore_config

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> Sent: Wednesday, April 10, 2019 10:46 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Subject: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for lcore_config
> 
> The fields of the internal EAL core configuration are currently laid bare as part of
> the API. This is not good practice and limits fixing issues with layout and sizes.
> 
> Make new accessor functions for the fields used by current drivers and
> examples. Mark return code functions as experimental since this value might
> change in the future and probably shouldn't have been used by non EAL code
> anyway.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
>  Shared Library Versions
>  -----------------------
> diff --git a/lib/librte_eal/common/eal_common_lcore.c
> b/lib/librte_eal/common/eal_common_lcore.c
> index 1cbac42286ba..6cf4d7abb0bd 100644
> --- a/lib/librte_eal/common/eal_common_lcore.c
> +++ b/lib/librte_eal/common/eal_common_lcore.c
> @@ -16,6 +16,45 @@
>  #include "eal_private.h"
>  #include "eal_thread.h"
> 
> +int rte_lcore_index(int lcore_id)
> +{
> +	if (unlikely(lcore_id >= RTE_MAX_LCORE))
> +		return -1;
> +
> +	if (lcore_id < 0)
> +		lcore_id = (int)rte_lcore_id();
> +
> +	return lcore_config[lcore_id].core_index;
> +}

If I understand it correctly, We are planning to do this scheme only for slow path functions. Right?
Is rte_lcore_* functions comes in slow path category? I thought a few of them can be used
In fast path too.
I am bit concerned about a low end arm cores where function invocation overhead significant vs inline.
ODP has taken a route of introducing a compile time config to choose inline vs separate function to address
the performance regression . I am not sure what would be the correct way to handle this.




 

Re: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for lcore_config

[Stephen Hemminger]

On Tue, 16 Apr 2019 17:03:47 +0000
Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:

> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> > Sent: Wednesday, April 10, 2019 10:46 PM
> > To: dev@dpdk.org
> > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > Subject: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for lcore_config
> > 
> > The fields of the internal EAL core configuration are currently laid bare as part of
> > the API. This is not good practice and limits fixing issues with layout and sizes.
> > 
> > Make new accessor functions for the fields used by current drivers and
> > examples. Mark return code functions as experimental since this value might
> > change in the future and probably shouldn't have been used by non EAL code
> > anyway.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > Reviewed-by: David Marchand <david.marchand@redhat.com>
> >  Shared Library Versions
> >  -----------------------
> > diff --git a/lib/librte_eal/common/eal_common_lcore.c
> > b/lib/librte_eal/common/eal_common_lcore.c
> > index 1cbac42286ba..6cf4d7abb0bd 100644
> > --- a/lib/librte_eal/common/eal_common_lcore.c
> > +++ b/lib/librte_eal/common/eal_common_lcore.c
> > @@ -16,6 +16,45 @@
> >  #include "eal_private.h"
> >  #include "eal_thread.h"
> > 
> > +int rte_lcore_index(int lcore_id)
> > +{
> > +	if (unlikely(lcore_id >= RTE_MAX_LCORE))
> > +		return -1;
> > +
> > +	if (lcore_id < 0)
> > +		lcore_id = (int)rte_lcore_id();
> > +
> > +	return lcore_config[lcore_id].core_index;
> > +}  
> 
> If I understand it correctly, We are planning to do this scheme only for slow path functions. Right?
> Is rte_lcore_* functions comes in slow path category? I thought a few of them can be used
> In fast path too.
> I am bit concerned about a low end arm cores where function invocation overhead significant vs inline.
> ODP has taken a route of introducing a compile time config to choose inline vs separate function to address
> the performance regression . I am not sure what would be the correct way to handle this.

The lcore config itself is not accessed anywhere in fastpath of any of the examples.
It is usually is part of the setup process. The rte_lcore_index is only used
by lthread in a log message.

The main fastpath is rte_lcore_id() which is already done by per-thread variable
and is unchanged by these patches.

I don't have facilities to do any deep dive performance testing on ARM.

Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/5] eal: add accessor functions for lcore_config

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, May 1, 2019 2:23 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: dev@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for
> lcore_config
> On Tue, 16 Apr 2019 17:03:47 +0000
> Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
> 
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> > > Sent: Wednesday, April 10, 2019 10:46 PM
> > > To: dev@dpdk.org
> > > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > > Subject: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for
> > > lcore_config
> > >
> > > The fields of the internal EAL core configuration are currently laid
> > > bare as part of the API. This is not good practice and limits fixing issues with
> layout and sizes.
> > >
> > > Make new accessor functions for the fields used by current drivers
> > > and examples. Mark return code functions as experimental since this
> > > value might change in the future and probably shouldn't have been
> > > used by non EAL code anyway.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > Reviewed-by: David Marchand <david.marchand@redhat.com>  Shared
> > > Library Versions
> > >  -----------------------
> > > diff --git a/lib/librte_eal/common/eal_common_lcore.c
> > > b/lib/librte_eal/common/eal_common_lcore.c
> > > index 1cbac42286ba..6cf4d7abb0bd 100644
> > > --- a/lib/librte_eal/common/eal_common_lcore.c
> > > +++ b/lib/librte_eal/common/eal_common_lcore.c
> > > @@ -16,6 +16,45 @@
> > >  #include "eal_private.h"
> > >  #include "eal_thread.h"
> > >
> > > +int rte_lcore_index(int lcore_id)
> > > +{
> > > +	if (unlikely(lcore_id >= RTE_MAX_LCORE))
> > > +		return -1;
> > > +
> > > +	if (lcore_id < 0)
> > > +		lcore_id = (int)rte_lcore_id();
> > > +
> > > +	return lcore_config[lcore_id].core_index;
> > > +}
> >
> > If I understand it correctly, We are planning to do this scheme only for slow
> path functions. Right?
> > Is rte_lcore_* functions comes in slow path category? I thought a few
> > of them can be used In fast path too.
> > I am bit concerned about a low end arm cores where function invocation
> overhead significant vs inline.
> > ODP has taken a route of introducing a compile time config to choose
> > inline vs separate function to address the performance regression . I am not
> sure what would be the correct way to handle this.
> 
> The lcore config itself is not accessed anywhere in fastpath of any of the
> examples.
> It is usually is part of the setup process. The rte_lcore_index is only used by
> lthread in a log message.

Makes sense.

> 
> The main fastpath is rte_lcore_id() which is already done by per-thread variable
> and is unchanged by these patches.
> 

How about creating a tag like __rte_experimental, say __rte_fastpath
for fastpath functions to avoid confusion between what is  fastpath and what is not
between developers and end users. By creating new section in linker, will
make all fast path function in one area.


> I don't have facilities to do any deep dive performance testing on ARM.

Re: [dpdk-dev] [PATCH v2 0/5] make lcore_config internal

[Stephen Hemminger]

On Wed, 10 Apr 2019 10:15:58 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> This set of patches makes the lcore_config structure less visible
> as part of the ABI.  This version does not break the ABI (yet)
> follow on patch moves lcore_config into eal_private.h
> 
> The changes in v2 is to:
>  - new patch to use unsigned int in lcore.h first
>  - incorporate feedback from David
>  - don't include last patch to make it private
> 	(to avoid accidental early merge)
> 	
> Stephen Hemminger (5):
>   eal: use unsigned int in rte_lcore.h functions
>   eal: add accessor functions for lcore_config
>   bus: use lcore accessor functions
>   examples/bond: use lcore accessor
>   app/test: use lcore accessor functions
> 
>  app/test/test_cryptodev.c                 |  2 +-
>  app/test/test_hash_readwrite_lf.c         | 14 +++---
>  app/test/test_ring_perf.c                 | 22 +++++----
>  app/test/test_stack_perf.c                | 20 ++++----
>  doc/guides/rel_notes/release_19_05.rst    |  6 +++
>  drivers/bus/dpaa/dpaa_bus.c               |  6 ++-
>  drivers/bus/fslmc/portal/dpaa2_hw_dpio.c  |  4 +-
>  examples/bond/main.c                      |  5 +-
>  lib/librte_eal/common/eal_common_lcore.c  | 39 +++++++++++++++
>  lib/librte_eal/common/include/rte_lcore.h | 58 ++++++++++++++++-------
>  lib/librte_eal/rte_eal_version.map        | 11 +++++
>  11 files changed, 136 insertions(+), 51 deletions(-)
> 

Why is this patchset still stuck in the queue?
The parts with deprecation and accessor functions should really go in 19.05
but since it has sat around for so long, it is probably too late.

Re: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for lcore_config

[David Marchand]

On Wed, Apr 10, 2019 at 7:16 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> The fields of the internal EAL core configuration are currently
> laid bare as part of the API. This is not good practice and limits
> fixing issues with layout and sizes.
>
> Make new accessor functions for the fields used by current drivers
> and examples. Mark return code functions as experimental
> since this value might change in the future and probably shouldn't
> have been used by non EAL code anyway.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> ---
>  doc/guides/rel_notes/release_19_05.rst    |  6 +++
>  lib/librte_eal/common/eal_common_lcore.c  | 39 ++++++++++++++++++
>  lib/librte_eal/common/include/rte_lcore.h | 50 ++++++++++++++++-------
>  lib/librte_eal/rte_eal_version.map        | 11 +++++
>  4 files changed, 92 insertions(+), 14 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_19_05.rst
> b/doc/guides/rel_notes/release_19_05.rst
> index dbdf07a0c05b..32aae5d3bcfa 100644
> --- a/doc/guides/rel_notes/release_19_05.rst
> +++ b/doc/guides/rel_notes/release_19_05.rst
> @@ -222,6 +222,12 @@ ABI Changes
>    alignment for ``rte_crypto_asym_op`` to restore expected
> ``rte_crypto_op``
>    layout and alignment.
>
> +* eal: the lcore config structure ``struct lcore_config`` will be made
> +  internal to the EAL in a future release. This will allow the structure
> to
> +  change without impacting API or ABI. All accesses to fields of this
> +  structure should be done by the corresponding accessor functions.
> +  For example, instead of using ``lcore_config[lcore_id].socket_id``
> +  the function ``rte_lcore_socket_id(lcore_id)`` should be used instead.
>
>  Shared Library Versions
>  -----------------------
> diff --git a/lib/librte_eal/common/eal_common_lcore.c
> b/lib/librte_eal/common/eal_common_lcore.c
> index 1cbac42286ba..6cf4d7abb0bd 100644
> --- a/lib/librte_eal/common/eal_common_lcore.c
> +++ b/lib/librte_eal/common/eal_common_lcore.c
> @@ -16,6 +16,45 @@
>  #include "eal_private.h"
>  #include "eal_thread.h"
>
> +int rte_lcore_index(int lcore_id)
> +{
> +       if (unlikely(lcore_id >= RTE_MAX_LCORE))
> +               return -1;
> +
> +       if (lcore_id < 0)
> +               lcore_id = (int)rte_lcore_id();
> +
> +       return lcore_config[lcore_id].core_index;
> +}
> +
> +int rte_lcore_to_cpu_id(int lcore_id)
> +{
> +       if (unlikely(lcore_id >= RTE_MAX_LCORE))
> +               return -1;
> +
> +       if (lcore_id < 0)
> +               lcore_id = (int)rte_lcore_id();
> +
> +       return lcore_config[lcore_id].core_id;
> +}
> +
> +rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id)
> +{
> +       return lcore_config[lcore_id].cpuset;
> +}
> +
> +unsigned int
> +rte_lcore_to_socket_id(unsigned int lcore_id)
> +{
> +       return lcore_config[lcore_id].socket_id;
> +}
> +
> +int
> +rte_lcore_return_code(unsigned int lcore_id)
> +{
> +       return lcore_config[lcore_id].ret;
> +}
> +
>  static int
>  socket_id_cmp(const void *a, const void *b)
>  {
> diff --git a/lib/librte_eal/common/include/rte_lcore.h
> b/lib/librte_eal/common/include/rte_lcore.h
> index 959ef9ece4b2..dc9f3dc0843d 100644
> --- a/lib/librte_eal/common/include/rte_lcore.h
> +++ b/lib/librte_eal/common/include/rte_lcore.h
> @@ -121,15 +121,8 @@ rte_lcore_count(void)
>   * @return
>   *   The relative index, or -1 if not enabled.
>   */
> -static inline int
> -rte_lcore_index(int lcore_id)
> -{
> -       if (lcore_id >= RTE_MAX_LCORE)
> -               return -1;
> -       if (lcore_id < 0)
> -               lcore_id = (int)rte_lcore_id();
> -       return lcore_config[lcore_id].core_index;
> -}
> +int __rte_experimental
> +rte_lcore_index(int lcore_id);
>

This is new from v2.
Please remove this __rte_experimental tag.



>  /**
>   * Return the ID of the physical socket of the logical core we are
> @@ -177,11 +170,40 @@ rte_socket_id_by_idx(unsigned int idx);
>   * @return
>   *   the ID of lcoreid's physical socket
>   */
> -static inline unsigned int
> -rte_lcore_to_socket_id(unsigned int lcore_id)
> -{
> -       return lcore_config[lcore_id].socket_id;
> -}
> +unsigned int
> +rte_lcore_to_socket_id(unsigned int lcore_id);
> +
> +/**
> + * Return the id of the lcore on a socket starting from zero.
> + *
> + * @param lcore_id
> + *   The targeted lcore, or -1 for the current one.
> + * @return
> + *   The relative index, or -1 if not enabled.
> + */
> +int
> +rte_lcore_to_cpu_id(int lcore_id);
> +
> +/**
> + * Return the cpuset for a given lcore.
> + * @param lcore_id
> + *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
> + * @return
> + *   The cpuset of that lcore
> + */
> +rte_cpuset_t
> +rte_lcore_cpuset(unsigned int lcore_id);
> +
> +/**
> + * Get the return code from a lcore thread.
> + * @param lcore_id
> + *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1
> + *   and finished
> + * @return
> + *   the return code from the lcore thread
> + */
> +int __rte_experimental
> +rte_lcore_return_code(unsigned int lcore_id);
>
>  /**
>   * Test if an lcore is enabled.
> diff --git a/lib/librte_eal/rte_eal_version.map
> b/lib/librte_eal/rte_eal_version.map
> index d6e375135ad1..f6688327cad3 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -268,6 +268,16 @@ DPDK_18.11 {
>
>  } DPDK_18.08;
>
> +DPDK_19.05 {
> +       global:
> +
> +       rte_lcore_cpuset;
> +       rte_lcore_index;
> +       rte_lcore_to_cpu_id;
> +       rte_lcore_to_socket_id;
> +
> +} DPDK_18.08;
> +
>

19.05 inherits from 18.11 not 18.08.


 EXPERIMENTAL {
>         global:
>
> @@ -329,6 +339,7 @@ EXPERIMENTAL {
>         rte_fbarray_set_free;
>         rte_fbarray_set_used;
>         rte_intr_callback_unregister_pending;
> +       rte_lcore_return_code;
>         rte_log_register_type_and_pick_level;
>         rte_malloc_dump_heaps;
>         rte_malloc_heap_create;
> --
> 2.17.1
>


And with these two changes, renewing my tag.
Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 1/5] eal: use unsigned int in rte_lcore.h functions

[David Marchand]

On Wed, Apr 10, 2019 at 7:16 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> Purely cosmetic change, use unsigned int instead of unsigned alone.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_eal/common/include/rte_lcore.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/rte_lcore.h
> b/lib/librte_eal/common/include/rte_lcore.h
> index dea17f500065..959ef9ece4b2 100644
> --- a/lib/librte_eal/common/include/rte_lcore.h
> +++ b/lib/librte_eal/common/include/rte_lcore.h
> @@ -137,7 +137,7 @@ rte_lcore_index(int lcore_id)
>   * @return
>   *   the ID of current lcoreid's physical socket
>   */
> -unsigned rte_socket_id(void);
> +unsigned int rte_socket_id(void);
>
>  /**
>   * Return number of physical sockets detected on the system.
> @@ -177,8 +177,8 @@ rte_socket_id_by_idx(unsigned int idx);
>   * @return
>   *   the ID of lcoreid's physical socket
>   */
> -static inline unsigned
> -rte_lcore_to_socket_id(unsigned lcore_id)
> +static inline unsigned int
> +rte_lcore_to_socket_id(unsigned int lcore_id)
>  {
>         return lcore_config[lcore_id].socket_id;
>  }
> @@ -193,7 +193,7 @@ rte_lcore_to_socket_id(unsigned lcore_id)
>   *   True if the given lcore is enabled; false otherwise.
>   */
>  static inline int
> -rte_lcore_is_enabled(unsigned lcore_id)
> +rte_lcore_is_enabled(unsigned int lcore_id)
>  {
>         struct rte_config *cfg = rte_eal_get_configuration();
>         if (lcore_id >= RTE_MAX_LCORE)
> @@ -214,8 +214,8 @@ rte_lcore_is_enabled(unsigned lcore_id)
>   * @return
>   *   The next lcore_id or RTE_MAX_LCORE if not found.
>   */
> -static inline unsigned
> -rte_get_next_lcore(unsigned i, int skip_master, int wrap)
> +static inline unsigned int
> +rte_get_next_lcore(unsigned int i, int skip_master, int wrap)
>  {
>         i++;
>         if (wrap)
> --
> 2.17.1
>
>
Reviewed-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 4/5] examples/bond: use lcore accessor

[David Marchand]

On Wed, Apr 10, 2019 at 7:16 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> Referring to lcore_config directly is no longer recommended.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  examples/bond/main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/examples/bond/main.c b/examples/bond/main.c
> index ef86194fff4a..b047ab23f01d 100644
> --- a/examples/bond/main.c
> +++ b/examples/bond/main.c
> @@ -526,8 +526,9 @@ static void cmd_start_parsed(__attribute__((unused))
> void *parsed_result,
>         int slave_core_id = rte_lcore_id();
>
>         rte_spinlock_trylock(&global_flag_stru_p->lock);
> -       if (global_flag_stru_p->LcoreMainIsRunning == 0)        {
> -               if (lcore_config[global_flag_stru_p->LcoreMainCore].state
> != WAIT)      {
> +       if (global_flag_stru_p->LcoreMainIsRunning == 0) {
> +               if
> (rte_eal_get_lcore_state(global_flag_stru_p->LcoreMainCore)
> +                   != WAIT) {
>                         rte_spinlock_unlock(&global_flag_stru_p->lock);
>                         return;
>                 }
> --
> 2.17.1
>
>
Same comment than in v2, you missed another lcore_config in this file at
line 800.


-- 
David Marchand

[dpdk-dev] [PATCH v3 0/5] prepare to make lcore_config not visible in ABI

[Stephen Hemminger]

This set of patches makes the lcore_config structure less visible
as part of the ABI.  This version does not break the ABI (yet)
follow on patch moves lcore_config into eal_private.h

Changes for v3 (based on David's feedback):
  - rte_lcore_index should not be experimental
  - eal map should chain from 18.11
  - more fixes in bond example

Stephen Hemminger (5):
  eal: use unsigned int in rte_lcore.h functions
  eal: add accessor functions for lcore_config
  bus: use lcore accessor functions
  examples/bond: use lcore accessor
  app/test: use lcore accessor functions

 app/test/test_cryptodev.c                 |  2 +-
 app/test/test_hash_readwrite_lf.c         | 14 +++---
 app/test/test_ring_perf.c                 | 22 +++++----
 app/test/test_stack_perf.c                | 20 ++++----
 doc/guides/rel_notes/release_19_05.rst    |  6 +++
 drivers/bus/dpaa/dpaa_bus.c               |  6 ++-
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c  |  4 +-
 examples/bond/main.c                      | 13 +++---
 lib/librte_eal/common/eal_common_lcore.c  | 39 ++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 57 ++++++++++++++++-------
 lib/librte_eal/rte_eal_version.map        | 11 +++++
 11 files changed, 139 insertions(+), 55 deletions(-)

-- 
2.20.1

[dpdk-dev] [PATCH v3 1/5] eal: use unsigned int in rte_lcore.h functions

[Stephen Hemminger]

Purely cosmetic change, use unsigned int instead of unsigned alone.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/include/rte_lcore.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index be757a32e085..705594acbb5e 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -137,7 +137,7 @@ rte_lcore_index(int lcore_id)
  * @return
  *   the ID of current lcoreid's physical socket
  */
-unsigned rte_socket_id(void);
+unsigned int rte_socket_id(void);
 
 /**
  * Return number of physical sockets detected on the system.
@@ -177,8 +177,8 @@ rte_socket_id_by_idx(unsigned int idx);
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned
-rte_lcore_to_socket_id(unsigned lcore_id)
+static inline unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
 {
 	return lcore_config[lcore_id].socket_id;
 }
@@ -193,7 +193,7 @@ rte_lcore_to_socket_id(unsigned lcore_id)
  *   True if the given lcore is enabled; false otherwise.
  */
 static inline int
-rte_lcore_is_enabled(unsigned lcore_id)
+rte_lcore_is_enabled(unsigned int lcore_id)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
 	if (lcore_id >= RTE_MAX_LCORE)
@@ -214,8 +214,8 @@ rte_lcore_is_enabled(unsigned lcore_id)
  * @return
  *   The next lcore_id or RTE_MAX_LCORE if not found.
  */
-static inline unsigned
-rte_get_next_lcore(unsigned i, int skip_master, int wrap)
+static inline unsigned int
+rte_get_next_lcore(unsigned int i, int skip_master, int wrap)
 {
 	i++;
 	if (wrap)
-- 
2.20.1

[dpdk-dev] [PATCH v3 2/5] eal: add accessor functions for lcore_config

[Stephen Hemminger]

The fields of the internal EAL core configuration are currently
laid bare as part of the API. This is not good practice and limits
fixing issues with layout and sizes.

Make new accessor functions for the fields used by current drivers
and examples. Mark return code functions as experimental
since this value might change in the future and probably shouldn't
have been used by non EAL code anyway.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 doc/guides/rel_notes/release_19_05.rst    |  6 +++
 lib/librte_eal/common/eal_common_lcore.c  | 39 ++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 49 ++++++++++++++++-------
 lib/librte_eal/rte_eal_version.map        | 11 +++++
 4 files changed, 91 insertions(+), 14 deletions(-)

diff --git a/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst
index 468e325395c7..35c0c9081958 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -275,6 +275,12 @@ ABI Changes
   alignment for ``rte_crypto_asym_op`` to restore expected ``rte_crypto_op``
   layout and alignment.
 
+* eal: the lcore config structure ``struct lcore_config`` will be made
+  internal to the EAL in a future release. This will allow the structure to
+  change without impacting API or ABI. All accesses to fields of this
+  structure should be done by the corresponding accessor functions.
+  For example, instead of using ``lcore_config[lcore_id].socket_id``
+  the function ``rte_lcore_socket_id(lcore_id)`` should be used instead.
 
 Shared Library Versions
 -----------------------
diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 8c2744fabcbf..ae59c98ca43a 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -16,6 +16,45 @@
 #include "eal_private.h"
 #include "eal_thread.h"
 
+int rte_lcore_index(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_index;
+}
+
+int rte_lcore_to_cpu_id(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_id;
+}
+
+rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].cpuset;
+}
+
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].socket_id;
+}
+
+int
+rte_lcore_return_code(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].ret;
+}
+
 static int
 socket_id_cmp(const void *a, const void *b)
 {
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 705594acbb5e..d0006a077fae 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -121,15 +121,7 @@ rte_lcore_count(void)
  * @return
  *   The relative index, or -1 if not enabled.
  */
-static inline int
-rte_lcore_index(int lcore_id)
-{
-	if (lcore_id >= RTE_MAX_LCORE)
-		return -1;
-	if (lcore_id < 0)
-		lcore_id = (int)rte_lcore_id();
-	return lcore_config[lcore_id].core_index;
-}
+int rte_lcore_index(int lcore_id);
 
 /**
  * Return the ID of the physical socket of the logical core we are
@@ -177,11 +169,40 @@ rte_socket_id_by_idx(unsigned int idx);
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned int
-rte_lcore_to_socket_id(unsigned int lcore_id)
-{
-	return lcore_config[lcore_id].socket_id;
-}
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id);
+
+/**
+ * Return the id of the lcore on a socket starting from zero.
+ *
+ * @param lcore_id
+ *   The targeted lcore, or -1 for the current one.
+ * @return
+ *   The relative index, or -1 if not enabled.
+ */
+int
+rte_lcore_to_cpu_id(int lcore_id);
+
+/**
+ * Return the cpuset for a given lcore.
+ * @param lcore_id
+ *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
+ * @return
+ *   The cpuset of that lcore
+ */
+rte_cpuset_t
+rte_lcore_cpuset(unsigned int lcore_id);
+
+/**
+ * Get the return code from a lcore thread.
+ * @param lcore_id
+ *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1
+ *   and finished
+ * @return
+ *   the return code from the lcore thread
+ */
+int __rte_experimental
+rte_lcore_return_code(unsigned int lcore_id);
 
 /**
  * Test if an lcore is enabled.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 245493461c36..b4aec1667122 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -287,6 +287,16 @@ DPDK_19.05 {
 
 } DPDK_18.11;
 
+DPDK_19.05 {
+	global:
+
+	rte_lcore_cpuset;
+	rte_lcore_index;
+	rte_lcore_to_cpu_id;
+	rte_lcore_to_socket_id;
+
+} DPDK_18.11;
+
 EXPERIMENTAL {
 	global:
 
@@ -337,6 +347,7 @@ EXPERIMENTAL {
 	rte_fbarray_set_free;
 	rte_fbarray_set_used;
 	rte_intr_callback_unregister_pending;
+	rte_lcore_return_code;
 	rte_log_register_type_and_pick_level;
 	rte_malloc_dump_heaps;
 	rte_malloc_heap_create;
-- 
2.20.1

[dpdk-dev] [PATCH v3 3/5] bus: use lcore accessor functions

[Stephen Hemminger]

The lcore_config structure will be hidden in future release.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/dpaa/dpaa_bus.c              | 6 ++++--
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index ac20eccd53c1..08c822781d9d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -254,6 +254,7 @@ int rte_dpaa_portal_init(void *arg)
 	unsigned int cpu, lcore = rte_lcore_id();
 	int ret;
 	struct dpaa_portal *dpaa_io_portal;
+	rte_cpuset_t cpuset;
 
 	BUS_INIT_FUNC_TRACE();
 
@@ -263,12 +264,13 @@ int rte_dpaa_portal_init(void *arg)
 		if (lcore >= RTE_MAX_LCORE)
 			return -1;
 
-	cpu = lcore_config[lcore].core_id;
+	cpu = rte_lcore_to_cpu_id(lcore);
 
 	/* Set CPU affinity for this thread.*/
 	id = pthread_self();
+	cpuset = rte_lcore_cpuset(lcore);
 	ret = pthread_setaffinity_np(id, sizeof(cpu_set_t),
-			&lcore_config[lcore].cpuset);
+				     &cpuset);
 	if (ret) {
 		DPAA_BUS_LOG(ERR, "pthread_setaffinity_np failed on core :%u"
 			     " (lcore=%u) with ret: %d", cpu, lcore, ret);
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index 7bcbde840e63..8efb24af5c6a 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -366,7 +366,9 @@ dpaa2_check_lcore_cpuset(void)
 
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		for (i = 0; i < RTE_MAX_LCORE; i++) {
-			if (CPU_ISSET(i, &lcore_config[lcore_id].cpuset)) {
+			rte_cpuset_t cpuset = rte_lcore_cpuset(lcore_id);
+
+			if (CPU_ISSET(i, &cpuset)) {
 				RTE_LOG(DEBUG, EAL, "lcore id = %u cpu=%u\n",
 					lcore_id, i);
 				if (dpaa2_cpu[lcore_id] != 0xffffffff) {
-- 
2.20.1

[dpdk-dev] [PATCH v3 4/5] examples/bond: use lcore accessor

[Stephen Hemminger]

Referring to lcore_config directly is no longer recommended.
Also remove unnecessary assignment of slave_core_id.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/bond/main.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/examples/bond/main.c b/examples/bond/main.c
index ef86194fff4a..1b35d53c8efa 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -526,8 +526,9 @@ static void cmd_start_parsed(__attribute__((unused)) void *parsed_result,
 	int slave_core_id = rte_lcore_id();
 
 	rte_spinlock_trylock(&global_flag_stru_p->lock);
-	if (global_flag_stru_p->LcoreMainIsRunning == 0)	{
-		if (lcore_config[global_flag_stru_p->LcoreMainCore].state != WAIT)	{
+	if (global_flag_stru_p->LcoreMainIsRunning == 0) {
+		if (rte_eal_get_lcore_state(global_flag_stru_p->LcoreMainCore)
+		    != WAIT) {
 			rte_spinlock_unlock(&global_flag_stru_p->lock);
 			return;
 		}
@@ -760,7 +761,7 @@ static void prompt(__attribute__((unused)) void *arg1)
 int
 main(int argc, char *argv[])
 {
-	int ret;
+	int ret, slave_core_id;
 	uint16_t nb_ports, i;
 
 	/* init EAL */
@@ -792,13 +793,13 @@ main(int argc, char *argv[])
 	bond_port_init(mbuf_pool);
 
 	rte_spinlock_init(&global_flag_stru_p->lock);
-	int slave_core_id = rte_lcore_id();
 
 	/* check state of lcores */
 	RTE_LCORE_FOREACH_SLAVE(slave_core_id) {
-	if (lcore_config[slave_core_id].state != WAIT)
-		return -EBUSY;
+		if (rte_eal_get_lcore_state(slave_core_id) != WAIT)
+			return -EBUSY;
 	}
+
 	/* start lcore main on core != master_core - ARP response thread */
 	slave_core_id = rte_get_next_lcore(rte_lcore_id(), 1, 0);
 	if ((slave_core_id >= RTE_MAX_LCORE) || (slave_core_id == 0))
-- 
2.20.1

[dpdk-dev] [PATCH v3 5/5] app/test: use lcore accessor functions

[Stephen Hemminger]

Don't refer to lcore_config directly.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_cryptodev.c         |  2 +-
 app/test/test_hash_readwrite_lf.c | 14 +++++++-------
 app/test/test_ring_perf.c         | 22 ++++++++++++----------
 app/test/test_stack_perf.c        | 20 ++++++++++----------
 4 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9f31aaa7e6b2..eca6d3db16a5 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -378,7 +378,7 @@ testsuite_setup(void)
 			strcpy(temp_str, vdev_args);
 			strlcat(temp_str, ";", sizeof(temp_str));
 			slave_core_count++;
-			socket_id = lcore_config[i].socket_id;
+			socket_id = rte_lcore_to_socket_id(i);
 		}
 		if (slave_core_count != 2) {
 			RTE_LOG(ERR, USER1,
diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 4ab4c8ee64cf..1361a8aa4c9c 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -741,7 +741,7 @@ test_hash_add_no_ks_lookup_hit(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			rte_eal_mp_wait_lcore();
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -813,7 +813,7 @@ test_hash_add_no_ks_lookup_miss(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -889,7 +889,7 @@ test_hash_add_ks_lookup_hit_non_sp(struct rwc_perf *rwc_perf_results,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -965,7 +965,7 @@ test_hash_add_ks_lookup_hit_sp(struct rwc_perf *rwc_perf_results, int rwc_lf,
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1040,7 +1040,7 @@ test_hash_add_ks_lookup_miss(struct rwc_perf *rwc_perf_results, int rwc_lf, int
 			if (ret < 0)
 				goto err;
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
@@ -1141,7 +1141,7 @@ test_hash_multi_add_lookup(struct rwc_perf *rwc_perf_results, int rwc_lf,
 				rte_eal_mp_wait_lcore();
 
 				for (i = 1; i <= rwc_core_cnt[n]; i++)
-					if (lcore_config[i].ret < 0)
+					if (rte_lcore_return_code(i) < 0)
 						goto err;
 
 				unsigned long long cycles_per_lookup =
@@ -1225,7 +1225,7 @@ test_hash_add_ks_lookup_hit_extbkt(struct rwc_perf *rwc_perf_results,
 			rte_eal_mp_wait_lcore();
 
 			for (i = 1; i <= rwc_core_cnt[n]; i++)
-				if (lcore_config[i].ret < 0)
+				if (rte_lcore_return_code(i) < 0)
 					goto err;
 
 			unsigned long long cycles_per_lookup =
diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
index ebb3939f51d0..6eccccfe93b4 100644
--- a/app/test/test_ring_perf.c
+++ b/app/test/test_ring_perf.c
@@ -52,10 +52,11 @@ get_two_hyperthreads(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 == c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -75,10 +76,11 @@ get_two_cores(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 != c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -98,8 +100,8 @@ get_two_sockets(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if (s1 != s2){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
diff --git a/app/test/test_stack_perf.c b/app/test/test_stack_perf.c
index ba27fbf7076d..70561fecda0d 100644
--- a/app/test/test_stack_perf.c
+++ b/app/test/test_stack_perf.c
@@ -44,10 +44,10 @@ get_two_hyperthreads(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] == core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -70,10 +70,10 @@ get_two_cores(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] != core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -95,8 +95,8 @@ get_two_sockets(struct lcore_pair *lcp)
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if (socket[0] != socket[1]) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
-- 
2.20.1

Re: [dpdk-dev] [PATCH v3 0/5] prepare to make lcore_config not visible in ABI

[David Marchand]

On Fri, May 3, 2019 at 7:25 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> This set of patches makes the lcore_config structure less visible
> as part of the ABI.  This version does not break the ABI (yet)
> follow on patch moves lcore_config into eal_private.h
>
> Changes for v3 (based on David's feedback):
>   - rte_lcore_index should not be experimental
>   - eal map should chain from 18.11
>   - more fixes in bond example
>
> Stephen Hemminger (5):
>   eal: use unsigned int in rte_lcore.h functions
>   eal: add accessor functions for lcore_config
>   bus: use lcore accessor functions
>   examples/bond: use lcore accessor
>   app/test: use lcore accessor functions
>
>  app/test/test_cryptodev.c                 |  2 +-
>  app/test/test_hash_readwrite_lf.c         | 14 +++---
>  app/test/test_ring_perf.c                 | 22 +++++----
>  app/test/test_stack_perf.c                | 20 ++++----
>  doc/guides/rel_notes/release_19_05.rst    |  6 +++
>  drivers/bus/dpaa/dpaa_bus.c               |  6 ++-
>  drivers/bus/fslmc/portal/dpaa2_hw_dpio.c  |  4 +-
>  examples/bond/main.c                      | 13 +++---
>  lib/librte_eal/common/eal_common_lcore.c  | 39 ++++++++++++++++
>  lib/librte_eal/common/include/rte_lcore.h | 57 ++++++++++++++++-------
>  lib/librte_eal/rte_eal_version.map        | 11 +++++
>  11 files changed, 139 insertions(+), 55 deletions(-)
>
> --
> 2.20.1
>
>
LGTM.

-- 
David Marchand

[dpdk-dev] [PATCH v4 0/5] make lcore_config internal

[David Marchand]

This set of patches makes the lcore_config structure less visible
as part of the ABI.  This version does not break the ABI (yet)
follow on patch moves lcore_config into eal_private.h

Changelog since v3:
I took the liberty of taking over Stephen series.
I rebased and did some adjustments following [1] cleanups.
As stated before, we will still need a deprecation notice when hiding
lcore_config but this series does not break API nor ABI.

Changelog since v2:
 - new patch to use unsigned int in lcore.h first
 - incorporate feedback from David
 - don't include last patch to make it private
        (to avoid accidental early merge)

1: http://patchwork.dpdk.org/patch/53621/

-- 
David Marchand

Stephen Hemminger (5):
  eal: use unsigned int in lcore API prototypes
  eal: add lcore accessors
  drivers/bus: use lcore accessors
  examples/bond: use lcore accessors
  test: use lcore accessors

 app/test/test_cryptodev.c                 |  2 +-
 app/test/test_ring_perf.c                 | 22 ++++++++-------
 app/test/test_stack_perf.c                | 20 +++++++-------
 drivers/bus/dpaa/dpaa_bus.c               |  6 ++--
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c  |  4 ++-
 examples/bond/main.c                      | 13 +++++----
 lib/librte_eal/common/eal_common_lcore.c  | 33 ++++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 46 +++++++++++++++++++------------
 lib/librte_eal/rte_eal_version.map        | 10 +++++++
 9 files changed, 108 insertions(+), 48 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v4 1/5] eal: use unsigned int in lcore API prototypes

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

Purely cosmetic change, use unsigned int instead of unsigned alone.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/include/rte_lcore.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Changelog since v3:
- updated title

diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index be757a3..705594a 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -137,7 +137,7 @@ struct lcore_config {
  * @return
  *   the ID of current lcoreid's physical socket
  */
-unsigned rte_socket_id(void);
+unsigned int rte_socket_id(void);
 
 /**
  * Return number of physical sockets detected on the system.
@@ -177,8 +177,8 @@ struct lcore_config {
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned
-rte_lcore_to_socket_id(unsigned lcore_id)
+static inline unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
 {
 	return lcore_config[lcore_id].socket_id;
 }
@@ -193,7 +193,7 @@ struct lcore_config {
  *   True if the given lcore is enabled; false otherwise.
  */
 static inline int
-rte_lcore_is_enabled(unsigned lcore_id)
+rte_lcore_is_enabled(unsigned int lcore_id)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
 	if (lcore_id >= RTE_MAX_LCORE)
@@ -214,8 +214,8 @@ struct lcore_config {
  * @return
  *   The next lcore_id or RTE_MAX_LCORE if not found.
  */
-static inline unsigned
-rte_get_next_lcore(unsigned i, int skip_master, int wrap)
+static inline unsigned int
+rte_get_next_lcore(unsigned int i, int skip_master, int wrap)
 {
 	i++;
 	if (wrap)
-- 
1.8.3.1

[dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

The fields of the internal EAL core configuration are currently
laid bare as part of the API. This is not good practice and limits
fixing issues with layout and sizes.

Make new accessor functions for the fields used by current drivers
and examples.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/eal_common_lcore.c  | 33 +++++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 38 +++++++++++++++++++------------
 lib/librte_eal/rte_eal_version.map        | 10 ++++++++
 3 files changed, 67 insertions(+), 14 deletions(-)

---
Changelog since v3:
- updated title
- rebased on master
- removed doc update
- removed unneeded rte_lcore_return_code

diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 8c2744f..38af260 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -16,6 +16,39 @@
 #include "eal_private.h"
 #include "eal_thread.h"
 
+int rte_lcore_index(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_index;
+}
+
+int rte_lcore_to_cpu_id(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_id;
+}
+
+rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].cpuset;
+}
+
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].socket_id;
+}
+
 static int
 socket_id_cmp(const void *a, const void *b)
 {
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 705594a..e688450 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -121,15 +121,7 @@ struct lcore_config {
  * @return
  *   The relative index, or -1 if not enabled.
  */
-static inline int
-rte_lcore_index(int lcore_id)
-{
-	if (lcore_id >= RTE_MAX_LCORE)
-		return -1;
-	if (lcore_id < 0)
-		lcore_id = (int)rte_lcore_id();
-	return lcore_config[lcore_id].core_index;
-}
+int rte_lcore_index(int lcore_id);
 
 /**
  * Return the ID of the physical socket of the logical core we are
@@ -177,11 +169,29 @@ struct lcore_config {
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned int
-rte_lcore_to_socket_id(unsigned int lcore_id)
-{
-	return lcore_config[lcore_id].socket_id;
-}
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id);
+
+/**
+ * Return the id of the lcore on a socket starting from zero.
+ *
+ * @param lcore_id
+ *   The targeted lcore, or -1 for the current one.
+ * @return
+ *   The relative index, or -1 if not enabled.
+ */
+int
+rte_lcore_to_cpu_id(int lcore_id);
+
+/**
+ * Return the cpuset for a given lcore.
+ * @param lcore_id
+ *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
+ * @return
+ *   The cpuset of that lcore
+ */
+rte_cpuset_t
+rte_lcore_cpuset(unsigned int lcore_id);
 
 /**
  * Test if an lcore is enabled.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 2454934..cb0eb69 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -287,6 +287,16 @@ DPDK_19.05 {
 
 } DPDK_18.11;
 
+DPDK_19.08 {
+	global:
+
+	rte_lcore_cpuset;
+	rte_lcore_index;
+	rte_lcore_to_cpu_id;
+	rte_lcore_to_socket_id;
+
+} DPDK_19.05;
+
 EXPERIMENTAL {
 	global:
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v4 3/5] drivers/bus: use lcore accessors

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

The lcore_config structure will be hidden in future release.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/dpaa/dpaa_bus.c              | 6 ++++--
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

---
Changelog since v3:
- updated title

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index ac20ecc..08c8227 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -254,6 +254,7 @@ int rte_dpaa_portal_init(void *arg)
 	unsigned int cpu, lcore = rte_lcore_id();
 	int ret;
 	struct dpaa_portal *dpaa_io_portal;
+	rte_cpuset_t cpuset;
 
 	BUS_INIT_FUNC_TRACE();
 
@@ -263,12 +264,13 @@ int rte_dpaa_portal_init(void *arg)
 		if (lcore >= RTE_MAX_LCORE)
 			return -1;
 
-	cpu = lcore_config[lcore].core_id;
+	cpu = rte_lcore_to_cpu_id(lcore);
 
 	/* Set CPU affinity for this thread.*/
 	id = pthread_self();
+	cpuset = rte_lcore_cpuset(lcore);
 	ret = pthread_setaffinity_np(id, sizeof(cpu_set_t),
-			&lcore_config[lcore].cpuset);
+				     &cpuset);
 	if (ret) {
 		DPAA_BUS_LOG(ERR, "pthread_setaffinity_np failed on core :%u"
 			     " (lcore=%u) with ret: %d", cpu, lcore, ret);
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index 7bcbde8..8efb24a 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -366,7 +366,9 @@ static struct dpaa2_dpio_dev *dpaa2_get_qbman_swp(int lcoreid)
 
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		for (i = 0; i < RTE_MAX_LCORE; i++) {
-			if (CPU_ISSET(i, &lcore_config[lcore_id].cpuset)) {
+			rte_cpuset_t cpuset = rte_lcore_cpuset(lcore_id);
+
+			if (CPU_ISSET(i, &cpuset)) {
 				RTE_LOG(DEBUG, EAL, "lcore id = %u cpu=%u\n",
 					lcore_id, i);
 				if (dpaa2_cpu[lcore_id] != 0xffffffff) {
-- 
1.8.3.1

[dpdk-dev] [PATCH v4 4/5] examples/bond: use lcore accessors

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

Referring to lcore_config directly is no longer recommended.
Also remove unnecessary assignment of slave_core_id.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 examples/bond/main.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

---
Changelog since v3:
- updated title

diff --git a/examples/bond/main.c b/examples/bond/main.c
index ef86194..1b35d53 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -526,8 +526,9 @@ static void cmd_start_parsed(__attribute__((unused)) void *parsed_result,
 	int slave_core_id = rte_lcore_id();
 
 	rte_spinlock_trylock(&global_flag_stru_p->lock);
-	if (global_flag_stru_p->LcoreMainIsRunning == 0)	{
-		if (lcore_config[global_flag_stru_p->LcoreMainCore].state != WAIT)	{
+	if (global_flag_stru_p->LcoreMainIsRunning == 0) {
+		if (rte_eal_get_lcore_state(global_flag_stru_p->LcoreMainCore)
+		    != WAIT) {
 			rte_spinlock_unlock(&global_flag_stru_p->lock);
 			return;
 		}
@@ -760,7 +761,7 @@ static void prompt(__attribute__((unused)) void *arg1)
 int
 main(int argc, char *argv[])
 {
-	int ret;
+	int ret, slave_core_id;
 	uint16_t nb_ports, i;
 
 	/* init EAL */
@@ -792,13 +793,13 @@ static void prompt(__attribute__((unused)) void *arg1)
 	bond_port_init(mbuf_pool);
 
 	rte_spinlock_init(&global_flag_stru_p->lock);
-	int slave_core_id = rte_lcore_id();
 
 	/* check state of lcores */
 	RTE_LCORE_FOREACH_SLAVE(slave_core_id) {
-	if (lcore_config[slave_core_id].state != WAIT)
-		return -EBUSY;
+		if (rte_eal_get_lcore_state(slave_core_id) != WAIT)
+			return -EBUSY;
 	}
+
 	/* start lcore main on core != master_core - ARP response thread */
 	slave_core_id = rte_get_next_lcore(rte_lcore_id(), 1, 0);
 	if ((slave_core_id >= RTE_MAX_LCORE) || (slave_core_id == 0))
-- 
1.8.3.1

[dpdk-dev] [PATCH v4 5/5] test: use lcore accessors

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

Don't refer to lcore_config directly.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_cryptodev.c  |  2 +-
 app/test/test_ring_perf.c  | 22 ++++++++++++----------
 app/test/test_stack_perf.c | 20 ++++++++++----------
 3 files changed, 23 insertions(+), 21 deletions(-)

---
Changelog since v3:
- updated title
- removed parts on test_hash_readwrite_lf

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9f31aaa..eca6d3d 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -378,7 +378,7 @@ struct crypto_unittest_params {
 			strcpy(temp_str, vdev_args);
 			strlcat(temp_str, ";", sizeof(temp_str));
 			slave_core_count++;
-			socket_id = lcore_config[i].socket_id;
+			socket_id = rte_lcore_to_socket_id(i);
 		}
 		if (slave_core_count != 2) {
 			RTE_LOG(ERR, USER1,
diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
index ebb3939..6eccccf 100644
--- a/app/test/test_ring_perf.c
+++ b/app/test/test_ring_perf.c
@@ -52,10 +52,11 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 == c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -75,10 +76,11 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 != c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -98,8 +100,8 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if (s1 != s2){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
diff --git a/app/test/test_stack_perf.c b/app/test/test_stack_perf.c
index ba27fbf..70561fe 100644
--- a/app/test/test_stack_perf.c
+++ b/app/test/test_stack_perf.c
@@ -44,10 +44,10 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] == core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -70,10 +70,10 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] != core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -95,8 +95,8 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if (socket[0] != socket[1]) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v4 0/5] make lcore_config internal

[Stephen Hemminger]

On Thu, 23 May 2019 15:58:56 +0200
David Marchand <david.marchand@redhat.com> wrote:

> This set of patches makes the lcore_config structure less visible
> as part of the ABI.  This version does not break the ABI (yet)
> follow on patch moves lcore_config into eal_private.h
> 
> Changelog since v3:
> I took the liberty of taking over Stephen series.
> I rebased and did some adjustments following [1] cleanups.
> As stated before, we will still need a deprecation notice when hiding
> lcore_config but this series does not break API nor ABI.
> 
> Changelog since v2:
>  - new patch to use unsigned int in lcore.h first
>  - incorporate feedback from David
>  - don't include last patch to make it private
>         (to avoid accidental early merge)
> 
> 1: http://patchwork.dpdk.org/patch/53621/
> 

Thanks for doing this.

We still need the documentation update to indicate deprecation.

Re: [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[Thomas Monjalon]

23/05/2019 15:58, David Marchand:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> The fields of the internal EAL core configuration are currently
> laid bare as part of the API. This is not good practice and limits
> fixing issues with layout and sizes.
> 
> Make new accessor functions for the fields used by current drivers
> and examples.
[...]
> +DPDK_19.08 {
> +	global:
> +
> +	rte_lcore_cpuset;
> +	rte_lcore_index;
> +	rte_lcore_to_cpu_id;
> +	rte_lcore_to_socket_id;
> +
> +} DPDK_19.05;
> +
>  EXPERIMENTAL {
>  	global:

Just to make sure, are we OK to introduce these functions
as non-experimental?

Re: [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[Stephen Hemminger]

On Thu, 30 May 2019 00:46:30 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 23/05/2019 15:58, David Marchand:
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > 
> > The fields of the internal EAL core configuration are currently
> > laid bare as part of the API. This is not good practice and limits
> > fixing issues with layout and sizes.
> > 
> > Make new accessor functions for the fields used by current drivers
> > and examples.  
> [...]
> > +DPDK_19.08 {
> > +	global:
> > +
> > +	rte_lcore_cpuset;
> > +	rte_lcore_index;
> > +	rte_lcore_to_cpu_id;
> > +	rte_lcore_to_socket_id;
> > +
> > +} DPDK_19.05;
> > +
> >  EXPERIMENTAL {
> >  	global:  
> 
> Just to make sure, are we OK to introduce these functions
> as non-experimental?

They were in previous releases as inlines this patch converts them
to real functions.

Re: [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[David Marchand]

On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Thu, 30 May 2019 00:46:30 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > 23/05/2019 15:58, David Marchand:
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > >
> > > The fields of the internal EAL core configuration are currently
> > > laid bare as part of the API. This is not good practice and limits
> > > fixing issues with layout and sizes.
> > >
> > > Make new accessor functions for the fields used by current drivers
> > > and examples.
> > [...]
> > > +DPDK_19.08 {
> > > +   global:
> > > +
> > > +   rte_lcore_cpuset;
> > > +   rte_lcore_index;
> > > +   rte_lcore_to_cpu_id;
> > > +   rte_lcore_to_socket_id;
> > > +
> > > +} DPDK_19.05;
> > > +
> > >  EXPERIMENTAL {
> > >     global:
> >
> > Just to make sure, are we OK to introduce these functions
> > as non-experimental?
>
> They were in previous releases as inlines this patch converts them
> to real functions.
>
>
Well, yes and no.

rte_lcore_index and rte_lcore_to_socket_id already existed, so making them
part of the ABI is fine for me.

rte_lcore_to_cpu_id is new but seems quite safe in how it can be used,
adding it to the ABI is ok for me.

rte_lcore_cpuset is new too, and still a bit obscure to me. I am not really
convinced we need it until I understand why dpaa2 and fslmc bus need to
know about this.
I might need more time to look at it, so flag this as experimental sounds
fair to me.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[Thomas Monjalon]

30/05/2019 09:31, David Marchand:
> On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> stephen@networkplumber.org> wrote:
> 
> > On Thu, 30 May 2019 00:46:30 +0200
> > Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > > 23/05/2019 15:58, David Marchand:
> > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > >
> > > > The fields of the internal EAL core configuration are currently
> > > > laid bare as part of the API. This is not good practice and limits
> > > > fixing issues with layout and sizes.
> > > >
> > > > Make new accessor functions for the fields used by current drivers
> > > > and examples.
> > > [...]
> > > > +DPDK_19.08 {
> > > > +   global:
> > > > +
> > > > +   rte_lcore_cpuset;
> > > > +   rte_lcore_index;
> > > > +   rte_lcore_to_cpu_id;
> > > > +   rte_lcore_to_socket_id;
> > > > +
> > > > +} DPDK_19.05;
> > > > +
> > > >  EXPERIMENTAL {
> > > >     global:
> > >
> > > Just to make sure, are we OK to introduce these functions
> > > as non-experimental?
> >
> > They were in previous releases as inlines this patch converts them
> > to real functions.
> >
> >
> Well, yes and no.
> 
> rte_lcore_index and rte_lcore_to_socket_id already existed, so making them
> part of the ABI is fine for me.
> 
> rte_lcore_to_cpu_id is new but seems quite safe in how it can be used,
> adding it to the ABI is ok for me.

It is used by DPAA and some test.
I guess adding as experimental is fine too?
I'm fine with both options, I'm just trying to apply the policy
we agreed on. Does this case deserve an exception?

> rte_lcore_cpuset is new too, and still a bit obscure to me. I am not really
> convinced we need it until I understand why dpaa2 and fslmc bus need to
> know about this.
> I might need more time to look at it, so flag this as experimental sounds
> fair to me.

Re: [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[Bruce Richardson]

On Thu, May 30, 2019 at 09:40:08AM +0200, Thomas Monjalon wrote:
> 30/05/2019 09:31, David Marchand:
> > On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> > stephen@networkplumber.org> wrote:
> > 
> > > On Thu, 30 May 2019 00:46:30 +0200
> > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > > 23/05/2019 15:58, David Marchand:
> > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > >
> > > > > The fields of the internal EAL core configuration are currently
> > > > > laid bare as part of the API. This is not good practice and limits
> > > > > fixing issues with layout and sizes.
> > > > >
> > > > > Make new accessor functions for the fields used by current drivers
> > > > > and examples.
> > > > [...]
> > > > > +DPDK_19.08 {
> > > > > +   global:
> > > > > +
> > > > > +   rte_lcore_cpuset;
> > > > > +   rte_lcore_index;
> > > > > +   rte_lcore_to_cpu_id;
> > > > > +   rte_lcore_to_socket_id;
> > > > > +
> > > > > +} DPDK_19.05;
> > > > > +
> > > > >  EXPERIMENTAL {
> > > > >     global:
> > > >
> > > > Just to make sure, are we OK to introduce these functions
> > > > as non-experimental?
> > >
> > > They were in previous releases as inlines this patch converts them
> > > to real functions.
> > >
> > >
> > Well, yes and no.
> > 
> > rte_lcore_index and rte_lcore_to_socket_id already existed, so making them
> > part of the ABI is fine for me.
> > 
> > rte_lcore_to_cpu_id is new but seems quite safe in how it can be used,
> > adding it to the ABI is ok for me.
> 
> It is used by DPAA and some test.
> I guess adding as experimental is fine too?
> I'm fine with both options, I'm just trying to apply the policy
> we agreed on. Does this case deserve an exception?
> 

While it may be a good candidate, I'm not sure how much making an exception
for it really matters. I'd be tempted to just mark it experimental and then
have it stable for the 19.11 release. What do we really lose by waiting a
release to stabilize it?

Re: [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[Thomas Monjalon]

30/05/2019 12:11, Bruce Richardson:
> On Thu, May 30, 2019 at 09:40:08AM +0200, Thomas Monjalon wrote:
> > 30/05/2019 09:31, David Marchand:
> > > On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> > > stephen@networkplumber.org> wrote:
> > > 
> > > > On Thu, 30 May 2019 00:46:30 +0200
> > > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > >
> > > > > 23/05/2019 15:58, David Marchand:
> > > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > > >
> > > > > > The fields of the internal EAL core configuration are currently
> > > > > > laid bare as part of the API. This is not good practice and limits
> > > > > > fixing issues with layout and sizes.
> > > > > >
> > > > > > Make new accessor functions for the fields used by current drivers
> > > > > > and examples.
> > > > > [...]
> > > > > > +DPDK_19.08 {
> > > > > > +   global:
> > > > > > +
> > > > > > +   rte_lcore_cpuset;
> > > > > > +   rte_lcore_index;
> > > > > > +   rte_lcore_to_cpu_id;
> > > > > > +   rte_lcore_to_socket_id;
> > > > > > +
> > > > > > +} DPDK_19.05;
> > > > > > +
> > > > > >  EXPERIMENTAL {
> > > > > >     global:
> > > > >
> > > > > Just to make sure, are we OK to introduce these functions
> > > > > as non-experimental?
> > > >
> > > > They were in previous releases as inlines this patch converts them
> > > > to real functions.
> > > >
> > > >
> > > Well, yes and no.
> > > 
> > > rte_lcore_index and rte_lcore_to_socket_id already existed, so making them
> > > part of the ABI is fine for me.
> > > 
> > > rte_lcore_to_cpu_id is new but seems quite safe in how it can be used,
> > > adding it to the ABI is ok for me.
> > 
> > It is used by DPAA and some test.
> > I guess adding as experimental is fine too?
> > I'm fine with both options, I'm just trying to apply the policy
> > we agreed on. Does this case deserve an exception?
> > 
> 
> While it may be a good candidate, I'm not sure how much making an exception
> for it really matters. I'd be tempted to just mark it experimental and then
> have it stable for the 19.11 release. What do we really lose by waiting a
> release to stabilize it?

I would agree Bruce.
If no more comment, I will wait for a v5 of this series.

Re: [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[David Marchand]

On Thu, May 30, 2019 at 3:39 PM Thomas Monjalon <thomas@monjalon.net> wrote:

> 30/05/2019 12:11, Bruce Richardson:
> > On Thu, May 30, 2019 at 09:40:08AM +0200, Thomas Monjalon wrote:
> > > 30/05/2019 09:31, David Marchand:
> > > > On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> > > > stephen@networkplumber.org> wrote:
> > > >
> > > > > On Thu, 30 May 2019 00:46:30 +0200
> > > > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > >
> > > > > > 23/05/2019 15:58, David Marchand:
> > > > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > > > >
> > > > > > > The fields of the internal EAL core configuration are currently
> > > > > > > laid bare as part of the API. This is not good practice and
> limits
> > > > > > > fixing issues with layout and sizes.
> > > > > > >
> > > > > > > Make new accessor functions for the fields used by current
> drivers
> > > > > > > and examples.
> > > > > > [...]
> > > > > > > +DPDK_19.08 {
> > > > > > > +   global:
> > > > > > > +
> > > > > > > +   rte_lcore_cpuset;
> > > > > > > +   rte_lcore_index;
> > > > > > > +   rte_lcore_to_cpu_id;
> > > > > > > +   rte_lcore_to_socket_id;
> > > > > > > +
> > > > > > > +} DPDK_19.05;
> > > > > > > +
> > > > > > >  EXPERIMENTAL {
> > > > > > >     global:
> > > > > >
> > > > > > Just to make sure, are we OK to introduce these functions
> > > > > > as non-experimental?
> > > > >
> > > > > They were in previous releases as inlines this patch converts them
> > > > > to real functions.
> > > > >
> > > > >
> > > > Well, yes and no.
> > > >
> > > > rte_lcore_index and rte_lcore_to_socket_id already existed, so
> making them
> > > > part of the ABI is fine for me.
> > > >
> > > > rte_lcore_to_cpu_id is new but seems quite safe in how it can be
> used,
> > > > adding it to the ABI is ok for me.
> > >
> > > It is used by DPAA and some test.
> > > I guess adding as experimental is fine too?
> > > I'm fine with both options, I'm just trying to apply the policy
> > > we agreed on. Does this case deserve an exception?
> > >
> >
> > While it may be a good candidate, I'm not sure how much making an
> exception
> > for it really matters. I'd be tempted to just mark it experimental and
> then
> > have it stable for the 19.11 release. What do we really lose by waiting a
> > release to stabilize it?
>
> I would agree Bruce.
> If no more comment, I will wait for a v5 of this series.
>

I agree that there is no reason we make an exception for those 2 new ones.

But to me the existing rte_lcore_index and rte_lcore_to_socket_id must be
marked as stable.
This is to avoid breaking existing users that did not set
ALLOW_EXPERIMENTAL_API.

I will prepare a v5 later.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors

[Bruce Richardson]

On Thu, May 30, 2019 at 07:00:36PM +0200, David Marchand wrote:
>    On Thu, May 30, 2019 at 3:39 PM Thomas Monjalon
>    <[1]thomas@monjalon.net> wrote:
> 
>      30/05/2019 12:11, Bruce Richardson:
>      > On Thu, May 30, 2019 at 09:40:08AM +0200, Thomas Monjalon wrote:
>      > > 30/05/2019 09:31, David Marchand:
>      > > > On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
>      > > > [2]stephen@networkplumber.org> wrote:
>      > > >
>      > > > > On Thu, 30 May 2019 00:46:30 +0200
>      > > > > Thomas Monjalon <[3]thomas@monjalon.net> wrote:
>      > > > >
>      > > > > > 23/05/2019 15:58, David Marchand:
>      > > > > > > From: Stephen Hemminger <[4]stephen@networkplumber.org>
>      > > > > > >
>      > > > > > > The fields of the internal EAL core configuration are
>      currently
>      > > > > > > laid bare as part of the API. This is not good practice
>      and limits
>      > > > > > > fixing issues with layout and sizes.
>      > > > > > >
>      > > > > > > Make new accessor functions for the fields used by
>      current drivers
>      > > > > > > and examples.
>      > > > > > [...]
>      > > > > > > +DPDK_19.08 {
>      > > > > > > +   global:
>      > > > > > > +
>      > > > > > > +   rte_lcore_cpuset;
>      > > > > > > +   rte_lcore_index;
>      > > > > > > +   rte_lcore_to_cpu_id;
>      > > > > > > +   rte_lcore_to_socket_id;
>      > > > > > > +
>      > > > > > > +} DPDK_19.05;
>      > > > > > > +
>      > > > > > >  EXPERIMENTAL {
>      > > > > > >     global:
>      > > > > >
>      > > > > > Just to make sure, are we OK to introduce these functions
>      > > > > > as non-experimental?
>      > > > >
>      > > > > They were in previous releases as inlines this patch
>      converts them
>      > > > > to real functions.
>      > > > >
>      > > > >
>      > > > Well, yes and no.
>      > > >
>      > > > rte_lcore_index and rte_lcore_to_socket_id already existed, so
>      making them
>      > > > part of the ABI is fine for me.
>      > > >
>      > > > rte_lcore_to_cpu_id is new but seems quite safe in how it can
>      be used,
>      > > > adding it to the ABI is ok for me.
>      > >
>      > > It is used by DPAA and some test.
>      > > I guess adding as experimental is fine too?
>      > > I'm fine with both options, I'm just trying to apply the policy
>      > > we agreed on. Does this case deserve an exception?
>      > >
>      >
>      > While it may be a good candidate, I'm not sure how much making an
>      exception
>      > for it really matters. I'd be tempted to just mark it experimental
>      and then
>      > have it stable for the 19.11 release. What do we really lose by
>      waiting a
>      > release to stabilize it?
>      I would agree Bruce.
>      If no more comment, I will wait for a v5 of this series.
> 
>    I agree that there is no reason we make an exception for those 2 new
>    ones.
>    But to me the existing rte_lcore_index and rte_lcore_to_socket_id must
>    be marked as stable.
>    This is to avoid breaking existing users that did not set
>    ALLOW_EXPERIMENTAL_API.
>    I will prepare a v5 later.
>    --
Yes, agreed. Any existing APIs that were already present as static inlines
can go straight to stable when added to the .map file.

/Bruce

[dpdk-dev] [PATCH v5 0/5] make lcore_config internal

[David Marchand]

This set of patches makes the lcore_config structure less visible
as part of the ABI.  This version does not break the ABI (yet)
follow on patch moves lcore_config into eal_private.h

Changelog since v4:
The only change is in patch 2: marked new apis as experimental.

Changelog since v3:
I took the liberty of taking over Stephen series.
I rebased and did some adjustments following [1] cleanups.
As stated before, we will still need a deprecation notice when hiding
lcore_config but this series does not break API nor ABI.

Changelog since v2:
 - new patch to use unsigned int in lcore.h first
 - incorporate feedback from David
 - don't include last patch to make it private
        (to avoid accidental early merge)

1: http://patchwork.dpdk.org/patch/53621/

-- 
David Marchand

Stephen Hemminger (5):
  eal: use unsigned int in lcore API prototypes
  eal: add lcore accessors
  drivers/bus: use lcore accessors
  examples/bond: use lcore accessors
  test: use lcore accessors

 app/test/test_cryptodev.c                 |  2 +-
 app/test/test_ring_perf.c                 | 22 +++++++------
 app/test/test_stack_perf.c                | 20 ++++++------
 drivers/bus/dpaa/dpaa_bus.c               |  6 ++--
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c  |  4 ++-
 examples/bond/main.c                      | 13 ++++----
 lib/librte_eal/common/eal_common_lcore.c  | 33 ++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 52 ++++++++++++++++++++-----------
 lib/librte_eal/rte_eal_version.map        | 12 +++++++
 9 files changed, 116 insertions(+), 48 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v5 1/5] eal: use unsigned int in lcore API prototypes

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

Purely cosmetic change, use unsigned int instead of unsigned alone.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/include/rte_lcore.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Changelog since v3:
- updated title

diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index be757a3..705594a 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -137,7 +137,7 @@ struct lcore_config {
  * @return
  *   the ID of current lcoreid's physical socket
  */
-unsigned rte_socket_id(void);
+unsigned int rte_socket_id(void);
 
 /**
  * Return number of physical sockets detected on the system.
@@ -177,8 +177,8 @@ struct lcore_config {
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned
-rte_lcore_to_socket_id(unsigned lcore_id)
+static inline unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
 {
 	return lcore_config[lcore_id].socket_id;
 }
@@ -193,7 +193,7 @@ struct lcore_config {
  *   True if the given lcore is enabled; false otherwise.
  */
 static inline int
-rte_lcore_is_enabled(unsigned lcore_id)
+rte_lcore_is_enabled(unsigned int lcore_id)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
 	if (lcore_id >= RTE_MAX_LCORE)
@@ -214,8 +214,8 @@ struct lcore_config {
  * @return
  *   The next lcore_id or RTE_MAX_LCORE if not found.
  */
-static inline unsigned
-rte_get_next_lcore(unsigned i, int skip_master, int wrap)
+static inline unsigned int
+rte_get_next_lcore(unsigned int i, int skip_master, int wrap)
 {
 	i++;
 	if (wrap)
-- 
1.8.3.1

[dpdk-dev] [PATCH v5 2/5] eal: add lcore accessors

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

The fields of the internal EAL core configuration are currently
laid bare as part of the API. This is not good practice and limits
fixing issues with layout and sizes.

Make new accessor functions for the fields used by current drivers
and examples.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/eal_common_lcore.c  | 33 +++++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h | 44 +++++++++++++++++++++----------
 lib/librte_eal/rte_eal_version.map        | 12 +++++++++
 3 files changed, 75 insertions(+), 14 deletions(-)

---
Changelog since v4:
- marked rte_lcore_to_cpu_id and rte_lcore_cpuset as experimental

Changelog since v3:
- updated title
- rebased on master
- removed doc update
- removed unneeded rte_lcore_return_code

diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 8c2744f..38af260 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -16,6 +16,39 @@
 #include "eal_private.h"
 #include "eal_thread.h"
 
+int rte_lcore_index(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_index;
+}
+
+int rte_lcore_to_cpu_id(int lcore_id)
+{
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -1;
+
+	if (lcore_id < 0)
+		lcore_id = (int)rte_lcore_id();
+
+	return lcore_config[lcore_id].core_id;
+}
+
+rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].cpuset;
+}
+
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
+{
+	return lcore_config[lcore_id].socket_id;
+}
+
 static int
 socket_id_cmp(const void *a, const void *b)
 {
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 705594a..1e3c887 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -121,15 +121,7 @@ struct lcore_config {
  * @return
  *   The relative index, or -1 if not enabled.
  */
-static inline int
-rte_lcore_index(int lcore_id)
-{
-	if (lcore_id >= RTE_MAX_LCORE)
-		return -1;
-	if (lcore_id < 0)
-		lcore_id = (int)rte_lcore_id();
-	return lcore_config[lcore_id].core_index;
-}
+int rte_lcore_index(int lcore_id);
 
 /**
  * Return the ID of the physical socket of the logical core we are
@@ -177,11 +169,35 @@ struct lcore_config {
  * @return
  *   the ID of lcoreid's physical socket
  */
-static inline unsigned int
-rte_lcore_to_socket_id(unsigned int lcore_id)
-{
-	return lcore_config[lcore_id].socket_id;
-}
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Return the id of the lcore on a socket starting from zero.
+ *
+ * @param lcore_id
+ *   The targeted lcore, or -1 for the current one.
+ * @return
+ *   The relative index, or -1 if not enabled.
+ */
+__rte_experimental int
+rte_lcore_to_cpu_id(int lcore_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Return the cpuset for a given lcore.
+ * @param lcore_id
+ *   the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
+ * @return
+ *   The cpuset of that lcore
+ */
+__rte_experimental rte_cpuset_t
+rte_lcore_cpuset(unsigned int lcore_id);
 
 /**
  * Test if an lcore is enabled.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 2454934..824edf0 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -287,6 +287,14 @@ DPDK_19.05 {
 
 } DPDK_18.11;
 
+DPDK_19.08 {
+	global:
+
+	rte_lcore_index;
+	rte_lcore_to_socket_id;
+
+} DPDK_19.05;
+
 EXPERIMENTAL {
 	global:
 
@@ -378,4 +386,8 @@ EXPERIMENTAL {
 	rte_service_lcore_attr_get;
 	rte_service_lcore_attr_reset_all;
 	rte_service_may_be_active;
+
+	# added in 19.08
+	rte_lcore_cpuset;
+	rte_lcore_to_cpu_id;
 };
-- 
1.8.3.1

[dpdk-dev] [PATCH v5 3/5] drivers/bus: use lcore accessors

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

The lcore_config structure will be hidden in future release.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/dpaa/dpaa_bus.c              | 6 ++++--
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

---
Changelog since v3:
- updated title

diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index ac20ecc..08c8227 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -254,6 +254,7 @@ int rte_dpaa_portal_init(void *arg)
 	unsigned int cpu, lcore = rte_lcore_id();
 	int ret;
 	struct dpaa_portal *dpaa_io_portal;
+	rte_cpuset_t cpuset;
 
 	BUS_INIT_FUNC_TRACE();
 
@@ -263,12 +264,13 @@ int rte_dpaa_portal_init(void *arg)
 		if (lcore >= RTE_MAX_LCORE)
 			return -1;
 
-	cpu = lcore_config[lcore].core_id;
+	cpu = rte_lcore_to_cpu_id(lcore);
 
 	/* Set CPU affinity for this thread.*/
 	id = pthread_self();
+	cpuset = rte_lcore_cpuset(lcore);
 	ret = pthread_setaffinity_np(id, sizeof(cpu_set_t),
-			&lcore_config[lcore].cpuset);
+				     &cpuset);
 	if (ret) {
 		DPAA_BUS_LOG(ERR, "pthread_setaffinity_np failed on core :%u"
 			     " (lcore=%u) with ret: %d", cpu, lcore, ret);
diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index 7bcbde8..8efb24a 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -366,7 +366,9 @@ static struct dpaa2_dpio_dev *dpaa2_get_qbman_swp(int lcoreid)
 
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		for (i = 0; i < RTE_MAX_LCORE; i++) {
-			if (CPU_ISSET(i, &lcore_config[lcore_id].cpuset)) {
+			rte_cpuset_t cpuset = rte_lcore_cpuset(lcore_id);
+
+			if (CPU_ISSET(i, &cpuset)) {
 				RTE_LOG(DEBUG, EAL, "lcore id = %u cpu=%u\n",
 					lcore_id, i);
 				if (dpaa2_cpu[lcore_id] != 0xffffffff) {
-- 
1.8.3.1

[dpdk-dev] [PATCH v5 4/5] examples/bond: use lcore accessors

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

Referring to lcore_config directly is no longer recommended.
Also remove unnecessary assignment of slave_core_id.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 examples/bond/main.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

---
Changelog since v3:
- updated title

diff --git a/examples/bond/main.c b/examples/bond/main.c
index 9b9ed56..4c650ef 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -531,8 +531,9 @@ static void cmd_start_parsed(__attribute__((unused)) void *parsed_result,
 	int slave_core_id = rte_lcore_id();
 
 	rte_spinlock_trylock(&global_flag_stru_p->lock);
-	if (global_flag_stru_p->LcoreMainIsRunning == 0)	{
-		if (lcore_config[global_flag_stru_p->LcoreMainCore].state != WAIT)	{
+	if (global_flag_stru_p->LcoreMainIsRunning == 0) {
+		if (rte_eal_get_lcore_state(global_flag_stru_p->LcoreMainCore)
+		    != WAIT) {
 			rte_spinlock_unlock(&global_flag_stru_p->lock);
 			return;
 		}
@@ -765,7 +766,7 @@ static void prompt(__attribute__((unused)) void *arg1)
 int
 main(int argc, char *argv[])
 {
-	int ret;
+	int ret, slave_core_id;
 	uint16_t nb_ports, i;
 
 	/* init EAL */
@@ -797,13 +798,13 @@ static void prompt(__attribute__((unused)) void *arg1)
 	bond_port_init(mbuf_pool);
 
 	rte_spinlock_init(&global_flag_stru_p->lock);
-	int slave_core_id = rte_lcore_id();
 
 	/* check state of lcores */
 	RTE_LCORE_FOREACH_SLAVE(slave_core_id) {
-	if (lcore_config[slave_core_id].state != WAIT)
-		return -EBUSY;
+		if (rte_eal_get_lcore_state(slave_core_id) != WAIT)
+			return -EBUSY;
 	}
+
 	/* start lcore main on core != master_core - ARP response thread */
 	slave_core_id = rte_get_next_lcore(rte_lcore_id(), 1, 0);
 	if ((slave_core_id >= RTE_MAX_LCORE) || (slave_core_id == 0))
-- 
1.8.3.1

[dpdk-dev] [PATCH v5 5/5] test: use lcore accessors

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

Don't refer to lcore_config directly.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_cryptodev.c  |  2 +-
 app/test/test_ring_perf.c  | 22 ++++++++++++----------
 app/test/test_stack_perf.c | 20 ++++++++++----------
 3 files changed, 23 insertions(+), 21 deletions(-)

---
Changelog since v3:
- updated title
- removed parts on test_hash_readwrite_lf

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9f31aaa..eca6d3d 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -378,7 +378,7 @@ struct crypto_unittest_params {
 			strcpy(temp_str, vdev_args);
 			strlcat(temp_str, ";", sizeof(temp_str));
 			slave_core_count++;
-			socket_id = lcore_config[i].socket_id;
+			socket_id = rte_lcore_to_socket_id(i);
 		}
 		if (slave_core_count != 2) {
 			RTE_LOG(ERR, USER1,
diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
index ebb3939..6eccccf 100644
--- a/app/test/test_ring_perf.c
+++ b/app/test/test_ring_perf.c
@@ -52,10 +52,11 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 == c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -75,10 +76,11 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			c1 = lcore_config[id1].core_id;
-			c2 = lcore_config[id2].core_id;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+
+			c1 = rte_lcore_to_cpu_id(id1);
+			c2 = rte_lcore_to_cpu_id(id2);
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if ((c1 != c2) && (s1 == s2)){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
@@ -98,8 +100,8 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id2) {
 			if (id1 == id2)
 				continue;
-			s1 = lcore_config[id1].socket_id;
-			s2 = lcore_config[id2].socket_id;
+			s1 = rte_lcore_to_socket_id(id1);
+			s2 = rte_lcore_to_socket_id(id2);
 			if (s1 != s2){
 				lcp->c1 = id1;
 				lcp->c2 = id2;
diff --git a/app/test/test_stack_perf.c b/app/test/test_stack_perf.c
index ba27fbf..70561fe 100644
--- a/app/test/test_stack_perf.c
+++ b/app/test/test_stack_perf.c
@@ -44,10 +44,10 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] == core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -70,10 +70,10 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			core[0] = lcore_config[id[0]].core_id;
-			core[1] = lcore_config[id[1]].core_id;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			core[0] = rte_lcore_to_cpu_id(id[0]);
+			core[1] = rte_lcore_to_cpu_id(id[1]);
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if ((core[0] != core[1]) && (socket[0] == socket[1])) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
@@ -95,8 +95,8 @@ struct lcore_pair {
 		RTE_LCORE_FOREACH(id[1]) {
 			if (id[0] == id[1])
 				continue;
-			socket[0] = lcore_config[id[0]].socket_id;
-			socket[1] = lcore_config[id[1]].socket_id;
+			socket[0] = rte_lcore_to_socket_id(id[0]);
+			socket[1] = rte_lcore_to_socket_id(id[1]);
 			if (socket[0] != socket[1]) {
 				lcp->c1 = id[0];
 				lcp->c2 = id[1];
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v5 0/5] make lcore_config internal

[Thomas Monjalon]

31/05/2019 17:36, David Marchand:
> This set of patches makes the lcore_config structure less visible
> as part of the ABI.  This version does not break the ABI (yet)
> follow on patch moves lcore_config into eal_private.h
> 
> Changelog since v4:
> The only change is in patch 2: marked new apis as experimental.
> 
> Changelog since v3:
> I took the liberty of taking over Stephen series.
> I rebased and did some adjustments following [1] cleanups.
> As stated before, we will still need a deprecation notice when hiding
> lcore_config but this series does not break API nor ABI.
> 
> Changelog since v2:
>  - new patch to use unsigned int in lcore.h first
>  - incorporate feedback from David
>  - don't include last patch to make it private
>         (to avoid accidental early merge)

Applied, thanks

Re: [dpdk-dev] [PATCH v5 0/5] make lcore_config internal

[Stephen Hemminger]

On Mon, 03 Jun 2019 12:32:02 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 31/05/2019 17:36, David Marchand:
> > This set of patches makes the lcore_config structure less visible
> > as part of the ABI.  This version does not break the ABI (yet)
> > follow on patch moves lcore_config into eal_private.h
> > 
> > Changelog since v4:
> > The only change is in patch 2: marked new apis as experimental.
> > 
> > Changelog since v3:
> > I took the liberty of taking over Stephen series.
> > I rebased and did some adjustments following [1] cleanups.
> > As stated before, we will still need a deprecation notice when hiding
> > lcore_config but this series does not break API nor ABI.
> > 
> > Changelog since v2:
> >  - new patch to use unsigned int in lcore.h first
> >  - incorporate feedback from David
> >  - don't include last patch to make it private
> >         (to avoid accidental early merge)  
> 
> Applied, thanks
> 
> 

Thanks. Now to get to eal_config

Re: [dpdk-dev] [PATCH v5 5/5] test: use lcore accessors

[Eads, Gage]

> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> Don't refer to lcore_config directly.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Gage Eads <gage.eads@intel.com>