pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31940 )

Change subject: Fix Lb SCCP conn lookup after recent regression in optimization patch ......................................................................

Fix Lb SCCP conn lookup after recent regression in optimization patch

The conn_id used by a gsm_subscriber_conn is stored in different places depending on the underlaying sccp_instance (Lb or A). Furthermore, a gsm_subscriber_conn can have one conn_id allocated on each of Lb and A at the same. As a result, a gsm_subscriber_conn needs 2 rbtree nodes, since it can be associated to both Lb and A sccp_instances. Hence, this commit duplicates the code to have function for both Lb and A, since each of these functions use an rb_node and conn_id from different places in each gscon.

Fixes: 85062ccad31e9fb73e0256a5ee556c6ae0a16449 Change-Id: If42d93adee71d646766929a09bc01ae92b734ef3 --- M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/bsc_sccp.c M src/osmo-bsc/bsc_subscr_conn_fsm.c M src/osmo-bsc/lb.c M src/osmo-bsc/osmo_bsc_sigtran.c 5 files changed, 153 insertions(+), 26 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/40/31940/1

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index dc7ee3f..7e4b016 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -389,6 +389,8 @@

/* Lb interface to the SMLC: BSSMAP-LE/SCCP connection associated with this subscriber */ struct { + /* entry in (struct bsc_sccp_inst)->connections */ + struct rb_node node; /* Sigtran connection ID: * if set: Range (0..SCCP_CONN_ID_MAX) (24 bit) * if unset: SCCP_CONN_ID_UNSET (-1) if unset */ @@ -884,10 +886,17 @@ };

struct bsc_sccp_inst *bsc_sccp_inst_alloc(void *ctx); -uint32_t bsc_sccp_inst_next_conn_id(struct bsc_sccp_inst *bsc_sccp); -int bsc_sccp_inst_register_gscon(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn); -void bsc_sccp_inst_unregister_gscon(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn); -struct gsm_subscriber_connection *bsc_sccp_inst_get_gscon_by_conn_id(const struct bsc_sccp_inst *bsc_sccp, uint32_t conn_id); + +uint32_t bsc_sccp_inst_next_conn_id_a(struct bsc_sccp_inst *bsc_sccp); +int bsc_sccp_inst_register_gscon_a(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn); +void bsc_sccp_inst_unregister_gscon_a(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn); +struct gsm_subscriber_connection *bsc_sccp_inst_get_gscon_by_conn_id_a(const struct bsc_sccp_inst *bsc_sccp, uint32_t conn_id); + +uint32_t bsc_sccp_inst_next_conn_id_lb(struct bsc_sccp_inst *bsc_sccp); +int bsc_sccp_inst_register_gscon_lb(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn); +void bsc_sccp_inst_unregister_gscon_lb(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn); +struct gsm_subscriber_connection *bsc_sccp_inst_get_gscon_by_conn_id_lb(const struct bsc_sccp_inst *bsc_sccp, uint32_t conn_id); +

struct gsm_network { struct osmo_plmn_id plmn; diff --git a/src/osmo-bsc/bsc_sccp.c b/src/osmo-bsc/bsc_sccp.c index 6fa1d4b..ab204d8 100644 --- a/src/osmo-bsc/bsc_sccp.c +++ b/src/osmo-bsc/bsc_sccp.c @@ -38,7 +38,7 @@ return bsc_sccp; }

-int bsc_sccp_inst_register_gscon(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn) +int bsc_sccp_inst_register_gscon_a(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn) { struct rb_node **n = &(bsc_sccp->connections.rb_node); struct rb_node *parent = NULL; @@ -68,22 +68,53 @@ return 0; }

-void bsc_sccp_inst_unregister_gscon(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn) +int bsc_sccp_inst_register_gscon_lb(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn) +{ + struct rb_node **n = &(bsc_sccp->connections.rb_node); + struct rb_node *parent = NULL; + uint32_t conn_id = conn->lcs.lb.conn_id; + + OSMO_ASSERT(conn_id != SCCP_CONN_ID_UNSET); + + while (*n) { + struct gsm_subscriber_connection *it; + + it = container_of(*n, struct gsm_subscriber_connection, lcs.lb.node); + + parent = *n; + if (conn_id < it->lcs.lb.conn_id) { + n = &((*n)->rb_left); + } else if (conn_id > it->lcs.lb.conn_id) { + n = &((*n)->rb_right); + } else { + LOGP(DMSC, LOGL_ERROR, + "Trying to reserve already reserved conn_id %u\n", conn_id); + return -EEXIST; + } + } + + rb_link_node(&conn->lcs.lb.node, parent, n); + rb_insert_color(&conn->lcs.lb.node, &bsc_sccp->connections); + return 0; +} + +void bsc_sccp_inst_unregister_gscon_a(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn) { OSMO_ASSERT(conn->sccp.conn_id != SCCP_CONN_ID_UNSET); rb_erase(&conn->sccp.node, &bsc_sccp->connections); }

-/* Helper function to Check if the given connection id is already assigned */ -struct gsm_subscriber_connection *bsc_sccp_inst_get_gscon_by_conn_id(const struct bsc_sccp_inst *bsc_sccp, uint32_t conn_id) +void bsc_sccp_inst_unregister_gscon_lb(struct bsc_sccp_inst *bsc_sccp, struct gsm_subscriber_connection *conn) +{ + OSMO_ASSERT(conn->lcs.lb.conn_id != SCCP_CONN_ID_UNSET); + rb_erase(&conn->lcs.lb.node, &bsc_sccp->connections); +} + +struct gsm_subscriber_connection *bsc_sccp_inst_get_gscon_by_conn_id_a(const struct bsc_sccp_inst *bsc_sccp, uint32_t conn_id) { const struct rb_node *node = bsc_sccp->connections.rb_node; struct gsm_subscriber_connection *conn;

- OSMO_ASSERT(conn_id != SCCP_CONN_ID_UNSET); - /* Range (0..SCCP_CONN_ID_MAX) expected, see bsc_sccp_inst_next_conn_id() */ - OSMO_ASSERT(conn_id <= SCCP_CONN_ID_MAX); - while (node) { conn = container_of(node, struct gsm_subscriber_connection, sccp.node); if (conn_id < conn->sccp.conn_id) @@ -97,8 +128,26 @@ return NULL; }

+struct gsm_subscriber_connection *bsc_sccp_inst_get_gscon_by_conn_id_lb(const struct bsc_sccp_inst *bsc_sccp, uint32_t conn_id) +{ + const struct rb_node *node = bsc_sccp->connections.rb_node; + struct gsm_subscriber_connection *conn; + + while (node) { + conn = container_of(node, struct gsm_subscriber_connection, lcs.lb.node); + if (conn_id < conn->lcs.lb.conn_id) + node = node->rb_left; + else if (conn_id > conn->lcs.lb.conn_id) + node = node->rb_right; + else + return conn; + } + + return NULL; +} + /* We need an unused SCCP conn_id across all SCCP users. */ -uint32_t bsc_sccp_inst_next_conn_id(struct bsc_sccp_inst *bsc_sccp) +uint32_t bsc_sccp_inst_next_conn_id_a(struct bsc_sccp_inst *bsc_sccp) { uint32_t first_id, test_id;

@@ -116,7 +165,46 @@ *let's simply use 24 bit ids to fit all link types (excluding 0x00ffffff). */

- while (bsc_sccp_inst_get_gscon_by_conn_id(bsc_sccp, test_id)) { + while (bsc_sccp_inst_get_gscon_by_conn_id_a(bsc_sccp, test_id)) { + /* Optimized modulo operation (% SCCP_CONN_ID_MAX) using bitwise AND plus CMP: */ + test_id = (test_id + 1) & 0x00FFFFFF; + if (OSMO_UNLIKELY(test_id == 0x00FFFFFF)) + test_id = 0; + + /* Did a whole loop, all used, fail */ + if (OSMO_UNLIKELY(test_id == first_id)) + return SCCP_CONN_ID_UNSET; + } + + bsc_sccp->next_id = test_id; + /* Optimized modulo operation (% SCCP_CONN_ID_MAX) using bitwise AND plus CMP: */ + bsc_sccp->next_id = (bsc_sccp->next_id + 1) & 0x00FFFFFF; + if (OSMO_UNLIKELY(bsc_sccp->next_id == 0x00FFFFFF)) + bsc_sccp->next_id = 0; + + return test_id; +} + +/* We need an unused SCCP conn_id across all SCCP users. */ +uint32_t bsc_sccp_inst_next_conn_id_lb(struct bsc_sccp_inst *bsc_sccp) +{ + uint32_t first_id, test_id; + + first_id = test_id = bsc_sccp->next_id; + + /* SUA: RFC3868 sec 3.10.4: + * The source reference number is a 4 octet long integer. + * This is allocated by the source SUA instance. + * M3UA/SCCP: ITU-T Q.713 sec 3.3: + * The "source local reference" parameter field is a three-octet field containing a + * reference number which is generated and used by the local node to identify the + * connection section after the connection section is set up. + * The coding "all ones" is reserved for future use. + *Hence, as we currently use the connection ID also as local reference, + *let's simply use 24 bit ids to fit all link types (excluding 0x00ffffff). + */ + + while (bsc_sccp_inst_get_gscon_by_conn_id_lb(bsc_sccp, test_id)) { /* Optimized modulo operation (% SCCP_CONN_ID_MAX) using bitwise AND plus CMP: */ test_id = (test_id + 1) & 0x00FFFFFF; if (OSMO_UNLIKELY(test_id == 0x00FFFFFF)) diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c index 8e21992..f6e1a1f 100644 --- a/src/osmo-bsc/bsc_subscr_conn_fsm.c +++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c @@ -1113,9 +1113,14 @@ } if (conn->sccp.conn_id != SCCP_CONN_ID_UNSET && conn->sccp.msc) { struct bsc_sccp_inst *bsc_sccp = osmo_sccp_get_priv(conn->sccp.msc->a.sccp); - bsc_sccp_inst_unregister_gscon(bsc_sccp, conn); + bsc_sccp_inst_unregister_gscon_a(bsc_sccp, conn); conn->sccp.conn_id = SCCP_CONN_ID_UNSET; } + if (conn->lcs.lb.conn_id != SCCP_CONN_ID_UNSET) { + struct bsc_sccp_inst *bsc_sccp = osmo_sccp_get_priv(bsc_gsmnet->smlc->sccp); + bsc_sccp_inst_unregister_gscon_lb(bsc_sccp, conn); + conn->lcs.lb.conn_id = SCCP_CONN_ID_UNSET; + }

if (conn->bsub) { LOGPFSML(fi, LOGL_DEBUG, "Putting bsc_subscr\n"); @@ -1232,6 +1237,7 @@ INIT_LLIST_HEAD(&conn->dtap_queue); INIT_LLIST_HEAD(&conn->hodec2.penalty_timers); conn->sccp.conn_id = SCCP_CONN_ID_UNSET; + conn->lcs.lb.conn_id = SCCP_CONN_ID_UNSET; /* Default clear cause (on RR translates to GSM48_RR_CAUSE_ABNORMAL_UNSPEC) */ conn->clear_cause = GSM0808_CAUSE_EQUIPMENT_FAILURE;

diff --git a/src/osmo-bsc/lb.c b/src/osmo-bsc/lb.c index 6c6f599..6c2e7a0 100644 --- a/src/osmo-bsc/lb.c +++ b/src/osmo-bsc/lb.c @@ -159,7 +159,7 @@ case OSMO_PRIM(OSMO_SCU_PRIM_N_CONNECT, PRIM_OP_CONFIRM): /* Handle inbound confirmation of outbound connection */ DEBUGP(DLCS, "N-CONNECT.cnf(%u)\n", scu_prim->u.connect.conn_id); - conn = bsc_sccp_inst_get_gscon_by_conn_id(bsc_sccp, scu_prim->u.connect.conn_id); + conn = bsc_sccp_inst_get_gscon_by_conn_id_lb(bsc_sccp, scu_prim->u.connect.conn_id); if (conn) { conn->lcs.lb.state = SUBSCR_SCCP_ST_CONNECTED; if (msgb_l2len(oph->msg) > 0) { @@ -175,7 +175,7 @@ /* Handle incoming connection oriented data */ DEBUGP(DLCS, "N-DATA.ind(%u)\n", scu_prim->u.data.conn_id);

- conn = bsc_sccp_inst_get_gscon_by_conn_id(bsc_sccp, scu_prim->u.data.conn_id); + conn = bsc_sccp_inst_get_gscon_by_conn_id_lb(bsc_sccp, scu_prim->u.data.conn_id); if (!conn) { LOGP(DLCS, LOGL_ERROR, "N-DATA.ind(%u) for unknown conn_id\n", scu_prim->u.data.conn_id); rc = -EINVAL; @@ -193,7 +193,7 @@ osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)), scu_prim->u.disconnect.cause); /* indication of disconnect */ - conn = bsc_sccp_inst_get_gscon_by_conn_id(bsc_sccp, scu_prim->u.disconnect.conn_id); + conn = bsc_sccp_inst_get_gscon_by_conn_id_lb(bsc_sccp, scu_prim->u.disconnect.conn_id); if (!conn) { LOGP(DLCS, LOGL_ERROR, "N-DISCONNECT.ind for unknown conn_id %u\n", scu_prim->u.disconnect.conn_id); @@ -232,12 +232,16 @@ return -EINVAL; }

- conn_id = bsc_sccp_inst_next_conn_id(bsc_sccp); + conn_id = bsc_sccp_inst_next_conn_id_lb(bsc_sccp); if (conn_id == SCCP_CONN_ID_UNSET) { LOGPFSMSL(conn->fi, DLCS, LOGL_ERROR, "Unable to allocate SCCP Connection ID for BSSMAP-LE to SMLC\n"); return -ENOSPC; } conn->lcs.lb.conn_id = conn_id; + if (bsc_sccp_inst_register_gscon_lb(bsc_sccp, conn) < 0) { + LOGP(DMSC, LOGL_ERROR, "Unable to register Lb SCCP connection (id=%u)\n", conn->lcs.lb.conn_id); + return -1; + } ss7 = osmo_ss7_instance_find(bsc_gsmnet->smlc->cs7_instance); OSMO_ASSERT(ss7); LOGPFSMSL(conn->fi, DLCS, LOGL_INFO, "Opening new SCCP connection (id=%u) to SMLC: %s\n", conn_id, diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c index 1642059..a2ee079 100644 --- a/src/osmo-bsc/osmo_bsc_sigtran.c +++ b/src/osmo-bsc/osmo_bsc_sigtran.c @@ -142,7 +142,7 @@ struct gsm_subscriber_connection *conn; int rc = 0;

- conn = bsc_sccp_inst_get_gscon_by_conn_id(bsc_sccp, scu_prim->u.connect.conn_id); + conn = bsc_sccp_inst_get_gscon_by_conn_id_a(bsc_sccp, scu_prim->u.connect.conn_id); if (conn) { LOGP(DMSC, LOGL_NOTICE, "(calling_addr=%s conn_id=%u) N-CONNECT.ind with already used conn_id, ignoring\n", @@ -171,7 +171,7 @@ return -ENOMEM; conn->sccp.msc = msc; conn->sccp.conn_id = scu_prim->u.connect.conn_id; - if (bsc_sccp_inst_register_gscon(bsc_sccp, conn) < 0) { + if (bsc_sccp_inst_register_gscon_a(bsc_sccp, conn) < 0) { LOGP(DMSC, LOGL_NOTICE, "(calling_addr=%s conn_id=%u) N-CONNECT.ind failed registering conn\n", osmo_sccp_addr_dump(&scu_prim->u.connect.calling_addr), scu_prim->u.connect.conn_id); osmo_fsm_inst_term(conn->fi, OSMO_FSM_TERM_REQUEST, NULL); @@ -215,7 +215,7 @@ /* Handle outbound connection confirmation */ DEBUGP(DMSC, "N-CONNECT.cnf(%u, %s)\n", scu_prim->u.connect.conn_id, osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg))); - conn = bsc_sccp_inst_get_gscon_by_conn_id(bsc_sccp, scu_prim->u.connect.conn_id); + conn = bsc_sccp_inst_get_gscon_by_conn_id_a(bsc_sccp, scu_prim->u.connect.conn_id); if (conn) { osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_A_CONN_CFM, scu_prim); conn->sccp.state = SUBSCR_SCCP_ST_CONNECTED; @@ -234,7 +234,7 @@ osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));

/* Incoming data is a sign of a vital connection */ - conn = bsc_sccp_inst_get_gscon_by_conn_id(bsc_sccp, scu_prim->u.data.conn_id); + conn = bsc_sccp_inst_get_gscon_by_conn_id_a(bsc_sccp, scu_prim->u.data.conn_id); if (conn) { a_reset_conn_success(conn->sccp.msc); handle_data_from_msc(conn, oph->msg); @@ -246,7 +246,7 @@ osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)), scu_prim->u.disconnect.cause); /* indication of disconnect */ - conn = bsc_sccp_inst_get_gscon_by_conn_id(bsc_sccp, scu_prim->u.disconnect.conn_id); + conn = bsc_sccp_inst_get_gscon_by_conn_id_a(bsc_sccp, scu_prim->u.disconnect.conn_id); if (conn) { conn->sccp.state = SUBSCR_SCCP_ST_NONE; if (msgb_l2len(oph->msg) > 0) @@ -318,12 +318,12 @@ }

bsc_sccp = osmo_sccp_get_priv(msc->a.sccp); - conn->sccp.conn_id = conn_id = bsc_sccp_inst_next_conn_id(bsc_sccp); + conn->sccp.conn_id = conn_id = bsc_sccp_inst_next_conn_id_a(bsc_sccp); if (conn->sccp.conn_id == SCCP_CONN_ID_UNSET) { LOGP(DMSC, LOGL_ERROR, "Unable to allocate SCCP Connection ID\n"); return -1; } - if (bsc_sccp_inst_register_gscon(bsc_sccp, conn) < 0) { + if (bsc_sccp_inst_register_gscon_a(bsc_sccp, conn) < 0) { LOGP(DMSC, LOGL_ERROR, "Unable to register SCCP connection (id=%u)\n", conn->sccp.conn_id); return -1; }