* [PATCH net v2 0/2] vsock/virtio: collapse receive queue under memory pressure
@ 2026-07-08 10:29 Stefano Garzarella
2026-07-08 10:29 ` [PATCH net v2 1/2] " Stefano Garzarella
2026-07-08 10:29 ` [PATCH net v2 2/2] vsock/test: add test for small packets under pressure Stefano Garzarella
0 siblings, 2 replies; 9+ messages in thread
From: Stefano Garzarella @ 2026-07-08 10:29 UTC (permalink / raw)
To: netdev
Cc: Jason Wang, Stefano Garzarella, Xuan Zhuo, Eric Dumazet,
Eugenio Pérez, Simon Horman, Stefan Hajnoczi,
David S. Miller, linux-kernel, Michael S. Tsirkin, kvm,
Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang
This series contains a patch (the first one) that is part of work I'm
doing to improve the tracking of memory used by AF_VSOCK sockets.
The second patch is a test for our suite that highlights the issue.
Since Brien reported an issue with his environment (based on Linux 6.12.y)
related to the work I’m doing, I extracted this patch and tried to make it
as easy as possible to backport. Brien tested it by backporting it to
6.12.y, which now contains the backport of the 059b7dbd20a6
("vsock/virtio: fix potential unbounded skb queue").
This patch primarily fixes STREAM sockets, but also partially fixes
SEQPACKET (with the exception of EOMs, which are kept in separate skbs to
avoid overcomplicating the code).
The rest of the work, I feel, is more net-next material and still needs
some work to be completed.
Changelog
---------
v2:
- defined MAX_COLLAPSE_LEN macro instead of using a variable [Paolo]
- added a threshold to avoid walking all the queue while collapsing
[Paolo]
- collapsed the queue before calling virtio_transport_inc_rx_pkt().
While working on the threshold, I figured out that the check I was
introducing can also be used to proactively trigger the collapse, so I
moved the call to virtio_transport_collapse_rx_queue() before acquiring
the rx_lock to have also a better diff to simplify backports
- improved code readability (removed `out` label, `keep` initialization,
etc.) [Paolo + other small stuff]
- Brien kindly retested this version as well (thank you so much)
v1: https://lore.kernel.org/netdev/20260626134823.206676-1-sgarzare@redhat.com/
Thanks,
Stefano
Stefano Garzarella (2):
vsock/virtio: collapse receive queue under memory pressure
vsock/test: add test for small packets under pressure
net/vmw_vsock/virtio_transport_common.c | 165 +++++++++++++++++++++++-
tools/testing/vsock/vsock_test.c | 87 +++++++++++++
2 files changed, 251 insertions(+), 1 deletion(-)
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure 2026-07-08 10:29 [PATCH net v2 0/2] vsock/virtio: collapse receive queue under memory pressure Stefano Garzarella @ 2026-07-08 10:29 ` Stefano Garzarella 2026-07-08 11:00 ` Michael S. Tsirkin 2026-07-09 19:07 ` Bobby Eshleman 2026-07-08 10:29 ` [PATCH net v2 2/2] vsock/test: add test for small packets under pressure Stefano Garzarella 1 sibling, 2 replies; 9+ messages in thread From: Stefano Garzarella @ 2026-07-08 10:29 UTC (permalink / raw) To: netdev Cc: Jason Wang, Stefano Garzarella, Xuan Zhuo, Eric Dumazet, Eugenio Pérez, Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, Michael S. Tsirkin, kvm, Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang, stable, Brien Oberstein From: Stefano Garzarella <sgarzare@redhat.com> When many small packets accumulate in the receive queue, the skb overhead can exceed buf_alloc even while the payload is within bounds. This causes virtio_transport_inc_rx_pkt() to reject packets, leading to connection resets during large transfers under backpressure. The issue was reported by Brien, who has a reproducer, but it is also easily reproducible with iperf-vsock [1] using a small packet size: iperf3 --vsock -c $CID -l 129 which fails immediately without this patch but with commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"). Inspired by TCP's tcp_collapse() which solves a similar problem, add virtio_transport_collapse_rx_queue() that walks the receive queue and re-copies data into compact linear skbs to reduce the overhead. The collapse is triggered proactively from when the number of skb queued is close to exceeding the overhead budget. A pre-scan counts the eligible bytes to size each allocation precisely, avoiding waste for isolated small packets. Partially consumed skbs are kept as-is to preserve buf_used/fwd_cnt accounting, EOM-marked skbs to maintain SEQPACKET message boundaries, and skbs already larger than the collapse target because they already have a good data-to-overhead ratio. Walking a large queue may take a significant amount of time and cache misses, causing traffic burstiness. To limit this, the collapse stops once enough room is freed for this packet and the next one, but may opportunistically free more to fill each collapsed skb to capacity. [1] https://github.com/stefano-garzarella/iperf-vsock Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue") Cc: stable@vger.kernel.org Reported-by: Brien Oberstein <brienpub@gmail.com> Closes: https://lore.kernel.org/netdev/618701dd023e$063de350$12b9a9f0$@gmail.com/ Tested-by: Brien Oberstein <brienpub@gmail.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- v2: - defined MAX_COLLAPSE_LEN macro instead of using a variable [Paolo] - added a threshold to avoid walking all the queue while collapsing [Paolo] - collapsed the queue before calling virtio_transport_inc_rx_pkt(). While working on the threshold, I figured out that the check I was introducing can also be used to proactively trigger the collapse, so I moved the call to virtio_transport_collapse_rx_queue() before acquiring the rx_lock to have also a better diff to simplify backports - improved code readability (removed `out` label, `keep` initialization, etc.) [Paolo + other small stuff] - Brien kindly retested this version as well (thank you so much) --- net/vmw_vsock/virtio_transport_common.c | 165 +++++++++++++++++++++++- 1 file changed, 164 insertions(+), 1 deletion(-) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 09475007165b..8becad81279c 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -26,6 +26,13 @@ /* Threshold for detecting small packets to copy */ #define GOOD_COPY_LEN 128 +/* Max payload that can be collapsed into a single linear skb, using the same + * allocation threshold as virtio_vsock_alloc_skb() to avoid adding pressure + * on the page allocator. + */ +#define MAX_COLLAPSE_LEN \ + SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER) + 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); @@ -420,6 +427,145 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk, return ret; } +static bool virtio_transport_can_collapse(struct sk_buff *skb) +{ + /* skbs that are partially consumed, mark a SEQPACKET message boundary, + * or are already large enough should not be collapsed: they either + * need special accounting, carry protocol state, or already have a + * good data-to-overhead ratio. + */ + if (VIRTIO_VSOCK_SKB_CB(skb)->offset) + return false; + if (le32_to_cpu(virtio_vsock_hdr(skb)->flags) & VIRTIO_VSOCK_SEQ_EOM) + return false; + if (skb->len >= MAX_COLLAPSE_LEN) + return false; + return true; +} + +/* Iterate through the packets in the queue starting from the current skb to + * count the number of bytes we can collapse. + */ +static unsigned int +virtio_transport_collapse_size(struct sk_buff *skb, struct sk_buff_head *queue) +{ + unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset; + + while ((skb = skb_peek_next(skb, queue)) && + virtio_transport_can_collapse(skb)) { + unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset; + + if (len > MAX_COLLAPSE_LEN - target) + return target; + + target += len; + } + + return target; +} + +/* Called under lock_sock to compact the receive queue by merging small skbs. + * @min_to_free: minimum number of skbs to eliminate from the queue. May free + * more to fill each collapsed skb to capacity. + */ +static void +virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs, + u32 min_to_free) +{ + struct sk_buff *skb, *next_skb, *new_skb = NULL; + struct sk_buff_head new_queue; + u32 saved = 0; + + __skb_queue_head_init(&new_queue); + + skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) { + struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb); + u32 src_off = VIRTIO_VSOCK_SKB_CB(skb)->offset; + u32 src_len = skb->len - src_off; + bool keep; + + keep = !virtio_transport_can_collapse(skb); + if (keep) { + /* Finalize pending collapsed skb to preserve packet + * ordering. + */ + if (new_skb) { + __skb_queue_tail(&new_queue, new_skb); + new_skb = NULL; + saved--; + } + goto next; + } + + /* Finalize if this packet won't fit in the remaining tailroom, + * so we can allocate a right-sized new_skb. + */ + if (new_skb && src_len > skb_tailroom(new_skb)) { + __skb_queue_tail(&new_queue, new_skb); + new_skb = NULL; + saved--; + } + + if (!new_skb) { + unsigned int alloc_size; + + /* Check after finalizing to opportunistically fill + * each collapsed skb to capacity, merging more skbs + * than strictly required. + */ + if (saved >= min_to_free) + break; + + alloc_size = virtio_transport_collapse_size(skb, &vvs->rx_queue); + + /* Only this skb's data is eligible, nothing to merge + * with. Keep as-is. + */ + if (alloc_size <= src_len) { + keep = true; + goto next; + } + + new_skb = virtio_vsock_alloc_linear_skb(alloc_size + + VIRTIO_VSOCK_SKB_HEADROOM, GFP_KERNEL); + if (!new_skb) + break; + + memcpy(virtio_vsock_hdr(new_skb), hdr, + sizeof(struct virtio_vsock_hdr)); + virtio_vsock_hdr(new_skb)->len = 0; + } + + /* Cannot fail since src_off/src_len are within bounds, but if + * it does, discard new_skb to avoid queuing corrupted data. + */ + if (WARN_ON_ONCE(skb_copy_bits(skb, src_off, + skb_put(new_skb, src_len), + src_len))) { + kfree_skb(new_skb); + new_skb = NULL; + break; + } + + le32_add_cpu(&virtio_vsock_hdr(new_skb)->len, src_len); + virtio_vsock_hdr(new_skb)->flags |= hdr->flags; + +next: + __skb_unlink(skb, &vvs->rx_queue); + if (keep) { + __skb_queue_tail(&new_queue, skb); + } else { + consume_skb(skb); + saved++; + } + } + + if (new_skb) + __skb_queue_tail(&new_queue, new_skb); + + skb_queue_splice(&new_queue, &vvs->rx_queue); +} + static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) { @@ -1354,12 +1500,29 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, { struct virtio_vsock_sock *vvs = vsk->trans; bool can_enqueue, free_pkt = false; + u32 len, queue_max, queue_len; struct virtio_vsock_hdr *hdr; - u32 len; hdr = virtio_vsock_hdr(skb); len = le32_to_cpu(hdr->len); + /* virtio_transport_inc_rx_pkt() rejects packets when the per-skb + * overhead (skb_queue_len * SKB_TRUESIZE(0)) exceeds buf_alloc. + * Proactively collapse the queue before that happens. + * No rx_lock needed: lock_sock is held by caller, preventing + * concurrent enqueue or dequeue. + */ + queue_max = vvs->buf_alloc / SKB_TRUESIZE(0); + queue_len = skb_queue_len(&vvs->rx_queue); + if (queue_len >= queue_max) { + /* Walking a large queue may take a significant amount of time + * and cache misses, causing traffic burstiness. Limit the + * collapse to freeing room for this packet and the next one. + * It may free more to fill each collapsed skb to capacity. + */ + virtio_transport_collapse_rx_queue(vvs, queue_len + 2 - queue_max); + } + spin_lock_bh(&vvs->rx_lock); can_enqueue = virtio_transport_inc_rx_pkt(vvs, len); -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure 2026-07-08 10:29 ` [PATCH net v2 1/2] " Stefano Garzarella @ 2026-07-08 11:00 ` Michael S. Tsirkin 2026-07-09 8:54 ` Stefano Garzarella 2026-07-09 19:07 ` Bobby Eshleman 1 sibling, 1 reply; 9+ messages in thread From: Michael S. Tsirkin @ 2026-07-08 11:00 UTC (permalink / raw) To: Stefano Garzarella Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez, Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm, Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang, stable, Brien Oberstein On Wed, Jul 08, 2026 at 12:29:03PM +0200, Stefano Garzarella wrote: > From: Stefano Garzarella <sgarzare@redhat.com> > > When many small packets accumulate in the receive queue, the skb overhead > can exceed buf_alloc even while the payload is within bounds. This causes > virtio_transport_inc_rx_pkt() to reject packets, leading to connection > resets during large transfers under backpressure. > > The issue was reported by Brien, who has a reproducer, but it is also > easily reproducible with iperf-vsock [1] using a small packet size: > > iperf3 --vsock -c $CID -l 129 > > which fails immediately without this patch but with commit 059b7dbd20a6 > ("vsock/virtio: fix potential unbounded skb queue"). > > Inspired by TCP's tcp_collapse() which solves a similar problem, add > virtio_transport_collapse_rx_queue() that walks the receive queue and > re-copies data into compact linear skbs to reduce the overhead. > > The collapse is triggered proactively from when the number of skb queued > is close to exceeding the overhead budget. > > A pre-scan counts the eligible bytes to size each allocation precisely, > avoiding waste for isolated small packets. Partially consumed skbs are > kept as-is to preserve buf_used/fwd_cnt accounting, EOM-marked skbs to > maintain SEQPACKET message boundaries, and skbs already larger than the > collapse target because they already have a good data-to-overhead ratio. > > Walking a large queue may take a significant amount of time and cache > misses, causing traffic burstiness. To limit this, the collapse stops > once enough room is freed for this packet and the next one, but may > opportunistically free more to fill each collapsed skb to capacity. > > [1] https://github.com/stefano-garzarella/iperf-vsock > > Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue") > Cc: stable@vger.kernel.org > Reported-by: Brien Oberstein <brienpub@gmail.com> > Closes: https://lore.kernel.org/netdev/618701dd023e$063de350$12b9a9f0$@gmail.com/ > Tested-by: Brien Oberstein <brienpub@gmail.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> this is the right approach Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > v2: > - defined MAX_COLLAPSE_LEN macro instead of using a variable [Paolo] > - added a threshold to avoid walking all the queue while collapsing > [Paolo] > - collapsed the queue before calling virtio_transport_inc_rx_pkt(). > While working on the threshold, I figured out that the check I was > introducing can also be used to proactively trigger the collapse, so I > moved the call to virtio_transport_collapse_rx_queue() before acquiring > the rx_lock to have also a better diff to simplify backports > - improved code readability (removed `out` label, `keep` initialization, > etc.) [Paolo + other small stuff] > - Brien kindly retested this version as well (thank you so much) > --- > net/vmw_vsock/virtio_transport_common.c | 165 +++++++++++++++++++++++- > 1 file changed, 164 insertions(+), 1 deletion(-) > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index 09475007165b..8becad81279c 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -26,6 +26,13 @@ > /* Threshold for detecting small packets to copy */ > #define GOOD_COPY_LEN 128 > > +/* Max payload that can be collapsed into a single linear skb, using the same > + * allocation threshold as virtio_vsock_alloc_skb() to avoid adding pressure > + * on the page allocator. > + */ > +#define MAX_COLLAPSE_LEN \ > + SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER) > + > 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); > @@ -420,6 +427,145 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk, > return ret; > } > > +static bool virtio_transport_can_collapse(struct sk_buff *skb) > +{ > + /* skbs that are partially consumed, mark a SEQPACKET message boundary, > + * or are already large enough should not be collapsed: they either > + * need special accounting, carry protocol state, or already have a > + * good data-to-overhead ratio. > + */ > + if (VIRTIO_VSOCK_SKB_CB(skb)->offset) > + return false; > + if (le32_to_cpu(virtio_vsock_hdr(skb)->flags) & VIRTIO_VSOCK_SEQ_EOM) > + return false; > + if (skb->len >= MAX_COLLAPSE_LEN) > + return false; > + return true; > +} > + > +/* Iterate through the packets in the queue starting from the current skb to > + * count the number of bytes we can collapse. > + */ > +static unsigned int > +virtio_transport_collapse_size(struct sk_buff *skb, struct sk_buff_head *queue) > +{ > + unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset; > + > + while ((skb = skb_peek_next(skb, queue)) && > + virtio_transport_can_collapse(skb)) { > + unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset; > + > + if (len > MAX_COLLAPSE_LEN - target) > + return target; > + > + target += len; > + } > + > + return target; > +} > + > +/* Called under lock_sock to compact the receive queue by merging small skbs. > + * @min_to_free: minimum number of skbs to eliminate from the queue. May free > + * more to fill each collapsed skb to capacity. > + */ > +static void > +virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs, > + u32 min_to_free) > +{ > + struct sk_buff *skb, *next_skb, *new_skb = NULL; > + struct sk_buff_head new_queue; > + u32 saved = 0; > + > + __skb_queue_head_init(&new_queue); > + > + skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) { > + struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb); > + u32 src_off = VIRTIO_VSOCK_SKB_CB(skb)->offset; > + u32 src_len = skb->len - src_off; > + bool keep; > + > + keep = !virtio_transport_can_collapse(skb); > + if (keep) { > + /* Finalize pending collapsed skb to preserve packet > + * ordering. > + */ > + if (new_skb) { > + __skb_queue_tail(&new_queue, new_skb); > + new_skb = NULL; > + saved--; > + } > + goto next; > + } > + > + /* Finalize if this packet won't fit in the remaining tailroom, > + * so we can allocate a right-sized new_skb. > + */ > + if (new_skb && src_len > skb_tailroom(new_skb)) { > + __skb_queue_tail(&new_queue, new_skb); > + new_skb = NULL; > + saved--; > + } > + > + if (!new_skb) { > + unsigned int alloc_size; > + > + /* Check after finalizing to opportunistically fill > + * each collapsed skb to capacity, merging more skbs > + * than strictly required. > + */ > + if (saved >= min_to_free) > + break; > + > + alloc_size = virtio_transport_collapse_size(skb, &vvs->rx_queue); > + > + /* Only this skb's data is eligible, nothing to merge > + * with. Keep as-is. > + */ > + if (alloc_size <= src_len) { > + keep = true; > + goto next; > + } > + > + new_skb = virtio_vsock_alloc_linear_skb(alloc_size + > + VIRTIO_VSOCK_SKB_HEADROOM, GFP_KERNEL); > + if (!new_skb) > + break; > + > + memcpy(virtio_vsock_hdr(new_skb), hdr, > + sizeof(struct virtio_vsock_hdr)); > + virtio_vsock_hdr(new_skb)->len = 0; > + } > + > + /* Cannot fail since src_off/src_len are within bounds, but if > + * it does, discard new_skb to avoid queuing corrupted data. > + */ > + if (WARN_ON_ONCE(skb_copy_bits(skb, src_off, > + skb_put(new_skb, src_len), > + src_len))) { > + kfree_skb(new_skb); > + new_skb = NULL; > + break; > + } > + > + le32_add_cpu(&virtio_vsock_hdr(new_skb)->len, src_len); > + virtio_vsock_hdr(new_skb)->flags |= hdr->flags; > + > +next: > + __skb_unlink(skb, &vvs->rx_queue); > + if (keep) { > + __skb_queue_tail(&new_queue, skb); > + } else { > + consume_skb(skb); > + saved++; > + } > + } > + > + if (new_skb) > + __skb_queue_tail(&new_queue, new_skb); > + > + skb_queue_splice(&new_queue, &vvs->rx_queue); > +} > + > static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, > u32 len) > { > @@ -1354,12 +1500,29 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, > { > struct virtio_vsock_sock *vvs = vsk->trans; > bool can_enqueue, free_pkt = false; > + u32 len, queue_max, queue_len; > struct virtio_vsock_hdr *hdr; > - u32 len; > > hdr = virtio_vsock_hdr(skb); > len = le32_to_cpu(hdr->len); > > + /* virtio_transport_inc_rx_pkt() rejects packets when the per-skb > + * overhead (skb_queue_len * SKB_TRUESIZE(0)) exceeds buf_alloc. > + * Proactively collapse the queue before that happens. > + * No rx_lock needed: lock_sock is held by caller, preventing > + * concurrent enqueue or dequeue. > + */ > + queue_max = vvs->buf_alloc / SKB_TRUESIZE(0); > + queue_len = skb_queue_len(&vvs->rx_queue); > + if (queue_len >= queue_max) { > + /* Walking a large queue may take a significant amount of time > + * and cache misses, causing traffic burstiness. Limit the > + * collapse to freeing room for this packet and the next one. > + * It may free more to fill each collapsed skb to capacity. > + */ > + virtio_transport_collapse_rx_queue(vvs, queue_len + 2 - queue_max); > + } > + > spin_lock_bh(&vvs->rx_lock); > > can_enqueue = virtio_transport_inc_rx_pkt(vvs, len); > -- > 2.55.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure 2026-07-08 11:00 ` Michael S. Tsirkin @ 2026-07-09 8:54 ` Stefano Garzarella 0 siblings, 0 replies; 9+ messages in thread From: Stefano Garzarella @ 2026-07-09 8:54 UTC (permalink / raw) To: Michael S. Tsirkin Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez, Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm, Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang, stable, Brien Oberstein On Wed, Jul 08, 2026 at 07:00:00AM -0400, Michael S. Tsirkin wrote: >On Wed, Jul 08, 2026 at 12:29:03PM +0200, Stefano Garzarella wrote: >> From: Stefano Garzarella <sgarzare@redhat.com> >> >> When many small packets accumulate in the receive queue, the skb overhead >> can exceed buf_alloc even while the payload is within bounds. This causes >> virtio_transport_inc_rx_pkt() to reject packets, leading to connection >> resets during large transfers under backpressure. >> >> The issue was reported by Brien, who has a reproducer, but it is also >> easily reproducible with iperf-vsock [1] using a small packet size: >> >> iperf3 --vsock -c $CID -l 129 >> >> which fails immediately without this patch but with commit 059b7dbd20a6 >> ("vsock/virtio: fix potential unbounded skb queue"). >> >> Inspired by TCP's tcp_collapse() which solves a similar problem, add >> virtio_transport_collapse_rx_queue() that walks the receive queue and >> re-copies data into compact linear skbs to reduce the overhead. >> >> The collapse is triggered proactively from when the number of skb queued >> is close to exceeding the overhead budget. >> >> A pre-scan counts the eligible bytes to size each allocation precisely, >> avoiding waste for isolated small packets. Partially consumed skbs are >> kept as-is to preserve buf_used/fwd_cnt accounting, EOM-marked skbs to >> maintain SEQPACKET message boundaries, and skbs already larger than the >> collapse target because they already have a good data-to-overhead ratio. >> >> Walking a large queue may take a significant amount of time and cache >> misses, causing traffic burstiness. To limit this, the collapse stops >> once enough room is freed for this packet and the next one, but may >> opportunistically free more to fill each collapsed skb to capacity. >> >> [1] https://github.com/stefano-garzarella/iperf-vsock >> >> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue") >> Cc: stable@vger.kernel.org >> Reported-by: Brien Oberstein <brienpub@gmail.com> >> Closes: https://lore.kernel.org/netdev/618701dd023e$063de350$12b9a9f0$@gmail.com/ >> Tested-by: Brien Oberstein <brienpub@gmail.com> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > >this is the right approach Yeah, I have a follow up to start to use skb->truesize, etc. but I guess more net-next material. > >Acked-by: Michael S. Tsirkin <mst@redhat.com> Thanks, Stefano ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure 2026-07-08 10:29 ` [PATCH net v2 1/2] " Stefano Garzarella 2026-07-08 11:00 ` Michael S. Tsirkin @ 2026-07-09 19:07 ` Bobby Eshleman 1 sibling, 0 replies; 9+ messages in thread From: Bobby Eshleman @ 2026-07-09 19:07 UTC (permalink / raw) To: Stefano Garzarella Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez, Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, Michael S. Tsirkin, kvm, Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang, stable, Brien Oberstein On Wed, Jul 08, 2026 at 12:29:03PM +0200, Stefano Garzarella wrote: > From: Stefano Garzarella <sgarzare@redhat.com> > > When many small packets accumulate in the receive queue, the skb overhead > can exceed buf_alloc even while the payload is within bounds. This causes > virtio_transport_inc_rx_pkt() to reject packets, leading to connection > resets during large transfers under backpressure. > > The issue was reported by Brien, who has a reproducer, but it is also > easily reproducible with iperf-vsock [1] using a small packet size: > > iperf3 --vsock -c $CID -l 129 > > which fails immediately without this patch but with commit 059b7dbd20a6 > ("vsock/virtio: fix potential unbounded skb queue"). > > Inspired by TCP's tcp_collapse() which solves a similar problem, add > virtio_transport_collapse_rx_queue() that walks the receive queue and > re-copies data into compact linear skbs to reduce the overhead. > > The collapse is triggered proactively from when the number of skb queued > is close to exceeding the overhead budget. > > A pre-scan counts the eligible bytes to size each allocation precisely, > avoiding waste for isolated small packets. Partially consumed skbs are > kept as-is to preserve buf_used/fwd_cnt accounting, EOM-marked skbs to > maintain SEQPACKET message boundaries, and skbs already larger than the > collapse target because they already have a good data-to-overhead ratio. > > Walking a large queue may take a significant amount of time and cache > misses, causing traffic burstiness. To limit this, the collapse stops > once enough room is freed for this packet and the next one, but may > opportunistically free more to fill each collapsed skb to capacity. > > [1] https://github.com/stefano-garzarella/iperf-vsock > > Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue") > Cc: stable@vger.kernel.org > Reported-by: Brien Oberstein <brienpub@gmail.com> > Closes: https://lore.kernel.org/netdev/618701dd023e$063de350$12b9a9f0$@gmail.com/ > Tested-by: Brien Oberstein <brienpub@gmail.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > v2: > - defined MAX_COLLAPSE_LEN macro instead of using a variable [Paolo] > - added a threshold to avoid walking all the queue while collapsing > [Paolo] > - collapsed the queue before calling virtio_transport_inc_rx_pkt(). > While working on the threshold, I figured out that the check I was > introducing can also be used to proactively trigger the collapse, so I > moved the call to virtio_transport_collapse_rx_queue() before acquiring > the rx_lock to have also a better diff to simplify backports > - improved code readability (removed `out` label, `keep` initialization, > etc.) [Paolo + other small stuff] > - Brien kindly retested this version as well (thank you so much) > --- > net/vmw_vsock/virtio_transport_common.c | 165 +++++++++++++++++++++++- > 1 file changed, 164 insertions(+), 1 deletion(-) > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index 09475007165b..8becad81279c 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -26,6 +26,13 @@ > /* Threshold for detecting small packets to copy */ > #define GOOD_COPY_LEN 128 > > +/* Max payload that can be collapsed into a single linear skb, using the same > + * allocation threshold as virtio_vsock_alloc_skb() to avoid adding pressure > + * on the page allocator. > + */ > +#define MAX_COLLAPSE_LEN \ > + SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER) > + > 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); > @@ -420,6 +427,145 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk, > return ret; > } > > +static bool virtio_transport_can_collapse(struct sk_buff *skb) > +{ > + /* skbs that are partially consumed, mark a SEQPACKET message boundary, > + * or are already large enough should not be collapsed: they either > + * need special accounting, carry protocol state, or already have a > + * good data-to-overhead ratio. > + */ > + if (VIRTIO_VSOCK_SKB_CB(skb)->offset) > + return false; > + if (le32_to_cpu(virtio_vsock_hdr(skb)->flags) & VIRTIO_VSOCK_SEQ_EOM) > + return false; > + if (skb->len >= MAX_COLLAPSE_LEN) > + return false; > + return true; > +} > + > +/* Iterate through the packets in the queue starting from the current skb to > + * count the number of bytes we can collapse. > + */ > +static unsigned int > +virtio_transport_collapse_size(struct sk_buff *skb, struct sk_buff_head *queue) > +{ > + unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset; > + > + while ((skb = skb_peek_next(skb, queue)) && > + virtio_transport_can_collapse(skb)) { > + unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset; > + > + if (len > MAX_COLLAPSE_LEN - target) > + return target; > + > + target += len; > + } > + > + return target; > +} > + > +/* Called under lock_sock to compact the receive queue by merging small skbs. > + * @min_to_free: minimum number of skbs to eliminate from the queue. May free > + * more to fill each collapsed skb to capacity. > + */ > +static void > +virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs, > + u32 min_to_free) > +{ > + struct sk_buff *skb, *next_skb, *new_skb = NULL; > + struct sk_buff_head new_queue; > + u32 saved = 0; > + > + __skb_queue_head_init(&new_queue); > + > + skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) { > + struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb); > + u32 src_off = VIRTIO_VSOCK_SKB_CB(skb)->offset; > + u32 src_len = skb->len - src_off; > + bool keep; > + > + keep = !virtio_transport_can_collapse(skb); > + if (keep) { > + /* Finalize pending collapsed skb to preserve packet > + * ordering. > + */ > + if (new_skb) { > + __skb_queue_tail(&new_queue, new_skb); > + new_skb = NULL; > + saved--; > + } > + goto next; > + } > + > + /* Finalize if this packet won't fit in the remaining tailroom, > + * so we can allocate a right-sized new_skb. > + */ > + if (new_skb && src_len > skb_tailroom(new_skb)) { > + __skb_queue_tail(&new_queue, new_skb); > + new_skb = NULL; > + saved--; > + } > + > + if (!new_skb) { > + unsigned int alloc_size; > + > + /* Check after finalizing to opportunistically fill > + * each collapsed skb to capacity, merging more skbs > + * than strictly required. > + */ > + if (saved >= min_to_free) > + break; > + > + alloc_size = virtio_transport_collapse_size(skb, &vvs->rx_queue); > + > + /* Only this skb's data is eligible, nothing to merge > + * with. Keep as-is. > + */ > + if (alloc_size <= src_len) { > + keep = true; > + goto next; > + } > + > + new_skb = virtio_vsock_alloc_linear_skb(alloc_size + > + VIRTIO_VSOCK_SKB_HEADROOM, GFP_KERNEL); > + if (!new_skb) > + break; > + > + memcpy(virtio_vsock_hdr(new_skb), hdr, > + sizeof(struct virtio_vsock_hdr)); > + virtio_vsock_hdr(new_skb)->len = 0; > + } > + > + /* Cannot fail since src_off/src_len are within bounds, but if > + * it does, discard new_skb to avoid queuing corrupted data. > + */ > + if (WARN_ON_ONCE(skb_copy_bits(skb, src_off, > + skb_put(new_skb, src_len), > + src_len))) { > + kfree_skb(new_skb); > + new_skb = NULL; > + break; > + } > + > + le32_add_cpu(&virtio_vsock_hdr(new_skb)->len, src_len); > + virtio_vsock_hdr(new_skb)->flags |= hdr->flags; > + > +next: > + __skb_unlink(skb, &vvs->rx_queue); > + if (keep) { > + __skb_queue_tail(&new_queue, skb); > + } else { > + consume_skb(skb); > + saved++; > + } > + } > + > + if (new_skb) > + __skb_queue_tail(&new_queue, new_skb); > + > + skb_queue_splice(&new_queue, &vvs->rx_queue); > +} > + > static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, > u32 len) > { > @@ -1354,12 +1500,29 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, > { > struct virtio_vsock_sock *vvs = vsk->trans; > bool can_enqueue, free_pkt = false; > + u32 len, queue_max, queue_len; > struct virtio_vsock_hdr *hdr; > - u32 len; > > hdr = virtio_vsock_hdr(skb); > len = le32_to_cpu(hdr->len); > > + /* virtio_transport_inc_rx_pkt() rejects packets when the per-skb > + * overhead (skb_queue_len * SKB_TRUESIZE(0)) exceeds buf_alloc. > + * Proactively collapse the queue before that happens. > + * No rx_lock needed: lock_sock is held by caller, preventing > + * concurrent enqueue or dequeue. > + */ > + queue_max = vvs->buf_alloc / SKB_TRUESIZE(0); > + queue_len = skb_queue_len(&vvs->rx_queue); > + if (queue_len >= queue_max) { > + /* Walking a large queue may take a significant amount of time > + * and cache misses, causing traffic burstiness. Limit the > + * collapse to freeing room for this packet and the next one. > + * It may free more to fill each collapsed skb to capacity. > + */ > + virtio_transport_collapse_rx_queue(vvs, queue_len + 2 - queue_max); > + } > + > spin_lock_bh(&vvs->rx_lock); > > can_enqueue = virtio_transport_inc_rx_pkt(vvs, len); The changes still look good to me. Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com> Thanks, Bobby ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v2 2/2] vsock/test: add test for small packets under pressure 2026-07-08 10:29 [PATCH net v2 0/2] vsock/virtio: collapse receive queue under memory pressure Stefano Garzarella 2026-07-08 10:29 ` [PATCH net v2 1/2] " Stefano Garzarella @ 2026-07-08 10:29 ` Stefano Garzarella 2026-07-08 10:59 ` Michael S. Tsirkin 2026-07-09 19:48 ` Bobby Eshleman 1 sibling, 2 replies; 9+ messages in thread From: Stefano Garzarella @ 2026-07-08 10:29 UTC (permalink / raw) To: netdev Cc: Jason Wang, Stefano Garzarella, Xuan Zhuo, Eric Dumazet, Eugenio Pérez, Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, Michael S. Tsirkin, kvm, Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang From: Stefano Garzarella <sgarzare@redhat.com> Add a test that sends 2 MB of data using randomly sized small packets (129-512 bytes) over a SOCK_STREAM connection. Packets above GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(), forcing each one into its own skb. Without receive queue collapsing, the per-skb overhead eventually exceeds buf_alloc and the connection is reset. The test verifies that all data arrives and that content integrity is preserved. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- tools/testing/vsock/vsock_test.c | 87 ++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index 76be0e4a7f0e..b4ff9f946565 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -2347,6 +2347,88 @@ static void test_stream_tx_credit_bounds_server(const struct test_opts *opts) close(fd); } +/* Test that many small packets don't cause a connection reset under pressure + * and that data integrity is preserved. Packet sizes vary randomly between + * 129 and 512 bytes, above GOOD_COPY_LEN (128) to bypass in-place coalescing + * in recv_enqueue, forcing each one into its own skb. Without receive queue + * collapsing, the per-skb overhead eventually exceeds buf_alloc and the + * connection is reset. + */ +#define COLLAPSE_PKT_MIN 129 +#define COLLAPSE_PKT_MAX 512 +#define COLLAPSE_TOTAL (2 * 1024 * 1024) + +static void test_stream_collapse_client(const struct test_opts *opts) +{ + unsigned char *data; + unsigned long hash; + size_t offset = 0; + int i, fd; + + data = malloc(COLLAPSE_TOTAL); + if (!data) { + perror("malloc"); + exit(EXIT_FAILURE); + } + + for (i = 0; i < COLLAPSE_TOTAL; i++) + data[i] = rand() & 0xff; + + fd = vsock_stream_connect(opts->peer_cid, opts->peer_port); + if (fd < 0) { + perror("connect"); + exit(EXIT_FAILURE); + } + + while (offset < COLLAPSE_TOTAL) { + size_t pkt_size = COLLAPSE_PKT_MIN + + rand() % (COLLAPSE_PKT_MAX - COLLAPSE_PKT_MIN + 1); + + pkt_size = min(pkt_size, COLLAPSE_TOTAL - offset); + + send_buf(fd, data + offset, pkt_size, 0, pkt_size); + offset += pkt_size; + } + + hash = hash_djb2(data, COLLAPSE_TOTAL); + control_writeulong(hash); + + free(data); + close(fd); +} + +static void test_stream_collapse_server(const struct test_opts *opts) +{ + unsigned long hash, remote_hash; + unsigned char *data; + int fd; + + data = malloc(COLLAPSE_TOTAL); + if (!data) { + perror("malloc"); + exit(EXIT_FAILURE); + } + + fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL); + if (fd < 0) { + perror("accept"); + exit(EXIT_FAILURE); + } + + recv_buf(fd, data, COLLAPSE_TOTAL, 0, COLLAPSE_TOTAL); + + hash = hash_djb2(data, COLLAPSE_TOTAL); + remote_hash = control_readulong(); + if (hash != remote_hash) { + fprintf(stderr, "hash mismatch: local %lu remote %lu\n", + hash, remote_hash); + exit(EXIT_FAILURE); + } + + free(data); + close(fd); +} + static struct test_case test_cases[] = { { .name = "SOCK_STREAM connection reset", @@ -2546,6 +2628,11 @@ static struct test_case test_cases[] = { .run_client = test_stream_msg_peek_client, .run_server = test_stream_peek_after_recv_server, }, + { + .name = "SOCK_STREAM small packets backpressure", + .run_client = test_stream_collapse_client, + .run_server = test_stream_collapse_server, + }, {}, }; -- 2.55.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net v2 2/2] vsock/test: add test for small packets under pressure 2026-07-08 10:29 ` [PATCH net v2 2/2] vsock/test: add test for small packets under pressure Stefano Garzarella @ 2026-07-08 10:59 ` Michael S. Tsirkin 2026-07-09 9:17 ` Stefano Garzarella 2026-07-09 19:48 ` Bobby Eshleman 1 sibling, 1 reply; 9+ messages in thread From: Michael S. Tsirkin @ 2026-07-08 10:59 UTC (permalink / raw) To: Stefano Garzarella Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez, Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm, Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang On Wed, Jul 08, 2026 at 12:29:04PM +0200, Stefano Garzarella wrote: > From: Stefano Garzarella <sgarzare@redhat.com> > > Add a test that sends 2 MB of data using randomly sized small packets > (129-512 bytes) over a SOCK_STREAM connection. Packets above > GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(), > forcing each one into its own skb. > > Without receive queue collapsing, the per-skb overhead eventually > exceeds buf_alloc and the connection is reset. The test verifies > that all data arrives and that content integrity is preserved. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> maybe cut down SO_VM_SOCKETS_BUFFER_SIZE? will make it easier to trigger? anyway Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > tools/testing/vsock/vsock_test.c | 87 ++++++++++++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > > diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c > index 76be0e4a7f0e..b4ff9f946565 100644 > --- a/tools/testing/vsock/vsock_test.c > +++ b/tools/testing/vsock/vsock_test.c > @@ -2347,6 +2347,88 @@ static void test_stream_tx_credit_bounds_server(const struct test_opts *opts) > close(fd); > } > > +/* Test that many small packets don't cause a connection reset under pressure > + * and that data integrity is preserved. Packet sizes vary randomly between > + * 129 and 512 bytes, above GOOD_COPY_LEN (128) to bypass in-place coalescing > + * in recv_enqueue, forcing each one into its own skb. Without receive queue > + * collapsing, the per-skb overhead eventually exceeds buf_alloc and the > + * connection is reset. > + */ > +#define COLLAPSE_PKT_MIN 129 > +#define COLLAPSE_PKT_MAX 512 > +#define COLLAPSE_TOTAL (2 * 1024 * 1024) > + > +static void test_stream_collapse_client(const struct test_opts *opts) > +{ > + unsigned char *data; > + unsigned long hash; > + size_t offset = 0; > + int i, fd; > + > + data = malloc(COLLAPSE_TOTAL); > + if (!data) { > + perror("malloc"); > + exit(EXIT_FAILURE); > + } > + > + for (i = 0; i < COLLAPSE_TOTAL; i++) > + data[i] = rand() & 0xff; > + > + fd = vsock_stream_connect(opts->peer_cid, opts->peer_port); > + if (fd < 0) { > + perror("connect"); > + exit(EXIT_FAILURE); > + } > + > + while (offset < COLLAPSE_TOTAL) { > + size_t pkt_size = COLLAPSE_PKT_MIN + > + rand() % (COLLAPSE_PKT_MAX - COLLAPSE_PKT_MIN + 1); > + > + pkt_size = min(pkt_size, COLLAPSE_TOTAL - offset); > + > + send_buf(fd, data + offset, pkt_size, 0, pkt_size); > + offset += pkt_size; > + } > + > + hash = hash_djb2(data, COLLAPSE_TOTAL); > + control_writeulong(hash); > + > + free(data); > + close(fd); > +} > + > +static void test_stream_collapse_server(const struct test_opts *opts) > +{ > + unsigned long hash, remote_hash; > + unsigned char *data; > + int fd; > + > + data = malloc(COLLAPSE_TOTAL); > + if (!data) { > + perror("malloc"); > + exit(EXIT_FAILURE); > + } > + > + fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL); > + if (fd < 0) { > + perror("accept"); > + exit(EXIT_FAILURE); > + } > + > + recv_buf(fd, data, COLLAPSE_TOTAL, 0, COLLAPSE_TOTAL); > + > + hash = hash_djb2(data, COLLAPSE_TOTAL); > + remote_hash = control_readulong(); > + if (hash != remote_hash) { > + fprintf(stderr, "hash mismatch: local %lu remote %lu\n", > + hash, remote_hash); > + exit(EXIT_FAILURE); > + } > + > + free(data); > + close(fd); > +} > + > static struct test_case test_cases[] = { > { > .name = "SOCK_STREAM connection reset", > @@ -2546,6 +2628,11 @@ static struct test_case test_cases[] = { > .run_client = test_stream_msg_peek_client, > .run_server = test_stream_peek_after_recv_server, > }, > + { > + .name = "SOCK_STREAM small packets backpressure", > + .run_client = test_stream_collapse_client, > + .run_server = test_stream_collapse_server, > + }, > {}, > }; > > -- > 2.55.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2 2/2] vsock/test: add test for small packets under pressure 2026-07-08 10:59 ` Michael S. Tsirkin @ 2026-07-09 9:17 ` Stefano Garzarella 0 siblings, 0 replies; 9+ messages in thread From: Stefano Garzarella @ 2026-07-09 9:17 UTC (permalink / raw) To: Michael S. Tsirkin Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez, Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm, Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang On Wed, Jul 08, 2026 at 06:59:41AM -0400, Michael S. Tsirkin wrote: >On Wed, Jul 08, 2026 at 12:29:04PM +0200, Stefano Garzarella wrote: >> From: Stefano Garzarella <sgarzare@redhat.com> >> >> Add a test that sends 2 MB of data using randomly sized small packets >> (129-512 bytes) over a SOCK_STREAM connection. Packets above >> GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(), >> forcing each one into its own skb. >> >> Without receive queue collapsing, the per-skb overhead eventually >> exceeds buf_alloc and the connection is reset. The test verifies >> that all data arrives and that content integrity is preserved. >> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > >maybe cut down SO_VM_SOCKETS_BUFFER_SIZE? will make it easier to >trigger? Currently, with the default value, the trigger is practically immediate for packets between 129 and 512 bytes, but yes, a smaller buffer size certainly makes this effect even more pronounced. > >anyway > >Acked-by: Michael S. Tsirkin <mst@redhat.com> Thanks! If I need to post a v3, I'll add it; otherwise, I guess we can leave it as is or send a follow-up for net-next. Thanks, Stefano ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2 2/2] vsock/test: add test for small packets under pressure 2026-07-08 10:29 ` [PATCH net v2 2/2] vsock/test: add test for small packets under pressure Stefano Garzarella 2026-07-08 10:59 ` Michael S. Tsirkin @ 2026-07-09 19:48 ` Bobby Eshleman 1 sibling, 0 replies; 9+ messages in thread From: Bobby Eshleman @ 2026-07-09 19:48 UTC (permalink / raw) To: Stefano Garzarella Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez, Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, Michael S. Tsirkin, kvm, Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang On Wed, Jul 08, 2026 at 12:29:04PM +0200, Stefano Garzarella wrote: > From: Stefano Garzarella <sgarzare@redhat.com> > > Add a test that sends 2 MB of data using randomly sized small packets > (129-512 bytes) over a SOCK_STREAM connection. Packets above > GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(), > forcing each one into its own skb. > > Without receive queue collapsing, the per-skb overhead eventually > exceeds buf_alloc and the connection is reset. The test verifies > that all data arrives and that content integrity is preserved. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > tools/testing/vsock/vsock_test.c | 87 ++++++++++++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > > diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c > index 76be0e4a7f0e..b4ff9f946565 100644 > --- a/tools/testing/vsock/vsock_test.c > +++ b/tools/testing/vsock/vsock_test.c > @@ -2347,6 +2347,88 @@ static void test_stream_tx_credit_bounds_server(const struct test_opts *opts) > close(fd); > } > > +/* Test that many small packets don't cause a connection reset under pressure > + * and that data integrity is preserved. Packet sizes vary randomly between > + * 129 and 512 bytes, above GOOD_COPY_LEN (128) to bypass in-place coalescing > + * in recv_enqueue, forcing each one into its own skb. Without receive queue > + * collapsing, the per-skb overhead eventually exceeds buf_alloc and the > + * connection is reset. > + */ > +#define COLLAPSE_PKT_MIN 129 > +#define COLLAPSE_PKT_MAX 512 > +#define COLLAPSE_TOTAL (2 * 1024 * 1024) > + > +static void test_stream_collapse_client(const struct test_opts *opts) > +{ > + unsigned char *data; > + unsigned long hash; > + size_t offset = 0; > + int i, fd; > + > + data = malloc(COLLAPSE_TOTAL); > + if (!data) { > + perror("malloc"); > + exit(EXIT_FAILURE); > + } > + > + for (i = 0; i < COLLAPSE_TOTAL; i++) > + data[i] = rand() & 0xff; > + > + fd = vsock_stream_connect(opts->peer_cid, opts->peer_port); > + if (fd < 0) { > + perror("connect"); > + exit(EXIT_FAILURE); > + } > + > + while (offset < COLLAPSE_TOTAL) { > + size_t pkt_size = COLLAPSE_PKT_MIN + > + rand() % (COLLAPSE_PKT_MAX - COLLAPSE_PKT_MIN + 1); > + > + pkt_size = min(pkt_size, COLLAPSE_TOTAL - offset); > + > + send_buf(fd, data + offset, pkt_size, 0, pkt_size); > + offset += pkt_size; > + } > + > + hash = hash_djb2(data, COLLAPSE_TOTAL); > + control_writeulong(hash); > + > + free(data); > + close(fd); > +} > + > +static void test_stream_collapse_server(const struct test_opts *opts) > +{ > + unsigned long hash, remote_hash; > + unsigned char *data; > + int fd; > + > + data = malloc(COLLAPSE_TOTAL); > + if (!data) { > + perror("malloc"); > + exit(EXIT_FAILURE); > + } > + > + fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL); > + if (fd < 0) { > + perror("accept"); > + exit(EXIT_FAILURE); > + } > + > + recv_buf(fd, data, COLLAPSE_TOTAL, 0, COLLAPSE_TOTAL); > + > + hash = hash_djb2(data, COLLAPSE_TOTAL); > + remote_hash = control_readulong(); > + if (hash != remote_hash) { > + fprintf(stderr, "hash mismatch: local %lu remote %lu\n", > + hash, remote_hash); > + exit(EXIT_FAILURE); > + } > + > + free(data); > + close(fd); > +} > + > static struct test_case test_cases[] = { > { > .name = "SOCK_STREAM connection reset", > @@ -2546,6 +2628,11 @@ static struct test_case test_cases[] = { > .run_client = test_stream_msg_peek_client, > .run_server = test_stream_peek_after_recv_server, > }, > + { > + .name = "SOCK_STREAM small packets backpressure", > + .run_client = test_stream_collapse_client, > + .run_server = test_stream_collapse_server, > + }, > {}, > }; > > -- > 2.55.0 > Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-09 19:49 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-08 10:29 [PATCH net v2 0/2] vsock/virtio: collapse receive queue under memory pressure Stefano Garzarella 2026-07-08 10:29 ` [PATCH net v2 1/2] " Stefano Garzarella 2026-07-08 11:00 ` Michael S. Tsirkin 2026-07-09 8:54 ` Stefano Garzarella 2026-07-09 19:07 ` Bobby Eshleman 2026-07-08 10:29 ` [PATCH net v2 2/2] vsock/test: add test for small packets under pressure Stefano Garzarella 2026-07-08 10:59 ` Michael S. Tsirkin 2026-07-09 9:17 ` Stefano Garzarella 2026-07-09 19:48 ` Bobby Eshleman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox