- gerrit-log - lists.osmocom.org

[XS] Change in libosmo-abis[master]: trau2rtp FR & EFR: fix uninitialized memory bug
by laforge 16 Sep '24

16 Sep '24

Attention is currently required from: falconia, fixeria. laforge has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38138?usp=email ) Change subject: trau2rtp FR & EFR: fix uninitialized memory bug ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > > Alternatively, we may clarify the API docs and state that it's responsibility of the caller to [ze […] I agree the current situation is not good API design - but we have been doing it this way [here] for a long time. Your patch now means we zero-initialize twice (the implicit memset in talloc_zero and the explicit one here). Shouldn't hurt performance as the data should be in cache after the first write, but still it's suboptimal. We also have no way of removing the memset in the existing code, as we have to keep forward/backward compatibility. All in all it's dissatisfactory either way. But safety first, so I think we should merge this. -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38138?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: I6e6693e096b920a973c8cc627e94884099d004b5 Gerrit-Change-Number: 38138 Gerrit-PatchSet: 1 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: falconia <falcon(a)freecalypso.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Sep 2024 07:33:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: falconia <falcon(a)freecalypso.org> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[S] Change in osmo-bsc[master]: Introduce hashtable to lookup bts by <LAC,CI>
by laforge 16 Sep '24

16 Sep '24

Attention is currently required from: fixeria, pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38127?usp=email ) Change subject: Introduce hashtable to lookup bts by <LAC,CI> ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/38127/comment/4eaee8cc_f8a3e503?usp… : PS2, Line 993: #define LAC_CI_HASHTABLE_KEY(lac, ci) ((((uint32_t)(ci)) << sizeof(lac)) | (uint32_t)(lac)) > I mean, please remove a tab before the `#define ...` (shift to the left). Acknowledged -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38127?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I47db6c7543e5c6c3b8f0de3ae5ee1b53c2b5f16f Gerrit-Change-Number: 38127 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-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Sep 2024 07:30:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-bsc[master]: Introduce hashtable to lookup bts by CI
by laforge 16 Sep '24

16 Sep '24

Attention is currently required from: pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38126?usp=email ) Change subject: Introduce hashtable to lookup bts by CI ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38126?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ia9d6a38496609fc906cc78124929e594e6a9d06e Gerrit-Change-Number: 38126 Gerrit-PatchSet: 1 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-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Sep 2024 07:30:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in osmo-bsc[master]: gsm_bts_num(): use hashtable to lookup bts
by laforge 16 Sep '24

16 Sep '24

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38085?usp=email ) Change subject: gsm_bts_num(): use hashtable to lookup bts ...................................................................... gsm_bts_num(): use hashtable to lookup bts gsm_bts_num() is mostly used to lookup bts object from its nr in VTY and CTRL interfaces. However, it's also used in hotter paths like pcu_rx(). It is also used during local neighbor resolution, mostly at startup or when the VTY is re-applied from CTRL interface. Related: SYS#7062 Change-Id: I7312da7d9aa80c6d0f2e92e9c7d20d32ce453ad1 --- M include/osmocom/bsc/bts.h M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/bts.c M src/osmo-bsc/gsm_data.c M src/osmo-bsc/net_init.c 5 files changed, 12 insertions(+), 3 deletions(-) Approvals: fixeria: Looks good to me, but someone else must approve laforge: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h index 38e675a..8e530ef 100644 --- a/include/osmocom/bsc/bts.h +++ b/include/osmocom/bsc/bts.h @@ -5,6 +5,7 @@ #include <stdbool.h> #include <osmocom/core/linuxlist.h> +#include <osmocom/core/hashtable.h> #include <osmocom/core/msgb.h> #include <osmocom/core/bitvec.h> #include <osmocom/gsm/tlv.h> @@ -340,6 +341,9 @@ /* Geographical location of the BTS, head list of "struct bts_location" */ struct llist_head loc_list; + /*! Entry in hash table network->bts_by_nr. */ + struct hlist_node node_by_nr; + /* number of this BTS in network */ gsm_bts_nr_t nr; /* human readable name / description */ diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index 715e8c9..5ed339d 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -20,6 +20,7 @@ #include <osmocom/core/tdef.h> #include <osmocom/core/time_cc.h> #include <osmocom/core/linuxlist.h> +#include <osmocom/core/hashtable.h> #include <osmocom/core/linuxrbtree.h> #include <osmocom/core/utils.h> @@ -986,6 +987,7 @@ gsm_bts_nr_t num_bts; struct llist_head bts_list; struct llist_head bts_rejected; + DECLARE_HASHTABLE(bts_by_nr, 10); /* BTS-based counters when we can't find the actual BTS * e.g. when conn->lchan is NULL */ diff --git a/src/osmo-bsc/bts.c b/src/osmo-bsc/bts.c index 790a8d4..38e5f11 100644 --- a/src/osmo-bsc/bts.c +++ b/src/osmo-bsc/bts.c @@ -159,6 +159,7 @@ /* Entries in bts->loc_list are freed by talloc recursively, no need to free them here. */ llist_del(&bts->list); + hash_del(&bts->node_by_nr); paging_destructor(bts); bts_setup_ramp_remove(bts); @@ -198,10 +199,12 @@ talloc_set_destructor(bts, gsm_bts_talloc_destructor); + bts->nr = bts_num; + llist_add_tail(&bts->list, &net->bts_list); + hash_add(net->bts_by_nr, &bts->node_by_nr, bts->nr); net->num_bts++; - bts->nr = bts_num; bts->num_trx = 0; INIT_LLIST_HEAD(&bts->trx_list); bts->network = net; diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c index d5c74ad..d725572 100644 --- a/src/osmo-bsc/gsm_data.c +++ b/src/osmo-bsc/gsm_data.c @@ -231,11 +231,10 @@ if (num >= net->num_bts) return NULL; - llist_for_each_entry(bts, &net->bts_list, list) { + hash_for_each_possible(net->bts_by_nr, bts, node_by_nr, num) { if (bts->nr == num) return bts; } - return NULL; } diff --git a/src/osmo-bsc/net_init.c b/src/osmo-bsc/net_init.c index e5d3fad..fc29d43 100644 --- a/src/osmo-bsc/net_init.c +++ b/src/osmo-bsc/net_init.c @@ -124,6 +124,7 @@ INIT_LLIST_HEAD(&net->bts_list); net->num_bts = 0; + hash_init(net->bts_by_nr); net->T_defs = gsm_network_T_defs; osmo_tdefs_reset(net->T_defs); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38085?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I7312da7d9aa80c6d0f2e92e9c7d20d32ce453ad1 Gerrit-Change-Number: 38085 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>

1 0

[XS] Change in osmo-bsc[master]: tests/ctrl/osmo-bsc-neigh-test.cfg: Fix duplicate <LAC,CI> between bt...
by laforge 16 Sep '24

16 Sep '24

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38086?usp=email ) Change subject: tests/ctrl/osmo-bsc-neigh-test.cfg: Fix duplicate <LAC,CI> between bts1 and bts2 ...................................................................... tests/ctrl/osmo-bsc-neigh-test.cfg: Fix duplicate <LAC,CI> between bts1 and bts2 According to TS23.003, CI should be unique in a LAC. Previous commit wrongly introduced a while ago a new "bts 2" with same LAC and CI as "bts 1" in config file. This makes current tests fails once a hashtable is added to lookup bts by lac, since the hashtable code does prepend new bts instead of appending them. As a result, bts 2 is returned instead of bts 1 being previously returned. Also change the ARFCN+BSIC since it was also duplicated. It makes not much sense to have 2 neighbor BTS with same ARFCN+BSIC. Fixes: 97ed3c8d971f3c231a1303efe61b3f98116f1e2f Change-Id: I7d1ebd529e050ecb4c14b9d3523637e4c8c87e1d --- M tests/ctrl/osmo-bsc-neigh-test.cfg 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/tests/ctrl/osmo-bsc-neigh-test.cfg b/tests/ctrl/osmo-bsc-neigh-test.cfg index d6a5c9d..35ff2cc 100644 --- a/tests/ctrl/osmo-bsc-neigh-test.cfg +++ b/tests/ctrl/osmo-bsc-neigh-test.cfg @@ -137,9 +137,9 @@ bts 2 type osmo-bts band DCS1800 - cell_identity 123 + cell_identity 124 location_area_code 0x0001 - base_station_id_code 55 + base_station_id_code 56 ms max power 15 cell reselection hysteresis 4 rxlev access min 0 @@ -158,7 +158,7 @@ gprs routing area 6 trx 0 rf_locked 0 - arfcn 880 + arfcn 881 nominal power 23 ! to use full TRX power, set max_power_red 0 max_power_red 20 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38086?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I7d1ebd529e050ecb4c14b9d3523637e4c8c87e1d Gerrit-Change-Number: 38086 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>

1 0

[XS] Change in osmo-bsc[master]: tests/ctrl/osmo-bsc-neigh-test.cfg: Fix duplicate <LAC,CI> between bt...
by laforge 16 Sep '24

16 Sep '24

Attention is currently required from: pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38086?usp=email ) Change subject: tests/ctrl/osmo-bsc-neigh-test.cfg: Fix duplicate <LAC,CI> between bts1 and bts2 ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38086?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I7d1ebd529e050ecb4c14b9d3523637e4c8c87e1d Gerrit-Change-Number: 38086 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-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Sep 2024 07:30:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in osmo-bsc[master]: gsm_bts_num(): use hashtable to lookup bts
by laforge 16 Sep '24

16 Sep '24

Attention is currently required from: pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38085?usp=email ) Change subject: gsm_bts_num(): use hashtable to lookup bts ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38085?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I7312da7d9aa80c6d0f2e92e9c7d20d32ce453ad1 Gerrit-Change-Number: 38085 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-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Sep 2024 07:30:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in osmo-bsc[master]: tests/ctrl/osmo-bsc-neigh-test.cfg: Fix duplicate <LAC,CI> between bt...
by laforge 16 Sep '24

16 Sep '24

Attention is currently required from: pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38086?usp=email ) Change subject: tests/ctrl/osmo-bsc-neigh-test.cfg: Fix duplicate <LAC,CI> between bts1 and bts2 ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38086?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I7d1ebd529e050ecb4c14b9d3523637e4c8c87e1d Gerrit-Change-Number: 38086 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-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Sep 2024 07:29:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in osmo-bsc[master]: gsm_bts_num(): use hashtable to lookup bts
by laforge 16 Sep '24

16 Sep '24

Attention is currently required from: pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38085?usp=email ) Change subject: gsm_bts_num(): use hashtable to lookup bts ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/38085/comment/597e8ea1_1cdf6a13?usp… : PS1, Line 990: 6 > I chose 10, since that's 2^10=1024 entries, which for a more or less expected target of ~1k BTS give […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38085?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I7312da7d9aa80c6d0f2e92e9c7d20d32ce453ad1 Gerrit-Change-Number: 38085 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-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Sep 2024 07:29:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-bsc[master]: Introduce hashtable to lookup bts by LAC
by laforge 16 Sep '24

16 Sep '24

Attention is currently required from: fixeria, pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38087?usp=email ) Change subject: Introduce hashtable to lookup bts by LAC ...................................................................... Patch Set 4: Code-Review+1 (1 comment) File src/osmo-bsc/bts_vty.c: https://gerrit.osmocom.org/c/osmo-bsc/+/38087/comment/d3f930c9_8b427fcb?usp… : PS4, Line 281: default > I know about this notation in the VTY. […] I agree this behaviour is a bug. The question is: Is the bug introduced or made more serious by this patch? If yes, then it should be addressed in this patch. If not, the fix should go in a separate patch. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38087?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id523027b49e0f58cd2c8c9b4dee619de415dbd15 Gerrit-Change-Number: 38087 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-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 16 Sep 2024 07:29:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log