File netlink-use-genetlink-ops-information-to-decide... of Package ethtool

Overview Repositories Revisions Requests Users Attributes Meta

File netlink-use-genetlink-ops-information-to-decide-abou.patch of Package ethtool (Revision 2318d9b5e16af1c6ea09dccb0ef62fa8)

Currently displaying revision 2318d9b5e16af1c6ea09dccb0ef62fa8 , Show latest

From: Michal Kubecek <mkubecek@suse.cz>
Date: Mon, 13 Apr 2020 17:39:32 +0200
Subject: netlink: use genetlink ops information to decide about fallback
Patch-mainline: v5.7
Git-commit: 6c19c0d559c80824383c5f0e2dc229878d9b19f5

Currently ethtool falls back to ioctl when netlink socket initialization
fails or if netlink request fails with -EOPNOTSUPP (and we do not know that
request cannot possibly work via ioctl, e.g. because of wildcard device
name or device name longer than IFNAMSIZ). This logic has one problem: we
cannot distinguish if -EOPNOTSUPP error code means that subcommand as such
is not implemented in kernel or that it failed for some other reason (e.g.
specific combination of parameters is not supported by driver). If the
latter is the case, fallback to ioctl is pointless as the same request
would fail via ioctl as well. In some cases we can even get a duplicate
error message.

Fortunately, genetlink provides information about supported family commands
in the form of CTRL_ATTR_OPS nested attribute. Parse this information when
available and use it to only fall back to ioctl only if kernel support for
netlink request as such is missing so that there is a chance that ioctl
request will work. In such case, do not send netlink request at all.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 netlink/netlink.c  | 138 ++++++++++++++++++++++++++++++++++++---------
 netlink/netlink.h  |   5 ++
 netlink/parser.c   |   7 +++
 netlink/settings.c |   7 +++
 4 files changed, 129 insertions(+), 28 deletions(-)

--- a/netlink/netlink.c
+++ b/netlink/netlink.c
@@ -92,16 +92,88 @@ int get_dev_info(const struct nlattr *nest, int *ifindex, char *ifname)
 	return 0;
 }
 
+/**
+ * netlink_cmd_check() - check support for netlink command
+ * @ctx:            ethtool command context
+ * @cmd:            netlink command id
+ * @devname:        device name from user
+ * @allow_wildcard: wildcard dumps supported
+ *
+ * Check if command @cmd is known to be unsupported based on ops information
+ * from genetlink family id request. Set nlctx->ioctl_fallback if ethtool
+ * should fall back to ioctl, i.e. when we do not know in advance that
+ * netlink request is supported. Set nlctx->wildcard_unsupported if "*" was
+ * used as device name but the request does not support wildcards (on either
+ * side).
+ *
+ * Return: true if we know the netlink request is not supported and should
+ * fail (and possibly fall back) without actually sending it to kernel.
+ */
+bool netlink_cmd_check(struct cmd_context *ctx, unsigned int cmd,
+		       bool allow_wildcard)
+{
+	bool is_dump = !strcmp(ctx->devname, WILDCARD_DEVNAME);
+	uint32_t cap = is_dump ? GENL_CMD_CAP_DUMP : GENL_CMD_CAP_DO;
+	struct nl_context *nlctx = ctx->nlctx;
+
+	if (is_dump && !allow_wildcard) {
+		nlctx->wildcard_unsupported = true;
+		return true;
+	}
+	if (!nlctx->ops_flags) {
+		nlctx->ioctl_fallback = true;
+		return false;
+	}
+	if (cmd > ETHTOOL_MSG_USER_MAX || !nlctx->ops_flags[cmd]) {
+		nlctx->ioctl_fallback = true;
+		return true;
+	}
+
+	if (is_dump && !(nlctx->ops_flags[cmd] & GENL_CMD_CAP_DUMP))
+		nlctx->wildcard_unsupported = true;
+
+	return !(nlctx->ops_flags[cmd] & cap);
+}
+
 /* initialization */
 
-struct fam_info {
-	const char	*fam_name;
-	const char	*grp_name;
-	uint16_t	fam_id;
-	uint32_t	grp_id;
-};
+static int genl_read_ops(struct nl_context *nlctx,
+			 const struct nlattr *ops_attr)
+{
+	struct nlattr *op_attr;
+	uint32_t *ops_flags;
+	int ret;
+
+	ops_flags = calloc(__ETHTOOL_MSG_USER_CNT, sizeof(ops_flags[0]));
+	if (!ops_flags)
+		return -ENOMEM;
+
+	mnl_attr_for_each_nested(op_attr, ops_attr) {
+		const struct nlattr *tb[CTRL_ATTR_OP_MAX + 1] = {};
+		DECLARE_ATTR_TB_INFO(tb);
+		uint32_t op_id;
+
+		ret = mnl_attr_parse_nested(op_attr, attr_cb, &tb_info);
+		if (ret < 0)
+			goto err;
+
+		if (!tb[CTRL_ATTR_OP_ID] || !tb[CTRL_ATTR_OP_FLAGS])
+			continue;
+		op_id = mnl_attr_get_u32(tb[CTRL_ATTR_OP_ID]);
+		if (op_id >= __ETHTOOL_MSG_USER_CNT)
+			continue;
+
+		ops_flags[op_id] = mnl_attr_get_u32(tb[CTRL_ATTR_OP_FLAGS]);
+	}
+
+	nlctx->ops_flags = ops_flags;
+	return 0;
+err:
+	free(ops_flags);
+	return ret;
+}
 
-static void find_mc_group(struct nlattr *nest, struct fam_info *info)
+static void find_mc_group(struct nl_context *nlctx, struct nlattr *nest)
 {
 	const struct nlattr *grp_tb[CTRL_ATTR_MCAST_GRP_MAX + 1] = {};
 	DECLARE_ATTR_TB_INFO(grp_tb);
@@ -116,9 +188,9 @@ static void find_mc_group(struct nlattr *nest, struct fam_info *info)
 		    !grp_tb[CTRL_ATTR_MCAST_GRP_ID])
 			continue;
 		if (strcmp(mnl_attr_get_str(grp_tb[CTRL_ATTR_MCAST_GRP_NAME]),
-			   info->grp_name))
+			   ETHTOOL_MCGRP_MONITOR_NAME))
 			continue;
-		info->grp_id =
+		nlctx->ethnl_mongrp =
 			mnl_attr_get_u32(grp_tb[CTRL_ATTR_MCAST_GRP_ID]);
 		return;
 	}
@@ -126,16 +198,21 @@ static void find_mc_group(struct nlattr *nest, struct fam_info *info)
 
 static int family_info_cb(const struct nlmsghdr *nlhdr, void *data)
 {
-	struct fam_info *info = data;
+	struct nl_context *nlctx = data;
 	struct nlattr *attr;
+	int ret;
 
 	mnl_attr_for_each(attr, nlhdr, GENL_HDRLEN) {
 		switch (mnl_attr_get_type(attr)) {
 		case CTRL_ATTR_FAMILY_ID:
-			info->fam_id = mnl_attr_get_u16(attr);
+			nlctx->ethnl_fam = mnl_attr_get_u16(attr);
 			break;
+		case CTRL_ATTR_OPS:
+			ret = genl_read_ops(nlctx, attr);
+			if (ret < 0)
+				return MNL_CB_ERROR;
 		case CTRL_ATTR_MCAST_GROUPS:
-			find_mc_group(attr, info);
+			find_mc_group(nlctx, attr);
 			break;
 		}
 	}
@@ -144,41 +221,37 @@ static int family_info_cb(const struct nlmsghdr *nlhdr, void *data)
 }
 
 #ifdef TEST_ETHTOOL
-static int get_genl_family(struct nl_socket *nlsk, struct fam_info *info)
+static int get_genl_family(struct nl_context *nlctx, struct nl_socket *nlsk)
 {
 	return 0;
 }
 #else
-static int get_genl_family(struct nl_socket *nlsk, struct fam_info *info)
+static int get_genl_family(struct nl_context *nlctx, struct nl_socket *nlsk)
 {
 	struct nl_msg_buff *msgbuff = &nlsk->msgbuff;
 	int ret;
 
-	nlsk->nlctx->suppress_nlerr = 2;
+	nlctx->suppress_nlerr = 2;
 	ret = __msg_init(msgbuff, GENL_ID_CTRL, CTRL_CMD_GETFAMILY,
 			 NLM_F_REQUEST | NLM_F_ACK, 1);
 	if (ret < 0)
 		goto out;
 	ret = -EMSGSIZE;
-	if (ethnla_put_strz(msgbuff, CTRL_ATTR_FAMILY_NAME, info->fam_name))
+	if (ethnla_put_strz(msgbuff, CTRL_ATTR_FAMILY_NAME, ETHTOOL_GENL_NAME))
 		goto out;
 
 	nlsock_sendmsg(nlsk, NULL);
-	nlsock_process_reply(nlsk, family_info_cb, info);
-	ret = info->fam_id ? 0 : -EADDRNOTAVAIL;
+	nlsock_process_reply(nlsk, family_info_cb, nlctx);
+	ret = nlctx->ethnl_fam ? 0 : -EADDRNOTAVAIL;
 
 out:
-	nlsk->nlctx->suppress_nlerr = 0;
+	nlctx->suppress_nlerr = 0;
 	return ret;
 }
 #endif
 
 int netlink_init(struct cmd_context *ctx)
 {
-	struct fam_info info = {
-		.fam_name	= ETHTOOL_GENL_NAME,
-		.grp_name	= ETHTOOL_MCGRP_MONITOR_NAME,
-	};
 	struct nl_context *nlctx;
 	int ret;
 
@@ -189,11 +262,9 @@ int netlink_init(struct cmd_context *ctx)
 	ret = nlsock_init(nlctx, &nlctx->ethnl_socket, NETLINK_GENERIC);
 	if (ret < 0)
 		goto out_free;
-	ret = get_genl_family(nlctx->ethnl_socket, &info);
+	ret = get_genl_family(nlctx, nlctx->ethnl_socket);
 	if (ret < 0)
 		goto out_nlsk;
-	nlctx->ethnl_fam = info.fam_id;
-	nlctx->ethnl_mongrp = info.grp_id;
 
 	ctx->nlctx = nlctx;
 	return 0;
@@ -201,6 +272,7 @@ int netlink_init(struct cmd_context *ctx)
 out_nlsk:
 	nlsock_done(nlctx->ethnl_socket);
 out_free:
+	free(nlctx->ops_flags);
 	free(nlctx);
 	return ret;
 }
@@ -210,6 +282,7 @@ static void netlink_done(struct cmd_context *ctx)
 	if (!ctx->nlctx)
 		return;
 
+	free(ctx->nlctx->ops_flags);
 	free(ctx->nlctx);
 	ctx->nlctx = NULL;
 	cleanup_all_strings();
@@ -228,6 +301,7 @@ void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc,
 			 bool no_fallback)
 {
 	bool wildcard = ctx->devname && !strcmp(ctx->devname, WILDCARD_DEVNAME);
+	struct nl_context *nlctx;
 	const char *reason;
 	int ret;
 
@@ -245,12 +319,20 @@ void netlink_run_handler(struct cmd_context *ctx, nl_func_t nlfunc,
 		reason = "netlink interface initialization failed";
 		goto no_support;
 	}
+	nlctx = ctx->nlctx;
 
 	ret = nlfunc(ctx);
 	netlink_done(ctx);
-	if (ret != -EOPNOTSUPP || no_fallback)
+	if (no_fallback || ret != -EOPNOTSUPP || !nlctx->ioctl_fallback) {
+		if (nlctx->wildcard_unsupported)
+			fprintf(stderr, "%s\n",
+				"subcommand does not support wildcard dump");
 		exit(ret >= 0 ? ret : 1);
-	reason = "kernel netlink support for subcommand missing";
+	}
+	if (nlctx->wildcard_unsupported)
+		reason = "subcommand does not support wildcard dump";
+	else
+		reason = "kernel netlink support for subcommand missing";
 
 no_support:
 	if (no_fallback) {
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -25,6 +25,7 @@ struct nl_context {
 	unsigned int		suppress_nlerr;
 	uint16_t		ethnl_fam;
 	uint32_t		ethnl_mongrp;
+	uint32_t		*ops_flags;
 	struct nl_socket	*ethnl_socket;
 	struct nl_socket	*ethnl2_socket;
 	struct nl_socket	*rtnl_socket;
@@ -36,6 +37,8 @@ struct nl_context {
 	const char		*param;
 	char			**argp;
 	int			argc;
+	bool			ioctl_fallback;
+	bool			wildcard_unsupported;
 };
 
 struct attr_tb_info {
@@ -50,6 +53,8 @@ int nomsg_reply_cb(const struct nlmsghdr *nlhdr, void *data);
 int attr_cb(const struct nlattr *attr, void *data);
 
 int netlink_init(struct cmd_context *ctx);
+bool netlink_cmd_check(struct cmd_context *ctx, unsigned int cmd,
+		       bool allow_wildcard);
 const char *get_dev_name(const struct nlattr *nest);
 int get_dev_info(const struct nlattr *nest, int *ifindex, char *ifname);
 
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -1023,6 +1023,13 @@ int nl_parser(struct nl_context *nlctx, const struct param_parser *params,
 			goto out_free;
 	}
 
+	if (group_style == PARSER_GROUP_MSG) {
+		ret = -EOPNOTSUPP;
+		for (buff = buffs; buff; buff = buff->next)
+			if (msgbuff_len(&buff->msgbuff) > buff->orig_len &&
+			    netlink_cmd_check(nlctx->ctx, buff->id, false))
+				goto out_free;
+	}
 	for (buff = buffs; buff; buff = buff->next) {
 		struct nl_msg_buff *msgbuff = &buff->msgbuff;
 
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -726,6 +726,13 @@ int nl_gset(struct cmd_context *ctx)
 	struct nl_socket *nlsk = nlctx->ethnl_socket;
 	int ret;
 
+	if (netlink_cmd_check(ctx, ETHTOOL_MSG_LINKMODES_GET, true) ||
+	    netlink_cmd_check(ctx, ETHTOOL_MSG_LINKINFO_GET, true) ||
+	    netlink_cmd_check(ctx, ETHTOOL_MSG_WOL_GET, true) ||
+	    netlink_cmd_check(ctx, ETHTOOL_MSG_DEBUG_GET, true) ||
+	    netlink_cmd_check(ctx, ETHTOOL_MSG_LINKSTATE_GET, true))
+		return -EOPNOTSUPP;
+
 	nlctx->suppress_nlerr = 1;
 
 	ret = gset_request(nlsk, ETHTOOL_MSG_LINKMODES_GET,

Places

File netlink-use-genetlink-ops-information-to-decide-abou.patch of Package ethtool (Revision 2318d9b5e16af1c6ea09dccb0ef62fa8)

Places