[PATCH net-next] selftests: drv-net: ncdevmem: make configure_channels() support combined channels

[PATCH net-next] selftests: drv-net: ncdevmem: make configure_channels() support combined channels

[Jakub Kicinski]

ncdevmem tests that the kernel correctly rejects attempts
to deactivate queues with MPs bound.

Make the configure_channels() test support combined channels.
Currently it tries to set the queue counts to rx N tx N-1,
which only makes sense for devices which have IRQs per ring
type. Most modern devices used combined IRQs/channels with
both Rx and Tx queues. Since the math is total Rx == combined+Rx
setting Rx when combined is non-zero will be increasing the total
queue count, not decreasing as the test intends.

Note that the test would previously also try to set the Tx
ring count to Rx - 1, for some reason. Which would be 0
if the device has only 2 queues configured.

With this change (device with 2 queues):
  setting channel count rx:1 tx:1
  YNL set channels: Kernel error: 'requested channel counts are too low for existing memory provider setting (2)'

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: shuah@kernel.org
CC: almasrymina@google.com
CC: sdf@fomichev.me
CC: joe@dama.to
CC: linux-kselftest@vger.kernel.org
---
 .../selftests/drivers/net/hw/ncdevmem.c       | 78 ++++++++++++++++++-
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
index be937542b4c0..71961a7688e6 100644
--- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c
+++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
@@ -356,7 +356,81 @@ static int configure_rss(void)
 
 static int configure_channels(unsigned int rx, unsigned int tx)
 {
-	return run_command("ethtool -L %s rx %u tx %u", ifname, rx, tx);
+	struct ethtool_channels_get_req *gchan;
+	struct ethtool_channels_set_req *schan;
+	struct ethtool_channels_get_rsp *chan;
+	struct ynl_error yerr;
+	struct ynl_sock *ys;
+	int ret;
+
+	fprintf(stderr, "setting channel count rx:%u tx:%u\n", rx, tx);
+
+	ys = ynl_sock_create(&ynl_ethtool_family, &yerr);
+	if (!ys) {
+		fprintf(stderr, "YNL: %s\n", yerr.msg);
+		return -1;
+	}
+
+	gchan = ethtool_channels_get_req_alloc();
+	if (!gchan) {
+		ret = -1;
+		goto exit_close_sock;
+	}
+
+	ethtool_channels_get_req_set_header_dev_index(gchan, ifindex);
+	chan = ethtool_channels_get(ys, gchan);
+	ethtool_channels_get_req_free(gchan);
+	if (!chan) {
+		fprintf(stderr, "YNL get channels: %s\n", ys->err.msg);
+		ret = -1;
+		goto exit_close_sock;
+	}
+
+	schan =	ethtool_channels_set_req_alloc();
+	if (!schan) {
+		ret = -1;
+		goto exit_free_chan;
+	}
+
+	ethtool_channels_set_req_set_header_dev_index(schan, ifindex);
+
+	if (chan->_present.combined_count) {
+		if (chan->_present.rx_count || chan->_present.tx_count) {
+			ethtool_channels_set_req_set_rx_count(schan, 0);
+			ethtool_channels_set_req_set_tx_count(schan, 0);
+		}
+
+		if (rx == tx) {
+			ethtool_channels_set_req_set_combined_count(schan, rx);
+		} else if (rx > tx) {
+			ethtool_channels_set_req_set_combined_count(schan, tx);
+			ethtool_channels_set_req_set_rx_count(schan, rx - tx);
+		} else {
+			ethtool_channels_set_req_set_combined_count(schan, rx);
+			ethtool_channels_set_req_set_tx_count(schan, tx - rx);
+		}
+
+		ret = ethtool_channels_set(ys, schan);
+		if (ret)
+			fprintf(stderr, "YNL set channels: %s\n", ys->err.msg);
+	} else if (chan->_present.rx_count) {
+		ethtool_channels_set_req_set_rx_count(schan, rx);
+		ethtool_channels_set_req_set_tx_count(schan, tx);
+
+		ret = ethtool_channels_set(ys, schan);
+		if (ret)
+			fprintf(stderr, "YNL set channels: %s\n", ys->err.msg);
+	} else {
+		fprintf(stderr, "Error: device has neither combined nor rx channels\n");
+		ret = -1;
+	}
+	ethtool_channels_set_req_free(schan);
+exit_free_chan:
+	ethtool_channels_get_rsp_free(chan);
+exit_close_sock:
+	ynl_sock_destroy(ys);
+
+	return ret;
 }
 
 static int configure_flow_steering(struct sockaddr_in6 *server_sin)
@@ -752,7 +826,7 @@ void run_devmem_tests(void)
 		error(1, 0, "Failed to bind\n");
 
 	/* Deactivating a bound queue should not be legal */
-	if (!configure_channels(num_queues, num_queues - 1))
+	if (!configure_channels(num_queues, num_queues))
 		error(1, 0, "Deactivating a bound queue should be illegal.\n");
 
 	/* Closing the netlink socket does an implicit unbind */
-- 
2.50.1

Re: [PATCH net-next] selftests: drv-net: ncdevmem: make configure_channels() support combined channels

[Mina Almasry]

On Fri, Aug 15, 2025 at 4:15 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> ncdevmem tests that the kernel correctly rejects attempts
> to deactivate queues with MPs bound.
>
> Make the configure_channels() test support combined channels.
> Currently it tries to set the queue counts to rx N tx N-1,
> which only makes sense for devices which have IRQs per ring
> type. Most modern devices used combined IRQs/channels with
> both Rx and Tx queues. Since the math is total Rx == combined+Rx
> setting Rx when combined is non-zero will be increasing the total
> queue count, not decreasing as the test intends.
>
> Note that the test would previously also try to set the Tx
> ring count to Rx - 1, for some reason. Which would be 0
> if the device has only 2 queues configured.
>

Yes, I think that was a mistake. I can't think of any reason we'd
really want to do that.

> With this change (device with 2 queues):
>   setting channel count rx:1 tx:1
>   YNL set channels: Kernel error: 'requested channel counts are too low for existing memory provider setting (2)'
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Mina Almasry <almasrymina@google.com>

--
Thanks,
Mina

Re: [PATCH net-next] selftests: drv-net: ncdevmem: make configure_channels() support combined channels

[patchwork-bot+netdevbpf]

Hello:

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

On Fri, 15 Aug 2025 16:15:13 -0700 you wrote:
> ncdevmem tests that the kernel correctly rejects attempts
> to deactivate queues with MPs bound.
> 
> Make the configure_channels() test support combined channels.
> Currently it tries to set the queue counts to rx N tx N-1,
> which only makes sense for devices which have IRQs per ring
> type. Most modern devices used combined IRQs/channels with
> both Rx and Tx queues. Since the math is total Rx == combined+Rx
> setting Rx when combined is non-zero will be increasing the total
> queue count, not decreasing as the test intends.
> 
> [...]

Here is the summary with links:
  - [net-next] selftests: drv-net: ncdevmem: make configure_channels() support combined channels
    https://git.kernel.org/netdev/net-next/c/51992f99f068

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