Change in libosmocore[master]: add identifier sanitation for setting FSM instance ids
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()
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
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
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
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
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
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
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()
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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()
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
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
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
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()
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()
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()
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
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
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()
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()
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
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
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
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
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
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
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
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
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
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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)
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
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...
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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()
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
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
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
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
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
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
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
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
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
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()
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
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
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