Rework the interface to libnet/internal/nftables

Add nftables.Modifier, to hold a queue of commands that can be applied
using Modifier.Apply. No updates are made to the underlying Table
until Apply is called, errors in the queue if commands are deferred
until Apply.

This has the advantages that:
- less error handling is needed in code that generates update commands
- it's transactional, without needing explicit transactions

Minor disadvantages are that it's slightly more difficult to debug updates,
as it's no longer possible to step through the call making an update to
the Table manipulation in a debugger - and errors in the command, and
errors like trying to update a nonexistent chain/set/vmap, deleting an
object that doesn't exist or creating a duplicate are not reported
until the updates are applied (but, the file/line where the rule was
added is reported).

Signed-off-by: Rob Murray <rob.murray@docker.com>

daemon/libnetwork/drivers/bridge/internal/nftabler/endpoint.go

@@ -25,18 +25,24 @@ func (n *network) DelEndpoint(ctx context.Context, epIPv4, epIPv6 netip.Addr) er
 
 func (n *network) modEndpoint(ctx context.Context, epIPv4, epIPv6 netip.Addr, enable bool) error {
 	if n.fw.config.IPv4 && epIPv4.IsValid() {
-		if err := n.filterDirectAccess(ctx, n.fw.table4, n.config.Config4, epIPv4, enable); err != nil {
-			return err
+		tm := nftables.Modifier{}
+		updater := tm.Create
+		if !enable {
+			updater = tm.Delete
 		}
-		if err := nftApply(ctx, n.fw.table4); err != nil {
+		n.filterDirectAccess(updater, nftables.IPv4, n.config.Config4, epIPv4)
+		if err := n.fw.table4.Apply(ctx, tm); err != nil {
 			return fmt.Errorf("adding rules for bridge %s: %w", n.config.IfName, err)
 		}
 	}
 	if n.fw.config.IPv6 && epIPv6.IsValid() {
-		if err := n.filterDirectAccess(ctx, n.fw.table6, n.config.Config6, epIPv6, enable); err != nil {
-			return err
+		tm := nftables.Modifier{}
+		updater := tm.Create
+		if !enable {
+			updater = tm.Delete
 		}
-		if err := nftApply(ctx, n.fw.table6); err != nil {
+		n.filterDirectAccess(updater, nftables.IPv6, n.config.Config6, epIPv6)
+		if err := n.fw.table6.Apply(ctx, tm); err != nil {
 			return fmt.Errorf("adding rules for bridge %s: %w", n.config.IfName, err)
 		}
 	}
@@ -53,17 +59,21 @@ func (n *network) modEndpoint(ctx context.Context, epIPv4, epIPv6 netip.Addr, en
 //     kernel support).
 //
 // Packets originating on the bridge's own interface and addressed directly to the
-// container are allowed - the host always has direct access to its own containers
-// (it doesn't need to use the port mapped to its own addresses, although it can).
+// container are allowed - the host always has direct access to its own containers.
+// (It doesn't need to use the port mapped to its own addresses, although it can.)
 //
 // "Trusted interfaces" are treated in the same way as the bridge itself.
-func (n *network) filterDirectAccess(ctx context.Context, table nftables.TableRef, conf firewaller.NetworkConfigFam, epIP netip.Addr, enable bool) error {
+func (n *network) filterDirectAccess(updater func(nftables.Obj), fam nftables.Family, conf firewaller.NetworkConfigFam, epIP netip.Addr) {
 	if n.config.Internal || conf.Unprotected || conf.Routed || n.fw.config.AllowDirectRouting {
-		return nil
+		return
 	}
-	updater := table.ChainUpdateFunc(ctx, rawPreroutingChain, enable)
 	ifNames := strings.Join(n.config.TrustedHostInterfaces, ", ")
-	return updater(ctx, rawPreroutingPortsRuleGroup,
-		`%s daddr %s iifname != { %s, %s } counter drop comment "DROP DIRECT ACCESS"`,
-		table.Family(), epIP, n.config.IfName, ifNames)
+	updater(nftables.Rule{
+		Chain: rawPreroutingChain,
+		Group: rawPreroutingPortsRuleGroup,
+		Rule: []string{
+			string(fam), "daddr", epIP.String(),
+			"iifname != {", n.config.IfName, ",", ifNames, `} counter drop comment "DROP DIRECT ACCESS"`,
+		},
+	})
 }

daemon/libnetwork/drivers/bridge/internal/nftabler/link.go

@@ -7,8 +7,10 @@ import (
 	"errors"
 	"fmt"
 	"net/netip"
+	"strconv"
 
 	"github.com/containerd/log"
+	"github.com/moby/moby/v2/daemon/libnetwork/internal/nftables"
 	"github.com/moby/moby/v2/daemon/libnetwork/types"
 )
 
@@ -20,44 +22,47 @@ func (n *network) AddLink(ctx context.Context, parentIP, childIP netip.Addr, por
 		return errors.New("cannot link to a container with an empty child IP address")
 	}
 
-	chain := n.fw.table4.Chain(ctx, chainFilterFwdIn(n.config.IfName))
+	tm := nftables.Modifier{}
 	for _, port := range ports {
-		for _, rule := range legacyLinkRules(parentIP, childIP, port) {
-			if err := chain.AppendRule(ctx, fwdInLegacyLinksRuleGroup, rule); err != nil {
-				return err
-			}
-		}
+		updateLegacyLinkRules(tm.Create, chainFilterFwdIn(n.config.IfName), parentIP, childIP, port)
 	}
-	if err := nftApply(ctx, n.fw.table4); err != nil {
+	if err := n.fw.table4.Apply(ctx, tm); err != nil {
 		return fmt.Errorf("adding rules for bridge %s: %w", n.config.IfName, err)
 	}
 	return nil
 }
 
 func (n *network) DelLink(ctx context.Context, parentIP, childIP netip.Addr, ports []types.TransportPort) {
-	chain := n.fw.table4.Chain(ctx, chainFilterFwdIn(n.config.IfName))
+	tm := nftables.Modifier{}
 	for _, port := range ports {
-		for _, rule := range legacyLinkRules(parentIP, childIP, port) {
-			if err := chain.DeleteRule(ctx, fwdInLegacyLinksRuleGroup, rule); err != nil {
-				log.G(ctx).WithFields(log.Fields{
-					"rule":  rule,
-					"error": err,
-				}).Warn("Failed to remove link between containers")
-			}
-		}
+		updateLegacyLinkRules(tm.Delete, chainFilterFwdIn(n.config.IfName), parentIP, childIP, port)
 	}
-	if err := nftApply(ctx, n.fw.table4); err != nil {
+	if err := n.fw.table4.Apply(ctx, tm); err != nil {
 		log.G(ctx).WithError(err).Warn("Removing link, failed to update nftables")
 	}
 }
 
-func legacyLinkRules(parentIP, childIP netip.Addr, port types.TransportPort) []string {
+func updateLegacyLinkRules(updater func(command nftables.Obj), chainName string, parentIP, childIP netip.Addr, port types.TransportPort) {
 	// TODO(robmry) - could combine rules for each proto by using an anonymous set.
-	return []string{
-		// Match the iptables implementation, but without checking iifname/oifname (not needed
-		// because the addresses belong to the bridge).
-		fmt.Sprintf("ip saddr %s ip daddr %s %s dport %d counter accept", parentIP.Unmap(), childIP.Unmap(), port.Proto, port.Port),
-		// Conntrack will allow responses. So, this must be to allow unsolicited packets from an exposed port.
-		fmt.Sprintf("ip daddr %s ip saddr %s %s sport %d counter accept", parentIP.Unmap(), childIP.Unmap(), port.Proto, port.Port),
-	}
+	// Match the iptables implementation, but without checking iifname/oifname (not needed
+	// because the addresses belong to the bridge).
+	updater(nftables.Rule{
+		Chain: chainName,
+		Group: fwdInLegacyLinksRuleGroup,
+		Rule: []string{
+			"ip saddr", parentIP.Unmap().String(),
+			"ip daddr", childIP.Unmap().String(), port.Proto.String(), "dport", strconv.Itoa(int(port.Port)),
+			"counter accept",
+		},
+	})
+	// Conntrack will allow responses. So, this must be to allow unsolicited packets from an exposed port.
+	updater(nftables.Rule{
+		Chain: chainName,
+		Group: fwdInLegacyLinksRuleGroup,
+		Rule: []string{
+			"ip saddr", childIP.Unmap().String(), port.Proto.String(), "sport", strconv.Itoa(int(port.Port)),
+			"ip daddr", parentIP.Unmap().String(),
+			"counter accept",
+		},
+	})
 }

daemon/libnetwork/drivers/bridge/internal/nftabler/network.go

@@ -9,16 +9,16 @@ import (
 	"strings"
 
 	"github.com/containerd/log"
-	"github.com/moby/moby/v2/daemon/internal/cleanups"
 	"github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge/internal/firewaller"
 	"github.com/moby/moby/v2/daemon/libnetwork/internal/nftables"
 	"go.opentelemetry.io/otel"
 )
 
 type network struct {
-	config  firewaller.NetworkConfig
-	cleaner func(ctx context.Context) error
-	fw      *Nftabler
+	config   firewaller.NetworkConfig
+	fw       *Nftabler
+	remover4 *nftables.Modifier
+	remover6 *nftables.Modifier
 }
 
 func (nft *Nftabler) NewNetwork(ctx context.Context, nc firewaller.NetworkConfig) (_ firewaller.Network, retErr error) {
@@ -28,108 +28,84 @@ func (nft *Nftabler) NewNetwork(ctx context.Context, nc firewaller.NetworkConfig
 	}
 	ctx = log.WithLogger(ctx, log.G(ctx).WithFields(log.Fields{"bridge": n.config.IfName}))
 
-	var cleaner cleanups.Composite
-	defer func() {
-		if err := cleaner.Call(ctx); err != nil {
-			log.G(ctx).WithError(err).Warn("Failed to clean up nftables rules for network")
-		}
-	}()
-
 	if nft.cleaner != nil {
 		nft.cleaner.DelNetwork(ctx, nc)
 	}
 
 	if n.fw.config.IPv4 {
-		clean, err := n.configure(ctx, nft.table4, n.config.Config4)
+		remover, err := n.configure(ctx, nft.table4, n.config.Config4)
 		if err != nil {
 			return nil, err
 		}
-		if clean != nil {
-			cleaner.Add(clean)
-		}
+		n.remover4 = remover
 	}
 	if n.fw.config.IPv6 {
-		clean, err := n.configure(ctx, nft.table6, n.config.Config6)
+		remover, err := n.configure(ctx, nft.table6, n.config.Config6)
 		if err != nil {
 			return nil, err
 		}
-		if clean != nil {
-			cleaner.Add(clean)
-		}
+		n.remover6 = remover
 	}
-
-	n.cleaner = cleaner.Release()
 	return n, nil
 }
 
-func (n *network) configure(ctx context.Context, table nftables.TableRef, conf firewaller.NetworkConfigFam) (func(context.Context) error, error) {
-	ctx, span := otel.Tracer("").Start(ctx, spanPrefix+".newNetwork."+string(table.Family()))
-	defer span.End()
-
+func (n *network) configure(ctx context.Context, table nftables.Table, conf firewaller.NetworkConfigFam) (*nftables.Modifier, error) {
 	if !conf.Prefix.IsValid() {
 		return nil, nil
 	}
+	tm := nftables.Modifier{}
+	ctx, span := otel.Tracer("").Start(ctx, spanPrefix+".newNetwork."+string(table.Family()))
+	defer span.End()
 
-	var cleanup cleanups.Composite
-	defer cleanup.Call(ctx)
-	var applied bool
-	cleanup.Add(func(ctx context.Context) error {
-		if applied {
-			return nftApply(ctx, table)
-		}
-		return nil
-	})
+	fwdInChain := chainFilterFwdIn(n.config.IfName)
+	fwdOutChain := chainFilterFwdOut(n.config.IfName)
+	natPostRtInChain := chainNatPostRtIn(n.config.IfName)
+	natPostRtOutChain := chainNatPostRtOut(n.config.IfName)
 
 	// Filter chain
 
-	fwdInChain := table.Chain(ctx, chainFilterFwdIn(n.config.IfName))
-	cleanup.Add(func(ctx context.Context) error { return table.DeleteChain(ctx, chainFilterFwdIn(n.config.IfName)) })
-	fwdOutChain := table.Chain(ctx, chainFilterFwdOut(n.config.IfName))
-	cleanup.Add(func(ctx context.Context) error { return table.DeleteChain(ctx, chainFilterFwdOut(n.config.IfName)) })
+	tm.Create(nftables.Chain{Name: fwdInChain})
+	tm.Create(nftables.Chain{Name: fwdOutChain})
 
-	cf, err := table.InterfaceVMap(ctx, filtFwdInVMap).AddElementCf(ctx, n.config.IfName, "jump "+chainFilterFwdIn(n.config.IfName))
-	if err != nil {
-		return nil, fmt.Errorf("adding filter-forward jump for %s to %q: %w", conf.Prefix, chainFilterFwdIn(n.config.IfName), err)
-	}
-	cleanup.Add(cf)
-
-	cf, err = table.InterfaceVMap(ctx, filtFwdOutVMap).AddElementCf(ctx, n.config.IfName, "jump "+chainFilterFwdOut(n.config.IfName))
-	if err != nil {
-		return nil, fmt.Errorf("adding filter-forward jump for %s to %q: %w", conf.Prefix, chainFilterFwdOut(n.config.IfName), err)
-	}
-	cleanup.Add(cf)
+	tm.Create(nftables.VMapElement{
+		VmapName: filtFwdInVMap,
+		Key:      n.config.IfName,
+		Verdict:  "jump " + fwdInChain,
+	})
+	tm.Create(nftables.VMapElement{
+		VmapName: filtFwdOutVMap,
+		Key:      n.config.IfName,
+		Verdict:  "jump " + fwdOutChain,
+	})
 
 	// NAT chain
 
-	natPostroutingIn := table.Chain(ctx, chainNatPostRtIn(n.config.IfName))
-	cleanup.Add(func(ctx context.Context) error { return table.DeleteChain(ctx, chainNatPostRtIn(n.config.IfName)) })
-	cf, err = table.InterfaceVMap(ctx, natPostroutingInVMap).AddElementCf(ctx, n.config.IfName, "jump "+chainNatPostRtIn(n.config.IfName))
-	if err != nil {
-		return nil, fmt.Errorf("adding postrouting ingress jump for %s to %q: %w", conf.Prefix, chainNatPostRtIn(n.config.IfName), err)
-	}
-	cleanup.Add(cf)
+	tm.Create(nftables.Chain{Name: natPostRtInChain})
+	tm.Create(nftables.VMapElement{
+		VmapName: natPostroutingInVMap,
+		Key:      n.config.IfName,
+		Verdict:  "jump " + natPostRtInChain,
+	})
 
-	natPostroutingOut := table.Chain(ctx, chainNatPostRtOut(n.config.IfName))
-	cleanup.Add(func(ctx context.Context) error { return table.DeleteChain(ctx, chainNatPostRtOut(n.config.IfName)) })
-	cf, err = table.InterfaceVMap(ctx, natPostroutingOutVMap).AddElementCf(ctx, n.config.IfName, "jump "+chainNatPostRtOut(n.config.IfName))
-	if err != nil {
-		return nil, fmt.Errorf("adding postrouting egress jump for %s to %q: %w", conf.Prefix, chainNatPostRtOut(n.config.IfName), err)
-	}
-	cleanup.Add(cf)
+	tm.Create(nftables.Chain{Name: chainNatPostRtOut(n.config.IfName)})
+	tm.Create(nftables.VMapElement{
+		VmapName: natPostroutingOutVMap,
+		Key:      n.config.IfName,
+		Verdict:  "jump " + chainNatPostRtOut(n.config.IfName),
+	})
 
 	// Conntrack
 
-	cf, err = fwdInChain.AppendRuleCf(ctx, initialRuleGroup, "ct state established,related counter accept")
-	if err != nil {
-		return nil, fmt.Errorf("adding conntrack ingress rule for %q: %w", n.config.IfName, err)
-	}
-	cleanup.Add(cf)
-
-	cf, err = fwdOutChain.AppendRuleCf(ctx, initialRuleGroup, "ct state established,related counter accept")
-	if err != nil {
-		return nil, fmt.Errorf("adding conntrack egress rule for %q: %w", n.config.IfName, err)
-	}
-	cleanup.Add(cf)
+	tm.Create(nftables.Rule{
+		Chain: chainFilterFwdIn(n.config.IfName),
+		Group: initialRuleGroup,
+		Rule:  []string{"ct state established,related counter accept"},
+	})
+	tm.Create(nftables.Rule{
+		Chain: chainFilterFwdOut(n.config.IfName),
+		Group: initialRuleGroup,
+		Rule:  []string{"ct state established,related counter accept"},
+	})
 
 	iccVerdict := "accept"
 	if !n.config.ICC {
@@ -138,26 +114,23 @@ func (n *network) configure(ctx context.Context, table nftables.TableRef, conf f
 
 	if n.config.Internal {
 		// Drop anything that's not from this network.
-		cf, err = fwdInChain.AppendRuleCf(ctx, initialRuleGroup,
-			`iifname != %s counter drop comment "INTERNAL NETWORK INGRESS"`, n.config.IfName)
-		if err != nil {
-			return nil, fmt.Errorf("adding INTERNAL NETWORK ingress rule for %q: %w", n.config.IfName, err)
-		}
-		cleanup.Add(cf)
-
-		cf, err = fwdOutChain.AppendRuleCf(ctx, initialRuleGroup,
-			`oifname != %s counter drop comment "INTERNAL NETWORK EGRESS"`, n.config.IfName)
-		if err != nil {
-			return nil, fmt.Errorf("adding INTERNAL NETWORK egress rule for %q: %w", n.config.IfName, err)
-		}
-		cleanup.Add(cf)
+		tm.Create(nftables.Rule{
+			Chain: fwdInChain,
+			Group: initialRuleGroup,
+			Rule:  []string{`iifname != `, n.config.IfName, `counter drop comment "INTERNAL NETWORK INGRESS"`},
+		})
+		tm.Create(nftables.Rule{
+			Chain: fwdOutChain,
+			Group: initialRuleGroup,
+			Rule:  []string{`oifname != `, n.config.IfName, `counter drop comment "INTERNAL NETWORK EGRESS"`},
+		})
 
 		// Accept or drop Inter-Container Communication.
-		cf, err = fwdInChain.AppendRuleCf(ctx, fwdInICCRuleGroup, "counter %s comment ICC", iccVerdict)
-		if err != nil {
-			return nil, fmt.Errorf("adding ICC ingress rule for %q: %w", n.config.IfName, err)
-		}
-		cleanup.Add(cf)
+		tm.Create(nftables.Rule{
+			Chain: fwdInChain,
+			Group: fwdInICCRuleGroup,
+			Rule:  []string{"counter", iccVerdict, "comment ICC"},
+		})
 	} else {
 		// AcceptFwMark
 		if n.config.AcceptFwMark != "" {
@@ -165,42 +138,40 @@ func (n *network) configure(ctx context.Context, table nftables.TableRef, conf f
 			if err != nil {
 				return nil, fmt.Errorf("adding fwmark %q for %q: %w", n.config.AcceptFwMark, n.config.IfName, err)
 			}
-			cf, err = fwdInChain.AppendRuleCf(ctx, fwdInAcceptFwMarkRuleGroup,
-				`meta mark %s counter accept comment "ALLOW FW MARK"`, fwm)
-			if err != nil {
-				return nil, fmt.Errorf("adding ALLOW FW MARK rule for %q: %w", n.config.IfName, err)
-			}
-			cleanup.Add(cf)
+			tm.Create(nftables.Rule{
+				Chain: fwdInChain,
+				Group: fwdInAcceptFwMarkRuleGroup,
+				Rule:  []string{`meta mark `, fwm, ` counter accept comment "ALLOW FW MARK"`},
+			})
 		}
 
 		// Inter-Container Communication
-		cf, err = fwdInChain.AppendRuleCf(ctx, fwdInICCRuleGroup, "iifname == %s counter %s comment ICC",
-			n.config.IfName, iccVerdict)
-		if err != nil {
-			return nil, fmt.Errorf("adding ICC rule for %q: %w", n.config.IfName, err)
-		}
-		cleanup.Add(cf)
+		tm.Create(nftables.Rule{
+			Chain: fwdInChain,
+			Group: fwdInICCRuleGroup,
+			Rule:  []string{"iifname ==", n.config.IfName, "counter", iccVerdict, "comment ICC"},
+		})
 
 		// Outgoing traffic
-		cf, err = fwdOutChain.AppendRuleCf(ctx, initialRuleGroup, "counter accept comment OUTGOING")
-		if err != nil {
-			return nil, fmt.Errorf("adding OUTGOING rule for %q: %w", n.config.IfName, err)
-		}
-		cleanup.Add(cf)
+		tm.Create(nftables.Rule{
+			Chain: fwdOutChain,
+			Group: initialRuleGroup,
+			Rule:  []string{"counter accept comment OUTGOING"},
+		})
 
 		// Incoming traffic
 		if conf.Unprotected {
-			cf, err = fwdInChain.AppendRuleCf(ctx, fwdInFinalRuleGroup, `counter accept comment "UNPROTECTED"`)
-			if err != nil {
-				return nil, fmt.Errorf("adding UNPROTECTED for %q: %w", n.config.IfName, err)
-			}
-			cleanup.Add(cf)
+			tm.Create(nftables.Rule{
+				Chain: fwdInChain,
+				Group: fwdInFinalRuleGroup,
+				Rule:  []string{`counter accept comment "UNPROTECTED"`},
+			})
 		} else {
-			cf, err = fwdInChain.AppendRuleCf(ctx, fwdInFinalRuleGroup, `counter drop comment "UNPUBLISHED PORT DROP"`)
-			if err != nil {
-				return nil, fmt.Errorf("adding UNPUBLISHED PORT DROP for %q: %w", n.config.IfName, err)
-			}
-			cleanup.Add(cf)
+			tm.Create(nftables.Rule{
+				Chain: fwdInChain,
+				Group: fwdInFinalRuleGroup,
+				Rule:  []string{`counter drop comment "UNPUBLISHED PORT DROP"`},
+			})
 		}
 
 		// ICMP
@@ -209,11 +180,11 @@ func (n *network) configure(ctx context.Context, table nftables.TableRef, conf f
 			if table.Family() == nftables.IPv6 {
 				rule = "meta l4proto ipv6-icmp"
 			}
-			cf, err = fwdInChain.AppendRuleCf(ctx, initialRuleGroup, rule+" counter accept comment ICMP")
-			if err != nil {
-				return nil, fmt.Errorf("adding ICMP rule for %q: %w", n.config.IfName, err)
-			}
-			cleanup.Add(cf)
+			tm.Create(nftables.Rule{
+				Chain: fwdInChain,
+				Group: initialRuleGroup,
+				Rule:  []string{rule, "counter accept comment ICMP"},
+			})
 		}
 
 		// Masquerade / SNAT - masquerade picks a source IP address based on next-hop, SNAT uses conf.HostIP.
@@ -224,21 +195,23 @@ func (n *network) configure(ctx context.Context, table nftables.TableRef, conf f
 			natPostroutingComment = "SNAT"
 		}
 		if n.config.Masquerade && !conf.Routed {
-			cf, err = natPostroutingOut.AppendRuleCf(ctx, initialRuleGroup, `oifname != %s %s saddr %s counter %s comment "%s"`,
-				n.config.IfName, table.Family(), conf.Prefix, natPostroutingVerdict, natPostroutingComment)
-			if err != nil {
-				return nil, fmt.Errorf("adding NAT rule for %q: %w", n.config.IfName, err)
-			}
-			cleanup.Add(cf)
+			tm.Create(nftables.Rule{
+				Chain: natPostRtOutChain,
+				Group: initialRuleGroup,
+				Rule: []string{
+					"oifname !=", n.config.IfName, string(table.Family()), "saddr", conf.Prefix.String(), "counter",
+					natPostroutingVerdict, "comment", natPostroutingComment,
+				},
+			})
 		}
 		if n.fw.config.Hairpin {
-			// Masquerade/SNAT traffic from localhost.
-			cf, err = natPostroutingIn.AppendRuleCf(ctx, initialRuleGroup, `fib saddr type local counter %s comment "%s FROM HOST"`,
-				natPostroutingVerdict, natPostroutingComment)
-			if err != nil {
-				return nil, fmt.Errorf("adding NAT local rule for %q: %w", n.config.IfName, err)
-			}
-			cleanup.Add(cf)
+			tm.Create(nftables.Rule{
+				Chain: natPostRtInChain,
+				Group: initialRuleGroup,
+				Rule: []string{
+					`fib saddr type local counter`, natPostroutingVerdict, `comment "` + natPostroutingComment + ` FROM HOST"`,
+				},
+			})
 		}
 	}
 
@@ -246,12 +219,11 @@ func (n *network) configure(ctx context.Context, table nftables.TableRef, conf f
 		"bridge": n.config.IfName,
 		"family": table.Family(),
 	}))
-	if err := nftApply(ctx, table); err != nil {
+	if err := table.Apply(ctx, tm); err != nil {
 		return nil, fmt.Errorf("adding rules for bridge %s: %w", n.config.IfName, err)
 	}
-	applied = true
-
-	return cleanup.Release(), nil
+	undoer := tm.Reverse()
+	return &undoer, nil
 }
 
 func (n *network) ReapplyNetworkLevelRules(ctx context.Context) error {
@@ -261,12 +233,21 @@ func (n *network) ReapplyNetworkLevelRules(ctx context.Context) error {
 }
 
 func (n *network) DelNetworkLevelRules(ctx context.Context) error {
-	if n.cleaner != nil {
-		ctx = log.WithLogger(ctx, log.G(ctx).WithFields(log.Fields{"bridge": n.config.IfName}))
-		if err := n.cleaner(ctx); err != nil {
-			log.G(ctx).WithError(err).Warn("Failed to remove network rules for network")
+	remove := func(t nftables.Table, remover *nftables.Modifier) {
+		if remover != nil {
+			ctx := log.WithLogger(ctx, log.G(ctx).WithFields(log.Fields{"bridge": n.config.IfName}))
+			if err := t.Apply(ctx, *remover); err != nil {
+				log.G(ctx).WithError(err).Warn("Failed to remove network rules for network")
+			}
 		}
-		n.cleaner = nil
+	}
+	if n.remover4 != nil {
+		remove(n.fw.table4, n.remover4)
+		n.remover4 = nil
+	}
+	if n.remover6 != nil {
+		remove(n.fw.table6, n.remover6)
+		n.remover6 = nil
 	}
 	return nil
 }

daemon/libnetwork/drivers/bridge/internal/nftabler/nftabler.go

@@ -4,12 +4,10 @@ package nftabler
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/containerd/log"
 	"github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge/internal/firewaller"
 	"github.com/moby/moby/v2/daemon/libnetwork/internal/nftables"
-	"go.opentelemetry.io/otel"
 )
 
 // Prefix for OTEL span names.
@@ -48,8 +46,8 @@ const (
 type Nftabler struct {
 	config  firewaller.Config
 	cleaner firewaller.FirewallCleaner
-	table4  nftables.TableRef
-	table6  nftables.TableRef
+	table4  nftables.Table
+	table6  nftables.Table
 }
 
 func NewNftabler(ctx context.Context, config firewaller.Config) (*Nftabler, error) {
@@ -61,9 +59,6 @@ func NewNftabler(ctx context.Context, config firewaller.Config) (*Nftabler, erro
 		if err != nil {
 			return nil, err
 		}
-		if err := nftApply(ctx, nft.table4); err != nil {
-			return nil, fmt.Errorf("IPv4 initialisation: %w", err)
-		}
 	}
 
 	if nft.config.IPv6 {
@@ -72,26 +67,18 @@ func NewNftabler(ctx context.Context, config firewaller.Config) (*Nftabler, erro
 		if err != nil {
 			return nil, err
 		}
-
-		if err := nftApply(ctx, nft.table6); err != nil {
-			// Perhaps the kernel has no IPv6 support. It won't be possible to create IPv6
-			// networks without enabling ip6_tables in the kernel, or disabling ip6tables in
-			// the daemon config. But, allow the daemon to start because IPv4 will work. So,
-			// log the problem, and continue.
-			log.G(ctx).WithError(err).Warn("ip6tables is enabled, but cannot set up IPv6 nftables table")
-		}
 	}
 
 	return nft, nil
 }
 
-// init creates the bridge driver's nftables table for IPv4 or IPv6.
-func (nft *Nftabler) init(ctx context.Context, family nftables.Family) (nftables.TableRef, error) {
+func (nft *Nftabler) init(ctx context.Context, family nftables.Family) (nftables.Table, error) {
 	// Instantiate the table.
 	table, err := nftables.NewTable(family, dockerTable)
 	if err != nil {
 		return table, err
 	}
+	tm := nftables.Modifier{}
 
 	// Set up the filter forward chain.
 	//
@@ -102,65 +89,89 @@ func (nft *Nftabler) init(ctx context.Context, family nftables.Family) (nftables
 	// So, packets that aren't related to docker don't need to traverse any per-network filter forward
 	// rules - and packets that are entering or leaving docker networks only need to traverse rules
 	// related to those networks.
-	fwdChain, err := table.BaseChain(ctx, forwardChain,
-		nftables.BaseChainTypeFilter,
-		nftables.BaseChainHookForward,
-		nftables.BaseChainPriorityFilter)
-	if err != nil {
-		return nftables.TableRef{}, fmt.Errorf("initialising nftables: %w", err)
-	}
+	tm.Create(nftables.BaseChain{
+		Name:      forwardChain,
+		ChainType: nftables.BaseChainTypeFilter,
+		Hook:      nftables.BaseChainHookForward,
+		Priority:  nftables.BaseChainPriorityFilter,
+	})
 	// Instantiate the verdict maps and add the jumps.
-	_ = table.InterfaceVMap(ctx, filtFwdInVMap)
-	if err := fwdChain.AppendRule(ctx, initialRuleGroup, "oifname vmap @"+filtFwdInVMap); err != nil {
-		return nftables.TableRef{}, fmt.Errorf("initialising nftables: %w", err)
-	}
-	_ = table.InterfaceVMap(ctx, filtFwdOutVMap)
-	if err := fwdChain.AppendRule(ctx, initialRuleGroup, "iifname vmap @"+filtFwdOutVMap); err != nil {
-		return nftables.TableRef{}, fmt.Errorf("initialising nftables: %w", err)
-	}
+	tm.Create(nftables.VMap{
+		Name:        filtFwdInVMap,
+		ElementType: nftables.NftTypeIfname,
+	})
+	tm.Create(nftables.Rule{
+		Chain: forwardChain,
+		Group: initialRuleGroup,
+		Rule:  []string{"oifname vmap @", filtFwdInVMap},
+	})
+
+	tm.Create(nftables.VMap{
+		Name:        filtFwdOutVMap,
+		ElementType: nftables.NftTypeIfname,
+	})
+	tm.Create(nftables.Rule{
+		Chain: forwardChain,
+		Group: initialRuleGroup,
+		Rule:  []string{"iifname vmap @", filtFwdOutVMap},
+	})
 
 	// Set up the NAT postrouting base chain.
 	//
 	// Like the filter-forward chain, its only rules are jumps to network-specific ingress and egress chains.
-	natPostRtChain, err := table.BaseChain(ctx, postroutingChain,
-		nftables.BaseChainTypeNAT,
-		nftables.BaseChainHookPostrouting,
-		nftables.BaseChainPrioritySrcNAT)
-	if err != nil {
-		return nftables.TableRef{}, err
-	}
-	_ = table.InterfaceVMap(ctx, natPostroutingOutVMap)
-	if err := natPostRtChain.AppendRule(ctx, initialRuleGroup, "iifname vmap @"+natPostroutingOutVMap); err != nil {
-		return nftables.TableRef{}, fmt.Errorf("initialising nftables: %w", err)
-	}
-	_ = table.InterfaceVMap(ctx, natPostroutingInVMap)
-	if err := natPostRtChain.AppendRule(ctx, initialRuleGroup, "oifname vmap @"+natPostroutingInVMap); err != nil {
-		return nftables.TableRef{}, fmt.Errorf("initialising nftables: %w", err)
-	}
+	tm.Create(nftables.BaseChain{
+		Name:      postroutingChain,
+		ChainType: nftables.BaseChainTypeNAT,
+		Hook:      nftables.BaseChainHookPostrouting,
+		Priority:  nftables.BaseChainPrioritySrcNAT,
+	})
+
+	tm.Create(nftables.VMap{
+		Name:        natPostroutingOutVMap,
+		ElementType: nftables.NftTypeIfname,
+	})
+	tm.Create(nftables.Rule{
+		Chain: postroutingChain,
+		Group: initialRuleGroup,
+		Rule:  []string{"iifname vmap @", natPostroutingOutVMap},
+	})
+
+	tm.Create(nftables.VMap{
+		Name:        natPostroutingInVMap,
+		ElementType: nftables.NftTypeIfname,
+	})
+	tm.Create(nftables.Rule{
+		Chain: postroutingChain,
+		Group: initialRuleGroup,
+		Rule:  []string{"oifname vmap @", natPostroutingInVMap},
+	})
 
 	// Instantiate natChain, for the NAT prerouting and output base chains to jump to.
-	_ = table.Chain(ctx, natChain)
+	tm.Create(nftables.Chain{
+		Name: natChain,
+	})
 
 	// Set up the NAT prerouting base chain.
-	natPreRtChain, err := table.BaseChain(ctx, preroutingChain,
-		nftables.BaseChainTypeNAT,
-		nftables.BaseChainHookPrerouting,
-		nftables.BaseChainPriorityDstNAT)
-	if err != nil {
-		return nftables.TableRef{}, err
-	}
-	if err := natPreRtChain.AppendRule(ctx, initialRuleGroup, "fib daddr type local counter jump "+natChain); err != nil {
-		return nftables.TableRef{}, fmt.Errorf("initialising nftables: %w", err)
-	}
+	tm.Create(nftables.BaseChain{
+		Name:      preroutingChain,
+		ChainType: nftables.BaseChainTypeNAT,
+		Hook:      nftables.BaseChainHookPrerouting,
+		Priority:  nftables.BaseChainPriorityDstNAT,
+	})
+	tm.Create(nftables.Rule{
+		Chain: preroutingChain,
+		Group: initialRuleGroup,
+		Rule:  []string{"fib daddr type local counter jump", natChain},
+	})
 
 	// Set up the NAT output base chain
-	natOutputChain, err := table.BaseChain(ctx, outputChain,
-		nftables.BaseChainTypeNAT,
-		nftables.BaseChainHookOutput,
-		nftables.BaseChainPriorityDstNAT)
-	if err != nil {
-		return nftables.TableRef{}, err
-	}
+	tm.Create(nftables.BaseChain{
+		Name:      outputChain,
+		ChainType: nftables.BaseChainTypeNAT,
+		Hook:      nftables.BaseChainHookOutput,
+		Priority:  nftables.BaseChainPriorityDstNAT,
+	})
+
 	// For output, don't jump to the NAT chain if hairpin is enabled (no userland proxy).
 	var skipLoopback string
 	if !nft.config.Hairpin {
@@ -170,32 +181,35 @@ func (nft *Nftabler) init(ctx context.Context, family nftables.Family) (nftables
 			skipLoopback = "ip6 daddr != ::1 "
 		}
 	}
-	if err := natOutputChain.AppendRule(ctx, initialRuleGroup, skipLoopback+"fib daddr type local counter jump "+natChain); err != nil {
-		return nftables.TableRef{}, fmt.Errorf("initialising nftables: %w", err)
-	}
+	tm.Create(nftables.Rule{
+		Chain: outputChain,
+		Group: initialRuleGroup,
+		Rule:  []string{skipLoopback, "fib daddr type local counter jump", natChain},
+	})
 
 	// Set up the raw prerouting base chain
-	if _, err := table.BaseChain(ctx, rawPreroutingChain,
-		nftables.BaseChainTypeFilter,
-		nftables.BaseChainHookPrerouting,
-		nftables.BaseChainPriorityRaw); err != nil {
-		return nftables.TableRef{}, err
+	tm.Create(nftables.BaseChain{
+		Name:      rawPreroutingChain,
+		ChainType: nftables.BaseChainTypeFilter,
+		Hook:      nftables.BaseChainHookPrerouting,
+		Priority:  nftables.BaseChainPriorityRaw,
+	})
+
+	// WSL2 does not (currently) support Windows<->Linux communication via ::1.
+	if !nft.config.Hairpin && nft.config.WSL2Mirrored && table.Family() == nftables.IPv4 {
+		mirroredWSL2Workaround(&tm)
 	}
 
-	if !nft.config.Hairpin && nft.config.WSL2Mirrored {
-		if err := mirroredWSL2Workaround(ctx, table); err != nil {
-			return nftables.TableRef{}, err
+	if err := table.Apply(ctx, tm); err != nil {
+		if family == nftables.IPv4 {
+			return nftables.Table{}, err
 		}
+		// Perhaps the kernel has no IPv6 support. It won't be possible to create IPv6
+		// networks without enabling ip6_tables in the kernel, or disabling ip6tables in
+		// the daemon config. But, allow the daemon to start because IPv4 will work. So,
+		// log the problem, and continue.
+		log.G(ctx).WithError(err).Warn("ip6tables is enabled, but cannot set up IPv6 nftables table")
+		return nftables.Table{}, nil
 	}
-
 	return table, nil
 }
-
-func nftApply(ctx context.Context, table nftables.TableRef) error {
-	ctx, span := otel.Tracer("").Start(ctx, spanPrefix+".nftApply."+string(table.Family()))
-	defer span.End()
-	if err := table.Apply(ctx); err != nil {
-		return fmt.Errorf("applying nftables rules: %w", err)
-	}
-	return nil
-}

daemon/libnetwork/drivers/bridge/internal/nftabler/port.go

@@ -4,23 +4,15 @@ package nftabler
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net"
 	"strconv"
 
 	"github.com/containerd/log"
-	"github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge/internal/firewaller"
 	"github.com/moby/moby/v2/daemon/libnetwork/internal/nftables"
 	"github.com/moby/moby/v2/daemon/libnetwork/types"
 )
 
-type pbContext struct {
-	table nftables.TableRef
-	conf  firewaller.NetworkConfigFam
-	ipv   firewaller.IPVersion
-}
-
 func (n *network) AddPorts(ctx context.Context, pbs []types.PortBinding) error {
 	return n.modPorts(ctx, pbs, true)
 }
@@ -42,14 +34,12 @@ func (n *network) modPorts(ctx context.Context, pbs []types.PortBinding, enable
 
 	pbs4, pbs6 := splitByContainerFam(pbs)
 	if n.fw.config.IPv4 && n.config.Config4.Prefix.IsValid() {
-		pbc := pbContext{table: n.fw.table4, conf: n.config.Config4, ipv: firewaller.IPv4}
-		if err := n.setPerPortRules(ctx, pbs4, pbc, enable); err != nil {
+		if err := n.setPerPortRules(ctx, pbs4, n.fw.table4, nftables.IPv4, n.config.Config4.Unprotected, enable); err != nil {
 			return err
 		}
 	}
 	if n.fw.config.IPv6 && n.config.Config6.Prefix.IsValid() {
-		pbc := pbContext{table: n.fw.table6, conf: n.config.Config6, ipv: firewaller.IPv6}
-		if err := n.setPerPortRules(ctx, pbs6, pbc, enable); err != nil {
+		if err := n.setPerPortRules(ctx, pbs6, n.fw.table6, nftables.IPv6, n.config.Config6.Unprotected, enable); err != nil {
 			return err
 		}
 	}
@@ -68,30 +58,26 @@ func splitByContainerFam(pbs []types.PortBinding) ([]types.PortBinding, []types.
 	return pbs4, pbs6
 }
 
-func (n *network) setPerPortRules(ctx context.Context, pbs []types.PortBinding, pbc pbContext, enable bool) error {
-	if err := n.setPerPortForwarding(ctx, pbs, pbc, enable); err != nil {
-		return err
+func (n *network) setPerPortRules(ctx context.Context, pbs []types.PortBinding, table nftables.Table, ipv nftables.Family, unprotected, enable bool) error {
+	tm := nftables.Modifier{}
+	updater := tm.Create
+	if !enable {
+		updater = tm.Delete
 	}
-	if err := n.setPerPortDNAT(ctx, pbs, pbc, enable); err != nil {
-		return err
+	if !unprotected {
+		n.setPerPortForwarding(pbs, updater, ipv)
 	}
-	if err := n.setPerPortHairpinMasq(ctx, pbs, pbc, enable); err != nil {
-		return err
-	}
-	if err := n.filterPortMappedOnLoopback(ctx, pbs, pbc, enable); err != nil {
-		return err
-	}
-	if err := nftApply(ctx, pbc.table); err != nil {
+	n.setPerPortDNAT(pbs, updater, ipv)
+	n.setPerPortHairpinMasq(pbs, updater, ipv)
+	n.filterPortMappedOnLoopback(pbs, updater, ipv)
+	if err := table.Apply(ctx, tm); err != nil {
 		return fmt.Errorf("adding rules for bridge %s: %w", n.config.IfName, err)
 	}
 	return nil
 }
 
-func (n *network) setPerPortForwarding(ctx context.Context, pbs []types.PortBinding, pbc pbContext, enable bool) error {
-	if pbc.conf.Unprotected {
-		return nil
-	}
-	updateFwdIn := pbc.table.ChainUpdateFunc(ctx, chainFilterFwdIn(n.config.IfName), enable)
+func (n *network) setPerPortForwarding(pbs []types.PortBinding, updater func(nftables.Obj), ipv nftables.Family) {
+	chainName := chainFilterFwdIn(n.config.IfName)
 	for _, pb := range pbs {
 		// When more than one host port is mapped to a single container port, this will
 		// generate the same rule for each host port. So, ignore duplicates when adding,
@@ -101,24 +87,25 @@ func (n *network) setPerPortForwarding(ctx context.Context, pbs []types.PortBind
 		// also be deleted more than once.)
 		//
 		// TODO(robmry) - track port mappings, use that to edit nftables sets when bindings are added/removed.
-		rule := fmt.Sprintf("%s daddr %s %s dport %d counter accept", pbc.table.Family(), pb.IP, pb.Proto, pb.Port)
-		if err := updateFwdIn(ctx, fwdInPortsRuleGroup, rule); err != nil &&
-			!errors.Is(err, nftables.ErrRuleExist) && !errors.Is(err, nftables.ErrRuleNotExist) {
-			return fmt.Errorf("updating forwarding rule for port %s %s:%d/%s on %s, enable=%v: %w",
-				pbc.table.Family(), pb.IP, pb.Port, pb.Proto, n.config.IfName, enable, err)
-		}
+		updater(nftables.Rule{
+			Chain: chainName,
+			Group: fwdInPortsRuleGroup,
+			Rule: []string{
+				string(ipv), "daddr", pb.IP.String(), pb.Proto.String(),
+				"dport", strconv.Itoa(int(pb.Port)), "counter accept",
+			},
+			IgnoreExist: true,
+		})
 	}
-	return nil
 }
 
-func (n *network) setPerPortDNAT(ctx context.Context, pbs []types.PortBinding, pbc pbContext, enable bool) error {
-	updater := pbc.table.ChainUpdateFunc(ctx, natChain, enable)
+func (n *network) setPerPortDNAT(pbs []types.PortBinding, updater func(nftables.Obj), ipv nftables.Family) {
 	var proxySkip string
 	if !n.fw.config.Hairpin {
 		proxySkip = fmt.Sprintf("iifname != %s ", n.config.IfName)
 	}
 	var v6LLSkip string
-	if pbc.ipv == firewaller.IPv6 {
+	if ipv == nftables.IPv6 {
 		v6LLSkip = "ip6 saddr != fe80::/10 "
 	}
 	for _, pb := range pbs {
@@ -133,26 +120,26 @@ func (n *network) setPerPortDNAT(ctx context.Context, pbs []types.PortBinding, p
 		}
 		var daddrMatch string
 		if !pb.HostIP.IsUnspecified() {
-			daddrMatch = fmt.Sprintf("%s daddr %s ", pbc.table.Family(), pb.HostIP)
-		}
-		rule := fmt.Sprintf("%s%s%s%s dport %d counter dnat to %s comment DNAT",
-			proxySkip, v6LLSkip, daddrMatch, pb.Proto, pb.HostPort,
-			net.JoinHostPort(pb.IP.String(), strconv.Itoa(int(pb.Port))))
-		if err := updater(ctx, initialRuleGroup, rule); err != nil {
-			return fmt.Errorf("adding DNAT for %s %s:%d -> %s:%d/%s on %s: %w",
-				pbc.table.Family(), pb.HostIP, pb.HostPort, pb.IP, pb.Port, pb.Proto, n.config.IfName, err)
+			daddrMatch = fmt.Sprintf("%s daddr %s ", ipv, pb.HostIP)
 		}
+		updater(nftables.Rule{
+			Chain: natChain,
+			Group: initialRuleGroup,
+			Rule: []string{
+				proxySkip, v6LLSkip, daddrMatch, pb.Proto.String(), "dport", strconv.Itoa(int(pb.HostPort)), "counter dnat to",
+				net.JoinHostPort(pb.IP.String(), strconv.Itoa(int(pb.Port))), "comment DNAT",
+			},
+		})
 	}
-	return nil
 }
 
 // setPerPortHairpinMasq allows containers to access their own published ports on the host
 // when hairpin is enabled (no docker-proxy), by masquerading.
-func (n *network) setPerPortHairpinMasq(ctx context.Context, pbs []types.PortBinding, pbc pbContext, enable bool) error {
+func (n *network) setPerPortHairpinMasq(pbs []types.PortBinding, updater func(nftables.Obj), ipv nftables.Family) {
 	if !n.fw.config.Hairpin {
-		return nil
+		return
 	}
-	updater := pbc.table.ChainUpdateFunc(ctx, chainNatPostRtIn(n.config.IfName), enable)
+	chainName := chainNatPostRtIn(n.config.IfName)
 	for _, pb := range pbs {
 		// Nothing to do if NAT is disabled.
 		if pb.HostPort == 0 {
@@ -171,15 +158,17 @@ func (n *network) setPerPortHairpinMasq(ctx context.Context, pbs []types.PortBin
 		// than once.)
 		//
 		// TODO(robmry) - track port mappings, use that to edit nftables sets when bindings are added/removed.
-		rule := fmt.Sprintf(`%s saddr %s %s daddr %s %s dport %d counter masquerade comment "MASQ TO OWN PORT"`,
-			pbc.table.Family(), pb.IP, pbc.table.Family(), pb.IP, pb.Proto, pb.Port)
-		if err := updater(ctx, initialRuleGroup, rule); err != nil &&
-			!errors.Is(err, nftables.ErrRuleExist) && !errors.Is(err, nftables.ErrRuleNotExist) {
-			return fmt.Errorf("adding MASQ TO OWN PORT for %d -> %s:%d/%s: %w",
-				pb.Port, pb.IP, pb.Port, pb.Proto, err)
-		}
+		updater(nftables.Rule{
+			Chain: chainName,
+			Group: initialRuleGroup,
+			Rule: []string{
+				string(ipv), "saddr", pb.IP.String(), string(ipv),
+				"daddr", pb.IP.String(), pb.Proto.String(),
+				"dport", strconv.Itoa(int(pb.Port)),
+				`counter masquerade comment "MASQ TO OWN PORT"`,
+			},
+		})
 	}
-	return nil
 }
 
 // filterPortMappedOnLoopback adds a rule that drops remote connections to ports
@@ -188,11 +177,10 @@ func (n *network) setPerPortHairpinMasq(ctx context.Context, pbs []types.PortBin
 // This is a no-op if the portBinding is for IPv6 (IPv6 loopback address is
 // non-routable), or over a network with gw_mode=routed (PBs in routed mode
 // don't map ports on the host).
-func (n *network) filterPortMappedOnLoopback(ctx context.Context, pbs []types.PortBinding, pbc pbContext, enable bool) error {
-	if pbc.ipv == firewaller.IPv6 {
-		return nil
+func (n *network) filterPortMappedOnLoopback(pbs []types.PortBinding, updater func(nftables.Obj), ipv nftables.Family) {
+	if ipv == nftables.IPv6 {
+		return
 	}
-	updater := pbc.table.ChainUpdateFunc(ctx, rawPreroutingChain, enable)
 	for _, pb := range pbs {
 		// Nothing to do if not binding to the loopback address.
 		if pb.HostPort == 0 || !pb.HostIP.IsLoopback() {
@@ -203,18 +191,24 @@ func (n *network) filterPortMappedOnLoopback(ctx context.Context, pbs []types.Po
 			continue
 		}
 		if n.fw.config.WSL2Mirrored {
-			if err := updater(ctx, rawPreroutingPortsRuleGroup,
-				`iifname loopback0 ip daddr %s %s dport %d counter accept comment "%s"`,
-				pb.HostIP, pb.Proto, pb.HostPort, "ACCEPT WSL2 LOOPBACK"); err != nil {
-				return fmt.Errorf("adding WSL2 loopback rule for %d: %w", pb.HostPort, err)
-			}
-		}
-		if err := updater(ctx, rawPreroutingPortsRuleGroup,
-			`iifname != lo ip daddr %s %s dport %d counter drop comment "DROP REMOTE LOOPBACK"`,
-			pb.HostIP, pb.Proto, pb.HostPort); err != nil {
-			return fmt.Errorf("adding loopback filter rule for %d: %w", pb.HostPort, err)
+			updater(nftables.Rule{
+				Chain: rawPreroutingChain,
+				Group: rawPreroutingPortsRuleGroup,
+				Rule: []string{
+					"iifname loopback0 ip daddr", pb.HostIP.String(), pb.Proto.String(),
+					"dport", strconv.Itoa(int(pb.HostPort)),
+					`counter accept comment "ACCEPT WSL2 LOOPBACK"`,
+				},
+			})
 		}
+		updater(nftables.Rule{
+			Chain: rawPreroutingChain,
+			Group: rawPreroutingPortsRuleGroup,
+			Rule: []string{
+				`iifname != lo ip daddr`, pb.HostIP.String(), pb.Proto.String(),
+				"dport", strconv.Itoa(int(pb.HostPort)),
+				`counter drop comment "DROP REMOTE LOOPBACK"`,
+			},
+		})
 	}
-
-	return nil
 }

daemon/libnetwork/drivers/bridge/internal/nftabler/wsl2.go

@@ -3,8 +3,6 @@
 package nftabler
 
 import (
-	"context"
-
 	"github.com/moby/moby/v2/daemon/libnetwork/internal/nftables"
 )
 
@@ -39,11 +37,10 @@ import (
 // arriving from any other bridge network. Similarly, this function adds (or
 // removes) a rule to RETURN early for packets delivered via loopback0 with
 // destination 127.0.0.0/8.
-func mirroredWSL2Workaround(ctx context.Context, table nftables.TableRef) error {
-	// WSL2 does not (currently) support Windows<->Linux communication via ::1.
-	if table.Family() != nftables.IPv4 {
-		return nil
-	}
-	return table.Chain(ctx, natChain).AppendRule(ctx,
-		initialRuleGroup, `iifname "loopback0" ip daddr 127.0.0.0/8 counter return`)
+func mirroredWSL2Workaround(tm *nftables.Modifier) {
+	tm.Create(nftables.Rule{
+		Chain: natChain,
+		Group: initialRuleGroup,
+		Rule:  []string{`iifname "loopback0" ip daddr 127.0.0.0/8 counter return`},
+	})
 }

daemon/libnetwork/internal/nftables/nftables_linux_test.go

@@ -31,11 +31,20 @@ func testSetup(t *testing.T) func() {
 	}
 }
 
-func applyAndCheck(t *testing.T, tbl TableRef, goldenFilename string) {
+func applyAndCheck(t *testing.T, tbl Table, tm Modifier, goldenFilename string) {
 	t.Helper()
-	err := tbl.Apply(context.Background())
+	err := tbl.Apply(context.Background(), tm)
 	assert.Check(t, err)
-	res := icmd.RunCommand("nft", "list", "ruleset")
+	res := icmd.RunCommand("nft", "list", "table", string(tbl.Family()), tbl.Name())
+	res.Assert(t, icmd.Success)
+	golden.Assert(t, res.Combined(), goldenFilename)
+}
+
+func reloadAndCheck(t *testing.T, tbl Table, ipv Family, goldenFilename string) {
+	t.Helper()
+	err := tbl.Reload(context.Background())
+	assert.Check(t, err)
+	res := icmd.RunCommand("nft", "list", "table", string(ipv), tbl.t.Name)
 	res.Assert(t, icmd.Success)
 	golden.Assert(t, res.Combined(), goldenFilename)
 }
@@ -48,17 +57,13 @@ func TestTable(t *testing.T) {
 	tbl6, err := NewTable(IPv6, "ipv6_table")
 	assert.NilError(t, err)
 
-	assert.Check(t, is.Equal(tbl4.Family(), IPv4))
-	assert.Check(t, is.Equal(tbl6.Family(), IPv6))
-
 	// Update nftables and check what happened.
-	applyAndCheck(t, tbl4, t.Name()+"/created4.golden")
-	applyAndCheck(t, tbl6, t.Name()+"/created46.golden")
+	applyAndCheck(t, tbl4, Modifier{}, t.Name()+"/created4.golden")
+	applyAndCheck(t, tbl6, Modifier{}, t.Name()+"/created6.golden")
 }
 
 func TestChain(t *testing.T) {
 	defer testSetup(t)()
-	ctx := context.Background()
 
 	// Create a table.
 	tbl, err := NewTable(IPv4, "this_is_a_table")
@@ -66,143 +71,134 @@ func TestChain(t *testing.T) {
 
 	// Create a base chain.
 	const bcName = "this_is_a_base_chain"
-	bc1, err := tbl.BaseChain(ctx, bcName, BaseChainTypeFilter, BaseChainHookForward, BaseChainPriorityFilter+10)
-	assert.NilError(t, err)
-
-	// Check that it's an error to add a new base chain with the same name.
-	_, err = tbl.BaseChain(ctx, bcName, BaseChainTypeNAT, BaseChainHookPrerouting, BaseChainPriorityDstNAT)
-	assert.Check(t, is.ErrorContains(err, "already exists"))
-
-	// Add a rule.
-	err = bc1.AppendRule(ctx, 0, "counter")
-	assert.NilError(t, err)
+	tm := Modifier{}
+	bcDesc := BaseChain{
+		Name:      bcName,
+		ChainType: BaseChainTypeFilter,
+		Hook:      BaseChainHookForward,
+		Priority:  BaseChainPriorityFilter + 10,
+		Policy:    "accept",
+	}
+	tm.Create(bcDesc)
+	// Add a rule to the base chain.
+	bcCounterRule := Rule{Chain: bcName, Group: 0, Rule: []string{"counter"}}
+	tm.Create(bcCounterRule)
 
 	// Add a regular chain.
 	const regularChainName = "this_is_a_regular_chain"
-	_ = tbl.Chain(ctx, regularChainName)
+	cDesc := Chain{Name: regularChainName}
+	tm.Create(cDesc)
+	// Add a rule to the regular chain.
+	cRule := Rule{Chain: regularChainName, Group: 0, Rule: []string{"counter", "accept"}}
+	tm.Create(cRule)
 
-	// Add a rule to the regular chain, use string formatting and a func retrieved
-	// from the table.
-	f := tbl.ChainUpdateFunc(ctx, regularChainName, true)
-	err = f(ctx, 0, "counter %s", "accept")
-	assert.Check(t, err)
-
-	// Fetch the base chain by name.
-	bc1 = tbl.Chain(ctx, bcName)
-
-	// Add another rule to the base chain, using the newly-retrieved handle.
-	err = bc1.AppendRule(ctx, 0, "jump %s", regularChainName)
-	assert.Check(t, err)
+	// Add another rule to the base chain.
+	bcJumpRule := Rule{Chain: bcName, Group: 0, Rule: []string{"jump", regularChainName}}
+	tm.Create(bcJumpRule)
 
 	// Update nftables and check what happened.
-	applyAndCheck(t, tbl, t.Name()+"/created.golden")
+	applyAndCheck(t, tbl, tm, t.Name()+"/created.golden")
 
 	// Delete a rule from the base chain.
-	f = tbl.ChainUpdateFunc(ctx, bcName, false)
-	err = f(ctx, 0, "counter")
-	assert.Check(t, err)
-
-	// Check it's an error to delete that rule again. This time, call the delete
-	// function directly on a newly retrieved handle.
-	err = tbl.Chain(ctx, bcName).DeleteRule(ctx, 0, "counter")
-	assert.Check(t, is.ErrorContains(err, "does not exist"))
-
-	// Update the base chain's policy.
-	err = tbl.Chain(ctx, bcName).SetPolicy("drop")
-	assert.Check(t, err)
-
-	// Check it's an error to set a policy on a regular chain.
-	err = tbl.Chain(ctx, regularChainName).SetPolicy("drop")
-	assert.Check(t, is.ErrorContains(err, "not a base chain"))
+	tm = Modifier{}
+	tm.Delete(bcCounterRule)
 
 	// Update nftables and check what happened.
-	applyAndCheck(t, tbl, t.Name()+"/modified.golden")
+	applyAndCheck(t, tbl, tm, t.Name()+"/modified.golden")
 
 	// Delete the base chain.
-	err = tbl.DeleteChain(ctx, bcName)
-	assert.Check(t, err)
-
-	// Delete the regular chain.
-	err = tbl.DeleteChain(ctx, regularChainName)
-	assert.Check(t, err)
-
-	// Check that it's an error to delete it again.
-	err = tbl.DeleteChain(ctx, regularChainName)
-	assert.Check(t, is.ErrorContains(err, "does not exist"))
+	tm = Modifier{}
+	tm.Delete(bcJumpRule)
+	tm.Delete(bcDesc)
+	tm.Delete(cRule)
+	tm.Delete(cDesc)
 
 	// Update nftables and check what happened.
-	applyAndCheck(t, tbl, t.Name()+"/deleted.golden")
+	applyAndCheck(t, tbl, tm, t.Name()+"/deleted.golden")
 }
 
 func TestChainRuleGroups(t *testing.T) {
 	defer testSetup(t)()
-	ctx := context.Background()
 
 	tbl, err := NewTable(IPv4, "testtable")
 	assert.NilError(t, err)
-	c := tbl.Chain(ctx, "testchain")
-	err = c.AppendRule(ctx, 100, "hello100")
-	assert.Check(t, err)
-	err = c.AppendRule(ctx, 200, "hello200")
-	assert.Check(t, err)
-	err = c.AppendRule(ctx, 100, "hello101")
-	assert.Check(t, err)
-	err = c.AppendRule(ctx, 200, "hello201")
-	assert.Check(t, err)
-	err = c.AppendRule(ctx, 100, "hello102")
-	assert.Check(t, err)
+	tm := Modifier{}
+	chainName := "testchain"
+	tm.Create(Chain{Name: chainName})
+	tm.Create(Rule{Chain: chainName, Group: 100, Rule: []string{"iifname hello100 counter"}})
+	tm.Create(Rule{Chain: chainName, Group: 200, Rule: []string{"iifname hello200 counter"}})
+	tm.Create(Rule{Chain: chainName, Group: 100, Rule: []string{"iifname hello101 counter"}})
+	tm.Create(Rule{Chain: chainName, Group: 200, Rule: []string{"iifname hello201 counter"}})
+	tm.Create(Rule{Chain: chainName, Group: 100, Rule: []string{"iifname hello102 counter"}})
+	applyAndCheck(t, tbl, tm, t.Name()+".golden")
+}
 
-	assert.Check(t, is.DeepEqual(c.c.Rules(), []string{
-		"hello100", "hello101", "hello102",
-		"hello200", "hello201",
-	}))
+func TestIgnoreExist(t *testing.T) {
+	defer testSetup(t)()
+	tbl, err := NewTable(IPv4, "this_is_a_table")
+	assert.NilError(t, err)
+	tm := Modifier{}
+
+	// Create a chain with a single rule, add the rule again but drop the duplicate.
+	const chainName = "this_is_a_chain"
+	tm.Create(Chain{Name: chainName})
+	tm.Create(Rule{Chain: chainName, Rule: []string{"counter"}})
+	tm.Create(Rule{Chain: chainName, Rule: []string{"counter"}, IgnoreExist: true})
+	applyAndCheck(t, tbl, tm, t.Name()+"/created.golden")
+
+	// Add the rule again, ignoring the duplicate, but in a modifier that has an
+	// error - check that the existing rule isn't removed by rollback of this modifier.
+	tmErr := Modifier{}
+	tmErr.Create(Rule{Chain: chainName, Rule: []string{"counter"}, IgnoreExist: true})
+	tmErr.Create(Rule{Chain: chainName})
+	err = tbl.Apply(context.Background(), tm)
+	assert.Check(t, err != nil, "Expected an error")
+
+	// Reload, to flush table state.
+	reloadAndCheck(t, tbl, IPv4, t.Name()+"/created.golden")
+
+	// Delete the rule.
+	tmDel := Modifier{}
+	tmDel.Delete(Rule{Chain: chainName, Rule: []string{"counter"}})
+	applyAndCheck(t, tbl, tmDel, t.Name()+"/deleted.golden")
+
+	// Delete it again, in another chain that will roll back, to check it's not resurrected.
+	tmReDel := Modifier{}
+	tmReDel.Delete(Rule{Chain: chainName, Rule: []string{"counter"}, IgnoreExist: true})
+	tmReDel.Create(Rule{Chain: chainName})
+	err = tbl.Apply(context.Background(), tmReDel)
+	assert.Check(t, err != nil, "Expected an error")
+
+	// Reload, to flush table state.
+	reloadAndCheck(t, tbl, IPv4, t.Name()+"/deleted.golden")
 }
 
 func TestVMap(t *testing.T) {
 	defer testSetup(t)()
-	ctx := context.Background()
 
 	// Create a table.
 	tbl, err := NewTable(IPv6, "this_is_a_table")
 	assert.NilError(t, err)
+	tm := Modifier{}
 
 	// Create a verdict map.
 	const mapName = "this_is_a_vmap"
-	m := tbl.InterfaceVMap(ctx, mapName)
-
-	// Add an element.
-	err = m.AddElement(ctx, "eth0", "return")
-	assert.Check(t, err)
-
-	// Check that it's an error to add the element again.
-	err = m.AddElement(ctx, "eth0", "return")
-	assert.Check(t, is.ErrorContains(err, "already contains element"))
-
-	// Fetch the existing vmap.
-	m = tbl.InterfaceVMap(ctx, mapName)
-
-	// Add another element.
-	err = m.AddElement(ctx, "eth1", "drop")
-	assert.Check(t, err)
+	tm.Create(VMap{Name: mapName, ElementType: NftTypeIfname})
+	tm.Create(VMapElement{VmapName: mapName, Key: "eth0", Verdict: "return"})
+	tm.Create(VMapElement{VmapName: mapName, Key: "eth1", Verdict: "drop"})
 
 	// Update nftables and check what happened.
-	applyAndCheck(t, tbl, t.Name()+"/created.golden")
+	applyAndCheck(t, tbl, tm, t.Name()+"/created.golden")
 
-	// Delete an element.
-	err = m.DeleteElement(ctx, "eth1")
-	assert.Check(t, err)
-
-	// Check it's an error to delete it again.
-	err = m.DeleteElement(ctx, "eth1")
-	assert.Check(t, is.ErrorContains(err, "does not contain element"))
+	// Undo those changes by reversing the commands.
+	tmRev := tm.Reverse()
 
 	// Update nftables and check what happened.
-	applyAndCheck(t, tbl, t.Name()+"/deleted.golden")
+	applyAndCheck(t, tbl, tmRev, t.Name()+"/deleted.golden")
 }
 
 func TestSet(t *testing.T) {
 	defer testSetup(t)()
-	ctx := context.Background()
 
 	// Create v4 and v6 tables.
 	tbl4, err := NewTable(IPv4, "table4")
@@ -211,62 +207,55 @@ func TestSet(t *testing.T) {
 	assert.NilError(t, err)
 
 	// Create a set in each table.
-	s4 := tbl4.PrefixSet(ctx, "set4")
-	s6 := tbl6.PrefixSet(ctx, "set6")
+	const set4Name = "set4"
+	tm4 := Modifier{}
+	tm4.Create(Set{Name: set4Name, ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}})
+	const set6Name = "set6"
+	tm6 := Modifier{}
+	tm6.Create(Set{Name: set6Name, ElementType: NftTypeIPv6Addr, Flags: []string{"interval"}})
 
 	// Add elements to each set.
-	err = s4.AddElement(ctx, "192.0.2.1/24")
-	assert.Check(t, err)
-	err = s6.AddElement(ctx, "2001:db8::1/64")
-	assert.Check(t, err)
-
-	// Check it's an error to add those elements again.
-	err = s4.AddElement(ctx, "192.0.2.1/24")
-	assert.Check(t, is.ErrorContains(err, "already contains element"))
-	err = s6.AddElement(ctx, "2001:db8::1/64")
-	assert.Check(t, is.ErrorContains(err, "already contains element"))
+	tm4.Create(SetElement{SetName: set4Name, Element: "192.0.2.0/24"})
+	tm6.Create(SetElement{SetName: set6Name, Element: "2001:db8::/64"})
 
 	// Update nftables and check what happened.
-	applyAndCheck(t, tbl4, t.Name()+"/created4.golden")
-	applyAndCheck(t, tbl6, t.Name()+"/created46.golden")
+	applyAndCheck(t, tbl4, tm4, t.Name()+"/created4.golden")
+	applyAndCheck(t, tbl6, tm6, t.Name()+"/created6.golden")
 
 	// Delete elements.
-	err = s4.DeleteElement(ctx, "192.0.2.1/24")
-	assert.Check(t, err)
-	err = s6.DeleteElement(ctx, "2001:db8::1/64")
-	assert.Check(t, err)
-
-	// Check it's an error to delete those elements again.
-	err = s4.DeleteElement(ctx, "192.0.2.1/24")
-	assert.Check(t, is.ErrorContains(err, "does not contain element"))
-	err = s6.DeleteElement(ctx, "2001:db8::1/64")
-	assert.Check(t, is.ErrorContains(err, "does not contain element"))
-
-	// Update nftables and check what happened.
-	applyAndCheck(t, tbl4, t.Name()+"/deleted4.golden")
-	applyAndCheck(t, tbl6, t.Name()+"/deleted46.golden")
+	applyAndCheck(t, tbl4, tm4.Reverse(), t.Name()+"/deleted4.golden")
+	applyAndCheck(t, tbl6, tm6.Reverse(), t.Name()+"/deleted6.golden")
 }
 
 func TestReload(t *testing.T) {
 	defer testSetup(t)()
-	ctx := context.Background()
 
 	// Create a table with some stuff in it.
 	const tableName = "this_is_a_table"
 	tbl, err := NewTable(IPv4, tableName)
 	assert.NilError(t, err)
-	bc, err := tbl.BaseChain(ctx, "a_base_chain", BaseChainTypeFilter, BaseChainHookForward, BaseChainPriorityFilter)
-	assert.NilError(t, err)
-	err = bc.AppendRule(ctx, 0, "counter")
-	assert.NilError(t, err)
-	m := tbl.InterfaceVMap(ctx, "this_is_a_vmap")
-	err = m.AddElement(ctx, "eth0", "return")
-	assert.Check(t, err)
-	err = m.AddElement(ctx, "eth1", "return")
-	assert.Check(t, err)
-	err = tbl.PrefixSet(ctx, "set4").AddElement(ctx, "192.0.2.0/24")
-	assert.Check(t, err)
-	applyAndCheck(t, tbl, t.Name()+"/created.golden")
+	tm := Modifier{}
+
+	const bcName = "a_base_chain"
+	tm.Create(BaseChain{
+		Name:      bcName,
+		ChainType: BaseChainTypeFilter,
+		Hook:      BaseChainHookForward,
+		Priority:  BaseChainPriorityFilter,
+		Policy:    "accept",
+	})
+	tm.Create(Rule{Chain: bcName, Group: 0, Rule: []string{"counter"}})
+
+	const vmapName = "this_is_a_vmap"
+	tm.Create(VMap{Name: vmapName, ElementType: NftTypeIfname})
+	tm.Create(VMapElement{VmapName: vmapName, Key: "eth0", Verdict: "return"})
+	tm.Create(VMapElement{VmapName: vmapName, Key: "eth1", Verdict: "return"})
+
+	const setName = "this_is_a_set"
+	tm.Create(Set{Name: setName, ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}})
+	tm.Create(SetElement{SetName: setName, Element: "192.0.2.0/24"})
+
+	applyAndCheck(t, tbl, tm, t.Name()+"/created.golden")
 
 	// Delete the underlying nftables table.
 	deleteTable := func() {
@@ -282,14 +271,371 @@ func TestReload(t *testing.T) {
 	// Reconstruct the nftables table.
 	err = tbl.Reload(context.Background())
 	assert.Check(t, err)
-	applyAndCheck(t, tbl, t.Name()+"/reloaded.golden")
+	res := icmd.RunCommand("nft", "list", "table", string(tbl.Family()), tbl.Name())
+	res.Assert(t, icmd.Success)
+	golden.Assert(t, res.Combined(), t.Name()+"/created.golden")
 
 	// Delete again.
 	deleteTable()
 
 	// Check implicit/recovery reload - only deleting something that's gone missing
 	// from a vmap/set will trigger this.
-	err = m.DeleteElement(ctx, "eth1")
-	assert.Check(t, err)
-	applyAndCheck(t, tbl, t.Name()+"/recovered.golden")
+	tm = Modifier{}
+	tm.Delete(SetElement{SetName: setName, Element: "192.0.2.0/24"})
+	applyAndCheck(t, tbl, tm, t.Name()+"/recovered.golden")
+}
+
+func TestValidation(t *testing.T) {
+	testcases := []struct {
+		name   string
+		cmds   []command
+		expErr string
+	}{
+		// BaseChain
+		{
+			name: "create with missing base chain name",
+			cmds: []command{
+				{obj: BaseChain{ChainType: BaseChainTypeNAT, Hook: BaseChainHookPostrouting, Priority: BaseChainPrioritySrcNAT}},
+			},
+			expErr: "base chain must have a name",
+		},
+		{
+			name: "create with missing base chain type",
+			cmds: []command{
+				{obj: BaseChain{Name: "achain", Hook: BaseChainHookPostrouting, Priority: BaseChainPrioritySrcNAT}},
+			},
+			expErr: "chain 'achain': fields ChainType and Hook are required",
+		},
+		{
+			name: "create with missing base chain hook",
+			cmds: []command{
+				{obj: BaseChain{Name: "achain", ChainType: BaseChainTypeNAT, Priority: BaseChainPrioritySrcNAT}},
+			},
+			expErr: "chain 'achain': fields ChainType and Hook are required",
+		},
+		{
+			name: "delete non-empty base chain",
+			cmds: []command{
+				{obj: BaseChain{
+					Name: "achain", ChainType: BaseChainTypeNAT, Hook: BaseChainHookPostrouting, Priority: BaseChainPrioritySrcNAT,
+				}},
+				{obj: Rule{Chain: "achain", Group: 0, Rule: []string{"counter"}}},
+				{
+					obj: BaseChain{
+						Name: "achain", ChainType: BaseChainTypeNAT, Hook: BaseChainHookPostrouting, Priority: BaseChainPrioritySrcNAT,
+					},
+					delete: true,
+				},
+			},
+			expErr: "cannot delete chain 'achain', it is not empty",
+		},
+		// Chain
+		{
+			name: "duplicate chain",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Chain{Name: "achain"}},
+			},
+			expErr: "already exists",
+		},
+		{
+			name: "delete missing chain",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}, delete: true},
+			},
+			expErr: "does not exist",
+		},
+		{
+			name: "missing chain name",
+			cmds: []command{
+				{obj: Chain{}},
+			},
+			expErr: "chain must have a name",
+		},
+		{
+			name: "delete non-empty chain",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}},
+				{obj: Chain{Name: "achain"}, delete: true},
+			},
+			expErr: "cannot delete chain 'achain', it is not empty",
+		},
+		// Rule
+		{
+			name: "bad rule",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Chain: "achain", Rule: []string{"this is nonsense"}}},
+			},
+			expErr: "syntax error",
+		},
+		{
+			name: "duplicate rule",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}},
+			},
+			expErr: "rule exists",
+		},
+		{
+			name: "delete missing rule",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}, delete: true},
+			},
+			expErr: "does not exist",
+		},
+		{
+			name: "duplicate rule delete",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}, delete: true},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}, delete: true},
+			},
+			expErr: "does not exist",
+		},
+		{
+			name: "create rule with missing chain name",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Rule: []string{"counter"}}},
+			},
+			expErr: "chain '' does not exist",
+		},
+		{
+			name: "delete rule with missing chain name",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Rule: []string{"counter"}}, delete: true},
+			},
+			expErr: "chain '' does not exist",
+		},
+		{
+			name: "create rule with nonexistent chain",
+			cmds: []command{
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}},
+			},
+			expErr: "chain 'achain' does not exist",
+		},
+		{
+			name: "delete rule with nonexistent chain",
+			cmds: []command{
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}, delete: true},
+			},
+			expErr: "chain 'achain' does not exist",
+		},
+		{
+			name: "create rule with no rule",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Chain: "achain"}},
+			},
+			expErr: "cannot add empty rule",
+		},
+		{
+			name: "delete rule with no rule",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Chain: "achain"}, delete: true},
+			},
+			expErr: "cannot delete empty rule",
+		},
+		{
+			name: "bad rule mid-sequence",
+			cmds: []command{
+				{obj: Chain{Name: "achain"}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}, delete: true},
+				{obj: Rule{Chain: "achain"}},
+				{obj: Rule{Chain: "achain", Rule: []string{"counter"}}},
+			},
+			expErr: "chain 'achain', cannot add empty rule",
+		},
+		// VMap
+		{
+			name: "duplicate vmap",
+			cmds: []command{
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}},
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}},
+			},
+			expErr: "vmap 'avmap' already exists",
+		},
+		{
+			name: "delete nonexistent vmap",
+			cmds: []command{
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}, delete: true},
+			},
+			expErr: "cannot delete vmap 'avmap', it does not exist",
+		},
+		{
+			name:   "missing vmap name",
+			cmds:   []command{{obj: VMap{ElementType: NftTypeIfname}}},
+			expErr: "vmap must have a name",
+		},
+		{
+			name:   "missing vmap element type",
+			cmds:   []command{{obj: VMap{Name: "avmap"}}},
+			expErr: "vmap 'avmap' has no element type",
+		},
+		{
+			name: "delete non-empty vmap",
+			cmds: []command{
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}},
+				{obj: VMapElement{VmapName: "avmap", Key: "eth0", Verdict: "drop"}},
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}, delete: true},
+			},
+			expErr: "cannot delete vmap 'avmap', it contains 1 elements",
+		},
+		// VMapElement
+		{
+			name: "duplicate vmap element",
+			cmds: []command{
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}},
+				{obj: VMapElement{VmapName: "avmap", Key: "eth0", Verdict: "drop"}},
+				{obj: VMapElement{VmapName: "avmap", Key: "eth0", Verdict: "drop"}},
+			},
+			expErr: "verdict map 'avmap' already contains element 'eth0'",
+		},
+		{
+			name: "add to vmap that does not exist",
+			cmds: []command{
+				{obj: VMapElement{VmapName: "avmap", Key: "eth0", Verdict: "drop"}},
+			},
+			expErr: "cannot add to vmap 'avmap', it does not exist",
+		},
+		{
+			name: "delete nonexistent vmap element",
+			cmds: []command{
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}},
+				{obj: VMapElement{VmapName: "avmap", Key: "eth0", Verdict: "drop"}, delete: true},
+			},
+			expErr: "verdict map 'avmap' does not contain element 'eth0'",
+		},
+		{
+			name: "vmap element with no named vmap",
+			cmds: []command{
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}},
+				{obj: VMapElement{Key: "eth0", Verdict: "drop"}},
+			},
+			expErr: "cannot add element to unnamed vmap",
+		},
+		{
+			name: "vmap element with no key",
+			cmds: []command{
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}},
+				{obj: VMapElement{VmapName: "avmap", Verdict: "drop"}},
+			},
+			expErr: "cannot add to vmap 'avmap', element must have key and verdict",
+		},
+		{
+			name: "vmap element with no verdict",
+			cmds: []command{
+				{obj: VMap{Name: "avmap", ElementType: NftTypeIfname}},
+				{obj: VMapElement{VmapName: "avmap", Key: "eth0"}},
+			},
+			expErr: "cannot add to vmap 'avmap', element must have key and verdict",
+		},
+		// Set
+		{
+			name: "duplicate set",
+			cmds: []command{
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+			},
+			expErr: "set 'aset' already exists",
+		},
+		{
+			name: "delete nonexistent set",
+			cmds: []command{
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}, delete: true},
+			},
+			expErr: "cannot delete set 'aset', it does not exist",
+		},
+		{
+			name: "missing set name",
+			cmds: []command{
+				{obj: Set{ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+			},
+			expErr: "set must have a name",
+		},
+		{
+			name: "missing set element type",
+			cmds: []command{
+				{obj: Set{Name: "aset", Flags: []string{"interval"}}},
+			},
+			expErr: "set 'aset' must have a type",
+		},
+		{
+			name: "delete non-empty set",
+			cmds: []command{
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+				{obj: SetElement{SetName: "aset", Element: "192.0.2.0/24"}},
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}, delete: true},
+			},
+			expErr: "cannot delete set 'aset', it contains 1 elements",
+		},
+		// SetElement
+		{
+			name: "duplicate set element",
+			cmds: []command{
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+				{obj: SetElement{SetName: "aset", Element: "192.0.2.0/24"}},
+				{obj: SetElement{SetName: "aset", Element: "192.0.2.0/24"}},
+			},
+			expErr: "set 'aset' already contains element '192.0.2.0/24'",
+		},
+		{
+			name: "delete nonexistent set element",
+			cmds: []command{
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+				{obj: SetElement{SetName: "aset", Element: "192.0.2.0/24"}, delete: true},
+			},
+			expErr: "cannot delete '192.0.2.0/24' from set 'aset', it does not exist",
+		},
+		{
+			name: "add set element to unnamed set",
+			cmds: []command{
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+				{obj: SetElement{Element: "192.0.2.0/24"}},
+			},
+			expErr: "cannot add to set '', it does not exist",
+		},
+		{
+			name: "add set element with no element",
+			cmds: []command{
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+				{obj: SetElement{SetName: "aset"}},
+			},
+			expErr: "cannot add to set 'aset', element not specified",
+		},
+		{
+			name: "mismatched set element type",
+			cmds: []command{
+				{obj: Set{Name: "aset", ElementType: NftTypeIPv4Addr, Flags: []string{"interval"}}},
+				{obj: SetElement{SetName: "aset", Element: "2001:db8::/64"}},
+			},
+			expErr: "Address family for hostname not supported",
+		},
+	}
+
+	testName := t.Name()
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			defer testSetup(t)()
+			tbl, err := NewTable(IPv4, "tablename")
+			assert.NilError(t, err)
+			tm := Modifier{cmds: tc.cmds}
+			err = tbl.Apply(context.Background(), tm)
+			assert.Check(t, err != nil, "expected error containing '%s'", tc.expErr)
+			assert.Check(t, is.ErrorContains(err, tc.expErr))
+			// Check the table wasn't created.
+			res := icmd.RunCommand("nft", "list", "table", string(IPv4), "tablename")
+			res.Assert(t, icmd.Expected{ExitCode: 1})
+			// Check the empty table can be created (the Table structure is still healthy).
+			applyAndCheck(t, tbl, Modifier{}, testName+"/empty.golden")
+		})
+	}
 }

daemon/libnetwork/internal/nftables/testdata/.gitattributes

@@ -0,0 +1 @@
+*.golden linguist-generated=true

daemon/libnetwork/internal/nftables/testdata/TestChain/modified.golden

@@ -1,6 +1,6 @@
 table ip this_is_a_table {
 	chain this_is_a_base_chain {
-		type filter hook forward priority filter + 10; policy drop;
+		type filter hook forward priority filter + 10; policy accept;
 		jump this_is_a_regular_chain
 	}
 

daemon/libnetwork/internal/nftables/testdata/TestChainRuleGroups.golden

@@ -0,0 +1,9 @@
+table ip testtable {
+	chain testchain {
+		iifname "hello100" counter packets 0 bytes 0
+		iifname "hello101" counter packets 0 bytes 0
+		iifname "hello102" counter packets 0 bytes 0
+		iifname "hello200" counter packets 0 bytes 0
+		iifname "hello201" counter packets 0 bytes 0
+	}
+}

daemon/libnetwork/internal/nftables/testdata/TestIgnoreExist/created.golden

@@ -0,0 +1,5 @@
+table ip this_is_a_table {
+	chain this_is_a_chain {
+		counter packets 0 bytes 0
+	}
+}

daemon/libnetwork/internal/nftables/testdata/TestIgnoreExist/deleted.golden

@@ -0,0 +1,4 @@
+table ip this_is_a_table {
+	chain this_is_a_chain {
+	}
+}

daemon/libnetwork/internal/nftables/testdata/TestReload/created.golden

@@ -5,7 +5,7 @@ table ip this_is_a_table {
 			     "eth1" : return }
 	}
 
-	set set4 {
+	set this_is_a_set {
 		type ipv4_addr
 		flags interval
 		elements = { 192.0.2.0/24 }

daemon/libnetwork/internal/nftables/testdata/TestReload/recovered.golden

@@ -1,13 +1,13 @@
 table ip this_is_a_table {
 	map this_is_a_vmap {
 		type ifname : verdict
-		elements = { "eth0" : return }
+		elements = { "eth0" : return,
+			     "eth1" : return }
 	}
 
-	set set4 {
+	set this_is_a_set {
 		type ipv4_addr
 		flags interval
-		elements = { 192.0.2.0/24 }
 	}
 
 	chain a_base_chain {

daemon/libnetwork/internal/nftables/testdata/TestSet/created6.golden

@@ -1,10 +1,3 @@
-table ip table4 {
-	set set4 {
-		type ipv4_addr
-		flags interval
-		elements = { 192.0.2.0/24 }
-	}
-}
 table ip6 table6 {
 	set set6 {
 		type ipv6_addr

daemon/libnetwork/internal/nftables/testdata/TestSet/deleted4.golden

@@ -1,13 +1,2 @@
 table ip table4 {
-	set set4 {
-		type ipv4_addr
-		flags interval
-	}
-}
-table ip6 table6 {
-	set set6 {
-		type ipv6_addr
-		flags interval
-		elements = { 2001:db8::/64 }
-	}
 }

daemon/libnetwork/internal/nftables/testdata/TestSet/deleted46.golden

@@ -1,12 +0,0 @@
-table ip table4 {
-	set set4 {
-		type ipv4_addr
-		flags interval
-	}
-}
-table ip6 table6 {
-	set set6 {
-		type ipv6_addr
-		flags interval
-	}
-}

daemon/libnetwork/internal/nftables/testdata/TestSet/deleted6.golden

@@ -0,0 +1,2 @@
+table ip6 table6 {
+}

daemon/libnetwork/internal/nftables/testdata/TestSetBaseChainPolicy/accept.golden

@@ -0,0 +1,8 @@
+table ip this_is_a_table {
+	chain aBaseChain {
+		type filter hook forward priority filter; policy accept;
+	}
+
+	chain aChain {
+	}
+}

daemon/libnetwork/internal/nftables/testdata/TestSetBaseChainPolicy/drop.golden

@@ -0,0 +1,8 @@
+table ip this_is_a_table {
+	chain aBaseChain {
+		type filter hook forward priority filter; policy drop;
+	}
+
+	chain aChain {
+	}
+}

daemon/libnetwork/internal/nftables/testdata/TestTable/created6.golden

@@ -1,4 +1,2 @@
-table ip ipv4_table {
-}
 table ip6 ipv6_table {
 }

daemon/libnetwork/internal/nftables/testdata/TestVMap/deleted.golden

@@ -1,6 +1,2 @@
 table ip6 this_is_a_table {
-	map this_is_a_vmap {
-		type ifname : verdict
-		elements = { "eth0" : return }
-	}
 }

daemon/libnetwork/internal/nftables/testdata/TestValidation/empty.golden

@@ -0,0 +1,2 @@
+table ip tablename {
+}

daemon/libnetwork/resolver_unix.go

@@ -97,32 +97,43 @@ func (r *Resolver) setupNftablesNAT(ctx context.Context, laddr, ltcpaddr, resolv
 	if err != nil {
 		return err
 	}
+	tm := nftables.Modifier{}
 
-	dnatChain, err := table.BaseChain(ctx, "dns-dnat", nftables.BaseChainTypeNAT, nftables.BaseChainHookOutput, nftables.BaseChainPriorityDstNAT)
-	if err != nil {
-		return err
-	}
-	if err := dnatChain.AppendRule(ctx, 0, "ip daddr %s udp dport %s counter dnat to %s", resolverIP, dnsPort, laddr); err != nil {
-		return err
-	}
-	if err := dnatChain.AppendRule(ctx, 0, "ip daddr %s tcp dport %s counter dnat to %s", resolverIP, dnsPort, ltcpaddr); err != nil {
-		return err
-	}
+	const dnatChain = "dns-dnat"
+	tm.Create(nftables.BaseChain{
+		Name:      dnatChain,
+		ChainType: nftables.BaseChainTypeNAT,
+		Hook:      nftables.BaseChainHookOutput,
+		Priority:  nftables.BaseChainPriorityDstNAT,
+	})
+	tm.Create(nftables.Rule{
+		Chain: dnatChain,
+		Rule:  []string{"ip daddr", resolverIP, "udp dport", dnsPort, "counter dnat to", laddr},
+	})
+	tm.Create(nftables.Rule{
+		Chain: dnatChain,
+		Rule:  []string{"ip daddr", resolverIP, "tcp dport", dnsPort, "counter dnat to", ltcpaddr},
+	})
 
-	snatChain, err := table.BaseChain(ctx, "dns-snat", nftables.BaseChainTypeNAT, nftables.BaseChainHookPostrouting, nftables.BaseChainPrioritySrcNAT)
-	if err != nil {
-		return err
-	}
-	if err := snatChain.AppendRule(ctx, 0, "ip saddr %s udp sport %s counter snat to :%s", resolverIP, ipPort, dnsPort); err != nil {
-		return err
-	}
-	if err := snatChain.AppendRule(ctx, 0, "ip saddr %s tcp sport %s counter snat to :%s", resolverIP, tcpPort, dnsPort); err != nil {
-		return err
-	}
+	const snatChain = "dns-snat"
+	tm.Create(nftables.BaseChain{
+		Name:      snatChain,
+		ChainType: nftables.BaseChainTypeNAT,
+		Hook:      nftables.BaseChainHookPostrouting,
+		Priority:  nftables.BaseChainPrioritySrcNAT,
+	})
+	tm.Create(nftables.Rule{
+		Chain: snatChain,
+		Rule:  []string{"ip saddr", resolverIP, "udp sport", ipPort, "counter snat to :" + dnsPort},
+	})
+	tm.Create(nftables.Rule{
+		Chain: snatChain,
+		Rule:  []string{"ip saddr", resolverIP, "tcp sport", tcpPort, "counter snat to :" + dnsPort},
+	})
 
 	var setupErr error
 	if err := r.backend.ExecFunc(func() {
-		setupErr = table.Apply(ctx)
+		setupErr = table.Apply(ctx, tm)
 	}); err != nil {
 		return err
 	}