Re: [dpdk-dev] [PATCH] Revert "app/testpmd: set fixed flag for exact link speed"

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 5/6/2019 9:09 AM, Andrew Rybchenko wrote:
> On 5/4/19 11:45 PM, Thomas Monjalon wrote:
>> 02/05/2019 22:27, Thomas Monjalon:
>>> 02/05/2019 20:31, Ferruh Yigit:
>>>> On 4/29/2019 10:52 AM, Thomas Monjalon wrote:
>>>>> 25/04/2019 17:27, Ferruh Yigit:
>>>>>> On 4/25/2019 2:29 PM, Thomas Monjalon wrote:
>>>>>>> 25/04/2019 13:47, Ferruh Yigit:
>>>>>>>> On 4/25/2019 9:19 AM, Thomas Monjalon wrote:
>>>>>>>>> 25/04/2019 00:03, Ferruh Yigit:
>>>>>>>>>> This reverts commit bdca79053b6aea504d02691d9319fa976062457f.
>>>>>>>>>>
>>>>>>>>>> Not all PMDs support the fixed link speed set, and link speed can be set
>>>>>>>>>> even with auto negotiation enabled. Reverting the patch to not break
>>>>>>>>>> existing usage.
>>>>>>>>> Which PMDs do not support this flag?
>>>>>>>>> Why not fixing the PMDs?
>>>>>>>>>
>>>>>>>> At least ixgbe and i40e is not supporting setting a fixed speed.
>>>>>>>> But I am not sure if this is something to fix, the command in testpmd is to set
>>>>>>>> the link speed, what is the problem with setting the link speed without
>>>>>>>> disabling the auto-negotiation?
>>>>>>> It means it will negotiate with only one speed proposed.
>>>>>> Yes.
>>>>>>
>>>>>>> The real issue is to not support the fixed flag.
>>>>>> I don't know if this is a real issue but
>>>>>> even it is, is it an issue in the scope of this testpmd command?
>>>>>>
>>>>>> right now we are first updating the command to set fixed speed flag, and
>>>>>> requesting PMDs to fix for it, I am suggesting not to update the command at all.
>>>>> I understand. But this change shows a broken behaviour.
>>>>> This is the intent of testpmd to show what works or not in PMDs.
>>>>> How hard is it to fix the PMDs in your opinion?
>>>>>
>>>> As far as I can see the the fixed link speed set is not supported in the PMD.
>>>>
>>>> It may be easy to add perhaps, I don't know, but is it really a "broken
>>>> behavior" to not have this support?
>>>> What defines that setting speed has to be "fixed speed", if this demand is not
>>>> there, should testpmd enforce it?
>>> I think a PMD should support both: fixed or not.
>>>
>>>> In mail thread we have talked that this testpmd command can get an extra
>>>> argument to define the speed fixed or not, this can be used to test fixed speed
>>>> by who wants to test/use fixed speed.
>>>>
>>>> I am for reverting this for the release, and adding a new version next release
>>>> with fixed speed argument, otherwise testpmd won't be used to set the speed for
>>>> some PMDs.
>>> OK
>> We could have an option in testpmd to test ETH_LINK_SPEED_FIXED.
>>
>> Revert applied.
> 
> I agree that revert is the best option for the release, but not long term.
> 
> Typical options are (in order):
>  1. Auto-negotiation of whatever port supports
>  2. Fixed exactly one speed with autoneg disabled
>  3. Auto-negotiation with limitations (and it is really the third option which
> makes sense if and only if interface allows to specify more than one speed to be
> negotiated)
> 
> Right now testpmd supports (1) and (3) with limitation to only one speed to be
> negotiated. I think it is wrong.

+1 to not leave this revert as it is for long term, what do you think for:

1) Implement 'fixed' link speed support in the missing drivers.
2) Send a new version of the testpmd patch with a "fixed" argument, so that we
can support all three above