mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Emerson Busson <emersonbusson@gmail.com>
To: mhklinux@outlook.com
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, andrew+netdev@lunn.ch, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
Subject: [PATCH v2 13/14] hv: netvsc: handle a NULL request address on empty completions
Date: Wed,  7 Oct 2026 16:07:51 -0300	[thread overview]
Message-ID: <20261007190752.336426-14-emersonbusson@gmail.com> (raw)
In-Reply-To: <20261007190752.336426-1-emersonbusson@gmail.com>

netvsc_send_pkt() registers (ulong)skb as the VMBus request address.
Control RNDIS sends carry no skb -- rndis_filter.c calls netvsc_send()
with skb == NULL -- so the registered address is NULL. That is
intentional: there is no guest object to hand back, and the request
slot is still allocated so the completion can reclaim it.

netvsc_send_tx_complete() already tolerates that with if (likely(skb)).
netvsc_send_completion()'s empty-payload branch does not. It casts the
request address to struct nvsp_message * and reads hdr.msg_type
unconditionally, so a completion that resolves to NULL is a fatal NULL
dereference in NAPI/softirq context.

The empty-payload branch exists for NVSP_MSG4_TYPE_SWITCH_DATA_PATH,
which netvsc_switch_datapath() sends with a real nvsp_message address.
A NULL request address is not that message and must not be
dereferenced. Validate VMBUS_NO_RQSTOR alongside VMBUS_RQST_ERROR in
both completion paths -- request_addr_callback() returns it when the
channel has no requestor -- and account a NULL-address completion the
same way the payload path accounts a NULL skb. The request id is
consumed by the lookup, so that completion is the one that owns the
queue_sends decrement and the possible queue wake.

This is reached by the runtime drill in this series once channel open
survives buddy fragmentation: control RNDIS traffic proceeds where it
used to fail with -ENOMEM, and an empty completion for one of those
requests takes the unguarded path.

Revert this patch if an empty completion with a NULL request address
dereferences again, if a netvsc TX stall or an "Invalid transaction
ID" flood appears under this patch, or if a SWITCH_DATA_PATH
completion is shown to be dropped instead of taken through its
nvsp_message path.

Adds eleven named netvsc completion KUnit cases covering the decision
layer of the empty-payload branch: a NULL request address owning the
queue_sends decrement, both invalid sentinels rejected without
accounting, SWITCH_DATA_PATH completing channel_init_wait without
accounting, an unexpected message type taking neither path, a
replayed transaction id accounting exactly once, N completions
decrementing the queue exactly N times, the destroy drain wait waking
only at zero, and netvsc_send_tx_complete() applying the same
sentinel rejection while still accounting a NULL skb. Two skb-backed
payload cases exercise the production completion entry with success
and error status, a published send slot and queue 1. They verify
synchronous skb consumption, exact packet/byte statistics, selected
queue accounting and duplicate transaction rejection. Confidential
DMA unmapping remains platform-specific and is not exercised here.

netvsc_send_acct() is extracted so a test can observe the decrement
and the drain wake directly; it is not a behaviour change. The
helpers lose static and are declared in hyperv_net.h so the cases
reach them without a new EXPORT_SYMBOL_GPL, matching how the VMBus
buffer tests are built into hv_vmbus.

Fixes: 8b31f8c982b7 ("hv_netvsc: Wait for completion on request SWITCH_DATA_PATH")
Signed-off-by: Emerson Busson <emersonbusson@gmail.com>
---
 drivers/net/hyperv/Makefile                 |   1 +
 drivers/net/hyperv/hyperv_net.h             |  15 +
 drivers/net/hyperv/netvsc.c                 |  81 ++--
 drivers/net/hyperv/netvsc_completion_test.c | 439 ++++++++++++++++++++
 4 files changed, 507 insertions(+), 29 deletions(-)
 create mode 100644 drivers/net/hyperv/netvsc_completion_test.c

diff --git a/drivers/net/hyperv/Makefile b/drivers/net/hyperv/Makefile
index 6f1abc756fde..b2d22855fd8a 100644
--- a/drivers/net/hyperv/Makefile
+++ b/drivers/net/hyperv/Makefile
@@ -2,4 +2,5 @@
 obj-$(CONFIG_HYPERV_NET) += hv_netvsc.o
 
 hv_netvsc-y := netvsc_drv.o netvsc.o rndis_filter.o netvsc_trace.o netvsc_bpf.o
+hv_netvsc-$(CONFIG_HYPERV_NET_KUNIT_TEST) += netvsc_completion_test.o
 hv_netvsc-$(CONFIG_HYPERV_NET_KUNIT_TEST) += rndis_request_test.o
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 6492e7e93ded..be3a63a168cf 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -229,6 +229,21 @@ int rndis_build_page_buffers(const void *data, u32 len,
 			     struct hv_page_buffer *page_bufs,
 			     u32 *page_buf_cnt);
 
+void netvsc_send_acct(struct net_device *ndev,
+		      struct netvsc_device *net_device,
+		      struct vmbus_channel *channel,
+		      u16 q_idx);
+void netvsc_send_tx_complete(struct net_device *ndev,
+			     struct netvsc_device *net_device,
+			     struct vmbus_channel *channel,
+			     const struct vmpacket_descriptor *desc,
+			     int budget);
+void netvsc_send_completion(struct net_device *ndev,
+			    struct netvsc_device *net_device,
+			    struct vmbus_channel *incoming_channel,
+			    const struct vmpacket_descriptor *desc,
+			    int budget);
+
 extern u32 netvsc_ring_bytes;
 
 int netvsc_workqueue_init(void);
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index fd8aa7a3dcb3..0d017f836b9e 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -764,20 +764,45 @@ static inline void netvsc_free_send_slot(struct netvsc_device *net_device,
 	sync_change_bit(index, net_device->send_section_map);
 }
 
-static void netvsc_send_tx_complete(struct net_device *ndev,
-				    struct netvsc_device *net_device,
-				    struct vmbus_channel *channel,
-				    const struct vmpacket_descriptor *desc,
-				    int budget)
+void netvsc_send_acct(struct net_device *ndev,
+		      struct netvsc_device *net_device,
+		      struct vmbus_channel *channel,
+		      u16 q_idx)
+{
+	struct net_device_context *ndev_ctx = netdev_priv(ndev);
+	int queue_sends;
+
+	queue_sends =
+		atomic_dec_return(&net_device->chan_table[q_idx].queue_sends);
+
+	if (unlikely(net_device->destroy)) {
+		if (queue_sends == 0)
+			wake_up(&net_device->wait_drain);
+	} else {
+		struct netdev_queue *txq = netdev_get_tx_queue(ndev, q_idx);
+
+		if (netif_tx_queue_stopped(txq) && !net_device->tx_disable &&
+		    (hv_get_avail_to_write_percent(&channel->outbound) >
+		     RING_AVAIL_PERCENT_HIWATER || queue_sends < 1)) {
+			netif_tx_wake_queue(txq);
+			ndev_ctx->eth_stats.wake_queue++;
+		}
+	}
+}
+
+void netvsc_send_tx_complete(struct net_device *ndev,
+			     struct netvsc_device *net_device,
+			     struct vmbus_channel *channel,
+			     const struct vmpacket_descriptor *desc,
+			     int budget)
 {
 	struct net_device_context *ndev_ctx = netdev_priv(ndev);
 	struct sk_buff *skb;
 	u16 q_idx = 0;
-	int queue_sends;
 	u64 cmd_rqst;
 
 	cmd_rqst = channel->request_addr_callback(channel, desc->trans_id);
-	if (cmd_rqst == VMBUS_RQST_ERROR) {
+	if (cmd_rqst == VMBUS_RQST_ERROR || cmd_rqst == VMBUS_NO_RQSTOR) {
 		netdev_err(ndev, "Invalid transaction ID %llx\n", desc->trans_id);
 		return;
 	}
@@ -806,29 +831,14 @@ static void netvsc_send_tx_complete(struct net_device *ndev,
 		napi_consume_skb(skb, budget);
 	}
 
-	queue_sends =
-		atomic_dec_return(&net_device->chan_table[q_idx].queue_sends);
-
-	if (unlikely(net_device->destroy)) {
-		if (queue_sends == 0)
-			wake_up(&net_device->wait_drain);
-	} else {
-		struct netdev_queue *txq = netdev_get_tx_queue(ndev, q_idx);
-
-		if (netif_tx_queue_stopped(txq) && !net_device->tx_disable &&
-		    (hv_get_avail_to_write_percent(&channel->outbound) >
-		     RING_AVAIL_PERCENT_HIWATER || queue_sends < 1)) {
-			netif_tx_wake_queue(txq);
-			ndev_ctx->eth_stats.wake_queue++;
-		}
-	}
+	netvsc_send_acct(ndev, net_device, channel, q_idx);
 }
 
-static void netvsc_send_completion(struct net_device *ndev,
-				   struct netvsc_device *net_device,
-				   struct vmbus_channel *incoming_channel,
-				   const struct vmpacket_descriptor *desc,
-				   int budget)
+void netvsc_send_completion(struct net_device *ndev,
+			    struct netvsc_device *net_device,
+			    struct vmbus_channel *incoming_channel,
+			    const struct vmpacket_descriptor *desc,
+			    int budget)
 {
 	const struct nvsp_message *nvsp_packet;
 	u32 msglen = hv_pkt_datalen(desc);
@@ -840,11 +850,24 @@ static void netvsc_send_completion(struct net_device *ndev,
 	if (!msglen) {
 		cmd_rqst = incoming_channel->request_addr_callback(incoming_channel,
 								   desc->trans_id);
-		if (cmd_rqst == VMBUS_RQST_ERROR) {
+		if (cmd_rqst == VMBUS_RQST_ERROR || cmd_rqst == VMBUS_NO_RQSTOR) {
 			netdev_err(ndev, "Invalid transaction ID %llx\n", desc->trans_id);
 			return;
 		}
 
+		/*
+		 * netvsc_send_pkt() registers (ulong)skb as the request
+		 * address. Control RNDIS sends carry no skb, so the
+		 * registered address is NULL and there is no nvsp_message
+		 * to inspect. The request id is consumed above, so this
+		 * completion owns the send accounting -- the same thing
+		 * netvsc_send_tx_complete() does when it sees a NULL skb.
+		 */
+		if (!cmd_rqst) {
+			netvsc_send_acct(ndev, net_device, incoming_channel, 0);
+			return;
+		}
+
 		pkt_rqst = (struct nvsp_message *)(uintptr_t)cmd_rqst;
 		switch (pkt_rqst->hdr.msg_type) {
 		case NVSP_MSG4_TYPE_SWITCH_DATA_PATH:
diff --git a/drivers/net/hyperv/netvsc_completion_test.c b/drivers/net/hyperv/netvsc_completion_test.c
new file mode 100644
index 000000000000..2614b1ece232
--- /dev/null
+++ b/drivers/net/hyperv/netvsc_completion_test.c
@@ -0,0 +1,439 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KUnit tests for empty and skb-backed netvsc completion handling.
+ *
+ * Built into the hv_netvsc object rather than a separate module so the
+ * cases can reach the internal helpers declared in hyperv_net.h
+ * without exporting them.
+ */
+#include <kunit/test.h>
+#include <linux/etherdevice.h>
+#include <linux/hyperv.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+
+#include "hyperv_net.h"
+
+/*
+ * Scripted requestor. request_addr_callback() only receives the channel
+ * and the transaction id, so the fixture is hung off a file-scope
+ * pointer. KUnit runs suite cases serially; one active fixture is
+ * enough and keeps the callback signature untouched.
+ */
+struct netvsc_completion_fixture {
+	struct net_device *ndev;
+	struct netvsc_device *nvdev;
+	struct vmbus_channel *channel;
+	/* Address returned on first use, then VMBUS_NO_RQSTOR: the id is
+	 * consumed exactly once, as vmbus_request_addr_match() does.
+	 */
+	u64 once_addr;
+	unsigned int calls;
+	u64 seen_ids[8];
+	unsigned int skb_frees;
+};
+
+static struct netvsc_completion_fixture *active_fx;
+
+static u64 test_request_addr(struct vmbus_channel *channel, u64 rqst_id)
+{
+	struct netvsc_completion_fixture *fx = active_fx;
+
+	if (fx->calls < ARRAY_SIZE(fx->seen_ids))
+		fx->seen_ids[fx->calls] = rqst_id;
+	fx->calls++;
+
+	if (fx->calls == 1)
+		return fx->once_addr;
+	return VMBUS_NO_RQSTOR;
+}
+
+/*
+ * A completion carrying no payload: hv_pkt_datalen() == 0. The
+ * empty-completion path looks the request up by trans_id and reads its
+ * message type, so the request itself is a separate object the fixture
+ * hands back through request_addr_callback().
+ */
+struct netvsc_empty_desc {
+	struct vmpacket_descriptor desc;
+} __packed;
+
+static void make_empty_desc(struct netvsc_empty_desc *pkt, u64 trans_id)
+{
+	memset(pkt, 0, sizeof(*pkt));
+	pkt->desc.offset8 = sizeof(pkt->desc) / 8;
+	pkt->desc.len8 = pkt->desc.offset8;
+	pkt->desc.trans_id = trans_id;
+}
+
+/*
+ * The production path never dereferences a net_device queue while
+ * tx_disable is set: netif_tx_queue_stopped() && !tx_disable
+ * short-circuits before hv_get_avail_to_write_percent() reads the
+ * ring. Set it here so a test needs no live outbound ring.
+ */
+static int netvsc_completion_fixture_init(struct netvsc_completion_fixture *fx)
+{
+	fx->ndev = alloc_netdev_mqs(sizeof(struct net_device_context),
+				    "hvcompl%d", NET_NAME_UNKNOWN,
+				    ether_setup, 2, 2);
+	if (!fx->ndev)
+		return -ENOMEM;
+
+	fx->nvdev = kzalloc_obj(*fx->nvdev, GFP_KERNEL);
+	if (!fx->nvdev) {
+		free_netdev(fx->ndev);
+		return -ENOMEM;
+	}
+
+	fx->channel = kzalloc_obj(*fx->channel, GFP_KERNEL);
+	if (!fx->channel) {
+		kfree(fx->nvdev);
+		free_netdev(fx->ndev);
+		return -ENOMEM;
+	}
+
+	init_waitqueue_head(&fx->nvdev->wait_drain);
+	init_completion(&fx->nvdev->channel_init_wait);
+	fx->nvdev->tx_disable = true;
+	fx->channel->request_addr_callback = test_request_addr;
+	fx->once_addr = 0;
+	fx->calls = 0;
+	memset(fx->seen_ids, 0, sizeof(fx->seen_ids));
+	active_fx = fx;
+	return 0;
+}
+
+static void netvsc_completion_fixture_exit(struct netvsc_completion_fixture *fx)
+{
+	active_fx = NULL;
+	kfree(fx->channel);
+	kfree(fx->nvdev);
+	free_netdev(fx->ndev);
+}
+
+static int netvsc_send_sends(struct netvsc_completion_fixture *fx, u16 q_idx)
+{
+	return atomic_read(&fx->nvdev->chan_table[q_idx].queue_sends);
+}
+
+/* Empty completion, request address NULL: owns the send accounting. */
+static void netvsc_completion_null_address_acct_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct netvsc_empty_desc pkt;
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 3);
+	make_empty_desc(&pkt, 0x11);
+	/* once_addr = 0 is the control-path NULL skb case. */
+	fx.once_addr = 0;
+
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 2);
+	KUNIT_EXPECT_FALSE(test, completion_done(&fx.nvdev->channel_init_wait));
+	KUNIT_EXPECT_EQ(test, fx.calls, 1U);
+	KUNIT_EXPECT_EQ(test, fx.seen_ids[0], 0x11U);
+
+	netvsc_completion_fixture_exit(&fx);
+}
+
+/* VMBUS_RQST_ERROR is rejected: no accounting, no completion. */
+static void netvsc_completion_rqst_error_sentinel_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct netvsc_empty_desc pkt;
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 3);
+	make_empty_desc(&pkt, 0x22);
+	fx.once_addr = VMBUS_RQST_ERROR;
+
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 3);
+	KUNIT_EXPECT_FALSE(test, completion_done(&fx.nvdev->channel_init_wait));
+
+	netvsc_completion_fixture_exit(&fx);
+}
+
+/* VMBUS_NO_RQSTOR is rejected the same way. */
+static void netvsc_completion_no_rqstor_sentinel_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct netvsc_empty_desc pkt;
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 3);
+	make_empty_desc(&pkt, 0x33);
+	fx.once_addr = VMBUS_NO_RQSTOR;
+
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 3);
+	KUNIT_EXPECT_FALSE(test, completion_done(&fx.nvdev->channel_init_wait));
+
+	netvsc_completion_fixture_exit(&fx);
+}
+
+/*
+ * A real request address whose message type is SWITCH_DATA_PATH
+ * completes the channel-init wait. It is not send accounting: the
+ * control request was not a send.
+ */
+static void netvsc_completion_switch_data_path_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct netvsc_empty_desc pkt;
+	struct nvsp_message req = {};
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 3);
+	make_empty_desc(&pkt, 0x44);
+	req.hdr.msg_type = NVSP_MSG4_TYPE_SWITCH_DATA_PATH;
+	fx.once_addr = (u64)(unsigned long)&req;
+
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+
+	KUNIT_EXPECT_TRUE(test, completion_done(&fx.nvdev->channel_init_wait));
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 3);
+
+	netvsc_completion_fixture_exit(&fx);
+}
+
+/* An unexpected message type takes neither the complete nor the acct path. */
+static void netvsc_completion_unknown_msg_type_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct netvsc_empty_desc pkt;
+	struct nvsp_message req = {};
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 3);
+	make_empty_desc(&pkt, 0x55);
+	req.hdr.msg_type = 0xdead;
+	fx.once_addr = (u64)(unsigned long)&req;
+
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+
+	KUNIT_EXPECT_FALSE(test, completion_done(&fx.nvdev->channel_init_wait));
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 3);
+
+	netvsc_completion_fixture_exit(&fx);
+}
+
+/*
+ * A repeated completion for an already-consumed transaction id must not
+ * account twice. The requestor returns VMBUS_NO_RQSTOR on the second
+ * use and the empty-completion path rejects it.
+ */
+static void netvsc_completion_duplicate_completion_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct netvsc_empty_desc pkt;
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 3);
+	make_empty_desc(&pkt, 0x66);
+	fx.once_addr = 0;
+
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 2);
+	KUNIT_EXPECT_EQ(test, fx.calls, 1U);
+
+	/* Second completion for the same id: already consumed. */
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 2);
+	KUNIT_EXPECT_EQ(test, fx.calls, 2U);
+	KUNIT_EXPECT_EQ(test, fx.seen_ids[0], 0x66U);
+	KUNIT_EXPECT_EQ(test, fx.seen_ids[1], 0x66U);
+
+	netvsc_completion_fixture_exit(&fx);
+}
+
+/* N completions decrement the queue exactly N times. */
+static void netvsc_completion_single_decrement_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct netvsc_empty_desc pkt;
+	int i;
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 5);
+
+	for (i = 0; i < 5; i++) {
+		make_empty_desc(&pkt, 0x100 + i);
+		fx.once_addr = 0;
+		fx.calls = 0;
+		netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel,
+				       &pkt.desc, 0);
+		KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 4 - i);
+		KUNIT_EXPECT_EQ(test, fx.calls, 1U);
+	}
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 0);
+
+	netvsc_completion_fixture_exit(&fx);
+}
+
+static int netvsc_test_wake(struct wait_queue_entry *wq_entry,
+			    unsigned int mode, int flags, void *key)
+{
+	unsigned int *woken = wq_entry->private;
+
+	(*woken)++;
+	return 0;
+}
+
+/* destroy + a decrement that reaches zero wakes the drain wait. */
+static void netvsc_send_acct_drain_wake_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct wait_queue_entry entry;
+	unsigned int woken = 0;
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	fx.nvdev->destroy = true;
+
+	memset(&entry, 0, sizeof(entry));
+	entry.func = netvsc_test_wake;
+	entry.private = &woken;
+	add_wait_queue(&fx.nvdev->wait_drain, &entry);
+
+	/* Non-zero remainder: the drain wait is not woken. */
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 2);
+	netvsc_send_acct(fx.ndev, fx.nvdev, fx.channel, 0);
+	KUNIT_EXPECT_EQ(test, woken, 0U);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 1);
+
+	/* Exactly zero: the drain wait is woken once. */
+	netvsc_send_acct(fx.ndev, fx.nvdev, fx.channel, 0);
+	KUNIT_EXPECT_EQ(test, woken, 1U);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 0);
+
+	remove_wait_queue(&fx.nvdev->wait_drain, &entry);
+	netvsc_completion_fixture_exit(&fx);
+}
+
+/*
+ * netvsc_send_tx_complete() applies the same sentinel rejection and
+ * still accounts a NULL skb.
+ */
+static void netvsc_tx_complete_null_skb_acct_test(struct kunit *test)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct netvsc_empty_desc pkt;
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 3);
+	make_empty_desc(&pkt, 0x77);
+	fx.once_addr = 0;
+
+	netvsc_send_tx_complete(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 2);
+
+	/* The sentinel is rejected before any accounting. */
+	make_empty_desc(&pkt, 0x78);
+	fx.once_addr = VMBUS_RQST_ERROR;
+	fx.calls = 0;
+	netvsc_send_tx_complete(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 2);
+
+	netvsc_completion_fixture_exit(&fx);
+}
+
+static void netvsc_test_skb_destructor(struct sk_buff *skb)
+{
+	active_fx->skb_frees++;
+}
+
+static void netvsc_tx_complete_skb(struct kunit *test, u32 status)
+{
+	struct netvsc_completion_fixture fx = {};
+	struct {
+		struct vmpacket_descriptor desc;
+		struct nvsp_message message;
+	} pkt = {};
+	unsigned long send_slots = BIT(3);
+	struct hv_netvsc_packet *packet;
+	struct netvsc_stats_tx *stats;
+	struct sk_buff *skb;
+
+	KUNIT_ASSERT_EQ(test, netvsc_completion_fixture_init(&fx), 0);
+	skb = alloc_skb(64, GFP_KERNEL);
+	if (!skb) {
+		KUNIT_FAIL(test, "skb allocation failed");
+		goto out;
+	}
+	skb->destructor = netvsc_test_skb_destructor;
+	packet = (struct hv_netvsc_packet *)skb->cb;
+	memset(packet, 0, sizeof(*packet));
+	packet->send_buf_index = 3;
+	packet->q_idx = 1;
+	packet->total_packets = 2;
+	packet->total_bytes = 64;
+	fx.nvdev->send_section_map = &send_slots;
+	stats = &fx.nvdev->chan_table[1].tx_stats;
+	u64_stats_init(&stats->syncp);
+	atomic_set(&fx.nvdev->chan_table[0].queue_sends, 7);
+	atomic_set(&fx.nvdev->chan_table[1].queue_sends, 3);
+	fx.once_addr = (u64)(unsigned long)skb;
+	pkt.desc.offset8 = sizeof(pkt.desc) / 8;
+	pkt.desc.len8 = DIV_ROUND_UP(sizeof(pkt), 8);
+	pkt.desc.trans_id = 0x79;
+	pkt.message.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE;
+	pkt.message.msg.v1_msg.send_rndis_pkt_complete.status = status;
+
+	/* Budget zero consumes the real skb synchronously, outside NAPI. */
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+	KUNIT_EXPECT_EQ(test, send_slots, 0UL);
+	KUNIT_EXPECT_EQ(test, fx.skb_frees, 1U);
+	KUNIT_EXPECT_EQ(test, stats->packets, 2ULL);
+	KUNIT_EXPECT_EQ(test, stats->bytes, 64ULL);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 0), 7);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 1), 2);
+
+	/* The consumed transaction cannot free or account the skb twice. */
+	netvsc_send_completion(fx.ndev, fx.nvdev, fx.channel, &pkt.desc, 0);
+	KUNIT_EXPECT_EQ(test, send_slots, 0UL);
+	KUNIT_EXPECT_EQ(test, fx.skb_frees, 1U);
+	KUNIT_EXPECT_EQ(test, stats->packets, 2ULL);
+	KUNIT_EXPECT_EQ(test, stats->bytes, 64ULL);
+	KUNIT_EXPECT_EQ(test, netvsc_send_sends(&fx, 1), 2);
+out:
+	netvsc_completion_fixture_exit(&fx);
+}
+
+static void netvsc_tx_complete_skb_success_test(struct kunit *test)
+{
+	netvsc_tx_complete_skb(test, NVSP_STAT_SUCCESS);
+}
+
+static void netvsc_tx_complete_skb_error_test(struct kunit *test)
+{
+	netvsc_tx_complete_skb(test, NVSP_STAT_FAIL);
+}
+
+static struct kunit_case netvsc_completion_test_cases[] = {
+	KUNIT_CASE(netvsc_completion_null_address_acct_test),
+	KUNIT_CASE(netvsc_completion_rqst_error_sentinel_test),
+	KUNIT_CASE(netvsc_completion_no_rqstor_sentinel_test),
+	KUNIT_CASE(netvsc_completion_switch_data_path_test),
+	KUNIT_CASE(netvsc_completion_unknown_msg_type_test),
+	KUNIT_CASE(netvsc_completion_duplicate_completion_test),
+	KUNIT_CASE(netvsc_completion_single_decrement_test),
+	KUNIT_CASE(netvsc_send_acct_drain_wake_test),
+	KUNIT_CASE(netvsc_tx_complete_null_skb_acct_test),
+	KUNIT_CASE(netvsc_tx_complete_skb_success_test),
+	KUNIT_CASE(netvsc_tx_complete_skb_error_test),
+	{}
+};
+
+static struct kunit_suite netvsc_completion_test_suite = {
+	.name = "hyperv-netvsc-completion",
+	.test_cases = netvsc_completion_test_cases,
+};
+
+kunit_test_suite(netvsc_completion_test_suite);
-- 
2.43.0


  parent reply	other threads:[~2026-10-07 19:09 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-07 19:07 [PATCH v2 0/14] hv: vmbus: make rings and host-visible buffers survive buddy fragmentation Emerson Busson
2026-10-07 19:07 ` [PATCH v2 01/14] hv: vmbus: convert ring backing through the chunk allocator Emerson Busson
2026-10-07 19:07 ` [PATCH v2 02/14] hv: vmbus: validate chunk buffer allocation and cleanup Emerson Busson
2026-10-08 21:17   ` kernel test robot
2026-10-07 19:07 ` [PATCH v2 03/14] uio: hv_generic: describe buffers for owned allocation Emerson Busson
2026-10-07 19:07 ` [PATCH v2 04/14] hv: vmbus: add KUnit tests for GPADL post failure injection Emerson Busson
2026-10-07 19:07 ` [PATCH v2 05/14] hv: vmbus: add KUnit test for order-zero allocation fallback Emerson Busson
2026-10-07 19:07 ` [PATCH v2 06/14] hv: vmbus: cover all shared-page policy combinations Emerson Busson
2026-10-07 19:07 ` [PATCH v2 07/14] hv: vmbus: distinguish host rescind from local channel unload Emerson Busson
2026-10-07 19:07 ` [PATCH v2 08/14] hv: vmbus: retain backing until ownership and references clear Emerson Busson
2026-10-07 19:07 ` [PATCH v2 09/14] hv: use owned VMBus buffers in NetVSC and UIO Emerson Busson
2026-10-07 19:07 ` [PATCH v2 10/14] hv: vmbus: pin buffer pages across UIO mmap to close the reclaim race Emerson Busson
2026-10-08 16:49   ` kernel test robot
2026-10-08 17:51     ` Nathan Chancellor
2026-10-08 17:02   ` kernel test robot
2026-10-07 19:07 ` [PATCH v2 11/14] hv: vmbus: vmalloc requestor metadata Emerson Busson
2026-10-07 19:07 ` [PATCH v2 12/14] hv: netvsc: allocate RNDIS request descriptors with kvzalloc_obj() Emerson Busson
2026-10-07 19:07 ` Emerson Busson [this message]
2026-10-07 19:07 ` [PATCH v2 14/14] hv: netvsc: use kvzalloc for device state Emerson Busson
2026-10-08 16:55 ` [PATCH v2 0/14] hv: vmbus: make rings and host-visible buffers survive buddy fragmentation Easwar Hariharan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261007190752.336426-14-emersonbusson@gmail.com \
    --to=emersonbusson@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=decui@microsoft.com \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haiyangz@microsoft.com \
    --cc=kuba@kernel.org \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhklinux@outlook.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®