osmith submitted this change.
Check VTY config against features reported by BTS
* Don't copy features for osmo-bts and nanobts initially, wait until
BTS reported its features
* Checks for BTS features in VTY cmds: pass if features are not known
(not yet reported by the BTS), fail if the feature is missing
* Once BTS reports its features, check relevant VTY config parts again
Related: SYS#5922, OS#5538
Change-Id: I7fca42a39a4bc98a6ea8b9cfab28c4bad3a6a0aa
---
A doc/bts-features.txt
M include/osmocom/bsc/bts.h
M src/osmo-bsc/abis_nm.c
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/bts.c
M src/osmo-bsc/bts_ipaccess_nanobts.c
M src/osmo-bsc/bts_osmobts.c
M src/osmo-bsc/bts_trx.c
M src/osmo-bsc/bts_trx_vty.c
M src/osmo-bsc/bts_vty.c
M tests/Makefile.am
A tests/bts_features.vty
12 files changed, 126 insertions(+), 22 deletions(-)
diff --git a/doc/bts-features.txt b/doc/bts-features.txt
new file mode 100644
index 0000000..a38b9a7
--- /dev/null
+++ b/doc/bts-features.txt
@@ -0,0 +1,42 @@
+Notes about BTS feature check code
+---
+
+Feature reporting:
+- For most BTS we hardcode a list of assumed features in the BTS model's
+ _init() function, e.g. bts_model_bs11_init(). These features get copied to
+ bts->features once the BTS model is set.
+- nanobts and osmo-bts are special, they support reporting features during OML
+ bring up (features_get_reported set in struct_gsm_bts_model):
+ - For osmo-bts, we do not assume any features in the BTS model and just let
+ it report all available features.
+ - For nanobts, we wait for the reported features and then extend them with
+ the features set in the bts model. This is needed because the features enum
+ gets extended by us for osmo-bts, it may have features that nanobts does
+ not report but has implemented.
+- Once features are available (either through feature reporting or copied from
+ the bts model), features_known is true in struct gsm_bts.
+
+Implementing a feature check:
+- Check that features_known is true, in case the check may be done before the
+ BTS is connected and has reported its features (e.g. in VTY config parsing)
+- Use osmo_bts_has_feature()
+- Example:
+ if (bts->features_known && !osmo_bts_has_feature(&bts->features, BTS_FEAT_MULTI_TSC))
+
+VTY and feature checks:
+- Some VTY commands only make sense if a BTS supports a certain feature
+- Implement the following checks:
+ - In the VTY command, check if the BTS has the feature.
+ - In gsm_bts_check_cfg() (or called funcs like trx_has_valid_pchan_config),
+ check if the VTY command for the feature is set and if the BTS has the
+ feature.
+- In both cases, do not fail the checks if bts->features_known is false.
+
+Resulting functionality:
+- For BTS that do not support feature reporting, the VTY config is checked
+ against the hardcoded feature set as it gets parsed.
+- For BTS that do support feature reporting, the VTY config is checked when
+ features get reported. The BTS gets rejected if the config is invalid for the
+ available features.
+- Once a BTS is up and running, VTY commands changing the behavior check
+ against the available feature sets.
diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h
index 7d51707..008eee5 100644
--- a/include/osmocom/bsc/bts.h
+++ b/include/osmocom/bsc/bts.h
@@ -295,9 +295,12 @@
struct tlv_definition nm_att_tlvdef;
- /* features of a given BTS model set via gsm_bts_model_register() locally */
+ /* features of a given BTS model set via gsm_bts_model_register()
+ * locally, see doc/bts-features.txt */
struct bitvec features;
uint8_t _features_data[MAX_BTS_FEATURES/8];
+ /* BTS reports features during OML bring up */
+ bool features_get_reported;
};
struct gsm_gprs_cell {
@@ -334,9 +337,13 @@
char version[MAX_VERSION_LENGTH];
char sub_model[MAX_VERSION_LENGTH];
- /* features of a given BTS set/reported via OML */
+ /* features of a given BTS either hardcoded or set/reported via OML,
+ * see doc/bts-features.txt */
struct bitvec features;
uint8_t _features_data[MAX_BTS_FEATURES/8];
+ /* Features have been reported by the BTS or were copied from the BTS
+ * model */
+ bool features_known;
/* Connected PCU version (if any) */
char pcu_version[MAX_VERSION_LENGTH];
diff --git a/src/osmo-bsc/abis_nm.c b/src/osmo-bsc/abis_nm.c
index b700994..bb44c70 100644
--- a/src/osmo-bsc/abis_nm.c
+++ b/src/osmo-bsc/abis_nm.c
@@ -600,6 +600,7 @@
}
memcpy(bts->_features_data, TLVP_VAL(tp, NM_ATT_MANUF_ID), len);
+ bts->features_known = true;
/* Log each BTS feature in the reported vector */
for (i = 0; i < len * 8; i++) {
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index d452a52..d293862 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -1700,6 +1700,7 @@
static int lchan_act_deact(struct vty *vty, const char **argv, int argc)
{
struct gsm_bts_trx_ts *ts;
+ struct gsm_bts *bts;
struct gsm_lchan *lchan;
bool vamos = (strcmp(argv[3], "vamos-sub-slot") == 0);
int ss_nr = atoi(argv[4]);
@@ -1725,7 +1726,8 @@
return CMD_WARNING;
}
- if (vamos && !osmo_bts_has_feature(&ts->trx->bts->features, BTS_FEAT_VAMOS)) {
+ bts = ts->trx->bts;
+ if (vamos && bts->features_known && !osmo_bts_has_feature(&bts->features, BTS_FEAT_VAMOS)) {
vty_out(vty, "BTS does not support VAMOS%s", VTY_NEWLINE);
return CMD_WARNING;
}
@@ -1955,6 +1957,7 @@
TSC_ARGS_DOC)
{
struct gsm_bts_trx_ts *ts;
+ struct gsm_bts *bts;
struct gsm_lchan *lchan;
int ss_nr = atoi(argv[3]);
const char *vamos_str = argv[4];
@@ -1971,7 +1974,8 @@
return CMD_WARNING;
}
- if (!osmo_bts_has_feature(&ts->trx->bts->features, BTS_FEAT_VAMOS)) {
+ bts = ts->trx->bts;
+ if (bts->features_known && !osmo_bts_has_feature(&bts->features, BTS_FEAT_VAMOS)) {
vty_out(vty, "%% BTS does not support VAMOS%s", VTY_NEWLINE);
return CMD_WARNING;
}
diff --git a/src/osmo-bsc/bts.c b/src/osmo-bsc/bts.c
index 4066cf1..02cd3a8 100644
--- a/src/osmo-bsc/bts.c
+++ b/src/osmo-bsc/bts.c
@@ -500,6 +500,14 @@
return -EINVAL;
}
+ if (bts->features_known) {
+ if (!bts_gprs_mode_is_compat(bts, bts->gprs.mode)) {
+ LOGP(DNM, LOGL_ERROR, "(bts=%u) GPRS mode set to '%s', but BTS does not support it\n", bts->nr,
+ bts_gprs_mode_name(bts->gprs.mode));
+ return -EINVAL;
+ }
+ }
+
/* Verify the physical channel mapping */
llist_for_each_entry(trx, &bts->trx_list, list) {
if (!trx_has_valid_pchan_config(trx)) {
@@ -621,11 +629,15 @@
{
bts->model = model;
- /* Copy hardcoded feature list from BTS model. For some BTS we support
- * reporting features at runtime (as of writing nanobts, OsmoBTS),
- * which will then replace this list. */
- if (model)
+ /* Copy hardcoded feature list from BTS model, unless the BTS supports
+ * reporting features at runtime (as of writing nanobts, OsmoBTS). */
+ if (!model || model->features_get_reported) {
+ memset(bts->_features_data, 0, sizeof(bts->_features_data));
+ bts->features_known = false;
+ } else {
memcpy(bts->_features_data, bts->model->_features_data, sizeof(bts->_features_data));
+ bts->features_known = true;
+ }
return 0;
}
diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c
index 1df6537..54848fb 100644
--- a/src/osmo-bsc/bts_ipaccess_nanobts.c
+++ b/src/osmo-bsc/bts_ipaccess_nanobts.c
@@ -131,6 +131,7 @@
[NM_ATT_IPACC_REVOC_DATE] = { TLV_TYPE_TL16V },
},
},
+ .features_get_reported = true,
};
diff --git a/src/osmo-bsc/bts_osmobts.c b/src/osmo-bsc/bts_osmobts.c
index ca5ddb2..078cfe7 100644
--- a/src/osmo-bsc/bts_osmobts.c
+++ b/src/osmo-bsc/bts_osmobts.c
@@ -205,13 +205,8 @@
sizeof(model_osmobts._features_data);
memset(model_osmobts.features.data, 0, model_osmobts.features.data_len);
- /* Order alphabetically and remember to adjust bts_init/bts_model_init
- * in OsmoBTS to report new features. */
- osmo_bts_set_feature(&model_osmobts.features, BTS_FEAT_CCN);
- osmo_bts_set_feature(&model_osmobts.features, BTS_FEAT_EGPRS);
- osmo_bts_set_feature(&model_osmobts.features, BTS_FEAT_GPRS);
- osmo_bts_set_feature(&model_osmobts.features, BTS_FEAT_IPV6_NSVC);
- osmo_bts_set_feature(&model_osmobts.features, BTS_FEAT_PAGING_COORDINATION);
+ /* Adjust bts_init/bts_model_init in OsmoBTS to report new features.
+ * See also: doc/bts-features.txt */
model_osmobts.nm_att_tlvdef.def[NM_ATT_OSMO_NS_LINK_CFG].type = TLV_TYPE_TL16V;
diff --git a/src/osmo-bsc/bts_trx.c b/src/osmo-bsc/bts_trx.c
index fa27b05..71d9fbf 100644
--- a/src/osmo-bsc/bts_trx.c
+++ b/src/osmo-bsc/bts_trx.c
@@ -338,6 +338,20 @@
result = false;
}
}
+
+ if (trx->bts->features_known) {
+ const struct bitvec *ft = &trx->bts->features;
+
+ if (ts->hopping.enabled && !osmo_bts_has_feature(ft, BTS_FEAT_HOPPING)) {
+ LOGP(DNM, LOGL_ERROR, "TS%d has freq. hopping enabled, but BTS does not support it\n", i);
+ result = false;
+ }
+
+ if (ts->tsc != -1 && !osmo_bts_has_feature(ft, BTS_FEAT_MULTI_TSC)) {
+ LOGP(DNM, LOGL_ERROR, "TS%d has TSC != BCC, but BTS does not support it\n", i);
+ result = false;
+ }
+ }
}
return result;
diff --git a/src/osmo-bsc/bts_trx_vty.c b/src/osmo-bsc/bts_trx_vty.c
index 27bba2c..a3d6033 100644
--- a/src/osmo-bsc/bts_trx_vty.c
+++ b/src/osmo-bsc/bts_trx_vty.c
@@ -316,8 +316,9 @@
"Training Sequence Code of the Timeslot\n" "TSC\n")
{
struct gsm_bts_trx_ts *ts = vty->index;
+ const struct gsm_bts *bts = ts->trx->bts;
- if (!osmo_bts_has_feature(&ts->trx->bts->features, BTS_FEAT_MULTI_TSC)) {
+ if (bts->features_known && !osmo_bts_has_feature(&bts->features, BTS_FEAT_MULTI_TSC)) {
vty_out(vty, "%% This BTS does not support a TSC != BCC, "
"falling back to BCC%s", VTY_NEWLINE);
ts->tsc = -1;
@@ -339,13 +340,12 @@
"Disable frequency hopping\n" "Enable frequency hopping\n")
{
struct gsm_bts_trx_ts *ts = vty->index;
+ const struct gsm_bts *bts = ts->trx->bts;
int enabled = atoi(argv[0]);
- if (enabled && !osmo_bts_has_feature(&ts->trx->bts->features, BTS_FEAT_HOPPING)) {
- vty_out(vty, "%% BTS model does not seem to support freq. hopping%s", VTY_NEWLINE);
- /* Allow enabling frequency hopping anyway, because the BTS might not have
- * connected yet (thus not sent the feature vector), so we cannot know for
- * sure. Jet print a warning and let it go. */
+ if (enabled && bts->features_known && !osmo_bts_has_feature(&bts->features, BTS_FEAT_HOPPING)) {
+ vty_out(vty, "%% BTS does not support freq. hopping%s", VTY_NEWLINE);
+ return CMD_WARNING;
}
ts->hopping.enabled = enabled;
diff --git a/src/osmo-bsc/bts_vty.c b/src/osmo-bsc/bts_vty.c
index 1a708ab..4e60cd8 100644
--- a/src/osmo-bsc/bts_vty.c
+++ b/src/osmo-bsc/bts_vty.c
@@ -1592,7 +1592,7 @@
struct gsm_bts *bts = vty->index;
enum bts_gprs_mode mode = bts_gprs_mode_parse(argv[0], NULL);
- if (!bts_gprs_mode_is_compat(bts, mode)) {
+ if (bts->features_known && !bts_gprs_mode_is_compat(bts, mode)) {
vty_out(vty, "%% This BTS type does not support %s%s", argv[0],
VTY_NEWLINE);
return CMD_WARNING;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a5f92ef..4421354 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -43,6 +43,7 @@
power_ctrl.vty \
interf_meas.vty \
acch_overpower.vty \
+ bts_features.vty \
ctrl/osmo-bsc-neigh-test.cfg \
ctrl/osmo-bsc-apply-config-file.cfg \
ctrl/osmo-bsc-apply-config-file-invalid.cfg \
diff --git a/tests/bts_features.vty b/tests/bts_features.vty
new file mode 100644
index 0000000..3768a4d
--- /dev/null
+++ b/tests/bts_features.vty
@@ -0,0 +1,27 @@
+OsmoBSC> ### see doc/bts-features.txt
+
+OsmoBSC> enable
+OsmoBSC# configure terminal
+OsmoBSC(config)# network
+
+OsmoBSC(config-net)# ### osmo-bts: all feature checks pass before it is connected (features_get_reported is true)
+OsmoBSC(config-net)# bts 0
+OsmoBSC(config-net-bts)# gprs mode egprs
+OsmoBSC(config-net-bts)# trx 0
+OsmoBSC(config-net-bts-trx)# timeslot 2
+OsmoBSC(config-net-bts-trx-ts)# hopping enabled 1
+OsmoBSC(config-net-bts-trx-ts)# exit
+OsmoBSC(config-net-bts-trx)# exit
+OsmoBSC(config-net-bts)# exit
+
+OsmoBSC(config-net)# ### bs11: checks against hardcoded features (features_get_reported is false)
+OsmoBSC(config-net)# bts 1
+OsmoBSC(config-net-bts)# type bs11
+OsmoBSC(config-net-bts)# gprs mode egprs
+% This BTS type does not support egprs
+OsmoBSC(config-net-bts)# trx 0
+OsmoBSC(config-net-bts-trx)# timeslot 2
+OsmoBSC(config-net-bts-trx-ts)# hopping enabled 1
+OsmoBSC(config-net-bts-trx-ts)# exit
+OsmoBSC(config-net-bts-trx)# exit
+OsmoBSC(config-net-bts)# exit
To view, visit change 27923. To unsubscribe, or for help writing mail filters, visit settings.