linux-kernel - Re: [PATCH net-next v1 2/7] net: ethtool: add support for structured PHY statistics

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: <Z1GV2cY_LFH7mcAn@shell.armlinux.org.uk>
Date: Thu, 5 Dec 2024 12:00:25 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Jonathan Corbet <corbet@....net>, kernel@...gutronix.de,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	Simon Horman <horms@...nel.org>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	linux-doc@...r.kernel.org
Subject: Re: [PATCH net-next v1 2/7] net: ethtool: add support for structured
 PHY statistics

On Tue, Dec 03, 2024 at 08:56:16AM +0100, Oleksij Rempel wrote:
> From: Jakub Kicinski <kuba@...nel.org>
> 
> Introduce a new way to report PHY statistics in a structured and
> standardized format using the netlink API. This new method does not
> replace the old driver-specific stats, which can still be accessed with
> `ethtool -S <eth name>`. The structured stats are available with
> `ethtool -S <eth name> --all-groups`.
> 
> This new method makes it easier to diagnose problems by organizing stats
> in a consistent and documented way.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
...
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 523195c724b5..20a0d43ab5d4 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1097,7 +1097,8 @@ struct phy_driver {
>  	 * must only set statistics which are actually collected by the device.
>  	 */
>  	void (*get_phy_stats)(struct phy_device *dev,
> -			      struct ethtool_eth_phy_stats *eth_stats);
> +			      struct ethtool_eth_phy_stats *eth_stats,
> +			      struct ethtool_phy_stats *stats);

So we introduce a new function pointer in patch 1, and then change its
prototype in patch 2... wouldn't it be better to introduce the new
structure in patch 1, and then the function pointer in the next patch?

Thanks.

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