Change in libosmocore[master]: add identifier sanitation for setting FSM instance ids

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13574 )

Change subject: add identifier sanitation for setting FSM instance ids
..


Patch Set 3: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13574
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: Ia40a6f3b2243c95fe428a080b938e11d8ab771a7
Gerrit-Change-Number: 13574
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:59:24 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: tweak OSMO_STRBUF_APPEND(), add OSMO_STRBUF_APPEND_NOLEN()

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13594 )

Change subject: tweak OSMO_STRBUF_APPEND(), add OSMO_STRBUF_APPEND_NOLEN()
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13594
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: I108cadf72deb3a3bcab9a07e50572d9da1ab0359
Gerrit-Change-Number: 13594
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-CC: Jenkins Builder (102)
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:57:43 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for standard args ordering

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13573 )

Change subject: add osmo_{escape,quote}_str_buf2() for standard args ordering
..


Patch Set 3: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13573
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: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:58:38 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for standard args ordering

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13573 )

Change subject: add osmo_{escape,quote}_str_buf2() for standard args ordering
..


Patch Set 3:

...and now I noticed the _c() variant merged to master.
Getting frustrated with string mangling and diverse ideas of how to do them :/


--
To view, visit https://gerrit.osmocom.org/13573
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: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Thu, 11 Apr 2019 05:51:17 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for standard args ordering

2019-04-10 Thread Neels Hofmeyr
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/13573

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

Change subject: add osmo_{escape,quote}_str_buf2() for standard args ordering
..

add osmo_{escape,quote}_str_buf2() for standard args ordering

To be able to append an escaped or quoted string using
OSMO_STRBUF_APPEND_NOLEN(), the function signature must have the buf and len as
first args, like most other *_buf() functions.

Add osmo_escape_str_buf2() and osmo_quote_str_buf2() to match this signature.

A recent patch [1] has changed the return value of osmo_escape_str_buf() to
char*, removing the const. However, the functions may return const strings,
hence re-add the const. The new signatures always return the non-const buffer.

To avoid code duplication, implement osmo_quote_str_buf() and
osmo_escape_str_buf() by calling the new functions.

I decided to allow slight changes to the behavior for current osmo_escape_str()
and osmo_escape_str_buf(), because impact on callers is minimal:

(1) The new implementation uses OSMO_STRBUF_*, and in consequence
osmo_quote_str() no longer prints an ending double quote after truncated
strings; Before, a truncated output was, sic:
  "this string is trunca"
and now this becomes, sic:
  "this string is truncat
I decided to not keep the old behavior because it is questionable to begin
with. It looks like the string actually ended at the truncation boundary
instead of the reason being not enough space in the output buffer.

(2) The new osmo_escape_str_buf2() function obviously cannot pass-thru an
unchanged char* if no escaping was needed. Sacrifice this tiny optimization
feature to avoid code duplication:
- it is an unnoticeable optimization,
- the caller anyway always passes a string buffer,
- the feature caused handling strings and buffers differently depending on
  their content (i.e. code that usually writes out strings in full length
  "suddenly" truncates because a non-printable character is contained, etc.)
I considered adding a skip_if_unescaped flag to the osmo_quote_str_buf2()
function signature, but in the end decided that the API clutter is not worth
having for all the above reasons.

Adjust tests to accomodate above changes.

[1] 4a62eda225ab7f3c9556990c81a6fc5e19b5eec8
Ibf85f79e93244f53b2684ff6f1095c5b41203e05

Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
---
M TODO-RELEASE
M include/osmocom/core/utils.h
M src/utils.c
M tests/utils/utils_test.c
M tests/utils/utils_test.ok
5 files changed, 115 insertions(+), 47 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 


Change in libosmocore[master]: fsm: support graceful osmo_fsm_inst_term() cascades

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13392 )

Change subject: fsm: support graceful osmo_fsm_inst_term() cascades
..

fsm: support graceful osmo_fsm_inst_term() cascades

Add global flag osmo_fsm_term_safely() -- if set to true, enable the following
behavior:

Detect osmo_fsm_inst_term() occuring within osmo_fsm_inst_term():
- collect deallocations until the outermost osmo_fsm_inst_term() is done.
- call osmo_fsm_inst_free() *after* dispatching the parent event.

If a struct osmo_fsm_inst enters osmo_fsm_inst_term() while another is already
within osmo_fsm_inst_term(), do not directly deallocate it, but talloc-reparent
it to a separate talloc context, to be deallocated with the outermost FSM inst.

The effect is that all osmo_fsm_inst freed within an osmo_fsm_inst_term()
cascade will stay allocated until all osmo_fsm_inst_term() are complete and all
of them will be deallocated at the same time.

Mark the deferred deallocation state as __thread in an attempt to make cascaded
deallocation handling threadsafe.  Keep the enable/disable flag separate, so
that it is global and not per-thread.

The feature is showcased by fsm_dealloc_test.c: with this feature, all of those
wild deallocation scenarios succeed.

Make fsm_dealloc_test a normal regression test in testsuite.at.

Rationale:

It is difficult to gracefully handle deallocations of groups of FSM instances
that reference each other. As soon as one child dispatching a cleanup event
causes its parent to deallocate before fsm.c was ready for it, deallocation
will hit a use-after-free. Before this patch, by using parent_term events and
distinct "terminating" FSM states, parent/child FSMs can be taught to wait for
all children to deallocate before deallocating the parent. But as soon as a
non-child / non-parent FSM instance is involved, or actually any other
cleanup() action that triggers parent FSMs or parent talloc contexts to become
unused, it is near impossible to think of all possible deallocation events
ricocheting, and to avoid running into freeing FSM instances that were still in
the middle of osmo_fsm_inst_term(), or FSM instances to enter
osmo_fsm_inst_term() more than once. This patch makes deallocation of "all
possible" setups of complex cross referencing FSM instances easy to handle
correctly, without running into use-after-free or double free situations, and,
notably, without changing calling code.

Change-Id: I8eda67540a1cd91beb7856b9fcd0a3143b18
---
M include/osmocom/core/fsm.h
M src/fsm.c
M tests/Makefile.am
M tests/fsm/fsm_dealloc_test.c
M tests/fsm/fsm_dealloc_test.err
M tests/testsuite.at
6 files changed, 3,503 insertions(+), 301 deletions(-)

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




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


Change in libosmocore[master]: add fsm_dealloc_test.c

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13391 )

Change subject: add fsm_dealloc_test.c
..

add fsm_dealloc_test.c

Despite efforts to properly handle "GONE" events and entering a ST_DESTROYING
only once, so far this test runs straight into a heap use-after-free. With
current fsm.c, it is hard to resolve the situation with the objects named
"other" also causing deallocations besides the FSM instance parent/child
relations.

For illustration, add an "expected" test output file fsm_dealloc_test.err,
making this pass will follow in a subsequent patch.

Change-Id: If801907c541bca9f524c9e5fd22ac280ca16979a
---
M tests/Makefile.am
A tests/fsm/fsm_dealloc_test.c
A tests/fsm/fsm_dealloc_test.err
3 files changed, 606 insertions(+), 1 deletion(-)

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



diff --git a/tests/Makefile.am b/tests/Makefile.am
index d123ee2..09a1c18 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -51,7 +51,11 @@
 endif

 if ENABLE_CTRL
-check_PROGRAMS += ctrl/ctrl_test fsm/fsm_test
+check_PROGRAMS += \
+   ctrl/ctrl_test \
+   fsm/fsm_test \
+   fsm/fsm_dealloc_test \
+   $(NULL)
 endif

 if ENABLE_STATS_TEST
@@ -207,6 +211,9 @@
$(top_builddir)/src/gsm/libosmogsm.la \
$(top_builddir)/src/vty/libosmovty.la

+fsm_fsm_dealloc_test_SOURCES = fsm/fsm_dealloc_test.c
+fsm_fsm_dealloc_test_LDADD = $(LDADD)
+
 write_queue_wqueue_test_SOURCES = write_queue/wqueue_test.c

 socket_socket_test_SOURCES = socket/socket_test.c
diff --git a/tests/fsm/fsm_dealloc_test.c b/tests/fsm/fsm_dealloc_test.c
new file mode 100644
index 000..5a493ad
--- /dev/null
+++ b/tests/fsm/fsm_dealloc_test.c
@@ -0,0 +1,476 @@
+/* Scenarios of parent/child FSM instances cleaning up and deallocating from 
various triggers. */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+enum event {
+   EV_DESTROY,
+   EV_CHILD_GONE,
+   EV_OTHER_GONE,
+};
+
+static const struct value_string test_fsm_event_names[] = {
+   OSMO_VALUE_STRING(EV_DESTROY),
+   OSMO_VALUE_STRING(EV_CHILD_GONE),
+   OSMO_VALUE_STRING(EV_OTHER_GONE),
+   {}
+};
+
+enum state {
+   ST_ALIVE,
+   ST_DESTROYING,
+};
+
+enum objname {
+   root = 0,
+branch0,
+ twig0a,
+ twig0b,
+branch1,
+ twig1a,
+ twig1b,
+
+   other,
+   scene_size
+};
+
+struct scene {
+   struct obj *o[scene_size];
+
+   /* The use count is actually just to help tracking what functions have 
not exited yet */
+   struct osmo_use_count use_count;
+};
+
+int use_cb(struct osmo_use_count_entry *use_count_entry, int32_t 
old_use_count, const char *file, int line)
+{
+   char buf[128];
+   LOGP(DLGLOBAL, LOGL_DEBUG, "%s\n", osmo_use_count_name_buf(buf, 
sizeof(buf), use_count_entry->use_count));
+   return 0;
+}
+
+/* References to related actual objects that are tied to FSM instances. */
+struct obj {
+   struct osmo_fsm_inst *fi;
+   struct scene *s;
+   struct obj *parent;
+   struct obj *child[2];
+   struct obj *other[3];
+};
+
+static void scene_forget_obj(struct scene *s, struct obj *obj)
+{
+   int i;
+   for (i = 0; i < ARRAY_SIZE(obj->s->o); i++) {
+   if (obj->s->o[i] != obj)
+   continue;
+   LOGPFSML(obj->fi, LOGL_DEBUG, "scene forgets %s\n", 
obj->fi->id);
+   obj->s->o[i] = NULL;
+   }
+}
+
+struct scene *g_scene = NULL;
+
+#define GET() \
+   char *token = talloc_asprintf(g_scene, "%s.%s()", obj->fi->id, 
__func__); \
+   osmo_use_count_get_put(_scene->use_count, token, 1)
+
+#define PUT() osmo_use_count_get_put(_scene->use_count, token, -1)
+
+void alive_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
+{
+   LOGPFSML(fi, LOGL_DEBUG, "%s()\n", __func__);
+}
+
+/* Remove obj->other[*] reference, return true if found and removed, false if 
not. */
+bool other_gone(struct obj *obj, struct obj *other)
+{
+   int i;
+   GET();
+   for (i = 0; i < ARRAY_SIZE(obj->other); i++) {
+   if (obj->other[i] == other) {
+   obj->other[i] = NULL;
+   LOGPFSML(obj->fi, LOGL_DEBUG, "EV_OTHER_GONE: Dropped 
reference %s.other[%d] = %s\n", obj->fi->id, i,
+other->fi->id);
+   PUT();
+   return true;
+   }
+   }
+   PUT();
+   return false;
+}
+
+/* Remove obj->child[*] reference, return true if more children remain after 
this, false if all are gone */
+bool child_gone(struct obj *obj, struct obj *child)
+{
+   int i;
+   bool found;
+   if (!child) {
+   LOGPFSML(obj->fi, LOGL_DEBUG, "EV_CHILD_GONE with NULL data, 
must be a parent_term event. Ignore.\n");
+   return true;
+   }
+   

Change in libosmocore[master]: make osmo_sockaddr_str_is_set() NULL-safe

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13572 )

Change subject: make osmo_sockaddr_str_is_set() NULL-safe
..

make osmo_sockaddr_str_is_set() NULL-safe

Obviously a NULL pointer should return false instead of segfaulting.

Change-Id: Iac025cf4d556cbed99f3924cd9ca05a05881cd9a
---
M src/sockaddr_str.c
1 file changed, 2 insertions(+), 1 deletion(-)

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



diff --git a/src/sockaddr_str.c b/src/sockaddr_str.c
index c9d9a94..d683c7d 100644
--- a/src/sockaddr_str.c
+++ b/src/sockaddr_str.c
@@ -60,7 +60,8 @@
  */
 bool osmo_sockaddr_str_is_set(const struct osmo_sockaddr_str *sockaddr_str)
 {
-   return *sockaddr_str->ip
+   return sockaddr_str
+   && *sockaddr_str->ip
&& sockaddr_str->port
&& (sockaddr_str->af == AF_INET || sockaddr_str->af == 
AF_INET6);
 }

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


Change in libosmocore[master]: add osmo_str_startswith()

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13394 )

Change subject: add osmo_str_startswith()
..

add osmo_str_startswith()

Move from a static implementation in tdef_vty.c to utils.c, I also want to use
this in osmo-msc.

The point is that the telnet VTY allows unambiguous partly matches of keyword
args. For example, if I have a command definition of:

compare (apples|oranges)

then it is perfectly legal as for the vty parser to write only

compare app

One could expect the VTY to then pass the unambiguous match of "apples" to the
parsing function, but that is not the case.

Hence a VTY function implementation is faced with parsing a keyword of "app"
instead of the expected "apples".

This is actually a very widespread bug in our VTY implementations, which assume
that exactly one full keyword will always be found. I am now writing new
commands in a way that are able to manage only the starts of keywords.

Arguably, strstr(a, b) == a does the same thing, but it searches the entire
string unnecessarily.

Change-Id: Ib2ffb0e9a870dd52e081c7e66d8818057d159513
---
M include/osmocom/core/utils.h
M src/utils.c
M src/vty/tdef_vty.c
M tests/utils/utils_test.c
M tests/utils/utils_test.ok
5 files changed, 60 insertions(+), 11 deletions(-)

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



diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h
index 51e43ee..474e36c 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -237,4 +237,6 @@
 #define OSMO_STRBUF_PRINTF(STRBUF, fmt, args...) \
OSMO_STRBUF_APPEND(STRBUF, snprintf, fmt, ##args)

+bool osmo_str_startswith(const char *str, const char *startswith_str);
+
 /*! @} */
diff --git a/src/utils.c b/src/utils.c
index b8b4ef5..9ab990a 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -928,4 +928,18 @@
return (sum * 9) % 10 + '0';
 }

+/*! Compare start of a string.
+ * This is an optimisation of 'strstr(str, startswith_str) == str' because it 
doesn't search through the entire string.
+ * \param str  (Longer) string to compare.
+ * \param startswith_str  (Shorter) string to compare with the start of str.
+ * \return true iff the first characters of str fully match startswith_str or 
startswith_str is empty. */
+bool osmo_str_startswith(const char *str, const char *startswith_str)
+{
+   if (!startswith_str || !*startswith_str)
+   return true;
+   if (!str)
+   return false;
+   return strncmp(str, startswith_str, strlen(startswith_str)) == 0;
+}
+
 /*! @} */
diff --git a/src/vty/tdef_vty.c b/src/vty/tdef_vty.c
index 28de21a..0dac2bf 100644
--- a/src/vty/tdef_vty.c
+++ b/src/vty/tdef_vty.c
@@ -247,16 +247,6 @@
 /*! Singleton Tnnn groups definition as set by osmo_tdef_vty_groups_init(). */
 static struct osmo_tdef_group *global_tdef_groups;

-/*! \return true iff the first characters of str fully match startswith_str or 
both are empty. */
-static bool startswith(const char *str, const char *startswith_str)
-{
-   if (!startswith_str)
-   return true;
-   if (!str)
-   return false;
-   return strncmp(str, startswith_str, strlen(startswith_str)) == 0;
-}
-
 DEFUN(show_timer, show_timer_cmd, "DYNAMIC", "DYNAMIC")
   /* show timer [(alpha|beta|gamma)] [T] */
 {
@@ -268,7 +258,7 @@
 * like "softw" or "t" (which can also be ambiguous). */

osmo_tdef_groups_for_each(g, global_tdef_groups) {
-   if (!group_arg || startswith(g->name, group_arg))
+   if (!group_arg || osmo_str_startswith(g->name, group_arg))
osmo_tdef_vty_show_cmd(vty, g->tdefs, T_arg, "%s: ", 
g->name);
}
return CMD_SUCCESS;
diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c
index 711d6e1..211b4d1 100644
--- a/tests/utils/utils_test.c
+++ b/tests/utils/utils_test.c
@@ -1014,6 +1014,33 @@
printf("(need %d chars, had size=63) %s\n", rc, buf);
 }

+static void startswith_test_str(const char *str, const char *startswith_str, 
bool expect_rc)
+{
+   bool rc = osmo_str_startswith(str, startswith_str);
+   printf("osmo_str_startswith(%s, ", osmo_quote_str(str, -1));
+   printf("%s) == %s\n", osmo_quote_str(startswith_str, -1), rc ? "true" : 
"false");
+   if (rc != expect_rc)
+   printf("   ERROR: EXPECTED %s\n", expect_rc ? "true" : "false");
+}
+
+static void startswith_test()
+{
+   printf("\n%s()\n", __func__);
+   startswith_test_str(NULL, NULL, true);
+   startswith_test_str("", NULL, true);
+   startswith_test_str(NULL, "", true);
+   startswith_test_str("", "", true);
+   startswith_test_str("abc", NULL, true);
+   startswith_test_str("abc", "", true);
+   startswith_test_str(NULL, "abc", false);
+   startswith_test_str("", "abc", false);
+   

Change in libosmocore[master]: fsm: add flag to ensure osmo_fsm_inst_term() happens only once

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13545 )

Change subject: fsm: add flag to ensure osmo_fsm_inst_term() happens only once
..

fsm: add flag to ensure osmo_fsm_inst_term() happens only once

To prevent re-entering osmo_fsm_inst_term() twice for the same osmo_fsm_inst,
add flag osmo_fsm_inst.proc.terminating. osmo_fsm_inst_term() sets this to
true, or exits if it already is true.

Update fsm_dealloc_test.err for illustration. It is not relevant for unit
testing yet, just showing the difference.

Change-Id: I0c02d76a86f90c49e0eae2f85db64704c96a7674
---
M TODO-RELEASE
M include/osmocom/core/fsm.h
M src/fsm.c
M tests/fsm/fsm_dealloc_test.err
4 files changed, 349 insertions(+), 38 deletions(-)

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



diff --git a/TODO-RELEASE b/TODO-RELEASE
index ba603c6..5ddc57a 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -10,3 +10,4 @@
 libosmogb  gprs_ns_instAdding bss_sns_fi member for IP-SNS 
support
 libosmogb  gprs_nsvc   Adding sig_weight and data_weight 
members for IP-SNS support
 libosmogb  various new symbols Adding functions related to IP-SNS 
support
+libosmocoreosmo_fsm_inst   Add flag proc.terminating (ABI change)
diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index c40d7f3..07bcd12 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -114,6 +114,8 @@
struct llist_head children;
/*! \ref llist_head linked to parent->proc.children */
struct llist_head child;
+   /*! Indicator whether osmo_fsm_inst_term() was already invoked 
on this instance. */
+   bool terminating;
} proc;
 };

diff --git a/src/fsm.c b/src/fsm.c
index d86ff4b..d18406a 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -710,6 +710,12 @@
struct osmo_fsm_inst *parent;
uint32_t parent_term_event = fi->proc.parent_term_event;

+   if (fi->proc.terminating) {
+   LOGPFSMSRC(fi, file, line, "Ignoring trigger to terminate: 
already terminating\n");
+   return;
+   }
+   fi->proc.terminating = true;
+
LOGPFSMSRC(fi, file, line, "Terminating (cause = %s)\n",
   osmo_fsm_term_cause_name(cause));

diff --git a/tests/fsm/fsm_dealloc_test.err b/tests/fsm/fsm_dealloc_test.err
index 1719677..7f41340 100644
--- a/tests/fsm/fsm_dealloc_test.err
+++ b/tests/fsm/fsm_dealloc_test.err
@@ -41,6 +41,310 @@
 DLGLOBAL DEBUG test(_branch0){alive}: state_chg to destroying
 DLGLOBAL DEBUG 6 
(__twig0a.cleanup(),other.alive(),other.destroying_onenter(),other.cleanup(),_branch0.alive(),_branch0.destroying_onenter())
 DLGLOBAL DEBUG test(_branch0){destroying}: destroying_onenter() from alive
+DLGLOBAL DEBUG test(_branch0){destroying}: Ignoring trigger to terminate: 
already terminating
+DLGLOBAL DEBUG 5 
(__twig0a.cleanup(),other.alive(),other.destroying_onenter(),other.cleanup(),_branch0.alive())
+DLGLOBAL DEBUG 4 
(__twig0a.cleanup(),other.alive(),other.destroying_onenter(),other.cleanup())
+DLGLOBAL DEBUG test(other){destroying}: cleanup() done
+DLGLOBAL DEBUG 3 (__twig0a.cleanup(),other.alive(),other.destroying_onenter())
+DLGLOBAL DEBUG test(other){destroying}: Freeing instance
+DLGLOBAL DEBUG test(other){destroying}: Deallocated
+DLGLOBAL DEBUG 2 (__twig0a.cleanup(),other.alive())
+DLGLOBAL DEBUG 1 (__twig0a.cleanup())
+DLGLOBAL DEBUG test(_branch0){destroying}: Received Event EV_CHILD_GONE
+DLGLOBAL DEBUG 2 (__twig0a.cleanup(),_branch0.destroying())
+DLGLOBAL DEBUG test(_branch0){destroying}: destroying(EV_CHILD_GONE)
+DLGLOBAL DEBUG 3 
(__twig0a.cleanup(),_branch0.destroying(),_branch0.child_gone())
+DLGLOBAL DEBUG test(_branch0){destroying}: EV_CHILD_GONE: Dropped reference 
_branch0.child[0] = __twig0a
+DLGLOBAL DEBUG test(_branch0){destroying}: No more children
+DLGLOBAL DEBUG 2 (__twig0a.cleanup(),_branch0.destroying())
+DLGLOBAL DEBUG 1 (__twig0a.cleanup())
+DLGLOBAL DEBUG test(__twig0a){alive}: cleanup() done
+DLGLOBAL DEBUG 0 (-)
+DLGLOBAL DEBUG test(__twig0a){alive}: Freeing instance
+DLGLOBAL DEBUG test(__twig0a){alive}: Deallocated
+DLGLOBAL DEBUG 1 (_branch0.cleanup())
+DLGLOBAL DEBUG test(_branch0){destroying}: cleanup()
+DLGLOBAL DEBUG test(_branch0){destroying}: scene forgets _branch0
+DLGLOBAL DEBUG test(_branch0){destroying}: cleanup() done
+DLGLOBAL DEBUG 0 (-)
+DLGLOBAL DEBUG test(_branch0){destroying}: Freeing instance
+DLGLOBAL DEBUG test(_branch0){destroying}: Deallocated
+DLGLOBAL DEBUG scene_alloc()
+DLGLOBAL DEBUG test(_branch0){alive}: Allocated
+DLGLOBAL DEBUG test(_branch0){alive}: Allocated
+DLGLOBAL DEBUG test(_branch0){alive}: is child of test(_branch0)
+DLGLOBAL DEBUG test(other){alive}: Allocated
+DLGLOBAL DEBUG test(_branch0){alive}: _branch0.other[0] = other
+DLGLOBAL DEBUG test(other){alive}: 

Change in libosmocore[master]: fsm_dealloc_test: no need for ST_DESTROYING

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13393 )

Change subject: fsm_dealloc_test: no need for ST_DESTROYING
..

fsm_dealloc_test: no need for ST_DESTROYING

A separate ST_DESTROYING state originally helped with certain deallocation
scenarios. But now that fsm.c avoids re-entering osmo_fsm_inst_term() twice and
gracefully handles FSM instance deallocations for termination cascades, it is
actually just as safe without a separate ST_DESTROYING state. ST_DESTROYING was
used to flag deallocation and prevent entering osmo_fsm_inst_term() twice,
which works only in a very limited range of scenarios.

Remove ST_DESTROYING from fsm_dealloc_test.c to show that all tested scenarios
still clean up gracefully.

Change-Id: I05354e6cad9b82ba474fa50ffd41d481b3c697b4
---
M tests/fsm/fsm_dealloc_test.c
M tests/fsm/fsm_dealloc_test.err
2 files changed, 1,522 insertions(+), 1,801 deletions(-)

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




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


Change in libosmocore[master]: add identifier sanitation for setting FSM instance ids

2019-04-10 Thread Neels Hofmeyr
Hello Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/13574

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

Change subject: add identifier sanitation for setting FSM instance ids
..

add identifier sanitation for setting FSM instance ids

We often compose FSM instance IDs from context information, for example placing
an MSISDN string or IP:port information in the FSM instance id, using
osmo_fsm_inst_update_id_f(). This fails if any characters are contained that
don't pass osmo_identifier_valid(). Hence it is the task of the caller to make
sure only characters allowed in an FSM id are applied.

Provide API to trivially allow this by replacing illegal chars:
- osmo_identifier_sanitize_buf(), with access to the same set of illegal
  characters defined in utils.c,
- osmo_fsm_inst_update_id_f_sanitize() implicitly replaces non-identifier
  chars.

This makes it easy to add strings like '192.168.0.1:2342' or '+4987654321' to
an FSM instance id, without adding string mangling to each place that sets an
id; e.g. replacing with '-' to yield '192-168-0-1:2342' or '-4987654321'.

Change-Id: Ia40a6f3b2243c95fe428a080b938e11d8ab771a7
---
M include/osmocom/core/fsm.h
M include/osmocom/core/utils.h
M src/fsm.c
M src/utils.c
4 files changed, 53 insertions(+), 2 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia40a6f3b2243c95fe428a080b938e11d8ab771a7
Gerrit-Change-Number: 13574
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: GSUP: add Class IE

2019-04-10 Thread Neels Hofmeyr
Hello Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/13576

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

Change subject: GSUP: add Class IE
..

GSUP: add Class IE

osmo-msc and osmo-hlr have distinct subsystems handling incoming GSUP messages.
So far we decide entirely by message type which code path should handle a GSUP
message. Thus no GSUP message type may be re-used across subsystems.

If we add a GSUP message to indicate a routing error, it would have to be a
distinct message type for subscriber management, another one for SMS, another
one for USSD...

To allow introducing common message types, introduce a GSUP Class IE.

In the presence of this IE, GSUP handlers can trivially direct a received
message to the right code path. If it is missing, handlers can fall back to the
previous switch(message_type) method.

Change-Id: Ic397a9f2c4a7224e47cab944c72e75ca5592efef
---
M include/osmocom/gsm/gsup.h
M src/gsm/gsup.c
M src/gsm/libosmogsm.map
M tests/gsup/gsup_test.c
M tests/gsup/gsup_test.err
5 files changed, 47 insertions(+), 5 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic397a9f2c4a7224e47cab944c72e75ca5592efef
Gerrit-Change-Number: 13576
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: add OSMO_IMSI_SIZE

2019-04-10 Thread Neels Hofmeyr
Hello Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/13575

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

Change subject: add OSMO_IMSI_SIZE
..

add OSMO_IMSI_SIZE

Various places in our code base figure out how many chars they need to safely
store an IMSI. An IMSI can have a checksum digit, which is not reflected by
GSM23003_IMSI_MAX_DIGITS. And we usually need a terminating \0.

Instead of having a magic +2 repeated every so often, rather define
OSMO_IMSI_SIZE to contain both checksum digit and nul char, and have the
explanatory comment with it here in libosmocore.

Change-Id: Id11ada4c96b79f7f0ad58185ab7dbf24622fb770
---
M include/osmocom/gsm/protocol/gsm_23_003.h
1 file changed, 3 insertions(+), 0 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id11ada4c96b79f7f0ad58185ab7dbf24622fb770
Gerrit-Change-Number: 13575
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: tweak OSMO_STRBUF_APPEND(), add OSMO_STRBUF_APPEND_NOLEN()

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13594


Change subject: tweak OSMO_STRBUF_APPEND(), add OSMO_STRBUF_APPEND_NOLEN()
..

tweak OSMO_STRBUF_APPEND(), add OSMO_STRBUF_APPEND_NOLEN()

In OSMO_STRBUF_APPEND, use local variable names that are less likely to shadow
other local variables: prefix with _sb_.

In OSMO_STRBUF_APPEND, add a check to add to .pos only if it is not NULL.

Add OSMO_STRBUF_APPEND_NOLEN(), which works for function signatures that don't
return a length. This is useful for any osmo_*_buf() string writing functions,
so that these write directly to the strbuf.

Change-Id: I108cadf72deb3a3bcab9a07e50572d9da1ab0359
---
M include/osmocom/core/utils.h
M tests/utils/utils_test.c
M tests/utils/utils_test.ok
3 files changed, 53 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/94/13594/1

diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h
index e19649a..0daad76 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -199,14 +199,14 @@
 #define OSMO_STRBUF_APPEND(STRBUF, func, args...) do { \
if (!(STRBUF).pos) \
(STRBUF).pos = (STRBUF).buf; \
-   size_t remain = (STRBUF).buf ? (STRBUF).len - ((STRBUF).pos - 
(STRBUF).buf) : 0; \
-   int l = func((STRBUF).pos, remain, ##args); \
-   if (l < 0 || l > remain) \
+   size_t _sb_remain = (STRBUF).buf ? (STRBUF).len - ((STRBUF).pos 
- (STRBUF).buf) : 0; \
+   int _sb_l = func((STRBUF).pos, _sb_remain, ##args); \
+   if (_sb_l < 0 || _sb_l > _sb_remain) \
(STRBUF).pos = (STRBUF).buf + (STRBUF).len; \
-   else \
-   (STRBUF).pos += l; \
-   if (l > 0) \
-   (STRBUF).chars_needed += l; \
+   else if ((STRBUF).pos) \
+   (STRBUF).pos += _sb_l; \
+   if (_sb_l > 0) \
+   (STRBUF).chars_needed += _sb_l; \
} while(0)

 /*! Shortcut for OSMO_STRBUF_APPEND() invocation using snprintf().
@@ -231,6 +231,29 @@
 #define OSMO_STRBUF_PRINTF(STRBUF, fmt, args...) \
OSMO_STRBUF_APPEND(STRBUF, snprintf, fmt, ##args)

+/*! Like OSMO_STRBUF_APPEND(), but for function signatures that return the 
char* buffer instead of a length.
+ * When using this function, the final STRBUF.chars_needed may not reflect the 
actual number of characters needed, since
+ * that number cannot be obtained from this kind of function signature.
+ * \param[inout] STRBUF  A struct osmo_strbuf instance.
+ * \param[in] func  A function with a signature of char *func(char *dst, 
size_t dst_len [, args]) where
+ *  the returned string is always written to dst.
+ * \param[in] args  Arguments passed to func, if any.
+ */
+#define OSMO_STRBUF_APPEND_NOLEN(STRBUF, func, args...) do { \
+   if (!(STRBUF).pos) \
+   (STRBUF).pos = (STRBUF).buf; \
+   size_t _sb_remain = (STRBUF).buf ? (STRBUF).len - ((STRBUF).pos 
- (STRBUF).buf) : 0; \
+   if (_sb_remain) { \
+   func((STRBUF).pos, _sb_remain, ##args); \
+   } \
+   size_t _sb_l = (STRBUF).pos ? strnlen((STRBUF).pos, _sb_remain) 
: 0; \
+   if (_sb_l > _sb_remain) \
+   (STRBUF).pos = (STRBUF).buf + (STRBUF).len; \
+   else if ((STRBUF).pos) \
+   (STRBUF).pos += _sb_l; \
+   (STRBUF).chars_needed += _sb_l; \
+   } while(0)
+
 bool osmo_str_startswith(const char *str, const char *startswith_str);

 /*! @} */
diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c
index 211b4d1..223f67d 100644
--- a/tests/utils/utils_test.c
+++ b/tests/utils/utils_test.c
@@ -1014,6 +1014,23 @@
printf("(need %d chars, had size=63) %s\n", rc, buf);
 }

+void strbuf_test_nolen()
+{
+   char buf[20];
+   struct osmo_strbuf sb = { .buf = buf, .len = sizeof(buf) };
+   uint8_t ubits[] = {0, 0, 0, 1, 0, 1, 1, 1, 0, 0, 1, 0, 1, 0, 1, 0};
+   printf("\n%s\n", __func__);
+
+   OSMO_STRBUF_APPEND_NOLEN(sb, osmo_ubit_dump_buf, ubits, sizeof(ubits));
+   printf("%zu: %s (need=%zu)\n", sb.len, buf, sb.chars_needed);
+   OSMO_STRBUF_APPEND_NOLEN(sb, osmo_ubit_dump_buf, ubits, sizeof(ubits));
+   printf("more: %s (need=%zu)\n", buf, sb.chars_needed);
+
+   sb = (struct osmo_strbuf){ .buf = buf, .len = 10 };
+   OSMO_STRBUF_APPEND_NOLEN(sb, osmo_ubit_dump_buf, ubits, sizeof(ubits));
+   printf("%zu: %s (need=%zu)\n", sb.len, buf, sb.chars_needed);
+}
+
 static void startswith_test_str(const char *str, const char *startswith_str, 
bool expect_rc)
 {
bool rc = osmo_str_startswith(str, startswith_str);
@@ -1059,6 +1076,7 @@

Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for standard args ordering

2019-04-10 Thread Neels Hofmeyr
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/13573

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

Change subject: add osmo_{escape,quote}_str_buf2() for standard args ordering
..

add osmo_{escape,quote}_str_buf2() for standard args ordering

To be able to append an escaped or quoted string using
OSMO_STRBUF_APPEND_NOLEN(), the function signature must have the buf and len as
first args, like most other *_buf() functions.

Add osmo_escape_str_buf2() and osmo_quote_str_buf2() to match this signature.

A recent patch [1] has changed the return value of osmo_escape_str_buf() and
osmo_quote_str_buf() to char*, removing the const. However, the functions may
return const strings, hence re-add the const. The new signatures are guaranteed
to return the non-const buffer.

To avoid code duplication, implement osmo_quote_str_buf() and
osmo_escape_str_buf() by calling the new functions.

I decided to allow slight changes to the behavior for current osmo_escape_str()
and osmo_escape_str_buf(), because impact on callers is minimal:

(1) The new implementation uses OSMO_STRBUF_*, and in consequence
osmo_quote_str() no longer prints an ending double quote after truncated
strings; Before, a truncated output was, sic:
  "this string is trunca"
and now this becomes, sic:
  "this string is truncat
I decided to not keep the old behavior because it is questionable to begin
with. It looks like the string actually ended at the truncation boundary
instead of the reason being not enough space in the output buffer.

(2) The new osmo_escape_str_buf2() function obviously cannot pass-thru an
unchanged char* if no escaping was needed. Sacrifice this tiny optimization
feature to avoid code duplication:
- it is an unnoticeable optimization,
- the caller anyway always passes a string buffer,
- the feature caused handling strings and buffers differently depending on
  their content (i.e. code that usually writes out strings in full length
  "suddenly" truncates because a non-printable character is contained, etc.)
I considered adding a skip_if_unescaped flag to the osmo_quote_str_buf2()
function signature, but in the end decided that the API clutter is not worth
having for all the above reasons.

Adjust tests to accomodate above changes.

[1] 4a62eda225ab7f3c9556990c81a6fc5e19b5eec8
Ibf85f79e93244f53b2684ff6f1095c5b41203e05

Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
---
M TODO-RELEASE
M include/osmocom/core/utils.h
M src/utils.c
M tests/utils/utils_test.c
M tests/utils/utils_test.ok
5 files changed, 115 insertions(+), 47 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 


Change in osmo-msc[master]: libmsc/ran_conn.c: send S_SUBSCR_ATTACHED after RAN_CONN_S_ACCEPTED

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has abandoned this change. ( https://gerrit.osmocom.org/13571 )

Change subject: libmsc/ran_conn.c: send S_SUBSCR_ATTACHED after 
RAN_CONN_S_ACCEPTED
..


Abandoned

This change was only needed to demonstrate the problem and a potential 
solution. Not compatible with neels/ho.
--
To view, visit https://gerrit.osmocom.org/13571
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Id5bc2d1d488a0ab92fd72e9cef6250e6794807a9
Gerrit-Change-Number: 13571
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder (102)


Change in osmo-msc[master]: smpp: Make libsmpp34 use talloc for its allocations

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13562 )

Change subject: smpp: Make libsmpp34 use talloc for its allocations
..


Patch Set 3:

> @vadim: please revert.

Done. My apologies...


--
To view, visit https://gerrit.osmocom.org/13562
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: Ie2725ffab6a225813e65768735f01678e2022128
Gerrit-Change-Number: 13562
Gerrit-PatchSet: 3
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Vadim Yanitskiy 
Gerrit-Comment-Date: Thu, 11 Apr 2019 04:38:55 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-msc[master]: smpp: Make libsmpp34 use talloc for its allocations

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has uploaded a new patch set (#3) to the change originally 
created by Harald Welte. ( https://gerrit.osmocom.org/13562 )

Change subject: smpp: Make libsmpp34 use talloc for its allocations
..

smpp: Make libsmpp34 use talloc for its allocations

We are just introducing smpp34_set_memory_functions() in libsmpp34
to allow applications like OsmoMSC to provide their own heap allocator
callback functions.  Let's used this to integrate with talloc and
hence allow talloc tracking/debugging for libsmpp34 internal
allocations.

Depends: libsmpp34 Change-Id I3656117115e89638c093bfbcbc4369ce302f7a94
Change-Id: Ie2725ffab6a225813e65768735f01678e2022128
Related: OS#3913
---
M src/libmsc/smpp_openbsc.c
1 file changed, 27 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/62/13562/3
--
To view, visit https://gerrit.osmocom.org/13562
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: Ie2725ffab6a225813e65768735f01678e2022128
Gerrit-Change-Number: 13562
Gerrit-PatchSet: 3
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Vadim Yanitskiy 


Change in libsmpp34[master]: Allow application to override default heap allocator

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has uploaded a new patch set (#6) to the change originally 
created by Harald Welte. ( https://gerrit.osmocom.org/13561 )

Change subject: Allow application to override default heap allocator
..

Allow application to override default heap allocator

Let's introduce a mechanism by which libsmpp34-using applications can
override the memory allocator functions.  This allows us e.g. in the
Osmocom context to use talloc which aids us in debugging memory leaks.

Closes: OS#3913
Change-Id: I3656117115e89638c093bfbcbc4369ce302f7a94
---
M TODO-RELEASE
M src/Makefile.am
A src/smpp34_heap.c
A src/smpp34_heap.h
M src/smpp34_params.c
M src/smpp34_unpack.c
6 files changed, 108 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libsmpp34 refs/changes/61/13561/6
--
To view, visit https://gerrit.osmocom.org/13561
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3656117115e89638c093bfbcbc4369ce302f7a94
Gerrit-Change-Number: 13561
Gerrit-PatchSet: 6
Gerrit-Owner: Harald Welte 
Gerrit-Assignee: fixeria 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Vadim Yanitskiy 
Gerrit-CC: fixeria 


Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for OSMO_STRBUF_APPEND() use

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13573 )

Change subject: add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use
..


Patch Set 1:

really annoying: osmo_quote_str_buf() has the buf arg in the end, unlike the 
other _buf functions :P


--
To view, visit https://gerrit.osmocom.org/13573
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: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Thu, 11 Apr 2019 04:13:41 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-bts[master]: pcu_sock: use %zu conversion specifier for printing sizeof() result

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13267 )

Change subject: pcu_sock: use %zu conversion specifier for printing sizeof() 
result
..


Patch Set 4: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/#/c/13267/4/src/common/pcu_sock.c
File src/common/pcu_sock.c:

https://gerrit.osmocom.org/#/c/13267/4/src/common/pcu_sock.c@801
PS4, Line 801: return 0;
BTW: it looks like we have a memleak here: msg is allocated, but not freed...



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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5cb656537b1b73b9361a132801ab47ab7f8a709
Gerrit-Change-Number: 13267
Gerrit-PatchSet: 4
Gerrit-Owner: dexter 
Gerrit-Reviewer: Daniel Willmann 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Reviewer: dexter 
Gerrit-Comment-Date: Thu, 11 Apr 2019 04:13:22 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for OSMO_STRBUF_APPEND() use

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13573 )

Change subject: add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use
..


Patch Set 1:

> So, this patch should probably not bother to introduce _buf functions that 
> adhere to the snprintf() signature. It is fine to return a char* and use 
> those with OSMO_STRBUF_PRINTF().

I remember now why i wanted a function that is directly usable for 
OSMO_STRBUF_APPEND(): it allows using the target buffer directly.

If I use OSMO_STRBUF_PRINTF(sb, "%s", osmo_escape_str_buf(buf2, ...)),
then I need to define a *second* buffer, and even if the final target buffer is 
large enough, if the intermediate buffer is too small, that part of the string 
will be truncated.

If instead I can use

  OSMO_STRBUF_APPEND(sb, osmo_escape_str_buf2, str, in_len);

then there is only one buffer involved, and that is the final target buffer 
that was probably passed in by the user.

So, having a signature returning char* is useful as direct printf() arg,
but also having a signature that returns int is useful for using the same 
buffer with OSMO_STRBUF_APPEND().

options that come to mind:

- add another signature kind that behaves like snprintf(), not *_buf but 
something named like snprintf(), which works with OSMO_STRBUF_APPEND() 
directly. maybe osmo_foo_name_snf()

- add another OSMO_STRBUF_xxx() macro that feeds the same sb buffer to *_buf() 
like signatures. That would have to do a strlen() to figure out how much of the 
buffer is left afterwards, but it would save the need for a lot of yet more str 
writing functions.

Going for the latter.


--
To view, visit https://gerrit.osmocom.org/13573
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: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Thu, 11 Apr 2019 04:07:28 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: make all library-internal static buffers thread-local

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13436 )

Change subject: make all library-internal static buffers thread-local
..


Patch Set 5: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/13436
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: I50eb2436a7c1261d79a9d2955584dce92780ca07
Gerrit-Change-Number: 13436
Gerrit-PatchSet: 5
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Thu, 11 Apr 2019 03:47:00 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-bts[master]: oc2gbts_mgr: use osmo_init_logging2() instead of osmo_init_logging()

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13275 )

Change subject: oc2gbts_mgr: use osmo_init_logging2() instead of 
osmo_init_logging()
..


Patch Set 4: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/#/c/13275/4/src/osmo-bts-oc2g/misc/oc2gbts_mgr.c
File src/osmo-bts-oc2g/misc/oc2gbts_mgr.c:

https://gerrit.osmocom.org/#/c/13275/4/src/osmo-bts-oc2g/misc/oc2gbts_mgr.c@264
PS4, Line 264: mgr_log_init
BTW: what are the benefit(s) of having this function? It basically calls 
osmo_init_logging2(), so why not to call it directly from main()? Not related 
to this change, can be done separately...



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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebc80cd1f77f10a879d4536d788377f522dd853f
Gerrit-Change-Number: 13275
Gerrit-PatchSet: 4
Gerrit-Owner: dexter 
Gerrit-Reviewer: Daniel Willmann 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Thu, 11 Apr 2019 03:20:37 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for OSMO_STRBUF_APPEND() use

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13573 )

Change subject: add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use
..


Patch Set 1:

> _buf changes

We've recently added the OSMO_STRBUF_APPEND() macro, which works well with 
function signatures like snprintf(), i.e. first arguments are buf and len, and 
it returns an *int* len of characters that would have been written.

The _buf functions generally return a char* to be useful for printf formats. 
They can't be used with OSMO_STRBUF_APPEND() directly because that requires the 
int return val. However, they *can* be used with OSMO_STRBUF_PRINTF():

  OSMO_STRBUF_PRINTF(sb, "%s", osmo_gsm48_classmark_a5_name_buf(...));

So, this patch should probably not bother to introduce _buf functions that 
adhere to the snprintf() signature. It is fine to return a char* and use those 
with OSMO_STRBUF_PRINTF().


--
To view, visit https://gerrit.osmocom.org/13573
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: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Thu, 11 Apr 2019 01:24:58 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmo-abis[master]: ipa_keepalive_fsm: Suppress error messages for INIT -> INIT transition

2019-04-10 Thread Harald Welte
Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13541 )

Change subject: ipa_keepalive_fsm: Suppress error messages for INIT -> INIT 
transition
..

ipa_keepalive_fsm: Suppress error messages for INIT -> INIT transition

If we receive an OSMO_IPA_KA_E_STOP in INIT state, we are trying to
re-enter INIT, which is not permitted as per the FSM definition.

Adding this permission avoids the below error message from hitting the
logs every time this happens:

<0003> input/ipa_keepalive.c:158 IPA-KEEPALIVE(server)[0x61200520]{INIT}: 
transition to state INIT not permitted!

Change-Id: I8db2f2e708fc4fbb81f5019973098a80e8f540d2
---
M src/input/ipa_keepalive.c
1 file changed, 1 insertion(+), 1 deletion(-)

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



diff --git a/src/input/ipa_keepalive.c b/src/input/ipa_keepalive.c
index 6467720..81b5a26 100644
--- a/src/input/ipa_keepalive.c
+++ b/src/input/ipa_keepalive.c
@@ -167,7 +167,7 @@
[OSMO_IPA_KA_S_INIT] = {
.name = "INIT",
.in_event_mask = S(OSMO_IPA_KA_E_START),
-   .out_state_mask = S(OSMO_IPA_KA_S_WAIT_RESP),
+   .out_state_mask = S(OSMO_IPA_KA_S_WAIT_RESP) | 
S(OSMO_IPA_KA_S_INIT),
.action = ipa_ka_init,
},
[OSMO_IPA_KA_S_IDLE] = {

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

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8db2f2e708fc4fbb81f5019973098a80e8f540d2
Gerrit-Change-Number: 13541
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 


Change in libosmo-abis[master]: ipa_keepalive_fsm: Fix OSMO_IPA_KA_E_STOP allstate event

2019-04-10 Thread Harald Welte
Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13540 )

Change subject: ipa_keepalive_fsm: Fix OSMO_IPA_KA_E_STOP allstate event
..

ipa_keepalive_fsm: Fix OSMO_IPA_KA_E_STOP allstate event

We had the allstate_action function registered, and that function
implemented handling of OSMO_IPA_KA_E_STOP.  However, the
allstate_event_mask was not set, rendering this functionality
inaccessible.

Change-Id: I83fd991bdacb7bab794878e47c7797fecf8b9286
---
M src/input/ipa_keepalive.c
1 file changed, 1 insertion(+), 0 deletions(-)

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



diff --git a/src/input/ipa_keepalive.c b/src/input/ipa_keepalive.c
index 1aae096..6467720 100644
--- a/src/input/ipa_keepalive.c
+++ b/src/input/ipa_keepalive.c
@@ -189,6 +189,7 @@
.states = ipa_keepalive_states,
.num_states = ARRAY_SIZE(ipa_keepalive_states),
.log_subsys = DLINP,
+   .allstate_event_mask = S(OSMO_IPA_KA_E_STOP),
.allstate_action = ipa_ka_allstate_action,
.event_names = ipa_keepalive_event_names,
.timer_cb = ipa_ka_fsm_timer_cb,

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

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I83fd991bdacb7bab794878e47c7797fecf8b9286
Gerrit-Change-Number: 13540
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 


Change in osmo-bts[master]: oc2gbts_mgr: use osmo_init_logging2() instead of osmo_init_logging()

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13275 )

Change subject: oc2gbts_mgr: use osmo_init_logging2() instead of 
osmo_init_logging()
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebc80cd1f77f10a879d4536d788377f522dd853f
Gerrit-Change-Number: 13275
Gerrit-PatchSet: 4
Gerrit-Owner: dexter 
Gerrit-Reviewer: Daniel Willmann 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Thu, 11 Apr 2019 00:25:32 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: gsm_utils.c: fix Doxygen description for gsm_get_octet_len()

2019-04-10 Thread Harald Welte
Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13552 )

Change subject: gsm_utils.c: fix Doxygen description for gsm_get_octet_len()
..

gsm_utils.c: fix Doxygen description for gsm_get_octet_len()

Change-Id: Id6fd2cd33be1cb7cd7ff6a43bfcfb1f368304522
---
M src/gsm/gsm_utils.c
1 file changed, 4 insertions(+), 4 deletions(-)

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



diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c
index 1450ed0..94c6ca5 100644
--- a/src/gsm/gsm_utils.c
+++ b/src/gsm/gsm_utils.c
@@ -185,10 +185,10 @@
return -1;
 }

-/*! \brife Compute number of octets from number of septets,
- *  for instance: 47 septets needs 41,125 = 42 octets
- *  \param[in sept_len Number of Septets
- *  \returns Number of octets required */
+/*! Compute number of octets from number of septets.
+ * For instance: 47 septets need 41,125 = 42 octets.
+ * \param[in] sept_len Number of septets
+ * \returns Number of octets required */
 uint8_t gsm_get_octet_len(const uint8_t sept_len){
int octet_len = (sept_len * 7) / 8;
if ((sept_len * 7) % 8 != 0)

--
To view, visit https://gerrit.osmocom.org/13552
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: Id6fd2cd33be1cb7cd7ff6a43bfcfb1f368304522
Gerrit-Change-Number: 13552
Gerrit-PatchSet: 2
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: vty/talloc_ctx_vty.c: use REG_NOSUB flag of regcomp()

2019-04-10 Thread Harald Welte
Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13536 )

Change subject: vty/talloc_ctx_vty.c: use REG_NOSUB flag of regcomp()
..

vty/talloc_ctx_vty.c: use REG_NOSUB flag of regcomp()

We don't need to know position of matches: just yes or no.
This change would save some computation power.

Change-Id: Id55ffe64cc1a35dd83f61dbb0f9828aa676696f9
---
M src/vty/talloc_ctx_vty.c
1 file changed, 1 insertion(+), 1 deletion(-)

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



diff --git a/src/vty/talloc_ctx_vty.c b/src/vty/talloc_ctx_vty.c
index c4d5a88..16cb763 100644
--- a/src/vty/talloc_ctx_vty.c
+++ b/src/vty/talloc_ctx_vty.c
@@ -199,7 +199,7 @@
int rc;

/* Attempt to compile a regular expression */
-   rc = regcomp(, argv[2], 0);
+   rc = regcomp(, argv[2], REG_NOSUB);
if (rc) {
vty_out(vty, "Invalid expression%s", VTY_NEWLINE);
return CMD_WARNING;

--
To view, visit https://gerrit.osmocom.org/13536
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: Id55ffe64cc1a35dd83f61dbb0f9828aa676696f9
Gerrit-Change-Number: 13536
Gerrit-PatchSet: 3
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: vty/talloc_ctx_vty.c: allocate walk_cb_params on stack, not heap

2019-04-10 Thread Harald Welte
Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13534 )

Change subject: vty/talloc_ctx_vty.c: allocate walk_cb_params on stack, not heap
..

vty/talloc_ctx_vty.c: allocate walk_cb_params on stack, not heap

There is no need to allocate struct 'walk_cb_params' dynamically.

Change-Id: I96f25f1ddb36b19b12055deaeeb6f58e59180e72
---
M src/vty/talloc_ctx_vty.c
1 file changed, 15 insertions(+), 43 deletions(-)

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



diff --git a/src/vty/talloc_ctx_vty.c b/src/vty/talloc_ctx_vty.c
index e7fe675..c4d5a88 100644
--- a/src/vty/talloc_ctx_vty.c
+++ b/src/vty/talloc_ctx_vty.c
@@ -180,22 +180,13 @@
 DEFUN(show_talloc_ctx, show_talloc_ctx_cmd,
BASE_CMD_STR, BASE_CMD_DESCR)
 {
-   struct walk_cb_params *params;
-
-   /* Allocate memory */
-   params = talloc_zero(tall_vty_ctx, struct walk_cb_params);
-   if (!params)
-   return CMD_WARNING;
+   struct walk_cb_params params = { 0 };

/* Set up callback parameters */
-   params->filter = WALK_FILTER_NONE;
-   params->vty = vty;
+   params.filter = WALK_FILTER_NONE;
+   params.vty = vty;

-   talloc_ctx_walk(argv[0], argv[1], params);
-
-   /* Free memory */
-   talloc_free(params);
-
+   talloc_ctx_walk(argv[0], argv[1], );
return CMD_SUCCESS;
 }

@@ -204,31 +195,22 @@
"Filter chunks using regular expression\n"
"Regular expression\n")
 {
-   struct walk_cb_params *params;
+   struct walk_cb_params params = { 0 };
int rc;

-   /* Allocate memory */
-   params = talloc_zero(tall_vty_ctx, struct walk_cb_params);
-   if (!params)
-   return CMD_WARNING;
-
/* Attempt to compile a regular expression */
-   rc = regcomp(>regexp, argv[2], 0);
+   rc = regcomp(, argv[2], 0);
if (rc) {
vty_out(vty, "Invalid expression%s", VTY_NEWLINE);
-   talloc_free(params);
return CMD_WARNING;
}

/* Set up callback parameters */
-   params->filter = WALK_FILTER_REGEXP;
-   params->vty = vty;
+   params.filter = WALK_FILTER_REGEXP;
+   params.vty = vty;

-   talloc_ctx_walk(argv[0], argv[1], params);
-
-   /* Free memory */
-   regfree(>regexp);
-   talloc_free(params);
+   talloc_ctx_walk(argv[0], argv[1], );
+   regfree();

return CMD_SUCCESS;
 }
@@ -238,31 +220,21 @@
"Display only a specific memory chunk\n"
"Chunk address (e.g. 0xdeadbeef)\n")
 {
-   struct walk_cb_params *params;
+   struct walk_cb_params params = { 0 };
int rc;

-   /* Allocate memory */
-   params = talloc_zero(tall_vty_ctx, struct walk_cb_params);
-   if (!params)
-   return CMD_WARNING;
-
/* Attempt to parse an address */
-   rc = sscanf(argv[2], "%p", >chunk_ptr);
+   rc = sscanf(argv[2], "%p", _ptr);
if (rc != 1) {
vty_out(vty, "Invalid chunk address%s", VTY_NEWLINE);
-   talloc_free(params);
return CMD_WARNING;
}

/* Set up callback parameters */
-   params->filter = WALK_FILTER_TREE;
-   params->vty = vty;
+   params.filter = WALK_FILTER_TREE;
+   params.vty = vty;

-   talloc_ctx_walk(argv[0], argv[1], params);
-
-   /* Free memory */
-   talloc_free(params);
-
+   talloc_ctx_walk(argv[0], argv[1], );
return CMD_SUCCESS;
 }


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


Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for OSMO_STRBUF_APPEND() use

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13573 )

Change subject: add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use
..


Patch Set 1:

> I see some overlap here with my _buf changes

I noticed the overlap with commit "osmo_escape_str_buf: Always copy, don't 
return input string pointer" -- this patch existed before you merged that, and 
this patch had the exact same changes to osmo_escape_str_buf() in it. After 
that I stripped this commit log msg.

Now noticing the commit "Add _buf() functions to bypass static string buffers" 
is also merged.

I should have another close look...


--
To view, visit https://gerrit.osmocom.org/13573
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: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Wed, 10 Apr 2019 23:23:44 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add osmo_str_startswith()

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13394 )

Change subject: add osmo_str_startswith()
..


Patch Set 2:

I agree that we should fix the vty keyword problem. Even though that's the 
reason why I wanted this function in the first place, it's still a useful 
function in general... maybe I should strip the commit log a bit.


--
To view, visit https://gerrit.osmocom.org/13394
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: Ib2ffb0e9a870dd52e081c7e66d8818057d159513
Gerrit-Change-Number: 13394
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Wed, 10 Apr 2019 23:18:14 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add vty_is_active()

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13581 )

Change subject: add vty_is_active()
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13581
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: I42cf2af47283dd42c101faae0fac293c3a68d599
Gerrit-Change-Number: 13581
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:53:46 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add osmo_bssap_tlv_parse2() for multiple identical T

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13580 )

Change subject: add osmo_bssap_tlv_parse2() for multiple identical T
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13580
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: Ib9a2095f7498dc2cda2a57154b2dbe4621df72f8
Gerrit-Change-Number: 13580
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:52:08 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: BSSMAP: add messages for inter-BSC and inter-MSC Handover

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13578 )

Change subject: BSSMAP: add messages for inter-BSC and inter-MSC Handover
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13578
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: I9dac375331f6bea744769e973725d58e35f87226
Gerrit-Change-Number: 13578
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:50:28 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add gsm48_decode_bcd_number2() from osmo-msc

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13579 )

Change subject: add gsm48_decode_bcd_number2() from osmo-msc
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13579
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: Idb6ae6e2f3bea11ad420dae14d021ac36d99e921
Gerrit-Change-Number: 13579
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:50:51 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: BSSMAP: tweaks

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13577 )

Change subject: BSSMAP: tweaks
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13577
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: I6387836bab76e1fa42daa0f42ab94fc14b70b112
Gerrit-Change-Number: 13577
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:48:21 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: GSUP: add Kind IE

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13576 )

Change subject: GSUP: add Kind IE
..


Patch Set 1: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/#/c/13576/1/include/osmocom/gsm/gsup.h
File include/osmocom/gsm/gsup.h:

https://gerrit.osmocom.org/#/c/13576/1/include/osmocom/gsm/gsup.h@234
PS1, Line 234: OSMO_GSUP_KIND_UNSET
I would prefer "message class" over "message kind".  There's already several 
3GPP protocols we deal with (among those, Abis OML) where there's a message 
class and a message type, and hence the class already has some pre-existing 
notion/definition.  Alternatively, one could also call it subsystem or the 
like, but I think class is quite ok?



--
To view, visit https://gerrit.osmocom.org/13576
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: Ic397a9f2c4a7224e47cab944c72e75ca5592efef
Gerrit-Change-Number: 13576
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:47:40 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add identifier sanitation for setting FSM instance ids

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13574 )

Change subject: add identifier sanitation for setting FSM instance ids
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13574
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: Ia40a6f3b2243c95fe428a080b938e11d8ab771a7
Gerrit-Change-Number: 13574
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:44:27 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add OSMO_IMSI_SIZE

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13575 )

Change subject: add OSMO_IMSI_SIZE
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13575
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: Id11ada4c96b79f7f0ad58185ab7dbf24622fb770
Gerrit-Change-Number: 13575
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:44:37 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: Add _c versions of functions that otherwise return static buffers

2019-04-10 Thread Harald Welte
Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13311 )

Change subject: Add _c versions of functions that otherwise return static 
buffers
..

Add _c versions of functions that otherwise return static buffers

We have a habit of returning static buffers from some functions,
particularly when generating some kind of string values.  This is
convenient in terms of memory management, but it comes at the expense
of not being thread-safe, and not allowing for two calls of the
related function within one printf() statement.

Let's introduce _c suffix versions of those functions where the
caller passes in a talloc context from which the output buffer shall
be allocated.

Change-Id: I8481c19b68ff67cfa22abb93c405ebcfcb0ab19b
---
M include/osmocom/core/msgb.h
M include/osmocom/core/socket.h
M include/osmocom/core/utils.h
M include/osmocom/gprs/gprs_ns.h
M include/osmocom/gsm/abis_nm.h
M include/osmocom/gsm/apn.h
M include/osmocom/gsm/gsm0808_utils.h
M include/osmocom/gsm/gsm23003.h
M include/osmocom/gsm/gsm48.h
M include/osmocom/gsm/gsm_utils.h
M include/osmocom/gsm/protocol/gsm_04_08.h
M include/osmocom/gsm/rsl.h
M include/osmocom/sim/sim.h
M src/gb/gprs_ns.c
M src/gb/libosmogb.map
M src/gsm/abis_nm.c
M src/gsm/apn.c
M src/gsm/gsm0808_utils.c
M src/gsm/gsm23003.c
M src/gsm/gsm48.c
M src/gsm/gsm_utils.c
M src/gsm/libosmogsm.map
M src/gsm/rsl.c
M src/msgb.c
M src/sim/core.c
M src/socket.c
M src/utils.c
27 files changed, 474 insertions(+), 11 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/include/osmocom/core/msgb.h b/include/osmocom/core/msgb.h
index 0c51ce2..e05d37f 100644
--- a/include/osmocom/core/msgb.h
+++ b/include/osmocom/core/msgb.h
@@ -60,6 +60,7 @@
unsigned char _data[0]; /*!< optional immediate data array */
 };

+extern struct msgb *msgb_alloc_c(const void *ctx, uint16_t size, const char 
*name);
 extern struct msgb *msgb_alloc(uint16_t size, const char *name);
 extern void msgb_free(struct msgb *m);
 extern void msgb_enqueue(struct llist_head *queue, struct msgb *msg);
@@ -68,9 +69,11 @@
 uint16_t msgb_length(const struct msgb *msg);
 extern const char *msgb_hexdump(const struct msgb *msg);
 char *msgb_hexdump_buf(char *buf, size_t buf_len, const struct msgb *msg);
+char *msgb_hexdump_c(const void *ctx, const struct msgb *msg);
 extern int msgb_resize_area(struct msgb *msg, uint8_t *area,
int old_size, int new_size);
 extern struct msgb *msgb_copy(const struct msgb *msg, const char *name);
+extern struct msgb *msgb_copy_c(const void *ctx, const struct msgb *msg, const 
char *name);
 static int msgb_test_invariant(const struct msgb *msg) __attribute__((pure));

 /*! Free all msgbs from a queue built with msgb_enqueue().
@@ -501,6 +504,29 @@
return msgb_trim(msg, (msg->l3h - msg->data) + l3len);
 }

+/*! Allocate message buffer with specified headroom from specified talloc 
context.
+ *  \param[in] ctx talloc context from which to allocate
+ *  \param[in] size size in bytes, including headroom
+ *  \param[in] headroom headroom in bytes
+ *  \param[in] name human-readable name
+ *  \returns allocated message buffer with specified headroom
+ *
+ * This function is a convenience wrapper around \ref msgb_alloc
+ * followed by \ref msgb_reserve in order to create a new \ref msgb with
+ * user-specified amount of headroom.
+ */
+static inline struct msgb *msgb_alloc_headroom_c(const void *ctx, int size, 
int headroom,
+const char *name)
+{
+   osmo_static_assert(size > headroom, headroom_bigger);
+
+   struct msgb *msg = msgb_alloc_c(ctx, size, name);
+   if (msg)
+   msgb_reserve(msg, headroom);
+   return msg;
+}
+
+
 /*! Allocate message buffer with specified headroom
  *  \param[in] size size in bytes, including headroom
  *  \param[in] headroom headroom in bytes
diff --git a/include/osmocom/core/socket.h b/include/osmocom/core/socket.h
index 4f6ed72..37b1eae 100644
--- a/include/osmocom/core/socket.h
+++ b/include/osmocom/core/socket.h
@@ -68,6 +68,7 @@

 char *osmo_sock_get_name(const void *ctx, int fd);
 const char *osmo_sock_get_name2(int fd);
+char *osmo_sock_get_name2_c(const void *ctx, int fd);
 int osmo_sock_get_name_buf(char *str, size_t str_len, int fd);
 int osmo_sock_get_ip_and_port(int fd, char *ip, size_t ip_len, char *port, 
size_t port_len, bool local);
 int osmo_sock_get_local_ip(int fd, char *host, size_t len);
diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h
index 6a2b7d5..51e43ee 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -56,7 +56,9 @@
 char *osmo_ubit_dump_buf(char *buf, size_t buf_len, const uint8_t *bits, 
unsigned int len);
 char *osmo_ubit_dump(const uint8_t *bits, unsigned int len);
 

Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for OSMO_STRBUF_APPEND() use

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13573 )

Change subject: add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use
..


Patch Set 1:

I see some overlap here with my _buf changes


--
To view, visit https://gerrit.osmocom.org/13573
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: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:41:54 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add osmo_str_startswith()

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13394 )

Change subject: add osmo_str_startswith()
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13394
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: Ib2ffb0e9a870dd52e081c7e66d8818057d159513
Gerrit-Change-Number: 13394
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:39:55 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add osmo_str_startswith()

2019-04-10 Thread Harald Welte
Harald Welte has removed a vote on this change.

Change subject: add osmo_str_startswith()
..


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib2ffb0e9a870dd52e081c7e66d8818057d159513
Gerrit-Change-Number: 13394
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max 
Gerrit-CC: Harald Welte 


Change in libosmocore[master]: make osmo_sockaddr_str_is_set() NULL-safe

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13572 )

Change subject: make osmo_sockaddr_str_is_set() NULL-safe
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13572
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: Iac025cf4d556cbed99f3924cd9ca05a05881cd9a
Gerrit-Change-Number: 13572
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:39:45 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: fsm: support graceful osmo_fsm_inst_term() cascades

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13392 )

Change subject: fsm: support graceful osmo_fsm_inst_term() cascades
..


Patch Set 8: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13392
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: I8eda67540a1cd91beb7856b9fcd0a3143b18
Gerrit-Change-Number: 13392
Gerrit-PatchSet: 8
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:39:20 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: smpp: Make libsmpp34 use talloc for its allocations

2019-04-10 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13562 )

Change subject: smpp: Make libsmpp34 use talloc for its allocations
..


Patch Set 2:

Indeed, I would be happy if there were no changes made to my original change, 
including the API naming.  The naming follows 1:1 how the interface of 
libsofia-sip works, and we provide the talloc wrappers for it in 
osmo-sip-connector. @vadim: please revert.


--
To view, visit https://gerrit.osmocom.org/13562
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: Ie2725ffab6a225813e65768735f01678e2022128
Gerrit-Change-Number: 13562
Gerrit-PatchSet: 2
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Vadim Yanitskiy 
Gerrit-Comment-Date: Wed, 10 Apr 2019 22:32:06 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13006 )

Change subject: hlr.c: forward GSUP messages between clients
..


Set Ready For Review


--
To view, visit https://gerrit.osmocom.org/13006
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: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 7
Gerrit-Owner: osmith 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: osmith 
Gerrit-CC: Vadim Yanitskiy 
Gerrit-Comment-Date: Wed, 10 Apr 2019 18:12:59 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-msc[master]: sms queue: avoid repeated Paging for a failed SMS

2019-04-10 Thread Neels Hofmeyr
Hello Vadim Yanitskiy, Keith Whyte, Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/13173

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

Change subject: sms queue: avoid repeated Paging for a failed SMS
..

sms queue: avoid repeated Paging for a failed SMS

A Paging might be attempted multiple times by the MSC, but if the MSC's paging
code decides that Paging failed, then that is that and pending requests should
end in failure. Make the SMS code heed that.

So far, sms_pending_failed() starts a new sms_queue_trigger() run. The
intention behind that might have been to fill up the queue when sending SMS has
failed, but the practical effect is actually bad:

As current ttcn3-msc-test runs show, a failed MT SMS gets triggered multiple
times in short succession, i.e. osmo-msc repeatedly sends Paging Requests for
the same subscriber. This is not because Paging decides that it might succeed
if we Page again -- this is a Paging failure being dispatched to the SMS code,
which then in turn re-launches Paging from scratch.

This special case happens actually only when there are few SMS still in the DB
to be delivered. In the TTCN3 test, there is exactly one MT SMS for one
subscriber, and retriggering the queue brings up the same SMS every time.

See f_tc_lu_and_mt_sms_paging_and_nothing() and f_tc_sgsap_mt_sms_and_nothing()
which say:
"/* Expect the MSC to page exactly 10 times before giving up */"

Fix: do not immediately trigger the SMS queue on a failed MT SMS. Instead,
leave it up to the periodical SMS queue trigger to decide.

This patch will cause the MT SMS tests in ttcn3-msc-tests to fail, because the
test expectations are bogus. The patch fixing the test run is listed 'Related'
below.

Related: I7dce12942a65eaaf97f78ca69401c7f93faacb9e (osmo-ttcn3-hacks)
Change-Id: I24bf9f1c1167efe1080ae4cf47ed2ef0bd981e49
---
M src/libmsc/sms_queue.c
1 file changed, 0 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/73/13173/4
--
To view, visit https://gerrit.osmocom.org/13173
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: I24bf9f1c1167efe1080ae4cf47ed2ef0bd981e49
Gerrit-Change-Number: 13173
Gerrit-PatchSet: 4
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Keith Whyte 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Vadim Yanitskiy 


Change in osmo-msc[master]: gsm_04_08_cc: improve logging for CC trans

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded a new patch set (#2). ( 
https://gerrit.osmocom.org/13593 )

Change subject: gsm_04_08_cc: improve logging for CC trans
..

gsm_04_08_cc: improve logging for CC trans

Pass trans around more functions as log context.
Add missing "rx" logging for two cases.

Change-Id: If79f724a2faca70023271398c618cfe490fb294e
---
M src/libmsc/gsm_04_08_cc.c
1 file changed, 12 insertions(+), 9 deletions(-)


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


Change in osmo-msc[master]: vlr_subscr: use osmo_use_count

2019-04-10 Thread Neels Hofmeyr
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/13136

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

Change subject: vlr_subscr: use osmo_use_count
..

vlr_subscr: use osmo_use_count

Depends: Ife31e6798b4e728a23913179e346552a7dd338c0 (libosmocore)
Change-Id: Ib06d030e8464abe415ff597d462ed40eeddef475
---
M include/osmocom/msc/db.h
M include/osmocom/msc/ran_conn.h
M include/osmocom/msc/sms_queue.h
M include/osmocom/msc/vlr.h
M include/osmocom/msc/vlr_sgs.h
M src/libmsc/ctrl_commands.c
M src/libmsc/db.c
M src/libmsc/gsm_04_08.c
M src/libmsc/gsm_04_08_cc.c
M src/libmsc/gsm_04_11.c
M src/libmsc/gsm_subscriber.c
M src/libmsc/msc_vty.c
M src/libmsc/ran_conn.c
M src/libmsc/sgs_iface.c
M src/libmsc/smpp_openbsc.c
M src/libmsc/sms_queue.c
M src/libmsc/transaction.c
M src/libvlr/vlr.c
M src/libvlr/vlr_access_req_fsm.c
M src/libvlr/vlr_lu_fsm.c
M src/libvlr/vlr_sgs.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_gsm_authen.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_gsm_ciph.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.c
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_hlr_timeout.err
M tests/msc_vlr/msc_vlr_test_ms_timeout.c
M tests/msc_vlr/msc_vlr_test_ms_timeout.err
M tests/msc_vlr/msc_vlr_test_no_authen.c
M tests/msc_vlr/msc_vlr_test_no_authen.err
M tests/msc_vlr/msc_vlr_test_reject_concurrency.c
M tests/msc_vlr/msc_vlr_test_reject_concurrency.err
M tests/msc_vlr/msc_vlr_test_rest.c
M tests/msc_vlr/msc_vlr_test_rest.err
M tests/msc_vlr/msc_vlr_test_ss.c
M tests/msc_vlr/msc_vlr_test_ss.err
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
M tests/sms_queue/sms_queue_test.c
45 files changed, 2,350 insertions(+), 2,287 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/36/13136/4
--
To view, visit https://gerrit.osmocom.org/13136
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: Ib06d030e8464abe415ff597d462ed40eeddef475
Gerrit-Change-Number: 13136
Gerrit-PatchSet: 4
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-CC: Max 


Change in osmo-msc[master]: enable osmo_fsm_term_safely(), apply logging changes

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13592


Change subject: enable osmo_fsm_term_safely(), apply logging changes
..

enable osmo_fsm_term_safely(), apply logging changes

Start using osmo_fsm_term_safely(true), the recently added feature of
libosmocore's fsm.c. Deallocates in slightly changed order and with slightly
modified logging. Adjust test expectations.

Depends: I8eda67540a1cd91beb7856b9fcd0a3143b18 (libosmocore)
Change-Id: I195a719d9ec1f6764ee5a361244f59f0144dc253
---
M src/osmo-msc/msc_main.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_hlr_timeout.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
M tests/msc_vlr/msc_vlr_tests.c
14 files changed, 627 insertions(+), 1,140 deletions(-)



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


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


Change in osmo-msc[master]: gsm_04_08_cc: improve logging for CC trans

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13593


Change subject: gsm_04_08_cc: improve logging for CC trans
..

gsm_04_08_cc: improve logging for CC trans

Pass trans around more functions as log context.
Add missing "rx" logging for two cases.

Change-Id: If79f724a2faca70023271398c618cfe490fb294e
---
M src/libmsc/gsm_04_08_cc.c
1 file changed, 12 insertions(+), 9 deletions(-)



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

diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index 4475d05..62b5d12 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -1601,7 +1601,7 @@
return mncc_recvmsg(trans->net, trans, MNCC_USERINFO_IND, );
 }

-static void mncc_recv_rtp(struct gsm_network *net, uint32_t callref,
+static void mncc_recv_rtp(struct gsm_network *net, struct gsm_trans *trans, 
uint32_t callref,
int cmd, uint32_t addr, uint16_t port, uint32_t payload_type,
uint32_t payload_msg_type)
 {
@@ -1617,7 +1617,7 @@
rtp->port = port;
rtp->payload_type = payload_type;
rtp->payload_msg_type = payload_msg_type;
-   mncc_recvmsg(net, NULL, cmd, (struct gsm_mncc *)data);
+   mncc_recvmsg(net, trans, cmd, (struct gsm_mncc *)data);
 }
 
 static void mncc_recv_rtp_sock(struct gsm_network *net, struct gsm_trans 
*trans, int cmd)
@@ -1652,16 +1652,16 @@
 * lchan->abis_ip.rtp_payload */
uint32_t payload_type = 0;

-   return mncc_recv_rtp(net, trans->callref, cmd,
+   return mncc_recv_rtp(net, trans, trans->callref, cmd,
addr,
port,
payload_type,
msg_type);
 }

-static void mncc_recv_rtp_err(struct gsm_network *net, uint32_t callref, int 
cmd)
+static void mncc_recv_rtp_err(struct gsm_network *net, struct gsm_trans 
*trans, uint32_t callref, int cmd)
 {
-   return mncc_recv_rtp(net, callref, cmd, 0, 0, 0, 0);
+   return mncc_recv_rtp(net, trans, callref, cmd, 0, 0, 0, 0);
 }

 static int tch_rtp_create(struct gsm_network *net, uint32_t callref)
@@ -1672,15 +1672,16 @@
trans = trans_find_by_callref(net, callref);
if (!trans) {
LOG_TRANS_CAT(trans, DMNCC, LOGL_ERROR, "RTP create for 
non-existing trans\n");
-   mncc_recv_rtp_err(net, callref, MNCC_RTP_CREATE);
+   mncc_recv_rtp_err(net, trans, callref, MNCC_RTP_CREATE);
return -EIO;
}
log_set_context(LOG_CTX_VLR_SUBSCR, trans->vsub);
if (!trans->conn) {
LOG_TRANS_CAT(trans, DMNCC, LOGL_NOTICE, "RTP create for trans 
without conn\n");
-   mncc_recv_rtp_err(net, callref, MNCC_RTP_CREATE);
+   mncc_recv_rtp_err(net, trans, callref, MNCC_RTP_CREATE);
return 0;
}
+   LOG_TRANS_CAT(trans, DMNCC, LOGL_DEBUG, "rx %s\n", 
get_mncc_name(MNCC_RTP_CREATE));

/* When we call msc_mgcp_call_assignment() we will trigger, depending
 * on the RAN type the call assignment on the A or Iu interface.
@@ -1733,16 +1734,18 @@
trans = trans_find_by_callref(net, rtp->callref);
if (!trans) {
LOG_TRANS_CAT(trans, DMNCC, LOGL_ERROR, "RTP connect for 
non-existing trans\n");
-   mncc_recv_rtp_err(net, rtp->callref, MNCC_RTP_CONNECT);
+   mncc_recv_rtp_err(net, trans, rtp->callref, MNCC_RTP_CONNECT);
return -EIO;
}
log_set_context(LOG_CTX_VLR_SUBSCR, trans->vsub);
if (!trans->conn) {
LOG_TRANS_CAT(trans, DMNCC, LOGL_ERROR, "RTP connect for trans 
without conn\n");
-   mncc_recv_rtp_err(net, rtp->callref, MNCC_RTP_CONNECT);
+   mncc_recv_rtp_err(net, trans, rtp->callref, MNCC_RTP_CONNECT);
return 0;
}

+   LOG_TRANS_CAT(trans, DMNCC, LOGL_DEBUG, "rx %s\n", 
get_mncc_name(MNCC_RTP_CONNECT));
+
addr.s_addr = osmo_htonl(rtp->ip);
return msc_mgcp_call_complete(trans, rtp->port, inet_ntoa(addr));
 }

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


Change in osmo-msc[master]: vlr_subscr: use osmo_use_count

2019-04-10 Thread Neels Hofmeyr
Hello Jenkins Builder,

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

https://gerrit.osmocom.org/13136

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

Change subject: vlr_subscr: use osmo_use_count
..

vlr_subscr: use osmo_use_count

Depends: Ife31e6798b4e728a23913179e346552a7dd338c0 (libosmocore)
Change-Id: Ib06d030e8464abe415ff597d462ed40eeddef475
---
M include/osmocom/msc/db.h
M include/osmocom/msc/ran_conn.h
M include/osmocom/msc/sms_queue.h
M include/osmocom/msc/vlr.h
M include/osmocom/msc/vlr_sgs.h
M src/libmsc/ctrl_commands.c
M src/libmsc/db.c
M src/libmsc/gsm_04_08.c
M src/libmsc/gsm_04_08_cc.c
M src/libmsc/gsm_04_11.c
M src/libmsc/gsm_subscriber.c
M src/libmsc/msc_vty.c
M src/libmsc/ran_conn.c
M src/libmsc/sgs_iface.c
M src/libmsc/smpp_openbsc.c
M src/libmsc/sms_queue.c
M src/libmsc/transaction.c
M src/libvlr/vlr.c
M src/libvlr/vlr_access_req_fsm.c
M src/libvlr/vlr_lu_fsm.c
M src/libvlr/vlr_sgs.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_gsm_authen.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_gsm_ciph.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.c
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_hlr_timeout.err
M tests/msc_vlr/msc_vlr_test_ms_timeout.c
M tests/msc_vlr/msc_vlr_test_ms_timeout.err
M tests/msc_vlr/msc_vlr_test_no_authen.c
M tests/msc_vlr/msc_vlr_test_no_authen.err
M tests/msc_vlr/msc_vlr_test_reject_concurrency.c
M tests/msc_vlr/msc_vlr_test_reject_concurrency.err
M tests/msc_vlr/msc_vlr_test_rest.c
M tests/msc_vlr/msc_vlr_test_rest.err
M tests/msc_vlr/msc_vlr_test_ss.c
M tests/msc_vlr/msc_vlr_test_ss.err
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
M tests/sms_queue/sms_queue_test.c
45 files changed, 2,350 insertions(+), 2,287 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/36/13136/3
--
To view, visit https://gerrit.osmocom.org/13136
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: Ib06d030e8464abe415ff597d462ed40eeddef475
Gerrit-Change-Number: 13136
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-CC: Max 


Change in osmo-mgw[master]: fix: multiple initial CRCX

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13591


Change subject: fix: multiple initial CRCX
..

fix: multiple initial CRCX

The first CRCX responds with the actual MGW endpoint name that is assigned (at
least for rtpbridge/*@mgw requests).

If multiple CRCX are scheduled at the same time on a fresh MGW endpoint, both
get fired with a '*' and each creates a separate MGW endpoint.

Make mgcp_client_endpoint_fsm avoid this, and schedule only one CRCX at first,
and the rest once the MGW endpoint name is fixated. It is thus possible to
safely issue two osmo_mgcpc_ep_ci_request() at the same time.

Change-Id: I92a9944acc96398acd6649f9c3c5badec5dd6dcc
---
M src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
1 file changed, 86 insertions(+), 11 deletions(-)



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

diff --git a/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c 
b/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
index a50491f..e8ca527 100644
--- a/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
+++ b/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
@@ -101,6 +101,7 @@
struct osmo_fsm_inst *fi;
char endpoint[MGCP_ENDPOINT_MAXLEN];
const struct osmo_tdef *T_defs;
+   bool first_crcx_complete;

struct osmo_mgcpc_ep_ci ci[USABLE_CI];
 };
@@ -114,6 +115,9 @@
{}
 };

+static void osmo_mgcpc_ep_count(struct osmo_mgcpc_ep *ep, int *occupied, int 
*pending_not_sent,
+   int *waiting_for_response);
+
 static struct osmo_mgcpc_ep_ci *osmo_mgcpc_ep_check_ci(struct osmo_mgcpc_ep_ci 
*ci)
 {
if (!ci)
@@ -323,6 +327,49 @@
osmo_fsm_inst_dispatch(notify, notify_failure, notify_data);
 }

+static int update_endpoint_name(struct osmo_mgcpc_ep_ci *ci, const char 
*new_endpoint_name)
+{
+   struct osmo_mgcpc_ep *ep = ci->ep;
+   int rc;
+   int i;
+
+   if (!strcmp(ep->endpoint, new_endpoint_name)) {
+   /* Same endpoint name, nothing to do. */
+   return 0;
+   }
+
+   /* The endpoint name should only change on the very first CRCX 
response. */
+   if (ep->first_crcx_complete) {
+   LOG_CI(ci, LOGL_ERROR, "Reponse returned mismatching endpoint 
name."
+  " This is endpoint %s, instead received %s\n",
+  ep->endpoint, new_endpoint_name);
+   on_failure(ci);
+   return -EINVAL;
+   }
+
+   /* This is the first CRCX response, update endpoint name. */
+   rc = OSMO_STRLCPY_ARRAY(ep->endpoint, new_endpoint_name);
+   if (rc <= 0 || rc >= sizeof(ep->endpoint)) {
+   LOG_CI(ci, LOGL_ERROR, "Unable to copy endpoint name %s\n", 
osmo_quote_str(new_endpoint_name, -1));
+   osmo_mgcpc_ep_ci_dlcx(ci);
+   on_failure(ci);
+   return -ENOSPC;
+   }
+
+   /* Make sure already pending requests use this updated endpoint name. */
+   for (i = 0; i < ARRAY_SIZE(ep->ci); i++) {
+   struct osmo_mgcpc_ep_ci *other_ci = >ci[i];
+   if (!other_ci->occupied)
+   continue;
+   if (!other_ci->pending)
+   continue;
+   if (other_ci->sent)
+   continue;
+   OSMO_STRLCPY_ARRAY(other_ci->verb_info.endpoint, ep->endpoint);
+   }
+   return 0;
+}
+
 static void on_success(struct osmo_mgcpc_ep_ci *ci, void *data)
 {
struct mgcp_conn_peer *rtp_info;
@@ -343,17 +390,12 @@
osmo_strlcpy(ci->mgcp_ci_str, 
mgcp_conn_get_ci(ci->mgcp_client_fi),
sizeof(ci->mgcp_ci_str));
if (rtp_info->endpoint[0]) {
-   int rc;
-   rc = osmo_strlcpy(ci->ep->endpoint, rtp_info->endpoint,
- sizeof(ci->ep->endpoint));
-   if (rc <= 0 || rc >= sizeof(ci->ep->endpoint)) {
-   LOG_CI(ci, LOGL_ERROR, "Unable to copy endpoint 
name '%s'\n",
-  rtp_info->endpoint);
-   osmo_mgcpc_ep_ci_dlcx(ci);
-   on_failure(ci);
+   /* On errors, this instance might already be 
deallocated. Make sure to not access anything after
+* error. */
+   if (update_endpoint_name(ci, rtp_info->endpoint))
return;
-   }
}
+   ci->ep->first_crcx_complete = true;
break;

default:
@@ -537,6 +579,7 @@
if (!ci->mgcp_client_fi){
LOG_CI_VERB(ci, LOGL_ERROR, "Cannot send\n");
on_failure(ci);
+   return -EINVAL;
}
 

Change in osmo-mgw[master]: move MGW endpoint FSM from osmo-bsc to here

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13590


Change subject: move MGW endpoint FSM from osmo-bsc to here
..

move MGW endpoint FSM from osmo-bsc to here

Move mgw_endpoint_fsm from osmo-bsc here as osmo_mgcpc_ep_fsm. Apply various
renames for consistency. Use osmo_tdef from libosmocore instead of osmo-bsc's
(so far) local T_defs API.

Change T23042 to T2427001, which is a slightly less arbitrary number and
slightly more extendable in the future (2427 corresponds to the default MGCP
port at osmo-mgw, 001 is the first MGCP timer and so far the only one).

Change-Id: I9a3effd38e72841529df6c135c077116981dea36
---
M include/Makefile.am
A include/osmocom/mgcp_client/mgcp_client_endpoint_fsm.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M src/libosmo-mgcp-client/Makefile.am
A src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
M src/libosmo-mgcp-client/mgcp_client_fsm.c
6 files changed, 810 insertions(+), 0 deletions(-)



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

diff --git a/include/Makefile.am b/include/Makefile.am
index c34553a..2daaf20 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -4,6 +4,7 @@

 nobase_include_HEADERS = \
osmocom/mgcp_client/mgcp_client.h \
+   osmocom/mgcp_client/mgcp_client_endpoint_fsm.h \
osmocom/mgcp_client/mgcp_client_fsm.h \
osmocom/mgcp_client/mgcp_common.h \
osmocom/mgcp/mgcp.h \
diff --git a/include/osmocom/mgcp_client/mgcp_client_endpoint_fsm.h 
b/include/osmocom/mgcp_client/mgcp_client_endpoint_fsm.h
new file mode 100644
index 000..a2297b5
--- /dev/null
+++ b/include/osmocom/mgcp_client/mgcp_client_endpoint_fsm.h
@@ -0,0 +1,46 @@
+/* FSM to manage multiple connections of an MGW endpoint */
+#pragma once
+
+#include 
+
+#define LOG_MGCPC_EP(ep, level, fmt, args...) do { \
+   LOGPFSML(ep->fi, level, "%s " fmt, \
+osmo_mgcpc_ep_name(ep), ## args); \
+   } while(0)
+
+struct osmo_mgcpc_ep;
+struct osmo_mgcpc_ep_ci;
+struct osmo_tdef;
+
+void osmo_mgcpc_ep_fsm_init();
+
+struct osmo_mgcpc_ep *osmo_mgcpc_ep_alloc(struct osmo_fsm_inst *parent, 
uint32_t parent_term_event,
+ struct mgcp_client *mgcp_client,
+ const struct osmo_tdef *T_defs,
+ const char *fsm_id,
+ const char *endpoint_str_fmt, ...);
+
+struct osmo_mgcpc_ep_ci *osmo_mgcpc_ep_ci_add(struct osmo_mgcpc_ep *ep, const 
char *label_fmt, ...);
+const struct mgcp_conn_peer *osmo_mgcpc_ep_ci_get_rtp_info(const struct 
osmo_mgcpc_ep_ci *ci);
+bool osmo_mgcpc_ep_ci_get_crcx_info_to_sockaddr(const struct osmo_mgcpc_ep_ci 
*ci, struct sockaddr_storage *dest);
+
+void osmo_mgcpc_ep_ci_request(struct osmo_mgcpc_ep_ci *ci,
+ enum mgcp_verb verb, const struct mgcp_conn_peer 
*verb_info,
+ struct osmo_fsm_inst *notify,
+ uint32_t event_success, uint32_t event_failure,
+ void *notify_data);
+
+static inline void osmo_mgcpc_ep_ci_dlcx(struct osmo_mgcpc_ep_ci *ci)
+{
+   osmo_mgcpc_ep_ci_request(ci, MGCP_VERB_DLCX, NULL, NULL, 0, 0, NULL);
+}
+
+void osmo_mgcpc_ep_clear(struct osmo_mgcpc_ep *ep);
+
+const char *osmo_mgcpc_ep_name(const struct osmo_mgcpc_ep *ep);
+const char *osmo_mgcpc_ep_ci_name(const struct osmo_mgcpc_ep_ci *ci);
+const char *osmo_mgcpc_ep_ci_id(const struct osmo_mgcpc_ep_ci *ci);
+
+extern const struct value_string osmo_mgcp_verb_names[];
+static inline const char *osmo_mgcp_verb_name(enum mgcp_verb val)
+{ return get_value_string(osmo_mgcp_verb_names, val); }
diff --git a/include/osmocom/mgcp_client/mgcp_client_fsm.h 
b/include/osmocom/mgcp_client/mgcp_client_fsm.h
index dabfcca..e170a25 100644
--- a/include/osmocom/mgcp_client/mgcp_client_fsm.h
+++ b/include/osmocom/mgcp_client/mgcp_client_fsm.h
@@ -64,3 +64,5 @@
 void mgcp_conn_delete(struct osmo_fsm_inst *fi);

 const char *mgcp_conn_get_ci(struct osmo_fsm_inst *fi);
+
+const char *osmo_mgcpc_conn_peer_name(const struct mgcp_conn_peer *info);
diff --git a/src/libosmo-mgcp-client/Makefile.am 
b/src/libosmo-mgcp-client/Makefile.am
index e59da2f..1529853 100644
--- a/src/libosmo-mgcp-client/Makefile.am
+++ b/src/libosmo-mgcp-client/Makefile.am
@@ -30,6 +30,7 @@
mgcp_client.c \
mgcp_client_vty.c \
mgcp_client_fsm.c \
+   mgcp_client_endpoint_fsm.c \
$(NULL)

 libosmo_mgcp_client_la_LDFLAGS = $(AM_LDFLAGS) -version-info 
$(MGCP_CLIENT_LIBVERSION)
diff --git a/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c 
b/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
new file mode 100644
index 000..a50491f
--- /dev/null
+++ b/src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c
@@ -0,0 +1,738 @@
+/* FSM to manage multiple connections of an MGW 

Change in osmo-hlr[master]: osmo-hlr: allow configuring db path from cfg file

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13586


Change subject: osmo-hlr: allow configuring db path from cfg file
..

osmo-hlr: allow configuring db path from cfg file

So far, the cmdline argument was the only way to set a database config file.
Add a similar config to VTY as 'hlr' / 'database'. The cmdline arg is stronger
than the 'database' cfg item. DB is not reloaded from VTY command.

Change-Id: I87b8673324e1e6225afb758fb4963ff3279ea3d8
---
M src/hlr.c
M src/hlr.h
M src/hlr_vty.c
3 files changed, 23 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/86/13586/1

diff --git a/src/hlr.c b/src/hlr.c
index ce8c388..836d57b 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -621,7 +621,7 @@
bool db_upgrade;
 } cmdline_opts = {
.config_file = "osmo-hlr.cfg",
-   .db_file = "hlr.db",
+   .db_file = NULL,
.daemonize = false,
.db_upgrade = false,
 };
@@ -736,6 +736,7 @@
INIT_LLIST_HEAD(_hlr->iuse_list);
INIT_LLIST_HEAD(_hlr->ss_sessions);
INIT_LLIST_HEAD(_hlr->ussd_routes);
+   osmo_talloc_replace_string(g_hlr, _hlr->db_file_path, 
HLR_DEFAULT_DB_FILE_PATH);

/* Init default (call independent) SS session guard timeout value */
g_hlr->ncss_guard_timeout = NCSS_GUARD_TIMEOUT_DEFAULT;
@@ -774,9 +775,12 @@
exit(1);
}

-   g_hlr->dbc = db_open(hlr_ctx, cmdline_opts.db_file, true, 
cmdline_opts.db_upgrade);
+   if (cmdline_opts.db_file)
+   osmo_talloc_replace_string(g_hlr, _hlr->db_file_path, 
cmdline_opts.db_file);
+
+   g_hlr->dbc = db_open(hlr_ctx, g_hlr->db_file_path, true, 
cmdline_opts.db_upgrade);
if (!g_hlr->dbc) {
-   LOGP(DMAIN, LOGL_FATAL, "Error opening database\n");
+   LOGP(DMAIN, LOGL_FATAL, "Error opening database %s\n", 
osmo_quote_str(g_hlr->db_file_path, -1));
exit(1);
}

diff --git a/src/hlr.h b/src/hlr.h
index 00fa43c..e2e96a4 100644
--- a/src/hlr.h
+++ b/src/hlr.h
@@ -25,6 +25,8 @@
 #include 
 #include 

+#define HLR_DEFAULT_DB_FILE_PATH "hlr.db"
+
 struct hlr_euse;

 struct hlr {
@@ -32,6 +34,7 @@
struct osmo_gsup_server *gs;

/* DB context */
+   char *db_file_path;
struct db_context *dbc;

/* Control Interface */
diff --git a/src/hlr_vty.c b/src/hlr_vty.c
index d0a623a..e4cc4be 100644
--- a/src/hlr_vty.c
+++ b/src/hlr_vty.c
@@ -74,6 +74,8 @@
vty_out(vty, "hlr%s", VTY_NEWLINE);
if (g_hlr->store_imei)
vty_out(vty, " store-imei%s", VTY_NEWLINE);
+   if (g_hlr->db_file_path && strcmp(g_hlr->db_file_path, 
HLR_DEFAULT_DB_FILE_PATH))
+   vty_out(vty, " database %s%s", g_hlr->db_file_path, 
VTY_NEWLINE);
return CMD_SUCCESS;
 }

@@ -224,6 +226,15 @@
return CMD_SUCCESS;
 }

+DEFUN(cfg_database, cfg_database_cmd,
+   "database PATH",
+   "Set the path to the HLR database file\n"
+   "Relative or absolute file system path to the database file (default is 
'" HLR_DEFAULT_DB_FILE_PATH "')\n")
+{
+   osmo_talloc_replace_string(g_hlr, _hlr->db_file_path, argv[0]);
+   return CMD_SUCCESS;
+}
+
 struct cmd_node euse_node = {
EUSE_NODE,
"%s(config-hlr-euse)# ",
@@ -380,6 +391,8 @@

install_element(GSUP_NODE, _hlr_gsup_bind_ip_cmd);

+   install_element(HLR_NODE, _database_cmd);
+
install_element(HLR_NODE, _euse_cmd);
install_element(HLR_NODE, _no_euse_cmd);
install_node(_node, config_write_euse);

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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I87b8673324e1e6225afb758fb4963ff3279ea3d8
Gerrit-Change-Number: 13586
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 


Change in osmo-hlr[master]: add missing error log: invalid IMSI

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13587


Change subject: add missing error log: invalid IMSI
..

add missing error log: invalid IMSI

Change-Id: I65e9ecac06dc6d1abb9802d621c385d3b4fab83a
---
M src/hlr.c
1 file changed, 3 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/87/13587/1

diff --git a/src/hlr.c b/src/hlr.c
index 836d57b..2614f35 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -533,8 +533,10 @@

/* 3GPP TS 23.003 Section 2.2 clearly states that an IMSI with less 
than 5
 * digits is impossible.  Even 5 digits is a highly theoretical case */
-   if (strlen(gsup.imsi) < 5)
+   if (strlen(gsup.imsi) < 5) {
+   LOGP(DMAIN, LOGL_ERROR, "IMSI too short: %s\n", 
osmo_quote_str(gsup.imsi, -1));
return gsup_send_err_reply(conn, gsup.imsi, gsup.message_type, 
GMM_CAUSE_INV_MAND_INFO);
+   }

if (gsup.destination_name_len)
return read_cb_forward(conn, msg, );

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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I65e9ecac06dc6d1abb9802d621c385d3b4fab83a
Gerrit-Change-Number: 13587
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 


Change in osmo-hlr[master]: GSUP routing: use Kind IE

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13588


Change subject: GSUP routing: use Kind IE
..

GSUP routing: use Kind IE

Include the GSUP Kind from original message in routing error responses.
Add the Kind to GSUP router logging.

Depends: Ic397a9f2c4a7224e47cab944c72e75ca5592efef (libosmocore)
Change-Id: I8dc3967d9372d63e9d57ca2608dd3316edb234a4
---
M src/hlr.c
1 file changed, 17 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/88/13588/1

diff --git a/src/hlr.c b/src/hlr.c
index 2614f35..6d2f7ac 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -453,6 +453,7 @@
/* Prepare error message (before IEs get deallocated) */
gsup_err = talloc_zero(hlr_ctx, struct osmo_gsup_message);
OSMO_STRLCPY_ARRAY(gsup_err->imsi, gsup->imsi);
+   gsup_err->kind = gsup->kind;
gsup_err->destination_name = talloc_memdup(gsup_err, 
gsup->destination_name, gsup->destination_name_len);
gsup_err->destination_name_len = gsup->destination_name_len;
gsup_err->message_type = OSMO_GSUP_MSGT_E_ROUTING_ERROR;
@@ -462,8 +463,9 @@

/* Check for routing IEs */
if (!gsup->source_name || !gsup->source_name_len || 
!gsup->destination_name || !gsup->destination_name_len) {
-   LOGP(DMAIN, LOGL_ERROR, "Can't forward GSUP message for IMSI %s 
(%s): missing routing IEs\n",
-gsup->imsi, 
osmo_gsup_message_type_name(gsup->message_type));
+   LOGP(DMAIN, LOGL_ERROR, "Can't forward GSUP message for IMSI %s 
(%s, kind %s): missing routing IEs\n",
+gsup->imsi, 
osmo_gsup_message_type_name(gsup->message_type),
+osmo_gsup_kind_name(gsup->kind));
goto end;
}

@@ -480,10 +482,11 @@
}

/* Forward message without re-encoding (so we don't remove unknown IEs) 
*/
-   LOGP(DMAIN, LOGL_NOTICE, "Forwarding GSUP message for IMSI %s from %s 
to %s: %s\n", gsup->imsi,
+   LOGP(DMAIN, LOGL_NOTICE, "Forwarding GSUP message for IMSI %s from %s 
to %s: %s, kind %s\n", gsup->imsi,
 osmo_quote_str_buf((const char *)gsup->source_name, 
gsup->source_name_len, buf, sizeof(buf)),
 osmo_quote_str((const char *)gsup->destination_name, 
gsup->destination_name_len),
-osmo_gsup_message_type_name(gsup->message_type));
+osmo_gsup_message_type_name(gsup->message_type),
+osmo_gsup_kind_name(gsup->kind));

/* Remove incoming IPA header to be able to prepend and outgoing IPA 
header */
msgb_pull_to_l2(msg);
@@ -494,16 +497,19 @@
gsup = NULL;
if (ret == -ENODEV)
LOGP(DMAIN, LOGL_ERROR,
-"Can't forward GSUP message for IMSI %s from %s to %s 
(%s): destination not connected\n", gsup_err->imsi,
-osmo_quote_str_buf((const char *)gsup_err->source_name, 
gsup_err->source_name_len, buf, sizeof(buf)),
-osmo_quote_str((const char *)gsup_err->destination_name, 
gsup_err->destination_name_len),
-osmo_gsup_message_type_name(gsup_err->message_type));
-   else if (ret)
-   LOGP(DMAIN, LOGL_ERROR, "Can't forward GSUP message for IMSI %s 
from %s to %s (%s): unknown error\n",
+"Can't forward GSUP message for IMSI %s from %s to %s (%s, 
kind %s): destination not connected\n",
 gsup_err->imsi,
 osmo_quote_str_buf((const char *)gsup_err->source_name, 
gsup_err->source_name_len, buf, sizeof(buf)),
 osmo_quote_str((const char *)gsup_err->destination_name, 
gsup_err->destination_name_len),
-osmo_gsup_message_type_name(gsup_err->message_type));
+osmo_gsup_message_type_name(gsup_err->message_type),
+osmo_gsup_kind_name(gsup_err->kind));
+   else if (ret)
+   LOGP(DMAIN, LOGL_ERROR, "Can't forward GSUP message for IMSI %s 
from %s to %s (%s, kind %s): unknown error\n",
+gsup_err->imsi,
+osmo_quote_str_buf((const char *)gsup_err->source_name, 
gsup_err->source_name_len, buf, sizeof(buf)),
+osmo_quote_str((const char *)gsup_err->destination_name, 
gsup_err->destination_name_len),
+osmo_gsup_message_type_name(gsup_err->message_type),
+osmo_gsup_kind_name(gsup_err->kind));

 end:
/* Send error back to source */

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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8dc3967d9372d63e9d57ca2608dd3316edb234a4
Gerrit-Change-Number: 13588
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 


Change in osmo-hlr[master]: use new OSMO_IMSI_SIZE

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13589


Change subject: use new OSMO_IMSI_SIZE
..

use new OSMO_IMSI_SIZE

Depends: Id11ada4c96b79f7f0ad58185ab7dbf24622fb770 (libosmocore)
Change-Id: I8e8fa221e97303df3c6cce96b25d31a53f67b939
---
M src/hlr_ussd.c
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/89/13589/1

diff --git a/src/hlr_ussd.c b/src/hlr_ussd.c
index 56a5a95..3c55c42 100644
--- a/src/hlr_ussd.c
+++ b/src/hlr_ussd.c
@@ -150,7 +150,7 @@
/* link us to hlr->ss_sessions */
struct llist_head list;
/* imsi of this session */
-   char imsi[GSM23003_IMSI_MAX_DIGITS+2];
+   char imsi[OSMO_IMSI_SIZE];
/* ID of this session (unique per IMSI) */
uint32_t session_id;
/* state of the session */

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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e8fa221e97303df3c6cce96b25d31a53f67b939
Gerrit-Change-Number: 13589
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 


Change in osmo-bsc[master]: fix inter-BSC-HO-incoming for AoIP (2/2)

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13585


Change subject: fix inter-BSC-HO-incoming for AoIP (2/2)
..

fix inter-BSC-HO-incoming for AoIP (2/2)

For AoIP, the AoIP Transport Layer Address IE must be included in the Handover
Request Acknowledge message, so the MSC can send RTP to the right place. Add
this IE for AoIP.

Depends: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576 (libosmocore)
Depends: Id617265337f09dfb6ddfe111ef5e578cd3dc9f63 (libosmocore)
Change-Id: Ia05e37da125eb6e7b7be9b974b73261bd72de1f4
---
M src/osmo-bsc/osmo_bsc_bssap.c
1 file changed, 32 insertions(+), 6 deletions(-)



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

diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 65618fd..4849b1d 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -40,8 +40,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 

 #define IP_V4_ADDR_LEN 4

@@ -1185,14 +1187,38 @@
 {
struct msgb *msg;
struct gsm_lchan *new_lchan = conn->ho.new_lchan;
+   struct sockaddr_storage ss;
+   struct gsm0808_handover_request_ack params = {
+   .l3_info = rr_ho_command->data,
+   .l3_info_len = rr_ho_command->len,
+   .chosen_channel_present = true,
+   .chosen_channel = gsm0808_chosen_channel(new_lchan->type, 
new_lchan->tch_mode),
+   .chosen_encr_alg = new_lchan->encr.alg_id,
+   .chosen_speech_version = 
gsm0808_permitted_speech(new_lchan->type, new_lchan->tch_mode),
+   };
+
+   if (gscon_is_aoip(conn)) {
+   struct osmo_sockaddr_str to_msc_rtp;
+   const struct mgcp_conn_peer *rtp_info = 
mgwep_ci_get_rtp_info(conn->user_plane.mgw_endpoint_ci_msc);
+   if (!rtp_info) {
+   LOG_HO(conn, LOGL_ERROR,
+  "Handover Request Acknowledge: no RTP address 
known to send as"
+  " AoIP Transport Layer Address\n");
+   return -EINVAL;
+   }
+   if (osmo_sockaddr_str_from_str(_msc_rtp, rtp_info->addr, 
rtp_info->port)) {
+   LOG_HO(conn, LOGL_ERROR, "Handover Request Acknowledge: 
cannot encode AoIP Transport Layer\n");
+   return -EINVAL;
+   }
+   if (osmo_sockaddr_str_to_sockaddr(_msc_rtp, )) {
+   LOG_HO(conn, LOGL_ERROR, "Handover Request Acknowledge: 
cannot encode AoIP Transport Layer\n");
+   return -EINVAL;
+   }
+   params.aoip_transport_layer = 
+   }

LOG_HO(conn, LOGL_DEBUG, "Sending BSSMAP Handover Request 
Acknowledge\n");
-   msg = gsm0808_create_handover_request_ack(rr_ho_command->data, 
rr_ho_command->len,
- 
gsm0808_chosen_channel(new_lchan->type,
-
new_lchan->tch_mode),
- new_lchan->encr.alg_id,
- 
gsm0808_permitted_speech(new_lchan->type,
-  
new_lchan->tch_mode));
+   msg = gsm0808_create_handover_request_ack2();
msgb_free(rr_ho_command);
if (!msg)
return -ENOMEM;

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia05e37da125eb6e7b7be9b974b73261bd72de1f4
Gerrit-Change-Number: 13585
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 


Change in osmo-bsc[master]: fix inter-BSC-HO-incoming for AoIP (1/2)

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13584


Change subject: fix inter-BSC-HO-incoming for AoIP (1/2)
..

fix inter-BSC-HO-incoming for AoIP (1/2)

Move the HO_ST_WAIT_MGW_ENDPOINT_TO_MSC state up to right after the lchan is
done establishing. For AoIP, the local RTP address towards the MSC already
needs to be known before the Handover Request Acknowledge is sent, so the AoIP
Transport Layer Address IE can be included. This patch only modifies the
handover FSM, a subsequent patch adds the IE.

Change-Id: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff
---
M include/osmocom/bsc/handover_fsm.h
M src/osmo-bsc/handover_fsm.c
2 files changed, 106 insertions(+), 85 deletions(-)



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

diff --git a/include/osmocom/bsc/handover_fsm.h 
b/include/osmocom/bsc/handover_fsm.h
index 4db0890..7c2145e 100644
--- a/include/osmocom/bsc/handover_fsm.h
+++ b/include/osmocom/bsc/handover_fsm.h
@@ -28,10 +28,10 @@
HO_ST_NOT_STARTED,

HO_ST_WAIT_LCHAN_ACTIVE,
+   HO_ST_WAIT_MGW_ENDPOINT_TO_MSC,
HO_ST_WAIT_RR_HO_DETECT,
HO_ST_WAIT_RR_HO_COMPLETE,
HO_ST_WAIT_LCHAN_ESTABLISHED,
-   HO_ST_WAIT_MGW_ENDPOINT_TO_MSC,

/* The inter-BSC Outgoing Handover FSM has completely separate states, 
but since it makes sense for it
 * to also live in conn->ho.fi, it should share the same event enum. 
From there it is merely
@@ -46,11 +46,11 @@
HO_EV_LCHAN_ACTIVE,
HO_EV_LCHAN_ESTABLISHED,
HO_EV_LCHAN_ERROR,
+   HO_EV_MSC_MGW_OK,
+   HO_EV_MSC_MGW_FAIL,
HO_EV_RR_HO_DETECT,
HO_EV_RR_HO_COMPLETE,
HO_EV_RR_HO_FAIL,
-   HO_EV_MSC_MGW_OK,
-   HO_EV_MSC_MGW_FAIL,
HO_EV_CONN_RELEASING,

HO_OUT_EV_BSSMAP_HO_COMMAND,
diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index 421c32e..3b5a660 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -161,10 +161,10 @@

 static const struct state_timeout ho_fsm_timeouts[32] = {
[HO_ST_WAIT_LCHAN_ACTIVE] = { .T = 23042 },
+   [HO_ST_WAIT_MGW_ENDPOINT_TO_MSC] = { .T = 23042 },
[HO_ST_WAIT_RR_HO_DETECT] = { .T = 23042 },
[HO_ST_WAIT_RR_HO_COMPLETE] = { .T = 23042 },
[HO_ST_WAIT_LCHAN_ESTABLISHED] = { .T = 23042 },
-   [HO_ST_WAIT_MGW_ENDPOINT_TO_MSC] = { .T = 23042 },
[HO_OUT_ST_WAIT_HO_COMMAND] = { .T = 7 },
[HO_OUT_ST_WAIT_CLEAR] = { .T = 8 },
 };
@@ -876,10 +876,24 @@
 static void ho_fsm_wait_lchan_active(struct osmo_fsm_inst *fi, uint32_t event, 
void *data)
 {
struct gsm_subscriber_connection *conn = ho_fi_conn(fi);
+   struct handover *ho = >ho;
switch (event) {

case HO_EV_LCHAN_ACTIVE:
-   ho_fsm_state_chg(HO_ST_WAIT_RR_HO_DETECT);
+   /* - If the lchan is voiceless, no need to even think about the 
MGW.
+* - If this is an intra-BSC Handover, we already have an RTP 
stream towards the MSC and aren't
+*   touching it.
+* - If we're on SCCPlite, the MSC manages the MGW endpoint, 
all we do is the BTS side CI, so we can
+*   skip the part that would CRCX towards the MSC.
+* So create an MSC side endpoint CI only if a voice lchan is 
established for an incoming inter-BSC
+* handover on AoIP. Otherwise go on to send a Handover Command 
and wait for the Detect.
+*/
+   if (ho->new_lchan->activate.info.requires_voice_stream
+   && (ho->scope & HO_INTER_BSC_IN)
+   && gscon_is_aoip(conn))
+   ho_fsm_state_chg(HO_ST_WAIT_MGW_ENDPOINT_TO_MSC);
+   else
+   ho_fsm_state_chg(HO_ST_WAIT_RR_HO_DETECT);
return;

case HO_EV_LCHAN_ERROR:
@@ -892,6 +906,76 @@
}
 }

+/* Only for voice, only for inter-BSC Handover into this BSC, and only for 
AoIP:
+ *
+ * Establish the MGW endpoint CI that points towards the MSC. This needs to 
happen after the lchan (lchan_rtp_fsm) has
+ * created an MGW endpoint with the first CRCX, so that an endpoint is 
available, and before sending the Handover
+ * Request Acknowledge, so that the RTP address and port established towards 
the MSC can be included in the Handover
+ * Request Acknowledge message.
+ * (For SCCPlite, the MSC manages the CN side endpoint CI itself, and we don't 
need to send any RTP address in the
+ * Handover Request Acknowledge.)
+ *
+ * Actually, it should be possible to kick this off even above in 
handover_start_inter_bsc_in(), to do the CRCX towards
+ * the MSC at the same time as establishing the lchan. The 
gscon_ensure_mgw_endpoint() doesn't care which one of
+ * lchan_rtp_fsm or handover_start_inter_bsc_in() calls it first. The benefit 
would be that we'd send out the 

Change in osmo-bsc[master]: lchan activation: add explicit encryption info to activation

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13582


Change subject: lchan activation: add explicit encryption info to activation
..

lchan activation: add explicit encryption info to activation

For intra-BSC handover, the previous encryption is copied from the old lchan,
which of course is not available during inter-BSC handover.  Hence the lchan
activation info needs to include an explicit encryption information, and we
must not rely on the presence of the previous lchan to copy encryption
information from.

Add struct lchan_activate_info.encr to allow passing encryption info through
lchan_activate() without requiring a previous struct gsm_lchan to be present.

Instead of copying from the old lchan, always copy encryption info to
lchan_activate_info, and during activation, just before sending the Channel
Activation, copy the lchan_activate_info.encr to the new lchan.

This prepares for upcoming I5b269f50bd2092516bfdf87746196983d3ac49d1 which
obtains the encryption information from an intra-BSC-incoming Handover Request
message.

Related: OS#3842
Related: I5b269f50bd2092516bfdf87746196983d3ac49d1
Change-Id: Ib3d259a5711add65ab7298bfa3977855a17a1642
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/assignment_fsm.c
M src/osmo-bsc/handover_fsm.c
M src/osmo-bsc/lchan_fsm.c
4 files changed, 10 insertions(+), 10 deletions(-)



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

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 47ca5e8..ba28a6b 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -528,6 +528,7 @@
/* This always is for a specific lchan, so its lchan->type indicates 
full or half rate.
 * When a dyn TS was selected, the lchan->type has been set to the 
desired rate. */
enum gsm48_chan_mode chan_mode;
+   struct gsm_encr encr;
/* AMR config */
uint16_t s15_s0;
bool requires_voice_stream;
diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index c17b555..9c0c400 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -484,6 +484,7 @@
.activ_for = FOR_ASSIGNMENT,
.for_conn = conn,
.chan_mode = conn->lchan->ch_mode_rate.chan_mode,
+   .encr = conn->lchan->encr,
.s15_s0 = conn->lchan->ch_mode_rate.s15_s0,
.requires_voice_stream = conn->assignment.requires_voice_stream,
.msc_assigned_cic = req->msc_assigned_cic,
diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index a3d25d6..9c86b70 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -358,6 +358,7 @@
.activ_for = FOR_HANDOVER,
.for_conn = conn,
.chan_mode = conn->lchan->tch_mode,
+   .encr = conn->lchan->encr,
.requires_voice_stream = conn->lchan->mgw_endpoint_ci_bts ? 
true : false,
.msc_assigned_cic = conn->ho.inter_bsc_in.msc_assigned_cic,
.re_use_mgw_endpoint_from_lchan = conn->lchan,
diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index 2b7dc97..7af2ea0 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -518,14 +518,6 @@

lchan->conn = info->for_conn;

-   if (old_lchan)
-   lchan->encr = old_lchan->encr;
-   else {
-   lchan->encr = (struct gsm_encr){
-   .alg_id = RSL_ENC_ALG_A5(0),/* no encryption */
-   };
-   }
-
/* If there is a previous lchan, and the new lchan is on the same cell 
as previous one,
 * take over power and TA values. Otherwise, use max power and zero TA. 
*/
if (old_lchan && old_lchan->ts->trx->bts == bts) {
@@ -585,14 +577,17 @@
use_mgwep_ci = lchan_use_mgw_endpoint_ci_bts(lchan);

LOG_LCHAN(lchan, LOGL_INFO,
- "Activation requested: %s voice=%s MGW-ci=%s type=%s 
tch-mode=%s\n",
+ "Activation requested: %s voice=%s MGW-ci=%s type=%s 
tch-mode=%s encr-alg=A5/%u ck=%s\n",
  lchan_activate_mode_name(lchan->activate.info.activ_for),
  lchan->activate.info.requires_voice_stream ? "yes" : "no",
  lchan->activate.info.requires_voice_stream ?
(use_mgwep_ci ? mgwep_ci_name(use_mgwep_ci) : "new")
: "none",
  gsm_lchant_name(lchan->type),
- gsm48_chan_mode_name(lchan->tch_mode));
+ gsm48_chan_mode_name(lchan->tch_mode),
+ (lchan->activate.info.encr.alg_id ? : 1)-1,
+ lchan->activate.info.encr.key_len ? 
osmo_hexdump_nospc(lchan->activate.info.encr.key,
+

Change in osmo-bsc[master]: Handover Request: also parse Chosen Algorithm IE, pass to lchan activ...

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13583


Change subject: Handover Request: also parse Chosen Algorithm IE, pass to lchan 
activation
..

Handover Request: also parse Chosen Algorithm IE, pass to lchan activation

During inter-BSC-incoming, the MSC sends the chosen encryption algorithm in the
Handover Request message. Actually parse this Chosen Encryption Algorithm IE.

Place the chosen algorithm and the CK into lchan_activate_info->encr so that
the new lchan will use the same ciphering on this new BSS as it did on the old
BSS.

Change-Id: I5b269f50bd2092516bfdf87746196983d3ac49d1
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/handover_fsm.c
2 files changed, 33 insertions(+), 0 deletions(-)



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

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index ba28a6b..131a53e 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -179,6 +179,10 @@
struct gsm0808_speech_codec_list scl;
struct gsm0808_encrypt_info ei;
struct gsm_classmark classmark;
+   /* chosen_encr_alg reflects the encoded value as in 
RSL_ENC_ALG_A5(a5_numer):
+* chosen_encr_alg == 1 means A5/0 i.e. no encryption, chosen_encr_alg 
== 4 means A5/3.
+* chosen_encr_alg == 0 means no such IE was present. */
+   uint8_t chosen_encr_alg;
struct gsm0808_cell_id cell_id_serving;
char cell_id_serving_name[64];
struct gsm0808_cell_id cell_id_target;
diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index 9c86b70..421c32e 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -432,6 +432,17 @@
   "Missing mandatory IE: 3GPP mandates either Classmark 
Information 1 or 2"
   " in BSSMAP Handover Request, but neither are present. 
Will continue without.\n");

+   if ((e = TLVP_GET(tp, GSM0808_IE_CHOSEN_ENCR_ALG))) {
+   req->chosen_encr_alg = e->val[0];
+   if (req->chosen_encr_alg < 1 || req->chosen_encr_alg > 8)
+   LOG_HO(conn, LOGL_ERROR, "Chosen Encryption Algorithm 
(Serving) is invalid: %u\n",
+  req->chosen_encr_alg);
+   }
+
+   LOG_HO(conn, LOGL_DEBUG, "Handover Request encryption info: 
chosen=A5/%u key=%s\n",
+  (req->chosen_encr_alg ? : 1) - 1, req->ei.key_len?
+  osmo_hexdump_nospc(req->ei.key, req->ei.key_len) : "none");
+
if (TLVP_PRESENT(tp, GSM0808_IE_AOIP_TRASP_ADDR)) {
int rc;
unsigned int u;
@@ -611,6 +622,24 @@
.msc_assigned_cic = req->msc_assigned_cic,
};

+   if (req->chosen_encr_alg) {
+   info.encr.alg_id = req->chosen_encr_alg;
+   if (info.encr.alg_id > 1 && !req->ei.key_len) {
+   ho_fail(HO_RESULT_ERROR, "Chosen Encryption Algorithm 
(Serving) reflects A5/%u"
+   " but there is no key (Encryption 
Information)", info.encr.alg_id - 1);
+   return;
+   }
+   }
+
+   if (req->ei.key_len) {
+   if (req->ei.key_len > sizeof(info.encr.key)) {
+   ho_fail(HO_RESULT_ERROR, "Encryption Information IE key 
length is too large: %u\n",
+   req->ei.key_len);
+   }
+   memcpy(info.encr.key, req->ei.key, req->ei.key_len);
+   info.encr.key_len = req->ei.key_len;
+   }
+
lchan_activate(ho->new_lchan, );
 }


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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5b269f50bd2092516bfdf87746196983d3ac49d1
Gerrit-Change-Number: 13583
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 


Change in libosmocore[master]: make osmo_sockaddr_str_is_set() NULL-safe

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13572


Change subject: make osmo_sockaddr_str_is_set() NULL-safe
..

make osmo_sockaddr_str_is_set() NULL-safe

Obviously a NULL pointer should return false instead of segfaulting.

Change-Id: Iac025cf4d556cbed99f3924cd9ca05a05881cd9a
---
M src/sockaddr_str.c
1 file changed, 2 insertions(+), 1 deletion(-)



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

diff --git a/src/sockaddr_str.c b/src/sockaddr_str.c
index c9d9a94..d683c7d 100644
--- a/src/sockaddr_str.c
+++ b/src/sockaddr_str.c
@@ -60,7 +60,8 @@
  */
 bool osmo_sockaddr_str_is_set(const struct osmo_sockaddr_str *sockaddr_str)
 {
-   return *sockaddr_str->ip
+   return sockaddr_str
+   && *sockaddr_str->ip
&& sockaddr_str->port
&& (sockaddr_str->af == AF_INET || sockaddr_str->af == 
AF_INET6);
 }

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


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-04-10 Thread Neels Hofmeyr
Hello Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/13259

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

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

One quirk I really don't like about this: I would prefer to directly use struct
sockaddr_storage as a member of the struct gsm0808_handover_request_ack. Even
though struct sockaddr_storage appears in various function signatures, the
gsm0808.c actually also gets built on embedded systems that lack arpa/inet.h
(for me indicated by the ARM build job on jenkins). Compiling gsm0808.c works
only because the actual coding of struct sockaddr_storage is implemented in
gsm0808_util.c, which (apparently) does not get built on embedded and hence,
even though there are undefined references to e.g.
gsm0808_enc_aoip_trasp_addr() it works.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 60 insertions(+), 9 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 9
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 


Change in libosmocore[master]: add OSMO_IMSI_SIZE

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13575


Change subject: add OSMO_IMSI_SIZE
..

add OSMO_IMSI_SIZE

Various places in our code base figure out how many char they need to safely
store an IMSI. An IMSI can have a checksum digit, which is not reflected by
GSM23003_IMSI_MAX_DIGITS. And we usually needs a terminating \0.

Instead of having a magic +2 repeated every so often, rather define
OSMO_IMSI_SIZE to contain both checksum digit and nul char, and have the
explanatory comment with it here in libosmocore.

Change-Id: Id11ada4c96b79f7f0ad58185ab7dbf24622fb770
---
M include/osmocom/gsm/protocol/gsm_23_003.h
1 file changed, 3 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/75/13575/1

diff --git a/include/osmocom/gsm/protocol/gsm_23_003.h 
b/include/osmocom/gsm/protocol/gsm_23_003.h
index babd0f4..9dcf4ac 100644
--- a/include/osmocom/gsm/protocol/gsm_23_003.h
+++ b/include/osmocom/gsm/protocol/gsm_23_003.h
@@ -5,6 +5,9 @@
 /* Chapter 2.2 */
 #define GSM23003_IMSI_MAX_DIGITS   15
 #define GSM23003_IMSI_MIN_DIGITS   6
+/*! The char[] buffer size to completely contain an IMSI including the 
optional checksum digit as well as the
+ * terminating nul character. */
+#define OSMO_IMSI_SIZE (GSM23003_IMSI_MAX_DIGITS+2)
 /* Chapter 2.4 */
 #define GSM23003_TMSI_NUM_BYTES4
 /* Chapter 2.5 */

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


Change in libosmocore[master]: add vty_is_active()

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13581


Change subject: add vty_is_active()
..

add vty_is_active()

For async callbacks it is useful to determine whether a given VTY pointer is 
still valid.

For example, in osmo-msc, a silent call can be triggered by VTY, which causes a
Paging. The paging_cb then writes to the VTY console that the silent call has
succeeded. Unless the telnet vty session has already ended, in which case
osmo-msc crashes; e.g. from an osmo_interact_vty.py command invocation. With
this function, osmo-msc can ask whether the vty pointer passed to the paging
callback is still active, and skip vty_out() if not.

Change-Id: I42cf2af47283dd42c101faae0fac293c3a68d599
---
M include/osmocom/vty/vty.h
M src/vty/telnet_interface.c
2 files changed, 12 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/81/13581/1

diff --git a/include/osmocom/vty/vty.h b/include/osmocom/vty/vty.h
index 8d2e1b3..03a2924 100644
--- a/include/osmocom/vty/vty.h
+++ b/include/osmocom/vty/vty.h
@@ -2,6 +2,7 @@

 #include 
 #include 
+#include 

 #include 

@@ -192,6 +193,7 @@
 void vty_reset (void);
 struct vty *vty_new (void);
 struct vty *vty_create (int vty_sock, void *priv);
+bool vty_is_active(struct vty *vty);
 int vty_out (struct vty *, const char *, ...) VTY_PRINTF_ATTRIBUTE(2, 3);
 int vty_out_va(struct vty *vty, const char *format, va_list ap);
 int vty_out_newline(struct vty *);
diff --git a/src/vty/telnet_interface.c b/src/vty/telnet_interface.c
index dc23b12..a1fc999 100644
--- a/src/vty/telnet_interface.c
+++ b/src/vty/telnet_interface.c
@@ -197,6 +197,16 @@
return 0;
 }

+bool vty_is_active(struct vty *vty)
+{
+   struct telnet_connection *connection;
+   llist_for_each_entry(connection, _connections, entry) {
+   if (connection->vty == vty)
+   return true;
+   }
+   return false;
+}
+
 /*! callback from core VTY code about VTY related events */
 void vty_event(enum event event, int sock, struct vty *vty)
 {

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


Change in libosmocore[master]: add osmo_bssap_tlv_parse2() for multiple identical T

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13580


Change subject: add osmo_bssap_tlv_parse2() for multiple identical T
..

add osmo_bssap_tlv_parse2() for multiple identical T

In BSSMAP messages, at least the Cell Identifier IE can appear more than once.

We have tlv_parse2() which allows decoding into an array of tlv_parsed to
cleanly handle multiple occurences. Hence add osmo_bssap_tlv_parse2() which
supports multiple occurences.

An alternative would be to directly call tlv_parse2() with gsm0808_att_tlvdef()
when multiple T occurences are needed, and I'm not really sure why
osmo_bssap_tlv_parse() exists in the first place. But because it does, add a
similar definition that is capable of handling multiple IEs with identical Tag
discriminator.

Change-Id: Ib9a2095f7498dc2cda2a57154b2dbe4621df72f8
---
M include/osmocom/gsm/gsm0808.h
1 file changed, 3 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/80/13580/1

diff --git a/include/osmocom/gsm/gsm0808.h b/include/osmocom/gsm/gsm0808.h
index b31f07d..a4e63a9 100644
--- a/include/osmocom/gsm/gsm0808.h
+++ b/include/osmocom/gsm/gsm0808.h
@@ -305,6 +305,9 @@

 /*! Parse BSSAP TLV structure using \ref tlv_parse */
 #define osmo_bssap_tlv_parse(dec, buf, len) tlv_parse(dec, 
gsm0808_att_tlvdef(), buf, len, 0, 0)
+/*! Parse BSSAP TLV structure using \ref tlv_parse2 */
+#define osmo_bssap_tlv_parse2(dec, dec_multiples, buf, len) \
+   tlv_parse2(dec, dec_multiples, gsm0808_att_tlvdef(), buf, len, 0, 0)

 const char *gsm0808_bssmap_name(uint8_t msg_type);
 const char *gsm0808_bssap_name(uint8_t msg_type);

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


Change in libosmocore[master]: BSSMAP: tweaks

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13577


Change subject: BSSMAP: tweaks
..

BSSMAP: tweaks

Change two instances of Speech Version values to enum gsm0808_permitted_speech.
It is often not trivial to find the right values for a uint8_t member, giving
the enum name makes it a lot easier/safer to use.

In gsm0808_create_handover_required(), use msgb_tv_put() so that the enum's
storage size doesn't matter. (Already used for handover_performed)

Fix typo in doc of gsm0808_create_handover_required().

Change-Id: I6387836bab76e1fa42daa0f42ab94fc14b70b112
---
M TODO-RELEASE
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
3 files changed, 6 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/77/13577/1

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 7c81e32..db3be49 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -19,3 +19,5 @@
osmo_quote_str_buf()  truncated string. This is no longer 
the case. e.g. a string 'truncated' in a
  9-char buffer used to print 
'"trunca"\0', which now becomes '"truncat\0'.
 libosmocoreosmo_quote_str_buf2()   New function signature similar to 
snprintf(), for use with OSMO_STRBUF_APPEND().
+libosmogsm gsm0808_handover_required   Storage size changed, 
speech_version_used now an enum.
+   gsm0808_handover_performed  Storage size changed, 
speech_version_chosen now an enum.
diff --git a/include/osmocom/gsm/gsm0808.h b/include/osmocom/gsm/gsm0808.h
index a1345c3..0f2bf1f 100644
--- a/include/osmocom/gsm/gsm0808.h
+++ b/include/osmocom/gsm/gsm0808.h
@@ -133,7 +133,7 @@
uint8_t current_channel_type_1;

bool speech_version_used_present;
-   uint8_t speech_version_used;
+   enum gsm0808_permitted_speech speech_version_used;

bool old_bss_to_new_bss_info_present;
struct gsm0808_old_bss_to_new_bss_info old_bss_to_new_bss_info;
@@ -196,7 +196,7 @@
uint8_t chosen_encr_alg;

bool speech_version_chosen_present;
-   uint8_t speech_version_chosen;
+   enum gsm0808_permitted_speech speech_version_chosen;

bool speech_codec_chosen_present;
struct gsm0808_speech_codec speech_codec_chosen;
diff --git a/src/gsm/gsm0808.c b/src/gsm/gsm0808.c
index 4873076..3307a5d 100644
--- a/src/gsm/gsm0808.c
+++ b/src/gsm/gsm0808.c
@@ -822,7 +822,7 @@

 /*! Create BSSMAP HANDOVER REQUIRED message.
  * \param[in] params  All information to be encoded.
- * \returns newly allocated msgb with BSSMAP REQUIRED message. */
+ * \returns newly allocated msgb with BSSMAP HANDOVER REQUIRED message. */
 struct msgb *gsm0808_create_handover_required(const struct 
gsm0808_handover_required *params)
 {
struct msgb *msg;
@@ -846,7 +846,7 @@

/* Speech Version (Used), 3.2.2.51 */
if (params->speech_version_used_present)
-   msgb_tv_fixed_put(msg, GSM0808_IE_SPEECH_VERSION, 1, 
>speech_version_used);
+   msgb_tv_put(msg, GSM0808_IE_SPEECH_VERSION, 
params->speech_version_used);

if (params->old_bss_to_new_bss_info_present)
put_old_bss_to_new_bss_information(msg, 
>old_bss_to_new_bss_info);

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


Change in libosmocore[master]: fsm: support graceful osmo_fsm_inst_term() cascades

2019-04-10 Thread Neels Hofmeyr
Hello Harald Welte, Jenkins Builder,

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

https://gerrit.osmocom.org/13392

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

Change subject: fsm: support graceful osmo_fsm_inst_term() cascades
..

fsm: support graceful osmo_fsm_inst_term() cascades

Add global flag osmo_fsm_term_safely() -- if set to true, enable the following
behavior:

Detect osmo_fsm_inst_term() occuring within osmo_fsm_inst_term():
- collect deallocations until the outermost osmo_fsm_inst_term() is done.
- call osmo_fsm_inst_free() *after* dispatching the parent event.

If a struct osmo_fsm_inst enters osmo_fsm_inst_term() while another is already
within osmo_fsm_inst_term(), do not directly deallocate it, but talloc-reparent
it to a separate talloc context, to be deallocated with the outermost FSM inst.

The effect is that all osmo_fsm_inst freed within an osmo_fsm_inst_term()
cascade will stay allocated until all osmo_fsm_inst_term() are complete and all
of them will be deallocated at the same time.

Mark the deferred deallocation state as __thread in an attempt to make cascaded
deallocation handling threadsafe.  Keep the enable/disable flag separate, so
that it is global and not per-thread.

The feature is showcased by fsm_dealloc_test.c: with this feature, all of those
wild deallocation scenarios succeed.

Make fsm_dealloc_test a normal regression test in testsuite.at.

Rationale:

It is difficult to gracefully handle deallocations of groups of FSM instances
that reference each other. As soon as one child dispatching a cleanup event
causes its parent to deallocate before fsm.c was ready for it, deallocation
will hit a use-after-free. Before this patch, by using parent_term events and
distinct "terminating" FSM states, parent/child FSMs can be taught to wait for
all children to deallocate before deallocating the parent. But as soon as a
non-child / non-parent FSM instance is involved, or actually any other
cleanup() action that triggers parent FSMs or parent talloc contexts to become
unused, it is near impossible to think of all possible deallocation events
ricocheting, and to avoid running into freeing FSM instances that were still in
the middle of osmo_fsm_inst_term(), or FSM instances to enter
osmo_fsm_inst_term() more than once. This patch makes deallocation of "all
possible" setups of complex cross referencing FSM instances easy to handle
correctly, without running into use-after-free or double free situations, and,
notably, without changing calling code.

Change-Id: I8eda67540a1cd91beb7856b9fcd0a3143b18
---
M include/osmocom/core/fsm.h
M src/fsm.c
M tests/Makefile.am
M tests/fsm/fsm_dealloc_test.c
M tests/fsm/fsm_dealloc_test.err
M tests/testsuite.at
6 files changed, 3,503 insertions(+), 301 deletions(-)


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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8eda67540a1cd91beb7856b9fcd0a3143b18
Gerrit-Change-Number: 13392
Gerrit-PatchSet: 8
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 


Change in libosmocore[master]: GSUP: add Kind IE

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13576


Change subject: GSUP: add Kind IE
..

GSUP: add Kind IE

osmo-msc and osmo-hlr have distinct subsystems handling incoming GSUP messages.
So far we decide entirely by message type which code path should handle a GSUP
message. Thus no GSUP message type may be re-used across subsystems.

If we add a GSUP message to indicate a routing error, it would have to be a
distinct message type for subscriber management, another one for SMS, another
one for USSD...

To allow introducing common message types, introduce a GSUP Kind IE.

In the presence of this IE, GSUP handlers can trivially direct a received
message to the right code path. If it is missing, handlers can fall back to the
previous switch(message_type) method.

Change-Id: Ic397a9f2c4a7224e47cab944c72e75ca5592efef
---
M include/osmocom/gsm/gsup.h
M src/gsm/gsup.c
M src/gsm/libosmogsm.map
M tests/gsup/gsup_test.c
M tests/gsup/gsup_test.err
5 files changed, 47 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/76/13576/1

diff --git a/include/osmocom/gsm/gsup.h b/include/osmocom/gsm/gsup.h
index 29ea11a..d123da5 100644
--- a/include/osmocom/gsm/gsup.h
+++ b/include/osmocom/gsm/gsup.h
@@ -68,6 +68,7 @@
OSMO_GSUP_FREEZE_PTMSI_IE   = 0x07,
OSMO_GSUP_MSISDN_IE = 0x08,
OSMO_GSUP_HLR_NUMBER_IE = 0x09,
+   OSMO_GSUP_KIND_IE   = 0x0a,
OSMO_GSUP_PDP_CONTEXT_ID_IE = 0x10,
OSMO_GSUP_PDP_TYPE_IE   = 0x11,
OSMO_GSUP_ACCESS_POINT_NAME_IE  = 0x12,
@@ -229,6 +230,21 @@
size_t  pdp_charg_enc_len;
 };

+enum osmo_gsup_kind {
+   OSMO_GSUP_KIND_UNSET = 0,
+   OSMO_GSUP_KIND_SUBSCRIBER_MANAGEMENT = 1,
+   OSMO_GSUP_KIND_SMS = 2,
+   OSMO_GSUP_KIND_USSD = 3,
+   OSMO_GSUP_KIND_INTER_MSC = 4,
+   /* Keep this as last entry with a value of max(enum osmo_gsup_kind) + 1.
+* This value shall serve as the size for an array to aid de-muxing all 
known GSUP kinds. */
+   OSMO_GSUP_KIND_ARRAYSIZE
+};
+
+extern const struct value_string osmo_gsup_kind_names[];
+static inline const char *osmo_gsup_kind_name(enum osmo_gsup_kind val)
+{ return get_value_string(osmo_gsup_kind_names, val); }
+
 /*! parsed/decoded GSUP protocol message */
 struct osmo_gsup_message {
enum osmo_gsup_message_type message_type;
@@ -286,6 +302,11 @@
const uint8_t   *imei_enc;
size_t  imei_enc_len;
enum osmo_gsup_imei_result  imei_result;
+
+   /*! Indicate the subsystem kind to trivially dispatch incoming GSUP 
messages to the right code paths.
+* Inter-MSC messages are *required* to set a kind = 
OSMO_GSUP_KIND_INTER_MSC. For older message kinds, this may
+* be omitted (for backwards compatibility only -- if in doubt, include 
it). */
+   enum osmo_gsup_kind kind;
 };

 int osmo_gsup_decode(const uint8_t *data, size_t data_len,
diff --git a/src/gsm/gsup.c b/src/gsm/gsup.c
index a089322..ea79e91 100644
--- a/src/gsm/gsup.c
+++ b/src/gsm/gsup.c
@@ -477,6 +477,10 @@
gsup_msg->imei_result = osmo_decode_big_endian(value, 
value_len) + 1;
break;

+   case OSMO_GSUP_KIND_IE:
+   gsup_msg->kind = osmo_decode_big_endian(value, 
value_len);
+   break;
+
default:
LOGP(DLGSUP, LOGL_NOTICE,
 "GSUP IE type %d unknown\n", iei);
@@ -718,7 +722,21 @@
msgb_tlv_put(msg, OSMO_GSUP_IMEI_RESULT_IE, sizeof(u8), );
}

+   if (gsup_msg->kind != OSMO_GSUP_KIND_UNSET) {
+   u8 = gsup_msg->kind;
+   msgb_tlv_put(msg, OSMO_GSUP_KIND_IE, sizeof(u8), );
+   }
+
return 0;
 }

+const struct value_string osmo_gsup_kind_names[] = {
+   { OSMO_GSUP_KIND_UNSET, "unset" },
+   { OSMO_GSUP_KIND_SUBSCRIBER_MANAGEMENT, "Subscriber-Management" },
+   { OSMO_GSUP_KIND_SMS, "SMS" },
+   { OSMO_GSUP_KIND_USSD, "USSD" },
+   { OSMO_GSUP_KIND_INTER_MSC, "Inter-MSC" },
+   {}
+};
+
 /*! @} */
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index a69fb60..659cfe6 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -567,6 +567,7 @@
 osmo_gsup_decode;
 osmo_gsup_message_type_names;
 osmo_gsup_session_state_names;
+osmo_gsup_kind_names;
 osmo_gsup_get_err_msg_type;

 osmo_gsup_sms_encode_sm_rp_da;
diff --git a/tests/gsup/gsup_test.c b/tests/gsup/gsup_test.c
index 4ad7431..3e27e99 100644
--- a/tests/gsup/gsup_test.c
+++ b/tests/gsup/gsup_test.c
@@ -11,6 +11,7 @@

 #define TEST_IMSI_IE 0x01, 0x08, 0x21, 0x43, 0x65, 0x87, 0x09, 0x21, 0x43, 0xf5
 #define 

Change in libosmocore[master]: add identifier sanitation for setting FSM instance ids

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13574


Change subject: add identifier sanitation for setting FSM instance ids
..

add identifier sanitation for setting FSM instance ids

We often compose FSM instance IDs from context information, for example placing
an MSISDN string or IP:port information in the FSM instance id, using
osmo_fsm_inst_update_id_f(). This fails if any characters are contained that
don't pass osmo_identifier_valid(). Hence it is the task of the caller to make
sure only characters allowed in an FSM id are applied.

Provide API to trivially allow this by replacing illegal chars:
- osmo_identifier_sanitize_buf(), with access to the same set of illegal
  characters defined in utils.c,
- osmo_fsm_inst_update_id_f_sanitize() implicitly replaces non-identifier
  chars.

This makes it easy to add strings like '192.168.0.1:2342' or '+4987654321' to
an FSM instance id, without adding string mangling to each place that sets an
id; e.g. replacing with '-' to yield '192-168-0-1:2342' or '-4987654321'.

Change-Id: Ia40a6f3b2243c95fe428a080b938e11d8ab771a7
---
M include/osmocom/core/fsm.h
M include/osmocom/core/utils.h
M src/fsm.c
M src/utils.c
4 files changed, 54 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/74/13574/1

diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index c9e1e0c..41d01a5 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -220,6 +220,7 @@

 int osmo_fsm_inst_update_id(struct osmo_fsm_inst *fi, const char *id);
 int osmo_fsm_inst_update_id_f(struct osmo_fsm_inst *fi, const char *fmt, ...);
+int osmo_fsm_inst_update_id_f_sanitize(struct osmo_fsm_inst *fi, char 
replace_with, const char *fmt, ...);

 const char *osmo_fsm_event_name(struct osmo_fsm *fsm, uint32_t event);
 const char *osmo_fsm_inst_name(struct osmo_fsm_inst *fi);
diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h
index ecb04b5..3f28eec 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -136,6 +136,7 @@

 bool osmo_identifier_valid(const char *str);
 bool osmo_separated_identifiers_valid(const char *str, const char *sep_chars);
+void osmo_identifier_sanitize_buf(char *str, const char *sep_chars, char 
replace_with);

 const char *osmo_escape_str(const char *str, int len);
 const char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t 
bufsize);
diff --git a/src/fsm.c b/src/fsm.c
index b6912c6..c32767b 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -364,6 +364,35 @@
return 0;
 }

+/*! Change id of the FSM instance using a string format, and ensuring a valid 
id.
+ * Replace any characters that are not permitted as FSM identifier with 
replace_with.
+ * \param[in] fi FSM instance.
+ * \param[in] replace_with Character to use instead of non-permitted FSM id 
characters.
+ * Make sure to choose a legal character, e.g. '-'.
+ * \param[in] fmt format string to compose new ID.
+ * \param[in] ... variable argument list for format string.
+ * \returns 0 if the ID was updated, otherwise -EINVAL.
+ */
+int osmo_fsm_inst_update_id_f_sanitize(struct osmo_fsm_inst *fi, char 
replace_with, const char *fmt, ...)
+{
+   char *id = NULL;
+   va_list ap;
+   int rc;
+
+   if (!fmt)
+   return osmo_fsm_inst_update_id(fi, NULL);
+
+   va_start(ap, fmt);
+   id = talloc_vasprintf(fi, fmt, ap);
+   va_end(ap);
+
+   osmo_identifier_sanitize_buf(id, NULL, replace_with);
+
+   rc = osmo_fsm_inst_update_id(fi, id);
+   talloc_free(id);
+   return rc;
+}
+
 /*! allocate a new instance of a specified FSM
  *  \param[in] fsm Descriptor of the FSM
  *  \param[in] ctx talloc context from which to allocate memory
diff --git a/src/utils.c b/src/utils.c
index 7def0b8..8ea51ed 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -510,6 +510,8 @@
return true;
 }

+static const char osmo_identifier_illegal_chars[] = "., 
{}[]()<>|~\\^`'\"?=;/+*&%$#!";
+
 /*! Determine if a given identifier is valid, i.e. doesn't contain illegal 
chars
  *  \param[in] str String to validate
  *  \param[in] sep_chars Permitted separation characters between identifiers.
@@ -518,7 +520,6 @@
 bool osmo_separated_identifiers_valid(const char *str, const char *sep_chars)
 {
/* characters that are illegal in names */
-   static const char illegal_chars[] = "., {}[]()<>|~\\^`'\"?=;/+*&%$#!";
unsigned int i;
size_t len;

@@ -535,7 +536,7 @@
if (!isprint((int)str[i]))
return false;
/* check for some explicit reserved control characters */
-   if (strchr(illegal_chars, str[i]))
+   if (strchr(osmo_identifier_illegal_chars, str[i]))
return false;
}

@@ -551,7 +552,26 @@
return 

Change in libosmocore[master]: add gsm48_decode_bcd_number2() from osmo-msc

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13579


Change subject: add gsm48_decode_bcd_number2() from osmo-msc
..

add gsm48_decode_bcd_number2() from osmo-msc

gsm48_decode_bcd_number() is unable to provide proper bounds validation of
input and output data, hence osmo-msc's vlr.c introduced a static
decode_bcd_number_safe() a long time ago. Move to libosmocore.

I need to use the same function to decode an MSISDN during inter-MSC Handover,
instead of making it public in osmo-msc, rather deprecate the unsafe function
and provide a safer version for all callers. Mark the old one deprecated.

Change-Id: Idb6ae6e2f3bea11ad420dae14d021ac36d99e921
---
M include/osmocom/gsm/gsm48_ie.h
M src/gsm/gsm48_ie.c
M src/gsm/libosmogsm.map
3 files changed, 33 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/79/13579/1

diff --git a/include/osmocom/gsm/gsm48_ie.h b/include/osmocom/gsm/gsm48_ie.h
index f7cc24e..71050df 100644
--- a/include/osmocom/gsm/gsm48_ie.h
+++ b/include/osmocom/gsm/gsm48_ie.h
@@ -13,7 +13,11 @@

 /* decode a 'called/calling/connect party BCD number' as in 10.5.4.7 */
 int gsm48_decode_bcd_number(char *output, int output_len,
-   const uint8_t *bcd_lv, int h_len);
+   const uint8_t *bcd_lv, int h_len)
+   OSMO_DEPRECATED("Use gsm48_decode_bcd_number2() for improved bounds 
checking");
+int gsm48_decode_bcd_number2(char *output, size_t output_len,
+const uint8_t *bcd_lv, size_t input_len,
+size_t h_len);

 /* convert a ASCII phone number to 'called/calling/connect party BCD number' */
 int gsm48_encode_bcd_number(uint8_t *bcd_lv, uint8_t max_len,
diff --git a/src/gsm/gsm48_ie.c b/src/gsm/gsm48_ie.c
index ffe3eba..049f5dc 100644
--- a/src/gsm/gsm48_ie.c
+++ b/src/gsm/gsm48_ie.c
@@ -46,7 +46,7 @@
'8', '9', '*', '#', 'a', 'b', 'c', '\0'
 };

-/*! decode a 'called/calling/connect party BCD number' as in 10.5.4.7
+/*! Like gsm48_decode_bcd_number2() but with less airtight bounds checking.
  *  \param[out] Caller-provided output buffer
  *  \param[in] bcd_lv Length-Value portion of to-be-decoded IE
  *  \param[in] h_len Length of an optional heder between L and V portion
@@ -76,6 +76,32 @@
return 0;
 }

+/*! Decode a 'called/calling/connect party BCD number' as in 10.5.4.7.
+ * \param[out] output  Caller-provided output buffer.
+ * \param[in] output_len  sizeof(output).
+ * \param[in] bcd_lv  Length-Value part of to-be-decoded IE.
+ * \param[in] input_len  Size of the buffer to read the IE from.
+ * \param[in] h_len  Length of an optional header between L and V parts.
+ * \return 0 in case of success, negative on error. Errors checked: no or too 
little input data, no or too little
+ * output buffer size, IE length exceeds input data size, decoded number 
exceeds size of the output buffer. The output
+ * is guaranteed to be nul terminated iff output_len > 0.
+ */
+int gsm48_decode_bcd_number2(char *output, size_t output_len,
+const uint8_t *bcd_lv, size_t input_len,
+size_t h_len)
+{
+   uint8_t len;
+   if (output_len < 1)
+   return -ENOSPC;
+   *output = '\0';
+   if (input_len < 1)
+   return -EIO;
+   len = bcd_lv[0];
+   if (input_len < len)
+   return -EIO;
+   return gsm48_decode_bcd_number(output, output_len, bcd_lv, h_len);
+}
+
 /*! convert a single ASCII character to call-control BCD */
 static int asc_to_bcd(const char asc)
 {
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index 1e40af7..b8c92ce 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -307,6 +307,7 @@
 gsm48_encode_ra;
 gsm48_hdr_gmm_cipherable;
 gsm48_decode_bcd_number;
+gsm48_decode_bcd_number2;
 gsm48_decode_bearer_cap;
 gsm48_decode_called;
 gsm48_decode_callerid;

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


Change in libosmocore[master]: add osmo_{escape, quote}_str_buf2() for OSMO_STRBUF_APPEND() use

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13573


Change subject: add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use
..

add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use

To be able to append an escaped or quoted string using OSMO_STRBUF_APPEND(),
the function signature must be like snprintf: buf and size are first arguments,
return value is characters-that-would-be-written.

Add osmo_escape_str_buf2() and osmo_quote_str_buf2() to match this signature.

To avoid code duplication, implement osmo_quote_str_buf() and
osmo_escape_str_buf() by calling the new functions.

I decided to allow slight changes to the behavior for current osmo_escape_str()
and osmo_escape_str_buf(), because impact on callers is minimal:

(1) The new implementation uses OSMO_STRBUF_*, and in consequence
osmo_quote_str() no longer prints an ending double quote after truncated
strings; Before, a truncated output was, sic:
  "this string is trunca"
and now this becomes, sic:
  "this string is truncat
I decided to not keep the old behavior because it is questionable to begin
with. It looks like the string actually ended at the truncation boundary
instead of the reason being not enough space in the output buffer.

(2) The new osmo_escape_str_buf2() function obviously cannot pass-thru an
unchanged char* if no escaping was needed. Sacrifice this tiny optimization
feature to avoid code duplication:
- it is an unnoticeable optimization,
- the caller anyway always passes a string buffer,
- the feature caused handling strings and buffers differently depending on
  their content (i.e. code that usually writes out strings in full length
  "suddenly" truncates because a non-printable character is contained, etc.)
I considered adding a skip_if_unescaped flag to the osmo_quote_str_buf2()
function signature, but in the end decided that the API clutter is not worth
having for all the above reasons.

Adjust tests to accomodate above changes.

Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
---
M TODO-RELEASE
M include/osmocom/core/utils.h
M src/utils.c
M tests/utils/utils_test.c
M tests/utils/utils_test.ok
5 files changed, 110 insertions(+), 44 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/73/13573/1

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 5ddc57a..7c81e32 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -11,3 +11,11 @@
 libosmogb  gprs_nsvc   Adding sig_weight and data_weight 
members for IP-SNS support
 libosmogb  various new symbols Adding functions related to IP-SNS 
support
 libosmocoreosmo_fsm_inst   Add flag proc.terminating (ABI change)
+libosmocoreosmo_escape_str(),  These now always copy to the buffer 
instead of returning the
+   osmo_escape_str_buf() unchanged input string when no chars 
needed escaping, hence
+ returned strings might now also be 
truncated even if all chars were printable.
+libosmocoreosmo_escape_str_buf2()  New function signature similar to 
snprintf(), for use with OSMO_STRBUF_APPEND().
+libosmocoreosmo_quote_str(),   On string truncation, these used to 
print a closing quote '"' after the
+   osmo_quote_str_buf()  truncated string. This is no longer 
the case. e.g. a string 'truncated' in a
+ 9-char buffer used to print 
'"trunca"\0', which now becomes '"truncat\0'.
+libosmocoreosmo_quote_str_buf2()   New function signature similar to 
snprintf(), for use with OSMO_STRBUF_APPEND().
diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h
index e19649a..ecb04b5 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -138,9 +138,11 @@
 bool osmo_separated_identifiers_valid(const char *str, const char *sep_chars);

 const char *osmo_escape_str(const char *str, int len);
-char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t 
bufsize);
+const char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t 
bufsize);
+int osmo_escape_str_buf2(char *buf, size_t bufsize, const char *str, int 
in_len);
 const char *osmo_quote_str(const char *str, int in_len);
-char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t 
bufsize);
+const char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t 
bufsize);
+int osmo_quote_str_buf2(char *buf, size_t bufsize, const char *str, int 
in_len);

 uint32_t osmo_isqrt32(uint32_t x);

diff --git a/src/utils.c b/src/utils.c
index b50ceab..7def0b8 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -557,22 +557,74 @@
  * \param[inout] buf  string buffer to write escaped characters to.
  * \param[in] bufsize  size of \a buf.
  * \returns buf containing an escaped representation, possibly truncated.
+ * \returns buf containing an escaped representation, 

Change in libosmocore[master]: BSSMAP: add messages for inter-BSC and inter-MSC Handover

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13578


Change subject: BSSMAP: add messages for inter-BSC and inter-MSC Handover
..

BSSMAP: add messages for inter-BSC and inter-MSC Handover

Change-Id: I9dac375331f6bea744769e973725d58e35f87226
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 241 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/78/13578/1

diff --git a/include/osmocom/gsm/gsm0808.h b/include/osmocom/gsm/gsm0808.h
index a6c5239..b31f07d 100644
--- a/include/osmocom/gsm/gsm0808.h
+++ b/include/osmocom/gsm/gsm0808.h
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #define BSSMAP_MSG_SIZE 1024
@@ -143,6 +144,56 @@
 };
 struct msgb *gsm0808_create_handover_required(const struct 
gsm0808_handover_required *params);

+/*! 3GPP TS 48.008 §3.2.1.37 HANDOVER REQUIRED REJECT */
+struct gsm0808_handover_required_reject {
+   uint16_t cause;
+
+   /* more items are defined in the spec and may be added later */
+   bool more_items; /*< always set this to false */
+};
+struct msgb *gsm0808_create_handover_required_reject(const struct 
gsm0808_handover_required_reject *params);
+
+/*! 3GPP TS 48.008 §3.2.1.8 HANDOVER REQUEST */
+struct gsm0808_handover_request {
+   struct gsm0808_channel_type channel_type;
+   struct gsm0808_encrypt_info encryption_information;
+   struct osmo_gsm48_classmark classmark_information;
+   struct gsm0808_cell_id cell_identifier_serving;
+   struct gsm0808_cell_id cell_identifier_target;
+   enum gsm0808_cause cause;
+
+   bool current_channel_type_1_present;
+   uint8_t current_channel_type_1;
+
+   enum gsm0808_permitted_speech speech_version_used;
+
+   uint8_t chosen_encryption_algorithm_serving;
+
+   /*! Pass either old_bss_to_new_bss_info or old_bss_to_new_bss_info_raw. 
*/
+   bool old_bss_to_new_bss_info_present;
+   struct gsm0808_old_bss_to_new_bss_info old_bss_to_new_bss_info;
+   /*! To feed the Old BSS to New BSS Information IE unchanged from the 
Handover Required message without having to
+* decode it. Pass either old_bss_to_new_bss_info or 
old_bss_to_new_bss_info_raw. Omit the TL part. */
+   const uint8_t *old_bss_to_new_bss_info_raw;
+   uint8_t old_bss_to_new_bss_info_raw_len;
+
+   const char *imsi;
+
+   const struct sockaddr_storage *aoip_transport_layer;
+
+   const struct gsm0808_speech_codec_list *codec_list_msc_preferred;
+
+   bool call_id_present;
+   uint32_t call_id;
+
+   const uint8_t *global_call_reference;
+   uint8_t global_call_reference_len;
+
+   /* more items are defined in the spec and may be added later */
+   bool more_items; /*!< always set this to false */
+};
+struct msgb *gsm0808_create_handover_request(const struct 
gsm0808_handover_request *params);
+
 struct gsm0808_handover_request_ack {
const uint8_t *l3_info;
uint8_t l3_info_len;
@@ -170,7 +221,22 @@
 uint8_t chosen_channel, 
uint8_t chosen_encr_alg,
 uint8_t chosen_speech_version);

+struct gsm0808_handover_command {
+   const uint8_t *l3_info;
+   uint8_t l3_info_len;
+
+   struct gsm0808_cell_id cell_identifier;
+
+   const uint8_t *new_bss_to_old_bss_info_raw;
+   size_t new_bss_to_old_bss_info_raw_len;
+
+   /* more items are defined in the spec and may be added later */
+   bool more_items; /*!< always set this to false */
+};
+struct msgb *gsm0808_create_handover_command(const struct 
gsm0808_handover_command *params);
+
 struct msgb *gsm0808_create_handover_detect();
+struct msgb *gsm0808_create_handover_succeeded();

 struct gsm0808_handover_complete {
bool rr_cause_present;
diff --git a/src/gsm/gsm0808.c b/src/gsm/gsm0808.c
index b46977b..3c77c77 100644
--- a/src/gsm/gsm0808.c
+++ b/src/gsm/gsm0808.c
@@ -857,6 +857,129 @@
return msg;
 }

+/*! Create BSSMAP HANDOVER REQUIRED REJECT message.
+ * \returns newly allocated msgb with BSSMAP HANDOVER REQUIRED REJECT message. 
*/
+struct msgb *gsm0808_create_handover_required_reject(const struct 
gsm0808_handover_required_reject *params)
+{
+   struct msgb *msg;
+
+   msg = msgb_alloc_headroom(BSSMAP_MSG_SIZE, BSSMAP_MSG_HEADROOM, 
"BSSMAP-HANDOVER-REQUIRED-REJECT");
+   if (!msg)
+   return NULL;
+
+   /* Message Type, 3.2.2.1 */
+   msgb_v_put(msg, BSS_MAP_MSG_HANDOVER_REQUIRED_REJECT);
+
+   /* Cause, 3.2.2.5 */
+   gsm0808_enc_cause(msg, params->cause);
+
+   /* prepend the header */
+   msg->l3h = msgb_tv_push(msg, BSSAP_MSG_BSS_MANAGEMENT, 
msgb_length(msg));
+
+   return msg;
+}
+
+/*! Create BSSMAP HANDOVER REQUEST message, 3GPP TS 48.008 3.2.1.8.
+ * Sent from the 

Change in libosmocore[master]: fsm: support graceful osmo_fsm_inst_term() cascades

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13392 )

Change subject: fsm: support graceful osmo_fsm_inst_term() cascades
..


Patch Set 7:

> it might make sense to expose the value of this global flag variable in the 
> fsm VTY code, so that "show fsm" would give you an indication?

So far we have 'show fsm all' and 'show fsm NAME', i.e. shows individual FSMs. 
There is nothing yet showing global FSM state that applies to all FSMs.

Let's leave it to a later patch if the need comes up?


-- 
To view, visit https://gerrit.osmocom.org/13392
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: I8eda67540a1cd91beb7856b9fcd0a3143b18
Gerrit-Change-Number: 13392
Gerrit-PatchSet: 7
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Wed, 10 Apr 2019 15:54:52 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: fsm: support graceful osmo_fsm_inst_term() cascades

2019-04-10 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13392 )

Change subject: fsm: support graceful osmo_fsm_inst_term() cascades
..


Patch Set 7:

(1 comment)

https://gerrit.osmocom.org/#/c/13392/7/src/fsm.c
File src/fsm.c:

https://gerrit.osmocom.org/#/c/13392/7/src/fsm.c@105
PS7, Line 105: = {};
> I'm not sure you can do this to a __thread variable. […]
just to be explicit ... because the concept that all static variables are 
guaranteed to be zero initialized is still new to me and I haven't settled into 
trusting that yet. I can drop it...

This is the first time I come across __thread, I don't know anything about it.



--
To view, visit https://gerrit.osmocom.org/13392
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: I8eda67540a1cd91beb7856b9fcd0a3143b18
Gerrit-Change-Number: 13392
Gerrit-PatchSet: 7
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Wed, 10 Apr 2019 15:50:45 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-msc[master]: libmsc/ran_conn.c: send S_SUBSCR_ATTACHED after RAN_CONN_S_ACCEPTED

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has uploaded this change for review. ( 
https://gerrit.osmocom.org/13571


Change subject: libmsc/ran_conn.c: send S_SUBSCR_ATTACHED after 
RAN_CONN_S_ACCEPTED
..

libmsc/ran_conn.c: send S_SUBSCR_ATTACHED after RAN_CONN_S_ACCEPTED

Change-Id: Id5bc2d1d488a0ab92fd72e9cef6250e6794807a9
---
M src/libmsc/ran_conn.c
M src/libmsc/sms_queue.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_no_authen.err
M tests/msc_vlr/msc_vlr_test_reject_concurrency.err
M tests/msc_vlr/msc_vlr_test_ss.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
10 files changed, 65 insertions(+), 45 deletions(-)



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

diff --git a/src/libmsc/ran_conn.c b/src/libmsc/ran_conn.c
index e54e542..3f75314 100644
--- a/src/libmsc/ran_conn.c
+++ b/src/libmsc/ran_conn.c
@@ -79,27 +79,18 @@
}
 }

+/* TODO: get rid of this function... */
 static void evaluate_acceptance_outcome(struct osmo_fsm_inst *fi, bool 
conn_accepted)
 {
struct ran_conn *conn = fi->priv;

update_counters(fi, conn_accepted);

-   /* Trigger transactions that we paged for */
-   if (conn->complete_layer3_type == COMPLETE_LAYER3_PAGING_RESP) {
-   subscr_paging_dispatch(GSM_HOOK_RR_PAGING,
-  conn_accepted ? GSM_PAGING_SUCCEEDED : 
GSM_PAGING_EXPIRED,
+   if (conn->complete_layer3_type == COMPLETE_LAYER3_PAGING_RESP
+   && !conn_accepted) {
+   subscr_paging_dispatch(GSM_HOOK_RR_PAGING, GSM_PAGING_EXPIRED,
   NULL, conn, conn->vsub);
}
-
-   if (conn->complete_layer3_type == COMPLETE_LAYER3_CM_SERVICE_REQ
-   && conn_accepted) {
-   conn->received_cm_service_request = true;
-   ran_conn_get(conn, RAN_CONN_USE_CM_SERVICE);
-   }
-
-   if (conn_accepted)
-   osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_ATTACHED, conn->vsub);
 }

 static void log_close_event(struct osmo_fsm_inst *fi, uint32_t event, void 
*data)
@@ -272,6 +263,23 @@
 * The LU expiry timer will restart once the connection is closed. */
conn->vsub->expire_lu = VLR_SUBSCRIBER_NO_EXPIRATION;

+   switch (conn->complete_layer3_type) {
+   case COMPLETE_LAYER3_CM_SERVICE_REQ:
+   conn->received_cm_service_request = true;
+   ran_conn_get(conn, RAN_CONN_USE_CM_SERVICE);
+   break;
+   case COMPLETE_LAYER3_PAGING_RESP:
+   subscr_paging_dispatch(GSM_HOOK_RR_PAGING, GSM_PAGING_SUCCEEDED,
+  NULL, conn, conn->vsub);
+   break;
+   default:
+   break;
+   }
+
+   /* Notify all sub-systems that a subscriber is available, so
+* they can e.g. initiate a MT SMS delivery or RRLP request. */
+   osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_ATTACHED, conn->vsub);
+
if (!ran_conn_fsm_has_active_transactions(fi))
osmo_fsm_inst_dispatch(fi, RAN_CONN_E_UNUSED, NULL);
 }
diff --git a/src/libmsc/sms_queue.c b/src/libmsc/sms_queue.c
index afd878f..f1d6279 100644
--- a/src/libmsc/sms_queue.c
+++ b/src/libmsc/sms_queue.c
@@ -404,8 +404,9 @@

 static int sub_ready_for_sm(struct gsm_network *net, struct vlr_subscr *vsub)
 {
-   struct gsm_sms *sms;
struct gsm_sms_pending *pending;
+   struct ran_conn *conn;
+   struct gsm_sms *sms;

/*
 * The code used to be very clever and tried to submit
@@ -430,6 +431,17 @@
return 0;
}

+   /* Check if RAN connection was used for Location Updating */
+   conn = connection_for_subscr(vsub);
+   if (!conn) {
+   LOGP(DMSC, LOGL_ERROR, "S_SUBSCR_ATTACHED, but no RAN 
connection?!?\n");
+   return -EINVAL;
+   }
+   if (conn->complete_layer3_type == COMPLETE_LAYER3_LU) {
+   /* FIXME: distinguish IMSI Attach and periodical LU! */
+   return 0;
+   }
+
/* Now try to deliver any pending SMS to this sub */
sms = db_sms_get_unsent_for_subscr(vsub, UINT_MAX);
if (!sms)
diff --git a/tests/msc_vlr/msc_vlr_test_authen_reuse.err 
b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
index 1a57097..468c8e9 100644
--- a/tests/msc_vlr/msc_vlr_test_authen_reuse.err
+++ b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
@@ -247,8 +247,8 @@
 DVLR 
Process_Access_Request_VLR(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:GERAN-A-0:CM_SERVICE_REQ){PR_ARQ_S_DONE}:
 Process Access Request result: PASSED
 - sending CM Service Accept for 
IMSI-90170010650:MSISDN-42342:TMSI-0x03020100
 DMM 
RAN_conn(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:GERAN-A-0:CM_SERVICE_REQ){RAN_CONN_S_AUTH_CIPH}:
 Received Event 

Change in osmo-ggsn[master]: process_pco() const-ify 'apn' argument

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13566 )

Change subject: process_pco() const-ify 'apn' argument
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a96b0fbe077c7c49342553de0880bfc58318669
Gerrit-Change-Number: 13566
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Wed, 10 Apr 2019 14:04:29 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-msc[master]: smpp: Make libsmpp34 use talloc for its allocations

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13562 )

Change subject: smpp: Make libsmpp34 use talloc for its allocations
..


Patch Set 2:

Hi Pau, Harald,

> Was there some communication with the original author before you changed API 
> names?
> This is confusing for reviewers and a bit rude imho, specially since you 
> didn't
> explain your intention with this kind of change.

There was no. For some reason, I thought that this change is also assigned to 
me... You're right, sorry for that. My intention was to make the symbol names a 
bit shorter, and a bit closer to what I remember from similar API of SQLite3 
(where one can use SQLITE_CONFIG_MALLOC).

Please let me know whether I should rollback to the previous version.


--
To view, visit https://gerrit.osmocom.org/13562
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: Ie2725ffab6a225813e65768735f01678e2022128
Gerrit-Change-Number: 13562
Gerrit-PatchSet: 2
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Vadim Yanitskiy 
Gerrit-Comment-Date: Wed, 10 Apr 2019 14:03:18 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libsmpp34[master]: Allow application to override default heap allocator

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has uploaded a new patch set (#5) to the change originally 
created by Harald Welte. ( https://gerrit.osmocom.org/13561 )

Change subject: Allow application to override default heap allocator
..

Allow application to override default heap allocator

Let's introduce a mechanism by which libsmpp34-using applications can
override the memory allocator functions.  This allows us e.g. in the
Osmocom context to use talloc which aids us in debugging memory leaks.

Change-Id: I3656117115e89638c093bfbcbc4369ce302f7a94
Closes: OS#3913
---
M TODO-RELEASE
M src/Makefile.am
A src/smpp34_heap.c
A src/smpp34_heap.h
M src/smpp34_params.c
M src/smpp34_unpack.c
6 files changed, 110 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libsmpp34 refs/changes/61/13561/5
--
To view, visit https://gerrit.osmocom.org/13561
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3656117115e89638c093bfbcbc4369ce302f7a94
Gerrit-Change-Number: 13561
Gerrit-PatchSet: 5
Gerrit-Owner: Harald Welte 
Gerrit-Assignee: fixeria 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Vadim Yanitskiy 
Gerrit-CC: fixeria 


Change in osmo-ggsn[master]: ggsn: Remove magic numbers from pco_contains_proto()

2019-04-10 Thread Harald Welte
Harald Welte has uploaded this change for review. ( 
https://gerrit.osmocom.org/13567


Change subject: ggsn: Remove magic numbers from pco_contains_proto()
..

ggsn: Remove magic numbers from pco_contains_proto()

Let's remove some magic numbers and use a data structure to describe
the PCO element header.

Change-Id: I9871ffced677320aa82438332bfdb951ab129f04
---
M ggsn/ggsn.c
1 file changed, 11 insertions(+), 6 deletions(-)



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

diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index 2ee9b1a..c31831a 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -462,18 +462,23 @@
PCO_P_REL_DATA_SVC  = 0x0018,
 };

+struct pco_element {
+   uint16_t protocol_id;   /* network byte order */
+   uint8_t length; /* length of data below */
+   uint8_t data[0];
+} __attribute__((packed));
+
 /* determine if PCO contains given protocol */
 static uint8_t *pco_contains_proto(struct ul255_t *pco, size_t offset, 
uint16_t prot, size_t prot_minlen)
 {
-   uint8_t *cur = pco->v + 1 + offset;
+   uint8_t *cur = pco->v + 1 /*length*/ + offset;

/* iterate over PCO and check if protocol contained */
-   while (cur + 3 <= pco->v + pco->l) {
-   uint16_t cur_prot = osmo_load16be(cur);
-   uint8_t cur_len = cur[2];
-   if (cur_prot == prot && cur_len >= prot_minlen)
+   while (cur + sizeof(struct pco_element) <= pco->v + pco->l) {
+   const struct pco_element *elem = (const struct pco_element 
*)cur;
+   if (ntohs(elem->protocol_id) == prot && elem->length >= 
prot_minlen)
return cur;
-   cur += cur_len + 3;
+   cur += elem->length + sizeof(struct pco_element);
}
return NULL;
 }

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

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9871ffced677320aa82438332bfdb951ab129f04
Gerrit-Change-Number: 13567
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte 


Change in osmo-ggsn[master]: ggsn: Fix build_ipcp_pco() in presence of invalid IPCP content

2019-04-10 Thread Harald Welte
Harald Welte has uploaded this change for review. ( 
https://gerrit.osmocom.org/13570


Change subject: ggsn: Fix build_ipcp_pco() in presence of invalid IPCP content
..

ggsn: Fix build_ipcp_pco() in presence of invalid IPCP content

When build_ipcp_pco() iterated over the PCO list, it didn't use
the "outer" pco length as an increment, but used the "inner" IPCP
length.

If an IPCP message with an invalid "inner" length was being processed
(see pcap file attached to OS#3914), the PCO iteration beyond that
broken IPCP would fail, possibly rendering false hits.

Let's make pco_contains_proto() return a pointer to the the pco_element,
so that the caller can use the outer length as an increment.

Change-Id: I8e9cffde092c8c5824abfaeecb742afcf949802c
Related: OS#3914
---
M ggsn/ggsn.c
1 file changed, 7 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ggsn refs/changes/70/13570/1

diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index 2d1368b..eac7e16 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -471,8 +471,8 @@
 } __attribute__((packed));

 /* determine if PCO contains given protocol */
-static const uint8_t *pco_contains_proto(const struct ul255_t *pco, size_t 
offset,
-uint16_t prot, size_t prot_minlen)
+static const struct pco_element *pco_contains_proto(const struct ul255_t *pco, 
size_t offset,
+   uint16_t prot, size_t 
prot_minlen)
 {
const uint8_t *cur = pco->v + 1 /*length*/ + offset;

@@ -480,7 +480,7 @@
while (cur + sizeof(struct pco_element) <= pco->v + pco->l) {
const struct pco_element *elem = (const struct pco_element 
*)cur;
if (ntohs(elem->protocol_id) == prot && elem->length >= 
prot_minlen)
-   return cur;
+   return elem;
cur += elem->length + sizeof(struct pco_element);
}
return NULL;
@@ -515,7 +515,8 @@
 {
const struct in46_addr *dns1 = >v4.cfg.dns[0];
const struct in46_addr *dns2 = >v4.cfg.dns[1];
-   const uint8_t *ipcp, *pco_ipcp;
+   const struct pco_element *pco_ipcp;
+   const uint8_t *ipcp;
uint16_t ipcp_len;
uint8_t *len1, *len2;
unsigned int len_appended;
@@ -527,7 +528,7 @@
while (pco_ipcp) {
uint8_t *start = msg->tail;

-   ipcp = (pco_ipcp + 3);  /* 2=type + 1=len */
+   ipcp = pco_ipcp->data;
consumed = (ipcp - >pco_req.v[0]);
remain = sizeof(pdp->pco_req.v) - consumed;
ipcp_len = osmo_load16be(ipcp + 2); /* 1=code + 1=id */
@@ -560,7 +561,7 @@
*len1 = len_appended - 3;
*len2 = len_appended - 3;

-   offset += 3 + ipcp_len;
+   offset += sizeof(pco_ipcp) + pco_ipcp->length;
pco_ipcp = pco_contains_proto(>pco_req, offset, 
PCO_P_IPCP, sizeof(struct ipcp_hdr));
}


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

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e9cffde092c8c5824abfaeecb742afcf949802c
Gerrit-Change-Number: 13570
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte 


Change in osmo-ggsn[master]: ggsn: const-ify input / read-only arguments of PCO related functions

2019-04-10 Thread Harald Welte
Harald Welte has uploaded this change for review. ( 
https://gerrit.osmocom.org/13568


Change subject: ggsn: const-ify input / read-only arguments of PCO related 
functions
..

ggsn: const-ify input / read-only arguments of PCO related functions

Change-Id: Ia0877988180ded4e3c033d7f1fb6e1c2acd60163
---
M ggsn/ggsn.c
1 file changed, 8 insertions(+), 6 deletions(-)



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

diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index c31831a..2c0d64e 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -413,9 +413,10 @@
 } __attribute__ ((packed));

 /* determine if IPCP contains given option */
-static uint8_t *ipcp_contains_option(uint8_t *ipcp, size_t ipcp_len, enum 
ipcp_options opt, size_t opt_minlen)
+static const uint8_t *ipcp_contains_option(const uint8_t *ipcp, size_t 
ipcp_len,
+  enum ipcp_options opt, size_t 
opt_minlen)
 {
-   uint8_t *cur_opt = ipcp + sizeof(struct ipcp_hdr);
+   const uint8_t *cur_opt = ipcp + sizeof(struct ipcp_hdr);

/* iterate over Options and check if protocol contained */
while (cur_opt + 2 <= ipcp + ipcp_len) {
@@ -469,9 +470,10 @@
 } __attribute__((packed));

 /* determine if PCO contains given protocol */
-static uint8_t *pco_contains_proto(struct ul255_t *pco, size_t offset, 
uint16_t prot, size_t prot_minlen)
+static const uint8_t *pco_contains_proto(const struct ul255_t *pco, size_t 
offset,
+uint16_t prot, size_t prot_minlen)
 {
-   uint8_t *cur = pco->v + 1 /*length*/ + offset;
+   const uint8_t *cur = pco->v + 1 /*length*/ + offset;

/* iterate over PCO and check if protocol contained */
while (cur + sizeof(struct pco_element) <= pco->v + pco->l) {
@@ -512,9 +514,9 @@
 {
const struct in46_addr *dns1 = >v4.cfg.dns[0];
const struct in46_addr *dns2 = >v4.cfg.dns[1];
-   uint8_t *ipcp;
+   const uint8_t *ipcp, *pco_ipcp;
uint16_t ipcp_len;
-   uint8_t *len1, *len2, *pco_ipcp;
+   uint8_t *len1, *len2;
unsigned int len_appended;
ptrdiff_t consumed;
size_t remain, offset = 0;

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

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia0877988180ded4e3c033d7f1fb6e1c2acd60163
Gerrit-Change-Number: 13568
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte 


Change in osmo-ggsn[master]: ggsn: Remove magic numbers from ipcp_contains_option()

2019-04-10 Thread Harald Welte
Harald Welte has uploaded this change for review. ( 
https://gerrit.osmocom.org/13569


Change subject: ggsn: Remove magic numbers from ipcp_contains_option()
..

ggsn: Remove magic numbers from ipcp_contains_option()

Let's remove some magic numbers and use a data structure instead.

Change-Id: I5b1abc6f403f85986407e9e8359924dfcb58031a
---
M ggsn/ggsn.c
1 file changed, 7 insertions(+), 6 deletions(-)



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

diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index 2c0d64e..2d1368b 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -419,14 +419,15 @@
const uint8_t *cur_opt = ipcp + sizeof(struct ipcp_hdr);

/* iterate over Options and check if protocol contained */
-   while (cur_opt + 2 <= ipcp + ipcp_len) {
-   uint8_t type = cur_opt[0];
-   uint8_t len = cur_opt[1]; /* length value includes 2 bytes 
type/length */
-   if (len < 2)
+   while (cur_opt + sizeof(struct ipcp_option_hdr) <= ipcp + ipcp_len) {
+   const struct ipcp_option_hdr *cur_opt_hdr = (const struct 
ipcp_option_hdr *)cur_opt;
+   /* length value includes 2 bytes type/length */
+   if (cur_opt_hdr->len < sizeof(struct ipcp_option_hdr))
return NULL;
-   if (type == opt && len >= 2 + opt_minlen)
+   if (cur_opt_hdr->type == opt &&
+   cur_opt_hdr->len >= sizeof(struct ipcp_option_hdr) + 
opt_minlen)
return cur_opt;
-   cur_opt += len;
+   cur_opt += cur_opt_hdr->len;
}
return NULL;
 }

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

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5b1abc6f403f85986407e9e8359924dfcb58031a
Gerrit-Change-Number: 13569
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte 


Change in osmo-ggsn[master]: process_pco() const-ify 'apn' argument

2019-04-10 Thread Harald Welte
Harald Welte has uploaded this change for review. ( 
https://gerrit.osmocom.org/13566


Change subject: process_pco() const-ify 'apn' argument
..

process_pco() const-ify 'apn' argument

Change-Id: I2a96b0fbe077c7c49342553de0880bfc58318669
---
M ggsn/ggsn.c
1 file changed, 4 insertions(+), 4 deletions(-)



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

diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index 39695b9..2ee9b1a 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -503,7 +503,7 @@
 }

 /* construct an IPCP PCO response from request*/
-static void build_ipcp_pco(struct apn_ctx *apn, struct pdp_t *pdp, struct msgb 
*msg)
+static void build_ipcp_pco(const struct apn_ctx *apn, struct pdp_t *pdp, 
struct msgb *msg)
 {
const struct in46_addr *dns1 = >v4.cfg.dns[0];
const struct in46_addr *dns2 = >v4.cfg.dns[1];
@@ -559,7 +559,7 @@
 }

 /* process one PCO request from a MS/UE, putting together the proper responses 
*/
-static void process_pco(struct apn_ctx *apn, struct pdp_t *pdp)
+static void process_pco(const struct apn_ctx *apn, struct pdp_t *pdp)
 {
struct msgb *msg = msgb_alloc(256, "PCO");
struct ippoolm_t *peer_v4 = pdp_get_peer_ipv(pdp, false);
@@ -573,7 +573,7 @@

if (pco_contains_proto(>pco_req, 0, PCO_P_DNS_IPv6_ADDR, 0)) {
for (i = 0; i < ARRAY_SIZE(apn->v6.cfg.dns); i++) {
-   struct in46_addr *i46a = >v6.cfg.dns[i];
+   const struct in46_addr *i46a = >v6.cfg.dns[i];
if (i46a->len != 16)
continue;
msgb_t16lv_put(msg, PCO_P_DNS_IPv6_ADDR, i46a->len, 
i46a->v6.s6_addr);
@@ -582,7 +582,7 @@

if (pco_contains_proto(>pco_req, 0, PCO_P_DNS_IPv4_ADDR, 0)) {
for (i = 0; i < ARRAY_SIZE(apn->v4.cfg.dns); i++) {
-   struct in46_addr *i46a = >v4.cfg.dns[i];
+   const struct in46_addr *i46a = >v4.cfg.dns[i];
if (i46a->len != 4)
continue;
msgb_t16lv_put(msg, PCO_P_DNS_IPv4_ADDR, i46a->len, 
(uint8_t *)>v4);

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

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2a96b0fbe077c7c49342553de0880bfc58318669
Gerrit-Change-Number: 13566
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte 


Change in libsmpp34[master]: Allow application to override default heap allocator

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13561 )

Change subject: Allow application to override default heap allocator
..


Patch Set 4:

(1 comment)

https://gerrit.osmocom.org/#/c/13561/4/src/smpp34_heap.c
File src/smpp34_heap.c:

https://gerrit.osmocom.org/#/c/13561/4/src/smpp34_heap.c@2
PS4, Line 2:  * Copyright (C) 2019 Raul Tremsal
> Why this copyright here?
Oh, sure, thanks!



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

Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3656117115e89638c093bfbcbc4369ce302f7a94
Gerrit-Change-Number: 13561
Gerrit-PatchSet: 4
Gerrit-Owner: Harald Welte 
Gerrit-Assignee: fixeria 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: Vadim Yanitskiy 
Gerrit-CC: fixeria 
Gerrit-Comment-Date: Wed, 10 Apr 2019 13:52:20 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-msc[master]: smpp: Make libsmpp34 use talloc for its allocations

2019-04-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/13562 )

Change subject: smpp: Make libsmpp34 use talloc for its allocations
..


Patch Set 2:

(1 comment)

Was there some communication with the original author before you changed API 
names? This is confusing for reviewers and a bit rude imho, specially since you 
didn't explain your intention with this kind of change.

https://gerrit.osmocom.org/#/c/13562/2//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/13562/2//COMMIT_MSG@9
PS2, Line 9: We are just introducing smpp34_set_memory_functions() in libsmpp34
You forgot to update this reference when changing API names



--
To view, visit https://gerrit.osmocom.org/13562
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: Ie2725ffab6a225813e65768735f01678e2022128
Gerrit-Change-Number: 13562
Gerrit-PatchSet: 2
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Comment-Date: Wed, 10 Apr 2019 13:16:11 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libsmpp34[master]: Allow application to override default heap allocator

2019-04-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/13561 )

Change subject: Allow application to override default heap allocator
..


Patch Set 4:

(1 comment)

https://gerrit.osmocom.org/#/c/13561/4/src/smpp34_heap.c
File src/smpp34_heap.c:

https://gerrit.osmocom.org/#/c/13561/4/src/smpp34_heap.c@2
PS4, Line 2:  * Copyright (C) 2019 Raul Tremsal
Why this copyright here?



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

Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3656117115e89638c093bfbcbc4369ce302f7a94
Gerrit-Change-Number: 13561
Gerrit-PatchSet: 4
Gerrit-Owner: Harald Welte 
Gerrit-Assignee: fixeria 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: fixeria 
Gerrit-Comment-Date: Wed, 10 Apr 2019 13:13:27 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libsmpp34[master]: configure.ac: enable kernel style compile messages

2019-04-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/13565 )

Change subject: configure.ac: enable kernel style compile messages
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a5d808e2a199c33a1c9eef77da3a040e305689
Gerrit-Change-Number: 13565
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Wed, 10 Apr 2019 13:11:58 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libsmpp34[master]: configure.ac: enable kernel style compile messages

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13565 )

Change subject: configure.ac: enable kernel style compile messages
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a5d808e2a199c33a1c9eef77da3a040e305689
Gerrit-Change-Number: 13565
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Wed, 10 Apr 2019 11:06:30 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: gsm_03_40.h: define max SM-TP-UDL (User-Data-Length) values

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has posted comments on this change. ( 
https://gerrit.osmocom.org/13553 )

Change subject: gsm_03_40.h: define max SM-TP-UDL (User-Data-Length) values
..


Patch Set 1: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/13553
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: I54f88d2908ac47228813fb8c049f4264e5145241
Gerrit-Change-Number: 13553
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-Comment-Date: Wed, 10 Apr 2019 11:03:23 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: gsm_03_40.h: define max SM-TP-UDL (User-Data-Length) values

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13553 )

Change subject: gsm_03_40.h: define max SM-TP-UDL (User-Data-Length) values
..

gsm_03_40.h: define max SM-TP-UDL (User-Data-Length) values

As per 3GPP TS 03.40, section 9.2.3.16 "TP-User-Data-Length (TP-UDL)"
field may contain up to 140 octets (or 140 * 8 / 7 = 160 septets).

Change-Id: I54f88d2908ac47228813fb8c049f4264e5145241
---
M include/osmocom/gsm/protocol/gsm_03_40.h
1 file changed, 5 insertions(+), 0 deletions(-)

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



diff --git a/include/osmocom/gsm/protocol/gsm_03_40.h 
b/include/osmocom/gsm/protocol/gsm_03_40.h
index a65203f..33a5c59 100644
--- a/include/osmocom/gsm/protocol/gsm_03_40.h
+++ b/include/osmocom/gsm/protocol/gsm_03_40.h
@@ -3,6 +3,11 @@

 #pragma once

+/* SM-TP-UD (User-Data) field may contain up to 140 octets
+ * (or 140 * 8 / 7 = 160 septets). See section 9.2.3.24. */
+#define GSM340_UDL_OCT_MAX 140  /*!< Maximum TP-UD length (in octets) for 
7-bit encoding */
+#define GSM340_UDL_SPT_MAX 160  /*!< Maximum TP-UD length (in seplets) for 
8-bit and UCS-2 encoding */
+
 /**
  * 9.1.2.5 Type Of Number
  */

--
To view, visit https://gerrit.osmocom.org/13553
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: I54f88d2908ac47228813fb8c049f4264e5145241
Gerrit-Change-Number: 13553
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Vadim Yanitskiy 


Change in osmo-msc[master]: smpp: Make libsmpp34 use talloc for its allocations

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has uploaded a new patch set (#2) to the change originally 
created by Harald Welte. ( https://gerrit.osmocom.org/13562 )

Change subject: smpp: Make libsmpp34 use talloc for its allocations
..

smpp: Make libsmpp34 use talloc for its allocations

We are just introducing smpp34_set_memory_functions() in libsmpp34
to allow applications like OsmoMSC to provide their own heap allocator
callback functions.  Let's used this to integrate with talloc and
hence allow talloc tracking/debugging for libsmpp34 internal
allocations.

Depends: libsmpp34 Change-Id I3656117115e89638c093bfbcbc4369ce302f7a94
Change-Id: Ie2725ffab6a225813e65768735f01678e2022128
Related: OS#3913
---
M src/libmsc/smpp_openbsc.c
1 file changed, 27 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/62/13562/2
--
To view, visit https://gerrit.osmocom.org/13562
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: Ie2725ffab6a225813e65768735f01678e2022128
Gerrit-Change-Number: 13562
Gerrit-PatchSet: 2
Gerrit-Owner: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 


Change in osmo-msc[master]: smpp_smsc: Call destroy_tlv() when using build_tlv()

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13560 )

Change subject: smpp_smsc: Call destroy_tlv() when using build_tlv()
..

smpp_smsc: Call destroy_tlv() when using build_tlv()

The libsmpp34 build_tlv() function is allocating dynamic memory
which we need to release again by calling destroy_tlv().

Change-Id: Iacc74c9948fb10fa79c0dd7b0cb72d4adbefdeed
Closes: OS#3912
---
M src/libmsc/smpp_smsc.c
1 file changed, 7 insertions(+), 2 deletions(-)

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



diff --git a/src/libmsc/smpp_smsc.c b/src/libmsc/smpp_smsc.c
index 2350d84..3bfb81a 100644
--- a/src/libmsc/smpp_smsc.c
+++ b/src/libmsc/smpp_smsc.c
@@ -520,7 +520,9 @@
tlv.value.val16 = esme->smpp_version;
build_tlv(_r.tlv, );

-   return PACK_AND_SEND(esme, _r);
+   rc = PACK_AND_SEND(esme, _r);
+   destroy_tlv(bind_r.tlv);
+   return rc;
 }

 /*! \brief handle an incoming SMPP BIND TRANSCEIVER */
@@ -632,6 +634,7 @@
 {
struct alert_notification_t alert;
struct tlv_t tlv;
+   int rc;

memset(, 0, sizeof(alert));
alert.command_length= 0;
@@ -652,7 +655,9 @@
alert.source_addr_npi,
get_value_string(smpp_avail_strs, avail_status));

-   return PACK_AND_SEND(esme, );
+   rc = PACK_AND_SEND(esme, );
+   destroy_tlv(alert.tlv);
+   return rc;
 }

 /* \brief send a DELIVER-SM message to given ESME */

--
To view, visit https://gerrit.osmocom.org/13560
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: Iacc74c9948fb10fa79c0dd7b0cb72d4adbefdeed
Gerrit-Change-Number: 13560
Gerrit-PatchSet: 3
Gerrit-Owner: Harald Welte 
Gerrit-Assignee: fixeria 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-CC: fixeria 


Change in libsmpp34[master]: configure.ac: enable kernel style compile messages

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has uploaded this change for review. ( 
https://gerrit.osmocom.org/13565


Change subject: configure.ac: enable kernel style compile messages
..

configure.ac: enable kernel style compile messages

Change-Id: I96a5d808e2a199c33a1c9eef77da3a040e305689
---
M configure.ac
1 file changed, 3 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libsmpp34 refs/changes/65/13565/1

diff --git a/configure.ac b/configure.ac
index 8ecbb77..2c974f8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -8,6 +8,9 @@
 AM_INIT_AUTOMAKE([foreign])
 AM_CONFIG_HEADER([aux_config/config.h])

+dnl kernel style compile messages
+m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
+
 dnl include release helper
 RELMAKE='-include osmo-release.mk'
 AC_SUBST([RELMAKE])

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

Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I96a5d808e2a199c33a1c9eef77da3a040e305689
Gerrit-Change-Number: 13565
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy 


Change in libsmpp34[master]: Allow application to override default heap allocator

2019-04-10 Thread Vadim Yanitskiy
Vadim Yanitskiy has uploaded a new patch set (#4) to the change originally 
created by Harald Welte. ( https://gerrit.osmocom.org/13561 )

Change subject: Allow application to override default heap allocator
..

Allow application to override default heap allocator

Let's introduce a mechanism by which libsmpp34-using applications can
override the memory allocator functions.  This allows us e.g. in the
Osmocom context to use talloc which aids us in debugging memory leaks.

Change-Id: I3656117115e89638c093bfbcbc4369ce302f7a94
Closes: OS#3913
---
M TODO-RELEASE
M src/Makefile.am
A src/smpp34_heap.c
A src/smpp34_heap.h
M src/smpp34_params.c
M src/smpp34_unpack.c
6 files changed, 110 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libsmpp34 refs/changes/61/13561/4
--
To view, visit https://gerrit.osmocom.org/13561
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3656117115e89638c093bfbcbc4369ce302f7a94
Gerrit-Change-Number: 13561
Gerrit-PatchSet: 4
Gerrit-Owner: Harald Welte 
Gerrit-Assignee: fixeria 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-CC: fixeria 


  1   2   >