ethtool: add RX flow steering (ntuple) bindings - #1206
Open
aarcamp wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughAdds Linux ethtool RX flow steering support. The change defines typed matchers, architecture-specific RXNFC serialization, validation, ioctl operations, rule listing, and Linux tests. ChangesLinux RX flow steering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant NetDevRxFlowInsert
participant UDPsocket
participant Linuxkernel
Caller->>NetDevRxFlowInsert: submit device and flow
NetDevRxFlowInsert->>NetDevRxFlowInsert: validate and serialize matcher
NetDevRxFlowInsert->>UDPsocket: open control socket
NetDevRxFlowInsert->>Linuxkernel: SIOCETHTOOL insert request
Linuxkernel-->>NetDevRxFlowInsert: selected rule location or error
NetDevRxFlowInsert-->>Caller: location or error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Add bindings for ethtool RX flow classification rules, used to steer a matching flow onto a specific RX queue. RX flow steering is exposed only through the SIOCETHTOOL ioctl; ethtool netlink does not provide messages for these operations. Add: - NetDevRxFlowInsert using ETHTOOL_SRXCLSRLINS - NetDevRxFlowDelete using ETHTOOL_SRXCLSRLDEL - NetDevRxFlowList using ETHTOOL_GRXCLSRLCNT and ETHTOOL_GRXCLSRLALL Typed matchers support ETHER_FLOW, TCP_V4_FLOW, and UDP_V4_FLOW. They serialize match values and masks into the 52-byte ethtool flow union, encoding ports, addresses, and EtherTypes in network byte order where required by the UAPI. Represent ethtool_rxnfc logically and serialize it explicitly into the native UAPI layout. Use the 8-byte-aligned, 192-byte layout on supported Linux architectures except 386, and the 4-byte-aligned, 180-byte layout on 386. Decode ioctl responses using the same layout, including the variable-length rule location array. Validate matcher values and interface names before issuing the ioctl. Tests cover both ABI layouts, golden-byte serialization for TCP4 and Ether matchers, rule-location parsing, malformed inputs, and interface name validation. A privileged test against a device without ntuple support confirms that a well-formed request reaches the driver and returns EOPNOTSUPP. A full insert/list/delete round trip requires a set_rxnfc-capable device because netdevsim does not implement rxnfc. Signed-off-by: Aaron Campbell <aaron@monkey.org>
aarcamp
force-pushed
the
pr/ac/ethtool-ntuple-steering
branch
from
August 4, 2026 01:20
51004b4 to
4c62d0f
Compare
aarcamp
marked this pull request as ready for review
August 4, 2026 12:58
![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
🧹 Nitpick comments (2)
ethtool_ntuple_linux.go (2)
148-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the two offset bases in
ethtoolRxnfcLayout.
ringCookieOffsetandlocationOffsetare relative to the start offs.ruleCntOrRssCtxOffsetandruleLocsOffsetare relative to the start of the wholeethtool_rxnfcbuffer. The values are correct, but the mixed bases are implicit. A new field added with the wrong base would produce a silently malformed ioctl payload.♻️ Suggested documentation, or rename the fields to encode the base
type ethtoolRxnfcLayout struct { - size int - ringCookieOffset int - locationOffset int - ruleCntOrRssCtxOffset int - ruleLocsOffset int + // size is the total size of struct ethtool_rxnfc. + size int + // ringCookieOffset and locationOffset are relative to the start of + // the embedded struct ethtool_rx_flow_spec (ethtoolRxnfcFlowSpecOffset). + ringCookieOffset int + locationOffset int + // ruleCntOrRssCtxOffset and ruleLocsOffset are relative to the start of + // struct ethtool_rxnfc. + ruleCntOrRssCtxOffset int + ruleLocsOffset int }🤖 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 `@ethtool_ntuple_linux.go` around lines 148 - 154, Document the offset bases in ethtoolRxnfcLayout: clarify that ringCookieOffset and locationOffset are relative to the start of fs, while ruleCntOrRssCtxOffset and ruleLocsOffset are relative to the complete ethtool_rxnfc buffer. Keep the existing values and layout unchanged.
374-397: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake
NetDevRxFlowListtolerate a rule count that grows between the two ioctls.The count comes from
ETHTOOL_GRXCLSRLCNTand the buffer is sized from it. If another process inserts a rule beforeETHTOOL_GRXCLSRLALLruns, the driver either returnsEMSGSIZEor reports a largerrule_cnt, and the call fails. Add a bounded retry so a concurrent insert does not turn a read into a hard error.♻️ Suggested bounded retry
- layout := nativeEthtoolRxnfcLayout() - nfc := ethtoolRxnfc{ - cmd: ETHTOOL_GRXCLSRLALL, - ruleCntOrRssCtx: n, - } - buf, err := serializeEthtoolRxnfc(&nfc, layout, n) - if err != nil { - return nil, err - } - if err := ethtoolIoctl(dev, unsafe.Pointer(&buf[0])); err != nil { - return nil, err - } - return parseNetDevRxFlowLocations(buf, layout, n) + layout := nativeEthtoolRxnfcLayout() + var lastErr error + for attempt := 0; attempt < 3; attempt++ { + nfc := ethtoolRxnfc{ + cmd: ETHTOOL_GRXCLSRLALL, + ruleCntOrRssCtx: n, + } + buf, err := serializeEthtoolRxnfc(&nfc, layout, n) + if err != nil { + return nil, err + } + if err := ethtoolIoctl(dev, unsafe.Pointer(&buf[0])); err != nil { + if !errors.Is(err, unix.EMSGSIZE) { + return nil, err + } + lastErr = err + n *= 2 + continue + } + locs, err := parseNetDevRxFlowLocations(buf, layout, n) + if err == nil { + return locs, nil + } + lastErr = err + n *= 2 + } + return nil, lastErrThis diff needs
errorsin the import block.🤖 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 `@ethtool_ntuple_linux.go` around lines 374 - 397, Update NetDevRxFlowList to retry the count-and-list ioctl sequence when ETHTOOL_GRXCLSRLALL reports EMSGSIZE or a larger rule_cnt, reusing the refreshed count to resize the buffer. Add the errors import needed for matching the ioctl error, bound the retries, and preserve existing error returns for other failures.
🤖 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 `@ethtool_ntuple_linux.go`:
- Around line 419-452: Update serializeEthtoolRxnfc and deserializeEthtoolRxnfc
to use encoding/binary.NativeEndian for native-order fields and
encoding/binary.BigEndian for network-order fields, replacing the undeclared
native and networkOrder identifiers. Add or reuse the required encoding/binary
import without changing the payload layout or ioctl flow.
---
Nitpick comments:
In `@ethtool_ntuple_linux.go`:
- Around line 148-154: Document the offset bases in ethtoolRxnfcLayout: clarify
that ringCookieOffset and locationOffset are relative to the start of fs, while
ruleCntOrRssCtxOffset and ruleLocsOffset are relative to the complete
ethtool_rxnfc buffer. Keep the existing values and layout unchanged.
- Around line 374-397: Update NetDevRxFlowList to retry the count-and-list ioctl
sequence when ETHTOOL_GRXCLSRLALL reports EMSGSIZE or a larger rule_cnt, reusing
the refreshed count to resize the buffer. Add the errors import needed for
matching the ioctl error, bound the retries, and preserve existing error returns
for other failures.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 08dad887-1dc7-4597-afa3-47312ffaef97
📒 Files selected for processing (2)
ethtool_ntuple_linux.goethtool_ntuple_linux_test.go
Refactored offset handling to render this feedback moot.
Declining this suggestion. NetDevRxFlowList intentionally matches ethtool’s two-ioctl behavior: if the table grows between calls, it returns the kernel error. A bounded retry would reduce, but not eliminate, the race. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add bindings for ethtool RX flow classification rules, used to steer a matching flow onto a specific RX queue. RX flow steering is exposed only through the SIOCETHTOOL ioctl; ethtool netlink does not provide messages for these operations.
Add:
Typed matchers support ETHER_FLOW, TCP_V4_FLOW, and UDP_V4_FLOW. They serialize match values and masks into the 52-byte ethtool flow union, encoding ports, addresses, and EtherTypes in network byte order where required by the UAPI.
Represent ethtool_rxnfc logically and serialize it explicitly into the native UAPI layout. Use the 8-byte-aligned, 192-byte layout on supported Linux architectures except 386, and the 4-byte-aligned, 180-byte layout on 386. Decode ioctl responses using the same layout, including the variable-length rule location array.
Validate matcher values and interface names before issuing the ioctl.
Tests cover both ABI layouts, golden-byte serialization for TCP4 and Ether matchers, rule-location parsing, malformed inputs, and interface name validation. A privileged test against a device without ntuple support confirms that a well-formed request reaches the driver and returns EOPNOTSUPP. A full insert/list/delete round trip requires a set_rxnfc-capable device because netdevsim does not implement rxnfc.
These changes will help Cilium take advantage of the zero-copy networking features added in Linux 7.1—see the upstream queue-leasing merge.
Hardware validation: Intel ICE native and 32-bit compat ioctl paths
I tested this PR on an Intel ICE PF after upgrading a server of mine to Ubuntu's Linux 7.0 HWE kernel.
Environment
7.0.0-28-genericx86_64go1.25.0 linux/amd64enp129s0f0np0ice, kernel version7.0.0-28-generic4.40 0x8001ce8e 0.387.490000:81:00.0ntuple-filters: onThe source for the standalone program used for the test is attached as
netlink-pr1206-hwe7-smoke.tgz.What the standalone program checks
For each matcher, the program:
NetDevRxFlowInsertwith queue 1 and location 100.NetDevRxFlowListand checks that location 100 is present.NetDevRxFlowDelete.NetDevRxFlowListagain and checks that the complete location set matches the initial set.It refuses to run if location 100 is already occupied, uses deferred best-effort deletion if a case fails after insertion, continues through all matcher cases to report every result, performs a final location-set comparison, and exits nonzero if any case fails.
The four rules tested were:
0xffff0xffff02:00:00:12:06:01, maskff:ff:ff:ff:ff:ff0x88b5, mask0xffffI also ran
ethtool -u enp129s0f0np0immediately before and after each complete binary run to independently verify that the driver started and ended with zero rules.Build and execution
Native amd64:
Linux i386 compat ABI:
fileconfirmed that the compat test was an actual statically linked 32-bit executable:Native amd64 result
Linux i386 compat result
This exercises the native 192-byte
ethtool_rxnfcrepresentation and, through the 32-bit executable and kernel compat ioctl path, the i386 180-byte representation implemented by this PR.Existing privileged loopback test
I separately ran the PR's existing
TestRxFlowInsertReachesDriver. That test creates a fresh network namespace and deliberately uses loopback, so its expected outcome isEOPNOTSUPP.Scope and limitations
This validates the complete ioctl control plane against the real ICE driver: insertion, returned location, listing, deletion, final cleanup, native ABI, and i386 compat ABI. No matching packets were transmitted, so this does not independently validate data-plane delivery to queue 1. That would require a traffic-generating peer and checking per-queue packet counters.
The final ICE rule table contained zero rules, no relevant ICE/Flow Director errors appeared in the kernel log, and all temporary remote checkouts and binaries were removed.
netlink-pr1206-hwe7-smoke.tgz
Summary by CodeRabbit
New Features
Bug Fixes