[PATCH net-next 0/3] netdevsim: Support for L3 HW stats

[PATCH net-next 0/3] netdevsim: Support for L3 HW stats

[Petr Machata]

"L3 stats" is a suite of interface statistics aimed at reflecting traffic
taking place in a HW device, on an object corresponding to some software
netdevice. Support for this stats suite has been added recently, in commit
ca0a53dcec94 ("Merge branch 'net-hw-counters-for-soft-devices'").

In this patch set:

- Patch #1 adds support for L3 stats to netdevsim.

  Real devices can have various conditions for when an L3 counter is
  available. To simulate this, netdevsim maintains a list of devices
  suitable for HW stats collection. Only when l3_stats is enabled on both a
  netdevice itself, and in netdevsim, will netdevsim contribute values to
  L3 stats.

  This enablement and disablement is done via debugfs:

    # echo $ifindex > /sys/kernel/debug/netdevsim/$DEV/hwstats/l3/enable_ifindex
    # echo $ifindex > /sys/kernel/debug/netdevsim/$DEV/hwstats/l3/disable_ifindex

  Besides this, there is a third toggle to mark a device for future failure:

    # echo $ifindex > /sys/kernel/debug/netdevsim/$DEV/hwstats/l3/fail_next_enable

- This allows HW-independent testing of stats reporting and in-kernel APIs,
  as well as a test for enablement rollback, which is difficult to do
  otherwise. This netdevsim-specific selftest is added in patch #2.

- Patch #3 adds another driver-specific selftest, namely a test aimed at
  checking mlxsw-induced stats monitoring events.

Petr Machata (3):
  netdevsim: Introduce support for L3 offload xstats
  selftests: netdevsim: hw_stats_l3: Add a new test
  selftests: mlxsw: hw_stats_l3: Add a new test

 drivers/net/netdevsim/Makefile                |   2 +-
 drivers/net/netdevsim/dev.c                   |  17 +-
 drivers/net/netdevsim/hwstats.c               | 497 ++++++++++++++++++
 drivers/net/netdevsim/netdevsim.h             |  23 +
 .../drivers/net/mlxsw/hw_stats_l3.sh          |  31 ++
 .../drivers/net/netdevsim/hw_stats_l3.sh      | 421 +++++++++++++++
 tools/testing/selftests/net/forwarding/lib.sh |  60 +++
 7 files changed, 1048 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/netdevsim/hwstats.c
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/hw_stats_l3.sh
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/hw_stats_l3.sh

-- 
2.31.1

[PATCH net-next 1/3] netdevsim: Introduce support for L3 offload xstats

[Petr Machata]

Add support for testing of HW stats support that was added recently, namely
the L3 stats support. L3 stats are provided for devices for which the L3
stats have been turned on, and that were enabled for netdevsim through a
debugfs toggle:

    # echo $ifindex > /sys/kernel/debug/netdevsim/$DEV/hwstats/l3/enable_ifindex

For fully enabled netdevices, netdevsim counts 10pps of ingress traffic and
20pps of egress traffic. Similarly, L3 stats can be disabled for a given
device, and netdevsim ceases pretending there is any HW traffic going on:

    # echo $ifindex > /sys/kernel/debug/netdevsim/$DEV/hwstats/l3/disable_ifindex

Besides this, there is a third toggle to mark a device for future failure:

    # echo $ifindex > /sys/kernel/debug/netdevsim/$DEV/hwstats/l3/fail_next_enable

A future request to enable L3 stats on such netdevice will be bounced by
netdevsim:

    # ip -j l sh dev d | jq '.[].ifindex'
    66
    # echo 66 > /sys/kernel/debug/netdevsim/netdevsim10/hwstats/l3/enable_ifindex
    # echo 66 > /sys/kernel/debug/netdevsim/netdevsim10/hwstats/l3/fail_next_enable
    # ip stats set dev d l3_stats on
    Error: netdevsim: Stats enablement set to fail.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/netdevsim/Makefile    |   2 +-
 drivers/net/netdevsim/dev.c       |  17 +-
 drivers/net/netdevsim/hwstats.c   | 497 ++++++++++++++++++++++++++++++
 drivers/net/netdevsim/netdevsim.h |  23 ++
 4 files changed, 536 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/netdevsim/hwstats.c

diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
index a1cbfa44a1e1..5735e5b1a2cb 100644
--- a/drivers/net/netdevsim/Makefile
+++ b/drivers/net/netdevsim/Makefile
@@ -3,7 +3,7 @@
 obj-$(CONFIG_NETDEVSIM) += netdevsim.o
 
 netdevsim-objs := \
-	netdev.o dev.o ethtool.o fib.o bus.o health.o udp_tunnels.o
+	netdev.o dev.o ethtool.o fib.o bus.o health.o hwstats.o udp_tunnels.o
 
 ifeq ($(CONFIG_BPF_SYSCALL),y)
 netdevsim-objs += \
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 08d7b465a0de..dbc8e88d2841 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1498,10 +1498,14 @@ static int nsim_dev_reload_create(struct nsim_dev *nsim_dev,
 	if (err)
 		goto err_health_exit;
 
-	err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
+	err = nsim_dev_hwstats_init(nsim_dev);
 	if (err)
 		goto err_psample_exit;
 
+	err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
+	if (err)
+		goto err_hwstats_exit;
+
 	nsim_dev->take_snapshot = debugfs_create_file("take_snapshot",
 						      0200,
 						      nsim_dev->ddir,
@@ -1509,6 +1513,8 @@ static int nsim_dev_reload_create(struct nsim_dev *nsim_dev,
 						&nsim_dev_take_snapshot_fops);
 	return 0;
 
+err_hwstats_exit:
+	nsim_dev_hwstats_exit(nsim_dev);
 err_psample_exit:
 	nsim_dev_psample_exit(nsim_dev);
 err_health_exit:
@@ -1595,15 +1601,21 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
 	if (err)
 		goto err_bpf_dev_exit;
 
-	err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
+	err = nsim_dev_hwstats_init(nsim_dev);
 	if (err)
 		goto err_psample_exit;
 
+	err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
+	if (err)
+		goto err_hwstats_exit;
+
 	nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_LEGACY;
 	devlink_set_features(devlink, DEVLINK_F_RELOAD);
 	devlink_register(devlink);
 	return 0;
 
+err_hwstats_exit:
+	nsim_dev_hwstats_exit(nsim_dev);
 err_psample_exit:
 	nsim_dev_psample_exit(nsim_dev);
 err_bpf_dev_exit:
@@ -1648,6 +1660,7 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
 	mutex_unlock(&nsim_dev->vfs_lock);
 
 	nsim_dev_port_del_all(nsim_dev);
+	nsim_dev_hwstats_exit(nsim_dev);
 	nsim_dev_psample_exit(nsim_dev);
 	nsim_dev_health_exit(nsim_dev);
 	nsim_fib_destroy(devlink, nsim_dev->fib_data);
diff --git a/drivers/net/netdevsim/hwstats.c b/drivers/net/netdevsim/hwstats.c
new file mode 100644
index 000000000000..a4245273e81f
--- /dev/null
+++ b/drivers/net/netdevsim/hwstats.c
@@ -0,0 +1,497 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/debugfs.h>
+
+#include "netdevsim.h"
+
+#define NSIM_DEV_HWSTATS_TRAFFIC_MS	100
+
+static struct list_head *
+nsim_dev_hwstats_get_list_head(struct nsim_dev_hwstats *hwstats,
+			       enum netdev_offload_xstats_type type)
+{
+	switch (type) {
+	case NETDEV_OFFLOAD_XSTATS_TYPE_L3:
+		return &hwstats->l3_list;
+	}
+
+	WARN_ON_ONCE(1);
+	return NULL;
+}
+
+static void nsim_dev_hwstats_traffic_bump(struct nsim_dev_hwstats *hwstats,
+					  enum netdev_offload_xstats_type type)
+{
+	struct nsim_dev_hwstats_netdev *hwsdev;
+	struct list_head *hwsdev_list;
+
+	hwsdev_list = nsim_dev_hwstats_get_list_head(hwstats, type);
+	if (WARN_ON(!hwsdev_list))
+		return;
+
+	list_for_each_entry(hwsdev, hwsdev_list, list) {
+		if (hwsdev->enabled) {
+			hwsdev->stats.rx_packets += 1;
+			hwsdev->stats.tx_packets += 2;
+			hwsdev->stats.rx_bytes += 100;
+			hwsdev->stats.tx_bytes += 300;
+		}
+	}
+}
+
+static void nsim_dev_hwstats_traffic_work(struct work_struct *work)
+{
+	struct nsim_dev_hwstats *hwstats;
+
+	hwstats = container_of(work, struct nsim_dev_hwstats, traffic_dw.work);
+	mutex_lock(&hwstats->hwsdev_list_lock);
+	nsim_dev_hwstats_traffic_bump(hwstats, NETDEV_OFFLOAD_XSTATS_TYPE_L3);
+	mutex_unlock(&hwstats->hwsdev_list_lock);
+
+	schedule_delayed_work(&hwstats->traffic_dw,
+			      msecs_to_jiffies(NSIM_DEV_HWSTATS_TRAFFIC_MS));
+}
+
+static struct nsim_dev_hwstats_netdev *
+nsim_dev_hwslist_find_hwsdev(struct list_head *hwsdev_list,
+			     int ifindex)
+{
+	struct nsim_dev_hwstats_netdev *hwsdev;
+
+	list_for_each_entry(hwsdev, hwsdev_list, list) {
+		if (hwsdev->netdev->ifindex == ifindex)
+			return hwsdev;
+	}
+
+	return NULL;
+}
+
+static int nsim_dev_hwsdev_enable(struct nsim_dev_hwstats_netdev *hwsdev,
+				  struct netlink_ext_ack *extack)
+{
+	if (hwsdev->fail_enable) {
+		hwsdev->fail_enable = false;
+		NL_SET_ERR_MSG_MOD(extack, "Stats enablement set to fail");
+		return -ECANCELED;
+	}
+
+	hwsdev->enabled = true;
+	return 0;
+}
+
+static void nsim_dev_hwsdev_disable(struct nsim_dev_hwstats_netdev *hwsdev)
+{
+	hwsdev->enabled = false;
+	memset(&hwsdev->stats, 0, sizeof(hwsdev->stats));
+}
+
+static int
+nsim_dev_hwsdev_report_delta(struct nsim_dev_hwstats_netdev *hwsdev,
+			     struct netdev_notifier_offload_xstats_info *info)
+{
+	netdev_offload_xstats_report_delta(info->report_delta, &hwsdev->stats);
+	memset(&hwsdev->stats, 0, sizeof(hwsdev->stats));
+	return 0;
+}
+
+static void
+nsim_dev_hwsdev_report_used(struct nsim_dev_hwstats_netdev *hwsdev,
+			    struct netdev_notifier_offload_xstats_info *info)
+{
+	if (hwsdev->enabled)
+		netdev_offload_xstats_report_used(info->report_used);
+}
+
+static int nsim_dev_hwstats_event_off_xstats(struct nsim_dev_hwstats *hwstats,
+					     struct net_device *dev,
+					     unsigned long event, void *ptr)
+{
+	struct netdev_notifier_offload_xstats_info *info;
+	struct nsim_dev_hwstats_netdev *hwsdev;
+	struct list_head *hwsdev_list;
+	int err = 0;
+
+	info = ptr;
+	hwsdev_list = nsim_dev_hwstats_get_list_head(hwstats, info->type);
+	if (!hwsdev_list)
+		return 0;
+
+	mutex_lock(&hwstats->hwsdev_list_lock);
+
+	hwsdev = nsim_dev_hwslist_find_hwsdev(hwsdev_list, dev->ifindex);
+	if (!hwsdev)
+		goto out;
+
+	switch (event) {
+	case NETDEV_OFFLOAD_XSTATS_ENABLE:
+		err = nsim_dev_hwsdev_enable(hwsdev, info->info.extack);
+		break;
+	case NETDEV_OFFLOAD_XSTATS_DISABLE:
+		nsim_dev_hwsdev_disable(hwsdev);
+		break;
+	case NETDEV_OFFLOAD_XSTATS_REPORT_USED:
+		nsim_dev_hwsdev_report_used(hwsdev, info);
+		break;
+	case NETDEV_OFFLOAD_XSTATS_REPORT_DELTA:
+		err = nsim_dev_hwsdev_report_delta(hwsdev, info);
+		break;
+	}
+
+out:
+	mutex_unlock(&hwstats->hwsdev_list_lock);
+	return err;
+}
+
+static void nsim_dev_hwsdev_fini(struct nsim_dev_hwstats_netdev *hwsdev)
+{
+	dev_put(hwsdev->netdev);
+	kfree(hwsdev);
+}
+
+static void
+__nsim_dev_hwstats_event_unregister(struct nsim_dev_hwstats *hwstats,
+				    struct net_device *dev,
+				    enum netdev_offload_xstats_type type)
+{
+	struct nsim_dev_hwstats_netdev *hwsdev;
+	struct list_head *hwsdev_list;
+
+	hwsdev_list = nsim_dev_hwstats_get_list_head(hwstats, type);
+	if (WARN_ON(!hwsdev_list))
+		return;
+
+	hwsdev = nsim_dev_hwslist_find_hwsdev(hwsdev_list, dev->ifindex);
+	if (!hwsdev)
+		return;
+
+	list_del(&hwsdev->list);
+	nsim_dev_hwsdev_fini(hwsdev);
+}
+
+static void nsim_dev_hwstats_event_unregister(struct nsim_dev_hwstats *hwstats,
+					      struct net_device *dev)
+{
+	mutex_lock(&hwstats->hwsdev_list_lock);
+	__nsim_dev_hwstats_event_unregister(hwstats, dev,
+					    NETDEV_OFFLOAD_XSTATS_TYPE_L3);
+	mutex_unlock(&hwstats->hwsdev_list_lock);
+}
+
+static int nsim_dev_hwstats_event(struct nsim_dev_hwstats *hwstats,
+				  struct net_device *dev,
+				  unsigned long event, void *ptr)
+{
+	switch (event) {
+	case NETDEV_OFFLOAD_XSTATS_ENABLE:
+	case NETDEV_OFFLOAD_XSTATS_DISABLE:
+	case NETDEV_OFFLOAD_XSTATS_REPORT_USED:
+	case NETDEV_OFFLOAD_XSTATS_REPORT_DELTA:
+		return nsim_dev_hwstats_event_off_xstats(hwstats, dev,
+							 event, ptr);
+	case NETDEV_UNREGISTER:
+		nsim_dev_hwstats_event_unregister(hwstats, dev);
+		break;
+	}
+
+	return 0;
+}
+
+static int nsim_dev_netdevice_event(struct notifier_block *nb,
+				    unsigned long event, void *ptr)
+{
+	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+	struct nsim_dev_hwstats *hwstats;
+	int err = 0;
+
+	hwstats = container_of(nb, struct nsim_dev_hwstats, netdevice_nb);
+	err = nsim_dev_hwstats_event(hwstats, dev, event, ptr);
+	if (err)
+		return notifier_from_errno(err);
+
+	return NOTIFY_OK;
+}
+
+static int
+nsim_dev_hwstats_enable_ifindex(struct nsim_dev_hwstats *hwstats,
+				int ifindex,
+				enum netdev_offload_xstats_type type,
+				struct list_head *hwsdev_list)
+{
+	struct nsim_dev_hwstats_netdev *hwsdev;
+	struct nsim_dev *nsim_dev;
+	struct net_device *netdev;
+	bool notify = false;
+	struct net *net;
+	int err = 0;
+
+	nsim_dev = container_of(hwstats, struct nsim_dev, hwstats);
+	net = nsim_dev_net(nsim_dev);
+
+	rtnl_lock();
+	mutex_lock(&hwstats->hwsdev_list_lock);
+	hwsdev = nsim_dev_hwslist_find_hwsdev(hwsdev_list, ifindex);
+	if (hwsdev)
+		goto out_unlock_list;
+
+	netdev = dev_get_by_index(net, ifindex);
+	if (!netdev) {
+		err = -ENODEV;
+		goto out_unlock_list;
+	}
+
+	hwsdev = kzalloc(sizeof(*hwsdev), GFP_KERNEL);
+	if (!hwsdev) {
+		err = -ENOMEM;
+		goto out_put_netdev;
+	}
+
+	hwsdev->netdev = netdev;
+	list_add_tail(&hwsdev->list, hwsdev_list);
+	mutex_unlock(&hwstats->hwsdev_list_lock);
+
+	if (netdev_offload_xstats_enabled(netdev, type)) {
+		nsim_dev_hwsdev_enable(hwsdev, NULL);
+		notify = true;
+	}
+
+	if (notify)
+		rtnl_offload_xstats_notify(netdev);
+	rtnl_unlock();
+	return err;
+
+out_put_netdev:
+	dev_put(netdev);
+out_unlock_list:
+	mutex_unlock(&hwstats->hwsdev_list_lock);
+	rtnl_unlock();
+	return err;
+}
+
+static int
+nsim_dev_hwstats_disable_ifindex(struct nsim_dev_hwstats *hwstats,
+				 int ifindex,
+				 enum netdev_offload_xstats_type type,
+				 struct list_head *hwsdev_list)
+{
+	struct nsim_dev_hwstats_netdev *hwsdev;
+	int err = 0;
+
+	rtnl_lock();
+	mutex_lock(&hwstats->hwsdev_list_lock);
+	hwsdev = nsim_dev_hwslist_find_hwsdev(hwsdev_list, ifindex);
+	if (hwsdev)
+		list_del(&hwsdev->list);
+	mutex_unlock(&hwstats->hwsdev_list_lock);
+
+	if (!hwsdev) {
+		err = -ENOENT;
+		goto unlock_out;
+	}
+
+	if (netdev_offload_xstats_enabled(hwsdev->netdev, type)) {
+		netdev_offload_xstats_push_delta(hwsdev->netdev, type,
+						 &hwsdev->stats);
+		rtnl_offload_xstats_notify(hwsdev->netdev);
+	}
+	nsim_dev_hwsdev_fini(hwsdev);
+
+unlock_out:
+	rtnl_unlock();
+	return err;
+}
+
+static int
+nsim_dev_hwstats_fail_ifindex(struct nsim_dev_hwstats *hwstats,
+			      int ifindex,
+			      enum netdev_offload_xstats_type type,
+			      struct list_head *hwsdev_list)
+{
+	struct nsim_dev_hwstats_netdev *hwsdev;
+	int err = 0;
+
+	mutex_lock(&hwstats->hwsdev_list_lock);
+
+	hwsdev = nsim_dev_hwslist_find_hwsdev(hwsdev_list, ifindex);
+	if (!hwsdev) {
+		err = -ENOENT;
+		goto err_hwsdev_list_unlock;
+	}
+
+	hwsdev->fail_enable = true;
+
+err_hwsdev_list_unlock:
+	mutex_unlock(&hwstats->hwsdev_list_lock);
+	return err;
+}
+
+enum nsim_dev_hwstats_do {
+	NSIM_DEV_HWSTATS_DO_DISABLE,
+	NSIM_DEV_HWSTATS_DO_ENABLE,
+	NSIM_DEV_HWSTATS_DO_FAIL,
+};
+
+static ssize_t
+nsim_dev_hwstats_do_write(struct file *file,
+			  const char __user *data,
+			  size_t count, loff_t *ppos,
+			  enum nsim_dev_hwstats_do action,
+			  enum netdev_offload_xstats_type type)
+{
+	struct nsim_dev_hwstats *hwstats = file->private_data;
+	struct list_head *hwsdev_list;
+	int ifindex;
+	int err;
+
+	err = kstrtoint_from_user(data, count, 0, &ifindex);
+	if (err)
+		return err;
+
+	hwsdev_list = nsim_dev_hwstats_get_list_head(hwstats, type);
+	if (WARN_ON(!hwsdev_list))
+		return -EINVAL;
+
+	switch (action) {
+	case NSIM_DEV_HWSTATS_DO_DISABLE:
+		err = nsim_dev_hwstats_disable_ifindex(hwstats, ifindex,
+						       type, hwsdev_list);
+		break;
+	case NSIM_DEV_HWSTATS_DO_ENABLE:
+		err = nsim_dev_hwstats_enable_ifindex(hwstats, ifindex,
+						      type, hwsdev_list);
+		break;
+	case NSIM_DEV_HWSTATS_DO_FAIL:
+		err = nsim_dev_hwstats_fail_ifindex(hwstats, ifindex,
+						    type, hwsdev_list);
+		break;
+	}
+	if (err)
+		return err;
+
+	return count;
+}
+
+static ssize_t nsim_dev_hwstats_l3_enable_write(struct file *file,
+						const char __user *data,
+						size_t count, loff_t *ppos)
+{
+	return nsim_dev_hwstats_do_write(file, data, count, ppos,
+					 NSIM_DEV_HWSTATS_DO_ENABLE,
+					 NETDEV_OFFLOAD_XSTATS_TYPE_L3);
+}
+
+static const struct file_operations nsim_dev_hwstats_l3_enable_fops = {
+	.open = simple_open,
+	.write = nsim_dev_hwstats_l3_enable_write,
+	.llseek = generic_file_llseek,
+	.owner = THIS_MODULE,
+};
+
+static ssize_t nsim_dev_hwstats_l3_disable_write(struct file *file,
+						 const char __user *data,
+						 size_t count, loff_t *ppos)
+{
+	return nsim_dev_hwstats_do_write(file, data, count, ppos,
+					 NSIM_DEV_HWSTATS_DO_DISABLE,
+					 NETDEV_OFFLOAD_XSTATS_TYPE_L3);
+}
+
+static const struct file_operations nsim_dev_hwstats_l3_disable_fops = {
+	.open = simple_open,
+	.write = nsim_dev_hwstats_l3_disable_write,
+	.llseek = generic_file_llseek,
+	.owner = THIS_MODULE,
+};
+
+static ssize_t nsim_dev_hwstats_l3_fail_write(struct file *file,
+					      const char __user *data,
+					      size_t count, loff_t *ppos)
+{
+	return nsim_dev_hwstats_do_write(file, data, count, ppos,
+					 NSIM_DEV_HWSTATS_DO_FAIL,
+					 NETDEV_OFFLOAD_XSTATS_TYPE_L3);
+}
+
+static const struct file_operations nsim_dev_hwstats_l3_fail_fops = {
+	.open = simple_open,
+	.write = nsim_dev_hwstats_l3_fail_write,
+	.llseek = generic_file_llseek,
+	.owner = THIS_MODULE,
+};
+
+int nsim_dev_hwstats_init(struct nsim_dev *nsim_dev)
+{
+	struct nsim_dev_hwstats *hwstats = &nsim_dev->hwstats;
+	struct net *net = nsim_dev_net(nsim_dev);
+	int err;
+
+	mutex_init(&hwstats->hwsdev_list_lock);
+	INIT_LIST_HEAD(&hwstats->l3_list);
+
+	hwstats->netdevice_nb.notifier_call = nsim_dev_netdevice_event;
+	err = register_netdevice_notifier_net(net, &hwstats->netdevice_nb);
+	if (err)
+		goto err_mutex_destroy;
+
+	hwstats->ddir = debugfs_create_dir("hwstats", nsim_dev->ddir);
+	if (IS_ERR(hwstats->ddir)) {
+		err = PTR_ERR(hwstats->ddir);
+		goto err_unregister_notifier;
+	}
+
+	hwstats->l3_ddir = debugfs_create_dir("l3", hwstats->ddir);
+	if (IS_ERR(hwstats->l3_ddir)) {
+		err = PTR_ERR(hwstats->l3_ddir);
+		goto err_remove_hwstats_recursive;
+	}
+
+	debugfs_create_file("enable_ifindex", 0600, hwstats->l3_ddir, hwstats,
+			    &nsim_dev_hwstats_l3_enable_fops);
+	debugfs_create_file("disable_ifindex", 0600, hwstats->l3_ddir, hwstats,
+			    &nsim_dev_hwstats_l3_disable_fops);
+	debugfs_create_file("fail_next_enable", 0600, hwstats->l3_ddir, hwstats,
+			    &nsim_dev_hwstats_l3_fail_fops);
+
+	INIT_DELAYED_WORK(&hwstats->traffic_dw,
+			  &nsim_dev_hwstats_traffic_work);
+	schedule_delayed_work(&hwstats->traffic_dw,
+			      msecs_to_jiffies(NSIM_DEV_HWSTATS_TRAFFIC_MS));
+	return 0;
+
+err_remove_hwstats_recursive:
+	debugfs_remove_recursive(hwstats->ddir);
+err_unregister_notifier:
+	unregister_netdevice_notifier_net(net, &hwstats->netdevice_nb);
+err_mutex_destroy:
+	mutex_destroy(&hwstats->hwsdev_list_lock);
+	return err;
+}
+
+static void nsim_dev_hwsdev_list_wipe(struct nsim_dev_hwstats *hwstats,
+				      enum netdev_offload_xstats_type type)
+{
+	struct nsim_dev_hwstats_netdev *hwsdev, *tmp;
+	struct list_head *hwsdev_list;
+
+	hwsdev_list = nsim_dev_hwstats_get_list_head(hwstats, type);
+	if (WARN_ON(!hwsdev_list))
+		return;
+
+	mutex_lock(&hwstats->hwsdev_list_lock);
+	list_for_each_entry_safe(hwsdev, tmp, hwsdev_list, list) {
+		list_del(&hwsdev->list);
+		nsim_dev_hwsdev_fini(hwsdev);
+	}
+	mutex_unlock(&hwstats->hwsdev_list_lock);
+}
+
+void nsim_dev_hwstats_exit(struct nsim_dev *nsim_dev)
+{
+	struct nsim_dev_hwstats *hwstats = &nsim_dev->hwstats;
+	struct net *net = nsim_dev_net(nsim_dev);
+
+	cancel_delayed_work_sync(&hwstats->traffic_dw);
+	debugfs_remove_recursive(hwstats->ddir);
+	unregister_netdevice_notifier_net(net, &hwstats->netdevice_nb);
+	nsim_dev_hwsdev_list_wipe(hwstats, NETDEV_OFFLOAD_XSTATS_TYPE_L3);
+	mutex_destroy(&hwstats->hwsdev_list_lock);
+}
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index c49771f27f17..128f229d9b4d 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -184,6 +184,28 @@ struct nsim_dev_health {
 int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink);
 void nsim_dev_health_exit(struct nsim_dev *nsim_dev);
 
+struct nsim_dev_hwstats_netdev {
+	struct list_head list;
+	struct net_device *netdev;
+	struct rtnl_hw_stats64 stats;
+	bool enabled;
+	bool fail_enable;
+};
+
+struct nsim_dev_hwstats {
+	struct dentry *ddir;
+	struct dentry *l3_ddir;
+
+	struct mutex hwsdev_list_lock; /* protects hwsdev list(s) */
+	struct list_head l3_list;
+
+	struct notifier_block netdevice_nb;
+	struct delayed_work traffic_dw;
+};
+
+int nsim_dev_hwstats_init(struct nsim_dev *nsim_dev);
+void nsim_dev_hwstats_exit(struct nsim_dev *nsim_dev);
+
 #if IS_ENABLED(CONFIG_PSAMPLE)
 int nsim_dev_psample_init(struct nsim_dev *nsim_dev);
 void nsim_dev_psample_exit(struct nsim_dev *nsim_dev);
@@ -261,6 +283,7 @@ struct nsim_dev {
 	bool fail_reload;
 	struct devlink_region *dummy_region;
 	struct nsim_dev_health health;
+	struct nsim_dev_hwstats hwstats;
 	struct flow_action_cookie *fa_cookie;
 	spinlock_t fa_cookie_lock; /* protects fa_cookie */
 	bool fail_trap_group_set;
-- 
2.31.1

[PATCH net-next 2/3] selftests: netdevsim: hw_stats_l3: Add a new test

[Petr Machata]

Add a test that verifies basic UAPI contracts, netdevsim operation,
rollbacks after partial enablement in core, and UAPI notifications.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 .../drivers/net/netdevsim/hw_stats_l3.sh      | 421 ++++++++++++++++++
 tools/testing/selftests/net/forwarding/lib.sh |  60 +++
 2 files changed, 481 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/hw_stats_l3.sh

diff --git a/tools/testing/selftests/drivers/net/netdevsim/hw_stats_l3.sh b/tools/testing/selftests/drivers/net/netdevsim/hw_stats_l3.sh
new file mode 100755
index 000000000000..fe1898402987
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/hw_stats_l3.sh
@@ -0,0 +1,421 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+lib_dir=$(dirname $0)/../../../net/forwarding
+
+ALL_TESTS="
+	l3_reporting_test
+	l3_fail_next_test
+	l3_counter_test
+	l3_rollback_test
+	l3_monitor_test
+"
+
+NETDEVSIM_PATH=/sys/bus/netdevsim/
+DEV_ADDR_1=1337
+DEV_ADDR_2=1057
+DEV_ADDR_3=5417
+NUM_NETIFS=0
+source $lib_dir/lib.sh
+
+DUMMY_IFINDEX=
+
+DEV_ADDR()
+{
+	local n=$1; shift
+	local var=DEV_ADDR_$n
+
+	echo ${!var}
+}
+
+DEV()
+{
+	echo netdevsim$(DEV_ADDR $1)
+}
+
+DEVLINK_DEV()
+{
+	echo netdevsim/$(DEV $1)
+}
+
+SYSFS_NET_DIR()
+{
+	echo /sys/bus/netdevsim/devices/$(DEV $1)/net/
+}
+
+DEBUGFS_DIR()
+{
+	echo /sys/kernel/debug/netdevsim/$(DEV $1)/
+}
+
+nsim_add()
+{
+	local n=$1; shift
+
+	echo "$(DEV_ADDR $n) 1" > ${NETDEVSIM_PATH}/new_device
+	while [ ! -d $(SYSFS_NET_DIR $n) ] ; do :; done
+}
+
+nsim_reload()
+{
+	local n=$1; shift
+	local ns=$1; shift
+
+	devlink dev reload $(DEVLINK_DEV $n) netns $ns
+
+	if [ $? -ne 0 ]; then
+		echo "Failed to reload $(DEV $n) into netns \"testns1\""
+		exit 1
+	fi
+
+}
+
+nsim_del()
+{
+	local n=$1; shift
+
+	echo "$(DEV_ADDR $n)" > ${NETDEVSIM_PATH}/del_device
+}
+
+nsim_hwstats_toggle()
+{
+	local action=$1; shift
+	local instance=$1; shift
+	local netdev=$1; shift
+	local type=$1; shift
+
+	local ifindex=$($IP -j link show dev $netdev | jq '.[].ifindex')
+
+	echo $ifindex > $(DEBUGFS_DIR $instance)/hwstats/$type/$action
+}
+
+nsim_hwstats_enable()
+{
+	nsim_hwstats_toggle enable_ifindex "$@"
+}
+
+nsim_hwstats_disable()
+{
+	nsim_hwstats_toggle disable_ifindex "$@"
+}
+
+nsim_hwstats_fail_next_enable()
+{
+	nsim_hwstats_toggle fail_next_enable "$@"
+}
+
+setup_prepare()
+{
+	modprobe netdevsim &> /dev/null
+	nsim_add 1
+	nsim_add 2
+	nsim_add 3
+
+	ip netns add testns1
+
+	if [ $? -ne 0 ]; then
+		echo "Failed to add netns \"testns1\""
+		exit 1
+	fi
+
+	nsim_reload 1 testns1
+	nsim_reload 2 testns1
+	nsim_reload 3 testns1
+
+	IP="ip -n testns1"
+
+	$IP link add name dummy1 type dummy
+	$IP link set dev dummy1 up
+	DUMMY_IFINDEX=$($IP -j link show dev dummy1 | jq '.[].ifindex')
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	$IP link del name dummy1
+	ip netns del testns1
+	nsim_del 3
+	nsim_del 2
+	nsim_del 1
+	modprobe -r netdevsim &> /dev/null
+}
+
+netdev_hwstats_used()
+{
+	local netdev=$1; shift
+	local type=$1; shift
+
+	$IP -j stats show dev "$netdev" group offload subgroup hw_stats_info |
+	    jq '.[].info.l3_stats.used'
+}
+
+netdev_check_used()
+{
+	local netdev=$1; shift
+	local type=$1; shift
+
+	[[ $(netdev_hwstats_used $netdev $type) == "true" ]]
+}
+
+netdev_check_unused()
+{
+	local netdev=$1; shift
+	local type=$1; shift
+
+	[[ $(netdev_hwstats_used $netdev $type) == "false" ]]
+}
+
+netdev_hwstats_request()
+{
+	local netdev=$1; shift
+	local type=$1; shift
+
+	$IP -j stats show dev "$netdev" group offload subgroup hw_stats_info |
+	    jq ".[].info.${type}_stats.request"
+}
+
+netdev_check_requested()
+{
+	local netdev=$1; shift
+	local type=$1; shift
+
+	[[ $(netdev_hwstats_request $netdev $type) == "true" ]]
+}
+
+netdev_check_unrequested()
+{
+	local netdev=$1; shift
+	local type=$1; shift
+
+	[[ $(netdev_hwstats_request $netdev $type) == "false" ]]
+}
+
+reporting_test()
+{
+	local type=$1; shift
+	local instance=1
+
+	RET=0
+
+	[[ -n $(netdev_hwstats_used dummy1 $type) ]]
+	check_err $? "$type stats not reported"
+
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used before either device or netdevsim request"
+
+	nsim_hwstats_enable $instance dummy1 $type
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used before device request"
+	netdev_check_unrequested dummy1 $type
+	check_err $? "$type stats reported as requested before device request"
+
+	$IP stats set dev dummy1 ${type}_stats on
+	netdev_check_used dummy1 $type
+	check_err $? "$type stats reported as not used after both device and netdevsim request"
+	netdev_check_requested dummy1 $type
+	check_err $? "$type stats reported as not requested after device request"
+
+	nsim_hwstats_disable $instance dummy1 $type
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used after netdevsim request withdrawn"
+
+	nsim_hwstats_enable $instance dummy1 $type
+	netdev_check_used dummy1 $type
+	check_err $? "$type stats reported as not used after netdevsim request reenabled"
+
+	$IP stats set dev dummy1 ${type}_stats off
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used after device request withdrawn"
+	netdev_check_unrequested dummy1 $type
+	check_err $? "$type stats reported as requested after device request withdrawn"
+
+	nsim_hwstats_disable $instance dummy1 $type
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used after both requests withdrawn"
+
+	log_test "Reporting of $type stats usage"
+}
+
+l3_reporting_test()
+{
+	reporting_test l3
+}
+
+__fail_next_test()
+{
+	local instance=$1; shift
+	local type=$1; shift
+
+	RET=0
+
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used before either device or netdevsim request"
+
+	nsim_hwstats_enable $instance dummy1 $type
+	nsim_hwstats_fail_next_enable $instance dummy1 $type
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used before device request"
+	netdev_check_unrequested dummy1 $type
+	check_err $? "$type stats reported as requested before device request"
+
+	$IP stats set dev dummy1 ${type}_stats on 2>/dev/null
+	check_fail $? "$type stats request not bounced as it should have been"
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used after bounce"
+	netdev_check_unrequested dummy1 $type
+	check_err $? "$type stats reported as requested after bounce"
+
+	$IP stats set dev dummy1 ${type}_stats on
+	check_err $? "$type stats request failed when it shouldn't have"
+	netdev_check_used dummy1 $type
+	check_err $? "$type stats reported as not used after both device and netdevsim request"
+	netdev_check_requested dummy1 $type
+	check_err $? "$type stats reported as not requested after device request"
+
+	$IP stats set dev dummy1 ${type}_stats off
+	nsim_hwstats_disable $instance dummy1 $type
+
+	log_test "Injected failure of $type stats enablement (netdevsim #$instance)"
+}
+
+fail_next_test()
+{
+	__fail_next_test 1 "$@"
+	__fail_next_test 2 "$@"
+	__fail_next_test 3 "$@"
+}
+
+l3_fail_next_test()
+{
+	fail_next_test l3
+}
+
+get_hwstat()
+{
+	local netdev=$1; shift
+	local type=$1; shift
+	local selector=$1; shift
+
+	$IP -j stats show dev $netdev group offload subgroup ${type}_stats |
+		  jq ".[0].stats64.${selector}"
+}
+
+counter_test()
+{
+	local type=$1; shift
+	local instance=1
+
+	RET=0
+
+	nsim_hwstats_enable $instance dummy1 $type
+	$IP stats set dev dummy1 ${type}_stats on
+	netdev_check_used dummy1 $type
+	check_err $? "$type stats reported as not used after both device and netdevsim request"
+
+	# Netdevsim counts 10pps on ingress. We should see maybe a couple
+	# packets, unless things take a reeealy long time.
+	local pkts=$(get_hwstat dummy1 l3 rx.packets)
+	((pkts < 10))
+	check_err $? "$type stats show >= 10 packets after first enablement"
+
+	sleep 2
+
+	local pkts=$(get_hwstat dummy1 l3 rx.packets)
+	((pkts >= 20))
+	check_err $? "$type stats show < 20 packets after 2s passed"
+
+	$IP stats set dev dummy1 ${type}_stats off
+
+	sleep 2
+
+	$IP stats set dev dummy1 ${type}_stats on
+	local pkts=$(get_hwstat dummy1 l3 rx.packets)
+	((pkts < 10))
+	check_err $? "$type stats show >= 10 packets after second enablement"
+
+	$IP stats set dev dummy1 ${type}_stats off
+	nsim_hwstats_fail_next_enable $instance dummy1 $type
+	$IP stats set dev dummy1 ${type}_stats on 2>/dev/null
+	check_fail $? "$type stats request not bounced as it should have been"
+
+	sleep 2
+
+	$IP stats set dev dummy1 ${type}_stats on
+	local pkts=$(get_hwstat dummy1 l3 rx.packets)
+	((pkts < 10))
+	check_err $? "$type stats show >= 10 packets after post-fail enablement"
+
+	$IP stats set dev dummy1 ${type}_stats off
+
+	log_test "Counter values in $type stats"
+}
+
+l3_counter_test()
+{
+	counter_test l3
+}
+
+rollback_test()
+{
+	local type=$1; shift
+
+	RET=0
+
+	nsim_hwstats_enable 1 dummy1 l3
+	nsim_hwstats_enable 2 dummy1 l3
+	nsim_hwstats_enable 3 dummy1 l3
+
+	# The three netdevsim instances are registered in order of their number
+	# one after another. It is reasonable to expect that whatever
+	# notifications take place hit no. 2 in between hitting nos. 1 and 3,
+	# whatever the actual order. This allows us to test that a fail caused
+	# by no. 2 does not leave the system in a partial state, and rolls
+	# everything back.
+
+	nsim_hwstats_fail_next_enable 2 dummy1 l3
+	$IP stats set dev dummy1 ${type}_stats on 2>/dev/null
+	check_fail $? "$type stats request not bounced as it should have been"
+
+	netdev_check_unused dummy1 $type
+	check_err $? "$type stats reported as used after bounce"
+	netdev_check_unrequested dummy1 $type
+	check_err $? "$type stats reported as requested after bounce"
+
+	sleep 2
+
+	$IP stats set dev dummy1 ${type}_stats on
+	check_err $? "$type stats request not upheld as it should have been"
+
+	local pkts=$(get_hwstat dummy1 l3 rx.packets)
+	((pkts < 10))
+	check_err $? "$type stats show $pkts packets after post-fail enablement"
+
+	$IP stats set dev dummy1 ${type}_stats off
+
+	nsim_hwstats_disable 3 dummy1 l3
+	nsim_hwstats_disable 2 dummy1 l3
+	nsim_hwstats_disable 1 dummy1 l3
+
+	log_test "Failure in $type stats enablement rolled back"
+}
+
+l3_rollback_test()
+{
+	rollback_test l3
+}
+
+l3_monitor_test()
+{
+	hw_stats_monitor_test dummy1 l3		   \
+		"nsim_hwstats_enable 1 dummy1 l3"  \
+		"nsim_hwstats_disable 1 dummy1 l3" \
+		"$IP"
+}
+
+trap cleanup EXIT
+
+setup_prepare
+tests_run
+
+exit $EXIT_STATUS
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 159afc7f0979..664b9ecaf228 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -1498,3 +1498,63 @@ brmcast_check_sg_state()
 		check_err_fail $should_fail $? "Entry $src has blocked flag"
 	done
 }
+
+start_ip_monitor()
+{
+	local mtype=$1; shift
+	local ip=${1-ip}; shift
+
+	# start the monitor in the background
+	tmpfile=`mktemp /var/run/nexthoptestXXX`
+	mpid=`($ip monitor $mtype > $tmpfile & echo $!) 2>/dev/null`
+	sleep 0.2
+	echo "$mpid $tmpfile"
+}
+
+stop_ip_monitor()
+{
+	local mpid=$1; shift
+	local tmpfile=$1; shift
+	local el=$1; shift
+	local what=$1; shift
+
+	sleep 0.2
+	kill $mpid
+	local lines=`grep '^\w' $tmpfile | wc -l`
+	test $lines -eq $el
+	check_err $? "$what: $lines lines of events, expected $el"
+	rm -rf $tmpfile
+}
+
+hw_stats_monitor_test()
+{
+	local dev=$1; shift
+	local type=$1; shift
+	local make_suitable=$1; shift
+	local make_unsuitable=$1; shift
+	local ip=${1-ip}; shift
+
+	RET=0
+
+	# Expect a notification about enablement.
+	local ipmout=$(start_ip_monitor stats "$ip")
+	$ip stats set dev $dev ${type}_stats on
+	stop_ip_monitor $ipmout 1 "${type}_stats enablement"
+
+	# Expect a notification about offload.
+	local ipmout=$(start_ip_monitor stats "$ip")
+	$make_suitable
+	stop_ip_monitor $ipmout 1 "${type}_stats installation"
+
+	# Expect a notification about loss of offload.
+	local ipmout=$(start_ip_monitor stats "$ip")
+	$make_unsuitable
+	stop_ip_monitor $ipmout 1 "${type}_stats deinstallation"
+
+	# Expect a notification about disablement
+	local ipmout=$(start_ip_monitor stats "$ip")
+	$ip stats set dev $dev ${type}_stats off
+	stop_ip_monitor $ipmout 1 "${type}_stats disablement"
+
+	log_test "${type}_stats notifications"
+}
-- 
2.31.1

[PATCH net-next 3/3] selftests: mlxsw: hw_stats_l3: Add a new test

[Petr Machata]

Add a test that verifies that UAPI notifications are emitted, as mlxsw
installs and deinstalls HW counters for the L3 offload xstats.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 .../drivers/net/mlxsw/hw_stats_l3.sh          | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/hw_stats_l3.sh

diff --git a/tools/testing/selftests/drivers/net/mlxsw/hw_stats_l3.sh b/tools/testing/selftests/drivers/net/mlxsw/hw_stats_l3.sh
new file mode 100755
index 000000000000..941ba4c485c9
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/hw_stats_l3.sh
@@ -0,0 +1,31 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+lib_dir=$(dirname $0)/../../../net/forwarding
+
+ALL_TESTS="
+	l3_monitor_test
+"
+NUM_NETIFS=0
+source $lib_dir/lib.sh
+
+swp=$NETIF_NO_CABLE
+
+cleanup()
+{
+	pre_cleanup
+}
+
+l3_monitor_test()
+{
+	hw_stats_monitor_test $swp l3		    \
+		"ip addr add dev $swp 192.0.2.1/28" \
+		"ip addr del dev $swp 192.0.2.1/28"
+}
+
+trap cleanup EXIT
+
+setup_wait
+tests_run
+
+exit $EXIT_STATUS
-- 
2.31.1

Re: [PATCH net-next 1/3] netdevsim: Introduce support for L3 offload xstats

[Jakub Kicinski]

On Thu, 10 Mar 2022 17:12:22 +0100 Petr Machata wrote:
> +static ssize_t nsim_dev_hwstats_l3_enable_write(struct file *file,
> +						const char __user *data,
> +						size_t count, loff_t *ppos)
> +{
> +	return nsim_dev_hwstats_do_write(file, data, count, ppos,
> +					 NSIM_DEV_HWSTATS_DO_ENABLE,
> +					 NETDEV_OFFLOAD_XSTATS_TYPE_L3);
> +}

I think you could avoid having the three wrappers if you kept 
separate fops and added a switch to the write function keying 
on debugfs_real_fops().

With that:

Acked-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH net-next 1/3] netdevsim: Introduce support for L3 offload xstats

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> On Thu, 10 Mar 2022 17:12:22 +0100 Petr Machata wrote:
>> +static ssize_t nsim_dev_hwstats_l3_enable_write(struct file *file,
>> +						const char __user *data,
>> +						size_t count, loff_t *ppos)
>> +{
>> +	return nsim_dev_hwstats_do_write(file, data, count, ppos,
>> +					 NSIM_DEV_HWSTATS_DO_ENABLE,
>> +					 NETDEV_OFFLOAD_XSTATS_TYPE_L3);
>> +}
>
> I think you could avoid having the three wrappers if you kept 
> separate fops and added a switch to the write function keying 
> on debugfs_real_fops().
>
> With that:
>
> Acked-by: Jakub Kicinski <kuba@kernel.org>

How about this?

struct nsim_dev_hwstats_fops {
	const struct file_operations fops;
	enum nsim_dev_hwstats_do action;
	enum netdev_offload_xstats_type type;
};

static ssize_t
nsim_dev_hwstats_do_write(struct file *file,
			  const char __user *data,
			  size_t count, loff_t *ppos)
{
	struct nsim_dev_hwstats *hwstats = file->private_data;
	struct nsim_dev_hwstats_fops *hwsfops;
	struct list_head *hwsdev_list;
	int ifindex;
	int err;

	hwsfops = container_of(debugfs_real_fops(file),
			       struct nsim_dev_hwstats_fops, fops);

	[...]

	switch (hwsfops->action) {
	case NSIM_DEV_HWSTATS_DO_DISABLE:
		err = nsim_dev_hwstats_disable_ifindex(hwstats, ifindex,
						       hwsfops->type,
						       hwsdev_list);
		break;
	[...]
}

const struct file_operations nsim_dev_hwstats_generic_fops = {
	.open = simple_open,
	.write = nsim_dev_hwstats_do_write,
	.llseek = generic_file_llseek,
	.owner = THIS_MODULE,
};

static const struct nsim_dev_hwstats_fops nsim_dev_hwstats_l3_disable_fops = {
	.fops = nsim_dev_hwstats_generic_fops,
	.action = NSIM_DEV_HWSTATS_DO_DISABLE,
	.type = NETDEV_OFFLOAD_XSTATS_TYPE_L3,
};

static const struct nsim_dev_hwstats_fops nsim_dev_hwstats_l3_enable_fops = {
	.fops = nsim_dev_hwstats_generic_fops,
	.action = NSIM_DEV_HWSTATS_DO_ENABLE,
	.type = NETDEV_OFFLOAD_XSTATS_TYPE_L3,
};

[...]

Re: [PATCH net-next 1/3] netdevsim: Introduce support for L3 offload xstats

[Jakub Kicinski]

On Fri, 11 Mar 2022 10:18:39 +0100 Petr Machata wrote:
> How about this?
> 
> struct nsim_dev_hwstats_fops {
> 	const struct file_operations fops;
> 	enum nsim_dev_hwstats_do action;
> 	enum netdev_offload_xstats_type type;
> };

Yeah, sure that also works. Using fops is relatively common, 
I thought, so:

+static ssize_t
+nsim_dev_hwstats_do_write(struct file *file,
+			  const char __user *data,
+			  size_t count, loff_t *ppos)
+{
+	struct nsim_dev_hwstats *hwstats = file->private_data;
+	struct list_head *hwsdev_list;
+	int ifindex;
+	int err;
+
+	err = kstrtoint_from_user(data, count, 0, &ifindex);
+	if (err)
+		return err;
+
+	hwsdev_list = nsim_dev_hwstats_get_list_head(hwstats, hwsfops->type);
+	if (WARN_ON(!hwsdev_list))
+		return -EINVAL;
+
+	switch (debugfs_real_fops(file)) {
+	case &nsim_dev_hwstats_l3_disable_fops:
+		err = nsim_dev_hwstats_disable_ifindex(hwstats, ifindex,
+						       NSIM_DEV_HWSTATS_DO_DISABLE,
+						       hwsdev_list);
+		break;

etc. would be the shortest version.

I'm okay with your version if you prefer, but the above works, right?
Or am I missing something?

Re: [PATCH net-next 1/3] netdevsim: Introduce support for L3 offload xstats

[Jakub Kicinski]

On Fri, 11 Mar 2022 08:06:31 -0800 Jakub Kicinski wrote:
> I'm okay with your version if you prefer, but the above works, right?
> Or am I missing something?

Ah, you only have one fops now, I should have read the patch more
carefully. Yup, that's also good.

Re: [PATCH net-next 1/3] netdevsim: Introduce support for L3 offload xstats

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> On Fri, 11 Mar 2022 08:06:31 -0800 Jakub Kicinski wrote:
>> I'm okay with your version if you prefer, but the above works, right?
>> Or am I missing something?

I think it would. I mostly wanted to avoid comparing the pointers,
expressing this using an enum feels cleaner.

> Ah, you only have one fops now, I should have read the patch more
> carefully. Yup, that's also good.

Actually no, you do need one fops instance per file. But the fops
themselves point to the same functions each time, so I can have one
"generic" fops struct and just copy it to each instance.