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] [day] [month] [year] [list]

Message-ID: <5dd2cfaa-4f3f-4bc2-ab20-6ec4a1887703@amd.com>
Date: Fri, 14 Feb 2025 09:47:30 -0800
From: "Nelson, Shannon" <shannon.nelson@....com>
To: Edward Cree <ecree.xilinx@...il.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 2/14/2025 8:23 AM, Edward Cree wrote:
> 
> 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.

Thanks, I can live with this.
sln