linux-kernel - Re: [PATCH net-next v5 1/9] net: phylink: provide mac_get_pcs_neg

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: <Zc47T/qv8Xg2SA21@shell.armlinux.org.uk>
Date: Thu, 15 Feb 2024 16:26:55 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Choong Yong Liang <yong.liang.choong@...ux.intel.com>
Cc: Rajneesh Bhardwaj <irenic.rajneesh@...il.com>,
	David E Box <david.e.box@...ux.intel.com>,
	Hans de Goede <hdegoede@...hat.com>,
	Mark Gross <markgross@...nel.org>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jose Abreu <Jose.Abreu@...opsys.com>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Richard Cochran <richardcochran@...il.com>,
	Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Jesper Dangaard Brouer <hawk@...nel.org>,
	John Fastabend <john.fastabend@...il.com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Andrew Halaney <ahalaney@...hat.com>,
	Serge Semin <fancer.lancer@...il.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org,
	platform-driver-x86@...r.kernel.org, linux-hwmon@...r.kernel.org,
	bpf@...r.kernel.org, Voon Wei Feng <weifeng.voon@...el.com>,
	Michael Sit Wei Hong <michael.wei.hong.sit@...el.com>,
	Lai Peter Jun Ann <jun.ann.lai@...el.com>,
	Abdul Rahim Faizal <faizal.abdul.rahim@...el.com>
Subject: Re: [PATCH net-next v5 1/9] net: phylink: provide
 mac_get_pcs_neg_mode() function

On Thu, Feb 15, 2024 at 11:04:51AM +0800, Choong Yong Liang wrote:
> Phylink invokes the 'mac_get_pcs_neg_mode' function during interface mode
> switching and initial startup.
> 
> This function is optional; if 'phylink_pcs_neg_mode' fails to accurately
> reflect the current PCS negotiation mode, the MAC driver can determine the
> mode based on the interface mode, current link negotiation mode, and
> advertising link mode.
> 
> For instance, if the interface switches from 2500baseX to SGMII mode,
> and the current link mode is MLO_AN_PHY, calling 'phylink_pcs_neg_mode'
> would yield PHYLINK_PCS_NEG_OUTBAND. Since the MAC and PCS driver require
> PHYLINK_PCS_NEG_INBAND_ENABLED, the 'mac_get_pcs_neg_mode' function
> will calculate the mode based on the interface, current link negotiation
> mode, and advertising link mode, returning PHYLINK_PCS_NEG_OUTBAND to
> enable the PCS to configure the correct settings.

This paragraph doesn't make sense - at least to me. It first talks about
requiring PHYLINK_PCS_NEG_INBAND_ENABLED when in SGMII mode. On this:

1) are you sure that the hardware can't be programmed for the SGMII
symbol repititions? 

2) what happens if you're paired with a PHY (e.g. on a SFP module)
which uses SGMII but has no capability of providing the inband data?
(They do exist.) If your hardware truly does require inband data, it
is going to be fundamentally inoperative with these modules.

Next, you then talk about returning PHYLINK_PCS_NEG_OUTBAND for the
"correct settings". How does this relate to the first part where you
basically describe the problem as SGMII requring inband? Basically
the two don't follow.

How, from a design point of view, because this fundamentally allows
drivers to change how the system behaves, it will allow radically
different behaviours for the same parameters between different drivers.
I am opposed to that - I want to see a situation where we have uniform
behaviour for the same configuration, and where hardware doesn't
support something, we have some way to indicate that via some form
of capabilities.

The issue of whether 2500base-X has inband or not is a long standing
issue, and there are arguments (and hardware) that take totally
opposing views on this. There is hardware where 2500base-X inband
_must_ be used or the link doesn't come up. There is also hardware
where 2500base-X inband is not "supported" in documentation but works
in practice. There is also hardware where 2500base-X inband doesn't
work. The whole thing is a total mess (thanks IEEE 802.3 for not
getting on top of this early enough... and what's now stated in 802.3
for 2500base-X is now irrelevant because they were too late to the
party.)

I haven't been able to look at this issue over the last few weeks
because of being at a summit, and then suffering with flu and its
recovery. However, I have been working on how we can identify the
capabilities of the PCS and PHY w.r.t. inband support in various
interface modes, and how we can handle the result. That work is
ongoing (as and when I have a clear head from after-flu effects.)

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