public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC iproute2 0/6] devlink: add policy check for all attributes
@ 2022-08-05 23:41 Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 1/6] mnlg: remove unnused mnlg_socket structure Jacob Keller
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Jacob Keller @ 2022-08-05 23:41 UTC (permalink / raw)
  To: netdev
  Cc: Jacob Keller, Jonathan Corbet, Jiri Pirko, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger

This series implements code to check the kernel policy for the devlink
commands to determine whether or not attributes are supported before adding
them to netlink messages.

It implements a new mnlu_gen_get_op_policy to extract the policy
information, and uses it to implement checks when parsing option arguments.
This is intended to eventually go along with improvements to the policy
reporting in devlink kernel code to report separate policy for each command.

I think checking every attribute makes sense and is easier to follow than
only checking specific attributes. This will help ensure that future
attributes don't accidentally get sent to commands when they aren't
supported (once the devlink kernel policy is improved to report correct
information for each command separately).

Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Jiri Pirko <jiri@nvidia.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: David Ahern <dsahern@kernel.org>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: netdev@vger.kernel.org

Jacob Keller (6):
  mnlg: remove unnused mnlg_socket structure
  utils: extract CTRL_ATTR_MAXATTR and save it
  mnl_utils: add function to dump command policy
  devlink: use dl_no_arg instead of checking dl_argc == 0
  devlink: remove dl_argv_parse_put
  devlink: check attributes against policy

 devlink/devlink.c   | 846 ++++++++++++++++++++++++++++++--------------
 devlink/mnlg.c      |   8 -
 include/mnl_utils.h |  28 ++
 lib/mnl_utils.c     | 258 +++++++++++++-
 4 files changed, 858 insertions(+), 282 deletions(-)

-- 
2.37.1.208.ge72d93e88cb2


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

* [RFC iproute2 1/6] mnlg: remove unnused mnlg_socket structure
  2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
@ 2022-08-05 23:41 ` Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 2/6] utils: extract CTRL_ATTR_MAXATTR and save it Jacob Keller
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2022-08-05 23:41 UTC (permalink / raw)
  To: netdev
  Cc: Jacob Keller, Jonathan Corbet, Jiri Pirko, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger

Commit 62ff25e51bb6 ("devlink: Use generic socket helpers from library")
removed all of the users of struct mnlg_socket, but didn't remove the
structure itself. Fix that.

Fixes: 62ff25e51bb6 ("devlink: Use generic socket helpers from library")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 devlink/mnlg.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/devlink/mnlg.c b/devlink/mnlg.c
index e6d92742c150..b2e0b8c0f274 100644
--- a/devlink/mnlg.c
+++ b/devlink/mnlg.c
@@ -22,14 +22,6 @@
 #include "utils.h"
 #include "mnlg.h"
 
-struct mnlg_socket {
-	struct mnl_socket *nl;
-	char *buf;
-	uint32_t id;
-	uint8_t version;
-	unsigned int seq;
-};
-
 int mnlg_socket_send(struct mnlu_gen_socket *nlg, const struct nlmsghdr *nlh)
 {
 	return mnl_socket_sendto(nlg->nl, nlh, nlh->nlmsg_len);
-- 
2.37.1.208.ge72d93e88cb2


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

* [RFC iproute2 2/6] utils: extract CTRL_ATTR_MAXATTR and save it
  2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 1/6] mnlg: remove unnused mnlg_socket structure Jacob Keller
@ 2022-08-05 23:41 ` Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 3/6] mnl_utils: add function to dump command policy Jacob Keller
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2022-08-05 23:41 UTC (permalink / raw)
  To: netdev
  Cc: Jacob Keller, Jonathan Corbet, Jiri Pirko, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger

mnlu_gen_socket_open opens a socket and configures it for use with a
generic netlink family. As part of this process it sends a
CTRL_CMD_GETFAMILY to get the ID for the family name requested.

In addition to the family id, this command reports a few other useful
values including the maximum attribute. The maximum attribute is useful in
order to know whether a given attribute is supported and for knowing the
necessary size to allocate for other operations such as policy dumping.

Since we already have to issue a CTRL_CMD_GETFAMILY to get the id, we can
also store the maximum attribute as well. Modify the callback functions to
parse the maximum attribute NLA and store it in the mnlu_gen_socket
structure.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 include/mnl_utils.h |  1 +
 lib/mnl_utils.c     | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/mnl_utils.h b/include/mnl_utils.h
index aa5f0a9b17c7..2193934849e1 100644
--- a/include/mnl_utils.h
+++ b/include/mnl_utils.h
@@ -6,6 +6,7 @@ struct mnlu_gen_socket {
 	struct mnl_socket *nl;
 	char *buf;
 	uint32_t family;
+	uint32_t maxattr;
 	unsigned int seq;
 	uint8_t version;
 };
diff --git a/lib/mnl_utils.c b/lib/mnl_utils.c
index d5abff58d816..79bac5cfd4de 100644
--- a/lib/mnl_utils.c
+++ b/lib/mnl_utils.c
@@ -110,7 +110,7 @@ int mnlu_socket_recv_run(struct mnl_socket *nl, unsigned int seq, void *buf, siz
 	return err;
 }
 
-static int get_family_id_attr_cb(const struct nlattr *attr, void *data)
+static int get_family_attrs_cb(const struct nlattr *attr, void *data)
 {
 	int type = mnl_attr_get_type(attr);
 	const struct nlattr **tb = data;
@@ -121,20 +121,26 @@ static int get_family_id_attr_cb(const struct nlattr *attr, void *data)
 	if (type == CTRL_ATTR_FAMILY_ID &&
 	    mnl_attr_validate(attr, MNL_TYPE_U16) < 0)
 		return MNL_CB_ERROR;
+	if (type == CTRL_ATTR_MAXATTR &&
+	    mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+		return MNL_CB_ERROR;
 	tb[type] = attr;
 	return MNL_CB_OK;
 }
 
-static int get_family_id_cb(const struct nlmsghdr *nlh, void *data)
+static int get_family_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
 	struct nlattr *tb[CTRL_ATTR_MAX + 1] = {};
-	uint32_t *p_id = data;
+	struct mnlu_gen_socket *nlg = data;
 
-	mnl_attr_parse(nlh, sizeof(*genl), get_family_id_attr_cb, tb);
+	mnl_attr_parse(nlh, sizeof(*genl), get_family_attrs_cb, tb);
 	if (!tb[CTRL_ATTR_FAMILY_ID])
 		return MNL_CB_ERROR;
-	*p_id = mnl_attr_get_u16(tb[CTRL_ATTR_FAMILY_ID]);
+	if (!tb[CTRL_ATTR_MAXATTR])
+		return MNL_CB_ERROR;
+	nlg->family = mnl_attr_get_u16(tb[CTRL_ATTR_FAMILY_ID]);
+	nlg->maxattr = mnl_attr_get_u32(tb[CTRL_ATTR_MAXATTR]);
 	return MNL_CB_OK;
 }
 
@@ -159,7 +165,7 @@ static int family_get(struct mnlu_gen_socket *nlg, const char *family_name)
 
 	err = mnlu_socket_recv_run(nlg->nl, nlh->nlmsg_seq, nlg->buf,
 				   MNL_SOCKET_BUFFER_SIZE,
-				   get_family_id_cb, &nlg->family);
+				   get_family_cb, nlg);
 	return err;
 }
 
-- 
2.37.1.208.ge72d93e88cb2


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

* [RFC iproute2 3/6] mnl_utils: add function to dump command policy
  2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 1/6] mnlg: remove unnused mnlg_socket structure Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 2/6] utils: extract CTRL_ATTR_MAXATTR and save it Jacob Keller
@ 2022-08-05 23:41 ` Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 4/6] devlink: use dl_no_arg instead of checking dl_argc == 0 Jacob Keller
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2022-08-05 23:41 UTC (permalink / raw)
  To: netdev
  Cc: Jacob Keller, Jonathan Corbet, Jiri Pirko, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger

Introduce a new mnlu_get_get_op_policy function which can extract the
kernel policy data for a command. This will enable checking policy to
determine whether an attribute is accepted by a netlink command.

The policy data is reported using a new mnlu_attr_policy structure. This
allows getting a single array of policy data for a command making it easy
to check if an attribute is supported. The structure does also contain the
table of pointers to the policy data attributes which would also allow
checking both type and range.

This policy information will be used in a future change to make devlink
ensure attributes it sends are supported by the reported policy for that
command.

Only one layer of policy checking is currently supported, but an additional
mnlu_gen_get_policy_idx function could be added to support nested policy
checking in the future.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 include/mnl_utils.h |  27 +++++
 lib/mnl_utils.c     | 240 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 267 insertions(+)

diff --git a/include/mnl_utils.h b/include/mnl_utils.h
index 2193934849e1..450f304bb302 100644
--- a/include/mnl_utils.h
+++ b/include/mnl_utils.h
@@ -11,6 +11,31 @@ struct mnlu_gen_socket {
 	uint8_t version;
 };
 
+/**
+ * struct mnlu_attr_policy
+ *
+ * Structure representing the policy for a single attribute. Pass an array of
+ * at least nlg->maxattr + 1 to mnlu_gen_get_op_policy to extract the policy
+ * for a netlink op.
+ *
+ * @attr_type: the attribute type, extracted from the NLA data
+ *
+ * @valid: if set, the attribute is accepted by the policy. If not set, the
+ *         attribute is not accepted by the policy.
+ *
+ * @tb: Pointers to the NLA data. Only remains valid until a new netlink
+ *      command is sent, as it points directly to the netlink message buffer.
+ */
+struct mnlu_attr_policy {
+	/* Pointers to the nla attributes describing the policy for this
+	 * attribute. Note that these only remain valid until the next netlink
+	 * message is sent.
+	 */
+	const struct nlattr *tb[NL_POLICY_TYPE_ATTR_MAX + 1];
+	uint32_t attr_type;
+	uint8_t valid : 1;
+};
+
 int mnlu_gen_socket_open(struct mnlu_gen_socket *nlg, const char *family_name,
 			 uint8_t version);
 void mnlu_gen_socket_close(struct mnlu_gen_socket *nlg);
@@ -30,5 +55,7 @@ int mnlu_socket_recv_run(struct mnl_socket *nl, unsigned int seq, void *buf, siz
 			 mnl_cb_t cb, void *data);
 int mnlu_gen_socket_recv_run(struct mnlu_gen_socket *nlg, mnl_cb_t cb,
 			     void *data);
+int mnlu_gen_get_op_policy(struct mnlu_gen_socket *nlg, uint32_t op, bool dump,
+			   struct mnlu_attr_policy *policy);
 
 #endif /* __MNL_UTILS_H__ */
diff --git a/lib/mnl_utils.c b/lib/mnl_utils.c
index 79bac5cfd4de..f9918afd517d 100644
--- a/lib/mnl_utils.c
+++ b/lib/mnl_utils.c
@@ -252,3 +252,243 @@ int mnlu_gen_socket_recv_run(struct mnlu_gen_socket *nlg, mnl_cb_t cb,
 				    MNL_SOCKET_BUFFER_SIZE,
 				    cb, data);
 }
+
+struct policy_info {
+	struct mnlu_gen_socket *nlg;
+	struct mnlu_attr_policy *policy;
+	uint32_t policy_idx;
+	uint32_t op;
+	bool dump;
+};
+
+static int
+parse_policy_idx_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, CTRL_ATTR_POLICY_DUMP_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch (type) {
+	case CTRL_ATTR_POLICY_DO:
+	case CTRL_ATTR_POLICY_DUMP:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+			return MNL_CB_ERROR;
+		break;
+	default:
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static int
+parse_policy_idx(const struct nlattr *attr, struct policy_info *info)
+{
+	struct nlattr *tb[CTRL_ATTR_POLICY_DUMP_MAX + 1] = {};
+	struct nlattr *idx_attr;
+	uint32_t op;
+
+	/* The type of this nested attribute is the op */
+	op = mnl_attr_get_type(attr);
+
+	/* Skip ops we're not interested in */
+	if (op != info->op)
+		return MNL_CB_OK;
+
+	mnl_attr_parse_nested(attr, parse_policy_idx_cb, tb);
+	if (info->dump)
+		idx_attr = tb[CTRL_ATTR_POLICY_DUMP];
+	else
+		idx_attr = tb[CTRL_ATTR_POLICY_DO];
+	if (!idx_attr)
+		return MNL_CB_ERROR;
+
+	info->policy_idx = mnl_attr_get_u32(idx_attr);
+
+	return MNL_CB_OK;
+}
+
+static int
+parse_policy_attr_data_cb(const struct nlattr *attr, void *data)
+{
+	struct mnlu_attr_policy *policy = data;
+	int type = mnl_attr_get_type(attr);
+
+	/* Unknown policy type attributes are ignored */
+	if (mnl_attr_type_valid(attr, NL_POLICY_TYPE_ATTR_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch (type) {
+	case NL_POLICY_TYPE_ATTR_TYPE:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+			return MNL_CB_ERROR;
+
+		policy->attr_type = mnl_attr_get_u32(attr);
+		policy->valid = 1;
+		break;
+	case NL_POLICY_TYPE_ATTR_MIN_VALUE_S:
+		/* libmnl doesn't yet have s64 */
+		if (mnl_attr_validate(attr, MNL_TYPE_U64) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_MAX_VALUE_S:
+		/* libmnl doesn't yet have s64 */
+		if (mnl_attr_validate(attr, MNL_TYPE_U64) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_MIN_VALUE_U:
+		if (mnl_attr_validate(attr, MNL_TYPE_U64) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_MAX_VALUE_U:
+		if (mnl_attr_validate(attr, MNL_TYPE_U64) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_MIN_LENGTH:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_MAX_LENGTH:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_POLICY_IDX:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_POLICY_MAXTYPE:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_BITFIELD32_MASK:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+			return MNL_CB_ERROR;
+		break;
+	case NL_POLICY_TYPE_ATTR_MASK:
+		if (mnl_attr_validate(attr, MNL_TYPE_U64) < 0)
+			return MNL_CB_ERROR;
+		break;
+	default:
+		break;
+	}
+
+	policy->tb[type] = attr;
+
+	return MNL_CB_OK;
+}
+
+static int
+parse_policy_data(const struct nlattr *attr, struct policy_info *info)
+{
+	const struct nlattr *nested_attr;
+	int policy_idx, attr_id;
+
+	/* The type of this nested attribute is the policy index */
+	policy_idx = mnl_attr_get_type(attr);
+
+	/* Skip policies we're not interested in */
+	if (policy_idx != info->policy_idx)
+		return MNL_CB_OK;
+
+	if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0)
+		return MNL_CB_ERROR;
+	nested_attr = mnl_attr_get_payload(attr);
+	if (!mnl_attr_ok(nested_attr, mnl_attr_get_payload_len(attr)))
+		return MNL_CB_ERROR;
+	if (mnl_attr_validate(nested_attr, MNL_TYPE_NESTED) < 0)
+		return MNL_CB_ERROR;
+	if (mnl_attr_type_valid(attr, info->nlg->maxattr) < 0)
+		return MNL_CB_ERROR;
+
+	attr_id = mnl_attr_get_type(nested_attr);
+
+	return mnl_attr_parse_nested(nested_attr, parse_policy_attr_data_cb,
+				     &info->policy[attr_id]);
+}
+
+static int get_policy_attrs_cb(const struct nlattr *attr, void *data)
+{
+	int type = mnl_attr_get_type(attr);
+	struct policy_info *info = data;
+	struct nlattr *nested_attr;
+
+	if (mnl_attr_type_valid(attr, CTRL_ATTR_MAX) < 0)
+		return MNL_CB_ERROR;
+
+	switch (type) {
+	case CTRL_ATTR_OP_POLICY:
+	case CTRL_ATTR_POLICY:
+		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0)
+			return MNL_CB_ERROR;
+
+		nested_attr = mnl_attr_get_payload(attr);
+		if (!mnl_attr_ok(nested_attr, mnl_attr_get_payload_len(attr)))
+			return MNL_CB_ERROR;
+
+		if (mnl_attr_validate(nested_attr, MNL_TYPE_NESTED) < 0)
+			return MNL_CB_ERROR;
+
+		if (type == CTRL_ATTR_OP_POLICY)
+			return parse_policy_idx(nested_attr, info);
+
+		return parse_policy_data(nested_attr, info);
+	default:
+		break;
+	}
+
+	return MNL_CB_OK;
+}
+
+static int get_policy_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+
+	return mnl_attr_parse(nlh, sizeof(*genl), get_policy_attrs_cb, data);
+}
+
+/**
+ * mnlu_get_op_policy - Get policy for a specific op.
+ * @nlg: the socket to get policy on
+ * @op: the op to get the policy for
+ * @dump: if true, get the policy NLM_F_DUMP
+ * @policy: an array of size nlg->maxattr used to return policy data
+ *
+ * Uses CTRL_CMD_GETPOLICY to extract policy information from the kernel for
+ * the specified op. The data is extracted into the provided policy buffer
+ * which is expected to be of size nlg->maxattr.
+ *
+ * This function must be called after mnlu_gen_socket_open.
+ */
+int mnlu_gen_get_op_policy(struct mnlu_gen_socket *nlg, uint32_t op, bool dump,
+			   struct mnlu_attr_policy *policy)
+{
+	struct policy_info info = {};
+	struct genlmsghdr hdr = {};
+	struct nlmsghdr *nlh;
+	int err;
+
+	info.nlg = nlg;
+	info.policy = policy;
+	info.dump = dump;
+	info.op = op;
+
+	hdr.cmd = CTRL_CMD_GETPOLICY;
+	hdr.version = 0x1;
+
+	nlh = mnlu_msg_prepare(nlg->buf, GENL_ID_CTRL,
+			       NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP,
+			       &hdr, sizeof(hdr));
+
+	mnl_attr_put_u16(nlh, CTRL_ATTR_FAMILY_ID, nlg->family);
+	mnl_attr_put_u32(nlh, CTRL_ATTR_OP, op);
+
+	err = mnl_socket_sendto(nlg->nl, nlh, nlh->nlmsg_len);
+	if (err < 0)
+		return err;
+
+	return mnlu_socket_recv_run(nlg->nl, nlh->nlmsg_seq, nlg->buf,
+				    MNL_SOCKET_BUFFER_SIZE,
+				    get_policy_cb, &info);
+}
-- 
2.37.1.208.ge72d93e88cb2


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

* [RFC iproute2 4/6] devlink: use dl_no_arg instead of checking dl_argc == 0
  2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
                   ` (2 preceding siblings ...)
  2022-08-05 23:41 ` [RFC iproute2 3/6] mnl_utils: add function to dump command policy Jacob Keller
@ 2022-08-05 23:41 ` Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 5/6] devlink: remove dl_argv_parse_put Jacob Keller
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2022-08-05 23:41 UTC (permalink / raw)
  To: netdev
  Cc: Jacob Keller, Jonathan Corbet, Jiri Pirko, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger

Use the helper dl_no_arg function to check for whether the command has any
arguments.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 devlink/devlink.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 21f26246f91b..485b1a82f9ef 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -3338,7 +3338,7 @@ static int cmd_dev_param_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PARAM_GET, flags);
@@ -3494,7 +3494,7 @@ static int cmd_dev_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_GET, flags);
@@ -3714,7 +3714,7 @@ static int cmd_dev_info(struct dl *dl)
 		return 0;
 	}
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_INFO_GET, flags);
@@ -4517,7 +4517,7 @@ static int cmd_port_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_GET, flags);
@@ -4585,7 +4585,7 @@ static int cmd_port_param_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_PARAM_GET,
@@ -4939,7 +4939,7 @@ static int cmd_port_fn_rate_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_RATE_GET, flags);
@@ -5303,7 +5303,7 @@ static int cmd_linecard_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_LINECARD_GET,
@@ -5418,7 +5418,7 @@ static int cmd_sb_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_GET, flags);
@@ -5495,7 +5495,7 @@ static int cmd_sb_pool_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_POOL_GET, flags);
@@ -5580,7 +5580,7 @@ static int cmd_sb_port_pool_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_PORT_POOL_GET, flags);
@@ -5684,7 +5684,7 @@ static int cmd_sb_tc_bind_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_TC_POOL_BIND_GET, flags);
@@ -8354,7 +8354,7 @@ static int cmd_region_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_REGION_GET, flags);
@@ -9024,7 +9024,7 @@ static int __cmd_health_show(struct dl *dl, bool show_device, bool show_port)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_GET,
 			       flags);
@@ -9216,7 +9216,7 @@ static int cmd_trap_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_GET, flags);
@@ -9291,7 +9291,7 @@ static int cmd_trap_group_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_GROUP_GET, flags);
@@ -9388,7 +9388,7 @@ static int cmd_trap_policer_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_argc(dl) == 0)
+	if (dl_no_arg(dl))
 		flags |= NLM_F_DUMP;
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_POLICER_GET, flags);
-- 
2.37.1.208.ge72d93e88cb2


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

* [RFC iproute2 5/6] devlink: remove dl_argv_parse_put
  2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
                   ` (3 preceding siblings ...)
  2022-08-05 23:41 ` [RFC iproute2 4/6] devlink: use dl_no_arg instead of checking dl_argc == 0 Jacob Keller
@ 2022-08-05 23:41 ` Jacob Keller
  2022-08-05 23:41 ` [RFC iproute2 6/6] devlink: check attributes against policy Jacob Keller
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2022-08-05 23:41 UTC (permalink / raw)
  To: netdev
  Cc: Jacob Keller, Jonathan Corbet, Jiri Pirko, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger

The dl_argv_parse_put function is used to extract arguments from the
command line and convert them to the appropriate netlink attributes. This
function is a combination of calling dl_argv_parse and dl_put_opts.

A future change is going to refactor dl_argv_parse to check the kernel's
netlink policy for the command. This requires issuing another netlink
message which requires calling dl_argv_parse before
mnlu_gen_socket_cmd_prepare. Otherwise, the get policy command issued in
dl_argv_parse would overwrite the prepared buffer.

This conflicts with dl_argv_parse_put which requires being called after
mnlu_gen_socket_cmd_prepare.

Remove dl_argv_parse_put and replace it with appropriate calls to
dl_argv_parse and dl_put_opts. This allows us to ensure dl_argv_parse is
called before mnlu_gen_socket_cmd_prepare while dl_put_opts is called
afterwards.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 devlink/devlink.c | 566 ++++++++++++++++++++++++++--------------------
 1 file changed, 317 insertions(+), 249 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 485b1a82f9ef..67a57e9ba550 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2318,18 +2318,6 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
 				  opts->linecard_type);
 }
 
-static int dl_argv_parse_put(struct nlmsghdr *nlh, struct dl *dl,
-			     uint64_t o_required, uint64_t o_optional)
-{
-	int err;
-
-	err = dl_argv_parse(dl, o_required, o_optional);
-	if (err)
-		return err;
-	dl_opts_put(nlh, dl);
-	return 0;
-}
-
 static bool dl_dump_filter(struct dl *dl, struct nlattr **tb)
 {
 	struct dl_opts *opts = &dl->opts;
@@ -2818,12 +2806,14 @@ static int cmd_dev_eswitch_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_ESWITCH_GET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	pr_out_section_start(dl, "dev");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_dev_eswitch_show_cb, dl);
@@ -2836,16 +2826,17 @@ static int cmd_dev_eswitch_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE,
+			    DL_OPT_ESWITCH_MODE |
+			    DL_OPT_ESWITCH_INLINE_MODE |
+			    DL_OPT_ESWITCH_ENCAP_MODE);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_ESWITCH_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE,
-				DL_OPT_ESWITCH_MODE |
-				DL_OPT_ESWITCH_INLINE_MODE |
-				DL_OPT_ESWITCH_ENCAP_MODE);
-
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	if (dl->opts.present == 1) {
 		pr_err("Need to set at least one option\n");
@@ -3338,18 +3329,18 @@ static int cmd_dev_param_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PARAM_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE |
-					DL_OPT_PARAM_NAME, 0);
+	} else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_PARAM_NAME, 0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PARAM_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "param");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_dev_param_show_cb, dl);
 	pr_out_section_end(dl);
@@ -3494,17 +3485,19 @@ static int cmd_dev_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE, 0);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "dev");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_dev_show_cb, dl);
 	pr_out_section_end(dl);
@@ -3572,14 +3565,16 @@ static int cmd_dev_reload(struct dl *dl)
 		return 0;
 	}
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE,
+			    DL_OPT_NETNS | DL_OPT_RELOAD_ACTION |
+			    DL_OPT_RELOAD_LIMIT);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_RELOAD,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE,
-				DL_OPT_NETNS | DL_OPT_RELOAD_ACTION |
-				DL_OPT_RELOAD_LIMIT);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_dev_reload_cb, dl);
 }
@@ -3714,17 +3709,19 @@ static int cmd_dev_info(struct dl *dl)
 		return 0;
 	}
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_INFO_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE, 0);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_INFO_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "info");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_versions_show_cb, dl);
 	pr_out_section_end(dl);
@@ -3979,13 +3976,15 @@ static int cmd_dev_flash(struct dl *dl)
 		return 0;
 	}
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_FLASH_FILE_NAME,
+			    DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_FLASH_UPDATE,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_FLASH_FILE_NAME,
-				DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	err = mnlu_gen_socket_open(&nlg_ntf, DEVLINK_GENL_NAME,
 				   DEVLINK_GENL_VERSION);
@@ -4517,17 +4516,19 @@ static int cmd_port_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP, 0);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLEP, 0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "port");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_port_show_cb, dl);
 	pr_out_section_end(dl);
@@ -4539,12 +4540,14 @@ static int cmd_port_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_PORT_TYPE, 0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP | DL_OPT_PORT_TYPE, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -4554,12 +4557,14 @@ static int cmd_port_split(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_PORT_COUNT, 0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_SPLIT,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP | DL_OPT_PORT_COUNT, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -4569,12 +4574,14 @@ static int cmd_port_unsplit(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLEP, 0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_UNSPLIT,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -4585,18 +4592,19 @@ static int cmd_port_param_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_PARAM_NAME, 0);
+		if (err)
+			return err;
+	}
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_PARAM_GET,
 					  flags);
 
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP |
-					DL_OPT_PARAM_NAME, 0);
-		if (err)
-			return err;
-	}
+	dl_opts_put(nlh, dl);
 
 	pr_out_section_start(dl, "param");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_port_param_show_cb, dl);
@@ -4620,13 +4628,15 @@ static int cmd_port_function_set(struct dl *dl)
 		cmd_port_function_help();
 		return 0;
 	}
+	err = dl_argv_parse(dl, DL_OPT_HANDLEP,
+			    DL_OPT_PORT_FUNCTION_HW_ADDR | DL_OPT_PORT_FUNCTION_STATE);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_SET,
 					  NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP,
-				DL_OPT_PORT_FUNCTION_HW_ADDR | DL_OPT_PORT_FUNCTION_STATE);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -4939,18 +4949,21 @@ static int cmd_port_fn_rate_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_RATE_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP |
-					DL_OPT_PORT_FN_RATE_NODE_NAME, 0);
+	}
+	else {
+		err = dl_argv_parse(dl,
+				    DL_OPT_HANDLEP | DL_OPT_PORT_FN_RATE_NODE_NAME,
+				    0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_RATE_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "rate");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_port_fn_rate_show_cb, dl);
 	pr_out_section_end(dl);
@@ -4982,14 +4995,15 @@ static int cmd_port_fn_rate_add(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_RATE_NEW,
-					  NLM_F_REQUEST | NLM_F_ACK);
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_PORT_FN_RATE_NODE_NAME,
-				DL_OPT_PORT_FN_RATE_TX_SHARE |
-				DL_OPT_PORT_FN_RATE_TX_MAX);
+	err = dl_argv_parse(dl, DL_OPT_PORT_FN_RATE_NODE_NAME,
+			    DL_OPT_PORT_FN_RATE_TX_SHARE | DL_OPT_PORT_FN_RATE_TX_MAX);
 	if (err)
 		return err;
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_RATE_NEW,
+					  NLM_F_REQUEST | NLM_F_ACK);
+	dl_opts_put(nlh, dl);
+
 	if ((dl->opts.present & DL_OPT_PORT_FN_RATE_TX_SHARE) &&
 	    (dl->opts.present & DL_OPT_PORT_FN_RATE_TX_MAX)) {
 		err = port_fn_check_tx_rates(dl->opts.rate_tx_share,
@@ -5006,12 +5020,14 @@ static int cmd_port_fn_rate_del(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_RATE_DEL,
-					  NLM_F_REQUEST | NLM_F_ACK);
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_PORT_FN_RATE_NODE_NAME, 0);
+	err = dl_argv_parse(dl, DL_OPT_PORT_FN_RATE_NODE_NAME, 0);
 	if (err)
 		return err;
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_RATE_DEL,
+					  NLM_F_REQUEST | NLM_F_ACK);
+	dl_opts_put(nlh, dl);
+
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
 
@@ -5139,14 +5155,16 @@ static int cmd_port_add(struct dl *dl)
 		return 0;
 	}
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HANDLEP |
+			    DL_OPT_PORT_FLAVOUR | DL_OPT_PORT_PFNUMBER,
+			    DL_OPT_PORT_SFNUMBER | DL_OPT_PORT_CONTROLLER);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_NEW,
 					  NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_HANDLEP |
-				DL_OPT_PORT_FLAVOUR | DL_OPT_PORT_PFNUMBER,
-				DL_OPT_PORT_SFNUMBER | DL_OPT_PORT_CONTROLLER);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_port_show_cb, dl);
 }
@@ -5166,12 +5184,14 @@ static int cmd_port_del(struct dl *dl)
 		return 0;
 	}
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLEP, 0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_PORT_DEL,
 					  NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -5303,18 +5323,19 @@ static int cmd_linecard_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_LINECARD);
+		if (err)
+			return err;
+	}
 
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_LINECARD_GET,
 					  flags);
 
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE,
-					DL_OPT_LINECARD);
-		if (err)
-			return err;
-	}
+	dl_opts_put(nlh, dl);
 
 	pr_out_section_start(dl, "lc");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_linecard_show_cb, dl);
@@ -5327,13 +5348,15 @@ static int cmd_linecard_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_LINECARD |
+			    DL_OPT_LINECARD_TYPE, 0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_LINECARD_SET,
 					  NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_LINECARD |
-					 DL_OPT_LINECARD_TYPE, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -5418,17 +5441,19 @@ static int cmd_sb_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE, DL_OPT_SB);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_SB);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "sb");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_sb_show_cb, dl);
 	pr_out_section_end(dl);
@@ -5495,18 +5520,20 @@ static int cmd_sb_pool_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_POOL_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_SB_POOL,
-					DL_OPT_SB);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_SB_POOL,
+				    DL_OPT_SB);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_POOL_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "pool");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_sb_pool_show_cb, dl);
 	pr_out_section_end(dl);
@@ -5518,13 +5545,15 @@ static int cmd_sb_pool_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_SB_POOL |
+			    DL_OPT_SB_SIZE | DL_OPT_SB_THTYPE, DL_OPT_SB);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_POOL_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_SB_POOL |
-				DL_OPT_SB_SIZE | DL_OPT_SB_THTYPE, DL_OPT_SB);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -5580,19 +5609,20 @@ static int cmd_sb_port_pool_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_PORT_POOL_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl,
-					DL_OPT_HANDLEP | DL_OPT_SB_POOL,
-					DL_OPT_SB);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_SB_POOL,
+				    DL_OPT_SB);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_PORT_POOL_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "port_pool");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_sb_port_pool_show_cb, dl);
 	pr_out_section_end(dl);
@@ -5604,13 +5634,15 @@ static int cmd_sb_port_pool_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_SB_POOL | DL_OPT_SB_TH,
+			    DL_OPT_SB);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_PORT_POOL_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP | DL_OPT_SB_POOL |
-				DL_OPT_SB_TH, DL_OPT_SB);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -5684,18 +5716,20 @@ static int cmd_sb_tc_bind_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_TC_POOL_BIND_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP | DL_OPT_SB_TC |
-					DL_OPT_SB_TYPE, DL_OPT_SB);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_SB_TC |
+				    DL_OPT_SB_TYPE, DL_OPT_SB);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_TC_POOL_BIND_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "tc_bind");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_sb_tc_bind_show_cb, dl);
 	pr_out_section_end(dl);
@@ -5707,14 +5741,16 @@ static int cmd_sb_tc_bind_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_SB_TC |
+			    DL_OPT_SB_TYPE | DL_OPT_SB_POOL | DL_OPT_SB_TH,
+			    DL_OPT_SB);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_TC_POOL_BIND_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP | DL_OPT_SB_TC |
-				DL_OPT_SB_TYPE | DL_OPT_SB_POOL | DL_OPT_SB_TH,
-				DL_OPT_SB);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -6061,12 +6097,14 @@ static int cmd_sb_occ_snapshot(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_SB);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_OCC_SNAPSHOT,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE, DL_OPT_SB);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -6076,12 +6114,14 @@ static int cmd_sb_occ_clearmax(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_SB);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_SB_OCC_MAX_CLEAR,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE, DL_OPT_SB);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -6983,12 +7023,14 @@ static int cmd_dpipe_headers_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_DPIPE_HEADERS_GET, flags);
-
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE, 0);
+	err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
 	if (err)
 		return err;
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_DPIPE_HEADERS_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	err = dpipe_ctx_init(&ctx, dl);
 	if (err)
 		return err;
@@ -7436,14 +7478,15 @@ static int cmd_dpipe_table_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_DPIPE_TABLE_NAME |
+			    DL_OPT_DPIPE_TABLE_COUNTERS, 0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl,
-				DL_OPT_HANDLE | DL_OPT_DPIPE_TABLE_NAME |
-				DL_OPT_DPIPE_TABLE_COUNTERS, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -8354,17 +8397,19 @@ static int cmd_region_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_REGION_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE_REGION, 0);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLE_REGION, 0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_REGION_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "regions");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_region_show_cb, dl);
 	pr_out_section_end(dl);
@@ -8376,13 +8421,15 @@ static int cmd_region_snapshot_del(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE_REGION |
+			    DL_OPT_REGION_SNAPSHOT_ID, 0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_REGION_DEL,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE_REGION |
-				DL_OPT_REGION_SNAPSHOT_ID, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -8426,13 +8473,16 @@ static int cmd_region_dump(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl,
+			    DL_OPT_HANDLE_REGION | DL_OPT_REGION_SNAPSHOT_ID,
+			    0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_REGION_READ,
 			       NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE_REGION |
-				DL_OPT_REGION_SNAPSHOT_ID, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	pr_out_section_start(dl, "dump");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_region_read_cb, dl);
@@ -8447,14 +8497,16 @@ static int cmd_region_read(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE_REGION | DL_OPT_REGION_ADDRESS |
+			    DL_OPT_REGION_LENGTH | DL_OPT_REGION_SNAPSHOT_ID,
+			    0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_REGION_READ,
 			       NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE_REGION |
-				DL_OPT_REGION_ADDRESS | DL_OPT_REGION_LENGTH |
-				DL_OPT_REGION_SNAPSHOT_ID, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	pr_out_section_start(dl, "read");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_region_read_cb, dl);
@@ -8486,13 +8538,15 @@ static int cmd_region_snapshot_new(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE_REGION,
+			    DL_OPT_REGION_SNAPSHOT_ID);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_REGION_NEW,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE_REGION,
-				DL_OPT_REGION_SNAPSHOT_ID);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	pr_out_section_start(dl, "regions");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_region_snapshot_new_cb, dl);
@@ -8559,14 +8613,16 @@ static int cmd_health_dump_clear(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HANDLEP |
+			    DL_OPT_HEALTH_REPORTER_NAME,
+			    0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl,
-				DL_OPT_HANDLE | DL_OPT_HANDLEP |
-				DL_OPT_HEALTH_REPORTER_NAME, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	dl_opts_put(nlh, dl);
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
@@ -8810,14 +8866,16 @@ static int cmd_health_object_common(struct dl *dl, uint8_t cmd, uint16_t flags)
 	struct nlmsghdr *nlh;
 	int err;
 
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, cmd, flags | NLM_F_REQUEST | NLM_F_ACK);
-
-	err = dl_argv_parse_put(nlh, dl,
-				DL_OPT_HANDLE | DL_OPT_HANDLEP |
-				DL_OPT_HEALTH_REPORTER_NAME, 0);
+	err = dl_argv_parse(dl,
+			    DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_HEALTH_REPORTER_NAME,
+			    0);
 	if (err)
 		return err;
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, cmd, flags | NLM_F_REQUEST | NLM_F_ACK);
+
+	dl_opts_put(nlh, dl);
+
 	cmd_fmsg_init(dl, &data);
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_fmsg_object_cb, &data);
 	free(data.name);
@@ -8850,14 +8908,16 @@ static int cmd_health_recover(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HANDLEP |
+			    DL_OPT_HEALTH_REPORTER_NAME,
+			    0);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_RECOVER,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl,
-				DL_OPT_HANDLE | DL_OPT_HANDLEP |
-				DL_OPT_HEALTH_REPORTER_NAME, 0);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	dl_opts_put(nlh, dl);
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
@@ -9024,19 +9084,21 @@ static int __cmd_health_show(struct dl *dl, bool show_device, bool show_port)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_GET,
-			       flags);
-
-	if (dl_argc(dl) > 0) {
+	} else {
 		ctx.show_port = true;
-		err = dl_argv_parse_put(nlh, dl,
-					DL_OPT_HANDLE | DL_OPT_HANDLEP |
-					DL_OPT_HEALTH_REPORTER_NAME, 0);
+		err = dl_argv_parse(dl,
+				    DL_OPT_HANDLE | DL_OPT_HANDLEP |
+				    DL_OPT_HEALTH_REPORTER_NAME, 0);
 		if (err)
 			return err;
 	}
+
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_GET,
+			       flags);
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "health");
 
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_health_show_cb, &ctx);
@@ -9216,18 +9278,19 @@ static int cmd_trap_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl,
-					DL_OPT_HANDLE | DL_OPT_TRAP_NAME, 0);
+	}
+	else {
+		err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_TRAP_NAME, 0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "trap");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_trap_show_cb, dl);
 	pr_out_section_end(dl);
@@ -9240,13 +9303,15 @@ static int cmd_trap_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_TRAP_NAME,
+			    DL_OPT_TRAP_ACTION);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_TRAP_NAME,
-				DL_OPT_TRAP_ACTION);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -9291,19 +9356,20 @@ static int cmd_trap_group_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_GROUP_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl,
-					DL_OPT_HANDLE | DL_OPT_TRAP_GROUP_NAME,
-					0);
+	}
+	else {
+		err = dl_argv_parse(dl,
+				    DL_OPT_HANDLE | DL_OPT_TRAP_GROUP_NAME, 0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_GROUP_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "trap_group");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_trap_group_show_cb, dl);
 	pr_out_section_end(dl);
@@ -9316,14 +9382,15 @@ static int cmd_trap_group_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_TRAP_GROUP_NAME,
+			    DL_OPT_TRAP_ACTION | DL_OPT_TRAP_POLICER_ID);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_GROUP_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl,
-				DL_OPT_HANDLE | DL_OPT_TRAP_GROUP_NAME,
-				DL_OPT_TRAP_ACTION | DL_OPT_TRAP_POLICER_ID);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -9388,19 +9455,20 @@ static int cmd_trap_policer_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	if (dl_no_arg(dl))
+	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
-
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_POLICER_GET, flags);
-
-	if (dl_argc(dl) > 0) {
-		err = dl_argv_parse_put(nlh, dl,
-					DL_OPT_HANDLE | DL_OPT_TRAP_POLICER_ID,
-					0);
+	}
+	else {
+		err = dl_argv_parse(dl,
+				    DL_OPT_HANDLE | DL_OPT_TRAP_POLICER_ID, 0);
 		if (err)
 			return err;
 	}
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_POLICER_GET, flags);
+
+	dl_opts_put(nlh, dl);
+
 	pr_out_section_start(dl, "trap_policer");
 	err = mnlu_gen_socket_sndrcv(&dl->nlg, nlh, cmd_trap_policer_show_cb, dl);
 	pr_out_section_end(dl);
@@ -9413,15 +9481,15 @@ static int cmd_trap_policer_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_TRAP_POLICER_ID,
+			    DL_OPT_TRAP_POLICER_RATE | DL_OPT_TRAP_POLICER_BURST);
+	if (err)
+		return err;
+
 	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_TRAP_POLICER_SET,
 			       NLM_F_REQUEST | NLM_F_ACK);
 
-	err = dl_argv_parse_put(nlh, dl,
-				DL_OPT_HANDLE | DL_OPT_TRAP_POLICER_ID,
-				DL_OPT_TRAP_POLICER_RATE |
-				DL_OPT_TRAP_POLICER_BURST);
-	if (err)
-		return err;
+	dl_opts_put(nlh, dl);
 
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
-- 
2.37.1.208.ge72d93e88cb2


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

* [RFC iproute2 6/6] devlink: check attributes against policy
  2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
                   ` (4 preceding siblings ...)
  2022-08-05 23:41 ` [RFC iproute2 5/6] devlink: remove dl_argv_parse_put Jacob Keller
@ 2022-08-05 23:41 ` Jacob Keller
  2022-08-08 10:32 ` [RFC iproute2 0/6] devlink: add policy check for all attributes Jiri Pirko
  2022-08-09  9:50 ` Jiri Pirko
  7 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2022-08-05 23:41 UTC (permalink / raw)
  To: netdev
  Cc: Jacob Keller, Jonathan Corbet, Jiri Pirko, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger

Current versions of the Linux kernel do not strictly validate attributes
for some devlink commands. Additionally, these kernels also report a fixed
policy for all commands rather than using a separate policy tailored to the
attributes accepted by that specific command.

This could lead to problems in the future if a new attribute is added to an
existing command. Userspace might issue a command and expect some behavior,
but the kernel could be silently ignoring the attribute.

To protect against this, modify the devlink userspace to check each
attribute before adding it to the header. This relies on the recently added
mnlu_gen_get_op_policy function that will extract the policy for the
command from the kernel.

Each attribute is checked to make sure the kernel recognizes it as valid
for the command. This ensures that the devlink userspace won't add an
attribute which is not supported by the kernel.

This definitely protects the userspace from silently passing unknown
attributes: the kernel policy would not include any attributes which are
not known at the point of its compilation. This is equivalent to checking
the maximum attribute repoorted by CTRL_CMD_GETFAMILY.

This is also a necessary step for protecting against an attribute which the
kernel knows about in one version, but only becomes accepted by a command
in a later version. Once the kernel has been fixed to provide per-command
policy instead of sharing the same policy for all commands, this code will
also help protect against accidentally sending an attribute not honored by
the kernel.

Given the amount of places which need to handle the DL_OPT_* ->
DEVLINK_ATTR_*, it might be worth investigating addition of a structured
lookup table or other flow to reduce some of this duplicated work.

It might also be worth adding a single helper which does
dl_argv_parse_checked, mnlu_gen_socket_cmd_prepare and then dl_opts_put.
This combination is done for almost all devlink commands.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 devlink/devlink.c | 452 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 347 insertions(+), 105 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 67a57e9ba550..ae017bb2a5bf 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2083,6 +2083,206 @@ static int dl_argv_parse(struct dl *dl, uint64_t o_required,
 	return dl_args_finding_required_validate(o_required, o_found);
 }
 
+static int dl_opts_check(struct dl *dl, uint32_t cmd)
+{
+	struct dl_opts *opts = &dl->opts;
+	struct mnlu_attr_policy *policy;
+	int err;
+
+	policy = calloc(DEVLINK_ATTR_MAX + 1, sizeof(*policy));
+	if (!policy)
+		return -ENOMEM;
+
+	/* We only check the DO policy currently, since none of the commands
+	 * with NLM_F_DUMP send any attributes.
+	 */
+	err = mnlu_gen_get_op_policy(&dl->nlg, cmd, false, policy);
+	if (err)
+		return err;
+
+	if (opts->present & DL_OPT_HANDLE) {
+		if (!policy[DEVLINK_ATTR_BUS_NAME].valid ||
+		    !policy[DEVLINK_ATTR_DEV_NAME].valid)
+			return -EOPNOTSUPP;
+	} else if (opts->present & DL_OPT_HANDLEP) {
+		if (!policy[DEVLINK_ATTR_BUS_NAME].valid ||
+		    !policy[DEVLINK_ATTR_DEV_NAME].valid ||
+		    !policy[DEVLINK_ATTR_PORT_INDEX].valid)
+			return -EOPNOTSUPP;
+	} else if (opts->present & DL_OPT_HANDLE_REGION) {
+		if (!policy[DEVLINK_ATTR_BUS_NAME].valid ||
+		    !policy[DEVLINK_ATTR_DEV_NAME].valid ||
+		    !policy[DEVLINK_ATTR_REGION_NAME].valid)
+			return -EOPNOTSUPP;
+	} else if (opts->present & DL_OPT_PORT_FN_RATE_NODE_NAME) {
+		if (!policy[DEVLINK_ATTR_BUS_NAME].valid ||
+		    !policy[DEVLINK_ATTR_DEV_NAME].valid ||
+		    !policy[DEVLINK_ATTR_RATE_NODE_NAME].valid)
+			return -EOPNOTSUPP;
+	}
+	if (opts->present & DL_OPT_PORT_TYPE)
+		if (!policy[DEVLINK_ATTR_PORT_TYPE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_COUNT)
+		if (!policy[DEVLINK_ATTR_PORT_SPLIT_COUNT].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_SB)
+		if (!policy[DEVLINK_ATTR_SB_INDEX].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_SB_POOL)
+		if (!policy[DEVLINK_ATTR_SB_POOL_INDEX].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_SB_SIZE)
+		if (!policy[DEVLINK_ATTR_SB_POOL_SIZE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_SB_TYPE)
+		if (!policy[DEVLINK_ATTR_SB_POOL_TYPE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_SB_THTYPE)
+		if (!policy[DEVLINK_ATTR_SB_POOL_THRESHOLD_TYPE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_SB_TH)
+		if (!policy[DEVLINK_ATTR_SB_THRESHOLD].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_SB_TC)
+		if (!policy[DEVLINK_ATTR_SB_TC_INDEX].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_ESWITCH_MODE)
+		if (!policy[DEVLINK_ATTR_ESWITCH_MODE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_ESWITCH_INLINE_MODE)
+		if (!policy[DEVLINK_ATTR_ESWITCH_INLINE_MODE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_DPIPE_TABLE_NAME)
+		if (!policy[DEVLINK_ATTR_DPIPE_TABLE_NAME].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_DPIPE_TABLE_COUNTERS)
+		if (!policy[DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_ESWITCH_ENCAP_MODE)
+		if (!policy[DEVLINK_ATTR_ESWITCH_ENCAP_MODE].valid)
+			return -EOPNOTSUPP;
+	if ((opts->present & DL_OPT_RESOURCE_PATH) && opts->resource_id_valid)
+		if (!policy[DEVLINK_ATTR_RESOURCE_ID].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_RESOURCE_SIZE)
+		if (!policy[DEVLINK_ATTR_RESOURCE_SIZE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PARAM_NAME)
+		if (!policy[DEVLINK_ATTR_PARAM_NAME].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PARAM_CMODE)
+		if (!policy[DEVLINK_ATTR_PARAM_VALUE_CMODE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_REGION_SNAPSHOT_ID)
+		if (!policy[DEVLINK_ATTR_REGION_SNAPSHOT_ID].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_REGION_ADDRESS)
+		if (!policy[DEVLINK_ATTR_REGION_CHUNK_ADDR].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_REGION_LENGTH)
+		if (!policy[DEVLINK_ATTR_REGION_CHUNK_LEN].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_FLASH_FILE_NAME)
+		if (!policy[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_FLASH_COMPONENT)
+		if (!policy[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_FLASH_OVERWRITE)
+		if (!policy[DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_HEALTH_REPORTER_NAME)
+		if (!policy[DEVLINK_ATTR_HEALTH_REPORTER_NAME].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD)
+		if (!policy[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_HEALTH_REPORTER_AUTO_RECOVER)
+		if (!policy[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_HEALTH_REPORTER_AUTO_DUMP)
+		if (!policy[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_DUMP].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_TRAP_NAME)
+		if (!policy[DEVLINK_ATTR_TRAP_NAME].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_TRAP_GROUP_NAME)
+		if (!policy[DEVLINK_ATTR_TRAP_GROUP_NAME].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_TRAP_ACTION)
+		if (!policy[DEVLINK_ATTR_TRAP_ACTION].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_NETNS)
+		if (!policy[opts->netns_is_pid ? DEVLINK_ATTR_NETNS_PID :
+		    DEVLINK_ATTR_NETNS_FD].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_RELOAD_ACTION)
+		if (!policy[DEVLINK_ATTR_RELOAD_ACTION].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_RELOAD_LIMIT)
+		if (!policy[DEVLINK_ATTR_RELOAD_LIMITS].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_TRAP_POLICER_ID)
+		if (!policy[DEVLINK_ATTR_TRAP_POLICER_ID].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_TRAP_POLICER_RATE)
+		if (!policy[DEVLINK_ATTR_TRAP_POLICER_RATE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_TRAP_POLICER_BURST)
+		if (!policy[DEVLINK_ATTR_TRAP_POLICER_BURST].valid)
+			return -EOPNOTSUPP;
+	/* TODO: figure how to properly handle nested attributes? */
+	if (opts->present & (DL_OPT_PORT_FUNCTION_HW_ADDR | DL_OPT_PORT_FUNCTION_STATE))
+		if (!policy[DEVLINK_ATTR_PORT_FUNCTION].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_FLAVOUR)
+		if (!policy[DEVLINK_ATTR_PORT_FLAVOUR].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_PFNUMBER)
+		if (!policy[DEVLINK_ATTR_PORT_PCI_PF_NUMBER].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_SFNUMBER)
+		if (!policy[DEVLINK_ATTR_PORT_PCI_SF_NUMBER].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_CONTROLLER)
+		if (!policy[DEVLINK_ATTR_PORT_CONTROLLER_NUMBER].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_FN_RATE_TYPE)
+		if (!policy[DEVLINK_ATTR_RATE_TYPE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_FN_RATE_TX_SHARE)
+		if (!policy[DEVLINK_ATTR_RATE_TX_SHARE].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_FN_RATE_TX_MAX)
+		if (!policy[DEVLINK_ATTR_RATE_TX_MAX].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_PORT_FN_RATE_PARENT)
+		if (!policy[DEVLINK_ATTR_RATE_PARENT_NODE_NAME].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_LINECARD)
+		if (!policy[DEVLINK_ATTR_LINECARD_INDEX].valid)
+			return -EOPNOTSUPP;
+	if (opts->present & DL_OPT_LINECARD_TYPE)
+		if (!policy[DEVLINK_ATTR_LINECARD_TYPE].valid)
+			return -EOPNOTSUPP;
+
+	return 0;
+}
+
+static int dl_argv_parse_checked(struct dl *dl, uint32_t cmd,
+				 uint64_t o_required,
+				 uint64_t o_optional)
+{
+	int err;
+
+	err = dl_argv_parse(dl, o_required, o_optional);
+	if (err)
+		return err;
+
+	return dl_opts_check(dl, cmd);
+}
+
 static void
 dl_function_attr_put(struct nlmsghdr *nlh, const struct dl_opts *opts)
 {
@@ -2806,7 +3006,8 @@ static int cmd_dev_eswitch_show(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_ESWITCH_GET,
+				    DL_OPT_HANDLE, 0);
 	if (err)
 		return err;
 
@@ -2826,10 +3027,9 @@ static int cmd_dev_eswitch_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE,
-			    DL_OPT_ESWITCH_MODE |
-			    DL_OPT_ESWITCH_INLINE_MODE |
-			    DL_OPT_ESWITCH_ENCAP_MODE);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_ESWITCH_SET,
+				    DL_OPT_HANDLE,
+				    DL_OPT_ESWITCH_MODE | DL_OPT_ESWITCH_INLINE_MODE | DL_OPT_ESWITCH_ENCAP_MODE);
 	if (err)
 		return err;
 
@@ -3201,10 +3401,9 @@ static int cmd_dev_param_set(struct dl *dl)
 	bool val_bool;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE |
-			    DL_OPT_PARAM_NAME |
-			    DL_OPT_PARAM_VALUE |
-			    DL_OPT_PARAM_CMODE, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_PARAM_GET,
+				    DL_OPT_HANDLE | DL_OPT_PARAM_NAME | DL_OPT_PARAM_VALUE | DL_OPT_PARAM_CMODE,
+				    0);
 	if (err)
 		return err;
 
@@ -3332,7 +3531,9 @@ static int cmd_dev_param_show(struct dl *dl)
 	if (dl_no_arg(dl)) {
 		flags |= NLM_F_DUMP;
 	} else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_PARAM_NAME, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_PARAM_GET,
+					    DL_OPT_HANDLE | DL_OPT_PARAM_NAME,
+					    0);
 		if (err)
 			return err;
 	}
@@ -3489,7 +3690,8 @@ static int cmd_dev_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_GET,
+					    DL_OPT_HANDLE, 0);
 		if (err)
 			return err;
 	}
@@ -3565,9 +3767,8 @@ static int cmd_dev_reload(struct dl *dl)
 		return 0;
 	}
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE,
-			    DL_OPT_NETNS | DL_OPT_RELOAD_ACTION |
-			    DL_OPT_RELOAD_LIMIT);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_RELOAD, DL_OPT_HANDLE,
+				    DL_OPT_NETNS | DL_OPT_RELOAD_ACTION | DL_OPT_RELOAD_LIMIT);
 	if (err)
 		return err;
 
@@ -3713,7 +3914,8 @@ static int cmd_dev_info(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_INFO_GET,
+					    DL_OPT_HANDLE, 0);
 		if (err)
 			return err;
 	}
@@ -3976,8 +4178,9 @@ static int cmd_dev_flash(struct dl *dl)
 		return 0;
 	}
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_FLASH_FILE_NAME,
-			    DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_FLASH_UPDATE,
+				    DL_OPT_HANDLE | DL_OPT_FLASH_FILE_NAME,
+				    DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE);
 	if (err)
 		return err;
 
@@ -4520,7 +4723,8 @@ static int cmd_port_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLEP, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_GET,
+					    DL_OPT_HANDLEP, 0);
 		if (err)
 			return err;
 	}
@@ -4540,7 +4744,8 @@ static int cmd_port_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_PORT_TYPE, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_SET,
+				    DL_OPT_HANDLEP | DL_OPT_PORT_TYPE, 0);
 	if (err)
 		return err;
 
@@ -4557,7 +4762,8 @@ static int cmd_port_split(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_PORT_COUNT, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_SPLIT,
+				    DL_OPT_HANDLEP | DL_OPT_PORT_COUNT, 0);
 	if (err)
 		return err;
 
@@ -4574,7 +4780,8 @@ static int cmd_port_unsplit(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLEP, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_UNSPLIT,
+				    DL_OPT_HANDLEP, 0);
 	if (err)
 		return err;
 
@@ -4596,7 +4803,9 @@ static int cmd_port_param_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_PARAM_NAME, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_PARAM_GET,
+					    DL_OPT_HANDLEP | DL_OPT_PARAM_NAME,
+					    0);
 		if (err)
 			return err;
 	}
@@ -4628,8 +4837,8 @@ static int cmd_port_function_set(struct dl *dl)
 		cmd_port_function_help();
 		return 0;
 	}
-	err = dl_argv_parse(dl, DL_OPT_HANDLEP,
-			    DL_OPT_PORT_FUNCTION_HW_ADDR | DL_OPT_PORT_FUNCTION_STATE);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_SET, DL_OPT_HANDLEP,
+				    DL_OPT_PORT_FUNCTION_HW_ADDR | DL_OPT_PORT_FUNCTION_STATE);
 	if (err)
 		return err;
 
@@ -4720,10 +4929,9 @@ static int cmd_port_param_set(struct dl *dl)
 	bool val_bool;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLEP |
-			    DL_OPT_PARAM_NAME |
-			    DL_OPT_PARAM_VALUE |
-			    DL_OPT_PARAM_CMODE, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_PARAM_GET,
+				    DL_OPT_HANDLEP | DL_OPT_PARAM_NAME | DL_OPT_PARAM_VALUE | DL_OPT_PARAM_CMODE,
+				    0);
 	if (err)
 		return err;
 
@@ -4953,9 +5161,9 @@ static int cmd_port_fn_rate_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl,
-				    DL_OPT_HANDLEP | DL_OPT_PORT_FN_RATE_NODE_NAME,
-				    0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_RATE_GET,
+					    DL_OPT_HANDLEP | DL_OPT_PORT_FN_RATE_NODE_NAME,
+					    0);
 		if (err)
 			return err;
 	}
@@ -4995,8 +5203,9 @@ static int cmd_port_fn_rate_add(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_PORT_FN_RATE_NODE_NAME,
-			    DL_OPT_PORT_FN_RATE_TX_SHARE | DL_OPT_PORT_FN_RATE_TX_MAX);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_RATE_NEW,
+				    DL_OPT_PORT_FN_RATE_NODE_NAME,
+				    DL_OPT_PORT_FN_RATE_TX_SHARE | DL_OPT_PORT_FN_RATE_TX_MAX);
 	if (err)
 		return err;
 
@@ -5020,7 +5229,8 @@ static int cmd_port_fn_rate_del(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_PORT_FN_RATE_NODE_NAME, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_RATE_DEL,
+				    DL_OPT_PORT_FN_RATE_NODE_NAME, 0);
 	if (err)
 		return err;
 
@@ -5155,9 +5365,9 @@ static int cmd_port_add(struct dl *dl)
 		return 0;
 	}
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HANDLEP |
-			    DL_OPT_PORT_FLAVOUR | DL_OPT_PORT_PFNUMBER,
-			    DL_OPT_PORT_SFNUMBER | DL_OPT_PORT_CONTROLLER);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_NEW,
+				    DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_PORT_FLAVOUR | DL_OPT_PORT_PFNUMBER,
+				    DL_OPT_PORT_SFNUMBER | DL_OPT_PORT_CONTROLLER);
 	if (err)
 		return err;
 
@@ -5184,7 +5394,8 @@ static int cmd_port_del(struct dl *dl)
 		return 0;
 	}
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLEP, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_PORT_DEL, DL_OPT_HANDLEP,
+				    0);
 	if (err)
 		return err;
 
@@ -5327,7 +5538,8 @@ static int cmd_linecard_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_LINECARD);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_LINECARD_GET,
+					    DL_OPT_HANDLE, DL_OPT_LINECARD);
 		if (err)
 			return err;
 	}
@@ -5348,8 +5560,9 @@ static int cmd_linecard_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_LINECARD |
-			    DL_OPT_LINECARD_TYPE, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_LINECARD_SET,
+				    DL_OPT_HANDLE | DL_OPT_LINECARD | DL_OPT_LINECARD_TYPE,
+				    0);
 	if (err)
 		return err;
 
@@ -5445,7 +5658,8 @@ static int cmd_sb_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_SB);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_SB_GET,
+					    DL_OPT_HANDLE, DL_OPT_SB);
 		if (err)
 			return err;
 	}
@@ -5524,8 +5738,9 @@ static int cmd_sb_pool_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_SB_POOL,
-				    DL_OPT_SB);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_SB_POOL_GET,
+					    DL_OPT_HANDLE | DL_OPT_SB_POOL,
+					    DL_OPT_SB);
 		if (err)
 			return err;
 	}
@@ -5545,8 +5760,9 @@ static int cmd_sb_pool_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_SB_POOL |
-			    DL_OPT_SB_SIZE | DL_OPT_SB_THTYPE, DL_OPT_SB);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_SB_POOL_SET,
+				    DL_OPT_HANDLE | DL_OPT_SB_POOL | DL_OPT_SB_SIZE | DL_OPT_SB_THTYPE,
+				    DL_OPT_SB);
 	if (err)
 		return err;
 
@@ -5613,8 +5829,9 @@ static int cmd_sb_port_pool_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_SB_POOL,
-				    DL_OPT_SB);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_SB_PORT_POOL_GET,
+					    DL_OPT_HANDLEP | DL_OPT_SB_POOL,
+					    DL_OPT_SB);
 		if (err)
 			return err;
 	}
@@ -5634,8 +5851,9 @@ static int cmd_sb_port_pool_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_SB_POOL | DL_OPT_SB_TH,
-			    DL_OPT_SB);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_SB_PORT_POOL_SET,
+				    DL_OPT_HANDLEP | DL_OPT_SB_POOL | DL_OPT_SB_TH,
+				    DL_OPT_SB);
 	if (err)
 		return err;
 
@@ -5720,8 +5938,10 @@ static int cmd_sb_tc_bind_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_SB_TC |
-				    DL_OPT_SB_TYPE, DL_OPT_SB);
+		err = dl_argv_parse_checked(dl,
+					    DEVLINK_CMD_SB_TC_POOL_BIND_GET,
+					    DL_OPT_HANDLEP | DL_OPT_SB_TC | DL_OPT_SB_TYPE,
+					    DL_OPT_SB);
 		if (err)
 			return err;
 	}
@@ -5741,9 +5961,9 @@ static int cmd_sb_tc_bind_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLEP | DL_OPT_SB_TC |
-			    DL_OPT_SB_TYPE | DL_OPT_SB_POOL | DL_OPT_SB_TH,
-			    DL_OPT_SB);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_SB_TC_POOL_BIND_SET,
+				    DL_OPT_HANDLEP | DL_OPT_SB_TC | DL_OPT_SB_TYPE | DL_OPT_SB_POOL | DL_OPT_SB_TH,
+				    DL_OPT_SB);
 	if (err)
 		return err;
 
@@ -6097,7 +6317,8 @@ static int cmd_sb_occ_snapshot(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_SB);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_SB_OCC_SNAPSHOT,
+				    DL_OPT_HANDLE, DL_OPT_SB);
 	if (err)
 		return err;
 
@@ -6114,7 +6335,8 @@ static int cmd_sb_occ_clearmax(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_SB);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_SB_OCC_MAX_CLEAR,
+				    DL_OPT_HANDLE, DL_OPT_SB);
 	if (err)
 		return err;
 
@@ -7023,7 +7245,8 @@ static int cmd_dpipe_headers_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_DPIPE_HEADERS_GET,
+				    DL_OPT_HANDLE, 0);
 	if (err)
 		return err;
 
@@ -7423,7 +7646,8 @@ static int cmd_dpipe_table_show(struct dl *dl)
 	uint16_t flags = NLM_F_REQUEST;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE, DL_OPT_DPIPE_TABLE_NAME);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_DPIPE_HEADERS_GET,
+				    DL_OPT_HANDLE, DL_OPT_DPIPE_TABLE_NAME);
 	if (err)
 		return err;
 
@@ -7478,8 +7702,9 @@ static int cmd_dpipe_table_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_DPIPE_TABLE_NAME |
-			    DL_OPT_DPIPE_TABLE_COUNTERS, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+				    DL_OPT_HANDLE | DL_OPT_DPIPE_TABLE_NAME | DL_OPT_DPIPE_TABLE_COUNTERS,
+				    0);
 	if (err)
 		return err;
 
@@ -7852,7 +8077,9 @@ static int cmd_dpipe_table_dump(struct dl *dl)
 	if (err)
 		return err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_DPIPE_TABLE_NAME, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_DPIPE_HEADERS_GET,
+				    DL_OPT_HANDLE | DL_OPT_DPIPE_TABLE_NAME,
+				    0);
 	if (err)
 		goto out;
 
@@ -8129,7 +8356,8 @@ static int cmd_resource_show(struct dl *dl)
 	struct resource_ctx resource_ctx = {};
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_DPIPE_TABLE_GET,
+				    DL_OPT_HANDLE, 0);
 	if (err)
 		return err;
 
@@ -8229,8 +8457,9 @@ static int cmd_resource_set(struct dl *dl)
 		return err;
 
 	ctx.print_resources = false;
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_RESOURCE_PATH |
-			    DL_OPT_RESOURCE_SIZE, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_RESOURCE_DUMP,
+				    DL_OPT_HANDLE | DL_OPT_RESOURCE_PATH |
+				    DL_OPT_RESOURCE_SIZE, 0);
 	if (err)
 		goto out;
 
@@ -8401,7 +8630,8 @@ static int cmd_region_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLE_REGION, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_REGION_GET,
+					    DL_OPT_HANDLE_REGION, 0);
 		if (err)
 			return err;
 	}
@@ -8421,8 +8651,9 @@ static int cmd_region_snapshot_del(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE_REGION |
-			    DL_OPT_REGION_SNAPSHOT_ID, 0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_REGION_DEL,
+				    DL_OPT_HANDLE_REGION | DL_OPT_REGION_SNAPSHOT_ID,
+				    0);
 	if (err)
 		return err;
 
@@ -8473,9 +8704,9 @@ static int cmd_region_dump(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl,
-			    DL_OPT_HANDLE_REGION | DL_OPT_REGION_SNAPSHOT_ID,
-			    0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_REGION_READ,
+				    DL_OPT_HANDLE_REGION | DL_OPT_REGION_SNAPSHOT_ID,
+				    0);
 	if (err)
 		return err;
 
@@ -8497,9 +8728,9 @@ static int cmd_region_read(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE_REGION | DL_OPT_REGION_ADDRESS |
-			    DL_OPT_REGION_LENGTH | DL_OPT_REGION_SNAPSHOT_ID,
-			    0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_REGION_READ,
+				    DL_OPT_HANDLE_REGION | DL_OPT_REGION_ADDRESS | DL_OPT_REGION_LENGTH | DL_OPT_REGION_SNAPSHOT_ID,
+				    0);
 	if (err)
 		return err;
 
@@ -8538,8 +8769,9 @@ static int cmd_region_snapshot_new(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE_REGION,
-			    DL_OPT_REGION_SNAPSHOT_ID);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_REGION_NEW,
+				    DL_OPT_HANDLE_REGION,
+				    DL_OPT_REGION_SNAPSHOT_ID);
 	if (err)
 		return err;
 
@@ -8595,15 +8827,16 @@ static int cmd_health_set_params(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_SET,
-			       NLM_F_REQUEST | NLM_F_ACK);
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_HEALTH_REPORTER_NAME,
-			    DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD |
-			    DL_OPT_HEALTH_REPORTER_AUTO_RECOVER |
-			    DL_OPT_HEALTH_REPORTER_AUTO_DUMP);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_HEALTH_REPORTER_SET,
+				    DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_HEALTH_REPORTER_NAME,
+				    DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD |
+				    DL_OPT_HEALTH_REPORTER_AUTO_RECOVER |
+				    DL_OPT_HEALTH_REPORTER_AUTO_DUMP);
 	if (err)
 		return err;
 
+	nlh = mnlu_gen_socket_cmd_prepare(&dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_SET,
+			       NLM_F_REQUEST | NLM_F_ACK);
 	dl_opts_put(nlh, dl);
 	return mnlu_gen_socket_sndrcv(&dl->nlg, nlh, NULL, NULL);
 }
@@ -8613,9 +8846,10 @@ static int cmd_health_dump_clear(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HANDLEP |
-			    DL_OPT_HEALTH_REPORTER_NAME,
-			    0);
+	err = dl_argv_parse_checked(dl,
+				    DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
+				    DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_HEALTH_REPORTER_NAME,
+				    0);
 	if (err)
 		return err;
 
@@ -8866,9 +9100,9 @@ static int cmd_health_object_common(struct dl *dl, uint8_t cmd, uint16_t flags)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl,
-			    DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_HEALTH_REPORTER_NAME,
-			    0);
+	err = dl_argv_parse_checked(dl, cmd,
+				    DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_HEALTH_REPORTER_NAME,
+				    0);
 	if (err)
 		return err;
 
@@ -8908,9 +9142,9 @@ static int cmd_health_recover(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HANDLEP |
-			    DL_OPT_HEALTH_REPORTER_NAME,
-			    0);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_HEALTH_REPORTER_RECOVER,
+				    DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_HEALTH_REPORTER_NAME,
+				    0);
 	if (err)
 		return err;
 
@@ -9088,9 +9322,10 @@ static int __cmd_health_show(struct dl *dl, bool show_device, bool show_port)
 		flags |= NLM_F_DUMP;
 	} else {
 		ctx.show_port = true;
-		err = dl_argv_parse(dl,
-				    DL_OPT_HANDLE | DL_OPT_HANDLEP |
-				    DL_OPT_HEALTH_REPORTER_NAME, 0);
+		err = dl_argv_parse_checked(dl,
+					    DEVLINK_CMD_HEALTH_REPORTER_GET,
+					    DL_OPT_HANDLE | DL_OPT_HANDLEP | DL_OPT_HEALTH_REPORTER_NAME,
+					    0);
 		if (err)
 			return err;
 	}
@@ -9282,7 +9517,9 @@ static int cmd_trap_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_TRAP_NAME, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_TRAP_GET,
+					    DL_OPT_HANDLE | DL_OPT_TRAP_NAME,
+					    0);
 		if (err)
 			return err;
 	}
@@ -9303,8 +9540,9 @@ static int cmd_trap_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_TRAP_NAME,
-			    DL_OPT_TRAP_ACTION);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_TRAP_SET,
+				    DL_OPT_HANDLE | DL_OPT_TRAP_NAME,
+				    DL_OPT_TRAP_ACTION);
 	if (err)
 		return err;
 
@@ -9360,8 +9598,9 @@ static int cmd_trap_group_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl,
-				    DL_OPT_HANDLE | DL_OPT_TRAP_GROUP_NAME, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_TRAP_GROUP_GET,
+					    DL_OPT_HANDLE | DL_OPT_TRAP_GROUP_NAME,
+					    0);
 		if (err)
 			return err;
 	}
@@ -9382,8 +9621,9 @@ static int cmd_trap_group_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_TRAP_GROUP_NAME,
-			    DL_OPT_TRAP_ACTION | DL_OPT_TRAP_POLICER_ID);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_TRAP_GROUP_SET,
+				    DL_OPT_HANDLE | DL_OPT_TRAP_GROUP_NAME,
+				    DL_OPT_TRAP_ACTION | DL_OPT_TRAP_POLICER_ID);
 	if (err)
 		return err;
 
@@ -9459,8 +9699,9 @@ static int cmd_trap_policer_show(struct dl *dl)
 		flags |= NLM_F_DUMP;
 	}
 	else {
-		err = dl_argv_parse(dl,
-				    DL_OPT_HANDLE | DL_OPT_TRAP_POLICER_ID, 0);
+		err = dl_argv_parse_checked(dl, DEVLINK_CMD_TRAP_POLICER_GET,
+					    DL_OPT_HANDLE | DL_OPT_TRAP_POLICER_ID,
+					    0);
 		if (err)
 			return err;
 	}
@@ -9481,8 +9722,9 @@ static int cmd_trap_policer_set(struct dl *dl)
 	struct nlmsghdr *nlh;
 	int err;
 
-	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_TRAP_POLICER_ID,
-			    DL_OPT_TRAP_POLICER_RATE | DL_OPT_TRAP_POLICER_BURST);
+	err = dl_argv_parse_checked(dl, DEVLINK_CMD_TRAP_POLICER_SET,
+				    DL_OPT_HANDLE | DL_OPT_TRAP_POLICER_ID,
+				    DL_OPT_TRAP_POLICER_RATE | DL_OPT_TRAP_POLICER_BURST);
 	if (err)
 		return err;
 
-- 
2.37.1.208.ge72d93e88cb2


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

* Re: [RFC iproute2 0/6] devlink: add policy check for all attributes
  2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
                   ` (5 preceding siblings ...)
  2022-08-05 23:41 ` [RFC iproute2 6/6] devlink: check attributes against policy Jacob Keller
@ 2022-08-08 10:32 ` Jiri Pirko
  2022-08-08 17:02   ` Keller, Jacob E
  2022-08-09  9:50 ` Jiri Pirko
  7 siblings, 1 reply; 11+ messages in thread
From: Jiri Pirko @ 2022-08-08 10:32 UTC (permalink / raw)
  To: Jacob Keller
  Cc: netdev, Jonathan Corbet, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, David Ahern, Stephen Hemminger

Sat, Aug 06, 2022 at 01:41:49AM CEST, jacob.e.keller@intel.com wrote:


[...]

>This is intended to eventually go along with improvements to the policy
>reporting in devlink kernel code to report separate policy for each command.

Can you explain this a bit please?

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

* RE: [RFC iproute2 0/6] devlink: add policy check for all attributes
  2022-08-08 10:32 ` [RFC iproute2 0/6] devlink: add policy check for all attributes Jiri Pirko
@ 2022-08-08 17:02   ` Keller, Jacob E
  2022-08-09  7:07     ` Jiri Pirko
  0 siblings, 1 reply; 11+ messages in thread
From: Keller, Jacob E @ 2022-08-08 17:02 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev@vger.kernel.org, Jonathan Corbet, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger



> -----Original Message-----
> From: Jiri Pirko <jiri@nvidia.com>
> Sent: Monday, August 08, 2022 3:32 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; Jonathan Corbet <corbet@lwn.net>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David Ahern
> <dsahern@kernel.org>; Stephen Hemminger <stephen@networkplumber.org>
> Subject: Re: [RFC iproute2 0/6] devlink: add policy check for all attributes
> 
> Sat, Aug 06, 2022 at 01:41:49AM CEST, jacob.e.keller@intel.com wrote:
> 
> 
> [...]
> 
> >This is intended to eventually go along with improvements to the policy
> >reporting in devlink kernel code to report separate policy for each command.
> 
> Can you explain this a bit please?

Currently devlink only reports a global policy which is the same for every command. This means that commands like DEVLINK_CMD_FLASH report the same attributes as valid as other commands like DEVLINK_CMD_INFO_GET. The policy (if the command is strict) would only require that attributes be one of the known attributes according to the general devlink policy.

However, none of the commands accept or honor all the attributes. The netlink policy support allows each command to report an individual policy that would only include the attributes which the command uses and would honor the meaning of.

Without per-command policy, there is no way for user space to tell when the kernel changed support for some attribute (such as the DEVLINK_ATTR_DRY_RUN I recently proposed). Yes, you can use maxattr to determine of the kernel knows about the attribute, but that doesn't guarantee that every command supports it.

The per-command policy would report only the subset of attributes supported by the command. In strict validation, this would also make the kernel reject commands which tried to send other attributes. Ideally we would also have nested attribute policy, so each nested attribute would express the policy for the subset of attributes which are valid within that nest as well.

By adding policy checking support to user space, we can make sure that at least iproute2 userspace won't accidentally send an unsupported attribute to a command, but that only works once the policy for the command in the kernel is updated to list the specific policy. Right now, this will effectively get the generic policy and indicate that all known attributes in the policy table are accepted.

Note that this means strict validation on its own is not enough.  It doesn't really matter if a command is set to strict validation when the validation still allows every attribute in the general policy, regardless of whether the command currently does anything with the attribute. Any of the unsupported ones get silently ignored, with no warning to the user.

Related to this, also think we should determine a plan for how to migrate devlink to strict validation, once the per-command policy is defined and implemented. However, I am not sure what the backwards-compatibility issues exist for switching.

Hope this explains things,
Jake

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

* Re: [RFC iproute2 0/6] devlink: add policy check for all attributes
  2022-08-08 17:02   ` Keller, Jacob E
@ 2022-08-09  7:07     ` Jiri Pirko
  0 siblings, 0 replies; 11+ messages in thread
From: Jiri Pirko @ 2022-08-09  7:07 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: netdev@vger.kernel.org, Jonathan Corbet, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern,
	Stephen Hemminger

Mon, Aug 08, 2022 at 07:02:49PM CEST, jacob.e.keller@intel.com wrote:
>
>
>> -----Original Message-----
>> From: Jiri Pirko <jiri@nvidia.com>
>> Sent: Monday, August 08, 2022 3:32 AM
>> To: Keller, Jacob E <jacob.e.keller@intel.com>
>> Cc: netdev@vger.kernel.org; Jonathan Corbet <corbet@lwn.net>; David S. Miller
>> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
>> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David Ahern
>> <dsahern@kernel.org>; Stephen Hemminger <stephen@networkplumber.org>
>> Subject: Re: [RFC iproute2 0/6] devlink: add policy check for all attributes
>> 
>> Sat, Aug 06, 2022 at 01:41:49AM CEST, jacob.e.keller@intel.com wrote:
>> 
>> 
>> [...]
>> 
>> >This is intended to eventually go along with improvements to the policy
>> >reporting in devlink kernel code to report separate policy for each command.
>> 
>> Can you explain this a bit please?
>
>Currently devlink only reports a global policy which is the same for every command. This means that commands like DEVLINK_CMD_FLASH report the same attributes as valid as other commands like DEVLINK_CMD_INFO_GET. The policy (if the command is strict) would only require that attributes be one of the known attributes according to the general devlink policy.
>
>However, none of the commands accept or honor all the attributes. The netlink policy support allows each command to report an individual policy that would only include the attributes which the command uses and would honor the meaning of.
>
>Without per-command policy, there is no way for user space to tell when the kernel changed support for some attribute (such as the DEVLINK_ATTR_DRY_RUN I recently proposed). Yes, you can use maxattr to determine of the kernel knows about the attribute, but that doesn't guarantee that every command supports it.
>
>The per-command policy would report only the subset of attributes supported by the command. In strict validation, this would also make the kernel reject commands which tried to send other attributes. Ideally we would also have nested attribute policy, so each nested attribute would express the policy for the subset of attributes which are valid within that nest as well.
>
>By adding policy checking support to user space, we can make sure that at least iproute2 userspace won't accidentally send an unsupported attribute to a command, but that only works once the policy for the command in the kernel is updated to list the specific policy. Right now, this will effectively get the generic policy and indicate that all known attributes in the policy table are accepted.
>
>Note that this means strict validation on its own is not enough.  It doesn't really matter if a command is set to strict validation when the validation still allows every attribute in the general policy, regardless of whether the command currently does anything with the attribute. Any of the unsupported ones get silently ignored, with no warning to the user.
>
>Related to this, also think we should determine a plan for how to migrate devlink to strict validation, once the per-command policy is defined and implemented. However, I am not sure what the backwards-compatibility issues exist for switching.
>
>Hope this explains things,

It is, thanks.

Do you have patches for the per-cmd policy?


>Jake

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

* Re: [RFC iproute2 0/6] devlink: add policy check for all attributes
  2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
                   ` (6 preceding siblings ...)
  2022-08-08 10:32 ` [RFC iproute2 0/6] devlink: add policy check for all attributes Jiri Pirko
@ 2022-08-09  9:50 ` Jiri Pirko
  7 siblings, 0 replies; 11+ messages in thread
From: Jiri Pirko @ 2022-08-09  9:50 UTC (permalink / raw)
  To: Jacob Keller
  Cc: netdev, Jonathan Corbet, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, David Ahern, Stephen Hemminger

Sat, Aug 06, 2022 at 01:41:49AM CEST, jacob.e.keller@intel.com wrote:
>This series implements code to check the kernel policy for the devlink
>commands to determine whether or not attributes are supported before adding
>them to netlink messages.
>
>It implements a new mnlu_gen_get_op_policy to extract the policy
>information, and uses it to implement checks when parsing option arguments.
>This is intended to eventually go along with improvements to the policy
>reporting in devlink kernel code to report separate policy for each command.
>
>I think checking every attribute makes sense and is easier to follow than
>only checking specific attributes. This will help ensure that future
>attributes don't accidentally get sent to commands when they aren't
>supported (once the devlink kernel policy is improved to report correct
>information for each command separately).

This patchset looks good to me. Thanks!

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

end of thread, other threads:[~2022-08-09  9:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-05 23:41 [RFC iproute2 0/6] devlink: add policy check for all attributes Jacob Keller
2022-08-05 23:41 ` [RFC iproute2 1/6] mnlg: remove unnused mnlg_socket structure Jacob Keller
2022-08-05 23:41 ` [RFC iproute2 2/6] utils: extract CTRL_ATTR_MAXATTR and save it Jacob Keller
2022-08-05 23:41 ` [RFC iproute2 3/6] mnl_utils: add function to dump command policy Jacob Keller
2022-08-05 23:41 ` [RFC iproute2 4/6] devlink: use dl_no_arg instead of checking dl_argc == 0 Jacob Keller
2022-08-05 23:41 ` [RFC iproute2 5/6] devlink: remove dl_argv_parse_put Jacob Keller
2022-08-05 23:41 ` [RFC iproute2 6/6] devlink: check attributes against policy Jacob Keller
2022-08-08 10:32 ` [RFC iproute2 0/6] devlink: add policy check for all attributes Jiri Pirko
2022-08-08 17:02   ` Keller, Jacob E
2022-08-09  7:07     ` Jiri Pirko
2022-08-09  9:50 ` Jiri Pirko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox