[dpdk-dev] [PATCH] eal: restrict ctrl threads to startup cpu affinity

[dpdk-dev] [PATCH] eal: restrict ctrl threads to startup cpu affinity

[David Marchand]

Spawning the ctrl threads on anything that is not part of the eal
coremask is not that polite to the rest of the system.

Rather than introduce yet another eal options for this, let's take
the startup cpu affinity as a reference and remove the eal coremask
from it.
If no cpu is left, then we default to the master core.

The cpuset is computed once at init before the original cpu affinity.

Fixes: d651ee4919cd ("eal: set affinity for control threads")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/eal_common_options.c | 28 ++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_common_thread.c  | 21 ++++-----------------
 lib/librte_eal/common/eal_internal_cfg.h   |  3 +++
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 6c96f45..b766252 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -217,6 +217,7 @@ struct device_option {
 	internal_cfg->create_uio_dev = 0;
 	internal_cfg->iova_mode = RTE_IOVA_DC;
 	internal_cfg->user_mbuf_pool_ops_name = NULL;
+	CPU_ZERO(&internal_cfg->ctrl_cpuset);
 	internal_cfg->init_complete = 0;
 }
 
@@ -1360,6 +1361,31 @@ static int xdigit2val(unsigned char c)
 	cfg->lcore_count -= removed;
 }
 
+static void
+compute_ctrl_threads_cpuset(struct internal_config *internal_cfg)
+{
+	rte_cpuset_t *cpuset = &internal_cfg->ctrl_cpuset;
+	rte_cpuset_t default_set;
+	unsigned int lcore_id;
+
+	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);
+		}
+	}
+
+	if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
+				&default_set) < 0)
+		CPU_ZERO(&default_set);
+
+	CPU_AND(cpuset, cpuset, &default_set);
+
+	/* if no detected cpu is off, use master core */
+	if (!CPU_COUNT(cpuset))
+		CPU_SET(rte_get_master_lcore(), cpuset);
+}
+
 int
 eal_cleanup_config(struct internal_config *internal_cfg)
 {
@@ -1393,6 +1419,8 @@ static int xdigit2val(unsigned char c)
 		lcore_config[cfg->master_lcore].core_role = ROLE_RTE;
 	}
 
+	compute_ctrl_threads_cpuset(internal_cfg);
+
 	/* if no memory amounts were requested, this will result in 0 and
 	 * will be overridden later, right after eal_hugepage_info_init() */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index 48ef4d6..893b0c1 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_internal_cfg.h"
 #include "eal_private.h"
 #include "eal_thread.h"
 
@@ -168,10 +169,9 @@ static void *rte_thread_init(void *arg)
 		const pthread_attr_t *attr,
 		void *(*start_routine)(void *), void *arg)
 {
+	rte_cpuset_t *cpuset = &internal_config.ctrl_cpuset;
 	struct rte_thread_ctrl_params *params;
-	unsigned int lcore_id;
-	rte_cpuset_t cpuset;
-	int cpu_found, ret;
+	int ret;
 
 	params = malloc(sizeof(*params));
 	if (!params)
@@ -195,20 +195,7 @@ static void *rte_thread_init(void *arg)
 				"Cannot set name for ctrl thread\n");
 	}
 
-	cpu_found = 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);
-			cpu_found = 1;
-		}
-	}
-	/* if no detected cpu is off, use master core */
-	if (!cpu_found)
-		CPU_SET(rte_get_master_lcore(), &cpuset);
-
-	ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
+	ret = pthread_setaffinity_np(*thread, sizeof(*cpuset), cpuset);
 	if (ret < 0)
 		goto fail;
 
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 60eaead..edff09d 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -13,6 +13,8 @@
 #include <rte_eal.h>
 #include <rte_pci_dev_feature_defs.h>
 
+#include "eal_thread.h"
+
 #define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
 
 /*
@@ -73,6 +75,7 @@ struct internal_config {
 	unsigned num_hugepage_sizes;      /**< how many sizes on this system */
 	struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
 	enum rte_iova_mode iova_mode ;    /**< Set IOVA mode on this system  */
+	rte_cpuset_t ctrl_cpuset;         /**< cpuset for ctrl threads */
 	volatile unsigned int init_complete;
 	/**< indicates whether EAL has completed initialization */
 };
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] eal: restrict ctrl threads to startup cpu affinity

[David Marchand]

On Wed, Feb 13, 2019 at 5:14 PM David Marchand <david.marchand@redhat.com>
wrote:

> Spawning the ctrl threads on anything that is not part of the eal
> coremask is not that polite to the rest of the system.
>
> Rather than introduce yet another eal options for this, let's take
> the startup cpu affinity as a reference and remove the eal coremask
> from it.
> If no cpu is left, then we default to the master core.
>
> The cpuset is computed once at init before the original cpu affinity.
>

Need to fix this last sentence...


> Fixes: d651ee4919cd ("eal: set affinity for control threads")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/librte_eal/common/eal_common_options.c | 28
> ++++++++++++++++++++++++++++
>  lib/librte_eal/common/eal_common_thread.c  | 21 ++++-----------------
>  lib/librte_eal/common/eal_internal_cfg.h   |  3 +++
>  3 files changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index 6c96f45..b766252 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -1360,6 +1361,31 @@ static int xdigit2val(unsigned char c)
>         cfg->lcore_count -= removed;
>  }
>
> +static void
> +compute_ctrl_threads_cpuset(struct internal_config *internal_cfg)
> +{
> +       rte_cpuset_t *cpuset = &internal_cfg->ctrl_cpuset;
> +       rte_cpuset_t default_set;
> +       unsigned int lcore_id;
> +
> +       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);
> +               }
> +       }
> +
> +       if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> +                               &default_set) < 0)
> +               CPU_ZERO(&default_set);
> +
> +       CPU_AND(cpuset, cpuset, &default_set);
>

CPU_AND is different on Freebsd.

     *CPU*_*AND*(*cpuset*_*t* **dst*, *cpuset*_*t* **src*);

     The *CPU*_*AND*() macro removes CPUs absent from *src* from
*dst*.	 (It is	the
     *cpuset(9)*
<https://www.freebsd.org/cgi/man.cgi?query=cpuset&sektion=9&apropos=0&manpath=FreeBSD+11.0-RELEASE+and+Ports>
equivalent of the scalar: *dst* &=	*src*.)  *CPU*_*AND*_*ATOMIC*()	is
     similar, with the same atomic semantics as	*CPU*_*OR*_*ATOMIC*().

Will fix in v2.

-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: restrict ctrl threads to startup cpu affinity

[Burakov, Anatoly]

On 13-Feb-19 4:13 PM, David Marchand wrote:
> Spawning the ctrl threads on anything that is not part of the eal
> coremask is not that polite to the rest of the system.
> 
> Rather than introduce yet another eal options for this, let's take
> the startup cpu affinity as a reference and remove the eal coremask
> from it.
> If no cpu is left, then we default to the master core.
> 
> The cpuset is computed once at init before the original cpu affinity.
> 
> Fixes: d651ee4919cd ("eal: set affinity for control threads")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Hi David,

Maybe i didn't have enough coffee today and i'm missing something here, 
but how is this different? Removing the coremask cores from the cpuset 
will effectively "spawn the ctrl threads on anything that is not part of 
the EAL coremask" (which is "not that polite to the rest of the 
system"), unless the application was run with taskset.

Is "taskset" the key point here? I.e. by default, we're still "not 
polite", unless the user asks nicely? :)

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] eal: restrict ctrl threads to startup cpu affinity

[David Marchand]

On Thu, Feb 14, 2019 at 10:39 AM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 13-Feb-19 4:13 PM, David Marchand wrote:
> > Spawning the ctrl threads on anything that is not part of the eal
> > coremask is not that polite to the rest of the system.
> >
> > Rather than introduce yet another eal options for this, let's take
> > the startup cpu affinity as a reference and remove the eal coremask
> > from it.
> > If no cpu is left, then we default to the master core.
> >
> > The cpuset is computed once at init before the original cpu affinity.
> >
> > Fixes: d651ee4919cd ("eal: set affinity for control threads")
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
>
> Hi David,
>
> Maybe i didn't have enough coffee today and i'm missing something here,
> but how is this different? Removing the coremask cores from the cpuset
> will effectively "spawn the ctrl threads on anything that is not part of
> the EAL coremask" (which is "not that polite to the rest of the
> system"), unless the application was run with taskset.
>
> Is "taskset" the key point here? I.e. by default, we're still "not
> polite", unless the user asks nicely? :)
>

Eheh, sorry, yes.
A bit more context then, if you want to clearly pin cpu resources for the
processes on your system (let's say having virtual machines and a popular
vswitch), I can only think of two solutions.
Either you have something to configure your processes to have them call
sched_setaffinity/pthread_set_affinity_np, or you use taskset to get them
"jailed" without them caring.

Before the incriminated commit, we were keeping all threads on the coremask
that had been passed, but as Olivier said, we would end up with ctrl
threads spanwed on core running dataplane threads as well.

Now, the ctrl threads can be spawned anywhere on all & ~coremask, with no
way to configure this.
I considered adding a new eal option, but I think relying on the current
cpu affinity is a better default behavior and I can't see drawbacks at the
moment.


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: restrict ctrl threads to startup cpu affinity

[Burakov, Anatoly]

On 14-Feb-19 9:53 AM, David Marchand wrote:
> On Thu, Feb 14, 2019 at 10:39 AM Burakov, Anatoly 
> <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
> 
>     On 13-Feb-19 4:13 PM, David Marchand wrote:
>      > Spawning the ctrl threads on anything that is not part of the eal
>      > coremask is not that polite to the rest of the system.
>      >
>      > Rather than introduce yet another eal options for this, let's take
>      > the startup cpu affinity as a reference and remove the eal coremask
>      > from it.
>      > If no cpu is left, then we default to the master core.
>      >
>      > The cpuset is computed once at init before the original cpu affinity.
>      >
>      > Fixes: d651ee4919cd ("eal: set affinity for control threads")
>      > Signed-off-by: David Marchand <david.marchand@redhat.com
>     <mailto:david.marchand@redhat.com>>
>      > ---
> 
>     Hi David,
> 
>     Maybe i didn't have enough coffee today and i'm missing something here,
>     but how is this different? Removing the coremask cores from the cpuset
>     will effectively "spawn the ctrl threads on anything that is not
>     part of
>     the EAL coremask" (which is "not that polite to the rest of the
>     system"), unless the application was run with taskset.
> 
>     Is "taskset" the key point here? I.e. by default, we're still "not
>     polite", unless the user asks nicely? :)
> 
> 
> Eheh, sorry, yes.
> A bit more context then, if you want to clearly pin cpu resources for 
> the processes on your system (let's say having virtual machines and a 
> popular vswitch), I can only think of two solutions.
> Either you have something to configure your processes to have them call 
> sched_setaffinity/pthread_set_affinity_np, or you use taskset to get 
> them "jailed" without them caring.
> 
> Before the incriminated commit, we were keeping all threads on the 
> coremask that had been passed, but as Olivier said, we would end up with 
> ctrl threads spanwed on core running dataplane threads as well.
> 
> Now, the ctrl threads can be spawned anywhere on all & ~coremask, with 
> no way to configure this.
> I considered adding a new eal option, but I think relying on the current 
> cpu affinity is a better default behavior and I can't see drawbacks at 
> the moment.
> 
> 
> -- 
> David Marchand
> 

OK, that makes sense. However, i feel this behavior (both old and new, 
for that matter) should be better documented somewhere in the EAL docs.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] eal: restrict ctrl threads to startup cpu affinity

[David Marchand]

On Thu, Feb 14, 2019 at 11:05 AM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 14-Feb-19 9:53 AM, David Marchand wrote:
>
> > A bit more context then, if you want to clearly pin cpu resources for
> > the processes on your system (let's say having virtual machines and a
> > popular vswitch), I can only think of two solutions.
> > Either you have something to configure your processes to have them call
> > sched_setaffinity/pthread_set_affinity_np, or you use taskset to get
> > them "jailed" without them caring.
> >
> > Before the incriminated commit, we were keeping all threads on the
> > coremask that had been passed, but as Olivier said, we would end up with
> > ctrl threads spanwed on core running dataplane threads as well.
> >
> > Now, the ctrl threads can be spawned anywhere on all & ~coremask, with
> > no way to configure this.
> > I considered adding a new eal option, but I think relying on the current
> > cpu affinity is a better default behavior and I can't see drawbacks at
> > the moment.
>
> OK, that makes sense. However, i feel this behavior (both old and new,
> for that matter) should be better documented somewhere in the EAL docs.
>
>
I'll see what I can add in the doc for v2.


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: restrict ctrl threads to startup cpu affinity

[David Marchand]

On Wed, Feb 13, 2019 at 5:14 PM David Marchand <david.marchand@redhat.com>
wrote:

> Spawning the ctrl threads on anything that is not part of the eal
> coremask is not that polite to the rest of the system.
>
> Rather than introduce yet another eal options for this, let's take
> the startup cpu affinity as a reference and remove the eal coremask
> from it.
> If no cpu is left, then we default to the master core.
>
> The cpuset is computed once at init before the original cpu affinity.
>
> Fixes: d651ee4919cd ("eal: set affinity for control threads")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/librte_eal/common/eal_common_options.c | 28
> ++++++++++++++++++++++++++++
>  lib/librte_eal/common/eal_common_thread.c  | 21 ++++-----------------
>  lib/librte_eal/common/eal_internal_cfg.h   |  3 +++
>  3 files changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index 6c96f45..b766252 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -1360,6 +1361,31 @@ static int xdigit2val(unsigned char c)
>         cfg->lcore_count -= removed;
>  }
>
> +static void
> +compute_ctrl_threads_cpuset(struct internal_config *internal_cfg)
> +{
> +       rte_cpuset_t *cpuset = &internal_cfg->ctrl_cpuset;
> +       rte_cpuset_t default_set;
> +       unsigned int lcore_id;
> +
> +       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);
> +               }
> +       }
> +
> +       if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> +                               &default_set) < 0)
> +               CPU_ZERO(&default_set);
>

Now that I am testing on Freebsd, I can see this block I took from the
existing "auto detect" function is just wrong.
pthread_(g|s)etaffinity_np return a > 0 error value when failing.


-- 
David Marchand

[dpdk-dev] [PATCH v2 1/2] eal: fix potential incorrect pinning for ctrl threads

[David Marchand]

pthread_setaffinity_np returns a >0 value on error.
We could end up letting the ctrl threads on the current process cpu
affinity.

Fixes: d651ee4919cd ("eal: set affinity for control threads")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/eal_common_thread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index 48ef4d6..a3985ce 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -209,7 +209,7 @@ static void *rte_thread_init(void *arg)
 		CPU_SET(rte_get_master_lcore(), &cpuset);
 
 	ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
-	if (ret < 0)
+	if (ret)
 		goto fail;
 
 	ret = pthread_barrier_wait(&params->configured);
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/2] eal: restrict ctrl threads to startup cpu affinity

[David Marchand]

Spawning the ctrl threads on anything that is not part of the eal
coremask is not that polite to the rest of the system, especially
when you took good care to pin your processes on cpu resources with
tools like taskset (linux) / cpuset (freebsd).

Rather than introduce yet another eal options to control on which cpu
those ctrl threads are created, let's take the startup cpu affinity
as a reference and remove the eal coremask from it.
If no cpu is left, then we default to the master core.

The cpuset is computed once at init before the original cpu affinity
is lost.

Introduced a RTE_CPU_AND macro to abstract the differences between linux
and freebsd respective macros.

Examples in a 4 cores FreeBSD vm:

$ ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
 -- -i --total-num-mbufs=2048

$ procstat -S 1057
  PID    TID COMM                TDNAME              CPU CSID CPU MASK
 1057 100131 testpmd             -                     2    1 2
 1057 100140 testpmd             eal-intr-thread       1    1 0-1
 1057 100141 testpmd             rte_mp_handle         1    1 0-1
 1057 100142 testpmd             lcore-slave-3         3    1 3

$ cpuset -l 1,2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
 -- -i --total-num-mbufs=2048

$ procstat -S 1061
  PID    TID COMM                TDNAME              CPU CSID CPU MASK
 1061 100131 testpmd             -                     2    2 2
 1061 100144 testpmd             eal-intr-thread       1    2 1
 1061 100145 testpmd             rte_mp_handle         1    2 1
 1061 100147 testpmd             lcore-slave-3         3    2 3

$ cpuset -l 2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
 -- -i --total-num-mbufs=2048

$ procstat -S 1065
  PID    TID COMM                TDNAME              CPU CSID CPU MASK
 1065 100131 testpmd             -                     2    2 2
 1065 100148 testpmd             eal-intr-thread       2    2 2
 1065 100149 testpmd             rte_mp_handle         2    2 2
 1065 100150 testpmd             lcore-slave-3         3    2 3

Fixes: d651ee4919cd ("eal: set affinity for control threads")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---

Changes since v1:
- added some description in the prog guide
- fixed FreeBSD build

---
 doc/guides/prog_guide/env_abstraction_layer.rst | 14 +++++++++++++
 lib/librte_eal/common/eal_common_options.c      | 28 +++++++++++++++++++++++++
 lib/librte_eal/common/eal_common_thread.c       | 21 ++++---------------
 lib/librte_eal/common/eal_internal_cfg.h        |  3 +++
 lib/librte_eal/common/include/rte_lcore.h       | 17 +++++++++++----
 5 files changed, 62 insertions(+), 21 deletions(-)

diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst
index 929d76d..bfc66d9 100644
--- a/doc/guides/prog_guide/env_abstraction_layer.rst
+++ b/doc/guides/prog_guide/env_abstraction_layer.rst
@@ -498,6 +498,20 @@ Those TLS include *_cpuset* and *_socket_id*:
 *	*_socket_id* stores the NUMA node of the CPU set. If the CPUs in CPU set belong to different NUMA node, the *_socket_id* will be set to SOCKET_ID_ANY.
 
 
+Control Thread API
+~~~~~~~~~~~~~~~~~~
+
+It is possible to create Control Threads using the public API ``rte_ctrl_thread_create()``.
+Those threads can be used for management/infrastructure tasks and are used internally by DPDK for multi process support and interrupt handling.
+
+Those threads will be scheduled on cpus part of the original process cpu affinity from which the dataplane and service lcores are excluded.
+
+For example, on a 8 cpus system, starting a dpdk application with -l 2,3 (dataplane cores), then depending on the affinity configuration which can be controlled with tools like taskset (Linux) or cpuset (FreeBSD),
+
+- with no affinity configuration, the Control Threads will end up on 0-1,4-7 cpus.
+- with affinity restricted to 2-4, the Control Threads will end up on cpu 4.
+- with affinity restricted to 2-3, the Control Threads will end up on cpu 2 (master lcore, which is the default when no cpu is available).
+
 .. _known_issue_label:
 
 Known Issues
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 1f45f82..fca3f83 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -217,6 +217,7 @@ struct device_option {
 	internal_cfg->create_uio_dev = 0;
 	internal_cfg->iova_mode = RTE_IOVA_DC;
 	internal_cfg->user_mbuf_pool_ops_name = NULL;
+	CPU_ZERO(&internal_cfg->ctrl_cpuset);
 	internal_cfg->init_complete = 0;
 }
 
@@ -1359,6 +1360,31 @@ static int xdigit2val(unsigned char c)
 	cfg->lcore_count -= removed;
 }
 
+static void
+compute_ctrl_threads_cpuset(struct internal_config *internal_cfg)
+{
+	rte_cpuset_t *cpuset = &internal_cfg->ctrl_cpuset;
+	rte_cpuset_t default_set;
+	unsigned int lcore_id;
+
+	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);
+		}
+	}
+
+	if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
+				&default_set) < 0)
+		CPU_ZERO(&default_set);
+
+	RTE_CPU_AND(cpuset, cpuset, &default_set);
+
+	/* if no detected cpu is off, use master core */
+	if (!CPU_COUNT(cpuset))
+		CPU_SET(rte_get_master_lcore(), cpuset);
+}
+
 int
 eal_cleanup_config(struct internal_config *internal_cfg)
 {
@@ -1392,6 +1418,8 @@ static int xdigit2val(unsigned char c)
 		lcore_config[cfg->master_lcore].core_role = ROLE_RTE;
 	}
 
+	compute_ctrl_threads_cpuset(internal_cfg);
+
 	/* if no memory amounts were requested, this will result in 0 and
 	 * will be overridden later, right after eal_hugepage_info_init() */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index a3985ce..14f206c 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_internal_cfg.h"
 #include "eal_private.h"
 #include "eal_thread.h"
 
@@ -168,10 +169,9 @@ static void *rte_thread_init(void *arg)
 		const pthread_attr_t *attr,
 		void *(*start_routine)(void *), void *arg)
 {
+	rte_cpuset_t *cpuset = &internal_config.ctrl_cpuset;
 	struct rte_thread_ctrl_params *params;
-	unsigned int lcore_id;
-	rte_cpuset_t cpuset;
-	int cpu_found, ret;
+	int ret;
 
 	params = malloc(sizeof(*params));
 	if (!params)
@@ -195,20 +195,7 @@ static void *rte_thread_init(void *arg)
 				"Cannot set name for ctrl thread\n");
 	}
 
-	cpu_found = 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);
-			cpu_found = 1;
-		}
-	}
-	/* if no detected cpu is off, use master core */
-	if (!cpu_found)
-		CPU_SET(rte_get_master_lcore(), &cpuset);
-
-	ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
+	ret = pthread_setaffinity_np(*thread, sizeof(*cpuset), cpuset);
 	if (ret)
 		goto fail;
 
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 60eaead..edff09d 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -13,6 +13,8 @@
 #include <rte_eal.h>
 #include <rte_pci_dev_feature_defs.h>
 
+#include "eal_thread.h"
+
 #define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
 
 /*
@@ -73,6 +75,7 @@ struct internal_config {
 	unsigned num_hugepage_sizes;      /**< how many sizes on this system */
 	struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
 	enum rte_iova_mode iova_mode ;    /**< Set IOVA mode on this system  */
+	rte_cpuset_t ctrl_cpuset;         /**< cpuset for ctrl threads */
 	volatile unsigned int init_complete;
 	/**< indicates whether EAL has completed initialization */
 };
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 6e09d91..eb49132 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -23,10 +23,18 @@
 #define LCORE_ID_ANY     UINT32_MAX       /**< Any lcore. */
 
 #if defined(__linux__)
-	typedef	cpu_set_t rte_cpuset_t;
+typedef	cpu_set_t rte_cpuset_t;
+#define RTE_CPU_AND(dst, src1, src2) CPU_AND(dst, src1, src2)
 #elif defined(__FreeBSD__)
 #include <pthread_np.h>
-	typedef cpuset_t rte_cpuset_t;
+typedef cpuset_t rte_cpuset_t;
+#define RTE_CPU_AND(dst, src1, src2) do \
+{ \
+	cpuset_t tmp; \
+	CPU_COPY(src1, &tmp); \
+	CPU_AND(&tmp, src2); \
+	CPU_COPY(&tmp, dst); \
+} while (0)
 #endif
 
 /**
@@ -280,8 +288,9 @@ struct lcore_config {
  * Create a control thread.
  *
  * 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.
+ * pthread_setaffinity_np(). The affinity of the new thread is based
+ * on the cpu affinity retrieved at the time rte_eal_init() was called,
+ * the dataplane and service lcores are then excluded.
  *
  * @param thread
  *   Filled with the thread id of the new created thread.
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 1/2] eal: fix potential incorrect pinning for ctrl threads

[Burakov, Anatoly]

On 14-Feb-19 1:30 PM, David Marchand wrote:
> pthread_setaffinity_np returns a >0 value on error.
> We could end up letting the ctrl threads on the current process cpu
> affinity.
> 
> Fixes: d651ee4919cd ("eal: set affinity for control threads")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>   lib/librte_eal/common/eal_common_thread.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
> index 48ef4d6..a3985ce 100644
> --- a/lib/librte_eal/common/eal_common_thread.c
> +++ b/lib/librte_eal/common/eal_common_thread.c
> @@ -209,7 +209,7 @@ static void *rte_thread_init(void *arg)
>   		CPU_SET(rte_get_master_lcore(), &cpuset);
>   
>   	ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
> -	if (ret < 0)
> +	if (ret)
>   		goto fail;
>   
>   	ret = pthread_barrier_wait(&params->configured);
> 

CC: stable?

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v2 1/2] eal: fix potential incorrect pinning for ctrl threads

[David Marchand]

On Thu, Feb 14, 2019 at 5:12 PM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 14-Feb-19 1:30 PM, David Marchand wrote:
> > pthread_setaffinity_np returns a >0 value on error.
> > We could end up letting the ctrl threads on the current process cpu
> > affinity.
> >
> > Fixes: d651ee4919cd ("eal: set affinity for control threads")
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >   lib/librte_eal/common/eal_common_thread.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_thread.c
> b/lib/librte_eal/common/eal_common_thread.c
> > index 48ef4d6..a3985ce 100644
> > --- a/lib/librte_eal/common/eal_common_thread.c
> > +++ b/lib/librte_eal/common/eal_common_thread.c
> > @@ -209,7 +209,7 @@ static void *rte_thread_init(void *arg)
> >               CPU_SET(rte_get_master_lcore(), &cpuset);
> >
> >       ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
> > -     if (ret < 0)
> > +     if (ret)
> >               goto fail;
> >
> >       ret = pthread_barrier_wait(&params->configured);
> >
>
> CC: stable?
>

Yes, I again forgot to check with check-git-log.sh...


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 2/2] eal: restrict ctrl threads to startup cpu affinity

[Burakov, Anatoly]

On 14-Feb-19 1:30 PM, David Marchand wrote:
> Spawning the ctrl threads on anything that is not part of the eal
> coremask is not that polite to the rest of the system, especially
> when you took good care to pin your processes on cpu resources with
> tools like taskset (linux) / cpuset (freebsd).
> 
> Rather than introduce yet another eal options to control on which cpu
> those ctrl threads are created, let's take the startup cpu affinity
> as a reference and remove the eal coremask from it.
> If no cpu is left, then we default to the master core.
> 
> The cpuset is computed once at init before the original cpu affinity
> is lost.
> 
> Introduced a RTE_CPU_AND macro to abstract the differences between linux
> and freebsd respective macros.
> 
> Examples in a 4 cores FreeBSD vm:
> 
> $ ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1057
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1057 100131 testpmd             -                     2    1 2
>   1057 100140 testpmd             eal-intr-thread       1    1 0-1
>   1057 100141 testpmd             rte_mp_handle         1    1 0-1
>   1057 100142 testpmd             lcore-slave-3         3    1 3
> 
> $ cpuset -l 1,2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1061
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1061 100131 testpmd             -                     2    2 2
>   1061 100144 testpmd             eal-intr-thread       1    2 1
>   1061 100145 testpmd             rte_mp_handle         1    2 1
>   1061 100147 testpmd             lcore-slave-3         3    2 3
> 
> $ cpuset -l 2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1065
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1065 100131 testpmd             -                     2    2 2
>   1065 100148 testpmd             eal-intr-thread       2    2 2
>   1065 100149 testpmd             rte_mp_handle         2    2 2
>   1065 100150 testpmd             lcore-slave-3         3    2 3
> 
> Fixes: d651ee4919cd ("eal: set affinity for control threads")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> 
> Changes since v1:
> - added some description in the prog guide
> - fixed FreeBSD build
> 
> ---
>   doc/guides/prog_guide/env_abstraction_layer.rst | 14 +++++++++++++
>   lib/librte_eal/common/eal_common_options.c      | 28 +++++++++++++++++++++++++
>   lib/librte_eal/common/eal_common_thread.c       | 21 ++++---------------
>   lib/librte_eal/common/eal_internal_cfg.h        |  3 +++
>   lib/librte_eal/common/include/rte_lcore.h       | 17 +++++++++++----
>   5 files changed, 62 insertions(+), 21 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst
> index 929d76d..bfc66d9 100644
> --- a/doc/guides/prog_guide/env_abstraction_layer.rst
> +++ b/doc/guides/prog_guide/env_abstraction_layer.rst
> @@ -498,6 +498,20 @@ Those TLS include *_cpuset* and *_socket_id*:
>   *	*_socket_id* stores the NUMA node of the CPU set. If the CPUs in CPU set belong to different NUMA node, the *_socket_id* will be set to SOCKET_ID_ANY.
>   
>   
> +Control Thread API
> +~~~~~~~~~~~~~~~~~~
> +
> +It is possible to create Control Threads using the public API ``rte_ctrl_thread_create()``.
> +Those threads can be used for management/infrastructure tasks and are used internally by DPDK for multi process support and interrupt handling.
> +
> +Those threads will be scheduled on cpus part of the original process cpu affinity from which the dataplane and service lcores are excluded.
> +
> +For example, on a 8 cpus system, starting a dpdk application with -l 2,3 (dataplane cores), then depending on the affinity configuration which can be controlled with tools like taskset (Linux) or cpuset (FreeBSD),
> +
> +- with no affinity configuration, the Control Threads will end up on 0-1,4-7 cpus.
> +- with affinity restricted to 2-4, the Control Threads will end up on cpu 4.
> +- with affinity restricted to 2-3, the Control Threads will end up on cpu 2 (master lcore, which is the default when no cpu is available).

You're not winning anything by foregoing the 80 char limit on 
documentation (doxygen will still generate this correctly), but you're 
losing in readability when working in terminal. I would prefer if you 
didn't do those long lines :)

Thomas, do we want checkpatch to warn about this?

> +
>   .. _known_issue_label:
>   
>   Known Issues
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 1f45f82..fca3f83 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -217,6 +217,7 @@ struct device_option {
>   	internal_cfg->create_uio_dev = 0;
>   	internal_cfg->iova_mode = RTE_IOVA_DC;
>   	internal_cfg->user_mbuf_pool_ops_name = NULL;
> +	CPU_ZERO(&internal_cfg->ctrl_cpuset);
>   	internal_cfg->init_complete = 0;
>   }
>   
> @@ -1359,6 +1360,31 @@ static int xdigit2val(unsigned char c)
>   	cfg->lcore_count -= removed;
>   }
>   
> +static void
> +compute_ctrl_threads_cpuset(struct internal_config *internal_cfg)
> +{
> +	rte_cpuset_t *cpuset = &internal_cfg->ctrl_cpuset;
> +	rte_cpuset_t default_set;
> +	unsigned int lcore_id;
> +
> +	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);
> +		}
> +	}
> +
> +	if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> +				&default_set) < 0)

Shouldn't this be != 0? Manpage doesn't say the error values will be 
negative.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v2 2/2] eal: restrict ctrl threads to startup cpu affinity

[David Marchand]

On Tue, Feb 19, 2019 at 12:38 PM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 14-Feb-19 1:30 PM, David Marchand wrote:
> > --- a/doc/guides/prog_guide/env_abstraction_layer.rst
> > +++ b/doc/guides/prog_guide/env_abstraction_layer.rst
> > @@ -498,6 +498,20 @@ Those TLS include *_cpuset* and *_socket_id*:
> >   *   *_socket_id* stores the NUMA node of the CPU set. If the CPUs in
> CPU set belong to different NUMA node, the *_socket_id* will be set to
> SOCKET_ID_ANY.
> >
> >
> > +Control Thread API
> > +~~~~~~~~~~~~~~~~~~
> > +
> > +It is possible to create Control Threads using the public API
> ``rte_ctrl_thread_create()``.
> > +Those threads can be used for management/infrastructure tasks and are
> used internally by DPDK for multi process support and interrupt handling.
> > +
> > +Those threads will be scheduled on cpus part of the original process
> cpu affinity from which the dataplane and service lcores are excluded.
> > +
> > +For example, on a 8 cpus system, starting a dpdk application with -l
> 2,3 (dataplane cores), then depending on the affinity configuration which
> can be controlled with tools like taskset (Linux) or cpuset (FreeBSD),
> > +
> > +- with no affinity configuration, the Control Threads will end up on
> 0-1,4-7 cpus.
> > +- with affinity restricted to 2-4, the Control Threads will end up on
> cpu 4.
> > +- with affinity restricted to 2-3, the Control Threads will end up on
> cpu 2 (master lcore, which is the default when no cpu is available).
>
> You're not winning anything by foregoing the 80 char limit on
> documentation (doxygen will still generate this correctly), but you're
> losing in readability when working in terminal. I would prefer if you
> didn't do those long lines :)
>

I don't really care, I will just wait for Thomas opinion.


> Thomas, do we want checkpatch to warn about this?
>
> > +
> >   .. _known_issue_label:
> >
> >   Known Issues
> > diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> > index 1f45f82..fca3f83 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -217,6 +217,7 @@ struct device_option {
> >       internal_cfg->create_uio_dev = 0;
> >       internal_cfg->iova_mode = RTE_IOVA_DC;
> >       internal_cfg->user_mbuf_pool_ops_name = NULL;
> > +     CPU_ZERO(&internal_cfg->ctrl_cpuset);
> >       internal_cfg->init_complete = 0;
> >   }
> >
> > @@ -1359,6 +1360,31 @@ static int xdigit2val(unsigned char c)
> >       cfg->lcore_count -= removed;
> >   }
> >
> > +static void
> > +compute_ctrl_threads_cpuset(struct internal_config *internal_cfg)
> > +{
> > +     rte_cpuset_t *cpuset = &internal_cfg->ctrl_cpuset;
> > +     rte_cpuset_t default_set;
> > +     unsigned int lcore_id;
> > +
> > +     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);
> > +             }
> > +     }
> > +
> > +     if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> > +                             &default_set) < 0)
>
> Shouldn't this be != 0? Manpage doesn't say the error values will be
> negative.
>

Good catch...
/me hides


Thanks for the review.

-- 
David Marchand

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 2/2] eal: restrict ctrl threads to startup cpu affinity

[Thomas Monjalon]

19/02/2019 12:51, David Marchand:
> On Tue, Feb 19, 2019 at 12:38 PM Burakov, Anatoly <anatoly.burakov@intel.com>
> wrote:
> > On 14-Feb-19 1:30 PM, David Marchand wrote:
> > > --- a/doc/guides/prog_guide/env_abstraction_layer.rst
> > > +++ b/doc/guides/prog_guide/env_abstraction_layer.rst
> > > +Control Thread API
> > > +~~~~~~~~~~~~~~~~~~
> > > +
> > > +It is possible to create Control Threads using the public API
> > ``rte_ctrl_thread_create()``.
> > > +Those threads can be used for management/infrastructure tasks and are
> > used internally by DPDK for multi process support and interrupt handling.
> > > +
> > > +Those threads will be scheduled on cpus part of the original process
> > cpu affinity from which the dataplane and service lcores are excluded.
> > > +
> > > +For example, on a 8 cpus system, starting a dpdk application with -l
> > 2,3 (dataplane cores), then depending on the affinity configuration which
> > can be controlled with tools like taskset (Linux) or cpuset (FreeBSD),
> > > +
> > > +- with no affinity configuration, the Control Threads will end up on
> > 0-1,4-7 cpus.
> > > +- with affinity restricted to 2-4, the Control Threads will end up on
> > cpu 4.
> > > +- with affinity restricted to 2-3, the Control Threads will end up on
> > cpu 2 (master lcore, which is the default when no cpu is available).
> >
> > You're not winning anything by foregoing the 80 char limit on
> > documentation (doxygen will still generate this correctly), but you're
> > losing in readability when working in terminal. I would prefer if you
> > didn't do those long lines :)
> 
> I don't really care, I will just wait for Thomas opinion.
> 
> > Thomas, do we want checkpatch to warn about this?

http://doc.dpdk.org/guides/contributing/documentation.html#line-length

Lines should not exceed 80 chars.
There is a tradition of being very flexible in docs.
The best is to wrap at the end of a logical group of words,
like after commas or dots. So the doc updates patches are easier to read.

[dpdk-dev] [PATCH v3 1/2] eal: fix potential incorrect pinning for ctrl threads

[David Marchand]

pthread_setaffinity_np returns a >0 value on error.
We could end up letting the ctrl threads on the current process cpu
affinity.

Fixes: d651ee4919cd ("eal: set affinity for control threads")
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v2:
- added missing Cc: stable@dpdk.org

---
 lib/librte_eal/common/eal_common_thread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index 48ef4d6..a3985ce 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -209,7 +209,7 @@ static void *rte_thread_init(void *arg)
 		CPU_SET(rte_get_master_lcore(), &cpuset);
 
 	ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
-	if (ret < 0)
+	if (ret)
 		goto fail;
 
 	ret = pthread_barrier_wait(&params->configured);
-- 
1.8.3.1

[dpdk-dev] [PATCH v3 2/2] eal: restrict ctrl threads to startup cpu affinity

[David Marchand]

Spawning the ctrl threads on anything that is not part of the eal
coremask is not that polite to the rest of the system, especially
when you took good care to pin your processes on cpu resources with
tools like taskset (linux) / cpuset (freebsd).

Rather than introduce yet another eal options to control on which cpu
those ctrl threads are created, let's take the startup cpu affinity
as a reference and remove the eal coremask from it.
If no cpu is left, then we default to the master core.

The cpuset is computed once at init before the original cpu affinity
is lost.

Introduced a RTE_CPU_AND macro to abstract the differences between linux
and freebsd respective macros.

Examples in a 4 cores FreeBSD vm:

$ ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
 -- -i --total-num-mbufs=2048

$ procstat -S 1057
  PID    TID COMM                TDNAME              CPU CSID CPU MASK
 1057 100131 testpmd             -                     2    1 2
 1057 100140 testpmd             eal-intr-thread       1    1 0-1
 1057 100141 testpmd             rte_mp_handle         1    1 0-1
 1057 100142 testpmd             lcore-slave-3         3    1 3

$ cpuset -l 1,2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
 -- -i --total-num-mbufs=2048

$ procstat -S 1061
  PID    TID COMM                TDNAME              CPU CSID CPU MASK
 1061 100131 testpmd             -                     2    2 2
 1061 100144 testpmd             eal-intr-thread       1    2 1
 1061 100145 testpmd             rte_mp_handle         1    2 1
 1061 100147 testpmd             lcore-slave-3         3    2 3

$ cpuset -l 2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
 -- -i --total-num-mbufs=2048

$ procstat -S 1065
  PID    TID COMM                TDNAME              CPU CSID CPU MASK
 1065 100131 testpmd             -                     2    2 2
 1065 100148 testpmd             eal-intr-thread       2    2 2
 1065 100149 testpmd             rte_mp_handle         2    2 2
 1065 100150 testpmd             lcore-slave-3         3    2 3

Fixes: d651ee4919cd ("eal: set affinity for control threads")
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v2:
- added missing Cc: stable@dpdk.org
- re-indent doc to comply with 80 columns
- fix incorrect check on pthread_getaffinity_np

Changes since v1:
- added some description in the prog guide
- fixed FreeBSD build

---
 doc/guides/prog_guide/env_abstraction_layer.rst | 22 +++++++++++++++++++
 lib/librte_eal/common/eal_common_options.c      | 28 +++++++++++++++++++++++++
 lib/librte_eal/common/eal_common_thread.c       | 21 ++++---------------
 lib/librte_eal/common/eal_internal_cfg.h        |  3 +++
 lib/librte_eal/common/include/rte_lcore.h       | 17 +++++++++++----
 5 files changed, 70 insertions(+), 21 deletions(-)

diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst
index 929d76d..1fd6ece 100644
--- a/doc/guides/prog_guide/env_abstraction_layer.rst
+++ b/doc/guides/prog_guide/env_abstraction_layer.rst
@@ -498,6 +498,28 @@ Those TLS include *_cpuset* and *_socket_id*:
 *	*_socket_id* stores the NUMA node of the CPU set. If the CPUs in CPU set belong to different NUMA node, the *_socket_id* will be set to SOCKET_ID_ANY.
 
 
+Control Thread API
+~~~~~~~~~~~~~~~~~~
+
+It is possible to create Control Threads using the public API
+``rte_ctrl_thread_create()``.
+Those threads can be used for management/infrastructure tasks and are used
+internally by DPDK for multi process support and interrupt handling.
+
+Those threads will be scheduled on cpus part of the original process cpu
+affinity from which the dataplane and service lcores are excluded.
+
+For example, on a 8 cpus system, starting a dpdk application with -l 2,3
+(dataplane cores), then depending on the affinity configuration which can be
+controlled with tools like taskset (Linux) or cpuset (FreeBSD),
+
+- with no affinity configuration, the Control Threads will end up on
+  0-1,4-7 cpus.
+- with affinity restricted to 2-4, the Control Threads will end up on
+  cpu 4.
+- with affinity restricted to 2-3, the Control Threads will end up on
+  cpu 2 (master lcore, which is the default when no cpu is available).
+
 .. _known_issue_label:
 
 Known Issues
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 1f45f82..f8b7ab9 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -217,6 +217,7 @@ struct device_option {
 	internal_cfg->create_uio_dev = 0;
 	internal_cfg->iova_mode = RTE_IOVA_DC;
 	internal_cfg->user_mbuf_pool_ops_name = NULL;
+	CPU_ZERO(&internal_cfg->ctrl_cpuset);
 	internal_cfg->init_complete = 0;
 }
 
@@ -1359,6 +1360,31 @@ static int xdigit2val(unsigned char c)
 	cfg->lcore_count -= removed;
 }
 
+static void
+compute_ctrl_threads_cpuset(struct internal_config *internal_cfg)
+{
+	rte_cpuset_t *cpuset = &internal_cfg->ctrl_cpuset;
+	rte_cpuset_t default_set;
+	unsigned int lcore_id;
+
+	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);
+		}
+	}
+
+	if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
+				&default_set))
+		CPU_ZERO(&default_set);
+
+	RTE_CPU_AND(cpuset, cpuset, &default_set);
+
+	/* if no detected cpu is off, use master core */
+	if (!CPU_COUNT(cpuset))
+		CPU_SET(rte_get_master_lcore(), cpuset);
+}
+
 int
 eal_cleanup_config(struct internal_config *internal_cfg)
 {
@@ -1392,6 +1418,8 @@ static int xdigit2val(unsigned char c)
 		lcore_config[cfg->master_lcore].core_role = ROLE_RTE;
 	}
 
+	compute_ctrl_threads_cpuset(internal_cfg);
+
 	/* if no memory amounts were requested, this will result in 0 and
 	 * will be overridden later, right after eal_hugepage_info_init() */
 	for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index a3985ce..14f206c 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_internal_cfg.h"
 #include "eal_private.h"
 #include "eal_thread.h"
 
@@ -168,10 +169,9 @@ static void *rte_thread_init(void *arg)
 		const pthread_attr_t *attr,
 		void *(*start_routine)(void *), void *arg)
 {
+	rte_cpuset_t *cpuset = &internal_config.ctrl_cpuset;
 	struct rte_thread_ctrl_params *params;
-	unsigned int lcore_id;
-	rte_cpuset_t cpuset;
-	int cpu_found, ret;
+	int ret;
 
 	params = malloc(sizeof(*params));
 	if (!params)
@@ -195,20 +195,7 @@ static void *rte_thread_init(void *arg)
 				"Cannot set name for ctrl thread\n");
 	}
 
-	cpu_found = 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);
-			cpu_found = 1;
-		}
-	}
-	/* if no detected cpu is off, use master core */
-	if (!cpu_found)
-		CPU_SET(rte_get_master_lcore(), &cpuset);
-
-	ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
+	ret = pthread_setaffinity_np(*thread, sizeof(*cpuset), cpuset);
 	if (ret)
 		goto fail;
 
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 60eaead..edff09d 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -13,6 +13,8 @@
 #include <rte_eal.h>
 #include <rte_pci_dev_feature_defs.h>
 
+#include "eal_thread.h"
+
 #define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
 
 /*
@@ -73,6 +75,7 @@ struct internal_config {
 	unsigned num_hugepage_sizes;      /**< how many sizes on this system */
 	struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
 	enum rte_iova_mode iova_mode ;    /**< Set IOVA mode on this system  */
+	rte_cpuset_t ctrl_cpuset;         /**< cpuset for ctrl threads */
 	volatile unsigned int init_complete;
 	/**< indicates whether EAL has completed initialization */
 };
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 6e09d91..eb49132 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -23,10 +23,18 @@
 #define LCORE_ID_ANY     UINT32_MAX       /**< Any lcore. */
 
 #if defined(__linux__)
-	typedef	cpu_set_t rte_cpuset_t;
+typedef	cpu_set_t rte_cpuset_t;
+#define RTE_CPU_AND(dst, src1, src2) CPU_AND(dst, src1, src2)
 #elif defined(__FreeBSD__)
 #include <pthread_np.h>
-	typedef cpuset_t rte_cpuset_t;
+typedef cpuset_t rte_cpuset_t;
+#define RTE_CPU_AND(dst, src1, src2) do \
+{ \
+	cpuset_t tmp; \
+	CPU_COPY(src1, &tmp); \
+	CPU_AND(&tmp, src2); \
+	CPU_COPY(&tmp, dst); \
+} while (0)
 #endif
 
 /**
@@ -280,8 +288,9 @@ struct lcore_config {
  * Create a control thread.
  *
  * 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.
+ * pthread_setaffinity_np(). The affinity of the new thread is based
+ * on the cpu affinity retrieved at the time rte_eal_init() was called,
+ * the dataplane and service lcores are then excluded.
  *
  * @param thread
  *   Filled with the thread id of the new created thread.
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v3 1/2] eal: fix potential incorrect pinning for ctrl threads

[Burakov, Anatoly]

On 19-Feb-19 8:41 PM, David Marchand wrote:
> pthread_setaffinity_np returns a >0 value on error.
> We could end up letting the ctrl threads on the current process cpu
> affinity.
> 
> Fixes: d651ee4919cd ("eal: set affinity for control threads")
> Cc: stable@dpdk.org
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

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

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v3 2/2] eal: restrict ctrl threads to startup cpu affinity

[Burakov, Anatoly]

On 19-Feb-19 8:41 PM, David Marchand wrote:
> Spawning the ctrl threads on anything that is not part of the eal
> coremask is not that polite to the rest of the system, especially
> when you took good care to pin your processes on cpu resources with
> tools like taskset (linux) / cpuset (freebsd).
> 
> Rather than introduce yet another eal options to control on which cpu
> those ctrl threads are created, let's take the startup cpu affinity
> as a reference and remove the eal coremask from it.
> If no cpu is left, then we default to the master core.
> 
> The cpuset is computed once at init before the original cpu affinity
> is lost.
> 
> Introduced a RTE_CPU_AND macro to abstract the differences between linux
> and freebsd respective macros.
> 
> Examples in a 4 cores FreeBSD vm:
> 
> $ ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1057
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1057 100131 testpmd             -                     2    1 2
>   1057 100140 testpmd             eal-intr-thread       1    1 0-1
>   1057 100141 testpmd             rte_mp_handle         1    1 0-1
>   1057 100142 testpmd             lcore-slave-3         3    1 3
> 
> $ cpuset -l 1,2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1061
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1061 100131 testpmd             -                     2    2 2
>   1061 100144 testpmd             eal-intr-thread       1    2 1
>   1061 100145 testpmd             rte_mp_handle         1    2 1
>   1061 100147 testpmd             lcore-slave-3         3    2 3
> 
> $ cpuset -l 2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
>   -- -i --total-num-mbufs=2048
> 
> $ procstat -S 1065
>    PID    TID COMM                TDNAME              CPU CSID CPU MASK
>   1065 100131 testpmd             -                     2    2 2
>   1065 100148 testpmd             eal-intr-thread       2    2 2
>   1065 100149 testpmd             rte_mp_handle         2    2 2
>   1065 100150 testpmd             lcore-slave-3         3    2 3
> 
> Fixes: d651ee4919cd ("eal: set affinity for control threads")
> Cc: stable@dpdk.org
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

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

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v3 1/2] eal: fix potential incorrect pinning for ctrl threads

[Olivier Matz]

On Wed, Feb 20, 2019 at 04:01:12PM +0000, Burakov, Anatoly wrote:
> On 19-Feb-19 8:41 PM, David Marchand wrote:
> > pthread_setaffinity_np returns a >0 value on error.
> > We could end up letting the ctrl threads on the current process cpu
> > affinity.
> > 
> > Fixes: d651ee4919cd ("eal: set affinity for control threads")
> > Cc: stable@dpdk.org
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> 
> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>

Reviewed-by: Olivier Matz <olivier.matz@6wind.com>

Re: [dpdk-dev] [PATCH v3 2/2] eal: restrict ctrl threads to startup cpu affinity

[Olivier Matz]

On Wed, Feb 20, 2019 at 04:01:33PM +0000, Burakov, Anatoly wrote:
> On 19-Feb-19 8:41 PM, David Marchand wrote:
> > Spawning the ctrl threads on anything that is not part of the eal
> > coremask is not that polite to the rest of the system, especially
> > when you took good care to pin your processes on cpu resources with
> > tools like taskset (linux) / cpuset (freebsd).
> > 
> > Rather than introduce yet another eal options to control on which cpu
> > those ctrl threads are created, let's take the startup cpu affinity
> > as a reference and remove the eal coremask from it.
> > If no cpu is left, then we default to the master core.
> > 
> > The cpuset is computed once at init before the original cpu affinity
> > is lost.
> > 
> > Introduced a RTE_CPU_AND macro to abstract the differences between linux
> > and freebsd respective macros.
> > 
> > Examples in a 4 cores FreeBSD vm:
> > 
> > $ ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
> >   -- -i --total-num-mbufs=2048
> > 
> > $ procstat -S 1057
> >    PID    TID COMM                TDNAME              CPU CSID CPU MASK
> >   1057 100131 testpmd             -                     2    1 2
> >   1057 100140 testpmd             eal-intr-thread       1    1 0-1
> >   1057 100141 testpmd             rte_mp_handle         1    1 0-1
> >   1057 100142 testpmd             lcore-slave-3         3    1 3
> > 
> > $ cpuset -l 1,2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
> >   -- -i --total-num-mbufs=2048
> > 
> > $ procstat -S 1061
> >    PID    TID COMM                TDNAME              CPU CSID CPU MASK
> >   1061 100131 testpmd             -                     2    2 2
> >   1061 100144 testpmd             eal-intr-thread       1    2 1
> >   1061 100145 testpmd             rte_mp_handle         1    2 1
> >   1061 100147 testpmd             lcore-slave-3         3    2 3
> > 
> > $ cpuset -l 2,3 ./build/app/testpmd -l 2,3 --no-huge --no-pci -m 512 \
> >   -- -i --total-num-mbufs=2048
> > 
> > $ procstat -S 1065
> >    PID    TID COMM                TDNAME              CPU CSID CPU MASK
> >   1065 100131 testpmd             -                     2    2 2
> >   1065 100148 testpmd             eal-intr-thread       2    2 2
> >   1065 100149 testpmd             rte_mp_handle         2    2 2
> >   1065 100150 testpmd             lcore-slave-3         3    2 3
> > 
> > Fixes: d651ee4919cd ("eal: set affinity for control threads")
> > Cc: stable@dpdk.org
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> 
> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>

Reviewed-by: Olivier Matz <olivier.matz@6wind.com>

Re: [dpdk-dev] [PATCH v3 2/2] eal: restrict ctrl threads to startup cpu affinity

[Thomas Monjalon]

25/02/2019 09:33, Olivier Matz:
> On Wed, Feb 20, 2019 at 04:01:33PM +0000, Burakov, Anatoly wrote:
> > On 19-Feb-19 8:41 PM, David Marchand wrote:
> > > Spawning the ctrl threads on anything that is not part of the eal
> > > coremask is not that polite to the rest of the system, especially
> > > when you took good care to pin your processes on cpu resources with
> > > tools like taskset (linux) / cpuset (freebsd).
> > > 
> > > Rather than introduce yet another eal options to control on which cpu
> > > those ctrl threads are created, let's take the startup cpu affinity
> > > as a reference and remove the eal coremask from it.
> > > If no cpu is left, then we default to the master core.
> > > 
> > > The cpuset is computed once at init before the original cpu affinity
> > > is lost.
> > > 
[...]
> > > 
> > > Fixes: d651ee4919cd ("eal: set affinity for control threads")
> > > Cc: stable@dpdk.org
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > 
> > Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Reviewed-by: Olivier Matz <olivier.matz@6wind.com>

Replaced cpu by uppercase CPU,
and applied, thanks