[MERGED] osmo-bsc[master]: HO: cfg: separate hodec1 from hodec2 parameters

2018-02-19 Thread Neels Hofmeyr 
Neels Hofmeyr has submitted this change and it was merged.

Change subject: HO: cfg: separate hodec1 from hodec2 parameters
..


HO: cfg: separate hodec1 from hodec2 parameters

Do not share config items between the current handover decision and the
upcoming handover_decision_2.

Rename current handover config items to hodec2_* and duplicate the ones
relevant to handover decision algorithm 1 with name prefix of hodec1_*.

I considered moving hodec2 parameters to an entirely separate .c file and
struct, but that causes considerable code bloat. Rather use the nice
handover_cfg net/bts level mechanism as-is, and simply prefix the names.

In the VTY, the hodec1 parameters are configurable by 'handover foo 23'
commands, while the hodec2 parameters are by 'handover2 foo 23'. The generic
VTY commands to enable/disable handover and to choose the algorithm are still
'handover (0|1)' and 'handover algorithm (1|2)'.

(Note, a subsequent commit will rename the 'handover foo' for hodec1 to
'handover1 foo' and add backwards-compat aliases.)

For example, the 'window rxlev averaging 5' command now exists both for
handover decision 1 and handover decision 2, and its values are independent.
This is valid config:

network
 # set up handover decision algorithm 1
 # (pending rename of these items to 'handover1 ...')
 handover window rxlev averaging 5
 handover window rxlev neighbor averaging 5

 # set up handover decision algorithm 2
 handover2 window rxlev averaging 7
 handover2 window rxlev neighbor averaging 7
 handover2 penalty-time max-distance 10

 # enable handover
 handover 1

 bts 0
  handover algorithm 1
 bts 1
  handover algorithm 2

In this example, bts 0 uses algo 1 with rxlev averaging of 5, while bts 1 uses
algorithm 2 where rxlev averaging of 7 is in effect.

Change-Id: I6475b2543b18d21710a6d774b214cb484f36ec8e
---
M include/osmocom/bsc/handover_cfg.h
M src/libbsc/handover_decision.c
M tests/handover_cfg.vty
3 files changed, 303 insertions(+), 124 deletions(-)

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



diff --git a/include/osmocom/bsc/handover_cfg.h 
b/include/osmocom/bsc/handover_cfg.h
index 63a393e..55b9dbc 100644
--- a/include/osmocom/bsc/handover_cfg.h
+++ b/include/osmocom/bsc/handover_cfg.h
@@ -12,11 +12,12 @@
 
 struct handover_cfg *ho_cfg_init(void *ctx, struct handover_cfg 
*higher_level_cfg);
 
-#define HO_CFG_STR_HANDOVER "Handover options\n"
-#define HO_CFG_STR_WIN HO_CFG_STR_HANDOVER "Measurement averaging settings\n"
+#define HO_CFG_STR_HANDOVER1 "Handover options for handover decision algorithm 
1\n"
+#define HO_CFG_STR_HANDOVER2 "Handover options for handover decision algorithm 
2\n"
+#define HO_CFG_STR_WIN "Measurement averaging settings\n"
 #define HO_CFG_STR_WIN_RXLEV HO_CFG_STR_WIN "Received-Level averaging\n"
 #define HO_CFG_STR_WIN_RXQUAL HO_CFG_STR_WIN "Received-Quality averaging\n"
-#define HO_CFG_STR_POWER_BUDGET HO_CFG_STR_HANDOVER "Neighbor cell power 
triggering\n" "Neighbor cell power triggering\n"
+#define HO_CFG_STR_POWER_BUDGET "Neighbor cell power triggering\n" "Neighbor 
cell power triggering\n"
 #define HO_CFG_STR_AVG_COUNT "Number of values to average over\n"
 #define HO_CFG_STR_2 " (HO algo 2 only)\n"
 #define HO_CFG_STR_MIN "Minimum Level/Quality thresholds before triggering HO" 
HO_CFG_STR_2
@@ -68,151 +69,210 @@
  * VTY_WRITE_CONV: function name to convert struct value to VTY_WRITE_FMT, 
e.g. 'as_is'.
  * VTY_DOC: VTY documentation strings to match VTY_CMD and VTY_CMD_ARGs.
  */
-#define HO_CFG_ALL_MEMBERS \
+#define HO_GENERAL_CFG_ALL_MEMBERS \
\
HO_CFG_ONE_MEMBER(bool, ho_active, 0, \
"handover", "0|1", a2bool, "%d", bool2i, \
-   HO_CFG_STR_HANDOVER \
+   "Handover general config\n" \
"Disable in-call handover\n" \
"Enable in-call handover\n" \
"Enable/disable handover: ") \
\
HO_CFG_ONE_MEMBER(int, algorithm, 1, \
"handover algorithm", "1|2", atoi, "%d", as_is, \
-   HO_CFG_STR_HANDOVER \
+   "Handover general config\n" \
"Choose algorithm for handover decision\n" \
"Algorithm 1: trigger handover based on comparing current cell 
and neighbor RxLev and RxQual," \
" only.\n" \
"Algorithm 2: trigger handover on RxLev/RxQual, and also to 
balance the load across several" \
" cells. Consider available codecs. Prevent repeated handover 
by penalty timers.\n") \
+
+
+#define HODEC1_CFG_ALL_MEMBERS \
\
-   HO_CFG_ONE_MEMBER(unsigned int, rxlev_avg_win, 10, \
+   HO_CFG_ONE_MEMBER(unsigned int, hodec1_rxlev_avg_win, 10, \
"handover window rxlev averaging", "<1-10>", atoi, "%u", as_is, 
\
+   HO_CFG_STR_HANDOVER1 \

osmo-bsc[master]: HO: cfg: separate hodec1 from hodec2 parameters

2018-02-19 Thread Neels Hofmeyr 

Patch Set 3:

(just to note, I actually missed that mail on openbsc@ telling me to not bother 
about this in the first place, sorry about that. For that matter, this patch 
took like half an hour to write and test.)

-- 
To view, visit https://gerrit.osmocom.org/6471
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6475b2543b18d21710a6d774b214cb484f36ec8e
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

osmo-bsc[master]: HO: cfg: separate hodec1 from hodec2 parameters

2018-02-19 Thread Harald Welte 

Patch Set 3: Code-Review+2

-- 
To view, visit https://gerrit.osmocom.org/6471
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6475b2543b18d21710a6d774b214cb484f36ec8e
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No

osmo-bsc[master]: HO: cfg: separate hodec1 from hodec2 parameters

2018-02-17 Thread Neels Hofmeyr 

Patch Set 3:

(2 comments)

I would spend more time on this if it is deemed worthwhile. Let me know what 
you think ...

https://gerrit.osmocom.org/#/c/6471/3//COMMIT_MSG
Commit Message:

Line 38:  handover2 window rxlev averaging 7
> What about using a node instead of prepending the algorithm to each command
I first was going to do that, then gave up on it a little because not 
introducing nodes took less time.

If "we" prefer, I can still review it. But note that I want to keep the old 
'handover foo' commands for backwards compatibility.

So we could add nodes like this:

  network
   # enable
   handover 1
   # choose algo
   handover algorithm 2

   # algo 1 config
   handover1
window rxlev averaging 5
#...

   # algo 2 config
   handover2
window rxlev averaging 5
#...

   # and then we would also still have aliases
   # to be backwards compatible, aliases for handover1
   handover window rxlev averaging 5


https://gerrit.osmocom.org/#/c/6471/3/src/libbsc/handover_decision.c
File src/libbsc/handover_decision.c:

Line 218:   avg = neigh_meas_avg(nmp, 
ho_get_hodec1_rxlev_neigh_avg_win(bts->ho));
> I think it makes more sense to call this ho_hodec1_{get,set}_*, as it's par
hodec means handover decision.

So at first I had the plain

  ho_get_foo / ho_set_foo

with all params shared across handover algos. So the variable part for naming 
is 'foo'.

The easiest way to add different realms then is to prefix the foo part. The way 
you request would add the concept of another section in a more complex way.

To illustrate the current patch state: the namespace currently looks like

  # general choice
  active
  algorithm
  hodec1_rxlev...
  hodec1_yada
  hodec2_rxlev...
  hodec2_yada

The get/set functions produced in handover_cfg.h then end up the above, with 
'ho_get_' and 'ho_set_' etc prefixed.

We could add explicit sections like

  common.active
  common.algorithm
  hodec1.rxlev...
  hodec2.rxlev...

which would make those macros a bit more complex still.

BTW I also considered tossing out those macros and rather generate the C code 
from some templates; the advantage being that we can properly browse the 
produced code with ctags and similar tools, and don't need to read obscure 
macros...

All in all, there is still polishing and perfectioning I could do here. The 
current patch is the easiest way to get it working, since I've already spent 
too much time with code structuring.

I'm ready to restructure more if everyone thinks it's worthwhile.


-- 
To view, visit https://gerrit.osmocom.org/6471
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6475b2543b18d21710a6d774b214cb484f36ec8e
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: Yes

osmo-bsc[master]: HO: cfg: separate hodec1 from hodec2 parameters

2018-02-16 Thread Pau Espin Pedrol 

Patch Set 3:

(2 comments)

https://gerrit.osmocom.org/#/c/6471/3//COMMIT_MSG
Commit Message:

Line 38:  handover2 window rxlev averaging 7
What about using a node instead of prepending the algorithm to each command?


https://gerrit.osmocom.org/#/c/6471/3/src/libbsc/handover_decision.c
File src/libbsc/handover_decision.c:

Line 218:   avg = neigh_meas_avg(nmp, 
ho_get_hodec1_rxlev_neigh_avg_win(bts->ho));
I think it makes more sense to call this ho_hodec1_{get,set}_*, as it's part of 
the scope/namespace imho. Also I'm not sure now what hodec1 stands for now, but 
you can at least drop the second "ho": ho_dec1_*, or use something like 
ho_alg1_* or ho_1_*.


-- 
To view, visit https://gerrit.osmocom.org/6471
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6475b2543b18d21710a6d774b214cb484f36ec8e
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: Yes

osmo-bsc[master]: HO: cfg: separate hodec1 from hodec2 parameters

2018-02-15 Thread Harald Welte 

Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.osmocom.org/6471
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6475b2543b18d21710a6d774b214cb484f36ec8e
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No

[PATCH] osmo-bsc[master]: HO: cfg: separate hodec1 from hodec2 parameters

2018-02-15 Thread Neels Hofmeyr 

Review at  https://gerrit.osmocom.org/6471

HO: cfg: separate hodec1 from hodec2 parameters

Do not share config items between the current handover decision and the
upcoming handover_decision_2.

Rename current handover config items to hodec2_* and duplicate the ones
relevant to handover decision algorithm 1 with name prefix of hodec1_*.

I considered moving hodec2 parameters to an entirely separate .c file and
struct, but that causes considerable code bloat. Rather use the nice
handover_cfg net/bts level mechanism as-is, and simply prefix the names.

In the VTY, the hodec1 parameters are configurable by 'handover foo 23'
commands, while the hodec2 parameters are by 'handover2 foo 23'. The generic
VTY commands to enable/disable handover and to choose the algorithm are still
'handover (0|1)' and 'handover algorithm (1|2)'.

(Note, a subsequent commit will rename the 'handover foo' for hodec1 to
'handover1 foo' and add backwards-compat aliases.)

For example, the 'window rxlev averaging 5' command now exists both for
handover decision 1 and handover decision 2, and its values are independent.
This is valid config:

network
 # set up handover decision algorithm 1
 # (pending rename of these items to 'handover1 ...')
 handover window rxlev averaging 5
 handover window rxlev neighbor averaging 5

 # set up handover decision algorithm 2
 handover2 window rxlev averaging 7
 handover2 window rxlev neighbor averaging 7
 handover2 penalty-time max-distance 10

 # enable handover
 handover 1

 bts 0
  handover algorithm 1
 bts 1
  handover algorithm 2

In this example, bts 0 uses algo 1 with rxlev averaging of 5, while bts 1 uses
algorithm 2 where rxlev averaging of 7 is in effect.

Change-Id: I6475b2543b18d21710a6d774b214cb484f36ec8e
---
M include/osmocom/bsc/handover_cfg.h
M src/libbsc/handover_decision.c
M tests/handover_cfg.vty
3 files changed, 303 insertions(+), 124 deletions(-)


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

diff --git a/include/osmocom/bsc/handover_cfg.h 
b/include/osmocom/bsc/handover_cfg.h
index 63a393e..55b9dbc 100644
--- a/include/osmocom/bsc/handover_cfg.h
+++ b/include/osmocom/bsc/handover_cfg.h
@@ -12,11 +12,12 @@
 
 struct handover_cfg *ho_cfg_init(void *ctx, struct handover_cfg 
*higher_level_cfg);
 
-#define HO_CFG_STR_HANDOVER "Handover options\n"
-#define HO_CFG_STR_WIN HO_CFG_STR_HANDOVER "Measurement averaging settings\n"
+#define HO_CFG_STR_HANDOVER1 "Handover options for handover decision algorithm 
1\n"
+#define HO_CFG_STR_HANDOVER2 "Handover options for handover decision algorithm 
2\n"
+#define HO_CFG_STR_WIN "Measurement averaging settings\n"
 #define HO_CFG_STR_WIN_RXLEV HO_CFG_STR_WIN "Received-Level averaging\n"
 #define HO_CFG_STR_WIN_RXQUAL HO_CFG_STR_WIN "Received-Quality averaging\n"
-#define HO_CFG_STR_POWER_BUDGET HO_CFG_STR_HANDOVER "Neighbor cell power 
triggering\n" "Neighbor cell power triggering\n"
+#define HO_CFG_STR_POWER_BUDGET "Neighbor cell power triggering\n" "Neighbor 
cell power triggering\n"
 #define HO_CFG_STR_AVG_COUNT "Number of values to average over\n"
 #define HO_CFG_STR_2 " (HO algo 2 only)\n"
 #define HO_CFG_STR_MIN "Minimum Level/Quality thresholds before triggering HO" 
HO_CFG_STR_2
@@ -68,151 +69,210 @@
  * VTY_WRITE_CONV: function name to convert struct value to VTY_WRITE_FMT, 
e.g. 'as_is'.
  * VTY_DOC: VTY documentation strings to match VTY_CMD and VTY_CMD_ARGs.
  */
-#define HO_CFG_ALL_MEMBERS \
+#define HO_GENERAL_CFG_ALL_MEMBERS \
\
HO_CFG_ONE_MEMBER(bool, ho_active, 0, \
"handover", "0|1", a2bool, "%d", bool2i, \
-   HO_CFG_STR_HANDOVER \
+   "Handover general config\n" \
"Disable in-call handover\n" \
"Enable in-call handover\n" \
"Enable/disable handover: ") \
\
HO_CFG_ONE_MEMBER(int, algorithm, 1, \
"handover algorithm", "1|2", atoi, "%d", as_is, \
-   HO_CFG_STR_HANDOVER \
+   "Handover general config\n" \
"Choose algorithm for handover decision\n" \
"Algorithm 1: trigger handover based on comparing current cell 
and neighbor RxLev and RxQual," \
" only.\n" \
"Algorithm 2: trigger handover on RxLev/RxQual, and also to 
balance the load across several" \
" cells. Consider available codecs. Prevent repeated handover 
by penalty timers.\n") \
+
+
+#define HODEC1_CFG_ALL_MEMBERS \
\
-   HO_CFG_ONE_MEMBER(unsigned int, rxlev_avg_win, 10, \
+   HO_CFG_ONE_MEMBER(unsigned int, hodec1_rxlev_avg_win, 10, \
"handover window rxlev averaging", "<1-10>", atoi, "%u", as_is, 
\
+   HO_CFG_STR_HANDOVER1 \
HO_CFG_STR_WIN_RXLEV \
"How many RxLev measurements are used for averaging\n" \
"RxLev averaging: " HO_CFG_STR_AVG_COUNT) \