sgsn: Reorganize and fix gsm48_gmm_authorize

Currently the order of the 'if' clauses in gsm48_gmm_authorize
doesn't match the order in which the conditional parts are entered.
This makes it difficult to maintain. In addition the t3350_mode is
not stored in every path, so that this information is lost when the
identification procedure is started. Since the default value
coincidentally is GMM_T3350_MODE_ATT, this doesn't hurt for Attach
Requests which are the only messages that initially trigger the
authentication yet.

This patch changes the order of the 'if' clause to match the
processing order, it removes the t3350_mode parameter entirely and
introduces a mm->pending_req field. The latter must be set when the
request that causes the authorization before calling
gsm48_gmm_authorize. The gprs_t3350_mode enum is extended by
GMM_T3350_MODE_NONE (value 0, which is the default) to make it
possible to detect related initialisation errors or race conditions.

Sponsored-by: On-Waves ehf
diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index 73731fe..75797db 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -48,6 +48,7 @@
 };
 
 enum gprs_t3350_mode {
+	GMM_T3350_MODE_NONE,
 	GMM_T3350_MODE_ATT,
 	GMM_T3350_MODE_RAU,
 	GMM_T3350_MODE_PTMSI_REALL,
@@ -111,6 +112,11 @@
 
 	enum gprs_t3350_mode	t3350_mode;
 	uint8_t			t3370_id_type;
+	uint8_t			pending_req;	/* the request's message type */
+	/* TODO: There isn't much semantic difference between t3350_mode
+	 * (refers to the timer) and pending_req (refers to the procedure),
+	 * where mm->T == 3350 => mm->t3350_mode == f(mm->pending_req). Check
+	 * whether one of them can be dropped. */
 };
 
 #define LOGMMCTXP(level, mm, fmt, args...) \
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index a5f9b78..e661b48 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -625,27 +625,15 @@
 }
 
 /* Check if we can already authorize a subscriber */
-static int gsm48_gmm_authorize(struct sgsn_mm_ctx *ctx,
-				enum gprs_t3350_mode t3350_mode)
+static int gsm48_gmm_authorize(struct sgsn_mm_ctx *ctx)
 {
-	if (strlen(ctx->imei) && strlen(ctx->imsi)) {
-#ifdef PTMSI_ALLOC
-		ctx->t3370_id_type = GSM_MI_TYPE_NONE;
-
-		/* Start T3350 and re-transmit up to 5 times until ATTACH COMPLETE */
-		ctx->t3350_mode = t3350_mode;
-		mmctx_timer_start(ctx, 3350, GSM0408_T3350_SECS);
-#endif
-		ctx->mm_state = GMM_REGISTERED_NORMAL;
-		return gsm48_tx_gmm_att_ack(ctx);
-	} 
+	/* Request IMSI and IMEI from the MS if they are unknown */
 	if (!strlen(ctx->imei)) {
 		ctx->mm_state = GMM_COMMON_PROC_INIT;
 		ctx->t3370_id_type = GSM_MI_TYPE_IMEI;
 		mmctx_timer_start(ctx, 3370, GSM0408_T3370_SECS);
 		return gsm48_tx_gmm_id_req(ctx, GSM_MI_TYPE_IMEI);
 	}
-
 	if (!strlen(ctx->imsi)) {
 		ctx->mm_state = GMM_COMMON_PROC_INIT;
 		ctx->t3370_id_type = GSM_MI_TYPE_IMSI;
@@ -653,6 +641,31 @@
 		return gsm48_tx_gmm_id_req(ctx, GSM_MI_TYPE_IMSI);
 	}
 
+	/* All information required for authentication is available */
+
+	ctx->t3370_id_type = GSM_MI_TYPE_NONE;
+
+	switch (ctx->pending_req) {
+	case 0:
+		LOGMMCTXP(LOGL_INFO, ctx,
+			  "no pending request, authorization completed\n");
+		break;
+	case GSM48_MT_GMM_ATTACH_REQ:
+#ifdef PTMSI_ALLOC
+		/* Start T3350 and re-transmit up to 5 times until ATTACH COMPLETE */
+		mmctx_timer_start(ctx, 3350, GSM0408_T3350_SECS);
+		ctx->t3350_mode = GMM_T3350_MODE_ATT;
+#endif
+
+		ctx->mm_state = GMM_REGISTERED_NORMAL;
+		return gsm48_tx_gmm_att_ack(ctx);
+	default:
+		LOGMMCTXP(LOGL_ERROR, ctx,
+			  "only Attach Request is supported yet, "
+			  "got request type %u\n", ctx->pending_req);
+		break;
+	}
+
 	return 0;
 }
 
@@ -715,7 +728,7 @@
 	}
 
 	/* Check if we can let the mobile station enter */
-	return gsm48_gmm_authorize(ctx, ctx->t3350_mode);
+	return gsm48_gmm_authorize(ctx);
 }
 
 /* Section 9.4.1 Attach request */
@@ -856,7 +869,8 @@
 	gprs_llgmm_assign(ctx->llme, ctx->tlli, ctx->tlli_new,
 			  GPRS_ALGO_GEA0, NULL);
 
-	return gsm48_gmm_authorize(ctx, GMM_T3350_MODE_ATT);
+	ctx->pending_req = GSM48_MT_GMM_ATTACH_REQ;
+	return gsm48_gmm_authorize(ctx);
 
 err_inval:
 	LOGPC(DMM, LOGL_INFO, "\n");
@@ -1168,7 +1182,9 @@
 		/* only in case SGSN offered new P-TMSI */
 		LOGMMCTXP(LOGL_INFO, mmctx, "-> ATTACH COMPLETE\n");
 		mmctx_timer_stop(mmctx, 3350);
+		mmctx->t3350_mode = GMM_T3350_MODE_NONE;
 		mmctx->p_tmsi_old = 0;
+		mmctx->pending_req = 0;
 		/* Unassign the old TLLI */
 		mmctx->tlli = mmctx->tlli_new;
 		gprs_llgmm_assign(mmctx->llme, 0xffffffff, mmctx->tlli_new,
@@ -1179,7 +1195,9 @@
 		/* only in case SGSN offered new P-TMSI */
 		LOGMMCTXP(LOGL_INFO, mmctx, "-> ROUTEING AREA UPDATE COMPLETE\n");
 		mmctx_timer_stop(mmctx, 3350);
+		mmctx->t3350_mode = GMM_T3350_MODE_NONE;
 		mmctx->p_tmsi_old = 0;
+		mmctx->pending_req = 0;
 		/* Unassign the old TLLI */
 		mmctx->tlli = mmctx->tlli_new;
 		gprs_llgmm_assign(mmctx->llme, 0xffffffff, mmctx->tlli_new,
@@ -1189,7 +1207,9 @@
 	case GSM48_MT_GMM_PTMSI_REALL_COMPL:
 		LOGMMCTXP(LOGL_INFO, mmctx, "-> PTMSI REALLLICATION COMPLETE\n");
 		mmctx_timer_stop(mmctx, 3350);
+		mmctx->t3350_mode = GMM_T3350_MODE_NONE;
 		mmctx->p_tmsi_old = 0;
+		mmctx->pending_req = 0;
 		/* Unassign the old TLLI */
 		mmctx->tlli = mmctx->tlli_new;
 		//gprs_llgmm_assign(mmctx->llme, 0xffffffff, mmctx->tlli_new, GPRS_ALGO_GEA0, NULL);
@@ -1219,6 +1239,8 @@
 		if (mm->num_T_exp >= 5) {
 			LOGMMCTXP(LOGL_NOTICE, mm, "T3350 expired >= 5 times\n");
 			mm->mm_state = GMM_DEREGISTERED;
+			mm->t3350_mode = GMM_T3350_MODE_NONE;
+			mm->pending_req = 0;
 			/* FIXME: should we return some error? */
 			break;
 		}
@@ -1233,6 +1255,10 @@
 		case GMM_T3350_MODE_PTMSI_REALL:
 			/* FIXME */
 			break;
+		case GMM_T3350_MODE_NONE:
+			LOGMMCTXP(LOGL_NOTICE, mm,
+				  "T3350 mode wasn't set, ignoring timeout\n");
+			break;
 		}
 		osmo_timer_schedule(&mm->timer, GSM0408_T3350_SECS, 0);
 		break;