Change in osmo-bsc[master]: lchan_fsm: generate proper multirate configuration IE on RSL

2018-10-24 Thread Harald Welte
Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/11445 )

Change subject: lchan_fsm: generate proper multirate configuration IE on RSL
..

lchan_fsm: generate proper multirate configuration IE on RSL

During the generation of the multirate configuration IE in the channel
activation message that is sent over RSL, all AMR rates except the
highest one are trimmed. This was to ensure that the multirate
configuration IE only contains one codec rate per active set. Lets fix
that and generate a proper IE with threshold and hysteresis values.

- extend lchan_mr_config so that it can generate a full multirate
  configuration IE

Change-Id: I7f9f8e8d9e2724cbe3ce2f3599bc0e5185fd8453
Related: OS#3529
---
M src/osmo-bsc/lchan_fsm.c
M tests/handover/handover_test.c
2 files changed, 72 insertions(+), 58 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved



diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index 39edaff..915b62a 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -36,6 +36,8 @@
 #include 
 #include 
 #include 
+#include 
+

 static struct osmo_fsm lchan_fsm;

@@ -402,66 +404,71 @@
osmo_fsm_inst_dispatch(lchan->ts->fi, TS_EV_LCHAN_UNUSED, lchan);
 }

-/*! Configure the multirate setting on this channel. */
-void lchan_mr_config(struct gsm_lchan *lchan, struct gsm48_multi_rate_conf 
*mr_conf)
+/* Configure the multirate setting on this channel. */
+static int lchan_mr_config(struct gsm_lchan *lchan, const struct 
gsm48_multi_rate_conf *mr_conf)
 {
-   struct gsm48_multi_rate_conf *ms_conf, *bts_conf;
bool full_rate = (lchan->type == GSM_LCHAN_TCH_F);
+   struct gsm_bts *bts = lchan->ts->trx->bts;
+   struct bsc_msc_data *msc = lchan->conn->sccp.msc;
+   struct amr_multirate_conf *mr;
+   int rc;
+   int rc_rate;
+   struct gsm48_multi_rate_conf mr_conf_filtered;
+   const struct gsm48_multi_rate_conf *mr_conf_bts;

-   /* initialize the data structure */
-   lchan->mr_ms_lv[0] = sizeof(*ms_conf);
-   lchan->mr_bts_lv[0] = sizeof(*bts_conf);
-   ms_conf = (struct gsm48_multi_rate_conf *) >mr_ms_lv[1];
-   bts_conf = (struct gsm48_multi_rate_conf *) >mr_bts_lv[1];
+   /* There are two different active sets, depending on the channel rate,
+* make sure the appropate one is selected. */
+   if (full_rate)
+   mr = >mr_full;
+   else
+   mr = >mr_half;

-   *ms_conf = *bts_conf = (struct gsm48_multi_rate_conf){
-   .ver = 1,
-   .icmi = 1,
-   .m4_75 = mr_conf->m4_75,
-   .m5_15 = mr_conf->m5_15,
-   .m5_90 = mr_conf->m5_90,
-   .m6_70 = mr_conf->m6_70,
-   .m7_40 = mr_conf->m7_40,
-   .m7_95 = mr_conf->m7_95,
-   .m10_2 = full_rate? mr_conf->m10_2 : 0,
-   .m12_2 = full_rate? mr_conf->m12_2 : 0,
-   };
-}
-
-/* Mask all rates instead of the highest possible */
-static void lchan_mr_config_mask(struct gsm48_multi_rate_conf *mr_conf)
-{
-   unsigned int i;
-   bool highest_seen = false;
-   uint8_t *_mr_conf = (uint8_t *) mr_conf;
-
-   /* FIXME: At the moment we can not support multiple codec rates in one
-* struct gsm48_multi_rate_conf, because the struct lacks the fields
-* for Threshold and Hysteresis. Those fields are not needed when only
-* a single codec rate is in place, but as soon as multiple codec
-* rates are used the parameters are mandatory. The layout for the
-* struct would then also be different because each rate needs its
-* own Threshold and Hysteresis value. (See also 3GPP TS 04.08,
-* chapter 10.5.2.21aa MultiRate configuration).
-*
-* Since we are unable to signal multiple codec rates properly, we just
-* remove all codec rates from the active set, except the highest
-* possible. Doing so we lack the functionality to switch towards the
-* other, lower codec rates that were offered by the MSC, but it is
-* still guaranteed that a rate is selected that is supported by all
-* entities.
-*
-* To fix this problem, we should implement a proper encoder for
-* struct gsm48_multi_rate_conf, in libosmocore and use it here.
-* struct amr_mode already seems to have members for threshold and
-* hysteresis we can use. */
-
-   for (i = 7; i > 0; i--) {
-   if (_mr_conf[1] & (1 << i) && highest_seen == false) {
-   highest_seen = true;
-   } else if (highest_seen)
-   _mr_conf[1] &= ~(1 << i);
+   /* The VTY allows to forbid certain codec rates. Unfortunately we can
+* not articulate all of the prohibitions on through S0-S15 on the A
+* 

Change in osmo-bsc[master]: lchan_fsm: generate proper multirate configuration IE on RSL

2018-10-23 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/11445 )

Change subject: lchan_fsm: generate proper multirate configuration IE on RSL
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/11445
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f9f8e8d9e2724cbe3ce2f3599bc0e5185fd8453
Gerrit-Change-Number: 11445
Gerrit-PatchSet: 2
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Tue, 23 Oct 2018 18:22:36 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-bsc[master]: lchan_fsm: generate proper multirate configuration IE on RSL

2018-10-23 Thread dexter
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/11445

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

Change subject: lchan_fsm: generate proper multirate configuration IE on RSL
..

lchan_fsm: generate proper multirate configuration IE on RSL

During the generation of the multirate configuration IE in the channel
activation message that is sent over RSL, all AMR rates except the
highest one are trimmed. This was to ensure that the multirate
configuration IE only contains one codec rate per active set. Lets fix
that and generate a proper IE with threshold and hysteresis values.

- extend lchan_mr_config so that it can generate a full multirate
  configuration IE

Change-Id: I7f9f8e8d9e2724cbe3ce2f3599bc0e5185fd8453
Related: OS#3529
---
M src/osmo-bsc/lchan_fsm.c
M tests/handover/handover_test.c
2 files changed, 72 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/45/11445/2
--
To view, visit https://gerrit.osmocom.org/11445
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7f9f8e8d9e2724cbe3ce2f3599bc0e5185fd8453
Gerrit-Change-Number: 11445
Gerrit-PatchSet: 2
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder (102)


Change in osmo-bsc[master]: lchan_fsm: generate proper multirate configuration IE on RSL

2018-10-23 Thread dexter
dexter has uploaded this change for review. ( https://gerrit.osmocom.org/11445


Change subject: lchan_fsm: generate proper multirate configuration IE on RSL
..

lchan_fsm: generate proper multirate configuration IE on RSL

During the generation of the multirate configuration IE in the channel
activation message that is sent over RSL, all AMR rates except the
highest one are trimmed. This was to ensure that the multirate
configuration IE only contains one codec rate per active set. Lets fix
that and generate a proper IE with threshold and hysteresis values.

- extend lchan_mr_config so that it can generate a full multirate
  configuration IE

Change-Id: I7f9f8e8d9e2724cbe3ce2f3599bc0e5185fd8453
Related: OS#3529
---
M src/osmo-bsc/lchan_fsm.c
M tests/handover/handover_test.c
2 files changed, 72 insertions(+), 58 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/45/11445/1

diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index 39edaff..915b62a 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -36,6 +36,8 @@
 #include 
 #include 
 #include 
+#include 
+

 static struct osmo_fsm lchan_fsm;

@@ -402,66 +404,71 @@
osmo_fsm_inst_dispatch(lchan->ts->fi, TS_EV_LCHAN_UNUSED, lchan);
 }

-/*! Configure the multirate setting on this channel. */
-void lchan_mr_config(struct gsm_lchan *lchan, struct gsm48_multi_rate_conf 
*mr_conf)
+/* Configure the multirate setting on this channel. */
+static int lchan_mr_config(struct gsm_lchan *lchan, const struct 
gsm48_multi_rate_conf *mr_conf)
 {
-   struct gsm48_multi_rate_conf *ms_conf, *bts_conf;
bool full_rate = (lchan->type == GSM_LCHAN_TCH_F);
+   struct gsm_bts *bts = lchan->ts->trx->bts;
+   struct bsc_msc_data *msc = lchan->conn->sccp.msc;
+   struct amr_multirate_conf *mr;
+   int rc;
+   int rc_rate;
+   struct gsm48_multi_rate_conf mr_conf_filtered;
+   const struct gsm48_multi_rate_conf *mr_conf_bts;

-   /* initialize the data structure */
-   lchan->mr_ms_lv[0] = sizeof(*ms_conf);
-   lchan->mr_bts_lv[0] = sizeof(*bts_conf);
-   ms_conf = (struct gsm48_multi_rate_conf *) >mr_ms_lv[1];
-   bts_conf = (struct gsm48_multi_rate_conf *) >mr_bts_lv[1];
+   /* There are two different active sets, depending on the channel rate,
+* make sure the appropate one is selected. */
+   if (full_rate)
+   mr = >mr_full;
+   else
+   mr = >mr_half;

-   *ms_conf = *bts_conf = (struct gsm48_multi_rate_conf){
-   .ver = 1,
-   .icmi = 1,
-   .m4_75 = mr_conf->m4_75,
-   .m5_15 = mr_conf->m5_15,
-   .m5_90 = mr_conf->m5_90,
-   .m6_70 = mr_conf->m6_70,
-   .m7_40 = mr_conf->m7_40,
-   .m7_95 = mr_conf->m7_95,
-   .m10_2 = full_rate? mr_conf->m10_2 : 0,
-   .m12_2 = full_rate? mr_conf->m12_2 : 0,
-   };
-}
-
-/* Mask all rates instead of the highest possible */
-static void lchan_mr_config_mask(struct gsm48_multi_rate_conf *mr_conf)
-{
-   unsigned int i;
-   bool highest_seen = false;
-   uint8_t *_mr_conf = (uint8_t *) mr_conf;
-
-   /* FIXME: At the moment we can not support multiple codec rates in one
-* struct gsm48_multi_rate_conf, because the struct lacks the fields
-* for Threshold and Hysteresis. Those fields are not needed when only
-* a single codec rate is in place, but as soon as multiple codec
-* rates are used the parameters are mandatory. The layout for the
-* struct would then also be different because each rate needs its
-* own Threshold and Hysteresis value. (See also 3GPP TS 04.08,
-* chapter 10.5.2.21aa MultiRate configuration).
-*
-* Since we are unable to signal multiple codec rates properly, we just
-* remove all codec rates from the active set, except the highest
-* possible. Doing so we lack the functionality to switch towards the
-* other, lower codec rates that were offered by the MSC, but it is
-* still guaranteed that a rate is selected that is supported by all
-* entities.
-*
-* To fix this problem, we should implement a proper encoder for
-* struct gsm48_multi_rate_conf, in libosmocore and use it here.
-* struct amr_mode already seems to have members for threshold and
-* hysteresis we can use. */
-
-   for (i = 7; i > 0; i--) {
-   if (_mr_conf[1] & (1 << i) && highest_seen == false) {
-   highest_seen = true;
-   } else if (highest_seen)
-   _mr_conf[1] &= ~(1 << i);
+   /* The VTY allows to forbid certain codec rates. Unfortunately we can
+* not articulate all of the prohibitions on through S0-S15 on the A
+* interface. To ensure