Migrate virtio-net from semu - #748
Conversation
There was a problem hiding this comment.
Benchmarks
Details
| Benchmark suite | Current: 1a52828 | Previous: f845a2e | Ratio |
|---|---|---|---|
Dhrystone |
1596.333 DMIPS |
1859.333 DMIPS |
1.16 |
CoreMark |
1116.946 iterations/sec |
1199.227 iterations/sec |
1.07 |
This comment was automatically generated by workflow using github-action-benchmark.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
7658d0a to
b775142
Compare
|
I’m not the original author of the virtio-net device. Please check the commit history via |
|
Thanks for the clarification, and sorry for the incorrect attribution. |
b775142 to
56a5c0b
Compare
jserv
left a comment
jserv
left a comment
There was a problem hiding this comment.
Refine CI pipeline to validate virtio-net.
Remove "This implementation is based on semu's virtio-net device, originally introduced by Jserv" as you already append "Co-authored-by: Jim Huang". |
56a5c0b to
6dd32bc
Compare
This commit migrates virtio-net support from semu with the following modifications: 1. Implement virtio-net device model The virtio-net implementation follows the VirtIO-MMIO flow used by virtio-blk, including feature negotiation, queue setup, QueueNotify handling, used ring update, interrupt status, and device status reset. The device currently supports a TAP-backend network interface and handles basic RX/TX virtqueue processing for guest network packets. 2. Add TAP backend helper Introduce netdev.c and netdev.h to provide host-side TAP device access. Future work may support other host-side backend. 3. Handle virtio-net header processing For guest TX, the device skips the virtio-net header before writing the Ethernet frame to the TAP backend. For guest RX, the device prepends a virtio-net header before copying the received Ethernet frame into the guest-provided RX buffer. 4. Implement MMIO_VIRTIONET Add MMIO routing for virtio-net and connect the device interrupt status to the PLIC, following the existing virtio-blk and virtio-rng interrupt update model. 5. Introduce new argument '-x vnet:<tap>' When virtio-net is enabled, rv32emu dynamically creates a virtio-mmio node in the generated device tree and assigns an MMIO base address and IRQ for the device. 6. Support coexistence with virtio-blk and virtio-rng Update the dynamic virtio-mmio device tree allocation path so virtio-net can coexist with existing virtio-blk and virtio-rng devices without reusing MMIO base addresses or IRQs. 7. Use virtio-net state Unlike semu's device integration model, rv32emu stores the virtio-net state in vm_attr_t so MMIO routing, interrupt routing, and device cleanup can access the same device instance. The emulator should be run with sudo when using the virtio-net TAP backend. Co-authored-by: Jim Huang <jserv@biilabs.io>
6dd32bc to
198ea5c
Compare
The host-arm64 CI job may fail before running any build or test when apt cannot update package indexes due to transient network issues on the Ubuntu ports mirror. Add retry logic and force IPv4 when updating the apt cache in install-llvm.sh to make LLVM repository setup more robust on GitHub-hosted ARM runners. The host-arm64 dependency installation also has a fallback path for partially failed apt installs. However, the fallback only attempted to install make, curl, and wget. As a result, the later Linux boot test could fail with "expect: command not found" even though expect is a required dependency for .ci/boot-linux.sh.
There was a problem hiding this comment.
2 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/system.h">
<violation number="1" location="src/system.h:89">
P1: Host packets received after the guest posts RX buffers can remain invisible: queue refresh sets virtio-net interrupt status but never asserts its PLIC line, while the guest is waiting for that interrupt. Update vnet interrupts immediately after `virtio_net_refresh_queue()` (and after any other asynchronous RX completion).</violation>
</file>
<file name="src/devices/virtio-net.c">
<violation number="1" location="src/devices/virtio-net.c:480">
P2: Enabling virtio-net adds a `poll(2)` syscall every 100 guest cycles even when no TAP or queue work exists, substantially slowing idle guest execution. Consider rate-limited polling or integrating TAP readiness with the emulator event loop.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| case MMIO_VIRTIONET: \ | ||
| IIF(rw)( /* read */ \ | ||
| mmio_read_val = virtio_net_read(PRIV(rv)->vnet, addr & 0xFFFFF); \ | ||
| emu_update_vnet_interrupts(rv); \ |
There was a problem hiding this comment.
P1: Host packets received after the guest posts RX buffers can remain invisible: queue refresh sets virtio-net interrupt status but never asserts its PLIC line, while the guest is waiting for that interrupt. Update vnet interrupts immediately after virtio_net_refresh_queue() (and after any other asynchronous RX completion).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/system.h, line 89:
<comment>Host packets received after the guest posts RX buffers can remain invisible: queue refresh sets virtio-net interrupt status but never asserts its PLIC line, while the guest is waiting for that interrupt. Update vnet interrupts immediately after `virtio_net_refresh_queue()` (and after any other asynchronous RX completion).</comment>
<file context>
@@ -82,6 +83,17 @@ enum SUPPORTED_MMIO {
+ case MMIO_VIRTIONET: \
+ IIF(rw)( /* read */ \
+ mmio_read_val = virtio_net_read(PRIV(rv)->vnet, addr & 0xFFFFF); \
+ emu_update_vnet_interrupts(rv); \
+ return mmio_read_val; \
+ , /* write */ \
</file context>
| .events = POLLIN | POLLOUT, | ||
| }; | ||
|
|
||
| poll(&pfd, 1, 0); |
There was a problem hiding this comment.
P2: Enabling virtio-net adds a poll(2) syscall every 100 guest cycles even when no TAP or queue work exists, substantially slowing idle guest execution. Consider rate-limited polling or integrating TAP readiness with the emulator event loop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/devices/virtio-net.c, line 480:
<comment>Enabling virtio-net adds a `poll(2)` syscall every 100 guest cycles even when no TAP or queue work exists, substantially slowing idle guest execution. Consider rate-limited polling or integrating TAP readiness with the emulator event loop.</comment>
<file context>
@@ -0,0 +1,693 @@
+ .events = POLLIN | POLLOUT,
+ };
+
+ poll(&pfd, 1, 0);
+
+ if (pfd.revents & POLLIN) {
</file context>
jserv
left a comment
jserv
left a comment
There was a problem hiding this comment.
Rework semu's network infrastructure, as described in networking.md:
- Linux: TAP (kernel-level) and user-mode (SLIRP) networking
- macOS: vmnet.framework (kernel-level NAT; bridge mode planned) and user-mode (SLIRP) networking
For this pull request, both TAP and SLIRP should be landed.
| } | ||
| opt_virtio_blk_img[opt_virtio_blk_idx++] = | ||
| optarg + 5; /* strlen("vblk:") */ | ||
| } else if (!strncmp("vnet:", optarg, 5)) { |
There was a problem hiding this comment.
Do we need to limit the vnet cli option passed by the user like the vblk?
There was a problem hiding this comment.
I added validation for the vnet option.
At this stage, only one virtio-net device is supported, so repeated vnet options are rejected. The backend is also validated and currently restricted to tap. The user backend will be enabled in the follow-up SLIRP commit.
| for (; !rv_has_halted(rv);) { /* run until the flag is done */ | ||
| rv_step(rv); /* step instructions */ | ||
|
|
||
| #if RV32_HAS(SYSTEM_MMIO) |
There was a problem hiding this comment.
Can we do the check in rv_step?
There was a problem hiding this comment.
I moved the virtio-net refresh and interrupt propagation into the per-step execution path because the current backend uses non-blocking polling to receive packets from the host side. I plan to improve this in a follow-up with an event-driven wakeup path, eg. using eventfd.
Remove the trailing space from the vnet help text, reject unsupported virtio-net backends, and avoid repeated virtio-net options when only one virtio-net device is supported. At this stage only the TAP backend is accepted. The user-mode backend will be enabled later when SLIRP support is added. Also remove the unnecessary device index increment after adding the virtio-net DTB node, since no later device uses the incremented value
Move the virtio-net validation logic into .ci/netdev.sh and invoke it from .ci/boot-linux.sh when VNET_BACKEND is set. This keeps the network-specific checks modular while reusing existing Linux boot test flow, including boot-linux-prepare.sh setup and cleanup, matching the structure used by the RTC tests. The TAP test still validates that the guest virtio_net driver binds, brings eth0 up, assigns the guest address, and can ping the host TAP gateway.
122d1dc to
50532e7
Compare
virtio-net queue refresh can complete RX/TX descriptors and set the device interrupt status, but PLIC line was not updated immediately after refresh. This could leave the guest waiting until later interrupt update point before observing completed network work, causing TAP ping latency spikes of around one second. Update the virtio-net interrupt after queue refresh. This delivers completed network packets to the guest promptly. Also make virtio_net_try_rx() and virtio_net_try_tx() raise used-ring interrupts only when used ring index actually advances. TAP is often reported writable by poll(), so trying TX without completing any descriptor must not set VIRTIO_INT_USED_RING. Otherwise guest may see repeated interrupts without corresponding used-ring updates. Before this change, TAP ping could show delayed replies such as: PING 192.168.100.1 (192.168.100.1): 56 data bytes 64 bytes from 192.168.100.1: seq=0 ttl=64 time=3.831 ms 64 bytes from 192.168.100.1: seq=1 ttl=64 time=1001.088 ms 64 bytes from 192.168.100.1: seq=2 ttl=64 time=1.389 ms After the change, replies are delivered promptly: PING 192.168.100.1 (192.168.100.1): 56 data bytes 64 bytes from 192.168.100.1: seq=0 ttl=64 time=1.181 ms 64 bytes from 192.168.100.1: seq=1 ttl=64 time=0.546 ms 64 bytes from 192.168.100.1: seq=2 ttl=64 time=0.565 ms
There was a problem hiding this comment.
1 issue found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/devices/slirp.c">
<violation number="1" location="src/devices/slirp.c:376">
P2: Idle user-mode networking issues two host `poll()` syscalls per 100 guest instructions, substantially throttling system emulation. Throttle polling or integrate SLIRP scheduling with the main loop instead of polling on every refresh.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| if (usr->pfd_len == 0) | ||
| return 0; | ||
|
|
||
| int ret = poll(usr->pfd, (nfds_t) usr->pfd_len, 0); |
There was a problem hiding this comment.
P2: Idle user-mode networking issues two host poll() syscalls per 100 guest instructions, substantially throttling system emulation. Throttle polling or integrate SLIRP scheduling with the main loop instead of polling on every refresh.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/devices/slirp.c, line 376:
<comment>Idle user-mode networking issues two host `poll()` syscalls per 100 guest instructions, substantially throttling system emulation. Throttle polling or integrate SLIRP scheduling with the main loop instead of polling on every refresh.</comment>
<file context>
@@ -0,0 +1,399 @@
+ if (usr->pfd_len == 0)
+ return 0;
+
+ int ret = poll(usr->pfd, (nfds_t) usr->pfd_len, 0);
+ if (ret < 0) {
+ if (errno == EINTR)
</file context>
There was a problem hiding this comment.
The current implementation uses non-blocking I/O and polls SLIRP from the device refresh path, which is simple and keeps the migration consistent with the existing execution loop. I plan to address this in a follow-up PR with a proper event-driven wakeup path.
4cf6367 to
c48ad2a
Compare
Migrate semu's user-mode virtio-net networking support to rv32emu. This adds SLIRP backend based on minislirp, allowing virtio-net to work without TAP, root privileges, or host network setup. The backend connects the virtio-net RX/TX paths with libslirp through non-blocking socketpairs and uses the standard SLIRP guest network configuration: guest IP: 10.0.2.15/24 gateway: 10.0.2.2 DNS: 10.0.2.3 Several semu-specific parts are adapted for rv32emu: - Replace semu's timer/event integration with a small CLOCK_MONOTONIC-based SLIRP timer wrapper. - Drive SLIRP progress from virtio_net_refresh_queue(), matching rv32emu's existing execution loop and virtio-net polling model. - Keep packet forwarding non-blocking so backend can be refreshed from the emulator loop without stalling guest execution. - Also extend virtio-net CI coverage through the existing Linux boot test flow. The CI now validates Linux TAP networking and user-mode SLIRP networking on Linux, and also runs the user-mode SLIRP boot test on macOS.
c48ad2a to
103be25
Compare
|
This update mainly covers two parts:
I moved virtio-net refresh and interrupt propagation into the per-step execution path. After queue refresh, the virtio-net interrupt state is pushed to the PLIC. This avoids delaying completed RX/TX work until a later interrupt update point. I also updated virtio_net_try_rx() and virtio_net_try_tx() so they only raise used-ring interrupts when the used ring index actually advances. This avoids repeated interrupts when no virtqueue progress was made, and fixes the observed TAP ping latency spike. Before this change, TAP ping could show delayed replies such as: After the change, replies are delivered promptly:
I added a minislirp-based user-mode backend for virtio-net, adapted from semu. The backend connects the virtio-net RX/TX paths with libslirp through non-blocking socketpairs, so guest networking can work without TAP, root rivileges, or host network setup. The current backend support is:
Emscripten is handled separately because rv32emu supports emcc builds, unlike semu's original networking setup. For emcc, the networking backends are disabled and unsupported vnet backends are rejected during argument parsing. CI coverage is arranged as follows:
Future PR may work:
|
| typedef struct netdev netdev_t; | ||
|
|
||
| typedef enum { | ||
| NETDEV_IMPL_none = 0, |
There was a problem hiding this comment.
Prefer all in upper case: NETDEV_IMPL_NONE.
| typedef enum { | ||
| NETDEV_IMPL_none = 0, | ||
| #if RV32EMU_NET_HAS_TAP | ||
| NETDEV_IMPL_tap, |
| NETDEV_IMPL_tap, | ||
| #endif | ||
| #if RV32EMU_NET_HAS_SLIRP | ||
| NETDEV_IMPL_user, |
|
Hi @Charlie-Tsai1123 , could you document how to test the user mode vnet backend as I only see the |
| "(default read and write). This option may be specified " | ||
| "multiple times for multiple block devices\n" | ||
| " -x vrng : enable virtio-rng device\n" | ||
| " -x vnet:<backend>: use <backend> as virtio-net backend interface\n" |
There was a problem hiding this comment.
Add possible vnet backend here.
E.g., -x vnet:<backend>: use <backend> as virtio-net backend interface(supported backend: user/tap)\n
But if vnet is togglable in Kconfig, this should be adjusted again.
There was a problem hiding this comment.
Done. I've updated the help message to include tap and user. I also made it dynamic based on Kconfig: the -x vnet help text will now only display the specific backends that are actually enabled in the current build configuration.
| } | ||
| #endif | ||
|
|
||
| bool netdev_init(netdev_t *netdev, const char *net_type) |
There was a problem hiding this comment.
Initialization for NETDEV_IMPL_user has duplicated logic. Please rewrite.
Something like this might be cleaner:
/* Linux specify */
if defined(__linux__) && RV32EMU_NET_HAS_TAP
init_tap()
else /* user mode for macOS and Linux */
init_user()
endifThere was a problem hiding this comment.
Done. I write netdev_setup for duplicated logic.
There was a problem hiding this comment.
1 issue found across 14 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mk/system.mk">
<violation number="1" location="mk/system.mk:61">
P3: When VIRTIO_NET=y but both backends are disabled (VIRTIO_NET_TAP=n and VIRTIO_NET_USER=n), this block still compiles virtio-net.o and netdev.o even though no backend is available, so vnet degrades to a runtime 'no backend compiled' failure instead of being excluded. Consider also filtering those objects when neither backend is enabled, mirroring the VIRTIO_NET!=y branch.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| ifneq ($(CONFIG_VIRTIO_NET),y) | ||
| DEV_OBJS := $(filter-out $(DEV_OUT)/virtio-net.o $(DEV_OUT)/netdev.o $(DEV_OUT)/slirp.o, $(DEV_OBJS)) | ||
| else | ||
| ifneq ($(CONFIG_VIRTIO_NET_USER),y) |
There was a problem hiding this comment.
P3: When VIRTIO_NET=y but both backends are disabled (VIRTIO_NET_TAP=n and VIRTIO_NET_USER=n), this block still compiles virtio-net.o and netdev.o even though no backend is available, so vnet degrades to a runtime 'no backend compiled' failure instead of being excluded. Consider also filtering those objects when neither backend is enabled, mirroring the VIRTIO_NET!=y branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mk/system.mk, line 61:
<comment>When VIRTIO_NET=y but both backends are disabled (VIRTIO_NET_TAP=n and VIRTIO_NET_USER=n), this block still compiles virtio-net.o and netdev.o even though no backend is available, so vnet degrades to a runtime 'no backend compiled' failure instead of being excluded. Consider also filtering those objects when neither backend is enabled, mirroring the VIRTIO_NET!=y branch.</comment>
<file context>
@@ -52,6 +52,17 @@ $(DEV_OUT)/%.o: $(DEV_SRC)/%.c $(EFFECTIVE_CONFIG_STAMP) | $(DEV_OUT)
+ifneq ($(CONFIG_VIRTIO_NET),y)
+DEV_OBJS := $(filter-out $(DEV_OUT)/virtio-net.o $(DEV_OUT)/netdev.o $(DEV_OUT)/slirp.o, $(DEV_OBJS))
+else
+ifneq ($(CONFIG_VIRTIO_NET_USER),y)
+DEV_OBJS := $(filter-out $(DEV_OUT)/slirp.o, $(DEV_OBJS))
+endif
</file context>
Add Kconfig options for the virtio-net device, Linux TAP backend, and user-mode SLIRP backend. Exclude unused network objects at build time, build minislirp only when the user backend is enabled, and guard the related CLI, runtime, MMIO, DTB, and interrupt integration. Also list only compiled backends in the CLI help and consolidate backend initialization through a shared helper (netdev_setup in src/devices/netdev.c).
d1cbd60 to
1a52828
Compare
|
Thanks for the review. I have added build-time backend selection through Kconfig and documented how to test the user-mode backend. test user-modeBuild (Host): Guest device verification (Guest): Expected result (Guest): or test with the existing automated user-mode network test (Host): Kconfig changesThe following options were added: Default select Under the system-emulation configuration, users can independently enable or disable: When When user cancel virtio-net, the binary size of rv32emu exe decrease. I copy the build file and test copy file after Then their size: |
|
Thanks for the suggestion. I rebased the virtio-net changes onto the latest version of PR #638 and updated the CI integration accordingly. Previously, the virtio-net user-mode and TAP tests were defined as separate GitHub Actions steps. They are now integrated into
Previous: All relevant build and boot tests passed. The only failing job is Temporary integration branch: I have not updated the branch of PR #748 yet because PR #638 is still open. |
4 participants
Summary
This PR migrates virtio-net support from semu and follows the existing virtio-blk and virtio-rng integration in rv32emu.
The implementation adds a TAP-backed virtio-net device model for system emulation mode, including MMIO register handling, queue setup, feature negotiation, RX/TX virtqueue handling, virtio-net header processing, interrupt delivery through the PLIC, dynamic DTB node creation, and a runtime option for enabling the device.
Implementation notes
virtio,mmioDTB node for virtio-net.sudoor equivalent permissions when using the TAP backend.Test
Build:
After run rv32emu with
-x vnet:taprv32emu would build TAP, so host linux doesn't need to build TAP again.Host TAP setup:
Guest device verification:
readlink /sys/bus/virtio/devices/virtio0/driver ip link set eth0 up ip addr add 192.168.100.2/24 dev eth0 ip addr show eth0 ping -c 3 192.168.100.1Expected Result:
Summary by cubic
Adds a virtio-net device with TAP and user-mode SLIRP backends over VirtIO‑MMIO and the PLIC. Backends are build-time configurable and selected at runtime with -x vnet:tap or -x vnet:user; the device is added to the DTB and covered by Linux and macOS boot tests.
New Features
tapanduser(SLIRP) with non-blocking I/O; macOS supportsuser; Emscripten disables networking; onevnetdevice only; CLI validates backend and lists only compiled backends.CONFIG_VIRTIO_NET,CONFIG_VIRTIO_NET_TAP,CONFIG_VIRTIO_NET_USER; exclude unused net objects; buildsrc/minislirponly whenuseris enabled (macOS links-lresolv); MMIO/DTB/interrupt logic is compiled only when enabled..ci/netdev.shruns via.ci/boot-linux.sh; adds Linux jobs foruserandtap, plus macOSuser; ARM64 setup retries apt-get with IPv4 and ensuresexpect; docs atdocs/networking.md.Bug Fixes
Written for commit 1a52828. Summary will update on new commits.