question about 'lai & lac' paging

Submitted by Stefan Sperling on Jan. 8, 2018, 11:43 a.m.

Details

Message ID 20180108114322.GA67306@byrne.stsp.name
State New
Series "question about 'lai & lac' paging"
Headers show

Commit Message

Stefan Sperling Jan. 8, 2018, 11:43 a.m. 
On Sun, Jan 07, 2018 at 07:15:07PM +0100, Neels Hofmeyr wrote:
> On Sat, Jan 06, 2018 at 11:20:37AM +0100, Harald Welte wrote:
> > please don't just use magic numbers like 6 + 4 but actually define
> > structs or otherwise parse the data.  Also, a LAI includes MCC+MNC, so
> 
> should probably be using gsm48_decode_lai() in libosmocore/include/osmocom/gsm/gsm48.h

Indeed, thanks for the hint.

> > you must verify if all parameters match.
> 
> since we currently have just a single global MCC+MNC setting, being correct
> means ignoring paging for mismatching MCC+MNC... I guess we want error log for
> that case, since the MSC shouldn't even send us paging for mismatching MCC+MNC?
> 
> > Also, as indicated, the existing code does not traverse the cell
> > identification list yet, does it?
> 
> IIUC receiving a LAI identification for paging is not related to a cell
> identification list?

There is in fact a list of LAIs but our tests send just one element.

The code below is probably better but the test seems to use MCC/MNC values
that don't match the configured network -- perhaps a byte-ordering issue?
When I run ttcn3 test TC_paging_imsi_nochan_lai I now see:
osmo_bsc_bssap.c:365 Not paging IMSI 001010000000009: MCC/MNC in Cell Identifier List (0/10) do not match our network (1/1) 

I haven't yet looked where ttcn3's MCC/MNC values are set. Any hints?


commit f27c6a09f7e96e29b61b313412b27dd0dd67ec90
Author: Stefan Sperling <ssperling@sysmocom.de>
Date:   Fri Jan 5 17:22:11 2018 +0100

    Implement support for paging by LAI.
    
    Also, parse the complete cell identifier list for both LAC and LAI.
    
    Change-Id: Ic3c62ff0fccea586794ea4b3c275a0685cc9326e
    Related: OS#2751

Patch hide | download patch | download mbox 

diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 45861ccc..9225e6dd 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -228,21 +228,52 @@  static int bssmap_handle_reset(struct bsc_msc_data *msc,
 	return 0;
 }
 
+/* Page a subscriber based on TMSI and LAC.
+ * A non-zero return value indicates a fatal out of memory condition. */
+static int
+page_subscriber(struct bsc_msc_data *msc, uint16_t tmsi, uint32_t lac,
+	const char *mi_string, uint8_t chan_needed)
+{
+	struct bsc_subscr *subscr;
+
+	subscr = bsc_subscr_find_or_create_by_imsi(msc->network->bsc_subscribers,
+						   mi_string);
+	if (!subscr) {
+		LOGP(DMSC, LOGL_ERROR, "Failed to allocate a subscriber for %s\n", mi_string);
+		return -1;
+	}
+
+	subscr->lac = lac;
+	subscr->tmsi = tmsi;
+
+	LOGP(DMSC, LOGL_INFO, "Paging request from MSC IMSI: '%s' TMSI: '0x%x/%u' LAC: 0x%x\n", mi_string, tmsi, tmsi, lac);
+	bsc_grace_paging_request(msc->network->bsc_data->rf_ctrl->policy,
+				 subscr, chan_needed, msc);
+
+	/* the paging code has grabbed its own references */
+	bsc_subscr_put(subscr);
+
+	return 0;
+}
+
 /* GSM 08.08 ยง 3.2.1.19 */
 static int bssmap_handle_paging(struct bsc_msc_data *msc,
 				struct msgb *msg, unsigned int payload_length)
 {
-	struct bsc_subscr *subscr;
 	struct tlv_parsed tp;
 	char mi_string[GSM48_MI_SIZE];
 	uint32_t tmsi = GSM_RESERVED_TMSI;
-	unsigned int lac;
+	uint16_t lac, *lacp_be;
+	uint16_t mcc;
+	uint16_t mnc;
 	uint8_t data_length;
+	int remain;
 	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);
+	remain = payload_length - 1;
 
 	if (!TLVP_PRESENT(&tp, GSM0808_IE_IMSI)) {
 		LOGP(DMSC, LOGL_ERROR, "Mandatory IMSI not present.\n");
@@ -251,6 +282,7 @@  static int bssmap_handle_paging(struct bsc_msc_data *msc,
 		LOGP(DMSC, LOGL_ERROR, "Wrong content in the IMSI\n");
 		return -1;
 	}
+	remain -= TLVP_LEN(&tp, GSM0808_IE_IMSI);
 
 	if (!TLVP_PRESENT(&tp, GSM0808_IE_CELL_IDENTIFIER_LIST)) {
 		LOGP(DMSC, LOGL_ERROR, "Mandatory CELL IDENTIFIER LIST not present.\n");
@@ -260,6 +292,12 @@  static int bssmap_handle_paging(struct bsc_msc_data *msc,
 	if (TLVP_PRESENT(&tp, GSM0808_IE_TMSI) &&
 	    TLVP_LEN(&tp, GSM0808_IE_TMSI) == 4) {
 		tmsi = ntohl(tlvp_val32_unal(&tp, GSM0808_IE_TMSI));
+		remain -= TLVP_LEN(&tp, GSM0808_IE_TMSI);
+	}
+
+	if (remain <= 0) {
+		LOGP(DMSC, LOGL_ERROR, "Payload too short.\n");
+		return -1;
 	}
 
 	/*
@@ -280,22 +318,67 @@  static int bssmap_handle_paging(struct bsc_msc_data *msc,
 		LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Zero length Cell Identifier List\n",
 		     mi_string);
 		return -1;
+	} else if (data_length > remain) {
+		LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Bogus Cell Identifier List length\n",
+		     mi_string);
+		return -1;
+	}
+	remain = data_length; /* ignore payload data beyond data_length */
+
+	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;
+
+	if (TLVP_PRESENT(&tp, GSM0808_IE_EMLPP_PRIORITY)) {
+		LOGP(DMSC, LOGL_ERROR, "eMLPP is not handled\n");
 	}
 
 	cell_ident = data[0] & 0xf;
+	remain -= 1; /* cell ident consumed */
 
 	/* Default fallback: page entire BSS */
 	lac = GSM_LAC_RESERVED_ALL_BTS;
 
 	switch (cell_ident) {
+	case CELL_IDENT_LAI_AND_LAC: {
+		struct gsm48_loc_area_id lai;
+		int i = 0;
+		while (remain >= sizeof(lai)) {
+			/* Parse and decode 5-byte LAI list element (see TS 08.08 3.2.2.27).
+			 * Copy data to stack to prevent unaligned access in gsm48_decode_lai(). */
+			lai.digits[0] = data[1 + i * sizeof(lai)];
+			lai.digits[1] = data[2 + i * sizeof(lai)];
+			lai.digits[2] = data[3 + i * sizeof(lai)];
+			memcpy(&lai.lac, &data[4 + i * sizeof(lai)], sizeof(lai.lac)); /* don't byte-swap yet */
+			if (gsm48_decode_lai(&lai, &mcc, &mnc, &lac) != 0) {
+				LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Invalid LAI in Cell Identifier List "
+				     "for BSS (0x%x), paging entire BSS anyway (%s)\n",
+				     mi_string, CELL_IDENT_BSS, osmo_hexdump(data, data_length));
+				lac = GSM_LAC_RESERVED_ALL_BTS;
+				break;
+			}
+			if (mcc == msc->network->country_code && mnc == msc->network->network_code) {
+				    if (page_subscriber(msc, tmsi, lac, mi_string, chan_needed) != 0)
+						break;
+			} else
+				LOGP(DMSC, LOGL_DEBUG, "Not paging IMSI %s: MCC/MNC in Cell Identifier List "
+				     "(%d/%d) do not match our network (%d/%d)\n", mi_string, mcc, mnc,
+				     msc->network->country_code, msc->network->network_code);
+
+			remain -= sizeof(lai);
+			i++;
+		}
+		break;
+	}
+
 	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;
+		lacp_be = (uint16_t *)(&data[1]);
+		while (remain >= sizeof(*lacp_be)) {
+			lac = osmo_load16be(lacp_be);
+			if (page_subscriber(msc, tmsi, lac, mi_string, chan_needed) != 0)
+				break;
+			remain -= sizeof(*lacp_be);
+			lacp_be++;
 		}
-		lac = osmo_load16be(&data[1]);
 		break;
 
 	case CELL_IDENT_BSS:
@@ -304,39 +387,19 @@  static int bssmap_handle_paging(struct bsc_msc_data *msc,
 			     " has invalid length: %u, paging entire BSS anyway (%s)\n",
 			     mi_string, CELL_IDENT_BSS, data_length, osmo_hexdump(data, data_length));
 		}
+		if (page_subscriber(msc, tmsi, GSM_LAC_RESERVED_ALL_BTS, mi_string, chan_needed) != 0)
+			break;
 		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));
+		if (page_subscriber(msc, tmsi, GSM_LAC_RESERVED_ALL_BTS, mi_string, chan_needed) != 0)
+			break;
 		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;
-
-	if (TLVP_PRESENT(&tp, GSM0808_IE_EMLPP_PRIORITY)) {
-		LOGP(DMSC, LOGL_ERROR, "eMLPP is not handled\n");
-	}
-
-	subscr = bsc_subscr_find_or_create_by_imsi(msc->network->bsc_subscribers,
-						   mi_string);
-	if (!subscr) {
-		LOGP(DMSC, LOGL_ERROR, "Failed to allocate a subscriber for %s\n", mi_string);
-		return -1;
-	}
-
-	subscr->lac = lac;
-	subscr->tmsi = tmsi;
-
-	LOGP(DMSC, LOGL_INFO, "Paging request from MSC IMSI: '%s' TMSI: '0x%x/%u' LAC: 0x%x\n", mi_string, tmsi, tmsi, lac);
-	bsc_grace_paging_request(msc->network->bsc_data->rf_ctrl->policy,
-				 subscr, chan_needed, msc);
-
-	/* the paging code has grabbed its own references */
-	bsc_subscr_put(subscr);
-
 	return 0;
 }
 
diff --git a/tests/bssap/bssap_test.c b/tests/bssap/bssap_test.c
index 579cae23..2fa2b1fe 100644
--- a/tests/bssap/bssap_test.c
+++ b/tests/bssap/bssap_test.c
@@ -71,7 +71,7 @@  struct {
 	{
 		"001952080859512069000743940904010844601a060415f5490065",
 		/*                                         ^^^^^^^^^^^^ Cell Identifier List: LAI */
-		GSM_LAC_RESERVED_ALL_BTS, 0
+		0x65, 0
 	},
 };

Comments

Harald Welte Jan. 8, 2018, 12:25 p.m. 
Hi Stefan,

On Mon, Jan 08, 2018 at 12:43:23PM +0100, Stefan Sperling wrote:
> The code below is probably better but the test seems to use MCC/MNC values
> that don't match the configured network -- perhaps a byte-ordering issue?
> When I run ttcn3 test TC_paging_imsi_nochan_lai I now see:
> osmo_bsc_bssap.c:365 Not paging IMSI 001010000000009: MCC/MNC in Cell Identifier List (0/10) do not match our network (1/1) 

what do the protocol traces say? If you look with wireshark at both BSSMAP and RSL
it should be pretty easy to see if wireshark also thinks there's some wrong encoding.

> I haven't yet looked where ttcn3's MCC/MNC values are set. Any hints?

See "private function f_enc_mcc_mnc(GsmMcc mcc, GsmMnc mnc)" in library/BSSMAP_Templates.ttcn
and its callers. It may very well be buggy.
Stefan Sperling Jan. 8, 2018, 2:30 p.m. 
On Mon, Jan 08, 2018 at 01:25:16PM +0100, Harald Welte wrote:
> Hi Stefan,
> 
> On Mon, Jan 08, 2018 at 12:43:23PM +0100, Stefan Sperling wrote:
> > The code below is probably better but the test seems to use MCC/MNC values
> > that don't match the configured network -- perhaps a byte-ordering issue?
> > When I run ttcn3 test TC_paging_imsi_nochan_lai I now see:
> > osmo_bsc_bssap.c:365 Not paging IMSI 001010000000009: MCC/MNC in Cell Identifier List (0/10) do not match our network (1/1) 
> 
> what do the protocol traces say? If you look with wireshark at both BSSMAP and RSL
> it should be pretty easy to see if wireshark also thinks there's some wrong encoding.

Wireshark shows "MCC 0" and "MNC 010" in the BSSMAP paging frame.

> > I haven't yet looked where ttcn3's MCC/MNC values are set. Any hints?
> 
> See "private function f_enc_mcc_mnc(GsmMcc mcc, GsmMnc mnc)" in library/BSSMAP_Templates.ttcn
> and its callers. It may very well be buggy.

With Daniel's help I could get the right MCC/MNC in wireshark.
See https://gerrit.osmocom.org/#/c/5682

I now see paging happening for both BTS 0 and 2 but the ttcn3
TC_paging_imsi_nochan_lai test is still unhappy (same error).
I suppose this means either it does not receive the RSL paging message
(which shows up in wireshark) or the message template doesn't match?
I've been trying to spot a problem in the template but so far no luck.