<p>Neels Hofmeyr has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/11662">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bsc: check channel release message presence<br><br>Instead of vaguely allowing any release messages to be present or not, exactly<br>pinpoint for each test case the exact release messages expected during lchan<br>release.<br><br>Related: an osmo-bsc change broke sending of RR Release messages, which was<br>utterly ignored and hence not caught by ttcn tests. That must not happen again.<br><br>I am not actually sure that these expectations are 100% correct; if errors<br>become apparent, we shall change the expectations in ttcn3 and then fix<br>osmo-bsc according to that.<br><br>Adjust f_expect_chan_rel() and callers,<br>and the Assignment procedures (as_assignment and f_establish_fully).<br><br>The current state of the bsc tests should all pass with osmo-bsc<br>Id3301df059582da2377ef82feae554e94fa42035<br><br>Related: OS#3413<br>Change-Id: Ibc64058f1e214bea585f4e8dcb66f3df8ead3845<br>---<br>M bsc/BSC_Tests.ttcn<br>M bsc/MSC_ConnectionHandler.ttcn<br>2 files changed, 53 insertions(+), 17 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/62/11662/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/bsc/BSC_Tests.ttcn b/bsc/BSC_Tests.ttcn</span><br><span>index b1f3a31..e06f496 100644</span><br><span>--- a/bsc/BSC_Tests.ttcn</span><br><span>+++ b/bsc/BSC_Tests.ttcn</span><br><span>@@ -429,7 +429,7 @@</span><br><span> </span><br><span>      /* expect BSC to disable the channel again if there's no response from MSC */</span><br><span>    /* MS waits 20s (T3210) at LU; 10s (T3230) at CM SERV REQ and 5s (T3220) AT detach */</span><br><span style="color: hsl(0, 100%, 40%);">-   f_expect_chan_rel(0, chan_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+        f_expect_chan_rel(0, chan_nr, expect_rll_rel_req := false);</span><br><span>  setverdict(pass);</span><br><span> }</span><br><span> </span><br><span>@@ -450,7 +450,7 @@</span><br><span>     BSSAP.send(ts_BSSAP_DISC_req(rx_c_ind.connectionId, 0));</span><br><span> </span><br><span>         /* expect BSC to disable the channel */</span><br><span style="color: hsl(0, 100%, 40%);">- f_expect_chan_rel(0, chan_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+        f_expect_chan_rel(0, chan_nr, expect_rll_rel_req := false);</span><br><span>  setverdict(pass);</span><br><span> }</span><br><span> </span><br><span>@@ -817,32 +817,38 @@</span><br><span> }</span><br><span> </span><br><span> function f_expect_chan_rel(integer bts_nr, RslChannelNr rsl_chan_nr,</span><br><span style="color: hsl(0, 100%, 40%);">-                        boolean handle_rll_rel := true) runs on test_CT {</span><br><span style="color: hsl(120, 100%, 40%);">+                     boolean expect_deact_sacch := true,</span><br><span style="color: hsl(120, 100%, 40%);">+                           boolean expect_rr_chan_rel := true,</span><br><span style="color: hsl(120, 100%, 40%);">+                           boolean expect_rll_rel_req := true,</span><br><span style="color: hsl(120, 100%, 40%);">+                           boolean handle_rll_rel := true</span><br><span style="color: hsl(120, 100%, 40%);">+                        ) runs on test_CT {</span><br><span> </span><br><span>   var RslLinkId main_dcch := valueof(ts_RslLinkID_DCCH(0));</span><br><span>    var octetstring l3_rr_chan_rel := '060D00'O;</span><br><span style="color: hsl(120, 100%, 40%);">+  var boolean got_deact_sacch := false;</span><br><span style="color: hsl(120, 100%, 40%);">+ var boolean got_rr_chan_rel := false;</span><br><span style="color: hsl(120, 100%, 40%);">+ var boolean got_rll_rel_req := false;</span><br><span style="color: hsl(120, 100%, 40%);">+ log("f_expect_chan_rel() expecting: expect_deact_sacch=", expect_deact_sacch, " expect_rr_chan_rel=", expect_rr_chan_rel,</span><br><span style="color: hsl(120, 100%, 40%);">+     " expect_rll_rel_req=", expect_rll_rel_req);</span><br><span>   alt {</span><br><span style="color: hsl(0, 100%, 40%);">-   /* ignore DEACTIVATE SACCH (if any) */</span><br><span>       [] IPA_RSL[bts_nr].receive(tr_ASP_RSL_UD(IPAC_PROTO_RSL_TRX0,</span><br><span>                                        tr_RSL_DEACT_SACCH(rsl_chan_nr))) {</span><br><span style="color: hsl(120, 100%, 40%);">+           got_deact_sacch := true;</span><br><span>             repeat;</span><br><span>      }</span><br><span>    [] IPA_RSL[bts_nr].receive(tr_ASP_RSL_UD(IPAC_PROTO_RSL_TRX0, tr_RSL_DATA_REQ(rsl_chan_nr, ?, l3_rr_chan_rel))) {</span><br><span style="color: hsl(120, 100%, 40%);">+             got_rr_chan_rel := true;</span><br><span>             repeat;</span><br><span>      }</span><br><span style="color: hsl(0, 100%, 40%);">-       /* acknowledge RLL release (if any)*/</span><br><span style="color: hsl(0, 100%, 40%);">-   [handle_rll_rel] IPA_RSL[bts_nr].receive(tr_ASP_RSL_UD(IPAC_PROTO_RSL_TRX0,</span><br><span style="color: hsl(120, 100%, 40%);">+   [] IPA_RSL[bts_nr].receive(tr_ASP_RSL_UD(IPAC_PROTO_RSL_TRX0,</span><br><span>                                        tr_RSL_REL_REQ(rsl_chan_nr, ?))) {</span><br><span style="color: hsl(120, 100%, 40%);">+            got_rll_rel_req := true;</span><br><span>             /* FIXME: Why are we getting this for LinkID SACCH? */</span><br><span style="color: hsl(0, 100%, 40%);">-          f_ipa_tx(0, ts_RSL_REL_CONF(rsl_chan_nr, main_dcch));</span><br><span style="color: hsl(120, 100%, 40%);">+         if (handle_rll_rel) {</span><br><span style="color: hsl(120, 100%, 40%);">+                 f_ipa_tx(0, ts_RSL_REL_CONF(rsl_chan_nr, main_dcch));</span><br><span style="color: hsl(120, 100%, 40%);">+         }</span><br><span>            repeat;</span><br><span>      }</span><br><span style="color: hsl(0, 100%, 40%);">-       [not handle_rll_rel] IPA_RSL[bts_nr].receive(tr_ASP_RSL_UD(IPAC_PROTO_RSL_TRX0,</span><br><span style="color: hsl(0, 100%, 40%);">-                                 tr_RSL_REL_REQ(rsl_chan_nr, ?))) {</span><br><span style="color: hsl(0, 100%, 40%);">-              /* Do not reply, just continue */</span><br><span style="color: hsl(0, 100%, 40%);">-               repeat;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-       /* Expect RF channel release from BSC on Abis */</span><br><span>     [] IPA_RSL[bts_nr].receive(tr_ASP_RSL_UD(IPAC_PROTO_RSL_TRX0,</span><br><span>                                                tr_RSL_MsgTypeD(RSL_MT_RF_CHAN_REL))) {</span><br><span>              /* respond with CHAN REL ACK */</span><br><span>@@ -853,6 +859,19 @@</span><br><span>               repeat;</span><br><span>              }</span><br><span>    }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   log("f_expect_chan_rel() summary: got_deact_sacch=", got_deact_sacch, " got_rr_chan_rel=", got_rr_chan_rel,</span><br><span style="color: hsl(120, 100%, 40%);">+           " got_rll_rel_req=", got_rll_rel_req);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+        if (expect_deact_sacch != got_deact_sacch) {</span><br><span style="color: hsl(120, 100%, 40%);">+          setverdict(fail, "f_expect_chan_rel(): expect_deact_sacch=", expect_deact_sacch, " got_deact_sacch=", got_deact_sacch);</span><br><span style="color: hsl(120, 100%, 40%);">+   }</span><br><span style="color: hsl(120, 100%, 40%);">+     if (expect_rr_chan_rel != got_rr_chan_rel) {</span><br><span style="color: hsl(120, 100%, 40%);">+          setverdict(fail, "f_expect_chan_rel(): expect_rr_chan_rel=", expect_rr_chan_rel, " got_rr_chan_rel=", got_rr_chan_rel);</span><br><span style="color: hsl(120, 100%, 40%);">+   }</span><br><span style="color: hsl(120, 100%, 40%);">+     if (expect_rll_rel_req != got_rll_rel_req) {</span><br><span style="color: hsl(120, 100%, 40%);">+          setverdict(fail, "f_expect_chan_rel(): expect_rll_rel_req=", expect_rll_rel_req, " got_rll_rel_req=", got_rll_rel_req);</span><br><span style="color: hsl(120, 100%, 40%);">+   }</span><br><span> }</span><br><span> </span><br><span> /* Test behavior of channel release after hard Clear Command from MSC */</span><br><span>@@ -874,7 +893,7 @@</span><br><span>                 BSSAP.send(ts_BSSAP_DISC_req(dt.sccp_conn_id, 0));</span><br><span>   }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   f_expect_chan_rel(0, dt.rsl_chan_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ f_expect_chan_rel(0, dt.rsl_chan_nr, expect_rll_rel_req := false);</span><br><span>   setverdict(pass);</span><br><span> }</span><br><span> </span><br><span>@@ -889,7 +908,7 @@</span><br><span>     /* release the SCCP connection */</span><br><span>    BSSAP.send(ts_BSSAP_DISC_req(dt.sccp_conn_id, 0));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  f_expect_chan_rel(0, dt.rsl_chan_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ f_expect_chan_rel(0, dt.rsl_chan_nr, expect_rll_rel_req := false);</span><br><span>   setverdict(pass);</span><br><span> }</span><br><span> </span><br><span>@@ -904,7 +923,7 @@</span><br><span>     /* release the SCCP connection */</span><br><span>    BSSAP.send(ts_BSSAP_DISC_req(dt.sccp_conn_id, 0));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  f_expect_chan_rel(0, dt.rsl_chan_nr, handle_rll_rel := false);</span><br><span style="color: hsl(120, 100%, 40%);">+        f_expect_chan_rel(0, dt.rsl_chan_nr, expect_rll_rel_req := false);</span><br><span>   setverdict(pass);</span><br><span> }</span><br><span> </span><br><span>@@ -926,7 +945,7 @@</span><br><span>     [] BSSAP.receive(tr_BSSAP_DISC_ind(dt.sccp_conn_id, ?, ?)) { }</span><br><span>       }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   f_expect_chan_rel(0, dt.rsl_chan_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ f_expect_chan_rel(0, dt.rsl_chan_nr, expect_rll_rel_req := false);</span><br><span>   setverdict(pass);</span><br><span> }</span><br><span> </span><br><span>diff --git a/bsc/MSC_ConnectionHandler.ttcn b/bsc/MSC_ConnectionHandler.ttcn</span><br><span>index 07eafc7..116d7af 100644</span><br><span>--- a/bsc/MSC_ConnectionHandler.ttcn</span><br><span>+++ b/bsc/MSC_ConnectionHandler.ttcn</span><br><span>@@ -603,6 +603,8 @@</span><br><span>        boolean is_assignment,</span><br><span>       /* Assignment related bits */</span><br><span>        boolean rr_ass_cmpl_seen,</span><br><span style="color: hsl(120, 100%, 40%);">+     boolean old_lchan_deact_sacch_seen,</span><br><span style="color: hsl(120, 100%, 40%);">+   boolean old_lchan_rll_rel_req_seen,</span><br><span>  boolean assignment_done,</span><br><span>     RslChannelNr old_chan_nr,</span><br><span>    /* Modify related bits */</span><br><span>@@ -614,6 +616,8 @@</span><br><span>      voice_call := false,</span><br><span>         is_assignment := false,</span><br><span>      rr_ass_cmpl_seen := false,</span><br><span style="color: hsl(120, 100%, 40%);">+    old_lchan_deact_sacch_seen := false,</span><br><span style="color: hsl(120, 100%, 40%);">+  old_lchan_rll_rel_req_seen := false,</span><br><span>         assignment_done := false,</span><br><span>    old_chan_nr := -,</span><br><span>    rr_modify_seen := false,</span><br><span>@@ -683,9 +687,11 @@</span><br><span>              }</span><br><span>            }</span><br><span>    [st.rr_ass_cmpl_seen] RSL.receive(tr_RSL_DEACT_SACCH(st.old_chan_nr)) {</span><br><span style="color: hsl(120, 100%, 40%);">+               st.old_lchan_deact_sacch_seen := true;</span><br><span>               repeat;</span><br><span>              }</span><br><span>    [st.rr_ass_cmpl_seen] RSL.receive(tr_RSL_REL_REQ(st.old_chan_nr, tr_RslLinkID_DCCH(0))) {</span><br><span style="color: hsl(120, 100%, 40%);">+             st.old_lchan_rll_rel_req_seen := true;</span><br><span>               RSL.send(ts_RSL_REL_CONF(st.old_chan_nr, valueof(ts_RslLinkID_DCCH(0))));</span><br><span>            repeat;</span><br><span>              }</span><br><span>@@ -994,6 +1000,17 @@</span><br><span>    }</span><br><span> </span><br><span>        f_check_mgcp_expectations();</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+        if (st.is_assignment and st.assignment_done) {</span><br><span style="color: hsl(120, 100%, 40%);">+            if (not st.old_lchan_deact_sacch_seen) {</span><br><span style="color: hsl(120, 100%, 40%);">+          setverdict(fail, "f_establish_fully(): Assignment completed, but the old lchan was not",</span><br><span style="color: hsl(120, 100%, 40%);">+                       " released properly: expected a Deact SACCH on the old lchan, but saw none.");</span><br><span style="color: hsl(120, 100%, 40%);">+       }</span><br><span style="color: hsl(120, 100%, 40%);">+     if (st.old_lchan_rll_rel_req_seen) {</span><br><span style="color: hsl(120, 100%, 40%);">+              setverdict(fail, "f_establish_fully(): Assignment completed, but the old lchan was not",</span><br><span style="color: hsl(120, 100%, 40%);">+                       " released properly: saw an RLL Release on the old lchan, but expecting none.");</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> type record HandoverState {</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11662">change 11662</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/11662"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-ttcn3-hacks </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ibc64058f1e214bea585f4e8dcb66f3df8ead3845 </div>
<div style="display:none"> Gerrit-Change-Number: 11662 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>