linux-kernel - Re: [PATCH net-next 1/3] net: phy: qcom: Add PHY counter support

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] [day] [month] [year] [list]

Message-ID: <0e60e901-2284-462e-b853-617dd66f1a30@quicinc.com>
Date: Wed, 9 Jul 2025 13:17:47 +0800
From: Luo Jie <quic_luoj@...cinc.com>
To: Andrew Lunn <andrew@...n.ch>
CC: Heiner Kallweit <hkallweit1@...il.com>,
        Russell King
	<linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni
	<pabeni@...hat.com>, <netdev@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 1/3] net: phy: qcom: Add PHY counter support



On 7/9/2025 12:31 AM, Andrew Lunn wrote:
>> +static const struct qcom_phy_hw_stat qcom_phy_hw_stats[] = {
>> +	{
>> +		.string		= "phy_rx_good_frame",
>> +		.devad		= MDIO_MMD_AN,
>> +		.cnt_31_16_reg	= QCA808X_MMD7_CNT_RX_GOOD_CRC_31_16,
>> +		.cnt_15_0_reg	= QCA808X_MMD7_CNT_RX_GOOD_CRC_15_0,
>> +	},
>> +	{
>> +		.string		= "phy_rx_bad_frame",
>> +		.devad		= MDIO_MMD_AN,
>> +		.cnt_31_16_reg	= 0xffff,
>> +		.cnt_15_0_reg	= QCA808X_MMD7_CNT_RX_BAD_CRC,
>> +	},
>> +	{
>> +		.string		= "phy_tx_good_frame",
>> +		.devad		= MDIO_MMD_AN,
>> +		.cnt_31_16_reg	= QCA808X_MMD7_CNT_TX_GOOD_CRC_31_16,
>> +		.cnt_15_0_reg	= QCA808X_MMD7_CNT_TX_GOOD_CRC_15_0,
>> +	},
>> +	{
>> +		.string		= "phy_tx_bad_frame",
>> +		.devad		= MDIO_MMD_AN,
> 
> Are there any counters which might be added later which are not in
> MDIO_MMD_AN? It seems pointless having this if it is fixed.

Yes, this structure is designed to be extended in the future to support
10G-capable PHY chip, where the counter resides in the MDIO_MMD_PCS
when the link speed is 2500 Mbps or higher. I will add a comment to the
structure to clarify this intent.

> 
>> +		.cnt_31_16_reg	= 0xffff,
>> +		.cnt_15_0_reg	= QCA808X_MMD7_CNT_TX_BAD_CRC,
>> +	},
>> +};
> 
> There has been an attempt to try to standardise PHY statistics. Please
> look at:
> 
> **
>   * struct ethtool_phy_stats - PHY-level statistics counters
>   * @rx_packets: Total successfully received frames
>   * @rx_bytes: Total successfully received bytes
>   * @rx_errors: Total received frames with errors (e.g., CRC errors)
>   * @tx_packets: Total successfully transmitted frames
>   * @tx_bytes: Total successfully transmitted bytes
>   * @tx_errors: Total transmitted frames with errors
>   *
>   * This structure provides a standardized interface for reporting
>   * PHY-level statistics counters. It is designed to expose statistics
>   * commonly provided by PHYs but not explicitly defined in the IEEE
>   * 802.3 standard.
>   */
> struct ethtool_phy_stats {
>          u64 rx_packets;
>          u64 rx_bytes;
>          u64 rx_errors;
>          u64 tx_packets;
>          u64 tx_bytes;
>          u64 tx_errors;
> };
> 
> Please use this if possible.
> 
>      Andrew

Thank you for the suggestion. I will review the standardized PHY
statistics framework and update the implementation accordingly to
ensure alignment.

> 
> ---
> pw-bot: cr