Change in osmo-hlr[master]: hlr.sql: move comment
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15928 ) Change subject: hlr.sql: move comment .. hlr.sql: move comment Move a comment for ind_bitlen column to a separate line, so that it doesn't show in PRAGMA_TABLE_INFO('subscriber'). An upcoming patch introduces db_upgrade_test, which dumps a sorted db schema. In newer sqlite3 versions, a comment following a 'DEFAULT' keyword actually shows up in the PRAGMA_TABLE_INFO() results (on my machine), but older versions (on the build slaves) drop that comment. The ind_bitlen column is the only one producing this odd side effect, because it is the last column and has no comma between 'DEFAULT' and the comment. The easiest way to get matching results across sqlite3 client versions is to move the comment to above ind_bitlen instead of having it on the same line. (Adding a comma doesn't work.) Change-Id: Id66ad68dd3f22d533fc3a428223ea6ad0282bde0 --- M sql/hlr.sql 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/sql/hlr.sql b/sql/hlr.sql index 10838f2..c1b0f1a 100644 --- a/sql/hlr.sql +++ b/sql/hlr.sql @@ -69,7 +69,8 @@ op VARCHAR(32),-- hex string: operator's secret key (128bit) opc VARCHAR(32),-- hex string: derived from OP and K (128bit) sqn INTEGER NOT NULL DEFAULT 0, -- sequence number of key usage - ind_bitlen INTEGER NOT NULL DEFAULT 5 -- nr of index bits at lower SQN end + -- nr of index bits at lower SQN end + ind_bitlen INTEGER NOT NULL DEFAULT 5 ); CREATE UNIQUE INDEX idx_subscr_imsi ON subscriber (imsi); -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15928 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: Id66ad68dd3f22d533fc3a428223ea6ad0282bde0 Gerrit-Change-Number: 15928 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-MessageType: merged
Change in osmo-hlr[master]: add db_upgrade test
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15913 ) Change subject: add db_upgrade test .. add db_upgrade test We have a database schema upgrade path, but so far nothing that verifies that we don't break it. It almost seems like the user data weren't important to us!? Add a db upgrade test: - Create a db with an .sql dump taken from a db created with an old osmo-hlr, producing DB schema version 0. - Run osmo-hlr --db-upgrade --db-check - Verify that the upgrade exited successfully. - Verify that restarting with the upgraded DB works. If python tests are enabled, also: - create a new database using the new osmo-hlr (just built). - replay a VTY transcript to create subscribers as in the old snapshot. - replay some sql modifications as done in the old snapshot. - Get a list of sorted table names, - a list of their sorted columns with all their properties, - and dump the table contents in a column- and value-sorted way. - Compare the resulting dumps and error if there are any diffs. (This is how I found the difference in the imei column that was fixed in I68a00014a3d603fcba8781470bc5285f78b538d0) Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 --- M configure.ac M tests/Makefile.am A tests/db_upgrade/Makefile.am A tests/db_upgrade/create_subscribers.vty A tests/db_upgrade/create_subscribers_step2.sql A tests/db_upgrade/db_upgrade_test.err A tests/db_upgrade/db_upgrade_test.ok A tests/db_upgrade/db_upgrade_test.sh A tests/db_upgrade/hlr_db_v0.sql A tests/db_upgrade/osmo-hlr.cfg M tests/testsuite.at 11 files changed, 417 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve osmith: Looks good to me, approved diff --git a/configure.ac b/configure.ac index 993d4d5..ca78f38 100644 --- a/configure.ac +++ b/configure.ac @@ -186,4 +186,5 @@ tests/gsup_server/Makefile tests/gsup/Makefile tests/db/Makefile + tests/db_upgrade/Makefile ) diff --git a/tests/Makefile.am b/tests/Makefile.am index 4da8ab1..357fbac 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -3,6 +3,7 @@ gsup_server \ db \ gsup \ + db_upgrade \ $(NULL) # The `:;' works around a Bash 3.2 bug when the output is not writeable. @@ -44,6 +45,7 @@ # don't run vty and ctrl tests concurrently so that the ports don't conflict $(MAKE) vty-test $(MAKE) ctrl-test + $(MAKE) db-upgrade-equivalence-test else python-tests: echo "Not running python-based external tests (determined at configure-time)" @@ -81,6 +83,9 @@ -rm -f $(CTRL_TEST_DB) -rm $(CTRL_TEST_DB)-* +db-upgrade-equivalence-test: + $(MAKE) -C db_upgrade upgrade-equivalence-test + check-local: atconfig $(TESTSUITE) $(SHELL) '$(TESTSUITE)' $(TESTSUITEFLAGS) $(MAKE) $(AM_MAKEFLAGS) python-tests diff --git a/tests/db_upgrade/Makefile.am b/tests/db_upgrade/Makefile.am new file mode 100644 index 000..79136c9 --- /dev/null +++ b/tests/db_upgrade/Makefile.am @@ -0,0 +1,14 @@ +EXTRA_DIST = \ + db_upgrade_test.sh \ + db_upgrade_test.err \ + db_upgrade_test.ok \ + hlr_db_v0.sql \ + osmo-hlr.cfg \ + create_subscribers.vty \ + $(NULL) + +update_exp: + $(srcdir)/db_upgrade_test.sh $(srcdir) $(builddir) >"$(srcdir)/db_upgrade_test.ok" 2>"$(srcdir)/db_upgrade_test.err" + +upgrade-equivalence-test: + $(srcdir)/db_upgrade_test.sh $(srcdir) $(builddir) do-equivalence-test diff --git a/tests/db_upgrade/create_subscribers.vty b/tests/db_upgrade/create_subscribers.vty new file mode 100644 index 000..30eeba6 --- /dev/null +++ b/tests/db_upgrade/create_subscribers.vty @@ -0,0 +1,47 @@ +OsmoHLR> enable +OsmoHLR# subscriber imsi 123456789012345 create +% Created subscriber 123456789012345 +ID: 1 +IMSI: 123456789012345 +MSISDN: none +OsmoHLR# subscriber imsi 123456789012345 update msisdn 098765432109876 +% Updated subscriber IMSI='123456789012345' to MSISDN='098765432109876' +OsmoHLR# subscriber imsi 123456789012345 update aud2g comp128v1 ki BeefedCafeFaceAcedAddedDecadeFee +OsmoHLR# subscriber imsi 123456789012345 update aud3g milenage k C01ffedC1cadaeAc1d1f1edAcac1aB0a opc CededEffacedAceFacedBadFadedBeef +OsmoHLR# subscriber imsi 1 create +% Created subscriber 1 +ID: 2 +IMSI: 1 +MSISDN: none +OsmoHLR# subscriber imsi 2 create +% Created subscriber 2 +ID: 3 +IMSI: 2 +MSISDN: none +OsmoHLR# subscriber imsi 2 update msisdn 2 +% Updated subscriber IMSI='2' to MSISDN='2' +OsmoHLR# subscriber imsi 33 create +% Created subscriber 33 +ID: 4 +IMSI: 33 +MSISDN: none +OsmoHLR# subscriber imsi 33 update msisdn 3 +% Updated subscriber IMSI='33' to MSIS
Change in osmo-hlr[master]: fix upgrade to version 2: imei column default value
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15911 ) Change subject: fix upgrade to version 2: imei column default value .. fix upgrade to version 2: imei column default value A subsequent commit will add a db_upgrade test, which verifies that the db resulting from an upgrade is identical to one created from scratch in the new version. That test currently would show a diff: an upgraded 'imei' column has 'default NULL', where a new db created in version 2 has no default value on the imei column. Fix the upgrade path to add an imei column without 'default NULL', so that adding the upgrade test will result in success. The test is added in I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 Change-Id: I68a00014a3d603fcba8781470bc5285f78b538d0 --- M src/db.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve osmith: Looks good to me, approved diff --git a/src/db.c b/src/db.c index f3ed863..31c4ba5 100644 --- a/src/db.c +++ b/src/db.c @@ -299,7 +299,7 @@ { sqlite3_stmt *stmt; int rc; - const char *update_stmt_sql = "ALTER TABLE subscriber ADD COLUMN imei VARCHAR(14) default NULL"; + const char *update_stmt_sql = "ALTER TABLE subscriber ADD COLUMN imei VARCHAR(14)"; const char *set_schema_version_sql = "PRAGMA user_version = 2"; rc = sqlite3_prepare_v2(dbc->db, update_stmt_sql, -1, , NULL); -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15911 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I68a00014a3d603fcba8781470bc5285f78b538d0 Gerrit-Change-Number: 15911 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-MessageType: merged
Change in osmo-hlr[master]: db upgrade to v2: log version 2, not 1
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15910 ) Change subject: db upgrade to v2: log version 2, not 1 .. db upgrade to v2: log version 2, not 1 Change-Id: I9237b64e5748e693a5f039c5a5554d417eed3633 --- M src/db.c 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved fixeria: Looks good to me, approved laforge: Looks good to me, approved diff --git a/src/db.c b/src/db.c index 5e6b5eb..f3ed863 100644 --- a/src/db.c +++ b/src/db.c @@ -311,7 +311,7 @@ db_remove_reset(stmt); sqlite3_finalize(stmt); if (rc != SQLITE_DONE) { - LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version %d\n", 1); + LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version 2\n"); return rc; } @@ -322,7 +322,7 @@ } rc = sqlite3_step(stmt); if (rc != SQLITE_DONE) - LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version %d\n", 1); + LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version 2\n"); db_remove_reset(stmt); sqlite3_finalize(stmt); -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15910 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I9237b64e5748e693a5f039c5a5554d417eed3633 Gerrit-Change-Number: 15910 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-MessageType: merged
Change in osmo-hlr[master]: add --db-check option
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15912 ) Change subject: add --db-check option .. add --db-check option This allows starting osmo-hlr to merely open the database, do upgrades if necessary, and quit, without opening any ports. So that no ports are opened, move the telnet VTY startup to below the database check. Needed for upcoming patch that introduces a db_upgrade test, in I0961bab0e17cfde5b030576c5bc243c2b51d9dc4. Change-Id: I1a4b3360690acd2cd3cffdadffbb00a28d421316 --- M src/hlr.c 1 file changed, 23 insertions(+), 6 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/hlr.c b/src/hlr.c index 8b9dff1..6bfc141 100644 --- a/src/hlr.c +++ b/src/hlr.c @@ -706,6 +706,7 @@ printf(" -T --timestamp Prefix every log line with a timestamp.\n"); printf(" -e --log-level number Set a global loglevel.\n"); printf(" -U --db-upgradeAllow HLR database schema upgrades.\n"); + printf(" -C --db-check Quit after opening (and upgrading) the database.\n"); printf(" -V --version Print the version of OsmoHLR.\n"); } @@ -714,6 +715,7 @@ const char *db_file; bool daemonize; bool db_upgrade; + bool db_check; } cmdline_opts = { .config_file = "osmo-hlr.cfg", .db_file = NULL, @@ -735,6 +737,7 @@ {"log-level", 1, 0, 'e'}, {"timestamp", 0, 0, 'T'}, {"db-upgrade", 0, 0, 'U' }, + {"db-check", 0, 0, 'C' }, {"version", 0, 0, 'V' }, {0, 0, 0, 0} }; @@ -773,6 +776,9 @@ case 'U': cmdline_opts.db_upgrade = true; break; + case 'C': + cmdline_opts.db_check = true; + break; case 'V': print_version(1); exit(0); @@ -856,12 +862,6 @@ return rc; } - /* start telnet after reading config for vty_get_bind_addr() */ - rc = telnet_init_dynif(hlr_ctx, NULL, vty_get_bind_addr(), - OSMO_VTY_PORT_HLR); - if (rc < 0) - return rc; - LOGP(DMAIN, LOGL_NOTICE, "hlr starting\n"); rc = rand_init(); @@ -879,6 +879,23 @@ exit(1); } + if (cmdline_opts.db_check) { + LOGP(DMAIN, LOGL_NOTICE, "Cmdline option --db-check: Database was opened successfully, quitting.\n"); + db_close(g_hlr->dbc); + log_fini(); + talloc_free(hlr_ctx); + talloc_free(tall_vty_ctx); + talloc_disable_null_tracking(); + exit(0); + } + + /* start telnet after reading config for vty_get_bind_addr() */ + rc = telnet_init_dynif(hlr_ctx, NULL, vty_get_bind_addr(), + OSMO_VTY_PORT_HLR); + if (rc < 0) + return rc; + + g_hlr->gs = osmo_gsup_server_create(hlr_ctx, g_hlr->gsup_bind_addr, OSMO_GSUP_PORT, read_cb, _lu_ops, g_hlr); if (!g_hlr->gs) { -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15912 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I1a4b3360690acd2cd3cffdadffbb00a28d421316 Gerrit-Change-Number: 15912 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: fixeria Gerrit-MessageType: merged
Change in osmo-msc[master]: add sdp_msg API: SDP parsing/composition
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/15947 ) Change subject: add sdp_msg API: SDP parsing/composition .. Patch Set 1: (3 comments) https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/include/osmocom/msc/debug.h File include/osmocom/msc/debug.h: https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/include/osmocom/msc/debug.h@38 PS1, Line 38: size_t len = 64; \ > Makes much more sense having len as a param to avoid almost-sure > talloc_realloc on some functions. you're right https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/include/osmocom/msc/debug.h@40 PS1, Line 40: char *str = (char*)talloc_named_const(ctx, len, #FUNC_BUF "_c()"); \ > where does this "ctx" var come from? it's a required parameter indicated above as foo_name_c(void *ctx, But hold on, I actually moved this to libosmocore in a more generalized fashion, and forgot to apply that here. Ah I remember now, I also needed this in osmo-hlr for the Distributed GSM task. I'll submit that libosmocore patch first and use that here as well. I've taken your points into consideration and submitted https://gerrit.osmocom.org/c/libosmocore/+/15957 https://gerrit.osmocom.org/c/osmo-msc/+/15947/1/include/osmocom/msc/debug.h@63 PS1, Line 63: #define NAME_IMPL(FUNC_C, FUNC_C_ARGS...) \ > So you return a talloc'ed pointer on success and a static string pointer on > error? That will break w […] the point being that I want to use this function in printf() formats, and there returning NULL is a crash. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15947 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 Gerrit-Change-Number: 15947 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Comment-Date: Mon, 04 Nov 2019 23:59:56 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in libosmocore[master]: add osmo_sockaddr_str_cmp() and _str_ip_cmp()
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/15961 to look at the new patch set (#4). Change subject: add osmo_sockaddr_str_cmp() and _str_ip_cmp() .. add osmo_sockaddr_str_cmp() and _str_ip_cmp() Currently planned user: for Distributed GSM in osmo-hlr: setting per-MSC service addresses in VTY: replace/remove existing entries. osmo_sockaddr_str_cmp() is useful to sort existing entries, while showing possible duplicates evaluating to the same actual IP address bytes. osmo_sockaddr_str_ip_cmp() is useful to catch identical resulting IP addresses, regardless of differing strings (e.g. '0::' and '::' are equal but differ in strings). Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501 --- M include/osmocom/core/sockaddr_str.h M include/osmocom/core/utils.h M src/sockaddr_str.c M tests/sockaddr_str/sockaddr_str_test.c M tests/sockaddr_str/sockaddr_str_test.ok 5 files changed, 1,263 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/61/15961/4 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15961 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501 Gerrit-Change-Number: 15961 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset
Change in osmo-ttcn3-hacks[master]: msc: overhaul voice call testing
Hello pespin, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938 to look at the new patch set (#2). Change subject: msc: overhaul voice call testing .. msc: overhaul voice call testing * Semantic: We don't really know which side the MSC first creates a CRCX for. Instead of assuming that the RAN side is always CRCX'd first, simply handle a "first" and a "second" CRCX, not making any assumptions which is for which side. Notably, there still are quite a few places assuming which CRCX corresponds to the RAN/CN side, but the changes are sufficient to still pass the tests when osmo-msc swaps the CRCX order; sometimes for slightly obscure reasons, for example because it doesn't matter that the wrong port number is returned during a subsequent MDCX... Cleaning up the rest is still todo for later. Remove code dup from call establishing code, particularly for MGCP. Use f_mo_call_establish() and f_mt_call() where ever possible, to make all of the call establishing tests handle upcoming changes in osmo-msc's order of messages, without re-implementing the changes for each test individually. The X-Osmux parameter was so far expected to appear in the first CRCX received, assuming that this first CRCX is for the RAN. Instead, detect whether X-Osmux is contained in a CRCX, and reply with an Osmux CID if so, regardless of it being the first or second CRCX. Count the number of X-Osmux parameters received in CRCX messages, and compare after call setup to verify X-Osmux presence. Since f_mo_call_establish() can't handle RANAP assignment, a few Iu tests that worked with the older code dup will break by this patch. This is fixed by a subsequent patch, see I0ead36333ab665147b8d222070ea5cf8afc555ec. * Details, per patch chunk: Change ts_BSSMAP_IE_AoIP_TLA4 to a non-value template, so that we can use a wildcard for the assigned port number from MGCP (depending on RAN or CN CRCX first, the RAN port number can be one or the other). In CallParameters, move MGCP handling instructions into a separate record "CrcxResponse", and have two of them for handling the first and the second CRCX, replacing mgw_rtp_{ip,port}_{bss,mss} and mgcp_connection_id_{bss,mss}. In CallParameters, add some flags for early-exiting call establishment with a particular desired behavior, for specialized tests. In CallParameters, use common default values and don't repeat them in each and every call establishing test. Set cpars.mo_call := {true,false} implicitly when f_{mo,mt}_call_establish() are invoked. Remove CRCX comments implying RAN or CN side, instead just talk of the "first" and the "second" CRCX. Implement one common f_handle_crcx() function, which is used by f_mo_call_establish(), f_mt_call_complete(), as_optional_mgcp_crcx(), and implicitly uses the first/second CRCX handling. For Assigment, use a wildcard RTP port so that we don't have to assume which CRCX was for the RAN side. In f_mo_call_establish(), insert special case conditions to make it enact errors at specific times, for individual tests. That saves re-implementing the entire call establishment (code dup). For error cases, add expectation of a CC Release message in the call establishment. This should not apply for normal successful operation, but because interleave does not support conditionals, add flags got_mncc_setup_compl_ind and got_cc_connect to break the interleave when establishing is complete, so that the CC Release is skipped. A CC Release always breaks the interleave, at whatever time it arrives. Tests adopting f_{mo,mt}_call instead of code dup: f_tc_mo_setup_and_nothing() f_tc_mo_crcx_ran_timeout() f_tc_mo_crcx_ran_reject() f_tc_mo_release_timeout() f_tc_mo_cc_bssmap_clear() Change-Id: I8b82476f55a98f7a94d5c4f1cd80eac427b2d20f --- M library/BSSMAP_Templates.ttcn M msc/BSC_ConnectionHandler.ttcn M msc/MSC_Tests.ttcn 3 files changed, 282 insertions(+), 338 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/38/15938/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I8b82476f55a98f7a94d5c4f1cd80eac427b2d20f Gerrit-Change-Number: 15938 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-MessageType: newpatchset
Change in osmo-ttcn3-hacks[master]: re-implement compare-results.sh as compare-results.py
Hello laforge, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15943 to look at the new patch set (#2). Change subject: re-implement compare-results.sh as compare-results.py .. re-implement compare-results.sh as compare-results.py The compare-results.sh is annoyingly slow. Since our ttcn3 tests containers support Python 2, re-implement in Python for much quicker evaluation. Change-Id: I0747c9d66ffc7e4121497a2416fca78d7b56c8e6 --- A compare-results.py D compare-results.sh M start-testsuite.sh 3 files changed, 138 insertions(+), 214 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/43/15943/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15943 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I0747c9d66ffc7e4121497a2416fca78d7b56c8e6 Gerrit-Change-Number: 15943 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in osmo-ttcn3-hacks[master]: msc: split off f_mgcp_find_param_entry()
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15963 ) Change subject: msc: split off f_mgcp_find_param_entry() .. msc: split off f_mgcp_find_param_entry() Split f_mgcp_find_param_entry() out of f_mgcp_find_param() to be able to act on an MgcpParameterList without an enclosing MgcpMessage. Will be used by upcoming I8b82476f55a98f7a94d5c4f1cd80eac427b2d20f Change-Id: I90f213d2a1be979afa024e0faa25d532f9858636 --- M library/MGCP_Templates.ttcn 1 file changed, 12 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/63/15963/1 diff --git a/library/MGCP_Templates.ttcn b/library/MGCP_Templates.ttcn index dae379e..e03fd8e 100644 --- a/library/MGCP_Templates.ttcn +++ b/library/MGCP_Templates.ttcn @@ -453,6 +453,17 @@ sdp := * } + function f_mgcp_find_param_entry(MgcpParameterList pars, MgcpInfoCode code, out charstring ret) + return boolean { + for (var integer i := 0; i < sizeof(pars); i := i+1) { + if (pars[i].code == code) { + ret := pars[i].val; + return true; + } + } + return false; + } + function f_mgcp_find_param(MgcpMessage msg, MgcpInfoCode code, out charstring ret) return boolean { var MgcpParameterList pars; @@ -461,13 +472,7 @@ } else { pars := msg.response.params; } - for (var integer i := 0; i < sizeof(pars); i := i+1) { - if (pars[i].code == code) { - ret := pars[i].val; - return true; - } - } - return false; + return f_mgcp_find_param_entry(pars, code, ret); } /* template to determine if a MGCP endpoint is a wildcard endpoint */ -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15963 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I90f213d2a1be979afa024e0faa25d532f9858636 Gerrit-Change-Number: 15963 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-hlr[master]: db upgrade: remove some code dup
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15964 ) Change subject: db upgrade: remove some code dup .. db upgrade: remove some code dup Instead of a switch() for each version number with identical switch cases except for the function name, use an array of function pointers and loop. Also print a success message after each individual version upgrade, instead of only one in the end (see change in db_upgrade_test.ok). Change-Id: I1736af3d9a3f02e29db836966ac15ce49f94737b --- M src/db.c M tests/db_upgrade/db_upgrade_test.ok 2 files changed, 16 insertions(+), 30 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/64/15964/1 diff --git a/src/db.c b/src/db.c index 75ca889..93771bd 100644 --- a/src/db.c +++ b/src/db.c @@ -423,6 +423,13 @@ return rc; } +typedef int (*db_upgrade_func_t)(struct db_context *dbc); +static db_upgrade_func_t db_upgrade_path[] = { + db_upgrade_v1, + db_upgrade_v2, + db_upgrade_v3, +}; + static int db_get_user_version(struct db_context *dbc) { const char *user_version_sql = "PRAGMA user_version"; @@ -534,40 +541,17 @@ LOGP(DDB, LOGL_NOTICE, "Database '%s' has HLR DB schema version %d\n", dbc->fname, version); if (version < CURRENT_SCHEMA_VERSION && allow_upgrade) { - switch (version) { - case 0: - rc = db_upgrade_v1(dbc); + for (;version < ARRAY_SIZE(db_upgrade_path); version++) { + db_upgrade_func_t upgrade_func = db_upgrade_path[version]; + rc = upgrade_func(dbc); if (rc != SQLITE_DONE) { - LOGP(DDB, LOGL_ERROR, "Failed to upgrade HLR DB schema to version 1: (rc=%d) %s\n", -rc, sqlite3_errmsg(dbc->db)); + LOGP(DDB, LOGL_ERROR, "Failed to upgrade HLR DB schema to version %d: (rc=%d) %s\n", +version+1, rc, sqlite3_errmsg(dbc->db)); goto out_free; } - version = 1; - /* fall through */ - case 1: - rc = db_upgrade_v2(dbc); - if (rc != SQLITE_DONE) { - LOGP(DDB, LOGL_ERROR, "Failed to upgrade HLR DB schema to version 2: (rc=%d) %s\n", -rc, sqlite3_errmsg(dbc->db)); - goto out_free; - } - version = 2; - /* fall through */ - case 2: - rc = db_upgrade_v3(dbc); - if (rc != SQLITE_DONE) { - LOGP(DDB, LOGL_ERROR, "Failed to upgrade HLR DB schema to version 3: (rc=%d) %s\n", -rc, sqlite3_errmsg(dbc->db)); - goto out_free; - } - version = 3; - /* fall through */ - /* case N: ... */ - default: - break; + LOGP(DDB, LOGL_NOTICE, "Database '%s' has been upgraded to HLR DB schema version %d\n", +dbc->fname, version+1); } - LOGP(DDB, LOGL_NOTICE, "Database '%s' has been upgraded to HLR DB schema version %d\n", -dbc->fname, version); } if (version != CURRENT_SCHEMA_VERSION) { diff --git a/tests/db_upgrade/db_upgrade_test.ok b/tests/db_upgrade/db_upgrade_test.ok index c1f0f9d..49e7151 100644 --- a/tests/db_upgrade/db_upgrade_test.ok +++ b/tests/db_upgrade/db_upgrade_test.ok @@ -80,6 +80,8 @@ DMAIN hlr starting DDB using database: test.db DDB Database test.db' has HLR DB schema version 0 +DDB Database test.db' has been upgraded to HLR DB schema version 1 +DDB Database test.db' has been upgraded to HLR DB schema version 2 DDB Database test.db' has been upgraded to HLR DB schema version 3 DMAIN Cmdline option --db-check: Database was opened successfully, quitting. -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15964 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I1736af3d9a3f02e29db836966ac15ce49f94737b Gerrit-Change-Number: 15964 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-hlr[master]: hlr db schema 3: hlr_number -> msc_number
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15914 ) Change subject: hlr db schema 3: hlr_number -> msc_number .. Patch Set 6: I'm still considering whether we should instead just ignore that misnamed column... -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15914 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I527e8627b24b79f3e9eec32675c7f5a3a6d25440 Gerrit-Change-Number: 15914 Gerrit-PatchSet: 6 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Comment-Date: Tue, 05 Nov 2019 01:05:35 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-msc[master]: BSSMAP: decode Codec List (BSS Supported)
Hello pespin, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-msc/+/15946 to look at the new patch set (#2). Change subject: BSSMAP: decode Codec List (BSS Supported) .. BSSMAP: decode Codec List (BSS Supported) Actually decode the Codec List (BSS Supported) in BSSMAP, in both the Complete Layer 3 Information and the Assignment Complete messages. An upcoming patch improves codec negotiation and requires the BSS supported codecs, which are so far ignored (which is/was a pity as osmo-bsc goes at great lengths to compose those IEs). Change-Id: I66c735c79e982388f06b5de783aa584c9d13569e --- M include/osmocom/msc/ran_msg.h M src/libmsc/ran_msg_a.c 2 files changed, 32 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/46/15946/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15946 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I66c735c79e982388f06b5de783aa584c9d13569e Gerrit-Change-Number: 15946 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in osmo-msc[master]: cc trans: remove unused tch_rtp_create
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15962 ) Change subject: cc trans: remove unused tch_rtp_create .. cc trans: remove unused tch_rtp_create Use of this flag was dropped when adding inter-BSC and inter-MSC Handover support, I forgot to remove it. Change-Id: I5ec78e30eb36fbe78a3f7c46bfa44af5a4eb7bf2 --- M include/osmocom/msc/transaction.h M src/libmsc/gsm_04_08_cc.c 2 files changed, 0 insertions(+), 18 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/62/15962/1 diff --git a/include/osmocom/msc/transaction.h b/include/osmocom/msc/transaction.h index 61d8c1a..928b137 100644 --- a/include/osmocom/msc/transaction.h +++ b/include/osmocom/msc/transaction.h @@ -87,10 +87,6 @@ /* bearer capabilities (rate and codec) */ struct gsm_mncc_bearer_cap bearer_cap; - /* if true, TCH_RTP_CREATE is sent after the -* assignment is done */ - bool tch_rtp_create; - union { struct { diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c index 792ab61..7e2d70b 100644 --- a/src/libmsc/gsm_04_08_cc.c +++ b/src/libmsc/gsm_04_08_cc.c @@ -1655,20 +1655,6 @@ } 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. -* msc_mgcp_call_assignment() also takes care about sending the CRCX -* command to the MGCP-GW. The CRCX will return the port number, -* where the PBX (e.g. Asterisk) will send its RTP stream to. We -* have to return this port number back to the MNCC by sending -* it back with the TCH_RTP_CREATE message. To make sure that -* this message is sent AFTER the response to CRCX from the -* MGCP-GW has arrived, we need will instruct msc_mgcp_call_assignment() -* to take care of this by setting trans->tch_rtp_create to true. -* This will make sure that gsm48_tch_rtp_create() (below) is -* called as soon as the local port number has become known. */ - trans->tch_rtp_create = true; - /* Assign call (if not done yet) */ return msc_a_try_call_assignment(trans); } -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15962 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I5ec78e30eb36fbe78a3f7c46bfa44af5a4eb7bf2 Gerrit-Change-Number: 15962 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-msc[master]: add sdp_msg API: SDP parsing/composition
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-msc/+/15947 to look at the new patch set (#2). Change subject: add sdp_msg API: SDP parsing/composition .. add sdp_msg API: SDP parsing/composition Rationale: in order to add full SDP to the MNCC protocol (upcoming patch I8c3b2de53ffae4ec3a66b9dabf308c290a2c999f), we need to parse and compose SDP messages. Obviously, libosmo-mgcp-client already contains similar code, but that is unfortunately heavily glued to the actual MGCP implementation. The simplest solution is to create this separate implementation, copy-pasting from the existing libosmo-mgcp-client code as is convenient. Various foo_name() functions are implemented for the sdp_msg API. Each is: a) implemented as foo_name_buf(buf, len, val), b) wrapped as foo_name_c(ctx, val) c) and as foo_name(val), a convienience variant using OTC_SELECT. (a) foo_name_buf() uses osmo_strbuf to write to a fixed-size caller provided buffer: osmo_strbuf is most convenient to implement optional parts / loops. (b) foo_name_c() uses a caller-provided talloc ctx to allocate such buffer; implemented using a generalized NAME_C_IMPL(func_buf, arg) macro, which calls func with an initial size buffer, and reallocates if more space is needed. (c) foo_name() then calls foo_name_c() with the OTC_SELECT context (we know that msc_main.c uses osmo_select_main_ctx()), implemented using a generalized NAME_IMPL() macro, and returns "ERROR" on failure instead of NULL. Change-Id: If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 --- M configure.ac M include/osmocom/msc/Makefile.am A include/osmocom/msc/sdp_msg.h M src/libmsc/Makefile.am A src/libmsc/sdp_msg.c M tests/Makefile.am A tests/sdp_msg/Makefile.am A tests/sdp_msg/sdp_msg_test.c A tests/sdp_msg/sdp_msg_test.err A tests/sdp_msg/sdp_msg_test.ok M tests/testsuite.at 11 files changed, 1,846 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/47/15947/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15947 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 Gerrit-Change-Number: 15947 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in osmo-hlr[master]: db upgrade: remove some code dup
neels has uploaded a new patch set (#2). ( https://gerrit.osmocom.org/c/osmo-hlr/+/15964 ) Change subject: db upgrade: remove some code dup .. db upgrade: remove some code dup Instead of a switch() for each version number with identical switch cases except for the function name, use an array of function pointers and loop. Also print a success message after each individual version upgrade, instead of only one in the end (see change in db_upgrade_test.ok). Change-Id: I1736af3d9a3f02e29db836966ac15ce49f94737b --- M src/db.c M tests/db_upgrade/db_upgrade_test.ok 2 files changed, 17 insertions(+), 33 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/64/15964/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15964 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I1736af3d9a3f02e29db836966ac15ce49f94737b Gerrit-Change-Number: 15964 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-CC: Jenkins Builder Gerrit-MessageType: newpatchset
Change in osmo-ttcn3-hacks[master]: msc: add and fix Iu mt call
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15940 ) Change subject: msc: add and fix Iu mt call .. msc: add and fix Iu mt call Change-Id: I3ce29f3d9254656dc295674e8cec72a741b7764a --- M msc/BSC_ConnectionHandler.ttcn M msc/MSC_Tests.ttcn M msc/MSC_Tests_Iu.ttcn 3 files changed, 88 insertions(+), 11 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/40/15940/1 diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn index b11d24b..63c64bb 100644 --- a/msc/BSC_ConnectionHandler.ttcn +++ b/msc/BSC_ConnectionHandler.ttcn @@ -905,21 +905,89 @@ } } else { - var template TransportLayerAddress rab_tla := ? /* FIXME: encode the mgw_rtp_ip/mgw_rtp_port */ - var template RAB_SetupOrModifyList rab_sml := tr_RAB_SML(rab_id := ?, tla := rab_tla, binding_id := ?); - interleave { [] MGCP.receive(tr_CRCX(cpars.mgcp_ep)) -> value mgcp_cmd { - var SDP_Message sdp := valueof(ts_SDP_CRCX_CN(cpars)); - MGCP.send(ts_CRCX_ACK(mgcp_cmd.line.trans_id, cpars.mgw_conn_2.mgcp_connection_id, sdp)); - /* MSC acknowledges the MNCC_CREATE to the MNCC handler */ - MNCC.receive(tr_MNCC_RTP_CREATE(cpars.mncc_callref)); + log("f_mt_call_complete 4.iu"); + if (not f_handle_crcx(cpars, mgcp_cmd)) { + break; } - [] BSSAP.receive(tr_RANAP_RabAssReq(rab_sml)) { - //BSSAP.send(ts_RANAP_RabAssResp(rab_sml)); FIXME } - /* FIXME: same MNCC and MGCP as in 2G above */ + /* MSC acknowledges the MNCC_CREATE to the MNCC handler */ + [] MNCC.receive(tr_MNCC_RTP_CREATE(cpars.mncc_callref)) { + log("f_mt_call_complete 5.iu"); + } + + [] BSSAP.receive(tr_RANAP_RabAssReq(?)) { + log("f_mt_call_complete 6.iu"); + var RAB_SetupOrModifiedList l := { + { + { + id := id_RAB_SetupOrModifiedItem, + criticality := ignore, + value_ := { + rAB_SetupOrModifiedItem := { + rAB_ID := int2bit(23, 8), + transportLayerAddress := hex2bit( '350001c0a8021500'H), + iuTransportAssociation := { + bindingID := '040c'O + }, + dl_dataVolumes := omit, + iE_Extensions := omit + } + } + } + } + }; + BSSAP.send(ts_RANAP_RabAssResp(l)); + + BSSAP.send(ts_PDU_DTAP_MO(ts_ML3_MO_CC_CONNECT(cpars.transaction_id))); + } + + [] MNCC.receive(tr_MNCC_SETUP_cnf(cpars.mncc_callref)) { + log("f_mt_call_complete 7.iu"); + MNCC.send(ts_MNCC_RTP_CONNECT(cpars.mncc_callref, + /* ip 42.23.11.5 */ hex2int('42231105'H), + /* port 423 */ 423, + /* payload type 3 = GSM FR */ 3)); + } + + /* MDCX setting up the RAN side remote RTP address received from Assignment Complete */ + [] MGCP.receive(tr_MDCX) -> value mgcp_cmd { + log("f_mt_call_complete 8.iu"); + var SDP_Message sdp := valueof(ts_SDP(cpars.mgw_conn_2.mgw_rtp_ip, cpars.mgw_conn_2.mgw_rtp_ip, + hex2str(cpars.mgcp_call_id), "42", + cpars.mgw_conn_2.mgw_rtp_port, + { int2str(cpars.rtp_payload_type) }, +
Change in osmo-ttcn3-hacks[master]: improve/fix f_tc_mo_setup_dtmf_dup
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15936 ) Change subject: improve/fix f_tc_mo_setup_dtmf_dup .. improve/fix f_tc_mo_setup_dtmf_dup - Fix error log for a missing final DTMF. - Instead of code dup to establish a call, use f_mo_call_establish(). This will make the test benefit from changes to f_mo_call_establish() (which will soon come up to accomodate changes in osmo-msc's codec negotiation). - Instead of hardcoding the expected N_SD counter values to detect DTAP duplicates, use f_bssmap_last_n_sd() and f_next_n_sd(), so that the N_SD counter is correct regardless of how many DTAP were sent in f_mo_call_establish(). - Instead of hardcoding a correct N_SD in the end, use skip_seq_patching == false, so that the ttcn DTAP correctly tracks N_SD for subsequent call release messages. - Release the call. Change-Id: Ibfa8b906764f2d5ed75fe74125be42af4546e864 --- M msc/BSC_ConnectionHandler.ttcn M msc/MSC_Tests.ttcn 2 files changed, 26 insertions(+), 16 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/36/15936/1 diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn index 5b0a125..0ddc911 100644 --- a/msc/BSC_ConnectionHandler.ttcn +++ b/msc/BSC_ConnectionHandler.ttcn @@ -1273,50 +1273,58 @@ var MgcpCommand mgcp_cmd; var template PDU_ML3_MS_NW dtmf_dtap; - f_establish_fully(); + f_mo_call_establish(cpars); - /* Create MNCC and MGCP expect */ - f_create_mncc_expect(hex2str(cpars.called_party)); - f_create_mgcp_expect(ExpectCriteria:{omit,omit,omit}); + /* Send DTMF: send the exact same DTAP message twice, the dup should be filtered out by +* 3GPP TS 24.007 11.2.3.2 Message Type Octet / Duplicate Detection. */ - BSSAP.send(ts_PDU_DTAP_MO(ts_ML3_MO_CC_SETUP(cpars.transaction_id, cpars.called_party))); - MNCC.receive(tr_MNCC_SETUP_ind(?, tr_MNCC_number(hex2str(cpars.called_party -> value mncc; - cpars.mncc_callref := mncc.u.signal.callref; + /* Find out the next NSD that will be used, from RAN emulation. */ + var N_Sd_Array last_n_sd := f_bssmap_last_n_sd(); + var uint2_t next_n_sd := f_next_n_sd(last_n_sd, 0 /* cc is index 0 */); - /* Send DTMF */ + /* Compose DTAP with this correct NSD */ dtmf_dtap := ts_ML3_MO_CC_START_DTMF(cpars.transaction_id, "2"); - dtmf_dtap.msgs.cc.startDTMF.nsd := int2bit(2, 2); - BSSAP.send(ts_PDU_DTAP_MO(dtmf_dtap, '00'O, true)); + + /* Here, pass skip_seq_patching == false so that the RAN Emulation NSD increments after this message. */ + BSSAP.send(ts_PDU_DTAP_MO(dtmf_dtap, '00'O, false)); T.start; alt { - [] MNCC.receive(tr_MNCC_START_DTMF_ind(cpars.mncc_callref, "2")) {} + [] MNCC.receive(tr_MNCC_START_DTMF_ind(cpars.mncc_callref, "2")) { + log("f_mo_seq_dtmf_dup() 1: got first START_DTMF_ind"); + } [] T.timeout { setverdict(fail, "Timeout waiting for START_DTMF_ind"); mtc.stop; } } + /* Send the exact same DTAP with above NSD, which is now incorrect (has not incremented), so that this message +* will get filtered by the duplicate detection. Write NSD into DTAP and pass skip_seq_patching == true. */ + dtmf_dtap.msgs.cc.startDTMF.nsd := int2bit(next_n_sd, 2); BSSAP.send(ts_PDU_DTAP_MO(dtmf_dtap, '00'O, true)); T.start; alt { [] MNCC.receive(tr_MNCC_START_DTMF_ind(cpars.mncc_callref, "2")) { - setverdict(fail, "Received duplicate START_DTMF_ind"); + setverdict(fail, "f_mo_seq_dtmf_dup() 2: Received duplicate START_DTMF_ind"); mtc.stop; } [] T.timeout { } } + /* Here the NSD should be correct again and we see a DTMF. */ dtmf_dtap := ts_ML3_MO_CC_START_DTMF(cpars.transaction_id, "3"); - dtmf_dtap.msgs.cc.startDTMF.nsd := int2bit(3, 2); - BSSAP.send(ts_PDU_DTAP_MO(dtmf_dtap, '00'O, true)); + BSSAP.send(ts_PDU_DTAP_MO(dtmf_dtap, '00'O, false)); alt { - [] MNCC.receive(tr_MNCC_START_DTMF_ind(cpars.mncc_callref, "3")) { } + [] MNCC.receive(tr_MNCC_START_DTMF_ind(cpars.mncc_callref, "3")) { + log("f_mo_seq_dtmf_dup() 3: got second START_DTMF_ind"); + } [] T.timeout { - setverdict(fail, "Received duplicate START_DTMF_ind"); + setverdict(fail, "Timeout waiting for final START_DTMF_ind"); mtc.stop; } } + f_call_hangup(cpars, true); setverdict(pass); } diff --g
Change in osmo-ttcn3-hacks[master]: msc: overhaul voice call testing
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938 ) Change subject: msc: overhaul voice call testing .. msc: overhaul voice call testing * Semantic: We don't really know which side the MSC first creates a CRCX for. Instead of assuming that the RAN side is always CRCX'd first, simply handle a "first" and a "second" CRCX, not making any assumptions which is for which side. Notably, there still are quite a few places assuming which CRCX corresponds to the RAN/CN side, but the changes are sufficient to still pass the tests when osmo-msc swaps the CRCX order; sometimes for slightly obscure reasons, for example because it doesn't matter that the wrong port number is returned during a subsequent MDCX... Cleaning up the rest is still todo for later. Remove code dup from call establishing code, particularly for MGCP. Use f_mo_call_establish() and f_mt_call() where ever possible, to make all of the call establishing tests handle upcoming changes in osmo-msc's order of messages, without re-implementing the changes for each test individually. The X-Osmux parameter was so far expected to appear in the first CRCX received, assuming that this first CRCX is for the RAN. Instead, detect whether X-Osmux is contained in a CRCX, and reply with an Osmux CID if so, regardless of it being the first or second CRCX. Count the number of X-Osmux parameters received in CRCX messages, and compare after call setup to verify X-Osmux presence. Since f_mo_call_establish() can't handle RANAP assignment, a few Iu tests that worked with the older code dup will break by this patch. This is fixed by a subsequent patch, see I0ead36333ab665147b8d222070ea5cf8afc555ec. * Details, per patch chunk: Change ts_BSSMAP_IE_AoIP_TLA4 to a non-value template, so that we can use a wildcard for the assigned port number from MGCP (depending on RAN or CN CRCX first, the RAN port number can be one or the other). Split f_mgcp_find_param_entry() out of f_mgcp_find_param() to be able to act on an MgcpParameterList without an enclosing MgcpMessage. In CallParameters, move MGCP handling instructions into a separate record "CrcxResponse", and have two of them for handling the first and the second CRCX, replacing mgw_rtp_{ip,port}_{bss,mss} and mgcp_connection_id_{bss,mss}. In CallParameters, add some flags for early-exiting call establishment with a particular desired behavior, for specialized tests. In CallParameters, use common default values and don't repeat them in each and every call establishing test. Set cpars.mo_call := {true,false} implicitly when f_{mo,mt}_call_establish() are invoked. Remove CRCX comments implying RAN or CN side, instead just talk of the "first" and the "second" CRCX. Implement one common f_handle_crcx() function, which is used by f_mo_call_establish(), f_mt_call_complete(), as_optional_mgcp_crcx(), and implicitly uses the first/second CRCX handling. For Assigment, use a wildcard RTP port so that we don't have to assume which CRCX was for the RAN side. In f_mo_call_establish(), insert special case conditions to make it enact errors at specific times, for individual tests. That saves re-implementing the entire call establishment (code dup). For error cases, add expectation of a CC Release message in the call establishment. This should not apply for normal successful operation, but because interleave does not support conditionals, add flags got_mncc_setup_compl_ind and got_cc_connect to break the interleave when establishing is complete, so that the CC Release is skipped. A CC Release always breaks the interleave, at whatever time it arrives. Tests adopting f_{mo,mt}_call instead of code dup: f_tc_mo_setup_and_nothing() f_tc_mo_crcx_ran_timeout() f_tc_mo_crcx_ran_reject() f_tc_mo_release_timeout() f_tc_mo_cc_bssmap_clear() Change-Id: I8b82476f55a98f7a94d5c4f1cd80eac427b2d20f --- M library/BSSMAP_Templates.ttcn M library/MGCP_Templates.ttcn M msc/BSC_ConnectionHandler.ttcn M msc/MSC_Tests.ttcn 4 files changed, 294 insertions(+), 345 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/38/15938/1 diff --git a/library/BSSMAP_Templates.ttcn b/library/BSSMAP_Templates.ttcn index 41755db..b40ec61 100644 --- a/library/BSSMAP_Templates.ttcn +++ b/library/BSSMAP_Templates.ttcn @@ -407,15 +407,15 @@ return cic; } -template (value) BSSMAP_IE_AoIP_TransportLayerAddress ts_BSSMAP_IE_AoIP_TLA(BSSMAP_FIELD_IPAddress addr, - uint16_t udp_port, +template BSSMAP_IE_AoIP_TransportLayerAddress ts_BSSMAP_IE_AoIP_TLA(BSSMAP_FIELD_IPAddress addr, + template uint16_t udp_port, in
Change in osmo-msc[master]: msc_vlr_test_call.c: add MNCC logging
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15950 ) Change subject: msc_vlr_test_call.c: add MNCC logging .. msc_vlr_test_call.c: add MNCC logging Change-Id: I03b25c134553c620d3fa9d23a67ad39414546861 --- M tests/msc_vlr/msc_vlr_test_call.c M tests/msc_vlr/msc_vlr_test_call.err 2 files changed, 41 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/50/15950/1 diff --git a/tests/msc_vlr/msc_vlr_test_call.c b/tests/msc_vlr/msc_vlr_test_call.c index 392d38d..dfa3141 100644 --- a/tests/msc_vlr/msc_vlr_test_call.c +++ b/tests/msc_vlr/msc_vlr_test_call.c @@ -25,11 +25,13 @@ #include -static void mncc_sends_to_cc(uint32_t msg_type, struct gsm_mncc *mncc) -{ - mncc->msg_type = msg_type; - mncc_tx_to_cc(net, mncc); -} +#define mncc_sends_to_cc(MSG_TYPE, MNCC) do { \ + (MNCC)->msg_type = MSG_TYPE; \ + log("MSC <-- MNCC: callref 0x%x: %s\n%s", (MNCC)->callref, \ + get_mncc_name((MNCC)->msg_type), \ + (MNCC)->sdp); \ + mncc_tx_to_cc(net, MNCC); \ + } while(0) /* static void on_call_release_mncc_sends_to_cc(uint32_t msg_type, struct gsm_mncc *mncc) diff --git a/tests/msc_vlr/msc_vlr_test_call.err b/tests/msc_vlr/msc_vlr_test_call.err index 7f9940b..07e10a5 100644 --- a/tests/msc_vlr/msc_vlr_test_call.err +++ b/tests/msc_vlr/msc_vlr_test_call.err @@ -297,6 +297,8 @@ MSC --> MNCC: callref 0x8001: MNCC_SETUP_IND DREF msc_a(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ){MSC_A_ST_COMMUNICATING}: - rx_from_ms: now used by 1 (cc) - MNCC replies with MNCC_RTP_CREATE, causing MGW endpoint CRCX to RAN + MSC <-- MNCC: callref 0x8001: MNCC_RTP_CREATE + DMNCC trans(CC:INITIATED IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) rx MNCC_RTP_CREATE DIUCS msc_a(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ){MSC_A_ST_COMMUNICATING}: Starting call assignment DCC call_leg(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ){ESTABLISHING}: Allocated @@ -328,6 +330,8 @@ DMNCC trans(CC:INITIATED IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) tx MNCC_RTP_CREATE MSC --> MNCC: callref 0x8001: MNCC_RTP_CREATE - MNCC says that's fine + MSC <-- MNCC: callref 0x8001: MNCC_CALL_PROC_REQ + DMNCC trans(CC:INITIATED IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) rx MNCC_CALL_PROC_REQ DCC trans(CC:INITIATED IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) stopping pending guard timer DCC trans(CC:INITIATED IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) starting guard timer with 180 seconds @@ -341,6 +345,8 @@ DIUCS msc_a(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ){MSC_A_ST_COMMUNICATING}: Assignment for this trans already started earlier - Total time passed: 1.23 s - The other call leg got established (not shown here), MNCC tells us so + MSC <-- MNCC: callref 0x8001: MNCC_ALERT_REQ + DMNCC trans(CC:MO_CALL_PROC IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) rx MNCC_ALERT_REQ DCC trans(CC:MO_CALL_PROC IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) stopping pending guard timer DCC trans(CC:MO_CALL_PROC IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) starting guard timer with 180 seconds @@ -351,6 +357,8 @@ - DTAP --UTRAN-Iu--> MS: GSM48_MT_CC_ALERTING: 8301 - DTAP matches expected message DMSC dummy_msc_i(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ){0}: Received Event MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST + MSC <-- MNCC: callref 0x8001: MNCC_SETUP_RSP + DMNCC trans(CC:CALL_DELIVERED IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) rx MNCC_SETUP_RSP DCC trans(CC:CALL_DELIVERED IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) stopping pending guard timer DCC trans(CC:CALL_DELIVERED IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) starting guard timer with 180 seconds @@ -390,6 +398,8 @@ DMNCC trans(CC:DISCONNECT_IND IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:CM_SERVICE_REQ callref-0x8001 tid-8) tx MNCC_DISC_IND MSC --> MNCC: callref 0x8001: MNCC_DISC_IND DREF msc_a(IMSI-90170010650:MSISDN-42342:TMSI-0x0302
Change in osmo-ttcn3-hacks[master]: msc: log tweaks for call / call hangup
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15941 ) Change subject: msc: log tweaks for call / call hangup .. msc: log tweaks for call / call hangup Change-Id: I06474e3d592195a8c422493166d9f042da1ac7e6 --- M msc/BSC_ConnectionHandler.ttcn 1 file changed, 5 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/41/15941/1 diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn index 63c64bb..853d1e5 100644 --- a/msc/BSC_ConnectionHandler.ttcn +++ b/msc/BSC_ConnectionHandler.ttcn @@ -1389,6 +1389,7 @@ t_clear := tr_BSSMAP_ClearCommandCSFB; } + log("f_call_hangup 0: tx MNCC_DISC_REQ"); MNCC.send(ts_MNCC_DISC_req(cpars.mncc_callref, valueof(ts_MNCC_cause(23; BSSAP.receive(tr_PDU_DTAP_MT(tr_ML3_MT_CC_DISC(cpars.transaction_id))); @@ -1449,9 +1450,10 @@ f_mt_call_establish(cpars); - /* Hold the call for some time */ + log("Hold the call for some time"); f_sleep(3.0); + log("Hangup"); f_call_hangup(cpars, true); setverdict(pass); @@ -1462,9 +1464,10 @@ f_mo_call_establish(cpars); - /* Hold the call for some time */ + log("Hold the call for some time"); f_sleep(3.0); + log("Hangup"); f_call_hangup(cpars, false); setverdict(pass); -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15941 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I06474e3d592195a8c422493166d9f042da1ac7e6 Gerrit-Change-Number: 15941 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-ttcn3-hacks[master]: msc: add f_tc_invalid_mgcp_crash
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15942 ) Change subject: msc: add f_tc_invalid_mgcp_crash .. msc: add f_tc_invalid_mgcp_crash Make sure that osmo-msc doesn't crash if a successful CRCX response contains an invalid IP address. Originally/recently, osmo-msc did not validate the IP addresses at all. In an intermediate patch I added error handling, releasing the call. That uncovered a use-after-free problem in libosmo-mgcp-client. This problem is fixed by osmo_fsm_set_dealloc_ctx() and an osmo-mgw fix (see I7df2e9202b04e7ca7366bb0a8ec53cf3bb14faf3 in osmo-mgw). Add this test to make sure the crash is not re-introduced. Change-Id: I0c76b0a7a33a96a39a242ecd387ba3769161cf7a --- M msc/BSC_ConnectionHandler.ttcn M msc/MSC_Tests.ttcn 2 files changed, 40 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/42/15942/1 diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn index 853d1e5..092906d 100644 --- a/msc/BSC_ConnectionHandler.ttcn +++ b/msc/BSC_ConnectionHandler.ttcn @@ -711,6 +711,7 @@ (f_mt_call and f_mt_call) */ boolean stop_after_cc_setup,/* Special case: stop call establish after CC Setup */ boolean ran_clear_when_alerting,/* Special case: send Clear upon CC Alerting */ + boolean expect_release, /* Special case: expect call establish to cause direct CC Rel */ MgcpCallId mgcp_call_id optional, /* MGCP Call ID; CallAgent allocated */ MgcpEndpoint mgcp_ep optional /* MGCP Endpoint, CallAgent or MGW allocated */, @@ -749,6 +750,7 @@ mgw_drop_dlcx := false, stop_after_cc_setup := false, ran_clear_when_alerting := false, + expect_release := false, mgcp_call_id := omit, mgcp_ep := "rtpbridge/1@mgw", use_osmux := false, @@ -1210,6 +1212,8 @@ ts_BSSMAP_IE_AoIP_TLA4(f_inet_addr(cpars.mgw_conn_1.mgw_rtp_ip), ?); var default mdcx := activate(as_optional_mgcp_mdcx(cpars.mgw_conn_2.mgw_rtp_ip, cpars.mgw_conn_2.mgw_rtp_port)); + var boolean got_mncc_setup_compl_ind := false; + var boolean got_cc_connect := false; interleave { [] MNCC.receive(tr_MNCC_SETUP_ind(?, tr_MNCC_number(hex2str(cpars.called_party -> value mncc { @@ -1347,15 +1351,27 @@ [] MNCC.receive(tr_MNCC_SETUP_COMPL_ind(?)) -> value mncc { log("f_mo_call_establish 8: rx MNCC SETUP COMPLETE ind"); + got_mncc_setup_compl_ind := true; + if (not cpars.expect_release and got_cc_connect) { + break; + } } [] BSSAP.receive(tr_PDU_DTAP_MT(tr_ML3_MT_CC_CONNECT(cpars.transaction_id))) { log("f_mo_call_establish 10: rx CC CONNECT"); BSSAP.send(ts_PDU_DTAP_MO(ts_ML3_MO_CC_CONNECT_ACK(cpars.transaction_id))); + got_cc_connect := true; + if (not cpars.expect_release and got_mncc_setup_compl_ind) { + break; + } } [] BSSAP.receive(tr_PDU_DTAP_MT(tr_ML3_MT_CC_RELEASE(cpars.transaction_id))) { log("f_mo_call_establish 11: rx CC RELEASE"); + if (not cpars.expect_release) { + setverdict(fail, "Got unexpected CC Release"); + mtc.stop; + } f_expect_clear(); break; } diff --git a/msc/MSC_Tests.ttcn b/msc/MSC_Tests.ttcn index 4ef592f..480ec96 100644 --- a/msc/MSC_Tests.ttcn +++ b/msc/MSC_Tests.ttcn @@ -5662,6 +5662,29 @@ vc_conn.done; } +friend function f_tc_invalid_mgcp_crash(charstring id, BSC_ConnHdlrPars pars) runs on BSC_ConnHdlr { + f_init_handler(pars); + var CallParameters cpars := valueof(t_CallParams('12345'H, 0)); + + /* Set invalid IP address so that osmo-msc discards the rtp_stream and MGCP endpoint FSM instances in the middle +* of successful MGCP response dispatch. If things aren't safeguarded, the on_success() in osmo_mgcpc_ep_fsm +* will cause a use-after-free after that event dispatch. */ + cpars.mgw_conn_1.mgw_rtp_ip := "0.0.0.0"; + cpars.mgw_conn_2.mgw_rtp_ip := "0.0.0.0"; + cpars.rtp_sdp_format := "FOO/8000"; + cpars.expect_release := true; + + f_perform_lu(); + f_mo_call_establish(cpars); +} +testcase TC_invalid_mgcp_crash() runs on MTC_CT { + var BSC_ConnHdlr vc_conn; + f_init(); + + vc_conn := f_start_handler(refers(f_tc_invalid_mgcp_crash), 7); + vc_conn.done; +} + control { ex
Change in osmo-msc[master]: add msc_log_to_ladder.py
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15944 ) Change subject: add msc_log_to_ladder.py .. add msc_log_to_ladder.py Add script that reads in an osmo-msc log output and extracts the interesting information for displaying a sequence chart of voice call log, in mscgen format. I want to visualize how the sequence of messages changes across patches. It is error prone to do it manually, and re-doing the sequence chart for every patch (and patch rework) would be prohibitively time consuming. Change-Id: I2e4d8778f7b83dee558517a9b23450b817ee325d --- A doc/sequence_charts/msc_log_to_ladder.py 1 file changed, 724 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/44/15944/1 diff --git a/doc/sequence_charts/msc_log_to_ladder.py b/doc/sequence_charts/msc_log_to_ladder.py new file mode 100755 index 000..1ceaeb6 --- /dev/null +++ b/doc/sequence_charts/msc_log_to_ladder.py @@ -0,0 +1,724 @@ +#!/usr/bin/env python3 +doc=r''' +Reading a log, it can be hard to figure out what is the important bit going on. +This is a tool that reads an osmo-msc log and tries to compose a ladder diagram from it automatically. +''' + +import argparse +import sys +import re +import tempfile +import os + +def error(*msg): + sys.stderr.write('%s\n' % (''.join(msg))) + exit(1) + +def quote(msg, quote='"'): + return '"%s"' % (msg.replace('"', r'\"')) + +class Entity: + def __init__(self, name, descr=None, attrs={}): + self.name = name + self.descr = descr + self.attrs = attrs + +class Arrow: + def __init__(self, mo_mt, left, arrow, right, descr=None, attrs={}, ran_conn=None, imsi=None, tmsi=None): + self.mo_mt = mo_mt + self.left = left + self.arrow = arrow + self.right = right + self.descr = descr + self.attrs = attrs + self.ran_conn = ran_conn + self.imsi = imsi + self.tmsi = tmsi + + def __repr__(self): + return 'Arrow(%s %s %s %s: %s IMSI=%s)' % (self.mo_mt, self.left, self.arrow, self.right, self.descr, self.imsi) + +class Separator: + def __init__(self): + self.separator = None + self.descr = None + self.attrs = {} + +class EmptyLine: + def __init__(self): + self.count = 1 + +MS = 'ms' +UE = 'ms' #'ue' +MS_UE_UNKNOWN = 'ms' #None +MSC = 'msc' +MGW = 'mgw' +SIP = 'sip' + +MO = 'mo' +MT = 'mt' +MO_MT_UNKNOWN = None + + +class OutputBase: + def __init__(self, write_to, start_with_re=None): + self._write_to = write_to + + self.start_with_re = None + if start_with_re is not None: + self.start_with_re = re.compile(start_with_re) + + def head(self): + self.writeln('# Generated by msc_log_to_ladder.py') + + def tail(self): + pass + + def render(self, diagram): + self.head() + if diagram.root_attrs: + self.root_attrs(diagram.root_attrs) + self.entities(diagram.entities) + + started = (self.start_with_re is None) + + for line in diagram.lines: + if not started: + if hasattr(line, 'descr') and self.start_with_re.match(line.descr): + started = True + else: + continue + self.add(line) + self.tail() + + def entities(self, entities): + for entity in entities: + self.entity(entity) + + def write(self, line): + self._write_to.write(line) + + def writeln(self, line): + self.write('%s\n' % line) + + def emptyline(self, emptyline): + self.write('\n' * emptyline.count); + + def add(self, thing): + func = getattr(self, thing.__class__.__name__.lower()) + func(thing) + + +class OutputLadder(OutputBase): + + def indent_multiline(self, s): + return s.replace('\n', '\n\t') + + def attrs_str(self, attrs, prefix=' '): + if not attrs: + return '' + return '%s{%s}' % (prefix or '', ','.join('%s=%s' % (k,v) for k,v in attrs.items())) + + def root_attrs(self, attrs): + self.writeln(self.attrs_str(attrs, prefix=None)) + + def entity(self, entity): + self.writeln('%s = %s%s' % (entity.name, self.indent_multiline(entity.descr), self.attrs_str(entity.attrs))) + + def arrow(self, arrow): + mo_mt = arrow.mo_mt or MO + +
Change in osmo-msc[master]: msc_vlr_tests: log descriptions in color with -v
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15951 ) Change subject: msc_vlr_tests: log descriptions in color with -v .. msc_vlr_tests: log descriptions in color with -v Change-Id: I2b28a94a5b27932e343952ba82e7e11c46f5e87d --- M tests/msc_vlr/msc_vlr_tests.h 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/51/15951/1 diff --git a/tests/msc_vlr/msc_vlr_tests.h b/tests/msc_vlr/msc_vlr_tests.h index 57022b4..88f80b4 100644 --- a/tests/msc_vlr/msc_vlr_tests.h +++ b/tests/msc_vlr/msc_vlr_tests.h @@ -35,9 +35,12 @@ #include extern bool _log_lines; +#define LOG_COLOR "\033[1;33m" +#define LOG_COLOR_OFF "\033[0;m" + #define _log(fmt, args...) do { \ if (_log_lines) \ - fprintf(stderr, " %4d:%s: " fmt "\n", \ + fprintf(stderr, LOG_COLOR " %4d:%s: " fmt LOG_COLOR_OFF "\n", \ __LINE__, __FILE__, ## args ); \ else \ fprintf(stderr, fmt "\n", ## args ); \ -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15951 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I2b28a94a5b27932e343952ba82e7e11c46f5e87d Gerrit-Change-Number: 15951 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-msc[master]: rtp_stream: sanely cancel MGW endpoint FSM notify
Hello pespin, laforge, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-msc/+/15851 to look at the new patch set (#2). Change subject: rtp_stream: sanely cancel MGW endpoint FSM notify .. rtp_stream: sanely cancel MGW endpoint FSM notify libosmo-mgcp-client recently introduced osmo_mgcpc_ep_cancel_notify() to cancel notification if a notify target FSM deallocates. Use it for sanity in rtp_stream FSM cleanup, the notify target for endpoint FSMs. Depends: I41687d7f3a808587ab7f7520f46dcc3c29cff92d (osmo-mgw) I14f7a46031327fb2b2047b998eae6ad0bb7324ad (osmo-mgw) Change-Id: I351bb8e8fbc46eb629bcd599f6453e2c84c15015 --- M src/libmsc/rtp_stream.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/51/15851/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15851 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I351bb8e8fbc46eb629bcd599f6453e2c84c15015 Gerrit-Change-Number: 15851 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-MessageType: newpatchset
Change in osmo-msc[master]: msc_vlr_test_call: rename lu_utran_tmsi
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15952 ) Change subject: msc_vlr_test_call: rename lu_utran_tmsi .. msc_vlr_test_call: rename lu_utran_tmsi Change-Id: I46a41321e6d1be3672a56a6e3cc36f013fdcd396 --- M tests/msc_vlr/msc_vlr_test_call.c M tests/msc_vlr/msc_vlr_test_call.err 2 files changed, 16 insertions(+), 16 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/52/15952/1 diff --git a/tests/msc_vlr/msc_vlr_test_call.c b/tests/msc_vlr/msc_vlr_test_call.c index dfa3141..339233c 100644 --- a/tests/msc_vlr/msc_vlr_test_call.c +++ b/tests/msc_vlr/msc_vlr_test_call.c @@ -43,7 +43,7 @@ #define IMSI "90170010650" -static void standard_lu() +static void lu_utran_tmsi() { struct vlr_subscr *vsub; @@ -173,7 +173,7 @@ fake_time_start(); - standard_lu(); + lu_utran_tmsi(); BTW("after a while, a new conn sends a CM Service Request. VLR responds with Auth Req, 2nd auth vector"); auth_request_sent = false; @@ -291,7 +291,7 @@ fake_time_start(); - standard_lu(); + lu_utran_tmsi(); BTW("after a while, MNCC asks us to setup a call, causing Paging"); @@ -394,7 +394,7 @@ fake_time_start(); - standard_lu(); + lu_utran_tmsi(); BTW("after a while, MNCC asks us to setup a call, causing Paging"); @@ -489,7 +489,7 @@ fake_time_start(); - standard_lu(); + lu_utran_tmsi(); BTW("after a while, a new conn sends a CM Service Request. VLR responds with Auth Req, 2nd auth vector"); auth_request_sent = false; @@ -585,7 +585,7 @@ fake_time_start(); - standard_lu(); + lu_utran_tmsi(); BTW("after a while, a new conn sends a CM Service Request. VLR responds with Auth Req, 2nd auth vector"); auth_request_sent = false; diff --git a/tests/msc_vlr/msc_vlr_test_call.err b/tests/msc_vlr/msc_vlr_test_call.err index 07e10a5..8c394d0 100644 --- a/tests/msc_vlr/msc_vlr_test_call.err +++ b/tests/msc_vlr/msc_vlr_test_call.err @@ -189,11 +189,11 @@ DMSC msc_a(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:LU){MSC_A_ST_RELEASED}: Deallocated, including all deferred deallocations - msub gone llist_count(_list) == 0 -DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 + standard_lu: now used by 2 (attached,standard_lu) +DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 + lu_utran_tmsi: now used by 2 (attached,lu_utran_tmsi) vsub != NULL == 1 strcmp(vsub->imsi, IMSI) == 0 LAC == 23 -DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 - standard_lu: now used by 1 (attached) +DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 - lu_utran_tmsi: now used by 1 (attached) - after a while, a new conn sends a CM Service Request. VLR responds with Auth Req, 2nd auth vector @@ -663,11 +663,11 @@ DMSC msc_a(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:LU){MSC_A_ST_RELEASED}: Deallocated, including all deferred deallocations - msub gone llist_count(_list) == 0 -DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 + standard_lu: now used by 2 (attached,standard_lu) +DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 + lu_utran_tmsi: now used by 2 (attached,lu_utran_tmsi) vsub != NULL == 1 strcmp(vsub->imsi, IMSI) == 0 LAC == 23 -DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 - standard_lu: now used by 1 (attached) +DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 - lu_utran_tmsi: now used by 1 (attached) - after a while, MNCC asks us to setup a call, causing Paging @@ -1134,11 +1134,11 @@ DMSC msc_a(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:LU){MSC_A_ST_RELEASED}: Deallocated, including all deferred deallocations - msub gone llist_count(_list) == 0 -DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 + standard_lu: now used by 2 (attached,standard_lu) +DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 + lu_utran_tmsi: now used by 2 (attached,lu_utran_tmsi) vsub != NULL == 1 strcmp(vsub->imsi, IMSI) == 0 LAC == 23 -DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 - standard_lu: now used by 1 (attached) +DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 - lu_utran_tmsi: now used by 1 (attached) - after a while, MNCC asks us to setup a call, causing Paging @@ -1559,11 +1559,11 @@ DMSC msc_a(IMSI-90170010650:MSISDN-42342:TMSI-0x03020100:UTRAN-Iu:LU){MSC_A_ST_RELEASED}: Deallocated, including all deferred deallocations - msub gone llist_count(_list) == 0 -DREF VLR subscr IMSI-90170010650:MSISDN-42342:TMSI-0x03020100 + standard_lu: now used by 2 (attached
Change in osmo-msc[master]: add full SDP codec information to the MNCC socket
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15953 ) Change subject: add full SDP codec information to the MNCC socket .. add full SDP codec information to the MNCC socket This way osmo-msc can benefit from the complete codec information received via SIP, which was so far terminated at osmo-sip-connector. osmo-sip-connector could/should have translated the received SDP to MNCC bearer_cap, but this was never implemented properly. Since osmo-msc already handles SDP towards the MGW, it makes most sense to pass SDP to osmo-msc transparently. To be able to send a valid RTP IP:port in the SDP upon the first MNCC_SETUP_IND going out, move the CN side CRCX to the very start of establishing a voice call. As a result, first create MGW conns for both RAN and CN before starting. The voice_call_full.msc chart shows the change in message sequence for MO and MT voice calls. Implement cc_sdp.c, which accumulates codec information from various sources (MS, BSS, Assignment, remote call leg) and provides filtering to get the available set of codecs at any point in time. Implement codec_sdp_cc_t9n.c, to translate between SDP and the various libosmo-mgcp-client, CC and BSSMAP representations of codecs: - Speech Version, - Permitted Speech, - Speech Codec Type, - default Payload Type numbers, - enum mgcp_codecs, - FR/HR compatibility - SDP audio codec names, - various AMR configurations. A codec_map lists these relations in one large data record. Various functions provide conversions by traversing this map. Add trans->cc.mnccc_release_sent: so far, avoiding to send an MNCC release during trans_free() was done by setting the callref = 0. But that also skips CC Release. On codec mismatch, we send a specific MNCC error code but still want a normal CC Release: hence send the MNCC message, set mnccc_release_sent = true and do normal CC Release in trans_free(). (A better way to do this would be to adopt the mncc_call FSM from inter-MSC handover also for local voice calls, but that is out of scope for now. I want to try that soon, as time permits.) Change-Id: I8c3b2de53ffae4ec3a66b9dabf308c290a2c999f --- M doc/sequence_charts/voice_call_full.msc M include/osmocom/msc/Makefile.am M include/osmocom/msc/call_leg.h A include/osmocom/msc/cc_sdp.h A include/osmocom/msc/codec_sdp_cc_t9n.h M include/osmocom/msc/gsm_04_08.h M include/osmocom/msc/msc_a.h M include/osmocom/msc/msc_ho.h M include/osmocom/msc/rtp_stream.h M include/osmocom/msc/sdp_msg.h M include/osmocom/msc/transaction.h M src/libmsc/Makefile.am M src/libmsc/call_leg.c A src/libmsc/cc_sdp.c A src/libmsc/codec_sdp_cc_t9n.c M src/libmsc/gsm_04_08_cc.c M src/libmsc/mncc_call.c M src/libmsc/msc_a.c M src/libmsc/msc_ho.c M src/libmsc/msc_t.c M src/libmsc/rtp_stream.c M src/libmsc/sdp_msg.c M tests/msc_vlr/msc_vlr_test_call.c M tests/msc_vlr/msc_vlr_test_call.err M tests/msc_vlr/msc_vlr_tests.c M tests/msc_vlr/msc_vlr_tests.h M tests/sdp_msg/sdp_msg_test.ok 27 files changed, 5,485 insertions(+), 463 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/53/15953/1 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15953 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8c3b2de53ffae4ec3a66b9dabf308c290a2c999f Gerrit-Change-Number: 15953 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-ttcn3-hacks[master]: cosmetic: remove brace from comment
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15937 ) Change subject: cosmetic: remove brace from comment .. cosmetic: remove brace from comment This '{' in the comment gets my auto indenting / syntax brace matching all confused. Change-Id: I303abe800037abd0c9694ae750a7acaa79c9754f --- M msc/BSC_ConnectionHandler.ttcn 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/37/15937/1 diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn index 0ddc911..7a94d85 100644 --- a/msc/BSC_ConnectionHandler.ttcn +++ b/msc/BSC_ConnectionHandler.ttcn @@ -1043,7 +1043,7 @@ MNCC.send(ts_MNCC_ALERT_req(cpars.mncc_callref)); } - //[g_pars.ran_is_geran] BSSAP.receive(tr_BSSMAP_AssignmentReq(omit, tla_ass)) -> value bssap { + //[g_pars.ran_is_geran] BSSAP.receive(tr_BSSMAP_AssignmentReq(omit, tla_ass)) -> value bssap [] BSSAP.receive(tr_BSSMAP_AssignmentReq(omit)) -> value bssap { log("f_mo_call_establish 4: rx Assignment Request"); var BSSMAP_IE_AoIP_TransportLayerAddress tla; -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15937 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I303abe800037abd0c9694ae750a7acaa79c9754f Gerrit-Change-Number: 15937 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-ttcn3-hacks[master]: msc: fix Iu mo call
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15939 ) Change subject: msc: fix Iu mo call .. msc: fix Iu mo call Change-Id: I0ead36333ab665147b8d222070ea5cf8afc555ec --- M library/ranap/RANAP_Templates.ttcn M msc/BSC_ConnectionHandler.ttcn 2 files changed, 48 insertions(+), 31 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/39/15939/1 diff --git a/library/ranap/RANAP_Templates.ttcn b/library/ranap/RANAP_Templates.ttcn index df4ea03..c616231 100644 --- a/library/ranap/RANAP_Templates.ttcn +++ b/library/ranap/RANAP_Templates.ttcn @@ -1305,7 +1305,7 @@ protocolIEs := { { id := id_RAB_SetupOrModifyList, - criticality := reject, + criticality := ?, value_ := { rAB_SetupOrModifyList := rab_sml } diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn index 23697da..b11d24b 100644 --- a/msc/BSC_ConnectionHandler.ttcn +++ b/msc/BSC_ConnectionHandler.ttcn @@ -1091,6 +1091,20 @@ } } +private altstep as_optional_mgcp_dlcx(CallParameters cpars) runs on BSC_ConnHdlr { + var MgcpCommand mgcp_cmd; + var boolean respond_to_dlcx := not (isbound(cpars.mgw_drop_dlcx) and valueof(cpars.mgw_drop_dlcx)); + [] MGCP.receive(tr_DLCX(?)) -> value mgcp_cmd { + log("as_optional_mgcp_dlcx: rx MGCP DLCX"); + if (respond_to_dlcx) { + MGCP.send(ts_DLCX_ACK2(mgcp_cmd.line.trans_id)); + } + /* Without this 'repeat', currently active other interleave and alt series exit as soon as a +* DLCX is handled. */ + repeat; + } +} + function f_mo_call_establish(inout CallParameters cpars) runs on BSC_ConnHdlr { @@ -1099,6 +1113,7 @@ var template MgcpResponse mgcp_resp; var boolean respond_to_dlcx; var PDU_BSSAP bssap; + var RANAP_PDU ranap; var MgcpOsmuxCID osmux_cid; cpars.mo_call := true; @@ -1193,11 +1208,29 @@ } BSSAP.send(bssap); } - /* - [!g_pars.ran_is_geran] BSSAP.receive(tr_RANAP_RabAssReq(rab_sml)) { - //BSSAP.send(ts_RANAP_RabAssResp(rab_sml)); FIXME + [] BSSAP.receive(tr_RANAP_RabAssReq(*)) -> value ranap { + log("f_mo_call_establish 4.iu: rx RANAP RAB Assignment Request"); + var RAB_SetupOrModifiedList l := { + { + { + id := id_RAB_SetupOrModifiedItem, + criticality := ignore, + value_ := { + rAB_SetupOrModifiedItem := { + rAB_ID := int2bit(23, 8), + transportLayerAddress := hex2bit( '350001c0a8021500'H), + iuTransportAssociation := { + bindingID := '040c'O + }, + dl_dataVolumes := omit, + iE_Extensions := omit + } + } + } + } + }; + BSSAP.send(ts_RANAP_RabAssResp(l)); } - */ /* MDCX setting up the RAN side remote RTP address received from Assignment Complete */ [] MGCP.receive(tr_MDCX) -> value mgcp_cmd { @@ -1315,48 +1348,32 @@ respond_to_dlcx := not (isbound(cpars.mgw_drop_dlcx) and valueof(cpars.mgw_drop_dlcx)); var default mdcx := activate(as_optional_mgcp_mdcx(cpars.mgw_conn_2.mgw_rtp_ip, cpars.mgw_conn_2.mgw_rtp_port)); + var default dlcx := activate(as_optional_mgcp_dlcx(cpars)); /* clearing of radio channel */ - interleave { - //[g_pars.ran_is_geran] BSSAP.receive(t_clear) { - [] BSSAP.receive(t_clear) { + alt { + [g_pars.ran_is_geran] BSSAP.receive(t_clear) { log("f_call_hangup 5: rx BSSAP Clear Command"); BSSAP.send(ts_BSSMAP_ClearComplete); BSSAP.receive(RAN_Conn_Prim:MSC_CONN_PRIM_DISC_IND); log("f_call_hangup 6: rx
Change in osmo-ttcn3-hacks[master]: msc-test: improve error log on Assignment RTP port mismatch
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15935 ) Change subject: msc-test: improve error log on Assignment RTP port mismatch .. msc-test: improve error log on Assignment RTP port mismatch Before this, if an Assignment Request contains an unexpected Transport Layer Address, the test completely rejects the Assignment Request. Instead, accept any tla in the Assignment, and compare the expected tla in the Assignment's interleave action. Thus we directly get logging hinting at the tla instead of a T_guard timeout. Change-Id: I04847c10d6c3bf9e04cfda6e343dfd4a65be71a5 --- M msc/BSC_ConnectionHandler.ttcn 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/35/15935/1 diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn index fb2d888..5b0a125 100644 --- a/msc/BSC_ConnectionHandler.ttcn +++ b/msc/BSC_ConnectionHandler.ttcn @@ -1044,12 +1044,19 @@ } //[g_pars.ran_is_geran] BSSAP.receive(tr_BSSMAP_AssignmentReq(omit, tla_ass)) -> value bssap { - [] BSSAP.receive(tr_BSSMAP_AssignmentReq(omit, tla_ass)) -> value bssap { + [] BSSAP.receive(tr_BSSMAP_AssignmentReq(omit)) -> value bssap { log("f_mo_call_establish 4: rx Assignment Request"); var BSSMAP_IE_AoIP_TransportLayerAddress tla; var BSSMAP_IE_SpeechCodec codec; var BSSMAP_IE_Osmo_OsmuxCID osmuxCID; + if (tla_ass != bssap.pdu.bssmap.assignmentRequest.aoIPTransportLayer) { + log("Expected:", tla_ass); + log("Got:", bssap.pdu.bssmap.assignmentRequest.aoIPTransportLayer); + setverdict(fail, "MSC sent Assignment Request with unexpected AoIP Transport Layer IE"); + mtc.stop; + } + tla := valueof(ts_BSSMAP_IE_AoIP_TLA4(f_inet_addr(cpars.bss_rtp_ip), cpars.bss_rtp_port)); codec := valueof(ts_BSSMAP_IE_SpeechCodec({ts_CodecFR})); if (cpars.use_osmux) { -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15935 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I04847c10d6c3bf9e04cfda6e343dfd4a65be71a5 Gerrit-Change-Number: 15935 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-ttcn3-hacks[master]: re-implement compare-results.sh as compare-results.py
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15943 ) Change subject: re-implement compare-results.sh as compare-results.py .. re-implement compare-results.sh as compare-results.py The compare-results.sh is annoyingly slow. Since our ttcn3 tests containers support Python 2, re-implement in Python for much quicker evaluation. Change-Id: I0747c9d66ffc7e4121497a2416fca78d7b56c8e6 --- A compare-results.py D compare-results.sh M start-testsuite.sh 3 files changed, 144 insertions(+), 214 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/43/15943/1 diff --git a/compare-results.py b/compare-results.py new file mode 100755 index 000..48efd78 --- /dev/null +++ b/compare-results.py @@ -0,0 +1,143 @@ +#!/usr/bin/env python +# Copyright 2018 sysmocom - s.f.m.c. GmbH +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import argparse +import re + +doc = "Compare TTCN3 test run results with expected results by junit logs." + +# The nicest would be to use an XML library, but I don't want to introduce dependencies on the build slaves. +re_testcase = re.compile(r'') +re_testcase_end = re.compile(r'''(|]*/>)''') +re_failure = re.compile(r'''(http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -usage() { - echo " -Usage: - - $(basename "$0") expected_results.junit-log current_results.junit-log [--allow-* [...]] - -Return 0 if the expected results match the current results exactly. - - --allow-skip Allow runnning less tests than are listed in the expected file. - Default is to return failure on any skipped tests. - --allow-newAllow more test results than found in the expected file. - Default is to return failure on any unknown tests. - --allow-xpass If a test was expected to fail but passed, return success. - Default is to return failure on any mismatch. -" -} - -if [ ! -f "$expected_file" ]; then - usage - echo "Expected file not found: '$expected_file'" - exit 1 -fi - -if [ ! -f "$results_file" ]; then - usage - echo "Current results file not found: '$results_file'" - exit 1 -fi - -shift -shift - -allow_xpass=0 -allow_skip=0 -allow_new=0 - -while test -n "$1"; do - arg="$1" - if [ "x$arg" = "x--allow-xpass" ]; then -allow_xpass=1 - elif [ "x$arg" = "x--allow-skip" ]; then -allow_skip=1 - elif [ "x$arg" = "x--allow-new" ]; then -allow_new=1 - else -usage -echo "Unknown argument: '$arg'" -exit 1 - fi - shift -done - -echo "Comparing expected results $expected_file against results in $results_file -" - -parse_testcase() { - line="$1" - suite_name="$(echo "$line" | sed 's,.*classname='"'"'\([^'"'"']*\)'"'"'.*,\1,')" - test_name="$(echo "$line" | sed 's,.*\$')" ]; then -test_result="pass" - else -test_result="FAIL" - fi -} - -pass=0 -xfail=0 -more_failures=0 -more_successes=0 -skipped=0 -new=0 - -while read line; do - parse_testcase "$line" - exp_suite_name="$suite_name" - exp_test_name="$test_name" - exp_test_result="$test_result" - matched="0" - - while read line; do -parse_testcase "$line" -if [ "x$exp_suite_name" != "x$suite_name" ]; then - continue -fi -if [ "x$exp_test_name" != "x$test_name" ]; then - continue -fi - -if [ "x$exp_test_result" = "x$test_result" ]; then - if [ "x$exp_test_result" = "xFAIL" ]; then -exp_test_result="xfail" - (( xfail += 1 )) - else -(( pass += 1 )) - fi - echo "$exp_test_result $suite_name.$test_name" -else - if [ "x$exp_test_result&q
Change in osmo-msc[master]: MNCC: add optional SDP to the socket protocol
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15948 ) Change subject: MNCC: add optional SDP to the socket protocol .. MNCC: add optional SDP to the socket protocol Add a char buffer of 1024 characters length as space for SDP to pass to / receive from MNCC. Actually support receiving MNCC without such an SDP tail. The main reason for this is to avoid the need to adjust the ttcn3 implementation of MNCC: it would stop working for older osmo-msc. Older or non-SIP MNCC peers could operate the previous MNCC protocol unchanged (save the protocol number bump) without having to implement SDP. The SDP part in the MNCC protocol will be used in upcoming patch I8c3b2de53ffae4ec3a66b9dabf308c290a2c999f. Change-Id: Ie16f0804c4d99760cd4a0c544d0889b6313eebb7 --- M include/osmocom/msc/mncc.h M src/libmsc/mncc.c 2 files changed, 11 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/48/15948/1 diff --git a/include/osmocom/msc/mncc.h b/include/osmocom/msc/mncc.h index 28ee9b3..2297b54 100644 --- a/include/osmocom/msc/mncc.h +++ b/include/osmocom/msc/mncc.h @@ -159,6 +159,9 @@ unsigned char lchan_type; unsigned char lchan_mode; + + /* A buffer to contain SDP ('\0' terminated) */ + charsdp[1024]; }; struct gsm_data_frame { @@ -167,7 +170,7 @@ unsigned char data[0]; }; -#define MNCC_SOCK_VERSION 5 +#define MNCC_SOCK_VERSION 6 struct gsm_mncc_hello { uint32_tmsg_type; uint32_tversion; @@ -190,6 +193,7 @@ uint16_tport; uint32_tpayload_type; uint32_tpayload_msg_type; + charsdp[1024]; }; struct gsm_mncc_bridge { diff --git a/src/libmsc/mncc.c b/src/libmsc/mncc.c index d0b2ff2..3d17ab9 100644 --- a/src/libmsc/mncc.c +++ b/src/libmsc/mncc.c @@ -262,7 +262,9 @@ case MNCC_RTP_FREE: case MNCC_RTP_CONNECT: case MNCC_RTP_CREATE: - if (len < sizeof(struct gsm_mncc_rtp)) { + /* Should we receive an MNCC message without SDP, the zero-initialized msgb will guarantee that the +* char sdp[] starts with a '\0'. */ + if (len < offsetof(struct gsm_mncc_rtp, sdp)) { LOGP(DMNCC, LOGL_ERROR, "Short MNCC RTP\n"); return -EINVAL; } @@ -279,7 +281,9 @@ } break; default: - if (len < sizeof(struct gsm_mncc)) { + /* Should we receive an MNCC message without SDP, the zero-initialized msgb will guarantee that the +* char sdp[] starts with a '\0'. */ + if (len < offsetof(struct gsm_mncc, sdp)) { LOGP(DMNCC, LOGL_ERROR, "Short MNCC Signalling\n"); return -EINVAL; } -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15948 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ie16f0804c4d99760cd4a0c544d0889b6313eebb7 Gerrit-Change-Number: 15948 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-msc[master]: validate dtap err logging
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15949 ) Change subject: validate dtap err logging .. validate dtap err logging Change-Id: I3edd90be40555dd648e9f16db5b6040665a19a95 --- M tests/msc_vlr/msc_vlr_tests.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/49/15949/1 diff --git a/tests/msc_vlr/msc_vlr_tests.c b/tests/msc_vlr/msc_vlr_tests.c index de4913a..e00c337 100644 --- a/tests/msc_vlr/msc_vlr_tests.c +++ b/tests/msc_vlr/msc_vlr_tests.c @@ -254,8 +254,10 @@ /* Mask the sequence number out before comparing */ msg->data[1] &= 0x3f; - if (!msgb_eq_data_print(msg, dtap_tx_expected->data, dtap_tx_expected->len)) + if (!msgb_eq_data_print(msg, dtap_tx_expected->data, dtap_tx_expected->len)) { + btw("Expected %s", osmo_hexdump(dtap_tx_expected->data, dtap_tx_expected->len)); abort(); + } btw("DTAP matches expected message"); -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15949 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I3edd90be40555dd648e9f16db5b6040665a19a95 Gerrit-Change-Number: 15949 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-msc[master]: BSSMAP: decode Codec List (BSS Supported)
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15946 ) Change subject: BSSMAP: decode Codec List (BSS Supported) .. BSSMAP: decode Codec List (BSS Supported) Actually decode the Codec List (BSS Supported) in BSSMAP, in both the Complete Layer 3 Information and the Assignment Complete messages. An upcoming patch improves codec negotiation and requires the BSS supported codecs, which are so far ignored (which is/was a pity as osmo-bsc goes at great lengths to compose those IEs). Change-Id: I66c735c79e982388f06b5de783aa584c9d13569e --- M include/osmocom/msc/ran_msg.h M src/libmsc/ran_msg_a.c 2 files changed, 33 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/46/15946/1 diff --git a/include/osmocom/msc/ran_msg.h b/include/osmocom/msc/ran_msg.h index 081c7ad..1b0e2e8 100644 --- a/include/osmocom/msc/ran_msg.h +++ b/include/osmocom/msc/ran_msg.h @@ -192,6 +192,7 @@ union { struct { const struct gsm0808_cell_id *cell_id; + const struct gsm0808_speech_codec_list *codec_list_bss_supported; struct msgb *msg; } compl_l3; struct msgb *dtap; @@ -226,6 +227,7 @@ struct osmo_sockaddr_str remote_rtp; bool codec_present; enum mgcp_codecs codec; + const struct gsm0808_speech_codec_list *codec_list_bss_supported; bool osmux_present; uint8_t osmux_cid; } assignment_complete; diff --git a/src/libmsc/ran_msg_a.c b/src/libmsc/ran_msg_a.c index fc9a9d7..2307fa6 100644 --- a/src/libmsc/ran_msg_a.c +++ b/src/libmsc/ran_msg_a.c @@ -52,6 +52,8 @@ struct gsm0808_cell_id cell_id; struct tlv_p_entry *ie_cell_id = TLVP_GET(tp, GSM0808_IE_CELL_IDENTIFIER); struct tlv_p_entry *ie_l3_info = TLVP_GET(tp, GSM0808_IE_LAYER_3_INFORMATION); + struct tlv_p_entry *ie_codec_list_bss_supported = TLVP_GET(tp, GSM0808_IE_SPEECH_CODEC_LIST); + struct gsm0808_speech_codec_list codec_list_bss_supported; struct ran_msg ran_dec_msg = { .msg_type = RAN_MSG_COMPL_L3, .msg_name = "BSSMAP Complete Layer 3 Information", @@ -114,6 +116,20 @@ return -ENODATA; } + /* Decode Codec List (BSS Supported) */ + if (ie_codec_list_bss_supported) { + rc = gsm0808_dec_speech_codec_list(_list_bss_supported, + ie_codec_list_bss_supported->val, ie_codec_list_bss_supported->len); + if (rc < 0) { + LOG_RAN_A_DEC_MSG(LOGL_ERROR, + "Complete Layer 3 Information: unable to decode IE Codec List (BSS Supported)" + " (rc=%d), continuing anyway\n", rc); + /* This IE is not critical, do not abort with error. */ + } else + ran_dec_msg.compl_l3.codec_list_bss_supported = _list_bss_supported; + } + + return ran_decoded(ran_dec, _dec_msg); } @@ -261,10 +277,12 @@ { struct tlv_p_entry *ie_aoip_transp_addr = TLVP_GET(tp, GSM0808_IE_AOIP_TRASP_ADDR); struct tlv_p_entry *ie_speech_codec = TLVP_GET(tp, GSM0808_IE_SPEECH_CODEC); + struct tlv_p_entry *ie_codec_list_bss_supported = TLVP_GET(tp, GSM0808_IE_SPEECH_CODEC_LIST); struct tlv_p_entry *ie_osmux_cid = TLVP_GET(tp, GSM0808_IE_OSMO_OSMUX_CID); struct sockaddr_storage rtp_addr; struct sockaddr_in *rtp_addr_in; struct gsm0808_speech_codec sc; + struct gsm0808_speech_codec_list codec_list_bss_supported; int rc; struct ran_msg ran_dec_msg = { .msg_type = RAN_MSG_ASSIGNMENT_COMPLETE, @@ -314,6 +332,19 @@ ran_dec_msg.assignment_complete.codec = ran_a_mgcp_codec_from_sc(); } + if (ie_codec_list_bss_supported) { + /* Decode Codec List (BSS Supported) */ + rc = gsm0808_dec_speech_codec_list(_list_bss_supported, + ie_codec_list_bss_supported->val, ie_codec_list_bss_supported->len); + if (rc < 0) { + LOG_RAN_A_DEC_MSG(LOGL_ERROR, + "Assignment Complete: unable to decode IE Codec List (BSS Supported)" + " (rc=%d), continuing anyway\n", rc); + /* This IE is not critical, do not abort with error. */ + } else + ran_dec_msg.assignment_complete.codec_list_bss_supported = _list_bss_supported; + } +
Change in osmo-msc[master]: charts: add full MO and MT voice call diagram
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/15945 ) Change subject: charts: add full MO and MT voice call diagram .. charts: add full MO and MT voice call diagram Add voice_call_full.msc, generated from a real 2G<->3G voice call log fed to msc_log_to_ladder.py. The idea is to document how the voice call sequence of events changes in upcoming patches. Change-Id: I8a907d6a4ece1f3ad78da75a8c3e3e76afd5418d --- M doc/sequence_charts/Makefile.am A doc/sequence_charts/voice_call_full.msc 2 files changed, 126 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/45/15945/1 diff --git a/doc/sequence_charts/Makefile.am b/doc/sequence_charts/Makefile.am index f1775c8..6782f44 100644 --- a/doc/sequence_charts/Makefile.am +++ b/doc/sequence_charts/Makefile.am @@ -13,18 +13,20 @@ inter_bsc_ho.png \ inter_msc_ho.png \ mncc_call_fsm.png \ + voice_call_full.png \ $(NULL) msc: \ $(builddir)/mncc_call_fsm.png \ $(builddir)/inter_bsc_ho.png \ $(builddir)/inter_msc_ho.png \ + $(builddir)/voice_call_full.png \ $(NULL) dot: \ $(NULL) -$(builddir)/%.png: $(srcdir)/%.msc +$(builddir)/%.png: %.msc mscgen -T png -o $@ $< $(builddir)/%.png: $(srcdir)/%.dot diff --git a/doc/sequence_charts/voice_call_full.msc b/doc/sequence_charts/voice_call_full.msc new file mode 100644 index 000..75fcef2 --- /dev/null +++ b/doc/sequence_charts/voice_call_full.msc @@ -0,0 +1,123 @@ +# Generated by msc_log_to_ladder.py +msc { +hscale="3"; +moms[label="MS,BSS (MO)\nUE,hNodeB (MO)"],momgw[label="MGW for MSC (MO)"],momsc[label="MSC (MO)"],sip[label="MNCC to PBX via\nosmo-sip-connector"],mtmsc[label="MSC (MT)"],mtmgw[label="MGW for MSC (MT)"],mtms[label="BSS,MS (MT)\nhNodeB,UE (MT)"]; +moms =>> momsc [label="MM CM_SERV_REQ"]; +moms <<= momsc [label="MM AUTH_REQ"]; +moms =>> momsc [label="MM AUTH_RESP"]; +moms <<= momsc [label="(BSSMAP) CIPHER_MODE_COMMAND"]; +moms =>> momsc [label="(BSSMAP) Ciphering Mode Complete"]; +moms =>> momsc [label="RR CIPH_M_COMPL"]; +moms =>> momsc [label="CC SETUP"]; +momsc note momsc [label="CC starts guard timer (180s)"]; +momsc abox momsc [label="CC state:\nINITIATED"]; +momsc =>> sip [label="MNCC_SETUP_IND"]; +momsc <<= sip [label="MNCC_RTP_CREATE"]; +momgw <<= momsc[label="for RAN: CRCX\nrtpbridge/*@msc"]; +momgw =>> momsc[label="for RAN: CRCX OK\nEP-1 CI-1"]; +moms <<= momsc [label="(BSSMAP) ASSIGNMENT_COMMAND"]; +moms =>> momsc [label="(BSSMAP) Assignment Complete"]; +momgw <<= momsc[label="for RAN: MDCX\nEP-1 CI-1"]; +momgw =>> momsc[label="for RAN: MDCX OK\nEP-1 CI-1"]; +momgw <<= momsc[label="for CN: CRCX\nEP-1"]; +momgw =>> momsc[label="for CN: CRCX OK\nEP-1 CI-2"]; +momsc =>> sip [label="MNCC_RTP_CREATE\nIP:port-1"]; +momsc <<= sip [label="MNCC_CALL_PROC_REQ"]; +momsc note momsc [label="CC stops guard timer"]; +momsc note momsc [label="CC starts guard timer (180s)"]; +momsc abox momsc [label="CC state:\nMO_CALL_PROC"]; +moms <<= momsc [label="CC CALL_PROC"]; +mtmsc <<= sip [label="MNCC_SETUP_REQ"]; +mtms <<= mtmsc [label="Paging"]; +mtms =>> mtmsc [label="RR PAG_RESP"]; +mtms <<= mtmsc [label="MM AUTH_REQ"]; +mtms =>> mtmsc [label="MM NULL"]; +mtms =>> mtmsc [label="MM AUTH_RESP"]; +mtms <<= mtmsc [label="(RANAP) SecurityModeCommand"]; +mtms =>> mtmsc [label="(RANAP) SecurityModeControl successfulOutcome"]; +mtms <<= mtmsc [label="(RANAP) CommonId"]; +mtmsc note mtmsc [label="CC starts timer T303 (30s)"]; +mtmsc abox mtmsc [label="CC state:\nCALL_PRESENT"]; +mtms <<= mtmsc [label="CC SETUP"]; +mtms =>> mtmsc [label="CC CALL_CONF"]; +mtmsc note mtmsc [label="CC stops timer T303"]; +mtmsc note mtmsc [label="CC starts timer T310 (30s)"]; +mtmsc abox mtmsc [label="CC state:\nMO_TERM_CALL_CONF"]; +mtmgw <<= mtmsc[label="for RAN: CRCX\nrtpbridge/*@msc"]; +mtmsc =>> sip [label="MNCC_CALL_CONF_IND"]; +mtmsc <<= sip [label="MNCC_RTP_CREATE"]; +mtmgw =>> mtmsc[label="for RAN: CRCX OK\nEP-2 CI-3"]; +
Change in osmo-hlr[master]: fix upgrade test in presence of ~/.sqliterc
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/16019 ) Change subject: fix upgrade test in presence of ~/.sqliterc .. fix upgrade test in presence of ~/.sqliterc db_upgrade_test.sh: - If an ~/.sqliterc file exists, it causes output of '-- Loading resources from ~/.sqliterc'. Use -batch option to omit that. - To make sure that column headers are off when required, add -noheaders in some places. Change-Id: I279a39984563594a4a3914b2ce3d803ad9468fe8 --- M tests/db_upgrade/db_upgrade_test.sh 1 file changed, 6 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/19/16019/1 diff --git a/tests/db_upgrade/db_upgrade_test.sh b/tests/db_upgrade/db_upgrade_test.sh index bf56c56..8e2b498 100755 --- a/tests/db_upgrade/db_upgrade_test.sh +++ b/tests/db_upgrade/db_upgrade_test.sh @@ -11,21 +11,21 @@ dump_sorted_schema(){ db_file="$1" - tables="$(sqlite3 "$db_file" "SELECT name FROM sqlite_master WHERE type = 'table' order by name")" + tables="$(sqlite3 -batch -noheader "$db_file" "SELECT name FROM sqlite_master WHERE type = 'table' order by name")" for table in $tables; do echo echo "Table: $table" - sqlite3 -header "$db_file" "SELECT name,type,\"notnull\",dflt_value,pk FROM PRAGMA_TABLE_INFO('$table') order by name;" + sqlite3 -batch -header "$db_file" "SELECT name,type,\"notnull\",dflt_value,pk FROM PRAGMA_TABLE_INFO('$table') order by name;" echo echo "Table $table contents:" - columns="$(sqlite3 "$db_file" "SELECT name FROM PRAGMA_TABLE_INFO('$table') order by name;")" - sqlite3 -header "$db_file" "SELECT $(echo $columns | sed 's/ /,/g') from $table;" + columns="$(sqlite3 -batch -noheader "$db_file" "SELECT name FROM PRAGMA_TABLE_INFO('$table') order by name;")" + sqlite3 -batch -header "$db_file" "SELECT $(echo $columns | sed 's/ /,/g') from $table;" done } rm -f "$db" echo "Creating db in schema version 0" -sqlite3 "$db" < "$srcdir/hlr_db_v0.sql" +sqlite3 -batch "$db" < "$srcdir/hlr_db_v0.sql" echo echo "Version 0 db:" @@ -61,7 +61,7 @@ -n OsmoHLR -p 4258 \ -r "$osmo_hlr -c $cfg -l $mint_db" \ "$srcdir/create_subscribers.vty" - sqlite3 "$mint_db" < "$srcdir/create_subscribers_step2.sql" + sqlite3 -batch "$mint_db" < "$srcdir/create_subscribers_step2.sql" set +x test_dump="$builddir/test.dump" -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/16019 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I279a39984563594a4a3914b2ce3d803ad9468fe8 Gerrit-Change-Number: 16019 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-ttcn3-hacks[master]: hlr: stop on various failures
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/16020 ) Change subject: hlr: stop on various failures .. hlr: stop on various failures I found some of the tests hard to analyse when geting failures, because they don't stop the test on failure. Spread some 'mtc.stop' so that the test stops at the failed message instead of carrying on. Change-Id: I804aca84d0ccf4767a5c097cf6c882ccbd87c4e1 --- M hlr/HLR_Tests.ttcn 1 file changed, 15 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/20/16020/1 diff --git a/hlr/HLR_Tests.ttcn b/hlr/HLR_Tests.ttcn index f309575..f8d7483 100644 --- a/hlr/HLR_Tests.ttcn +++ b/hlr/HLR_Tests.ttcn @@ -425,15 +425,19 @@ } [exp_fail] GSUP.receive(tr_GSUP_UL_ERR(imsi, ?)) -> value ret { setverdict(fail, "Unexpected UL ERROR Cause"); + mtc.stop; } [exp_fail] GSUP.receive(tr_GSUP_UL_RES(imsi)) -> value ret { setverdict(fail, "Unexpected UL.res for unknown IMSI"); + mtc.stop; } [exp_fail] GSUP.receive(tr_GSUP_ISD_REQ(imsi)) -> value ret { setverdict(fail, "Unexpected ISD.req in error case"); + mtc.stop; } [not exp_fail] GSUP.receive(tr_GSUP_UL_ERR(imsi, ?)) -> value ret { setverdict(fail, "Unexpected UL ERROR"); + mtc.stop; } [not exp_fail and not isd_done] GSUP.receive(tr_GSUP_ISD_REQ(imsi, msisdn)) -> value ret { GSUP.send(ts_GSUP_ISD_RES(imsi)); @@ -540,12 +544,14 @@ alt { [] GSUP.receive(tr_GSUP_PROC_SS_ERR(imsi, sid, ?, ?)) -> value ret { setverdict(fail, "Unexpected PROC_SS ERROR Cause"); + mtc.stop; } [not exp_ss] GSUP.receive(tr_GSUP_PROC_SS_RES(imsi, sid, state, omit)) -> value ret { setverdict(pass); } [exp_ss] GSUP.receive(tr_GSUP_PROC_SS_RES(imsi, sid, state, omit)) -> value ret { setverdict(fail, "Unexpected PROC_SS.res without SS IE"); + mtc.stop; } /* [exp_ss] GSUP.receive(tr_GSUP_PROC_SS_RES(imsi, sid, state, decmatch facility)) -> value ret { @@ -562,9 +568,13 @@ setverdict(pass); } else { setverdict(fail, "Unexpected PROC_SS.res with non-matching facility IE"); + mtc.stop; } } - [] GSUP.receive { repeat; } + [] GSUP.receive { + setverdict(fail, "Unexpected GSUP"); + mtc.stop; + } [] T.timeout { setverdict(fail, "Timeout waiting for PROC_SS response"); self.stop; @@ -593,18 +603,22 @@ } [exp_fail] GSUP.receive(tr_GSUP_CHECK_IMEI_ERR(imsi, ?)) -> value pdu { setverdict(fail, "Unexpected CHECK IMEI ERROR Cause: ", pdu); + mtc.stop; } [exp_fail] GSUP.receive(tr_GSUP_CHECK_IMEI_RES(imsi, ?)) -> value pdu { setverdict(fail, "Unexpected CHECK IMEI RES instead of ERR"); + mtc.stop; } [not exp_fail] GSUP.receive(tr_GSUP_CHECK_IMEI_ERR(imsi, ?)) -> value pdu { setverdict(fail, "Unexpected CHECK IMEI ERROR"); + mtc.stop; } [not exp_fail] GSUP.receive(tr_GSUP_CHECK_IMEI_RES(imsi, result)) -> value pdu { setverdict(pass); } [not exp_fail] GSUP.receive(tr_GSUP_CHECK_IMEI_RES(imsi, ?)) -> value pdu { setverdict(fail, "Unexpected CHECK IMEI RES"); + mtc.stop; } [] GSUP.receive { repeat; } [] T.timeout { -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/16020 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I804aca84d0ccf4767a5c097cf6c882ccbd87c4e1 Gerrit-Change-Number: 16020 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-ttcn3-hacks[master]: hlr: add tests for GSUP proxy routing
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/16021 ) Change subject: hlr: add tests for GSUP proxy routing .. hlr: add tests for GSUP proxy routing GSUP proxy routing, as it is implemented in an upcoming osmo-hlr patch, requires that osmo-hlr returns a received Source Name IE back as Destination Name IE. Add tests for these, for various situations. At the time of writing, these tests still fail on master, and will pass as soon as GSUP request handling with request->response association is introduced to osmo-hlr in an upcoming patch (I179ebb0385b5b355f4740e14d43be97bf93622e3). Implement this by adding a source_name to the g_pars, which should be sent out in ts_GSUP_* to osmo-hlr, and expected back as destination_name in returned messages. Add source_name and destination_name to various templates, with default := omit. Add f_gen_ts_ies() and f_gen_tr_ies() to compose expected IEs more generically. Change-Id: I3728776d862c5e5fa7628ca28d74c1ef247459fa --- M hlr/HLR_Tests.ttcn M library/GSUP_Types.ttcn 2 files changed, 233 insertions(+), 65 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/21/16021/1 diff --git a/hlr/HLR_Tests.ttcn b/hlr/HLR_Tests.ttcn index f8d7483..c3bddab 100644 --- a/hlr/HLR_Tests.ttcn +++ b/hlr/HLR_Tests.ttcn @@ -81,26 +81,38 @@ type record HLR_ConnHdlrPars { HlrSubscriber sub, - HLR_ConnHdlrParsUssd ussd optional + HLR_ConnHdlrParsUssd ussd optional, + octetstring source_name optional } type record HLR_ConnHdlrParsUssd { OCT4 sid } -template (value) HLR_ConnHdlrPars t_Pars(hexstring imsi, hexstring msisdn := ''H) := { +template (value) HLR_ConnHdlrPars t_Pars(hexstring imsi, hexstring msisdn := ''H, +template (omit) octetstring source_name := omit) := { sub := { imsi := imsi, msisdn := msisdn, aud2g := omit, aud3g := omit }, - ussd := omit + ussd := omit, + source_name := source_name } +template (value) HLR_ConnHdlrPars t_Pars_via_proxy(hexstring imsi, hexstring msisdn := ''H) := + t_Pars(imsi, msisdn, source_name := '7468652d736f757263650a'O); // "the-source" template (value) HLR_ConnHdlrPars t_Pars_sub(HlrSubscriber sub) := { sub := sub, - ussd := omit + ussd := omit, + source_name := omit +} + +template (value) HLR_ConnHdlrPars t_Pars_sub_via_proxy(HlrSubscriber sub) := { + sub := sub, + ussd := omit, + source_name := '7468652d736f757263650a'O // "the-source" } type function void_fn() runs on HLR_ConnHdlr; @@ -407,7 +419,8 @@ } function f_perform_UL(hexstring imsi, template hexstring msisdn, - template (omit) integer exp_err_cause := omit) + template (omit) integer exp_err_cause := omit, + template (omit) octetstring source_name := omit) runs on HLR_ConnHdlr return GSUP_PDU { var GSUP_PDU ret; timer T := 3.0; @@ -417,34 +430,34 @@ exp_fail := true; } - GSUP.send(valueof(ts_GSUP_UL_REQ(imsi))); + GSUP.send(valueof(ts_GSUP_UL_REQ(imsi, source_name := source_name))); T.start; alt { - [exp_fail] GSUP.receive(tr_GSUP_UL_ERR(imsi, exp_err_cause)) -> value ret { + [exp_fail] GSUP.receive(tr_GSUP_UL_ERR(imsi, exp_err_cause, destination_name := source_name)) -> value ret { setverdict(pass); } - [exp_fail] GSUP.receive(tr_GSUP_UL_ERR(imsi, ?)) -> value ret { + [exp_fail] GSUP.receive(tr_GSUP_UL_ERR(imsi, ?, destination_name := source_name)) -> value ret { setverdict(fail, "Unexpected UL ERROR Cause"); mtc.stop; } - [exp_fail] GSUP.receive(tr_GSUP_UL_RES(imsi)) -> value ret { + [exp_fail] GSUP.receive(tr_GSUP_UL_RES(imsi, destination_name := source_name)) -> value ret { setverdict(fail, "Unexpected UL.res for unknown IMSI"); mtc.stop; } - [exp_fail] GSUP.receive(tr_GSUP_ISD_REQ(imsi)) -> value ret { + [exp_fail] GSUP.receive(tr_GSUP_ISD_REQ(imsi, destination_name := source_name)) -> value ret { setverdict(fail, "Unexpected ISD.req in error case"); mtc.stop; } - [not exp_fail] GSUP.receive(tr_GSUP_UL_ERR(imsi, ?)) -> value ret { + [not exp_fail] GSUP.receive(tr_GSUP_UL_ERR(imsi, ?, destination_name := source_name)) -> value ret { setverdict(fail, "Unexpected UL ERROR"); mtc.stop; } - [not exp_fail and not isd_done] GSUP.receive(tr_GSUP_ISD_REQ(imsi, msisdn)) -> valu
Change in libosmocore[master]: fix OSMO_SOCKADDR_STR_FMT for IPv6
neels has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15959 ) Change subject: fix OSMO_SOCKADDR_STR_FMT for IPv6 .. fix OSMO_SOCKADDR_STR_FMT for IPv6 The format prints IP:port separated by a colon, which of course is confusing when the IPv6 address itself contains mostly colons. The new format adds square braces. cafe:face::1:42 -> [cafe:face::1]:42 The IPv4 format remains unchanged: 1.2.3.4:42 Change-Id: I161f8427729ae31be0eac719b7a4a9290715e37f --- M include/osmocom/core/sockaddr_str.h M tests/sockaddr_str/sockaddr_str_test.ok 2 files changed, 17 insertions(+), 13 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved pespin: Looks good to me, approved diff --git a/include/osmocom/core/sockaddr_str.h b/include/osmocom/core/sockaddr_str.h index d47b2a4..6dd428c 100644 --- a/include/osmocom/core/sockaddr_str.h +++ b/include/osmocom/core/sockaddr_str.h @@ -61,8 +61,12 @@ * struct osmo_sockaddr_str *my_sockaddr_str = ...; * printf("got " OSMO_SOCKADDR_STR_FMT "\n", OSMO_SOCKADDR_STR_FMT_ARGS(my_sockaddr_str)); */ -#define OSMO_SOCKADDR_STR_FMT "%s:%u" -#define OSMO_SOCKADDR_STR_FMT_ARGS(R) ((R)? (R)->ip : "NULL"), ((R)? (R)->port : 0) +#define OSMO_SOCKADDR_STR_FMT "%s%s%s:%u" +#define OSMO_SOCKADDR_STR_FMT_ARGS(R) \ + ((R) && (R)->af == AF_INET6)? "[" : "", \ + (R)? (R)->ip : "NULL", \ + ((R) && (R)->af == AF_INET6)? "]" : "", \ + (R)? (R)->port : 0 bool osmo_sockaddr_str_is_set(const struct osmo_sockaddr_str *sockaddr_str); bool osmo_sockaddr_str_is_nonzero(const struct osmo_sockaddr_str *sockaddr_str); diff --git a/tests/sockaddr_str/sockaddr_str_test.ok b/tests/sockaddr_str/sockaddr_str_test.ok index 5ebf7be..bc18225 100644 --- a/tests/sockaddr_str/sockaddr_str_test.ok +++ b/tests/sockaddr_str/sockaddr_str_test.ok @@ -86,7 +86,7 @@ { .af = AF_INET6, .ip = "1:2:3::4", .port = 5 } - OSMO_SOCKADDR_STR_FMT: '1:2:3::4:5' + OSMO_SOCKADDR_STR_FMT: '[1:2:3::4]:5' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = true osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -103,7 +103,7 @@ { .af = AF_INET6, .ip = "::", .port = 0 } - OSMO_SOCKADDR_STR_FMT: ':::0' + OSMO_SOCKADDR_STR_FMT: '[::]:0' osmo_sockaddr_str_is_set() = false osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -120,7 +120,7 @@ { .af = AF_INET6, .ip = "::1", .port = 0 } - OSMO_SOCKADDR_STR_FMT: '::1:0' + OSMO_SOCKADDR_STR_FMT: '[::1]:0' osmo_sockaddr_str_is_set() = false osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -137,7 +137,7 @@ { .af = AF_INET6, .ip = ":::::::", .port = 65535 } - OSMO_SOCKADDR_STR_FMT: '::::::::65535' + OSMO_SOCKADDR_STR_FMT: '[:::::::]:65535' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = true osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -154,7 +154,7 @@ { .af = AF_INET6, .ip = ":::::::", .port = 65535 } - OSMO_SOCKADDR_STR_FMT: '::::::::65535' + OSMO_SOCKADDR_STR_FMT: '[:::::::]:65535' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = true osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -174,7 +174,7 @@ { .af = AF_INET6, .ip = "::f", .port = 1 } - OSMO_SOCKADDR_STR_FMT: '::f:1' + OSMO_SOCKADDR_STR_FMT: '[::f]:1' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -188,7 +188,7 @@ { .af = AF_INET6, .ip = "not an ip address", .port = 1 } - OSMO_SOCKADDR_STR_FMT: 'not an ip address:1' + OSMO_SOCKADDR_STR_FMT: '[not an ip address]:1' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -202,7 +202,7 @@ { .af = AF_INET6, .ip = "1.2.3.4", .port = 5 } - OSMO_SOCKADDR_STR_FMT: '1.2.3.4:5' + OSMO_SOCKADDR_STR_FMT: '[1.2.3.4]:5' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -261,7 +261,7 @@ { .af = AF_INET6, .ip = "", .port = 5 } - OSMO_SOCKADDR_STR_FMT: ':5' + OSMO_SOCKADDR_STR_FMT: '[]:5' osmo_sockaddr_str_is_set() = false osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -309,7 +309,7 @@ { .af = AF_I
Change in libosmocore[master]: add osmo_sockaddr_str_cmp() and _str_ip_cmp()
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15961 ) Change subject: add osmo_sockaddr_str_cmp() and _str_ip_cmp() .. Patch Set 4: (1 comment) https://gerrit.osmocom.org/c/libosmocore/+/15961/4/src/sockaddr_str.c File src/sockaddr_str.c: https://gerrit.osmocom.org/c/libosmocore/+/15961/4/src/sockaddr_str.c@104 PS4, Line 104: int osmo_sockaddr_str_cmp(const struct osmo_sockaddr_str *a, const struct osmo_sockaddr_str *b) > It's not clear to me why do you want to have this one as a public API having > the other. […] This patch first had only this one, and later I figured it would be very useful to also compare by actual resulting IP address. I figured, to sort a list of addresses by the string representation chosen by the user, then this cmp() would be more desirable, so I was just going to keep this. indeed I don't currently have a caller for it... I'm slightly reluctant, but fair enough then. In that case I'll just have one osmo_sockaddr_str_cmp() that compares resulting IP addresses and keep the pure string variant static. If we need it later it can be called osmo_sockaddr_str_cmp_by_string() or something. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15961 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501 Gerrit-Change-Number: 15961 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Mon, 11 Nov 2019 18:43:05 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in osmo-msc[master]: cc trans: remove unused tch_rtp_create
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/15962 ) Change subject: cc trans: remove unused tch_rtp_create .. cc trans: remove unused tch_rtp_create Use of this flag was dropped when adding inter-BSC and inter-MSC Handover support, I forgot to remove it. Change-Id: I5ec78e30eb36fbe78a3f7c46bfa44af5a4eb7bf2 --- M include/osmocom/msc/transaction.h M src/libmsc/gsm_04_08_cc.c 2 files changed, 0 insertions(+), 18 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/include/osmocom/msc/transaction.h b/include/osmocom/msc/transaction.h index 61d8c1a..928b137 100644 --- a/include/osmocom/msc/transaction.h +++ b/include/osmocom/msc/transaction.h @@ -87,10 +87,6 @@ /* bearer capabilities (rate and codec) */ struct gsm_mncc_bearer_cap bearer_cap; - /* if true, TCH_RTP_CREATE is sent after the -* assignment is done */ - bool tch_rtp_create; - union { struct { diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c index 792ab61..7e2d70b 100644 --- a/src/libmsc/gsm_04_08_cc.c +++ b/src/libmsc/gsm_04_08_cc.c @@ -1655,20 +1655,6 @@ } 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. -* msc_mgcp_call_assignment() also takes care about sending the CRCX -* command to the MGCP-GW. The CRCX will return the port number, -* where the PBX (e.g. Asterisk) will send its RTP stream to. We -* have to return this port number back to the MNCC by sending -* it back with the TCH_RTP_CREATE message. To make sure that -* this message is sent AFTER the response to CRCX from the -* MGCP-GW has arrived, we need will instruct msc_mgcp_call_assignment() -* to take care of this by setting trans->tch_rtp_create to true. -* This will make sure that gsm48_tch_rtp_create() (below) is -* called as soon as the local port number has become known. */ - trans->tch_rtp_create = true; - /* Assign call (if not done yet) */ return msc_a_try_call_assignment(trans); } -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15962 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I5ec78e30eb36fbe78a3f7c46bfa44af5a4eb7bf2 Gerrit-Change-Number: 15962 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-MessageType: merged
Change in osmo-msc[master]: fix msc_vlr_test_call.c
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/15347 ) Change subject: fix msc_vlr_test_call.c .. Patch Set 3: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15347 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ie995e264eb1e3dd9558a1753ff6f9b55c1d084e1 Gerrit-Change-Number: 15347 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Mon, 11 Nov 2019 20:26:52 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15957 ) Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. Patch Set 3: nice catch, thx! -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Mon, 11 Nov 2019 18:31:32 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
Hello laforge, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/15957 to look at the new patch set (#7). Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. utils.h: add OSMO_NAME_C_IMPL() macro Provide a common implementation for foo_name_c() functions that base on foo_name_buf() functions. char *foo_name_c(void *ctx, example_t arg) { OSMO_NAME_C_IMPL(ctx, 64, "ERROR", foo_name_buf, arg) } Rationale: the most efficient way of composing strings that have optional parts or require loops for composition is by writing to a ready char[], and this in turn is easiest done by using OSMO_STRBUF_* API. Using such a basic name string implementation which typically returns a length, I often want a more convenient version that returns a char*, which can just be inlined in a "%s" string format -- crucially: skipping string composition when inlined in a LOGP(). This common implementation allows saving code dup, only the function signature is needed. Why not include the function signature in the macro? The two sets of varargs (1: signature args, 2: function call args) are hard to do. Also, having an explicit signature is good for readability and code grepping / ctags. Upcoming uses: in libosmocore in the mslookup (D-GSM) implementation (osmo_mslookup_result_name_c()), and in osmo_msc's codec negotiation implementation (sdp_audio_codecs_name_c(), sdp_msg_name_c(), ...). I54b6c0810f181259da307078977d9ef3d90458c9 (libosmocore) If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 (osmo-msc) Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 --- M include/osmocom/core/utils.h M tests/utils/utils_test.c M tests/utils/utils_test.ok 3 files changed, 157 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/57/15957/7 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in libosmocore[master]: fix osmo_quote_str_c() to alloc sufficient size
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/16029 ) Change subject: fix osmo_quote_str_c() to alloc sufficient size .. Patch Set 4: (1 comment) osmo_quote_str* functions would make for an interesting anecdotal review of various ways of composing strings that we (I?) thought were great but later needed improving upon. https://gerrit.osmocom.org/c/libosmocore/+/16029/4/src/utils.c File src/utils.c: https://gerrit.osmocom.org/c/libosmocore/+/16029/4/src/utils.c@780 PS4, Line 780: * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()). interestingly enough, the \return doc didn't need any change, it was plain wrong for osmo_quote_str_buf2() :/ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16029 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I04d97e8eec93ffb74006503c356a68cceaf429ac Gerrit-Change-Number: 16029 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Comment-Date: Mon, 11 Nov 2019 20:25:40 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in libosmocore[master]: test: add OSMO_SOCKADDR_STR_FMT to sockaddr_str_test.c
neels has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15958 ) Change subject: test: add OSMO_SOCKADDR_STR_FMT to sockaddr_str_test.c .. test: add OSMO_SOCKADDR_STR_FMT to sockaddr_str_test.c This shows the weird format choice for showing IPv6 addresses' port, fixed in subsequent patch. Change-Id: I8e5ebfbbc3a2b88aed820e8f845d9f6ededb29de --- M tests/sockaddr_str/sockaddr_str_test.c M tests/sockaddr_str/sockaddr_str_test.ok 2 files changed, 24 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/tests/sockaddr_str/sockaddr_str_test.c b/tests/sockaddr_str/sockaddr_str_test.c index 4284387..bf7d738 100644 --- a/tests/sockaddr_str/sockaddr_str_test.c +++ b/tests/sockaddr_str/sockaddr_str_test.c @@ -107,6 +107,8 @@ printf("\n\n"); dump_oip(x); + printf(" OSMO_SOCKADDR_STR_FMT: '" OSMO_SOCKADDR_STR_FMT "'\n", + OSMO_SOCKADDR_STR_FMT_ARGS(x)); printf(" osmo_sockaddr_str_is_set() = %s\n", osmo_sockaddr_str_is_set(x) ? "true" : "false"); printf(" osmo_sockaddr_str_is_nonzero() = %s\n", osmo_sockaddr_str_is_nonzero(x) ? "true" : "false"); diff --git a/tests/sockaddr_str/sockaddr_str_test.ok b/tests/sockaddr_str/sockaddr_str_test.ok index 781e9d1..5ebf7be 100644 --- a/tests/sockaddr_str/sockaddr_str_test.ok +++ b/tests/sockaddr_str/sockaddr_str_test.ok @@ -1,6 +1,7 @@ { .af = AF_INET, .ip = "1.2.3.4", .port = 5 } + OSMO_SOCKADDR_STR_FMT: '1.2.3.4:5' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = true osmo_sockaddr_str_to_in_addr() rc == 0 in_addr=01020304 @@ -19,6 +20,7 @@ { .af = AF_INET, .ip = "0.0.0.0", .port = 0 } + OSMO_SOCKADDR_STR_FMT: '0.0.0.0:0' osmo_sockaddr_str_is_set() = false osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc == 0 in_addr= @@ -37,6 +39,7 @@ { .af = AF_INET, .ip = "255.255.255.255", .port = 65535 } + OSMO_SOCKADDR_STR_FMT: '255.255.255.255:65535' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = true osmo_sockaddr_str_to_in_addr() rc == 0 in_addr= @@ -55,6 +58,7 @@ { .af = AF_INET, .ip = "0.0.0.256", .port = 1 } + OSMO_SOCKADDR_STR_FMT: '0.0.0.256:1' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -68,6 +72,7 @@ { .af = AF_INET, .ip = "not an ip address", .port = 1 } + OSMO_SOCKADDR_STR_FMT: 'not an ip address:1' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -81,6 +86,7 @@ { .af = AF_INET6, .ip = "1:2:3::4", .port = 5 } + OSMO_SOCKADDR_STR_FMT: '1:2:3::4:5' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = true osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -97,6 +103,7 @@ { .af = AF_INET6, .ip = "::", .port = 0 } + OSMO_SOCKADDR_STR_FMT: ':::0' osmo_sockaddr_str_is_set() = false osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -113,6 +120,7 @@ { .af = AF_INET6, .ip = "::1", .port = 0 } + OSMO_SOCKADDR_STR_FMT: '::1:0' osmo_sockaddr_str_is_set() = false osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -129,6 +137,7 @@ { .af = AF_INET6, .ip = ":::::::", .port = 65535 } + OSMO_SOCKADDR_STR_FMT: '::::::::65535' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = true osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -145,6 +154,7 @@ { .af = AF_INET6, .ip = ":::::::", .port = 65535 } + OSMO_SOCKADDR_STR_FMT: '::::::::65535' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = true osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -164,6 +174,7 @@ { .af = AF_INET6, .ip = "::f", .port = 1 } + OSMO_SOCKADDR_STR_FMT: '::f:1' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -177,6 +188,7 @@ { .af = AF_INET6, .ip = "not an ip address", .port = 1 } + OSMO_SOCKADDR_STR_FMT: 'not an ip address:1' osmo_sockaddr_str_is_set() = true osmo_sockaddr_str_is_nonzero() = false osmo_sockaddr_str_to_in_addr() rc < 0 in_addr= @@ -190,6 +202,7 @@ { .af = AF_INET6, .ip =
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
Hello laforge, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/15957 to look at the new patch set (#6). Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. utils.h: add OSMO_NAME_C_IMPL() macro Provide a common implementation for foo_name_c() functions that base on foo_name_buf() functions. char *foo_name_c(void *ctx, example_t arg) { OSMO_NAME_C_IMPL(ctx, 64, "ERROR", foo_name_buf, arg) } Rationale: the most efficient way of composing strings that have optional parts or require loops for composition is by writing to a ready char[], and this in turn is easiest done by using OSMO_STRBUF_* API. Using such a basic name string implementation which typically returns a length, I often want a more convenient version that returns a char*, which can just be inlined in a "%s" string format -- crucially: skipping string composition when inlined in a LOGP(). This common implementation allows saving code dup, only the function signature is needed. Why not include the function signature in the macro? The two sets of varargs (1: signature args, 2: function call args) are hard to do. Also, having an explicit signature is good for readability and code grepping / ctags. Upcoming uses: in libosmocore in the mslookup (D-GSM) implementation (osmo_mslookup_result_name_c()), and in osmo_msc's codec negotiation implementation (sdp_audio_codecs_name_c(), sdp_msg_name_c(), ...). I54b6c0810f181259da307078977d9ef3d90458c9 (libosmocore) If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 (osmo-msc) Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 --- M include/osmocom/core/utils.h M tests/utils/utils_test.c M tests/utils/utils_test.ok 3 files changed, 157 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/57/15957/6 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 6 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in libosmocore[master]: add osmo_sockaddr_str_cmp()
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/15961 to look at the new patch set (#5). Change subject: add osmo_sockaddr_str_cmp() .. add osmo_sockaddr_str_cmp() Currently planned user: for Distributed GSM in osmo-hlr: setting per-MSC service addresses in VTY: replace/remove existing entries. osmo_sockaddr_str_cmp() is useful to catch identical resulting IP addresses, regardless of differing strings (e.g. '0::' and '::' are equal but differ in strings). Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501 --- M include/osmocom/core/sockaddr_str.h M include/osmocom/core/utils.h M src/sockaddr_str.c M tests/sockaddr_str/sockaddr_str_test.c M tests/sockaddr_str/sockaddr_str_test.ok 5 files changed, 681 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/61/15961/5 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15961 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501 Gerrit-Change-Number: 15961 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in libosmocore[master]: osmo_sockaddr_str: fix 32bit addr mixup of host/network byte order
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16030 ) Change subject: osmo_sockaddr_str: fix 32bit addr mixup of host/network byte order .. osmo_sockaddr_str: fix 32bit addr mixup of host/network byte order Of course both v4 and v6 addresses are kept in network byte order when represented in bytes, but when writing, I somehow must have assumed that inet_pton() returns host byte order. Fix that mixup: osmo_sockaddr_str_from_32() and osmo_sockaddr_str_to_32() actually use network byte order. Fix the doc strings. osmo_sockaddr_str_from_32n() and osmo_sockaddr_str_to_32n() actually use host byte order, though reflecting 'n' in their name. Deprecate these. Add osmo_sockaddr_str_from_32h() and osmo_sockaddr_str_to_32h() that are identical to the "32n" variants, but without the confusing names. sockaddr_str_test: use hexdump instead of %x to show the osmo_sockaddr_str_to_32*() conversions so that the error becomes obvious. (Printing %x reverses the bytes again and made it look correct.) Change-Id: I3cf150cc0cc06dd36039fbde091bc71b01697322 --- M TODO-RELEASE M include/osmocom/core/sockaddr_str.h M src/sockaddr_str.c M tests/sockaddr_str/sockaddr_str_test.c M tests/sockaddr_str/sockaddr_str_test.ok 5 files changed, 92 insertions(+), 66 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/30/16030/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index 692bdc1..be858ae 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -11,3 +11,7 @@ core struct osmo_tdeffields min_val,max_val added, ABI break (arrays of structs used in programs) gsmAPI added osmo_gsm48_rfpowercap2powerclass() gb API added bssgp_bvc_ctx_free() +core osmo_sockaddr_str_from_32n(), + osmo_sockaddr_str_to_32n() Deprecate: named 'n' but use host byte order. +core osmo_sockaddr_str_from_32h(), + osmo_sockaddr_str_to_32h() New, use host byte order and are named appropriately. diff --git a/include/osmocom/core/sockaddr_str.h b/include/osmocom/core/sockaddr_str.h index d7a8cdf..e42216a 100644 --- a/include/osmocom/core/sockaddr_str.h +++ b/include/osmocom/core/sockaddr_str.h @@ -31,6 +31,7 @@ #include #include #include +#include struct in_addr; struct in6_addr; @@ -77,7 +78,7 @@ int osmo_sockaddr_str_from_in_addr(struct osmo_sockaddr_str *sockaddr_str, const struct in_addr *addr, uint16_t port); int osmo_sockaddr_str_from_in6_addr(struct osmo_sockaddr_str *sockaddr_str, const struct in6_addr *addr, uint16_t port); int osmo_sockaddr_str_from_32(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port); -int osmo_sockaddr_str_from_32n(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port); +int osmo_sockaddr_str_from_32h(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port); int osmo_sockaddr_str_from_sockaddr_in(struct osmo_sockaddr_str *sockaddr_str, const struct sockaddr_in *src); int osmo_sockaddr_str_from_sockaddr_in6(struct osmo_sockaddr_str *sockaddr_str, const struct sockaddr_in6 *src); int osmo_sockaddr_str_from_sockaddr(struct osmo_sockaddr_str *sockaddr_str, const struct sockaddr_storage *src); @@ -85,9 +86,14 @@ int osmo_sockaddr_str_to_in_addr(const struct osmo_sockaddr_str *sockaddr_str, struct in_addr *dst); int osmo_sockaddr_str_to_in6_addr(const struct osmo_sockaddr_str *sockaddr_str, struct in6_addr *dst); int osmo_sockaddr_str_to_32(const struct osmo_sockaddr_str *sockaddr_str, uint32_t *ip); -int osmo_sockaddr_str_to_32n(const struct osmo_sockaddr_str *sockaddr_str, uint32_t *ip); +int osmo_sockaddr_str_to_32h(const struct osmo_sockaddr_str *sockaddr_str, uint32_t *ip); int osmo_sockaddr_str_to_sockaddr_in(const struct osmo_sockaddr_str *sockaddr_str, struct sockaddr_in *dst); int osmo_sockaddr_str_to_sockaddr_in6(const struct osmo_sockaddr_str *sockaddr_str, struct sockaddr_in6 *dst); int osmo_sockaddr_str_to_sockaddr(const struct osmo_sockaddr_str *sockaddr_str, struct sockaddr_storage *dst); +int osmo_sockaddr_str_from_32n(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port) + OSMO_DEPRECATED("osmo_sockaddr_str_from_32n() actually uses *host* byte order. Use osmo_sockaddr_str_from_32h() instead"); +int osmo_sockaddr_str_to_32n(const struct osmo_sockaddr_str *sockaddr_str, uint32_t *ip) + OSMO_DEPRECATED("osmo_sockaddr_str_to_32n() actually uses *host* byte order. Use osmo_sockaddr_str_to_32h() instead"); + /*! @} */ diff --git a/src/sockaddr_str.c b/src/sockaddr_str.c index 7a81866..72534b3 100644 --- a/src/sockaddr_str.c +++ b/src/sockaddr_str.c @@ -258,7 +258,7 @@ return 0; } -/*! Convert IPv4 address from 32bit host-byte-order to osmo_sockaddr_str, and set port. +/*! Convert IPv4 address from 32bit network-b
Change in libosmocore[master]: fix osmo_quote_str_c() to alloc sufficient size
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16029 ) Change subject: fix osmo_quote_str_c() to alloc sufficient size .. fix osmo_quote_str_c() to alloc sufficient size osmo_quote_str_c() allocates too small buffers when the quoted string also contains unprintable characters that need escaping (which of course are longer than strlen() + 3 that it currently naively assumes). Add osmo_quote_str_buf3() to actually return the required buffer size (sb.chars_needed), osmo_quote_str_buf2() unfortunately returns the char buffer itself. Hence implement osmo_quote_str_c() using OSMO_NAME_C_IMPL(), which now always allocates sufficient size. Change-Id: I04d97e8eec93ffb74006503c356a68cceaf429ac --- M include/osmocom/core/utils.h M src/utils.c 2 files changed, 18 insertions(+), 17 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/29/16029/1 diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h index ef58eeb..cb69b64 100644 --- a/include/osmocom/core/utils.h +++ b/include/osmocom/core/utils.h @@ -151,6 +151,7 @@ char *osmo_escape_str_c(const void *ctx, const char *str, int in_len); const char *osmo_quote_str(const char *str, int in_len); char *osmo_quote_str_buf2(char *buf, size_t bufsize, const char *str, int in_len); +size_t osmo_quote_str_buf3(char *buf, size_t bufsize, const char *str, int in_len); const char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t bufsize); char *osmo_quote_str_c(const void *ctx, const char *str, int in_len); diff --git a/src/utils.c b/src/utils.c index ea1de0f..95245f5 100644 --- a/src/utils.c +++ b/src/utils.c @@ -756,6 +756,20 @@ return osmo_escape_str_buf2(buf, in_len+1, str, in_len); } +/*! Like osmo_quote_str_buf3(), but returning the buf instead of the required number of characters. + * The function signature is suitable for OSMO_STRBUF_APPEND_NOLEN(). + * \param[out] buf string buffer to write escaped characters to. + * \param[in] bufsize sizeof(buf). + * \param[in] str A string that may contain any characters. + * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length. + * \return buf. + */ +char *osmo_quote_str_buf2(char *buf, size_t bufsize, const char *str, int in_len) +{ + osmo_quote_str_buf3(buf, bufsize, str, in_len); + return buf; +} + /*! Like osmo_escape_str_buf2(), but returns double-quotes around a string, or "NULL" for a NULL string. * This allows passing any char* value and get its C representation as string. * The function signature is suitable for OSMO_STRBUF_APPEND_NOLEN(). @@ -765,7 +779,7 @@ * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length. * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()). */ -char *osmo_quote_str_buf2(char *buf, size_t bufsize, const char *str, int in_len) +size_t osmo_quote_str_buf3(char *buf, size_t bufsize, const char *str, int in_len) { struct osmo_strbuf sb = { .buf = buf, .len = bufsize }; if (!str) @@ -775,7 +789,7 @@ OSMO_STRBUF_APPEND_NOLEN(sb, osmo_escape_str_buf2, str, in_len); OSMO_STRBUF_PRINTF(sb, "\""); } - return buf; + return sb.chars_needed; } /*! Like osmo_quote_str_buf2, but with unusual ordering of arguments, and may sometimes return string constants instead @@ -815,21 +829,7 @@ */ char *osmo_quote_str_c(const void *ctx, const char *str, int in_len) { - size_t len = in_len == -1 ? strlen(str) : in_len; - char *buf; - - /* account for two quote characters + terminating NUL */ - len += 3; - - /* some minimum length for things like "NULL" or "(error)" */ - if (len < 32) - len = 32; - - buf = talloc_size(ctx, len); - if (!buf) - return NULL; - - return osmo_quote_str_buf2(buf, len, str, in_len); + OSMO_NAME_C_IMPL(ctx, 32, "(error)", osmo_quote_str_buf3, str, in_len) } /*! perform an integer square root operation on unsigned 32bit integer. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16029 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I04d97e8eec93ffb74006503c356a68cceaf429ac Gerrit-Change-Number: 16029 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
Hello laforge, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/15957 to look at the new patch set (#4). Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. utils.h: add OSMO_NAME_C_IMPL() macro Provide a common implementation for foo_name_c() functions that base on foo_name_buf() functions. char *foo_name_c(void *ctx, example_t arg) { OSMO_NAME_C_IMPL(ctx, 64, "ERROR", foo_name_buf, arg) } Rationale: the most efficient way of composing strings that have optional parts or require loops for composition is by writing to a ready char[], and this in turn is easiest done by using OSMO_STRBUF_* API. Using such a basic name string implementation which typically returns a length, I often want a more convenient version that returns a char*, which can just be inlined in a "%s" string format -- crucially: skipping string composition when inlined in a LOGP(). This common implementation allows saving code dup, only the function signature is needed. Why not include the function signature in the macro? The two sets of varargs (1: signature args, 2: function call args) are hard to do. Also, having an explicit signature is good for readability and code grepping / ctags. Upcoming uses: in libosmocore in the mslookup (D-GSM) implementation (osmo_mslookup_result_name_c()), and in osmo_msc's codec negotiation implementation (sdp_audio_codecs_name_c(), sdp_msg_name_c(), ...). I54b6c0810f181259da307078977d9ef3d90458c9 (libosmocore) If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 (osmo-msc) Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 --- M include/osmocom/core/utils.h M tests/utils/utils_test.c M tests/utils/utils_test.ok 3 files changed, 156 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/57/15957/4 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15957 ) Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. Patch Set 7: (1 comment) https://gerrit.osmocom.org/c/libosmocore/+/15957/7/include/osmocom/core/utils.h File include/osmocom/core/utils.h: https://gerrit.osmocom.org/c/libosmocore/+/15957/7/include/osmocom/core/utils.h@313 PS7, Line 313: _str = (char*)talloc_named_const(CTX, _len, __func__); \ I also had to not use realloc here, for embedded builds (pseudotalloc cannot do talloc_realloc_size()) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Mon, 11 Nov 2019 20:19:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-hlr[master]: add db_upgrade test
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15913 ) Change subject: add db_upgrade test .. Patch Set 2: hrm the changes in sqlite3 clients makes it hard to get this test to act reproducably... -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15913 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 Gerrit-Change-Number: 15913 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Thu, 31 Oct 2019 16:57:01 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-hlr[master]: add --db-check option
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15912 ) Change subject: add --db-check option .. Patch Set 2: (1 comment) https://gerrit.osmocom.org/c/osmo-hlr/+/15912/1/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/c/osmo-hlr/+/15912/1/src/hlr.c@866 PS1, Line 866: rc = telnet_init_dynif(hlr_ctx, NULL, vty_get_bind_addr(), > huh I thought I had moved it?? I think I lost a fixup somewhere. checking... I had squashed it to the wrong patch, thanks for spotting -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15912 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I1a4b3360690acd2cd3cffdadffbb00a28d421316 Gerrit-Change-Number: 15912 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Thu, 31 Oct 2019 16:54:59 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: neels Gerrit-MessageType: comment
Change in osmo-hlr[master]: hlr.sql: move comment
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15928 ) Change subject: hlr.sql: move comment .. hlr.sql: move comment Move a comment for ind_bitlen column to a separate line, so that it doesn't show in PRAGMA_TABLE_INFO('subscriber'). An upcoming patch introduces db_upgrade_test, which dumps a sorted db schema. In newer sqlite3 versions, a comment following a 'DEFAULT' keyword actually shows up in the PRAGMA_TABLE_INFO() results (on my machine), but older versions (on the build slaves) drop that comment. The ind_bitlen column is the only one producing this odd side effect, because it is the last column and has no comma between 'DEFAULT' and the comment. The easiest way to get matching results across sqlite3 client versions is to move the comment to above ind_bitlen instead of having it on the same line. (Adding a comma doesn't work.) Change-Id: Id66ad68dd3f22d533fc3a428223ea6ad0282bde0 --- M sql/hlr.sql 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/28/15928/1 diff --git a/sql/hlr.sql b/sql/hlr.sql index 10838f2..c1b0f1a 100644 --- a/sql/hlr.sql +++ b/sql/hlr.sql @@ -69,7 +69,8 @@ op VARCHAR(32),-- hex string: operator's secret key (128bit) opc VARCHAR(32),-- hex string: derived from OP and K (128bit) sqn INTEGER NOT NULL DEFAULT 0, -- sequence number of key usage - ind_bitlen INTEGER NOT NULL DEFAULT 5 -- nr of index bits at lower SQN end + -- nr of index bits at lower SQN end + ind_bitlen INTEGER NOT NULL DEFAULT 5 ); CREATE UNIQUE INDEX idx_subscr_imsi ON subscriber (imsi); -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15928 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: Id66ad68dd3f22d533fc3a428223ea6ad0282bde0 Gerrit-Change-Number: 15928 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-hlr[master]: hlr db schema 3: hlr_number -> msc_number
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15914 ) Change subject: hlr db schema 3: hlr_number -> msc_number .. Patch Set 4: argh, it seems the build slave's sqlite version doesn't support "RENAME COLUMN" -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15914 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I527e8627b24b79f3e9eec32675c7f5a3a6d25440 Gerrit-Change-Number: 15914 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Comment-Date: Thu, 31 Oct 2019 19:21:49 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-hlr[master]: add db_upgrade test
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/15913 to look at the new patch set (#4). Change subject: add db_upgrade test .. add db_upgrade test We have a database schema upgrade path, but so far nothing that verifies that we don't break it. It almost seems like the user data weren't important to us!? Add a db upgrade test: - Create a db with an .sql dump taken from a db created with an old osmo-hlr, producing DB schema version 0. - Run osmo-hlr --db-upgrade --db-check - Verify that the upgrade exited successfully. - Verify that restarting with the upgraded DB works. If python tests are enabled, also: - create a new database using the new osmo-hlr (just built). - replay a VTY transcript to create subscribers as in the old snapshot. - replay some sql modifications as done in the old snapshot. - Get a list of sorted table names, - a list of their sorted columns with all their properties, - and dump the table contents in a column- and value-sorted way. - Compare the resulting dumps and error if there are any diffs. (This is how I found the difference in the imei column that was fixed in I68a00014a3d603fcba8781470bc5285f78b538d0) Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 --- M configure.ac M tests/Makefile.am A tests/db_upgrade/Makefile.am A tests/db_upgrade/create_subscribers.vty A tests/db_upgrade/create_subscribers_step2.sql A tests/db_upgrade/db_upgrade_test.err A tests/db_upgrade/db_upgrade_test.ok A tests/db_upgrade/db_upgrade_test.sh A tests/db_upgrade/hlr_db_v0.sql A tests/db_upgrade/osmo-hlr.cfg M tests/testsuite.at 11 files changed, 416 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/13/15913/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15913 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 Gerrit-Change-Number: 15913 Gerrit-PatchSet: 4 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in osmo-hlr[master]: add osmo_gsup_conn_send_err_reply()
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/15916 to look at the new patch set (#5). Change subject: add osmo_gsup_conn_send_err_reply() .. add osmo_gsup_conn_send_err_reply() Remove hlr.c's static gsup_send_err_reply(), and create new osmo_gsup_conn_send_err_reply(), as used in osmo-msc. It includes more of the newer IEs in the response, like an SS/USSD session id. Prepares for adding D-GSM / MS lookup, which will need this function moved to a non-static context. Change-Id: I792fd9993ab2a323af58782a357d71205c43b72a --- M src/gsup_server.c M src/gsup_server.h M src/hlr.c 3 files changed, 58 insertions(+), 23 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/16/15916/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15916 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I792fd9993ab2a323af58782a357d71205c43b72a Gerrit-Change-Number: 15916 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-MessageType: newpatchset
Change in osmo-hlr[master]: db.c: code dup: add db_run_statements() for arrays of statements
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15929 ) Change subject: db.c: code dup: add db_run_statements() for arrays of statements .. db.c: code dup: add db_run_statements() for arrays of statements The db bootstrap as well as the upgrade code all execute a number of statements, and have massive code dup around each statement execution. Instead have one db_run_statements() that takes an array of strings and runs all. Change-Id: I2721dfc0a9aadcc7f5ac81a1c0fa87452098996f --- M src/db.c 1 file changed, 34 insertions(+), 74 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/29/15929/1 diff --git a/src/db.c b/src/db.c index d2564e6..75ca889 100644 --- a/src/db.c +++ b/src/db.c @@ -201,28 +201,38 @@ talloc_free(dbc); } -static int db_bootstrap(struct db_context *dbc) +static int db_run_statements(struct db_context *dbc, const char **statements, size_t statements_count) { + int rc; int i; - for (i = 0; i < ARRAY_SIZE(stmt_bootstrap_sql); i++) { - int rc; + for (i = 0; i < statements_count; i++) { + const char *stmt_str = statements[i]; sqlite3_stmt *stmt; - rc = sqlite3_prepare_v2(dbc->db, stmt_bootstrap_sql[i], -1, , NULL); + + rc = sqlite3_prepare_v2(dbc->db, stmt_str, -1, , NULL); if (rc != SQLITE_OK) { - LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", stmt_bootstrap_sql[i]); + LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", stmt_str); return rc; } - rc = sqlite3_step(stmt); db_remove_reset(stmt); sqlite3_finalize(stmt); if (rc != SQLITE_DONE) { - LOGP(DDB, LOGL_ERROR, "Cannot bootstrap database: SQL error: (%d) %s," -" during stmt '%s'", -rc, sqlite3_errmsg(dbc->db), stmt_bootstrap_sql[i]); + LOGP(DDB, LOGL_ERROR, "SQL error: (%d) %s, during stmt '%s'", +rc, sqlite3_errmsg(dbc->db), stmt_str); return rc; } } + return rc; +} + +static int db_bootstrap(struct db_context *dbc) +{ + int rc = db_run_statements(dbc, stmt_bootstrap_sql, ARRAY_SIZE(stmt_bootstrap_sql)); + if (rc != SQLITE_DONE) { + LOGP(DDB, LOGL_ERROR, "Cannot bootstrap database\n"); + return rc; + } return SQLITE_OK; } @@ -263,75 +273,38 @@ static int db_upgrade_v1(struct db_context *dbc) { - sqlite3_stmt *stmt; int rc; - const char *update_stmt_sql = "ALTER TABLE subscriber ADD COLUMN last_lu_seen TIMESTAMP default NULL"; - const char *set_schema_version_sql = "PRAGMA user_version = 1"; + const char *statements[] = { + "ALTER TABLE subscriber ADD COLUMN last_lu_seen TIMESTAMP default NULL", + "PRAGMA user_version = 1", + }; - rc = sqlite3_prepare_v2(dbc->db, update_stmt_sql, -1, , NULL); - if (rc != SQLITE_OK) { - LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", update_stmt_sql); - return rc; - } - rc = sqlite3_step(stmt); - db_remove_reset(stmt); - sqlite3_finalize(stmt); + rc = db_run_statements(dbc, statements, ARRAY_SIZE(statements)); if (rc != SQLITE_DONE) { - LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version %d\n", 1); + LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version 1\n"); return rc; } - - rc = sqlite3_prepare_v2(dbc->db, set_schema_version_sql, -1, , NULL); - if (rc != SQLITE_OK) { - LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", set_schema_version_sql); - return rc; - } - rc = sqlite3_step(stmt); - if (rc != SQLITE_DONE) - LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version %d\n", 1); - - db_remove_reset(stmt); - sqlite3_finalize(stmt); return rc; } static int db_upgrade_v2(struct db_context *dbc) { - sqlite3_stmt *stmt; int rc; - const char *update_stmt_sql = "ALTER TABLE subscriber ADD COLUMN imei VARCHAR(14)"; - const char *set_schema_version_sql = "PRAGMA user_version = 2"; + const char *statements[] = { + "ALTER TABLE subscriber ADD COLUMN imei VARCHAR(14)", + "PRAGMA user_version = 2&quo
Change in osmo-hlr[master]: add db_upgrade test
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/15913 to look at the new patch set (#5). Change subject: add db_upgrade test .. add db_upgrade test We have a database schema upgrade path, but so far nothing that verifies that we don't break it. It almost seems like the user data weren't important to us!? Add a db upgrade test: - Create a db with an .sql dump taken from a db created with an old osmo-hlr, producing DB schema version 0. - Run osmo-hlr --db-upgrade --db-check - Verify that the upgrade exited successfully. - Verify that restarting with the upgraded DB works. If python tests are enabled, also: - create a new database using the new osmo-hlr (just built). - replay a VTY transcript to create subscribers as in the old snapshot. - replay some sql modifications as done in the old snapshot. - Get a list of sorted table names, - a list of their sorted columns with all their properties, - and dump the table contents in a column- and value-sorted way. - Compare the resulting dumps and error if there are any diffs. (This is how I found the difference in the imei column that was fixed in I68a00014a3d603fcba8781470bc5285f78b538d0) Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 --- M configure.ac M tests/Makefile.am A tests/db_upgrade/Makefile.am A tests/db_upgrade/create_subscribers.vty A tests/db_upgrade/create_subscribers_step2.sql A tests/db_upgrade/db_upgrade_test.err A tests/db_upgrade/db_upgrade_test.ok A tests/db_upgrade/db_upgrade_test.sh A tests/db_upgrade/hlr_db_v0.sql A tests/db_upgrade/osmo-hlr.cfg M tests/testsuite.at 11 files changed, 417 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/13/15913/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15913 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 Gerrit-Change-Number: 15913 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in osmo-hlr[master]: add --db-check option
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15912 ) Change subject: add --db-check option .. Patch Set 1: (1 comment) https://gerrit.osmocom.org/c/osmo-hlr/+/15912/1/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/c/osmo-hlr/+/15912/1/src/hlr.c@866 PS1, Line 866: rc = telnet_init_dynif(hlr_ctx, NULL, vty_get_bind_addr(), > Isn't this one here opening a port already? huh I thought I had moved it?? I think I lost a fixup somewhere. checking... -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15912 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I1a4b3360690acd2cd3cffdadffbb00a28d421316 Gerrit-Change-Number: 15912 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Thu, 31 Oct 2019 16:46:00 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in osmo-hlr[master]: add db_upgrade test
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/15913 to look at the new patch set (#2). Change subject: add db_upgrade test .. add db_upgrade test We have a database schema upgrade path, but so far nothing that verifies that we don't break it. It almost seems like the user data weren't important to us!? Add a db upgrade test: - Create a db with an .sql dump taken from a db created with an old osmo-hlr, producing DB schema version 0. - Run osmo-hlr --db-upgrade --db-check - Verify that the upgrade exited successfully. - Verify that restarting with the upgraded DB works. If python tests are enabled, also: - create a new database using the new osmo-hlr (just built). - replay a VTY transcript to create subscribers as in the old snapshot. - replay some sql modifications as done in the old snapshot. - Get a list of sorted table names, - a list of their sorted columns with all their properties, - and dump the table contents in a column- and value-sorted way. - Compare the resulting dumps and error if there are any diffs. (This is how I found the difference in the imei column that was fixed in I68a00014a3d603fcba8781470bc5285f78b538d0) Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 --- M configure.ac M tests/Makefile.am A tests/db_upgrade/Makefile.am A tests/db_upgrade/create_subscribers.vty A tests/db_upgrade/create_subscribers_step2.sql A tests/db_upgrade/db_upgrade_test.err A tests/db_upgrade/db_upgrade_test.ok A tests/db_upgrade/db_upgrade_test.sh A tests/db_upgrade/hlr_db_v0.sql A tests/db_upgrade/osmo-hlr.cfg M tests/testsuite.at 11 files changed, 415 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/13/15913/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15913 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 Gerrit-Change-Number: 15913 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in osmo-hlr[master]: hlr db schema 3: hlr_number -> msc_number
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/15914 to look at the new patch set (#2). Change subject: hlr db schema 3: hlr_number -> msc_number .. hlr db schema 3: hlr_number -> msc_number The osmo-hlr DB schema indicates a hlr_number column and references it as 3GPP TS 23.008 chapter 2.4.6. However, chapter 2.4.6 refers to the "MSC number", while the "HLR number" is chapter 2.4.7. Taking a closer look, 2.4.6 says "The MSC number is [...] stored in the HLR", while 2.4.7 says "The HLR number may be stored in the VLR". As quite obvious, the HLR does not store the HLR number. This was a typo from the start. The osmo-hlr code base so far does not use the hlr_number column at all, so we get away with renaming the column without any effects on the code base. However, let's rather make this a new schema version to be safe. Change-Id: I527e8627b24b79f3e9eec32675c7f5a3a6d25440 --- M doc/manuals/chapters/subscribers.adoc M sql/hlr.sql M src/db.c M tests/db_upgrade/db_upgrade_test.ok 4 files changed, 57 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/14/15914/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15914 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I527e8627b24b79f3e9eec32675c7f5a3a6d25440 Gerrit-Change-Number: 15914 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset
Change in osmo-hlr[master]: add --db-check option
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/15912 to look at the new patch set (#2). Change subject: add --db-check option .. add --db-check option This allows starting osmo-hlr to merely open the database, do upgrades if necessary, and quit, without opening any ports. So that no ports are opened, move the telnet VTY startup to below the database check. Needed for upcoming patch that introduces a db_upgrade test, in I0961bab0e17cfde5b030576c5bc243c2b51d9dc4. Change-Id: I1a4b3360690acd2cd3cffdadffbb00a28d421316 --- M src/hlr.c 1 file changed, 23 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/12/15912/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15912 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I1a4b3360690acd2cd3cffdadffbb00a28d421316 Gerrit-Change-Number: 15912 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in osmo-hlr[master]: hlr db schema 3: hlr_number -> msc_number
Hello fixeria, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hlr/+/15914 to look at the new patch set (#5). Change subject: hlr db schema 3: hlr_number -> msc_number .. hlr db schema 3: hlr_number -> msc_number The osmo-hlr DB schema indicates a hlr_number column and references it as 3GPP TS 23.008 chapter 2.4.6. However, chapter 2.4.6 refers to the "MSC number", while the "HLR number" is chapter 2.4.7. Taking a closer look, 2.4.6 says "The MSC number is [...] stored in the HLR", while 2.4.7 says "The HLR number may be stored in the VLR". As quite obvious, the HLR does not store the HLR number. This was a typo from the start. The osmo-hlr code base so far does not use the hlr_number column at all, so we get away with renaming the column without any effects on the code base. However, let's rather make this a new schema version to be safe. Change-Id: I527e8627b24b79f3e9eec32675c7f5a3a6d25440 --- M doc/manuals/chapters/subscribers.adoc M sql/hlr.sql M src/db.c M tests/db_upgrade/db_upgrade_test.ok 4 files changed, 157 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/14/15914/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15914 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I527e8627b24b79f3e9eec32675c7f5a3a6d25440 Gerrit-Change-Number: 15914 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-MessageType: newpatchset
Change in osmo-hlr[master]: add db_upgrade test
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15913 ) Change subject: add db_upgrade test .. add db_upgrade test We have a database schema upgrade path, but so far nothing that verifies that we don't break it. It almost seems like the user data weren't important to us!? Add a db upgrade test: - Create a db with an .sql dump taken from a db created with an old osmo-hlr, producing DB schema version 0. - Run osmo-hlr --db-upgrade --db-check - Verify that the upgrade exited successfully. - Verify that restarting with the upgraded DB works. If python tests are enabled, also: - create a new database using the new osmo-hlr (just built). - replay a VTY transcript to create subscribers as in the old snapshot. - replay some sql modifications as done in the old snapshot. - Get a list of sorted table names, - a list of their sorted columns with all their properties, - and dump the table contents in a column- and value-sorted way. - Compare the resulting dumps and error if there are any diffs. (This is how I found the difference in the imei column that was fixed in I68a00014a3d603fcba8781470bc5285f78b538d0) Change-Id: I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 --- M configure.ac M src/hlr.c M tests/Makefile.am A tests/db_upgrade/Makefile.am A tests/db_upgrade/create_subscribers.vty A tests/db_upgrade/create_subscribers_step2.sql A tests/db_upgrade/db_upgrade_test.err A tests/db_upgrade/db_upgrade_test.ok A tests/db_upgrade/db_upgrade_test.sh A tests/db_upgrade/hlr_db_v0.sql A tests/db_upgrade/osmo-hlr.cfg M tests/testsuite.at 12 files changed, 434 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/13/15913/1 diff --git a/configure.ac b/configure.ac index 993d4d5..ca78f38 100644 --- a/configure.ac +++ b/configure.ac @@ -186,4 +186,5 @@ tests/gsup_server/Makefile tests/gsup/Makefile tests/db/Makefile + tests/db_upgrade/Makefile ) diff --git a/src/hlr.c b/src/hlr.c index f9cc2f5..6bfc141 100644 --- a/src/hlr.c +++ b/src/hlr.c @@ -862,12 +862,6 @@ return rc; } - /* start telnet after reading config for vty_get_bind_addr() */ - rc = telnet_init_dynif(hlr_ctx, NULL, vty_get_bind_addr(), - OSMO_VTY_PORT_HLR); - if (rc < 0) - return rc; - LOGP(DMAIN, LOGL_NOTICE, "hlr starting\n"); rc = rand_init(); @@ -895,6 +889,13 @@ exit(0); } + /* start telnet after reading config for vty_get_bind_addr() */ + rc = telnet_init_dynif(hlr_ctx, NULL, vty_get_bind_addr(), + OSMO_VTY_PORT_HLR); + if (rc < 0) + return rc; + + g_hlr->gs = osmo_gsup_server_create(hlr_ctx, g_hlr->gsup_bind_addr, OSMO_GSUP_PORT, read_cb, _lu_ops, g_hlr); if (!g_hlr->gs) { diff --git a/tests/Makefile.am b/tests/Makefile.am index 4da8ab1..357fbac 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -3,6 +3,7 @@ gsup_server \ db \ gsup \ + db_upgrade \ $(NULL) # The `:;' works around a Bash 3.2 bug when the output is not writeable. @@ -44,6 +45,7 @@ # don't run vty and ctrl tests concurrently so that the ports don't conflict $(MAKE) vty-test $(MAKE) ctrl-test + $(MAKE) db-upgrade-equivalence-test else python-tests: echo "Not running python-based external tests (determined at configure-time)" @@ -81,6 +83,9 @@ -rm -f $(CTRL_TEST_DB) -rm $(CTRL_TEST_DB)-* +db-upgrade-equivalence-test: + $(MAKE) -C db_upgrade upgrade-equivalence-test + check-local: atconfig $(TESTSUITE) $(SHELL) '$(TESTSUITE)' $(TESTSUITEFLAGS) $(MAKE) $(AM_MAKEFLAGS) python-tests diff --git a/tests/db_upgrade/Makefile.am b/tests/db_upgrade/Makefile.am new file mode 100644 index 000..79136c9 --- /dev/null +++ b/tests/db_upgrade/Makefile.am @@ -0,0 +1,14 @@ +EXTRA_DIST = \ + db_upgrade_test.sh \ + db_upgrade_test.err \ + db_upgrade_test.ok \ + hlr_db_v0.sql \ + osmo-hlr.cfg \ + create_subscribers.vty \ + $(NULL) + +update_exp: + $(srcdir)/db_upgrade_test.sh $(srcdir) $(builddir) >"$(srcdir)/db_upgrade_test.ok" 2>"$(srcdir)/db_upgrade_test.err" + +upgrade-equivalence-test: + $(srcdir)/db_upgrade_test.sh $(srcdir) $(builddir) do-equivalence-test diff --git a/tests/db_upgrade/create_subscribers.vty b/tests/db_upgrade/create_subscribers.vty new file mode 100644 index 000..30eeba6 --- /dev/null +++ b/tests/db_upgrade/create_subscribers.vty @@ -0,0 +1,47 @@ +OsmoHLR> enable +OsmoHLR# subscriber imsi 123456789012345 create +% Created subscriber 123456789012345 +ID: 1 +IMSI: 123456789012345 +MSISDN: none +OsmoHL
Change in osmo-hlr[master]: fix upgrade to version 2: imei column default value
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15911 ) Change subject: fix upgrade to version 2: imei column default value .. fix upgrade to version 2: imei column default value A subsequent commit will add a db_upgrade test, which verifies that the db resulting from an upgrade is identical to one created from scratch in the new version. That test currently would show a diff: an upgraded 'imei' column has 'default NULL', where a new db created in version 2 has no default value on the imei column. Fix the upgrade path to add an imei column without 'default NULL', so that adding the upgrade test will result in success. The test is added in I0961bab0e17cfde5b030576c5bc243c2b51d9dc4 Change-Id: I68a00014a3d603fcba8781470bc5285f78b538d0 --- M src/db.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/11/15911/1 diff --git a/src/db.c b/src/db.c index f3ed863..31c4ba5 100644 --- a/src/db.c +++ b/src/db.c @@ -299,7 +299,7 @@ { sqlite3_stmt *stmt; int rc; - const char *update_stmt_sql = "ALTER TABLE subscriber ADD COLUMN imei VARCHAR(14) default NULL"; + const char *update_stmt_sql = "ALTER TABLE subscriber ADD COLUMN imei VARCHAR(14)"; const char *set_schema_version_sql = "PRAGMA user_version = 2"; rc = sqlite3_prepare_v2(dbc->db, update_stmt_sql, -1, , NULL); -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15911 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I68a00014a3d603fcba8781470bc5285f78b538d0 Gerrit-Change-Number: 15911 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-hlr[master]: db upgrade to v2: log version 2, not 1
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15910 ) Change subject: db upgrade to v2: log version 2, not 1 .. db upgrade to v2: log version 2, not 1 Change-Id: I9237b64e5748e693a5f039c5a5554d417eed3633 --- M src/db.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/10/15910/1 diff --git a/src/db.c b/src/db.c index 5e6b5eb..f3ed863 100644 --- a/src/db.c +++ b/src/db.c @@ -311,7 +311,7 @@ db_remove_reset(stmt); sqlite3_finalize(stmt); if (rc != SQLITE_DONE) { - LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version %d\n", 1); + LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version 2\n"); return rc; } @@ -322,7 +322,7 @@ } rc = sqlite3_step(stmt); if (rc != SQLITE_DONE) - LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version %d\n", 1); + LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version 2\n"); db_remove_reset(stmt); sqlite3_finalize(stmt); -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15910 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I9237b64e5748e693a5f039c5a5554d417eed3633 Gerrit-Change-Number: 15910 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-hlr[master]: add osmo_gsup_msgb_alloc()
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15915 ) Change subject: add osmo_gsup_msgb_alloc() .. add osmo_gsup_msgb_alloc() Throughout osmo-hlr's code, the GSUP msgb allocation is duplicated as: msgb_alloc_headroom(1024+16, 16, "foo"); Instead, use one common function to keep the magic numbers in one place. Change-Id: I40e99b5bc4fd8f750da7643c03b2119ac3bfd95e --- M src/gsup_server.c M src/gsup_server.h M src/hlr.c M src/hlr_ussd.c M src/luop.c 5 files changed, 17 insertions(+), 9 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/15/15915/1 diff --git a/src/gsup_server.c b/src/gsup_server.c index e75bbd7..2870e9d 100644 --- a/src/gsup_server.c +++ b/src/gsup_server.c @@ -30,6 +30,13 @@ #include "gsup_server.h" #include "gsup_router.h" +struct msgb *osmo_gsup_msgb_alloc(const char *label) +{ + struct msgb *msg = msgb_alloc_headroom(1024+16, 16, label); + OSMO_ASSERT(msg); + return msg; +} + static void osmo_gsup_server_send(struct osmo_gsup_conn *conn, int proto_ext, struct msgb *msg_tx) { diff --git a/src/gsup_server.h b/src/gsup_server.h index 9c4d483..14f5013 100644 --- a/src/gsup_server.h +++ b/src/gsup_server.h @@ -47,6 +47,7 @@ bool supports_ps; /* client supports OSMO_GSUP_CN_DOMAIN_PS */ }; +struct msgb *osmo_gsup_msgb_alloc(const char *label); int osmo_gsup_conn_send(struct osmo_gsup_conn *conn, struct msgb *msg); int osmo_gsup_conn_ccm_get(const struct osmo_gsup_conn *clnt, uint8_t **addr, diff --git a/src/hlr.c b/src/hlr.c index 6bfc141..aef890e 100644 --- a/src/hlr.c +++ b/src/hlr.c @@ -128,7 +128,7 @@ } /* Send ISD to MSC/SGSN */ - msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP ISD UPDATE"); + msg_out = osmo_gsup_msgb_alloc("GSUP ISD UPDATE"); if (msg_out == NULL) { LOGP(DLGSUP, LOGL_ERROR, "IMSI='%s': Cannot notify GSUP client; could not allocate msg buffer " @@ -271,7 +271,7 @@ gsup_out.num_auth_vectors = rc; } - msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP AUC response"); + msg_out = osmo_gsup_msgb_alloc("GSUP AUC response"); osmo_gsup_encode(msg_out, _out); return osmo_gsup_conn_send(conn, msg_out); } @@ -451,7 +451,7 @@ gsup_reply.cause = GMM_CAUSE_NET_FAIL; } - msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP AUC response"); + msg_out = osmo_gsup_msgb_alloc("GSUP AUC response"); osmo_gsup_encode(msg_out, _reply); return osmo_gsup_conn_send(conn, msg_out); } @@ -466,7 +466,7 @@ OSMO_STRLCPY_ARRAY(gsup_reply.imsi, imsi); gsup_reply.message_type = type_err; gsup_reply.cause = err_cause; - msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP ERR response"); + msg_out = osmo_gsup_msgb_alloc("GSUP ERR response"); OSMO_ASSERT(msg_out); osmo_gsup_encode(msg_out, _reply); LOGP(DMAIN, LOGL_NOTICE, "Tx %s\n", osmo_gsup_message_type_name(type_err)); @@ -525,7 +525,7 @@ /* Accept all IMEIs */ gsup_reply.imei_result = OSMO_GSUP_IMEI_RESULT_ACK; gsup_reply.message_type = OSMO_GSUP_MSGT_CHECK_IMEI_RESULT; - msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP Check_IMEI response"); + msg_out = osmo_gsup_msgb_alloc("GSUP Check_IMEI response"); memcpy(gsup_reply.imsi, gsup->imsi, sizeof(gsup_reply.imsi)); osmo_gsup_encode(msg_out, _reply); return osmo_gsup_conn_send(conn, msg_out); @@ -591,7 +591,7 @@ end: /* Send error back to source */ if (ret) { - struct msgb *msg_err = msgb_alloc_headroom(1024+16, 16, "GSUP forward ERR response"); + struct msgb *msg_err = osmo_gsup_msgb_alloc("GSUP forward ERR response"); OSMO_ASSERT(msg_err); gsup_err->message_type = OSMO_GSUP_MSGT_E_ROUTING_ERROR; osmo_gsup_encode(msg_err, gsup_err); diff --git a/src/hlr_ussd.c b/src/hlr_ussd.c index 1568815..be0fee0 100644 --- a/src/hlr_ussd.c +++ b/src/hlr_ussd.c @@ -464,7 +464,7 @@ } if (is_euse_originated) { - msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP USSD FW"); + msg_out = osmo_gsup_msgb_alloc("GSUP USSD FW"); OSMO_ASSERT(msg_out); /* Received from EUSE, Forward to VLR */ osmo_gsup_encode(msg_out, gsup); @@ -481,7 +481,7 @@ LOGPSS(ss, LOGL_ERROR, "Cannot find conn for EUSE %s\n", addr);
Change in osmo-hlr[master]: add osmo_gsup_conn_send_err_reply()
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15916 ) Change subject: add osmo_gsup_conn_send_err_reply() .. add osmo_gsup_conn_send_err_reply() Remove hlr.c's static gsup_send_err_reply(), and create new osmo_gsup_conn_send_err_reply(), as used in osmo-msc. It includes more of the newer IEs in the response, like an SS/USSD session id. Prepares for adding D-GSM / MS lookup, which will need this function moved to a non-static context. Change-Id: I792fd9993ab2a323af58782a357d71205c43b72a --- M src/gsup_server.c M src/gsup_server.h M src/hlr.c 3 files changed, 41 insertions(+), 23 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/16/15916/1 diff --git a/src/gsup_server.c b/src/gsup_server.c index 2870e9d..b08d6ce 100644 --- a/src/gsup_server.c +++ b/src/gsup_server.c @@ -57,6 +57,39 @@ return 0; } +int osmo_gsup_conn_send_err_reply(struct osmo_gsup_conn *conn, const struct osmo_gsup_message *gsup_orig, + enum gsm48_gmm_cause cause) +{ + struct osmo_gsup_message gsup_reply; + struct msgb *msg_out; + + /* No need to answer if we couldn't parse an ERROR message type, only REQUESTs need an error reply. */ + if (!OSMO_GSUP_IS_MSGT_REQUEST(gsup_orig->message_type)) + return 0; + + gsup_reply = (struct osmo_gsup_message){ + .cause = cause, + .message_type = OSMO_GSUP_TO_MSGT_ERROR(gsup_orig->message_type), + .message_class = gsup_orig->message_class, + + /* RP-Message-Reference is mandatory for SM Service */ + .sm_rp_mr = gsup_orig->sm_rp_mr, + }; + + OSMO_STRLCPY_ARRAY(gsup_reply.imsi, gsup_orig->imsi); + + /* For SS/USSD, it's important to keep both session state and ID IEs */ + if (gsup_orig->session_state != OSMO_GSUP_SESSION_STATE_NONE) { + gsup_reply.session_state = OSMO_GSUP_SESSION_STATE_END; + gsup_reply.session_id = gsup_orig->session_id; + } + + msg_out = osmo_gsup_msgb_alloc("GSUP ERR response"); + OSMO_ASSERT(msg_out); + osmo_gsup_encode(msg_out, _reply); + return osmo_gsup_conn_send(conn, msg_out); +} + static int osmo_gsup_conn_oap_handle(struct osmo_gsup_conn *conn, struct msgb *msg_rx) { diff --git a/src/gsup_server.h b/src/gsup_server.h index 14f5013..1f67c29 100644 --- a/src/gsup_server.h +++ b/src/gsup_server.h @@ -50,6 +50,8 @@ struct msgb *osmo_gsup_msgb_alloc(const char *label); int osmo_gsup_conn_send(struct osmo_gsup_conn *conn, struct msgb *msg); +int osmo_gsup_conn_send_err_reply(struct osmo_gsup_conn *conn, const struct osmo_gsup_message *gsup_orig, + enum gsm48_gmm_cause cause); int osmo_gsup_conn_ccm_get(const struct osmo_gsup_conn *clnt, uint8_t **addr, uint8_t tag); diff --git a/src/hlr.c b/src/hlr.c index aef890e..0cc0448 100644 --- a/src/hlr.c +++ b/src/hlr.c @@ -456,23 +456,6 @@ return osmo_gsup_conn_send(conn, msg_out); } -static int gsup_send_err_reply(struct osmo_gsup_conn *conn, const char *imsi, - enum osmo_gsup_message_type type_in, uint8_t err_cause) -{ - int type_err = OSMO_GSUP_TO_MSGT_ERROR(type_in); - struct osmo_gsup_message gsup_reply = {0}; - struct msgb *msg_out; - - OSMO_STRLCPY_ARRAY(gsup_reply.imsi, imsi); - gsup_reply.message_type = type_err; - gsup_reply.cause = err_cause; - msg_out = osmo_gsup_msgb_alloc("GSUP ERR response"); - OSMO_ASSERT(msg_out); - osmo_gsup_encode(msg_out, _reply); - LOGP(DMAIN, LOGL_NOTICE, "Tx %s\n", osmo_gsup_message_type_name(type_err)); - return osmo_gsup_conn_send(conn, msg_out); -} - static int rx_check_imei_req(struct osmo_gsup_conn *conn, const struct osmo_gsup_message *gsup) { struct osmo_gsup_message gsup_reply = {0}; @@ -483,7 +466,7 @@ /* Require IMEI */ if (!gsup->imei_enc) { LOGP(DMAIN, LOGL_ERROR, "%s: missing IMEI\n", gsup->imsi); - gsup_send_err_reply(conn, gsup->imsi, gsup->message_type, GMM_CAUSE_INV_MAND_INFO); + osmo_gsup_conn_send_err_reply(conn, gsup, GMM_CAUSE_INV_MAND_INFO); return -1; } @@ -491,7 +474,7 @@ rc = gsm48_decode_bcd_number2(imei, sizeof(imei), gsup->imei_enc, gsup->imei_enc_len, 0); if (rc < 0) { LOGP(DMAIN, LOGL_ERROR, "%s: failed to decode IMEI (rc: %i)\n", gsup->imsi, rc); - gsup_send_err_reply(conn, gsup->imsi, gsup->message_type, GMM_CAUSE_INV_MAND_INFO); + osmo_gsup_conn_send_err_reply(conn, gsup, GMM_CAUSE_INV_MAND_INFO);
Change in osmo-hlr[master]: add --db-check option
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15912 ) Change subject: add --db-check option .. add --db-check option This allows starting osmo-hlr to merely open the database, do upgrades if necessary, and quit, without opening any ports. Needed for upcoming patch that introduces a db_upgrade test, in I0961bab0e17cfde5b030576c5bc243c2b51d9dc4. Change-Id: I1a4b3360690acd2cd3cffdadffbb00a28d421316 --- M src/hlr.c 1 file changed, 16 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/12/15912/1 diff --git a/src/hlr.c b/src/hlr.c index 8b9dff1..f9cc2f5 100644 --- a/src/hlr.c +++ b/src/hlr.c @@ -706,6 +706,7 @@ printf(" -T --timestamp Prefix every log line with a timestamp.\n"); printf(" -e --log-level number Set a global loglevel.\n"); printf(" -U --db-upgradeAllow HLR database schema upgrades.\n"); + printf(" -C --db-check Quit after opening (and upgrading) the database.\n"); printf(" -V --version Print the version of OsmoHLR.\n"); } @@ -714,6 +715,7 @@ const char *db_file; bool daemonize; bool db_upgrade; + bool db_check; } cmdline_opts = { .config_file = "osmo-hlr.cfg", .db_file = NULL, @@ -735,6 +737,7 @@ {"log-level", 1, 0, 'e'}, {"timestamp", 0, 0, 'T'}, {"db-upgrade", 0, 0, 'U' }, + {"db-check", 0, 0, 'C' }, {"version", 0, 0, 'V' }, {0, 0, 0, 0} }; @@ -773,6 +776,9 @@ case 'U': cmdline_opts.db_upgrade = true; break; + case 'C': + cmdline_opts.db_check = true; + break; case 'V': print_version(1); exit(0); @@ -879,6 +885,16 @@ exit(1); } + if (cmdline_opts.db_check) { + LOGP(DMAIN, LOGL_NOTICE, "Cmdline option --db-check: Database was opened successfully, quitting.\n"); + db_close(g_hlr->dbc); + log_fini(); + talloc_free(hlr_ctx); + talloc_free(tall_vty_ctx); + talloc_disable_null_tracking(); + exit(0); + } + g_hlr->gs = osmo_gsup_server_create(hlr_ctx, g_hlr->gsup_bind_addr, OSMO_GSUP_PORT, read_cb, _lu_ops, g_hlr); if (!g_hlr->gs) { -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/15912 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I1a4b3360690acd2cd3cffdadffbb00a28d421316 Gerrit-Change-Number: 15912 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-hlr[master]: hlr db schema 3: hlr_number -> msc_number
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/15914 ) Change subject: hlr db schema 3: hlr_number -> msc_number .. hlr db schema 3: hlr_number -> msc_number The osmo-hlr DB schema indicates a hlr_number column and references it as 3GPP TS 23.008 chapter 2.4.6. However, chapter 2.4.6 refers to the "MSC number", while the "HLR number" is chapter 2.4.7. Taking a closer look, 2.4.6 says "The MSC number is [...] stored in the HLR", while 2.4.7 says "The HLR number may be stored in the VLR". As quite obvious, the HLR does not store the HLR number. This was a typo from the start. The osmo-hlr code base so far does not use the hlr_number column at all, so we get away with renaming the column without any effects on the code base. However, let's rather make this a new schema version to be safe. Change-Id: I527e8627b24b79f3e9eec32675c7f5a3a6d25440 --- M doc/manuals/chapters/subscribers.adoc M sql/hlr.sql M src/db.c M tests/db_upgrade/db_upgrade_test.ok 4 files changed, 50 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/14/15914/1 diff --git a/doc/manuals/chapters/subscribers.adoc b/doc/manuals/chapters/subscribers.adoc index e09e99a..ab41b0f 100644 --- a/doc/manuals/chapters/subscribers.adoc +++ b/doc/manuals/chapters/subscribers.adoc @@ -52,7 +52,7 @@ |aud3g.ind_bitlen|5|Nr of index bits at lower SQN end |apn|| |vlr_number||3GPP TS 23.008 chapter 2.4.5 -|hlr_number||3GPP TS 23.008 chapter 2.4.6 +|msc_number||3GPP TS 23.008 chapter 2.4.6 |sgsn_number||3GPP TS 23.008 chapter 2.4.8.1 |sgsn_address||3GPP TS 23.008 chapter 2.13.10 |ggsn_number||3GPP TS 23.008 chapter 2.4.8.2 diff --git a/sql/hlr.sql b/sql/hlr.sql index 10838f2..a627f0c 100644 --- a/sql/hlr.sql +++ b/sql/hlr.sql @@ -12,7 +12,7 @@ -- Chapter 2.4.5 vlr_number VARCHAR(15), -- Chapter 2.4.6 - hlr_number VARCHAR(15), + msc_number VARCHAR(15), -- Chapter 2.4.8.1 sgsn_number VARCHAR(15), -- Chapter 2.13.10 @@ -76,4 +76,4 @@ -- Set HLR database schema version number -- Note: This constant is currently duplicated in src/db.c and must be kept in sync! -PRAGMA user_version = 2; +PRAGMA user_version = 3; diff --git a/src/db.c b/src/db.c index 31c4ba5..4ae491e 100644 --- a/src/db.c +++ b/src/db.c @@ -28,7 +28,7 @@ #include "db_bootstrap.h" /* This constant is currently duplicated in sql/hlr.sql and must be kept in sync! */ -#define CURRENT_SCHEMA_VERSION 2 +#define CURRENT_SCHEMA_VERSION 3 #define SEL_COLUMNS \ "id," \ @@ -329,6 +329,40 @@ return rc; } +static int db_upgrade_v3(struct db_context *dbc) +{ + sqlite3_stmt *stmt; + int rc; + const char *update_stmt_sql = "ALTER TABLE subscriber RENAME COLUMN hlr_number TO msc_number"; + const char *set_schema_version_sql = "PRAGMA user_version = 3"; + + rc = sqlite3_prepare_v2(dbc->db, update_stmt_sql, -1, , NULL); + if (rc != SQLITE_OK) { + LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", update_stmt_sql); + return rc; + } + rc = sqlite3_step(stmt); + db_remove_reset(stmt); + sqlite3_finalize(stmt); + if (rc != SQLITE_DONE) { + LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version 3\n"); + return rc; + } + + rc = sqlite3_prepare_v2(dbc->db, set_schema_version_sql, -1, , NULL); + if (rc != SQLITE_OK) { + LOGP(DDB, LOGL_ERROR, "Unable to prepare SQL statement '%s'\n", set_schema_version_sql); + return rc; + } + rc = sqlite3_step(stmt); + if (rc != SQLITE_DONE) + LOGP(DDB, LOGL_ERROR, "Unable to update HLR database schema to version 3\n"); + + db_remove_reset(stmt); + sqlite3_finalize(stmt); + return rc; +} + static int db_get_user_version(struct db_context *dbc) { const char *user_version_sql = "PRAGMA user_version"; @@ -459,6 +493,15 @@ } version = 2; /* fall through */ + case 2: + rc = db_upgrade_v3(dbc); + if (rc != SQLITE_DONE) { + LOGP(DDB, LOGL_ERROR, "Failed to upgrade HLR DB schema to version 3: (rc=%d) %s\n", +rc, sqlite3_errmsg(dbc->db)); + goto out_free; + } + version = 3; + /* fall through */ /* case N: ... */ default: break; diff --git a/tests/db_upgrade/db_upgrade_test.ok b/tes
Change in ...osmo-python-tests[master]: osmo_interact_vty.py: fix py3 encoding bug
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/python/osmo-python-tests/+/15917 ) Change subject: osmo_interact_vty.py: fix py3 encoding bug .. osmo_interact_vty.py: fix py3 encoding bug That code in common.py is hit when invoking via osmo_interact_vty.py. It has a unicode string already, its attempt to decode hits a python exception (no 'decode' method). Must be a long standing bug that no-one saw because we're only ever using the osmo_verify_transcript_vty.py variant. Change-Id: I1b4a629f12863c498a8681b555f57b4e255cebfb --- M osmopy/osmo_interact/common.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/python/osmo-python-tests refs/changes/17/15917/1 diff --git a/osmopy/osmo_interact/common.py b/osmopy/osmo_interact/common.py index 39163a2..cc7e190 100644 --- a/osmopy/osmo_interact/common.py +++ b/osmopy/osmo_interact/common.py @@ -429,7 +429,7 @@ for f_path in (cmd_files or []): with open(f_path, 'r') as f: -interact.feed_commands(output, f.read().decode('utf-8').splitlines()) +interact.feed_commands(output, f.read().splitlines()) if not (cmd_str or cmd_files): while True: -- To view, visit https://gerrit.osmocom.org/c/python/osmo-python-tests/+/15917 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: python/osmo-python-tests Gerrit-Branch: master Gerrit-Change-Id: I1b4a629f12863c498a8681b555f57b4e255cebfb Gerrit-Change-Number: 15917 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in osmo-ttcn3-hacks[master]: msc: overhaul voice call testing
Hello pespin, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938 to look at the new patch set (#3). Change subject: msc: overhaul voice call testing .. msc: overhaul voice call testing * Semantic: We don't really know which side the MSC first creates a CRCX for. Instead of assuming that the RAN side is always CRCX'd first, simply handle a "first" and a "second" CRCX, not making any assumptions which is for which side. Notably, there still are quite a few places assuming which CRCX corresponds to the RAN/CN side, but the changes are sufficient to still pass the tests when osmo-msc swaps the CRCX order; sometimes for slightly obscure reasons, for example because it doesn't matter that the wrong port number is returned during a subsequent MDCX... Cleaning up the rest is still todo for later. Remove code dup from call establishing code, particularly for MGCP. Use f_mo_call_establish() and f_mt_call() where ever possible, to make all of the call establishing tests handle upcoming changes in osmo-msc's order of messages, without re-implementing the changes for each test individually. The X-Osmux parameter was so far expected to appear in the first CRCX received, assuming that this first CRCX is for the RAN. Instead, detect whether X-Osmux is contained in a CRCX, and reply with an Osmux CID if so, regardless of it being the first or second CRCX. Count the number of X-Osmux parameters received in CRCX messages, and compare after call setup to verify X-Osmux presence. Since f_mo_call_establish() can't handle RANAP assignment, a few Iu tests that worked with the older code dup will break by this patch. This is fixed by a subsequent patch, see I0ead36333ab665147b8d222070ea5cf8afc555ec. * Details, per patch chunk: Change ts_BSSMAP_IE_AoIP_TLA4 to a non-value template, so that we can use a wildcard for the assigned port number from MGCP (depending on RAN or CN CRCX first, the RAN port number can be one or the other). In CallParameters, move MGCP handling instructions into a separate record "CrcxResponse", and have two of them for handling the first and the second CRCX, replacing mgw_rtp_{ip,port}_{bss,mss} and mgcp_connection_id_{bss,mss}. In CallParameters, add some flags for early-exiting call establishment with a particular desired behavior, for specialized tests. In CallParameters, use common default values and don't repeat them in each and every call establishing test. Set cpars.mo_call := {true,false} implicitly when f_{mo,mt}_call_establish() are invoked. Remove CRCX comments implying RAN or CN side, instead just talk of the "first" and the "second" CRCX. Implement one common f_handle_crcx() function, which is used by f_mo_call_establish(), f_mt_call_complete(), as_optional_mgcp_crcx(), and implicitly uses the first/second CRCX handling. For Assigment, use a wildcard RTP port so that we don't have to assume which CRCX was for the RAN side. In f_mo_call_establish(), insert special case conditions to make it enact errors at specific times, for individual tests. That saves re-implementing the entire call establishment (code dup). For error cases, add expectation of a CC Release message in the call establishment. This should not apply for normal successful operation, but because interleave does not support conditionals, add flags got_mncc_setup_compl_ind and got_cc_connect to break the interleave when establishing is complete, so that the CC Release is skipped. A CC Release always breaks the interleave, at whatever time it arrives. Tests adopting f_{mo,mt}_call instead of code dup: f_tc_mo_setup_and_nothing() f_tc_mo_crcx_ran_timeout() f_tc_mo_crcx_ran_reject() f_tc_mo_release_timeout() f_tc_mo_cc_bssmap_clear() Change-Id: I8b82476f55a98f7a94d5c4f1cd80eac427b2d20f --- M library/BSSMAP_Templates.ttcn M msc/BSC_ConnectionHandler.ttcn M msc/MSC_Tests.ttcn 3 files changed, 292 insertions(+), 335 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/38/15938/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I8b82476f55a98f7a94d5c4f1cd80eac427b2d20f Gerrit-Change-Number: 15938 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-MessageType: newpatchset
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15957 ) Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. Patch Set 7: (1 comment) https://gerrit.osmocom.org/c/libosmocore/+/15957/7/include/osmocom/core/utils.h File include/osmocom/core/utils.h: https://gerrit.osmocom.org/c/libosmocore/+/15957/7/include/osmocom/core/utils.h@313 PS7, Line 313: _str = (char*)talloc_named_const(CTX, _len, __func__); \ > Why not implementing talloc_realloz_size() with realloc() in pseudotalloc? because pseudotalloc doesn't know the size of the allocated buffer. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-CC: pespin Gerrit-Comment-Date: Wed, 13 Nov 2019 15:47:01 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: neels Gerrit-MessageType: comment
Change in osmo-ttcn3-hacks[master]: msc: overhaul voice call testing
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938 ) Change subject: msc: overhaul voice call testing .. Patch Set 2: (1 comment) https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938/2/library/BSSMAP_Templates.ttcn File library/BSSMAP_Templates.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938/2/library/BSSMAP_Templates.ttcn@a410 PS2, Line 410: template (value) BSSMAP_IE_AoIP_TransportLayerAddress ts_BSSMAP_IE_AoIP_TLA(BSSMAP_FIELD_IPAddress addr, : uint16_t udp_port, : integer len) := { > this looks odd. […] curious, maybe it's a typo of sorts, i'll check... Now I know, we are lacking a tr_* variant of this and were somehow using the ts_* one for receiving an Assignment Request. I never noticed the ts_ name and assumed that the tr_ needs fixing. Adding a tr_. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15938 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I8b82476f55a98f7a94d5c4f1cd80eac427b2d20f Gerrit-Change-Number: 15938 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Comment-Date: Wed, 13 Nov 2019 16:03:22 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Gerrit-MessageType: comment
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15957 ) Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. Patch Set 7: (1 comment) https://gerrit.osmocom.org/c/libosmocore/+/15957/7/include/osmocom/core/utils.h File include/osmocom/core/utils.h: https://gerrit.osmocom.org/c/libosmocore/+/15957/7/include/osmocom/core/utils.h@313 PS7, Line 313: _str = (char*)talloc_named_const(CTX, _len, __func__); \ > because pseudotalloc doesn't know the size of the allocated buffer. hmm, now that I think of it, realloc() would work. But this is much simpler, particularly because we don't need to preserve the data in the buffer. A realloc would require the OS to keep the data intact, so free and new alloc is actually my favorite now anyway. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-CC: pespin Gerrit-Comment-Date: Wed, 13 Nov 2019 15:49:46 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: neels Gerrit-MessageType: comment
Change in libosmocore[master]: osmo_sockaddr_str: fix 32bit addr mixup of host/network byte order
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/16030 ) Change subject: osmo_sockaddr_str: fix 32bit addr mixup of host/network byte order .. Patch Set 2: (1 comment) https://gerrit.osmocom.org/c/libosmocore/+/16030/2/src/sockaddr_str.c File src/sockaddr_str.c: https://gerrit.osmocom.org/c/libosmocore/+/16030/2/src/sockaddr_str.c@409 PS2, Line 409: /*! Convert osmo_sockaddr_str address string to IPv4 address data in host-byte-order. > The API (osmo_sockaddr_str_to_32n) was already defined correctly, since it > states it converted from […] no, that is not an option. osmo_sockaddr_str_to_32n() is already being used in programs. Regardless of what the API doc said and what I was thinking when using that function, if we modify the effect of this function, then we break those other programs. They are using host-byte-order and they need host-byte-order, and are calling osmo_sockaddr_str_{to,from}_32n() to obtain that. They should move to *_32h() whenever ready, we cannot change the behavior of this function without renaming. osmo_sockaddr_str_to_32() already does network byte order correctly. There we can just fix the API doc, because it remains ABI compatible. Actually almost no-one anywhere needs host byte order, except our MNCC socket it seems. I wish I had never added _32n()... but now that it's there, I followed through with the _32h() version. (should be just deprecate without replacement instead?) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16030 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cf150cc0cc06dd36039fbde091bc71b01697322 Gerrit-Change-Number: 16030 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Comment-Date: Wed, 13 Nov 2019 15:45:50 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in osmo-msc[master]: MNCC: add optional SDP to the socket protocol
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/15948 ) Change subject: MNCC: add optional SDP to the socket protocol .. Patch Set 2: (1 comment) https://gerrit.osmocom.org/c/osmo-msc/+/15948/1/src/libmsc/mncc.c File src/libmsc/mncc.c: https://gerrit.osmocom.org/c/osmo-msc/+/15948/1/src/libmsc/mncc.c@266 PS1, Line 266: * char sdp[] starts with a '\0'. */ > for performance reasons I would expect at some point msgb cease to be > zero-initialized. […] The core problem here: I must not assume anything about bytes that remain after the "official" data section in the msgb. So I'll think of a different solution. (We were discussing how msgb might change in API. You are saying that it might not be zero initialized at some point. But also we might choose to randomize unused data for security testing. All that is moot, the byte after the data section in the buffer must not ever matter / be assumed to be anything / be accessed in the first place. Even though it might be an allocated byte of the msgb, the fact that it was never msgb_put() or is later removed again makes it a no-go byte.) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15948 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ie16f0804c4d99760cd4a0c544d0889b6313eebb7 Gerrit-Change-Number: 15948 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Mon, 11 Nov 2019 20:47:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: neels Gerrit-MessageType: comment
Change in osmo-msc[master]: BSSMAP: decode Codec List (BSS Supported)
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/15946 ) Change subject: BSSMAP: decode Codec List (BSS Supported) .. Patch Set 2: (2 comments) https://gerrit.osmocom.org/c/osmo-msc/+/15946/1/src/libmsc/ran_msg_a.c File src/libmsc/ran_msg_a.c: https://gerrit.osmocom.org/c/osmo-msc/+/15946/1/src/libmsc/ran_msg_a.c@127 PS1, Line 127: /* This IE is not critical, do not abort with error. */ > then probably set log level as WARN/NOTICE? I still think it's an error, because the IE was present, but wasn't decodable. We can continue but might end up with a working phone call, or we might end up with codec mismatches or codecs that the BSS doesn't support. I chose to continue to maybe provide the user with a working setup "by coincidence", but the IE was broken. https://gerrit.osmocom.org/c/osmo-msc/+/15946/1/src/libmsc/ran_msg_a.c@129 PS1, Line 129: ran_dec_msg.compl_l3.codec_list_bss_supported = _list_bss_supported; > I have the feeling that's intended and fine here. […] ran_decoded() is the only scope that is using the decoded message. It is intended this way to avoid a lot of allocation and ownership issues. So, to decode a RAN message, the caller provides a callback which ran_decoded() below invokes. Anything that should remain after the callback returns must be copied. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15946 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I66c735c79e982388f06b5de783aa584c9d13569e Gerrit-Change-Number: 15946 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Comment-Date: Mon, 11 Nov 2019 20:33:15 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: laforge Gerrit-MessageType: comment
Change in osmo-ttcn3-hacks[master]: msc: add sdp to MNCC
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/16032 ) Change subject: msc: add sdp to MNCC .. msc: add sdp to MNCC SDP is added to the MNCC protocol in osmo-msc Ie16f0804c4d99760cd4a0c544d0889b6313eebb7. This patch adds SDP to the ttcn3 MNCC messaging. These changes still work with current osmo-msc master that doesn't send SDP / ignores received SDP in MNCC. Change-Id: Ic9568c8927507e161aadfad1a4d20aa896d8ae30 --- M library/MNCC_EncDec.cc M library/MNCC_Types.ttcn M library/mncc.h 3 files changed, 106 insertions(+), 50 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/32/16032/1 diff --git a/library/MNCC_EncDec.cc b/library/MNCC_EncDec.cc index b4937e7..f2692d7 100644 --- a/library/MNCC_EncDec.cc +++ b/library/MNCC_EncDec.cc @@ -259,7 +259,7 @@ case MNCC_RTP_FREE: in_rtp = (const struct gsm_mncc_rtp *) in_mncc; rtp = MNCC__PDU__Rtp(in_rtp->callref, in_rtp->ip, in_rtp->port, in_rtp->payload_type, -in_rtp->payload_msg_type); +in_rtp->payload_msg_type, in_rtp->sdp); u.rtp() = rtp; break; default: @@ -315,6 +315,7 @@ sign.imsi() = CHARSTRING(in_mncc->imsi); sign.lchan__type() = in_mncc->lchan_type; sign.lchan__mode() = in_mncc->lchan_mode; + sign.sdp() = in_mncc->sdp; u.signal() = sign; break; } diff --git a/library/MNCC_Types.ttcn b/library/MNCC_Types.ttcn index 828f341..5296579 100644 --- a/library/MNCC_Types.ttcn +++ b/library/MNCC_Types.ttcn @@ -360,7 +360,9 @@ charstring imsi, uint8_t lchan_type, /* empty in OSmoMSC */ - uint8_t lchan_mode /* empty in OsmoMSC */ + uint8_t lchan_mode, /* empty in OsmoMSC */ + + charstring sdp optional }; @@ -374,7 +376,9 @@ uint32_tip, uint16_trtp_port, uint32_tpayload_type, - uint32_tpayload_msg_type + uint32_tpayload_msg_type, + + charstring sdp optional }; type record MNCC_PDU_Hello { @@ -464,7 +468,8 @@ emergency := omit, imsi := "", lchan_type := 0, - lchan_mode := 0 + lchan_mode := 0, + sdp := "" } } } @@ -494,7 +499,8 @@ emergency := *, imsi := ?, lchan_type := ?, - lchan_mode := ? + lchan_mode := ?, + sdp := * } } } @@ -527,7 +533,8 @@ emergency := omit, imsi := imsi, lchan_type := 0, - lchan_mode := 0 + lchan_mode := 0, + sdp := "" } } }; @@ -559,7 +566,8 @@ emergency := *, imsi := imsi, lchan_type := ?, - lchan_mode := ? + lchan_mode := ?, + sdp := * } } }; @@ -592,7 +600,8 @@ emergency := omit, imsi := imsi, lchan_type := 0, - lchan_mode := 0 + lchan_mode := 0, + sdp := "" } } }; @@ -623,7 +632,8 @@ emergency := omit, imsi := imsi, lchan_type := ?, - lchan_mode := ? + lchan_mode := ?, + sdp := * } } }; @@ -656,7 +666,8 @@ emergency := *, imsi := imsi, lchan_type := ?, - lchan_mode := ? + lchan_mode := ?, + sdp := * } } } @@ -688,7 +699,8 @@ emergency := omit, imsi := imsi, lchan_type := 0, - lchan_mode := 0 + lchan_mode := 0, + sdp := "" } } } @@ -719,7 +731,8 @@ emergency := omit, imsi := "", lchan_type := 0, - lchan_mode := 0 + lchan_mode := 0, + s
Change in libosmo-sccp[master]: ss7: Set ASP default remote addr to 127.0.0.1 if none set in VTY
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/16011 ) Change subject: ss7: Set ASP default remote addr to 127.0.0.1 if none set in VTY .. Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/16011 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: I33672e76a51a5d5a483906749d30e4c4e08b66ce Gerrit-Change-Number: 16011 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Comment-Date: Fri, 08 Nov 2019 14:54:53 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmocore[master]: GSUP: rename E_ROUTING_ERROR to ROUTING_ERROR
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16161 ) Change subject: GSUP: rename E_ROUTING_ERROR to ROUTING_ERROR .. GSUP: rename E_ROUTING_ERROR to ROUTING_ERROR GSUP routing was introduced when adding the E interface. Hence that was the first realm where routing errors could occur. I did notice back then that this message type was special: it does not convey a response to a particular message kind -- it does not make sense, for example, to return an Updating Location Error cause, and do that for all conceivable message types. Instead, this tells the sender that a deeper error exists, i.e. that the desired peer is completely gone and unreachable. I did not foresee though that for D-GSM, there would also be arbitrary GSUP proxy routing, and that this error is not limited to E interface semantics. >From today's point of view, adding the "_E_" in the name was a mistake. Remove that "_E_" to yield OSMO_GSUP_MSGT_ROUTING_ERROR (with unchanged message type discriminator), but provide a #define linking the old name OSMO_GSUP_MSGT_E_ROUTING_ERROR to the new one. The only visible change should be that osmo_gsup_message_type_names[] now returns the new name without "_E_". I am not aware of any regression test fallout from that. Change-Id: Ic8e8bd11522d6c51ac7aaf946516cbce26bc6e1e --- M include/osmocom/gsm/gsup.h M src/gsm/gsup.c 2 files changed, 4 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/61/16161/1 diff --git a/include/osmocom/gsm/gsup.h b/include/osmocom/gsm/gsup.h index 49ddb74..c883dfb 100644 --- a/include/osmocom/gsm/gsup.h +++ b/include/osmocom/gsm/gsup.h @@ -196,9 +196,11 @@ OSMO_GSUP_MSGT_E_CLOSE = 0b01000111, OSMO_GSUP_MSGT_E_ABORT = 0b01001011, - OSMO_GSUP_MSGT_E_ROUTING_ERROR = 0b01001110, + OSMO_GSUP_MSGT_ROUTING_ERROR= 0b01001110, }; +#define OSMO_GSUP_MSGT_E_ROUTING_ERROR OSMO_GSUP_MSGT_ROUTING_ERROR + #define OSMO_GSUP_IS_MSGT_REQUEST(msgt) (((msgt) & 0b0011) == 0b00) #define OSMO_GSUP_IS_MSGT_ERROR(msgt) (((msgt) & 0b0011) == 0b01) #define OSMO_GSUP_TO_MSGT_ERROR(msgt) (((msgt) & 0b1100) | 0b01) diff --git a/src/gsm/gsup.c b/src/gsm/gsup.c index 2f9d85d..ad7a2a4 100644 --- a/src/gsm/gsup.c +++ b/src/gsm/gsup.c @@ -101,7 +101,7 @@ OSMO_VALUE_STRING(OSMO_GSUP_MSGT_E_CLOSE), OSMO_VALUE_STRING(OSMO_GSUP_MSGT_E_ABORT), - OSMO_VALUE_STRING(OSMO_GSUP_MSGT_E_ROUTING_ERROR), + OSMO_VALUE_STRING(OSMO_GSUP_MSGT_ROUTING_ERROR), { 0, NULL } }; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16161 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ic8e8bd11522d6c51ac7aaf946516cbce26bc6e1e Gerrit-Change-Number: 16161 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: add osmo_escape_c_str and osmo_quote_c_str
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16160 ) Change subject: add osmo_escape_c_str and osmo_quote_c_str .. add osmo_escape_c_str and osmo_quote_c_str Provide string escaping that - returns the required buffer size, so it can be used with OSMO_STRBUF_APPEND(). - uses C compatible string constant escaping sequences. This is intended as a replacement for all previous osmo_escape_str* and osmo_quote_str* API. It pains me that I didn't get them right the first nor the second time: - The buffer functions do not return the chars needed, which is required for allocating sufficient memory in the *_c versions of the functions. - Because of that, these functions are accurately usable for OSMO_STRBUF_APPEND(), producing truncated strings, for example when dumping a GSUP message. - They do not use the C equivalent string constant escaping: for some reason I thought "\15" would be valid, but it should be "\x0f". If I could, I would completely drop those mislead implementations ... but backwards compat prohibits that. A previous patch already provided internal static functions that accurately return the required buffer size. Enhance these to also support C compatible string escaping, and use them as implementation of the new functions: osmo_escape_cstr_buf() osmo_escape_cstr_c() osmo_quote_cstr_buf() osmo_quote_cstr_c() In the tests for these, also test C string equivalence. Naming: from API versions, it would be kind of logical to call them osmo_escape_str_buf3() and osmo_escape_str_c2(). Since these anyway return a different escaping, it makes sense to me to have distinct names instead. Quasi missing are variants of the non-C-compatible weird legacy escaping that return the required buffer size, but I refrain from adding those, because we have enough API cruft as it is. Just always use these new cstr variants. Change-Id: I3dfb892036e0133dd8e7e4a6a0c32a3caa9b --- M include/osmocom/core/utils.h M src/utils.c M tests/utils/utils_test.c M tests/utils/utils_test.ok 4 files changed, 301 insertions(+), 21 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/60/16160/1 diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h index 86d45bc..4e7037a 100644 --- a/include/osmocom/core/utils.h +++ b/include/osmocom/core/utils.h @@ -147,6 +147,11 @@ 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); +size_t osmo_escape_cstr_buf(char *buf, size_t bufsize, const char *str, int in_len); +char *osmo_escape_cstr_c(void *ctx, const char *str, int in_len); +size_t osmo_quote_cstr_buf(char *buf, size_t bufsize, const char *str, int in_len); +char *osmo_quote_cstr_c(void *ctx, const char *str, int in_len); + const char *osmo_escape_str(const char *str, int len); char *osmo_escape_str_buf2(char *buf, size_t bufsize, const char *str, int in_len); const char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize); diff --git a/src/utils.c b/src/utils.c index 904f6e4..8dfa7ec 100644 --- a/src/utils.c +++ b/src/utils.c @@ -669,14 +669,18 @@ /*! Return the string with all non-printable characters escaped. * This internal function is the implementation for all osmo_escape_str* and osmo_quote_str* API versions. - * It provides a return value of characters-needed, to allow producing un-truncated strings in all cases. + * It provides both the legacy (non C compatible) escaping, as well as C compatible string constant syntax, + * and it provides a return value of characters-needed, to allow producing un-truncated strings in all cases. * \param[out] buf string buffer to write escaped characters to. * \param[in] bufsize sizeof(buf). * \param[in] str A string that may contain any characters. * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length (also past nul chars). + * \param[in] legacy_format If false, return C compatible string constants ("\x0f"), if true the legacy + * escaping format ("\15"). The legacy format also escapes as "\a\b\f\v", while + * the non-legacy format also escapes those as "\xNN" sequences. * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()). */ -static size_t _osmo_escape_str_buf(char *buf, size_t bufsize, const char *str, int in_len) +static size_t _osmo_escape_str_buf(char *buf, size_t bufsize, const char *str, int in_len, bool legacy_format) { struct osmo_strbuf sb = { .buf = buf, .len = bufsize }; int in_pos = 0; @@ -715,19 +719,28 @@ BACKSLASH_CASE('\r', 'r'); BACKSLASH_CASE('\t', 't');
Change in libosmocore[master]: fix osmo_escape_str_c() and osmo_quote_str_c()
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16159 ) Change subject: fix osmo_escape_str_c() and osmo_quote_str_c() .. fix osmo_escape_str_c() and osmo_quote_str_c() The osmo_escape_str_c() and osmo_quote_str_c() functions return truncated results when characters need escaping. For example: osmo_quote_str_c(NULL, "foo"); --> "foo" osmo_quote_str_c(NULL, "foo\n"); --> "foo\n osmo_quote_str_c(NULL, "foo\tbar\t\n"); --> "foo\tbar\t Implement these _c variants using OSMO_NAME_C_IMPL() to always allocate sufficient memory. However, current osmo_escape_str_buf2() and osmo_quote_str_buf2() fail to return the required buffer size (even though that information is readily avaiable), so these don't qualify for accurate use of OSMO_NAME_C_IMPL(). Hence, move the implementations of osmo_escape_str and osmo_quote_str to an internal static function that returns the characters needed, so that all dynamically allocating implementations can return un-truncated results. Of course, external callers would also benefit from escape/quote API that accurately returns the amount of characters needed, but I am not changing public API in this patch, on purpose, ... yet. Change-Id: I16c08eced41bf1b7acf6e95f658068ace99ca4c8 --- M src/utils.c 1 file changed, 49 insertions(+), 31 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/16159/1 diff --git a/src/utils.c b/src/utils.c index 6fc2ee6..904f6e4 100644 --- a/src/utils.c +++ b/src/utils.c @@ -668,13 +668,15 @@ } /*! Return the string with all non-printable characters escaped. + * This internal function is the implementation for all osmo_escape_str* and osmo_quote_str* API versions. + * It provides a return value of characters-needed, to allow producing un-truncated strings in all cases. * \param[out] buf string buffer to write escaped characters to. * \param[in] bufsize sizeof(buf). * \param[in] str A string that may contain any characters. * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length (also past nul chars). - * \return The output buffer (buf). + * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()). */ -char *osmo_escape_str_buf2(char *buf, size_t bufsize, const char *str, int in_len) +static size_t _osmo_escape_str_buf(char *buf, size_t bufsize, const char *str, int in_len) { struct osmo_strbuf sb = { .buf = buf, .len = bufsize }; int in_pos = 0; @@ -729,6 +731,19 @@ } done: + return sb.chars_needed; +} + +/*! Return the string with all non-printable characters escaped. + * \param[out] buf string buffer to write escaped characters to. + * \param[in] bufsize sizeof(buf). + * \param[in] str A string that may contain any characters. + * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length (also past nul chars). + * \return The output buffer (buf). + */ +char *osmo_escape_str_buf2(char *buf, size_t bufsize, const char *str, int in_len) +{ + _osmo_escape_str_buf(buf, bufsize, str, in_len); return buf; } @@ -750,10 +765,31 @@ */ char *osmo_escape_str_c(const void *ctx, const char *str, int in_len) { - char *buf = talloc_size(ctx, in_len+1); - if (!buf) - return NULL; - return osmo_escape_str_buf2(buf, in_len+1, str, in_len); + /* The string will be at least as long as in_len, but some characters might need escaping. +* These extra bytes should catch most usual escaping situations, avoiding a second run in OSMO_NAME_C_IMPL. */ + OSMO_NAME_C_IMPL(ctx, in_len + 16, "ERROR", _osmo_escape_str_buf, str, in_len); +} + +/*! Return a quoted and escaped representation of the string. + * This internal function is the implementation for all osmo_quote_str* API versions. + * It provides a return value of characters-needed, to allow producing un-truncated strings in all cases. + * \param[out] buf string buffer to write escaped characters to. + * \param[in] bufsize sizeof(buf). + * \param[in] str A string that may contain any characters. + * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length (also past nul chars). + * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()). + */ +static size_t _osmo_quote_str_buf(char *buf, size_t bufsize, const char *str, int in_len) +{ + struct osmo_strbuf sb = { .buf = buf, .len = bufsize }; + if (!str) + OSMO_STRBUF_PRINTF(sb, "NULL"); + else { + OSMO_STRBUF_PRINTF(sb, "\""); + OSMO_STRBUF_APPEND(sb, _osmo_escape_str_buf, str, in_len); + OSMO_STRBUF_PRINTF(sb, "\""); +
Change in libosmocore[master]: utils.c: fix various inaccurate API doc about return values
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16155 ) Change subject: utils.c: fix various inaccurate API doc about return values .. utils.c: fix various inaccurate API doc about return values Change-Id: I9ee6416decd23f8d5d634197620a63ae408cead3 --- M src/utils.c 1 file changed, 3 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/55/16155/1 diff --git a/src/utils.c b/src/utils.c index ea1de0f..6fc2ee6 100644 --- a/src/utils.c +++ b/src/utils.c @@ -280,7 +280,7 @@ * \param[out] buf_len size of buf in bytes * \param[in] bits A sequence of unpacked bits * \param[in] len Length of bits - * \returns string representation in static buffer. + * \return The output buffer (buf). */ char *osmo_ubit_dump_buf(char *buf, size_t buf_len, const uint8_t *bits, unsigned int len) { @@ -672,7 +672,7 @@ * \param[in] bufsize sizeof(buf). * \param[in] str A string that may contain any characters. * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length (also past nul chars). - * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()). + * \return The output buffer (buf). */ char *osmo_escape_str_buf2(char *buf, size_t bufsize, const char *str, int in_len) { @@ -763,7 +763,7 @@ * \param[in] bufsize sizeof(buf). * \param[in] str A string that may contain any characters. * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length. - * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()). + * \return The output buffer (buf). */ char *osmo_quote_str_buf2(char *buf, size_t bufsize, const char *str, int in_len) { @@ -808,7 +808,6 @@ } /*! Like osmo_quote_str_buf() but returns the result in a dynamically-allocated buffer. - * The static buffer is shared with get_value_string() and osmo_escape_str(). * \param[in] str A string that may contain any characters. * \param[in] in_len Pass -1 to print until nul char, or >= 0 to force a length. * \returns dynamically-allocated buffer containing a quoted and escaped representation. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16155 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9ee6416decd23f8d5d634197620a63ae408cead3 Gerrit-Change-Number: 16155 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: cosmetic: logging.h: fix comment s/levels/subsystems
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16158 ) Change subject: cosmetic: logging.h: fix comment s/levels/subsystems .. cosmetic: logging.h: fix comment s/levels/subsystems Change-Id: I242a4a44649bc4dac055985ba8fd63b2f784ee6d --- M include/osmocom/core/logging.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/58/16158/1 diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h index 0846c1b..aab4c56 100644 --- a/include/osmocom/core/logging.h +++ b/include/osmocom/core/logging.h @@ -105,7 +105,7 @@ #define LOGL_ERROR 7 /*!< error condition, requires user action */ #define LOGL_FATAL 8 /*!< fatal, program aborted */ -/* logging levels defined by the library itself */ +/* logging subsystems defined by the library itself */ #define DLGLOBAL -1 /*!< global logging */ #define DLLAPD -2 /*!< LAPD implementation */ #define DLINP -3 /*!< (A-bis) Input sub-system */ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16158 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I242a4a44649bc4dac055985ba8fd63b2f784ee6d Gerrit-Change-Number: 16158 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
Hello laforge, osmith, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/15957 to look at the new patch set (#8). Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. utils.h: add OSMO_NAME_C_IMPL() macro Provide a common implementation for foo_name_c() functions that base on foo_name_buf() functions. char *foo_name_c(void *ctx, example_t arg) { OSMO_NAME_C_IMPL(ctx, 64, "ERROR", foo_name_buf, arg) } Rationale: the most efficient way of composing strings that have optional parts or require loops for composition is by writing to a ready char[], and this in turn is easiest done by using OSMO_STRBUF_* API. Using such a basic name string implementation which typically returns a length, I often want a more convenient version that returns a char*, which can just be inlined in a "%s" string format -- crucially: skipping string composition when inlined in a LOGP(). This common implementation allows saving code dup, only the function signature is needed. Why not include the function signature in the macro? The two sets of varargs (1: signature args, 2: function call args) are hard to do. Also, having an explicit signature is good for readability and code grepping / ctags. Upcoming uses: in libosmocore in the mslookup (D-GSM) implementation (osmo_mslookup_result_name_c()), and in osmo_msc's codec negotiation implementation (sdp_audio_codecs_name_c(), sdp_msg_name_c(), ...). I54b6c0810f181259da307078977d9ef3d90458c9 (libosmocore) If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 (osmo-msc) Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 --- M include/osmocom/core/utils.h M tests/utils/utils_test.c M tests/utils/utils_test.ok 3 files changed, 154 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/57/15957/8 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 8 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in libosmocore[master]: add osmo_sockaddr_str_cmp()
Hello laforge, osmith, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/15961 to look at the new patch set (#7). Change subject: add osmo_sockaddr_str_cmp() .. add osmo_sockaddr_str_cmp() Currently planned user: for Distributed GSM in osmo-hlr: setting per-MSC service addresses in VTY: replace/remove existing entries. osmo_sockaddr_str_cmp() is useful to catch identical resulting IP addresses, regardless of differing strings (e.g. '0::' and '::' are equal but differ in strings). Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501 --- M include/osmocom/core/sockaddr_str.h M include/osmocom/core/utils.h M src/sockaddr_str.c M tests/sockaddr_str/sockaddr_str_test.c M tests/sockaddr_str/sockaddr_str_test.ok 5 files changed, 678 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/61/15961/7 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15961 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501 Gerrit-Change-Number: 15961 Gerrit-PatchSet: 7 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in libosmocore[master]: logging.h: define ansi color constants
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16156 ) Change subject: logging.h: define ansi color constants .. logging.h: define ansi color constants It's hard to figure out what color logging categories have with those ANSI color code strings. Instead, define these OSMO_LOGCOLOR_* constants. Naming: commonly, the logging.h header has the "LOG" prefix in the name, but it seems saner to include the OSMO_ prefix: it seems too likely that some libosmocore user somewhere already has defined "LOGCOLOR_RED" somewhere. Change-Id: I03b6b1f73ae7ee61d37ff921e071a3d0881d3e9a --- M include/osmocom/core/logging.h 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/56/16156/1 diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h index 139d291..0846c1b 100644 --- a/include/osmocom/core/logging.h +++ b/include/osmocom/core/logging.h @@ -127,6 +127,24 @@ #define DLRSPRO-19 /*!< Osmocom Remote SIM Protocol */ #define OSMO_NUM_DLIB 19 /*!< Number of logging sub-systems in libraries */ +/* Colors that can be used in log_info_cat.color */ +#define OSMO_LOGCOLOR_NORMAL NULL +#define OSMO_LOGCOLOR_RED "\033[1;31m" +#define OSMO_LOGCOLOR_GREEN "\033[1;32m" +#define OSMO_LOGCOLOR_YELLOW "\033[1;33m" +#define OSMO_LOGCOLOR_BLUE "\033[1;34m" +#define OSMO_LOGCOLOR_PURPLE "\033[1;35m" +#define OSMO_LOGCOLOR_CYAN "\033[1;36m" +#define OSMO_LOGCOLOR_DARKRED "\033[31m" +#define OSMO_LOGCOLOR_DARKGREEN "\033[32m" +#define OSMO_LOGCOLOR_DARKYELLOW "\033[33m" +#define OSMO_LOGCOLOR_DARKBLUE "\033[34m" +#define OSMO_LOGCOLOR_DARKPURPLE "\033[35m" +#define OSMO_LOGCOLOR_DARKCYAN "\033[36m" +#define OSMO_LOGCOLOR_DARKGREY "\033[1;30m" +#define OSMO_LOGCOLOR_GREY "\033[37m" +#define OSMO_LOGCOLOR_BRIGHTWHITE "\033[1;37m" + /*! Configuration of single log category / sub-system */ struct log_category { uint8_t loglevel; /*!< configured log-level */ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16156 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I03b6b1f73ae7ee61d37ff921e071a3d0881d3e9a Gerrit-Change-Number: 16156 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: fix DLSMS logging category color: '[1:38m' isn't actually defined
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16157 ) Change subject: fix DLSMS logging category color: '[1:38m' isn't actually defined .. fix DLSMS logging category color: '[1:38m' isn't actually defined Instead it apparently renders as bright white, so just use that constant instead. Change-Id: Ic775b6e37ccf61dc71a540b41d6a16a8a9291dc2 --- M src/logging.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/57/16157/1 diff --git a/src/logging.c b/src/logging.c index b030f8a..1254869 100644 --- a/src/logging.c +++ b/src/logging.c @@ -163,7 +163,7 @@ .name = "DLSMS", .description = "Layer3 Short Message Service (SMS)", .enabled = 1, .loglevel = LOGL_NOTICE, - .color = "\033[1;38m", + .color = OSMO_LOGCOLOR_BRIGHTWHITE, }, [INT2IDX(DLCTRL)] = { .name = "DLCTRL", -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16157 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ic775b6e37ccf61dc71a540b41d6a16a8a9291dc2 Gerrit-Change-Number: 16157 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: msgb_put: more elaborate logging of head/tailroom failure
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16164 ) Change subject: msgb_put: more elaborate logging of head/tailroom failure .. msgb_put: more elaborate logging of head/tailroom failure Change-Id: I55b68098e1037c74ebe5faa86e34bd4494f5b726 --- M include/osmocom/core/msgb.h 1 file changed, 12 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/64/16164/1 diff --git a/include/osmocom/core/msgb.h b/include/osmocom/core/msgb.h index 1833a6c..cc76e3a 100644 --- a/include/osmocom/core/msgb.h +++ b/include/osmocom/core/msgb.h @@ -239,7 +239,11 @@ { unsigned char *tmp = msgb->tail; if (msgb_tailroom(msgb) < (int) len) - MSGB_ABORT(msgb, "Not enough tailroom msgb_put (%u < %u)\n", + MSGB_ABORT(msgb, "Not enough tailroom msgb_put" + " (allocated %u, head at %u, len %u, tailroom %u < want tailroom %u)\n", + msgb->data_len - sizeof(struct msgb), + msgb->head - msgb->_data, + msgb->len, msgb_tailroom(msgb), len); msgb->tail += len; msgb->len += len; @@ -335,8 +339,13 @@ static inline unsigned char *msgb_push(struct msgb *msgb, unsigned int len) { if (msgb_headroom(msgb) < (int) len) - MSGB_ABORT(msgb, "Not enough headroom msgb_push (%u < %u)\n", - msgb_headroom(msgb), len); + MSGB_ABORT(msgb, "Not enough headroom msgb_push" + " (allocated %u, head at %u < want headroom %u, len %u, tailroom %u)\n", + msgb->data_len - sizeof(struct msgb), + msgb->head - msgb->_data, + len, + msgb->len, + msgb_tailroom(msgb)); msgb->data -= len; msgb->len += len; return msgb->data; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16164 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I55b68098e1037c74ebe5faa86e34bd4494f5b726 Gerrit-Change-Number: 16164 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: logging.h: define ansi color constants
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/16156 to look at the new patch set (#2). Change subject: logging.h: define ansi color constants .. logging.h: define ansi color constants It's hard to figure out what color logging categories have with those ANSI color code strings. Instead, define these OSMO_LOGCOLOR_* constants. Naming: commonly, the logging.h header has the "LOG" prefix in the name, but it seems saner to include the OSMO_ prefix: it seems too likely that some libosmocore user somewhere already has defined "LOGCOLOR_RED" somewhere. Change-Id: I03b6b1f73ae7ee61d37ff921e071a3d0881d3e9a --- M include/osmocom/core/logging.h M src/logging.c M tests/logging/logging_test.c M tests/loggingrb/loggingrb_test.c 4 files changed, 33 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/56/16156/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16156 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I03b6b1f73ae7ee61d37ff921e071a3d0881d3e9a Gerrit-Change-Number: 16156 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset
Change in libosmocore[master]: utils_test: add osmo_print_n_test()
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16165 ) Change subject: utils_test: add osmo_print_n_test() .. utils_test: add osmo_print_n_test() A couple of times recently I've needed to copy out a substring to a buffer with limited size. Use of strncpy() or osmo_strlcpy() are nontrivial here. I wanted to have a dedicated function. After I wrote that function with a test, I noticed that I had already implemented the same thing a while ago, as osmo_print_n() :P So here is just the test. Change-Id: Ia716abdc1f58af6065b84f4f567388a32a7b39fc --- M src/utils.c M tests/utils/utils_test.c M tests/utils/utils_test.ok 3 files changed, 82 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/65/16165/1 diff --git a/src/utils.c b/src/utils.c index 8dfa7ec..4e55a89 100644 --- a/src/utils.c +++ b/src/utils.c @@ -509,6 +509,8 @@ * Copy at most \a siz bytes from \a src to \a dst, ensuring that the result is * NUL terminated. The NUL character is included in \a siz, i.e. passing the * actual sizeof(*dst) is correct. + * + * Note, a similar function that also limits the input buffer size is osmo_print_n(). */ size_t osmo_strlcpy(char *dst, const char *src, size_t siz) { diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c index 3c6d17b..458458e 100644 --- a/tests/utils/utils_test.c +++ b/tests/utils/utils_test.c @@ -1264,6 +1264,59 @@ talloc_free(ctx); } +static void osmo_print_n_test(void) +{ + struct token_test { + const char *src; + size_t token_len; + size_t buf_size; + const char *expect_token; + int expect_rc; + }; + struct token_test tests[] = { + { "foo=bar", 3, 100, "foo", 3 }, + { "foo", 10, 100, "foo", 3 }, + { "foo", 3, 100, "foo", 3 }, + { NULL, 10, 100, "", 0 }, + { "", 10, 100, "", 0 }, + { "foo=bar", 0, 100, "", 0 }, + + { "foo=bar", 3, 2, "f", 3 }, + { "foo", 10, 2, "f", 3 }, + { "foo", 3, 2, "f", 3 }, + { NULL, 10, 2, "", 0 }, + { "", 10, 2, "", 0 }, + { "foo=bar", 0, 2, "", 0 }, + + { "foo=bar", 3, 1, "", 3 }, + { "foo", 10, 1, "", 3 }, + { "foo", 3, 1, "", 3 }, + { NULL, 10, 1, "", 0 }, + { "", 10, 1, "", 0 }, + { "foo=bar", 0, 1, "", 0 }, + + { "foo=bar", 3, 0, "unchanged", 3 }, + { "foo", 10, 0, "unchanged", 3 }, + { "foo", 3, 0, "unchanged", 3 }, + { NULL, 10, 0, "unchanged", 0 }, + { "", 10, 0, "unchanged", 0 }, + { "foo=bar", 0, 0, "unchanged", 0 }, + }; + struct token_test *t; + printf("\n%s()\n", __func__); + for (t = tests; t - tests < ARRAY_SIZE(tests); t++) { + char buf[100] = "unchanged"; + int rc = osmo_print_n(buf, t->buf_size, t->src, t->token_len); + printf("%s token_len=%zu buf_size=%zu", osmo_quote_str(t->src, -1), t->token_len, t->buf_size); + printf(" -> token=%s rc=%d", osmo_quote_str(buf, -1), rc); + if (strcmp(buf, t->expect_token)) + printf(" ERROR: expected token %s", osmo_quote_str(t->expect_token, -1)); + if (rc != t->expect_rc) + printf(" ERROR: expected rc %d", t->expect_rc); + printf("\n"); + } +} + int main(int argc, char **argv) { static const struct log_info log_info = {}; @@ -1287,5 +1340,6 @@ strbuf_test_nolen(); startswith_test(); name_c_impl_test(); + osmo_print_n_test(); return 0; } diff --git a/tests/utils/utils_test.ok b/tests/utils/utils_test.ok index 10436ce..bd43365 100644 --- a/tests/utils/utils_test.ok +++ b/tests/utils/utils_test.ok @@ -449,3 +449,29 @@ OSMO_NAME_C_IMPL(10, NULL) -> NULL allocated 0 OSMO_NAME_C_IMPL(0, "ERROR") -> "ERROR" allocated 1 6 bytes, name 'foo_name_c_zero' OSMO_NAME_C_IMPL(0, NULL) -> NULL allocated 0 + +osmo_print_n_test() +"foo=bar" token_len=3 buf_size=100 -> token="foo" rc=3 +"foo"
Change in libosmocore[master]: vty: track parent nodes also for telnet sessions
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16162 ) Change subject: vty: track parent nodes also for telnet sessions .. vty: track parent nodes also for telnet sessions Keep track of parent nodes and go back hierarchically, not only for .cfg file reading, but also for telnet VTY sessions. A long time ago cfg file parsing was made strictly hierarchical: node exits go back to parent nodes exactly as they were entered. However, live telnet VTY sessions still lacked this and depended on the go_parent_cb(). >From this commit on, implementing a go_parent_cb() is completely optional. The go_parent_cb() no longer has the task to determine the correct parent node, neither for cfg files (as already the case before this patch) nor for telnet VTY sessions (added by this patch). Instead, a go_parent_cb() implementation can merely take actions it requires on node exits, for example applying some config when leaving a specific node. The node value that is returned by the go_parent_cb() and the vty->node and vty->index values that might be set are completely ignored; instead the implicit parent node tracking determines the parent and node object. As a side effect, the is_config_node() callback is no longer needed, since the VTY now always implicitly knows when to exit back to the CONFIG_NODE. For example, osmo_ss7_is_config_node() could now be dropped, and the osmo_ss7_vty_go_parent() could be shortened by five switch cases, does no longer need to set vty->node nor vty->index and could thus be shortened to: int osmo_ss7_vty_go_parent(struct vty *vty) { struct osmo_ss7_asp *asp; struct osmo_xua_server *oxs; switch (vty->node) { case L_CS7_ASP_NODE: asp = vty->index; /* If no local addr was set */ if (!asp->cfg.local.host_cnt) { asp->cfg.local.host[0] = NULL; asp->cfg.local.host_cnt = 1; } osmo_ss7_asp_restart(asp); break; case L_CS7_XUA_NODE: oxs = vty->index; /* If no local addr was set, or erased after _create(): */ if (!oxs->cfg.local.host_cnt) osmo_ss7_xua_server_set_local_host(oxs, NULL); if (osmo_ss7_xua_server_bind(oxs) < 0) vty_out(vty, "%% Unable to bind xUA server to IP(s)%s", VTY_NEWLINE); break; } return 0; } Before parent tracking, every program was required to write a go_parent_cb() which has to return every node's parent node, basically a switch() statement that manually traces the way back out of child nodes. If the go_parent_cb() has errors, we may wildly jump around the node tree: a common error is to jump right out to the top config node with one exit, even though we were N levels deep. This kind of error has been eliminated for cfg files long ago, but still exists for telnet VTY sessions, which this patch fixes. This came up when I was adding multi-level config nodes to osmo-hlr to support Distributed GSM / remote MS lookup: the config file worked fine, while vty node tests failed to exit to the correct nodes. Change-Id: I2b32b4fe20732728db6e9cdac7e484d96ab86dc5 --- M include/osmocom/vty/vty.h M src/vty/command.c M tests/vty/vty_test.c 3 files changed, 31 insertions(+), 48 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/62/16162/1 diff --git a/include/osmocom/vty/vty.h b/include/osmocom/vty/vty.h index 03a2924..9acaa7d 100644 --- a/include/osmocom/vty/vty.h +++ b/include/osmocom/vty/vty.h @@ -178,9 +178,14 @@ const char *copyright; /*! \ref talloc context */ void *tall_ctx; - /*! call-back for returning to parent n ode */ + /*! Call-back for taking actions upon exiting a node. +* The return value is ignored, and changes to vty->node and vty->index made in this callback are ignored. +* Implicit parent node tracking always sets the correct parent node and vty->index after this callback exits, +* so this callback can handle only those nodes that should take specific actions upon node exit, or can be left +* NULL entirely. */ int (*go_parent_cb)(struct vty *vty); - /*! call-back to determine if node is config node */ + /*! OBSOLETED: Implicit parent node tracking has replaced the use of this callback. This callback is no longer +* called, ever, and can be left NULL. */ int (*is_config_node)(struct vty *vty, int node); /*! Check if the config is consistent before write */ int (*config_is_consistent)(struct vty *vty); diff --git a/src/vty/command.c b/src/vty/command.c index 6a9d18a..daee5c5 100644 --- a/src/vty/command.c +++ b/src/vty/command.c
Change in libosmocore[master]: fsm.h: add missing include of logging.h
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16163 ) Change subject: fsm.h: add missing include of logging.h .. fsm.h: add missing include of logging.h Change-Id: I783bf0eb40b674fb6a77f7673563fdf156975f5a --- M include/osmocom/core/fsm.h 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/63/16163/1 diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h index 269befa..7b262c7 100644 --- a/include/osmocom/core/fsm.h +++ b/include/osmocom/core/fsm.h @@ -10,6 +10,7 @@ #include #include #include +#include /*! \defgroup fsm Finite State Machine abstraction * @{ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16163 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I783bf0eb40b674fb6a77f7673563fdf156975f5a Gerrit-Change-Number: 16163 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: utils: add osmo_strnchr()
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16166 ) Change subject: utils: add osmo_strnchr() .. utils: add osmo_strnchr() When finding a char in a string, I want to be able to limit the search area by size, not only by nul terminator. Change-Id: I48f8ace9f51f8a06796648883afcabe3b4e8b537 --- M include/osmocom/core/utils.h M src/utils.c M tests/utils/utils_test.c M tests/utils/utils_test.ok 4 files changed, 65 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/66/16166/1 diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h index 4e7037a..01c4de6 100644 --- a/include/osmocom/core/utils.h +++ b/include/osmocom/core/utils.h @@ -139,6 +139,7 @@ uint8_t *osmo_encode_big_endian(uint64_t value, size_t data_len); size_t osmo_strlcpy(char *dst, const char *src, size_t siz); +const char *osmo_strnchr(const char *str, size_t str_size, char c); bool osmo_is_hexstr(const char *str, int min_digits, int max_digits, bool require_even); diff --git a/src/utils.c b/src/utils.c index 4e55a89..038288c 100644 --- a/src/utils.c +++ b/src/utils.c @@ -525,6 +525,28 @@ return ret; } +/*! Find first occurence of a char in a size limited string. + * Like strchr() but with a buffer size limit. + * \param[in] str String buffer to examine. + * \param[in] str_size sizeof(str). + * \param[in] c Character to look for. + * \return Pointer to the matched char, or NULL if not found. + */ +const char *osmo_strnchr(const char *str, size_t str_size, char c) +{ + const char *end = str + str_size; + const char *pos; + if (!str) + return NULL; + for (pos = str; pos < end; pos++) { + if (c == *pos) + return pos; + if (!*pos) + return NULL; + } + return NULL; +} + /*! Validate that a given string is a hex string within given size limits. * Note that each hex digit amounts to a nibble, so if checking for a hex * string to result in N bytes, pass amount of digits as 2*N. diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c index 458458e..e87cb22 100644 --- a/tests/utils/utils_test.c +++ b/tests/utils/utils_test.c @@ -1317,6 +1317,39 @@ } } +static void osmo_strnchr_test(void) +{ + struct test { + const char *haystack; + size_t haystack_len; + const char *needle; + int expect_offset; + }; + struct test tests[] = { + { "foo=bar", 8, "=", 3 }, + { "foo=bar", 4, "=", 3 }, + { "foo=bar", 3, "=", -1 }, + { "foo=bar", 0, "=", -1 }, + { "foo\0=bar", 9, "=", -1 }, + { "foo\0=bar", 9, "\0", 3 }, + }; + struct test *t; + printf("\n%s()\n", __func__); + for (t = tests; t - tests < ARRAY_SIZE(tests); t++) { + const char *r = osmo_strnchr(t->haystack, t->haystack_len, t->needle[0]); + int offset = -1; + if (r) + offset = r - t->haystack; + printf("osmo_strnchr(%s, %zu, ", + osmo_quote_str(t->haystack, -1), t->haystack_len); + printf("'%s') -> %d", + osmo_escape_str(t->needle, 1), offset); + if (offset != t->expect_offset) + printf(" ERROR expected %d", t->expect_offset); + printf("\n"); + } +} + int main(int argc, char **argv) { static const struct log_info log_info = {}; @@ -1341,5 +1374,6 @@ startswith_test(); name_c_impl_test(); osmo_print_n_test(); + osmo_strnchr_test(); return 0; } diff --git a/tests/utils/utils_test.ok b/tests/utils/utils_test.ok index bd43365..baa708e 100644 --- a/tests/utils/utils_test.ok +++ b/tests/utils/utils_test.ok @@ -475,3 +475,11 @@ NULL token_len=10 buf_size=0 -> token="unchanged" rc=0 "" token_len=10 buf_size=0 -> token="unchanged" rc=0 "foo=bar" token_len=0 buf_size=0 -> token="unchanged" rc=0 + +osmo_strnchr_test() +osmo_strnchr("foo=bar", 8, '=') -> 3 +osmo_strnchr("foo=bar", 4, '=') -> 3 +osmo_strnchr("foo=bar", 3, '=') -> -1 +osmo_strnchr("foo=bar", 0, '=') -> -1 +osmo_strnchr("foo", 9, '=') -> -1 +osmo_strnchr("foo", 9, '\0') -> 3 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16166 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48f8ace9f51f8a06796648883afcabe3b4e8b537 Gerrit-Change-Number: 16166 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-MessageType: newchange
Change in libosmocore[master]: utils.h: add OSMO_NAME_C_IMPL() macro
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15957 ) Change subject: utils.h: add OSMO_NAME_C_IMPL() macro .. Patch Set 8: Code-Review+2 re-applying previous +2, just a small simplification applied in the latest patch set -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 Gerrit-Change-Number: 15957 Gerrit-PatchSet: 8 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-CC: pespin Gerrit-Comment-Date: Thu, 21 Nov 2019 20:20:19 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in libosmocore[master]: osmo_sockaddr_str: deprecate osmo_sockaddr_str_*_32n()
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/16167 ) Change subject: osmo_sockaddr_str: deprecate osmo_sockaddr_str_*_32n() .. osmo_sockaddr_str: deprecate osmo_sockaddr_str_*_32n() Follow up for patch I3cf150cc0cc06dd36039fbde091bc71b01697322 osmo_sockaddr_str_{from,to}_32n actually use host byte order. Deprecate these and introduce a more accurately named version ending in h. Change-Id: Ic7fc279bf3c741811cfc002538e28e8f8560e338 --- M TODO-RELEASE M include/osmocom/core/sockaddr_str.h M src/sockaddr_str.c M tests/sockaddr_str/sockaddr_str_test.c M tests/sockaddr_str/sockaddr_str_test.ok 5 files changed, 60 insertions(+), 36 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/67/16167/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index 692bdc1..be858ae 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -11,3 +11,7 @@ core struct osmo_tdeffields min_val,max_val added, ABI break (arrays of structs used in programs) gsmAPI added osmo_gsm48_rfpowercap2powerclass() gb API added bssgp_bvc_ctx_free() +core osmo_sockaddr_str_from_32n(), + osmo_sockaddr_str_to_32n() Deprecate: named 'n' but use host byte order. +core osmo_sockaddr_str_from_32h(), + osmo_sockaddr_str_to_32h() New, use host byte order and are named appropriately. diff --git a/include/osmocom/core/sockaddr_str.h b/include/osmocom/core/sockaddr_str.h index d7a8cdf..e42216a 100644 --- a/include/osmocom/core/sockaddr_str.h +++ b/include/osmocom/core/sockaddr_str.h @@ -31,6 +31,7 @@ #include #include #include +#include struct in_addr; struct in6_addr; @@ -77,7 +78,7 @@ int osmo_sockaddr_str_from_in_addr(struct osmo_sockaddr_str *sockaddr_str, const struct in_addr *addr, uint16_t port); int osmo_sockaddr_str_from_in6_addr(struct osmo_sockaddr_str *sockaddr_str, const struct in6_addr *addr, uint16_t port); int osmo_sockaddr_str_from_32(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port); -int osmo_sockaddr_str_from_32n(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port); +int osmo_sockaddr_str_from_32h(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port); int osmo_sockaddr_str_from_sockaddr_in(struct osmo_sockaddr_str *sockaddr_str, const struct sockaddr_in *src); int osmo_sockaddr_str_from_sockaddr_in6(struct osmo_sockaddr_str *sockaddr_str, const struct sockaddr_in6 *src); int osmo_sockaddr_str_from_sockaddr(struct osmo_sockaddr_str *sockaddr_str, const struct sockaddr_storage *src); @@ -85,9 +86,14 @@ int osmo_sockaddr_str_to_in_addr(const struct osmo_sockaddr_str *sockaddr_str, struct in_addr *dst); int osmo_sockaddr_str_to_in6_addr(const struct osmo_sockaddr_str *sockaddr_str, struct in6_addr *dst); int osmo_sockaddr_str_to_32(const struct osmo_sockaddr_str *sockaddr_str, uint32_t *ip); -int osmo_sockaddr_str_to_32n(const struct osmo_sockaddr_str *sockaddr_str, uint32_t *ip); +int osmo_sockaddr_str_to_32h(const struct osmo_sockaddr_str *sockaddr_str, uint32_t *ip); int osmo_sockaddr_str_to_sockaddr_in(const struct osmo_sockaddr_str *sockaddr_str, struct sockaddr_in *dst); int osmo_sockaddr_str_to_sockaddr_in6(const struct osmo_sockaddr_str *sockaddr_str, struct sockaddr_in6 *dst); int osmo_sockaddr_str_to_sockaddr(const struct osmo_sockaddr_str *sockaddr_str, struct sockaddr_storage *dst); +int osmo_sockaddr_str_from_32n(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port) + OSMO_DEPRECATED("osmo_sockaddr_str_from_32n() actually uses *host* byte order. Use osmo_sockaddr_str_from_32h() instead"); +int osmo_sockaddr_str_to_32n(const struct osmo_sockaddr_str *sockaddr_str, uint32_t *ip) + OSMO_DEPRECATED("osmo_sockaddr_str_to_32n() actually uses *host* byte order. Use osmo_sockaddr_str_to_32h() instead"); + /*! @} */ diff --git a/src/sockaddr_str.c b/src/sockaddr_str.c index 304363d..c38a05c 100644 --- a/src/sockaddr_str.c +++ b/src/sockaddr_str.c @@ -277,13 +277,20 @@ * \param[in] port Port number. * \return 0 on success, negative on error. */ -int osmo_sockaddr_str_from_32n(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port) +int osmo_sockaddr_str_from_32h(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port) { if (!sockaddr_str) return -ENOSPC; return osmo_sockaddr_str_from_32(sockaddr_str, osmo_ntohl(ip), port); } +/*! DEPRECATED: the name suggests a conversion from network byte order, but actually converts from host byte order. Use + * osmo_sockaddr_str_from_32 for network byte order and osmo_sockaddr_str_from_32h for host byte order. */ +int osmo_sockaddr_str_from_32n(struct osmo_sockaddr_str *sockaddr_str, uint32_t ip, uint16_t port) +{ + return osmo_s
Change in libosmocore[master]: osmo_sockaddr_str: API doc: fix 32bit addr mixup of host/network byte...
Hello pespin, osmith, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/16030 to look at the new patch set (#5). Change subject: osmo_sockaddr_str: API doc: fix 32bit addr mixup of host/network byte order .. osmo_sockaddr_str: API doc: fix 32bit addr mixup of host/network byte order Of course both v4 and v6 addresses are kept in network byte order when represented in bytes, but when writing, I somehow must have assumed that inet_pton() returns host byte order. Fix that mixup in the API docs: osmo_sockaddr_str_from_32() and osmo_sockaddr_str_to_32() actually use network byte order. osmo_sockaddr_str_from_32n() and osmo_sockaddr_str_to_32n() actually use host byte order, though reflecting 'n' in their name. sockaddr_str_test: use hexdump instead of %x to show the osmo_sockaddr_str_to_32*() conversions so that the error becomes obvious. (Printing %x reverses the bytes again and made it look correct.) Change-Id: I3cf150cc0cc06dd36039fbde091bc71b01697322 --- M src/sockaddr_str.c M tests/sockaddr_str/sockaddr_str_test.c M tests/sockaddr_str/sockaddr_str_test.ok 3 files changed, 58 insertions(+), 54 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/30/16030/5 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16030 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cf150cc0cc06dd36039fbde091bc71b01697322 Gerrit-Change-Number: 16030 Gerrit-PatchSet: 5 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-MessageType: newpatchset