neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32041?usp=email )
Change subject: drop list of HNBAP UE Context ......................................................................
drop list of HNBAP UE Context
Last year, 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 --- 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 4 files changed, 35 insertions(+), 154 deletions(-)
Approvals: Jenkins Builder: Verified neels: Looks good to me, approved
diff --git a/include/osmocom/hnbgw/hnbgw.h b/include/osmocom/hnbgw/hnbgw.h index a647abd..caaff5f 100644 --- a/include/osmocom/hnbgw/hnbgw.h +++ b/include/osmocom/hnbgw/hnbgw.h @@ -470,8 +470,6 @@ DECLARE_HASHTABLE(hnb_persistent_by_id, 5);
struct osmo_timer_list store_uptime_timer; - /* 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; @@ -516,13 +514,6 @@ struct hnb_context *hnb_context_by_identity_info(const char *identity_info); const char *hnb_context_name(struct hnb_context *ctx);
-struct ue_context *ue_context_by_id(uint32_t id); -struct ue_context *ue_context_by_imsi(const char *imsi); -struct ue_context *ue_context_by_tmsi(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); - void hnb_context_release(struct hnb_context *ctx); void hnb_context_release_ue_state(struct hnb_context *ctx);
@@ -550,3 +541,5 @@ int hnbgw_peek_l3_ul(struct hnbgw_context_map *map, struct msgb *ranap_msg);
unsigned long long hnb_get_updowntime(const struct hnb_context *ctx); + +uint32_t get_next_ue_ctx_id(void); diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 43fcaca..d4515a8 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -94,91 +94,11 @@ * UE Context ***********************************************************************/
-struct ue_context *ue_context_by_id(uint32_t id) +uint32_t get_next_ue_ctx_id(void) { - struct ue_context *ue; - - llist_for_each_entry(ue, &g_hnbgw->ue_list, list) { - if (ue->context_id == id) - return ue; - } - return NULL; - + return g_hnbgw->next_ue_ctx_id++; }
-struct ue_context *ue_context_by_imsi(const char *imsi) -{ - struct ue_context *ue; - - llist_for_each_entry(ue, &g_hnbgw->ue_list, list) { - if (!strcmp(ue->imsi, imsi)) - return ue; - } - return NULL; -} - -struct ue_context *ue_context_by_tmsi(uint32_t tmsi) -{ - struct ue_context *ue; - - llist_for_each_entry(ue, &g_hnbgw->ue_list, list) { - if (ue->tmsi == tmsi) - return ue; - } - return NULL; -} - -static void ue_context_free_by_hnb(const struct hnb_context *hnb) -{ - struct ue_context *ue, *tmp; - - llist_for_each_entry_safe(ue, tmp, &g_hnbgw->ue_list, list) { - if (ue->hnb == hnb) - ue_context_free(ue); - } -} - -static uint32_t get_next_ue_ctx_id(void) -{ - uint32_t id; - - do { - id = g_hnbgw->next_ue_ctx_id++; - } while (ue_context_by_id(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(g_hnbgw, struct ue_context); - OSMO_ASSERT(ue); - - 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(); - llist_add_tail(&ue->list, &g_hnbgw->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); -} - - /*********************************************************************** * HNB Context ***********************************************************************/ @@ -340,7 +260,6 @@ context_map_hnb_released(map); /* hnbgw_context_map will remove itself from lists when it is ready. */ } - ue_context_free_by_hnb(ctx); }
void hnb_context_release(struct hnb_context *ctx) @@ -1068,7 +987,6 @@ INIT_LLIST_HEAD(&g_hnbgw->hnb_persistent_list); hash_init(g_hnbgw->hnb_persistent_by_id);
- INIT_LLIST_HEAD(&g_hnbgw->ue_list); INIT_LLIST_HEAD(&g_hnbgw->sccp.users);
g_hnbgw->mgw_pool = mgcp_client_pool_alloc(g_hnbgw); diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c index f860173..df4ec9d 100644 --- a/src/osmo-hnbgw/hnbgw_hnbap.c +++ b/src/osmo-hnbgw/hnbgw_hnbap.c @@ -171,7 +171,7 @@ }
-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; @@ -182,21 +182,20 @@ 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); ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_OCTET_STRING, &accept.uE_Identity.choice.iMSI); if (rc < 0) { - LOGHNB(ue->hnb, DHNBAP, LOGL_ERROR, - "Failed to encode HNBAP UE Register Accept message for UE IMSI-%s TMSI-0x%08x\n", - ue->imsi, ue->tmsi); + LOGHNB(hnb, DHNBAP, LOGL_ERROR, + "Failed to encode HNBAP UE Register Accept message for UE IMSI-%s\n", imsi); return rc; }
@@ -207,11 +206,10 @@
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_HNBAP_UERegisterAccept, &accept_out);
- rc = hnbgw_hnbap_tx(ue->hnb, msg); + rc = hnbgw_hnbap_tx(hnb, msg); if (rc) - LOGHNB(ue->hnb, DHNBAP, LOGL_ERROR, - "Failed to enqueue HNBAP UE Register Accept message for UE IMSI-%s TMSI-0x%08x\n", - ue->imsi, ue->tmsi); + LOGHNB(hnb, DHNBAP, LOGL_ERROR, + "Failed to enqueue HNBAP UE Register Accept message for UE IMSI-%s\n", imsi); return rc; }
@@ -350,8 +348,6 @@ 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)); @@ -397,11 +393,7 @@ tmsi = ntohl(tmsi); LOGHNB(hnb, DHNBAP, LOGL_DEBUG, "HNBAP register with TMSI %x\n", tmsi);
- ue = ue_context_by_tmsi(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());
memset(&accept_out, 0, sizeof(accept_out)); rc = hnbap_encode_ueregisteraccepties(&accept_out, &accept); @@ -436,10 +428,6 @@ if (rc < 0) { LOGHNB(hnb, DHNBAP, LOGL_ERROR, "Failed to encode HNBAP UE Register Accept for TMSI 0x%08x\n", tmsi); /* Encoding failed. Nothing in 'accept_out'. */ - /* 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); return rc; }
@@ -592,8 +580,6 @@ { HNBAP_UERegisterRequestIEs_t ies; HNBAP_Cause_t cause; - struct ue_context *ue; - struct ue_context *ue_allocated = NULL; char imsi[GSM23003_IMSI_MAX_DIGITS+1]; int rc;
@@ -650,23 +636,10 @@ 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(imsi); - if (!ue) - ue = ue_allocated = ue_context_alloc(ctx, imsi, 0); - else - LOGHNB(ctx, DHNBAP, LOGL_DEBUG, "UE context for IMSI %s already exists\n", imsi); - /* Send UERegisterAccept */ - rc = hnbgw_tx_ue_register_acc(ue); - if (rc < 0) { + rc = hnbgw_tx_ue_register_acc(ctx, imsi, get_next_ue_ctx_id()); + if (rc < 0) LOGHNB(ctx, DHNBAP, LOGL_ERROR, "Failed to transmit HNBAP UE Register Accept for IMSI %s\n", imsi); - /* 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); - LOGHNB(ctx, DHNBAP, LOGL_INFO, "Freed UE context for IMSI %s\n", imsi); - } - } free_and_return_rc: hnbap_free_ueregisterrequesties(&ies); return rc; @@ -676,7 +649,6 @@ { HNBAP_UEDe_RegisterIEs_t ies; HNBAP_Cause_t cause; - struct ue_context *ue; int rc; uint32_t ctxid;
@@ -699,9 +671,6 @@ HNBAP_Criticality_ignore, HNBAP_TriggeringMessage_initiating_message); } else { LOGHNB(ctx, DHNBAP, LOGL_DEBUG, "UE-DE-REGISTER context=%u cause=%s\n", ctxid, hnbap_cause_str(&ies.cause)); - ue = ue_context_by_id(ctxid); - if (ue) - ue_context_free(ue); }
hnbap_free_uede_registeries(&ies); diff --git a/src/osmo-hnbgw/hnbgw_vty.c b/src/osmo-hnbgw/hnbgw_vty.c index d719c29..68d4b1f 100644 --- a/src/osmo-hnbgw/hnbgw_vty.c +++ b/src/osmo-hnbgw/hnbgw_vty.c @@ -229,12 +229,6 @@ } }
-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); -} - #define SHOW_HNB_STR SHOW_STR "Display information about HNB\n"
DEFUN(show_hnb, show_hnb_cmd, "show hnb all", @@ -279,18 +273,6 @@ return CMD_SUCCESS; }
-DEFUN(show_ue, show_ue_cmd, "show ue all", - SHOW_STR "Display HNBAP information about UE\n" "All UE\n") -{ - struct ue_context *ue; - - llist_for_each_entry(ue, &g_hnbgw->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(g_hnbgw, stderr); @@ -1169,7 +1151,6 @@ 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);
-
neels