[PATCH] osmo-msc[master]: Don't answer to BSC-originated RESET with another RESET

Harald Welte gerrit-no-reply at lists.osmocom.org
Fri Feb 9 01:21:10 UTC 2018


Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/6326

to look at the new patch set (#3).

Don't answer to BSC-originated RESET with another RESET

If the BSC is contacting us for the first time and sending a BSSMAP
RESET, then we should simply ACK that and transition into the
"connected" state, where connection-oriented and connectionless
procedures are permitted.

This patch is a bit large for such a seemingly simple behavioural
change, but the existing data model didn't permit a more
straight-forward implementation.

Change-Id: Ie67e7ed20a6c42afe99bafef96d85a4e083dd057
Closes: OS#2914
---
M include/osmocom/msc/a_iface.h
M include/osmocom/msc/a_iface_bssap.h
M src/libmsc/a_iface.c
M src/libmsc/a_iface_bssap.c
4 files changed, 120 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/26/6326/3

diff --git a/include/osmocom/msc/a_iface.h b/include/osmocom/msc/a_iface.h
index 7417298..466e70b 100644
--- a/include/osmocom/msc/a_iface.h
+++ b/include/osmocom/msc/a_iface.h
@@ -73,6 +73,8 @@
  * (Helper function for a_iface_bssap.c) */
 void a_clear_all(struct osmo_sccp_user *scu, const struct osmo_sccp_addr *bsc_addr);
 
+void a_start_reset(struct bsc_context *bsc_ctx, bool already_connected);
+
 /* Delete info of a closed connection from the active connection list
  * (Helper function for a_iface_bssap.c) */
 void a_delete_bsc_con(uint32_t conn_id);
diff --git a/include/osmocom/msc/a_iface_bssap.h b/include/osmocom/msc/a_iface_bssap.h
index 79b8390..d4b67e3 100644
--- a/include/osmocom/msc/a_iface_bssap.h
+++ b/include/osmocom/msc/a_iface_bssap.h
@@ -20,17 +20,17 @@
 
 #pragma once
 
+#include <osmocom/msc/a_iface.h>
+
 /* Note: The structs and functions presented in this header file are intended
  * to be used only by a_iface.c. */
 
 /* A structure to hold tha most basic information about a sigtran connection
  * we use this struct internally here to pass connection data around */
 struct a_conn_info {
-	struct osmo_sccp_addr *msc_addr;
-	struct osmo_sccp_addr *bsc_addr;
+	struct bsc_context *bsc;
 	uint32_t conn_id;
 	struct gsm_network *network;
-	struct a_reset_ctx *reset;
 };
 
 /* Receive incoming connection less data messages via sccp */
diff --git a/src/libmsc/a_iface.c b/src/libmsc/a_iface.c
index 16df46d..600dbb8 100644
--- a/src/libmsc/a_iface.c
+++ b/src/libmsc/a_iface.c
@@ -65,6 +65,7 @@
 struct bsc_conn {
 	struct llist_head list;
 	uint32_t conn_id;			/* Connection identifier */
+	struct bsc_context *bsc;
 };
 
 /* Internal list with connections we currently maintain. This
@@ -72,7 +73,7 @@
 static LLIST_HEAD(active_connections);
 
 /* Record info of a new active connection in the active connection list */
-static void record_bsc_con(const void *ctx, uint32_t conn_id)
+static void record_bsc_con(const void *ctx, struct bsc_context *bsc, uint32_t conn_id)
 {
 	struct bsc_conn *conn;
 
@@ -80,6 +81,7 @@
 	OSMO_ASSERT(conn);
 
 	conn->conn_id = conn_id;
+	conn->bsc = bsc;
 
 	llist_add_tail(&conn->list, &active_connections);
 }
@@ -99,23 +101,32 @@
 	}
 }
 
-/* Check if a specified connection id has an active SCCP connection */
-static bool check_connection_active(uint32_t conn_id)
+/* Find a specified connection id */
+static struct bsc_conn *find_bsc_con(uint32_t conn_id)
 {
 	struct bsc_conn *conn;
 
 	/* Find the address for the current connection id */
 	llist_for_each_entry(conn, &active_connections, list) {
 		if (conn->conn_id == conn_id) {
-			return true;
+			return conn;
 		}
 	}
 
-	return false;
+	return NULL;
 }
 
-/* Get the reset context for a specifiec calling (BSC) address */
-static struct a_reset_ctx *get_reset_ctx_by_sccp_addr(const struct osmo_sccp_addr *addr)
+/* Check if a specified connection id has an active SCCP connection */
+static bool check_connection_active(uint32_t conn_id)
+{
+	if (find_bsc_con(conn_id))
+		return true;
+	else
+		return false;
+}
+
+/* Get the context for a specific calling (BSC) address */
+static struct bsc_context *get_bsc_context_by_sccp_addr(const struct osmo_sccp_addr *addr)
 {
 	struct bsc_context *bsc_ctx;
 	struct osmo_ss7_instance *ss7;
@@ -125,7 +136,7 @@
 
 	llist_for_each_entry(bsc_ctx, &gsm_network->a.bscs, list) {
 		if (memcmp(&bsc_ctx->bsc_addr, addr, sizeof(*addr)) == 0)
-			return bsc_ctx->reset;
+			return bsc_ctx;
 	}
 
 	ss7 = osmo_ss7_instance_find(gsm_network->a.cs7_instance);
@@ -452,20 +463,11 @@
 }
 
 /* Add a new BSC connection to our internal list with known BSCs */
-static void add_bsc(const struct osmo_sccp_addr *msc_addr, const struct osmo_sccp_addr *bsc_addr,
-		    struct osmo_sccp_user *scu)
+static struct bsc_context *add_bsc(const struct osmo_sccp_addr *msc_addr,
+				   const struct osmo_sccp_addr *bsc_addr, struct osmo_sccp_user *scu)
 {
 	struct bsc_context *bsc_ctx;
 	struct osmo_ss7_instance *ss7;
-	char bsc_name[32];
-
-	OSMO_ASSERT(bsc_addr);
-	OSMO_ASSERT(msc_addr);
-	OSMO_ASSERT(scu);
-
-	/* Check if we already know this BSC, if yes, skip adding it. */
-	if (get_reset_ctx_by_sccp_addr(bsc_addr))
-		return;
 
 	ss7 = osmo_ss7_instance_find(gsm_network->a.cs7_instance);
 	OSMO_ASSERT(ss7);
@@ -479,9 +481,34 @@
 	bsc_ctx->sccp_user = scu;
 	llist_add_tail(&bsc_ctx->list, &gsm_network->a.bscs);
 
+	return bsc_ctx;
+}
+
+/* start the BSSMAP RESET fsm */
+void a_start_reset(struct bsc_context *bsc_ctx, bool already_connected)
+{
+	char bsc_name[32];
+	OSMO_ASSERT(bsc_ctx->reset == NULL);
 	/* Start reset procedure to make the new connection active */
-	snprintf(bsc_name, sizeof(bsc_name), "bsc-%i", bsc_addr->pc);
-	bsc_ctx->reset = a_reset_alloc(bsc_ctx, bsc_name, a_reset_cb, bsc_ctx, false);
+	snprintf(bsc_name, sizeof(bsc_name), "bsc-%i", bsc_ctx->bsc_addr.pc);
+	bsc_ctx->reset = a_reset_alloc(bsc_ctx, bsc_name, a_reset_cb, bsc_ctx, already_connected);
+}
+
+/* determine if given msg is a BSSMAP RESET (true) or not (false) */
+static bool bssmap_is_reset(struct msgb *msg)
+{
+	struct bssmap_header *bs = (struct bssmap_header *)msgb_l2(msg);
+
+	if (msgb_l2len(msg) < sizeof(*bs))
+		return false;
+
+	if (bs->type != BSSAP_MSG_BSS_MANAGEMENT)
+		return false;
+
+	if (msg->l2h[sizeof(*bs)] == BSS_MAP_MSG_RESET)
+		return true;
+
+	return false;
 }
 
 /* Callback function, called by the SSCP stack when data arrives */
@@ -491,39 +518,53 @@
 	struct osmo_scu_prim *scu_prim = (struct osmo_scu_prim *)oph;
 	int rc = 0;
 	struct a_conn_info a_conn_info;
+	struct bsc_conn *bsc_con;
+
 	memset(&a_conn_info, 0, sizeof(a_conn_info));
 	a_conn_info.network = gsm_network;
-	a_conn_info.reset = NULL;
 
 	switch (OSMO_PRIM_HDR(&scu_prim->oph)) {
 	case OSMO_PRIM(OSMO_SCU_PRIM_N_CONNECT, PRIM_OP_INDICATION):
 		/* Handle inbound connection indication */
-		add_bsc(&scu_prim->u.connect.called_addr, &scu_prim->u.connect.calling_addr, scu);
 		a_conn_info.conn_id = scu_prim->u.connect.conn_id;
-		a_conn_info.msc_addr = &scu_prim->u.connect.called_addr;
-		a_conn_info.bsc_addr = &scu_prim->u.connect.calling_addr;
-		a_conn_info.reset = get_reset_ctx_by_sccp_addr(&scu_prim->u.unitdata.calling_addr);
+		a_conn_info.bsc = get_bsc_context_by_sccp_addr(&scu_prim->u.unitdata.calling_addr);
+		if (!a_conn_info.bsc) {
+			/* We haven't heard from this BSC before, allocate it */
+			a_conn_info.bsc = add_bsc(&scu_prim->u.connect.called_addr,
+						  &scu_prim->u.connect.calling_addr, scu);
+			a_start_reset(a_conn_info.bsc, false);
+		} else {
+			/* This BSC is already known to us, check if we have been through reset yet */
+			if (a_reset_conn_ready(a_conn_info.bsc->reset) == false) {
+				LOGP(DMSC, LOGL_NOTICE, "Refusing N-CONNECT.ind(%u, %s), BSC not reset yet\n",
+				     scu_prim->u.connect.conn_id, osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));
+				rc = osmo_sccp_tx_disconn(scu, a_conn_info.conn_id, &a_conn_info.bsc->msc_addr,
+							  SCCP_RETURN_CAUSE_UNQUALIFIED);
+				break;
+			}
 
-		if (a_reset_conn_ready(a_conn_info.reset) == false) {
-			rc = osmo_sccp_tx_disconn(scu, a_conn_info.conn_id, a_conn_info.msc_addr,
-						  SCCP_RETURN_CAUSE_UNQUALIFIED);
-			break;
+			osmo_sccp_tx_conn_resp(scu, scu_prim->u.connect.conn_id, &scu_prim->u.connect.called_addr, NULL, 0);
+			if (msgb_l2len(oph->msg) > 0) {
+				LOGP(DMSC, LOGL_DEBUG, "N-CONNECT.ind(%u, %s)\n",
+				     scu_prim->u.connect.conn_id, osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));
+				rc = a_sccp_rx_dt(scu, &a_conn_info, oph->msg);
+			} else
+				LOGP(DMSC, LOGL_DEBUG, "N-CONNECT.ind(%u)\n", scu_prim->u.connect.conn_id);
+
+			record_bsc_con(scu, a_conn_info.bsc, scu_prim->u.connect.conn_id);
 		}
-
-		osmo_sccp_tx_conn_resp(scu, scu_prim->u.connect.conn_id, &scu_prim->u.connect.called_addr, NULL, 0);
-		if (msgb_l2len(oph->msg) > 0) {
-			LOGP(DMSC, LOGL_DEBUG, "N-CONNECT.ind(%u, %s)\n",
-			     scu_prim->u.connect.conn_id, osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));
-			rc = a_sccp_rx_dt(scu, &a_conn_info, oph->msg);
-		} else
-			LOGP(DMSC, LOGL_DEBUG, "N-CONNECT.ind(%u)\n", scu_prim->u.connect.conn_id);
-
-		record_bsc_con(scu, scu_prim->u.connect.conn_id);
 		break;
 
 	case OSMO_PRIM(OSMO_SCU_PRIM_N_DATA, PRIM_OP_INDICATION):
 		/* Handle incoming connection oriented data */
+		bsc_con = find_bsc_con(scu_prim->u.data.conn_id);
+		if (!bsc_con) {
+			LOGP(DMSC, LOGL_ERROR, "N-DATA.ind(%u, %s) for unknown conn_id\n",
+				scu_prim->u.data.conn_id, osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));
+			break;
+		}
 		a_conn_info.conn_id = scu_prim->u.data.conn_id;
+		a_conn_info.bsc = bsc_con->bsc;
 		LOGP(DMSC, LOGL_DEBUG, "N-DATA.ind(%u, %s)\n",
 		     scu_prim->u.data.conn_id, osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));
 		a_sccp_rx_dt(scu, &a_conn_info, oph->msg);
@@ -531,10 +572,26 @@
 
 	case OSMO_PRIM(OSMO_SCU_PRIM_N_UNITDATA, PRIM_OP_INDICATION):
 		/* Handle inbound UNITDATA */
-		add_bsc(&scu_prim->u.unitdata.called_addr, &scu_prim->u.unitdata.calling_addr, scu);
-		a_conn_info.msc_addr = &scu_prim->u.unitdata.called_addr;
-		a_conn_info.bsc_addr = &scu_prim->u.unitdata.calling_addr;
-		a_conn_info.reset = get_reset_ctx_by_sccp_addr(&scu_prim->u.unitdata.calling_addr);
+		a_conn_info.bsc = get_bsc_context_by_sccp_addr(&scu_prim->u.unitdata.calling_addr);
+		if (!a_conn_info.bsc) {
+			/* We haven't heard from this BSC before, allocate it */
+			a_conn_info.bsc = add_bsc(&scu_prim->u.unitdata.called_addr,
+						&scu_prim->u.unitdata.calling_addr, scu);
+			/* if this not an inbound RESET, trigger an outbound RESET */
+			if (!bssmap_is_reset(oph->msg)) {
+				LOGP(DMSC, LOGL_NOTICE, "Ignoring N-UNITDATA.ind(%s), BSC not reset yet\n",
+					osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));
+				a_start_reset(a_conn_info.bsc, false);
+				break;
+			}
+		} else {
+			/* This BSC is already known to us, check if we have been through reset yet */
+			if (a_reset_conn_ready(a_conn_info.bsc->reset) == false) {
+				LOGP(DMSC, LOGL_NOTICE, "Ignoring N-UNITDATA.ind(%s), BSC not reset yet\n",
+					osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));
+				break;
+			}
+		}
 		DEBUGP(DMSC, "N-UNITDATA.ind(%s)\n", osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)));
 		a_sccp_rx_udt(scu, &a_conn_info, oph->msg);
 		break;
diff --git a/src/libmsc/a_iface_bssap.c b/src/libmsc/a_iface_bssap.c
index fc95dfb..7f0350b 100644
--- a/src/libmsc/a_iface_bssap.c
+++ b/src/libmsc/a_iface_bssap.c
@@ -66,7 +66,7 @@
 
 	/* Also backup the calling address of the BSC, this allows us to
 	 * identify later which BSC is responsible for this subscriber connection */
-	memcpy(&conn->a.bsc_addr, a_conn_info->bsc_addr, sizeof(conn->a.bsc_addr));
+	memcpy(&conn->a.bsc_addr, &a_conn_info->bsc->bsc_addr, sizeof(conn->a.bsc_addr));
 
 	llist_add_tail(&conn->entry, &network->subscr_conns);
 	LOGP(DMSC, LOGL_NOTICE, "A-Interface subscriber connection successfully allocated!\n");
@@ -111,11 +111,15 @@
 	OSMO_ASSERT(ss7);
 
 	LOGP(DMSC, LOGL_NOTICE, "Rx RESET from BSC %s, sending RESET ACK\n",
-	     osmo_sccp_addr_name(ss7, a_conn_info->bsc_addr));
-	osmo_sccp_tx_unitdata_msg(scu, a_conn_info->msc_addr, a_conn_info->bsc_addr, gsm0808_create_reset_ack());
+	     osmo_sccp_addr_name(ss7, &a_conn_info->bsc->bsc_addr));
+	osmo_sccp_tx_unitdata_msg(scu, &a_conn_info->bsc->msc_addr, &a_conn_info->bsc->bsc_addr,
+				  gsm0808_create_reset_ack());
 
 	/* Make sure all orphand subscriber connections will be cleard */
-	a_clear_all(scu, a_conn_info->bsc_addr);
+	a_clear_all(scu, &a_conn_info->bsc->bsc_addr);
+
+	if (!a_conn_info->bsc->reset)
+		a_start_reset(a_conn_info->bsc, true);
 
 	msgb_free(msg);
 }
@@ -131,17 +135,18 @@
 	ss7 = osmo_ss7_instance_find(network->a.cs7_instance);
 	OSMO_ASSERT(ss7);
 
-	if (a_conn_info->reset == NULL) {
+	if (a_conn_info->bsc->reset == NULL) {
 		LOGP(DMSC, LOGL_ERROR, "Received RESET ACK from an unknown BSC %s, ignoring...\n",
-		     osmo_sccp_addr_name(ss7, a_conn_info->bsc_addr));
+		     osmo_sccp_addr_name(ss7, &a_conn_info->bsc->bsc_addr));
 		goto fail;
 	}
 
-	LOGP(DMSC, LOGL_NOTICE, "Received RESET ACK from BSC %s\n", osmo_sccp_addr_name(ss7, a_conn_info->bsc_addr));
+	LOGP(DMSC, LOGL_NOTICE, "Received RESET ACK from BSC %s\n",
+		osmo_sccp_addr_name(ss7, &a_conn_info->bsc->bsc_addr));
 
 	/* Confirm that we managed to get the reset ack message
 	 * towards the connection reset logic */
-	a_reset_ack_confirm(a_conn_info->reset);
+	a_reset_ack_confirm(a_conn_info->bsc->reset);
 
 fail:
 	msgb_free(msg);
@@ -261,7 +266,7 @@
 
 	LOGP(DMSC, LOGL_NOTICE, "Releasing connection (conn_id=%i)\n", a_conn_info->conn_id);
 	rc = osmo_sccp_tx_disconn(scu, a_conn_info->conn_id,
-				  a_conn_info->msc_addr, SCCP_RELEASE_CAUSE_END_USER_ORIGINATED);
+				  NULL, SCCP_RELEASE_CAUSE_END_USER_ORIGINATED);
 
 	/* Remove the record from the list with active connections. */
 	a_delete_bsc_con(a_conn_info->conn_id);

-- 
To view, visit https://gerrit.osmocom.org/6326
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie67e7ed20a6c42afe99bafef96d85a4e083dd057
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder


More information about the gerrit-log mailing list