From 78c96d7b7c9a6aa41d109ca9b040c3955db90c88 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:29 +0200 Subject: [PATCH 01/12] netlink: specs: add dump-strict flag for dont-validate property Allow user to specify GENL_DONT_VALIDATE_DUMP_STRICT flag for validation and add this flag to netlink spec schema. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-2-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- Documentation/netlink/genetlink-c.yaml | 2 +- Documentation/netlink/genetlink-legacy.yaml | 2 +- Documentation/netlink/genetlink.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/netlink/genetlink-c.yaml b/Documentation/netlink/genetlink-c.yaml index 57d1c1c4918f..4c1f8c22627b 100644 --- a/Documentation/netlink/genetlink-c.yaml +++ b/Documentation/netlink/genetlink-c.yaml @@ -274,7 +274,7 @@ properties: description: Kernel attribute validation flags. type: array items: - enum: [ strict, dump ] + enum: [ strict, dump, dump-strict ] do: &subop-type description: Main command handler. type: object diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml index 43b769c98fb2..196076dfa309 100644 --- a/Documentation/netlink/genetlink-legacy.yaml +++ b/Documentation/netlink/genetlink-legacy.yaml @@ -321,7 +321,7 @@ properties: description: Kernel attribute validation flags. type: array items: - enum: [ strict, dump ] + enum: [ strict, dump, dump-strict ] # Start genetlink-legacy fixed-header: *fixed-header # End genetlink-legacy diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml index 1cbb448d2f1c..3d338c48bf21 100644 --- a/Documentation/netlink/genetlink.yaml +++ b/Documentation/netlink/genetlink.yaml @@ -243,7 +243,7 @@ properties: description: Kernel attribute validation flags. type: array items: - enum: [ strict, dump ] + enum: [ strict, dump, dump-strict ] do: &subop-type description: Main command handler. type: object From dc7b81a828db0d9801e7977edff2735d660937a8 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:30 +0200 Subject: [PATCH 02/12] ynl-gen-c.py: filter rendering of validate field values for split ops For split ops, do and dump has different meaningful values in validate field. Fix the rendering to allow the values per op type as follows: do: strict dump: dump, strict-dump Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-3-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- tools/net/ynl/ynl-gen-c.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index 650be9b8b693..a3f70ca929fb 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -1988,9 +1988,17 @@ def print_kernel_op_table(family, cw): cw.block_start() members = [('cmd', op.enum_name)] if 'dont-validate' in op: + dont_validate = [] + for x in op['dont-validate']: + if op_mode == 'do' and x in ['dump', 'dump-strict']: + continue + if op_mode == "dump" and x == 'strict': + continue + dont_validate.append(x) + members.append(('validate', ' | '.join([c_upper('genl-dont-validate-' + x) - for x in op['dont-validate']])), ) + for x in dont_validate])), ) name = c_lower(f"{family.name}-nl-{op_name}-{op_mode}it") if 'pre' in op[op_mode]: members.append((cb_names[op_mode]['pre'], c_lower(op[op_mode]['pre']))) From eab7be688b4416588d19ee1fe5edbfa79a639b42 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:31 +0200 Subject: [PATCH 03/12] ynl-gen-c.py: allow directional model for kernel mode Directional model limitation is only applicable for uapi mode. For kernel mode, the code is generated correctly using right cmd values for do/dump requests. Lift the limitation. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-4-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- tools/net/ynl/ynl-gen-c.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index a3f70ca929fb..2ea6af892b68 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -2317,7 +2317,7 @@ def main(): return supported_models = ['unified'] - if args.mode == 'user': + if args.mode in ['user', 'kernel']: supported_models += ['directional'] if parsed.msg_id_model not in supported_models: print(f'Message enum-model {parsed.msg_id_model} not supported for {args.mode} generation') From fa8ba3502ade9e4a8e447066b0d94261fbbe1906 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:32 +0200 Subject: [PATCH 04/12] ynl-gen-c.py: render netlink policies static for split ops When policies are rendered for split ops, they are consumed in the same file. No need to expose them for user outside, make them static. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-5-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- tools/net/ynl/ynl-gen-c.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index 2ea6af892b68..e64311331726 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -1843,13 +1843,13 @@ def print_ntf_type_free(ri): def print_req_policy_fwd(cw, struct, ri=None, terminate=True): - if terminate and ri and kernel_can_gen_family_struct(struct.family): + if terminate and ri and policy_should_be_static(struct.family): return if terminate: prefix = 'extern ' else: - if kernel_can_gen_family_struct(struct.family) and ri: + if ri and policy_should_be_static(struct.family): prefix = 'static ' else: prefix = '' @@ -1877,6 +1877,10 @@ def kernel_can_gen_family_struct(family): return family.proto == 'genetlink' +def policy_should_be_static(family): + return family.kernel_policy == 'split' or kernel_can_gen_family_struct(family) + + def print_kernel_op_table_fwd(family, cw, terminate): exported = not kernel_can_gen_family_struct(family) From ba0f66c95fa670d99ad40e4a6c1ede425446a291 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:33 +0200 Subject: [PATCH 05/12] devlink: rename devlink_nl_ops to devlink_nl_small_ops In order to avoid name collision with the generated split ops array which is going to be introduced as a follow-up patch, rename the existing ops array to devlink_nl_small_ops. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-6-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 2 +- net/devlink/leftover.c | 2 +- net/devlink/netlink.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index 62921b2eb0d3..c67f074641d4 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -119,7 +119,7 @@ struct devlink_cmd { struct netlink_callback *cb); }; -extern const struct genl_small_ops devlink_nl_ops[56]; +extern const struct genl_small_ops devlink_nl_small_ops[56]; struct devlink * devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs); diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 5128b9c7eea8..8f42f1f45705 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -6278,7 +6278,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb, return devlink_trap_policer_set(devlink, policer_item, info); } -const struct genl_small_ops devlink_nl_ops[56] = { +const struct genl_small_ops devlink_nl_small_ops[56] = { { .cmd = DEVLINK_CMD_GET, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index 7a332eb70f70..82a3238d5344 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -243,8 +243,8 @@ struct genl_family devlink_nl_family __ro_after_init = { .pre_doit = devlink_nl_pre_doit, .post_doit = devlink_nl_post_doit, .module = THIS_MODULE, - .small_ops = devlink_nl_ops, - .n_small_ops = ARRAY_SIZE(devlink_nl_ops), + .small_ops = devlink_nl_small_ops, + .n_small_ops = ARRAY_SIZE(devlink_nl_small_ops), .resv_start_op = DEVLINK_CMD_SELFTESTS_RUN + 1, .mcgrps = devlink_nl_mcgrps, .n_mcgrps = ARRAY_SIZE(devlink_nl_mcgrps), From d61aedcf628ef7467d69638c3b3e5e33cafd75aa Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:34 +0200 Subject: [PATCH 06/12] devlink: rename couple of doit netlink callbacks to match generated names The generated names of the doit netlink callback are missing "cmd" in their names. Change names to be ready to switch to generated split ops header. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-7-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- net/devlink/dev.c | 4 ++-- net/devlink/devl_internal.h | 4 ++-- net/devlink/leftover.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/net/devlink/dev.c b/net/devlink/dev.c index bf1d6f1bcfc7..2e120b3da220 100644 --- a/net/devlink/dev.c +++ b/net/devlink/dev.c @@ -196,7 +196,7 @@ void devlink_notify(struct devlink *devlink, enum devlink_command cmd) msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL); } -int devlink_nl_cmd_get_doit(struct sk_buff *skb, struct genl_info *info) +int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info) { struct devlink *devlink = info->user_ptr[0]; struct sk_buff *msg; @@ -804,7 +804,7 @@ err_cancel_msg: return err; } -int devlink_nl_cmd_info_get_doit(struct sk_buff *skb, struct genl_info *info) +int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info) { struct devlink *devlink = info->user_ptr[0]; struct sk_buff *msg; diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index c67f074641d4..d32a32705430 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -214,11 +214,11 @@ struct devlink_rate * devlink_rate_node_get_from_info(struct devlink *devlink, struct genl_info *info); /* Devlink nl cmds */ -int devlink_nl_cmd_get_doit(struct sk_buff *skb, struct genl_info *info); +int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_eswitch_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info); -int devlink_nl_cmd_info_get_doit(struct sk_buff *skb, struct genl_info *info); +int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_flash_update(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_selftests_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_selftests_run(struct sk_buff *skb, struct genl_info *info); diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 8f42f1f45705..094cd0e8224e 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -6282,7 +6282,7 @@ const struct genl_small_ops devlink_nl_small_ops[56] = { { .cmd = DEVLINK_CMD_GET, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, - .doit = devlink_nl_cmd_get_doit, + .doit = devlink_nl_get_doit, .dumpit = devlink_nl_instance_iter_dumpit, /* can be retrieved by unprivileged users */ }, @@ -6536,7 +6536,7 @@ const struct genl_small_ops devlink_nl_small_ops[56] = { { .cmd = DEVLINK_CMD_INFO_GET, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, - .doit = devlink_nl_cmd_info_get_doit, + .doit = devlink_nl_info_get_doit, .dumpit = devlink_nl_instance_iter_dumpit, /* can be retrieved by unprivileged users */ }, From 491a24872a64cbb8edf996b6113a97586702e5ca Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:35 +0200 Subject: [PATCH 07/12] devlink: introduce couple of dumpit callbacks for split ops Introduce couple of dumpit callbacks for generated split ops. Have them as a thin wrapper around iteration function and allow to pass dump_one() function pointer directly without need to store in devlink_cmd structs. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-8-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- net/devlink/dev.c | 22 ++++++++++++---------- net/devlink/devl_internal.h | 17 +++++++++++------ net/devlink/leftover.c | 4 ++-- net/devlink/netlink.c | 21 ++++++++++++--------- 4 files changed, 37 insertions(+), 27 deletions(-) diff --git a/net/devlink/dev.c b/net/devlink/dev.c index 2e120b3da220..5dfba2248b90 100644 --- a/net/devlink/dev.c +++ b/net/devlink/dev.c @@ -217,17 +217,18 @@ int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info) } static int -devlink_nl_cmd_get_dump_one(struct sk_buff *msg, struct devlink *devlink, - struct netlink_callback *cb) +devlink_nl_get_dump_one(struct sk_buff *msg, struct devlink *devlink, + struct netlink_callback *cb) { return devlink_nl_fill(msg, devlink, DEVLINK_CMD_NEW, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, NLM_F_MULTI); } -const struct devlink_cmd devl_cmd_get = { - .dump_one = devlink_nl_cmd_get_dump_one, -}; +int devlink_nl_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb) +{ + return devlink_nl_dumpit(msg, cb, devlink_nl_get_dump_one); +} static void devlink_reload_failed_set(struct devlink *devlink, bool reload_failed) @@ -826,8 +827,8 @@ int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info) } static int -devlink_nl_cmd_info_get_dump_one(struct sk_buff *msg, struct devlink *devlink, - struct netlink_callback *cb) +devlink_nl_info_get_dump_one(struct sk_buff *msg, struct devlink *devlink, + struct netlink_callback *cb) { int err; @@ -840,9 +841,10 @@ devlink_nl_cmd_info_get_dump_one(struct sk_buff *msg, struct devlink *devlink, return err; } -const struct devlink_cmd devl_cmd_info_get = { - .dump_one = devlink_nl_cmd_info_get_dump_one, -}; +int devlink_nl_info_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb) +{ + return devlink_nl_dumpit(msg, cb, devlink_nl_info_get_dump_one); +} static int devlink_nl_flash_update_fill(struct sk_buff *msg, struct devlink *devlink, diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index d32a32705430..7d0a9dd1aceb 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -114,9 +114,12 @@ struct devlink_nl_dump_state { }; }; +typedef int devlink_nl_dump_one_func_t(struct sk_buff *msg, + struct devlink *devlink, + struct netlink_callback *cb); + struct devlink_cmd { - int (*dump_one)(struct sk_buff *msg, struct devlink *devlink, - struct netlink_callback *cb); + devlink_nl_dump_one_func_t *dump_one; }; extern const struct genl_small_ops devlink_nl_small_ops[56]; @@ -127,8 +130,9 @@ devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs); void devlink_notify_unregister(struct devlink *devlink); void devlink_notify_register(struct devlink *devlink); -int devlink_nl_instance_iter_dumpit(struct sk_buff *msg, - struct netlink_callback *cb); +int devlink_nl_dumpit(struct sk_buff *msg, struct netlink_callback *cb, + devlink_nl_dump_one_func_t *dump_one); +int devlink_nl_instance_iter_dumpit(struct sk_buff *msg, struct netlink_callback *cb); static inline struct devlink_nl_dump_state * devlink_dump_state(struct netlink_callback *cb) @@ -149,7 +153,6 @@ devlink_nl_put_handle(struct sk_buff *msg, struct devlink *devlink) } /* Commands */ -extern const struct devlink_cmd devl_cmd_get; extern const struct devlink_cmd devl_cmd_port_get; extern const struct devlink_cmd devl_cmd_sb_get; extern const struct devlink_cmd devl_cmd_sb_pool_get; @@ -157,7 +160,6 @@ extern const struct devlink_cmd devl_cmd_sb_port_pool_get; extern const struct devlink_cmd devl_cmd_sb_tc_pool_bind_get; extern const struct devlink_cmd devl_cmd_param_get; extern const struct devlink_cmd devl_cmd_region_get; -extern const struct devlink_cmd devl_cmd_info_get; extern const struct devlink_cmd devl_cmd_health_reporter_get; extern const struct devlink_cmd devl_cmd_trap_get; extern const struct devlink_cmd devl_cmd_trap_group_get; @@ -215,10 +217,13 @@ devlink_rate_node_get_from_info(struct devlink *devlink, struct genl_info *info); /* Devlink nl cmds */ int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info); +int devlink_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_eswitch_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info); +int devlink_nl_info_get_dumpit(struct sk_buff *skb, + struct netlink_callback *cb); int devlink_nl_cmd_flash_update(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_selftests_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_selftests_run(struct sk_buff *skb, struct genl_info *info); diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 094cd0e8224e..895b732bed8e 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -6283,7 +6283,7 @@ const struct genl_small_ops devlink_nl_small_ops[56] = { .cmd = DEVLINK_CMD_GET, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, .doit = devlink_nl_get_doit, - .dumpit = devlink_nl_instance_iter_dumpit, + .dumpit = devlink_nl_get_dumpit, /* can be retrieved by unprivileged users */ }, { @@ -6537,7 +6537,7 @@ const struct genl_small_ops devlink_nl_small_ops[56] = { .cmd = DEVLINK_CMD_INFO_GET, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, .doit = devlink_nl_info_get_doit, - .dumpit = devlink_nl_instance_iter_dumpit, + .dumpit = devlink_nl_info_get_dumpit, /* can be retrieved by unprivileged users */ }, { diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index 82a3238d5344..624d0db7e229 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -178,7 +178,6 @@ static void devlink_nl_post_doit(const struct genl_split_ops *ops, } static const struct devlink_cmd *devl_cmds[] = { - [DEVLINK_CMD_GET] = &devl_cmd_get, [DEVLINK_CMD_PORT_GET] = &devl_cmd_port_get, [DEVLINK_CMD_SB_GET] = &devl_cmd_sb_get, [DEVLINK_CMD_SB_POOL_GET] = &devl_cmd_sb_pool_get, @@ -186,7 +185,6 @@ static const struct devlink_cmd *devl_cmds[] = { [DEVLINK_CMD_SB_TC_POOL_BIND_GET] = &devl_cmd_sb_tc_pool_bind_get, [DEVLINK_CMD_PARAM_GET] = &devl_cmd_param_get, [DEVLINK_CMD_REGION_GET] = &devl_cmd_region_get, - [DEVLINK_CMD_INFO_GET] = &devl_cmd_info_get, [DEVLINK_CMD_HEALTH_REPORTER_GET] = &devl_cmd_health_reporter_get, [DEVLINK_CMD_TRAP_GET] = &devl_cmd_trap_get, [DEVLINK_CMD_TRAP_GROUP_GET] = &devl_cmd_trap_group_get, @@ -196,23 +194,19 @@ static const struct devlink_cmd *devl_cmds[] = { [DEVLINK_CMD_SELFTESTS_GET] = &devl_cmd_selftests_get, }; -int devlink_nl_instance_iter_dumpit(struct sk_buff *msg, - struct netlink_callback *cb) +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 devlink_nl_dump_state *state = devlink_dump_state(cb); - const struct devlink_cmd *cmd; struct devlink *devlink; int err = 0; - cmd = devl_cmds[info->op.cmd]; - while ((devlink = devlinks_xa_find_get(sock_net(msg->sk), &state->instance))) { devl_lock(devlink); if (devl_is_registered(devlink)) - err = cmd->dump_one(msg, devlink, cb); + err = dump_one(msg, devlink, cb); else err = 0; @@ -233,6 +227,15 @@ int devlink_nl_instance_iter_dumpit(struct sk_buff *msg, return msg->len; } +int devlink_nl_instance_iter_dumpit(struct sk_buff *msg, + struct netlink_callback *cb) +{ + const struct genl_dumpit_info *info = genl_dumpit_info(cb); + const struct devlink_cmd *cmd = devl_cmds[info->op.cmd]; + + return devlink_nl_dumpit(msg, cb, cmd->dump_one); +} + struct genl_family devlink_nl_family __ro_after_init = { .name = DEVLINK_GENL_NAME, .version = DEVLINK_GENL_VERSION, From 8300dce542e45c50ef318c4493bbb2e7966a883c Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:36 +0200 Subject: [PATCH 08/12] devlink: un-static devlink_nl_pre/post_doit() To be prepared for the follow-up generated split ops addition, make the functions devlink_nl_pre_doit() and devlink_nl_post_doit() usable outside of netlink.c. Introduce temporary prototypes which are going to be removed once the generated header will be included. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-9-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 4 ++++ net/devlink/netlink.c | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index 7d0a9dd1aceb..0befa1869fde 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -216,6 +216,10 @@ struct devlink_rate * devlink_rate_node_get_from_info(struct devlink *devlink, struct genl_info *info); /* Devlink nl cmds */ +int devlink_nl_pre_doit(const struct genl_split_ops *ops, + struct sk_buff *skb, struct genl_info *info); +void devlink_nl_post_doit(const struct genl_split_ops *ops, + struct sk_buff *skb, struct genl_info *info); int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info); diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index 624d0db7e229..98d5c6b0acd8 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -109,8 +109,8 @@ devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs) return ERR_PTR(-ENODEV); } -static int devlink_nl_pre_doit(const struct genl_split_ops *ops, - struct sk_buff *skb, struct genl_info *info) +int devlink_nl_pre_doit(const struct genl_split_ops *ops, + struct sk_buff *skb, struct genl_info *info) { struct devlink_linecard *linecard; struct devlink_port *devlink_port; @@ -167,8 +167,8 @@ unlock: return err; } -static void devlink_nl_post_doit(const struct genl_split_ops *ops, - struct sk_buff *skb, struct genl_info *info) +void devlink_nl_post_doit(const struct genl_split_ops *ops, + struct sk_buff *skb, struct genl_info *info) { struct devlink *devlink; From 759f661012d19e61e8d7e8d2bc8d4395ba45a689 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:37 +0200 Subject: [PATCH 09/12] netlink: specs: devlink: add info-get dump op Add missing dump op for info-get command and re-generate related devlink-user.[ch] code. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-10-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- Documentation/netlink/specs/devlink.yaml | 4 +- tools/net/ynl/generated/devlink-user.c | 53 ++++++++++++++++++++++++ tools/net/ynl/generated/devlink-user.h | 10 +++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml index 5d46ca966979..12699b7ce292 100644 --- a/Documentation/netlink/specs/devlink.yaml +++ b/Documentation/netlink/specs/devlink.yaml @@ -194,7 +194,7 @@ operations: request: value: 51 attributes: *dev-id-attrs - reply: + reply: &info-get-reply value: 51 attributes: - bus-name @@ -204,3 +204,5 @@ operations: - info-version-fixed - info-version-running - info-version-stored + dump: + reply: *info-get-reply diff --git a/tools/net/ynl/generated/devlink-user.c b/tools/net/ynl/generated/devlink-user.c index 939bd45feaca..8492789433b9 100644 --- a/tools/net/ynl/generated/devlink-user.c +++ b/tools/net/ynl/generated/devlink-user.c @@ -716,6 +716,59 @@ err_free: return NULL; } +/* DEVLINK_CMD_INFO_GET - dump */ +void devlink_info_get_list_free(struct devlink_info_get_list *rsp) +{ + struct devlink_info_get_list *next = rsp; + + while ((void *)next != YNL_LIST_END) { + unsigned int i; + + rsp = next; + next = rsp->next; + + free(rsp->obj.bus_name); + free(rsp->obj.dev_name); + free(rsp->obj.info_driver_name); + free(rsp->obj.info_serial_number); + for (i = 0; i < rsp->obj.n_info_version_fixed; i++) + devlink_dl_info_version_free(&rsp->obj.info_version_fixed[i]); + free(rsp->obj.info_version_fixed); + for (i = 0; i < rsp->obj.n_info_version_running; i++) + devlink_dl_info_version_free(&rsp->obj.info_version_running[i]); + free(rsp->obj.info_version_running); + for (i = 0; i < rsp->obj.n_info_version_stored; i++) + devlink_dl_info_version_free(&rsp->obj.info_version_stored[i]); + free(rsp->obj.info_version_stored); + free(rsp); + } +} + +struct devlink_info_get_list *devlink_info_get_dump(struct ynl_sock *ys) +{ + struct ynl_dump_state yds = {}; + struct nlmsghdr *nlh; + int err; + + yds.ys = ys; + yds.alloc_sz = sizeof(struct devlink_info_get_list); + yds.cb = devlink_info_get_rsp_parse; + yds.rsp_cmd = DEVLINK_CMD_INFO_GET; + yds.rsp_policy = &devlink_nest; + + nlh = ynl_gemsg_start_dump(ys, ys->family_id, DEVLINK_CMD_INFO_GET, 1); + + err = ynl_exec_dump(ys, nlh, &yds); + if (err < 0) + goto free_list; + + return yds.first; + +free_list: + devlink_info_get_list_free(yds.first); + return NULL; +} + const struct ynl_family ynl_devlink_family = { .name = "devlink", }; diff --git a/tools/net/ynl/generated/devlink-user.h b/tools/net/ynl/generated/devlink-user.h index a008b99b6e24..af65e2f2f529 100644 --- a/tools/net/ynl/generated/devlink-user.h +++ b/tools/net/ynl/generated/devlink-user.h @@ -207,4 +207,14 @@ void devlink_info_get_rsp_free(struct devlink_info_get_rsp *rsp); struct devlink_info_get_rsp * devlink_info_get(struct ynl_sock *ys, struct devlink_info_get_req *req); +/* DEVLINK_CMD_INFO_GET - dump */ +struct devlink_info_get_list { + struct devlink_info_get_list *next; + struct devlink_info_get_rsp obj __attribute__ ((aligned (8))); +}; + +void devlink_info_get_list_free(struct devlink_info_get_list *rsp); + +struct devlink_info_get_list *devlink_info_get_dump(struct ynl_sock *ys); + #endif /* _LINUX_DEVLINK_GEN_H */ From 6b7c486cae8136050df8fca2fbebedb685c49b2d Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:38 +0200 Subject: [PATCH 10/12] devlink: add split ops generated according to spec Improve the existing devlink spec in order to serve as a source for generation of valid devlink split ops for the existing commands. Add the generated sources. Node that the policies are narrowed down only to the attributes that are actually parsed. The dont-validate-strict parsing policy makes sure that other possibly passed garbage attributes from userspace are ignored during validation. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-11-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- Documentation/netlink/specs/devlink.yaml | 10 ++++ net/devlink/Makefile | 2 +- net/devlink/netlink_gen.c | 59 ++++++++++++++++++++++++ net/devlink/netlink_gen.h | 29 ++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 net/devlink/netlink_gen.c create mode 100644 net/devlink/netlink_gen.h diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml index 12699b7ce292..f6df0b3fd502 100644 --- a/Documentation/netlink/specs/devlink.yaml +++ b/Documentation/netlink/specs/devlink.yaml @@ -165,8 +165,13 @@ operations: name: get doc: Get devlink instances. attribute-set: devlink + dont-validate: + - strict + - dump do: + pre: devlink-nl-pre-doit + post: devlink-nl-post-doit request: value: 1 attributes: &dev-id-attrs @@ -189,8 +194,13 @@ operations: name: info-get doc: Get device information, like driver name, hardware and firmware versions etc. attribute-set: devlink + dont-validate: + - strict + - dump do: + pre: devlink-nl-pre-doit + post: devlink-nl-post-doit request: value: 51 attributes: *dev-id-attrs diff --git a/net/devlink/Makefile b/net/devlink/Makefile index ef91a76646a3..a087af581847 100644 --- a/net/devlink/Makefile +++ b/net/devlink/Makefile @@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y := leftover.o core.o netlink.o dev.o health.o +obj-y := leftover.o core.o netlink.o netlink_gen.o dev.o health.o diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c new file mode 100644 index 000000000000..32d8cbed0c30 --- /dev/null +++ b/net/devlink/netlink_gen.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) +/* Do not edit directly, auto-generated from: */ +/* Documentation/netlink/specs/devlink.yaml */ +/* YNL-GEN kernel source */ + +#include +#include + +#include "netlink_gen.h" + +#include + +/* DEVLINK_CMD_GET - do */ +static const struct nla_policy devlink_get_nl_policy[DEVLINK_ATTR_DEV_NAME + 1] = { + [DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, }, + [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, }, +}; + +/* DEVLINK_CMD_INFO_GET - do */ +static const struct nla_policy devlink_info_get_nl_policy[DEVLINK_ATTR_DEV_NAME + 1] = { + [DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, }, + [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, }, +}; + +/* Ops table for devlink */ +const struct genl_split_ops devlink_nl_ops[4] = { + { + .cmd = DEVLINK_CMD_GET, + .validate = GENL_DONT_VALIDATE_STRICT, + .pre_doit = devlink_nl_pre_doit, + .doit = devlink_nl_get_doit, + .post_doit = devlink_nl_post_doit, + .policy = devlink_get_nl_policy, + .maxattr = DEVLINK_ATTR_DEV_NAME, + .flags = GENL_CMD_CAP_DO, + }, + { + .cmd = DEVLINK_CMD_GET, + .validate = GENL_DONT_VALIDATE_DUMP, + .dumpit = devlink_nl_get_dumpit, + .flags = GENL_CMD_CAP_DUMP, + }, + { + .cmd = DEVLINK_CMD_INFO_GET, + .validate = GENL_DONT_VALIDATE_STRICT, + .pre_doit = devlink_nl_pre_doit, + .doit = devlink_nl_info_get_doit, + .post_doit = devlink_nl_post_doit, + .policy = devlink_info_get_nl_policy, + .maxattr = DEVLINK_ATTR_DEV_NAME, + .flags = GENL_CMD_CAP_DO, + }, + { + .cmd = DEVLINK_CMD_INFO_GET, + .validate = GENL_DONT_VALIDATE_DUMP, + .dumpit = devlink_nl_info_get_dumpit, + .flags = GENL_CMD_CAP_DUMP, + }, +}; diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h new file mode 100644 index 000000000000..11980e04a718 --- /dev/null +++ b/net/devlink/netlink_gen.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */ +/* Do not edit directly, auto-generated from: */ +/* Documentation/netlink/specs/devlink.yaml */ +/* YNL-GEN kernel header */ + +#ifndef _LINUX_DEVLINK_GEN_H +#define _LINUX_DEVLINK_GEN_H + +#include +#include + +#include + +/* Ops table for devlink */ +extern const struct genl_split_ops devlink_nl_ops[4]; + +int devlink_nl_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb, + struct genl_info *info); +void +devlink_nl_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb, + struct genl_info *info); + +int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info); +int devlink_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); +int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info); +int devlink_nl_info_get_dumpit(struct sk_buff *skb, + struct netlink_callback *cb); + +#endif /* _LINUX_DEVLINK_GEN_H */ From b2551b1517d86ff5e258e7bc834e5be0e1ce08de Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:39 +0200 Subject: [PATCH 11/12] devlink: include the generated netlink header Put the newly added generated header to the include list. Remove the duplicated temporary function prototypes. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-12-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index 0befa1869fde..51de0e1fc769 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -12,6 +12,8 @@ #include #include +#include "netlink_gen.h" + #define DEVLINK_REGISTERED XA_MARK_1 #define DEVLINK_RELOAD_STATS_ARRAY_SIZE \ @@ -216,18 +218,9 @@ struct devlink_rate * devlink_rate_node_get_from_info(struct devlink *devlink, struct genl_info *info); /* Devlink nl cmds */ -int devlink_nl_pre_doit(const struct genl_split_ops *ops, - struct sk_buff *skb, struct genl_info *info); -void devlink_nl_post_doit(const struct genl_split_ops *ops, - struct sk_buff *skb, struct genl_info *info); -int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info); -int devlink_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_eswitch_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info); -int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info); -int devlink_nl_info_get_dumpit(struct sk_buff *skb, - struct netlink_callback *cb); int devlink_nl_cmd_flash_update(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_selftests_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_cmd_selftests_run(struct sk_buff *skb, struct genl_info *info); From 6e067d0cab68a64ef3488b5639a66b65e91b146d Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 3 Aug 2023 13:13:40 +0200 Subject: [PATCH 12/12] devlink: use generated split ops and remove duplicated commands from small ops Do the switch and use generated split ops for get and info_get commands. Remove those from small ops array. Signed-off-by: Jiri Pirko Reviewed-by: Jakub Kicinski Link: https://lore.kernel.org/r/20230803111340.1074067-13-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 2 +- net/devlink/leftover.c | 16 +--------------- net/devlink/netlink.c | 2 ++ 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index 51de0e1fc769..7fdd956ff992 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -124,7 +124,7 @@ struct devlink_cmd { devlink_nl_dump_one_func_t *dump_one; }; -extern const struct genl_small_ops devlink_nl_small_ops[56]; +extern const struct genl_small_ops devlink_nl_small_ops[54]; struct devlink * devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs); diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 895b732bed8e..3bf42f5335ed 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -6278,14 +6278,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb, return devlink_trap_policer_set(devlink, policer_item, info); } -const struct genl_small_ops devlink_nl_small_ops[56] = { - { - .cmd = DEVLINK_CMD_GET, - .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, - .doit = devlink_nl_get_doit, - .dumpit = devlink_nl_get_dumpit, - /* can be retrieved by unprivileged users */ - }, +const struct genl_small_ops devlink_nl_small_ops[54] = { { .cmd = DEVLINK_CMD_PORT_GET, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, @@ -6533,13 +6526,6 @@ const struct genl_small_ops devlink_nl_small_ops[56] = { .dumpit = devlink_nl_cmd_region_read_dumpit, .flags = GENL_ADMIN_PERM, }, - { - .cmd = DEVLINK_CMD_INFO_GET, - .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, - .doit = devlink_nl_info_get_doit, - .dumpit = devlink_nl_info_get_dumpit, - /* can be retrieved by unprivileged users */ - }, { .cmd = DEVLINK_CMD_HEALTH_REPORTER_GET, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index 98d5c6b0acd8..bada2819827b 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -248,6 +248,8 @@ struct genl_family devlink_nl_family __ro_after_init = { .module = THIS_MODULE, .small_ops = devlink_nl_small_ops, .n_small_ops = ARRAY_SIZE(devlink_nl_small_ops), + .split_ops = devlink_nl_ops, + .n_split_ops = ARRAY_SIZE(devlink_nl_ops), .resv_start_op = DEVLINK_CMD_SELFTESTS_RUN + 1, .mcgrps = devlink_nl_mcgrps, .n_mcgrps = ARRAY_SIZE(devlink_nl_mcgrps),