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

[XL] Change in osmo-hnbgw[master]: per-HNB GTP-U traffic counters via nft

by pespin

Attention is currently required from: laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email ) Change subject: per-HNB GTP-U traffic counters via nft ...................................................................... Patch Set 12: (8 comments) File include/osmocom/hnbgw/hnbgw.h: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/6665b6ba_668798f4 PS6, Line 402: struct { > yes i mean that too. […] Done File src/osmo-hnbgw/hnbgw_hnbap.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/7411b3df_81f63be8 PS6, Line 571: hnb_persistent_connected(ctx->persistent); > yes, but a hnb can be connected without being registered. (that is for a different redmine ticket) […] It really depends on the specific event you want to track here. I would only use the "connected/disconnected" wording for SCTP/TCP layer here. If you are actually tracking the registered state, then I'd for for "registered" or "active" or alike. Up to you. File src/osmo-hnbgw/hnbgw_vty.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/8d040060_34f92616 PS9, Line 894: vty_out(vty, "%% WARNING: nft configuration changes need a restart of osmo-hnbw%s", VTY_NEWLINE); > hey! i fixed that! Done File src/osmo-hnbgw/nft_kpi.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/c0bb0fc3_540b6ff2 PS6, Line 193: static const char * const nft_thread_req_type_name[] = { > i want to avoid iteration ala value_string, because all callers are entirely constants within this . […] Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/77d602fc_5b43f7d4 PS6, Line 415: static int update_ctr(struct rate_ctr_group *cg, int cgidx, uint64_t *last_val, uint64_t new_val) > kk Done File src/osmo-hnbgw/nft_kpi.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/cbf89028_0749936e PS9, Line 626: /* From here on, until we receive the next NFT_THREAD_GET_COUNTERS in this thread, the > if ever the time it takes to retrieve counters does overlap the wait time configured in timer X34, t […] Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/ee81f850_2fa5823b PS9, Line 700: osmo_timer_setup(&g_hnbgw->nft_kpi.get_counters_timer, nft_kpi_get_counters_cb, NULL); > possible idea for the future: keep interval tracking at the worker, use osmo_timerfd_*() here, this […] Done https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539/comment/bba5ff67_9b66038e PS9, Line 994: nft_kpi_get_counters_schedule(); > i believe this was the result of our discussion: to wait X34 time *between* getting counters. […] I really think we want to at least do as I described, which is really quick to implement (adding one "prev" timestamp and one timestmap subtraction). This way logic conforms much more to what user would expect: If I configure X34 to 10000, I get updated counters every 10 seconds, not every 10+X seconds. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36539?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Ib2f0a9252715ea4b2fe9c367aa65f771357768ca Gerrit-Change-Number: 36539 Gerrit-PatchSet: 12 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-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 23 May 2024 12:24:16 +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

1 year, 1 month

1
0
0 0

[M] Change in osmocom-bb[master]: trxcon/l1sched: refactor prim management in tx_tch[fh]_fn()

by fixeria

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/36915?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: trxcon/l1sched: refactor prim management in tx_tch[fh]_fn() ...................................................................... trxcon/l1sched: refactor prim management in tx_tch[fh]_fn() This change significantly reduces the number of goto statements in these two functions and makes them easier to read/follow. Change-Id: I5292abf6fcd308c9f7f12c7145d004103c9c7675 --- M src/host/trxcon/src/sched_lchan_tchf.c M src/host/trxcon/src/sched_lchan_tchh.c 2 files changed, 82 insertions(+), 69 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/15/36915/2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/36915?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I5292abf6fcd308c9f7f12c7145d004103c9c7675 Gerrit-Change-Number: 36915 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 year, 1 month

1
0
0 0

[M] Change in osmocom-bb[master]: trxcon/l1sched: refactor prim management in tx_tch[fh]_fn()

by fixeria

fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/36915?usp=email ) Change subject: trxcon/l1sched: refactor prim management in tx_tch[fh]_fn() ...................................................................... Set Ready For Review -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/36915?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I5292abf6fcd308c9f7f12c7145d004103c9c7675 Gerrit-Change-Number: 36915 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Thu, 23 May 2024 09:26:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in osmocom-bb[master]: trxcon/l1sched: make l1sched_lchan_emit_data_cnf() NULL-safe

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/36914?usp=email ) Change subject: trxcon/l1sched: make l1sched_lchan_emit_data_cnf() NULL-safe ...................................................................... trxcon/l1sched: make l1sched_lchan_emit_data_cnf() NULL-safe Change-Id: Ie64c176265f66a6c1515c66eb465d7e60f6768db --- M src/host/trxcon/src/sched_prim.c 1 file changed, 11 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/14/36914/1 diff --git a/src/host/trxcon/src/sched_prim.c b/src/host/trxcon/src/sched_prim.c index 67be75e..4974557 100644 --- a/src/host/trxcon/src/sched_prim.c +++ b/src/host/trxcon/src/sched_prim.c @@ -337,7 +337,8 @@ { struct l1sched_prim *prim; - OSMO_ASSERT(msg != NULL); + if (msg == NULL) + return -ENODEV; /* convert from DATA.req to DATA.cnf */ prim = l1sched_prim_from_msgb(msg); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/36914?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ie64c176265f66a6c1515c66eb465d7e60f6768db Gerrit-Change-Number: 36914 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 1 month

1
0
0 0

[M] Change in osmocom-bb[master]: trxcon/l1sched: refactor prim management in tx_tch[fh]_fn()

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/36915?usp=email ) Change subject: trxcon/l1sched: refactor prim management in tx_tch[fh]_fn() ...................................................................... trxcon/l1sched: refactor prim management in tx_tch[fh]_fn() This change significantly reduces the number of goto statements in these two functions and makes them easier to read/follow. Change-Id: I5292abf6fcd308c9f7f12c7145d004103c9c7675 --- M src/host/trxcon/src/sched_lchan_tchf.c M src/host/trxcon/src/sched_lchan_tchh.c 2 files changed, 81 insertions(+), 69 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/15/36915/1 diff --git a/src/host/trxcon/src/sched_lchan_tchf.c b/src/host/trxcon/src/sched_lchan_tchf.c index 7cc1856..36c588c 100644 --- a/src/host/trxcon/src/sched_lchan_tchf.c +++ b/src/host/trxcon/src/sched_lchan_tchf.c @@ -3,7 +3,7 @@ * TDMA scheduler: handlers for DL / UL bursts on logical channels * * (C) 2017-2022 by Vadim Yanitskiy <axilirator(a)gmail.com> - * (C) 2021-2023 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> + * (C) 2021-2024 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> * * All Rights Reserved * @@ -290,39 +290,49 @@ /* fall-through */ case GSM48_CMODE_SPEECH_V1: case GSM48_CMODE_SPEECH_EFR: - if (msg == NULL) { - /* transmit a dummy speech block with inverted CRC3 */ - gsm0503_tch_fr_encode(bursts_p, NULL, 0, 1); - goto send_burst; - } + /* if msg == NULL, transmit a dummy speech block with inverted CRC3 */ rc = gsm0503_tch_fr_encode(BUFPOS(bursts_p, 0), - msgb_l2(msg), - msgb_l2len(msg), 1); + msg ? msgb_l2(msg) : NULL, + msg ? msgb_l2len(msg) : 0, 1); + /* confirm traffic sending (pass ownership of the msgb/prim) */ + if (OSMO_LIKELY(rc == 0)) + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); + else /* unlikely: encoding failed, drop msgb/prim */ + msgb_free(msg); + /* drop the other msgb/prim */ + msgb_free((msg == msg_facch) ? msg_tch : msg_facch); break; case GSM48_CMODE_SPEECH_AMR: { bool amr_fn_is_cmr = !sched_tchf_ul_amr_cmi_map[br->fn % 26]; - const uint8_t *data = msg ? msgb_l2(msg) : NULL; - size_t data_len = msg ? msgb_l2len(msg) : 0; + unsigned int offset = 0; if (msg != NULL && msg != msg_facch) { /* TCH/AFS: speech */ - if (!l1sched_lchan_amr_prim_is_valid(lchan, msg, amr_fn_is_cmr)) - goto free_bad_msg; + if (!l1sched_lchan_amr_prim_is_valid(lchan, msg, amr_fn_is_cmr)) { + msgb_free(msg); + msg_tch = NULL; + msg = NULL; + } /* pull the AMR header - sizeof(struct amr_hdr) */ - data_len -= 2; - data += 2; + offset = 2; } /* if msg == NULL, transmit a dummy speech block with inverted CRC6 */ rc = gsm0503_tch_afs_encode(BUFPOS(bursts_p, 0), - data, data_len, + msg ? msgb_l2(msg) + offset : NULL, + msg ? msgb_l2len(msg) - offset : 0, amr_fn_is_cmr, lchan->amr.codec, lchan->amr.codecs, lchan->amr.ul_ft, lchan->amr.ul_cmr); - if (msg == NULL) - goto send_burst; + /* confirm traffic sending (pass ownership of the msgb/prim) */ + if (OSMO_LIKELY(rc == 0)) + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); + else /* unlikely: encoding failed, drop prim */ + msgb_free(msg); + /* drop the other primitive */ + msgb_free((msg == msg_facch) ? msg_tch : msg_facch); break; } /* CSD (TCH/F14.4): 14.5 kbit/s radio interface rate */ @@ -342,7 +352,7 @@ /* Confirm FACCH sending (pass ownership of the msgb/prim) */ l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); } - goto send_burst; + break; /* CSD (TCH/F9.6): 12.0 kbit/s radio interface rate */ case GSM48_CMODE_DATA_12k0: if ((msg = msg_tch) != NULL) { @@ -360,7 +370,7 @@ /* Confirm FACCH sending (pass ownership of the msgb/prim) */ l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); } - goto send_burst; + break; /* CSD (TCH/F4.8): 6.0 kbit/s radio interface rate */ case GSM48_CMODE_DATA_6k0: if ((msg = msg_tch) != NULL) { @@ -378,42 +388,32 @@ /* Confirm FACCH sending (pass ownership of the msgb/prim) */ l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); } - goto send_burst; + break; /* CSD (TCH/F2.4): 3.6 kbit/s radio interface rate */ case GSM48_CMODE_DATA_3k6: if ((msg = msg_facch) != NULL) { /* FACCH/F does steal a TCH/F2.4 frame completely */ - rc = gsm0503_tch_fr_facch_encode(BUFPOS(bursts_p, 0), msgb_l2(msg)); + gsm0503_tch_fr_facch_encode(BUFPOS(bursts_p, 0), msgb_l2(msg)); + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); } else if ((msg = msg_tch) != NULL) { OSMO_ASSERT(msgb_l2len(msg) == 2 * 36); - rc = gsm0503_tch_fr24_encode(BUFPOS(bursts_p, 0), msgb_l2(msg)); + gsm0503_tch_fr24_encode(BUFPOS(bursts_p, 0), msgb_l2(msg)); + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); } else { ubit_t idle[2 * 36]; memset(&idle[0], 0x01, sizeof(idle)); gsm0503_tch_fr24_encode(BUFPOS(bursts_p, 0), &idle[0]); - goto send_burst; } break; default: LOGP_LCHAND(lchan, LOGL_ERROR, "TCH mode %s is unknown or not supported\n", gsm48_chan_mode_name(lchan->tch_mode)); - goto free_bad_msg; - } - - if (rc) { - LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n", - msgb_l2len(msg), msgb_hexdump_l2(msg)); -free_bad_msg: msgb_free(msg_facch); msgb_free(msg_tch); - return -EINVAL; + break; } - /* Confirm data / traffic sending (pass ownership of the msgb/prim) */ - l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); - msgb_free((msg == msg_facch) ? msg_tch : msg_facch); - send_burst: /* Determine which burst should be sent */ burst = BUFPOS(bursts_p, br->bid); diff --git a/src/host/trxcon/src/sched_lchan_tchh.c b/src/host/trxcon/src/sched_lchan_tchh.c index 6c88d19..d684976 100644 --- a/src/host/trxcon/src/sched_lchan_tchh.c +++ b/src/host/trxcon/src/sched_lchan_tchh.c @@ -4,7 +4,7 @@ * * (C) 2018-2022 by Vadim Yanitskiy <axilirator(a)gmail.com> * (C) 2018 by Harald Welte <laforge(a)gnumonks.org> - * (C) 2020-2023 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> + * (C) 2020-2024 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> * * All Rights Reserved * @@ -497,39 +497,53 @@ msg = l1sched_lchan_prim_dummy_lapdm(lchan); /* fall-through */ case GSM48_CMODE_SPEECH_V1: - if (msg == NULL) { - /* transmit a dummy speech block with inverted CRC3 */ - gsm0503_tch_hr_encode(bursts_p, NULL, 0); - goto send_burst; - } + /* if msg == NULL, transmit a dummy speech block with inverted CRC3 */ rc = gsm0503_tch_hr_encode(BUFPOS(bursts_p, 0), - msgb_l2(msg), - msgb_l2len(msg)); + msg ? msgb_l2(msg) : NULL, + msg ? msgb_l2len(msg) : 0); + /* confirm traffic sending (pass ownership of the msgb/prim) */ + if (OSMO_LIKELY(rc == 0)) { + if (msg && msgb_l2len(msg) == GSM_MACBLOCK_LEN) + lchan->ul_facch_blocks = 6; + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); + } else /* unlikely: encoding failed, drop msgb/prim */ + msgb_free(msg); + /* drop the other msgb/prim */ + msgb_free((msg == msg_facch) ? msg_tch : msg_facch); break; case GSM48_CMODE_SPEECH_AMR: { bool amr_fn_is_cmr = !sched_tchh_ul_amr_cmi_map[br->fn % 26]; - const uint8_t *data = msg ? msgb_l2(msg) : NULL; - size_t data_len = msg ? msgb_l2len(msg) : 0; + unsigned int offset = 0; if (msg != NULL && msg != msg_facch) { /* TCH/AHS: speech */ - if (!l1sched_lchan_amr_prim_is_valid(lchan, msg, amr_fn_is_cmr)) - goto free_bad_msg; + if (!l1sched_lchan_amr_prim_is_valid(lchan, msg, amr_fn_is_cmr)) { + msgb_free(msg); + msg_tch = NULL; + msg = NULL; + } /* pull the AMR header - sizeof(struct amr_hdr) */ - data_len -= 2; - data += 2; + offset = 2; } /* if msg == NULL, transmit a dummy speech block with inverted CRC6 */ rc = gsm0503_tch_ahs_encode(BUFPOS(bursts_p, 0), - data, data_len, + msg ? msgb_l2(msg) + offset : NULL, + msg ? msgb_l2len(msg) - offset : 0, amr_fn_is_cmr, lchan->amr.codec, lchan->amr.codecs, lchan->amr.ul_ft, lchan->amr.ul_cmr); - if (msg == NULL) - goto send_burst; + /* confirm traffic sending (pass ownership of the msgb/prim) */ + if (OSMO_LIKELY(rc == 0)) { + if (msg && msgb_l2len(msg) == GSM_MACBLOCK_LEN) + lchan->ul_facch_blocks = 6; + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); + } else /* unlikely: encoding failed, drop msgb/prim */ + msgb_free(msg); + /* drop the other msgb/prim */ + msgb_free((msg == msg_facch) ? msg_tch : msg_facch); break; } /* CSD (TCH/H4.8): 6.0 kbit/s radio interface rate */ @@ -549,7 +563,7 @@ /* Confirm FACCH sending (pass ownership of the msgb/prim) */ l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); } - goto send_burst; + break; /* CSD (TCH/H2.4): 3.6 kbit/s radio interface rate */ case GSM48_CMODE_DATA_3k6: if ((msg = msg_tch) != NULL) { @@ -567,30 +581,16 @@ /* Confirm FACCH sending (pass ownership of the msgb/prim) */ l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); } - goto send_burst; + break; default: LOGP_LCHAND(lchan, LOGL_ERROR, "TCH mode %s is unknown or not supported\n", gsm48_chan_mode_name(lchan->tch_mode)); - goto free_bad_msg; - } - - if (rc) { - LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n", - msgb_l2len(msg), msgb_hexdump_l2(msg)); -free_bad_msg: msgb_free(msg_facch); msgb_free(msg_tch); - return -EINVAL; + break; } - if (msgb_l2len(msg) == GSM_MACBLOCK_LEN) - lchan->ul_facch_blocks = 6; - - /* Confirm data / traffic sending (pass ownership of the msgb/prim) */ - l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); - msgb_free((msg == msg_facch) ? msg_tch : msg_facch); - send_burst: /* Determine which burst should be sent */ burst = BUFPOS(bursts_p, br->bid); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/36915?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I5292abf6fcd308c9f7f12c7145d004103c9c7675 Gerrit-Change-Number: 36915 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 1 month

1
0
0 0

[S] Change in libosmocore[master]: gsm0808: add knowledge of TW-TS-003 BSSMAP IE

by neels

Attention is currently required from: falconia, fixeria, laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36669?usp=email ) Change subject: gsm0808: add knowledge of TW-TS-003 BSSMAP IE ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: I'm in two minds here; this seems to be a single person effort. On the one hand, an open community wants to allow open use, but on the other hand there are bounds to what amount of API we can hope to maintain. I find this curious description of apparently a single person, I suspect the author of this patch =) "under the authority of the Presiding Sisterhood (government) of the Women's Republic of Themyscira as part of Themyscira Wireless technical initiative." in https://www.freecalypso.org/specs/tw-ts-001-v010100.txt This stuff puts me off -- I want to quickly understand what things are about, and adding noise around the core idea, like mimicking the kludgy phrases often found in 3GPP TS, adding glossaries and whimsical prose doesn't help in that regard... In general I'm critical to "RTP extension". RTP is maximally flexible by allowing *any* payload types to be used, even if they are pure fantasy. Adding custom IEs to RSL is done in osmcom, and its use is managed by the BTS features, seems this patch set is using that properly indeed. Adding custom IEs to BSSMAP is not done in osmocom yet, and we have no way of indicating the BSS' custom capabilities to osmo-msc. How do you solve that? BTW, if we have custom BSSMAP IEs, then I might want to add an IE to use an array of octets to describe all permitted AMR bitrates in a sane way (re: the AMR talk at osmodevcon '24: BSSMAP is a bottleneck in AMR rates negotiation). But there are ample reasons to not deviate from 3GPP if at all possible. Every time we add custom stuff, we need to support, test and maintain both the 3GPP compliant as well as the custom stuff for all eternity... Me personally, I'm a bit reluctant here, because I'm not familiar with the extensions. I would need a good TL;DR to start comprehending. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36669?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Idd6f1838bdc0cd71a2f6357e9b6d8b8fa4f7c46c Gerrit-Change-Number: 36669 Gerrit-PatchSet: 3 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-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: falconia <falcon(a)freecalypso.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 23 May 2024 03:04:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in osmo-hnbgw[master]: add hnb_persistent hashtable: optimize lookup by cell id

by neels

Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36889?usp=email ) Change subject: add hnb_persistent hashtable: optimize lookup by cell id ...................................................................... Patch Set 5: (1 comment) File src/osmo-hnbgw/hnbgw.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/36889/comment/0930460c_579800f2 PS4, Line 245: /* source: http://www.cse.yorku.ca/~oz/hash.html */ > That's one of the reasons I end up using rb trees. […] the next patch following this one switches to the new osmo_jhash(). i left the patch separate so it's easy to manage the new libosmocore dependency. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36889?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Iecb81eba28263ecf90a09c108995f6fb6f5f81f2 Gerrit-Change-Number: 36889 Gerrit-PatchSet: 5 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-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 23 May 2024 02:24:08 +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> Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[L] Change in libosmocore[master]: add jhash.h, copied from linux/jhash.h

by Jenkins Builder

Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email ) Change subject: add jhash.h, copied from linux/jhash.h ...................................................................... Patch Set 1: (2 comments) File tests/jhash/jhash_test.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-16267): https://gerrit.osmocom.org/c/libosmocore/+/36913/comment/83386e5c_e36d6080 PS1, Line 40: hash_for_each(haystack, i, item, node) space required before the open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-gerrit-lint-16267): https://gerrit.osmocom.org/c/libosmocore/+/36913/comment/84cb7273_a7ab3947 PS1, Line 49: hash_for_each_possible(haystack, item, node, hash) space required before the open parenthesis '(' -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa Gerrit-Change-Number: 36913 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Thu, 23 May 2024 02:20:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[L] Change in libosmocore[master]: add jhash.h, copied from linux/jhash.h

by neels

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email ) Change subject: add jhash.h, copied from linux/jhash.h ...................................................................... add jhash.h, copied from linux/jhash.h Allow using arbitrary length data as hashtable key: Copy jhash.h implementation from the linux kernel. Apply osmo_ prefix to all global symbols. Add jhash_test to ensure the code import works as intended. First application will be a hashtable keyed by umts_cell_id in osmo-hnbgw.git. Related: SYS#6773 Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa --- M include/osmocom/core/Makefile.am A include/osmocom/core/jhash.h M tests/Makefile.am A tests/jhash/jhash_test.c A tests/jhash/jhash_test.ok M tests/testsuite.at 6 files changed, 304 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/13/36913/1 diff --git a/include/osmocom/core/Makefile.am b/include/osmocom/core/Makefile.am index 7c29ca1..980f813 100644 --- a/include/osmocom/core/Makefile.am +++ b/include/osmocom/core/Makefile.am @@ -27,6 +27,7 @@ hashtable.h \ isdnhdlc.h \ it_q.h \ + jhash.h \ linuxlist.h \ linuxrbtree.h \ log2.h \ diff --git a/include/osmocom/core/jhash.h b/include/osmocom/core/jhash.h new file mode 100644 index 0000000..a63e930 --- /dev/null +++ b/include/osmocom/core/jhash.h @@ -0,0 +1,191 @@ +#pragma once +#include <osmocom/core/bit32gen.h> + +/* Below is a partial copy of + * https://raw.githubusercontent.com/torvalds/linux/master/tools/include/linux… + * with an osmo_ prefix applied to avoid any collisions. + */ +/* SPDX-License-Identifier: GPL-2.0 */ +/** + * rol32 - rotate a 32-bit value left + * @word: value to rotate + * @shift: bits to roll + */ +static inline uint32_t osmo_rol32(uint32_t word, unsigned int shift) +{ + return (word << shift) | (word >> ((-shift) & 31)); +} + +/* Below is a copy of + * https://raw.githubusercontent.com/torvalds/linux/79e3ea5aab48c83de9410e43b5… + * with an osmo_ prefix applied to avoid any collisions. + * Using osmo_load32le() instead of __get_unaligned_cpu32(). + */ +/* jhash.h: Jenkins hash support. + * + * Copyright (C) 2006. Bob Jenkins (bob_jenkins(a)burtleburtle.net) + * + * https://burtleburtle.net/bob/hash/ + * + * These are the credits from Bob's sources: + * + * lookup3.c, by Bob Jenkins, May 2006, Public Domain. + * + * These are functions for producing 32-bit hashes for hash table lookup. + * hashword(), hashlittle(), hashlittle2(), hashbig(), mix(), and final() + * are externally useful functions. Routines to test the hash are included + * if SELF_TEST is defined. You can use this free for any purpose. It's in + * the public domain. It has no warranty. + * + * Copyright (C) 2009-2010 Jozsef Kadlecsik (kadlec(a)blackhole.kfki.hu) + * + * I've modified Bob's hash to be useful in the Linux kernel, and + * any bugs present are my fault. + * Jozsef + */ + +/* Best hash sizes are of power of two */ +#define osmo_jhash_size(n) ((uint32_t)1<<(n)) +/* Mask the hash value, i.e (value & osmo_jhash_mask(n)) instead of (value % n) */ +#define osmo_jhash_mask(n) (osmo_jhash_size(n)-1) + +/* __osmo_jhash_mix -- mix 3 32-bit values reversibly. */ +#define __osmo_jhash_mix(a, b, c) \ +{ \ + a -= c; a ^= osmo_rol32(c, 4); c += b; \ + b -= a; b ^= osmo_rol32(a, 6); a += c; \ + c -= b; c ^= osmo_rol32(b, 8); b += a; \ + a -= c; a ^= osmo_rol32(c, 16); c += b; \ + b -= a; b ^= osmo_rol32(a, 19); a += c; \ + c -= b; c ^= osmo_rol32(b, 4); b += a; \ +} + +/* __osmo_jhash_final - final mixing of 3 32-bit values (a,b,c) into c */ +#define __osmo_jhash_final(a, b, c) \ +{ \ + c ^= b; c -= osmo_rol32(b, 14); \ + a ^= c; a -= osmo_rol32(c, 11); \ + b ^= a; b -= osmo_rol32(a, 25); \ + c ^= b; c -= osmo_rol32(b, 16); \ + a ^= c; a -= osmo_rol32(c, 4); \ + b ^= a; b -= osmo_rol32(a, 14); \ + c ^= b; c -= osmo_rol32(b, 24); \ +} + +/* An arbitrary initial parameter */ +#define JHASH_INITVAL 0xdeadbeef + +/* osmo_jhash - hash an arbitrary key + * @k: sequence of bytes as key + * @length: the length of the key + * @initval: the previous hash, or an arbitray value + * + * The generic version, hashes an arbitrary sequence of bytes. + * No alignment or length assumptions are made about the input key. + * + * Returns the hash value of the key. The result depends on endianness. + */ +static inline uint32_t osmo_jhash(const void *key, uint32_t length, uint32_t initval) +{ + uint32_t a, b, c; + const uint8_t *k = key; + + /* Set up the internal state */ + a = b = c = JHASH_INITVAL + length + initval; + + /* All but the last block: affect some 32 bits of (a,b,c) */ + while (length > 12) { + a += osmo_load32le(k); + b += osmo_load32le(k + 4); + c += osmo_load32le(k + 8); + __osmo_jhash_mix(a, b, c); + length -= 12; + k += 12; + } + /* Last block: affect all 32 bits of (c) */ + /* All the case statements fall through */ + switch (length) { + case 12: c += (uint32_t)k[11]<<24; + case 11: c += (uint32_t)k[10]<<16; + case 10: c += (uint32_t)k[9]<<8; + case 9: c += k[8]; + case 8: b += (uint32_t)k[7]<<24; + case 7: b += (uint32_t)k[6]<<16; + case 6: b += (uint32_t)k[5]<<8; + case 5: b += k[4]; + case 4: a += (uint32_t)k[3]<<24; + case 3: a += (uint32_t)k[2]<<16; + case 2: a += (uint32_t)k[1]<<8; + case 1: a += k[0]; + __osmo_jhash_final(a, b, c); + case 0: /* Nothing left to add */ + break; + } + + return c; +} + +/* osmo_jhash2 - hash an array of uint32_t's + * @k: the key which must be an array of uint32_t's + * @length: the number of uint32_t's in the key + * @initval: the previous hash, or an arbitray value + * + * Returns the hash value of the key. + */ +static inline uint32_t osmo_jhash2(const uint32_t *k, uint32_t length, uint32_t initval) +{ + uint32_t a, b, c; + + /* Set up the internal state */ + a = b = c = JHASH_INITVAL + (length<<2) + initval; + + /* Handle most of the key */ + while (length > 3) { + a += k[0]; + b += k[1]; + c += k[2]; + __osmo_jhash_mix(a, b, c); + length -= 3; + k += 3; + } + + /* Handle the last 3 uint32_t's: all the case statements fall through */ + switch (length) { + case 3: c += k[2]; + case 2: b += k[1]; + case 1: a += k[0]; + __osmo_jhash_final(a, b, c); + case 0: /* Nothing left to add */ + break; + } + + return c; +} + + +/* __osmo_jhash_nwords - hash exactly 3, 2 or 1 word(s) */ +static inline uint32_t __osmo_jhash_nwords(uint32_t a, uint32_t b, uint32_t c, uint32_t initval) +{ + a += initval; + b += initval; + c += initval; + + __osmo_jhash_final(a, b, c); + + return c; +} + +static inline uint32_t osmo_jhash_3words(uint32_t a, uint32_t b, uint32_t c, uint32_t initval) +{ + return __osmo_jhash_nwords(a, b, c, initval + JHASH_INITVAL + (3 << 2)); +} + +static inline uint32_t osmo_jhash_2words(uint32_t a, uint32_t b, uint32_t initval) +{ + return __osmo_jhash_nwords(a, b, 0, initval + JHASH_INITVAL + (2 << 2)); +} + +static inline uint32_t osmo_jhash_1word(uint32_t a, uint32_t initval) +{ + return __osmo_jhash_nwords(a, 0, 0, initval + JHASH_INITVAL + (1 << 2)); +} diff --git a/tests/Makefile.am b/tests/Makefile.am index 58900d3..1e8997c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -59,6 +59,7 @@ osmo_io/osmo_io_test \ soft_uart/soft_uart_test \ rlp/rlp_test \ + jhash/jhash_test \ $(NULL) if ENABLE_MSGFILE @@ -391,6 +392,7 @@ rlp_rlp_test_SOURCES = rlp/rlp_test.c rlp_rlp_test_LDADD = $(top_builddir)/src/gsm/libosmogsm.la $(LDADD) +jhash_jhash_test_SOURCES = jhash/jhash_test.c # The `:;' works around a Bash 3.2 bug when the output is not writeable. $(srcdir)/package.m4: $(top_srcdir)/configure.ac @@ -505,6 +507,7 @@ soft_uart/soft_uart_test.ok \ rlp/rlp_test.ok \ socket/socket_sctp_test.ok socket/socket_sctp_test.err \ + jhash/jhash_test.ok \ $(NULL) if ENABLE_LIBSCTP @@ -729,8 +732,9 @@ soft_uart/soft_uart_test \ >$(srcdir)/soft_uart/soft_uart_test.ok rlp/rlp_test \ - >$(srcdir)/rlp/rlp_test.ok \ - $(NULL) + >$(srcdir)/rlp/rlp_test.ok + jhash/jhash_test \ + >$(srcdir)/jhash/jhash_test.ok check-local: atconfig $(TESTSUITE) diff --git a/tests/jhash/jhash_test.c b/tests/jhash/jhash_test.c new file mode 100644 index 0000000..da498c8 --- /dev/null +++ b/tests/jhash/jhash_test.c @@ -0,0 +1,56 @@ +#include <osmocom/core/linuxlist.h> +#include <osmocom/core/hashtable.h> +#include <osmocom/core/jhash.h> + +struct item { + const char blob[32]; + struct hlist_node node; +}; + +struct item items[] = { + { "blob one", }, + { "blob two and five are the same", }, + { "third blob", }, + { "fourth blob", }, + { "blob two and five are the same", }, +}; + +uint32_t item_hash(const struct item *item) +{ + return osmo_jhash(item->blob, strlen(item->blob), 0); +} + +int main(void) +{ + int i; + struct item *item; + + DECLARE_HASHTABLE(haystack, 5); + hash_init(haystack); + + printf("add:\n"); + for (i = 0; i < ARRAY_SIZE(items); i++) { + uint32_t hash; + item = &items[i]; + hash_add(haystack, &item->node, hash = item_hash(item)); + printf("- adding items[%d]#%x = %s\n", i, hash, item->blob); + } + + printf("list:\n"); + hash_for_each(haystack, i, item, node) + printf("- %s [%d]\n", item->blob, (int)(item - items)); + + printf("find:\n"); + for (i = 0; i < ARRAY_SIZE(items); i++) { + uint32_t hash; + struct item *needle = &items[i]; + hash = item_hash(needle); + printf("- looking up items[%d]#%x = %s\n", i, hash, needle->blob); + hash_for_each_possible(haystack, item, node, hash) + printf(" - %s items[%d]\n", + (item == needle) ? "found" : "not", + (int)(item - items)); + } + + return 0; +} diff --git a/tests/jhash/jhash_test.ok b/tests/jhash/jhash_test.ok new file mode 100644 index 0000000..e28ccc4 --- /dev/null +++ b/tests/jhash/jhash_test.ok @@ -0,0 +1,25 @@ +add: +- adding items[0]#b286bb61 = blob one +- adding items[1]#ad4eb7b = blob two and five are the same +- adding items[2]#6b8eae61 = third blob +- adding items[3]#24a546dc = fourth blob +- adding items[4]#ad4eb7b = blob two and five are the same +list: +- blob two and five are the same [4] +- blob two and five are the same [1] +- blob one [0] +- fourth blob [3] +- third blob [2] +find: +- looking up items[0]#b286bb61 = blob one + - found items[0] +- looking up items[1]#ad4eb7b = blob two and five are the same + - not items[4] + - found items[1] +- looking up items[2]#6b8eae61 = third blob + - found items[2] +- looking up items[3]#24a546dc = fourth blob + - found items[3] +- looking up items[4]#ad4eb7b = blob two and five are the same + - found items[4] + - not items[1] diff --git a/tests/testsuite.at b/tests/testsuite.at index e721b93..3433fb0 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -547,3 +547,9 @@ cat $abs_srcdir/rlp/rlp_test.ok > expout AT_CHECK([$abs_top_builddir/tests/rlp/rlp_test], [0], [expout], [ignore]) AT_CLEANUP + +AT_SETUP([jhash]) +AT_KEYWORDS([jhash]) +cat $abs_srcdir/jhash/jhash_test.ok > expout +AT_CHECK([$abs_top_builddir/tests/jhash/jhash_test], [0], [expout], [ignore]) +AT_CLEANUP -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36913?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa Gerrit-Change-Number: 36913 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 1 month

1
0
0 0

[S] Change in osmo-hnbgw[master]: use osmo_jhash for the hnb_persistent hashtable

by neels

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36912?usp=email ) Change subject: use osmo_jhash for the hnb_persistent hashtable ...................................................................... use osmo_jhash for the hnb_persistent hashtable Use hashing like the linux kernel. Related: SYS#6773 Depends: I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa Change-Id: I5441db4293dc6b57a1c606ef830656fa9fa01943 --- M TODO-RELEASE M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/hnbgw_hnbap.c 3 files changed, 27 insertions(+), 22 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/12/36912/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index b0ad6a0..a1206c1 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -12,3 +12,4 @@ MGW MGCP CRCX osmo-hnbgw used to CRCX in loopback mode, to trigger a legacy IuUP hack. CRCX is no longer in loopback mode now, so older osmo-mgw may fail to respond to IuUP Initialization. osmo-iuh >1.5.1 decoding of more RANAP procedures in ranap_common required for DTAP rate_counters +libosmocore >1.9.0 require osmo_jhash from osmocom/core/jhash.h added in I0c9652bbc9e2a18b1200e7d63bb6f64ded7d75fa diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index e6073de..86ad1c0 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -25,6 +25,7 @@ #include <osmocom/core/stats.h> #include <osmocom/core/rate_ctr.h> #include <osmocom/core/stat_item.h> +#include <osmocom/core/jhash.h> #include <osmocom/vty/vty.h> @@ -242,36 +243,22 @@ ucid->sac, ucid->cid); } -/* source: http://www.cse.yorku.ca/~oz/hash.html */ -static inline void mkhash_init(uint32_t *hash) -{ - *hash = 5381; -} -static inline void mkhash_add(uint32_t *hash, int32_t val) -{ - uint32_t h = *hash; - h = ((h << 5) + h) ^ val; /* (h * 33) ^ val */ - *hash = h; -} - /* Useful to index a hash table by struct umts_cell_id. */ uint32_t umts_cell_id_hash(const struct umts_cell_id *ucid) { - uint32_t hash; - mkhash_init(&hash); - mkhash_add(&hash, ucid->mcc); - mkhash_add(&hash, ucid->mnc); - mkhash_add(&hash, ucid->lac); - mkhash_add(&hash, ucid->rac); - mkhash_add(&hash, ucid->sac); - mkhash_add(&hash, ucid->cid); - return hash; + return osmo_jhash(ucid, sizeof(*ucid), 0x423423); } /* parse a string representation of an umts_cell_id into its decoded representation */ int umts_cell_id_from_str(struct umts_cell_id *ucid, const char *instr) { - int rc = sscanf(instr, "%hu-%hu-L%hu-R%hu-S%hu-C%u", &ucid->mcc, &ucid->mnc, &ucid->lac, &ucid->rac, &ucid->sac, &ucid->cid); + int rc; + + /* We want to use struct umts_cell_id as hashtable key. If it ever happens to contain any padding bytes, make + * sure everything is deterministically zero. */ + memset(ucid, 0, sizeof(*ucid)); + + rc = sscanf(instr, "%hu-%hu-L%hu-R%hu-S%hu-C%u", &ucid->mcc, &ucid->mnc, &ucid->lac, &ucid->rac, &ucid->sac, &ucid->cid); if (rc < 0) return -errno; diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c index e663196..8b823a4 100644 --- a/src/osmo-hnbgw/hnbgw_hnbap.c +++ b/src/osmo-hnbgw/hnbgw_hnbap.c @@ -492,6 +492,10 @@ /* copy all identity parameters from the message to ctx */ OSMO_STRLCPY_ARRAY(ctx->identity_info, identity_str); + + /* We want to use struct umts_cell_id as hashtable key. If it ever happens to contain any padding bytes, make + * sure everything is deterministically zero. */ + memset(&ctx->id, 0, sizeof(ctx->id)); ctx->id.lac = asn1str_to_u16(&ies.lac); ctx->id.sac = asn1str_to_u16(&ies.sac); ctx->id.rac = asn1str_to_u8(&ies.rac); -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36912?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I5441db4293dc6b57a1c606ef830656fa9fa01943 Gerrit-Change-Number: 36912 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 1 month

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2024