netdev - Re: [PATCH RFC net-next 7/9] net: pcs: xpcs: correct pause resolution

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: <ZF/TLuDSHDZmwonu@shell.armlinux.org.uk>
Date: Sat, 13 May 2023 19:13:02 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: Jose Abreu <Jose.Abreu@...opsys.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH RFC net-next 7/9] net: pcs: xpcs: correct pause resolution

On Sat, May 13, 2023 at 07:47:35PM +0200, Andrew Lunn wrote:
> On Fri, May 12, 2023 at 06:27:35PM +0100, Russell King (Oracle) wrote:
> > xpcs was indicating symmetric pause should be enabled regardless of
> > the advertisements by either party. Fix this to use
> > linkmode_resolve_pause() now that we're no longer obliterating the
> > link partner's advertisement by logically anding it with our own.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> > ---
> >  drivers/net/pcs/pcs-xpcs.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> > index 43115d04c01a..beed799a69a7 100644
> > --- a/drivers/net/pcs/pcs-xpcs.c
> > +++ b/drivers/net/pcs/pcs-xpcs.c
> > @@ -538,11 +538,20 @@ static void xpcs_resolve_lpa_c73(struct dw_xpcs *xpcs,
> >  				 struct phylink_link_state *state)
> >  {
> >  	__ETHTOOL_DECLARE_LINK_MODE_MASK(res);
> > +	bool tx_pause, rx_pause;
> >  
> >  	/* Calculate the union of the advertising masks */
> >  	linkmode_and(res, state->lp_advertising, state->advertising);
> >  
> > -	state->pause = MLO_PAUSE_TX | MLO_PAUSE_RX;
> > +	/* Resolve pause modes */
> > +	linkmode_resolve_pause(state->advertising, state->lp_advertising,
> > +			       &tx_pause, &rx_pause);
> > +
> > +	if (tx_pause)
> > +		state->pause |= MLO_PAUSE_TX;
> > +	if (rx_pause)
> > +		state->pause |= MLO_PAUSE_RX;
> > +
> 
> Hi Russell
> 
> I must be missing something. Why not use phylink_resolve_an_pause()?

Check the next few patches... it eventually gets to using the c73
helper, entirely eliminating this function. This is a staged
conversion, so that its easier to bisect down to the change that
caused the breakage. Converting straight to the c73 helper would
be a big change - not only fixing the pause resolution mechanism
but also how we do the c73 priority resolution.

Moreover, the above patch can be backported to stable kernels
without too much effort if there's a desire to do so.

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