Skip to content

Commit 67d59ef

Browse files
anthonyrisingeranthonyrisinger
committed
set: fix byte-order metadata round-trips for sets/maps
Fix two userdata metadata issues so byte-order information round-trips correctly through set serialization and deserialization. 1. Parse missing set-level byte-order userdata on read: - NFTNL_UDATA_SET_KEYBYTEORDER - NFTNL_UDATA_SET_DATABYTEORDER This mirrors what AddSet emits and ensures callers retrieving sets get the same key/data byte-order metadata back. 2. Improve byte-order test coverage with a simpler round-trip approach: - Add TestSetByteOrderRoundTrip in set_test.go. - Cover constant/anonymous/interval sets and map cases with explicit host/big-endian combinations for key/data. - Verify both set metadata decoding (setsFromMsg) and element decoding (elementsFromMsg). This keeps behavior aligned with nft/libnftnl expectations for KEYBYTEORDER and DATABYTEORDER handling while making tests easier to reason about and maintain.
1 parent 1db35da commit 67d59ef

File tree

2 files changed

+221
-6
lines changed

2 files changed

+221
-6
lines changed

‎set.go‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,9 @@ type Set struct {
267267
DataType SetDatatype
268268
// Either host (binaryutil.NativeEndian) or big (binaryutil.BigEndian) endian as per
269269
// https://git.netfilter.org/nftables/tree/include/datatype.h?id=d486c9e626405e829221b82d7355558005b26d8a#n109
270-
KeyByteOrder binaryutil.ByteOrder
271-
Comment string
270+
KeyByteOrder binaryutil.ByteOrder
271+
DataByteOrder binaryutil.ByteOrder
272+
Comment string
272273
// Indicates that the set has "size" specifier
273274
Size uint32
274275
}
@@ -716,12 +717,27 @@ func (cc *Conn) AddSet(s *Set, vals []SetElement) error {
716717
// https://git.netfilter.org/libnftnl/tree/include/udata.h#n17
717718
var userData []byte
718719

719-
if s.Anonymous || s.Constant || s.Interval || s.KeyByteOrder == binaryutil.BigEndian {
720-
// Semantically useless - kept for binary compatability with nft
721-
userData = userdata.AppendUint32(userData, userdata.NFTNL_UDATA_SET_KEYBYTEORDER, 2)
722-
} else if s.KeyByteOrder == binaryutil.NativeEndian {
720+
// Emit KEYBYTEORDER metadata matching nft C tool behavior (mnl.c:mnl_nft_set_add).
721+
// Anonymous, constant, and interval sets always need byte order metadata.
722+
// When KeyByteOrder is explicitly set, use it; otherwise default to big-endian
723+
// for backward compatibility with prior library behavior.
724+
if s.KeyByteOrder == binaryutil.NativeEndian {
723725
// Per https://git.netfilter.org/nftables/tree/src/mnl.c?id=187c6d01d35722618c2711bbc49262c286472c8f#n1165
724726
userData = userdata.AppendUint32(userData, userdata.NFTNL_UDATA_SET_KEYBYTEORDER, 1)
727+
} else if s.Anonymous || s.Constant || s.Interval || s.KeyByteOrder == binaryutil.BigEndian {
728+
userData = userdata.AppendUint32(userData, userdata.NFTNL_UDATA_SET_KEYBYTEORDER, 2)
729+
}
730+
731+
// Emit DATABYTEORDER for maps, matching nft C tool behavior (mnl.c:mnl_nft_set_add).
732+
// Without this, nft list ruleset cannot determine the data byte order and displays
733+
// host-endian values (like marks) as byte-swapped on LE systems.
734+
if s.IsMap {
735+
switch s.DataByteOrder {
736+
case binaryutil.NativeEndian:
737+
userData = userdata.AppendUint32(userData, userdata.NFTNL_UDATA_SET_DATABYTEORDER, 1)
738+
case binaryutil.BigEndian:
739+
userData = userdata.AppendUint32(userData, userdata.NFTNL_UDATA_SET_DATABYTEORDER, 2)
740+
}
725741
}
726742

727743
if s.Interval && s.AutoMerge {
@@ -862,6 +878,12 @@ func setsFromMsg(msg netlink.Message) (*Set, error) {
862878
set.DataType.Bytes = binary.BigEndian.Uint32(ad.Bytes())
863879
case unix.NFTA_SET_USERDATA:
864880
data := ad.Bytes()
881+
if val, ok := userdata.GetUint32(data, userdata.NFTNL_UDATA_SET_KEYBYTEORDER); ok {
882+
set.KeyByteOrder = parseSetByteOrder(val)
883+
}
884+
if val, ok := userdata.GetUint32(data, userdata.NFTNL_UDATA_SET_DATABYTEORDER); ok {
885+
set.DataByteOrder = parseSetByteOrder(val)
886+
}
865887
if val, ok := userdata.GetString(data, userdata.NFTNL_UDATA_SET_COMMENT); ok {
866888
set.Comment = val
867889
}
@@ -891,6 +913,17 @@ func setsFromMsg(msg netlink.Message) (*Set, error) {
891913
return &set, nil
892914
}
893915

916+
func parseSetByteOrder(v uint32) binaryutil.ByteOrder {
917+
switch v {
918+
case 1:
919+
return binaryutil.NativeEndian
920+
case 2:
921+
return binaryutil.BigEndian
922+
default:
923+
return nil
924+
}
925+
}
926+
894927
func parseSetDatatype(magic uint32) (SetDatatype, error) {
895928
types := make([]SetDatatype, 0, 32/SetConcatTypeBits)
896929
for magic != 0 {

‎set_test.go‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66
"time"
77

8+
"github.com/google/nftables/binaryutil"
89
"github.com/mdlayher/netlink"
910
)
1011

@@ -308,3 +309,184 @@ func TestMarshalSet(t *testing.T) {
308309
})
309310
}
310311
}
312+
313+
func TestSetByteOrderRoundTrip(t *testing.T) {
314+
t.Parallel()
315+
316+
tbl := &Table{
317+
Name: "ipv4table",
318+
Family: TableFamilyIPv4,
319+
}
320+
321+
c, err := New(WithTestDial(
322+
func(req []netlink.Message) ([]netlink.Message, error) {
323+
return req, nil
324+
}))
325+
if err != nil {
326+
t.Fatal(err)
327+
}
328+
329+
c.AddTable(tbl)
330+
331+
tests := []struct {
332+
name string
333+
set Set
334+
elems []SetElement
335+
wantKeyOrder binaryutil.ByteOrder
336+
wantDataOrder binaryutil.ByteOrder
337+
}{
338+
{
339+
name: "constant set defaults key byteorder to big-endian",
340+
set: Set{
341+
Name: "ports",
342+
Constant: true,
343+
KeyType: TypeInetService,
344+
},
345+
elems: []SetElement{
346+
{Key: binaryutil.BigEndian.PutUint16(80)},
347+
},
348+
wantKeyOrder: binaryutil.BigEndian,
349+
},
350+
{
351+
name: "explicit host-endian key byteorder",
352+
set: Set{
353+
Name: "marks",
354+
KeyType: TypeMark,
355+
KeyByteOrder: binaryutil.NativeEndian,
356+
},
357+
elems: []SetElement{
358+
{Key: binaryutil.NativeEndian.PutUint32(1)},
359+
},
360+
wantKeyOrder: binaryutil.NativeEndian,
361+
},
362+
{
363+
name: "anonymous set with explicit host-endian key byteorder",
364+
set: Set{
365+
Anonymous: true,
366+
Constant: true,
367+
KeyType: TypeMark,
368+
KeyByteOrder: binaryutil.NativeEndian,
369+
},
370+
elems: []SetElement{
371+
{Key: binaryutil.NativeEndian.PutUint32(1)},
372+
},
373+
wantKeyOrder: binaryutil.NativeEndian,
374+
},
375+
{
376+
name: "interval set defaults key byteorder to big-endian",
377+
set: Set{
378+
Name: "interval-ports",
379+
Interval: true,
380+
KeyType: TypeInetService,
381+
},
382+
elems: []SetElement{
383+
{Key: binaryutil.BigEndian.PutUint16(443)},
384+
},
385+
wantKeyOrder: binaryutil.BigEndian,
386+
},
387+
{
388+
name: "map with explicit host-endian key and data byteorder",
389+
set: Set{
390+
Name: "mark-map",
391+
KeyType: TypeMark,
392+
DataType: TypeMark,
393+
KeyByteOrder: binaryutil.NativeEndian,
394+
DataByteOrder: binaryutil.NativeEndian,
395+
IsMap: true,
396+
},
397+
elems: []SetElement{
398+
{
399+
Key: binaryutil.NativeEndian.PutUint32(1),
400+
Val: binaryutil.NativeEndian.PutUint32(2),
401+
},
402+
},
403+
wantKeyOrder: binaryutil.NativeEndian,
404+
wantDataOrder: binaryutil.NativeEndian,
405+
},
406+
{
407+
name: "map with explicit data byteorder only",
408+
set: Set{
409+
Name: "service-to-mark",
410+
KeyType: TypeInetService,
411+
DataType: TypeMark,
412+
DataByteOrder: binaryutil.NativeEndian,
413+
IsMap: true,
414+
},
415+
elems: []SetElement{
416+
{
417+
Key: binaryutil.BigEndian.PutUint16(22),
418+
Val: binaryutil.NativeEndian.PutUint32(1),
419+
},
420+
},
421+
wantDataOrder: binaryutil.NativeEndian,
422+
},
423+
{
424+
name: "anonymous map with default big-endian key byteorder and explicit host-endian data byteorder",
425+
set: Set{
426+
Anonymous: true,
427+
Constant: true,
428+
KeyType: TypeInetService,
429+
DataType: TypeMark,
430+
DataByteOrder: binaryutil.NativeEndian,
431+
IsMap: true,
432+
},
433+
elems: []SetElement{
434+
{
435+
Key: binaryutil.BigEndian.PutUint16(22),
436+
Val: binaryutil.NativeEndian.PutUint32(1),
437+
},
438+
},
439+
wantKeyOrder: binaryutil.BigEndian,
440+
wantDataOrder: binaryutil.NativeEndian,
441+
},
442+
}
443+
444+
for _, tt := range tests {
445+
t.Run(tt.name, func(t *testing.T) {
446+
start := len(c.messages)
447+
448+
tt.set.Table = tbl
449+
if err := c.AddSet(&tt.set, tt.elems); err != nil {
450+
t.Fatalf("AddSet() failed: %v", err)
451+
}
452+
453+
// AddSet emits one message for the set and one for initial elements.
454+
gotNewMessages := len(c.messages) - start
455+
if gotNewMessages < 2 {
456+
t.Fatalf("AddSet() emitted %d messages, want at least 2", gotNewMessages)
457+
}
458+
459+
setMsg := c.messages[start]
460+
gotSet, err := setsFromMsg(netlink.Message{
461+
Header: setMsg.Header,
462+
Data: setMsg.Data,
463+
})
464+
if err != nil {
465+
t.Fatalf("setsFromMsg() failed: %v", err)
466+
}
467+
if gotSet.KeyByteOrder != tt.wantKeyOrder {
468+
t.Fatalf("set.KeyByteOrder = %v, want %v", gotSet.KeyByteOrder, tt.wantKeyOrder)
469+
}
470+
if gotSet.DataByteOrder != tt.wantDataOrder {
471+
t.Fatalf("set.DataByteOrder = %v, want %v", gotSet.DataByteOrder, tt.wantDataOrder)
472+
}
473+
474+
elemMsg := c.messages[start+1]
475+
gotElems, err := elementsFromMsg(byte(tbl.Family), netlink.Message{
476+
Header: elemMsg.Header,
477+
Data: elemMsg.Data,
478+
})
479+
if err != nil {
480+
t.Fatalf("elementsFromMsg() failed: %v", err)
481+
}
482+
if got, want := len(gotElems), len(tt.elems); got != want {
483+
t.Fatalf("decoded %d elements, want %d", got, want)
484+
}
485+
for i := range tt.elems {
486+
if !reflect.DeepEqual(gotElems[i], tt.elems[i]) {
487+
t.Fatalf("element[%d] mismatch: got %+v, want %+v", i, gotElems[i], tt.elems[i])
488+
}
489+
}
490+
})
491+
}
492+
}

0 commit comments

Comments
 (0)