Napi#629
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds NAPI receive mode behind a new Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/infra/datapath/main_loop.c`:
- Around line 224-227: The code unconditionally calls vec_add(*registered, *qm)
after attempting to register the queue with
rte_eth_dev_rx_intr_ctl_q(qm->port_id, qm->queue_id, RTE_EPOLL_PER_THREAD,
RTE_INTR_EVENT_ADD, NULL); so a failing registration still marks the queue as
registered and later gets skipped in napi_wait(). Change this to capture the
return value of rte_eth_dev_rx_intr_ctl_q, check for success (e.g., ret == 0),
only call vec_add(*registered, *qm) on success, and handle/log the failure path
(using process logging or similar) and do not mark the queue as registered when
the call fails; reference symbols: rte_eth_dev_rx_intr_ctl_q, vec_add,
registered, qm, napi_wait.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c9daab1-60e9-44a6-b828-7de881b450d8
📒 Files selected for processing (4)
main/config.hmain/main.cmodules/infra/control/port.cmodules/infra/datapath/main_loop.c
vjardin
left a comment
vjardin
left a comment
There was a problem hiding this comment.
sched_setattr() shall be used from glibc.
| // utilization and downclocks the core even at line rate. Pin uclamp_min to the | ||
| // max capacity: the governor runs the core at full speed while the worker is | ||
| // runnable and lets it drop only when it actually sleeps on the interrupt. | ||
| // glibc exposes neither struct sched_attr nor a sched_setattr() wrapper. |
There was a problem hiding this comment.
|
Disclaimer: I haven't looked at the patch in detail, so my feedback is high-level only. Have you measured the RX interrupt wakeup latency? In other words: Are user-space interrupts fast enough for general use, or only for low-traffic hours? Another detail: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/infra/control/worker.c`:
- Around line 50-54: The issue is that attr (used with pthread_attr_destroy) may
be uninitialized if eventfd() fails and the function jumps to end; to fix,
ensure pthread_attr_t attr is initialized before any early goto that can skip
pthread_attr_init or rearrange the control flow so pthread_attr_destroy is only
called when pthread_attr_init succeeded: either move the
pthread_attr_init(&attr) before the eventfd() call (so attr is always
initialized) or add a boolean/flag (e.g., attr_initialized) set after
pthread_attr_init and check it before calling pthread_attr_destroy(&attr);
update references in this function where worker->wakeup_fd, eventfd,
pthread_attr_init, pthread_attr_destroy, and attr are used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90a04b41-6c86-4ef5-a98d-1c2a45c64c6d
📒 Files selected for processing (7)
main/config.hmain/main.cmeson.buildmodules/infra/control/port.cmodules/infra/control/worker.cmodules/infra/control/worker.hmodules/infra/datapath/main_loop.c
✅ Files skipped from review due to trivial changes (1)
- main/config.h
🚧 Files skipped from review as they are similar to previous changes (3)
- main/main.c
- modules/infra/control/port.c
- modules/infra/datapath/main_loop.c
|
Thanks @MortenBroerup , both points are spot on. Let me split them. Wakeup latency / timerslackThe timerslack concern doesn't apply to the napi path: an idle worker doesn't So the dominant latency term isn't the scheduler, it's the PMD's interrupt To answer the underlying question directly: this is opt-in ( Other wakeup sourcesAgreed this is the important design point. Two cases: Control-plane events already work: the worker's own wakeup eventfd is in the Datapath timers: today grout's datapath is purely packet-driven, there's no |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
modules/infra/control/port.c (2)
259-282: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winWake workers before returning on MTU-set errors.
p->startedis cleared before these early returns, butworker_wakeup_all()only runs on success. A NAPI worker can stay blocked inrte_epoll_wait(-1)until some unrelated event arrives, so the failure path needs the same kick.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/infra/control/port.c` around lines 259 - 282, The MTU update path in the port control logic leaves workers asleep on error because `worker_wakeup_all()` only runs after a successful `rte_eth_dev_start` in the MTU-setting flow. Update the MTU change handling around `rte_eth_dev_stop`, `rte_eth_dev_set_mtu`, and `rte_eth_dev_start` so any early return after `p->started` is cleared also wakes workers before exiting. Keep the wakeup behavior consistent with the success path in the same function that updates `iface->mtu` and `subinterfaces`.
313-368: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winWake workers on failed reconfig
p->startedis cleared before the stop/reset/configure path, but every early return before the finalworker_wakeup_all()skips the kick entirely. That leaves NAPI workers blocked on the old graph until some unrelated event arrives. Move the wakeup into a shared cleanup path so failures still break the sleep.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/infra/control/port.c` around lines 313 - 368, The reconfiguration path in port handling clears p->started and then has several early returns before reaching worker_wakeup_all(), which means workers are not kicked when stop/reset/configure fails. Update the control flow in the port reconfig logic so worker_wakeup_all() runs through a shared cleanup/exit path in the function that performs port_unplug(), port_configure(), rte_eth_dev_start(), and the reset/stop branch, ensuring workers are always awakened even on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/infra/control/worker.c`:
- Around line 50-54: The early eventfd() failure path in worker initialization
is using the shared end cleanup even though the worker has not yet been inserted
into the queue or had its thread created. Update the worker setup flow in the
worker initialization routine to track acquisition state with flags like
inserted and thread_created, or split into staged cleanup labels, so
STAILQ_REMOVE() and pthread_cancel() only run after those resources actually
exist. Make sure the cleanup path for the eventfd() failure exits before any
teardown that assumes insertion or thread creation has occurred.
- Around line 139-140: The wakeup write in worker wakeup handling does not retry
interrupted writes, so an EINTR can leave the NAPI wakeup undelivered and stall
teardown. Update the wakeup path around the write() call in the worker wakeup
logic to loop and retry when errno is EINTR, while still treating EAGAIN as a
harmless already-pending wakeup; keep the existing error logging in the worker
code for real failures only.
---
Outside diff comments:
In `@modules/infra/control/port.c`:
- Around line 259-282: The MTU update path in the port control logic leaves
workers asleep on error because `worker_wakeup_all()` only runs after a
successful `rte_eth_dev_start` in the MTU-setting flow. Update the MTU change
handling around `rte_eth_dev_stop`, `rte_eth_dev_set_mtu`, and
`rte_eth_dev_start` so any early return after `p->started` is cleared also wakes
workers before exiting. Keep the wakeup behavior consistent with the success
path in the same function that updates `iface->mtu` and `subinterfaces`.
- Around line 313-368: The reconfiguration path in port handling clears
p->started and then has several early returns before reaching
worker_wakeup_all(), which means workers are not kicked when
stop/reset/configure fails. Update the control flow in the port reconfig logic
so worker_wakeup_all() runs through a shared cleanup/exit path in the function
that performs port_unplug(), port_configure(), rte_eth_dev_start(), and the
reset/stop branch, ensuring workers are always awakened even on failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 226ac4be-c212-4ebd-ace0-499661461c6e
📒 Files selected for processing (7)
main/config.hmain/main.cmeson.buildmodules/infra/control/port.cmodules/infra/control/worker.cmodules/infra/control/worker.hmodules/infra/datapath/main_loop.c
🚧 Files skipped from review as they are similar to previous changes (5)
- main/config.h
- meson.build
- modules/infra/control/worker.h
- main/main.c
- modules/infra/datapath/main_loop.c
| if (write(w->wakeup_fd, &one, sizeof(one)) != sizeof(one) && errno != EAGAIN) | ||
| LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retry interrupted eventfd writes.
If write() returns -1/EINTR, the NAPI wakeup is not delivered; a worker blocked in rte_epoll_wait() can remain asleep and make teardown hang. Retry EINTR, while keeping EAGAIN as “already pending”.
Proposed fix
- if (write(w->wakeup_fd, &one, sizeof(one)) != sizeof(one) && errno != EAGAIN)
- LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno));
+ ssize_t n;
+
+ do {
+ n = write(w->wakeup_fd, &one, sizeof(one));
+ } while (n < 0 && errno == EINTR);
+
+ if (n < 0 && errno != EAGAIN)
+ LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno));
+ else if (n >= 0 && n != sizeof(one))
+ LOG(ERR, "worker %u wakeup_fd short write", w->cpu_id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (write(w->wakeup_fd, &one, sizeof(one)) != sizeof(one) && errno != EAGAIN) | |
| LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno)); | |
| ssize_t n; | |
| do { | |
| n = write(w->wakeup_fd, &one, sizeof(one)); | |
| } while (n < 0 && errno == EINTR); | |
| if (n < 0 && errno != EAGAIN) | |
| LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno)); | |
| else if (n >= 0 && n != sizeof(one)) | |
| LOG(ERR, "worker %u wakeup_fd short write", w->cpu_id); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/infra/control/worker.c` around lines 139 - 140, The wakeup write in
worker wakeup handling does not retry interrupted writes, so an EINTR can leave
the NAPI wakeup undelivered and stall teardown. Update the wakeup path around
the write() call in the worker wakeup logic to loop and retry when errno is
EINTR, while still treating EAGAIN as a harmless already-pending wakeup; keep
the existing error logging in the worker code for real failures only.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
modules/infra/control/worker.c (2)
139-140: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRetry interrupted eventfd writes.
If
write()returns-1witherrno == EINTR, the NAPI wakeup signal is not successfully delivered. A worker thread currently blocked inrte_epoll_wait()will remain asleep, potentially causing teardown or reconfiguration routines to hang indefinitely. Wrap the write operation in a retry loop to safely handleEINTR, while preserving the existing logic that correctly treatsEAGAINas an already-pending wakeup.🐛 Proposed fix
- if (write(w->wakeup_fd, &one, sizeof(one)) != sizeof(one) && errno != EAGAIN) - LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno)); + ssize_t n; + + do { + n = write(w->wakeup_fd, &one, sizeof(one)); + } while (n < 0 && errno == EINTR); + + if (n < 0 && errno != EAGAIN) + LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno)); + else if (n >= 0 && n != sizeof(one)) + LOG(ERR, "worker %u wakeup_fd short write", w->cpu_id);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/infra/control/worker.c` around lines 139 - 140, Update the wakeup write logic in the worker wakeup path to retry write() when it fails with EINTR, ensuring the NAPI wakeup signal is eventually delivered. Preserve the existing behavior of treating EAGAIN as a pending wakeup and logging other failures through the current LOG call.
50-54: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winSplit cleanup by acquired resource state.
The early
eventfd()failure path (and other failures prior to thread creation/insertion) jumps to the sharedendcleanup block, which unconditionally executesSTAILQ_REMOVE()andpthread_cancel()on the worker. Since the worker hasn't been inserted into the queue at this point,STAILQ_REMOVEwill traverse the list and dereference aNULLpointer, causing a guaranteed crash. Similarly,pthread_cancel()will act on an uninitialized thread handle.Track the acquisition state using flags to prevent acting on unacquired resources.
🐛 Proposed fix
Modify the
worker_createfunction to track state using booleans. For example:bool thread_created = false; bool inserted = false; // ... if (!!(ret = pthread_create(&worker->thread, &attr, gr_datapath_loop, worker))) goto end; thread_created = true; STAILQ_INSERT_TAIL(&workers, worker, next); inserted = true; // ... } else { if (worker != NULL) { if (inserted) STAILQ_REMOVE(&workers, worker, worker, next); if (thread_created) pthread_cancel(worker->thread); pthread_cond_destroy(&worker->wakeup.cond); // ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/infra/control/worker.c` around lines 50 - 54, Update worker_create to track thread creation and queue insertion with boolean state flags. Set each flag immediately after the corresponding pthread_create and STAILQ_INSERT_TAIL operations, then guard STAILQ_REMOVE and pthread_cancel in the shared cleanup path with those flags while preserving cleanup for resources that were acquired.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@modules/infra/control/worker.c`:
- Around line 139-140: Update the wakeup write logic in the worker wakeup path
to retry write() when it fails with EINTR, ensuring the NAPI wakeup signal is
eventually delivered. Preserve the existing behavior of treating EAGAIN as a
pending wakeup and logging other failures through the current LOG call.
- Around line 50-54: Update worker_create to track thread creation and queue
insertion with boolean state flags. Set each flag immediately after the
corresponding pthread_create and STAILQ_INSERT_TAIL operations, then guard
STAILQ_REMOVE and pthread_cancel in the shared cleanup path with those flags
while preserving cleanup for resources that were acquired.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3621a3a0-6ff1-42c9-92e4-5e4366522fcd
📒 Files selected for processing (12)
main/config.hmain/main.cmeson.buildmodules/infra/control/iface_test.cmodules/infra/control/port.cmodules/infra/control/worker.cmodules/infra/control/worker.hmodules/infra/datapath/main_loop.csmoke/_init.shsubprojects/dpdk-25.11.wrapsubprojects/packagefiles/dpdk/net-tap-drain-queue-fd-in-Rx-interrupt-mode.patchsubprojects/packagefiles/dpdk/net-tap-support-Rx-queue-interrupt-enable-disable.patch
🚧 Files skipped from review as they are similar to previous changes (5)
- meson.build
- modules/infra/control/worker.h
- main/config.h
- main/main.c
- modules/infra/datapath/main_loop.c
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/infra/control/worker.c`:
- Around line 52-56: Split the worker initialization cleanup around the
resource-acquisition stages in the worker creation function: when eventfd()
fails, do not remove the worker from the queue or cancel its thread because
neither resource exists yet. Track queue insertion and thread creation state, or
use staged cleanup labels, and guard STAILQ_REMOVE and
pthread_cancel(worker->thread) so each runs only after its corresponding
initialization succeeds.
- Around line 139-140: Update the wakeup write logic near the worker wakeup path
to retry the eventfd write when it fails with errno == EINTR, continuing until
it succeeds or fails for another reason. Preserve the existing EAGAIN
suppression and LOG(ERR) behavior for non-EINTR failures, using the write to
w->wakeup_fd in the worker wakeup code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92a351f1-0743-4e5c-8b56-1437b847e21f
📒 Files selected for processing (14)
main/config.hmain/main.cmeson.buildmodules/infra/control/iface_test.cmodules/infra/control/port.cmodules/infra/control/worker.cmodules/infra/control/worker.hmodules/infra/datapath/control_input.cmodules/infra/datapath/control_input.hmodules/infra/datapath/main_loop.csmoke/_init.shsubprojects/dpdk-25.11.wrapsubprojects/packagefiles/dpdk/net-tap-drain-queue-fd-in-Rx-interrupt-mode.patchsubprojects/packagefiles/dpdk/net-tap-support-Rx-queue-interrupt-enable-disable.patch
🚧 Files skipped from review as they are similar to previous changes (8)
- modules/infra/control/port.c
- smoke/_init.sh
- modules/infra/control/iface_test.c
- main/config.h
- subprojects/packagefiles/dpdk/net-tap-support-Rx-queue-interrupt-enable-disable.patch
- meson.build
- subprojects/packagefiles/dpdk/net-tap-drain-queue-fd-in-Rx-interrupt-mode.patch
- modules/infra/datapath/main_loop.c
| if (write(w->wakeup_fd, &val, sizeof(val)) != sizeof(val) && errno != EAGAIN) | ||
| LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Retry interrupted eventfd writes.
If the write() to the eventfd is interrupted by a signal (EINTR), it currently fails without retrying. This means a NAPI worker blocked in rte_epoll_wait() might miss the wakeup, potentially leading to a stalled worker or hung teardown. Loop and retry when errno == EINTR.
🛠️ Proposed fix
- if (write(w->wakeup_fd, &val, sizeof(val)) != sizeof(val) && errno != EAGAIN)
- LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno));
+ ssize_t n;
+ do {
+ n = write(w->wakeup_fd, &val, sizeof(val));
+ } while (n < 0 && errno == EINTR);
+
+ if (n < 0 && errno != EAGAIN)
+ LOG(ERR, "worker %u wakeup_fd write: %s", w->cpu_id, strerror(errno));
+ else if (n >= 0 && n != sizeof(val))
+ LOG(ERR, "worker %u wakeup_fd short write", w->cpu_id);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/infra/control/worker.c` around lines 139 - 140, Update the wakeup
write logic near the worker wakeup path to retry the eventfd write when it fails
with errno == EINTR, continuing until it succeeds or fails for another reason.
Preserve the existing EAGAIN suppression and LOG(ERR) behavior for non-EINTR
failures, using the write to w->wakeup_fd in the worker wakeup code.
ddbecd8 to
946c073
Compare
Add an opt-in --napi mode where an idle worker arms the interrupts on its rx queues and blocks on them through the generic rte_eth_dev_rx_intr_* / rte_epoll_wait API instead of busy-polling. A packet wakes the worker, which disarms and resumes polling: the usual poll/interrupt hybrid, with the interrupt acting only as a doorbell since frames are still pulled by the graph walk. A worker blocks only after staying idle for NAPI_EMPTY_WINDOWS housekeeping windows with all of its queues empty, so a single busy queue keeps it polling. --napi implies poll-mode and replaces the micro-sleep ramp with the interrupt block. As that block can last up to a second it is measured explicitly and the timestamp advanced past it, keeping the sleep in total_cycles but out of busy_cycles. napi_wait() tracks the queues it actually armed and disarms them through a single exit path, so a queue without interrupt support does not leave its predecessors armed, and only marks a queue epoll-registered once. A PMD without rx queue interrupt support keeps polling. It iterates a per-worker snapshot of the rxqs, refreshed when the worker picks up a config, so it never reads w->rxqs while the control plane frees and reassigns it during a queue redistribution. A port stop/start (e.g. the MTU applied at interface creation) may drop the rxq interrupt epoll registration on PMDs that free it on dev_stop (tap, mlx4, mlx5, mana, failsafe via rte_intr_free_epoll_fd). worker_wakeup_all breaks the worker's block, and a drained kick invalidates the registration cache so the queues are re-registered on the next wait. Signed-off-by: Maxime Leroy <maxime@leroys.fr>
A packet injected by the control plane via post_to_stack (replies to local UDP, OSPF, NDP, DHCP) would sit undrained while every worker was idle blocked on the napi rxq interrupt. Track the number of workers running the graph; when none is, post_to_stack wakes one, and napi_wait rechecks the control input ring before blocking (eventcount, seq_cst). That wake must not re-register the rxq interrupts: nothing changed for them, unlike the port stop/start case. So far any drained kick invalidated the registration cache; split the two by encoding the kick value. worker_rearm_all (stop/start) sets a high bit (WORKER_KICK_REARM) and only that invalidates the cache, while worker_wakeup_any (control input) writes a plain wake that leaves the registration untouched. Signed-off-by: Maxime Leroy <maxime@leroys.fr>
In --napi mode an idle worker blocks on the rxq interrupt, so the schedutil governor sees a low utilization and downclocks the core even when it later runs at line rate. Pin the worker's uclamp_min to the max capacity through sched_setattr(): the governor keeps the core at full speed while the worker is runnable and lets it drop only when it actually sleeps on the interrupt. glibc exposes neither struct sched_attr nor a sched_setattr() wrapper, so both are declared locally. Once pinned, going idle would otherwise leave the core clocked high, so before blocking for good napi_wait does a few short timeout waits (NAPI_SETTLE_TRIES x NAPI_SETTLE_MS): each wake lets schedutil re-evaluate the decayed utilization and ratchet the frequency back down. The syscall fails on kernels without uclamp support or without the privilege to set it, which would otherwise warn on every worker. Report the expected EOPNOTSUPP/ENOSYS/EPERM/EINVAL cases at NOTICE and keep WARNING for anything unexpected. Signed-off-by: Maxime Leroy <maxime@leroys.fr>
Run the smoke tests with --napi by default (overridable with napi=false) so the datapath is exercised in interrupt mode and idle workers do not burn CPU when there is no traffic. This needs the tap PMD to support the generic Rx queue interrupt API, so vendor two net/tap patches: net/tap: support Rx queue interrupt enable/disable net/tap: drain queue fd in Rx interrupt mode Signed-off-by: Maxime Leroy <maxime@leroys.fr>
3 participants
Opt-in --napi mode: an idle worker stops busy-polling and blocks on its rx queue interrupts via rte_eth_dev_rx_intr_* / rte_epoll_wait, resuming polling when a packet wakes it. A second commit pins the worker's uclamp_min to max so schedutil keeps the core at full clock while runnable, dropping only when it actually sleeps.
NAPI opt-in idle mode: RX interrupt blocking via per-thread epoll
Configuration / CLI
-n/--napi(gr_config.napiinmain/config.h).--napiis enabled, option parsing forcesgr_config.poll_mode = true(so idle uses the NAPI/interrupt-blocking path).--napiinmain/main.chelp text.Port setup: enable RX queue interrupts only in NAPI mode
port_configure(), RX queue interrupt configuration (conf.intr_conf.rxq) is enabled only whengr_config.napiis set; link-status interrupt logic is unchanged.rte_eth_dev_start()succeeds (in bothport_mtu_set()andiface_port_reconfig()re-start paths),worker_rearm_all()is called so dataplane workers re-arm against the started ports.Worker wakeups while blocked in
rte_epoll_wait()struct workergains aneventfd(wakeup_fd) used to wake dataplane workers that are blocked inrte_epoll_wait().worker_create()createswakeup_fd(EFD_NONBLOCK | EFD_CLOEXEC); both failure cleanup andworker_destroy()close it.wakeup_fdso epoll-blocked workers resume:worker_wakeup()writes1worker_rearm_all()issues a special “re-arm” kick (encoded via the newWORKER_KICK_REARMconstant inworker.h)Datapath idle behavior (
modules/infra/datapath/main_loop.c)gr_config.napi:NAPI_EMPTY_WINDOWSit entersnapi_wait().napi_wait():rte_eth_dev_rx_intr_enable()rte_epoll_wait()(uses a short “settle” timeout first, then waits indefinitely)wakeup_fdon return (so control-plane/rearm kicks break/adjust the wait)sleep_cycles/n_sleeps; otherwise the existingusleep()-based accounting remains.CPU utilization floor while runnable
worker_perf_floor()(called from the main loop whengr_config.napiis set) to raiseuclamp_minviasched_setattrwhen available, soschedutilholds higher frequency while the worker is runnable.meson.builddetects build-time availability ofsched_setattrand definesHAVE_SCHED_SETATTR.Control-plane wake behavior
control_input.cnow includesworker.hand uses an atomicgr_worker_activecheck after queueing control messages:worker_wakeup_any()to ensure the pending ring element is processed.control_input_pending()helper and declares it incontrol_input.h.Smoke test enablement
smoke/_init.shappends-ntogrout_extra_optionswhen${napi:-true}is true (default enabled).DPDK tap PMD support needed for RX interrupt blocking
dpdk-25.11.wrapto include two new tap-net patches:net-tap-support-Rx-queue-interrupt-enable-disable.patch: adds missing Rx interrupt enable/disable ops so the generic RX interrupt API can succeed (no-op implementations returning0).net-tap-drain-queue-fd-in-Rx-interrupt-mode.patch: records/locks RX interrupt mode (intr_mode/intr_mode_set), adjuststun_allocandpmd_rx_burstearly-exit behavior so epoll wakeups don’t incorrectly stop bursts while the fd remains readable.