From: Stefano Garzarella <sgarzare@redhat.com>

In `struct virtio_vsock_sock`, we maintain two counters:
- `rx_bytes`: used internally to track how many bytes have been read.
  This supports mechanisms like .stream_has_data() and sock_rcvlowat().
- `fwd_cnt`: used for the credit mechanism to inform available receive
  buffer space to the remote peer.

These counters are updated via virtio_transport_inc_rx_pkt() and
virtio_transport_dec_rx_pkt().

Since the beginning with commit 06a8fc78367d ("VSOCK: Introduce
virtio_vsock_common.ko"), we call virtio_transport_dec_rx_pkt() in
virtio_transport_stream_do_dequeue() only when we consume the entire
packet, so partial reads, do not update `rx_bytes` and `fwd_cnt`.

This is fine for `fwd_cnt`, because we still have space used for the
entire packet, and we don't want to update the credit for the other
peer until we free the space of the entire packet. However, this
causes `rx_bytes` to be stale on partial reads.

Previously, this didn’t cause issues because `rx_bytes` was used only by
.stream_has_data(), and any unread portion of a packet implied data was
still available. However, since commit 93b808876682
("virtio/vsock: fix logic which reduces credit update messages"), we now
rely on `rx_bytes` to determine if a credit update should be sent when
the data in the RX queue drops below SO_RCVLOWAT value.

This patch fixes the accounting by updating `rx_bytes` with the number
of bytes actually read, even on partial reads, while leaving `fwd_cnt`
untouched until the packet is fully consumed. Also introduce a new
`buf_used` counter to check that the remote peer is honoring the given
credit; this was previously done via `rx_bytes`.

Fixes: 93b808876682 ("virtio/vsock: fix logic which reduces credit update messages")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/linux/virtio_vsock.h            |  1 +
 net/vmw_vsock/virtio_transport_common.c | 26 +++++++++++++++----------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 0387d64e2c66..36fb3edfa403 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -140,6 +140,7 @@ struct virtio_vsock_sock {
 	u32 last_fwd_cnt;
 	u32 rx_bytes;
 	u32 buf_alloc;
+	u32 buf_used;
 	struct sk_buff_head rx_queue;
 	u32 msg_count;
 };
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 7f7de6d88096..2c9b1011cdcc 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -441,18 +441,20 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
 					u32 len)
 {
-	if (vvs->rx_bytes + len > vvs->buf_alloc)
+	if (vvs->buf_used + len > vvs->buf_alloc)
 		return false;
 
 	vvs->rx_bytes += len;
+	vvs->buf_used += len;
 	return true;
 }
 
 static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs,
-					u32 len)
+					u32 bytes_read, u32 bytes_dequeued)
 {
-	vvs->rx_bytes -= len;
-	vvs->fwd_cnt += len;
+	vvs->rx_bytes -= bytes_read;
+	vvs->buf_used -= bytes_dequeued;
+	vvs->fwd_cnt += bytes_dequeued;
 }
 
 void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb)
@@ -581,11 +583,11 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
 				   size_t len)
 {
 	struct virtio_vsock_sock *vvs = vsk->trans;
-	size_t bytes, total = 0;
 	struct sk_buff *skb;
 	u32 fwd_cnt_delta;
 	bool low_rx_bytes;
 	int err = -EFAULT;
+	size_t total = 0;
 	u32 free_space;
 
 	spin_lock_bh(&vvs->rx_lock);
@@ -597,6 +599,8 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
 	}
 
 	while (total < len && !skb_queue_empty(&vvs->rx_queue)) {
+		size_t bytes, dequeued = 0;
+
 		skb = skb_peek(&vvs->rx_queue);
 
 		bytes = min_t(size_t, len - total,
@@ -620,12 +624,12 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
 		VIRTIO_VSOCK_SKB_CB(skb)->offset += bytes;
 
 		if (skb->len == VIRTIO_VSOCK_SKB_CB(skb)->offset) {
-			u32 pkt_len = le32_to_cpu(virtio_vsock_hdr(skb)->len);
-
-			virtio_transport_dec_rx_pkt(vvs, pkt_len);
+			dequeued = le32_to_cpu(virtio_vsock_hdr(skb)->len);
 			__skb_unlink(skb, &vvs->rx_queue);
 			consume_skb(skb);
 		}
+
+		virtio_transport_dec_rx_pkt(vvs, bytes, dequeued);
 	}
 
 	fwd_cnt_delta = vvs->fwd_cnt - vvs->last_fwd_cnt;
@@ -781,7 +785,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
 				msg->msg_flags |= MSG_EOR;
 		}
 
-		virtio_transport_dec_rx_pkt(vvs, pkt_len);
+		virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
 		kfree_skb(skb);
 	}
 
@@ -1735,6 +1739,7 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto
 	struct sock *sk = sk_vsock(vsk);
 	struct virtio_vsock_hdr *hdr;
 	struct sk_buff *skb;
+	u32 pkt_len;
 	int off = 0;
 	int err;
 
@@ -1752,7 +1757,8 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto
 	if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
 		vvs->msg_count--;
 
-	virtio_transport_dec_rx_pkt(vvs, le32_to_cpu(hdr->len));
+	pkt_len = le32_to_cpu(hdr->len);
+	virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
 	spin_unlock_bh(&vvs->rx_lock);
 
 	virtio_transport_send_credit_update(vsk);
-- 
2.49.0

No comments since last month.

The patch [1], which adds SIOCINQ ioctl support for vsock, depends on this
patch. Could I get more eyes on this one?

[1]: https://lore.kernel.org/lkml/bbn4lvdwh42m2zvi3rdyws66y5ulew32rchtz3kxirqlllkr63@7toa4tcepax3/#t

Thanks,
Xuewei

On Thu, Jun 12, 2025 at 01:32:01PM +0800, Xuewei Niu wrote:
> No comments since last month.
> 
> The patch [1], which adds SIOCINQ ioctl support for vsock, depends on this
> patch. Could I get more eyes on this one?
> 
> [1]: https://lore.kernel.org/lkml/bbn4lvdwh42m2zvi3rdyws66y5ulew32rchtz3kxirqlllkr63@7toa4tcepax3/#t
> 
> Thanks,
> Xuewei

it's been in net for two weeks now, no?

> On Thu, Jun 12, 2025 at 01:32:01PM +0800, Xuewei Niu wrote:
> > No comments since last month.
> > 
> > The patch [1], which adds SIOCINQ ioctl support for vsock, depends on this
> > patch. Could I get more eyes on this one?
> > 
> > [1]: https://lore.kernel.org/lkml/bbn4lvdwh42m2zvi3rdyws66y5ulew32rchtz3kxirqlllkr63@7toa4tcepax3/#t
> > 
> > Thanks,
> > Xuewei
> 
> it's been in net for two weeks now, no?

Umm sorry, I didn't check the date carefully, because there are several
ongoing patches. Next time I'll check it carefully. Sorry again.

It looks like no one is paying attention to this patch. I am requesting
someone interested in vsock to review this. I'd appreciate that!

Thanks,
Xuewei

On Thu, 12 Jun 2025 at 08:50, Xuewei Niu <niuxuewei97@gmail.com> wrote:
>
> > On Thu, Jun 12, 2025 at 01:32:01PM +0800, Xuewei Niu wrote:
> > > No comments since last month.
> > >
> > > The patch [1], which adds SIOCINQ ioctl support for vsock, depends on this
> > > patch. Could I get more eyes on this one?
> > >
> > > [1]: https://lore.kernel.org/lkml/bbn4lvdwh42m2zvi3rdyws66y5ulew32rchtz3kxirqlllkr63@7toa4tcepax3/#t
> > >
> > > Thanks,
> > > Xuewei
> >
> > it's been in net for two weeks now, no?
>
> Umm sorry, I didn't check the date carefully, because there are several
> ongoing patches. Next time I'll check it carefully. Sorry again.
>
> It looks like no one is paying attention to this patch. I am requesting
> someone interested in vsock to review this. I'd appreciate that!

Which patch do you mean?

Thanks,
Stefano

> On Thu, 12 Jun 2025 at 08:50, Xuewei Niu <niuxuewei97@gmail.com> wrote:
> >
> > > On Thu, Jun 12, 2025 at 01:32:01PM +0800, Xuewei Niu wrote:
> > > > No comments since last month.
> > > >
> > > > The patch [1], which adds SIOCINQ ioctl support for vsock, depends on this
> > > > patch. Could I get more eyes on this one?
> > > >
> > > > [1]: https://lore.kernel.org/lkml/bbn4lvdwh42m2zvi3rdyws66y5ulew32rchtz3kxirqlllkr63@7toa4tcepax3/#t
> > > >
> > > > Thanks,
> > > > Xuewei
> > >
> > > it's been in net for two weeks now, no?
> >
> > Umm sorry, I didn't check the date carefully, because there are several
> > ongoing patches. Next time I'll check it carefully. Sorry again.
> >
> > It looks like no one is paying attention to this patch. I am requesting
> > someone interested in vsock to review this. I'd appreciate that!
> 
> Which patch do you mean?
> 
> Thanks,
> Stefano

I am saying your patch, "vsock/virtio: fix `rx_bytes` accounting for stream
sockets".

Once this gets merged, I will send a new version of my patch to support
SIOCINQ ioctl. Thus, I can reuse `rx_bytes` to count unread bytes, as we
discussed.

Thanks,
Xuewei

On Thu, 12 Jun 2025 at 10:21, Xuewei Niu <niuxuewei97@gmail.com> wrote:
>
> > On Thu, 12 Jun 2025 at 08:50, Xuewei Niu <niuxuewei97@gmail.com> wrote:
> > >
> > > > On Thu, Jun 12, 2025 at 01:32:01PM +0800, Xuewei Niu wrote:
> > > > > No comments since last month.
> > > > >
> > > > > The patch [1], which adds SIOCINQ ioctl support for vsock, depends on this
> > > > > patch. Could I get more eyes on this one?
> > > > >
> > > > > [1]: https://lore.kernel.org/lkml/bbn4lvdwh42m2zvi3rdyws66y5ulew32rchtz3kxirqlllkr63@7toa4tcepax3/#t
> > > > >
> > > > > Thanks,
> > > > > Xuewei
> > > >
> > > > it's been in net for two weeks now, no?
> > >
> > > Umm sorry, I didn't check the date carefully, because there are several
> > > ongoing patches. Next time I'll check it carefully. Sorry again.
> > >
> > > It looks like no one is paying attention to this patch. I am requesting
> > > someone interested in vsock to review this. I'd appreciate that!
> >
> > Which patch do you mean?
> >
> > Thanks,
> > Stefano
>
> I am saying your patch, "vsock/virtio: fix `rx_bytes` accounting for stream
> sockets".
>
> Once this gets merged, I will send a new version of my patch to support
> SIOCINQ ioctl. Thus, I can reuse `rx_bytes` to count unread bytes, as we
> discussed.

As Michael pointed out, it was merged several weeks ago in net tree,
see https://lore.kernel.org/netdev/174827942876.985160.7017354014266756923.git-patchwork-notify@kernel.org/
And it also landed in Linus tree:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=45ca7e9f0730ae36fc610e675b990e9cc9ca0714

So, I think you can go head with your patch, right?

Please remember to target net-next, since it will be a new feature IIRC.

Thanks,
Stefano

> On Thu, 12 Jun 2025 at 10:21, Xuewei Niu <niuxuewei97@gmail.com> wrote:
> >
> > > On Thu, 12 Jun 2025 at 08:50, Xuewei Niu <niuxuewei97@gmail.com> wrote:
> > > >
> > > > > On Thu, Jun 12, 2025 at 01:32:01PM +0800, Xuewei Niu wrote:
> > > > > > No comments since last month.
> > > > > >
> > > > > > The patch [1], which adds SIOCINQ ioctl support for vsock, depends on this
> > > > > > patch. Could I get more eyes on this one?
> > > > > >
> > > > > > [1]: https://lore.kernel.org/lkml/bbn4lvdwh42m2zvi3rdyws66y5ulew32rchtz3kxirqlllkr63@7toa4tcepax3/#t
> > > > > >
> > > > > > Thanks,
> > > > > > Xuewei
> > > > >
> > > > > it's been in net for two weeks now, no?
> > > >
> > > > Umm sorry, I didn't check the date carefully, because there are several
> > > > ongoing patches. Next time I'll check it carefully. Sorry again.
> > > >
> > > > It looks like no one is paying attention to this patch. I am requesting
> > > > someone interested in vsock to review this. I'd appreciate that!
> > >
> > > Which patch do you mean?
> > >
> > > Thanks,
> > > Stefano
> >
> > I am saying your patch, "vsock/virtio: fix `rx_bytes` accounting for stream
> > sockets".
> >
> > Once this gets merged, I will send a new version of my patch to support
> > SIOCINQ ioctl. Thus, I can reuse `rx_bytes` to count unread bytes, as we
> > discussed.
> 
> As Michael pointed out, it was merged several weeks ago in net tree,
> see https://lore.kernel.org/netdev/174827942876.985160.7017354014266756923.git-patchwork-notify@kernel.org/
> And it also landed in Linus tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=45ca7e9f0730ae36fc610e675b990e9cc9ca0714

I misunderstood Michael's point. I am new to this, and not familiar with
the process. Sorry about that...

> So, I think you can go head with your patch, right?
>
> Please remember to target net-next, since it will be a new feature IIRC.
> 
> Thanks,
> Stefano

Yes, I'll do it ASAP.

Thanks,
Xuewei

On Thu, Jun 12, 2025 at 04:55:14PM +0800, Xuewei Niu wrote:
>> On Thu, 12 Jun 2025 at 10:21, Xuewei Niu <niuxuewei97@gmail.com> wrote:
>> >
>> > > On Thu, 12 Jun 2025 at 08:50, Xuewei Niu <niuxuewei97@gmail.com> wrote:
>> > > >
>> > > > > On Thu, Jun 12, 2025 at 01:32:01PM +0800, Xuewei Niu wrote:
>> > > > > > No comments since last month.
>> > > > > >
>> > > > > > The patch [1], which adds SIOCINQ ioctl support for vsock, depends on this
>> > > > > > patch. Could I get more eyes on this one?
>> > > > > >
>> > > > > > [1]: https://lore.kernel.org/lkml/bbn4lvdwh42m2zvi3rdyws66y5ulew32rchtz3kxirqlllkr63@7toa4tcepax3/#t
>> > > > > >
>> > > > > > Thanks,
>> > > > > > Xuewei
>> > > > >
>> > > > > it's been in net for two weeks now, no?
>> > > >
>> > > > Umm sorry, I didn't check the date carefully, because there are several
>> > > > ongoing patches. Next time I'll check it carefully. Sorry again.
>> > > >
>> > > > It looks like no one is paying attention to this patch. I am requesting
>> > > > someone interested in vsock to review this. I'd appreciate that!
>> > >
>> > > Which patch do you mean?
>> > >
>> > > Thanks,
>> > > Stefano
>> >
>> > I am saying your patch, "vsock/virtio: fix `rx_bytes` accounting for stream
>> > sockets".
>> >
>> > Once this gets merged, I will send a new version of my patch to support
>> > SIOCINQ ioctl. Thus, I can reuse `rx_bytes` to count unread bytes, as we
>> > discussed.
>>
>> As Michael pointed out, it was merged several weeks ago in net tree,
>> see https://lore.kernel.org/netdev/174827942876.985160.7017354014266756923.git-patchwork-notify@kernel.org/
>> And it also landed in Linus tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=45ca7e9f0730ae36fc610e675b990e9cc9ca0714
>
>I misunderstood Michael's point. I am new to this, and not familiar with
>the process. Sorry about that...

Don't worry ;-)

Hope now it's clear!

Thanks,
Stefano