Change in osmo-sgsn[master]: Store GSN address in libosmocore struct

2018-12-18 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12352


Change subject: Store GSN address in libosmocore struct
..

Store GSN address in libosmocore struct

That's automated code change made using following program:
//spatch --in-place --sp-file dup.spatch -I include --dir ./ --all-includes
@@
@@
struct
- gsn_addr
+ osmo_gsn_address

@@
struct osmo_gsn_address a;
@@
(
a.
- len
+ length
|
a.
- buf
+ addr
)

Note that --all-includes is necessary because affected functions are
defined and implemented in files with different subdirectory names under
include and src correspondingly.

Change-Id: I6ed32a91483dc608c47df77869033a6e891e9e6a
---
M include/osmocom/sgsn/gtphub.h
M src/gprs/gtphub.c
M src/gprs/gtphub_ares.c
M tests/gtphub/gtphub_test.c
4 files changed, 61 insertions(+), 53 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/52/12352/1

diff --git a/include/osmocom/sgsn/gtphub.h b/include/osmocom/sgsn/gtphub.h
index 8fd9f38..7afe83c 100644
--- a/include/osmocom/sgsn/gtphub.h
+++ b/include/osmocom/sgsn/gtphub.h
@@ -163,22 +163,25 @@
uint8_t buf[16];
 };

-void gsn_addr_copy(struct gsn_addr *gsna, const struct gsn_addr *src);
-int gsn_addr_from_str(struct gsn_addr *gsna, const char *numeric_addr_str);
+void gsn_addr_copy(struct osmo_gsn_address *gsna,
+  const struct osmo_gsn_address *src);
+int gsn_addr_from_str(struct osmo_gsn_address *gsna,
+ const char *numeric_addr_str);

 /* Return gsna in numeric string form, in a static buffer. */
-const char *gsn_addr_to_str(const struct gsn_addr *gsna);
+const char *gsn_addr_to_str(const struct osmo_gsn_address *gsna);

 /* note: strbuf_len doesn't need to be larger than INET6_ADDRSTRLEN + 1. */
-const char *gsn_addr_to_strb(const struct gsn_addr *gsna,
+const char *gsn_addr_to_strb(const struct osmo_gsn_address *gsna,
 char *strbuf, int strbuf_len);

 /* Return 1 on match, zero otherwise. */
-int gsn_addr_same(const struct gsn_addr *a, const struct gsn_addr *b);
+int gsn_addr_same(const struct osmo_gsn_address *a,
+ const struct osmo_gsn_address *b);

 /* Decode sa to gsna. Return 0 on success. If port is non-NULL, the port number
  * from sa is also returned. */
-int gsn_addr_from_sockaddr(struct gsn_addr *gsna, uint16_t *port,
+int gsn_addr_from_sockaddr(struct osmo_gsn_address *gsna, uint16_t *port,
   const struct osmo_sockaddr *sa);

 /* expiry */
@@ -379,7 +382,7 @@
struct llist_head entry;

struct gtphub_peer *peer;
-   struct gsn_addr addr;
+   struct osmo_gsn_address addr;
struct llist_head ports;
 };

@@ -411,7 +414,7 @@
 };

 struct gtphub_bind {
-   struct gsn_addr local_addr;
+   struct osmo_gsn_address local_addr;
uint16_t local_port;
struct osmo_fd ofd;

@@ -506,14 +509,14 @@

 struct gtphub_peer_port *gtphub_port_have(struct gtphub *hub,
  struct gtphub_bind *bind,
- const struct gsn_addr *addr,
+ const struct osmo_gsn_address *addr,
  uint16_t port);

 struct gtphub_peer_port *gtphub_port_find_sa(const struct gtphub_bind *bind,
 const struct osmo_sockaddr *addr);

 void gtphub_resolved_ggsn(struct gtphub *hub, const char *apn_oi_str,
- struct gsn_addr *resolved_addr,
+ struct osmo_gsn_address *resolved_addr,
  time_t now);

 const char *gtphub_port_str(struct gtphub_peer_port *port);
diff --git a/src/gprs/gtphub.c b/src/gprs/gtphub.c
index ca5857b..a17306e 100644
--- a/src/gprs/gtphub.c
+++ b/src/gprs/gtphub.c
@@ -161,12 +161,13 @@
}
 }

-void gsn_addr_copy(struct gsn_addr *gsna, const struct gsn_addr *src)
+void gsn_addr_copy(struct osmo_gsn_address *gsna,
+  const struct osmo_gsn_address *src)
 {
*gsna = *src;
 }

-int gsn_addr_from_sockaddr(struct gsn_addr *gsna, uint16_t *port,
+int gsn_addr_from_sockaddr(struct osmo_gsn_address *gsna, uint16_t *port,
   const struct osmo_sockaddr *sa)
 {
char addr_str[256];
@@ -185,23 +186,24 @@
return gsn_addr_from_str(gsna, addr_str);
 }

-int gsn_addr_from_str(struct gsn_addr *gsna, const char *numeric_addr_str)
+int gsn_addr_from_str(struct osmo_gsn_address *gsna,
+ const char *numeric_addr_str)
 {
if ((!gsna) || (!numeric_addr_str))
return -1;

int af = AF_INET;
-   gsna->len = 4;
+   gsna->length = 4;
const char *pos = numeric_addr_str;
for (; *pos; pos++) {
if (*pos == ':') {
af = AF_INET6;
-   gsna->len = 16;
+  

Change in osmo-msc[master]: Use proper type for tch_rtp_connect() parameter

2018-12-18 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12330 )

Change subject: Use proper type for tch_rtp_connect() parameter
..

Use proper type for tch_rtp_connect() parameter

Change-Id: I6e2efcd2e25d6ec2ff35a4b8cfcda02abe97fa59
---
M src/libmsc/gsm_04_08_cc.c
1 file changed, 1 insertion(+), 2 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Max: Looks good to me, approved
  Stefan Sperling: Looks good to me, but someone else must approve
  Pau Espin Pedrol: Looks good to me, but someone else must approve



diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index 15c6d9d..5500f6f 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -1761,10 +1761,9 @@
return 0;
 }

-static int tch_rtp_connect(struct gsm_network *net, void *arg)
+static int tch_rtp_connect(struct gsm_network *net, struct gsm_mncc_rtp *rtp)
 {
struct gsm_trans *trans;
-   struct gsm_mncc_rtp *rtp = arg;
struct in_addr addr;

/* FIXME: in *rtp we should get the codec information of the remote

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6e2efcd2e25d6ec2ff35a4b8cfcda02abe97fa59
Gerrit-Change-Number: 12330
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: osmith 


Change in osmo-msc[master]: Use proper type for tch_rtp_connect() parameter

2018-12-18 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12330 )

Change subject: Use proper type for tch_rtp_connect() parameter
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e2efcd2e25d6ec2ff35a4b8cfcda02abe97fa59
Gerrit-Change-Number: 12330
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Tue, 18 Dec 2018 17:12:50 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: Use msgb helper instead of local #define for debug print

2018-12-18 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 )

Change subject: Use msgb helper instead of local #define for debug print
..


Patch Set 2:

This change is ready for review.


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b
Gerrit-Change-Number: 12271
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Tue, 18 Dec 2018 16:27:43 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: cleanup: remove unused define

2018-12-18 Thread Max
Max has abandoned this change. ( https://gerrit.osmocom.org/12272 )

Change subject: cleanup: remove unused define
..


Abandoned

Superseded by new revision of parent patch.
--
To view, visit https://gerrit.osmocom.org/12272
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ibff603dd581f60b600f2469ad464a0bf77e24bfe
Gerrit-Change-Number: 12272
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 


Change in osmo-msc[master]: MNCC: use log wrapper for call processing

2018-12-18 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12329 )

Change subject: MNCC: use log wrapper for call processing
..


Patch Set 2:

> Patch Set 2:
>
> Yep, looks fine.

Feel free to add +1/+2 than :)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c44d7bb28f1ff895dd4f839d75840495503c916
Gerrit-Change-Number: 12329
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Tue, 18 Dec 2018 15:33:02 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-msc[master]: VLR: drop unused struct members

2018-12-18 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12335 )

Change subject: VLR: drop unused struct members
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12335/3/include/osmocom/msc/vlr.h
File include/osmocom/msc/vlr.h:

https://gerrit.osmocom.org/#/c/12335/3/include/osmocom/msc/vlr.h@174
PS3, Line 174:  } ps;
> Any reason we keep 'ps' around if there's only one list left as a struct 
> member?
See my earlier comment on this patch.



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I322072653b41cf250aa2c1e346e00bae884feb84
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Tue, 18 Dec 2018 15:31:12 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-msc[master]: VLR tests: avoid leaking LAC access details

2018-12-18 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12337 )

Change subject: VLR tests: avoid leaking LAC access details
..

VLR tests: avoid leaking LAC access details

Avoid leaking details on accessing data structure for LAC value into
test output: that's irrelevant clutter which forces unnecessary test
output modifications.

Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
---
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_ss.c
M tests/msc_vlr/msc_vlr_test_ss.err
4 files changed, 9 insertions(+), 9 deletions(-)

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



diff --git a/tests/msc_vlr/msc_vlr_test_call.c 
b/tests/msc_vlr/msc_vlr_test_call.c
index ef88c5a..4a4f243 100644
--- a/tests/msc_vlr/msc_vlr_test_call.c
+++ b/tests/msc_vlr/msc_vlr_test_call.c
@@ -154,7 +154,7 @@
vsub = vlr_subscr_find_by_imsi(net->vlr, IMSI);
VERBOSE_ASSERT(vsub != NULL, == true, "%d");
VERBOSE_ASSERT(strcmp(vsub->imsi, IMSI), == 0, "%d");
-   VERBOSE_ASSERT(vsub->lac, == 23, "%u");
+   VAL_ASSERT("LAC", vsub->lac, == 23, "%u");
vlr_subscr_put(vsub);
 }

diff --git a/tests/msc_vlr/msc_vlr_test_call.err 
b/tests/msc_vlr/msc_vlr_test_call.err
index 481a2db..db0d58c 100644
--- a/tests/msc_vlr/msc_vlr_test_call.err
+++ b/tests/msc_vlr/msc_vlr_test_call.err
@@ -175,7 +175,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, a new conn sends a CM Service Request. VLR responds with Auth 
Req, 2nd auth vector
@@ -555,7 +555,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, MNCC asks us to setup a call, causing Paging
@@ -934,7 +934,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, MNCC asks us to setup a call, causing Paging
@@ -1279,7 +1279,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, a new conn sends a CM Service Request. VLR responds with Auth 
Req, 2nd auth vector
@@ -1621,7 +1621,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, a new conn sends a CM Service Request. VLR responds with Auth 
Req, 2nd auth vector
diff --git a/tests/msc_vlr/msc_vlr_test_ss.c b/tests/msc_vlr/msc_vlr_test_ss.c
index 34aa634..4e8077e 100644
--- a/tests/msc_vlr/msc_vlr_test_ss.c
+++ b/tests/msc_vlr/msc_vlr_test_ss.c
@@ -68,7 +68,7 @@
vsub = vlr_subscr_find_by_imsi(net->vlr, IMSI);
VERBOSE_ASSERT(vsub != NULL, == true, "%d");
VERBOSE_ASSERT(strcmp(vsub->imsi, IMSI), == 0, "%d");
-   VERBOSE_ASSERT(vsub->lac, == 23, "%u");
+   VAL_ASSERT("LAC", vsub->lac, == 23, "%u");
vlr_subscr_put(vsub);

bss_sends_clear_complete();
diff --git a/tests/msc_vlr/msc_vlr_test_ss.err 
b/tests/msc_vlr/msc_vlr_test_ss.err
index fe869ad..1d4a0c6 100644
--- a/tests/msc_vlr/msc_vlr_test_ss.err
+++ b/tests/msc_vlr/msc_vlr_test_ss.err
@@ -91,7 +91,7 @@
 DREF VLR subscr MSISDN:46071 usage increases to: 3
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:46071 usage decreases to: 2
 - BSS sends BSSMAP Clear Complete
 DREF MSISDN:46071: MSC conn use - release == 0 (0x0: )
@@ -287,7 +287,7 @@
 DREF VLR subscr MSISDN:46071 usage increases to: 3
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:46071 usage decreases to: 2
 - BSS sends BSSMAP Clear Complete
 DREF MSISDN:46071: MSC conn use - release == 0 (0x0: )

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
Gerrit-Change-Number: 12337
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in osmo-msc[master]: VLR tests: avoid leaking LAC access details

2018-12-18 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12337 )

Change subject: VLR tests: avoid leaking LAC access details
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
Gerrit-Change-Number: 12337
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Tue, 18 Dec 2018 14:47:53 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: VLR tests: avoid leaking LAC access details

2018-12-18 Thread Max
Hello Stefan Sperling, Pau Espin Pedrol, Neels Hofmeyr, Jenkins Builder,

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

https://gerrit.osmocom.org/12337

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

Change subject: VLR tests: avoid leaking LAC access details
..

VLR tests: avoid leaking LAC access details

Avoid leaking details on accessing data structure for LAC value into
test output: that's irrelevant clutter which forces unnecessary test
output modifications.

Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
---
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_ss.c
M tests/msc_vlr/msc_vlr_test_ss.err
4 files changed, 9 insertions(+), 9 deletions(-)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
Gerrit-Change-Number: 12337
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in osmo-msc[master]: VLR tests: add logging macro with explicit value description

2018-12-18 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12336 )

Change subject: VLR tests: add logging macro with explicit value description
..

VLR tests: add logging macro with explicit value description

To avoid leaking structure details into test we sometimes have to
separate value description from actual value. Introduce new macro which
makes that possible and convert old one into trivial wrapper around it.

Change-Id: Ic462297edac4c55689f93cc45771c8b5e2aed864
---
M tests/msc_vlr/msc_vlr_tests.h
1 file changed, 5 insertions(+), 3 deletions(-)

Approvals:
  Max: Looks good to me, approved
  Pau Espin Pedrol: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/tests/msc_vlr/msc_vlr_tests.h b/tests/msc_vlr/msc_vlr_tests.h
index f7ff940..7eb6d24 100644
--- a/tests/msc_vlr/msc_vlr_tests.h
+++ b/tests/msc_vlr/msc_vlr_tests.h
@@ -182,11 +182,13 @@
OSMO_ASSERT(accepted == expect_accepted); \
} while (false)

-#define VERBOSE_ASSERT(val, expect_op, fmt) \
+#define VAL_ASSERT(desc, val, expect_op, fmt)  \
do { \
-   log(#val " == " fmt, (val)); \
+   log(desc " == " fmt, (val)); \
OSMO_ASSERT((val) expect_op); \
-   } while (0);
+   } while (0)
+
+#define VERBOSE_ASSERT(val, expect_op, fmt) VAL_ASSERT(#val, val, expect_op, 
fmt)

 #define EXPECT_CONN_COUNT(N) VERBOSE_ASSERT(llist_count(>ran_conns), == 
N, "%d")


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic462297edac4c55689f93cc45771c8b5e2aed864
Gerrit-Change-Number: 12336
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in osmo-msc[master]: VLR tests: avoid leaking LAC access details

2018-12-18 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12337 )

Change subject: VLR tests: avoid leaking LAC access details
..


Patch Set 2: Code-Review+2

Trivial macro rename from earlier revision which got +2 already.


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
Gerrit-Change-Number: 12337
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Tue, 18 Dec 2018 14:33:22 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: VLR tests: add logging macro with explicit value description

2018-12-18 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12336 )

Change subject: VLR tests: add logging macro with explicit value description
..


Patch Set 2: Code-Review+2

Trivial rename from previous revision which got +2 already.


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic462297edac4c55689f93cc45771c8b5e2aed864
Gerrit-Change-Number: 12336
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Tue, 18 Dec 2018 14:32:46 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: VLR tests: avoid leaking LAC access details

2018-12-18 Thread Max
Hello Stefan Sperling, Pau Espin Pedrol, Neels Hofmeyr, Jenkins Builder,

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

https://gerrit.osmocom.org/12337

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

Change subject: VLR tests: avoid leaking LAC access details
..

VLR tests: avoid leaking LAC access details

Avoid leaking details on accessing data structure for LAC value into
test output: that's irrelevant clutter which forces unnucessary test
output modifications.

Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
---
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_ss.c
M tests/msc_vlr/msc_vlr_test_ss.err
4 files changed, 9 insertions(+), 9 deletions(-)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
Gerrit-Change-Number: 12337
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in osmo-msc[master]: VLR: drop unused struct members

2018-12-18 Thread Max
Hello Stefan Sperling, Pau Espin Pedrol, Neels Hofmeyr, Jenkins Builder,

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

https://gerrit.osmocom.org/12335

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

Change subject: VLR: drop unused struct members
..

VLR: drop unused struct members

Change-Id: I322072653b41cf250aa2c1e346e00bae884feb84
---
M include/osmocom/msc/vlr.h
1 file changed, 0 insertions(+), 3 deletions(-)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I322072653b41cf250aa2c1e346e00bae884feb84
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in osmo-msc[master]: VLR tests: add logging macro with explicit value description

2018-12-18 Thread Max
Hello Stefan Sperling, Pau Espin Pedrol, Neels Hofmeyr, Jenkins Builder,

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

https://gerrit.osmocom.org/12336

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

Change subject: VLR tests: add logging macro with explicit value description
..

VLR tests: add logging macro with explicit value description

To avoid leaking structure details into test we sometimes have to
separate value description from actual value. Introduce new macro which
makes that possible and convert old one into trivial wrapper around it.

Change-Id: Ic462297edac4c55689f93cc45771c8b5e2aed864
---
M tests/msc_vlr/msc_vlr_tests.h
1 file changed, 5 insertions(+), 3 deletions(-)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic462297edac4c55689f93cc45771c8b5e2aed864
Gerrit-Change-Number: 12336
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in osmo-msc[master]: Remove redundancy in LAC processing

2018-12-18 Thread Max
Hello Stefan Sperling, Pau Espin Pedrol, Neels Hofmeyr, Jenkins Builder,

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

https://gerrit.osmocom.org/12338

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

Change subject: Remove redundancy in LAC processing
..

Remove redundancy in LAC processing

Always use LAC which is part of Cell Global ID otherwise we might end up
in a situation where separately stored LAC differs.

Both are described in 3GPP TS 23.008 $2.4 as temporary subscriber data
to be stored in VLR. Both are defined in 3GPP TS 23.003. The LAC is part
of LAI which is part of CGI so there should be no case when those values
differ for a given subscriber.

Change-Id: I993ebc3e14f25e83124b6d3f8461a4b18f971f8e
---
M include/osmocom/msc/vlr.h
M src/libmsc/gsm_04_08_cc.c
M src/libmsc/gsm_09_11.c
M src/libmsc/gsm_subscriber.c
M src/libmsc/msc_vty.c
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_ss.c
8 files changed, 9 insertions(+), 11 deletions(-)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I993ebc3e14f25e83124b6d3f8461a4b18f971f8e
Gerrit-Change-Number: 12338
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in osmo-ttcn3-hacks[master]: Remove -Wall for autogenerated code

2018-12-18 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12241 )

Change subject: Remove -Wall for autogenerated code
..

Remove -Wall for autogenerated code

There seems to be no option for ttcn3_makefilegen to disable generated
code warnings so the only way to clear output from useless warnings
about indentation and such is to manually strip -Wall using sed.

Change-Id: I7ef141f7f3370a1bf909845ce8a4eb650b33fa81
---
M regen-makefile.sh
1 file changed, 3 insertions(+), 0 deletions(-)

Approvals:
  dexter: Looks good to me, but someone else must approve
  Pau Espin Pedrol: Looks good to me, but someone else must approve
  osmith: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/regen-makefile.sh b/regen-makefile.sh
index a9f8562..5a4dd4c 100755
--- a/regen-makefile.sh
+++ b/regen-makefile.sh
@@ -33,6 +33,9 @@
 # see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=879816 for details
 sed -i -e 's/CPPFLAGS = -D$(PLATFORM) -I$(TTCN3_DIR)\/include/CPPFLAGS = 
-D$(PLATFORM) -DMAKEDEPEND_RUN -DUSE_SCTP -I$(TTCN3_DIR)\/include 
-I\/usr\/include\/titan/' Makefile

+#remove -Wall from CXXFLAGS: we're not interested in generic warnings for 
autogenerated code cluttering the logs
+sed -i -e 's/-Wall//' Makefile
+
 if [ "x$CPPFLAGS_TTCN3" != "x" ]; then
sed -i -e 's/CPPFLAGS_TTCN3 =/CPPFLAGS_TTCN3 = '"$CPPFLAGS_TTCN3"'/' 
Makefile
 fi

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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7ef141f7f3370a1bf909845ce8a4eb650b33fa81
Gerrit-Change-Number: 12241
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: osmith 


Change in osmo-ttcn3-hacks[master]: MSC: match default expectation with config

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12296 )

Change subject: MSC: match default expectation with config
..


Patch Set 1: Code-Review+2

1 + 1 = 2 :)


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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I540645ef4b1e08d05b89251f074af84a516e7a88
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: daniel 
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 17 Dec 2018 17:46:49 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-ttcn3-hacks[master]: MSC: match default expectation with config

2018-12-17 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12296 )

Change subject: MSC: match default expectation with config
..

MSC: match default expectation with config

In MSC_Tests.default we expect /tmp/mncc.sock as MNCC socket path -
let's match this expectation with osmo-msc.cfg to make sure that tests
work out of the box without the need to use specific command-line
option.

Change-Id: I540645ef4b1e08d05b89251f074af84a516e7a88
---
M msc/osmo-msc.cfg
1 file changed, 1 insertion(+), 0 deletions(-)

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



diff --git a/msc/osmo-msc.cfg b/msc/osmo-msc.cfg
index 3f9e192..ddac4cd 100644
--- a/msc/osmo-msc.cfg
+++ b/msc/osmo-msc.cfg
@@ -73,6 +73,7 @@
  cs7-instance-iu 0
  mgw remote-ip 127.0.0.1
  emergency-call route-to-msisdn 112
+ mncc external /tmp/mncc.sock
 mncc-int
  default-codec tch-f fr
  default-codec tch-h hr

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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I540645ef4b1e08d05b89251f074af84a516e7a88
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: daniel 
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: osmith 


Change in osmo-msc[master]: CC: log more details about unhandled message/state

2018-12-17 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12333 )

Change subject: CC: log more details about unhandled message/state
..

CC: log more details about unhandled message/state

Change-Id: I8e0febd04f7338aed7222dcfcd9bfddc7b8fda59
---
M src/libmsc/gsm_04_08_cc.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Pau Espin Pedrol: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index a172b47..15c6d9d 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -2047,7 +2047,7 @@
 && ((1 << trans->cc.state) & downstatelist[i].states))
break;
if (i == DOWNSLLEN) {
-   DEBUGP(DCC, "Message unhandled at this state.\n");
+   DEBUGP(DCC, "Message '%s' unhandled at state '%s'\n", 
get_mncc_name(msg_type), gsm48_cc_state_name(trans->cc.state));
return 0;
}


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8e0febd04f7338aed7222dcfcd9bfddc7b8fda59
Gerrit-Change-Number: 12333
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 


Change in osmo-msc[master]: comment: vlr: drop unused struct members

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12335 )

Change subject: comment: vlr: drop unused struct members
..


Patch Set 1:

Not sure if entire .ps can be dropped - it looks like pre-split leftover but 
it's called in GSUP processing and I don't see direct equivalent in SGSN so 
let's wait for Neels' feedback.


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I322072653b41cf250aa2c1e346e00bae884feb84
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 17 Dec 2018 16:44:22 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-msc[master]: CC: log more details about unhandled message/state

2018-12-17 Thread Max
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/12333

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

Change subject: CC: log more details about unhandled message/state
..

CC: log more details about unhandled message/state

Change-Id: I8e0febd04f7338aed7222dcfcd9bfddc7b8fda59
---
M src/libmsc/gsm_04_08_cc.c
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8e0febd04f7338aed7222dcfcd9bfddc7b8fda59
Gerrit-Change-Number: 12333
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-CC: Pau Espin Pedrol 


Change in osmo-msc[master]: Remove redundancy in LAC processing

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12338


Change subject: Remove redundancy in LAC processing
..

Remove redundancy in LAC processing

Always use LAC which is part of Cell Global ID otherwise we might end up
in a situation where separately stored LAC differs.

Both are described in 3GPP TS 23.008 $2.4 as temporary subscriber data
to be stored in VLR. Both are defined in 3GPP TS 23.003. The LAC is part
of LAI which is part of CGI so there should be no case when those values
differ for a given subscriber.

Change-Id: I993ebc3e14f25e83124b6d3f8461a4b18f971f8e
---
M include/osmocom/msc/vlr.h
M src/libmsc/gsm_04_08_cc.c
M src/libmsc/gsm_09_11.c
M src/libmsc/gsm_subscriber.c
M src/libmsc/msc_vty.c
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_ss.c
8 files changed, 9 insertions(+), 11 deletions(-)



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

diff --git a/include/osmocom/msc/vlr.h b/include/osmocom/msc/vlr.h
index 2f31063..20a9c0f 100644
--- a/include/osmocom/msc/vlr.h
+++ b/include/osmocom/msc/vlr.h
@@ -142,9 +142,7 @@
/* Newly allocated TMSI that was not yet acked by MS */
uint32_t tmsi_new;

-   /* some redundancy in information below? */
struct osmo_cell_global_id cgi; /* 2.4.16 */
-   uint16_t lac;   /* 2.4.2 */

char imeisv[GSM23003_IMEISV_NUM_DIGITS+1];  /* 2.2.3 */
char imei[GSM23003_IMEISV_NUM_DIGITS+1];/* 2.1.9 */
diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index a172b47..70f5c29 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -1940,7 +1940,7 @@
GSM48_CC_CAUSE_UNASSIGNED_NR);
}
/* If subscriber is not "attached" */
-   if (!vsub->lac) {
+   if (!vsub->cgi.lai.lac) {
DEBUGP(DCC, "(bts - trx - ts - ti -- sub %s) "
"Received '%s' from MNCC with "
"detached subscriber %s\n", data->called.number,
@@ -1979,7 +1979,7 @@
"unallocated channel, paging already "
"started for lac %d.\n",
data->called.number,
-   get_mncc_name(msg_type), vsub->lac);
+   get_mncc_name(msg_type), 
vsub->cgi.lai.lac);
vlr_subscr_put(vsub);
trans_free(trans);
return 0;
diff --git a/src/libmsc/gsm_09_11.c b/src/libmsc/gsm_09_11.c
index 43bf48c..3ee6e92 100644
--- a/src/libmsc/gsm_09_11.c
+++ b/src/libmsc/gsm_09_11.c
@@ -287,7 +287,7 @@
}

/* If subscriber is not "attached" */
-   if (!vsub->lac) {
+   if (!vsub->cgi.lai.lac) {
LOGP(DMM, LOGL_ERROR, "Network-originated session "
"rejected - subscriber is not attached\n");
return NULL;
diff --git a/src/libmsc/gsm_subscriber.c b/src/libmsc/gsm_subscriber.c
index 188807e..0e76efc 100644
--- a/src/libmsc/gsm_subscriber.c
+++ b/src/libmsc/gsm_subscriber.c
@@ -116,12 +116,12 @@
 * SCCP connections (if any). */
switch (vsub->cs.attached_via_ran) {
case RAN_GERAN_A:
-   return a_iface_tx_paging(vsub->imsi, vsub->tmsi, vsub->lac);
+   return a_iface_tx_paging(vsub->imsi, vsub->tmsi, 
vsub->cgi.lai.lac);
case RAN_UTRAN_IU:
return ranap_iu_page_cs(vsub->imsi,
vsub->tmsi == GSM_RESERVED_TMSI?
NULL : >tmsi,
-   vsub->lac);
+   vsub->cgi.lai.lac);
default:
break;
}
diff --git a/src/libmsc/msc_vty.c b/src/libmsc/msc_vty.c
index 87adc82..59112a7 100644
--- a/src/libmsc/msc_vty.c
+++ b/src/libmsc/msc_vty.c
@@ -631,7 +631,7 @@
vty_out(vty, "Extension: %s%s", vsub->msisdn,
VTY_NEWLINE);
vty_out(vty, "LAC: %d/0x%x%s",
-   vsub->lac, vsub->lac, VTY_NEWLINE);
+   vsub->cgi.lai.lac, vsub->cgi.lai.lac, VTY_NEWLINE);
vty_out(vty, "IMSI: %s%s", vsub->imsi, VTY_NEWLINE);
if (vsub->tmsi != GSM_RESERVED_TMSI)
vty_out(vty, "TMSI: %08X%s", vsub->tmsi,
diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index 5d171d5..e635305 100644
--- a/

Change in osmo-msc[master]: VLR tests: add logging macro with explicit value description

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12336


Change subject: VLR tests: add logging macro with explicit value description
..

VLR tests: add logging macro with explicit value description

To avoid leaking structure details into test we sometimes have to
separate value description from actual value. Introduce new macro which
makes that possible and convert old one into trivial wrapper around it.

Change-Id: Ic462297edac4c55689f93cc45771c8b5e2aed864
---
M tests/msc_vlr/msc_vlr_tests.h
1 file changed, 5 insertions(+), 3 deletions(-)



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

diff --git a/tests/msc_vlr/msc_vlr_tests.h b/tests/msc_vlr/msc_vlr_tests.h
index f7ff940..3629967 100644
--- a/tests/msc_vlr/msc_vlr_tests.h
+++ b/tests/msc_vlr/msc_vlr_tests.h
@@ -182,11 +182,13 @@
OSMO_ASSERT(accepted == expect_accepted); \
} while (false)

-#define VERBOSE_ASSERT(val, expect_op, fmt) \
+#define V_ASSERT(desc, val, expect_op, fmt)\
do { \
-   log(#val " == " fmt, (val)); \
+   log(desc " == " fmt, (val)); \
OSMO_ASSERT((val) expect_op); \
-   } while (0);
+   } while (0)
+
+#define VERBOSE_ASSERT(val, expect_op, fmt) V_ASSERT(#val, val, expect_op, fmt)

 #define EXPECT_CONN_COUNT(N) VERBOSE_ASSERT(llist_count(>ran_conns), == 
N, "%d")
 

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic462297edac4c55689f93cc45771c8b5e2aed864
Gerrit-Change-Number: 12336
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-msc[master]: VLR tests: avoid leaking LAC access details

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12337


Change subject: VLR tests: avoid leaking LAC access details
..

VLR tests: avoid leaking LAC access details

Avoid leaking details on accessing data structure for LAC value into
test output: that's irrelevant clutter which forces unnucessary test
output modifications.

Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
---
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_ss.c
M tests/msc_vlr/msc_vlr_test_ss.err
4 files changed, 9 insertions(+), 9 deletions(-)



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

diff --git a/tests/msc_vlr/msc_vlr_test_call.c 
b/tests/msc_vlr/msc_vlr_test_call.c
index ef88c5a..83bc84b 100644
--- a/tests/msc_vlr/msc_vlr_test_call.c
+++ b/tests/msc_vlr/msc_vlr_test_call.c
@@ -154,7 +154,7 @@
vsub = vlr_subscr_find_by_imsi(net->vlr, IMSI);
VERBOSE_ASSERT(vsub != NULL, == true, "%d");
VERBOSE_ASSERT(strcmp(vsub->imsi, IMSI), == 0, "%d");
-   VERBOSE_ASSERT(vsub->lac, == 23, "%u");
+   V_ASSERT("LAC", vsub->lac, == 23, "%u");
vlr_subscr_put(vsub);
 }

diff --git a/tests/msc_vlr/msc_vlr_test_call.err 
b/tests/msc_vlr/msc_vlr_test_call.err
index 481a2db..db0d58c 100644
--- a/tests/msc_vlr/msc_vlr_test_call.err
+++ b/tests/msc_vlr/msc_vlr_test_call.err
@@ -175,7 +175,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, a new conn sends a CM Service Request. VLR responds with Auth 
Req, 2nd auth vector
@@ -555,7 +555,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, MNCC asks us to setup a call, causing Paging
@@ -934,7 +934,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, MNCC asks us to setup a call, causing Paging
@@ -1279,7 +1279,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, a new conn sends a CM Service Request. VLR responds with Auth 
Req, 2nd auth vector
@@ -1621,7 +1621,7 @@
 DREF VLR subscr MSISDN:42342 usage increases to: 2
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:42342 usage decreases to: 1
 ---
 - after a while, a new conn sends a CM Service Request. VLR responds with Auth 
Req, 2nd auth vector
diff --git a/tests/msc_vlr/msc_vlr_test_ss.c b/tests/msc_vlr/msc_vlr_test_ss.c
index 34aa634..d8167bd 100644
--- a/tests/msc_vlr/msc_vlr_test_ss.c
+++ b/tests/msc_vlr/msc_vlr_test_ss.c
@@ -68,7 +68,7 @@
vsub = vlr_subscr_find_by_imsi(net->vlr, IMSI);
VERBOSE_ASSERT(vsub != NULL, == true, "%d");
VERBOSE_ASSERT(strcmp(vsub->imsi, IMSI), == 0, "%d");
-   VERBOSE_ASSERT(vsub->lac, == 23, "%u");
+   V_ASSERT("LAC", vsub->lac, == 23, "%u");
vlr_subscr_put(vsub);

bss_sends_clear_complete();
diff --git a/tests/msc_vlr/msc_vlr_test_ss.err 
b/tests/msc_vlr/msc_vlr_test_ss.err
index fe869ad..1d4a0c6 100644
--- a/tests/msc_vlr/msc_vlr_test_ss.err
+++ b/tests/msc_vlr/msc_vlr_test_ss.err
@@ -91,7 +91,7 @@
 DREF VLR subscr MSISDN:46071 usage increases to: 3
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:46071 usage decreases to: 2
 - BSS sends BSSMAP Clear Complete
 DREF MSISDN:46071: MSC conn use - release == 0 (0x0: )
@@ -287,7 +287,7 @@
 DREF VLR subscr MSISDN:46071 usage increases to: 3
   vsub != NULL == 1
   strcmp(vsub->imsi, IMSI) == 0
-  vsub->lac == 23
+  LAC == 23
 DREF VLR subscr MSISDN:46071 usage decreases to: 2
 - BSS sends BSSMAP Clear Complete
 DREF MSISDN:46071: MSC conn use - release == 0 (0x0: )

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4a1d7884cf47ad513d7d6fb27c5c6f1b829dff2e
Gerrit-Change-Number: 12337
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-msc[master]: comment: vlr: drop unused struct members

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12335


Change subject: comment: vlr: drop unused struct members
..

comment: vlr: drop unused struct members

Change-Id: I322072653b41cf250aa2c1e346e00bae884feb84
---
M include/osmocom/msc/vlr.h
1 file changed, 0 insertions(+), 3 deletions(-)



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

diff --git a/include/osmocom/msc/vlr.h b/include/osmocom/msc/vlr.h
index 68e0759..2f31063 100644
--- a/include/osmocom/msc/vlr.h
+++ b/include/osmocom/msc/vlr.h
@@ -171,9 +171,6 @@
/* PS (SGSN) specific parts */
struct {
struct llist_head pdp_list;
-   uint8_t rac;
-   uint8_t sac;
-   struct gprs_mm_ctx *mmctx;
} ps;
/* CS (NITB/CSCN) specific parts */
struct {

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I322072653b41cf250aa2c1e346e00bae884feb84
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-msc[master]: cosmetic: log more details about unhandled message/state

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12333


Change subject: cosmetic: log more details about unhandled message/state
..

cosmetic: log more details about unhandled message/state

Change-Id: I8e0febd04f7338aed7222dcfcd9bfddc7b8fda59
---
M src/libmsc/gsm_04_08_cc.c
1 file changed, 1 insertion(+), 1 deletion(-)



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

diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index a172b47..15c6d9d 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -2047,7 +2047,7 @@
 && ((1 << trans->cc.state) & downstatelist[i].states))
break;
if (i == DOWNSLLEN) {
-   DEBUGP(DCC, "Message unhandled at this state.\n");
+   DEBUGP(DCC, "Message '%s' unhandled at state '%s'\n", 
get_mncc_name(msg_type), gsm48_cc_state_name(trans->cc.state));
return 0;
}


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e0febd04f7338aed7222dcfcd9bfddc7b8fda59
Gerrit-Change-Number: 12333
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-msc[master]: cosmetic: drop unused variable

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12334


Change subject: cosmetic: drop unused variable
..

cosmetic: drop unused variable

Change-Id: Iff358eb2328cdd052e66b572aeec1b767174949b
---
M src/libmsc/a_iface.c
1 file changed, 1 insertion(+), 2 deletions(-)



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

diff --git a/src/libmsc/a_iface.c b/src/libmsc/a_iface.c
index 66644ee..a96f247 100644
--- a/src/libmsc/a_iface.c
+++ b/src/libmsc/a_iface.c
@@ -360,7 +360,6 @@
struct ran_conn *conn;
struct gsm0808_channel_type ct;
struct gsm0808_speech_codec_list scl;
-   uint32_t *ci_ptr = NULL;
struct msgb *msg;
struct sockaddr_storage rtp_addr;
struct sockaddr_in rtp_addr_in;
@@ -404,7 +403,7 @@
memset(_addr, 0, sizeof(rtp_addr));
memcpy(_addr, _addr_in, sizeof(rtp_addr_in));

-   msg = gsm0808_create_ass(, NULL, _addr, , ci_ptr);
+   msg = gsm0808_create_ass(, NULL, _addr, , NULL);

LOGPCONN(conn, LOGL_DEBUG, "N-DATA.req(%s)\n", msgb_hexdump_l2(msg));
return osmo_sccp_tx_data_msg(conn->a.scu, conn->a.conn_id, msg);

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iff358eb2328cdd052e66b572aeec1b767174949b
Gerrit-Change-Number: 12334
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in libosmocore[master]: add/clean big-endian packed structs (struct_endianess.py)

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11787 )

Change subject: add/clean big-endian packed structs (struct_endianess.py)
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0b99d76932aeb03e93bd0c62d3bf025dec5f9d2
Gerrit-Change-Number: 11787
Gerrit-PatchSet: 4
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Assignee: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 17 Dec 2018 14:50:59 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: msc_vty.c: configurable retrieval of IMEI, IMEISV

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12302 )

Change subject: msc_vty.c: configurable retrieval of IMEI, IMEISV
..


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/12302/1/src/libmsc/msc_vty.c
File src/libmsc/msc_vty.c:

https://gerrit.osmocom.org/#/c/12302/1/src/libmsc/msc_vty.c@448
PS1, Line 448: ""
> Can somebody suggest a good description here? […]
It might be helpful to understand what "early" means in here. Is there some 
spec reference around cfg.retrieve_imeisv_*?



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee516b9cd7877b21207ce9a6d954109f19558163
Gerrit-Change-Number: 12302
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: osmith 
Gerrit-CC: Max 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Mon, 17 Dec 2018 14:01:01 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libosmocore[master]: vty: Introduce telnet_init_default and make vty port configurable

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12321 )

Change subject: vty: Introduce telnet_init_default and make vty port 
configurable
..


Patch Set 1: Code-Review-1

(2 comments)

Please clarify why signed values are used for port which starts from 0.

https://gerrit.osmocom.org/#/c/12321/1/include/osmocom/vty/vty.h
File include/osmocom/vty/vty.h:

https://gerrit.osmocom.org/#/c/12321/1/include/osmocom/vty/vty.h@214
PS1, Line 214: int vty_get_bind_port(int default_port);
Is there any use-case where we want to call vty_get_bind_port() with negative 
value?


https://gerrit.osmocom.org/#/c/12321/1/src/vty/vty.c
File src/vty/vty.c:

https://gerrit.osmocom.org/#/c/12321/1/src/vty/vty.c@1638
PS1, Line 1638: return default_port;
So this can return negative value only if it receives it as parameter.



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5fb2faaf4311bd7284ee870526a6f87b7e260f3
Gerrit-Change-Number: 12321
Gerrit-PatchSet: 1
Gerrit-Owner: Holger Freyther 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Holger Freyther 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Mon, 17 Dec 2018 13:57:53 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: MNCC: use log wrapper for call processing

2018-12-17 Thread Max
Hello Pau Espin Pedrol, Jenkins Builder,

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

https://gerrit.osmocom.org/12329

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

Change subject: MNCC: use log wrapper for call processing
..

MNCC: use log wrapper for call processing

Add log/debug wrappers to conveniently print local and remote call
references.

Change-Id: I5c44d7bb28f1ff895dd4f839d75840495503c916
---
M src/libmsc/mncc_builtin.c
1 file changed, 12 insertions(+), 13 deletions(-)


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5c44d7bb28f1ff895dd4f839d75840495503c916
Gerrit-Change-Number: 12329
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 


Change in osmocom-bb[master]: Revert "mobile: use VTY bind addr from config, deprecate cmd line opt...

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12208 )

Change subject: Revert "mobile: use VTY bind addr from config, deprecate cmd 
line options"
..


Patch Set 1: Code-Review-1

> Patch Set 1:
>
> From my point of view the original change should not have been submitted as 
> it takes away a feature that is actively used and causes a regression.

The removed options were marked as deprecated so I also don't think reverting 
the change is the right way. Unfortunately we don't have releases in osmocom-bb 
which would allow us to handle deprecations better.


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

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89bc16b667dbe05baa76cfa4f86b4946f8019e8
Gerrit-Change-Number: 12208
Gerrit-PatchSet: 1
Gerrit-Owner: Holger Freyther 
Gerrit-Reviewer: Holger Freyther 
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Mon, 17 Dec 2018 13:10:49 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: cosmetic: drop duplicated #include

2018-12-17 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12328 )

Change subject: cosmetic: drop duplicated #include
..

cosmetic: drop duplicated #include

Change-Id: I216425ba5994a49981d51bce6cfa7c3fa5fe9e40
---
M src/libmsc/transaction.c
1 file changed, 0 insertions(+), 1 deletion(-)

Approvals:
  Jenkins Builder: Verified
  Pau Espin Pedrol: Looks good to me, approved



diff --git a/src/libmsc/transaction.c b/src/libmsc/transaction.c
index 47124a0..a0f7b31 100644
--- a/src/libmsc/transaction.c
+++ b/src/libmsc/transaction.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 

 void *tall_trans_ctx;

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I216425ba5994a49981d51bce6cfa7c3fa5fe9e40
Gerrit-Change-Number: 12328
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 


Change in docker-playground[master]: MSC: use config file for mncc path

2018-12-17 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12277 )

Change subject: MSC: use config file for mncc path
..

MSC: use config file for mncc path

Use config file parameter instead of command line option for MNCC socket
path.

Change-Id: I689cfb3bad09b76859ef7e42c1312c636eebe637
---
M ttcn3-msc-test/jenkins.sh
M ttcn3-msc-test/osmo-msc.cfg
2 files changed, 2 insertions(+), 1 deletion(-)

Approvals:
  daniel: Looks good to me, approved
  Harald Welte: Looks good to me, approved
  osmith: Looks good to me, but someone else must approve
  Max: Verified



diff --git a/ttcn3-msc-test/jenkins.sh b/ttcn3-msc-test/jenkins.sh
index 662c377..82ec483 100755
--- a/ttcn3-msc-test/jenkins.sh
+++ b/ttcn3-msc-test/jenkins.sh
@@ -38,7 +38,7 @@
-v $VOL_BASE_DIR/unix:/data/unix \
--name ${BUILD_TAG}-msc -d \
$REPO_USER/osmo-msc-$IMAGE_SUFFIX \
-   /usr/local/bin/osmo-msc -M /data/unix/mncc
+   /usr/local/bin/osmo-msc

 echo Starting container with MSC testsuite
 docker run --rm \
diff --git a/ttcn3-msc-test/osmo-msc.cfg b/ttcn3-msc-test/osmo-msc.cfg
index f0180d8..4c3555b 100644
--- a/ttcn3-msc-test/osmo-msc.cfg
+++ b/ttcn3-msc-test/osmo-msc.cfg
@@ -73,6 +73,7 @@
  cs7-instance-iu 0
  mgw remote-ip 172.18.1.103
  emergency-call route-to-msisdn 112
+ mncc external /data/unix/mncc
 mncc-int
  default-codec tch-f fr
  default-codec tch-h hr

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

Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I689cfb3bad09b76859ef7e42c1312c636eebe637
Gerrit-Change-Number: 12277
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: daniel 
Gerrit-Reviewer: osmith 


Change in docker-playground[master]: MSC: use config file for mncc path

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12277 )

Change subject: MSC: use config file for mncc path
..


Patch Set 1: Verified+1


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

Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I689cfb3bad09b76859ef7e42c1312c636eebe637
Gerrit-Change-Number: 12277
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: daniel 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 17 Dec 2018 13:03:15 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-sysmon[master]: ctrl: log host/port on errors

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12318 )

Change subject: ctrl: log host/port on errors
..


Patch Set 1:

> Patch Set 1:
>
> > What would be the right approach to make host:port available for logging 
> > than?
>
> Perhaps store the information you need to log in struct simple_ctrl_handle?

It'll breaks layering as well: we'll either have to duplicate host:port info 
from anonymous struct inside config and keep it in sync or move this data out 
of config. Both ways seems worse to me - we either loose single source of truth 
or we spread config parameters over several structs.


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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I788d51359965a66c54075a3971aa7824c3bfb0bf
Gerrit-Change-Number: 12318
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Mon, 17 Dec 2018 12:28:21 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-sysmon[master]: Use absolute path for default config

2018-12-17 Thread Max
Max has abandoned this change. ( https://gerrit.osmocom.org/12319 )

Change subject: Use absolute path for default config
..


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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I37c559af99872de9290335699e2118924ae2156a
Gerrit-Change-Number: 12319
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 


Change in osmo-msc[master]: cosmetic: drop duplicated #include

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12328


Change subject: cosmetic: drop duplicated #include
..

cosmetic: drop duplicated #include

Change-Id: I216425ba5994a49981d51bce6cfa7c3fa5fe9e40
---
M src/libmsc/transaction.c
1 file changed, 0 insertions(+), 1 deletion(-)



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

diff --git a/src/libmsc/transaction.c b/src/libmsc/transaction.c
index 47124a0..a0f7b31 100644
--- a/src/libmsc/transaction.c
+++ b/src/libmsc/transaction.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 

 void *tall_trans_ctx;

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I216425ba5994a49981d51bce6cfa7c3fa5fe9e40
Gerrit-Change-Number: 12328
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-msc[master]: Use proper type for tch_rtp_connect() parameter

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12330


Change subject: Use proper type for tch_rtp_connect() parameter
..

Use proper type for tch_rtp_connect() parameter

Change-Id: I6e2efcd2e25d6ec2ff35a4b8cfcda02abe97fa59
---
M src/libmsc/gsm_04_08_cc.c
1 file changed, 1 insertion(+), 2 deletions(-)



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

diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index a172b47..3fe427e 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -1761,10 +1761,9 @@
return 0;
 }

-static int tch_rtp_connect(struct gsm_network *net, void *arg)
+static int tch_rtp_connect(struct gsm_network *net, struct gsm_mncc_rtp *rtp)
 {
struct gsm_trans *trans;
-   struct gsm_mncc_rtp *rtp = arg;
struct in_addr addr;

/* FIXME: in *rtp we should get the codec information of the remote

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e2efcd2e25d6ec2ff35a4b8cfcda02abe97fa59
Gerrit-Change-Number: 12330
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-msc[master]: MNCC: use log wrapper for call processing

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12329


Change subject: MNCC: use log wrapper for call processing
..

MNCC: use log wrapper for call processing

Add log/debug wrappers to conveniently print local and remote call
references.

Change-Id: I5c44d7bb28f1ff895dd4f839d75840495503c916
---
M src/libmsc/mncc_builtin.c
1 file changed, 13 insertions(+), 13 deletions(-)



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

diff --git a/src/libmsc/mncc_builtin.c b/src/libmsc/mncc_builtin.c
index 135a5e4..f3fe128 100644
--- a/src/libmsc/mncc_builtin.c
+++ b/src/libmsc/mncc_builtin.c
@@ -35,6 +35,9 @@
 #include 
 #include 

+#define DEBUGCC(ss, l, r, fmt, args...)   DEBUGP(ss, "(call %x, remote %x) " 
fmt "\n", l->callref, r->callref, ##args)
+#define LOGCC(ss, ll, l, r, fmt, args...) LOGP(ss, ll, "(call %x, remote %x) " 
fmt "\n", l->callref, r->callref, ##args)
+
 void *tall_call_ctx;

 static LLIST_HEAD(call_list);
@@ -105,8 +108,7 @@
llist_add_tail(>entry, _list);
remote->net = call->net;
remote->callref = new_callref++;
-   DEBUGP(DMNCC, "(call %x) Creating new remote instance %x.\n",
-   call->callref, remote->callref);
+   DEBUGCC(DMNCC, call, remote, "Creating new remote instance.");

/* link remote call */
call->remote_ref = remote->callref;
@@ -115,20 +117,20 @@
/* send call proceeding */
memset(, 0, sizeof(struct gsm_mncc));
mncc.callref = call->callref;
-   DEBUGP(DMNCC, "(call %x) Accepting call.\n", call->callref);
+   DEBUGCC(DMNCC, call, remote, "Accepting call.");
mncc_tx_to_cc(call->net, MNCC_CALL_PROC_REQ, );

/* modify mode */
memset(, 0, sizeof(struct gsm_mncc));
mncc.callref = call->callref;
-   DEBUGP(DMNCC, "(call %x) Modify channel mode\n", call->callref);
+   DEBUGCC(DMNCC, call, remote, "Modify channel mode.");
mncc_tx_to_cc(call->net, MNCC_LCHAN_MODIFY, );

/* send setup to remote */
 // setup->fields |= MNCC_F_SIGNAL;
 // setup->signal = GSM48_SIGNAL_DIALTONE;
setup->callref = remote->callref;
-   DEBUGP(DMNCC, "(call %x) Forwarding SETUP to remote.\n", call->callref);
+   DEBUGCC(DMNCC, call, remote, "Forwarding SETUP to remote.");
return mncc_tx_to_cc(remote->net, MNCC_SETUP_REQ, setup);

 out_reject:
@@ -146,7 +148,7 @@
if (!(remote = get_call_ref(call->remote_ref)))
return 0;
alert->callref = remote->callref;
-   DEBUGP(DMNCC, "(call %x) Forwarding ALERT to remote.\n", call->callref);
+   DEBUGCC(DMNCC, call, remote, "Forwarding ALERT to remote.");
return mncc_tx_to_cc(remote->net, MNCC_ALERT_REQ, alert);
 }

@@ -159,7 +161,7 @@
if (!(remote = get_call_ref(call->remote_ref)))
return 0;
notify->callref = remote->callref;
-   DEBUGP(DMNCC, "(call %x) Forwarding NOTIF to remote.\n", call->callref);
+   DEBUGCC(DMNCC, call, remote, "Forwarding NOTIF to remote.");
return mncc_tx_to_cc(remote->net, MNCC_NOTIFY_REQ, notify);
 }

@@ -180,13 +182,13 @@
if (!(remote = get_call_ref(call->remote_ref)))
return 0;
connect->callref = remote->callref;
-   DEBUGP(DMNCC, "(call %x) Sending CONNECT to remote.\n", call->callref);
+   DEBUGCC(DMNCC, call, remote, "Sending CONNECT to remote.");
mncc_tx_to_cc(remote->net, MNCC_SETUP_RSP, connect);

/* bridge tch */
bridge.callref[0] = call->callref;
bridge.callref[1] = call->remote_ref;
-   DEBUGP(DMNCC, "(call %x) Bridging with remote.\n", call->callref);
+   DEBUGCC(DMNCC, call, remote, "Bridging with remote.");

return mncc_tx_to_cc(call->net, MNCC_BRIDGE, );
 }
@@ -206,8 +208,7 @@
return 0;
}
disc->callref = remote->callref;
-   DEBUGP(DMNCC, "(call %x) Disconnecting remote with cause %d\n",
-   remote->callref, disc->cause.value);
+   DEBUGCC(DMNCC, call, remote, "Disconnecting remote with cause %d", 
disc->cause.value);
return mncc_tx_to_cc(remote->net, MNCC_DISC_REQ, disc);
 }

@@ -222,8 +223,7 @@
}

rel->callref = remote->callref;
-   DEBUGP(DMNCC, "(call %x) Releasing remote with cause %d\n",
-   call->callref, rel->cause.value);
+   DEBUGCC(DMNCC, call, remote, "Releasing remote with cause %d", 
rel->cause.value);

/*
 * Release this side of the call right now. Otherwise we end up

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5c44d7bb28f1ff895dd4f839d75840495503c916
Gerrit-Change-Number: 12329
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-msc[master]: MNCC: internalize bridge error handling

2018-12-17 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12331


Change subject: MNCC: internalize bridge error handling
..

MNCC: internalize bridge error handling

This can be handled internally instead of checking tch_bridge() outcome
and than calling disconnect_bridge() with the same arguments.

Change-Id: I66f6fac254d78dcf64bcb6aa4a443b899fb378a7
---
M src/libmsc/gsm_04_08_cc.c
1 file changed, 30 insertions(+), 33 deletions(-)



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

diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index 3fe427e..723c9f3 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -345,6 +345,31 @@
return 0;
 }

+static int gsm48_cc_tx_disconnect(struct gsm_trans *trans, void *arg);
+
+/* disconnect both calls from the bridge */
+static inline int disconnect_bridge(struct gsm_trans *trans0, struct gsm_trans 
*trans1, int err)
+{
+   struct gsm_mncc mx_rel;
+   if (!trans0 || !trans1)
+   return -err;
+
+   DEBUGP(DCC, "Failed to bridge TCH for calls %x <-> %x :: %s \n",
+  trans0->callref, trans1->callref, strerror(err));
+
+   memset(_rel, 0, sizeof(struct gsm_mncc));
+   mncc_set_cause(_rel, GSM48_CAUSE_LOC_INN_NET,
+  GSM48_CC_CAUSE_CHAN_UNACCEPT);
+
+   mx_rel.callref = trans0->callref;
+   gsm48_cc_tx_disconnect(trans0, _rel);
+
+   mx_rel.callref = trans1->callref;
+   gsm48_cc_tx_disconnect(trans1, _rel);
+
+   return -err;
+}
+
 /* bridge channels of two transactions */
 static int tch_bridge(struct gsm_network *net, struct gsm_mncc_bridge *bridge)
 {
@@ -353,10 +378,10 @@
int rc;

if (!trans1 || !trans2)
-   return -EIO;
+   return disconnect_bridge(trans1, trans2, EIO);

if (!trans1->conn || !trans2->conn)
-   return -EIO;
+   return disconnect_bridge(trans1, trans2, EIO);

/* Which subscriber do we want to track trans1 or trans2? */
log_set_context(LOG_CTX_VLR_SUBSCR, trans1->vsub);
@@ -374,12 +399,12 @@
rc = msc_mgcp_call_complete(trans1, trans2->conn->rtp.local_port_cn,
trans2->conn->rtp.local_addr_cn);
if (rc)
-   return -EINVAL;
+   return disconnect_bridge(trans1, trans2, EINVAL);

rc = msc_mgcp_call_complete(trans2, trans1->conn->rtp.local_port_cn,
trans1->conn->rtp.local_addr_cn);
if (rc)
-   return -EINVAL;
+   return disconnect_bridge(trans1, trans2, EINVAL);

return 0;
 }
@@ -391,7 +416,6 @@
 }

 static int gsm48_cc_tx_release(struct gsm_trans *trans, void *arg);
-static int gsm48_cc_tx_disconnect(struct gsm_trans *trans, void *arg);

 static void gsm48_cc_timeout(void *arg)
 {
@@ -475,30 +499,6 @@

 }

-/* disconnect both calls from the bridge */
-static inline void disconnect_bridge(struct gsm_network *net,
-struct gsm_mncc_bridge *bridge, int err)
-{
-   struct gsm_trans *trans0 = trans_find_by_callref(net, 
bridge->callref[0]);
-   struct gsm_trans *trans1 = trans_find_by_callref(net, 
bridge->callref[1]);
-   struct gsm_mncc mx_rel;
-   if (!trans0 || !trans1)
-   return;
-
-   DEBUGP(DCC, "Failed to bridge TCH for calls %x <-> %x :: %s \n",
-  trans0->callref, trans1->callref, strerror(err));
-
-   memset(_rel, 0, sizeof(struct gsm_mncc));
-   mncc_set_cause(_rel, GSM48_CAUSE_LOC_INN_NET,
-  GSM48_CC_CAUSE_CHAN_UNACCEPT);
-
-   mx_rel.callref = trans0->callref;
-   gsm48_cc_tx_disconnect(trans0, _rel);
-
-   mx_rel.callref = trans1->callref;
-   gsm48_cc_tx_disconnect(trans1, _rel);
-}
-
 static void gsm48_start_cc_timer(struct gsm_trans *trans, int current,
 int sec, int micro)
 {
@@ -1865,10 +1865,7 @@
/* handle special messages */
switch(msg_type) {
case MNCC_BRIDGE:
-   rc = tch_bridge(net, arg);
-   if (rc < 0)
-   disconnect_bridge(net, arg, -rc);
-   return rc;
+   return tch_bridge(net, arg);
case MNCC_RTP_CREATE:
return tch_rtp_create(net, data->callref);
case MNCC_RTP_CONNECT:

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I66f6fac254d78dcf64bcb6aa4a443b899fb378a7
Gerrit-Change-Number: 12331
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-msc[master]: Store subscriber's cell identity

2018-12-17 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11746 )

Change subject: Store subscriber's cell identity
..


Patch Set 3:

(1 comment)

This change is ready for review.

https://gerrit.osmocom.org/#/c/11746/2/src/libmsc/a_iface_bssap.c
File src/libmsc/a_iface_bssap.c:

https://gerrit.osmocom.org/#/c/11746/2/src/libmsc/a_iface_bssap.c@51
PS2, Line 51: struct osmo_sccp_user *scu, int conn_id)
> TBH, this looks ugly. I am not a big fun of such fancy […]
How this makes it worse? The line I've added is properly aligned. Feel free to 
submit patch which aligns the rest of them right.



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8544c30ea800ce8356a227b03a8b21bf3252be7e
Gerrit-Change-Number: 11746
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Mon, 17 Dec 2018 09:39:50 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-sysmon[master]: Use absolute path for default config

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12319 )

Change subject: Use absolute path for default config
..


Patch Set 1:

> Patch Set 1: Code-Review-1
>
> As far as I can tell, in all repos we have default cfg_file as $CWD/*.cfg, so 
> I'd rather keep same behavior for all osmocom projects.
>
> Another discussion is whether we want to move all osmocom projects to have 
> /etc/osmocom/*.cfg by default. It'd then make sense to by default check first 
> $CWD and if not found look for /etc/osmocom, to maintain it 
> backwards-compatible.

The plan is to use this as login shell and I'm not sure what's $CWD in this 
case so it's better to use absolute path.


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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37c559af99872de9290335699e2118924ae2156a
Gerrit-Change-Number: 12319
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Fri, 14 Dec 2018 15:58:42 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-sysmon[master]: Use absolute path for default config

2018-12-14 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12319


Change subject: Use absolute path for default config
..

Use absolute path for default config

Change-Id: I37c559af99872de9290335699e2118924ae2156a
Related: SYS#2655
---
M src/osysmon_main.c
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-sysmon refs/changes/19/12319/1

diff --git a/src/osysmon_main.c b/src/osysmon_main.c
index 486ee8f..caa25d8 100644
--- a/src/osysmon_main.c
+++ b/src/osysmon_main.c
@@ -130,7 +130,7 @@
const char *config_file;
bool daemonize;
 } cmdline_opts = {
-   .config_file = "osmo-sysmon.cfg",
+   .config_file = "/etc/osmocom/osmo-sysmon.cfg",
.daemonize = false,
 };


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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I37c559af99872de9290335699e2118924ae2156a
Gerrit-Change-Number: 12319
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-sysmon[master]: ctrl: log host/port on errors

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12318 )

Change subject: ctrl: log host/port on errors
..


Patch Set 1:

> Patch Set 1: Code-Review-1
>
> I feel this entire patchset breaks layering.  The simple_ctrl_client doesnt 
> have a configuration. the configuration is part of the main program. but now 
> you're passing the configuration around everywhere, adding additional 
> arguments to the functions, making them introspect somethin that's not theirs.

What would be the right approach to make host:port available for logging than?


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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I788d51359965a66c54075a3971aa7824c3bfb0bf
Gerrit-Change-Number: 12318
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Comment-Date: Fri, 14 Dec 2018 15:20:39 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-sysmon[master]: ctrl: make config structure public

2018-12-14 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12316


Change subject: ctrl: make config structure public
..

ctrl: make config structure public

Moving configuration data from anonymous struct into shared header as a
preparation for ctrl logging improvements in follow-up patches.

Change-Id: I3520e14ca6e1b8e270dbd4b1bf2378fe486991ce
Related: SYS#2655
---
M src/osysmon_ctrl.c
M src/simple_ctrl.h
2 files changed, 26 insertions(+), 19 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-sysmon refs/changes/16/12316/1

diff --git a/src/osysmon_ctrl.c b/src/osysmon_ctrl.c
index 25c2b4c..097e24c 100644
--- a/src/osysmon_ctrl.c
+++ b/src/osysmon_ctrl.c
@@ -38,14 +38,7 @@
 struct ctrl_client {
/* links to osysmon.ctrl_clients */
struct llist_head list;
-   struct {
-   /* name of this CTRL client */
-   const char *name;
-   /* remote host/IP */
-   const char *remote_host;
-   /* remote CTRL port */
-   uint16_t remote_port;
-   } cfg;
+   struct ctrl_cfg *cfg;
struct simple_ctrl_handle *sch;
/* list of ctrl_client_get_var objects */
struct llist_head get_vars;
@@ -69,7 +62,7 @@
 {
struct ctrl_client *cc;
llist_for_each_entry(cc, >ctrl_clients, list) {
-   if (!strcmp(name, cc->cfg.name))
+   if (!strcmp(name, cc->cfg->name))
return cc;
}
return NULL;
@@ -86,9 +79,14 @@
cc = talloc_zero(os, struct ctrl_client);
if (!cc)
return NULL;
-   cc->cfg.name = talloc_strdup(cc, name);
-   cc->cfg.remote_host = talloc_strdup(cc, host);
-   cc->cfg.remote_port = port;
+
+   cc->cfg = talloc_zero(cc, struct ctrl_cfg);
+   if (!cc->cfg)
+   return NULL;
+
+   cc->cfg->name = talloc_strdup(cc, name);
+   cc->cfg->remote_host = talloc_strdup(cc, host);
+   cc->cfg->remote_port = port;
INIT_LLIST_HEAD(>get_vars);
llist_add_tail(>list, >ctrl_clients);
/* FIXME */
@@ -163,10 +161,10 @@
struct ctrl_client *cc;
cc = ctrl_client_find(g_oss, argv[0]);
if (cc) {
-   if ((strcmp(cc->cfg.remote_host, argv[1])) ||
-   (cc->cfg.remote_port != atoi(argv[2]))) {
+   if ((strcmp(cc->cfg->remote_host, argv[1])) ||
+   (cc->cfg->remote_port != atoi(argv[2]))) {
vty_out(vty, "Client %s has different IP/port, please 
remove it first%s",
-   cc->cfg.name, VTY_NEWLINE);
+   cc->cfg->name, VTY_NEWLINE);
return CMD_WARNING;
}
} else
@@ -222,8 +220,8 @@
 static void write_one_ctrl_client(struct vty *vty, struct ctrl_client *cc)
 {
struct ctrl_client_get_var *ccgv;
-   vty_out(vty, "ctrl-client %s %s %u%s", cc->cfg.name,
-   cc->cfg.remote_host, cc->cfg.remote_port, VTY_NEWLINE);
+   vty_out(vty, "ctrl-client %s %s %u%s", cc->cfg->name,
+   cc->cfg->remote_host, cc->cfg->remote_port, VTY_NEWLINE);
llist_for_each_entry(ccgv, >get_vars, list) {
vty_out(vty, " get-variable %s%s", ccgv->cfg.name, VTY_NEWLINE);
if (ccgv->cfg.display_name)
@@ -266,11 +264,11 @@
 static int ctrl_client_poll(struct ctrl_client *cc, struct value_node *parent)
 {
struct ctrl_client_get_var *ccgv;
-   struct value_node *vn_clnt = value_node_add(parent, parent, 
cc->cfg.name, NULL);
+   struct value_node *vn_clnt = value_node_add(parent, parent, 
cc->cfg->name, NULL);

/* attempt to re-connect */
if (!cc->sch)
-   cc->sch = simple_ctrl_open(cc, cc->cfg.remote_host, 
cc->cfg.remote_port, 1000);
+   cc->sch = simple_ctrl_open(cc, cc->cfg->remote_host, 
cc->cfg->remote_port, 1000);
/* abort, if that failed */
if (!cc->sch) {
return -1;
diff --git a/src/simple_ctrl.h b/src/simple_ctrl.h
index 81a759d..f5aa770 100644
--- a/src/simple_ctrl.h
+++ b/src/simple_ctrl.h
@@ -2,6 +2,15 @@

 #include 

+struct ctrl_cfg {
+   /* name of this CTRL client */
+   const char *name;
+   /* remote host/IP */
+   const char *remote_host;
+   /* remote CTRL port */
+   uint16_t remote_port;
+};
+
 struct simple_ctrl_handle;

 struct simple_ctrl_handle *simple_ctrl_open(void *ctx, const char *host, 
uint16_t dport,

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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3520e14ca6e1b8e270dbd4b1bf2378fe486991ce
Gerrit-Change-Number: 12316
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-sysmon[master]: ctrl: log host/port on errors

2018-12-14 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12318


Change subject: ctrl: log host/port on errors
..

ctrl: log host/port on errors

In case of multiple ctrl-client entries in .cfg file it's impossible to
see which one is causing particular ctrl error. Fix this by introducing
macro wrapper for stderr logging which always show host:port relevant to
the error.

Change-Id: I788d51359965a66c54075a3971aa7824c3bfb0bf
Related: SYS#2655
---
M src/simple_ctrl.c
1 file changed, 18 insertions(+), 16 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-sysmon refs/changes/18/12318/1

diff --git a/src/simple_ctrl.c b/src/simple_ctrl.c
index 24d92b9..433f3bf 100644
--- a/src/simple_ctrl.c
+++ b/src/simple_ctrl.c
@@ -37,6 +37,8 @@

 #include "simple_ctrl.h"

+#define CTRL_ERR(c, fmt, args...) fprintf(stderr, "CTRL %s:%u error: " fmt 
"\n", c->remote_host, c->remote_port, ##args)
+
 /***
  * blocking I/O with timeout helpers
  ***/
@@ -110,7 +112,7 @@
fd = osmo_sock_init(AF_INET, SOCK_STREAM, IPPROTO_TCP, 
cfg->remote_host, cfg->remote_port,
OSMO_SOCK_F_CONNECT | OSMO_SOCK_F_NONBLOCK);
if (fd < 0) {
-   fprintf(stderr, "CTRL: error connecting socket: %s\n", 
strerror(errno));
+   CTRL_ERR(cfg, "connecting socket: %s", strerror(errno));
return NULL;
}

@@ -119,17 +121,17 @@
FD_SET(fd, );
rc = select(fd+1, NULL, , NULL, timeval_from_msec(tout_msec));
if (rc == 0) {
-   fprintf(stderr, "CTRL: timeout during connect\n");
+   CTRL_ERR(cfg, "timeout during connect");
goto out_close;
}
if (rc < 0) {
-   fprintf(stderr, "CTRL: error connecting socket: %s\n", 
strerror(errno));
+   CTRL_ERR(cfg, "error connecting socket: %s", strerror(errno));
goto out_close;
}

/* set FD blocking again */
if (ioctl(fd, FIONBIO, (unsigned char *)) < 0) {
-   fprintf(stderr, "CTRL: cannot set socket blocking: %s\n", 
strerror(errno));
+   CTRL_ERR(cfg, "cannot set socket blocking: %s", 
strerror(errno));
goto out_close;
}

@@ -156,7 +158,7 @@
talloc_free(sch);
 }

-static struct msgb *simple_ipa_receive(struct simple_ctrl_handle *sch)
+static struct msgb *simple_ipa_receive(const struct ctrl_cfg *cfg, struct 
simple_ctrl_handle *sch)
 {
struct ipaccess_head hh;
struct msgb *resp;
@@ -164,10 +166,10 @@

rc = read_timeout(sch->fd, (uint8_t *) , sizeof(hh), sch->tout_msec);
if (rc < 0) {
-   fprintf(stderr, "CTRL: Error during read: %d\n", rc);
+   CTRL_ERR(cfg, "read(): %d", rc);
return NULL;
} else if (rc < sizeof(hh)) {
-   fprintf(stderr, "CTRL: ERROR: short read (header)\n");
+   CTRL_ERR(cfg, "short read (header)");
return NULL;
}
len = ntohs(hh.len);
@@ -181,7 +183,7 @@
resp->l2h = resp->tail;
rc = read(sch->fd, resp->l2h, len);
if (rc < len) {
-   fprintf(stderr, "CTRL: ERROR: short read (payload)\n");
+   CTRL_ERR(cfg, "short read (payload)");
msgb_free(resp);
return NULL;
}
@@ -199,7 +201,7 @@

/* loop until we've received a CTRL message */
while (true) {
-   resp = simple_ipa_receive(sch);
+   resp = simple_ipa_receive(cfg, sch);
if (!resp)
return NULL;

@@ -213,13 +215,13 @@
*tmp = '\0';
return resp;
} else {
-   fprintf(stderr, "unknown IPA message %s\n", 
msgb_hexdump(resp));
+   CTRL_ERR(cfg, "unknown IPA message %s", 
msgb_hexdump(resp));
msgb_free(resp);
}
}
 }

-static int simple_ctrl_send(struct simple_ctrl_handle *sch, struct msgb *msg)
+static int simple_ctrl_send(const struct ctrl_cfg *cfg, struct 
simple_ctrl_handle *sch, struct msgb *msg)
 {
int rc;

@@ -228,10 +230,10 @@

rc = write_timeout(sch->fd, msg->data, msg->len, sch->tout_msec);
if (rc < 0) {
-   fprintf(stderr, "CTRL: Error during write: %d\n", rc);
+   CTRL_ERR(cfg, "write(): %d", rc);
return rc;
} else if (rc < msg->len) {
-   fprintf(s

Change in osmo-sysmon[master]: ctrl: pass config struct to all simple_ctrl_*()

2018-12-14 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12317


Change subject: ctrl: pass config struct to all simple_ctrl_*()
..

ctrl: pass config struct to all simple_ctrl_*()

Make basic logging-related information available to simple_ctrl_*()
functions.

Change-Id: I783dda27dfc5fd57401d971b2e970ede0efc7b2c
Related: SYS#2655
---
M src/osmo-ctrl-client.c
M src/osysmon_ctrl.c
M src/simple_ctrl.c
M src/simple_ctrl.h
4 files changed, 22 insertions(+), 25 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-sysmon refs/changes/17/12317/1

diff --git a/src/osmo-ctrl-client.c b/src/osmo-ctrl-client.c
index 83e9b7b..6a196eb 100644
--- a/src/osmo-ctrl-client.c
+++ b/src/osmo-ctrl-client.c
@@ -45,19 +45,18 @@
 int main(int argc, char **argv)
 {
struct simple_ctrl_handle *sch;
-   const char *host;
-   uint16_t port;
+   struct ctrl_cfg cfg = { .name = "osmo-ctrl-client" };
int rc;

if (argc < 4)
exit_help();

-   host = argv[1];
-   port = atoi(argv[2]);
+   cfg.remote_host = argv[1];
+   cfg.remote_port = atoi(argv[2]);

osmo_init_logging2(NULL, _info);

-   sch = simple_ctrl_open(NULL, host, port, 1000);
+   sch = simple_ctrl_open(NULL, , 1000);
if (!sch)
exit(1);

@@ -65,20 +64,20 @@
char *val;
if (argc < 5)
exit_help();
-   val = simple_ctrl_get(sch, argv[4]);
+   val = simple_ctrl_get(, sch, argv[4]);
if (!val)
exit(2);
printf("%s\n", val);
} else if (!strcmp(argv[3], "set")) {
if (argc < 6)
exit_help();
-   rc = simple_ctrl_set(sch, argv[4], argv[5]);
+   rc = simple_ctrl_set(, sch, argv[4], argv[5]);
if (rc < 0)
exit(1);
} else if (!strcmp(argv[3], "monitor")) {
simple_ctrl_set_timeout(sch, 0);
while (true) {
-   struct msgb *msg = simple_ctrl_receive(sch);
+   struct msgb *msg = simple_ctrl_receive(, sch);
if (!msg)
exit(1);
printf("%s", (char *) msgb_l2(msg));
diff --git a/src/osysmon_ctrl.c b/src/osysmon_ctrl.c
index 097e24c..bcab94c 100644
--- a/src/osysmon_ctrl.c
+++ b/src/osysmon_ctrl.c
@@ -268,14 +268,14 @@

/* attempt to re-connect */
if (!cc->sch)
-   cc->sch = simple_ctrl_open(cc, cc->cfg->remote_host, 
cc->cfg->remote_port, 1000);
+   cc->sch = simple_ctrl_open(cc, cc->cfg, 1000);
/* abort, if that failed */
if (!cc->sch) {
return -1;
}

llist_for_each_entry(ccgv, >get_vars, list) {
-   char *value = simple_ctrl_get(cc->sch, ccgv->cfg.name);
+   char *value = simple_ctrl_get(cc->cfg, cc->sch, ccgv->cfg.name);

/* FIXME: Distinguish between ERROR reply and
 * connection issues */
diff --git a/src/simple_ctrl.c b/src/simple_ctrl.c
index 2261323..24d92b9 100644
--- a/src/simple_ctrl.c
+++ b/src/simple_ctrl.c
@@ -100,15 +100,14 @@
uint32_t tout_msec;
 };

-struct simple_ctrl_handle *simple_ctrl_open(void *ctx, const char *host, 
uint16_t dport,
-   uint32_t tout_msec)
+struct simple_ctrl_handle *simple_ctrl_open(void *ctx, const struct ctrl_cfg 
*cfg, uint32_t tout_msec)
 {
struct simple_ctrl_handle *sch;
fd_set writeset;
int off = 0;
int rc, fd;

-   fd = osmo_sock_init(AF_INET, SOCK_STREAM, IPPROTO_TCP, host, dport,
+   fd = osmo_sock_init(AF_INET, SOCK_STREAM, IPPROTO_TCP, 
cfg->remote_host, cfg->remote_port,
OSMO_SOCK_F_CONNECT | OSMO_SOCK_F_NONBLOCK);
if (fd < 0) {
fprintf(stderr, "CTRL: error connecting socket: %s\n", 
strerror(errno));
@@ -191,7 +190,7 @@
return resp;
 }

-struct msgb *simple_ctrl_receive(struct simple_ctrl_handle *sch)
+struct msgb *simple_ctrl_receive(const struct ctrl_cfg *cfg, struct 
simple_ctrl_handle *sch)
 {
struct msgb *resp;
struct ipaccess_head *ih;
@@ -241,7 +240,7 @@
}
 }

-static struct msgb *simple_ctrl_xceive(struct simple_ctrl_handle *sch, struct 
msgb *msg)
+static struct msgb *simple_ctrl_xceive(const struct ctrl_cfg *cfg, struct 
simple_ctrl_handle *sch, struct msgb *msg)
 {
int rc;

@@ -250,10 +249,10 @@
return NULL;

/* FIXME: ignore any TRAP */
-   return simple_ctrl_receive(sch);
+   return simple_ctrl_receive(cfg, sch);
 }

-char *simple_ctrl_get(struct simple_ctrl_han

Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11992 )

Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..


Patch Set 7:

(1 comment)

https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h
File include/osmocom/msc/gsm_data.h:

https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h@188
PS7, Line 188:  int ncss_guard_timeout;
> Same as mncc_guard_timeout. Integer is expected by libosmocore: […]
If that's the only reason than it's better to use uint8_t (which can be casted 
to int without any issues) to keep it consistent with vty code.



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 7
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Fri, 14 Dec 2018 14:29:43 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libosmocore[master]: LCLS: add gsm0808_create_ass2()

2018-12-14 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/11826 )

Change subject: LCLS: add gsm0808_create_ass2()
..

LCLS: add gsm0808_create_ass2()

It allows setting additional assignment parameters explicitly.

Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba
Related: OS#2487
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
5 files changed, 133 insertions(+), 6 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Stefan Sperling: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/include/osmocom/gsm/gsm0808.h b/include/osmocom/gsm/gsm0808.h
index e3fb6ad..79d89e5 100644
--- a/include/osmocom/gsm/gsm0808.h
+++ b/include/osmocom/gsm/gsm0808.h
@@ -64,6 +64,12 @@
const struct sockaddr_storage *ss,
const struct gsm0808_speech_codec_list *scl,
const uint32_t *ci);
+struct msgb *gsm0808_create_ass2(const struct gsm0808_channel_type *ct,
+const uint16_t *cic,
+const struct sockaddr_storage *ss,
+const struct gsm0808_speech_codec_list *scl,
+const uint32_t *ci,
+const uint8_t *kc, const struct osmo_lcls 
*lcls);
 struct msgb *gsm0808_create_ass_compl(uint8_t rr_cause, uint8_t chosen_channel,
  uint8_t encr_alg_id, uint8_t speech_mode,
  const struct sockaddr_storage *ss,
diff --git a/src/gsm/gsm0808.c b/src/gsm/gsm0808.c
index e951ab1..69da57d 100644
--- a/src/gsm/gsm0808.c
+++ b/src/gsm/gsm0808.c
@@ -425,18 +425,22 @@
return msg;
 }

-/*! Create BSSMAP Assignment Request message, 3GPP TS 48.008 §3.2.1.1
+/*! Create BSSMAP Assignment Request message, 3GPP TS 48.008 §3.2.1.1.
+ *  This is identical to gsm0808_create_ass(), but adds KC and LCLS IEs.
  *  \param[in] ct Channel Type
  *  \param[in] cic Circuit Identity Code (Classic A only)
  *  \param[in] ss Socket Address of MSC-side RTP socket (AoIP only)
  *  \param[in] scl Speech Codec List (AoIP only)
  *  \param[in] ci Call Identifier (Optional), §3.2.2.105
+ *  \param[in] kc Kc128 ciphering key (Optional, A5/4), §3.2.2.109
+ *  \param[in] lcls Optional LCLS parameters
  *  \returns callee-allocated msgb with BSSMAP Assignment Request message */
-struct msgb *gsm0808_create_ass(const struct gsm0808_channel_type *ct,
-   const uint16_t *cic,
-   const struct sockaddr_storage *ss,
-   const struct gsm0808_speech_codec_list *scl,
-   const uint32_t *ci)
+struct msgb *gsm0808_create_ass2(const struct gsm0808_channel_type *ct,
+const uint16_t *cic,
+const struct sockaddr_storage *ss,
+const struct gsm0808_speech_codec_list *scl,
+const uint32_t *ci,
+const uint8_t *kc, const struct osmo_lcls 
*lcls)
 {
/* See also: 3GPP TS 48.008 3.2.1.1 ASSIGNMENT REQUEST */
struct msgb *msg;
@@ -481,6 +485,27 @@
  (uint8_t *) & ci_sw);
}

+   if (kc)
+   msgb_tv_fixed_put(msg, GSM0808_IE_KC_128, 16, kc);
+
+   if (lcls) {
+   /* LCLS: §3.2.2.115 Global Call Reference */
+   if (lcls->gcr)
+   gsm0808_enc_gcr(msg, lcls->gcr);
+
+   /* LCLS: §3.2.2.116 Configuration */
+   if (lcls->config != GSM0808_LCLS_CFG_NA)
+   msgb_tv_put(msg, GSM0808_IE_LCLS_CONFIG, lcls->config);
+
+   /* LCLS: §3.2.2.117 Connection Status Control */
+   if (lcls->control != GSM0808_LCLS_CSC_NA)
+   msgb_tv_put(msg, GSM0808_IE_LCLS_CONN_STATUS_CTRL, 
lcls->control);
+
+   /* LCLS: §3.2.2.118 Correlation-Not-Needed */
+   if (!lcls->corr_needed)
+   msgb_v_put(msg, GSM0808_IE_LCLS_CORR_NOT_NEEDED);
+   }
+
/* push the bssmap header */
msg->l3h =
msgb_tv_push(msg, BSSAP_MSG_BSS_MANAGEMENT, msgb_length(msg));
@@ -488,6 +513,22 @@
return msg;
 }

+/*! Create BSSMAP Assignment Request message, 3GPP TS 48.008 §3.2.1.1.
+ *  \param[in] ct Channel Type
+ *  \param[in] cic Circuit Identity Code (Classic A only)
+ *  \param[in] ss Socket Address of MSC-side RTP socket (AoIP only)
+ *  \param[in] scl Speech Codec List (AoIP only)
+ *  \param[in] ci Call Identifier (Optional), §3.2.2.105
+ *  \returns callee-allocated msgb with BSSMAP Assignment Req

Change in libosmocore[master]: LCLS, TS 48.008: add GCR IE encoding/decoding

2018-12-14 Thread Max
Max has removed a vote on this change.

Change subject: LCLS, TS 48.008: add GCR IE encoding/decoding
..


Removed Code-Review+2 by Max 
--
To view, visit https://gerrit.osmocom.org/12020
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I82ce0207dc8de50689a8806c6471ad7fbae6219d
Gerrit-Change-Number: 12020
Gerrit-PatchSet: 18
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 


Change in libosmocore[master]: LCLS, TS 29.205: add GCR routines

2018-12-14 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/11827 )

Change subject: LCLS, TS 29.205: add GCR routines
..

LCLS, TS 29.205: add GCR routines

Add functions to encode and decode Global Call Reference as per
3GPP TS 29.205 Table B 2.1.9.1 add corresponding tests.

Change-Id: Iee95aa4e5c056645b6cb5667e4a067097d52dfbf
Related: OS#2487
---
M include/Makefile.am
M include/osmocom/gsm/gsm0808_utils.h
A include/osmocom/gsm/gsm29205.h
M src/gsm/Makefile.am
A src/gsm/gsm29205.c
M src/gsm/libosmogsm.map
M tests/Makefile.am
A tests/gsm29205/gsm29205_test.c
A tests/gsm29205/gsm29205_test.ok
M tests/testsuite.at
10 files changed, 263 insertions(+), 4 deletions(-)

Approvals:
  Harald Welte: Looks good to me, but someone else must approve
  Pau Espin Pedrol: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/Makefile.am b/include/Makefile.am
index 366fd70..ccf9e10 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -86,6 +86,7 @@
osmocom/coding/gsm0503_interleaving.h \
osmocom/coding/gsm0503_coding.h \
osmocom/gsm/gsm0808.h \
+   osmocom/gsm/gsm29205.h \
osmocom/gsm/gsm0808_utils.h \
osmocom/gsm/gsm23003.h \
osmocom/gsm/gsm29118.h \
diff --git a/include/osmocom/gsm/gsm0808_utils.h 
b/include/osmocom/gsm/gsm0808_utils.h
index 90ff677..5d5803b 100644
--- a/include/osmocom/gsm/gsm0808_utils.h
+++ b/include/osmocom/gsm/gsm0808_utils.h
@@ -62,7 +62,7 @@
 struct osmo_lcls {
enum gsm0808_lcls_config config;   /**< §3.2.2.116 Configuration */
enum gsm0808_lcls_control control; /**< §3.2.2.117 Connection Status 
Control */
-   struct gsm29205_gcr *gcr;  /**< §3.2.2.115 Global Call 
Reference */
+   struct osmo_gcr_parsed *gcr;   /**< §3.2.2.115 Global Call 
Reference */
bool corr_needed;  /**< §3.2.2.118 
Correlation-Not-Needed */
 };

diff --git a/include/osmocom/gsm/gsm29205.h b/include/osmocom/gsm/gsm29205.h
new file mode 100644
index 000..0c3c153
--- /dev/null
+++ b/include/osmocom/gsm/gsm29205.h
@@ -0,0 +1,41 @@
+/*! \defgroup gsm29205 3GPP TS 29.205
+ *  @{
+ *  \file gsm29205.h */
+/*
+ * (C) 2018 by sysmocom - s.f.m.c. GmbH
+ * All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#pragma once
+
+#include 
+
+#include 
+
+#define OSMO_GCR_MIN_LEN 13
+
+/*! Parsed representation of Global Call Reference, 3GPP TS 29.205 Table B 
2.1.9.1. */
+struct osmo_gcr_parsed {
+   uint8_t net[5];  /** Network ID, ITU-T Q.1902.3 */
+   uint8_t net_len; /** length (3-5 octets) of gsm29205_gcr#net */
+   uint16_t node;   /** Node ID */
+   uint8_t cr[5];   /** Call Reference ID */
+};
+
+uint8_t osmo_enc_gcr(struct msgb *msg, const struct osmo_gcr_parsed *g);
+int osmo_dec_gcr(struct osmo_gcr_parsed *gcr, const uint8_t *elem, uint8_t 
len);
diff --git a/src/gsm/Makefile.am b/src/gsm/Makefile.am
index e28ea33..ccb38ad 100644
--- a/src/gsm/Makefile.am
+++ b/src/gsm/Makefile.am
@@ -24,7 +24,7 @@
gsm_utils.c rsl.c gsm48.c gsm48_ie.c gsm0808.c 
sysinfo.c \
gprs_cipher_core.c gprs_rlc.c gsm0480.c abis_nm.c 
gsm0502.c \
gsm0411_utils.c gsm0411_smc.c gsm0411_smr.c gsm0414.c \
-   lapd_core.c lapdm.c kasumi.c gsm_04_08_gprs.c \
+   lapd_core.c lapdm.c kasumi.c gsm29205.c 
gsm_04_08_gprs.c \
auth_core.c auth_comp128v1.c auth_comp128v23.c \
auth_milenage.c milenage/aes-encblock.c gea.c \
milenage/aes-internal.c milenage/aes-internal-enc.c \
diff --git a/src/gsm/gsm29205.c b/src/gsm/gsm29205.c
new file mode 100644
index 000..0ef29b7
--- /dev/null
+++ b/src/gsm/gsm29205.c
@@ -0,0 +1,93 @@
+/*
+ * (C) 2018 by sysmocom - s.f.m.c. GmbH
+ * All Rights Reserved
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Fou

Change in libosmocore[master]: LCLS, TS 48.008: add GCR IE encoding/decoding

2018-12-14 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12020 )

Change subject: LCLS, TS 48.008: add GCR IE encoding/decoding
..

LCLS, TS 48.008: add GCR IE encoding/decoding

* add functions to encode Global Call. Ref. from TS 29.205 as 3GPP TS
  48.008 §3.2.2.115 information element
* add corresponding tests

Change-Id: I82ce0207dc8de50689a8806c6471ad7fbae6219d
---
M include/osmocom/gsm/gsm0808_utils.h
M src/gsm/gsm0808_utils.c
M src/gsm/libosmogsm.map
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
5 files changed, 123 insertions(+), 0 deletions(-)

Approvals:
  Pau Espin Pedrol: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/gsm/gsm0808_utils.h 
b/include/osmocom/gsm/gsm0808_utils.h
index 5d5803b..22050b5 100644
--- a/include/osmocom/gsm/gsm0808_utils.h
+++ b/include/osmocom/gsm/gsm0808_utils.h
@@ -27,6 +27,7 @@

 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -82,6 +83,10 @@
const struct sockaddr_storage *ss);
 int gsm0808_dec_aoip_trasp_addr(struct sockaddr_storage *ss,
const uint8_t *elem, uint8_t len);
+
+uint8_t gsm0808_enc_gcr(struct msgb *msg, const struct osmo_gcr_parsed *g);
+int gsm0808_dec_gcr(struct osmo_gcr_parsed *g, const struct tlv_parsed *tp);
+
 uint8_t gsm0808_enc_speech_codec(struct msgb *msg,
 const struct gsm0808_speech_codec *sc);
 int gsm0808_dec_speech_codec(struct gsm0808_speech_codec *sc,
diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c
index 38a8664..a04adde 100644
--- a/src/gsm/gsm0808_utils.c
+++ b/src/gsm/gsm0808_utils.c
@@ -508,6 +508,41 @@
return (int)(elem - old_elem);
 }

+/*! Create BSSMAP Global Call Reference, 3GPP TS 48.008 §3.2.2.115.
+ *  \param[out] msg Message Buffer for appending IE
+ *  \param[in] g Global Call Reference, 3GPP TS 29.205 Table B 2.1.9.1
+ *  \returns number of bytes added to \a msg or 0 on error */
+uint8_t gsm0808_enc_gcr(struct msgb *msg, const struct osmo_gcr_parsed *g)
+{
+   uint8_t enc, *len = msgb_tl_put(msg, GSM0808_IE_GLOBAL_CALL_REF);
+
+   enc = osmo_enc_gcr(msg, g);
+   if (!enc)
+   return 0;
+
+   *len = enc;
+   return enc + 2; /* type (1 byte) + length (1 byte) */
+}
+
+/*! Decode BSSMAP Global Call Reference, 3GPP TS 29.205 Table B 2.1.9.1.
+ *  \param[out] gcr Caller-provided memory to store Global Call Reference
+ *  \param[in] elem IE value to be decoded
+ *  \param[in] len Length of \a elem in bytes
+ *  \returns number of bytes parsed; negative on error */
+int gsm0808_dec_gcr(struct osmo_gcr_parsed *gcr, const struct tlv_parsed *tp)
+{
+   int ret;
+   const uint8_t *buf = TLVP_VAL_MINLEN(tp, GSM0808_IE_GLOBAL_CALL_REF, 
OSMO_GCR_MIN_LEN);
+   if (!buf)
+   return -EINVAL;
+
+   ret = osmo_dec_gcr(gcr, buf, TLVP_LEN(tp, GSM0808_IE_GLOBAL_CALL_REF));
+   if (ret < 0)
+   return -ENOENT;
+
+   return 2 + ret;
+}
+
 /*! Encode TS 08.08 Encryption Information IE
  *  \param[out] msg Message Buffer to which IE is to be appended
  *  \param[in] ei Encryption Information to be encoded
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index e85ed6d..94ae76a 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -219,6 +219,8 @@
 gsm0808_lcls_config_names;
 gsm0808_lcls_control_names;
 gsm0808_lcls_status_names;
+gsm0808_enc_gcr;
+gsm0808_dec_gcr;

 gsm29118_msgb_alloc;
 gsm29118_create_alert_req;
diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c
index 63b8720..f0f3165 100644
--- a/tests/gsm0808/gsm0808_test.c
+++ b/tests/gsm0808/gsm0808_test.c
@@ -22,6 +22,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 

 #include 
 #include 
@@ -597,6 +599,75 @@
msgb_free(in_msg);
 }

+static void test_enc_dec_gcr()
+{
+   static const uint8_t res[] = {
+   GSM0808_IE_GLOBAL_CALL_REF,
+   0x0d, /* GCR length */
+   0x03, /* .net_len */
+   0xf1, 0xf2, 0xf3, /* .net */
+   0x02, /* .node length */
+   0xde, 0xad, /* .node */
+   0x05, /* length of Call. Ref. */
+   0x41, 0x42, 0x43, 0x44, 0x45 /* .cr - Call. Ref. */
+   };
+   uint8_t len;
+   struct msgb *msg;
+   struct osmo_gcr_parsed p = { 0 }, g = {
+   .net_len = 3,
+   .net = { 0xf1, 0xf2, 0xf3 },
+   .node = 0xDEAD,
+   .cr = { 0x41, 0x42, 0x43, 0x44, 0x45 },
+   };
+   int rc;
+   struct tlv_parsed tp;
+   msg = msgb_alloc_headroom(BSSMAP_MSG_SIZE, BSSMAP_MSG_HEADROOM, "global 
call reference");
+   if (!msg)
+   return;
+
+   len = gsm0808_enc_gcr(msg, );
+   printf("Testing Global Call Reference IE encoder...\n\t%d 

Change in libosmocore[master]: LCLS, TS 29.205: add GCR routines

2018-12-14 Thread Max
Max has removed a vote on this change.

Change subject: LCLS, TS 29.205: add GCR routines
..


Removed Code-Review+2 by Max 
--
To view, visit https://gerrit.osmocom.org/11827
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Iee95aa4e5c056645b6cb5667e4a067097d52dfbf
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 24
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11992 )

Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..


Patch Set 7: Code-Review-1

(4 comments)

https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h
File include/osmocom/msc/gsm_data.h:

https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h@188
PS7, Line 188:  int ncss_guard_timeout;
Why do you use int in here?


https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c
File src/libmsc/gsm_09_11.c:

https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c@141
PS7, Line 141:  /* Init self-destruction timer */
That sounds ominous - I like it :)


https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c@477
PS7, Line 477:  if (net->ncss_guard_timeout > 0) {
Seems like we always check ncss_guard_timeout > 0


https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/msc_vty.c
File src/libmsc/msc_vty.c:

https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/msc_vty.c@382
PS7, Line 382:   "guard timer value (sec.), or 0 to disable\n")
So ncss_guard_timeout is always positive and fit into uint8_t.



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 7
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Fri, 14 Dec 2018 13:14:49 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in osmo-hlr[master]: gsupclient: add osmo_gsup_msg_enc_send()

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11989 )

Change subject: gsupclient: add osmo_gsup_msg_enc_send()
..


Patch Set 4: Code-Review-1

(3 comments)

Sorry, haven't noticed those earlier.

https://gerrit.osmocom.org/#/c/11989/4/src/gsupclient/gsup_client.c
File src/gsupclient/gsup_client.c:

https://gerrit.osmocom.org/#/c/11989/4/src/gsupclient/gsup_client.c@366
PS4, Line 366:  *  \returns 0 in case of success, otherwise errno
Errno is positive and we return negative I believe - better state that 
explicitly.


https://gerrit.osmocom.org/#/c/11989/4/src/gsupclient/gsup_client.c@381
PS4, Line 381:  if (rc) {
I think it's better to explicitly check rc < 0 in here to make reading code 
easier.


https://gerrit.osmocom.org/#/c/11989/4/src/gsupclient/gsup_client.c@387
PS4, Line 387:  if (rc) {
And here too.



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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0589ff27933e9bca2bcf93b8259004935778db8f
Gerrit-Change-Number: 11989
Gerrit-PatchSet: 4
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Fri, 14 Dec 2018 13:07:38 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add/clean big-endian packed structs (struct_endianess.py)

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11787 )

Change subject: add/clean big-endian packed structs (struct_endianess.py)
..


Patch Set 3: Code-Review-1

(1 comment)

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

https://gerrit.osmocom.org/#/c/11787/3//COMMIT_MSG@7
PS3, Line 7: add/clean big-endian packed structs (struct_endianess.py)
Please add exact call to struct_endianess.py you've used to make this change to 
make this reproducible. Also, please clarify if any manual editing was 
necessary or it's purely automated code change.



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0b99d76932aeb03e93bd0c62d3bf025dec5f9d2
Gerrit-Change-Number: 11787
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Assignee: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Fri, 14 Dec 2018 13:02:08 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11992 )

Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..


Patch Set 7:

Is this some named timer from the spec or it's just smth which makes sense in 
practice?


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 7
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-CC: Max 
Gerrit-Comment-Date: Fri, 14 Dec 2018 12:59:55 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-msc[master]: tweak comment to indicate sub_pres_vlr fsm as dead code

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12234 )

Change subject: tweak comment to indicate sub_pres_vlr fsm as dead code
..


Patch Set 3:

(1 comment)

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

https://gerrit.osmocom.org/#/c/12234/3//COMMIT_MSG@18
PS3, Line 18: this models some FSM definition from 3GPP specs, and we have a 
couple other
It would be better to add actual spec reference here as well as in the comment.



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99
Gerrit-Change-Number: 12234
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Max 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Fri, 14 Dec 2018 12:57:24 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-msc[master]: vlr_lu_fsm: drop unused out_state INIT -> WAIT_IMEI

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12236 )

Change subject: vlr_lu_fsm: drop unused out_state INIT -> WAIT_IMEI
..


Patch Set 2:

Do you need to update some .adoc or .msc as well or this isn't documented?


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18ef9e8c96b52401d98f49dc410f13681231b533
Gerrit-Change-Number: 12236
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Max 
Gerrit-Comment-Date: Fri, 14 Dec 2018 12:54:55 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmocom-bb[master]: trx_toolkit/ctrl_if_trx.py: drop meaningless warnings

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12291 )

Change subject: trx_toolkit/ctrl_if_trx.py: drop meaningless warnings
..


Patch Set 2:

(1 comment)

Could you clarify in commit message if RESET was introduced or why removing 
this TODO is the right thing.

https://gerrit.osmocom.org/#/c/12291/2/src/target/trx_toolkit/ctrl_if_trx.py
File src/target/trx_toolkit/ctrl_if_trx.py:

https://gerrit.osmocom.org/#/c/12291/2/src/target/trx_toolkit/ctrl_if_trx.py@a126
PS2, Line 126:
That's rather puzzling TODO in here.



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

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a69ebceed5aa724093e6d1b23faad8c16705055
Gerrit-Change-Number: 12291
Gerrit-PatchSet: 2
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-CC: Max 
Gerrit-Comment-Date: Fri, 14 Dec 2018 12:52:57 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-hlr[master]: gsupclient: add osmo_gsup_msg_enc_send()

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11989 )

Change subject: gsupclient: add osmo_gsup_msg_enc_send()
..


Patch Set 3: Code-Review-1

(2 comments)

Please double check.

https://gerrit.osmocom.org/#/c/11989/3/src/gsupclient/gsup_client.c
File src/gsupclient/gsup_client.c:

https://gerrit.osmocom.org/#/c/11989/3/src/gsupclient/gsup_client.c@369
PS3, Line 369: struct osmo_gsup_message *gsup_msg)
Why this isn't const? What modifies it?


https://gerrit.osmocom.org/#/c/11989/3/src/gsupclient/gsup_client.c@380
PS3, Line 380:  rc = osmo_gsup_encode(gsup_msgb, gsup_msg);
gsup_msg is const in here according to osmo_gsup_encode() type signature.



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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0589ff27933e9bca2bcf93b8259004935778db8f
Gerrit-Change-Number: 11989
Gerrit-PatchSet: 3
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Fri, 14 Dec 2018 12:51:12 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in libosmocore[master]: LCLS, TS 48.008: add GCR IE encoding/decoding

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12020 )

Change subject: LCLS, TS 48.008: add GCR IE encoding/decoding
..


Patch Set 18: Code-Review+2

Will merge at the end of the day unless some objections will be raised.


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82ce0207dc8de50689a8806c6471ad7fbae6219d
Gerrit-Change-Number: 12020
Gerrit-PatchSet: 18
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Fri, 14 Dec 2018 12:33:55 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: LCLS, TS 29.205: add GCR routines

2018-12-14 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11827 )

Change subject: LCLS, TS 29.205: add GCR routines
..


Patch Set 24: Code-Review+2


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee95aa4e5c056645b6cb5667e4a067097d52dfbf
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 24
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Fri, 14 Dec 2018 12:32:54 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in meta-telephony[201705]: Introduce recipe for osmo-sysmon

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12300 )

Change subject: Introduce recipe for osmo-sysmon
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: meta-telephony
Gerrit-Branch: 201705
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e1da3a930e04d17ba0461874d60e6f204bc5b45
Gerrit-Change-Number: 12300
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Thu, 13 Dec 2018 16:51:42 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in meta-telephony[201705]: Introduce recipe liboping 1.10.0

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12299 )

Change subject: Introduce recipe liboping 1.10.0
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: meta-telephony
Gerrit-Branch: 201705
Gerrit-MessageType: comment
Gerrit-Change-Id: I98ffbf39870b309da0ccdde81047d4c124633ad4
Gerrit-Change-Number: 12299
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Thu, 13 Dec 2018 16:47:49 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: LCLS, TS 29.205: add GCR routines

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/11827 )

Change subject: LCLS, TS 29.205: add GCR routines
..


Patch Set 24:

(1 comment)

https://gerrit.osmocom.org/#/c/11827/23/tests/gsm29205/gsm29205_test.c
File tests/gsm29205/gsm29205_test.c:

https://gerrit.osmocom.org/#/c/11827/23/tests/gsm29205/gsm29205_test.c@69
PS23, Line 69:  abort();
> Same problem as pointed out by Pau in 
> https://gerrit.osmocom.org/c/libosmocore/+/12020 […]
Done



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee95aa4e5c056645b6cb5667e4a067097d52dfbf
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 24
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Thu, 13 Dec 2018 16:22:06 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-ttcn3-hacks[master]: MSC: adjust gsup log level to error

2018-12-13 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12298


Change subject: MSC: adjust gsup log level to error
..

MSC: adjust gsup log level to error

The HLR emulation is active only for short time during each test so we
got lots of useless "gsup_client.c:73 GSUP connecting to 127.0.0.1:4222"
messages due to continuous attempts to reconnect to HLR. Let's use error
log level to obtain relevant errors (if any).

Change-Id: If79db99ceb7a9d4e2ec8d8b375aa628c4088c65c
---
M msc/osmo-msc.cfg
1 file changed, 1 insertion(+), 1 deletion(-)



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

diff --git a/msc/osmo-msc.cfg b/msc/osmo-msc.cfg
index ddac4cd..79d509e 100644
--- a/msc/osmo-msc.cfg
+++ b/msc/osmo-msc.cfg
@@ -34,7 +34,7 @@
  logging level lctrl info
  logging level lgtp notice
  logging level lstats notice
- logging level lgsup notice
+ logging level lgsup error
  logging level loap notice
  logging level lss7 notice
  logging level lsccp notice

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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If79db99ceb7a9d4e2ec8d8b375aa628c4088c65c
Gerrit-Change-Number: 12298
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-sgsn[master]: make gsup ipa name configurable in osmo-sgsn.cfg

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12243 )

Change subject: make gsup ipa name configurable in osmo-sgsn.cfg
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12243/3/src/gprs/sgsn_vty.c
File src/gprs/sgsn_vty.c:

https://gerrit.osmocom.org/#/c/12243/3/src/gprs/sgsn_vty.c@1454
PS3, Line 1454: parsing_config_file = true;
Why initializing it here instead of where it's declared?



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2f65fed9f56b9718e8a9647e3f01dce69870c1f
Gerrit-Change-Number: 12243
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Thu, 13 Dec 2018 15:39:38 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-ttcn3-hacks[master]: MSC: match default expectation with config

2018-12-13 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12296


Change subject: MSC: match default expectation with config
..

MSC: match default expectation with config

In MSC_Tests.default we expect /tmp/mncc.sock as MNCC socket path -
let's match this expectation with osmo-msc.cfg to make sure that tests
work out of the box without the need to use specific command-line
option.

Change-Id: I540645ef4b1e08d05b89251f074af84a516e7a88
---
M msc/osmo-msc.cfg
1 file changed, 1 insertion(+), 0 deletions(-)



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

diff --git a/msc/osmo-msc.cfg b/msc/osmo-msc.cfg
index 3f9e192..ddac4cd 100644
--- a/msc/osmo-msc.cfg
+++ b/msc/osmo-msc.cfg
@@ -73,6 +73,7 @@
  cs7-instance-iu 0
  mgw remote-ip 127.0.0.1
  emergency-call route-to-msisdn 112
+ mncc external /tmp/mncc.sock
 mncc-int
  default-codec tch-f fr
  default-codec tch-h hr

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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I540645ef4b1e08d05b89251f074af84a516e7a88
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-sysmon[master]: Introduce systemd service file and install with autotools

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12286 )

Change subject: Introduce systemd service file and install with autotools
..


Patch Set 2: Code-Review-1

Is it enabled by default in OE? If so than it might interfere with normal 
testing/use so I propose to abandon this altogether or move to WIP until we 
actually have the hypothetical use case described by Pau.


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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4875b74bcf2b6232d915070b77fc202a03ef997
Gerrit-Change-Number: 12286
Gerrit-PatchSet: 2
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Thu, 13 Dec 2018 15:12:54 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: Use msgb helper instead of local #define for debug print

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 )

Change subject: Use msgb helper instead of local #define for debug print
..


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/12271/1/tests/gsm0808/gsm0808_test.c
File tests/gsm0808/gsm0808_test.c:

https://gerrit.osmocom.org/#/c/12271/1/tests/gsm0808/gsm0808_test.c@40
PS1, Line 40: #define VERIFY(msg, data, len)
\
> I would rather change this to: […]
Unfortunately I haven't found a way to do this with spatch. Feel free to add 
corrected variant of .spatch if you know how.



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b
Gerrit-Change-Number: 12271
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Thu, 13 Dec 2018 15:05:46 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-sysmon[master]: Install systemd services with autotools

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12286 )

Change subject: Install systemd services with autotools
..


Patch Set 1:

(1 comment)

In general, see comment on the previous patch: I'm not convinced we would ever 
run this as daemon because the whole purpose of this is to regularly display 
stuff on the console. Hence detaching it from the console seems kinda pointless 
to me.

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

https://gerrit.osmocom.org/#/c/12286/1//COMMIT_MSG@7
PS1, Line 7: Install systemd services with autotools
That's misleading - you're not just installing it, you've also added the 
service etc. More elaborate description please in such cases.



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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4875b74bcf2b6232d915070b77fc202a03ef997
Gerrit-Change-Number: 12286
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-CC: Max 
Gerrit-Comment-Date: Thu, 13 Dec 2018 11:11:12 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-sysmon[master]: Install systemd services with autotools

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12286 )

Change subject: Install systemd services with autotools
..


Patch Set 1: Code-Review-1


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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4875b74bcf2b6232d915070b77fc202a03ef997
Gerrit-Change-Number: 12286
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Comment-Date: Thu, 13 Dec 2018 11:11:17 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-sysmon[master]: Move source code to src subdir

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12284 )

Change subject: Move source code to src subdir
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1aa2893e2b274f8d087a0d2f126486cd4afcbdfe
Gerrit-Change-Number: 12284
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Comment-Date: Thu, 13 Dec 2018 11:07:51 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-sysmon[master]: Add cmdline option parsing support

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12285 )

Change subject: Add cmdline option parsing support
..


Patch Set 1: Code-Review-1

(1 comment)

This program regularly print stuff on screen based on config file - kinda 
similar to "watch". Why and when would we want to run it as daemon?

https://gerrit.osmocom.org/#/c/12285/1/src/osysmon_main.c
File src/osysmon_main.c:

https://gerrit.osmocom.org/#/c/12285/1/src/osysmon_main.c@224
PS1, Line 224:  rc = osmo_daemonize();
What would be use-case for this?



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

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742974bd1440b09b49d26703c13361dd1c41008b
Gerrit-Change-Number: 12285
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Comment-Date: Thu, 13 Dec 2018 11:06:08 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: require 'ipa-name' option to be set via config file

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12293 )

Change subject: require 'ipa-name' option to be set via config file
..


Patch Set 1:

(2 comments)

That's a minor note though - mostly commenting just in case I need the same 
thing for some config option in future :)

https://gerrit.osmocom.org/#/c/12293/1/src/osmo-msc/msc_main.c
File src/osmo-msc/msc_main.c:

https://gerrit.osmocom.org/#/c/12293/1/src/osmo-msc/msc_main.c@518
PS1, Line 518: bool msc_parsing_config_file = false;
Why not initialize it with true to begin with?


https://gerrit.osmocom.org/#/c/12293/1/src/osmo-msc/msc_main.c@572
PS1, Line 572:  msc_parsing_config_file = true;
So, here it's set to true.



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6cff91793e646e0396e8f1bc87d0f52709e5f12a
Gerrit-Change-Number: 12293
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-CC: Max 
Gerrit-Comment-Date: Thu, 13 Dec 2018 11:01:01 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-sgsn[master]: ACL: integrate sanitize check into sgsn_acl_* functions

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12227 )

Change subject: ACL: integrate sanitize check into sgsn_acl_* functions
..


Patch Set 5:

> Patch Set 5:
> I did, maybe it was not clear enough: "I think sanitizing the imsi should be 
> done by caller of sgsn_acl_* based on where the information come from (from 
> the wire or from known sanitized source)."
> 

Hmm, I was sure I've addressed it already. Anyway, the "where the information 
come from" is not applicable because it only comes from a single source which 
is not trusted so we always sanitize it.

> If you know your data is sane there's no need to re-sanitize it.

That's not our case.

> You should expect the caller of a data struct to provide sane data instead of 
> internally sanitizing it and storing different data from what was provided.

Sorry, you've lost me with "caller of a data struct" - what do you mean by that?

> It's responsibility of the caller (vty code for instance) to make sure parse 
> of human input is correctly parsed and sanitized.

I disagree, and the code I've looked over seems to disagree as well.
For example, in osmo_bsc_vty.c:
* osmo_mcc_from_str() sanitize data internally
* gsm_parse_reg() regexp compiled and result checked outside of vty

In general, I don't see any non-trivial checks done inside vty which I think is 
the right thing. What would be the advantage of having this check in separate 
file instead of static function in the same file? We can also move it to 
libosmocore but I don't see any benefits from keeping it in vty. Do you?

> You can do checks inside the data structure if you want (I wouldn't), but I'd 
> avoid changing content of the data being handled in there.

You mean inside function?

The rest would be addressed in a next revision.


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3dff108148683b107e9edac430a0475283580e9
Gerrit-Change-Number: 12227
Gerrit-PatchSet: 5
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Thu, 13 Dec 2018 10:49:18 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-sgsn[master]: remove pointless declaration of struct gsm_network

2018-12-13 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12242 )

Change subject: remove pointless declaration of struct gsm_network
..


Patch Set 2: Code-Review+1

> Patch Set 2:
> What is wrong with the current commit message?

My bad, probably looked at old version of the patch.


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4151afa5bff01e63b462cca517fb60ac0503759
Gerrit-Change-Number: 12242
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Thu, 13 Dec 2018 10:23:17 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: Use msgb helper instead of local #define for debug print

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 )

Change subject: Use msgb helper instead of local #define for debug print
..


Patch Set 1:

> Patch Set 1: Code-Review+2
>
> I don't agree, I don't like leaving a define dangling in a commit, but 
> anyway, not that important.

So far I haven't found a way to make this cleanup with spatch as well 
unfortunately and I'd rather have 2 separate patches than mix up manual and 
autogenerated changes.


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b
Gerrit-Change-Number: 12271
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Wed, 12 Dec 2018 18:10:54 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-sgsn[master]: remove pointless declaration of struct gsm_network

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12242 )

Change subject: remove pointless declaration of struct gsm_network
..


Patch Set 2:

Good, but now commit message require adjustment as well :)


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4151afa5bff01e63b462cca517fb60ac0503759
Gerrit-Change-Number: 12242
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Wed, 12 Dec 2018 17:46:12 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in docker-playground[master]: MSC: use config file for mncc path

2018-12-12 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12277


Change subject: MSC: use config file for mncc path
..

MSC: use config file for mncc path

Use config file parameter instead of command line option for MNCC socket
path.

Change-Id: I689cfb3bad09b76859ef7e42c1312c636eebe637
---
M ttcn3-msc-test/jenkins.sh
M ttcn3-msc-test/osmo-msc.cfg
2 files changed, 2 insertions(+), 1 deletion(-)



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

diff --git a/ttcn3-msc-test/jenkins.sh b/ttcn3-msc-test/jenkins.sh
index 662c377..82ec483 100755
--- a/ttcn3-msc-test/jenkins.sh
+++ b/ttcn3-msc-test/jenkins.sh
@@ -38,7 +38,7 @@
-v $VOL_BASE_DIR/unix:/data/unix \
--name ${BUILD_TAG}-msc -d \
$REPO_USER/osmo-msc-$IMAGE_SUFFIX \
-   /usr/local/bin/osmo-msc -M /data/unix/mncc
+   /usr/local/bin/osmo-msc

 echo Starting container with MSC testsuite
 docker run --rm \
diff --git a/ttcn3-msc-test/osmo-msc.cfg b/ttcn3-msc-test/osmo-msc.cfg
index f0180d8..4c3555b 100644
--- a/ttcn3-msc-test/osmo-msc.cfg
+++ b/ttcn3-msc-test/osmo-msc.cfg
@@ -73,6 +73,7 @@
  cs7-instance-iu 0
  mgw remote-ip 172.18.1.103
  emergency-call route-to-msisdn 112
+ mncc external /data/unix/mncc
 mncc-int
  default-codec tch-f fr
  default-codec tch-h hr

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

Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I689cfb3bad09b76859ef7e42c1312c636eebe637
Gerrit-Change-Number: 12277
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in libosmocore[master]: Use msgb helper instead of local #define for debug print

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 )

Change subject: Use msgb helper instead of local #define for debug print
..


Patch Set 1:

> Patch Set 1:
> I know you wanted to split them based on spatch

Exactly.

> but this way adds extra verbosity to history and makes it more difficult to 
> revert for no good reason.

I don't think it's actually a problem: reverting 2 patches would be 2 clicks 
instead of 1 (and we rarely have to do this anyway). I think keeping manual 
changes separated from automated code editing is worth it.


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b
Gerrit-Change-Number: 12271
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Wed, 12 Dec 2018 16:20:09 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-bts[master]: clear GPRS indicator in SI3 while PCU is disconnected

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/10170 )

Change subject: clear GPRS indicator in SI3 while PCU is disconnected
..


Patch Set 1:

Shouldn't this be marked as WIP while we're waiting on license clarification 
and related gerrit patch?


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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a6f5c636c0fe098ee31c280d4572a3f8122b44b
Gerrit-Change-Number: 10170
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Harald Welte 
Gerrit-CC: Max 
Gerrit-Comment-Date: Wed, 12 Dec 2018 15:00:53 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-msc[master]: comment: vlr: put the 'balancing' comment closer to the put()

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12240 )

Change subject: comment: vlr: put the 'balancing' comment closer to the put()
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic42768b22d63d182455c8d860961c44159973d0c
Gerrit-Change-Number: 12240
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Wed, 12 Dec 2018 14:47:11 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: vty: show subscriber: put() before printing the use count

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12266 )

Change subject: vty: show subscriber: put() before printing the use count
..


Patch Set 1:

(2 comments)

Please clarify.

https://gerrit.osmocom.org/#/c/12266/1/src/libmsc/msc_vty.c
File src/libmsc/msc_vty.c:

https://gerrit.osmocom.org/#/c/12266/1/src/libmsc/msc_vty.c@821
PS1, Line 821: argv[1]);
If this taints the count than what about all other calls to get_vsub_by_argv()?


https://gerrit.osmocom.org/#/c/12266/1/src/libmsc/msc_vty.c@830
PS1, Line 830:   * and since it existed before we called _get() on it above. */
Are you referring to get_vsub_by_argv()? Better use function name - it's more 
likely to persist than relative positions.



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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id02b57b7ed299b010b9f8b9e809548eb1e6aa699
Gerrit-Change-Number: 12266
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-CC: Max 
Gerrit-Comment-Date: Wed, 12 Dec 2018 14:44:24 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libosmocore[master]: cleanup: remove unused define

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12272 )

Change subject: cleanup: remove unused define
..


Patch Set 1:

This change is ready for review.


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibff603dd581f60b600f2469ad464a0bf77e24bfe
Gerrit-Change-Number: 12272
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Comment-Date: Wed, 12 Dec 2018 14:24:58 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: cleanup: remove unused define

2018-12-12 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12272


Change subject: cleanup: remove unused define
..

cleanup: remove unused define

Change-Id: Ibff603dd581f60b600f2469ad464a0bf77e24bfe
---
M tests/gsm0808/gsm0808_test.c
1 file changed, 0 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/72/12272/1

diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c
index a31e7d4..23961c8 100644
--- a/tests/gsm0808/gsm0808_test.c
+++ b/tests/gsm0808/gsm0808_test.c
@@ -37,19 +37,6 @@
OSMO_ASSERT(rc_enc == msg->len); \
} while(0)

-#define VERIFY(msg, data, len) 
\
-   if (msgb_l3len(msg) != len) {   \
-   printf("%s:%d Length don't match: %d vs. %d. %s\n", \
-   __func__, __LINE__, msgb_l3len(msg), (int) len, \
-   osmo_hexdump(msg->l3h, msgb_l3len(msg)));   \
-   abort();\
-   } else if (memcmp(msg->l3h, data, len) != 0) {  \
-   printf("%s:%d didn't match: got: %s\n", \
-   __func__, __LINE__, \
-   osmo_hexdump(msg->l3h, msgb_l3len(msg)));   \
-   abort();\
-   }
-
 /* Setup a fake codec list for testing */
 static void setup_codec_list(struct gsm0808_speech_codec_list *scl)
 {

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibff603dd581f60b600f2469ad464a0bf77e24bfe
Gerrit-Change-Number: 12272
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in libosmocore[master]: Use msgb helper instead of local #define for debug print

2018-12-12 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12271


Change subject: Use msgb helper instead of local #define for debug print
..

Use msgb helper instead of local #define for debug print

This change was made using following spatch program:
@@
expression a, b, c;
@@
- VERIFY(a, b, c);
+ if (!msgb_eq_l3_data_print(a, b, c))
+  abort();

Which was applied as follows:
spatch --in-place --sp-file verif.spatch tests/gsm0808/gsm0808_test.c

Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b
---
M tests/gsm0808/gsm0808_test.c
1 file changed, 62 insertions(+), 31 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/71/12271/1

diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c
index 63b8720..a31e7d4 100644
--- a/tests/gsm0808/gsm0808_test.c
+++ b/tests/gsm0808/gsm0808_test.c
@@ -116,7 +116,8 @@
msgb_v_put(in_msg, 0x23);

msg = gsm0808_create_layer3_2(in_msg, , NULL);
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();
msgb_free(msg);
msgb_free(in_msg);
 }
@@ -152,7 +153,8 @@

msg = gsm0808_create_layer3_2(in_msg, , _list);

-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();

msgb_free(msg);
msgb_free(in_msg);
@@ -165,7 +167,8 @@

printf("Testing creating Reset\n");
msg = gsm0808_create_reset();
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();
msgb_free(msg);
 }
 
@@ -176,7 +179,8 @@

printf("Testing creating Reset Ack\n");
msg = gsm0808_create_reset_ack();
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();
msgb_free(msg);
 }

@@ -188,7 +192,8 @@

printf("Testing creating Clear Command\n");
msg = gsm0808_create_clear_command(0x23);
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();
msgb_free(msg);
 }

@@ -199,7 +204,8 @@

printf("Testing creating Clear Complete\n");
msg = gsm0808_create_clear_complete();
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();
msgb_free(msg);
 }

@@ -234,12 +240,14 @@
printf("Testing creating Chipher Mode Command\n");
msg = gsm0808_create_cipher(, NULL);
OSMO_ASSERT(msg);
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);

msg = gsm0808_create_cipher(, _imeisv);
OSMO_ASSERT(msg);
-   VERIFY(msg, res2, ARRAY_SIZE(res2));
+   if (!msgb_eq_l3_data_print(msg, res2, ARRAY_SIZE(res2)))
+ abort();
msgb_free(msg);
 }

@@ -259,19 +267,22 @@

/* with l3 data */
msg = gsm0808_create_cipher_complete(l3, 4);
-   VERIFY(msg, res1, ARRAY_SIZE(res1));
+   if (!msgb_eq_l3_data_print(msg, res1, ARRAY_SIZE(res1)))
+   abort();
msgb_free(msg);

/* with l3 data but short */
l3->len -= 1;
l3->tail -= 1;
msg = gsm0808_create_cipher_complete(l3, 4);
-   VERIFY(msg, res2, ARRAY_SIZE(res2));
+   if (!msgb_eq_l3_data_print(msg, res2, ARRAY_SIZE(res2)))
+   abort();
msgb_free(msg);

/* without l3 data */
msg = gsm0808_create_cipher_complete(NULL, 4);
-   VERIFY(msg, res2, ARRAY_SIZE(res2));
+   if (!msgb_eq_l3_data_print(msg, res2, ARRAY_SIZE(res2)))
+   abort();
msgb_free(msg);


@@ -308,7 +319,8 @@

printf("Testing creating Cipher Reject\n");
msg = gsm0808_create_cipher_reject(cause);
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();

parse_cipher_reject(msg, cause);

@@ -323,7 +335,8 @@

printf("Testing creating Cipher Reject (extended)\n");
msg = gsm0808_create_cipher_reject_ext(GSM0808_CAUSE_CLASS_INVAL, 
cause);
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();

parse_cipher_reject(msg, cause);

@@ -342,12 +355,14 @@

printf("Testing creating CM U\n");
msg = gsm0808_create_classmark_update(, 1, , 1);
-   VERIFY(msg, res, ARRAY_SIZE(res));
+   if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+   abort();

msgb_free(msg);

msg = gsm0808_create_classmark_update(, 1, NULL, 0);
- 

Change in libosmocore[master]: LCLS, TS 48.008: add GCR IE encoding/decoding

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12020 )

Change subject: LCLS, TS 48.008: add GCR IE encoding/decoding
..


Patch Set 18:

(1 comment)

https://gerrit.osmocom.org/#/c/12020/4/tests/gsm0808/gsm0808_test.c
File tests/gsm0808/gsm0808_test.c:

https://gerrit.osmocom.org/#/c/12020/4/tests/gsm0808/gsm0808_test.c@586
PS4, Line 586: {
No need to be bitter about it: it's not the first time people disagree with you 
(or me, or Pau...) nor the last.

> you dismiss a sound argument
If you find it sound it doesn't mean others would automatically share your 
opinion.

Here're some more details since you insist:

> memset() is the byte hacking way and I don't like it...
It's a standard function and your liking/disliking it is irrelevant and hardly 
a sound argument.

> just 'var = {0}' doesn't work you need to supply the type name to the 
> compiler (unless it is a new variable declaration init...
Irrelevant in this case because "it's a new variable declaration init".

> the struct variable is part of ran_conn...
Irrelevant again - it's standalone struct which we initialize entirely.

> to write "{0}" means that I indicate the first item's value to be zero...
The only part of your link which is actually applicable to the code you're 
commenting on. Yes, initializing via {} is equivalent to {0} precisely because 
{} initialize entire thing with 0 and { x } initialize first byte with x and 
the rest with 0. And yes, it's "more general" if you will. But nobody have 
argued with those points to begin with. My point was (and still is) that it's 
easier to read the code because the 0 immediately reminds you how the rest is 
initialized. That's also seems to be preferred variant in libosmocore so far 
according to git grep.



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82ce0207dc8de50689a8806c6471ad7fbae6219d
Gerrit-Change-Number: 12020
Gerrit-PatchSet: 18
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Wed, 12 Dec 2018 12:46:30 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in openbsc[master]: CTRL: remove boilerplate

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/1576 )

Change subject: CTRL: remove boilerplate
..


Patch Set 5:

> Easiest to get rid of them is to just "Reply", so bear with me

You mean you've saved yourself 2 mouse clicks to publish instead of discard? In 
future, please try to choose slightly less easy solution to avoid spamming 
people - clicking couple of times is hardly a backbreaking effort.


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

Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5bcea0b4f4b8f535bef2b423f2013b8b4a218b5b
Gerrit-Change-Number: 1576
Gerrit-PatchSet: 5
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Holger Freyther 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-CC: Neels Hofmeyr 
Gerrit-Comment-Date: Wed, 12 Dec 2018 10:06:09 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-sgsn[master]: remove pointless declaration of struct gsm_network

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12242 )

Change subject: remove pointless declaration of struct gsm_network
..


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/12242/1/src/gprs/sgsn_ctrl.c
File src/gprs/sgsn_ctrl.c:

https://gerrit.osmocom.org/#/c/12242/1/src/gprs/sgsn_ctrl.c@64
PS1, Line 64: struct ctrl_handle *sgsn_controlif_setup(void *data, const char 
*bind_addr, uint16_t port)
> I think it's even more confusing this way. If parameter is unused than simply 
> drop the parameter.
In fact, entire function is trivial one-line wrapper used in single place. 
Better drop it completely and just call ctrl_interface_setup_dynip() directly 
from main().



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4151afa5bff01e63b462cca517fb60ac0503759
Gerrit-Change-Number: 12242
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Wed, 12 Dec 2018 09:56:26 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libosmocore[master]: LCLS, TS 48.008: add GCR IE encoding/decoding

2018-12-12 Thread Max
Max has posted comments on this change. ( https://gerrit.osmocom.org/12020 )

Change subject: LCLS, TS 48.008: add GCR IE encoding/decoding
..


Patch Set 18:

(2 comments)

https://gerrit.osmocom.org/#/c/12020/4/tests/gsm0808/gsm0808_test.c
File tests/gsm0808/gsm0808_test.c:

https://gerrit.osmocom.org/#/c/12020/4/tests/gsm0808/gsm0808_test.c@586
PS4, Line 586: {
> It's "better" using {0} for two reasons, but after reading in lots of places 
> about the topic, there' […]
Since the opinions differ I'd rather keep the code as it is because I find it 
easier to read. I've read Neels argument but have not found it convincing.


https://gerrit.osmocom.org/#/c/12020/17/tests/gsm0808/gsm0808_test.c
File tests/gsm0808/gsm0808_test.c:

https://gerrit.osmocom.org/#/c/12020/17/tests/gsm0808/gsm0808_test.c@1864
PS17, Line 1864:void *ctx = talloc_named_const(NULL, 0, "gsm0808 test");
> and possibly trace the logging in expected stderr output

No output is expected on stderr as long as test passes.



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82ce0207dc8de50689a8806c6471ad7fbae6219d
Gerrit-Change-Number: 12020
Gerrit-PatchSet: 18
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Wed, 12 Dec 2018 09:53:43 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libosmocore[master]: LCLS, TS 29.205: add GCR routines

2018-12-12 Thread Max
Hello Stefan Sperling, Pau Espin Pedrol, Neels Hofmeyr, Harald Welte, Jenkins 
Builder,

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

https://gerrit.osmocom.org/11827

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

Change subject: LCLS, TS 29.205: add GCR routines
..

LCLS, TS 29.205: add GCR routines

Add functions to encode and decode Global Call Reference as per
3GPP TS 29.205 Table B 2.1.9.1 add corresponding tests.

Change-Id: Iee95aa4e5c056645b6cb5667e4a067097d52dfbf
Related: OS#2487
---
M include/Makefile.am
M include/osmocom/gsm/gsm0808_utils.h
A include/osmocom/gsm/gsm29205.h
M src/gsm/Makefile.am
A src/gsm/gsm29205.c
M src/gsm/libosmogsm.map
M tests/Makefile.am
A tests/gsm29205/gsm29205_test.c
A tests/gsm29205/gsm29205_test.ok
M tests/testsuite.at
10 files changed, 263 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/27/11827/24
--
To view, visit https://gerrit.osmocom.org/11827
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee95aa4e5c056645b6cb5667e4a067097d52dfbf
Gerrit-Change-Number: 11827
Gerrit-PatchSet: 24
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


Change in libosmocore[master]: LCLS, TS 48.008: add GCR IE encoding/decoding

2018-12-12 Thread Max
Hello Pau Espin Pedrol, Neels Hofmeyr, Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/12020

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

Change subject: LCLS, TS 48.008: add GCR IE encoding/decoding
..

LCLS, TS 48.008: add GCR IE encoding/decoding

* add functions to encode Global Call. Ref. from TS 29.205 as 3GPP TS
  48.008 §3.2.2.115 information element
* add corresponding tests

Change-Id: I82ce0207dc8de50689a8806c6471ad7fbae6219d
---
M include/osmocom/gsm/gsm0808_utils.h
M src/gsm/gsm0808_utils.c
M src/gsm/libosmogsm.map
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
5 files changed, 123 insertions(+), 0 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82ce0207dc8de50689a8806c6471ad7fbae6219d
Gerrit-Change-Number: 12020
Gerrit-PatchSet: 18
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 


Change in libosmocore[master]: msgb: add test helpers

2018-12-12 Thread Max
Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12017 )

Change subject: msgb: add test helpers
..

msgb: add test helpers

It's often handy to compare certain msgb layer to a given array and
print the position where they differ. Add simple pretty-printer and
corresponding L* wrappers.

Change-Id: I3bc95f2f5ab6e3f4b502647fb3e0aaaf1f7c4cf5
---
M include/osmocom/core/msgb.h
M src/msgb.c
2 files changed, 231 insertions(+), 0 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Pau Espin Pedrol: Looks good to me, but someone else must approve
  Stefan Sperling: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/include/osmocom/core/msgb.h b/include/osmocom/core/msgb.h
index 1bb5fe5..b7c8422 100644
--- a/include/osmocom/core/msgb.h
+++ b/include/osmocom/core/msgb.h
@@ -560,6 +560,145 @@
return lbound <= msg->head +  msg->data_len;
 }

+
+/* msgb data comparison helpers */
+
+/*! Compare: check data in msgb against given data
+ *  \param[in] msg message buffer
+ *  \param[in] data expected data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb content is equal to the given data
+ */
+#define msgb_eq_data(msg, data, len)   \
+   _msgb_eq(__FILE__, __LINE__, __func__, 0, msg, data, len, false)
+
+/*! Compare: check L1 data in msgb against given data
+ *  \param[in] msg message buffer
+ *  \param[in] data expected L1 data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb L1 content is equal to the given 
data
+ */
+#define msgb_eq_l1_data(msg, data, len)\
+   _msgb_eq(__FILE__, __LINE__, __func__, 1, msg, data, len, false)
+
+/*! Compare: check L2 data in msgb against given data
+ *  \param[in] msg message buffer
+ *  \param[in] data expected L2 data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb L2 content is equal to the given 
data
+ */
+#define msgb_eq_l2_data(msg, data, len)\
+   _msgb_eq(__FILE__, __LINE__, __func__, 2, msg, data, len, false)
+
+/*! Compare: check L3 data in msgb against given data
+ *  \param[in] msg message buffer
+ *  \param[in] data expected L3 data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb L3 content is equal to the given 
data
+ */
+#define msgb_eq_l3_data(msg, data, len)\
+   _msgb_eq(__FILE__, __LINE__, __func__, 3, msg, data, len, false)
+
+/*! Compare: check L4 data in msgb against given data
+ *  \param[in] msg message buffer
+ *  \param[in] data expected L4 data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb L4 content is equal to the given 
data
+ */
+#define msgb_eq_l4_data(msg, data, len)\
+   _msgb_eq(__FILE__, __LINE__, __func__, 4, msg, data, len, false)
+
+
+/* msgb test/debug helpers */
+
+/*! Compare and print: check data in msgb against given data and print errors 
if any
+ *  \param[in] msg message buffer
+ *  \param[in] data expected data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb content is equal to the given data
+ */
+#define msgb_eq_data_print(msg, data, len) \
+   _msgb_eq(__FILE__, __LINE__, __func__, 0, msg, data, len, true)
+
+/*! Compare and print: check L1 data in msgb against given data and print 
errors if any
+ *  \param[in] msg message buffer
+ *  \param[in] data expected L1 data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb L1 content is equal to the given 
data
+ */
+#define msgb_eq_l1_data_print(msg, data, len)  \
+   _msgb_eq(__FILE__, __LINE__, __func__, 1, msg, data, len, true)
+
+/*! Compare and print: check L2 data in msgb against given data and print 
errors if any
+ *  \param[in] msg message buffer
+ *  \param[in] data expected L2 data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb L2 content is equal to the given 
data
+ */
+#define msgb_eq_l2_data_print(msg, data, len)  \
+   _msgb_eq(__FILE__, __LINE__, __func__, 2, msg, data, len, true)
+
+/*! Compare and print: check L3 data in msgb against given data and print 
errors if any
+ *  \param[in] msg message buffer
+ *  \param[in] data expected L3 data
+ *  \param[in] len length of data
+ *  \returns boolean indicating whether msgb L3 content is equal to the given 
data
+ */
+#define msgb_eq_l3_data_print(msg, data, len)  \
+   _msgb_eq(__FILE__, __LINE__, __func__, 3, msg, data, len, true)
+
+
+/*! Compare and print: check L4 data in msgb against given data and print 
errors if any
+ *  \param[in] msg message buffer
+ *  \param[in] data expected L4 data
+ *  \para

Change in osmo-pcu[master]: cosmetic: move bit counter outside of egprs_window_size()

2018-12-11 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12247


Change subject: cosmetic: move bit counter outside of egprs_window_size()
..

cosmetic: move bit counter outside of egprs_window_size()

As a preparation to moving window size calculation to C code, let's move
bit counter call outside. It makes more sense that way as well because
egprs_window_size() now deals with actual number of allocated slots
instead of raw bitmap.

Change-Id: I5b59919e7b4c9fd2c91958659bafe470ed8fcff7
---
M src/tbf.cpp
M src/tbf_dl.cpp
M src/tbf_ul.cpp
3 files changed, 2 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/47/12247/1

diff --git a/src/tbf.cpp b/src/tbf.cpp
index 6792d08..832aa60 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -500,8 +500,6 @@

 uint16_t egprs_window_size(const struct gprs_rlcmac_bts *bts_data, uint8_t 
slots)
 {
-   uint8_t num_pdch = pcu_bitcount(slots);
-
return OSMO_MIN((num_pdch != 1) ? (128 * num_pdch) : 192,
OSMO_MAX(64, (bts_data->ws_base + num_pdch * 
bts_data->ws_pdch) / 32 * 32));
 }
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 01331a6..a3fef2c 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -1358,7 +1358,7 @@
 void gprs_rlcmac_dl_tbf::set_window_size()
 {
const struct gprs_rlcmac_bts *b = bts->bts_data();
-   uint16_t ws = egprs_window_size(b, dl_slots());
+   uint16_t ws = egprs_window_size(b, pcu_bitcount(dl_slots()));
LOGPTBFDL(this, LOGL_INFO, "setting EGPRS DL window size to %u, 
base(%u) slots(%u) ws_pdch(%u)\n",
  ws, b->ws_base, pcu_bitcount(dl_slots()), b->ws_pdch);
m_window.set_ws(ws);
diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index f877484..9233f2c 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -594,7 +594,7 @@
 void gprs_rlcmac_ul_tbf::set_window_size()
 {
const struct gprs_rlcmac_bts *b = bts->bts_data();
-   uint16_t ws = egprs_window_size(b, ul_slots());
+   uint16_t ws = egprs_window_size(b, pcu_bitcount(ul_slots()));
LOGPTBFUL(this, LOGL_INFO, "setting EGPRS UL window size to %u, 
base(%u) slots(%u) ws_pdch(%u)\n",
  ws, b->ws_base, pcu_bitcount(ul_slots()), b->ws_pdch);
m_window.set_ws(ws);

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

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5b59919e7b4c9fd2c91958659bafe470ed8fcff7
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


Change in osmo-pcu[master]: cosmetic: use const pointer for bts_data

2018-12-11 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12246


Change subject: cosmetic: use const pointer for bts_data
..

cosmetic: use const pointer for bts_data

It's used several time for logging so let's call it once to make code
easier to follow.

Change-Id: Icfd9e5603a5d8701f487f17e9c0335d458e9e80b
---
M src/tbf_dl.cpp
M src/tbf_ul.cpp
2 files changed, 6 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/46/12246/1

diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index dd24963..01331a6 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -1357,9 +1357,10 @@

 void gprs_rlcmac_dl_tbf::set_window_size()
 {
-   uint16_t ws = egprs_window_size(bts->bts_data(), dl_slots());
+   const struct gprs_rlcmac_bts *b = bts->bts_data();
+   uint16_t ws = egprs_window_size(b, dl_slots());
LOGPTBFDL(this, LOGL_INFO, "setting EGPRS DL window size to %u, 
base(%u) slots(%u) ws_pdch(%u)\n",
- ws, bts->bts_data()->ws_base, pcu_bitcount(dl_slots()), 
bts->bts_data()->ws_pdch);
+ ws, b->ws_base, pcu_bitcount(dl_slots()), b->ws_pdch);
m_window.set_ws(ws);
 }

diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index 02f4ddb..f877484 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -593,8 +593,9 @@

 void gprs_rlcmac_ul_tbf::set_window_size()
 {
-   uint16_t ws = egprs_window_size(bts->bts_data(), ul_slots());
+   const struct gprs_rlcmac_bts *b = bts->bts_data();
+   uint16_t ws = egprs_window_size(b, ul_slots());
LOGPTBFUL(this, LOGL_INFO, "setting EGPRS UL window size to %u, 
base(%u) slots(%u) ws_pdch(%u)\n",
- ws, bts->bts_data()->ws_base, pcu_bitcount(ul_slots()), 
bts->bts_data()->ws_pdch);
+ ws, b->ws_base, pcu_bitcount(ul_slots()), b->ws_pdch);
m_window.set_ws(ws);
 }

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

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icfd9e5603a5d8701f487f17e9c0335d458e9e80b
Gerrit-Change-Number: 12246
Gerrit-PatchSet: 1
Gerrit-Owner: Max 


<    5   6   7   8   9   10   11   12   13   14   >