Fix the issue GARP not being sent

pkg/agent/cniserver/interface_configuration_linux.go

@@ -98,46 +98,41 @@ func configureContainerAddr(netns ns.NetNS, containerInterface *current.Interfac
 // installed async, and the gratuitous ARP could be sent out after the Openflow entries are installed. Using another
 // goroutine to ensure the processing of CNI ADD request is not blocked.
 func (ic *ifConfigurator) advertiseContainerAddr(containerNetNS string, containerIfaceName string, result *current.Result) error {
-	netns, err := ns.GetNS(containerNetNS)
-	if err != nil {
-		klog.Errorf("Failed to open netns with %s: %v", containerNetNS, err)
-		return err
+	if err := ns.IsNSorErr(containerNetNS); err != nil {
+		return fmt.Errorf("%s is not a valid network namespace: %v", containerNetNS, err)
 	}
-	defer netns.Close()
-	if err := netns.Do(func(containerNs ns.NetNS) error {
-		go func() {
-			// The container veth must exist when this function is called, and do not check error here.
-			containerVeth, err := net.InterfaceByName(containerIfaceName)
-			if err != nil {
-				klog.Errorf("Failed to find container interface %s in ns %s", containerIfaceName, netns.Path())
-				return
-			}
-			var targetIP net.IP
-			for _, ipc := range result.IPs {
-				if ipc.Version == "4" {
-					targetIP = ipc.Address.IP
-					arping.GratuitousArpOverIface(ipc.Address.IP, *containerVeth)
-				}
-			}
-			if targetIP == nil {
-				klog.Warning("Failed to find a valid IP address for Gratuitous ARP, not send GARP")
-				return
+	// Sending Gratuitous ARP is a best-effort action and is unlikely to fail as we have ensured the netns is valid.
+	go ns.WithNetNSPath(containerNetNS, func(_ ns.NetNS) error {
+		iface, err := net.InterfaceByName(containerIfaceName)
+		if err != nil {
+			klog.Errorf("Failed to find container interface %s in ns %s: %v", containerIfaceName, containerNetNS, err)
+			return nil
+		}
+		var targetIP net.IP
+		for _, ipc := range result.IPs {
+			if ipc.Version == "4" {
+				targetIP = ipc.Address.IP
 			}
-			count := 0
-			for count < 3 {
-				select {
-				case <-time.Tick(50 * time.Millisecond):
-					// Send gratuitous ARP to network in case of stale mappings for this IP address
-					// (e.g. if a previous - deleted - Pod was using the same IP).
-					arping.GratuitousArpOverIface(targetIP, *containerVeth)
-				}
-				count++
+		}
+		if targetIP == nil {
+			klog.Warning("Failed to find an IPv4 address, skipped sending Gratuitous ARP")
+			return nil
+		}
+		ticker := time.NewTicker(50 * time.Millisecond)
+		defer ticker.Stop()
+		count := 0
+		for {
+			// Send gratuitous ARP to network in case of stale mappings for this IP address
+			// (e.g. if a previous - deleted - Pod was using the same IP).
+			arping.GratuitousArpOverIface(targetIP, *iface)
+			count++
+			if count == 3 {
+				break
 			}
-		}()
+			<-ticker.C
+		}
 		return nil
-	}); err != nil {
-		return err
-	}
+	})
 	return nil
 }
 

test/e2e/basic_test.go

@@ -593,3 +593,54 @@ func TestDeletePreviousRoundFlowsOnStartup(t *testing.T) {
 		t.Errorf("Flow was still present after timeout")
 	}
 }
+
+// TestGratuitousARP verifies that we receive 3 GARP packets after a Pod is up.
+// There might be ARP packets other than GARP sent if there is any unintentional
+// traffic. So we just check the number of ARP packets is greater than 3.
+func TestGratuitousARP(t *testing.T) {
+	data, err := setupTest(t)
+	if err != nil {
+		t.Fatalf("Error when setting up test: %v", err)
+	}
+	defer teardownTest(t, data)
+
+	podName := randName("test-pod-")
+	nodeName := workerNodeName(1)
+
+	t.Logf("Creating Pod '%s' on '%s'", podName, nodeName)
+	if err := data.createBusyboxPodOnNode(podName, nodeName); err != nil {
+		t.Fatalf("Error when creating Pod '%s': %v", podName, err)
+	}
+	defer deletePodWrapper(t, data, podName)
+
+	antreaPodName, err := data.getAntreaPodOnNode(nodeName)
+	if err != nil {
+		t.Fatalf("Error when retrieving the name of the Antrea Pod running on Node '%s': %v", nodeName, err)
+	}
+
+	podIP, err := data.podWaitForIP(defaultTimeout, podName, testNamespace)
+	if err != nil {
+		t.Fatalf("Error when waiting for IP for Pod '%s': %v", podName, err)
+	}
+
+	// Sending GARP is an asynchronous operation. The last GARP is supposed to
+	// be sent 100ms after processing CNI ADD request.
+	time.Sleep(100 * time.Millisecond)
+
+	cmd := []string{"ovs-ofctl", "dump-flows", defaultBridgeName, fmt.Sprintf("table=10,arp,arp_spa=%s", podIP)}
+	stdout, _, err := data.runCommandFromPod(antreaNamespace, antreaPodName, ovsContainerName, cmd)
+	if err != nil {
+		t.Fatalf("Error when querying openflow: %v", err)
+	}
+
+	re := regexp.MustCompile(`n_packets=([0-9]+)`)
+	matches := re.FindStringSubmatch(stdout)
+	if len(matches) == 0 {
+		t.Fatalf("cannot retrieve n_packets, unexpected output: %s", stdout)
+	}
+	arpPackets, _ := strconv.ParseUint(matches[1], 10, 32)
+	if arpPackets < 3 {
+		t.Errorf("Expected at least 3 ARP packets, got %d", arpPackets)
+	}
+	t.Logf("Got %d ARP packets after Pod was up", arpPackets)
+}