netdev - Re: [RFC net-next 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: <Y5w4/HH71W3O5EOC@nanopsycho>
Date:   Fri, 16 Dec 2022 10:23:08 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
        pabeni@...hat.com, jacob.e.keller@...el.com, leon@...nel.org
Subject: Re: [RFC net-next 14/15] devlink: add by-instance dump infra

Thu, Dec 15, 2022 at 08:47:06PM CET, kuba@...nel.org wrote:
>On Thu, 15 Dec 2022 10:11:03 +0100 Jiri Pirko wrote:
>> Instead of having this extra list of ops struct, woudn't it make sence
>> to rather implement this dumpit_one infra directly as a part of generic
>> netlink code?
>
>I was wondering about that, but none of the ideas were sufficiently
>neat to implement :( There's a lot of improvements that can be done
>in the core, starting with making more of the info structures shared
>between do and dump in genl :( 
>
>> Something like:
>> 
>>  	{
>>  		.cmd = DEVLINK_CMD_RATE_GET,
>>  		.doit = devlink_nl_cmd_rate_get_doit,
>> 		.dumpit_one = devlink_nl_cmd_rate_get_dumpit_one,
>> 		.dumpit_one_walk = devlink_nl_dumpit_one_walk,
>>  		.internal_flags = DEVLINK_NL_FLAG_NEED_RATE,
>>  		/* can be retrieved by unprivileged users */
>>  	},
>
>Growing the struct ops (especially the one called _small_) may be 
>a hard sale for a single user. For split ops, it's a different story,
>because we can possibly have a flag that changes the interpretation
>of the union. Maybe.
>
>I'd love to have a way of breaking down the ops so that we can factor
>out the filling of the message (the code that is shared between doit
>and dump). Just for the walk I don't think it's worth it.

Okay, that is something I thought about as well. Let me take a stab at
it.


>
>I went in the same direction as ethtool because if over time we arrive
>at a similar structure we can use that as a corner stone.
>
>All in all, I think this patch is a reasonable step forward. 

Yeah, could be always changed...


>But definitely agree that the genl infra is still painfully basic.