pespin submitted this change.

View Change

Approvals: fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified
ipa: Try picking unused asp_id in AS to use as SLS

Initially (b93e1d01205cdd7bd7a65e15945f664f31fb4bcb), incoming traffic
from IPA sockets was tagged with SLS using the LSB of the socket file
descriptor. that did however not achieve sufficient entropy in real
world use cases.

Later on (2c9ba16c2c0210a189c72064eafad5ef336254cd), SLS tagging was
changed to pseudo-random generation.
Still it has been found too that it may create bad entropy under certain
condtions, with eg. havig only 2 incoming ASPs and both ending up with
the same asp_id/SLS, which won't allow for proper load sharing towards
other peers.

This commit changes the previous method by delaying SLS assignment
decision of the socket until the ASP becomes assigned to an AS
(basically during IPA handshake, when we receive IPA IDENTITY RESPONSE
with the Unit-Name, or when we receive the IPA IDENTITY ACK for it).
It is fine delaying SLS assignment to that point, since it's impossible
to receive any userplane data before that handhsake ocurrs.
Once the ASP is assigned to the AS (IPA ASPs can only be part of one
AS), we lookup the other ASPs in the AS to try to allocate an unused
4-bit asp_id which will end up being used as SLS.

Related: SYS#6543
Related: SYS#6802

Change-Id: I723eac25e59002630dca87a738c8eb7c62edec75
---
M src/osmo_ss7_asp.c
M src/osmo_ss7_xua_srv.c
M src/xua_asp_fsm.c
3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/src/osmo_ss7_asp.c b/src/osmo_ss7_asp.c
index f785264..7e444b4 100644
--- a/src/osmo_ss7_asp.c
+++ b/src/osmo_ss7_asp.c
@@ -979,12 +979,6 @@
if (asp->cfg.trans_proto == IPPROTO_SCTP) {
rc = ss7_asp_apply_peer_primary_address(asp);
rc = ss7_asp_apply_primary_address(asp);
- } else {
- if (asp->cfg.proto == OSMO_SS7_ASP_PROT_IPA) {
- /* we use the lower 4 bits of the asp_id field as SLS;
- * let's initialize it here from a pseudo-random value */
- asp->asp_id = rand() & 0xf;
- }
}

if (asp->lm && asp->lm->prim_cb) {
diff --git a/src/osmo_ss7_xua_srv.c b/src/osmo_ss7_xua_srv.c
index 35e360f..52a3508 100644
--- a/src/osmo_ss7_xua_srv.c
+++ b/src/osmo_ss7_xua_srv.c
@@ -180,12 +180,6 @@
if (asp->cfg.trans_proto == IPPROTO_SCTP) {
rc = ss7_asp_apply_peer_primary_address(asp);
rc = ss7_asp_apply_primary_address(asp);
- } else {
- if (asp->cfg.proto == OSMO_SS7_ASP_PROT_IPA) {
- /* we use the lower 4 bits of the asp_id field as SLS;
- * let's initialize it here from a pseudo-random value */
- asp->asp_id = rand() & 0xf;
- }
}

/* send M-SCTP_ESTABLISH.ind to Layer Manager */
diff --git a/src/xua_asp_fsm.c b/src/xua_asp_fsm.c
index cd60df2..93c6d4d 100644
--- a/src/xua_asp_fsm.c
+++ b/src/xua_asp_fsm.c
@@ -1104,6 +1104,36 @@
}
}

+/* 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)
+{
+ for (unsigned int asp_id = 0; asp_id <= 0x0f; asp_id++) {
+ 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)
+ continue;
+ if (as->cfg.asps[i]->asp_id == asp_id) {
+ 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);
+ 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;
+}
+
static void ipa_asp_allstate(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
struct ipa_asp_fsm_priv *iafp = fi->priv;
@@ -1130,6 +1160,12 @@
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 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");
@@ -1274,10 +1310,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);
} 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. */
} else {
/* ASP in client mode will be brought up when this ASP is added
* to an AS, see XUA_ASP_E_AS_ASSIGNED. */
@@ -1286,6 +1325,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. */
}

iafp->role = role;

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

Gerrit-MessageType: merged
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I723eac25e59002630dca87a738c8eb7c62edec75
Gerrit-Change-Number: 40003
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann@sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: osmith <osmith@sysmocom.de>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>