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
