Re: [PATCH net-next v2] r8169: use the extended tally counter available from RTL8125

[Heiner Kallweit <hkallweit1@gmail.com>]

On 10.10.2024 12:50, Heiner Kallweit wrote:
> The new hw stat fields partially duplicate existing fields, but with a
> larger field size now. Use these new fields to reduce the risk of
> overflows. In addition add support for relevant new fields which are
> available from RTL8125 only.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - added code for enabling the extended tally counter
> - included relevant new fields 
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 40 ++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 665105430..71339910b 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -1777,11 +1777,26 @@ static const char rtl8169_gstrings[][ETH_GSTRING_LEN] = {
>  	"tx_underrun",
>  };
>  
> +static const char rtl8125_gstrings[][ETH_GSTRING_LEN] = {
> +	"tx_bytes",
> +	"rx_bytes",
> +	"tx_pause_on",
> +	"tx_pause_off",
> +	"rx_pause_on",
> +	"rx_pause_off",
> +};
> +
>  static int rtl8169_get_sset_count(struct net_device *dev, int sset)
>  {
>  	switch (sset) {
>  	case ETH_SS_STATS:
> -		return ARRAY_SIZE(rtl8169_gstrings);
> +		struct rtl8169_private *tp = netdev_priv(dev);
> +
> +		if (rtl_is_8125(tp))
> +			return ARRAY_SIZE(rtl8169_gstrings) +
> +			       ARRAY_SIZE(rtl8125_gstrings);
> +		else
> +			return ARRAY_SIZE(rtl8169_gstrings);
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -1873,13 +1888,33 @@ static void rtl8169_get_ethtool_stats(struct net_device *dev,
>  	data[10] = le32_to_cpu(counters->rx_multicast);
>  	data[11] = le16_to_cpu(counters->tx_aborted);
>  	data[12] = le16_to_cpu(counters->tx_underrun);
> +
> +	if (rtl_is_8125(tp)) {
> +		data[5] = le32_to_cpu(counters->align_errors32);
> +		data[10] = le64_to_cpu(counters->rx_multicast64);
> +		data[11] = le32_to_cpu(counters->tx_aborted32);
> +		data[12] = le32_to_cpu(counters->tx_underrun32);
> +
> +		data[13] = le64_to_cpu(counters->tx_octets);
> +		data[14] = le64_to_cpu(counters->rx_octets);
> +		data[15] = le32_to_cpu(counters->tx_pause_on);
> +		data[16] = le32_to_cpu(counters->tx_pause_off);
> +		data[17] = le32_to_cpu(counters->rx_pause_on);
> +		data[18] = le32_to_cpu(counters->rx_pause_off);
> +	}
>  }
>  
>  static void rtl8169_get_strings(struct net_device *dev, u32 stringset, u8 *data)
>  {
>  	switch(stringset) {
>  	case ETH_SS_STATS:
> +		struct rtl8169_private *tp = netdev_priv(dev);

patchwork lists the following warning for a clang build:
warning: label followed by a declaration is a C23 extension [-Wc23-extensions]

gcc 14.2.1 however had no problem with this code, and also checkpatch didn't
complain. Is this code acceptable or should it be changed?