[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <71eded85-49bf-4f9d-a604-7b8129aa19ba@amd.com>
Date: Thu, 14 Dec 2023 16:05:50 -0800
From: "Nelson, Shannon" <shannon.nelson@....com>
To: edward.cree@....com, linux-net-drivers@....com, davem@...emloft.net,
kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com
Cc: Edward Cree <ecree.xilinx@...il.com>, netdev@...r.kernel.org,
habetsm.xilinx@...il.com, Jonathan Cooper <jonathan.s.cooper@....com>
Subject: Re: [PATCH net-next 7/7] sfc: add debugfs node for filter table
contents
On 12/11/2023 9:18 AM, edward.cree@....com wrote:
>
> From: Edward Cree <ecree.xilinx@...il.com>
>
> Expose the filter table entries.
>
> Reviewed-by: Jonathan Cooper <jonathan.s.cooper@....com>
> Signed-off-by: Edward Cree <ecree.xilinx@...il.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;
>
Powered by blists - more mailing lists