Re: [PATCH net-next v8 2/4] ethtool: netlink: add ETHTOOL_MSG_MSE_GET and wire up PHY MSE access

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Thu, Oct 30, 2025 at 12:04:11PM +0100, Paolo Abeni wrote:
> On 10/27/25 1:27 PM, Oleksij Rempel wrote:
> > Introduce the userspace entry point for PHY MSE diagnostics via
> > ethtool netlink. This exposes the core API added previously and
> > returns both capability information and one or more snapshots.
> > 
> > Userspace sends ETHTOOL_MSG_MSE_GET. The reply carries:
> > - ETHTOOL_A_MSE_CAPABILITIES: scale limits and timing information
> > - ETHTOOL_A_MSE_CHANNEL_* nests: one or more snapshots (per-channel
> >   if available, otherwise WORST, otherwise LINK)
> > 
> > Link down returns -ENETDOWN.
> > 
> > Changes:
> >   - YAML: add attribute sets (mse, mse-capabilities, mse-snapshot)
> >     and the mse-get operation
> >   - UAPI (generated): add ETHTOOL_A_MSE_* enums and message IDs,
> >     ETHTOOL_MSG_MSE_GET/REPLY
> >   - ethtool core: add net/ethtool/mse.c implementing the request,
> >     register genl op, and hook into ethnl dispatch
> >   - docs: document MSE_GET in ethtool-netlink.rst
> > 
> > The include/uapi/linux/ethtool_netlink_generated.h is generated
> > from Documentation/netlink/specs/ethtool.yaml.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> > changes v8:
> > - drop user-space channel selector; kernel always returns available selectors
> 
> Overall LGTM, but it's unclear why you dropped the above. I understand
> one of the goal here is to achieve fast data retrival. I _guess_ most of
> the overhead is possibly due to phy regs access. Explicitly selecting a
> single/limited number of channels could/should reduce the number of
> registers access; it looks like a worthy option. What am I missing?

Yes the goal was fast data retrieval. However, I realized the initial
channel selector was just a guess at the right optimization.

My concern is that the channel selector do not fully achieve the goal.
While reading metrics from just one channel is an advantage, the
optimization could be much better if we also allowed reading only one
metric from that channel, instead of all of them.

So, rather than push another half-guess, I decided it is safer to
remove this part of the interface entirely for now. Once we have data on
where the actual performance bottlenecks are, we can design and add a
filter that solves the right problem.

-- 
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 |