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

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Sat, Jun 25, 2022 at 01:03:17AM +0300, Vladimir Oltean wrote:
> 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?

Yes, it will be interesting to know how to proceed with it. For example
KSZ switch do count pause frame Bytes together will other frames. At
same time, atheros switch do not count pause frame bytes at all.

To make things worse, i can manually send pause frame of any size, so it
will not be accounted by HW. What ever decision we will made, i will
need to calculate typical pause frame size and hope it will fit for 90%
of cases.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |