Merge branch 'jerlbeck/fixes/sgsn'

Remove the assert in the llme by avoiding the usage of foreign to local
mappings of TLLIs. The asserts had been hit at 32C3 and the fixes were
created by Jacob and had been tested at the convention.

4 files changed, 68 insertions, 61 deletions

diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index 7d3e3de75..d93ba3f77 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -1164,9 +1164,41 @@ 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. */
+		/* 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_llc.c b/openbsc/src/gprs/gprs_llc.c
index 936354a09..4cf51631b 100644
--- a/openbsc/src/gprs/gprs_llc.c
+++ b/openbsc/src/gprs/gprs_llc.c
@@ -38,21 +38,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 +57,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 +158,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 +183,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/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c
index c4dc9d7ec..b7bda49ba 100644
--- a/openbsc/src/gprs/gprs_sgsn.c
+++ b/openbsc/src/gprs/gprs_sgsn.c
@@ -90,52 +90,41 @@ 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)
-{
-	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 &&
-		    ra_id_equals(raid, &ctx->ra))
+		if ((tlli == ctx->tlli || tlli == ctx->tlli_new) &&
+		    gprs_ra_id_equals(raid, &ctx->ra))
 			return ctx;
 	}
 
+	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);
-	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) &&
-			    ra_id_equals(raid, &ctx->ra))
-				return ctx;
-		}
-		break;
-	default:
-		break;
+	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;
diff --git a/openbsc/src/gprs/gprs_utils.c b/openbsc/src/gprs/gprs_utils.c
index ad479db81..895a03384 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);
+}