-
Notifications
You must be signed in to change notification settings - Fork 405
interfaces: T7730: Add interrupt coalescing settings for Ethernet interfaces #4919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: current
Are you sure you want to change the base?
Conversation
|
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds CLI support for configuring network interface interrupt coalescing parameters via ethtool --coalesce. This allows users to tune interrupt moderation settings including adaptive modes (adaptive-rx, adaptive-tx) and static delay values (rx-usecs, tx-usecs) for better network performance optimization.
Key Changes:
- Adds new CLI configuration nodes under
interfaces ethernet <name> coalescefor adaptive-rx, adaptive-tx, rx-usecs, and tx-usecs - Implements verification logic to ensure NIC driver support before applying settings
- Includes comprehensive smoketests to validate functionality across different NIC types
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| interface-definitions/interfaces_ethernet.xml.in | Defines new CLI configuration nodes for coalesce settings with appropriate constraints (0-16384 range for usecs values) |
| python/vyos/ethtool.py | Adds coalesce parsing, storage, and accessor methods (check_coalesce, get_coalesce) to the Ethtool class |
| python/vyos/ifconfig/ethernet.py | Implements set_coalesce method to apply settings via ethtool command and integrates it into the update flow |
| src/conf_mode/interfaces_ethernet.py | Adds verify_coalesce function to validate driver support and parameter compatibility before committing |
| smoketest/scripts/cli/test_interfaces_ethernet.py | Provides comprehensive test coverage for both manual and adaptive coalesce configuration modes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9ab239b to
018c417
Compare
zdc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the code-related feedback, there is an important functional consideration.
Exposing only rx/tx-usecs is not practical because disabling adaptive mode activates all settings - not just the usecs values. This means we effectively switch to a "manual" configuration mode, yet we only grant users access to a subset of the controls they actually need to use this mode effectively.
Therefore, if we proceed with this approach, all coalesce settings should be introduced simultaneously.
018c417 to
950fc79
Compare
|
@zdc I have pushed an updated version of the implementation which contains all the available settings from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adds the ethtool coalesce configuration feature for an interface
(checked only working options, other options not allowed on my virtio device, needs to check HELP values if we ok with them)
vyos@r14# run show conf com | match coa
set interfaces ethernet eth1 coalesce rx-frames '23'
[edit]
vyos@r14# sudo ethtool --show-coalesce eth1
Coalesce parameters for eth1:
Adaptive RX: n/a TX: n/a
stats-block-usecs: n/a
sample-interval: n/a
pkt-rate-low: n/a
pkt-rate-high: n/a
rx-usecs: n/a
rx-frames: 23
rx-usecs-irq: n/a
rx-frames-irq: n/a
tx-usecs: n/a
tx-frames: n/a
tx-usecs-irq: n/a
tx-frames-irq: n/a
rx-usecs-low: n/a
rx-frame-low: n/a
tx-usecs-low: n/a
tx-frame-low: n/a
rx-usecs-high: n/a
rx-frame-high: n/a
tx-usecs-high: n/a
tx-frame-high: n/a
CQE mode RX: n/a TX: n/a
tx-aggr-max-bytes: n/a
tx-aggr-max-frames: n/a
tx-aggr-time-usecs: n/a
[edit]
vyos@r14#
[edit]
vyos@r14# set interfaces ethernet eth1 coalesce
Possible completions:
adaptive-rx Enable adaptive receive interrupt moderation
adaptive-tx Enable adaptive transmit interrupt moderation
cqe-mode-rx Enable RX CQE (Completion Queue Entry) mode
cqe-mode-tx Enable TX CQE (Completion Queue Entry) mode
pkt-rate-high Upper packet rate threshold for adaptive coalescing
pkt-rate-low Lower packet rate threshold for adaptive coalescing
rx-frames Number of RX frames before generating interrupt
rx-frames-high RX coalescing frames threshold for high packet rate
rx-frames-irq Number of RX frames before generating interrupt while servicing
IRQ
rx-frames-low RX coalescing frames threshold for low packet rate
rx-usecs Delay in microseconds before generating RX interrupt
rx-usecs-high RX coalescing delay (usecs) for high packet rate
rx-usecs-irq Delay in microseconds before generating RX interrupt while
servicing IRQ
rx-usecs-low RX coalescing delay (usecs) for low packet rate
sample-interval Sampling interval for adaptive coalescing (in seconds)
stats-block-usecs Time in microseconds between updating coalescing statistics
tx-aggr-max-bytes Maximum number of bytes to aggregate before transmitting
tx-aggr-max-frames Maximum number of frames to aggregate before transmitting
tx-aggr-time-usecs Maximum time in microseconds to wait before transmitting
aggregated frames
tx-frames Number of TX frames before generating interrupt
tx-frames-high TX coalescing frames threshold for high packet rate
tx-frames-irq Number of TX frames before generating interrupt while servicing
IRQ
tx-frames-low TX coalescing frames threshold for low packet rate
tx-usecs Delay in microseconds before generating TX interrupt
tx-usecs-high TX coalescing delay (usecs) for high packet rate
tx-usecs-irq Delay in microseconds before generating TX interrupt while
servicing IRQ
tx-usecs-low TX coalescing delay (usecs) for low packet rate
[edit]
vyos@r14#
950fc79 to
c6a8b6d
Compare
c-po
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All requested changes addressed. Code looks good and clean.
Verified by smoketests which got extended for this new feature.
|
Re-requested review from @zdc |
dmbaturin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic looks good to me. I have aesthetic comments, though.
First, I don't like that the same thing is sometimes called "coalescing" and sometimes "moderation" in help strings — they are different terms for the same thing and generally people understand them both, but it may be better to stick with one term consistently. I don't have a strong opinion which one to use, we should discuss that; or toss a coin if nothing else.
I also wonder if the node name should be something like interrupt-moderation/interrupt-coalescing to make it clear to people less experienced with ethtool what exactly the option is about.
|
It's already difficult for me to think of a new name, because I'm used to using |
dmbaturin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a discussion we settled on "coalescing" and @alexandr-san4ez agreed to change all instances of "moderation" to "coalescing".
c6a8b6d to
1ecfceb
Compare
1ecfceb to
d43afeb
Compare
d43afeb to
51b81a2
Compare
dmbaturin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestions about option names are addressed now, and the logic was always fine.
|
@zdc provided me example of using |
51b81a2 to
8ad8666
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8ad8666 to
38b52b2
Compare
This change introduces CLI support for configuring network interface interrupt coalescing parameters via `netlink`. Note: Not all NIC drivers support interrupt coalescing. On unsupported interfaces, `netlink` returns an error and the VyOS commit will fail.
38b52b2 to
f01ff15
Compare
|
CI integration 👍 passed! Details
|
Change summary
This change introduces CLI support for configuring network interface interrupt coalescing parameters (
ethtool --coalesce).Types of changes
Related Task(s)
Related PR(s)
How to test / Smoketest result
Manual test:
Smoketest:
Checklist: