sgsn: Add sgsn_mm_ctx_cleanup_free for safe shutdown

Currently the MM context cleanup code is distributed over several
functions. sgsn_mm_ctx_free not only frees data structure but also
eventually stops the timer and does the subscriber clean-up.
mm_ctx_cleanup_free (gprs_gmm.c) cleans up the PDP contexts and
unassign the TLLI.

This commit moves the cleanup code from both functions into a new
unifying function sgsn_mm_ctx_cleanup_free that cares about the
clean-up of all related sub-systems.

Sponsored-by: On-Waves ehf

4 files changed, 35 insertions, 39 deletions

diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index ce73e0189..f566ab9f9 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -150,6 +150,7 @@ struct sgsn_mm_ctx *sgsn_mm_ctx_by_imsi(const char *imsi);
 struct sgsn_mm_ctx *sgsn_mm_ctx_alloc(uint32_t tlli,
 					const struct gprs_ra_id *raid);
 void sgsn_mm_ctx_free(struct sgsn_mm_ctx *mm);
+void sgsn_mm_ctx_cleanup_free(struct sgsn_mm_ctx *ctx);
 
 
 enum pdp_ctx_state {
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index 03773a61a..abda327f0 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -181,34 +181,14 @@ static void mmctx2msgid(struct msgb *msg, const struct sgsn_mm_ctx *mm)
 	msgb_nsei(msg) = mm->nsei;
 }
 
-static void delete_pdp_contexts(struct sgsn_mm_ctx *ctx, const char *log_text)
-{
-	struct sgsn_pdp_ctx *pdp, *pdp2;
-
-	/* delete all existing PDP contexts for this MS */
-	llist_for_each_entry_safe(pdp, pdp2, &ctx->pdp_list, list) {
-		LOGMMCTXP(LOGL_NOTICE, ctx,
-			  "Dropping PDP context for NSAPI=%u due to %s\n",
-			  pdp->nsapi, log_text);
-		sgsn_pdp_ctx_terminate(pdp);
-	}
-}
-
 static void mm_ctx_cleanup_free(struct sgsn_mm_ctx *ctx, const char *log_text)
 {
-	struct gprs_llc_llme *llme = ctx->llme;
-	uint32_t tlli = ctx->tlli;
+	LOGMMCTXP(LOGL_INFO, ctx, "Cleaning MM context due to %s\n", log_text);
 
 	/* Mark MM state as deregistered */
 	ctx->mm_state = GMM_DEREGISTERED;
 
-	delete_pdp_contexts(ctx, log_text);
-
-	sgsn_mm_ctx_free(ctx);
-	ctx = NULL;
-
-	/* TLLI unassignment, must be called after sgsn_mm_ctx_free */
-	gprs_llgmm_assign(llme, tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
+	sgsn_mm_ctx_cleanup_free(ctx);
 }
 
 /* Chapter 9.4.18 */
diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c
index 14b925400..555be57f5 100644
--- a/openbsc/src/gprs/gprs_sgsn.c
+++ b/openbsc/src/gprs/gprs_sgsn.c
@@ -186,14 +186,36 @@ void sgsn_mm_ctx_free(struct sgsn_mm_ctx *mm)
 {
 	struct sgsn_pdp_ctx *pdp, *pdp2;
 
+	/* Unlink from global list of MM contexts */
+	llist_del(&mm->list);
+
+	/* Free all PDP contexts */
+	llist_for_each_entry_safe(pdp, pdp2, &mm->pdp_list, list)
+		sgsn_pdp_ctx_free(pdp);
+
+	rate_ctr_group_free(mm->ctrg);
+
+	talloc_free(mm);
+}
+
+void sgsn_mm_ctx_cleanup_free(struct sgsn_mm_ctx *mm)
+{
+	struct gprs_llc_llme *llme = mm->llme;
+	uint32_t tlli = mm->tlli;
+	struct sgsn_pdp_ctx *pdp, *pdp2;
+
+	/* delete all existing PDP contexts for this MS */
+	llist_for_each_entry_safe(pdp, pdp2, &mm->pdp_list, list) {
+		LOGMMCTXP(LOGL_NOTICE, mm,
+			  "Dropping PDP context for NSAPI=%u\n", pdp->nsapi);
+		sgsn_pdp_ctx_terminate(pdp);
+	}
+
 	if (osmo_timer_pending(&mm->timer)) {
 		LOGMMCTXP(LOGL_INFO, mm, "Cancelling MM timer %u\n", mm->T);
 		osmo_timer_del(&mm->timer);
 	}
 
-	/* Unlink from global list of MM contexts */
-	llist_del(&mm->list);
-
 	/* Detach from subscriber which is possibly freed then */
 	if (mm->subscr) {
 		struct gsm_subscriber *subscr = subscr_get(mm->subscr);
@@ -201,15 +223,14 @@ void sgsn_mm_ctx_free(struct sgsn_mm_ctx *mm)
 		subscr_put(subscr);
 	}
 
-	/* Free all PDP contexts */
-	llist_for_each_entry_safe(pdp, pdp2, &mm->pdp_list, list)
-		sgsn_pdp_ctx_free(pdp);
-	
-	rate_ctr_group_free(mm->ctrg);
+	sgsn_mm_ctx_free(mm);
+	mm = NULL;
 
-	talloc_free(mm);
+	/* TLLI unassignment, must be called after sgsn_mm_ctx_free */
+	gprs_llgmm_assign(llme, tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
 }
 
+
 /* look up PDP context by MM context and NSAPI */
 struct sgsn_pdp_ctx *sgsn_pdp_ctx_by_nsapi(const struct sgsn_mm_ctx *mm,
 					   uint8_t nsapi)
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index da7da855f..733380a59 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -306,7 +306,6 @@ static void test_auth_triplets(void)
 	struct sgsn_mm_ctx *ctx;
 	struct gprs_ra_id raid = { 0, };
 	uint32_t local_tlli = 0xffeeddcc;
-	struct gprs_llc_llme *llme;
 
 	printf("Testing authentication triplet handling\n");
 
@@ -355,11 +354,9 @@ static void test_auth_triplets(void)
 
 	/* Free MM context and subscriber */
 	subscr_put(s1);
-	llme = ctx->llme;
-	sgsn_mm_ctx_free(ctx);
+	sgsn_mm_ctx_cleanup_free(ctx);
 	s1found = gprs_subscr_get_by_imsi(imsi1);
 	OSMO_ASSERT(s1found == NULL);
-	gprs_llgmm_assign(llme, local_tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
 }
 
 #define TEST_GSUP_IMSI1_IE 0x01, 0x05, 0x21, 0x43, 0x65, 0x87, 0x09
@@ -595,7 +592,6 @@ static void test_subscriber_blocking(void)
 	struct sgsn_mm_ctx *ctx;
 	struct gprs_ra_id raid = { 0, };
 	uint32_t local_tlli = 0xffeeddcc;
-	struct gprs_llc_llme *llme;
 	int rc;
 
 	printf("Testing subcriber procedure blocking\n");
@@ -609,7 +605,6 @@ static void test_subscriber_blocking(void)
 	/* Create a context */
 	OSMO_ASSERT(count(gprs_llme_list()) == 0);
 	ctx = alloc_mm_ctx(local_tlli, &raid);
-	llme = ctx->llme;
 	strncpy(ctx->imsi, imsi1, sizeof(ctx->imsi) - 1);
 
 	/* Allocate and attach a subscriber */
@@ -661,8 +656,7 @@ static void test_subscriber_blocking(void)
 	OSMO_ASSERT(rc == 0);
 
 	subscr_put(s1);
-	sgsn_mm_ctx_free(ctx);
-	gprs_llgmm_assign(llme, local_tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
+	sgsn_mm_ctx_cleanup_free(ctx);
 
 	assert_no_subscrs();