May 2022 - gerrit-log - lists.osmocom.org

Change in osmo-bsc[master]: fix performance for chan_counts and all_allocated stats

by pespin

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28205 ) Change subject: fix performance for chan_counts and all_allocated stats ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/chan_counts.c: https://gerrit.osmocom.org/c/osmo-bsc/+/28205/comment/ae278fd7_d1352524 PS1, Line 233: case NM_OC_RADIO_CARRIER: > Pau, I'd appreciate very much if you could confirm which signals should […] The situation you are trying to catch ("the trx is enabled, just after trx_is_usable() starts to return true." || "the trx is disabled, just after trx_is_usable() returns false.") is precisely what S_NM_RUNNING_CHG I itnroduced recently is for. You should be using that one only in that case (you also need to do the checks for the BBTRANSC also, since trx_is_usable() checks both RCARRIER and BBTRANSC). Please have a look at the commit messages I posted above, they contain a couple examples on how to use the signal. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28205 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I580bfae329aac8d4552723164741536af6512011 Gerrit-Change-Number: 28205 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 31 May 2022 17:14:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years

1
0
0 0

Change in osmo-bsc[master]: fix performance for chan_counts and all_allocated stats

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28205 ) Change subject: fix performance for chan_counts and all_allocated stats ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/chan_counts.c: https://gerrit.osmocom.org/c/osmo-bsc/+/28205/comment/56910c2f_b8b7bf9b PS1, Line 233: case NM_OC_RADIO_CARRIER: > I guess this is to update when ts_is_usable (trx_is_usable()) changes. […] Pau, I'd appreciate very much if you could confirm which signals should be used. Here is what I am trying to do: What i need is to call chan_counts_trx_update(trx) when: - the trx is enabled, just after trx_is_usable() starts to return true. (So we need to wait until after the opstart ACK is received and the state changed) - the trx is disabled, just after trx_is_usable() returns false. I noticed this from BSC_Tests.TC_ctrl_trx_rf_locked. Is there also an rf_locked on the BTS level? In that case we probably also need the BTS level obj_class, indeed. And there should then probably also be a ttcn3 test for that? I find it quite hard to understand how to detect when a TRX actually becomes active or inactive. Particularly because of different BTS models apparently doing things differently. For lchans and timeslots it is relatively obvious, because of the bts-model agnostic timeslot_fsm and lchan_fsm. For TRX and BTS levels it's not so easy. After a long while of looking and testing, I finally came up with this signal that works. Before, I tried to put the counting in nm_rcarrier_fsm_becomes_enabled(), which did not work, so that's why I doubt that S_NM_RUNNING_CHG is the right signal (or maybe i am confusing the signals). The counting has to happen right after trx_is_usable() returns the new state, and before anything else happens. BTW if we nail those state transitions, we might be able to get rid of that periodic bsc_update_connection_stats(); when writing that code i already tried to pinpoint these state changes and gave up in the end. But, all of this is not critical. When we miss some state update, there will still be the periodic verification that fixes errors once per second. So we are now "just" discussing how to avoid harmless errors in the log. Even if this patch misses some state changes, the counts will be correct, just not immediately. All the state changes on lchans where it is important to immediately update the counts are covered by this patch. Still, it would be great to understand which signals are correct. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28205 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I580bfae329aac8d4552723164741536af6512011 Gerrit-Change-Number: 28205 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 31 May 2022 17:08:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years

1
0
0 0

Change in osmo-bsc[master]: code cleanup for all_allocated timers, no functional change

by fixeria

Attention is currently required from: neels. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28204 ) Change subject: code cleanup for all_allocated timers, no functional change ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File src/osmo-bsc/bsc_stats.c: https://gerrit.osmocom.org/c/osmo-bsc/+/28204/comment/5c4f97bf_db5c6810 PS1, Line 224: void all_allocated_update_bsc() { > src/osmo-bsc/bsc_stats. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28204 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id7a82c65d56a87818fc35bbeedf67e2af2f89f11 Gerrit-Change-Number: 28204 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 31 May 2022 16:26:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

3 years

1
0
0 0

Change in osmo-bsc[master]: code cleanup for all_allocated timers, no functional change

by pespin

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28204 ) Change subject: code cleanup for all_allocated timers, no functional change ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28204 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id7a82c65d56a87818fc35bbeedf67e2af2f89f11 Gerrit-Change-Number: 28204 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 31 May 2022 16:11:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

3 years

1
0
0 0

Change in osmo-bsc[master]: fix performance for chan_counts and all_allocated stats

by neels

Attention is currently required from: neels. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/28205 to look at the new patch set (#2). Change subject: fix performance for chan_counts and all_allocated stats ...................................................................... fix performance for chan_counts and all_allocated stats The all_allocated_update_bsc() does inefficient iterating to count active/inactive lchans, which scales badly for high numbers of TRX managed by osmo-bsc. We need to update the all_allocated flags immediately (periodic counting alone would suffer from undersampling), so, until now, we are calling this inefficient function every time a channel state changes. Instead of iterating all channels for any chan state changes anywhere, keep global state of the current channel counts, and on channel state change only update those ts, trx, bts counts that actually change. A desirable side effect: for connection stats and handover decision 2, we can now also use the globally updated channel counts and save a bunch of inefficient iterations. To get accurate channel counts at all times, spread around some chan_counts_ts_update() calls in pivotal places. It re-counts the given timeslot and cascades counter changes, iff required. Just in case I missed some channel accounting, still run an inefficient iterating count regularly that detects errors, logs them and fixes them. No real harm done if such error appears. None show in ttcn3 BSC_Tests. It is fine to do the inefficient iteration once per second; channel state changes can realistically happen hundreds of times per second. Related: SYS#5976 Change-Id: I580bfae329aac8d4552723164741536af6512011 --- M include/osmocom/bsc/bsc_stats.h M include/osmocom/bsc/bts.h M include/osmocom/bsc/bts_trx.h M include/osmocom/bsc/chan_counts.h M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/abis_rsl.c M src/osmo-bsc/bsc_stats.c M src/osmo-bsc/chan_counts.c M src/osmo-bsc/handover_decision_2.c M src/osmo-bsc/lchan_fsm.c M src/osmo-bsc/osmo_bsc_main.c M src/osmo-bsc/timeslot_fsm.c M tests/handover/handover_test.c 13 files changed, 319 insertions(+), 94 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/05/28205/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28205 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I580bfae329aac8d4552723164741536af6512011 Gerrit-Change-Number: 28205 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

3 years

1
0
0 0

Change in osmo-bsc[master]: code cleanup for all_allocated timers, no functional change

by neels

Attention is currently required from: neels. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/28204 to look at the new patch set (#2). Change subject: code cleanup for all_allocated timers, no functional change ...................................................................... code cleanup for all_allocated timers, no functional change Reduce some code dup in all_allocated accounting and cosmetically prepare for upcoming performance fix. Have a struct all_allocated, allow easy re-use of function all_allocated_update(). Rename function to all_allocated_update_bsc(). Upcoming patch will also add all_allocated_update_bts(). Related: SYS#5976 Change-Id: Id7a82c65d56a87818fc35bbeedf67e2af2f89f11 --- M include/osmocom/bsc/bsc_stats.h M include/osmocom/bsc/bts.h M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/bsc_init.c M src/osmo-bsc/bsc_stats.c M src/osmo-bsc/bts.c M src/osmo-bsc/lchan_fsm.c 7 files changed, 51 insertions(+), 70 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/04/28204/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28204 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id7a82c65d56a87818fc35bbeedf67e2af2f89f11 Gerrit-Change-Number: 28204 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

3 years

1
0
0 0

Change in osmo-bsc[master]: fix rare segfault in MGCP client handling

by neels

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28210 ) Change subject: fix rare segfault in MGCP client handling ...................................................................... fix rare segfault in MGCP client handling Add missing conn->assignment.created_ci_for_msc to gscon_forget_mgw_endpoint_ci(). Before this patch, when assignment.created_ci_for_msc lingers after a DLCX, it can cause a use-after-free on assignment_reset(). Possible scenario is rx BSSMAP Clear Cmd during ongoing Assignment. In assignment_reset(), locally cache the ci pointer, because gscon_forget_mgw_endpoint_ci() now NULLs created_ci_for_msc. Related: OS#5572 Change-Id: If89610020f47fd6517081dd11b83911b043bd0f1 --- M src/osmo-bsc/assignment_fsm.c M src/osmo-bsc/bsc_subscr_conn_fsm.c 2 files changed, 8 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve neels: Looks good to me, approved diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c index 7deca65..a0d008d 100644 --- a/src/osmo-bsc/assignment_fsm.c +++ b/src/osmo-bsc/assignment_fsm.c @@ -128,10 +128,13 @@ } if (conn->assignment.created_ci_for_msc) { - gscon_forget_mgw_endpoint_ci(conn, conn->assignment.created_ci_for_msc); + /* Store ci pointer locally, because gscon_forget_mgw_endpoint_ci() NULLs + * conn->assignment.created_ci_for_msc. */ + struct osmo_mgcpc_ep_ci *ci = conn->assignment.created_ci_for_msc; + gscon_forget_mgw_endpoint_ci(conn, ci); /* If this is the last endpoint released, the mgw_endpoint_fsm will terminate and tell * the gscon about it. */ - osmo_mgcpc_ep_ci_dlcx(conn->assignment.created_ci_for_msc); + osmo_mgcpc_ep_ci_dlcx(ci); } conn->assignment = (struct assignment_fsm_data){ diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c index 54d3975..9af28c7 100644 --- a/src/osmo-bsc/bsc_subscr_conn_fsm.c +++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c @@ -955,6 +955,9 @@ if (conn->user_plane.mgw_endpoint_ci_msc == ci) conn->user_plane.mgw_endpoint_ci_msc = NULL; + + if (conn->assignment.created_ci_for_msc == ci) + conn->assignment.created_ci_for_msc = NULL; } static void gscon_fsm_allstate(struct osmo_fsm_inst *fi, uint32_t event, void *data) -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28210 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: If89610020f47fd6517081dd11b83911b043bd0f1 Gerrit-Change-Number: 28210 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years

1
0
0 0

Change in osmo-bsc[master]: fix rare segfault in MGCP client handling

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28210 ) Change subject: fix rare segfault in MGCP client handling ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File src/osmo-bsc/assignment_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/28210/comment/a26a33f8_7ebd1f5c PS1, Line 134: gscon_forget_mgw_endpoint_ci(conn, ci); > wouldn't it make sense to call osmo_mgcpc_ep_ci_dlcx(ci); inside gscon_forget_mgw_endpoint_ci() befo […] the dlcx() emits events that may cause FSMs to terminate and clean up (again), so we need to invalidate the reference to the ci first. The *_forget_* is exactly about NULLing references in various places, the actual reason for NULLing should be kept apart. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28210 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: If89610020f47fd6517081dd11b83911b043bd0f1 Gerrit-Change-Number: 28210 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 31 May 2022 13:10:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years

1
0
0 0

Change in pysim[master]: pySim-shell: catch exceptions from walk() while exporting

by dexter

Attention is currently required from: fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/28162 ) Change subject: pySim-shell: catch exceptions from walk() while exporting ...................................................................... Patch Set 3: (1 comment) File pySim-shell.py: https://gerrit.osmocom.org/c/pysim/+/28162/comment/a49927b4_6e56fe5b PS2, Line 679: except Exception as e: > Does pySim-shell support multi-line comments in a script file? If so, I would do: […] unfortunately cmd2 does not support this. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/28162 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I3edc250ef2a84550c5b821a72e207e4d685790a5 Gerrit-Change-Number: 28162 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 31 May 2022 12:09:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

3 years

1
0
0 0

Change in pysim[master]: pySim-shell: catch exceptions from walk() while exporting

by dexter

Attention is currently required from: fixeria. Hello Jenkins Builder, laforge, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/28162 to look at the new patch set (#3). Change subject: pySim-shell: catch exceptions from walk() while exporting ...................................................................... pySim-shell: catch exceptions from walk() while exporting When we run the exporter we also get an error summary at the end. However, if walk() throws an eception this stops the exporter immediately and we won't get the summpary. Lets catch exceptions from walk as well so that we are able to end gracefully. Change-Id: I3edc250ef2a84550c5b821a72e207e4d685790a5 --- M pySim-shell.py 1 file changed, 12 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/62/28162/3 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/28162 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I3edc250ef2a84550c5b821a72e207e4d685790a5 Gerrit-Change-Number: 28162 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

3 years

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2022