Merge dae73eaa9c into e2fedeb355

There was an existing mechanism to allocate IDs for sets, but this was
using a global counter without any synchronization to prevent data
races. I replaced this by a new mechanism which uses a connection-scoped
counter, protected by the Conn.mu Mutex. This can then also be used in
other places where IDs need to be allocated.

As an additional safeguard, it will panic instead of allocating the same
ID twice in a transaction. Most likely, your program will run out of
memory before reaching this point.

.gitignore

@@ -0,0 +1 @@
+nftables.test

README.md

@@ -21,4 +21,12 @@ the data types/API will be identified as more functionality is added.
 
 Contributions are very welcome!
 
+### Testing Changes
 
+Run the following commands to test your changes:
+
+```bash
+go test ./...
+go test -c github.com/google/nftables
+sudo ./nftables.test -test.v -run_system_tests
+```

conn.go

@@ -17,6 +17,7 @@ package nftables
 import (
 	"errors"
 	"fmt"
+	"math"
 	"os"
 	"sync"
 	"syscall"
@@ -38,12 +39,14 @@ type Conn struct {
 	TestDial nltest.Func // for testing only; passed to nltest.Dial
 	NetNS    int         // fd referencing the network namespace netlink will interact with.
 
-	lasting     bool       // establish a lasting connection to be used across multiple netlink operations.
-	mu          sync.Mutex // protects the following state
-	messages    []netlink.Message
-	err         error
-	nlconn      *netlink.Conn // netlink socket using NETLINK_NETFILTER protocol.
-	sockOptions []SockOption
+	lasting      bool       // establish a lasting connection to be used across multiple netlink operations.
+	mu           sync.Mutex // protects the following state
+	messages     []netlink.Message
+	err          error
+	nlconn       *netlink.Conn // netlink socket using NETLINK_NETFILTER protocol.
+	sockOptions  []SockOption
+	lastID       uint32
+	allocatedIDs uint32
 }
 
 // ConnOption is an option to change the behavior of the nftables Conn returned by Open.
@@ -244,6 +247,7 @@ func (cc *Conn) Flush() error {
 	cc.mu.Lock()
 	defer func() {
 		cc.messages = nil
+		cc.allocatedIDs = 0
 		cc.mu.Unlock()
 	}()
 	if len(cc.messages) == 0 {
@@ -369,3 +373,20 @@ func batch(messages []netlink.Message) []netlink.Message {
 
 	return batch
 }
+
+// allocateTransactionID allocates an identifier which is only valid in the
+// current transaction.
+func (cc *Conn) allocateTransactionID() uint32 {
+	if cc.allocatedIDs == math.MaxUint32 {
+		panic(fmt.Sprintf("trying to allocate more than %d IDs in a single nftables transaction", math.MaxUint32))
+	}
+	// To make it more likely to catch when a transaction ID is erroneously used
+	// in a later transaction, cc.lastID is not reset after each transaction;
+	// instead it is only reset once it rolls over from math.MaxUint32 to 0.
+	cc.allocatedIDs++
+	cc.lastID++
+	if cc.lastID == 0 {
+		cc.lastID = 1
+	}
+	return cc.lastID
+}

expr/verdict.go

@@ -24,6 +24,15 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+const (
+	NFT_DROP   = 0
+	NFT_ACCEPT = 1
+	NFT_STOLEN = 2
+	NFT_QUEUE  = 3
+	NFT_REPEAT = 4
+	NFT_STOP   = 5
+)
+
 // This code assembles the verdict structure, as expected by the
 // nftables netlink API.
 // For further information, consult:
@@ -129,3 +138,37 @@ func (e *Verdict) unmarshal(fam byte, data []byte) error {
 	}
 	return ad.Err()
 }
+
+func (e *Verdict) String() string {
+	var v string
+	switch e.Kind {
+	case unix.NFT_RETURN:
+		v = "return" // -0x5
+	case unix.NFT_GOTO:
+		v = "goto" // -0x4
+	case unix.NFT_JUMP:
+		v = "jump" // NFT_JUMP = -0x3
+	case unix.NFT_BREAK:
+		v = "break" // NFT_BREAK = -0x2
+	case unix.NFT_CONTINUE:
+		v = "continue" // NFT_CONTINUE = -0x1
+	case NFT_DROP:
+		v = "drop"
+	case NFT_ACCEPT:
+		v = "accept"
+	case NFT_STOLEN:
+		v = "stolen"
+	case NFT_QUEUE:
+		v = "queue"
+	case NFT_REPEAT:
+		v = "repeat"
+	case NFT_STOP:
+		v = "stop"
+	default:
+		v = fmt.Sprintf("verdict %v", e.Kind)
+	}
+	if e.Chain != "" {
+		return v + " " + e.Chain
+	}
+	return v
+}

go.mod

@@ -1,17 +1,17 @@
 module github.com/google/nftables
 
-go 1.21
+go 1.23.0
 
 require (
 	github.com/google/go-cmp v0.6.0
 	github.com/mdlayher/netlink v1.7.3-0.20250113171957-fbb4dce95f42
 	github.com/vishvananda/netlink v1.3.0
 	github.com/vishvananda/netns v0.0.4
-	golang.org/x/sys v0.28.0
+	golang.org/x/sys v0.31.0
 )
 
 require (
 	github.com/mdlayher/socket v0.5.0 // indirect
-	golang.org/x/net v0.33.0 // indirect
+	golang.org/x/net v0.37.0 // indirect
 	golang.org/x/sync v0.6.0 // indirect
 )

go.sum

@@ -8,11 +8,11 @@ github.com/vishvananda/netlink v1.3.0 h1:X7l42GfcV4S6E4vHTsw48qbrV+9PVojNfIhZcwQ
 github.com/vishvananda/netlink v1.3.0/go.mod h1:i6NetklAujEcC6fK0JPjT8qSwWyO0HLn4UKG+hGqeJs=
 github.com/vishvananda/netns v0.0.4 h1:Oeaw1EM2JMxD51g9uhtC0D7erkIjgmj8+JZc26m1YX8=
 github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZlaprSwhNNJM=
-golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
-golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
+golang.org/x/net v0.37.0 h1:1zLorHbz+LYj7MQlSf1+2tPIIgibq2eL5xkrGk6f+2c=
+golang.org/x/net v0.37.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8=
 golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
 golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
-golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik=
+golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=

nftables_test.go

@@ -222,12 +222,27 @@ func TestRuleOperations(t *testing.T) {
 		expr.VerdictDrop,
 	}
 
+	wantStrings := []string{
+		"queue",
+		"accept",
+		"queue",
+		"accept",
+		"drop",
+		"drop",
+	}
+
 	for i, r := range rules {
 		rr, _ := r.Exprs[0].(*expr.Verdict)
 
 		if rr.Kind != want[i] {
 			t.Fatalf("bad verdict kind at %d", i)
 		}
+
+		if rr.String() != wantStrings[i] {
+			t.Fatalf("bad verdict string at %d: %s (received) vs. %s (expected)", i, rr.String(), wantStrings[i])
+		}
+
+		t.Logf("%s", rr)
 	}
 }
 

nftables_test.sh

@@ -0,0 +1,3 @@
+go test ./...
+go test -c github.com/google/nftables
+sudo ./nftables.test -test.v -run_system_tests

set.go

@@ -46,8 +46,6 @@ const (
 	NFTA_SET_ELEM_EXPRESSIONS = 0x11
 )
 
-var allocSetID uint32
-
 // SetDatatype represents a datatype declared by nft.
 type SetDatatype struct {
 	Name  string
@@ -532,8 +530,7 @@ func (cc *Conn) AddSet(s *Set, vals []SetElement) error {
 	}
 
 	if s.ID == 0 {
-		allocSetID++
-		s.ID = allocSetID
+		s.ID = cc.allocateTransactionID()
 		if s.Anonymous {
 			s.Name = "__set%d"
 			if s.IsMap {