fixeria has submitted this change. (
https://gerrit.osmocom.org/c/osmo-bts/+/39830?usp=email )
Change subject: fixup: rsl: properly initialize MS/BS Power Control state
......................................................................
fixup: rsl: properly initialize MS/BS Power Control state
Commit 75162427 fixed one problem, but introduced another one:
'''
/* Initialize MS Power Control defaults */
lchan->ms_power_ctrl = (struct lchan_power_ctrl_state) {
.max = ms_pwr_ctl_lvl(lchan->ts->trx->bts->band, 0),
.current = lchan->ms_power_ctrl.max, /* XXX */
};
'''
The intention here was to assign the same value to both 'max' and
'current' fields. However, field 'current' actually gets assigned
whatever value was assigned to field 'max' previously.
This regression was caught by running the BTS_Tests.TC_rsl_ms_pwr_ctrl
several times against the same osmo-bts-trx process.
Let's use the good-old memset() to initialize the MS/BS Power Control
state structures, and move initialization of 'max' and 'current' fields
to the else branches of the RSL_IE_{MS,BS}_POWER IE checks.
Change-Id: I46c881d5a3959c2542610ed767e0f131d01f9f98
Fixes: 75162427 ("rsl: properly initialize MS/BS Power Control state")
Related: SYS#4918, OS#6672
---
M src/common/rsl.c
1 file changed, 11 insertions(+), 14 deletions(-)
Approvals:
fixeria: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/src/common/rsl.c b/src/common/rsl.c
index 1104ef8..f16fe7c 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -1987,17 +1987,9 @@
}
}
- /* Initialize MS Power Control defaults */
- lchan->ms_power_ctrl = (struct lchan_power_ctrl_state) {
- .max = ms_pwr_ctl_lvl(lchan->ts->trx->bts->band, 0),
- .current = lchan->ms_power_ctrl.max,
- };
-
- /* Initialize BS Power Control defaults */
- lchan->bs_power_ctrl = (struct lchan_power_ctrl_state) {
- .max = 2 * 15, /* maximum defined in 9.3.4 */
- .current = 0,
- };
+ /* Initialize MS/BS Power Control state */
+ memset(&lchan->ms_power_ctrl, 0, sizeof(lchan->ms_power_ctrl));
+ memset(&lchan->bs_power_ctrl, 0, sizeof(lchan->bs_power_ctrl));
/* 9.3.6 Channel Mode */
if (type != RSL_ACT_OSMO_PDCH) {
@@ -2054,13 +2046,18 @@
LOGPLCHAN(lchan, DRSL, LOGL_DEBUG, "BS Power attenuation %u
dB\n",
lchan->bs_power_ctrl.current);
+ } else {
+ lchan->bs_power_ctrl.max = 2 * 15; /* maximum defined in 9.3.4
*/
+ lchan->bs_power_ctrl.current = 0;
}
/* 9.3.13 MS Power */
- if (TLVP_PRES_LEN(&tp, RSL_IE_MS_POWER, 1)) {
+ if (TLVP_PRES_LEN(&tp, RSL_IE_MS_POWER, 1))
lchan->ms_power_ctrl.max = *TLVP_VAL(&tp, RSL_IE_MS_POWER) &
0x1F;
- lchan->ms_power_ctrl.current = lchan->ms_power_ctrl.max;
- }
+ else /* XXX: should we use the maximum power level instead of 0 dBm? */
+ lchan->ms_power_ctrl.max =
ms_pwr_ctl_lvl(lchan->ts->trx->bts->band, 0);
+ lchan->ms_power_ctrl.current = lchan->ms_power_ctrl.max;
+
/* 9.3.24 Timing Advance */
if (TLVP_PRES_LEN(&tp, RSL_IE_TIMING_ADVANCE, 1))
lchan->ta_ctrl.current = *TLVP_VAL(&tp, RSL_IE_TIMING_ADVANCE);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/39830?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I46c881d5a3959c2542610ed767e0f131d01f9f98
Gerrit-Change-Number: 39830
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
