neels has submitted this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/31609 )
Change subject: simplify storage of bsc_msc_data->audio_support
......................................................................
simplify storage of bsc_msc_data->audio_support
Make it a plain array, no dynamic allocation needed.
Change-Id: I625cedc4bb040d649fd6e1794ba468f4c6ad6adc
---
M include/osmocom/bsc/bsc_msc_data.h
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/codec_pref.c
M src/osmo-bsc/osmo_bsc_msc.c
M tests/codec_pref/codec_pref_test.c
M tests/msc.vty
6 files changed, 87 insertions(+), 130 deletions(-)
Approvals:
laforge: Looks good to me, but someone else must approve
neels: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmocom/bsc/bsc_msc_data.h
b/include/osmocom/bsc/bsc_msc_data.h
index 3568716..47bdf12 100644
--- a/include/osmocom/bsc/bsc_msc_data.h
+++ b/include/osmocom/bsc/bsc_msc_data.h
@@ -133,8 +133,13 @@
/* audio codecs */
struct gsm48_multi_rate_conf amr_conf;
bool amr_octet_aligned;
- struct gsm_audio_support **audio_support;
+
+ /* Practically, there can be only 5 entries in osmo-bsc: FR1 FR2 FR3
HR1 HR3. Historically, osmo-bsc allowed
+ * *any* number of entries -- we should not break too strictly on old
cfg files. Theoretically, there should be
+ * room for FR1 to FR7 plus HR1 to HR7 less HR2. Let's just give ample
room for all these aspects: */
+ struct gsm_audio_support audio_support[16];
int audio_length;
+
enum bsc_lcls_mode lcls_mode;
bool lcls_codec_mismatch_allow;
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index bba84e2..e0a7a95 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -2550,10 +2550,10 @@
if (i != 0)
vty_out(vty, " ");
- if (msc->audio_support[i]->hr)
- vty_out(vty, "hr%u",
msc->audio_support[i]->ver);
+ if (msc->audio_support[i].hr)
+ vty_out(vty, "hr%u", msc->audio_support[i].ver);
else
- vty_out(vty, "fr%u",
msc->audio_support[i]->ver);
+ vty_out(vty, "fr%u", msc->audio_support[i].ver);
}
vty_out(vty, "%s", VTY_NEWLINE);
@@ -2724,27 +2724,21 @@
goto error;
}
- /* free the old list... if it exists */
- if (data->audio_support) {
- talloc_free(data->audio_support);
- data->audio_support = NULL;
- data->audio_length = 0;
+ if (argc > ARRAY_SIZE(data->audio_support)) {
+ vty_out(vty, "Too many items in 'msc' / 'codec-list': %d. There
can be at most %zu entries.%s",
+ argc, ARRAY_SIZE(data->audio_support), VTY_NEWLINE);
+ return CMD_ERR_EXEED_ARGC_MAX;
}
- /* create a new array */
- data->audio_support =
- talloc_zero_array(bsc_gsmnet, struct gsm_audio_support *, argc);
data->audio_length = argc;
for (i = 0; i < argc; ++i) {
- data->audio_support[i] = talloc_zero(data->audio_support,
- struct gsm_audio_support);
- data->audio_support[i]->ver = atoi(argv[i] + 2);
+ data->audio_support[i].ver = atoi(argv[i] + 2);
if (strncmp("hr", argv[i], 2) == 0)
- data->audio_support[i]->hr = 1;
+ data->audio_support[i].hr = 1;
else if (strncmp("fr", argv[i], 2) == 0)
- data->audio_support[i]->hr = 0;
+ data->audio_support[i].hr = 0;
}
return CMD_SUCCESS;
diff --git a/src/osmo-bsc/codec_pref.c b/src/osmo-bsc/codec_pref.c
index 2c90831..31b5215 100644
--- a/src/osmo-bsc/codec_pref.c
+++ b/src/osmo-bsc/codec_pref.c
@@ -335,7 +335,7 @@
* indeed available with the current BTS and MSC configuration */
for (i = 0; i < msc->audio_length; i++) {
/* Pick a permitted speech value from the global codec
configuration list */
- perm_spch = audio_support_to_gsm88(msc->audio_support[i]);
+ perm_spch = audio_support_to_gsm88(&msc->audio_support[i]);
/* Determine if the result is a half or full rate codec */
rc = full_rate_from_perm_spch(&full_rate, perm_spch);
@@ -406,7 +406,7 @@
for (i = 0; i < msc->audio_length; i++) {
/* Pick a permitted speech value from the global codec
configuration list */
- perm_spch = audio_support_to_gsm88(msc->audio_support[i]);
+ perm_spch = audio_support_to_gsm88(&msc->audio_support[i]);
/* Check this permitted speech value against the BTS specific
parameters.
* if the BTS does not support the codec, try the next one */
@@ -421,8 +421,8 @@
/* AMR (HR/FR version 3) is the only codec that requires a codec
* configuration (S0-S15). Determine the current configuration
and update
* the cfg flag. */
- if (msc->audio_support[i]->ver == 3)
- scl->codec[scl->len].cfg =
gen_bss_supported_amr_s15_s0(msc, bts, msc->audio_support[i]->hr);
+ if (msc->audio_support[i].ver == 3)
+ scl->codec[scl->len].cfg =
gen_bss_supported_amr_s15_s0(msc, bts, msc->audio_support[i].hr);
scl->len++;
}
diff --git a/src/osmo-bsc/osmo_bsc_msc.c b/src/osmo-bsc/osmo_bsc_msc.c
index 4d5d5b2..08a2466 100644
--- a/src/osmo-bsc/osmo_bsc_msc.c
+++ b/src/osmo-bsc/osmo_bsc_msc.c
@@ -207,7 +207,6 @@
struct bsc_msc_data *osmo_msc_data_alloc(struct gsm_network *net, int nr)
{
struct bsc_msc_data *msc_data;
- unsigned int i;
/* check if there is already one */
msc_data = osmo_msc_data_find(net, nr);
@@ -251,24 +250,16 @@
/* Allow the full set of possible codecs by default */
msc_data->audio_length = 5;
- msc_data->audio_support =
- talloc_zero_array(msc_data, struct gsm_audio_support *,
- msc_data->audio_length);
- for (i = 0; i < msc_data->audio_length; i++) {
- msc_data->audio_support[i] =
- talloc_zero(msc_data->audio_support,
- struct gsm_audio_support);
- }
- msc_data->audio_support[0]->ver = 1;
- msc_data->audio_support[0]->hr = 0;
- msc_data->audio_support[1]->ver = 1;
- msc_data->audio_support[1]->hr = 1;
- msc_data->audio_support[2]->ver = 2;
- msc_data->audio_support[2]->hr = 0;
- msc_data->audio_support[3]->ver = 3;
- msc_data->audio_support[3]->hr = 0;
- msc_data->audio_support[4]->ver = 3;
- msc_data->audio_support[4]->hr = 1;
+ msc_data->audio_support[0].ver = 1;
+ msc_data->audio_support[0].hr = 0;
+ msc_data->audio_support[1].ver = 1;
+ msc_data->audio_support[1].hr = 1;
+ msc_data->audio_support[2].ver = 2;
+ msc_data->audio_support[2].hr = 0;
+ msc_data->audio_support[3].ver = 3;
+ msc_data->audio_support[3].hr = 0;
+ msc_data->audio_support[4].ver = 3;
+ msc_data->audio_support[4].hr = 1;
osmo_fd_setup(&msc_data->mgcp_ipa.ofd, -1, OSMO_FD_READ,
&bsc_sccplite_mgcp_proxy_cb, msc_data, 0);
msc_data->mgcp_ipa.local_addr = NULL; /* = INADDR(6)_ANY */
diff --git a/tests/codec_pref/codec_pref_test.c
b/tests/codec_pref/codec_pref_test.c
index 5f4c831..920352c 100644
--- a/tests/codec_pref/codec_pref_test.c
+++ b/tests/codec_pref/codec_pref_test.c
@@ -30,27 +30,8 @@
void *ctx = NULL;
-#define MSC_AUDIO_SUPPORT_MAX 5
#define N_CONFIG_VARIANTS 9
-/* Make sure that there is some memory to put our test configuration. */
-static void init_msc_config(struct bsc_msc_data *msc)
-{
- unsigned int i;
-
- msc->audio_support = talloc_zero_array(ctx, struct gsm_audio_support *,
MSC_AUDIO_SUPPORT_MAX);
- msc->audio_length = MSC_AUDIO_SUPPORT_MAX;
- for (i = 0; i < MSC_AUDIO_SUPPORT_MAX; i++) {
- msc->audio_support[i] = talloc_zero(msc->audio_support, struct
gsm_audio_support);
- }
-}
-
-/* Free memory that we have used for the test configuration. */
-static void free_msc_config(struct bsc_msc_data *msc)
-{
- talloc_free(msc->audio_support);
-}
-
/* The speech codec list is sent by the MS and lists the voice codec settings
* that the MS is able to support. The BSC must select one of this codecs
* depending on what the MSC is able to support. The following function
@@ -215,74 +196,74 @@
switch (config_no) {
case 0:
/* FR1 only */
- msc->audio_support[0]->ver = 1;
- msc->audio_support[0]->hr = 0;
+ msc->audio_support[0].ver = 1;
+ msc->audio_support[0].hr = 0;
msc->audio_length = 1;
break;
case 1:
/* HR1 only */
- msc->audio_support[0]->ver = 1;
- msc->audio_support[0]->hr = 1;
+ msc->audio_support[0].ver = 1;
+ msc->audio_support[0].hr = 1;
msc->audio_length = 1;
break;
case 2:
/* FR2 only */
- msc->audio_support[0]->ver = 2;
- msc->audio_support[0]->hr = 0;
+ msc->audio_support[0].ver = 2;
+ msc->audio_support[0].hr = 0;
msc->audio_length = 1;
break;
case 3:
/* FR3 only */
- msc->audio_support[0]->ver = 3;
- msc->audio_support[0]->hr = 0;
+ msc->audio_support[0].ver = 3;
+ msc->audio_support[0].hr = 0;
msc->audio_length = 1;
break;
case 4:
/* HR3 only */
- msc->audio_support[0]->ver = 3;
- msc->audio_support[0]->hr = 1;
+ msc->audio_support[0].ver = 3;
+ msc->audio_support[0].hr = 1;
msc->audio_length = 1;
break;
case 5:
/* FR1 and HR1 */
- msc->audio_support[0]->ver = 1;
- msc->audio_support[0]->hr = 0;
- msc->audio_support[1]->ver = 1;
- msc->audio_support[1]->hr = 1;
+ msc->audio_support[0].ver = 1;
+ msc->audio_support[0].hr = 0;
+ msc->audio_support[1].ver = 1;
+ msc->audio_support[1].hr = 1;
msc->audio_length = 2;
break;
case 6:
/* FR1, FR2 and HR1 */
- msc->audio_support[0]->ver = 1;
- msc->audio_support[0]->hr = 0;
- msc->audio_support[1]->ver = 2;
- msc->audio_support[1]->hr = 0;
- msc->audio_support[2]->ver = 1;
- msc->audio_support[2]->hr = 1;
+ msc->audio_support[0].ver = 1;
+ msc->audio_support[0].hr = 0;
+ msc->audio_support[1].ver = 2;
+ msc->audio_support[1].hr = 0;
+ msc->audio_support[2].ver = 1;
+ msc->audio_support[2].hr = 1;
msc->audio_length = 3;
break;
case 7:
/* FR1, FR3 and HR3 */
- msc->audio_support[0]->ver = 1;
- msc->audio_support[0]->hr = 0;
- msc->audio_support[1]->ver = 3;
- msc->audio_support[1]->hr = 0;
- msc->audio_support[2]->ver = 3;
- msc->audio_support[2]->hr = 1;
+ msc->audio_support[0].ver = 1;
+ msc->audio_support[0].hr = 0;
+ msc->audio_support[1].ver = 3;
+ msc->audio_support[1].hr = 0;
+ msc->audio_support[2].ver = 3;
+ msc->audio_support[2].hr = 1;
msc->audio_length = 3;
break;
case 8:
/* FR1, FR2, FR3, HR1 and HR3 */
- msc->audio_support[0]->ver = 1;
- msc->audio_support[0]->hr = 0;
- msc->audio_support[1]->ver = 2;
- msc->audio_support[1]->hr = 0;
- msc->audio_support[2]->ver = 3;
- msc->audio_support[2]->hr = 0;
- msc->audio_support[3]->ver = 1;
- msc->audio_support[3]->hr = 1;
- msc->audio_support[4]->ver = 3;
- msc->audio_support[4]->hr = 1;
+ msc->audio_support[0].ver = 1;
+ msc->audio_support[0].hr = 0;
+ msc->audio_support[1].ver = 2;
+ msc->audio_support[1].hr = 0;
+ msc->audio_support[2].ver = 3;
+ msc->audio_support[2].hr = 0;
+ msc->audio_support[3].ver = 1;
+ msc->audio_support[3].hr = 1;
+ msc->audio_support[4].ver = 3;
+ msc->audio_support[4].hr = 1;
msc->audio_length = 5;
break;
}
@@ -394,10 +375,10 @@
printf(" * BSS: audio support settings (%u items):\n",
msc->audio_length);
for (i = 0; i < msc->audio_length; i++)
- if (msc->audio_support[i]->hr)
- printf(" audio_support[%u]=HR%u\n", i,
msc->audio_support[i]->ver);
+ if (msc->audio_support[i].hr)
+ printf(" audio_support[%u]=HR%u\n", i,
msc->audio_support[i].ver);
else
- printf(" audio_support[%u]=FR%u\n", i,
msc->audio_support[i]->ver);
+ printf(" audio_support[%u]=FR%u\n", i,
msc->audio_support[i].ver);
printf(" * BTS: audio support settings:\n");
printf(" (GSM-FR implicitly supported)\n");
@@ -426,8 +407,6 @@
printf("============== test_one_to_one ==============\n\n");
- init_msc_config(&msc_local);
-
for (i = 0; i < N_CONFIG_VARIANTS; i++) {
make_msc_config(&msc_local, i);
make_scl_config(&scl_ms, i);
@@ -436,8 +415,6 @@
rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local,
&bts_local);
OSMO_ASSERT(rc == 0);
}
-
- free_msc_config(&msc_local);
}
/* Network supports all combinations, MS varies */
@@ -452,8 +429,6 @@
printf("============== test_ms ==============\n\n");
- init_msc_config(&msc_local);
-
make_msc_config(&msc_local, 8);
make_ct_config(&ct_msc, 8);
make_bts_config(&bts_local, 8);
@@ -462,8 +437,6 @@
rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local,
&bts_local);
OSMO_ASSERT(rc == 0);
}
-
- free_msc_config(&msc_local);
}
/* BSS and MS support all combinations, MSC varies */
@@ -478,8 +451,6 @@
printf("============== test_ct ==============\n\n");
- init_msc_config(&msc_local);
-
make_msc_config(&msc_local, 8);
make_scl_config(&scl_ms, 8);
make_bts_config(&bts_local, 8);
@@ -488,8 +459,6 @@
rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local,
&bts_local);
OSMO_ASSERT(rc == 0);
}
-
- free_msc_config(&msc_local);
}
/* MSC and MS support all combinations, BSS varies */
@@ -504,8 +473,6 @@
printf("============== test_msc ==============\n\n");
- init_msc_config(&msc_local);
-
make_ct_config(&ct_msc, 8);
make_scl_config(&scl_ms, 8);
make_bts_config(&bts_local, 8);
@@ -514,8 +481,6 @@
rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local,
&bts_local);
OSMO_ASSERT(rc == 0);
}
-
- free_msc_config(&msc_local);
}
/* Some mixed configurations that are supposed to work */
@@ -529,8 +494,6 @@
printf("============== test_selected_working ==============\n\n");
- init_msc_config(&msc_local);
-
make_scl_config(&scl_ms, 6);
make_ct_config(&ct_msc, 5);
make_msc_config(&msc_local, 7);
@@ -572,8 +535,6 @@
make_bts_config(&bts_local, 1);
rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local, &bts_local);
OSMO_ASSERT(rc == 0);
-
- free_msc_config(&msc_local);
}
/* Some mixed configurations that can not work */
@@ -587,8 +548,6 @@
printf("============== test_selected_non_working ==============\n\n");
- init_msc_config(&msc_local);
-
make_scl_config(&scl_ms, 1);
make_ct_config(&ct_msc, 5);
make_msc_config(&msc_local, 7);
@@ -630,8 +589,6 @@
make_bts_config(&bts_local, 7);
rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local, &bts_local);
OSMO_ASSERT(rc == -1);
-
- free_msc_config(&msc_local);
}
/* Try execute bss_supp_codec_list(), display input and output parameters */
@@ -644,10 +601,10 @@
printf(" * BSS: audio support settings (%u items):\n",
msc->audio_length);
for (i = 0; i < msc->audio_length; i++)
- if (msc->audio_support[i]->hr)
- printf(" audio_support[%u]=HR%u\n", i,
msc->audio_support[i]->ver);
+ if (msc->audio_support[i].hr)
+ printf(" audio_support[%u]=HR%u\n", i,
msc->audio_support[i].ver);
else
- printf(" audio_support[%u]=FR%u\n", i,
msc->audio_support[i]->ver);
+ printf(" audio_support[%u]=FR%u\n", i,
msc->audio_support[i].ver);
printf(" * BTS: audio support settings:\n");
printf(" (GSM-FR implicitly supported)\n");
@@ -660,7 +617,7 @@
printf(" * result: speech codec list (%u items):\n", scl.len);
for (i = 0; i < scl.len; i++) {
printf(" codec[%u]->type=%s", i,
gsm0808_speech_codec_type_name(scl.codec[i].type));
- if (msc->audio_support[i]->ver == 3)
+ if (msc->audio_support[i].ver == 3)
printf(" S15-S0=%04x", scl.codec[i].cfg);
printf("\n");
}
@@ -676,8 +633,6 @@
uint8_t k;
printf("============== test_gen_bss_supp_codec_list_cfgs
==============\n\n");
- init_msc_config(&msc_local);
-
for (i = 0; i < N_CONFIG_VARIANTS; i++) {
for (k = 0; k < N_CONFIG_VARIANTS; k++) {
make_msc_config(&msc_local, i);
@@ -686,8 +641,6 @@
test_gen_bss_supported_codec_list(&msc_local,
&bts_local);
}
}
-
- free_msc_config(&msc_local);
}
static const struct log_info_cat log_categories[] = {
diff --git a/tests/msc.vty b/tests/msc.vty
index 56a3fe2..af4c33e 100644
--- a/tests/msc.vty
+++ b/tests/msc.vty
@@ -114,3 +114,6 @@
codec-list fr1 fr2 fr3 fr4
...
OsmoBSC(config-msc)# # TODO: should fr4 thru fr7 be rejected
+
+OsmoBSC(config-msc)# codec-list fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1
fr1 fr1 fr1 fr1 fr1 fr1
+Too many items in 'msc' / 'codec-list': 17. There can be at most 16 entries.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31609
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I625cedc4bb040d649fd6e1794ba468f4c6ad6adc
Gerrit-Change-Number: 31609
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-CC: pespin <pes...@sysmocom.de>
Gerrit-MessageType: merged
