OCPBUGS-65618: [release-4.20] OCP4: 4.18.22 EgressIP Failover does not succeed - extended downtime and no reassignment between egress-capable peers

go-controller/pkg/kubevirt/pod.go

@@ -532,7 +532,10 @@ func (r *DefaultGatewayReconciler) ReconcileIPv4AfterLiveMigration(liveMigration
 		if gwIP == nil {
 			continue
 		}
-		garp := util.GARP{IP: gwIP, MAC: &lrpMAC}
+		garp, err := util.NewGARP(gwIP, &lrpMAC)
+		if err != nil {
+			return fmt.Errorf("failed to create GARP for gateway IP %s: %w", gwIP, err)
+		}
 		if err := util.BroadcastGARP(r.interfaceName, garp); err != nil {
 			return err
 		}

go-controller/pkg/node/linkmanager/link_network_manager.go

@@ -11,6 +11,7 @@ import (
 	utilnet "k8s.io/utils/net"
 
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util/ndp"
 )
 
 // Gather all suitable interface address + network mask and offer this as a service.
@@ -196,8 +197,25 @@ func (c *Controller) syncLink(link netlink.Link) error {
 		// For IPv4, use arping to try to update other hosts ARP caches, in case this IP was
 		// previously active on another node
 		if addressWanted.IP.To4() != nil {
-			if err = util.BroadcastGARP(linkName, util.GARP{IP: addressWanted.IP}); err != nil {
-				klog.Errorf("Failed to send a GARP for IP %s over interface %s: %v", addressWanted.IP.String(),
+			garp, err := util.NewGARP(addressWanted.IP, nil)
+			if err != nil {
+				klog.Errorf("Link manager: failed to create GARP for IP %s: %v", addressWanted.IP.String(), err)
+				continue
+			}
+			if err = util.BroadcastGARP(linkName, garp); err != nil {
+				klog.Errorf("Link manager: failed to send GARP for IP %s over interface %s: %v", addressWanted.IP.String(),
+					linkName, err)
+			}
+		} else if addressWanted.IP.To16() != nil {
+			// For IPv6, send an unsolicited neighbor advertisement to update neighbor caches, in case this IP was
+			// previously active on another node
+			na, err := ndp.NewNeighborAdvertisement(addressWanted.IP, nil)
+			if err != nil {
+				klog.Errorf("Link manager: failed to create NeighborAdvertisement for IP %s: %v", addressWanted.IP.String(), err)
+				continue
+			}
+			if err = ndp.SendUnsolicitedNeighborAdvertisement(linkName, na); err != nil {
+				klog.Errorf("Link manager: failed to send an unsolicited neighbor advertisement for IP %s over interface %s: %v", addressWanted.IP.String(),
 					linkName, err)
 			}
 		}

go-controller/pkg/util/arp.go

@@ -6,29 +6,69 @@ import (
 	"net/netip"
 
 	"github.com/mdlayher/arp"
+
+	"k8s.io/klog/v2"
 )
 
-type GARP struct {
-	// IP to advertise the MAC address
-	IP net.IP
-	// MAC to advertise (optional), default: link mac address
-	MAC *net.HardwareAddr
+// GARP represents a gratuitous ARP request for an IPv4 address.
+type GARP interface {
+	// IP returns the IPv4 address as a net.IP
+	IP() net.IP
+	// IPv4 returns the raw 4-byte IPv4 address
+	IPv4() [net.IPv4len]byte
+	// MAC returns the MAC address to advertise (nil means use interface MAC)
+	MAC() *net.HardwareAddr
+}
+
+// garp is the private implementation of GARP
+type garp struct {
+	ip  [4]byte
+	mac *net.HardwareAddr
+}
+
+// NewGARP creates a new GARP with validation that the IP is IPv4.
+// Returns error if the IP is not a valid IPv4 address.
+// mac can be nil to use the interface's MAC address.
+func NewGARP(ip net.IP, mac *net.HardwareAddr) (GARP, error) {
+	ip4 := ip.To4()
+	if ip4 == nil {
+		return nil, fmt.Errorf("GARP only supports IPv4 addresses, got %s (len=%d bytes)", ip.String(), len(ip))
+	}
+	return &garp{
+		ip:  [4]byte(ip4),
+		mac: mac,
+	}, nil
+}
+
+// IP returns the IPv4 address as a net.IP
+func (g *garp) IP() net.IP {
+	return net.IP(g.ip[:])
+}
+
+// IPv4 returns the raw 4-byte IPv4 address
+func (g *garp) IPv4() [4]byte {
+	return g.ip
+}
+
+// MAC returns the MAC address to advertise
+func (g *garp) MAC() *net.HardwareAddr {
+	return g.mac
 }
 
 // BroadcastGARP send a pair of GARPs with "request" and "reply" operations
 // since some system response to request and others to reply.
 // If "garp.MAC" is not passed the link form "interfaceName" mac will be
 // advertise
 func BroadcastGARP(interfaceName string, garp GARP) error {
-	srcIP := netip.AddrFrom4([4]byte(garp.IP))
-
 	iface, err := net.InterfaceByName(interfaceName)
 	if err != nil {
 		return fmt.Errorf("failed finding interface %s: %v", interfaceName, err)
 	}
 
-	if garp.MAC == nil {
-		garp.MAC = &iface.HardwareAddr
+	srcIP := netip.AddrFrom4(garp.IPv4())
+	mac := garp.MAC()
+	if mac == nil {
+		mac = &iface.HardwareAddr
 	}
 
 	c, err := arp.Dial(iface)
@@ -50,7 +90,7 @@ func BroadcastGARP(interfaceName string, garp GARP) error {
 	for _, op := range []arp.Operation{arp.OperationRequest, arp.OperationReply} {
 		// At at GARP the source and target IP should be the same and point to the
 		// the IP we want to reconcile -> https://wiki.wireshark.org/Gratuitous_ARP
-		p, err := arp.NewPacket(op, *garp.MAC /* srcHw */, srcIP, net.HardwareAddr{0, 0, 0, 0, 0, 0}, srcIP)
+		p, err := arp.NewPacket(op, *mac /* srcHw */, srcIP, net.HardwareAddr{0, 0, 0, 0, 0, 0}, srcIP)
 		if err != nil {
 			return fmt.Errorf("failed creating %q GARP %+v: %w", op, garp, err)
 		}
@@ -60,5 +100,6 @@ func BroadcastGARP(interfaceName string, garp GARP) error {
 		}
 	}
 
+	klog.Infof("BroadcastGARP: completed GARP broadcast for IP %s on interface %s with MAC: %s", garp.IP().String(), interfaceName, mac.String())
 	return nil
 }

go-controller/pkg/util/ndp/na.go

@@ -0,0 +1,104 @@
+package ndp
+
+import (
+	"fmt"
+	"net"
+	"net/netip"
+
+	"github.com/mdlayher/ndp"
+	"golang.org/x/net/ipv6"
+
+	"k8s.io/klog/v2"
+)
+
+// NeighborAdvertisement represents a Neighbor Advertisement for an IPv6 address.
+type NeighborAdvertisement interface {
+	// IP returns the IPv6 address
+	IP() net.IP
+	// MAC returns the MAC address to advertise (nil means use interface MAC)
+	MAC() *net.HardwareAddr
+}
+
+type neighborAdvertisement struct {
+	ip  net.IP
+	mac *net.HardwareAddr
+}
+
+// NewNeighborAdvertisement creates a new Unsolicited Neighbor Advertisement with validation that the IP is IPv6.
+func NewNeighborAdvertisement(ip net.IP, mac *net.HardwareAddr) (NeighborAdvertisement, error) {
+	if ip.To4() != nil {
+		return nil, fmt.Errorf("only IPv6 addresses can be used for NeighborAdvertisement, got IPv4 %s", ip.String())
+	}
+	if ip.To16() == nil {
+		return nil, fmt.Errorf("only IPv6 addresses can be used for NeighborAdvertisement, got %s", ip.String())
+	}
+	if ip.IsMulticast() || ip.IsUnspecified() {
+		return nil, fmt.Errorf("invalid IPv6 NA target address: %s", ip.String())
+	}
+
+	return &neighborAdvertisement{
+		ip:  ip.To16(),
+		mac: mac,
+	}, nil
+}
+
+// IP returns the IPv6 address
+func (u *neighborAdvertisement) IP() net.IP {
+	return u.ip
+}
+
+// MAC returns the MAC address to advertise
+func (u *neighborAdvertisement) MAC() *net.HardwareAddr {
+	return u.mac
+}
+
+// SendUnsolicitedNeighborAdvertisement sends an unsolicited neighbor advertisement for the given IPv6 address.
+// If the mac address is not provided it will use the one from the interface.
+// The advertisement is sent to the all-nodes multicast address (ff02::1).
+// https://datatracker.ietf.org/doc/html/rfc4861#section-4.4
+func SendUnsolicitedNeighborAdvertisement(interfaceName string, na NeighborAdvertisement) error {
+	iface, err := net.InterfaceByName(interfaceName)
+	if err != nil {
+		return fmt.Errorf("failed finding interface %s: %v", interfaceName, err)
+	}
+
+	targetIP := na.IP()
+	mac := na.MAC()
+	if mac == nil {
+		mac = &iface.HardwareAddr
+	}
+
+	targetAddr, ok := netip.AddrFromSlice(targetIP)
+	if !ok {
+		return fmt.Errorf("failed to convert IP %s to netip.Addr", targetIP.String())
+	}
+
+	// Use unspecified address to handle cases where the advertised IP is not assigned to the interface.
+	c, _, err := ndp.Listen(iface, ndp.Unspecified)
+	if err != nil {
+		return fmt.Errorf("failed to create NDP connection on %s: %w", interfaceName, err)
+	}
+	defer c.Close()
+
+	// Unsolicited neighbor advertisement from a host, should override any existing cache entries
+	una := &ndp.NeighborAdvertisement{
+		Router:        false,
+		Solicited:     false,
+		Override:      true,
+		TargetAddress: targetAddr,
+		Options: []ndp.Option{
+			&ndp.LinkLayerAddress{
+				Direction: ndp.Target,
+				Addr:      *mac,
+			},
+		},
+	}
+
+	// rfc4861 - hop Limit 255 for unsolicited neighbor advertisements as per RFC, send to all-nodes multicast address
+	if err := c.WriteTo(una, &ipv6.ControlMessage{HopLimit: ndp.HopLimit}, netip.IPv6LinkLocalAllNodes()); err != nil {
+		return fmt.Errorf("failed to send an unsolicited neighbor advertisement for IP %s over interface %s: %w", targetIP.String(), interfaceName, err)
+	}
+
+	klog.Infof("Sent an unsolicited neighbor advertisement for IP %s on interface %s with MAC: %s", targetIP.String(), interfaceName, mac.String())
+	return nil
+}

test/e2e/egressip.go

@@ -2957,6 +2957,115 @@ spec:
 			"and verify the expected IP, failed for EgressIP %s: %v", egressIPName, err)
 	})
 
+	ginkgo.It("[secondary-host-eip] should send address advertisements for EgressIP", func() {
+		if isUserDefinedNetwork(netConfigParams) {
+			ginkgo.Skip("Unsupported for UDNs")
+		}
+
+		egressIPSecondaryHost := "10.10.10.220"
+		isV6Node := utilnet.IsIPv6(net.ParseIP(egress1Node.nodeIP))
+		if isV6Node {
+			egressIPSecondaryHost = "2001:db8:abcd:1234:c001::"
+		}
+
+		// flush any potentially stale MACs
+		_, err := infraprovider.Get().ExecExternalContainerCommand(secondaryTargetExternalContainer,
+			[]string{"ip", "neigh", "flush", egressIPSecondaryHost})
+		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should flush neighbor cache")
+
+		networks, err := providerCtx.GetAttachedNetworks()
+		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should get attached networks")
+		secondaryNetwork, exists := networks.Get(secondaryNetworkName)
+		gomega.Expect(exists).Should(gomega.BeTrue(), "network %s must exist", secondaryNetworkName)
+
+		inf, err := infraprovider.Get().GetExternalContainerNetworkInterface(secondaryTargetExternalContainer, secondaryNetwork)
+		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should have network interface for network %s on instance %s", secondaryNetwork.Name(), secondaryTargetExternalContainer.Name)
+
+		// The following is required for the test purposes since we are sending and unsolicited advertisement
+		// for an address that is not tracked already
+		if !isV6Node {
+			_, err = infraprovider.Get().ExecExternalContainerCommand(secondaryTargetExternalContainer,
+				[]string{"sysctl", "-w", fmt.Sprintf("net.ipv4.conf.%s.arp_accept=1", inf.InfName)})
+			gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should enable arp_accept")
+		} else {
+			_, err = infraprovider.Get().ExecExternalContainerCommand(secondaryTargetExternalContainer,
+				[]string{"sysctl", "-w", fmt.Sprintf("net.ipv6.conf.%s.forwarding=1", inf.InfName)})
+			gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should enable forwarding")
+
+			_, err = infraprovider.Get().ExecExternalContainerCommand(secondaryTargetExternalContainer,
+				[]string{"sysctl", "-w", fmt.Sprintf("net.ipv6.conf.%s.accept_untracked_na=1", inf.InfName)})
+			gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should enable accept_untracked_na")
+		}
+
+		podNamespace := f.Namespace
+		labels := map[string]string{"name": f.Namespace.Name}
+		updateNamespaceLabels(f, podNamespace, labels)
+
+		ginkgo.By("Labeling node as available for egress")
+		egressNodeAvailabilityHandler := egressNodeAvailabilityHandlerViaLabel{f}
+		egressNodeAvailabilityHandler.Enable(egress1Node.name)
+		defer egressNodeAvailabilityHandler.Restore(egress1Node.name)
+
+		_, err = createGenericPodWithLabel(f, pod1Name, egress1Node.name, f.Namespace.Name, []string{"/agnhost", "pause"}, podEgressLabel)
+		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should create egress pod")
+
+		egressIPConfig := `apiVersion: k8s.ovn.org/v1
+kind: EgressIP
+metadata:
+    name: ` + egressIPName + `
+spec:
+    egressIPs:
+    - "` + egressIPSecondaryHost + `"
+    podSelector:
+        matchLabels:
+            wants: egress
+    namespaceSelector:
+        matchLabels:
+            name: ` + f.Namespace.Name + `
+`
+		if err := os.WriteFile(egressIPYaml, []byte(egressIPConfig), 0644); err != nil {
+			framework.Failf("Unable to write CRD config to disk: %v", err)
+		}
+		defer func() {
+			if err := os.Remove(egressIPYaml); err != nil {
+				framework.Logf("Unable to remove the CRD config from disk: %v", err)
+			}
+		}()
+		e2ekubectl.RunKubectlOrDie("default", "create", "-f", egressIPYaml)
+
+		status := verifyEgressIPStatusLengthEquals(1, nil)
+		inf, err = infraprovider.Get().GetK8NodeNetworkInterface(status[0].Node, secondaryNetwork)
+		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should have network interface for network %s on instance %s", secondaryNetwork.Name(), egress1Node.name)
+
+		ginkgo.By("Verifying neighbor table")
+		var neighborMAC string
+		gomega.Eventually(func() bool {
+			output, err := infraprovider.Get().ExecExternalContainerCommand(secondaryTargetExternalContainer,
+				[]string{"ip", "-j", "neigh", "show", egressIPSecondaryHost})
+			if err != nil {
+				framework.Logf("Failed to get neighbor table: %v", err)
+				return false
+			}
+
+			var neighbors []IpNeighbor
+			if err := json.Unmarshal([]byte(output), &neighbors); err != nil {
+				framework.Logf("Failed to parse neighbor JSON: %v", err)
+				return false
+			}
+
+			for _, n := range neighbors {
+				if n.Lladdr != "" {
+					neighborMAC = n.Lladdr
+					framework.Logf("Neighbor entry found for %s -> MAC %s", egressIPSecondaryHost, neighborMAC)
+					return true
+				}
+			}
+			return false
+		}, 30*time.Second, 2*time.Second).Should(gomega.BeTrue(),
+			"Neighbor entry should appear")
+		gomega.Expect(neighborMAC).Should(gomega.Equal(inf.MAC), "neighbor entry should have the correct MAC address")
+	})
+
 	// two pods attached to different namespaces but the same role primary user defined network
 	// One pod is deleted and ensure connectivity for the other pod is ok
 	// The previous pod namespace is deleted and again, ensure connectivity for the other pod is ok

test/e2e/util.go

@@ -57,8 +57,8 @@ const (
 var singleNodePerZoneResult *bool
 
 type IpNeighbor struct {
-	Dst    string `dst`
-	Lladdr string `lladdr`
+	Dst    string `json:"dst"`
+	Lladdr string `json:"lladdr"`
 }
 
 // PodAnnotation describes the assigned network details for a single pod network. (The