Currently foreign TLLI are sometimes mapped to local TLLI in the hope that they will match. This seems to sometimes introduce inconsisties, possibly leading to a failing assertion in _bssgp_tx_dl_ud.
This mapping should probably reduce the allocation of additional LLME during routing area changes.
This commit removes tlli_foreign2local.
Sponsored-by: On-Waves ehf --- openbsc/src/gprs/gprs_llc.c | 24 ++---------------------- openbsc/tests/sgsn/sgsn_test.c | 5 ----- 2 files changed, 2 insertions(+), 27 deletions(-)
diff --git a/openbsc/src/gprs/gprs_llc.c b/openbsc/src/gprs/gprs_llc.c index 936354a..06ccd4c 100644 --- a/openbsc/src/gprs/gprs_llc.c +++ b/openbsc/src/gprs/gprs_llc.c @@ -39,20 +39,6 @@ static struct gprs_llc_llme *llme_alloc(uint32_t tlli);
/* If the TLLI is foreign, return its local version */ -static inline uint32_t tlli_foreign2local(uint32_t tlli) -{ - uint32_t new_tlli; - - if (gprs_tlli_type(tlli) == TLLI_FOREIGN) { - new_tlli = tlli | 0x40000000; - LOGP(DLLC, LOGL_NOTICE, "TLLI 0x%08x is foreign, converting to " - "local TLLI 0x%08x\n", tlli, new_tlli); - } else - new_tlli = tlli; - - return new_tlli; -} - /* Entry function from upper level (LLC), asking us to transmit a BSSGP PDU * to a remote MS (identified by TLLI) at a BTS identified by its BVCI and NSEI */ static int _bssgp_tx_dl_ud(struct msgb *msg, struct sgsn_mm_ctx *mmctx) @@ -72,9 +58,7 @@ static int _bssgp_tx_dl_ud(struct msgb *msg, struct sgsn_mm_ctx *mmctx)
/* make sure we only send it to the right llme */ OSMO_ASSERT(msgb_tlli(msg) == mmctx->llme->tlli - || msgb_tlli(msg) == mmctx->llme->old_tlli - || tlli_foreign2local(msgb_tlli(msg)) == mmctx->llme->tlli - || tlli_foreign2local(msgb_tlli(msg)) == mmctx->llme->old_tlli); + || msgb_tlli(msg) == mmctx->llme->old_tlli); } memcpy(&dup.qos_profile, qos_profile_default, sizeof(qos_profile_default)); @@ -175,10 +159,6 @@ struct gprs_llc_lle *gprs_lle_get_or_create(const uint32_t tlli, uint8_t sapi) if (lle) return lle;
- lle = lle_by_tlli_sapi(tlli_foreign2local(tlli), sapi); - if (lle) - return lle; - LOGP(DLLC, LOGL_NOTICE, "LLC: unknown TLLI 0x%08x, " "creating LLME on the fly\n", tlli); llme = llme_alloc(tlli); @@ -204,7 +184,7 @@ static struct gprs_llc_lle *lle_for_rx_by_tlli_sapi(const uint32_t tlli,
/* Maybe it is a routing area update but we already know this sapi? */ if (gprs_tlli_type(tlli) == TLLI_FOREIGN) { - lle = lle_by_tlli_sapi(tlli_foreign2local(tlli), sapi); + lle = lle_by_tlli_sapi(tlli, sapi); if (lle) { LOGP(DLLC, LOGL_NOTICE, "LLC RX: Found a local entry for TLLI 0x%08x\n", diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c index 859223f..01e94c7 100644 --- a/openbsc/tests/sgsn/sgsn_test.c +++ b/openbsc/tests/sgsn/sgsn_test.c @@ -203,11 +203,9 @@ static void test_llme(void) { struct gprs_llc_lle *lle, *lle_copy; uint32_t local_tlli; - uint32_t foreign_tlli;
printf("Testing LLME allocations\n"); local_tlli = gprs_tmsi2tlli(0x234, TLLI_LOCAL); - foreign_tlli = gprs_tmsi2tlli(0x234, TLLI_FOREIGN);
/* initial state */ OSMO_ASSERT(count(gprs_llme_list()) == 0); @@ -221,9 +219,6 @@ static void test_llme(void) lle_copy = gprs_lle_get_or_create(local_tlli, 3); OSMO_ASSERT(lle == lle_copy); OSMO_ASSERT(count(gprs_llme_list()) == 1); - lle_copy = gprs_lle_get_or_create(foreign_tlli, 3); - OSMO_ASSERT(lle == lle_copy); - OSMO_ASSERT(count(gprs_llme_list()) == 1);
/* unassign which should delete it*/ gprs_llgmm_assign(lle->llme, lle->llme->tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
The BSSGP cell identifier is used to get the RA for the TLLI lookup. The send_0408_message function used in the tests does not set this, so the RA identifier is always 0-0-0-0.
This commit adds a parameters to pass the RAID and adds missing dummy RAIDs.
Note that the CI can still not be set and thus is always 0.
Sponsored-by: On-Waves ehf --- openbsc/tests/sgsn/sgsn_test.c | 55 +++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c index 01e94c7..6d7ba38 100644 --- a/openbsc/tests/sgsn/sgsn_test.c +++ b/openbsc/tests/sgsn/sgsn_test.c @@ -186,6 +186,7 @@ static struct sgsn_mm_ctx *alloc_mm_ctx(uint32_t tlli, struct gprs_ra_id *raid) }
static void send_0408_message(struct gprs_llc_llme *llme, uint32_t tlli, + const struct gprs_ra_id *bssgp_raid, const uint8_t *data, size_t data_len) { struct msgb *msg; @@ -195,6 +196,7 @@ static void send_0408_message(struct gprs_llc_llme *llme, uint32_t tlli,
msg = create_msg(data, data_len); msgb_tlli(msg) = tlli; + bssgp_create_cell_id(msgb_bcid(msg), bssgp_raid, 0); gsm0408_gprs_rcvmsg(msg, llme); msgb_free(msg); } @@ -728,7 +730,7 @@ static void test_gmm_detach(void) ctx = alloc_mm_ctx(local_tlli, &raid);
/* inject the detach */ - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, detach_req, ARRAY_SIZE(detach_req));
/* verify that a single message (hopefully the Detach Accept) has been @@ -769,7 +771,7 @@ static void test_gmm_detach_power_off(void) ctx = alloc_mm_ctx(local_tlli, &raid);
/* inject the detach */ - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, detach_req, ARRAY_SIZE(detach_req));
/* verify that no message (and therefore no Detach Accept) has been @@ -789,6 +791,7 @@ static void test_gmm_detach_power_off(void) */ static void test_gmm_detach_no_mmctx(void) { + struct gprs_ra_id raid = { 0, }; struct gprs_llc_lle *lle; uint32_t local_tlli;
@@ -809,7 +812,7 @@ static void test_gmm_detach_no_mmctx(void) OSMO_ASSERT(count(gprs_llme_list()) == 1);
/* inject the detach */ - send_0408_message(lle->llme, local_tlli, + send_0408_message(lle->llme, local_tlli, &raid, detach_req, ARRAY_SIZE(detach_req));
/* verify that the LLME is gone */ @@ -824,6 +827,7 @@ static void test_gmm_detach_no_mmctx(void) */ static void test_gmm_detach_accept_unexpected(void) { + struct gprs_ra_id raid = { 0, }; struct gprs_llc_lle *lle; uint32_t local_tlli;
@@ -841,7 +845,7 @@ static void test_gmm_detach_accept_unexpected(void) lle = gprs_lle_get_or_create(local_tlli, 3);
/* inject the detach */ - send_0408_message(lle->llme, local_tlli, + send_0408_message(lle->llme, local_tlli, &raid, detach_acc, ARRAY_SIZE(detach_acc));
/* verify that no message (and therefore no Status or XID reset) has been @@ -859,6 +863,7 @@ static void test_gmm_detach_accept_unexpected(void) */ static void test_gmm_status_no_mmctx(void) { + struct gprs_ra_id raid = { 0, }; struct gprs_llc_lle *lle; uint32_t local_tlli;
@@ -877,7 +882,7 @@ static void test_gmm_status_no_mmctx(void) OSMO_ASSERT(count(gprs_llme_list()) == 1);
/* inject the detach */ - send_0408_message(lle->llme, local_tlli, + send_0408_message(lle->llme, local_tlli, &raid, gmm_status, ARRAY_SIZE(gmm_status));
/* verify that no message has been sent by the SGSN */ @@ -952,7 +957,7 @@ static void test_gmm_attach(int retry) OSMO_ASSERT(count(gprs_llme_list()) == 1);
/* inject the attach request */ - send_0408_message(lle->llme, foreign_tlli, + send_0408_message(lle->llme, foreign_tlli, &raid, attach_req, ARRAY_SIZE(attach_req));
ctx = sgsn_mm_ctx_by_tlli(foreign_tlli, &raid); @@ -963,14 +968,14 @@ static void test_gmm_attach(int retry) OSMO_ASSERT(sgsn_tx_counter == 1);
/* inject the identity response (IMEI) */ - send_0408_message(ctx->llme, foreign_tlli, + send_0408_message(ctx->llme, foreign_tlli, &raid, ident_resp_imei, ARRAY_SIZE(ident_resp_imei));
/* we expect an identity request (IMSI) */ OSMO_ASSERT(sgsn_tx_counter == 1);
/* inject the identity response (IMSI) */ - send_0408_message(ctx->llme, foreign_tlli, + send_0408_message(ctx->llme, foreign_tlli, &raid, ident_resp_imsi, ARRAY_SIZE(ident_resp_imsi));
/* check that the MM context has not been removed due to a failed @@ -984,7 +989,7 @@ retry_attach_req: if (retry && sgsn_tx_counter == 0) { fprintf(stderr, "Retrying attach request\n"); /* re-inject the attach request */ - send_0408_message(lle->llme, foreign_tlli, + send_0408_message(lle->llme, foreign_tlli, &raid, attach_req, ARRAY_SIZE(attach_req)); }
@@ -992,7 +997,7 @@ retry_attach_req: /* we got an auth & ciph request */
/* inject the auth & ciph response */ - send_0408_message(ctx->llme, foreign_tlli, + send_0408_message(ctx->llme, foreign_tlli, &raid, auth_ciph_resp, ARRAY_SIZE(auth_ciph_resp));
/* check that the MM context has not been removed due to a @@ -1014,7 +1019,7 @@ retry_attach_req: local_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL);
/* inject the attach complete */ - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, attach_compl, ARRAY_SIZE(attach_compl));
OSMO_ASSERT(ctx->mm_state == GMM_REGISTERED_NORMAL); @@ -1023,7 +1028,7 @@ retry_attach_req: OSMO_ASSERT(sgsn_tx_counter == 0);
/* inject the detach */ - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, detach_req, ARRAY_SIZE(detach_req));
/* verify that things are gone */ @@ -1470,7 +1475,7 @@ static void test_gmm_reject(void) OSMO_ASSERT(count(gprs_llme_list()) == 1);
/* Inject the Request message */ - send_0408_message(lle->llme, foreign_tlli, + send_0408_message(lle->llme, foreign_tlli, &raid, test->msg, test->msg_len);
/* We expect a Reject message */ @@ -1540,7 +1545,7 @@ static void test_gmm_cancel(void) OSMO_ASSERT(count(gprs_llme_list()) == 1);
/* inject the attach request */ - send_0408_message(lle->llme, foreign_tlli, + send_0408_message(lle->llme, foreign_tlli, &raid, attach_req, ARRAY_SIZE(attach_req));
ctx = sgsn_mm_ctx_by_tlli(foreign_tlli, &raid); @@ -1551,14 +1556,14 @@ static void test_gmm_cancel(void) OSMO_ASSERT(sgsn_tx_counter == 1);
/* inject the identity response (IMEI) */ - send_0408_message(ctx->llme, foreign_tlli, + send_0408_message(ctx->llme, foreign_tlli, &raid, ident_resp_imei, ARRAY_SIZE(ident_resp_imei));
/* we expect an identity request (IMSI) */ OSMO_ASSERT(sgsn_tx_counter == 1);
/* inject the identity response (IMSI) */ - send_0408_message(ctx->llme, foreign_tlli, + send_0408_message(ctx->llme, foreign_tlli, &raid, ident_resp_imsi, ARRAY_SIZE(ident_resp_imsi));
/* check that the MM context has not been removed due to a failed @@ -1576,7 +1581,7 @@ static void test_gmm_cancel(void) local_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL);
/* inject the attach complete */ - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, attach_compl, ARRAY_SIZE(attach_compl));
OSMO_ASSERT(ctx->mm_state == GMM_REGISTERED_NORMAL); @@ -1688,7 +1693,7 @@ static void test_gmm_ptmsi_allocation(void) OSMO_ASSERT(count(gprs_llme_list()) == 1);
/* inject the attach request */ - send_0408_message(lle->llme, foreign_tlli, + send_0408_message(lle->llme, foreign_tlli, &raid, attach_req, ARRAY_SIZE(attach_req));
ctx = sgsn_mm_ctx_by_tlli(foreign_tlli, &raid); @@ -1703,7 +1708,7 @@ static void test_gmm_ptmsi_allocation(void) OSMO_ASSERT(sgsn_tx_counter == 1);
/* inject the identity response (IMEI) */ - send_0408_message(ctx->llme, foreign_tlli, + send_0408_message(ctx->llme, foreign_tlli, &raid, ident_resp_imei, ARRAY_SIZE(ident_resp_imei));
/* check that the MM context has not been removed due to a failed @@ -1719,7 +1724,7 @@ static void test_gmm_ptmsi_allocation(void) OSMO_ASSERT(received_ptmsi == ptmsi1);
/* we ignore this and send the attach again */ - send_0408_message(lle->llme, foreign_tlli, + send_0408_message(lle->llme, foreign_tlli, &raid, attach_req, ARRAY_SIZE(attach_req));
/* the allocated P-TMSI should be the same */ @@ -1736,7 +1741,7 @@ static void test_gmm_ptmsi_allocation(void)
/* inject the attach complete */ local_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL); - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, attach_compl, ARRAY_SIZE(attach_compl));
/* we don't expect a response */ @@ -1749,7 +1754,7 @@ static void test_gmm_ptmsi_allocation(void) printf(" - Repeated RA Update Request\n");
/* inject the RA update request */ - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, ra_upd_req, ARRAY_SIZE(ra_upd_req));
/* we expect an RA update accept */ @@ -1762,7 +1767,7 @@ static void test_gmm_ptmsi_allocation(void) ptmsi2 = ctx->p_tmsi;
/* repeat the RA update request */ - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, ra_upd_req, ARRAY_SIZE(ra_upd_req));
/* we expect an RA update accept */ @@ -1776,7 +1781,7 @@ static void test_gmm_ptmsi_allocation(void)
/* inject the RA update complete */ local_tlli = gprs_tmsi2tlli(ptmsi2, TLLI_LOCAL); - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, ra_upd_complete, ARRAY_SIZE(ra_upd_complete));
/* we don't expect a response */ @@ -1787,7 +1792,7 @@ static void test_gmm_ptmsi_allocation(void) OSMO_ASSERT(ctx->p_tmsi == ptmsi2);
/* inject the detach */ - send_0408_message(ctx->llme, local_tlli, + send_0408_message(ctx->llme, local_tlli, &raid, detach_req, ARRAY_SIZE(detach_req));
/* verify that things are gone */
The function is moved to gprs_utils.c, renamed, and made non-static to be usable in other modules, too.
Sponsored-by: On-Waves ehf --- openbsc/include/openbsc/gprs_utils.h | 2 ++ openbsc/src/gprs/gprs_sgsn.c | 11 ++--------- openbsc/src/gprs/gprs_utils.c | 7 +++++++ 3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/openbsc/include/openbsc/gprs_utils.h b/openbsc/include/openbsc/gprs_utils.h index 6880e05..474eb45 100644 --- a/openbsc/include/openbsc/gprs_utils.h +++ b/openbsc/include/openbsc/gprs_utils.h @@ -25,6 +25,7 @@ #include <sys/types.h>
struct msgb; +struct gprs_ra_id;
struct msgb *gprs_msgb_copy(const struct msgb *msg, const char *name); int gprs_msgb_resize_area(struct msgb *msg, uint8_t *area, @@ -52,3 +53,4 @@ int gprs_match_tlv(uint8_t **data, size_t *data_len, int gprs_shift_lv(uint8_t **data, size_t *data_len, uint8_t **value, size_t *value_len);
+int gprs_ra_id_equals(const struct gprs_ra_id *id1, const struct gprs_ra_id *id2); diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c index c4dc9d7..8f8bdc6 100644 --- a/openbsc/src/gprs/gprs_sgsn.c +++ b/openbsc/src/gprs/gprs_sgsn.c @@ -90,13 +90,6 @@ static const struct rate_ctr_group_desc pdpctx_ctrg_desc = { .class_id = OSMO_STATS_CLASS_SUBSCRIBER, };
-static int ra_id_equals(const struct gprs_ra_id *id1, - const struct gprs_ra_id *id2) -{ - return (id1->mcc == id2->mcc && id1->mnc == id2->mnc && - id1->lac == id2->lac && id1->rac == id2->rac); -} - /* See 03.02 Chapter 2.6 */ static inline uint32_t tlli_foreign(uint32_t tlli) { @@ -112,7 +105,7 @@ struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli(uint32_t tlli,
llist_for_each_entry(ctx, &sgsn_mm_ctxts, list) { if (tlli == ctx->tlli && - ra_id_equals(raid, &ctx->ra)) + gprs_ra_id_equals(raid, &ctx->ra)) return ctx; }
@@ -130,7 +123,7 @@ struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli(uint32_t tlli, case TLLI_FOREIGN: llist_for_each_entry(ctx, &sgsn_mm_ctxts, list) { if (tlli == tlli_foreign(ctx->tlli) && - ra_id_equals(raid, &ctx->ra)) + gprs_ra_id_equals(raid, &ctx->ra)) return ctx; } break; diff --git a/openbsc/src/gprs/gprs_utils.c b/openbsc/src/gprs/gprs_utils.c index ad479db..895a033 100644 --- a/openbsc/src/gprs/gprs_utils.c +++ b/openbsc/src/gprs/gprs_utils.c @@ -26,6 +26,7 @@ #include <osmocom/gprs/gprs_ns.h>
#include <osmocom/gsm/protocol/gsm_04_08.h> +#include <osmocom/gsm/gsm48.h>
#include <string.h>
@@ -399,3 +400,9 @@ fail: return -1; }
+int gprs_ra_id_equals(const struct gprs_ra_id *id1, + const struct gprs_ra_id *id2) +{ + return (id1->mcc == id2->mcc && id1->mnc == id2->mnc && + id1->lac == id2->lac && id1->rac == id2->rac); +}
Currently the code also matches the TLLI against LOCAL and FOREIGN mappings of the P-TMSI, thus eventually finding MM contexts not consistent with the TLLI (both tlli and tlli_new differ). On the other hand, tlli_new is not checked at all.
This commit changes the function to only look at mmctx->tlli, mmctx->tlli_new, and the routing area.
Sponsored-by: On-Waves ehf --- openbsc/src/gprs/gprs_sgsn.c | 31 +------------------------------ openbsc/tests/sgsn/sgsn_test.c | 2 +- 2 files changed, 2 insertions(+), 31 deletions(-)
diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c index 8f8bdc6..f71066d 100644 --- a/openbsc/src/gprs/gprs_sgsn.c +++ b/openbsc/src/gprs/gprs_sgsn.c @@ -90,47 +90,18 @@ static const struct rate_ctr_group_desc pdpctx_ctrg_desc = { .class_id = OSMO_STATS_CLASS_SUBSCRIBER, };
-/* See 03.02 Chapter 2.6 */ -static inline uint32_t tlli_foreign(uint32_t tlli) -{ - return ((tlli | 0x80000000) & ~0x40000000); -} - /* look-up a SGSN MM context based on TLLI + RAI */ struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli(uint32_t tlli, const struct gprs_ra_id *raid) { struct sgsn_mm_ctx *ctx; - int tlli_type;
llist_for_each_entry(ctx, &sgsn_mm_ctxts, list) { - if (tlli == ctx->tlli && + if ((tlli == ctx->tlli || tlli == ctx->tlli_new) && gprs_ra_id_equals(raid, &ctx->ra)) return ctx; }
- tlli_type = gprs_tlli_type(tlli); - switch (tlli_type) { - case TLLI_LOCAL: - llist_for_each_entry(ctx, &sgsn_mm_ctxts, list) { - if ((ctx->p_tmsi | 0xC0000000) == tlli || - (ctx->p_tmsi_old && (ctx->p_tmsi_old | 0xC0000000) == tlli)) { - ctx->tlli = tlli; - return ctx; - } - } - break; - case TLLI_FOREIGN: - llist_for_each_entry(ctx, &sgsn_mm_ctxts, list) { - if (tlli == tlli_foreign(ctx->tlli) && - gprs_ra_id_equals(raid, &ctx->ra)) - return ctx; - } - break; - default: - break; - } - return NULL; }
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c index 6d7ba38..2098972 100644 --- a/openbsc/tests/sgsn/sgsn_test.c +++ b/openbsc/tests/sgsn/sgsn_test.c @@ -1607,7 +1607,7 @@ static void test_gmm_cancel(void) */ static void test_gmm_ptmsi_allocation(void) { - struct gprs_ra_id raid = { 0, }; + struct gprs_ra_id raid = {332, 112, 16464, 96}; struct sgsn_mm_ctx *ctx = NULL; struct sgsn_mm_ctx *ictx; uint32_t foreign_tlli;
Currently the MM context is just overwritten by a call to sgsn_mm_ctx_by_tlli(msgb_tlli(msg), &old_ra_id) even if it has already been found by using the BSSGP info. With the changes made to sgsn_mm_ctx_by_tlli this will never find a MM context if the routing area has changed. If the routing area has not changed, the mmctx has already been found if it exists.
This commit splits searching for an MM context (if it hasn't been found already) from checking, whether a found one can really be used. The actual search is removed, so that the MS will be forced to restart the attach procedure, which is less efficient but safe.
Sponsored-by: On-Waves ehf --- openbsc/src/gprs/gprs_gmm.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c index 115e898..6e7e5f1 100644 --- a/openbsc/src/gprs/gprs_gmm.c +++ b/openbsc/src/gprs/gprs_gmm.c @@ -1164,9 +1164,21 @@ static int gsm48_rx_gmm_ra_upd_req(struct sgsn_mm_ctx *mmctx, struct msgb *msg, break; }
- /* Look-up the MM context based on old RA-ID and TLLI */ - mmctx = sgsn_mm_ctx_by_tlli(msgb_tlli(msg), &old_ra_id); - if (!mmctx || mmctx->mm_state == GMM_DEREGISTERED) { + if (!mmctx) { + /* BSSGP doesn't give us an mmctx */ + + /* TODO: Check if there is an MM CTX with old_ra_id and + * the P-TMSI (if given, reguired for UMTS) or as last resort + * if the TLLI matches foreign_tlli (P-TMSI). Note that this + * is an optimization to avoid the RA reject (impl detached) + * below, which will cause a new attach cycle. */ + } + + if (!mmctx || !gprs_ra_id_equals(&mmctx->ra, &old_ra_id) || + mmctx->mm_state == GMM_DEREGISTERED) + { + /* We cannot use the mmctx */ + /* send a XID reset to re-set all LLC sequence numbers * in the MS */ LOGMMCTXP(LOGL_NOTICE, mmctx, "LLC XID RESET\n");
This test add different cases of routing area changes.
Sponsored-by: On-Waves ehf --- openbsc/tests/sgsn/sgsn_test.c | 347 ++++++++++++++++++++++++++++++++++++++++ openbsc/tests/sgsn/sgsn_test.ok | 8 + 2 files changed, 355 insertions(+)
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c index 2098972..d27b755 100644 --- a/openbsc/tests/sgsn/sgsn_test.c +++ b/openbsc/tests/sgsn/sgsn_test.c @@ -1805,6 +1805,352 @@ static void test_gmm_ptmsi_allocation(void) cleanup_test(); }
+/* + * Test changing of routing areas + */ +static void test_gmm_routing_areas(void) +{ + struct gprs_ra_id raid1 = {332, 112, 16464, 96}; + struct gprs_ra_id raid2 = {332, 112, 16464, 97}; + struct sgsn_mm_ctx *ctx = NULL; + struct sgsn_mm_ctx *ictx; + uint32_t ptmsi1; + uint32_t received_ptmsi; + uint32_t ms_tlli = 0; + struct gprs_llc_lle *lle; + const enum sgsn_auth_policy saved_auth_policy = sgsn->cfg.auth_policy; + + /* DTAP - Attach Request (IMSI 12131415161718) */ + static const unsigned char attach_req[] = { + 0x08, 0x01, 0x02, 0xf5, 0xe0, 0x21, 0x08, 0x02, + 0x08, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x11, 0x22, 0x33, 0x40, 0x50, 0x60, 0x19, + 0x18, 0xb3, 0x43, 0x2b, 0x25, 0x96, 0x62, 0x00, + 0x60, 0x80, 0x9a, 0xc2, 0xc6, 0x62, 0x00, 0x60, + 0x80, 0xba, 0xc8, 0xc6, 0x62, 0x00, 0x60, 0x80, + 0x00, + }; + + /* DTAP - Attach Request (IMSI 12131415161718) (RA 2) */ + static const unsigned char attach_req2[] = { + 0x08, 0x01, 0x02, 0xf5, 0xe0, 0x21, 0x08, 0x02, + 0x08, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x11, 0x22, 0x33, 0x40, 0x50, 0x61, 0x19, + 0x18, 0xb3, 0x43, 0x2b, 0x25, 0x96, 0x62, 0x00, + 0x60, 0x80, 0x9a, 0xc2, 0xc6, 0x62, 0x00, 0x60, + 0x80, 0xba, 0xc8, 0xc6, 0x62, 0x00, 0x60, 0x80, + 0x00, + }; + + /* DTAP - Identity Response IMEI */ + static const unsigned char ident_resp_imei[] = { + 0x08, 0x16, 0x08, 0x9a, 0x78, 0x56, 0x34, 0x12, 0x90, 0x78, + 0x56 + }; + + /* DTAP - Attach Complete */ + static const unsigned char attach_compl[] = { + 0x08, 0x03 + }; + + /* DTAP - Routing Area Update Request (coming from RA 1) */ + static const unsigned char ra_upd_req1[] = { + 0x08, 0x08, 0x10, 0x11, 0x22, 0x33, 0x40, 0x50, + 0x60, 0x1d, 0x19, 0x13, 0x42, 0x33, 0x57, 0x2b, + 0xf7, 0xc8, 0x48, 0x02, 0x13, 0x48, 0x50, 0xc8, + 0x48, 0x02, 0x14, 0x48, 0x50, 0xc8, 0x48, 0x02, + 0x17, 0x49, 0x10, 0xc8, 0x48, 0x02, 0x00, 0x19, + 0x8b, 0xb2, 0x92, 0x17, 0x16, 0x27, 0x07, 0x04, + 0x31, 0x02, 0xe5, 0xe0, 0x32, 0x02, 0x20, 0x00 + }; + + /* DTAP - Routing Area Update Request (coming from RA 2) */ + static const unsigned char ra_upd_req2[] = { + 0x08, 0x08, 0x10, 0x11, 0x22, 0x33, 0x40, 0x50, + 0x61, 0x1d, 0x19, 0x13, 0x42, 0x33, 0x57, 0x2b, + 0xf7, 0xc8, 0x48, 0x02, 0x13, 0x48, 0x50, 0xc8, + 0x48, 0x02, 0x14, 0x48, 0x50, 0xc8, 0x48, 0x02, + 0x17, 0x49, 0x10, 0xc8, 0x48, 0x02, 0x00, 0x19, + 0x8b, 0xb2, 0x92, 0x17, 0x16, 0x27, 0x07, 0x04, + 0x31, 0x02, 0xe5, 0xe0, 0x32, 0x02, 0x20, 0x00 + }; + + /* DTAP - Routing Area Update Request (coming from RA other) */ + /* raid_other = {443, 223, 16464, 98}; */ + static const unsigned char ra_upd_req_other[] = { + 0x08, 0x08, 0x10, 0x22, 0x33, 0x44, 0x40, 0x50, + 0x62, 0x1d, 0x19, 0x13, 0x42, 0x33, 0x57, 0x2b, + 0xf7, 0xc8, 0x48, 0x02, 0x13, 0x48, 0x50, 0xc8, + 0x48, 0x02, 0x14, 0x48, 0x50, 0xc8, 0x48, 0x02, + 0x17, 0x49, 0x10, 0xc8, 0x48, 0x02, 0x00, 0x19, + 0x8b, 0xb2, 0x92, 0x17, 0x16, 0x27, 0x07, 0x04, + 0x31, 0x02, 0xe5, 0xe0, 0x32, 0x02, 0x20, 0x00 + }; + + /* DTAP - Routing Area Update Complete */ + static const unsigned char ra_upd_complete[] = { + 0x08, 0x0a + }; + + /* DTAP - Detach Request (MO) */ + /* normal detach, power_off = 1 */ + static const unsigned char detach_req[] = { + 0x08, 0x05, 0x09, 0x18, 0x05, 0xf4, 0xef, 0xe2, + 0xb7, 0x00, 0x19, 0x03, 0xb9, 0x97, 0xcb + }; + + sgsn->cfg.auth_policy = SGSN_AUTH_POLICY_OPEN; + + printf("Testing routing area changes\n"); + + /* reset the PRNG used by sgsn_alloc_ptmsi */ + srand(1); + + ptmsi1 = GSM_RESERVED_TMSI; + + printf(" - Attach Request (RA 1)\n"); + + ms_tlli = gprs_tmsi2tlli(0x00000023, TLLI_RANDOM); + + /* Create a LLE/LLME */ + OSMO_ASSERT(count(gprs_llme_list()) == 0); + lle = gprs_lle_get_or_create(ms_tlli, 3); + OSMO_ASSERT(count(gprs_llme_list()) == 1); + + /* inject the attach request */ + send_0408_message(lle->llme, ms_tlli, &raid1, + attach_req, ARRAY_SIZE(attach_req)); + + ctx = sgsn_mm_ctx_by_tlli(ms_tlli, &raid1); + OSMO_ASSERT(ctx != NULL); + OSMO_ASSERT(ctx->mm_state == GMM_COMMON_PROC_INIT); + OSMO_ASSERT(ctx->p_tmsi != GSM_RESERVED_TMSI); + + /* we expect an identity request (IMEI) */ + OSMO_ASSERT(sgsn_tx_counter == 1); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_ID_REQ); + OSMO_ASSERT(last_dl_parse_ctx.tlli == ms_tlli); + + /* inject the identity response (IMEI) */ + send_0408_message(ctx->llme, ms_tlli, &raid1, + ident_resp_imei, ARRAY_SIZE(ident_resp_imei)); + + /* check that the MM context has not been removed due to a failed + * authorization */ + OSMO_ASSERT(ctx == sgsn_mm_ctx_by_tlli(ms_tlli, &raid1)); + + OSMO_ASSERT(ctx->mm_state == GMM_COMMON_PROC_INIT); + + /* we expect an attach accept */ + OSMO_ASSERT(sgsn_tx_counter == 1); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_ATTACH_ACK); + OSMO_ASSERT(last_dl_parse_ctx.tlli == ms_tlli); + + received_ptmsi = get_new_ptmsi(&last_dl_parse_ctx); + OSMO_ASSERT(received_ptmsi == ctx->p_tmsi); + ptmsi1 = received_ptmsi; + + /* inject the attach complete */ + ms_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL); + send_0408_message(ctx->llme, ms_tlli, &raid1, + attach_compl, ARRAY_SIZE(attach_compl)); + + /* we don't expect a response */ + OSMO_ASSERT(sgsn_tx_counter == 0); + + OSMO_ASSERT(ctx->mm_state == GMM_REGISTERED_NORMAL); + OSMO_ASSERT(ctx->p_tmsi_old == 0); + OSMO_ASSERT(ctx->p_tmsi == ptmsi1); + + printf(" - RA Update Request (RA 1 -> RA 1)\n"); + + /* inject the RA update request */ + send_0408_message(ctx->llme, ms_tlli, &raid1, + ra_upd_req1, ARRAY_SIZE(ra_upd_req1)); + + /* we expect an RA update accept */ + OSMO_ASSERT(sgsn_tx_counter == 1); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_RA_UPD_ACK); + // OSMO_ASSERT(last_dl_parse_ctx.tlli == ms_tlli); + + OSMO_ASSERT(ctx->mm_state == GMM_COMMON_PROC_INIT); + OSMO_ASSERT(ctx->p_tmsi_old == ptmsi1); + OSMO_ASSERT(ctx->p_tmsi != GSM_RESERVED_TMSI); + OSMO_ASSERT(ctx->p_tmsi != ptmsi1); + + received_ptmsi = get_new_ptmsi(&last_dl_parse_ctx); + OSMO_ASSERT(received_ptmsi == ctx->p_tmsi); + ptmsi1 = received_ptmsi; + + /* inject the RA update complete */ + ms_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL); + send_0408_message(ctx->llme, ms_tlli, &raid1, + ra_upd_complete, ARRAY_SIZE(ra_upd_complete)); + + /* we don't expect a response */ + OSMO_ASSERT(sgsn_tx_counter == 0); + + OSMO_ASSERT(ctx->mm_state == GMM_REGISTERED_NORMAL); + OSMO_ASSERT(ctx->p_tmsi_old == 0); + OSMO_ASSERT(ctx->p_tmsi == ptmsi1); + OSMO_ASSERT(ctx->tlli == ms_tlli); + + + printf(" - RA Update Request (RA 1 -> RA 2)\n"); + + /* inject the RA update request */ + ms_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_FOREIGN); + + /* It is coming from RA 1 => ra_upd_req1 */ + send_0408_message(ctx->llme, ms_tlli, &raid2, + ra_upd_req1, ARRAY_SIZE(ra_upd_req1)); + + /* we expect an RA update reject (and a LLC XID RESET) */ + OSMO_ASSERT(sgsn_tx_counter == 2); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_RA_UPD_REJ); + /* this has killed the LLE/LLME */ + + printf(" - Attach Request (RA 2)\n"); + + /* Create a LLE/LLME */ + OSMO_ASSERT(count(gprs_llme_list()) == 1); + lle = gprs_lle_get_or_create(ms_tlli, 3); + OSMO_ASSERT(count(gprs_llme_list()) == 1); + + /* inject the attach request */ + send_0408_message(lle->llme, ms_tlli, &raid2, + attach_req2, ARRAY_SIZE(attach_req2)); + + ctx = sgsn_mm_ctx_by_tlli(ms_tlli, &raid2); + OSMO_ASSERT(ctx != NULL); + OSMO_ASSERT(ctx->mm_state == GMM_COMMON_PROC_INIT); + OSMO_ASSERT(ctx->p_tmsi != GSM_RESERVED_TMSI); + + /* we expect an attach accept */ + OSMO_ASSERT(sgsn_tx_counter == 1); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_ATTACH_ACK); + + received_ptmsi = get_new_ptmsi(&last_dl_parse_ctx); + OSMO_ASSERT(received_ptmsi == ctx->p_tmsi); + ptmsi1 = received_ptmsi; + + /* inject the attach complete */ + ms_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL); + ictx = sgsn_mm_ctx_by_tlli(ms_tlli, &raid2); + OSMO_ASSERT(ictx != NULL); + OSMO_ASSERT(ictx == ctx); + + send_0408_message(ctx->llme, ms_tlli, &raid2, + attach_compl, ARRAY_SIZE(attach_compl)); + + /* we don't expect a response */ + OSMO_ASSERT(sgsn_tx_counter == 0); + + OSMO_ASSERT(ctx->mm_state == GMM_REGISTERED_NORMAL); + OSMO_ASSERT(ctx->p_tmsi_old == 0); + OSMO_ASSERT(ctx->p_tmsi == ptmsi1); + + printf(" - RA Update Request (RA other -> RA 2)\n"); + + /* inject the RA update request */ + ms_tlli = gprs_tmsi2tlli(0x12345678, TLLI_FOREIGN); + + /* It is coming from RA 1 => ra_upd_req1 */ + send_0408_message(ctx->llme, ms_tlli, &raid2, + ra_upd_req_other, ARRAY_SIZE(ra_upd_req_other)); + + /* we expect an RA update reject (and a LLC XID RESET) */ + OSMO_ASSERT(sgsn_tx_counter == 2); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_RA_UPD_REJ); + /* this has killed the LLE/LLME */ + + printf(" - Attach Request (RA 2)\n"); + + /* Create a LLE/LLME */ + OSMO_ASSERT(count(gprs_llme_list()) == 1); + lle = gprs_lle_get_or_create(ms_tlli, 3); + OSMO_ASSERT(count(gprs_llme_list()) == 1); + + /* inject the attach request */ + send_0408_message(lle->llme, ms_tlli, &raid2, + attach_req2, ARRAY_SIZE(attach_req2)); + + ctx = sgsn_mm_ctx_by_tlli(ms_tlli, &raid2); + OSMO_ASSERT(ctx != NULL); + OSMO_ASSERT(ctx->mm_state == GMM_COMMON_PROC_INIT); + OSMO_ASSERT(ctx->p_tmsi != GSM_RESERVED_TMSI); + + /* we expect an attach accept */ + OSMO_ASSERT(sgsn_tx_counter == 1); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_ATTACH_ACK); + + received_ptmsi = get_new_ptmsi(&last_dl_parse_ctx); + OSMO_ASSERT(received_ptmsi == ctx->p_tmsi); + ptmsi1 = received_ptmsi; + + /* inject the attach complete */ + ms_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL); + ictx = sgsn_mm_ctx_by_tlli(ms_tlli, &raid2); + OSMO_ASSERT(ictx != NULL); + OSMO_ASSERT(ictx == ctx); + + send_0408_message(ctx->llme, ms_tlli, &raid2, + attach_compl, ARRAY_SIZE(attach_compl)); + + /* we don't expect a response */ + OSMO_ASSERT(sgsn_tx_counter == 0); + + OSMO_ASSERT(ctx->mm_state == GMM_REGISTERED_NORMAL); + OSMO_ASSERT(ctx->p_tmsi_old == 0); + OSMO_ASSERT(ctx->p_tmsi == ptmsi1); + + printf(" - RA Update Request (RA 2 -> RA 2)\n"); + + /* inject the RA update request */ + send_0408_message(ctx->llme, ms_tlli, &raid2, + ra_upd_req2, ARRAY_SIZE(ra_upd_req2)); + + /* we expect an RA update accept */ + OSMO_ASSERT(sgsn_tx_counter == 1); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_RA_UPD_ACK); + + OSMO_ASSERT(ctx->mm_state == GMM_COMMON_PROC_INIT); + OSMO_ASSERT(ctx->p_tmsi_old == ptmsi1); + OSMO_ASSERT(ctx->p_tmsi != GSM_RESERVED_TMSI); + OSMO_ASSERT(ctx->p_tmsi != ptmsi1); + + received_ptmsi = get_new_ptmsi(&last_dl_parse_ctx); + OSMO_ASSERT(received_ptmsi == ctx->p_tmsi); + ptmsi1 = received_ptmsi; + + /* inject the RA update complete */ + ms_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL); + send_0408_message(ctx->llme, ms_tlli, &raid2, + ra_upd_complete, ARRAY_SIZE(ra_upd_complete)); + + /* we don't expect a response */ + OSMO_ASSERT(sgsn_tx_counter == 0); + + OSMO_ASSERT(ctx->mm_state == GMM_REGISTERED_NORMAL); + OSMO_ASSERT(ctx->p_tmsi_old == 0); + OSMO_ASSERT(ctx->p_tmsi == ptmsi1); + OSMO_ASSERT(ctx->tlli == ms_tlli); + + + /* inject the detach */ + send_0408_message(ctx->llme, ms_tlli, &raid2, + detach_req, ARRAY_SIZE(detach_req)); + + /* verify that things are gone */ + OSMO_ASSERT(count(gprs_llme_list()) == 0); + ictx = sgsn_mm_ctx_by_tlli(ms_tlli, &raid2); + OSMO_ASSERT(!ictx); + + sgsn->cfg.auth_policy = saved_auth_policy; + + cleanup_test(); +} + static void test_apn_matching(void) { struct apn_ctx *actx, *actxs[9]; @@ -2129,6 +2475,7 @@ int main(int argc, char **argv) test_gmm_reject(); test_gmm_cancel(); test_gmm_ptmsi_allocation(); + test_gmm_routing_areas(); test_apn_matching(); test_ggsn_selection(); printf("Done\n"); diff --git a/openbsc/tests/sgsn/sgsn_test.ok b/openbsc/tests/sgsn/sgsn_test.ok index 7913a39..5b8fc40 100644 --- a/openbsc/tests/sgsn/sgsn_test.ok +++ b/openbsc/tests/sgsn/sgsn_test.ok @@ -25,6 +25,14 @@ Testing P-TMSI allocation - sgsn_alloc_ptmsi - Repeated Attach Request - Repeated RA Update Request +Testing routing area changes + - Attach Request (RA 1) + - RA Update Request (RA 1 -> RA 1) + - RA Update Request (RA 1 -> RA 2) + - Attach Request (RA 2) + - RA Update Request (RA other -> RA 2) + - Attach Request (RA 2) + - RA Update Request (RA 2 -> RA 2) Testing APN matching Testing GGSN selection Done
If an MM context cannot be found based on BBSGP info and a RA UPDATE REQUEST is received, try to find an MM context with an P-TMSI from which the TLLI could have been derived. This also checks, whether the routing area matches.
This is similar to the old behaviour removed by the commits "sgsn: Only look at TLLIs in sgsn_mm_ctx_by_tlli" and "sgsn: Remove tlli_foreign2local", except that this will only be done for RA UPDATE REQUESTs now.
Sponsored-by: On-Waves ehf --- openbsc/include/openbsc/gprs_sgsn.h | 4 ++++ openbsc/src/gprs/gprs_gmm.c | 26 ++++++++++++++++++--- openbsc/src/gprs/gprs_sgsn.c | 25 ++++++++++++++++++++ openbsc/tests/sgsn/sgsn_test.c | 46 ++----------------------------------- openbsc/tests/sgsn/sgsn_test.ok | 1 - 5 files changed, 54 insertions(+), 48 deletions(-)
diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h index 61120b1..74f0735 100644 --- a/openbsc/include/openbsc/gprs_sgsn.h +++ b/openbsc/include/openbsc/gprs_sgsn.h @@ -178,6 +178,10 @@ struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli(uint32_t tlli, struct sgsn_mm_ctx *sgsn_mm_ctx_by_ptmsi(uint32_t tmsi); struct sgsn_mm_ctx *sgsn_mm_ctx_by_imsi(const char *imsi);
+/* look-up by matching TLLI and P-TMSI (think twice before using this) */ +struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli_and_ptmsi(uint32_t tlli, + const struct gprs_ra_id *raid); + /* Allocate a new SGSN MM context */ struct sgsn_mm_ctx *sgsn_mm_ctx_alloc(uint32_t tlli, const struct gprs_ra_id *raid); diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c index 6e7e5f1..212c7d7 100644 --- a/openbsc/src/gprs/gprs_gmm.c +++ b/openbsc/src/gprs/gprs_gmm.c @@ -1172,13 +1172,33 @@ static int gsm48_rx_gmm_ra_upd_req(struct sgsn_mm_ctx *mmctx, struct msgb *msg, * if the TLLI matches foreign_tlli (P-TMSI). Note that this * is an optimization to avoid the RA reject (impl detached) * below, which will cause a new attach cycle. */ - } - - if (!mmctx || !gprs_ra_id_equals(&mmctx->ra, &old_ra_id) || + /* Look-up the MM context based on old RA-ID and TLLI */ + mmctx = sgsn_mm_ctx_by_tlli_and_ptmsi(msgb_tlli(msg), &old_ra_id); + if (mmctx) { + LOGMMCTXP(LOGL_INFO, mmctx, + "Looked up by matching TLLI and P_TMSI. " + "BSSGP TLLI: %08x, P-TMSI: %08x (%08x), " + "TLLI: %08x (%08x), RA: %d-%d-%d-%d\n", + msgb_tlli(msg), + mmctx->p_tmsi, mmctx->p_tmsi_old, + mmctx->tlli, mmctx->tlli_new, + mmctx->ra.mcc, mmctx->ra.mnc, + mmctx->ra.lac, mmctx->ra.rac); + + mmctx->mm_state = GMM_COMMON_PROC_INIT; + } + } else if (!gprs_ra_id_equals(&mmctx->ra, &old_ra_id) || mmctx->mm_state == GMM_DEREGISTERED) { /* We cannot use the mmctx */ + LOGMMCTXP(LOGL_INFO, mmctx, + "The MM context cannot be used, RA: %d-%d-%d-%d\n", + mmctx->ra.mcc, mmctx->ra.mnc, + mmctx->ra.lac, mmctx->ra.rac); + mmctx = NULL; + }
+ if (!mmctx) { /* send a XID reset to re-set all LLC sequence numbers * in the MS */ LOGMMCTXP(LOGL_NOTICE, mmctx, "LLC XID RESET\n"); diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c index f71066d..b7bda49 100644 --- a/openbsc/src/gprs/gprs_sgsn.c +++ b/openbsc/src/gprs/gprs_sgsn.c @@ -105,6 +105,31 @@ struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli(uint32_t tlli, return NULL; }
+struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli_and_ptmsi(uint32_t tlli, + const struct gprs_ra_id *raid) +{ + struct sgsn_mm_ctx *ctx; + int tlli_type; + + /* TODO: Also check the P_TMSI signature to be safe. That signature + * should be different (at least with a sufficiently high probability) + * after SGSN restarts and for multiple SGSN instances. + */ + + tlli_type = gprs_tlli_type(tlli); + if (tlli_type != TLLI_FOREIGN && tlli_type != TLLI_LOCAL) + return NULL; + + llist_for_each_entry(ctx, &sgsn_mm_ctxts, list) { + if ((gprs_tmsi2tlli(ctx->p_tmsi, tlli_type) == tlli || + gprs_tmsi2tlli(ctx->p_tmsi_old, tlli_type) == tlli) && + gprs_ra_id_equals(raid, &ctx->ra)) + return ctx; + } + + return NULL; +} + struct sgsn_mm_ctx *sgsn_mm_ctx_by_ptmsi(uint32_t p_tmsi) { struct sgsn_mm_ctx *ctx; diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c index d27b755..1acd6f2 100644 --- a/openbsc/tests/sgsn/sgsn_test.c +++ b/openbsc/tests/sgsn/sgsn_test.c @@ -1995,7 +1995,6 @@ static void test_gmm_routing_areas(void) OSMO_ASSERT(ctx->p_tmsi == ptmsi1); OSMO_ASSERT(ctx->tlli == ms_tlli);
- printf(" - RA Update Request (RA 1 -> RA 2)\n");
/* inject the RA update request */ @@ -2005,50 +2004,9 @@ static void test_gmm_routing_areas(void) send_0408_message(ctx->llme, ms_tlli, &raid2, ra_upd_req1, ARRAY_SIZE(ra_upd_req1));
- /* we expect an RA update reject (and a LLC XID RESET) */ - OSMO_ASSERT(sgsn_tx_counter == 2); - OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_RA_UPD_REJ); - /* this has killed the LLE/LLME */ - - printf(" - Attach Request (RA 2)\n"); - - /* Create a LLE/LLME */ - OSMO_ASSERT(count(gprs_llme_list()) == 1); - lle = gprs_lle_get_or_create(ms_tlli, 3); - OSMO_ASSERT(count(gprs_llme_list()) == 1); - - /* inject the attach request */ - send_0408_message(lle->llme, ms_tlli, &raid2, - attach_req2, ARRAY_SIZE(attach_req2)); - - ctx = sgsn_mm_ctx_by_tlli(ms_tlli, &raid2); - OSMO_ASSERT(ctx != NULL); - OSMO_ASSERT(ctx->mm_state == GMM_COMMON_PROC_INIT); - OSMO_ASSERT(ctx->p_tmsi != GSM_RESERVED_TMSI); - - /* we expect an attach accept */ + /* we expect an RA update accept */ OSMO_ASSERT(sgsn_tx_counter == 1); - OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_ATTACH_ACK); - - received_ptmsi = get_new_ptmsi(&last_dl_parse_ctx); - OSMO_ASSERT(received_ptmsi == ctx->p_tmsi); - ptmsi1 = received_ptmsi; - - /* inject the attach complete */ - ms_tlli = gprs_tmsi2tlli(ptmsi1, TLLI_LOCAL); - ictx = sgsn_mm_ctx_by_tlli(ms_tlli, &raid2); - OSMO_ASSERT(ictx != NULL); - OSMO_ASSERT(ictx == ctx); - - send_0408_message(ctx->llme, ms_tlli, &raid2, - attach_compl, ARRAY_SIZE(attach_compl)); - - /* we don't expect a response */ - OSMO_ASSERT(sgsn_tx_counter == 0); - - OSMO_ASSERT(ctx->mm_state == GMM_REGISTERED_NORMAL); - OSMO_ASSERT(ctx->p_tmsi_old == 0); - OSMO_ASSERT(ctx->p_tmsi == ptmsi1); + OSMO_ASSERT(last_dl_parse_ctx.g48_hdr->msg_type == GSM48_MT_GMM_RA_UPD_ACK);
printf(" - RA Update Request (RA other -> RA 2)\n");
diff --git a/openbsc/tests/sgsn/sgsn_test.ok b/openbsc/tests/sgsn/sgsn_test.ok index 5b8fc40..cdd2006 100644 --- a/openbsc/tests/sgsn/sgsn_test.ok +++ b/openbsc/tests/sgsn/sgsn_test.ok @@ -29,7 +29,6 @@ Testing routing area changes - Attach Request (RA 1) - RA Update Request (RA 1 -> RA 1) - RA Update Request (RA 1 -> RA 2) - - Attach Request (RA 2) - RA Update Request (RA other -> RA 2) - Attach Request (RA 2) - RA Update Request (RA 2 -> RA 2)
On 04 Jan 2016, at 18:43, Jacob Erlbeck jerlbeck@sysmocom.de wrote:
Dear Jacob,
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c index 6e7e5f1..212c7d7 100644 --- a/openbsc/src/gprs/gprs_gmm.c +++ b/openbsc/src/gprs/gprs_gmm.c @@ -1172,13 +1172,33 @@ static int gsm48_rx_gmm_ra_upd_req(struct sgsn_mm_ctx *mmctx, struct msgb *msg, * if the TLLI matches foreign_tlli (P-TMSI). Note that this * is an optimization to avoid the RA reject (impl detached) * below, which will cause a new attach cycle. */
- }
the todo above reads:
/* TODO: Check if there is an MM CTX with old_ra_id and * the P-TMSI (if given, reguired for UMTS) or as last resort * if the TLLI matches foreign_tlli (P-TMSI). Note that this * is an optimization to avoid the RA reject (impl detached) * below, which will cause a new attach cycle. */
I think this todo is addressed with sgsn_mm_ctx_by_tlli_and_ptmsi? Can the comment be removed?
What about the test? Do we have one that gets the "XID RESET" we expect?
holger
-
Holger Freyther -
Jacob Erlbeck