- gerrit-log - lists.osmocom.org

[S] Change in libosmo-abis[master]: rtp2trau_hr16: fix UFE bit setting
by falconia 10 Jun '24

10 Jun '24

falconia has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-abis/+/37148?usp=email ) Change subject: rtp2trau_hr16: fix UFE bit setting ...................................................................... rtp2trau_hr16: fix UFE bit setting Per TS 48.061 section 5.1.4.1.1, UFE=1 means UL was good and UFE=0 means UL was bad. In the absence of logic that reports the actual state of UL, we need to tell the BTS that UL was good (like we do for FR & EFR) - hence set UFE=1. Change-Id: Ibeb8db332f2e20806bebb077844a509064b2d89d --- M src/trau/trau_rtp_conv.c 1 file changed, 15 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/48/37148/1 diff --git a/src/trau/trau_rtp_conv.c b/src/trau/trau_rtp_conv.c index 5c87c46..c70736d 100644 --- a/src/trau/trau_rtp_conv.c +++ b/src/trau/trau_rtp_conv.c @@ -407,7 +407,7 @@ tf->c_bits[15] = 0; /* C16: SP */ tf->c_bits[16] = 0; /* C17: DTXd shall not be applied */ } else { - tf->c_bits[11] = 0; /* C12: UFE */ + tf->c_bits[11] = 1; /* C12: UFE */ tf->c_bits[12] = 1; /* C13: spare */ tf->c_bits[13] = 1; /* C14: spare */ tf->c_bits[14] = 1; /* C15: spare */ -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/37148?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Ibeb8db332f2e20806bebb077844a509064b2d89d Gerrit-Change-Number: 37148 Gerrit-PatchSet: 1 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-MessageType: newchange

1 0

[M] Change in osmo-hnbgw[master]: hnb_persistent: introduce disconnected timeout
by neels 10 Jun '24

10 Jun '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36894?usp=email ) Change subject: hnb_persistent: introduce disconnected timeout ...................................................................... hnb_persistent: introduce disconnected timeout Add timer hnbgw X35: Clean up all hNodeB persistent state after this time of the hNodeB being disconnected. Set to zero to never clear hNodeB persistent state. (default is 60*60*24*27 = a week). The idea is to bound memory usage at sites where hNodeB do not stay on one fixed cell id, but use a different cell id after each Iuh reconnection. Related: SYS#6773 Related: osmo-ttcn3-hacks Ibec009203d38f65714561b7c28edbdbd8b34e704 Change-Id: Ic819d7cbc03fb39e98c204b70d016c5170dc6307 --- M include/osmocom/hnbgw/hnbgw.h M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/tdefs.c 3 files changed, 58 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved osmith: Looks good to me, but someone else must approve diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index c28f8bc..36358b1 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -412,6 +412,8 @@ struct nft_kpi_val v; } dl; } nft_kpi; + + struct osmo_timer_list disconnected_timeout; }; struct ue_context { diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 20d6aa6..6f74d04 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -44,6 +44,7 @@ #include <osmocom/hnbgw/hnbgw_cn.h> #include <osmocom/hnbgw/context_map.h> #include <osmocom/hnbgw/mgw_fsm.h> +#include <osmocom/hnbgw/tdefs.h> struct hnbgw *g_hnbgw = NULL; @@ -560,6 +561,24 @@ .item_desc = hnb_stat_desc, }; +static void hnb_persistent_disconnected_timeout_cb(void *data) +{ + hnb_persistent_free(data); +} + +static void hnb_persistent_disconnected_timeout_schedule(struct hnb_persistent *hnbp) +{ + unsigned long period_s = osmo_tdef_get(hnbgw_T_defs, -35, OSMO_TDEF_S, 60*60*24*7); + if (period_s < 1) { + LOG_HNBP(hnbp, LOGL_INFO, + "timer X35 is zero, not setting a disconnected timeout for this hnb-persistent instance.\n"); + return; + } + /* It is fine if the timer is already active, osmo_timer_del() is done implicitly by the osmo_timer API. */ + osmo_timer_setup(&hnbp->disconnected_timeout, hnb_persistent_disconnected_timeout_cb, hnbp); + osmo_timer_schedule(&hnbp->disconnected_timeout, period_s, 0); +} + struct hnb_persistent *hnb_persistent_alloc(const struct umts_cell_id *id) { struct hnb_persistent *hnbp = talloc_zero(g_hnbgw, struct hnb_persistent); @@ -583,6 +602,11 @@ if (g_hnbgw->nft_kpi.active) nft_kpi_hnb_persistent_add(hnbp); + /* Normally the disconnected timer runs only when the hNodeB is not currently connected on Iuh. This here is paranoia: + * In case we have to HNBAP HNB Register Reject, the disconnected timer should be active on this unused hnbp. + * On success, hnb_persistent_registered() will stop the disconnected timer directly after this. */ + hnb_persistent_disconnected_timeout_schedule(hnbp); + return hnbp; out_free_ctrs: @@ -645,6 +669,9 @@ return; } + /* The hNodeB is now connected, i.e. not disconnected. */ + osmo_timer_del(&hnbp->disconnected_timeout); + /* start counting traffic */ if (g_hnbgw->nft_kpi.active) hnb_persistent_update_remote_addr(hnbp); @@ -664,11 +691,15 @@ /* stop counting traffic */ nft_kpi_hnb_stop(hnbp); + + /* The hNodeB is now disconnected. Clear out hnb_persistent when the disconnected timeout has passed. */ + hnb_persistent_disconnected_timeout_schedule(hnbp); } void hnb_persistent_free(struct hnb_persistent *hnbp) { /* FIXME: check if in use? */ + osmo_timer_del(&hnbp->disconnected_timeout); nft_kpi_hnb_stop(hnbp); nft_kpi_hnb_persistent_remove(hnbp); osmo_stat_item_group_free(hnbp->statg); diff --git a/src/osmo-hnbgw/tdefs.c b/src/osmo-hnbgw/tdefs.c index cfcd4c2..5092922 100644 --- a/src/osmo-hnbgw/tdefs.c +++ b/src/osmo-hnbgw/tdefs.c @@ -36,6 +36,12 @@ {.T = 4, .default_val = 5, .desc = "Timeout to receive RANAP RESET ACKNOWLEDGE from an MSC/SGSN" }, {.T = -31, .default_val = 15, .desc = "Timeout for establishing and releasing context maps (RUA <-> SCCP)" }, {.T = -34, .default_val = 1000, .unit = OSMO_TDEF_MS, .desc = "Period to query network traffic stats from netfilter" }, + { + .T = -35, + .default_val = 60*60*24*7, + .desc = "Clean up all hNodeB persistent state after this time of the hNodeB being disconnected." + " Set to zero to never clear hNodeB persistent state. (default is 60*60*24*27 = a week)", + }, {.T = -1002, .default_val = 10, .desc = "Timeout for the HNB to respond to PS RAB Assignment Request" }, { } }; -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36894?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: Ic819d7cbc03fb39e98c204b70d016c5170dc6307 Gerrit-Change-Number: 36894 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-hnbgw[master]: hnb_persistent: introduce disconnected timeout
by neels 10 Jun '24

10 Jun '24

neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36894?usp=email ) Change subject: hnb_persistent: introduce disconnected timeout ...................................................................... Patch Set 3: -Code-Review (2 comments) Patchset: PS3: I figured out that segfault, it is unrelated to this patch. PS3: > let's delay this a bit until https://osmocom. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36894?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: Ic819d7cbc03fb39e98c204b70d016c5170dc6307 Gerrit-Change-Number: 36894 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Mon, 10 Jun 2024 01:15:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-hnbgw[master]: prevent use-after-free after FSM instance termination
by neels 10 Jun '24

10 Jun '24

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37147?usp=email ) Change subject: prevent use-after-free after FSM instance termination ...................................................................... prevent use-after-free after FSM instance termination - Set osmo_fsm_set_dealloc_ctx(OTC_SELECT) in osmo-hnbgw's main(). - Only dispatch RANAP when FSM instances aren't terminated. This way we possibly pre-empt use-after-free crashes for deallocating FSM "nests" for obscure corner cases. Use-after-free is a general problem for FSM design. For this, we created osmo_fsm_set_dealloc_ctx(): When an FSM is terminated, move it to a separate talloc context, instead of being deallocated. An actual use-after-free was observed as described in OS#6484, but that needs a separate, orthogonal fix: When the Iuh link is lost while the CN link is waiting for SCCP CC or CREF -- the better solution is described in OS#6085: don't wait for CC at all, just dispatch DISCONN to SCCP-SCOC. So even though the code where a crash was observed will be removed, this patch is a general safeguard against corner case crashes, improving general stability. Related: OS#6484 Change-Id: Ib41e1a996aaa03221e73643636140947ac8f99e2 --- M src/osmo-hnbgw/context_map_rua.c M src/osmo-hnbgw/context_map_sccp.c M src/osmo-hnbgw/osmo_hnbgw_main.c 3 files changed, 42 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/47/37147/1 diff --git a/src/osmo-hnbgw/context_map_rua.c b/src/osmo-hnbgw/context_map_rua.c index 3c6ea1e..7286906 100644 --- a/src/osmo-hnbgw/context_map_rua.c +++ b/src/osmo-hnbgw/context_map_rua.c @@ -158,6 +158,11 @@ static int handle_rx_rua(struct osmo_fsm_inst *fi, struct msgb *ranap_msg) { struct hnbgw_context_map *map = fi->priv; + + /* If the FSM instance has already terminated, don't dispatch anything. */ + if (fi->proc.terminating) + return 0; + if (!msg_has_l2_data(ranap_msg)) return 0; diff --git a/src/osmo-hnbgw/context_map_sccp.c b/src/osmo-hnbgw/context_map_sccp.c index 10ddaf2..511577c 100644 --- a/src/osmo-hnbgw/context_map_sccp.c +++ b/src/osmo-hnbgw/context_map_sccp.c @@ -198,6 +198,10 @@ struct hnbgw_context_map *map = fi->priv; int rc; + /* If the FSM instance has already terminated, don't dispatch anything. */ + if (fi->proc.terminating) + return 0; + /* When there was no message received along with the received event, then there is nothing to forward to RUA. */ if (!msg_has_l2_data(ranap_msg)) return 0; diff --git a/src/osmo-hnbgw/osmo_hnbgw_main.c b/src/osmo-hnbgw/osmo_hnbgw_main.c index 2f71251..3e715b0 100644 --- a/src/osmo-hnbgw/osmo_hnbgw_main.c +++ b/src/osmo-hnbgw/osmo_hnbgw_main.c @@ -360,6 +360,8 @@ signal(SIGUSR2, &signal_handler); osmo_init_ignore_signals(); + osmo_fsm_set_dealloc_ctx(OTC_SELECT); + while (1) { rc = osmo_select_main_ctx(0); if (rc < 0) -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37147?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: Ib41e1a996aaa03221e73643636140947ac8f99e2 Gerrit-Change-Number: 37147 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-hnbgw[master]: add LOG_HNBP() for hnb_persistent
by neels 10 Jun '24

10 Jun '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37075?usp=email ) Change subject: add LOG_HNBP() for hnb_persistent ...................................................................... add LOG_HNBP() for hnb_persistent Add macro for the pattern LOGP(DHNB, ..., "%s: ...", hnbp->id_str) and use where applicable. Change-Id: I23334124074491fb3b91e720b67c97181c16bc21 --- M include/osmocom/hnbgw/hnbgw.h M src/osmo-hnbgw/hnbgw.c 2 files changed, 23 insertions(+), 7 deletions(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index 9b07584..c28f8bc 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -41,6 +41,12 @@ #define LOGHNB(HNB_CTX, ss, lvl, fmt, args ...) \ LOGP(ss, lvl, "(%s) " fmt, hnb_context_name(HNB_CTX), ## args) +#define LOG_HNBP(HNBP, lvl, fmt, args...) \ + LOGP(DHNB, lvl, "(%s) " fmt, \ + (HNBP) ? \ + (((HNBP)->id_str && *(HNBP)->id_str) ? (HNBP)->id_str : "no-cell-id") \ + : "null", ## args) + #define DOMAIN_CS RANAP_CN_DomainIndicator_cs_domain #define DOMAIN_PS RANAP_CN_DomainIndicator_ps_domain diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 20a665e..20d6aa6 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -613,19 +613,17 @@ fd = osmo_stream_srv_get_fd(hnbp->ctx->conn); if (fd < 0) { - LOGP(DHNB, LOGL_ERROR, "%s: no active socket fd, cannot set up traffic counters\n", hnbp->id_str); + LOG_HNBP(hnbp, LOGL_ERROR, "no active socket fd, cannot set up traffic counters\n"); return; } socklen = sizeof(struct osmo_sockaddr); if (getpeername(fd, &osa.u.sa, &socklen)) { - LOGP(DHNB, LOGL_ERROR, "%s: cannot read remote address, cannot set up traffic counters\n", - hnbp->id_str); + LOG_HNBP(hnbp, LOGL_ERROR, "cannot read remote address, cannot set up traffic counters\n"); return; } if (osmo_sockaddr_str_from_osa(&remote_str, &osa)) { - LOGP(DHNB, LOGL_ERROR, "%s: cannot parse remote address, cannot set up traffic counters\n", - hnbp->id_str); + LOG_HNBP(hnbp, LOGL_ERROR, "cannot parse remote address, cannot set up traffic counters\n"); return; } @@ -634,7 +632,7 @@ remote_str.port = 2152; if (nft_kpi_hnb_start(hnbp, &remote_str)) - LOGP(DHNB, LOGL_ERROR, "%s: failed to set up traffic counters\n", hnbp->id_str); + LOG_HNBP(hnbp, LOGL_ERROR, "failed to set up traffic counters\n"); } /* Whenever HNBAP registers a HNB, hnbgw_hnbap.c calls this function to let the hnb_persistent update its state to the @@ -643,7 +641,7 @@ void hnb_persistent_registered(struct hnb_persistent *hnbp) { if (!hnbp->ctx) { - LOGP(DHNB, LOGL_ERROR, "hnb_persistent_registered() invoked, but there is no hnb_ctx\n"); + LOG_HNBP(hnbp, LOGL_ERROR, "hnb_persistent_registered() invoked, but there is no hnb_ctx\n"); return; } -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37075?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: I23334124074491fb3b91e720b67c97181c16bc21 Gerrit-Change-Number: 37075 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-hnbgw[master]: umts_cell_id: add umts_cell_id_to_str_buf()
by neels 10 Jun '24

10 Jun '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36891?usp=email ) Change subject: umts_cell_id: add umts_cell_id_to_str_buf() ...................................................................... umts_cell_id: add umts_cell_id_to_str_buf() Prepare for umts_cell_id_test.c: allow testing the to_str conversion without using talloc. Change-Id: I3cc0169593c73c2e658637e4116ddd578f83df6d --- M include/osmocom/hnbgw/hnbgw.h M src/osmo-hnbgw/hnbgw.c 2 files changed, 26 insertions(+), 3 deletions(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index 2c7e014..87844f4 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -173,6 +173,8 @@ uint16_t sac; /*!< Service Area Code */ uint32_t cid; /*!< Cell ID */ }; +int umts_cell_id_to_str_buf(char *buf, size_t buflen, const struct umts_cell_id *ucid); +char *umts_cell_id_to_str_c(void *ctx, const struct umts_cell_id *ucid); const char *umts_cell_id_to_str(const struct umts_cell_id *ucid); int umts_cell_id_from_str(struct umts_cell_id *ucid, const char *instr); uint32_t umts_cell_id_hash(const struct umts_cell_id *ucid); diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index c41fb54..4f3fafa 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -232,15 +232,24 @@ return ctx; } -const char *umts_cell_id_to_str(const struct umts_cell_id *ucid) +int umts_cell_id_to_str_buf(char *buf, size_t buflen, const struct umts_cell_id *ucid) { const char *fmtstr = "%03u-%02u-L%u-R%u-S%u-C%u"; if (g_hnbgw->config.plmn.mnc_3_digits) fmtstr = "%03u-%03u-L%u-R%u-S%u-C%u"; + return snprintf(buf, buflen, fmtstr, ucid->mcc, ucid->mnc, ucid->lac, ucid->rac, + ucid->sac, ucid->cid); +} - return talloc_asprintf(OTC_SELECT, fmtstr, ucid->mcc, ucid->mnc, ucid->lac, ucid->rac, - ucid->sac, ucid->cid); +char *umts_cell_id_to_str_c(void *ctx, const struct umts_cell_id *ucid) +{ + OSMO_NAME_C_IMPL(ctx, 64, "ERROR", umts_cell_id_to_str_buf, ucid) +} + +const char *umts_cell_id_to_str(const struct umts_cell_id *ucid) +{ + return umts_cell_id_to_str_c(OTC_SELECT, ucid); } /* Useful to index a hash table by struct umts_cell_id. */ -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36891?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: I3cc0169593c73c2e658637e4116ddd578f83df6d Gerrit-Change-Number: 36891 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[L] Change in osmo-hnbgw[master]: add umts_cell_id_test.c
by neels 10 Jun '24

10 Jun '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36892?usp=email ) Change subject: add umts_cell_id_test.c ...................................................................... add umts_cell_id_test.c Prepare for adding proper mnc_3_digits support to struct umts_cell_id. Show current behavior of the umts_cell_id <-> string conversions. Show two expected errors in umts_cell_id_test.ok: the three-digit MNC with leading zeros is lost (because the g_hnbgw->config.plmn has mnc_3_digits == false). The expected errors will be fixed in upcoming patch Id9a91c80cd2745424a916aef4736993bb7cd8ba0 Related: SYS#6773 Change-Id: Ibbb61a2c53a11dea794f451d3074bc9ba50862fe --- M configure.ac M tests/Makefile.am M tests/testsuite.at A tests/umts_cell_id/Makefile.am A tests/umts_cell_id/umts_cell_id_test.c A tests/umts_cell_id/umts_cell_id_test.ok 6 files changed, 252 insertions(+), 1 deletion(-) Approvals: fixeria: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/configure.ac b/configure.ac index 48842db..c81d2b3 100644 --- a/configure.ac +++ b/configure.ac @@ -240,6 +240,7 @@ tests/Makefile tests/atlocal tests/ranap_rab_ass/Makefile + tests/umts_cell_id/Makefile doc/Makefile doc/examples/Makefile doc/manuals/Makefile diff --git a/tests/Makefile.am b/tests/Makefile.am index df9e5a9..a4dcf4c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,5 +1,6 @@ SUBDIRS = \ ranap_rab_ass \ + umts_cell_id \ $(NULL) # The `:;' works around a Bash 3.2 bug when the output is not writeable. diff --git a/tests/testsuite.at b/tests/testsuite.at index 84d85cf..c54521a 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -5,4 +5,10 @@ AT_KEYWORDS([ranap_rab_ass]) cat $abs_srcdir/ranap_rab_ass/ranap_rab_ass_test.ok > expout AT_CHECK([$abs_top_builddir/tests/ranap_rab_ass/ranap_rab_ass_test], [0], [expout], [ignore]) -AT_CLEANUP \ No newline at end of file +AT_CLEANUP + +AT_SETUP([umts_cell_id]) +AT_KEYWORDS([umts_cell_id]) +cat $abs_srcdir/umts_cell_id/umts_cell_id_test.ok > expout +AT_CHECK([$abs_top_builddir/tests/umts_cell_id/umts_cell_id_test], [0], [expout], [ignore]) +AT_CLEANUP diff --git a/tests/umts_cell_id/Makefile.am b/tests/umts_cell_id/Makefile.am new file mode 100644 index 0000000..d32fa25 --- /dev/null +++ b/tests/umts_cell_id/Makefile.am @@ -0,0 +1,38 @@ +AM_CPPFLAGS = \ + $(all_includes) \ + -I$(top_srcdir)/include \ + $(NULL) + +AM_CFLAGS = \ + -Wall \ + -ggdb3 \ + $(LIBASN1C_CFLAGS) \ + $(LIBOSMOCORE_CFLAGS) \ + $(LIBOSMOVTY_CFLAGS) \ + $(LIBOSMORANAP_CFLAGS) \ + $(LIBOSMOSIGTRAN_CFLAGS) \ + $(LIBOSMOMGCPCLIENT_CFLAGS) \ + $(COVERAGE_CFLAGS) \ + $(NULL) + +AM_LDFLAGS = -no-install + +EXTRA_DIST = \ + umts_cell_id_test.ok \ + $(NULL) + +check_PROGRAMS = \ + umts_cell_id_test \ + $(NULL) + +umts_cell_id_test_SOURCES = \ + umts_cell_id_test.c \ + $(NULL) + +umts_cell_id_test_LDADD = \ + $(top_builddir)/src/osmo-hnbgw/libhnbgw.la \ + $(NULL) + +.PHONY: update_exp +update_exp: + $(builddir)/umts_cell_id_test >$(srcdir)/umts_cell_id_test.ok diff --git a/tests/umts_cell_id/umts_cell_id_test.c b/tests/umts_cell_id/umts_cell_id_test.c new file mode 100644 index 0000000..e9183c4 --- /dev/null +++ b/tests/umts_cell_id/umts_cell_id_test.c @@ -0,0 +1,151 @@ +#include <stdio.h> + +#include <osmocom/hnbgw/hnbgw.h> + +struct test { + const char *id_str; + int expect_rc; + struct umts_cell_id id; +}; + +struct test tests[] = { + { + .id_str = "001-01-L1-R1-S1-C1", + .id = { + .mcc = 1, + .mnc = 1, + .lac = 1, + .rac = 1, + .sac = 1, + .cid = 1, + }, + }, + + /* ensure that a 3-digit MNC with leading zeroes is kept separate from two-digit MNC */ + { + .id_str = "001-001-L1-R1-S1-C1", + .id = { + .mcc = 1, + .mnc = 1, + .lac = 1, + .rac = 1, + .sac = 1, + .cid = 1, + }, + }, + { + .id_str = "001-099-L1-R1-S1-C1", + .id = { + .mcc = 1, + .mnc = 99, + .lac = 1, + .rac = 1, + .sac = 1, + .cid = 1, + }, + }, + { + .id_str = "001-99-L1-R1-S1-C1", + .id = { + .mcc = 1, + .mnc = 99, + .lac = 1, + .rac = 1, + .sac = 1, + .cid = 1, + }, + }, + + { + .id_str = "999-999-L65534-R65535-S65535-C268435455", + .id = { + .mcc = 999, + .mnc = 999, + .lac = 65534, + .rac = 65535, + .sac = 65535, + .cid = (1 << 28) - 1, + }, + }, + + { + .id_str = "1000-001-L1-R1-S1-C1", + .expect_rc = -EINVAL, + }, + { + .id_str = "001-001-L65535-R1-S1-C1", + .expect_rc = -EINVAL, + }, + /* TODO? There is no bounds checking on RAC and SAC. + { + .id_str = "001-001-L1-R65536-S1-C1", + .expect_rc = -EINVAL, + }, + { + .id_str = "001-001-L1-R1-S65536-C1", + .expect_rc = -EINVAL, + }, + */ + { + .id_str = "001-001-L1-R1-S1-C268435456", + .expect_rc = -EINVAL, + }, +}; + +int main(void) +{ + struct hnbgw hnbgw_dummy = {}; + struct test *t; + + /* umts_cell_id_to_str() accesses g_hnbgw->config.plmn.mnc_3_digits, so make sure it is valid mem: */ + g_hnbgw = &hnbgw_dummy; + + for (t = tests; (t - tests) < ARRAY_SIZE(tests); t++) { + int rc; + struct umts_cell_id parsed; + char to_str[128] = {}; + + printf("\"%s\"\n", t->id_str); + + memset(&parsed, 0x2b, sizeof(parsed)); + rc = umts_cell_id_from_str(&parsed, t->id_str); + if (rc != t->expect_rc) { + printf(" ERROR: umts_cell_id_from_str(): expected rc == %d, got %d\n", + t->expect_rc, rc); + continue; + } + + if (rc) { + if (rc == t->expect_rc) + printf(" expected rc != 0: ok\n"); + continue; + } + printf(" -> umts_cell_id_from_str(): ok\n"); + + rc = umts_cell_id_to_str_buf(to_str, sizeof(to_str), &parsed); + if (rc <= 0) { + printf(" ERROR: umts_cell_id_to_str_buf(): expected rc == 0, got %d\n", rc); + continue; + } else { + printf(" -> umts_cell_id_to_str_buf(): ok\n"); + + if (strcmp(t->id_str, to_str)) + printf(" ERROR: conversion to umts_cell_id and back to string doesn't return the original string\n"); + printf(" -> \"%s\"\n", to_str); + } + + if (umts_cell_id_equal(&t->id, &parsed)) { + printf(" umts_cell_id_equal(expected, parsed): ok\n"); + } else { + char to_str_expect[128] = {}; + umts_cell_id_to_str_buf(to_str_expect, sizeof(to_str_expect), &t->id); + printf(" ERROR: umts_cell_id_equal(expected, parsed) == false\n"); + printf(" expected %s\n", to_str_expect); + printf(" got %s\n", to_str); + printf(" expected %s\n", osmo_hexdump((void *)&t->id, sizeof(t->id))); + printf(" got %s\n", osmo_hexdump((void *)&parsed, sizeof(t->id))); + } + } + + return 0; +} diff --git a/tests/umts_cell_id/umts_cell_id_test.ok b/tests/umts_cell_id/umts_cell_id_test.ok new file mode 100644 index 0000000..457f78a --- /dev/null +++ b/tests/umts_cell_id/umts_cell_id_test.ok @@ -0,0 +1,33 @@ +"001-01-L1-R1-S1-C1" + -> umts_cell_id_from_str(): ok + -> umts_cell_id_to_str_buf(): ok + -> "001-01-L1-R1-S1-C1" + umts_cell_id_equal(expected, parsed): ok +"001-001-L1-R1-S1-C1" + -> umts_cell_id_from_str(): ok + -> umts_cell_id_to_str_buf(): ok + ERROR: conversion to umts_cell_id and back to string doesn't return the original string + -> "001-01-L1-R1-S1-C1" + umts_cell_id_equal(expected, parsed): ok +"001-099-L1-R1-S1-C1" + -> umts_cell_id_from_str(): ok + -> umts_cell_id_to_str_buf(): ok + ERROR: conversion to umts_cell_id and back to string doesn't return the original string + -> "001-99-L1-R1-S1-C1" + umts_cell_id_equal(expected, parsed): ok +"001-99-L1-R1-S1-C1" + -> umts_cell_id_from_str(): ok + -> umts_cell_id_to_str_buf(): ok + -> "001-99-L1-R1-S1-C1" + umts_cell_id_equal(expected, parsed): ok +"999-999-L65534-R65535-S65535-C268435455" + -> umts_cell_id_from_str(): ok + -> umts_cell_id_to_str_buf(): ok + -> "999-999-L65534-R65535-S65535-C268435455" + umts_cell_id_equal(expected, parsed): ok +"1000-001-L1-R1-S1-C1" + expected rc != 0: ok +"001-001-L65535-R1-S1-C1" + expected rc != 0: ok +"001-001-L1-R1-S1-C268435456" + expected rc != 0: ok -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36892?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: Ibbb61a2c53a11dea794f451d3074bc9ba50862fe Gerrit-Change-Number: 36892 Gerrit-PatchSet: 4 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: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-hnbgw[master]: 3-digit MNC: use osmo_plmn_id in struct umts_cell_id
by neels 10 Jun '24

10 Jun '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36893?usp=email ) Change subject: 3-digit MNC: use osmo_plmn_id in struct umts_cell_id ...................................................................... 3-digit MNC: use osmo_plmn_id in struct umts_cell_id Properly represent the mnc_3_digits flag in umts_cell_id, and preserve the three digit indicator as received on the wire. Before this patch, the indicator for a three digit MNC received on the wire was discarded, and instead g_hnbgw->config.plmn.mnc_3_digits was used to convert any PLMN to string, whether it had 3 digits or not. == hnb_persistent_list: The cell id is used as primary key in the list of hnb_persistent instances. This patch prevents any collisions between 2-digit and 3-digit MNCs (however unlikely in practice this may be). == nft_kpi.c: Just like the cell ids in hnb_persistent, the ids' strings are used as primary key in nftables rulesets in nft_kpi.c -- also prevent MNC collisions there: Properly transport the 3-digit property in conversions: struct umts_cell_id <-> string Uncouple to_str conversion from the PLMN set in the hnbgw VTY cfg. Related: OS#6457 Change-Id: Id9a91c80cd2745424a916aef4736993bb7cd8ba0 --- M include/osmocom/hnbgw/hnbgw.h M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/hnbgw_hnbap.c M src/osmo-hnbgw/hnbgw_vty.c M tests/umts_cell_id/umts_cell_id_test.c M tests/umts_cell_id/umts_cell_id_test.ok 6 files changed, 94 insertions(+), 46 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve neels: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index 87844f4..9b07584 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -166,8 +166,7 @@ }; struct umts_cell_id { - uint16_t mcc; /*!< Mobile Country Code (0-999) */ - uint16_t mnc; /*!< Mobile Network Code (0-999) */ + struct osmo_plmn_id plmn; /*!< Mobile Country Code and Mobile Network Code (000-00 to 999-999) */ uint16_t lac; /*!< Locaton Area Code (1-65534) */ uint16_t rac; /*!< Routing Area Code (0-255) */ uint16_t sac; /*!< Service Area Code */ @@ -182,9 +181,7 @@ /*! are both given umts_cell_id euqal? */ static inline bool umts_cell_id_equal(const struct umts_cell_id *a, const struct umts_cell_id *b) { - if (a->mcc != b->mcc) - return false; - if (a->mnc != b->mnc) + if (osmo_plmn_cmp(&a->plmn, &b->plmn)) return false; if (a->lac != b->lac) return false; diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 4f3fafa..20a665e 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -234,12 +234,10 @@ int umts_cell_id_to_str_buf(char *buf, size_t buflen, const struct umts_cell_id *ucid) { - const char *fmtstr = "%03u-%02u-L%u-R%u-S%u-C%u"; - - if (g_hnbgw->config.plmn.mnc_3_digits) - fmtstr = "%03u-%03u-L%u-R%u-S%u-C%u"; - return snprintf(buf, buflen, fmtstr, ucid->mcc, ucid->mnc, ucid->lac, ucid->rac, - ucid->sac, ucid->cid); + struct osmo_strbuf sb = { .buf = buf, .len = buflen }; + OSMO_STRBUF_APPEND_NOLEN(sb, osmo_plmn_name_buf, &ucid->plmn); + OSMO_STRBUF_PRINTF(sb, "-L%u-R%u-S%u-C%u", ucid->lac, ucid->rac, ucid->sac, ucid->cid); + return sb.chars_needed; } char *umts_cell_id_to_str_c(void *ctx, const struct umts_cell_id *ucid) @@ -262,22 +260,38 @@ int umts_cell_id_from_str(struct umts_cell_id *ucid, const char *instr) { int rc; + char buf[4]; + const char *pos = instr; + const char *end; /* 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); + /* read MCC */ + end = strchr(pos, '-'); + if (!end || end <= pos || (end - pos) >= sizeof(buf)) + return -EINVAL; + osmo_strlcpy(buf, pos, end - pos + 1); + if (osmo_mcc_from_str(buf, &ucid->plmn.mcc)) + return -EINVAL; + pos = end + 1; + + /* read MNC -- here the number of leading zeros matters. */ + end = strchr(pos, '-'); + if (!end || end == pos || (end - pos) >= sizeof(buf)) + return -EINVAL; + osmo_strlcpy(buf, pos, end - pos + 1); + if (osmo_mnc_from_str(buf, &ucid->plmn.mnc, &ucid->plmn.mnc_3_digits)) + return -EINVAL; + pos = end + 1; + + /* parse the rest, where leading zeros do not matter */ + rc = sscanf(pos, "L%hu-R%hu-S%hu-C%u", &ucid->lac, &ucid->rac, &ucid->sac, &ucid->cid); if (rc < 0) return -errno; - if (rc != 6) - return -EINVAL; - - if (ucid->mcc > 999) - return -EINVAL; - - if (ucid->mnc > 999) + if (rc != 4) return -EINVAL; if (ucid->lac == 0 || ucid->lac == 0xffff) diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c index 29851a0..b220918 100644 --- a/src/osmo-hnbgw/hnbgw_hnbap.c +++ b/src/osmo-hnbgw/hnbgw_hnbap.c @@ -472,7 +472,6 @@ struct hnb_context *hnb, *tmp; HNBAP_HNBRegisterRequestIEs_t ies; int rc; - struct osmo_plmn_id plmn; struct osmo_fd *ofd = osmo_stream_srv_get_ofd(ctx->conn); char identity_str[256]; const char *cell_id_str; @@ -500,9 +499,7 @@ ctx->id.sac = asn1str_to_u16(&ies.sac); ctx->id.rac = asn1str_to_u8(&ies.rac); ctx->id.cid = asn1bitstr_to_u28(&ies.cellIdentity); - osmo_plmn_from_bcd(ies.plmNidentity.buf, &plmn); - ctx->id.mcc = plmn.mcc; - ctx->id.mnc = plmn.mnc; + osmo_plmn_from_bcd(ies.plmNidentity.buf, &ctx->id.plmn); cell_id_str = umts_cell_id_to_str(&ctx->id); if (getpeername(ofd->fd, &cur_osa.u.sa, &len) < 0) { @@ -761,8 +758,7 @@ { /* We don't care much about HNBAP */ LOGHNB(hnb, DHNBAP, LOGL_ERROR, "Received Unsuccessful Outcome, procedureCode %ld, criticality %ld," - " cell mcc %u mnc %u lac %u rac %u sac %u cid %u\n", msg->procedureCode, msg->criticality, - hnb->id.mcc, hnb->id.mnc, hnb->id.lac, hnb->id.rac, hnb->id.sac, hnb->id.cid); + " cell %s\n", msg->procedureCode, msg->criticality, umts_cell_id_to_str(&hnb->id)); return 0; } diff --git a/src/osmo-hnbgw/hnbgw_vty.c b/src/osmo-hnbgw/hnbgw_vty.c index 8163197..d719c29 100644 --- a/src/osmo-hnbgw/hnbgw_vty.c +++ b/src/osmo-hnbgw/hnbgw_vty.c @@ -210,8 +210,9 @@ vty_out(vty, "HNB "); vty_out_ofd_addr(vty, hnb->conn? osmo_stream_srv_get_ofd(hnb->conn) : NULL); vty_out(vty, " \"%s\"%s", hnb->identity_info, VTY_NEWLINE); - vty_out(vty, " MCC %u MNC %u LAC %u RAC %u SAC %u CID %u SCTP-stream:HNBAP=%u,RUA=%u%s", - hnb->id.mcc, hnb->id.mnc, hnb->id.lac, hnb->id.rac, hnb->id.sac, hnb->id.cid, + vty_out(vty, " MCC %s MNC %s LAC %u RAC %u SAC %u CID %u SCTP-stream:HNBAP=%u,RUA=%u%s", + osmo_mcc_name(hnb->id.plmn.mcc), osmo_mnc_name(hnb->id.plmn.mnc, hnb->id.plmn.mnc_3_digits), + hnb->id.lac, hnb->id.rac, hnb->id.sac, hnb->id.cid, hnb->hnbap_stream, hnb->rua_stream, VTY_NEWLINE); llist_for_each_entry(map, &hnb->map_list, hnb_list) { diff --git a/tests/umts_cell_id/umts_cell_id_test.c b/tests/umts_cell_id/umts_cell_id_test.c index e9183c4..a25b3d1 100644 --- a/tests/umts_cell_id/umts_cell_id_test.c +++ b/tests/umts_cell_id/umts_cell_id_test.c @@ -12,8 +12,10 @@ { .id_str = "001-01-L1-R1-S1-C1", .id = { - .mcc = 1, - .mnc = 1, + .plmn = { + .mcc = 1, + .mnc = 1, + }, .lac = 1, .rac = 1, .sac = 1, @@ -25,8 +27,11 @@ { .id_str = "001-001-L1-R1-S1-C1", .id = { - .mcc = 1, - .mnc = 1, + .plmn = { + .mcc = 1, + .mnc = 1, + .mnc_3_digits = true, + }, .lac = 1, .rac = 1, .sac = 1, @@ -36,8 +41,11 @@ { .id_str = "001-099-L1-R1-S1-C1", .id = { - .mcc = 1, - .mnc = 99, + .plmn = { + .mcc = 1, + .mnc = 99, + .mnc_3_digits = true, + }, .lac = 1, .rac = 1, .sac = 1, @@ -47,8 +55,11 @@ { .id_str = "001-99-L1-R1-S1-C1", .id = { - .mcc = 1, - .mnc = 99, + .plmn = { + .mcc = 1, + .mnc = 99, + .mnc_3_digits = false, + }, .lac = 1, .rac = 1, .sac = 1, @@ -59,8 +70,11 @@ { .id_str = "999-999-L65534-R65535-S65535-C268435455", .id = { - .mcc = 999, - .mnc = 999, + .plmn = { + .mcc = 999, + .mnc = 999, + .mnc_3_digits = true, + }, .lac = 65534, .rac = 65535, .sac = 65535, @@ -94,12 +108,7 @@ int main(void) { - struct hnbgw hnbgw_dummy = {}; struct test *t; - - /* umts_cell_id_to_str() accesses g_hnbgw->config.plmn.mnc_3_digits, so make sure it is valid mem: */ - g_hnbgw = &hnbgw_dummy; - for (t = tests; (t - tests) < ARRAY_SIZE(tests); t++) { int rc; struct umts_cell_id parsed; diff --git a/tests/umts_cell_id/umts_cell_id_test.ok b/tests/umts_cell_id/umts_cell_id_test.ok index 457f78a..2c1992f 100644 --- a/tests/umts_cell_id/umts_cell_id_test.ok +++ b/tests/umts_cell_id/umts_cell_id_test.ok @@ -6,14 +6,12 @@ "001-001-L1-R1-S1-C1" -> umts_cell_id_from_str(): ok -> umts_cell_id_to_str_buf(): ok - ERROR: conversion to umts_cell_id and back to string doesn't return the original string - -> "001-01-L1-R1-S1-C1" + -> "001-001-L1-R1-S1-C1" umts_cell_id_equal(expected, parsed): ok "001-099-L1-R1-S1-C1" -> umts_cell_id_from_str(): ok -> umts_cell_id_to_str_buf(): ok - ERROR: conversion to umts_cell_id and back to string doesn't return the original string - -> "001-99-L1-R1-S1-C1" + -> "001-099-L1-R1-S1-C1" umts_cell_id_equal(expected, parsed): ok "001-99-L1-R1-S1-C1" -> umts_cell_id_from_str(): ok -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36893?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: Id9a91c80cd2745424a916aef4736993bb7cd8ba0 Gerrit-Change-Number: 36893 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-hnbgw[master]: rename to umts_cell_id_to_str()
by neels 10 Jun '24

10 Jun '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/36890?usp=email ) Change subject: rename to umts_cell_id_to_str() ...................................................................... rename to umts_cell_id_to_str() A subsequent patch adds umts_cell_id_to_str_buf(), and before the old foo_name() pattern spreads further, I'd rather rename it. Rationale: - There is a umts_cell_id_from_str() function. - The foo_name() is an old pattern, we prefer foo_to_str() now. - Contained within osmo-hnbgw, no API problems. Change-Id: I3124d1f5e634bc895ec347cb1a9816789fd9ab69 --- M include/osmocom/hnbgw/hnbgw.h M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/hnbgw_hnbap.c M src/osmo-hnbgw/nft_kpi.c 4 files changed, 27 insertions(+), 10 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve neels: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index 666088c..2c7e014 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -173,7 +173,7 @@ uint16_t sac; /*!< Service Area Code */ uint32_t cid; /*!< Cell ID */ }; -const char *umts_cell_id_name(const struct umts_cell_id *ucid); +const char *umts_cell_id_to_str(const struct umts_cell_id *ucid); int umts_cell_id_from_str(struct umts_cell_id *ucid, const char *instr); uint32_t umts_cell_id_hash(const struct umts_cell_id *ucid); diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index a36d107..c41fb54 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -232,7 +232,7 @@ return ctx; } -const char *umts_cell_id_name(const struct umts_cell_id *ucid) +const char *umts_cell_id_to_str(const struct umts_cell_id *ucid) { const char *fmtstr = "%03u-%02u-L%u-R%u-S%u-C%u"; @@ -304,7 +304,7 @@ if (g_hnbgw->config.log_prefix_hnb_id) result = talloc_asprintf(OTC_SELECT, "%s %s", result, ctx->identity_info); else - result = talloc_asprintf(OTC_SELECT, "%s %s", result, umts_cell_id_name(&ctx->id)); + result = talloc_asprintf(OTC_SELECT, "%s %s", result, umts_cell_id_to_str(&ctx->id)); return result; } @@ -544,7 +544,7 @@ return NULL; hnbp->id = *id; - hnbp->id_str = talloc_strdup(hnbp, umts_cell_id_name(id)); + hnbp->id_str = talloc_strdup(hnbp, umts_cell_id_to_str(id)); hnbp->ctrs = rate_ctr_group_alloc(hnbp, &hnb_ctrg_desc, 0); if (!hnbp->ctrs) goto out_free; diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c index d7e4dc3..29851a0 100644 --- a/src/osmo-hnbgw/hnbgw_hnbap.c +++ b/src/osmo-hnbgw/hnbgw_hnbap.c @@ -503,7 +503,7 @@ osmo_plmn_from_bcd(ies.plmNidentity.buf, &plmn); ctx->id.mcc = plmn.mcc; ctx->id.mnc = plmn.mnc; - cell_id_str = umts_cell_id_name(&ctx->id); + cell_id_str = umts_cell_id_to_str(&ctx->id); if (getpeername(ofd->fd, &cur_osa.u.sa, &len) < 0) { LOGHNB(ctx, DHNBAP, LOGL_ERROR, "HNB-REGISTER-REQ %s: rejecting due to getpeername() error: %s\n", diff --git a/src/osmo-hnbgw/nft_kpi.c b/src/osmo-hnbgw/nft_kpi.c index 42c009f..9eff35e 100644 --- a/src/osmo-hnbgw/nft_kpi.c +++ b/src/osmo-hnbgw/nft_kpi.c @@ -361,7 +361,7 @@ { char cmd[1024]; struct osmo_strbuf sb = { .buf = cmd, .len = sizeof(cmd) }; - const char *cell_id_str = umts_cell_id_name(&req->hnbp_init_remove.cell_id); + const char *cell_id_str = umts_cell_id_to_str(&req->hnbp_init_remove.cell_id); OSMO_STRBUF_PRINTF(sb, "add counter inet %s ul-%s;\n" "add counter inet %s dl-%s;\n", @@ -376,7 +376,7 @@ { char cmd[1024]; struct osmo_strbuf sb = { .buf = cmd, .len = sizeof(cmd) }; - const char *cell_id_str = umts_cell_id_name(&req->hnbp_init_remove.cell_id); + const char *cell_id_str = umts_cell_id_to_str(&req->hnbp_init_remove.cell_id); OSMO_STRBUF_PRINTF(sb, "delete counter inet %s ul-%s;\n" "delete counter inet %s dl-%s;\n", @@ -406,7 +406,7 @@ { char cmd[1024]; struct osmo_strbuf sb = { .buf = cmd, .len = sizeof(cmd) }; - const char *cell_id_str = umts_cell_id_name(&req->hnb_start.cell_id); + const char *cell_id_str = umts_cell_id_to_str(&req->hnb_start.cell_id); int rc; OSMO_STRBUF_PRINTF(sb, @@ -911,7 +911,7 @@ /* Paranoid corner case: while we added the rules for this hNodeB, it was apparently removed and does * not exist anymore. We need to just drop the rules again right away. */ LOGP(DNFT, LOGL_ERROR, "Added nft rules for unknown cell %s; removing rules again\n", - umts_cell_id_name(&req->hnb_start.cell_id)); + umts_cell_id_to_str(&req->hnb_start.cell_id)); drop_again = true; } @@ -921,7 +921,7 @@ * drop the rules again right away. */ LOGP(DNFT, LOGL_INFO, "Cell %s disconnected before adding counter rules completed. removing rules again\n", - umts_cell_id_name(&req->hnb_start.cell_id)); + umts_cell_id_to_str(&req->hnb_start.cell_id)); drop_again = true; } -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/36890?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: I3124d1f5e634bc895ec347cb1a9816789fd9ab69 Gerrit-Change-Number: 36890 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-hnbgw[master]: fix stat_item leak in hnb_persistent_free()
by neels 10 Jun '24

10 Jun '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37074?usp=email ) Change subject: fix stat_item leak in hnb_persistent_free() ...................................................................... fix stat_item leak in hnb_persistent_free() Add missing stat_item free in hnb_persistent_free(). We recently fixed a rate_ctr leak in I14e050bfb91b993f194e3800eacdc0d10f2b1a4e, but missed the also leaking stat_item. Particularly relevant with upcoming patch Ic819d7cbc03fb39e98c204b70d016c5170dc6307 -- testing that patch revealed the leak. Related: osmo-ttcn3-hacks Ibec009203d38f65714561b7c28edbdbd8b34e704 Change-Id: I7326c53d595dce7b442eced89ff8f4b972bd2a82 --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 21 insertions(+), 0 deletions(-) Approvals: laforge: Looks good to me, approved pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 43162fc..8dcf5f2 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -663,6 +663,7 @@ /* FIXME: check if in use? */ nft_kpi_hnb_stop(hnbp); nft_kpi_hnb_persistent_remove(hnbp); + osmo_stat_item_group_free(hnbp->statg); rate_ctr_group_free(hnbp->ctrs); llist_del(&hnbp->list); hash_del(&hnbp->node_by_id); -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37074?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: I7326c53d595dce7b442eced89ff8f4b972bd2a82 Gerrit-Change-Number: 37074 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

2026

2025

2024

2023

2022

gerrit-log