Change in osmo-pcu[master]: Fix configuration mess of initial_cs/mcs between PCUIF and VTY

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/.

pespin gerrit-no-reply at lists.osmocom.org
Tue Jan 19 10:05:34 UTC 2021


pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/22195 )

Change subject: Fix configuration mess of initial_cs/mcs between PCUIF and VTY
......................................................................

Fix configuration mess of initial_cs/mcs between PCUIF and VTY

Both values (optionally) set (forced) by VTY and the values received
from PCUIF were stored in the same variable, meaning that for instance
the PCUIF values wouldn't really be used if someone applied eg "no cs"
during runtime.

This commit does something similar to what was already done for the
max_(m)cs fields. We store PCUIF values in one place and VTY ones in
another place, and then trigger a bts object internal process to find
out exactly which initial CS should it be using.

Change-Id: I80a6ba401f9c0c85bdf6e0cc99a9d2008d31e1b0
---
M src/bts.cpp
M src/bts.h
M src/gprs_pcu.c
M src/gprs_pcu.h
M src/pcu_l1_if.cpp
M src/pcu_vty.c
6 files changed, 110 insertions(+), 55 deletions(-)

Approvals:
  Jenkins Builder: Verified
  osmith: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved



diff --git a/src/bts.cpp b/src/bts.cpp
index 92abdbf..a50e8ae 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -1172,6 +1172,62 @@
 			trx->pdch[i].unreserve(dir);
 }
 
+void bts_recalc_initial_cs(struct gprs_rlcmac_bts *bts)
+{
+	uint8_t max_cs_dl, max_cs_ul;
+
+	if (the_pcu->vty.force_initial_cs) {
+		bts->initial_cs_dl = the_pcu->vty.initial_cs_dl;
+		bts->initial_cs_ul = the_pcu->vty.initial_cs_ul;
+		return;
+	}
+
+	max_cs_dl = bts->bts->max_cs_dl();
+	if (bts->pcuif_info_ind.initial_cs > max_cs_dl) {
+		LOGP(DL1IF, LOGL_DEBUG, " downgrading initial_cs_dl to %d\n", max_cs_dl);
+		bts->initial_cs_dl = max_cs_dl;
+	} else {
+		bts->initial_cs_dl = bts->pcuif_info_ind.initial_cs;
+	}
+	if (bts->initial_cs_dl == 0)
+		bts->initial_cs_dl = 1; /* CS1 Must always be supported */
+
+	max_cs_ul = bts->bts->max_cs_ul();
+	if (bts->pcuif_info_ind.initial_cs > max_cs_ul) {
+		LOGP(DL1IF, LOGL_DEBUG, " downgrading initial_cs_ul to %d\n", max_cs_ul);
+		bts->initial_cs_ul = max_cs_ul;
+	} else {
+		bts->initial_cs_ul = bts->pcuif_info_ind.initial_cs;
+	}
+	if (bts->initial_cs_ul == 0)
+		bts->initial_cs_ul = 1; /* CS1 Must always be supported */
+}
+void bts_recalc_initial_mcs(struct gprs_rlcmac_bts *bts)
+{
+	uint8_t max_mcs_dl, max_mcs_ul;
+
+	if (the_pcu->vty.force_initial_mcs) {
+		bts->initial_mcs_dl = the_pcu->vty.initial_mcs_dl;
+		bts->initial_mcs_ul = the_pcu->vty.initial_mcs_ul;
+		return;
+	}
+
+	max_mcs_dl = bts->bts->max_mcs_dl();
+	if (bts->pcuif_info_ind.initial_mcs > max_mcs_dl) {
+		LOGP(DL1IF, LOGL_DEBUG, " downgrading initial_mcs_dl to %d\n", max_mcs_dl);
+		bts->initial_mcs_dl = max_mcs_dl;
+	} else {
+		bts->initial_mcs_dl = bts->pcuif_info_ind.initial_mcs;
+	}
+	max_mcs_ul = bts->bts->max_mcs_ul();
+	if (bts->pcuif_info_ind.initial_mcs > max_mcs_ul) {
+		LOGP(DL1IF, LOGL_DEBUG, " downgrading initial_mcs_ul to %d\n", max_mcs_ul);
+		bts->initial_mcs_ul = max_mcs_ul;
+	} else {
+		bts->initial_mcs_ul = bts->pcuif_info_ind.initial_mcs;
+	}
+}
+
 void bts_recalc_max_cs(struct gprs_rlcmac_bts *bts)
 {
 	int i;
diff --git a/src/bts.h b/src/bts.h
index bf6e681..b1f73b4 100644
--- a/src/bts.h
+++ b/src/bts.h
@@ -87,6 +87,10 @@
 	uint8_t bsic;
 	uint8_t cs_mask; /* Allowed CS mask from BTS */
 	uint16_t mcs_mask;  /* Allowed MCS mask from BTS */
+	struct { /* information stored from last received PCUIF info_ind message */
+		uint8_t initial_cs;
+		uint8_t initial_mcs;
+	} pcuif_info_ind;
 	uint8_t initial_cs_dl, initial_cs_ul;
 	uint8_t initial_mcs_dl, initial_mcs_ul;
 	/* Timer defintions */
@@ -399,6 +403,8 @@
 	struct gprs_rlcmac_bts *bts_main_data();
 	struct rate_ctr_group *bts_main_data_stats();
 	struct osmo_stat_item_group *bts_main_data_stat_items();
+	void bts_recalc_initial_cs(struct gprs_rlcmac_bts *bts);
+	void bts_recalc_initial_mcs(struct gprs_rlcmac_bts *bts);
 	void bts_recalc_max_cs(struct gprs_rlcmac_bts *bts);
 	void bts_recalc_max_mcs(struct gprs_rlcmac_bts *bts);
 	struct GprsMs *bts_ms_by_imsi(struct BTS *bts, const char *imsi);
diff --git a/src/gprs_pcu.c b/src/gprs_pcu.c
index 013c026..bc9b350 100644
--- a/src/gprs_pcu.c
+++ b/src/gprs_pcu.c
@@ -103,6 +103,24 @@
 	return pcu;
 }
 
+void gprs_pcu_set_initial_cs(struct gprs_pcu *pcu, uint8_t cs_dl, uint8_t cs_ul)
+{
+	the_pcu->vty.initial_cs_dl = cs_dl;
+	the_pcu->vty.initial_cs_ul = cs_ul;
+
+	/*TODO: once we support multiple bts, foreach(bts) apply */
+	struct gprs_rlcmac_bts *bts = bts_data(pcu->bts);
+	bts_recalc_initial_cs(bts);
+}
+void gprs_pcu_set_initial_mcs(struct gprs_pcu *pcu, uint8_t mcs_dl, uint8_t mcs_ul)
+{
+	the_pcu->vty.initial_mcs_dl = mcs_dl;
+	the_pcu->vty.initial_mcs_ul = mcs_ul;
+
+	/*TODO: once we support multiple bts, foreach(bts) apply */
+	struct gprs_rlcmac_bts *bts = bts_data(pcu->bts);
+	bts_recalc_initial_mcs(bts);
+}
 
 void gprs_pcu_set_max_cs(struct gprs_pcu *pcu, uint8_t cs_dl, uint8_t cs_ul)
 {
diff --git a/src/gprs_pcu.h b/src/gprs_pcu.h
index 8fc15dc..8913001 100644
--- a/src/gprs_pcu.h
+++ b/src/gprs_pcu.h
@@ -79,6 +79,8 @@
 		uint32_t fc_ms_leak_rate;
 		bool force_initial_cs;	/* false=use from BTS true=use from VTY */
 		bool force_initial_mcs;	/* false=use from BTS true=use from VTY */
+		uint8_t initial_cs_dl, initial_cs_ul;
+		uint8_t initial_mcs_dl, initial_mcs_ul;
 		uint8_t max_cs_dl, max_cs_ul;
 		uint8_t max_mcs_dl, max_mcs_ul;
 		uint8_t force_two_phase;
@@ -122,5 +124,7 @@
 
 struct gprs_pcu *gprs_pcu_alloc(void *ctx);
 
+void gprs_pcu_set_initial_cs(struct gprs_pcu *pcu, uint8_t cs_dl, uint8_t cs_ul);
+void gprs_pcu_set_initial_mcs(struct gprs_pcu *pcu, uint8_t mcs_dl, uint8_t mcs_ul);
 void gprs_pcu_set_max_cs(struct gprs_pcu *pcu, uint8_t cs_dl, uint8_t cs_ul);
 void gprs_pcu_set_max_mcs(struct gprs_pcu *pcu, uint8_t mcs_dl, uint8_t mcs_ul);
diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp
index 4a6c53e..a11ec7e 100644
--- a/src/pcu_l1_if.cpp
+++ b/src/pcu_l1_if.cpp
@@ -631,37 +631,13 @@
 
 	LOGP(DL1IF, LOGL_DEBUG, " initial_cs=%u%s\n", info_ind->initial_cs,
 	     the_pcu->vty.force_initial_cs ? " (VTY forced, ignoring)" : "");
-	if (!the_pcu->vty.force_initial_cs) {
-		if (info_ind->initial_cs > bts->bts->max_cs_dl()) {
-			LOGP(DL1IF, LOGL_DEBUG, " downgrading initial_cs_dl to %d\n", bts->bts->max_cs_dl());
-			bts->initial_cs_dl = bts->bts->max_cs_dl();
-		} else {
-			bts->initial_cs_dl =  info_ind->initial_cs;
-		}
-		if (info_ind->initial_cs > bts->bts->max_cs_ul()) {
-			LOGP(DL1IF, LOGL_DEBUG, " downgrading initial_cs_ul to %d\n", bts->bts->max_cs_ul());
-			bts->initial_cs_ul = bts->bts->max_cs_ul();
-		} else {
-			bts->initial_cs_ul =  info_ind->initial_cs;
-		}
-	}
+	bts->pcuif_info_ind.initial_cs = info_ind->initial_cs;
+	bts_recalc_initial_cs(bts);
 
 	LOGP(DL1IF, LOGL_DEBUG, " initial_mcs=%u%s\n", info_ind->initial_mcs,
 	     the_pcu->vty.force_initial_mcs ? " (VTY forced, ignoring)" : "");
-	if (!the_pcu->vty.force_initial_mcs) {
-		if (info_ind->initial_mcs > bts->bts->max_mcs_dl()) {
-			LOGP(DL1IF, LOGL_DEBUG, " downgrading initial_mcs_dl to %d\n", bts->bts->max_mcs_dl());
-			bts->initial_mcs_dl = bts->bts->max_mcs_dl();
-		} else {
-			bts->initial_mcs_dl =  info_ind->initial_mcs;
-		}
-		if (info_ind->initial_mcs > bts->bts->max_mcs_ul()) {
-			LOGP(DL1IF, LOGL_DEBUG, " downgrading initial_mcs_ul to %d\n", bts->bts->max_mcs_ul());
-			bts->initial_mcs_ul = bts->bts->max_mcs_ul();
-		} else {
-			bts->initial_mcs_ul =  info_ind->initial_mcs;
-		}
-	}
+	bts->pcuif_info_ind.initial_mcs = info_ind->initial_mcs;
+	bts_recalc_initial_mcs(bts);
 
 	pcu = gprs_bssgp_init(
 			bts,
diff --git a/src/pcu_vty.c b/src/pcu_vty.c
index 0be914b..ed1a6a4 100644
--- a/src/pcu_vty.c
+++ b/src/pcu_vty.c
@@ -102,7 +102,6 @@
 
 static int config_write_pcu(struct vty *vty)
 {
-	struct gprs_rlcmac_bts *bts = bts_main_data();
 	unsigned int i;
 
 	vty_out(vty, "pcu%s", VTY_NEWLINE);
@@ -121,12 +120,12 @@
 		vty_out(vty, " flow-control force-ms-leak-rate %d%s",
 			the_pcu->vty.fc_ms_leak_rate, VTY_NEWLINE);
 	if (the_pcu->vty.force_initial_cs) {
-		if (bts->initial_cs_ul == bts->initial_cs_dl)
-			vty_out(vty, " cs %d%s", bts->initial_cs_dl,
+		if (the_pcu->vty.initial_cs_ul == the_pcu->vty.initial_cs_dl)
+			vty_out(vty, " cs %d%s", the_pcu->vty.initial_cs_dl,
 				VTY_NEWLINE);
 		else
-			vty_out(vty, " cs %d %d%s", bts->initial_cs_dl,
-				bts->initial_cs_ul, VTY_NEWLINE);
+			vty_out(vty, " cs %d %d%s", the_pcu->vty.initial_cs_dl,
+				the_pcu->vty.initial_cs_ul, VTY_NEWLINE);
 	}
 	if (the_pcu->vty.max_cs_dl && the_pcu->vty.max_cs_ul) {
 		if (the_pcu->vty.max_cs_ul == the_pcu->vty.max_cs_dl)
@@ -178,12 +177,12 @@
 		VTY_NEWLINE);
 
 	if (the_pcu->vty.force_initial_mcs) {
-		if (bts->initial_mcs_ul == bts->initial_mcs_dl)
-			vty_out(vty, " mcs %d%s", bts->initial_mcs_dl,
+		if (the_pcu->vty.initial_mcs_ul == the_pcu->vty.initial_mcs_dl)
+			vty_out(vty, " mcs %d%s", the_pcu->vty.initial_mcs_dl,
 				VTY_NEWLINE);
 		else
-			vty_out(vty, " mcs %d %d%s", bts->initial_mcs_dl,
-				bts->initial_mcs_ul, VTY_NEWLINE);
+			vty_out(vty, " mcs %d %d%s", the_pcu->vty.initial_mcs_dl,
+				the_pcu->vty.initial_mcs_ul, VTY_NEWLINE);
 	}
 
 	if (the_pcu->vty.max_mcs_dl && the_pcu->vty.max_mcs_ul) {
@@ -411,16 +410,14 @@
 	   "Use a different initial CS value for the uplink",
 	   CMD_ATTR_IMMEDIATE)
 {
-	struct gprs_rlcmac_bts *bts = bts_main_data();
-	uint8_t cs = atoi(argv[0]);
-
-	the_pcu->vty.force_initial_cs = true;
-	bts->initial_cs_dl = cs;
+	uint8_t cs_dl, cs_ul;
+	cs_dl = atoi(argv[0]);
 	if (argc > 1)
-		bts->initial_cs_ul = atoi(argv[1]);
+		cs_ul = atoi(argv[1]);
 	else
-		bts->initial_cs_ul = cs;
-
+		cs_ul = cs_dl;
+	the_pcu->vty.force_initial_cs = true;
+	gprs_pcu_set_initial_cs(the_pcu, cs_dl, cs_ul);
 	return CMD_SUCCESS;
 }
 
@@ -431,7 +428,7 @@
 	   CMD_ATTR_IMMEDIATE)
 {
 	the_pcu->vty.force_initial_cs = false;
-
+	gprs_pcu_set_initial_cs(the_pcu, 0, 0);
 	return CMD_SUCCESS;
 }
 
@@ -476,16 +473,14 @@
 	   "Use a different initial MCS value for the uplink",
 	   CMD_ATTR_IMMEDIATE)
 {
-	struct gprs_rlcmac_bts *bts = bts_main_data();
-	uint8_t mcs = atoi(argv[0]);
-
-	the_pcu->vty.force_initial_mcs = true;
-	bts->initial_mcs_dl = mcs;
+	uint8_t mcs_dl, mcs_ul;
+	mcs_dl = atoi(argv[0]);
 	if (argc > 1)
-		bts->initial_mcs_ul = atoi(argv[1]);
+		mcs_ul = atoi(argv[1]);
 	else
-		bts->initial_mcs_ul = mcs;
-
+		mcs_ul = mcs_dl;
+	the_pcu->vty.force_initial_mcs = true;
+	gprs_pcu_set_initial_mcs(the_pcu, mcs_dl, mcs_ul);
 	return CMD_SUCCESS;
 }
 
@@ -496,7 +491,7 @@
 	   CMD_ATTR_IMMEDIATE)
 {
 	the_pcu->vty.force_initial_mcs = false;
-
+	gprs_pcu_set_initial_mcs(the_pcu, 0, 0);
 	return CMD_SUCCESS;
 }
 

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/22195
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I80a6ba401f9c0c85bdf6e0cc99a9d2008d31e1b0
Gerrit-Change-Number: 22195
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: lynxis lazus <lynxis at fe80.eu>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210119/a6ea9e78/attachment.htm>


More information about the gerrit-log mailing list