pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/42111?usp=email )
(
3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: asp: Introduce asp->assoc_as_list ......................................................................
asp: Introduce asp->assoc_as_list
This way we can iterate easily/quickly the AS served by a given ASP. This in turn allows simplifying some code since we cache the amount of AS served by the ASP at any time, so we save some counting.
Related: SYS#6953 Change-Id: I1ca90748286374109dbb4277cbc7b2337ce2072a --- M src/ipa.c M src/sccp_instance.c M src/ss7_as.c M src/ss7_as.h M src/ss7_asp.c M src/ss7_asp.h M src/ss7_asp_vty.c M src/tcap_as_loadshare.c M src/xua_asp_fsm.c M src/xua_default_lm_fsm.c M src/xua_internal.h M src/xua_shared.c M tests/vty/osmo_stp_test.vty M tests/vty/ss7_asp_test.vty 14 files changed, 77 insertions(+), 124 deletions(-)
Approvals: pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve
diff --git a/src/ipa.c b/src/ipa.c index 6cf7104..29dc5c0 100644 --- a/src/ipa.c +++ b/src/ipa.c @@ -169,22 +169,6 @@ return rc; }
-struct osmo_ss7_as *ipa_find_as_for_asp(struct osmo_ss7_asp *asp) -{ - struct osmo_ss7_as *as; - - /* in the IPA case, we assume there is a 1:1 mapping between the - * ASP and the AS. An AS without ASP means there is no - * connection, and an ASP without AS means that we don't (yet?) - * know the identity of the peer */ - - llist_for_each_entry(as, &asp->inst->as_list, list) { - if (osmo_ss7_as_has_asp(as, asp)) - return as; - } - return NULL; -} - /* Patch a SCCP message and add point codes to Called/Calling Party (if missing) */ static struct msgb *patch_sccp_with_pc(const struct osmo_ss7_asp *asp, const struct msgb *sccp_msg_in, uint32_t opc, uint32_t dpc) @@ -254,7 +238,7 @@ enum ipaccess_proto ipa_proto = osmo_ipa_msgb_cb_proto(msg); struct m3ua_data_hdr data_hdr; struct xua_msg *xua = NULL; - struct osmo_ss7_as *as = ipa_find_as_for_asp(asp); + struct osmo_ss7_as *as = ss7_asp_get_first_as(asp); uint32_t opc, dpc;
if (!as) { diff --git a/src/sccp_instance.c b/src/sccp_instance.c index 6388b4b..d6937d6 100644 --- a/src/sccp_instance.c +++ b/src/sccp_instance.c @@ -562,18 +562,6 @@ * Convenience function for CLIENT ***********************************************************************/
- /* Returns whether AS is already associated to any AS. - * Helper function for osmo_sccp_simple_client_on_ss7_id(). */ -static bool asp_serves_some_as(const struct osmo_ss7_asp *asp) -{ - struct osmo_ss7_as *as_i; - llist_for_each_entry(as_i, &asp->inst->as_list, list) { - if (osmo_ss7_as_has_asp(as_i, asp)) - return true; - } - return false; -} - /*! \brief request an sccp client instance * \param[in] ctx talloc context * \param[in] ss7_id of the SS7/CS7 instance @@ -695,7 +683,7 @@ llist_for_each_entry(asp_i, &ss7->asp_list, list) { if (asp_i->cfg.proto != prot) continue; - if (asp_serves_some_as(asp_i)) { + if (asp_i->num_assoc_as > 0) { /* This ASP is already on another AS. * If it was on this AS, we'd have found it above. */ continue; diff --git a/src/ss7_as.c b/src/ss7_as.c index 8a5c1a1..e22fc6c 100644 --- a/src/ss7_as.c +++ b/src/ss7_as.c @@ -65,7 +65,9 @@ assoc->as = as; assoc->asp = asp; llist_add_tail(&assoc->as_entry, &as->assoc_asp_list); + llist_add_tail(&assoc->asp_entry, &asp->assoc_as_list); as->num_assoc_asps++; + asp->num_assoc_as++;
return assoc; } @@ -74,6 +76,8 @@ { llist_del(&assoc->as_entry); assoc->as->num_assoc_asps--; + llist_del(&assoc->asp_entry); + assoc->asp->num_assoc_as--; talloc_free(assoc); }
diff --git a/src/ss7_as.h b/src/ss7_as.h index db219e0..2e7f484 100644 --- a/src/ss7_as.h +++ b/src/ss7_as.h @@ -86,6 +86,8 @@ struct ss7_as_asp_assoc { /* Entry in (struct osmo_ss7_as*)->assoc_asp_list */ struct llist_head as_entry; + /* Entry in (struct osmo_ss7_asp*)->assoc_as_list */ + struct llist_head asp_entry; struct osmo_ss7_as *as; /* backpointer */ struct osmo_ss7_asp *asp; /* backpointer */ }; diff --git a/src/ss7_asp.c b/src/ss7_asp.c index 5e96519..9b8f164 100644 --- a/src/ss7_asp.c +++ b/src/ss7_asp.c @@ -742,6 +742,7 @@ } rate_ctr_group_set_name(asp->ctrg, name); asp->inst = inst; + INIT_LLIST_HEAD(&asp->assoc_as_list); /* ASP in "no shutdown" state by default: */ asp->cfg.adm_state = OSMO_SS7_ASP_ADM_S_ENABLED; ss7_asp_peer_init(&asp->cfg.remote); @@ -769,7 +770,7 @@
void osmo_ss7_asp_destroy(struct osmo_ss7_asp *asp) { - struct osmo_ss7_as *as, *as2; + struct ss7_as_asp_assoc *assoc, *assoc2;
OSMO_ASSERT(ss7_initialized); LOGPASP(asp, DLSS7, LOGL_INFO, "Destroying ASP\n"); @@ -784,8 +785,8 @@
/* 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); + llist_for_each_entry_safe(assoc, assoc2, &asp->assoc_as_list, asp_entry) + ss7_as_del_asp(assoc->as, asp);
/* unlink from ss7_instance */ asp->inst = NULL; @@ -1552,13 +1553,12 @@ const struct osmo_ss7_as *excl_as, bool network_byte_order) { unsigned int count = 0; - struct osmo_ss7_as *as; + struct ss7_as_asp_assoc *assoc;
- llist_for_each_entry(as, &asp->inst->as_list, list) { + llist_for_each_entry(assoc, &asp->assoc_as_list, asp_entry) { + struct osmo_ss7_as *as = assoc->as; if (as == excl_as) continue; - if (!osmo_ss7_as_has_asp(as, asp)) - continue; if (as->cfg.routing_key.context == 0) continue; if (count >= rctx_size) @@ -1586,6 +1586,17 @@ return _ss7_asp_get_all_rctx(asp, rctx, rctx_size, excl_as, false); }
+/* Get first AS in the ASP, or NULL if no AS associated. + * This is useful for instance in IPA code, where we assume only up to 1 AS is configured per ASP. */ +struct osmo_ss7_as *ss7_asp_get_first_as(const struct osmo_ss7_asp *asp) +{ + struct ss7_as_asp_assoc *assoc; + assoc = llist_first_entry_or_null(&asp->assoc_as_list, struct ss7_as_asp_assoc, asp_entry); + if (!assoc) + return NULL; + return assoc->as; +} + /* determine the osmo_ss7_as_traffic_mode to be used by this ASP; will * iterate over all AS configured for this ASP. If they're compatible, * a single traffic mode is returned as enum osmo_ss7_as_traffic_mode. @@ -1593,12 +1604,11 @@ * configured, -1 is returned */ int ss7_asp_determine_traf_mode(const struct osmo_ss7_asp *asp) { - const struct osmo_ss7_as *as; + const struct ss7_as_asp_assoc *assoc; int tmode = -1;
- llist_for_each_entry(as, &asp->inst->as_list, list) { - if (!osmo_ss7_as_has_asp(as, asp)) - continue; + llist_for_each_entry(assoc, &asp->assoc_as_list, asp_entry) { + const struct osmo_ss7_as *as = assoc->as; /* we only care about traffic modes explicitly set */ if (!as->cfg.mode_set_by_vty) continue; diff --git a/src/ss7_asp.h b/src/ss7_asp.h index 80a0a8a..0d047cb 100644 --- a/src/ss7_asp.h +++ b/src/ss7_asp.h @@ -55,6 +55,9 @@ struct osmo_ss7_asp { /*! entry in \ref osmo_ss7_instance.asp_list */ struct llist_head list; + struct llist_head assoc_as_list; /* list of struct ss7_as_asp_assoc */ + unsigned int num_assoc_as; /* amount of ss7_as_asp_assoc/ss7_as in as_list */ + struct osmo_ss7_instance *inst;
/*! ASP FSM */ @@ -187,6 +190,7 @@ const struct osmo_ss7_as *excl_as); unsigned int ss7_asp_get_all_rctx_be(const struct osmo_ss7_asp *asp, uint32_t *rctx, unsigned int rctx_size, const struct osmo_ss7_as *excl_as); +struct osmo_ss7_as *ss7_asp_get_first_as(const struct osmo_ss7_asp *asp);
int ss7_asp_determine_traf_mode(const struct osmo_ss7_asp *asp);
diff --git a/src/ss7_asp_vty.c b/src/ss7_asp_vty.c index 6731398..241863a 100644 --- a/src/ss7_asp_vty.c +++ b/src/ss7_asp_vty.c @@ -116,6 +116,24 @@ xua_tx_snm_daud(asp, rctx, num_rctx, &aff_pc, 1, "VTY"); }
+static char *as_list_for_asp(const struct osmo_ss7_asp *asp, char *buf, size_t buf_len) +{ + struct osmo_strbuf sb = { .buf = buf, .len = buf_len }; + const struct ss7_as_asp_assoc *assoc; + unsigned int count = 0; + + if (asp->num_assoc_as == 0) { + OSMO_STRBUF_PRINTF(sb, "?"); + return buf; + } + + llist_for_each_entry(assoc, &asp->assoc_as_list, asp_entry) { + OSMO_STRBUF_PRINTF(sb, "%s%s", count != 0 ? "," : "", assoc->as->cfg.name); + count++; + } + return buf; +} + DEFUN_ATTR(cs7_asp_audit, cs7_asp_audit_cmd, "cs7 instance <0-15> asp NAME audit point-code POINT_CODE", CS7_STR "Instance related commands\n" "SS7 Instance Number\n" @@ -232,7 +250,6 @@ struct osmo_ss7_instance *inst = vty->index; const char *name = argv[0]; struct osmo_ss7_asp *asp; - struct osmo_ss7_as *as;
asp = osmo_ss7_asp_find_by_name(inst, name); if (!asp) { @@ -240,13 +257,12 @@ return CMD_WARNING; }
- llist_for_each_entry(as, &inst->as_list, list) { - if (osmo_ss7_as_has_asp(as, asp)) { - vty_out(vty, "%% ASP '%s' currently configured in AS '%s'. " - "You must first remove the ASP from the AS configuration%s", - name, as->cfg.name, VTY_NEWLINE); - return CMD_WARNING; - } + if (asp->num_assoc_as > 0) { + char as_buf[512]; + vty_out(vty, "%% ASP '%s' currently configured in %u AS: '%s'. " + "You must first remove the ASP from the AS configuration%s", + name, asp->num_assoc_as, as_list_for_asp(asp, as_buf, sizeof(as_buf)), VTY_NEWLINE); + return CMD_WARNING; }
osmo_ss7_asp_destroy(asp); @@ -871,24 +887,6 @@ } }
-static char *as_list_for_asp(const struct osmo_ss7_asp *asp, char *buf, size_t buf_len) -{ - struct osmo_strbuf sb = { .buf = buf, .len = buf_len }; - const struct osmo_ss7_as *as; - unsigned int count = 0; - llist_for_each_entry(as, &asp->inst->as_list, list) { - if (!osmo_ss7_as_has_asp(as, asp)) - continue; - OSMO_STRBUF_PRINTF(sb, "%s%s", count != 0 ? "," : "", as->cfg.name); - count++; - break; - } - - if (count == 0) - OSMO_STRBUF_PRINTF(sb, "?"); - return buf; -} - /* Similar to osmo_sock_multiaddr_get_name_buf(), but aimed at listening sockets (only local part): */ static char *get_sockname_buf(char *buf, size_t buf_len, int fd, int proto, bool local) { diff --git a/src/tcap_as_loadshare.c b/src/tcap_as_loadshare.c index 356a982..9c969d3 100644 --- a/src/tcap_as_loadshare.c +++ b/src/tcap_as_loadshare.c @@ -716,7 +716,11 @@ goto out; }
- as = ipa_find_as_for_asp(asp); + /* in the IPA case, we assume there is a 1:1 mapping between the + * ASP and the AS. An AS without ASP means there is no + * connection, and an ASP without AS means that we don't (yet?) + * know the identity of the peer */ + as = ss7_asp_get_first_as(asp); if (!as) { LOGPASP(asp, DLTCAP, LOGL_ERROR, "Rx message for IPA ASP without AS?!\n"); rc = -ENOENT; diff --git a/src/xua_asp_fsm.c b/src/xua_asp_fsm.c index 32986fc..4204e31 100644 --- a/src/xua_asp_fsm.c +++ b/src/xua_asp_fsm.c @@ -394,14 +394,10 @@ { struct xua_asp_fsm_priv *xafp = fi->priv; struct osmo_ss7_asp *asp = xafp->asp; - struct osmo_ss7_instance *inst = asp->inst; - struct osmo_ss7_as *as; + struct ss7_as_asp_assoc *assoc;
- llist_for_each_entry(as, &inst->as_list, list) { - if (!osmo_ss7_as_has_asp(as, asp)) - continue; - osmo_fsm_inst_dispatch(as->fi, event, data); - } + llist_for_each_entry(assoc, &asp->assoc_as_list, asp_entry) + osmo_fsm_inst_dispatch(assoc->as->fi, event, data); }
/* check if expected message was received + stop t_ack */ @@ -483,17 +479,14 @@
static void common_asp_fsm_down_onenter(struct osmo_ss7_asp *asp) { - struct osmo_ss7_as *as, *as2; - struct osmo_ss7_instance *inst = asp->inst; + struct ss7_as_asp_assoc *assoc, *assoc2;
/* First notify all AS associated to the ASP that it went down: */ dispatch_to_all_as(asp->fi, XUA_ASPAS_ASP_DOWN_IND, asp);
/* Implicit clean up tasks: */ - llist_for_each_entry_safe(as, as2, &inst->as_list, list) { - if (!osmo_ss7_as_has_asp(as, asp)) - continue; - + llist_for_each_entry_safe(assoc, assoc2, &asp->assoc_as_list, asp_entry) { + struct osmo_ss7_as *as = assoc->as; #ifdef WITH_TCAP_LOADSHARING tcap_as_del_asp(as, asp); #endif @@ -702,10 +695,9 @@
if (traf_mode) { /* if the peer has specified a traffic mode at all */ enum osmo_ss7_as_traffic_mode tmode = osmo_ss7_tmode_from_xua(traf_mode); - llist_for_each_entry(as, &asp->inst->as_list, list) { - if (!osmo_ss7_as_has_asp(as, asp)) - continue; - + struct ss7_as_asp_assoc *assoc; + llist_for_each_entry(assoc, &asp->assoc_as_list, asp_entry) { + as = assoc->as; if (!as->cfg.mode_set_by_peer && !as->cfg.mode_set_by_vty) { as->cfg.mode = tmode; LOGPAS(as, DLSS7, LOGL_INFO, @@ -1531,7 +1523,7 @@ { struct osmo_fsm_inst *fi; struct ipa_asp_fsm_priv *iafp; - struct osmo_ss7_as *as = ipa_find_as_for_asp(asp); + struct osmo_ss7_as *as = ss7_asp_get_first_as(asp); const char *unit_name; bool can_start = true;
diff --git a/src/xua_default_lm_fsm.c b/src/xua_default_lm_fsm.c index 11dd32e..1c8df3e 100644 --- a/src/xua_default_lm_fsm.c +++ b/src/xua_default_lm_fsm.c @@ -155,21 +155,6 @@ #define ENSURE_SG_OR_IPSP(fi, event) \ ENSURE_ROLE_COND(fi, event, _role == OSMO_SS7_ASP_ROLE_SG || _role == OSMO_SS7_ASP_ROLE_IPSP)
-static struct osmo_ss7_as *find_first_as_in_asp(struct osmo_ss7_asp *asp) -{ - struct osmo_ss7_as *as; - - llist_for_each_entry(as, &asp->inst->as_list, list) { - struct ss7_as_asp_assoc *assoc; - llist_for_each_entry(assoc, &as->assoc_asp_list, as_entry) { - if (assoc->asp == asp) - return as; - } - } - - return NULL; -} - /* handle an incoming RKM registration response */ static int handle_reg_conf(struct osmo_fsm_inst *fi, uint32_t l_rk_id, uint32_t rctx) { @@ -358,7 +343,7 @@ 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 = ss7_asp_get_first_as(lmp->asp); if (!as) { LOGPFSML(fi, LOGL_ERROR, "Unable to find AS!\n"); ss7_asp_disconnect_stream(lmp->asp); diff --git a/src/xua_internal.h b/src/xua_internal.h index 81c8a1b..c8ae79f 100644 --- a/src/xua_internal.h +++ b/src/xua_internal.h @@ -147,7 +147,6 @@ struct msgb *ipa_to_msg(struct xua_msg *xua); int ipa_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua); int ipa_rx_msg(struct osmo_ss7_asp *asp, struct msgb *msg, uint8_t sls); -struct osmo_ss7_as *ipa_find_as_for_asp(struct osmo_ss7_asp *asp);
int osmo_isup_party_parse(char *out_digits, const uint8_t *in, unsigned int in_num_bytes, bool odd); diff --git a/src/xua_shared.c b/src/xua_shared.c index 022b8ed..0fa94c4 100644 --- a/src/xua_shared.c +++ b/src/xua_shared.c @@ -39,23 +39,6 @@ #include "ss7_internal.h" #include "xua_internal.h"
-/* if given ASP only has one AS, return that AS */ -static struct osmo_ss7_as *find_single_as_for_asp(const struct osmo_ss7_asp *asp) -{ - struct osmo_ss7_as *as, *as_found = NULL; - - llist_for_each_entry(as, &asp->inst->as_list, list) { - if (!osmo_ss7_as_has_asp(as, asp)) - continue; - /* check if we already had found another AS within this ASP -> not unique */ - if (as_found) - return NULL; - as_found = as; - } - - return as_found; -} - /* this is why we can use the M3UA constants below in a function shared between M3UA + SUA */ osmo_static_assert(M3UA_ERR_INVAL_ROUT_CTX == SUA_ERR_INVAL_ROUT_CTX, _err_rctx); osmo_static_assert(M3UA_ERR_NO_CONFGD_AS_FOR_ASP == SUA_ERR_NO_CONFGD_AS_FOR_ASP, _err_as_for_asp); @@ -94,13 +77,13 @@ } } else { /* no explicit routing context; this only works if there is only one AS in the ASP */ - *as = find_single_as_for_asp(asp); - if (!*as) { + if (asp->num_assoc_as != 1) { LOGPASP(asp, log_ss, LOGL_ERROR, "%s(): ASP sent M3UA without Routing Context IE but unable to uniquely " "identify the AS for this message\n", __func__); return M3UA_ERR_INVAL_ROUT_CTX; } + *as = ss7_asp_get_first_as(asp); }
return 0; diff --git a/tests/vty/osmo_stp_test.vty b/tests/vty/osmo_stp_test.vty index 61cbd66..72eb0de 100644 --- a/tests/vty/osmo_stp_test.vty +++ b/tests/vty/osmo_stp_test.vty @@ -657,7 +657,7 @@ No ASP named 'unknown-asp' found
OsmoSTP(config-cs7)# no asp my-asp -% ASP 'my-asp' currently configured in AS 'my-ass'. You must first remove the ASP from the AS configuration +% ASP 'my-asp' currently configured in 1 AS: 'my-ass'. You must first remove the ASP from the AS configuration OsmoSTP(config-cs7)# as my-ass m3ua OsmoSTP(config-cs7-as)# no asp my-asp OsmoSTP(config-cs7-as)# exit diff --git a/tests/vty/ss7_asp_test.vty b/tests/vty/ss7_asp_test.vty index 0cd0c39..1ff4e36 100644 --- a/tests/vty/ss7_asp_test.vty +++ b/tests/vty/ss7_asp_test.vty @@ -502,7 +502,7 @@ No ASP named 'unknown-asp' found
ss7_asp_vty_test(config-cs7)# no asp my-asp -% ASP 'my-asp' currently configured in AS 'my-ass'. You must first remove the ASP from the AS configuration +% ASP 'my-asp' currently configured in 1 AS: 'my-ass'. You must first remove the ASP from the AS configuration ss7_asp_vty_test(config-cs7)# as my-ass m3ua ss7_asp_vty_test(config-cs7-as)# no asp my-asp ss7_asp_vty_test(config-cs7-as)# exit