On Tue, 2024-04-02 at 19:34 -0700, Jakub Kicinski wrote: > Real devices should implement qstats. Devices which support > pause or FEC configuration should also report the relevant stats. > > nsim was missing FEC stats completely, some of the qstats > and pause stats required toggling a debugfs knob. > > Note that the tests which used pause always initialize the setting > so they shouldn't be affected by the different starting value. > > Signed-off-by: Jakub Kicinski <[email protected]> > --- > drivers/net/netdevsim/ethtool.c | 11 ++++++++ > drivers/net/netdevsim/netdev.c | 45 +++++++++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+) > > diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c > index bd546d4d26c6..3f9c9327f149 100644 > --- a/drivers/net/netdevsim/ethtool.c > +++ b/drivers/net/netdevsim/ethtool.c > @@ -140,6 +140,13 @@ nsim_set_fecparam(struct net_device *dev, struct > ethtool_fecparam *fecparam) > return 0; > } > > +static void > +nsim_get_fec_stats(struct net_device *dev, struct ethtool_fec_stats > *fec_stats) > +{ > + fec_stats->corrected_blocks.total = 123; > + fec_stats->uncorrectable_blocks.total = 4; > +} > + > static int nsim_get_ts_info(struct net_device *dev, > struct ethtool_ts_info *info) > { > @@ -163,6 +170,7 @@ static const struct ethtool_ops nsim_ethtool_ops = { > .set_channels = nsim_set_channels, > .get_fecparam = nsim_get_fecparam, > .set_fecparam = nsim_set_fecparam, > + .get_fec_stats = nsim_get_fec_stats, > .get_ts_info = nsim_get_ts_info, > }; > > @@ -182,6 +190,9 @@ void nsim_ethtool_init(struct netdevsim *ns) > > nsim_ethtool_ring_init(ns); > > + ns->ethtool.pauseparam.report_stats_rx = true; > + ns->ethtool.pauseparam.report_stats_tx = true; > + > ns->ethtool.fec.fec = ETHTOOL_FEC_NONE; > ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE; > > diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c > index 8330bc0bcb7e..096ac0abbc02 100644 > --- a/drivers/net/netdevsim/netdev.c > +++ b/drivers/net/netdevsim/netdev.c > @@ -19,6 +19,7 @@ > #include <linux/module.h> > #include <linux/netdevice.h> > #include <linux/slab.h> > +#include <net/netdev_queues.h> > #include <net/netlink.h> > #include <net/pkt_cls.h> > #include <net/rtnetlink.h> > @@ -330,6 +331,49 @@ static const struct net_device_ops nsim_vf_netdev_ops = { > .ndo_set_features = nsim_set_features, > }; > > +/* We don't have true par-queue stats, yet, so do some random fakery here. */ > +static void nsim_get_queue_stats_rx(struct net_device *dev, int idx, > + struct netdev_queue_stats_rx *stats) > +{ > + struct rtnl_link_stats64 rtstats = {}; > + > + nsim_get_stats64(dev, &rtstats); > + > + stats->packets = rtstats.rx_packets - !!rtstats.rx_packets; > + stats->bytes = rtstats.rx_bytes; > +} > + > +static void nsim_get_queue_stats_tx(struct net_device *dev, int idx, > + struct netdev_queue_stats_tx *stats) > +{ > + struct rtnl_link_stats64 rtstats = {}; > + > + nsim_get_stats64(dev, &rtstats); > + > + stats->packets = rtstats.tx_packets - !!rtstats.tx_packets; > + stats->bytes = rtstats.tx_bytes;
It looks the stats will not be self-consistent with multiple queues enabled. What about zeroing 'stats' when idx > 0 ? /P
