gbproxy: Remove gbproxy_register_tlli

This function is a remainder of the initial implemenation that was
not meant for TLLI patching and can be used for the BSS side only.
The SGSN side is already using a composition of more flexible
single purpose functions.

This patch changes the implementation to use a similar approach. The
function is moved to gbproxy_test.c and renamed to register_tlli to
keep the tests intact.

Sponsored-by: On-Waves ehf

3 files changed, 89 insertions, 77 deletions

diff --git a/openbsc/include/openbsc/gb_proxy.h b/openbsc/include/openbsc/gb_proxy.h
index 43eb0beaa..4a20bfb50 100644
--- a/openbsc/include/openbsc/gb_proxy.h
+++ b/openbsc/include/openbsc/gb_proxy.h
@@ -189,9 +189,13 @@ void gbproxy_delete_tlli(struct gbproxy_peer *peer,
 			 struct gbproxy_tlli_info *tlli_info);
 void gbproxy_tlli_info_discard_messages(struct gbproxy_tlli_info *tlli_info);
 
-struct gbproxy_tlli_info *gbproxy_register_tlli(
-	struct gbproxy_peer *peer, uint32_t tlli,
-	const uint8_t *imsi, size_t imsi_len, time_t now);
+void gbproxy_attach_tlli_info(struct gbproxy_peer *peer, time_t now,
+			      struct gbproxy_tlli_info *tlli_info);
+void gbproxy_update_tlli_info(struct gbproxy_tlli_info *tlli_info,
+			      const uint8_t *imsi, size_t imsi_len);
+void gbproxy_detach_tlli_info(struct gbproxy_peer *peer,
+			      struct gbproxy_tlli_info *tlli_info);
+struct gbproxy_tlli_info *gbproxy_tlli_info_alloc( struct gbproxy_peer *peer);
 
 struct gbproxy_tlli_info *gbproxy_find_tlli(
 	struct gbproxy_peer *peer, uint32_t tlli);
diff --git a/openbsc/src/gprs/gb_proxy_tlli.c b/openbsc/src/gprs/gb_proxy_tlli.c
index c3dc7baf2..78459b75d 100644
--- a/openbsc/src/gprs/gb_proxy_tlli.c
+++ b/openbsc/src/gprs/gb_proxy_tlli.c
@@ -135,8 +135,8 @@ void gbproxy_delete_tllis(struct gbproxy_peer *peer)
 	OSMO_ASSERT(llist_empty(&state->enabled_tllis));
 }
 
-static void gbproxy_attach_tlli_info(struct gbproxy_peer *peer, time_t now,
-				     struct gbproxy_tlli_info *tlli_info)
+void gbproxy_attach_tlli_info(struct gbproxy_peer *peer, time_t now,
+			      struct gbproxy_tlli_info *tlli_info)
 {
 	struct gbproxy_patch_state *state = &peer->patch_state;
 
@@ -201,8 +201,7 @@ int gbproxy_remove_stale_tllis(struct gbproxy_peer *peer, time_t now)
 	return deleted_count;
 }
 
-static struct gbproxy_tlli_info *gbproxy_tlli_info_alloc(
-	struct gbproxy_peer *peer)
+struct gbproxy_tlli_info *gbproxy_tlli_info_alloc( struct gbproxy_peer *peer)
 {
 	struct gbproxy_tlli_info *tlli_info;
 
@@ -215,7 +214,7 @@ static struct gbproxy_tlli_info *gbproxy_tlli_info_alloc(
 	return tlli_info;
 }
 
-static void gbproxy_detach_tlli_info(
+void gbproxy_detach_tlli_info(
 	struct gbproxy_peer *peer,
 	struct gbproxy_tlli_info *tlli_info)
 {
@@ -229,8 +228,8 @@ static void gbproxy_detach_tlli_info(
 		state->enabled_tllis_count;
 }
 
-static void gbproxy_update_tlli_info(struct gbproxy_tlli_info *tlli_info,
-				     const uint8_t *imsi, size_t imsi_len)
+void gbproxy_update_tlli_info(struct gbproxy_tlli_info *tlli_info,
+			      const uint8_t *imsi, size_t imsi_len)
 {
 	if (!gprs_is_mi_imsi(imsi, imsi_len))
 		return;
@@ -317,57 +316,6 @@ void gbproxy_touch_tlli(struct gbproxy_peer *peer,
 	gbproxy_attach_tlli_info(peer, now, tlli_info);
 }
 
-struct gbproxy_tlli_info *gbproxy_register_tlli(
-	struct gbproxy_peer *peer, uint32_t tlli,
-	const uint8_t *imsi, size_t imsi_len, time_t now)
-{
-	struct gbproxy_tlli_info *tlli_info;
-	int enable_patching = -1;
-	int tlli_already_known = 0;
-
-	/* Check, whether the IMSI matches */
-	if (gprs_is_mi_imsi(imsi, imsi_len)) {
-		enable_patching = gbproxy_check_imsi(peer, imsi, imsi_len);
-		if (enable_patching < 0)
-			return NULL;
-	}
-
-	tlli_info = gbproxy_find_tlli(peer, tlli);
-
-	if (!tlli_info) {
-		tlli_info = gbproxy_find_tlli_by_imsi(peer, imsi, imsi_len);
-
-		if (tlli_info) {
-			/* TLLI has changed somehow, adjust it */
-			LOGP(DGPRS, LOGL_INFO,
-			     "The TLLI has changed from %08x to %08x\n",
-			     tlli_info->tlli.current, tlli);
-			tlli_info->tlli.current = tlli;
-		}
-	}
-
-	if (!tlli_info) {
-		tlli_info = gbproxy_tlli_info_alloc(peer);
-		tlli_info->tlli.current = tlli;
-	} else {
-		gbproxy_detach_tlli_info(peer, tlli_info);
-		tlli_already_known = 1;
-	}
-
-	OSMO_ASSERT(tlli_info != NULL);
-
-	if (!tlli_already_known)
-		LOGP(DGPRS, LOGL_INFO, "Adding TLLI %08x to list\n", tlli);
-
-	gbproxy_attach_tlli_info(peer, now, tlli_info);
-	gbproxy_update_tlli_info(tlli_info, imsi, imsi_len);
-
-	if (enable_patching >= 0)
-		tlli_info->enable_patching = enable_patching;
-
-	return tlli_info;
-}
-
 static void gbproxy_unregister_tlli(struct gbproxy_peer *peer, uint32_t tlli)
 {
 	struct gbproxy_tlli_info *tlli_info;
@@ -397,20 +345,27 @@ struct gbproxy_tlli_info *gbproxy_update_tlli_state_ul(
 {
 	struct gbproxy_tlli_info *tlli_info = NULL;
 
-	if (parse_ctx->tlli_enc)
+	if (parse_ctx->tlli_enc) {
 		tlli_info = gbproxy_find_tlli(peer, parse_ctx->tlli);
 
+		if (!tlli_info && parse_ctx->imsi)
+			tlli_info = gbproxy_find_tlli_by_imsi(
+				peer, parse_ctx->imsi, parse_ctx->imsi_len);
+	}
+
 	if (parse_ctx->tlli_enc && parse_ctx->llc) {
 		uint32_t sgsn_tlli;
 		if (!tlli_info) {
-			tlli_info =
-				gbproxy_register_tlli(peer, parse_ctx->tlli,
-						      parse_ctx->imsi,
-						      parse_ctx->imsi_len, now);
+			LOGP(DGPRS, LOGL_INFO, "Adding TLLI %08x to list\n",
+			    parse_ctx->tlli);
+			tlli_info = gbproxy_tlli_info_alloc(peer);
+			gbproxy_attach_tlli_info(peer, now, tlli_info);
+
 			/* Setup TLLIs */
 			sgsn_tlli = gbproxy_make_sgsn_tlli(peer, tlli_info,
 							   parse_ctx->tlli);
 			tlli_info->sgsn_tlli.current = sgsn_tlli;
+			tlli_info->tlli.current = parse_ctx->tlli;;
 		} else {
 			sgsn_tlli = gbproxy_map_tlli(parse_ctx->tlli, tlli_info, 0);
 			if (!sgsn_tlli)
@@ -476,6 +431,7 @@ struct gbproxy_tlli_info *gbproxy_update_tlli_state_dl(
 		   !peer->cfg->patch_ptmsi) {
 		/* A new P-TMSI has been signalled in the message with an unknown
 		 * TLLI, create a new tlli_info */
+		/* TODO: Add a test case for this branch */
 		uint32_t new_ptmsi;
 		if (!gprs_parse_mi_tmsi(parse_ctx->new_ptmsi_enc, GSM48_TMSI_LEN,
 					&new_ptmsi)) {
@@ -494,6 +450,7 @@ struct gbproxy_tlli_info *gbproxy_update_tlli_state_dl(
 		tlli_info->tlli.current = parse_ctx->tlli;;
 		tlli_info->sgsn_tlli.ptmsi = new_ptmsi;
 		tlli_info->tlli.ptmsi = new_ptmsi;
+		gbproxy_attach_tlli_info(peer, now, tlli_info);
 	} else if (parse_ctx->tlli_enc && parse_ctx->llc && !tlli_info &&
 		   !peer->cfg->patch_ptmsi) {
 		/* Unknown SGSN TLLI, create a new tlli_info */
diff --git a/openbsc/tests/gbproxy/gbproxy_test.c b/openbsc/tests/gbproxy/gbproxy_test.c
index f2f8546f6..95024b661 100644
--- a/openbsc/tests/gbproxy/gbproxy_test.c
+++ b/openbsc/tests/gbproxy/gbproxy_test.c
@@ -2795,6 +2795,57 @@ static void test_tlv_shift_functions()
 	}
 }
 
+struct gbproxy_tlli_info *register_tlli(
+	struct gbproxy_peer *peer, uint32_t tlli,
+	const uint8_t *imsi, size_t imsi_len, time_t now)
+{
+	struct gbproxy_tlli_info *tlli_info;
+	int enable_patching = -1;
+	int tlli_already_known = 0;
+
+	/* Check, whether the IMSI matches */
+	if (gprs_is_mi_imsi(imsi, imsi_len)) {
+		enable_patching = gbproxy_check_imsi(peer, imsi, imsi_len);
+		if (enable_patching < 0)
+			return NULL;
+	}
+
+	tlli_info = gbproxy_find_tlli(peer, tlli);
+
+	if (!tlli_info) {
+		tlli_info = gbproxy_find_tlli_by_imsi(peer, imsi, imsi_len);
+
+		if (tlli_info) {
+			/* TLLI has changed somehow, adjust it */
+			LOGP(DGPRS, LOGL_INFO,
+			     "The TLLI has changed from %08x to %08x\n",
+			     tlli_info->tlli.current, tlli);
+			tlli_info->tlli.current = tlli;
+		}
+	}
+
+	if (!tlli_info) {
+		tlli_info = gbproxy_tlli_info_alloc(peer);
+		tlli_info->tlli.current = tlli;
+	} else {
+		gbproxy_detach_tlli_info(peer, tlli_info);
+		tlli_already_known = 1;
+	}
+
+	OSMO_ASSERT(tlli_info != NULL);
+
+	if (!tlli_already_known)
+		LOGP(DGPRS, LOGL_INFO, "Adding TLLI %08x to list\n", tlli);
+
+	gbproxy_attach_tlli_info(peer, now, tlli_info);
+	gbproxy_update_tlli_info(tlli_info, imsi, imsi_len);
+
+	if (enable_patching >= 0)
+		tlli_info->enable_patching = enable_patching;
+
+	return tlli_info;
+}
+
 static void test_gbproxy_tlli_expire(void)
 {
 	struct gbproxy_config cfg = {0};
@@ -2830,7 +2881,7 @@ static void test_gbproxy_tlli_expire(void)
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 0);
 
 		printf("  Add TLLI 1, IMSI 1\n");
-		tlli_info = gbproxy_register_tlli(peer, tlli1,
+		tlli_info = register_tlli(peer, tlli1,
 						  imsi1, ARRAY_SIZE(imsi1), now);
 		OSMO_ASSERT(tlli_info);
 		OSMO_ASSERT(tlli_info->tlli.current == tlli1);
@@ -2838,7 +2889,7 @@ static void test_gbproxy_tlli_expire(void)
 
 		/* replace the old entry */
 		printf("  Add TLLI 2, IMSI 1 (should replace TLLI 1)\n");
-		tlli_info = gbproxy_register_tlli(peer, tlli2,
+		tlli_info = register_tlli(peer, tlli2,
 						  imsi1, ARRAY_SIZE(imsi1), now);
 		OSMO_ASSERT(tlli_info);
 		OSMO_ASSERT(tlli_info->tlli.current == tlli2);
@@ -2869,7 +2920,7 @@ static void test_gbproxy_tlli_expire(void)
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 0);
 
 		printf("  Add TLLI 1, IMSI 1\n");
-		tlli_info = gbproxy_register_tlli(peer, tlli1,
+		tlli_info = register_tlli(peer, tlli1,
 						  imsi1, ARRAY_SIZE(imsi1), now);
 		OSMO_ASSERT(tlli_info);
 		OSMO_ASSERT(tlli_info->tlli.current == tlli1);
@@ -2877,7 +2928,7 @@ static void test_gbproxy_tlli_expire(void)
 
 		/* try to replace the old entry */
 		printf("  Add TLLI 1, IMSI 2 (should replace IMSI 1)\n");
-		tlli_info = gbproxy_register_tlli(peer, tlli1,
+		tlli_info = register_tlli(peer, tlli1,
 						  imsi2, ARRAY_SIZE(imsi2), now);
 		OSMO_ASSERT(tlli_info);
 		OSMO_ASSERT(tlli_info->tlli.current == tlli1);
@@ -2909,12 +2960,12 @@ static void test_gbproxy_tlli_expire(void)
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 0);
 
 		printf("  Add TLLI 1, IMSI 1\n");
-		gbproxy_register_tlli(peer, tlli1, imsi1, ARRAY_SIZE(imsi1), now);
+		register_tlli(peer, tlli1, imsi1, ARRAY_SIZE(imsi1), now);
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 1);
 
 		/* replace the old entry */
 		printf("  Add TLLI 2, IMSI 2 (should replace IMSI 1)\n");
-		gbproxy_register_tlli(peer, tlli2, imsi2, ARRAY_SIZE(imsi2), now);
+		register_tlli(peer, tlli2, imsi2, ARRAY_SIZE(imsi2), now);
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 2);
 
 		num_removed = gbproxy_remove_stale_tllis(peer, time(NULL) + 2);
@@ -2947,11 +2998,11 @@ static void test_gbproxy_tlli_expire(void)
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 0);
 
 		printf("  Add TLLI 1, IMSI 1 (should expire after timeout)\n");
-		gbproxy_register_tlli(peer, tlli1, imsi1, ARRAY_SIZE(imsi1), now);
+		register_tlli(peer, tlli1, imsi1, ARRAY_SIZE(imsi1), now);
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 1);
 
 		printf("  Add TLLI 2, IMSI 2 (should not expire after timeout)\n");
-		gbproxy_register_tlli(peer, tlli2, imsi2, ARRAY_SIZE(imsi2),
+		register_tlli(peer, tlli2, imsi2, ARRAY_SIZE(imsi2),
 				     now + 1);
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 2);
 
@@ -2985,16 +3036,16 @@ static void test_gbproxy_tlli_expire(void)
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 0);
 
 		printf("  Add TLLI 1, IMSI 1 (should expire)\n");
-		gbproxy_register_tlli(peer, tlli1, imsi1, ARRAY_SIZE(imsi1), now);
+		register_tlli(peer, tlli1, imsi1, ARRAY_SIZE(imsi1), now);
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 1);
 
 		printf("  Add TLLI 2, IMSI 2 (should expire after timeout)\n");
-		gbproxy_register_tlli(peer, tlli2, imsi2, ARRAY_SIZE(imsi2),
+		register_tlli(peer, tlli2, imsi2, ARRAY_SIZE(imsi2),
 				     now + 1);
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 2);
 
 		printf("  Add TLLI 3, IMSI 3 (should not expire after timeout)\n");
-		gbproxy_register_tlli(peer, tlli3, imsi3, ARRAY_SIZE(imsi3),
+		register_tlli(peer, tlli3, imsi3, ARRAY_SIZE(imsi3),
 				      now + 2);
 		OSMO_ASSERT(peer->patch_state.enabled_tllis_count == 3);