From e5b6feabbb3d1a9a00ef93a60709554b3ed52992 Mon Sep 17 00:00:00 2001 From: Neels Janosch Hofmeyr Date: Fri, 24 Mar 2023 02:04:28 +0100 Subject: [PATCH] [RFC] drop list of HNBAP UE Context *Request for comments:* In previous patches, I have fixed some more ue_context leak situations. But since we don't really use ue_context for anything, we could also just drop this completely. On HNBAP UE Register, we collect the ue_contexts in a ue_list. But we never do anything with this list, at all. Furthermore, users are reporting the list of ue_context growing indefinitely. Simply drop the ue_context listing. Simply acknowledge all HNBAP UE Register and DeRegister requests without storing any context IDs. Change-Id: Ida7eadf36abcf465ae40003725c49e8e321a28c9 --- include/osmocom/hnbgw/hnbgw.h | 11 +---- src/osmo-hnbgw/hnbgw.c | 86 +---------------------------------- src/osmo-hnbgw/hnbgw_hnbap.c | 42 +++-------------- src/osmo-hnbgw/hnbgw_vty.c | 18 -------- 4 files changed, 11 insertions(+), 146 deletions(-) diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index 1bf140a..e4f4f00 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -150,8 +150,6 @@ struct hnb_gw { struct osmo_stream_srv_link *iuh; /* list of struct hnb_context */ struct llist_head hnb_list; - /* list of struct ue_context */ - struct llist_head ue_list; /* next availble UE Context ID */ uint32_t next_ue_ctx_id; struct ctrl_handle *ctrl; @@ -180,13 +178,6 @@ struct hnb_context *hnb_context_by_identity_info(struct hnb_gw *gw, const char * const char *hnb_context_name(struct hnb_context *ctx); unsigned hnb_contexts(const struct hnb_gw *gw); -struct ue_context *ue_context_by_id(struct hnb_gw *gw, uint32_t id); -struct ue_context *ue_context_by_imsi(struct hnb_gw *gw, const char *imsi); -struct ue_context *ue_context_by_tmsi(struct hnb_gw *gw, uint32_t tmsi); -struct ue_context *ue_context_alloc(struct hnb_context *hnb, const char *imsi, - uint32_t tmsi); -void ue_context_free(struct ue_context *ue); - struct hnb_context *hnb_context_alloc(struct hnb_gw *gw, struct osmo_stream_srv_link *link, int new_fd); void hnb_context_release(struct hnb_context *ctx); void hnb_context_release_ue_state(struct hnb_context *ctx); @@ -205,3 +196,5 @@ static inline bool hnb_gw_is_gtp_mapping_enabled(const struct hnb_gw *gw) } struct msgb *hnbgw_ranap_msg_alloc(const char *name); + +uint32_t get_next_ue_ctx_id(struct hnb_gw *gw); diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 0624803..66df9bf 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -96,7 +96,6 @@ static struct hnb_gw *hnb_gw_create(void *ctx) gw->next_ue_ctx_id = 23; INIT_LLIST_HEAD(&gw->hnb_list); - INIT_LLIST_HEAD(&gw->ue_list); gw->mgw_pool = mgcp_client_pool_alloc(gw); gw->config.mgcp_client = talloc_zero(tall_hnb_ctx, struct mgcp_client_conf); @@ -146,89 +145,9 @@ unsigned hnb_contexts(const struct hnb_gw *gw) return num_ctx; } -struct ue_context *ue_context_by_id(struct hnb_gw *gw, uint32_t id) +uint32_t get_next_ue_ctx_id(struct hnb_gw *gw) { - struct ue_context *ue; - - llist_for_each_entry(ue, &gw->ue_list, list) { - if (ue->context_id == id) - return ue; - } - return NULL; - -} - -struct ue_context *ue_context_by_imsi(struct hnb_gw *gw, const char *imsi) -{ - struct ue_context *ue; - - llist_for_each_entry(ue, &gw->ue_list, list) { - if (!strcmp(ue->imsi, imsi)) - return ue; - } - return NULL; -} - -struct ue_context *ue_context_by_tmsi(struct hnb_gw *gw, uint32_t tmsi) -{ - struct ue_context *ue; - - llist_for_each_entry(ue, &gw->ue_list, list) { - if (ue->tmsi == tmsi) - return ue; - } - return NULL; -} - -void ue_context_free_by_hnb(struct hnb_gw *gw, const struct hnb_context *hnb) -{ - struct ue_context *ue, *tmp; - - llist_for_each_entry_safe(ue, tmp, &gw->ue_list, list) { - if (ue->hnb == hnb) - ue_context_free(ue); - } -} - -static uint32_t get_next_ue_ctx_id(struct hnb_gw *gw) -{ - uint32_t id; - - do { - id = gw->next_ue_ctx_id++; - } while (ue_context_by_id(gw, id)); - - return id; -} - -struct ue_context *ue_context_alloc(struct hnb_context *hnb, const char *imsi, - uint32_t tmsi) -{ - struct ue_context *ue; - - ue = talloc_zero(tall_hnb_ctx, struct ue_context); - if (!ue) - return NULL; - - ue->hnb = hnb; - if (imsi) - OSMO_STRLCPY_ARRAY(ue->imsi, imsi); - else - ue->imsi[0] = '\0'; - ue->tmsi = tmsi; - ue->context_id = get_next_ue_ctx_id(hnb->gw); - llist_add_tail(&ue->list, &hnb->gw->ue_list); - - LOGP(DHNBAP, LOGL_INFO, "created UE context: id 0x%x, imsi %s, tmsi 0x%x\n", - ue->context_id, imsi? imsi : "-", tmsi); - - return ue; -} - -void ue_context_free(struct ue_context *ue) -{ - llist_del(&ue->list); - talloc_free(ue); + return gw->next_ue_ctx_id++; } static int hnb_read_cb(struct osmo_stream_srv *conn) @@ -399,7 +318,6 @@ void hnb_context_release_ue_state(struct hnb_context *ctx) context_map_hnb_released(map); /* hnbgw_context_map will remove itself from lists when it is ready. */ } - ue_context_free_by_hnb(ctx->gw, ctx); } void hnb_context_release(struct hnb_context *ctx) diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c index a1cbec5..b39629e 100644 --- a/src/osmo-hnbgw/hnbgw_hnbap.c +++ b/src/osmo-hnbgw/hnbgw_hnbap.c @@ -125,7 +125,7 @@ static int hnbgw_tx_hnb_register_acc(struct hnb_context *ctx) } -static int hnbgw_tx_ue_register_acc(struct ue_context *ue) +static int hnbgw_tx_ue_register_acc(struct hnb_context *hnb, const char *imsi, uint32_t context_id) { HNBAP_UERegisterAccept_t accept_out; HNBAP_UERegisterAcceptIEs_t accept; @@ -136,13 +136,13 @@ static int hnbgw_tx_ue_register_acc(struct ue_context *ue) int rc; encoded_imsi_len = ranap_imsi_encode(encoded_imsi, - sizeof(encoded_imsi), ue->imsi); + sizeof(encoded_imsi), imsi); memset(&accept, 0, sizeof(accept)); accept.uE_Identity.present = HNBAP_UE_Identity_PR_iMSI; OCTET_STRING_fromBuf(&accept.uE_Identity.choice.iMSI, (const char *)encoded_imsi, encoded_imsi_len); - asn1_u24_to_bitstring(&accept.context_ID, &ctx_id, ue->context_id); + asn1_u24_to_bitstring(&accept.context_ID, &ctx_id, context_id); memset(&accept_out, 0, sizeof(accept_out)); rc = hnbap_encode_ueregisteraccepties(&accept_out, &accept); @@ -158,7 +158,7 @@ static int hnbgw_tx_ue_register_acc(struct ue_context *ue) ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_OCTET_STRING, &accept.uE_Identity.choice.iMSI); ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_HNBAP_UERegisterAccept, &accept_out); - return hnbgw_hnbap_tx(ue->hnb, msg); + return hnbgw_hnbap_tx(hnb, msg); } static int hnbgw_tx_ue_register_rej_tmsi(struct hnb_context *hnb, HNBAP_UE_Identity_t *ue_id) @@ -283,8 +283,6 @@ static int hnbgw_tx_ue_register_acc_tmsi(struct hnb_context *hnb, HNBAP_UE_Ident struct msgb *msg; uint32_t ctx_id; uint32_t tmsi = 0; - struct ue_context *ue; - struct ue_context *ue_allocated = NULL; int rc; memset(&accept, 0, sizeof(accept)); @@ -330,21 +328,12 @@ static int hnbgw_tx_ue_register_acc_tmsi(struct hnb_context *hnb, HNBAP_UE_Ident tmsi = ntohl(tmsi); LOGHNB(hnb, DHNBAP, LOGL_DEBUG, "HNBAP register with TMSI %x\n", tmsi); - ue = ue_context_by_tmsi(hnb->gw, tmsi); - if (!ue) - ue = ue_allocated = ue_context_alloc(hnb, NULL, tmsi); - - asn1_u24_to_bitstring(&accept.context_ID, &ctx_id, ue->context_id); + asn1_u24_to_bitstring(&accept.context_ID, &ctx_id, get_next_ue_ctx_id(hnb->gw)); memset(&accept_out, 0, sizeof(accept_out)); rc = hnbap_encode_ueregisteraccepties(&accept_out, &accept); - if (rc < 0) { - /* If we allocated the UE context but the UE REGISTER fails, get rid of it again: there will likely - * never be a UE DE-REGISTER for this UE from the HNB, and the ue_context would linger forever. */ - if (ue_allocated) - ue_context_free(ue_allocated); + if (rc < 0) return rc; - } msg = hnbap_generate_successful_outcome(HNBAP_ProcedureCode_id_UERegister, HNBAP_Criticality_reject, @@ -486,8 +475,6 @@ static int hnbgw_rx_hnb_register_req(struct hnb_context *ctx, ANY_t *in) static int hnbgw_rx_ue_register_req(struct hnb_context *ctx, ANY_t *in) { HNBAP_UERegisterRequestIEs_t ies; - struct ue_context *ue; - struct ue_context *ue_allocated = NULL; char imsi[16]; int rc; @@ -527,26 +514,15 @@ static int hnbgw_rx_ue_register_req(struct hnb_context *ctx, ANY_t *in) LOGHNB(ctx, DHNBAP, LOGL_DEBUG, "UE-REGISTER-REQ ID_type=%d imsi=%s cause=%ld\n", ies.uE_Identity.present, imsi, ies.registration_Cause); - ue = ue_context_by_imsi(ctx->gw, imsi); - if (!ue) - ue = ue_allocated = ue_context_alloc(ctx, imsi, 0); - hnbap_free_ueregisterrequesties(&ies); /* Send UERegisterAccept */ - rc = hnbgw_tx_ue_register_acc(ue); - if (rc < 0) { - /* If we allocated the UE context but the UE REGISTER fails, get rid of it again: there will likely - * never be a UE DE-REGISTER for this UE from the HNB, and the ue_context would linger forever. */ - if (ue_allocated) - ue_context_free(ue_allocated); - } + rc = hnbgw_tx_ue_register_acc(ctx, imsi, get_next_ue_ctx_id(ctx->gw)); return rc; } static int hnbgw_rx_ue_deregister(struct hnb_context *ctx, ANY_t *in) { HNBAP_UEDe_RegisterIEs_t ies; - struct ue_context *ue; int rc; uint32_t ctxid; @@ -558,10 +534,6 @@ static int hnbgw_rx_ue_deregister(struct hnb_context *ctx, ANY_t *in) LOGHNB(ctx, DHNBAP, LOGL_DEBUG, "UE-DE-REGISTER context=%u cause=%s\n", ctxid, hnbap_cause_str(&ies.cause)); - ue = ue_context_by_id(ctx->gw, ctxid); - if (ue) - ue_context_free(ue); - hnbap_free_uede_registeries(&ies); return 0; } diff --git a/src/osmo-hnbgw/hnbgw_vty.c b/src/osmo-hnbgw/hnbgw_vty.c index 8d2366f..9786b9b 100644 --- a/src/osmo-hnbgw/hnbgw_vty.c +++ b/src/osmo-hnbgw/hnbgw_vty.c @@ -215,12 +215,6 @@ static void vty_dump_hnb_info(struct vty *vty, struct hnb_context *hnb) vty_dump_hnb_info__map_states(vty, "IuPS", map_count[1], state_count[1]); } -static void vty_dump_ue_info(struct vty *vty, struct ue_context *ue) -{ - vty_out(vty, "UE IMSI \"%s\" context ID %u HNB %s%s", ue->imsi, ue->context_id, - hnb_context_name(ue->hnb), VTY_NEWLINE); -} - DEFUN(show_hnb, show_hnb_cmd, "show hnb all", SHOW_STR "Display information about all HNB") { struct hnb_context *hnb; @@ -261,17 +255,6 @@ DEFUN(show_one_hnb, show_one_hnb_cmd, "show hnb NAME ", SHOW_STR "Display inform return CMD_SUCCESS; } -DEFUN(show_ue, show_ue_cmd, "show ue all", SHOW_STR "Display information about a UE") -{ - struct ue_context *ue; - - llist_for_each_entry(ue, &g_hnb_gw->ue_list, list) { - vty_dump_ue_info(vty, ue); - } - - return CMD_SUCCESS; -} - DEFUN(show_talloc, show_talloc_cmd, "show talloc", SHOW_STR "Display talloc info") { talloc_report_full(tall_hnb_ctx, stderr); @@ -509,7 +492,6 @@ void hnbgw_vty_init(struct hnb_gw *gw, void *tall_ctx) install_element_ve(&show_cnlink_cmd); install_element_ve(&show_hnb_cmd); install_element_ve(&show_one_hnb_cmd); - install_element_ve(&show_ue_cmd); install_element_ve(&show_talloc_cmd); install_element(HNBGW_NODE, &cfg_hnbgw_mgcp_cmd);