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

[Neels Hofmeyr]

Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/12236


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

vlr_lu_fsm: drop unused out_state INIT -> WAIT_IMEI

There is no state transition from INIT to WAIT_IMEI, only to WAIT_SUB_PRES.

If there were code to skip WAIT_SUB_PRES, the allowed state transitions would
have to be the same as for WAIT_SUB_PRES, i.e. also WAIT_IMEI_IMSI and
WAIT_TMSI_CNF. For now just opt for the status quo.

Change-Id: I18ef9e8c96b52401d98f49dc410f13681231b533
---
M src/libvlr/vlr_lu_fsm.c
1 file changed, 1 insertion(+), 2 deletions(-)



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

diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index a0cbcab..5d171d5 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -555,8 +555,7 @@
[LU_COMPL_VLR_S_INIT] = {
.in_event_mask = S(LU_COMPL_VLR_E_START),
.out_state_mask = S(LU_COMPL_VLR_S_DONE) |
- S(LU_COMPL_VLR_S_WAIT_SUB_PRES) |
- S(LU_COMPL_VLR_S_WAIT_IMEI),
+ S(LU_COMPL_VLR_S_WAIT_SUB_PRES),
.name = OSMO_STRINGIFY(LU_COMPL_VLR_S_INIT),
.action = lu_compl_vlr_init,
},

--
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: newchange
Gerrit-Change-Id: I18ef9e8c96b52401d98f49dc410f13681231b533
Gerrit-Change-Number: 12236
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 

Change in libosmocore[master]: fix api doc for osmo_bcd2str()

[Neels Hofmeyr]

Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12235 )

Change subject: fix api doc for osmo_bcd2str()
..

fix api doc for osmo_bcd2str()

Change-Id: I504ea849fc9daeb34a1b3c5343371161deba743e
---
M src/utils.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Jenkins Builder: Verified
  Neels Hofmeyr: Looks good to me, approved



diff --git a/src/utils.c b/src/utils.c
index 4b4e6d2..35d70ac 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -136,7 +136,7 @@
  *  \param[in] dst_size  sizeof() the output string buffer.
  *  \param[in] bcd  Binary coded data buffer.
  *  \param[in] start_nibble  Offset to start from, in nibbles, typically 1 to 
skip the first nibble.
- *  \param[in] end_nibble  Offset to stop before, in nibbles, e.g. sizeof(bcd) 
- (bcd[0] & GSM_MI_ODD? 0:1).
+ *  \param[in] end_nibble  Offset to stop before, in nibbles, e.g. 
sizeof(bcd)*2 - (bcd[0] & GSM_MI_ODD? 0:1).
  *  \param[in] allow_hex  If false, return error if there are digits other 
than 0-9. If true, return those as [A-F].
  *  \returns The strlen that would be written if the output buffer is large 
enough, excluding nul byte (like
  *   snprintf()), or -EINVAL if allow_hex is false and a digit > 9 is 
encountered. On -EINVAL, the conversion is

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I504ea849fc9daeb34a1b3c5343371161deba743e
Gerrit-Change-Number: 12235
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 

Change in libosmocore[master]: fix api doc for osmo_bcd2str()

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/12235 )

Change subject: fix api doc for osmo_bcd2str()
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12235
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: I504ea849fc9daeb34a1b3c5343371161deba743e
Gerrit-Change-Number: 12235
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Tue, 11 Dec 2018 01:18:01 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-bsc[master]: set gscon FSM instances' log level to DEBUG

[Neels Hofmeyr]

Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12211 )

Change subject: set gscon FSM instances' log level to DEBUG
..

set gscon FSM instances' log level to DEBUG

Currently, we see all subscribers' FSM transitions on NOTICE level even though
the code uses LOGPFSML(LOGL_DEBUG), because LOGPFSML() uses the max loglevel of
the passed level and the FSM instance's level. Too noisy!

By default, start out all gscon FSM instances on DEBUG level, so it is possible
to silence the osmo-bsc log. Individual instances can still be lifted (I
presume using the CTRL interface?).

Change-Id: Ie021483e93ab174abac51357bcca8895756566c4
---
M src/osmo-bsc/bsc_subscr_conn_fsm.c
1 file changed, 1 insertion(+), 1 deletion(-)

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



diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c 
b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index 074c238..fac0bc0 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -877,7 +877,7 @@

/* don't allocate from 'conn' context, as gscon_cleanup() will call 
talloc_free(conn) before
 * libosmocore will call talloc_free(conn->fi), i.e. avoid 
use-after-free during cleanup */
-   conn->fi = osmo_fsm_inst_alloc(_fsm, net, conn, LOGL_NOTICE, 
NULL);
+   conn->fi = osmo_fsm_inst_alloc(_fsm, net, conn, LOGL_DEBUG, NULL);
if (!conn->fi) {
talloc_free(conn);
return NULL;

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie021483e93ab174abac51357bcca8895756566c4
Gerrit-Change-Number: 12211
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 

Change in osmo-msc[master]: move ASS-COMPL MGCP handling out of a_iface_bssap.c

[Neels Hofmeyr]

Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12201 )

Change subject: move ASS-COMPL MGCP handling out of a_iface_bssap.c
..

move ASS-COMPL MGCP handling out of a_iface_bssap.c

BSSMAP Assignment Complete: sort MGCP handling upon Assignment Complete to the
proper locations. a_iface_bssap.c is not the right place to invoke the MGCP
related procedures.

- in a_iface_bssap.c only decode the IEs.
- call ran_conn_assign_compl() and pass decoded values.
- drop msc_assign_compl(), it was dead code; instead:
- add ran_conn_assign_compl()
- pass on all MGCP related info to msc_mgcp_ass_complete()
- move all MGCP ctx related handling from a_iface_bssap.c to msc_mgcp.c.

I'm dropping some comments to save some time, because if I adjust them IMHO
they would still anyway restate the obvious.

ran_conn_assign_compl() is now quite a thin shim, but it makes sense to have
it:

- This is the place that should tear down the ran_conn in case assignment
  failed, left for a future patch.

- In the light of upcoming inter-MSC handover, ran_conn_assign_compl() will be
  the place where the Assignment Complete message might be relayed to a remote
  MSC.

Change-Id: I8137215c443239bddf3e69b5715839a365b73b6c
---
M include/osmocom/msc/msc_mgcp.h
M include/osmocom/msc/ran_conn.h
M src/libmsc/a_iface_bssap.c
M src/libmsc/msc_mgcp.c
M src/libmsc/osmo_msc.c
5 files changed, 88 insertions(+), 88 deletions(-)

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



diff --git a/include/osmocom/msc/msc_mgcp.h b/include/osmocom/msc/msc_mgcp.h
index 9d8b01d..db8dff2 100644
--- a/include/osmocom/msc/msc_mgcp.h
+++ b/include/osmocom/msc/msc_mgcp.h
@@ -24,6 +24,8 @@
 #include 

 struct ran_conn;
+struct gsm0808_speech_codec;
+struct sockaddr_storage;

 /* MGCP state handler context. This context information stores all information
  * to handle the direction of the RTP streams via MGCP. There is one instance
@@ -58,6 +60,7 @@
 };

 int msc_mgcp_call_assignment(struct gsm_trans *trans);
-int msc_mgcp_ass_complete(struct ran_conn *conn, uint16_t port, char *addr);
+int msc_mgcp_ass_complete(struct ran_conn *conn, const struct 
gsm0808_speech_codec *speech_codec_chosen,
+ const struct sockaddr_storage *aoip_transport_addr);
 int msc_mgcp_call_complete(struct gsm_trans *trans, uint16_t port, char *addr);
 int msc_mgcp_call_release(struct gsm_trans *trans);
diff --git a/include/osmocom/msc/ran_conn.h b/include/osmocom/msc/ran_conn.h
index d71872e..3716f86 100644
--- a/include/osmocom/msc/ran_conn.h
+++ b/include/osmocom/msc/ran_conn.h
@@ -7,6 +7,9 @@
 #include 
 #include 

+struct gsm0808_speech_codec;
+struct sockaddr_storage;
+
 enum ran_type {
RAN_UNKNOWN,
RAN_GERAN_A,/* 2G / A-interface */
@@ -197,6 +200,8 @@
 void ran_conn_classmark_chg(struct ran_conn *conn,
const uint8_t *cm2, uint8_t cm2_len,
const uint8_t *cm3, uint8_t cm3_len);
+void ran_conn_assign_compl(struct ran_conn *conn, const struct 
gsm0808_speech_codec *speech_codec_chosen,
+  const struct sockaddr_storage *aoip_transport_addr);
 void ran_conn_assign_fail(struct ran_conn *conn, uint8_t cause, uint8_t 
*rr_cause);

 void ran_conn_init(void);
diff --git a/src/libmsc/a_iface_bssap.c b/src/libmsc/a_iface_bssap.c
index d84a234..4e8b146 100644
--- a/src/libmsc/a_iface_bssap.c
+++ b/src/libmsc/a_iface_bssap.c
@@ -34,7 +34,6 @@
 #include 
 #include 
 #include 
-#include 

 #include 

@@ -498,51 +497,12 @@
return 0;
 }

-/* Use the speech codec info we go with the assignment complete to dtermine
- * which codec we will signal to the MGW */
-static enum mgcp_codecs mgcp_codec_from_sc(struct gsm0808_speech_codec *sc)
-{
-   switch (sc->type) {
-   case GSM0808_SCT_FR1:
-   return CODEC_GSM_8000_1;
-   break;
-   case GSM0808_SCT_FR2:
-   return CODEC_GSMEFR_8000_1;
-   break;
-   case GSM0808_SCT_FR3:
-   return CODEC_AMR_8000_1;
-   break;
-   case GSM0808_SCT_FR4:
-   return CODEC_AMRWB_16000_1;
-   break;
-   case GSM0808_SCT_FR5:
-   return CODEC_AMRWB_16000_1;
-   break;
-   case GSM0808_SCT_HR1:
-   return CODEC_GSMHR_8000_1;
-   break;
-   case GSM0808_SCT_HR3:
-   return CODEC_AMR_8000_1;
-   break;
-   case GSM0808_SCT_HR4:
-   return CODEC_AMRWB_16000_1;
-   break;
-   case GSM0808_SCT_HR6:
-   return CODEC_AMRWB_16000_1;
-   break;
-   default:
-   return CODEC_PCMU_8000_1;
-   break;
-   }
-}
-
 

Change in osmo-msc[master]: drop gsm48 RR ciph mode compl from permitted initial messages

[Neels Hofmeyr]

Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12207 )

Change subject: drop gsm48 RR ciph mode compl from permitted initial messages
..

drop gsm48 RR ciph mode compl from permitted initial messages

It is a message that is initially permitted, but it is in fact not handled in
the L3 code but already before, upon receiving
BSS_MAP_MSG_CIPHER_MODE_COMPLETE.

Change-Id: I0079f07271ca76bd457d0e700f3a736eb9066b47
---
M src/libmsc/gsm_04_08.c
1 file changed, 2 insertions(+), 1 deletion(-)

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



diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 2962760..95c3183 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -1296,7 +1296,8 @@
break;
case GSM48_PDISC_RR:
switch (msg_type) {
-   case GSM48_MT_RR_CIPH_M_COMPL:
+   /* GSM48_MT_RR_CIPH_M_COMPL is actually handled in 
bssmap_rx_ciph_compl() and gets redirected in the
+* BSSAP layer to ran_conn_cipher_mode_compl() (before this 
here is reached) */
case GSM48_MT_RR_PAG_RESP:
return true;
default:

--
To view, visit https://gerrit.osmocom.org/12207
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: I0079f07271ca76bd457d0e700f3a736eb9066b47
Gerrit-Change-Number: 12207
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Stefan Sperling 

Change in libosmocore[master]: fix api doc for osmo_bcd2str()

[Neels Hofmeyr]

Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/12235


Change subject: fix api doc for osmo_bcd2str()
..

fix api doc for osmo_bcd2str()

Change-Id: I504ea849fc9daeb34a1b3c5343371161deba743e
---
M src/utils.c
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/35/12235/1

diff --git a/src/utils.c b/src/utils.c
index 4b4e6d2..35d70ac 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -136,7 +136,7 @@
  *  \param[in] dst_size  sizeof() the output string buffer.
  *  \param[in] bcd  Binary coded data buffer.
  *  \param[in] start_nibble  Offset to start from, in nibbles, typically 1 to 
skip the first nibble.
- *  \param[in] end_nibble  Offset to stop before, in nibbles, e.g. sizeof(bcd) 
- (bcd[0] & GSM_MI_ODD? 0:1).
+ *  \param[in] end_nibble  Offset to stop before, in nibbles, e.g. 
sizeof(bcd)*2 - (bcd[0] & GSM_MI_ODD? 0:1).
  *  \param[in] allow_hex  If false, return error if there are digits other 
than 0-9. If true, return those as [A-F].
  *  \returns The strlen that would be written if the output buffer is large 
enough, excluding nul byte (like
  *   snprintf()), or -EINVAL if allow_hex is false and a digit > 9 is 
encountered. On -EINVAL, the conversion is

--
To view, visit https://gerrit.osmocom.org/12235
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: I504ea849fc9daeb34a1b3c5343371161deba743e
Gerrit-Change-Number: 12235
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 

Change in osmo-msc[master]: LU: do not always invoke sub_pres_vlr_fsm_start()

[Neels Hofmeyr]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/12233

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

Change subject: LU: do not always invoke sub_pres_vlr_fsm_start()
..

LU: do not always invoke sub_pres_vlr_fsm_start()

sub_pres_vlr_fsm_start() starts the FSM, invokes the START event, and then this
FSM invariably always directly terminates when vsub->ms_not_reachable_flag ==
false.

So if it is false, there is not much use in instantiating a whole FSM instance
that just terminates again, we might as well directly issue the
parent-term-event and save some logging space.

The same condition is already in place in the vlr_proc_acc_fsm.c in
_proc_arq_vlr_node2_post_vlr() for CM Service Request and Paging Response. Now
also skip this for LU.

Change-Id: Id2303a795dfd381f76e94ff8ff2f495926ca8ba0
---
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_ms_timeout.err
M tests/msc_vlr/msc_vlr_test_no_authen.err
M tests/msc_vlr/msc_vlr_test_reject_concurrency.err
M tests/msc_vlr/msc_vlr_test_rest.err
M tests/msc_vlr/msc_vlr_test_ss.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
12 files changed, 6 insertions(+), 497 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/33/12233/2
--
To view, visit https://gerrit.osmocom.org/12233
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: Id2303a795dfd381f76e94ff8ff2f495926ca8ba0
Gerrit-Change-Number: 12233
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)

Change in osmo-msc[master]: LU: do not always invoke sub_pres_vlr_fsm_start()

[Neels Hofmeyr]

Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/12233


Change subject: LU: do not always invoke sub_pres_vlr_fsm_start()
..

LU: do not always invoke sub_pres_vlr_fsm_start()

sub_pres_vlr_fsm_start() starts the FSM, invokes the START event, and then this
FSM invariably always directly terminates when vsub->ms_not_reachable_flag ==
false.

So if it is false, there is not much use in instantiating a whole FSM instance
that just terminates again, we might as well directly issue the
parent-term-event and save some logging space.

The same condition is already in place in the vlr_proc_acc_fsm.c in
_proc_arq_vlr_node2_post_vlr() for CM Service Request and Paging Response. Now
also skip this for LU.

Change-Id: Id2303a795dfd381f76e94ff8ff2f495926ca8ba0
---
M src/libvlr/vlr_lu_fsm.c
1 file changed, 6 insertions(+), 1 deletion(-)



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

diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index 02e49e0..74a6939 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -413,7 +413,12 @@
osmo_fsm_inst_state_chg(fi, LU_COMPL_VLR_S_WAIT_SUB_PRES,
LU_TIMEOUT_LONG, 0);

-   sub_pres_vlr_fsm_start(>sub_pres_vlr_fsm, fi, vsub, 
LU_COMPL_VLR_E_SUB_PRES_COMPL);
+   /* If ms_not_reachable_flag == false, the sub_pres_vlr_fsm will anyway 
terminate straight away and dispatch
+* LU_COMPL_VLR_E_SUB_PRES_COMPL to this fi, so we might as well skip 
that dance here and save some logging. */
+   if (vsub->ms_not_reachable_flag)
+   sub_pres_vlr_fsm_start(>sub_pres_vlr_fsm, fi, vsub, 
LU_COMPL_VLR_E_SUB_PRES_COMPL);
+   else
+   osmo_fsm_inst_dispatch(fi, LU_COMPL_VLR_E_SUB_PRES_COMPL, NULL);
 }

 static void lu_compl_vlr_new_tmsi(struct osmo_fsm_inst *fi)

--
To view, visit https://gerrit.osmocom.org/12233
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: Id2303a795dfd381f76e94ff8ff2f495926ca8ba0
Gerrit-Change-Number: 12233
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 

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

[Neels Hofmeyr]

Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/12234


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

tweak comment to indicate sub_pres_vlr fsm as dead code

sub_pres_vlr_fsm_start() only ever has an effect if ms_not_reachable_flag ==
true. But there simply is no code that sets this flag. So
sub_pres_vlr_fsm_start() is currently dead code.

Also, examining the FSM, if it should ever be set to true, this would halt the
LU/CM Service/Paging response, since the FSM would merely change its state
without dispatching asynchronous messages. No chance of finishing.

Short of dropping the code entirely, first just mark it. The point being that
this models some FSM definition from 3GPP specs, and we have a couple other
"if (0)" branches in the VLR...

Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99
---
M src/libvlr/vlr_lu_fsm.c
1 file changed, 3 insertions(+), 1 deletion(-)



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

diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index 74a6939..a0cbcab 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -271,7 +271,9 @@
return (struct vlr_subscr*)fi->priv;
 }

-/* Note that the start event is dispatched right away, so in case the FSM 
immediately concludes from that
+/* THIS IS CURRENTLY DEAD CODE, SINCE WE NEVER SET vsub->ms_not_reachable_flag 
= true.
+ *
+ * Note that the start event is dispatched right away, so in case the FSM 
immediately concludes from that
  * event, the created FSM struct may no longer be valid as it already 
deallocated again, and it may
  * furthermore already have invoked the parent FSM instance's deallocation as 
well. Hence, instead of
  * returning, store the created FSM instance address in *fi_p before 
dispatching the event. It is thus

--
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: newchange
Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99
Gerrit-Change-Number: 12234
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/12218 )

Change subject: What about -1 vote counting?
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 10 Dec 2018 23:22:01 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/12218 )

Change subject: What about -1 vote counting?
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 10 Dec 2018 23:21:03 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/12218 )

Change subject: What about -1 vote counting?
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 10 Dec 2018 23:20:34 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/12218 )

Change subject: What about -1 vote counting?
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 10 Dec 2018 23:19:31 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


Removed Verified+1 by Pau Espin Pedrol 
--
To view, visit https://gerrit.osmocom.org/12218
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/12218 )

Change subject: What about -1 vote counting?
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 10 Dec 2018 23:20:11 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/12218 )

Change subject: What about -1 vote counting?
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 10 Dec 2018 23:18:47 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


Removed Code-Review-1 by osmith 
--
To view, visit https://gerrit.osmocom.org/12218
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in sandbox[master]: What about -1 vote counting?

[Neels Hofmeyr]

Neels Hofmeyr has removed a vote on this change.

Change subject: What about -1 vote counting?
..


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

Gerrit-Project: sandbox
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ieab13e41e5a8c438653cfb5d058da3e588bc5603
Gerrit-Change-Number: 12218
Gerrit-PatchSet: 1
Gerrit-Owner: osmith 
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 

Change in libosmocore[master]: GSUP/SMS: introduce MO-/MT-FORWARD-SM messages

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/11069 )

Change subject: GSUP/SMS: introduce MO-/MT-FORWARD-SM messages
..


Patch Set 10: Code-Review-1

(25 comments)

many comments aren't critical, except:

- I think the OA/DA IE structure needs to be flattened.
- Use punctuation and other doxygen misc.

couldn't resist to remark on some peculiarities that you are just taking over 
from other code around there, please ignore those...

https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup.h
File include/osmocom/gsm/gsup.h:

https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup.h@94
PS10, Line 94:  OSMO_GSUP_SM_RP_MR_IE   = 0x40,
oh ok, we really have "_IE" in the end then? well... ok then.


https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup.h@134
PS10, Line 134: OSMO_GSUP_MSGT_MO_FORWARD_SM_REQUEST= 0b00100100,
also interesting that we define the message type in binary... ok then


https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup.h@235
PS10, Line 235:  * Please note that there is no SM-RP-MR in TCAP/MAP! 
SM-RP-MR
in doxygen this becomes

  SM-RP-MR (see 3GPP TS 29.002, 7.6.1.1), Message Reference Please note that...

Please use punctuation!!


https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup.h@237
PS10, Line 237: const uint8_t   *sm_rp_mr;
oh wow, so we don't inline uint8_t[] then? hmm. ok. ok then...

and we use tabs... well then


https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup_sms.h
File include/osmocom/gsm/gsup_sms.h:

https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup_sms.h@17
PS10, Line 17:  OSMO_GSUP_SMS_SM_RP_ODA_NONE= 0x00,
isn't "SMS_SM" redundant? could it be just OSMO_GSUP_SMS_ or just OSMO_GSUP_SM_?


https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup_sms.h@25
PS10, Line 25: /* Forward declarations (to avoid mutual include) */
yes, that's what they are, and I think all C programmers should be aware of 
that concept?


https://gerrit.osmocom.org/#/c/11069/10/include/osmocom/gsm/gsup_sms.h@29
PS10, Line 29: /* SM-RP-DA IE coding functions */
a) I see that in the function name; unless you also explain "SM", "RP" or "DA", 
you might as well drop the entire comment. (I would opt for dropping cruft)

b) place function comments in the .c file plz


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup.c
File src/gsm/gsup.c:

https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup.c@569
PS10, Line 569: int idx, rc;
(rather place each var on its own line)


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup.c@668
PS10, Line 668: sizeof(u8), gsup_msg->sm_rp_mr);
(either line up with '(' or use a single tab as indent)


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c
File src/gsm/gsup_sms.c:

https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c@33
PS10, Line 33:  *  SMS (Short Message Service) extensions for Osmocom GSUP
please please use punctuation to end the summary line.


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c@35
PS10, Line 35:
(I think we usually write in one line

  /*! Foo yada

but then again I think Linus Torvalds referred to that as brain damaged once, 
so whatever.)


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c@37
PS10, Line 37:  * Encode SM-RP-DA IE (see 7.6.8.1), Destination Address
please please use punctuation to end the summary line.


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c@43
PS10, Line 43:  const struct osmo_gsup_message *gsup_msg)
(I guess this fits on a 120 wide line?)


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c@53
PS10, Line 53:  /*! Special case for noSM-RP-DA and noSM-RP-OA */
i doubt code inline doxygen makes sense. it might even end up in above function 
doc block?


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c@62
PS10, Line 62:  "(type=0x%02x)!\n", gsup_msg->sm_rp_da_type);
(also looks like comfortable fit for 120 width, at least for the char constant)


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c@74
PS10, Line 74:  ie_len = msg->tail + 1; /* To be calculated later */
max recently added API for this, see msgb_tl_put()


https://gerrit.osmocom.org/#/c/11069/10/src/gsm/gsup_sms.c@82
PS10, Line 82:  *ie_len = msg->tail - (ie_len + 1);
so ... this looks like this now?

  [DA_IE, len, [DA_TYPE, len, DA data...] ]

Can this instead remain a flat layer of TLV structures? There are two nested 
layers, I think we should avoid that. Otherwise we are opening up corner cases, 
and both len fields need to be checked: the first one implicitly by our TLV 
decoder (nice), and the second one manually after having retrieved the first V 
(sigh)

Define the IE as

   [ DA_IE, len, TYPE, data ]

and you can derive data_len = overall_len - 

Change in osmo-gsm-manuals[master]: chapters/gsup.adoc: document MO-/MT-forwardSM messages

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/11836 )

Change subject: chapters/gsup.adoc: document MO-/MT-forwardSM messages
..


Patch Set 4: Code-Review+2

(5 comments)

I still see some of my points not addressed, but it's better to have this now 
than nothing. If you will, you could address below review and ping me to +2 
again; or merge and come back with anoter patch later; or merge and disagree 
with me and I will eternally dislike it but probably forget about it soon :P

https://gerrit.osmocom.org/#/c/11836/4/common/chapters/gsup.adoc
File common/chapters/gsup.adoc:

https://gerrit.osmocom.org/#/c/11836/4/common/chapters/gsup.adoc@509
PS4, Line 509: Direction: MSC / SGSN => SMSC (through HLR)
(a nicer term is "via", not "through" ... but nm)


https://gerrit.osmocom.org/#/c/11836/4/common/chapters/gsup.adoc@541
PS4, Line 541: _MO (Mobile Originated)_ short message delivery. The 
corresponding
I still think it is really irritating to explain the same abbreviations over 
and over. If you have to, place the braces once above and then don't do that 
anymore for the rest of the doc. But actually, instead make sure MO and MT are 
in the glossary and then rely on that completely. We did agree on that before, 
right? I think we also agreed before on less fancy fontsy stuff, i.e. try to 
avoid italics and bold script. Especially don't place an entire "FOO (yada)" in 
italics?


https://gerrit.osmocom.org/#/c/11836/4/common/chapters/gsup.adoc@543
PS4, Line 543: section 12.2.
it's much nicer with the reference in the end, thanks :)


https://gerrit.osmocom.org/#/c/11836/4/common/chapters/gsup.adoc@578
PS4, Line 578: This message is used to forward _MT (Mobile Terminated)_ short 
messages
MT from glossary


https://gerrit.osmocom.org/#/c/11836/4/common/chapters/gsup.adoc@579
PS4, Line 579: from an SMSC to MSC (CS domain) / SGSN (PS domain). The 
corresponding MAP
and I still don't like this. I am still of the same opinion: the protocol 
definition is not the place to explain MSC = CS domain and SGSN = PS domain, 
especially not N times over and over.



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

Gerrit-Project: osmo-gsm-manuals
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0150756c33c1352bc4eb49421824542c711175c
Gerrit-Change-Number: 11836
Gerrit-PatchSet: 4
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Mon, 10 Dec 2018 20:46:25 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes

Change in osmo-gsm-manuals[master]: chapters/gsup.adoc: document READY-FOR-SM message

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/11837 )

Change subject: chapters/gsup.adoc: document READY-FOR-SM message
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: osmo-gsm-manuals
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I549b6c8840a1e86caac09e77fb8bc5042d939e62
Gerrit-Change-Number: 11837
Gerrit-PatchSet: 4
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Mon, 10 Dec 2018 20:09:50 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in libosmocore[master]: GSUP/SMS: introduce READY-FOR-SM message

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/11751 )

Change subject: GSUP/SMS: introduce READY-FOR-SM message
..


Patch Set 5: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/11751
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: Ic37f3b2114b8095cfce22977e67133b9103942e3
Gerrit-Change-Number: 11751
Gerrit-PatchSet: 5
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Mon, 10 Dec 2018 20:09:03 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

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

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12227 )

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


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12227/3/src/gprs/sgsn_auth.c
File src/gprs/sgsn_auth.c:

https://gerrit.osmocom.org/#/c/12227/3/src/gprs/sgsn_auth.c@58
PS3, Line 58:   return true;
The docstring says the imsi is truncated if we return true from this function.
"truncated" implies that dst contains all imsi data up to MAX_DIGITS, and any 
remaining imsi data beyond MAX_DIGITS will be missing from dst.
But in fact, when we return true here, the destination buffer is still empty - 
it does not contain any imsi data!
So this is a contradiciton.

I cannot actully tell what this code intends to do.
It looks to me like either the docstring or the code should be fixed.



--
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: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:35:50 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Add tests for IMSI ACLs

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12226 )

Change subject: Add tests for IMSI ACLs
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12226/2/tests/sgsn/sgsn_test.c
File tests/sgsn/sgsn_test.c:

https://gerrit.osmocom.org/#/c/12226/2/tests/sgsn/sgsn_test.c@1343
PS2, Line 1343: }
> Doing these stuff while having another variable after calling the function 
> separate by a comma? Let  […]
May I suggest a variable name change: old_count and new_count?



--
To view, visit https://gerrit.osmocom.org/12226
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: Ia782f12b49bed6428bc9b9f513237e4e6aefdec9
Gerrit-Change-Number: 12226
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:21:34 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Use explicit parameter for sgsn_auth_init()

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12221 )

Change subject: Use explicit parameter for sgsn_auth_init()
..


Patch Set 2: Code-Review+1

Can you provide some patches to get rid of the global variable then? or at 
least keep it only for VTY if needed.


--
To view, visit https://gerrit.osmocom.org/12221
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: Ibeba371234680f33ad35afbfffce9dca185228c1
Gerrit-Change-Number: 12221
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:18:24 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-sgsn[master]: Use explicit parameter for sgsn_auth_init()

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12221 )

Change subject: Use explicit parameter for sgsn_auth_init()
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12221/1/tests/sgsn/sgsn_test.c
File tests/sgsn/sgsn_test.c:

https://gerrit.osmocom.org/#/c/12221/1/tests/sgsn/sgsn_test.c@51
PS1, Line 51: struct sgsn_instance *sgsn = _inst;
> Ah here it is. […]
Probably because other tests, such as  test_gmm_cancel(), still depend on the 
global variable.



--
To view, visit https://gerrit.osmocom.org/12221
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: Ibeba371234680f33ad35afbfffce9dca185228c1
Gerrit-Change-Number: 12221
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:14:53 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-bsc[master]: Add VTY option to avoid sending empty Full BCCH Info for disabled SI

[Pau Espin Pedrol]

Hello Stefan Sperling, Vadim Yanitskiy, Jenkins Builder,

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

https://gerrit.osmocom.org/12133

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

Change subject: Add VTY option to avoid sending empty Full BCCH Info for 
disabled SI
..

Add VTY option to avoid sending empty Full BCCH Info for disabled SI

According to 3GPP TS 08.58 §8.5.1 BCCH INFORMATION:
"If the Full BCCH information element is not included this indicates that
transmission of the indicated SYSTEM INFORMATION message shall be stopped."

However, some ipaccess nanoBTS firmware versions are known to not support
some SI elements and also to dislike receiving BCCH Information for those SI,
even if received with empty BCCH Information meaning they should not be used.

Upon receival of this kind of message, nanoBTS sends a Failure Report
with following text:
Type=processing failure, Severity=critical failure, Probable cause=Manufacturer 
specific values: Fatal software error, Additional Text=l2_bch.c:1149

** l2_bch.c#1149:BCHbcchSItypeValid( prim_p->infoType )
** IPA_SW_FATAL_ERROR
** In task "TRX Proc:L2_BCH" @ (325).


This kind of issue only appears with some fw versions, since it's known
to work fine in other ones, so let's not disable this kind of mesage by
default on all BTs of type "nanobts".

Instead, add a VTY command that allows disabling this kind of message in
order to be able to operate those nanoBTS units.

Fixes: OS#3707
Change-Id: Idec1daabc21de4eea5c55edd1dbb0e0775720fc7
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/bsc_init.c
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/gsm_data.c
4 files changed, 50 insertions(+), 6 deletions(-)


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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idec1daabc21de4eea5c55edd1dbb0e0775720fc7
Gerrit-Change-Number: 12133
Gerrit-PatchSet: 3
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 

Change in osmo-bsc[master]: set gscon FSM instances' log level to DEBUG

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12211 )

Change subject: set gscon FSM instances' log level to DEBUG
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie021483e93ab174abac51357bcca8895756566c4
Gerrit-Change-Number: 12211
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:06:47 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-msc[master]: move ASS-COMPL MGCP handling out of a_iface_bssap.c

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12201 )

Change subject: move ASS-COMPL MGCP handling out of a_iface_bssap.c
..


Patch Set 3: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/#/c/12201/3/include/osmocom/msc/msc_mgcp.h
File include/osmocom/msc/msc_mgcp.h:

https://gerrit.osmocom.org/#/c/12201/3/include/osmocom/msc/msc_mgcp.h@27
PS3, Line 27: struct gsm0808_speech_codec;
> Why not adding correct headers instead?
Yes, seems like osmocom/msc/ran_conn.h and osmocom/gsm/protocol/gsm_08_08.h 
would do?



--
To view, visit https://gerrit.osmocom.org/12201
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: I8137215c443239bddf3e69b5715839a365b73b6c
Gerrit-Change-Number: 12201
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:03:59 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes

Change in docker-playground[master]: cosmetic: gerrit: use variables for the files to patch

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12205 )

Change subject: cosmetic: gerrit: use variables for the files to patch
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12205
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: I285f7908de64f2d92fa99ae9d74fb2e29ec72771
Gerrit-Change-Number: 12205
Gerrit-PatchSet: 2
Gerrit-Owner: osmith 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 19:01:11 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in libosmo-netif[master]: logging: fix typo in stream.c

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12217 )

Change subject: logging: fix typo in stream.c
..

logging: fix typo in stream.c

Change-Id: I5dcae1f19e18f04709ce7585943af1d582ebc7ed
---
M src/stream.c
1 file changed, 1 insertion(+), 1 deletion(-)

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



diff --git a/src/stream.c b/src/stream.c
index 4548414..902e688 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -91,7 +91,7 @@
, sizeof(event));

if (rc < 0)
-   LOGP(DLINP, LOGL_ERROR, "coudldn't activate SCTP events "
+   LOGP(DLINP, LOGL_ERROR, "couldn't activate SCTP events "
 "on FD %u\n", fd);
return rc;
 #else

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

Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5dcae1f19e18f04709ce7585943af1d582ebc7ed
Gerrit-Change-Number: 12217
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 

Change in libosmocore[master]: stats.h: Fix build on MacOS

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12212 )

Change subject: stats.h: Fix build on MacOS
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12212
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: I037c3ca141ecee2d457e0a881a56e32ee24cec4d
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:59:34 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in libosmo-netif[master]: logging: fix typo in stream.c

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12217 )

Change subject: logging: fix typo in stream.c
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5dcae1f19e18f04709ce7585943af1d582ebc7ed
Gerrit-Change-Number: 12217
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:59:50 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-msc[master]: detach cancelled subscribers from VLR

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12232 )

Change subject: detach cancelled subscribers from VLR
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12232
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: I5918106e4a94ba2e6c61bcd7b90d3bf0565513cc
Gerrit-Change-Number: 12232
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:59:21 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-msc[master]: move ASS-COMPL MGCP handling out of a_iface_bssap.c

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12201 )

Change subject: move ASS-COMPL MGCP handling out of a_iface_bssap.c
..


Patch Set 3: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12201
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: I8137215c443239bddf3e69b5715839a365b73b6c
Gerrit-Change-Number: 12201
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:58:13 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-msc[master]: drop gsm48 RR ciph mode compl from permitted initial messages

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12207 )

Change subject: drop gsm48 RR ciph mode compl from permitted initial messages
..


Patch Set 3: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12207
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: I0079f07271ca76bd457d0e700f3a736eb9066b47
Gerrit-Change-Number: 12207
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:56:27 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-ttcn3-hacks[master]: bsc: TC_chan_exhaustion: Send correct RA to alloc all different channels

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12215 )

Change subject: bsc: TC_chan_exhaustion: Send correct RA to alloc all different 
channels
..

bsc: TC_chan_exhaustion: Send correct RA to alloc all different channels

Previous RA value (23, Establishment cause = 0010) meant MS was dual
rate capable but was asking speciifically for only TCH/F channel. As a
result, TCH/H was not being allocated and an immediate assignment reject
was sent.

Change-Id: I3e58592c661fc004e648dbe46b67a3b3f5a20bc8
---
M bsc/BSC_Tests.ttcn
1 file changed, 4 insertions(+), 2 deletions(-)

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



diff --git a/bsc/BSC_Tests.ttcn b/bsc/BSC_Tests.ttcn
index aaa4580..77da306 100644
--- a/bsc/BSC_Tests.ttcn
+++ b/bsc/BSC_Tests.ttcn
@@ -489,9 +489,11 @@
chreq_total := f_ctrl_get_ratectr_abs(IPA_CTRL, "bts", 0, 
"chreq:total");
chreq_nochan := f_ctrl_get_ratectr_abs(IPA_CTRL, "bts", 0, 
"chreq:no_channel");

-   /* expect 5xTCH/F to succeed */
+   /* GSM 04.08 Table 9.9a:
+* RA = '33'O -> Establishment cause = 0011 (MS dual rate capable 
and asks for "TCH/H or TCH/F").
+* With current setup, expect 4xSDCCH + 4xTCH/F + 1xTCH/H to succeed */
for (i := 0; i < NUM_TCHF_PER_BTS + NUM_TCHH_PER_BTS + 
NUM_SDCCH_PER_BTS; i := i+1) {
-   var RslChannelNr chan_nr := f_chreq_act_ack('23'O, i);
+   var RslChannelNr chan_nr := f_chreq_act_ack('33'O, i);
}

IPA_RSL[0].clear;

--
To view, visit https://gerrit.osmocom.org/12215
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: I3e58592c661fc004e648dbe46b67a3b3f5a20bc8
Gerrit-Change-Number: 12215
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 

Change in osmo-bsc[master]: Add VTY option to avoid sending empty Full BCCH Info for disabled SI

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12133 )

Change subject: Add VTY option to avoid sending empty Full BCCH Info for 
disabled SI
..


Patch Set 2: Code-Review+1

(4 comments)

https://gerrit.osmocom.org/#/c/12133/2/src/osmo-bsc/bsc_init.c
File src/osmo-bsc/bsc_init.c:

https://gerrit.osmocom.org/#/c/12133/2/src/osmo-bsc/bsc_init.c@184
PS2, Line 184:  rc = 0; /* some nanoBTs fw don't like 
receiving empty unsupported SI */
BTs -> BTS ?


https://gerrit.osmocom.org/#/c/12133/2/src/osmo-bsc/bsc_vty.c
File src/osmo-bsc/bsc_vty.c:

https://gerrit.osmocom.org/#/c/12133/2/src/osmo-bsc/bsc_vty.c@3096
PS2, Line 3096: "Some nanoBTS fw versions are known to fail upron 
receival of these messages.\n")
typo: upron -> upon


https://gerrit.osmocom.org/#/c/12133/2/src/osmo-bsc/bsc_vty.c@3109
PS2, Line 3109: "Some nanoBTS fw versions are known to fail upron 
receival of these messages.\n")
Same typo


https://gerrit.osmocom.org/#/c/12133/1/src/osmo-bsc/gsm_data.c
File src/osmo-bsc/gsm_data.c:

https://gerrit.osmocom.org/#/c/12133/1/src/osmo-bsc/gsm_data.c@858
PS1, Line 858: bts->si_unused_send_empty = true;
> Hm perhaps si_unused_send_empty?
Yeah, si_unused_send_empy seems fine.



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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idec1daabc21de4eea5c55edd1dbb0e0775720fc7
Gerrit-Change-Number: 12133
Gerrit-PatchSet: 2
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:53:41 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes

Change in osmo-ttcn3-hacks[master]: bsc: TC_chan_exhaustion: Send correct RA to alloc all different channels

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12215 )

Change subject: bsc: TC_chan_exhaustion: Send correct RA to alloc all different 
channels
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12215
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: I3e58592c661fc004e648dbe46b67a3b3f5a20bc8
Gerrit-Change-Number: 12215
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:54:00 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-ttcn3-hacks[master]: bsc: TC_chan_exhaustion: Send correct RA to alloc all different channels

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12215 )

Change subject: bsc: TC_chan_exhaustion: Send correct RA to alloc all different 
channels
..


Patch Set 1: Code-Review+1

(1 comment)

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

https://gerrit.osmocom.org/#/c/12215/1//COMMIT_MSG@10
PS1, Line 10: rate capable but was asking speciifically for only TCH/F channel. 
As a
Typo: speciifically -> specifically



--
To view, visit https://gerrit.osmocom.org/12215
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: I3e58592c661fc004e648dbe46b67a3b3f5a20bc8
Gerrit-Change-Number: 12215
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:48:18 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes

Change in osmo-msc[master]: drop gsm48 RR ciph mode compl from permitted initial messages

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12207 )

Change subject: drop gsm48 RR ciph mode compl from permitted initial messages
..


Patch Set 3: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/12207
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: I0079f07271ca76bd457d0e700f3a736eb9066b47
Gerrit-Change-Number: 12207
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:39:30 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in libosmo-netif[master]: logging: fix typo in stream.c

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12217 )

Change subject: logging: fix typo in stream.c
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5dcae1f19e18f04709ce7585943af1d582ebc7ed
Gerrit-Change-Number: 12217
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:36:30 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

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

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/11827 )

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


Patch Set 23: Code-Review-1

(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:
Same problem as pointed out by Pau in 
https://gerrit.osmocom.org/c/libosmocore/+/12020
The decoder failed, so we should treat the contents of p as undefined and just 
skip the code below.
The osmo_dec_gcr() API does not allow for any other reasonable course of action 
because the caller can't tell how the decoder failed exactly.



--
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: 23
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: Mon, 10 Dec 2018 18:34:59 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes

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

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12020 )

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


Patch Set 17:

(1 comment)

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

https://gerrit.osmocom.org/#/c/12020/13/tests/gsm0808/gsm0808_test.c@615
PS13, Line 615: struct msgb *msg;
> If decoding failed (gsm0808_dec_gcr returned <0), it makes no sense using p 
> and printing related p s […]
I agree with Pau. This code can simply skip all further p checks if decoding 
failed in the first place.



--
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: 17
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: Mon, 10 Dec 2018 18:29:39 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in libosmocore[master]: stats.h: Fix build on MacOS

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12212 )

Change subject: stats.h: Fix build on MacOS
..


Patch Set 1: Code-Review+1

Oooohh! a non-Linux platform! :)


--
To view, visit https://gerrit.osmocom.org/12212
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: I037c3ca141ecee2d457e0a881a56e32ee24cec4d
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:25:01 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Pau Espin Pedrol]

Pau Espin Pedrol has removed a vote on this change.

Change subject: disable recording of LU timestamps by default
..


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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 4: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:14:38 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 4:

Please don't merge this before Harald has had a chance to respond. In the issue 
he suggested that he does not agree with this approach but I wrote this patch 
anyway, hoping to convince him.


--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:12:27 +
Gerrit-HasComments: No
Gerrit-HasLabels: No

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 4: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:11:48 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
> I'll restore the previous version then. I liked it better.
I actually prefer this version, it's still more split between different parts.



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:11:13 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 4:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  * \param[in] by_imsi  ASCII string of IMSI digits.
> Ah good point about the goto :)
I'll restore the previous version then. I liked it better.



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:08:57 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-msc[master]: detach cancelled subscribers from VLR

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12232 )

Change subject: detach cancelled subscribers from VLR
..


Patch Set 1: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/12232
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: I5918106e4a94ba2e6c61bcd7b90d3bf0565513cc
Gerrit-Change-Number: 12232
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:08:50 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Stefan Sperling]

Hello Pau Espin Pedrol, Jenkins Builder,

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

https://gerrit.osmocom.org/12228

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

Change subject: disable recording of LU timestamps by default
..

disable recording of LU timestamps by default

Add VTY commands which enable or disable recording of
Location Update timestamps in the HLR database.

Because this feature has implications for the privacy of
network users, it is now off by default. It needs to be
explicitly enabled by the administrator who will see a
warning about potential privacy concerns when doing so.

The new commands added to the hlr configuration space are:

  record-lu-timestamps
  no record-lu-timestamps

DB tests keep recording timestamps in the test database to
ensure that the corresponding code is being exercised.

Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Related: OS#2838
Depends: Ie180c434f02ffec0d4b2f651a73258a8126b2e1a
---
M src/db.h
M src/db_hlr.c
M src/hlr.c
M src/hlr.h
M src/hlr_vty.c
M tests/db/db_test.c
M tests/db/db_test.err
M tests/test_nodes.vty
8 files changed, 65 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/28/12228/4
--
To view, visit https://gerrit.osmocom.org/12228
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 3: Code-Review+2

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
> Maybe the previous version was better after all? Refactoring this block of 
> code actually adds a lot  […]
Ah good point about the goto :)



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:06:52 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes

Change in osmo-sgsn[master]: Drop unused osmo-sgsn.pc

[Max]

Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12231 )

Change subject: Drop unused osmo-sgsn.pc
..

Drop unused osmo-sgsn.pc

We do not install any libraries so we don't need it: most likely it's a
forgotten leftover from pre-split repo time.

Change-Id: Ifabb26d1e6384659789061bc2abe23cb5ceca4cb
---
M Makefile.am
M configure.ac
M debian/copyright
D osmo-sgsn.pc.in
4 files changed, 0 insertions(+), 15 deletions(-)

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



diff --git a/Makefile.am b/Makefile.am
index 7ff989d..3f89896 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -16,9 +16,6 @@
tests \
$(NULL)

-pkgconfigdir = $(libdir)/pkgconfig
-pkgconfig_DATA = osmo-sgsn.pc
-
 BUILT_SOURCES = $(top_srcdir)/.version
 EXTRA_DIST = git-version-gen osmoappdesc.py .version

diff --git a/configure.ac b/configure.ac
index f0bef52..c0d3770 100644
--- a/configure.ac
+++ b/configure.ac
@@ -242,7 +242,6 @@
 AM_CONFIG_HEADER(bscconfig.h)

 AC_OUTPUT(
-osmo-sgsn.pc
 include/Makefile
 include/osmocom/Makefile
 include/osmocom/sgsn/Makefile
diff --git a/debian/copyright b/debian/copyright
index cc631f4..4e53664 100644
--- a/debian/copyright
+++ b/debian/copyright
@@ -45,7 +45,6 @@
include/osmocom/sgsn/sgsn.h
include/osmocom/sgsn/vty.h
m4/README
-   osmo-sgsn.pc.in
src/Makefile.am
src/gprs/.gitignore
src/gprs/Makefile.am
diff --git a/osmo-sgsn.pc.in b/osmo-sgsn.pc.in
deleted file mode 100644
index 45aef57..000
--- a/osmo-sgsn.pc.in
+++ /dev/null
@@ -1,10 +0,0 @@
-prefix=@prefix@
-exec_prefix=@exec_prefix@
-libdir=@libdir@
-includedir=@includedir@/
-
-Name: OsmoSGSN
-Description: Osmocom's Serving GPRS Support Node for 2G and 3G packet-switched 
mobile networks
-Requires:
-Version: @VERSION@
-Cflags: -I${includedir}

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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifabb26d1e6384659789061bc2abe23cb5ceca4cb
Gerrit-Change-Number: 12231
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 

Change in osmo-sgsn[master]: Use explicit length check

[Max]

Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12229 )

Change subject: Use explicit length check
..

Use explicit length check

Use OSMO_MIN macro to check for MSISDN length. This makes the code
cleaner and will, hopefully, aid static analysis tools.

Change-Id: Ic0fbeb8d248c74e54bfb51ba2cdea55c4f386ac7
Fixes: CID57879
---
M src/gprs/sgsn_libgtp.c
1 file changed, 1 insertion(+), 3 deletions(-)

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



diff --git a/src/gprs/sgsn_libgtp.c b/src/gprs/sgsn_libgtp.c
index 478d402..a8a1502 100644
--- a/src/gprs/sgsn_libgtp.c
+++ b/src/gprs/sgsn_libgtp.c
@@ -176,9 +176,7 @@

/* Put the MSISDN in case we have it */
if (mmctx->subscr && mmctx->subscr->sgsn_data->msisdn_len) {
-   pdp->msisdn.l = mmctx->subscr->sgsn_data->msisdn_len;
-   if (pdp->msisdn.l > sizeof(pdp->msisdn.v))
-   pdp->msisdn.l = sizeof(pdp->msisdn.v);
+   pdp->msisdn.l = OSMO_MIN(mmctx->subscr->sgsn_data->msisdn_len, 
sizeof(pdp->msisdn.v));
memcpy(pdp->msisdn.v, mmctx->subscr->sgsn_data->msisdn,
pdp->msisdn.l);
} else {

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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic0fbeb8d248c74e54bfb51ba2cdea55c4f386ac7
Gerrit-Change-Number: 12229
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 

Change in osmo-sgsn[master]: Drop unused osmo-sgsn.pc

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12231 )

Change subject: Drop unused osmo-sgsn.pc
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12231
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: Ifabb26d1e6384659789061bc2abe23cb5ceca4cb
Gerrit-Change-Number: 12231
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:04:23 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

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

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12227 )

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


Patch Set 2:

(1 comment)

So I'm fine with moving it into a function, but I think the function should be 
called by the caller of sgsn_acl_* only if needed, not always inside sgsn_acl_*

https://gerrit.osmocom.org/#/c/12227/2/src/gprs/sgsn_vty.c
File src/gprs/sgsn_vty.c:

https://gerrit.osmocom.org/#/c/12227/2/src/gprs/sgsn_vty.c@a650
PS2, Line 650: 
> In which library it's defined?
sorry, I meant the imsi_sanitize() you add in this commit.



--
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: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:03:24 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Add tests for IMSI ACLs

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12226 )

Change subject: Add tests for IMSI ACLs
..


Patch Set 2:

(2 comments)

https://gerrit.osmocom.org/#/c/12226/3/include/osmocom/sgsn/gprs_sgsn.h
File include/osmocom/sgsn/gprs_sgsn.h:

https://gerrit.osmocom.org/#/c/12226/3/include/osmocom/sgsn/gprs_sgsn.h@466
PS3, Line 466: size_t sgsn_acl_count(const struct sgsn_config *cfg);
unrelated whitespace.


https://gerrit.osmocom.org/#/c/12226/2/tests/sgsn/sgsn_test.c
File tests/sgsn/sgsn_test.c:

https://gerrit.osmocom.org/#/c/12226/2/tests/sgsn/sgsn_test.c@1343
PS2, Line 1343: size_t old = sgsn_acl_count(cfg), new;
> That's pretty common code patern both in osmocom and other projects - I don't 
> see anything ugly abou […]
Doing these stuff while having another variable after calling the function 
separate by a comma? Let me doubt so (and if there's any place like that it 
should be cleaned up).



--
To view, visit https://gerrit.osmocom.org/12226
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: Ia782f12b49bed6428bc9b9f513237e4e6aefdec9
Gerrit-Change-Number: 12226
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:01:18 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Use libosmocore constant for IMSI length in ACL entry

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12224 )

Change subject: Use libosmocore constant for IMSI length in ACL entry
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h
File include/osmocom/sgsn/gprs_sgsn.h:

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h@417
PS2, Line 417:  char imsi[GSM23003_IMSI_MAX_DIGITS + 2];
> > And in there we use +1, which makes more sense […]
So commit openbsc d3fa84dbba3b67cdbe2d8c789b2833b5ddf42068 changed 
GSM_IMSI_LENGTH to be GSM23003_IMSI_MAX_DIGITS+1 in all places except one to 
GSM23003_IMSI_MAX_DIGITS+2.

That looks like a typo in that commit to me. @Harald, can you confirm (you are 
the author of the commit)?



--
To view, visit https://gerrit.osmocom.org/12224
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: I138aea409aab0c748c75546e628797fc7498bf40
Gerrit-Change-Number: 12224
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:57:47 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-msc[master]: detach cancelled subscribers from VLR

[Stefan Sperling]

Stefan Sperling has uploaded this change for review. ( 
https://gerrit.osmocom.org/12232


Change subject: detach cancelled subscribers from VLR
..

detach cancelled subscribers from VLR

When a subscriber is cancelled, fake an IMSI detach to
ensure that the subscriber gets removed from the VLR.

I am not entirely sure if this change is correct but
it does make TTCN3 test MSC_Tests.TC_gsup_cancel pass.

Change-Id: I5918106e4a94ba2e6c61bcd7b90d3bf0565513cc
Related: OS#2886
---
M src/libvlr/vlr.c
1 file changed, 2 insertions(+), 0 deletions(-)



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

diff --git a/src/libvlr/vlr.c b/src/libvlr/vlr.c
index 76c84de..3369914 100644
--- a/src/libvlr/vlr.c
+++ b/src/libvlr/vlr.c
@@ -970,6 +970,8 @@
gmm_cause_to_fsm_and_mm_cause(gsup_msg->cause, _cause, _rej);
vlr_subscr_cancel_attach_fsm(vsub, fsm_cause, gsm48_rej);

+   vlr_subscr_rx_imsi_detach(vsub);
+
return rc;
 }
 

--
To view, visit https://gerrit.osmocom.org/12232
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: I5918106e4a94ba2e6c61bcd7b90d3bf0565513cc
Gerrit-Change-Number: 12232
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling 

Change in osmo-sgsn[master]: Constify sgsn_acl_lookup() parameter

[Max]

Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/1 )

Change subject: Constify sgsn_acl_lookup() parameter
..

Constify sgsn_acl_lookup() parameter

This requires I414e67a3de733fab407161b3264d3b89070ba537 in libosmocore
to avoid warning about discarded const.

Change-Id: Ie92637dd900b0f9eba891d5aad0b4ba0ee69c08c
---
M include/osmocom/sgsn/gprs_sgsn.h
M src/gprs/sgsn_auth.c
2 files changed, 2 insertions(+), 2 deletions(-)

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



diff --git a/include/osmocom/sgsn/gprs_sgsn.h b/include/osmocom/sgsn/gprs_sgsn.h
index cf78766..0b721a3 100644
--- a/include/osmocom/sgsn/gprs_sgsn.h
+++ b/include/osmocom/sgsn/gprs_sgsn.h
@@ -460,7 +460,7 @@
 extern const struct value_string *sgsn_auth_state_names;

 void sgsn_auth_init(void);
-struct imsi_acl_entry *sgsn_acl_lookup(const char *imsi, struct sgsn_config 
*cfg);
+struct imsi_acl_entry *sgsn_acl_lookup(const char *imsi, const struct 
sgsn_config *cfg);
 int sgsn_acl_add(const char *imsi, struct sgsn_config *cfg);
 int sgsn_acl_del(const char *imsi, struct sgsn_config *cfg);
 /* Request authorization */
diff --git a/src/gprs/sgsn_auth.c b/src/gprs/sgsn_auth.c
index e3d9127..50f2126 100644
--- a/src/gprs/sgsn_auth.c
+++ b/src/gprs/sgsn_auth.c
@@ -43,7 +43,7 @@
INIT_LLIST_HEAD(>cfg.imsi_acl);
 }

-struct imsi_acl_entry *sgsn_acl_lookup(const char *imsi, struct sgsn_config 
*cfg)
+struct imsi_acl_entry *sgsn_acl_lookup(const char *imsi, const struct 
sgsn_config *cfg)
 {
struct imsi_acl_entry *acl;
llist_for_each_entry(acl, >imsi_acl, list) {

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

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

Change in osmo-sgsn[master]: Use libosmocore constant for IMSI length in ACL entry

[Max]

Max has posted comments on this change. ( https://gerrit.osmocom.org/12224 )

Change subject: Use libosmocore constant for IMSI length in ACL entry
..


Patch Set 3:

(1 comment)

I do agree that extra byte is probably unnecessary but lets wait for Harald's 
comment since he did OpenBSC change.

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h
File include/osmocom/sgsn/gprs_sgsn.h:

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h@417
PS2, Line 417:  char imsi[GSM23003_IMSI_MAX_DIGITS + 2];
> And in there we use +1, which makes more sense

Not only - there's also +2 in gsup part.



--
To view, visit https://gerrit.osmocom.org/12224
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: I138aea409aab0c748c75546e628797fc7498bf40
Gerrit-Change-Number: 12224
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:47:11 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
> Yes it looks weird, but it is correct. […]
Maybe the previous version was better after all? Refactoring this block of code 
actually adds a lot of line churn no matter what we do.



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:46:40 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Use libosmocore constant for IMSI length in ACL entry

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12224 )

Change subject: Use libosmocore constant for IMSI length in ACL entry
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h
File include/osmocom/sgsn/gprs_sgsn.h:

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h@417
PS2, Line 417:  char imsi[GSM23003_IMSI_MAX_DIGITS + 2];
> Comes from openbsc d3fa84dbba3b67cdbe2d8c789b2833b5ddf42068
And in there we use +1, which makes more sense:

openbsc/src/osmo-bsc_nat/bsc_ussd.c
405:if (strlen(con->filter_state.imsi) > GSM23003_IMSI_MAX_DIGITS)

openbsc/src/libmsc/ctrl_commands.c
67: else if (strlen(imsi) > GSM23003_IMSI_MAX_DIGITS)

openbsc/include/openbsc/bsc_subscriber.h
16: char imsi[GSM23003_IMSI_MAX_DIGITS+1];

openbsc/include/openbsc/gsm_subscriber.h
47: char imsi[GSM23003_IMSI_MAX_DIGITS+1];

openbsc/include/openbsc/ipaccess.h
15: charimsi[GSM23003_IMSI_MAX_DIGITS+1];



--
To view, visit https://gerrit.osmocom.org/12224
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: I138aea409aab0c748c75546e628797fc7498bf40
Gerrit-Change-Number: 12224
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:43:33 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Use libosmocore constant for IMSI length in ACL entry

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12224 )

Change subject: Use libosmocore constant for IMSI length in ACL entry
..


Patch Set 3: Code-Review-1


--
To view, visit https://gerrit.osmocom.org/12224
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: I138aea409aab0c748c75546e628797fc7498bf40
Gerrit-Change-Number: 12224
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:43:42 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
> This looks weird. […]
Yes it looks weird, but it is correct.

Is it OK to run other statements before the most recently used statement has 
been reset? If so we could simplify this code.
Otherwise, moving some code into a function has the downside that the 'goto 
out:' doesn't work anymore for the code which was moved to the function.



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:42:22 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Use libosmocore constant for IMSI length in ACL entry

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12224 )

Change subject: Use libosmocore constant for IMSI length in ACL entry
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h
File include/osmocom/sgsn/gprs_sgsn.h:

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h@417
PS2, Line 417:  char imsi[GSM23003_IMSI_MAX_DIGITS + 2];
> I'm not sure why this length is used and there're no comments about it before 
> the patch. […]
Comes from openbsc d3fa84dbba3b67cdbe2d8c789b2833b5ddf42068



--
To view, visit https://gerrit.osmocom.org/12224
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: I138aea409aab0c748c75546e628797fc7498bf40
Gerrit-Change-Number: 12224
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:41:18 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Jenkins build is back to normal : master-osmo-iuh » a1=default,a2=default,a3=default,a4=default,osmocom-master-debian9 #2638

[jenkins]

See 

Change in osmo-sgsn[master]: Constify sgsn_acl_lookup() parameter

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/1 )

Change subject: Constify sgsn_acl_lookup() parameter
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/1
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: Ie92637dd900b0f9eba891d5aad0b4ba0ee69c08c
Gerrit-Change-Number: 1
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:34:41 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 3: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
This looks weird. Why not always first do db_remove_reset(stmt), then remove it 
second time inside the if  after calling db_subscr_lu_record_timestamp()? 
Doesn't look more sane to you?



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:34:12 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes

Change in osmo-sgsn[master]: Drop unused osmo-sgsn.pc

[Max]

Max has uploaded this change for review. ( https://gerrit.osmocom.org/12231


Change subject: Drop unused osmo-sgsn.pc
..

Drop unused osmo-sgsn.pc

We do not install any libraries so we don't need it: most likely it's a
forgotten leftover from pre-split repo time.

Change-Id: Ifabb26d1e6384659789061bc2abe23cb5ceca4cb
---
M Makefile.am
M configure.ac
M debian/copyright
D osmo-sgsn.pc.in
4 files changed, 0 insertions(+), 15 deletions(-)



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

diff --git a/Makefile.am b/Makefile.am
index 7ff989d..3f89896 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -16,9 +16,6 @@
tests \
$(NULL)

-pkgconfigdir = $(libdir)/pkgconfig
-pkgconfig_DATA = osmo-sgsn.pc
-
 BUILT_SOURCES = $(top_srcdir)/.version
 EXTRA_DIST = git-version-gen osmoappdesc.py .version

diff --git a/configure.ac b/configure.ac
index f0bef52..c0d3770 100644
--- a/configure.ac
+++ b/configure.ac
@@ -242,7 +242,6 @@
 AM_CONFIG_HEADER(bscconfig.h)

 AC_OUTPUT(
-osmo-sgsn.pc
 include/Makefile
 include/osmocom/Makefile
 include/osmocom/sgsn/Makefile
diff --git a/debian/copyright b/debian/copyright
index cc631f4..4e53664 100644
--- a/debian/copyright
+++ b/debian/copyright
@@ -45,7 +45,6 @@
include/osmocom/sgsn/sgsn.h
include/osmocom/sgsn/vty.h
m4/README
-   osmo-sgsn.pc.in
src/Makefile.am
src/gprs/.gitignore
src/gprs/Makefile.am
diff --git a/osmo-sgsn.pc.in b/osmo-sgsn.pc.in
deleted file mode 100644
index 45aef57..000
--- a/osmo-sgsn.pc.in
+++ /dev/null
@@ -1,10 +0,0 @@
-prefix=@prefix@
-exec_prefix=@exec_prefix@
-libdir=@libdir@
-includedir=@includedir@/
-
-Name: OsmoSGSN
-Description: Osmocom's Serving GPRS Support Node for 2G and 3G packet-switched 
mobile networks
-Requires:
-Version: @VERSION@
-Cflags: -I${includedir}

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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifabb26d1e6384659789061bc2abe23cb5ceca4cb
Gerrit-Change-Number: 12231
Gerrit-PatchSet: 1
Gerrit-Owner: Max 

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Pau Espin Pedrol]

Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c
File src/hlr_vty.c:

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c@133
PS2, Line 133:  vty_out(vty, "Timestamps of Location Updates will be stored in 
the HLR database.%s", VTY_NEWLINE);
> Can you elaborate what you're referring to when you say we're storing 
> subscriber names? […]
Hm I cannot find it anymore, so maybe it got remove, but I'm pretty sure at 
some point it was possible to attach some "human information" to subscriber 
through VTY in order to identify them.



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:30:18 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

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

[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 3:

(2 comments)

https://gerrit.osmocom.org/#/c/12227/2/src/gprs/sgsn_auth.c
File src/gprs/sgsn_auth.c:

https://gerrit.osmocom.org/#/c/12227/2/src/gprs/sgsn_auth.c@89
PS2, Line 89:   return -ENOMEM;
> based on where the information come from
The sgsn_acl_add() and _del() are only called by vty code so where it comes 
from is always the same.

> Is there any good reason to do it here?
Yes - see commit description.


https://gerrit.osmocom.org/#/c/12227/2/src/gprs/sgsn_vty.c
File src/gprs/sgsn_vty.c:

https://gerrit.osmocom.org/#/c/12227/2/src/gprs/sgsn_vty.c@a650
PS2, Line 650:
> See my comment, I think osmo_imsi_sanitize() should be used here.
In which library it's defined?



--
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: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:27:45 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Add tests for IMSI ACLs

[Max]

Max has posted comments on this change. ( https://gerrit.osmocom.org/12226 )

Change subject: Add tests for IMSI ACLs
..


Patch Set 3:

(1 comment)

The rest is fixed in next revision.

https://gerrit.osmocom.org/#/c/12226/2/tests/sgsn/sgsn_test.c
File tests/sgsn/sgsn_test.c:

https://gerrit.osmocom.org/#/c/12226/2/tests/sgsn/sgsn_test.c@1343
PS2, Line 1343: }
> This line is super ugly, mixing declaration, initialization and function 
> call.
That's pretty common code patern both in osmocom and other projects - I don't 
see anything ugly about it.



--
To view, visit https://gerrit.osmocom.org/12226
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: Ia782f12b49bed6428bc9b9f513237e4e6aefdec9
Gerrit-Change-Number: 12226
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:23:19 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Use libosmocore constant for IMSI length in ACL entry

[Max]

Max has posted comments on this change. ( https://gerrit.osmocom.org/12224 )

Change subject: Use libosmocore constant for IMSI length in ACL entry
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h
File include/osmocom/sgsn/gprs_sgsn.h:

https://gerrit.osmocom.org/#/c/12224/2/include/osmocom/sgsn/gprs_sgsn.h@417
PS2, Line 417:  char imsi[GSM23003_IMSI_MAX_DIGITS + 2];
> why +2? Explain in commit description or/and here in this line.
I'm not sure why this length is used and there're no comments about it before 
the patch. I've added my guess to commit log.



--
To view, visit https://gerrit.osmocom.org/12224
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: I138aea409aab0c748c75546e628797fc7498bf40
Gerrit-Change-Number: 12224
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:21:38 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-sgsn[master]: Add sgsn_acl_count()

[Max]

Max has abandoned this change. ( https://gerrit.osmocom.org/12223 )

Change subject: Add sgsn_acl_count()
..


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ia7c5b36d07672ea43bfa2b531b9b6c56ba65161d
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 

Change in osmo-sgsn[master]: ACL: move allocation check forward in sgsn_acl_add()

[Max]

Max has abandoned this change. ( https://gerrit.osmocom.org/12225 )

Change subject: ACL: move allocation check forward in sgsn_acl_add()
..


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I5906304fd3c96002ca1fdc649609e5a4d9ca299b
Gerrit-Change-Number: 12225
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 

Change in osmo-sgsn[master]: Use libosmocore constant for IMSI length in ACL entry

[Max]

Hello Pau Espin Pedrol, Jenkins Builder,

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

https://gerrit.osmocom.org/12224

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

Change subject: Use libosmocore constant for IMSI length in ACL entry
..

Use libosmocore constant for IMSI length in ACL entry

Presumably the length is chosen to match that of imsi in
osmo_gsup_message.

Change-Id: I138aea409aab0c748c75546e628797fc7498bf40
---
M include/osmocom/sgsn/gprs_sgsn.h
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I138aea409aab0c748c75546e628797fc7498bf40
Gerrit-Change-Number: 12224
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 

Change in osmo-sgsn[master]: Add tests for IMSI ACLs

[Max]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/12226

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

Change subject: Add tests for IMSI ACLs
..

Add tests for IMSI ACLs

Change-Id: Ia782f12b49bed6428bc9b9f513237e4e6aefdec9
---
M include/osmocom/sgsn/gprs_sgsn.h
M tests/sgsn/sgsn_test.c
M tests/sgsn/sgsn_test.ok
3 files changed, 121 insertions(+), 0 deletions(-)


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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia782f12b49bed6428bc9b9f513237e4e6aefdec9
Gerrit-Change-Number: 12226
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-CC: Pau Espin Pedrol 

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

[Max]

Hello Pau Espin Pedrol, Jenkins Builder,

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

https://gerrit.osmocom.org/12227

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

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

ACL: integrate sanitize check into sgsn_acl_* functions

Having this check in vty makes it hard to unit-test. Let's move this
into separate static function and use it directly from sgsn_acl_*
functions. Adjust test output accordingly.

Related: SYS#4300
Change-Id: Ic3dff108148683b107e9edac430a0475283580e9
---
M src/gprs/sgsn_auth.c
M src/gprs/sgsn_vty.c
M tests/sgsn/sgsn_test.ok
3 files changed, 33 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/27/12227/3
--
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: newpatchset
Gerrit-Change-Id: Ic3dff108148683b107e9edac430a0475283580e9
Gerrit-Change-Number: 12227
Gerrit-PatchSet: 3
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 

Change in osmo-sgsn[master]: Constify sgsn_acl_lookup() parameter

[Max]

Hello Pau Espin Pedrol, Jenkins Builder,

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

https://gerrit.osmocom.org/1

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

Change subject: Constify sgsn_acl_lookup() parameter
..

Constify sgsn_acl_lookup() parameter

This requires I414e67a3de733fab407161b3264d3b89070ba537 in libosmocore
to avoid warning about discarded const.

Change-Id: Ie92637dd900b0f9eba891d5aad0b4ba0ee69c08c
---
M include/osmocom/sgsn/gprs_sgsn.h
M src/gprs/sgsn_auth.c
2 files changed, 2 insertions(+), 2 deletions(-)


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

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

Change in osmo-sgsn[master]: Use explicit parameter for sgsn_auth_init()

[Max]

Hello Pau Espin Pedrol, Jenkins Builder,

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

https://gerrit.osmocom.org/12221

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

Change subject: Use explicit parameter for sgsn_auth_init()
..

Use explicit parameter for sgsn_auth_init()

This is necessary to properly test ACLs in follow-up patches.

Change-Id: Ibeba371234680f33ad35afbfffce9dca185228c1
---
M include/osmocom/sgsn/gprs_sgsn.h
M src/gprs/sgsn_auth.c
M src/gprs/sgsn_main.c
M tests/sgsn/sgsn_test.c
4 files changed, 4 insertions(+), 4 deletions(-)


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

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

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Stefan Sperling]

Hello Jenkins Builder,

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

https://gerrit.osmocom.org/12228

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

Change subject: disable recording of LU timestamps by default
..

disable recording of LU timestamps by default

Add VTY commands which enable or disable recording of
Location Update timestamps in the HLR database.

Because this feature has implications for the privacy of
network users, it is now off by default. It needs to be
explicitly enabled by the administrator who will see a
warning about potential privacy concerns when doing so.

The new commands added to the hlr configuration space are:

  record-lu-timestamps
  no record-lu-timestamps

DB tests keep recording timestamps in the test database to
ensure that the corresponding code is being exercised.

Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Related: OS#2838
Depends: Ie180c434f02ffec0d4b2f651a73258a8126b2e1a
---
M src/db.h
M src/db_hlr.c
M src/hlr.c
M src/hlr.h
M src/hlr_vty.c
M tests/db/db_test.c
M tests/db/db_test.err
M tests/test_nodes.vty
8 files changed, 117 insertions(+), 71 deletions(-)


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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Pau Espin Pedrol 

Change in libosmocore[master]: gsm48_mi_to_string: use osmo_bcd2str(), fix some corner cases

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12154 )

Change subject: gsm48_mi_to_string: use osmo_bcd2str(), fix some corner cases
..

gsm48_mi_to_string: use osmo_bcd2str(), fix some corner cases

By using osmo_bcd2str(), ensure that the resulting string is always nul
terminated, and always return strlen()+1 whether truncated or not.

Still keep up the previous return value style, even if that isn't consistent at
all.

The difference between IMSI/IMEI and TMSI return values remains and is not part
of this patch.

Change-Id: I1b51b72a721e1cc9d69796b804ebda741ff0f36b
---
M src/gsm/gsm48.c
M tests/gsm0408/gsm0408_test.c
M tests/gsm0408/gsm0408_test.ok
3 files changed, 24 insertions(+), 29 deletions(-)

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



diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c
index e684a3c..4558dfb 100644
--- a/src/gsm/gsm48.c
+++ b/src/gsm/gsm48.c
@@ -638,7 +638,8 @@
return gsm48_generate_mid(buf, imsi, GSM_MI_TYPE_IMSI);
 }

-/*! Convert TS 04.08 Mobile Identity (10.5.1.4) to string
+/*! Convert TS 04.08 Mobile Identity (10.5.1.4) to string.
+ * This function does not validate the Mobile Identity digits, i.e. digits > 9 
are returned as 'A'-'F'.
  *  \param[out] string Caller-provided buffer for output
  *  \param[in] str_len Length of \a string in bytes
  *  \param[in] mi Mobile Identity to be stringified
@@ -650,7 +651,7 @@
 int gsm48_mi_to_string(char *string, const int str_len, const uint8_t *mi,
   const int mi_len)
 {
-   int i;
+   int rc;
uint8_t mi_type;
char *str_cur = string;
uint32_t tmsi;
@@ -670,17 +671,15 @@
case GSM_MI_TYPE_IMSI:
case GSM_MI_TYPE_IMEI:
case GSM_MI_TYPE_IMEISV:
-   *str_cur++ = osmo_bcd2char(mi[0] >> 4);
-
-for (i = 1; i < mi_len; i++) {
-   if (str_cur + 2 >= string + str_len)
-   return str_cur - string;
-   *str_cur++ = osmo_bcd2char(mi[i] & 0xf);
-   /* skip last nibble in last input byte when GSM_EVEN */
-   if( (i != mi_len-1) || (mi[0] & GSM_MI_ODD))
-   *str_cur++ = osmo_bcd2char(mi[i] >> 4);
-   }
-   break;
+   rc = osmo_bcd2str(string, str_len, mi,
+ 1, mi_len * 2 - ((mi[0] & GSM_MI_ODD) ? 0 : 
1), true);
+   /* osmo_bcd2str() returns snprintf style strlen(), this returns 
bytes written. */
+   if (rc < 0)
+   return 0;
+   else if (rc < str_len)
+   return rc + 1;
+   else
+   return strlen(string) + 1;
default:
break;
}
diff --git a/tests/gsm0408/gsm0408_test.c b/tests/gsm0408/gsm0408_test.c
index 24f903a..c786d38 100644
--- a/tests/gsm0408/gsm0408_test.c
+++ b/tests/gsm0408/gsm0408_test.c
@@ -393,7 +393,6 @@
.expect_mi_tlv_hex = "170449322453",
.str_size = 4,
.expect_str = "423",
-   .expect_rc = 3, /* exception: on truncation, 
gsm48_mi_to_string() returns strlen(), not bytes! */
},
{
.mi_type = GSM_MI_TYPE_IMEI,
@@ -421,7 +420,6 @@
.expect_mi_tlv_hex = "170a937856341290785634f2",
.str_size = 16,
.expect_str = "987654321098765",
-   .expect_rc = 15, /* exception: on truncation, 
gsm48_mi_to_string() returns strlen(), not bytes! */
},
{
/* gsm48 treats TMSI as decimal string */
diff --git a/tests/gsm0408/gsm0408_test.ok b/tests/gsm0408/gsm0408_test.ok
index 1dc4249..2db58de 100644
--- a/tests/gsm0408/gsm0408_test.ok
+++ b/tests/gsm0408/gsm0408_test.ok
@@ -23,8 +23,7 @@
   -> MI-str="4234235" rc=8
 - IMSI 4234235
   -> MI-TLV-hex='170449322453'
-  -> MI-str="423" rc=3
- ERROR: resulting string is not explicitly nul terminated
+  -> MI-str="423" rc=4
 - IMEI 123456789012345
   -> MI-TLV-hex='17081a32547698103254'
   -> MI-str="123456789012345" rc=16
@@ -39,8 +38,7 @@
   -> MI-str="987654321098765432" rc=19
 - IMEI-SV 987654321098765432
   -> MI-TLV-hex='170a937856341290785634f2'
-  -> MI-str="987654321098765" rc=15
- ERROR: resulting string is not explicitly nul terminated
+  -> MI-str="987654321098765" rc=16
 - TMSI 305419896
   -> MI-TLV-hex='1705f412345678'
   -> MI-str="305419896" rc=9
@@ -66,14 +64,14 @@
 Decoding zero length Mobile Identities
 - MI type: IMSI
   - writing to zero-length string:
-rc=1
-ERROR: Wrote to invalid memory!
+rc=0
+nothing written
   - writing to 1-byte-length string:
 rc=1
-ERROR: Wrote unexpected string "1"
+returned empty string
 

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/2/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/2/src/db_hlr.c@644
PS2, Line 644:  if (osmo_clock_gettime(CLOCK_REALTIME, ) != 0) {
> Since starting from this line it seems to be a separate logic (updating VLR 
> subscriber timestamp), I […]
Yes, good idea. I'll roll that into this patch.



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:07:07 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in docker-playground[master]: gerrit: fix libopenid path

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12204 )

Change subject: gerrit: fix libopenid path
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12204
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: Iaed2429a776ae587043dc2651293bb354fceeb72
Gerrit-Change-Number: 12204
Gerrit-PatchSet: 2
Gerrit-Owner: osmith 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:07:36 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-sgsn[master]: Remove misleading comment

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12220 )

Change subject: Remove misleading comment
..

Remove misleading comment

The code which has been used for 4 years is hardly temporary.

Change-Id: Ibe9c62e0beb89eecced941b8831d49ca266c7330
---
M src/gprs/sgsn_auth.c
1 file changed, 0 insertions(+), 1 deletion(-)

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



diff --git a/src/gprs/sgsn_auth.c b/src/gprs/sgsn_auth.c
index 694bece..e3d9127 100644
--- a/src/gprs/sgsn_auth.c
+++ b/src/gprs/sgsn_auth.c
@@ -43,7 +43,6 @@
INIT_LLIST_HEAD(>cfg.imsi_acl);
 }

-/* temporary IMSI ACL hack */
 struct imsi_acl_entry *sgsn_acl_lookup(const char *imsi, struct sgsn_config 
*cfg)
 {
struct imsi_acl_entry *acl;

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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibe9c62e0beb89eecced941b8831d49ca266c7330
Gerrit-Change-Number: 12220
Gerrit-PatchSet: 2
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 

Change in libosmocore[master]: gsm48_mi_to_string(): guard against zero length output buffer

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12156 )

Change subject: gsm48_mi_to_string(): guard against zero length output buffer
..

gsm48_mi_to_string(): guard against zero length output buffer

All successful cases already return from the switch(), so simply handle all
errors below it by returning an empty string (if there is enough string
buffer).

Change-Id: I709ac3b9efb7b4258d8660715b10312e11b9b571
---
M src/gsm/gsm48.c
M tests/gsm0408/gsm0408_test.ok
2 files changed, 13 insertions(+), 13 deletions(-)

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



diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c
index 0f0889b..af3e14c 100644
--- a/src/gsm/gsm48.c
+++ b/src/gsm/gsm48.c
@@ -653,14 +653,11 @@
 {
int rc;
uint8_t mi_type;
-   char *str_cur = string;
uint32_t tmsi;

mi_type = mi[0] & GSM_MI_TYPE_MASK;

switch (mi_type) {
-   case GSM_MI_TYPE_NONE:
-   break;
case GSM_MI_TYPE_TMSI:
/* Table 10.5.4.3, reverse generate_mid_from_tmsi */
if (mi_len == GSM48_TMSI_LEN && mi[0] == (0xf0 | 
GSM_MI_TYPE_TMSI)) {
@@ -680,12 +677,15 @@
return rc + 1;
else
return strlen(string) + 1;
+
default:
break;
}
-   *str_cur++ = '\0';

-   return str_cur - string;
+   if (str_len < 1)
+   return 0;
+   *string = '\0';
+   return 1;
 }

 /*! Parse TS 04.08 Routing Area Identifier
diff --git a/tests/gsm0408/gsm0408_test.ok b/tests/gsm0408/gsm0408_test.ok
index 6e99f5b..d6579e5 100644
--- a/tests/gsm0408/gsm0408_test.ok
+++ b/tests/gsm0408/gsm0408_test.ok
@@ -72,8 +72,8 @@
 returned empty string
 - MI type: TMSI
   - writing to zero-length string:
-rc=1
-ERROR: Wrote to invalid memory!
+rc=0
+nothing written
   - writing to 1-byte-length string:
 rc=1
 returned empty string
@@ -82,8 +82,8 @@
 returned empty string
 - MI type: NONE
   - writing to zero-length string:
-rc=1
-ERROR: Wrote to invalid memory!
+rc=0
+nothing written
   - writing to 1-byte-length string:
 rc=1
 returned empty string
@@ -102,8 +102,8 @@
 returned empty string
 - MI type: TMSI | GSM_MI_ODD
   - writing to zero-length string:
-rc=1
-ERROR: Wrote to invalid memory!
+rc=0
+nothing written
   - writing to 1-byte-length string:
 rc=1
 returned empty string
@@ -112,8 +112,8 @@
 returned empty string
 - MI type: NONE | GSM_MI_ODD
   - writing to zero-length string:
-rc=1
-ERROR: Wrote to invalid memory!
+rc=0
+nothing written
   - writing to 1-byte-length string:
 rc=1
 returned empty string

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I709ac3b9efb7b4258d8660715b10312e11b9b571
Gerrit-Change-Number: 12156
Gerrit-PatchSet: 8
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 

Change in libosmocore[master]: gsm48_mi_to_string(): guard against zero length output buffer

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12156 )

Change subject: gsm48_mi_to_string(): guard against zero length output buffer
..


Patch Set 7: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12156
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: I709ac3b9efb7b4258d8660715b10312e11b9b571
Gerrit-Change-Number: 12156
Gerrit-PatchSet: 7
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:06:11 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in libosmocore[master]: gsm48_mi_to_string(): do not read from zero length input buffer

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12157 )

Change subject: gsm48_mi_to_string(): do not read from zero length input buffer
..

gsm48_mi_to_string(): do not read from zero length input buffer

Change-Id: I12cada7c2c5187146ca5a33d1ebfefb4cad65632
---
M src/gsm/gsm48.c
1 file changed, 1 insertion(+), 1 deletion(-)

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



diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c
index af3e14c..190622f 100644
--- a/src/gsm/gsm48.c
+++ b/src/gsm/gsm48.c
@@ -655,7 +655,7 @@
uint8_t mi_type;
uint32_t tmsi;

-   mi_type = mi[0] & GSM_MI_TYPE_MASK;
+   mi_type = (mi && mi_len) ? (mi[0] & GSM_MI_TYPE_MASK) : 
GSM_MI_TYPE_NONE;

switch (mi_type) {
case GSM_MI_TYPE_TMSI:

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I12cada7c2c5187146ca5a33d1ebfefb4cad65632
Gerrit-Change-Number: 12157
Gerrit-PatchSet: 9
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 

Change in osmo-hlr[master]: disable recording of LU timestamps by default

[Stefan Sperling]

Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

Change subject: disable recording of LU timestamps by default
..


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c
File src/hlr_vty.c:

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c@133
PS2, Line 133:  vty_out(vty, "Timestamps of Location Updates will be stored in 
the HLR database.%s", VTY_NEWLINE);
> IMHO these vty_out lines can be dropped, but fine otherwise. […]
Can you elaborate what you're referring to when you say we're storing 
subscriber names?
Which field of the DB do you mean? Do you mean some other database then the HLR 
db?



--
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:06:20 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in libosmocore[master]: gsm48_mi_to_string(): do not read from zero length input buffer

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12157 )

Change subject: gsm48_mi_to_string(): do not read from zero length input buffer
..


Patch Set 8: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/12157
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: I12cada7c2c5187146ca5a33d1ebfefb4cad65632
Gerrit-Change-Number: 12157
Gerrit-PatchSet: 8
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:06:28 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes