pespin submitted this change.

View Change

Destroy rkm_dyn_allocated AS automatically when running out of ASPs

This allows merging the freeing of AS inside the ASP/AS FSMs.
It also should fix cases like SG in transport-role client.

Change-Id: I743af584cd77924654d22ccf5d36d4479ba3b7f5
---
M src/ss7_as.c
M src/ss7_asp.c
M src/xua_asp_fsm.c
M src/xua_rkm.c
4 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/src/ss7_as.c b/src/ss7_as.c
index cd6d66a..440385a 100644
--- a/src/ss7_as.c
+++ b/src/ss7_as.c
@@ -193,10 +193,13 @@
 /*! \brief Delete given ASP from given AS
  *  \param[in] as Application Server from which \ref asp is deleted
  *  \param[in] asp Application Server Process to delete from \ref as
- *  \returns 0 on success; negative in case of error */
+ *  \returns 0 on success; negative in case of error
+ *
+ * \ref as may be freed during the function call. */
 int ss7_as_del_asp(struct osmo_ss7_as *as, struct osmo_ss7_asp *asp)
 {
 	unsigned int i;
+	bool found = false;
 
 	LOGPAS(as, DLSS7, LOGL_INFO, "Removing ASP %s from AS\n", asp->cfg.name);
 
@@ -211,11 +214,20 @@
 	for (i = 0; i < ARRAY_SIZE(as->cfg.asps); i++) {
 		if (as->cfg.asps[i] == asp) {
 			as->cfg.asps[i] = NULL;
-			return 0;
+			found = true;
+			break;
 		}
 	}
 
-	return -EINVAL;
+	/* RKM-dynamically allocated AS: If there are no other ASPs, destroy the AS.
+	 * RFC 4666 4.4.2: "If a Deregistration results in no more ASPs in an
+	 * Application Server, an SG MAY delete the Routing Key data."
+	 */
+	if (as->rkm_dyn_allocated && osmo_ss7_as_count_asp(as) == 0)
+		osmo_ss7_as_destroy(as);
+
+
+	return found ? 0 : -EINVAL;
 }
 
 /*! \brief Delete given ASP from given AS
diff --git a/src/ss7_asp.c b/src/ss7_asp.c
index 377d9e4..076d91f 100644
--- a/src/ss7_asp.c
+++ b/src/ss7_asp.c
@@ -745,7 +745,7 @@
 
 void osmo_ss7_asp_destroy(struct osmo_ss7_asp *asp)
 {
-	struct osmo_ss7_as *as;
+	struct osmo_ss7_as *as, *as2;
 
 	OSMO_ASSERT(ss7_initialized);
 	LOGPASP(asp, DLSS7, LOGL_INFO, "Destroying ASP\n");
@@ -757,8 +757,9 @@
 	if (asp->fi)
 		osmo_fsm_inst_term(asp->fi, OSMO_FSM_TERM_REQUEST, NULL);
 
-	/* unlink from all ASs we are part of */
-	llist_for_each_entry(as, &asp->inst->as_list, list)
+	/* Unlink from all ASs we are part of.
+	 * Some RKM-dynamically allocated ASs may be freed as a result from this: */
+	llist_for_each_entry_safe(as, as2, &asp->inst->as_list, list)
 		ss7_as_del_asp(as, asp);
 
 	/* unlink from ss7_instance */
@@ -1351,9 +1352,6 @@
 	/* notify ASP FSM and everyone else */
 	osmo_fsm_inst_dispatch(asp->fi, XUA_ASP_E_SCTP_COMM_DOWN_IND, NULL);
 
-	/* delete any RKM-dynamically allocated ASs for this ASP */
-	xua_rkm_cleanup_dyn_as_for_asp(asp);
-
 	/* send M-SCTP_RELEASE.ind to Layer Manager */
 	xua_asp_send_xlm_prim_simple(asp, OSMO_XLM_PRIM_M_SCTP_RELEASE, PRIM_OP_INDICATION);
 
diff --git a/src/xua_asp_fsm.c b/src/xua_asp_fsm.c
index 17071c3..d10420f 100644
--- a/src/xua_asp_fsm.c
+++ b/src/xua_asp_fsm.c
@@ -514,6 +514,13 @@
 	struct osmo_ss7_asp *asp = xafp->asp;
 	xua_t_beat_stop(fi);
 	dispatch_to_all_as(fi, XUA_ASPAS_ASP_DOWN_IND, asp);
+	/* RFC 4666 4.4.2: "An ASP SHOULD deregister from all Application Servers of which it is a
+	 * member before attempting to move to the ASP-Down state [...]
+	 * If a Deregistration results in no more ASPs in an Application Server, an SG MAY delete
+	 * the Routing Key data."
+	 * In case it didn't deregsitrer explicitly, make sure to implicitly deregister it:
+	 */
+	xua_rkm_cleanup_dyn_as_for_asp(asp);
 }
 
 static void xua_asp_fsm_down(struct osmo_fsm_inst *fi, uint32_t event, void *data)
@@ -1101,6 +1108,13 @@
 	struct osmo_ss7_asp *asp = xafp->asp;
 	ipa_t_beat_stop(fi);
 	dispatch_to_all_as(fi, XUA_ASPAS_ASP_DOWN_IND, asp);
+	/* RFC 4666 4.4.2: "An ASP SHOULD deregister from all Application Servers of which it is a
+	 * member before attempting to move to the ASP-Down state [...]
+	 * If a Deregistration results in no more ASPs in an Application Server, an SG MAY delete
+	 * the Routing Key data."
+	 * In case it didn't deregsitrer explicitly, make sure to implicitly deregister it:
+	 */
+	xua_rkm_cleanup_dyn_as_for_asp(asp);
 }
 
 static void ipa_asp_fsm_down(struct osmo_fsm_inst *fi, uint32_t event, void *data)
diff --git a/src/xua_rkm.c b/src/xua_rkm.c
index d88fda7..2de830f 100644
--- a/src/xua_rkm.c
+++ b/src/xua_rkm.c
@@ -422,15 +422,11 @@
 	LOGPASP(asp, DLSS7, LOGL_INFO, "RKM: De-Registering rctx %u for DPC %s\n",
 		rctx, osmo_ss7_pointcode_print(inst, as->cfg.routing_key.pc));
 
-	/* remove ASP from AS */
-	ss7_as_del_asp(as, asp);
-	/* FIXME: Rather than spoofing teh ASP-DOWN.ind to the AS here,
-	 * we should refuse RKM DEREG if the ASP is still ACTIVE */
-	osmo_fsm_inst_dispatch(as->fi, XUA_ASPAS_ASP_DOWN_IND, asp);
-
-	/* Release the associated route and destroy the dynamically allocated AS */
+	/* Release the associated route */
 	ss7_route_destroy(rt);
-	osmo_ss7_as_destroy(as);
+	/* Dissassociate the ASP from the dynamically allocated AS.
+	 * The AS may be freed if it is serving no more ASPs. */
+	ss7_as_del_asp(as, asp);
 
 	/* report success */
 	msgb_append_dereg_res(resp, M3UA_RKM_DEREG_SUCCESS, rctx);
@@ -635,13 +631,10 @@
 	struct osmo_ss7_as *as, *as2;
 
 	llist_for_each_entry_safe(as, as2, &inst->as_list, list) {
-		if (!osmo_ss7_as_has_asp(as, asp))
-			continue;
 		if (!as->rkm_dyn_allocated)
 			continue;
-
-		/* If there are no other ASPs, destroy the AS: */
-		if (osmo_ss7_as_count_asp(as) == 1)
-			osmo_ss7_as_destroy(as);
+		if (!osmo_ss7_as_has_asp(as, asp))
+			continue;
+		ss7_as_del_asp(as, asp);
 	}
 }

To view, visit change 41512. To unsubscribe, or for help writing mail filters, visit settings.