On 2024-04-01 18:05, 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 <kuba@xxxxxxxxxx>
> ---
> 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. */
nit: per-queue
> +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;
Why subtract !!rtstats.rx_packets? This evaluates to 0 if rx_packets is
0 and 1 if rx_packets is non-zero.
> + 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;
> +}
> +
> +static void nsim_get_base_stats(struct net_device *dev,
> + struct netdev_queue_stats_rx *rx,
> + struct netdev_queue_stats_tx *tx)
> +{
> + struct rtnl_link_stats64 rtstats = {};
> +
> + nsim_get_stats64(dev, &rtstats);
> +
> + rx->packets = !!rtstats.rx_packets;
> + rx->bytes = 0;
> + tx->packets = !!rtstats.tx_packets;
> + tx->bytes = 0;
> +}
> +
> +static const struct netdev_stat_ops nsim_stat_ops = {
> + .get_queue_stats_tx = nsim_get_queue_stats_tx,
> + .get_queue_stats_rx = nsim_get_queue_stats_rx,
> + .get_base_stats = nsim_get_base_stats,
> +};
> +
> static void nsim_setup(struct net_device *dev)
> {
> ether_setup(dev);
> @@ -360,6 +404,7 @@ static int nsim_init_netdevsim(struct netdevsim *ns)
>
> ns->phc = phc;
> ns->netdev->netdev_ops = &nsim_netdev_ops;
> + ns->netdev->stat_ops = &nsim_stat_ops;
>
> err = nsim_udp_tunnels_info_create(ns->nsim_dev, ns->netdev);
> if (err)
