Messages in this thread | | | Date | Mon, 18 Jul 2022 17:22:35 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH net-next v3 10/47] net: phylink: Adjust link settings based on rate adaptation |
| |
On Sun, Jul 17, 2022 at 03:39:39AM +0200, Andrew Lunn wrote: > > > 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". > > You say 1/2 duplex simply does not work with rate adaptation. So i > would actually return -EINVAL at the point the MAC indicates what > modes it supports if there is a 1/2 duplex mode in the list.
If we have a PHY that supports rate adaption using pause frames, which implies a full duplex link between the PHY and MAC, one would hope that someone isn't silly enough to integrate it with a half-duplex only MAC.
This ought to be handled while bringing up the PHY. If the PHY uses pause frames but the MAC doesn't support full-duplex at the PHY interface speed, then we should not allow the PHY to do rate adaption. The easiest way to achieve that is to not allow the PHY to advertise anything except the PHY interface speed on its media. If that means there's nothing to advertise, then we fail.
> Right, so we need a table somewhere in the documentation listing the > different combinations and what should happen. > > If the MAC does not support rx_pause, rate adaptation is turned off. > If the negotiation results in no rx_pause, force it on anyway with > Pause based adaptation. If ethtool turns pause off, turn off rate > adaptation.
That last bit is really awkward - what if the link partner is doing 100M on the media because that's the fastest it's capable of, but our local PHY is doing rate adaption to 1G, and we turn pause off, causing rate adaption in the PHY to be turned off. We need to reconfigure the advertisement to drop anything except the 1G speed and renegotiate the link, which will cause the link to go down.
That's going to be really odd behaviour for a user to get their head around.
> Does 802.3 say anything about this?
I think rate adaption is out of scope of 802.3.
> We might also want to add an additional state to the ethtool get for > pause, to indicate rx_pause is enabled because of rate adaptation, not > because of autoneg.
That may well be a much better approach; it lets the user see what is going on and it becomes more understandable to the user IMHO.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
| |