| Message ID | 20180105170458.GA19240@byrne.stsp.name |
|---|---|
| State | New |
| Headers | show |
| Series | question about 'lai & lac' paging | expand |
Hi Stefan, On Fri, Jan 05, 2018 at 06:04:59PM +0100, Stefan Sperling wrote: > I am trying to fix issue OS#2754 "Paging on LAI not working". > But the second test fails. It looks as if the MSC expects a response > which is not delivered. I am unsure if the test has wrong expectations > or if osmo-bsc is not sending a required response? It's not a "response", but the BSC converts the (single) BSSMAP PAGING into 0-N (typically 1-N) "RSL PAGING CMD" towards the BTSs. The TTCN-3 tests always simulate the entire surrounding of the program in a fixture. So for the BSC tests, it doesn't just simulate the MSC, but simulates the MSC and BTSs (and soon also the MGW) to be able to control all external interfaces. The cell identifier list included in the BSSMAP paging tells the BSC which BTSs it should start to page. The current code (AFAIR) a) doesn't understand the concept of the list but only looks at the first element? b) doesn't understand a lot of the different address formats (CGI, CI, ...) > Test case TC_paging_imsi_nochan_lai finished. Verdict: fail reason: > Timeout expecting { msg_disc := { msg_group := RSL_MDISC_CCHAN (6), > transparent := false }, msg_type := RSL_MT_PAGING_CMD (21), ies := { { > iei := ?, body := { chan_nr := { u := { ch0 := RSL_CHAN_NR_PCH_AGCH (18) > }, tn := ? } } }, { iei := ?, body := { paging_group := ? } }, { iei := > ?, body := { ms_identity := { len := ?, payload := ? } } }, * } } This means that one one of the BTSs where we expect the RSL PAGING it didn't arrive. > Any hints how to proceed? you can verify the test correctness by monitoring the RSL link and checking which of the BTSs (if any) get the RSL PAGING CMD and which not, and whether that reflects the LAC/CI values configured in osmo-bsc.cfg. Regarding your patch: I think it falls quite a bit short. > switch (cell_ident) { > + case CELL_IDENT_LAI_AND_LAC: > + if (data_length != 6) { > + 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)); > + } > + lac = osmo_load16be(&data[4]); > + break; 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 you must verify if all parameters match. Also, as indicated, the existing code does not traverse the cell identification list yet, does it?
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 > 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? ~N
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 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 }, };
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.
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.
diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c index 45861ccc..9bc59623 100644 --- a/src/osmo-bsc/osmo_bsc_bssap.c +++ b/src/osmo-bsc/osmo_bsc_bssap.c @@ -288,6 +288,15 @@ static int bssmap_handle_paging(struct bsc_msc_data *msc, lac = GSM_LAC_RESERVED_ALL_BTS; switch (cell_ident) { + case CELL_IDENT_LAI_AND_LAC: + if (data_length != 6) { + 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)); + } + lac = osmo_load16be(&data[4]); + break; + case CELL_IDENT_LAC: if (data_length != 3) { LOGP(DMSC, LOGL_ERROR, "Paging IMSI %s: Cell Identifier List for LAC (0x%x)" 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 }, };
I am trying to fix issue OS#2754 "Paging on LAI not working". There are two tests related to this issue: 1) osmo-bsc/tests/bssap/bssap_test 2) TC_paging_imsi_nochan_lai in osmo-ttcn3-hacks/bss The first test is successful with my patch below. But the second test fails. It looks as if the MSC expects a response which is not delivered. I am unsure if the test has wrong expectations or if osmo-bsc is not sending a required response? The test's verdict is: Test case TC_paging_imsi_nochan_lai finished. Verdict: fail reason: Timeout expecting { msg_disc := { msg_group := RSL_MDISC_CCHAN (6), transparent := false }, msg_type := RSL_MT_PAGING_CMD (21), ies := { { iei := ?, body := { chan_nr := { u := { ch0 := RSL_CHAN_NR_PCH_AGCH (18) }, tn := ? } } }, { iei := ?, body := { paging_group := ? } }, { iei := ?, body := { ms_identity := { len := ?, payload := ? } } }, * } } Note that the existing ttcn3 test for the LAC case, which is already supported, and also covered by the bssap_test, fails in the same way: Test case TC_paging_imsi_nochan_lac finished. Verdict: fail reason: Timeout expecting { msg_disc := { msg_group := RSL_MDISC_CCHAN (6), transparent := false }, msg_type := RSL_MT_PAGING_CMD (21), ies := { { iei := ?, body := { chan_nr := { u := { ch0 := RSL_CHAN_NR_PCH_AGCH (18) }, tn := ? } } }, { iei := ?, body := { paging_group := ? } }, { iei := ?, body := { ms_identity := { len := ?, payload := ? } } }, * } } Any hints how to proceed? Thanks, Stefan From 01200d452971cd20c8a0304e180c2d11ff399d3f Mon Sep 17 00:00:00 2001 From: Stefan Sperling <ssperling@sysmocom.de> Date: Fri, 5 Jan 2018 17:22:11 +0100 Subject: [PATCH] Implement support for paging by LAI. This is very similar to the paging by LAC case. We can simply extract the LAC at a different offset. Change-Id: Ic3c62ff0fccea586794ea4b3c275a0685cc9326e Related: OS#2751 --- src/osmo-bsc/osmo_bsc_bssap.c | 9 +++++++++ tests/bssap/bssap_test.c | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-)