* [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead
@ 2026-11-03 4:11 Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 1/5] vsock/virtio: split socket lookup from locked RX processing Jia Jia
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Jia Jia @ 2026-11-03 4:11 UTC (permalink / raw)
To: netdev
Cc: stefanha, sgarzare, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
This series reduces per-packet socket overhead in the virtio-vsock RX
worker.
Patch 1: separate socket lookup from locked packet processing.
Patch 2: factor out the RX skb metadata-budget check.
Patch 3: keep one socket lock across a bounded run of established
STREAM/RW packets for the same native socket, and reuse the locked socket
for later same-flow packets.
Patch 4: coalesce default write-space notifications while that
lock is held.
Patch 5: defer the default readable callback until after the batch
unlocks.
Performance:
Tested on a single-stream vsock connection (one Guest receiver thread on
CPU0, virtio-vsock IRQ on Guest CPU1, fresh QEMU Guest per state, with 16
balanced AB/BA pairs per payload for each baseline/patchset comparison).
The host uses vhost-vsock.
For throughput, vsock_perf runs on the host as the
sender, with a custom single-threaded Guest receiver pinned to Guest CPU0
using blocking recv(). The fixed-64K table uses a 64K buffer; the matched
table uses the sender's request size. The virtio-vsock IRQ/RX worker is
pinned to Guest CPU1. To normalize run times across payloads into a
bounded, deterministic window and avoid long runs being skewed by host
background noise or virtualization scheduling jitter, transfer sizes scale
proportionally with the request size (64B/64M, 256B/256M, etc.), capped at
2G for 4K–64K. The custom receiver keeps the recv() size and count
explicit; vsock_perf's receiver uses poll()+read().
Latency uses a custom user-space vsock ping-pong tool.
The measurements below were collected for the RX batching series.
This benchmark does not run vsock_perf on both ends. Instead, it uses
vsock_perf as the sender on the host (vhost-vsock) side and a custom
blocking recv() program without poll() on the Guest side, with the receiver
thread pinned to Guest CPU0 (and the virtio-vsock RX worker pinned to Guest
CPU1). Plain blocking recv() combined with strict core pinning gives
audit-level determinism for tracing a single-threaded TID.
With all patches applied:
End-to-end RX throughput (64K Guest recv() size, SO_RCVLOWAT=1; Gbit/s):
payload baseline RX PATCH RX throughput change (95% CI)
(Gbit/s) (Gbit/s)
64B 0.1290 0.2489 +92.91% [+89.31%, +96.58%]
256B 0.5780 0.9727 +68.29% [+66.14%, +70.48%]
512B 1.1688 1.8316 +56.70% [+53.85%, +59.61%]
1K 2.3059 3.3377 +44.75% [+43.39%, +46.12%]
4K 3.8787 5.7310 +47.76% [+45.20%, +50.36%]
8K 4.5983 5.4575 +18.68% [+16.42%, +20.99%]
16K 5.0806 5.5615 +9.47% [ +8.05%, +10.90%]
64K 5.4842 5.6423 +2.88% [ +1.13%, +4.67%]
Fixed-syscall stress test (receiver recv() size = sender write size;
Gbit/s):
payload baseline PATCH throughput change (95% CI) recv()
RX RX (base/patch)
(Gbit/s) (Gbit/s)
64B 0.1384 0.2532 +82.99% [+74.19%, +92.24%] 1048577/1048577
256B 0.4042 0.8983 +122.25% [+113.06%, +131.85%] 1048577/1048577
512B 0.8056 1.6035 +99.05% [+89.34%, +109.26%] 1048577/1048577
1K 1.6030 2.7158 +69.42% [+63.22%, +75.85%] 1048577/1048577
4K 3.0194 4.9586 +64.22% [+58.43%, +70.23%] 536799/525225
8K 3.8316 5.1255 +33.77% [+31.00%, +36.59%] 274864/263013
16K 4.5702 5.3808 +17.74% [+14.87%, +20.67%] 144958/131332
64K 5.4736 5.7118 +4.35% [ +2.79%, +5.94%] 49039/32938
Request-response latency (Ping-Pong RTT, 10,000 requests):
payload baseline mean RTT (us) PATCH mean RTT (us) change (95% CI)
64B 224.148 224.151 +0.00%
[-2.26%, +2.32%]
256B 222.037 224.499 +1.11%
[-1.66%, +3.95%]
4K 225.982 227.026 +0.46%
[-1.15%, +2.10%]
Guest CPU1 aggregate cycles per received byte (fixed 64K receiver, with perf):
Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
64B 65.282 57.843 -11.40% [-13.77%, -8.95%]
256B 14.710 13.813 -6.10% [ -7.89%, -4.28%]
512B 7.709 7.188 -6.75% [ -8.79%, -4.66%]
1K 4.075 3.862 -5.23% [ -6.80%, -3.63%]
4K 2.034 1.887 -7.21% [ -8.67%, -5.72%]
8K 1.664 1.568 -5.78% [ -7.83%, -3.69%]
16K 1.450 1.385 -4.48% [ -6.08%, -2.86%]
64K 1.338 1.317 -1.51% [ -3.21%, +0.22%]
P1-P3 contribution:
payload baseline RX PATCH RX throughput change (95% CI)
(Gbit/s) (Gbit/s)
64B 0.1285 0.2462 +91.59% [+79.74%, +104.22%]
256B 0.5909 0.9666 +63.59% [+61.03%, +66.19%]
512B 1.1817 1.8350 +55.29% [+46.76%, +64.32%]
1K 2.3252 3.3600 +44.50% [+38.93%, +50.30%]
4K 3.8436 5.5661 +44.82% [+40.30%, +49.47%]
8K 4.6609 5.4115 +16.10% [ +8.91%, +23.77%]
16K 5.1196 5.6399 +10.16% [ +4.07%, +16.61%]
64K 5.6095 5.7062 +1.72% [-3.19%, +6.89%]
Fixed-syscall stress test (receiver recv() size = sender write size;
no perf; Gbit/s):
payload baseline PATCH throughput change (95% CI) recv()
RX RX (base/patch)
(Gbit/s) (Gbit/s)
64B 0.1446 0.2540 +75.71% [+63.67%, +88.62%] 1048577/1048577
256B 0.4211 0.9061 +115.18% [+72.13%, +169.00%] 1048577/1048577
512B 0.7980 1.6112 +101.90% [+95.30%, +108.72%] 1048577/1048577
1K 1.4965 2.6986 +80.33% [+68.93%, +92.49%] 1048577/1048577
4K 2.9384 4.8590 +65.36% [+60.17%, +70.72%] 539544/524982
8K 3.8170 5.1887 +35.94% [+27.66%, +44.75%] 274857/262991
16K 4.6856 5.5996 +19.51% [+15.36%, +23.81%] 147518/131351
64K 5.5182 5.8800 +6.56% [-0.83%, +14.49%] 48492/32910
Guest CPU1 aggregate cycles per received byte (fixed 64K receiver, with perf):
Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
64B 66.728 61.220 -8.25% [-18.99%, +3.91%]
256B 14.363 14.515 +1.06% [ -4.22%, +6.64%]
512B 7.792 7.578 -2.74% [ -6.55%, +1.23%]
1K 3.988 4.032 +1.09% [ -1.64%, +3.88%]
4K 2.063 1.973 -4.38% [ -8.23%, -0.37%]
8K 1.702 1.667 -2.06% [ -7.99%, +4.25%]
16K 1.473 1.475 +0.17% [ -5.06%, +5.68%]
64K 1.385 1.358 -1.96% [-10.61%, +7.53%]
P4-P5 contribution (combined):
End-to-end RX throughput (64K Guest recv() size, SO_RCVLOWAT=1; Gbit/s):
payload P1-P3 RX P1-P5 RX throughput change (95% CI)
(Gbit/s) (Gbit/s)
64B 0.2449 0.2463 +0.55% [-3.86%, +5.17%]
256B 0.9649 0.9931 +2.93% [+0.83%, +5.07%]
512B 1.8205 1.8260 +0.30% [-4.27%, +5.10%]
1K 3.3431 3.3420 -0.03% [-2.15%, +2.12%]
4K 5.6541 5.8428 +3.34% [+1.10%, +5.63%]
8K 5.4280 5.3364 -1.69% [-15.55%, +14.46%]
16K 5.5935 5.6701 +1.37% [-1.41%, +4.23%]
64K 5.6917 5.8351 +2.52% [-0.20%, +5.31%]
Fixed-syscall stress test (receiver recv() size = sender write size;
Gbit/s):
payload P1-P3 P1-P5 throughput change (95% CI) recv()
RX RX (P1-P3/P1-P5)
(Gbit/s) (Gbit/s)
64B 0.2553 0.2575 +0.87% [-3.68%, +5.63%] 1048577/1048577
256B 0.9069 0.9183 +1.26% [-3.96%, +6.76%] 1048577/1048577
512B 1.6048 1.6459 +2.56% [+0.98%, +4.18%] 1048577/1048577
1K 2.6939 2.7357 +1.55% [-1.35%, +4.54%] 1048577/1048577
4K 4.9168 4.9678 +1.04% [-1.84%, +4.00%] 524965/525283
8K 5.1476 5.2005 +1.03% [-4.02%, +6.34%] 262926/263156
16K 5.5964 5.6609 +1.15% [-2.86%, +5.33%] 131311/131475
64K 5.9439 5.9073 -0.62% [-3.00%, +1.83%] 32875/32937
Guest CPU1 aggregate cycles per received byte (fixed 64K receiver, with perf):
payload P1-P3 P1-P5 change (95% CI)
64B 60.281 58.258 -3.36% [-8.74%, +2.35%]
256B 14.242 13.648 -4.17% [-9.81%, +1.82%]
512B 7.547 7.211 -4.45% [-9.73%, +1.13%]
1K 3.987 3.853 -3.35% [-8.52%, +2.12%]
4K 1.983 1.880 -5.17% [-7.97%, -2.27%]
8K 1.665 1.620 -2.75% [-9.03%, +3.96%]
16K 1.409 1.346 -4.52% [-10.94%, +2.37%]
64K 1.322 1.281 -3.11% [-6.49%, +0.38%]
---
This rerun used a stable host with a single Guest running and no background
workload. The baseline throughput is higher and more stable than in v4, and
the previous anomalous 4K throughput spike was not reproduced, confirming it
was an artifact of host-side scheduling jitter. Consequently, while the
absolute throughput numbers differ due to the cleaner host environment, the
relative CPU cycle savings and syscall reductions remain consistent and
robust.
v5:
- split the former batching patch into a metadata-budget helper (patch 2)
and the batching policy itself (patch 3)
- retain write-space coalescing and deferred data-ready handling as
separate patches (patches 4 and 5)
- document why a connected lookup type mismatch returns to the regular
receive path, which sends the reset
- restore reverse-Christmas-tree declaration order in the batch helpers
- add the requested cap rationale, guest-virtio-only scope explanation,
and cumulative performance results to patch 3
- add cumulative full-series performance results to patch 5 and identify
the P4/P5 result as a combined measurement
- use AF_VSOCK helpers for native callback access and save native
callbacks before socket publication
- serialize callback/protocol/psock eligibility checks with
sk_callback_lock at every notification batch boundary
- make the callback timing dependency explicit: patch 4 retains the
under-lock context and patch 5 moves both notification types after
release_sock()
- share the receive low-watermark predicate between the ordinary and
deferred data-ready paths
- clarify the caller-provided network namespace and locking contract in
the preparatory lookup split
v4:
- use only the connected socket lookup for batch admission and return
misses to the normal receive path
- read packet-specific addresses from the skb in the locked receive
helper and use the matched socket namespace for reset replies
- remove the extra address and network namespace arguments from the
locked receive helper
v3:
- restrict RX batching to sockets using the original protocol and
data-ready/write-space callbacks
- exclude sockets carrying the psock marker published before sockmap
protocol/callback replacement, and recheck it at each packet boundary
- preserve existing sockmap update return values; an attach racing a
native batch makes the next packet end the batch without queued-skb
handoff
- remove a leftover blank line after an opening brace
- remove callback handoff work and attach/detach kicks, preserving the
existing per-packet behavior for sockmap and other non-native sockets
- avoid making this series drain queued or partially consumed skbs when a
socket is added to or removed from a sockmap
- clarify that Patch 1 prepares for batching and intends no functional
change
- inline address decoding and receive tracing, simplify the socket lookup
helper, and pass locked receive arguments directly
- fold same-flow lookup reuse into Patch 2 and match packets against the
locked socket state, reducing the series from five patches to four
v2:
- stop a batch after it consumes the last skb metadata slot, sharing the
admission predicate with virtio_transport_inc_rx_pkt()
- move replacement-callback handoff into vsock, process queued skbs one
at a time, and trigger the handoff from vsock_bpf_update_proto() on
attach and detach
- reduce the packet-context diff in the locked receive helper
- keep RX batch counters in the batch state and reset them in the finish
path
- use the vsock/virtio subject prefix and add the requested measurements
v1: https://lore.kernel.org/virtualization/20261002074551.318789-1-physicalmtea@gmail.com/
Jia Jia (5):
vsock/virtio: split socket lookup from locked RX processing
vsock/virtio: factor RX skb metadata budget check
vsock/virtio: amortize RX socket locking for stream packets
vsock/virtio: coalesce native RX write-space notifications
vsock/virtio: defer native RX readiness notifications until batch
unlock
include/linux/virtio_vsock.h | 26 ++
include/net/af_vsock.h | 11 +
net/vmw_vsock/af_vsock.c | 45 ++-
net/vmw_vsock/virtio_transport.c | 7 +-
net/vmw_vsock/virtio_transport_common.c | 440 +++++++++++++++++++++---
5 files changed, 471 insertions(+), 58 deletions(-)
base-commit: 8df0638138d3e0344fd1fb36cf2d1ca1cf5028f0
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v5 1/5] vsock/virtio: split socket lookup from locked RX processing
2026-11-03 4:11 [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead Jia Jia
@ 2026-11-03 4:11 ` Jia Jia
2026-11-05 9:04 ` Stefano Garzarella
2026-11-03 4:11 ` [PATCH net-next v5 2/5] vsock/virtio: factor RX skb metadata budget check Jia Jia
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Jia Jia @ 2026-11-03 4:11 UTC (permalink / raw)
To: netdev
Cc: stefanha, sgarzare, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
Split virtio_transport_recv_pkt() so socket lookup is separate from
processing that occurs while holding the socket lock. This prepares the
receive path to retain a looked-up socket across multiple packets in a
later change.
Keep virtio_transport_recv_pkt() on its existing per-packet locking path.
Pass the caller-provided network namespace to the locked helper so reset
handling retains its existing semantics. Preserve source validation and
failure handling.
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
net/vmw_vsock/virtio_transport_common.c | 131 ++++++++++++++----------
1 file changed, 78 insertions(+), 53 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index f225f53ed4bab..43325d7aebbb1 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1774,6 +1774,80 @@ static bool virtio_transport_valid_type(u16 type)
(type == VIRTIO_VSOCK_TYPE_SEQPACKET);
}
+static struct sock *
+virtio_transport_find_socket(struct sockaddr_vm *src,
+ struct sockaddr_vm *dst, struct net *net)
+{
+ struct sock *sk;
+
+ sk = vsock_find_connected_socket_net(src, dst, net);
+ if (!sk)
+ sk = vsock_find_bound_socket_net(dst, net);
+
+ return sk;
+}
+
+/*
+ * The caller holds sk's socket lock.
+ */
+static void
+virtio_transport_recv_pkt_locked(struct virtio_transport *t,
+ struct sk_buff *skb, struct sock *sk,
+ struct net *net)
+{
+ struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
+ struct vsock_sock *vsk = vsock_sk(sk);
+ struct sockaddr_vm src;
+ bool space_available;
+
+ vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
+ le32_to_cpu(hdr->src_port));
+
+ /*
+ * The socket may have been closed or reassigned after lookup but before
+ * its socket lock was acquired. Listener sockets have no assigned
+ * transport.
+ */
+ if (sock_flag(sk, SOCK_DONE) ||
+ (sk->sk_state != TCP_LISTEN &&
+ !vsock_check_source(vsk, &t->transport, &src))) {
+ (void)virtio_transport_reset_no_sock(t, skb, net);
+ kfree_skb(skb);
+ return;
+ }
+
+ space_available = virtio_transport_space_update(sk, skb);
+
+ /* Update CID in case it has changed after a transport reset event */
+ if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
+ vsk->local_addr.svm_cid = le64_to_cpu(hdr->dst_cid);
+
+ if (space_available)
+ sk->sk_write_space(sk);
+
+ switch (sk->sk_state) {
+ case TCP_LISTEN:
+ virtio_transport_recv_listen(sk, skb, t);
+ kfree_skb(skb);
+ break;
+ case TCP_SYN_SENT:
+ virtio_transport_recv_connecting(sk, skb);
+ kfree_skb(skb);
+ break;
+ case TCP_ESTABLISHED:
+ virtio_transport_recv_connected(sk, skb);
+ break;
+ case TCP_CLOSING:
+ virtio_transport_recv_disconnecting(sk, skb);
+ kfree_skb(skb);
+ break;
+ default:
+ (void)virtio_transport_reset_no_sock(t, skb, net);
+ kfree_skb(skb);
+ break;
+ }
+}
+
/* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
* lock.
*/
@@ -1782,9 +1856,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
{
struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
struct sockaddr_vm src, dst;
- struct vsock_sock *vsk;
struct sock *sk;
- bool space_available;
vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
le32_to_cpu(hdr->src_port));
@@ -1808,13 +1880,10 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
/* The socket must be in connected or bound table
* otherwise send reset back
*/
- sk = vsock_find_connected_socket_net(&src, &dst, net);
+ sk = virtio_transport_find_socket(&src, &dst, net);
if (!sk) {
- sk = vsock_find_bound_socket_net(&dst, net);
- if (!sk) {
- (void)virtio_transport_reset_no_sock(t, skb, net);
- goto free_pkt;
- }
+ (void)virtio_transport_reset_no_sock(t, skb, net);
+ goto free_pkt;
}
if (virtio_transport_get_type(sk) != le16_to_cpu(hdr->type)) {
@@ -1828,52 +1897,8 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
goto free_pkt;
}
- vsk = vsock_sk(sk);
-
lock_sock(sk);
-
- /* Check if sk has been closed or assigned to another transport before
- * lock_sock (note: listener sockets are not assigned to any transport)
- */
- if (sock_flag(sk, SOCK_DONE) ||
- (sk->sk_state != TCP_LISTEN &&
- !vsock_check_source(vsk, &t->transport, &src))) {
- (void)virtio_transport_reset_no_sock(t, skb, net);
- release_sock(sk);
- sock_put(sk);
- goto free_pkt;
- }
-
- space_available = virtio_transport_space_update(sk, skb);
-
- /* Update CID in case it has changed after a transport reset event */
- if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
- vsk->local_addr.svm_cid = dst.svm_cid;
-
- if (space_available)
- sk->sk_write_space(sk);
-
- switch (sk->sk_state) {
- case TCP_LISTEN:
- virtio_transport_recv_listen(sk, skb, t);
- kfree_skb(skb);
- break;
- case TCP_SYN_SENT:
- virtio_transport_recv_connecting(sk, skb);
- kfree_skb(skb);
- break;
- case TCP_ESTABLISHED:
- virtio_transport_recv_connected(sk, skb);
- break;
- case TCP_CLOSING:
- virtio_transport_recv_disconnecting(sk, skb);
- kfree_skb(skb);
- break;
- default:
- (void)virtio_transport_reset_no_sock(t, skb, net);
- kfree_skb(skb);
- break;
- }
+ virtio_transport_recv_pkt_locked(t, skb, sk, net);
release_sock(sk);
--
2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v5 2/5] vsock/virtio: factor RX skb metadata budget check
2026-11-03 4:11 [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 1/5] vsock/virtio: split socket lookup from locked RX processing Jia Jia
@ 2026-11-03 4:11 ` Jia Jia
2026-11-05 9:06 ` Stefano Garzarella
2026-11-03 4:11 ` [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets Jia Jia
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Jia Jia @ 2026-11-03 4:11 UTC (permalink / raw)
To: netdev
Cc: stefanha, sgarzare, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
Extract the skb metadata budget check from virtio_transport_inc_rx_pkt()
into virtio_transport_rx_buf_full().
The helper retains the existing condition and will be reused by a
following RX batching change.
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
net/vmw_vsock/virtio_transport_common.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 43325d7aebbb1..2a9cfed085515 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -576,11 +576,17 @@ virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs,
skb_queue_splice(&new_queue, &vvs->rx_queue);
}
-static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
- u32 len)
+static bool
+virtio_transport_rx_buf_full(struct virtio_vsock_sock *vvs)
{
u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
+ return skb_overhead > vvs->buf_alloc;
+}
+
+static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
+ u32 len)
+{
/* Allow at most buf_alloc * 2 total budget (payload + overhead),
* similar to how SO_RCVBUF is doubled to reserve space for sk_buff
* metadata. Check payload against buf_alloc to be sure the other
@@ -588,7 +594,7 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
* queue growth.
*/
if ((u64)vvs->buf_used + len > vvs->buf_alloc ||
- skb_overhead > vvs->buf_alloc)
+ virtio_transport_rx_buf_full(vvs))
return false;
vvs->rx_bytes += len;
--
2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets
2026-11-03 4:11 [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 1/5] vsock/virtio: split socket lookup from locked RX processing Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 2/5] vsock/virtio: factor RX skb metadata budget check Jia Jia
@ 2026-11-03 4:11 ` Jia Jia
2026-11-04 19:26 ` Bobby Eshleman
2026-11-05 8:57 ` Stefano Garzarella
2026-11-03 4:11 ` [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 5/5] vsock/virtio: defer native RX readiness notifications until batch unlock Jia Jia
4 siblings, 2 replies; 18+ messages in thread
From: Jia Jia @ 2026-11-03 4:11 UTC (permalink / raw)
To: netdev
Cc: stefanha, sgarzare, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
The virtio-vsock RX worker looks up a socket and takes its lock for every
packet. Retain the connected lookup reference and socket lock across a
bounded run of STREAM/RW packets for one established native socket.
Reuse the locked socket while packets match the same flow. Finish the
batch on a malformed packet, flow change, non-RW traffic, or a change to
the socket state, transport, protocol, sockmap state, or receive buffer
limits. A connected lookup miss falls back to the existing receive path,
which can still find a bound socket.
Limit an uninterrupted socket-lock ownership interval to 64 packets and
64 KiB of payload. The payload limit is one
VIRTIO_VSOCK_MAX_PKT_BUF_SIZE. Small or zero-length packets still incur
per-packet work, so an independent packet limit is needed. The 64-packet
limit uses VHOST_NET_BATCH as a reference; it is a policy choice rather
than a protocol limit.
Beyond these limits, a larger batch would only further amortize socket
lookup and lock/unlock operations, while virtqueue work, validation,
tracing, skb ownership, credit update, and queueing remain per-packet. I
therefore expect diminishing returns while the uninterrupted socket-lock
interval continues to grow, but I have not measured larger limits yet.
Only the guest virtio RX worker is changed. The benchmark exercises the
host-sender-to-guest-receiver direction, where this worker is on the
receive path. I have not measured the corresponding worker and socket-lock
overhead in vhost-vsock or vsock_loopback, and they use different worker
and locking models. Leave those paths unchanged pending separate policy
and validation. I plan to evaluate vhost-vsock separately as follow-up
work.
Performance:
These results cover P1-P3, before the notification changes in P4 and P5.
Tested on a single-stream vsock connection (one Guest receiver thread on
CPU0, virtio-vsock IRQ on Guest CPU1, fresh QEMU Guest per state). The host
uses vhost-vsock.
For throughput, vsock_perf runs on the host as the
sender, with a custom single-threaded Guest receiver pinned to Guest CPU0
using blocking recv(). The fixed-64K table uses a 64K buffer; the matched
table uses the sender's request size. The virtio-vsock IRQ/RX worker is
pinned to Guest CPU1. To normalize run times across payloads into a
bounded, deterministic window and avoid long runs being skewed by host
background noise or virtualization scheduling jitter, transfer sizes scale
proportionally with the request size (64B/64M, 256B/256M, etc.), capped at
2G for 4K-64K. The custom receiver keeps the recv() size and count
explicit; vsock_perf's receiver uses poll()+read().
End-to-end RX throughput (fixed-64K receiver; Gbit/s):
payload baseline RX PATCH RX throughput change (95% CI)
(Gbit/s) (Gbit/s)
64B 0.1285 0.2462 +91.59% [+79.74%, +104.22%]
256B 0.5909 0.9666 +63.59% [+61.03%, +66.19%]
512B 1.1817 1.8350 +55.29% [+46.76%, +64.32%]
1K 2.3252 3.3600 +44.50% [+38.93%, +50.30%]
4K 3.8436 5.5661 +44.82% [+40.30%, +49.47%]
8K 4.6609 5.4115 +16.10% [ +8.91%, +23.77%]
16K 5.1196 5.6399 +10.16% [ +4.07%, +16.61%]
64K 5.6095 5.7062 +1.72% [-3.19%, +6.89%]
Fixed-syscall stress test (receiver recv() size = sender write size;
no perf; Gbit/s):
payload baseline RX PATCH RX throughput change (95% CI)
64B 0.1446 0.2540 +75.71% [+63.67%, +88.62%]
recv() calls (baseline/PATCH): 1048577/1048577
256B 0.4211 0.9061 +115.18% [+72.13%, +169.00%]
recv() calls (baseline/PATCH): 1048577/1048577
512B 0.7980 1.6112 +101.90% [+95.30%, +108.72%]
recv() calls (baseline/PATCH): 1048577/1048577
1K 1.4965 2.6986 +80.33% [+68.93%, +92.49%]
recv() calls (baseline/PATCH): 1048577/1048577
4K 2.9384 4.8590 +65.36% [+60.17%, +70.72%]
recv() calls (baseline/PATCH): 539544/524982
8K 3.8170 5.1887 +35.94% [+27.66%, +44.75%]
recv() calls (baseline/PATCH): 274857/262991
16K 4.6856 5.5996 +19.51% [+15.36%, +23.81%]
recv() calls (baseline/PATCH): 147518/131351
64K 5.5182 5.8800 +6.56% [-0.83%, +14.49%]
recv() calls (baseline/PATCH): 48492/32910
Guest CPU1 aggregate cycles per received byte
(fixed 64K receiver, with perf):
Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
64B 66.728 61.220 -8.25% [-18.99%, +3.91%]
256B 14.363 14.515 +1.06% [ -4.22%, +6.64%]
512B 7.792 7.578 -2.74% [ -6.55%, +1.23%]
1K 3.988 4.032 +1.09% [ -1.64%, +3.88%]
4K 2.063 1.973 -4.38% [ -8.23%, -0.37%]
8K 1.702 1.667 -2.06% [ -7.99%, +4.25%]
16K 1.473 1.475 +0.17% [ -5.06%, +5.68%]
64K 1.385 1.358 -1.96% [-10.61%, +7.53%]
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
include/linux/virtio_vsock.h | 23 ++
net/vmw_vsock/virtio_transport.c | 7 +-
net/vmw_vsock/virtio_transport_common.c | 303 ++++++++++++++++++++++--
3 files changed, 318 insertions(+), 15 deletions(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index f91704731057e..748633aa0cc81 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -282,6 +282,29 @@ void virtio_transport_destruct(struct vsock_sock *vsk);
void virtio_transport_recv_pkt(struct virtio_transport *t,
struct sk_buff *skb, struct net *net);
+
+/*
+ * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
+ * lock and the reference returned by vsock_find_connected_socket_net(). The
+ * caller retains both until it calls virtio_transport_rx_batch_finish(). An
+ * empty batch owns neither.
+ */
+struct virtio_transport_rx_batch {
+ struct sock *sk;
+ unsigned int pkts;
+ size_t bytes;
+};
+
+static inline bool
+virtio_transport_rx_batch_empty(const struct virtio_transport_rx_batch *batch)
+{
+ return !batch->sk;
+}
+
+void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
+ struct sk_buff *skb, struct net *net,
+ struct virtio_transport_rx_batch *batch);
+void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch);
void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 4f9aa9c4c3aa5..9af85712bd387 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -631,6 +631,7 @@ virtio_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
static void virtio_transport_rx_work(struct work_struct *work)
{
struct virtio_vsock *vsock =
container_of(work, struct virtio_vsock, rx_work);
+ struct virtio_transport_rx_batch batch = {};
struct virtqueue *vq;
@@ -666,6 +667,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
/* Drop short/long packets */
if (unlikely(len < sizeof(*hdr) ||
len > virtio_vsock_skb_len(skb))) {
+ virtio_transport_rx_batch_finish(&batch);
kfree_skb(skb);
continue;
}
@@ -673,6 +675,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
hdr = virtio_vsock_hdr(skb);
payload_len = le32_to_cpu(hdr->len);
if (unlikely(payload_len > len - sizeof(*hdr))) {
+ virtio_transport_rx_batch_finish(&batch);
kfree_skb(skb);
continue;
}
@@ -685,11 +688,13 @@ static void virtio_transport_rx_work(struct work_struct *work)
/* Force virtio-transport into global mode since it
* does not yet support local-mode namespacing.
*/
- virtio_transport_recv_pkt(&virtio_transport, skb, NULL);
+ virtio_transport_recv_pkt_batch(&virtio_transport, skb,
+ NULL, &batch);
}
} while (!virtqueue_enable_cb(vq));
out:
+ virtio_transport_rx_batch_finish(&batch);
if (vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2)
virtio_vsock_rx_fill(vsock);
out_nofill:
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 2a9cfed085515..f83e5b2edce35 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -33,6 +33,15 @@
#define MAX_COLLAPSE_LEN \
SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER)
+/*
+ * Bound the interval that an RX worker owns a socket lock. One maximum packet
+ * buffer (64 KiB) bounds payload in one uninterrupted ownership interval.
+ * Small and zero-length packets need a packet limit to bound per-packet work;
+ * the 64-packet policy uses VHOST_NET_BATCH as a reference.
+ */
+#define VIRTIO_TRANSPORT_RX_BATCH_MAX_PKTS 64
+#define VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES VIRTIO_VSOCK_MAX_PKT_BUF_SIZE
+
static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
bool cancel_timeout);
static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
@@ -1793,13 +1802,94 @@ virtio_transport_find_socket(struct sockaddr_vm *src,
return sk;
}
+static void
+virtio_transport_trace_recv_pkt(const struct virtio_vsock_hdr *hdr,
+ const struct sockaddr_vm *src,
+ const struct sockaddr_vm *dst)
+{
+ trace_virtio_transport_recv_pkt(src->svm_cid, src->svm_port,
+ dst->svm_cid, dst->svm_port,
+ le32_to_cpu(hdr->len),
+ le16_to_cpu(hdr->type),
+ le16_to_cpu(hdr->op),
+ le32_to_cpu(hdr->flags),
+ le32_to_cpu(hdr->buf_alloc),
+ le32_to_cpu(hdr->fwd_cnt));
+}
+
+static bool
+virtio_transport_recv_pkt_set_owner(struct sk_buff *skb, struct sock *sk)
+{
+ if (skb_set_owner_sk_safe(skb, sk))
+ return true;
+
+ WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
+ return false;
+}
+
+static bool
+virtio_transport_sock_has_psock(const struct sock *sk)
+{
+ return (uintptr_t)rcu_access_pointer(__sk_user_data(sk)) &
+ SK_USER_DATA_PSOCK;
+}
+
+/* The caller holds sk's socket lock. */
+static bool
+virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
+ struct sock *sk, u32 len)
+{
+ struct virtio_vsock_sock *vvs;
+ struct vsock_sock *vsk;
+
+ vsk = vsock_sk(sk);
+ vvs = vsk->trans;
+
+ /*
+ * Keep a connected lookup reference only for the native, established
+ * STREAM path. Sockets outside ESTABLISHED can change table membership;
+ * a completed or reassigned socket must resume the per-packet path.
+ */
+ if (sk->sk_state != TCP_ESTABLISHED ||
+ sk->sk_type != SOCK_STREAM ||
+ sock_flag(sk, SOCK_DONE) ||
+ vsk->transport != &t->transport || !vvs)
+ return false;
+
+ /*
+ * Do not admit a packet that would hit an existing RX limit while the
+ * worker owns the socket lock. The caller holds that lock, so no reader can
+ * dequeue until the batch is released.
+ */
+ return (u64)vvs->buf_used + len <= vvs->buf_alloc &&
+ !virtio_transport_rx_buf_full(vvs);
+}
+
+/* The caller holds sk's socket lock. */
+static bool
+virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
+ struct sock *sk, u32 len)
+{
+ if (!virtio_transport_rx_batch_can_continue(t, sk, len))
+ return false;
+
+ /*
+ * Sockmap can replace sk_prot and publish a psock without the socket lock.
+ * Check both at each packet boundary; a boundary that sees a replacement
+ * returns to the existing per-packet path.
+ */
+ return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
+ !virtio_transport_sock_has_psock(sk);
+}
+
/*
- * The caller holds sk's socket lock.
+ * The caller holds sk's socket lock. If @batchable is non-NULL, update it
+ * after processing @skb to say whether a native STREAM/RW batch can continue.
*/
static void
virtio_transport_recv_pkt_locked(struct virtio_transport *t,
struct sk_buff *skb, struct sock *sk,
- struct net *net)
+ struct net *net, bool *batchable)
{
struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
struct vsock_sock *vsk = vsock_sk(sk);
@@ -1809,6 +1899,9 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
le32_to_cpu(hdr->src_port));
+ if (batchable)
+ *batchable = false;
+
/*
* The socket may have been closed or reassigned after lookup but before
* its socket lock was acquired. Listener sockets have no assigned
@@ -1852,6 +1945,9 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
kfree_skb(skb);
break;
}
+
+ if (batchable)
+ *batchable = virtio_transport_rx_batch_can_receive(t, sk, 0);
}
/* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
@@ -1869,14 +1965,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
vsock_addr_init(&dst, le64_to_cpu(hdr->dst_cid),
le32_to_cpu(hdr->dst_port));
- trace_virtio_transport_recv_pkt(src.svm_cid, src.svm_port,
- dst.svm_cid, dst.svm_port,
- le32_to_cpu(hdr->len),
- le16_to_cpu(hdr->type),
- le16_to_cpu(hdr->op),
- le32_to_cpu(hdr->flags),
- le32_to_cpu(hdr->buf_alloc),
- le32_to_cpu(hdr->fwd_cnt));
+ virtio_transport_trace_recv_pkt(hdr, &src, &dst);
if (!virtio_transport_valid_type(le16_to_cpu(hdr->type))) {
(void)virtio_transport_reset_no_sock(t, skb, net);
@@ -1898,13 +1987,11 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
goto free_pkt;
}
- if (!skb_set_owner_sk_safe(skb, sk)) {
- WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
+ if (!virtio_transport_recv_pkt_set_owner(skb, sk))
goto free_pkt;
- }
lock_sock(sk);
- virtio_transport_recv_pkt_locked(t, skb, sk, net);
+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
release_sock(sk);
@@ -1919,6 +2006,194 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
}
EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
+/*
+ * Release an active RX batch. A non-empty batch holds the socket lock and the
+ * reference returned by vsock_find_connected_socket_net(). An empty batch
+ * holds neither.
+ */
+void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
+{
+ struct sock *sk = batch->sk;
+
+ batch->sk = NULL;
+ batch->pkts = 0;
+ batch->bytes = 0;
+
+ if (!sk)
+ return;
+
+ release_sock(sk);
+ sock_put(sk);
+}
+EXPORT_SYMBOL_GPL(virtio_transport_rx_batch_finish);
+
+static bool
+virtio_transport_rx_batch_matches(const struct virtio_transport_rx_batch *batch,
+ const struct sockaddr_vm *src,
+ const struct sockaddr_vm *dst,
+ struct net *net)
+{
+ struct vsock_sock *vsk = vsock_sk(batch->sk);
+
+ /* The connected table is keyed by peer address and local port. */
+ return vsock_addr_equals_addr(src, &vsk->remote_addr) &&
+ dst->svm_port == vsk->local_addr.svm_port &&
+ vsock_net_check_mode(sock_net(batch->sk), net);
+}
+
+static bool
+virtio_transport_rx_batch_can_add(const struct virtio_transport_rx_batch *batch,
+ u32 len)
+{
+ if (batch->pkts >= VIRTIO_TRANSPORT_RX_BATCH_MAX_PKTS ||
+ batch->bytes >= VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES)
+ return false;
+
+ return len <= VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES - batch->bytes;
+}
+
+static void
+virtio_transport_rx_batch_account(struct virtio_transport_rx_batch *batch,
+ u32 len)
+{
+ batch->pkts++;
+ batch->bytes += len;
+}
+
+static void
+virtio_transport_rx_batch_process(struct virtio_transport *t,
+ struct sk_buff *skb, struct net *net,
+ struct virtio_transport_rx_batch *batch,
+ const struct sockaddr_vm *src,
+ const struct sockaddr_vm *dst)
+{
+ struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
+ bool batchable;
+ u32 len;
+
+ len = le32_to_cpu(hdr->len);
+ virtio_transport_trace_recv_pkt(hdr, src, dst);
+
+ if (!virtio_transport_recv_pkt_set_owner(skb, batch->sk)) {
+ virtio_transport_rx_batch_finish(batch);
+ kfree_skb(skb);
+ return;
+ }
+
+ virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable);
+ if (!batchable) {
+ virtio_transport_rx_batch_finish(batch);
+ return;
+ }
+
+ virtio_transport_rx_batch_account(batch, len);
+ if (!virtio_transport_rx_batch_can_add(batch, 0))
+ virtio_transport_rx_batch_finish(batch);
+}
+
+void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
+ struct sk_buff *skb, struct net *net,
+ struct virtio_transport_rx_batch *batch)
+{
+ struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
+ struct sockaddr_vm src, dst;
+ struct sock *sk;
+ bool batchable;
+ u32 len;
+
+ /*
+ * Batch only STREAM data packets. Control packets can change connection
+ * state or send a reply. Keep SEQPACKET on its existing path: its EOM/EOR
+ * record-boundary semantics have not been separately evaluated for batching.
+ */
+ if (le16_to_cpu(hdr->type) != VIRTIO_VSOCK_TYPE_STREAM ||
+ le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_RW) {
+ virtio_transport_rx_batch_finish(batch);
+ goto recv_pkt;
+ }
+
+ len = le32_to_cpu(hdr->len);
+ vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
+ le32_to_cpu(hdr->src_port));
+ vsock_addr_init(&dst, le64_to_cpu(hdr->dst_cid),
+ le32_to_cpu(hdr->dst_port));
+
+ if (!virtio_transport_rx_batch_empty(batch)) {
+ /* A post-processing check cannot bound a packet already admitted. */
+ if (!virtio_transport_rx_batch_can_add(batch, len) ||
+ !virtio_transport_rx_batch_matches(batch, &src, &dst, net)) {
+ virtio_transport_rx_batch_finish(batch);
+ } else if (!virtio_transport_rx_batch_can_receive(t, batch->sk,
+ len)) {
+ virtio_transport_rx_batch_finish(batch);
+ goto recv_pkt;
+ }
+ }
+
+ if (!virtio_transport_rx_batch_empty(batch)) {
+ virtio_transport_rx_batch_process(t, skb, net, batch, &src, &dst);
+ return;
+ }
+
+ /*
+ * A packet larger than the byte cap must take the regular path. Starting a
+ * one-packet batch would already exceed the bound on socket-lock ownership.
+ */
+ if (!virtio_transport_rx_batch_can_add(batch, len))
+ goto recv_pkt;
+
+ sk = vsock_find_connected_socket_net(&src, &dst, net);
+ if (!sk) {
+ /*
+ * A connected lookup miss can still match a bound socket in
+ * virtio_transport_recv_pkt().
+ */
+ goto recv_pkt;
+ }
+
+ if (virtio_transport_get_type(sk) != le16_to_cpu(hdr->type)) {
+ /* Let the regular receive path send the type-mismatch reset. */
+ sock_put(sk);
+ goto recv_pkt;
+ }
+
+ lock_sock(sk);
+ /*
+ * Serialize initial admission with sockmap's callback updates. Later
+ * packet boundaries recheck native state and end a batch when they
+ * observe a replacement.
+ */
+ read_lock_bh(&sk->sk_callback_lock);
+ batchable = virtio_transport_rx_batch_can_receive(t, sk, len);
+ read_unlock_bh(&sk->sk_callback_lock);
+ if (!batchable) {
+ /* State or receive-buffer failures keep the regular lookup path. */
+ if (!virtio_transport_rx_batch_can_continue(t, sk, len)) {
+ release_sock(sk);
+ sock_put(sk);
+ goto recv_pkt;
+ }
+
+ virtio_transport_trace_recv_pkt(hdr, &src, &dst);
+ if (virtio_transport_recv_pkt_set_owner(skb, sk))
+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
+ else
+ kfree_skb(skb);
+
+ release_sock(sk);
+ sock_put(sk);
+ return;
+ }
+
+ batch->sk = sk;
+ virtio_transport_rx_batch_process(t, skb, net, batch, &src, &dst);
+ return;
+
+recv_pkt:
+ virtio_transport_recv_pkt(t, skb, net);
+}
+EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt_batch);
+
/* Remove skbs found in a queue that have a vsk that matches.
*
* Each skb is freed.
--
2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications
2026-11-03 4:11 [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead Jia Jia
` (2 preceding siblings ...)
2026-11-03 4:11 ` [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets Jia Jia
@ 2026-11-03 4:11 ` Jia Jia
2026-11-05 9:21 ` Stefano Garzarella
2026-11-03 4:11 ` [PATCH net-next v5 5/5] vsock/virtio: defer native RX readiness notifications until batch unlock Jia Jia
4 siblings, 1 reply; 18+ messages in thread
From: Jia Jia @ 2026-11-03 4:11 UTC (permalink / raw)
To: netdev
Cc: stefanha, sgarzare, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
Each received packet can update peer credit and call sk_write_space(). A
writer cannot consume that credit while the virtio RX worker retains the
socket lock, so repeated native write-space notifications in one RX batch
are unnecessary.
This patch coalesces write-space notifications on the native AF_VSOCK path;
data-ready remains per-packet here.
The batching patch establishes batch ownership before its first packet.
This lets this patch record a pending notification for that packet and
flush it even if the packet immediately ends the batch. Deliver it while
holding the socket lock to preserve the callback context of the existing
receive path. A following patch changes that timing together with
data-ready handling.
Capture the native write-space callback before socket publication and
access it through AF_VSOCK helpers. Sockmap can change the protocol,
write-space callback, and psock marker without the socket lock. Serialize
each batch eligibility check with sk_callback_lock; if a packet boundary
observes a replacement, finish the batch and use the regular per-packet
path.
Packets admitted on the native path use the saved native callback at batch
finish. Reloading a current sockmap callback would not synchronize its
invocation with teardown; a wrapper seen before teardown need not forward
the native wakeup after its psock is gone.
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
include/linux/virtio_vsock.h | 4 +-
include/net/af_vsock.h | 7 +++
net/vmw_vsock/af_vsock.c | 16 +++++
net/vmw_vsock/virtio_transport_common.c | 78 +++++++++++++++++--------
4 files changed, 81 insertions(+), 24 deletions(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 748633aa0cc81..bf28e8e32e280 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -287,12 +287,14 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
* Initialize a batch to zero before use. A non-empty batch owns @sk's socket
* lock and the reference returned by vsock_find_connected_socket_net(). The
* caller retains both until it calls virtio_transport_rx_batch_finish(). An
- * empty batch owns neither.
+ * empty batch owns neither. A pending native write-space notification belongs
+ * to the lock holder and is delivered by virtio_transport_rx_batch_finish().
*/
struct virtio_transport_rx_batch {
struct sock *sk;
unsigned int pkts;
size_t bytes;
+ bool write_space_pending;
};
static inline bool
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 5549298c1ec6b..b76ab565ea58b 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -63,6 +63,11 @@ struct vsock_sock {
u32 peer_shutdown;
bool sent_request;
bool ignore_connecting_rst;
+ /*
+ * Captured before the socket is published. This identifies AF_VSOCK's
+ * native write-space callback after a consumer has replaced it.
+ */
+ void (*native_write_space)(struct sock *sk);
/* Protected by lock_sock(sk) */
u64 buffer_size;
@@ -78,6 +83,8 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk);
s64 vsock_stream_has_space(struct vsock_sock *vsk);
struct sock *vsock_create_connected(struct sock *parent);
void vsock_data_ready(struct sock *sk);
+bool vsock_write_space_is_native(const struct sock *sk);
+void vsock_call_native_write_space(struct sock *sk);
/**** TRANSPORT ****/
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 44cee74519555..e39567990539f 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -958,6 +958,7 @@ static struct sock *__vsock_create(struct net *net,
sk->sk_type = type;
vsk = vsock_sk(sk);
+ vsk->native_write_space = sk->sk_write_space;
vsock_addr_init(&vsk->local_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
vsock_addr_init(&vsk->remote_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
@@ -1122,6 +1123,21 @@ void vsock_data_ready(struct sock *sk)
}
EXPORT_SYMBOL_GPL(vsock_data_ready);
+bool vsock_write_space_is_native(const struct sock *sk)
+{
+ const struct vsock_sock *vsk = vsock_sk(sk);
+
+ return READ_ONCE(sk->sk_write_space) ==
+ READ_ONCE(vsk->native_write_space);
+}
+EXPORT_SYMBOL_GPL(vsock_write_space_is_native);
+
+void vsock_call_native_write_space(struct sock *sk)
+{
+ vsock_sk(sk)->native_write_space(sk);
+}
+EXPORT_SYMBOL_GPL(vsock_call_native_write_space);
+
/* Dummy callback required by sockmap.
* See unconditional call of saved_close() in sock_map_close().
*/
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index f83e5b2edce35..af26344678e41 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1865,31 +1865,55 @@ virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
!virtio_transport_rx_buf_full(vvs);
}
-/* The caller holds sk's socket lock. */
+/* The caller holds sk's socket and callback locks. */
static bool
-virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
- struct sock *sk, u32 len)
+__virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
+ struct sock *sk, u32 len)
{
if (!virtio_transport_rx_batch_can_continue(t, sk, len))
return false;
/*
- * Sockmap can replace sk_prot and publish a psock without the socket lock.
- * Check both at each packet boundary; a boundary that sees a replacement
- * returns to the existing per-packet path.
+ * Sockmap can replace sk_prot or sk_write_space, and publish a psock,
+ * without the socket lock. Coalescing is safe only for the native
+ * write-space callback, so a boundary that sees a replacement returns to
+ * the existing per-packet path.
*/
return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
+ vsock_write_space_is_native(sk) &&
!virtio_transport_sock_has_psock(sk);
}
+/*
+ * Sockmap teardown restores the native protocol and write-space callback
+ * under sk_callback_lock. Serialize each packet admission check with those
+ * updates.
+ */
+static bool
+virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
+ struct sock *sk, u32 len)
+{
+ bool batchable;
+
+ read_lock_bh(&sk->sk_callback_lock);
+ batchable = __virtio_transport_rx_batch_can_receive(t, sk, len);
+ read_unlock_bh(&sk->sk_callback_lock);
+
+ return batchable;
+}
+
/*
* The caller holds sk's socket lock. If @batchable is non-NULL, update it
* after processing @skb to say whether a native STREAM/RW batch can continue.
+ * When @batch is non-NULL, record a native write-space notification for batch
+ * finish. A writer cannot consume the updated credit until this lock is
+ * released.
*/
static void
virtio_transport_recv_pkt_locked(struct virtio_transport *t,
struct sk_buff *skb, struct sock *sk,
- struct net *net, bool *batchable)
+ struct net *net, bool *batchable,
+ struct virtio_transport_rx_batch *batch)
{
struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
struct vsock_sock *vsk = vsock_sk(sk);
@@ -1921,8 +1945,12 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
vsk->local_addr.svm_cid = le64_to_cpu(hdr->dst_cid);
- if (space_available)
- sk->sk_write_space(sk);
+ if (space_available) {
+ if (batch)
+ batch->write_space_pending = true;
+ else
+ sk->sk_write_space(sk);
+ }
switch (sk->sk_state) {
case TCP_LISTEN:
@@ -1947,7 +1975,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
}
if (batchable)
- *batchable = virtio_transport_rx_batch_can_receive(t, sk, 0);
+ *batchable = virtio_transport_rx_batch_can_continue(t, sk, 0);
}
/* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
@@ -1991,7 +2019,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
goto free_pkt;
lock_sock(sk);
- virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL, NULL);
release_sock(sk);
@@ -2008,20 +2036,31 @@ EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
/*
* Release an active RX batch. A non-empty batch holds the socket lock and the
- * reference returned by vsock_find_connected_socket_net(). An empty batch
- * holds neither.
+ * reference returned by vsock_find_connected_socket_net(). Deliver any
+ * deferred native write-space notification while holding the lock, preserving
+ * the callback context of the regular receive path. An empty batch holds
+ * neither.
*/
void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
{
+ bool write_space_pending = batch->write_space_pending;
struct sock *sk = batch->sk;
batch->sk = NULL;
batch->pkts = 0;
batch->bytes = 0;
+ batch->write_space_pending = false;
if (!sk)
return;
+ /*
+ * The packet was admitted with the native callback. Do not reload a current
+ * callback here: sockmap can replace or remove its wrapper independently.
+ */
+ if (write_space_pending)
+ vsock_call_native_write_space(sk);
+
release_sock(sk);
sock_put(sk);
}
@@ -2080,7 +2119,8 @@ virtio_transport_rx_batch_process(struct virtio_transport *t,
return;
}
- virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable);
+ virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable,
+ batch);
if (!batchable) {
virtio_transport_rx_batch_finish(batch);
return;
@@ -2158,16 +2198,8 @@ void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
}
lock_sock(sk);
- /*
- * Serialize initial admission with sockmap's callback updates. Later
- * packet boundaries recheck native state and end a batch when they
- * observe a replacement.
- */
- read_lock_bh(&sk->sk_callback_lock);
batchable = virtio_transport_rx_batch_can_receive(t, sk, len);
- read_unlock_bh(&sk->sk_callback_lock);
if (!batchable) {
- /* State or receive-buffer failures keep the regular lookup path. */
if (!virtio_transport_rx_batch_can_continue(t, sk, len)) {
release_sock(sk);
sock_put(sk);
@@ -2176,7 +2208,7 @@ void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
virtio_transport_trace_recv_pkt(hdr, &src, &dst);
if (virtio_transport_recv_pkt_set_owner(skb, sk))
- virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL, NULL);
else
kfree_skb(skb);
--
2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v5 5/5] vsock/virtio: defer native RX readiness notifications until batch unlock
2026-11-03 4:11 [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead Jia Jia
` (3 preceding siblings ...)
2026-11-03 4:11 ` [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications Jia Jia
@ 2026-11-03 4:11 ` Jia Jia
2026-11-05 9:38 ` Stefano Garzarella
4 siblings, 1 reply; 18+ messages in thread
From: Jia Jia @ 2026-11-03 4:11 UTC (permalink / raw)
To: netdev
Cc: stefanha, sgarzare, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
An RX batch can make a socket readable while the RX worker retains its
socket lock. Waking a blocked reader before release_sock() merely makes it
wait for the same lock.
P4 coalesces native sk_write_space notifications while retaining their
under-lock callback context. This patch also coalesces native sk_data_ready
notifications and moves both callbacks after release_sock(), so awakened
tasks do not immediately block on the RX worker's socket lock.
Save the native data-ready callback before the AF_VSOCK socket is
published. Batch only while the native protocol and both native readiness
callbacks are present and no psock is observed under sk_callback_lock. A
socket with a replacement callback continues down the existing per-packet
path.
At batch finish, deliver write-space before data-ready and hold the
connected lookup reference until both callbacks return. Packets admitted on
the native path continue to use their saved native callbacks. Loading a
current sockmap callback at finish would not serialize its invocation
against sockmap teardown and could lose the native wakeup or invoke a stale
data-ready wrapper.
Extract the data-ready predicate from vsock_data_ready() so the deferred
and regular paths share the low-watermark test.
Performance:
P4-P5 contribution (P1-P3 vs P1-P5):
End-to-end RX throughput (64K Guest recv() size, SO_RCVLOWAT=1; Gbit/s):
payload P1-P3 RX P1-P5 RX throughput change (95% CI)
(Gbit/s) (Gbit/s)
64B 0.2449 0.2463 +0.55% [-3.86%, +5.17%]
256B 0.9649 0.9931 +2.93% [+0.83%, +5.07%]
512B 1.8205 1.8260 +0.30% [-4.27%, +5.10%]
1K 3.3431 3.3420 -0.03% [-2.15%, +2.12%]
4K 5.6541 5.8428 +3.34% [+1.10%, +5.63%]
8K 5.4280 5.3364 -1.69% [-15.55%, +14.46%]
16K 5.5935 5.6701 +1.37% [-1.41%, +4.23%]
64K 5.6917 5.8351 +2.52% [-0.20%, +5.31%]
Fixed-syscall stress test (receiver recv() size = sender write size;
Gbit/s):
payload P1-P3 RX P1-P5 RX throughput change (95% CI)
recv() calls are listed as P1-P3/P1-P5.
64B 0.2553 0.2575 +0.87% [-3.68%, +5.63%]
1048577/1048577
256B 0.9069 0.9183 +1.26% [-3.96%, +6.76%]
1048577/1048577
512B 1.6048 1.6459 +2.56% [+0.98%, +4.18%]
1048577/1048577
1K 2.6939 2.7357 +1.55% [-1.35%, +4.54%]
1048577/1048577
4K 4.9168 4.9678 +1.04% [-1.84%, +4.00%]
524965/525283
8K 5.1476 5.2005 +1.03% [-4.02%, +6.34%]
262926/263156
16K 5.5964 5.6609 +1.15% [-2.86%, +5.33%]
131311/131475
64K 5.9439 5.9073 -0.62% [-3.00%, +1.83%]
32875/32937
Guest CPU1 aggregate cycles per received byte
(fixed 64K receiver, with perf):
payload P1-P3 P1-P5 change (95% CI)
64B 60.281 58.258 -3.36% [-8.74%, +2.35%]
256B 14.242 13.648 -4.17% [-9.81%, +1.82%]
512B 7.547 7.211 -4.45% [-9.73%, +1.13%]
1K 3.987 3.853 -3.35% [-8.52%, +2.12%]
4K 1.983 1.880 -5.17% [-7.97%, -2.27%]
8K 1.665 1.620 -2.75% [-9.03%, +3.96%]
16K 1.409 1.346 -4.52% [-10.94%, +2.37%]
64K 1.322 1.281 -3.11% [-6.49%, +0.38%]
Full-series cycles/B (baseline vs P1-P5):
Guest CPU1 aggregate cycles per received byte
(fixed 64K receiver, with perf):
payload baseline P1-P5 change (95% CI)
64B 65.282 57.843 -11.40% [-13.77%, -8.95%]
256B 14.710 13.813 -6.10% [ -7.89%, -4.28%]
512B 7.709 7.188 -6.75% [ -8.79%, -4.66%]
1K 4.075 3.862 -5.23% [ -6.80%, -3.63%]
4K 2.034 1.887 -7.21% [ -8.67%, -5.72%]
8K 1.664 1.568 -5.78% [ -7.83%, -3.69%]
16K 1.450 1.385 -4.48% [ -6.08%, -2.86%]
64K 1.338 1.317 -1.51% [ -3.21%, +0.22%]
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
include/linux/virtio_vsock.h | 5 ++-
include/net/af_vsock.h | 9 +++--
net/vmw_vsock/af_vsock.c | 27 ++++++++++---
net/vmw_vsock/virtio_transport_common.c | 52 +++++++++++++++----------
4 files changed, 62 insertions(+), 31 deletions(-)
Tested:
Concurrent map-entry attach/detach during sequenced RX and duplex traffic
passed on baseline, P1-P3, P1-P4, and P1-P5. The active SK_PASS case also
passed on each state, with 200,000 verdict invocations. The concurrent
SK_PASS attach/detach case completed 1,000 insert/remove pairs per state;
its 10,000-record suffix after the final detach was verified. No kernel
diagnostic markers were found in successful runs.
Sockmap may replace sk_data_ready and sk_write_space. With non-native
callbacks, native RX batching must be declined and the ordinary per-packet
receive path used. The active SK_PASS case covers the condition expected to
trigger this fallback. The test does not trace each packet's internal
branch, so it verifies the fallback scenario without asserting that every
packet took that branch.
The duplex cases exercised write-space backpressure: the guest's nonblocking
send reached EAGAIN while the host was not reading. After the host began
draining data, the guest observed POLLOUT and resumed sending. Counts below
are observed EAGAIN/POLLOUT recoveries.
Test / result baseline P1-P3 P1-P4 P1-P5
No-verdict RX churn: attach/detach pairs 433493 455507 430985 380351
No-verdict duplex: attach/detach pairs 229793 212410 214198 148755
No-verdict duplex: EAGAIN/POLLOUT 365/365 266/266 264/264 443/443
Active SK_PASS: EAGAIN/POLLOUT 2548/2548 3013/3013 3093/3093 4208/4208
SK_PASS attach/detach: intervals with verdict progress 705 616 631 692
SK_PASS attach/detach: verdict invocations 99747 99894 97475 82409
Post-detach SK_PASS suffix 10,000/10,000 records per run
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index bf28e8e32e280..2aa0e81e655d5 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -287,14 +287,15 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
* Initialize a batch to zero before use. A non-empty batch owns @sk's socket
* lock and the reference returned by vsock_find_connected_socket_net(). The
* caller retains both until it calls virtio_transport_rx_batch_finish(). An
- * empty batch owns neither. A pending native write-space notification belongs
- * to the lock holder and is delivered by virtio_transport_rx_batch_finish().
+ * empty batch owns neither. Pending native readiness notifications belong to
+ * the lock holder and are delivered by virtio_transport_rx_batch_finish().
*/
struct virtio_transport_rx_batch {
struct sock *sk;
unsigned int pkts;
size_t bytes;
bool write_space_pending;
+ bool data_ready_pending;
};
static inline bool
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index b76ab565ea58b..57ee545fdb496 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -64,9 +64,10 @@ struct vsock_sock {
bool sent_request;
bool ignore_connecting_rst;
/*
- * Captured before the socket is published. This identifies AF_VSOCK's
- * native write-space callback after a consumer has replaced it.
+ * Captured before the socket is published. These identify AF_VSOCK's
+ * native readiness callbacks after a consumer has replaced them.
*/
+ void (*native_data_ready)(struct sock *sk);
void (*native_write_space)(struct sock *sk);
/* Protected by lock_sock(sk) */
@@ -83,7 +84,9 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk);
s64 vsock_stream_has_space(struct vsock_sock *vsk);
struct sock *vsock_create_connected(struct sock *parent);
void vsock_data_ready(struct sock *sk);
-bool vsock_write_space_is_native(const struct sock *sk);
+bool vsock_data_ready_pending(struct sock *sk);
+bool vsock_callbacks_are_native(const struct sock *sk);
+void vsock_call_native_data_ready(struct sock *sk);
void vsock_call_native_write_space(struct sock *sk);
/**** TRANSPORT ****/
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index e39567990539f..0617073e99ed8 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -958,6 +958,7 @@ static struct sock *__vsock_create(struct net *net,
sk->sk_type = type;
vsk = vsock_sk(sk);
+ vsk->native_data_ready = sk->sk_data_ready;
vsk->native_write_space = sk->sk_write_space;
vsock_addr_init(&vsk->local_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
vsock_addr_init(&vsk->remote_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
@@ -1113,24 +1114,38 @@ s64 vsock_stream_has_space(struct vsock_sock *vsk)
}
EXPORT_SYMBOL_GPL(vsock_stream_has_space);
-void vsock_data_ready(struct sock *sk)
+bool vsock_data_ready_pending(struct sock *sk)
{
struct vsock_sock *vsk = vsock_sk(sk);
- if (vsock_stream_has_data(vsk) >= sk->sk_rcvlowat ||
- sock_flag(sk, SOCK_DONE))
+ return vsock_stream_has_data(vsk) >= sk->sk_rcvlowat ||
+ sock_flag(sk, SOCK_DONE);
+}
+EXPORT_SYMBOL_GPL(vsock_data_ready_pending);
+
+void vsock_data_ready(struct sock *sk)
+{
+ if (vsock_data_ready_pending(sk))
sk->sk_data_ready(sk);
}
EXPORT_SYMBOL_GPL(vsock_data_ready);
-bool vsock_write_space_is_native(const struct sock *sk)
+bool vsock_callbacks_are_native(const struct sock *sk)
{
const struct vsock_sock *vsk = vsock_sk(sk);
- return READ_ONCE(sk->sk_write_space) ==
+ return READ_ONCE(sk->sk_data_ready) ==
+ READ_ONCE(vsk->native_data_ready) &&
+ READ_ONCE(sk->sk_write_space) ==
READ_ONCE(vsk->native_write_space);
}
-EXPORT_SYMBOL_GPL(vsock_write_space_is_native);
+EXPORT_SYMBOL_GPL(vsock_callbacks_are_native);
+
+void vsock_call_native_data_ready(struct sock *sk)
+{
+ vsock_sk(sk)->native_data_ready(sk);
+}
+EXPORT_SYMBOL_GPL(vsock_call_native_data_ready);
void vsock_call_native_write_space(struct sock *sk)
{
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index af26344678e41..9d02e81042c21 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1598,7 +1598,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
static int
virtio_transport_recv_connected(struct sock *sk,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ bool *data_ready_pending)
{
struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
struct vsock_sock *vsk = vsock_sk(sk);
@@ -1617,7 +1618,12 @@ virtio_transport_recv_connected(struct sock *sk,
vsock_remove_sock(vsk);
break;
}
- vsock_data_ready(sk);
+ if (data_ready_pending) {
+ if (!*data_ready_pending && vsock_data_ready_pending(sk))
+ *data_ready_pending = true;
+ } else {
+ vsock_data_ready(sk);
+ }
return err;
case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
virtio_transport_send_credit_update(vsk);
@@ -1874,20 +1880,19 @@ __virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
return false;
/*
- * Sockmap can replace sk_prot or sk_write_space, and publish a psock,
- * without the socket lock. Coalescing is safe only for the native
- * write-space callback, so a boundary that sees a replacement returns to
- * the existing per-packet path.
+ * Sockmap can replace sk_prot or either readiness callback, and publish a
+ * psock, without the socket lock. Coalescing is safe only for native
+ * callbacks, so a boundary that sees a replacement returns to the existing
+ * per-packet path.
*/
return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
- vsock_write_space_is_native(sk) &&
+ vsock_callbacks_are_native(sk) &&
!virtio_transport_sock_has_psock(sk);
}
/*
- * Sockmap teardown restores the native protocol and write-space callback
- * under sk_callback_lock. Serialize each packet admission check with those
- * updates.
+ * Sockmap teardown restores the native protocol and callbacks under
+ * sk_callback_lock. Serialize each packet admission check with those updates.
*/
static bool
virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
@@ -1905,9 +1910,8 @@ virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
/*
* The caller holds sk's socket lock. If @batchable is non-NULL, update it
* after processing @skb to say whether a native STREAM/RW batch can continue.
- * When @batch is non-NULL, record a native write-space notification for batch
- * finish. A writer cannot consume the updated credit until this lock is
- * released.
+ * When @batch is non-NULL, record native readiness notifications for batch
+ * finish. A reader or writer cannot make progress until this lock is released.
*/
static void
virtio_transport_recv_pkt_locked(struct virtio_transport *t,
@@ -1917,6 +1921,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
{
struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
struct vsock_sock *vsk = vsock_sk(sk);
+ bool *data_ready_pending;
struct sockaddr_vm src;
bool space_available;
@@ -1926,6 +1931,8 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
if (batchable)
*batchable = false;
+ data_ready_pending = batch ? &batch->data_ready_pending : NULL;
+
/*
* The socket may have been closed or reassigned after lookup but before
* its socket lock was acquired. Listener sockets have no assigned
@@ -1962,7 +1969,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
kfree_skb(skb);
break;
case TCP_ESTABLISHED:
- virtio_transport_recv_connected(sk, skb);
+ virtio_transport_recv_connected(sk, skb, data_ready_pending);
break;
case TCP_CLOSING:
virtio_transport_recv_disconnecting(sk, skb);
@@ -2036,32 +2043,37 @@ EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
/*
* Release an active RX batch. A non-empty batch holds the socket lock and the
- * reference returned by vsock_find_connected_socket_net(). Deliver any
- * deferred native write-space notification while holding the lock, preserving
- * the callback context of the regular receive path. An empty batch holds
- * neither.
+ * reference returned by vsock_find_connected_socket_net(). Release the lock,
+ * deliver deferred native write-space and data-ready notifications in that
+ * order, then drop the reference. An empty batch holds neither.
*/
void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
{
bool write_space_pending = batch->write_space_pending;
+ bool data_ready_pending = batch->data_ready_pending;
struct sock *sk = batch->sk;
batch->sk = NULL;
batch->pkts = 0;
batch->bytes = 0;
batch->write_space_pending = false;
+ batch->data_ready_pending = false;
if (!sk)
return;
+ release_sock(sk);
+
/*
- * The packet was admitted with the native callback. Do not reload a current
+ * Packets were admitted with native callbacks. Do not reload a current
* callback here: sockmap can replace or remove its wrapper independently.
*/
if (write_space_pending)
vsock_call_native_write_space(sk);
- release_sock(sk);
+ if (data_ready_pending)
+ vsock_call_native_data_ready(sk);
+
sock_put(sk);
}
EXPORT_SYMBOL_GPL(virtio_transport_rx_batch_finish);
--
2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets
2026-11-03 4:11 ` [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets Jia Jia
@ 2026-11-04 19:26 ` Bobby Eshleman
2026-11-05 8:16 ` Jia Jia
2026-11-05 8:57 ` Stefano Garzarella
1 sibling, 1 reply; 18+ messages in thread
From: Bobby Eshleman @ 2026-11-04 19:26 UTC (permalink / raw)
To: Jia Jia
Cc: netdev, stefanha, sgarzare, mst, jasowangio, eperezma, xuanzhuo,
davem, edumazet, kuba, pabeni, horms, kvm, virtualization,
linux-kernel
On Tue, Nov 03, 2026 at 12:11:32PM +0800, Jia Jia wrote:
> The virtio-vsock RX worker looks up a socket and takes its lock for every
> packet. Retain the connected lookup reference and socket lock across a
> bounded run of STREAM/RW packets for one established native socket.
>
> Reuse the locked socket while packets match the same flow. Finish the
> batch on a malformed packet, flow change, non-RW traffic, or a change to
> the socket state, transport, protocol, sockmap state, or receive buffer
> limits. A connected lookup miss falls back to the existing receive path,
> which can still find a bound socket.
>
> Limit an uninterrupted socket-lock ownership interval to 64 packets and
> 64 KiB of payload. The payload limit is one
> VIRTIO_VSOCK_MAX_PKT_BUF_SIZE. Small or zero-length packets still incur
> per-packet work, so an independent packet limit is needed. The 64-packet
> limit uses VHOST_NET_BATCH as a reference; it is a policy choice rather
> than a protocol limit.
>
> Beyond these limits, a larger batch would only further amortize socket
> lookup and lock/unlock operations, while virtqueue work, validation,
> tracing, skb ownership, credit update, and queueing remain per-packet. I
> therefore expect diminishing returns while the uninterrupted socket-lock
> interval continues to grow, but I have not measured larger limits yet.
>
> Only the guest virtio RX worker is changed. The benchmark exercises the
> host-sender-to-guest-receiver direction, where this worker is on the
> receive path. I have not measured the corresponding worker and socket-lock
> overhead in vhost-vsock or vsock_loopback, and they use different worker
> and locking models. Leave those paths unchanged pending separate policy
> and validation. I plan to evaluate vhost-vsock separately as follow-up
> work.
>
> Performance:
>
> These results cover P1-P3, before the notification changes in P4 and P5.
>
> Tested on a single-stream vsock connection (one Guest receiver thread on
> CPU0, virtio-vsock IRQ on Guest CPU1, fresh QEMU Guest per state). The host
> uses vhost-vsock.
>
> For throughput, vsock_perf runs on the host as the
> sender, with a custom single-threaded Guest receiver pinned to Guest CPU0
> using blocking recv(). The fixed-64K table uses a 64K buffer; the matched
> table uses the sender's request size. The virtio-vsock IRQ/RX worker is
> pinned to Guest CPU1. To normalize run times across payloads into a
> bounded, deterministic window and avoid long runs being skewed by host
> background noise or virtualization scheduling jitter, transfer sizes scale
> proportionally with the request size (64B/64M, 256B/256M, etc.), capped at
> 2G for 4K-64K. The custom receiver keeps the recv() size and count
> explicit; vsock_perf's receiver uses poll()+read().
>
> End-to-end RX throughput (fixed-64K receiver; Gbit/s):
>
> payload baseline RX PATCH RX throughput change (95% CI)
> (Gbit/s) (Gbit/s)
> 64B 0.1285 0.2462 +91.59% [+79.74%, +104.22%]
> 256B 0.5909 0.9666 +63.59% [+61.03%, +66.19%]
> 512B 1.1817 1.8350 +55.29% [+46.76%, +64.32%]
> 1K 2.3252 3.3600 +44.50% [+38.93%, +50.30%]
> 4K 3.8436 5.5661 +44.82% [+40.30%, +49.47%]
> 8K 4.6609 5.4115 +16.10% [ +8.91%, +23.77%]
> 16K 5.1196 5.6399 +10.16% [ +4.07%, +16.61%]
> 64K 5.6095 5.7062 +1.72% [-3.19%, +6.89%]
>
> Fixed-syscall stress test (receiver recv() size = sender write size;
> no perf; Gbit/s):
>
> payload baseline RX PATCH RX throughput change (95% CI)
> 64B 0.1446 0.2540 +75.71% [+63.67%, +88.62%]
> recv() calls (baseline/PATCH): 1048577/1048577
> 256B 0.4211 0.9061 +115.18% [+72.13%, +169.00%]
> recv() calls (baseline/PATCH): 1048577/1048577
> 512B 0.7980 1.6112 +101.90% [+95.30%, +108.72%]
> recv() calls (baseline/PATCH): 1048577/1048577
> 1K 1.4965 2.6986 +80.33% [+68.93%, +92.49%]
> recv() calls (baseline/PATCH): 1048577/1048577
> 4K 2.9384 4.8590 +65.36% [+60.17%, +70.72%]
> recv() calls (baseline/PATCH): 539544/524982
> 8K 3.8170 5.1887 +35.94% [+27.66%, +44.75%]
> recv() calls (baseline/PATCH): 274857/262991
> 16K 4.6856 5.5996 +19.51% [+15.36%, +23.81%]
> recv() calls (baseline/PATCH): 147518/131351
> 64K 5.5182 5.8800 +6.56% [-0.83%, +14.49%]
> recv() calls (baseline/PATCH): 48492/32910
>
> Guest CPU1 aggregate cycles per received byte
> (fixed 64K receiver, with perf):
>
> Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
> 64B 66.728 61.220 -8.25% [-18.99%, +3.91%]
> 256B 14.363 14.515 +1.06% [ -4.22%, +6.64%]
> 512B 7.792 7.578 -2.74% [ -6.55%, +1.23%]
> 1K 3.988 4.032 +1.09% [ -1.64%, +3.88%]
> 4K 2.063 1.973 -4.38% [ -8.23%, -0.37%]
> 8K 1.702 1.667 -2.06% [ -7.99%, +4.25%]
> 16K 1.473 1.475 +0.17% [ -5.06%, +5.68%]
> 64K 1.385 1.358 -1.96% [-10.61%, +7.53%]
>
> Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> ---
> include/linux/virtio_vsock.h | 23 ++
> net/vmw_vsock/virtio_transport.c | 7 +-
> net/vmw_vsock/virtio_transport_common.c | 303 ++++++++++++++++++++++--
> 3 files changed, 318 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index f91704731057e..748633aa0cc81 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -282,6 +282,29 @@ void virtio_transport_destruct(struct vsock_sock *vsk);
>
> void virtio_transport_recv_pkt(struct virtio_transport *t,
> struct sk_buff *skb, struct net *net);
> +
> +/*
> + * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
> + * lock and the reference returned by vsock_find_connected_socket_net(). The
> + * caller retains both until it calls virtio_transport_rx_batch_finish(). An
> + * empty batch owns neither.
> + */
> +struct virtio_transport_rx_batch {
> + struct sock *sk;
> + unsigned int pkts;
> + size_t bytes;
> +};
> +
> +static inline bool
> +virtio_transport_rx_batch_empty(const struct virtio_transport_rx_batch *batch)
> +{
> + return !batch->sk;
> +}
> +
> +void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
> + struct sk_buff *skb, struct net *net,
> + struct virtio_transport_rx_batch *batch);
> +void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch);
> void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
> u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 4f9aa9c4c3aa5..9af85712bd387 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -631,6 +631,7 @@ virtio_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
>
> static void virtio_transport_rx_work(struct work_struct *work)
> {
> struct virtio_vsock *vsock =
> container_of(work, struct virtio_vsock, rx_work);
> + struct virtio_transport_rx_batch batch = {};
> struct virtqueue *vq;
> @@ -666,6 +667,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
> /* Drop short/long packets */
> if (unlikely(len < sizeof(*hdr) ||
> len > virtio_vsock_skb_len(skb))) {
> + virtio_transport_rx_batch_finish(&batch);
> kfree_skb(skb);
> continue;
> }
> @@ -673,6 +675,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
> hdr = virtio_vsock_hdr(skb);
> payload_len = le32_to_cpu(hdr->len);
> if (unlikely(payload_len > len - sizeof(*hdr))) {
> + virtio_transport_rx_batch_finish(&batch);
> kfree_skb(skb);
> continue;
> }
> @@ -685,11 +688,13 @@ static void virtio_transport_rx_work(struct work_struct *work)
> /* Force virtio-transport into global mode since it
> * does not yet support local-mode namespacing.
> */
> - virtio_transport_recv_pkt(&virtio_transport, skb, NULL);
> + virtio_transport_recv_pkt_batch(&virtio_transport, skb,
> + NULL, &batch);
> }
> } while (!virtqueue_enable_cb(vq));
>
> out:
> + virtio_transport_rx_batch_finish(&batch);
> if (vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2)
> virtio_vsock_rx_fill(vsock);
> out_nofill:
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 2a9cfed085515..f83e5b2edce35 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -33,6 +33,15 @@
> #define MAX_COLLAPSE_LEN \
> SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER)
>
> +/*
> + * Bound the interval that an RX worker owns a socket lock. One maximum packet
> + * buffer (64 KiB) bounds payload in one uninterrupted ownership interval.
> + * Small and zero-length packets need a packet limit to bound per-packet work;
> + * the 64-packet policy uses VHOST_NET_BATCH as a reference.
> + */
> +#define VIRTIO_TRANSPORT_RX_BATCH_MAX_PKTS 64
> +#define VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES VIRTIO_VSOCK_MAX_PKT_BUF_SIZE
> +
> static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
> bool cancel_timeout);
> static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
> @@ -1793,13 +1802,94 @@ virtio_transport_find_socket(struct sockaddr_vm *src,
> return sk;
> }
>
> +static void
> +virtio_transport_trace_recv_pkt(const struct virtio_vsock_hdr *hdr,
> + const struct sockaddr_vm *src,
> + const struct sockaddr_vm *dst)
> +{
> + trace_virtio_transport_recv_pkt(src->svm_cid, src->svm_port,
> + dst->svm_cid, dst->svm_port,
> + le32_to_cpu(hdr->len),
> + le16_to_cpu(hdr->type),
> + le16_to_cpu(hdr->op),
> + le32_to_cpu(hdr->flags),
> + le32_to_cpu(hdr->buf_alloc),
> + le32_to_cpu(hdr->fwd_cnt));
> +}
I'd rather see trace_virtio_transport_recv_pkt() refactored to take hdr,
src, and dst instead of wrapping it. I'd guess the trace definition
field assignment block can handle the dereferencing and leXY_to_cpu()
calls.
> +
> +static bool
> +virtio_transport_recv_pkt_set_owner(struct sk_buff *skb, struct sock *sk)
> +{
> + if (skb_set_owner_sk_safe(skb, sk))
> + return true;
> +
> + WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
> + return false;
> +}
> +
> +static bool
> +virtio_transport_sock_has_psock(const struct sock *sk)
> +{
> + return (uintptr_t)rcu_access_pointer(__sk_user_data(sk)) &
> + SK_USER_DATA_PSOCK;
> +}
> +
> +/* The caller holds sk's socket lock. */
> +static bool
> +virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
> + struct sock *sk, u32 len)
> +{
> + struct virtio_vsock_sock *vvs;
> + struct vsock_sock *vsk;
> +
> + vsk = vsock_sk(sk);
> + vvs = vsk->trans;
> +
> + /*
> + * Keep a connected lookup reference only for the native, established
> + * STREAM path. Sockets outside ESTABLISHED can change table membership;
> + * a completed or reassigned socket must resume the per-packet path.
> + */
> + if (sk->sk_state != TCP_ESTABLISHED ||
> + sk->sk_type != SOCK_STREAM ||
> + sock_flag(sk, SOCK_DONE) ||
> + vsk->transport != &t->transport || !vvs)
> + return false;
> +
> + /*
> + * Do not admit a packet that would hit an existing RX limit while the
> + * worker owns the socket lock. The caller holds that lock, so no reader can
> + * dequeue until the batch is released.
> + */
> + return (u64)vvs->buf_used + len <= vvs->buf_alloc &&
> + !virtio_transport_rx_buf_full(vvs);
> +}
> +
> +/* The caller holds sk's socket lock. */
> +static bool
> +virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> + struct sock *sk, u32 len)
> +{
> + if (!virtio_transport_rx_batch_can_continue(t, sk, len))
> + return false;
> +
> + /*
> + * Sockmap can replace sk_prot and publish a psock without the socket lock.
> + * Check both at each packet boundary; a boundary that sees a replacement
> + * returns to the existing per-packet path.
> + */
> + return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
> + !virtio_transport_sock_has_psock(sk);
Can you elaborate on the problem caused by this changing between packet
boundaries? Sorry its been a while since I refreshed my memory on the
sockmap stuff.
> +}
> +
> /*
> - * The caller holds sk's socket lock.
> + * The caller holds sk's socket lock. If @batchable is non-NULL, update it
> + * after processing @skb to say whether a native STREAM/RW batch can continue.
> */
> static void
> virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> struct sk_buff *skb, struct sock *sk,
> - struct net *net)
> + struct net *net, bool *batchable)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct vsock_sock *vsk = vsock_sk(sk);
> @@ -1809,6 +1899,9 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
> le32_to_cpu(hdr->src_port));
>
> + if (batchable)
> + *batchable = false;
> +
> /*
> * The socket may have been closed or reassigned after lookup but before
> * its socket lock was acquired. Listener sockets have no assigned
> @@ -1852,6 +1945,9 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> kfree_skb(skb);
> break;
> }
> +
> + if (batchable)
> + *batchable = virtio_transport_rx_batch_can_receive(t, sk, 0);
> }
>
> /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
> @@ -1869,14 +1965,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> vsock_addr_init(&dst, le64_to_cpu(hdr->dst_cid),
> le32_to_cpu(hdr->dst_port));
>
> - trace_virtio_transport_recv_pkt(src.svm_cid, src.svm_port,
> - dst.svm_cid, dst.svm_port,
> - le32_to_cpu(hdr->len),
> - le16_to_cpu(hdr->type),
> - le16_to_cpu(hdr->op),
> - le32_to_cpu(hdr->flags),
> - le32_to_cpu(hdr->buf_alloc),
> - le32_to_cpu(hdr->fwd_cnt));
> + virtio_transport_trace_recv_pkt(hdr, &src, &dst);
>
> if (!virtio_transport_valid_type(le16_to_cpu(hdr->type))) {
> (void)virtio_transport_reset_no_sock(t, skb, net);
> @@ -1898,13 +1987,11 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> goto free_pkt;
> }
>
> - if (!skb_set_owner_sk_safe(skb, sk)) {
> - WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
> + if (!virtio_transport_recv_pkt_set_owner(skb, sk))
> goto free_pkt;
> - }
>
> lock_sock(sk);
> - virtio_transport_recv_pkt_locked(t, skb, sk, net);
> + virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
>
> release_sock(sk);
>
> @@ -1919,6 +2006,194 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> }
> EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
>
> +/*
> + * Release an active RX batch. A non-empty batch holds the socket lock and the
> + * reference returned by vsock_find_connected_socket_net(). An empty batch
> + * holds neither.
> + */
> +void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
> +{
> + struct sock *sk = batch->sk;
> +
> + batch->sk = NULL;
> + batch->pkts = 0;
> + batch->bytes = 0;
> +
> + if (!sk)
> + return;
> +
> + release_sock(sk);
> + sock_put(sk);
> +}
> +EXPORT_SYMBOL_GPL(virtio_transport_rx_batch_finish);
> +
> +static bool
> +virtio_transport_rx_batch_matches(const struct virtio_transport_rx_batch *batch,
> + const struct sockaddr_vm *src,
> + const struct sockaddr_vm *dst,
> + struct net *net)
> +{
> + struct vsock_sock *vsk = vsock_sk(batch->sk);
> +
> + /* The connected table is keyed by peer address and local port. */
> + return vsock_addr_equals_addr(src, &vsk->remote_addr) &&
> + dst->svm_port == vsk->local_addr.svm_port &&
> + vsock_net_check_mode(sock_net(batch->sk), net);
> +}
> +
> +static bool
> +virtio_transport_rx_batch_can_add(const struct virtio_transport_rx_batch *batch,
> + u32 len)
> +{
> + if (batch->pkts >= VIRTIO_TRANSPORT_RX_BATCH_MAX_PKTS ||
> + batch->bytes >= VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES)
> + return false;
> +
> + return len <= VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES - batch->bytes;
> +}
> +
> +static void
> +virtio_transport_rx_batch_account(struct virtio_transport_rx_batch *batch,
> + u32 len)
> +{
> + batch->pkts++;
> + batch->bytes += len;
> +}
> +
> +static void
> +virtio_transport_rx_batch_process(struct virtio_transport *t,
> + struct sk_buff *skb, struct net *net,
> + struct virtio_transport_rx_batch *batch,
> + const struct sockaddr_vm *src,
> + const struct sockaddr_vm *dst)
> +{
> + struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> + bool batchable;
> + u32 len;
> +
> + len = le32_to_cpu(hdr->len);
> + virtio_transport_trace_recv_pkt(hdr, src, dst);
> +
> + if (!virtio_transport_recv_pkt_set_owner(skb, batch->sk)) {
> + virtio_transport_rx_batch_finish(batch);
> + kfree_skb(skb);
> + return;
> + }
> +
> + virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable);
> + if (!batchable) {
> + virtio_transport_rx_batch_finish(batch);
> + return;
> + }
> +
> + virtio_transport_rx_batch_account(batch, len);
> + if (!virtio_transport_rx_batch_can_add(batch, 0))
> + virtio_transport_rx_batch_finish(batch);
> +}
> +
> +void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
> + struct sk_buff *skb, struct net *net,
> + struct virtio_transport_rx_batch *batch)
> +{
> + struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> + struct sockaddr_vm src, dst;
> + struct sock *sk;
> + bool batchable;
> + u32 len;
> +
> + /*
> + * Batch only STREAM data packets. Control packets can change connection
> + * state or send a reply. Keep SEQPACKET on its existing path: its EOM/EOR
> + * record-boundary semantics have not been separately evaluated for batching.
> + */
> + if (le16_to_cpu(hdr->type) != VIRTIO_VSOCK_TYPE_STREAM ||
> + le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_RW) {
> + virtio_transport_rx_batch_finish(batch);
> + goto recv_pkt;
> + }
> +
> + len = le32_to_cpu(hdr->len);
> + vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
> + le32_to_cpu(hdr->src_port));
> + vsock_addr_init(&dst, le64_to_cpu(hdr->dst_cid),
> + le32_to_cpu(hdr->dst_port));
> +
> + if (!virtio_transport_rx_batch_empty(batch)) {
> + /* A post-processing check cannot bound a packet already admitted. */
> + if (!virtio_transport_rx_batch_can_add(batch, len) ||
> + !virtio_transport_rx_batch_matches(batch, &src, &dst, net)) {
> + virtio_transport_rx_batch_finish(batch);
> + } else if (!virtio_transport_rx_batch_can_receive(t, batch->sk,
> + len)) {
> + virtio_transport_rx_batch_finish(batch);
> + goto recv_pkt;
> + }
> + }
> +
> + if (!virtio_transport_rx_batch_empty(batch)) {
> + virtio_transport_rx_batch_process(t, skb, net, batch, &src, &dst);
> + return;
> + }
> +
> + /*
> + * A packet larger than the byte cap must take the regular path. Starting a
> + * one-packet batch would already exceed the bound on socket-lock ownership.
> + */
> + if (!virtio_transport_rx_batch_can_add(batch, len))
> + goto recv_pkt;
> +
> + sk = vsock_find_connected_socket_net(&src, &dst, net);
> + if (!sk) {
> + /*
> + * A connected lookup miss can still match a bound socket in
> + * virtio_transport_recv_pkt().
> + */
> + goto recv_pkt;
> + }
> +
> + if (virtio_transport_get_type(sk) != le16_to_cpu(hdr->type)) {
> + /* Let the regular receive path send the type-mismatch reset. */
> + sock_put(sk);
> + goto recv_pkt;
> + }
> +
> + lock_sock(sk);
> + /*
> + * Serialize initial admission with sockmap's callback updates. Later
> + * packet boundaries recheck native state and end a batch when they
> + * observe a replacement.
> + */
> + read_lock_bh(&sk->sk_callback_lock);
> + batchable = virtio_transport_rx_batch_can_receive(t, sk, len);
> + read_unlock_bh(&sk->sk_callback_lock);
Why do we need sk_callback_lock? what happens if we don't take it? The
comment suggests we're synchronizing with sockmap callback updates, but
we don't hold the lock for all of batch processing, so the callback can
still be updated later anyway (and I'm not sure this would be a bug)?
> + if (!batchable) {
> + /* State or receive-buffer failures keep the regular lookup path. */
> + if (!virtio_transport_rx_batch_can_continue(t, sk, len)) {
> + release_sock(sk);
> + sock_put(sk);
> + goto recv_pkt;
> + }
> +
> + virtio_transport_trace_recv_pkt(hdr, &src, &dst);
> + if (virtio_transport_recv_pkt_set_owner(skb, sk))
> + virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
> + else
> + kfree_skb(skb);
> +
> + release_sock(sk);
> + sock_put(sk);
> + return;
> + }
> +
> + batch->sk = sk;
> + virtio_transport_rx_batch_process(t, skb, net, batch, &src, &dst);
> + return;
> +
> +recv_pkt:
> + virtio_transport_recv_pkt(t, skb, net);
> +}
> +EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt_batch);
> +
> /* Remove skbs found in a queue that have a vsk that matches.
> *
> * Each skb is freed.
> --
> 2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets
2026-11-04 19:26 ` Bobby Eshleman
@ 2026-11-05 8:16 ` Jia Jia
0 siblings, 0 replies; 18+ messages in thread
From: Jia Jia @ 2026-11-05 8:16 UTC (permalink / raw)
To: Bobby Eshleman
Cc: netdev, stefanha, sgarzare, mst, jasowangio, eperezma, xuanzhuo,
davem, edumazet, kuba, pabeni, horms, kvm, virtualization,
linux-kernel
>
> On Tue, Nov 03, 2026 at 12:11:32PM +0800, Jia Jia wrote:
> > The virtio-vsock RX worker looks up a socket and takes its lock for every
> > packet. Retain the connected lookup reference and socket lock across a
> > bounded run of STREAM/RW packets for one established native socket.
> >
> > Reuse the locked socket while packets match the same flow. Finish the
> > batch on a malformed packet, flow change, non-RW traffic, or a change to
> > the socket state, transport, protocol, sockmap state, or receive buffer
> > limits. A connected lookup miss falls back to the existing receive path,
> > which can still find a bound socket.
> >
> > Limit an uninterrupted socket-lock ownership interval to 64 packets and
> > 64 KiB of payload. The payload limit is one
> > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE. Small or zero-length packets still incur
> > per-packet work, so an independent packet limit is needed. The 64-packet
> > limit uses VHOST_NET_BATCH as a reference; it is a policy choice rather
> > than a protocol limit.
> >
> > Beyond these limits, a larger batch would only further amortize socket
> > lookup and lock/unlock operations, while virtqueue work, validation,
> > tracing, skb ownership, credit update, and queueing remain per-packet. I
> > therefore expect diminishing returns while the uninterrupted socket-lock
> > interval continues to grow, but I have not measured larger limits yet.
> >
> > Only the guest virtio RX worker is changed. The benchmark exercises the
> > host-sender-to-guest-receiver direction, where this worker is on the
> > receive path. I have not measured the corresponding worker and socket-lock
> > overhead in vhost-vsock or vsock_loopback, and they use different worker
> > and locking models. Leave those paths unchanged pending separate policy
> > and validation. I plan to evaluate vhost-vsock separately as follow-up
> > work.
> >
> > Performance:
> >
> > These results cover P1-P3, before the notification changes in P4 and P5.
> >
> > Tested on a single-stream vsock connection (one Guest receiver thread on
> > CPU0, virtio-vsock IRQ on Guest CPU1, fresh QEMU Guest per state). The host
> > uses vhost-vsock.
> >
> > For throughput, vsock_perf runs on the host as the
> > sender, with a custom single-threaded Guest receiver pinned to Guest CPU0
> > using blocking recv(). The fixed-64K table uses a 64K buffer; the matched
> > table uses the sender's request size. The virtio-vsock IRQ/RX worker is
> > pinned to Guest CPU1. To normalize run times across payloads into a
> > bounded, deterministic window and avoid long runs being skewed by host
> > background noise or virtualization scheduling jitter, transfer sizes scale
> > proportionally with the request size (64B/64M, 256B/256M, etc.), capped at
> > 2G for 4K-64K. The custom receiver keeps the recv() size and count
> > explicit; vsock_perf's receiver uses poll()+read().
> >
> > End-to-end RX throughput (fixed-64K receiver; Gbit/s):
> >
> > payload baseline RX PATCH RX throughput change (95% CI)
> > (Gbit/s) (Gbit/s)
> > 64B 0.1285 0.2462 +91.59% [+79.74%, +104.22%]
> > 256B 0.5909 0.9666 +63.59% [+61.03%, +66.19%]
> > 512B 1.1817 1.8350 +55.29% [+46.76%, +64.32%]
> > 1K 2.3252 3.3600 +44.50% [+38.93%, +50.30%]
> > 4K 3.8436 5.5661 +44.82% [+40.30%, +49.47%]
> > 8K 4.6609 5.4115 +16.10% [ +8.91%, +23.77%]
> > 16K 5.1196 5.6399 +10.16% [ +4.07%, +16.61%]
> > 64K 5.6095 5.7062 +1.72% [-3.19%, +6.89%]
> >
> > Fixed-syscall stress test (receiver recv() size = sender write size;
> > no perf; Gbit/s):
> >
> > payload baseline RX PATCH RX throughput change (95% CI)
> > 64B 0.1446 0.2540 +75.71% [+63.67%, +88.62%]
> > recv() calls (baseline/PATCH): 1048577/1048577
> > 256B 0.4211 0.9061 +115.18% [+72.13%, +169.00%]
> > recv() calls (baseline/PATCH): 1048577/1048577
> > 512B 0.7980 1.6112 +101.90% [+95.30%, +108.72%]
> > recv() calls (baseline/PATCH): 1048577/1048577
> > 1K 1.4965 2.6986 +80.33% [+68.93%, +92.49%]
> > recv() calls (baseline/PATCH): 1048577/1048577
> > 4K 2.9384 4.8590 +65.36% [+60.17%, +70.72%]
> > recv() calls (baseline/PATCH): 539544/524982
> > 8K 3.8170 5.1887 +35.94% [+27.66%, +44.75%]
> > recv() calls (baseline/PATCH): 274857/262991
> > 16K 4.6856 5.5996 +19.51% [+15.36%, +23.81%]
> > recv() calls (baseline/PATCH): 147518/131351
> > 64K 5.5182 5.8800 +6.56% [-0.83%, +14.49%]
> > recv() calls (baseline/PATCH): 48492/32910
> >
> > Guest CPU1 aggregate cycles per received byte
> > (fixed 64K receiver, with perf):
> >
> > Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
> > 64B 66.728 61.220 -8.25% [-18.99%, +3.91%]
> > 256B 14.363 14.515 +1.06% [ -4.22%, +6.64%]
> > 512B 7.792 7.578 -2.74% [ -6.55%, +1.23%]
> > 1K 3.988 4.032 +1.09% [ -1.64%, +3.88%]
> > 4K 2.063 1.973 -4.38% [ -8.23%, -0.37%]
> > 8K 1.702 1.667 -2.06% [ -7.99%, +4.25%]
> > 16K 1.473 1.475 +0.17% [ -5.06%, +5.68%]
> > 64K 1.385 1.358 -1.96% [-10.61%, +7.53%]
> >
> > Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> > ---
> > include/linux/virtio_vsock.h | 23 ++
> > net/vmw_vsock/virtio_transport.c | 7 +-
> > net/vmw_vsock/virtio_transport_common.c | 303 ++++++++++++++++++++++--
> > 3 files changed, 318 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> > index f91704731057e..748633aa0cc81 100644
> > --- a/include/linux/virtio_vsock.h
> > +++ b/include/linux/virtio_vsock.h
> > @@ -282,6 +282,29 @@ void virtio_transport_destruct(struct vsock_sock *vsk);
> >
> > void virtio_transport_recv_pkt(struct virtio_transport *t,
> > struct sk_buff *skb, struct net *net);
> > +
> > +/*
> > + * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
> > + * lock and the reference returned by vsock_find_connected_socket_net(). The
> > + * caller retains both until it calls virtio_transport_rx_batch_finish(). An
> > + * empty batch owns neither.
> > + */
> > +struct virtio_transport_rx_batch {
> > + struct sock *sk;
> > + unsigned int pkts;
> > + size_t bytes;
> > +};
> > +
> > +static inline bool
> > +virtio_transport_rx_batch_empty(const struct virtio_transport_rx_batch *batch)
> > +{
> > + return !batch->sk;
> > +}
> > +
> > +void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
> > + struct sk_buff *skb, struct net *net,
> > + struct virtio_transport_rx_batch *batch);
> > +void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch);
> > void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
> > u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> > void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
> > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> > index 4f9aa9c4c3aa5..9af85712bd387 100644
> > --- a/net/vmw_vsock/virtio_transport.c
> > +++ b/net/vmw_vsock/virtio_transport.c
> > @@ -631,6 +631,7 @@ virtio_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
> >
> > static void virtio_transport_rx_work(struct work_struct *work)
> > {
> > struct virtio_vsock *vsock =
> > container_of(work, struct virtio_vsock, rx_work);
> > + struct virtio_transport_rx_batch batch = {};
> > struct virtqueue *vq;
> > @@ -666,6 +667,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
> > /* Drop short/long packets */
> > if (unlikely(len < sizeof(*hdr) ||
> > len > virtio_vsock_skb_len(skb))) {
> > + virtio_transport_rx_batch_finish(&batch);
> > kfree_skb(skb);
> > continue;
> > }
> > @@ -673,6 +675,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
> > hdr = virtio_vsock_hdr(skb);
> > payload_len = le32_to_cpu(hdr->len);
> > if (unlikely(payload_len > len - sizeof(*hdr))) {
> > + virtio_transport_rx_batch_finish(&batch);
> > kfree_skb(skb);
> > continue;
> > }
> > @@ -685,11 +688,13 @@ static void virtio_transport_rx_work(struct work_struct *work)
> > /* Force virtio-transport into global mode since it
> > * does not yet support local-mode namespacing.
> > */
> > - virtio_transport_recv_pkt(&virtio_transport, skb, NULL);
> > + virtio_transport_recv_pkt_batch(&virtio_transport, skb,
> > + NULL, &batch);
> > }
> > } while (!virtqueue_enable_cb(vq));
> >
> > out:
> > + virtio_transport_rx_batch_finish(&batch);
> > if (vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2)
> > virtio_vsock_rx_fill(vsock);
> > out_nofill:
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 2a9cfed085515..f83e5b2edce35 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -33,6 +33,15 @@
> > #define MAX_COLLAPSE_LEN \
> > SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER)
> >
> > +/*
> > + * Bound the interval that an RX worker owns a socket lock. One maximum packet
> > + * buffer (64 KiB) bounds payload in one uninterrupted ownership interval.
> > + * Small and zero-length packets need a packet limit to bound per-packet work;
> > + * the 64-packet policy uses VHOST_NET_BATCH as a reference.
> > + */
> > +#define VIRTIO_TRANSPORT_RX_BATCH_MAX_PKTS 64
> > +#define VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES VIRTIO_VSOCK_MAX_PKT_BUF_SIZE
> > +
> > static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
> > bool cancel_timeout);
> > static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
> > @@ -1793,13 +1802,94 @@ virtio_transport_find_socket(struct sockaddr_vm *src,
> > return sk;
> > }
> >
> > +static void
> > +virtio_transport_trace_recv_pkt(const struct virtio_vsock_hdr *hdr,
> > + const struct sockaddr_vm *src,
> > + const struct sockaddr_vm *dst)
> > +{
> > + trace_virtio_transport_recv_pkt(src->svm_cid, src->svm_port,
> > + dst->svm_cid, dst->svm_port,
> > + le32_to_cpu(hdr->len),
> > + le16_to_cpu(hdr->type),
> > + le16_to_cpu(hdr->op),
> > + le32_to_cpu(hdr->flags),
> > + le32_to_cpu(hdr->buf_alloc),
> > + le32_to_cpu(hdr->fwd_cnt));
> > +}
>
Hi Bobby,
> I'd rather see trace_virtio_transport_recv_pkt() refactored to take hdr,
> src, and dst instead of wrapping it. I'd guess the trace definition
> field assignment block can handle the dereferencing and leXY_to_cpu()
> calls.
>
Sure, will do.
> > +
> > +static bool
> > +virtio_transport_recv_pkt_set_owner(struct sk_buff *skb, struct sock *sk)
> > +{
> > + if (skb_set_owner_sk_safe(skb, sk))
> > + return true;
> > +
> > + WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
> > + return false;
> > +}
> > +
> > +static bool
> > +virtio_transport_sock_has_psock(const struct sock *sk)
> > +{
> > + return (uintptr_t)rcu_access_pointer(__sk_user_data(sk)) &
> > + SK_USER_DATA_PSOCK;
> > +}
> > +
> > +/* The caller holds sk's socket lock. */
> > +static bool
> > +virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
> > + struct sock *sk, u32 len)
> > +{
> > + struct virtio_vsock_sock *vvs;
> > + struct vsock_sock *vsk;
> > +
> > + vsk = vsock_sk(sk);
> > + vvs = vsk->trans;
> > +
> > + /*
> > + * Keep a connected lookup reference only for the native, established
> > + * STREAM path. Sockets outside ESTABLISHED can change table membership;
> > + * a completed or reassigned socket must resume the per-packet path.
> > + */
> > + if (sk->sk_state != TCP_ESTABLISHED ||
> > + sk->sk_type != SOCK_STREAM ||
> > + sock_flag(sk, SOCK_DONE) ||
> > + vsk->transport != &t->transport || !vvs)
> > + return false;
> > +
> > + /*
> > + * Do not admit a packet that would hit an existing RX limit while the
> > + * worker owns the socket lock. The caller holds that lock, so no reader can
> > + * dequeue until the batch is released.
> > + */
> > + return (u64)vvs->buf_used + len <= vvs->buf_alloc &&
> > + !virtio_transport_rx_buf_full(vvs);
> > +}
> > +
> > +/* The caller holds sk's socket lock. */
> > +static bool
> > +virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> > + struct sock *sk, u32 len)
> > +{
> > + if (!virtio_transport_rx_batch_can_continue(t, sk, len))
> > + return false;
> > +
> > + /*
> > + * Sockmap can replace sk_prot and publish a psock without the socket lock.
> > + * Check both at each packet boundary; a boundary that sees a replacement
> > + * returns to the existing per-packet path.
> > + */
> > + return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
> > + !virtio_transport_sock_has_psock(sk);
>
> Can you elaborate on the problem caused by this changing between packet
> boundaries? Sorry its been a while since I refreshed my memory on the
> sockmap stuff.
>
sock_map_update_elem() (the BPF helper path) only takes bh_lock_sock()
without checking sock_owned_by_user(), and sock_map_delete_elem() does not
take lock_sock() either, so both can run while the RX worker holds
lock_sock() for the batch.
Patch 4 coalesces sk_write_space and calls the saved native callback at
batch finish. sk_data_ready is still per packet. Patch 5 also coalesces
sk_data_ready onto the saved native callback, so a socket that stays in
the batch after a sockmap attach skips sk_psock_verdict_data_ready().
That callback pulls one skb through virtio_transport_read_skb() per call.
The packet that sees the change goes back to the per-packet path. Packets
already admitted finish with the saved native callbacks.
> > +}
> > +
> > /*
> > - * The caller holds sk's socket lock.
> > + * The caller holds sk's socket lock. If @batchable is non-NULL, update it
> > + * after processing @skb to say whether a native STREAM/RW batch can continue.
> > */
> > static void
> > virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > struct sk_buff *skb, struct sock *sk,
> > - struct net *net)
> > + struct net *net, bool *batchable)
> > {
> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > struct vsock_sock *vsk = vsock_sk(sk);
> > @@ -1809,6 +1899,9 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
> > le32_to_cpu(hdr->src_port));
> >
> > + if (batchable)
> > + *batchable = false;
> > +
> > /*
> > * The socket may have been closed or reassigned after lookup but before
> > * its socket lock was acquired. Listener sockets have no assigned
> > @@ -1852,6 +1945,9 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > kfree_skb(skb);
> > break;
> > }
> > +
> > + if (batchable)
> > + *batchable = virtio_transport_rx_batch_can_receive(t, sk, 0);
> > }
> >
> > /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
> > @@ -1869,14 +1965,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> > vsock_addr_init(&dst, le64_to_cpu(hdr->dst_cid),
> > le32_to_cpu(hdr->dst_port));
> >
> > - trace_virtio_transport_recv_pkt(src.svm_cid, src.svm_port,
> > - dst.svm_cid, dst.svm_port,
> > - le32_to_cpu(hdr->len),
> > - le16_to_cpu(hdr->type),
> > - le16_to_cpu(hdr->op),
> > - le32_to_cpu(hdr->flags),
> > - le32_to_cpu(hdr->buf_alloc),
> > - le32_to_cpu(hdr->fwd_cnt));
> > + virtio_transport_trace_recv_pkt(hdr, &src, &dst);
> >
> > if (!virtio_transport_valid_type(le16_to_cpu(hdr->type))) {
> > (void)virtio_transport_reset_no_sock(t, skb, net);
> > @@ -1898,13 +1987,11 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> > goto free_pkt;
> > }
> >
> > - if (!skb_set_owner_sk_safe(skb, sk)) {
> > - WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
> > + if (!virtio_transport_recv_pkt_set_owner(skb, sk))
> > goto free_pkt;
> > - }
> >
> > lock_sock(sk);
> > - virtio_transport_recv_pkt_locked(t, skb, sk, net);
> > + virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
> >
> > release_sock(sk);
> >
> > @@ -1919,6 +2006,194 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> > }
> > EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
> >
> > +/*
> > + * Release an active RX batch. A non-empty batch holds the socket lock and the
> > + * reference returned by vsock_find_connected_socket_net(). An empty batch
> > + * holds neither.
> > + */
> > +void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
> > +{
> > + struct sock *sk = batch->sk;
> > +
> > + batch->sk = NULL;
> > + batch->pkts = 0;
> > + batch->bytes = 0;
> > +
> > + if (!sk)
> > + return;
> > +
> > + release_sock(sk);
> > + sock_put(sk);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_transport_rx_batch_finish);
> > +
> > +static bool
> > +virtio_transport_rx_batch_matches(const struct virtio_transport_rx_batch *batch,
> > + const struct sockaddr_vm *src,
> > + const struct sockaddr_vm *dst,
> > + struct net *net)
> > +{
> > + struct vsock_sock *vsk = vsock_sk(batch->sk);
> > +
> > + /* The connected table is keyed by peer address and local port. */
> > + return vsock_addr_equals_addr(src, &vsk->remote_addr) &&
> > + dst->svm_port == vsk->local_addr.svm_port &&
> > + vsock_net_check_mode(sock_net(batch->sk), net);
> > +}
> > +
> > +static bool
> > +virtio_transport_rx_batch_can_add(const struct virtio_transport_rx_batch *batch,
> > + u32 len)
> > +{
> > + if (batch->pkts >= VIRTIO_TRANSPORT_RX_BATCH_MAX_PKTS ||
> > + batch->bytes >= VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES)
> > + return false;
> > +
> > + return len <= VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES - batch->bytes;
> > +}
> > +
> > +static void
> > +virtio_transport_rx_batch_account(struct virtio_transport_rx_batch *batch,
> > + u32 len)
> > +{
> > + batch->pkts++;
> > + batch->bytes += len;
> > +}
> > +
> > +static void
> > +virtio_transport_rx_batch_process(struct virtio_transport *t,
> > + struct sk_buff *skb, struct net *net,
> > + struct virtio_transport_rx_batch *batch,
> > + const struct sockaddr_vm *src,
> > + const struct sockaddr_vm *dst)
> > +{
> > + struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > + bool batchable;
> > + u32 len;
> > +
> > + len = le32_to_cpu(hdr->len);
> > + virtio_transport_trace_recv_pkt(hdr, src, dst);
> > +
> > + if (!virtio_transport_recv_pkt_set_owner(skb, batch->sk)) {
> > + virtio_transport_rx_batch_finish(batch);
> > + kfree_skb(skb);
> > + return;
> > + }
> > +
> > + virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable);
> > + if (!batchable) {
> > + virtio_transport_rx_batch_finish(batch);
> > + return;
> > + }
> > +
> > + virtio_transport_rx_batch_account(batch, len);
> > + if (!virtio_transport_rx_batch_can_add(batch, 0))
> > + virtio_transport_rx_batch_finish(batch);
> > +}
> > +
> > +void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
> > + struct sk_buff *skb, struct net *net,
> > + struct virtio_transport_rx_batch *batch)
> > +{
> > + struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > + struct sockaddr_vm src, dst;
> > + struct sock *sk;
> > + bool batchable;
> > + u32 len;
> > +
> > + /*
> > + * Batch only STREAM data packets. Control packets can change connection
> > + * state or send a reply. Keep SEQPACKET on its existing path: its EOM/EOR
> > + * record-boundary semantics have not been separately evaluated for batching.
> > + */
> > + if (le16_to_cpu(hdr->type) != VIRTIO_VSOCK_TYPE_STREAM ||
> > + le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_RW) {
> > + virtio_transport_rx_batch_finish(batch);
> > + goto recv_pkt;
> > + }
> > +
> > + len = le32_to_cpu(hdr->len);
> > + vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
> > + le32_to_cpu(hdr->src_port));
> > + vsock_addr_init(&dst, le64_to_cpu(hdr->dst_cid),
> > + le32_to_cpu(hdr->dst_port));
> > +
> > + if (!virtio_transport_rx_batch_empty(batch)) {
> > + /* A post-processing check cannot bound a packet already admitted. */
> > + if (!virtio_transport_rx_batch_can_add(batch, len) ||
> > + !virtio_transport_rx_batch_matches(batch, &src, &dst, net)) {
> > + virtio_transport_rx_batch_finish(batch);
> > + } else if (!virtio_transport_rx_batch_can_receive(t, batch->sk,
> > + len)) {
> > + virtio_transport_rx_batch_finish(batch);
> > + goto recv_pkt;
> > + }
> > + }
> > +
> > + if (!virtio_transport_rx_batch_empty(batch)) {
> > + virtio_transport_rx_batch_process(t, skb, net, batch, &src, &dst);
> > + return;
> > + }
> > +
> > + /*
> > + * A packet larger than the byte cap must take the regular path. Starting a
> > + * one-packet batch would already exceed the bound on socket-lock ownership.
> > + */
> > + if (!virtio_transport_rx_batch_can_add(batch, len))
> > + goto recv_pkt;
> > +
> > + sk = vsock_find_connected_socket_net(&src, &dst, net);
> > + if (!sk) {
> > + /*
> > + * A connected lookup miss can still match a bound socket in
> > + * virtio_transport_recv_pkt().
> > + */
> > + goto recv_pkt;
> > + }
> > +
> > + if (virtio_transport_get_type(sk) != le16_to_cpu(hdr->type)) {
> > + /* Let the regular receive path send the type-mismatch reset. */
> > + sock_put(sk);
> > + goto recv_pkt;
> > + }
> > +
> > + lock_sock(sk);
> > + /*
> > + * Serialize initial admission with sockmap's callback updates. Later
> > + * packet boundaries recheck native state and end a batch when they
> > + * observe a replacement.
> > + */
> > + read_lock_bh(&sk->sk_callback_lock);
> > + batchable = virtio_transport_rx_batch_can_receive(t, sk, len);
> > + read_unlock_bh(&sk->sk_callback_lock);
>
> Why do we need sk_callback_lock? what happens if we don't take it? The
> comment suggests we're synchronizing with sockmap callback updates, but
> we don't hold the lock for all of batch processing, so the callback can
> still be updated later anyway (and I'm not sure this would be a bug)?
>
Patch 3 only checks sk_prot and the psock bit, so I will drop
sk_callback_lock from that patch.
Patch 4 reads sk_write_space, and patch 5 also reads sk_data_ready.
sk_psock_drop() runs under write_lock_bh(&sk->sk_callback_lock):
sk_psock_restore_proto() restores sk_write_space and sk_prot, then
sk_user_data is cleared, and sk_psock_stop_verdict() restores sk_data_ready
last. Without read_lock_bh(&sk->sk_callback_lock), patch 4 can observe the
first three steps and admit the socket while sk_data_ready is still
sk_psock_verdict_data_ready.
We only need the lock during the admission check so we don't observe a
partially torn-down socket; if an update happens after admission, the
current packet finishes as native and the next packet boundary catches the
change.
Thanks,
Jia
> > + if (!batchable) {
> > + /* State or receive-buffer failures keep the regular lookup path. */
> > + if (!virtio_transport_rx_batch_can_continue(t, sk, len)) {
> > + release_sock(sk);
> > + sock_put(sk);
> > + goto recv_pkt;
> > + }
> > +
> > + virtio_transport_trace_recv_pkt(hdr, &src, &dst);
> > + if (virtio_transport_recv_pkt_set_owner(skb, sk))
> > + virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
> > + else
> > + kfree_skb(skb);
> > +
> > + release_sock(sk);
> > + sock_put(sk);
> > + return;
> > + }
> > +
> > + batch->sk = sk;
> > + virtio_transport_rx_batch_process(t, skb, net, batch, &src, &dst);
> > + return;
> > +
> > +recv_pkt:
> > + virtio_transport_recv_pkt(t, skb, net);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt_batch);
> > +
> > /* Remove skbs found in a queue that have a vsk that matches.
> > *
> > * Each skb is freed.
> > --
> > 2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets
2026-11-03 4:11 ` [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets Jia Jia
2026-11-04 19:26 ` Bobby Eshleman
@ 2026-11-05 8:57 ` Stefano Garzarella
2026-11-06 12:27 ` Jia Jia
1 sibling, 1 reply; 18+ messages in thread
From: Stefano Garzarella @ 2026-11-05 8:57 UTC (permalink / raw)
To: Jia Jia
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
On Tue, Nov 03, 2026 at 12:11:32PM +0800, Jia Jia wrote:
>The virtio-vsock RX worker looks up a socket and takes its lock for every
>packet. Retain the connected lookup reference and socket lock across a
>bounded run of STREAM/RW packets for one established native socket.
>
>Reuse the locked socket while packets match the same flow. Finish the
>batch on a malformed packet, flow change, non-RW traffic, or a change to
>the socket state, transport, protocol, sockmap state, or receive buffer
>limits. A connected lookup miss falls back to the existing receive path,
>which can still find a bound socket.
>
>Limit an uninterrupted socket-lock ownership interval to 64 packets and
>64 KiB of payload. The payload limit is one
>VIRTIO_VSOCK_MAX_PKT_BUF_SIZE. Small or zero-length packets still incur
>per-packet work, so an independent packet limit is needed. The 64-packet
>limit uses VHOST_NET_BATCH as a reference; it is a policy choice rather
>than a protocol limit.
>
>Beyond these limits, a larger batch would only further amortize socket
>lookup and lock/unlock operations, while virtqueue work, validation,
>tracing, skb ownership, credit update, and queueing remain per-packet. I
>therefore expect diminishing returns while the uninterrupted socket-lock
>interval continues to grow, but I have not measured larger limits yet.
>
>Only the guest virtio RX worker is changed. The benchmark exercises the
>host-sender-to-guest-receiver direction, where this worker is on the
>receive path. I have not measured the corresponding worker and socket-lock
>overhead in vhost-vsock or vsock_loopback, and they use different worker
>and locking models. Leave those paths unchanged pending separate policy
>and validation. I plan to evaluate vhost-vsock separately as follow-up
>work.
>
>Performance:
>
>These results cover P1-P3, before the notification changes in P4 and P5.
>
>Tested on a single-stream vsock connection (one Guest receiver thread on
>CPU0, virtio-vsock IRQ on Guest CPU1, fresh QEMU Guest per state). The host
>uses vhost-vsock.
>
>For throughput, vsock_perf runs on the host as the
>sender, with a custom single-threaded Guest receiver pinned to Guest CPU0
>using blocking recv(). The fixed-64K table uses a 64K buffer; the matched
>table uses the sender's request size. The virtio-vsock IRQ/RX worker is
>pinned to Guest CPU1. To normalize run times across payloads into a
>bounded, deterministic window and avoid long runs being skewed by host
>background noise or virtualization scheduling jitter, transfer sizes scale
>proportionally with the request size (64B/64M, 256B/256M, etc.), capped at
>2G for 4K-64K. The custom receiver keeps the recv() size and count
>explicit; vsock_perf's receiver uses poll()+read().
>
>End-to-end RX throughput (fixed-64K receiver; Gbit/s):
>
> payload baseline RX PATCH RX throughput change (95% CI)
> (Gbit/s) (Gbit/s)
> 64B 0.1285 0.2462 +91.59% [+79.74%, +104.22%]
> 256B 0.5909 0.9666 +63.59% [+61.03%, +66.19%]
> 512B 1.1817 1.8350 +55.29% [+46.76%, +64.32%]
> 1K 2.3252 3.3600 +44.50% [+38.93%, +50.30%]
> 4K 3.8436 5.5661 +44.82% [+40.30%, +49.47%]
> 8K 4.6609 5.4115 +16.10% [ +8.91%, +23.77%]
> 16K 5.1196 5.6399 +10.16% [ +4.07%, +16.61%]
> 64K 5.6095 5.7062 +1.72% [-3.19%, +6.89%]
>
>Fixed-syscall stress test (receiver recv() size = sender write size;
>no perf; Gbit/s):
>
> payload baseline RX PATCH RX throughput change (95% CI)
> 64B 0.1446 0.2540 +75.71% [+63.67%, +88.62%]
> recv() calls (baseline/PATCH): 1048577/1048577
> 256B 0.4211 0.9061 +115.18% [+72.13%, +169.00%]
> recv() calls (baseline/PATCH): 1048577/1048577
> 512B 0.7980 1.6112 +101.90% [+95.30%, +108.72%]
> recv() calls (baseline/PATCH): 1048577/1048577
> 1K 1.4965 2.6986 +80.33% [+68.93%, +92.49%]
> recv() calls (baseline/PATCH): 1048577/1048577
> 4K 2.9384 4.8590 +65.36% [+60.17%, +70.72%]
> recv() calls (baseline/PATCH): 539544/524982
> 8K 3.8170 5.1887 +35.94% [+27.66%, +44.75%]
> recv() calls (baseline/PATCH): 274857/262991
> 16K 4.6856 5.5996 +19.51% [+15.36%, +23.81%]
> recv() calls (baseline/PATCH): 147518/131351
> 64K 5.5182 5.8800 +6.56% [-0.83%, +14.49%]
> recv() calls (baseline/PATCH): 48492/32910
>
>Guest CPU1 aggregate cycles per received byte
>(fixed 64K receiver, with perf):
>
> Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
> 64B 66.728 61.220 -8.25% [-18.99%, +3.91%]
> 256B 14.363 14.515 +1.06% [ -4.22%, +6.64%]
> 512B 7.792 7.578 -2.74% [ -6.55%, +1.23%]
> 1K 3.988 4.032 +1.09% [ -1.64%, +3.88%]
> 4K 2.063 1.973 -4.38% [ -8.23%, -0.37%]
> 8K 1.702 1.667 -2.06% [ -7.99%, +4.25%]
> 16K 1.473 1.475 +0.17% [ -5.06%, +5.68%]
> 64K 1.385 1.358 -1.96% [-10.61%, +7.53%]
>
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> include/linux/virtio_vsock.h | 23 ++
> net/vmw_vsock/virtio_transport.c | 7 +-
> net/vmw_vsock/virtio_transport_common.c | 303 ++++++++++++++++++++++--
> 3 files changed, 318 insertions(+), 15 deletions(-)
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index f91704731057e..748633aa0cc81 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -282,6 +282,29 @@ void virtio_transport_destruct(struct vsock_sock *vsk);
>
> void virtio_transport_recv_pkt(struct virtio_transport *t,
> struct sk_buff *skb, struct net *net);
>+
>+/*
>+ * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
>+ * lock and the reference returned by vsock_find_connected_socket_net(). The
>+ * caller retains both until it calls virtio_transport_rx_batch_finish(). An
>+ * empty batch owns neither.
>+ */
>+struct virtio_transport_rx_batch {
>+ struct sock *sk;
>+ unsigned int pkts;
>+ size_t bytes;
>+};
>+
>+static inline bool
>+virtio_transport_rx_batch_empty(const struct virtio_transport_rx_batch *batch)
>+{
>+ return !batch->sk;
>+}
>+
>+void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
>+ struct sk_buff *skb, struct net *net,
>+ struct virtio_transport_rx_batch *batch);
>+void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch);
> void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
> u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index 4f9aa9c4c3aa5..9af85712bd387 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -631,6 +631,7 @@ virtio_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
>
> static void virtio_transport_rx_work(struct work_struct *work)
> {
> struct virtio_vsock *vsock =
> container_of(work, struct virtio_vsock, rx_work);
>+ struct virtio_transport_rx_batch batch = {};
> struct virtqueue *vq;
>@@ -666,6 +667,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
> /* Drop short/long packets */
> if (unlikely(len < sizeof(*hdr) ||
> len > virtio_vsock_skb_len(skb))) {
>+ virtio_transport_rx_batch_finish(&batch);
> kfree_skb(skb);
> continue;
> }
>@@ -673,6 +675,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
> hdr = virtio_vsock_hdr(skb);
> payload_len = le32_to_cpu(hdr->len);
> if (unlikely(payload_len > len - sizeof(*hdr))) {
>+ virtio_transport_rx_batch_finish(&batch);
> kfree_skb(skb);
> continue;
> }
>@@ -685,11 +688,13 @@ static void virtio_transport_rx_work(struct work_struct *work)
> /* Force virtio-transport into global mode since it
> * does not yet support local-mode namespacing.
> */
>- virtio_transport_recv_pkt(&virtio_transport, skb, NULL);
>+ virtio_transport_recv_pkt_batch(&virtio_transport, skb,
>+ NULL, &batch);
> }
> } while (!virtqueue_enable_cb(vq));
>
> out:
>+ virtio_transport_rx_batch_finish(&batch);
> if (vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2)
> virtio_vsock_rx_fill(vsock);
> out_nofill:
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 2a9cfed085515..f83e5b2edce35 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -33,6 +33,15 @@
> #define MAX_COLLAPSE_LEN \
> SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER)
>
>+/*
>+ * Bound the interval that an RX worker owns a socket lock. One maximum packet
>+ * buffer (64 KiB) bounds payload in one uninterrupted ownership interval.
>+ * Small and zero-length packets need a packet limit to bound per-packet work;
>+ * the 64-packet policy uses VHOST_NET_BATCH as a reference.
>+ */
>+#define VIRTIO_TRANSPORT_RX_BATCH_MAX_PKTS 64
>+#define VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES VIRTIO_VSOCK_MAX_PKT_BUF_SIZE
>+
> static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
> bool cancel_timeout);
> static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
>@@ -1793,13 +1802,94 @@ virtio_transport_find_socket(struct sockaddr_vm *src,
> return sk;
> }
>
>+static void
>+virtio_transport_trace_recv_pkt(const struct virtio_vsock_hdr *hdr,
>+ const struct sockaddr_vm *src,
>+ const struct sockaddr_vm *dst)
>+{
>+ trace_virtio_transport_recv_pkt(src->svm_cid, src->svm_port,
>+ dst->svm_cid, dst->svm_port,
>+ le32_to_cpu(hdr->len),
>+ le16_to_cpu(hdr->type),
>+ le16_to_cpu(hdr->op),
>+ le32_to_cpu(hdr->flags),
>+ le32_to_cpu(hdr->buf_alloc),
>+ le32_to_cpu(hdr->fwd_cnt));
>+}
>+
>+static bool
>+virtio_transport_recv_pkt_set_owner(struct sk_buff *skb, struct sock *sk)
>+{
>+ if (skb_set_owner_sk_safe(skb, sk))
>+ return true;
>+
>+ WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
>+ return false;
>+}
>+
>+static bool
>+virtio_transport_sock_has_psock(const struct sock *sk)
>+{
>+ return (uintptr_t)rcu_access_pointer(__sk_user_data(sk)) &
>+ SK_USER_DATA_PSOCK;
>+}
>+
>+/* The caller holds sk's socket lock. */
>+static bool
>+virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
>+ struct sock *sk, u32 len)
>+{
>+ struct virtio_vsock_sock *vvs;
>+ struct vsock_sock *vsk;
>+
>+ vsk = vsock_sk(sk);
>+ vvs = vsk->trans;
>+
>+ /*
>+ * Keep a connected lookup reference only for the native, established
>+ * STREAM path. Sockets outside ESTABLISHED can change table membership;
>+ * a completed or reassigned socket must resume the per-packet path.
>+ */
>+ if (sk->sk_state != TCP_ESTABLISHED ||
>+ sk->sk_type != SOCK_STREAM ||
>+ sock_flag(sk, SOCK_DONE) ||
>+ vsk->transport != &t->transport || !vvs)
>+ return false;
>+
>+ /*
>+ * Do not admit a packet that would hit an existing RX limit while the
>+ * worker owns the socket lock. The caller holds that lock, so no reader can
>+ * dequeue until the batch is released.
>+ */
>+ return (u64)vvs->buf_used + len <= vvs->buf_alloc &&
>+ !virtio_transport_rx_buf_full(vvs);
I don't understand this. We recently introduce the collapse, so when we
will call virtio_transport_recv_enqueue(), all the queue will be
collapsed, including packets in the batch.
Or am I missing something?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 1/5] vsock/virtio: split socket lookup from locked RX processing
2026-11-03 4:11 ` [PATCH net-next v5 1/5] vsock/virtio: split socket lookup from locked RX processing Jia Jia
@ 2026-11-05 9:04 ` Stefano Garzarella
0 siblings, 0 replies; 18+ messages in thread
From: Stefano Garzarella @ 2026-11-05 9:04 UTC (permalink / raw)
To: Jia Jia
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
On Tue, Nov 03, 2026 at 12:11:30PM +0800, Jia Jia wrote:
>Split virtio_transport_recv_pkt() so socket lookup is separate from
>processing that occurs while holding the socket lock. This prepares the
>receive path to retain a looked-up socket across multiple packets in a
>later change.
>
>Keep virtio_transport_recv_pkt() on its existing per-packet locking path.
>Pass the caller-provided network namespace to the locked helper so reset
>handling retains its existing semantics. Preserve source validation and
>failure handling.
>
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 131 ++++++++++++++----------
> 1 file changed, 78 insertions(+), 53 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index f225f53ed4bab..43325d7aebbb1 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1774,6 +1774,80 @@ static bool virtio_transport_valid_type(u16 type)
> (type == VIRTIO_VSOCK_TYPE_SEQPACKET);
> }
>
>+static struct sock *
>+virtio_transport_find_socket(struct sockaddr_vm *src,
>+ struct sockaddr_vm *dst, struct net *net)
>+{
>+ struct sock *sk;
>+
>+ sk = vsock_find_connected_socket_net(src, dst, net);
>+ if (!sk)
>+ sk = vsock_find_bound_socket_net(dst, net);
>+
>+ return sk;
>+}
>+
>+/*
>+ * The caller holds sk's socket lock.
>+ */
>+static void
>+virtio_transport_recv_pkt_locked(struct virtio_transport *t,
>+ struct sk_buff *skb, struct sock *sk,
>+ struct net *net)
>+{
>+ struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
>+ struct vsock_sock *vsk = vsock_sk(sk);
>+ struct sockaddr_vm src;
>+ bool space_available;
>+
>+ vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
>+ le32_to_cpu(hdr->src_port));
>+
>+ /*
>+ * The socket may have been closed or reassigned after lookup but before
>+ * its socket lock was acquired. Listener sockets have no assigned
>+ * transport.
>+ */
>+ if (sock_flag(sk, SOCK_DONE) ||
>+ (sk->sk_state != TCP_LISTEN &&
>+ !vsock_check_source(vsk, &t->transport, &src))) {
>+ (void)virtio_transport_reset_no_sock(t, skb, net);
>+ kfree_skb(skb);
>+ return;
>+ }
>+
>+ space_available = virtio_transport_space_update(sk, skb);
>+
>+ /* Update CID in case it has changed after a transport reset event */
>+ if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
>+ vsk->local_addr.svm_cid = le64_to_cpu(hdr->dst_cid);
>+
>+ if (space_available)
>+ sk->sk_write_space(sk);
>+
>+ switch (sk->sk_state) {
>+ case TCP_LISTEN:
>+ virtio_transport_recv_listen(sk, skb, t);
>+ kfree_skb(skb);
>+ break;
>+ case TCP_SYN_SENT:
>+ virtio_transport_recv_connecting(sk, skb);
>+ kfree_skb(skb);
>+ break;
>+ case TCP_ESTABLISHED:
>+ virtio_transport_recv_connected(sk, skb);
>+ break;
>+ case TCP_CLOSING:
>+ virtio_transport_recv_disconnecting(sk, skb);
>+ kfree_skb(skb);
>+ break;
>+ default:
>+ (void)virtio_transport_reset_no_sock(t, skb, net);
>+ kfree_skb(skb);
>+ break;
>+ }
>+}
>+
> /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
> * lock.
> */
>@@ -1782,9 +1856,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct sockaddr_vm src, dst;
>- struct vsock_sock *vsk;
> struct sock *sk;
>- bool space_available;
>
> vsock_addr_init(&src, le64_to_cpu(hdr->src_cid),
> le32_to_cpu(hdr->src_port));
>@@ -1808,13 +1880,10 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> /* The socket must be in connected or bound table
> * otherwise send reset back
> */
>- sk = vsock_find_connected_socket_net(&src, &dst, net);
>+ sk = virtio_transport_find_socket(&src, &dst, net);
> if (!sk) {
>- sk = vsock_find_bound_socket_net(&dst, net);
>- if (!sk) {
>- (void)virtio_transport_reset_no_sock(t, skb, net);
>- goto free_pkt;
>- }
>+ (void)virtio_transport_reset_no_sock(t, skb, net);
>+ goto free_pkt;
> }
>
> if (virtio_transport_get_type(sk) != le16_to_cpu(hdr->type)) {
>@@ -1828,52 +1897,8 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> goto free_pkt;
> }
>
>- vsk = vsock_sk(sk);
>-
> lock_sock(sk);
>-
>- /* Check if sk has been closed or assigned to another transport before
>- * lock_sock (note: listener sockets are not assigned to any transport)
>- */
>- if (sock_flag(sk, SOCK_DONE) ||
>- (sk->sk_state != TCP_LISTEN &&
>- !vsock_check_source(vsk, &t->transport, &src))) {
>- (void)virtio_transport_reset_no_sock(t, skb, net);
>- release_sock(sk);
>- sock_put(sk);
>- goto free_pkt;
>- }
>-
>- space_available = virtio_transport_space_update(sk, skb);
>-
>- /* Update CID in case it has changed after a transport reset event */
>- if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
>- vsk->local_addr.svm_cid = dst.svm_cid;
>-
>- if (space_available)
>- sk->sk_write_space(sk);
>-
>- switch (sk->sk_state) {
>- case TCP_LISTEN:
>- virtio_transport_recv_listen(sk, skb, t);
>- kfree_skb(skb);
>- break;
>- case TCP_SYN_SENT:
>- virtio_transport_recv_connecting(sk, skb);
>- kfree_skb(skb);
>- break;
>- case TCP_ESTABLISHED:
>- virtio_transport_recv_connected(sk, skb);
>- break;
>- case TCP_CLOSING:
>- virtio_transport_recv_disconnecting(sk, skb);
>- kfree_skb(skb);
>- break;
>- default:
>- (void)virtio_transport_reset_no_sock(t, skb, net);
>- kfree_skb(skb);
>- break;
>- }
>+ virtio_transport_recv_pkt_locked(t, skb, sk, net);
>
nit: if you need to resend, please remove this blank line or add a blank
line after lock_sock().
The rest LGTM:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> release_sock(sk);
>
>--
>2.53.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 2/5] vsock/virtio: factor RX skb metadata budget check
2026-11-03 4:11 ` [PATCH net-next v5 2/5] vsock/virtio: factor RX skb metadata budget check Jia Jia
@ 2026-11-05 9:06 ` Stefano Garzarella
2026-11-06 12:28 ` Jia Jia
0 siblings, 1 reply; 18+ messages in thread
From: Stefano Garzarella @ 2026-11-05 9:06 UTC (permalink / raw)
To: Jia Jia
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
On Tue, Nov 03, 2026 at 12:11:31PM +0800, Jia Jia wrote:
>Extract the skb metadata budget check from virtio_transport_inc_rx_pkt()
>into virtio_transport_rx_buf_full().
>
>The helper retains the existing condition and will be reused by a
>following RX batching change.
>
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 43325d7aebbb1..2a9cfed085515 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -576,11 +576,17 @@ virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs,
> skb_queue_splice(&new_queue, &vvs->rx_queue);
> }
>
>-static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
>- u32 len)
>+static bool
>+virtio_transport_rx_buf_full(struct virtio_vsock_sock *vvs)
> {
> u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
>
>+ return skb_overhead > vvs->buf_alloc;
>+}
As I pointed out in patch 3, I'm not sure we need this.
After we introduced the collapse, SOCK_STREAM should collapse rx_queue
before hitting this, so not sure we really need this function.
Thanks,
Stefano
>+
>+static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
>+ u32 len)
>+{
> /* Allow at most buf_alloc * 2 total budget (payload + overhead),
> * similar to how SO_RCVBUF is doubled to reserve space for sk_buff
> * metadata. Check payload against buf_alloc to be sure the other
>@@ -588,7 +594,7 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> * queue growth.
> */
> if ((u64)vvs->buf_used + len > vvs->buf_alloc ||
>- skb_overhead > vvs->buf_alloc)
>+ virtio_transport_rx_buf_full(vvs))
> return false;
>
> vvs->rx_bytes += len;
>--
>2.53.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications
2026-11-03 4:11 ` [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications Jia Jia
@ 2026-11-05 9:21 ` Stefano Garzarella
2026-11-06 12:49 ` Jia Jia
0 siblings, 1 reply; 18+ messages in thread
From: Stefano Garzarella @ 2026-11-05 9:21 UTC (permalink / raw)
To: Jia Jia
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
On Tue, Nov 03, 2026 at 12:11:33PM +0800, Jia Jia wrote:
>Each received packet can update peer credit and call sk_write_space(). A
>writer cannot consume that credit while the virtio RX worker retains the
>socket lock, so repeated native write-space notifications in one RX batch
>are unnecessary.
>
>This patch coalesces write-space notifications on the native AF_VSOCK path;
>data-ready remains per-packet here.
>
>The batching patch establishes batch ownership before its first packet.
>This lets this patch record a pending notification for that packet and
>flush it even if the packet immediately ends the batch. Deliver it while
>holding the socket lock to preserve the callback context of the existing
>receive path. A following patch changes that timing together with
>data-ready handling.
Can you report performance data related to this patch also here?
IIUC this is not giving a lot of advantage, so my question is if this
more complexity really add a value for us.
>
>Capture the native write-space callback before socket publication and
>access it through AF_VSOCK helpers. Sockmap can change the protocol,
>write-space callback, and psock marker without the socket lock. Serialize
>each batch eligibility check with sk_callback_lock; if a packet boundary
>observes a replacement, finish the batch and use the regular per-packet
>path.
>
>Packets admitted on the native path use the saved native callback at batch
>finish. Reloading a current sockmap callback would not synchronize its
>invocation with teardown; a wrapper seen before teardown need not forward
>the native wakeup after its psock is gone.
I really don't understand the goal of this patch, and why we need to
store the "native" sk_write_space.
>
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> include/linux/virtio_vsock.h | 4 +-
> include/net/af_vsock.h | 7 +++
> net/vmw_vsock/af_vsock.c | 16 +++++
> net/vmw_vsock/virtio_transport_common.c | 78 +++++++++++++++++--------
> 4 files changed, 81 insertions(+), 24 deletions(-)
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index 748633aa0cc81..bf28e8e32e280 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -287,12 +287,14 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
> * lock and the reference returned by vsock_find_connected_socket_net(). The
> * caller retains both until it calls virtio_transport_rx_batch_finish(). An
>- * empty batch owns neither.
>+ * empty batch owns neither. A pending native write-space notification belongs
>+ * to the lock holder and is delivered by virtio_transport_rx_batch_finish().
> */
> struct virtio_transport_rx_batch {
> struct sock *sk;
> unsigned int pkts;
> size_t bytes;
>+ bool write_space_pending;
> };
>
> static inline bool
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index 5549298c1ec6b..b76ab565ea58b 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -63,6 +63,11 @@ struct vsock_sock {
> u32 peer_shutdown;
> bool sent_request;
> bool ignore_connecting_rst;
>+ /*
>+ * Captured before the socket is published. This identifies AF_VSOCK's
>+ * native write-space callback after a consumer has replaced it.
>+ */
>+ void (*native_write_space)(struct sock *sk);
>
> /* Protected by lock_sock(sk) */
> u64 buffer_size;
>@@ -78,6 +83,8 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk);
> s64 vsock_stream_has_space(struct vsock_sock *vsk);
> struct sock *vsock_create_connected(struct sock *parent);
> void vsock_data_ready(struct sock *sk);
>+bool vsock_write_space_is_native(const struct sock *sk);
>+void vsock_call_native_write_space(struct sock *sk);
>
> /**** TRANSPORT ****/
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 44cee74519555..e39567990539f 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -958,6 +958,7 @@ static struct sock *__vsock_create(struct net *net,
> sk->sk_type = type;
>
> vsk = vsock_sk(sk);
>+ vsk->native_write_space = sk->sk_write_space;
> vsock_addr_init(&vsk->local_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
> vsock_addr_init(&vsk->remote_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
>
>@@ -1122,6 +1123,21 @@ void vsock_data_ready(struct sock *sk)
> }
> EXPORT_SYMBOL_GPL(vsock_data_ready);
>
>+bool vsock_write_space_is_native(const struct sock *sk)
>+{
>+ const struct vsock_sock *vsk = vsock_sk(sk);
>+
>+ return READ_ONCE(sk->sk_write_space) ==
>+ READ_ONCE(vsk->native_write_space);
>+}
>+EXPORT_SYMBOL_GPL(vsock_write_space_is_native);
>+
>+void vsock_call_native_write_space(struct sock *sk)
>+{
>+ vsock_sk(sk)->native_write_space(sk);
>+}
>+EXPORT_SYMBOL_GPL(vsock_call_native_write_space);
>+
> /* Dummy callback required by sockmap.
> * See unconditional call of saved_close() in sock_map_close().
> */
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index f83e5b2edce35..af26344678e41 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1865,31 +1865,55 @@ virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
> !virtio_transport_rx_buf_full(vvs);
> }
>
>-/* The caller holds sk's socket lock. */
>+/* The caller holds sk's socket and callback locks. */
> static bool
>-virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
>- struct sock *sk, u32 len)
>+__virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
>+ struct sock *sk, u32 len)
> {
> if (!virtio_transport_rx_batch_can_continue(t, sk, len))
> return false;
>
> /*
>- * Sockmap can replace sk_prot and publish a psock without the socket lock.
>- * Check both at each packet boundary; a boundary that sees a replacement
>- * returns to the existing per-packet path.
>+ * Sockmap can replace sk_prot or sk_write_space, and publish a psock,
>+ * without the socket lock. Coalescing is safe only for the native
>+ * write-space callback, so a boundary that sees a replacement returns to
>+ * the existing per-packet path.
> */
> return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
>+ vsock_write_space_is_native(sk) &&
I'm confused, why for sk_prot we are doing the check here, while for
sk_write_space we need a new API?
> !virtio_transport_sock_has_psock(sk);
> }
>
>+/*
>+ * Sockmap teardown restores the native protocol and write-space callback
>+ * under sk_callback_lock. Serialize each packet admission check with those
>+ * updates.
>+ */
>+static bool
>+virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
>+ struct sock *sk, u32 len)
>+{
>+ bool batchable;
>+
>+ read_lock_bh(&sk->sk_callback_lock);
>+ batchable = __virtio_transport_rx_batch_can_receive(t, sk, len);
>+ read_unlock_bh(&sk->sk_callback_lock);
>+
>+ return batchable;
>+}
>+
> /*
> * The caller holds sk's socket lock. If @batchable is non-NULL, update it
> * after processing @skb to say whether a native STREAM/RW batch can continue.
>+ * When @batch is non-NULL, record a native write-space notification for batch
>+ * finish. A writer cannot consume the updated credit until this lock is
>+ * released.
> */
> static void
> virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> struct sk_buff *skb, struct sock *sk,
>- struct net *net, bool *batchable)
>+ struct net *net, bool *batchable,
>+ struct virtio_transport_rx_batch *batch)
Do we really need bot batchable and batch ?
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct vsock_sock *vsk = vsock_sk(sk);
>@@ -1921,8 +1945,12 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
> vsk->local_addr.svm_cid = le64_to_cpu(hdr->dst_cid);
>
>- if (space_available)
>- sk->sk_write_space(sk);
>+ if (space_available) {
>+ if (batch)
>+ batch->write_space_pending = true;
>+ else
>+ sk->sk_write_space(sk);
>+ }
>
> switch (sk->sk_state) {
> case TCP_LISTEN:
>@@ -1947,7 +1975,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> }
>
> if (batchable)
>- *batchable = virtio_transport_rx_batch_can_receive(t, sk, 0);
>+ *batchable = virtio_transport_rx_batch_can_continue(t, sk, 0);
Why this change?
> }
>
> /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
>@@ -1991,7 +2019,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> goto free_pkt;
>
> lock_sock(sk);
>- virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
>+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL, NULL);
>
> release_sock(sk);
>
>@@ -2008,20 +2036,31 @@ EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
>
> /*
> * Release an active RX batch. A non-empty batch holds the socket lock and the
>- * reference returned by vsock_find_connected_socket_net(). An empty batch
>- * holds neither.
>+ * reference returned by vsock_find_connected_socket_net(). Deliver any
>+ * deferred native write-space notification while holding the lock, preserving
>+ * the callback context of the regular receive path. An empty batch holds
>+ * neither.
> */
> void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
> {
>+ bool write_space_pending = batch->write_space_pending;
> struct sock *sk = batch->sk;
>
> batch->sk = NULL;
> batch->pkts = 0;
> batch->bytes = 0;
>+ batch->write_space_pending = false;
>
> if (!sk)
> return;
>
>+ /*
>+ * The packet was admitted with the native callback. Do not reload a current
>+ * callback here: sockmap can replace or remove its wrapper independently.
>+ */
>+ if (write_space_pending)
>+ vsock_call_native_write_space(sk);
>+
> release_sock(sk);
> sock_put(sk);
> }
>@@ -2080,7 +2119,8 @@ virtio_transport_rx_batch_process(struct virtio_transport *t,
> return;
> }
>
>- virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable);
>+ virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable,
>+ batch);
> if (!batchable) {
> virtio_transport_rx_batch_finish(batch);
> return;
>@@ -2158,16 +2198,8 @@ void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
> }
>
> lock_sock(sk);
>- /*
>- * Serialize initial admission with sockmap's callback updates. Later
>- * packet boundaries recheck native state and end a batch when they
>- * observe a replacement.
>- */
>- read_lock_bh(&sk->sk_callback_lock);
> batchable = virtio_transport_rx_batch_can_receive(t, sk, len);
>- read_unlock_bh(&sk->sk_callback_lock);
Can we avoid this change and do this in the previous patch?
Please avoid as much as you can change code introduced in the same series.
> if (!batchable) {
>- /* State or receive-buffer failures keep the regular lookup path. */
Why removing this comment just added by previous patch?
> if (!virtio_transport_rx_batch_can_continue(t, sk, len)) {
> release_sock(sk);
> sock_put(sk);
>@@ -2176,7 +2208,7 @@ void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
>
> virtio_transport_trace_recv_pkt(hdr, &src, &dst);
> if (virtio_transport_recv_pkt_set_owner(skb, sk))
>- virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
>+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL, NULL);
> else
> kfree_skb(skb);
>
>--
>2.53.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 5/5] vsock/virtio: defer native RX readiness notifications until batch unlock
2026-11-03 4:11 ` [PATCH net-next v5 5/5] vsock/virtio: defer native RX readiness notifications until batch unlock Jia Jia
@ 2026-11-05 9:38 ` Stefano Garzarella
2026-11-06 13:33 ` Jia Jia
0 siblings, 1 reply; 18+ messages in thread
From: Stefano Garzarella @ 2026-11-05 9:38 UTC (permalink / raw)
To: Jia Jia
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
On Tue, Nov 03, 2026 at 12:11:34PM +0800, Jia Jia wrote:
>An RX batch can make a socket readable while the RX worker retains its
>socket lock. Waking a blocked reader before release_sock() merely makes it
>wait for the same lock.
>
>P4 coalesces native sk_write_space notifications while retaining their
>under-lock callback context. This patch also coalesces native sk_data_ready
>notifications and moves both callbacks after release_sock(), so awakened
>tasks do not immediately block on the RX worker's socket lock.
>
>Save the native data-ready callback before the AF_VSOCK socket is
>published. Batch only while the native protocol and both native readiness
>callbacks are present and no psock is observed under sk_callback_lock. A
>socket with a replacement callback continues down the existing per-packet
>path.
>
>At batch finish, deliver write-space before data-ready and hold the
>connected lookup reference until both callbacks return. Packets admitted on
>the native path continue to use their saved native callbacks. Loading a
>current sockmap callback at finish would not serialize its invocation
>against sockmap teardown and could lose the native wakeup or invoke a stale
>data-ready wrapper.
>
>Extract the data-ready predicate from vsock_data_ready() so the deferred
>and regular paths share the low-watermark test.
>
>Performance:
>
>P4-P5 contribution (P1-P3 vs P1-P5):
I'm still confused on the reason why P4-P5 are not squashed.
This patch is essentially changing 90% of the previous patch...
>
>End-to-end RX throughput (64K Guest recv() size, SO_RCVLOWAT=1; Gbit/s):
>
> payload P1-P3 RX P1-P5 RX throughput change (95% CI)
> (Gbit/s) (Gbit/s)
> 64B 0.2449 0.2463 +0.55% [-3.86%, +5.17%]
> 256B 0.9649 0.9931 +2.93% [+0.83%, +5.07%]
> 512B 1.8205 1.8260 +0.30% [-4.27%, +5.10%]
> 1K 3.3431 3.3420 -0.03% [-2.15%, +2.12%]
> 4K 5.6541 5.8428 +3.34% [+1.10%, +5.63%]
> 8K 5.4280 5.3364 -1.69% [-15.55%, +14.46%]
> 16K 5.5935 5.6701 +1.37% [-1.41%, +4.23%]
> 64K 5.6917 5.8351 +2.52% [-0.20%, +5.31%]
>
>Fixed-syscall stress test (receiver recv() size = sender write size;
>Gbit/s):
>
> payload P1-P3 RX P1-P5 RX throughput change (95% CI)
> recv() calls are listed as P1-P3/P1-P5.
> 64B 0.2553 0.2575 +0.87% [-3.68%, +5.63%]
> 1048577/1048577
> 256B 0.9069 0.9183 +1.26% [-3.96%, +6.76%]
> 1048577/1048577
> 512B 1.6048 1.6459 +2.56% [+0.98%, +4.18%]
> 1048577/1048577
> 1K 2.6939 2.7357 +1.55% [-1.35%, +4.54%]
> 1048577/1048577
> 4K 4.9168 4.9678 +1.04% [-1.84%, +4.00%]
> 524965/525283
> 8K 5.1476 5.2005 +1.03% [-4.02%, +6.34%]
> 262926/263156
> 16K 5.5964 5.6609 +1.15% [-2.86%, +5.33%]
> 131311/131475
> 64K 5.9439 5.9073 -0.62% [-3.00%, +1.83%]
> 32875/32937
>
>Guest CPU1 aggregate cycles per received byte
>(fixed 64K receiver, with perf):
>
> payload P1-P3 P1-P5 change (95% CI)
> 64B 60.281 58.258 -3.36% [-8.74%, +2.35%]
> 256B 14.242 13.648 -4.17% [-9.81%, +1.82%]
> 512B 7.547 7.211 -4.45% [-9.73%, +1.13%]
> 1K 3.987 3.853 -3.35% [-8.52%, +2.12%]
> 4K 1.983 1.880 -5.17% [-7.97%, -2.27%]
> 8K 1.665 1.620 -2.75% [-9.03%, +3.96%]
> 16K 1.409 1.346 -4.52% [-10.94%, +2.37%]
> 64K 1.322 1.281 -3.11% [-6.49%, +0.38%]
>
>Full-series cycles/B (baseline vs P1-P5):
>
>Guest CPU1 aggregate cycles per received byte
>(fixed 64K receiver, with perf):
>
> payload baseline P1-P5 change (95% CI)
> 64B 65.282 57.843 -11.40% [-13.77%, -8.95%]
> 256B 14.710 13.813 -6.10% [ -7.89%, -4.28%]
> 512B 7.709 7.188 -6.75% [ -8.79%, -4.66%]
> 1K 4.075 3.862 -5.23% [ -6.80%, -3.63%]
> 4K 2.034 1.887 -7.21% [ -8.67%, -5.72%]
> 8K 1.664 1.568 -5.78% [ -7.83%, -3.69%]
> 16K 1.450 1.385 -4.48% [ -6.08%, -2.86%]
> 64K 1.338 1.317 -1.51% [ -3.21%, +0.22%]
>
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> include/linux/virtio_vsock.h | 5 ++-
> include/net/af_vsock.h | 9 +++--
> net/vmw_vsock/af_vsock.c | 27 ++++++++++---
> net/vmw_vsock/virtio_transport_common.c | 52 +++++++++++++++----------
> 4 files changed, 62 insertions(+), 31 deletions(-)
>
>Tested:
> Concurrent map-entry attach/detach during sequenced RX and duplex traffic
> passed on baseline, P1-P3, P1-P4, and P1-P5. The active SK_PASS case also
> passed on each state, with 200,000 verdict invocations. The concurrent
> SK_PASS attach/detach case completed 1,000 insert/remove pairs per state;
> its 10,000-record suffix after the final detach was verified. No kernel
> diagnostic markers were found in successful runs.
>
> Sockmap may replace sk_data_ready and sk_write_space. With non-native
> callbacks, native RX batching must be declined and the ordinary per-packet
> receive path used. The active SK_PASS case covers the condition expected to
> trigger this fallback. The test does not trace each packet's internal
> branch, so it verifies the fallback scenario without asserting that every
> packet took that branch.
>
> The duplex cases exercised write-space backpressure: the guest's nonblocking
> send reached EAGAIN while the host was not reading. After the host began
> draining data, the guest observed POLLOUT and resumed sending. Counts below
> are observed EAGAIN/POLLOUT recoveries.
>
> Test / result baseline P1-P3 P1-P4 P1-P5
> No-verdict RX churn: attach/detach pairs 433493 455507 430985 380351
> No-verdict duplex: attach/detach pairs 229793 212410 214198 148755
> No-verdict duplex: EAGAIN/POLLOUT 365/365 266/266 264/264 443/443
> Active SK_PASS: EAGAIN/POLLOUT 2548/2548 3013/3013 3093/3093 4208/4208
> SK_PASS attach/detach: intervals with verdict progress 705 616 631 692
> SK_PASS attach/detach: verdict invocations 99747 99894 97475 82409
> Post-detach SK_PASS suffix 10,000/10,000 records per run
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index bf28e8e32e280..2aa0e81e655d5 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -287,14 +287,15 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
> * lock and the reference returned by vsock_find_connected_socket_net(). The
> * caller retains both until it calls virtio_transport_rx_batch_finish(). An
>- * empty batch owns neither. A pending native write-space notification belongs
>- * to the lock holder and is delivered by virtio_transport_rx_batch_finish().
>+ * empty batch owns neither. Pending native readiness notifications belong to
>+ * the lock holder and are delivered by virtio_transport_rx_batch_finish().
> */
> struct virtio_transport_rx_batch {
> struct sock *sk;
> unsigned int pkts;
> size_t bytes;
> bool write_space_pending;
>+ bool data_ready_pending;
> };
>
> static inline bool
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index b76ab565ea58b..57ee545fdb496 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -64,9 +64,10 @@ struct vsock_sock {
> bool sent_request;
> bool ignore_connecting_rst;
> /*
>- * Captured before the socket is published. This identifies AF_VSOCK's
>- * native write-space callback after a consumer has replaced it.
>+ * Captured before the socket is published. These identify AF_VSOCK's
>+ * native readiness callbacks after a consumer has replaced them.
> */
>+ void (*native_data_ready)(struct sock *sk);
> void (*native_write_space)(struct sock *sk);
>
> /* Protected by lock_sock(sk) */
>@@ -83,7 +84,9 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk);
> s64 vsock_stream_has_space(struct vsock_sock *vsk);
> struct sock *vsock_create_connected(struct sock *parent);
> void vsock_data_ready(struct sock *sk);
>-bool vsock_write_space_is_native(const struct sock *sk);
>+bool vsock_data_ready_pending(struct sock *sk);
>+bool vsock_callbacks_are_native(const struct sock *sk);
>+void vsock_call_native_data_ready(struct sock *sk);
> void vsock_call_native_write_space(struct sock *sk);
>
> /**** TRANSPORT ****/
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index e39567990539f..0617073e99ed8 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -958,6 +958,7 @@ static struct sock *__vsock_create(struct net *net,
> sk->sk_type = type;
>
> vsk = vsock_sk(sk);
>+ vsk->native_data_ready = sk->sk_data_ready;
> vsk->native_write_space = sk->sk_write_space;
> vsock_addr_init(&vsk->local_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
> vsock_addr_init(&vsk->remote_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
>@@ -1113,24 +1114,38 @@ s64 vsock_stream_has_space(struct vsock_sock *vsk)
> }
> EXPORT_SYMBOL_GPL(vsock_stream_has_space);
>
>-void vsock_data_ready(struct sock *sk)
>+bool vsock_data_ready_pending(struct sock *sk)
> {
> struct vsock_sock *vsk = vsock_sk(sk);
>
>- if (vsock_stream_has_data(vsk) >= sk->sk_rcvlowat ||
>- sock_flag(sk, SOCK_DONE))
>+ return vsock_stream_has_data(vsk) >= sk->sk_rcvlowat ||
>+ sock_flag(sk, SOCK_DONE);
>+}
>+EXPORT_SYMBOL_GPL(vsock_data_ready_pending);
>+
>+void vsock_data_ready(struct sock *sk)
>+{
>+ if (vsock_data_ready_pending(sk))
> sk->sk_data_ready(sk);
> }
> EXPORT_SYMBOL_GPL(vsock_data_ready);
>
>-bool vsock_write_space_is_native(const struct sock *sk)
>+bool vsock_callbacks_are_native(const struct sock *sk)
> {
> const struct vsock_sock *vsk = vsock_sk(sk);
>
>- return READ_ONCE(sk->sk_write_space) ==
>+ return READ_ONCE(sk->sk_data_ready) ==
>+ READ_ONCE(vsk->native_data_ready) &&
>+ READ_ONCE(sk->sk_write_space) ==
> READ_ONCE(vsk->native_write_space);
> }
>-EXPORT_SYMBOL_GPL(vsock_write_space_is_native);
>+EXPORT_SYMBOL_GPL(vsock_callbacks_are_native);
>+
>+void vsock_call_native_data_ready(struct sock *sk)
>+{
>+ vsock_sk(sk)->native_data_ready(sk);
>+}
>+EXPORT_SYMBOL_GPL(vsock_call_native_data_ready);
>
> void vsock_call_native_write_space(struct sock *sk)
> {
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index af26344678e41..9d02e81042c21 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1598,7 +1598,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>
> static int
> virtio_transport_recv_connected(struct sock *sk,
>- struct sk_buff *skb)
>+ struct sk_buff *skb,
>+ bool *data_ready_pending)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct vsock_sock *vsk = vsock_sk(sk);
>@@ -1617,7 +1618,12 @@ virtio_transport_recv_connected(struct sock *sk,
> vsock_remove_sock(vsk);
> break;
> }
>- vsock_data_ready(sk);
>+ if (data_ready_pending) {
>+ if (!*data_ready_pending &&
>vsock_data_ready_pending(sk))
>+ *data_ready_pending = true;
>+ } else {
>+ vsock_data_ready(sk);
>+ }
> return err;
> case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> virtio_transport_send_credit_update(vsk);
>@@ -1874,20 +1880,19 @@ __virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> return false;
>
> /*
>- * Sockmap can replace sk_prot or sk_write_space, and publish a psock,
>- * without the socket lock. Coalescing is safe only for the native
>- * write-space callback, so a boundary that sees a replacement returns to
>- * the existing per-packet path.
>+ * Sockmap can replace sk_prot or either readiness callback, and publish a
>+ * psock, without the socket lock. Coalescing is safe only for native
>+ * callbacks, so a boundary that sees a replacement returns to the existing
>+ * per-packet path.
> */
> return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
>- vsock_write_space_is_native(sk) &&
>+ vsock_callbacks_are_native(sk) &&
> !virtio_transport_sock_has_psock(sk);
> }
>
> /*
>- * Sockmap teardown restores the native protocol and write-space callback
>- * under sk_callback_lock. Serialize each packet admission check with those
>- * updates.
>+ * Sockmap teardown restores the native protocol and callbacks under
>+ * sk_callback_lock. Serialize each packet admission check with those updates.
> */
> static bool
> virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
>@@ -1905,9 +1910,8 @@ virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> /*
> * The caller holds sk's socket lock. If @batchable is non-NULL, update it
> * after processing @skb to say whether a native STREAM/RW batch can continue.
>- * When @batch is non-NULL, record a native write-space notification for batch
>- * finish. A writer cannot consume the updated credit until this lock is
>- * released.
>+ * When @batch is non-NULL, record native readiness notifications for batch
>+ * finish. A reader or writer cannot make progress until this lock is released.
> */
> static void
> virtio_transport_recv_pkt_locked(struct virtio_transport *t,
>@@ -1917,6 +1921,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct vsock_sock *vsk = vsock_sk(sk);
>+ bool *data_ready_pending;
> struct sockaddr_vm src;
> bool space_available;
>
>@@ -1926,6 +1931,8 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> if (batchable)
> *batchable = false;
>
>+ data_ready_pending = batch ? &batch->data_ready_pending : NULL;
Can we just pass `batch` to virtio_transport_recv_connected and avoid
all of this complexity?
>+
> /*
> * The socket may have been closed or reassigned after lookup but before
> * its socket lock was acquired. Listener sockets have no assigned
>@@ -1962,7 +1969,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> kfree_skb(skb);
> break;
> case TCP_ESTABLISHED:
>- virtio_transport_recv_connected(sk, skb);
>+ virtio_transport_recv_connected(sk, skb, data_ready_pending);
> break;
> case TCP_CLOSING:
> virtio_transport_recv_disconnecting(sk, skb);
>@@ -2036,32 +2043,37 @@ EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
>
> /*
> * Release an active RX batch. A non-empty batch holds the socket lock and the
>- * reference returned by vsock_find_connected_socket_net(). Deliver any
>- * deferred native write-space notification while holding the lock, preserving
>- * the callback context of the regular receive path. An empty batch holds
>- * neither.
>+ * reference returned by vsock_find_connected_socket_net(). Release the lock,
>+ * deliver deferred native write-space and data-ready notifications in that
>+ * order, then drop the reference. An empty batch holds neither.
> */
Really, avoid comments that just describe what a function is doing. They
really don't add much value and we need to update them every time, like
this...
Comments should explain reasons, hidden behaviours, etc.
Something unclear just looking at the code.
> void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
> {
> bool write_space_pending = batch->write_space_pending;
>+ bool data_ready_pending = batch->data_ready_pending;
> struct sock *sk = batch->sk;
>
> batch->sk = NULL;
> batch->pkts = 0;
> batch->bytes = 0;
> batch->write_space_pending = false;
>+ batch->data_ready_pending = false;
>
> if (!sk)
> return;
>
Here should be nice to have a comment to explain WHY it's safe to
release in this new position...
>+ release_sock(sk);
>+
> /*
>- * The packet was admitted with the native callback. Do not reload a current
>+ * Packets were admitted with native callbacks. Do not reload a current
Why this change?
Sorry, this series is really hard to review :-(
> * callback here: sockmap can replace or remove its wrapper independently.
> */
> if (write_space_pending)
> vsock_call_native_write_space(sk);
>
>- release_sock(sk);
>+ if (data_ready_pending)
>+ vsock_call_native_data_ready(sk);
>+
> sock_put(sk);
> }
> EXPORT_SYMBOL_GPL(virtio_transport_rx_batch_finish);
>--
>2.53.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets
2026-11-05 8:57 ` Stefano Garzarella
@ 2026-11-06 12:27 ` Jia Jia
0 siblings, 0 replies; 18+ messages in thread
From: Jia Jia @ 2026-11-06 12:27 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
>
> On Tue, Nov 03, 2026 at 12:11:32PM +0800, Jia Jia wrote:
> >The virtio-vsock RX worker looks up a socket and takes its lock for every
> >packet. Retain the connected lookup reference and socket lock across a
> >bounded run of STREAM/RW packets for one established native socket.
> >
> >Reuse the locked socket while packets match the same flow. Finish the
> >batch on a malformed packet, flow change, non-RW traffic, or a change to
> >the socket state, transport, protocol, sockmap state, or receive buffer
> >limits. A connected lookup miss falls back to the existing receive path,
> >which can still find a bound socket.
> >
> >Limit an uninterrupted socket-lock ownership interval to 64 packets and
> >64 KiB of payload. The payload limit is one
> >VIRTIO_VSOCK_MAX_PKT_BUF_SIZE. Small or zero-length packets still incur
> >per-packet work, so an independent packet limit is needed. The 64-packet
> >limit uses VHOST_NET_BATCH as a reference; it is a policy choice rather
> >than a protocol limit.
> >
> >Beyond these limits, a larger batch would only further amortize socket
> >lookup and lock/unlock operations, while virtqueue work, validation,
> >tracing, skb ownership, credit update, and queueing remain per-packet. I
> >therefore expect diminishing returns while the uninterrupted socket-lock
> >interval continues to grow, but I have not measured larger limits yet.
> >
> >Only the guest virtio RX worker is changed. The benchmark exercises the
> >host-sender-to-guest-receiver direction, where this worker is on the
> >receive path. I have not measured the corresponding worker and socket-lock
> >overhead in vhost-vsock or vsock_loopback, and they use different worker
> >and locking models. Leave those paths unchanged pending separate policy
> >and validation. I plan to evaluate vhost-vsock separately as follow-up
> >work.
> >
> >Performance:
> >
> >These results cover P1-P3, before the notification changes in P4 and P5.
> >
> >Tested on a single-stream vsock connection (one Guest receiver thread on
> >CPU0, virtio-vsock IRQ on Guest CPU1, fresh QEMU Guest per state). The host
> >uses vhost-vsock.
> >
> >For throughput, vsock_perf runs on the host as the
> >sender, with a custom single-threaded Guest receiver pinned to Guest CPU0
> >using blocking recv(). The fixed-64K table uses a 64K buffer; the matched
> >table uses the sender's request size. The virtio-vsock IRQ/RX worker is
> >pinned to Guest CPU1. To normalize run times across payloads into a
> >bounded, deterministic window and avoid long runs being skewed by host
> >background noise or virtualization scheduling jitter, transfer sizes scale
> >proportionally with the request size (64B/64M, 256B/256M, etc.), capped at
> >2G for 4K-64K. The custom receiver keeps the recv() size and count
> >explicit; vsock_perf's receiver uses poll()+read().
> >
> >End-to-end RX throughput (fixed-64K receiver; Gbit/s):
> >
> > payload baseline RX PATCH RX throughput change (95% CI)
> > (Gbit/s) (Gbit/s)
> > 64B 0.1285 0.2462 +91.59% [+79.74%, +104.22%]
> > 256B 0.5909 0.9666 +63.59% [+61.03%, +66.19%]
> > 512B 1.1817 1.8350 +55.29% [+46.76%, +64.32%]
> > 1K 2.3252 3.3600 +44.50% [+38.93%, +50.30%]
> > 4K 3.8436 5.5661 +44.82% [+40.30%, +49.47%]
> > 8K 4.6609 5.4115 +16.10% [ +8.91%, +23.77%]
> > 16K 5.1196 5.6399 +10.16% [ +4.07%, +16.61%]
> > 64K 5.6095 5.7062 +1.72% [-3.19%, +6.89%]
> >
> >Fixed-syscall stress test (receiver recv() size = sender write size;
> >no perf; Gbit/s):
> >
> > payload baseline RX PATCH RX throughput change (95% CI)
> > 64B 0.1446 0.2540 +75.71% [+63.67%, +88.62%]
> > recv() calls (baseline/PATCH): 1048577/1048577
> > 256B 0.4211 0.9061 +115.18% [+72.13%, +169.00%]
> > recv() calls (baseline/PATCH): 1048577/1048577
> > 512B 0.7980 1.6112 +101.90% [+95.30%, +108.72%]
> > recv() calls (baseline/PATCH): 1048577/1048577
> > 1K 1.4965 2.6986 +80.33% [+68.93%, +92.49%]
> > recv() calls (baseline/PATCH): 1048577/1048577
> > 4K 2.9384 4.8590 +65.36% [+60.17%, +70.72%]
> > recv() calls (baseline/PATCH): 539544/524982
> > 8K 3.8170 5.1887 +35.94% [+27.66%, +44.75%]
> > recv() calls (baseline/PATCH): 274857/262991
> > 16K 4.6856 5.5996 +19.51% [+15.36%, +23.81%]
> > recv() calls (baseline/PATCH): 147518/131351
> > 64K 5.5182 5.8800 +6.56% [-0.83%, +14.49%]
> > recv() calls (baseline/PATCH): 48492/32910
> >
> >Guest CPU1 aggregate cycles per received byte
> >(fixed 64K receiver, with perf):
> >
> > Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
> > 64B 66.728 61.220 -8.25% [-18.99%, +3.91%]
> > 256B 14.363 14.515 +1.06% [ -4.22%, +6.64%]
> > 512B 7.792 7.578 -2.74% [ -6.55%, +1.23%]
> > 1K 3.988 4.032 +1.09% [ -1.64%, +3.88%]
> > 4K 2.063 1.973 -4.38% [ -8.23%, -0.37%]
> > 8K 1.702 1.667 -2.06% [ -7.99%, +4.25%]
> > 16K 1.473 1.475 +0.17% [ -5.06%, +5.68%]
> > 64K 1.385 1.358 -1.96% [-10.61%, +7.53%]
> >
> >Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> >---
> > include/linux/virtio_vsock.h | 23 ++
> > net/vmw_vsock/virtio_transport.c | 7 +-
> > net/vmw_vsock/virtio_transport_common.c | 303 ++++++++++++++++++++++--
> > 3 files changed, 318 insertions(+), 15 deletions(-)
> >
> >diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> >index f91704731057e..748633aa0cc81 100644
> >--- a/include/linux/virtio_vsock.h
> >+++ b/include/linux/virtio_vsock.h
> >@@ -282,6 +282,29 @@ void virtio_transport_destruct(struct vsock_sock *vsk);
> >
> > void virtio_transport_recv_pkt(struct virtio_transport *t,
> > struct sk_buff *skb, struct net *net);
> >+
> >+/*
> >+ * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
> >+ * lock and the reference returned by vsock_find_connected_socket_net(). The
> >+ * caller retains both until it calls virtio_transport_rx_batch_finish(). An
> >+ * empty batch owns neither.
> >+ */
> >+struct virtio_transport_rx_batch {
> >+ struct sock *sk;
> >+ unsigned int pkts;
> >+ size_t bytes;
> >+};
> >+
> >+static inline bool
> >+virtio_transport_rx_batch_empty(const struct virtio_transport_rx_batch *batch)
> >+{
> >+ return !batch->sk;
> >+}
> >+
> >+void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
> >+ struct sk_buff *skb, struct net *net,
> >+ struct virtio_transport_rx_batch *batch);
> >+void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch);
> > void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
> > u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> > void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
> >diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> >index 4f9aa9c4c3aa5..9af85712bd387 100644
> >--- a/net/vmw_vsock/virtio_transport.c
> >+++ b/net/vmw_vsock/virtio_transport.c
> >@@ -631,6 +631,7 @@ virtio_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
> >
> > static void virtio_transport_rx_work(struct work_struct *work)
> > {
> > struct virtio_vsock *vsock =
> > container_of(work, struct virtio_vsock, rx_work);
> >+ struct virtio_transport_rx_batch batch = {};
> > struct virtqueue *vq;
> >@@ -666,6 +667,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
> > /* Drop short/long packets */
> > if (unlikely(len < sizeof(*hdr) ||
> > len > virtio_vsock_skb_len(skb))) {
> >+ virtio_transport_rx_batch_finish(&batch);
> > kfree_skb(skb);
> > continue;
> > }
> >@@ -673,6 +675,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
> > hdr = virtio_vsock_hdr(skb);
> > payload_len = le32_to_cpu(hdr->len);
> > if (unlikely(payload_len > len - sizeof(*hdr))) {
> >+ virtio_transport_rx_batch_finish(&batch);
> > kfree_skb(skb);
> > continue;
> > }
> >@@ -685,11 +688,13 @@ static void virtio_transport_rx_work(struct work_struct *work)
> > /* Force virtio-transport into global mode since it
> > * does not yet support local-mode namespacing.
> > */
> >- virtio_transport_recv_pkt(&virtio_transport, skb, NULL);
> >+ virtio_transport_recv_pkt_batch(&virtio_transport, skb,
> >+ NULL, &batch);
> > }
> > } while (!virtqueue_enable_cb(vq));
> >
> > out:
> >+ virtio_transport_rx_batch_finish(&batch);
> > if (vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2)
> > virtio_vsock_rx_fill(vsock);
> > out_nofill:
> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >index 2a9cfed085515..f83e5b2edce35 100644
> >--- a/net/vmw_vsock/virtio_transport_common.c
> >+++ b/net/vmw_vsock/virtio_transport_common.c
> >@@ -33,6 +33,15 @@
> > #define MAX_COLLAPSE_LEN \
> > SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER)
> >
> >+/*
> >+ * Bound the interval that an RX worker owns a socket lock. One maximum packet
> >+ * buffer (64 KiB) bounds payload in one uninterrupted ownership interval.
> >+ * Small and zero-length packets need a packet limit to bound per-packet work;
> >+ * the 64-packet policy uses VHOST_NET_BATCH as a reference.
> >+ */
> >+#define VIRTIO_TRANSPORT_RX_BATCH_MAX_PKTS 64
> >+#define VIRTIO_TRANSPORT_RX_BATCH_MAX_BYTES VIRTIO_VSOCK_MAX_PKT_BUF_SIZE
> >+
> > static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
> > bool cancel_timeout);
> > static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
> >@@ -1793,13 +1802,94 @@ virtio_transport_find_socket(struct sockaddr_vm *src,
> > return sk;
> > }
> >
> >+static void
> >+virtio_transport_trace_recv_pkt(const struct virtio_vsock_hdr *hdr,
> >+ const struct sockaddr_vm *src,
> >+ const struct sockaddr_vm *dst)
> >+{
> >+ trace_virtio_transport_recv_pkt(src->svm_cid, src->svm_port,
> >+ dst->svm_cid, dst->svm_port,
> >+ le32_to_cpu(hdr->len),
> >+ le16_to_cpu(hdr->type),
> >+ le16_to_cpu(hdr->op),
> >+ le32_to_cpu(hdr->flags),
> >+ le32_to_cpu(hdr->buf_alloc),
> >+ le32_to_cpu(hdr->fwd_cnt));
> >+}
> >+
> >+static bool
> >+virtio_transport_recv_pkt_set_owner(struct sk_buff *skb, struct sock *sk)
> >+{
> >+ if (skb_set_owner_sk_safe(skb, sk))
> >+ return true;
> >+
> >+ WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
> >+ return false;
> >+}
> >+
> >+static bool
> >+virtio_transport_sock_has_psock(const struct sock *sk)
> >+{
> >+ return (uintptr_t)rcu_access_pointer(__sk_user_data(sk)) &
> >+ SK_USER_DATA_PSOCK;
> >+}
> >+
> >+/* The caller holds sk's socket lock. */
> >+static bool
> >+virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
> >+ struct sock *sk, u32 len)
> >+{
> >+ struct virtio_vsock_sock *vvs;
> >+ struct vsock_sock *vsk;
> >+
> >+ vsk = vsock_sk(sk);
> >+ vvs = vsk->trans;
> >+
> >+ /*
> >+ * Keep a connected lookup reference only for the native, established
> >+ * STREAM path. Sockets outside ESTABLISHED can change table membership;
> >+ * a completed or reassigned socket must resume the per-packet path.
> >+ */
> >+ if (sk->sk_state != TCP_ESTABLISHED ||
> >+ sk->sk_type != SOCK_STREAM ||
> >+ sock_flag(sk, SOCK_DONE) ||
> >+ vsk->transport != &t->transport || !vvs)
> >+ return false;
> >+
> >+ /*
> >+ * Do not admit a packet that would hit an existing RX limit while the
> >+ * worker owns the socket lock. The caller holds that lock, so no reader can
> >+ * dequeue until the batch is released.
> >+ */
> >+ return (u64)vvs->buf_used + len <= vvs->buf_alloc &&
> >+ !virtio_transport_rx_buf_full(vvs);
>
Hi Stefano,
> I don't understand this. We recently introduce the collapse, so when we
> will call virtio_transport_recv_enqueue(), all the queue will be
> collapsed, including packets in the batch.
>
> Or am I missing something?
>
Yes, collapse operates on the same receive queue, which also contains
skbs queued earlier in the current batch.
The rx_buf_full() check is not intended to bypass collapse. It prevents
the batch from retaining the socket lock while processing a packet when
there is no metadata room for another skb. Collapse is best effort:
some skbs cannot be merged, and allocation of a replacement skb can
fail. If the packet remained in the batch and collapse did not make
enough room, virtio_transport_inc_rx_pkt() would fail and
virtio_transport_recv_connected() would reset the socket with ENOBUFS
while a waiting reader was still unable to drain the queue.
When the check makes can_continue() return false before the current
packet has been handled, the caller releases the socket lock and passes
the packet to virtio_transport_recv_pkt(). This gives a waiting reader a
chance to make progress. If the queue is still at the metadata limit,
the normal enqueue path attempts collapse. If that still cannot make
enough room, the existing ENOBUFS reset occurs.
Thanks,
Jia
> Thanks,
> Stefano
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 2/5] vsock/virtio: factor RX skb metadata budget check
2026-11-05 9:06 ` Stefano Garzarella
@ 2026-11-06 12:28 ` Jia Jia
0 siblings, 0 replies; 18+ messages in thread
From: Jia Jia @ 2026-11-06 12:28 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
>
> On Tue, Nov 03, 2026 at 12:11:31PM +0800, Jia Jia wrote:
> >Extract the skb metadata budget check from virtio_transport_inc_rx_pkt()
> >into virtio_transport_rx_buf_full().
> >
> >The helper retains the existing condition and will be reused by a
> >following RX batching change.
> >
> >Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> >---
> > net/vmw_vsock/virtio_transport_common.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >index 43325d7aebbb1..2a9cfed085515 100644
> >--- a/net/vmw_vsock/virtio_transport_common.c
> >+++ b/net/vmw_vsock/virtio_transport_common.c
> >@@ -576,11 +576,17 @@ virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs,
> > skb_queue_splice(&new_queue, &vvs->rx_queue);
> > }
> >
> >-static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> >- u32 len)
> >+static bool
> >+virtio_transport_rx_buf_full(struct virtio_vsock_sock *vvs)
> > {
> > u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
> >
> >+ return skb_overhead > vvs->buf_alloc;
> >+}
>
> As I pointed out in patch 3, I'm not sure we need this.
> After we introduced the collapse, SOCK_STREAM should collapse rx_queue
> before hitting this, so not sure we really need this function.
>
> Thanks,
> Stefano
>
This patch only factors out the existing metadata check from
virtio_transport_inc_rx_pkt() into virtio_transport_rx_buf_full() so
can_continue() in patch 3 can reuse it.
virtio_transport_recv_enqueue() attempts collapse when the queue is
already at this limit, and then inc_rx_pkt() runs the check again.
Collapse may not free enough slots: some skbs cannot be merged, and
allocation of a replacement skb can fail. As explained in the patch 3
reply, when virtio_transport_rx_buf_full() is true before the packet is
handled, the caller releases the socket lock. A waiting reader then has
a chance to drain before that packet reaches
virtio_transport_recv_enqueue().
> >+
> >+static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> >+ u32 len)
> >+{
> > /* Allow at most buf_alloc * 2 total budget (payload + overhead),
> > * similar to how SO_RCVBUF is doubled to reserve space for sk_buff
> > * metadata. Check payload against buf_alloc to be sure the other
> >@@ -588,7 +594,7 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> > * queue growth.
> > */
> > if ((u64)vvs->buf_used + len > vvs->buf_alloc ||
> >- skb_overhead > vvs->buf_alloc)
> >+ virtio_transport_rx_buf_full(vvs))
> > return false;
> >
> > vvs->rx_bytes += len;
> >--
> >2.53.0
> >
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications
2026-11-05 9:21 ` Stefano Garzarella
@ 2026-11-06 12:49 ` Jia Jia
0 siblings, 0 replies; 18+ messages in thread
From: Jia Jia @ 2026-11-06 12:49 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
>
> On Tue, Nov 03, 2026 at 12:11:33PM +0800, Jia Jia wrote:
> >Each received packet can update peer credit and call sk_write_space(). A
> >writer cannot consume that credit while the virtio RX worker retains the
> >socket lock, so repeated native write-space notifications in one RX batch
> >are unnecessary.
> >
> >This patch coalesces write-space notifications on the native AF_VSOCK path;
> >data-ready remains per-packet here.
> >
> >The batching patch establishes batch ownership before its first packet.
> >This lets this patch record a pending notification for that packet and
> >flush it even if the packet immediately ends the batch. Deliver it while
> >holding the socket lock to preserve the callback context of the existing
> >receive path. A following patch changes that timing together with
> >data-ready handling.
>
> Can you report performance data related to this patch also here?
>
> IIUC this is not giving a lot of advantage, so my question is if this
> more complexity really add a value for us.
>
Sure. The performance tests currently cover patches 4 and 5 together,
so I will include those results here.
They do not add another broad throughput improvement. I kept the
combined notification change because of the cycles/byte results.
With P1-P3 only, against baseline:
Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
64B 66.728 61.220 -8.25% [-18.99%, +3.91%]
256B 14.363 14.515 +1.06% [ -4.22%, +6.64%]
512B 7.792 7.578 -2.74% [ -6.55%, +1.23%]
1K 3.988 4.032 +1.09% [ -1.64%, +3.88%]
4K 2.063 1.973 -4.38% [ -8.23%, -0.37%]
8K 1.702 1.667 -2.06% [ -7.99%, +4.25%]
16K 1.473 1.475 +0.17% [ -5.06%, +5.68%]
64K 1.385 1.358 -1.96% [-10.61%, +7.53%]
Only the 4K interval excluded zero. With the full series, against
baseline:
Buffer size baseline cycles/B PATCH cycles/B cycles change (95% CI)
64B 65.282 57.843 -11.40% [-13.77%, -8.95%]
256B 14.710 13.813 -6.10% [ -7.89%, -4.28%]
512B 7.709 7.188 -6.75% [ -8.79%, -4.66%]
1K 4.075 3.862 -5.23% [ -6.80%, -3.63%]
4K 2.034 1.887 -7.21% [ -8.67%, -5.72%]
8K 1.664 1.568 -5.78% [ -7.83%, -3.69%]
16K 1.450 1.385 -4.48% [ -6.08%, -2.86%]
64K 1.338 1.317 -1.51% [ -3.21%, +0.22%]
From 64B through 16K those intervals stay below zero. 64B through 1K
did not show that in the P1-P3 run.
This is also why I spent the time making this work with sockmap.
The value I see is lower RX CPU cost, not another large throughput
gain. I am still unsure the extra code is worth it, and I would like
your opinion.
Thanks,
Jia
> >
> >Capture the native write-space callback before socket publication and
> >access it through AF_VSOCK helpers. Sockmap can change the protocol,
> >write-space callback, and psock marker without the socket lock. Serialize
> >each batch eligibility check with sk_callback_lock; if a packet boundary
> >observes a replacement, finish the batch and use the regular per-packet
> >path.
> >
> >Packets admitted on the native path use the saved native callback at batch
> >finish. Reloading a current sockmap callback would not synchronize its
> >invocation with teardown; a wrapper seen before teardown need not forward
> >the native wakeup after its psock is gone.
>
> I really don't understand the goal of this patch, and why we need to
> store the "native" sk_write_space.
>
Actually, I kept this as a separate patch so the two notification changes
could be tested separately. This patch provides a complete intermediate
state: it coalesces native sk_write_space() notifications while keeping the
callback invocation under the socket lock. Patch 5 then coalesces
sk_data_ready() and moves both deferred callbacks after release_sock().
The coalescing is limited to packets for which can_receive() sees
sk_write_space still set to the native callback. That callback wakes waiters.
The sockmap wrapper can also schedule psock work before forwarding
the saved callback, so I do not coalesce it.
Batch finish also needs the saved pointer if the callback changes before
the batch ends. If another packet arrives, can_receive() detects the
replacement
and its caller finishes the batch. A write-space notification already
recorded for
the batch still uses the native callback.
Reloading sk->sk_write_space at finish could capture the sockmap wrapper just
before teardown. If teardown removes the psock before the callback is invoked,
the wrapper no longer forwards the native wakeup.
> >
> >Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> >---
> > include/linux/virtio_vsock.h | 4 +-
> > include/net/af_vsock.h | 7 +++
> > net/vmw_vsock/af_vsock.c | 16 +++++
> > net/vmw_vsock/virtio_transport_common.c | 78 +++++++++++++++++--------
> > 4 files changed, 81 insertions(+), 24 deletions(-)
> >
> >diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> >index 748633aa0cc81..bf28e8e32e280 100644
> >--- a/include/linux/virtio_vsock.h
> >+++ b/include/linux/virtio_vsock.h
> >@@ -287,12 +287,14 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> > * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
> > * lock and the reference returned by vsock_find_connected_socket_net(). The
> > * caller retains both until it calls virtio_transport_rx_batch_finish(). An
> >- * empty batch owns neither.
> >+ * empty batch owns neither. A pending native write-space notification belongs
> >+ * to the lock holder and is delivered by virtio_transport_rx_batch_finish().
> > */
> > struct virtio_transport_rx_batch {
> > struct sock *sk;
> > unsigned int pkts;
> > size_t bytes;
> >+ bool write_space_pending;
> > };
> >
> > static inline bool
> >diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> >index 5549298c1ec6b..b76ab565ea58b 100644
> >--- a/include/net/af_vsock.h
> >+++ b/include/net/af_vsock.h
> >@@ -63,6 +63,11 @@ struct vsock_sock {
> > u32 peer_shutdown;
> > bool sent_request;
> > bool ignore_connecting_rst;
> >+ /*
> >+ * Captured before the socket is published. This identifies AF_VSOCK's
> >+ * native write-space callback after a consumer has replaced it.
> >+ */
> >+ void (*native_write_space)(struct sock *sk);
> >
> > /* Protected by lock_sock(sk) */
> > u64 buffer_size;
> >@@ -78,6 +83,8 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk);
> > s64 vsock_stream_has_space(struct vsock_sock *vsk);
> > struct sock *vsock_create_connected(struct sock *parent);
> > void vsock_data_ready(struct sock *sk);
> >+bool vsock_write_space_is_native(const struct sock *sk);
> >+void vsock_call_native_write_space(struct sock *sk);
> >
> > /**** TRANSPORT ****/
> >
> >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> >index 44cee74519555..e39567990539f 100644
> >--- a/net/vmw_vsock/af_vsock.c
> >+++ b/net/vmw_vsock/af_vsock.c
> >@@ -958,6 +958,7 @@ static struct sock *__vsock_create(struct net *net,
> > sk->sk_type = type;
> >
> > vsk = vsock_sk(sk);
> >+ vsk->native_write_space = sk->sk_write_space;
> > vsock_addr_init(&vsk->local_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
> > vsock_addr_init(&vsk->remote_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
> >
> >@@ -1122,6 +1123,21 @@ void vsock_data_ready(struct sock *sk)
> > }
> > EXPORT_SYMBOL_GPL(vsock_data_ready);
> >
> >+bool vsock_write_space_is_native(const struct sock *sk)
> >+{
> >+ const struct vsock_sock *vsk = vsock_sk(sk);
> >+
> >+ return READ_ONCE(sk->sk_write_space) ==
> >+ READ_ONCE(vsk->native_write_space);
> >+}
> >+EXPORT_SYMBOL_GPL(vsock_write_space_is_native);
> >+
> >+void vsock_call_native_write_space(struct sock *sk)
> >+{
> >+ vsock_sk(sk)->native_write_space(sk);
> >+}
> >+EXPORT_SYMBOL_GPL(vsock_call_native_write_space);
> >+
> > /* Dummy callback required by sockmap.
> > * See unconditional call of saved_close() in sock_map_close().
> > */
> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >index f83e5b2edce35..af26344678e41 100644
> >--- a/net/vmw_vsock/virtio_transport_common.c
> >+++ b/net/vmw_vsock/virtio_transport_common.c
> >@@ -1865,31 +1865,55 @@ virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
> > !virtio_transport_rx_buf_full(vvs);
> > }
> >
> >-/* The caller holds sk's socket lock. */
> >+/* The caller holds sk's socket and callback locks. */
> > static bool
> >-virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> >- struct sock *sk, u32 len)
> >+__virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> >+ struct sock *sk, u32 len)
> > {
> > if (!virtio_transport_rx_batch_can_continue(t, sk, len))
> > return false;
> >
> > /*
> >- * Sockmap can replace sk_prot and publish a psock without the socket lock.
> >- * Check both at each packet boundary; a boundary that sees a replacement
> >- * returns to the existing per-packet path.
> >+ * Sockmap can replace sk_prot or sk_write_space, and publish a psock,
> >+ * without the socket lock. Coalescing is safe only for the native
> >+ * write-space callback, so a boundary that sees a replacement returns to
> >+ * the existing per-packet path.
> > */
> > return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
> >+ vsock_write_space_is_native(sk) &&
>
> I'm confused, why for sk_prot we are doing the check here, while for
> sk_write_space we need a new API?
>
sk_prot can be compared with sk_prot_creator. sk_write_space
has no field like that, and sock_def_write_space() is static.
I save the callback in __vsock_create(), after the socket is
initialized and before it is published. The helpers compare
against that pointer and call it.
> > !virtio_transport_sock_has_psock(sk);
> > }
> >
> >+/*
> >+ * Sockmap teardown restores the native protocol and write-space callback
> >+ * under sk_callback_lock. Serialize each packet admission check with those
> >+ * updates.
> >+ */
> >+static bool
> >+virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> >+ struct sock *sk, u32 len)
> >+{
> >+ bool batchable;
> >+
> >+ read_lock_bh(&sk->sk_callback_lock);
> >+ batchable = __virtio_transport_rx_batch_can_receive(t, sk, len);
> >+ read_unlock_bh(&sk->sk_callback_lock);
> >+
> >+ return batchable;
> >+}
> >+
> > /*
> > * The caller holds sk's socket lock. If @batchable is non-NULL, update it
> > * after processing @skb to say whether a native STREAM/RW batch can continue.
> >+ * When @batch is non-NULL, record a native write-space notification for batch
> >+ * finish. A writer cannot consume the updated credit until this lock is
> >+ * released.
> > */
> > static void
> > virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > struct sk_buff *skb, struct sock *sk,
> >- struct net *net, bool *batchable)
> >+ struct net *net, bool *batchable,
> >+ struct virtio_transport_rx_batch *batch)
>
> Do we really need bot batchable and batch ?
>
Agreed, we don't. I'll clean that up in the next version. Thanks
> > {
> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > struct vsock_sock *vsk = vsock_sk(sk);
> >@@ -1921,8 +1945,12 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
> > vsk->local_addr.svm_cid = le64_to_cpu(hdr->dst_cid);
> >
> >- if (space_available)
> >- sk->sk_write_space(sk);
> >+ if (space_available) {
> >+ if (batch)
> >+ batch->write_space_pending = true;
> >+ else
> >+ sk->sk_write_space(sk);
> >+ }
> >
> > switch (sk->sk_state) {
> > case TCP_LISTEN:
> >@@ -1947,7 +1975,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > }
> >
> > if (batchable)
> >- *batchable = virtio_transport_rx_batch_can_receive(t, sk, 0);
> >+ *batchable = virtio_transport_rx_batch_can_continue(t, sk, 0);
>
> Why this change?
>
This runs after the packet has been handled. can_receive() decides
whether a packet may be processed in the batch, so I do not call it
again here. can_continue() checks whether the socket state, type,
transport, and RX limits still allow the batch to remain active. If it
returns false, the caller releases the socket lock and drops the
connected reference.
A change to sk_prot, sk_write_space, or the psock is seen by
can_receive() before the next packet is processed, and that packet takes
the regular path.
> > }
> >
> > /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
> >@@ -1991,7 +2019,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> > goto free_pkt;
> >
> > lock_sock(sk);
> >- virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
> >+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL, NULL);
> >
> > release_sock(sk);
> >
> >@@ -2008,20 +2036,31 @@ EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
> >
> > /*
> > * Release an active RX batch. A non-empty batch holds the socket lock and the
> >- * reference returned by vsock_find_connected_socket_net(). An empty batch
> >- * holds neither.
> >+ * reference returned by vsock_find_connected_socket_net(). Deliver any
> >+ * deferred native write-space notification while holding the lock, preserving
> >+ * the callback context of the regular receive path. An empty batch holds
> >+ * neither.
> > */
> > void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
> > {
> >+ bool write_space_pending = batch->write_space_pending;
> > struct sock *sk = batch->sk;
> >
> > batch->sk = NULL;
> > batch->pkts = 0;
> > batch->bytes = 0;
> >+ batch->write_space_pending = false;
> >
> > if (!sk)
> > return;
> >
> >+ /*
> >+ * The packet was admitted with the native callback. Do not reload a current
> >+ * callback here: sockmap can replace or remove its wrapper independently.
> >+ */
> >+ if (write_space_pending)
> >+ vsock_call_native_write_space(sk);
> >+
> > release_sock(sk);
> > sock_put(sk);
> > }
> >@@ -2080,7 +2119,8 @@ virtio_transport_rx_batch_process(struct virtio_transport *t,
> > return;
> > }
> >
> >- virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable);
> >+ virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable,
> >+ batch);
> > if (!batchable) {
> > virtio_transport_rx_batch_finish(batch);
> > return;
> >@@ -2158,16 +2198,8 @@ void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
> > }
> >
> > lock_sock(sk);
> >- /*
> >- * Serialize initial admission with sockmap's callback updates. Later
> >- * packet boundaries recheck native state and end a batch when they
> >- * observe a replacement.
> >- */
> >- read_lock_bh(&sk->sk_callback_lock);
> > batchable = virtio_transport_rx_batch_can_receive(t, sk, len);
> >- read_unlock_bh(&sk->sk_callback_lock);
>
> Can we avoid this change and do this in the previous patch?
>
> Please avoid as much as you can change code introduced in the same series.
>
Sure, understood.
> > if (!batchable) {
> >- /* State or receive-buffer failures keep the regular lookup path. */
>
> Why removing this comment just added by previous patch?
>
This patch adds a replaced sk_write_space as another reason for
can_receive() to return false, and that case does not take the regular
lookup path. I removed the old comment because it no longer
described the distinction completely, but I forgot to add the replacement.
Sorry about that, and thanks for catching it.
> > if (!virtio_transport_rx_batch_can_continue(t, sk, len)) {
> > release_sock(sk);
> > sock_put(sk);
> >@@ -2176,7 +2208,7 @@ void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
> >
> > virtio_transport_trace_recv_pkt(hdr, &src, &dst);
> > if (virtio_transport_recv_pkt_set_owner(skb, sk))
> >- virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
> >+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL, NULL);
> > else
> > kfree_skb(skb);
> >
> >--
> >2.53.0
> >
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v5 5/5] vsock/virtio: defer native RX readiness notifications until batch unlock
2026-11-05 9:38 ` Stefano Garzarella
@ 2026-11-06 13:33 ` Jia Jia
0 siblings, 0 replies; 18+ messages in thread
From: Jia Jia @ 2026-11-06 13:33 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, stefanha, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
>
> On Tue, Nov 03, 2026 at 12:11:34PM +0800, Jia Jia wrote:
> >An RX batch can make a socket readable while the RX worker retains its
> >socket lock. Waking a blocked reader before release_sock() merely makes it
> >wait for the same lock.
> >
> >P4 coalesces native sk_write_space notifications while retaining their
> >under-lock callback context. This patch also coalesces native sk_data_ready
> >notifications and moves both callbacks after release_sock(), so awakened
> >tasks do not immediately block on the RX worker's socket lock.
> >
> >Save the native data-ready callback before the AF_VSOCK socket is
> >published. Batch only while the native protocol and both native readiness
> >callbacks are present and no psock is observed under sk_callback_lock. A
> >socket with a replacement callback continues down the existing per-packet
> >path.
> >
> >At batch finish, deliver write-space before data-ready and hold the
> >connected lookup reference until both callbacks return. Packets admitted on
> >the native path continue to use their saved native callbacks. Loading a
> >current sockmap callback at finish would not serialize its invocation
> >against sockmap teardown and could lose the native wakeup or invoke a stale
> >data-ready wrapper.
> >
> >Extract the data-ready predicate from vsock_data_ready() so the deferred
> >and regular paths share the low-watermark test.
> >
> >Performance:
> >
> >P4-P5 contribution (P1-P3 vs P1-P5):
>
> I'm still confused on the reason why P4-P5 are not squashed.
> This patch is essentially changing 90% of the previous patch...
>
I think I may have misunderstood how far to carry our v4 discussion
into v5. I understood that keeping write-space coalescing under the
socket lock as a complete intermediate step, before adding data-ready
and moving both callbacks after release_sock(), would keep the two
changes separately testable. That is why I kept the same split in v5.
In the resulting v5 series, however, P5 changes too much of P4 for that
split to remain useful. I will squash P4 and P5. Sorry for the confusion.
> >
> >End-to-end RX throughput (64K Guest recv() size, SO_RCVLOWAT=1; Gbit/s):
> >
> > payload P1-P3 RX P1-P5 RX throughput change (95% CI)
> > (Gbit/s) (Gbit/s)
> > 64B 0.2449 0.2463 +0.55% [-3.86%, +5.17%]
> > 256B 0.9649 0.9931 +2.93% [+0.83%, +5.07%]
> > 512B 1.8205 1.8260 +0.30% [-4.27%, +5.10%]
> > 1K 3.3431 3.3420 -0.03% [-2.15%, +2.12%]
> > 4K 5.6541 5.8428 +3.34% [+1.10%, +5.63%]
> > 8K 5.4280 5.3364 -1.69% [-15.55%, +14.46%]
> > 16K 5.5935 5.6701 +1.37% [-1.41%, +4.23%]
> > 64K 5.6917 5.8351 +2.52% [-0.20%, +5.31%]
> >
> >Fixed-syscall stress test (receiver recv() size = sender write size;
> >Gbit/s):
> >
> > payload P1-P3 RX P1-P5 RX throughput change (95% CI)
> > recv() calls are listed as P1-P3/P1-P5.
> > 64B 0.2553 0.2575 +0.87% [-3.68%, +5.63%]
> > 1048577/1048577
> > 256B 0.9069 0.9183 +1.26% [-3.96%, +6.76%]
> > 1048577/1048577
> > 512B 1.6048 1.6459 +2.56% [+0.98%, +4.18%]
> > 1048577/1048577
> > 1K 2.6939 2.7357 +1.55% [-1.35%, +4.54%]
> > 1048577/1048577
> > 4K 4.9168 4.9678 +1.04% [-1.84%, +4.00%]
> > 524965/525283
> > 8K 5.1476 5.2005 +1.03% [-4.02%, +6.34%]
> > 262926/263156
> > 16K 5.5964 5.6609 +1.15% [-2.86%, +5.33%]
> > 131311/131475
> > 64K 5.9439 5.9073 -0.62% [-3.00%, +1.83%]
> > 32875/32937
> >
> >Guest CPU1 aggregate cycles per received byte
> >(fixed 64K receiver, with perf):
> >
> > payload P1-P3 P1-P5 change (95% CI)
> > 64B 60.281 58.258 -3.36% [-8.74%, +2.35%]
> > 256B 14.242 13.648 -4.17% [-9.81%, +1.82%]
> > 512B 7.547 7.211 -4.45% [-9.73%, +1.13%]
> > 1K 3.987 3.853 -3.35% [-8.52%, +2.12%]
> > 4K 1.983 1.880 -5.17% [-7.97%, -2.27%]
> > 8K 1.665 1.620 -2.75% [-9.03%, +3.96%]
> > 16K 1.409 1.346 -4.52% [-10.94%, +2.37%]
> > 64K 1.322 1.281 -3.11% [-6.49%, +0.38%]
> >
> >Full-series cycles/B (baseline vs P1-P5):
> >
> >Guest CPU1 aggregate cycles per received byte
> >(fixed 64K receiver, with perf):
> >
> > payload baseline P1-P5 change (95% CI)
> > 64B 65.282 57.843 -11.40% [-13.77%, -8.95%]
> > 256B 14.710 13.813 -6.10% [ -7.89%, -4.28%]
> > 512B 7.709 7.188 -6.75% [ -8.79%, -4.66%]
> > 1K 4.075 3.862 -5.23% [ -6.80%, -3.63%]
> > 4K 2.034 1.887 -7.21% [ -8.67%, -5.72%]
> > 8K 1.664 1.568 -5.78% [ -7.83%, -3.69%]
> > 16K 1.450 1.385 -4.48% [ -6.08%, -2.86%]
> > 64K 1.338 1.317 -1.51% [ -3.21%, +0.22%]
> >
> >Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> >---
> > include/linux/virtio_vsock.h | 5 ++-
> > include/net/af_vsock.h | 9 +++--
> > net/vmw_vsock/af_vsock.c | 27 ++++++++++---
> > net/vmw_vsock/virtio_transport_common.c | 52 +++++++++++++++----------
> > 4 files changed, 62 insertions(+), 31 deletions(-)
> >
> >Tested:
> > Concurrent map-entry attach/detach during sequenced RX and duplex traffic
> > passed on baseline, P1-P3, P1-P4, and P1-P5. The active SK_PASS case also
> > passed on each state, with 200,000 verdict invocations. The concurrent
> > SK_PASS attach/detach case completed 1,000 insert/remove pairs per state;
> > its 10,000-record suffix after the final detach was verified. No kernel
> > diagnostic markers were found in successful runs.
> >
> > Sockmap may replace sk_data_ready and sk_write_space. With non-native
> > callbacks, native RX batching must be declined and the ordinary per-packet
> > receive path used. The active SK_PASS case covers the condition expected to
> > trigger this fallback. The test does not trace each packet's internal
> > branch, so it verifies the fallback scenario without asserting that every
> > packet took that branch.
> >
> > The duplex cases exercised write-space backpressure: the guest's nonblocking
> > send reached EAGAIN while the host was not reading. After the host began
> > draining data, the guest observed POLLOUT and resumed sending. Counts below
> > are observed EAGAIN/POLLOUT recoveries.
> >
> > Test / result baseline P1-P3 P1-P4 P1-P5
> > No-verdict RX churn: attach/detach pairs 433493 455507 430985 380351
> > No-verdict duplex: attach/detach pairs 229793 212410 214198 148755
> > No-verdict duplex: EAGAIN/POLLOUT 365/365 266/266 264/264 443/443
> > Active SK_PASS: EAGAIN/POLLOUT 2548/2548 3013/3013 3093/3093 4208/4208
> > SK_PASS attach/detach: intervals with verdict progress 705 616 631 692
> > SK_PASS attach/detach: verdict invocations 99747 99894 97475 82409
> > Post-detach SK_PASS suffix 10,000/10,000 records per run
> >
> >diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> >index bf28e8e32e280..2aa0e81e655d5 100644
> >--- a/include/linux/virtio_vsock.h
> >+++ b/include/linux/virtio_vsock.h
> >@@ -287,14 +287,15 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> > * Initialize a batch to zero before use. A non-empty batch owns @sk's socket
> > * lock and the reference returned by vsock_find_connected_socket_net(). The
> > * caller retains both until it calls virtio_transport_rx_batch_finish(). An
> >- * empty batch owns neither. A pending native write-space notification belongs
> >- * to the lock holder and is delivered by virtio_transport_rx_batch_finish().
> >+ * empty batch owns neither. Pending native readiness notifications belong to
> >+ * the lock holder and are delivered by virtio_transport_rx_batch_finish().
> > */
> > struct virtio_transport_rx_batch {
> > struct sock *sk;
> > unsigned int pkts;
> > size_t bytes;
> > bool write_space_pending;
> >+ bool data_ready_pending;
> > };
> >
> > static inline bool
> >diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> >index b76ab565ea58b..57ee545fdb496 100644
> >--- a/include/net/af_vsock.h
> >+++ b/include/net/af_vsock.h
> >@@ -64,9 +64,10 @@ struct vsock_sock {
> > bool sent_request;
> > bool ignore_connecting_rst;
> > /*
> >- * Captured before the socket is published. This identifies AF_VSOCK's
> >- * native write-space callback after a consumer has replaced it.
> >+ * Captured before the socket is published. These identify AF_VSOCK's
> >+ * native readiness callbacks after a consumer has replaced them.
> > */
> >+ void (*native_data_ready)(struct sock *sk);
> > void (*native_write_space)(struct sock *sk);
> >
> > /* Protected by lock_sock(sk) */
> >@@ -83,7 +84,9 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk);
> > s64 vsock_stream_has_space(struct vsock_sock *vsk);
> > struct sock *vsock_create_connected(struct sock *parent);
> > void vsock_data_ready(struct sock *sk);
> >-bool vsock_write_space_is_native(const struct sock *sk);
> >+bool vsock_data_ready_pending(struct sock *sk);
> >+bool vsock_callbacks_are_native(const struct sock *sk);
> >+void vsock_call_native_data_ready(struct sock *sk);
> > void vsock_call_native_write_space(struct sock *sk);
> >
> > /**** TRANSPORT ****/
> >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> >index e39567990539f..0617073e99ed8 100644
> >--- a/net/vmw_vsock/af_vsock.c
> >+++ b/net/vmw_vsock/af_vsock.c
> >@@ -958,6 +958,7 @@ static struct sock *__vsock_create(struct net *net,
> > sk->sk_type = type;
> >
> > vsk = vsock_sk(sk);
> >+ vsk->native_data_ready = sk->sk_data_ready;
> > vsk->native_write_space = sk->sk_write_space;
> > vsock_addr_init(&vsk->local_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
> > vsock_addr_init(&vsk->remote_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
> >@@ -1113,24 +1114,38 @@ s64 vsock_stream_has_space(struct vsock_sock *vsk)
> > }
> > EXPORT_SYMBOL_GPL(vsock_stream_has_space);
> >
> >-void vsock_data_ready(struct sock *sk)
> >+bool vsock_data_ready_pending(struct sock *sk)
> > {
> > struct vsock_sock *vsk = vsock_sk(sk);
> >
> >- if (vsock_stream_has_data(vsk) >= sk->sk_rcvlowat ||
> >- sock_flag(sk, SOCK_DONE))
> >+ return vsock_stream_has_data(vsk) >= sk->sk_rcvlowat ||
> >+ sock_flag(sk, SOCK_DONE);
> >+}
> >+EXPORT_SYMBOL_GPL(vsock_data_ready_pending);
> >+
> >+void vsock_data_ready(struct sock *sk)
> >+{
> >+ if (vsock_data_ready_pending(sk))
> > sk->sk_data_ready(sk);
> > }
> > EXPORT_SYMBOL_GPL(vsock_data_ready);
> >
> >-bool vsock_write_space_is_native(const struct sock *sk)
> >+bool vsock_callbacks_are_native(const struct sock *sk)
> > {
> > const struct vsock_sock *vsk = vsock_sk(sk);
> >
> >- return READ_ONCE(sk->sk_write_space) ==
> >+ return READ_ONCE(sk->sk_data_ready) ==
> >+ READ_ONCE(vsk->native_data_ready) &&
> >+ READ_ONCE(sk->sk_write_space) ==
> > READ_ONCE(vsk->native_write_space);
> > }
> >-EXPORT_SYMBOL_GPL(vsock_write_space_is_native);
> >+EXPORT_SYMBOL_GPL(vsock_callbacks_are_native);
> >+
> >+void vsock_call_native_data_ready(struct sock *sk)
> >+{
> >+ vsock_sk(sk)->native_data_ready(sk);
> >+}
> >+EXPORT_SYMBOL_GPL(vsock_call_native_data_ready);
> >
> > void vsock_call_native_write_space(struct sock *sk)
> > {
> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >index af26344678e41..9d02e81042c21 100644
> >--- a/net/vmw_vsock/virtio_transport_common.c
> >+++ b/net/vmw_vsock/virtio_transport_common.c
> >@@ -1598,7 +1598,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> >
> > static int
> > virtio_transport_recv_connected(struct sock *sk,
> >- struct sk_buff *skb)
> >+ struct sk_buff *skb,
> >+ bool *data_ready_pending)
> > {
> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > struct vsock_sock *vsk = vsock_sk(sk);
> >@@ -1617,7 +1618,12 @@ virtio_transport_recv_connected(struct sock *sk,
> > vsock_remove_sock(vsk);
> > break;
> > }
> >- vsock_data_ready(sk);
> >+ if (data_ready_pending) {
> >+ if (!*data_ready_pending &&
> >vsock_data_ready_pending(sk))
> >+ *data_ready_pending = true;
> >+ } else {
> >+ vsock_data_ready(sk);
> >+ }
> > return err;
> > case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> > virtio_transport_send_credit_update(vsk);
> >@@ -1874,20 +1880,19 @@ __virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> > return false;
> >
> > /*
> >- * Sockmap can replace sk_prot or sk_write_space, and publish a psock,
> >- * without the socket lock. Coalescing is safe only for the native
> >- * write-space callback, so a boundary that sees a replacement returns to
> >- * the existing per-packet path.
> >+ * Sockmap can replace sk_prot or either readiness callback, and publish a
> >+ * psock, without the socket lock. Coalescing is safe only for native
> >+ * callbacks, so a boundary that sees a replacement returns to the existing
> >+ * per-packet path.
> > */
> > return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
> >- vsock_write_space_is_native(sk) &&
> >+ vsock_callbacks_are_native(sk) &&
> > !virtio_transport_sock_has_psock(sk);
> > }
> >
> > /*
> >- * Sockmap teardown restores the native protocol and write-space callback
> >- * under sk_callback_lock. Serialize each packet admission check with those
> >- * updates.
> >+ * Sockmap teardown restores the native protocol and callbacks under
> >+ * sk_callback_lock. Serialize each packet admission check with those updates.
> > */
> > static bool
> > virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> >@@ -1905,9 +1910,8 @@ virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
> > /*
> > * The caller holds sk's socket lock. If @batchable is non-NULL, update it
> > * after processing @skb to say whether a native STREAM/RW batch can continue.
> >- * When @batch is non-NULL, record a native write-space notification for batch
> >- * finish. A writer cannot consume the updated credit until this lock is
> >- * released.
> >+ * When @batch is non-NULL, record native readiness notifications for batch
> >+ * finish. A reader or writer cannot make progress until this lock is released.
> > */
> > static void
> > virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> >@@ -1917,6 +1921,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > {
> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > struct vsock_sock *vsk = vsock_sk(sk);
> >+ bool *data_ready_pending;
> > struct sockaddr_vm src;
> > bool space_available;
> >
> >@@ -1926,6 +1931,8 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > if (batchable)
> > *batchable = false;
> >
> >+ data_ready_pending = batch ? &batch->data_ready_pending : NULL;
>
> Can we just pass `batch` to virtio_transport_recv_connected and avoid
> all of this complexity?
>
Sure, understood.
> >+
> > /*
> > * The socket may have been closed or reassigned after lookup but before
> > * its socket lock was acquired. Listener sockets have no assigned
> >@@ -1962,7 +1969,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> > kfree_skb(skb);
> > break;
> > case TCP_ESTABLISHED:
> >- virtio_transport_recv_connected(sk, skb);
> >+ virtio_transport_recv_connected(sk, skb, data_ready_pending);
> > break;
> > case TCP_CLOSING:
> > virtio_transport_recv_disconnecting(sk, skb);
> >@@ -2036,32 +2043,37 @@ EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
> >
> > /*
> > * Release an active RX batch. A non-empty batch holds the socket lock and the
> >- * reference returned by vsock_find_connected_socket_net(). Deliver any
> >- * deferred native write-space notification while holding the lock, preserving
> >- * the callback context of the regular receive path. An empty batch holds
> >- * neither.
> >+ * reference returned by vsock_find_connected_socket_net(). Release the lock,
> >+ * deliver deferred native write-space and data-ready notifications in that
> >+ * order, then drop the reference. An empty batch holds neither.
> > */
>
> Really, avoid comments that just describe what a function is doing. They
> really don't add much value and we need to update them every time, like
> this...
>
> Comments should explain reasons, hidden behaviours, etc.
> Something unclear just looking at the code.
>
Understood, thanks. I have not been doing this well. I will be more careful.
> > void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
> > {
> > bool write_space_pending = batch->write_space_pending;
> >+ bool data_ready_pending = batch->data_ready_pending;
> > struct sock *sk = batch->sk;
> >
> > batch->sk = NULL;
> > batch->pkts = 0;
> > batch->bytes = 0;
> > batch->write_space_pending = false;
> >+ batch->data_ready_pending = false;
> >
> > if (!sk)
> > return;
> >
>
> Here should be nice to have a comment to explain WHY it's safe to
> release in this new position...
>
Sure. I will add a comment there.
> >+ release_sock(sk);
> >+
> > /*
> >- * The packet was admitted with the native callback. Do not reload a current
> >+ * Packets were admitted with native callbacks. Do not reload a current
>
> Why this change?
>
I only changed the wording because a batch
can hold more than one packet and both callbacks are now deferred.
> Sorry, this series is really hard to review :-(
>
Sorry again. This is related to my misunderstanding.
> > * callback here: sockmap can replace or remove its wrapper independently.
> > */
> > if (write_space_pending)
> > vsock_call_native_write_space(sk);
> >
> >- release_sock(sk);
> >+ if (data_ready_pending)
> >+ vsock_call_native_data_ready(sk);
> >+
> > sock_put(sk);
> > }
> > EXPORT_SYMBOL_GPL(virtio_transport_rx_batch_finish);
> >--
> >2.53.0
> >
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications
2026-10-23 7:51 [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead Jia Jia
@ 2026-10-23 7:51 ` Jia Jia
0 siblings, 0 replies; 18+ messages in thread
From: Jia Jia @ 2026-10-23 7:51 UTC (permalink / raw)
To: netdev
Cc: stefanha, sgarzare, bobbyeshleman, mst, jasowangio, eperezma,
xuanzhuo, davem, edumazet, kuba, pabeni, horms, kvm,
virtualization, linux-kernel
Each received packet can update peer credit and call sk_write_space(). A
writer cannot consume that credit while the virtio RX worker retains the
socket lock, so repeated native write-space notifications in one RX batch
are unnecessary.
This patch coalesces write-space notifications on the native AF_VSOCK path;
data-ready remains per-packet here.
The batching patch establishes batch ownership before its first packet.
This lets this patch record a pending notification for that packet and
flush it even if the packet immediately ends the batch. Deliver it while
holding the socket lock to preserve the callback context of the existing
receive path. A following patch changes that timing together with
data-ready handling.
Capture the native write-space callback before socket publication and
access it through AF_VSOCK helpers. Sockmap can change the protocol,
write-space callback, and psock marker without the socket lock. Serialize
each batch eligibility check with sk_callback_lock; if a packet boundary
observes a replacement, finish the batch and use the regular per-packet
path.
Packets admitted on the native path use the saved native callback at batch
finish. Reloading a current sockmap callback would not synchronize its
invocation with teardown; a wrapper seen before teardown need not forward
the native wakeup after its psock is gone.
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
include/linux/virtio_vsock.h | 4 +-
include/net/af_vsock.h | 7 +++
net/vmw_vsock/af_vsock.c | 16 +++++
net/vmw_vsock/virtio_transport_common.c | 78 +++++++++++++++++--------
4 files changed, 81 insertions(+), 24 deletions(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 748633aa0cc81..bf28e8e32e280 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -287,12 +287,14 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
* Initialize a batch to zero before use. A non-empty batch owns @sk's socket
* lock and the reference returned by vsock_find_connected_socket_net(). The
* caller retains both until it calls virtio_transport_rx_batch_finish(). An
- * empty batch owns neither.
+ * empty batch owns neither. A pending native write-space notification belongs
+ * to the lock holder and is delivered by virtio_transport_rx_batch_finish().
*/
struct virtio_transport_rx_batch {
struct sock *sk;
unsigned int pkts;
size_t bytes;
+ bool write_space_pending;
};
static inline bool
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 5549298c1ec6b..b76ab565ea58b 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -63,6 +63,11 @@ struct vsock_sock {
u32 peer_shutdown;
bool sent_request;
bool ignore_connecting_rst;
+ /*
+ * Captured before the socket is published. This identifies AF_VSOCK's
+ * native write-space callback after a consumer has replaced it.
+ */
+ void (*native_write_space)(struct sock *sk);
/* Protected by lock_sock(sk) */
u64 buffer_size;
@@ -78,6 +83,8 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk);
s64 vsock_stream_has_space(struct vsock_sock *vsk);
struct sock *vsock_create_connected(struct sock *parent);
void vsock_data_ready(struct sock *sk);
+bool vsock_write_space_is_native(const struct sock *sk);
+void vsock_call_native_write_space(struct sock *sk);
/**** TRANSPORT ****/
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 44cee74519555..e39567990539f 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -958,6 +958,7 @@ static struct sock *__vsock_create(struct net *net,
sk->sk_type = type;
vsk = vsock_sk(sk);
+ vsk->native_write_space = sk->sk_write_space;
vsock_addr_init(&vsk->local_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
vsock_addr_init(&vsk->remote_addr, VMADDR_CID_ANY, VMADDR_PORT_ANY);
@@ -1122,6 +1123,21 @@ void vsock_data_ready(struct sock *sk)
}
EXPORT_SYMBOL_GPL(vsock_data_ready);
+bool vsock_write_space_is_native(const struct sock *sk)
+{
+ const struct vsock_sock *vsk = vsock_sk(sk);
+
+ return READ_ONCE(sk->sk_write_space) ==
+ READ_ONCE(vsk->native_write_space);
+}
+EXPORT_SYMBOL_GPL(vsock_write_space_is_native);
+
+void vsock_call_native_write_space(struct sock *sk)
+{
+ vsock_sk(sk)->native_write_space(sk);
+}
+EXPORT_SYMBOL_GPL(vsock_call_native_write_space);
+
/* Dummy callback required by sockmap.
* See unconditional call of saved_close() in sock_map_close().
*/
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index f83e5b2edce35..af26344678e41 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1865,31 +1865,55 @@ virtio_transport_rx_batch_can_continue(struct virtio_transport *t,
!virtio_transport_rx_buf_full(vvs);
}
-/* The caller holds sk's socket lock. */
+/* The caller holds sk's socket and callback locks. */
static bool
-virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
- struct sock *sk, u32 len)
+__virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
+ struct sock *sk, u32 len)
{
if (!virtio_transport_rx_batch_can_continue(t, sk, len))
return false;
/*
- * Sockmap can replace sk_prot and publish a psock without the socket lock.
- * Check both at each packet boundary; a boundary that sees a replacement
- * returns to the existing per-packet path.
+ * Sockmap can replace sk_prot or sk_write_space, and publish a psock,
+ * without the socket lock. Coalescing is safe only for the native
+ * write-space callback, so a boundary that sees a replacement returns to
+ * the existing per-packet path.
*/
return READ_ONCE(sk->sk_prot) == sk->sk_prot_creator &&
+ vsock_write_space_is_native(sk) &&
!virtio_transport_sock_has_psock(sk);
}
+/*
+ * Sockmap teardown restores the native protocol and write-space callback
+ * under sk_callback_lock. Serialize each packet admission check with those
+ * updates.
+ */
+static bool
+virtio_transport_rx_batch_can_receive(struct virtio_transport *t,
+ struct sock *sk, u32 len)
+{
+ bool batchable;
+
+ read_lock_bh(&sk->sk_callback_lock);
+ batchable = __virtio_transport_rx_batch_can_receive(t, sk, len);
+ read_unlock_bh(&sk->sk_callback_lock);
+
+ return batchable;
+}
+
/*
* The caller holds sk's socket lock. If @batchable is non-NULL, update it
* after processing @skb to say whether a native STREAM/RW batch can continue.
+ * When @batch is non-NULL, record a native write-space notification for batch
+ * finish. A writer cannot consume the updated credit until this lock is
+ * released.
*/
static void
virtio_transport_recv_pkt_locked(struct virtio_transport *t,
struct sk_buff *skb, struct sock *sk,
- struct net *net, bool *batchable)
+ struct net *net, bool *batchable,
+ struct virtio_transport_rx_batch *batch)
{
struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
struct vsock_sock *vsk = vsock_sk(sk);
@@ -1921,8 +1945,12 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
vsk->local_addr.svm_cid = le64_to_cpu(hdr->dst_cid);
- if (space_available)
- sk->sk_write_space(sk);
+ if (space_available) {
+ if (batch)
+ batch->write_space_pending = true;
+ else
+ sk->sk_write_space(sk);
+ }
switch (sk->sk_state) {
case TCP_LISTEN:
@@ -1947,7 +1975,7 @@ virtio_transport_recv_pkt_locked(struct virtio_transport *t,
}
if (batchable)
- *batchable = virtio_transport_rx_batch_can_receive(t, sk, 0);
+ *batchable = virtio_transport_rx_batch_can_continue(t, sk, 0);
}
/* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
@@ -1991,7 +2019,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
goto free_pkt;
lock_sock(sk);
- virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL, NULL);
release_sock(sk);
@@ -2008,20 +2036,31 @@ EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt);
/*
* Release an active RX batch. A non-empty batch holds the socket lock and the
- * reference returned by vsock_find_connected_socket_net(). An empty batch
- * holds neither.
+ * reference returned by vsock_find_connected_socket_net(). Deliver any
+ * deferred native write-space notification while holding the lock, preserving
+ * the callback context of the regular receive path. An empty batch holds
+ * neither.
*/
void virtio_transport_rx_batch_finish(struct virtio_transport_rx_batch *batch)
{
+ bool write_space_pending = batch->write_space_pending;
struct sock *sk = batch->sk;
batch->sk = NULL;
batch->pkts = 0;
batch->bytes = 0;
+ batch->write_space_pending = false;
if (!sk)
return;
+ /*
+ * The packet was admitted with the native callback. Do not reload a current
+ * callback here: sockmap can replace or remove its wrapper independently.
+ */
+ if (write_space_pending)
+ vsock_call_native_write_space(sk);
+
release_sock(sk);
sock_put(sk);
}
@@ -2080,7 +2119,8 @@ virtio_transport_rx_batch_process(struct virtio_transport *t,
return;
}
- virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable);
+ virtio_transport_recv_pkt_locked(t, skb, batch->sk, net, &batchable,
+ batch);
if (!batchable) {
virtio_transport_rx_batch_finish(batch);
return;
@@ -2158,16 +2198,8 @@ void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
}
lock_sock(sk);
- /*
- * Serialize initial admission with sockmap's callback updates. Later
- * packet boundaries recheck native state and end a batch when they
- * observe a replacement.
- */
- read_lock_bh(&sk->sk_callback_lock);
batchable = virtio_transport_rx_batch_can_receive(t, sk, len);
- read_unlock_bh(&sk->sk_callback_lock);
if (!batchable) {
- /* State or receive-buffer failures keep the regular lookup path. */
if (!virtio_transport_rx_batch_can_continue(t, sk, len)) {
release_sock(sk);
sock_put(sk);
@@ -2176,7 +2208,7 @@ void virtio_transport_recv_pkt_batch(struct virtio_transport *t,
virtio_transport_trace_recv_pkt(hdr, &src, &dst);
if (virtio_transport_recv_pkt_set_owner(skb, sk))
- virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL);
+ virtio_transport_recv_pkt_locked(t, skb, sk, net, NULL, NULL);
else
kfree_skb(skb);
--
2.53.0
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-11-06 13:34 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03 4:11 [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 1/5] vsock/virtio: split socket lookup from locked RX processing Jia Jia
2026-11-05 9:04 ` Stefano Garzarella
2026-11-03 4:11 ` [PATCH net-next v5 2/5] vsock/virtio: factor RX skb metadata budget check Jia Jia
2026-11-05 9:06 ` Stefano Garzarella
2026-11-06 12:28 ` Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 3/5] vsock/virtio: amortize RX socket locking for stream packets Jia Jia
2026-11-04 19:26 ` Bobby Eshleman
2026-11-05 8:16 ` Jia Jia
2026-11-05 8:57 ` Stefano Garzarella
2026-11-06 12:27 ` Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications Jia Jia
2026-11-05 9:21 ` Stefano Garzarella
2026-11-06 12:49 ` Jia Jia
2026-11-03 4:11 ` [PATCH net-next v5 5/5] vsock/virtio: defer native RX readiness notifications until batch unlock Jia Jia
2026-11-05 9:38 ` Stefano Garzarella
2026-11-06 13:33 ` Jia Jia
-- strict thread matches above, loose matches on Subject: below --
2026-10-23 7:51 [PATCH net-next v5 0/5] vsock/virtio: reduce RX per-packet socket overhead Jia Jia
2026-10-23 7:51 ` [PATCH net-next v5 4/5] vsock/virtio: coalesce native RX write-space notifications Jia Jia
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®