* [PATCH net-next 0/7] sfc: initial debugfs support
@ 2023-12-11 17:18 edward.cree
2023-12-11 17:18 ` [PATCH net-next 1/7] sfc: initial debugfs implementation edward.cree
` (6 more replies)
0 siblings, 7 replies; 23+ messages in thread
From: edward.cree @ 2023-12-11 17:18 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx
From: Edward Cree <ecree.xilinx@gmail.com>
Expose some basic information about the NIC, its channels and queues
and their assignments.
Edward Cree (7):
sfc: initial debugfs implementation
sfc: debugfs for channels
sfc: debugfs for (nic) RX queues
sfc: debugfs for (nic) TX queues
sfc: add debugfs nodes for loopback mode
sfc: add debugfs entries for filter table status
sfc: add debugfs node for filter table contents
drivers/net/ethernet/sfc/Makefile | 1 +
drivers/net/ethernet/sfc/debugfs.c | 539 ++++++++++++++++++++++++
drivers/net/ethernet/sfc/debugfs.h | 149 +++++++
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_channels.c | 8 +
drivers/net/ethernet/sfc/efx_common.c | 7 +
drivers/net/ethernet/sfc/mcdi_filters.c | 57 +++
drivers/net/ethernet/sfc/mcdi_filters.h | 4 +
drivers/net/ethernet/sfc/net_driver.h | 47 +++
drivers/net/ethernet/sfc/rx_common.c | 9 +
drivers/net/ethernet/sfc/tx_common.c | 8 +
14 files changed, 868 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/sfc/debugfs.c
create mode 100644 drivers/net/ethernet/sfc/debugfs.h
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH net-next 1/7] sfc: initial debugfs implementation
2023-12-11 17:18 [PATCH net-next 0/7] sfc: initial debugfs support edward.cree
@ 2023-12-11 17:18 ` edward.cree
2023-12-15 0:05 ` Nelson, Shannon
2023-12-11 17:18 ` [PATCH net-next 2/7] sfc: debugfs for channels edward.cree
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: edward.cree @ 2023-12-11 17:18 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
From: Edward Cree <ecree.xilinx@gmail.com>
Just a handful of nodes, including one enum with a string table for
pretty printing the values.
Reviewed-by: Jonathan Cooper <jonathan.s.cooper@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
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
diff --git a/drivers/net/ethernet/sfc/Makefile b/drivers/net/ethernet/sfc/Makefile
index 8f446b9bd5ee..1fbdd04dc2c5 100644
--- a/drivers/net/ethernet/sfc/Makefile
+++ b/drivers/net/ethernet/sfc/Makefile
@@ -12,6 +12,7 @@ sfc-$(CONFIG_SFC_MTD) += mtd.o
sfc-$(CONFIG_SFC_SRIOV) += sriov.o ef10_sriov.o ef100_sriov.o ef100_rep.o \
mae.o tc.o tc_bindings.o tc_counters.o \
tc_encap_actions.o tc_conntrack.o
+sfc-$(CONFIG_DEBUG_FS) += debugfs.o
obj-$(CONFIG_SFC) += sfc.o
diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
new file mode 100644
index 000000000000..cf800addb4ff
--- /dev/null
+++ b/drivers/net/ethernet/sfc/debugfs.c
@@ -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;
+}
+
+/* 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);
+}
+
+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;
+}
diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
new file mode 100644
index 000000000000..1fe40fbffa5e
--- /dev/null
+++ b/drivers/net/ethernet/sfc/debugfs.h
@@ -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 */
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 6dfa062feebc..58e18fc92093 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -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>
@@ -580,6 +581,13 @@ static int efx_ef10_probe(struct efx_nic *efx)
if (rc)
goto fail3;
+ /* Populate debugfs */
+#ifdef CONFIG_DEBUG_FS
+ rc = efx_init_debugfs_nic(efx);
+ if (rc)
+ pci_err(efx->pci_dev, "failed to init device debugfs\n");
+#endif
+
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);
diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
index 7f7d560cb2b4..e844d57754b7 100644
--- a/drivers/net/ethernet/sfc/ef100_netdev.c
+++ b/drivers/net/ethernet/sfc/ef100_netdev.c
@@ -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)
@@ -405,6 +407,11 @@ void ef100_remove_netdev(struct efx_probe_data *probe_data)
ef100_pf_unset_devlink_port(efx);
efx_fini_tc(efx);
#endif
+#ifdef CONFIG_DEBUG_FS
+ mutex_lock(&efx->debugfs_symlink_mutex);
+ efx_fini_debugfs_netdev(efx->net_dev);
+ mutex_unlock(&efx->debugfs_symlink_mutex);
+#endif
down_write(&efx->filter_sem);
efx_mcdi_filter_table_remove(efx);
diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
index 6da06931187d..ad378aa05dc3 100644
--- a/drivers/net/ethernet/sfc/ef100_nic.c
+++ b/drivers/net/ethernet/sfc/ef100_nic.c
@@ -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
@@ -1077,6 +1078,12 @@ static int ef100_probe_main(struct efx_nic *efx)
/* Post-IO section. */
+ /* Populate debugfs */
+#ifdef CONFIG_DEBUG_FS
+ rc = efx_init_debugfs_nic(efx);
+ if (rc)
+ pci_err(efx->pci_dev, "failed to init device debugfs\n");
+#endif
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);
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 19f4b4d0b851..9266c7b5b4fd 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -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"
@@ -401,6 +402,11 @@ static void efx_remove_all(struct efx_nic *efx)
#endif
efx_remove_port(efx);
efx_remove_nic(efx);
+#ifdef CONFIG_DEBUG_FS
+ mutex_lock(&efx->debugfs_symlink_mutex);
+ efx_fini_debugfs_netdev(efx->net_dev);
+ mutex_unlock(&efx->debugfs_symlink_mutex);
+#endif
}
/**************************************************************************
@@ -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);
diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
index 175bd9cdfdac..7a9d6b6b66e5 100644
--- a/drivers/net/ethernet/sfc/efx_common.c
+++ b/drivers/net/ethernet/sfc/efx_common.c
@@ -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);
+#ifdef CONFIG_DEBUG_FS
+ mutex_init(&efx->debugfs_symlink_mutex);
+#endif
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) {
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 27d86e90a3bb..961e2db31c6e 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -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;
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH net-next 2/7] sfc: debugfs for channels
2023-12-11 17:18 [PATCH net-next 0/7] sfc: initial debugfs support edward.cree
2023-12-11 17:18 ` [PATCH net-next 1/7] sfc: initial debugfs implementation edward.cree
@ 2023-12-11 17:18 ` edward.cree
2023-12-12 19:54 ` kernel test robot
2023-12-11 17:18 ` [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues edward.cree
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: edward.cree @ 2023-12-11 17:18 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
From: Edward Cree <ecree.xilinx@gmail.com>
Expose each channel's IRQ number and EVQ read pointer.
Reviewed-by: Jonathan Cooper <jonathan.s.cooper@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
drivers/net/ethernet/sfc/debugfs.c | 51 +++++++++++++++++++++++++
drivers/net/ethernet/sfc/debugfs.h | 15 ++++++++
drivers/net/ethernet/sfc/efx_channels.c | 8 ++++
drivers/net/ethernet/sfc/net_driver.h | 6 +++
4 files changed, 80 insertions(+)
diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
index cf800addb4ff..b46339249794 100644
--- a/drivers/net/ethernet/sfc/debugfs.c
+++ b/drivers/net/ethernet/sfc/debugfs.c
@@ -78,6 +78,54 @@ void efx_update_debugfs_netdev(struct efx_nic *efx)
mutex_unlock(&efx->debugfs_symlink_mutex);
}
+#define EFX_DEBUGFS_CHANNEL(_type, _name) \
+ debugfs_create_##_type(#_name, 0444, channel->debug_dir, &channel->_name)
+
+/* Create basic debugfs parameter files for an Efx channel */
+static void efx_init_debugfs_channel_files(struct efx_channel *channel)
+{
+ EFX_DEBUGFS_CHANNEL(bool, enabled);
+ EFX_DEBUGFS_CHANNEL(u32, irq); /* actually an int */
+ EFX_DEBUGFS_CHANNEL(u32, eventq_read_ptr);
+}
+
+/**
+ * efx_init_debugfs_channel - create debugfs directory for channel
+ * @channel: Efx channel
+ *
+ * Create a debugfs directory containing parameter-files for @channel.
+ * The directory must be cleaned up using efx_fini_debugfs_channel().
+ *
+ * Return: a negative error code or 0 on success.
+ */
+int efx_init_debugfs_channel(struct efx_channel *channel)
+{
+ char name[EFX_DEBUGFS_NAME_LEN];
+
+ if (!channel->efx->debug_channels_dir)
+ return -ENODEV;
+ if (snprintf(name, sizeof(name), "%d", channel->channel)
+ >= sizeof(name))
+ return -ENAMETOOLONG;
+ channel->debug_dir = debugfs_create_dir(name, channel->efx->debug_channels_dir);
+ if (!channel->debug_dir)
+ return -ENOMEM;
+ efx_init_debugfs_channel_files(channel);
+ return 0;
+}
+
+/**
+ * efx_fini_debugfs_channel - remove debugfs directory for channel
+ * @channel: Efx channel
+ *
+ * Remove directory created for @channel by efx_init_debugfs_channel().
+ */
+void efx_fini_debugfs_channel(struct efx_channel *channel)
+{
+ debugfs_remove_recursive(channel->debug_dir);
+ channel->debug_dir = NULL;
+}
+
static int efx_debugfs_enum_read(struct seq_file *s, void *d)
{
struct efx_debugfs_enum_data *data = s->private;
@@ -160,6 +208,9 @@ int efx_init_debugfs_nic(struct efx_nic *efx)
if (!efx->debug_dir)
return -ENOMEM;
efx_init_debugfs_nic_files(efx);
+ /* Create channels subdirectory */
+ efx->debug_channels_dir = debugfs_create_dir("channels",
+ efx->debug_dir);
return 0;
}
diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
index 1fe40fbffa5e..4af4a03d1b97 100644
--- a/drivers/net/ethernet/sfc/debugfs.h
+++ b/drivers/net/ethernet/sfc/debugfs.h
@@ -22,11 +22,20 @@
* 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.
+ *
+ * Under this directory, besides top-level parameter files, are:
+ *
+ * * "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).
*/
void efx_fini_debugfs_netdev(struct net_device *net_dev);
void efx_update_debugfs_netdev(struct efx_nic *efx);
+int efx_init_debugfs_channel(struct efx_channel *channel);
+void efx_fini_debugfs_channel(struct efx_channel *channel);
+
int efx_init_debugfs_nic(struct efx_nic *efx);
void efx_fini_debugfs_nic(struct efx_nic *efx);
@@ -39,6 +48,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_channel(struct efx_channel *channel)
+{
+ return 0;
+}
+void efx_fini_debugfs_channel(struct efx_channel *channel) {}
+
static inline int efx_init_debugfs_nic(struct efx_nic *efx)
{
return 0;
diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index c9e17a8208a9..804a25ceea7f 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -19,6 +19,7 @@
#include "nic.h"
#include "sriov.h"
#include "workarounds.h"
+#include "debugfs.h"
/* This is the first interrupt mode to try out of:
* 0 => MSI-X
@@ -667,6 +668,12 @@ static int efx_probe_channel(struct efx_channel *channel)
channel->rx_list = NULL;
+ rc = efx_init_debugfs_channel(channel);
+ if (rc) /* not fatal */
+ netif_err(channel->efx, drv, channel->efx->net_dev,
+ "Failed to create debugfs for channel %d, rc=%d\n",
+ channel->channel, rc);
+
return 0;
fail:
@@ -743,6 +750,7 @@ void efx_remove_channel(struct efx_channel *channel)
netif_dbg(channel->efx, drv, channel->efx->net_dev,
"destroy chan %d\n", channel->channel);
+ efx_fini_debugfs_channel(channel);
efx_for_each_channel_rx_queue(rx_queue, channel)
efx_remove_rx_queue(rx_queue);
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 961e2db31c6e..2b92c5461fe3 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -528,6 +528,10 @@ struct efx_channel {
#define RPS_FLOW_ID_INVALID 0xFFFFFFFF
u32 *rps_flow_id;
#endif
+#ifdef CONFIG_DEBUG_FS
+ /** @debug_dir: Channel debugfs directory (under @efx->debug_channels_dir) */
+ struct dentry *debug_dir;
+#endif
unsigned int n_rx_tobe_disc;
unsigned int n_rx_ip_hdr_chksum_err;
@@ -1144,6 +1148,8 @@ struct efx_nic {
#ifdef CONFIG_DEBUG_FS
/** @debug_dir: NIC debugfs directory */
struct dentry *debug_dir;
+ /** @debug_channels_dir: contains channel debugfs dirs. Under @debug_dir */
+ struct dentry *debug_channels_dir;
/** @debug_symlink: NIC debugfs symlink (``nic_eth%d``) */
struct dentry *debug_symlink;
/** @debug_interrupt_mode: debugfs details for printing @interrupt_mode */
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues
2023-12-11 17:18 [PATCH net-next 0/7] sfc: initial debugfs support edward.cree
2023-12-11 17:18 ` [PATCH net-next 1/7] sfc: initial debugfs implementation edward.cree
2023-12-11 17:18 ` [PATCH net-next 2/7] sfc: debugfs for channels edward.cree
@ 2023-12-11 17:18 ` edward.cree
2023-12-12 22:06 ` kernel test robot
2023-12-15 0:05 ` Nelson, Shannon
2023-12-11 17:18 ` [PATCH net-next 4/7] sfc: debugfs for (nic) TX queues edward.cree
` (3 subsequent siblings)
6 siblings, 2 replies; 23+ messages in thread
From: edward.cree @ 2023-12-11 17:18 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
From: Edward Cree <ecree.xilinx@gmail.com>
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 <jonathan.s.cooper@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
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(-)
diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
index b46339249794..43b1d06a985e 100644
--- a/drivers/net/ethernet/sfc/debugfs.c
+++ b/drivers/net/ethernet/sfc/debugfs.c
@@ -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.
+ */
+int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue)
+{
+ struct efx_channel *channel = efx_rx_queue_channel(rx_queue);
+ char target[EFX_DEBUGFS_NAME_LEN];
+ char name[EFX_DEBUGFS_NAME_LEN];
+
+ if (!rx_queue->efx->debug_queues_dir)
+ return -ENODEV;
+ /* Create directory */
+ if (snprintf(name, sizeof(name), "rx-%d", efx_rx_queue_index(rx_queue))
+ >= 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);
+
+ /* 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;
+
+ 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;
}
diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
index 4af4a03d1b97..53c98a2fb4c9 100644
--- a/drivers/net/ethernet/sfc/debugfs.h
+++ b/drivers/net/ethernet/sfc/debugfs.h
@@ -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;
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 2b92c5461fe3..63eb32670826 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -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 */
diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
index d2f35ee15eff..7f63f70f082d 100644
--- a/drivers/net/ethernet/sfc/rx_common.c
+++ b/drivers/net/ethernet/sfc/rx_common.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH net-next 4/7] sfc: debugfs for (nic) TX queues
2023-12-11 17:18 [PATCH net-next 0/7] sfc: initial debugfs support edward.cree
` (2 preceding siblings ...)
2023-12-11 17:18 ` [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues edward.cree
@ 2023-12-11 17:18 ` edward.cree
2023-12-13 0:15 ` kernel test robot
2023-12-11 17:18 ` [PATCH net-next 5/7] sfc: add debugfs nodes for loopback mode edward.cree
` (2 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: edward.cree @ 2023-12-11 17:18 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
From: Edward Cree <ecree.xilinx@gmail.com>
Expose each TX queue's label, type (csum offloads), TSO and timestamping
capabilities, and the read/write/etc pointers for the descriptor ring.
Each TXQ dir also symlinks to its owning channel.
Reviewed-by: Jonathan Cooper <jonathan.s.cooper@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
drivers/net/ethernet/sfc/debugfs.c | 71 +++++++++++++++++++++++++++
drivers/net/ethernet/sfc/debugfs.h | 14 ++++++
drivers/net/ethernet/sfc/net_driver.h | 4 ++
drivers/net/ethernet/sfc/tx_common.c | 8 +++
4 files changed, 97 insertions(+)
diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
index 43b1d06a985e..8ee6e401ea44 100644
--- a/drivers/net/ethernet/sfc/debugfs.c
+++ b/drivers/net/ethernet/sfc/debugfs.c
@@ -78,6 +78,77 @@ void efx_update_debugfs_netdev(struct efx_nic *efx)
mutex_unlock(&efx->debugfs_symlink_mutex);
}
+#define EFX_DEBUGFS_TXQ(_type, _name) \
+ debugfs_create_##_type(#_name, 0444, tx_queue->debug_dir, &tx_queue->_name)
+
+/* Create basic debugfs parameter files for an Efx TXQ */
+static void efx_init_debugfs_tx_queue_files(struct efx_tx_queue *tx_queue)
+{
+ EFX_DEBUGFS_TXQ(u32, label);
+ EFX_DEBUGFS_TXQ(bool, xdp_tx);
+ /* offload features */
+ EFX_DEBUGFS_TXQ(u32, type);
+ EFX_DEBUGFS_TXQ(u32, tso_version);
+ EFX_DEBUGFS_TXQ(bool, tso_encap);
+ EFX_DEBUGFS_TXQ(bool, timestamping);
+ /* descriptor ring indices */
+ EFX_DEBUGFS_TXQ(u32, read_count);
+ EFX_DEBUGFS_TXQ(u32, insert_count);
+ EFX_DEBUGFS_TXQ(u32, write_count);
+ EFX_DEBUGFS_TXQ(u32, notify_count);
+}
+
+/**
+ * efx_init_debugfs_tx_queue - create debugfs directory for TX queue
+ * @tx_queue: Efx TX queue
+ *
+ * Create a debugfs directory containing parameter-files for @tx_queue.
+ * The directory must be cleaned up using efx_fini_debugfs_tx_queue(),
+ * even if this function returns an error.
+ *
+ * Return: a negative error code or 0 on success.
+ */
+int efx_init_debugfs_tx_queue(struct efx_tx_queue *tx_queue)
+{
+ char target[EFX_DEBUGFS_NAME_LEN];
+ char name[EFX_DEBUGFS_NAME_LEN];
+
+ if (!tx_queue->efx->debug_queues_dir)
+ return -ENODEV;
+ /* Create directory */
+ if (snprintf(name, sizeof(name), "tx-%d", tx_queue->queue)
+ >= sizeof(name))
+ return -ENAMETOOLONG;
+ tx_queue->debug_dir = debugfs_create_dir(name,
+ tx_queue->efx->debug_queues_dir);
+ if (!tx_queue->debug_dir)
+ return -ENOMEM;
+
+ /* Create files */
+ efx_init_debugfs_tx_queue_files(tx_queue);
+
+ /* Create symlink to channel */
+ if (snprintf(target, sizeof(target), "../../channels/%d",
+ tx_queue->channel->channel) >= sizeof(target))
+ return -ENAMETOOLONG;
+ if (!debugfs_create_symlink("channel", tx_queue->debug_dir, target))
+ return -ENOMEM;
+
+ return 0;
+}
+
+/**
+ * efx_fini_debugfs_tx_queue - remove debugfs directory for TX queue
+ * @tx_queue: Efx TX queue
+ *
+ * Remove directory created for @tx_queue by efx_init_debugfs_tx_queue().
+ */
+void efx_fini_debugfs_tx_queue(struct efx_tx_queue *tx_queue)
+{
+ debugfs_remove_recursive(tx_queue->debug_dir);
+ tx_queue->debug_dir = NULL;
+}
+
#define EFX_DEBUGFS_RXQ(_type, _name) \
debugfs_create_##_type(#_name, 0444, rx_queue->debug_dir, &rx_queue->_name)
diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
index 53c98a2fb4c9..3e8d2e2b5bad 100644
--- a/drivers/net/ethernet/sfc/debugfs.h
+++ b/drivers/net/ethernet/sfc/debugfs.h
@@ -34,11 +34,19 @@
* (&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.
+ * * For each NIC TX queue, this will contain a directory
+ * (&efx_tx_queue.debug_dir), whose name is "tx-N" where N is the TX queue
+ * 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.
*/
void efx_fini_debugfs_netdev(struct net_device *net_dev);
void efx_update_debugfs_netdev(struct efx_nic *efx);
+int efx_init_debugfs_tx_queue(struct efx_tx_queue *tx_queue);
+void efx_fini_debugfs_tx_queue(struct efx_tx_queue *tx_queue);
+
int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue);
void efx_fini_debugfs_rx_queue(struct efx_rx_queue *rx_queue);
@@ -57,6 +65,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_tx_queue(struct efx_tx_queue *tx_queue)
+{
+ return 0;
+}
+void efx_fini_debugfs_tx_queue(struct efx_tx_queue *tx_queue) {}
+
int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue)
{
return 0;
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 63eb32670826..feb87979059c 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -273,6 +273,10 @@ struct efx_tx_queue {
bool initialised;
bool timestamping;
bool xdp_tx;
+#ifdef CONFIG_DEBUG_FS
+ /** @debug_dir: Queue debugfs directory (under @efx->debug_queues_dir) */
+ struct dentry *debug_dir;
+#endif
/* Members used mainly on the completion path */
unsigned int read_count ____cacheline_aligned_in_smp;
diff --git a/drivers/net/ethernet/sfc/tx_common.c b/drivers/net/ethernet/sfc/tx_common.c
index 9f2393d34371..3780da849e98 100644
--- a/drivers/net/ethernet/sfc/tx_common.c
+++ b/drivers/net/ethernet/sfc/tx_common.c
@@ -12,6 +12,7 @@
#include "efx.h"
#include "nic_common.h"
#include "tx_common.h"
+#include "debugfs.h"
#include <net/gso.h>
static unsigned int efx_tx_cb_page_count(struct efx_tx_queue *tx_queue)
@@ -47,6 +48,11 @@ int efx_probe_tx_queue(struct efx_tx_queue *tx_queue)
rc = -ENOMEM;
goto fail1;
}
+ rc = efx_init_debugfs_tx_queue(tx_queue);
+ if (rc) /* not fatal */
+ netif_err(efx, drv, efx->net_dev,
+ "Failed to create debugfs for TXQ %d, rc=%d\n",
+ tx_queue->queue, rc);
/* Allocate hardware ring, determine TXQ type */
rc = efx_nic_probe_tx(tx_queue);
@@ -133,6 +139,8 @@ void efx_remove_tx_queue(struct efx_tx_queue *tx_queue)
"destroying TX queue %d\n", tx_queue->queue);
efx_nic_remove_tx(tx_queue);
+ efx_fini_debugfs_tx_queue(tx_queue);
+
if (tx_queue->cb_page) {
for (i = 0; i < efx_tx_cb_page_count(tx_queue); i++)
efx_nic_free_buffer(tx_queue->efx,
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH net-next 5/7] sfc: add debugfs nodes for loopback mode
2023-12-11 17:18 [PATCH net-next 0/7] sfc: initial debugfs support edward.cree
` (3 preceding siblings ...)
2023-12-11 17:18 ` [PATCH net-next 4/7] sfc: debugfs for (nic) TX queues edward.cree
@ 2023-12-11 17:18 ` edward.cree
2023-12-11 17:18 ` [PATCH net-next 6/7] sfc: add debugfs entries for filter table status edward.cree
2023-12-11 17:18 ` [PATCH net-next 7/7] sfc: add debugfs node for filter table contents edward.cree
6 siblings, 0 replies; 23+ messages in thread
From: edward.cree @ 2023-12-11 17:18 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
From: Edward Cree <ecree.xilinx@gmail.com>
'loopback_mode' shows the currently selected mode, while 'loopback_modes'
is a bitmask of modes supported by the NIC+FW.
Reviewed-by: Jonathan Cooper <jonathan.s.cooper@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
drivers/net/ethernet/sfc/debugfs.c | 7 +++++++
drivers/net/ethernet/sfc/net_driver.h | 2 ++
2 files changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
index 8ee6e401ea44..549ff1ee273e 100644
--- a/drivers/net/ethernet/sfc/debugfs.c
+++ b/drivers/net/ethernet/sfc/debugfs.c
@@ -326,6 +326,13 @@ static void efx_init_debugfs_nic_files(struct efx_nic *efx)
efx->debug_interrupt_mode.value = &efx->interrupt_mode;
efx_debugfs_create_enum("interrupt_mode", 0444, efx->debug_dir,
&efx->debug_interrupt_mode);
+ EFX_DEBUGFS_EFX(x64, loopback_modes);
+ efx->debug_loopback_mode.max = efx_loopback_mode_max;
+ efx->debug_loopback_mode.names = efx_loopback_mode_names;
+ efx->debug_loopback_mode.vlen = sizeof(efx->loopback_mode);
+ efx->debug_loopback_mode.value = &efx->loopback_mode;
+ efx_debugfs_create_enum("loopback_mode", 0444, efx->debug_dir,
+ &efx->debug_loopback_mode);
}
/**
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index feb87979059c..e3605c361117 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -1164,6 +1164,8 @@ struct efx_nic {
struct dentry *debug_symlink;
/** @debug_interrupt_mode: debugfs details for printing @interrupt_mode */
struct efx_debugfs_enum_data debug_interrupt_mode;
+ /** @debug_loopback_mode: debugfs details for printing @loopback_mode */
+ struct efx_debugfs_enum_data debug_loopback_mode;
/** @debugfs_symlink_mutex: protects debugfs @debug_symlink */
struct mutex debugfs_symlink_mutex;
#endif
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH net-next 6/7] sfc: add debugfs entries for filter table status
2023-12-11 17:18 [PATCH net-next 0/7] sfc: initial debugfs support edward.cree
` (4 preceding siblings ...)
2023-12-11 17:18 ` [PATCH net-next 5/7] sfc: add debugfs nodes for loopback mode edward.cree
@ 2023-12-11 17:18 ` edward.cree
2023-12-11 19:25 ` Simon Horman
2023-12-15 0:05 ` Nelson, Shannon
2023-12-11 17:18 ` [PATCH net-next 7/7] sfc: add debugfs node for filter table contents edward.cree
6 siblings, 2 replies; 23+ messages in thread
From: edward.cree @ 2023-12-11 17:18 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
From: Edward Cree <ecree.xilinx@gmail.com>
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 <jonathan.s.cooper@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
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(+)
diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
index 3e8d2e2b5bad..7a96f3798cbd 100644
--- a/drivers/net/ethernet/sfc/debugfs.h
+++ b/drivers/net/ethernet/sfc/debugfs.h
@@ -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);
diff --git a/drivers/net/ethernet/sfc/mcdi_filters.c b/drivers/net/ethernet/sfc/mcdi_filters.c
index 4ff6586116ee..a4ab45082c8f 100644
--- a/drivers/net/ethernet/sfc/mcdi_filters.c
+++ b/drivers/net/ethernet/sfc/mcdi_filters.c
@@ -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;
@@ -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);
}
diff --git a/drivers/net/ethernet/sfc/mcdi_filters.h b/drivers/net/ethernet/sfc/mcdi_filters.h
index c0d6558b9fd2..897843ade3ec 100644
--- a/drivers/net/ethernet/sfc/mcdi_filters.h
+++ b/drivers/net/ethernet/sfc/mcdi_filters.h
@@ -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);
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH net-next 7/7] sfc: add debugfs node for filter table contents
2023-12-11 17:18 [PATCH net-next 0/7] sfc: initial debugfs support edward.cree
` (5 preceding siblings ...)
2023-12-11 17:18 ` [PATCH net-next 6/7] sfc: add debugfs entries for filter table status edward.cree
@ 2023-12-11 17:18 ` edward.cree
2023-12-11 19:17 ` Simon Horman
2023-12-15 0:05 ` Nelson, Shannon
6 siblings, 2 replies; 23+ messages in thread
From: edward.cree @ 2023-12-11 17:18 UTC (permalink / raw)
To: linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
From: Edward Cree <ecree.xilinx@gmail.com>
Expose the filter table entries.
Reviewed-by: Jonathan Cooper <jonathan.s.cooper@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
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(-)
diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
index 549ff1ee273e..e67b0fc927fe 100644
--- a/drivers/net/ethernet/sfc/debugfs.c
+++ b/drivers/net/ethernet/sfc/debugfs.c
@@ -9,10 +9,6 @@
*/
#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
@@ -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,
+ (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, ",rip=?");
+ }
+ }
+ 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");
+}
diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
index 7a96f3798cbd..f50b4bf33a6b 100644
--- a/drivers/net/ethernet/sfc/debugfs.h
+++ b/drivers/net/ethernet/sfc/debugfs.h
@@ -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 */
diff --git a/drivers/net/ethernet/sfc/mcdi_filters.c b/drivers/net/ethernet/sfc/mcdi_filters.c
index a4ab45082c8f..465226c3e8c7 100644
--- a/drivers/net/ethernet/sfc/mcdi_filters.c
+++ b/drivers/net/ethernet/sfc/mcdi_filters.c
@@ -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;
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 7/7] sfc: add debugfs node for filter table contents
2023-12-11 17:18 ` [PATCH net-next 7/7] sfc: add debugfs node for filter table contents edward.cree
@ 2023-12-11 19:17 ` Simon Horman
2023-12-12 13:58 ` Edward Cree
2023-12-15 0:05 ` Nelson, Shannon
1 sibling, 1 reply; 23+ messages in thread
From: Simon Horman @ 2023-12-11 19:17 UTC (permalink / raw)
To: edward.cree
Cc: linux-net-drivers, davem, kuba, pabeni, edumazet, Edward Cree,
netdev, habetsm.xilinx, Jonathan Cooper
On Mon, Dec 11, 2023 at 05:18:32PM +0000, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
>
> Expose the filter table entries.
>
> Reviewed-by: Jonathan Cooper <jonathan.s.cooper@amd.com>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
...
> diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
...
> @@ -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); \
> +} \
Hi Edward,
I think that probably the above should be static inline.
...
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 6/7] sfc: add debugfs entries for filter table status
2023-12-11 17:18 ` [PATCH net-next 6/7] sfc: add debugfs entries for filter table status edward.cree
@ 2023-12-11 19:25 ` Simon Horman
2023-12-15 0:05 ` Nelson, Shannon
1 sibling, 0 replies; 23+ messages in thread
From: Simon Horman @ 2023-12-11 19:25 UTC (permalink / raw)
To: edward.cree
Cc: linux-net-drivers, davem, kuba, pabeni, edumazet, Edward Cree,
netdev, habetsm.xilinx, Jonathan Cooper
On Mon, Dec 11, 2023 at 05:18:31PM +0000, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
>
> 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 <jonathan.s.cooper@amd.com>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
...
> index 4ff6586116ee..a4ab45082c8f 100644
> --- a/drivers/net/ethernet/sfc/mcdi_filters.c
> +++ b/drivers/net/ethernet/sfc/mcdi_filters.c
> @@ -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;
> @@ -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);
> }
>
Hi Edward,
I think debugfs.h needs to be included so that debugfs_*() are defined.
...
--
pw-bot: changes-requested
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 7/7] sfc: add debugfs node for filter table contents
2023-12-11 19:17 ` Simon Horman
@ 2023-12-12 13:58 ` Edward Cree
2023-12-12 15:14 ` Edward Cree
0 siblings, 1 reply; 23+ messages in thread
From: Edward Cree @ 2023-12-12 13:58 UTC (permalink / raw)
To: Simon Horman, edward.cree
Cc: linux-net-drivers, davem, kuba, pabeni, edumazet, netdev,
habetsm.xilinx, Jonathan Cooper
On 11/12/2023 19:17, Simon Horman wrote:
> On Mon, Dec 11, 2023 at 05:18:32PM +0000, edward.cree@amd.com wrote:
>> @@ -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); \
>> +} \
>
> 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 :/
Will fix in v2, along with the build break on #6.
Thanks for reviewing.
-ed
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 7/7] sfc: add debugfs node for filter table contents
2023-12-12 13:58 ` Edward Cree
@ 2023-12-12 15:14 ` Edward Cree
2023-12-12 16:19 ` Jakub Kicinski
0 siblings, 1 reply; 23+ messages in thread
From: Edward Cree @ 2023-12-12 15:14 UTC (permalink / raw)
To: Simon Horman, edward.cree
Cc: linux-net-drivers, davem, kuba, pabeni, edumazet, netdev,
habetsm.xilinx, Jonathan Cooper
On 12/12/2023 13:58, Edward Cree wrote:
> On 11/12/2023 19:17, Simon Horman wrote:
>> On Mon, Dec 11, 2023 at 05:18:32PM +0000, edward.cree@amd.com wrote:
>>> @@ -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); \
>>> +} \
>>
>> 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.
-ed
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 7/7] sfc: add debugfs node for filter table contents
2023-12-12 15:14 ` Edward Cree
@ 2023-12-12 16:19 ` Jakub Kicinski
2023-12-13 12:15 ` Edward Cree
0 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2023-12-12 16:19 UTC (permalink / raw)
To: Edward Cree
Cc: Simon Horman, edward.cree, linux-net-drivers, davem, pabeni,
edumazet, netdev, habetsm.xilinx, Jonathan Cooper
On Tue, 12 Dec 2023 15:14:17 +0000 Edward Cree wrote:
> On 12/12/2023 13:58, Edward Cree wrote:
> > On 11/12/2023 19:17, Simon Horman wrote:
> >> On Mon, Dec 11, 2023 at 05:18:32PM +0000, edward.cree@amd.com wrote:
> [...]
> >>
> >> 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.
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) {}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 2/7] sfc: debugfs for channels
2023-12-11 17:18 ` [PATCH net-next 2/7] sfc: debugfs for channels edward.cree
@ 2023-12-12 19:54 ` kernel test robot
0 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2023-12-12 19:54 UTC (permalink / raw)
To: edward.cree, linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: oe-kbuild-all, Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/edward-cree-amd-com/sfc-initial-debugfs-implementation/20231212-013223
base: net-next/main
patch link: https://lore.kernel.org/r/df43d737fda6b6aa0cda3f2cb300916ca4b2e8f8.1702314695.git.ecree.xilinx%40gmail.com
patch subject: [PATCH net-next 2/7] sfc: debugfs for channels
config: mips-ip27_defconfig (https://download.01.org/0day-ci/archive/20231213/202312130301.cKD8l8IO-lkp@intel.com/config)
compiler: mips64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231213/202312130301.cKD8l8IO-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312130301.cKD8l8IO-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/net/ethernet/sfc/efx.c:36:
>> drivers/net/ethernet/sfc/debugfs.h:51:5: warning: no previous prototype for 'efx_init_debugfs_channel' [-Wmissing-prototypes]
51 | int efx_init_debugfs_channel(struct efx_channel *channel)
| ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/sfc/debugfs.h:55:6: warning: no previous prototype for 'efx_fini_debugfs_channel' [-Wmissing-prototypes]
55 | void efx_fini_debugfs_channel(struct efx_channel *channel) {}
| ^~~~~~~~~~~~~~~~~~~~~~~~
vim +/efx_init_debugfs_channel +51 drivers/net/ethernet/sfc/debugfs.h
50
> 51 int efx_init_debugfs_channel(struct efx_channel *channel)
52 {
53 return 0;
54 }
> 55 void efx_fini_debugfs_channel(struct efx_channel *channel) {}
56
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues
2023-12-11 17:18 ` [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues edward.cree
@ 2023-12-12 22:06 ` kernel test robot
2023-12-15 0:05 ` Nelson, Shannon
1 sibling, 0 replies; 23+ messages in thread
From: kernel test robot @ 2023-12-12 22:06 UTC (permalink / raw)
To: edward.cree, linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: oe-kbuild-all, Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/edward-cree-amd-com/sfc-initial-debugfs-implementation/20231212-013223
base: net-next/main
patch link: https://lore.kernel.org/r/a5c5491d3d0b58b8f8dff65cb53f892d7b13c32a.1702314695.git.ecree.xilinx%40gmail.com
patch subject: [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues
config: mips-ip27_defconfig (https://download.01.org/0day-ci/archive/20231213/202312130527.xlkFaOuC-lkp@intel.com/config)
compiler: mips64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231213/202312130527.xlkFaOuC-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312130527.xlkFaOuC-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/net/ethernet/sfc/efx.c:36:
>> drivers/net/ethernet/sfc/debugfs.h:60:5: warning: no previous prototype for 'efx_init_debugfs_rx_queue' [-Wmissing-prototypes]
60 | int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/sfc/debugfs.h:64:6: warning: no previous prototype for 'efx_fini_debugfs_rx_queue' [-Wmissing-prototypes]
64 | void efx_fini_debugfs_rx_queue(struct efx_rx_queue *rx_queue) {}
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sfc/debugfs.h:66:5: warning: no previous prototype for 'efx_init_debugfs_channel' [-Wmissing-prototypes]
66 | int efx_init_debugfs_channel(struct efx_channel *channel)
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sfc/debugfs.h:70:6: warning: no previous prototype for 'efx_fini_debugfs_channel' [-Wmissing-prototypes]
70 | void efx_fini_debugfs_channel(struct efx_channel *channel) {}
| ^~~~~~~~~~~~~~~~~~~~~~~~
vim +/efx_init_debugfs_rx_queue +60 drivers/net/ethernet/sfc/debugfs.h
59
> 60 int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue)
61 {
62 return 0;
63 }
> 64 void efx_fini_debugfs_rx_queue(struct efx_rx_queue *rx_queue) {}
65
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 4/7] sfc: debugfs for (nic) TX queues
2023-12-11 17:18 ` [PATCH net-next 4/7] sfc: debugfs for (nic) TX queues edward.cree
@ 2023-12-13 0:15 ` kernel test robot
0 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2023-12-13 0:15 UTC (permalink / raw)
To: edward.cree, linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: oe-kbuild-all, Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/edward-cree-amd-com/sfc-initial-debugfs-implementation/20231212-013223
base: net-next/main
patch link: https://lore.kernel.org/r/91beef38162b8e243c2275b41a6f37c01f19850f.1702314695.git.ecree.xilinx%40gmail.com
patch subject: [PATCH net-next 4/7] sfc: debugfs for (nic) TX queues
config: mips-ip27_defconfig (https://download.01.org/0day-ci/archive/20231213/202312130801.r8TbjUfD-lkp@intel.com/config)
compiler: mips64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231213/202312130801.r8TbjUfD-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312130801.r8TbjUfD-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/net/ethernet/sfc/efx.c:36:
>> drivers/net/ethernet/sfc/debugfs.h:68:5: warning: no previous prototype for 'efx_init_debugfs_tx_queue' [-Wmissing-prototypes]
68 | int efx_init_debugfs_tx_queue(struct efx_tx_queue *tx_queue)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/sfc/debugfs.h:72:6: warning: no previous prototype for 'efx_fini_debugfs_tx_queue' [-Wmissing-prototypes]
72 | void efx_fini_debugfs_tx_queue(struct efx_tx_queue *tx_queue) {}
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sfc/debugfs.h:74:5: warning: no previous prototype for 'efx_init_debugfs_rx_queue' [-Wmissing-prototypes]
74 | int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sfc/debugfs.h:78:6: warning: no previous prototype for 'efx_fini_debugfs_rx_queue' [-Wmissing-prototypes]
78 | void efx_fini_debugfs_rx_queue(struct efx_rx_queue *rx_queue) {}
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sfc/debugfs.h:80:5: warning: no previous prototype for 'efx_init_debugfs_channel' [-Wmissing-prototypes]
80 | int efx_init_debugfs_channel(struct efx_channel *channel)
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sfc/debugfs.h:84:6: warning: no previous prototype for 'efx_fini_debugfs_channel' [-Wmissing-prototypes]
84 | void efx_fini_debugfs_channel(struct efx_channel *channel) {}
| ^~~~~~~~~~~~~~~~~~~~~~~~
vim +/efx_init_debugfs_tx_queue +68 drivers/net/ethernet/sfc/debugfs.h
67
> 68 int efx_init_debugfs_tx_queue(struct efx_tx_queue *tx_queue)
69 {
70 return 0;
71 }
> 72 void efx_fini_debugfs_tx_queue(struct efx_tx_queue *tx_queue) {}
73
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 7/7] sfc: add debugfs node for filter table contents
2023-12-12 16:19 ` Jakub Kicinski
@ 2023-12-13 12:15 ` Edward Cree
2023-12-14 16:56 ` Simon Horman
0 siblings, 1 reply; 23+ messages in thread
From: Edward Cree @ 2023-12-13 12:15 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Simon Horman, edward.cree, linux-net-drivers, davem, pabeni,
edumazet, netdev, habetsm.xilinx, Jonathan Cooper
On 12/12/2023 16:19, Jakub Kicinski wrote:
> 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.
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 7/7] sfc: add debugfs node for filter table contents
2023-12-13 12:15 ` Edward Cree
@ 2023-12-14 16:56 ` Simon Horman
0 siblings, 0 replies; 23+ messages in thread
From: Simon Horman @ 2023-12-14 16:56 UTC (permalink / raw)
To: Edward Cree
Cc: Jakub Kicinski, edward.cree, linux-net-drivers, davem, pabeni,
edumazet, netdev, habetsm.xilinx, Jonathan Cooper
On Wed, Dec 13, 2023 at 12:15:04PM +0000, Edward Cree wrote:
> On 12/12/2023 16:19, Jakub Kicinski wrote:
> > 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.
>
> > 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).
Thanks, and sorry for the false positives.
Also, I like coffee :)
>
> -ed
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 1/7] sfc: initial debugfs implementation
2023-12-11 17:18 ` [PATCH net-next 1/7] sfc: initial debugfs implementation edward.cree
@ 2023-12-15 0:05 ` Nelson, Shannon
2024-02-08 21:25 ` Edward Cree
0 siblings, 1 reply; 23+ messages in thread
From: Nelson, Shannon @ 2023-12-15 0:05 UTC (permalink / raw)
To: edward.cree, linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
On 12/11/2023 9:18 AM, edward.cree@amd.com wrote:
>
> From: Edward Cree <ecree.xilinx@gmail.com>
>
Hi Ed, a few minor nits I thought I'd call out.
Cheers,
sln
> Just a handful of nodes, including one enum with a string table for
> pretty printing the values.
It would be nice to have a couple of example paths listed here
>
> Reviewed-by: Jonathan Cooper <jonathan.s.cooper@amd.com>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
> 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
>
> diff --git a/drivers/net/ethernet/sfc/Makefile b/drivers/net/ethernet/sfc/Makefile
> index 8f446b9bd5ee..1fbdd04dc2c5 100644
> --- a/drivers/net/ethernet/sfc/Makefile
> +++ b/drivers/net/ethernet/sfc/Makefile
> @@ -12,6 +12,7 @@ sfc-$(CONFIG_SFC_MTD) += mtd.o
> sfc-$(CONFIG_SFC_SRIOV) += sriov.o ef10_sriov.o ef100_sriov.o ef100_rep.o \
> mae.o tc.o tc_bindings.o tc_counters.o \
> tc_encap_actions.o tc_conntrack.o
> +sfc-$(CONFIG_DEBUG_FS) += debugfs.o
Just as you are using #ifdef CONFIG_DEBUG_FS in your debugfs.h, you
could use it in your debugfs.c and simply add the .o file to your sfc-y
list here.
>
> obj-$(CONFIG_SFC) += sfc.o
>
> diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
> new file mode 100644
> index 000000000000..cf800addb4ff
> --- /dev/null
> +++ b/drivers/net/ethernet/sfc/debugfs.c
> @@ -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;
> +}
> +
> +/* 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.
> +
> +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;
> +}
> diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
> new file mode 100644
> index 000000000000..1fe40fbffa5e
> --- /dev/null
> +++ b/drivers/net/ethernet/sfc/debugfs.h
> @@ -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 */
> diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
> index 6dfa062feebc..58e18fc92093 100644
> --- a/drivers/net/ethernet/sfc/ef10.c
> +++ b/drivers/net/ethernet/sfc/ef10.c
> @@ -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>
> @@ -580,6 +581,13 @@ static int efx_ef10_probe(struct efx_nic *efx)
> if (rc)
> goto fail3;
>
> + /* Populate debugfs */
> +#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
> +
> 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);
> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
> index 7f7d560cb2b4..e844d57754b7 100644
> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
> @@ -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)
> @@ -405,6 +407,11 @@ void ef100_remove_netdev(struct efx_probe_data *probe_data)
> ef100_pf_unset_devlink_port(efx);
> efx_fini_tc(efx);
> #endif
> +#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?
>
> down_write(&efx->filter_sem);
> efx_mcdi_filter_table_remove(efx);
> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
> index 6da06931187d..ad378aa05dc3 100644
> --- a/drivers/net/ethernet/sfc/ef100_nic.c
> +++ b/drivers/net/ethernet/sfc/ef100_nic.c
> @@ -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
> @@ -1077,6 +1078,12 @@ static int ef100_probe_main(struct efx_nic *efx)
>
> /* Post-IO section. */
>
> + /* Populate debugfs */
> +#ifdef CONFIG_DEBUG_FS
> + rc = efx_init_debugfs_nic(efx);
> + if (rc)
> + pci_err(efx->pci_dev, "failed to init device debugfs\n");
> +#endif
Shouldn't need the ifdef
> 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);
> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
> index 19f4b4d0b851..9266c7b5b4fd 100644
> --- a/drivers/net/ethernet/sfc/efx.c
> +++ b/drivers/net/ethernet/sfc/efx.c
> @@ -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"
> @@ -401,6 +402,11 @@ static void efx_remove_all(struct efx_nic *efx)
> #endif
> efx_remove_port(efx);
> efx_remove_nic(efx);
> +#ifdef CONFIG_DEBUG_FS
> + mutex_lock(&efx->debugfs_symlink_mutex);
> + efx_fini_debugfs_netdev(efx->net_dev);
> + mutex_unlock(&efx->debugfs_symlink_mutex);
> +#endif
Same mutex comment
> }
>
> /**************************************************************************
> @@ -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);
> diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
> index 175bd9cdfdac..7a9d6b6b66e5 100644
> --- a/drivers/net/ethernet/sfc/efx_common.c
> +++ b/drivers/net/ethernet/sfc/efx_common.c
> @@ -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);
>
> +#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)
> 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) {
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 27d86e90a3bb..961e2db31c6e 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -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;
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues
2023-12-11 17:18 ` [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues edward.cree
2023-12-12 22:06 ` kernel test robot
@ 2023-12-15 0:05 ` Nelson, Shannon
1 sibling, 0 replies; 23+ messages in thread
From: Nelson, Shannon @ 2023-12-15 0:05 UTC (permalink / raw)
To: edward.cree, linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
On 12/11/2023 9:18 AM, edward.cree@amd.com wrote:
>
> From: Edward Cree <ecree.xilinx@gmail.com>
>
> 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 <jonathan.s.cooper@amd.com>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
> 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(-)
>
> diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
> index b46339249794..43b1d06a985e 100644
> --- a/drivers/net/ethernet/sfc/debugfs.c
> +++ b/drivers/net/ethernet/sfc/debugfs.c
> @@ -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.
> + */
> +int efx_init_debugfs_rx_queue(struct efx_rx_queue *rx_queue)
> +{
> + struct efx_channel *channel = efx_rx_queue_channel(rx_queue);
> + char target[EFX_DEBUGFS_NAME_LEN];
> + char name[EFX_DEBUGFS_NAME_LEN];
> +
> + if (!rx_queue->efx->debug_queues_dir)
> + return -ENODEV;
> + /* Create directory */
> + 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
> + >= 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);
> +
> + /* 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()?
> +
> + 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;
> }
>
> diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
> index 4af4a03d1b97..53c98a2fb4c9 100644
> --- a/drivers/net/ethernet/sfc/debugfs.h
> +++ b/drivers/net/ethernet/sfc/debugfs.h
> @@ -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;
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 2b92c5461fe3..63eb32670826 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -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 */
> diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
> index d2f35ee15eff..7f63f70f082d 100644
> --- a/drivers/net/ethernet/sfc/rx_common.c
> +++ b/drivers/net/ethernet/sfc/rx_common.c
> @@ -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;
> }
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 6/7] sfc: add debugfs entries for filter table status
2023-12-11 17:18 ` [PATCH net-next 6/7] sfc: add debugfs entries for filter table status edward.cree
2023-12-11 19:25 ` Simon Horman
@ 2023-12-15 0:05 ` Nelson, Shannon
1 sibling, 0 replies; 23+ messages in thread
From: Nelson, Shannon @ 2023-12-15 0:05 UTC (permalink / raw)
To: edward.cree, linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
On 12/11/2023 9:18 AM, edward.cree@amd.com wrote:
>
> From: Edward Cree <ecree.xilinx@gmail.com>
>
> 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 <jonathan.s.cooper@amd.com>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
> 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(+)
>
> diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
> index 3e8d2e2b5bad..7a96f3798cbd 100644
> --- a/drivers/net/ethernet/sfc/debugfs.h
> +++ b/drivers/net/ethernet/sfc/debugfs.h
> @@ -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);
> diff --git a/drivers/net/ethernet/sfc/mcdi_filters.c b/drivers/net/ethernet/sfc/mcdi_filters.c
> index 4ff6586116ee..a4ab45082c8f 100644
> --- a/drivers/net/ethernet/sfc/mcdi_filters.c
> +++ b/drivers/net/ethernet/sfc/mcdi_filters.c
> @@ -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
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
> +
> 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);
> }
>
> diff --git a/drivers/net/ethernet/sfc/mcdi_filters.h b/drivers/net/ethernet/sfc/mcdi_filters.h
> index c0d6558b9fd2..897843ade3ec 100644
> --- a/drivers/net/ethernet/sfc/mcdi_filters.h
> +++ b/drivers/net/ethernet/sfc/mcdi_filters.h
> @@ -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);
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 7/7] sfc: add debugfs node for filter table contents
2023-12-11 17:18 ` [PATCH net-next 7/7] sfc: add debugfs node for filter table contents edward.cree
2023-12-11 19:17 ` Simon Horman
@ 2023-12-15 0:05 ` Nelson, Shannon
1 sibling, 0 replies; 23+ messages in thread
From: Nelson, Shannon @ 2023-12-15 0:05 UTC (permalink / raw)
To: edward.cree, linux-net-drivers, davem, kuba, pabeni, edumazet
Cc: Edward Cree, netdev, habetsm.xilinx, Jonathan Cooper
On 12/11/2023 9:18 AM, edward.cree@amd.com wrote:
>
> From: Edward Cree <ecree.xilinx@gmail.com>
>
> Expose the filter table entries.
>
> Reviewed-by: Jonathan Cooper <jonathan.s.cooper@amd.com>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
> 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(-)
>
> diff --git a/drivers/net/ethernet/sfc/debugfs.c b/drivers/net/ethernet/sfc/debugfs.c
> index 549ff1ee273e..e67b0fc927fe 100644
> --- a/drivers/net/ethernet/sfc/debugfs.c
> +++ b/drivers/net/ethernet/sfc/debugfs.c
> @@ -9,10 +9,6 @@
> */
>
> #include "debugfs.h"
> -#include <linux/module.h>
> -#include <linux/debugfs.h>
> -#include <linux/dcache.h>
> -#include <linux/seq_file.h>
Can you leave these out of the original patch and not have to remove
them here?
>
> /* 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,
> + (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, ",rip=?");
> + }
Since you have this code more than once, it might be a candidate for a
utility function, if one doesn't already exist somewhere in the kernel
already.
> + }
> + 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");
> +}
> diff --git a/drivers/net/ethernet/sfc/debugfs.h b/drivers/net/ethernet/sfc/debugfs.h
> index 7a96f3798cbd..f50b4bf33a6b 100644
> --- a/drivers/net/ethernet/sfc/debugfs.h
> +++ b/drivers/net/ethernet/sfc/debugfs.h
> @@ -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 */
> diff --git a/drivers/net/ethernet/sfc/mcdi_filters.c b/drivers/net/ethernet/sfc/mcdi_filters.c
> index a4ab45082c8f..465226c3e8c7 100644
> --- a/drivers/net/ethernet/sfc/mcdi_filters.c
> +++ b/drivers/net/ethernet/sfc/mcdi_filters.c
> @@ -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;
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH net-next 1/7] sfc: initial debugfs implementation
2023-12-15 0:05 ` Nelson, Shannon
@ 2024-02-08 21:25 ` Edward Cree
0 siblings, 0 replies; 23+ messages in thread
From: Edward Cree @ 2024-02-08 21:25 UTC (permalink / raw)
To: Nelson, Shannon, edward.cree, linux-net-drivers, davem, kuba,
pabeni, edumazet
Cc: netdev, habetsm.xilinx, Jonathan Cooper
On 15/12/2023 00:05, Nelson, Shannon wrote:
> It would be nice to have a couple of example paths listed here
Sure; added this to v2:
See included DOC: comment for directory structure; leaf nodes are
rx_dma_len, rx_buffer_order, rx_buffer_truesize and interrupt_mode.
Is that what you had in mind? Or more like
"grep -H . /sys/kernel/debug/sfc" output on a running system?
>> +/* 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.
AFAIK we've had it out-of-tree for a very long time and not found it
to need any real maintenance effort. And while it's not strictly
necessary, it is fairly convenient.
>> + /* Populate debugfs */
>> +#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
You're right; I'll fix these.
>> +#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?
Yes, although I needed to refactor slightly because it's also
called by efx_update_debugfs_netdev() which is already holding
the mutex.
>> diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
>> index 175bd9cdfdac..7a9d6b6b66e5 100644
>> --- a/drivers/net/ethernet/sfc/efx_common.c
>> +++ b/drivers/net/ethernet/sfc/efx_common.c
>> @@ -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);
>>
>> +#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)
Ifdefs for struct members that may not exist seems to be the
existing pattern in efx_init_struct and efx_fini_struct, so
I'd rather leave this here than wrap this single call in an
efx_init_struct_debugfs function.
Thanks for the review!
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-02-08 21:25 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-11 17:18 [PATCH net-next 0/7] sfc: initial debugfs support edward.cree
2023-12-11 17:18 ` [PATCH net-next 1/7] sfc: initial debugfs implementation edward.cree
2023-12-15 0:05 ` Nelson, Shannon
2024-02-08 21:25 ` Edward Cree
2023-12-11 17:18 ` [PATCH net-next 2/7] sfc: debugfs for channels edward.cree
2023-12-12 19:54 ` kernel test robot
2023-12-11 17:18 ` [PATCH net-next 3/7] sfc: debugfs for (nic) RX queues edward.cree
2023-12-12 22:06 ` kernel test robot
2023-12-15 0:05 ` Nelson, Shannon
2023-12-11 17:18 ` [PATCH net-next 4/7] sfc: debugfs for (nic) TX queues edward.cree
2023-12-13 0:15 ` kernel test robot
2023-12-11 17:18 ` [PATCH net-next 5/7] sfc: add debugfs nodes for loopback mode edward.cree
2023-12-11 17:18 ` [PATCH net-next 6/7] sfc: add debugfs entries for filter table status edward.cree
2023-12-11 19:25 ` Simon Horman
2023-12-15 0:05 ` Nelson, Shannon
2023-12-11 17:18 ` [PATCH net-next 7/7] sfc: add debugfs node for filter table contents edward.cree
2023-12-11 19:17 ` Simon Horman
2023-12-12 13:58 ` Edward Cree
2023-12-12 15:14 ` Edward Cree
2023-12-12 16:19 ` Jakub Kicinski
2023-12-13 12:15 ` Edward Cree
2023-12-14 16:56 ` Simon Horman
2023-12-15 0:05 ` Nelson, Shannon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.