On Tue, May 12, 2026 at 12:03:38PM +0200, Stefano Garzarella wrote: > On Tue, May 12, 2026 at 04:54:34AM -0400, Michael S. Tsirkin wrote: > > On Tue, May 12, 2026 at 10:07:37AM +0200, Stefano Garzarella wrote: > > > From: Stefano Garzarella <[email protected]> > > > > > > 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). > > > > > > With this approach we currently have 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 using `buf_alloc * 2` as the total budget for payload plus > > > skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF > > > is doubled to reserve space for sk_buff metadata. This preserves the > > > full buf_alloc for payload under normal operation, while still bounding > > > the skb queue growth. > > > > > > When the total budget (buf_alloc * 2) is exceeded (e.g. under small-packet > > > flooding where overhead dominates), the connection is reset and local > > > socket error set to ENOBUFS, so both peers are explicitly notified of > > > the failure rather than silently losing data. > > > > > > With this patch, all tests in tools/testing/vsock/vsock_test.c are > > > now passing again. > > > > > > A solution to handle small-packet overhead efficiently also for > > > SEQPACKET (we already do that for STREAM) is planned as follow-up work. > > > This patch is needed in any case to prevent silent data loss, because > > > even if we reduce the overhead, we can't eliminate it entirely. > > > > > > Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue") > > > Signed-off-by: Stefano Garzarella <[email protected]> > > > > Thanks for the patch! I'd like to split this: > > 1. buf alloc boost > > 2. reset when out of credits > > Good point, also the reset maybe should have an other fixes tag (i.e. when > we introduced that check) > > > > > this way we can revert 2 easier later. > > I'm not sure if we should revert them at some point, even though we'll be > able to handle the overhead better,
I mean we'll prevent the overflow, the condition will never be met. > but I agree that we should split them. > > I'll wait for a few more comments and then send v3 with the split. > > Thanks, > Stefano
