From 39f8bd736693c86c84e92348f6b53e9ce300a4a6 Mon Sep 17 00:00:00 2001 From: Patryk Diak Date: Tue, 14 Oct 2025 15:09:06 +0200 Subject: [PATCH 1/2] Fix GARP sending 0.0.0.0 due to incorrect IPv4 byte extraction net.ParseIP() returns 16-byte IPv4-mapped IPv6 format where IPv4 bytes are at the END, not beginning. [4]byte(garp.IP) took wrong bytes. Fixed by calling To4() before, forcing validated creation via NewGARP(). Interface prevents bypassing extracting the correct IPv4 address. Signed-off-by: Patryk Diak (cherry picked from commit cc15dc1c0b87dcd98e2c8eb48fe06080aede552a) Conflicts: go-controller/pkg/kubevirt/pod.go --- go-controller/pkg/kubevirt/pod.go | 5 +- .../node/linkmanager/link_network_manager.go | 9 +- go-controller/pkg/util/arp.go | 61 ++++++++++-- test/e2e/egressip.go | 99 +++++++++++++++++++ test/e2e/util.go | 4 +- 5 files changed, 163 insertions(+), 15 deletions(-) diff --git a/go-controller/pkg/kubevirt/pod.go b/go-controller/pkg/kubevirt/pod.go index 8cde9d713e..c4fd4166d2 100644 --- a/go-controller/pkg/kubevirt/pod.go +++ b/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 } diff --git a/go-controller/pkg/node/linkmanager/link_network_manager.go b/go-controller/pkg/node/linkmanager/link_network_manager.go index ce047965e5..d188b854df 100644 --- a/go-controller/pkg/node/linkmanager/link_network_manager.go +++ b/go-controller/pkg/node/linkmanager/link_network_manager.go @@ -196,8 +196,13 @@ 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) } } diff --git a/go-controller/pkg/util/arp.go b/go-controller/pkg/util/arp.go index d2205eafeb..c9d74bcedf 100644 --- a/go-controller/pkg/util/arp.go +++ b/go-controller/pkg/util/arp.go @@ -6,13 +6,53 @@ 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 @@ -20,15 +60,15 @@ type GARP struct { // 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 } diff --git a/test/e2e/egressip.go b/test/e2e/egressip.go index 0b782fc97c..3bee766acc 100644 --- a/test/e2e/egressip.go +++ b/test/e2e/egressip.go @@ -2957,6 +2957,105 @@ 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 utilnet.IsIPv6(net.ParseIP(egress1Node.nodeIP)) { + ginkgo.Skip("GARP test only supports IPv4") + } + + if isUserDefinedNetwork(netConfigParams) { + ginkgo.Skip("Unsupported for UDNs") + } + + egressIPSecondaryHost := "10.10.10.220" + + // 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 + _, 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") + + 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 diff --git a/test/e2e/util.go b/test/e2e/util.go index 85be483cce..f267f819ac 100644 --- a/test/e2e/util.go +++ b/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 From 6ea54c78ac0752165c76bf8e6b83008f6b13312c Mon Sep 17 00:00:00 2001 From: Patryk Diak Date: Thu, 16 Oct 2025 10:48:09 +0200 Subject: [PATCH 2/2] Send Unsolicited Neighbor Advertisement for IPv6 EgressIPs IPv6 equivalent of GARP functionality for EgressIP failover. Sends unsolicited NAs when IPv6 addresses are added to secondary host interfaces. Signed-off-by: Patryk Diak (cherry picked from commit 9eb161768d7ae17988d97da5e55e3c5a095c30e1) --- .../node/linkmanager/link_network_manager.go | 13 +++ go-controller/pkg/util/ndp/na.go | 104 ++++++++++++++++++ test/e2e/egressip.go | 24 ++-- 3 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 go-controller/pkg/util/ndp/na.go diff --git a/go-controller/pkg/node/linkmanager/link_network_manager.go b/go-controller/pkg/node/linkmanager/link_network_manager.go index d188b854df..d43442e4d8 100644 --- a/go-controller/pkg/node/linkmanager/link_network_manager.go +++ b/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. @@ -205,6 +206,18 @@ func (c *Controller) syncLink(link netlink.Link) error { 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) + } } klog.Infof("Link manager: completed adding address %s to link %s", addressWanted, linkName) } diff --git a/go-controller/pkg/util/ndp/na.go b/go-controller/pkg/util/ndp/na.go new file mode 100644 index 0000000000..b83310c27c --- /dev/null +++ b/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 +} diff --git a/test/e2e/egressip.go b/test/e2e/egressip.go index 3bee766acc..282d23c515 100644 --- a/test/e2e/egressip.go +++ b/test/e2e/egressip.go @@ -2958,15 +2958,15 @@ spec: }) ginkgo.It("[secondary-host-eip] should send address advertisements for EgressIP", func() { - if utilnet.IsIPv6(net.ParseIP(egress1Node.nodeIP)) { - ginkgo.Skip("GARP test only supports IPv4") - } - 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, @@ -2983,9 +2983,19 @@ spec: // The following is required for the test purposes since we are sending and unsolicited advertisement // for an address that is not tracked already - _, 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") + 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}