netdev - Re: [PATCH net-next v5 1/5] ethtool: introduce core UAPI and driver API for PHY MSE diagnostics

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: <aMP0F0NVrIHk7jBY@pengutronix.de>
Date: Fri, 12 Sep 2025 12:21:11 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Andrew Lunn <andrew@...n.ch>, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Donald Hunter <donald.hunter@...il.com>,
	Jonathan Corbet <corbet@....net>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Kory Maincent <kory.maincent@...tlin.com>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	Nishanth Menon <nm@...com>, kernel@...gutronix.de,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	UNGLinuxDriver@...rochip.com, linux-doc@...r.kernel.org,
	Michal Kubecek <mkubecek@...e.cz>, Roan van Dijk <roan@...tonic.nl>
Subject: Re: [PATCH net-next v5 1/5] ethtool: introduce core UAPI and driver
 API for PHY MSE diagnostics

On Thu, Sep 11, 2025 at 07:23:18PM -0700, Jakub Kicinski wrote:
> On Mon,  8 Sep 2025 14:46:06 +0200 Oleksij Rempel wrote:
> > Add the base infrastructure for Mean Square Error (MSE) diagnostics,
> > as proposed by the OPEN Alliance "Advanced diagnostic features for
> > 100BASE-T1 automotive Ethernet PHYs" [1] specification.
> > 
> > The OPEN Alliance spec defines only average MSE and average peak MSE
> > over a fixed number of symbols. However, other PHYs, such as the
> > KSZ9131, additionally expose a worst-peak MSE value latched since the
> > last channel capture. This API accounts for such vendor extensions by
> > adding a distinct capability bit and snapshot field.
> > 
> > Channel-to-pair mapping is normally straightforward, but in some cases
> > (e.g. 100BASE-TX with MDI-X resolution unknown) the mapping is ambiguous.
> > If hardware does not expose MDI-X status, the exact pair cannot be
> > determined. To avoid returning misleading per-channel data in this case,
> > a LINK selector is defined for aggregate MSE measurements.
> > 
> > All investigated devices differ in MSE configuration parameters, such
> > as sample rate, number of analyzed symbols, and scaling factors.
> > For example, the KSZ9131 uses different scaling for MSE and pMSE.
> > To make this visible to userspace, scale limits and timing information
> > are returned via get_mse_config().
> 
> But the parameter set is set by the standard? If not we should annotate
> which one is and which isn't.

Do you mean we should show which parameters are defined by a standard
(for example Open-Alliance - MSE/pMSE) or which parts of the measurement
method - like how many samples in what time - are vendor or product
specific?

And should we only write this in comments/docs, or add a flag/enum so
user space can detect it?

> > +  -
> > +    name: phy-mse-capability
> > +    doc: |
> > +      Bitmask flags for MSE capabilities.
> > +
> > +      These flags are used in the 'supported_caps' field of struct
> > +      phy_mse_config to indicate which measurement capabilities are supported
> > +      by the PHY hardware.
> > +    type: flags
> > +    name-prefix: phy-mse-cap-
> > +    entries:
> > +      -
> > +        name: avg
> > +        doc: Average MSE value is supported.
> > +      -
> > +        name: peak
> > +        doc: Current peak MSE value is supported.
> > +      -
> > +        name: worst-peak
> > +        doc: Worst-case peak MSE (latched high-water mark) is supported.
> > +      -
> > +        name: channel-a
> > +        doc: Diagnostics for Channel A are supported.
> > +      -
> > +        name: channel-b
> > +        doc: Diagnostics for Channel B are supported.
> > +      -
> > +        name: channel-c
> > +        doc: Diagnostics for Channel C are supported.
> > +      -
> > +        name: channel-d
> > +        doc: Diagnostics for Channel D are supported.
> > +      -
> > +        name: worst-channel
> > +        doc: |
> > +          Hardware or drivers can identify the single worst-performing channel
> > +          without needing to query each one individually.
> > +      -
> > +        name: link
> > +        doc: |
> > +          Hardware provides only a link-wide aggregate MSE or cannot map
> > +          the measurement to a specific channel/pair. Typical for media where
> > +          the MDI/MDI-X resolution or pair mapping is unknown (e.g. 100BASE-TX).
> 
> Should we invert the order here? I think it's more likely we'd
> encounter new statistical measures rather than new channels.
> So channels should go first, and then the measures?

ack, sounds good.

> > +  -
> > +    name: phy-mse-channel
> > +    doc: |
> > +      Identifiers for the 'channel' parameter used to select which diagnostic
> > +      data to retrieve.
> > +    type: enum
> > +    name-prefix: phy-mse-channel-
> > +    entries:
> > +      -
> > +        name: a
> > +        value: 0
> 
> Don't enums default to starting from 0?  I think setting value is unnecessary

ack.

> > +        doc: Request data for channel A.
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |