laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/40005?usp=email )
Change subject: ipa: Store ASP IPA SLS into its own field ......................................................................
ipa: Store ASP IPA SLS into its own field
Storing the 4-bit SLS into asp_id field is a total hack which just pollutes the code and makes it more difficult to understand.
Add a new field into ss7_asp to store that information cleanly.
Moreover, asp_id field itself is right now a total mess on its own, since it is used both for local and remote ASP Identifiers at the same time. This will be fixed in a follow-up commitset.
Change-Id: I6ec346417ceeabfce9c7b856040be2179c59195e --- M src/osmo_ss7_asp.c M src/ss7_asp.h M src/xua_asp_fsm.c 3 files changed, 30 insertions(+), 28 deletions(-)
Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve
diff --git a/src/osmo_ss7_asp.c b/src/osmo_ss7_asp.c index 7e444b4..bb3c2dd 100644 --- a/src/osmo_ss7_asp.c +++ b/src/osmo_ss7_asp.c @@ -851,9 +851,7 @@
msg->dst = asp; rate_ctr_inc2(asp->ctrg, SS7_ASP_CTR_PKT_RX_TOTAL); - /* we simply use the lower 4 bits of the asp_id, which is initialized to a pseudo-random value upon - * connect */ - return ipa_rx_msg(asp, msg, asp->asp_id & 0xf); + return ipa_rx_msg(asp, msg, asp->ipa.sls); }
/* netif code tells us we can read something from the socket */ @@ -1035,9 +1033,7 @@
msg->dst = asp; rate_ctr_inc2(asp->ctrg, SS7_ASP_CTR_PKT_RX_TOTAL); - /* we simply use the lower 4 bits of the asp_id, which is initialized to a pseudo-random value upon - * connect */ - return ipa_rx_msg(asp, msg, asp->asp_id & 0xf); + return ipa_rx_msg(asp, msg, asp->ipa.sls); }
/* read call-back for M3UA-over-TCP socket */ diff --git a/src/ss7_asp.h b/src/ss7_asp.h index 483c5ea..df2f4c3 100644 --- a/src/ss7_asp.h +++ b/src/ss7_asp.h @@ -63,6 +63,15 @@ /*! Pending message for non-blocking IPA read */ struct msgb *pending_msg;
+ /* IPA proto ASP specific fields. */ + struct { + /* Incoming IPA PDUs have no SLS field, hence a potentially + * unique one within AS is assigned to this ASP and applied + * manually when received. */ + uint8_t sls:4; + bool sls_assigned; + } ipa; + struct { char *name; char *description; diff --git a/src/xua_asp_fsm.c b/src/xua_asp_fsm.c index 93c6d4d..4956417 100644 --- a/src/xua_asp_fsm.c +++ b/src/xua_asp_fsm.c @@ -1104,34 +1104,34 @@ } }
-/* Assign a 4 bit asp_id (as unqiue as possible) which will be used as SLS for incoming IPA PDUs.*/ -static void _ipa_asp_pick_unused_asp_id_as_sls(struct osmo_ss7_asp *asp, const struct osmo_ss7_as *as) +/* Assign a 4 bit SLS (as unqiue as possible) for incoming IPA PDUs.*/ +static void _ipa_asp_pick_unused_sls(struct osmo_ss7_asp *asp, const struct osmo_ss7_as *as) { - for (unsigned int asp_id = 0; asp_id <= 0x0f; asp_id++) { + for (unsigned int sls = 0; sls <= 0x0f; sls++) { bool used = false; for (unsigned i = 0; i < ARRAY_SIZE(as->cfg.asps); i++) { if (!as->cfg.asps[i]) continue; if (as->cfg.asps[i] == asp) continue; - if (!as->cfg.asps[i]->asp_id_present) + if (!as->cfg.asps[i]->ipa.sls_assigned) continue; - if (as->cfg.asps[i]->asp_id == asp_id) { + if (as->cfg.asps[i]->ipa.sls == sls) { used = true; break; } } if (used) continue; - /* Found an unused asp_id, use it: */ - asp->asp_id = asp_id; - asp->asp_id_present = true; - LOGPASP(asp, DLSS7, LOGL_DEBUG, "Assigned unsued asp_id = %u to be used as SLS\n", asp_id); + /* Found an unused SLS, use it: */ + asp->ipa.sls = sls; + asp->ipa.sls_assigned = true; + LOGPASP(asp, DLSS7, LOGL_DEBUG, "Assigned unsued SLS = %u\n", sls); return; } - LOGPASP(asp, DLSS7, LOGL_INFO, "All asp_ids in IPA AS picked, unique SLS not possible, picking random one\n"); - asp->asp_id = rand() & 0x0f; - asp->asp_id_present = true; + LOGPASP(asp, DLSS7, LOGL_INFO, "All SLS in IPA AS picked, unique SLS not possible, picking random one\n"); + asp->ipa.sls = rand() & 0x0f; + asp->ipa.sls_assigned = true; }
static void ipa_asp_allstate(struct osmo_fsm_inst *fi, uint32_t event, void *data) @@ -1160,12 +1160,9 @@ case XUA_ASP_E_AS_ASSIGNED: as = data; osmo_talloc_replace_string(iafp->ipa_unit, &iafp->ipa_unit->unit_name, as->cfg.name); - /* In IPA, asp_id is not really used on the wire, and we - * actually use internally the lower 4 bits of the field to - * fill in a potentailly unique SLS to apply to PDUs received from the IPA socket. - * Now that AS is known, try picking an unused asp_id inside the AS. - * we use the lower 4 bits of the asp_id field as SLS; */ - _ipa_asp_pick_unused_asp_id_as_sls(iafp->asp, as); + /* Now that AS is known, try picking an unused SLS inside the AS. + * It will be applied to PDUs received from the IPA socket. */ + _ipa_asp_pick_unused_sls(iafp->asp, as); /* Now that the AS is known, start the client side: */ if (iafp->role == OSMO_SS7_ASP_ROLE_ASP && fi->state == IPA_ASP_S_DOWN) { LOGPFSML(fi, LOGL_NOTICE, "Bringing up ASP now once it has been assigned to an AS\n"); @@ -1310,13 +1307,13 @@
if (as) { unit_name = as->cfg.name; - /* Allocacate potentially unique asp_id within AS since AS is already known: */ - _ipa_asp_pick_unused_asp_id_as_sls(asp, as); + /* Allocacate potentially unique SLS within AS since AS is already known: */ + _ipa_asp_pick_unused_sls(asp, as); } else if (asp->dyn_allocated) { LOGPFSML(fi, LOGL_INFO, "Dynamic ASP is not assigned to any AS, " "using ASP name instead of AS name as ipa_unit_name\n"); unit_name = asp->cfg.name; - /* asp->asp_id will be assigned together with AS unit_name during XUA_ASP_E_AS_ASSIGNED. */ + /* asp->ipa.sls will be assigned together with AS unit_name during XUA_ASP_E_AS_ASSIGNED. */ } else { /* ASP in client mode will be brought up when this ASP is added * to an AS, see XUA_ASP_E_AS_ASSIGNED. */ @@ -1325,7 +1322,7 @@ can_start = false; } unit_name = asp->cfg.name; - /* asp->asp_id will be assigned together with AS unit_name during XUA_ASP_E_AS_ASSIGNED. */ + /* asp->ipa.sls will be assigned together with AS unit_name during XUA_ASP_E_AS_ASSIGNED. */ }
iafp->role = role;