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(a)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
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
},
};