<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/11445">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">lchan_fsm: generate proper multirate configuration IE on RSL<br><br>During the generation of the multirate configuration IE in the channel<br>activation message that is sent over RSL, all AMR rates except the<br>highest one are trimmed. This was to ensure that the multirate<br>configuration IE only contains one codec rate per active set. Lets fix<br>that and generate a proper IE with threshold and hysteresis values.<br><br>- extend lchan_mr_config so that it can generate a full multirate<br> configuration IE<br><br>Change-Id: I7f9f8e8d9e2724cbe3ce2f3599bc0e5185fd8453<br>Related: OS#3529<br>---<br>M src/osmo-bsc/lchan_fsm.c<br>M tests/handover/handover_test.c<br>2 files changed, 72 insertions(+), 58 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c</span><br><span>index 39edaff..915b62a 100644</span><br><span>--- a/src/osmo-bsc/lchan_fsm.c</span><br><span>+++ b/src/osmo-bsc/lchan_fsm.c</span><br><span>@@ -36,6 +36,8 @@</span><br><span> #include <osmocom/bsc/assignment_fsm.h></span><br><span> #include <osmocom/bsc/handover_fsm.h></span><br><span> #include <osmocom/bsc/bsc_msc_data.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/bsc/codec_pref.h></span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> </span><br><span> static struct osmo_fsm lchan_fsm;</span><br><span> </span><br><span>@@ -402,66 +404,71 @@</span><br><span> osmo_fsm_inst_dispatch(lchan->ts->fi, TS_EV_LCHAN_UNUSED, lchan);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*! Configure the multirate setting on this channel. */</span><br><span style="color: hsl(0, 100%, 40%);">-void lchan_mr_config(struct gsm_lchan *lchan, struct gsm48_multi_rate_conf *mr_conf)</span><br><span style="color: hsl(120, 100%, 40%);">+/* Configure the multirate setting on this channel. */</span><br><span style="color: hsl(120, 100%, 40%);">+static int lchan_mr_config(struct gsm_lchan *lchan, const struct gsm48_multi_rate_conf *mr_conf)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm48_multi_rate_conf *ms_conf, *bts_conf;</span><br><span> bool full_rate = (lchan->type == GSM_LCHAN_TCH_F);</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts *bts = lchan->ts->trx->bts;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bsc_msc_data *msc = lchan->conn->sccp.msc;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct amr_multirate_conf *mr;</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc;</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc_rate;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_multi_rate_conf mr_conf_filtered;</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm48_multi_rate_conf *mr_conf_bts;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* initialize the data structure */</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->mr_ms_lv[0] = sizeof(*ms_conf);</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->mr_bts_lv[0] = sizeof(*bts_conf);</span><br><span style="color: hsl(0, 100%, 40%);">- ms_conf = (struct gsm48_multi_rate_conf *) &lchan->mr_ms_lv[1];</span><br><span style="color: hsl(0, 100%, 40%);">- bts_conf = (struct gsm48_multi_rate_conf *) &lchan->mr_bts_lv[1];</span><br><span style="color: hsl(120, 100%, 40%);">+ /* There are two different active sets, depending on the channel rate,</span><br><span style="color: hsl(120, 100%, 40%);">+ * make sure the appropate one is selected. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (full_rate)</span><br><span style="color: hsl(120, 100%, 40%);">+ mr = &bts->mr_full;</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+ mr = &bts->mr_half;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- *ms_conf = *bts_conf = (struct gsm48_multi_rate_conf){</span><br><span style="color: hsl(0, 100%, 40%);">- .ver = 1,</span><br><span style="color: hsl(0, 100%, 40%);">- .icmi = 1,</span><br><span style="color: hsl(0, 100%, 40%);">- .m4_75 = mr_conf->m4_75,</span><br><span style="color: hsl(0, 100%, 40%);">- .m5_15 = mr_conf->m5_15,</span><br><span style="color: hsl(0, 100%, 40%);">- .m5_90 = mr_conf->m5_90,</span><br><span style="color: hsl(0, 100%, 40%);">- .m6_70 = mr_conf->m6_70,</span><br><span style="color: hsl(0, 100%, 40%);">- .m7_40 = mr_conf->m7_40,</span><br><span style="color: hsl(0, 100%, 40%);">- .m7_95 = mr_conf->m7_95,</span><br><span style="color: hsl(0, 100%, 40%);">- .m10_2 = full_rate? mr_conf->m10_2 : 0,</span><br><span style="color: hsl(0, 100%, 40%);">- .m12_2 = full_rate? mr_conf->m12_2 : 0,</span><br><span style="color: hsl(0, 100%, 40%);">- };</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-/* Mask all rates instead of the highest possible */</span><br><span style="color: hsl(0, 100%, 40%);">-static void lchan_mr_config_mask(struct gsm48_multi_rate_conf *mr_conf)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- unsigned int i;</span><br><span style="color: hsl(0, 100%, 40%);">- bool highest_seen = false;</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t *_mr_conf = (uint8_t *) mr_conf;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* FIXME: At the moment we can not support multiple codec rates in one</span><br><span style="color: hsl(0, 100%, 40%);">- * struct gsm48_multi_rate_conf, because the struct lacks the fields</span><br><span style="color: hsl(0, 100%, 40%);">- * for Threshold and Hysteresis. Those fields are not needed when only</span><br><span style="color: hsl(0, 100%, 40%);">- * a single codec rate is in place, but as soon as multiple codec</span><br><span style="color: hsl(0, 100%, 40%);">- * rates are used the parameters are mandatory. The layout for the</span><br><span style="color: hsl(0, 100%, 40%);">- * struct would then also be different because each rate needs its</span><br><span style="color: hsl(0, 100%, 40%);">- * own Threshold and Hysteresis value. (See also 3GPP TS 04.08,</span><br><span style="color: hsl(0, 100%, 40%);">- * chapter 10.5.2.21aa MultiRate configuration).</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * Since we are unable to signal multiple codec rates properly, we just</span><br><span style="color: hsl(0, 100%, 40%);">- * remove all codec rates from the active set, except the highest</span><br><span style="color: hsl(0, 100%, 40%);">- * possible. Doing so we lack the functionality to switch towards the</span><br><span style="color: hsl(0, 100%, 40%);">- * other, lower codec rates that were offered by the MSC, but it is</span><br><span style="color: hsl(0, 100%, 40%);">- * still guaranteed that a rate is selected that is supported by all</span><br><span style="color: hsl(0, 100%, 40%);">- * entities.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * To fix this problem, we should implement a proper encoder for</span><br><span style="color: hsl(0, 100%, 40%);">- * struct gsm48_multi_rate_conf, in libosmocore and use it here.</span><br><span style="color: hsl(0, 100%, 40%);">- * struct amr_mode already seems to have members for threshold and</span><br><span style="color: hsl(0, 100%, 40%);">- * hysteresis we can use. */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- for (i = 7; i > 0; i--) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (_mr_conf[1] & (1 << i) && highest_seen == false) {</span><br><span style="color: hsl(0, 100%, 40%);">- highest_seen = true;</span><br><span style="color: hsl(0, 100%, 40%);">- } else if (highest_seen)</span><br><span style="color: hsl(0, 100%, 40%);">- _mr_conf[1] &= ~(1 << i);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The VTY allows to forbid certain codec rates. Unfortunately we can</span><br><span style="color: hsl(120, 100%, 40%);">+ * not articulate all of the prohibitions on through S0-S15 on the A</span><br><span style="color: hsl(120, 100%, 40%);">+ * interface. To ensure that the VTY settings are observed we create</span><br><span style="color: hsl(120, 100%, 40%);">+ * a manipulated copy of the mr_conf that ensures forbidden codec rates</span><br><span style="color: hsl(120, 100%, 40%);">+ * are not used in the multirate configuration IE. */</span><br><span style="color: hsl(120, 100%, 40%);">+ rc_rate = calc_amr_rate_intersection(&mr_conf_filtered, &msc->amr_conf, mr_conf);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc_rate < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "can not encode multirate configuration (invalid amr rate setting, MSC)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The two last codec rates which are defined for AMR do only work with</span><br><span style="color: hsl(120, 100%, 40%);">+ * full rate channels. We will pinch off those rates für half-rate</span><br><span style="color: hsl(120, 100%, 40%);">+ * channels to ensure they are not included accidently. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!full_rate) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (mr_conf_filtered.m10_2 || mr_conf_filtered.m12_2)</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR, "ignoring unsupported amr codec rates\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_conf_filtered.m10_2 = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_conf_filtered.m12_2 = 0;</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%);">+ /* Ensure that the resulting filtered conf is coherent with the</span><br><span style="color: hsl(120, 100%, 40%);">+ * configuration that is set for the BTS and the specified rate */</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_conf_bts = (struct gsm48_multi_rate_conf *)mr->gsm48_ie;</span><br><span style="color: hsl(120, 100%, 40%);">+ rc_rate = calc_amr_rate_intersection(&mr_conf_filtered, mr_conf_bts, &mr_conf_filtered);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc_rate < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "can not encode multirate configuration (invalid amr rate setting, BTS)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</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%);">+ /* Proceed with the generation of the multirate configuration IE</span><br><span style="color: hsl(120, 100%, 40%);">+ * (MS and BTS) */</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(lchan->mr_ms_lv, &mr_conf_filtered, mr->ms_mode, mr->num_modes);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc != 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR, "can not encode multirate configuration (MS)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(lchan->mr_bts_lv, &mr_conf_filtered, mr->bts_mode, mr->num_modes);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc != 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR, "can not encode multirate configuration (BTS)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</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%);">+ return 0;</span><br><span> }</span><br><span> </span><br><span> static void lchan_fsm_unused(struct osmo_fsm_inst *fi, uint32_t event, void *data)</span><br><span>@@ -511,9 +518,10 @@</span><br><span> </span><br><span> if (info->chan_mode == GSM48_CMODE_SPEECH_AMR) {</span><br><span> gsm48_mr_cfg_from_gsm0808_sc_cfg(&mr_conf, info->s15_s0);</span><br><span style="color: hsl(0, 100%, 40%);">- /* FIXME: See above. */</span><br><span style="color: hsl(0, 100%, 40%);">- lchan_mr_config_mask(&mr_conf);</span><br><span style="color: hsl(0, 100%, 40%);">- lchan_mr_config(lchan, &mr_conf);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan_mr_config(lchan, &mr_conf) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan_fail("Can not generate multirate configuration IE\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> switch (info->chan_mode) {</span><br><span>diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c</span><br><span>index 76e5fe1..7cb4086 100644</span><br><span>--- a/tests/handover/handover_test.c</span><br><span>+++ b/tests/handover/handover_test.c</span><br><span>@@ -224,6 +224,12 @@</span><br><span> struct gsm_network *net = lchan->ts->trx->bts->network;</span><br><span> struct gsm_subscriber_connection *conn;</span><br><span> struct mgcp_client *fake_mgcp_client = (void*)talloc_zero(net, int);</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *amr_conf;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* HACK: lchan_fsm.c requires some AMR codec rates to be enabled,</span><br><span style="color: hsl(120, 100%, 40%);">+ * lets pretend that all AMR codec rates are allowed */</span><br><span style="color: hsl(120, 100%, 40%);">+ amr_conf = (uint8_t*) &fake_msc_data.amr_conf;</span><br><span style="color: hsl(120, 100%, 40%);">+ amr_conf[1] = 0xff;</span><br><span> </span><br><span> conn = bsc_subscr_con_allocate(net);</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11445">change 11445</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/11445"/><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-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I7f9f8e8d9e2724cbe3ce2f3599bc0e5185fd8453 </div>
<div style="display:none"> Gerrit-Change-Number: 11445 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>