* Re: [PATCH] vsock/virtio: fix vsockmon info leak in non-linear tap, copy
From: Arseniy Krasnov @ 2026-05-08 16:17 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Paolo Abeni, Bobby Eshleman, netdev, linux-kernel,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Yiqi Sun, kvm,
virtualization
In-Reply-To: <af4GIeSEQ_65V5C2@sgarzare-redhat>
On 08/05/2026 18:50, Stefano Garzarella wrote:
> On Thu, May 07, 2026 at 11:03:57PM +0300, Arseniy Krasnov wrote:
>>> CCing Arseniy and Bobby.
>>
>> Thanks!
>>
>>>
>>> On Tue, May 05, 2026 at 12:26:21PM +0200, Paolo Abeni wrote:
>>>> On 4/30/26 9:11 AM, Yiqi Sun wrote:
>>>>> vsockmon mirrors packets through virtio_transport_build_skb(), which
>>>>> builds a new skb and copies the payload into it. For non-linear skbs,
>>>>> this goes through virtio_transport_copy_nonlinear_skb().
>>>>>
>>>>> Helper manually initializes a iov_iter, but leaves iov_iter.count unset.
>>>>> As a result, skb_copy_datagram_iter() sees zero writable bytes
>>>>> in the destination iterator and copies no payload data.
>>>>>
>>>>> This becomes an info leak because virtio_transport_build_skb() has
>>>>> already reserved payload_len bytes in the new skb with skb_put(). The
>>>>> skb is then returned to the tap path with that payload area still
>>>>> uninitialized, so userspace reading from a vsockmon device can observe
>>>>> heap contents and potentially kernel address.
>>>>>
>>>>> Fix it by initializing iov_iter.count to the number of bytes to copy.
>>>>>
>>>>> Fixes: 4b0bf10eb077 ("vsock/virtio: non-linear skb handling for tap")
>>>>> Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
>>>>> ---
>>>>> net/vmw_vsock/virtio_transport_common.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>>>>> index 416d533f493d..6b26ee57ccab 100644
>>>>> --- a/net/vmw_vsock/virtio_transport_common.c
>>>>> +++ b/net/vmw_vsock/virtio_transport_common.c
>>>>> @@ -152,7 +152,7 @@ static void virtio_transport_copy_nonlinear_skb(const struct sk_buff *skb,
>>>>> iov_iter.nr_segs = 1;
>>>>>
>>>>> to_copy = min_t(size_t, len, skb->len);
>>>>> -
>>>>> + iov_iter.count = to_copy;
>>>>> skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
>>>>> &iov_iter, to_copy);
>>>>
>>>> @Stefano, @Stefan, the patch LGTM, but sashiko pointed out to a
>>>> pre-existing issue you should probably want to address:
>>>>
>>>>> to_copy = min_t(size_t, len, skb->len);
>>>> Does this length calculation account for the offset when a packet is
>>>> split across multiple transmissions?
>>>> If a packet is requeued, VIRTIO_VSOCK_SKB_CB(skb)->offset is increased,
>>>> but to_copy still evaluates to the full length of the skb.
>>>
>>> Yep, I just checked and vhost-vsock is the only place where we call
>>> virtio_transport_deliver_tap_pkt() wiht an offset != 0, but I agree that
>>> we should also fix it.
>>
>> Yes, looks like the only place where offset could be non zero is 'vhost_transport_do_send_pkt()'.
>> And we set valid length in header every attempt to send it:
>>
>> /* Set the correct length in the header */
>> hdr->len = cpu_to_le32(payload_len);
>>
>> In all other places we call 'virtio_transport_deliver_tap_pkt()' with offset == 0. And thus
>> skb->len == hdr->len.
>>
>> So for me looks ok. E.g. len in header is actual data.
>>
>>>
>>> Looking better in net/vmw_vsock/virtio_transport_common.c I think this
>>> is a regression, indeed we have this comment in
>>> virtio_transport_build_skb():
>>>
>>> /* A packet could be split to fit the RX buffer, so we can retrieve
>>> * the payload length from the header and the buffer pointer taking
>>> * care of the offset in the original packet.
>>> */
>>> pkt_hdr = virtio_vsock_hdr(pkt);
>>>
>>> Before commit 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with
>>> sk_buff") we read the payload lenght from the header that is always set
>>> to the right value before delivering the packet to the tap.
>>>
>>> From that commit, we don't to consider the offset anymore since we
>>> started to use `len` from the skb, so IMO we should go back to what we
>>> did before it, I mean:
>>>
>>> payload_len = le32_to_cpu(pkt->hdr.len);
>>>
>>> @Bobby do you remember why we did that change? Or if you see any issue
>>> going back to what we did initially?
>>>
>>>
>>> Also IMO we should avoid to set all the iov_iter fields by hand and
>>> start to use iov_iter_kvec(). Plus, we can just use
>>> skb_copy_datagram_iter() in any case, like we already do in vhost-vsock,
>>> since it already handles linear vs non linear.
>>>
>>> At the end I mean something like this:
>>>
>>> @@ -171,7 +150,7 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>>> * care of the offset in the original packet.
>>> */
>>> pkt_hdr = virtio_vsock_hdr(pkt);
>>> - payload_len = pkt->len;
>>> + payload_len = le32_to_cpu(pkt_hdr->len);
>>>
>>> skb = alloc_skb(sizeof(*hdr) + sizeof(*pkt_hdr) + payload_len,
>>> GFP_ATOMIC);
>>> @@ -214,13 +193,17 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>>> skb_put_data(skb, pkt_hdr, sizeof(*pkt_hdr));
>>>
>>> if (payload_len) {
>>> - if (skb_is_nonlinear(pkt)) {
>>> - void *data = skb_put(skb, payload_len);
>>> + struct iov_iter iov_iter;
>>> + struct kvec kvec;
>>> + void *data = skb_put(skb, payload_len);
>>>
>>> - virtio_transport_copy_nonlinear_skb(pkt, data, payload_len);
>>> - } else {
>>> - skb_put_data(skb, pkt->data, payload_len);
>>> - }
>>> + kvec.iov_base = data;
>>> + kvec.iov_len = payload_len;
>>> + iov_iter_kvec(&iov_iter, READ, &kvec, 1, payload_len);
>>> +
>>> + skb_copy_datagram_iter(pkt,
>>> + VIRTIO_VSOCK_SKB_CB(pkt)->offset,
>>> + &iov_iter, payload_len);
>>> }
>>>
>>> return skb;
>>>
>>> And removing virtio_transport_copy_nonlinear_skb().
>>
>> Yes, this looks shorter and better.
>
> Thanks for confirming, I'll send a series soon and CC you.
> Please review it :-)
Sure, thanks!
>
> Thanks,
> Stefano
>
^ permalink raw reply
* Re: [PATCH] vsock/virtio: fix vsockmon info leak in non-linear tap copy
From: Stefano Garzarella @ 2026-05-08 15:58 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Paolo Abeni, Arseniy Krasnov, Bobby Eshleman, stefanha, netdev,
linux-kernel, mst, jasowang, xuanzhuo, eperezma, davem, edumazet,
kuba, horms, Yiqi Sun, kvm, virtualization
In-Reply-To: <afzyM6fTTAl0Hnvm@devvm29614.prn0.facebook.com>
On Thu, May 07, 2026 at 01:12:35PM -0700, Bobby Eshleman wrote:
>On Tue, May 05, 2026 at 02:44:16PM +0200, Stefano Garzarella wrote:
>> CCing Arseniy and Bobby.
>>
>> On Tue, May 05, 2026 at 12:26:21PM +0200, Paolo Abeni wrote:
>> > On 4/30/26 9:11 AM, Yiqi Sun wrote:
>> > > vsockmon mirrors packets through virtio_transport_build_skb(), which
>> > > builds a new skb and copies the payload into it. For non-linear skbs,
>> > > this goes through virtio_transport_copy_nonlinear_skb().
>> > >
>> > > Helper manually initializes a iov_iter, but leaves iov_iter.count unset.
>> > > As a result, skb_copy_datagram_iter() sees zero writable bytes
>> > > in the destination iterator and copies no payload data.
>> > >
>> > > This becomes an info leak because virtio_transport_build_skb() has
>> > > already reserved payload_len bytes in the new skb with skb_put(). The
>> > > skb is then returned to the tap path with that payload area still
>> > > uninitialized, so userspace reading from a vsockmon device can observe
>> > > heap contents and potentially kernel address.
>> > >
>> > > Fix it by initializing iov_iter.count to the number of bytes to copy.
>> > >
>> > > Fixes: 4b0bf10eb077 ("vsock/virtio: non-linear skb handling for tap")
>> > > Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
>> > > ---
>> > > net/vmw_vsock/virtio_transport_common.c | 2 +-
>> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> > > index 416d533f493d..6b26ee57ccab 100644
>> > > --- a/net/vmw_vsock/virtio_transport_common.c
>> > > +++ b/net/vmw_vsock/virtio_transport_common.c
>> > > @@ -152,7 +152,7 @@ static void virtio_transport_copy_nonlinear_skb(const struct sk_buff *skb,
>> > > iov_iter.nr_segs = 1;
>> > >
>> > > to_copy = min_t(size_t, len, skb->len);
>> > > -
>> > > + iov_iter.count = to_copy;
>> > > skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
>> > > &iov_iter, to_copy);
>> >
>> > @Stefano, @Stefan, the patch LGTM, but sashiko pointed out to a
>> > pre-existing issue you should probably want to address:
>> >
>> > > to_copy = min_t(size_t, len, skb->len);
>> > Does this length calculation account for the offset when a packet is
>> > split across multiple transmissions?
>> > If a packet is requeued, VIRTIO_VSOCK_SKB_CB(skb)->offset is increased,
>> > but to_copy still evaluates to the full length of the skb.
>>
>> Yep, I just checked and vhost-vsock is the only place where we call
>> virtio_transport_deliver_tap_pkt() wiht an offset != 0, but I agree that we
>> should also fix it.
>>
>> Looking better in net/vmw_vsock/virtio_transport_common.c I think this is a
>> regression, indeed we have this comment in virtio_transport_build_skb():
>>
>> /* A packet could be split to fit the RX buffer, so we can retrieve
>> * the payload length from the header and the buffer pointer taking
>> * care of the offset in the original packet.
>> */
>> pkt_hdr = virtio_vsock_hdr(pkt);
>>
>> Before commit 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with
>> sk_buff") we read the payload lenght from the header that is always set to
>> the right value before delivering the packet to the tap.
>>
>> From that commit, we don't to consider the offset anymore since we started
>> to use `len` from the skb, so IMO we should go back to what we did before
>> it, I mean:
>>
>> payload_len = le32_to_cpu(pkt->hdr.len);
>>
>> @Bobby do you remember why we did that change? Or if you see any issue going
>> back to what we did initially?
>
>I think this was just one that made it through the cracks. I vaguely
>recall a few other instances where I assumed skb->len could stand-in for
>hdr.len, but it didn't hold.
>
>Using hdr.len like the original looks correct to me.
Thanks for confirming, I'll fix it.
Stefano
^ permalink raw reply
* Re: [PATCH] vsock/virtio: fix vsockmon info leak in non-linear tap, copy
From: Stefano Garzarella @ 2026-05-08 15:50 UTC (permalink / raw)
To: Arseniy Krasnov
Cc: Paolo Abeni, Bobby Eshleman, netdev, linux-kernel,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Yiqi Sun, kvm,
virtualization
In-Reply-To: <a7b95294-3aa6-4bca-9eab-a4bcd0202ae2@salutedevices.com>
On Thu, May 07, 2026 at 11:03:57PM +0300, Arseniy Krasnov wrote:
>>CCing Arseniy and Bobby.
>
>Thanks!
>
>>
>>On Tue, May 05, 2026 at 12:26:21PM +0200, Paolo Abeni wrote:
>>>On 4/30/26 9:11 AM, Yiqi Sun wrote:
>>>> vsockmon mirrors packets through virtio_transport_build_skb(), which
>>>> builds a new skb and copies the payload into it. For non-linear skbs,
>>>> this goes through virtio_transport_copy_nonlinear_skb().
>>>>
>>>> Helper manually initializes a iov_iter, but leaves iov_iter.count unset.
>>>> As a result, skb_copy_datagram_iter() sees zero writable bytes
>>>> in the destination iterator and copies no payload data.
>>>>
>>>> This becomes an info leak because virtio_transport_build_skb() has
>>>> already reserved payload_len bytes in the new skb with skb_put(). The
>>>> skb is then returned to the tap path with that payload area still
>>>> uninitialized, so userspace reading from a vsockmon device can observe
>>>> heap contents and potentially kernel address.
>>>>
>>>> Fix it by initializing iov_iter.count to the number of bytes to copy.
>>>>
>>>> Fixes: 4b0bf10eb077 ("vsock/virtio: non-linear skb handling for tap")
>>>> Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
>>>> ---
>>>> net/vmw_vsock/virtio_transport_common.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>>>> index 416d533f493d..6b26ee57ccab 100644
>>>> --- a/net/vmw_vsock/virtio_transport_common.c
>>>> +++ b/net/vmw_vsock/virtio_transport_common.c
>>>> @@ -152,7 +152,7 @@ static void virtio_transport_copy_nonlinear_skb(const struct sk_buff *skb,
>>>> iov_iter.nr_segs = 1;
>>>>
>>>> to_copy = min_t(size_t, len, skb->len);
>>>> -
>>>> + iov_iter.count = to_copy;
>>>> skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
>>>> &iov_iter, to_copy);
>>>
>>>@Stefano, @Stefan, the patch LGTM, but sashiko pointed out to a
>>>pre-existing issue you should probably want to address:
>>>
>>>> to_copy = min_t(size_t, len, skb->len);
>>>Does this length calculation account for the offset when a packet is
>>>split across multiple transmissions?
>>>If a packet is requeued, VIRTIO_VSOCK_SKB_CB(skb)->offset is increased,
>>>but to_copy still evaluates to the full length of the skb.
>>
>>Yep, I just checked and vhost-vsock is the only place where we call
>>virtio_transport_deliver_tap_pkt() wiht an offset != 0, but I agree that
>>we should also fix it.
>
>Yes, looks like the only place where offset could be non zero is 'vhost_transport_do_send_pkt()'.
>And we set valid length in header every attempt to send it:
>
> /* Set the correct length in the header */
> hdr->len = cpu_to_le32(payload_len);
>
>In all other places we call 'virtio_transport_deliver_tap_pkt()' with offset == 0. And thus
>skb->len == hdr->len.
>
>So for me looks ok. E.g. len in header is actual data.
>
>>
>>Looking better in net/vmw_vsock/virtio_transport_common.c I think this
>>is a regression, indeed we have this comment in
>>virtio_transport_build_skb():
>>
>> /* A packet could be split to fit the RX buffer, so we can retrieve
>> * the payload length from the header and the buffer pointer taking
>> * care of the offset in the original packet.
>> */
>> pkt_hdr = virtio_vsock_hdr(pkt);
>>
>>Before commit 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with
>>sk_buff") we read the payload lenght from the header that is always set
>>to the right value before delivering the packet to the tap.
>>
>> From that commit, we don't to consider the offset anymore since we
>>started to use `len` from the skb, so IMO we should go back to what we
>>did before it, I mean:
>>
>> payload_len = le32_to_cpu(pkt->hdr.len);
>>
>>@Bobby do you remember why we did that change? Or if you see any issue
>>going back to what we did initially?
>>
>>
>>Also IMO we should avoid to set all the iov_iter fields by hand and
>>start to use iov_iter_kvec(). Plus, we can just use
>>skb_copy_datagram_iter() in any case, like we already do in vhost-vsock,
>>since it already handles linear vs non linear.
>>
>>At the end I mean something like this:
>>
>>@@ -171,7 +150,7 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>> * care of the offset in the original packet.
>> */
>> pkt_hdr = virtio_vsock_hdr(pkt);
>>- payload_len = pkt->len;
>>+ payload_len = le32_to_cpu(pkt_hdr->len);
>>
>> skb = alloc_skb(sizeof(*hdr) + sizeof(*pkt_hdr) + payload_len,
>> GFP_ATOMIC);
>>@@ -214,13 +193,17 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
>> skb_put_data(skb, pkt_hdr, sizeof(*pkt_hdr));
>>
>> if (payload_len) {
>>- if (skb_is_nonlinear(pkt)) {
>>- void *data = skb_put(skb, payload_len);
>>+ struct iov_iter iov_iter;
>>+ struct kvec kvec;
>>+ void *data = skb_put(skb, payload_len);
>>
>>- virtio_transport_copy_nonlinear_skb(pkt, data, payload_len);
>>- } else {
>>- skb_put_data(skb, pkt->data, payload_len);
>>- }
>>+ kvec.iov_base = data;
>>+ kvec.iov_len = payload_len;
>>+ iov_iter_kvec(&iov_iter, READ, &kvec, 1, payload_len);
>>+
>>+ skb_copy_datagram_iter(pkt,
>>+ VIRTIO_VSOCK_SKB_CB(pkt)->offset,
>>+ &iov_iter, payload_len);
>> }
>>
>> return skb;
>>
>>And removing virtio_transport_copy_nonlinear_skb().
>
>Yes, this looks shorter and better.
Thanks for confirming, I'll send a series soon and CC you.
Please review it :-)
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v4 3/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with vfio_pci_core_get_iomap()
From: Matt Evans @ 2026-05-08 15:30 UTC (permalink / raw)
To: Alex Williamson
Cc: Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple,
Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas,
Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
In-Reply-To: <20260507162141.072483ce@shazbot.org>
Hi Alex,
On 07/05/2026 23:21, Alex Williamson wrote:
>
> On Tue, 5 May 2026 10:38:31 -0700
> Matt Evans <mattev@meta.com> wrote:
>
>> Since "vfio/pci: Set up barmap in vfio_pci_core_enable()", the
>> resource request and iomap for the BARs was performed early, and
>> vfio_pci_core_setup_barmap() just checks those actions succeeded.
>>
>> Move this logic to a new helper that checks success and returns the
>> iomap address, replacing the various bare vdev->barmap[] lookups.
>> This maintains the error behaviour of the previous on-demand
>> vfio_pci_core_setup_barmap() scheme.
>>
>> Signed-off-by: Matt Evans <mattev@meta.com>
>> ---
>> drivers/vfio/pci/nvgrace-gpu/main.c | 11 ++++-------
>> drivers/vfio/pci/vfio_pci_core.c | 11 +++++------
>> drivers/vfio/pci/vfio_pci_dmabuf.c | 2 +-
>> drivers/vfio/pci/vfio_pci_rdwr.c | 30 ++++++++---------------------
>> drivers/vfio/pci/virtio/legacy_io.c | 13 ++++++-------
>> include/linux/vfio_pci_core.h | 20 ++++++++++++++++++-
>> 6 files changed, 43 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c
>> index fa056b69f899..e153002258ce 100644
>> --- a/drivers/vfio/pci/nvgrace-gpu/main.c
>> +++ b/drivers/vfio/pci/nvgrace-gpu/main.c
>> @@ -184,13 +184,10 @@ static int nvgrace_gpu_open_device(struct vfio_device *core_vdev)
>>
>> /*
>> * GPU readiness is checked by reading the BAR0 registers.
>> - *
>> - * ioremap BAR0 to ensure that the BAR0 mapping is present before
>> - * register reads on first fault before establishing any GPU
>> - * memory mapping.
>> + * The BAR map was just set up by vfio_pci_core_enable(), so
>> + * check that was successful and bail early if not:
>> */
>> - ret = vfio_pci_core_setup_barmap(vdev, 0);
>> - if (ret)
>> + if (IS_ERR(vfio_pci_core_get_iomap(vdev, 0)))
>> goto error_exit;
>
> Sashiko notes we're not setting ret here. The bots are also paranoid
> about the unreachable condition that the get_iomap below could return an
> ERR_PTR. Maybe head off both by adding an __iomem pointer to the
> nvgrace_gpu_pci_core_device struct and a temporary one here. Store the
> iomap in the temporary variable, use it to test for IS_ERR() and
> PTR_ERR(), then set the pointer in the structure after the last error
> condition here. Add one line in the close_device to set it NULL. Then
> just use nvdev->bar0_io below.
Right about ret. On the 2nd, the bots could benefit from a comment on
the ...get_iomap() below saying that it "cannot fail" if this one
passes, but hey. I can add a struct member to track it (bots can then
worry that it might be NULL, if they don't notice that
nvgrace_gpu_check_device_ready() can't happen if
nvgrace_gpu_open_device() didn't succeed, etc. etc.).
>>
>> if (nvdev->resmem.memlength) {
>> @@ -275,7 +272,7 @@ nvgrace_gpu_check_device_ready(struct nvgrace_gpu_pci_core_device *nvdev)
>> if (!__vfio_pci_memory_enabled(vdev))
>> return -EIO;
>>
>> - ret = nvgrace_gpu_wait_device_ready(vdev->barmap[0]);
>> + ret = nvgrace_gpu_wait_device_ready(vfio_pci_core_get_iomap(vdev, 0));
>> if (ret)
>> return ret;
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> index 62931dc381d8..5c8bd13f10d0 100644
>> --- a/drivers/vfio/pci/vfio_pci_core.c
>> +++ b/drivers/vfio/pci/vfio_pci_core.c
>> @@ -1761,7 +1761,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
>> struct pci_dev *pdev = vdev->pdev;
>> unsigned int index;
>> u64 phys_len, req_len, pgoff, req_start;
>> - int ret;
>> + void __iomem *bar_io;
>>
>> index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
>>
>> @@ -1795,12 +1795,11 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
>> return -EINVAL;
>>
>> /*
>> - * Even though we don't make use of the barmap for the mmap,
>> - * we need to request the region and the barmap tracks that.
>> + * Ensure the BAR resource region is reserved for use.
>> */
>> - ret = vfio_pci_core_setup_barmap(vdev, index);
>> - if (ret)
>> - return ret;
>> + bar_io = vfio_pci_core_get_iomap(vdev, index);
>> + if (IS_ERR(bar_io))
>> + return PTR_ERR(bar_io);
>>
>> vma->vm_private_data = vdev;
>> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
>> index 69a5c2d511e6..46cd44b22c9c 100644
>> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
>> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
>> @@ -248,7 +248,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
>> * else. Check that PCI resources have been claimed for it.
>> */
>> if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX ||
>> - vfio_pci_core_setup_barmap(vdev, get_dma_buf.region_index))
>> + IS_ERR(vfio_pci_core_get_iomap(vdev, get_dma_buf.region_index)))
>> return -ENODEV;
>>
>> dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges,
>> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
>> index 3bfbb879a005..7f14dd46de17 100644
>> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
>> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
>> @@ -198,19 +198,6 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
>> }
>> EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
>>
>> -/*
>> - * The barmap is set up in vfio_pci_core_enable(). Callers use this
>> - * function to check that the BAR resources are requested or that the
>> - * pci_iomap() was done.
>> - */
>> -int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar)
>> -{
>> - if (IS_ERR(vdev->barmap[bar]))
>> - return PTR_ERR(vdev->barmap[bar]);
>> - return 0;
>> -}
>> -EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap);
>> -
>> ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>> size_t count, loff_t *ppos, bool iswrite)
>> {
>> @@ -262,13 +249,11 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>> */
>> max_width = VFIO_PCI_IO_WIDTH_4;
>> } else {
>> - int ret = vfio_pci_core_setup_barmap(vdev, bar);
>> - if (ret) {
>> - done = ret;
>> + io = vfio_pci_core_get_iomap(vdev, bar);
>> + if (IS_ERR(io)) {
>> + done = PTR_ERR(io);
>> goto out;
>> }
>> -
>> - io = vdev->barmap[bar];
>> }
>>
>> if (bar == vdev->msix_bar) {
>> @@ -423,6 +408,7 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset,
>> loff_t pos = offset & VFIO_PCI_OFFSET_MASK;
>> int ret, bar = VFIO_PCI_OFFSET_TO_INDEX(offset);
>> struct vfio_pci_ioeventfd *ioeventfd;
>> + void __iomem *io;
>>
>> /* Only support ioeventfds into BARs */
>> if (bar > VFIO_PCI_BAR5_REGION_INDEX)
>> @@ -440,9 +426,9 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset,
>> if (count == 8)
>> return -EINVAL;
>>
>> - ret = vfio_pci_core_setup_barmap(vdev, bar);
>> - if (ret)
>> - return ret;
>> + io = vfio_pci_core_get_iomap(vdev, bar);
>> + if (IS_ERR(io))
>> + return PTR_ERR(io);
>
> Sashiko seems to note a real existing error here that should also be
> pulled out to a separate fix. Given the right offset, this could
> generate a negative BAR value.
Yuck, loff_t signed, yep. Isn't the real root of this that it
never makes sense for VFIO_PCI_OFFSET_TO_INDEX() to return a negative
index here or anywhere else?
I suggest instead, to also avoid this elsewhere in future, something
like:
#define VFIO_PCI_OFFSET_TO_INDEX(off) ((u64)(off) >> VFIO_PCI_OFFSET_SHIFT)
> The test at the end of the previous
> chunk should should be expanded to `if (bar < 0 || bar > ...BAR5...)`.
Not necessary if VFIO_PCI_OFFSET_TO_INDEX() can't return < 0 (the
magnitude would be 24b so can't overflow the `int bar` it's assigned
into).
> Do you want to pick that up in this series? I think it's the only case
> that lets that slip through. Thanks,
Sure, I'll post a fix. I don't think it needs to be part of this series
though if it's just the macro, do you agree?
Do you know why drivers/gpu/drm/i915/gvt/kvmgt.c has copied
VFIO_PCI_OFFSET_TO_INDEX() and friends? Perhaps the shift was different
(the reason drivers/vfio/pci/ism/main.c has its own versions). The same
loff_t issue seems to exist in both of those places, unfortunately.
Matt
PS: with minor question:
Relatedly, I'd made `bar` an int following existing convention in
vfio_pci_core_get_iomap(struct vfio_pci_core_device *vdev, int bar)
But I'll make this `unsigned int`, please flag if this violates taste
and decency. IMO any BAR/index parameter should be unsigned; most are,
some signed remain.
^ permalink raw reply
* Re: [PATCH] vdpa/mlx5: Use kvzalloc_flex() for MTT command memory
From: Dragos Tatulea @ 2026-05-08 15:13 UTC (permalink / raw)
To: Rosen Penev, virtualization
Cc: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
open list
In-Reply-To: <20260508051837.1744409-1-rosenp@gmail.com>
On Thu, May 07, 2026 at 10:18:37PM -0700, Rosen Penev wrote:
> The create mkey command memory embeds the MTT array as a flexible array
> member. Use kvzalloc_flex() to allocate it directly instead of open-coding
> the struct_size() calculation with kvcalloc().
>
> The MTT allocation still needs to be aligned to MLX5_VDPA_MTT_ALIGN bytes.
> Since each MTT entry is __be64, align the entry count directly and avoid
> carrying a separate byte length variable.
>
> Assisted-by: Codex:GPT-5.5
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/vdpa/mlx5/core/mr.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index 42c2705077a6..6d02ccf9eb91 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -221,11 +221,10 @@ static int create_direct_keys(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *
>
> list_for_each_entry(dmr, &mr->head, list) {
> struct mlx5_create_mkey_mem *cmd_mem;
> - int mttlen, mttcount;
> + int mttcount;
>
> - mttlen = roundup(MLX5_ST_SZ_BYTES(mtt) * dmr->nsg, MLX5_VDPA_MTT_ALIGN);
> - mttcount = mttlen / sizeof(cmd_mem->mtt[0]);
> - cmd_mem = kvcalloc(1, struct_size(cmd_mem, mtt, mttcount), GFP_KERNEL);
> + mttcount = ALIGN(dmr->nsg, MLX5_VDPA_MTT_ALIGN / sizeof(cmd_mem->mtt[0]));
> + cmd_mem = kvzalloc_flex(*cmd_mem, mtt, mttcount);
> if (!cmd_mem) {
> err = -ENOMEM;
> goto done;
> --
> 2.54.0
>
Thanks for the patch. Code and commit message looks good.
Not sure about the Assisted-by tag, but that is not my call.
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Thanks,
Dragos
^ permalink raw reply
* [PATCH net-next v11 1/4] tun/tap: add ptr_ring consume helper with netdev queue wakeup
From: Simon Schippers @ 2026-05-08 15:10 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260508151048.183125-1-simon.schippers@tu-dortmund.de>
Introduce tun_ring_consume() that wraps ptr_ring_consume() and calls
__tun_wake_queue(). The latter wakes the stopped netdev subqueue once
half of the ring capacity has been consumed, tracked via the new
cons_cnt field in tun_file. As a safety net, the queue is also woken on
the last consumed entry if it leaves the ring empty. The point is to
allow the queue to be stopped when it gets full, which is required for
traffic shaping - implemented by the following "avoid ptr_ring tail-drop
when a qdisc is present".
Some implementation details:
- tun_ring_recv() replaces ptr_ring_consume() with tun_ring_consume()
to properly wake the queue on purge.
- tun_queue_purge() also replaces ptr_ring_consume()
with tun_ring_consume().
- __tun_detach() locks the tx_ring.consumer_lock to avoid races with
the consumer on the queue_index.
- Reset cons_cnt in tun_attach() so the half-ring wake threshold is
valid for the new ring size after ptr_ring_resize().
- The upcoming patch explains the pairing of the smp_mb() of
__tun_wake_queue().
- tun_queue_resize() wakes all queues after resizing with the proper
tx_ring.consumer_lock and resets the cons_cnt to avoid a possible
stale queue.
Without the corresponding queue stopping, this patch alone causes no
regression for a tap setup sending to a qemu VM: 1.132 Mpps
to 1.134 Mpps.
Details: AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU
threads, pktgen sender; Avg over 50 runs @ 100,000,000 packets;
SRSO and spectre v2 mitigations disabled.
Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
drivers/net/tun.c | 73 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 64 insertions(+), 9 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index b183189f1853..b24cc899a890 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -145,6 +145,8 @@ struct tun_file {
struct list_head next;
struct tun_struct *detached;
struct ptr_ring tx_ring;
+ /* Protected by tx_ring.consumer_lock */
+ int cons_cnt;
struct xdp_rxq_info xdp_rxq;
};
@@ -557,11 +559,43 @@ void tun_ptr_free(void *ptr)
}
EXPORT_SYMBOL_GPL(tun_ptr_free);
-static void tun_queue_purge(struct tun_file *tfile)
+/* Callers must hold ring.consumer_lock */
+static void __tun_wake_queue(struct tun_struct *tun,
+ struct tun_file *tfile, int consumed)
+{
+ struct netdev_queue *txq = netdev_get_tx_queue(tun->dev,
+ tfile->queue_index);
+
+ /* Paired with smp_mb__after_atomic() in tun_net_xmit() */
+ smp_mb();
+ if (netif_tx_queue_stopped(txq)) {
+ tfile->cons_cnt += consumed;
+ if (tfile->cons_cnt >= tfile->tx_ring.size / 2 ||
+ __ptr_ring_empty(&tfile->tx_ring)) {
+ netif_tx_wake_queue(txq);
+ tfile->cons_cnt = 0;
+ }
+ }
+}
+
+static void *tun_ring_consume(struct tun_struct *tun, struct tun_file *tfile)
+{
+ void *ptr;
+
+ spin_lock(&tfile->tx_ring.consumer_lock);
+ ptr = __ptr_ring_consume(&tfile->tx_ring);
+ if (ptr)
+ __tun_wake_queue(tun, tfile, 1);
+
+ spin_unlock(&tfile->tx_ring.consumer_lock);
+ return ptr;
+}
+
+static void tun_queue_purge(struct tun_struct *tun, struct tun_file *tfile)
{
void *ptr;
- while ((ptr = ptr_ring_consume(&tfile->tx_ring)) != NULL)
+ while ((ptr = tun_ring_consume(tun, tfile)) != NULL)
tun_ptr_free(ptr);
skb_queue_purge(&tfile->sk.sk_write_queue);
@@ -588,8 +622,10 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
rcu_assign_pointer(tun->tfiles[index],
tun->tfiles[tun->numqueues - 1]);
ntfile = rtnl_dereference(tun->tfiles[index]);
+ spin_lock(&ntfile->tx_ring.consumer_lock);
ntfile->queue_index = index;
ntfile->xdp_rxq.queue_index = index;
+ spin_unlock(&ntfile->tx_ring.consumer_lock);
rcu_assign_pointer(tun->tfiles[tun->numqueues - 1],
NULL);
@@ -605,7 +641,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
synchronize_net();
tun_flow_delete_by_queue(tun, tun->numqueues + 1);
/* Drop read queue */
- tun_queue_purge(tfile);
+ tun_queue_purge(tun, tfile);
tun_set_real_num_queues(tun);
} else if (tfile->detached && clean) {
tun = tun_enable_queue(tfile);
@@ -670,14 +706,14 @@ static void tun_detach_all(struct net_device *dev)
tfile = rtnl_dereference(tun->tfiles[i]);
tun_napi_del(tfile);
/* Drop read queue */
- tun_queue_purge(tfile);
+ tun_queue_purge(tun, tfile);
xdp_rxq_info_unreg(&tfile->xdp_rxq);
sock_put(&tfile->sk);
}
list_for_each_entry_safe(tfile, tmp, &tun->disabled, next) {
tun_napi_del(tfile);
tun_enable_queue(tfile);
- tun_queue_purge(tfile);
+ tun_queue_purge(tun, tfile);
xdp_rxq_info_unreg(&tfile->xdp_rxq);
sock_put(&tfile->sk);
}
@@ -687,6 +723,13 @@ static void tun_detach_all(struct net_device *dev)
module_put(THIS_MODULE);
}
+static void tun_reset_cons_cnt(struct tun_file *tfile)
+{
+ spin_lock(&tfile->tx_ring.consumer_lock);
+ tfile->cons_cnt = 0;
+ spin_unlock(&tfile->tx_ring.consumer_lock);
+}
+
static int tun_attach(struct tun_struct *tun, struct file *file,
bool skip_filter, bool napi, bool napi_frags,
bool publish_tun)
@@ -730,6 +773,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
goto out;
}
+ tun_reset_cons_cnt(tfile);
tfile->queue_index = tun->numqueues;
tfile->socket.sk->sk_shutdown &= ~RCV_SHUTDOWN;
@@ -2115,13 +2159,14 @@ static ssize_t tun_put_user(struct tun_struct *tun,
return total;
}
-static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
+static void *tun_ring_recv(struct tun_struct *tun, struct tun_file *tfile,
+ int noblock, int *err)
{
DECLARE_WAITQUEUE(wait, current);
void *ptr = NULL;
int error = 0;
- ptr = ptr_ring_consume(&tfile->tx_ring);
+ ptr = tun_ring_consume(tun, tfile);
if (ptr)
goto out;
if (noblock) {
@@ -2133,7 +2178,7 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
while (1) {
set_current_state(TASK_INTERRUPTIBLE);
- ptr = ptr_ring_consume(&tfile->tx_ring);
+ ptr = tun_ring_consume(tun, tfile);
if (ptr)
break;
if (signal_pending(current)) {
@@ -2170,7 +2215,7 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
if (!ptr) {
/* Read frames from ring */
- ptr = tun_ring_recv(tfile, noblock, &err);
+ ptr = tun_ring_recv(tun, tfile, noblock, &err);
if (!ptr)
return err;
}
@@ -3622,6 +3667,16 @@ static int tun_queue_resize(struct tun_struct *tun)
dev->tx_queue_len, GFP_KERNEL,
tun_ptr_free);
+ if (!ret) {
+ for (i = 0; i < tun->numqueues; i++) {
+ tfile = rtnl_dereference(tun->tfiles[i]);
+ spin_lock(&tfile->tx_ring.consumer_lock);
+ netif_wake_subqueue(tun->dev, tfile->queue_index);
+ tfile->cons_cnt = 0;
+ spin_unlock(&tfile->tx_ring.consumer_lock);
+ }
+ }
+
kfree(rings);
return ret;
}
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v11 2/4] vhost-net: wake queue of tun/tap after ptr_ring consume
From: Simon Schippers @ 2026-05-08 15:10 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260508151048.183125-1-simon.schippers@tu-dortmund.de>
Add tun_wake_queue() to tun.c and export it for use by vhost-net. The
function validates that the file belongs to a tun/tap device and that
the tfile exists, dereferences the tun_struct under RCU, and delegates
to __tun_wake_queue().
vhost_net_buf_produce() now calls tun_wake_queue() after a successful
batched consume of the ring to allow the netdev subqueue to be woken up.
The point is to allow the queue to be stopped when it gets full, which
is required for traffic shaping - implemented by the following
"avoid ptr_ring tail-drop when a qdisc is present".
Without the corresponding queue stopping, this patch alone causes no
throughput regression for a tap+vhost-net setup sending to a qemu VM:
3.857 Mpps to 3.891 Mpps.
Details: AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU
threads, XDP drop program active in VM, pktgen sender; Avg over
50 runs @ 100,000,000 packets. SRSO and spectre v2 mitigations disabled.
Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
drivers/net/tun.c | 23 +++++++++++++++++++++++
drivers/vhost/net.c | 21 +++++++++++++++------
include/linux/if_tun.h | 3 +++
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index b24cc899a890..4ee1ed6e815a 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -3785,6 +3785,29 @@ struct ptr_ring *tun_get_tx_ring(struct file *file)
}
EXPORT_SYMBOL_GPL(tun_get_tx_ring);
+/* Callers must hold ring.consumer_lock */
+void tun_wake_queue(struct file *file, int consumed)
+{
+ struct tun_file *tfile;
+ struct tun_struct *tun;
+
+ if (file->f_op != &tun_fops)
+ return;
+
+ tfile = file->private_data;
+ if (!tfile)
+ return;
+
+ rcu_read_lock();
+
+ tun = rcu_dereference(tfile->tun);
+ if (tun)
+ __tun_wake_queue(tun, tfile, consumed);
+
+ rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(tun_wake_queue);
+
module_init(tun_init);
module_exit(tun_cleanup);
MODULE_DESCRIPTION(DRV_DESCRIPTION);
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 80965181920c..ee583d6cc0fa 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -176,13 +176,21 @@ static void *vhost_net_buf_consume(struct vhost_net_buf *rxq)
return ret;
}
-static int vhost_net_buf_produce(struct vhost_net_virtqueue *nvq)
+static int vhost_net_buf_produce(struct sock *sk,
+ struct vhost_net_virtqueue *nvq)
{
+ struct file *file = sk->sk_socket->file;
struct vhost_net_buf *rxq = &nvq->rxq;
rxq->head = 0;
- rxq->tail = ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
- VHOST_NET_BATCH);
+ spin_lock(&nvq->rx_ring->consumer_lock);
+ rxq->tail = __ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
+ VHOST_NET_BATCH);
+
+ if (rxq->tail)
+ tun_wake_queue(file, rxq->tail);
+
+ spin_unlock(&nvq->rx_ring->consumer_lock);
return rxq->tail;
}
@@ -209,14 +217,15 @@ static int vhost_net_buf_peek_len(void *ptr)
return __skb_array_len_with_tag(ptr);
}
-static int vhost_net_buf_peek(struct vhost_net_virtqueue *nvq)
+static int vhost_net_buf_peek(struct sock *sk,
+ struct vhost_net_virtqueue *nvq)
{
struct vhost_net_buf *rxq = &nvq->rxq;
if (!vhost_net_buf_is_empty(rxq))
goto out;
- if (!vhost_net_buf_produce(nvq))
+ if (!vhost_net_buf_produce(sk, nvq))
return 0;
out:
@@ -995,7 +1004,7 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
unsigned long flags;
if (rvq->rx_ring)
- return vhost_net_buf_peek(rvq);
+ return vhost_net_buf_peek(sk, rvq);
spin_lock_irqsave(&sk->sk_receive_queue.lock, flags);
head = skb_peek(&sk->sk_receive_queue);
diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
index 80166eb62f41..5f3e206c7a73 100644
--- a/include/linux/if_tun.h
+++ b/include/linux/if_tun.h
@@ -22,6 +22,7 @@ struct tun_msg_ctl {
#if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE)
struct socket *tun_get_socket(struct file *);
struct ptr_ring *tun_get_tx_ring(struct file *file);
+void tun_wake_queue(struct file *file, int consumed);
static inline bool tun_is_xdp_frame(void *ptr)
{
@@ -55,6 +56,8 @@ static inline struct ptr_ring *tun_get_tx_ring(struct file *f)
return ERR_PTR(-EINVAL);
}
+static inline void tun_wake_queue(struct file *f, int consumed) {}
+
static inline bool tun_is_xdp_frame(void *ptr)
{
return false;
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v11 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Simon Schippers @ 2026-05-08 15:10 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260508151048.183125-1-simon.schippers@tu-dortmund.de>
This commit prevents tail-drop when a qdisc is present and the ptr_ring
becomes full. Once the ring reaches capacity after a produce attempt,
the netdev queue is stopped instead of dropping subsequent packets.
If no qdisc is present, the previous tail-drop behavior is preserved.
If producing an entry fails anyway due to a race, tun_net_xmit() drops
the packet. Such races are expected because LLTX is enabled and the
transmit path operates without the usual locking.
The __tun_wake_queue() function of the consumer races with the producer
for waking/stopping the netdev queue, which could result in a stalled
queue. Therefore, an smp_mb__after_atomic() is introduced that pairs
with the smp_mb() of the consumer. It follows the principle of store
buffering described in tools/memory-model/Documentation/recipes.txt:
- The producer in tun_net_xmit() first sets __QUEUE_STATE_DRV_XOFF,
followed by an smp_mb__after_atomic() (= smp_mb()), and then reads the
ring with __ptr_ring_check_produce().
- The consumer in __tun_wake_queue() first writes zero to the ring in
__ptr_ring_consume(), followed by an smp_mb(), and then reads the queue
status with netif_tx_queue_stopped().
=> Following the aforementioned principle, it is impossible for the
producer to see a full ring (and therefore not wake the queue on the
re-check) while the consumer simultaneously fails to see a stopped
queue (and therefore also does not wake it).
Benchmarks:
The benchmarks show a slight regression in raw transmission performance
when using two sending threads. Packet loss also occurs only in the
two-thread sending case; no packet loss was observed with a single
sending thread.
Test setup:
AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
mitigations disabled.
Note for tap+vhost-net:
XDP drop program active in VM -> ~2.5x faster; slower for tap due to
more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
+--------------------------+--------------+----------------+----------+
| 1 thread | Stock | Patched with | diff |
| sending | | fq_codel qdisc | |
+------------+-------------+--------------+----------------+----------+
| TAP | Received | 1.132 Mpps | 1.123 Mpps | -0.8% |
| +-------------+--------------+----------------+----------+
| | Lost/s | 3.765 Mpps | 0 pps | |
+------------+-------------+--------------+----------------+----------+
| TAP | Received | 3.857 Mpps | 3.901 Mpps | +1.1% |
| +-------------+--------------+----------------+----------+
| +vhost-net | Lost/s | 0.802 Mpps | 0 pps | |
+------------+-------------+--------------+----------------+----------+
+--------------------------+--------------+----------------+----------+
| 2 threads | Stock | Patched with | diff |
| sending | | fq_codel qdisc | |
+------------+-------------+--------------+----------------+----------+
| TAP | Received | 1.115 Mpps | 1.081 Mpps | -3.0% |
| +-------------+--------------+----------------+----------+
| | Lost/s | 8.490 Mpps | 391 pps | |
+------------+-------------+--------------+----------------+----------+
| TAP | Received | 3.664 Mpps | 3.555 Mpps | -3.0% |
| +-------------+--------------+----------------+----------+
| +vhost-net | Lost/s | 5.330 Mpps | 938 pps | |
+------------+-------------+--------------+----------------+----------+
Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
drivers/net/tun.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 4ee1ed6e815a..e56358878c36 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1052,6 +1052,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
struct netdev_queue *queue;
struct tun_file *tfile;
int len = skb->len;
+ int ret;
rcu_read_lock();
tfile = rcu_dereference(tun->tfiles[txq]);
@@ -1106,13 +1107,33 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
nf_reset_ct(skb);
- if (ptr_ring_produce(&tfile->tx_ring, skb)) {
+ queue = netdev_get_tx_queue(dev, txq);
+
+ spin_lock(&tfile->tx_ring.producer_lock);
+ ret = __ptr_ring_produce(&tfile->tx_ring, skb);
+ if (!qdisc_txq_has_no_queue(queue) &&
+ __ptr_ring_check_produce(&tfile->tx_ring) == -ENOSPC) {
+ netif_tx_stop_queue(queue);
+ /* Paired with smp_mb() in __tun_wake_queue() */
+ smp_mb__after_atomic();
+ if (!__ptr_ring_check_produce(&tfile->tx_ring))
+ netif_tx_wake_queue(queue);
+ }
+ spin_unlock(&tfile->tx_ring.producer_lock);
+
+ if (ret) {
+ /* This should be a rare case if a qdisc is present, but
+ * can happen due to lltx.
+ * Since skb_tx_timestamp(), skb_orphan(),
+ * run_ebpf_filter() and pskb_trim() could have tinkered
+ * with the SKB, returning NETDEV_TX_BUSY is unsafe and
+ * we must drop instead.
+ */
drop_reason = SKB_DROP_REASON_FULL_RING;
goto drop;
}
/* dev->lltx requires to do our own update of trans_start */
- queue = netdev_get_tx_queue(dev, txq);
txq_trans_cond_update(queue);
/* Notify and wake up reader process */
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v11 3/4] ptr_ring: move free-space check into separate helper
From: Simon Schippers @ 2026-05-08 15:10 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260508151048.183125-1-simon.schippers@tu-dortmund.de>
This patch moves the check for available free space for a new entry into
a separate function. Existing callers that only check for a non-zero
return value are unaffected; __ptr_ring_produce() now returns -EINVAL
for a zero-size ring and -ENOSPC when full, whereas before both cases
returned -ENOSPC. The new helper allows callers to determine in advance
whether subsequent __ptr_ring_produce() calls will succeed. This
information can, for example, be used to temporarily stop producing until
__ptr_ring_check_produce() indicates that space is available again.
Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
include/linux/ptr_ring.h | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
index d2c3629bbe45..c95e891903f0 100644
--- a/include/linux/ptr_ring.h
+++ b/include/linux/ptr_ring.h
@@ -96,6 +96,20 @@ static inline bool ptr_ring_full_bh(struct ptr_ring *r)
return ret;
}
+/* Note: callers invoking this in a loop must use a compiler barrier,
+ * for example cpu_relax(). Callers must hold producer_lock.
+ */
+static inline int __ptr_ring_check_produce(struct ptr_ring *r)
+{
+ if (unlikely(!r->size))
+ return -EINVAL;
+
+ if (data_race(r->queue[r->producer]))
+ return -ENOSPC;
+
+ return 0;
+}
+
/* Note: callers invoking this in a loop must use a compiler barrier,
* for example cpu_relax(). Callers must hold producer_lock.
* Callers are responsible for making sure pointer that is being queued
@@ -103,8 +117,10 @@ static inline bool ptr_ring_full_bh(struct ptr_ring *r)
*/
static inline int __ptr_ring_produce(struct ptr_ring *r, void *ptr)
{
- if (unlikely(!r->size) || data_race(r->queue[r->producer]))
- return -ENOSPC;
+ int p = __ptr_ring_check_produce(r);
+
+ if (p)
+ return p;
/* Make sure the pointer we are storing points to a valid data. */
/* Pairs with the dependency ordering in __ptr_ring_consume. */
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v11 0/4] tun/tap & vhost-net: apply qdisc backpressure on full ptr_ring to reduce TX drops
From: Simon Schippers @ 2026-05-08 15:10 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
This patch series deals with tun/tap & vhost-net which drop incoming
SKBs whenever their internal ptr_ring buffer is full. Instead, with this
patch series, the associated netdev queue is stopped - but only when a
qdisc is attached. If no qdisc is present the existing behavior is
preserved. The XDP transmit path is not affected. This patch series
touches tun/tap and vhost-net, as they share common logic and must be
updated together. Modifying only one of them would break the other.
By applying proper backpressure, this change allows the connected qdisc to
operate correctly, as reported in [1], and significantly improves
performance in real-world scenarios, as demonstrated in our paper [2]. For
example, we observed a 36% TCP throughput improvement for an OpenVPN
connection between Germany and the USA.
Synthetic pktgen benchmarks indicate a slight regression, and packet
loss is reduced to near zero. Pktgen benchmarks are provided per commit,
with the final commit showing the overall performance.
Thanks!
[1] Link: https://unix.stackexchange.com/questions/762935/traffic-shaping-ineffective-on-tun-device
[2] Link: https://cni.etit.tu-dortmund.de/storages/cni-etit/r/Research/Publications/2025/Gebauer_2025_VTCFall/Gebauer_VTCFall2025_AuthorsVersion.pdf
---
Changelog:
v11:
- Renamed __ptr_ring_produce_peek() to __ptr_ring_check_produce()
(Sashiko)
- Add return code -EINVAL to __ptr_ring_check_produce() which lets
tun_net_xmit() stop the queue only on -ENOSPC. (MST)
- Resolve race on tfile->queue_index by locking tx_ring.consumer_lock
in __tun_detach(). (Sashiko)
- Wake the queue in tun_queue_resize() to avoid possible stalls.
- Other minor adjustments & reran the benchmarks.
v10: https://lore.kernel.org/netdev/20260506141033.180450-1-simon.schippers@tu-dortmund.de/
- Changed the term "Transmitted" to "Received" in the benchmarks,
as correctly pointed out by MST, and reran the benchmarks.
Addressed the Sashiko AI review:
- Avoid a data race on tfile->cons_cnt by always locking.
- Correctly count the number of consumed packets for vhost-net.
- Corrected a typo in the commit message of commit 3.
- Added a missing barrier on the consumer side.
--> The barriers now follow the "store buffering" principle.
- No longer return NETDEV_TX_BUSY at all, because it is unsafe.
--> Result: There are still a few drops with multiple senders, which
would be avoided by disabling LLTX.
V9: https://lore.kernel.org/netdev/20260428123859.19578-1-simon.schippers@tu-dortmund.de/
- Addressed minor nit by MST in patches 1 and 2.
- Rebased patch 3 because of commit d748047
("ptr_ring: disable KCSAN warnings").
- Documented the pair of the smp_mb__after_atomic() in tun_net_xmit()
with tun_ring_consume().
--> It simply pairs with the test_and_clear_bit() inside of
netif_wake_subqueue().
- Use 1 ptr_ring consumer spinlock instead of 2.
- Ran pktgen benchmarks with pg_set SHARED for 50 iterations on
latest kernel
--> No significant performance difference noticed
V8: https://lore.kernel.org/netdev/20260312130639.138988-1-simon.schippers@tu-dortmund.de/
- Drop code changes in drivers/net/tap.c; The code there deals with
ipvtap/macvtap which are unrelated to the goal of this patch series
and I did not realize that before
-> Greatly simplified logic, 4 instead of 9 commits
-> No more duplicated logics and distinction in vhost required
- Only wake after the queue stopped and half of the ring was consumed
as suggested by MST
-> Performance improvements for TAP, but still slightly slower
- Better benchmarking with pinned threads, XDP drop program for
tap+vhost-net and disabling CPU mitigations (and newer Ryzen 5 5600X
processor) as suggested by Jason Wang
V7: https://lore.kernel.org/netdev/20260107210448.37851-1-simon.schippers@tu-dortmund.de/
- Switch to an approach similar to veth (excluding the recently fixed
variant), as suggested by MST, with minor adjustments discussed in V6
- Rename the cover-letter title
- Add multithreaded pktgen and iperf3 benchmarks, as suggested by Jason
Wang
- Rework __ptr_ring_consume_created_space() so it can also be used after
batched consume
...
---
Simon Schippers (4):
tun/tap: add ptr_ring consume helper with netdev queue wakeup
vhost-net: wake queue of tun/tap after ptr_ring consume
ptr_ring: move free-space check into separate helper
tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
drivers/net/tun.c | 121 +++++++++++++++++++++++++++++++++++----
drivers/vhost/net.c | 21 +++++--
include/linux/if_tun.h | 3 +
include/linux/ptr_ring.h | 20 ++++++-
4 files changed, 146 insertions(+), 19 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v4 1/3] vfio/pci: Set up BAR resources and maps in vfio_pci_core_enable()
From: Matt Evans @ 2026-05-08 14:14 UTC (permalink / raw)
To: Alex Williamson
Cc: Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple,
Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas,
Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
In-Reply-To: <20260507162116.3b9cbd98@shazbot.org>
Hi Alex,
On 07/05/2026 23:21, Alex Williamson wrote:
>
> On Tue, 5 May 2026 10:38:29 -0700
> Matt Evans <mattev@meta.com> wrote:
>
>> Previously BAR resource requests and the corresponding pci_iomap()
>> were performed on-demand and without synchronisation, which was racy.
>> Rather than add synchronisation, it's simplest to address this by
>> doing both activities from vfio_pci_core_enable().
>>
>> The resource allocation and/or pci_iomap() can still fail; their
>> status is tracked and existing calls to vfio_pci_core_setup_barmap()
>> will fail in a similar way to before. This keeps the point of failure
>> as observed by userspace the same, i.e. failures to request/map unused
>> BARs are benign.
>>
>> Fixes: 89e1f7d4c66d ("vfio: Add PCI device driver")
>> Signed-off-by: Matt Evans <mattev@meta.com>
>> ---
>> drivers/vfio/pci/vfio_pci_core.c | 36 +++++++++++++++++++++++++++++++-
>> drivers/vfio/pci/vfio_pci_rdwr.c | 26 +++++++----------------
>> 2 files changed, 42 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> index 3f8d093aacf8..62931dc381d8 100644
>> --- a/drivers/vfio/pci/vfio_pci_core.c
>> +++ b/drivers/vfio/pci/vfio_pci_core.c
>> @@ -482,6 +482,39 @@ static int vfio_pci_core_runtime_resume(struct device *dev)
>> }
>> #endif /* CONFIG_PM */
>>
>> +/*
>> + * Eager-request BAR resources, and iomap them. Soft failures are
>> + * allowed, and consumers must check the barmap before use in order to
>> + * give compatible user-visible behaviour with the previous on-demand
>> + * allocation method.
>> + */
>> +static void vfio_pci_core_map_bars(struct vfio_pci_core_device *vdev)
>> +{
>> + struct pci_dev *pdev = vdev->pdev;
>> + int i;
>> +
>> + for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>> + int bar = i + PCI_STD_RESOURCES;
>> +
>> + vdev->barmap[bar] = ERR_PTR(-ENODEV);
>> +
>> + if (!pci_resource_len(pdev, i))
>> + continue;
>> +
>> + if (pci_request_selected_regions(pdev, 1 << bar, "vfio")) {
>> + pci_dbg(vdev->pdev, "Failed to reserve region %d\n", bar);
>> + vdev->barmap[bar] = ERR_PTR(-EBUSY);
>> + continue;
>> + }
>> +
>> + vdev->barmap[bar] = pci_iomap(pdev, bar, 0);
>> + if (!vdev->barmap[bar]) {
>
> Sashiko notes[1] correctly that we need to release the requested region
> here.
>
> [1]https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260505173835.2324179-1-mattev@meta.com__;Iw!!Bt8RZUm9aw!75pHBGTcV8AYGiGGjzomqZLfDp7iR_j2JC6qCiJufoo7TxJTPuViQZjqp7I3ZRPPxwj1YtYSNQ$
Hnnng. Right, fixed.
-Matt
>> + pci_dbg(vdev->pdev, "Failed to iomap region %d\n", bar);
>> + vdev->barmap[bar] = ERR_PTR(-ENOMEM);
>> + }
>> + }
>> +}
>> +
>> /*
>> * The pci-driver core runtime PM routines always save the device state
>> * before going into suspended state. If the device is going into low power
>> @@ -568,6 +601,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
>> if (!vfio_vga_disabled() && vfio_pci_is_vga(pdev))
>> vdev->has_vga = true;
>>
>> + vfio_pci_core_map_bars(vdev);
>>
>> return 0;
>>
>> @@ -648,7 +682,7 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
>>
>> for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>> bar = i + PCI_STD_RESOURCES;
>> - if (!vdev->barmap[bar])
>> + if (IS_ERR_OR_NULL(vdev->barmap[bar]))
>> continue;
>> pci_iounmap(pdev, vdev->barmap[bar]);
>> pci_release_selected_regions(pdev, 1 << bar);
>> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
>> index 4251ee03e146..3bfbb879a005 100644
>> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
>> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
>> @@ -198,27 +198,15 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
>> }
>> EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
>>
>> +/*
>> + * The barmap is set up in vfio_pci_core_enable(). Callers use this
>> + * function to check that the BAR resources are requested or that the
>> + * pci_iomap() was done.
>> + */
>> int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar)
>> {
>> - struct pci_dev *pdev = vdev->pdev;
>> - int ret;
>> - void __iomem *io;
>> -
>> - if (vdev->barmap[bar])
>> - return 0;
>> -
>> - ret = pci_request_selected_regions(pdev, 1 << bar, "vfio");
>> - if (ret)
>> - return ret;
>> -
>> - io = pci_iomap(pdev, bar, 0);
>> - if (!io) {
>> - pci_release_selected_regions(pdev, 1 << bar);
>> - return -ENOMEM;
>> - }
>> -
>> - vdev->barmap[bar] = io;
>> -
>> + if (IS_ERR(vdev->barmap[bar]))
>> + return PTR_ERR(vdev->barmap[bar]);
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap);
>
^ permalink raw reply
* Re: [PATCH v2] mm/balloon: expose per-node balloon pages in node meminfo
From: David Hildenbrand (Arm) @ 2026-05-08 10:57 UTC (permalink / raw)
To: Hao Ge, Andrew Morton; +Cc: linux-mm, virtualization, linux-kernel
In-Reply-To: <20260508094736.142467-1-hao.ge@linux.dev>
On 5/8/26 11:47, Hao Ge wrote:
> Commit 835de37603ef ("meminfo: add a per node counter for balloon
> drivers") added NR_BALLOON_PAGES and exposed it in /proc/meminfo.
> However, the per-node view at /sys/devices/system/node/nodeX/meminfo
> was not updated, even though the counter is already tracked per-node.
>
> Add it to node_read_meminfo() so users can see balloon usage per
> NUMA node without having to parse the raw vmstat file.
>
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> ---
Patch subject should not start with "mm/balloon:".
"proc/meminfo: " might be the better choice. Likely can be changed when picking up
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-08 10:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, Eric Dumazet, Stefan Hajnoczi, virtualization,
David S. Miller, Jason Wang, Simon Horman, linux-kernel,
Paolo Abeni, Xuan Zhuo, kvm, Jakub Kicinski, Eugenio Pérez
In-Reply-To: <20260508063104-mutt-send-email-mst@kernel.org>
On Fri, May 08, 2026 at 06:33:13AM -0400, Michael S. Tsirkin wrote:
>On Fri, May 08, 2026 at 12:01:50PM +0200, Stefano Garzarella wrote:
>> On Fri, May 08, 2026 at 05:53:07AM -0400, Michael S. Tsirkin wrote:
>> > On Fri, May 08, 2026 at 11:23:30AM +0200, Stefano Garzarella wrote:
>> > > From: Stefano Garzarella <sgarzare@redhat.com>
>> > >
>> > > After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
>> > > queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
>> > > buf_alloc when checking whether a new packet fits. This reduces the
>> > > effective receive buffer below what the user configured via
>> > > SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
>> > > silently dropped and applications that rely on the full buffer size
>> > > to deadlock.
>> > >
>> > > Also, the reduced space is not communicated to the remote peer, so
>> > > its credit calculation accounts more credit than the receiver will
>> > > actually accept, causing data loss (there is no retransmission).
>> > >
>> > > This also causes failures in tools/testing/vsock/vsock_test.c.
>> > > Test 18 sometimes fails, while test 22 always fails in this way:
>> > > 18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
>> > >
>> > > 22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
>> > > Resource temporarily unavailable
>> > >
>> > > Fix this by introducing virtio_transport_rx_buf_size() to calculate the
>> > > size of the RX buffer based on the overhead. Using it in the acceptance
>> > > check, the advertised buf_alloc, and the credit update decision.
>> > > Use buf_alloc * 2 as total budget (payload + overhead), similar to how
>> > > SO_RCVBUF is doubled to reserve space for sk_buff metadata.
>> > > The function returns buf_alloc as long as overhead fits within the
>> > > reservation, then gradually reduces toward 0 as overhead exceeds
>> > > buf_alloc (e.g. under small-packet flooding), informing the peer to
>> > > slow down.
>> > >
>> > > Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
>> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> >
>> >
>> > unfortunately, this is a bit of a spec violation and there is no guarantee
>> > it helps.
>>
>> Loosing data like we are doing in 059b7dbd20a6 is even worse IMHO.
>>
>> >
>> > a spec violation because the spec says:
>> > Only payload bytes are counted and header bytes are not
>> > included
>> >
>> > and the implication is that a side can not reduce its own buf_alloc.
>> >
>> > no guarantee because the other side is not required to process your
>> > packets, so it might not see your buf alloc reduction.
>> >
>> > as designed in the current spec, you can only increase your buf alloc,
>> > not decrease it.
>>
>> We never enforced this, currently an user can reduce it by
>> SO_VM_SOCKETS_BUFFER_SIZE and we haven't blocked it since virtio-vsock was
>> introduced, should we update the spec?
>
>
>it's not that we need to enforce it, it's that all synchronization
>assumes this. as in, anyone can use an old copy until they run out
>of credits.
>
>
>> >
>> > what can be done:
>> > - more efficient storage for small packets (poc i posted)
>> > - reduce buf alloc ahead of the time
>>
>> That's basically what I'm doing here: I'm using twice the size of
>> `buf_alloc` (just like `SO_RCVBUF` does for other socket types) and telling
>> the other peer just `buf_alloc`.
>>
>> But then, somehow, we have to let the other person know that we're running
>> out of space. With this patch that only happens when the other peer isn't
>> behaving properly, sending so many small packets that the overhead exceeds
>> `buf_alloc`.
>>
>> Stefano
>
>what is "not proper" here, it is up to the application what to send.
Sure, but here we're just slowing down the application by telling it we
don't have any more space.
Again, without this patch we are just dropping data, which IMO is even
worse.
So I think we should merge this for now, while we handle better the EOM.
If you prefer, I can drop the part where we reduce the buf_alloc
advertised to the other peer, but at least we should drop data after
`buf_alloc * 2` IMO.
Stefano
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Michael S. Tsirkin @ 2026-05-08 10:33 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Eric Dumazet, Stefan Hajnoczi, virtualization,
David S. Miller, Jason Wang, Simon Horman, linux-kernel,
Paolo Abeni, Xuan Zhuo, kvm, Jakub Kicinski, Eugenio Pérez
In-Reply-To: <af2y2Fyp7H-om-ur@sgarzare-redhat>
On Fri, May 08, 2026 at 12:01:50PM +0200, Stefano Garzarella wrote:
> On Fri, May 08, 2026 at 05:53:07AM -0400, Michael S. Tsirkin wrote:
> > On Fri, May 08, 2026 at 11:23:30AM +0200, Stefano Garzarella wrote:
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > >
> > > After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
> > > queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
> > > buf_alloc when checking whether a new packet fits. This reduces the
> > > effective receive buffer below what the user configured via
> > > SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
> > > silently dropped and applications that rely on the full buffer size
> > > to deadlock.
> > >
> > > Also, the reduced space is not communicated to the remote peer, so
> > > its credit calculation accounts more credit than the receiver will
> > > actually accept, causing data loss (there is no retransmission).
> > >
> > > This also causes failures in tools/testing/vsock/vsock_test.c.
> > > Test 18 sometimes fails, while test 22 always fails in this way:
> > > 18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
> > >
> > > 22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
> > > Resource temporarily unavailable
> > >
> > > Fix this by introducing virtio_transport_rx_buf_size() to calculate the
> > > size of the RX buffer based on the overhead. Using it in the acceptance
> > > check, the advertised buf_alloc, and the credit update decision.
> > > Use buf_alloc * 2 as total budget (payload + overhead), similar to how
> > > SO_RCVBUF is doubled to reserve space for sk_buff metadata.
> > > The function returns buf_alloc as long as overhead fits within the
> > > reservation, then gradually reduces toward 0 as overhead exceeds
> > > buf_alloc (e.g. under small-packet flooding), informing the peer to
> > > slow down.
> > >
> > > Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >
> >
> > unfortunately, this is a bit of a spec violation and there is no guarantee
> > it helps.
>
> Loosing data like we are doing in 059b7dbd20a6 is even worse IMHO.
>
> >
> > a spec violation because the spec says:
> > Only payload bytes are counted and header bytes are not
> > included
> >
> > and the implication is that a side can not reduce its own buf_alloc.
> >
> > no guarantee because the other side is not required to process your
> > packets, so it might not see your buf alloc reduction.
> >
> > as designed in the current spec, you can only increase your buf alloc,
> > not decrease it.
>
> We never enforced this, currently an user can reduce it by
> SO_VM_SOCKETS_BUFFER_SIZE and we haven't blocked it since virtio-vsock was
> introduced, should we update the spec?
it's not that we need to enforce it, it's that all synchronization
assumes this. as in, anyone can use an old copy until they run out
of credits.
> >
> > what can be done:
> > - more efficient storage for small packets (poc i posted)
> > - reduce buf alloc ahead of the time
>
> That's basically what I'm doing here: I'm using twice the size of
> `buf_alloc` (just like `SO_RCVBUF` does for other socket types) and telling
> the other peer just `buf_alloc`.
>
> But then, somehow, we have to let the other person know that we're running
> out of space. With this patch that only happens when the other peer isn't
> behaving properly, sending so many small packets that the overhead exceeds
> `buf_alloc`.
>
> Stefano
what is "not proper" here, it is up to the application what to send.
> >
> > > ---
> > > net/vmw_vsock/virtio_transport_common.c | 31 +++++++++++++++++++++----
> > > 1 file changed, 27 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > > index 9b8014516f4f..94a4beb8fd61 100644
> > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > @@ -444,12 +444,32 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > > return ret;
> > > }
> > >
> > > +/* vvs->rx_lock held by the caller */
> > > +static u32 virtio_transport_rx_buf_size(struct virtio_vsock_sock *vvs)
> > > +{
> > > + u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
> > > + /* Use buf_alloc * 2 as total budget (payload + overhead), similar to
> > > + * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
> > > + */
> > > + u64 total_budget = (u64)vvs->buf_alloc * 2;
> > > +
> > > + /* Overhead within buf_alloc: full buf_alloc available for payload */
> > > + if (skb_overhead < vvs->buf_alloc)
> > > + return vvs->buf_alloc;
> > > +
> > > + /* Overhead exceeded buf_alloc: gradually reduce to bound skb queue */
> > > + if (skb_overhead < total_budget)
> > > + return total_budget - skb_overhead;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> > > u32 len)
> > > {
> > > - u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
> > > + u32 rx_buf_size = virtio_transport_rx_buf_size(vvs);
> > >
> > > - if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
> > > + if (!rx_buf_size || vvs->buf_used + len > rx_buf_size)
> > > return false;
> > >
> > > vvs->rx_bytes += len;
> > > @@ -472,7 +492,7 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *
> > > spin_lock_bh(&vvs->rx_lock);
> > > vvs->last_fwd_cnt = vvs->fwd_cnt;
> > > hdr->fwd_cnt = cpu_to_le32(vvs->fwd_cnt);
> > > - hdr->buf_alloc = cpu_to_le32(vvs->buf_alloc);
> > > + hdr->buf_alloc = cpu_to_le32(virtio_transport_rx_buf_size(vvs));
> > > spin_unlock_bh(&vvs->rx_lock);
> > > }
> > > EXPORT_SYMBOL_GPL(virtio_transport_inc_tx_pkt);
> > > @@ -594,6 +614,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> > > bool low_rx_bytes;
> > > int err = -EFAULT;
> > > size_t total = 0;
> > > + u32 rx_buf_size;
> > > u32 free_space;
> > >
> > > spin_lock_bh(&vvs->rx_lock);
> > > @@ -639,7 +660,9 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> > > }
> > >
> > > fwd_cnt_delta = vvs->fwd_cnt - vvs->last_fwd_cnt;
> > > - free_space = vvs->buf_alloc - fwd_cnt_delta;
> > > + rx_buf_size = virtio_transport_rx_buf_size(vvs);
> > > + free_space = rx_buf_size > fwd_cnt_delta ?
> > > + rx_buf_size - fwd_cnt_delta : 0;
> > > low_rx_bytes = (vvs->rx_bytes <
> > > sock_rcvlowat(sk_vsock(vsk), 0, INT_MAX));
> > >
> > > --
> > > 2.54.0
> >
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix potential unbounded skb queue
From: Stefano Garzarella @ 2026-05-08 10:11 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eric Dumazet, Arseniy Krasnov, Bobby Eshleman, Stefan Hajnoczi,
David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
netdev, eric.dumazet, Arseniy Krasnov, Jason Wang, Xuan Zhuo,
Eugenio Pérez, kvm, virtualization
In-Reply-To: <20260508055343-mutt-send-email-mst@kernel.org>
On Fri, May 08, 2026 at 05:58:06AM -0400, Michael S. Tsirkin wrote:
>On Fri, May 08, 2026 at 11:41:21AM +0200, Stefano Garzarella wrote:
>> On Thu, May 07, 2026 at 06:48:47PM -0400, Michael S. Tsirkin wrote:
>> > On Thu, May 07, 2026 at 02:59:13PM +0200, Stefano Garzarella wrote:
>> > > On Thu, May 07, 2026 at 07:45:10AM -0400, Michael S. Tsirkin wrote:
>> > > > On Thu, May 07, 2026 at 11:09:47AM +0200, Stefano Garzarella wrote:
>>
>> [...]
>>
>> > > > > For now, we're already doing something:
>> > > > > merging the skuffs if they don't have EOM set.
>> > > >
>> > > >
>> > > > Right that's good. You could go further and merge with EOM too
>> > > > if you stick the info about message boundaries somewhere else.
>> > >
>> > > This adds a lot of complexity IMO, but we can try.
>> > >
>> > > Do you have something in mind?
>> >
>> > BER is clearly overkill but here's a POC that claude made for me,
>> > just to give u an idea. It's clearly has a ton of issues,
>> > for example I dislike how GFP_ATOMIC is handled.
>>
>> Okay, I somewhat understand, but clearly this isn't net material
>
>I doubt we have many other options given reverting the regression was
>ruled out.
As Eric pointed out, we can't revert it.
>
>
>> so for now
>> I think the best thing to do is to merge the fixup I sent (or something
>> similar):
>> https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
>
>I reviewed that one, problem is it's a spec violation/change that we'll
>have to support forever.
I have a few points to make on this, but let's discuss them there.
>
>> This is a major change that should be merged with more caution.
>> Could this have too much of an impact on performance?
>>
>> Thanks,
>> Stefano
>
>It's really a POC, real patch is left as an excersise for the reader
>:).
eheh, I see, but honestly, this overcomplication scares me. I'll try to
think it over.
>The correct approach IMHO is to only start using this
>when we wasted a lot of memory on small packets.
>
>For example, if sum(truesize) >= buf size.
>
>then we'll not see a perf impact unless it's already pathological.
Agree on this, which is similar to what I'm doing in that patch.
Reducing the advertised buf_alloc only in pathological cases (e.g.
overhead > buf_alloc).
Stefano
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-08 10:01 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, Eric Dumazet, Stefan Hajnoczi, virtualization,
David S. Miller, Jason Wang, Simon Horman, linux-kernel,
Paolo Abeni, Xuan Zhuo, kvm, Jakub Kicinski, Eugenio Pérez
In-Reply-To: <20260508055125-mutt-send-email-mst@kernel.org>
On Fri, May 08, 2026 at 05:53:07AM -0400, Michael S. Tsirkin wrote:
>On Fri, May 08, 2026 at 11:23:30AM +0200, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
>> queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
>> buf_alloc when checking whether a new packet fits. This reduces the
>> effective receive buffer below what the user configured via
>> SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
>> silently dropped and applications that rely on the full buffer size
>> to deadlock.
>>
>> Also, the reduced space is not communicated to the remote peer, so
>> its credit calculation accounts more credit than the receiver will
>> actually accept, causing data loss (there is no retransmission).
>>
>> This also causes failures in tools/testing/vsock/vsock_test.c.
>> Test 18 sometimes fails, while test 22 always fails in this way:
>> 18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
>>
>> 22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
>> Resource temporarily unavailable
>>
>> Fix this by introducing virtio_transport_rx_buf_size() to calculate the
>> size of the RX buffer based on the overhead. Using it in the acceptance
>> check, the advertised buf_alloc, and the credit update decision.
>> Use buf_alloc * 2 as total budget (payload + overhead), similar to how
>> SO_RCVBUF is doubled to reserve space for sk_buff metadata.
>> The function returns buf_alloc as long as overhead fits within the
>> reservation, then gradually reduces toward 0 as overhead exceeds
>> buf_alloc (e.g. under small-packet flooding), informing the peer to
>> slow down.
>>
>> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>
>
>unfortunately, this is a bit of a spec violation and there is no guarantee
>it helps.
Loosing data like we are doing in 059b7dbd20a6 is even worse IMHO.
>
>a spec violation because the spec says:
>Only payload bytes are counted and header bytes are not
>included
>
>and the implication is that a side can not reduce its own buf_alloc.
>
>no guarantee because the other side is not required to process your
>packets, so it might not see your buf alloc reduction.
>
>as designed in the current spec, you can only increase your buf alloc,
>not decrease it.
We never enforced this, currently an user can reduce it by
SO_VM_SOCKETS_BUFFER_SIZE and we haven't blocked it since virtio-vsock
was introduced, should we update the spec?
>
>what can be done:
>- more efficient storage for small packets (poc i posted)
>- reduce buf alloc ahead of the time
That's basically what I'm doing here: I'm using twice the size of
`buf_alloc` (just like `SO_RCVBUF` does for other socket types) and
telling the other peer just `buf_alloc`.
But then, somehow, we have to let the other person know that we're
running out of space. With this patch that only happens when the other
peer isn't behaving properly, sending so many small packets that the
overhead exceeds `buf_alloc`.
Stefano
>
>> ---
>> net/vmw_vsock/virtio_transport_common.c | 31 +++++++++++++++++++++----
>> 1 file changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> index 9b8014516f4f..94a4beb8fd61 100644
>> --- a/net/vmw_vsock/virtio_transport_common.c
>> +++ b/net/vmw_vsock/virtio_transport_common.c
>> @@ -444,12 +444,32 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> return ret;
>> }
>>
>> +/* vvs->rx_lock held by the caller */
>> +static u32 virtio_transport_rx_buf_size(struct virtio_vsock_sock *vvs)
>> +{
>> + u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
>> + /* Use buf_alloc * 2 as total budget (payload + overhead), similar to
>> + * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
>> + */
>> + u64 total_budget = (u64)vvs->buf_alloc * 2;
>> +
>> + /* Overhead within buf_alloc: full buf_alloc available for payload */
>> + if (skb_overhead < vvs->buf_alloc)
>> + return vvs->buf_alloc;
>> +
>> + /* Overhead exceeded buf_alloc: gradually reduce to bound skb queue */
>> + if (skb_overhead < total_budget)
>> + return total_budget - skb_overhead;
>> +
>> + return 0;
>> +}
>> +
>> static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
>> u32 len)
>> {
>> - u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
>> + u32 rx_buf_size = virtio_transport_rx_buf_size(vvs);
>>
>> - if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
>> + if (!rx_buf_size || vvs->buf_used + len > rx_buf_size)
>> return false;
>>
>> vvs->rx_bytes += len;
>> @@ -472,7 +492,7 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *
>> spin_lock_bh(&vvs->rx_lock);
>> vvs->last_fwd_cnt = vvs->fwd_cnt;
>> hdr->fwd_cnt = cpu_to_le32(vvs->fwd_cnt);
>> - hdr->buf_alloc = cpu_to_le32(vvs->buf_alloc);
>> + hdr->buf_alloc = cpu_to_le32(virtio_transport_rx_buf_size(vvs));
>> spin_unlock_bh(&vvs->rx_lock);
>> }
>> EXPORT_SYMBOL_GPL(virtio_transport_inc_tx_pkt);
>> @@ -594,6 +614,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
>> bool low_rx_bytes;
>> int err = -EFAULT;
>> size_t total = 0;
>> + u32 rx_buf_size;
>> u32 free_space;
>>
>> spin_lock_bh(&vvs->rx_lock);
>> @@ -639,7 +660,9 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
>> }
>>
>> fwd_cnt_delta = vvs->fwd_cnt - vvs->last_fwd_cnt;
>> - free_space = vvs->buf_alloc - fwd_cnt_delta;
>> + rx_buf_size = virtio_transport_rx_buf_size(vvs);
>> + free_space = rx_buf_size > fwd_cnt_delta ?
>> + rx_buf_size - fwd_cnt_delta : 0;
>> low_rx_bytes = (vvs->rx_bytes <
>> sock_rcvlowat(sk_vsock(vsk), 0, INT_MAX));
>>
>> --
>> 2.54.0
>
^ permalink raw reply
* Re: [PATCH] mm/balloon: expose per-node balloon pages in node meminfo
From: Hao Ge @ 2026-05-08 9:59 UTC (permalink / raw)
To: David Hildenbrand (Arm), Andrew Morton
Cc: linux-mm, virtualization, linux-kernel
In-Reply-To: <731d7324-4eb7-44d2-bb7d-62b6fbe11c52@kernel.org>
Hi David
On 2026/5/8 16:23, David Hildenbrand (Arm) wrote:
> On 5/8/26 03:53, Hao Ge wrote:
>> Commit 835de37603ef ("meminfo: add a per node counter for balloon
>> drivers") added NR_BALLOON_PAGES and exposed it in /proc/meminfo.
>> However, the per-node view at /sys/devices/system/node/nodeX/meminfo
>> was not updated, even though the counter is already tracked per-node.
>>
>> Add it to node_read_meminfo() so users can see balloon usage per
>> NUMA node without having to parse the raw vmstat file.
> Using ballooning with vNUMA is rather rare. But sure, why not.
Yeah, it's indeed not common.
We came across this while analyzing a customer's 16C 32G VM with 2 vNUMA
nodes and balloon enabled.
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> ---
>> drivers/base/node.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/node.c b/drivers/base/node.c
>> index d7647d077b66..53f4e51d6d82 100644
>> --- a/drivers/base/node.c
>> +++ b/drivers/base/node.c
>> @@ -513,6 +513,7 @@ static ssize_t node_read_meminfo(struct device *dev,
>> "Node %d Slab: %8lu kB\n"
>> "Node %d SReclaimable: %8lu kB\n"
>> "Node %d SUnreclaim: %8lu kB\n"
>> + "Node %d Balloon: %8lu kB\n"
>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> "Node %d AnonHugePages: %8lu kB\n"
>> "Node %d ShmemHugePages: %8lu kB\n"
>> @@ -543,7 +544,8 @@ static ssize_t node_read_meminfo(struct device *dev,
>> node_page_state(pgdat, NR_KERNEL_MISC_RECLAIMABLE)),
>> nid, K(sreclaimable + sunreclaimable),
>> nid, K(sreclaimable),
>> - nid, K(sunreclaimable)
>> + nid, K(sunreclaimable),
>> + nid, K(node_page_state(pgdat, NR_BALLOON_PAGES))
>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> ,
>> nid, K(node_page_state(pgdat, NR_ANON_THPS)),
>
> Shouldn't it be placed under "Unaccepted:", just like for /proc/meminfo?
>
Good catch, thanks. I overlooked this detail -- will fix in v2.
Thanks
Best Regards
Hao
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix potential unbounded skb queue
From: Michael S. Tsirkin @ 2026-05-08 9:58 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Eric Dumazet, Arseniy Krasnov, Bobby Eshleman, Stefan Hajnoczi,
David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
netdev, eric.dumazet, Arseniy Krasnov, Jason Wang, Xuan Zhuo,
Eugenio Pérez, kvm, virtualization
In-Reply-To: <af2sXvVCBT05XF0D@sgarzare-redhat>
On Fri, May 08, 2026 at 11:41:21AM +0200, Stefano Garzarella wrote:
> On Thu, May 07, 2026 at 06:48:47PM -0400, Michael S. Tsirkin wrote:
> > On Thu, May 07, 2026 at 02:59:13PM +0200, Stefano Garzarella wrote:
> > > On Thu, May 07, 2026 at 07:45:10AM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, May 07, 2026 at 11:09:47AM +0200, Stefano Garzarella wrote:
>
> [...]
>
> > > > > For now, we're already doing something:
> > > > > merging the skuffs if they don't have EOM set.
> > > >
> > > >
> > > > Right that's good. You could go further and merge with EOM too
> > > > if you stick the info about message boundaries somewhere else.
> > >
> > > This adds a lot of complexity IMO, but we can try.
> > >
> > > Do you have something in mind?
> >
> > BER is clearly overkill but here's a POC that claude made for me,
> > just to give u an idea. It's clearly has a ton of issues,
> > for example I dislike how GFP_ATOMIC is handled.
>
> Okay, I somewhat understand, but clearly this isn't net material
I doubt we have many other options given reverting the regression was
ruled out.
> so for now
> I think the best thing to do is to merge the fixup I sent (or something
> similar):
> https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
I reviewed that one, problem is it's a spec violation/change that we'll
have to support forever.
> This is a major change that should be merged with more caution.
> Could this have too much of an impact on performance?
>
> Thanks,
> Stefano
It's really a POC, real patch is left as an excersise for the reader :).
The correct approach IMHO is to only start using this
when we wasted a lot of memory on small packets.
For example, if sum(truesize) >= buf size.
then we'll not see a perf impact unless it's already pathological.
> > Yet it seems to work fine in light testing.
> >
> > -->
> >
> >
> > vsock/virtio: use DWARF ULEB128 to record EOM boundaries, enable cross-EOM skb coalescing
> >
> > virtio_transport_recv_enqueue() currently refuses to coalesce an
> > incoming skb with the previous one when the previous skb carries
> > VIRTIO_VSOCK_SEQ_EOM. This forces one skb per seqpacket message.
> > For workloads with many small or zero-byte messages the per-skb
> > overhead (~960 bytes) dominates, causing unbounded memory growth.
> >
> > Decouple message boundary tracking from the skb structure: store
> > boundary offsets in a compact side buffer using DWARF ULEB128
> > encoding with the EOR flag folded into the low bit, then allow
> > the data of multiple complete messages to be coalesced into a single
> > skb.
> >
> > Cross-EOM coalescing fires only when:
> > - both the tail skb and the incoming packet carry EOM (complete msgs)
> > - the incoming packet fits in the tail skb's tailroom
> > - no BPF psock is attached (read_skb expects one msg per skb)
> >
> > On allocation failure the code falls back to separate skbs (existing
> > behaviour). Credit accounting is unchanged; the boundary buffer is
> > capped at PAGE_SIZE.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
> >
> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> > index f91704731057..e36b9ab28372 100644
> > --- a/include/linux/virtio_vsock.h
> > +++ b/include/linux/virtio_vsock.h
> > @@ -12,6 +12,7 @@
> > struct virtio_vsock_skb_cb {
> > bool reply;
> > bool tap_delivered;
> > + bool has_boundary_entries;
> > u32 offset;
> > };
> >
> > @@ -167,6 +168,12 @@ struct virtio_vsock_sock {
> > u32 buf_used;
> > struct sk_buff_head rx_queue;
> > u32 msg_count;
> > +
> > + /* ULEB128-encoded seqpacket message boundary buffer */
> > + u8 *boundary_buf;
> > + u32 boundary_len;
> > + u32 boundary_alloc;
> > + u32 boundary_off;
> > };
> >
> > struct virtio_vsock_pkt_info {
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 416d533f493d..81654f70f72c 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -11,6 +11,7 @@
> > #include <linux/sched/signal.h>
> > #include <linux/ctype.h>
> > #include <linux/list.h>
> > +#include <linux/skmsg.h>
> > #include <linux/virtio_vsock.h>
> > #include <uapi/linux/vsockmon.h>
> >
> > @@ -26,6 +27,91 @@
> > /* Threshold for detecting small packets to copy */
> > #define GOOD_COPY_LEN 128
> >
> > +#define VSOCK_BOUNDARY_BUF_INIT 64
> > +#define VSOCK_BOUNDARY_BUF_MAX PAGE_SIZE
> > +
> > +/* ULEB128 boundary encoding: value = (msg_len << 1) | eor.
> > + * Each byte carries 7 data bits; bit 7 is set on all but the last byte.
> > + * Max 5 bytes for a u32 msg_len (33 bits with eor shift).
> > + */
> > +static int vsock_uleb_encode_boundary(u8 *buf, u32 msg_len, bool eor)
> > +{
> > + u64 val = ((u64)msg_len << 1) | eor;
> > + int n = 0;
> > +
> > + do {
> > + buf[n] = val & 0x7f;
> > + val >>= 7;
> > + if (val)
> > + buf[n] |= 0x80;
> > + n++;
> > + } while (val);
> > +
> > + return n;
> > +}
> > +
> > +static int vsock_uleb_decode_boundary(const u8 *buf, u32 avail,
> > + u32 *msg_len, bool *eor)
> > +{
> > + u64 val = 0;
> > + int shift = 0;
> > + int n = 0;
> > +
> > + do {
> > + if (n >= avail || shift >= 35)
> > + return -EINVAL;
> > + val |= (u64)(buf[n] & 0x7f) << shift;
> > + shift += 7;
> > + } while (buf[n++] & 0x80);
> > +
> > + *eor = val & 1;
> > + *msg_len = val >> 1;
> > + return n;
> > +}
> > +
> > +static void vsock_boundary_buf_compact(struct virtio_vsock_sock *vvs)
> > +{
> > + if (vvs->boundary_off == 0)
> > + return;
> > +
> > + vvs->boundary_len -= vvs->boundary_off;
> > + memmove(vvs->boundary_buf, vvs->boundary_buf + vvs->boundary_off,
> > + vvs->boundary_len);
> > + vvs->boundary_off = 0;
> > +}
> > +
> > +static int vsock_boundary_buf_ensure(struct virtio_vsock_sock *vvs, u32 needed)
> > +{
> > + u32 new_alloc;
> > + u8 *new_buf;
> > +
> > + if (vvs->boundary_alloc >= needed)
> > + return 0;
> > +
> > + /* Reclaim consumed space before growing */
> > + if (vvs->boundary_off) {
> > + needed -= vvs->boundary_off;
> > + vsock_boundary_buf_compact(vvs);
> > + if (vvs->boundary_alloc >= needed)
> > + return 0;
> > + }
> > +
> > + new_alloc = max(needed, vvs->boundary_alloc ? vvs->boundary_alloc * 2
> > + : VSOCK_BOUNDARY_BUF_INIT);
> > + if (new_alloc > VSOCK_BOUNDARY_BUF_MAX)
> > + new_alloc = VSOCK_BOUNDARY_BUF_MAX;
> > + if (new_alloc < needed)
> > + return -ENOMEM;
> > +
> > + new_buf = krealloc(vvs->boundary_buf, new_alloc, GFP_ATOMIC);
> > + if (!new_buf)
> > + return -ENOMEM;
> > +
> > + vvs->boundary_buf = new_buf;
> > + vvs->boundary_alloc = new_alloc;
> > + return 0;
> > +}
> > +
> > 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);
> > @@ -682,41 +768,74 @@ virtio_transport_seqpacket_do_peek(struct vsock_sock *vsk,
> > total = 0;
> > len = msg_data_left(msg);
> >
> > - skb_queue_walk(&vvs->rx_queue, skb) {
> > - struct virtio_vsock_hdr *hdr;
> > + skb = skb_peek(&vvs->rx_queue);
> > + if (skb && VIRTIO_VSOCK_SKB_CB(skb)->has_boundary_entries) {
> > + u32 msg_len, offset;
> > + size_t bytes;
> > + bool eor;
> > + int ret;
> >
> > - if (total < len) {
> > - size_t bytes;
> > + ret = vsock_uleb_decode_boundary(
> > + vvs->boundary_buf + vvs->boundary_off,
> > + vvs->boundary_len - vvs->boundary_off,
> > + &msg_len, &eor);
> > + if (ret < 0)
> > + goto unlock;
> > +
> > + offset = VIRTIO_VSOCK_SKB_CB(skb)->offset;
> > + bytes = min(len, (size_t)msg_len);
> > +
> > + if (bytes) {
> > int err;
> >
> > - bytes = len - total;
> > - if (bytes > skb->len)
> > - bytes = skb->len;
> > -
> > spin_unlock_bh(&vvs->rx_lock);
> > -
> > - /* sk_lock is held by caller so no one else can dequeue.
> > - * Unlock rx_lock since skb_copy_datagram_iter() may sleep.
> > - */
> > - err = skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
> > + err = skb_copy_datagram_iter(skb, offset,
> > &msg->msg_iter, bytes);
> > if (err)
> > return err;
> > -
> > spin_lock_bh(&vvs->rx_lock);
> > }
> >
> > - total += skb->len;
> > - hdr = virtio_vsock_hdr(skb);
> > + total = msg_len;
> > + if (eor)
> > + msg->msg_flags |= MSG_EOR;
> > + } else {
> > + skb_queue_walk(&vvs->rx_queue, skb) {
> > + struct virtio_vsock_hdr *hdr;
> >
> > - if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> > - if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR)
> > - msg->msg_flags |= MSG_EOR;
> > + if (total < len) {
> > + size_t bytes;
> > + int err;
> >
> > - break;
> > + bytes = len - total;
> > + if (bytes > skb->len)
> > + bytes = skb->len;
> > +
> > + spin_unlock_bh(&vvs->rx_lock);
> > +
> > + err = skb_copy_datagram_iter(
> > + skb,
> > + VIRTIO_VSOCK_SKB_CB(skb)->offset,
> > + &msg->msg_iter, bytes);
> > + if (err)
> > + return err;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > + }
> > +
> > + total += skb->len;
> > + hdr = virtio_vsock_hdr(skb);
> > +
> > + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> > + if (le32_to_cpu(hdr->flags) &
> > + VIRTIO_VSOCK_SEQ_EOR)
> > + msg->msg_flags |= MSG_EOR;
> > + break;
> > + }
> > }
> > }
> >
> > +unlock:
> > spin_unlock_bh(&vvs->rx_lock);
> >
> > return total;
> > @@ -740,57 +859,105 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
> > }
> >
> > while (!msg_ready) {
> > - struct virtio_vsock_hdr *hdr;
> > - size_t pkt_len;
> > -
> > - skb = __skb_dequeue(&vvs->rx_queue);
> > + skb = skb_peek(&vvs->rx_queue);
> > if (!skb)
> > break;
> > - hdr = virtio_vsock_hdr(skb);
> > - pkt_len = (size_t)le32_to_cpu(hdr->len);
> >
> > - if (dequeued_len >= 0) {
> > + if (VIRTIO_VSOCK_SKB_CB(skb)->has_boundary_entries) {
> > size_t bytes_to_copy;
> > + u32 msg_len, offset;
> > + bool eor;
> > + int ret;
> >
> > - bytes_to_copy = min(user_buf_len, pkt_len);
> > + ret = vsock_uleb_decode_boundary(
> > + vvs->boundary_buf + vvs->boundary_off,
> > + vvs->boundary_len - vvs->boundary_off,
> > + &msg_len, &eor);
> > + if (ret < 0)
> > + break;
> > + vvs->boundary_off += ret;
> >
> > - if (bytes_to_copy) {
> > + offset = VIRTIO_VSOCK_SKB_CB(skb)->offset;
> > + bytes_to_copy = min(user_buf_len, (size_t)msg_len);
> > +
> > + if (bytes_to_copy && dequeued_len >= 0) {
> > int err;
> >
> > - /* sk_lock is held by caller so no one else can dequeue.
> > - * Unlock rx_lock since skb_copy_datagram_iter() may sleep.
> > - */
> > spin_unlock_bh(&vvs->rx_lock);
> > -
> > - err = skb_copy_datagram_iter(skb, 0,
> > + err = skb_copy_datagram_iter(skb, offset,
> > &msg->msg_iter,
> > bytes_to_copy);
> > - if (err) {
> > - /* Copy of message failed. Rest of
> > - * fragments will be freed without copy.
> > - */
> > - dequeued_len = err;
> > - } else {
> > - user_buf_len -= bytes_to_copy;
> > - }
> > -
> > spin_lock_bh(&vvs->rx_lock);
> > + if (err)
> > + dequeued_len = err;
> > + else
> > + user_buf_len -= bytes_to_copy;
> > }
> >
> > if (dequeued_len >= 0)
> > - dequeued_len += pkt_len;
> > - }
> > + dequeued_len += msg_len;
> >
> > - if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> > + VIRTIO_VSOCK_SKB_CB(skb)->offset += msg_len;
> > msg_ready = true;
> > vvs->msg_count--;
> >
> > - if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR)
> > + if (eor)
> > msg->msg_flags |= MSG_EOR;
> > - }
> >
> > - virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
> > - kfree_skb(skb);
> > + virtio_transport_dec_rx_pkt(vvs, msg_len, msg_len);
> > +
> > + if (VIRTIO_VSOCK_SKB_CB(skb)->offset >= skb->len) {
> > + __skb_unlink(skb, &vvs->rx_queue);
> > + kfree_skb(skb);
> > + }
> > +
> > + if (vvs->boundary_off >= vvs->boundary_len / 2)
> > + vsock_boundary_buf_compact(vvs);
> > + } else {
> > + struct virtio_vsock_hdr *hdr;
> > + size_t pkt_len;
> > +
> > + skb = __skb_dequeue(&vvs->rx_queue);
> > + if (!skb)
> > + break;
> > + hdr = virtio_vsock_hdr(skb);
> > + pkt_len = (size_t)le32_to_cpu(hdr->len);
> > +
> > + if (dequeued_len >= 0) {
> > + size_t bytes_to_copy;
> > +
> > + bytes_to_copy = min(user_buf_len, pkt_len);
> > +
> > + if (bytes_to_copy) {
> > + int err;
> > +
> > + spin_unlock_bh(&vvs->rx_lock);
> > + err = skb_copy_datagram_iter(
> > + skb, 0, &msg->msg_iter,
> > + bytes_to_copy);
> > + if (err)
> > + dequeued_len = err;
> > + else
> > + user_buf_len -= bytes_to_copy;
> > + spin_lock_bh(&vvs->rx_lock);
> > + }
> > +
> > + if (dequeued_len >= 0)
> > + dequeued_len += pkt_len;
> > + }
> > +
> > + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> > + msg_ready = true;
> > + vvs->msg_count--;
> > +
> > + if (le32_to_cpu(hdr->flags) &
> > + VIRTIO_VSOCK_SEQ_EOR)
> > + msg->msg_flags |= MSG_EOR;
> > + }
> > +
> > + virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
> > + kfree_skb(skb);
> > + }
> > }
> >
> > spin_unlock_bh(&vvs->rx_lock);
> > @@ -1132,6 +1299,7 @@ void virtio_transport_destruct(struct vsock_sock *vsk)
> >
> > virtio_transport_cancel_close_work(vsk, true);
> >
> > + kfree(vvs->boundary_buf);
> > kfree(vvs);
> > vsk->trans = NULL;
> > }
> > @@ -1224,6 +1392,11 @@ static void virtio_transport_remove_sock(struct vsock_sock *vsk)
> > * removing it.
> > */
> > __skb_queue_purge(&vvs->rx_queue);
> > + kfree(vvs->boundary_buf);
> > + vvs->boundary_buf = NULL;
> > + vvs->boundary_len = 0;
> > + vvs->boundary_alloc = 0;
> > + vvs->boundary_off = 0;
> > vsock_remove_sock(vsk);
> > }
> >
> > @@ -1395,23 +1568,62 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > !skb_is_nonlinear(skb)) {
> > struct virtio_vsock_hdr *last_hdr;
> > struct sk_buff *last_skb;
> > + bool last_has_eom;
> > + bool has_eom;
> >
> > last_skb = skb_peek_tail(&vvs->rx_queue);
> > last_hdr = virtio_vsock_hdr(last_skb);
> > + last_has_eom = le32_to_cpu(last_hdr->flags) & VIRTIO_VSOCK_SEQ_EOM;
> > + has_eom = le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM;
> >
> > - /* If there is space in the last packet queued, we copy the
> > - * new packet in its buffer. We avoid this if the last packet
> > - * queued has VIRTIO_VSOCK_SEQ_EOM set, because this is
> > - * delimiter of SEQPACKET message, so 'pkt' is the first packet
> > - * of a new message.
> > - */
> > - if (skb->len < skb_tailroom(last_skb) &&
> > - !(le32_to_cpu(last_hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)) {
> > - memcpy(skb_put(last_skb, skb->len), skb->data, skb->len);
> > - free_pkt = true;
> > - last_hdr->flags |= hdr->flags;
> > - le32_add_cpu(&last_hdr->len, len);
> > - goto out;
> > + if (skb->len < skb_tailroom(last_skb)) {
> > + if (!last_has_eom) {
> > + /* Same-message coalescing (existing path) */
> > + memcpy(skb_put(last_skb, skb->len),
> > + skb->data, skb->len);
> > + free_pkt = true;
> > + last_hdr->flags |= hdr->flags;
> > + le32_add_cpu(&last_hdr->len, len);
> > + goto out;
> > + }
> > +
> > + /* Cross-EOM: coalesce complete messages into one skb,
> > + * recording message boundaries in a compact BER buffer.
> > + * Only when incoming packet also has EOM (complete msg).
> > + */
> > + if (has_eom && !sk_psock(sk_vsock(vsk))) {
> > + bool prev_eor, cur_eor;
> > + u8 tmp[12];
> > + int n = 0;
> > +
> > + cur_eor = le32_to_cpu(hdr->flags) &
> > + VIRTIO_VSOCK_SEQ_EOR;
> > +
> > + if (!VIRTIO_VSOCK_SKB_CB(last_skb)->has_boundary_entries) {
> > + u32 prev_len = le32_to_cpu(last_hdr->len);
> > +
> > + prev_eor = le32_to_cpu(last_hdr->flags) &
> > + VIRTIO_VSOCK_SEQ_EOR;
> > + n += vsock_uleb_encode_boundary(
> > + tmp + n, prev_len, prev_eor);
> > + }
> > + n += vsock_uleb_encode_boundary(
> > + tmp + n, len, cur_eor);
> > +
> > + if (!vsock_boundary_buf_ensure(
> > + vvs, vvs->boundary_len + n)) {
> > + memcpy(vvs->boundary_buf +
> > + vvs->boundary_len, tmp, n);
> > + vvs->boundary_len += n;
> > + VIRTIO_VSOCK_SKB_CB(last_skb)->has_boundary_entries = true;
> > + memcpy(skb_put(last_skb, skb->len),
> > + skb->data, skb->len);
> > + free_pkt = true;
> > + last_hdr->flags |= hdr->flags;
> > + le32_add_cpu(&last_hdr->len, len);
> > + goto out;
> > + }
> > + }
> > }
> > }
> >
> >
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Michael S. Tsirkin @ 2026-05-08 9:53 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Eric Dumazet, Stefan Hajnoczi, virtualization,
David S. Miller, Jason Wang, Simon Horman, linux-kernel,
Paolo Abeni, Xuan Zhuo, kvm, Jakub Kicinski, Eugenio Pérez
In-Reply-To: <20260508092330.69690-1-sgarzare@redhat.com>
On Fri, May 08, 2026 at 11:23:30AM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
> queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
> buf_alloc when checking whether a new packet fits. This reduces the
> effective receive buffer below what the user configured via
> SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
> silently dropped and applications that rely on the full buffer size
> to deadlock.
>
> Also, the reduced space is not communicated to the remote peer, so
> its credit calculation accounts more credit than the receiver will
> actually accept, causing data loss (there is no retransmission).
>
> This also causes failures in tools/testing/vsock/vsock_test.c.
> Test 18 sometimes fails, while test 22 always fails in this way:
> 18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
>
> 22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
> Resource temporarily unavailable
>
> Fix this by introducing virtio_transport_rx_buf_size() to calculate the
> size of the RX buffer based on the overhead. Using it in the acceptance
> check, the advertised buf_alloc, and the credit update decision.
> Use buf_alloc * 2 as total budget (payload + overhead), similar to how
> SO_RCVBUF is doubled to reserve space for sk_buff metadata.
> The function returns buf_alloc as long as overhead fits within the
> reservation, then gradually reduces toward 0 as overhead exceeds
> buf_alloc (e.g. under small-packet flooding), informing the peer to
> slow down.
>
> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
unfortunately, this is a bit of a spec violation and there is no guarantee
it helps.
a spec violation because the spec says:
Only payload bytes are counted and header bytes are not
included
and the implication is that a side can not reduce its own buf_alloc.
no guarantee because the other side is not required to process your
packets, so it might not see your buf alloc reduction.
as designed in the current spec, you can only increase your buf alloc,
not decrease it.
what can be done:
- more efficient storage for small packets (poc i posted)
- reduce buf alloc ahead of the time
> ---
> net/vmw_vsock/virtio_transport_common.c | 31 +++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 9b8014516f4f..94a4beb8fd61 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -444,12 +444,32 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> return ret;
> }
>
> +/* vvs->rx_lock held by the caller */
> +static u32 virtio_transport_rx_buf_size(struct virtio_vsock_sock *vvs)
> +{
> + u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
> + /* Use buf_alloc * 2 as total budget (payload + overhead), similar to
> + * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
> + */
> + u64 total_budget = (u64)vvs->buf_alloc * 2;
> +
> + /* Overhead within buf_alloc: full buf_alloc available for payload */
> + if (skb_overhead < vvs->buf_alloc)
> + return vvs->buf_alloc;
> +
> + /* Overhead exceeded buf_alloc: gradually reduce to bound skb queue */
> + if (skb_overhead < total_budget)
> + return total_budget - skb_overhead;
> +
> + return 0;
> +}
> +
> static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> u32 len)
> {
> - u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
> + u32 rx_buf_size = virtio_transport_rx_buf_size(vvs);
>
> - if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
> + if (!rx_buf_size || vvs->buf_used + len > rx_buf_size)
> return false;
>
> vvs->rx_bytes += len;
> @@ -472,7 +492,7 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *
> spin_lock_bh(&vvs->rx_lock);
> vvs->last_fwd_cnt = vvs->fwd_cnt;
> hdr->fwd_cnt = cpu_to_le32(vvs->fwd_cnt);
> - hdr->buf_alloc = cpu_to_le32(vvs->buf_alloc);
> + hdr->buf_alloc = cpu_to_le32(virtio_transport_rx_buf_size(vvs));
> spin_unlock_bh(&vvs->rx_lock);
> }
> EXPORT_SYMBOL_GPL(virtio_transport_inc_tx_pkt);
> @@ -594,6 +614,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> bool low_rx_bytes;
> int err = -EFAULT;
> size_t total = 0;
> + u32 rx_buf_size;
> u32 free_space;
>
> spin_lock_bh(&vvs->rx_lock);
> @@ -639,7 +660,9 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> }
>
> fwd_cnt_delta = vvs->fwd_cnt - vvs->last_fwd_cnt;
> - free_space = vvs->buf_alloc - fwd_cnt_delta;
> + rx_buf_size = virtio_transport_rx_buf_size(vvs);
> + free_space = rx_buf_size > fwd_cnt_delta ?
> + rx_buf_size - fwd_cnt_delta : 0;
> low_rx_bytes = (vvs->rx_bytes <
> sock_rcvlowat(sk_vsock(vsk), 0, INT_MAX));
>
> --
> 2.54.0
^ permalink raw reply
* [PATCH v2] mm/balloon: expose per-node balloon pages in node meminfo
From: Hao Ge @ 2026-05-08 9:47 UTC (permalink / raw)
To: Andrew Morton, David Hildenbrand
Cc: linux-mm, virtualization, linux-kernel, Hao Ge
Commit 835de37603ef ("meminfo: add a per node counter for balloon
drivers") added NR_BALLOON_PAGES and exposed it in /proc/meminfo.
However, the per-node view at /sys/devices/system/node/nodeX/meminfo
was not updated, even though the counter is already tracked per-node.
Add it to node_read_meminfo() so users can see balloon usage per
NUMA node without having to parse the raw vmstat file.
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
v2: Move Balloon field after Unaccepted to match /proc/meminfo ordering
(suggested by David Hildenbrand)
---
drivers/base/node.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index d7647d077b66..85c20aa3c7f2 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -523,6 +523,7 @@ static ssize_t node_read_meminfo(struct device *dev,
#ifdef CONFIG_UNACCEPTED_MEMORY
"Node %d Unaccepted: %8lu kB\n"
#endif
+ "Node %d Balloon: %8lu kB\n"
,
nid, K(node_page_state(pgdat, NR_FILE_DIRTY)),
nid, K(node_page_state(pgdat, NR_WRITEBACK)),
@@ -556,6 +557,8 @@ static ssize_t node_read_meminfo(struct device *dev,
,
nid, K(sum_zone_node_page_state(nid, NR_UNACCEPTED))
#endif
+ ,
+ nid, K(node_page_state(pgdat, NR_BALLOON_PAGES))
);
len += hugetlb_report_node_meminfo(buf, len, nid);
return len;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net] vsock/virtio: fix potential unbounded skb queue
From: Michael S. Tsirkin @ 2026-05-08 9:43 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Eric Dumazet, Arseniy Krasnov, Bobby Eshleman, Stefan Hajnoczi,
David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
netdev, eric.dumazet, Arseniy Krasnov, Jason Wang, Xuan Zhuo,
Eugenio Pérez, kvm, virtualization
In-Reply-To: <af2sXvVCBT05XF0D@sgarzare-redhat>
On Fri, May 08, 2026 at 11:41:21AM +0200, Stefano Garzarella wrote:
> On Thu, May 07, 2026 at 06:48:47PM -0400, Michael S. Tsirkin wrote:
> > On Thu, May 07, 2026 at 02:59:13PM +0200, Stefano Garzarella wrote:
> > > On Thu, May 07, 2026 at 07:45:10AM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, May 07, 2026 at 11:09:47AM +0200, Stefano Garzarella wrote:
>
> [...]
>
> > > > > For now, we're already doing something:
> > > > > merging the skuffs if they don't have EOM set.
> > > >
> > > >
> > > > Right that's good. You could go further and merge with EOM too
> > > > if you stick the info about message boundaries somewhere else.
> > >
> > > This adds a lot of complexity IMO, but we can try.
> > >
> > > Do you have something in mind?
> >
> > BER is clearly overkill but here's a POC that claude made for me,
> > just to give u an idea. It's clearly has a ton of issues,
> > for example I dislike how GFP_ATOMIC is handled.
>
> Okay, I somewhat understand, but clearly this isn't net material, so for now
> I think the best thing to do is to merge the fixup I sent (or something
> similar):
> https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
will respond there.
> This is a major change that should be merged with more caution.
> Could this have too much of an impact on performance?
>
> Thanks,
> Stefano
Upstream is so broken now, I'd not worry about perf even.
> > Yet it seems to work fine in light testing.
> >
> > -->
> >
> >
> > vsock/virtio: use DWARF ULEB128 to record EOM boundaries, enable cross-EOM skb coalescing
> >
> > virtio_transport_recv_enqueue() currently refuses to coalesce an
> > incoming skb with the previous one when the previous skb carries
> > VIRTIO_VSOCK_SEQ_EOM. This forces one skb per seqpacket message.
> > For workloads with many small or zero-byte messages the per-skb
> > overhead (~960 bytes) dominates, causing unbounded memory growth.
> >
> > Decouple message boundary tracking from the skb structure: store
> > boundary offsets in a compact side buffer using DWARF ULEB128
> > encoding with the EOR flag folded into the low bit, then allow
> > the data of multiple complete messages to be coalesced into a single
> > skb.
> >
> > Cross-EOM coalescing fires only when:
> > - both the tail skb and the incoming packet carry EOM (complete msgs)
> > - the incoming packet fits in the tail skb's tailroom
> > - no BPF psock is attached (read_skb expects one msg per skb)
> >
> > On allocation failure the code falls back to separate skbs (existing
> > behaviour). Credit accounting is unchanged; the boundary buffer is
> > capped at PAGE_SIZE.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
> >
> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> > index f91704731057..e36b9ab28372 100644
> > --- a/include/linux/virtio_vsock.h
> > +++ b/include/linux/virtio_vsock.h
> > @@ -12,6 +12,7 @@
> > struct virtio_vsock_skb_cb {
> > bool reply;
> > bool tap_delivered;
> > + bool has_boundary_entries;
> > u32 offset;
> > };
> >
> > @@ -167,6 +168,12 @@ struct virtio_vsock_sock {
> > u32 buf_used;
> > struct sk_buff_head rx_queue;
> > u32 msg_count;
> > +
> > + /* ULEB128-encoded seqpacket message boundary buffer */
> > + u8 *boundary_buf;
> > + u32 boundary_len;
> > + u32 boundary_alloc;
> > + u32 boundary_off;
> > };
> >
> > struct virtio_vsock_pkt_info {
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 416d533f493d..81654f70f72c 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -11,6 +11,7 @@
> > #include <linux/sched/signal.h>
> > #include <linux/ctype.h>
> > #include <linux/list.h>
> > +#include <linux/skmsg.h>
> > #include <linux/virtio_vsock.h>
> > #include <uapi/linux/vsockmon.h>
> >
> > @@ -26,6 +27,91 @@
> > /* Threshold for detecting small packets to copy */
> > #define GOOD_COPY_LEN 128
> >
> > +#define VSOCK_BOUNDARY_BUF_INIT 64
> > +#define VSOCK_BOUNDARY_BUF_MAX PAGE_SIZE
> > +
> > +/* ULEB128 boundary encoding: value = (msg_len << 1) | eor.
> > + * Each byte carries 7 data bits; bit 7 is set on all but the last byte.
> > + * Max 5 bytes for a u32 msg_len (33 bits with eor shift).
> > + */
> > +static int vsock_uleb_encode_boundary(u8 *buf, u32 msg_len, bool eor)
> > +{
> > + u64 val = ((u64)msg_len << 1) | eor;
> > + int n = 0;
> > +
> > + do {
> > + buf[n] = val & 0x7f;
> > + val >>= 7;
> > + if (val)
> > + buf[n] |= 0x80;
> > + n++;
> > + } while (val);
> > +
> > + return n;
> > +}
> > +
> > +static int vsock_uleb_decode_boundary(const u8 *buf, u32 avail,
> > + u32 *msg_len, bool *eor)
> > +{
> > + u64 val = 0;
> > + int shift = 0;
> > + int n = 0;
> > +
> > + do {
> > + if (n >= avail || shift >= 35)
> > + return -EINVAL;
> > + val |= (u64)(buf[n] & 0x7f) << shift;
> > + shift += 7;
> > + } while (buf[n++] & 0x80);
> > +
> > + *eor = val & 1;
> > + *msg_len = val >> 1;
> > + return n;
> > +}
> > +
> > +static void vsock_boundary_buf_compact(struct virtio_vsock_sock *vvs)
> > +{
> > + if (vvs->boundary_off == 0)
> > + return;
> > +
> > + vvs->boundary_len -= vvs->boundary_off;
> > + memmove(vvs->boundary_buf, vvs->boundary_buf + vvs->boundary_off,
> > + vvs->boundary_len);
> > + vvs->boundary_off = 0;
> > +}
> > +
> > +static int vsock_boundary_buf_ensure(struct virtio_vsock_sock *vvs, u32 needed)
> > +{
> > + u32 new_alloc;
> > + u8 *new_buf;
> > +
> > + if (vvs->boundary_alloc >= needed)
> > + return 0;
> > +
> > + /* Reclaim consumed space before growing */
> > + if (vvs->boundary_off) {
> > + needed -= vvs->boundary_off;
> > + vsock_boundary_buf_compact(vvs);
> > + if (vvs->boundary_alloc >= needed)
> > + return 0;
> > + }
> > +
> > + new_alloc = max(needed, vvs->boundary_alloc ? vvs->boundary_alloc * 2
> > + : VSOCK_BOUNDARY_BUF_INIT);
> > + if (new_alloc > VSOCK_BOUNDARY_BUF_MAX)
> > + new_alloc = VSOCK_BOUNDARY_BUF_MAX;
> > + if (new_alloc < needed)
> > + return -ENOMEM;
> > +
> > + new_buf = krealloc(vvs->boundary_buf, new_alloc, GFP_ATOMIC);
> > + if (!new_buf)
> > + return -ENOMEM;
> > +
> > + vvs->boundary_buf = new_buf;
> > + vvs->boundary_alloc = new_alloc;
> > + return 0;
> > +}
> > +
> > 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);
> > @@ -682,41 +768,74 @@ virtio_transport_seqpacket_do_peek(struct vsock_sock *vsk,
> > total = 0;
> > len = msg_data_left(msg);
> >
> > - skb_queue_walk(&vvs->rx_queue, skb) {
> > - struct virtio_vsock_hdr *hdr;
> > + skb = skb_peek(&vvs->rx_queue);
> > + if (skb && VIRTIO_VSOCK_SKB_CB(skb)->has_boundary_entries) {
> > + u32 msg_len, offset;
> > + size_t bytes;
> > + bool eor;
> > + int ret;
> >
> > - if (total < len) {
> > - size_t bytes;
> > + ret = vsock_uleb_decode_boundary(
> > + vvs->boundary_buf + vvs->boundary_off,
> > + vvs->boundary_len - vvs->boundary_off,
> > + &msg_len, &eor);
> > + if (ret < 0)
> > + goto unlock;
> > +
> > + offset = VIRTIO_VSOCK_SKB_CB(skb)->offset;
> > + bytes = min(len, (size_t)msg_len);
> > +
> > + if (bytes) {
> > int err;
> >
> > - bytes = len - total;
> > - if (bytes > skb->len)
> > - bytes = skb->len;
> > -
> > spin_unlock_bh(&vvs->rx_lock);
> > -
> > - /* sk_lock is held by caller so no one else can dequeue.
> > - * Unlock rx_lock since skb_copy_datagram_iter() may sleep.
> > - */
> > - err = skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
> > + err = skb_copy_datagram_iter(skb, offset,
> > &msg->msg_iter, bytes);
> > if (err)
> > return err;
> > -
> > spin_lock_bh(&vvs->rx_lock);
> > }
> >
> > - total += skb->len;
> > - hdr = virtio_vsock_hdr(skb);
> > + total = msg_len;
> > + if (eor)
> > + msg->msg_flags |= MSG_EOR;
> > + } else {
> > + skb_queue_walk(&vvs->rx_queue, skb) {
> > + struct virtio_vsock_hdr *hdr;
> >
> > - if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> > - if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR)
> > - msg->msg_flags |= MSG_EOR;
> > + if (total < len) {
> > + size_t bytes;
> > + int err;
> >
> > - break;
> > + bytes = len - total;
> > + if (bytes > skb->len)
> > + bytes = skb->len;
> > +
> > + spin_unlock_bh(&vvs->rx_lock);
> > +
> > + err = skb_copy_datagram_iter(
> > + skb,
> > + VIRTIO_VSOCK_SKB_CB(skb)->offset,
> > + &msg->msg_iter, bytes);
> > + if (err)
> > + return err;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > + }
> > +
> > + total += skb->len;
> > + hdr = virtio_vsock_hdr(skb);
> > +
> > + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> > + if (le32_to_cpu(hdr->flags) &
> > + VIRTIO_VSOCK_SEQ_EOR)
> > + msg->msg_flags |= MSG_EOR;
> > + break;
> > + }
> > }
> > }
> >
> > +unlock:
> > spin_unlock_bh(&vvs->rx_lock);
> >
> > return total;
> > @@ -740,57 +859,105 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
> > }
> >
> > while (!msg_ready) {
> > - struct virtio_vsock_hdr *hdr;
> > - size_t pkt_len;
> > -
> > - skb = __skb_dequeue(&vvs->rx_queue);
> > + skb = skb_peek(&vvs->rx_queue);
> > if (!skb)
> > break;
> > - hdr = virtio_vsock_hdr(skb);
> > - pkt_len = (size_t)le32_to_cpu(hdr->len);
> >
> > - if (dequeued_len >= 0) {
> > + if (VIRTIO_VSOCK_SKB_CB(skb)->has_boundary_entries) {
> > size_t bytes_to_copy;
> > + u32 msg_len, offset;
> > + bool eor;
> > + int ret;
> >
> > - bytes_to_copy = min(user_buf_len, pkt_len);
> > + ret = vsock_uleb_decode_boundary(
> > + vvs->boundary_buf + vvs->boundary_off,
> > + vvs->boundary_len - vvs->boundary_off,
> > + &msg_len, &eor);
> > + if (ret < 0)
> > + break;
> > + vvs->boundary_off += ret;
> >
> > - if (bytes_to_copy) {
> > + offset = VIRTIO_VSOCK_SKB_CB(skb)->offset;
> > + bytes_to_copy = min(user_buf_len, (size_t)msg_len);
> > +
> > + if (bytes_to_copy && dequeued_len >= 0) {
> > int err;
> >
> > - /* sk_lock is held by caller so no one else can dequeue.
> > - * Unlock rx_lock since skb_copy_datagram_iter() may sleep.
> > - */
> > spin_unlock_bh(&vvs->rx_lock);
> > -
> > - err = skb_copy_datagram_iter(skb, 0,
> > + err = skb_copy_datagram_iter(skb, offset,
> > &msg->msg_iter,
> > bytes_to_copy);
> > - if (err) {
> > - /* Copy of message failed. Rest of
> > - * fragments will be freed without copy.
> > - */
> > - dequeued_len = err;
> > - } else {
> > - user_buf_len -= bytes_to_copy;
> > - }
> > -
> > spin_lock_bh(&vvs->rx_lock);
> > + if (err)
> > + dequeued_len = err;
> > + else
> > + user_buf_len -= bytes_to_copy;
> > }
> >
> > if (dequeued_len >= 0)
> > - dequeued_len += pkt_len;
> > - }
> > + dequeued_len += msg_len;
> >
> > - if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> > + VIRTIO_VSOCK_SKB_CB(skb)->offset += msg_len;
> > msg_ready = true;
> > vvs->msg_count--;
> >
> > - if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR)
> > + if (eor)
> > msg->msg_flags |= MSG_EOR;
> > - }
> >
> > - virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
> > - kfree_skb(skb);
> > + virtio_transport_dec_rx_pkt(vvs, msg_len, msg_len);
> > +
> > + if (VIRTIO_VSOCK_SKB_CB(skb)->offset >= skb->len) {
> > + __skb_unlink(skb, &vvs->rx_queue);
> > + kfree_skb(skb);
> > + }
> > +
> > + if (vvs->boundary_off >= vvs->boundary_len / 2)
> > + vsock_boundary_buf_compact(vvs);
> > + } else {
> > + struct virtio_vsock_hdr *hdr;
> > + size_t pkt_len;
> > +
> > + skb = __skb_dequeue(&vvs->rx_queue);
> > + if (!skb)
> > + break;
> > + hdr = virtio_vsock_hdr(skb);
> > + pkt_len = (size_t)le32_to_cpu(hdr->len);
> > +
> > + if (dequeued_len >= 0) {
> > + size_t bytes_to_copy;
> > +
> > + bytes_to_copy = min(user_buf_len, pkt_len);
> > +
> > + if (bytes_to_copy) {
> > + int err;
> > +
> > + spin_unlock_bh(&vvs->rx_lock);
> > + err = skb_copy_datagram_iter(
> > + skb, 0, &msg->msg_iter,
> > + bytes_to_copy);
> > + if (err)
> > + dequeued_len = err;
> > + else
> > + user_buf_len -= bytes_to_copy;
> > + spin_lock_bh(&vvs->rx_lock);
> > + }
> > +
> > + if (dequeued_len >= 0)
> > + dequeued_len += pkt_len;
> > + }
> > +
> > + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> > + msg_ready = true;
> > + vvs->msg_count--;
> > +
> > + if (le32_to_cpu(hdr->flags) &
> > + VIRTIO_VSOCK_SEQ_EOR)
> > + msg->msg_flags |= MSG_EOR;
> > + }
> > +
> > + virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
> > + kfree_skb(skb);
> > + }
> > }
> >
> > spin_unlock_bh(&vvs->rx_lock);
> > @@ -1132,6 +1299,7 @@ void virtio_transport_destruct(struct vsock_sock *vsk)
> >
> > virtio_transport_cancel_close_work(vsk, true);
> >
> > + kfree(vvs->boundary_buf);
> > kfree(vvs);
> > vsk->trans = NULL;
> > }
> > @@ -1224,6 +1392,11 @@ static void virtio_transport_remove_sock(struct vsock_sock *vsk)
> > * removing it.
> > */
> > __skb_queue_purge(&vvs->rx_queue);
> > + kfree(vvs->boundary_buf);
> > + vvs->boundary_buf = NULL;
> > + vvs->boundary_len = 0;
> > + vvs->boundary_alloc = 0;
> > + vvs->boundary_off = 0;
> > vsock_remove_sock(vsk);
> > }
> >
> > @@ -1395,23 +1568,62 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > !skb_is_nonlinear(skb)) {
> > struct virtio_vsock_hdr *last_hdr;
> > struct sk_buff *last_skb;
> > + bool last_has_eom;
> > + bool has_eom;
> >
> > last_skb = skb_peek_tail(&vvs->rx_queue);
> > last_hdr = virtio_vsock_hdr(last_skb);
> > + last_has_eom = le32_to_cpu(last_hdr->flags) & VIRTIO_VSOCK_SEQ_EOM;
> > + has_eom = le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM;
> >
> > - /* If there is space in the last packet queued, we copy the
> > - * new packet in its buffer. We avoid this if the last packet
> > - * queued has VIRTIO_VSOCK_SEQ_EOM set, because this is
> > - * delimiter of SEQPACKET message, so 'pkt' is the first packet
> > - * of a new message.
> > - */
> > - if (skb->len < skb_tailroom(last_skb) &&
> > - !(le32_to_cpu(last_hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)) {
> > - memcpy(skb_put(last_skb, skb->len), skb->data, skb->len);
> > - free_pkt = true;
> > - last_hdr->flags |= hdr->flags;
> > - le32_add_cpu(&last_hdr->len, len);
> > - goto out;
> > + if (skb->len < skb_tailroom(last_skb)) {
> > + if (!last_has_eom) {
> > + /* Same-message coalescing (existing path) */
> > + memcpy(skb_put(last_skb, skb->len),
> > + skb->data, skb->len);
> > + free_pkt = true;
> > + last_hdr->flags |= hdr->flags;
> > + le32_add_cpu(&last_hdr->len, len);
> > + goto out;
> > + }
> > +
> > + /* Cross-EOM: coalesce complete messages into one skb,
> > + * recording message boundaries in a compact BER buffer.
> > + * Only when incoming packet also has EOM (complete msg).
> > + */
> > + if (has_eom && !sk_psock(sk_vsock(vsk))) {
> > + bool prev_eor, cur_eor;
> > + u8 tmp[12];
> > + int n = 0;
> > +
> > + cur_eor = le32_to_cpu(hdr->flags) &
> > + VIRTIO_VSOCK_SEQ_EOR;
> > +
> > + if (!VIRTIO_VSOCK_SKB_CB(last_skb)->has_boundary_entries) {
> > + u32 prev_len = le32_to_cpu(last_hdr->len);
> > +
> > + prev_eor = le32_to_cpu(last_hdr->flags) &
> > + VIRTIO_VSOCK_SEQ_EOR;
> > + n += vsock_uleb_encode_boundary(
> > + tmp + n, prev_len, prev_eor);
> > + }
> > + n += vsock_uleb_encode_boundary(
> > + tmp + n, len, cur_eor);
> > +
> > + if (!vsock_boundary_buf_ensure(
> > + vvs, vvs->boundary_len + n)) {
> > + memcpy(vvs->boundary_buf +
> > + vvs->boundary_len, tmp, n);
> > + vvs->boundary_len += n;
> > + VIRTIO_VSOCK_SKB_CB(last_skb)->has_boundary_entries = true;
> > + memcpy(skb_put(last_skb, skb->len),
> > + skb->data, skb->len);
> > + free_pkt = true;
> > + last_hdr->flags |= hdr->flags;
> > + le32_add_cpu(&last_hdr->len, len);
> > + goto out;
> > + }
> > + }
> > }
> > }
> >
> >
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix potential unbounded skb queue
From: Stefano Garzarella @ 2026-05-08 9:41 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eric Dumazet, Arseniy Krasnov, Bobby Eshleman, Stefan Hajnoczi,
David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
netdev, eric.dumazet, Arseniy Krasnov, Jason Wang, Xuan Zhuo,
Eugenio Pérez, kvm, virtualization
In-Reply-To: <20260507163710-mutt-send-email-mst@kernel.org>
On Thu, May 07, 2026 at 06:48:47PM -0400, Michael S. Tsirkin wrote:
>On Thu, May 07, 2026 at 02:59:13PM +0200, Stefano Garzarella wrote:
>> On Thu, May 07, 2026 at 07:45:10AM -0400, Michael S. Tsirkin wrote:
>> > On Thu, May 07, 2026 at 11:09:47AM +0200, Stefano Garzarella wrote:
[...]
>> > > For now, we're already doing something:
>> > > merging the skuffs if they don't have EOM set.
>> >
>> >
>> > Right that's good. You could go further and merge with EOM too
>> > if you stick the info about message boundaries somewhere else.
>>
>> This adds a lot of complexity IMO, but we can try.
>>
>> Do you have something in mind?
>
>BER is clearly overkill but here's a POC that claude made for me,
>just to give u an idea. It's clearly has a ton of issues,
>for example I dislike how GFP_ATOMIC is handled.
Okay, I somewhat understand, but clearly this isn't net material, so for
now I think the best thing to do is to merge the fixup I sent (or
something similar):
https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
This is a major change that should be merged with more caution.
Could this have too much of an impact on performance?
Thanks,
Stefano
>Yet it seems to work fine in light testing.
>
>-->
>
>
>vsock/virtio: use DWARF ULEB128 to record EOM boundaries, enable cross-EOM skb coalescing
>
>virtio_transport_recv_enqueue() currently refuses to coalesce an
>incoming skb with the previous one when the previous skb carries
>VIRTIO_VSOCK_SEQ_EOM. This forces one skb per seqpacket message.
>For workloads with many small or zero-byte messages the per-skb
>overhead (~960 bytes) dominates, causing unbounded memory growth.
>
>Decouple message boundary tracking from the skb structure: store
>boundary offsets in a compact side buffer using DWARF ULEB128
>encoding with the EOR flag folded into the low bit, then allow
>the data of multiple complete messages to be coalesced into a single
>skb.
>
>Cross-EOM coalescing fires only when:
>- both the tail skb and the incoming packet carry EOM (complete msgs)
>- the incoming packet fits in the tail skb's tailroom
>- no BPF psock is attached (read_skb expects one msg per skb)
>
>On allocation failure the code falls back to separate skbs (existing
>behaviour). Credit accounting is unchanged; the boundary buffer is
>capped at PAGE_SIZE.
>
>Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index f91704731057..e36b9ab28372 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -12,6 +12,7 @@
> struct virtio_vsock_skb_cb {
> bool reply;
> bool tap_delivered;
>+ bool has_boundary_entries;
> u32 offset;
> };
>
>@@ -167,6 +168,12 @@ struct virtio_vsock_sock {
> u32 buf_used;
> struct sk_buff_head rx_queue;
> u32 msg_count;
>+
>+ /* ULEB128-encoded seqpacket message boundary buffer */
>+ u8 *boundary_buf;
>+ u32 boundary_len;
>+ u32 boundary_alloc;
>+ u32 boundary_off;
> };
>
> struct virtio_vsock_pkt_info {
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 416d533f493d..81654f70f72c 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -11,6 +11,7 @@
> #include <linux/sched/signal.h>
> #include <linux/ctype.h>
> #include <linux/list.h>
>+#include <linux/skmsg.h>
> #include <linux/virtio_vsock.h>
> #include <uapi/linux/vsockmon.h>
>
>@@ -26,6 +27,91 @@
> /* Threshold for detecting small packets to copy */
> #define GOOD_COPY_LEN 128
>
>+#define VSOCK_BOUNDARY_BUF_INIT 64
>+#define VSOCK_BOUNDARY_BUF_MAX PAGE_SIZE
>+
>+/* ULEB128 boundary encoding: value = (msg_len << 1) | eor.
>+ * Each byte carries 7 data bits; bit 7 is set on all but the last byte.
>+ * Max 5 bytes for a u32 msg_len (33 bits with eor shift).
>+ */
>+static int vsock_uleb_encode_boundary(u8 *buf, u32 msg_len, bool eor)
>+{
>+ u64 val = ((u64)msg_len << 1) | eor;
>+ int n = 0;
>+
>+ do {
>+ buf[n] = val & 0x7f;
>+ val >>= 7;
>+ if (val)
>+ buf[n] |= 0x80;
>+ n++;
>+ } while (val);
>+
>+ return n;
>+}
>+
>+static int vsock_uleb_decode_boundary(const u8 *buf, u32 avail,
>+ u32 *msg_len, bool *eor)
>+{
>+ u64 val = 0;
>+ int shift = 0;
>+ int n = 0;
>+
>+ do {
>+ if (n >= avail || shift >= 35)
>+ return -EINVAL;
>+ val |= (u64)(buf[n] & 0x7f) << shift;
>+ shift += 7;
>+ } while (buf[n++] & 0x80);
>+
>+ *eor = val & 1;
>+ *msg_len = val >> 1;
>+ return n;
>+}
>+
>+static void vsock_boundary_buf_compact(struct virtio_vsock_sock *vvs)
>+{
>+ if (vvs->boundary_off == 0)
>+ return;
>+
>+ vvs->boundary_len -= vvs->boundary_off;
>+ memmove(vvs->boundary_buf, vvs->boundary_buf + vvs->boundary_off,
>+ vvs->boundary_len);
>+ vvs->boundary_off = 0;
>+}
>+
>+static int vsock_boundary_buf_ensure(struct virtio_vsock_sock *vvs, u32 needed)
>+{
>+ u32 new_alloc;
>+ u8 *new_buf;
>+
>+ if (vvs->boundary_alloc >= needed)
>+ return 0;
>+
>+ /* Reclaim consumed space before growing */
>+ if (vvs->boundary_off) {
>+ needed -= vvs->boundary_off;
>+ vsock_boundary_buf_compact(vvs);
>+ if (vvs->boundary_alloc >= needed)
>+ return 0;
>+ }
>+
>+ new_alloc = max(needed, vvs->boundary_alloc ? vvs->boundary_alloc * 2
>+ : VSOCK_BOUNDARY_BUF_INIT);
>+ if (new_alloc > VSOCK_BOUNDARY_BUF_MAX)
>+ new_alloc = VSOCK_BOUNDARY_BUF_MAX;
>+ if (new_alloc < needed)
>+ return -ENOMEM;
>+
>+ new_buf = krealloc(vvs->boundary_buf, new_alloc, GFP_ATOMIC);
>+ if (!new_buf)
>+ return -ENOMEM;
>+
>+ vvs->boundary_buf = new_buf;
>+ vvs->boundary_alloc = new_alloc;
>+ return 0;
>+}
>+
> 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);
>@@ -682,41 +768,74 @@ virtio_transport_seqpacket_do_peek(struct vsock_sock *vsk,
> total = 0;
> len = msg_data_left(msg);
>
>- skb_queue_walk(&vvs->rx_queue, skb) {
>- struct virtio_vsock_hdr *hdr;
>+ skb = skb_peek(&vvs->rx_queue);
>+ if (skb && VIRTIO_VSOCK_SKB_CB(skb)->has_boundary_entries) {
>+ u32 msg_len, offset;
>+ size_t bytes;
>+ bool eor;
>+ int ret;
>
>- if (total < len) {
>- size_t bytes;
>+ ret = vsock_uleb_decode_boundary(
>+ vvs->boundary_buf + vvs->boundary_off,
>+ vvs->boundary_len - vvs->boundary_off,
>+ &msg_len, &eor);
>+ if (ret < 0)
>+ goto unlock;
>+
>+ offset = VIRTIO_VSOCK_SKB_CB(skb)->offset;
>+ bytes = min(len, (size_t)msg_len);
>+
>+ if (bytes) {
> int err;
>
>- bytes = len - total;
>- if (bytes > skb->len)
>- bytes = skb->len;
>-
> spin_unlock_bh(&vvs->rx_lock);
>-
>- /* sk_lock is held by caller so no one else can dequeue.
>- * Unlock rx_lock since skb_copy_datagram_iter() may sleep.
>- */
>- err = skb_copy_datagram_iter(skb, VIRTIO_VSOCK_SKB_CB(skb)->offset,
>+ err = skb_copy_datagram_iter(skb, offset,
> &msg->msg_iter, bytes);
> if (err)
> return err;
>-
> spin_lock_bh(&vvs->rx_lock);
> }
>
>- total += skb->len;
>- hdr = virtio_vsock_hdr(skb);
>+ total = msg_len;
>+ if (eor)
>+ msg->msg_flags |= MSG_EOR;
>+ } else {
>+ skb_queue_walk(&vvs->rx_queue, skb) {
>+ struct virtio_vsock_hdr *hdr;
>
>- if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
>- if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR)
>- msg->msg_flags |= MSG_EOR;
>+ if (total < len) {
>+ size_t bytes;
>+ int err;
>
>- break;
>+ bytes = len - total;
>+ if (bytes > skb->len)
>+ bytes = skb->len;
>+
>+ spin_unlock_bh(&vvs->rx_lock);
>+
>+ err = skb_copy_datagram_iter(
>+ skb,
>+ VIRTIO_VSOCK_SKB_CB(skb)->offset,
>+ &msg->msg_iter, bytes);
>+ if (err)
>+ return err;
>+
>+ spin_lock_bh(&vvs->rx_lock);
>+ }
>+
>+ total += skb->len;
>+ hdr = virtio_vsock_hdr(skb);
>+
>+ if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
>+ if (le32_to_cpu(hdr->flags) &
>+ VIRTIO_VSOCK_SEQ_EOR)
>+ msg->msg_flags |= MSG_EOR;
>+ break;
>+ }
> }
> }
>
>+unlock:
> spin_unlock_bh(&vvs->rx_lock);
>
> return total;
>@@ -740,57 +859,105 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
> }
>
> while (!msg_ready) {
>- struct virtio_vsock_hdr *hdr;
>- size_t pkt_len;
>-
>- skb = __skb_dequeue(&vvs->rx_queue);
>+ skb = skb_peek(&vvs->rx_queue);
> if (!skb)
> break;
>- hdr = virtio_vsock_hdr(skb);
>- pkt_len = (size_t)le32_to_cpu(hdr->len);
>
>- if (dequeued_len >= 0) {
>+ if (VIRTIO_VSOCK_SKB_CB(skb)->has_boundary_entries) {
> size_t bytes_to_copy;
>+ u32 msg_len, offset;
>+ bool eor;
>+ int ret;
>
>- bytes_to_copy = min(user_buf_len, pkt_len);
>+ ret = vsock_uleb_decode_boundary(
>+ vvs->boundary_buf + vvs->boundary_off,
>+ vvs->boundary_len - vvs->boundary_off,
>+ &msg_len, &eor);
>+ if (ret < 0)
>+ break;
>+ vvs->boundary_off += ret;
>
>- if (bytes_to_copy) {
>+ offset = VIRTIO_VSOCK_SKB_CB(skb)->offset;
>+ bytes_to_copy = min(user_buf_len, (size_t)msg_len);
>+
>+ if (bytes_to_copy && dequeued_len >= 0) {
> int err;
>
>- /* sk_lock is held by caller so no one else can dequeue.
>- * Unlock rx_lock since skb_copy_datagram_iter() may sleep.
>- */
> spin_unlock_bh(&vvs->rx_lock);
>-
>- err = skb_copy_datagram_iter(skb, 0,
>+ err = skb_copy_datagram_iter(skb, offset,
> &msg->msg_iter,
> bytes_to_copy);
>- if (err) {
>- /* Copy of message failed. Rest of
>- * fragments will be freed without copy.
>- */
>- dequeued_len = err;
>- } else {
>- user_buf_len -= bytes_to_copy;
>- }
>-
> spin_lock_bh(&vvs->rx_lock);
>+ if (err)
>+ dequeued_len = err;
>+ else
>+ user_buf_len -= bytes_to_copy;
> }
>
> if (dequeued_len >= 0)
>- dequeued_len += pkt_len;
>- }
>+ dequeued_len += msg_len;
>
>- if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
>+ VIRTIO_VSOCK_SKB_CB(skb)->offset += msg_len;
> msg_ready = true;
> vvs->msg_count--;
>
>- if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR)
>+ if (eor)
> msg->msg_flags |= MSG_EOR;
>- }
>
>- virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
>- kfree_skb(skb);
>+ virtio_transport_dec_rx_pkt(vvs, msg_len, msg_len);
>+
>+ if (VIRTIO_VSOCK_SKB_CB(skb)->offset >= skb->len) {
>+ __skb_unlink(skb, &vvs->rx_queue);
>+ kfree_skb(skb);
>+ }
>+
>+ if (vvs->boundary_off >= vvs->boundary_len / 2)
>+ vsock_boundary_buf_compact(vvs);
>+ } else {
>+ struct virtio_vsock_hdr *hdr;
>+ size_t pkt_len;
>+
>+ skb = __skb_dequeue(&vvs->rx_queue);
>+ if (!skb)
>+ break;
>+ hdr = virtio_vsock_hdr(skb);
>+ pkt_len = (size_t)le32_to_cpu(hdr->len);
>+
>+ if (dequeued_len >= 0) {
>+ size_t bytes_to_copy;
>+
>+ bytes_to_copy = min(user_buf_len, pkt_len);
>+
>+ if (bytes_to_copy) {
>+ int err;
>+
>+ spin_unlock_bh(&vvs->rx_lock);
>+ err = skb_copy_datagram_iter(
>+ skb, 0, &msg->msg_iter,
>+ bytes_to_copy);
>+ if (err)
>+ dequeued_len = err;
>+ else
>+ user_buf_len -= bytes_to_copy;
>+ spin_lock_bh(&vvs->rx_lock);
>+ }
>+
>+ if (dequeued_len >= 0)
>+ dequeued_len += pkt_len;
>+ }
>+
>+ if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
>+ msg_ready = true;
>+ vvs->msg_count--;
>+
>+ if (le32_to_cpu(hdr->flags) &
>+ VIRTIO_VSOCK_SEQ_EOR)
>+ msg->msg_flags |= MSG_EOR;
>+ }
>+
>+ virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
>+ kfree_skb(skb);
>+ }
> }
>
> spin_unlock_bh(&vvs->rx_lock);
>@@ -1132,6 +1299,7 @@ void virtio_transport_destruct(struct vsock_sock *vsk)
>
> virtio_transport_cancel_close_work(vsk, true);
>
>+ kfree(vvs->boundary_buf);
> kfree(vvs);
> vsk->trans = NULL;
> }
>@@ -1224,6 +1392,11 @@ static void virtio_transport_remove_sock(struct vsock_sock *vsk)
> * removing it.
> */
> __skb_queue_purge(&vvs->rx_queue);
>+ kfree(vvs->boundary_buf);
>+ vvs->boundary_buf = NULL;
>+ vvs->boundary_len = 0;
>+ vvs->boundary_alloc = 0;
>+ vvs->boundary_off = 0;
> vsock_remove_sock(vsk);
> }
>
>@@ -1395,23 +1568,62 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> !skb_is_nonlinear(skb)) {
> struct virtio_vsock_hdr *last_hdr;
> struct sk_buff *last_skb;
>+ bool last_has_eom;
>+ bool has_eom;
>
> last_skb = skb_peek_tail(&vvs->rx_queue);
> last_hdr = virtio_vsock_hdr(last_skb);
>+ last_has_eom = le32_to_cpu(last_hdr->flags) & VIRTIO_VSOCK_SEQ_EOM;
>+ has_eom = le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM;
>
>- /* If there is space in the last packet queued, we copy the
>- * new packet in its buffer. We avoid this if the last packet
>- * queued has VIRTIO_VSOCK_SEQ_EOM set, because this is
>- * delimiter of SEQPACKET message, so 'pkt' is the first packet
>- * of a new message.
>- */
>- if (skb->len < skb_tailroom(last_skb) &&
>- !(le32_to_cpu(last_hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)) {
>- memcpy(skb_put(last_skb, skb->len), skb->data, skb->len);
>- free_pkt = true;
>- last_hdr->flags |= hdr->flags;
>- le32_add_cpu(&last_hdr->len, len);
>- goto out;
>+ if (skb->len < skb_tailroom(last_skb)) {
>+ if (!last_has_eom) {
>+ /* Same-message coalescing (existing path) */
>+ memcpy(skb_put(last_skb, skb->len),
>+ skb->data, skb->len);
>+ free_pkt = true;
>+ last_hdr->flags |= hdr->flags;
>+ le32_add_cpu(&last_hdr->len, len);
>+ goto out;
>+ }
>+
>+ /* Cross-EOM: coalesce complete messages into one skb,
>+ * recording message boundaries in a compact BER buffer.
>+ * Only when incoming packet also has EOM (complete msg).
>+ */
>+ if (has_eom && !sk_psock(sk_vsock(vsk))) {
>+ bool prev_eor, cur_eor;
>+ u8 tmp[12];
>+ int n = 0;
>+
>+ cur_eor = le32_to_cpu(hdr->flags) &
>+ VIRTIO_VSOCK_SEQ_EOR;
>+
>+ if (!VIRTIO_VSOCK_SKB_CB(last_skb)->has_boundary_entries) {
>+ u32 prev_len = le32_to_cpu(last_hdr->len);
>+
>+ prev_eor = le32_to_cpu(last_hdr->flags) &
>+ VIRTIO_VSOCK_SEQ_EOR;
>+ n += vsock_uleb_encode_boundary(
>+ tmp + n, prev_len, prev_eor);
>+ }
>+ n += vsock_uleb_encode_boundary(
>+ tmp + n, len, cur_eor);
>+
>+ if (!vsock_boundary_buf_ensure(
>+ vvs, vvs->boundary_len + n)) {
>+ memcpy(vvs->boundary_buf +
>+ vvs->boundary_len, tmp, n);
>+ vvs->boundary_len += n;
>+ VIRTIO_VSOCK_SKB_CB(last_skb)->has_boundary_entries = true;
>+ memcpy(skb_put(last_skb, skb->len),
>+ skb->data, skb->len);
>+ free_pkt = true;
>+ last_hdr->flags |= hdr->flags;
>+ le32_add_cpu(&last_hdr->len, len);
>+ goto out;
>+ }
>+ }
> }
> }
>
>
^ permalink raw reply
* Re: [PATCH net] vsock/virtio: fix potential unbounded skb queue
From: Stefano Garzarella @ 2026-05-08 9:26 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Eric Dumazet, Michael S. Tsirkin, Arseniy Krasnov, Bobby Eshleman,
Stefan Hajnoczi, David S . Miller, Paolo Abeni, Simon Horman,
netdev, eric.dumazet, Arseniy Krasnov, Jason Wang, Xuan Zhuo,
Eugenio Pérez, kvm, virtualization
In-Reply-To: <20260507101805.6c3335d0@kernel.org>
On Thu, May 07, 2026 at 10:18:05AM -0700, Jakub Kicinski wrote:
>On Thu, 7 May 2026 09:32:24 -0700 Eric Dumazet wrote:
>> On Thu, May 7, 2026 at 9:05 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> > On Thu, May 07, 2026 at 07:33:40AM -0700, Jakub Kicinski wrote:
>> > >We can revert if you think that the risk of regression is high..
>> > >Please LMK soon, we can do it before patch reaches Linus.
>> >
>> > Some tests in tools/testing/vsock/vsock_test.c are failing with this
>> > patch applied.
>> >
>> > Test 18 are failing sometime in this way (I guess because we are
>> > dropping packets):
>> >
>> > 18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
>> >
>> > Test 22 is failing 100% in this way:
>> >
>> > 22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
>> > Resource temporarily unavailable
>> >
>> >
>> > With my followup patch adding also advertisement to the other peer
>> > (still draft locally, waiting for Michael proposal) I saw 22 failing,
>> > because tests expects that can use the entire buf_alloc, but now we are
>> > reducing it. So IMO we should do like in `__sock_set_rcvbuf()` and
>> > double the buffer size, or at least digest an overhead equal to the
>> > buffer size set by the user via SO_VM_SOCKETS_BUFFER_SIZE (yeah,
>> > AF_VSOCK has it owns sockopt since the beginning :-().
>> >
>> > With that approach tests are passing, but I'd like to stress a bit more
>> > that patch. I'll send it tomorrow as fixup of this patch, or if you
>> > prefer to revert, I'll send as standalone.
>>
>> A plain revert is a big issue, now users now how to crash hypervisors.
>>
>> This vulnerability allows a compromised guest (controlling
>> virtio_vsock_hdr fields)
>> to continuously flood the host's vsock receive queue without
>> triggering any memory
>> accounting limits or reader wakeups, resulting in unbounded host
>> kernel memory consumption (Host DoS via OOM).
>>
>> A vulnerability where a KVM guest can crash or deadlock its host is
>> classified as a KVM DoS.
>>
>> Am I missing something?
>
>Alright, let's leave it.
>
I posted a potential fixup:
https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
Thanks,
Stefano
^ permalink raw reply
* [PATCH net] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-08 9:23 UTC (permalink / raw)
To: netdev
Cc: Eric Dumazet, Michael S. Tsirkin, Stefan Hajnoczi, virtualization,
David S. Miller, Jason Wang, Simon Horman, linux-kernel,
Paolo Abeni, Xuan Zhuo, kvm, Jakub Kicinski, Stefano Garzarella,
Eugenio Pérez
From: Stefano Garzarella <sgarzare@redhat.com>
After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
buf_alloc when checking whether a new packet fits. This reduces the
effective receive buffer below what the user configured via
SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
silently dropped and applications that rely on the full buffer size
to deadlock.
Also, the reduced space is not communicated to the remote peer, so
its credit calculation accounts more credit than the receiver will
actually accept, causing data loss (there is no retransmission).
This also causes failures in tools/testing/vsock/vsock_test.c.
Test 18 sometimes fails, while test 22 always fails in this way:
18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
Resource temporarily unavailable
Fix this by introducing virtio_transport_rx_buf_size() to calculate the
size of the RX buffer based on the overhead. Using it in the acceptance
check, the advertised buf_alloc, and the credit update decision.
Use buf_alloc * 2 as total budget (payload + overhead), similar to how
SO_RCVBUF is doubled to reserve space for sk_buff metadata.
The function returns buf_alloc as long as overhead fits within the
reservation, then gradually reduces toward 0 as overhead exceeds
buf_alloc (e.g. under small-packet flooding), informing the peer to
slow down.
Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport_common.c | 31 +++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 9b8014516f4f..94a4beb8fd61 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -444,12 +444,32 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
return ret;
}
+/* vvs->rx_lock held by the caller */
+static u32 virtio_transport_rx_buf_size(struct virtio_vsock_sock *vvs)
+{
+ u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
+ /* Use buf_alloc * 2 as total budget (payload + overhead), similar to
+ * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
+ */
+ u64 total_budget = (u64)vvs->buf_alloc * 2;
+
+ /* Overhead within buf_alloc: full buf_alloc available for payload */
+ if (skb_overhead < vvs->buf_alloc)
+ return vvs->buf_alloc;
+
+ /* Overhead exceeded buf_alloc: gradually reduce to bound skb queue */
+ if (skb_overhead < total_budget)
+ return total_budget - skb_overhead;
+
+ return 0;
+}
+
static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
u32 len)
{
- u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
+ u32 rx_buf_size = virtio_transport_rx_buf_size(vvs);
- if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
+ if (!rx_buf_size || vvs->buf_used + len > rx_buf_size)
return false;
vvs->rx_bytes += len;
@@ -472,7 +492,7 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *
spin_lock_bh(&vvs->rx_lock);
vvs->last_fwd_cnt = vvs->fwd_cnt;
hdr->fwd_cnt = cpu_to_le32(vvs->fwd_cnt);
- hdr->buf_alloc = cpu_to_le32(vvs->buf_alloc);
+ hdr->buf_alloc = cpu_to_le32(virtio_transport_rx_buf_size(vvs));
spin_unlock_bh(&vvs->rx_lock);
}
EXPORT_SYMBOL_GPL(virtio_transport_inc_tx_pkt);
@@ -594,6 +614,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
bool low_rx_bytes;
int err = -EFAULT;
size_t total = 0;
+ u32 rx_buf_size;
u32 free_space;
spin_lock_bh(&vvs->rx_lock);
@@ -639,7 +660,9 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
}
fwd_cnt_delta = vvs->fwd_cnt - vvs->last_fwd_cnt;
- free_space = vvs->buf_alloc - fwd_cnt_delta;
+ rx_buf_size = virtio_transport_rx_buf_size(vvs);
+ free_space = rx_buf_size > fwd_cnt_delta ?
+ rx_buf_size - fwd_cnt_delta : 0;
low_rx_bytes = (vvs->rx_bytes <
sock_rcvlowat(sk_vsock(vsk), 0, INT_MAX));
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] mm/balloon: expose per-node balloon pages in node meminfo
From: David Hildenbrand (Arm) @ 2026-05-08 8:23 UTC (permalink / raw)
To: Hao Ge, Andrew Morton; +Cc: linux-mm, virtualization, linux-kernel
In-Reply-To: <20260508015316.25722-1-hao.ge@linux.dev>
On 5/8/26 03:53, Hao Ge wrote:
> Commit 835de37603ef ("meminfo: add a per node counter for balloon
> drivers") added NR_BALLOON_PAGES and exposed it in /proc/meminfo.
> However, the per-node view at /sys/devices/system/node/nodeX/meminfo
> was not updated, even though the counter is already tracked per-node.
>
> Add it to node_read_meminfo() so users can see balloon usage per
> NUMA node without having to parse the raw vmstat file.
Using ballooning with vNUMA is rather rare. But sure, why not.
>
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> ---
> drivers/base/node.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index d7647d077b66..53f4e51d6d82 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -513,6 +513,7 @@ static ssize_t node_read_meminfo(struct device *dev,
> "Node %d Slab: %8lu kB\n"
> "Node %d SReclaimable: %8lu kB\n"
> "Node %d SUnreclaim: %8lu kB\n"
> + "Node %d Balloon: %8lu kB\n"
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> "Node %d AnonHugePages: %8lu kB\n"
> "Node %d ShmemHugePages: %8lu kB\n"
> @@ -543,7 +544,8 @@ static ssize_t node_read_meminfo(struct device *dev,
> node_page_state(pgdat, NR_KERNEL_MISC_RECLAIMABLE)),
> nid, K(sreclaimable + sunreclaimable),
> nid, K(sreclaimable),
> - nid, K(sunreclaimable)
> + nid, K(sunreclaimable),
> + nid, K(node_page_state(pgdat, NR_BALLOON_PAGES))
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> ,
> nid, K(node_page_state(pgdat, NR_ANON_THPS)),
Shouldn't it be placed under "Unaccepted:", just like for /proc/meminfo?
--
Cheers,
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox