pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/42054?usp=email )
Change subject: Refactor xua_layer_manager object ......................................................................
Refactor xua_layer_manager object
Several changes to improve separation of the LM object and its lifecycle: * Store in ss7_asp a pointer to a xua_layer_manager, which contains all implementation dependent information inside its priv pointer. * Add a new free_func field to the xua_layer_manager struct in order to be able to free it without knowing how it was allocated. * Get rid og the loglevel param, LOGL_DEBUG was always passed and that was actually an implementation specific detail of the default xua layer manager. * get rid of struct lm_fsm_priv, it's not needed since it acutally only contain an asp pointer which can be passed directly as fi->priv.
Change-Id: Ia96ebf40444f46ad718d61befbecb523f267fd6c --- M include/osmocom/sigtran/osmo_ss7.h M src/ss7_asp.c M src/ss7_asp.h M src/xua_default_lm_fsm.c 4 files changed, 61 insertions(+), 66 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/54/42054/1
diff --git a/include/osmocom/sigtran/osmo_ss7.h b/include/osmocom/sigtran/osmo_ss7.h index 9ea929f..162994e 100644 --- a/include/osmocom/sigtran/osmo_ss7.h +++ b/include/osmocom/sigtran/osmo_ss7.h @@ -295,7 +295,6 @@ void osmo_ss7_asp_destroy(struct osmo_ss7_asp *asp); int osmo_ss7_asp_send(struct osmo_ss7_asp *asp, struct msgb *msg); int osmo_ss7_asp_restart(struct osmo_ss7_asp *asp); -int osmo_ss7_asp_use_default_lm(struct osmo_ss7_asp *asp, int log_level); bool osmo_ss7_asp_active(const struct osmo_ss7_asp *asp); int osmo_ss7_asp_get_log_subsys(const struct osmo_ss7_asp *asp); const char *osmo_ss7_asp_get_name(const struct osmo_ss7_asp *asp); diff --git a/src/ss7_asp.c b/src/ss7_asp.c index ded2c67..6071317 100644 --- a/src/ss7_asp.c +++ b/src/ss7_asp.c @@ -758,7 +758,11 @@ osmo_stream_cli_destroy(asp->client); if (asp->fi) osmo_fsm_inst_term(asp->fi, OSMO_FSM_TERM_REQUEST, NULL); - osmo_ss7_asp_remove_default_lm(asp); + if (asp->lm) { + if (asp->lm->free_func) + asp->lm->free_func(asp->lm); + asp->lm = NULL; + }
/* Unlink from all ASs we are part of. * Some RKM-dynamically allocated ASs may be freed as a result from this: */ @@ -907,14 +911,18 @@ OSMO_ASSERT(!asp->fi); }
+ /* Remove existing LayerManager: */ + if (asp->lm) { + if (asp->lm->free_func) + asp->lm->free_func(asp->lm); + asp->lm = NULL; + } + /* Apply default LM FSM for client ASP */ if (asp->cfg.proto != OSMO_SS7_ASP_PROT_IPA && asp->cfg.role == OSMO_SS7_ASP_ROLE_ASP && - !asp->cfg.is_server) { - osmo_ss7_asp_use_default_lm(asp, LOGL_DEBUG); - } else { - osmo_ss7_asp_remove_default_lm(asp); - } + !asp->cfg.is_server) + asp->lm = xua_layer_manager_default_alloc(asp);
if ((rc = xua_asp_fsm_start(asp, LOGL_DEBUG)) < 0) return rc; diff --git a/src/ss7_asp.h b/src/ss7_asp.h index 0e156f2..886d6c8 100644 --- a/src/ss7_asp.h +++ b/src/ss7_asp.h @@ -25,7 +25,10 @@ * cb_data: (struct osmo_ss7_asp *) */ osmo_prim_cb prim_cb; + void (*free_func)(struct osmo_xua_layer_manager *lm); + void *priv; }; +struct osmo_xua_layer_manager *xua_layer_manager_default_alloc(struct osmo_ss7_asp *asp);
enum ss7_asp_xua_timer { /* 0 kept unused on purpose since it's handled specially by osmo_fsm */ @@ -77,8 +80,7 @@ bool remote_asp_id_present;
/* Layer Manager to which we talk */ - const struct osmo_xua_layer_manager *lm; - void *lm_priv; + struct osmo_xua_layer_manager *lm;
/*! Were we dynamically allocated */ bool dyn_allocated; @@ -180,7 +182,6 @@ int ss7_asp_apply_drop_local_address(const struct osmo_ss7_asp *asp, unsigned int loc_idx);
void ss7_asp_restart_after_reconfigure(struct osmo_ss7_asp *asp); -void osmo_ss7_asp_remove_default_lm(struct osmo_ss7_asp *asp);
unsigned int ss7_asp_get_all_rctx(const struct osmo_ss7_asp *asp, uint32_t *rctx, unsigned int rctx_size, const struct osmo_ss7_as *excl_as); diff --git a/src/xua_default_lm_fsm.c b/src/xua_default_lm_fsm.c index 92a6ea2..bd6f148 100644 --- a/src/xua_default_lm_fsm.c +++ b/src/xua_default_lm_fsm.c @@ -116,14 +116,10 @@ [S_ACTIVE] = { }, };
-struct lm_fsm_priv { - struct osmo_ss7_asp *asp; -}; - #define lm_fsm_state_chg(fi, NEXT_STATE) \ osmo_tdef_fsm_inst_state_chg(fi, NEXT_STATE, \ lm_fsm_timeouts, \ - ((struct lm_fsm_priv *)(fi->priv))->asp->cfg.T_defs_lm, \ + ((struct osmo_ss7_asp *)(fi->priv))->cfg.T_defs_lm, \ -1)
static struct osmo_ss7_as *find_first_as_in_asp(struct osmo_ss7_asp *asp) @@ -144,8 +140,7 @@ /* handle an incoming RKM registration response */ static int handle_reg_conf(struct osmo_fsm_inst *fi, uint32_t l_rk_id, uint32_t rctx) { - struct lm_fsm_priv *lmp = fi->priv; - struct osmo_ss7_asp *asp = lmp->asp; + struct osmo_ss7_asp *asp = fi->priv; struct osmo_ss7_as *as;
/* update the application server with the routing context as @@ -162,13 +157,13 @@
static void lm_idle(struct osmo_fsm_inst *fi, uint32_t event, void *data) { - struct lm_fsm_priv *lmp = fi->priv; + struct osmo_ss7_asp *asp = fi->priv;
switch (event) { case LM_E_SCTP_EST_IND: /* Try to transition to ASP-UP, wait to receive message for a few seconds */ lm_fsm_state_chg(fi, S_WAIT_ASP_UP); - osmo_fsm_inst_dispatch(lmp->asp->fi, XUA_ASP_E_M_ASP_UP_REQ, NULL); + osmo_fsm_inst_dispatch(asp->fi, XUA_ASP_E_M_ASP_UP_REQ, NULL); break; } } @@ -187,7 +182,7 @@
static int lm_timer_cb(struct osmo_fsm_inst *fi) { - struct lm_fsm_priv *lmp = fi->priv; + struct osmo_ss7_asp *asp = fi->priv; struct osmo_xlm_prim *prim; struct osmo_ss7_as *as;
@@ -196,10 +191,10 @@ /* we have been waiting for the ASP to come up, but it * failed to do so */ LOGPFSML(fi, LOGL_NOTICE, "Peer didn't send any ASP_UP in time! Restarting ASP\n"); - ss7_asp_disconnect_stream(lmp->asp); + ss7_asp_disconnect_stream(asp); break; case SS7_ASP_LM_T_WAIT_NOTIFY: - if (lmp->asp->cfg.quirks & OSMO_SS7_ASP_QUIRK_NO_NOTIFY) { + if (asp->cfg.quirks & OSMO_SS7_ASP_QUIRK_NO_NOTIFY) { /* some implementations don't send the NOTIFY which they SHOULD * according to RFC4666 (see OS#5145) */ LOGPFSM(fi, "quirk no_notify active; locally emulate AS-INACTIVE.ind\n"); @@ -212,25 +207,25 @@ lm_fsm_state_chg(fi, S_RKM_REG); prim = xua_xlm_prim_alloc(OSMO_XLM_PRIM_M_RK_REG, PRIM_OP_REQUEST); OSMO_ASSERT(prim); - as = find_first_as_in_asp(lmp->asp); + as = find_first_as_in_asp(asp); if (!as) { LOGPFSML(fi, LOGL_ERROR, "Unable to find AS!\n"); - ss7_asp_disconnect_stream(lmp->asp); + ss7_asp_disconnect_stream(asp); return 0; } /* Fill in settings from first AS (TODO: multiple AS support) */ prim->u.rk_reg.key = as->cfg.routing_key; prim->u.rk_reg.traf_mode = as->cfg.mode; - osmo_xlm_sap_down(lmp->asp, &prim->oph); + osmo_xlm_sap_down(asp, &prim->oph); break; case SS7_ASP_LM_T_WAIT_NOTIY_RKM: /* No AS has reported via NOTIFY even after dynamic RKM * configuration */ - ss7_asp_disconnect_stream(lmp->asp); + ss7_asp_disconnect_stream(asp); break; case SS7_ASP_LM_T_WAIT_RK_REG_RESP: /* timeout of registration of routing key */ - ss7_asp_disconnect_stream(lmp->asp); + ss7_asp_disconnect_stream(asp); break; } return 0; @@ -238,7 +233,7 @@
static void lm_wait_notify(struct osmo_fsm_inst *fi, uint32_t event, void *data) { - struct lm_fsm_priv *lmp = fi->priv; + struct osmo_ss7_asp *asp = fi->priv; struct osmo_xlm_prim *oxp = data;
switch (event) { @@ -251,12 +246,12 @@ break;
/* Don't change active ASP if there's already one active. */ - if (ss7_asp_determine_traf_mode(lmp->asp) == OSMO_SS7_AS_TMOD_OVERRIDE && + if (ss7_asp_determine_traf_mode(asp) == OSMO_SS7_AS_TMOD_OVERRIDE && oxp->u.notify.status_info == M3UA_NOTIFY_I_AS_ACT) break;
lm_fsm_state_chg(fi, S_ACTIVE); - osmo_fsm_inst_dispatch(lmp->asp->fi, XUA_ASP_E_M_ASP_ACTIVE_REQ, NULL); + osmo_fsm_inst_dispatch(asp->fi, XUA_ASP_E_M_ASP_ACTIVE_REQ, NULL); break; case LM_E_AS_INACTIVE_IND: /* we now know that an AS is associated with this ASP at @@ -264,14 +259,14 @@ /* request the ASP to go into active state (which * hopefully will bring the AS to active, too) */ lm_fsm_state_chg(fi, S_ACTIVE); - osmo_fsm_inst_dispatch(lmp->asp->fi, XUA_ASP_E_M_ASP_ACTIVE_REQ, NULL); + osmo_fsm_inst_dispatch(asp->fi, XUA_ASP_E_M_ASP_ACTIVE_REQ, NULL); break; } };
static void lm_rkm_reg(struct osmo_fsm_inst *fi, uint32_t event, void *data) { - struct lm_fsm_priv *lmp = fi->priv; + struct osmo_ss7_asp *asp = fi->priv; struct osmo_xlm_prim *oxp; int rc;
@@ -280,16 +275,16 @@ oxp = data; if (oxp->u.rk_reg.status != M3UA_RKM_REG_SUCCESS) { LOGPFSML(fi, LOGL_NOTICE, "Received RKM_REG_RSP with negative result\n"); - ss7_asp_disconnect_stream(lmp->asp); + ss7_asp_disconnect_stream(asp); } else { unsigned long timeout_sec; rc = handle_reg_conf(fi, oxp->u.rk_reg.key.l_rk_id, oxp->u.rk_reg.key.context); if (rc < 0) - ss7_asp_disconnect_stream(lmp->asp); + ss7_asp_disconnect_stream(asp); /* RKM registration was successful, we can transition to WAIT_NOTIFY * state and assume that an NOTIFY/AS-INACTIVE arrives within * T_WAIT_NOTIFY_RKM seconds */ - timeout_sec = osmo_tdef_get(lmp->asp->cfg.T_defs_lm, SS7_ASP_LM_T_WAIT_NOTIY_RKM, OSMO_TDEF_S, -1); + timeout_sec = osmo_tdef_get(asp->cfg.T_defs_lm, SS7_ASP_LM_T_WAIT_NOTIY_RKM, OSMO_TDEF_S, -1); osmo_fsm_inst_state_chg(fi, S_WAIT_NOTIFY, timeout_sec, SS7_ASP_LM_T_WAIT_NOTIY_RKM); } break; @@ -298,13 +293,13 @@
static void lm_active(struct osmo_fsm_inst *fi, uint32_t event, void *data) { - struct lm_fsm_priv *lmp = fi->priv; + struct osmo_ss7_asp *asp = fi->priv; struct osmo_xlm_prim *oxp;
switch (event) { case LM_E_AS_INACTIVE_IND: /* request the ASP to go into active state */ - osmo_fsm_inst_dispatch(lmp->asp->fi, XUA_ASP_E_M_ASP_ACTIVE_REQ, NULL); + osmo_fsm_inst_dispatch(asp->fi, XUA_ASP_E_M_ASP_ACTIVE_REQ, NULL); break; case LM_E_NOTIFY_IND: oxp = data; @@ -396,7 +391,7 @@ static int default_lm_prim_cb(struct osmo_prim_hdr *oph, void *_asp) { struct osmo_ss7_asp *asp = _asp; - struct osmo_fsm_inst *fi = asp->lm_priv; + struct osmo_fsm_inst *fi = asp->lm->priv; uint32_t event = osmo_event_for_prim(oph, lm_event_map); char *prim_name = osmo_xlm_prim_name(oph);
@@ -412,42 +407,34 @@ return 0; }
-static const struct osmo_xua_layer_manager default_layer_manager = { - .prim_cb = default_lm_prim_cb, -}; - -void osmo_ss7_asp_remove_default_lm(struct osmo_ss7_asp *asp) +void xua_layer_manager_default_free(struct osmo_xua_layer_manager *lm) { - if (!asp->lm_priv) + if (!lm) return; - osmo_fsm_inst_term(asp->lm_priv, OSMO_FSM_TERM_ERROR, NULL); - asp->lm_priv = NULL; + if (lm->priv) { + osmo_fsm_inst_term(lm->priv, OSMO_FSM_TERM_ERROR, NULL); + lm->priv = NULL; + } + talloc_free(lm); }
-int osmo_ss7_asp_use_default_lm(struct osmo_ss7_asp *asp, int log_level) +struct osmo_xua_layer_manager *xua_layer_manager_default_alloc(struct osmo_ss7_asp *asp) { - struct lm_fsm_priv *lmp; struct osmo_fsm_inst *fi; + struct osmo_xua_layer_manager *lm;
- if (asp->lm_priv) { - osmo_fsm_inst_term(asp->lm_priv, OSMO_FSM_TERM_ERROR, NULL); - asp->lm_priv = NULL; + + lm = talloc_zero(asp, struct osmo_xua_layer_manager); + OSMO_ASSERT(lm); + + fi = osmo_fsm_inst_alloc(&xua_default_lm_fsm, lm, asp, LOGL_DEBUG, asp->cfg.name); + if (!fi) { + talloc_free(lm); + return NULL; } - - fi = osmo_fsm_inst_alloc(&xua_default_lm_fsm, asp, NULL, log_level, asp->cfg.name); - if (!fi) - return -EINVAL; - - lmp = talloc_zero(fi, struct lm_fsm_priv); - if (!lmp) { - osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL); - return -ENOMEM; - } - lmp->asp = asp; - fi->priv = lmp; - - asp->lm = &default_layer_manager; - asp->lm_priv = fi; + lm->prim_cb = default_lm_prim_cb; + lm->free_func = xua_layer_manager_default_free; + lm->priv = fi;
return 0; }