vdpa: support vdpa dev set mac #1141
Conversation
WalkthroughAdds a new VDPA netlink command constant and implements VDPASetAttr APIs (package-level and Handle method) to set device attributes via netlink; includes tests for MAC/parameter handling and related error cases. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant API as VDPASetAttr (pkg)
participant H as Handle.VDPASetAttr
participant Req as vdpaRequest
participant NL as Netlink
User->>API: VDPASetAttr(name, params)
API->>H: delegate to pkgHandle.VDPASetAttr
H->>H: validate params (must not be empty)
H->>H: build attributes (MACAddr, MaxVQP, MTU, Features)
H->>H: append device name attribute
H->>Req: send VDPA_CMD_DEV_ATTR_SET request
Req->>NL: netlink message out
NL-->>Req: response (success / error)
Req-->>H: return result
H-->>API: return error (if any)
API-->>User: return error (if any)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (1)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
nl/vdpa_linux.go(1 hunks)vdpa_linux.go(2 hunks)vdpa_linux_test.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
vdpa_linux.go (2)
nl/nl_linux.go (5)
RtAttr(395-399)NewRtAttr(402-410)Uint16Attr(1013-1018)Uint64Attr(1039-1044)ZeroTerminated(987-994)nl/vdpa_linux.go (6)
VDPA_ATTR_DEV_NET_CFG_MACADDR(31-31)VDPA_ATTR_DEV_NET_CFG_MAX_VQP(33-33)VDPA_ATTR_DEV_NET_CFG_MTU(34-34)VDPA_ATTR_DEV_FEATURES(41-41)VDPA_ATTR_DEV_NAME(25-25)VDPA_CMD_DEV_ATTR_SET(17-17)
vdpa_linux_test.go (2)
nl/vdpa_linux.go (3)
VDPA_CMD_DEV_GET(14-14)VDPA_CMD_DEV_CONFIG_GET(15-15)VDPA_CMD_DEV_ATTR_SET(17-17)vdpa_linux.go (4)
VDPANewDevParams(83-88)VDPASetAttr(171-173)VDPAGetDevConfigByName(146-148)VDPANewDev(110-112)
🔇 Additional comments (8)
nl/vdpa_linux.go (1)
17-17: LGTM!The new command constant is correctly placed in the enumeration sequence and follows the established naming convention.
vdpa_linux.go (2)
171-173: LGTM!The package-level function correctly delegates to the handle, following the established pattern in this codebase.
500-519: LGTM!The attribute building logic is well-structured:
- Conditionally adds each parameter when provided
- Validates that at least one parameter is set before making the request
- Appends the device name attribute
- Properly encodes values using nl helper functions
vdpa_linux_test.go (5)
5-5: LGTM!The
netpackage import is necessary for MAC address parsing in the new tests.
226-249: LGTM!The test comprehensively validates MAC address setting:
- Creates device with initial MAC
- Sets a new MAC address
- Verifies the change by reading back the configuration
251-272: LGTM!The error handling tests properly validate:
- Non-existent device returns
ENODEV- Empty parameters are rejected with an error
274-306: LGTM!These tests validate that MTU, MaxVQP, and Features currently return
ENOTSUPfrom the kernel, consistent with the PR description noting that kernel 6.12 only supports MAC address setting. The tests document the current behavior and will help identify when kernel support expands.
322-324: LGTM!The helper function provides a clean way to create test devices with specific parameters, following the pattern of the existing
createVDPATestDevfunction.
The Linux kernel supports setting mac addresses to existing vdpa devices since 6.12. The feature was introduced by commit 2f87e9cf. This commit brings support for that feature to the netlink library by introducing the VDPASetAttr function. Note that even though setting the mac address is the only supported operation currently, the implementation has been left open for MTU, MaxVQP and Feature setting. In other words, it relies on whether the kernel supports it or not, and just feeds the kernel's error back to the caller. That way, no further changes will be needed in the future, if support for those operations is implemented. Well, apart from tests, as they currently assume those are not supported. Signed-off-by: Beñat Gartzia Arruabarrena <[email protected]>
|
Fixed the issue pointed out by coderabbit as I was setting |
1 participant
The Linux kernel supports setting mac addresses to existing vdpa devices since 6.12. The feature was introduced by commit 2f87e9cf.
This commit brings support for that feature to the netlink library by introducing the
VDPASetAttrfunction.Note that even though setting the mac address is the only supported operation currently, the implementation has been left open for MTU, MaxVQP and Feature setting. In other words, it relies on whether the kernel supports it or not, and just feeds the kernel's error back to the caller. That way, no further changes will be needed in the future, if support for those operations is implemented.
Well... apart from tests, as they currently assume those are not supported.
This is implements a iproute's
vdpa dev set name <name> mac <mac>equivalent.Summary by CodeRabbit