netdev - Re: [PATCH net-next 6/7] sfc: add debugfs entries for filter table status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <235217f3-97fc-9925-cf9d-7ebbb77f0487@gmail.com>
Date: Fri, 14 Feb 2025 16:23:50 +0000
From: Edward Cree <ecree.xilinx@...il.com>
To: "Nelson, Shannon" <shannon.nelson@....com>, edward.cree@....com,
 linux-net-drivers@....com, davem@...emloft.net, kuba@...nel.org,
 pabeni@...hat.com, edumazet@...gle.com
Cc: netdev@...r.kernel.org, habetsm.xilinx@...il.com,
 Jonathan Cooper <jonathan.s.cooper@....com>
Subject: Re: [PATCH net-next 6/7] sfc: add debugfs entries for filter table
 status

On 15/12/2023 00:05, Nelson, Shannon wrote:
> On 12/11/2023 9:18 AM, edward.cree@....com wrote:
>> +#ifdef CONFIG_DEBUG_FS
>> +       table->debug_dir = debugfs_create_dir("filters", efx->debug_dir);
>> +       debugfs_create_bool("uc_promisc", 0444, table->debug_dir,
>> +                           &table->uc_promisc);
>> +       debugfs_create_bool("mc_promisc", 0444, table->debug_dir,
>> +                           &table->mc_promisc);
>> +       debugfs_create_bool("mc_promisc_last", 0444, table->debug_dir,
>> +                           &table->mc_promisc_last);
>> +       debugfs_create_bool("mc_overflow", 0444, table->debug_dir,
>> +                           &table->mc_overflow);
>> +       debugfs_create_bool("mc_chaining", 0444, table->debug_dir,
>> +                           &table->mc_chaining);
>> +#endif
> 
> It would be good to continue the practice of using the debugfs_* primitives in your debugfs.c and just make a single call here that doesn't need the ifdef

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