Brijesh-Thakkar opened a new pull request, #21303:
URL: https://github.com/apache/nuttx/pull/21303
## Summary
`virtio_net_send()` ignored the return value of `virtio_net_addbuffer()`.
When the TX virtqueue has no room, `virtqueue_add_buffer()` fails with
`ERROR_VRING_FULL` without queueing anything, but send still returned `OK`.
According to the `transmit()` contract in `netdev_lowerhalf.h`, returning
`OK` hands the netpkt over to the driver, which has to free it with
`netpkt_free()` once it is sent. A packet that was never queued is never freed,
so every failed send leaks the IOB chain and one TX quota. After `bufnum`
failures the TX quota is exhausted, `netdev_upper_can_tx()` never returns true
again and the interface stops transmitting for good.
The fix returns a negated errno on failure (`-ENOMEM`, as `e1000`/`igb` do
when their TX ring is full; the virtqueue error code is not an errno), so the
upper half recycles the packet and restores the TX quota. Before returning it
also reclaims the buffers the other side has already consumed and enables the
TX done callback, so the transmission resumes when the ring drains. This is the
same sequence the successful path already runs at the end of send.
`virtio_net_addbuffer()` only writes the virtio header into the guard area
in front of the L2 header (guaranteed by the `static_assert` on
`CONFIG_NET_LL_GUARDSIZE`), so the packet is left in a state `netpkt_put()`
handles.
## Impact
virtio-net only. No API or Kconfig change. The failure path was previously
unreachable in practice because the upper half limits the in-flight packets
with the TX quota, so it only matters when the quota is bypassed (see "Allowing
temporarily exceeding quota" in `netpkt_get()`), but when it does happen the
effect is a permanent TX stall.
## Testing
Host: Ubuntu x86_64, QEMU 10.2.1 (`qemu-system-riscv64 -M virt,aclint=on
-cpu rv32 -smp 1 -bios none -semihosting -netdev user -device
virtio-net-device`), xPack riscv-none-elf-gcc 15.2.0, `rv-virt:netnsh` built on
top of current master. `tools/nxstyle` and `./tools/checkpatch.sh -g
upstream/master..HEAD` pass.
Normal operation with the fix: `ping -c 20 10.0.2.2` gives 20/20 replies,
free IOBs unchanged (18 before, 18 after).
Fault injection (temporary local change, not part of the PR):
`virtio_net_addbuffer()` returns an error on every 5th TX call, `ping -c 100
10.0.2.2` in the background, `/proc/iobinfo` sampled at about 0, 20, 60 and 110
s:
| | original | with this fix |
|---|---|---|
| free IOBs (nfree) | 18, 14, 12, 12 | 18, 18, 18, 18 |
| ping -c 100 | stalls at the 6th failure (= `bufnum`), never finishes |
completes, 80/100 received (only the 20 injected failures are lost) |
```
# original
nsh> cat /proc/iobinfo
ntotal nfree nwait nthrottle
24 12 0 10 <- 6 IOBs leaked, ping stuck
after icmp_seq=27
# with the fix
100 packets transmitted, 80 received, 20% packet loss, time 101006 ms
nsh> cat /proc/iobinfo
ntotal nfree nwait nthrottle
24 18 0 16
```
The RX counterpart (`virtio_net_rxfill()` ignoring the
`virtio_net_addbuffer()` result) was already fixed upstream by #20896, so no
follow-up is needed for it.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
