Change in osmo-bts[master]: A-bis/RSL: refactor handling of BS Power IE (power reduction)

2020-06-16 Thread laforge 
laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/18835 )

Change subject: A-bis/RSL: refactor handling of BS Power IE (power reduction)
..

A-bis/RSL: refactor handling of BS Power IE (power reduction)

According to 3GPP TS 08.58, section 9.3.4, BS Power IE indicates
the transmission power attenuation on a particular channel:

  +--+-+-+
  | Reserved (3) | FPC (1) | Power level (4) |
  +--+-+-+

so let's change handling of this IE as follows:

  - s/bs_power/bs_power_red/g, so it reflects 'reduction';
  - store power attenuation value in dB, not in 2 db steps;
  - get rid of ms_power_ctrl.bts_tx_pwr, it's always 0 anyway;
- fix rsl_tx_meas_res(): use lchan->bs_power_red;
  - always check if FPC (Fast Power Control) flag is set;
- we don't support it, so reject messages containing it;
- fix rsl_rx_chan_activ(): properly apply the bitmask.

Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
---
M include/osmo-bts/gsm_data_shared.h
M include/osmo-bts/tx_power.h
M src/common/rsl.c
M src/common/scheduler.c
M src/common/tx_power.c
M src/common/vty.c
M tests/tx_power/tx_power_test.c
7 files changed, 50 insertions(+), 43 deletions(-)

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



diff --git a/include/osmo-bts/gsm_data_shared.h 
b/include/osmo-bts/gsm_data_shared.h
index 7cfbfeb..bbc1bd4 100644
--- a/include/osmo-bts/gsm_data_shared.h
+++ b/include/osmo-bts/gsm_data_shared.h
@@ -193,8 +193,6 @@
uint8_t flags;
/* RSL measurement result number, 0 at lchan_act */
uint8_t res_nr;
-   /* current Tx power level of the BTS */
-   uint8_t bts_tx_pwr;
/* number of measurements stored in array below */
uint8_t num_ul_meas;
struct bts_ul_meas uplink[MAX_NUM_UL_MEAS];
@@ -267,8 +265,9 @@
uint8_t max;
bool fixed;
} ms_power_ctrl;
-   /* Power levels for BTS */
-   uint8_t bs_power;
+
+   /* BTS power reduction (in dB) */
+   uint8_t bs_power_red;

struct msgb *pending_rel_ind_msg;

diff --git a/include/osmo-bts/tx_power.h b/include/osmo-bts/tx_power.h
index 10129eb..8f33100 100644
--- a/include/osmo-bts/tx_power.h
+++ b/include/osmo-bts/tx_power.h
@@ -62,15 +62,15 @@

 int get_p_nominal_mdBm(struct gsm_bts_trx *trx);

-int get_p_target_mdBm(struct gsm_bts_trx *trx, uint8_t bs_power_ie);
+int get_p_target_mdBm(struct gsm_bts_trx *trx, uint8_t bs_power_red);
 int get_p_target_mdBm_lchan(struct gsm_lchan *lchan);

 int get_p_actual_mdBm(struct gsm_bts_trx *trx, int p_target_mdBm);

-int get_p_trxout_target_mdBm(struct gsm_bts_trx *trx, uint8_t bs_power_ie);
+int get_p_trxout_target_mdBm(struct gsm_bts_trx *trx, uint8_t bs_power_red);
 int get_p_trxout_target_mdBm_lchan(struct gsm_lchan *lchan);

-int get_p_trxout_actual_mdBm(struct gsm_bts_trx *trx, uint8_t bs_power_ie);
+int get_p_trxout_actual_mdBm(struct gsm_bts_trx *trx, uint8_t bs_power_red);
 int get_p_trxout_actual_mdBm_lchan(struct gsm_lchan *lchan);

 int power_ramp_start(struct gsm_bts_trx *trx, int p_total_tgt_mdBm, int 
bypass);
diff --git a/src/common/rsl.c b/src/common/rsl.c
index f057a89..b518245 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -60,6 +60,9 @@

 //#define FAKE_CIPH_MODE_COMPL

+/* Parse power attenuation (in dB) from BS Power IE (see 9.3.4) */
+#define BS_POWER2DB(bs_power) \
+   ((bs_power & 0x0f) * 2)

 static int rsl_tx_error_report(struct gsm_bts_trx *trx, uint8_t cause, const 
uint8_t *chan_nr,
const uint8_t *link_id, const struct msgb 
*orig_msg);
@@ -1001,7 +1004,7 @@
lchan->tch_mode = 0;
memset(>encr, 0, sizeof(lchan->encr));
memset(>ho, 0, sizeof(lchan->ho));
-   lchan->bs_power = 0;
+   lchan->bs_power_red = 0;
memset(>ms_power_ctrl, 0, sizeof(lchan->ms_power_ctrl));
lchan->rqd_ta = 0;
copy_sacch_si_to_lchan(lchan);
@@ -1146,8 +1149,18 @@
}

/* 9.3.4 BS Power */
-   if (TLVP_PRES_LEN(, RSL_IE_BS_POWER, 1))
-   lchan->bs_power = *TLVP_VAL(, RSL_IE_BS_POWER);
+   if (TLVP_PRES_LEN(, RSL_IE_BS_POWER, 1)) {
+   if (*TLVP_VAL(, RSL_IE_BS_POWER) & (1 << 4)) {
+   LOGPLCHAN(lchan, DRSL, LOGL_NOTICE,
+ "Fast Power Control is not supported\n");
+   return rsl_tx_chan_act_nack(lchan, 
RSL_ERR_SERV_OPT_UNIMPL);
+   }
+
+   lchan->bs_power_red = BS_POWER2DB(*TLVP_VAL(, 
RSL_IE_BS_POWER));
+   LOGPLCHAN(lchan, DRSL, LOGL_DEBUG, "BS Power attenuation %u 
dB\n",
+ lchan->bs_power_red);
+   }
+
/*

Change in osmo-bts[master]: A-bis/RSL: refactor handling of BS Power IE (power reduction)

2020-06-16 Thread laforge 
laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/18835 )

Change subject: A-bis/RSL: refactor handling of BS Power IE (power reduction)
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
Gerrit-Change-Number: 18835
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 16 Jun 2020 08:32:04 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-bts[master]: A-bis/RSL: refactor handling of BS Power IE (power reduction)

2020-06-15 Thread fixeria 
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/18835 )

Change subject: A-bis/RSL: refactor handling of BS Power IE (power reduction)
..


Patch Set 2: Verified+1

Makes both BTS_Tests.TC_rsl_bs_pwr_static_ass and 
BTS_Tests.TC_rsl_bs_pwr_static_power_control pass.


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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
Gerrit-Change-Number: 18835
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Mon, 15 Jun 2020 18:45:34 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-bts[master]: A-bis/RSL: refactor handling of BS Power IE (power reduction)

2020-06-15 Thread pespin 
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/18835 )

Change subject: A-bis/RSL: refactor handling of BS Power IE (power reduction)
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
Gerrit-Change-Number: 18835
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Mon, 15 Jun 2020 15:57:13 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-bts[master]: A-bis/RSL: refactor handling of BS Power IE (power reduction)

2020-06-15 Thread fixeria 
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/c/osmo-bts/+/18835

to look at the new patch set (#2).

Change subject: A-bis/RSL: refactor handling of BS Power IE (power reduction)
..

A-bis/RSL: refactor handling of BS Power IE (power reduction)

According to 3GPP TS 08.58, section 9.3.4, BS Power IE indicates
the transmission power attenuation on a particular channel:

  +--+-+-+
  | Reserved (3) | FPC (1) | Power level (4) |
  +--+-+-+

so let's change handling of this IE as follows:

  - s/bs_power/bs_power_red/g, so it reflects 'reduction';
  - store power attenuation value in dB, not in 2 db steps;
  - get rid of ms_power_ctrl.bts_tx_pwr, it's always 0 anyway;
- fix rsl_tx_meas_res(): use lchan->bs_power_red;
  - always check if FPC (Fast Power Control) flag is set;
- we don't support it, so reject messages containing it;
- fix rsl_rx_chan_activ(): properly apply the bitmask.

Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
---
M include/osmo-bts/gsm_data_shared.h
M include/osmo-bts/tx_power.h
M src/common/rsl.c
M src/common/scheduler.c
M src/common/tx_power.c
M src/common/vty.c
M tests/tx_power/tx_power_test.c
7 files changed, 50 insertions(+), 43 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/35/18835/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/18835
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
Gerrit-Change-Number: 18835
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-CC: pespin 
Gerrit-MessageType: newpatchset

Change in osmo-bts[master]: A-bis/RSL: refactor handling of BS Power IE (power reduction)

2020-06-15 Thread fixeria 
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/18835 )

Change subject: A-bis/RSL: refactor handling of BS Power IE (power reduction)
..


Patch Set 1:

(2 comments)

https://gerrit.osmocom.org/c/osmo-bts/+/18835/1/include/osmo-bts/gsm_data_shared.h
File include/osmo-bts/gsm_data_shared.h:

https://gerrit.osmocom.org/c/osmo-bts/+/18835/1/include/osmo-bts/gsm_data_shared.h@269
PS1, Line 269:  /* BTS power reduction (2 dB steps) */
> I think it makes more sense to keep it in dB internally, like we do for all 
> other power, pwr reducti […]
ACK.


https://gerrit.osmocom.org/c/osmo-bts/+/18835/1/src/common/tx_power.c
File src/common/tx_power.c:

https://gerrit.osmocom.org/c/osmo-bts/+/18835/1/src/common/tx_power.c@116
PS1, Line 116: int get_p_trxout_target_mdBm(struct gsm_bts_trx *trx, uint8_t 
bs_power_ie)
I'll change it in the next patch set.

> but not sure if this code is actually expecting 1dB unit instead 2db step.

The code does * 2, so it's correct. I'll change it too.



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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
Gerrit-Change-Number: 18835
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Mon, 15 Jun 2020 15:52:51 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in osmo-bts[master]: A-bis/RSL: refactor handling of BS Power IE (power reduction)

2020-06-15 Thread pespin 
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/18835 )

Change subject: A-bis/RSL: refactor handling of BS Power IE (power reduction)
..


Patch Set 1:

(2 comments)

https://gerrit.osmocom.org/c/osmo-bts/+/18835/1/include/osmo-bts/gsm_data_shared.h
File include/osmo-bts/gsm_data_shared.h:

https://gerrit.osmocom.org/c/osmo-bts/+/18835/1/include/osmo-bts/gsm_data_shared.h@269
PS1, Line 269:  /* BTS power reduction (2 dB steps) */
I think it makes more sense to keep it in dB internally, like we do for all 
other power, pwr reduction/attenuations.


https://gerrit.osmocom.org/c/osmo-bts/+/18835/1/src/common/tx_power.c
File src/common/tx_power.c:

https://gerrit.osmocom.org/c/osmo-bts/+/18835/1/src/common/tx_power.c@116
PS1, Line 116: int get_p_trxout_target_mdBm(struct gsm_bts_trx *trx, uint8_t 
bs_power_ie)
here too: bs_power_red_ie.

BTW, current name may be wrong too, as in it says "ie" meaning it's 2db step, 
but not sure if this code is actually expecting 1dB unit instead 2db step.



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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
Gerrit-Change-Number: 18835
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Mon, 15 Jun 2020 15:12:59 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in osmo-bts[master]: A-bis/RSL: refactor handling of BS Power IE (power reduction)

2020-06-15 Thread fixeria 
fixeria has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/18835 )


Change subject: A-bis/RSL: refactor handling of BS Power IE (power reduction)
..

A-bis/RSL: refactor handling of BS Power IE (power reduction)

According to 3GPP TS 08.58, section 9.3.4, BS Power IE indicates
the transmission power attenuation on a particular channel:

  +--+-+-+
  | Reserved (3) | FPC (1) | Power level (4) |
  +--+-+-+

so let's change handling of this IE as follows:

  - s/bs_power/bs_power_red/g, so it reflects 'reduction';
  - get rid of ms_power_ctrl.bts_tx_pwr, it's always 0 anyway;
- fix rsl_tx_meas_res(): use lchan->bs_power_red;
  - always check if FPC (Fast Power Control) flag is set;
- we don't support it, so reject messages containing it;
- fix rsl_rx_chan_activ(): properly apply the bitmask.

Change-Id: I16cc50dfca102030380a06e16c234d5f6698f38f
---
M include/osmo-bts/gsm_data_shared.h
M src/common/rsl.c
M src/common/tx_power.c
M src/common/vty.c
4 files changed, 26 insertions(+), 24 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/35/18835/1

diff --git a/include/osmo-bts/gsm_data_shared.h 
b/include/osmo-bts/gsm_data_shared.h
index 7cfbfeb..c3dba3c 100644
--- a/include/osmo-bts/gsm_data_shared.h
+++ b/include/osmo-bts/gsm_data_shared.h
@@ -193,8 +193,6 @@
uint8_t flags;
/* RSL measurement result number, 0 at lchan_act */
uint8_t res_nr;
-   /* current Tx power level of the BTS */
-   uint8_t bts_tx_pwr;
/* number of measurements stored in array below */
uint8_t num_ul_meas;
struct bts_ul_meas uplink[MAX_NUM_UL_MEAS];
@@ -267,8 +265,9 @@
uint8_t max;
bool fixed;
} ms_power_ctrl;
-   /* Power levels for BTS */
-   uint8_t bs_power;
+
+   /* BTS power reduction (2 dB steps) */
+   uint8_t bs_power_red;

struct msgb *pending_rel_ind_msg;
 
diff --git a/src/common/rsl.c b/src/common/rsl.c
index f057a89..b9c9695 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -1001,7 +1001,7 @@
lchan->tch_mode = 0;
memset(>encr, 0, sizeof(lchan->encr));
memset(>ho, 0, sizeof(lchan->ho));
-   lchan->bs_power = 0;
+   lchan->bs_power_red = 0;
memset(>ms_power_ctrl, 0, sizeof(lchan->ms_power_ctrl));
lchan->rqd_ta = 0;
copy_sacch_si_to_lchan(lchan);
@@ -1146,8 +1146,15 @@
}

/* 9.3.4 BS Power */
-   if (TLVP_PRES_LEN(, RSL_IE_BS_POWER, 1))
-   lchan->bs_power = *TLVP_VAL(, RSL_IE_BS_POWER);
+   if (TLVP_PRES_LEN(, RSL_IE_BS_POWER, 1)) {
+   if (*TLVP_VAL(, RSL_IE_BS_POWER) & (1 << 4)) {
+   LOGPLCHAN(lchan, DRSL, LOGL_NOTICE, "Fast Power Control 
is not supported\n");
+   return rsl_tx_chan_act_nack(lchan, 
RSL_ERR_SERV_OPT_UNIMPL);
+   }
+
+   lchan->bs_power_red = *TLVP_VAL(, RSL_IE_BS_POWER) & 0x0f;
+   }
+
/* 9.3.13 MS Power */
if (TLVP_PRES_LEN(, RSL_IE_MS_POWER, 1)) {
lchan->ms_power_ctrl.max = *TLVP_VAL(, RSL_IE_MS_POWER) & 
0x1F;
@@ -1664,21 +1671,13 @@
return 0;
 }

-/* See TS 48.058 Section 9.3.4 */
-static int bs_power_attenuation_dB(uint8_t bs_power)
-{
-   /* the lower nibble contains the number of 2dB steps that the BS power 
is reduced compared
-* to its nominal transmit power */
-   return - ((bs_power & 0xF) *2);
-}
-
 /* 8.4.16 BS POWER CONTROL */
 static int rsl_rx_bs_pwr_ctrl(struct msgb *msg)
 {
struct abis_rsl_dchan_hdr *dch = msgb_l2(msg);
struct gsm_lchan *lchan = msg->lchan;
struct tlv_parsed tp;
-   uint8_t new_bs_power;
+   uint8_t old_bs_power_red;

rsl_tlv_parse(, msgb_l3(msg), msgb_l3len(msg));

@@ -1686,12 +1685,16 @@
if (!TLVP_PRES_LEN(, RSL_IE_BS_POWER, 1))
return rsl_tx_error_report(msg->trx, RSL_ERR_MAND_IE_ERROR, 
>chan_nr, NULL, msg);

-   new_bs_power = *TLVP_VAL(, RSL_IE_BS_POWER);
+   if (*TLVP_VAL(, RSL_IE_BS_POWER) & (1 << 4)) {
+   LOGPLCHAN(lchan, DRSL, LOGL_NOTICE, "Fast Power Control is not 
supported\n");
+   return rsl_tx_error_report(msg->trx, RSL_ERR_SERV_OPT_UNIMPL, 
>chan_nr, NULL, msg);
+   }
+
+   old_bs_power_red = lchan->bs_power_red;
+   lchan->bs_power_red = *TLVP_VAL(, RSL_IE_BS_POWER) & 0x0f;

LOGPLCHAN(lchan, DRSL, LOGL_INFO, "BS POWER CONTROL Attenuation %d -> 
%d dB\n",
- bs_power_attenuation_dB(lchan->bs_power), 
bs_power_attenuation_dB(new_bs_power));
-
-   lchan->bs_power = new_bs_power;
+ old_bs_power_red * 2, lchan->bs_power_red * 2);

/* 9.3.31 MS Power Parameters (O) */
if