Re: [PATCH v2 5/6] common_linuxapp: Added CONFIG_RTE_ETHDEV_LRO_SUPPORT option

[Vlad Zolotarov <vladz-RmZWMc9puTNJc61us3aD9laTQe2KTcn/@public.gmane.org>]

On 03/05/15 15:19, Thomas Monjalon wrote:
> 2015-03-05 13:28, Vlad Zolotarov:
>> Enables LRO support in PMDs.
>>
>> Signed-off-by: Vlad Zolotarov <vladz-RmZWMc9puTNJc61us3aD9laTQe2KTcn/@public.gmane.org>
>> ---
>>   config/common_linuxapp | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/config/common_linuxapp b/config/common_linuxapp
>> index 97f1c9e..5b98595 100644
>> --- a/config/common_linuxapp
>> +++ b/config/common_linuxapp
>> @@ -137,6 +137,7 @@ CONFIG_RTE_MAX_ETHPORTS=32
>>   CONFIG_RTE_LIBRTE_IEEE1588=n
>>   CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
>>   CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y
>> +CONFIG_RTE_ETHDEV_LRO_SUPPORT=y
> Sorry I don't really follow this ixgbe discussion but I wonder why you
> would add a compile time option for this feature.

The only reason is to be able to detect that the feature is present in 
the DPDK version u r compiling against because of the API change.
Currently, this can't be done using the DPDK version thus we may either 
do a try-compilation and if it fails define some application-level macro 
disabling
the feature usage or we may define a macro in the library level 
(together with tons of other such macros like those in the patch snippet 
above).


> What is the benefit of disabling it?

No benefit whatsoever.

> And if really needed, this patch would make more sense merged with the
> code under ifdef.

I strongly disagree - the amount of #ifdefs in the DPDK source is 
absolutely enormous. It makes reading and  understanding the code really 
hard.
Therefore, I tried to reduce the amount of time the already existing 
macros have to be queried (see PATCH4). And of course I don't see any 
sense of adding new ones more than really needed. And in LRO case - it's 
a single time, where the feature is manifested by the HW.


>