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

Fri, Jan 06, 2023 at 01:16:05AM CET, jacob.e.keller@intel.com wrote:

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.

iterator for whatever reason", so genl feels misleading.

I guess gen is also kind of misleading but I can't think of anything better.

quoted

quoted

+	int (*dump_one)(struct sk_buff *msg, struct devlink *devlink,
+			struct netlink_callback *cb);
+};
+
/* Iterate over registered devlink instances for devlink dump.
 * devlink_put() needs to be called for each iterated devlink pointer
 * in loop body in order to release the reference.

[...]

quoted

@@ -9130,7 +9123,7 @@ const struct genl_small_ops devlink_nl_ops[56] = {

	{
		.cmd = DEVLINK_CMD_RATE_GET,
		.doit = devlink_nl_cmd_rate_get_doit,
-		.dumpit = devlink_nl_cmd_rate_get_dumpit,
+		.dumpit = devlink_nl_instance_iter_dump,

devlink_nl_instance_iter_dumpit to match ".dumpit".

quoted

		.internal_flags = DEVLINK_NL_FLAG_NEED_RATE,
		/* can be retrieved by unprivileged users */
	},

[...]