Re: [PATCH net-next v1 2/3] net: dsa: ar9331: add support for pause stats

From: Vladimir Oltean <olteanv@gmail.com>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Woojung Huh <woojung.huh@microchip.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	kernel@pengutronix.de, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, Lukas Wunner <lukas@wunner.de>,
	UNGLinuxDriver@microchip.com
Subject: Re: [PATCH net-next v1 2/3] net: dsa: ar9331: add support for pause stats
Date: Sat, 25 Jun 2022 01:03:17 +0300	[thread overview]
Message-ID: <20220624220317.ckhx6z7cmzegvoqi@skbuf> (raw)
In-Reply-To: <20220624125902.4068436-2-o.rempel@pengutronix.de>

On Fri, Jun 24, 2022 at 02:59:01PM +0200, Oleksij Rempel wrote:
> Add support for pause stats and fix rx_packets/tx_packets calculation.
> 
> Pause packets are counted by raw.rx64byte/raw.tx64byte counters, so
> subtract it from main rx_packets/tx_packets counters.
> 
> tx_/rx_bytes are not affected.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/dsa/qca/ar9331.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
> index fb3fe74abfe6..82412f54c432 100644
> --- a/drivers/net/dsa/qca/ar9331.c
> +++ b/drivers/net/dsa/qca/ar9331.c
> @@ -606,6 +607,7 @@ static void ar9331_sw_phylink_mac_link_up(struct dsa_switch *ds, int port,
>  static void ar9331_read_stats(struct ar9331_sw_port *port)
>  {
>  	struct ar9331_sw_priv *priv = ar9331_sw_port_to_priv(port);
> +	struct ethtool_pause_stats *pstats = &port->pause_stats;
>  	struct rtnl_link_stats64 *stats = &port->stats;
>  	struct ar9331_sw_stats_raw raw;
>  	int ret;
> @@ -625,9 +627,11 @@ static void ar9331_read_stats(struct ar9331_sw_port *port)
>  	stats->tx_bytes += raw.txbyte;
>  
>  	stats->rx_packets += raw.rx64byte + raw.rx128byte + raw.rx256byte +
> -		raw.rx512byte + raw.rx1024byte + raw.rx1518byte + raw.rxmaxbyte;
> +		raw.rx512byte + raw.rx1024byte + raw.rx1518byte +
> +		raw.rxmaxbyte - raw.rxpause;
>  	stats->tx_packets += raw.tx64byte + raw.tx128byte + raw.tx256byte +
> -		raw.tx512byte + raw.tx1024byte + raw.tx1518byte + raw.txmaxbyte;
> +		raw.tx512byte + raw.tx1024byte + raw.tx1518byte +
> +		raw.txmaxbyte - raw.txpause;

Is there an authoritative source who is able to tell whether rtnl_link_stats64 ::
rx_packets and tx_packets should count PAUSE frames or not?

>  
>  	stats->rx_length_errors += raw.rxrunt + raw.rxfragment + raw.rxtoolong;
>  	stats->rx_crc_errors += raw.rxfcserr;
> @@ -646,6 +650,9 @@ static void ar9331_read_stats(struct ar9331_sw_port *port)
>  	stats->multicast += raw.rxmulti;
>  	stats->collisions += raw.txcollision;
>  
> +	pstats->tx_pause_frames += raw.txpause;
> +	pstats->rx_pause_frames += raw.rxpause;
> +
>  	spin_unlock(&port->stats_lock);
>  }