Revert "Check VTY config against features reported by BTS"

This reverts commit 159246f94f233ecb3c0b610588747b414cfdb9ce.

12 files changed, 22 insertions, 126 deletions

diff --git a/doc/bts-features.txt b/doc/bts-features.txt
deleted file mode 100644
index a38b9a769..000000000
--- a/doc/bts-features.txt
+++ /dev/null
@@ -1,42 +0,0 @@
-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 c5a09f2ea..9c954415a 100644
--- a/include/osmocom/bsc/bts.h
+++ b/include/osmocom/bsc/bts.h
@@ -298,12 +298,9 @@ struct gsm_bts_model {
 
 	struct tlv_definition nm_att_tlvdef;
 
-	/* features of a given BTS model set via gsm_bts_model_register()
-	 * locally, see doc/bts-features.txt */
+	/* features of a given BTS model set via gsm_bts_model_register() locally */
 	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 {
@@ -340,13 +337,9 @@ struct gsm_bts {
 	char version[MAX_VERSION_LENGTH];
 	char sub_model[MAX_VERSION_LENGTH];
 
-	/* features of a given BTS either hardcoded or set/reported via OML,
-	 * see doc/bts-features.txt */
+	/* features of a given BTS set/reported via OML */
 	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 e1e8efa09..ee5bdf973 100644
--- a/src/osmo-bsc/abis_nm.c
+++ b/src/osmo-bsc/abis_nm.c
@@ -591,7 +591,6 @@ 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 d29386219..d452a52d3 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -1700,7 +1700,6 @@ 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]);
@@ -1726,8 +1725,7 @@ static int lchan_act_deact(struct vty *vty, const char **argv, int argc)
 		return CMD_WARNING;
 	}
 
-	bts = ts->trx->bts;
-	if (vamos && bts->features_known && !osmo_bts_has_feature(&bts->features, BTS_FEAT_VAMOS)) {
+	if (vamos && !osmo_bts_has_feature(&ts->trx->bts->features, BTS_FEAT_VAMOS)) {
 		vty_out(vty, "BTS does not support VAMOS%s", VTY_NEWLINE);
 		return CMD_WARNING;
 	}
@@ -1957,7 +1955,6 @@ 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];
@@ -1974,8 +1971,7 @@ DEFUN(vamos_modify_lchan, vamos_modify_lchan_cmd,
 		return CMD_WARNING;
 	}
 
-	bts = ts->trx->bts;
-	if (bts->features_known && !osmo_bts_has_feature(&bts->features, BTS_FEAT_VAMOS)) {
+	if (!osmo_bts_has_feature(&ts->trx->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 8be41902c..c3dd70af4 100644
--- a/src/osmo-bsc/bts.c
+++ b/src/osmo-bsc/bts.c
@@ -503,14 +503,6 @@ 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)) {
@@ -632,15 +624,11 @@ int gsm_set_bts_model(struct gsm_bts *bts, struct gsm_bts_model *model)
 {
 	bts->model = 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 {
+	/* 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)
 		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 d2a2545b0..1edc94a51 100644
--- a/src/osmo-bsc/bts_ipaccess_nanobts.c
+++ b/src/osmo-bsc/bts_ipaccess_nanobts.c
@@ -131,7 +131,6 @@ 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 078cfe7a6..ca5ddb2e7 100644
--- a/src/osmo-bsc/bts_osmobts.c
+++ b/src/osmo-bsc/bts_osmobts.c
@@ -205,8 +205,13 @@ int bts_model_osmobts_init(void)
 				sizeof(model_osmobts._features_data);
 	memset(model_osmobts.features.data, 0, model_osmobts.features.data_len);
 
-	/* Adjust bts_init/bts_model_init in OsmoBTS to report new features.
-	 * See also: doc/bts-features.txt */
+	/* 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);
 
 	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 71d9fbf35..fa27b05da 100644
--- a/src/osmo-bsc/bts_trx.c
+++ b/src/osmo-bsc/bts_trx.c
@@ -338,20 +338,6 @@ 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 d232d46a9..efe6b63ab 100644
--- a/src/osmo-bsc/bts_trx_vty.c
+++ b/src/osmo-bsc/bts_trx_vty.c
@@ -316,9 +316,8 @@ 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 (bts->features_known && !osmo_bts_has_feature(&bts->features, BTS_FEAT_MULTI_TSC)) {
+	if (!osmo_bts_has_feature(&ts->trx->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;
@@ -340,12 +339,13 @@ 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 && 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;
+	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. */
 	}
 
 	ts->hopping.enabled = enabled;
diff --git a/src/osmo-bsc/bts_vty.c b/src/osmo-bsc/bts_vty.c
index 8af96ed9b..af1d09c9f 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->features_known && !bts_gprs_mode_is_compat(bts, mode)) {
+	if (!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 fcfc4eaf1..be693ccc8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -44,7 +44,6 @@ 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
deleted file mode 100644
index 3768a4d52..000000000
--- a/tests/bts_features.vty
+++ /dev/null
@@ -1,27 +0,0 @@
-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