netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes
@ 2025-05-05 11:45 Jiri Pirko
  2025-05-05 11:45 ` [PATCH net-next v2 1/4] tools: ynl-gen: allow noncontiguous enums Jiri Pirko
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Jiri Pirko @ 2025-05-05 11:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni, saeedm, horms, donald.hunter

From: Jiri Pirko <jiri@nvidia.com>

This is continuation based on first two patches
of https://lore.kernel.org/netdev/20250425214808.507732-1-saeed@kernel.org/

Better to take it as a separate patchset, as the rest of the original
patchset is probably settled.

This patchset is taking care of incorrect usage of internal NLA_* values
in uapi, introduces new enum (in patch #2) that shadows NLA_* values and
makes that part of UAPI.

The last two patches removes unnecessary translations with maintaining
clear devlink param driver api. I hope this might be acceptable.

Please check and merge to get this over with :)

Jiri Pirko (4):
  tools: ynl-gen: allow noncontiguous enums
  devlink: define enum for attr types of dynamic attributes
  devlink: avoid param type value translations
  devlink: use DEVLINK_VAR_ATTR_TYPE_* instead of NLA_* in fmsg

 Documentation/netlink/specs/devlink.yaml | 24 ++++++++++
 include/net/devlink.h                    | 10 ++--
 include/uapi/linux/devlink.h             | 15 ++++++
 net/devlink/health.c                     | 52 +++++++++------------
 net/devlink/netlink_gen.c                | 29 +++++++++++-
 net/devlink/param.c                      | 46 +------------------
 tools/net/ynl/pyynl/ynl_gen_c.py         | 58 +++++++++++++++++++++---
 7 files changed, 147 insertions(+), 87 deletions(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 1/4] tools: ynl-gen: allow noncontiguous enums
  2025-05-05 11:45 [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes Jiri Pirko
@ 2025-05-05 11:45 ` Jiri Pirko
  2025-05-06  6:38   ` Jiri Pirko
  2025-05-05 11:45 ` [PATCH net-next v2 2/4] devlink: define enum for attr types of dynamic attributes Jiri Pirko
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Jiri Pirko @ 2025-05-05 11:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni, saeedm, horms, donald.hunter

From: Jiri Pirko <jiri@nvidia.com>

in case the enum has holes, instead of hard stop, generate a validation
callback to check valid enum values.

signed-off-by: jiri pirko <jiri@nvidia.com>
---
v1->v2:
- fixed min/max none check
- changed switch-case to fall-through with single return statement
- removed not needed special indentation treating
saeed's v3->v1:
- added validation callback generation
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 58 ++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 09b87c9a6908..acba03bbe67d 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -389,11 +389,14 @@ class TypeScalar(Type):
         if 'enum' in self.attr:
             enum = self.family.consts[self.attr['enum']]
             low, high = enum.value_range()
-            if 'min' not in self.checks:
-                if low != 0 or self.type[0] == 's':
-                    self.checks['min'] = low
-            if 'max' not in self.checks:
-                self.checks['max'] = high
+            if low == None and high == None:
+                self.checks['sparse'] = True
+            else:
+                if 'min' not in self.checks:
+                    if low != 0 or self.type[0] == 's':
+                        self.checks['min'] = low
+                if 'max' not in self.checks:
+                    self.checks['max'] = high
 
         if 'min' in self.checks and 'max' in self.checks:
             if self.get_limit('min') > self.get_limit('max'):
@@ -425,6 +428,8 @@ class TypeScalar(Type):
             return f"NLA_POLICY_MIN({policy}, {self.get_limit_str('min')})"
         elif 'max' in self.checks:
             return f"NLA_POLICY_MAX({policy}, {self.get_limit_str('max')})"
+        elif 'sparse' in self.checks:
+            return f"NLA_POLICY_VALIDATE_FN({policy}, &{c_lower(self.enum_name)}_validate)"
         return super()._attr_policy(policy)
 
     def _attr_typol(self):
@@ -930,7 +935,7 @@ class EnumSet(SpecEnumSet):
         high = max([x.value for x in self.entries.values()])
 
         if high - low + 1 != len(self.entries):
-            raise Exception("Can't get value range for a noncontiguous enum")
+            return None, None
 
         return low, high
 
@@ -2426,6 +2431,46 @@ def print_kernel_policy_ranges(family, cw):
             cw.nl()
 
 
+def print_kernel_policy_sparse_enum_validates(family, cw):
+    first = True
+    for _, attr_set in family.attr_sets.items():
+        if attr_set.subset_of:
+            continue
+
+        for _, attr in attr_set.items():
+            if not attr.request:
+                continue
+            if not attr.enum_name:
+                continue
+            if 'sparse' not in attr.checks:
+                continue
+
+            if first:
+                cw.p('/* Sparse enums validation callbacks */')
+                first = False
+
+            sign = '' if attr.type[0] == 'u' else '_signed'
+            suffix = 'ULL' if attr.type[0] == 'u' else 'LL'
+            cw.write_func_prot('static int', f'{c_lower(attr.enum_name)}_validate',
+                               ['const struct nlattr *attr', 'struct netlink_ext_ack *extack'])
+            cw.block_start()
+            cw.block_start(line=f'switch (nla_get_{attr["type"]}(attr))')
+            enum = family.consts[attr['enum']]
+            first_entry = True
+            for entry in enum.entries.values():
+                if first_entry:
+                    first_entry = False
+                else:
+                    cw.p('fallthrough;')
+                cw.p(f'case {entry.c_name}:')
+            cw.p('return 0;')
+            cw.block_end()
+            cw.p('NL_SET_ERR_MSG_ATTR(extack, attr, "invalid enum value");')
+            cw.p('return -EINVAL;')
+            cw.block_end()
+            cw.nl()
+
+
 def print_kernel_op_table_fwd(family, cw, terminate):
     exported = not kernel_can_gen_family_struct(family)
 
@@ -3097,6 +3142,7 @@ def main():
             print_kernel_family_struct_hdr(parsed, cw)
         else:
             print_kernel_policy_ranges(parsed, cw)
+            print_kernel_policy_sparse_enum_validates(parsed, cw)
 
             for _, struct in sorted(parsed.pure_nested_structs.items()):
                 if struct.request:
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 2/4] devlink: define enum for attr types of dynamic attributes
  2025-05-05 11:45 [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes Jiri Pirko
  2025-05-05 11:45 ` [PATCH net-next v2 1/4] tools: ynl-gen: allow noncontiguous enums Jiri Pirko
@ 2025-05-05 11:45 ` Jiri Pirko
  2025-05-05 11:45 ` [PATCH net-next v2 3/4] devlink: avoid param type value translations Jiri Pirko
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2025-05-05 11:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni, saeedm, horms, donald.hunter

From: Jiri Pirko <jiri@nvidia.com>

Devlink param and health reporter fmsg use attributes with dynamic type
which is determined according to a different type. Currently used values
are NLA_*. The problem is, they are not part of UAPI. They may change
which would cause a break.

To make this future safe, introduce a enum that shadows NLA_* values in
it and is part of UAPI.

Also, this allows to possibly carry types that are unrelated to NLA_*
values.

Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- re-generated netlink_gen.c according to changes in previous patch
- changed enum comment to be non-kdoc
Saeed's v3->v1:
- s/Dynamic/Variable/ in comment
- re-generated netlink_gen.c according to changes in previous patch
---
 Documentation/netlink/specs/devlink.yaml | 24 +++++++++++++++++++
 include/uapi/linux/devlink.h             | 15 ++++++++++++
 net/devlink/health.c                     | 17 ++++++++++++--
 net/devlink/netlink_gen.c                | 29 ++++++++++++++++++++++-
 net/devlink/param.c                      | 30 ++++++++++++------------
 5 files changed, 97 insertions(+), 18 deletions(-)

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index bd9726269b4f..05fee1b7fe19 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -202,6 +202,28 @@ definitions:
         name: exception
       -
         name: control
+  -
+    type: enum
+    name: var-attr-type
+    entries:
+      -
+        name: u8
+        value: 1
+      -
+        name: u16
+      -
+        name: u32
+      -
+        name: u64
+      -
+        name: string
+      -
+        name: flag
+      -
+        name: nul_string
+        value: 10
+      -
+        name: binary
 
 attribute-sets:
   -
@@ -498,6 +520,7 @@ attribute-sets:
       -
         name: param-type
         type: u8
+        enum: var-attr-type
 
       # TODO: fill in the attributes in between
 
@@ -592,6 +615,7 @@ attribute-sets:
       -
         name: fmsg-obj-value-type
         type: u8
+        enum: var-attr-type
 
       # TODO: fill in the attributes in between
 
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 9401aa343673..a5ee0f13740a 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -385,6 +385,21 @@ enum devlink_linecard_state {
 	DEVLINK_LINECARD_STATE_MAX = __DEVLINK_LINECARD_STATE_MAX - 1
 };
 
+/* Variable attribute type. */
+enum devlink_var_attr_type {
+	/* Following values relate to the internal NLA_* values */
+	DEVLINK_VAR_ATTR_TYPE_U8 = 1,
+	DEVLINK_VAR_ATTR_TYPE_U16,
+	DEVLINK_VAR_ATTR_TYPE_U32,
+	DEVLINK_VAR_ATTR_TYPE_U64,
+	DEVLINK_VAR_ATTR_TYPE_STRING,
+	DEVLINK_VAR_ATTR_TYPE_FLAG,
+	DEVLINK_VAR_ATTR_TYPE_NUL_STRING = 10,
+	DEVLINK_VAR_ATTR_TYPE_BINARY,
+	__DEVLINK_VAR_ATTR_TYPE_CUSTOM_BASE = 0x80,
+	/* Any possible custom types, unrelated to NLA_* values go below */
+};
+
 enum devlink_attr {
 	/* don't change the order or add anything between, this is ABI! */
 	DEVLINK_ATTR_UNSPEC,
diff --git a/net/devlink/health.c b/net/devlink/health.c
index 57db6799722a..95a7a62d85a2 100644
--- a/net/devlink/health.c
+++ b/net/devlink/health.c
@@ -930,18 +930,31 @@ EXPORT_SYMBOL_GPL(devlink_fmsg_binary_pair_put);
 static int
 devlink_fmsg_item_fill_type(struct devlink_fmsg_item *msg, struct sk_buff *skb)
 {
+	enum devlink_var_attr_type var_attr_type;
+
 	switch (msg->nla_type) {
 	case NLA_FLAG:
+		var_attr_type = DEVLINK_VAR_ATTR_TYPE_FLAG;
+		break;
 	case NLA_U8:
+		var_attr_type = DEVLINK_VAR_ATTR_TYPE_U8;
+		break;
 	case NLA_U32:
+		var_attr_type = DEVLINK_VAR_ATTR_TYPE_U32;
+		break;
 	case NLA_U64:
+		var_attr_type = DEVLINK_VAR_ATTR_TYPE_U64;
+		break;
 	case NLA_NUL_STRING:
+		var_attr_type = DEVLINK_VAR_ATTR_TYPE_NUL_STRING;
+		break;
 	case NLA_BINARY:
-		return nla_put_u8(skb, DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE,
-				  msg->nla_type);
+		var_attr_type = DEVLINK_VAR_ATTR_TYPE_BINARY;
+		break;
 	default:
 		return -EINVAL;
 	}
+	return nla_put_u8(skb, DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE, var_attr_type);
 }
 
 static int
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index f9786d51f68f..e340d955cf3b 100644
--- a/net/devlink/netlink_gen.c
+++ b/net/devlink/netlink_gen.c
@@ -10,6 +10,33 @@
 
 #include <uapi/linux/devlink.h>
 
+/* Sparse enums validation callbacks */
+static int
+devlink_attr_param_type_validate(const struct nlattr *attr,
+				 struct netlink_ext_ack *extack)
+{
+	switch (nla_get_u8(attr)) {
+	case DEVLINK_VAR_ATTR_TYPE_U8:
+		fallthrough;
+	case DEVLINK_VAR_ATTR_TYPE_U16:
+		fallthrough;
+	case DEVLINK_VAR_ATTR_TYPE_U32:
+		fallthrough;
+	case DEVLINK_VAR_ATTR_TYPE_U64:
+		fallthrough;
+	case DEVLINK_VAR_ATTR_TYPE_STRING:
+		fallthrough;
+	case DEVLINK_VAR_ATTR_TYPE_FLAG:
+		fallthrough;
+	case DEVLINK_VAR_ATTR_TYPE_NUL_STRING:
+		fallthrough;
+	case DEVLINK_VAR_ATTR_TYPE_BINARY:
+		return 0;
+	}
+	NL_SET_ERR_MSG_ATTR(extack, attr, "invalid enum value");
+	return -EINVAL;
+}
+
 /* Common nested types */
 const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_CAPS + 1] = {
 	[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] = { .type = NLA_BINARY, },
@@ -273,7 +300,7 @@ static const struct nla_policy devlink_param_set_nl_policy[DEVLINK_ATTR_PARAM_VA
 	[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
 	[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
 	[DEVLINK_ATTR_PARAM_NAME] = { .type = NLA_NUL_STRING, },
-	[DEVLINK_ATTR_PARAM_TYPE] = { .type = NLA_U8, },
+	[DEVLINK_ATTR_PARAM_TYPE] = NLA_POLICY_VALIDATE_FN(NLA_U8, &devlink_attr_param_type_validate),
 	[DEVLINK_ATTR_PARAM_VALUE_CMODE] = NLA_POLICY_MAX(NLA_U8, 2),
 };
 
diff --git a/net/devlink/param.c b/net/devlink/param.c
index dcf0d1ccebba..d0fb7c88bdb8 100644
--- a/net/devlink/param.c
+++ b/net/devlink/param.c
@@ -167,19 +167,19 @@ static int devlink_param_set(struct devlink *devlink,
 }
 
 static int
-devlink_param_type_to_nla_type(enum devlink_param_type param_type)
+devlink_param_type_to_var_attr_type(enum devlink_param_type param_type)
 {
 	switch (param_type) {
 	case DEVLINK_PARAM_TYPE_U8:
-		return NLA_U8;
+		return DEVLINK_VAR_ATTR_TYPE_U8;
 	case DEVLINK_PARAM_TYPE_U16:
-		return NLA_U16;
+		return DEVLINK_VAR_ATTR_TYPE_U16;
 	case DEVLINK_PARAM_TYPE_U32:
-		return NLA_U32;
+		return DEVLINK_VAR_ATTR_TYPE_U32;
 	case DEVLINK_PARAM_TYPE_STRING:
-		return NLA_STRING;
+		return DEVLINK_VAR_ATTR_TYPE_STRING;
 	case DEVLINK_PARAM_TYPE_BOOL:
-		return NLA_FLAG;
+		return DEVLINK_VAR_ATTR_TYPE_FLAG;
 	default:
 		return -EINVAL;
 	}
@@ -247,7 +247,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 	struct devlink_param_gset_ctx ctx;
 	struct nlattr *param_values_list;
 	struct nlattr *param_attr;
-	int nla_type;
+	int var_attr_type;
 	void *hdr;
 	int err;
 	int i;
@@ -294,10 +294,10 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 	if (param->generic && nla_put_flag(msg, DEVLINK_ATTR_PARAM_GENERIC))
 		goto param_nest_cancel;
 
-	nla_type = devlink_param_type_to_nla_type(param->type);
-	if (nla_type < 0)
+	var_attr_type = devlink_param_type_to_var_attr_type(param->type);
+	if (var_attr_type < 0)
 		goto param_nest_cancel;
-	if (nla_put_u8(msg, DEVLINK_ATTR_PARAM_TYPE, nla_type))
+	if (nla_put_u8(msg, DEVLINK_ATTR_PARAM_TYPE, var_attr_type))
 		goto param_nest_cancel;
 
 	param_values_list = nla_nest_start_noflag(msg,
@@ -420,19 +420,19 @@ devlink_param_type_get_from_info(struct genl_info *info,
 		return -EINVAL;
 
 	switch (nla_get_u8(info->attrs[DEVLINK_ATTR_PARAM_TYPE])) {
-	case NLA_U8:
+	case DEVLINK_VAR_ATTR_TYPE_U8:
 		*param_type = DEVLINK_PARAM_TYPE_U8;
 		break;
-	case NLA_U16:
+	case DEVLINK_VAR_ATTR_TYPE_U16:
 		*param_type = DEVLINK_PARAM_TYPE_U16;
 		break;
-	case NLA_U32:
+	case DEVLINK_VAR_ATTR_TYPE_U32:
 		*param_type = DEVLINK_PARAM_TYPE_U32;
 		break;
-	case NLA_STRING:
+	case DEVLINK_VAR_ATTR_TYPE_STRING:
 		*param_type = DEVLINK_PARAM_TYPE_STRING;
 		break;
-	case NLA_FLAG:
+	case DEVLINK_VAR_ATTR_TYPE_FLAG:
 		*param_type = DEVLINK_PARAM_TYPE_BOOL;
 		break;
 	default:
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 3/4] devlink: avoid param type value translations
  2025-05-05 11:45 [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes Jiri Pirko
  2025-05-05 11:45 ` [PATCH net-next v2 1/4] tools: ynl-gen: allow noncontiguous enums Jiri Pirko
  2025-05-05 11:45 ` [PATCH net-next v2 2/4] devlink: define enum for attr types of dynamic attributes Jiri Pirko
@ 2025-05-05 11:45 ` Jiri Pirko
  2025-05-05 11:45 ` [PATCH net-next v2 4/4] devlink: use DEVLINK_VAR_ATTR_TYPE_* instead of NLA_* in fmsg Jiri Pirko
  2025-05-07  1:30 ` [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes patchwork-bot+netdevbpf
  4 siblings, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2025-05-05 11:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni, saeedm, horms, donald.hunter

From: Jiri Pirko <jiri@nvidia.com>

Assign DEVLINK_PARAM_TYPE_* enum values to DEVLINK_VAR_ATTR_TYPE_* to
ensure the same values are used internally and in UAPI. Benefit from
that by removing the value translations.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 include/net/devlink.h | 10 +++++-----
 net/devlink/param.c   | 46 ++-----------------------------------------
 2 files changed, 7 insertions(+), 49 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index b8783126c1ed..0091f23a40f7 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -420,11 +420,11 @@ typedef u64 devlink_resource_occ_get_t(void *priv);
 
 #define __DEVLINK_PARAM_MAX_STRING_VALUE 32
 enum devlink_param_type {
-	DEVLINK_PARAM_TYPE_U8,
-	DEVLINK_PARAM_TYPE_U16,
-	DEVLINK_PARAM_TYPE_U32,
-	DEVLINK_PARAM_TYPE_STRING,
-	DEVLINK_PARAM_TYPE_BOOL,
+	DEVLINK_PARAM_TYPE_U8 = DEVLINK_VAR_ATTR_TYPE_U8,
+	DEVLINK_PARAM_TYPE_U16 = DEVLINK_VAR_ATTR_TYPE_U16,
+	DEVLINK_PARAM_TYPE_U32 = DEVLINK_VAR_ATTR_TYPE_U32,
+	DEVLINK_PARAM_TYPE_STRING = DEVLINK_VAR_ATTR_TYPE_STRING,
+	DEVLINK_PARAM_TYPE_BOOL = DEVLINK_VAR_ATTR_TYPE_FLAG,
 };
 
 union devlink_param_value {
diff --git a/net/devlink/param.c b/net/devlink/param.c
index d0fb7c88bdb8..b29abf8d3ed4 100644
--- a/net/devlink/param.c
+++ b/net/devlink/param.c
@@ -166,25 +166,6 @@ static int devlink_param_set(struct devlink *devlink,
 	return param->set(devlink, param->id, ctx, extack);
 }
 
-static int
-devlink_param_type_to_var_attr_type(enum devlink_param_type param_type)
-{
-	switch (param_type) {
-	case DEVLINK_PARAM_TYPE_U8:
-		return DEVLINK_VAR_ATTR_TYPE_U8;
-	case DEVLINK_PARAM_TYPE_U16:
-		return DEVLINK_VAR_ATTR_TYPE_U16;
-	case DEVLINK_PARAM_TYPE_U32:
-		return DEVLINK_VAR_ATTR_TYPE_U32;
-	case DEVLINK_PARAM_TYPE_STRING:
-		return DEVLINK_VAR_ATTR_TYPE_STRING;
-	case DEVLINK_PARAM_TYPE_BOOL:
-		return DEVLINK_VAR_ATTR_TYPE_FLAG;
-	default:
-		return -EINVAL;
-	}
-}
-
 static int
 devlink_nl_param_value_fill_one(struct sk_buff *msg,
 				enum devlink_param_type type,
@@ -247,7 +228,6 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 	struct devlink_param_gset_ctx ctx;
 	struct nlattr *param_values_list;
 	struct nlattr *param_attr;
-	int var_attr_type;
 	void *hdr;
 	int err;
 	int i;
@@ -293,11 +273,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 		goto param_nest_cancel;
 	if (param->generic && nla_put_flag(msg, DEVLINK_ATTR_PARAM_GENERIC))
 		goto param_nest_cancel;
-
-	var_attr_type = devlink_param_type_to_var_attr_type(param->type);
-	if (var_attr_type < 0)
-		goto param_nest_cancel;
-	if (nla_put_u8(msg, DEVLINK_ATTR_PARAM_TYPE, var_attr_type))
+	if (nla_put_u8(msg, DEVLINK_ATTR_PARAM_TYPE, param->type))
 		goto param_nest_cancel;
 
 	param_values_list = nla_nest_start_noflag(msg,
@@ -419,25 +395,7 @@ devlink_param_type_get_from_info(struct genl_info *info,
 	if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_PARAM_TYPE))
 		return -EINVAL;
 
-	switch (nla_get_u8(info->attrs[DEVLINK_ATTR_PARAM_TYPE])) {
-	case DEVLINK_VAR_ATTR_TYPE_U8:
-		*param_type = DEVLINK_PARAM_TYPE_U8;
-		break;
-	case DEVLINK_VAR_ATTR_TYPE_U16:
-		*param_type = DEVLINK_PARAM_TYPE_U16;
-		break;
-	case DEVLINK_VAR_ATTR_TYPE_U32:
-		*param_type = DEVLINK_PARAM_TYPE_U32;
-		break;
-	case DEVLINK_VAR_ATTR_TYPE_STRING:
-		*param_type = DEVLINK_PARAM_TYPE_STRING;
-		break;
-	case DEVLINK_VAR_ATTR_TYPE_FLAG:
-		*param_type = DEVLINK_PARAM_TYPE_BOOL;
-		break;
-	default:
-		return -EINVAL;
-	}
+	*param_type = nla_get_u8(info->attrs[DEVLINK_ATTR_PARAM_TYPE]);
 
 	return 0;
 }
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 4/4] devlink: use DEVLINK_VAR_ATTR_TYPE_* instead of NLA_* in fmsg
  2025-05-05 11:45 [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes Jiri Pirko
                   ` (2 preceding siblings ...)
  2025-05-05 11:45 ` [PATCH net-next v2 3/4] devlink: avoid param type value translations Jiri Pirko
@ 2025-05-05 11:45 ` Jiri Pirko
  2025-05-07  1:30 ` [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes patchwork-bot+netdevbpf
  4 siblings, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2025-05-05 11:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni, saeedm, horms, donald.hunter

From: Jiri Pirko <jiri@nvidia.com>

Use newly introduced DEVLINK_VAR_ATTR_TYPE_* enum values instead of
internal NLA_* in fmsg health reporter code.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/devlink/health.c | 65 ++++++++++++++------------------------------
 1 file changed, 21 insertions(+), 44 deletions(-)

diff --git a/net/devlink/health.c b/net/devlink/health.c
index 95a7a62d85a2..b3ce8ecbb7fb 100644
--- a/net/devlink/health.c
+++ b/net/devlink/health.c
@@ -735,7 +735,7 @@ static void devlink_fmsg_put_name(struct devlink_fmsg *fmsg, const char *name)
 		return;
 	}
 
-	item->nla_type = NLA_NUL_STRING;
+	item->nla_type = DEVLINK_VAR_ATTR_TYPE_NUL_STRING;
 	item->len = strlen(name) + 1;
 	item->attrtype = DEVLINK_ATTR_FMSG_OBJ_NAME;
 	memcpy(&item->value, name, item->len);
@@ -822,32 +822,37 @@ static void devlink_fmsg_put_value(struct devlink_fmsg *fmsg,
 static void devlink_fmsg_bool_put(struct devlink_fmsg *fmsg, bool value)
 {
 	devlink_fmsg_err_if_binary(fmsg);
-	devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_FLAG);
+	devlink_fmsg_put_value(fmsg, &value, sizeof(value),
+			       DEVLINK_VAR_ATTR_TYPE_FLAG);
 }
 
 static void devlink_fmsg_u8_put(struct devlink_fmsg *fmsg, u8 value)
 {
 	devlink_fmsg_err_if_binary(fmsg);
-	devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U8);
+	devlink_fmsg_put_value(fmsg, &value, sizeof(value),
+			       DEVLINK_VAR_ATTR_TYPE_U8);
 }
 
 void devlink_fmsg_u32_put(struct devlink_fmsg *fmsg, u32 value)
 {
 	devlink_fmsg_err_if_binary(fmsg);
-	devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U32);
+	devlink_fmsg_put_value(fmsg, &value, sizeof(value),
+			       DEVLINK_VAR_ATTR_TYPE_U32);
 }
 EXPORT_SYMBOL_GPL(devlink_fmsg_u32_put);
 
 static void devlink_fmsg_u64_put(struct devlink_fmsg *fmsg, u64 value)
 {
 	devlink_fmsg_err_if_binary(fmsg);
-	devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U64);
+	devlink_fmsg_put_value(fmsg, &value, sizeof(value),
+			       DEVLINK_VAR_ATTR_TYPE_U64);
 }
 
 void devlink_fmsg_string_put(struct devlink_fmsg *fmsg, const char *value)
 {
 	devlink_fmsg_err_if_binary(fmsg);
-	devlink_fmsg_put_value(fmsg, value, strlen(value) + 1, NLA_NUL_STRING);
+	devlink_fmsg_put_value(fmsg, value, strlen(value) + 1,
+			       DEVLINK_VAR_ATTR_TYPE_NUL_STRING);
 }
 EXPORT_SYMBOL_GPL(devlink_fmsg_string_put);
 
@@ -857,7 +862,8 @@ void devlink_fmsg_binary_put(struct devlink_fmsg *fmsg, const void *value,
 	if (!fmsg->err && !fmsg->putting_binary)
 		fmsg->err = -EINVAL;
 
-	devlink_fmsg_put_value(fmsg, value, value_len, NLA_BINARY);
+	devlink_fmsg_put_value(fmsg, value, value_len,
+			       DEVLINK_VAR_ATTR_TYPE_BINARY);
 }
 EXPORT_SYMBOL_GPL(devlink_fmsg_binary_put);
 
@@ -927,36 +933,6 @@ void devlink_fmsg_binary_pair_put(struct devlink_fmsg *fmsg, const char *name,
 }
 EXPORT_SYMBOL_GPL(devlink_fmsg_binary_pair_put);
 
-static int
-devlink_fmsg_item_fill_type(struct devlink_fmsg_item *msg, struct sk_buff *skb)
-{
-	enum devlink_var_attr_type var_attr_type;
-
-	switch (msg->nla_type) {
-	case NLA_FLAG:
-		var_attr_type = DEVLINK_VAR_ATTR_TYPE_FLAG;
-		break;
-	case NLA_U8:
-		var_attr_type = DEVLINK_VAR_ATTR_TYPE_U8;
-		break;
-	case NLA_U32:
-		var_attr_type = DEVLINK_VAR_ATTR_TYPE_U32;
-		break;
-	case NLA_U64:
-		var_attr_type = DEVLINK_VAR_ATTR_TYPE_U64;
-		break;
-	case NLA_NUL_STRING:
-		var_attr_type = DEVLINK_VAR_ATTR_TYPE_NUL_STRING;
-		break;
-	case NLA_BINARY:
-		var_attr_type = DEVLINK_VAR_ATTR_TYPE_BINARY;
-		break;
-	default:
-		return -EINVAL;
-	}
-	return nla_put_u8(skb, DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE, var_attr_type);
-}
-
 static int
 devlink_fmsg_item_fill_data(struct devlink_fmsg_item *msg, struct sk_buff *skb)
 {
@@ -964,20 +940,20 @@ devlink_fmsg_item_fill_data(struct devlink_fmsg_item *msg, struct sk_buff *skb)
 	u8 tmp;
 
 	switch (msg->nla_type) {
-	case NLA_FLAG:
+	case DEVLINK_VAR_ATTR_TYPE_FLAG:
 		/* Always provide flag data, regardless of its value */
 		tmp = *(bool *)msg->value;
 
 		return nla_put_u8(skb, attrtype, tmp);
-	case NLA_U8:
+	case DEVLINK_VAR_ATTR_TYPE_U8:
 		return nla_put_u8(skb, attrtype, *(u8 *)msg->value);
-	case NLA_U32:
+	case DEVLINK_VAR_ATTR_TYPE_U32:
 		return nla_put_u32(skb, attrtype, *(u32 *)msg->value);
-	case NLA_U64:
+	case DEVLINK_VAR_ATTR_TYPE_U64:
 		return devlink_nl_put_u64(skb, attrtype, *(u64 *)msg->value);
-	case NLA_NUL_STRING:
+	case DEVLINK_VAR_ATTR_TYPE_NUL_STRING:
 		return nla_put_string(skb, attrtype, (char *)&msg->value);
-	case NLA_BINARY:
+	case DEVLINK_VAR_ATTR_TYPE_BINARY:
 		return nla_put(skb, attrtype, msg->len, (void *)&msg->value);
 	default:
 		return -EINVAL;
@@ -1011,7 +987,8 @@ devlink_fmsg_prepare_skb(struct devlink_fmsg *fmsg, struct sk_buff *skb,
 			err = nla_put_flag(skb, item->attrtype);
 			break;
 		case DEVLINK_ATTR_FMSG_OBJ_VALUE_DATA:
-			err = devlink_fmsg_item_fill_type(item, skb);
+			err = nla_put_u8(skb, DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE,
+					 item->nla_type);
 			if (err)
 				break;
 			err = devlink_fmsg_item_fill_data(item, skb);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 1/4] tools: ynl-gen: allow noncontiguous enums
  2025-05-05 11:45 ` [PATCH net-next v2 1/4] tools: ynl-gen: allow noncontiguous enums Jiri Pirko
@ 2025-05-06  6:38   ` Jiri Pirko
  2025-05-07  1:22     ` Jakub Kicinski
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Pirko @ 2025-05-06  6:38 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, kuba, pabeni, saeedm, horms, donald.hunter

Mon, May 05, 2025 at 01:45:10PM +0200, jiri@resnulli.us wrote:
>From: Jiri Pirko <jiri@nvidia.com>
>
>in case the enum has holes, instead of hard stop, generate a validation
>callback to check valid enum values.
>
>signed-off-by: jiri pirko <jiri@nvidia.com>

By some accident I managed to remove uppercases from this line. Should I
repost or would you fix this during apply in case there are no changes
requested?

Thanks!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 1/4] tools: ynl-gen: allow noncontiguous enums
  2025-05-06  6:38   ` Jiri Pirko
@ 2025-05-07  1:22     ` Jakub Kicinski
  0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2025-05-07  1:22 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, edumazet, pabeni, saeedm, horms, donald.hunter

On Tue, 6 May 2025 08:38:36 +0200 Jiri Pirko wrote:
> Mon, May 05, 2025 at 01:45:10PM +0200, jiri@resnulli.us wrote:
> >From: Jiri Pirko <jiri@nvidia.com>
> >
> >in case the enum has holes, instead of hard stop, generate a validation
> >callback to check valid enum values.
> >
> >signed-off-by: jiri pirko <jiri@nvidia.com>  
> 
> By some accident I managed to remove uppercases from this line. Should I
> repost or would you fix this during apply in case there are no changes
> requested?

No worries, I don't think it's a big deal as long as the email matches.
But I'll fix when applying.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes
  2025-05-05 11:45 [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes Jiri Pirko
                   ` (3 preceding siblings ...)
  2025-05-05 11:45 ` [PATCH net-next v2 4/4] devlink: use DEVLINK_VAR_ATTR_TYPE_* instead of NLA_* in fmsg Jiri Pirko
@ 2025-05-07  1:30 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-05-07  1:30 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, edumazet, kuba, pabeni, saeedm, horms,
	donald.hunter

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  5 May 2025 13:45:09 +0200 you wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> This is continuation based on first two patches
> of https://lore.kernel.org/netdev/20250425214808.507732-1-saeed@kernel.org/
> 
> Better to take it as a separate patchset, as the rest of the original
> patchset is probably settled.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/4] tools: ynl-gen: allow noncontiguous enums
    https://git.kernel.org/netdev/net-next/c/37006af675e8
  - [net-next,v2,2/4] devlink: define enum for attr types of dynamic attributes
    https://git.kernel.org/netdev/net-next/c/429ac6211494
  - [net-next,v2,3/4] devlink: avoid param type value translations
    https://git.kernel.org/netdev/net-next/c/f9e78932eac6
  - [net-next,v2,4/4] devlink: use DEVLINK_VAR_ATTR_TYPE_* instead of NLA_* in fmsg
    https://git.kernel.org/netdev/net-next/c/88debb521f15

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-05-07  1:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-05 11:45 [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes Jiri Pirko
2025-05-05 11:45 ` [PATCH net-next v2 1/4] tools: ynl-gen: allow noncontiguous enums Jiri Pirko
2025-05-06  6:38   ` Jiri Pirko
2025-05-07  1:22     ` Jakub Kicinski
2025-05-05 11:45 ` [PATCH net-next v2 2/4] devlink: define enum for attr types of dynamic attributes Jiri Pirko
2025-05-05 11:45 ` [PATCH net-next v2 3/4] devlink: avoid param type value translations Jiri Pirko
2025-05-05 11:45 ` [PATCH net-next v2 4/4] devlink: use DEVLINK_VAR_ATTR_TYPE_* instead of NLA_* in fmsg Jiri Pirko
2025-05-07  1:30 ` [PATCH net-next v2 0/4] devlink: sanitize variable typed attributes patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).