sfc: initial debugfs support

From: Edward Cree <[email protected]>

Just a handful of nodes, including one enum with a string table for
pretty printing the values.

Reviewed-by Jonathan Cooper <[email protected]>

Signed-off-by Edward Cree <[email protected]>

---
  drivers/net/ethernet/sfc/Makefile       |   1 +
  drivers/net/ethernet/sfc/debugfs.c      | 234 ++++++++++++++++++++++++
  drivers/net/ethernet/sfc/debugfs.h      |  56 ++++++
  drivers/net/ethernet/sfc/ef10.c         |  10 +
  drivers/net/ethernet/sfc/ef100_netdev.c |   7 +
  drivers/net/ethernet/sfc/ef100_nic.c    |   8 +
  drivers/net/ethernet/sfc/efx.c          |  15 +-
  drivers/net/ethernet/sfc/efx_common.c   |   7 +
  drivers/net/ethernet/sfc/net_driver.h   |  29 +++
  9 files changed, 366 insertions(+), 1 deletion(-)

create mode 100644 drivers/net/ethernet/sfc/debugfs.c create mode 100644 drivers/net/ethernet/sfc/debugfs.h

drivers/net/ethernet/sfc/Makefile +0 -0

index 8f446b9bd5ee..1fbdd04dc2c5 100644

drivers/net/ethernet/sfc/Makefile +0 -0

  obj-$(CONFIG_SFC)      += sfc.o
 

drivers/net/ethernet/sfc/debugfs.c new file +69 -0

new file mode 100644
index 000000000000..cf800addb4ff
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/****************************************************************************
+ * Driver for Solarflare network controllers and boards
+ * Copyright 2023, Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#include "debugfs.h"
+#include <linux/module.h>
+#include <linux/debugfs.h>
+#include <linux/dcache.h>
+#include <linux/seq_file.h>
+
+/* Maximum length for a name component or symlink target */
+#define EFX_DEBUGFS_NAME_LEN 32
+
+/* Top-level debug directory ([/sys/kernel]/debug/sfc) */
+static struct dentry *efx_debug_root;
+
+/* "cards" directory ([/sys/kernel]/debug/sfc/cards) */
+static struct dentry *efx_debug_cards;
+
+/**
+ * efx_init_debugfs_netdev - create debugfs sym-link for net device
+ * @net_dev:           Net device
+ *
+ * Create sym-link named after @net_dev to the debugfs directories for the
+ * corresponding NIC.  The sym-link must be cleaned up using
+ * efx_fini_debugfs_netdev().
+ *
+ * Return: a negative error code or 0 on success.
+ */
+static int efx_init_debugfs_netdev(struct net_device *net_dev)
+{
+       struct efx_nic *efx = efx_netdev_priv(net_dev);
+       char target[EFX_DEBUGFS_NAME_LEN];
+       char name[EFX_DEBUGFS_NAME_LEN];
+
+       if (snprintf(name, sizeof(name), "nic_%s", net_dev->name) >=
+                       sizeof(name))
+               return -ENAMETOOLONG;
+       if (snprintf(target, sizeof(target), "cards/%s", pci_name(efx->pci_dev))
+           >= sizeof(target))
+               return -ENAMETOOLONG;
+       efx->debug_symlink = debugfs_create_symlink(name,
+                                                   efx_debug_root, target);
+       if (IS_ERR_OR_NULL(efx->debug_symlink))
+               return efx->debug_symlink ? PTR_ERR(efx->debug_symlink) :
+                                           -ENOMEM;
+
+       return 0;
+}
+
+/**
+ * efx_fini_debugfs_netdev - remove debugfs sym-link for net device
+ * @net_dev:           Net device
+ *
+ * Remove sym-link created for @net_dev by efx_init_debugfs_netdev().
+ */
+void efx_fini_debugfs_netdev(struct net_device *net_dev)
+{
+       struct efx_nic *efx = efx_netdev_priv(net_dev);
+
+       debugfs_remove(efx->debug_symlink);
+       efx->debug_symlink = NULL;
+}

drivers/net/ethernet/sfc/debugfs.c new file +155 -0

+
+static int efx_debugfs_enum_read(struct seq_file *s, void *d)
+{
+       struct efx_debugfs_enum_data *data = s->private;
+       u64 value = 0;
+       size_t len;
+
+       len = min(data->vlen, sizeof(value));
+#ifdef BIG_ENDIAN
+       /* If data->value is narrower than u64, we need to copy into the
+        * far end of value, as that's where the low bits live.
+        */
+       memcpy(((void *)&value) + sizeof(value) - len, data->value, len);
+#else
+       memcpy(&value, data->value, len);
+#endif
+       seq_printf(s, "%#llx => %s\n", value,
+                  value < data->max ? data->names[value] : "(invalid)");
+       return 0;
+}
+
+static int efx_debugfs_enum_open(struct inode *inode, struct file *f)
+{
+       struct efx_debugfs_enum_data *data = inode->i_private;
+
+       return single_open(f, efx_debugfs_enum_read, data);
+}
+
+static const struct file_operations efx_debugfs_enum_ops = {
+       .owner = THIS_MODULE,
+       .open = efx_debugfs_enum_open,
+       .release = single_release,
+       .read = seq_read,
+       .llseek = seq_lseek,
+};
+
+static void efx_debugfs_create_enum(const char *name, umode_t mode,
+                                   struct dentry *parent,
+                                   struct efx_debugfs_enum_data *data)
+{
+       debugfs_create_file(name, mode, parent, data, &efx_debugfs_enum_ops);
+}
+
+static const char *const efx_interrupt_mode_names[] = {
+       [EFX_INT_MODE_MSIX]   = "MSI-X",
+       [EFX_INT_MODE_MSI]    = "MSI",
+       [EFX_INT_MODE_LEGACY] = "legacy",
+};
+
+#define EFX_DEBUGFS_EFX(_type, _name)  \
+       debugfs_create_##_type(#_name, 0444, efx->debug_dir, &efx->_name)
+
+/* Create basic debugfs parameter files for an Efx NIC */
+static void efx_init_debugfs_nic_files(struct efx_nic *efx)
+{
+       EFX_DEBUGFS_EFX(x32, rx_dma_len);
+       EFX_DEBUGFS_EFX(x32, rx_buffer_order);
+       EFX_DEBUGFS_EFX(x32, rx_buffer_truesize);
+       efx->debug_interrupt_mode.max = ARRAY_SIZE(efx_interrupt_mode_names);
+       efx->debug_interrupt_mode.names = efx_interrupt_mode_names;
+       efx->debug_interrupt_mode.vlen = sizeof(efx->interrupt_mode);
+       efx->debug_interrupt_mode.value = &efx->interrupt_mode;
+       efx_debugfs_create_enum("interrupt_mode", 0444, efx->debug_dir,
+                               &efx->debug_interrupt_mode);
+}
+
+/**
+ * efx_init_debugfs_nic - create debugfs directory for NIC
+ * @efx:               Efx NIC
+ *
+ * Create debugfs directory containing parameter-files for @efx.
+ * The directory must be cleaned up using efx_fini_debugfs_nic().
+ *
+ * Return: a negative error code or 0 on success.
+ */
+int efx_init_debugfs_nic(struct efx_nic *efx)
+{
+       /* Create directory */
+       efx->debug_dir = debugfs_create_dir(pci_name(efx->pci_dev),
+                                           efx_debug_cards);
+       if (!efx->debug_dir)
+               return -ENOMEM;
+       efx_init_debugfs_nic_files(efx);
+       return 0;
+}
+
+/**
+ * efx_fini_debugfs_nic - remove debugfs directory for NIC
+ * @efx:               Efx NIC
+ *
+ * Remove debugfs directory created for @efx by efx_init_debugfs_nic().
+ */
+void efx_fini_debugfs_nic(struct efx_nic *efx)
+{
+       debugfs_remove_recursive(efx->debug_dir);
+       efx->debug_dir = NULL;
+}
+
+/**
+ * efx_init_debugfs - create debugfs directories for sfc driver
+ *
+ * Create debugfs directories "sfc" and "sfc/cards".  This must be
+ * called before any of the other functions that create debugfs
+ * directories.  The directories must be cleaned up using
+ * efx_fini_debugfs().
+ *
+ * Return: a negative error code or 0 on success.
+ */
+int efx_init_debugfs(void)
+{
+       int rc;
+
+       /* Create top-level directory */
+       efx_debug_root = debugfs_create_dir(KBUILD_MODNAME, NULL);
+       if (!efx_debug_root) {
+               pr_err("debugfs_create_dir %s failed.\n", KBUILD_MODNAME);
+               rc = -ENOMEM;
+               goto err;
+       } else if (IS_ERR(efx_debug_root)) {
+               rc = PTR_ERR(efx_debug_root);
+               pr_err("debugfs_create_dir %s failed, rc=%d.\n",
+                      KBUILD_MODNAME, rc);
+               goto err;
+       }
+
+       /* Create "cards" directory */
+       efx_debug_cards = debugfs_create_dir("cards", efx_debug_root);
+       if (!efx_debug_cards) {
+               pr_err("debugfs_create_dir cards failed.\n");
+               rc = -ENOMEM;
+               goto err;
+       } else if (IS_ERR(efx_debug_cards)) {
+               rc = PTR_ERR(efx_debug_cards);
+               pr_err("debugfs_create_dir cards failed, rc=%d.\n", rc);
+               goto err;
+       }
+
+       return 0;
+
+err:
+       efx_fini_debugfs();
+       return rc;
+}
+
+/**
+ * efx_fini_debugfs - remove debugfs directories for sfc driver
+ *
+ * Remove directories created by efx_init_debugfs().
+ */
+void efx_fini_debugfs(void)
+{
+       debugfs_remove_recursive(efx_debug_root);
+       efx_debug_cards = NULL;
+       efx_debug_root = NULL;
+}

drivers/net/ethernet/sfc/debugfs.h new file +56 -0

new file mode 100644
index 000000000000..1fe40fbffa5e
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/****************************************************************************
+ * Driver for Solarflare network controllers and boards
+ * Copyright 2023, Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#ifndef EFX_DEBUGFS_H
+#define EFX_DEBUGFS_H
+#include "net_driver.h"
+
+#ifdef CONFIG_DEBUG_FS
+
+/**
+ * DOC: Directory layout for sfc debugfs
+ *
+ * At top level ([/sys/kernel]/debug/sfc) are per-netdev symlinks "nic_$name"
+ * and the "cards" directory.  For each PCI device to which the driver has
+ * bound and created a &struct efx_nic, there is a directory &efx_nic.debug_dir
+ * in "cards" whose name is the PCI address of the device; it is to this
+ * directory that the netdev symlink points.
+ */
+
+void efx_fini_debugfs_netdev(struct net_device *net_dev);
+void efx_update_debugfs_netdev(struct efx_nic *efx);
+
+int efx_init_debugfs_nic(struct efx_nic *efx);
+void efx_fini_debugfs_nic(struct efx_nic *efx);
+
+int efx_init_debugfs(void);
+void efx_fini_debugfs(void);
+
+#else /* CONFIG_DEBUG_FS */
+
+static inline void efx_fini_debugfs_netdev(struct net_device *net_dev) {}
+
+static inline void efx_update_debugfs_netdev(struct efx_nic *efx) {}
+
+static inline int efx_init_debugfs_nic(struct efx_nic *efx)
+{
+       return 0;
+}
+static inline void efx_fini_debugfs_nic(struct efx_nic *efx) {}
+
+static inline int efx_init_debugfs(void)
+{
+       return 0;
+}
+static inline void efx_fini_debugfs(void) {}
+
+#endif /* CONFIG_DEBUG_FS */
+
+#endif /* EFX_DEBUGFS_H */

drivers/net/ethernet/sfc/ef10.c +1 -0

index 6dfa062feebc..58e18fc92093 100644
@@ -19,6 +19,7 @@
  #include "workarounds.h"
  #include "selftest.h"
  #include "ef10_sriov.h"
+#include "debugfs.h"
  #include <linux/in.h>
  #include <linux/jhash.h>
  #include <linux/wait.h>

drivers/net/ethernet/sfc/ef10.c +3 -0

+
         rc = device_create_file(&efx->pci_dev->dev,
                                 &dev_attr_link_control_flag);
         if (rc)

@@ -693,6 +701,7 @@ static int efx_ef10_probe(struct efx_nic *efx)
  fail4:
         device_remove_file(&efx->pci_dev->dev, &dev_attr_link_control_flag);
  fail3:
+       efx_fini_debugfs_nic(efx);
         efx_mcdi_detach(efx);
 
         mutex_lock(&nic_data->udp_tunnels_lock);

@@ -962,6 +971,7 @@ static void efx_ef10_remove(struct efx_nic *efx)
         device_remove_file(&efx->pci_dev->dev, &dev_attr_link_control_flag);
 
         efx_mcdi_detach(efx);
+       efx_fini_debugfs_nic(efx);
 
         memset(nic_data->udp_tunnels, 0, sizeof(nic_data->udp_tunnels));
         mutex_lock(&nic_data->udp_tunnels_lock);

drivers/net/ethernet/sfc/ef100_netdev.c +2 -0

index 7f7d560cb2b4..e844d57754b7 100644
@@ -26,10 +26,12 @@
  #include "tc_bindings.h"
  #include "tc_encap_actions.h"
  #include "efx_devlink.h"
+#include "debugfs.h"
 
  static void ef100_update_name(struct efx_nic *efx)
  {
         strcpy(efx->name, efx->net_dev->name);
+       efx_update_debugfs_netdev(efx);
  }
 
  static int ef100_alloc_vis(struct efx_nic *efx, unsigned int *allocated_vis)

drivers/net/ethernet/sfc/ef100_netdev.c +0 -0

         down_write(&efx->filter_sem);
         efx_mcdi_filter_table_remove(efx);

drivers/net/ethernet/sfc/ef100_nic.c +1 -0

index 6da06931187d..ad378aa05dc3 100644
@@ -27,6 +27,7 @@
  #include "tc.h"
  #include "mae.h"
  #include "rx_common.h"
+#include "debugfs.h"
 
  #define EF100_MAX_VIS 4096
  #define EF100_NUM_MCDI_BUFFERS 1

drivers/net/ethernet/sfc/ef100_nic.c +1 -0

         rc = efx_mcdi_init(efx);
         if (rc)
                 goto fail;

@@ -1213,6 +1220,7 @@ void ef100_remove(struct efx_nic *efx)
 
         efx_mcdi_detach(efx);
         efx_mcdi_fini(efx);
+       efx_fini_debugfs_nic(efx);
         if (nic_data)
                 efx_nic_free_buffer(efx, &nic_data->mcdi_buf);
         kfree(nic_data);

drivers/net/ethernet/sfc/efx.c +1 -0

index 19f4b4d0b851..9266c7b5b4fd 100644
@@ -33,6 +33,7 @@
  #include "selftest.h"
  #include "sriov.h"
  #include "efx_devlink.h"
+#include "debugfs.h"
 
  #include "mcdi_port_common.h"
  #include "mcdi_pcol.h"

drivers/net/ethernet/sfc/efx.c +8 -1

  }
 
  /**************************************************************************

@@ -667,6 +673,7 @@ static void efx_update_name(struct efx_nic *efx)
         strcpy(efx->name, efx->net_dev->name);
         efx_mtd_rename(efx);
         efx_set_channel_names(efx);
+       efx_update_debugfs_netdev(efx);
  }
 
  static int efx_netdev_event(struct notifier_block *this,

@@ -1319,6 +1326,10 @@ static int __init efx_init_module(void)
 
         printk(KERN_INFO "Solarflare NET driver\n");
 
+       rc = efx_init_debugfs();
+       if (rc)
+               goto err_debugfs;
+
         rc = register_netdevice_notifier(&efx_netdev_notifier);
         if (rc)
                 goto err_notifier;

@@ -1344,6 +1355,8 @@ static int __init efx_init_module(void)
   err_reset:
         unregister_netdevice_notifier(&efx_netdev_notifier);
   err_notifier:
+       efx_fini_debugfs();
+ err_debugfs:
         return rc;
  }
 

@@ -1355,7 +1368,7 @@ static void __exit efx_exit_module(void)
         pci_unregister_driver(&efx_pci_driver);
         efx_destroy_reset_workqueue();
         unregister_netdevice_notifier(&efx_netdev_notifier);
-
+       efx_fini_debugfs();
  }
 
  module_init(efx_init_module);

drivers/net/ethernet/sfc/efx_common.c +0 -0

index 175bd9cdfdac..7a9d6b6b66e5 100644

drivers/net/ethernet/sfc/efx_common.c +4 -0

         efx->tx_queues_per_channel = 1;
         efx->rxq_entries = EFX_DEFAULT_DMAQ_SIZE;
         efx->txq_entries = EFX_DEFAULT_DMAQ_SIZE;

@@ -1056,6 +1059,10 @@ void efx_fini_struct(struct efx_nic *efx)
 
         efx_fini_channels(efx);
 
+#ifdef CONFIG_DEBUG_FS
+       mutex_destroy(&efx->debugfs_symlink_mutex);
+#endif
+
         kfree(efx->vpd_sn);
 
         if (efx->workqueue) {

drivers/net/ethernet/sfc/net_driver.h +29 -0

index 27d86e90a3bb..961e2db31c6e 100644
@@ -107,6 +107,24 @@ struct hwtstamp_config;
 
  struct efx_self_tests;
 
+/**
+ * struct efx_debugfs_enum_data - information for pretty-printing enums
+ * @value: pointer to the actual enum
+ * @vlen: sizeof the enum
+ * @names: array of names of enumerated values.  May contain some %NULL entries.
+ * @max: number of entries in @names, typically from ARRAY_SIZE()
+ *
+ * Where a driver struct contains an enum member which we wish to expose in
+ * debugfs, we also embed an instance of this struct, which
+ * efx_debugfs_enum_read() uses to pretty-print the value.
+ */
+struct efx_debugfs_enum_data {
+       void *value;
+       size_t vlen;
+       const char *const *names;
+       unsigned int max;
+};
+
  /**
   * struct efx_buffer - A general-purpose DMA buffer
   * @addr: host base address of the buffer

@@ -1123,6 +1141,17 @@ struct efx_nic {
         u32 rps_next_id;
  #endif
 
+#ifdef CONFIG_DEBUG_FS
+       /** @debug_dir: NIC debugfs directory */
+       struct dentry *debug_dir;
+       /** @debug_symlink: NIC debugfs symlink (``nic_eth%d``) */
+       struct dentry *debug_symlink;
+       /** @debug_interrupt_mode: debugfs details for printing @interrupt_mode */
+       struct efx_debugfs_enum_data debug_interrupt_mode;
+       /** @debugfs_symlink_mutex: protects debugfs @debug_symlink */
+       struct mutex debugfs_symlink_mutex;
+#endif
+
         atomic_t active_queues;
         atomic_t rxq_flush_pending;
         atomic_t rxq_flush_outstanding;

It would be nice to have a couple of example paths listed here

drivers/net/ethernet/sfc/debugfs.c line 79

Quoted 9 lines

drivers/net/ethernet/sfc/debugfs.c new file +9 -0

+/* replace debugfs sym-links on net device rename */
+void efx_update_debugfs_netdev(struct efx_nic *efx)
+{
+       mutex_lock(&efx->debugfs_symlink_mutex);
+       if (efx->debug_symlink)
+               efx_fini_debugfs_netdev(efx->net_dev);
+       efx_init_debugfs_netdev(efx->net_dev);
+       mutex_unlock(&efx->debugfs_symlink_mutex);
+}

How necessary is this netdev symlink?  This seems like a bunch of extra maintenance.

drivers/net/ethernet/sfc/ef10.c static int efx_ef10_probe(struct efx_nic *efx) · line 589

Quoted 6 lines

+       /* Populate debugfs */

drivers/net/ethernet/sfc/ef10.c +5 -0

+#ifdef CONFIG_DEBUG_FS
+       rc = efx_init_debugfs_nic(efx);
+       if (rc)
+               pci_err(efx->pci_dev, "failed to init device debugfs\n");
+#endif

I don't think you need the ifdef here because you have the static version defined in debugfs.h

drivers/net/ethernet/sfc/ef100_netdev.c void ef100_remove_netdev(struct efx_probe_data *probe_data) · line 414

Quoted 5 lines

drivers/net/ethernet/sfc/ef100_netdev.c +5 -0

+#ifdef CONFIG_DEBUG_FS
+       mutex_lock(&efx->debugfs_symlink_mutex);
+       efx_fini_debugfs_netdev(efx->net_dev);
+       mutex_unlock(&efx->debugfs_symlink_mutex);
+#endif

Can you do the mutex dance inside of efx_fini_debugfs_netdev() and then not need the ifdef here?

Quoted 8 lines

drivers/net/ethernet/sfc/efx_common.c +0 -0

index 175bd9cdfdac..7a9d6b6b66e5 100644
@@ -1022,6 +1022,9 @@ int efx_init_struct(struct efx_nic *efx, struct pci_dev *pci_dev)
          INIT_WORK(&efx->mac_work, efx_mac_work);
          init_waitqueue_head(&efx->flush_wq);
 

drivers/net/ethernet/sfc/efx_common.c int efx_init_struct(struct efx_nic *efx, struct pci_dev *pci_dev) · line 1027

Quoted 3 lines

drivers/net/ethernet/sfc/efx_common.c +3 -0

+#ifdef CONFIG_DEBUG_FS
+       mutex_init(&efx->debugfs_symlink_mutex);
+#endif

Can we do this without the ifdefs in the mainline code? (okay, I'll stop grinding on that one for now)

From: Edward Cree <[email protected]>

Expose each RX queue's core RXQ association, and the read/write/etc
pointers for the descriptor ring.
Each RXQ dir also symlinks to its owning channel.

Reviewed-by Jonathan Cooper <[email protected]>

Signed-off-by Edward Cree <[email protected]>

---
  drivers/net/ethernet/sfc/debugfs.c    | 69 ++++++++++++++++++++++++++-
  drivers/net/ethernet/sfc/debugfs.h    | 15 ++++++
  drivers/net/ethernet/sfc/net_driver.h |  6 +++
  drivers/net/ethernet/sfc/rx_common.c  |  9 ++++
  4 files changed, 98 insertions(+), 1 deletion(-)

drivers/net/ethernet/sfc/debugfs.c +24 -0

index b46339249794..43b1d06a985e 100644
@@ -78,6 +78,72 @@ void efx_update_debugfs_netdev(struct efx_nic *efx)
         mutex_unlock(&efx->debugfs_symlink_mutex);
  }
 
+#define EFX_DEBUGFS_RXQ(_type, _name)  \
+       debugfs_create_##_type(#_name, 0444, rx_queue->debug_dir, &rx_queue->_name)
+
+/* Create basic debugfs parameter files for an Efx RXQ */
+static void efx_init_debugfs_rx_queue_files(struct efx_rx_queue *rx_queue)
+{
+       EFX_DEBUGFS_RXQ(u32, core_index); /* actually an int */
+       /* descriptor ring indices */
+       EFX_DEBUGFS_RXQ(u32, added_count);
+       EFX_DEBUGFS_RXQ(u32, notified_count);
+       EFX_DEBUGFS_RXQ(u32, granted_count);
+       EFX_DEBUGFS_RXQ(u32, removed_count);
+}
+
+/**
+ * efx_init_debugfs_rx_queue - create debugfs directory for RX queue
+ * @rx_queue:          Efx RX queue
+ *
+ * Create a debugfs directory containing parameter-files for @rx_queue.
+ * The directory must be cleaned up using efx_fini_debugfs_rx_queue(),
+ * even if this function returns an error.
+ *
+ * Return: a negative error code or 0 on success.
+ */

drivers/net/ethernet/sfc/debugfs.c +6 -0

+           >= sizeof(name))
+               return -ENAMETOOLONG;
+       rx_queue->debug_dir = debugfs_create_dir(name,
+                                                rx_queue->efx->debug_queues_dir);
+       if (!rx_queue->debug_dir)
+               return -ENOMEM;

drivers/net/ethernet/sfc/debugfs.c +18 -1

+
+       return 0;
+}
+
+/**
+ * efx_fini_debugfs_rx_queue - remove debugfs directory for RX queue
+ * @rx_queue:          Efx RX queue
+ *
+ * Remove directory created for @rx_queue by efx_init_debugfs_rx_queue().
+ */
+void efx_fini_debugfs_rx_queue(struct efx_rx_queue *rx_queue)
+{
+       debugfs_remove_recursive(rx_queue->debug_dir);
+       rx_queue->debug_dir = NULL;
+}
+
  #define EFX_DEBUGFS_CHANNEL(_type, _name)      \
         debugfs_create_##_type(#_name, 0444, channel->debug_dir, &channel->_name)
 

@@ -208,9 +274,10 @@ int efx_init_debugfs_nic(struct efx_nic *efx)
         if (!efx->debug_dir)
                 return -ENOMEM;
         efx_init_debugfs_nic_files(efx);
-       /* Create channels subdirectory */
+       /* Create subdirectories */
         efx->debug_channels_dir = debugfs_create_dir("channels",
                                                      efx->debug_dir);
+       efx->debug_queues_dir = debugfs_create_dir("queues", efx->debug_dir);
         return 0;
  }
 

drivers/net/ethernet/sfc/debugfs.h +15 -0

index 4af4a03d1b97..53c98a2fb4c9 100644
@@ -28,11 +28,20 @@
   * * "channels/" (&efx_nic.debug_channels_dir).  For each channel, this will
   *   contain a directory (&efx_channel.debug_dir), whose name is the channel
   *   index (in decimal).
+ * * "queues/" (&efx_nic.debug_queues_dir).
+ *
+ *   * For each NIC RX queue, this will contain a directory
+ *     (&efx_rx_queue.debug_dir), whose name is "rx-N" where N is the RX queue
+ *     index.  (This may not be the same as the kernel core RX queue index.)
+ *     The directory will contain a symlink to the owning channel.
   */
 
  void efx_fini_debugfs_netdev(struct net_device *net_dev);
  void efx_update_debugfs_netdev(struct efx_nic *efx);
 
+int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue);
+void efx_fini_debugfs_rx_queue(struct efx_rx_queue *rx_queue);
+
  int efx_init_debugfs_channel(struct efx_channel *channel);
  void efx_fini_debugfs_channel(struct efx_channel *channel);
 

@@ -48,6 +57,12 @@ static inline void efx_fini_debugfs_netdev(struct net_device *net_dev) {}
 
  static inline void efx_update_debugfs_netdev(struct efx_nic *efx) {}
 
+int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue)
+{
+       return 0;
+}
+void efx_fini_debugfs_rx_queue(struct efx_rx_queue *rx_queue) {}
+
  int efx_init_debugfs_channel(struct efx_channel *channel)
  {
         return 0;

drivers/net/ethernet/sfc/net_driver.h +6 -0

index 2b92c5461fe3..63eb32670826 100644
@@ -424,6 +424,10 @@ struct efx_rx_queue {
         struct work_struct grant_work;
         /* Statistics to supplement MAC stats */
         unsigned long rx_packets;
+#ifdef CONFIG_DEBUG_FS
+       /** @debug_dir: Queue debugfs directory (under @efx->debug_queues_dir) */
+       struct dentry *debug_dir;
+#endif
         struct xdp_rxq_info xdp_rxq_info;
         bool xdp_rxq_info_valid;
  };

@@ -1150,6 +1154,8 @@ struct efx_nic {
         struct dentry *debug_dir;
         /** @debug_channels_dir: contains channel debugfs dirs.  Under @debug_dir */
         struct dentry *debug_channels_dir;
+       /** @debug_queues_dir: contains RX/TX queue debugfs dirs.  Under @debug_dir */
+       struct dentry *debug_queues_dir;
         /** @debug_symlink: NIC debugfs symlink (``nic_eth%d``) */
         struct dentry *debug_symlink;
         /** @debug_interrupt_mode: debugfs details for printing @interrupt_mode */

drivers/net/ethernet/sfc/rx_common.c +9 -0

index d2f35ee15eff..7f63f70f082d 100644
@@ -14,6 +14,7 @@
  #include "efx.h"
  #include "nic.h"
  #include "rx_common.h"
+#include "debugfs.h"
 
  /* This is the percentage fill level below which new RX descriptors
   * will be added to the RX descriptor ring.

@@ -208,6 +209,12 @@ int efx_probe_rx_queue(struct efx_rx_queue *rx_queue)
         if (!rx_queue->buffer)
                 return -ENOMEM;
 
+       rc = efx_init_debugfs_rx_queue(rx_queue);
+       if (rc) /* not fatal */
+               netif_err(efx, drv, efx->net_dev,
+                         "Failed to create debugfs for RXQ %d, rc=%d\n",
+                         efx_rx_queue_index(rx_queue), rc);
+
         rc = efx_nic_probe_rx(rx_queue);
         if (rc) {
                 kfree(rx_queue->buffer);

@@ -311,6 +318,8 @@ void efx_remove_rx_queue(struct efx_rx_queue *rx_queue)
 
         efx_nic_remove_rx(rx_queue);
 
+       efx_fini_debugfs_rx_queue(rx_queue);
+
         kfree(rx_queue->buffer);
         rx_queue->buffer = NULL;
  }

Quoted 1 line · On 12/11/2023 9:18 AM, [email protected] wrote:

+       if (snprintf(name, sizeof(name), "rx-%d", efx_rx_queue_index(rx_queue))

Adding leading 0's here can be helpful for directory entry sorting

Quoted 10 lines

+           >= sizeof(name))
+               return -ENAMETOOLONG;
+       rx_queue->debug_dir = debugfs_create_dir(name,
+                                                rx_queue->efx->debug_queues_dir); +       if (!rx_queue->debug_dir)
+               return -ENOMEM;

drivers/net/ethernet/sfc/debugfs.c +4 -0

+
+       /* Create files */
+       efx_init_debugfs_rx_queue_files(rx_queue);
+

Quoted 6 lines

+       /* Create symlink to channel */
+       if (snprintf(target, sizeof(target), "../../channels/%d", +                    channel->channel) >= sizeof(target))
+               return -ENAMETOOLONG;
+       if (!debugfs_create_symlink("channel", rx_queue->debug_dir, target)) +               return -ENOMEM;

If these fail, should you clean up the earlier create_dir()?

Quoted 2 lines

+ * The directory must be cleaned up using efx_fini_debugfs_rx_queue(),
+ * even if this function returns an error.

On 15/12/2023—
wait, has it really been more than a year? Yikes.

Quoted 4 lines · On 12/11/2023 9:18 AM, [email protected] wrote:

Quoted 1 line

+ if (snprintf(name, sizeof(name), "rx-%d", efx_rx_queue_index(rx_queue))

Adding leading 0's here can be helpful for directory entry sorting

True, but it's not clear how many to use — the hardware supports over
1000 in principle, and in practice it's normal to have one per core (and more than that on TX) which can get over 100 on powerful systems.
Yet on something like an 8-core box having queues 000 to 007 just looks
silly imho. I don't plan to change this line in v2.

Quoted 11 lines

Quoted 11 lines

drivers/net/ethernet/sfc/debugfs.c +10 -0

+           >= sizeof(name))
+               return -ENAMETOOLONG;
+       rx_queue->debug_dir = debugfs_create_dir(name,
+                                                rx_queue->efx->debug_queues_dir);
+       if (!rx_queue->debug_dir)
+               return -ENOMEM;
+
+       /* Create files */
+       efx_init_debugfs_rx_queue_files(rx_queue);
+

Quoted 8 lines

drivers/net/ethernet/sfc/debugfs.c void efx_update_debugfs_netdev(struct efx_nic *efx) · line 130

Quoted 6 lines

drivers/net/ethernet/sfc/debugfs.c +6 -0

+       /* Create symlink to channel */
+       if (snprintf(target, sizeof(target), "../../channels/%d",
+                    channel->channel) >= sizeof(target))
+               return -ENAMETOOLONG;
+       if (!debugfs_create_symlink("channel", rx_queue->debug_dir, target))
+               return -ENOMEM;

If these fail, should you clean up the earlier create_dir()?

No; these errors mean "we didn't do everything we wanted to", not
"it's all broken", and the files/dir previously created are still useful. The caller treats errors as non-fatal.
See also the kdoc comment on efx_init_debugfs_rx_queue:

Quoted 2 lines

Quoted 2 lines

+ * The directory must be cleaned up using efx_fini_debugfs_rx_queue(),
+ * even if this function returns an error.

I can't think of a suitable comment on these return statements to
clarify this, but suggestions are welcome.

From: Edward Cree <[email protected]>

Filter table management is complicated by the possibility of overflow
kicking us into a promiscuous fallback for either unicast or multicast. Expose the internal flags that drive this.
Since the table state (efx->filter_state) has a separate, shorter
lifetime than struct efx_nic, put its debugfs nodes in a subdirectory (efx->filter_state->debug_dir) so that they can be cleaned up easily before the filter_state is freed.

Reviewed-by Jonathan Cooper <[email protected]>

Signed-off-by Edward Cree <[email protected]>

index 4ff6586116ee..a4ab45082c8f 100644

drivers/net/ethernet/sfc/mcdi_filters.c +14 -0

@@ -1348,6 +1348,20 @@ int efx_mcdi_filter_table_probe(struct efx_nic *efx, bool multicast_chaining)
 	INIT_LIST_HEAD(&table->vlan_list);
 	init_rwsem(&table->lock);
 
+#ifdef CONFIG_DEBUG_FS
+	table->debug_dir = debugfs_create_dir("filters", efx->debug_dir);
+	debugfs_create_bool("uc_promisc", 0444, table->debug_dir,
+			    &table->uc_promisc);
+	debugfs_create_bool("mc_promisc", 0444, table->debug_dir,
+			    &table->mc_promisc);
+	debugfs_create_bool("mc_promisc_last", 0444, table->debug_dir,
+			    &table->mc_promisc_last);
+	debugfs_create_bool("mc_overflow", 0444, table->debug_dir,
+			    &table->mc_overflow);
+	debugfs_create_bool("mc_chaining", 0444, table->debug_dir,
+			    &table->mc_chaining);
+#endif
+
 	efx->filter_state = table;
 
 	return 0;

From: Edward Cree <[email protected]>

Filter table management is complicated by the possibility of overflow
kicking us into a promiscuous fallback for either unicast or multicast. Expose the internal flags that drive this.
Since the table state (efx->filter_state) has a separate, shorter
lifetime than struct efx_nic, put its debugfs nodes in a subdirectory (efx->filter_state->debug_dir) so that they can be cleaned up easily before the filter_state is freed.

Reviewed-by Jonathan Cooper <[email protected]>

Signed-off-by Edward Cree <[email protected]>

---
  drivers/net/ethernet/sfc/debugfs.h      |  4 ++++
  drivers/net/ethernet/sfc/mcdi_filters.c | 18 ++++++++++++++++++
  drivers/net/ethernet/sfc/mcdi_filters.h |  4 ++++
  3 files changed, 26 insertions(+)

drivers/net/ethernet/sfc/debugfs.h +4 -0

index 3e8d2e2b5bad..7a96f3798cbd 100644
@@ -39,6 +39,10 @@
   *     index.  (This may differ from both the kernel core TX queue index and
   *     the hardware queue label of the TXQ.)
   *     The directory will contain a symlink to the owning channel.
+ *
+ * * "filters/" (&efx_mcdi_filter_table.debug_dir).
+ *   This contains parameter files for the NIC receive filter table
+ *   (@efx->filter_state).
   */
 
  void efx_fini_debugfs_netdev(struct net_device *net_dev);

drivers/net/ethernet/sfc/mcdi_filters.c +3 -0

index 4ff6586116ee..a4ab45082c8f 100644
@@ -1348,6 +1348,20 @@ int efx_mcdi_filter_table_probe(struct efx_nic *efx, bool multicast_chaining)
         INIT_LIST_HEAD(&table->vlan_list);
         init_rwsem(&table->lock);
 
+#ifdef CONFIG_DEBUG_FS
+       table->debug_dir = debugfs_create_dir("filters", efx->debug_dir);
+       debugfs_create_bool("uc_promisc", 0444, table->debug_dir,

drivers/net/ethernet/sfc/mcdi_filters.c +5 -0

+
         efx->filter_state = table;
 
         return 0;

@@ -1518,6 +1532,10 @@ void efx_mcdi_filter_table_remove(struct efx_nic *efx)
                 return;
 
         vfree(table->entry);
+#ifdef CONFIG_DEBUG_FS
+       /* Remove debugfs entries pointing into @table */
+       debugfs_remove_recursive(table->debug_dir);
+#endif
         kfree(table);
  }
 

drivers/net/ethernet/sfc/mcdi_filters.h +4 -0

index c0d6558b9fd2..897843ade3ec 100644
@@ -91,6 +91,10 @@ struct efx_mcdi_filter_table {
         bool vlan_filter;
         /* Entries on the vlan_list are added/removed under filter_sem */
         struct list_head vlan_list;
+#ifdef CONFIG_DEBUG_FS
+       /* filter table debugfs directory */
+       struct dentry *debug_dir;
+#endif
  };
 
  int efx_mcdi_filter_table_probe(struct efx_nic *efx, bool multicast_chaining);

drivers/net/ethernet/sfc/mcdi_filters.c int efx_mcdi_filter_table_probe(struct efx_nic *efx, bool multicast_chaining) · line 1363

Quoted 13 lines · On 12/11/2023 9:18 AM, [email protected] wrote:

drivers/net/ethernet/sfc/mcdi_filters.c +13 -0

+#ifdef CONFIG_DEBUG_FS
+       table->debug_dir = debugfs_create_dir("filters", efx->debug_dir);
+       debugfs_create_bool("uc_promisc", 0444, table->debug_dir,
+                           &table->uc_promisc);
+       debugfs_create_bool("mc_promisc", 0444, table->debug_dir,
+                           &table->mc_promisc);
+       debugfs_create_bool("mc_promisc_last", 0444, table->debug_dir,
+                           &table->mc_promisc_last);
+       debugfs_create_bool("mc_overflow", 0444, table->debug_dir,
+                           &table->mc_overflow);
+       debugfs_create_bool("mc_chaining", 0444, table->debug_dir,
+                           &table->mc_chaining);
+#endif

It would be good to continue the practice of using the debugfs_* primitives in your debugfs.c and just make a single call here that doesn't need the ifdef

Quoted 16 lines · Shannon

drivers/net/ethernet/sfc/mcdi_filters.c int efx_mcdi_filter_table_probe(struct efx_nic *efx, bool multicast_chaining) · line 1363

Quoted 13 lines · On 12/11/2023 9:18 AM, [email protected] wrote:

drivers/net/ethernet/sfc/mcdi_filters.c +13 -0

+#ifdef CONFIG_DEBUG_FS
+       table->debug_dir = debugfs_create_dir("filters", efx->debug_dir);
+       debugfs_create_bool("uc_promisc", 0444, table->debug_dir,
+                           &table->uc_promisc);
+       debugfs_create_bool("mc_promisc", 0444, table->debug_dir,
+                           &table->mc_promisc);
+       debugfs_create_bool("mc_promisc_last", 0444, table->debug_dir,
+                           &table->mc_promisc_last);
+       debugfs_create_bool("mc_overflow", 0444, table->debug_dir,
+                           &table->mc_overflow);
+       debugfs_create_bool("mc_chaining", 0444, table->debug_dir,
+                           &table->mc_chaining);
+#endif

It would be good to continue the practice of using the debugfs_* primitives in your debugfs.c and just make a single call here that doesn't need the ifdef

I'm still in two minds about this. While it makes sense in isolation
to do it that way here, in the following patch we add a more complex dumper, and I think it makes more sense to put that in mcdi_filters.c and have filters code know a bit about debugfs, than put it in debugfs.c and have debug code know *everything* about filters — and every other part of the driver that subsequently gets its own debug nodes.
I already have some follow-up patches that add EF100 MAE debugfs nodes
which also have custom dumpers, but those are in a separate file (tc_debugfs.c) because there are a lot of them and tc/mae code is already split into several pieces, whereas I'm not sure whether adding a separate file for filter-table debugfs code really makes sense, or whether it's sufficient just to refactor this code into a(n unconditionally-called) function that continues to live in mcdi_filters.c and has the ifdef within it.

From: Edward Cree <[email protected]>

Expose the filter table entries.

Reviewed-by Jonathan Cooper <[email protected]>

Signed-off-by Edward Cree <[email protected]>

drivers/net/ethernet/sfc/debugfs.h +0 -0

drivers/net/ethernet/sfc/debugfs.h +7 -0

@@ -63,6 +67,45 @@ void efx_fini_debugfs_nic(struct efx_nic *efx);
 int efx_init_debugfs(void);
 void efx_fini_debugfs(void);
 
+void efx_debugfs_print_filter(char *s, size_t l, struct efx_filter_spec *spec);
+
+/* Generate operations for a debugfs node with a custom reader function.
+ * The reader should have signature int (*)(struct seq_file *s, void *data)
+ * where data is the pointer passed to EFX_DEBUGFS_CREATE_RAW.
+ */
+#define EFX_DEBUGFS_RAW_PARAMETER(_reader)				       \

Quoted 11 lines · On Mon, Dec 11, 2023 at 05:18:32PM +0000, [email protected] wrote:

drivers/net/ethernet/sfc/debugfs.h +7 -0

@@ -63,6 +67,45 @@ void efx_fini_debugfs_nic(struct efx_nic *efx);
 int efx_init_debugfs(void);
 void efx_fini_debugfs(void);
 
+void efx_debugfs_print_filter(char *s, size_t l, struct efx_filter_spec *spec);
+
+/* Generate operations for a debugfs node with a custom reader function.
+ * The reader should have signature int (*)(struct seq_file *s, void *data)
+ * where data is the pointer passed to EFX_DEBUGFS_CREATE_RAW.
+ */
+#define EFX_DEBUGFS_RAW_PARAMETER(_reader)				       \

drivers/net/ethernet/sfc/debugfs.h void efx_fini_debugfs_nic(struct efx_nic *efx); · line 86

Quoted 10 lines

drivers/net/ethernet/sfc/debugfs.h +10 -0

+									       \
+static int efx_debugfs_##_reader##_read(struct seq_file *s, void *d)	       \
+{									       \
+	return _reader(s, s->private);					       \
+}									       \
+									       \
+static int efx_debugfs_##_reader##_open(struct inode *inode, struct file *f)   \
+{									       \
+	return single_open(f, efx_debugfs_##_reader##_read, inode->i_private); \
+}									       \

Hi Edward,

I think that probably the above should be static inline.

Quoted 25 lines · Simon Horman

Quoted 11 lines · On Mon, Dec 11, 2023 at 05:18:32PM +0000, [email protected] wrote:

drivers/net/ethernet/sfc/debugfs.h +7 -0

@@ -63,6 +67,45 @@ void efx_fini_debugfs_nic(struct efx_nic *efx);
 int efx_init_debugfs(void);
 void efx_fini_debugfs(void);
 
+void efx_debugfs_print_filter(char *s, size_t l, struct efx_filter_spec *spec);
+
+/* Generate operations for a debugfs node with a custom reader function.
+ * The reader should have signature int (*)(struct seq_file *s, void *data)
+ * where data is the pointer passed to EFX_DEBUGFS_CREATE_RAW.
+ */
+#define EFX_DEBUGFS_RAW_PARAMETER(_reader)				       \

drivers/net/ethernet/sfc/debugfs.h void efx_fini_debugfs_nic(struct efx_nic *efx); · line 86

Quoted 10 lines

drivers/net/ethernet/sfc/debugfs.h +10 -0

+									       \
+static int efx_debugfs_##_reader##_read(struct seq_file *s, void *d)	       \
+{									       \
+	return _reader(s, s->private);					       \
+}									       \
+									       \
+static int efx_debugfs_##_reader##_open(struct inode *inode, struct file *f)   \
+{									       \
+	return single_open(f, efx_debugfs_##_reader##_read, inode->i_private); \
+}									       \

Hi Edward,

Quoted 1 line

I think that probably the above should be static inline.

Yep, in fact there are instances of this from patch 2 onwards (most
of those aren't even static). Clearly I hadn't had enough sleep the day I wrote this :/

Quoted 2 lines · Edward Cree

Quoted 1 line · Simon Horman

On Mon, Dec 11, 2023 at 05:18:32PM +0000, [email protected] wrote:

[...]

Quoted 8 lines

Quoted 4 lines

Hi Edward,

I think that probably the above should be static inline.

Yep, in fact there are instances of this from patch 2 onwards (most
of those aren't even static). Clearly I hadn't had enough sleep the day I wrote this :/

Or maybe it's *today* I haven't had enough sleep... Unlike the functions in patches 2-4, which are stubs for the
CONFIG_DEBUG_FS=n build, these functions should *not* be "static inline", because they are intended to be referenced from ops structs or passed as callbacks.
The check on patchwork is actually a false positive here, because
this is not a function that's defined in the header file. It's part of the body of a *macro*, EFX_DEBUGFS_RAW_PARAMETER.
Functions are only defined when some C file expands the macro.

I will update the commit message to call out and explain this; I
believe the code is actually fine.

Quoted 2 lines · On Tue, 12 Dec 2023 15:14:17 +0000 Edward Cree wrote:

I will update the commit message to call out and explain this; I
believe the code is actually fine.

Fair point, second time in a ~month we see this sort of false positive. I'll throw [^\\]$ at the end of the regex to try to avoid matching stuff that's most likely a macro.

This one looks legit tho:

+void efx_debugfs_print_filter(char *s, size_t l, struct efx_filter_spec *spec) {}

Quoted 7 lines · Jakub Kicinski

Quoted 2 lines · On Tue, 12 Dec 2023 15:14:17 +0000 Edward Cree wrote:

I will update the commit message to call out and explain this; I
believe the code is actually fine.

Fair point, second time in a ~month we see this sort of false positive. I'll throw [^\\]$ at the end of the regex to try to avoid matching stuff that's most likely a macro.

Sounds good, thanks.

Quoted 3 lines

This one looks legit tho:

+void efx_debugfs_print_filter(char *s, size_t l, struct efx_filter_spec *spec) {}

Yep, that one's real, will be fixed in v2.
And this time I'll actually build-test with CONFIG_DEBUG_FS=n,
which I forgot to do with v1 (sorry).

-ed

From: Edward Cree <[email protected]>

Expose the filter table entries.

Reviewed-by Jonathan Cooper <[email protected]>

Signed-off-by Edward Cree <[email protected]>

---
  drivers/net/ethernet/sfc/debugfs.c      | 117 +++++++++++++++++++++++-
  drivers/net/ethernet/sfc/debugfs.h      |  45 +++++++++
  drivers/net/ethernet/sfc/mcdi_filters.c |  39 ++++++++
  3 files changed, 197 insertions(+), 4 deletions(-)

drivers/net/ethernet/sfc/debugfs.c +0 -0

index 549ff1ee273e..e67b0fc927fe 100644

drivers/net/ethernet/sfc/debugfs.c +92 -0

  /* Maximum length for a name component or symlink target */
  #define EFX_DEBUGFS_NAME_LEN 32

@@ -428,3 +424,116 @@ void efx_fini_debugfs(void)
         efx_debug_cards = NULL;
         efx_debug_root = NULL;
  }
+
+/**
+ * efx_debugfs_print_filter - format a filter spec for display
+ * @s: buffer to write result into
+ * @l: length of buffer @s
+ * @spec: filter specification
+ */
+void efx_debugfs_print_filter(char *s, size_t l, struct efx_filter_spec *spec)
+{
+       u32 ip[4];
+       int p = snprintf(s, l, "match=%#x,pri=%d,flags=%#x,q=%d",
+                        spec->match_flags, spec->priority, spec->flags,
+                        spec->dmaq_id);
+
+       if (spec->vport_id)
+               p += snprintf(s + p, l - p, ",vport=%#x", spec->vport_id);
+
+       if (spec->flags & EFX_FILTER_FLAG_RX_RSS)
+               p += snprintf(s + p, l - p, ",rss=%#x", spec->rss_context);
+
+       if (spec->match_flags & EFX_FILTER_MATCH_OUTER_VID)
+               p += snprintf(s + p, l - p,
+                             ",ovid=%d", ntohs(spec->outer_vid));
+       if (spec->match_flags & EFX_FILTER_MATCH_INNER_VID)
+               p += snprintf(s + p, l - p,
+                             ",ivid=%d", ntohs(spec->inner_vid));
+       if (spec->match_flags & EFX_FILTER_MATCH_ENCAP_TYPE)
+               p += snprintf(s + p, l - p,
+                             ",encap=%d", spec->encap_type);
+       if (spec->match_flags & EFX_FILTER_MATCH_LOC_MAC)
+               p += snprintf(s + p, l - p,
+                             ",lmac=%02x:%02x:%02x:%02x:%02x:%02x",
+                             spec->loc_mac[0], spec->loc_mac[1],
+                             spec->loc_mac[2], spec->loc_mac[3],
+                             spec->loc_mac[4], spec->loc_mac[5]);
+       if (spec->match_flags & EFX_FILTER_MATCH_REM_MAC)
+               p += snprintf(s + p, l - p,
+                             ",rmac=%02x:%02x:%02x:%02x:%02x:%02x",
+                             spec->rem_mac[0], spec->rem_mac[1],
+                             spec->rem_mac[2], spec->rem_mac[3],
+                             spec->rem_mac[4], spec->rem_mac[5]);
+       if (spec->match_flags & EFX_FILTER_MATCH_ETHER_TYPE)
+               p += snprintf(s + p, l - p,
+                             ",ether=%#x", ntohs(spec->ether_type));
+       if (spec->match_flags & EFX_FILTER_MATCH_IP_PROTO)
+               p += snprintf(s + p, l - p,
+                             ",ippr=%#x", spec->ip_proto);
+       if (spec->match_flags & EFX_FILTER_MATCH_LOC_HOST) {
+               if (ntohs(spec->ether_type) == ETH_P_IP) {
+                       ip[0] = (__force u32) spec->loc_host[0];
+                       p += snprintf(s + p, l - p,
+                                     ",lip=%d.%d.%d.%d",
+                                     ip[0] & 0xff,
+                                     (ip[0] >> 8) & 0xff,
+                                     (ip[0] >> 16) & 0xff,
+                                     (ip[0] >> 24) & 0xff);
+               } else if (ntohs(spec->ether_type) == ETH_P_IPV6) {
+                       ip[0] = (__force u32) spec->loc_host[0];
+                       ip[1] = (__force u32) spec->loc_host[1];
+                       ip[2] = (__force u32) spec->loc_host[2];
+                       ip[3] = (__force u32) spec->loc_host[3];
+                       p += snprintf(s + p, l - p,
+                                     ",lip=%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x",
+                                     ip[0] & 0xffff,
+                                     (ip[0] >> 16) & 0xffff,
+                                     ip[1] & 0xffff,
+                                     (ip[1] >> 16) & 0xffff,
+                                     ip[2] & 0xffff,
+                                     (ip[2] >> 16) & 0xffff,
+                                     ip[3] & 0xffff,
+                                     (ip[3] >> 16) & 0xffff);
+               } else {
+                       p += snprintf(s + p, l - p, ",lip=?");
+               }
+       }
+       if (spec->match_flags & EFX_FILTER_MATCH_REM_HOST) {
+               if (ntohs(spec->ether_type) == ETH_P_IP) {
+                       ip[0] = (__force u32) spec->rem_host[0];
+                       p += snprintf(s + p, l - p,
+                                     ",rip=%d.%d.%d.%d",
+                                     ip[0] & 0xff,
+                                     (ip[0] >> 8) & 0xff,
+                                     (ip[0] >> 16) & 0xff,
+                                     (ip[0] >> 24) & 0xff);
+               } else if (ntohs(spec->ether_type) == ETH_P_IPV6) {
+                       ip[0] = (__force u32) spec->rem_host[0];
+                       ip[1] = (__force u32) spec->rem_host[1];
+                       ip[2] = (__force u32) spec->rem_host[2];
+                       ip[3] = (__force u32) spec->rem_host[3];
+                       p += snprintf(s + p, l - p,
+                                     ",rip=%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x",
+                                     ip[0] & 0xffff,

drivers/net/ethernet/sfc/debugfs.c +11 -0

+       }
+       if (spec->match_flags & EFX_FILTER_MATCH_LOC_PORT)
+               p += snprintf(s + p, l - p,
+                             ",lport=%d", ntohs(spec->loc_port));
+       if (spec->match_flags & EFX_FILTER_MATCH_REM_PORT)
+               p += snprintf(s + p, l - p,
+                             ",rport=%d", ntohs(spec->rem_port));
+       if (spec->match_flags & EFX_FILTER_MATCH_LOC_MAC_IG)
+               p += snprintf(s + p, l - p, ",%s",
+                             spec->loc_mac[0] ? "mc" : "uc");
+}

drivers/net/ethernet/sfc/debugfs.h +45 -0

index 7a96f3798cbd..f50b4bf33a6b 100644
@@ -10,6 +10,10 @@
 
  #ifndef EFX_DEBUGFS_H
  #define EFX_DEBUGFS_H
+#include <linux/module.h>
+#include <linux/debugfs.h>
+#include <linux/dcache.h>
+#include <linux/seq_file.h>
  #include "net_driver.h"
 
  #ifdef CONFIG_DEBUG_FS

@@ -63,6 +67,45 @@ void efx_fini_debugfs_nic(struct efx_nic *efx);
  int efx_init_debugfs(void);
  void efx_fini_debugfs(void);
 
+void efx_debugfs_print_filter(char *s, size_t l, struct efx_filter_spec *spec);
+
+/* Generate operations for a debugfs node with a custom reader function.
+ * The reader should have signature int (*)(struct seq_file *s, void *data)
+ * where data is the pointer passed to EFX_DEBUGFS_CREATE_RAW.
+ */
+#define EFX_DEBUGFS_RAW_PARAMETER(_reader)                                    \
+                                                                              \
+static int efx_debugfs_##_reader##_read(struct seq_file *s, void *d)          \
+{                                                                             \
+       return _reader(s, s->private);                                         \
+}                                                                             \
+                                                                              \
+static int efx_debugfs_##_reader##_open(struct inode *inode, struct file *f)   \
+{                                                                             \
+       return single_open(f, efx_debugfs_##_reader##_read, inode->i_private); \
+}                                                                             \
+                                                                              \
+static const struct file_operations efx_debugfs_##_reader##_ops = {           \
+       .owner = THIS_MODULE,                                                  \
+       .open = efx_debugfs_##_reader##_open,                                  \
+       .release = single_release,                                             \
+       .read = seq_read,                                                      \
+       .llseek = seq_lseek,                                                   \
+};                                                                            \
+                                                                              \
+static void efx_debugfs_create_##_reader(const char *name, umode_t mode,       \
+                                        struct dentry *parent, void *data)    \
+{                                                                             \
+       debugfs_create_file(name, mode, parent, data,                          \
+                           &efx_debugfs_##_reader##_ops);                     \
+}
+
+/* Instantiate a debugfs node with a custom reader function.  The operations
+ * must have been generated with EFX_DEBUGFS_RAW_PARAMETER(_reader).
+ */
+#define EFX_DEBUGFS_CREATE_RAW(_name, _mode, _parent, _data, _reader)         \
+               efx_debugfs_create_##_reader(_name, _mode, _parent, _data)
+
  #else /* CONFIG_DEBUG_FS */
 
  static inline void efx_fini_debugfs_netdev(struct net_device *net_dev) {}

@@ -99,6 +142,8 @@ static inline int efx_init_debugfs(void)
  }
  static inline void efx_fini_debugfs(void) {}
 
+void efx_debugfs_print_filter(char *s, size_t l, struct efx_filter_spec *spec) {}
+
  #endif /* CONFIG_DEBUG_FS */
 
  #endif /* EFX_DEBUGFS_H */

drivers/net/ethernet/sfc/mcdi_filters.c +39 -0

index a4ab45082c8f..465226c3e8c7 100644
@@ -13,6 +13,7 @@
  #include "mcdi.h"
  #include "nic.h"
  #include "rx_common.h"
+#include "debugfs.h"
 
  /* The maximum size of a shared RSS context */
  /* TODO: this should really be from the mcdi protocol export */

@@ -1173,6 +1174,42 @@ s32 efx_mcdi_filter_get_rx_ids(struct efx_nic *efx,
         return count;
  }
 
+static int efx_debugfs_read_filter_list(struct seq_file *file, void *data)
+{
+       struct efx_mcdi_filter_table *table;
+       struct efx_nic *efx = data;
+       int i;
+
+       down_read(&efx->filter_sem);
+       table = efx->filter_state;
+       if (!table || !table->entry) {
+               up_read(&efx->filter_sem);
+               return -ENETDOWN;
+       }
+
+       /* deliberately don't lock the table->lock, so that we can
+        * still dump the table even if we hang mid-operation.
+        */
+       for (i = 0; i < EFX_MCDI_FILTER_TBL_ROWS; ++i) {
+               struct efx_filter_spec *spec =
+                       efx_mcdi_filter_entry_spec(table, i);
+               char filter[256];
+
+               if (spec) {
+                       efx_debugfs_print_filter(filter, sizeof(filter), spec);
+
+                       seq_printf(file, "%d[%#04llx],%#x = %s\n",
+                                  i, table->entry[i].handle & 0xffff,
+                                  efx_mcdi_filter_entry_flags(table, i),
+                                  filter);
+               }
+       }
+
+       up_read(&efx->filter_sem);
+       return 0;
+}
+EFX_DEBUGFS_RAW_PARAMETER(efx_debugfs_read_filter_list);
+
  static int efx_mcdi_filter_match_flags_from_mcdi(bool encap, u32 mcdi_flags)
  {
         int match_flags = 0;

@@ -1360,6 +1397,8 @@ int efx_mcdi_filter_table_probe(struct efx_nic *efx, bool multicast_chaining)
                             &table->mc_overflow);
         debugfs_create_bool("mc_chaining", 0444, table->debug_dir,
                             &table->mc_chaining);
+       EFX_DEBUGFS_CREATE_RAW("entries", 0444, table->debug_dir, efx,
+                              efx_debugfs_read_filter_list);
  #endif
 
         efx->filter_state = table;