From: Jiri Pirko Instead of relying on MNL_TYPE_*, use the values exposed in UAPI. Signed-off-by: Jiri Pirko Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 122 ++++++++++++++++++++++++---------------------- 1 file changed, 65 insertions(+), 57 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index d14f3f45..8195cb2b 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3399,7 +3399,8 @@ static const struct param_val_conv param_val_conv[] = { #define PARAM_VAL_CONV_LEN ARRAY_SIZE(param_val_conv) static void pr_out_param_value(struct dl *dl, const char *nla_name, - int nla_type, struct nlattr *nl) + enum devlink_var_attr_type type, + struct nlattr *nl) { struct nlattr *nla_value[DEVLINK_ATTR_MAX + 1] = {}; struct nlattr *val_attr; @@ -3412,7 +3413,7 @@ static void pr_out_param_value(struct dl *dl, const char *nla_name, return; if (!nla_value[DEVLINK_ATTR_PARAM_VALUE_CMODE] || - (nla_type != MNL_TYPE_FLAG && + (type != DEVLINK_VAR_ATTR_TYPE_FLAG && !nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA])) return; @@ -3425,8 +3426,8 @@ static void pr_out_param_value(struct dl *dl, const char *nla_name, conv_exists = param_val_conv_exists(param_val_conv, PARAM_VAL_CONV_LEN, nla_name); - switch (nla_type) { - case MNL_TYPE_U8: + switch (type) { + case DEVLINK_VAR_ATTR_TYPE_U8: if (conv_exists) { err = param_val_conv_str_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -3441,7 +3442,7 @@ static void pr_out_param_value(struct dl *dl, const char *nla_name, mnl_attr_get_u8(val_attr)); } break; - case MNL_TYPE_U16: + case DEVLINK_VAR_ATTR_TYPE_U16: if (conv_exists) { err = param_val_conv_str_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -3456,7 +3457,7 @@ static void pr_out_param_value(struct dl *dl, const char *nla_name, mnl_attr_get_u16(val_attr)); } break; - case MNL_TYPE_U32: + case DEVLINK_VAR_ATTR_TYPE_U32: if (conv_exists) { err = param_val_conv_str_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -3471,13 +3472,15 @@ static void pr_out_param_value(struct dl *dl, const char *nla_name, mnl_attr_get_u32(val_attr)); } break; - case MNL_TYPE_STRING: + case DEVLINK_VAR_ATTR_TYPE_STRING: print_string(PRINT_ANY, "value", " value %s", mnl_attr_get_str(val_attr)); break; - case MNL_TYPE_FLAG: + case DEVLINK_VAR_ATTR_TYPE_FLAG: print_bool(PRINT_ANY, "value", " value %s", val_attr); break; + default: + break; } } @@ -3486,8 +3489,8 @@ static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array, { struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {}; struct nlattr *param_value_attr; + enum devlink_var_attr_type type; const char *nla_name; - int nla_type; int err; err = mnl_attr_parse_nested(tb[DEVLINK_ATTR_PARAM], attr_cb, nla_param); @@ -3509,7 +3512,7 @@ static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array, else __pr_out_handle_start(dl, tb, true, false); - nla_type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]); + type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]); nla_name = mnl_attr_get_str(nla_param[DEVLINK_ATTR_PARAM_NAME]); check_indent_newline(dl); @@ -3523,7 +3526,7 @@ static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array, mnl_attr_for_each_nested(param_value_attr, nla_param[DEVLINK_ATTR_PARAM_VALUES_LIST]) { pr_out_entry_start(dl); - pr_out_param_value(dl, nla_name, nla_type, param_value_attr); + pr_out_param_value(dl, nla_name, type, param_value_attr); pr_out_entry_end(dl); } pr_out_array_end(dl); @@ -3549,7 +3552,7 @@ static int cmd_dev_param_show_cb(const struct nlmsghdr *nlh, void *data) struct param_ctx { struct dl *dl; - int nla_type; + enum devlink_var_attr_type type; bool cmode_found; union { uint8_t vu8; @@ -3566,10 +3569,10 @@ static int cmd_dev_param_set_cb(const struct nlmsghdr *nlh, void *data) struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {}; struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {}; struct nlattr *param_value_attr; + enum devlink_var_attr_type type; enum devlink_param_cmode cmode; struct param_ctx *ctx = data; struct dl *dl = ctx->dl; - int nla_type; int err; mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb); @@ -3585,7 +3588,7 @@ static int cmd_dev_param_set_cb(const struct nlmsghdr *nlh, void *data) !nla_param[DEVLINK_ATTR_PARAM_VALUES_LIST]) return MNL_CB_ERROR; - nla_type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]); + type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]); mnl_attr_for_each_nested(param_value_attr, nla_param[DEVLINK_ATTR_PARAM_VALUES_LIST]) { struct nlattr *nla_value[DEVLINK_ATTR_MAX + 1] = {}; @@ -3597,7 +3600,7 @@ static int cmd_dev_param_set_cb(const struct nlmsghdr *nlh, void *data) return MNL_CB_ERROR; if (!nla_value[DEVLINK_ATTR_PARAM_VALUE_CMODE] || - (nla_type != MNL_TYPE_FLAG && + (type != DEVLINK_VAR_ATTR_TYPE_FLAG && !nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA])) return MNL_CB_ERROR; @@ -3605,27 +3608,29 @@ static int cmd_dev_param_set_cb(const struct nlmsghdr *nlh, void *data) if (cmode == dl->opts.cmode) { ctx->cmode_found = true; val_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA]; - switch (nla_type) { - case MNL_TYPE_U8: + switch (type) { + case DEVLINK_VAR_ATTR_TYPE_U8: ctx->value.vu8 = mnl_attr_get_u8(val_attr); break; - case MNL_TYPE_U16: + case DEVLINK_VAR_ATTR_TYPE_U16: ctx->value.vu16 = mnl_attr_get_u16(val_attr); break; - case MNL_TYPE_U32: + case DEVLINK_VAR_ATTR_TYPE_U32: ctx->value.vu32 = mnl_attr_get_u32(val_attr); break; - case MNL_TYPE_STRING: + case DEVLINK_VAR_ATTR_TYPE_STRING: ctx->value.vstr = mnl_attr_get_str(val_attr); break; - case MNL_TYPE_FLAG: + case DEVLINK_VAR_ATTR_TYPE_FLAG: ctx->value.vbool = val_attr ? true : false; break; + default: + break; } break; } } - ctx->nla_type = nla_type; + ctx->type = type; return MNL_CB_OK; } @@ -3668,9 +3673,9 @@ static int cmd_dev_param_set(struct dl *dl) conv_exists = param_val_conv_exists(param_val_conv, PARAM_VAL_CONV_LEN, dl->opts.param_name); - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, ctx.nla_type); - switch (ctx.nla_type) { - case MNL_TYPE_U8: + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, ctx.type); + switch (ctx.type) { + case DEVLINK_VAR_ATTR_TYPE_U8: if (conv_exists) { err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -3687,7 +3692,7 @@ static int cmd_dev_param_set(struct dl *dl) return 0; mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u8); break; - case MNL_TYPE_U16: + case DEVLINK_VAR_ATTR_TYPE_U16: if (conv_exists) { err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -3704,7 +3709,7 @@ static int cmd_dev_param_set(struct dl *dl) return 0; mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u16); break; - case MNL_TYPE_U32: + case DEVLINK_VAR_ATTR_TYPE_U32: if (conv_exists) err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -3719,7 +3724,7 @@ static int cmd_dev_param_set(struct dl *dl) return 0; mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u32); break; - case MNL_TYPE_FLAG: + case DEVLINK_VAR_ATTR_TYPE_FLAG: err = strtobool(dl->opts.param_value, &val_bool); if (err) goto err_param_value_parse; @@ -3729,7 +3734,7 @@ static int cmd_dev_param_set(struct dl *dl) mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, 0, NULL); break; - case MNL_TYPE_STRING: + case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, dl->opts.param_value); if (!strcmp(dl->opts.param_value, ctx.value.vstr)) @@ -5143,10 +5148,10 @@ static int cmd_port_param_set_cb(const struct nlmsghdr *nlh, void *data) struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {}; struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {}; struct nlattr *param_value_attr; + enum devlink_var_attr_type type; enum devlink_param_cmode cmode; struct param_ctx *ctx = data; struct dl *dl = ctx->dl; - int nla_type; int err; mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb); @@ -5162,7 +5167,7 @@ static int cmd_port_param_set_cb(const struct nlmsghdr *nlh, void *data) !nla_param[DEVLINK_ATTR_PARAM_VALUES_LIST]) return MNL_CB_ERROR; - nla_type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]); + type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]); mnl_attr_for_each_nested(param_value_attr, nla_param[DEVLINK_ATTR_PARAM_VALUES_LIST]) { struct nlattr *nla_value[DEVLINK_ATTR_MAX + 1] = {}; @@ -5174,34 +5179,36 @@ static int cmd_port_param_set_cb(const struct nlmsghdr *nlh, void *data) return MNL_CB_ERROR; if (!nla_value[DEVLINK_ATTR_PARAM_VALUE_CMODE] || - (nla_type != MNL_TYPE_FLAG && + (type != DEVLINK_VAR_ATTR_TYPE_FLAG && !nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA])) return MNL_CB_ERROR; cmode = mnl_attr_get_u8(nla_value[DEVLINK_ATTR_PARAM_VALUE_CMODE]); if (cmode == dl->opts.cmode) { val_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA]; - switch (nla_type) { - case MNL_TYPE_U8: + switch (type) { + case DEVLINK_VAR_ATTR_TYPE_U8: ctx->value.vu8 = mnl_attr_get_u8(val_attr); break; - case MNL_TYPE_U16: + case DEVLINK_VAR_ATTR_TYPE_U16: ctx->value.vu16 = mnl_attr_get_u16(val_attr); break; - case MNL_TYPE_U32: + case DEVLINK_VAR_ATTR_TYPE_U32: ctx->value.vu32 = mnl_attr_get_u32(val_attr); break; - case MNL_TYPE_STRING: + case DEVLINK_VAR_ATTR_TYPE_STRING: ctx->value.vstr = mnl_attr_get_str(val_attr); break; - case MNL_TYPE_FLAG: + case DEVLINK_VAR_ATTR_TYPE_FLAG: ctx->value.vbool = val_attr ? true : false; break; + default: + break; } break; } } - ctx->nla_type = nla_type; + ctx->type = type; return MNL_CB_OK; } @@ -5240,9 +5247,9 @@ static int cmd_port_param_set(struct dl *dl) conv_exists = param_val_conv_exists(param_val_conv, PARAM_VAL_CONV_LEN, dl->opts.param_name); - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, ctx.nla_type); - switch (ctx.nla_type) { - case MNL_TYPE_U8: + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, ctx.type); + switch (ctx.type) { + case DEVLINK_VAR_ATTR_TYPE_U8: if (conv_exists) { err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -5259,7 +5266,7 @@ static int cmd_port_param_set(struct dl *dl) return 0; mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u8); break; - case MNL_TYPE_U16: + case DEVLINK_VAR_ATTR_TYPE_U16: if (conv_exists) { err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -5276,7 +5283,7 @@ static int cmd_port_param_set(struct dl *dl) return 0; mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u16); break; - case MNL_TYPE_U32: + case DEVLINK_VAR_ATTR_TYPE_U32: if (conv_exists) err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, @@ -5291,7 +5298,7 @@ static int cmd_port_param_set(struct dl *dl) return 0; mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u32); break; - case MNL_TYPE_FLAG: + case DEVLINK_VAR_ATTR_TYPE_FLAG: err = strtobool(dl->opts.param_value, &val_bool); if (err) goto err_param_value_parse; @@ -5301,7 +5308,7 @@ static int cmd_port_param_set(struct dl *dl) mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, 0, NULL); break; - case MNL_TYPE_STRING: + case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, dl->opts.param_value); if (!strcmp(dl->opts.param_value, ctx.value.vstr)) @@ -9136,7 +9143,8 @@ static int cmd_health_dump_clear(struct dl *dl) return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL); } -static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data) +static int fmsg_value_show(struct dl *dl, enum devlink_var_attr_type type, + struct nlattr *nl_data) { const char *num_fmt = dl->hex ? "%#x" : "%u"; const char *num64_fmt = dl->hex ? "%#"PRIx64 : "%"PRIu64; @@ -9145,25 +9153,25 @@ static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data) check_indent_newline(dl); switch (type) { - case MNL_TYPE_FLAG: + case DEVLINK_VAR_ATTR_TYPE_FLAG: print_bool(PRINT_ANY, NULL, "%s", mnl_attr_get_u8(nl_data)); break; - case MNL_TYPE_U8: + case DEVLINK_VAR_ATTR_TYPE_U8: print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u8(nl_data)); break; - case MNL_TYPE_U16: + case DEVLINK_VAR_ATTR_TYPE_U16: print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u16(nl_data)); break; - case MNL_TYPE_U32: + case DEVLINK_VAR_ATTR_TYPE_U32: print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u32(nl_data)); break; - case MNL_TYPE_U64: + case DEVLINK_VAR_ATTR_TYPE_U64: print_u64(PRINT_ANY, NULL, num64_fmt, mnl_attr_get_u64(nl_data)); break; - case MNL_TYPE_NUL_STRING: + case DEVLINK_VAR_ATTR_TYPE_NUL_STRING: print_string(PRINT_ANY, NULL, "%s", mnl_attr_get_str(nl_data)); break; - case MNL_TYPE_BINARY: + case DEVLINK_VAR_ATTR_TYPE_BINARY: len = mnl_attr_get_payload_len(nl_data); data = mnl_attr_get_payload(nl_data); pr_out_binary_value(dl, data, len); @@ -9192,7 +9200,7 @@ struct nest_entry { struct fmsg_cb_data { char *name; struct dl *dl; - uint8_t value_type; + enum devlink_var_attr_type type; struct list_head entry_list; }; @@ -9338,11 +9346,11 @@ static int cmd_fmsg_object_cb(const struct nlmsghdr *nlh, void *data) return -ENOMEM; break; case DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE: - fmsg_data->value_type = mnl_attr_get_u8(nla_object); + fmsg_data->type = mnl_attr_get_u8(nla_object); break; case DEVLINK_ATTR_FMSG_OBJ_VALUE_DATA: pr_out_fmsg_name(dl, &fmsg_data->name); - err = fmsg_value_show(dl, fmsg_data->value_type, + err = fmsg_value_show(dl, fmsg_data->type, nla_object); if (err != MNL_CB_OK) return err; -- 2.50.0 From: Saeed Mahameed Param value attribute DEVLINK_ATTR_PARAM_VALUE_DATA can be passed to/from kernel as type DEVLINK_VAR_ATTR_TYPE_U32_ARRAY with encoded data of U32 list of values. Handle this case by outputting the value as comma separated list or json list objects for get/dump requests. example: $ devlink dev param show name foo name foo type driver-specific values: cmode permanent value: 1,2,3,4,5,6,7,8 Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 77 ++++++++++++++++++++++++++++++++++++ include/uapi/linux/devlink.h | 1 + 2 files changed, 78 insertions(+) diff --git a/devlink/devlink.c b/devlink/devlink.c index 8195cb2b..2abf8ff7 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -556,6 +556,30 @@ static void pr_out_array_end(struct dl *dl) } } +static void pr_out_val_array_start(struct dl *dl, const char *name, + const char *delimeter) +{ + if (dl->json_output) { + open_json_array(PRINT_JSON, name); + } else { + __pr_out_indent_inc(); + pr_out(" %s:", name); + if (delimeter) + pr_out("%s", delimeter); + __pr_out_indent_inc(); + } +} + +static void pr_out_val_array_end(struct dl *dl) +{ + if (dl->json_output) { + close_json_array(PRINT_JSON, NULL); + } else { + __pr_out_indent_dec(); + __pr_out_indent_dec(); + } +} + static void pr_out_object_start(struct dl *dl, const char *name) { if (dl->json_output) { @@ -3396,6 +3420,41 @@ static const struct param_val_conv param_val_conv[] = { }, }; +struct dl_param_val_list { + size_t len; + uint32_t vu32[]; +}; + +/* Parse nested param value list + * @val_list_attr: nested attribute containing the list of values + * usually : val_list_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA] + * @list: pointer to the list of values, reallocated to the new size + * Returns: 0 on success, -errno on failure + */ +static int +dl_mnl_parse_param_val_nested(struct nlattr *val_list_attr, + struct dl_param_val_list **list) +{ + struct dl_param_val_list *new_list; + struct nlattr *val_attr; + int i = 0, len = 0; + + len = mnl_attr_get_payload_len(val_list_attr)/(MNL_ATTR_HDRLEN + sizeof(uint32_t)); + if (!len) + return -EINVAL; + + new_list = realloc(*list, sizeof(new_list) + len * sizeof(uint32_t)); + if (!new_list) + return -ENOMEM; + + mnl_attr_for_each_nested(val_attr, val_list_attr) + new_list->vu32[i++] = mnl_attr_get_u32(val_attr); + + new_list->len = i; + *list = new_list; + return 0; +} + #define PARAM_VAL_CONV_LEN ARRAY_SIZE(param_val_conv) static void pr_out_param_value(struct dl *dl, const char *nla_name, @@ -3479,6 +3538,24 @@ static void pr_out_param_value(struct dl *dl, const char *nla_name, case DEVLINK_VAR_ATTR_TYPE_FLAG: print_bool(PRINT_ANY, "value", " value %s", val_attr); break; + case DEVLINK_VAR_ATTR_TYPE_U32_ARRAY: { + struct dl_param_val_list *list = NULL; + int err; + int i; + + err = dl_mnl_parse_param_val_nested(val_attr, &list); + if (err) + return; + + pr_out_val_array_start(dl, "value", " "); + + for (i = 0; i < list->len - 1; i++) + print_uint(PRINT_ANY, NULL, "%u,", list->vu32[i]); + print_uint(PRINT_ANY, NULL, "%u", list->vu32[i]); + pr_out_val_array_end(dl); + free(list); + break; + } default: break; } diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h index 9a1bdc94..a22fc073 100644 --- a/include/uapi/linux/devlink.h +++ b/include/uapi/linux/devlink.h @@ -398,6 +398,7 @@ enum devlink_var_attr_type { DEVLINK_VAR_ATTR_TYPE_BINARY, __DEVLINK_VAR_ATTR_TYPE_CUSTOM_BASE = 0x80, /* Any possible custom types, unrelated to NLA_* values go below */ + DEVLINK_VAR_ATTR_TYPE_U32_ARRAY, }; enum devlink_attr { -- 2.50.0 From: Saeed Mahameed cmd_dev_param_set_cb and cmd_port_param_set_cb are almost identical, except the DEVLINK_ATTR_PORT_INDEX part, which is easily identifiable in cmd_dev_param_set_cb. Check for port handle and port index attribute in cmd_dev_param_set_cb then we can reuse it for cmd_port_param_set. This allows single location for param values attribute parsing for set operations. Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 79 ++++------------------------------------------- 1 file changed, 6 insertions(+), 73 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index 2abf8ff7..fb89757c 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3640,7 +3640,7 @@ struct param_ctx { } value; }; -static int cmd_dev_param_set_cb(const struct nlmsghdr *nlh, void *data) +static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) { struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh); struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {}; @@ -3657,6 +3657,9 @@ static int cmd_dev_param_set_cb(const struct nlmsghdr *nlh, void *data) !tb[DEVLINK_ATTR_PARAM]) return MNL_CB_ERROR; + if ((dl->opts.present & DL_OPT_HANDLEP) && !tb[DEVLINK_ATTR_PORT_INDEX]) + return MNL_CB_ERROR; + err = mnl_attr_parse_nested(tb[DEVLINK_ATTR_PARAM], attr_cb, nla_param); if (err != MNL_CB_OK) return MNL_CB_ERROR; @@ -3735,7 +3738,7 @@ static int cmd_dev_param_set(struct dl *dl) dl_opts_put(nlh, dl); ctx.dl = dl; - err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_dev_param_set_cb, &ctx); + err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_param_set_cb, &ctx); if (err) return err; if (!ctx.cmode_found) { @@ -5219,76 +5222,6 @@ static int cmd_port_function_set(struct dl *dl) return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL); } -static int cmd_port_param_set_cb(const struct nlmsghdr *nlh, void *data) -{ - struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh); - struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {}; - struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {}; - struct nlattr *param_value_attr; - enum devlink_var_attr_type type; - enum devlink_param_cmode cmode; - struct param_ctx *ctx = data; - struct dl *dl = ctx->dl; - int err; - - mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb); - if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] || - !tb[DEVLINK_ATTR_PORT_INDEX] || !tb[DEVLINK_ATTR_PARAM]) - return MNL_CB_ERROR; - - err = mnl_attr_parse_nested(tb[DEVLINK_ATTR_PARAM], attr_cb, nla_param); - if (err != MNL_CB_OK) - return MNL_CB_ERROR; - - if (!nla_param[DEVLINK_ATTR_PARAM_TYPE] || - !nla_param[DEVLINK_ATTR_PARAM_VALUES_LIST]) - return MNL_CB_ERROR; - - type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]); - mnl_attr_for_each_nested(param_value_attr, - nla_param[DEVLINK_ATTR_PARAM_VALUES_LIST]) { - struct nlattr *nla_value[DEVLINK_ATTR_MAX + 1] = {}; - struct nlattr *val_attr; - - err = mnl_attr_parse_nested(param_value_attr, - attr_cb, nla_value); - if (err != MNL_CB_OK) - return MNL_CB_ERROR; - - if (!nla_value[DEVLINK_ATTR_PARAM_VALUE_CMODE] || - (type != DEVLINK_VAR_ATTR_TYPE_FLAG && - !nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA])) - return MNL_CB_ERROR; - - cmode = mnl_attr_get_u8(nla_value[DEVLINK_ATTR_PARAM_VALUE_CMODE]); - if (cmode == dl->opts.cmode) { - val_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA]; - switch (type) { - case DEVLINK_VAR_ATTR_TYPE_U8: - ctx->value.vu8 = mnl_attr_get_u8(val_attr); - break; - case DEVLINK_VAR_ATTR_TYPE_U16: - ctx->value.vu16 = mnl_attr_get_u16(val_attr); - break; - case DEVLINK_VAR_ATTR_TYPE_U32: - ctx->value.vu32 = mnl_attr_get_u32(val_attr); - break; - case DEVLINK_VAR_ATTR_TYPE_STRING: - ctx->value.vstr = mnl_attr_get_str(val_attr); - break; - case DEVLINK_VAR_ATTR_TYPE_FLAG: - ctx->value.vbool = val_attr ? true : false; - break; - default: - break; - } - break; - } - } - ctx->type = type; - return MNL_CB_OK; -} - static int cmd_port_param_set(struct dl *dl) { struct param_ctx ctx = {}; @@ -5313,7 +5246,7 @@ static int cmd_port_param_set(struct dl *dl) dl_opts_put(nlh, dl); ctx.dl = dl; - err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_port_param_set_cb, &ctx); + err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_param_set_cb, &ctx); if (err) return err; -- 2.50.0 From: Saeed Mahameed dl_param structure will be used in down stream patches to store and help process devlink param values that are read from user input or kernel output. Rename it to reflect a more suitable name for its purpose. Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 62 +++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index fb89757c..57f71bb8 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3627,7 +3627,7 @@ static int cmd_dev_param_show_cb(const struct nlmsghdr *nlh, void *data) return MNL_CB_OK; } -struct param_ctx { +struct dl_param { struct dl *dl; enum devlink_var_attr_type type; bool cmode_found; @@ -3648,8 +3648,8 @@ static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) struct nlattr *param_value_attr; enum devlink_var_attr_type type; enum devlink_param_cmode cmode; - struct param_ctx *ctx = data; - struct dl *dl = ctx->dl; + struct dl_param *param = data; + struct dl *dl = param->dl; int err; mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb); @@ -3686,23 +3686,23 @@ static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) cmode = mnl_attr_get_u8(nla_value[DEVLINK_ATTR_PARAM_VALUE_CMODE]); if (cmode == dl->opts.cmode) { - ctx->cmode_found = true; + param->cmode_found = true; val_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA]; switch (type) { case DEVLINK_VAR_ATTR_TYPE_U8: - ctx->value.vu8 = mnl_attr_get_u8(val_attr); + param->value.vu8 = mnl_attr_get_u8(val_attr); break; case DEVLINK_VAR_ATTR_TYPE_U16: - ctx->value.vu16 = mnl_attr_get_u16(val_attr); + param->value.vu16 = mnl_attr_get_u16(val_attr); break; case DEVLINK_VAR_ATTR_TYPE_U32: - ctx->value.vu32 = mnl_attr_get_u32(val_attr); + param->value.vu32 = mnl_attr_get_u32(val_attr); break; case DEVLINK_VAR_ATTR_TYPE_STRING: - ctx->value.vstr = mnl_attr_get_str(val_attr); + param->value.vstr = mnl_attr_get_str(val_attr); break; case DEVLINK_VAR_ATTR_TYPE_FLAG: - ctx->value.vbool = val_attr ? true : false; + param->value.vbool = val_attr ? true : false; break; default: break; @@ -3710,13 +3710,13 @@ static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) break; } } - ctx->type = type; + param->type = type; return MNL_CB_OK; } static int cmd_dev_param_set(struct dl *dl) { - struct param_ctx ctx = {}; + struct dl_param kparam = {}; /* kernel param */ struct nlmsghdr *nlh; bool conv_exists; uint32_t val_u32 = 0; @@ -3737,11 +3737,11 @@ static int cmd_dev_param_set(struct dl *dl) NLM_F_REQUEST | NLM_F_ACK); dl_opts_put(nlh, dl); - ctx.dl = dl; - err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_param_set_cb, &ctx); + kparam.dl = dl; + err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_param_set_cb, &kparam); if (err) return err; - if (!ctx.cmode_found) { + if (!kparam.cmode_found) { pr_err("Configuration mode not supported\n"); return -ENOTSUP; } @@ -3753,8 +3753,8 @@ static int cmd_dev_param_set(struct dl *dl) conv_exists = param_val_conv_exists(param_val_conv, PARAM_VAL_CONV_LEN, dl->opts.param_name); - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, ctx.type); - switch (ctx.type) { + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, kparam.type); + switch (kparam.type) { case DEVLINK_VAR_ATTR_TYPE_U8: if (conv_exists) { err = param_val_conv_uint_get(param_val_conv, @@ -3768,7 +3768,7 @@ static int cmd_dev_param_set(struct dl *dl) } if (err) goto err_param_value_parse; - if (val_u8 == ctx.value.vu8) + if (val_u8 == kparam.value.vu8) return 0; mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u8); break; @@ -3785,7 +3785,7 @@ static int cmd_dev_param_set(struct dl *dl) } if (err) goto err_param_value_parse; - if (val_u16 == ctx.value.vu16) + if (val_u16 == kparam.value.vu16) return 0; mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u16); break; @@ -3800,7 +3800,7 @@ static int cmd_dev_param_set(struct dl *dl) err = get_u32(&val_u32, dl->opts.param_value, 10); if (err) goto err_param_value_parse; - if (val_u32 == ctx.value.vu32) + if (val_u32 == kparam.value.vu32) return 0; mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u32); break; @@ -3808,7 +3808,7 @@ static int cmd_dev_param_set(struct dl *dl) err = strtobool(dl->opts.param_value, &val_bool); if (err) goto err_param_value_parse; - if (val_bool == ctx.value.vbool) + if (val_bool == kparam.value.vbool) return 0; if (val_bool) mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, @@ -3817,7 +3817,7 @@ static int cmd_dev_param_set(struct dl *dl) case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, dl->opts.param_value); - if (!strcmp(dl->opts.param_value, ctx.value.vstr)) + if (!strcmp(dl->opts.param_value, kparam.value.vstr)) return 0; break; default: @@ -5224,7 +5224,7 @@ static int cmd_port_function_set(struct dl *dl) static int cmd_port_param_set(struct dl *dl) { - struct param_ctx ctx = {}; + struct dl_param kparam = {}; /* kernel param */ struct nlmsghdr *nlh; bool conv_exists; uint32_t val_u32 = 0; @@ -5245,8 +5245,8 @@ static int cmd_port_param_set(struct dl *dl) NLM_F_REQUEST | NLM_F_ACK); dl_opts_put(nlh, dl); - ctx.dl = dl; - err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_param_set_cb, &ctx); + kparam.dl = dl; + err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_param_set_cb, &kparam); if (err) return err; @@ -5257,8 +5257,8 @@ static int cmd_port_param_set(struct dl *dl) conv_exists = param_val_conv_exists(param_val_conv, PARAM_VAL_CONV_LEN, dl->opts.param_name); - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, ctx.type); - switch (ctx.type) { + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, kparam.type); + switch (kparam.type) { case DEVLINK_VAR_ATTR_TYPE_U8: if (conv_exists) { err = param_val_conv_uint_get(param_val_conv, @@ -5272,7 +5272,7 @@ static int cmd_port_param_set(struct dl *dl) } if (err) goto err_param_value_parse; - if (val_u8 == ctx.value.vu8) + if (val_u8 == kparam.value.vu8) return 0; mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u8); break; @@ -5289,7 +5289,7 @@ static int cmd_port_param_set(struct dl *dl) } if (err) goto err_param_value_parse; - if (val_u16 == ctx.value.vu16) + if (val_u16 == kparam.value.vu16) return 0; mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u16); break; @@ -5304,7 +5304,7 @@ static int cmd_port_param_set(struct dl *dl) err = get_u32(&val_u32, dl->opts.param_value, 10); if (err) goto err_param_value_parse; - if (val_u32 == ctx.value.vu32) + if (val_u32 == kparam.value.vu32) return 0; mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u32); break; @@ -5312,7 +5312,7 @@ static int cmd_port_param_set(struct dl *dl) err = strtobool(dl->opts.param_value, &val_bool); if (err) goto err_param_value_parse; - if (val_bool == ctx.value.vbool) + if (val_bool == kparam.value.vbool) return 0; if (val_bool) mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, @@ -5321,7 +5321,7 @@ static int cmd_port_param_set(struct dl *dl) case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, dl->opts.param_value); - if (!strcmp(dl->opts.param_value, ctx.value.vstr)) + if (!strcmp(dl->opts.param_value, kparam.value.vstr)) return 0; break; default: -- 2.50.0 From: Saeed Mahameed Refactor user option parsing into a centralized helper that produces dl_param from the user options, and store the values into struct dl_param. Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 207 ++++++++++++++++++++++------------------------ 1 file changed, 99 insertions(+), 108 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index 57f71bb8..95302308 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3640,6 +3640,77 @@ struct dl_param { } value; }; +/* Get the parameter value from the options and convert it to the + * appropriate type. + * @dl: dl struct + * @nla_type: type of the parameter value + * @param: parameter struct to store the value + * Returns: 0 on success, -errno on failure + */ +static int dl_param_opts_get(struct dl *dl, enum devlink_var_attr_type type, + struct dl_param *param) +{ + uint32_t val_u32 = UINT32_MAX; + bool conv_exists; + int err = 0; + + conv_exists = param_val_conv_exists(param_val_conv, PARAM_VAL_CONV_LEN, + dl->opts.param_name); + param->type = type; + if (!conv_exists || + type == DEVLINK_VAR_ATTR_TYPE_STRING || + type == DEVLINK_VAR_ATTR_TYPE_FLAG || + type == DEVLINK_VAR_ATTR_TYPE_U32_ARRAY) { + switch (type) { + case DEVLINK_VAR_ATTR_TYPE_U8: + err = get_u8(¶m->value.vu8, dl->opts.param_value, 10); + break; + case DEVLINK_VAR_ATTR_TYPE_U16: + err = get_u16(¶m->value.vu16, dl->opts.param_value, 10); + break; + case DEVLINK_VAR_ATTR_TYPE_U32: + err = get_u32(¶m->value.vu32, dl->opts.param_value, 10); + break; + case DEVLINK_VAR_ATTR_TYPE_FLAG: + err = strtobool(dl->opts.param_value, ¶m->value.vbool); + break; + case DEVLINK_VAR_ATTR_TYPE_STRING: + param->value.vstr = dl->opts.param_value; + err = 0; + break; + default: + err = -ENOTSUP; + } + return err; + } + + /* conv_exists */ + switch (type) { + case DEVLINK_VAR_ATTR_TYPE_U8: + err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, + dl->opts.param_name, dl->opts.param_value, + &val_u32); + param->value.vu8 = val_u32; + break; + case DEVLINK_VAR_ATTR_TYPE_U16: + err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, + dl->opts.param_name, dl->opts.param_value, + &val_u32); + param->value.vu16 = val_u32; + break; + case DEVLINK_VAR_ATTR_TYPE_U32: + err = param_val_conv_uint_get(param_val_conv, PARAM_VAL_CONV_LEN, + dl->opts.param_name, dl->opts.param_value, + &val_u32); + param->value.vu32 = val_u32; + break; + default: + err = -ENOTSUP; + } + + return err; +} + static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) { struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh); @@ -3717,12 +3788,8 @@ static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) static int cmd_dev_param_set(struct dl *dl) { struct dl_param kparam = {}; /* kernel param */ + struct dl_param uparam = {}; /* user param */ struct nlmsghdr *nlh; - bool conv_exists; - uint32_t val_u32 = 0; - uint16_t val_u16; - uint8_t val_u8; - bool val_bool; int err; err = dl_argv_parse(dl, DL_OPT_HANDLE | @@ -3750,74 +3817,38 @@ static int cmd_dev_param_set(struct dl *dl) NLM_F_REQUEST | NLM_F_ACK); dl_opts_put(nlh, dl); - conv_exists = param_val_conv_exists(param_val_conv, PARAM_VAL_CONV_LEN, - dl->opts.param_name); + err = dl_param_opts_get(dl, kparam.type, &uparam); + if (err) + goto err_param_value_parse; mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, kparam.type); switch (kparam.type) { case DEVLINK_VAR_ATTR_TYPE_U8: - if (conv_exists) { - err = param_val_conv_uint_get(param_val_conv, - PARAM_VAL_CONV_LEN, - dl->opts.param_name, - dl->opts.param_value, - &val_u32); - val_u8 = val_u32; - } else { - err = get_u8(&val_u8, dl->opts.param_value, 10); - } - if (err) - goto err_param_value_parse; - if (val_u8 == kparam.value.vu8) + if (uparam.value.vu8 == kparam.value.vu8) return 0; - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u8); + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu8); break; case DEVLINK_VAR_ATTR_TYPE_U16: - if (conv_exists) { - err = param_val_conv_uint_get(param_val_conv, - PARAM_VAL_CONV_LEN, - dl->opts.param_name, - dl->opts.param_value, - &val_u32); - val_u16 = val_u32; - } else { - err = get_u16(&val_u16, dl->opts.param_value, 10); - } - if (err) - goto err_param_value_parse; - if (val_u16 == kparam.value.vu16) + if (uparam.value.vu16 == kparam.value.vu16) return 0; - mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u16); + mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu16); break; case DEVLINK_VAR_ATTR_TYPE_U32: - if (conv_exists) - err = param_val_conv_uint_get(param_val_conv, - PARAM_VAL_CONV_LEN, - dl->opts.param_name, - dl->opts.param_value, - &val_u32); - else - err = get_u32(&val_u32, dl->opts.param_value, 10); - if (err) - goto err_param_value_parse; - if (val_u32 == kparam.value.vu32) + if (uparam.value.vu32 == kparam.value.vu32) return 0; - mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u32); + mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu32); break; case DEVLINK_VAR_ATTR_TYPE_FLAG: - err = strtobool(dl->opts.param_value, &val_bool); - if (err) - goto err_param_value_parse; - if (val_bool == kparam.value.vbool) + if (uparam.value.vbool == kparam.value.vbool) return 0; - if (val_bool) + if (uparam.value.vbool) mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, 0, NULL); break; case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, - dl->opts.param_value); - if (!strcmp(dl->opts.param_value, kparam.value.vstr)) + uparam.value.vstr); + if (!strcmp(uparam.value.vstr, kparam.value.vstr)) return 0; break; default: @@ -5225,12 +5256,8 @@ static int cmd_port_function_set(struct dl *dl) static int cmd_port_param_set(struct dl *dl) { struct dl_param kparam = {}; /* kernel param */ + struct dl_param uparam = {}; /* user param */ struct nlmsghdr *nlh; - bool conv_exists; - uint32_t val_u32 = 0; - uint16_t val_u16; - uint8_t val_u8; - bool val_bool; int err; err = dl_argv_parse(dl, DL_OPT_HANDLEP | @@ -5254,74 +5281,38 @@ static int cmd_port_param_set(struct dl *dl) NLM_F_REQUEST | NLM_F_ACK); dl_opts_put(nlh, dl); - conv_exists = param_val_conv_exists(param_val_conv, PARAM_VAL_CONV_LEN, - dl->opts.param_name); + err = dl_param_opts_get(dl, kparam.type, &uparam); + if (err) + goto err_param_value_parse; mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, kparam.type); switch (kparam.type) { case DEVLINK_VAR_ATTR_TYPE_U8: - if (conv_exists) { - err = param_val_conv_uint_get(param_val_conv, - PARAM_VAL_CONV_LEN, - dl->opts.param_name, - dl->opts.param_value, - &val_u32); - val_u8 = val_u32; - } else { - err = get_u8(&val_u8, dl->opts.param_value, 10); - } - if (err) - goto err_param_value_parse; - if (val_u8 == kparam.value.vu8) + if (uparam.value.vu8 == kparam.value.vu8) return 0; - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u8); + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu8); break; case DEVLINK_VAR_ATTR_TYPE_U16: - if (conv_exists) { - err = param_val_conv_uint_get(param_val_conv, - PARAM_VAL_CONV_LEN, - dl->opts.param_name, - dl->opts.param_value, - &val_u32); - val_u16 = val_u32; - } else { - err = get_u16(&val_u16, dl->opts.param_value, 10); - } - if (err) - goto err_param_value_parse; - if (val_u16 == kparam.value.vu16) + if (uparam.value.vu16 == kparam.value.vu16) return 0; - mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u16); + mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu16); break; case DEVLINK_VAR_ATTR_TYPE_U32: - if (conv_exists) - err = param_val_conv_uint_get(param_val_conv, - PARAM_VAL_CONV_LEN, - dl->opts.param_name, - dl->opts.param_value, - &val_u32); - else - err = get_u32(&val_u32, dl->opts.param_value, 10); - if (err) - goto err_param_value_parse; - if (val_u32 == kparam.value.vu32) + if (uparam.value.vu32 == kparam.value.vu32) return 0; - mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, val_u32); + mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu32); break; case DEVLINK_VAR_ATTR_TYPE_FLAG: - err = strtobool(dl->opts.param_value, &val_bool); - if (err) - goto err_param_value_parse; - if (val_bool == kparam.value.vbool) + if (uparam.value.vbool == kparam.value.vbool) return 0; - if (val_bool) + if (uparam.value.vbool) mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, 0, NULL); break; case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, - dl->opts.param_value); - if (!strcmp(dl->opts.param_value, kparam.value.vstr)) + uparam.value.vstr); + if (!strcmp(uparam.value.vstr, kparam.value.vstr)) return 0; break; default: -- 2.50.0 From: Saeed Mahameed In cmd_{dev,port}_param_set, we compare the kernel param values with the user input, fold this code into a helper function. Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 62 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 20 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index 95302308..d1b2caa1 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3711,6 +3711,36 @@ static int dl_param_opts_get(struct dl *dl, enum devlink_var_attr_type type, return err; } +/* dl_param_cmp: compare two dl_param structs + * @p1: first dl_param struct + * @p2: second dl_param struct + * Returns: 0 if the two structs are equal, 1 when different, + * -errno on validation error + */ +static int dl_param_cmp(struct dl_param *p1, struct dl_param *p2) +{ + if (p1->type != p2->type) + return -EINVAL; + + switch (p1->type) { + case DEVLINK_VAR_ATTR_TYPE_U8: + return p1->value.vu8 != p2->value.vu8; + case DEVLINK_VAR_ATTR_TYPE_U16: + return p1->value.vu16 != p2->value.vu16; + case DEVLINK_VAR_ATTR_TYPE_U32: + return p1->value.vu32 != p2->value.vu32; + case DEVLINK_VAR_ATTR_TYPE_FLAG: + return p1->value.vbool != p2->value.vbool; + case DEVLINK_VAR_ATTR_TYPE_STRING: + if (strcmp(p1->value.vstr, p2->value.vstr)) + return 1; + break; + default: + return -EINVAL; + } + return 0; +} + static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) { struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh); @@ -3821,26 +3851,24 @@ static int cmd_dev_param_set(struct dl *dl) if (err) goto err_param_value_parse; + err = dl_param_cmp(&uparam, &kparam); + if (err < 0) + goto err_param_value_parse; + if (!err) /* Value is the same */ + return 0; + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, kparam.type); switch (kparam.type) { case DEVLINK_VAR_ATTR_TYPE_U8: - if (uparam.value.vu8 == kparam.value.vu8) - return 0; mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu8); break; case DEVLINK_VAR_ATTR_TYPE_U16: - if (uparam.value.vu16 == kparam.value.vu16) - return 0; mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu16); break; case DEVLINK_VAR_ATTR_TYPE_U32: - if (uparam.value.vu32 == kparam.value.vu32) - return 0; mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu32); break; case DEVLINK_VAR_ATTR_TYPE_FLAG: - if (uparam.value.vbool == kparam.value.vbool) - return 0; if (uparam.value.vbool) mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, 0, NULL); @@ -3848,8 +3876,6 @@ static int cmd_dev_param_set(struct dl *dl) case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vstr); - if (!strcmp(uparam.value.vstr, kparam.value.vstr)) - return 0; break; default: printf("Value type not supported\n"); @@ -5285,26 +5311,24 @@ static int cmd_port_param_set(struct dl *dl) if (err) goto err_param_value_parse; + err = dl_param_cmp(&uparam, &kparam); + if (err < 0) + goto err_param_value_parse; + if (!err) /* Value is the same */ + return 0; + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, kparam.type); switch (kparam.type) { case DEVLINK_VAR_ATTR_TYPE_U8: - if (uparam.value.vu8 == kparam.value.vu8) - return 0; mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu8); break; case DEVLINK_VAR_ATTR_TYPE_U16: - if (uparam.value.vu16 == kparam.value.vu16) - return 0; mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu16); break; case DEVLINK_VAR_ATTR_TYPE_U32: - if (uparam.value.vu32 == kparam.value.vu32) - return 0; mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu32); break; case DEVLINK_VAR_ATTR_TYPE_FLAG: - if (uparam.value.vbool == kparam.value.vbool) - return 0; if (uparam.value.vbool) mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, 0, NULL); @@ -5312,8 +5336,6 @@ static int cmd_port_param_set(struct dl *dl) case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vstr); - if (!strcmp(uparam.value.vstr, kparam.value.vstr)) - return 0; break; default: printf("Value type not supported\n"); -- 2.50.0 From: Saeed Mahameed Introduce a helper function that takes a dl_param and calls the type corresponding mnl_attr_put_() equivalent to populate the nlmmsg with the parameter value. Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 88 +++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 48 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index d1b2caa1..9a6adf06 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3741,6 +3741,38 @@ static int dl_param_cmp(struct dl_param *p1, struct dl_param *p2) return 0; } +/* dl_param_mnl_put: put the value in the netlink message + * @nlh: netlink message + * @param: dl_param struct containing the value + * Returns: 0 on success, -errno on failure + */ +static int dl_param_mnl_put(struct nlmsghdr *nlh, struct dl_param *param) +{ + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, param->type); + switch (param->type) { + case DEVLINK_VAR_ATTR_TYPE_U8: + mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, param->value.vu8); + break; + case DEVLINK_VAR_ATTR_TYPE_U16: + mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, param->value.vu16); + break; + case DEVLINK_VAR_ATTR_TYPE_U32: + mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, param->value.vu32); + break; + case DEVLINK_VAR_ATTR_TYPE_FLAG: + if (param->value.vbool) + mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, 0, NULL); + break; + case DEVLINK_VAR_ATTR_TYPE_STRING: + mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, param->value.vstr); + break; + default: + pr_err("Value type(%d) not supported\n", param->type); + return -ENOTSUP; + } + return 0; +} + static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) { struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh); @@ -3857,30 +3889,10 @@ static int cmd_dev_param_set(struct dl *dl) if (!err) /* Value is the same */ return 0; - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, kparam.type); - switch (kparam.type) { - case DEVLINK_VAR_ATTR_TYPE_U8: - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu8); - break; - case DEVLINK_VAR_ATTR_TYPE_U16: - mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu16); - break; - case DEVLINK_VAR_ATTR_TYPE_U32: - mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu32); - break; - case DEVLINK_VAR_ATTR_TYPE_FLAG: - if (uparam.value.vbool) - mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, - 0, NULL); - break; - case DEVLINK_VAR_ATTR_TYPE_STRING: - mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, - uparam.value.vstr); - break; - default: - printf("Value type not supported\n"); - return -ENOTSUP; - } + err = dl_param_mnl_put(nlh, &uparam); + if (err) + return err; + return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL); err_param_value_parse: @@ -5317,30 +5329,10 @@ static int cmd_port_param_set(struct dl *dl) if (!err) /* Value is the same */ return 0; - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_TYPE, kparam.type); - switch (kparam.type) { - case DEVLINK_VAR_ATTR_TYPE_U8: - mnl_attr_put_u8(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu8); - break; - case DEVLINK_VAR_ATTR_TYPE_U16: - mnl_attr_put_u16(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu16); - break; - case DEVLINK_VAR_ATTR_TYPE_U32: - mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, uparam.value.vu32); - break; - case DEVLINK_VAR_ATTR_TYPE_FLAG: - if (uparam.value.vbool) - mnl_attr_put(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, - 0, NULL); - break; - case DEVLINK_VAR_ATTR_TYPE_STRING: - mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, - uparam.value.vstr); - break; - default: - printf("Value type not supported\n"); - return -ENOTSUP; - } + err = dl_param_mnl_put(nlh, &uparam); + if (err) + return err; + return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL); err_param_value_parse: -- 2.50.0 From: Saeed Mahameed Centralized location to parse different types of param values. This is useful for upcoming new type of parameters support to be added to dl_param related functions. Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 68 ++++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index 9a6adf06..fe64f2dc 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3773,6 +3773,40 @@ static int dl_param_mnl_put(struct nlmsghdr *nlh, struct dl_param *param) return 0; } +/* dl_param_val_attr_parse: parse the value attribute and store the value + * in the dl_param struct + * @data_attr: value data attribute + * @nla_type: type of the value attribute + * @param: dl_param struct to store the value + */ +static int +dl_param_val_attr_parse(struct nlattr *data_attr, + enum devlink_var_attr_type type, + struct dl_param *param) +{ + switch (type) { + case DEVLINK_VAR_ATTR_TYPE_U8: + param->value.vu8 = mnl_attr_get_u8(data_attr); + break; + case DEVLINK_VAR_ATTR_TYPE_U16: + param->value.vu16 = mnl_attr_get_u16(data_attr); + break; + case DEVLINK_VAR_ATTR_TYPE_U32: + param->value.vu32 = mnl_attr_get_u32(data_attr); + break; + case DEVLINK_VAR_ATTR_TYPE_STRING: + param->value.vstr = mnl_attr_get_str(data_attr); + break; + case DEVLINK_VAR_ATTR_TYPE_FLAG: + param->value.vbool = data_attr ? true : false; + break; + default: + pr_err("Value type(%d) not supported\n", type); + return -ENOTSUP; + } + return 0; +} + static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) { struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh); @@ -3805,7 +3839,7 @@ static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) mnl_attr_for_each_nested(param_value_attr, nla_param[DEVLINK_ATTR_PARAM_VALUES_LIST]) { struct nlattr *nla_value[DEVLINK_ATTR_MAX + 1] = {}; - struct nlattr *val_attr; + struct nlattr *data_attr; err = mnl_attr_parse_nested(param_value_attr, attr_cb, nla_value); @@ -3818,30 +3852,14 @@ static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) return MNL_CB_ERROR; cmode = mnl_attr_get_u8(nla_value[DEVLINK_ATTR_PARAM_VALUE_CMODE]); - if (cmode == dl->opts.cmode) { - param->cmode_found = true; - val_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA]; - switch (type) { - case DEVLINK_VAR_ATTR_TYPE_U8: - param->value.vu8 = mnl_attr_get_u8(val_attr); - break; - case DEVLINK_VAR_ATTR_TYPE_U16: - param->value.vu16 = mnl_attr_get_u16(val_attr); - break; - case DEVLINK_VAR_ATTR_TYPE_U32: - param->value.vu32 = mnl_attr_get_u32(val_attr); - break; - case DEVLINK_VAR_ATTR_TYPE_STRING: - param->value.vstr = mnl_attr_get_str(val_attr); - break; - case DEVLINK_VAR_ATTR_TYPE_FLAG: - param->value.vbool = val_attr ? true : false; - break; - default: - break; - } - break; - } + if (cmode != dl->opts.cmode) + continue; + + param->cmode_found = true; + data_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA]; + if (dl_param_val_attr_parse(data_attr, type, param)) + return MNL_CB_ERROR; + break; } param->type = type; return MNL_CB_OK; -- 2.50.0 From: Saeed Mahameed Update the dl_params helper functions to process nested value attributes, currently the kernel supports variably sized arrays of u32 via param enum type DEVLINK_VAR_ATTR_TYPE_U32_ARRAY. Add command line parsing to parse comma separated u32 user inputs and fill the nlmsg accordingly, check for size mismatch between current kernel value and user input. example: $ devlink dev param set name foo value 1,2,3,4,5,6,7,8 cmode ... Signed-off-by: Saeed Mahameed --- devlink/devlink.c | 102 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 90 insertions(+), 12 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index fe64f2dc..15f2a80b 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3426,29 +3426,29 @@ struct dl_param_val_list { }; /* Parse nested param value list - * @val_list_attr: nested attribute containing the list of values - * usually : val_list_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA] + * @val_nest_attr: nested attribute containing the list of values + * val_nest_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE] * @list: pointer to the list of values, reallocated to the new size * Returns: 0 on success, -errno on failure */ static int -dl_mnl_parse_param_val_nested(struct nlattr *val_list_attr, +dl_mnl_parse_param_val_nested(struct nlattr *val_nest_attr, struct dl_param_val_list **list) { struct dl_param_val_list *new_list; - struct nlattr *val_attr; + struct nlattr *val_data_attr; int i = 0, len = 0; - len = mnl_attr_get_payload_len(val_list_attr)/(MNL_ATTR_HDRLEN + sizeof(uint32_t)); - if (!len) - return -EINVAL; - + mnl_attr_for_each_nested(val_data_attr, val_nest_attr) + if (mnl_attr_get_type(val_data_attr) == DEVLINK_ATTR_PARAM_VALUE_DATA) + len++; new_list = realloc(*list, sizeof(new_list) + len * sizeof(uint32_t)); if (!new_list) return -ENOMEM; - mnl_attr_for_each_nested(val_attr, val_list_attr) - new_list->vu32[i++] = mnl_attr_get_u32(val_attr); + mnl_attr_for_each_nested(val_data_attr, val_nest_attr) + if (mnl_attr_get_type(val_data_attr) == DEVLINK_ATTR_PARAM_VALUE_DATA) + new_list->vu32[i++] = mnl_attr_get_u32(val_data_attr); new_list->len = i; *list = new_list; @@ -3543,7 +3543,7 @@ static void pr_out_param_value(struct dl *dl, const char *nla_name, int err; int i; - err = dl_mnl_parse_param_val_nested(val_attr, &list); + err = dl_mnl_parse_param_val_nested(nl, &list); if (err) return; @@ -3637,9 +3637,53 @@ struct dl_param { uint32_t vu32; const char *vstr; bool vbool; + struct dl_param_val_list *vlist; } value; }; +/* Get the parameter value from the options and fill the param struct + * @dl: dl struct + * @nla_type: type of the parameter value + * @param: parameter struct to store the value + * + * Note: + * param->value.vlist reallocated to the new size + * + * Returns: 0 on success, -errno on failure + */ +static int dl_param_opts_get_arr(struct dl *dl, struct dl_param *param) +{ + char *tmp = strdup(dl->opts.param_value); + struct dl_param_val_list *list; + const char *p = NULL; + int err = 0, i = 1; + + if (!tmp) { + pr_err("Memory allocation failed\n"); + return -ENOMEM; + } + for (p = tmp; *p; p++) + i += (*p == ','); + + list = realloc(param->value.vlist, sizeof(*list) + i * sizeof(uint32_t)); + if (!list) { + pr_err("Memory allocation failed\n"); + err = -ENOMEM; + goto out; + } + param->value.vlist = list; /* update vlist to new size */ + i = list->len = 0; /* reset len */ + for (p = strtok(tmp, ","); p; p = strtok(NULL, ",")) { + err = get_u32(&list->vu32[i++], p, 10); + if (err) + goto out; + } + /* update len only when all values are filled */ + list->len = i; +out: + free(tmp); + return err; +} /* Get the parameter value from the options and convert it to the * appropriate type. * @dl: dl struct @@ -3678,6 +3722,9 @@ static int dl_param_opts_get(struct dl *dl, enum devlink_var_attr_type type, param->value.vstr = dl->opts.param_value; err = 0; break; + case DEVLINK_VAR_ATTR_TYPE_U32_ARRAY: + err = dl_param_opts_get_arr(dl, param); + break; default: err = -ENOTSUP; } @@ -3735,6 +3782,18 @@ static int dl_param_cmp(struct dl_param *p1, struct dl_param *p2) if (strcmp(p1->value.vstr, p2->value.vstr)) return 1; break; + case DEVLINK_VAR_ATTR_TYPE_U32_ARRAY: + if (!p1->value.vlist || !p2->value.vlist) + return -EINVAL; + if (p1->value.vlist->len != p2->value.vlist->len) { + pr_err("Error: expecting value list of legnth %ld\n", + p2->value.vlist->len); + return -EINVAL; /* different lengths is not expected */ + } + if (memcmp(p1->value.vlist->vu32, p2->value.vlist->vu32, + p1->value.vlist->len * sizeof(uint32_t))) + return 1; + break; default: return -EINVAL; } @@ -3766,6 +3825,18 @@ static int dl_param_mnl_put(struct nlmsghdr *nlh, struct dl_param *param) case DEVLINK_VAR_ATTR_TYPE_STRING: mnl_attr_put_strz(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, param->value.vstr); break; + case DEVLINK_VAR_ATTR_TYPE_U32_ARRAY: { + struct dl_param_val_list *list = param->value.vlist; + int i; + + if (!list) + return -EINVAL; + + for (i = 0; i < list->len; i++) + mnl_attr_put_u32(nlh, DEVLINK_ATTR_PARAM_VALUE_DATA, list->vu32[i]); + + break; + } default: pr_err("Value type(%d) not supported\n", param->type); return -ENOTSUP; @@ -3776,11 +3847,13 @@ static int dl_param_mnl_put(struct nlmsghdr *nlh, struct dl_param *param) /* dl_param_val_attr_parse: parse the value attribute and store the value * in the dl_param struct * @data_attr: value data attribute + * @val_nest_attr: parent attribute containing the values * @nla_type: type of the value attribute * @param: dl_param struct to store the value */ static int dl_param_val_attr_parse(struct nlattr *data_attr, + struct nlattr *val_nest_attr, enum devlink_var_attr_type type, struct dl_param *param) { @@ -3800,6 +3873,11 @@ dl_param_val_attr_parse(struct nlattr *data_attr, case DEVLINK_VAR_ATTR_TYPE_FLAG: param->value.vbool = data_attr ? true : false; break; + case DEVLINK_VAR_ATTR_TYPE_U32_ARRAY: + if(dl_mnl_parse_param_val_nested(val_nest_attr, + ¶m->value.vlist)) + return -ENOMEM; + break; default: pr_err("Value type(%d) not supported\n", type); return -ENOTSUP; @@ -3857,7 +3935,7 @@ static int cmd_param_set_cb(const struct nlmsghdr *nlh, void *data) param->cmode_found = true; data_attr = nla_value[DEVLINK_ATTR_PARAM_VALUE_DATA]; - if (dl_param_val_attr_parse(data_attr, type, param)) + if (dl_param_val_attr_parse(data_attr, param_value_attr, type, param)) return MNL_CB_ERROR; break; } -- 2.50.0