netdev - Re: [PATCH net-next 1/5] net: wangxun: add flow control support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZV4ssdbQyxtYgURN@shell.armlinux.org.uk>
Date: Wed, 22 Nov 2023 16:30:41 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: Jiawen Wu <jiawenwu@...stnetic.com>, netdev@...r.kernel.org,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, horms@...nel.org, mengyuanlou@...-swift.com
Subject: Re: [PATCH net-next 1/5] net: wangxun: add flow control support

On Wed, Nov 22, 2023 at 05:15:05PM +0100, Andrew Lunn wrote:
> > +static void ngbe_get_pauseparam(struct net_device *netdev,
> > +				struct ethtool_pauseparam *pause)
> > +{
> > +	struct wx *wx = netdev_priv(netdev);
> > +
> > +	pause->autoneg = !wx->fc.disable_fc_autoneg;
> 
> Maybe call it enable_fs_autoneg, since that is what the kAPI uses?

fs?

> > +	wx->fc.disable_fc_autoneg = !pause->autoneg;
> > +	wx->fc.tx_pause = pause->tx_pause;
> > +	wx->fc.rx_pause = pause->rx_pause;
> > +
> > +	phy_set_asym_pause(wx->phydev, pause->rx_pause, pause->tx_pause);
> 
> You should only be doing this if pause->autoneg is true. If it is
> false, you ignore the results from autoneg, and just configure the
> hardware as indicated by pause->{tr}x_pause.

However, if autoneg is supported, but pause autoneg is disabled, there
is still the need to update the PHY's advertisement so the remote end
knows what's going on, as documented in the user API:

" * If the link is autonegotiated, drivers should use
  * mii_advertise_flowctrl() or similar code to set the advertised
  * pause frame capabilities based on the @rx_pause and @tx_pause flags,
  * even if @autoneg is zero. ... "

You are correct that when !pause->autoneg, tx_pause/rx_pause are to be
used in place of the negotiated versions.

Also... when getting the pause parameters, tx_pause/rx_pause _should_
reflect what was set for these parameters via the set function, *not*
the current state affected by negotiation.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!