Re: [PATCH 10/10] eal: add option --master-lcore

[Simon Kuenzer <simon.kuenzer-kcmmt4fgdiuHXe+LvDLADg@public.gmane.org>]

On 25.11.2014 14:39, Bruce Richardson wrote:
> On Tue, Nov 25, 2014 at 01:45:22PM +0100, Thomas Monjalon wrote:
>> Hi Simon,
>>
>> 2014-11-25 10:09, Simon Kuenzer:
>>> thanks for your work. I have one (minor) comment for this patch that
>>> should be fixed in a later version.
>>
>>>> +	/* default master lcore is the first one */
>>>> +	if (cfg->master_lcore == 0)
>>>> +		cfg->master_lcore = rte_get_next_lcore(-1, 0, 0);
>>>> +
>>>
>>> Might be confusing if a user specifies --master-lcore 0 and uses a
>>> coremask/corelist where core id 0 is not specified.
>>
>> Yes, in this corner case, master-lcore will be adjusted instead of having
>> an error.
>>
>>> What about setting cfg->master_lcore to (RTE_MAX_LCORE + 1) on
>>> initialization in order to distinguish if a master_lcore got specified
>>> by the user or not?
>>
>> Even simpler, I can fix it by introducing a flag master_lcore_parsed and
>> do the adjustment only if the option is not parsed.
>>
> I agree that that sounds like a simpler approach, since we already have flags
> for what args are parsed or not.
>
> /Bruce
>

Fine with me :-). I also agree that having the flag is even a cleaner 
solution.

Thanks,

Simon