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;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31940
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: If42d93adee71d646766929a09bc01ae92b734ef3
Gerrit-Change-Number: 31940
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
Gerrit-MessageType: newchange
