netdev - Re: [PATCH net-next v2 14/15] devlink: add by-instance dump infra

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: <29f9505c-2a9a-69fa-bae3-d95ee1113646@intel.com>
Date:   Fri, 6 Jan 2023 08:57:42 -0800
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Jiri Pirko <jiri@...nulli.us>
CC:     Jakub Kicinski <kuba@...nel.org>, <davem@...emloft.net>,
        <netdev@...r.kernel.org>, <edumazet@...gle.com>,
        <pabeni@...hat.com>
Subject: Re: [PATCH net-next v2 14/15] devlink: add by-instance dump infra



On 1/6/2023 12:59 AM, Jiri Pirko wrote:
> Fri, Jan 06, 2023 at 01:16:05AM CET, jacob.e.keller@...el.com wrote:
>>
>>
>> On 1/5/2023 1:24 AM, Jiri Pirko wrote:
>>> Thu, Jan 05, 2023 at 05:05:30AM CET, kuba@...nel.org wrote:
>>>> Most dumpit implementations walk the devlink instances.
>>>> This requires careful lock taking and reference dropping.
>>>> Factor the loop out and provide just a callback to handle
>>>> a single instance dump.
>>>>
>>>> Convert one user as an example, other users converted
>>>> in the next change.
>>>>
>>>> Slightly inspired by ethtool netlink code.
>>>>
>>>> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
>>>> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
>>>> ---
>>>> net/devlink/devl_internal.h | 10 +++++++
>>>> net/devlink/leftover.c      | 55 ++++++++++++++++---------------------
>>>> net/devlink/netlink.c       | 34 +++++++++++++++++++++++
>>>> 3 files changed, 68 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
>>>> index 15149b0a68af..734553beccde 100644
>>>> --- a/net/devlink/devl_internal.h
>>>> +++ b/net/devlink/devl_internal.h
>>>> @@ -122,6 +122,11 @@ struct devlink_nl_dump_state {
>>>> 	};
>>>> };
>>>>
>>>> +struct devlink_gen_cmd {
>>>
>>> As I wrote in reply to v1, could this be "genl"?
>>>
>>
>> Except Kuba already said this wasn't about "generic netlink" but
>> "generic devlink command" vs "complicated command that can't use the new
> 
> Okay, that confuses me. What is supposed to be "generic devlink
> command"? I don't see anything "generic" about these.
> 


I read it as: "uses this new infrastructure for iteration" vs "does not
use it". I assume (but haven't double checked) that at least one command
still doesn't use this for some reason?

I don't know if there's a simpler name to get that across. I don't think
genl is helpful because devlink is always generic netlink.

maybe "devlink_iter_cmd"? but I dunno.

Thanks,
Jake