netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2] r8169: use the extended tally counter available from RTL8125
@ 2024-10-10 10:50 Heiner Kallweit
  2024-10-10 20:08 ` Heiner Kallweit
  0 siblings, 1 reply; 3+ messages in thread
From: Heiner Kallweit @ 2024-10-10 10:50 UTC (permalink / raw)
  To: Realtek linux nic maintainers, Paolo Abeni, Jakub Kicinski,
	David Miller, Eric Dumazet
  Cc: netdev@vger.kernel.org

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);
+
 		memcpy(data, rtl8169_gstrings, sizeof(rtl8169_gstrings));
+		if (rtl_is_8125(tp)) {
+			data += sizeof(rtl8169_gstrings);
+			memcpy(data, rtl8125_gstrings, sizeof(rtl8125_gstrings));
+		}
 		break;
 	}
 }
@@ -3894,6 +3929,9 @@ static void rtl_hw_start_8125(struct rtl8169_private *tp)
 		break;
 	}
 
+	/* enable extended tally counter */
+	r8168_mac_ocp_modify(tp, 0xea84, 0, BIT(1) | BIT(0));
+
 	rtl_hw_config(tp);
 }
 
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v2] r8169: use the extended tally counter available from RTL8125
  2024-10-10 10:50 [PATCH net-next v2] r8169: use the extended tally counter available from RTL8125 Heiner Kallweit
@ 2024-10-10 20:08 ` Heiner Kallweit
  2024-10-11  9:55   ` Simon Horman
  0 siblings, 1 reply; 3+ messages in thread
From: Heiner Kallweit @ 2024-10-10 20:08 UTC (permalink / raw)
  To: Realtek linux nic maintainers, Paolo Abeni, Jakub Kicinski,
	David Miller, Eric Dumazet
  Cc: netdev@vger.kernel.org

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?


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v2] r8169: use the extended tally counter available from RTL8125
  2024-10-10 20:08 ` Heiner Kallweit
@ 2024-10-11  9:55   ` Simon Horman
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Horman @ 2024-10-11  9:55 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Realtek linux nic maintainers, Paolo Abeni, Jakub Kicinski,
	David Miller, Eric Dumazet, netdev@vger.kernel.org

On Thu, Oct 10, 2024 at 10:08:02PM +0200, Heiner Kallweit wrote:
> 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>

...

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

Yes, I see that too.
My feeling is that it would be best to change it as it
seems likely that this will break compilation somewhere.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-10-11  9:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-10 10:50 [PATCH net-next v2] r8169: use the extended tally counter available from RTL8125 Heiner Kallweit
2024-10-10 20:08 ` Heiner Kallweit
2024-10-11  9:55   ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).