This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
dexter gerrit-no-reply at lists.osmocom.orgdexter has submitted this change and it was merged. ( https://gerrit.osmocom.org/11443 )
Change subject: gsm_04_08: improve gsm48_multirate_config()
......................................................................
gsm_04_08: improve gsm48_multirate_config()
The function gsm48_multirate_config() generates the multirate
configuration IE, that is sent via RSL to configure the active set of
AMR codecs inside the BTS. The function already works, but it does not
check the input data for consistancy. Lets add some consistancy check to
make sure that inconsistant parameters are rejected. Also allow the
output pointer to be NULL, so that the function can be used to perform
a dry run to be able to verify parameters.
- Check for invalid / inconsistant configuration parameters
- Perform a dry-run when lv pointer is set to NULL
Change-Id: I06beb7dd7236c81c3a91af4d09c31891f4b910a4
Related: OS#3529
---
M include/osmocom/bsc/gsm_04_08_rr.h
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/gsm_04_08_rr.c
M tests/gsm0408/Makefile.am
M tests/gsm0408/gsm0408_test.c
M tests/gsm0408/gsm0408_test.ok
6 files changed, 237 insertions(+), 26 deletions(-)
Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
diff --git a/include/osmocom/bsc/gsm_04_08_rr.h b/include/osmocom/bsc/gsm_04_08_rr.h
index e2e861d..8e4f787 100644
--- a/include/osmocom/bsc/gsm_04_08_rr.h
+++ b/include/osmocom/bsc/gsm_04_08_rr.h
@@ -23,7 +23,8 @@
struct msgb *msg, struct bsc_subscr *bsub);
int gsm48_send_rr_classmark_enquiry(struct gsm_lchan *lchan);
int gsm48_send_rr_ciph_mode(struct gsm_lchan *lchan, int want_imeisv);
-int gsm48_multirate_config(uint8_t *lv, const struct amr_multirate_conf *mr, const struct amr_mode *modes);
+int gsm48_multirate_config(uint8_t *lv, const struct gsm48_multi_rate_conf *mr_conf,
+ const struct amr_mode *modes, unsigned int num_modes);
struct msgb *gsm48_make_ho_cmd(struct gsm_lchan *new_lchan, uint8_t power_command, uint8_t ho_ref);
int gsm48_send_ho_cmd(struct gsm_lchan *old_lchan, struct gsm_lchan *new_lchan,
uint8_t power_command, uint8_t ho_ref);
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index f156cc8..08b5dad 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -4391,11 +4391,12 @@
mr.ms_mode[0].mode = amr_mode;
mr.bts_mode[0].mode = amr_mode;
+ mr.num_modes = 1;
/* encode this configuration into the lchan for both uplink and
* downlink direction */
- gsm48_multirate_config(lchan->mr_ms_lv, &mr, mr.ms_mode);
- gsm48_multirate_config(lchan->mr_bts_lv, &mr, mr.bts_mode);
+ gsm48_multirate_config(lchan->mr_ms_lv, mr_conf, mr.ms_mode, mr.num_modes);
+ gsm48_multirate_config(lchan->mr_bts_lv, mr_conf, mr.bts_mode, mr.num_modes);
return 0;
}
diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c
index 35044a3..4659c1a 100644
--- a/src/osmo-bsc/gsm_04_08_rr.c
+++ b/src/osmo-bsc/gsm_04_08_rr.c
@@ -321,47 +321,113 @@
cd->arfcn_lo = bts->c0->arfcn & 0xff;
}
-/*! \brief Encode a TS 04.08 multirate config LV according to 10.5.2.21aa
- * \param[out] lv caller-allocated buffer of 7 bytes. First octet is IS length
- * \param[in] mr multi-rate configuration to encode
- * \param[in] modes array describing the AMR modes
- * \returns 0 on success */
-int gsm48_multirate_config(uint8_t *lv, const struct amr_multirate_conf *mr, const struct amr_mode *modes)
+/*! \brief Encode a TS 04.08 multirate config LV according to 10.5.2.21aa.
+ * \param[out] lv caller-allocated buffer of 7 bytes. First octet is is length.
+ * \param[in] mr_conf multi-rate configuration to encode (selected modes).
+ * \param[in] modes array describing the AMR modes.
+ * \param[in] num_modes length of the modes array.
+ * \returns 0 on success, -EINVAL on failure. */
+int gsm48_multirate_config(uint8_t *lv,
+ const struct gsm48_multi_rate_conf *mr_conf,
+ const struct amr_mode *modes, unsigned int num_modes)
{
- int num = 0, i;
+ int num = 0;
+ unsigned int i;
+ unsigned int k;
+ unsigned int m = 0;
+ bool mode_valid;
+ uint8_t *gsm48_ie = (uint8_t *) mr_conf;
+ const struct amr_mode *modes_selected[4];
+ /* Check if modes for consistency (order and duplicates) */
+ for (i = 0; i < num_modes; i++) {
+ if (i > 0 && modes[i - 1].mode > modes[i].mode) {
+ LOGP(DRR, LOGL_ERROR,
+ "BUG: Multirate codec with inconsistant config (mode order).\n");
+ return -EINVAL;
+ }
+ if (i > 0 && modes[i - 1].mode == modes[i].mode) {
+ LOGP(DRR, LOGL_ERROR,
+ "BUG: Multirate codec with inconsistant config (duplicate modes).\n");
+ return -EINVAL;
+ }
+ }
+
+ /* Check if the active set that is defined in mr_conf has at least one
+ * mode but not more than 4 modes set */
for (i = 0; i < 8; i++) {
- if (((mr->gsm48_ie[1] >> i) & 1))
+ if (((gsm48_ie[1] >> i) & 1))
num++;
}
if (num > 4) {
- LOGP(DRR, LOGL_ERROR, "BUG: Using multirate codec with too "
- "many modes in config.\n");
- num = 4;
+ LOGP(DRR, LOGL_ERROR,
+ "BUG: Multirate codec with too many modes in config.\n");
+ return -EINVAL;
}
if (num < 1) {
- LOGP(DRR, LOGL_ERROR, "BUG: Using multirate codec with no "
- "mode in config.\n");
- num = 1;
+ LOGP(DRR, LOGL_ERROR,
+ "BUG: Multirate codec with no mode in config.\n");
+ return -EINVAL;
}
+ /* Do not accept excess hysteresis or threshold values */
+ for (i = 0; i < num_modes; i++) {
+ if (modes[i].threshold >= 64) {
+ LOGP(DRR, LOGL_ERROR,
+ "BUG: Multirate codec with excessive threshold values.\n");
+ return -EINVAL;
+ }
+ if (modes[i].hysteresis >= 16) {
+ LOGP(DRR, LOGL_ERROR,
+ "BUG: Multirate codec with excessive hysteresis values.\n");
+ return -EINVAL;
+ }
+ }
+
+ /* Scan through the selected modes and find a matching threshold/
+ * hysteresis value for that mode. */
+ for (i = 0; i < 8; i++) {
+ if (((gsm48_ie[1] >> i) & 1)) {
+ mode_valid = false;
+ for (k = 0; k < num_modes; k++) {
+ if (modes[k].mode == i) {
+ mode_valid = true;
+ modes_selected[m] = &modes[k];
+ m++;
+ }
+ }
+ if (!mode_valid) {
+ LOGP(DRR, LOGL_ERROR,
+ "BUG: Multirate codec with inconsistant config (no mode defined).\n");
+ return -EINVAL;
+ }
+ }
+ }
+ OSMO_ASSERT(m <= 4);
+
+ /* When the caller is not interested in any result, skip the actual
+ * composition of the IE (dry run) */
+ if (!lv)
+ return 0;
+
+ /* Compose output buffer */
lv[0] = (num == 1) ? 2 : (num + 2);
- memcpy(lv + 1, mr->gsm48_ie, 2);
+ memcpy(lv + 1, gsm48_ie, 2);
if (num == 1)
return 0;
- lv[3] = modes[0].threshold & 0x3f;
- lv[4] = modes[0].hysteresis << 4;
+ lv[3] = modes_selected[0]->threshold & 0x3f;
+ lv[4] = modes_selected[0]->hysteresis << 4;
if (num == 2)
return 0;
- lv[4] |= (modes[1].threshold & 0x3f) >> 2;
- lv[5] = modes[1].threshold << 6;
- lv[5] |= (modes[1].hysteresis & 0x0f) << 2;
+ lv[4] |= (modes_selected[1]->threshold & 0x3f) >> 2;
+ lv[5] = modes_selected[1]->threshold << 6;
+ lv[5] |= (modes_selected[1]->hysteresis & 0x0f) << 2;
if (num == 3)
return 0;
- lv[5] |= (modes[2].threshold & 0x3f) >> 4;
- lv[6] = modes[2].threshold << 4;
- lv[6] |= modes[2].hysteresis & 0x0f;
+ lv[5] |= (modes_selected[2]->threshold & 0x3f) >> 4;
+ lv[6] = modes_selected[2]->threshold << 4;
+ lv[6] |= modes_selected[2]->hysteresis & 0x0f;
return 0;
}
diff --git a/tests/gsm0408/Makefile.am b/tests/gsm0408/Makefile.am
index d790fc8..b207f8b 100644
--- a/tests/gsm0408/Makefile.am
+++ b/tests/gsm0408/Makefile.am
@@ -23,6 +23,7 @@
$(NULL)
gsm0408_test_LDADD = \
+ $(top_builddir)/src/osmo-bsc/gsm_04_08_rr.o \
$(top_builddir)/src/osmo-bsc/arfcn_range_encode.o \
$(top_builddir)/src/osmo-bsc/gsm_data.o \
$(top_builddir)/src/osmo-bsc/gsm_timers.o \
diff --git a/tests/gsm0408/gsm0408_test.c b/tests/gsm0408/gsm0408_test.c
index bc2777d..faeca39 100644
--- a/tests/gsm0408/gsm0408_test.c
+++ b/tests/gsm0408/gsm0408_test.c
@@ -35,6 +35,8 @@
#include <osmocom/gsm/sysinfo.h>
#include <osmocom/gsm/gsm48.h>
+#include <osmocom/bsc/gsm_04_08_rr.h>
+
#define COMPARE(result, op, value) \
if (!((result) op (value))) {\
fprintf(stderr, "Compare failed. Was %x should be %x in %s:%d\n",result, value, __FILE__, __LINE__); \
@@ -799,6 +801,106 @@
OSMO_ASSERT(pass);
}
+static void test_gsm48_multirate_config()
+{
+ uint8_t lv[7];
+ struct gsm48_multi_rate_conf *gsm48_ie;
+ struct amr_multirate_conf mr;
+ int rc;
+
+ memset(&mr, 0, sizeof(mr));
+
+ /* Use some made up threshold and hysteresis values */
+ mr.ms_mode[0].threshold = 11;
+ mr.ms_mode[1].threshold = 12;
+ mr.ms_mode[2].threshold = 13;
+ mr.ms_mode[0].hysteresis = 15;
+ mr.ms_mode[1].hysteresis = 12;
+ mr.ms_mode[2].hysteresis = 8;
+
+ gsm48_ie = (struct gsm48_multi_rate_conf *)&mr.gsm48_ie;
+ gsm48_ie->ver = 1;
+ gsm48_ie->m5_90 = 1;
+ gsm48_ie->m7_40 = 1;
+ gsm48_ie->m7_95 = 1;
+ gsm48_ie->m12_2 = 1;
+
+ /* Test #1: Normal configuration with 4 active set members */
+ mr.ms_mode[0].mode = 2;
+ mr.ms_mode[1].mode = 4;
+ mr.ms_mode[2].mode = 5;
+ mr.ms_mode[3].mode = 7;
+ rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);
+ OSMO_ASSERT(rc == 0);
+ printf("gsm48_multirate_config(): rc=%i, lv=%s\n", rc,
+ osmo_hexdump_nospc(lv, lv[0]));
+
+ /* Test #2: 4 active set members, but wrong mode order: */
+ mr.ms_mode[3].mode = 2;
+ mr.ms_mode[2].mode = 4;
+ mr.ms_mode[1].mode = 5;
+ mr.ms_mode[0].mode = 7;
+ rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 4);
+ OSMO_ASSERT(rc == -EINVAL);
+
+ /* Test #3: Normal configuration with 3 active set members */
+ mr.ms_mode[0].mode = 2;
+ mr.ms_mode[1].mode = 4;
+ mr.ms_mode[2].mode = 5;
+ mr.ms_mode[3].mode = 0;
+ gsm48_ie->m12_2 = 0;
+ mr.ms_mode[2].threshold = 0;
+ mr.ms_mode[2].hysteresis = 0;
+
+ rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 3);
+ OSMO_ASSERT(rc == 0);
+ printf("gsm48_multirate_config(): rc=%i, lv=%s\n", rc,
+ osmo_hexdump_nospc(lv, lv[0]));
+
+ /* Test #4: 3 active set members, but wrong mode order: */
+ mr.ms_mode[0].mode = 2;
+ mr.ms_mode[2].mode = 4;
+ mr.ms_mode[1].mode = 5;
+ rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 3);
+ OSMO_ASSERT(rc == -EINVAL);
+
+ /* Test #5: Normal configuration with 2 active set members */
+ mr.ms_mode[0].mode = 2;
+ mr.ms_mode[1].mode = 4;
+ mr.ms_mode[2].mode = 0;
+ gsm48_ie->m7_95 = 0;
+ mr.ms_mode[1].threshold = 0;
+ mr.ms_mode[1].hysteresis = 0;
+
+ rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 2);
+ OSMO_ASSERT(rc == 0);
+ printf("gsm48_multirate_config(): rc=%i, lv=%s\n", rc,
+ osmo_hexdump_nospc(lv, lv[0]));
+
+ /* Test #6: 2 active set members, but wrong mode order: */
+ mr.ms_mode[1].mode = 2;
+ mr.ms_mode[0].mode = 4;
+ rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 2);
+ OSMO_ASSERT(rc == -EINVAL);
+
+ /* Test #7: Normal configuration with 1 active set member */
+ mr.ms_mode[0].mode = 2;
+ mr.ms_mode[1].mode = 0;
+ gsm48_ie->m7_40 = 0;
+ mr.ms_mode[0].threshold = 0;
+ mr.ms_mode[0].hysteresis = 0;
+
+ rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 1);
+ OSMO_ASSERT(rc == 0);
+ printf("gsm48_multirate_config(): rc=%i, lv=%s\n", rc,
+ osmo_hexdump_nospc(lv, lv[0]));
+
+ /* Test #8: 0 active set members: */
+ mr.ms_mode[0].mode = 0;
+ rc = gsm48_multirate_config(lv, gsm48_ie, mr.ms_mode, 1);
+ OSMO_ASSERT(rc == -EINVAL);
+}
+
static const struct log_info_cat log_categories[] = {
};
@@ -839,6 +941,8 @@
test_gsm48_ra_id_by_bts();
+ test_gsm48_multirate_config();
+
printf("Done.\n");
return EXIT_SUCCESS;
@@ -854,3 +958,37 @@
}
void ts_fsm_alloc(struct gsm_bts_trx_ts *ts) {}
+
+void bsc_cm_update(struct gsm_subscriber_connection *conn,
+ const uint8_t *cm2, uint8_t cm2_len,
+ const uint8_t *cm3, uint8_t cm3_len) {}
+
+int rsl_siemens_mrpci(struct gsm_lchan *lchan, struct rsl_mrpci *mrpci)
+{ return 0; }
+
+int rsl_chan_mode_modify_req(struct gsm_lchan *ts) { return 0; }
+
+int rsl_tx_ipacc_crcx(struct gsm_lchan *lchan) { return 0; }
+
+void gscon_submit_rsl_dtap(struct gsm_subscriber_connection *conn,
+ struct msgb *msg, int link_id, int allow_sacch) {}
+
+bool lchan_may_receive_data(struct gsm_lchan *lchan) { return true; }
+
+int bsc_compl_l3(struct gsm_subscriber_connection *conn, struct msgb *msg,
+ uint16_t chosen_channel) { return 0; }
+
+void bsc_dtap(struct gsm_subscriber_connection *conn, uint8_t link_id,
+ struct msgb *msg) {}
+
+void bsc_cipher_mode_compl(struct gsm_subscriber_connection *conn,
+ struct msgb *msg, uint8_t chosen_encr) {}
+
+const char *bsc_subscr_name(struct bsc_subscr *bsub) { return NULL; }
+
+void lchan_release(struct gsm_lchan *lchan, bool sacch_deact,
+ bool err, enum gsm48_rr_cause cause_rr) {}
+
+int rsl_data_request(struct msgb *msg, uint8_t link_id) { return 0; }
+
+int rsl_encryption_cmd(struct msgb *msg) { return 0; }
diff --git a/tests/gsm0408/gsm0408_test.ok b/tests/gsm0408/gsm0408_test.ok
index 7270721..9424167 100644
--- a/tests/gsm0408/gsm0408_test.ok
+++ b/tests/gsm0408/gsm0408_test.ok
@@ -226,4 +226,8 @@
test_gsm48_ra_id_by_bts[4]: digits='999999' lac=0xffff=htons(65535) rac=0xff=255 pass
test_gsm48_ra_id_by_bts[5]: digits='09f909' lac=0xcdab=htons(43981) rac=0xab=171 pass
test_gsm48_ra_id_by_bts[6]: digits='090990' lac=0xcdab=htons(43981) rac=0xab=171 pass
+gsm48_multirate_config(): rc=0, lv=0620b40bf330
+gsm48_multirate_config(): rc=0, lv=0520340bf3
+gsm48_multirate_config(): rc=0, lv=0420140b
+gsm48_multirate_config(): rc=0, lv=0220
Done.
--
To view, visit https://gerrit.osmocom.org/11443
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I06beb7dd7236c81c3a91af4d09c31891f4b910a4
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181024/fe343289/attachment.htm>