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@intel.com wrote:

quoted

On 1/5/2023 1:24 AM, Jiri Pirko wrote:

quoted

Thu, Jan 05, 2023 at 05:05:30AM CET, kuba@kernel.org wrote:

quoted

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@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.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