netdev - [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL

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-next>] [day] [month] [year] [list]

Message-Id: <20240913093828.2549217-1-vladimir.oltean@nxp.com>
Date: Fri, 13 Sep 2024 12:38:28 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: netdev@...r.kernel.org
Cc: Michal Kubecek <mkubecek@...e.cz>,
	Jakub Kicinski <kuba@...nel.org>,
	Sudheer Mogilappagari <sudheer.mogilappagari@...el.com>
Subject: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

Several drivers regressed when ethtool --show-rxfh was converted from
ioctl to netlink. This is because ETHTOOL_GRXRINGS was converted to
ETHTOOL_MSG_CHANNELS_GET, which is semantically equivalent to
ETHTOOL_GCHANNELS but different from ETHTOOL_GRXRINGS. Drivers which
implement ETHTOOL_GRXRINGS do not necessarily implement ETHTOOL_GCHANNELS
or its netlink equivalent.

According to the man page, "A channel is an IRQ and the set of queues
that can trigger that IRQ.", which is different from the definition of
a queue/ring. So we shouldn't be attempting to query the # of rings for
the ioctl variant, but the # of channels for the netlink variant anyway.

Reimplement the args->num_rings retrieval as in do_grxfh(), aka using
the ETHTOOL_GRXRINGS ioctl.

Link: https://lore.kernel.org/netdev/20240711114535.pfrlbih3ehajnpvh@skbuf/
Fixes: ffab99c1f382 ("netlink: add netlink handler for get rss (-x)")
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
 ethtool.c     |  2 +-
 internal.h    |  1 +
 netlink/rss.c | 55 +++++++++++++++++++--------------------------------
 3 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 98690df05992..89c0edefe8eb 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6369,7 +6369,7 @@ static int do_perqueue(struct cmd_context *ctx)
 	return 0;
 }
 
-static int ioctl_init(struct cmd_context *ctx, bool no_dev)
+int ioctl_init(struct cmd_context *ctx, bool no_dev)
 {
 	if (no_dev) {
 		ctx->fd = -1;
diff --git a/internal.h b/internal.h
index 3923719c39d5..1bd5515fc9f7 100644
--- a/internal.h
+++ b/internal.h
@@ -293,6 +293,7 @@ int test_fclose(FILE *fh);
 #endif
 #endif
 
+int ioctl_init(struct cmd_context *ctx, bool no_dev);
 int send_ioctl(struct cmd_context *ctx, void *cmd);
 
 void dump_hex(FILE *f, const u8 *data, int len, int offset);
diff --git a/netlink/rss.c b/netlink/rss.c
index 4ad6065ef698..d0045b1f0f8f 100644
--- a/netlink/rss.c
+++ b/netlink/rss.c
@@ -54,29 +54,29 @@ void dump_json_rss_info(struct cmd_context *ctx, u32 *indir_table,
 	close_json_object();
 }
 
-int get_channels_cb(const struct nlmsghdr *nlhdr, void *data)
+/* There is no netlink equivalent for ETHTOOL_GRXRINGS. */
+static int get_num_rings(struct cb_args *args)
 {
-	const struct nlattr *tb[ETHTOOL_A_CHANNELS_MAX + 1] = {};
-	DECLARE_ATTR_TB_INFO(tb);
-	struct cb_args *args = data;
 	struct nl_context *nlctx = args->nlctx;
-	bool silent;
-	int err_ret;
+	struct cmd_context *ctx = nlctx->ctx;
+	struct ethtool_rxnfc ring_count = {
+		.cmd = ETHTOOL_GRXRINGS,
+	};
 	int ret;
 
-	silent = nlctx->is_dump || nlctx->is_monitor;
-	err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR;
-	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
-	if (ret < 0)
-		return err_ret;
-	nlctx->devname = get_dev_name(tb[ETHTOOL_A_CHANNELS_HEADER]);
-	if (!dev_ok(nlctx))
-		return err_ret;
-	if (tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT])
-		args->num_rings = mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT]);
-	if (tb[ETHTOOL_A_CHANNELS_RX_COUNT])
-		args->num_rings += mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_RX_COUNT]);
-	return MNL_CB_OK;
+	ret = ioctl_init(ctx, false);
+	if (ret)
+		return ret;
+
+	ret = send_ioctl(ctx, &ring_count);
+	if (ret) {
+		perror("Cannot get RX ring count");
+		return ret;
+	}
+
+	args->num_rings = (u32)ring_count.data;
+
+	return 0;
 }
 
 int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data)
@@ -131,22 +131,7 @@ int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 	if (ret < 0)
 		return silent ? MNL_CB_OK : MNL_CB_ERROR;
 
-	nlctx->devname = get_dev_name(tb[ETHTOOL_A_RSS_HEADER]);
-	if (!dev_ok(nlctx))
-		return MNL_CB_OK;
-
-	/* Fetch ring count info into args->num_rings */
-	ret = nlsock_prep_get_request(nlctx->ethnl2_socket,
-				      ETHTOOL_MSG_CHANNELS_GET,
-				      ETHTOOL_A_CHANNELS_HEADER, 0);
-	if (ret < 0)
-		return MNL_CB_ERROR;
-
-	ret = nlsock_sendmsg(nlctx->ethnl2_socket, NULL);
-	if (ret < 0)
-		return MNL_CB_ERROR;
-
-	ret = nlsock_process_reply(nlctx->ethnl2_socket, get_channels_cb, args);
+	ret = get_num_rings(args);
 	if (ret < 0)
 		return MNL_CB_ERROR;
 
-- 
2.34.1