Attention is currently required from: daniel, fixeria, lynxis lazus, osmith.
Hello Jenkins Builder, daniel, fixeria, lynxis lazus, osmith,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41498?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+1 by fixeria, Code-Review+1 by osmith, Verified+1 by Jenkins Builder
Change subject: xua_rkm: Avoid unnecesary ASP lookup by name
......................................................................
xua_rkm: Avoid unnecesary ASP lookup by name
Change-Id: I5ca8f0180a0389d5de7b495cfe9f769985296383
---
M src/ss7_as.c
M src/ss7_as.h
M src/xua_rkm.c
3 files changed, 19 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/98/41498/3
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/41498?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I5ca8f0180a0389d5de7b495cfe9f769985296383
Gerrit-Change-Number: 41498
Gerrit-PatchSet: 3
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: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Attention is currently required from: daniel, fixeria, lynxis lazus.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/41498?usp=email )
Change subject: xua_rkm: Avoid unnecesary ASP lookup by name
......................................................................
Patch Set 2:
(1 comment)
File src/ss7_as.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41498/comment/887a51ba_f6254… :
PS2, Line 223: asp
> param name mismatch, should be `asp_name`
Done
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/41498?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I5ca8f0180a0389d5de7b495cfe9f769985296383
Gerrit-Change-Number: 41498
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: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Tue, 25 Nov 2025 13:22:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: daniel, lynxis lazus, pespin.
fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/41498?usp=email )
Change subject: xua_rkm: Avoid unnecesary ASP lookup by name
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File src/ss7_as.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41498/comment/712b5198_4688b… :
PS2, Line 223: asp
param name mismatch, should be `asp_name`
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/41498?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I5ca8f0180a0389d5de7b495cfe9f769985296383
Gerrit-Change-Number: 41498
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: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Tue, 25 Nov 2025 13:20:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/41502?usp=email )
Change subject: ss7_xua_srv: Refactor xua_accept_cb()
......................................................................
ss7_xua_srv: Refactor xua_accept_cb()
Move all "readonly" (no state change) checks at the start of the
function. This simplifies the later code paths in the function, also
allowing for more early return checks. This in turn allows simplifying
different destruction code paths of asp and its osmo_stream.
As a result, we can now call existing ss7_asp_disconnect_stream() API to
close the stream when needed, and also make sure we set
asp->xua_server=NULL and remove asp from oxs->siblings when the conn
disconnects, otherwise it could create unexpected states if the config
of the asp changes transport role client<->server or it gets assigned
due to a different oxs due to change in config too.
The new logic also fixes some corner-case scenarios, like (dis)allwoing
newly created dynamic ASPs for already existing previous conn after the
feature configuration has changed.
Change-Id: I191f97e8545ce2f9899e3cd3cc3a7eed854daf1e
---
M src/ss7_asp.c
M src/ss7_xua_srv.c
2 files changed, 58 insertions(+), 56 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/02/41502/1
diff --git a/src/ss7_asp.c b/src/ss7_asp.c
index 5325de9..377d9e4 100644
--- a/src/ss7_asp.c
+++ b/src/ss7_asp.c
@@ -756,8 +756,6 @@
osmo_stream_cli_destroy(asp->client);
if (asp->fi)
osmo_fsm_inst_term(asp->fi, OSMO_FSM_TERM_REQUEST, NULL);
- if (asp->xua_server)
- llist_del(&asp->siblings);
/* unlink from all ASs we are part of */
llist_for_each_entry(as, &asp->inst->as_list, list)
@@ -1360,6 +1358,10 @@
xua_asp_send_xlm_prim_simple(asp, OSMO_XLM_PRIM_M_SCTP_RELEASE, PRIM_OP_INDICATION);
asp->server = NULL;
+ if (asp->xua_server) {
+ llist_del(&asp->siblings);
+ asp->xua_server = NULL;
+ }
/* if we were dynamically allocated at accept_cb() time, let's
* self-destruct now. A new connection will re-create the ASP. */
diff --git a/src/ss7_xua_srv.c b/src/ss7_xua_srv.c
index 7e55777..85906b7 100644
--- a/src/ss7_xua_srv.c
+++ b/src/ss7_xua_srv.c
@@ -76,6 +76,7 @@
asp = ss7_asp_find_by_socket_addr(fd, oxs->cfg.trans_proto);
if (asp) {
+ LOGP(DLSS7, LOGL_INFO, "%s: matched connection to ASP %s\n", sock_name, asp->cfg.name);
if (!asp->cfg.is_server) {
LOGPASP(asp, DLSS7, LOGL_NOTICE,
"Reject incoming new connection from %s for ASP configured as 'transport-role client'\n",
@@ -92,6 +93,26 @@
talloc_free(sock_name);
return 0;
}
+ /* we need to check if we already have a socket associated, and close it. Otherwise it might
+ * happen that both the listen-fd for this accept() and the old socket are marked 'readable'
+ * during the same scheduling interval, and we're processing them in the "wrong" order, i.e.
+ * we first see the accept of the new fd before we see the close on the old fd */
+ if (asp->server) {
+ LOGPASP(asp, DLSS7, LOGL_FATAL, "accept of new connection from %s before old was closed "
+ "-> close old one\n", sock_name);
+ ss7_asp_disconnect_stream(asp);
+ /* Dynamic ASPs (transport-role=server) may be destroyed as a consequence, re-fetch the ASP: */
+ asp = ss7_asp_find_by_socket_addr(fd, oxs->cfg.trans_proto);
+ }
+ }
+
+ if (!asp && !oxs->cfg.accept_dyn_reg) {
+ LOGP(DLSS7, LOGL_NOTICE, "%s: %s connection without matching "
+ "ASP definition and no dynamic registration enabled, terminating\n",
+ sock_name, proto_name);
+ close(fd);
+ talloc_free(sock_name);
+ return -1;
}
srv = osmo_stream_srv_create2(oxs, link, fd, NULL);
@@ -127,71 +148,50 @@
}
osmo_stream_srv_set_closed_cb(srv, ss7_asp_xua_srv_conn_closed_cb);
- if (asp) {
- LOGP(DLSS7, LOGL_INFO, "%s: matched connection to ASP %s\n",
- sock_name, asp->cfg.name);
- /* we need to check if we already have a socket associated, and close it. Otherwise it might
- * happen that both the listen-fd for this accept() and the old socket are marked 'readable'
- * during the same scheduling interval, and we're processing them in the "wrong" order, i.e.
- * we first see the accept of the new fd before we see the close on the old fd */
- if (asp->server) {
- LOGPASP(asp, DLSS7, LOGL_FATAL, "accept of new connection from %s before old was closed "
- "-> close old one\n", sock_name);
- osmo_stream_srv_set_data(asp->server, NULL);
- osmo_stream_srv_destroy(asp->server);
- asp->server = NULL;
- }
- } else {
- if (!oxs->cfg.accept_dyn_reg) {
- LOGP(DLSS7, LOGL_NOTICE, "%s: %s connection without matching "
- "ASP definition and no dynamic registration enabled, terminating\n",
- sock_name, proto_name);
- } else {
- char namebuf[32];
- static uint32_t dyn_asp_num = 0;
- snprintf(namebuf, sizeof(namebuf), "asp-dyn-%u", dyn_asp_num++);
- asp = osmo_ss7_asp_find_or_create2(oxs->inst, namebuf, 0, 0,
- oxs->cfg.trans_proto,
- oxs->cfg.proto);
- if (asp) {
- char hostbuf[INET6_ADDRSTRLEN];
- const char *hostbuf_ptr = &hostbuf[0];
- char portbuf[16];
-
- osmo_sock_get_ip_and_port(fd, hostbuf, sizeof(hostbuf), portbuf, sizeof(portbuf), false);
- LOGP(DLSS7, LOGL_INFO, "%s: created dynamic ASP %s\n",
- sock_name, asp->cfg.name);
- asp->cfg.is_server = true;
- asp->cfg.role = OSMO_SS7_ASP_ROLE_SG;
- asp->cfg.local.port = oxs->cfg.local.port;
- asp->cfg.remote.port = atoi(portbuf);
- asp->dyn_allocated = true;
- asp->server = srv;
- ss7_asp_peer_set_hosts(&asp->cfg.local, asp,
- (const char * const*)oxs->cfg.local.host,
- oxs->cfg.local.host_cnt);
- ss7_asp_peer_set_hosts(&asp->cfg.remote, asp,
- &hostbuf_ptr, 1);
- if ((rc = xua_asp_fsm_start(asp, asp->cfg.role, LOGL_DEBUG)) < 0) {
- talloc_free(sock_name);
- osmo_ss7_asp_destroy(asp);
- return rc;
- }
- OSMO_ASSERT(asp->fi);
- }
- }
+ if (!asp) {
+ /* Dynamic registration: */
+ char namebuf[32];
+ char hostbuf[INET6_ADDRSTRLEN];
+ const char *hostbuf_ptr = &hostbuf[0];
+ char portbuf[16];
+ static uint32_t dyn_asp_num = 0;
+ snprintf(namebuf, sizeof(namebuf), "asp-dyn-%u", dyn_asp_num++);
+ asp = osmo_ss7_asp_find_or_create2(oxs->inst, namebuf, 0, 0,
+ oxs->cfg.trans_proto,
+ oxs->cfg.proto);
if (!asp) {
osmo_stream_srv_destroy(srv);
talloc_free(sock_name);
return -1;
}
- llist_add_tail(&asp->siblings, &oxs->asp_list);
+
+ osmo_sock_get_ip_and_port(fd, hostbuf, sizeof(hostbuf), portbuf, sizeof(portbuf), false);
+ LOGP(DLSS7, LOGL_INFO, "%s: created dynamic ASP %s\n", sock_name, asp->cfg.name);
+ asp->cfg.is_server = true;
+ asp->cfg.role = OSMO_SS7_ASP_ROLE_SG;
+ asp->cfg.local.port = oxs->cfg.local.port;
+ asp->cfg.remote.port = atoi(portbuf);
+ asp->dyn_allocated = true;
+ asp->server = srv;
+ ss7_asp_peer_set_hosts(&asp->cfg.local, asp,
+ (const char * const*)oxs->cfg.local.host,
+ oxs->cfg.local.host_cnt);
+ ss7_asp_peer_set_hosts(&asp->cfg.remote, asp,
+ &hostbuf_ptr, 1);
+ if ((rc = xua_asp_fsm_start(asp, asp->cfg.role, LOGL_DEBUG)) < 0) {
+ talloc_free(sock_name);
+ osmo_ss7_asp_destroy(asp);
+ return rc;
+ }
+ OSMO_ASSERT(asp->fi);
}
+ OSMO_ASSERT(asp);
/* update the ASP reference back to the server over which the
* connection came in */
asp->server = srv;
asp->xua_server = oxs;
+ llist_add_tail(&asp->siblings, &oxs->asp_list);
/* update the ASP socket name */
talloc_free(asp->sock_name);
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/41502?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I191f97e8545ce2f9899e3cd3cc3a7eed854daf1e
Gerrit-Change-Number: 41502
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Attention is currently required from: daniel, fixeria, laforge.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-dev/+/41496?usp=email )
Change subject: all.desp: libosmo-sigtran depends on libosmo-asn1-tcap
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://gerrit.osmocom.org/c/osmo-dev/+/41496/comment/6e04cff9_d4916dd7?usp… :
PS1, Line 9: --enable-tcap-loadsharing
> Maybe add an `*.opts` file for this? For example, see `hnbgw_with_nftables.opts`.
Is that going to be picked up by osmo-ttcn3-hacks.git just fine then?
--
To view, visit https://gerrit.osmocom.org/c/osmo-dev/+/41496?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-Change-Id: Ibc51a818b859d40ab8a34d9299d0f955cf6d966e
Gerrit-Change-Number: 41496
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-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 25 Nov 2025 13:16:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>