<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/19214">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">RR Channel Release: pass Cause code from BSSMAP Clear to the BTS<br><br>In lchan.release, add 'cause_rr', and set RR Channel Release message's cause<br>value to lchan.release.cause_rr.<br><br>In lchan_release(), do not set lchan.release.rsl_error_cause to the RR cause<br>value, these are unrelated. Store in new lchan.release.cause_rr instead. The<br>rsl_error_cause is apparently only used for logging, except for one place in<br>lchan_fsm_wait_activ_ack() that compares it to RSL_ERR_RCH_ALR_ACTV_ALLOC, so<br>there should not be a functional difference by this fix.<br><br>Propagate the BSSMAP Clear Command cause to the RR Channel Release:<br><br>Add struct gscon_clear_cmd_data as event data for GSCON_EV_A_CLEAR_CMD -- so<br>far it sent the is_csfb flag, add the gsm0808_cause; invoking the event happens<br>in bssmap_handle_clear_cmd().<br><br>Adjust event handling in gscon_fsm_allstate(); there, pass the cause to<br>gscon_release_lchans(). In gscon_release_lchans(), pass the cause to<br>gscon_release_lchan(), and then lchan_release(), which sets the new<br>lchan.release.cause_rr to the passed cause value.<br><br>As soon as the lchan FSM enters the proper state, it calls<br>gsm48_send_rr_release(). There, set the cause value in the encoded message to<br>lchan.release.cause_rr.<br><br>Interworking with osmo-msc: so far, osmo-msc fails to set the Clear Command<br>cause code for normal release, it just passes 0 which amounts to<br>GSM0808_CAUSE_RADIO_INTERFACE_MESSAGE_FAILURE. Before this patch, osmo-bsc<br>always sent GSM48_RR_CAUSE_NORMAL in the RR Channel Release, and after this<br>patch it will receive 0 == GSM0808_CAUSE_RADIO_INTERFACE_MESSAGE_FAILURE from<br>osmo-msc and more accurately translate that to GSM48_RR_CAUSE_PROT_ERROR_UNSPC.<br>This means in practice that we will now see an error cause in RR Channel<br>Release instead of GSM48_RR_CAUSE_NORMAL when working with osmo-msc. For<br>changing osmo-msc to send GSM0808_CAUSE_CALL_CONTROL instead (which translates<br>to GSM48_RR_CAUSE_NORMAL), see OS#4664 and change-id<br>I1347ed72ae7d7ea73a557b866e764819c5ef8c42 (osmo-msc).<br><br>A test for this is in Ie6c99f28b610a67f2d59ec00b3541940e882251b<br>(osmo-ttcn3-hacks).<br><br>Related: SYS#4872<br>Change-Id: I734cc55c501d61bbdadee81a223b26f9df57f959<br>---<br>M include/osmocom/bsc/bsc_subscr_conn_fsm.h<br>M include/osmocom/bsc/gsm_data.h<br>M src/osmo-bsc/bsc_subscr_conn_fsm.c<br>M src/osmo-bsc/gsm_04_08_rr.c<br>M src/osmo-bsc/gsm_data.c<br>M src/osmo-bsc/lchan_fsm.c<br>M src/osmo-bsc/osmo_bsc_bssap.c<br>7 files changed, 65 insertions(+), 13 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/bsc/bsc_subscr_conn_fsm.h b/include/osmocom/bsc/bsc_subscr_conn_fsm.h</span><br><span>index 5475272..1a827d9 100644</span><br><span>--- a/include/osmocom/bsc/bsc_subscr_conn_fsm.h</span><br><span>+++ b/include/osmocom/bsc/bsc_subscr_conn_fsm.h</span><br><span>@@ -1,4 +1,6 @@</span><br><span> #pragma once</span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/gsm/protocol/gsm_08_08.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/gsm/protocol/gsm_04_08.h></span><br><span> #include <osmocom/core/fsm.h></span><br><span> </span><br><span> enum gscon_fsm_event {</span><br><span>@@ -40,6 +42,11 @@</span><br><span> GSCON_EV_FORGET_MGW_ENDPOINT,</span><br><span> };</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct gscon_clear_cmd_data {</span><br><span style="color: hsl(120, 100%, 40%);">+ enum gsm0808_cause cause_0808;</span><br><span style="color: hsl(120, 100%, 40%);">+ bool is_csfb;</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> struct gsm_subscriber_connection;</span><br><span> struct gsm_network;</span><br><span> struct msgb;</span><br><span>@@ -71,7 +78,7 @@</span><br><span> struct assignment_request *req);</span><br><span> </span><br><span> void gscon_change_primary_lchan(struct gsm_subscriber_connection *conn, struct gsm_lchan *new_lchan);</span><br><span style="color: hsl(0, 100%, 40%);">-void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool do_rr_release);</span><br><span style="color: hsl(120, 100%, 40%);">+void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool do_rr_release, enum gsm48_rr_cause cause_rr);</span><br><span> </span><br><span> void gscon_lchan_releasing(struct gsm_subscriber_connection *conn, struct gsm_lchan *lchan);</span><br><span> void gscon_forget_lchan(struct gsm_subscriber_connection *conn, struct gsm_lchan *lchan);</span><br><span>diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h</span><br><span>index 5336ebe..43d7040 100644</span><br><span>--- a/include/osmocom/bsc/gsm_data.h</span><br><span>+++ b/include/osmocom/bsc/gsm_data.h</span><br><span>@@ -571,6 +571,7 @@</span><br><span> * flag, so that the lchan will gracefully release at the next sensible junction. */</span><br><span> bool requested;</span><br><span> bool do_rr_release;</span><br><span style="color: hsl(120, 100%, 40%);">+ enum gsm48_rr_cause rr_cause;</span><br><span> </span><br><span> /* There is an RSL error cause of value 0, so we need a separate flag. */</span><br><span> bool in_error;</span><br><span>@@ -1896,4 +1897,6 @@</span><br><span> </span><br><span> bool trx_has_valid_pchan_config(const struct gsm_bts_trx *trx);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+enum gsm48_rr_cause bsc_gsm48_rr_cause_from_gsm0808_cause(enum gsm0808_cause c);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #endif /* _GSM_DATA_H */</span><br><span>diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c</span><br><span>index 1d30246..0ed6715 100644</span><br><span>--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c</span><br><span>+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c</span><br><span>@@ -182,14 +182,14 @@</span><br><span> lchan_release(lchan, do_rr_release, err, cause_rr);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool do_rr_release)</span><br><span style="color: hsl(120, 100%, 40%);">+void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool do_rr_release, enum gsm48_rr_cause cause_rr)</span><br><span> {</span><br><span> if (conn->ho.fi)</span><br><span> handover_end(conn, HO_RESULT_CONN_RELEASE);</span><br><span> </span><br><span> assignment_reset(conn);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- gscon_release_lchan(conn, conn->lchan, do_rr_release, false, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ gscon_release_lchan(conn, conn->lchan, do_rr_release, false, cause_rr);</span><br><span> }</span><br><span> </span><br><span> static void handle_bssap_n_connect(struct osmo_fsm_inst *fi, struct osmo_scu_prim *scu_prim)</span><br><span>@@ -746,17 +746,20 @@</span><br><span> static void gscon_fsm_allstate(struct osmo_fsm_inst *fi, uint32_t event, void *data)</span><br><span> {</span><br><span> struct gsm_subscriber_connection *conn = fi->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gscon_clear_cmd_data *ccd;</span><br><span> </span><br><span> /* Regular allstate event processing */</span><br><span> switch (event) {</span><br><span> case GSCON_EV_A_CLEAR_CMD:</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(data);</span><br><span style="color: hsl(120, 100%, 40%);">+ ccd = data;</span><br><span> if (conn->lchan)</span><br><span style="color: hsl(0, 100%, 40%);">- conn->lchan->release.is_csfb = *(bool *)data;</span><br><span style="color: hsl(120, 100%, 40%);">+ conn->lchan->release.is_csfb = ccd->is_csfb;</span><br><span> /* MSC tells us to cleanly shut down */</span><br><span> if (conn->fi->state != ST_CLEARING)</span><br><span> osmo_fsm_inst_state_chg(fi, ST_CLEARING, 60, 999);</span><br><span> LOGPFSML(fi, LOGL_DEBUG, "Releasing all lchans (if any) after BSSMAP Clear Command\n");</span><br><span style="color: hsl(0, 100%, 40%);">- gscon_release_lchans(conn, true);</span><br><span style="color: hsl(120, 100%, 40%);">+ gscon_release_lchans(conn, true, bsc_gsm48_rr_cause_from_gsm0808_cause(ccd->cause_0808));</span><br><span> /* FIXME: Release all terestrial resources in ST_CLEARING */</span><br><span> /* According to 3GPP 48.008 3.1.9.1. "The BSS need not wait for the radio channel</span><br><span> * release to be completed or for the guard timer to expire before returning the</span><br><span>@@ -833,7 +836,9 @@</span><br><span> }</span><br><span> </span><br><span> LOGPFSML(fi, LOGL_DEBUG, "Releasing all lchans (if any) because this conn is terminating\n");</span><br><span style="color: hsl(0, 100%, 40%);">- gscon_release_lchans(conn, true);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* when things go smoothly, the lchan should have been released before FSM instance termination. So if this is</span><br><span style="color: hsl(120, 100%, 40%);">+ * necessary it's probably "abnormal". */</span><br><span style="color: hsl(120, 100%, 40%);">+ gscon_release_lchans(conn, true, GSM48_RR_CAUSE_ABNORMAL_UNSPEC);</span><br><span> </span><br><span> /* drop pending messages */</span><br><span> gscon_dtap_queue_flush(conn, 0);</span><br><span>diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c</span><br><span>index f425b16..07978de 100644</span><br><span>--- a/src/osmo-bsc/gsm_04_08_rr.c</span><br><span>+++ b/src/osmo-bsc/gsm_04_08_rr.c</span><br><span>@@ -303,7 +303,7 @@</span><br><span> gh->msg_type = GSM48_MT_RR_CHAN_REL;</span><br><span> </span><br><span> cause = msgb_put(msg, 1);</span><br><span style="color: hsl(0, 100%, 40%);">- cause[0] = GSM48_RR_CAUSE_NORMAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ cause[0] = lchan->release.rr_cause;</span><br><span> </span><br><span> if (lchan->release.is_csfb) {</span><br><span> uint8_t buf[CELL_SEL_IND_AFTER_REL_MAX_BYTES];</span><br><span>diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c</span><br><span>index f790b90..860e236 100644</span><br><span>--- a/src/osmo-bsc/gsm_data.c</span><br><span>+++ b/src/osmo-bsc/gsm_data.c</span><br><span>@@ -1840,3 +1840,30 @@</span><br><span> </span><br><span> return result;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* This may be specific to RR Channel Release, and the mappings were chosen by pure naive guessing without a proper</span><br><span style="color: hsl(120, 100%, 40%);">+ * specification available. */</span><br><span style="color: hsl(120, 100%, 40%);">+enum gsm48_rr_cause bsc_gsm48_rr_cause_from_gsm0808_cause(enum gsm0808_cause c)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (c) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_PREEMPTION:</span><br><span style="color: hsl(120, 100%, 40%);">+ return GSM48_RR_CAUSE_PREMPTIVE_REL;</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_RADIO_INTERFACE_MESSAGE_FAILURE:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_INVALID_MESSAGE_CONTENTS:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_INFORMATION_ELEMENT_OR_FIELD_MISSING:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_INCORRECT_VALUE:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_UNKNOWN_MESSAGE_TYPE:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_UNKNOWN_INFORMATION_ELEMENT:</span><br><span style="color: hsl(120, 100%, 40%);">+ return GSM48_RR_CAUSE_PROT_ERROR_UNSPC;</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_CALL_CONTROL:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_HANDOVER_SUCCESSFUL:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_BETTER_CELL:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_DIRECTED_RETRY:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_REDUCE_LOAD_IN_SERVING_CELL:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_RELOCATION_TRIGGERED:</span><br><span style="color: hsl(120, 100%, 40%);">+ case GSM0808_CAUSE_ALT_CHAN_CONFIG_REQUESTED:</span><br><span style="color: hsl(120, 100%, 40%);">+ return GSM48_RR_CAUSE_NORMAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ return GSM48_RR_CAUSE_ABNORMAL_UNSPEC;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span>diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c</span><br><span>index 68eef4b..dfb8a05 100644</span><br><span>--- a/src/osmo-bsc/lchan_fsm.c</span><br><span>+++ b/src/osmo-bsc/lchan_fsm.c</span><br><span>@@ -402,6 +402,8 @@</span><br><span> .meas_rep_last_seen_nr = 255,</span><br><span> </span><br><span> .last_error = lchan->last_error,</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ .release.rr_cause = GSM48_RR_CAUSE_NORMAL,</span><br><span> };</span><br><span> }</span><br><span> </span><br><span>@@ -1369,8 +1371,8 @@</span><br><span> struct osmo_fsm_inst *fi = lchan->fi;</span><br><span> </span><br><span> lchan->release.in_error = err;</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->release.rsl_error_cause = cause_rr;</span><br><span> lchan->release.do_rr_release = do_rr_release;</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->release.rr_cause = cause_rr;</span><br><span> </span><br><span> /* States waiting for events will notice the desire to release when done waiting, so it is enough</span><br><span> * to mark for release. */</span><br><span>diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c</span><br><span>index 2deb7f4..1cacfe9 100644</span><br><span>--- a/src/osmo-bsc/osmo_bsc_bssap.c</span><br><span>+++ b/src/osmo-bsc/osmo_bsc_bssap.c</span><br><span>@@ -453,15 +453,23 @@</span><br><span> struct msgb *msg, unsigned int length)</span><br><span> {</span><br><span> struct tlv_parsed tp;</span><br><span style="color: hsl(0, 100%, 40%);">- bool is_csfb = false;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gscon_clear_cmd_data ccd = {</span><br><span style="color: hsl(120, 100%, 40%);">+ .is_csfb = false,</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span> </span><br><span> tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, length - 1, 0, 0);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* FIXME: Check for mandatory cause IE, and use that in RR RELEASE cause! */</span><br><span style="color: hsl(0, 100%, 40%);">- if (TLVP_PRESENT(&tp, GSM0808_IE_CSFB_INDICATION))</span><br><span style="color: hsl(0, 100%, 40%);">- is_csfb = true;</span><br><span style="color: hsl(120, 100%, 40%);">+ ccd.cause_0808 = gsm0808_get_cause(&tp);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ccd.cause_0808 < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPFSML(conn->fi, LOGL_ERROR, "Clear Command: Mandatory Cause IE not present.\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Clear anyway, but without a proper cause. */</span><br><span style="color: hsl(120, 100%, 40%);">+ ccd.cause_0808 = GSM0808_CAUSE_RADIO_INTERFACE_MESSAGE_FAILURE;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_A_CLEAR_CMD, &is_csfb);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (TLVP_PRESENT(&tp, GSM0808_IE_CSFB_INDICATION))</span><br><span style="color: hsl(120, 100%, 40%);">+ ccd.is_csfb = true;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_A_CLEAR_CMD, &ccd);</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/19214">change 19214</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-bsc/+/19214"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I734cc55c501d61bbdadee81a223b26f9df57f959 </div>
<div style="display:none"> Gerrit-Change-Number: 19214 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>