From f247a6cc2eeb2b3fc4169d88418a59b49a8a548e Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Fri, 13 Feb 2026 13:35:28 +0100 Subject: [PATCH 1/2] fix(RRC): add PCell to serving_cells in fallback path and refactor update logic Fix assertion failure in rrc_gNB_send_NGAP_NAS_FIRST_REQ() when RRC Reestablishment falls back to RRC Setup. The fallback path was not adding the PCell to the UE's serving_cells array, causing rrc_get_pcell_for_ue() to return NULL. Major changes: - Add rrc_update_ue_pcell() helper function that handles PCell updates for both new UEs (fallback) and existing UEs (normal reestablishment) - Refactor normal reestablishment path to use rrc_update_ue_pcell() helper instead of manual cleanup + add sequence - Add PCell update to fallback_rrc_setup path using the same helper - Move C-RNTI validation after current_cell fetch to ensure current_cell is initialized before fallback_rrc_setup label - Change error handling: return instead of fallback when PCell add fails in normal reestablishment path --- openair2/RRC/NR/rrc_gNB.c | 45 ++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c index a68ac6691e..50a8ed16e1 100644 --- a/openair2/RRC/NR/rrc_gNB.c +++ b/openair2/RRC/NR/rrc_gNB.c @@ -1490,6 +1490,26 @@ static const nr_rrc_cell_container_t *get_previous_cell_by_pci_in_du(gNB_RRC_INS return rrc_get_cell_by_pci_for_du(&du->cells, pci); } +/** @brief Update UE's PCell in serving_cells array + * @param[in] UE UE context + * @param[in] cell Cell container to set as PCell + * @return Pointer to added serving cell entry, or NULL on failure + * @note If UE already has a PCell, removes all serving cells from the old PCell's DU first */ +static ue_serving_cell_t *rrc_update_ue_pcell(gNB_RRC_UE_t *UE, const nr_rrc_cell_container_t *cell) +{ + DevAssert(UE != NULL); + DevAssert(cell != NULL); + + // If UE already has a PCell, remove all serving cells from that DU first + const ue_serving_cell_t *existing_pcell = ue_get_pcell_entry(UE); + if (existing_pcell != NULL) { + rrc_remove_ue_scells_from_du(UE, existing_pcell->assoc_id); + } + + // Add the new PCell + return rrc_add_ue_serving_cell(UE, cell, RRC_PCELL_INDEX); +} + static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc, sctp_assoc_t assoc_id, const NR_RRCReestablishmentRequest_IEs_t *req, @@ -1501,6 +1521,7 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc, long ngap_cause = NGAP_CAUSE_RADIO_NETWORK_UNSPECIFIED; /* cause in case of NGAP release req */ const rnti_t old_rnti = req->ue_Identity.c_RNTI; rrc_gNB_ue_context_t *ue_context_p = NULL; + ue_serving_cell_t *added = NULL; LOG_I(NR_RRC, "Reestablishment RNTI %04x req C-RNTI %04x physCellId %ld cause %s\n", msg->crnti, @@ -1514,12 +1535,6 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc, return; } - // Validate C-RNTI range (3GPP TS 38.321 version 15.13.0 Section 7.1 Table 7.1-1) - if (old_rnti < 0x1 || old_rnti > 0xffef) { - LOG_E(NR_RRC, "NR_RRCReestablishmentRequest c_RNTI %04x range error, fallback to RRC setup\n", old_rnti); - goto fallback_rrc_setup; - } - // Fetch current cell: where the reestablishment request was received nr_rrc_cell_container_t *current_cell = get_cell_by_cell_id(&rrc->cells, msg->nr_cellid); if (current_cell == NULL || current_cell->assoc_id != du->assoc_id) { @@ -1530,6 +1545,12 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc, return; } + // Validate C-RNTI range (3GPP TS 38.321 version 15.13.0 Section 7.1 Table 7.1-1) + if (old_rnti < 0x1 || old_rnti > 0xffef) { + LOG_E(NR_RRC, "NR_RRCReestablishmentRequest c_RNTI %04x range error, fallback to RRC setup\n", old_rnti); + goto fallback_rrc_setup; + } + if (current_cell->mtc == NULL) { // some UEs don't send MeasurementTimingConfiguration, so we don't know the // SSB ARFCN and can't do reestablishment. handle it gracefully by doing @@ -1660,14 +1681,10 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc, /* Update PCell in serving_cells array */ DevAssert(cell->info.cell_id == msg->nr_cellid); - // Reset the serving set to the new PCell, remove all serving cells from the current PCell's DU. - const ue_serving_cell_t *existing_pcell = ue_get_pcell_entry(UE); - DevAssert(existing_pcell != NULL); - rrc_remove_ue_scells_from_du(UE, existing_pcell->assoc_id); - ue_serving_cell_t *added = rrc_add_ue_serving_cell(UE, cell, RRC_PCELL_INDEX); + added = rrc_update_ue_pcell(UE, cell); if (added == NULL) { - LOG_E(NR_RRC, "Reestablishment: failed to add PCell (cell %ld), rejecting reestablishment\n", cell->info.cell_id); - goto fallback_rrc_setup; + LOG_E(NR_RRC, "Reestablishment: failed to add PCell (cell %ld)\n", cell->info.cell_id); + return; } ue_data.secondary_ue = msg->gNB_DU_ue_id; @@ -1688,6 +1705,8 @@ fallback_rrc_setup: rrc_gNB_ue_context_t *new = rrc_gNB_create_ue_context(assoc_id, msg->crnti, rrc, random_value, msg->gNB_DU_ue_id); activate_srb(&new->ue_context, 1); + added = rrc_update_ue_pcell(&new->ue_context, current_cell); + DevAssert(added); rrc_gNB_generate_RRCSetup(0, msg->crnti, new, msg->du2cu_rrc_container, msg->du2cu_rrc_container_length); return; } From 372f121cf6e0f20cacb7f13dd839efdb19609edb Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Mon, 16 Feb 2026 10:26:21 +0100 Subject: [PATCH 2/2] Refactor (RRC): extract rrc_update_ue_pcell() to public API and adopt consistently Move rrc_update_ue_pcell() from static function in rrc_gNB.c to public function in rrc_cell_management.c, making it reusable across RRC code paths. Replace duplicate PCell addition logic in handover paths (intra-CU and inter-CU) and RRCSetup path with calls to this shared function. This eliminates code duplication between handover and RRCSetup paths, centralizes PCell update logic (including cleanup of old PCell's DU serving cells) in a single reusable function and makes the function available for future use in other RRC procedures --- openair2/RRC/NR/rrc_cell_management.c | 19 ++++++++++++++ openair2/RRC/NR/rrc_cell_management.h | 1 + openair2/RRC/NR/rrc_gNB.c | 29 +++++--------------- openair2/RRC/NR/rrc_gNB_mobility.c | 38 +++++++++------------------ 4 files changed, 39 insertions(+), 48 deletions(-) diff --git a/openair2/RRC/NR/rrc_cell_management.c b/openair2/RRC/NR/rrc_cell_management.c index 0df2665c04..844ee6ffdc 100644 --- a/openair2/RRC/NR/rrc_cell_management.c +++ b/openair2/RRC/NR/rrc_cell_management.c @@ -409,3 +409,22 @@ nr_rrc_cell_container_t *rrc_get_pcell_for_ue(gNB_RRC_INST *rrc, const gNB_RRC_U return get_cell_by_cell_id(&rrc->cells, pcell_entry->nci); } +/** @brief Update UE's PCell in serving_cells array + * @param[in] UE UE context + * @param[in] cell Cell container to set as PCell + * @return Pointer to added serving cell entry, or NULL on failure + * @note If UE already has a PCell, removes all serving cells from the old PCell's DU first */ +ue_serving_cell_t *rrc_update_ue_pcell(gNB_RRC_UE_t *UE, const nr_rrc_cell_container_t *cell) +{ + DevAssert(UE != NULL); + DevAssert(cell != NULL); + + // If UE already has a PCell, remove all serving cells from that DU first + const ue_serving_cell_t *existing_pcell = ue_get_pcell_entry(UE); + if (existing_pcell != NULL) { + rrc_remove_ue_scells_from_du(UE, existing_pcell->assoc_id); + } + + // Add the new PCell + return rrc_add_ue_serving_cell(UE, cell, RRC_PCELL_INDEX); +} diff --git a/openair2/RRC/NR/rrc_cell_management.h b/openair2/RRC/NR/rrc_cell_management.h index 99071cceb2..d24fd095dd 100644 --- a/openair2/RRC/NR/rrc_cell_management.h +++ b/openair2/RRC/NR/rrc_cell_management.h @@ -69,5 +69,6 @@ void rrc_remove_ue_scells_from_du(gNB_RRC_UE_t *ue, sctp_assoc_t assoc_id); ue_serving_cell_t *rrc_get_ue_serving_cell_by_id(const gNB_RRC_UE_t *ue, uint8_t serving_cell_id); const ue_serving_cell_t *ue_get_pcell_entry(const gNB_RRC_UE_t *ue); nr_rrc_cell_container_t *rrc_get_pcell_for_ue(gNB_RRC_INST *rrc, const gNB_RRC_UE_t *ue); +ue_serving_cell_t *rrc_update_ue_pcell(gNB_RRC_UE_t *UE, const nr_rrc_cell_container_t *cell); #endif /* RRC_CELL_MANAGEMENT_H_ */ diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c index 50a8ed16e1..704317febe 100644 --- a/openair2/RRC/NR/rrc_gNB.c +++ b/openair2/RRC/NR/rrc_gNB.c @@ -1416,8 +1416,13 @@ static void rrc_handle_RRCSetupRequest(gNB_RRC_INST *rrc, rrc_gNB_generate_RRCReject(rrc, ue_context_p); return; } - /* Add PCell to serving_cells array */ - rrc_add_ue_serving_cell(UE, cell, RRC_PCELL_INDEX); + /* Update PCell in serving_cells array */ + ue_serving_cell_t *added = rrc_update_ue_pcell(UE, cell); + if (added == NULL) { + LOG_E(NR_RRC, "RRCSetup: failed to add PCell (cell %ld)\n", cell->info.cell_id); + rrc_gNB_generate_RRCReject(rrc, ue_context_p); + return; + } UE->ongoing_reconfiguration = false; UE->measConfig = nr_rrc_get_measconfig(rrc, msg->nr_cellid); activate_srb(UE, 1); @@ -1490,26 +1495,6 @@ static const nr_rrc_cell_container_t *get_previous_cell_by_pci_in_du(gNB_RRC_INS return rrc_get_cell_by_pci_for_du(&du->cells, pci); } -/** @brief Update UE's PCell in serving_cells array - * @param[in] UE UE context - * @param[in] cell Cell container to set as PCell - * @return Pointer to added serving cell entry, or NULL on failure - * @note If UE already has a PCell, removes all serving cells from the old PCell's DU first */ -static ue_serving_cell_t *rrc_update_ue_pcell(gNB_RRC_UE_t *UE, const nr_rrc_cell_container_t *cell) -{ - DevAssert(UE != NULL); - DevAssert(cell != NULL); - - // If UE already has a PCell, remove all serving cells from that DU first - const ue_serving_cell_t *existing_pcell = ue_get_pcell_entry(UE); - if (existing_pcell != NULL) { - rrc_remove_ue_scells_from_du(UE, existing_pcell->assoc_id); - } - - // Add the new PCell - return rrc_add_ue_serving_cell(UE, cell, RRC_PCELL_INDEX); -} - static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc, sctp_assoc_t assoc_id, const NR_RRCReestablishmentRequest_IEs_t *req, diff --git a/openair2/RRC/NR/rrc_gNB_mobility.c b/openair2/RRC/NR/rrc_gNB_mobility.c index d18937fbc5..4d2785d9a0 100644 --- a/openair2/RRC/NR/rrc_gNB_mobility.c +++ b/openair2/RRC/NR/rrc_gNB_mobility.c @@ -453,36 +453,22 @@ bool nr_rrc_update_cell_assoc_after_ho(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE) return false; } - if (UE->ho_context->source) { - /* F1 HO (intra-CU): migrate serving cells from source DU to target cell */ - nr_ho_source_cu_t *source_ctx = UE->ho_context->source; - - // Remove SCells that belong to the source DU during handover (cell group changes) - // This also removes PCell if it was on the source DU - rrc_remove_ue_scells_from_du(UE, source_ctx->cell->assoc_id); - - // Update PCell in serving_cells - ue_serving_cell_t *added = rrc_add_ue_serving_cell(UE, target_ctx->cell, RRC_PCELL_INDEX); - if (added == NULL) { - LOG_E(NR_RRC, - "UE %d handover: failed to add PCell (cell %ld) to serving cells\n", - UE->rrc_ue_id, - target_ctx->cell->info.cell_id); - return false; - } - } else { + if (!UE->ho_context->source) { /* Inter-CU (N2/Xn target): this UE context has no serving cells yet */ const ue_serving_cell_t *existing_pcell = ue_get_pcell_entry(UE); AssertFatal(existing_pcell == NULL, "UE context should not have PCell yet (inter-CU target)\n"); + } - ue_serving_cell_t *added = rrc_add_ue_serving_cell(UE, target_ctx->cell, RRC_PCELL_INDEX); - if (added == NULL) { - LOG_E(NR_RRC, - "UE %d handover: failed to add PCell (cell %ld) to serving cells\n", - UE->rrc_ue_id, - target_ctx->cell->info.cell_id); - return false; - } + /* Update PCell: removes all serving cells from existing PCell's DU (if any) + * and adds the target cell as the new PCell. For F1 HO (intra-CU), this + * migrates serving cells from source DU to target DU. */ + ue_serving_cell_t *added = rrc_update_ue_pcell(UE, target_ctx->cell); + if (added == NULL) { + LOG_E(NR_RRC, + "UE %d handover: failed to add PCell (cell %ld) to serving cells\n", + UE->rrc_ue_id, + target_ctx->cell->info.cell_id); + return false; } return true;