optional DAD skip for managed VIP addresses

This PR adds an explicit configuration option `vip_skipdad`. This allows
for a proper behaviour in IPv6 networks with RT mode and ECMP. Without
this option only one node can ever hold the same IPv6 VIP what defeats
the purpose of ECMP.

While the ARP mode currently has a recovery mode for failed DAD, RT mode
misses it. It is also more feasible to use config option, as recovery
mode in RT mode would always introduce a ~5s blackholing (we know there
will be a conflict, so first attempt to add a VIP will fail and only the
next one will force IFA_F_NODAD).

For simplicitly we ignore IPv4/IPv6 checks, as kernel transparently
strips IFA_F_NODAD from v4 addresses without throwing errors.

For systems with IPv4 duplicate detection (called ACD and used e.g. in
RHEL10's NetworkManager) this code has no effect as it's only userspace
action and addresses we add via netlink bypass any check.

First discovered on a dual-stack cluster with two routing-table-mode
advertisers: the ingress VIP answered or timed out depending on which
ECMP path the flow hashed to; with NODAD both advertisers accept
traffic.

Signed-off-by: Mat Kowalski <mko@redhat.com>

pkg/cluster/cluster.go

@@ -81,7 +81,7 @@ func startNetworking(c *kubevip.Config, intfMgr *networkinterface.Manager) ([]vi
 		network, err := vip.NewConfig(addr, c.Interface, c.LoInterfaceGlobalScope, c.VIPSubnet, c.DDNS, c.DHCPMode,
 			c.RequireDualStack, c.IsDualStack, c.RoutingTableID, c.RoutingTableType, c.RoutingProtocol, c.DNSMode,
 			c.LoadBalancerForwardingMethod, c.IptablesBackend, c.EnableLoadBalancer, c.LoadBalancerPort,
-			c.EnableServiceSecurity, intfMgr, c.EgressWithNftables)
+			c.EnableServiceSecurity, intfMgr, c.EgressWithNftables, c.SkipDAD)
 		if err != nil {
 			return nil, err
 		}

pkg/instance/instance.go

@@ -208,6 +208,7 @@ func NewInstance(ctx context.Context, svc *v1.Service, config *kubevip.Config,
 			RoutingTableID:              config.RoutingTableID,
 			RoutingTableType:            config.RoutingTableType,
 			RoutingProtocol:             config.RoutingProtocol,
+			SkipDAD:                     config.SkipDAD,
 			ArpBroadcastRate:            config.ArpBroadcastRate,
 			EnableServiceSecurity:       config.EnableServiceSecurity,
 			DNSMode:                     config.DNSMode,
@@ -273,6 +274,7 @@ func NewInstance(ctx context.Context, svc *v1.Service, config *kubevip.Config,
 			RoutingTableID:         config.RoutingTableID,
 			RoutingTableType:       config.RoutingTableType,
 			RoutingProtocol:        config.RoutingProtocol,
+			SkipDAD:                config.SkipDAD,
 			ArpBroadcastRate:       config.ArpBroadcastRate,
 			EnableServiceSecurity:  config.EnableServiceSecurity,
 			DNSMode:                config.DNSMode,

pkg/kubevip/config_environment.go

@@ -406,6 +406,16 @@ func ParseEnvironment(c *Config) error {
 		c.CleanRoutingTable = b
 	}
 
+	// Skip Duplicate Address Detection when adding the VIP address
+	env = os.Getenv(vipSkipDAD)
+	if env != "" {
+		b, err := strconv.ParseBool(env)
+		if err != nil {
+			return err
+		}
+		c.SkipDAD = b
+	}
+
 	// DNS mode
 	env = os.Getenv(dnsMode)
 	if env != "" {

pkg/kubevip/config_environment_test.go

@@ -0,0 +1,41 @@
+package kubevip
+
+import (
+	"testing"
+)
+
+func TestParseEnvironmentSkipDAD(t *testing.T) {
+	cases := []struct {
+		name    string
+		value   string
+		want    bool
+		wantErr bool
+	}{
+		{name: "unset keeps default false", value: "", want: false},
+		{name: "true enables", value: "true", want: true},
+		{name: "false disables", value: "false", want: false},
+		{name: "garbage errors", value: "not-a-bool", wantErr: true},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.value != "" {
+				t.Setenv(vipSkipDAD, tc.value)
+			}
+			c := &Config{}
+			err := ParseEnvironment(c)
+			if tc.wantErr {
+				if err == nil {
+					t.Fatal("expected an error, got nil")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if c.SkipDAD != tc.want {
+				t.Fatalf("SkipDAD = %v, want %v", c.SkipDAD, tc.want)
+			}
+		})
+	}
+}

pkg/kubevip/config_envvar.go

@@ -181,6 +181,9 @@ const (
 	// vipCleanRoutingTable - defines if routing table will be cleaned of redundant routes on kube-vip's start
 	vipCleanRoutingTable = "vip_cleanroutingtable" //nolint
 
+	// vipSkipDAD - defines if Duplicate Address Detection is skipped when adding the VIP address (IFA_F_NODAD)
+	vipSkipDAD = "vip_skipdad" //nolint
+
 	// cpNamespace defines the namespace the control plane pods will run in
 	cpNamespace = "cp_namespace"
 

pkg/kubevip/config_types.go

@@ -142,6 +142,9 @@ type Config struct {
 	// Clean routing table of redundant routes on start
 	CleanRoutingTable bool `yaml:"cleanRoutingTable"`
 
+	// Skip Duplicate Address Detection when adding the VIP address (IFA_F_NODAD)
+	SkipDAD bool `yaml:"skipDAD"`
+
 	// BGP Configuration
 	BGPConfig     BGPConfig
 	BGPPeerConfig BGPPeer

pkg/vip/address.go

@@ -116,13 +116,17 @@ type network struct {
 	ipvsMark uint32
 
 	ipvsPort uint16
+
+	// dadSkip marks the address with IFA_F_NODAD on every add:
+	// anycast semantics, e.g. ECMP, must not use DAD
+	dadSkip bool
 }
 
 // NewConfig will attempt to provide an interface to the kernel network configuration
 func NewConfig(address string, iface string, loGlobalScope bool, subnet string, isDDNS bool,
 	dhcpMode string, requireDualStack, isDualStack bool, tableID int, tableType int, routingProtocol int,
 	dnsMode, forwardMethod, iptablesBackend string, ipvsEnabled bool, ipvsPort uint16, enableSecurity bool,
-	intfMgr *networkinterface.Manager, nftables bool) ([]Network, error) {
+	intfMgr *networkinterface.Manager, nftables bool, skipDAD bool) ([]Network, error) {
 	networks := []Network{}
 
 	link, err := netlink.LinkByName(iface)
@@ -147,6 +151,7 @@ func NewConfig(address string, iface string, loGlobalScope bool, subnet string,
 			nftables:         nftables,
 			ipvsMark:         ipvsMark,
 			ipvsPort:         ipvsPort,
+			dadSkip:          skipDAD,
 		}
 
 		subnet, err = SelectSubnet(address, subnet)
@@ -201,6 +206,7 @@ func NewConfig(address string, iface string, loGlobalScope bool, subnet string,
 						nftables:         nftables,
 						ipvsMark:         ipvsMark,
 						ipvsPort:         ipvsPort,
+						dadSkip:          skipDAD,
 					}
 
 					networks = append(networks, result)
@@ -222,6 +228,7 @@ func NewConfig(address string, iface string, loGlobalScope bool, subnet string,
 						nftables:         nftables,
 						ipvsMark:         ipvsMark,
 						ipvsPort:         ipvsPort,
+						dadSkip:          skipDAD,
 					}
 
 					networks = append(networks, result)
@@ -247,6 +254,7 @@ func NewConfig(address string, iface string, loGlobalScope bool, subnet string,
 				nftables:         nftables,
 				ipvsMark:         ipvsMark,
 				ipvsPort:         ipvsPort,
+				dadSkip:          skipDAD,
 			}
 
 			s, err := SelectSubnet(ip, subnet)
@@ -420,6 +428,13 @@ func (configurator *network) UpdateRoutes() (bool, error) {
 	return isUpdated, nil
 }
 
+// shouldSkipDAD returns whether the address must carry the IFA_F_NODAD flag:
+// either because configuration or because the caller requests it for this
+// specific add (e.g. DADFAILED state recovery in ARP mode).
+func (configurator *network) shouldSkipDAD(override bool) bool {
+	return override || configurator.dadSkip
+}
+
 // AddIP - Add an IP address to the interface
 // precheck: if true, check if the IP already exists before adding
 // skipDAD: if true, set IFA_F_NODAD flag for IPv6 addresses to skip Duplicate Address Detection
@@ -447,8 +462,10 @@ func (configurator *network) AddIP(precheck bool, skipDAD bool, minLifetime ...i
 	// For IPv6 addresses, optionally set NODAD flag to skip Duplicate Address Detection (DAD)
 	// This prevents DADFAILED loops when recovering from a previous DADFAILED state
 	// The flag tells the kernel to skip DAD, which is safe when we're re-adding
-	// an address that we know should be ours (e.g., after DADFAILED recovery)
-	if skipDAD && utils.IsIPv6(configurator.address.IP.String()) {
+	// an address that we know should be ours (e.g., after DADFAILED recovery).
+	// We also allow to globally configure NODAD in case user knows they are running in an
+	// environment where multiple nodes may advertise the same VIP (e.g., ECMP routing).
+	if configurator.shouldSkipDAD(skipDAD) && utils.IsIPv6(configurator.address.IP.String()) {
 		configurator.address.Flags |= unix.IFA_F_NODAD
 		log.Debug("Setting IFA_F_NODAD flag for IPv6 address to skip DAD", "ip", configurator.address.IP.String())
 	}

pkg/vip/address_test.go

@@ -0,0 +1,28 @@
+package vip
+
+import (
+	"testing"
+)
+
+func TestShouldSkipDAD(t *testing.T) {
+	cases := []struct {
+		name     string
+		dadSkip  bool
+		override bool
+		want     bool
+	}{
+		{name: "default: DAD runs", dadSkip: false, override: false, want: false},
+		{name: "DAD skip enabled", dadSkip: true, override: false, want: true},
+		{name: "per-call override skips DAD", dadSkip: false, override: true, want: true},
+		{name: "both set", dadSkip: true, override: true, want: true},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			n := &network{dadSkip: tc.dadSkip}
+			if got := n.shouldSkipDAD(tc.override); got != tc.want {
+				t.Fatalf("shouldSkipDAD(%v) with dadSkip=%v = %v, want %v", tc.override, tc.dadSkip, got, tc.want)
+			}
+		})
+	}
+}