diff --git a/pkg/agent/cniserver/interface_configuration_linux.go b/pkg/agent/cniserver/interface_configuration_linux.go index 098e0959d3f..cd267dd3834 100644 --- a/pkg/agent/cniserver/interface_configuration_linux.go +++ b/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 } diff --git a/test/e2e/basic_test.go b/test/e2e/basic_test.go index 374f4747710..78f1bb5250b 100755 --- a/test/e2e/basic_test.go +++ b/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) +}