from:"neels"

Change in ...osmo-bsc[master]: doc/manuals: review and tweak handover docs

2019-06-20 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14515 )

Change subject: doc/manuals: review and tweak handover docs
..

doc/manuals: review and tweak handover docs

Change-Id: Ib25cee8fd8c243881b99173a9a3036ad19d0f8af
---
M doc/manuals/chapters/handover.adoc
M doc/manuals/chapters/handover_inter_bsc.dot
M doc/manuals/chapters/handover_intra_bsc.dot
3 files changed, 62 insertions(+), 53 deletions(-)

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



diff --git a/doc/manuals/chapters/handover.adoc 
b/doc/manuals/chapters/handover.adoc
index c75b03c..2f9d598 100644
--- a/doc/manuals/chapters/handover.adoc
+++ b/doc/manuals/chapters/handover.adoc
@@ -22,9 +22,7 @@
 is currently not implemented.  However, you may still advertise 3G and 4G 
neighbor cells
 in order to facilitate cell/RAT re-selection to those neighbors.

-At the time of writing, OsmoMSC's inter-BSC handover support is not complete
-yet, so OsmoBSC can perform handover between separate BSS only in conjunction
-with a 3rd party MSC implementation.
+Since 2019, OsmoMSC fully supports both inter-BSC and inter-MSC handover.

 .Handover support in Osmocom at the time of writing
 [cols="^,^,^,^,^"]
@@ -34,6 +32,7 @@
 | OsmoMSC | (not involved, except for codec changes) | (planned)  | (planned)  
| -
 |

+Most handover related procedures are explained in 3GPP TS 48.008.

 === How Handover Works

@@ -45,9 +44,20 @@
 cells, its "neighbors". On the MS/BTS/BSS level, individual cells are
 identified by ARFCN+BSIC (frequency + 6-bit identification code).

-Each BTS is told by the BSC which cells identified by ARFCN+BSIC are its
-adjacent cells. Via System Information, each MS receives a list of these
-ARFCN+BSIC, and the MS then return measurements of reception levels.
+The BSC instructs each BTS with a list of ARFCNs (i.e. GSM frequency bands)
+that qualify as neighbor cells, as part of the System Information Type 2. Each
+MS served by a BTS receives the System Information Type 2 and thus knows which
+ARFCNs to measure for potential handover. Each MS with an active channel then
+returns up to 6 measurements of reception levels (RXLEV) to the BTS, to be
+forwarded to the BSC in RSL Measurement Report messages.
+
+Note that the BTS and MS are told only the ARFCNs, not the BSICs, of neighbor
+cells; the BSICs are however included in the measurements that an MS returns to
+BTS and BSC. Commonly, each ARFCN is owned by one specific operator, so, an MS
+considers all visible cells on a given ARFCN as possible neighbors. However, as
+soon as an MS reports RXLEV of a specific neighbor cell, the BSC needs to know
+which exact cell to possibly handover to, which is why the MS pinpoints the
+specific BSIC that it reported measurements for.

 The BSC is the point of decision whether to do handover or not. This can be a
 hugely complex combination of heuristics, knowledge of cell load and codec
@@ -74,18 +84,18 @@
 Should handover fail at any point, e.g. the new lchan never receives a RACH, or
 the MS reports a Handover Failure, then the new lchan is simply released again,
 and the old lchan remains in use. If the RTP stream has already been switched
-over to the new lchan, it may actually be switched back to the old lchan.
+over to the new lchan, it is switched back to the old lchan.

 This is simple enough if the new cell is managed by the same BSC: the OsmoMGW
 is simply instructed to relay the BTS-side of the RTP stream to another IP
 address and port, and the BSC continues to forward DTAP to the MSC
-transparently. The operation happens completely within the BSS. If the voice
-codec has remained unchanged, the MSC/MNCC may not even be notified that
-anything has happened at all.
+transparently. The operation happens completely within the BSS, except for the
+BSSMAP Handover Performed message sent to the MSC once the handover is
+completed (see 3GPP TS 48.008).

  External / Inter-BSC Handover

-If the adjacent target cell belongs to a different BSS, the RR procedure for
+If the handover target cell belongs to a different BSS, the RR procedure for
 handover remains the same, but we need to tell the _remote_ BSC to allocate the
 new lchan.

@@ -108,7 +118,7 @@
 The first part, identifying the remote BSC, is not as trivial as it sounds: as
 mentioned above, on the level of cell information seen by BTS and MS, the
 neighbor cells are identified by ARFCN+BSIC. However, on the A-interface and in
-the MSC, there is no knowledge of ARFCN+BSIC configurations, and instead each
+the MSC, there is no knowledge of ARFCN+BSIC configurations. Instead, each
 cell is identified by a LAC and CI (Location Area Code and Cell Identifier).

 NOTE: There are several different cell identification types on the A-interface:
@@ -116,15 +126,7 @@
 most of these (see

Change in ...osmo-bsc[master]: doc/manuals: review and tweak handover docs

2019-06-20 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14515 )

Change subject: doc/manuals: review and tweak handover docs
..


Patch Set 1: Code-Review+2

let me just apply this now (only doc)


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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ib25cee8fd8c243881b99173a9a3036ad19d0f8af
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Fri, 21 Jun 2019 00:07:33 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-bsc[master]: doc/manuals, vty doc: more handover doc clarifications

2019-06-24 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14582


Change subject: doc/manuals, vty doc: more handover doc clarifications
..

doc/manuals, vty doc: more handover doc clarifications

Related: OS#3487
Change-Id: I1639efb2dbcca4f0e9c33a74f3067606ce5f4209
---
M doc/manuals/chapters/handover.adoc
M include/osmocom/bsc/handover_cfg.h
M src/osmo-bsc/handover_vty.c
3 files changed, 25 insertions(+), 19 deletions(-)



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

diff --git a/doc/manuals/chapters/handover.adoc 
b/doc/manuals/chapters/handover.adoc
index 2f9d598..bb99751 100644
--- a/doc/manuals/chapters/handover.adoc
+++ b/doc/manuals/chapters/handover.adoc
@@ -434,7 +434,8 @@

 Configuration settings relevant for algorithm 1 start with `handover1`. For
 further details, please refer to the OsmoBSC VTY Reference
-(<>) or the telnet VTY online documentation.
+(<>) or the telnet VTY online documentation. See the
+`handover1` settings on the `config-net` and `config-net-bts` nodes.

  Handover Algorithm 2

@@ -451,7 +452,8 @@

 Configuration settings relevant for algorithm 2 start with `handover2`. For
 further details, please refer to the OsmoBSC VTY Reference
-<> or the telnet VTY online documentation.
+<> or the telnet VTY online documentation. See the `handover2`
+settings on the `config-net` and `config-net-bts` nodes.

 = Load Distribution

@@ -472,8 +474,8 @@
 adhere to minimum reception levels and distance, see `min rxlev` and `max
 distance`.

-Load distribution will take effect only for already established voice channels.
-An MS will always first establish a voice call with its current cell choice; in
+Load distribution will take effect only for already established channels.
+For example, an MS will always first establish a voice call with its current 
cell choice; in
 load situations, it might be moved to another cell shortly after that.
 Considering the best neighbor _before_ starting a new voice call might be
 desirable, but is currently not implemented. Consider that RXLEV/RXQUAL ratings
diff --git a/include/osmocom/bsc/handover_cfg.h 
b/include/osmocom/bsc/handover_cfg.h
index 92b5cd4..f174aad 100644
--- a/include/osmocom/bsc/handover_cfg.h
+++ b/include/osmocom/bsc/handover_cfg.h
@@ -96,22 +96,22 @@
"handover1 ", "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" \
+   "How many RxLev measurements to use for averaging\n" \
"RxLev averaging: " HO_CFG_STR_AVG_COUNT) \
\
HO_CFG_ONE_MEMBER(unsigned int, hodec1_rxqual_avg_win, 1, \
"handover1 ", "window rxqual averaging", "<1-10>", atoi, "%u", 
as_is, \
HO_CFG_STR_HANDOVER1 \
HO_CFG_STR_WIN_RXQUAL \
-   "How many RxQual measurements are used for averaging\n" \
+   "How many RxQual measurements to use for averaging\n" \
"RxQual averaging: " HO_CFG_STR_AVG_COUNT) \
\
HO_CFG_ONE_MEMBER(unsigned int, hodec1_rxlev_neigh_avg_win, 10, \
"handover1 ", "window rxlev neighbor averaging", "<1-10>", 
atoi, "%u", as_is, \
HO_CFG_STR_HANDOVER1 \
HO_CFG_STR_WIN_RXLEV \
-   "How many Neighbor RxLev measurements are used for averaging\n" 
\
-   "How many Neighbor RxLev measurements are used for averaging\n" 
\
+   "How many Neighbor RxLev measurements to use for averaging\n" \
+   "How many Neighbor RxLev measurements to use for averaging\n" \
"Neighbor RxLev averaging: " HO_CFG_STR_AVG_COUNT) \
\
HO_CFG_ONE_MEMBER(unsigned int, hodec1_pwr_interval, 6, \
@@ -142,22 +142,22 @@
"handover2 ", "window rxlev averaging", "<1-10>", atoi, "%u", 
as_is, \
HO_CFG_STR_HANDOVER2 \
HO_CFG_STR_WIN_RXLEV \
-   "How many RxLev measurements are used for averaging\n" \
+   "How many RxLev measurements to use for averaging\n" \
"RxLev averaging: " HO_CFG_STR_AVG_COUNT) \
\
HO_CFG_ONE_MEMBER(unsigned int, hodec2_rxqual_avg_win, 1, \
"handover2 ", "window rxqual averaging", "<1-10>", atoi, "%u", 
as_is, \
HO_CFG_STR_HANDOVER2 \
HO_CFG_STR_WIN

Change in ...osmo-bsc[master]: add vty 'no neighbors' to remove all HO targets

2019-07-12 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14768


Change subject: add vty 'no neighbors' to remove all HO targets
..

add vty 'no neighbors' to remove all HO targets

This is required for an upcoming TTCN3 test that plays through various neighbor
configurations.

Change-Id: I8623ab581639e9f8af6a9ff1eca990518d1b1211
---
M src/osmo-bsc/neighbor_ident_vty.c
M tests/neighbor_ident.vty
2 files changed, 93 insertions(+), 0 deletions(-)



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

diff --git a/src/osmo-bsc/neighbor_ident_vty.c 
b/src/osmo-bsc/neighbor_ident_vty.c
index 203b150..715ee8b 100644
--- a/src/osmo-bsc/neighbor_ident_vty.c
+++ b/src/osmo-bsc/neighbor_ident_vty.c
@@ -372,6 +372,85 @@
return CMD_SUCCESS;
 }

+struct nil_match_bts_data {
+   int bts_nr;
+   const struct neighbor_ident_key *found;
+};
+
+static bool nil_match_bts(const struct neighbor_ident_key *key,
+ const struct gsm0808_cell_id_list2 *val,
+ void *cb_data)
+{
+   struct nil_match_bts_data *d = cb_data;
+   if (key->from_bts == d->bts_nr) {
+   d->found = key;
+   return false;
+   }
+   return true;
+}
+
+static int del_all(struct vty *vty)
+{
+   int rc;
+   int removed = 0;
+   struct gsm_bts *bts = vty->index;
+
+   if (vty->node != BTS_NODE) {
+   vty_out(vty, "%% Error: cannot remove BTS neighbor, not on BTS 
node%s",
+   VTY_NEWLINE);
+   return CMD_WARNING;
+   }
+   if (!bts) {
+   vty_out(vty, "%% Error: cannot remove BTS neighbor, no BTS on 
this node%s",
+   VTY_NEWLINE);
+   return CMD_WARNING;
+   }
+
+   /* Remove all local neighbors */
+   while (1) {
+   struct gsm_bts_ref *neigh = 
llist_first_entry_or_null(>local_neighbors, struct gsm_bts_ref, entry);
+   struct gsm_bts *neigh_bts = neigh ? neigh->bts : NULL;
+   if (!neigh)
+   break;
+
+   rc = gsm_bts_local_neighbor_del(bts, neigh_bts);
+   if (rc > 0) {
+   vty_out(vty, "%% Removed local neighbor bts %u to bts 
%u%s",
+   bts->nr, neigh_bts->nr, VTY_NEWLINE);
+   removed += rc;
+   } else {
+   vty_out(vty, "%% Error while removing local neigbor bts 
%u to bts %u, aborted%s",
+   bts->nr, neigh_bts->nr, VTY_NEWLINE);
+   return CMD_WARNING;
+   }
+   }
+
+   /* Remove all remote-BSS neighbors */
+   while (1) {
+   struct neighbor_ident_key k;
+   struct nil_match_bts_data d = {};
+   neighbor_ident_iter(g_neighbor_cells, nil_match_bts, );
+   if (!d.found)
+   break;
+   k = *d.found;
+   if (neighbor_ident_del(g_neighbor_cells, )) {
+   vty_out(vty, "%% Removed remote BSS neighbor %s%s",
+   neighbor_ident_key_name(), VTY_NEWLINE);
+   removed++;
+   } else {
+   vty_out(vty, "%% Error while removing remote BSS 
neighbor %s, aborted%s",
+   neighbor_ident_key_name(), VTY_NEWLINE);
+   return CMD_WARNING;
+   }
+   }
+
+   if (!removed) {
+   vty_out(vty, "%% Cannot remove, no neighbors configured%s", 
VTY_NEWLINE);
+   return CMD_WARNING;
+   }
+   return CMD_SUCCESS;
+}
+
 DEFUN(cfg_neighbor_add_lac_arfcn_bsic, cfg_neighbor_add_lac_arfcn_bsic_cmd,
NEIGHBOR_ADD_CMD LAC_PARAMS " " NEIGHBOR_IDENT_VTY_KEY_PARAMS,
NEIGHBOR_ADD_DOC LAC_DOC NEIGHBOR_IDENT_VTY_KEY_DOC)
@@ -430,6 +509,15 @@
return del_by_key(vty, );
 }

+DEFUN(cfg_neighbor_del_all, cfg_neighbor_del_all_cmd,
+   "no neighbors",
+   NO_STR
+   "Remove all local and remote-BSS neighbor config for this cell."
+   " Note that this falls back to the legacy behavior of regarding all 
local cells as neighbors.\n")
+{
+   return del_all(vty);
+}
+
 struct write_neighbor_ident_entry_data {
struct vty *vty;
const char *indent;
@@ -576,5 +664,6 @@
install_element(BTS_NODE, _neighbor_add_cgi_arfcn_bsic_cmd);
install_element(BTS_NODE, _neighbor_del_bts_nr_cmd);
install_element(BTS_NODE, _neighbor_del_arfcn_bsic_cmd);
+   install_element(BTS_NODE, _neighbor_del_all_cmd);
install_element_ve(_bts_neighbor_cmd);
 }
diff --git a/tests/neighbor_ident.vty b/tests/neighbor_ident.vty
inde

Change in ...osmo-bsc[master]: comment and VTY doc tweaks

2019-07-12 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14766


Change subject: comment and VTY doc tweaks
..

comment and VTY doc tweaks

Clarify some in-code comments.

Fix descriptions of some handover timers, which still talked of "MO" and "MT"
handover -- which we now call "inter-BSC out" or "inter-BSC in" instead.

Change-Id: I8429a830edd0325893ac90f22fcc05309617bd2d
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/handover_fsm.c
M src/osmo-bsc/net_init.c
3 files changed, 6 insertions(+), 5 deletions(-)



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

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index a464001..d82d1ba 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -1505,7 +1505,7 @@
struct llist_head bts_list;
struct llist_head bts_rejected;

-   /* shall reference gsm_network_T[] */
+   /* see gsm_network_T_defs */
struct osmo_tdef *T_defs;

enum gsm_chan_t ctype_by_chreq[_NUM_CHREQ_T];
diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index 22618c5..7406a97 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -200,7 +200,8 @@
conn = req->old_lchan->conn;
OSMO_ASSERT(conn && conn->fi);

-   /* To make sure we're allowed to start a handover, go through a gscon 
event dispatch. */
+   /* To make sure we're allowed to start a handover, go through a gscon 
event dispatch. If that is accepted, the
+* same req is passed to handover_start(). */
osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_HANDOVER_START, req);
 }

diff --git a/src/osmo-bsc/net_init.c b/src/osmo-bsc/net_init.c
index 1ef9bd5..34403fa 100644
--- a/src/osmo-bsc/net_init.c
+++ b/src/osmo-bsc/net_init.c
@@ -26,10 +26,10 @@
 #include 

 static struct osmo_tdef gsm_network_T_defs[] = {
-   { .T=7, .default_val=10, .desc="inter-BSC Handover MO, HO Required to 
HO Command" },
-   { .T=8, .default_val=10, .desc="inter-BSC Handover MO, HO Command to 
final Clear" },
+   { .T=7, .default_val=10, .desc="inter-BSC/MSC Handover outgoing, BSSMAP 
HO Required to HO Command timeout" },
+   { .T=8, .default_val=10, .desc="inter-BSC/MSC Handover outgoing, BSSMAP 
HO Command to final Clear timeout" },
{ .T=10, .default_val=6, .desc="RR Assignment" },
-   { .T=101, .default_val=10, .desc="inter-BSC Handover MT, HO Request to 
HO Accept" },
+   { .T=101, .default_val=10, .desc="inter-BSC/MSC Handover incoming, 
BSSMAP HO Request to HO Accept" },
{ .T=3101, .default_val=3, .desc="RR Immediate Assignment" },
{ .T=3103, .default_val=5, .desc="Handover" },
{ .T=3105, .default_val=100, .unit=OSMO_TDEF_MS, .desc="Physical 
Information" },

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I8429a830edd0325893ac90f22fcc05309617bd2d
Gerrit-Change-Number: 14766
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-bsc[master]: silence error for "invalid enum handover_scope value: none"

2019-07-12 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14767


Change subject: silence error for "invalid enum handover_scope value: none"
..

silence error for "invalid enum handover_scope value: none"

If no target cell got selected in a handover attempt, enum value NO_HANDOVER is
used. In that case, do not log a lot of errors saying
"invalid enum handover_scope value: none" -- they are misleading.

Change-Id: I98e748bea58ebb02812b6aaa6431c7d4b813242d
---
M src/osmo-bsc/handover_fsm.c
1 file changed, 1 insertion(+), 0 deletions(-)



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

diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index 7406a97..6d0c2d4 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -694,6 +694,7 @@
LOGP(DHO, LOGL_ERROR, "invalid enum handover_scope value: %s\n",
 handover_scope_name(scope));
/* use "normal" HO_INTRA_BSC counter... */
+   case HO_NO_HANDOVER:
case HO_INTRA_BSC:
return result_counter_HANDOVER(result);
case HO_INTER_BSC_OUT:

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I98e748bea58ebb02812b6aaa6431c7d4b813242d
Gerrit-Change-Number: 14767
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-bsc[master]: neighbor config: allow re-using ARFCN+BSIC pairs

2019-07-12 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14769


Change subject: neighbor config: allow re-using ARFCN+BSIC pairs
..

neighbor config: allow re-using ARFCN+BSIC pairs

Fix neighbor config to match OsmoBSC manual: implement the plan for neighbor
configuration that was so far only described in the manual without actually
being in operation.

This first allows re-using ARFCN+BSIC pairs in and across BSS.

So far the handover_start() code always looked for handover target cells across
*all* local cells, even if they were not listed as neighbors to a source cell.
Imply all cells as neighbors only as long as there are no explicit neighbors
configured. As soon as the first 'neighbor' line appears in a 'bts' config,
only the listed neighbors are regarded as handover target cells. (The
'neighbor-list' commands are not related to this, only the relatively new
'neighbor (bts|lac|cgi|...)' commands affect actual handover procedures.)

TTCN3 tests TC_ho_neighbor_config_1 thru _7 play through the various aspects of
neighbor configuration: both the legacy implicit all-cells-are-neighbors as
well as allowing only explicit neighbors by config.

Related: OS#4056
Related: osmo-ttcn3-hacks Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
Change-Id: I29bca59ab232eddc74e0d4698efb9c9992443983
---
M include/osmocom/bsc/handover.h
M include/osmocom/bsc/handover_fsm.h
M include/osmocom/bsc/neighbor_ident.h
M src/osmo-bsc/handover_decision_2.c
M src/osmo-bsc/handover_fsm.c
M src/osmo-bsc/handover_logic.c
M src/osmo-bsc/neighbor_ident_vty.c
M tests/bsc/bsc_test.c
8 files changed, 244 insertions(+), 62 deletions(-)



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

diff --git a/include/osmocom/bsc/handover.h b/include/osmocom/bsc/handover.h
index 322913d..b00ee60 100644
--- a/include/osmocom/bsc/handover.h
+++ b/include/osmocom/bsc/handover.h
@@ -10,6 +10,15 @@
 #include 
 #include 

+#define LOG_HO(conn, level, fmt, args...) do { \
+   if (conn->ho.fi) \
+   LOGPFSML(conn->ho.fi, level, "%s: " fmt, \
+handover_status(conn), ## args); \
+   else \
+   LOGP(DHODEC, level, "%s: " fmt, \
+handover_status(conn), ## args); \
+   } while(0)
+
 struct gsm_network;
 struct gsm_lchan;
 struct gsm_bts;
@@ -25,6 +34,8 @@
HO_RESULT_ERROR,
 };

+const char *handover_status(struct gsm_subscriber_connection *conn);
+
 extern const struct value_string handover_result_names[];
 inline static const char *handover_result_name(enum handover_result val)
 { return get_value_string(handover_result_names, val); }
@@ -70,8 +81,11 @@
   struct gsm_lchan *lchan);
 void bsc_tx_bssmap_ho_failure(struct gsm_subscriber_connection *conn);

-struct gsm_bts *bts_by_neighbor_ident(const struct gsm_network *net,
- const struct neighbor_ident_key 
*search_for);
+int find_handover_target_cell(struct gsm_bts **local_target_cell_p,
+ const struct gsm0808_cell_id_list2 
**remote_target_cell_p,
+ struct gsm_subscriber_connection *conn, const 
struct neighbor_ident_key *search_for,
+ bool log_errors);
+
 struct neighbor_ident_key *bts_ident_key(const struct gsm_bts *bts);

 void handover_parse_inter_bsc_mt(struct gsm_subscriber_connection *conn,
diff --git a/include/osmocom/bsc/handover_fsm.h 
b/include/osmocom/bsc/handover_fsm.h
index 7c2145e..1628d8f 100644
--- a/include/osmocom/bsc/handover_fsm.h
+++ b/include/osmocom/bsc/handover_fsm.h
@@ -4,18 +4,6 @@
 #include 
 #include 

-const char *handover_status(struct gsm_subscriber_connection *conn);
-
-/* This macro automatically includes a final \n, if omitted. */
-#define LOG_HO(conn, level, fmt, args...) do { \
-   if (conn->ho.fi) \
-   LOGPFSML(conn->ho.fi, level, "%s: " fmt, \
-handover_status(conn), ## args); \
-   else \
-   LOGP(DHODEC, level, "%s: " fmt, \
-handover_status(conn), ## args); \
-   } while(0)
-
 /* Terminology:
  * Intra-Cell: stays within one BTS, this should actually be an Assignment.
  * Intra-BSC: stays within one BSC, but moves between BTSes.
diff --git a/include/osmocom/bsc/neighbor_ident.h 
b/include/osmocom/bsc/neighbor_ident.h
index 17bffbc..aa38276 100644
--- a/include/osmocom/bsc/neighbor_ident.h
+++ b/include/osmocom/bsc/neighbor_ident.h
@@ -47,6 +47,8 @@
 void neighbor_ident_vty_init(struct gsm_network *net, struct 
neighbor_ident_list *nil);
 void neighbor_ident_vty_write(struct vty *vty, const char *indent, struct 
gsm_bts *bts);

+bool neighbor_ident_bts_entry_exists(uint8_t from_bts);
+
 #define NEIGHBOR_IDENT_VTY_KEY_PARAMS "arfcn <0-1023> bsic (<0-63>|any)&quo

Change in ...osmo-ttcn3-hacks[master]: bsc: add TC_ho_neighbor_config_1 thru _7

2019-07-12 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/14765


Change subject: bsc: add TC_ho_neighbor_config_1 thru _7
..

bsc: add TC_ho_neighbor_config_1 thru _7

Add tests to play through various neighbor configurations.
Tests will pass as soon as osmo-bsc I29bca59ab232eddc74e0d4698efb9c9992443983
is merged.

Add RSL2 to allow triggering handover to BTS 2.

Adjust osmo-bsc.cfg to match the new tests. Also applied in docker-playground
I1c57a04747f5ec004ccf4657954dcb0b003c24fc.
- Actually enable handover.
- Add bts 3

Depends: osmo-bsc I8623ab581639e9f8af6a9ff1eca990518d1b1211 ('no neighbors')
Related: OS#4056
Change-Id: Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
---
M bsc/BSC_Tests.ttcn
M bsc/BSC_Tests_LCLS.ttcn
M bsc/osmo-bsc.cfg
M library/RSL_Emulation.ttcn
4 files changed, 395 insertions(+), 171 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks 
refs/changes/65/14765/1

diff --git a/bsc/BSC_Tests.ttcn b/bsc/BSC_Tests.ttcn
index 44a9172..adffdf9 100644
--- a/bsc/BSC_Tests.ttcn
+++ b/bsc/BSC_Tests.ttcn
@@ -1719,6 +1719,10 @@
connect(vc_conn:RSL1, bts[1].rsl.vc_RSL:CLIENT_PT);
connect(vc_conn:RSL1_PROC, bts[1].rsl.vc_RSL:RSL_PROC);
}
+   if (isvalue(bts[2])) {
+   connect(vc_conn:RSL2, bts[2].rsl.vc_RSL:CLIENT_PT);
+   connect(vc_conn:RSL2_PROC, bts[2].rsl.vc_RSL:RSL_PROC);
+   }
connect(vc_conn:BSSAP, g_bssap.vc_RAN:CLIENT);
connect(vc_conn:MGCP, vc_MGCP:MGCP_CLIENT);
connect(vc_conn:MGCP_MULTI, vc_MGCP:MGCP_CLIENT_MULTI);
@@ -2837,6 +2841,11 @@
f_vty_transceive(BSCVTY, cmd & suffix);
 }

+/* Even though the VTY command to trigger handover takes a new BTS number as 
argument, behind the scenes osmo-bsc always
+ * translates that to a target ARFCN+BSIC first. See bsc_vty.c 
trigger_ho_or_as(), which puts the selected BTS' neighbor
+ * ident key (ARFCN + BSIC) in the struct passed on to handover_request(). 
handover_start() then resolves that to a
+ * viable actual neighbor cell. So from the internal osmo-bsc perspective, we 
always request handover to an ARFCN + BSIC
+ * pair, not really to a specific BTS number. */
 private function f_vty_handover(integer bts_nr, integer trx_nr, RslChannelNr 
chan_nr,
integer new_bts_nr)
 runs on MSC_ConnHdlr {
@@ -3587,6 +3596,353 @@
vc_conn.done;
 }

+type record of charstring Commands;
+
+private function f_bts_0_cfg(Commands cmds := {}) runs on MSC_ConnHdlr
+{
+   f_vty_enter_cfg_bts(BSCVTY, 0);
+   for (var integer i := 0; i < sizeof(cmds); i := i+1) {
+   f_vty_transceive(BSCVTY, cmds[i]);
+   }
+   f_vty_transceive(BSCVTY, "end");
+}
+
+private function f_probe_for_handover(charstring log_label,
+ charstring log_descr,
+ charstring handover_vty_cmd,
+ boolean expect_handover,
+ boolean is_inter_bsc_handover := false)
+runs on MSC_ConnHdlr
+{
+   var RSL_Message rsl;
+
+   var charstring log_msg := " (expecting handover)"
+   if (not expect_handover) {
+   log_msg := " (expecting NO handover)";
+   }
+   log("f_probe_for_handover starting: " & log_label & ": " & log_descr & 
log_msg);
+   f_vty_transceive(BSCVTY, handover_vty_cmd);
+
+   /* We're going to thwart any and all handover attempts, just be ready 
to handle (and ignore) handover target
+* lchans to be established on bts 1 or bts 2. */
+   f_rslem_suspend(RSL1_PROC);
+   f_rslem_suspend(RSL2_PROC);
+
+   timer T := 2.0;
+   T.start;
+
+   alt {
+   [] RSL.receive(tr_RSL_DATA_REQ(g_chan_nr)) -> value rsl {
+   var PDU_ML3_NW_MS l3 := 
dec_PDU_ML3_NW_MS(rsl.ies[2].body.l3_info.payload);
+   log("Rx L3 from net: ", l3);
+   if (ischosen(l3.msgs.rrm.handoverCommand)) {
+   var RslChannelNr new_chan_nr;
+   var GsmArfcn arfcn;
+   
f_ChDesc2RslChanNr(l3.msgs.rrm.handoverCommand.channelDescription2,
+  new_chan_nr, arfcn);
+   log("Handover to new chan ", new_chan_nr, " on ARFCN ", 
arfcn);
+   log(l3.msgs.rrm.handoverCommand);
+
+   /* Need to register for new lchan on new BTS -- it's 
either bts 1 or bts 2.  It doesn't really
+* matter on which BTS it really is, we're not going to 
follow through an entire handover
+* anyway. */
+   f_rslem_register(0, new_chan_nr, RSL1_PROC);
+

Change in ...docker-playground[master]: adjust osmo-bsc.cfg for TC_ho_neighbor_config_1 thru _7

2019-07-12 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/docker-playground/+/14764


Change subject: adjust osmo-bsc.cfg for TC_ho_neighbor_config_1 thru _7
..

adjust osmo-bsc.cfg for TC_ho_neighbor_config_1 thru _7

- Actually enable handover.
- Add bts 3

Related: osmo-ttcn3-hacks Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
Change-Id: I1c57a04747f5ec004ccf4657954dcb0b003c24fc
---
M ttcn3-bsc-test/osmo-bsc.cfg
1 file changed, 30 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.osmocom.org:29418/docker-playground 
refs/changes/64/14764/1

diff --git a/ttcn3-bsc-test/osmo-bsc.cfg b/ttcn3-bsc-test/osmo-bsc.cfg
index 3d226e9..6cfaf7c 100644
--- a/ttcn3-bsc-test/osmo-bsc.cfg
+++ b/ttcn3-bsc-test/osmo-bsc.cfg
@@ -66,7 +66,7 @@
  encryption a5 0 1 3
  neci 1
  paging any use tch 0
- handover 0
+ handover 1
  handover window rxlev averaging 10
  handover window rxqual averaging 1
  handover window rxlev neighbor averaging 10
@@ -350,6 +350,35 @@
timeslot 7
 phys_chan_config PDCH
 hopping enabled 0
+ bts 3
+  type sysmobts
+  band DCS1800
+  cell_identity 3
+  location_area_code 3
+  # re-use bts 2's ARFCN 871 and BSIC 12 (to test handover config)
+  base_station_id_code 12
+  trx 0
+   rf_locked 0
+   arfcn 871
+   nominal power 23
+   max_power_red 20
+   rsl e1 tei 0
+   timeslot 0
+phys_chan_config CCCH+SDCCH4
+   timeslot 1
+phys_chan_config TCH/F
+   timeslot 2
+phys_chan_config TCH/F
+   timeslot 3
+phys_chan_config TCH/F
+   timeslot 4
+phys_chan_config TCH/F
+   timeslot 5
+phys_chan_config TCH/H
+   timeslot 6
+phys_chan_config PDCH
+   timeslot 7
+phys_chan_config PDCH
 msc 0
  ip.access rtp-base 4000
  no bsc-welcome-text

--
To view, visit https://gerrit.osmocom.org/c/docker-playground/+/14764
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Change-Id: I1c57a04747f5ec004ccf4657954dcb0b003c24fc
Gerrit-Change-Number: 14764
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-ttcn3-hacks[master]: bsc: add TC_ho_neighbor_config_1 thru _7

2019-07-12 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/14765

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

Change subject: bsc: add TC_ho_neighbor_config_1 thru _7
..

bsc: add TC_ho_neighbor_config_1 thru _7

Add tests to play through various neighbor configurations.
Tests will pass as soon as osmo-bsc I29bca59ab232eddc74e0d4698efb9c9992443983
is merged.

Add RSL2 to allow triggering handover to BTS 2.

Adjust osmo-bsc.cfg to match the new tests. Also applied in docker-playground
I1c57a04747f5ec004ccf4657954dcb0b003c24fc.
- Actually enable handover.
- Add bts 3

Depends: osmo-bsc I8623ab581639e9f8af6a9ff1eca990518d1b1211 ('no neighbors')
Related: OS#4056
Change-Id: Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
---
M bsc/BSC_Tests.ttcn
M bsc/osmo-bsc.cfg
M library/RSL_Emulation.ttcn
3 files changed, 396 insertions(+), 1 deletion(-)


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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
Gerrit-Change-Number: 14765
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset

Change in ...osmo-bsc[master]: doc/manuals, vty doc: more handover doc clarifications

2019-07-09 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14582 )

Change subject: doc/manuals, vty doc: more handover doc clarifications
..

doc/manuals, vty doc: more handover doc clarifications

Related: OS#3487
Change-Id: I1639efb2dbcca4f0e9c33a74f3067606ce5f4209
---
M doc/manuals/chapters/handover.adoc
M include/osmocom/bsc/handover_cfg.h
M src/osmo-bsc/handover_vty.c
M tests/handover_cfg.vty
4 files changed, 55 insertions(+), 49 deletions(-)

Approvals:
  laforge: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/doc/manuals/chapters/handover.adoc 
b/doc/manuals/chapters/handover.adoc
index 2f9d598..bb99751 100644
--- a/doc/manuals/chapters/handover.adoc
+++ b/doc/manuals/chapters/handover.adoc
@@ -434,7 +434,8 @@

 Configuration settings relevant for algorithm 1 start with `handover1`. For
 further details, please refer to the OsmoBSC VTY Reference
-(<>) or the telnet VTY online documentation.
+(<>) or the telnet VTY online documentation. See the
+`handover1` settings on the `config-net` and `config-net-bts` nodes.

  Handover Algorithm 2

@@ -451,7 +452,8 @@

 Configuration settings relevant for algorithm 2 start with `handover2`. For
 further details, please refer to the OsmoBSC VTY Reference
-<> or the telnet VTY online documentation.
+<> or the telnet VTY online documentation. See the `handover2`
+settings on the `config-net` and `config-net-bts` nodes.

 = Load Distribution

@@ -472,8 +474,8 @@
 adhere to minimum reception levels and distance, see `min rxlev` and `max
 distance`.

-Load distribution will take effect only for already established voice channels.
-An MS will always first establish a voice call with its current cell choice; in
+Load distribution will take effect only for already established channels.
+For example, an MS will always first establish a voice call with its current 
cell choice; in
 load situations, it might be moved to another cell shortly after that.
 Considering the best neighbor _before_ starting a new voice call might be
 desirable, but is currently not implemented. Consider that RXLEV/RXQUAL ratings
diff --git a/include/osmocom/bsc/handover_cfg.h 
b/include/osmocom/bsc/handover_cfg.h
index 92b5cd4..f174aad 100644
--- a/include/osmocom/bsc/handover_cfg.h
+++ b/include/osmocom/bsc/handover_cfg.h
@@ -96,22 +96,22 @@
"handover1 ", "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" \
+   "How many RxLev measurements to use for averaging\n" \
"RxLev averaging: " HO_CFG_STR_AVG_COUNT) \
\
HO_CFG_ONE_MEMBER(unsigned int, hodec1_rxqual_avg_win, 1, \
"handover1 ", "window rxqual averaging", "<1-10>", atoi, "%u", 
as_is, \
HO_CFG_STR_HANDOVER1 \
HO_CFG_STR_WIN_RXQUAL \
-   "How many RxQual measurements are used for averaging\n" \
+   "How many RxQual measurements to use for averaging\n" \
"RxQual averaging: " HO_CFG_STR_AVG_COUNT) \
\
HO_CFG_ONE_MEMBER(unsigned int, hodec1_rxlev_neigh_avg_win, 10, \
"handover1 ", "window rxlev neighbor averaging", "<1-10>", 
atoi, "%u", as_is, \
HO_CFG_STR_HANDOVER1 \
HO_CFG_STR_WIN_RXLEV \
-   "How many Neighbor RxLev measurements are used for averaging\n" 
\
-   "How many Neighbor RxLev measurements are used for averaging\n" 
\
+   "How many Neighbor RxLev measurements to use for averaging\n" \
+   "How many Neighbor RxLev measurements to use for averaging\n" \
"Neighbor RxLev averaging: " HO_CFG_STR_AVG_COUNT) \
\
HO_CFG_ONE_MEMBER(unsigned int, hodec1_pwr_interval, 6, \
@@ -142,22 +142,22 @@
"handover2 ", "window rxlev averaging", "<1-10>", atoi, "%u", 
as_is, \
HO_CFG_STR_HANDOVER2 \
HO_CFG_STR_WIN_RXLEV \
-   "How many RxLev measurements are used for averaging\n" \
+   "How many RxLev measurements to use for averaging\n" \
"RxLev averaging: " HO_CFG_STR_AVG_COUNT) \
\
HO_CFG_ONE_MEMBER(unsigned int, hodec2_rxqual_avg_win, 1, \
"handover2 ", "window rxqual averaging", "<1-10>", atoi, "%u", 
as_is, \
HO_CFG_STR_HANDOVER2 \
HO_CFG

Change in ...osmo-python-tests[master]: tweak README

2019-07-09 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/python/osmo-python-tests/+/14698 )

Change subject: tweak README
..

tweak README

Most importantly, mention 'python3 setup.py'.

Tweak indenting and some wording.

Change-Id: Id8c277de280b54d04edcafa77ed93017d6da473d
---
M README
1 file changed, 15 insertions(+), 5 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/README b/README
index 69eb764..dbb8d88 100644
--- a/README
+++ b/README
@@ -1,11 +1,20 @@
 Building/installation:
-sudo python setup.py install
+
+   sudo python setup.py install
+   sudo python3 setup.py install
+
 If you prefer to have it cleanly removable, install checkinstall and run
-sudo checkinstall python setup.py install
-Alternatively, just run 'pip install --user -e ./' or 'pip3 install --user -e 
./'
+
+   sudo checkinstall python setup.py install
+
+Alternatively, just run
+
+   pip install --user -e ./
+   pip3 install --user -e ./
+
 depending on your python version.

-Use
+Use:
 There are currently following scripts in this package:
 osmotestconfig.py - test that apps start/write with example configs
 soap.py - implementation of SOAP <-> Ctrl proxy implemented on top of Twisted 
(deprecated, unmaintained)
@@ -20,8 +29,9 @@
 osmodumpdoc.py - dump documentation, superseded by osmo_interact_vty.py -X
 osmotestvty.py - test vty operations, superseded by 
osmo_verify_transcript_vty.py

-Each of these scripts imports a project-specific osmoappdesc.py,
+Some of these scripts import a project-specific osmoappdesc.py,
 which provides information about the available apps, configs, vty ports, etc.
+and is provided by other source trees (like osmo-bsc.git, osmo-msc.git, ...)

 Run the scripts with osmoappdesc.py in the current directory (preferred)
 or with -p .

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

Gerrit-Project: python/osmo-python-tests
Gerrit-Branch: master
Gerrit-Change-Id: Id8c277de280b54d04edcafa77ed93017d6da473d
Gerrit-Change-Number: 14698
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-MessageType: merged

Change in ...osmo-bsc[master]: remove double BSSMAP Clear on HO failure

2019-07-09 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14700 )

Change subject: remove double BSSMAP Clear on HO failure
..


Patch Set 1: Code-Review+2

combining 2 +1s from others


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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Iac1519eb8b24e8523caec682f9ac8e6dcf1327ce
Gerrit-Change-Number: 14700
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Tue, 09 Jul 2019 15:45:32 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-bsc[master]: remove double BSSMAP Clear on HO failure

2019-07-09 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14700 )

Change subject: remove double BSSMAP Clear on HO failure
..

remove double BSSMAP Clear on HO failure

If a handover fails when the new lchan is already fully established, osmo-bsc
so far caused two BSSMAP Clear Requests to be sent out to the MSC: one caused
by detaching the lchan from the gscon, one from returning the gscon back to
ST_ACTIVE, which detects that no lchan is present and Clears. In fact only one
of those is necessary.

Checking for the presence of an lchan when entering ST_ACTIVE is an earlier
attempt to catch insane situations. Since then, osmo-bsc has acquired other
logic that will ensure sending a Clear Request in all cases, see
gscon_forget_lchan(). Sending another BSSMAP Clear Request in ST_ACTIVE's
onenter is simply not necessary. Drop gscon_fsm_active_onenter() entirely.

Note: the double Clear Request is currently hit by
TC_ho_out_fail_no_ho_detect(), which currently fails and will pass again after
this patch; however, osmo-bsc should actually not release the lchan at all
during this test, see OS#4093. In other words, osmo-bsc behavior for this
scenario as well as TC_ho_out_fail_no_ho_detect() need to be changed, and the
test will, once fixed, not be useful to trigger this issue anymore.

Related: OS#4078
Change-Id: Iac1519eb8b24e8523caec682f9ac8e6dcf1327ce
---
M src/osmo-bsc/bsc_subscr_conn_fsm.c
1 file changed, 0 insertions(+), 8 deletions(-)

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



diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c 
b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index bc5cb27..f8784f9 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -343,13 +343,6 @@
}
 }

-static void gscon_fsm_active_onenter(struct osmo_fsm_inst *fi, uint32_t 
prev_state)
-{
-   struct gsm_subscriber_connection *conn = fi->priv;
-   if (!conn->lchan)
-   gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE);
-}
-
 /* We're on an active subscriber connection, passing DTAP back and forth */
 static void gscon_fsm_active(struct osmo_fsm_inst *fi, uint32_t event, void 
*data)
 {
@@ -605,7 +598,6 @@
 S(GSCON_EV_HANDOVER_START),
.out_state_mask = S(ST_CLEARING) | S(ST_ASSIGNMENT) |
  S(ST_HANDOVER),
-   .onenter = gscon_fsm_active_onenter,
.action = gscon_fsm_active,
},
[ST_ASSIGNMENT] = {

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Iac1519eb8b24e8523caec682f9ac8e6dcf1327ce
Gerrit-Change-Number: 14700
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: osmith 
Gerrit-MessageType: merged

Change in ...osmo-python-tests[master]: tweak README

2019-07-08 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/python/osmo-python-tests/+/14698


Change subject: tweak README
..

tweak README

Most importantly, mention 'python3 setup.py'.

Tweak indenting and some wording.

Change-Id: Id8c277de280b54d04edcafa77ed93017d6da473d
---
M README
1 file changed, 15 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/python/osmo-python-tests 
refs/changes/98/14698/1

diff --git a/README b/README
index 69eb764..dbb8d88 100644
--- a/README
+++ b/README
@@ -1,11 +1,20 @@
 Building/installation:
-sudo python setup.py install
+
+   sudo python setup.py install
+   sudo python3 setup.py install
+
 If you prefer to have it cleanly removable, install checkinstall and run
-sudo checkinstall python setup.py install
-Alternatively, just run 'pip install --user -e ./' or 'pip3 install --user -e 
./'
+
+   sudo checkinstall python setup.py install
+
+Alternatively, just run
+
+   pip install --user -e ./
+   pip3 install --user -e ./
+
 depending on your python version.

-Use
+Use:
 There are currently following scripts in this package:
 osmotestconfig.py - test that apps start/write with example configs
 soap.py - implementation of SOAP <-> Ctrl proxy implemented on top of Twisted 
(deprecated, unmaintained)
@@ -20,8 +29,9 @@
 osmodumpdoc.py - dump documentation, superseded by osmo_interact_vty.py -X
 osmotestvty.py - test vty operations, superseded by 
osmo_verify_transcript_vty.py

-Each of these scripts imports a project-specific osmoappdesc.py,
+Some of these scripts import a project-specific osmoappdesc.py,
 which provides information about the available apps, configs, vty ports, etc.
+and is provided by other source trees (like osmo-bsc.git, osmo-msc.git, ...)

 Run the scripts with osmoappdesc.py in the current directory (preferred)
 or with -p .

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

Gerrit-Project: python/osmo-python-tests
Gerrit-Branch: master
Gerrit-Change-Id: Id8c277de280b54d04edcafa77ed93017d6da473d
Gerrit-Change-Number: 14698
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-bsc[master]: remove double BSSMAP Clear on HO failure

2019-07-08 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14700


Change subject: remove double BSSMAP Clear on HO failure
..

remove double BSSMAP Clear on HO failure

If a handover fails when the new lchan is already fully established, osmo-bsc
so far caused two BSSMAP Clear Requests to be sent out to the MSC: one caused
by detaching the lchan from the gscon, one from returning the gscon back to
ST_ACTIVE, which detects that no lchan is present and Clears. In fact only one
of those is necessary.

Checking for the presence of an lchan when entering ST_ACTIVE is an earlier
attempt to catch insane situations. Since then, osmo-bsc has acquired other
logic that will ensure sending a Clear Request in all cases, see
gscon_forget_lchan(). Sending another BSSMAP Clear Request in ST_ACTIVE's
onenter is simply not necessary. Drop gscon_fsm_active_onenter() entirely.

Note: the double Clear Request is currently hit by
TC_ho_out_fail_no_ho_detect(), which currently fails and will pass again after
this patch; however, osmo-bsc should actually not release the lchan at all
during this test, see OS#4093. In other words, osmo-bsc behavior for this
scenario as well as TC_ho_out_fail_no_ho_detect() need to be changed, and the
test will, once fixed, not be useful to trigger this issue anymore.

Related: OS#4078
Change-Id: Iac1519eb8b24e8523caec682f9ac8e6dcf1327ce
---
M src/osmo-bsc/bsc_subscr_conn_fsm.c
1 file changed, 0 insertions(+), 8 deletions(-)



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

diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c 
b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index bc5cb27..f8784f9 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -343,13 +343,6 @@
}
 }

-static void gscon_fsm_active_onenter(struct osmo_fsm_inst *fi, uint32_t 
prev_state)
-{
-   struct gsm_subscriber_connection *conn = fi->priv;
-   if (!conn->lchan)
-   gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE);
-}
-
 /* We're on an active subscriber connection, passing DTAP back and forth */
 static void gscon_fsm_active(struct osmo_fsm_inst *fi, uint32_t event, void 
*data)
 {
@@ -605,7 +598,6 @@
 S(GSCON_EV_HANDOVER_START),
.out_state_mask = S(ST_CLEARING) | S(ST_ASSIGNMENT) |
  S(ST_HANDOVER),
-   .onenter = gscon_fsm_active_onenter,
.action = gscon_fsm_active,
},
[ST_ASSIGNMENT] = {

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Iac1519eb8b24e8523caec682f9ac8e6dcf1327ce
Gerrit-Change-Number: 14700
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-bsc[master]: make bsc_clear_request() static

2019-07-08 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14699


Change subject: make bsc_clear_request() static
..

make bsc_clear_request() static

bsc_clear_request() is in fact used only within gsm_08_08.c, make it static to
that file.

Since the gscon FSM, "real" BSSMAP Clear are sent only by gscon_bssmap_clear().

bsc_clear_request() remains in use for legacy code paths in gsm_08_08.c:
- the bsc_filter, i.e. for IMSI filtering;
- in move_to_msc(), from handle_cc_setup(), a code path that is in fact not
  entirely clear to me. It seems to be an old functionality to serve multiple
  MSCs?

Both of which I personally haven't seen in use, are not tested and should
probably be completely removed.

For now contain legacy code in the static context.

Adjust comment.

Change-Id: Ic89d0afad42e4b11183a13d2dc6b7bbf0b822fd9
---
M include/osmocom/bsc/gsm_08_08.h
M src/osmo-bsc/gsm_08_08.c
M tests/bsc/bsc_test.c
M tests/handover/handover_test.c
4 files changed, 4 insertions(+), 7 deletions(-)



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

diff --git a/include/osmocom/bsc/gsm_08_08.h b/include/osmocom/bsc/gsm_08_08.h
index 5241295..b46a8d3 100644
--- a/include/osmocom/bsc/gsm_08_08.h
+++ b/include/osmocom/bsc/gsm_08_08.h
@@ -10,7 +10,6 @@
 void bsc_cipher_mode_compl(struct gsm_subscriber_connection *conn, struct msgb 
*msg, uint8_t chosen_encr);
 int bsc_compl_l3(struct gsm_subscriber_connection *conn, struct msgb *msg, 
uint16_t chosen_channel);
 void bsc_dtap(struct gsm_subscriber_connection *conn, uint8_t link_id, struct 
msgb *msg);
-int bsc_clear_request(struct gsm_subscriber_connection *conn, uint32_t cause);
 void bsc_cm_update(struct gsm_subscriber_connection *conn,
   const uint8_t *cm2, uint8_t cm2_len,
   const uint8_t *cm3, uint8_t cm3_len);
diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c
index 2c6a689..6ca5455 100644
--- a/src/osmo-bsc/gsm_08_08.c
+++ b/src/osmo-bsc/gsm_08_08.c
@@ -509,6 +509,8 @@
return false;
 }

+static int bsc_clear_request(struct gsm_subscriber_connection *conn, uint32_t 
cause);
+
 /*
  * Plastic surgery... we want to give up the current connection
  */
@@ -635,8 +637,8 @@
return;
 }

-/*! BSC->MSC: RR conn has been cleared. */
-int bsc_clear_request(struct gsm_subscriber_connection *conn, uint32_t cause)
+/*! BSSMAP Clear Request for legacy code paths, instead see 
gscon_bssmap_clear(). */
+static int bsc_clear_request(struct gsm_subscriber_connection *conn, uint32_t 
cause)
 {
int rc;
struct msgb *resp;
diff --git a/tests/bsc/bsc_test.c b/tests/bsc/bsc_test.c
index 8e88ba8..492f0c5 100644
--- a/tests/bsc/bsc_test.c
+++ b/tests/bsc/bsc_test.c
@@ -243,8 +243,6 @@
 void bsc_dtap(struct gsm_subscriber_connection *conn, uint8_t link_id, struct 
msgb *msg) {}
 void bsc_assign_compl(struct gsm_subscriber_connection *conn, uint8_t 
rr_cause) {}
 void bsc_assign_fail(struct gsm_subscriber_connection *conn, uint8_t cause, 
uint8_t *rr_cause) {}
-int bsc_clear_request(struct gsm_subscriber_connection *conn, uint32_t cause)
-{ return 0; }
 void bsc_cm_update(struct gsm_subscriber_connection *conn,
   const uint8_t *cm2, uint8_t cm2_len,
   const uint8_t *cm3, uint8_t cm3_len) {}
diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c
index a8a77be..bedf6f9 100644
--- a/tests/handover/handover_test.c
+++ b/tests/handover/handover_test.c
@@ -1779,8 +1779,6 @@
 { return 0; }
 void bsc_dtap(struct gsm_subscriber_connection *conn, uint8_t link_id, struct 
msgb *msg) {}
 void bsc_assign_compl(struct gsm_subscriber_connection *conn, uint8_t 
rr_cause) {}
-int bsc_clear_request(struct gsm_subscriber_connection *conn, uint32_t cause)
-{ return 0; }
 void bsc_cm_update(struct gsm_subscriber_connection *conn,
   const uint8_t *cm2, uint8_t cm2_len,
   const uint8_t *cm3, uint8_t cm3_len) {}

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ic89d0afad42e4b11183a13d2dc6b7bbf0b822fd9
Gerrit-Change-Number: 14699
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-bsc[master]: doc/manuals, vty doc: more handover doc clarifications

2019-07-08 Thread neels

Hello laforge, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-bsc/+/14582

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

Change subject: doc/manuals, vty doc: more handover doc clarifications
..

doc/manuals, vty doc: more handover doc clarifications

Related: OS#3487
Change-Id: I1639efb2dbcca4f0e9c33a74f3067606ce5f4209
---
M doc/manuals/chapters/handover.adoc
M include/osmocom/bsc/handover_cfg.h
M src/osmo-bsc/handover_vty.c
M tests/handover_cfg.vty
4 files changed, 55 insertions(+), 49 deletions(-)


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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1639efb2dbcca4f0e9c33a74f3067606ce5f4209
Gerrit-Change-Number: 14582
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-MessageType: newpatchset

Change in ...osmo-msc[master]: add separate vty cfg for Iu encryption / fix ttcn3 iu tests

2019-08-13 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15175


Change subject: add separate vty cfg for Iu encryption / fix ttcn3 iu tests
..

add separate vty cfg for Iu encryption / fix ttcn3 iu tests

Recently, the ability to run Iu without encryption was added, but the config
for it was tied to the A5 GERAN encryption configuration. This affected
osmo-msc's default behavior of Iu, breaking osmo-msc ttcn3 Iu tests: the ttcn3
test suite sets A5 to 0 (no encryption) but still expects Iu encryption. Fix
this "regression".

Add a separate vty config option for Iu encryption, even if it does not provide
full granularity to select individual UEA algorithms yet.

As a result, Iu default behavior remains to use encryption regardless of the A5
config. Iu encryption can be disabled by the new cfg option "no encryption iu"
alone.

Revert most changes to the msc_vlr test suite in commit "do not force
encryption on UTRAN" (I04ecd7a3b1cc603b2e3feb630e8c7c93fc36ccd7): use new
net->iu_encryption instead of net->a5_encryption_mask.

Adjust/add test_nodes.vty transcript tests.

Related: OS#4144
Change-Id: Ie138f2fcb105533f7bc06a6d2e6deccf6faccc5b
---
M doc/manuals/chapters/net.adoc
M include/osmocom/msc/gsm_data.h
M src/libmsc/gsm_04_08.c
M src/libmsc/msc_net_init.c
M src/libmsc/msc_vty.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.c
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_tests.h
M tests/test_nodes.vty
10 files changed, 129 insertions(+), 62 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/75/15175/1

diff --git a/doc/manuals/chapters/net.adoc b/doc/manuals/chapters/net.adoc
index 4bf34a3..6cb31d1 100644
--- a/doc/manuals/chapters/net.adoc
+++ b/doc/manuals/chapters/net.adoc
@@ -188,11 +188,22 @@

 While authentication is always required on 3G, ciphering is optional.

-So far OsmoMSC lacks explicit configuration for ciphering on 3G. As an interim
-solution, ciphering is enabled on 3G exactly when ciphering is enabled on 2G,
-i.e. when any cipher other than A5/0 is enabled in the configuration. If only
-A5/0 is configured, ciphering will be disabled on both 2G and 3G. The future
-aim is to add comprehensive configuration for 3G ciphering that is independent
-from the 2G setting.
+So far OsmoMSC allows switching ciphering on 3G either on or off -- the default
+behavior is to enable ciphering. (Individual choice of algorithms may be added
+in the future.)
+
+Disable 3G ciphering:
+
+
+network
+ no encryption iu
+
+
+Enable 3G ciphering (default):
+
+
+network
+ encryption iu
+

 OsmoMSC indicates UEA1 and UEA2 as permitted encryption algorithms on 3G.
diff --git a/include/osmocom/msc/gsm_data.h b/include/osmocom/msc/gsm_data.h
index e926b3f..b2b9e70 100644
--- a/include/osmocom/msc/gsm_data.h
+++ b/include/osmocom/msc/gsm_data.h
@@ -149,6 +149,11 @@
bool authentication_required;
int send_mm_info;

+   /* Whether to use encryption on Iu.
+* TODO: we should offer a choice of UEA1 and/or UEA2, and probably 
replace this bool with a bit-mask of
+* permitted Iu encryption algorithms. See also OS#4143 */
+   bool iu_encryption;
+
struct rate_ctr_group *msc_ctrs;
struct osmo_stat_item_group *statg;

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index cd37cff..f3e3d92 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -375,7 +375,7 @@
net->vlr, msc_a, vlr_lu_type, tmsi, imsi,
_lai, _a->via_cell.lai,
is_utran || net->authentication_required,
-   net->a5_encryption_mask > 0x01,
+   is_utran ? net->iu_encryption : 
net->a5_encryption_mask > 0x01,
lu->key_seq,
osmo_gsm48_classmark1_is_r99(>classmark1),
is_utran,
@@ -780,7 +780,7 @@
 req->cm_service_type,
 mi-1, _a->via_cell.lai,
 is_utran || net->authentication_required,
-net->a5_encryption_mask > 0x01,
+is_utran ? net->iu_encryption : 
net->a5_encryption_mask > 0x01,
 req->cipher_key_seq,
 osmo_gsm48_classmark2_is_r99(cm2, cm2_len),
 is_utran);
@@ -1152,7 +1152,7 @@
 net->vlr, msc_a,
 VLR_PR_ARQ_T_PAGING_RESP, 0, mi_lv, 
_a->via_cell.lai,
 is_utran || net->authentication_required,
-net->a5_encryption_mask > 0x01,
+

Change in ...osmo-msc[master]: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests

2019-08-13 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-msc/+/15175

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

Change subject: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests
..

add 'encryption uea 1 2' cfg / fix ttcn3 iu tests

Recently, the ability to run UTRAN without encryption was added, but the config
for it was tied to the A5 GERAN encryption configuration. This affected
osmo-msc's default behavior of Iu, breaking osmo-msc ttcn3 Iu tests: the ttcn3
test suite sets A5 to 0 (no encryption) but still expects Iu to enable air
encryption. Fix this "regression".

Add a separate vty config option for UEA encryption, even if it does not
provide full granularity to select individual UEA algorithms yet.

As a result, Iu default behavior remains to enable encryption regardless of the
A5 config. UTRAN encryption can be disabled by the new cfg option
"encryption uea 0" alone.

Even though the new vty command already allows passing various combinations of
the UEA algorithm numbers, only '0' and '1 2' are accepted as valid
combinations, to reflect current osmo-msc capabilities.

Revert most changes to the msc_vlr test suite in commit "do not force
encryption on UTRAN" (I04ecd7a3b1cc603b2e3feb630e8c7c93fc36ccd7): use new
net->iu_encryption instead of net->a5_encryption_mask.

Adjust/add to test_nodes.vty transcript tests.

Related: OS#4144
Change-Id: Ie138f2fcb105533f7bc06a6d2e6deccf6faccc5b
---
M doc/manuals/chapters/net.adoc
M include/osmocom/msc/gsm_data.h
M src/libmsc/gsm_04_08.c
M src/libmsc/msc_net_init.c
M src/libmsc/msc_vty.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.c
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_tests.h
M tests/test_nodes.vty
10 files changed, 141 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/75/15175/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15175
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie138f2fcb105533f7bc06a6d2e6deccf6faccc5b
Gerrit-Change-Number: 15175
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-MessageType: newpatchset

Change in ...osmo-bsc[master]: add vty 'no neighbors' to remove all HO targets

2019-08-13 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14768 )

Change subject: add vty 'no neighbors' to remove all HO targets
..


Patch Set 3: Code-Review+2

> I did not add any further comments as my original comments remain unmodified.

I did explain my reasons and didn't see them nacked.
I still argue that, in general, using llist_for_each_entry_safe() is actually 
not safe when entries get removed. I'd prefer not spending time on it.

>  If you merge it like this I will likely send a follow-up patch to use llist 
> iteration helpers.

I take this as an ok to merge this, then; Follow up if you might.


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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I8623ab581639e9f8af6a9ff1eca990518d1b1211
Gerrit-Change-Number: 14768
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-CC: laforge 
Gerrit-Comment-Date: Tue, 13 Aug 2019 21:44:45 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: test_mgcp_codec_pt_translate(): more tests

2019-08-13 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15135 )

Change subject: test_mgcp_codec_pt_translate(): more tests
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/15135/3/tests/mgcp/mgcp_test.c
File tests/mgcp/mgcp_test.c:

https://gerrit.osmocom.org/#/c/15135/3/tests/mgcp/mgcp_test.c@1795
PS3, Line 1795: .descr = "both sides have the same payload_type 
numbers assigned to conflicting codecs",
> Are they really conflicting? This looks misleading, since it's actually fine, 
> right?
with conflicting I mean the same number means something different on the other 
side.
The test verifies that even those get translated properly.



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I334a075ac2800ae4a7c4e2d6eaeb17dd8c6b09a1
Gerrit-Change-Number: 15135
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 13 Aug 2019 22:21:56 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in ...osmo-msc[master]: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests

2019-08-13 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-msc/+/15175

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

Change subject: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests
..

add 'encryption uea 1 2' cfg / fix ttcn3 iu tests

Recently, the ability to run UTRAN without encryption was added, but the config
for it was tied to the A5 GERAN encryption configuration. This affected
osmo-msc's default behavior of Iu, breaking osmo-msc ttcn3 Iu tests: the ttcn3
test suite sets A5 to 0 (no encryption) but still expects Iu to enable air
encryption. Fix this "regression".

Add a separate vty config option for UEA encryption, even if it does not
provide full granularity to select individual UEA algorithms yet.

As a result, Iu default behavior remains to enable encryption regardless of the
A5 config. UTRAN encryption can be disabled by the new cfg option
"encryption uea 0" alone.

Even though the new vty command already allows passing various combinations of
the UEA algorithm numbers, only '0' and '1 2' are accepted as valid
combinations, to reflect current osmo-msc capabilities.

Revert most changes to the msc_vlr test suite in commit "do not force
encryption on UTRAN" (I04ecd7a3b1cc603b2e3feb630e8c7c93fc36ccd7): use new
net->iu_encryption instead of net->a5_encryption_mask.

Adjust/add to test_nodes.vty transcript tests.

Related: OS#4144
Change-Id: Ie138f2fcb105533f7bc06a6d2e6deccf6faccc5b
---
M doc/manuals/chapters/net.adoc
M include/osmocom/msc/gsm_data.h
M src/libmsc/gsm_04_08.c
M src/libmsc/msc_net_init.c
M src/libmsc/msc_vty.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.c
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_tests.h
M tests/test_nodes.vty
10 files changed, 153 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/75/15175/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15175
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie138f2fcb105533f7bc06a6d2e6deccf6faccc5b
Gerrit-Change-Number: 15175
Gerrit-PatchSet: 4
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-MessageType: newpatchset

Change in ...docker-playground[master]: adjust osmo-bsc.cfg for TC_ho_neighbor_config_1 thru _7

2019-08-13 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/docker-playground/+/14764 )

Change subject: adjust osmo-bsc.cfg for TC_ho_neighbor_config_1 thru _7
..

adjust osmo-bsc.cfg for TC_ho_neighbor_config_1 thru _7

- Actually enable handover.
- Add bts 3

Related: osmo-ttcn3-hacks Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
Change-Id: I1c57a04747f5ec004ccf4657954dcb0b003c24fc
---
M ttcn3-bsc-test/osmo-bsc.cfg
1 file changed, 30 insertions(+), 1 deletion(-)

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



diff --git a/ttcn3-bsc-test/osmo-bsc.cfg b/ttcn3-bsc-test/osmo-bsc.cfg
index 3d226e9..6cfaf7c 100644
--- a/ttcn3-bsc-test/osmo-bsc.cfg
+++ b/ttcn3-bsc-test/osmo-bsc.cfg
@@ -66,7 +66,7 @@
  encryption a5 0 1 3
  neci 1
  paging any use tch 0
- handover 0
+ handover 1
  handover window rxlev averaging 10
  handover window rxqual averaging 1
  handover window rxlev neighbor averaging 10
@@ -350,6 +350,35 @@
timeslot 7
 phys_chan_config PDCH
 hopping enabled 0
+ bts 3
+  type sysmobts
+  band DCS1800
+  cell_identity 3
+  location_area_code 3
+  # re-use bts 2's ARFCN 871 and BSIC 12 (to test handover config)
+  base_station_id_code 12
+  trx 0
+   rf_locked 0
+   arfcn 871
+   nominal power 23
+   max_power_red 20
+   rsl e1 tei 0
+   timeslot 0
+phys_chan_config CCCH+SDCCH4
+   timeslot 1
+phys_chan_config TCH/F
+   timeslot 2
+phys_chan_config TCH/F
+   timeslot 3
+phys_chan_config TCH/F
+   timeslot 4
+phys_chan_config TCH/F
+   timeslot 5
+phys_chan_config TCH/H
+   timeslot 6
+phys_chan_config PDCH
+   timeslot 7
+phys_chan_config PDCH
 msc 0
  ip.access rtp-base 4000
  no bsc-welcome-text

--
To view, visit https://gerrit.osmocom.org/c/docker-playground/+/14764
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Change-Id: I1c57a04747f5ec004ccf4657954dcb0b003c24fc
Gerrit-Change-Number: 14764
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: merged

Change in ...osmo-msc[master]: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests

2019-08-13 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15175 )

Change subject: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests
..


Patch Set 2:

This change is ready for review.


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie138f2fcb105533f7bc06a6d2e6deccf6faccc5b
Gerrit-Change-Number: 15175
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Tue, 13 Aug 2019 21:28:35 +
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...osmo-bsc[master]: add vty 'no neighbors' to remove all HO targets

2019-08-13 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14768 )

Change subject: add vty 'no neighbors' to remove all HO targets
..

add vty 'no neighbors' to remove all HO targets

This is required for an upcoming TTCN3 test that plays through various neighbor
configurations.

Related: OS#4056
 Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc (osmo-ttcn3-hacks)
Change-Id: I8623ab581639e9f8af6a9ff1eca990518d1b1211
---
M src/osmo-bsc/neighbor_ident_vty.c
M tests/neighbor_ident.vty
2 files changed, 111 insertions(+), 0 deletions(-)

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



diff --git a/src/osmo-bsc/neighbor_ident_vty.c 
b/src/osmo-bsc/neighbor_ident_vty.c
index 203b150..2b8cd7e 100644
--- a/src/osmo-bsc/neighbor_ident_vty.c
+++ b/src/osmo-bsc/neighbor_ident_vty.c
@@ -372,6 +372,82 @@
return CMD_SUCCESS;
 }

+struct nil_match_bts_data {
+   int bts_nr;
+   const struct neighbor_ident_key *found;
+};
+
+static bool nil_match_bts(const struct neighbor_ident_key *key,
+ const struct gsm0808_cell_id_list2 *val,
+ void *cb_data)
+{
+   struct nil_match_bts_data *d = cb_data;
+   if (key->from_bts == d->bts_nr) {
+   d->found = key;
+   return false;
+   }
+   return true;
+}
+
+static int neighbor_del_all(struct vty *vty)
+{
+   int rc;
+   int removed = 0;
+   struct gsm_bts *bts = vty->index;
+
+   OSMO_ASSERT((vty->node == BTS_NODE) && bts);
+
+   /* Remove all local neighbors and print to VTY for the user to know 
what changed */
+   while (1) {
+   struct gsm_bts_ref *neigh = 
llist_first_entry_or_null(>local_neighbors, struct gsm_bts_ref, entry);
+   struct gsm_bts *neigh_bts;
+   if (!neigh)
+   break;
+
+   neigh_bts = neigh->bts;
+   OSMO_ASSERT(neigh_bts);
+
+   /* It would be more efficient to just llist_del() the 
gsm_bts_ref directly, but for the sake of
+* safe/sane API use and against code dup, rather invoke the 
central gsm_bts_local_neighbor_del()
+* function intended for this task. */
+   rc = gsm_bts_local_neighbor_del(bts, neigh_bts);
+   if (rc > 0) {
+   vty_out(vty, "%% Removed local neighbor bts %u to bts 
%u%s",
+   bts->nr, neigh_bts->nr, VTY_NEWLINE);
+   removed += rc;
+   } else {
+   vty_out(vty, "%% Error while removing local neigbor bts 
%u to bts %u, aborted%s",
+   bts->nr, neigh_bts->nr, VTY_NEWLINE);
+   return CMD_WARNING;
+   }
+   }
+
+   /* Remove all remote-BSS neighbors */
+   while (1) {
+   struct neighbor_ident_key k;
+   struct nil_match_bts_data d = {
+   .bts_nr = bts->nr,
+   };
+   neighbor_ident_iter(g_neighbor_cells, nil_match_bts, );
+   if (!d.found)
+   break;
+   k = *d.found;
+   if (neighbor_ident_del(g_neighbor_cells, )) {
+   vty_out(vty, "%% Removed remote BSS neighbor %s%s",
+   neighbor_ident_key_name(), VTY_NEWLINE);
+   removed++;
+   } else {
+   vty_out(vty, "%% Error while removing remote BSS 
neighbor %s, aborted%s",
+   neighbor_ident_key_name(), VTY_NEWLINE);
+   return CMD_WARNING;
+   }
+   }
+
+   if (!removed)
+   vty_out(vty, "%% No neighbors configured%s", VTY_NEWLINE);
+   return CMD_SUCCESS;
+}
+
 DEFUN(cfg_neighbor_add_lac_arfcn_bsic, cfg_neighbor_add_lac_arfcn_bsic_cmd,
NEIGHBOR_ADD_CMD LAC_PARAMS " " NEIGHBOR_IDENT_VTY_KEY_PARAMS,
NEIGHBOR_ADD_DOC LAC_DOC NEIGHBOR_IDENT_VTY_KEY_DOC)
@@ -430,6 +506,15 @@
return del_by_key(vty, );
 }

+DEFUN(cfg_neighbor_del_all, cfg_neighbor_del_all_cmd,
+   "no neighbors",
+   NO_STR
+   "Remove all local and remote-BSS neighbor config for this cell."
+   " Note that this falls back to the legacy behavior of regarding all 
local cells as neighbors.\n")
+{
+   return neighbor_del_all(vty);
+}
+
 struct write_neighbor_ident_entry_data {
struct vty *vty;
const char *indent;
@@ -576,5 +661,6 @@
install_element(BTS_NODE, _neighbor_add_cgi_arfcn_bsic_cmd);
install_element(BTS_NODE, _neighbor_del_bts_nr_cmd);
install_element(BTS_NODE, _neighbor_de

Change in ...osmo-mgw[master]: mgcp_codec: split codec_free() off of codec_init()

2019-08-13 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15132 )

Change subject: mgcp_codec: split codec_free() off of codec_init()
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/15132/2/src/libosmo-mgcp/mgcp_codec.c
File src/libosmo-mgcp/mgcp_codec.c:

https://gerrit.osmocom.org/#/c/15132/2/src/libosmo-mgcp/mgcp_codec.c@252
PS2, Line 252:  conn->end.codecs_assigned++;
> Is the some scenario where we shrink the list and this counter is decreased? 
> It's only set to 0 upon […]
There would be a theoretical scenario where codec negotiation ends up changing 
/ decreasing the set of permitted codecs.

In code there should be a place that completely clears the codecs somewhere, 
IIRC when it starts parsing the SDP, which might come in any number of times 
(MDCX) on a given conn.
It is cleared also when the conn gets deallocated (DLCX).

This is preparation to fix a memory leak that keeps cleared entries in memory 
until the conn gets deallocated.

So any number of MDCXes might clear the list and read new entries, and the 
talloc ctx being the conn still keeps the audio names of the cleared entries 
until the conn gets DLCXed)



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I120cab0a352a1e7b31c8f9c720c47b2c291311d7
Gerrit-Change-Number: 15132
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 13 Aug 2019 22:18:53 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in ...osmo-bsc[master]: neighbor config: allow re-using ARFCN+BSIC pairs

2019-08-13 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/14769 )

Change subject: neighbor config: allow re-using ARFCN+BSIC pairs
..

neighbor config: allow re-using ARFCN+BSIC pairs

Fix neighbor config to match OsmoBSC manual: implement the plan for neighbor
configuration that was so far only described in the manual without actually
being in operation.

This first allows re-using ARFCN+BSIC pairs in and across BSS.

So far the handover_start() code always looked for handover target cells across
*all* local cells, even if they were not listed as neighbors to a source cell.
Imply all cells as neighbors only as long as there are no explicit neighbors
configured. As soon as the first 'neighbor' line appears in a 'bts' config,
only the listed neighbors are regarded as handover target cells. (The
'neighbor-list' commands are not related to this, only the relatively new
'neighbor (bts|lac|cgi|...)' commands affect actual handover procedures.)

TTCN3 tests TC_ho_neighbor_config_1 thru _7 play through the various aspects of
neighbor configuration: both the legacy implicit all-cells-are-neighbors as
well as allowing only explicit neighbors by config.

Related: OS#4056
Related: osmo-ttcn3-hacks Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
Change-Id: I29bca59ab232eddc74e0d4698efb9c9992443983
---
M include/osmocom/bsc/handover.h
M include/osmocom/bsc/handover_fsm.h
M include/osmocom/bsc/neighbor_ident.h
M src/osmo-bsc/handover_decision_2.c
M src/osmo-bsc/handover_fsm.c
M src/osmo-bsc/handover_logic.c
M src/osmo-bsc/neighbor_ident_vty.c
M tests/bsc/bsc_test.c
8 files changed, 241 insertions(+), 61 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/include/osmocom/bsc/handover.h b/include/osmocom/bsc/handover.h
index 322913d..b00ee60 100644
--- a/include/osmocom/bsc/handover.h
+++ b/include/osmocom/bsc/handover.h
@@ -10,6 +10,15 @@
 #include 
 #include 

+#define LOG_HO(conn, level, fmt, args...) do { \
+   if (conn->ho.fi) \
+   LOGPFSML(conn->ho.fi, level, "%s: " fmt, \
+handover_status(conn), ## args); \
+   else \
+   LOGP(DHODEC, level, "%s: " fmt, \
+handover_status(conn), ## args); \
+   } while(0)
+
 struct gsm_network;
 struct gsm_lchan;
 struct gsm_bts;
@@ -25,6 +34,8 @@
HO_RESULT_ERROR,
 };

+const char *handover_status(struct gsm_subscriber_connection *conn);
+
 extern const struct value_string handover_result_names[];
 inline static const char *handover_result_name(enum handover_result val)
 { return get_value_string(handover_result_names, val); }
@@ -70,8 +81,11 @@
   struct gsm_lchan *lchan);
 void bsc_tx_bssmap_ho_failure(struct gsm_subscriber_connection *conn);

-struct gsm_bts *bts_by_neighbor_ident(const struct gsm_network *net,
- const struct neighbor_ident_key 
*search_for);
+int find_handover_target_cell(struct gsm_bts **local_target_cell_p,
+ const struct gsm0808_cell_id_list2 
**remote_target_cell_p,
+ struct gsm_subscriber_connection *conn, const 
struct neighbor_ident_key *search_for,
+ bool log_errors);
+
 struct neighbor_ident_key *bts_ident_key(const struct gsm_bts *bts);

 void handover_parse_inter_bsc_mt(struct gsm_subscriber_connection *conn,
diff --git a/include/osmocom/bsc/handover_fsm.h 
b/include/osmocom/bsc/handover_fsm.h
index 7c2145e..1628d8f 100644
--- a/include/osmocom/bsc/handover_fsm.h
+++ b/include/osmocom/bsc/handover_fsm.h
@@ -4,18 +4,6 @@
 #include 
 #include 

-const char *handover_status(struct gsm_subscriber_connection *conn);
-
-/* This macro automatically includes a final \n, if omitted. */
-#define LOG_HO(conn, level, fmt, args...) do { \
-   if (conn->ho.fi) \
-   LOGPFSML(conn->ho.fi, level, "%s: " fmt, \
-handover_status(conn), ## args); \
-   else \
-   LOGP(DHODEC, level, "%s: " fmt, \
-handover_status(conn), ## args); \
-   } while(0)
-
 /* Terminology:
  * Intra-Cell: stays within one BTS, this should actually be an Assignment.
  * Intra-BSC: stays within one BSC, but moves between BTSes.
diff --git a/include/osmocom/bsc/neighbor_ident.h 
b/include/osmocom/bsc/neighbor_ident.h
index 17bffbc..aa38276 100644
--- a/include/osmocom/bsc/neighbor_ident.h
+++ b/include/osmocom/bsc/neighbor_ident.h
@@ -47,6 +47,8 @@
 void neighbor_ident_vty_init(struct gsm_network *net, struct 
neighbor_ident_list *nil);
 void neighbor_ident_vty_write(struct vty *vty, const char *indent, struct 
gsm_bts *bts);
 
+bool neighbor_ident_bts_entry_exists(uint8_t from_bts);
+
 #define NEIGHBOR_IDENT_VTY_KEY_PARAMS "arfcn <0-1023> bsic (&l

Change in ...libosmocore[master]: add vty logp command to echo on all log targets

2019-08-13 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/14986 )

Change subject: add vty logp command to echo on all log targets
..


Patch Set 1:




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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ife5dc8999174c74e0d133729284fe526d6eaf8d9
Gerrit-Change-Number: 14986
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Tue, 13 Aug 2019 21:58:04 +
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...osmo-ttcn3-hacks[master]: bsc: add TC_ho_neighbor_config_1 thru _7

2019-08-13 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/14765 )

Change subject: bsc: add TC_ho_neighbor_config_1 thru _7
..

bsc: add TC_ho_neighbor_config_1 thru _7

Add tests to play through various neighbor configurations.
Tests will pass as soon as osmo-bsc I29bca59ab232eddc74e0d4698efb9c9992443983
is merged.

Add RSL2 to allow triggering handover to BTS 2.

Adjust osmo-bsc.cfg to match the new tests. Also applied in docker-playground
I1c57a04747f5ec004ccf4657954dcb0b003c24fc.
- Actually enable handover.
- Add bts 3

Depends: osmo-bsc I8623ab581639e9f8af6a9ff1eca990518d1b1211 ('no neighbors')
Related: OS#4056
Change-Id: Ia4ba0e75abd3d45a3422b2525e5f938cdc5a04cc
---
M bsc/BSC_Tests.ttcn
M bsc/osmo-bsc.cfg
M library/RSL_Emulation.ttcn
3 files changed, 396 insertions(+), 1 deletion(-)

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



diff --git a/bsc/BSC_Tests.ttcn b/bsc/BSC_Tests.ttcn
index 44a9172..4497a2e 100644
--- a/bsc/BSC_Tests.ttcn
+++ b/bsc/BSC_Tests.ttcn
@@ -1719,6 +1719,10 @@
connect(vc_conn:RSL1, bts[1].rsl.vc_RSL:CLIENT_PT);
connect(vc_conn:RSL1_PROC, bts[1].rsl.vc_RSL:RSL_PROC);
}
+   if (isvalue(bts[2])) {
+   connect(vc_conn:RSL2, bts[2].rsl.vc_RSL:CLIENT_PT);
+   connect(vc_conn:RSL2_PROC, bts[2].rsl.vc_RSL:RSL_PROC);
+   }
connect(vc_conn:BSSAP, g_bssap.vc_RAN:CLIENT);
connect(vc_conn:MGCP, vc_MGCP:MGCP_CLIENT);
connect(vc_conn:MGCP_MULTI, vc_MGCP:MGCP_CLIENT_MULTI);
@@ -2837,6 +2841,11 @@
f_vty_transceive(BSCVTY, cmd & suffix);
 }

+/* Even though the VTY command to trigger handover takes a new BTS number as 
argument, behind the scenes osmo-bsc always
+ * translates that to a target ARFCN+BSIC first. See bsc_vty.c 
trigger_ho_or_as(), which puts the selected BTS' neighbor
+ * ident key (ARFCN + BSIC) in the struct passed on to handover_request(). 
handover_start() then resolves that to a
+ * viable actual neighbor cell. So from the internal osmo-bsc perspective, we 
always request handover to an ARFCN + BSIC
+ * pair, not really to a specific BTS number. */
 private function f_vty_handover(integer bts_nr, integer trx_nr, RslChannelNr 
chan_nr,
integer new_bts_nr)
 runs on MSC_ConnHdlr {
@@ -3587,6 +3596,353 @@
vc_conn.done;
 }

+type record of charstring Commands;
+
+private function f_bts_0_cfg(Commands cmds := {}) runs on MSC_ConnHdlr
+{
+   f_vty_enter_cfg_bts(BSCVTY, 0);
+   for (var integer i := 0; i < sizeof(cmds); i := i+1) {
+   f_vty_transceive(BSCVTY, cmds[i]);
+   }
+   f_vty_transceive(BSCVTY, "end");
+}
+
+private function f_probe_for_handover(charstring log_label,
+ charstring log_descr,
+ charstring handover_vty_cmd,
+ boolean expect_handover,
+ boolean is_inter_bsc_handover := false)
+runs on MSC_ConnHdlr
+{
+   var RSL_Message rsl;
+
+   var charstring log_msg := " (expecting handover)"
+   if (not expect_handover) {
+   log_msg := " (expecting NO handover)";
+   }
+   log("f_probe_for_handover starting: " & log_label & ": " & log_descr & 
log_msg);
+   f_vty_transceive(BSCVTY, handover_vty_cmd);
+
+   /* We're going to thwart any and all handover attempts, just be ready 
to handle (and ignore) handover target
+* lchans to be established on bts 1 or bts 2. */
+   f_rslem_suspend(RSL1_PROC);
+   f_rslem_suspend(RSL2_PROC);
+
+   timer T := 2.0;
+   T.start;
+
+   alt {
+   [] RSL.receive(tr_RSL_DATA_REQ(g_chan_nr)) -> value rsl {
+   var PDU_ML3_NW_MS l3 := 
dec_PDU_ML3_NW_MS(rsl.ies[2].body.l3_info.payload);
+   log("Rx L3 from net: ", l3);
+   if (ischosen(l3.msgs.rrm.handoverCommand)) {
+   var RslChannelNr new_chan_nr;
+   var GsmArfcn arfcn;
+   
f_ChDesc2RslChanNr(l3.msgs.rrm.handoverCommand.channelDescription2,
+  new_chan_nr, arfcn);
+   log("Handover to new chan ", new_chan_nr, " on ARFCN ", 
arfcn);
+   log(l3.msgs.rrm.handoverCommand);
+
+   /* Need to register for new lchan on new BTS -- it's 
either bts 1 or bts 2.  It doesn't really
+* matter on which BTS it really is, we're not going to 
follow through an entire handover
+* anyway. */
+

Change in ...osmo-sip-connector[master]: avoid bogus error logs when no cmd_timer is set

2019-08-13 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sip-connector/+/15142 )

Change subject: avoid bogus error logs when no cmd_timer is set
..


Patch Set 1:

(2 comments)

https://gerrit.osmocom.org/#/c/15142/1//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/15142/1//COMMIT_MSG@9
PS1, Line 9: mncc.c often calls stop_cmd_timer() even if no timer is running. 
If no timer is
> I can't say I've seen this "often". […]
"often": I thought I saw it every time I end a call...? not positively sure 
since recently I always ran with this patch. But I did see it repeatedly before 
patching.

There are various places calling the stop_cmd_timer() function. Each one of 
those invocations would have to become

  if (...timer->active)
  stop_cmd_timer();

thus it makes more sense to me to check for that once, inside the function.


https://gerrit.osmocom.org/#/c/15142/1/src/mncc.c
File src/mncc.c:

https://gerrit.osmocom.org/#/c/15142/1/src/mncc.c@74
PS1, Line 74:   if (!osmo_timer_pending(>cmd_timeout))
> As far as I can make out, we only start timers immediately before sending an 
> MNCC command to which w […]
I got this:

http://people.osmocom.org/neels/izap8No0/osmo-sip-connector.log.txt

so it seems that the REL_CNF stopped the timer as a "stop it in case it is 
running" measure. those should IMHO not cause error logs if none was running.



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

Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: I70f85a71df55ab8618ed78864cefb6fe5b26f581
Gerrit-Change-Number: 15142
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-CC: keith 
Gerrit-Comment-Date: Tue, 13 Aug 2019 22:09:56 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: keith 
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: test_mgcp_codec_pt_translate(): more tests

2019-08-13 Thread neels

Hello pespin, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15135

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

Change subject: test_mgcp_codec_pt_translate(): more tests
..

test_mgcp_codec_pt_translate(): more tests

Change-Id: I334a075ac2800ae4a7c4e2d6eaeb17dd8c6b09a1
---
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
2 files changed, 76 insertions(+), 2 deletions(-)


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I334a075ac2800ae4a7c4e2d6eaeb17dd8c6b09a1
Gerrit-Change-Number: 15135
Gerrit-PatchSet: 4
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: mgcp_codec: codec_set(): log about all possible errors

2019-08-13 Thread neels

Hello pespin, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15138

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

Change subject: mgcp_codec: codec_set(): log about all possible errors
..

mgcp_codec: codec_set(): log about all possible errors

In codec_set(), for each 'goto error', log the specific error cause.

Also add a TODO and a FIXME comment about inventing dynamic payload type
numbers.

Change-Id: I0b44b574c814882b6f8ae7cd738a6f481cd721fd
---
M src/libosmo-mgcp/mgcp_codec.c
1 file changed, 25 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/38/15138/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/15138
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I0b44b574c814882b6f8ae7cd738a6f481cd721fd
Gerrit-Change-Number: 15138
Gerrit-PatchSet: 4
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: tweak mgcp_parse_audio_ptime_rtpmap()

2019-08-13 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15140 )

Change subject: tweak mgcp_parse_audio_ptime_rtpmap()
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/15140/1//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/15140/1//COMMIT_MSG@12
PS1, Line 12: - instead of strstr("rtpmap"), use 
osmo_str_startswith("a=rtpmap:") to more
> What you say makes sense, but still not sure if the specs talks about 
> whitespacing at the start of t […]
No spaces at line beginnings.
https://tools.ietf.org/html/rfc4566#page-4

   An SDP session description consists of a number of lines of text of
   the form:

  =

   where  MUST be exactly one case-significant character and
is structured text whose format depends on .  In
   general,  is either a number of fields delimited by a single
   space character or a free format string,...



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I730111e245da8485c1b5e8811f75d140e379cec6
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 13 Aug 2019 23:10:44 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Comment-In-Reply-To: neels 
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: mgcp_codec: codec_set(): log about all possible errors

2019-08-13 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15138 )

Change subject: mgcp_codec: codec_set(): log about all possible errors
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/15138/3/src/libosmo-mgcp/mgcp_codec.c
File src/libosmo-mgcp/mgcp_codec.c:

https://gerrit.osmocom.org/#/c/15138/3/src/libosmo-mgcp/mgcp_codec.c@218
PS3, Line 218:   * payload type number is unknown in this function??? */
> you may find information with git blame? Or send an email to ML asking the 
> author. […]
This dynamic number (e.g. 96) would be the one stored in codec->payload_type.

But I found it... it's for codecs in the MGCP header, without SDP.
Opened https://osmocom.org/issues/4150
So that's for a different issue then.



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I0b44b574c814882b6f8ae7cd738a6f481cd721fd
Gerrit-Change-Number: 15138
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 13 Aug 2019 23:03:55 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in ...osmo-ttcn3-hacks[master]: fix TC_ho_int; make neighbor config more robust

2019-08-14 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15216

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

Change subject: fix TC_ho_int; make neighbor config more robust
..

fix TC_ho_int; make neighbor config more robust

Since osmo-bsc commit "neighbor config: allow re-using ARFCN+BSIC pairs"
(I29bca59ab232eddc74e0d4698efb9c9992443983), osmo-bsc considers only those
cells as neighbors that are explicitly listed, or all local cells if none are
listed. This lead to breaking TC_ho_int, because the osmo-bsc.cfg has only one
remote-cell neighbor for bts 0, and hence a handover to local cell bts 1 is now
regarded as invalid.

The remote-cell neighbor is needed for inter-BSC handover tests; also consider
that the TC_ho_neighbor_config_* tests each place individual neighbor
configuration by live VTY interaction.

Hence make all of these tests more robust: remove the neighbor config from the
osmo-bsc.cfg file, and instead include VTY interaction for each test case that
sets the particularly needed neighbor configuration at runtime.

An analogous osmo-bsc.cfg change in docker-playground is in change
If44dd6b578cdc55076c8180707d1c2d69fe5f2a8.

(It is not actually harmful to leave the neighbor config in osmo-bsc.cfg, but
remove that since it is also not needed anymore.)

Change-Id: If44dd6b578cdc55076c8180707d1c2d69fe5f2a8
---
M bsc/BSC_Tests.ttcn
M bsc/osmo-bsc.cfg
2 files changed, 5 insertions(+), 2 deletions(-)


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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If44dd6b578cdc55076c8180707d1c2d69fe5f2a8
Gerrit-Change-Number: 15216
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset

Change in ...osmo-sgsn[master]: gprs_gmm: Avoid spaces in fsm events and enum strings

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15212 )

Change subject: gprs_gmm: Avoid spaces in fsm events and enum strings
..


Patch Set 1: Code-Review+1

(2 comments)

https://gerrit.osmocom.org/#/c/15212/1/src/gprs/gprs_gmm.c
File src/gprs/gprs_gmm.c:

https://gerrit.osmocom.org/#/c/15212/1/src/gprs/gprs_gmm.c@110
PS1, Line 110:  { PMM_DETACHED, "PMM_DETACH" },
maybe while at it fix to "PMM_DETACHED" (or what Vadim said)


https://gerrit.osmocom.org/#/c/15212/1/src/gprs/gprs_gmm_attach.c
File src/gprs/gprs_gmm_attach.c:

https://gerrit.osmocom.org/#/c/15212/1/src/gprs/gprs_gmm_attach.c@328
PS1, Line 328:  { E_VLR_ANSWERED,   "VLR_ANSWERED"},
(this could also use OSMO_VALUE_STRING(); IMHO it's an advantage to print 
exactly the event names as found in the code)



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I16ede8bf8352b09bc772fd7b43fad2c2274b3ec1
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: fixeria 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:55:30 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-ttcn3-hacks[master]: fix TC_ho_int; make neighbor config more robust

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15216 )

Change subject: fix TC_ho_int; make neighbor config more robust
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If44dd6b578cdc55076c8180707d1c2d69fe5f2a8
Gerrit-Change-Number: 15216
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:08:34 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-ttcn3-hacks[master]: fix TC_ho_int; make neighbor config more robust

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15216 )

Change subject: fix TC_ho_int; make neighbor config more robust
..


Patch Set 2:

+2 to self to fix a ttcn3-bsc-test failure


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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If44dd6b578cdc55076c8180707d1c2d69fe5f2a8
Gerrit-Change-Number: 15216
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:09:09 +
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...osmo-ttcn3-hacks[master]: fix TC_ho_int; make neighbor config more robust

2019-08-14 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15216 )

Change subject: fix TC_ho_int; make neighbor config more robust
..

fix TC_ho_int; make neighbor config more robust

Since osmo-bsc commit "neighbor config: allow re-using ARFCN+BSIC pairs"
(I29bca59ab232eddc74e0d4698efb9c9992443983), osmo-bsc considers only those
cells as neighbors that are explicitly listed, or all local cells if none are
listed. This lead to breaking TC_ho_int, because the osmo-bsc.cfg has only one
remote-cell neighbor for bts 0, and hence a handover to local cell bts 1 is now
regarded as invalid.

The remote-cell neighbor is needed for inter-BSC handover tests; also consider
that the TC_ho_neighbor_config_* tests each place individual neighbor
configuration by live VTY interaction.

Hence make all of these tests more robust: remove the neighbor config from the
osmo-bsc.cfg file, and instead include VTY interaction for each test case that
sets the particularly needed neighbor configuration at runtime.

An analogous osmo-bsc.cfg change in docker-playground is in change
If44dd6b578cdc55076c8180707d1c2d69fe5f2a8.

(It is not actually harmful to leave the neighbor config in osmo-bsc.cfg, but
remove that since it is also not needed anymore.)

Change-Id: If44dd6b578cdc55076c8180707d1c2d69fe5f2a8
---
M bsc/BSC_Tests.ttcn
M bsc/osmo-bsc.cfg
2 files changed, 5 insertions(+), 2 deletions(-)

Approvals:
  Jenkins Builder: Verified
  neels: Looks good to me, approved



diff --git a/bsc/BSC_Tests.ttcn b/bsc/BSC_Tests.ttcn
index 4497a2e..09c5e5c 100644
--- a/bsc/BSC_Tests.ttcn
+++ b/bsc/BSC_Tests.ttcn
@@ -2863,6 +2863,7 @@
ass_cmd.pdu.bssmap.assignmentRequest.codecList := 
valueof(ts_BSSMAP_IE_CodecList({ts_CodecFR}));

f_establish_fully(ass_cmd, exp_compl);
+   f_bts_0_cfg({"neighbor bts 1"});

var HandoverState hs := {
rr_ho_cmpl_seen := false,
@@ -2944,6 +2945,7 @@
var template PDU_BSSAP exp_compl := f_gen_exp_compl();
f_establish_fully(ass_req, exp_compl);

+   f_bts_0_cfg({"neighbor lac 99 arfcn 123 bsic any"});
f_vty_transceive(BSCVTY, "handover any to arfcn 123 bsic any");

BSSAP.receive(tr_BSSMAP_HandoverRequired);
@@ -3010,6 +3012,7 @@
var template PDU_BSSAP exp_compl := f_gen_exp_compl();
f_establish_fully(ass_req, exp_compl);

+   f_bts_0_cfg({"neighbor lac 99 arfcn 123 bsic any"});
f_vty_transceive(BSCVTY, "handover any to arfcn 123 bsic any");

BSSAP.receive(tr_BSSMAP_HandoverRequired);
@@ -3056,6 +3059,7 @@
var template PDU_BSSAP exp_compl := f_gen_exp_compl();
f_establish_fully(ass_req, exp_compl);

+   f_bts_0_cfg({"neighbor lac 99 arfcn 123 bsic any"});
f_vty_transceive(BSCVTY, "handover any to arfcn 123 bsic any");

BSSAP.receive(tr_BSSMAP_HandoverRequired);
@@ -3140,6 +3144,7 @@
var template PDU_BSSAP exp_compl := f_gen_exp_compl();
f_establish_fully(ass_req, exp_compl);

+   f_bts_0_cfg({"neighbor lac 99 arfcn 123 bsic any"});
f_vty_transceive(BSCVTY, "handover any to arfcn 123 bsic any");

BSSAP.receive(tr_BSSMAP_HandoverRequired);
diff --git a/bsc/osmo-bsc.cfg b/bsc/osmo-bsc.cfg
index 9384491..74fe255 100644
--- a/bsc/osmo-bsc.cfg
+++ b/bsc/osmo-bsc.cfg
@@ -88,8 +88,6 @@
   early-classmark-sending forbidden
   ip.access unit_id 1234 0
   oml ip.access stream_id 255 line 0
-  # remote-BSS neighbor:
-  neighbor lac 99 arfcn 123 bsic any
   neighbor-list mode manual-si5
   neighbor-list add arfcn 100
   neighbor-list add arfcn 200

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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If44dd6b578cdc55076c8180707d1c2d69fe5f2a8
Gerrit-Change-Number: 15216
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-MessageType: merged

Change in ...docker-playground[master]: apply osmo-bsc.cfg changes from osmo-ttcn3-hacks

2019-08-14 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/docker-playground/+/15217 )

Change subject: apply osmo-bsc.cfg changes from osmo-ttcn3-hacks
..

apply osmo-bsc.cfg changes from osmo-ttcn3-hacks

Analogous to osmo-ttcn3-hacks patch If44dd6b578cdc55076c8180707d1c2d69fe5f2a8,
also remove the neighbor config from osmo-bsc.cfg here.

Change-Id: I23d9a2046709452f4d3904230871f7421c6987a9
---
M ttcn3-bsc-test/osmo-bsc.cfg
1 file changed, 0 insertions(+), 2 deletions(-)

Approvals:
  neels: Looks good to me, approved; Verified



diff --git a/ttcn3-bsc-test/osmo-bsc.cfg b/ttcn3-bsc-test/osmo-bsc.cfg
index 6cfaf7c..5bb1525 100644
--- a/ttcn3-bsc-test/osmo-bsc.cfg
+++ b/ttcn3-bsc-test/osmo-bsc.cfg
@@ -96,8 +96,6 @@
   early-classmark-sending forbidden
   ip.access unit_id 1234 0
   oml ip.access stream_id 255 line 0
-  # remote-BSS neighbor:
-  neighbor lac 99 arfcn 123 bsic any
   neighbor-list mode manual-si5
   neighbor-list add arfcn 100
   neighbor-list add arfcn 200

--
To view, visit https://gerrit.osmocom.org/c/docker-playground/+/15217
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Change-Id: I23d9a2046709452f4d3904230871f7421c6987a9
Gerrit-Change-Number: 15217
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: neels 
Gerrit-MessageType: merged

Change in ...docker-playground[master]: apply osmo-bsc.cfg changes from osmo-ttcn3-hacks

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/docker-playground/+/15217 )

Change subject: apply osmo-bsc.cfg changes from osmo-ttcn3-hacks
..


Patch Set 1: Verified+1 Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/docker-playground/+/15217
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Change-Id: I23d9a2046709452f4d3904230871f7421c6987a9
Gerrit-Change-Number: 15217
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:10:35 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-sgsn[master]: Introduce and use log macros when no mm ctx available

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15211 )

Change subject: Introduce and use log macros when no mm ctx available
..


Patch Set 1: Code-Review+1

(4 comments)

https://gerrit.osmocom.org/#/c/15211/1/include/osmocom/sgsn/gprs_sgsn.h
File include/osmocom/sgsn/gprs_sgsn.h:

https://gerrit.osmocom.org/#/c/15211/1/include/osmocom/sgsn/gprs_sgsn.h@264
PS1, Line 264: #define LOGIUP(level, ue, fmt, args...) \
(cosmetic)

hmm, I see that LOGMMCTXP() also has level as first arg, but AFAIK all the rest 
of the Osmocom code base uses this order:

  LOGFOO(object, level, fmt, args...)

Probably not worth the effort, but I'd prefer new LOG macros to be consistent 
with that ordering, and maybe one day use sed to adjust LOGMMCTXP() to that 
order as well...?


https://gerrit.osmocom.org/#/c/15211/1/include/osmocom/sgsn/gprs_sgsn.h@284
PS1, Line 284:
(in osmo-msc and -bsc I named the logging macros more like LOG_HO and 
LOG_MSC_A, for readability. Here that would be LOG_IU, LOG_GB... I'd prefer 
that but you decide.)


https://gerrit.osmocom.org/#/c/15211/1/src/gprs/gprs_gmm.c
File src/gprs/gprs_gmm.c:

https://gerrit.osmocom.org/#/c/15211/1/src/gprs/gprs_gmm.c@185
PS1, Line 185:  LOGIUP(LOGL_NOTICE, ctx, "Cannot find mm ctx for IU 
event %d\n", type); \
(we have a value string for those events, see ranap_iu_event_type_str() from 
iu_client.h)


https://gerrit.osmocom.org/#/c/15211/1/src/gprs/gprs_gmm.c@221
PS1, Line 221:  LOGIUP(LOGL_NOTICE, ctx, "Unknown event 
received: %i\n", type);
ranap_iu_event_type_str()?



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: Iba22060d8646bc8ec6227684ccb91d98cb4c7be2
Gerrit-Change-Number: 15211
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:52:46 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-bsc[master]: gsm_08_08.c: always pick first msc for unsolicit paging responses

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/15209 )

Change subject: gsm_08_08.c: always pick first msc for unsolicit paging 
responses
..


Patch Set 1:

(2 comments)

agree to fix the Change-Id duality, otherwise +1 from me

https://gerrit.osmocom.org/#/c/15209/1/src/osmo-bsc/gsm_08_08.c
File src/osmo-bsc/gsm_08_08.c:

https://gerrit.osmocom.org/#/c/15209/1/src/osmo-bsc/gsm_08_08.c@328
PS1, Line 328:  LOGP(DMSC, LOGL_ERROR, "Got paged but no subscriber 
found, will now (blindly) deliver the paging response to the first configured 
MSC!\n");
s/Got paged/Got Paging Response ?


https://gerrit.osmocom.org/#/c/15209/1/src/osmo-bsc/gsm_08_08.c@358
PS1, Line 358:   * blindly. */
> As commented on other patch, shouldn't this be VTY configurable? (to which 
> MSC send them).
This entire code looks like weird legacy bsc-nat stuff, I've never used 
osmo-bsc with more than one MSC.
I think it makes a lot of sense to send the CSFB Paging Response to The One MSC.
The entire going through lists of MSCs is a legacy nonsense that we should 
probably remove anyway.

Correct me if I'm wrong (laforge?) but I think we don't need more vty for 
multiple MSC scenarios.



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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I7f091ed1bbc2afe12656e42031e122144eeb6826
Gerrit-Change-Number: 15209
Gerrit-PatchSet: 1
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-CC: neels 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:28:08 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in ...libosmocore[master]: osmo_tdef_get(): allow passing -1 as default timeout

2019-08-14 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/libosmocore/+/15218


Change subject: osmo_tdef_get(): allow passing -1 as default timeout
..

osmo_tdef_get(): allow passing -1 as default timeout

The intention of osmo_tdef_get()'s val_if_not_present argument was to return a
default timeout, or to optionally abort the program for missing timer
definitions if the default timeout is < 0. This was the case in the original
implementation of this API in osmo-bsc, but in the migration to libosmocore,
the argument was by accident changed to an unsigned type. In consequence, the
assertion in the implementation that was intended to abort the program seemed
bogus to coverity, and was fixed by removal in
I7a544d2d43b83135def296674f777e48fe5fd80a -- the wrong direction, as is obvious
from the API doc for osmo_tdef_get().

Note that osmo-bsc master passes -1 in various places and expects the
program-abort behavior that was missing from the libosmocore implementation.

Change the val_if_not_present argument to a signed type, and revert removal of
the assertion, so that passing -1 has the effect described in the API doc:
program abort on missing timer definition.

This bug was not detected because it is hard to write tests that expect a
program abort to happen, hence no tests for this API feature exist.

Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
---
M TODO-RELEASE
M include/osmocom/core/tdef.h
M src/tdef.c
3 files changed, 4 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/18/15218/1

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 8ccfa49..665ecf7 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -7,3 +7,4 @@
 # If any interfaces have been added since the last public release: c:r:a + 1.
 # If any interfaces have been removed or changed since the last public 
release: c:r:0.
 #library   whatdescription / commit summary line
+core   osmo_tdef_get() change val_if_not_present arg from 
unsigned long to long to allow passing -1
diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h
index c8d9053..566f5dd 100644
--- a/include/osmocom/core/tdef.h
+++ b/include/osmocom/core/tdef.h
@@ -95,7 +95,7 @@

 void osmo_tdefs_reset(struct osmo_tdef *tdefs);
 unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum 
osmo_tdef_unit as_unit,
-   unsigned long val_if_not_present);
+   long val_if_not_present);
 struct osmo_tdef *osmo_tdef_get_entry(struct osmo_tdef *tdefs, int T);

 /*! Using osmo_tdef for osmo_fsm_inst: array entry for a mapping of state 
numbers to timeout definitions.
diff --git a/src/tdef.c b/src/tdef.c
index 3cfb17c..40a9900 100644
--- a/src/tdef.c
+++ b/src/tdef.c
@@ -183,10 +183,11 @@
  * \param[in] val_if_not_present  Fallback value to return if no timeout is 
defined.
  * \return Timeout value in the unit given by as_unit, rounded up if 
necessary, or val_if_not_present.
  */
-unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum 
osmo_tdef_unit as_unit, unsigned long val_if_not_present)
+unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum 
osmo_tdef_unit as_unit, long val_if_not_present)
 {
const struct osmo_tdef *t = osmo_tdef_get_entry((struct 
osmo_tdef*)tdefs, T);
if (!t) {
+   OSMO_ASSERT(val_if_not_present >= 0);
return val_if_not_present;
}
return osmo_tdef_round(t->val, t->unit, as_unit);

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
Gerrit-Change-Number: 15218
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...libosmocore[master]: osmo_tdef_get(): allow passing -1 as default timeout

2019-08-14 Thread neels

neels has uploaded a new patch set (#2). ( 
https://gerrit.osmocom.org/c/libosmocore/+/15218 )

Change subject: osmo_tdef_get(): allow passing -1 as default timeout
..

osmo_tdef_get(): allow passing -1 as default timeout

The intention of osmo_tdef_get()'s val_if_not_present argument was to return a
default timeout, or to optionally abort the program for missing timer
definitions if the default timeout is < 0. This was the case in the original
implementation of this API in osmo-bsc, but in the migration to libosmocore,
the argument was by accident changed to an unsigned type. In consequence, the
assertion in the implementation that was intended to abort the program seemed
bogus to coverity, and was fixed by removal in
I7a544d2d43b83135def296674f777e48fe5fd80a -- the wrong direction, as is obvious
from the API doc for osmo_tdef_get().

Note that osmo-bsc master passes -1 in various places and expects the
program-abort behavior that was missing from the libosmocore implementation.

Change the val_if_not_present argument to a signed type, and revert removal of
the assertion, so that passing -1 has the effect described in the API doc:
program abort on missing timer definition.

This bug was not detected because it is hard to write tests that expect a
program abort to happen, hence no tests for this API feature exist.

Related: OS#4152
Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
---
M TODO-RELEASE
M include/osmocom/core/tdef.h
M src/tdef.c
3 files changed, 4 insertions(+), 2 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
Gerrit-Change-Number: 15218
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-MessageType: newpatchset

Change in ...osmo-sgsn[master]: Replace own timer infra with libosmocore osmo_tdef

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15214 )

Change subject: Replace own timer infra with libosmocore osmo_tdef
..


Patch Set 1: Code-Review-1

(2 comments)

nice!

https://gerrit.osmocom.org/#/c/15214/1/src/gprs/gprs_gmm.c
File src/gprs/gprs_gmm.c:

https://gerrit.osmocom.org/#/c/15214/1/src/gprs/gprs_gmm.c@475
PS1, Line 475:  t = osmo_tdef_get(sgsn->cfg.T_defs, 3312, OSMO_TDEF_S, -1);
Hmm, I just notice a flaw in the osmo_tdef() API.

The val_if_not_present argument is an unsigned long, and I see -1 explicitly in 
the API docs, indicating that a negative value causes a program abort if no 
timer is found. That is in fact not possible with an unsigned long arg.

I created https://osmocom.org/issues/4152 for this.


https://gerrit.osmocom.org/#/c/15214/1/src/gprs/gprs_gmm_attach.c
File src/gprs/gprs_gmm_attach.c:

https://gerrit.osmocom.org/#/c/15214/1/src/gprs/gprs_gmm_attach.c@15
PS1, Line 15: static const struct osmo_tdef_state_timeout 
gmm_attach_fsm_timeouts[] = {
-1: you *have* to explicitly use an array size of [32] to be safe. See 
osmo_tdef_get_state_timeout() API doc.



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: Ia0cf5f0a49737fbc419e2ccc86312d01c6e0056e
Gerrit-Change-Number: 15214
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:18:34 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-sgsn[master]: tests: Check timers can be set over VTY

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15213 )

Change subject: tests: Check timers can be set over VTY
..


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/15213/1/tests/vty_test_runner.py
File tests/vty_test_runner.py:

https://gerrit.osmocom.org/#/c/15213/1/tests/vty_test_runner.py@277
PS1, Line 277: self.assertTrue(self.vty.verify('timer t%d 10' % t, 
['']))
> I would encourage to instead add vty transcript tests. They are far easier to 
> maintain... […]
Oh wait, you actually did add transcript tests in a later patch! :D



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I486fc2a56e235a539836894d2042c1ca6e514ab9
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:20:11 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels 
Gerrit-MessageType: comment

Change in ...osmo-sgsn[master]: tests: Introduce vty-transcript-test tests

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15215 )

Change subject: tests: Introduce vty-transcript-test tests
..


Patch Set 4: Code-Review+2

excellent!


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: Ifd092b9561d49be1f62769d95ba49f6e4aeb4066
Gerrit-Change-Number: 15215
Gerrit-PatchSet: 4
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:21:08 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-sgsn[master]: gprs_gmm.c: Call mmctx_set_(p)mm_state only on related ran_type

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15186 )

Change subject: gprs_gmm.c: Call mmctx_set_(p)mm_state only on related ran_type
..


Patch Set 2:

(7 comments)

I'm advocating for OSMO_ASSERTS(); they would probably give us more SGSN 
crashes, but then we stand a chance of finding bugs related to a subscriber 
switching RAN types. The aim could also be to not crash and instead log errors 
and detach the subscriber, not so sure.

https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c
File src/gprs/gprs_gmm.c:

https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@140
PS2, Line 140: {
Vadim's idea seems very good, and I would add:
Place an OSMO_ASSERT for matching RAN type to catch all bugs that mix and mess 
it up.
(Am not aware of the callers but would assume no mismatching RAN type state 
should ever be passed to this.)


https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@153
PS2, Line 153:  case PMM_CONNECTED:
(while at it we could lose this 'case', or even make the entire switch into an 
if())


https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@204
PS2, Line 204:  mmctx_set_pmm_state(mm, PMM_IDLE);
how could a RANAP_IU_EVENT come in on a ran_type != MM_CTX_T_UTRAN_Iu? I guess 
that should be an OSMO_ASSERT()?


https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@1096
PS2, Line 1096: case GSM48_MT_GMM_SERVICE_REQ:
again this looks like it is clearly related to Iu and ran_type should always be 
UTRAN_Iu?


https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@2084
PS2, Line 2084: mmctx->gmm_state = GMM_REGISTERED_NORMAL;
I know this was in the code before, but it seems to overwrite the previous 
state. Both RAN types set the state after this, so this is redundant and may 
confuse logging?
(It says "Changing state from X to Y", where the X is here always 
GMM_REGISTERED_NORMAL, although the state would have been something else before 
overwriting?)


https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@2097
PS2, Line 2097: case MM_CTX_T_GERAN_Iu:
(instead of all of these dead switch cases, maybe we should rather "#if 0" away 
the unused enum member)


https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@2116
PS2, Line 2116: mmctx->gmm_state = GMM_REGISTERED_NORMAL;
(again weird overwriting of the current state, like above)



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I65ad9e180177bc9fc7c4a037cd85cfe33b161f73
Gerrit-Change-Number: 15186
Gerrit-PatchSet: 2
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria 
Gerrit-CC: neels 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:39:43 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...osmo-sgsn[master]: tests: Check timers can be set over VTY

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15213 )

Change subject: tests: Check timers can be set over VTY
..


Patch Set 1: Code-Review+1

(2 comments)

https://gerrit.osmocom.org/#/c/15213/1//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/15213/1//COMMIT_MSG@7
PS1, Line 7: tests: Check timers can be set over VTY
you mean "add tests to verify that...", right?
(I didn't understand when I read it)


https://gerrit.osmocom.org/#/c/15213/1/tests/vty_test_runner.py
File tests/vty_test_runner.py:

https://gerrit.osmocom.org/#/c/15213/1/tests/vty_test_runner.py@277
PS1, Line 277: self.assertTrue(self.vty.verify('timer t%d 10' % t, 
['']))
I would encourage to instead add vty transcript tests. They are far easier to 
maintain...
You can easily copy-paste the Makefile.am bits from osmo-msc and add a 
timers.vty file.
Or maybe next time.



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I486fc2a56e235a539836894d2042c1ca6e514ab9
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:00:02 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-ttcn3-hacks[master]: BSC_Tests: Change test-expectation of TC_paging_resp_unsol

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15208 )

Change subject: BSC_Tests: Change test-expectation of TC_paging_resp_unsol
..


Patch Set 1: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/#/c/15208/1/bsc/BSC_Tests.ttcn
File bsc/BSC_Tests.ttcn:

https://gerrit.osmocom.org/#/c/15208/1/bsc/BSC_Tests.ttcn@1631
PS1, Line 1631: /* Expevct a CR with a matching Paging response on the 
A-Interface */
(typo)



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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I5562cbf61a2aa42e6950860bc0f9c6c20c61a9fe
Gerrit-Change-Number: 15208
Gerrit-PatchSet: 1
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:29:53 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-ttcn3-hacks[master]: SGSN: introduce f_send_l3() to allow one function for Gb & Iu

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15171 )

Change subject: SGSN: introduce f_send_l3() to allow one function for Gb & Iu
..


Patch Set 1: Code-Review+1

what's up with the [gb_idx >= NUM_GB] conditions?


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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If47ad2be459ca7b87d9071d9ff020a51821e4433
Gerrit-Change-Number: 15171
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:32:41 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-ttcn3-hacks[master]: fix TC_ho_int; make neighbor config more robust

2019-08-14 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15216


Change subject: fix TC_ho_int; make neighbor config more robust
..

fix TC_ho_int; make neighbor config more robust

Since osmo-bsc commit "neighbor config: allow re-using ARFCN+BSIC pairs"
(I29bca59ab232eddc74e0d4698efb9c9992443983), osmo-bsc considers only those
cells as neighbors that are explicitly listed, or all local cells if none are
listed. This lead to breaking TC_ho_int, because the osmo-bsc.cfg has only one
remote-cell neighbor for bts 0, and hence a handover to local cell bts 1 is now
regarded as invalid.

The remote-cell neighbor is needed for inter-BSC handover tests; also consider
that the TC_ho_neighbor_config_* tests each place individual neighbor
configuration by live VTY interaction.

Hence make all of these tests more robust: remove the neighbor config from the
osmo-bsc.cfg file, and instead include VTY interaction for each test case that
sets the particularly needed neighbor configuration at runtime.

An analogous osmo-bsc.cfg change in docker-playground is in change
If44dd6b578cdc55076c8180707d1c2d69fe5f2a8.

(It is not actually harmful to leave the neighbor config in osmo-bsc.cfg, but
remove that since it is also not needed anymore.)

Change-Id: If44dd6b578cdc55076c8180707d1c2d69fe5f2a8
---
M bsc/BSC_Tests.ttcn
M bsc/BSC_Tests_LCLS.ttcn
M bsc/osmo-bsc.cfg
3 files changed, 5 insertions(+), 157 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks 
refs/changes/16/15216/1

diff --git a/bsc/BSC_Tests.ttcn b/bsc/BSC_Tests.ttcn
index 4497a2e..e286905 100644
--- a/bsc/BSC_Tests.ttcn
+++ b/bsc/BSC_Tests.ttcn
@@ -2863,6 +2863,7 @@
ass_cmd.pdu.bssmap.assignmentRequest.codecList := 
valueof(ts_BSSMAP_IE_CodecList({ts_CodecFR}));

f_establish_fully(ass_cmd, exp_compl);
+   f_bts_0_cfg({"neighbor bts 1"});

var HandoverState hs := {
rr_ho_cmpl_seen := false,
@@ -2944,6 +2945,7 @@
var template PDU_BSSAP exp_compl := f_gen_exp_compl();
f_establish_fully(ass_req, exp_compl);

+   f_bts_0_cfg({"neighbor lac 99 arfcn 123 bsic any"});
f_vty_transceive(BSCVTY, "handover any to arfcn 123 bsic any");

BSSAP.receive(tr_BSSMAP_HandoverRequired);
@@ -3010,6 +3012,7 @@
var template PDU_BSSAP exp_compl := f_gen_exp_compl();
f_establish_fully(ass_req, exp_compl);

+   f_bts_0_cfg({"neighbor lac 99 arfcn 123 bsic any"});
f_vty_transceive(BSCVTY, "handover any to arfcn 123 bsic any");

BSSAP.receive(tr_BSSMAP_HandoverRequired);
@@ -3056,6 +3059,7 @@
var template PDU_BSSAP exp_compl := f_gen_exp_compl();
f_establish_fully(ass_req, exp_compl);

+   f_bts_0_cfg({"neighbor lac 99 arfcn 123 bsic any"});
f_vty_transceive(BSCVTY, "handover any to arfcn 123 bsic any");

BSSAP.receive(tr_BSSMAP_HandoverRequired);
@@ -3140,6 +3144,7 @@
var template PDU_BSSAP exp_compl := f_gen_exp_compl();
f_establish_fully(ass_req, exp_compl);

+   f_bts_0_cfg({"neighbor lac 99 arfcn 123 bsic any"});
f_vty_transceive(BSCVTY, "handover any to arfcn 123 bsic any");

BSSAP.receive(tr_BSSMAP_HandoverRequired);
@@ -4357,131 +4362,6 @@
  */

 control {
-   /* CTRL interface testing */
-   execute( TC_ctrl_msc_connection_status() );
-   execute( TC_ctrl_msc0_connection_status() );
-   execute( TC_ctrl() );
-   if (mp_bssap_cfg.transport == BSSAP_TRANSPORT_SCCPlite_SERVER) {
-   execute( TC_ctrl_location() );
-   }
-
-   /* RSL DCHAN Channel ACtivation / Deactivation */
-   execute( TC_chan_act_noreply() );
-   execute( TC_chan_act_counter() );
-   execute( TC_chan_act_ack_noest() );
-   execute( TC_chan_act_ack_est_ind_noreply() );
-   execute( TC_chan_act_ack_est_ind_refused() );
-   execute( TC_chan_act_nack() );
-   execute( TC_chan_exhaustion() );
-   execute( TC_chan_deact_silence() );
-   execute( TC_chan_rel_rll_rel_ind() );
-   execute( TC_chan_rel_conn_fail() );
-   execute( TC_chan_rel_hard_clear() );
-   execute( TC_chan_rel_hard_clear_csfb() );
-   execute( TC_chan_rel_hard_rlsd() );
-   execute( TC_chan_rel_hard_rlsd_ms_dead() );
-   execute( TC_chan_rel_a_reset() );
-
-   execute( TC_outbound_connect() );
-
-   /* Assignment related */
-   execute( TC_assignment_cic_only() );
-   execute( TC_assignment_csd() );
-   execute( TC_assignment_ctm() );
-   execute( TC_assignment_sign() );
-   execute( TC_assignment_fr_a5_0() );
-   execute( TC_assignment_fr_a5_1() );
-   if (mp_bssap_cfg.transport == BSSAP_TRANSPORT_AoIP) {
-   execute( TC_assignment_fr_a5_1_codec_missing() );
-   }
-   execute( TC_as

Change in ...docker-playground[master]: apply osmo-bsc.cfg changes from osmo-ttcn3-hacks

2019-08-14 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/docker-playground/+/15217


Change subject: apply osmo-bsc.cfg changes from osmo-ttcn3-hacks
..

apply osmo-bsc.cfg changes from osmo-ttcn3-hacks

Analogous to osmo-ttcn3-hacks patch If44dd6b578cdc55076c8180707d1c2d69fe5f2a8,
also remove the neighbor config from osmo-bsc.cfg here.

Change-Id: I23d9a2046709452f4d3904230871f7421c6987a9
---
M ttcn3-bsc-test/osmo-bsc.cfg
1 file changed, 0 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/docker-playground 
refs/changes/17/15217/1

diff --git a/ttcn3-bsc-test/osmo-bsc.cfg b/ttcn3-bsc-test/osmo-bsc.cfg
index 6cfaf7c..5bb1525 100644
--- a/ttcn3-bsc-test/osmo-bsc.cfg
+++ b/ttcn3-bsc-test/osmo-bsc.cfg
@@ -96,8 +96,6 @@
   early-classmark-sending forbidden
   ip.access unit_id 1234 0
   oml ip.access stream_id 255 line 0
-  # remote-BSS neighbor:
-  neighbor lac 99 arfcn 123 bsic any
   neighbor-list mode manual-si5
   neighbor-list add arfcn 100
   neighbor-list add arfcn 200

--
To view, visit https://gerrit.osmocom.org/c/docker-playground/+/15217
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Change-Id: I23d9a2046709452f4d3904230871f7421c6987a9
Gerrit-Change-Number: 15217
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-sgsn[master]: gprs_gmm: Introduce assert to guard against unexpected condition

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15167 )

Change subject: gprs_gmm: Introduce assert to guard against unexpected condition
..


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/#/c/15167/2/src/gprs/gprs_gmm.c
File src/gprs/gprs_gmm.c:

https://gerrit.osmocom.org/#/c/15167/2/src/gprs/gprs_gmm.c@1747
PS2, Line 1747: mmctx->ran_type == MM_CTX_T_GERAN_Gb
> Oh, sorry, I was confused. Can we simplify the code a bit? […]
I'm also confused.
The condition matching the current patch would have to be

  if (mmctx->ran_type == MM_CTX_T_GERAN_Gb)
OSMO_ASSERT(mmctx->gb.llme);

But it makes no sense when reading the comment above it.
To make sure of no dangling pointers I also expected an 'gb.llme = NULL' 
assignment instead of an assert?

Pau, can you explain better what the intention is and possibly fix the 
condition?



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I8e1eaeb9b3ebee8e45704b4fe007190c7db609e4
Gerrit-Change-Number: 15167
Gerrit-PatchSet: 2
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: lynxis lazus 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-CC: fixeria 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:20:25 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment

Change in ...osmo-sgsn[master]: gprs_sgsn.h: Flag MM_CTX_T_GERAN_Iu as not supported

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15184 )

Change subject: gprs_sgsn.h: Flag MM_CTX_T_GERAN_Iu as not supported
..


Patch Set 2: Code-Review+2

why is that even in there...


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I3b53a530ab25434e2b2f4d80ad70a8a5f22bfcac
Gerrit-Change-Number: 15184
Gerrit-PatchSet: 2
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:21:38 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-sgsn[master]: gprs_gmm.c: Flag mmctx_set_(p)mm_state() functions static

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/15185 )

Change subject: gprs_gmm.c: Flag mmctx_set_(p)mm_state() functions static
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I061144b6994ee40d5b32eb321dd4f3d3786d028d
Gerrit-Change-Number: 15185
Gerrit-PatchSet: 2
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:21:59 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-msc[master]: Implement a global switch on the network to disable call waiting.

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15120 )

Change subject: Implement a global switch on the network to disable call 
waiting.
..


Patch Set 12: Code-Review+1

(2 comments)

non-mandatory stuff...

https://gerrit.osmocom.org/#/c/15120/12/src/libmsc/msc_vty.c
File src/libmsc/msc_vty.c:

https://gerrit.osmocom.org/#/c/15120/12/src/libmsc/msc_vty.c@309
PS12, Line 309:   "Disable Call Waiting on the Network\n")
(could follow up with a longer description like "...: while a call is active, 
reject all other incoming calls")


https://gerrit.osmocom.org/#/c/15120/12/tests/test_nodes.vty
File tests/test_nodes.vty:

https://gerrit.osmocom.org/#/c/15120/12/tests/test_nodes.vty@30
PS12, Line 30:
it would also be nice to add tests that ensure the write-back works:

  OsmoMSC(config-net)# show running-config
  ...
   call-waiting
  ...
  OsmoMSC(config-net)# no call-waiting
  OsmoMSC(config-net)# show running-config
  ...
   no call-waiting
  ...
  OsmoMSC(config-net)# call-waiting
  OsmoMSC(config-net)# show running-config
  ...
   call-waiting
  ...



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I3eb6f23f7103e3002874fb5d3a30c9de952202ae
Gerrit-Change-Number: 15120
Gerrit-PatchSet: 12
Gerrit-Owner: keith 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-CC: laforge 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:40:02 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-sgsn[master]: libgtp: don't call sgsn_pdp_ctx_free() with NULL pdp

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-sgsn/+/13929 )

Change subject: libgtp: don't call sgsn_pdp_ctx_free() with NULL pdp
..


Patch Set 5:

(1 comment)

hmm, I can't vote here because...

https://gerrit.osmocom.org/#/c/13929/5/src/gprs/sgsn_libgtp.c
File src/gprs/sgsn_libgtp.c:

https://gerrit.osmocom.org/#/c/13929/5/src/gprs/sgsn_libgtp.c@543
PS5, Line 543:  struct sgsn_pdp_ctx *pctx = cbp;
I wish I knew why there's a pdp_t *and* a separate sgsn_pdp_ctx



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I184dcce27b26104c61d80b2d910388d5d3323def
Gerrit-Change-Number: 13929
Gerrit-PatchSet: 5
Gerrit-Owner: keith 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith 
Gerrit-Reviewer: lynxis lazus 
Gerrit-CC: fixeria 
Gerrit-CC: neels 
Gerrit-Comment-Date: Thu, 15 Aug 2019 00:43:27 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: test_mgcp_codec_pt_translate(): more tests

2019-08-12 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15135

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

Change subject: test_mgcp_codec_pt_translate(): more tests
..

test_mgcp_codec_pt_translate(): more tests

Change-Id: I334a075ac2800ae4a7c4e2d6eaeb17dd8c6b09a1
---
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
2 files changed, 76 insertions(+), 2 deletions(-)


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I334a075ac2800ae4a7c4e2d6eaeb17dd8c6b09a1
Gerrit-Change-Number: 15135
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: mgcp_test: extend / rewrite test_mgcp_codec_pt_translate()

2019-08-12 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15134

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

Change subject: mgcp_test: extend / rewrite test_mgcp_codec_pt_translate()
..

mgcp_test: extend / rewrite test_mgcp_codec_pt_translate()

Instead of manually entering codec values, use mgcp_codec_add() to populate
test conns with codecs. The idea is to better test what actually happens when
parsing SDP codec strings.

Rewrite current test_mgcp_codec_pt_translate() from procedural to a data model
with human readable stdout logging.

This prepares to enable interpreting codec strings like "FOO/8000/1" as
equivalent with "FOO/8000": the SDP standard defines the final "/1", indicating
the nr of channels, as optional for a single channel, but osmo-mgw currently is
unable to match these two formats as identical. So prepare the
test_mgcp_codec_pt_translate() so that upcoming patches can incorporate strings
with and without the final "/1" by extending the struct arrays.

Change-Id: I888000d77512cfecb0f199b86ef6003e7fc0e6cb
---
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
2 files changed, 204 insertions(+), 86 deletions(-)


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I888000d77512cfecb0f199b86ef6003e7fc0e6cb
Gerrit-Change-Number: 15134
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: ptmap: implicitly match '/8000' and '/8000/1'

2019-08-12 Thread neels

Hello pespin, laforge, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15137

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

Change subject: ptmap: implicitly match  '/8000' and '/8000/1'
..

ptmap: implicitly match  '/8000' and '/8000/1'

In codecs_cmp(), do not compare the complete audio_name. The parts of it are
already checked individually:
- subtype_name ("AMR"),
- rate ("8000"; defaults to 8000 if omitted) and
- channels ("1"; defaults to 1 if omitted)
So by also checking the complete audio_name, we brushed over the match of
implicit "/8000" and "/8000/1", which otherwise works out fine.

As a result, translating payload type numbers in RTP headers now also works if
one conn of an endpoint set an rtpmap with "AMR/8000" and the other conn set
"AMR/8000/1".

It seems to me that most PBX out there generate ptmaps omitting the "/1", so
fixing this should make us more interoperable with third party SDP.

See IETF RFC4566 section 6. SDP Attributes:
  For audio streams,  indicates the number
  of audio channels.  This parameter is OPTIONAL and may be
  omitted if the number of channels is one, provided that no
  additional parameters are needed.

Also allowing to omit the "/8000" is a mere side effect of this patch.
Omitting the rate does not seem to be specified in an RFC, but is logical for
audio codecs defined to require exactly 8000 set as rate (most GSM codecs).

Add tests in mgcp_test.c.

Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2
---
M src/libosmo-mgcp/mgcp_codec.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 108 insertions(+), 2 deletions(-)


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2
Gerrit-Change-Number: 15137
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: differentiate AMR octet-aligned=0 vs =1

2019-08-12 Thread neels

Hello pespin, keith, laforge, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15136

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

Change subject: differentiate AMR octet-aligned=0 vs =1
..

differentiate AMR octet-aligned=0 vs =1

Add corresponding tests in mgcp_test.c

Change-Id: Ib8be73a7ca1b95ce794d130e8eb206dcee700124
---
M src/libosmo-mgcp/mgcp_codec.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 95 insertions(+), 2 deletions(-)


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib8be73a7ca1b95ce794d130e8eb206dcee700124
Gerrit-Change-Number: 15136
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: tweak mgcp_parse_audio_ptime_rtpmap()

2019-08-12 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15140 )

Change subject: tweak mgcp_parse_audio_ptime_rtpmap()
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/15140/1//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/15140/1//COMMIT_MSG@12
PS1, Line 12: - instead of strstr("rtpmap"), use 
osmo_str_startswith("a=rtpmap:") to more
> Does spec state there shall be no whitespace at the start of the line?
(There are no spaces at SDP line beginnings.)

The point being that 'strstr("rtpmap")' would also match these convoluted 
examples:

  o=openrtpmaptool 1565090289 1565090290 IN IP4 192.168.11.151
  s=rtpmap-test

or strstr("ptime") would even match

  s=sleeptimer

i.e. we must not match the middle of a free-format string. The only accurate 
match is at the start of a line after 'a='

  a=rtpmap:123 FOO/8000



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I730111e245da8485c1b5e8811f75d140e379cec6
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Mon, 12 Aug 2019 22:50:03 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: mgcp_codec: constify 'param' arg

2019-08-12 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15131 )

Change subject: mgcp_codec: constify 'param' arg
..

mgcp_codec: constify 'param' arg

Change-Id: I3ec6b57298f78604d5cd453f1db6d90ddfd6a2ba
---
M include/osmocom/mgcp/mgcp_codec.h
M src/libosmo-mgcp/mgcp_codec.c
2 files changed, 3 insertions(+), 3 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved
  pespin: Looks good to me, approved



diff --git a/include/osmocom/mgcp/mgcp_codec.h 
b/include/osmocom/mgcp/mgcp_codec.h
index 2bbb86e..3ead60a 100644
--- a/include/osmocom/mgcp/mgcp_codec.h
+++ b/include/osmocom/mgcp/mgcp_codec.h
@@ -2,6 +2,6 @@

 void mgcp_codec_summary(struct mgcp_conn_rtp *conn);
 void mgcp_codec_reset_all(struct mgcp_conn_rtp *conn);
-int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char 
*audio_name, struct mgcp_codec_param *param);
+int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char 
*audio_name, const struct mgcp_codec_param *param);
 int mgcp_codec_decide(struct mgcp_conn_rtp *conn);
 int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct 
mgcp_conn_rtp *conn_dst, int payload_type);
diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index e9e2c62..d5b99e8 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -101,7 +101,7 @@

 /* Set members of struct mgcp_rtp_codec, extrapolate in missing information. 
Param audio_name is expected in uppercase. */
 static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int 
payload_type, const char *audio_name,
-unsigned int pt_offset, struct mgcp_codec_param *param)
+unsigned int pt_offset, const struct mgcp_codec_param 
*param)
 {
int rate;
int channels;
@@ -242,7 +242,7 @@
  *  \param[in] audio_name audio codec name, in uppercase (e.g. "GSM/8000/1").
  *  \param[in] param optional codec parameters (set to NULL when unused).
  *  \returns 0 on success, -EINVAL on failure. */
-int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char 
*audio_name, struct mgcp_codec_param *param)
+int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char 
*audio_name, const struct mgcp_codec_param *param)
 {
int rc;


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I3ec6b57298f78604d5cd453f1db6d90ddfd6a2ba
Gerrit-Change-Number: 15131
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: merged

Change in ...osmo-mgw[master]: rename codecs_cmp() to codecs_same()

2019-08-12 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15130 )

Change subject: rename codecs_cmp() to codecs_same()
..

rename codecs_cmp() to codecs_same()

The name 'cmp' implies a return value of -1, 0, 1 to indicate smaller, match or
larger. Since this function returns bool, it should not be named with 'cmp'.

Change-Id: I2d41b1a32300e295551e85d3f9ab82dd2b0e86b8
---
M src/libosmo-mgcp/mgcp_codec.c
1 file changed, 2 insertions(+), 2 deletions(-)

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



diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 8be4c3c..e9e2c62 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -352,7 +352,7 @@

 /* Compare two codecs, all parameters must match up, except for the payload 
type
  * number. */
-static bool codecs_cmp(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec 
*codec_b)
+static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec 
*codec_b)
 {
if (codec_a->rate != codec_b->rate)
return false;
@@ -406,7 +406,7 @@
codecs_assigned = rtp_dst->codecs_assigned;
OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS);
for (i = 0; i < codecs_assigned; i++) {
-   if (codecs_cmp(codec_src, _dst->codecs[i])) {
+   if (codecs_same(codec_src, _dst->codecs[i])) {
codec_dst = _dst->codecs[i];
break;
}

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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I2d41b1a32300e295551e85d3f9ab82dd2b0e86b8
Gerrit-Change-Number: 15130
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: merged

Change in ...osmo-mgw[master]: differentiate AMR octet-aligned=0 vs =1

2019-08-12 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15136 )

Change subject: differentiate AMR octet-aligned=0 vs =1
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/15136/1/src/libosmo-mgcp/mgcp_codec.c
File src/libosmo-mgcp/mgcp_codec.c:

https://gerrit.osmocom.org/#/c/15136/1/src/libosmo-mgcp/mgcp_codec.c@361
PS1, Line 361: /* Default to octet-aligned=0, i.e. bandwidth-efficient mode */
> http://git.osmocom. […]
yes did check, will add a pointer



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib8be73a7ca1b95ce794d130e8eb206dcee700124
Gerrit-Change-Number: 15136
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Mon, 12 Aug 2019 22:42:16 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Comment-In-Reply-To: keith 
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: mgcp_codec: split codec_free() off of codec_init()

2019-08-12 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15132 )

Change subject: mgcp_codec: split codec_free() off of codec_init()
..


Patch Set 1:

(2 comments)

https://gerrit.osmocom.org/#/c/15132/1/src/libosmo-mgcp/mgcp_codec.c
File src/libosmo-mgcp/mgcp_codec.c:

https://gerrit.osmocom.org/#/c/15132/1/src/libosmo-mgcp/mgcp_codec.c@117
PS1, Line 117:  codec_init(codec);
> So in here you are not longer freeing codec->subtype_name and 
> codec->audio_name. […]
Yes: codec_set() is only used by codec_add(), and invariably it uses an 
*unpopulated* array entry.
So there could be random data in that entry and we must not free those pointers.
Instead it is important to free the memory when removing a codec entry, i.e. 
before it becomes an unpopulated array entry.

For clarity this function should probably go inside codec_add()...

yes I'll do that quickly.


https://gerrit.osmocom.org/#/c/15132/1/src/libosmo-mgcp/mgcp_codec.c@239
PS1, Line 239:  memset(codec, 0, sizeof(*codec));
> You can either drop this memset or line 96: "*codec = (struct 
> mgcp_rtp_codec){};"
nice, thx



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I120cab0a352a1e7b31c8f9c720c47b2c291311d7
Gerrit-Change-Number: 15132
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Mon, 12 Aug 2019 22:35:49 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: ptmap: implicitly match '/8000' and '/8000/1'

2019-08-12 Thread neels

Hello pespin, laforge, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15137

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

Change subject: ptmap: implicitly match  '/8000' and '/8000/1'
..

ptmap: implicitly match  '/8000' and '/8000/1'

In codecs_same(), do not compare the complete audio_name. The parts of it are
already checked individually:
- subtype_name ("AMR"),
- rate ("8000"; defaults to 8000 if omitted) and
- channels ("1"; defaults to 1 if omitted)
So by also checking the complete audio_name, we brushed over the match of
implicit "/8000" and "/8000/1", which otherwise works out fine.

As a result, translating payload type numbers in RTP headers now also works if
one conn of an endpoint set an rtpmap with "AMR/8000" and the other conn set
"AMR/8000/1".

It seems to me that most PBX out there generate ptmaps omitting the "/1", so
fixing this should make us more interoperable with third party SDP.

See IETF RFC4566 section 6. SDP Attributes:
  For audio streams,  indicates the number
  of audio channels.  This parameter is OPTIONAL and may be
  omitted if the number of channels is one, provided that no
  additional parameters are needed.

Also allowing to omit the "/8000" is a mere side effect of this patch.
Omitting the rate does not seem to be specified in an RFC, but is logical for
audio codecs defined to require exactly 8000 set as rate (most GSM codecs).

Add tests in mgcp_test.c.

Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2
---
M src/libosmo-mgcp/mgcp_codec.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 108 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/37/15137/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/15137
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2
Gerrit-Change-Number: 15137
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: mgcp_codec: split codec_free() off of codec_init()

2019-08-12 Thread neels

Hello pespin, laforge, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15132

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

Change subject: mgcp_codec: split codec_free() off of codec_init()
..

mgcp_codec: split codec_free() off of codec_init()

Both are used only in the same .c file, so make them static.

Move codec_set() guts into codec_add(): codec_set is only called by codec_add.
If codec_set were left separate, it'd look like the codec_init() is a bug and
lacks a codec_free() first. When looking at the entire context in codec_add(),
it becomes obvious that codec_init() should be called, not codec_free(),
because it is populating a previously unused entry.

Preparation to fix a memleak in a conn's codec list.

Change-Id: I120cab0a352a1e7b31c8f9c720c47b2c291311d7
---
M src/libosmo-mgcp/mgcp_codec.c
1 file changed, 35 insertions(+), 39 deletions(-)


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I120cab0a352a1e7b31c8f9c720c47b2c291311d7
Gerrit-Change-Number: 15132
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: differentiate AMR octet-aligned=0 vs =1

2019-08-12 Thread neels

Hello pespin, keith, laforge, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15136

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

Change subject: differentiate AMR octet-aligned=0 vs =1
..

differentiate AMR octet-aligned=0 vs =1

Add corresponding tests in mgcp_test.c

Change-Id: Ib8be73a7ca1b95ce794d130e8eb206dcee700124
---
M src/libosmo-mgcp/mgcp_codec.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 104 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/36/15136/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/15136
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib8be73a7ca1b95ce794d130e8eb206dcee700124
Gerrit-Change-Number: 15136
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...libosmocore[master]: osmo_tdef_get(): allow passing -1 as default timeout

2019-08-19 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/15218 )

Change subject: osmo_tdef_get(): allow passing -1 as default timeout
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/15218/2/src/tdef.c
File src/tdef.c:

https://gerrit.osmocom.org/#/c/15218/2/src/tdef.c@186
PS2, Line 186: osmo_tdef_get
> Alternatively, you could add another function without that optional 
> parameter, so it would simply cr […]
quite possible, but osmo-bsc already uses the API as it was intended, so I'd 
rather just fix it here...



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
Gerrit-Change-Number: 15218
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Mon, 19 Aug 2019 20:09:18 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment

Change in ...libosmocore[master]: osmo_tdef_get(): allow passing -1 as default timeout

2019-08-19 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/libosmocore/+/15218 )

Change subject: osmo_tdef_get(): allow passing -1 as default timeout
..

osmo_tdef_get(): allow passing -1 as default timeout

The intention of osmo_tdef_get()'s val_if_not_present argument was to return a
default timeout, or to optionally abort the program for missing timer
definitions if the default timeout is < 0. This was the case in the original
implementation of this API in osmo-bsc, but in the migration to libosmocore,
the argument was by accident changed to an unsigned type. In consequence, the
assertion in the implementation that was intended to abort the program seemed
bogus to coverity, and was fixed by removal in
I7a544d2d43b83135def296674f777e48fe5fd80a -- the wrong direction, as is obvious
from the API doc for osmo_tdef_get().

Note that osmo-bsc master passes -1 in various places and expects the
program-abort behavior that was missing from the libosmocore implementation.

Change the val_if_not_present argument to a signed type, and revert removal of
the assertion, so that passing -1 has the effect described in the API doc:
program abort on missing timer definition.

This bug was not detected because it is hard to write tests that expect a
program abort to happen, hence no tests for this API feature exist.

Related: OS#4152
Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
---
M TODO-RELEASE
M include/osmocom/core/tdef.h
M src/tdef.c
3 files changed, 4 insertions(+), 2 deletions(-)

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



diff --git a/TODO-RELEASE b/TODO-RELEASE
index 8ccfa49..665ecf7 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -7,3 +7,4 @@
 # If any interfaces have been added since the last public release: c:r:a + 1.
 # If any interfaces have been removed or changed since the last public 
release: c:r:0.
 #library   whatdescription / commit summary line
+core   osmo_tdef_get() change val_if_not_present arg from 
unsigned long to long to allow passing -1
diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h
index c8d9053..566f5dd 100644
--- a/include/osmocom/core/tdef.h
+++ b/include/osmocom/core/tdef.h
@@ -95,7 +95,7 @@

 void osmo_tdefs_reset(struct osmo_tdef *tdefs);
 unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum 
osmo_tdef_unit as_unit,
-   unsigned long val_if_not_present);
+   long val_if_not_present);
 struct osmo_tdef *osmo_tdef_get_entry(struct osmo_tdef *tdefs, int T);

 /*! Using osmo_tdef for osmo_fsm_inst: array entry for a mapping of state 
numbers to timeout definitions.
diff --git a/src/tdef.c b/src/tdef.c
index 3cfb17c..40a9900 100644
--- a/src/tdef.c
+++ b/src/tdef.c
@@ -183,10 +183,11 @@
  * \param[in] val_if_not_present  Fallback value to return if no timeout is 
defined.
  * \return Timeout value in the unit given by as_unit, rounded up if 
necessary, or val_if_not_present.
  */
-unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum 
osmo_tdef_unit as_unit, unsigned long val_if_not_present)
+unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum 
osmo_tdef_unit as_unit, long val_if_not_present)
 {
const struct osmo_tdef *t = osmo_tdef_get_entry((struct 
osmo_tdef*)tdefs, T);
if (!t) {
+   OSMO_ASSERT(val_if_not_present >= 0);
return val_if_not_present;
}
return osmo_tdef_round(t->val, t->unit, as_unit);

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
Gerrit-Change-Number: 15218
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: merged

Change in ...libosmocore[master]: add vty logp command to echo on all log targets

2019-08-19 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/14986 )

Change subject: add vty logp command to echo on all log targets
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ife5dc8999174c74e0d133729284fe526d6eaf8d9
Gerrit-Change-Number: 14986
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Mon, 19 Aug 2019 20:09:21 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...libosmocore[master]: add vty logp command to echo on all log targets

2019-08-19 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/libosmocore/+/14986 )

Change subject: add vty logp command to echo on all log targets
..

add vty logp command to echo on all log targets

When reading SUT logs resulting from TTCN3 runs, it can be hard to figure out
which log section corresponds to which test code. Add a 'logp' command on VIEW
and ENABLE nodes that simply echos an arbitrary message on log output, useful
to set markers / explanations from the TTCN3 code, which then appear in all log
outputs and can make it trivial to figure out which log section is interesting.

logging_vty_test# logp lglobal notice This is the log message
DLGLOBAL NOTICE This is the log message

>From TTCN3, could be used like this, e.g. in BSC_Tests.ttcn:

private function f_logp(charstring log_msg) runs on MSC_ConnHdlr
{
// log on TTCN3 log output
log(log_msg);
// log in stderr log
f_vty_transceive(BSCVTY, "logp lglobal notice " & log_msg);
}

...

f_logp("f_probe_for_handover(" & log_label & "): Ending the test: 
Handover Failure stops the procedure.");

Change-Id: Ife5dc8999174c74e0d133729284fe526d6eaf8d9
---
M src/vty/logging_vty.c
M tests/logging/logging_vty_test.vty
2 files changed, 94 insertions(+), 0 deletions(-)

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



diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c
index b785be4..243d6eb 100644
--- a/src/vty/logging_vty.c
+++ b/src/vty/logging_vty.c
@@ -991,6 +991,44 @@
install_element(CFG_LOG_NODE, cmd);
 }

+/* logp () (debug|...|fatal) .LOGMESSAGE*/
+DEFUN(vty_logp,
+  vty_logp_cmd,
+  NULL, /* cmdstr is dynamically set in gen_vty_logp_cmd_strs(). */
+  NULL) /* same thing for helpstr. */
+{
+   int category = log_parse_category(argv[0]);
+   int level = log_parse_level(argv[1]);
+   char *str = argv_concat(argv, argc, 2);
+   LOGP(category, level, "%s\n", str);
+   return CMD_SUCCESS;
+}
+
+static void gen_vty_logp_cmd_strs(struct cmd_element *cmd)
+{
+   char *cmd_str = NULL;
+   char *doc_str = NULL;
+
+   assert_loginfo(__func__);
+
+   OSMO_ASSERT(cmd->string == NULL);
+   OSMO_ASSERT(cmd->doc == NULL);
+
+   osmo_talloc_asprintf(tall_log_ctx, cmd_str, "logp (");
+   osmo_talloc_asprintf(tall_log_ctx, doc_str,
+"Print a message on all log outputs; useful for 
placing markers in test logs\n");
+   add_category_strings(_str, _str, osmo_log_info);
+   osmo_talloc_asprintf(tall_log_ctx, cmd_str, ") %s", LOG_LEVEL_ARGS);
+   osmo_talloc_asprintf(tall_log_ctx, doc_str, "%s", LOG_LEVEL_STRS);
+
+   osmo_talloc_asprintf(tall_log_ctx, cmd_str, " .LOGMESSAGE");
+   osmo_talloc_asprintf(tall_log_ctx, doc_str,
+"Arbitrary message to log on given category and 
log level\n");
+
+   cmd->string = cmd_str;
+   cmd->doc = doc_str;
+}
+
 /*! Register logging related commands to the VTY. Call this once from
  *  your application if you want to support those commands. */
 void logging_vty_add_cmds()
@@ -1026,6 +1064,9 @@
install_element_ve(_logging_vty_cmd);
install_element_ve(_alarms_cmd);

+   gen_vty_logp_cmd_strs(_logp_cmd);
+   install_element_ve(_logp_cmd);
+
install_node(_log_node, config_write_log);
install_element(CFG_LOG_NODE, _fltr_all_cmd);
install_element(CFG_LOG_NODE, _use_clr_cmd);
diff --git a/tests/logging/logging_vty_test.vty 
b/tests/logging/logging_vty_test.vty
index 895d2bc..d77f8ce 100644
--- a/tests/logging/logging_vty_test.vty
+++ b/tests/logging/logging_vty_test.vty
@@ -468,3 +468,56 @@
 D ERROR Log message for D on level LOGL_ERROR
 D FATAL Log message for D on level LOGL_FATAL
 DEEE FATAL Log message for DEEE on level LOGL_FATAL
+
+logging_vty_test# list
+...
+  logp 
(aa|bb|ccc||eee|lglobal|llapd|linp|lmux|lmi|lmib|lsms|lctrl|lgtp|lstats|lgsup|loap|lss7|lsccp|lsua|lm3ua|lmgcp|ljibuf|lrspro)
 (debug|info|notice|error|fatal) .LOGMESSAGE
+...
+
+logging_vty_test# logp?
+  logp  Print a message on all log outputs; useful for placing markers in test 
logs
+
+logging_vty_test# logp ?
+  aa   Antropomorphic Armadillos (AA)
+  bb   Bidirectional Breadspread (BB)
+  ccc  Chaos Communication Congress (CCC)
+   Dehydrated Dribbling Duck Dunkers ()
+  eee  Exhaustive Entropy Extraction (EEE)
+  lglobal  Library-internal global log family
+  llapdLAPD in libosmogsm
+  linp A-bis Intput Subsystem
+  lmux A-bi

Change in ...osmo-mgw[master]: fix crashes: don't assert on incoming RTP packet size

2019-08-19 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15242


Change subject: fix crashes: don't assert on incoming RTP packet size
..

fix crashes: don't assert on incoming RTP packet size

Remove various OSMO_ASSERT() on size of incoming packets. Doing an assert on
incoming data is a DoS attack vector, absolute no-go. Instead, return -EINVAL
and keep running.

Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 31 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/42/15242/1

diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index bb29d2b..d5f197d 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -475,7 +475,12 @@
uint8_t pt_in;
int pt_out;

-   OSMO_ASSERT(len >= sizeof(struct rtp_hdr));
+   if (msg->len < sizeof(struct rtp_hdr)) {
+   LOG_CONN_RTP(conn_src, LOGL_ERROR, "RTP packet too short (%u < 
%zu)\n",
+msg->len, sizeof(struct rtp_hdr));
+   return -EINVAL;
+   }
+
rtp_hdr = (struct rtp_hdr *)data;

pt_in = rtp_hdr->payload_type;
@@ -641,7 +646,7 @@
  * the receiving end expects GSM-HR data to be formated after RFC 5993, this
  * function is used to convert between RFC 5993 and TS 101318, which we 
normally
  * use. */
-static void rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int 
*len)
+static int rfc5993_hr_convert(struct mgcp_endpoint *endp, char *data, int *len)
 {
/* NOTE: *data has an overall length of RTP_BUF_SIZE, so there is
 * plenty of space available to store the slightly larger, converted
@@ -649,7 +654,12 @@

struct rtp_hdr *rtp_hdr;

-   OSMO_ASSERT(*len >= sizeof(struct rtp_hdr));
+   if (*len < sizeof(struct rtp_hdr)) {
+   LOGPENDP(endp, DRTP, LOGL_ERROR, "AMR RTP packet too short (%d 
< %zu)\n",
+*len, sizeof(struct rtp_hdr));
+   return -EINVAL;
+   }
+
rtp_hdr = (struct rtp_hdr *)data;

if (*len == GSM_HR_BYTES + sizeof(struct rtp_hdr)) {
@@ -667,7 +677,9 @@
 * packet. This is not supported yet. */
LOGPENDP(endp, DRTP, LOGL_ERROR,
 "cannot figure out how to convert RTP packet\n");
+   return -ENOTSUP;
}
+   return 0;
 }

 /* For AMR RTP two framing modes are defined RFC3267. There is a bandwith
@@ -685,7 +697,11 @@
unsigned int payload_len;
int rc;

-   OSMO_ASSERT(*len >= sizeof(struct rtp_hdr));
+   if (*len < sizeof(struct rtp_hdr)) {
+   LOGPENDP(endp, DRTP, LOGL_ERROR, "AMR RTP packet too short (%d 
< %zu)\n", *len, sizeof(struct rtp_hdr));
+   return -EINVAL;
+   }
+
rtp_hdr = (struct rtp_hdr *)data;

payload_len = *len - sizeof(struct rtp_hdr);
@@ -736,17 +752,21 @@


 /* Check if a given RTP with AMR payload for octet-aligned mode */
-static bool amr_oa_check(char *data, int len)
+static int amr_oa_check(char *data, int len)
 {
struct rtp_hdr *rtp_hdr;
unsigned int payload_len;

-   OSMO_ASSERT(len >= sizeof(struct rtp_hdr));
+   if (len < sizeof(struct rtp_hdr))
+   return -EINVAL;
+
rtp_hdr = (struct rtp_hdr *)data;

payload_len = len - sizeof(struct rtp_hdr);
+   if (payload_len < sizeof(struct amr_hdr))
+   return -EINVAL;

-   return osmo_amr_is_oa(rtp_hdr->data, payload_len);
+   return osmo_amr_is_oa(rtp_hdr->data, payload_len) ? 1 : 0;
 }

 /* Forward data to a debug tap. This is debug function that is intended for
@@ -1340,7 +1360,10 @@
 * define, then we check if the incoming payload matches that
 * expectation. */
if (amr_oa_bwe_convert_indicated(conn_src->end.codec)) {
-   if (amr_oa_check(buf, len) != 
conn_src->end.codec->param.amr_octet_aligned)
+   int oa = amr_oa_check(buf, len);
+   if (oa < 0)
+   return -1;
+   if (((bool)oa) != conn_src->end.codec->param.amr_octet_aligned)
return -1;
}


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
Gerrit-Change-Number: 15242
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-mgw[master]: mgcp_send(): stop looping on conversion error

2019-08-19 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15243


Change subject: mgcp_send(): stop looping on conversion error
..

mgcp_send(): stop looping on conversion error

If mgcp_send() runs a transcoder loop, break the loop if rfc5993_hr_convert()
or amr_oa_bwe_convert() return with error. Possibly fixes an infinite loop
situation for erratic packets? (Didn't check for that in detail.)

Change-Id: Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 13 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/43/15243/1

diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index d5f197d..190ccd4 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -871,13 +871,23 @@
 addr, buf, buflen);

if (amr_oa_bwe_convert_indicated(conn_dst->end.codec)) {
-   amr_oa_bwe_convert(endp, buf, ,
-  
conn_dst->end.codec->param.amr_octet_aligned);
+   rc = amr_oa_bwe_convert(endp, buf, ,
+   
conn_dst->end.codec->param.amr_octet_aligned);
+   if (rc < 0) {
+   LOGPENDP(endp, DRTP, LOGL_ERROR,
+"Error in AMR octet-aligned 
<-> bandwidth-efficient mode conversion\n");
+   break;
+   }
}
else if (rtp_end->rfc5993_hr_convert
&& strcmp(conn_src->end.codec->subtype_name,
  "GSM-HR-08") == 0)
-   rfc5993_hr_convert(endp, buf, );
+   rc = rfc5993_hr_convert(endp, buf, );
+   if (rc < 0) {
+   LOGPENDP(endp, DRTP, LOGL_ERROR, "Error 
while converting to GSM-HR-08\n");
+   break;
+   }
+   }

LOGPENDP(endp, DRTP, LOGL_DEBUG,
 "process/send to %s %s "

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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c
Gerrit-Change-Number: 15243
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-mgw[master]: mgcp_send(): stop looping on conversion error

2019-08-19 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15243

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

Change subject: mgcp_send(): stop looping on conversion error
..

mgcp_send(): stop looping on conversion error

If mgcp_send() runs a transcoder loop, break the loop if rfc5993_hr_convert()
or amr_oa_bwe_convert() return with error. Possibly fixes an infinite loop
situation for erratic packets? (Didn't check for that in detail.)

Change-Id: Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 14 insertions(+), 4 deletions(-)


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c
Gerrit-Change-Number: 15243
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: fix crashes: don't assert on incoming RTP packet size

2019-08-19 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15242

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

Change subject: fix crashes: don't assert on incoming RTP packet size
..

fix crashes: don't assert on incoming RTP packet size

Remove various OSMO_ASSERT() on size of incoming packets. Doing an assert on
incoming data is a DoS attack vector, absolute no-go. Instead, return -EINVAL
and keep running.

Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 28 insertions(+), 8 deletions(-)


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
Gerrit-Change-Number: 15242
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset

Change in ...osmo-msc[master]: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests

2019-08-14 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15175 )

Change subject: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests
..


Patch Set 4:

(1 comment)

https://gerrit.osmocom.org/#/c/15175/4/src/libmsc/msc_vty.c
File src/libmsc/msc_vty.c:

https://gerrit.osmocom.org/#/c/15175/4/src/libmsc/msc_vty.c@152
PS4, Line 152:
> Cosmetic: alignment.
yes I know. Am keeping the previous alignment, decided to not fix it here.



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie138f2fcb105533f7bc06a6d2e6deccf6faccc5b
Gerrit-Change-Number: 15175
Gerrit-PatchSet: 4
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 14 Aug 2019 14:35:23 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment

Change in ...osmo-msc[master]: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests

2019-08-14 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15175 )

Change subject: add 'encryption uea 1 2' cfg / fix ttcn3 iu tests
..

add 'encryption uea 1 2' cfg / fix ttcn3 iu tests

Recently, the ability to run UTRAN without encryption was added, but the config
for it was tied to the A5 GERAN encryption configuration. This affected
osmo-msc's default behavior of Iu, breaking osmo-msc ttcn3 Iu tests: the ttcn3
test suite sets A5 to 0 (no encryption) but still expects Iu to enable air
encryption. Fix this "regression".

Add a separate vty config option for UEA encryption, even if it does not
provide full granularity to select individual UEA algorithms yet.

As a result, Iu default behavior remains to enable encryption regardless of the
A5 config. UTRAN encryption can be disabled by the new cfg option
"encryption uea 0" alone.

Even though the new vty command already allows passing various combinations of
the UEA algorithm numbers, only '0' and '1 2' are accepted as valid
combinations, to reflect current osmo-msc capabilities.

Revert most changes to the msc_vlr test suite in commit "do not force
encryption on UTRAN" (I04ecd7a3b1cc603b2e3feb630e8c7c93fc36ccd7): use new
net->iu_encryption instead of net->a5_encryption_mask.

Adjust/add to test_nodes.vty transcript tests.

Related: OS#4144
Change-Id: Ie138f2fcb105533f7bc06a6d2e6deccf6faccc5b
---
M doc/manuals/chapters/net.adoc
M include/osmocom/msc/gsm_data.h
M src/libmsc/gsm_04_08.c
M src/libmsc/msc_net_init.c
M src/libmsc/msc_vty.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.c
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_tests.h
M tests/test_nodes.vty
10 files changed, 153 insertions(+), 62 deletions(-)

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



diff --git a/doc/manuals/chapters/net.adoc b/doc/manuals/chapters/net.adoc
index 4bf34a3..6edb9ee 100644
--- a/doc/manuals/chapters/net.adoc
+++ b/doc/manuals/chapters/net.adoc
@@ -188,11 +188,22 @@

 While authentication is always required on 3G, ciphering is optional.

-So far OsmoMSC lacks explicit configuration for ciphering on 3G. As an interim
-solution, ciphering is enabled on 3G exactly when ciphering is enabled on 2G,
-i.e. when any cipher other than A5/0 is enabled in the configuration. If only
-A5/0 is configured, ciphering will be disabled on both 2G and 3G. The future
-aim is to add comprehensive configuration for 3G ciphering that is independent
-from the 2G setting.
+So far OsmoMSC allows switching ciphering on 3G either on or off -- the default
+behavior is to enable ciphering. (Individual choice of algorithms may be added
+in the future.)
+
+Disable 3G ciphering:
+
+
+network
+ encryption uea 0
+
+
+Enable 3G ciphering (default):
+
+
+network
+ encryption uea 1 2
+

 OsmoMSC indicates UEA1 and UEA2 as permitted encryption algorithms on 3G.
diff --git a/include/osmocom/msc/gsm_data.h b/include/osmocom/msc/gsm_data.h
index e926b3f..a90b732 100644
--- a/include/osmocom/msc/gsm_data.h
+++ b/include/osmocom/msc/gsm_data.h
@@ -149,6 +149,11 @@
bool authentication_required;
int send_mm_info;

+   /* Whether to use encryption on UTRAN.
+* TODO: we should offer a choice of UEA1 and/or UEA2, and probably 
replace this bool with a bit-mask of
+* permitted Iu encryption algorithms. See also OS#4143 and the 
'encryption uea' vty command. */
+   bool uea_encryption;
+
struct rate_ctr_group *msc_ctrs;
struct osmo_stat_item_group *statg;

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index cd37cff..086116f 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -375,7 +375,7 @@
net->vlr, msc_a, vlr_lu_type, tmsi, imsi,
_lai, _a->via_cell.lai,
is_utran || net->authentication_required,
-   net->a5_encryption_mask > 0x01,
+   is_utran ? net->uea_encryption : 
net->a5_encryption_mask > 0x01,
lu->key_seq,
osmo_gsm48_classmark1_is_r99(>classmark1),
is_utran,
@@ -780,7 +780,7 @@
 req->cm_service_type,
 mi-1, _a->via_cell.lai,
 is_utran || net->authentication_required,
-net->a5_encryption_mask > 0x01,
+is_utran ? net->uea_encryption : 
net->a5_encryption_mask > 0x01,
 req->cipher_key_seq,
 osmo_

Change in ...osmo-mgw[master]: fix crashes: don't assert on incoming RTP packet size

2019-08-21 Thread neels

Hello pespin, Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-mgw/+/15242

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

Change subject: fix crashes: don't assert on incoming RTP packet size
..

fix crashes: don't assert on incoming RTP packet size

Remove various OSMO_ASSERT() on size of incoming packets. Doing an assert on
incoming data is a DoS attack vector, absolute no-go. Instead, return -EINVAL
and keep running.

Change some return values to be able to distinguish successful operation from
invalid RTP sizes. In rtp_data_net(), make sure to return negative if the RTP
packet was invalid.

Some of the error return codes implemented here will only be used in upcoming
patch Iba115a0b1d74e7cefba5dcdd777e98ddea9eba8c.

Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 32 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/42/15242/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/15242
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
Gerrit-Change-Number: 15242
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-mgw[master]: fix crashes: don't assert on incoming RTP packet size

2019-08-21 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15242 )

Change subject: fix crashes: don't assert on incoming RTP packet size
..


Patch Set 2:

(2 comments)

https://gerrit.osmocom.org/#/c/15242/2/src/libosmo-mgcp/mgcp_network.c
File src/libosmo-mgcp/mgcp_network.c:

https://gerrit.osmocom.org/#/c/15242/2/src/libosmo-mgcp/mgcp_network.c@752
PS2, Line 752: static int amr_oa_check(char *data, int len)
> non-related. […]
if you return true or false, it always says whether octet-aligned is on or off. 
I need a third value saying that the RTP header was invalid, so ... should add 
an api doc


https://gerrit.osmocom.org/#/c/15242/2/src/libosmo-mgcp/mgcp_network.c@1363
PS2, Line 1363: if (((bool)oa) != 
conn_src->end.codec->param.amr_octet_aligned)
> non-related change.
it is related: instead of asserting on invalid header size, I need to handle 
that error by returning -1



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6bc6ee950ce07bcc2c585c30fad02b81153bdde2
Gerrit-Change-Number: 15242
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 21 Aug 2019 20:49:36 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in ...libosmocore[master]: fix: vty crash by logging to killed telnet session

2019-08-27 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/libosmocore/+/15265

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

Change subject: fix: vty crash by logging to killed telnet session
..

fix: vty crash by logging to killed telnet session

When a telnet session dies (e.g. killall telnet) and also has logging enabled,
the closing of the telnet session logs to the killed telnet session and
segfaults: the vty->obuf is already NULL.

In vty_out(), guard against this situation by not composing an output if
vty->obuf is NULL.

Also guard all buffer_*() functions against a NULL buffer argument, which
should catch all other hypothetical code paths trying to add to a closed
vty->obuf.

Related: OS#4164
Change-Id: Idca3f54dc986abf6784790c12e69e02bdf77cb41
---
M src/vty/buffer.c
M src/vty/vty.c
2 files changed, 35 insertions(+), 3 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Idca3f54dc986abf6784790c12e69e02bdf77cb41
Gerrit-Change-Number: 15265
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-msc[master]: ran_dec logging: log message sizes on errors

2019-08-30 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15342 )

Change subject: ran_dec logging: log message sizes on errors
..


Patch Set 1:

The --enable-iu build only warns: "ran_msg_a.c:763:3: warning: format ‘%u’ 
expects..."
The --disable-iu build errors: "ran_msg_a.c:763:3: error: format ‘%u’ 
expects..."

I see, it's actually done on purpose in the jenkins.sh, I guess to avoid 
breaking on the million RANAP warnings from the generated code.


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Id08e4ee5a4dbf552dbb107d8f0519110664f6acb
Gerrit-Change-Number: 15342
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Fri, 30 Aug 2019 12:21:02 +
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...osmo-msc[master]: ran_dec logging: log message sizes on errors

2019-08-30 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15342 )

Change subject: ran_dec logging: log message sizes on errors
..


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/15342/1/src/libmsc/ran_msg_a.c
File src/libmsc/ran_msg_a.c:

https://gerrit.osmocom.org/#/c/15342/1/src/libmsc/ran_msg_a.c@763
PS1, Line 763: ,
that's weird: there is definitely a string format bug in this patch, i.e. a 
comma is separating two parts of the string format, yet this error is only 
caught by the --disable-iu build, and the --enable-iu builds fine?? How is that 
possible!?



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Id08e4ee5a4dbf552dbb107d8f0519110664f6acb
Gerrit-Change-Number: 15342
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Fri, 30 Aug 2019 12:18:09 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...libosmocore[master]: OSMO_SOCKADDR_STR_FMT_ARGS: remove useless condition

2019-08-30 Thread neels

Hello fixeria, laforge, Jenkins Builder,

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

https://gerrit.osmocom.org/c/libosmocore/+/15340

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

Change subject: OSMO_SOCKADDR_STR_FMT_ARGS: remove useless condition
..

OSMO_SOCKADDR_STR_FMT_ARGS: remove useless condition

Since (R)->ip is a char[], it is always non-NULL. The (x ? : "") condition is
completely pointless. Remove it.

Change-Id: I13ed06776a784cfa99bbdfca2bb4dfe12913a1ec
---
M include/osmocom/core/sockaddr_str.h
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/40/15340/3
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15340
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I13ed06776a784cfa99bbdfca2bb4dfe12913a1ec
Gerrit-Change-Number: 15340
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-MessageType: newpatchset

Change in ...osmo-msc[master]: ran_dec logging: log message sizes on errors

2019-08-30 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15342 )

Change subject: ran_dec logging: log message sizes on errors
..


Patch Set 1:

> Patch Set 1:
>
> Compilation warnings, jenkins fails.

I can see that :)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Id08e4ee5a4dbf552dbb107d8f0519110664f6acb
Gerrit-Change-Number: 15342
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Fri, 30 Aug 2019 12:15:03 +
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: mgcp_codec: split codec_free() off of codec_init()

2019-08-27 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15132 )

Change subject: mgcp_codec: split codec_free() off of codec_init()
..

mgcp_codec: split codec_free() off of codec_init()

Both are used only in the same .c file, so make them static.

Move codec_set() guts into codec_add(): codec_set is only called by codec_add.
If codec_set were left separate, it'd look like the codec_init() is a bug and
lacks a codec_free() first. When looking at the entire context in codec_add(),
it becomes obvious that codec_init() should be called, not codec_free(),
because it is populating a previously unused entry.

Preparation to fix a memleak in a conn's codec list.

Change-Id: I120cab0a352a1e7b31c8f9c720c47b2c291311d7
---
M src/libosmo-mgcp/mgcp_codec.c
1 file changed, 35 insertions(+), 39 deletions(-)

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



diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index d5b99e8..30c185c 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -76,18 +76,24 @@
 }

 /* Initalize or reset codec information with default data. */
-void codec_init(struct mgcp_rtp_codec *codec)
+static void codec_init(struct mgcp_rtp_codec *codec)
+{
+   *codec = (struct mgcp_rtp_codec){
+   .payload_type = -1,
+   .frame_duration_num = DEFAULT_RTP_AUDIO_FRAME_DUR_NUM,
+   .frame_duration_den = DEFAULT_RTP_AUDIO_FRAME_DUR_DEN,
+   .rate = DEFAULT_RTP_AUDIO_DEFAULT_RATE,
+   .channels = DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS,
+   };
+}
+
+static void codec_free(struct mgcp_rtp_codec *codec)
 {
if (codec->subtype_name)
talloc_free(codec->subtype_name);
if (codec->audio_name)
talloc_free(codec->audio_name);
-   memset(codec, 0, sizeof(*codec));
-   codec->payload_type = -1;
-   codec->frame_duration_num = DEFAULT_RTP_AUDIO_FRAME_DUR_NUM;
-   codec->frame_duration_den = DEFAULT_RTP_AUDIO_FRAME_DUR_DEN;
-   codec->rate = DEFAULT_RTP_AUDIO_DEFAULT_RATE;
-   codec->channels = DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS;
+   *codec = (struct mgcp_rtp_codec){};
 }

 /*! Initalize or reset codec information with default data.
@@ -99,13 +105,30 @@
conn->end.codec = NULL;
 }

-/* Set members of struct mgcp_rtp_codec, extrapolate in missing information. 
Param audio_name is expected in uppercase. */
-static int codec_set(void *ctx, struct mgcp_rtp_codec *codec, int 
payload_type, const char *audio_name,
-unsigned int pt_offset, const struct mgcp_codec_param 
*param)
+/*! Add codec configuration depending on payload type and/or codec name. This
+ *  function uses the input parameters to extrapolate the full codec 
information.
+ *  \param[out] codec configuration (caller provided memory).
+ *  \param[out] conn related rtp-connection.
+ *  \param[in] payload_type codec type id (e.g. 3 for GSM, -1 when undefined).
+ *  \param[in] audio_name audio codec name, in uppercase (e.g. "GSM/8000/1").
+ *  \param[in] param optional codec parameters (set to NULL when unused).
+ *  \returns 0 on success, -EINVAL on failure. */
+int mgcp_codec_add(struct mgcp_conn_rtp *conn, int payload_type, const char 
*audio_name, const struct mgcp_codec_param *param)
 {
int rate;
int channels;
char audio_codec[64];
+   struct mgcp_rtp_codec *codec;
+   unsigned int pt_offset = conn->end.codecs_assigned;
+   void *ctx = conn->conn;
+
+   /* The amount of codecs we can store is limited, make sure we do not
+* overrun this limit. */
+   if (conn->end.codecs_assigned >= MGCP_MAX_CODECS)
+   return -EINVAL;
+
+   /* First unused entry */
+   codec = >end.codecs[conn->end.codecs_assigned];

/* Initalize the codec struct with some default data to begin with */
codec_init(codec);
@@ -226,41 +249,14 @@
} else
codec->param_present = false;

+   conn->end.codecs_assigned++;
return 0;
 error:
/* Make sure we leave a clean codec entry on error. */
-   codec_init(codec);
-   memset(codec, 0, sizeof(*codec));
+   codec_free(codec);
return -EINVAL;
 }

-/*! Add codec configuration depending on payload type and/or codec name. This
- *  function uses the input parameters to extrapolate the full codec 
information.
- *  \param[out] codec configuration (caller provided memory).
- *  \param[out] conn related rtp-connection.
- *  \param[in] payload_type codec type id (e.g. 3 for GSM, -1 when undefined).
- *  \param[in] audio_name audio codec name, in uppercase (e.g. "GSM/8000/1").
- *  \param[in] param optional codec parameters (set to

Change in ...osmo-mgw[master]: fix memleak: actually free strings in mgcp_codec_reset_all()

2019-08-27 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15133 )

Change subject: fix memleak: actually free strings in mgcp_codec_reset_all()
..

fix memleak: actually free strings in mgcp_codec_reset_all()

The audio_name and subtype_name are allocated from talloc, so they need to be
freed before resetting the codec array. Use mgcp_codec_free() to ensure this.

Change-Id: I07f207dcb7ce66bbf3445a30af41e696677b384f
---
M src/libosmo-mgcp/mgcp_codec.c
1 file changed, 3 insertions(+), 1 deletion(-)

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



diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 30c185c..7f1a6d1 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -100,7 +100,9 @@
  *  \param[out] conn related rtp-connection. */
 void mgcp_codec_reset_all(struct mgcp_conn_rtp *conn)
 {
-   memset(conn->end.codecs, 0, sizeof(conn->end.codecs));
+   int i;
+   for (i = 0; i < conn->end.codecs_assigned; i++)
+   codec_free(>end.codecs[i]);
conn->end.codecs_assigned = 0;
conn->end.codec = NULL;
 }

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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I07f207dcb7ce66bbf3445a30af41e696677b384f
Gerrit-Change-Number: 15133
Gerrit-PatchSet: 3
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: merged

Change in ...osmo-mgw[master]: mgcp_codec: split codec_free() off of codec_init()

2019-08-27 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15132 )

Change subject: mgcp_codec: split codec_free() off of codec_init()
..


Patch Set 2: Code-Review+2

now I notice there are actually two +1s I can combine


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I120cab0a352a1e7b31c8f9c720c47b2c291311d7
Gerrit-Change-Number: 15132
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 27 Aug 2019 21:52:50 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: mgcp_codec: codec_set(): log about all possible errors

2019-08-27 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15138 )

Change subject: mgcp_codec: codec_set(): log about all possible errors
..


Patch Set 4:

(1 comment)

https://gerrit.osmocom.org/#/c/15138/4/src/libosmo-mgcp/mgcp_codec.c
File src/libosmo-mgcp/mgcp_codec.c:

https://gerrit.osmocom.org/#/c/15138/4/src/libosmo-mgcp/mgcp_codec.c@182
PS4, Line 182:  if (strlen(audio_name) > sizeof(audio_codec)) {
> this should probably be >=
unrelated to this patch, which is just about logging. fixing separately.



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I0b44b574c814882b6f8ae7cd738a6f481cd721fd
Gerrit-Change-Number: 15138
Gerrit-PatchSet: 4
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 28 Aug 2019 02:55:42 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: SDP: store all ptmap entries

2019-08-27 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15141 )

Change subject: SDP: store all ptmap entries
..


Patch Set 5: Code-Review+2

combining votes


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I2a69c21e68c602daf804744212d335ab1eafd81b
Gerrit-Change-Number: 15141
Gerrit-PatchSet: 5
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 28 Aug 2019 03:01:40 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-mgw[master]: mgcp_codec_add: fix audio_name size check

2019-08-27 Thread neels

neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15301


Change subject: mgcp_codec_add: fix audio_name size check
..

mgcp_codec_add: fix audio_name size check

Needs to account for terminating '\0'.

Change-Id: I27896beef6ffcc1cb6207daaba6c8b2b03eb513d
---
M src/libosmo-mgcp/mgcp_codec.c
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/01/15301/1

diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 704b7e6..9e55ab0 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -179,7 +179,7 @@
/* Now we extract the codec subtype name, rate and channels. The latter
 * two are optional. If they are not present we use the safe defaults
 * above. */
-   if (strlen(audio_name) > sizeof(audio_codec)) {
+   if (strlen(audio_name) >= sizeof(audio_codec)) {
LOGP(DLMGCP, LOGL_ERROR, "Audio codec too long: %s\n", 
osmo_quote_str(audio_name, -1));
goto error;
}

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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I27896beef6ffcc1cb6207daaba6c8b2b03eb513d
Gerrit-Change-Number: 15301
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange

Change in ...osmo-mgw[master]: ptmap: implicitly match '/8000' and '/8000/1'

2019-08-27 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/15137 )

Change subject: ptmap: implicitly match  '/8000' and '/8000/1'
..


Patch Set 4: Code-Review+2

combining votes


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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2
Gerrit-Change-Number: 15137
Gerrit-PatchSet: 4
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 28 Aug 2019 02:53:09 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-msc[master]: ran_dec logging: log message sizes on errors

2019-08-30 Thread neels

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15342 )

Change subject: ran_dec logging: log message sizes on errors
..


Patch Set 1:

interesting to note that my gcc seems to not warn about this at all.


gcc -DHAVE_CONFIG_H -I. -I../../../../src/osmo-msc/src/libmsc -I../..   
-I../../../../src/osmo-msc/include -I../..  -fsanitize=address 
-fsanitize=undefined -Werror -Wno-error=deprecated 
-Wno-error=deprecated-declarations -Wno-error=cpp -Wall -I/usr/local/include/ 
-I/usr/local/include/ -I/usr/local/include/  -I/usr/local/include/ 
-I/usr/local/include/ -I/usr/local/include/asn1c -I/usr/local/include/ 
-I/usr/local/include/ -I/usr/local/include/ -I/usr/local/include/ 
-I/usr/local/include/ -I/usr/local/include/  -g -O0 -fsanitize=address 
-fsanitize=undefined -Werror -Wno-error=deprecated 
-Wno-error=deprecated-declarations -Wno-error=cpp -Werror=implicit 
-Werror=maybe-uninitialized -Werror=memset-transposed-args -Wnull-dereference 
-Werror=sizeof-array-argument -Werror=sizeof-pointer-memaccess -MT ran_msg_a.o 
-MD -MP -MF .deps/ran_msg_a.Tpo -c -o ran_msg_a.o 
../../../../src/osmo-msc/src/libmsc/ran_msg_a.c
mv -f .deps/ran_msg_a.Tpo .deps/ran_msg_a.Po


gcc (Debian 8.3.0-6) 8.3.0


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Id08e4ee5a4dbf552dbb107d8f0519110664f6acb
Gerrit-Change-Number: 15342
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: pespin 
Gerrit-Comment-Date: Fri, 30 Aug 2019 12:30:05 +
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in ...osmo-msc[master]: ran_dec logging: log message sizes on errors

2019-08-30 Thread neels

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/c/osmo-msc/+/15342

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

Change subject: ran_dec logging: log message sizes on errors
..

ran_dec logging: log message sizes on errors

Change-Id: Id08e4ee5a4dbf552dbb107d8f0519110664f6acb
---
M src/libmsc/ran_msg_a.c
1 file changed, 7 insertions(+), 3 deletions(-)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Id08e4ee5a4dbf552dbb107d8f0519110664f6acb
Gerrit-Change-Number: 15342
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-CC: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-msc[master]: msc_a fsm: ignore state chg to same state

2019-09-03 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15389 )

Change subject: msc_a fsm: ignore state chg to same state
..

msc_a fsm: ignore state chg to same state

We sometimes see errors like

   libmsc/msc_a.c:361 msc_a(...){MSC_A_ST_RELEASING}: transition to state 
MSC_A_ST_RELEASING not permitted!

i.e. changing state to the state msc_a is already in.

Ignore re-entering the same state for most state changes. However, there is one
state change in msc_a where re-entering the MSC_A_ST_VALIDATE_L3 is necessary
to start the timeout.

Hence add msc_a_state_chg_always() and use that for re-entering
MSC_A_ST_VALIDATE_L3. Change msc_a_state_chg() to skip no-op state changes.

This should silence all no-op state change error messages for msc_a.

Related: OS#4169
Change-Id: I0c74c10b5fa7bbdd6ae3674926cc0393edf15a35
---
M src/libmsc/msc_a.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_hlr_timeout.err
M tests/msc_vlr/msc_vlr_test_ms_timeout.err
5 files changed, 8 insertions(+), 13 deletions(-)

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



diff --git a/src/libmsc/msc_a.c b/src/libmsc/msc_a.c
index a082cb8..b414574 100644
--- a/src/libmsc/msc_a.c
+++ b/src/libmsc/msc_a.c
@@ -63,9 +63,15 @@
 /* Transition to a state, using the T timer defined in msc_a_fsm_timeouts.
  * The actual timeout value is in turn obtained from network->T_defs.
  * Assumes local variable fi exists. */
-#define msc_a_state_chg(msc_a, state) \
+#define msc_a_state_chg_always(msc_a, state) \
osmo_tdef_fsm_inst_state_chg((msc_a)->c.fi, state, msc_a_fsm_timeouts, 
(msc_a)->c.ran->tdefs, 5)

+/* Same as msc_a_state_chg_always() but ignore if the msc_a already is in the 
target state. */
+#define msc_a_state_chg(msc_a, STATE) do { \
+   if ((msc_a)->c.fi->state != STATE) \
+   msc_a_state_chg_always(msc_a, STATE); \
+   } while(0)
+
 struct gsm_network *msc_a_net(const struct msc_a *msc_a)
 {
return msub_net(msc_a->c.msub);
@@ -1036,7 +1042,7 @@
};
osmo_use_count_make_static_entries(_a->use_count, 
msc_a->use_count_buf, ARRAY_SIZE(msc_a->use_count_buf));
/* Start timeout for first state */
-   msc_a_state_chg(msc_a, MSC_A_ST_VALIDATE_L3);
+   msc_a_state_chg_always(msc_a, MSC_A_ST_VALIDATE_L3);
return msc_a;
 }

diff --git a/tests/msc_vlr/msc_vlr_test_gsm_authen.err 
b/tests/msc_vlr/msc_vlr_test_gsm_authen.err
index c9be4ef..4905881 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_authen.err
+++ b/tests/msc_vlr/msc_vlr_test_gsm_authen.err
@@ -1926,7 +1926,6 @@
 DMSC msc_a(IMSI-90170004620:MSISDN-46071:GERAN-A:LU){MSC_A_ST_RELEASING}: 
RAN encode: CLEAR_COMMAND on GERAN-A
 DMSC dummy_msc_i(IMSI-90170004620:MSISDN-46071:GERAN-A:LU){0}: Received 
Event MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST
 DREF VLR subscr IMSI-90170004620:MSISDN-46071 - 
msc_a_fsm_releasing_onenter: now used by 2 (active-conn,vlr_gsup_rx)
-DMSC msc_a(IMSI-90170004620:MSISDN-46071:GERAN-A:LU){MSC_A_ST_RELEASING}: 
transition to state MSC_A_ST_RELEASING not permitted!
 DREF VLR subscr IMSI-90170004620:MSISDN-46071 - vlr_gsup_rx: now used by 1 
(active-conn)
 <-- GSUP rx OSMO_GSUP_MSGT_CHECK_IMEI_RESULT: vlr_gsupc_read_cb() returns 0
 msc_a_is_accepted() == false
@@ -2192,7 +2191,6 @@
 DMSC msc_a(IMSI-90170004620:MSISDN-46071:GERAN-A:LU){MSC_A_ST_RELEASING}: 
RAN encode: CLEAR_COMMAND on GERAN-A
 DMSC dummy_msc_i(IMSI-90170004620:MSISDN-46071:GERAN-A:LU){0}: Received 
Event MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST
 DREF VLR subscr IMSI-90170004620:MSISDN-46071 - 
msc_a_fsm_releasing_onenter: now used by 2 (active-conn,vlr_gsup_rx)
-DMSC msc_a(IMSI-90170004620:MSISDN-46071:GERAN-A:LU){MSC_A_ST_RELEASING}: 
transition to state MSC_A_ST_RELEASING not permitted!
 DREF VLR subscr IMSI-90170004620:MSISDN-46071 - vlr_gsup_rx: now used by 1 
(active-conn)
 <-- GSUP rx OSMO_GSUP_MSGT_CHECK_IMEI_ERROR: vlr_gsupc_read_cb() returns 0
 msc_a_is_accepted() == false
diff --git a/tests/msc_vlr/msc_vlr_test_hlr_reject.err 
b/tests/msc_vlr/msc_vlr_test_hlr_reject.err
index 9d0737a..e4ea226 100644
--- a/tests/msc_vlr/msc_vlr_test_hlr_reject.err
+++ b/tests/msc_vlr/msc_vlr_test_hlr_reject.err
@@ -65,7 +65,6 @@
 DMSC msc_a(IMSI-90170004620:GERAN-A:LU){MSC_A_ST_RELEASING}: RAN encode: 
CLEAR_COMMAND on GERAN-A
 DMSC dummy_msc_i(IMSI-90170004620:GERAN-A:LU){0}: Received Event 
MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST
 DREF VLR subscr IMSI-90170004620 - msc_a_fsm_releasing_onenter: now used 
by 2 (active-conn,vlr_gsup_rx)
-DMSC msc_a(IMSI-90170004620:GERAN-A:LU){MSC_A

Change in ...osmo-msc[master]: ran_dec logging: log message sizes on errors

2019-09-03 Thread neels

neels has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/15342 )

Change subject: ran_dec logging: log message sizes on errors
..

ran_dec logging: log message sizes on errors

Change-Id: Id08e4ee5a4dbf552dbb107d8f0519110664f6acb
---
M src/libmsc/ran_msg_a.c
1 file changed, 7 insertions(+), 3 deletions(-)

Approvals:
  fixeria: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/libmsc/ran_msg_a.c b/src/libmsc/ran_msg_a.c
index fd8afdc..43e27f6 100644
--- a/src/libmsc/ran_msg_a.c
+++ b/src/libmsc/ran_msg_a.c
@@ -761,13 +761,17 @@
}

if (msgb_l3len(bssmap) < h->length) {
-   LOG_RAN_A_DEC(ran_dec, LOGL_ERROR, "BSSMAP data truncated, 
discarding message\n");
+   LOG_RAN_A_DEC(ran_dec, LOGL_ERROR, "BSSMAP data truncated, 
discarding message:"
+ " msgb_l3len(bssmap) == %u < 
bssmap_header->length == %u\n",
+ msgb_l3len(bssmap), h->length);
return -1;
}

if (msgb_l3len(bssmap) > h->length) {
-   LOG_RAN_A_DEC(ran_dec, LOGL_NOTICE, "There are %u extra bytes 
after the BSSMAP data, truncating\n",
-msgb_l3len(bssmap) - h->length);
+   LOG_RAN_A_DEC(ran_dec, LOGL_NOTICE, "There are %u extra bytes 
after the BSSMAP data, truncating:"
+ " msgb_l3len(bssmap) == %u > 
bssmap_header->length == %u\n",
+ msgb_l3len(bssmap) - h->length,
+ msgb_l3len(bssmap), h->length);
msgb_l3trim(bssmap, h->length);
}


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Id08e4ee5a4dbf552dbb107d8f0519110664f6acb
Gerrit-Change-Number: 15342
Gerrit-PatchSet: 6
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: merged

1 2 3 4 5 6 7 8 9 10 >

1 - 100 of 13624 matches

Mail list logo