diff mbox

[7/7] sgsn: Re-add searching for MM ctx based on TLLI / P-TMSI matches

Message ID 1451929418-32581-7-git-send-email-jerlbeck@sysmocom.de
State Accepted
Headers show

Commit Message

Jacob Erlbeck Jan. 4, 2016, 5:43 p.m. UTC 
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(-)

Comments

Holger Freyther Jan. 15, 2016, 5:03 p.m. UTC | #1 
> 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
diff mbox

Patch

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)