[PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure

[PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure

[Stefano Garzarella]

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 from virtio_transport_recv_enqueue() when
virtio_transport_inc_rx_pkt() fails. 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.

[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>
---
 net/vmw_vsock/virtio_transport_common.c | 148 +++++++++++++++++++++++-
 1 file changed, 146 insertions(+), 2 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 09475007165b..304ea424995d 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -420,6 +420,137 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 	return ret;
 }
 
+static bool virtio_transport_can_collapse(struct sk_buff *skb,
+					  unsigned int size)
+{
+	/* 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 >= size)
+		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 max_size)
+{
+	unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
+
+	while ((skb = skb_peek_next(skb, queue)) &&
+	       virtio_transport_can_collapse(skb, max_size)) {
+		unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
+
+		if (len > max_size - target)
+			return target;
+
+		target += len;
+	}
+
+	return target;
+}
+
+/* Called under lock_sock when skb overhead exceeds the budget. */
+static void virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs)
+{
+	/* Use the same linear allocation threshold as virtio_vsock_alloc_skb()
+	 * to avoid adding pressure on the page allocator.
+	 */
+	unsigned int collapse_max = SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM,
+						  PAGE_ALLOC_COSTLY_ORDER);
+	struct sk_buff *skb, *next_skb, *new_skb = NULL;
+	struct sk_buff_head new_queue;
+
+	__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 = false;
+
+		if (!virtio_transport_can_collapse(skb, collapse_max)) {
+			/* Finalize pending collapsed skb to preserve packet
+			 * ordering.
+			 */
+			if (new_skb) {
+				__skb_queue_tail(&new_queue, new_skb);
+				new_skb = NULL;
+			}
+			keep = true;
+			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;
+		}
+
+		if (!new_skb) {
+			unsigned int alloc_size;
+
+			alloc_size = virtio_transport_collapse_size(skb, &vvs->rx_queue,
+								    collapse_max);
+
+			/* 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)
+				goto out;
+
+			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;
+			goto out;
+		}
+
+		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);
+	}
+out:
+	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)
 {
@@ -1363,8 +1494,21 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
 	spin_lock_bh(&vvs->rx_lock);
 
 	can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
-	if (!can_enqueue)
-		goto out;
+	if (!can_enqueue) {
+		/* Try to collapse the receive queue to reduce skb overhead and
+		 * make room for this packet.
+		 * Unlock rx_lock since the collapse may sleep or, in any case,
+		 * take some time to collapse the skbs, but this is safe, since
+		 * sk_lock is held by caller so no one else can enqueue or
+		 * dequeue.
+		 */
+		spin_unlock_bh(&vvs->rx_lock);
+		virtio_transport_collapse_rx_queue(vvs);
+		spin_lock_bh(&vvs->rx_lock);
+		can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
+		if (!can_enqueue)
+			goto out;
+	}
 
 	if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
 		vvs->msg_count++;
-- 
2.54.0

Re: [PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure

[Paolo Abeni]

On 6/26/26 3:48 PM, 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 from virtio_transport_recv_enqueue() when
> virtio_transport_inc_rx_pkt() fails. 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.
> 
> [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>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 148 +++++++++++++++++++++++-
>  1 file changed, 146 insertions(+), 2 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 09475007165b..304ea424995d 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -420,6 +420,137 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  	return ret;
>  }
>  
> +static bool virtio_transport_can_collapse(struct sk_buff *skb,
> +					  unsigned int size)

Why passing a `size` argument here? AFAICS the actual argument is always
a constant and IMHO rightfully so.

> +{
> +	/* 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 >= size)
> +		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 max_size)
> +{
> +	unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> +	while ((skb = skb_peek_next(skb, queue)) &&
> +	       virtio_transport_can_collapse(skb, max_size)) {
> +		unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> +		if (len > max_size - target)
> +			return target;
> +
> +		target += len;
> +	}
> +
> +	return target;
> +}
> +
> +/* Called under lock_sock when skb overhead exceeds the budget. */
> +static void virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs)
> +{
> +	/* Use the same linear allocation threshold as virtio_vsock_alloc_skb()
> +	 * to avoid adding pressure on the page allocator.
> +	 */
> +	unsigned int collapse_max = SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM,
> +						  PAGE_ALLOC_COSTLY_ORDER);
> +	struct sk_buff *skb, *next_skb, *new_skb = NULL;
> +	struct sk_buff_head new_queue;
> +
> +	__skb_queue_head_init(&new_queue);
> +
> +	skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) {

If the queue is relevantly big, walking all of it may take a significant
amount of time/cache misses and causes traffic burstines. I think you
could add an additional stop condition, i.e. when the current queue size
is below a reasonable threshold (allowing the current packet to be
inserted plus some more slack).

/P

> +		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 = false;
> +
> +		if (!virtio_transport_can_collapse(skb, collapse_max)) {

Minor nit, possibly something alike the following lead to more
compact/more readable code:


		keep = !virtio_transport_can_collapse(skb, collapse_max);
		if (keep) {

> +			/* Finalize pending collapsed skb to preserve packet
> +			 * ordering.
> +			 */
> +			if (new_skb) {
> +				__skb_queue_tail(&new_queue, new_skb);
> +				new_skb = NULL;
> +			}
> +			keep = true;
> +			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;

Possibly introduce an helper for the above 2 statements?

/P

Re: [PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure

[Stefano Garzarella]

On Tue, Jun 30, 2026 at 11:53:04AM +0200, Paolo Abeni wrote:
>On 6/26/26 3:48 PM, 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 from virtio_transport_recv_enqueue() when
>> virtio_transport_inc_rx_pkt() fails. 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.
>>
>> [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>
>> ---
>>  net/vmw_vsock/virtio_transport_common.c | 148 +++++++++++++++++++++++-
>>  1 file changed, 146 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> index 09475007165b..304ea424995d 100644
>> --- a/net/vmw_vsock/virtio_transport_common.c
>> +++ b/net/vmw_vsock/virtio_transport_common.c
>> @@ -420,6 +420,137 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>>  	return ret;
>>  }
>>
>> +static bool virtio_transport_can_collapse(struct sk_buff *skb,
>> +					  unsigned int size)
>
>Why passing a `size` argument here? AFAICS the actual argument is always
>a constant and IMHO rightfully so.

This comes from a previous implementation where this was not constant.
With the current code, I agree that a macro should be better.

I'll fix it.

>
>> +{
>> +	/* 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 >= size)
>> +		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 max_size)
>> +{
>> +	unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
>> +
>> +	while ((skb = skb_peek_next(skb, queue)) &&
>> +	       virtio_transport_can_collapse(skb, max_size)) {
>> +		unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
>> +
>> +		if (len > max_size - target)
>> +			return target;
>> +
>> +		target += len;
>> +	}
>> +
>> +	return target;
>> +}
>> +
>> +/* Called under lock_sock when skb overhead exceeds the budget. */
>> +static void virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs)
>> +{
>> +	/* Use the same linear allocation threshold as virtio_vsock_alloc_skb()
>> +	 * to avoid adding pressure on the page allocator.
>> +	 */
>> +	unsigned int collapse_max = SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM,
>> +						  PAGE_ALLOC_COSTLY_ORDER);
>> +	struct sk_buff *skb, *next_skb, *new_skb = NULL;
>> +	struct sk_buff_head new_queue;
>> +
>> +	__skb_queue_head_init(&new_queue);
>> +
>> +	skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) {
>
>If the queue is relevantly big, walking all of it may take a significant
>amount of time/cache misses and causes traffic burstines. I think you
>could add an additional stop condition, i.e. when the current queue size
>is below a reasonable threshold (allowing the current packet to be
>inserted plus some more slack).

Makes sense, any suggestion on the threshold?

I was thinking something like this: merge until we have space for at 
least 2 skbs (because for now we estimate the overhead based on the 
number of skbs, but in the future I'd like to support truesize), but 
still trying to fill collapse_max as much as possible.

Does that make sense, or should we be more aggressive?

>
>/P
>
>> +		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 = false;
>> +
>> +		if (!virtio_transport_can_collapse(skb, collapse_max)) {
>
>Minor nit, possibly something alike the following lead to more
>compact/more readable code:
>
>
>		keep = !virtio_transport_can_collapse(skb, collapse_max);
>		if (keep) {
>

Yeah, so I can remove the initialization to false. I'll change it.

>> +			/* Finalize pending collapsed skb to preserve packet
>> +			 * ordering.
>> +			 */
>> +			if (new_skb) {
>> +				__skb_queue_tail(&new_queue, new_skb);
>> +				new_skb = NULL;
>> +			}
>> +			keep = true;
>> +			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;
>
>Possibly introduce an helper for the above 2 statements?

Do you mean something like this?

static void virtio_transport_queue_skb(struct sk_buff_head *queue,
				       struct sk_buff **skb)
{
	__skb_queue_tail(queue, *skb);
	*skb = NULL;
}

Not sure, just for 2 places, but if you prefer it, I can change.

Thanks,
Stefano

Re: [PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure

[Bobby Eshleman]

On Fri, Jun 26, 2026 at 03:48:22PM +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 from virtio_transport_recv_enqueue() when
> virtio_transport_inc_rx_pkt() fails. 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.
> 
> [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>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 148 +++++++++++++++++++++++-
>  1 file changed, 146 insertions(+), 2 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 09475007165b..304ea424995d 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -420,6 +420,137 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  	return ret;
>  }
>  
> +static bool virtio_transport_can_collapse(struct sk_buff *skb,
> +					  unsigned int size)
> +{
> +	/* 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 >= size)
> +		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 max_size)
> +{
> +	unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> +	while ((skb = skb_peek_next(skb, queue)) &&
> +	       virtio_transport_can_collapse(skb, max_size)) {
> +		unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> +		if (len > max_size - target)
> +			return target;
> +
> +		target += len;
> +	}
> +
> +	return target;
> +}
> +
> +/* Called under lock_sock when skb overhead exceeds the budget. */
> +static void virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs)
> +{
> +	/* Use the same linear allocation threshold as virtio_vsock_alloc_skb()
> +	 * to avoid adding pressure on the page allocator.
> +	 */
> +	unsigned int collapse_max = SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM,
> +						  PAGE_ALLOC_COSTLY_ORDER);
> +	struct sk_buff *skb, *next_skb, *new_skb = NULL;
> +	struct sk_buff_head new_queue;
> +
> +	__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 = false;
> +
> +		if (!virtio_transport_can_collapse(skb, collapse_max)) {
> +			/* Finalize pending collapsed skb to preserve packet
> +			 * ordering.
> +			 */
> +			if (new_skb) {
> +				__skb_queue_tail(&new_queue, new_skb);
> +				new_skb = NULL;
> +			}
> +			keep = true;
> +			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;
> +		}
> +
> +		if (!new_skb) {
> +			unsigned int alloc_size;
> +
> +			alloc_size = virtio_transport_collapse_size(skb, &vvs->rx_queue,
> +								    collapse_max);
> +
> +			/* 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)
> +				goto out;
> +
> +			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;
> +			goto out;
> +		}
> +
> +		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);
> +	}
> +out:
> +	if (new_skb)
> +		__skb_queue_tail(&new_queue, new_skb);
> +
> +	skb_queue_splice(&new_queue, &vvs->rx_queue);

I think the new skbs will also need skb_set_owner_sk_safe(skb, sk)
when adding to rx_queue?

Best,
Bobby

> +}
> +
>  static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
>  					u32 len)
>  {
> @@ -1363,8 +1494,21 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>  	spin_lock_bh(&vvs->rx_lock);
>  
>  	can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> -	if (!can_enqueue)
> -		goto out;
> +	if (!can_enqueue) {
> +		/* Try to collapse the receive queue to reduce skb overhead and
> +		 * make room for this packet.
> +		 * Unlock rx_lock since the collapse may sleep or, in any case,
> +		 * take some time to collapse the skbs, but this is safe, since
> +		 * sk_lock is held by caller so no one else can enqueue or
> +		 * dequeue.
> +		 */
> +		spin_unlock_bh(&vvs->rx_lock);
> +		virtio_transport_collapse_rx_queue(vvs);
> +		spin_lock_bh(&vvs->rx_lock);
> +		can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> +		if (!can_enqueue)
> +			goto out;
> +	}
>  
>  	if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
>  		vvs->msg_count++;
> -- 
> 2.54.0
> 
> 

Re: [PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure

[Stefano Garzarella]

On Wed, Jul 01, 2026 at 09:34:35AM -0700, Bobby Eshleman wrote:
>On Fri, Jun 26, 2026 at 03:48:22PM +0200, Stefano Garzarella wrote:

[...]

>> +out:
>> +	if (new_skb)
>> +		__skb_queue_tail(&new_queue, new_skb);
>> +
>> +	skb_queue_splice(&new_queue, &vvs->rx_queue);
>
>I think the new skbs will also need skb_set_owner_sk_safe(skb, sk)
>when adding to rx_queue?

IIRC we added it in the rx path, mainily for loopback to pass the 
ownership from the tx socket to the rx socket, but here we are already 
in the rx path, so the skb will never leave this socket.

Maybe it's necessary for the eBPF path?

In any case, I can add it, but if you can help me better understand what 
it prevents, that will also help me add a comment above it.

Thanks,
Stefano

Re: [PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure

[Bobby Eshleman]

On Thu, Jul 02, 2026 at 10:56:04AM +0200, Stefano Garzarella wrote:
> On Wed, Jul 01, 2026 at 09:34:35AM -0700, Bobby Eshleman wrote:
> > On Fri, Jun 26, 2026 at 03:48:22PM +0200, Stefano Garzarella wrote:
> 
> [...]
> 
> > > +out:
> > > +	if (new_skb)
> > > +		__skb_queue_tail(&new_queue, new_skb);
> > > +
> > > +	skb_queue_splice(&new_queue, &vvs->rx_queue);
> > 
> > I think the new skbs will also need skb_set_owner_sk_safe(skb, sk)
> > when adding to rx_queue?
> 
> IIRC we added it in the rx path, mainily for loopback to pass the ownership
> from the tx socket to the rx socket, but here we are already in the rx path,
> so the skb will never leave this socket.
> 

Ah that's right, I stand corrected. There is no sender to leak in this
case.

> Maybe it's necessary for the eBPF path?

Looking through sockmap, I don't think it depends on skb->sk being
non-null either (it reassigns owner to the redirect socket anyway using
skb_set_owner_r()).

Sorry for the false alarm. LGTM.

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure

[Stefano Garzarella]

On Thu, Jul 02, 2026 at 01:09:20PM -0700, Bobby Eshleman wrote:
>On Thu, Jul 02, 2026 at 10:56:04AM +0200, Stefano Garzarella wrote:
>> On Wed, Jul 01, 2026 at 09:34:35AM -0700, Bobby Eshleman wrote:
>> > On Fri, Jun 26, 2026 at 03:48:22PM +0200, Stefano Garzarella wrote:
>>
>> [...]
>>
>> > > +out:
>> > > +	if (new_skb)
>> > > +		__skb_queue_tail(&new_queue, new_skb);
>> > > +
>> > > +	skb_queue_splice(&new_queue, &vvs->rx_queue);
>> >
>> > I think the new skbs will also need skb_set_owner_sk_safe(skb, sk)
>> > when adding to rx_queue?
>>
>> IIRC we added it in the rx path, mainily for loopback to pass the ownership
>> from the tx socket to the rx socket, but here we are already in the rx path,
>> so the skb will never leave this socket.
>>
>
>Ah that's right, I stand corrected. There is no sender to leak in this
>case.
>
>> Maybe it's necessary for the eBPF path?
>
>Looking through sockmap, I don't think it depends on skb->sk being
>non-null either (it reassigns owner to the redirect socket anyway using
>skb_set_owner_r()).
>
>Sorry for the false alarm. LGTM.

Thanks for checking!

>
>Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
>

I'm going to add a threashold as Paolo suggested. Do you prefer to look 
at v2 or should I carry your R-b ?

Thanks,
Stefano