Address review: VLAN-safe procfs path, rollback partial v6 enable, aggregate routing errors

client/firewall/iptables/manager_linux.go

@@ -405,26 +405,30 @@ func (m *Manager) EnableRouting() error {
 	if err := m.router.ipFwdState.RequestForwarding(false); err != nil {
 		return fmt.Errorf("enable IPv4 forwarding: %w", err)
 	}
-	// Only flip v6 forwarding when the WG interface actually has v6, so that
-	// v4-only routing setups don't disable RA acceptance on the host.
-	if m.router6 != nil {
-		if err := m.router.ipFwdState.RequestForwarding(true); err != nil {
-			return fmt.Errorf("enable IPv6 forwarding: %w", err)
+	// v6 only when the overlay actually has v6.
+	if m.router6 == nil {
+		return nil
+	}
+	if err := m.router.ipFwdState.RequestForwarding(true); err != nil {
+		if rerr := m.router.ipFwdState.ReleaseForwarding(false); rerr != nil {
+			log.Warnf("rollback v4 forwarding: %v", rerr)
 		}
+		return fmt.Errorf("enable IPv6 forwarding: %w", err)
 	}
 	return nil
 }
 
 func (m *Manager) DisableRouting() error {
+	var merr *multierror.Error
 	if err := m.router.ipFwdState.ReleaseForwarding(false); err != nil {
-		return fmt.Errorf("disable IPv4 forwarding: %w", err)
+		merr = multierror.Append(merr, fmt.Errorf("disable IPv4 forwarding: %w", err))
 	}
 	if m.router6 != nil {
 		if err := m.router.ipFwdState.ReleaseForwarding(true); err != nil {
-			return fmt.Errorf("disable IPv6 forwarding: %w", err)
+			merr = multierror.Append(merr, fmt.Errorf("disable IPv6 forwarding: %w", err))
 		}
 	}
-	return nil
+	return nberrors.FormatErrorOrNil(merr)
 }
 
 // AddDNATRule adds a DNAT rule

client/firewall/nftables/manager_linux.go

@@ -533,26 +533,30 @@ func (m *Manager) EnableRouting() error {
 	if err := m.router.ipFwdState.RequestForwarding(false); err != nil {
 		return fmt.Errorf("enable IPv4 forwarding: %w", err)
 	}
-	// Only flip v6 forwarding when the WG interface actually has v6, so that
-	// v4-only routing setups don't disable RA acceptance on the host.
-	if m.router6 != nil {
-		if err := m.router.ipFwdState.RequestForwarding(true); err != nil {
-			return fmt.Errorf("enable IPv6 forwarding: %w", err)
+	// v6 only when the overlay actually has v6.
+	if m.router6 == nil {
+		return nil
+	}
+	if err := m.router.ipFwdState.RequestForwarding(true); err != nil {
+		if rerr := m.router.ipFwdState.ReleaseForwarding(false); rerr != nil {
+			log.Warnf("rollback v4 forwarding: %v", rerr)
 		}
+		return fmt.Errorf("enable IPv6 forwarding: %w", err)
 	}
 	return nil
 }
 
 func (m *Manager) DisableRouting() error {
+	var merr *multierror.Error
 	if err := m.router.ipFwdState.ReleaseForwarding(false); err != nil {
-		return fmt.Errorf("disable IPv4 forwarding: %w", err)
+		merr = multierror.Append(merr, fmt.Errorf("disable IPv4 forwarding: %w", err))
 	}
 	if m.router6 != nil {
 		if err := m.router.ipFwdState.ReleaseForwarding(true); err != nil {
-			return fmt.Errorf("disable IPv6 forwarding: %w", err)
+			merr = multierror.Append(merr, fmt.Errorf("disable IPv6 forwarding: %w", err))
 		}
 	}
-	return nil
+	return nberrors.FormatErrorOrNil(merr)
 }
 
 // Flush rule/chain/set operations from the buffer

client/internal/routemanager/ipfwdstate/ipfwdstate.go

@@ -9,31 +9,23 @@ import (
 	"github.com/netbirdio/netbird/client/internal/routemanager/systemops"
 )
 
-// IPForwardingState tracks per-family IP-forwarding sysctl enables with a
-// refcount. v4 and v6 are independent so a v4-only routing setup doesn't flip
-// net.ipv6.conf.all.forwarding, which on Linux disables RA acceptance by
-// default and lets host RA-installed defaults silently expire.
+// IPForwardingState tracks v4 and v6 IP-forwarding sysctl enables with
+// independent refcounts so a v4-only routing setup doesn't flip v6 sysctls.
 type IPForwardingState struct {
 	mu sync.Mutex
 
 	v4Count int
 	v6Count int
 
-	// wgIfaceName is excluded from the v6 accept_ra bump since the overlay
-	// interface doesn't carry upstream RAs.
 	wgIfaceName string
-	// v6Saved records the sysctl values captured when v6 forwarding was
-	// enabled (forwarding + per-interface accept_ra), restored on the last
-	// release.
-	v6Saved map[string]int
+	v6Saved     map[string]int
 }
 
 func NewIPForwardingState(wgIfaceName string) *IPForwardingState {
 	return &IPForwardingState{wgIfaceName: wgIfaceName}
 }
 
-// RequestForwarding bumps the per-family counter, enabling the underlying
-// sysctl on the first request.
+// RequestForwarding enables the family's forwarding sysctl on first request.
 func (f *IPForwardingState) RequestForwarding(v6 bool) error {
 	f.mu.Lock()
 	defer f.mu.Unlock()
@@ -44,9 +36,9 @@ func (f *IPForwardingState) RequestForwarding(v6 bool) error {
 	return f.requestV4()
 }
 
-// ReleaseForwarding decrements the per-family counter. The last v6 release
-// also restores the sysctls v6 enable captured. v4 stays on: net.ipv4.ip_forward
-// is a global knob other tools (docker, k8s, libvirt) co-own.
+// ReleaseForwarding decrements the family counter. The last v6 release restores
+// what enable captured. v4 stays on: net.ipv4.ip_forward is co-owned by other
+// tooling (docker, k8s, libvirt).
 func (f *IPForwardingState) ReleaseForwarding(v6 bool) error {
 	f.mu.Lock()
 	defer f.mu.Unlock()
@@ -78,10 +70,13 @@ func (f *IPForwardingState) releaseV4() {
 func (f *IPForwardingState) requestV6() error {
 	if f.v6Count == 0 {
 		saved, err := systemops.EnableV6IPForwarding(f.wgIfaceName)
-		f.v6Saved = saved
 		if err != nil {
+			if rerr := systemops.DisableV6IPForwarding(saved); rerr != nil {
+				log.Warnf("rollback partial v6 sysctls: %v", rerr)
+			}
 			return fmt.Errorf("enable IPv6 forwarding: %w", err)
 		}
+		f.v6Saved = saved
 		log.Info("IPv6 forwarding enabled")
 	}
 	f.v6Count++

client/internal/routemanager/systemops/systemops_android.go

@@ -39,7 +39,7 @@ func EnableV4IPForwarding() error {
 
 func EnableV6IPForwarding(string) (map[string]int, error) {
 	log.Infof("Enable IPv6 forwarding is not implemented on %s", runtime.GOOS)
-	return nil, nil
+	return map[string]int{}, nil
 }
 
 func DisableV6IPForwarding(map[string]int) error {

client/internal/routemanager/systemops/systemops_ios.go

@@ -65,7 +65,7 @@ func EnableV4IPForwarding() error {
 
 func EnableV6IPForwarding(string) (map[string]int, error) {
 	log.Infof("Enable IPv6 forwarding is not implemented on %s", runtime.GOOS)
-	return nil, nil
+	return map[string]int{}, nil
 }
 
 func DisableV6IPForwarding(map[string]int) error {

client/internal/routemanager/systemops/systemops_linux.go

@@ -9,7 +9,6 @@ import (
 	"net"
 	"net/netip"
 	"os"
-	"strings"
 	"syscall"
 
 	"github.com/hashicorp/go-multierror"
@@ -56,11 +55,11 @@ const (
 	ipv4ForwardingPath = "net.ipv4.ip_forward"
 	// ipv6ForwardingPath is the path to the file containing the IPv6 forwarding setting.
 	ipv6ForwardingPath = "net.ipv6.conf.all.forwarding"
-	// acceptRAInterfacePath toggles per-interface IPv6 RA acceptance.
-	// 1 (kernel default) accepts RAs only when forwarding is off; 2 keeps
-	// RA processing enabled even when forwarding is on, so RA-installed host
-	// defaults survive our v6 forwarding flip.
-	acceptRAInterfacePath = "net.ipv6.conf.%s.accept_ra"
+	// 1 (default) accepts RAs only while forwarding is off; 2 keeps RA
+	// acceptance on regardless, so RA-installed host defaults survive our
+	// v6 forwarding flip.
+	acceptRAInterfacePath  = "net.ipv6.conf.%s.accept_ra"
+	acceptRAProcPathFormat = "/proc/sys/net/ipv6/conf/%s/accept_ra"
 )
 
 var ErrTableIDExists = errors.New("ID exists with different name")
@@ -776,14 +775,9 @@ func EnableV4IPForwarding() error {
 	return nil
 }
 
-// EnableV6IPForwarding bumps accept_ra=2 on every non-loopback v6 interface
-// before flipping forwarding=1, so RA-installed host defaults survive the flip.
-// wgIfaceName is excluded since the overlay interface doesn't carry upstream RAs.
-//
-// The returned map records prior sysctl values for entries we actually changed
-// (forwarding + per-interface accept_ra); DisableV6IPForwarding restores from
-// it. Entries we found already at the target value are omitted so another
-// process's sysctls aren't reset by our cleanup.
+// EnableV6IPForwarding bumps accept_ra=2 on host v6 interfaces before flipping
+// forwarding=1, so RA-installed host defaults survive. Returns the prior values
+// of sysctls we actually changed; entries already at the target are omitted.
 func EnableV6IPForwarding(wgIfaceName string) (map[string]int, error) {
 	saved := map[string]int{}
 	bumpAcceptRA(saved, wgIfaceName)
@@ -798,9 +792,7 @@ func EnableV6IPForwarding(wgIfaceName string) (map[string]int, error) {
 	return saved, nil
 }
 
-// DisableV6IPForwarding restores every sysctl value EnableV6IPForwarding
-// captured. v4 is intentionally not disabled: net.ipv4.ip_forward is a global
-// knob other tools (docker, k8s, libvirt) co-own.
+// DisableV6IPForwarding restores what EnableV6IPForwarding captured.
 func DisableV6IPForwarding(saved map[string]int) error {
 	var result *multierror.Error
 	for key, value := range saved {
@@ -827,13 +819,11 @@ func bumpAcceptRA(saved map[string]int, wgIfaceName string) {
 
 func bumpAcceptRAForInterface(saved map[string]int, name string) {
 	key := fmt.Sprintf(acceptRAInterfacePath, name)
-	procPath := "/proc/sys/" + strings.ReplaceAll(key, ".", "/")
-	if _, err := os.Stat(procPath); err != nil {
-		// No IPv6 stack on this interface.
+	// Build procfs path from name, not the dotted key: VLAN names like eth0.100.
+	if _, err := os.Stat(fmt.Sprintf(acceptRAProcPathFormat, name)); err != nil {
 		return
 	}
-	// onlyIfOne=true: only bump from the kernel default; preserves admin
-	// overrides of 0 (don't accept RAs) or 2 (already what we want).
+	// onlyIfOne=true: leave admin overrides (0, 2) alone.
 	oldVal, err := sysctl.Set(key, 2, true)
 	if err != nil {
 		log.Warnf("bump %s: %v", key, err)

client/internal/routemanager/systemops/systemops_nonlinux.go

@@ -50,7 +50,7 @@ func EnableV4IPForwarding() error {
 
 func EnableV6IPForwarding(string) (map[string]int, error) {
 	log.Infof("Enable IPv6 forwarding is not implemented on %s", runtime.GOOS)
-	return nil, nil
+	return map[string]int{}, nil
 }
 
 func DisableV6IPForwarding(map[string]int) error {