Merge pull request #18538 from nabahr/autorp-enabling

pimd: Only create and bind the autorp socket when really needed

3 files changed, 90 insertions, 18 deletions

diff --git a/pimd/pim_autorp.c b/pimd/pim_autorp.c
index d3f3517efd..8fb57a29eb 100644
--- a/pimd/pim_autorp.c
+++ b/pimd/pim_autorp.c
@@ -113,10 +113,33 @@ static void pim_autorp_free(struct pim_autorp *autorp)
 		XFREE(MTYPE_PIM_AUTORP_ANNOUNCE, autorp->announce_pkt);
 }
 
+static bool autorp_is_pim_interface(struct interface *ifp)
+{
+	struct pim_interface *pim_ifp = ifp->info;
+
+	return CHECK_FLAG(ifp->status, ZEBRA_INTERFACE_ACTIVE) && pim_ifp && pim_ifp->pim_enable &&
+	       !pim_ifp->pim_passive_enable;
+}
+
+static bool pim_autorp_should_enable_socket(struct pim_autorp *autorp)
+{
+	struct interface *ifp;
+
+	/* Only enable the socket if there are any PIM enabled interfaces */
+	FOR_ALL_INTERFACES (autorp->pim->vrf, ifp) {
+		if (autorp_is_pim_interface(ifp))
+			return true;
+	}
+	return false;
+}
+
 static bool pim_autorp_should_close(struct pim_autorp *autorp)
 {
-	/* If discovery or mapping agent is active, then we need the socket open */
-	return !autorp->do_discovery && !autorp->send_rp_discovery;
+	/* If discovery or mapping agent is active, then we need the socket open. We also want to leave
+	 * the socket open if there are any pim interfaces and we have an announcement packet to send.
+	 */
+	return !autorp->do_discovery && !autorp->send_rp_discovery &&
+	       !(pim_autorp_should_enable_socket(autorp) && autorp->announce_timer != NULL);
 }
 
 static bool pim_autorp_join_groups(struct interface *ifp)
@@ -684,8 +707,14 @@ static void autorp_send_discovery_on(struct pim_autorp *autorp)
 	int interval = 5;
 
 	/* Make sure the socket is open and ready */
-	if (!pim_autorp_socket_enable(autorp)) {
-		zlog_err("%s: AutoRP failed to open socket", __func__);
+	if (pim_autorp_should_enable_socket(autorp)) {
+		if (!pim_autorp_socket_enable(autorp)) {
+			zlog_err("%s: AutoRP failed to open socket", __func__);
+			return;
+		}
+	} else {
+		if (PIM_DEBUG_AUTORP)
+			zlog_debug("%s: No PIM interfaces, not enabling socket", __func__);
 		return;
 	}
 
@@ -964,6 +993,7 @@ err:
 	return;
 }
 
+static void pim_autorp_new_announcement(struct pim_instance *pim);
 static bool pim_autorp_socket_enable(struct pim_autorp *autorp)
 {
 	int fd;
@@ -1006,9 +1036,19 @@ static bool pim_autorp_socket_enable(struct pim_autorp *autorp)
 	if (PIM_DEBUG_AUTORP)
 		zlog_debug("%s: AutoRP socket enabled (fd=%u)", __func__, fd);
 
+	if (autorp->do_discovery)
+		autorp_read_on(autorp);
+
+	if (autorp->send_rp_discovery)
+		autorp_send_discovery_on(autorp);
+
+	/* Try to build a new announcement to make sure the send timer is enabled */
+	pim_autorp_new_announcement(autorp->pim);
+
 	return true;
 }
 
+static void autorp_announcement_off(struct pim_autorp *autorp);
 static bool pim_autorp_socket_disable(struct pim_autorp *autorp)
 {
 	/* Return early if socket is already disabled */
@@ -1022,6 +1062,8 @@ static bool pim_autorp_socket_disable(struct pim_autorp *autorp)
 		return false;
 	}
 
+	autorp_send_discovery_off(autorp);
+	autorp_announcement_off(autorp);
 	autorp_read_off(autorp);
 	autorp->sock = -1;
 
@@ -1058,8 +1100,7 @@ static void autorp_send_announcement(struct event *evt)
 			/* Only send on active interfaces with full pim enabled, non-passive
 			 * and have a primary address set.
 			 */
-			if (CHECK_FLAG(ifp->status, ZEBRA_INTERFACE_ACTIVE) && pim_ifp &&
-			    pim_ifp->pim_enable && !pim_ifp->pim_passive_enable &&
+			if (autorp_is_pim_interface(ifp) &&
 			    !pim_addr_is_any(pim_ifp->primary_address)) {
 				if (setsockopt(autorp->sock, IPPROTO_IP, IP_MULTICAST_IF,
 					       &(pim_ifp->primary_address),
@@ -1108,6 +1149,10 @@ static void autorp_announcement_off(struct pim_autorp *autorp)
 		if (PIM_DEBUG_AUTORP)
 			zlog_debug("%s: AutoRP announcement sending disabled", __func__);
 	event_cancel(&(autorp->announce_timer));
+
+	/* Close the socket if we need to */
+	if (pim_autorp_should_close(autorp) && !pim_autorp_socket_disable(autorp))
+		zlog_warn("%s: AutoRP failed to close socket", __func__);
 }
 
 /* Pack the groups of the RP
@@ -1279,8 +1324,20 @@ static void pim_autorp_new_announcement(struct pim_instance *pim)
 	autorp->announce_pkt_sz += sizeof(struct autorp_pkt_hdr);
 
 	/* Only turn on the announcement timer if we have a packet to send */
-	if (autorp->announce_pkt_sz >= MIN_AUTORP_PKT_SZ)
+	if (autorp->announce_pkt_sz >= MIN_AUTORP_PKT_SZ) {
+		/* We are sending an announcement, but discovery could be off, so make sure the socket is open */
+		if (pim_autorp_should_enable_socket(autorp)) {
+			if (!pim_autorp_socket_enable(autorp)) {
+				zlog_err("%s: AutoRP failed to open socket", __func__);
+				return;
+			}
+		} else {
+			if (PIM_DEBUG_AUTORP)
+				zlog_debug("%s: No PIM interfaces, not enabling socket", __func__);
+			return;
+		}
 		autorp_announcement_on(autorp);
+	}
 }
 
 void pim_autorp_prefix_list_update(struct pim_instance *pim, struct prefix_list *plist)
@@ -1453,10 +1510,15 @@ void pim_autorp_add_ifp(struct interface *ifp)
 	struct pim_interface *pim_ifp;
 
 	pim_ifp = ifp->info;
-	if (CHECK_FLAG(ifp->status, ZEBRA_INTERFACE_ACTIVE) && pim_ifp && pim_ifp->pim_enable) {
+	if (autorp_is_pim_interface(ifp)) {
 		pim = pim_ifp->pim;
-		if (pim && pim->autorp &&
-		    (pim->autorp->do_discovery || pim->autorp->send_rp_discovery)) {
+		if (pim && pim->autorp && !pim_autorp_should_close(pim->autorp)) {
+			/* Make sure the socket is open and ready */
+			if (!pim_autorp_socket_enable(pim->autorp)) {
+				zlog_err("%s: AutoRP failed to open socket", __func__);
+				return;
+			}
+
 			if (PIM_DEBUG_AUTORP)
 				zlog_debug("%s: Adding interface %s to AutoRP, joining AutoRP groups",
 					   __func__, ifp->name);
@@ -1475,11 +1537,13 @@ void pim_autorp_rm_ifp(struct interface *ifp)
 	 */
 	struct pim_instance *pim;
 	struct pim_interface *pim_ifp;
+	struct pim_autorp *autorp = NULL;
 
 	pim_ifp = ifp->info;
 	if (CHECK_FLAG(ifp->status, ZEBRA_INTERFACE_ACTIVE) && pim_ifp) {
 		pim = pim_ifp->pim;
 		if (pim && pim->autorp) {
+			autorp = pim->autorp;
 			if (PIM_DEBUG_AUTORP)
 				zlog_debug("%s: Removing interface %s from AutoRP, leaving AutoRP groups",
 					   __func__, ifp->name);
@@ -1488,6 +1552,11 @@ void pim_autorp_rm_ifp(struct interface *ifp)
 					  safe_strerror(errno));
 		}
 	}
+
+	if (autorp != NULL && !pim_autorp_should_enable_socket(autorp)) {
+		/* Removed the last pim enabled interface, close the socket */
+		pim_autorp_socket_disable(autorp);
+	}
 }
 
 void pim_autorp_start_discovery(struct pim_instance *pim)
@@ -1500,8 +1569,14 @@ void pim_autorp_start_discovery(struct pim_instance *pim)
 	autorp->do_discovery = true;
 
 	/* Make sure the socket is open and ready */
-	if (!pim_autorp_socket_enable(autorp)) {
-		zlog_err("%s: AutoRP failed to open socket", __func__);
+	if (pim_autorp_should_enable_socket(autorp)) {
+		if (!pim_autorp_socket_enable(autorp)) {
+			zlog_err("%s: AutoRP failed to open socket", __func__);
+			return;
+		}
+	} else {
+		if (PIM_DEBUG_AUTORP)
+			zlog_debug("%s: No PIM interfaces, not enabling socket", __func__);
 		return;
 	}
 
diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c
index 86dc826378..510ca398ff 100644
--- a/pimd/pim_iface.c
+++ b/pimd/pim_iface.c
@@ -2025,12 +2025,12 @@ void pim_pim_interface_delete(struct interface *ifp)
 	if (!pim_ifp)
 		return;
 
+	pim_ifp->pim_enable = false;
+
 #if PIM_IPV == 4
 	pim_autorp_rm_ifp(ifp);
 #endif
 
-	pim_ifp->pim_enable = false;
-
 	pim_if_membership_clear(ifp);
 
 	/*
diff --git a/pimd/pim_nb_config.c b/pimd/pim_nb_config.c
index 1be5e9cb88..65fcd6ec51 100644
--- a/pimd/pim_nb_config.c
+++ b/pimd/pim_nb_config.c
@@ -3285,7 +3285,6 @@ int routing_control_plane_protocols_control_plane_protocol_pim_address_family_rp
 {
 	struct vrf *vrf;
 	struct pim_instance *pim;
-	bool enabled;
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
@@ -3295,10 +3294,8 @@ int routing_control_plane_protocols_control_plane_protocol_pim_address_family_rp
 	case NB_EV_APPLY:
 		vrf = nb_running_get_entry(args->dnode, NULL, true);
 		pim = vrf->info;
-		enabled = yang_dnode_get_bool(args->dnode, NULL);
 		/* Run AutoRP discovery by default */
-		if (!enabled)
-			pim_autorp_start_discovery(pim);
+		pim_autorp_start_discovery(pim);
 		break;
 	}