[Asterisk-code-review] res_pjsip: Add support for partial transport reload. (asterisk[16])

[Joshua Colp]

Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15634 )

Change subject: res_pjsip: Add support for partial transport reload.
......................................................................

res_pjsip: Add support for partial transport reload.

Some configuration items for a transport do not result in
the underlying transport changing, but instead are just
state we keep ourselves and use. It is perfectly reasonable
to change these items.

These include local_net and external_* information.

ASTERISK-29354

Change-Id: I027857ccfe4419f460243e562b5f098434b3d43a
---
A doc/CHANGES-staging/pjsip_transport_partial_reload.txt
M res/res_pjsip/config_transport.c
2 files changed, 24 insertions(+), 19 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, approved
  George Joseph: Looks good to me, but someone else must approve
  Joshua Colp: Approved for Submit



diff --git a/doc/CHANGES-staging/pjsip_transport_partial_reload.txt b/doc/CHANGES-staging/pjsip_transport_partial_reload.txt
new file mode 100644
index 0000000..1d1b0b6
--- /dev/null
+++ b/doc/CHANGES-staging/pjsip_transport_partial_reload.txt
@@ -0,0 +1,4 @@
+Subject: res_pjsip
+
+PJSIP transports can now be partially reloaded safely. This allows the
+local_net and external_* options to be updated without restarting Asterisk.
diff --git a/res/res_pjsip/config_transport.c b/res/res_pjsip/config_transport.c
index f3a8d97..c1001f4 100644
--- a/res/res_pjsip/config_transport.c
+++ b/res/res_pjsip/config_transport.c
@@ -499,28 +499,24 @@
 			ao2_replace(perm_state->transport, transport);
 			return 0;
 		}
-
+		/* If we aren't allowed to reload then we copy values that can't be changed from perm_state */
 		if (!transport->allow_reload) {
-			if (!perm_state->change_detected) {
-				perm_state->change_detected = 1;
-				ast_log(LOG_WARNING, "Transport '%s' is not reloadable, maintaining previous values\n", transport_id);
-			}
-			/* In case someone is using the deprecated fields, reset them */
-			transport->state = perm_state->state;
-			copy_state_to_transport(transport);
-			ao2_replace(perm_state->transport, transport);
-			return 0;
+			memcpy(&temp_state->state->host, &perm_state->state->host, sizeof(temp_state->state->host));
+			memcpy(&temp_state->state->tls, &perm_state->state->tls, sizeof(temp_state->state->tls));
+			memcpy(&temp_state->state->ciphers, &perm_state->state->ciphers, sizeof(temp_state->state->ciphers));
 		}
 	}
 
-	if (temp_state->state->host.addr.sa_family != PJ_AF_INET && temp_state->state->host.addr.sa_family != PJ_AF_INET6) {
-		ast_log(LOG_ERROR, "Transport '%s' could not be started as binding not specified\n", transport_id);
-		return -1;
-	}
+	if (!perm_state || transport->allow_reload) {
+		if (temp_state->state->host.addr.sa_family != PJ_AF_INET && temp_state->state->host.addr.sa_family != PJ_AF_INET6) {
+			ast_log(LOG_ERROR, "Transport '%s' could not be started as binding not specified\n", transport_id);
+			return -1;
+		}
 
-	/* Set default port if not present */
-	if (!pj_sockaddr_get_port(&temp_state->state->host)) {
-		pj_sockaddr_set_port(&temp_state->state->host, (transport->type == AST_TRANSPORT_TLS) ? 5061 : 5060);
+		/* Set default port if not present */
+		if (!pj_sockaddr_get_port(&temp_state->state->host)) {
+			pj_sockaddr_set_port(&temp_state->state->host, (transport->type == AST_TRANSPORT_TLS) ? 5061 : 5060);
+		}
 	}
 
 	/* Now that we know what address family we can set up a dnsmgr refresh for the external addresses if present */
@@ -558,8 +554,13 @@
 		}
 	}
 
-	if (transport->type == AST_TRANSPORT_UDP) {
-
+	if (!transport->allow_reload && perm_state) {
+		/* We inherit the transport from perm state, untouched */
+		ast_log(LOG_WARNING, "Transport '%s' is not fully reloadable, not reloading: protocol, bind, TLS, TCP, ToS, or CoS options.\n", transport_id);
+		temp_state->state->transport = perm_state->state->transport;
+		perm_state->state->transport = NULL;
+		res = PJ_SUCCESS;
+	} else if (transport->type == AST_TRANSPORT_UDP) {
 		for (i = 0; i < BIND_TRIES && res != PJ_SUCCESS; i++) {
 			if (perm_state && perm_state->state && perm_state->state->transport) {
 				pjsip_udp_transport_pause(perm_state->state->transport,

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15634
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I027857ccfe4419f460243e562b5f098434b3d43a
Gerrit-Change-Number: 15634
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210322/99bd83d8/attachment.html>