<p>neels has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/24358">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">AMR config cleanup step 3: generate AMR LV on msg composition<br><br>Firstly, do not store the encoded AMR length-value bits in gsm_lchan->*<br>before an activation/modify has actually succeeded.<br><br>And secondly, do not store the AMR LV structure in struct gsm_lchan at<br>all, but only generate the TLV exactly when a message is being composed.<br><br>In gsm48_multirate_config(), generate the LV directly to a msgb instead<br>of a static buffer first. gsm0408_test.c expected output verifies that<br>the generated LV bytes remain unchanged.<br><br>In lchan_mr_config(), introduce a target mr_conf argument, so that Chan<br>Act and Mode Modify may generate the filtered AMR config to different<br>locations (lchan->{activate,modify}.mr_conf_filtered).<br><br>Only after receiving an ACK for Activate/Modify, set<br>lchan->current_mr_conf from lchan->{activate,modify}.mr_conf_filtered.<br><br>Use the properly scoped lchan->activate.mr_conf_filtered for Chan Act,<br>lchan->modify.mr_conf_filtered for Mode Modify and<br>new_lchan->current_mr_conf for Handover Command as appropriate.<br><br>Change-Id: Ie57f9d0e3912632903d9740291225bfd1634ed47<br>---<br>M include/osmocom/bsc/gsm_04_08_rr.h<br>M include/osmocom/bsc/gsm_data.h<br>M src/osmo-bsc/abis_rsl.c<br>M src/osmo-bsc/gsm_04_08_rr.c<br>M src/osmo-bsc/lchan_fsm.c<br>M tests/gsm0408/gsm0408_test.c<br>6 files changed, 143 insertions(+), 103 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/58/24358/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/bsc/gsm_04_08_rr.h b/include/osmocom/bsc/gsm_04_08_rr.h</span><br><span>index 91dcbe3..5ddee7f 100644</span><br><span>--- a/include/osmocom/bsc/gsm_04_08_rr.h</span><br><span>+++ b/include/osmocom/bsc/gsm_04_08_rr.h</span><br><span>@@ -21,7 +21,8 @@</span><br><span> uint8_t *classmark2_lv);</span><br><span> int gsm48_send_rr_classmark_enquiry(struct gsm_lchan *lchan);</span><br><span> int gsm48_send_rr_ciph_mode(struct gsm_lchan *lchan, int want_imeisv);</span><br><span style="color: hsl(0, 100%, 40%);">-int gsm48_multirate_config(uint8_t *lv, const struct gsm48_multi_rate_conf *mr_conf,</span><br><span style="color: hsl(120, 100%, 40%);">+int gsm48_multirate_config(struct msgb *msg,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm48_multi_rate_conf *mr_conf,</span><br><span> const struct amr_mode *modes, unsigned int num_modes);</span><br><span> struct msgb *gsm48_make_ho_cmd(struct gsm_lchan *new_lchan, uint8_t power_command, uint8_t ho_ref);</span><br><span> int gsm48_send_ho_cmd(struct gsm_lchan *old_lchan, struct gsm_lchan *new_lchan,</span><br><span>diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h</span><br><span>index 04d5e52..3ed624f 100644</span><br><span>--- a/include/osmocom/bsc/gsm_data.h</span><br><span>+++ b/include/osmocom/bsc/gsm_data.h</span><br><span>@@ -634,6 +634,7 @@</span><br><span> </span><br><span> struct {</span><br><span> struct lchan_activate_info info;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_multi_rate_conf mr_conf_filtered;</span><br><span> bool activ_ack; /*< true as soon as RSL Chan Activ Ack is received */</span><br><span> bool immediate_assignment_sent;</span><br><span> /*! This flag ensures that when an lchan activation has succeeded, and we have already</span><br><span>@@ -645,6 +646,7 @@</span><br><span> </span><br><span> struct {</span><br><span> struct lchan_modify_info info;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_multi_rate_conf mr_conf_filtered;</span><br><span> bool concluded;</span><br><span> } modify;</span><br><span> </span><br><span>@@ -675,10 +677,6 @@</span><br><span> /* Encryption information */</span><br><span> struct gsm_encr encr;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* AMR bits */</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t mr_ms_lv[7];</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t mr_bts_lv[7];</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* Established data link layer services */</span><br><span> uint8_t sapis[8];</span><br><span> </span><br><span>@@ -722,6 +720,7 @@</span><br><span> /* After the Channel Activation ACK or RSL Mode Modify ACK is received, this reflects the actually used</span><br><span> * channel_mode_and_rate. */</span><br><span> struct channel_mode_and_rate current_ch_mode_rate;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm48_multi_rate_conf current_mr_conf;</span><br><span> };</span><br><span> </span><br><span> /* One Timeslot in a TRX */</span><br><span>diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c</span><br><span>index 5ea1c11..31adf80 100644</span><br><span>--- a/src/osmo-bsc/abis_rsl.c</span><br><span>+++ b/src/osmo-bsc/abis_rsl.c</span><br><span>@@ -465,21 +465,11 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static void mr_config_for_bts(struct gsm_lchan *lchan, struct msgb *msg,</span><br><span style="color: hsl(0, 100%, 40%);">- const struct channel_mode_and_rate *ch_mode_rate)</span><br><span style="color: hsl(120, 100%, 40%);">+static int put_mr_config_for_bts(struct msgb *msg, const struct gsm48_multi_rate_conf *mr_conf_filtered,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct amr_multirate_conf *mr_modes)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t len;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (ch_mode_rate->chan_mode != GSM48_CMODE_SPEECH_AMR)</span><br><span style="color: hsl(0, 100%, 40%);">- return;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- len = lchan->mr_bts_lv[0];</span><br><span style="color: hsl(0, 100%, 40%);">- if (!len) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR, "Missing Multirate Config (len is zero)\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_tlv_put(msg, RSL_IE_MR_CONFIG, lchan->mr_bts_lv[0],</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->mr_bts_lv + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_put_u8(msg, RSL_IE_MR_CONFIG);</span><br><span style="color: hsl(120, 100%, 40%);">+ return gsm48_multirate_config(msg, mr_conf_filtered, mr_modes->bts_mode, mr_modes->num_modes);</span><br><span> }</span><br><span> </span><br><span> /* indicate FACCH/SACCH Repetition to be performed by BTS,</span><br><span>@@ -605,7 +595,16 @@</span><br><span> add_power_control_params(msg, RSL_IE_BS_POWER_PARAM, lchan);</span><br><span> add_power_control_params(msg, RSL_IE_MS_POWER_PARAM, lchan);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- mr_config_for_bts(lchan, msg, &lchan->activate.info.ch_mode_rate);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->activate.info.ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = put_mr_config_for_bts(msg, &lchan->activate.mr_conf_filtered,</span><br><span style="color: hsl(120, 100%, 40%);">+ (lchan->type == GSM_LCHAN_TCH_F) ? &bts->mr_full : &bts->mr_half);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR, "Cannot encode MultiRate Configuration IE\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</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%);">+</span><br><span> rep_acch_cap_for_bts(lchan, msg);</span><br><span> </span><br><span> msg->dst = trx->rsl_link;</span><br><span>@@ -635,6 +634,7 @@</span><br><span> </span><br><span> uint8_t chan_nr = gsm_lchan2chan_nr(lchan);</span><br><span> struct rsl_ie_chan_mode cm;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts *bts = lchan->ts->trx->bts;</span><br><span> </span><br><span> rc = channel_mode_from_lchan(&cm, lchan, &lchan->modify.info.ch_mode_rate);</span><br><span> if (rc < 0)</span><br><span>@@ -655,7 +655,16 @@</span><br><span> msgb_tlv_put(msg, RSL_IE_ENCR_INFO, rc, encr_info);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- mr_config_for_bts(lchan, msg, &lchan->modify.info.ch_mode_rate);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->modify.info.ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = put_mr_config_for_bts(msg, &lchan->modify.mr_conf_filtered,</span><br><span style="color: hsl(120, 100%, 40%);">+ (lchan->type == GSM_LCHAN_TCH_F) ? &bts->mr_full : &bts->mr_half);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR, "Cannot encode MultiRate Configuration IE\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</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%);">+</span><br><span> rep_acch_cap_for_bts(lchan, msg);</span><br><span> </span><br><span> msg->dst = lchan->ts->trx->rsl_link;</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 bcc61a5..df497c9 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>@@ -232,11 +232,11 @@</span><br><span> return GSM_CHREQ_REASON_OTHER;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static void mr_config_for_ms(struct gsm_lchan *lchan, struct msgb *msg)</span><br><span style="color: hsl(120, 100%, 40%);">+static int put_mr_config_for_ms(struct msgb *msg, const struct gsm48_multi_rate_conf *mr_conf_filtered,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct amr_multirate_conf *mr_modes)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan->current_ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR)</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_tlv_put(msg, GSM48_IE_MUL_RATE_CFG, lchan->mr_ms_lv[0],</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->mr_ms_lv + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_put_u8(msg, GSM48_IE_MUL_RATE_CFG);</span><br><span style="color: hsl(120, 100%, 40%);">+ return gsm48_multirate_config(msg, mr_conf_filtered, mr_modes->ms_mode, mr_modes->num_modes);</span><br><span> }</span><br><span> </span><br><span> #define CELL_SEL_IND_AFTER_REL_EARCFN_ENTRY (1+16+4+1+1)</span><br><span>@@ -397,12 +397,12 @@</span><br><span> }</span><br><span> </span><br><span> /*! \brief Encode a TS 04.08 multirate config LV according to 10.5.2.21aa.</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[out] lv caller-allocated buffer of 7 bytes. First octet is is length.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[out] msg msgb to append to.</span><br><span> * \param[in] mr_conf multi-rate configuration to encode (selected modes).</span><br><span> * \param[in] modes array describing the AMR modes.</span><br><span> * \param[in] num_modes length of the modes array.</span><br><span> * \returns 0 on success, -EINVAL on failure. */</span><br><span style="color: hsl(0, 100%, 40%);">-int gsm48_multirate_config(uint8_t *lv,</span><br><span style="color: hsl(120, 100%, 40%);">+int gsm48_multirate_config(struct msgb *msg,</span><br><span> const struct gsm48_multi_rate_conf *mr_conf,</span><br><span> const struct amr_mode *modes, unsigned int num_modes)</span><br><span> {</span><br><span>@@ -413,15 +413,17 @@</span><br><span> bool mode_valid;</span><br><span> uint8_t *gsm48_ie = (uint8_t *) mr_conf;</span><br><span> const struct amr_mode *modes_selected[4];</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *len;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t *data;</span><br><span> </span><br><span> /* Check if modes for consistency (order and duplicates) */</span><br><span style="color: hsl(0, 100%, 40%);">- for (i = 0; i < num_modes; i++) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (i > 0 && modes[i - 1].mode > modes[i].mode) {</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 1; i < num_modes; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (modes[i - 1].mode > modes[i].mode) {</span><br><span> LOGP(DRR, LOGL_ERROR,</span><br><span> "BUG: Multirate codec with inconsistent config (mode order).\n");</span><br><span> return -EINVAL;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (i > 0 && modes[i - 1].mode == modes[i].mode) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (modes[i - 1].mode == modes[i].mode) {</span><br><span> LOGP(DRR, LOGL_ERROR,</span><br><span> "BUG: Multirate codec with inconsistent config (duplicate modes).\n");</span><br><span> return -EINVAL;</span><br><span>@@ -482,28 +484,49 @@</span><br><span> </span><br><span> /* When the caller is not interested in any result, skip the actual</span><br><span> * composition of the IE (dry run) */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!lv)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!msg)</span><br><span> return 0;</span><br><span> </span><br><span> /* Compose output buffer */</span><br><span style="color: hsl(0, 100%, 40%);">- lv[0] = (num == 1) ? 2 : (num + 2);</span><br><span style="color: hsl(0, 100%, 40%);">- memcpy(lv + 1, gsm48_ie, 2);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* length */</span><br><span style="color: hsl(120, 100%, 40%);">+ len = msgb_put(msg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Write octet 3 (Multirate speech version, NSCB, ICMI, spare, Start mode)</span><br><span style="color: hsl(120, 100%, 40%);">+ * and octet 4 (Set of AMR codec modes) */</span><br><span style="color: hsl(120, 100%, 40%);">+ data = msgb_put(msg, 2);</span><br><span style="color: hsl(120, 100%, 40%);">+ memcpy(data, gsm48_ie, 2);</span><br><span> if (num == 1)</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto return_msg;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- lv[3] = modes_selected[0]->threshold & 0x3f;</span><br><span style="color: hsl(0, 100%, 40%);">- lv[4] = modes_selected[0]->hysteresis << 4;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* more than 1 mode: write octet 5 and 6: threshold 1 and hysteresis 1 */</span><br><span style="color: hsl(120, 100%, 40%);">+ data = msgb_put(msg, 2);</span><br><span style="color: hsl(120, 100%, 40%);">+ data[0] = modes_selected[0]->threshold & 0x3f;</span><br><span style="color: hsl(120, 100%, 40%);">+ data[1] = modes_selected[0]->hysteresis << 4;</span><br><span> if (num == 2)</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">- lv[4] |= (modes_selected[1]->threshold & 0x3f) >> 2;</span><br><span style="color: hsl(0, 100%, 40%);">- lv[5] = modes_selected[1]->threshold << 6;</span><br><span style="color: hsl(0, 100%, 40%);">- lv[5] |= (modes_selected[1]->hysteresis & 0x0f) << 2;</span><br><span style="color: hsl(0, 100%, 40%);">- if (num == 3)</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">- lv[5] |= (modes_selected[2]->threshold & 0x3f) >> 4;</span><br><span style="color: hsl(0, 100%, 40%);">- lv[6] = modes_selected[2]->threshold << 4;</span><br><span style="color: hsl(0, 100%, 40%);">- lv[6] |= modes_selected[2]->hysteresis & 0x0f;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto return_msg;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* more than 2 modes: complete octet 6 and add octet 7: threshold 2 and hysteresis 2.</span><br><span style="color: hsl(120, 100%, 40%);">+ * Threshold 2 starts in octet 6. */</span><br><span style="color: hsl(120, 100%, 40%);">+ data[1] |= (modes_selected[1]->threshold & 0x3f) >> 2;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* octet 7 */</span><br><span style="color: hsl(120, 100%, 40%);">+ data = msgb_put(msg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ data[0] = modes_selected[1]->threshold << 6;</span><br><span style="color: hsl(120, 100%, 40%);">+ data[0] |= (modes_selected[1]->hysteresis & 0x0f) << 2;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (num == 3)</span><br><span style="color: hsl(120, 100%, 40%);">+ goto return_msg;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* four modes: complete octet 7 and add octet 8: threshold 3 and hysteresis 3.</span><br><span style="color: hsl(120, 100%, 40%);">+ * Threshold 3 starts in octet 7. */</span><br><span style="color: hsl(120, 100%, 40%);">+ data[0] |= (modes_selected[2]->threshold & 0x3f) >> 4;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* octet 8 */</span><br><span style="color: hsl(120, 100%, 40%);">+ data = msgb_put(msg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ data[0] = modes_selected[2]->threshold << 4;</span><br><span style="color: hsl(120, 100%, 40%);">+ data[0] |= modes_selected[2]->hysteresis & 0x0f;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+return_msg:</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Place written len in the IE length field. msg->tail points one byte after the last data octet, len points at</span><br><span style="color: hsl(120, 100%, 40%);">+ * the L octet of the TLV. */</span><br><span style="color: hsl(120, 100%, 40%);">+ *len = (msg->tail - 1) - len;</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>@@ -516,6 +539,7 @@</span><br><span> struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_put(msg, sizeof(*gh));</span><br><span> struct gsm48_ho_cmd *ho =</span><br><span> (struct gsm48_ho_cmd *) msgb_put(msg, sizeof(*ho));</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts *bts = new_lchan->ts->trx->bts;</span><br><span> </span><br><span> gh->proto_discr = GSM48_PDISC_RR;</span><br><span> gh->msg_type = GSM48_MT_RR_HANDO_CMD;</span><br><span>@@ -548,9 +572,14 @@</span><br><span> }</span><br><span> </span><br><span> /* in case of multi rate we need to attach a config */</span><br><span style="color: hsl(0, 100%, 40%);">- if (new_lchan->current_ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR)</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_tlv_put(msg, GSM48_IE_MUL_RATE_CFG, new_lchan->mr_ms_lv[0],</span><br><span style="color: hsl(0, 100%, 40%);">- new_lchan->mr_ms_lv + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (new_lchan->current_ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (put_mr_config_for_ms(msg, &new_lchan->current_mr_conf,</span><br><span style="color: hsl(120, 100%, 40%);">+ (new_lchan->type == GSM_LCHAN_TCH_F) ? &bts->mr_full : &bts->mr_half)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(new_lchan, LOGL_ERROR, "Cannot encode MultiRate Configuration IE\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> return msg;</span><br><span> }</span><br><span>@@ -572,9 +601,10 @@</span><br><span> struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_put(msg, sizeof(*gh));</span><br><span> struct gsm48_ass_cmd *ass =</span><br><span> (struct gsm48_ass_cmd *) msgb_put(msg, sizeof(*ass));</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts *bts = new_lchan->ts->trx->bts;</span><br><span> </span><br><span> DEBUGP(DRR, "-> ASSIGNMENT COMMAND tch_mode=0x%02x\n",</span><br><span style="color: hsl(0, 100%, 40%);">- current_lchan->conn->assignment.selected_ch_mode_rate.chan_mode);</span><br><span style="color: hsl(120, 100%, 40%);">+ new_lchan->current_ch_mode_rate.chan_mode);</span><br><span> </span><br><span> msg->lchan = current_lchan;</span><br><span> gh->proto_discr = GSM48_PDISC_RR;</span><br><span>@@ -593,14 +623,13 @@</span><br><span> </span><br><span> /* Cell Channel Description (freq. hopping), TV (see 3GPP TS 44.018, 10.5.2.1b) */</span><br><span> if (new_lchan->ts->hopping.enabled) {</span><br><span style="color: hsl(0, 100%, 40%);">- const struct gsm48_system_information_type_1 *si1 = \</span><br><span style="color: hsl(0, 100%, 40%);">- GSM_BTS_SI(new_lchan->ts->trx->bts, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct gsm48_system_information_type_1 *si1 = GSM_BTS_SI(bts, 1);</span><br><span> msgb_tv_fixed_put(msg, GSM48_IE_CELL_CH_DESC,</span><br><span> sizeof(si1->cell_channel_description),</span><br><span> si1->cell_channel_description);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- msgb_tv_put(msg, GSM48_IE_CHANMODE_1, current_lchan->conn->assignment.selected_ch_mode_rate.chan_mode);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_tv_put(msg, GSM48_IE_CHANMODE_1, new_lchan->current_ch_mode_rate.chan_mode);</span><br><span> </span><br><span> /* Mobile Allocation (freq. hopping), TLV (see 3GPP TS 44.018, 10.5.2.21) */</span><br><span> if (new_lchan->ts->hopping.enabled) {</span><br><span>@@ -609,7 +638,15 @@</span><br><span> }</span><br><span> </span><br><span> /* in case of multi rate we need to attach a config */</span><br><span style="color: hsl(0, 100%, 40%);">- mr_config_for_ms(new_lchan, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (new_lchan->current_ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc = put_mr_config_for_ms(msg, &new_lchan->current_mr_conf,</span><br><span style="color: hsl(120, 100%, 40%);">+ (new_lchan->type == GSM_LCHAN_TCH_F) ? &bts->mr_full : &bts->mr_half);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(current_lchan, LOGL_ERROR, "Cannot encode MultiRate Configuration IE\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> return gsm48_sendmsg(msg);</span><br><span> }</span><br><span>@@ -643,6 +680,7 @@</span><br><span> struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_put(msg, sizeof(*gh));</span><br><span> struct gsm48_chan_mode_modify *cmm =</span><br><span> (struct gsm48_chan_mode_modify *) msgb_put(msg, sizeof(*cmm));</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts *bts = lchan->ts->trx->bts;</span><br><span> </span><br><span> DEBUGP(DRR, "-> CHANNEL MODE MODIFY mode=0x%02x\n", mode);</span><br><span> </span><br><span>@@ -656,7 +694,15 @@</span><br><span> cmm->mode = mode;</span><br><span> </span><br><span> /* in case of multi rate we need to attach a config */</span><br><span style="color: hsl(0, 100%, 40%);">- mr_config_for_ms(lchan, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->modify.info.ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(120, 100%, 40%);">+ int rc = put_mr_config_for_ms(msg, &lchan->modify.mr_conf_filtered,</span><br><span style="color: hsl(120, 100%, 40%);">+ (lchan->type == GSM_LCHAN_TCH_F) ? &bts->mr_full : &bts->mr_half);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR, "Cannot encode MultiRate Configuration IE\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> return gsm48_sendmsg(msg);</span><br><span> }</span><br><span>diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c</span><br><span>index 3d181ff..0e0f1aa 100644</span><br><span>--- a/src/osmo-bsc/lchan_fsm.c</span><br><span>+++ b/src/osmo-bsc/lchan_fsm.c</span><br><span>@@ -546,51 +546,23 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int mr_config_render_lv(uint8_t *mr_ms_lv, uint8_t *mr_bts_lv,</span><br><span style="color: hsl(0, 100%, 40%);">- const struct gsm48_multi_rate_conf *mr_conf,</span><br><span style="color: hsl(0, 100%, 40%);">- const struct amr_multirate_conf *amr_mrc,</span><br><span style="color: hsl(0, 100%, 40%);">- const struct gsm_lchan *logging)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 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(0, 100%, 40%);">- LOG_LCHAN(logging, 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%);">- if (gsm48_multirate_config(mr_bts_lv, mr_conf, amr_mrc->bts_mode, amr_mrc->num_modes)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(logging, 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(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</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 lchan_mr_config(struct gsm48_multi_rate_conf *mr_conf, const struct gsm_lchan *lchan, uint16_t s15_s0)</span><br><span> {</span><br><span> struct gsm_bts *bts = lchan->ts->trx->bts;</span><br><span> bool full_rate = lchan->type == GSM_LCHAN_TCH_F;</span><br><span> struct amr_multirate_conf *amr_mrc = full_rate ? &bts->mr_full : &bts->mr_half;</span><br><span> struct gsm48_multi_rate_conf *mr_filter_msc = NULL;</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%);">- int rc;</span><br><span> </span><br><span> /* If activated for VTY, there may not be a conn indicating an MSC AMR configuration. */</span><br><span> if (lchan->conn && lchan->conn->sccp.msc)</span><br><span> mr_filter_msc = &lchan->conn->sccp.msc->amr_conf;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- rc = mr_config_filter(&mr_conf,</span><br><span style="color: hsl(0, 100%, 40%);">- full_rate,</span><br><span style="color: hsl(0, 100%, 40%);">- amr_mrc, mr_filter_msc,</span><br><span style="color: hsl(0, 100%, 40%);">- s15_s0,</span><br><span style="color: hsl(0, 100%, 40%);">- lchan);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rc)</span><br><span style="color: hsl(0, 100%, 40%);">- return rc;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* 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 style="color: hsl(0, 100%, 40%);">- * 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 = mr_config_render_lv(lchan->mr_ms_lv, lchan->mr_bts_lv, &mr_conf, amr_mrc, lchan);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rc)</span><br><span style="color: hsl(0, 100%, 40%);">- return rc;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ return 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_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> }</span><br><span> </span><br><span> static void lchan_fsm_unused(struct osmo_fsm_inst *fi, uint32_t event, void *data)</span><br><span>@@ -655,7 +627,7 @@</span><br><span> }</span><br><span> </span><br><span> if (info->ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan_mr_config(lchan, info->ch_mode_rate.s15_s0) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan_mr_config(&lchan->activate.mr_conf_filtered, lchan, info->ch_mode_rate.s15_s0) < 0) {</span><br><span> lchan_fail("Can not generate multirate configuration IE\n");</span><br><span> return;</span><br><span> }</span><br><span>@@ -813,6 +785,7 @@</span><br><span> struct gsm_lchan *lchan = lchan_fi_lchan(fi);</span><br><span> </span><br><span> lchan->current_ch_mode_rate = lchan->activate.info.ch_mode_rate;</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->current_mr_conf = lchan->activate.mr_conf_filtered;</span><br><span> LOG_LCHAN(lchan, LOGL_INFO, "Rx Activ ACK %s\n",</span><br><span> gsm48_chan_mode_name(lchan->current_ch_mode_rate.chan_mode));</span><br><span> </span><br><span>@@ -984,6 +957,7 @@</span><br><span> case LCHAN_EV_RSL_CHAN_MODE_MODIFY_ACK:</span><br><span> /* The Channel Mode Modify was ACKed, now the requested values become the accepted and used values. */</span><br><span> lchan->current_ch_mode_rate = lchan->modify.info.ch_mode_rate;</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->current_mr_conf = lchan->modify.mr_conf_filtered;</span><br><span> </span><br><span> if (lchan->modify.info.requires_voice_stream</span><br><span> && !lchan->fi_rtp) {</span><br><span>@@ -1136,7 +1110,8 @@</span><br><span> use_mgwep_ci = lchan_use_mgw_endpoint_ci_bts(lchan);</span><br><span> </span><br><span> if (modif_info->ch_mode_rate.chan_mode == GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan_mr_config(lchan, modif_info->ch_mode_rate.s15_s0) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan_mr_config(&lchan->modify.mr_conf_filtered, lchan, modif_info->ch_mode_rate.s15_s0)</span><br><span style="color: hsl(120, 100%, 40%);">+ < 0) {</span><br><span> lchan_fail("Can not generate multirate configuration IE\n");</span><br><span> return;</span><br><span> }</span><br><span>diff --git a/tests/gsm0408/gsm0408_test.c b/tests/gsm0408/gsm0408_test.c</span><br><span>index 8220c4f..a1aa5f4 100644</span><br><span>--- a/tests/gsm0408/gsm0408_test.c</span><br><span>+++ b/tests/gsm0408/gsm0408_test.c</span><br><span>@@ -780,10 +780,10 @@</span><br><span> </span><br><span> static void test_gsm48_multirate_config()</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t lv[7];</span><br><span> struct gsm48_multi_rate_conf *gsm48_ie;</span><br><span> struct amr_multirate_conf mr;</span><br><span> int rc;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct msgb *msg = msgb_alloc(32, "test_gsm48_multirate_config");</span><br><span> </span><br><span> memset(&mr, 0, sizeof(mr));</span><br><span> </span><br><span>@@ -807,17 +807,19 @@</span><br><span> mr.ms_mode[1].mode = 4;</span><br><span> mr.ms_mode[2].mode = 5;</span><br><span> mr.ms_mode[3].mode = 7;</span><br><span style="color: hsl(0, 100%, 40%);">- rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_trim(msg, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(msg, gsm48_ie, mr.ms_mode, 4);</span><br><span> OSMO_ASSERT(rc == 0);</span><br><span> printf("gsm48_multirate_config(): rc=%i, lv=%s\n", rc,</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_hexdump_nospc(lv, 1 + lv[0]));</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_hexdump_nospc(msg->data, msg->len));</span><br><span> </span><br><span> /* Test #2: 4 active set members, but wrong mode order: */</span><br><span> mr.ms_mode[3].mode = 2;</span><br><span> mr.ms_mode[2].mode = 4;</span><br><span> mr.ms_mode[1].mode = 5;</span><br><span> mr.ms_mode[0].mode = 7;</span><br><span style="color: hsl(0, 100%, 40%);">- rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_trim(msg, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(msg, gsm48_ie, mr.ms_mode, 4);</span><br><span> OSMO_ASSERT(rc == -EINVAL);</span><br><span> </span><br><span> /* Test #3: Normal configuration with 3 active set members */</span><br><span>@@ -829,16 +831,18 @@</span><br><span> mr.ms_mode[2].threshold = 0;</span><br><span> mr.ms_mode[2].hysteresis = 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_trim(msg, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(msg, gsm48_ie, mr.ms_mode, 3);</span><br><span> OSMO_ASSERT(rc == 0);</span><br><span> printf("gsm48_multirate_config(): rc=%i, lv=%s\n", rc,</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_hexdump_nospc(lv, 1 + lv[0]));</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_hexdump_nospc(msg->data, msg->len));</span><br><span> </span><br><span> /* Test #4: 3 active set members, but wrong mode order: */</span><br><span> mr.ms_mode[0].mode = 2;</span><br><span> mr.ms_mode[2].mode = 4;</span><br><span> mr.ms_mode[1].mode = 5;</span><br><span style="color: hsl(0, 100%, 40%);">- rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_trim(msg, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(msg, gsm48_ie, mr.ms_mode, 3);</span><br><span> OSMO_ASSERT(rc == -EINVAL);</span><br><span> </span><br><span> /* Test #5: Normal configuration with 2 active set members */</span><br><span>@@ -850,15 +854,17 @@</span><br><span> mr.ms_mode[1].threshold = 0;</span><br><span> mr.ms_mode[1].hysteresis = 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_trim(msg, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(msg, gsm48_ie, mr.ms_mode, 2);</span><br><span> OSMO_ASSERT(rc == 0);</span><br><span> printf("gsm48_multirate_config(): rc=%i, lv=%s\n", rc,</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_hexdump_nospc(lv, 1 + lv[0]));</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_hexdump_nospc(msg->data, msg->len));</span><br><span> </span><br><span> /* Test #6: 2 active set members, but wrong mode order: */</span><br><span> mr.ms_mode[1].mode = 2;</span><br><span> mr.ms_mode[0].mode = 4;</span><br><span style="color: hsl(0, 100%, 40%);">- rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_trim(msg, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(msg, gsm48_ie, mr.ms_mode, 2);</span><br><span> OSMO_ASSERT(rc == -EINVAL);</span><br><span> </span><br><span> /* Test #7: Normal configuration with 1 active set member */</span><br><span>@@ -870,15 +876,19 @@</span><br><span> mr.ms_mode[0].threshold = 0;</span><br><span> mr.ms_mode[0].hysteresis = 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_trim(msg, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(msg, gsm48_ie, mr.ms_mode, 1);</span><br><span> OSMO_ASSERT(rc == 0);</span><br><span> printf("gsm48_multirate_config(): rc=%i, lv=%s\n", rc,</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_hexdump_nospc(lv, 1 + lv[0]));</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_hexdump_nospc(msg->data, msg->len));</span><br><span> </span><br><span> /* Test #8: 0 active set members: */</span><br><span> mr.ms_mode[0].mode = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_trim(msg, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = gsm48_multirate_config(msg, gsm48_ie, mr.ms_mode, 1);</span><br><span> OSMO_ASSERT(rc == -EINVAL);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_free(msg);</span><br><span> }</span><br><span> </span><br><span> static const struct log_info_cat log_categories[] = {</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/24358">change 24358</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/+/24358"/><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: Ie57f9d0e3912632903d9740291225bfd1634ed47 </div>
<div style="display:none"> Gerrit-Change-Number: 24358 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>