lkml.org 
[lkml]   [2022]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net-next v3 10/47] net: phylink: Adjust link settings based on rate adaptation
From
Date
On 7/16/22 4:17 PM, Andrew Lunn wrote:
> On Fri, Jul 15, 2022 at 05:59:17PM -0400, Sean Anderson wrote:
>> If the phy is configured to use pause-based rate adaptation, ensure that
>> the link is full duplex with pause frame reception enabled. Note that these
>> settings may be overridden by ethtool.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>>
>> Changes in v3:
>> - New
>>
>> drivers/net/phy/phylink.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
>> index 7fa21941878e..7f65413aa778 100644
>> --- a/drivers/net/phy/phylink.c
>> +++ b/drivers/net/phy/phylink.c
>> @@ -1445,6 +1445,10 @@ static void phylink_phy_change(struct phy_device *phydev, bool up)
>> pl->phy_state.speed = phy_interface_speed(phydev->interface,
>> phydev->speed);
>> pl->phy_state.duplex = phydev->duplex;
>> + if (phydev->rate_adaptation == RATE_ADAPT_PAUSE) {
>> + pl->phy_state.duplex = DUPLEX_FULL;
>> + rx_pause = true;
>> + }
>
> I would not do this. If the requirements for rate adaptation are not
> fulfilled, you should turn off rate adaptation.
>
> A MAC which knows rate adaptation is going on can help out, by not
> advertising 10Half, 100Half etc. Autoneg will then fail for modes
> where rate adaptation does not work.

OK, so maybe it is better to phylink_warn here. Something along the
lines of "phy using pause-based rate adaptation, but duplex is %s".

> The MAC should also be declaring what sort of pause it supports, so
> disable rate adaptation if it does not have async pause.

That's what we do in the previous patch.

The problem is that rx_pause and tx_pause are resolved based on our
advertisement and the link partner's advertisement. However, the link
partner may not support pause frames at all. In that case, we will get
rx_pause and tx_pause as false. However, we still want to enable rx_pause,
because we know that the phy will be emitting pause frames. And of course
the user can always force disable pause frames anyway through ethtool.

--Sean

\
 
 \ /
  Last update: 2022-07-17 00:38    [W:0.222 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site