From d4e80b67c3bce38a32a631bf505c5d3cd1a5be78 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 30 May 2024 13:03:51 +0200 Subject: [PATCH] Wait for carrier before announcing IPs via GARP/NA After setting up the interface, it might take a bit for kernel to detect carrier. If we then already send the GARP/NA packets, they are lost. Instead, wait for up to 200 msec for the interface to get carrier. This time is chosen somewhat arbitrarily. We don't want to block the process too long, but we also need to wait long enough, that kernel and driver has time to detect carrier. Also, while busy waiting, sleep with an exponential back-off time (growth factor 1.5). Fixes: c241dcb4367c ('Send IPv4 GARP and IPv6 Unsolicited NA in "cmdAdd"') See-also: https://issues.redhat.com/browse/OCPBUGS-30549 Signed-off-by: Thomas Haller --- cmd/sriov/main.go | 12 +++++++++--- pkg/utils/packet.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/cmd/sriov/main.go b/cmd/sriov/main.go index 6f34bde0c..cfd358913 100644 --- a/cmd/sriov/main.go +++ b/cmd/sriov/main.go @@ -5,6 +5,7 @@ import ( "fmt" "runtime" "strings" + "time" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" @@ -205,10 +206,15 @@ func cmdAdd(args *skel.CmdArgs) error { * may be sending packets with the incorrect link-layer address. Eventually, most network stacks would send ARPs and/or Neighbor * Solicitation packets when the connection is unreachable. This would correct the invalid cache; however this may take a significant * amount of time to complete. - * - * The error is ignored here because enabling this feature is only a performance enhancement. */ - _ = utils.AnnounceIPs(args.IfName, result.IPs) + + /* The interface might not yet have carrier. Wait for it for a short time. */ + hasCarrier := utils.WaitForCarrier(args.IfName, 200*time.Millisecond) + + /* The error is ignored here because enabling this feature is only a performance enhancement. */ + err = utils.AnnounceIPs(args.IfName, result.IPs) + + logging.Debug("announcing IPs", "hasCarrier", hasCarrier, "IPs", result.IPs, "announceError", err) return nil }) } diff --git a/pkg/utils/packet.go b/pkg/utils/packet.go index 7628dc820..88a52bc78 100644 --- a/pkg/utils/packet.go +++ b/pkg/utils/packet.go @@ -6,11 +6,13 @@ import ( "fmt" "net" "syscall" + "time" current "github.com/containernetworking/cni/pkg/types/100" "github.com/vishvananda/netlink" "golang.org/x/net/icmp" "golang.org/x/net/ipv6" + "golang.org/x/sys/unix" ) var ( @@ -196,3 +198,38 @@ func AnnounceIPs(ifName string, ipConfigs []*current.IPConfig) error { } return nil } + +// Blocking wait for interface ifName to have carrier (!NO_CARRIER flag). +func WaitForCarrier(ifName string, waitTime time.Duration) bool { + var sleepDuration time.Duration + + myNetLink := MyNetlink{} + + start := time.Now() + + for time.Since(start) < waitTime { + if sleepDuration == 0 { + sleepDuration = 2 * time.Millisecond + } else { + time.Sleep(sleepDuration) + /* Grow wait time exponentionally (factor 1.5). */ + sleepDuration += sleepDuration / 2 + } + + linkObj, err := myNetLink.LinkByName(ifName) + if err != nil { + return false + } + + /* Wait for carrier, i.e. IFF_UP|IFF_RUNNING. Note that there is also + * IFF_LOWER_UP, but we follow iproute2 ([1]). + * + * [1] https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/ipaddress.c?id=f9601b10c21145f76c3d46c163bac39515ed2061#n86 + */ + if linkObj.Attrs().RawFlags&(unix.IFF_UP|unix.IFF_RUNNING) == (unix.IFF_UP | unix.IFF_RUNNING) { + return true + } + } + + return false +}