[dpdk-dev,v2] eal: restrict cores detection

  This patch uses pthread_getaffinity_np() to narrow down detected
cores before parsing coremask (-c), corelist (-l), and coremap
(--lcores).

The purpose of this patch is to leave out these core related options
when DPDK applications are deployed under container env, so that
users only specify core restriction as starting the instance.

Note: previously, some users are using isolated CPUs, which could
be excluded by default. Please add commands like taskset to use
those cores.

Test example:
$ taskset 0xc0000 ./examples/helloworld/build/helloworld -m 1024

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
v2:
  - Make it as default instead of adding the new options.
 lib/librte_eal/common/eal_common_lcore.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
  

On Wed, 31 Aug 2016 03:07:10 +0000
Jianfeng Tan <jianfeng.tan@intel.com> wrote:

> This patch uses pthread_getaffinity_np() to narrow down detected
> cores before parsing coremask (-c), corelist (-l), and coremap
> (--lcores).
> 
> The purpose of this patch is to leave out these core related options
> when DPDK applications are deployed under container env, so that
> users only specify core restriction as starting the instance.
> 
> Note: previously, some users are using isolated CPUs, which could
> be excluded by default. Please add commands like taskset to use
> those cores.
> 
> Test example:
> $ taskset 0xc0000 ./examples/helloworld/build/helloworld -m 1024
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> ---
> v2:
>   - Make it as default instead of adding the new options.
>  lib/librte_eal/common/eal_common_lcore.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
> index 2cd4132..62e4f67 100644
> --- a/lib/librte_eal/common/eal_common_lcore.c
> +++ b/lib/librte_eal/common/eal_common_lcore.c
> @@ -57,6 +57,14 @@ rte_eal_cpu_init(void)
>  	struct rte_config *config = rte_eal_get_configuration();
>  	unsigned lcore_id;
>  	unsigned count = 0;
> +	rte_cpuset_t cs;
> +	pthread_t tid = pthread_self();
> +
> +	/* Add below method to obtain core restrictions, like ulimit,
> +	 * cgroup.cpuset, etc. Will not use those cores, which are rebuffed.
> +	 */
> +	if (pthread_getaffinity_np(tid, sizeof(rte_cpuset_t), &cs) < 0)
> +		CPU_ZERO(&cs);
>  

This patch makes sense but the comment is hard to read because of wording
and grammar.

If you choose variable names better then there really is no need for
a comment in many cases. Code is often easier to read/write than comments
for non-native English speakers.

Remove the comment and rename 'cs' as 'affinity_set' or something equally
as descriptive.
  

Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, August 31, 2016 11:31 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; david.marchand@6wind.com; pmatilai@redhat.com;
> thomas.monjalon@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v2] eal: restrict cores detection
> 
> On Wed, 31 Aug 2016 03:07:10 +0000
> Jianfeng Tan <jianfeng.tan@intel.com> wrote:
> 
> > This patch uses pthread_getaffinity_np() to narrow down detected
> > cores before parsing coremask (-c), corelist (-l), and coremap
> > (--lcores).
> >
> > The purpose of this patch is to leave out these core related options
> > when DPDK applications are deployed under container env, so that
> > users only specify core restriction as starting the instance.
> >
> > Note: previously, some users are using isolated CPUs, which could
> > be excluded by default. Please add commands like taskset to use
> > those cores.
> >
> > Test example:
> > $ taskset 0xc0000 ./examples/helloworld/build/helloworld -m 1024
> >
> > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > ---
> > v2:
> >   - Make it as default instead of adding the new options.
> >  lib/librte_eal/common/eal_common_lcore.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_lcore.c
> b/lib/librte_eal/common/eal_common_lcore.c
> > index 2cd4132..62e4f67 100644
> > --- a/lib/librte_eal/common/eal_common_lcore.c
> > +++ b/lib/librte_eal/common/eal_common_lcore.c
> > @@ -57,6 +57,14 @@ rte_eal_cpu_init(void)
> >  	struct rte_config *config = rte_eal_get_configuration();
> >  	unsigned lcore_id;
> >  	unsigned count = 0;
> > +	rte_cpuset_t cs;
> > +	pthread_t tid = pthread_self();
> > +
> > +	/* Add below method to obtain core restrictions, like ulimit,
> > +	 * cgroup.cpuset, etc. Will not use those cores, which are rebuffed.
> > +	 */
> > +	if (pthread_getaffinity_np(tid, sizeof(rte_cpuset_t), &cs) < 0)
> > +		CPU_ZERO(&cs);
> >
> 
> This patch makes sense but the comment is hard to read because of wording
> and grammar.
> 
> If you choose variable names better then there really is no need for
> a comment in many cases. Code is often easier to read/write than comments
> for non-native English speakers.
> 
> Remove the comment and rename 'cs' as 'affinity_set' or something equally
> as descriptive.

Great suggestion. I'll resend one as you suggest.

Thanks,
Jianfeng