Skip to content

configure_nic_irq_affinity: replace oneshot with monitor daemon#984

Open
insatomcat wants to merge 1 commit into
mainfrom
irqnic
Open

configure_nic_irq_affinity: replace oneshot with monitor daemon#984
insatomcat wants to merge 1 commit into
mainfrom
irqnic

Conversation

@insatomcat

@insatomcat insatomcat commented Jul 23, 2026

Copy link
Copy Markdown
Member

Problems with the previous implementation:

  1. Boot-ordering fragility: /proc/irq/X/smp_affinity_list is reset when the kernel calls request_irq(), which happens in ndo_open() every time the interface is brought up. To be effective the oneshot therefore had to run after every managed interface's first ndo_open() at boot: an ordering that is hard to guarantee across networkd/NetworkManager/ifupdown. If it ran too early, its writes were discarded when the interface came up.

  2. Driver reload not covered: modprobe -r / modprobe causes the device to disappear and reappear. The oneshot service was never re-triggered in these cases.

  3. Race with irqbalance: the service ran After=irqbalance.service but irqbalance is a daemon that continuously rebalances IRQs (every ~10s), overwriting the affinity within seconds.

  4. Wrong script location: the Python script was deployed to /etc/systemd/system/, mixing executables into the systemd unit directory.

  5. Unnecessary complexity: the Python script did nothing more than iterate /proc/irq/ and write to smp_affinity_list, a few lines of shell.

Replace with a long-running daemon based on ip monitor link. This monitors RTM_NEWLINK messages via the kernel netlink socket, which is independent of the network manager in use (networkd, NetworkManager, ifupdown all produce the same netlink events). The UP event is delivered after ndo_open() completes, so IRQs are registered in /proc/irq/ with their default affinity when we apply the configured affinity: correct by construction, regardless of boot ordering.

The daemon also applies affinity at startup for interfaces already up, covering manual service restarts. With Restart=always it survives crashes and re-applies on driver reload or an ethtool channel/ring change.

Note: carrier events (cable unplug/replug) and a network-manager restart do not re-open the device, so they do not reset the affinity and need no handling. The reset only occurs on an actual ndo_open() (first boot bring-up, admin down/up, driver reload, ethtool channel/ring change).

Legacy service and files are cleaned up on existing hosts.

@eroussy

eroussy commented Jul 24, 2026

Copy link
Copy Markdown
Member

I don't like this solution. Running a script every second to handle IRQs affinity will absolutely create hazardous RT problems

The script indeed conflicts with irqbalance, but Tuned documentation explicitly tell to disable the irqbalance service so it should not run on SEAPATH.
Kernel parameters added by tuned (like isolcpus=managed_irq,domain,4-32 and irqaffinity=0,1,2,3 already do the job)

That said, it is true that the current service doesn't account for modprobe or for specific kernel events. But this shouldn't happen.
On a correctly configured system, all of this should be handled by Ansible, and the service should be restarted when necessary.

I would advise you to

  • disable irqbalance
  • keep the current script fixing the minor issues you point out, like path issues or unnecessary complexity

@eroussy eroussy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment

@eroussy

eroussy commented Jul 24, 2026

Copy link
Copy Markdown
Member

After consideration, I would even advise to remove irqbalance from SEAPATH images entirely

@insatomcat

insatomcat commented Jul 24, 2026

Copy link
Copy Markdown
Member Author

Thanks for the review. I think there's a misunderstanding on the "every second" part.

The daemon does not poll every second. The only "1s" in the PR is RestartSec=1 in the unit, that's the restart delay if the daemon crashes, not a polling interval. The script itself blocks on ip monitor link, which sleeps on the netlink socket and only wakes up on an actual RTM_NEWLINK event (interface up/down, driver reload). At steady state it's 0% CPU and does zero writes, so there's no new RT hazard compared to the old oneshot; it just re-applies at the right moment instead of once at boot.

On irqbalance: fully agreed. We're disabling it (and removing it from the images) in #985, so that conflict goes away as you suggest — it's already planned.

On the kernel params: irqaffinity= / isolcpus=managed_irq set a global default for all IRQs. This role exists to give per-NIC affinity (pinning a specific interface's queues to a specific housekeeping CPU), which the global mask can't express. So that argument is really about whether the role should exist at all, not about this PR — which only changes the mechanism (oneshot → monitor), not the intent.

The core reason for the change is problem #1, and it's not an edge case: /proc/irq/X/smp_affinity_list is reset by request_irq() in ndo_open(), i.e. every time the interface is brought up on the normal path. The oneshot runs at boot before interfaces are up, so its writes are always overwritten once networkd/NM/ifupdown activates them. Restarting the service via Ansible doesn't help unless it runs after every up event, which is exactly what the monitor does. The modprobe/driver-reload case is a bonus, not the main motivation.

@insatomcat
insatomcat requested a review from eroussy July 24, 2026 12:09
@eroussy

eroussy commented Jul 24, 2026

Copy link
Copy Markdown
Member

Indeed, I missunderstood the Restart=always config.

I understand the problem now.

Out of curiosity, have you tried the tuned irq plugin ?
I'm not sure to understand if it dynamically applies the affinity at interface being up or not though. Curious to have your opinion on that

@eroussy

eroussy commented Jul 24, 2026

Copy link
Copy Markdown
Member

Also, I checked and only putting the interface manually down then up resets the affinity
Restarting systemd-networkd doesn't affect that.

What SEAPATH concrete behavior would lead for ifupdown to be used ? Seems like a manual intervention to me.

Sorry to be a bit picky, but I try to get the big picture here so that we can correct that the right way

@eroussy

eroussy commented Jul 24, 2026

Copy link
Copy Markdown
Member

And last idea that I have in mind, If we really have to merge a script like that in SEAPATH, I would advise for python instead of bash for readability and maintainability
What do you think ?

@insatomcat

insatomcat commented Jul 24, 2026

Copy link
Copy Markdown
Member Author

Good question, I hadn't set up the tuned IRQ handling for this specifically, so take the following with a grain of salt, it's my current understanding rather than something I've benchmarked head-to-head.

From what I can tell, tuned doesn't have a dedicated "irq" plugin that dynamically re-pins a given NIC's queues when the interface comes up. IRQ affinity is handled by the scheduler plugin (isolated_cores / default_irq_smp_affinity), and as far as I understand it applies statically at profile-apply time rather than reacting to link-up events. If that's correct, it would hit the same limitation as the old oneshot: request_irq() in ndo_open() resets the affinity when the interface is brought up after the profile was applied. So it likely wouldn't cover problem #1 on its own, though I'd be happy to be proven wrong if you've seen it behave differently.

There's also a bit of tension with #985, where we're deliberately making tuned as inert as possible at runtime on a live node (disabling the scheduler, net and irqbalance plugins, since their apply-time actions were breaking Corosync/quorum). Moving IRQ management back into a tuned plugin would partly reintroduce the runtime tuned actions we're trying to remove there.

That said, I want to be clear I'm not against reusing tuned in principle, and to your earlier suggestion, keeping/fixing the existing IRQ role is a perfectly safe, working and low-effort option too. The standalone monitor mostly appealed to me because it decouples the per-NIC affinity from tuned's apply/rollback cycle, which lines up with the direction of #985. But if you'd prefer to explore the tuned plugin route, I'm open to it, happy to test it if you think it can re-apply on link-up.

@insatomcat

insatomcat commented Jul 24, 2026

Copy link
Copy Markdown
Member Author

On the down/up point, you're right, and thanks for actually testing it. Carrier loss and a systemd-networkd restart don't re-open the device, so they don't reset the affinity; it really takes an admin down/up, a driver reload, or an ethtool -L/-G (channel/ring change) to trigger ndo_open() again. So I'll walk back the "networkd overwrites it" framing in the commit message, that was too strong.

The remaining justification I'd stand behind is ordering rather than frequency: with the oneshot you have to guarantee it runs after every managed interface's first ndo_open() at boot, across networkd/NM/ifupdown. That ordering is exactly what was fragile. The monitor is correct by construction, it applies on the actual UP event, whatever brought the interface up, and as a bonus it also covers the driver-reload / ethtool cases without needing an Ansible re-run. It's defense-in-depth more than "the system is broken without it", agreed.

On bash vs python: this was a deliberate switch, not an oversight. The daemon is essentially a thin loop around ip monitor link, read a line, match the UP flag, write smp_affinity_list. That's precisely what a shell read-loop expresses directly. A python rewrite would either shell out to ip monitor and do the exact same text parsing in another language (no real readability gain), or pull in pyroute2 to talk netlink directly, a new runtime dependency for what is a handful of lines. And since this is an always-on daemon on an RT hypervisor, a resident sh blocked on read() is a lighter, simpler footprint than a resident python interpreter. That's also the spirit of point #5: the original python wasn't dropped out of dogma, it was a boot-time /proc/irq iterator that a few lines of shell express more directly, and the new event loop is the same story.

Happy to add comments to the parsing bit if the ip monitor handling reads as too terse, that's the one part I'd agree isn't self-evident.

Problems with the previous implementation:

1. Boot-ordering fragility: /proc/irq/X/smp_affinity_list is reset when the
   kernel calls request_irq(), which happens in ndo_open() every time the
   interface is brought up. To be effective the oneshot therefore had to run
   *after* every managed interface's first ndo_open() at boot — an ordering
   that is hard to guarantee across networkd/NetworkManager/ifupdown. If it
   ran too early, its writes were discarded when the interface came up.

2. Driver reload not covered: modprobe -r / modprobe causes the device to
   disappear and reappear. The oneshot service was never re-triggered in
   these cases.

3. Race with irqbalance: the service ran After=irqbalance.service but
   irqbalance is a daemon that continuously rebalances IRQs (every ~10s),
   overwriting the affinity within seconds.

4. Wrong script location: the Python script was deployed to
   /etc/systemd/system/, mixing executables into the systemd unit directory.

5. Unnecessary complexity: the Python script did nothing more than iterate
   /proc/irq/ and write to smp_affinity_list — a few lines of shell.

Replace with a long-running daemon based on ip monitor link. This monitors
RTM_NEWLINK messages via the kernel netlink socket, which is independent of
the network manager in use (networkd, NetworkManager, ifupdown all produce
the same netlink events). The UP event is delivered after ndo_open()
completes, so IRQs are registered in /proc/irq/ with their default affinity
when we apply the configured affinity — correct by construction, regardless
of boot ordering.

The daemon also applies affinity at startup for interfaces already up,
covering manual service restarts. With Restart=always it survives crashes
and re-applies on driver reload or an ethtool channel/ring change.

Legacy service and files are cleaned up on existing hosts.

Signed-off-by: Florent Carli <florent.carli@rte-france.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants