[dpdk-dev] net/sfc: enable TSO by default

  Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
It is a mistake that TSO support is compiled out by default.
We would be happy to enable it by deafult, but strictly speaking
it is not a bug fix.

Arguments to enable are:
 - be more feature-rich (and user-friendly) in default config
 - the most of internal testing is done with TSO enabled

 config/common_base | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

2017-01-19 15:52, Andrew Rybchenko:
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> It is a mistake that TSO support is compiled out by default.
> We would be happy to enable it by deafult, but strictly speaking
> it is not a bug fix.
> 
> Arguments to enable are:
>  - be more feature-rich (and user-friendly) in default config
>  - the most of internal testing is done with TSO enabled
> 
>  config/common_base | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/config/common_base b/config/common_base
> index b9fb8e2..134e9b9 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -261,7 +261,7 @@ CONFIG_RTE_LIBRTE_BNXT_PMD=y
>  #
>  CONFIG_RTE_LIBRTE_SFC_EFX_PMD=y
>  CONFIG_RTE_LIBRTE_SFC_EFX_DEBUG=n
> -CONFIG_RTE_LIBRTE_SFC_EFX_TSO=n
> +CONFIG_RTE_LIBRTE_SFC_EFX_TSO=y

There should not be such option in the build-time configuration.
Why keeping it (even enabled by default)?
  

On 01/20/2017 05:29 PM, Thomas Monjalon wrote:
> 2017-01-19 15:52, Andrew Rybchenko:
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>> It is a mistake that TSO support is compiled out by default.
>> We would be happy to enable it by deafult, but strictly speaking
>> it is not a bug fix.
>>
>> Arguments to enable are:
>>   - be more feature-rich (and user-friendly) in default config
>>   - the most of internal testing is done with TSO enabled
>>
>>   config/common_base | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/config/common_base b/config/common_base
>> index b9fb8e2..134e9b9 100644
>> --- a/config/common_base
>> +++ b/config/common_base
>> @@ -261,7 +261,7 @@ CONFIG_RTE_LIBRTE_BNXT_PMD=y
>>   #
>>   CONFIG_RTE_LIBRTE_SFC_EFX_PMD=y
>>   CONFIG_RTE_LIBRTE_SFC_EFX_DEBUG=n
>> -CONFIG_RTE_LIBRTE_SFC_EFX_TSO=n
>> +CONFIG_RTE_LIBRTE_SFC_EFX_TSO=y
> There should not be such option in the build-time configuration.
> Why keeping it (even enabled by default)?

Initially it was introduced since packet rates with TSO compiled out
were slightly but noticeable better. Finally we have changed approach
and performance improvements planned in 17.05 do not require it.
I'll be happy to remove it completely, if there is no problems to
include it in 17.02 (it will be a bit bigger but functionally equal to
this one).
  

On 1/20/2017 2:37 PM, Andrew Rybchenko wrote:
> On 01/20/2017 05:29 PM, Thomas Monjalon wrote:
>> 2017-01-19 15:52, Andrew Rybchenko:
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> ---
>>> It is a mistake that TSO support is compiled out by default.
>>> We would be happy to enable it by deafult, but strictly speaking
>>> it is not a bug fix.
>>>
>>> Arguments to enable are:
>>>  - be more feature-rich (and user-friendly) in default config
>>>  - the most of internal testing is done with TSO enabled
>>>
>>>  config/common_base | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/config/common_base b/config/common_base
>>> index b9fb8e2..134e9b9 100644
>>> --- a/config/common_base
>>> +++ b/config/common_base
>>> @@ -261,7 +261,7 @@ CONFIG_RTE_LIBRTE_BNXT_PMD=y
>>>  #
>>>  CONFIG_RTE_LIBRTE_SFC_EFX_PMD=y
>>>  CONFIG_RTE_LIBRTE_SFC_EFX_DEBUG=n
>>> -CONFIG_RTE_LIBRTE_SFC_EFX_TSO=n
>>> +CONFIG_RTE_LIBRTE_SFC_EFX_TSO=y
>> There should not be such option in the build-time configuration.
>> Why keeping it (even enabled by default)?
> 
> Initially it was introduced since packet rates with TSO compiled out
> were slightly but noticeable better. Finally we have changed approach
> and performance improvements planned in 17.05 do not require it.
> I'll be happy to remove it completely, if there is no problems to
> include it in 17.02 (it will be a bit bigger but functionally equal to
> this one).

If it will be just removing ifdef, with keeping same functionality, I
wouldn't mind bigger patch.