bssap: paging: page entire BSS for unimplemented cell id list

3GPP TS § 08.08 defines various types of Cell Identifier List IEs, but we only
implement "entire BSS" and "one LAC". If the MSC sends a Cell Identifier List
that we don't implement, it is best for interoperability to page the entire BSS
and post a log message instead of rejecting the paging altogether. Apart from
resource management, it is not harmful to page more than the MSC requested; if
use of resources becomes an issue, the log message will guide towards the
solution of providing an actually implemented Cell Identifier List IE.

Upon IE length that is other than we expect, log the error, but also fall back
to paging the entire BSS. Overall message length correctness has been checked
earlier.

The particular case observed is that a Huwaei MSC sends a LAI for Cell
Identifier List (MCC+MNC in bcd, followed by a LAC), parsing of which we may
want to add later.

Improve logging: identify the subscriber that is being paged.

Coding style: use a switch() statement to clarify flow and provide a place to
add more implementations later.

Add regression test bssap_test.c: fabricates BSSAP Paging messages with the two
implemented Cell Identifier List IEs as well as the unimplemented LAI
identifier, verify the resulting paging LAC in wrapped function and stderr.

Change-Id: Ie934c5d229140a89763bf2efff86d6a3766cd351

8 files changed, 240 insertions, 9 deletions

diff --git a/configure.ac b/configure.ac
index 79523d141..02a81c691 100644
--- a/configure.ac
+++ b/configure.ac
@@ -149,6 +149,7 @@ AC_OUTPUT(
     tests/trau/Makefile
     tests/subscr/Makefile
     tests/nanobts_omlattr/Makefile
+    tests/bssap/Makefile
     doc/Makefile
     doc/examples/Makefile
     contrib/Makefile
diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 431125024..93e9274a0 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -233,10 +233,11 @@ static int bssmap_handle_paging(struct bsc_msc_data *msc,
 	struct tlv_parsed tp;
 	char mi_string[GSM48_MI_SIZE];
 	uint32_t tmsi = GSM_RESERVED_TMSI;
-	unsigned int lac = GSM_LAC_RESERVED_ALL_BTS;
+	unsigned int lac;
 	uint8_t data_length;
 	const uint8_t *data;
 	uint8_t chan_needed = RSL_CHANNEED_ANY;
+	uint8_t cell_ident;
 
 	tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, payload_length - 1, 0, 0);
 
@@ -265,21 +266,50 @@ static int bssmap_handle_paging(struct bsc_msc_data *msc,
 			   TLVP_VAL(&tp, GSM0808_IE_IMSI), TLVP_LEN(&tp, GSM0808_IE_IMSI));
 
 	/*
-	 * parse the cell identifier list
+	 * There are various cell identifier list types defined at 3GPP TS § 08.08, we don't support all
+	 * of them yet. To not disrupt paging operation just because we're lacking some implementation,
+	 * interpret any unknown cell identifier type as "page the entire BSS".
 	 */
 	data_length = TLVP_LEN(&tp, GSM0808_IE_CELL_IDENTIFIER_LIST);
 	data = TLVP_VAL(&tp, GSM0808_IE_CELL_IDENTIFIER_LIST);
 
-	/*
-	 * Support paging to all network or one BTS at one LAC
-	 */
-	if (data_length == 3 && data[0] == CELL_IDENT_LAC) {
-		lac = osmo_load16be(&data[1]);
-	} else if (data_length > 1 || (data[0] & 0x0f) != CELL_IDENT_BSS) {
-		LOGP(DMSC, LOGL_ERROR, "Unsupported Cell Identifier List: %s\n", osmo_hexdump(data, data_length));
+	if (data_length < 1) {
+		LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Zero length Cell Identifier List\n",
+		     mi_string);
 		return -1;
 	}
 
+	cell_ident = data[0] & 0xf;
+
+	/* Default fallback: page entire BSS */
+	lac = GSM_LAC_RESERVED_ALL_BTS;
+
+	switch (cell_ident) {
+	case CELL_IDENT_LAC:
+		if (data_length != 3) {
+			LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Cell Identifier List for LAC (0x%x)"
+			     " has invalid length: %u, paging entire BSS instead (%s)\n",
+			     mi_string, CELL_IDENT_LAC, data_length, osmo_hexdump(data, data_length));
+			break;
+		}
+		lac = osmo_load16be(&data[1]);
+		break;
+
+	case CELL_IDENT_BSS:
+		if (data_length != 1) {
+			LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Cell Identifier List for BSS (0x%x)"
+			     " has invalid length: %u, paging entire BSS anyway (%s)\n",
+			     mi_string, CELL_IDENT_BSS, data_length, osmo_hexdump(data, data_length));
+		}
+		break;
+
+	default:
+		LOGP(DMSC, LOGL_NOTICE, "Paging IMSI %s: unimplemented Cell Identifier List (0x%x),"
+		     " paging entire BSS instead (%s)\n",
+		     mi_string, cell_ident, osmo_hexdump(data, data_length));
+		break;
+	}
+
 	if (TLVP_PRESENT(&tp, GSM0808_IE_CHANNEL_NEEDED) && TLVP_LEN(&tp, GSM0808_IE_CHANNEL_NEEDED) == 1)
 		chan_needed = TLVP_VAL(&tp, GSM0808_IE_CHANNEL_NEEDED)[0] & 0x03;
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index aff05bb44..474f82167 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -8,6 +8,7 @@ SUBDIRS = \
 	nanobts_omlattr \
 	bsc-nat \
 	bsc-nat-trie \
+	bssap \
 	$(NULL)
 
 # The `:;' works around a Bash 3.2 bug when the output is not writeable.
diff --git a/tests/bssap/Makefile.am b/tests/bssap/Makefile.am
new file mode 100644
index 000000000..80d655fe7
--- /dev/null
+++ b/tests/bssap/Makefile.am
@@ -0,0 +1,50 @@
+AM_CPPFLAGS = \
+	$(all_includes) \
+	-I$(top_srcdir)/include \
+	$(NULL)
+
+AM_CFLAGS = \
+	-Wall \
+	-ggdb3 \
+	$(LIBOSMOCORE_CFLAGS) \
+	$(LIBOSMOGSM_CFLAGS) \
+	$(LIBOSMOABIS_CFLAGS) \
+	$(LIBOSMOSIGTRAN_CFLAGS) \
+	$(COVERAGE_CFLAGS) \
+	$(NULL)
+
+EXTRA_DIST = \
+	bssap_test.ok \
+	bssap_test.err \
+	$(NULL)
+
+noinst_PROGRAMS = \
+	bssap_test \
+	$(NULL)
+
+bssap_test_SOURCES = \
+	bssap_test.c \
+	$(top_srcdir)/src/osmo-bsc/osmo_bsc_bssap.c \
+	$(top_srcdir)/src/osmo-bsc/osmo_bsc_sigtran.c \
+	$(top_srcdir)/src/osmo-bsc/osmo_bsc_filter.c \
+	$(top_srcdir)/src/osmo-bsc/osmo_bsc_grace.c \
+	$(NULL)
+
+bssap_test_LDADD = \
+	$(top_builddir)/src/libbsc/libbsc.a \
+	$(top_builddir)/src/libcommon/libcommon.a \
+	$(top_builddir)/src/libcommon-cs/libcommon-cs.a \
+	$(top_builddir)/src/libtrau/libtrau.a \
+	$(LIBOSMOCORE_LIBS) \
+	$(LIBOSMOGSM_LIBS) \
+	$(LIBOSMOABIS_LIBS) \
+	$(LIBOSMOSIGTRAN_LIBS) \
+	$(NULL)
+
+bssap_test_LDFLAGS = \
+	-Wl,--wrap=bsc_grace_paging_request \
+	$(NULL)
+
+.PHONY: update_exp
+update_exp:
+	$(builddir)/bssap_test >$(srcdir)/bssap_test.ok 2>$(srcdir)/bssap_test.err
diff --git a/tests/bssap/bssap_test.c b/tests/bssap/bssap_test.c
new file mode 100644
index 000000000..2b154c100
--- /dev/null
+++ b/tests/bssap/bssap_test.c
@@ -0,0 +1,120 @@
+/*
+ * (C) 2017 by sysmocom - s.f.m.c. GmbH <info@sysmocom.de>
+ * All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <osmocom/core/application.h>
+
+#include <osmocom/bsc/debug.h>
+#include <osmocom/bsc/osmo_bsc.h>
+#include <osmocom/bsc/signal.h>
+#include <osmocom/bsc/bsc_subscriber.h>
+#include <osmocom/bsc/bsc_msc_data.h>
+#include <osmocom/bsc/common_bsc.h>
+#include <osmocom/bsc/osmo_bsc_rf.h>
+
+struct msgb *msgb_from_hex(const char *label, uint16_t size, const char *hex)
+{
+	struct msgb *msg = msgb_alloc(size, label);
+	unsigned char *rc;
+	msg->l2h = msg->l3h = msg->head;
+	rc = msgb_put(msg, osmo_hexparse(hex, msg->head, msgb_tailroom(msg)));
+	OSMO_ASSERT(rc == msg->l2h);
+	return msg;
+}
+
+uint16_t gl_expect_lac = 0;
+
+/* override, requires '-Wl,--wrap=bsc_grace_paging_request' */
+int __real_bsc_grace_paging_request(enum signal_rf rf_policy, struct bsc_subscr *subscr, int chan_needed,
+				    struct bsc_msc_data *msc);
+int __wrap_bsc_grace_paging_request(enum signal_rf rf_policy, struct bsc_subscr *subscr, int chan_needed,
+				    struct bsc_msc_data *msc)
+{
+	if (subscr->lac == GSM_LAC_RESERVED_ALL_BTS)
+		fprintf(stderr, "BSC paging started on entire BSS (%u)\n", subscr->lac);
+	else
+		fprintf(stderr, "BSC paging started with LAC %u\n", subscr->lac);
+	OSMO_ASSERT(gl_expect_lac == subscr->lac);
+	return 0;
+}
+
+struct {
+	const char *msg;
+	uint16_t expect_lac;
+	int expect_rc;
+} cell_identifier_tests[] = {
+	{
+		"001652080859512069000743940904010844601a03050065",
+		/*                                         ^^^^^^ Cell Identifier List: LAC */
+		0x65, 0
+	},
+	{
+		"001452080859512069000743940904010844601a0106",
+		/*                                         ^^ Cell Identifier List: BSS */
+		GSM_LAC_RESERVED_ALL_BTS, 0
+	},
+	{
+		"001952080859512069000743940904010844601a060415f5490065",
+		/*                                         ^^^^^^^^^^^^ Cell Identifier List: LAI */
+		GSM_LAC_RESERVED_ALL_BTS, 0
+	},
+};
+
+void test_cell_identifier()
+{
+	int i;
+	int rc;
+	struct gsm_network *net;
+	struct bsc_msc_data *msc;
+
+	net = bsc_network_init(NULL, 1, 1, NULL);
+	net->bsc_data->rf_ctrl = talloc_zero(NULL, struct osmo_bsc_rf);
+	net->bsc_data->rf_ctrl->policy = S_RF_ON;
+
+	msc = talloc_zero(net, struct bsc_msc_data);
+	msc->network = net;
+
+	log_set_log_level(osmo_stderr_target, LOGL_DEBUG);
+
+	for (i = 0; i < ARRAY_SIZE(cell_identifier_tests); i++) {
+		struct msgb *msg;
+		fprintf(stderr, "\n%d:\n", i);
+		msg = msgb_from_hex("test_cell_identifier", 1024, cell_identifier_tests[i].msg);
+
+		gl_expect_lac = cell_identifier_tests[i].expect_lac;
+		rc = bsc_handle_udt(msc, msg, msgb_l2len(msg));
+
+		fprintf(stderr, "bsc_handle_udt() returned %d\n", rc);
+		OSMO_ASSERT(rc == cell_identifier_tests[i].expect_rc);
+
+		msgb_free(msg);
+	}
+}
+
+int main(int argc, char **argv)
+{
+	osmo_init_logging(&log_info);
+	log_set_use_color(osmo_stderr_target, 0);
+	log_set_print_timestamp(osmo_stderr_target, 0);
+	log_set_print_filename(osmo_stderr_target, 0);
+	log_set_print_category(osmo_stderr_target, 1);
+
+	test_cell_identifier();
+
+	return 0;
+}
diff --git a/tests/bssap/bssap_test.err b/tests/bssap/bssap_test.err
new file mode 100644
index 000000000..1c432eb08
--- /dev/null
+++ b/tests/bssap/bssap_test.err
@@ -0,0 +1,22 @@
+
+0:
+DMSC Rx MSC UDT: 00 16 52 08 08 59 51 20 69 00 07 43 94 09 04 01 08 44 60 1a 03 05 00 65 
+DMSC Rx MSC UDT BSSMAP PAGING
+DMSC Paging request from MSC IMSI: '515029600703449' TMSI: '0x1084460/17319008' LAC: 0x65
+BSC paging started with LAC 101
+bsc_handle_udt() returned 0
+
+1:
+DMSC Rx MSC UDT: 00 14 52 08 08 59 51 20 69 00 07 43 94 09 04 01 08 44 60 1a 01 06 
+DMSC Rx MSC UDT BSSMAP PAGING
+DMSC Paging request from MSC IMSI: '515029600703449' TMSI: '0x1084460/17319008' LAC: 0xfffe
+BSC paging started on entire BSS (65534)
+bsc_handle_udt() returned 0
+
+2:
+DMSC Rx MSC UDT: 00 19 52 08 08 59 51 20 69 00 07 43 94 09 04 01 08 44 60 1a 06 04 15 f5 49 00 65 
+DMSC Rx MSC UDT BSSMAP PAGING
+DMSC Paging IMSI 515029600703449: unimplemented Cell Identifier List (0x4), paging entire BSS instead (04 15 f5 49 00 65 )
+DMSC Paging request from MSC IMSI: '515029600703449' TMSI: '0x1084460/17319008' LAC: 0xfffe
+BSC paging started on entire BSS (65534)
+bsc_handle_udt() returned 0
diff --git a/tests/bssap/bssap_test.ok b/tests/bssap/bssap_test.ok
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/bssap/bssap_test.ok
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 50f68e12e..13f54e119 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -50,3 +50,10 @@ AT_KEYWORDS([nanobts_omlattr])
 cat $abs_srcdir/nanobts_omlattr/nanobts_omlattr_test.ok > expout
 AT_CHECK([$abs_top_builddir/tests/nanobts_omlattr/nanobts_omlattr_test], [], [expout], [ignore])
 AT_CLEANUP
+
+AT_SETUP([bssap])
+AT_KEYWORDS([bssap])
+cat $abs_srcdir/bssap/bssap_test.ok > expout
+cat $abs_srcdir/bssap/bssap_test.err > experr
+AT_CHECK([$abs_top_builddir/tests/bssap/bssap_test], [], [expout], [experr])
+AT_CLEANUP