July 2025 - gerrit-log - lists.osmocom.org

[M] Change in osmo-msc[master]: ran_peer: Add specific API to free object
by neels 14 Jul '25

14 Jul '25

Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40632?usp=email ) Change subject: ran_peer: Add specific API to free object ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > I disagree with this patch, because you are mixing the "there is a free() call in the source" with " […] (marking not resolved) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40632?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I721de21a68a4e336ae508a995e3cfcca05d57efe Gerrit-Change-Number: 40632 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 14 Jul 2025 14:17:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>

1 0

[S] Change in osmo-msc[master]: vty: Get rid of unneeded iu_client vty config
by neels 14 Jul '25

14 Jul '25

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/40645?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: vty: Get rid of unneeded iu_client vty config ...................................................................... vty: Get rid of unneeded iu_client vty config Use of osmo-iuh's iu_client API was removed from osmo-msc long time ago (see c4628a3ad4d3c5f65782b152b771bf80357235d6), and its removal is also scheduled in osmo-sgsn. Still, some small reference bits were still left. Remove them. The VTY init functions being dropped basically set g_rab_assign_addr_enc, which doesn't really seem to be used at all, only place is that file for vty related stuff which has actually no use in osmo-msc. Other 2 are asn1c debugging related, which can also probably be dropped. This completelly removes references to iu_client.h in osmo-msc. Related: OS#5487 Change-Id: I9cf83f2255e1e9aa83f3139b88ea81b2f5b686c3 --- M src/libmsc/msc_vty.c 1 file changed, 0 insertions(+), 10 deletions(-) Approvals: neels: Looks good to me, approved fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/libmsc/msc_vty.c b/src/libmsc/msc_vty.c index 2b21f70..41cb0d4 100644 --- a/src/libmsc/msc_vty.c +++ b/src/libmsc/msc_vty.c @@ -43,10 +43,6 @@ #include <osmocom/vty/misc.h> #include <osmocom/vty/stats.h> -#ifdef BUILD_IU -#include <osmocom/ranap/iu_client.h> -#endif - #include <osmocom/msc/vty.h> #include <osmocom/msc/gsm_data.h> #include <osmocom/msc/gsm_subscriber.h> @@ -815,9 +811,6 @@ } mgcp_client_config_write(vty, " "); -#ifdef BUILD_IU - ranap_iu_vty_config_write(vty, " "); -#endif neighbor_ident_vty_write(vty); @@ -2125,9 +2118,6 @@ /* Deprecated: Old MGCP config without pooling support in MSC node: */ mgcp_client_vty_init(msc_network, MSC_NODE, msc_network->mgw.conf); -#ifdef BUILD_IU - ranap_iu_vty_init(MSC_NODE, (enum ranap_nsap_addr_enc*)&msc_network->iu.rab_assign_addr_enc); -#endif sgs_vty_init(); smsc_vty_init(msc_network); asci_vty_init(msc_network); -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40645?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I9cf83f2255e1e9aa83f3139b88ea81b2f5b686c3 Gerrit-Change-Number: 40645 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>

1 0

[XS] Change in osmo-msc[master]: msc_main: Remove unnedeed include of ranap/iu_client.h
by neels 14 Jul '25

14 Jul '25

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/40642?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: msc_main: Remove unnedeed include of ranap/iu_client.h ...................................................................... msc_main: Remove unnedeed include of ranap/iu_client.h iu_client is so far only used to set up some VTY configs in msc_vty.c, hence not needed here. Change-Id: I4716c5229273b74ee165ebaaf0914e9eebef9e6f --- M src/osmo-msc/msc_main.c 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: fixeria: Looks good to me, approved neels: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/osmo-msc/msc_main.c b/src/osmo-msc/msc_main.c index 2435029..3c09742 100644 --- a/src/osmo-msc/msc_main.c +++ b/src/osmo-msc/msc_main.c @@ -76,7 +76,6 @@ #include <osmocom/msc/mncc_call.h> #ifdef BUILD_IU -#include <osmocom/ranap/iu_client.h> #include <asn1c/asn_internal.h> #endif -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40642?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I4716c5229273b74ee165ebaaf0914e9eebef9e6f Gerrit-Change-Number: 40642 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>

1 0

[S] Change in osmo-msc[master]: vty: Get rid of unneeded iu_client vty config
by neels 14 Jul '25

14 Jul '25

Attention is currently required from: lynxis lazus, pespin. neels has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40645?usp=email ) Change subject: vty: Get rid of unneeded iu_client vty config ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40645?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I9cf83f2255e1e9aa83f3139b88ea81b2f5b686c3 Gerrit-Change-Number: 40645 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Mon, 14 Jul 2025 14:05:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in osmo-msc[master]: msc_main: Remove unnedeed include of ranap/iu_client.h
by neels 14 Jul '25

14 Jul '25

Attention is currently required from: lynxis lazus, pespin. neels has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40642?usp=email ) Change subject: msc_main: Remove unnedeed include of ranap/iu_client.h ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40642?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I4716c5229273b74ee165ebaaf0914e9eebef9e6f Gerrit-Change-Number: 40642 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Mon, 14 Jul 2025 14:03:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in osmo-msc[master]: ran_peer: Introuce stats msc.ran_peers.{total,active}
by neels 14 Jul '25

14 Jul '25

Attention is currently required from: pespin. neels has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40633?usp=email ) Change subject: ran_peer: Introuce stats msc.ran_peers.{total,active} ...................................................................... Patch Set 4: Code-Review-1 (2 comments) File include/osmocom/msc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-msc/+/40633/comment/12389ce5_8ff9c31c?usp… : PS4, Line 127: [MSC_STAT_RAN_PEERS_TOTAL] = { "msc.ran_peers.total", "Total RAN peers (BSC, RNC)", OSMO_STAT_ITEM_NO_UNIT, 4, 0}, (maybe add "seen"? like "Total RAN peers seen since startup" ... just thinking to clarify the difference between TOTAL and ACTIVE) File src/libmsc/ran_peer.c: https://gerrit.osmocom.org/c/osmo-msc/+/40633/comment/d81c63e8_1d062bf6?usp… : PS4, Line 95: osmo_stat_item_dec(osmo_stat_item_group_get_item(net->statg, MSC_STAT_RAN_PEERS_TOTAL), 1); you already count the state change in onleave(), so just make sure to change the state before/on deallocation. This FSM has a DISCARDING state exactly for this purpose. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40633?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I36e40510c9a95f0c9cf5f32d2a7baab840aa8660 Gerrit-Change-Number: 40633 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 14 Jul 2025 14:02:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[M] Change in osmo-msc[master]: msc: Initial implementation of N-PCSTATE.ind
by neels 14 Jul '25

14 Jul '25

Attention is currently required from: laforge, osmith, pespin. neels has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40630?usp=email ) Change subject: msc: Initial implementation of N-PCSTATE.ind ...................................................................... Patch Set 3: Code-Review+1 (10 comments) Patchset: PS3: some optional nitpicks.. File include/osmocom/msc/ran_peer.h: https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/1576b4ec_fb08873f?usp… : PS3, Line 94: struct ran_peer *ran_peer_find_by_addr(const struct sccp_ran_inst *sri, const struct osmo_sccp_addr *peer_addr); please consider placing orthogonal cosmetic fixes of prior code in a separate patch, because you should know that it steals time from every reader File src/libmsc/ran_peer.c: https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/252e3ef4_2e26dc29?usp… : PS3, Line 131: /* N-PCSTATE.ind informs us the peer went down and is no longer reachable: */ in this comment i expect to read what the *function* does File src/libmsc/sccp_ran.c: https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/87204a15_e8441d73?usp… : PS3, Line 63: address you mean PC? PC is a subset of possible SCCP addresses https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/8a2c6af2_0c48aa76?usp… : PS3, Line 63: n r typo https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/be104396_61685ce3?usp… : PS3, Line 64: get_ran_ below you call function ran_peer_find_by_addr(). let's use the same naming pattern here: ran_peer_find_by_pc() secondly, consider putting this function in ran_peer.h/c because static functions hidden in .c files tend to be duplicated; and this looks generally useful. https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/483a5b43_0dbdd2fb?usp… : PS3, Line 74: LOGP(DMSC, LOGL_DEBUG, "No ran_peer found under remote address: %s\n", osmo_sccp_addr_name(cs7, &rem_addr)); (if this function becomes public API: error logging should be done at the caller, because these helpers tend to be called from various places, half of which expect to gracefully handle missing objects instead of logging) https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/dc8ad4bf_dafccf55?usp… : PS3, Line 85: LOGP(DMSC, LOGL_DEBUG, "N-PCSTATE ind: affected_pc=%u=%s sp_status=%s remote_sccp_status=%s\n", (not sure if we should log *all* N-PCSTATE events) https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/9cdd08e2_20a07df3?usp… : PS3, Line 95: /* See if this marks the point code to have become available, or to have been lost. hehe i know this comment from somewhere =) high five https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/a72fb695_5a1e53f2?usp… : PS3, Line 146: } (... this logic exists also in osmo-bsc.git, should we move this to a library and re-use instead of copying?) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40630?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ice1b2c163b1b0d134fcaa1c8bf543038a35fabdf Gerrit-Change-Number: 40630 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 14 Jul 2025 13:50:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[XS] Change in osmo-msc[master]: ran_peer: Avoid paging attempt if not ready
by neels 14 Jul '25

14 Jul '25

Attention is currently required from: fixeria, pespin. neels has removed a vote from this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/40631?usp=email ) Change subject: ran_peer: Avoid paging attempt if not ready ...................................................................... Removed Code-Review+2 by fixeria <vyanitskiy(a)sysmocom.de> -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40631?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: deleteVote Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ia858291f4454e4caf293e1aaf60ea04d2d4a64e9 Gerrit-Change-Number: 40631 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-msc[master]: ran_peer: Avoid paging attempt if not ready
by neels 14 Jul '25

14 Jul '25

Attention is currently required from: pespin. neels has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40631?usp=email ) Change subject: ran_peer: Avoid paging attempt if not ready ...................................................................... Patch Set 3: Code-Review-1 (1 comment) File src/libmsc/ran_peer.c: https://gerrit.osmocom.org/c/osmo-msc/+/40631/comment/d99cccfb_14cf37a5?usp… : PS3, Line 686: if (!rp->fi || rp->fi->state != RAN_PEER_ST_READY) have you noticed that osmo-msc generally does not need to check "if (!obj->fi)" because the priv object is the talloc child of the fi. Please only check the fi->state, because if rp exists, then rp->fi exists. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40631?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ia858291f4454e4caf293e1aaf60ea04d2d4a64e9 Gerrit-Change-Number: 40631 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 14 Jul 2025 13:30:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[M] Change in osmo-msc[master]: ran_peer: Add specific API to free object
by neels 14 Jul '25

14 Jul '25

Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40632?usp=email ) Change subject: ran_peer: Add specific API to free object ...................................................................... Patch Set 4: Code-Review-2 (5 comments) Patchset: PS4: I disagree with this patch, because you are mixing the "there is a free() call in the source" with "it is not freed anywhere". In osmo-msc, you need to grep for osmo_fsm_inst_term instead. The memory management in osmo-msc is far from trivial, and it took a good amount of time to make sure that each object tears down dependent objects, while not causing double frees or leaks. The decision that each FSM instance takes care of its own priv object is PIVOTAL to this design. So in short, you are starting at a big refactoring, which I disagree with. Commit Message: https://gerrit.osmocom.org/c/osmo-msc/+/40632/comment/008e3430_79910dfb?usp… : PS2, Line 11: Swap the talloc tree parentship to have the fi be a child of the object, : which simplifies tear down triggered by FSM cleanup. > It's logical, but it has drawbacks, like not being able to quickly figuring out where the object is […] you may find that, but you have not convinced me yet. All FSMs that I implement have a priv object as the child of the fsm inst, hence this is the pattern i expect to see. It also accurately reflects that the FSM instance is responsible for deallocation. Could you describe an objective drawback, thx File src/libmsc/ran_peer.c: https://gerrit.osmocom.org/c/osmo-msc/+/40632/comment/21ab372d_6eef772c?usp… : PS4, Line 551: .cleanup = ran_peer_fsm_cleanup, The fsm.cleanup function is an integral part of osmo-msc's object management, to handle corner cases gracefully. I am pretty sure that ran_peer needs a cleanup function to be resilient against failures. Beyond this, I subjectively prefer that we stick to the pattern that is used in the other FSM instances in osmo-msc. https://gerrit.osmocom.org/c/osmo-msc/+/40632/comment/82f9d6fb_c56735d6?usp… : PS4, Line 81: void ran_peer_free(struct ran_peer *rp) AFAIR the idea is to instead call osmo_fsm_inst_term() https://gerrit.osmocom.org/c/osmo-msc/+/40632/comment/523d1d62_caa97f9a?usp… : PS4, Line 129: static void ran_peer_discard_all_conns(struct ran_peer *rp) separate patch -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40632?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I721de21a68a4e336ae508a995e3cfcca05d57efe Gerrit-Change-Number: 40632 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 14 Jul 2025 13:26:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

1 0