<p>neels <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/24356">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">AMR config cleanup step 1: split lchan_mr_config()<br><br>Split off two function from lchan_mr_config() which do not directly<br>manipulate struct gsm_lchan members. This allows subsequent patches to<br>re-use mr_config_filter() for Channel Mode Modify without depending on<br>lchan->activate.info; allows to filter AMR modes without modifying the<br>state of an already active lchan before sending a Channel Activation or<br>Channel Mode Modify; and allows to move mr_config_render_lv() to<br>gsm_04_08_rr.c so that the mr_ms_lv and mr_bts_lv no longer need to be<br>stored in struct gsm_lchan -- they essentially duplicate s15_s0.<br><br>Rationale:<br><br>This is a follow-up for the AMR configuration in the sense that previous<br>patch Ie0da36124d73efc28a8809b63d7c96e2167fc412 started for channel mode<br>and rate, and the s15_s0 bits:<br><br>The AMR mode filtering directly manipulates struct gsm_lchan members and<br>takes parameters from lchan->activate.info. This makes it hard to<br>separate lchan activation from the Channel Mode Modify procedure.<br><br>Related: SYS#5315 OS#4940 OS#3787 OS#3833<br>Change-Id: Iebac2dc26412d877e5364f90d6f2ed7a7952351e<br>---<br>M src/osmo-bsc/lchan_fsm.c<br>1 file changed, 85 insertions(+), 73 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 d648202..4aac6da 100644</span><br><span>--- a/src/osmo-bsc/lchan_fsm.c</span><br><span>+++ b/src/osmo-bsc/lchan_fsm.c</span><br><span>@@ -484,103 +484,115 @@</span><br><span> }</span><br><span> </span><br><span> /* Configure the multirate setting on this channel. */</span><br><span style="color: hsl(0, 100%, 40%);">-static int lchan_mr_config(struct gsm_lchan *lchan, uint16_t s15_s0)</span><br><span style="color: hsl(120, 100%, 40%);">+static int mr_config_filter(struct gsm48_multi_rate_conf *mr_conf_result,</span><br><span style="color: hsl(120, 100%, 40%);">+ bool full_rate,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct amr_multirate_conf *amr_mrc,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm48_multi_rate_conf *mr_filter_bts,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm48_multi_rate_conf *mr_filter_msc,</span><br><span style="color: hsl(120, 100%, 40%);">+ uint16_t s15_s0,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm_lchan *lchan_for_logging)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm48_multi_rate_conf mr_conf;</span><br><span style="color: hsl(0, 100%, 40%);">- bool full_rate = (lchan->type == GSM_LCHAN_TCH_F);</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm_bts *bts = lchan->ts->trx->bts;</span><br><span style="color: hsl(0, 100%, 40%);">- struct amr_multirate_conf *mr;</span><br><span> int rc;</span><br><span style="color: hsl(0, 100%, 40%);">- int rc_rate;</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm48_multi_rate_conf mr_conf_filtered;</span><br><span style="color: hsl(0, 100%, 40%);">- const struct gsm48_multi_rate_conf *mr_conf_bts;</span><br><span> </span><br><span> /* Generate mr conf struct from S15-S0 bits */</span><br><span style="color: hsl(0, 100%, 40%);">- if (gsm48_mr_cfg_from_gsm0808_sc_cfg(&mr_conf, s15_s0) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gsm48_mr_cfg_from_gsm0808_sc_cfg(mr_conf_result, s15_s0) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan_for_logging, LOGL_ERROR,</span><br><span> "can not determine multirate configuration, S15-S0 (%04x) are ambiguous!\n", s15_s0);</span><br><span> return -EINVAL;</span><br><span> }</span><br><span> </span><br><span> /* Do not include 12.2 kbps rate when S1 is set. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan->type == GSM_LCHAN_TCH_H && (s15_s0 & GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20)) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* See also 3GPP TS 28.062, chapter 7.11.3.1.3: "In case this Configuration</span><br><span style="color: hsl(0, 100%, 40%);">- * "Config-NB-Code = 1" is signalled in the TFO Negotiation for the HR_AMR</span><br><span style="color: hsl(0, 100%, 40%);">- * Codec Type,then it shall be assumed that AMR mode 12.2 kbps is (of course)</span><br><span style="color: hsl(0, 100%, 40%);">- * not included. */</span><br><span style="color: hsl(0, 100%, 40%);">- mr_conf.m12_2 = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((!full_rate) && (s15_s0 & GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* See also 3GPP TS 28.062, chapter 7.11.3.1.3:</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * In case this Configuration "Config-NB-Code = 1" is signalled in the TFO Negotiation for the HR_AMR</span><br><span style="color: hsl(120, 100%, 40%);">+ * Codec Type, then it shall be assumed that AMR mode 12.2 kbps is (of course) not included.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * Further below, we log an error if 12k2 is included for a TCH/H lchan: removing this here ensures that</span><br><span style="color: hsl(120, 100%, 40%);">+ * we don't log that error for GSM0808_SC_CFG_AMR_4_75_5_90_7_40_12_20 on a TCH/H lchan. */</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_conf_result->m12_2 = 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* There are two different active sets, depending on the channel rate,</span><br><span style="color: hsl(0, 100%, 40%);">- * make sure the appropate one is selected. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (full_rate)</span><br><span style="color: hsl(0, 100%, 40%);">- mr = &bts->mr_full;</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- mr = &bts->mr_half;</span><br><span style="color: hsl(0, 100%, 40%);">- mr_conf_bts = (struct gsm48_multi_rate_conf *)mr->gsm48_ie;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan->activate.info.activ_for == ACTIVATE_FOR_VTY)</span><br><span style="color: hsl(0, 100%, 40%);">- /* If the channel is activated manually from VTY, then there is no</span><br><span style="color: hsl(0, 100%, 40%);">- * conn attached to the lchan, also no MSC is involved. Since this</span><br><span style="color: hsl(0, 100%, 40%);">- * option is for debugging and the codec choice is an intentional</span><br><span style="color: hsl(0, 100%, 40%);">- * decision by the VTY user, we do not filter the mr_conf. */</span><br><span style="color: hsl(0, 100%, 40%);">- memcpy(&mr_conf_filtered, &mr_conf, sizeof(mr_conf_filtered));</span><br><span style="color: hsl(0, 100%, 40%);">- else {</span><br><span style="color: hsl(0, 100%, 40%);">- /* The VTY allows to forbid certain codec rates. Unfortunately we can</span><br><span style="color: hsl(0, 100%, 40%);">- * not articulate all of the prohibitions on through S0-S15 on the A</span><br><span style="color: hsl(0, 100%, 40%);">- * interface. To ensure that the VTY settings are observed we create</span><br><span style="color: hsl(0, 100%, 40%);">- * a manipulated copy of the mr_conf that ensures forbidden codec rates</span><br><span style="color: hsl(0, 100%, 40%);">- * are not used in the multirate configuration IE. */</span><br><span style="color: hsl(0, 100%, 40%);">- rc_rate = calc_amr_rate_intersection(&mr_conf_filtered, &lchan->conn->sccp.msc->amr_conf, &mr_conf);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rc_rate < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ if (mr_filter_msc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = calc_amr_rate_intersection(mr_conf_result, mr_filter_msc, mr_conf_result);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan_for_logging, LOGL_ERROR,</span><br><span> "can not encode multirate configuration (invalid amr rate setting, MSC)\n");</span><br><span> return -EINVAL;</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* The two last codec rates which are defined for AMR do only work with</span><br><span style="color: hsl(0, 100%, 40%);">- * full rate channels. We will pinch off those rates für half-rate</span><br><span style="color: hsl(0, 100%, 40%);">- * channels to ensure they are not included accidently. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!full_rate) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (mr_conf_filtered.m10_2 || mr_conf_filtered.m12_2)</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR, "ignoring unsupported amr codec rates\n");</span><br><span style="color: hsl(0, 100%, 40%);">- mr_conf_filtered.m10_2 = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- mr_conf_filtered.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%);">- /* Ensure that the resulting filtered conf is coherent with the</span><br><span style="color: hsl(0, 100%, 40%);">- * configuration that is set for the BTS and the specified rate.</span><br><span style="color: hsl(0, 100%, 40%);">- * if the channel activation was triggerd by the VTY, do not</span><br><span style="color: hsl(0, 100%, 40%);">- * filter anything (see also comment above) */</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan->activate.info.activ_for != ACTIVATE_FOR_VTY) {</span><br><span style="color: hsl(0, 100%, 40%);">- rc_rate = calc_amr_rate_intersection(&mr_conf_filtered, mr_conf_bts, &mr_conf_filtered);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rc_rate < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ if (mr_filter_bts) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = calc_amr_rate_intersection(mr_conf_result, mr_filter_bts, mr_conf_result);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan_for_logging, LOGL_ERROR,</span><br><span> "can not encode multirate configuration (invalid amr rate setting, BTS)\n");</span><br><span> return -EINVAL;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Set the ICMI according to the BTS. Above gsm48_mr_cfg_from_gsm0808_sc_cfg() always sets ICMI = 1, which</span><br><span style="color: hsl(120, 100%, 40%);">+ * carried through all of the above rate intersections. */</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_conf_result->icmi = mr_filter_bts->icmi;</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_conf_result->smod = mr_filter_bts->smod;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Set the ICMI according to the BTS. Above gsm48_mr_cfg_from_gsm0808_sc_cfg() always sets ICMI = 1, which</span><br><span style="color: hsl(0, 100%, 40%);">- * carried through all of the above rate intersections. */</span><br><span style="color: hsl(0, 100%, 40%);">- mr_conf_filtered.icmi = mr_conf_bts->icmi;</span><br><span style="color: hsl(0, 100%, 40%);">- mr_conf_filtered.smod = mr_conf_bts->smod;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* 10k2 and 12k2 only work for full rate */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!full_rate) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (mr_conf_result->m10_2 || mr_conf_result->m12_2)</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan_for_logging, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "half rate lchan: ignoring unsupported AMR codec rates 10k2 and 12k2\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_conf_result->m10_2 = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_conf_result->m12_2 = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Proceed with the generation of the multirate configuration IE</span><br><span style="color: hsl(0, 100%, 40%);">- * (MS and BTS) */</span><br><span style="color: hsl(120, 100%, 40%);">+ return 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%);">+static int mr_config_render_lv(uint8_t *mr_ms_lv, uint8_t *mr_bts_lv,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm48_multi_rate_conf *mr_conf,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct amr_multirate_conf *amr_mrc,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm_lchan *logging)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gsm48_multirate_config(mr_ms_lv, mr_conf, amr_mrc->ms_mode, amr_mrc->num_modes)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(logging, 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%);">+ if (gsm48_multirate_config(mr_bts_lv, mr_conf, amr_mrc->bts_mode, amr_mrc->num_modes)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(logging, 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%);">+ return 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%);">+/* 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, uint16_t s15_s0)</span><br><span style="color: hsl(120, 100%, 40%);">+{</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%);">+ bool full_rate = lchan->type == GSM_LCHAN_TCH_F;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct amr_multirate_conf *amr_mrc = full_rate ? &bts->mr_full : &bts->mr_half;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_multi_rate_conf *mr_filter_bts = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_multi_rate_conf *mr_filter_msc = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_multi_rate_conf mr_conf;</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->activate.info.activ_for != ACTIVATE_FOR_VTY) {</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_filter_bts = (struct gsm48_multi_rate_conf*)amr_mrc->gsm48_ie;</span><br><span style="color: hsl(120, 100%, 40%);">+ mr_filter_msc = &lchan->conn->sccp.msc->amr_conf;</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%);">+ rc = mr_config_filter(&mr_conf,</span><br><span style="color: hsl(120, 100%, 40%);">+ full_rate,</span><br><span style="color: hsl(120, 100%, 40%);">+ amr_mrc, mr_filter_bts, mr_filter_msc,</span><br><span style="color: hsl(120, 100%, 40%);">+ s15_s0,</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc)</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* FIXME: this actually modifies the lchan->mr_ms_lv and ->mr_bts_lv before an ACK for these AMR bits has been</span><br><span> * received. Until an ACK is received, all state should live in lchan->activate.* or lchan->modify.* ONLY. */</span><br><span style="color: hsl(0, 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(0, 100%, 40%);">- if (rc != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR, "can not encode multirate configuration (MS)\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return -EINVAL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 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(0, 100%, 40%);">- if (rc != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR, "can not encode multirate configuration (BTS)\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return -EINVAL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = mr_config_render_lv(lchan->mr_ms_lv, lchan->mr_bts_lv, &mr_conf, amr_mrc, lchan);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc)</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</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/+/24356">change 24356</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/+/24356"/><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: Iebac2dc26412d877e5364f90d6f2ed7a7952351e </div>
<div style="display:none"> Gerrit-Change-Number: 24356 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </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: fixeria <vyanitskiy@sysmocom.de> </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-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>