netdev - [PATCH ethtool 2/2] netlink: Print and return an error when features weren't changed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200814131745.32215-3-maximmi@mellanox.com>
Date:   Fri, 14 Aug 2020 16:17:45 +0300
From:   Maxim Mikityanskiy <maximmi@...lanox.com>
To:     "David S. Miller" <davem@...emloft.net>,
        Michal Kubecek <mkubecek@...e.cz>, Andrew Lunn <andrew@...n.ch>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        Maxim Mikityanskiy <maximmi@...lanox.com>
Subject: [PATCH ethtool 2/2] netlink: Print and return an error when features weren't changed

The legacy ethtool prints an error message and returns 1 if no features
were changed as requested. Port this behavior to ethtool-netlink.
req_mask is compared to wanted_mask to detect if any feature was
changed. If these masks are equal, it means that the kernel hasn't
changed anything, and all bits got to wanted.

Signed-off-by: Maxim Mikityanskiy <maximmi@...lanox.com>
---
 netlink/features.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/netlink/features.c b/netlink/features.c
index 133529d..4f63fa2 100644
--- a/netlink/features.c
+++ b/netlink/features.c
@@ -378,7 +378,7 @@ err:
 	return ret;
 }
 
-static void show_feature_changes(struct nl_context *nlctx,
+static bool show_feature_changes(struct nl_context *nlctx,
 				 const struct nlattr *const *tb)
 {
 	struct sfeatures_context *sfctx = nlctx->cmd_private;
@@ -388,8 +388,8 @@ static void show_feature_changes(struct nl_context *nlctx,
 	const uint32_t *wanted_val;
 	const uint32_t *active_val;
 	unsigned int count, words;
+	bool any_changed, diff;
 	unsigned int i;
-	bool diff;
 	int ret;
 
 	feature_names = global_stringset(ETH_SS_FEATURES, nlctx->ethnl_socket);
@@ -411,12 +411,20 @@ static void show_feature_changes(struct nl_context *nlctx,
 	if (!wanted_val || !wanted_mask || !active_val || !active_mask)
 		goto err;
 
+	any_changed = false;
 	diff = false;
-	for (i = 0; i < words; i++)
+	for (i = 0; i < words; i++) {
+		if (wanted_mask[i] != sfctx->req_mask[i])
+			any_changed = true;
 		if (wanted_mask[i] || (active_mask[i] & ~sfctx->req_mask[i]))
 			diff = true;
+	}
+	if (!any_changed) {
+		fprintf(stderr, "Could not change any device features\n");
+		nlctx->exit_code = 1;
+	}
 	if (!diff)
-		return;
+		return any_changed;
 
 	/* result is not exactly as requested, show differences */
 	printf("Actual changes:\n");
@@ -442,9 +450,10 @@ static void show_feature_changes(struct nl_context *nlctx,
 		fputc('\n', stdout);
 	}
 
-	return;
+	return any_changed;
 err:
 	fprintf(stderr, "malformed diff info from kernel\n");
+	return false;
 }
 
 int sfeatures_reply_cb(const struct nlmsghdr *nlhdr, void *data)
@@ -471,8 +480,10 @@ int sfeatures_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 		return MNL_CB_OK;
 	}
 
-	show_feature_changes(nlctx, tb);
-	return MNL_CB_OK;
+	if (show_feature_changes(nlctx, tb))
+		return MNL_CB_OK;
+	else
+		return MNL_CB_ERROR;
 }
 
 int nl_sfeatures(struct cmd_context *ctx)
-- 
2.21.0