[client] Gate IPv6 forwarding on overlay v6 and preserve host RA acceptance (#6221)

client/firewall/iptables/dnat_refcount_linux_test.go

@@ -0,0 +1,240 @@
+//go:build privileged
+
+package iptables
+
+import (
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	fw "github.com/netbirdio/netbird/client/firewall/manager"
+	"github.com/netbirdio/netbird/client/iface"
+	"github.com/netbirdio/netbird/client/iface/wgaddr"
+)
+
+func iptRefcountIfaceV4() *iFaceMock {
+	return &iFaceMock{
+		NameFunc: func() string { return "wt-refcount" },
+		AddressFunc: func() wgaddr.Address {
+			return wgaddr.Address{
+				IP:      netip.MustParseAddr("10.20.0.1"),
+				Network: netip.MustParsePrefix("10.20.0.0/24"),
+			}
+		},
+	}
+}
+
+func iptRefcountIfaceDual() *iFaceMock {
+	return &iFaceMock{
+		NameFunc: func() string { return "wt-refcount" },
+		AddressFunc: func() wgaddr.Address {
+			return wgaddr.Address{
+				IP:      netip.MustParseAddr("10.20.0.1"),
+				Network: netip.MustParsePrefix("10.20.0.0/24"),
+				IPv6:    netip.MustParseAddr("fd00::1"),
+				IPv6Net: netip.MustParsePrefix("fd00::/64"),
+			}
+		},
+	}
+}
+
+func newIptRefcountManager(t *testing.T, dual bool) *Manager {
+	t.Helper()
+	var ifMock *iFaceMock
+	if dual {
+		ifMock = iptRefcountIfaceDual()
+	} else {
+		ifMock = iptRefcountIfaceV4()
+	}
+	m, err := Create(ifMock, iface.DefaultMTU)
+	require.NoError(t, err, "create manager")
+	require.NoError(t, m.Init(nil), "init manager")
+	t.Cleanup(func() {
+		require.NoError(t, m.Close(nil), "close manager")
+	})
+	return m
+}
+
+func iptDnatV4(port uint16) fw.ForwardRule {
+	return fw.ForwardRule{
+		Protocol:          fw.ProtocolTCP,
+		DestinationPort:   fw.Port{Values: []uint16{port}},
+		TranslatedAddress: netip.MustParseAddr("10.20.0.2"),
+		TranslatedPort:    fw.Port{Values: []uint16{80}},
+	}
+}
+
+func iptDnatV6(port uint16) fw.ForwardRule {
+	return fw.ForwardRule{
+		Protocol:          fw.ProtocolTCP,
+		DestinationPort:   fw.Port{Values: []uint16{port}},
+		TranslatedAddress: netip.MustParseAddr("fd00::2"),
+		TranslatedPort:    fw.Port{Values: []uint16{80}},
+	}
+}
+
+// TestIptablesRouting_RepeatedEnableSingleReference verifies that EnableRouting
+// (called on every network-map update) holds at most one reference per family
+// and a single DisableRouting drops both back to zero.
+func TestIptablesRouting_RepeatedEnableSingleReference(t *testing.T) {
+	m := newIptRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	require.NoError(t, m.EnableRouting(), "first enable")
+	require.NoError(t, m.EnableRouting(), "second enable")
+	require.NoError(t, m.EnableRouting(), "third enable")
+	v4, v6 := state.Counts()
+	assert.Equal(t, 1, v4, "repeated enable holds a single v4 reference")
+	assert.Equal(t, 1, v6, "repeated enable holds a single v6 reference")
+
+	require.NoError(t, m.DisableRouting(), "disable")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4, "single disable releases the v4 reference")
+	assert.Equal(t, 0, v6, "single disable releases the v6 reference")
+}
+
+// TestIptablesRouting_DisableKeepsDNATReference verifies that an unpaired
+// DisableRouting does not release references held by active DNAT rules.
+func TestIptablesRouting_DisableKeepsDNATReference(t *testing.T) {
+	m := newIptRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	r1, err := m.AddDNATRule(iptDnatV6(9095))
+	require.NoError(t, err, "add v6 dnat")
+
+	require.NoError(t, m.DisableRouting(), "unpaired disable")
+	_, v6 := state.Counts()
+	assert.Equal(t, 1, v6, "DNAT-held reference survives unpaired DisableRouting")
+
+	require.NoError(t, m.DeleteDNATRule(r1), "delete v6 dnat")
+	_, v6 = state.Counts()
+	assert.Equal(t, 0, v6, "delete releases the DNAT reference")
+}
+
+// TestIptablesDNAT_RefcountBalancedV4 covers a Balanced Add/Delete pair on v4.
+func TestIptablesDNAT_RefcountBalancedV4(t *testing.T) {
+	m := newIptRefcountManager(t, false)
+	state := m.router.ipFwdState
+
+	r1, err := m.AddDNATRule(iptDnatV4(7081))
+	require.NoError(t, err, "add v4 dnat 1")
+	v4, v6 := state.Counts()
+	assert.Equal(t, 1, v4, "v4 refcount after first add")
+	assert.Equal(t, 0, v6, "v6 refcount unchanged")
+
+	r2, err := m.AddDNATRule(iptDnatV4(7082))
+	require.NoError(t, err, "add v4 dnat 2")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 2, v4, "v4 refcount after second add")
+	assert.Equal(t, 0, v6, "v6 refcount unchanged")
+
+	require.NoError(t, m.DeleteDNATRule(r1))
+	v4, v6 = state.Counts()
+	assert.Equal(t, 1, v4, "v4 refcount after first delete")
+	assert.Equal(t, 0, v6, "v6 refcount unchanged")
+
+	require.NoError(t, m.DeleteDNATRule(r2))
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4, "v4 refcount after second delete")
+	assert.Equal(t, 0, v6, "v6 refcount unchanged")
+}
+
+// TestIptablesDNAT_RefcountBalancedV6 checks the v6 path increments v6 only and
+// decrements back to zero.
+func TestIptablesDNAT_RefcountBalancedV6(t *testing.T) {
+	m := newIptRefcountManager(t, true)
+	require.NotNil(t, m.router6, "v6 router")
+	require.Same(t, m.router.ipFwdState, m.router6.ipFwdState, "shared state")
+	state := m.router.ipFwdState
+
+	r1, err := m.AddDNATRule(iptDnatV6(9081))
+	require.NoError(t, err, "add v6 dnat 1")
+	v4, v6 := state.Counts()
+	assert.Equal(t, 0, v4)
+	assert.Equal(t, 1, v6, "v6 refcount after first add")
+
+	r2, err := m.AddDNATRule(iptDnatV6(9082))
+	require.NoError(t, err, "add v6 dnat 2")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4, "v4 refcount unchanged")
+	assert.Equal(t, 2, v6, "v6 refcount after second add")
+
+	require.NoError(t, m.DeleteDNATRule(r1))
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4, "v4 refcount unchanged")
+	assert.Equal(t, 1, v6, "v6 refcount after first delete")
+
+	require.NoError(t, m.DeleteDNATRule(r2))
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4)
+	assert.Equal(t, 0, v6, "v6 refcount after second delete")
+}
+
+// TestIptablesDNAT_DuplicateAddNoLeak verifies the duplicate-rule path returns
+// without bumping the refcount.
+func TestIptablesDNAT_DuplicateAddNoLeak(t *testing.T) {
+	m := newIptRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	rule := iptDnatV4(7083)
+	r1, err := m.AddDNATRule(rule)
+	require.NoError(t, err)
+	v4, _ := state.Counts()
+	assert.Equal(t, 1, v4)
+
+	_, err = m.AddDNATRule(rule)
+	require.NoError(t, err, "duplicate add")
+	v4, _ = state.Counts()
+	assert.Equal(t, 1, v4, "duplicate add must not increment")
+
+	require.NoError(t, m.DeleteDNATRule(r1))
+	v4, _ = state.Counts()
+	assert.Equal(t, 0, v4, "single delete must drop to zero")
+}
+
+// TestIptablesDNAT_DeleteMissingNoUnderflow verifies Delete on an unknown rule
+// neither errors nor releases the refcount.
+func TestIptablesDNAT_DeleteMissingNoUnderflow(t *testing.T) {
+	m := newIptRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	phantom := iptDnatV4(7099)
+	require.NoError(t, m.DeleteDNATRule(&phantom), "delete missing v4")
+	v4, v6 := state.Counts()
+	assert.Equal(t, 0, v4)
+	assert.Equal(t, 0, v6)
+
+	phantom6 := iptDnatV6(9099)
+	require.NoError(t, m.DeleteDNATRule(&phantom6), "delete missing v6")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4)
+	assert.Equal(t, 0, v6)
+
+	r1, err := m.AddDNATRule(iptDnatV4(7100))
+	require.NoError(t, err)
+	v4, _ = state.Counts()
+	assert.Equal(t, 1, v4, "real add still increments after phantom delete")
+	require.NoError(t, m.DeleteDNATRule(r1))
+}
+
+// TestIptablesDNAT_DoubleDeleteNoUnderflow verifies a second Delete on the same
+// rule is a no-op.
+func TestIptablesDNAT_DoubleDeleteNoUnderflow(t *testing.T) {
+	m := newIptRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	r1, err := m.AddDNATRule(iptDnatV6(9083))
+	require.NoError(t, err)
+	_, v6 := state.Counts()
+	assert.Equal(t, 1, v6)
+
+	require.NoError(t, m.DeleteDNATRule(r1), "first delete")
+	_, v6 = state.Counts()
+	assert.Equal(t, 0, v6)
+
+	require.NoError(t, m.DeleteDNATRule(r1), "second delete must be no-op")
+	_, v6 = state.Counts()
+	assert.Equal(t, 0, v6, "double delete must not underflow")
+}

client/firewall/iptables/manager_linux.go

@@ -89,7 +89,7 @@ func (m *Manager) createIPv6Components(wgIface iFaceMapper, mtu uint16) error {
 	}
 
 	// Share the same IP forwarding state with the v4 router, since
-	// EnableIPForwarding controls both v4 and v6 sysctls.
+	// Forwarding refcounter is per-family but shared between v4 and v6 routers.
 	m.router6.ipFwdState = m.router.ipFwdState
 
 	m.aclMgr6, err = newAclManager(ip6Client, wgIface)
@@ -402,17 +402,12 @@ func (m *Manager) SetLogLevel(log.Level) {
 }
 
 func (m *Manager) EnableRouting() error {
-	if err := m.router.ipFwdState.RequestForwarding(); err != nil {
-		return fmt.Errorf("enable IP forwarding: %w", err)
-	}
-	return nil
+	// v6 only when the overlay actually has v6.
+	return m.router.ipFwdState.RequestRouting(m.router6 != nil)
 }
 
 func (m *Manager) DisableRouting() error {
-	if err := m.router.ipFwdState.ReleaseForwarding(); err != nil {
-		return fmt.Errorf("disable IP forwarding: %w", err)
-	}
-	return nil
+	return m.router.ipFwdState.ReleaseRouting()
 }
 
 // AddDNATRule adds a DNAT rule

client/firewall/iptables/router_linux.go

@@ -102,7 +102,7 @@ func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint1
 		wgIface:        wgIface,
 		mtu:            mtu,
 		v6:             iptablesClient.Proto() == iptables.ProtocolIPv6,
-		ipFwdState:     ipfwdstate.NewIPForwardingState(),
+		ipFwdState:     ipfwdstate.NewIPForwardingState(wgIface.Name()),
 	}
 
 	r.ipsetCounter = refcounter.New(
@@ -770,10 +770,6 @@ func (r *router) updateState() {
 }
 
 func (r *router) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) {
-	if err := r.ipFwdState.RequestForwarding(); err != nil {
-		return nil, err
-	}
-
 	ruleKey := rule.ID()
 	if _, exists := r.rules[ruleKey+dnatSuffix]; exists {
 		return rule, nil
@@ -840,18 +836,34 @@ func (r *router) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) {
 
 	for key, ruleInfo := range rules {
 		if err := r.iptablesClient.Append(ruleInfo.table, ruleInfo.chain, ruleInfo.rule...); err != nil {
-			if rollbackErr := r.rollbackRules(rules); rollbackErr != nil {
-				log.Errorf("rollback failed: %v", rollbackErr)
-			}
+			r.cleanupFailedDNATAdd(rules)
 			return nil, fmt.Errorf("add rule %s: %w", key, err)
 		}
 		r.rules[key] = ruleInfo.rule
 	}
 
+	if err := r.ipFwdState.RequestForwarding(r.v6); err != nil {
+		r.cleanupFailedDNATAdd(rules)
+		return nil, fmt.Errorf("enable forwarding: %w", err)
+	}
+
 	r.updateState()
 	return rule, nil
 }
 
+// cleanupFailedDNATAdd removes the bookkeeping written by a partially applied
+// AddDNATRule before rolling back the kernel rules, so no entries remain that
+// never got a forwarding refcount. rollbackRules re-adds entries it failed to
+// remove from the kernel.
+func (r *router) cleanupFailedDNATAdd(rules map[string]ruleInfo) {
+	for key := range rules {
+		delete(r.rules, key)
+	}
+	if err := r.rollbackRules(rules); err != nil {
+		log.Errorf("rollback failed: %v", err)
+	}
+}
+
 func (r *router) rollbackRules(rules map[string]ruleInfo) error {
 	var merr *multierror.Error
 	for key, ruleInfo := range rules {
@@ -868,32 +880,47 @@ func (r *router) rollbackRules(rules map[string]ruleInfo) error {
 }
 
 func (r *router) DeleteDNATRule(rule firewall.Rule) error {
-	if err := r.ipFwdState.ReleaseForwarding(); err != nil {
-		log.Errorf("%v", err)
-	}
-
 	ruleKey := rule.ID()
 
+	_, hadDNAT := r.rules[ruleKey+dnatSuffix]
+	_, hadSNAT := r.rules[ruleKey+snatSuffix]
+	_, hadFWD := r.rules[ruleKey+fwdSuffix]
+	if !hadDNAT && !hadSNAT && !hadFWD {
+		return nil
+	}
+
 	var merr *multierror.Error
 	if dnatRule, exists := r.rules[ruleKey+dnatSuffix]; exists {
 		if err := r.iptablesClient.Delete(tableNat, chainRTRDR, dnatRule...); err != nil {
 			merr = multierror.Append(merr, fmt.Errorf("delete DNAT rule: %w", err))
+		} else {
+			delete(r.rules, ruleKey+dnatSuffix)
 		}
-		delete(r.rules, ruleKey+dnatSuffix)
 	}
 
 	if snatRule, exists := r.rules[ruleKey+snatSuffix]; exists {
 		if err := r.iptablesClient.Delete(tableNat, chainRTNAT, snatRule...); err != nil {
 			merr = multierror.Append(merr, fmt.Errorf("delete SNAT rule: %w", err))
+		} else {
+			delete(r.rules, ruleKey+snatSuffix)
 		}
-		delete(r.rules, ruleKey+snatSuffix)
 	}
 
 	if fwdRule, exists := r.rules[ruleKey+fwdSuffix]; exists {
 		if err := r.iptablesClient.Delete(tableFilter, chainRTFWDOUT, fwdRule...); err != nil {
 			merr = multierror.Append(merr, fmt.Errorf("delete forward rule: %w", err))
+		} else {
+			delete(r.rules, ruleKey+fwdSuffix)
+		}
+	}
+
+	// Release the refcount only once all rules are gone from the kernel. On
+	// partial failure the failed entries stay in r.rules so a retry can remove
+	// them and release then.
+	if merr == nil {
+		if err := r.ipFwdState.ReleaseForwarding(r.v6); err != nil {
+			log.Errorf("%v", err)
 		}
-		delete(r.rules, ruleKey+fwdSuffix)
 	}
 
 	r.updateState()

client/firewall/nftables/dnat_refcount_linux_test.go

@@ -0,0 +1,249 @@
+//go:build privileged
+
+package nftables
+
+import (
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	fw "github.com/netbirdio/netbird/client/firewall/manager"
+	"github.com/netbirdio/netbird/client/iface"
+	"github.com/netbirdio/netbird/client/iface/wgaddr"
+)
+
+func nftRefcountIfaceV4() *iFaceMock {
+	return &iFaceMock{
+		NameFunc: func() string { return "wt-refcount" },
+		AddressFunc: func() wgaddr.Address {
+			return wgaddr.Address{
+				IP:      netip.MustParseAddr("100.96.0.1"),
+				Network: netip.MustParsePrefix("100.96.0.0/16"),
+			}
+		},
+	}
+}
+
+func nftRefcountIfaceDual() *iFaceMock {
+	return &iFaceMock{
+		NameFunc: func() string { return "wt-refcount" },
+		AddressFunc: func() wgaddr.Address {
+			return wgaddr.Address{
+				IP:      netip.MustParseAddr("100.96.0.1"),
+				Network: netip.MustParsePrefix("100.96.0.0/16"),
+				IPv6:    netip.MustParseAddr("fd00::1"),
+				IPv6Net: netip.MustParsePrefix("fd00::/64"),
+			}
+		},
+	}
+}
+
+func newNftRefcountManager(t *testing.T, dual bool) *Manager {
+	t.Helper()
+	if check() != NFTABLES {
+		t.Skip("nftables not supported on this system")
+	}
+	var ifMock *iFaceMock
+	if dual {
+		ifMock = nftRefcountIfaceDual()
+	} else {
+		ifMock = nftRefcountIfaceV4()
+	}
+	m, err := Create(ifMock, iface.DefaultMTU)
+	require.NoError(t, err, "create manager")
+	require.NoError(t, m.Init(nil), "init manager")
+	t.Cleanup(func() {
+		require.NoError(t, m.Close(nil), "close manager")
+	})
+	return m
+}
+
+func dnatV4(port uint16) fw.ForwardRule {
+	return fw.ForwardRule{
+		Protocol:          fw.ProtocolTCP,
+		DestinationPort:   fw.Port{Values: []uint16{port}},
+		TranslatedAddress: netip.MustParseAddr("100.96.0.2"),
+		TranslatedPort:    fw.Port{Values: []uint16{80}},
+	}
+}
+
+func dnatV6(port uint16) fw.ForwardRule {
+	return fw.ForwardRule{
+		Protocol:          fw.ProtocolTCP,
+		DestinationPort:   fw.Port{Values: []uint16{port}},
+		TranslatedAddress: netip.MustParseAddr("fd00::2"),
+		TranslatedPort:    fw.Port{Values: []uint16{80}},
+	}
+}
+
+// TestNftablesDNAT_RefcountBalancedV4 verifies that Add/Delete pairs leave the
+// v4 refcount at zero.
+func TestNftablesDNAT_RefcountBalancedV4(t *testing.T) {
+	m := newNftRefcountManager(t, false)
+	state := m.router.ipFwdState
+
+	r1, err := m.AddDNATRule(dnatV4(8081))
+	require.NoError(t, err, "add v4 dnat 1")
+	v4, v6 := state.Counts()
+	assert.Equal(t, 1, v4, "v4 refcount after first add")
+	assert.Equal(t, 0, v6, "v6 refcount unchanged")
+
+	r2, err := m.AddDNATRule(dnatV4(8082))
+	require.NoError(t, err, "add v4 dnat 2")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 2, v4, "v4 refcount after second add")
+	assert.Equal(t, 0, v6, "v6 refcount unchanged")
+
+	require.NoError(t, m.DeleteDNATRule(r1), "delete v4 dnat 1")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 1, v4, "v4 refcount after first delete")
+	assert.Equal(t, 0, v6, "v6 refcount unchanged")
+
+	require.NoError(t, m.DeleteDNATRule(r2), "delete v4 dnat 2")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4, "v4 refcount after second delete")
+	assert.Equal(t, 0, v6, "v6 refcount unchanged")
+}
+
+// TestNftablesDNAT_RefcountBalancedV6 verifies the v6 path increments v6 only
+// and decrements back to zero on Delete.
+func TestNftablesDNAT_RefcountBalancedV6(t *testing.T) {
+	m := newNftRefcountManager(t, true)
+	require.NotNil(t, m.router6, "v6 router")
+	require.Same(t, m.router.ipFwdState, m.router6.ipFwdState, "shared state")
+	state := m.router.ipFwdState
+
+	r1, err := m.AddDNATRule(dnatV6(9091))
+	require.NoError(t, err, "add v6 dnat 1")
+	v4, v6 := state.Counts()
+	assert.Equal(t, 0, v4, "v4 refcount unchanged")
+	assert.Equal(t, 1, v6, "v6 refcount after first add")
+
+	r2, err := m.AddDNATRule(dnatV6(9092))
+	require.NoError(t, err, "add v6 dnat 2")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4)
+	assert.Equal(t, 2, v6, "v6 refcount after second add")
+
+	require.NoError(t, m.DeleteDNATRule(r1), "delete v6 dnat 1")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4, "v4 refcount unchanged")
+	assert.Equal(t, 1, v6, "v6 refcount after first delete")
+
+	require.NoError(t, m.DeleteDNATRule(r2), "delete v6 dnat 2")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4)
+	assert.Equal(t, 0, v6, "v6 refcount after second delete")
+}
+
+// TestNftablesDNAT_DuplicateAddNoLeak verifies that a duplicate Add (same
+// ForwardRule) does not double-increment the refcount.
+func TestNftablesDNAT_DuplicateAddNoLeak(t *testing.T) {
+	m := newNftRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	rule := dnatV4(8083)
+	r1, err := m.AddDNATRule(rule)
+	require.NoError(t, err, "add v4 dnat")
+	v4, _ := state.Counts()
+	assert.Equal(t, 1, v4)
+
+	// duplicate add: same rule ID, must be a no-op for the refcount.
+	_, err = m.AddDNATRule(rule)
+	require.NoError(t, err, "duplicate add")
+	v4, _ = state.Counts()
+	assert.Equal(t, 1, v4, "duplicate add must not increment")
+
+	require.NoError(t, m.DeleteDNATRule(r1), "delete v4 dnat")
+	v4, _ = state.Counts()
+	assert.Equal(t, 0, v4, "single delete must drop to zero")
+}
+
+// TestNftablesDNAT_DeleteMissingNoUnderflow verifies deleting a rule that was
+// never added does not underflow the refcount.
+func TestNftablesDNAT_DeleteMissingNoUnderflow(t *testing.T) {
+	m := newNftRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	// Construct a Rule reference for something never added. The router stores
+	// rules by ID(), and DeleteDNATRule looks them up in r.rules; a missing
+	// entry must be a no-op rather than calling Release.
+	phantom := dnatV4(8099)
+	require.NoError(t, m.DeleteDNATRule(&phantom), "delete missing v4 dnat")
+	v4, v6 := state.Counts()
+	assert.Equal(t, 0, v4, "v4 refcount unaffected by missing delete")
+	assert.Equal(t, 0, v6, "v6 refcount unaffected")
+
+	phantom6 := dnatV6(9099)
+	require.NoError(t, m.DeleteDNATRule(&phantom6), "delete missing v6 dnat")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4)
+	assert.Equal(t, 0, v6, "v6 refcount unaffected by missing delete")
+
+	// And after a phantom delete, a real add still results in count=1.
+	r1, err := m.AddDNATRule(dnatV4(8100))
+	require.NoError(t, err, "add v4 dnat after phantom delete")
+	v4, _ = state.Counts()
+	assert.Equal(t, 1, v4, "real add still increments after phantom delete")
+	require.NoError(t, m.DeleteDNATRule(r1))
+}
+
+// TestNftablesRouting_RepeatedEnableSingleReference verifies that EnableRouting
+// (called on every network-map update) holds at most one reference per family
+// and a single DisableRouting drops both back to zero.
+func TestNftablesRouting_RepeatedEnableSingleReference(t *testing.T) {
+	m := newNftRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	require.NoError(t, m.EnableRouting(), "first enable")
+	require.NoError(t, m.EnableRouting(), "second enable")
+	require.NoError(t, m.EnableRouting(), "third enable")
+	v4, v6 := state.Counts()
+	assert.Equal(t, 1, v4, "repeated enable holds a single v4 reference")
+	assert.Equal(t, 1, v6, "repeated enable holds a single v6 reference")
+
+	require.NoError(t, m.DisableRouting(), "disable")
+	v4, v6 = state.Counts()
+	assert.Equal(t, 0, v4, "single disable releases the v4 reference")
+	assert.Equal(t, 0, v6, "single disable releases the v6 reference")
+}
+
+// TestNftablesRouting_DisableKeepsDNATReference verifies that an unpaired
+// DisableRouting does not release references held by active DNAT rules.
+func TestNftablesRouting_DisableKeepsDNATReference(t *testing.T) {
+	m := newNftRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	r1, err := m.AddDNATRule(dnatV6(9095))
+	require.NoError(t, err, "add v6 dnat")
+
+	require.NoError(t, m.DisableRouting(), "unpaired disable")
+	_, v6 := state.Counts()
+	assert.Equal(t, 1, v6, "DNAT-held reference survives unpaired DisableRouting")
+
+	require.NoError(t, m.DeleteDNATRule(r1), "delete v6 dnat")
+	_, v6 = state.Counts()
+	assert.Equal(t, 0, v6, "delete releases the DNAT reference")
+}
+
+// TestNftablesDNAT_DoubleDeleteNoUnderflow verifies that deleting the same rule
+// twice does not underflow the refcount (the second delete is a no-op).
+func TestNftablesDNAT_DoubleDeleteNoUnderflow(t *testing.T) {
+	m := newNftRefcountManager(t, true)
+	state := m.router.ipFwdState
+
+	r1, err := m.AddDNATRule(dnatV6(9093))
+	require.NoError(t, err)
+	_, v6 := state.Counts()
+	assert.Equal(t, 1, v6)
+
+	require.NoError(t, m.DeleteDNATRule(r1), "first delete")
+	_, v6 = state.Counts()
+	assert.Equal(t, 0, v6)
+
+	require.NoError(t, m.DeleteDNATRule(r1), "second delete must be no-op")
+	_, v6 = state.Counts()
+	assert.Equal(t, 0, v6, "double delete must not underflow")
+}

client/firewall/nftables/manager_linux.go

@@ -105,8 +105,8 @@ func (m *Manager) createIPv6Components(tableName string, wgIface iFaceMapper, mt
 		return fmt.Errorf("create v6 router: %w", err)
 	}
 
-	// Share the same IP forwarding state with the v4 router, since
-	// EnableIPForwarding controls both v4 and v6 sysctls.
+	// Share the per-family forwarding refcounter with the v4 router so a v4
+	// rule and a v6 rule against the same state machine cooperate cleanly.
 	m.router6.ipFwdState = m.router.ipFwdState
 
 	m.aclManager6, err = newAclManager(workTable6, wgIface, chainNameRoutingFw)
@@ -530,17 +530,12 @@ func (m *Manager) SetLogLevel(log.Level) {
 }
 
 func (m *Manager) EnableRouting() error {
-	if err := m.router.ipFwdState.RequestForwarding(); err != nil {
-		return fmt.Errorf("enable IP forwarding: %w", err)
-	}
-	return nil
+	// v6 only when the overlay actually has v6.
+	return m.router.ipFwdState.RequestRouting(m.router6 != nil)
 }
 
 func (m *Manager) DisableRouting() error {
-	if err := m.router.ipFwdState.ReleaseForwarding(); err != nil {
-		return fmt.Errorf("disable IP forwarding: %w", err)
-	}
-	return nil
+	return m.router.ipFwdState.ReleaseRouting()
 }
 
 // Flush rule/chain/set operations from the buffer

client/firewall/nftables/router_linux.go

@@ -93,7 +93,7 @@ func newRouter(workTable *nftables.Table, wgIface iFaceMapper, mtu uint16) (*rou
 		rules:      make(map[string]*nftables.Rule),
 		af:         familyForAddr(workTable.Family == nftables.TableFamilyIPv4),
 		wgIface:    wgIface,
-		ipFwdState: ipfwdstate.NewIPForwardingState(),
+		ipFwdState: ipfwdstate.NewIPForwardingState(wgIface.Name()),
 		mtu:        mtu,
 	}
 
@@ -1553,10 +1553,6 @@ func (r *router) refreshRulesMap() error {
 }
 
 func (r *router) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) {
-	if err := r.ipFwdState.RequestForwarding(); err != nil {
-		return nil, err
-	}
-
 	ruleKey := rule.ID()
 	if _, exists := r.rules[ruleKey+dnatSuffix]; exists {
 		return rule, nil
@@ -1567,7 +1563,18 @@ func (r *router) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) {
 		return nil, fmt.Errorf("convert protocol to number: %w", err)
 	}
 
+	// Request forwarding before queueing rules: addDnatRedirect/addDnatMasq
+	// buffer netlink messages on r.conn that the next caller's Flush would
+	// commit if we returned without flushing them ourselves.
+	v6 := r.af.tableFamily == nftables.TableFamilyIPv6
+	if err := r.ipFwdState.RequestForwarding(v6); err != nil {
+		return nil, fmt.Errorf("enable forwarding: %w", err)
+	}
+
 	if err := r.addDnatRedirect(rule, protoNum, ruleKey); err != nil {
+		if rerr := r.ipFwdState.ReleaseForwarding(v6); rerr != nil {
+			log.Warnf("rollback forwarding refcount: %v", rerr)
+		}
 		return nil, err
 	}
 
@@ -1579,6 +1586,11 @@ func (r *router) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) {
 	// TODO: find chains with drop policies and add rules there
 
 	if err := r.conn.Flush(); err != nil {
+		if rerr := r.ipFwdState.ReleaseForwarding(v6); rerr != nil {
+			log.Warnf("rollback forwarding refcount: %v", rerr)
+		}
+		delete(r.rules, ruleKey+dnatSuffix)
+		delete(r.rules, ruleKey+snatSuffix)
 		return nil, fmt.Errorf("flush rules: %w", err)
 	}
 
@@ -1781,16 +1793,18 @@ func (r *router) addDnatMasq(rule firewall.ForwardRule, protoNum uint8, ruleKey
 }
 
 func (r *router) DeleteDNATRule(rule firewall.Rule) error {
-	if err := r.ipFwdState.ReleaseForwarding(); err != nil {
-		log.Errorf("%v", err)
-	}
-
 	ruleKey := rule.ID()
 
 	if err := r.refreshRulesMap(); err != nil {
 		return fmt.Errorf(refreshRulesMapError, err)
 	}
 
+	_, hadDNAT := r.rules[ruleKey+dnatSuffix]
+	_, hadSNAT := r.rules[ruleKey+snatSuffix]
+	if !hadDNAT && !hadSNAT {
+		return nil
+	}
+
 	var merr *multierror.Error
 	var needsFlush bool
 
@@ -1822,9 +1836,16 @@ func (r *router) DeleteDNATRule(rule firewall.Rule) error {
 		}
 	}
 
+	// Release the refcount only once the rules are gone from the kernel. On
+	// failure (including the refreshRulesMap error above) the rules and their
+	// map entries remain, keeping forwarding on until a retry removes them.
 	if merr == nil {
 		delete(r.rules, ruleKey+dnatSuffix)
 		delete(r.rules, ruleKey+snatSuffix)
+
+		if err := r.ipFwdState.ReleaseForwarding(r.af.tableFamily == nftables.TableFamilyIPv6); err != nil {
+			log.Errorf("%v", err)
+		}
 	}
 
 	return nberrors.FormatErrorOrNil(merr)