<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-sgsn/+/15186">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">gprs_gmm.c: Call mmctx_set_(p)mm_state only on related ran_type<br><br>For new readers it's very confusing why PMM states and MM states are in<br>the same enum, but handled with different functions, and sometimes<br>called one right after the other with different enums. Calling them when<br>on a different ran_type makes the function early return, so let's better<br>conditionally call the function to make it clear in the flow when the<br>function is expected to do something.<br><br>Change-Id: I65ad9e180177bc9fc7c4a037cd85cfe33b161f73<br>---<br>M src/gprs/gprs_gmm.c<br>1 file changed, 34 insertions(+), 18 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/86/15186/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gprs/gprs_gmm.c b/src/gprs/gprs_gmm.c</span><br><span>index 61c5908..1e586c2 100644</span><br><span>--- a/src/gprs/gprs_gmm.c</span><br><span>+++ b/src/gprs/gprs_gmm.c</span><br><span>@@ -138,9 +138,6 @@</span><br><span> </span><br><span> static void mmctx_set_pmm_state(struct sgsn_mm_ctx *ctx, enum gprs_pmm_state state)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-      if (ctx->ran_type != MM_CTX_T_UTRAN_Iu)</span><br><span style="color: hsl(0, 100%, 40%);">-              return;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>      if (ctx->pmm_state == state)</span><br><span>              return;</span><br><span> </span><br><span>@@ -164,9 +161,6 @@</span><br><span> </span><br><span> static void mmctx_set_mm_state(struct sgsn_mm_ctx *ctx, enum gprs_pmm_state state)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-        if (ctx->ran_type != MM_CTX_T_GERAN_Gb)</span><br><span style="color: hsl(0, 100%, 40%);">-              return;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>      if (ctx->pmm_state == state)</span><br><span>              return;</span><br><span> </span><br><span>@@ -206,7 +200,7 @@</span><br><span>            else</span><br><span>                         LOGMMCTXP(LOGL_INFO, mm, "IU release for UE conn 0x%x\n",</span><br><span>                            ctx->conn_id);</span><br><span style="color: hsl(0, 100%, 40%);">-             if (mm && mm->pmm_state == PMM_CONNECTED)</span><br><span style="color: hsl(120, 100%, 40%);">+          if (mm && mm->ran_type == MM_CTX_T_UTRAN_Iu && mm->pmm_state == PMM_CONNECTED)</span><br><span>                         mmctx_set_pmm_state(mm, PMM_IDLE);</span><br><span>           rc = 0;</span><br><span>              break;</span><br><span>@@ -326,8 +320,17 @@</span><br><span> </span><br><span>    /* Mark MM state as deregistered */</span><br><span>  ctx->gmm_state = GMM_DEREGISTERED;</span><br><span style="color: hsl(0, 100%, 40%);">-   mmctx_set_pmm_state(ctx, PMM_DETACHED);</span><br><span style="color: hsl(0, 100%, 40%);">- mmctx_set_mm_state(ctx, MM_IDLE);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   switch(ctx->ran_type) {</span><br><span style="color: hsl(120, 100%, 40%);">+    case MM_CTX_T_UTRAN_Iu:</span><br><span style="color: hsl(120, 100%, 40%);">+               mmctx_set_pmm_state(ctx, PMM_DETACHED);</span><br><span style="color: hsl(120, 100%, 40%);">+               break;</span><br><span style="color: hsl(120, 100%, 40%);">+        case MM_CTX_T_GERAN_Gb:</span><br><span style="color: hsl(120, 100%, 40%);">+               mmctx_set_mm_state(ctx, MM_IDLE);</span><br><span style="color: hsl(120, 100%, 40%);">+             break;</span><br><span style="color: hsl(120, 100%, 40%);">+        case MM_CTX_T_GERAN_Iu:</span><br><span style="color: hsl(120, 100%, 40%);">+               break;</span><br><span style="color: hsl(120, 100%, 40%);">+        }</span><br><span> </span><br><span>        sgsn_mm_ctx_cleanup_free(ctx);</span><br><span> }</span><br><span>@@ -1092,7 +1095,8 @@</span><br><span> #ifdef BUILD_IU</span><br><span>       case GSM48_MT_GMM_SERVICE_REQ:</span><br><span>               ctx->pending_req = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-                mmctx_set_pmm_state(ctx, PMM_CONNECTED);</span><br><span style="color: hsl(120, 100%, 40%);">+              if (ctx->ran_type == MM_CTX_T_UTRAN_Iu)</span><br><span style="color: hsl(120, 100%, 40%);">+                    mmctx_set_pmm_state(ctx, PMM_CONNECTED);</span><br><span>             rc = gsm48_tx_gmm_service_ack(ctx);</span><br><span> </span><br><span>              if (ctx->iu.service.type != GPRS_SERVICE_T_SIGNALLING)</span><br><span>@@ -2073,16 +2077,22 @@</span><br><span>          mmctx->t3350_mode = GMM_T3350_MODE_NONE;</span><br><span>          mmctx->p_tmsi_old = 0;</span><br><span>            mmctx->pending_req = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-              if (mmctx->ran_type == MM_CTX_T_GERAN_Gb) {</span><br><span style="color: hsl(120, 100%, 40%);">+                mmctx->gmm_state = GMM_REGISTERED_NORMAL;</span><br><span style="color: hsl(120, 100%, 40%);">+          switch(mmctx->ran_type) {</span><br><span style="color: hsl(120, 100%, 40%);">+          case MM_CTX_T_UTRAN_Iu:</span><br><span style="color: hsl(120, 100%, 40%);">+                       mmctx_set_pmm_state(mmctx, PMM_CONNECTED);</span><br><span style="color: hsl(120, 100%, 40%);">+                    break;</span><br><span style="color: hsl(120, 100%, 40%);">+                case MM_CTX_T_GERAN_Gb:</span><br><span>                      /* Unassign the old TLLI */</span><br><span>                  mmctx->gb.tlli = mmctx->gb.tlli_new;</span><br><span>                   gprs_llme_copy_key(mmctx, mmctx->gb.llme);</span><br><span>                        gprs_llgmm_assign(mmctx->gb.llme, TLLI_UNASSIGNED,</span><br><span>                                          mmctx->gb.tlli_new);</span><br><span style="color: hsl(120, 100%, 40%);">+                     mmctx_set_mm_state(mmctx, MM_READY);</span><br><span style="color: hsl(120, 100%, 40%);">+                  break;</span><br><span style="color: hsl(120, 100%, 40%);">+                case MM_CTX_T_GERAN_Iu:</span><br><span style="color: hsl(120, 100%, 40%);">+                       break;</span><br><span>               }</span><br><span style="color: hsl(0, 100%, 40%);">-               mmctx->gmm_state = GMM_REGISTERED_NORMAL;</span><br><span style="color: hsl(0, 100%, 40%);">-            mmctx_set_pmm_state(mmctx, PMM_CONNECTED);</span><br><span style="color: hsl(0, 100%, 40%);">-              mmctx_set_mm_state(mmctx, MM_READY);</span><br><span>                 rc = 0;</span><br><span> </span><br><span>          osmo_fsm_inst_dispatch(mmctx->gmm_att_req.fsm, E_ATTACH_COMPLETE_RECV, 0);</span><br><span>@@ -2099,15 +2109,21 @@</span><br><span>              mmctx->t3350_mode = GMM_T3350_MODE_NONE;</span><br><span>          mmctx->p_tmsi_old = 0;</span><br><span>            mmctx->pending_req = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-              if (mmctx->ran_type == MM_CTX_T_GERAN_Gb) {</span><br><span style="color: hsl(120, 100%, 40%);">+                mmctx->gmm_state = GMM_REGISTERED_NORMAL;</span><br><span style="color: hsl(120, 100%, 40%);">+          switch(mmctx->ran_type) {</span><br><span style="color: hsl(120, 100%, 40%);">+          case MM_CTX_T_UTRAN_Iu:</span><br><span style="color: hsl(120, 100%, 40%);">+                       mmctx_set_pmm_state(mmctx, PMM_CONNECTED);</span><br><span style="color: hsl(120, 100%, 40%);">+                    break;</span><br><span style="color: hsl(120, 100%, 40%);">+                case MM_CTX_T_GERAN_Gb:</span><br><span>                      /* Unassign the old TLLI */</span><br><span>                  mmctx->gb.tlli = mmctx->gb.tlli_new;</span><br><span>                   gprs_llgmm_assign(mmctx->gb.llme, TLLI_UNASSIGNED,</span><br><span>                                          mmctx->gb.tlli_new);</span><br><span style="color: hsl(120, 100%, 40%);">+                     mmctx_set_mm_state(mmctx, MM_READY);</span><br><span style="color: hsl(120, 100%, 40%);">+                  break;</span><br><span style="color: hsl(120, 100%, 40%);">+                case MM_CTX_T_GERAN_Iu:</span><br><span style="color: hsl(120, 100%, 40%);">+                       break;</span><br><span>               }</span><br><span style="color: hsl(0, 100%, 40%);">-               mmctx->gmm_state = GMM_REGISTERED_NORMAL;</span><br><span style="color: hsl(0, 100%, 40%);">-            mmctx_set_pmm_state(mmctx, PMM_CONNECTED);</span><br><span style="color: hsl(0, 100%, 40%);">-              mmctx_set_mm_state(mmctx, MM_READY);</span><br><span>                 rc = 0;</span><br><span> </span><br><span>          memset(&sig_data, 0, sizeof(sig_data));</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-sgsn/+/15186">change 15186</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-sgsn/+/15186"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-sgsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I65ad9e180177bc9fc7c4a037cd85cfe33b161f73 </div>
<div style="display:none"> Gerrit-Change-Number: 15186 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>