[v4] vsock/virtio: SKB allocation improvements

[PATCH v4 4/9] vsock/virtio: Resize receive buffers so that each SKB fits in a 4K page

Posted by Will Deacon 6 months, 3 weeks ago

When allocating receive buffers for the vsock virtio RX virtqueue, an
SKB is allocated with a 4140 data payload (the 44-byte packet header +
VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE). Even when factoring in the SKB
overhead, the resulting 8KiB allocation thanks to the rounding in
kmalloc_reserve() is wasteful (~3700 unusable bytes) and results in a
higher-order page allocation on systems with 4KiB pages just for the
sake of a few hundred bytes of packet data.

Limit the vsock virtio RX buffers to 4KiB per SKB, resulting in much
better memory utilisation and removing the need to allocate higher-order
pages entirely.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
 include/linux/virtio_vsock.h     | 7 ++++++-
 net/vmw_vsock/virtio_transport.c | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 97465f378ade..879f1dfa7d3a 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -106,7 +106,12 @@ static inline size_t virtio_vsock_skb_len(struct sk_buff *skb)
 	return (size_t)(skb_end_pointer(skb) - skb->head);
 }
 
-#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	(1024 * 4)
+/* Dimension the RX SKB so that the entire thing fits exactly into
+ * a single 4KiB page. This avoids wasting memory due to alloc_skb()
+ * rounding up to the next page order and also means that we
+ * don't leave higher-order pages sitting around in the RX queue.
+ */
+#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	SKB_WITH_OVERHEAD(1024 * 4)
 #define VIRTIO_VSOCK_MAX_BUF_SIZE		0xFFFFFFFFUL
 #define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE		(1024 * 64)
 
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 0166919f8705..39f346890f7f 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -307,7 +307,7 @@ virtio_transport_cancel_pkt(struct vsock_sock *vsk)
 
 static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
 {
-	int total_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE + VIRTIO_VSOCK_SKB_HEADROOM;
+	int total_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
 	struct scatterlist pkt, *p;
 	struct virtqueue *vq;
 	struct sk_buff *skb;
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v4 4/9] vsock/virtio: Resize receive buffers so that each SKB fits in a 4K page

Posted by David Woodhouse 4 weeks, 1 day ago

On Thu, 2025-07-17 at 10:01 +0100, Will Deacon wrote:
> 
> -#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	(1024 * 4)
> +/* Dimension the RX SKB so that the entire thing fits exactly into
> + * a single 4KiB page. This avoids wasting memory due to alloc_skb()
> + * rounding up to the next page order and also means that we
> + * don't leave higher-order pages sitting around in the RX queue.
> + */
> +#define
> VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	SKB_WITH_OVERHEAD(1024 * 4)

Should this be SKB_WITH_OVERHEAD()?

Or should it subtract VIRTIO_VSOCK_SKB_HEADROOM instead?

(And also, I have use cases where I want to expand this to 64KiB. Can I
make it controllable with a sockopt? module param?)

Re: [PATCH v4 4/9] vsock/virtio: Resize receive buffers so that each SKB fits in a 4K page

Posted by Stefano Garzarella 3 weeks, 5 days ago

On Thu, Jan 08, 2026 at 05:33:42PM +0100, David Woodhouse wrote:
>On Thu, 2025-07-17 at 10:01 +0100, Will Deacon wrote:
>>
>> -#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	(1024 * 4)
>> +/* Dimension the RX SKB so that the entire thing fits exactly into
>> + * a single 4KiB page. This avoids wasting memory due to alloc_skb()
>> + * rounding up to the next page order and also means that we
>> + * don't leave higher-order pages sitting around in the RX queue.
>> + */
>> +#define
>> VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	SKB_WITH_OVERHEAD(1024 * 4)
>
>Should this be SKB_WITH_OVERHEAD()?

ehm, is what the patch is doing, no?

>
>Or should it subtract VIRTIO_VSOCK_SKB_HEADROOM instead?

Why?

IIRC the goal of the patch was to have an SKB that fit entirely on one 
page, to avoid wasting memory, so yes, we are reducing the payload a 
little bit (4K vs 4K - VIRTIO_VSOCK_SKB_HEADROOM - SKB_OVERHEAD), but we 
are also reducing segmentation.

>
>(And also, I have use cases where I want to expand this to 64KiB. Can I
>make it controllable with a sockopt? module param?)

I'm not sure about sockopt, because this is really device specific and 
can't be linked to a specific socket, since the device will pre-fill the 
queue with buffers that can be assigned to different sockets.

But yeah, perhaps a module parameter would suffice, provided that it can 
only be modified at load time, otherwise we would have to do something 
similar to NIC and ethtool, but I feel that would be too complicated for 
this use case.

Thanks,
Stefano

Re: [PATCH v4 4/9] vsock/virtio: Resize receive buffers so that each SKB fits in a 4K page

Posted by Will Deacon 3 weeks, 5 days ago

On Mon, Jan 12, 2026 at 03:48:11PM +0100, Stefano Garzarella wrote:
> On Thu, Jan 08, 2026 at 05:33:42PM +0100, David Woodhouse wrote:
> > On Thu, 2025-07-17 at 10:01 +0100, Will Deacon wrote:
> > > 
> > > -#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	(1024 * 4)
> > > +/* Dimension the RX SKB so that the entire thing fits exactly into
> > > + * a single 4KiB page. This avoids wasting memory due to alloc_skb()
> > > + * rounding up to the next page order and also means that we
> > > + * don't leave higher-order pages sitting around in the RX queue.
> > > + */
> > > +#define
> > > VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	SKB_WITH_OVERHEAD(1024 * 4)
> > 
> > Should this be SKB_WITH_OVERHEAD()?
> 
> ehm, is what the patch is doing, no?
> 
> > 
> > Or should it subtract VIRTIO_VSOCK_SKB_HEADROOM instead?
> 
> Why?
> 
> IIRC the goal of the patch was to have an SKB that fit entirely on one page,
> to avoid wasting memory, so yes, we are reducing the payload a little bit
> (4K vs 4K - VIRTIO_VSOCK_SKB_HEADROOM - SKB_OVERHEAD), but we are also
> reducing segmentation.
> 
> > 
> > (And also, I have use cases where I want to expand this to 64KiB. Can I
> > make it controllable with a sockopt? module param?)

What page size are you using? At some point I had this as PAGE_SIZE but
it wasn't popular:

https://lore.kernel.org/all/20250701201400.52442b0e@pumpkin/

> I'm not sure about sockopt, because this is really device specific and can't
> be linked to a specific socket, since the device will pre-fill the queue
> with buffers that can be assigned to different sockets.
> 
> But yeah, perhaps a module parameter would suffice, provided that it can
> only be modified at load time, otherwise we would have to do something
> similar to NIC and ethtool, but I feel that would be too complicated for
> this use case.

FWIW, we carried something similar in Android for a while on the
transmit side and it was a bit of a pain to maintain; we ended up in
situations where the guest and the host had to be configured similarly
for things to work, although the non-linear support should solve those
issues now. I'm not against the idea, I just wouldn't wish that pain on
anybody else!

Anyway, if we wanted to support something similar upstream for the rx
buffers, I'd suggest specifying it as a page-order for the entire
SKB allocation and clamping it to PAGE_ALLOC_COSTLY_ORDER.

Will