pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/41512?usp=email )

Change subject: Destroy rkm_dyn_allocated AS automatically when running out of ASPs ......................................................................

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(-)

Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved osmith: Looks good to me, but someone else must approve

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); } }