diff mbox series

[net-next,v3,10/13] devlink: allow user to narrow per-instance dumps by passing handle attrs

Message ID 20230810131539.1602299-11-jiri@resnulli.us (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series devlink: introduce selective dumps | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1332 this patch: 1332
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1355 this patch: 1355
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 63 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jiri Pirko Aug. 10, 2023, 1:15 p.m. UTC 
From: Jiri Pirko <jiri@nvidia.com>

For SFs, one devlink instance per SF is created. There might be
thousands of these on a single host. When a user needs to know port
handle for specific SF, he needs to dump all devlink ports on the host
which does not scale good.

Allow user to pass devlink handle attributes alongside the dump command
and dump only objects which are under selected devlink instance.

Example:
$ devlink port show
auxiliary/mlx5_core.eth.0/65535: type eth netdev eth2 flavour physical port 0 splittable false
auxiliary/mlx5_core.eth.1/131071: type eth netdev eth3 flavour physical port 1 splittable false

$ devlink port show auxiliary/mlx5_core.eth.0
auxiliary/mlx5_core.eth.0/65535: type eth netdev eth2 flavour physical port 0 splittable false

$ devlink port show auxiliary/mlx5_core.eth.1
auxiliary/mlx5_core.eth.1/131071: type eth netdev eth3 flavour physical port 1 splittable false

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- rebased on top of the generated split ops changes
- removed SELECTOR nested attr
- rebased on top of flags arg propagation, added NLM_F_DUMP_FILTERED
- removed no longer needed start() and done() dump callbacks
- adjusted patch subject and description to better match what
  the patch does now
v1->v2:
- extended to patch that covers all dumpit commands
- used start() and done() callback to parse the selector attr
- changed the selector attr netlink policy to be created on fly
- changed patch description accordingly
---
 net/devlink/netlink.c | 45 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

Comments

Jakub Kicinski Aug. 11, 2023, 2:17 a.m. UTC | #1 
On Thu, 10 Aug 2023 15:15:36 +0200 Jiri Pirko wrote:
> +	struct nlattr **attrs = info->attrs;
> +	int flags = NLM_F_MULTI;
> +
> +	if (attrs)
> +		flags |= NLM_F_DUMP_FILTERED;

Are attrs NULL if user passed no TLVs?
TBH I'm not sure how valuable NLM_F_DUMP_FILTERED is in the first place.
Jiri Pirko Aug. 11, 2023, 8:41 a.m. UTC | #2 
Fri, Aug 11, 2023 at 04:17:22AM CEST, kuba@kernel.org wrote:
>On Thu, 10 Aug 2023 15:15:36 +0200 Jiri Pirko wrote:
>> +	struct nlattr **attrs = info->attrs;
>> +	int flags = NLM_F_MULTI;
>> +
>> +	if (attrs)
>> +		flags |= NLM_F_DUMP_FILTERED;
>
>Are attrs NULL if user passed no TLVs?

True, I fixed this for v4.


>TBH I'm not sure how valuable NLM_F_DUMP_FILTERED is in the first place.

Yeah, I just noticed this flag, so I decided to throw it in. I don't see
any harm in that. And I believe it is actually correct to use this flag.
diff mbox series

Patch

diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 47e44fb45815..cd274ae13ae4 100644
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -170,8 +170,29 @@  void devlink_nl_post_doit(const struct genl_split_ops *ops,
 	devlink_put(devlink);
 }
 
-int devlink_nl_dumpit(struct sk_buff *msg, struct netlink_callback *cb,
-		      devlink_nl_dump_one_func_t *dump_one)
+static int devlink_nl_inst_single_dumpit(struct sk_buff *msg,
+					 struct netlink_callback *cb, int flags,
+					 devlink_nl_dump_one_func_t *dump_one,
+					 struct nlattr **attrs)
+{
+	struct devlink *devlink;
+	int err;
+
+	devlink = devlink_get_from_attrs_lock(sock_net(msg->sk), attrs);
+	if (IS_ERR(devlink))
+		return PTR_ERR(devlink);
+	err = dump_one(msg, devlink, cb, flags);
+	devl_unlock(devlink);
+	devlink_put(devlink);
+
+	if (err != -EMSGSIZE)
+		return err;
+	return msg->len;
+}
+
+static int devlink_nl_inst_iter_dumpit(struct sk_buff *msg,
+				       struct netlink_callback *cb, int flags,
+				       devlink_nl_dump_one_func_t *dump_one)
 {
 	struct devlink_nl_dump_state *state = devlink_dump_state(cb);
 	struct devlink *devlink;
@@ -182,7 +203,7 @@  int devlink_nl_dumpit(struct sk_buff *msg, struct netlink_callback *cb,
 		devl_lock(devlink);
 
 		if (devl_is_registered(devlink))
-			err = dump_one(msg, devlink, cb, NLM_F_MULTI);
+			err = dump_one(msg, devlink, cb, flags);
 		else
 			err = 0;
 
@@ -203,6 +224,24 @@  int devlink_nl_dumpit(struct sk_buff *msg, struct netlink_callback *cb,
 	return msg->len;
 }
 
+int devlink_nl_dumpit(struct sk_buff *msg, struct netlink_callback *cb,
+		      devlink_nl_dump_one_func_t *dump_one)
+{
+	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
+	struct nlattr **attrs = info->attrs;
+	int flags = NLM_F_MULTI;
+
+	if (attrs)
+		flags |= NLM_F_DUMP_FILTERED;
+
+	if (attrs &&
+	    (attrs[DEVLINK_ATTR_BUS_NAME] || attrs[DEVLINK_ATTR_DEV_NAME]))
+		return devlink_nl_inst_single_dumpit(msg, cb, flags, dump_one,
+						     attrs);
+	else
+		return devlink_nl_inst_iter_dumpit(msg, cb, flags, dump_one);
+}
+
 struct genl_family devlink_nl_family __ro_after_init = {
 	.name		= DEVLINK_GENL_NAME,
 	.version	= DEVLINK_GENL_VERSION,