virtio-net: fix UDP TX stall and improve TCP throughput - #102
Merged
Conversation
agicy
force-pushed
the
perf/virtio-net
branch
2 times, most recently
from
July 8, 2026 07:39
0b98c73 to
35eed39
Compare
Contributor
Author
|
6 additional commits:
|
…leak
Replace static char pad[64] with stack-local char pad[64] = {0}.
A static variable is shared across all net devices; when per-vq
threads are introduced, concurrent TX would race on the shared
buffer. Zero-initialization prevents leaking previous stack
contents into the TAP device on the short-packet padding path.
…thmetic A malicious or buggy guest may provide iov[0].iov_len < header_len. Without this check, iov_len underflows (unsigned wrap to a huge value) and writev reads beyond the guest buffer — leaking host memory. Add a bounds check that returns the descriptor via update_used_ring with len=0 when the header doesn't fit, matching the existing pattern in rm_iov_header on the RX path.
When rm_iov_header fails (iov[0].iov_len < header_len), the descriptor has already been consumed from the avail ring by process_descriptor_chain but was never posted to the used ring. This leaks the descriptor permanently from the guest's perspective. Fix: call update_used_ring(vq, idx, 0) to return the descriptor before freeing the iov.
When readv returns a non-EAGAIN error (EIO, EFAULT, etc.), the descriptor was consumed from the avail ring but never returned. The code fell through to the success path which would call update_used_ring with a bogus length. Fix: return the descriptor via update_used_ring(vq, idx, 0) on any readv error that is not EWOULDBLOCK.
When the TAP device is closed or the bridge goes down, readv returns 0 (EOF). Without handling this case, the code falls through to the success path and calls update_used_ring with header_len bytes, plus EPOLLIN fires continuously on an EOF fd, creating a tight loop that drains guest RX buffers. Fix: detect len==0, return the descriptor via update_used_ring(vq,idx,0), set rx_ready=0 to stop further RX attempts, and break out.
The file-scope static uint8_t trashbuf[1600] is shared across all net device instances. When per-vq threads are introduced, concurrent RX drops from multiple devices would race on this single buffer. Move the buffer to a stack-local variable inside virtio_net_event_handler.
UDP streams lose descriptors because the guest may add new TX descriptors between the last empty check and enable_notify, and the suppressed notification never re-fires. Wrap the drain+enable in a loop with a post-enable re-check.
…criptor traversal Single-pass traversal that counts and fills in one loop. INDIRECT descriptors are handled inline: the indirect table size is known from vdesc->len/16, so no separate counting pass is needed. Single pass halves the cross-VM desc_table accesses for the common non-INDIRECT path (net), saving ~150-400 cycles per packet on A55. The original process_descriptor_chain() (malloc version) is unchanged.
…ket malloc/free Replace process_descriptor_chain() + free(iov) in rx_event_handler and virtq_tx_handle_one_request with process_descriptor_chain_buf() using stack-allocated iov arrays (NET_IOV_MAX=8 entries). Eliminates ~80-230 cycles of malloc/free overhead per packet on A55.
update_used_ring() emits two write_barrier() (DMB ishst) calls for every single used-ring entry. On in-order cores like A55 each barrier stalls the store buffer for ~30-50 cycles, so N entries cost 2N barriers. The new update_used_ring_batch() writes all N entries in a tight loop and emits only two barriers total — one before the idx update and one after — regardless of N. This reduces the per-packet barrier cost from 2N to 2. Original update_used_ring() is kept for callers that process one request at a time (blk, console, gpu).
Switch to batch used-ring updates and inject an IRQ after every batch flush so the guest can recycle completed TX descriptors. Without the IRQ, UDP sendto() blocks permanently after filling the virtqueue.
The expression `!(uint16_t)VRING_USED_F_NO_NOTIFY` evaluates to 0 (logical NOT of 1), causing `flags &= 0` to clear every flag on the used ring — not just the intended VRING_USED_F_NO_NOTIFY bit. Replace with `~(uint16_t)VRING_USED_F_NO_NOTIFY` (bitwise NOT, 0xFFFE) to only clear the target bit.
Hook into the STATUS register write (the virtio spec-defined device state machine). Devices register a callback to react to state transitions: FEATURES_OK, DRIVER_OK, reset, etc.
With IFF_VNET_HDR, TAP reads/writes [virtio_net_hdr | eth_frame] instead of raw frames, matching the guest buffer layout. This eliminates: - rm_iov_header() and all manual iov manipulation in RX path - Manual virtio_net_hdr fill (memset + num_buffers) after readv - Header stripping in TX path before writev The TAP vnet header size is configured via TUNSETVNETHDRSZ in the status_changed hook when FEATURES_OK is set (features negotiated).
…ET_IOV_MAX from queue size NET_IOV_MAX is now VIRTQUEUE_NET_MAX_SIZE (256) in virtio_net.h. The per-chain iov buffers are heap-allocated in NetDev instead of on the stack, removing the arbitrary 8-entry limit.
update_used_ring_batch's count parameter, virtq_tx_handle_one_request's out_count, and both RX/TX batch_count locals now use size_t. Matches the type convention for sizes and counts.
…hain parse failure When process_descriptor_chain_buf returns < 1 (corrupt descriptor chain): - last_avail_idx was not advanced, causing the TX handler to spin infinitely on the same broken descriptor - no used ring entry was written, leaking the guest buffer Same issue existed in the RX handler: broken descriptor would cause the handler to retry indefinitely on each EPOLLIN event. Fix both TX and RX paths by advancing last_avail_idx and writing a used ring entry with len=0 (undefined for TX/OUT descriptors per spec), matching the existing pattern used for malformed TX packets.
When writev fails, the packet was not sent to TAP. The used ring len field is undefined for TX/OUT descriptors per the VirtIO spec, but recording all_len is misleading. Use 0 to clearly indicate no data was written (even though the guest driver ignores this field).
dallasxy
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two virtio-net TX correctness bugs fixed:
virtqueue_enable_notify: the guest may add TX descriptors during theVRING_USED_F_NO_NOTIFYwindow. Without a re-check those descriptors are lost.sendto().Also included are per-packet CPU optimizations (
process_descriptor_chain_bufreplaces per-packetmalloc/free,update_used_ring_batchamortises DMB overhead) and six standalone bug fixes.Test results
RK3588, zone1 (
192.168.255.3/24) <-> root-linux (192.168.255.2/24), iperf3 60s:-RRXUDP TX and TCP throughput is improved.
References
Related issue #101.
Partially-fixes hvisor #240.