[dpdk-dev] [RFC 0/4] new function to create control threads

[dpdk-dev] [RFC 0/4] new function to create control threads

[Olivier Matz]

This patchset targets 18.05.
It applies on top of 17.11 + [1] and [2].

Some parts of dpdk use their own management threads. Most of the time,
the affinity of the thread is not properly set: it should not be scheduled
on the dataplane cores, because interrupting them can cause packet losses.

This patchset introduces a new wrapper for thread creation that does
the job automatically, avoiding code duplication.

[1] http://dpdk.org/ml/archives/dev/2017-December/083279.html
[2] http://dpdk.org/ml/archives/dev/2017-December/083280.html

Olivier Matz (4):
  eal: use sizeof to avoid a double use of a define
  eal: new function to create control threads
  eal: set name when creating a control thread
  eal: set affinity for control threads

 lib/librte_eal/bsdapp/eal/eal.c                |  2 +-
 lib/librte_eal/bsdapp/eal/eal_thread.c         |  2 +-
 lib/librte_eal/common/eal_common_thread.c      | 74 ++++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h      | 25 +++++++++
 lib/librte_eal/linuxapp/eal/eal.c              |  4 +-
 lib/librte_eal/linuxapp/eal/eal_interrupts.c   | 17 ++----
 lib/librte_eal/linuxapp/eal/eal_thread.c       |  2 +-
 lib/librte_eal/linuxapp/eal/eal_timer.c        | 12 +----
 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 10 +---
 lib/librte_eal/rte_eal_version.map             |  1 +
 lib/librte_pdump/rte_pdump.c                   | 13 ++---
 lib/librte_vhost/socket.c                      |  7 +--
 12 files changed, 122 insertions(+), 47 deletions(-)

-- 
2.11.0

[dpdk-dev] [RFC 1/4] eal: use sizeof to avoid a double use of a define

[Olivier Matz]

Only a cosmetic change.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c          | 2 +-
 lib/librte_eal/bsdapp/eal/eal_thread.c   | 2 +-
 lib/librte_eal/linuxapp/eal/eal.c        | 4 ++--
 lib/librte_eal/linuxapp/eal/eal_thread.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 369a682a8..8d88119fc 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -642,7 +642,7 @@ rte_eal_init(int argc, char **argv)
 
 	eal_thread_init_master(rte_config.master_lcore);
 
-	ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
 	RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
 		rte_config.master_lcore, thread_id, cpuset,
diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c
index 2a2136a21..2498c4773 100644
--- a/lib/librte_eal/bsdapp/eal/eal_thread.c
+++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
@@ -148,7 +148,7 @@ eal_thread_loop(__attribute__((unused)) void *arg)
 	if (eal_thread_set_affinity() < 0)
 		rte_panic("cannot set affinity\n");
 
-	ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
 		lcore_id, thread_id, cpuset, ret == 0 ? "" : "...");
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 229eec9f1..9c7d7620e 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -898,7 +898,7 @@ rte_eal_init(int argc, char **argv)
 
 	eal_thread_init_master(rte_config.master_lcore);
 
-	ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
 	RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%x;cpuset=[%s%s])\n",
 		rte_config.master_lcore, (int)thread_id, cpuset,
@@ -929,7 +929,7 @@ rte_eal_init(int argc, char **argv)
 			rte_panic("Cannot create thread\n");
 
 		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
+		snprintf(thread_name, sizeof(thread_name),
 			"lcore-slave-%d", i);
 		ret = rte_thread_setname(lcore_config[i].thread_id,
 						thread_name);
diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c b/lib/librte_eal/linuxapp/eal/eal_thread.c
index e9a579e46..082d091d0 100644
--- a/lib/librte_eal/linuxapp/eal/eal_thread.c
+++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
@@ -148,7 +148,7 @@ eal_thread_loop(__attribute__((unused)) void *arg)
 	if (eal_thread_set_affinity() < 0)
 		rte_panic("cannot set affinity\n");
 
-	ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%x;cpuset=[%s%s])\n",
 		lcore_id, (int)thread_id, cpuset, ret == 0 ? "" : "...");
-- 
2.11.0

[dpdk-dev] [RFC 2/4] eal: new function to create control threads

[Olivier Matz]

Many parts of dpdk use their own management threads. Introduce a new
wrapper for thread creation that will be extended in next commit to set
the name and affinity.

To be consistent with other DPDK APIs, the return value is negative in
case of error, which was not the case for pthread_create().

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/common/eal_common_thread.c      |  7 +++++++
 lib/librte_eal/common/include/rte_lcore.h      | 20 ++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_interrupts.c   |  6 +++---
 lib/librte_eal/linuxapp/eal/eal_timer.c        |  2 +-
 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c |  2 +-
 lib/librte_eal/rte_eal_version.map             |  1 +
 lib/librte_pdump/rte_pdump.c                   |  5 +++--
 lib/librte_vhost/socket.c                      |  6 +++---
 8 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index 55e96963b..f1a88f85e 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -169,3 +169,10 @@ eal_thread_dump_affinity(char *str, unsigned size)
 
 	return ret;
 }
+
+int rte_ctrl_thread_create(pthread_t *thread,
+			const pthread_attr_t *attr,
+			void *(*start_routine)(void *), void *arg)
+{
+	return pthread_create(thread, attr, start_routine, arg);
+}
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index c89e6bab1..11f481c52 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -263,6 +263,26 @@ void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
 int rte_thread_setname(pthread_t id, const char *name);
 
 /**
+ * Create a control thread.
+ *
+ * Wrapper to pthread_create().
+ *
+ * @param thread
+ *   Filled with the thread id of the new created thread.
+ * @param attr
+ *   Attributes for the new thread.
+ * @param start_routine
+ *   Function to be executed by the new thread.
+ * @param arg
+ *   Argument passed to start_routine.
+ * @return
+ *   On success, returns 0; on error, it returns a negative value
+ *   corresponding to the error number.
+ */
+int rte_ctrl_thread_create(pthread_t *thread, const pthread_attr_t *attr,
+			void *(*start_routine)(void *), void *arg);
+
+/**
  * Test if the core supplied has a specific role
  *
  * @param lcore_id
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 1c20693d9..8fee317e5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -889,10 +889,10 @@ rte_eal_intr_init(void)
 	}
 
 	/* create the host thread to wait/handle the interrupt */
-	ret = pthread_create(&intr_thread, NULL,
+	ret = rte_ctrl_thread_create(&intr_thread, NULL,
 			eal_intr_thread_main, NULL);
 	if (ret != 0) {
-		rte_errno = ret;
+		rte_errno = -ret;
 		RTE_LOG(ERR, EAL,
 			"Failed to create thread for interrupt handling\n");
 	} else {
@@ -905,7 +905,7 @@ rte_eal_intr_init(void)
 			"Failed to set thread name for interrupt handling\n");
 	}
 
-	return -ret;
+	return ret;
 }
 
 static void
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index a616928be..1766f9353 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -207,7 +207,7 @@ rte_eal_hpet_init(int make_default)
 
 	/* create a thread that will increment a global variable for
 	 * msb (hpet is 32 bits by default under linux) */
-	ret = pthread_create(&msb_inc_thread_id, NULL,
+	ret = rte_ctrl_thread_create(&msb_inc_thread_id, NULL,
 			(void *(*)(void *))hpet_msb_inc, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, EAL, "ERROR: Cannot create HPET timer thread!\n");
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index b53ed7eb3..d104f8920 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -403,7 +403,7 @@ vfio_mp_sync_setup(void)
 		return -1;
 	}
 
-	ret = pthread_create(&socket_thread, NULL,
+	ret = rte_ctrl_thread_create(&socket_thread, NULL,
 			vfio_mp_sync_thread, NULL);
 	if (ret) {
 		RTE_LOG(ERR, EAL,
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index f4f46c1be..49b02aa08 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -203,6 +203,7 @@ DPDK_17.11 {
 EXPERIMENTAL {
 	global:
 
+	rte_ctrl_thread_create;
 	rte_eal_devargs_insert;
 	rte_eal_devargs_parse;
 	rte_eal_devargs_remove;
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 44dcc9531..70de0c48b 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -577,11 +577,12 @@ rte_pdump_init(const char *path)
 	}
 
 	/* create the host thread to wait/handle pdump requests */
-	ret = pthread_create(&pdump_thread, NULL, pdump_thread_main, NULL);
+	ret = rte_ctrl_thread_create(&pdump_thread, NULL,
+				pdump_thread_main, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
 			"Failed to create the pdump thread:%s, %s:%d\n",
-			strerror(ret), __func__, __LINE__);
+			strerror(-ret), __func__, __LINE__);
 		return -1;
 	}
 	/* Set thread_name for aid in debugging. */
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 811e6bf16..905d040a4 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -470,7 +470,7 @@ vhost_user_reconnect_init(void)
 	}
 	TAILQ_INIT(&reconn_list.head);
 
-	ret = pthread_create(&reconn_tid, NULL,
+	ret = rte_ctrl_thread_create(&reconn_tid, NULL,
 			     vhost_user_client_reconnect, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, VHOST_CONFIG, "failed to create reconnect thread");
@@ -834,8 +834,8 @@ rte_vhost_driver_start(const char *path)
 		return -1;
 
 	if (fdset_tid == 0) {
-		int ret = pthread_create(&fdset_tid, NULL, fdset_event_dispatch,
-				     &vhost_user.fdset);
+		int ret = rte_ctrl_thread_create(&fdset_tid, NULL,
+			fdset_event_dispatch, &vhost_user.fdset);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"failed to create fdset handling thread");
-- 
2.11.0

[dpdk-dev] [RFC 3/4] eal: set name when creating a control thread

[Olivier Matz]

To avoid code duplication, add a parameter to rte_ctrl_thread_create()
to specify the name of the thread.

This requires to add a wrapper for the thread start routine in
rte_thread_init(), which will first wait that the thread is configured.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/common/eal_common_thread.c      | 53 +++++++++++++++++++++++++-
 lib/librte_eal/common/include/rte_lcore.h      |  7 +++-
 lib/librte_eal/linuxapp/eal/eal_interrupts.c   | 13 ++-----
 lib/librte_eal/linuxapp/eal/eal_timer.c        | 12 +-----
 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 10 +----
 lib/librte_pdump/rte_pdump.c                   | 10 +----
 lib/librte_vhost/socket.c                      |  7 ++--
 7 files changed, 69 insertions(+), 43 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index f1a88f85e..b4335e04e 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -36,6 +36,7 @@
 #include <stdint.h>
 #include <unistd.h>
 #include <pthread.h>
+#include <signal.h>
 #include <sched.h>
 #include <assert.h>
 #include <string.h>
@@ -170,9 +171,57 @@ eal_thread_dump_affinity(char *str, unsigned size)
 	return ret;
 }
 
-int rte_ctrl_thread_create(pthread_t *thread,
+
+struct rte_thread_ctrl_params {
+	void *(*start_routine)(void *);
+	void *arg;
+	pthread_barrier_t launched;
+	pthread_barrier_t configured;
+};
+
+static void *rte_thread_init(void *arg)
+{
+	struct rte_thread_ctrl_params *params = arg;
+	void *(*start_routine)(void *) = params->start_routine;
+	void *routine_arg = params->arg;
+
+	pthread_barrier_wait(&params->launched);
+	pthread_barrier_wait(&params->configured);
+
+	return start_routine(routine_arg);
+}
+
+int rte_ctrl_thread_create(pthread_t *thread, const char *name,
 			const pthread_attr_t *attr,
 			void *(*start_routine)(void *), void *arg)
 {
-	return pthread_create(thread, attr, start_routine, arg);
+	struct rte_thread_ctrl_params params = {
+		.start_routine = start_routine,
+		.arg = arg,
+	};
+	int ret;
+
+	pthread_barrier_init(&params.launched, NULL, 2);
+	pthread_barrier_init(&params.configured, NULL, 2);
+
+	ret = pthread_create(thread, attr, rte_thread_init, (void *)&params);
+	if (ret != 0)
+		return ret;
+
+	pthread_barrier_wait(&params.launched);
+
+	if (name != NULL) {
+		ret = rte_thread_setname(*thread, name);
+		if (ret < 0)
+			goto fail;
+	}
+
+	pthread_barrier_wait(&params.configured);
+
+	return 0;
+
+fail:
+	pthread_kill(*thread, SIGTERM);
+	pthread_join(*thread, NULL);
+	return ret;
 }
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 11f481c52..f7f67868c 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -265,10 +265,12 @@ int rte_thread_setname(pthread_t id, const char *name);
 /**
  * Create a control thread.
  *
- * Wrapper to pthread_create().
+ * Wrapper to pthread_create() and pthread_setname_np().
  *
  * @param thread
  *   Filled with the thread id of the new created thread.
+ * @param name
+ *   The name of the control thread (max 16 characters including '\0').
  * @param attr
  *   Attributes for the new thread.
  * @param start_routine
@@ -279,7 +281,8 @@ int rte_thread_setname(pthread_t id, const char *name);
  *   On success, returns 0; on error, it returns a negative value
  *   corresponding to the error number.
  */
-int rte_ctrl_thread_create(pthread_t *thread, const pthread_attr_t *attr,
+int rte_ctrl_thread_create(pthread_t *thread, const char *name,
+			const pthread_attr_t *attr,
 			void *(*start_routine)(void *), void *arg);
 
 /**
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 8fee317e5..4b5e6da31 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -873,7 +873,7 @@ eal_intr_thread_main(__rte_unused void *arg)
 int
 rte_eal_intr_init(void)
 {
-	int ret = 0, ret_1 = 0;
+	int ret = 0;
 	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	/* init the global interrupt source head */
@@ -889,20 +889,13 @@ rte_eal_intr_init(void)
 	}
 
 	/* create the host thread to wait/handle the interrupt */
-	ret = rte_ctrl_thread_create(&intr_thread, NULL,
+	snprintf(thread_name, sizeof(thread_name), "eal-intr-thread");
+	ret = rte_ctrl_thread_create(&intr_thread, thread_name, NULL,
 			eal_intr_thread_main, NULL);
 	if (ret != 0) {
 		rte_errno = -ret;
 		RTE_LOG(ERR, EAL,
 			"Failed to create thread for interrupt handling\n");
-	} else {
-		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
-			"eal-intr-thread");
-		ret_1 = rte_thread_setname(intr_thread, thread_name);
-		if (ret_1 != 0)
-			RTE_LOG(DEBUG, EAL,
-			"Failed to set thread name for interrupt handling\n");
 	}
 
 	return ret;
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index 1766f9353..669dc9ceb 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -207,7 +207,8 @@ rte_eal_hpet_init(int make_default)
 
 	/* create a thread that will increment a global variable for
 	 * msb (hpet is 32 bits by default under linux) */
-	ret = rte_ctrl_thread_create(&msb_inc_thread_id, NULL,
+	snprintf(thread_name, sizeof(thread_name), "hpet-msb-inc");
+	ret = rte_ctrl_thread_create(&msb_inc_thread_id, thread_name, NULL,
 			(void *(*)(void *))hpet_msb_inc, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, EAL, "ERROR: Cannot create HPET timer thread!\n");
@@ -215,15 +216,6 @@ rte_eal_hpet_init(int make_default)
 		return -1;
 	}
 
-	/*
-	 * Set thread_name for aid in debugging.
-	 */
-	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "hpet-msb-inc");
-	ret = rte_thread_setname(msb_inc_thread_id, thread_name);
-	if (ret != 0)
-		RTE_LOG(DEBUG, EAL,
-			"Cannot set HPET timer thread name!\n");
-
 	if (make_default)
 		eal_timer_source = EAL_TIMER_HPET;
 	return 0;
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index d104f8920..a20fb8691 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -403,7 +403,8 @@ vfio_mp_sync_setup(void)
 		return -1;
 	}
 
-	ret = rte_ctrl_thread_create(&socket_thread, NULL,
+	snprintf(thread_name, sizeof(thread_name), "vfio-sync");
+	ret = rte_ctrl_thread_create(&socket_thread, thread_name, NULL,
 			vfio_mp_sync_thread, NULL);
 	if (ret) {
 		RTE_LOG(ERR, EAL,
@@ -412,13 +413,6 @@ vfio_mp_sync_setup(void)
 		return -1;
 	}
 
-	/* Set thread_name for aid in debugging. */
-	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "vfio-sync");
-	ret = rte_thread_setname(socket_thread, thread_name);
-	if (ret)
-		RTE_LOG(DEBUG, EAL,
-			"Failed to set thread name for secondary processes!\n");
-
 	return 0;
 }
 
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 70de0c48b..061263236 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -577,7 +577,8 @@ rte_pdump_init(const char *path)
 	}
 
 	/* create the host thread to wait/handle pdump requests */
-	ret = rte_ctrl_thread_create(&pdump_thread, NULL,
+	snprintf(thread_name, sizeof(thread_name), "pdump-thread");
+	ret = rte_ctrl_thread_create(&pdump_thread, thread_name, NULL,
 				pdump_thread_main, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
@@ -585,13 +586,6 @@ rte_pdump_init(const char *path)
 			strerror(-ret), __func__, __LINE__);
 		return -1;
 	}
-	/* Set thread_name for aid in debugging. */
-	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "pdump-thread");
-	ret = rte_thread_setname(pdump_thread, thread_name);
-	if (ret != 0) {
-		RTE_LOG(DEBUG, PDUMP,
-			"Failed to set thread name for pdump handling\n");
-	}
 
 	return 0;
 }
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 905d040a4..43d699ef7 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -470,7 +470,7 @@ vhost_user_reconnect_init(void)
 	}
 	TAILQ_INIT(&reconn_list.head);
 
-	ret = rte_ctrl_thread_create(&reconn_tid, NULL,
+	ret = rte_ctrl_thread_create(&reconn_tid, "vhost_reconnect", NULL,
 			     vhost_user_client_reconnect, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, VHOST_CONFIG, "failed to create reconnect thread");
@@ -834,8 +834,9 @@ rte_vhost_driver_start(const char *path)
 		return -1;
 
 	if (fdset_tid == 0) {
-		int ret = rte_ctrl_thread_create(&fdset_tid, NULL,
-			fdset_event_dispatch, &vhost_user.fdset);
+		int ret = rte_ctrl_thread_create(&fdset_tid,
+			"vhost_dispatch", NULL, fdset_event_dispatch,
+			&vhost_user.fdset);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"failed to create fdset handling thread");
-- 
2.11.0

[dpdk-dev] [RFC 4/4] eal: set affinity for control threads

[Olivier Matz]

The management threads must not bother the dataplane or service cores.
Set the affinity of these threads accordingly.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/common/eal_common_thread.c | 20 +++++++++++++++++++-
 lib/librte_eal/common/include/rte_lcore.h |  4 +++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index b4335e04e..f1798cebc 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -45,6 +45,7 @@
 #include <rte_memory.h>
 #include <rte_log.h>
 
+#include "eal_private.h"
 #include "eal_thread.h"
 
 RTE_DECLARE_PER_LCORE(unsigned , _socket_id);
@@ -199,7 +200,9 @@ int rte_ctrl_thread_create(pthread_t *thread, const char *name,
 		.start_routine = start_routine,
 		.arg = arg,
 	};
-	int ret;
+	unsigned int lcore_id;
+	rte_cpuset_t cpuset;
+	int set_affinity, ret;
 
 	pthread_barrier_init(&params.launched, NULL, 2);
 	pthread_barrier_init(&params.configured, NULL, 2);
@@ -216,6 +219,21 @@ int rte_ctrl_thread_create(pthread_t *thread, const char *name,
 			goto fail;
 	}
 
+	set_affinity = 0;
+	CPU_ZERO(&cpuset);
+	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+		if (eal_cpu_detected(lcore_id) &&
+				rte_lcore_has_role(lcore_id, ROLE_OFF)) {
+			CPU_SET(lcore_id, &cpuset);
+			set_affinity = 1;
+		}
+	}
+	if (set_affinity) {
+		ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
+		if (ret < 0)
+			goto fail;
+	}
+
 	pthread_barrier_wait(&params.configured);
 
 	return 0;
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index f7f67868c..0d85a91a8 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -265,7 +265,9 @@ int rte_thread_setname(pthread_t id, const char *name);
 /**
  * Create a control thread.
  *
- * Wrapper to pthread_create() and pthread_setname_np().
+ * Wrapper to pthread_create(), pthread_setname_np() and
+ * pthread_setaffinity_np(). The dataplane and service lcores are
+ * excluded from the affinity of the new thread.
  *
  * @param thread
  *   Filled with the thread id of the new created thread.
-- 
2.11.0

[dpdk-dev] [PATCH 0/4] fix control thread affinities

[Olivier Matz]

Some parts of dpdk use their own management threads. Most of the time,
the affinity of the thread is not properly set: it should not be scheduled
on the dataplane cores, because interrupting them can cause packet losses.

This patchset introduces a new wrapper for thread creation that does
the job automatically, avoiding code duplication.

Olivier Matz (4):
  eal: use sizeof to avoid a double use of a define
  eal: new function to create control threads
  eal: set name when creating a control thread
  eal: set affinity for control threads

 drivers/net/kni/Makefile                       |  1 +
 drivers/net/kni/rte_eth_kni.c                  |  3 +-
 lib/librte_eal/bsdapp/eal/eal.c                |  2 +-
 lib/librte_eal/bsdapp/eal/eal_thread.c         |  2 +-
 lib/librte_eal/common/eal_common_thread.c      | 70 ++++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h      | 26 ++++++++++
 lib/librte_eal/linuxapp/eal/eal.c              |  4 +-
 lib/librte_eal/linuxapp/eal/eal_interrupts.c   | 17 ++-----
 lib/librte_eal/linuxapp/eal/eal_thread.c       |  2 +-
 lib/librte_eal/linuxapp/eal/eal_timer.c        | 12 +----
 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 10 +---
 lib/librte_eal/rte_eal_version.map             |  1 +
 lib/librte_pdump/Makefile                      |  1 +
 lib/librte_pdump/rte_pdump.c                   | 13 ++---
 lib/librte_vhost/socket.c                      |  7 +--
 15 files changed, 123 insertions(+), 48 deletions(-)

-- 
2.11.0

[dpdk-dev] [PATCH 1/4] eal: use sizeof to avoid a double use of a define

[Olivier Matz]

Only a cosmetic change: the *_LEN defines are already used
when defining the buffer. Using sizeof() ensures that the length
stays consistent, even if the definition is modified.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c          | 2 +-
 lib/librte_eal/bsdapp/eal/eal_thread.c   | 2 +-
 lib/librte_eal/linuxapp/eal/eal.c        | 4 ++--
 lib/librte_eal/linuxapp/eal/eal_thread.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 4eafcb5ad..0b0fb9973 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -632,7 +632,7 @@ rte_eal_init(int argc, char **argv)
 
 	eal_thread_init_master(rte_config.master_lcore);
 
-	ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
 	RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
 		rte_config.master_lcore, thread_id, cpuset,
diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c
index d602daf81..309b58726 100644
--- a/lib/librte_eal/bsdapp/eal/eal_thread.c
+++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
@@ -119,7 +119,7 @@ eal_thread_loop(__attribute__((unused)) void *arg)
 	if (eal_thread_set_affinity() < 0)
 		rte_panic("cannot set affinity\n");
 
-	ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
 		lcore_id, thread_id, cpuset, ret == 0 ? "" : "...");
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 38306bf5c..1cb87ca25 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -888,7 +888,7 @@ rte_eal_init(int argc, char **argv)
 
 	eal_thread_init_master(rte_config.master_lcore);
 
-	ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
 	RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%x;cpuset=[%s%s])\n",
 		rte_config.master_lcore, (int)thread_id, cpuset,
@@ -919,7 +919,7 @@ rte_eal_init(int argc, char **argv)
 			rte_panic("Cannot create thread\n");
 
 		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
+		snprintf(thread_name, sizeof(thread_name),
 			"lcore-slave-%d", i);
 		ret = rte_thread_setname(lcore_config[i].thread_id,
 						thread_name);
diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c b/lib/librte_eal/linuxapp/eal/eal_thread.c
index 08e150b77..f652ff988 100644
--- a/lib/librte_eal/linuxapp/eal/eal_thread.c
+++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
@@ -119,7 +119,7 @@ eal_thread_loop(__attribute__((unused)) void *arg)
 	if (eal_thread_set_affinity() < 0)
 		rte_panic("cannot set affinity\n");
 
-	ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+	ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
 
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%x;cpuset=[%s%s])\n",
 		lcore_id, (int)thread_id, cpuset, ret == 0 ? "" : "...");
-- 
2.11.0

[dpdk-dev] [PATCH 2/4] eal: new function to create control threads

[Olivier Matz]

Many parts of dpdk use their own management threads. Introduce a new
wrapper for thread creation that will be extended in next commits to set
the name and affinity.

To be consistent with other DPDK APIs, the return value is negative in
case of error, which was not the case for pthread_create().

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/kni/Makefile                       |  1 +
 drivers/net/kni/rte_eth_kni.c                  |  2 +-
 lib/librte_eal/common/eal_common_thread.c      |  8 ++++++++
 lib/librte_eal/common/include/rte_lcore.h      | 21 +++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_interrupts.c   |  6 +++---
 lib/librte_eal/linuxapp/eal/eal_timer.c        |  2 +-
 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c |  2 +-
 lib/librte_eal/rte_eal_version.map             |  1 +
 lib/librte_pdump/Makefile                      |  1 +
 lib/librte_pdump/rte_pdump.c                   |  5 +++--
 lib/librte_vhost/socket.c                      |  6 +++---
 11 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/drivers/net/kni/Makefile b/drivers/net/kni/Makefile
index 01eaef056..562e8d2da 100644
--- a/drivers/net/kni/Makefile
+++ b/drivers/net/kni/Makefile
@@ -10,6 +10,7 @@ LIB = librte_pmd_kni.a
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 LDLIBS += -lpthread
 LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
 LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs -lrte_kni
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index dc4e65f5d..26718eb3e 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -149,7 +149,7 @@ eth_kni_dev_start(struct rte_eth_dev *dev)
 	}
 
 	if (internals->no_request_thread == 0) {
-		ret = pthread_create(&internals->thread, NULL,
+		ret = rte_ctrl_thread_create(&internals->thread, NULL,
 			kni_handle_request, internals);
 		if (ret) {
 			RTE_LOG(ERR, PMD,
diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index 40902e49b..efbccddbc 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -140,3 +140,11 @@ eal_thread_dump_affinity(char *str, unsigned size)
 
 	return ret;
 }
+
+__rte_experimental int
+rte_ctrl_thread_create(pthread_t *thread,
+			const pthread_attr_t *attr,
+			void *(*start_routine)(void *), void *arg)
+{
+	return pthread_create(thread, attr, start_routine, arg);
+}
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 047222030..f19075a88 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -247,6 +247,27 @@ void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
 int rte_thread_setname(pthread_t id, const char *name);
 
 /**
+ * Create a control thread.
+ *
+ * Wrapper to pthread_create().
+ *
+ * @param thread
+ *   Filled with the thread id of the new created thread.
+ * @param attr
+ *   Attributes for the new thread.
+ * @param start_routine
+ *   Function to be executed by the new thread.
+ * @param arg
+ *   Argument passed to start_routine.
+ * @return
+ *   On success, returns 0; on error, it returns a negative value
+ *   corresponding to the error number.
+ */
+__rte_experimental int
+rte_ctrl_thread_create(pthread_t *thread, const pthread_attr_t *attr,
+		void *(*start_routine)(void *), void *arg);
+
+/**
  * Test if the core supplied has a specific role
  *
  * @param lcore_id
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index f86f22f7b..d927fb45d 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -860,10 +860,10 @@ rte_eal_intr_init(void)
 	}
 
 	/* create the host thread to wait/handle the interrupt */
-	ret = pthread_create(&intr_thread, NULL,
+	ret = rte_ctrl_thread_create(&intr_thread, NULL,
 			eal_intr_thread_main, NULL);
 	if (ret != 0) {
-		rte_errno = ret;
+		rte_errno = -ret;
 		RTE_LOG(ERR, EAL,
 			"Failed to create thread for interrupt handling\n");
 	} else {
@@ -876,7 +876,7 @@ rte_eal_intr_init(void)
 			"Failed to set thread name for interrupt handling\n");
 	}
 
-	return -ret;
+	return ret;
 }
 
 static void
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index 161322f23..f12d2e134 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -178,7 +178,7 @@ rte_eal_hpet_init(int make_default)
 
 	/* create a thread that will increment a global variable for
 	 * msb (hpet is 32 bits by default under linux) */
-	ret = pthread_create(&msb_inc_thread_id, NULL,
+	ret = rte_ctrl_thread_create(&msb_inc_thread_id, NULL,
 			(void *(*)(void *))hpet_msb_inc, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, EAL, "ERROR: Cannot create HPET timer thread!\n");
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index 7cc3c1527..072c0a978 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -374,7 +374,7 @@ vfio_mp_sync_setup(void)
 		return -1;
 	}
 
-	ret = pthread_create(&socket_thread, NULL,
+	ret = rte_ctrl_thread_create(&socket_thread, NULL,
 			vfio_mp_sync_thread, NULL);
 	if (ret) {
 		RTE_LOG(ERR, EAL,
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index d12360235..24bb80567 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -214,6 +214,7 @@ DPDK_18.02 {
 EXPERIMENTAL {
 	global:
 
+	rte_ctrl_thread_create;
 	rte_eal_cleanup;
 	rte_eal_devargs_insert;
 	rte_eal_devargs_parse;
diff --git a/lib/librte_pdump/Makefile b/lib/librte_pdump/Makefile
index 98fa752ec..d72a90a60 100644
--- a/lib/librte_pdump/Makefile
+++ b/lib/librte_pdump/Makefile
@@ -6,6 +6,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
 # library name
 LIB = librte_pdump.a
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
 CFLAGS += -D_GNU_SOURCE
 LDLIBS += -lpthread
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index ec8a5d84c..917a99957 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -548,11 +548,12 @@ rte_pdump_init(const char *path)
 	}
 
 	/* create the host thread to wait/handle pdump requests */
-	ret = pthread_create(&pdump_thread, NULL, pdump_thread_main, NULL);
+	ret = rte_ctrl_thread_create(&pdump_thread, NULL,
+				pdump_thread_main, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
 			"Failed to create the pdump thread:%s, %s:%d\n",
-			strerror(ret), __func__, __LINE__);
+			strerror(-ret), __func__, __LINE__);
 		return -1;
 	}
 	/* Set thread_name for aid in debugging. */
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 83befdced..d261bf4b0 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -445,7 +445,7 @@ vhost_user_reconnect_init(void)
 	}
 	TAILQ_INIT(&reconn_list.head);
 
-	ret = pthread_create(&reconn_tid, NULL,
+	ret = rte_ctrl_thread_create(&reconn_tid, NULL,
 			     vhost_user_client_reconnect, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, VHOST_CONFIG, "failed to create reconnect thread");
@@ -829,8 +829,8 @@ rte_vhost_driver_start(const char *path)
 		return -1;
 
 	if (fdset_tid == 0) {
-		int ret = pthread_create(&fdset_tid, NULL, fdset_event_dispatch,
-				     &vhost_user.fdset);
+		int ret = rte_ctrl_thread_create(&fdset_tid, NULL,
+			fdset_event_dispatch, &vhost_user.fdset);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"failed to create fdset handling thread");
-- 
2.11.0

[dpdk-dev] [PATCH 3/4] eal: set name when creating a control thread

[Olivier Matz]

To avoid code duplication, add a parameter to rte_ctrl_thread_create()
to specify the name of the thread.

This requires to add a wrapper for the thread start routine in
rte_thread_init(), which will first wait that the thread is configured.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/kni/rte_eth_kni.c                  |  3 +-
 lib/librte_eal/common/eal_common_thread.c      | 52 ++++++++++++++++++++++++--
 lib/librte_eal/common/include/rte_lcore.h      |  7 +++-
 lib/librte_eal/linuxapp/eal/eal_interrupts.c   | 13 ++-----
 lib/librte_eal/linuxapp/eal/eal_timer.c        | 12 +-----
 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 10 +----
 lib/librte_pdump/rte_pdump.c                   | 10 +----
 lib/librte_vhost/socket.c                      |  7 ++--
 8 files changed, 68 insertions(+), 46 deletions(-)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 26718eb3e..6b036d8e1 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -149,7 +149,8 @@ eth_kni_dev_start(struct rte_eth_dev *dev)
 	}
 
 	if (internals->no_request_thread == 0) {
-		ret = rte_ctrl_thread_create(&internals->thread, NULL,
+		ret = rte_ctrl_thread_create(&internals->thread,
+			"kni_handle_request", NULL,
 			kni_handle_request, internals);
 		if (ret) {
 			RTE_LOG(ERR, PMD,
diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index efbccddbc..575b03e9d 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -7,6 +7,7 @@
 #include <stdint.h>
 #include <unistd.h>
 #include <pthread.h>
+#include <signal.h>
 #include <sched.h>
 #include <assert.h>
 #include <string.h>
@@ -141,10 +142,53 @@ eal_thread_dump_affinity(char *str, unsigned size)
 	return ret;
 }
 
+
+struct rte_thread_ctrl_params {
+	void *(*start_routine)(void *);
+	void *arg;
+	pthread_barrier_t configured;
+};
+
+static void *rte_thread_init(void *arg)
+{
+	struct rte_thread_ctrl_params *params = arg;
+	void *(*start_routine)(void *) = params->start_routine;
+	void *routine_arg = params->arg;
+
+	pthread_barrier_wait(&params->configured);
+
+	return start_routine(routine_arg);
+}
+
 __rte_experimental int
-rte_ctrl_thread_create(pthread_t *thread,
-			const pthread_attr_t *attr,
-			void *(*start_routine)(void *), void *arg)
+rte_ctrl_thread_create(pthread_t *thread, const char *name,
+		const pthread_attr_t *attr,
+		void *(*start_routine)(void *), void *arg)
 {
-	return pthread_create(thread, attr, start_routine, arg);
+	struct rte_thread_ctrl_params params = {
+		.start_routine = start_routine,
+		.arg = arg,
+	};
+	int ret;
+
+	pthread_barrier_init(&params.configured, NULL, 2);
+
+	ret = pthread_create(thread, attr, rte_thread_init, (void *)&params);
+	if (ret != 0)
+		return ret;
+
+	if (name != NULL) {
+		ret = rte_thread_setname(*thread, name);
+		if (ret < 0)
+			goto fail;
+	}
+
+	pthread_barrier_wait(&params.configured);
+
+	return 0;
+
+fail:
+	pthread_kill(*thread, SIGTERM);
+	pthread_join(*thread, NULL);
+	return ret;
 }
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index f19075a88..f3d9bbb91 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -249,10 +249,12 @@ int rte_thread_setname(pthread_t id, const char *name);
 /**
  * Create a control thread.
  *
- * Wrapper to pthread_create().
+ * Wrapper to pthread_create() and pthread_setname_np().
  *
  * @param thread
  *   Filled with the thread id of the new created thread.
+ * @param name
+ *   The name of the control thread (max 16 characters including '\0').
  * @param attr
  *   Attributes for the new thread.
  * @param start_routine
@@ -264,7 +266,8 @@ int rte_thread_setname(pthread_t id, const char *name);
  *   corresponding to the error number.
  */
 __rte_experimental int
-rte_ctrl_thread_create(pthread_t *thread, const pthread_attr_t *attr,
+rte_ctrl_thread_create(pthread_t *thread, const char *name,
+		const pthread_attr_t *attr,
 		void *(*start_routine)(void *), void *arg);
 
 /**
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index d927fb45d..3f184bed3 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -844,7 +844,7 @@ eal_intr_thread_main(__rte_unused void *arg)
 int
 rte_eal_intr_init(void)
 {
-	int ret = 0, ret_1 = 0;
+	int ret = 0;
 	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	/* init the global interrupt source head */
@@ -860,20 +860,13 @@ rte_eal_intr_init(void)
 	}
 
 	/* create the host thread to wait/handle the interrupt */
-	ret = rte_ctrl_thread_create(&intr_thread, NULL,
+	snprintf(thread_name, sizeof(thread_name), "eal-intr-thread");
+	ret = rte_ctrl_thread_create(&intr_thread, thread_name, NULL,
 			eal_intr_thread_main, NULL);
 	if (ret != 0) {
 		rte_errno = -ret;
 		RTE_LOG(ERR, EAL,
 			"Failed to create thread for interrupt handling\n");
-	} else {
-		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
-			"eal-intr-thread");
-		ret_1 = rte_thread_setname(intr_thread, thread_name);
-		if (ret_1 != 0)
-			RTE_LOG(DEBUG, EAL,
-			"Failed to set thread name for interrupt handling\n");
 	}
 
 	return ret;
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index f12d2e134..17b178e80 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -178,7 +178,8 @@ rte_eal_hpet_init(int make_default)
 
 	/* create a thread that will increment a global variable for
 	 * msb (hpet is 32 bits by default under linux) */
-	ret = rte_ctrl_thread_create(&msb_inc_thread_id, NULL,
+	snprintf(thread_name, sizeof(thread_name), "hpet-msb-inc");
+	ret = rte_ctrl_thread_create(&msb_inc_thread_id, thread_name, NULL,
 			(void *(*)(void *))hpet_msb_inc, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, EAL, "ERROR: Cannot create HPET timer thread!\n");
@@ -186,15 +187,6 @@ rte_eal_hpet_init(int make_default)
 		return -1;
 	}
 
-	/*
-	 * Set thread_name for aid in debugging.
-	 */
-	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "hpet-msb-inc");
-	ret = rte_thread_setname(msb_inc_thread_id, thread_name);
-	if (ret != 0)
-		RTE_LOG(DEBUG, EAL,
-			"Cannot set HPET timer thread name!\n");
-
 	if (make_default)
 		eal_timer_source = EAL_TIMER_HPET;
 	return 0;
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index 072c0a978..0cc726c9b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -374,7 +374,8 @@ vfio_mp_sync_setup(void)
 		return -1;
 	}
 
-	ret = rte_ctrl_thread_create(&socket_thread, NULL,
+	snprintf(thread_name, sizeof(thread_name), "vfio-sync");
+	ret = rte_ctrl_thread_create(&socket_thread, thread_name, NULL,
 			vfio_mp_sync_thread, NULL);
 	if (ret) {
 		RTE_LOG(ERR, EAL,
@@ -383,13 +384,6 @@ vfio_mp_sync_setup(void)
 		return -1;
 	}
 
-	/* Set thread_name for aid in debugging. */
-	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "vfio-sync");
-	ret = rte_thread_setname(socket_thread, thread_name);
-	if (ret)
-		RTE_LOG(DEBUG, EAL,
-			"Failed to set thread name for secondary processes!\n");
-
 	return 0;
 }
 
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 917a99957..824033f9a 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -548,7 +548,8 @@ rte_pdump_init(const char *path)
 	}
 
 	/* create the host thread to wait/handle pdump requests */
-	ret = rte_ctrl_thread_create(&pdump_thread, NULL,
+	snprintf(thread_name, sizeof(thread_name), "pdump-thread");
+	ret = rte_ctrl_thread_create(&pdump_thread, thread_name, NULL,
 				pdump_thread_main, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, PDUMP,
@@ -556,13 +557,6 @@ rte_pdump_init(const char *path)
 			strerror(-ret), __func__, __LINE__);
 		return -1;
 	}
-	/* Set thread_name for aid in debugging. */
-	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "pdump-thread");
-	ret = rte_thread_setname(pdump_thread, thread_name);
-	if (ret != 0) {
-		RTE_LOG(DEBUG, PDUMP,
-			"Failed to set thread name for pdump handling\n");
-	}
 
 	return 0;
 }
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index d261bf4b0..6fe5859d1 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -445,7 +445,7 @@ vhost_user_reconnect_init(void)
 	}
 	TAILQ_INIT(&reconn_list.head);
 
-	ret = rte_ctrl_thread_create(&reconn_tid, NULL,
+	ret = rte_ctrl_thread_create(&reconn_tid, "vhost_reconnect", NULL,
 			     vhost_user_client_reconnect, NULL);
 	if (ret != 0) {
 		RTE_LOG(ERR, VHOST_CONFIG, "failed to create reconnect thread");
@@ -829,8 +829,9 @@ rte_vhost_driver_start(const char *path)
 		return -1;
 
 	if (fdset_tid == 0) {
-		int ret = rte_ctrl_thread_create(&fdset_tid, NULL,
-			fdset_event_dispatch, &vhost_user.fdset);
+		int ret = rte_ctrl_thread_create(&fdset_tid,
+			"vhost_dispatch", NULL, fdset_event_dispatch,
+			&vhost_user.fdset);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"failed to create fdset handling thread");
-- 
2.11.0

[dpdk-dev] [PATCH 4/4] eal: set affinity for control threads

[Olivier Matz]

The management threads must not bother the dataplane or service cores.
Set the affinity of these threads accordingly.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/common/eal_common_thread.c | 20 +++++++++++++++++++-
 lib/librte_eal/common/include/rte_lcore.h |  4 +++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index 575b03e9d..f2e588c97 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -16,6 +16,7 @@
 #include <rte_memory.h>
 #include <rte_log.h>
 
+#include "eal_private.h"
 #include "eal_thread.h"
 
 RTE_DECLARE_PER_LCORE(unsigned , _socket_id);
@@ -169,7 +170,9 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
 		.start_routine = start_routine,
 		.arg = arg,
 	};
-	int ret;
+	unsigned int lcore_id;
+	rte_cpuset_t cpuset;
+	int set_affinity, ret;
 
 	pthread_barrier_init(&params.configured, NULL, 2);
 
@@ -183,6 +186,21 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
 			goto fail;
 	}
 
+	set_affinity = 0;
+	CPU_ZERO(&cpuset);
+	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+		if (eal_cpu_detected(lcore_id) &&
+				rte_lcore_has_role(lcore_id, ROLE_OFF)) {
+			CPU_SET(lcore_id, &cpuset);
+			set_affinity = 1;
+		}
+	}
+	if (set_affinity) {
+		ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
+		if (ret < 0)
+			goto fail;
+	}
+
 	pthread_barrier_wait(&params.configured);
 
 	return 0;
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index f3d9bbb91..354717c5d 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -249,7 +249,9 @@ int rte_thread_setname(pthread_t id, const char *name);
 /**
  * Create a control thread.
  *
- * Wrapper to pthread_create() and pthread_setname_np().
+ * Wrapper to pthread_create(), pthread_setname_np() and
+ * pthread_setaffinity_np(). The dataplane and service lcores are
+ * excluded from the affinity of the new thread.
  *
  * @param thread
  *   Filled with the thread id of the new created thread.
-- 
2.11.0

Re: [dpdk-dev] [PATCH 0/4] fix control thread affinities

[Olivier Matz]

Hi,

On Tue, Feb 27, 2018 at 03:46:26PM +0100, Olivier Matz wrote:
> Some parts of dpdk use their own management threads. Most of the time,
> the affinity of the thread is not properly set: it should not be scheduled
> on the dataplane cores, because interrupting them can cause packet losses.
> 
> This patchset introduces a new wrapper for thread creation that does
> the job automatically, avoiding code duplication.
> 
> Olivier Matz (4):
>   eal: use sizeof to avoid a double use of a define
>   eal: new function to create control threads
>   eal: set name when creating a control thread
>   eal: set affinity for control threads
> 
>  drivers/net/kni/Makefile                       |  1 +
>  drivers/net/kni/rte_eth_kni.c                  |  3 +-
>  lib/librte_eal/bsdapp/eal/eal.c                |  2 +-
>  lib/librte_eal/bsdapp/eal/eal_thread.c         |  2 +-
>  lib/librte_eal/common/eal_common_thread.c      | 70 ++++++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_lcore.h      | 26 ++++++++++
>  lib/librte_eal/linuxapp/eal/eal.c              |  4 +-
>  lib/librte_eal/linuxapp/eal/eal_interrupts.c   | 17 ++-----
>  lib/librte_eal/linuxapp/eal/eal_thread.c       |  2 +-
>  lib/librte_eal/linuxapp/eal/eal_timer.c        | 12 +----
>  lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 10 +---
>  lib/librte_eal/rte_eal_version.map             |  1 +
>  lib/librte_pdump/Makefile                      |  1 +
>  lib/librte_pdump/rte_pdump.c                   | 13 ++---
>  lib/librte_vhost/socket.c                      |  7 +--
>  15 files changed, 123 insertions(+), 48 deletions(-)

Any comment about this patchset?


Thanks,
Olivier

Re: [dpdk-dev] [PATCH 4/4] eal: set affinity for control threads

[Burakov, Anatoly]

On 27-Feb-18 2:46 PM, Olivier Matz wrote:
> The management threads must not bother the dataplane or service cores.
> Set the affinity of these threads accordingly.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---

<...>

>   
> +	set_affinity = 0;
> +	CPU_ZERO(&cpuset);
> +	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> +		if (eal_cpu_detected(lcore_id) &&
> +				rte_lcore_has_role(lcore_id, ROLE_OFF)) {
> +			CPU_SET(lcore_id, &cpuset);
> +			set_affinity = 1;
> +		}
> +	}
> +	if (set_affinity) {
> +		ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
> +		if (ret < 0)
> +			goto fail;
> +	}

Hi Olivier,

Please correct me if i'm wrong here, but if all detected cores are busy 
doing something (such as would be the case if DPDK is run without a 
coremask specified), affinity is not set? Maybe set it to master lcore 
instead (perhaps unconditionally - we usually recommend not using master 
lcore for anything important - perhaps setting it to master core always 
would be better, instead of trying to find unused lcores)?

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 4/4] eal: set affinity for control threads

[Olivier Matz]

On Thu, Mar 29, 2018 at 09:04:52AM +0100, Burakov, Anatoly wrote:
> On 27-Feb-18 2:46 PM, Olivier Matz wrote:
> > The management threads must not bother the dataplane or service cores.
> > Set the affinity of these threads accordingly.
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> 
> <...>
> 
> > +	set_affinity = 0;
> > +	CPU_ZERO(&cpuset);
> > +	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> > +		if (eal_cpu_detected(lcore_id) &&
> > +				rte_lcore_has_role(lcore_id, ROLE_OFF)) {
> > +			CPU_SET(lcore_id, &cpuset);
> > +			set_affinity = 1;
> > +		}
> > +	}
> > +	if (set_affinity) {
> > +		ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
> > +		if (ret < 0)
> > +			goto fail;
> > +	}
> 
> Hi Olivier,
> 
> Please correct me if i'm wrong here, but if all detected cores are busy
> doing something (such as would be the case if DPDK is run without a coremask
> specified), affinity is not set? Maybe set it to master lcore instead
> (perhaps unconditionally - we usually recommend not using master lcore for
> anything important - perhaps setting it to master core always would be
> better, instead of trying to find unused lcores)?

Good point and good idea, I will update the patchset.

Thanks,
Olivier