From 230258370b52cceb33fedfed6e0945e71f10a76e Mon Sep 17 00:00:00 2001 From: Robert Schmidt Date: Fri, 3 Nov 2023 07:30:17 +0100 Subject: [PATCH 1/3] Free F1 UE data struct only on UE release Before this commit, as visible in the handler of an F1 UE release, it can happen that we forward the RRC Release message to a UE and start the release timer, while also removing the F1 UE data (containing e.g., the CU UE ID). This can lead to asserts, since there might still be subsequent traffic for such UE during a short time, while the DU does not have the secondary UE ID stored anymore. This commit changes the logic. It introduces function nr_mac_release_ue() that removes the UE at MAC and RLC, frees the F1 UE data (if applicable), and sends the complete, after timing expiry. If the UE is out of sync, this function is used to free the UE immediately. This fixes #685 and #698. --- .../NR_MAC_gNB/gNB_scheduler_primitives.c | 29 +++++++++++++++++-- openair2/LAYER2/NR_MAC_gNB/mac_proto.h | 1 + .../LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c | 21 ++++++-------- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c index 9b6528d7b0..7c4322f654 100644 --- a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c +++ b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c @@ -2809,6 +2809,30 @@ int nr_mac_enable_ue_rrc_processing_timer(gNB_MAC_INST *mac, NR_UE_info_t *UE, b return 0; } +void nr_mac_release_ue(gNB_MAC_INST *mac, int rnti) +{ + NR_SCHED_ENSURE_LOCKED(&mac->sched_lock); + + nr_rlc_remove_ue(rnti); + mac_remove_nr_ue(mac, rnti); + + // the CU might not know such UE, e.g., because we never sent a message to + // it, so there might not be a corresponding entry for such UE in the look up + // table. This can happen, e.g., on Msg.3 with C-RNTI, where we create a UE + // MAC context, decode the PDU, find the C-RNTI MAC CE, and then throw the + // newly created context away. See also in _nr_rx_sdu() and commit 93f59a3c6e56f + if (du_exists_f1_ue_data(rnti)) { + f1_ue_data_t ue_data = du_get_f1_ue_data(rnti); + f1ap_ue_context_release_complete_t complete = { + .gNB_CU_ue_id = ue_data.secondary_ue, + .gNB_DU_ue_id = rnti, + }; + mac->mac_rrc.ue_context_release_complete(&complete); + + du_remove_f1_ue_data(rnti); + } +} + void nr_mac_update_timers(module_id_t module_id, frame_t frame, sub_frame_t slot) @@ -2823,8 +2847,7 @@ void nr_mac_update_timers(module_id_t module_id, NR_UE_sched_ctrl_t *sched_ctrl = &UE->UE_sched_ctrl; if (nr_mac_check_release(sched_ctrl, UE->rnti)) { - nr_rlc_remove_ue(UE->rnti); - mac_remove_nr_ue(mac, UE->rnti); + nr_mac_release_ue(mac, UE->rnti); // go back to examine the next UE, which is at the position the // current UE was UE--; @@ -2963,7 +2986,7 @@ void prepare_initial_ul_rrc_message(gNB_MAC_INST *mac, NR_UE_info_t *UE) void nr_mac_trigger_release_timer(NR_UE_sched_ctrl_t *sched_ctrl, NR_SubcarrierSpacing_t subcarrier_spacing) { // trigger 60ms - sched_ctrl->release_timer = 60 << subcarrier_spacing; + sched_ctrl->release_timer = 100 << subcarrier_spacing; } bool nr_mac_check_release(NR_UE_sched_ctrl_t *sched_ctrl, int rnti) diff --git a/openair2/LAYER2/NR_MAC_gNB/mac_proto.h b/openair2/LAYER2/NR_MAC_gNB/mac_proto.h index 9f96a0d106..e58d2c12ae 100644 --- a/openair2/LAYER2/NR_MAC_gNB/mac_proto.h +++ b/openair2/LAYER2/NR_MAC_gNB/mac_proto.h @@ -443,6 +443,7 @@ void abort_nr_dl_harq(NR_UE_info_t* UE, int8_t harq_pid); void nr_mac_trigger_release_timer(NR_UE_sched_ctrl_t *sched_ctrl, NR_SubcarrierSpacing_t subcarrier_spacing); bool nr_mac_check_release(NR_UE_sched_ctrl_t *sched_ctrl, int rnti); +void nr_mac_release_ue(gNB_MAC_INST *mac, int rnti); void nr_mac_trigger_ul_failure(NR_UE_sched_ctrl_t *sched_ctrl, NR_SubcarrierSpacing_t subcarrier_spacing); void nr_mac_reset_ul_failure(NR_UE_sched_ctrl_t *sched_ctrl); diff --git a/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c b/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c index ea0d08e152..75357ef4ae 100644 --- a/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c +++ b/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c @@ -466,27 +466,24 @@ void ue_context_release_command(const f1ap_ue_context_release_cmd_t *cmd) { /* mark UE as to be deleted after PUSCH failure */ gNB_MAC_INST *mac = RC.nrmac[0]; - pthread_mutex_lock(&mac->sched_lock); + NR_SCHED_LOCK(&mac->sched_lock); NR_UE_info_t *UE = find_nr_UE(&mac->UE_info, cmd->gNB_DU_ue_id); + if (UE == NULL) { + LOG_E(MAC, "ERROR: unknown UE with RNTI %04x, ignoring UE Context Release Command\n", cmd->gNB_DU_ue_id); + NR_SCHED_UNLOCK(&mac->sched_lock); + return; + } + if (UE->UE_sched_ctrl.ul_failure || cmd->rrc_container_length == 0) { /* The UE is already not connected anymore or we have nothing to forward*/ - nr_rlc_remove_ue(cmd->gNB_DU_ue_id); - mac_remove_nr_ue(mac, cmd->gNB_DU_ue_id); + nr_mac_release_ue(mac, cmd->gNB_DU_ue_id); } else { /* UE is in sync: forward release message and mark to be deleted * after UL failure */ nr_rlc_srb_recv_sdu(cmd->gNB_DU_ue_id, cmd->srb_id, cmd->rrc_container, cmd->rrc_container_length); nr_mac_trigger_release_timer(&UE->UE_sched_ctrl, UE->current_UL_BWP.scs); } - pthread_mutex_unlock(&mac->sched_lock); - - f1ap_ue_context_release_complete_t complete = { - .gNB_CU_ue_id = cmd->gNB_CU_ue_id, - .gNB_DU_ue_id = cmd->gNB_DU_ue_id, - }; - mac->mac_rrc.ue_context_release_complete(&complete); - - du_remove_f1_ue_data(cmd->gNB_DU_ue_id); + NR_SCHED_UNLOCK(&mac->sched_lock); } void dl_rrc_message_transfer(const f1ap_dl_rrc_message_t *dl_rrc) From 102cbf0d0c88159bac8908bb7aaa22a1f590a2f6 Mon Sep 17 00:00:00 2001 From: Robert Schmidt Date: Fri, 3 Nov 2023 13:54:36 +0100 Subject: [PATCH 2/3] Prevent possible deadlock between scheduler and F1 UE ID --- openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c index 7c4322f654..501eb963c0 100644 --- a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c +++ b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_primitives.c @@ -2822,6 +2822,9 @@ void nr_mac_release_ue(gNB_MAC_INST *mac, int rnti) // MAC context, decode the PDU, find the C-RNTI MAC CE, and then throw the // newly created context away. See also in _nr_rx_sdu() and commit 93f59a3c6e56f if (du_exists_f1_ue_data(rnti)) { + // unlock the scheduler temporarily to prevent possible deadlocks with + // du_remove_f1_ue_data() (and also while sending the message to RRC) + NR_SCHED_UNLOCK(&mac->sched_lock); f1_ue_data_t ue_data = du_get_f1_ue_data(rnti); f1ap_ue_context_release_complete_t complete = { .gNB_CU_ue_id = ue_data.secondary_ue, @@ -2830,6 +2833,7 @@ void nr_mac_release_ue(gNB_MAC_INST *mac, int rnti) mac->mac_rrc.ue_context_release_complete(&complete); du_remove_f1_ue_data(rnti); + NR_SCHED_LOCK(&mac->sched_lock); } } From 31ed25acd7a725eb0d6a6344f709a0eb74d4d88a Mon Sep 17 00:00:00 2001 From: Robert Schmidt Date: Fri, 3 Nov 2023 14:43:06 +0100 Subject: [PATCH 3/3] Only print UE caps if DU available The DU UE capability printing depends on data on SCS and band, which currently is only stored with the DU. To avoid any assert, we check that the DU is present. A follow-up commit will rework this. --- openair2/RRC/NR/rrc_gNB.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c index 027fc69862..47b1a770be 100644 --- a/openair2/RRC/NR/rrc_gNB.c +++ b/openair2/RRC/NR/rrc_gNB.c @@ -2499,10 +2499,6 @@ static void write_rrc_stats(const gNB_RRC_INST *rrc) { const gNB_RRC_UE_t *ue_ctxt = &ue_context_p->ue_context; f1_ue_data_t ue_data = cu_get_f1_ue_data(ue_ctxt->rrc_ue_id); - /* currently, we support only one DU. If we support multiple, need to - * search for the DU corresponding to this UE here */ - const nr_rrc_du_container_t *du = rrc->du; - DevAssert(du != NULL); fprintf(f, "UE %d CU UE ID %d DU UE ID %d RNTI %04x random identity %016lx", @@ -2524,7 +2520,10 @@ static void write_rrc_stats(const gNB_RRC_INST *rrc) fprintf(f, " PDU session %d ID %d status %s\n", nb_pdu, pdu->param.pdusession_id, get_pdusession_status_text(pdu->status)); } - if (ue_ctxt->UE_Capability_nr) { + /* currently, we support only one DU. If we support multiple, need to + * search for the DU corresponding to this UE here */ + const nr_rrc_du_container_t *du = rrc->du; + if (du != NULL && ue_ctxt->UE_Capability_nr) { AssertFatal(du->setup_req->num_cells_available == 1, "only one cell supported at the moment\n"); const f1ap_served_cell_info_t *cell_info = &du->setup_req->cell[0].info; fprintf(f,