<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/23791">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">rsl: rename, fix and refactor lchan_tchmode_from_cmode()<br><br>In change [1] I added the missing 'default' branch to the 'switch'<br>statement in lchan_tchmode_from_cmode(). This caused massive<br>regressions in ttcn3-bts-test, because osmo-bts started to NACK<br>some RSL CHANnel ACTIVation messages.<br><br>What caused a lot of regressions in ttcn3-bts-test is actually the<br>missing branch for RSL_CMOD_SPD_SIGN in the 'switch' statement.<br>It was not a problem before [1], because the 'default' branch was<br>not there. I was about to add the missing 'cause' when I realized<br>that this function needs to be reworked first...<br><br>First of all, lchan_tchmode_from_cmode() does a bit more than just<br>deriving RR (TS 44.018) channel mode from RSL (TS 48.058) channel<br>mode. It additionally stores the 'Speech or data indicator' to<br>the logical channel state, and also changes some global DTXd related<br>flags in 'struct gsm_bts'. Let's use a more precise name.<br><br> lchan_tchmode_from_cmode() -> rsl_handle_chan_mod_ie()<br><br>Together with renaming, it becomes logical to have the IE presence<br>check in rsl_handle_chan_mod_ie(), so that we can reduce code<br>duplication in the calling functions a bit.<br><br>Finally, the main problem is that coding and interpretation of the<br>6-th octet 'Speech coding algor./data rate + transp ind' depends on<br>the 4-th octet of the Channel Mode IE. We cannot handle all values<br>in one 'switch' statement without proper discrimination:<br><br> a) If octet 4 indicates Speech, then octet 6 shall be interpreted<br> as the GSM speech coding algorithm (FR, HR, AMR, etc.).<br><br> b) If octet 4 indicates Signalling, then octet 6 shall be set<br> to '00'O, because this is the only value defined in version<br> 16.0.0 of 3GPP TS 48.058. All other values are reserved.<br><br> c) If octet 4 indicates Data, then octet 6 shall be interpreted<br> as CSD data rate further discriminated by service transparency.<br><br>Therefore, we need take both values into account. This can be<br>achieved by mixing them together using the bitwise operators,<br>just like we do in L1SAP code.<br><br>Change-Id: Iba967f5bd0cc8ad6cd3ccd40cca38b15ffe96b2c<br>Related: [1] I67a70132999be6580a29e6b814763309a6df4ae9<br>Related: SYS#4895, OS#4941<br>---<br>M src/common/rsl.c<br>1 file changed, 99 insertions(+), 42 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/common/rsl.c b/src/common/rsl.c</span><br><span>index 8488ab7..6884712 100644</span><br><span>--- a/src/common/rsl.c</span><br><span>+++ b/src/common/rsl.c</span><br><span>@@ -111,36 +111,108 @@</span><br><span> out[1] = (gtime->t3 << 5) | gtime->t2;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* compute lchan->rsl_cmode and lchan->tch_mode from RSL CHAN MODE IE */</span><br><span style="color: hsl(0, 100%, 40%);">-static int lchan_tchmode_from_cmode(struct gsm_lchan *lchan,</span><br><span style="color: hsl(0, 100%, 40%);">- const struct rsl_ie_chan_mode *cm)</span><br><span style="color: hsl(120, 100%, 40%);">+/* Handle RSL Channel Mode IE (see section 9.3.6) */</span><br><span style="color: hsl(120, 100%, 40%);">+static int rsl_handle_chan_mod_ie(struct gsm_lchan *lchan,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct tlv_parsed *tp,</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *cause)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct rsl_ie_chan_mode *cm;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!TLVP_PRES_LEN(tp, RSL_IE_CHAN_MODE, sizeof(*cm))) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Channel Mode IE is not present\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ *cause = RSL_ERR_MAND_IE_ERROR;</span><br><span style="color: hsl(120, 100%, 40%);">+ return -ENODEV;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ cm = (const struct rsl_ie_chan_mode *) TLVP_VAL(tp, RSL_IE_CHAN_MODE);</span><br><span> lchan->rsl_cmode = cm->spd_ind;</span><br><span> lchan->ts->trx->bts->dtxd = (cm->dtx_dtu & RSL_CMOD_DTXd) ? true : false;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- switch (cm->chan_rate) {</span><br><span style="color: hsl(0, 100%, 40%);">- case RSL_CMOD_SP_GSM1:</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->tch_mode = GSM48_CMODE_SPEECH_V1;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case RSL_CMOD_SP_GSM2:</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->tch_mode = GSM48_CMODE_SPEECH_EFR;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case RSL_CMOD_SP_GSM3:</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->tch_mode = GSM48_CMODE_SPEECH_AMR;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case RSL_CMOD_SP_NT_14k5:</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->tch_mode = GSM48_CMODE_DATA_14k5;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case RSL_CMOD_SP_NT_12k0:</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->tch_mode = GSM48_CMODE_DATA_12k0;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case RSL_CMOD_SP_NT_6k0:</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->tch_mode = GSM48_CMODE_DATA_6k0;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Octet 5: Channel rate and type */</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (cm->chan_rt) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMOD_CRT_SDCCH:</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMOD_CRT_TCH_Bm:</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMOD_CRT_TCH_Lm:</span><br><span> break;</span><br><span> default:</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Channel Mode IE contains "</span><br><span style="color: hsl(120, 100%, 40%);">+ "unknown 'Channel rate and type' value 0x%02x\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ cm->chan_rate);</span><br><span style="color: hsl(120, 100%, 40%);">+ *cause = RSL_ERR_IE_CONTENT;</span><br><span> return -ENOTSUP;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define RSL_CMODE(spd_ind, chan_rate) \</span><br><span style="color: hsl(120, 100%, 40%);">+ ((spd_ind << 8) | chan_rate)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Octet 6: Speech coding algorithm/data rate + transparency indicator.</span><br><span style="color: hsl(120, 100%, 40%);">+ * NOTE: coding of this octet depends on 'Speech or data indicator' */</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (RSL_CMODE(cm->spd_ind, cm->chan_rate)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If octet 4 indicates signalling */</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_SIGN, 0x00):</span><br><span style="color: hsl(120, 100%, 40%);">+ /* No resources required, all other values are reserved */</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->tch_mode = GSM48_CMODE_SIGN;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If octet 4 indicates speech */</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_SPEECH, RSL_CMOD_SP_GSM1):</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->tch_mode = GSM48_CMODE_SPEECH_V1;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_SPEECH, RSL_CMOD_SP_GSM2):</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->tch_mode = GSM48_CMODE_SPEECH_EFR;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_SPEECH, RSL_CMOD_SP_GSM3):</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->tch_mode = GSM48_CMODE_SPEECH_AMR;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* TODO: also handle RSL_CMOD_SP_{GSM4,GSM5,GSM6} */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If octet 4 indicates non-transparent data */</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_14k5):</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->tch_mode = GSM48_CMODE_DATA_14k5;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_12k0):</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->tch_mode = GSM48_CMODE_DATA_12k0;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_6k0):</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->tch_mode = GSM48_CMODE_DATA_6k0;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_43k5):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_28k8):</span><br><span style="color: hsl(120, 100%, 40%);">+ /* TODO: also handle non-transparent asymmetric data rates */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Channel Mode IE contains "</span><br><span style="color: hsl(120, 100%, 40%);">+ "unhandled non-transparent CSD data rate 0x%02x\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ cm->chan_rate & 0x3f);</span><br><span style="color: hsl(120, 100%, 40%);">+ *cause = RSL_ERR_IE_CONTENT;</span><br><span style="color: hsl(120, 100%, 40%);">+ return -ENOTSUP;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If octet 4 indicates transparent data */</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_32000):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_29000):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_14400):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_9600):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_4800):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_2400):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_1200):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_600):</span><br><span style="color: hsl(120, 100%, 40%);">+ case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_T_1200_75):</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Channel Mode IE contains "</span><br><span style="color: hsl(120, 100%, 40%);">+ "unhandled transparent CSD data rate 0x%02x\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ cm->chan_rate & 0x3f);</span><br><span style="color: hsl(120, 100%, 40%);">+ *cause = RSL_ERR_IE_CONTENT;</span><br><span style="color: hsl(120, 100%, 40%);">+ return -ENOTSUP;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Channel Mode IE contains "</span><br><span style="color: hsl(120, 100%, 40%);">+ "an unknown/unhandled combination of "</span><br><span style="color: hsl(120, 100%, 40%);">+ "'Speech or data indicator' 0x%02x and "</span><br><span style="color: hsl(120, 100%, 40%);">+ "'Speech coding algorithm/data rate' 0x%02x\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ cm->spd_ind, cm->chan_rate);</span><br><span style="color: hsl(120, 100%, 40%);">+ *cause = RSL_ERR_IE_CONTENT;</span><br><span style="color: hsl(120, 100%, 40%);">+ return -ENOPROTOOPT;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+#undef RSL_CMODE</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>@@ -1302,10 +1374,9 @@</span><br><span> struct abis_rsl_dchan_hdr *dch = msgb_l2(msg);</span><br><span> struct gsm_lchan *lchan = msg->lchan;</span><br><span> struct gsm_bts_trx_ts *ts = lchan->ts;</span><br><span style="color: hsl(0, 100%, 40%);">- struct rsl_ie_chan_mode *cm;</span><br><span> struct tlv_parsed tp;</span><br><span> const struct tlv_p_entry *ie;</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t type;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t type, cause;</span><br><span> int rc;</span><br><span> </span><br><span> if (lchan->state != LCHAN_S_NONE) {</span><br><span>@@ -1358,15 +1429,8 @@</span><br><span> </span><br><span> /* 9.3.6 Channel Mode */</span><br><span> if (type != RSL_ACT_OSMO_PDCH) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (!TLVP_PRESENT(&tp, RSL_IE_CHAN_MODE)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPLCHAN(lchan, DRSL, LOGL_NOTICE, "missing Channel Mode\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return rsl_tx_chan_act_nack(lchan, RSL_ERR_MAND_IE_ERROR);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- cm = (struct rsl_ie_chan_mode *) TLVP_VAL(&tp, RSL_IE_CHAN_MODE);</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan_tchmode_from_cmode(lchan, cm) != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPLCHAN(lchan, DRSL, LOGL_NOTICE, "Unhandled RSL Channel Mode\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return rsl_tx_chan_act_nack(lchan, RSL_ERR_IE_CONTENT);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rsl_handle_chan_mod_ie(lchan, &tp, &cause) != 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ return rsl_tx_chan_act_nack(lchan, cause);</span><br><span> }</span><br><span> </span><br><span> /* 9.3.7 Encryption Information */</span><br><span>@@ -1869,22 +1933,15 @@</span><br><span> {</span><br><span> struct abis_rsl_dchan_hdr *dch = msgb_l2(msg);</span><br><span> struct gsm_lchan *lchan = msg->lchan;</span><br><span style="color: hsl(0, 100%, 40%);">- struct rsl_ie_chan_mode *cm;</span><br><span> struct tlv_parsed tp;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t cause;</span><br><span> int rc;</span><br><span> </span><br><span> rsl_tlv_parse(&tp, msgb_l3(msg), msgb_l3len(msg));</span><br><span> </span><br><span> /* 9.3.6 Channel Mode */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!TLVP_PRESENT(&tp, RSL_IE_CHAN_MODE)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPLCHAN(lchan, DRSL, LOGL_NOTICE, "missing Channel Mode\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return rsl_tx_mode_modif_nack(lchan, RSL_ERR_MAND_IE_ERROR);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- cm = (struct rsl_ie_chan_mode *) TLVP_VAL(&tp, RSL_IE_CHAN_MODE);</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan_tchmode_from_cmode(lchan, cm) != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPLCHAN(lchan, DRSL, LOGL_NOTICE, "Unhandled RSL Channel Mode\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return rsl_tx_mode_modif_nack(lchan, RSL_ERR_IE_CONTENT);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rsl_handle_chan_mod_ie(lchan, &tp, &cause) != 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ return rsl_tx_mode_modif_nack(lchan, cause);</span><br><span> </span><br><span> if (bts_supports_cm(lchan->ts->trx->bts, ts_pchan(lchan->ts), lchan->tch_mode) != 1) {</span><br><span> LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "%s: invalid mode: %s (wrong BSC configuration?)\n",</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/23791">change 23791</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-bts/+/23791"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bts </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Iba967f5bd0cc8ad6cd3ccd40cca38b15ffe96b2c </div>
<div style="display:none"> Gerrit-Change-Number: 23791 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: fixeria <vyanitskiy@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: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>