zebra: rework arp-nd-redirect udp sock handling

Currently we only try to to open the UDP socket used for arp-nd-redirect if 1) the first ES is configured, or 2) the "base l2vni" changes. If the call to bind() fails during both of those triggers, then the sock will stay closed indefinitely. A customer hit a scenario where this happened due to the bind() getting called before the ES Originator IP (derived from the base l2vni's vxlan local-ip) was assigned to an interface as a local address. This adds an additional trigger point for the sock creation when an RTM_NEWADDR is received for an IPv4 address added to lo. Testing Done: evpn-mh-smoke Ticket: #3131423 Signed-off-by: Trey Aspelund <[email protected]>
donaldsharp · Dec 8, 2024 · f636782 · f636782
1 parent ebea3c5
commit f636782
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 40 deletions.
diff --git a/zebra/redistribute.c b/zebra/redistribute.c
@@ -17,6 +17,7 @@
 #include "vrf.h"
 #include "srcdest_table.h"
 #include "frrdistance.h"
+#include "if.h"
 
 #include "zebra/rib.h"
 #include "zebra/zebra_router.h"
@@ -30,6 +31,7 @@
 #include "zebra/zebra_vxlan.h"
 #include "zebra/zebra_errors.h"
 #include "zebra/zebra_neigh.h"
+#include "zebra/zebra_evpn_arp_nd.h"
 
 #define ZEBRA_PTM_SUPPORT
 
@@ -594,6 +596,9 @@ void zebra_interface_address_add_update(struct interface *ifp,
 
 	router_id_add_address(ifc);
 
+	if (if_is_loopback(ifp))
+		zebra_evpn_arp_nd_failover_enable();
+
 	for (ALL_LIST_ELEMENTS(zrouter.client_list, node, nnode, client)) {
 		/* Do not send unsolicited messages to synchronous clients. */
 		if (client->synchronous)

diff --git a/zebra/zebra_evpn_arp_nd.c b/zebra/zebra_evpn_arp_nd.c
@@ -578,51 +578,68 @@ void zebra_evpn_arp_nd_if_update(struct zebra_if *zif, bool enable)
  */
 void zebra_evpn_arp_nd_udp_sock_create(void)
 {
-	if (!(zevpn_arp_nd_info.flags & ZEBRA_EVPN_ARP_ND_FAILOVER))
-		return;
+	struct interface *ifp;
+	struct prefix lo_p;
+	struct connected *ifc = NULL;
+	struct sockaddr_in sin;
+	int reuse = 1;
 
-	if (zmh_info->es_originator_ip.s_addr) {
-		struct sockaddr_in sin;
-		int reuse = 1;
+	if (!zmh_info->es_originator_ip.s_addr)
+		goto close_sock;
+
+	lo_p.family = AF_INET;
+	lo_p.prefixlen = IPV4_MAX_BITLEN;
+	lo_p.u.prefix4 = zmh_info->es_originator_ip;
+
+	ifp = if_lookup_by_name("lo", VRF_DEFAULT);
+	if (!ifp)
+		goto close_sock;
+
+	ifc = connected_lookup_prefix_exact(ifp, &lo_p);
+	if (!ifc)
+		goto close_sock;
+
+	if (IS_ZEBRA_DEBUG_EVPN_MH_ARP_ND_EVT)
+		zlog_debug("Create UDP sock for arp_nd redirect from %pI4",
+			   &zmh_info->es_originator_ip);
+
+	if (zevpn_arp_nd_info.udp_fd <= 0) {
+		zevpn_arp_nd_info.udp_fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
 
-		if (IS_ZEBRA_DEBUG_EVPN_MH_ARP_ND_EVT)
-			zlog_debug("Create UDP sock for arp_nd redirect from %pI4",
-				   &zmh_info->es_originator_ip);
 		if (zevpn_arp_nd_info.udp_fd <= 0) {
-			zevpn_arp_nd_info.udp_fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
-
-			if (zevpn_arp_nd_info.udp_fd <= 0) {
-				flog_err(EC_LIB_SOCKET,
-					 "evpn arp_nd UDP sock socket(): fd %d errno %s",
-					 zevpn_arp_nd_info.udp_fd, safe_strerror(errno));
-				return;
-			}
-			if (setsockopt(zevpn_arp_nd_info.udp_fd, SOL_SOCKET, SO_REUSEADDR,
-				       (void *)&reuse, sizeof(reuse)))
-				flog_err(EC_LIB_SOCKET,
-					 "evpn arp_nd UDP sock SO_REUSEADDR set: fd %d errno %s",
-					 zevpn_arp_nd_info.udp_fd, safe_strerror(errno));
+			flog_err(EC_LIB_SOCKET, "evpn arp_nd UDP sock socket(): fd %d errno %s",
+				 zevpn_arp_nd_info.udp_fd, safe_strerror(errno));
+			goto close_sock;
 		}
-
-		memset(&sin, 0, sizeof(sin));
-		sin.sin_family = AF_INET;
-		sin.sin_addr = zmh_info->es_originator_ip;
-		if (bind(zevpn_arp_nd_info.udp_fd, (struct sockaddr *)&sin,
-			 sizeof(sin)) < 0) {
+		if (setsockopt(zevpn_arp_nd_info.udp_fd, SOL_SOCKET, SO_REUSEADDR, (void *)&reuse,
+			       sizeof(reuse)))
 			flog_err(EC_LIB_SOCKET,
-				 "evpn arp_nd UDP sock fd %d bind to %pI4 errno %s",
-				 zevpn_arp_nd_info.udp_fd,
-				 &zmh_info->es_originator_ip,
-				 safe_strerror(errno));
-			close(zevpn_arp_nd_info.udp_fd);
-			zevpn_arp_nd_info.udp_fd = -1;
-		}
-	} else {
-		if (zevpn_arp_nd_info.udp_fd > 0) {
+				 "evpn arp_nd UDP sock SO_REUSEADDR set: fd %d errno %s",
+				 zevpn_arp_nd_info.udp_fd, safe_strerror(errno));
+	}
+
+	memset(&sin, 0, sizeof(sin));
+	sin.sin_family = AF_INET;
+	sin.sin_addr = zmh_info->es_originator_ip;
+	if (bind(zevpn_arp_nd_info.udp_fd, (struct sockaddr *)&sin, sizeof(sin)) < 0) {
+		flog_err(EC_LIB_SOCKET, "evpn arp_nd UDP sock fd %d bind to %pI4 errno %s",
+			 zevpn_arp_nd_info.udp_fd, &zmh_info->es_originator_ip,
+			 safe_strerror(errno));
+		close(zevpn_arp_nd_info.udp_fd);
+		zevpn_arp_nd_info.udp_fd = -1;
+	}
+
+	if (zevpn_arp_nd_info.udp_fd > 0)
+		zevpn_arp_nd_info.flags |= ZEBRA_EVPN_ARP_ND_FAILOVER;
+	return;
+
+close_sock:
+	zevpn_arp_nd_info.flags &= ~ZEBRA_EVPN_ARP_ND_FAILOVER;
+	if (zevpn_arp_nd_info.udp_fd > 0) {
+		if (IS_ZEBRA_DEBUG_EVPN_MH_ARP_ND_EVT)
 			zlog_debug("Close arp_nd redirect UDP socket");
-			close(zevpn_arp_nd_info.udp_fd);
-			zevpn_arp_nd_info.udp_fd = -1;
-		}
+		close(zevpn_arp_nd_info.udp_fd);
+		zevpn_arp_nd_info.udp_fd = -1;
 	}
 }
 
@@ -650,6 +667,9 @@ void zebra_evpn_arp_nd_failover_enable(void)
 	if (zmh_info->flags & ZEBRA_EVPN_MH_REDIRECT_OFF)
 		return;
 
+	/* If socket is already open, nothing to do.
+	 * TODO: rebuild sock if it's open and is bound to an
+	 * address that doesn't match the current originator_ip */
 	if (zevpn_arp_nd_info.flags & ZEBRA_EVPN_ARP_ND_FAILOVER)
 		return;
 

diff --git a/zebra/zebra_evpn_arp_nd.h b/zebra/zebra_evpn_arp_nd.h
@@ -42,6 +42,7 @@ struct zebra_evpn_arp_nd_stats {
 /* ARP/ND global information */
 struct zebra_evpn_arp_nd_info {
 	uint32_t flags;
+/* arp-nd-redirect socket is open */
 #define ZEBRA_EVPN_ARP_ND_FAILOVER (1 << 0)
 
 	/*

diff --git a/zebra/zebra_evpn_mh.c b/zebra/zebra_evpn_mh.c
@@ -3851,7 +3851,7 @@ void zebra_evpn_es_set_base_evpn(struct zebra_evpn *zevpn)
 					true /* es_evi_re_reval */);
 	}
 
-	zebra_evpn_arp_nd_udp_sock_create();
+	zebra_evpn_arp_nd_failover_enable();
 }
 
 /* called when a vni is removed or becomes oper down or is removed from a