set: fix byte order metadata for host-endian types and map data#353
Closed
anthonyrisinger wants to merge 1 commit intogoogle:mainfrom
Closed
set: fix byte order metadata for host-endian types and map data#353anthonyrisinger wants to merge 1 commit intogoogle:mainfrom
anthonyrisinger wants to merge 1 commit intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
70c670c to
5ef1e8e
Compare
Fix two bugs in the userdata byte order metadata emitted by AddSet: 1. KEYBYTEORDER override: Anonymous, constant, and interval sets unconditionally emitted KEYBYTEORDER=2 (big-endian), ignoring the actual key type. Host-endian types like mark and ifname need KEYBYTEORDER=1 for correct display in `nft list ruleset` on little-endian systems. The nft C tool always uses the key expression's actual byte order (mnl.c:mnl_nft_set_add), not a blanket big-endian default. 2. Missing DATABYTEORDER: The library never emitted NFTNL_UDATA_SET_DATABYTEORDER for maps despite the constant existing in userdata.go. Without it, `nft list ruleset` cannot determine the data byte order, displaying host-endian values (like marks) as byte-swapped on little-endian systems. Add a DataByteOrder field to the Set struct and simplify the KEYBYTEORDER logic to a switch statement where explicit KeyByteOrder=NativeEndian always takes precedence. Add TestSetByteOrder with 6 subtests covering anonymous/named sets and maps with various byte order combinations.
5ef1e8e to
4c01b2f
Compare
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
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.
Fix two issues with set/map userdata byte order metadata that cause
nft list rulesetto misinterpret element keys and data values on little-endian systems. The kernel-level data is always correct — these are display/round-trip issues affecting the userdata TLVs that thenftCLI uses to reconstruct human-readable output.Bug 1: Anonymous sets ignore explicit
KeyByteOrderAnonymous/constant/interval sets unconditionally emit
KEYBYTEORDER=2(big-endian), ignoring theKeyByteOrderfield. ThenftC tool always uses the key expression's actual byte order (mnl.c:mnl_nft_set_add), not a blanket default.For host-endian types like
mark_type(BYTEORDER_HOST_ENDIAN) andifname_type(BYTEORDER_HOST_ENDIAN), this causesnft list rulesetto misinterpret the element data on LE systems:meta mark { 0x00000000, 0x01000000 }instead of{ 0x00000000, 0x00000001 }iifname vmap { "" : jump chain1 }instead of{ "eth0" : jump chain1 }The existing comment "Semantically useless - kept for binary compatability with nft" is incorrect —
KEYBYTEORDERIS semantically meaningful for host-endian types. ThenftC tool uses it innetlink_delinearize_setelemto decide whether to callmpz_switch_byteorderon element keys.Fix: When the anonymous/constant/interval condition fires, check if
KeyByteOrderis explicitly set toNativeEndianand emit1(host) instead of2(big). Falls back to the original big-endian default whenKeyByteOrderis unset (zero value), preserving backwards compatibility.Bug 2: Maps never emit
DATABYTEORDERThe
NFTNL_UDATA_SET_DATABYTEORDERconstant exists inuserdata.gobut is never used. TheSetstruct has noDataByteOrderfield, and the userdata construction has no code path that emits this TLV.The
nftC tool emitsDATABYTEORDERfor all datamaps:Without it,
nft list rulesetusesBYTEORDER_INVALIDfor map data, skips thempz_switch_byteordercall, and displays native-endian values as byte-swapped on LE systems:Fix: Add
DataByteOrderfield toSetstruct and emitNFTNL_UDATA_SET_DATABYTEORDERin the userdata construction for maps when set.Discovery context
Found while debugging a WireGuard mesh overlay that uses
meta marksets and maps for zone-based connection latching. The incorrect display made it extremely difficult to diagnose whether mark values were being set correctly, sincenft list rulesetshowed byte-swapped values but the kernel data was actually correct (verified vianft --debug=netlink list ruleset).Testing
Verified on linux/arm64 (little-endian) that after this patch:
TypeMarksets display correct values (0x00000001not0x01000000)TypeMarkdisplays correct valuesnft --debug=netlink list rulesetshows identical kernel-level bytes before and after (no functional change)GOOS=linux go build .andGOOS=linux go vet .pass cleanly