Merge pull request #17934 from nabahr/autorp-close

pimd: Close AutoRP socket when not needed

2 files changed, 41 insertions, 22 deletions

diff --git a/pimd/pim_autorp.c b/pimd/pim_autorp.c
index caed914a87..dc077dbbd6 100644
--- a/pimd/pim_autorp.c
+++ b/pimd/pim_autorp.c
@@ -113,6 +113,12 @@ static void pim_autorp_free(struct pim_autorp *autorp)
 		XFREE(MTYPE_PIM_AUTORP_ANNOUNCE, autorp->announce_pkt);
 }
 
+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;
+}
+
 static bool pim_autorp_join_groups(struct interface *ifp)
 {
 	struct pim_interface *pim_ifp;
@@ -670,10 +676,19 @@ static void autorp_send_discovery(struct event *evt)
 			&(autorp->send_discovery_timer));
 }
 
+static bool pim_autorp_socket_enable(struct pim_autorp *autorp);
+static bool pim_autorp_socket_disable(struct pim_autorp *autorp);
+
 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__);
+		return;
+	}
+
 	/* Send the first discovery shortly after being enabled.
 	 * If the configured interval is less than 5 seconds, then just use that.
 	 */
@@ -695,6 +710,10 @@ static void autorp_send_discovery_off(struct pim_autorp *autorp)
 		if (PIM_DEBUG_AUTORP)
 			zlog_debug("%s: AutoRP discovery sending disabled", __func__);
 	event_cancel(&(autorp->send_discovery_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__);
 }
 
 static bool autorp_recv_discovery(struct pim_autorp *autorp, uint8_t rpcnt, uint16_t holdtime,
@@ -949,6 +968,10 @@ static bool pim_autorp_socket_enable(struct pim_autorp *autorp)
 {
 	int fd;
 
+	/* Return early if socket is already enabled */
+	if (autorp->sock != -1)
+		return true;
+
 	frr_with_privs (&pimd_privs) {
 		fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_IP);
 		if (fd < 0) {
@@ -975,6 +998,10 @@ static bool pim_autorp_socket_enable(struct pim_autorp *autorp)
 
 static bool pim_autorp_socket_disable(struct pim_autorp *autorp)
 {
+	/* Return early if socket is already disabled */
+	if (autorp->sock == -1)
+		return true;
+
 	if (close(autorp->sock)) {
 		zlog_warn("Failure closing autorp socket: fd=%d errno=%d: %s", autorp->sock, errno,
 			  safe_strerror(errno));
@@ -1453,6 +1480,12 @@ void pim_autorp_start_discovery(struct pim_instance *pim)
 	struct interface *ifp;
 	struct pim_autorp *autorp = pim->autorp;
 
+	/* Make sure the socket is open and ready */
+	if (!pim_autorp_socket_enable(autorp)) {
+		zlog_err("%s: AutoRP failed to open socket", __func__);
+		return;
+	}
+
 	if (!autorp->do_discovery) {
 		autorp->do_discovery = true;
 		autorp_read_on(autorp);
@@ -1482,6 +1515,10 @@ void pim_autorp_stop_discovery(struct pim_instance *pim)
 		if (PIM_DEBUG_AUTORP)
 			zlog_debug("%s: AutoRP Discovery stopped", __func__);
 	}
+
+	/* 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__);
 }
 
 void pim_autorp_init(struct pim_instance *pim)
@@ -1510,12 +1547,6 @@ void pim_autorp_init(struct pim_instance *pim)
 
 	pim->autorp = autorp;
 
-	if (!pim_autorp_socket_enable(autorp)) {
-		zlog_warn("%s: AutoRP failed to initialize, feature will not work correctly",
-			  __func__);
-		return;
-	}
-
 	if (PIM_DEBUG_AUTORP)
 		zlog_debug("%s: AutoRP Initialized", __func__);
 
diff --git a/pimd/pim_cmd_common.c b/pimd/pim_cmd_common.c
index 8aebce7d27..9f09852a94 100644
--- a/pimd/pim_cmd_common.c
+++ b/pimd/pim_cmd_common.c
@@ -608,26 +608,14 @@ int pim_process_no_rp_plist_cmd(struct vty *vty, const char *rp_str,
 
 int pim_process_autorp_cmd(struct vty *vty)
 {
-	char xpath[XPATH_MAXLEN];
-
-	snprintf(xpath, sizeof(xpath), "%s/%s", FRR_PIM_AUTORP_XPATH,
-		 "discovery-enabled");
-
-	nb_cli_enqueue_change(vty, xpath, NB_OP_MODIFY, "true");
-
-	return nb_cli_apply_changes(vty, NULL);
+	nb_cli_enqueue_change(vty, "./discovery-enabled", NB_OP_MODIFY, "true");
+	return nb_cli_apply_changes(vty, "%s", FRR_PIM_AUTORP_XPATH);
 }
 
 int pim_process_no_autorp_cmd(struct vty *vty)
 {
-	char xpath[XPATH_MAXLEN];
-
-	snprintf(xpath, sizeof(xpath), "%s/%s", FRR_PIM_AUTORP_XPATH,
-		 "discovery-enabled");
-
-	nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL);
-
-	return nb_cli_apply_changes(vty, NULL);
+	nb_cli_enqueue_change(vty, "./discovery-enabled", NB_OP_MODIFY, "false");
+	return nb_cli_apply_changes(vty, "%s", FRR_PIM_AUTORP_XPATH);
 }
 
 int pim_process_autorp_candidate_rp_cmd(struct vty *vty, bool no, const char *rpaddr_str,