- gerrit-log - lists.osmocom.org

[L] Change in libosmo-abis[master]: ipaccess: Convert BTS OML & RSL link to use stream_cli
by pespin 28 Nov '24

28 Nov '24

Attention is currently required from: daniel, fixeria, laforge. Hello Jenkins Builder, daniel, fixeria, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-abis/+/38927?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by fixeria, Code-Review+1 by laforge, Verified+1 by Jenkins Builder Change subject: ipaccess: Convert BTS OML & RSL link to use stream_cli ...................................................................... ipaccess: Convert BTS OML & RSL link to use stream_cli This in turn allows running BTS Abis interfaces through io-uring backend, which should provide performance improvements when used. Related: SYS#7063 Related: OS#5756 Depends: libosmo-netif.git Change-Id I952938474fa2780bf3c906cbdffb2d024b03c1b7 Depends: libosmocore.git Change-Id 8bcfe62521a977a05b2498efe906d6db6e2be4e8 Change-Id: I35c214fbe930c695a1475d8b4bc3dc44dff83eea --- M TODO-RELEASE M libosmoabis.pc.in M src/Makefile.am M src/input/ipaccess.c 4 files changed, 354 insertions(+), 148 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/27/38927/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38927?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I35c214fbe930c695a1475d8b4bc3dc44dff83eea Gerrit-Change-Number: 38927 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>

1 0

[L] Change in libosmo-abis[master]: ipaccess: Convert BTS OML & RSL link to use stream_cli
by pespin 28 Nov '24

28 Nov '24

Attention is currently required from: daniel, fixeria. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38927?usp=email ) Change subject: ipaccess: Convert BTS OML & RSL link to use stream_cli ...................................................................... Patch Set 1: (6 comments) File src/input/ipaccess.c: https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/bb9f384d_156b5e3a… : PS1, Line 494: OSMO_ASSERT(cli); > Why are you so assert()ive? ;) […] If you dive into the code you'd like to put even more asserts! After work on the BSC side I think I indeed can remove this one, will do. https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/bc454569_45f14ee9… : PS1, Line 545: osmo_stream_cli_send > Why not returning `rc` of this function? because osmo_stream_cli_send() returns void. YES, I KNOW, BIG FAIL!!!!! Remember this case next time you see a new API added returning void 😊 I'm also thinking about creating a osmo_stream_cli_send2() which returns int, and deprectate the other one. That can be done later on though. For osmo_fd backend that was not much of a problem (because we were not checking for a max queue size at the time), but for osmo_io backend it can fail when reaching max length of its internal queue... https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/184dcdbb_cf82fb52… : PS1, Line 928: *msgb_put(nmsg2, 1) = IPAC_MSGT_ID_ACK; > `msgb_v_put(nmsg2, IPAC_MSGT_ID_ACK)` Done https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/08dd7de2_d0209970… : PS1, Line 944: uint8_t *data = msgb_l2(msg); > `const`? Done https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/e67b92cd_7ef2b5a0… : PS1, Line 992: err: : return -1; > `goto`s can be replaced with `return -1`? Yes, but I prefer having all error paths easily visible this way. https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/b122a1da_85c99e60… : PS1, Line 1014: e1i_ts->line > `s/e1i_ts->line/line/` Done -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38927?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I35c214fbe930c695a1475d8b4bc3dc44dff83eea Gerrit-Change-Number: 38927 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 28 Nov 2024 15:59:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in libosmo-netif[master]: stream_cli: Explicitly ignore return code of stream_cli_close
by pespin 28 Nov '24

28 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/38979?usp=email ) Change subject: stream_cli: Explicitly ignore return code of stream_cli_close ...................................................................... stream_cli: Explicitly ignore return code of stream_cli_close Make coverity happy. We don't really case about th return code of the stream_cli_close() function in the code path, since the return value is only used internally/privately in the object and there's no further access/use of the object after calling it in osmo_stream_cli_close(). Take the chance to update syntax of one return code check to match more similarly other return checks of the same function. Related: Coverity CID#435092 Change-Id: Ia6c9e3ca3af08b386f017460b0a0210ed756a929 --- M src/stream_cli.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/79/38979/1 diff --git a/src/stream_cli.c b/src/stream_cli.c index 43877d3..85ede60 100644 --- a/src/stream_cli.c +++ b/src/stream_cli.c @@ -219,7 +219,7 @@ * abstraction and close the socket */ void osmo_stream_cli_close(struct osmo_stream_cli *cli) { - stream_cli_close(cli); + (void)stream_cli_close(cli); } /*! Re-connect an Osmocom Stream Client. @@ -967,7 +967,7 @@ return; LOGSCLI(cli, LOGL_DEBUG, "destroy()\n"); - OSMO_ASSERT(!stream_cli_close(cli)); + OSMO_ASSERT(stream_cli_close(cli) == false); osmo_timer_del(&cli->timer); msgb_queue_free(&cli->tx_queue); cli->tx_queue_count = 0; -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/38979?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ia6c9e3ca3af08b386f017460b0a0210ed756a929 Gerrit-Change-Number: 38979 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-cbc[master]: cbsp_link: Log unexpected return error codes
by pespin 28 Nov '24

28 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-cbc/+/38977?usp=email ) Change subject: cbsp_link: Log unexpected return error codes ...................................................................... cbsp_link: Log unexpected return error codes This easies spotting the cause of close/destroy code paths. Change-Id: Ibfd9a33798ba88e2332c00ed90709e38bb2222d7 --- M src/cbsp_link.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-cbc refs/changes/77/38977/1 diff --git a/src/cbsp_link.c b/src/cbsp_link.c index 2c31455..07fbadd 100644 --- a/src/cbsp_link.c +++ b/src/cbsp_link.c @@ -121,6 +121,7 @@ /* message de-segmentation */ rc = osmo_cbsp_recv_buffered(conn, ofd->fd, &msg, &link->rx_msg); if (rc <= 0) { + LOGPCC(link, LOGL_NOTICE, "osmo_cbsp_recv_buffered() ret %d\n", rc); if (rc == -EAGAIN || rc == -EINTR) { /* more data needs to be read */ return 0; @@ -205,6 +206,7 @@ /* message de-segmentation */ rc = osmo_cbsp_recv_buffered(conn, ofd->fd, &msg, &link->rx_msg); if (rc <= 0) { + LOGPCC(link, LOGL_NOTICE, "osmo_cbsp_recv_buffered() ret %d\n", rc); if (rc == -EAGAIN || rc == -EINTR) { /* more data needs to be read */ return 0; -- To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/38977?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-cbc Gerrit-Branch: master Gerrit-Change-Id: Ibfd9a33798ba88e2332c00ed90709e38bb2222d7 Gerrit-Change-Number: 38977 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-cbc[master]: abis: Fix reusing link->conn while it is being destroyed
by pespin 28 Nov '24

28 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-cbc/+/38978?usp=email ) Change subject: abis: Fix reusing link->conn while it is being destroyed ...................................................................... abis: Fix reusing link->conn while it is being destroyed Call to osmo_stream_cli_destroy() may end up triggering disconnect_cb(), and we don't want to attempt doing more stuff with that conn when that happens (we were even trying to reconnect() it...). Avoid it by marking the pointer as NULL immediatelly before calling e1inp_sign_link_destroy(). Change-Id: I56605766af974d86610d0958f4b82e6e4ff94502 --- M src/cbsp_link.c 1 file changed, 5 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-cbc refs/changes/78/38978/1 diff --git a/src/cbsp_link.c b/src/cbsp_link.c index 07fbadd..6b355df 100644 --- a/src/cbsp_link.c +++ b/src/cbsp_link.c @@ -102,6 +102,8 @@ static int cbc_cbsp_link_cli_disconnect_cb(struct osmo_stream_cli *conn) { struct cbc_cbsp_link *link = osmo_stream_cli_get_data(conn); + if (!link->conn) /* conn is being destroyed by us, we called osmo_stream_cli_destroy() */ + return 0; LOGPCC(link, LOGL_NOTICE, "Disconnected.\n"); LOGPCC(link, LOGL_NOTICE, "Reconnecting...\n"); osmo_stream_cli_reconnect(conn); @@ -364,10 +366,11 @@ return; if (link->is_client) { - osmo_stream_cli_destroy(link->cli_conn); + struct osmo_stream_cli *cli_conn = link->cli_conn; + link->cli_conn = NULL; + osmo_stream_cli_destroy(cli_conn); if (link->peer) link->peer->link.cbsp = NULL; - link->cli_conn = NULL; if (link->fi) osmo_fsm_inst_dispatch(link->fi, CBSP_LINK_E_CMD_CLOSE, NULL); } else { -- To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/38978?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-cbc Gerrit-Branch: master Gerrit-Change-Id: I56605766af974d86610d0958f4b82e6e4ff94502 Gerrit-Change-Number: 38978 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-cbc[master]: .gitignore: Add *~
by pespin 28 Nov '24

28 Nov '24

Attention is currently required from: laforge, pespin. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-cbc/+/38973?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by laforge, Verified+1 by Jenkins Builder Change subject: .gitignore: Add *~ ...................................................................... .gitignore: Add *~ This allows getting rid of further files like install-sh~. Change-Id: Iad7ce44a2ecd90116376a458cc40f355d3883c53 --- M .gitignore 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-cbc refs/changes/73/38973/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/38973?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-cbc Gerrit-Branch: master Gerrit-Change-Id: Iad7ce44a2ecd90116376a458cc40f355d3883c53 Gerrit-Change-Number: 38973 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-ttcn3-hacks[master]: bsc: osmo-bsc.cfg: Listen IPA Abis and CTRL interfaces on 127.0.0.1
by pespin 28 Nov '24

28 Nov '24

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38963?usp=email ) Change subject: bsc: osmo-bsc.cfg: Listen IPA Abis and CTRL interfaces on 127.0.0.1 ...................................................................... bsc: osmo-bsc.cfg: Listen IPA Abis and CTRL interfaces on 127.0.0.1 CTRL seems to bind to 127.0.0.1 by default, but IPA Abis listents to 0.0.0.0 by default, which is not needed and may create problems with concurrent instances. Change-Id: I56c8c6af16502e8f98b28902a251e190da0c0490 --- M bsc/osmo-bsc.cfg 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: osmith: Looks good to me, approved Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve diff --git a/bsc/osmo-bsc.cfg b/bsc/osmo-bsc.cfg index 958db6a..a03aeaf 100644 --- a/bsc/osmo-bsc.cfg +++ b/bsc/osmo-bsc.cfg @@ -45,11 +45,14 @@ line vty no login bind 127.0.0.1 +ctrl + bind 127.0.0.1 ! e1_input e1_line 0 driver ipa e1_line 0 port 0 no e1_line 0 keepalive + ipa bind 127.0.0.1 cs7 instance 0 asp asp-clnt-msc-0 2905 1905 m3ua remote-ip 127.0.0.1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38963?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I56c8c6af16502e8f98b28902a251e190da0c0490 Gerrit-Change-Number: 38963 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmo-netif[master]: stream_cli: Assert reentrant disconnect_cb() never happens
by pespin 28 Nov '24

28 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/38976?usp=email ) Change subject: stream_cli: Assert reentrant disconnect_cb() never happens ...................................................................... stream_cli: Assert reentrant disconnect_cb() never happens This should in fact already be guarded by early return in stream_cli_close() setting and checking for CLOSED state, but it doesn't hurt having this assert which also provides extra information to the reader. Change-Id: I9a12d76e27dce2b31a2aa993b70cbed30bdea7f2 --- M src/stream_cli.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/76/38976/1 diff --git a/src/stream_cli.c b/src/stream_cli.c index 2c1b00d..43877d3 100644 --- a/src/stream_cli.c +++ b/src/stream_cli.c @@ -203,6 +203,7 @@ * Also, if reconnect is disabled by user, notify the user that connect() failed: */ if (old_state == STREAM_CLI_STATE_CONNECTED || (old_state == STREAM_CLI_STATE_CONNECTING && cli->reconnect_timeout < 0)) { + OSMO_ASSERT(!(cli->in_cb_mask & IN_CB_MASK_DISCONNECT_CB)); cli->in_cb_mask |= IN_CB_MASK_DISCONNECT_CB; if (cli->disconnect_cb) cli->disconnect_cb(cli); -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/38976?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9a12d76e27dce2b31a2aa993b70cbed30bdea7f2 Gerrit-Change-Number: 38976 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmo-netif[master]: stream_cli: steal talloc ctx upon delaying free to avoid use after free
by pespin 28 Nov '24

28 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/38975?usp=email ) Change subject: stream_cli: steal talloc ctx upon delaying free to avoid use after free ...................................................................... stream_cli: steal talloc ctx upon delaying free to avoid use after free Scenario: read_cb() [user code] osmo_stream_cli_destroy() stream_cli_close() disconnect_cb() (free is delayed because we are in cb) [user code] (frees parent struct of "cli", which would free cli) free_delayed_if_needed() called on an already freed poiter Fixes: a6af7cb9dce9d4567d355bad873c60be68896eee Related: OS#6651 Change-Id: Id7625558d4b1343d621fb51406fa7f841db864ee --- M src/stream_cli.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/75/38975/1 diff --git a/src/stream_cli.c b/src/stream_cli.c index 89681b0..2c1b00d 100644 --- a/src/stream_cli.c +++ b/src/stream_cli.c @@ -974,6 +974,11 @@ if (cli->in_cb_mask != 0) { LOGSCLI(cli, LOGL_DEBUG, "delay free() in_cb_mask=0x%02x\n", cli->in_cb_mask); cli->delay_free = true; + /* Move ptr to avoid double free if parent ctx of cli is freed + * meanwhile (eg. during user callback after calling + * osmo_stream_client_destroy() and before returning from user + * callback. */ + talloc_steal(OTC_GLOBAL, cli); } else { LOGSCLI(cli, LOGL_DEBUG, "free(destroy)\n"); talloc_free(cli); -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/38975?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Id7625558d4b1343d621fb51406fa7f841db864ee Gerrit-Change-Number: 38975 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-ci[master]: jobs/ttcn3-ns-test{,-sns}: tweak timer
by osmith 28 Nov '24

28 Nov '24

osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/38971?usp=email ) Change subject: jobs/ttcn3-ns-test{,-sns}: tweak timer ...................................................................... jobs/ttcn3-ns-test{,-sns}: tweak timer Since the timers were adjusted last time, ttcn3-ns-test{,-sns} started failing every other day with: python3-osmopy-utils : Depends: osmocom-nightly (= 202411222026) but it is not going to be installed The reason is that the binary packages are not ready yet at that time. Run the jobs 1h later to avoid this. Change-Id: I4f61013661d5de5382c3431650051989200fd785 --- M jobs/ttcn3-testsuites.yml 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified osmith: Looks good to me, approved diff --git a/jobs/ttcn3-testsuites.yml b/jobs/ttcn3-testsuites.yml index 5ad31c8..92b7e6d 100644 --- a/jobs/ttcn3-testsuites.yml +++ b/jobs/ttcn3-testsuites.yml @@ -38,9 +38,9 @@ timer: H 03 * * * node: hdlc - ttcn3-ns-test: # ~7 min - timer: H 03 * * * + timer: H 04 * * * - ttcn3-ns-test-sns: # ~3 min - timer: H 03 * * * + timer: H 04 * * * - ttcn3-ns-test-fr: # ~10 min timer: H 04 * * * node: hdlc -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/38971?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: I4f61013661d5de5382c3431650051989200fd785 Gerrit-Change-Number: 38971 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log