April 2023 - gerrit-log - lists.osmocom.org

[M] Change in osmo-bts[master]: Move GPRS NSE under BTS SiteMgr
by pespin 11 Apr '23

11 Apr '23

Attention is currently required from: fixeria. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32240 to look at the new patch set (#2). Change subject: Move GPRS NSE under BTS SiteMgr ...................................................................... Move GPRS NSE under BTS SiteMgr As per ipaccess expectancies and following TS 12.21. Change-Id: If44d8f256cab7b2660900cedfb0ed9fe67eb3420 --- M include/osmo-bts/bts.h M include/osmo-bts/bts_sm.h M src/common/bts.c M src/common/bts_sm.c M src/common/nm_bts_sm_fsm.c M src/common/nm_gprs_cell_fsm.c M src/common/nm_gprs_nse_fsm.c M src/common/nm_gprs_nsvc_fsm.c M src/common/oml.c M src/common/pcu_sock.c M src/common/vty.c 11 files changed, 88 insertions(+), 65 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/40/32240/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32240 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: If44d8f256cab7b2660900cedfb0ed9fe67eb3420 Gerrit-Change-Number: 32240 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in osmo-bts[master]: Merge gsm_network into gsm_bts_sm and place gsm_bts under it
by pespin 11 Apr '23

11 Apr '23

Attention is currently required from: fixeria. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32236 to look at the new patch set (#5). Change subject: Merge gsm_network into gsm_bts_sm and place gsm_bts under it ...................................................................... Merge gsm_network into gsm_bts_sm and place gsm_bts under it This way the data model in TS 12.21 (Figure 1) is followed, where there's a BTS Site Manager containing one or more BTS. In our case we only support 1 BTS (cell) so far. Change-Id: Ideb0d458ec631008223f861cf8b46d09524a1a21 Related: OS#5994 --- M include/osmo-bts/Makefile.am M include/osmo-bts/bts.h A include/osmo-bts/bts_sm.h M include/osmo-bts/gsm_data.h M include/osmo-bts/pcu_if.h M include/osmo-bts/vty.h M src/common/Makefile.am M src/common/abis_osmo.c M src/common/bts.c M src/common/bts_shutdown_fsm.c A src/common/bts_sm.c M src/common/main.c M src/common/nm_bts_sm_fsm.c M src/common/oml.c M src/common/pcu_sock.c M src/common/vty.c M src/osmo-bts-oc2g/oc2gbts_vty.c M src/osmo-bts-omldummy/main.c M tests/agch/agch_test.c M tests/cipher/cipher_test.c M tests/handover/handover_test.c M tests/meas/meas_test.c M tests/misc/misc_test.c M tests/paging/paging_test.c M tests/tx_power/tx_power_test.c 25 files changed, 272 insertions(+), 161 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/36/32236/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32236 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ideb0d458ec631008223f861cf8b46d09524a1a21 Gerrit-Change-Number: 32236 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in osmo-bts[master]: Merge gsm_network into gsm_bts_sm and place gsm_bts under it
by pespin 11 Apr '23

11 Apr '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32236 ) Change subject: Merge gsm_network into gsm_bts_sm and place gsm_bts under it ...................................................................... Patch Set 4: (1 comment) File src/common/bts.c: https://gerrit.osmocom.org/c/osmo-bts/+/32236/comment/cde1c099_71285387 PS4, Line 406: g_bts_sm->num_bts++; > Ack This change would not be part of this patch however, I'll fix it in a followup patch. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32236 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ideb0d458ec631008223f861cf8b46d09524a1a21 Gerrit-Change-Number: 32236 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Apr 2023 09:24:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: Move GPRS NSE under BTS SiteMgr
by pespin 11 Apr '23

11 Apr '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32240 ) Change subject: Move GPRS NSE under BTS SiteMgr ...................................................................... Patch Set 1: (2 comments) File include/osmo-bts/bts_sm.h: https://gerrit.osmocom.org/c/osmo-bts/+/32240/comment/7d832391_069306d3 PS1, Line 43: gprs > `pcu_state` is also related to GPRS, but not part of this struct. […] I liked having the .gprs suffix in the previous place, so I'm keeping here. We can move pcu_state to .gprs later on in another commit. File src/common/bts_sm.c: https://gerrit.osmocom.org/c/osmo-bts/+/32240/comment/bbd58577_7b9eb955 PS1, Line 77: %d", 0 > just have `0` in the format string and use `osmo_fsm_inst_update_id()`? Ack -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32240 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: If44d8f256cab7b2660900cedfb0ed9fe67eb3420 Gerrit-Change-Number: 32240 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Apr 2023 09:01:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-bts[master]: Merge gsm_network into gsm_bts_sm and place gsm_bts under it
by pespin 11 Apr '23

11 Apr '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32236 ) Change subject: Merge gsm_network into gsm_bts_sm and place gsm_bts under it ...................................................................... Patch Set 4: (5 comments) File src/common/bts.c: https://gerrit.osmocom.org/c/osmo-bts/+/32236/comment/fee8f64d_5be68596 PS4, Line 406: g_bts_sm->num_bts++; > I find this weird that a BTS is added to `&g_bts_sm->bts_list` in `gsm_bts_alloc()`, but `g_bts_sm-> […] Ack File src/common/bts_sm.c: https://gerrit.osmocom.org/c/osmo-bts/+/32236/comment/4b300e48_d802b7d0 PS4, Line 37: talloc_free > talloc does free() all its children anyway, do we really need to do this manually? I prefer doing it this way in order to control the order of stuff being freed. File src/common/oml.c: https://gerrit.osmocom.org/c/osmo-bts/+/32236/comment/914ddc11_1d905977 PS4, Line 154: if (!bts) { > `OSMO_UNLIKELY`? Ack File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/32236/comment/ab0b3e3a_a507e439 PS4, Line 897: struct gsm_bts_sm *bts_sm > Can we remove this argument since `g_bts_sm` is available globally? Yes we could, I'm not sure if it makes sense doing it in this commit or not, I simply changed the minimum required stuff in this file. https://gerrit.osmocom.org/c/osmo-bts/+/32236/comment/0cddd3b8_ec40428b PS4, Line 957: struct gsm_bts_sm *bts_sm > Can we remove this argument since `g_bts_sm` is available globally? Yes we could, I'm not sure if it makes sense doing it in this commit or not, I simply changed the minimum required stuff in this file. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32236 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ideb0d458ec631008223f861cf8b46d09524a1a21 Gerrit-Change-Number: 32236 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Apr 2023 09:00:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: si2quater: check return value of osmo_earfcn_del()
by fixeria 11 Apr '23

11 Apr '23

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32251 ) Change subject: si2quater: check return value of osmo_earfcn_del() ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/bts_vty.c: https://gerrit.osmocom.org/c/osmo-bsc/+/32251/comment/bd34aa3c_8768fbbe PS2, Line 2082: if (osmo_earfcn_del(e, arfcn) != 0) > IIUC if you arrived here is because you failed adding the earfcn to the list, so why would you want […] Not really. We end up here if generating si2quater fails after adding an EARFCN, so we need to remove it. See `si2q_num()` in `src/osmo-bsc/system_information.c`. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32251 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I227dad57737721c40a508f67616d9f5003bb1a3e Gerrit-Change-Number: 32251 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Apr 2023 08:59:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: si2quater: check return value of osmo_earfcn_del()
by pespin 11 Apr '23

11 Apr '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32251 ) Change subject: si2quater: check return value of osmo_earfcn_del() ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/bts_vty.c: https://gerrit.osmocom.org/c/osmo-bsc/+/32251/comment/9f57dc27_6c61eb5a PS2, Line 2082: if (osmo_earfcn_del(e, arfcn) != 0) IIUC if you arrived here is because you failed adding the earfcn to the list, so why would you want to remove it? -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32251 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I227dad57737721c40a508f67616d9f5003bb1a3e Gerrit-Change-Number: 32251 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Apr 2023 08:56:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: bsc: add TC_mscpool_sccp_n_pcstate_attaches_msc
by pespin 11 Apr '23

11 Apr '23

Attention is currently required from: neels, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32217 ) Change subject: bsc: add TC_mscpool_sccp_n_pcstate_attaches_msc ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32217 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: Ib4a5330df30a73e744c316898817b2fa3271d75e Gerrit-Change-Number: 32217 Gerrit-PatchSet: 3 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-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Apr 2023 08:54:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: logging: add 'logging timezone (localtime|utc)'
by pespin 11 Apr '23

11 Apr '23

Attention is currently required from: neels, laforge, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32043 ) Change subject: logging: add 'logging timezone (localtime|utc)' ...................................................................... Patch Set 2: (1 comment) File src/core/logging.c: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/d9ff978e_25f73f0f PS1, Line 874: target->timezone = timezone; > But we do not *need* to care whether a given timezone function is available. If the system cannot generate a timestamp the way it is configured, then we just don't print one in the logs. I'm sorry but this behavior makes no sense to me. Why they hell if I set a specific tiemstamp format I end up with having no timestamp at all? It really makes no sense. If I try to set a timestamp and the system/app is not supporting it, it should just tell me when I try to configure it that way. The only reason you argue for not doing that you are presenting are: - because it makes code more complex: A few more lines of code checking errors? - because it adds non-conforming API: aka having a function return a return code while others do. I really see no problem here. Specially in a lib public API, it makes totally sense to have it return an error code. - make it harder to launch an osmo program...: Not really. If you attempt to configure it in a way the system doesn't support it then it fails with the related error message, then you remove/change the config option which makes no sense in that system and then get an expected behavior. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32043 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4 Gerrit-Change-Number: 32043 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-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Apr 2023 08:50:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: nm: Dispatch NM_EV_SW_ACT in cascade to BTS SiteMgr children
by pespin 11 Apr '23

11 Apr '23

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32239 ) Change subject: nm: Dispatch NM_EV_SW_ACT in cascade to BTS SiteMgr children ...................................................................... nm: Dispatch NM_EV_SW_ACT in cascade to BTS SiteMgr children Change-Id: I97445812bb1b6de450411aceaeece2427027ae67 Related: OS#5994 --- M src/common/bts.c M src/common/nm_bts_sm_fsm.c 2 files changed, 11 insertions(+), 2 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/common/bts.c b/src/common/bts.c index 9c48a1d..37a3023 100644 --- a/src/common/bts.c +++ b/src/common/bts.c @@ -457,8 +457,6 @@ /* BTS SITE MGR becomes Offline (tx SW ACT Report), BTS, NSE, etc. is DEPENDENCY */ osmo_fsm_inst_dispatch(bts->site_mgr.mo.fi, NM_EV_SW_ACT, NULL); - osmo_fsm_inst_dispatch(bts->mo.fi, NM_EV_SW_ACT, NULL); - osmo_fsm_inst_dispatch(bts->gprs.nse.mo.fi, NM_EV_SW_ACT, NULL); /* All other objects start off-line until the BTS Model code says otherwise */ for (i = 0; i < bts->num_trx; i++) { diff --git a/src/common/nm_bts_sm_fsm.c b/src/common/nm_bts_sm_fsm.c index f775265..8752a90 100644 --- a/src/common/nm_bts_sm_fsm.c +++ b/src/common/nm_bts_sm_fsm.c @@ -69,6 +69,7 @@ case NM_EV_SW_ACT: oml_mo_tx_sw_act_rep(&site_mgr->mo); nm_bts_sm_fsm_state_chg(fi, NM_BTS_SM_ST_OP_DISABLED_OFFLINE); + ev_dispatch_children(site_mgr, event); return; default: OSMO_ASSERT(0); -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32239 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I97445812bb1b6de450411aceaeece2427027ae67 Gerrit-Change-Number: 32239 Gerrit-PatchSet: 2 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: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0