Skip to content

Commit 45ca7e9

Browse files
stefano-garzarellaPaolo Abeni
authored andcommitted
vsock/virtio: fix rx_bytes accounting for stream sockets
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 06a8fc7 ("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 93b8088 ("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: 93b8088 ("virtio/vsock: fix logic which reduces credit update messages") Signed-off-by: Stefano Garzarella <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
1 parent ba5cb47 commit 45ca7e9

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

include/linux/virtio_vsock.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ struct virtio_vsock_sock {
140140
u32 last_fwd_cnt;
141141
u32 rx_bytes;
142142
u32 buf_alloc;
143+
u32 buf_used;
143144
struct sk_buff_head rx_queue;
144145
u32 msg_count;
145146
};

net/vmw_vsock/virtio_transport_common.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -441,18 +441,20 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
441441
static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
442442
u32 len)
443443
{
444-
if (vvs->rx_bytes + len > vvs->buf_alloc)
444+
if (vvs->buf_used + len > vvs->buf_alloc)
445445
return false;
446446

447447
vvs->rx_bytes += len;
448+
vvs->buf_used += len;
448449
return true;
449450
}
450451

451452
static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs,
452-
u32 len)
453+
u32 bytes_read, u32 bytes_dequeued)
453454
{
454-
vvs->rx_bytes -= len;
455-
vvs->fwd_cnt += len;
455+
vvs->rx_bytes -= bytes_read;
456+
vvs->buf_used -= bytes_dequeued;
457+
vvs->fwd_cnt += bytes_dequeued;
456458
}
457459

458460
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,
581583
size_t len)
582584
{
583585
struct virtio_vsock_sock *vvs = vsk->trans;
584-
size_t bytes, total = 0;
585586
struct sk_buff *skb;
586587
u32 fwd_cnt_delta;
587588
bool low_rx_bytes;
588589
int err = -EFAULT;
590+
size_t total = 0;
589591
u32 free_space;
590592

591593
spin_lock_bh(&vvs->rx_lock);
@@ -597,6 +599,8 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
597599
}
598600

599601
while (total < len && !skb_queue_empty(&vvs->rx_queue)) {
602+
size_t bytes, dequeued = 0;
603+
600604
skb = skb_peek(&vvs->rx_queue);
601605

602606
bytes = min_t(size_t, len - total,
@@ -620,12 +624,12 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
620624
VIRTIO_VSOCK_SKB_CB(skb)->offset += bytes;
621625

622626
if (skb->len == VIRTIO_VSOCK_SKB_CB(skb)->offset) {
623-
u32 pkt_len = le32_to_cpu(virtio_vsock_hdr(skb)->len);
624-
625-
virtio_transport_dec_rx_pkt(vvs, pkt_len);
627+
dequeued = le32_to_cpu(virtio_vsock_hdr(skb)->len);
626628
__skb_unlink(skb, &vvs->rx_queue);
627629
consume_skb(skb);
628630
}
631+
632+
virtio_transport_dec_rx_pkt(vvs, bytes, dequeued);
629633
}
630634

631635
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,
781785
msg->msg_flags |= MSG_EOR;
782786
}
783787

784-
virtio_transport_dec_rx_pkt(vvs, pkt_len);
788+
virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
785789
kfree_skb(skb);
786790
}
787791

@@ -1735,6 +1739,7 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto
17351739
struct sock *sk = sk_vsock(vsk);
17361740
struct virtio_vsock_hdr *hdr;
17371741
struct sk_buff *skb;
1742+
u32 pkt_len;
17381743
int off = 0;
17391744
int err;
17401745

@@ -1752,7 +1757,8 @@ int virtio_transport_read_skb(struct vsock_sock *vsk, skb_read_actor_t recv_acto
17521757
if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
17531758
vvs->msg_count--;
17541759

1755-
virtio_transport_dec_rx_pkt(vvs, le32_to_cpu(hdr->len));
1760+
pkt_len = le32_to_cpu(hdr->len);
1761+
virtio_transport_dec_rx_pkt(vvs, pkt_len, pkt_len);
17561762
spin_unlock_bh(&vvs->rx_lock);
17571763

17581764
virtio_transport_send_credit_update(vsk);

0 commit comments

Comments
 (0)