Re: [PATCH v2 6/8] vhost/vsock: Allocate nonlinear SKBs for handling large receive buffers

[Will Deacon <will@kernel.org>]

On Wed, Jul 02, 2025 at 06:50:59PM +0200, Stefano Garzarella wrote:
> On Tue, Jul 01, 2025 at 05:45:05PM +0100, Will Deacon wrote:
> > -static inline struct sk_buff *virtio_vsock_alloc_skb(unsigned int size, gfp_t mask)
> > +static inline struct sk_buff *
> > +__virtio_vsock_alloc_skb_with_frags(unsigned int header_len,
> > +				    unsigned int data_len,
> > +				    gfp_t mask)
> > {
> > 	struct sk_buff *skb;
> > +	int err;
> > 
> > -	if (size < VIRTIO_VSOCK_SKB_HEADROOM)
> > -		return NULL;
> 
> I would have made this change in a separate patch, but IIUC the only other
> caller is virtio_transport_alloc_skb() where this condition is implied,
> right?

At this point in the series, virtio_vsock_alloc_skb() only has a single
caller (vhost_vsock_alloc_skb()) which already has a partial bounds check
and so this patch extends it to cover the lower bound.

Later (in "vsock/virtio: Allocate nonlinear SKBs for handling large
transmit buffers"), virtio_transport_alloc_skb() gets converted over
and that's fine because it never allocates less than
VIRTIO_VSOCK_SKB_HEADROOM.

> I don't know, maybe we could have one patch where you touch this and
> virtio_vsock_skb_rx_put(), and another where you introduce nonlinear
> allocation for vhost/vsock.  What do you think? (not a strong opinion, just
> worried about doing 2 things in a single patch)

I can spin a separate patch for the bounds check.

Will