[dpdk-dev] eal/linuxapp: Add parameter to specify master lcore id

  This commit enables users to specify the lcore id that
is used as master lcore.

Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu>
---
 lib/librte_eal/linuxapp/eal/eal.c |   33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
  

Here are some comments about the use case of this patch:

This patch is especially useful in cases where DPDK applications scale 
their CPU resources at runtime via starting and stopping slave lcores. 
Since the coremask defines the maximum scale-out for such a application, 
the master lcore becomes to the minimum scale-in.
Imagine, running multiple primary processed of such DPDK applications, 
users might want to overlap the coremasks for scaling. However, it would 
still make sense to run the master lcores on different CPU cores.

In DPDK vSwitch we might end up in such a scenario with a future release:
   https://lists.01.org/pipermail/dpdk-ovs/2014-March/000770.html
   https://lists.01.org/pipermail/dpdk-ovs/2014-March/000773.html

Thanks,

Simon

On 08.07.2014 10:28, Simon Kuenzer wrote:
> This commit enables users to specify the lcore id that
> is used as master lcore.
>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu>
> ---
>   lib/librte_eal/linuxapp/eal/eal.c |   33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 573fd06..4ad5b9b 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -101,6 +101,7 @@
>   #define OPT_XEN_DOM0    "xen-dom0"
>   #define OPT_CREATE_UIO_DEV "create-uio-dev"
>   #define OPT_VFIO_INTR    "vfio-intr"
> +#define OPT_MASTER_LCORE "master-lcore"
>
>   #define RTE_EAL_BLACKLIST_SIZE	0x100
>
> @@ -336,6 +337,7 @@ eal_usage(const char *prgname)
>   	       "[--proc-type primary|secondary|auto] \n\n"
>   	       "EAL options:\n"
>   	       "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
> +	       "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
>   	       "  -n NUM       : Number of memory channels\n"
>   		   "  -v           : Display version information on startup\n"
>   	       "  -d LIB.so    : add driver (can be used multiple times)\n"
> @@ -468,6 +470,21 @@ eal_parse_coremask(const char *coremask)
>   	return 0;
>   }
>
> +/* Changes the lcore id of the master thread */
> +static int
> +eal_parse_master_lcore(const char *arg)
> +{
> +	struct rte_config *cfg = rte_eal_get_configuration();
> +	int master_lcore = atoi(arg);
> +
> +	if (!(master_lcore >= 0 && master_lcore < RTE_MAX_LCORE))
> +		return -1;
> +	if (cfg->lcore_role[master_lcore] != ROLE_RTE)
> +		return -1;
> +	cfg->master_lcore = master_lcore;
> +	return 0;
> +}
> +
>   static int
>   eal_parse_syslog(const char *facility)
>   {
> @@ -653,6 +670,7 @@ eal_parse_args(int argc, char **argv)
>   		{OPT_HUGE_DIR, 1, 0, 0},
>   		{OPT_NO_SHCONF, 0, 0, 0},
>   		{OPT_PROC_TYPE, 1, 0, 0},
> +		{OPT_MASTER_LCORE, 1, 0, 0},
>   		{OPT_FILE_PREFIX, 1, 0, 0},
>   		{OPT_SOCKET_MEM, 1, 0, 0},
>   		{OPT_PCI_WHITELIST, 1, 0, 0},
> @@ -802,6 +820,21 @@ eal_parse_args(int argc, char **argv)
>   			else if (!strcmp(lgopts[option_index].name, OPT_PROC_TYPE)) {
>   				internal_config.process_type = eal_parse_proc_type(optarg);
>   			}
> +			else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) {
> +				if (!coremask_ok) {
> +					RTE_LOG(ERR, EAL, "please specify the master "
> +							"lcore id after specifying "
> +							"the coremask\n");
> +					eal_usage(prgname);
> +					return -1;
> +				}
> +				if (eal_parse_master_lcore(optarg) < 0) {
> +					RTE_LOG(ERR, EAL, "invalid parameter for --"
> +							OPT_MASTER_LCORE "\n");
> +					eal_usage(prgname);
> +					return -1;
> +				}
> +			}
>   			else if (!strcmp(lgopts[option_index].name, OPT_FILE_PREFIX)) {
>   				internal_config.hugefile_prefix = optarg;
>   			}
>
  

Comments?

On 08.07.2014 11:42, Simon Kuenzer wrote:
> Here are some comments about the use case of this patch:
>
> This patch is especially useful in cases where DPDK applications scale
> their CPU resources at runtime via starting and stopping slave lcores.
> Since the coremask defines the maximum scale-out for such a application,
> the master lcore becomes to the minimum scale-in.
> Imagine, running multiple primary processed of such DPDK applications,
> users might want to overlap the coremasks for scaling. However, it would
> still make sense to run the master lcores on different CPU cores.
>
> In DPDK vSwitch we might end up in such a scenario with a future release:
>    https://lists.01.org/pipermail/dpdk-ovs/2014-March/000770.html
>    https://lists.01.org/pipermail/dpdk-ovs/2014-March/000773.html
>
> Thanks,
>
> Simon
>
> On 08.07.2014 10:28, Simon Kuenzer wrote:
>> This commit enables users to specify the lcore id that
>> is used as master lcore.
>>
>> Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu>
>> ---
>>   lib/librte_eal/linuxapp/eal/eal.c |   33
>> +++++++++++++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
>> b/lib/librte_eal/linuxapp/eal/eal.c
>> index 573fd06..4ad5b9b 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -101,6 +101,7 @@
>>   #define OPT_XEN_DOM0    "xen-dom0"
>>   #define OPT_CREATE_UIO_DEV "create-uio-dev"
>>   #define OPT_VFIO_INTR    "vfio-intr"
>> +#define OPT_MASTER_LCORE "master-lcore"
>>
>>   #define RTE_EAL_BLACKLIST_SIZE    0x100
>>
>> @@ -336,6 +337,7 @@ eal_usage(const char *prgname)
>>              "[--proc-type primary|secondary|auto] \n\n"
>>              "EAL options:\n"
>>              "  -c COREMASK  : A hexadecimal bitmask of cores to run
>> on\n"
>> +           "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
>>              "  -n NUM       : Number of memory channels\n"
>>              "  -v           : Display version information on startup\n"
>>              "  -d LIB.so    : add driver (can be used multiple times)\n"
>> @@ -468,6 +470,21 @@ eal_parse_coremask(const char *coremask)
>>       return 0;
>>   }
>>
>> +/* Changes the lcore id of the master thread */
>> +static int
>> +eal_parse_master_lcore(const char *arg)
>> +{
>> +    struct rte_config *cfg = rte_eal_get_configuration();
>> +    int master_lcore = atoi(arg);
>> +
>> +    if (!(master_lcore >= 0 && master_lcore < RTE_MAX_LCORE))
>> +        return -1;
>> +    if (cfg->lcore_role[master_lcore] != ROLE_RTE)
>> +        return -1;
>> +    cfg->master_lcore = master_lcore;
>> +    return 0;
>> +}
>> +
>>   static int
>>   eal_parse_syslog(const char *facility)
>>   {
>> @@ -653,6 +670,7 @@ eal_parse_args(int argc, char **argv)
>>           {OPT_HUGE_DIR, 1, 0, 0},
>>           {OPT_NO_SHCONF, 0, 0, 0},
>>           {OPT_PROC_TYPE, 1, 0, 0},
>> +        {OPT_MASTER_LCORE, 1, 0, 0},
>>           {OPT_FILE_PREFIX, 1, 0, 0},
>>           {OPT_SOCKET_MEM, 1, 0, 0},
>>           {OPT_PCI_WHITELIST, 1, 0, 0},
>> @@ -802,6 +820,21 @@ eal_parse_args(int argc, char **argv)
>>               else if (!strcmp(lgopts[option_index].name,
>> OPT_PROC_TYPE)) {
>>                   internal_config.process_type =
>> eal_parse_proc_type(optarg);
>>               }
>> +            else if (!strcmp(lgopts[option_index].name,
>> OPT_MASTER_LCORE)) {
>> +                if (!coremask_ok) {
>> +                    RTE_LOG(ERR, EAL, "please specify the master "
>> +                            "lcore id after specifying "
>> +                            "the coremask\n");
>> +                    eal_usage(prgname);
>> +                    return -1;
>> +                }
>> +                if (eal_parse_master_lcore(optarg) < 0) {
>> +                    RTE_LOG(ERR, EAL, "invalid parameter for --"
>> +                            OPT_MASTER_LCORE "\n");
>> +                    eal_usage(prgname);
>> +                    return -1;
>> +                }
>> +            }
>>               else if (!strcmp(lgopts[option_index].name,
>> OPT_FILE_PREFIX)) {
>>                   internal_config.hugefile_prefix = optarg;
>>               }
>>
>
  

Hi all,

does anyone have interest in this functionality?

I think this is important and useful.
Since we should care about core assignment to get high performance
and the master lcore thread is special in DPDK, we will want to
assign the master to the target core.
For example, with hyperthreading I'd like to make a pair of packet
processing threads into one physical core and separate the master
thread which does some management.

thanks,
Hiroshi

> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id
> 
> Comments?
> 
> On 08.07.2014 11:42, Simon Kuenzer wrote:
> > Here are some comments about the use case of this patch:
> >
> > This patch is especially useful in cases where DPDK applications scale
> > their CPU resources at runtime via starting and stopping slave lcores.
> > Since the coremask defines the maximum scale-out for such a application,
> > the master lcore becomes to the minimum scale-in.
> > Imagine, running multiple primary processed of such DPDK applications,
> > users might want to overlap the coremasks for scaling. However, it would
> > still make sense to run the master lcores on different CPU cores.
> >
> > In DPDK vSwitch we might end up in such a scenario with a future release:
> >    https://lists.01.org/pipermail/dpdk-ovs/2014-March/000770.html
> >    https://lists.01.org/pipermail/dpdk-ovs/2014-March/000773.html
> >
> > Thanks,
> >
> > Simon
> >
> > On 08.07.2014 10:28, Simon Kuenzer wrote:
> >> This commit enables users to specify the lcore id that
> >> is used as master lcore.
> >>
> >> Signed-off-by: Simon Kuenzer <simon.kuenzer@neclab.eu>
> >> ---
> >>   lib/librte_eal/linuxapp/eal/eal.c |   33
> >> +++++++++++++++++++++++++++++++++
> >>   1 file changed, 33 insertions(+)
> >>
> >> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
> >> b/lib/librte_eal/linuxapp/eal/eal.c
> >> index 573fd06..4ad5b9b 100644
> >> --- a/lib/librte_eal/linuxapp/eal/eal.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> >> @@ -101,6 +101,7 @@
> >>   #define OPT_XEN_DOM0    "xen-dom0"
> >>   #define OPT_CREATE_UIO_DEV "create-uio-dev"
> >>   #define OPT_VFIO_INTR    "vfio-intr"
> >> +#define OPT_MASTER_LCORE "master-lcore"
> >>
> >>   #define RTE_EAL_BLACKLIST_SIZE    0x100
> >>
> >> @@ -336,6 +337,7 @@ eal_usage(const char *prgname)
> >>              "[--proc-type primary|secondary|auto] \n\n"
> >>              "EAL options:\n"
> >>              "  -c COREMASK  : A hexadecimal bitmask of cores to run
> >> on\n"
> >> +           "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
> >>              "  -n NUM       : Number of memory channels\n"
> >>              "  -v           : Display version information on startup\n"
> >>              "  -d LIB.so    : add driver (can be used multiple times)\n"
> >> @@ -468,6 +470,21 @@ eal_parse_coremask(const char *coremask)
> >>       return 0;
> >>   }
> >>
> >> +/* Changes the lcore id of the master thread */
> >> +static int
> >> +eal_parse_master_lcore(const char *arg)
> >> +{
> >> +    struct rte_config *cfg = rte_eal_get_configuration();
> >> +    int master_lcore = atoi(arg);
> >> +
> >> +    if (!(master_lcore >= 0 && master_lcore < RTE_MAX_LCORE))
> >> +        return -1;
> >> +    if (cfg->lcore_role[master_lcore] != ROLE_RTE)
> >> +        return -1;
> >> +    cfg->master_lcore = master_lcore;
> >> +    return 0;
> >> +}
> >> +
> >>   static int
> >>   eal_parse_syslog(const char *facility)
> >>   {
> >> @@ -653,6 +670,7 @@ eal_parse_args(int argc, char **argv)
> >>           {OPT_HUGE_DIR, 1, 0, 0},
> >>           {OPT_NO_SHCONF, 0, 0, 0},
> >>           {OPT_PROC_TYPE, 1, 0, 0},
> >> +        {OPT_MASTER_LCORE, 1, 0, 0},
> >>           {OPT_FILE_PREFIX, 1, 0, 0},
> >>           {OPT_SOCKET_MEM, 1, 0, 0},
> >>           {OPT_PCI_WHITELIST, 1, 0, 0},
> >> @@ -802,6 +820,21 @@ eal_parse_args(int argc, char **argv)
> >>               else if (!strcmp(lgopts[option_index].name,
> >> OPT_PROC_TYPE)) {
> >>                   internal_config.process_type =
> >> eal_parse_proc_type(optarg);
> >>               }
> >> +            else if (!strcmp(lgopts[option_index].name,
> >> OPT_MASTER_LCORE)) {
> >> +                if (!coremask_ok) {
> >> +                    RTE_LOG(ERR, EAL, "please specify the master "
> >> +                            "lcore id after specifying "
> >> +                            "the coremask\n");
> >> +                    eal_usage(prgname);
> >> +                    return -1;
> >> +                }
> >> +                if (eal_parse_master_lcore(optarg) < 0) {
> >> +                    RTE_LOG(ERR, EAL, "invalid parameter for --"
> >> +                            OPT_MASTER_LCORE "\n");
> >> +                    eal_usage(prgname);
> >> +                    return -1;
> >> +                }
> >> +            }
> >>               else if (!strcmp(lgopts[option_index].name,
> >> OPT_FILE_PREFIX)) {
> >>                   internal_config.hugefile_prefix = optarg;
> >>               }
> >>
> >
  

Hi Hiroshi,

2014-07-22 23:40, Hiroshi Shimamoto:
> does anyone have interest in this functionality?
> 
> I think this is important and useful.
> Since we should care about core assignment to get high performance
> and the master lcore thread is special in DPDK, we will want to
> assign the master to the target core.
> For example, with hyperthreading I'd like to make a pair of packet
> processing threads into one physical core and separate the master
> thread which does some management.

Thank you for showing your interest.
Does it mean you carefully reviewed this patch? In this case, I'd appreciate
a note "Reviewed-by:".

Thanks
  

> Hi all,
> 
> does anyone have interest in this functionality?

Yes, I think this is useful for DPDK vSwitch.
> 
> I think this is important and useful.
> Since we should care about core assignment to get high performance and the
> master lcore thread is special in DPDK, we will want to assign the master to
> the target core.
> For example, with hyperthreading I'd like to make a pair of packet processing
> threads into one physical core and separate the master thread which does
> some management.
> 
> thanks,
> Hiroshi
>
  

Hi,

> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id
> 
> Hi Hiroshi,
> 
> 2014-07-22 23:40, Hiroshi Shimamoto:
> > does anyone have interest in this functionality?
> >
> > I think this is important and useful.
> > Since we should care about core assignment to get high performance
> > and the master lcore thread is special in DPDK, we will want to
> > assign the master to the target core.
> > For example, with hyperthreading I'd like to make a pair of packet
> > processing threads into one physical core and separate the master
> > thread which does some management.
> 
> Thank you for showing your interest.
> Does it mean you carefully reviewed this patch? In this case, I'd appreciate
> a note "Reviewed-by:".

Not yet deeply, wait a bit, we're testing this patch in our application.
Will report if it works fine.

By the way, we should add the same code into the BSD code, right?

thanks,
Hiroshi

> 
> Thanks
> --
> Thomas
  

2014-07-23 08:53, Hiroshi Shimamoto:
> 2014-07-23 09:50, Thomas Monjalon:
> > 2014-07-22 23:40, Hiroshi Shimamoto:
> > > does anyone have interest in this functionality?
> > >
> > > I think this is important and useful.
> > > Since we should care about core assignment to get high performance
> > > and the master lcore thread is special in DPDK, we will want to
> > > assign the master to the target core.
> > > For example, with hyperthreading I'd like to make a pair of packet
> > > processing threads into one physical core and separate the master
> > > thread which does some management.
> > 
> > Thank you for showing your interest.
> > Does it mean you carefully reviewed this patch? In this case, I'd appreciate
> > a note "Reviewed-by:".
> 
> Not yet deeply, wait a bit, we're testing this patch in our application.
> Will report if it works fine.
> 
> By the way, we should add the same code into the BSD code, right?

Right.
I'd prefer to reduce the duplicated footprint and have more common code
between BSD and Linux. But waiting this enhancement, we have to maintain
the duplicated code for BSD.
  

On 23.07.2014 11:04, Thomas Monjalon wrote:
> 2014-07-23 08:53, Hiroshi Shimamoto:
>> 2014-07-23 09:50, Thomas Monjalon:
>>> 2014-07-22 23:40, Hiroshi Shimamoto:
>>>> does anyone have interest in this functionality?
>>>>
>>>> I think this is important and useful.
>>>> Since we should care about core assignment to get high performance
>>>> and the master lcore thread is special in DPDK, we will want to
>>>> assign the master to the target core.
>>>> For example, with hyperthreading I'd like to make a pair of packet
>>>> processing threads into one physical core and separate the master
>>>> thread which does some management.
>>>
>>> Thank you for showing your interest.
>>> Does it mean you carefully reviewed this patch? In this case, I'd appreciate
>>> a note "Reviewed-by:".
>>
>> Not yet deeply, wait a bit, we're testing this patch in our application.
>> Will report if it works fine.
>>
>> By the way, we should add the same code into the BSD code, right?
>
> Right.
> I'd prefer to reduce the duplicated footprint and have more common code
> between BSD and Linux. But waiting this enhancement, we have to maintain
> the duplicated code for BSD.
>

Hi all,

I can provide the same patch also for BSD. However, I do not have a 
machine to test it. Interested?

Thanks,

Simon
  

Hi all,

the only issue I could imagine is that current DPDK applications are
utilizing the implicit assumption that the master lcore is always set to
the first available lcore. I would consider this as a "bug" in the
application because it sets up its worker threads not "properly".

However, as far I could check it, the DPDK framework seems to cope with
it correctly.
It would be nice if somebody else could confirm my statement.

Thanks,

Simon

On 23.07.2014 10:53, Hiroshi Shimamoto wrote:
> Hi,
> 
>> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id
>>
>> Hi Hiroshi,
>>
>> 2014-07-22 23:40, Hiroshi Shimamoto:
>>> does anyone have interest in this functionality?
>>>
>>> I think this is important and useful.
>>> Since we should care about core assignment to get high performance
>>> and the master lcore thread is special in DPDK, we will want to
>>> assign the master to the target core.
>>> For example, with hyperthreading I'd like to make a pair of packet
>>> processing threads into one physical core and separate the master
>>> thread which does some management.
>>
>> Thank you for showing your interest.
>> Does it mean you carefully reviewed this patch? In this case, I'd appreciate
>> a note "Reviewed-by:".
> 
> Not yet deeply, wait a bit, we're testing this patch in our application.
> Will report if it works fine.
> 
> By the way, we should add the same code into the BSD code, right?
> 
> thanks,
> Hiroshi
> 
>>
>> Thanks
>> --
>> Thomas
  

Hi,

> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id
> 
> 2014-07-23 08:53, Hiroshi Shimamoto:
> > 2014-07-23 09:50, Thomas Monjalon:
> > > 2014-07-22 23:40, Hiroshi Shimamoto:
> > > > does anyone have interest in this functionality?
> > > >
> > > > I think this is important and useful.
> > > > Since we should care about core assignment to get high performance
> > > > and the master lcore thread is special in DPDK, we will want to
> > > > assign the master to the target core.
> > > > For example, with hyperthreading I'd like to make a pair of packet
> > > > processing threads into one physical core and separate the master
> > > > thread which does some management.
> > >
> > > Thank you for showing your interest.
> > > Does it mean you carefully reviewed this patch? In this case, I'd appreciate
> > > a note "Reviewed-by:".
> >
> > Not yet deeply, wait a bit, we're testing this patch in our application.
> > Will report if it works fine.

Sorry a delay, I had confirmed the functionality.
I'm fine to add
Reviewed-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

thanks,
Hiroshi

> >
> > By the way, we should add the same code into the BSD code, right?
> 
> Right.
> I'd prefer to reduce the duplicated footprint and have more common code
> between BSD and Linux. But waiting this enhancement, we have to maintain
> the duplicated code for BSD.
> 
> --
> Thomas
  

> On Jul 8, 2014, at 5:28 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote:
> 
> +			else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) {
> +				if (!coremask_ok) {
> +					RTE_LOG(ERR, EAL, "please specify the master "
> +							"lcore id after specifying "
> +							"the coremask\n");
> +					eal_usage(prgname);
> +					return -1;
> +				}


Hi Simon,

I think that forcing a particular command line order is not that clean.  It might be better to remove the cfg->master_lcore setting from eal_parse_coremask(), and defer the selection of the master lcore until all of the command-line options have been parsed.  If —master-lcore was specified, save the value and use that, otherwise rte_get_next_lcore(-1, 0, 0) can return the first bit set in the coremask.

-Aaron
  

Hi Simon,

Thanks for the patch, this will be useful for us.  I responded separately to your original post with one suggestion.

Our application currently assumes that DPDK will assign the first bit set in the coremask to the master lcore.  As far as I can tell, this is hard-coded as of 1.7.1.  But we would like the ability for our application to specify any bit from the coremask to serve as the master lcore.

I don’t see any compatibility issues with this.  Existing applications should behave as before.

Thomas, could this be accepted for the 1.8 release?  Or will that only happen if the BSD side can be patched as well?

-Aaron

> On Jul 23, 2014, at 9:10 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote:
> 
> Hi all,
> 
> the only issue I could imagine is that current DPDK applications are
> utilizing the implicit assumption that the master lcore is always set to
> the first available lcore. I would consider this as a "bug" in the
> application because it sets up its worker threads not "properly".
> 
> However, as far I could check it, the DPDK framework seems to cope with
> it correctly.
> It would be nice if somebody else could confirm my statement.
> 
> Thanks,
> 
> Simon
> 
> On 23.07.2014 10:53, Hiroshi Shimamoto wrote:
>> Hi,
>> 
>>> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id
>>> 
>>> Hi Hiroshi,
>>> 
>>> 2014-07-22 23:40, Hiroshi Shimamoto:
>>>> does anyone have interest in this functionality?
>>>> 
>>>> I think this is important and useful.
>>>> Since we should care about core assignment to get high performance
>>>> and the master lcore thread is special in DPDK, we will want to
>>>> assign the master to the target core.
>>>> For example, with hyperthreading I'd like to make a pair of packet
>>>> processing threads into one physical core and separate the master
>>>> thread which does some management.
>>> 
>>> Thank you for showing your interest.
>>> Does it mean you carefully reviewed this patch? In this case, I'd appreciate
>>> a note "Reviewed-by:".
>> 
>> Not yet deeply, wait a bit, we're testing this patch in our application.
>> Will report if it works fine.
>> 
>> By the way, we should add the same code into the BSD code, right?
>> 
>> thanks,
>> Hiroshi
>> 
>>> 
>>> Thanks
>>> --
>>> Thomas
>
  

2014-11-03 13:02, Aaron Campbell:
> Hi Simon,
> 
> Thanks for the patch, this will be useful for us.  I responded separately to your original post with one suggestion.
> 
> Our application currently assumes that DPDK will assign the first bit set in the coremask to the master lcore.  As far as I can tell, this is hard-coded as of 1.7.1.  But we would like the ability for our application to specify any bit from the coremask to serve as the master lcore.
> 
> I don’t see any compatibility issues with this.  Existing applications should behave as before.
> 
> Thomas, could this be accepted for the 1.8 release?  Or will that only happen if the BSD side can be patched as well?

No need for BSD side patch because option management is now common between
BSD and Linux. I'm going to send an updated version of this patch.

> > On Jul 23, 2014, at 9:10 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote:
> > 
> > Hi all,
> > 
> > the only issue I could imagine is that current DPDK applications are
> > utilizing the implicit assumption that the master lcore is always set to
> > the first available lcore. I would consider this as a "bug" in the
> > application because it sets up its worker threads not "properly".
> > 
> > However, as far I could check it, the DPDK framework seems to cope with
> > it correctly.
> > It would be nice if somebody else could confirm my statement.
> > 
> > Thanks,
> > 
> > Simon
> > 
> > On 23.07.2014 10:53, Hiroshi Shimamoto wrote:
> >> Hi,
> >> 
> >>> Subject: Re: [dpdk-dev] [PATCH] eal/linuxapp: Add parameter to specify master lcore id
> >>> 
> >>> Hi Hiroshi,
> >>> 
> >>> 2014-07-22 23:40, Hiroshi Shimamoto:
> >>>> does anyone have interest in this functionality?
> >>>> 
> >>>> I think this is important and useful.
> >>>> Since we should care about core assignment to get high performance
> >>>> and the master lcore thread is special in DPDK, we will want to
> >>>> assign the master to the target core.
> >>>> For example, with hyperthreading I'd like to make a pair of packet
> >>>> processing threads into one physical core and separate the master
> >>>> thread which does some management.
> >>> 
> >>> Thank you for showing your interest.
> >>> Does it mean you carefully reviewed this patch? In this case, I'd appreciate
> >>> a note "Reviewed-by:".
> >> 
> >> Not yet deeply, wait a bit, we're testing this patch in our application.
> >> Will report if it works fine.
> >> 
> >> By the way, we should add the same code into the BSD code, right?
> >> 
> >> thanks,
> >> Hiroshi
  

2014-11-03 13:02, Aaron Campbell:
> > On Jul 8, 2014, at 5:28 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote:
> > 
> > +			else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) {
> > +				if (!coremask_ok) {
> > +					RTE_LOG(ERR, EAL, "please specify the master "
> > +							"lcore id after specifying "
> > +							"the coremask\n");
> > +					eal_usage(prgname);
> > +					return -1;
> > +				}
> 
> 
> Hi Simon,
> 
> I think that forcing a particular command line order is not that clean.
> It might be better to remove the cfg->master_lcore setting from
> eal_parse_coremask(), and defer the selection of the master lcore until
> all of the command-line options have been parsed.  If —master-lcore was
> specified, save the value and use that, otherwise
> rte_get_next_lcore(-1, 0, 0) can return the first bit set in the coremask.

It's not sufficient: eal_parse_master_lcore() requires cfg->lcore_role
to be set. There is a real dependency between these 2 options.
I'm going to submit a v2. Feel free to improve it with another patch.
  

> On Nov 4, 2014, at 3:00 PM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 
> 2014-11-03 13:02, Aaron Campbell:
>>> On Jul 8, 2014, at 5:28 AM, Simon Kuenzer <simon.kuenzer@neclab.eu> wrote:
>>> 
>>> +			else if (!strcmp(lgopts[option_index].name, OPT_MASTER_LCORE)) {
>>> +				if (!coremask_ok) {
>>> +					RTE_LOG(ERR, EAL, "please specify the master "
>>> +							"lcore id after specifying "
>>> +							"the coremask\n");
>>> +					eal_usage(prgname);
>>> +					return -1;
>>> +				}
>> 
>> 
>> Hi Simon,
>> 
>> I think that forcing a particular command line order is not that clean.
>> It might be better to remove the cfg->master_lcore setting from
>> eal_parse_coremask(), and defer the selection of the master lcore until
>> all of the command-line options have been parsed.  If —master-lcore was
>> specified, save the value and use that, otherwise
>> rte_get_next_lcore(-1, 0, 0) can return the first bit set in the coremask.
> 
> It's not sufficient: eal_parse_master_lcore() requires cfg->lcore_role
> to be set. There is a real dependency between these 2 options.
> I'm going to submit a v2. Feel free to improve it with another patch.

I was nit-picking; although it might be nice if the new option is given, to verify the specified lcore is in the coremask.  I will ack v2 though and this can be improved some other time.

-Aaron