set: fix byte order metadata for host-endian types and map data#355
set: fix byte order metadata for host-endian types and map data#355anthonyrisinger wants to merge 1 commit intogoogle:mainfrom
Conversation
|
Company decided they wanted the PR from here—apologies for the noise from the last two closed!—I couldn't figure out how to open #353 again; it was the same exact commit on all three. |
There was a problem hiding this comment.
I took a look through this and left some suggestions / personal preferences. @stapelberg I don't mean to step on your toes. Just thought those comments might be helpful.
9f528f2 to
67d59ef
Compare
|
Let me know if there's any other changes—I simplify tests with a round-trip through both encode/decode paths. |
Add DataByteOrder to Set for tracking map data byte order. Emit NFTNL_UDATA_SET_DATABYTEORDER in AddSet for maps and parse both KEYBYTEORDER and DATABYTEORDER from userdata in setsFromMsg. Also fix KEYBYTEORDER emission: check NativeEndian before the anonymous/constant/interval/BigEndian catch-all so that sets with an explicit NativeEndian key order emit the correct value. Without these fixes, host-endian map data (e.g. marks) appeared byte-swapped when read back on little-endian systems, and neither KeyByteOrder nor DataByteOrder was populated when deserializing sets.
67d59ef to
8da11b2
Compare
|
Should be all set! Validated with a Docker container: $ docker run --rm --privileged -v ~/devel/nftables:/work -w /work golang:1.24-bookworm bash -lc '/usr/local/go/bin/go test -count=5 -run "^TestSetByteOrderRoundTrip$" -run_system_tests .'
go: downloading github.com/mdlayher/netlink v1.8.1-0.20251028132421-dcc6cab9a6eb
go: downloading golang.org/x/sys v0.35.0
go: downloading github.com/vishvananda/netns v0.0.4
go: downloading golang.org/x/net v0.43.0
go: downloading github.com/mdlayher/socket v0.5.1
go: downloading golang.org/x/sync v0.6.0
PASS
ok github.com/google/nftables 0.022s |
|
No real rush on my end (I'm using a private fork at the moment), but is it reasonable to believe this will find its way into the next release, whenever that may be? Mostly looking to check any boxes before I file this away to the back of my mind |
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