ms: Use the IMSI to retrieve the MS object

This commit extends get_ms() to really compare the IMSI if it has
been given. Matching by TLLI has a higher precedence than matching by
IMSI.

Ticket: #1674
Sponsored-by: On-Waves ehf

5 files changed, 50 insertions, 18 deletions

diff --git a/src/gprs_ms_storage.cpp b/src/gprs_ms_storage.cpp
index 1cdb7537..a0a32081 100644
--- a/src/gprs_ms_storage.cpp
+++ b/src/gprs_ms_storage.cpp
@@ -53,22 +53,30 @@ void GprsMsStorage::ms_active(class GprsMs *ms)
 
 GprsMs *GprsMsStorage::get_ms(uint32_t tlli, uint32_t old_tlli, const char *imsi) const
 {
-	GprsMs *ms = NULL;
+	GprsMs *ms;
 	LListHead<GprsMs> *pos;
 
-	llist_for_each(pos, &m_list) {
-		ms = pos->entry();
-		if (ms->check_tlli(tlli))
-			break;
-		if (ms->check_tlli(old_tlli))
-			break;
-		/* TODO: Check for IMSI */
+	if (tlli || old_tlli) {
+		llist_for_each(pos, &m_list) {
+			ms = pos->entry();
+			if (ms->check_tlli(tlli))
+				return ms;
+			if (ms->check_tlli(old_tlli))
+				return ms;
+		}
+	}
+
+	/* not found by TLLI */
 
-		/* not found */
-		ms = NULL;
+	if (imsi && imsi[0]) {
+		llist_for_each(pos, &m_list) {
+			ms = pos->entry();
+			if (strcmp(imsi, ms->imsi()) == 0)
+				return ms;
+		}
 	}
 
-	return ms;
+	return NULL;
 }
 
 GprsMs *GprsMsStorage::create_ms(uint32_t tlli, enum gprs_rlcmac_tbf_direction dir)
diff --git a/tests/ms/MsTest.cpp b/tests/ms/MsTest.cpp
index 75328f48..9c8ec2c3 100644
--- a/tests/ms/MsTest.cpp
+++ b/tests/ms/MsTest.cpp
@@ -328,6 +328,9 @@ static void test_ms_change_tlli()
 static void test_ms_storage()
 {
 	uint32_t tlli = 0xffeeddbb;
+	const char *imsi1 = "001001987654321";
+	const char *imsi2 = "001001987654322";
+
 	gprs_rlcmac_ul_tbf *ul_tbf;
 	GprsMs *ms, *ms_tmp;
 	GprsMsStorage store;
@@ -343,19 +346,36 @@ static void test_ms_storage()
 	ms = store.create_ms(tlli + 0, GPRS_RLCMAC_UL_TBF);
 	OSMO_ASSERT(ms != NULL);
 	OSMO_ASSERT(ms->tlli() == tlli + 0);
+	ms->set_imsi(imsi1);
+	OSMO_ASSERT(strcmp(ms->imsi(), imsi1) == 0);
 
 	ms_tmp = store.get_ms(tlli + 0);
 	OSMO_ASSERT(ms == ms_tmp);
 	OSMO_ASSERT(ms->tlli() == tlli + 0);
 
+	ms_tmp = store.get_ms(0, 0, imsi1);
+	OSMO_ASSERT(ms == ms_tmp);
+	OSMO_ASSERT(strcmp(ms->imsi(), imsi1) == 0);
+	ms_tmp = store.get_ms(0, 0, imsi2);
+	OSMO_ASSERT(ms_tmp == NULL);
+
 	ms = store.create_ms(tlli + 1, GPRS_RLCMAC_UL_TBF);
 	OSMO_ASSERT(ms != NULL);
 	OSMO_ASSERT(ms->tlli() == tlli + 1);
+	ms->set_imsi(imsi2);
+	OSMO_ASSERT(strcmp(ms->imsi(), imsi2) == 0);
 
 	ms_tmp = store.get_ms(tlli + 1);
 	OSMO_ASSERT(ms == ms_tmp);
 	OSMO_ASSERT(ms->tlli() == tlli + 1);
 
+	ms_tmp = store.get_ms(0, 0, imsi1);
+	OSMO_ASSERT(ms_tmp != NULL);
+	OSMO_ASSERT(ms_tmp != ms);
+	ms_tmp = store.get_ms(0, 0, imsi2);
+	OSMO_ASSERT(ms == ms_tmp);
+	OSMO_ASSERT(strcmp(ms->imsi(), imsi2) == 0);
+
 	/* delete ms */
 	ms = store.get_ms(tlli + 0);
 	OSMO_ASSERT(ms != NULL);
diff --git a/tests/ms/MsTest.err b/tests/ms/MsTest.err
index 4f704cbc..86dbb5b0 100644
--- a/tests/ms/MsTest.err
+++ b/tests/ms/MsTest.err
@@ -37,8 +37,10 @@ Modifying MS object, TLLI: 0xaa000000 -> 0xff001111, already confirmed partly
 Destroying MS object, TLLI = 0xff001111
 Creating MS object, TLLI = 0x00000000
 Modifying MS object, UL TLLI: 0x00000000 -> 0xffeeddbb, not yet confirmed
+Modifying MS object, TLLI = 0xffeeddbb, IMSI '' -> '001001987654321'
 Creating MS object, TLLI = 0x00000000
 Modifying MS object, UL TLLI: 0x00000000 -> 0xffeeddbc, not yet confirmed
+Modifying MS object, TLLI = 0xffeeddbc, IMSI '' -> '001001987654322'
 Attaching TBF to MS object, TLLI = 0xffeeddbb, TBF = TBF(TFI=0 TLLI=0x00000000 DIR=UL STATE=NULL)
 Detaching TBF from MS object, TLLI = 0xffeeddbb, TBF = TBF(TFI=0 TLLI=0x00000000 DIR=UL STATE=NULL)
 Destroying MS object, TLLI = 0xffeeddbb
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index bc783176..b0106caf 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -327,28 +327,28 @@ static void test_tbf_imsi()
 
 	dl_tbf[0]->assign_imsi("001001000000001");
 	ms1 = the_bts.ms_store().get_ms(0, 0, "001001000000001");
-	/* OSMO_ASSERT(ms1 != NULL); */
+	OSMO_ASSERT(ms1 != NULL);
 	ms2 = the_bts.ms_store().get_ms(0xf1000001);
 	OSMO_ASSERT(ms2 != NULL);
 	OSMO_ASSERT(strcmp(ms2->imsi(), "001001000000001") == 0);
-	/* OSMO_ASSERT(ms1 == ms2); */
+	OSMO_ASSERT(ms1 == ms2);
 
 	/* change the IMSI on TBF 0 */
 	dl_tbf[0]->assign_imsi("001001000000002");
 	ms1 = the_bts.ms_store().get_ms(0, 0, "001001000000001");
 	OSMO_ASSERT(ms1 == NULL);
 	ms1 = the_bts.ms_store().get_ms(0, 0, "001001000000002");
-	/* OSMO_ASSERT(ms1 != NULL); */
+	OSMO_ASSERT(ms1 != NULL);
 	OSMO_ASSERT(strcmp(ms2->imsi(), "001001000000002") == 0);
-	/* OSMO_ASSERT(ms1 == ms2); */
+	OSMO_ASSERT(ms1 == ms2);
 
 	/* use the same IMSI on TBF 2 */
 	dl_tbf[1]->assign_imsi("001001000000002");
 	ms1 = the_bts.ms_store().get_ms(0, 0, "001001000000002");
-	/* OSMO_ASSERT(ms1 != NULL); */
+	OSMO_ASSERT(ms1 != NULL);
 	OSMO_ASSERT(ms1 != ms2);
-	/* OSMO_ASSERT(strcmp(ms1->imsi(), "001001000000002") == 0); */
-	/* OSMO_ASSERT(strcmp(ms2->imsi(), "") == 0); */
+	OSMO_ASSERT(strcmp(ms1->imsi(), "001001000000002") == 0);
+	OSMO_ASSERT(strcmp(ms2->imsi(), "") == 0);
 
 	tbf_free(dl_tbf[1]);
 	ms1 = the_bts.ms_store().get_ms(0, 0, "001001000000002");
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index 7fa1283b..8c411f89 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -395,6 +395,8 @@ The MS object cannot fully confirm an unexpected TLLI: 0xf1000002, partly confir
 Attaching TBF to MS object, TLLI = 0xf1000002, TBF = TBF(TFI=1 TLLI=0xf1000002 DIR=DL STATE=FLOW)
 Modifying MS object, TLLI = 0xf1000001, IMSI '' -> '001001000000001'
 Modifying MS object, TLLI = 0xf1000001, IMSI '001001000000001' -> '001001000000002'
+TBF(TFI=1 TLLI=0xf1000002 DIR=DL STATE=FLOW) the IMSI '001001000000002' was already assigned to another MS object: TLLI = 0xf1000001, that IMSI will be removed
+Modifying MS object, TLLI = 0xf1000001, IMSI '001001000000002' -> ''
 Modifying MS object, TLLI = 0xf1000002, IMSI '' -> '001001000000002'
 TBF(TFI=1 TLLI=0xf1000002 DIR=DL STATE=FLOW) free
 TBF(TFI=1 TLLI=0xf1000002 DIR=DL STATE=FLOW) Software error: Pending downlink assignment. This may not happen, because the assignment message never gets transmitted. Please be sure not to free in this state. PLEASE FIX!