From d0cf7b55bfbfcd35395537296f035047ab8db22f Mon Sep 17 00:00:00 2001
From: Eran Ifrach <eifrach@redhat.com>
Date: Sun, 22 Dec 2024 17:26:48 +0200
Subject: [PATCH] OCPBUGS-43352: calculate outgoing interface for arping

---
 src/connectivity_check/ipv4_arping_checker.go | 105 ++++++++++--------
 1 file changed, 60 insertions(+), 45 deletions(-)

diff --git a/src/connectivity_check/ipv4_arping_checker.go b/src/connectivity_check/ipv4_arping_checker.go
index 0119cc787..a3be39c63 100644
--- a/src/connectivity_check/ipv4_arping_checker.go
+++ b/src/connectivity_check/ipv4_arping_checker.go
@@ -1,6 +1,7 @@
 package connectivity_check
 
 import (
+	"net"
 	"os/exec"
 	"regexp"
 	"sort"
@@ -24,61 +25,75 @@ func (p *arpingChecker) Check(attributes Attributes) ResultReporter {
 		return nil
 	}
 
-	result, err := p.executer.Execute("arping", "-c", "10", "-w", "5", "-I", attributes.OutgoingNIC.Name, attributes.RemoteIPAddress)
-	if err != nil {
-		// Ignore exit code of 1; only 2 or -1 are actual errors
-		if exitErr, ok := err.(*exec.ExitError); !ok || exitErr.ExitCode() != 1 {
-			log.WithError(err).Error("Error while processing 'arping' command")
+	for _, addr := range attributes.OutgoingNIC.Addresses {
+		_, nicNetwork, err := net.ParseCIDR(addr.String())
+		if err != nil {
+			log.WithError(err).Errorf("Cannot Parse CIDR: %s", addr.String())
 			return nil
 		}
-	}
-	lines := strings.Split(result, "\n")
-	if len(lines) == 0 {
-		log.Warn("Missing output for arping")
-		return nil
-	}
 
-	hRegex := regexp.MustCompile("^ARPING ([^ ]+) from ([^ ]+) ([^ ]+)$")
-	parts := hRegex.FindStringSubmatch(lines[0])
-	if len(parts) != 4 {
-		log.Warnf("Wrong format for header line: %s", lines[0])
-		return nil
-	}
-
-	outgoingIpAddress := parts[2]
-	rRegexp := regexp.MustCompile(`^Unicast reply from ([^ ]+) \[([^]]+)\]  [^ ]+$`)
-
-	// We use a map to de-duplicate arping responses from the same mac. They are redundant
-	// because the reason we're reporting more than one line of arping in the first place is to allow
-	// the service to also detect devices in the network that have IP conflict with cluster hosts
-	returnValues := make(map[string]*models.L2Connectivity)
-	for _, line := range lines[1:] {
-		parts = rRegexp.FindStringSubmatch(line)
-		if len(parts) != 3 {
+		remoteIP := net.ParseIP(attributes.RemoteIPAddress)
+		if !nicNetwork.Contains(remoteIP) {
 			continue
 		}
-		remoteMAC := strings.ToLower(parts[2])
-		successful := macInDstMacs(remoteMAC, attributes.RemoteMACAddresses)
-		if !successful {
-			log.Warnf("Unexpected mac address for arping %s on nic %s: %s", attributes.RemoteIPAddress, attributes.OutgoingNIC.Name, remoteMAC)
+		result, err := p.executer.Execute("arping", "-c", "10", "-w", "5", "-I", attributes.OutgoingNIC.Name, attributes.RemoteIPAddress)
+		if err != nil {
+			// Ignore exit code of 1; only 2 or -1 are actual errors
+			if exitErr, ok := err.(*exec.ExitError); !ok || exitErr.ExitCode() != 1 {
+				log.WithError(err).Error("Error while processing 'arping' command")
+				return nil
+			}
 		}
-		if strings.ToLower(attributes.RemoteMACAddress) != remoteMAC {
-			log.Infof("Received remote mac %s different then expected mac %s", remoteMAC, attributes.RemoteMACAddress)
+		lines := strings.Split(result, "\n")
+		if len(lines) == 0 {
+			log.Warn("Missing output for arping")
+			return nil
 		}
 
-		returnValues[remoteMAC] = &models.L2Connectivity{
-			OutgoingIPAddress: outgoingIpAddress,
-			OutgoingNic:       attributes.OutgoingNIC.Name,
-			RemoteIPAddress:   attributes.RemoteIPAddress,
-			RemoteMac:         remoteMAC,
-			Successful:        successful,
+		hRegex := regexp.MustCompile("^ARPING ([^ ]+) from ([^ ]+) ([^ ]+)$")
+		parts := hRegex.FindStringSubmatch(lines[0])
+		if len(parts) != 4 {
+			log.Warnf("Wrong format for header line: %s", lines[0])
+			return nil
 		}
+
+		outgoingIpAddress := parts[2]
+		rRegexp := regexp.MustCompile(`^Unicast reply from ([^ ]+) \[([^]]+)\]  [^ ]+$`)
+
+		// We use a map to de-duplicate arping responses from the same mac. They are redundant
+		// because the reason we're reporting more than one line of arping in the first place is to allow
+		// the service to also detect devices in the network that have IP conflict with cluster hosts
+		returnValues := make(map[string]*models.L2Connectivity)
+		for _, line := range lines[1:] {
+			parts = rRegexp.FindStringSubmatch(line)
+			if len(parts) != 3 {
+				continue
+			}
+			remoteMAC := strings.ToLower(parts[2])
+			successful := macInDstMacs(remoteMAC, attributes.RemoteMACAddresses)
+			if !successful {
+				log.Warnf("Unexpected mac address for arping %s on nic %s: %s", attributes.RemoteIPAddress, attributes.OutgoingNIC.Name, remoteMAC)
+			}
+			if strings.ToLower(attributes.RemoteMACAddress) != remoteMAC {
+				log.Infof("Received remote mac %s different then expected mac %s", remoteMAC, attributes.RemoteMACAddress)
+			}
+
+			returnValues[remoteMAC] = &models.L2Connectivity{
+				OutgoingIPAddress: outgoingIpAddress,
+				OutgoingNic:       attributes.OutgoingNIC.Name,
+				RemoteIPAddress:   attributes.RemoteIPAddress,
+				RemoteMac:         remoteMAC,
+				Successful:        successful,
+			}
+		}
+		var l2Connectivity []*models.L2Connectivity
+		for _, v := range returnValues {
+			l2Connectivity = append(l2Connectivity, v)
+		}
+		return newArpingResultReporter(l2Connectivity)
 	}
-	var l2Connectivity []*models.L2Connectivity
-	for _, v := range returnValues {
-		l2Connectivity = append(l2Connectivity, v)
-	}
-	return newArpingResultReporter(l2Connectivity)
+
+	return nil
 }
 
 func (p *arpingChecker) Finalize(resultingHost *models.ConnectivityRemoteHost) {