diff --git a/doc/bts-features.txt b/doc/bts-features.txt new file mode 100644 index 000000000..a38b9a769 --- /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 7d51707be..008eee524 100644 --- a/include/osmocom/bsc/bts.h +++ b/include/osmocom/bsc/bts.h @@ -295,9 +295,12 @@ struct gsm_bts_model { 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 @@ struct gsm_bts { 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 b7009940f..bb44c70dc 100644 --- a/src/osmo-bsc/abis_nm.c +++ b/src/osmo-bsc/abis_nm.c @@ -600,6 +600,7 @@ static int parse_attr_resp_info_attr(struct gsm_bts *bts, const struct gsm_bts_t } 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 d452a52d3..d29386219 100644 --- a/src/osmo-bsc/bsc_vty.c +++ b/src/osmo-bsc/bsc_vty.c @@ -1700,6 +1700,7 @@ static int lchan_act_trx(struct vty *vty, struct gsm_bts_trx *trx, int activate) 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 @@ static int lchan_act_deact(struct vty *vty, const char **argv, int argc) 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 @@ DEFUN(vamos_modify_lchan, vamos_modify_lchan_cmd, 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 @@ DEFUN(vamos_modify_lchan, vamos_modify_lchan_cmd, 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 4066cf10f..02cd3a8c8 100644 --- a/src/osmo-bsc/bts.c +++ b/src/osmo-bsc/bts.c @@ -500,6 +500,14 @@ int gsm_bts_check_cfg(struct gsm_bts *bts) 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 @@ int gsm_set_bts_model(struct gsm_bts *bts, struct gsm_bts_model *model) { 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 1df6537f6..54848fbcd 100644 --- a/src/osmo-bsc/bts_ipaccess_nanobts.c +++ b/src/osmo-bsc/bts_ipaccess_nanobts.c @@ -131,6 +131,7 @@ struct gsm_bts_model bts_model_nanobts = { [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 ca5ddb2e7..078cfe7a6 100644 --- a/src/osmo-bsc/bts_osmobts.c +++ b/src/osmo-bsc/bts_osmobts.c @@ -205,13 +205,8 @@ int bts_model_osmobts_init(void) 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 fa27b05da..71d9fbf35 100644 --- a/src/osmo-bsc/bts_trx.c +++ b/src/osmo-bsc/bts_trx.c @@ -338,6 +338,20 @@ bool trx_has_valid_pchan_config(const struct gsm_bts_trx *trx) 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 27bba2cae..a3d603321 100644 --- a/src/osmo-bsc/bts_trx_vty.c +++ b/src/osmo-bsc/bts_trx_vty.c @@ -316,8 +316,9 @@ DEFUN_USRATTR(cfg_ts_tsc, "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 @@ DEFUN_USRATTR(cfg_ts_hopping, "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 1a708abda..4e60cd897 100644 --- a/src/osmo-bsc/bts_vty.c +++ b/src/osmo-bsc/bts_vty.c @@ -1592,7 +1592,7 @@ DEFUN_USRATTR(cfg_bts_gprs_mode, 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 a5f92efcc..442135423 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -43,6 +43,7 @@ EXTRA_DIST = \ 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 000000000..3768a4d52 --- /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