From 1fea6108a8c546f9e33add264e1f078a669f49ee Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Wed, 15 Oct 2025 12:27:34 +0200 Subject: [PATCH] RRC: unify NG delay control before calling the RRC NGAP handler - Add delayed_action_state_t in the UE context for ongoing delayed action - Move delay check and timer re-enqueue to rrc thread in rrc_gNB.c (rrc_delay_transaction) - Call delay before NGAP setup/release handlers; remove local delay logic from rrc_gNB_NGAP.c - Clear on RRCReconfigurationComplete (reset_delayed_action) Closes #960 --- openair2/RRC/NR/nr_rrc_defs.h | 9 ++- openair2/RRC/NR/nr_rrc_proto.h | 1 + openair2/RRC/NR/rrc_gNB.c | 102 +++++++++++++++++++++++++-- openair2/RRC/NR/rrc_gNB_NGAP.c | 46 +----------- openair2/RRC/NR/rrc_gNB_UE_context.c | 2 - 5 files changed, 108 insertions(+), 52 deletions(-) diff --git a/openair2/RRC/NR/nr_rrc_defs.h b/openair2/RRC/NR/nr_rrc_defs.h index e850ce373a..4d24a771c0 100644 --- a/openair2/RRC/NR/nr_rrc_defs.h +++ b/openair2/RRC/NR/nr_rrc_defs.h @@ -148,6 +148,12 @@ typedef enum { RRC_UECAPABILITY_ENQUIRY, } rrc_action_t; +/* Small state for delaying NG-triggered actions (setup/release) */ +typedef struct { + int max_delays; + bool ongoing_transaction; +} delayed_action_state_t; + typedef struct nr_redcap_ue_cap { bool support_of_redcap_r17; bool support_of_16drb_redcap_r17; @@ -241,8 +247,7 @@ typedef struct gNB_RRC_UE_s { byte_array_t nas_pdu; /* hack, see rrc_gNB_process_NGAP_PDUSESSION_SETUP_REQ() for more info */ - int max_delays_pdu_session; - bool ongoing_pdusession_setup_request; + delayed_action_state_t delayed_action; nr_redcap_ue_cap_t *redcap_cap; } gNB_RRC_UE_t; diff --git a/openair2/RRC/NR/nr_rrc_proto.h b/openair2/RRC/NR/nr_rrc_proto.h index 2fc7bf5e5e..a36a0778c6 100644 --- a/openair2/RRC/NR/nr_rrc_proto.h +++ b/openair2/RRC/NR/nr_rrc_proto.h @@ -122,5 +122,6 @@ NR_SRB_ToAddModList_t *createSRBlist(gNB_RRC_UE_t *ue, uint8_t reestablish); NR_DRB_ToAddModList_t *createDRBlist(gNB_RRC_UE_t *ue, bool reestablish, bool do_integrity, bool do_ciphering); void activate_srb(gNB_RRC_UE_t *UE, int srb_id); void e1_notify_pdcp_status(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, const ngap_drb_status_t *drb_status); +void init_delayed_action(delayed_action_state_t *delayed_action); #endif diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c index a6c24fc579..b18e820e91 100644 --- a/openair2/RRC/NR/rrc_gNB.c +++ b/openair2/RRC/NR/rrc_gNB.c @@ -115,6 +115,97 @@ mui_t rrc_gNB_mui = 0; +/* Per-transaction max_delays counter to limit retry attempts */ +#define MAX_DELAYS 100 + +/** @brief clone and re-enqueue an NGAP message after delaying + * delays the ongoing transaction (in msg_p) by setting a timer to wait + * 10ms; upon expiry, delivers to RRC, which sends the message to itself */ +static void delay_transaction(MessageDef *msg_p, int wait_us) +{ + MessagesIds id = ITTI_MSG_ID(msg_p); + AssertFatal(id == NGAP_PDUSESSION_SETUP_REQ || id == NGAP_PDUSESSION_RELEASE_COMMAND, + "delay_transaction(): unsupported message id %d\n", + id); + + MessageDef *new = itti_alloc_new_message(TASK_RRC_GNB, 0, id); + + // Copy only the specific message struct, not the entire union. + // The union (msg_t) contains all message types and is much larger than + // the allocated space (which is sized for the specific message type only). + if (id == NGAP_PDUSESSION_SETUP_REQ) { + NGAP_PDUSESSION_SETUP_REQ(new) = NGAP_PDUSESSION_SETUP_REQ(msg_p); + } else if (id == NGAP_PDUSESSION_RELEASE_COMMAND) { + NGAP_PDUSESSION_RELEASE_COMMAND(new) = NGAP_PDUSESSION_RELEASE_COMMAND(msg_p); + } + + int instance = msg_p->ittiMsgHeader.originInstance; + long timer_id; + timer_setup(0, wait_us, TASK_RRC_GNB, instance, TIMER_ONE_SHOT, new, &timer_id); +} + +static void reset_delayed_action(delayed_action_state_t *delayed_action) +{ + delayed_action->ongoing_transaction = false; + delayed_action->max_delays = 0; +} + +void init_delayed_action(delayed_action_state_t *delayed_action) +{ + delayed_action->ongoing_transaction = true; + delayed_action->max_delays = MAX_DELAYS; +} + +/* \brief checks if any transaction is ongoing for any xid of this UE */ +static bool transaction_ongoing(const gNB_RRC_UE_t *UE) +{ + for (int xid = 0; xid < NR_RRC_TRANSACTION_IDENTIFIER_NUMBER; ++xid) { + if (UE->xids[xid] != RRC_ACTION_NONE) + return true; + } + return false; +} + +/** @brief delay control: returns true if delayed, false if should proceed + * This is a hack. We observed that with some UEs, PDU session requests might + * come in quick succession, faster than the RRC reconfiguration for the PDU + * session requests can be carried out (UE is doing reconfig, and second PDU + * session request arrives). We don't have currently the means to "queue up" + * these transactions, which would probably involve some rework of the RRC. + * To still allow these requests to come in and succeed, we below check and delay transactions + * for 10ms. However, to not accidentally end up in infinite loops, the + * maximum number is capped on a per-UE basis as indicated in variable + * max_delays_pdu_session. See commit 277f8da0 for more details. */ +static bool rrc_delay_transaction(instance_t instance, MessageDef *msg_p) +{ + uint32_t cu_ue_id = 0; + if (ITTI_MSG_ID(msg_p) == NGAP_PDUSESSION_SETUP_REQ) { + cu_ue_id = NGAP_PDUSESSION_SETUP_REQ(msg_p).gNB_ue_ngap_id; + } else if (ITTI_MSG_ID(msg_p) == NGAP_PDUSESSION_RELEASE_COMMAND) { + cu_ue_id = NGAP_PDUSESSION_RELEASE_COMMAND(msg_p).gNB_ue_ngap_id; + } + AssertFatal(cu_ue_id > 0, "cu_ue_id not found in message %s\n", ITTI_MSG_NAME(msg_p)); + + rrc_gNB_ue_context_t *ue_context_p = rrc_gNB_get_ue_context(RC.nrrrc[instance], cu_ue_id); + DevAssert(ue_context_p); + + gNB_RRC_UE_t *UE = &ue_context_p->ue_context; + bool delay = UE->delayed_action.ongoing_transaction && UE->delayed_action.max_delays > 0; + /* Check if any PDU session action is ongoing */ + if (delay || transaction_ongoing(UE)) { + int wait_us = 10000; + LOG_I(NR_RRC, + "UE %d: ongoing transaction, delaying incoming transaction by %d us\n", + UE->rrc_ue_id, + wait_us); + delay_transaction(msg_p, wait_us); + UE->delayed_action.max_delays--; + return true; /* delayed */ + } + LOG_D(NR_RRC, "UE %d: no delayed action ongoing, proceeding with incoming transaction\n", UE->rrc_ue_id); + return false; /* not delayed */ +} + typedef struct deliver_ue_ctxt_release_data_t { gNB_RRC_INST *rrc; f1ap_ue_context_rel_cmd_t *release_cmd; @@ -1856,6 +1947,7 @@ static void handle_rrcReconfigurationComplete(gNB_RRC_INST *rrc, gNB_RRC_UE_t *U switch (UE->xids[xid]) { case RRC_PDUSESSION_RELEASE: { rrc_gNB_send_NGAP_PDUSESSION_RELEASE_RESPONSE(rrc, UE, xid); + reset_delayed_action(&UE->delayed_action); } break; case RRC_PDUSESSION_ESTABLISH: if (UE->n_initial_pdu > 0) { @@ -1870,6 +1962,7 @@ static void handle_rrcReconfigurationComplete(gNB_RRC_INST *rrc, gNB_RRC_UE_t *U LOG_W(NR_RRC, "UE %d: RRC Reconfiguration Complete for PDU session establishment, but no PDU sessions were setup\n", UE->rrc_ue_id); + reset_delayed_action(&UE->delayed_action); break; case RRC_PDUSESSION_MODIFY: rrc_gNB_send_NGAP_PDUSESSION_MODIFY_RESP(rrc, UE, xid); @@ -1886,9 +1979,6 @@ static void handle_rrcReconfigurationComplete(gNB_RRC_INST *rrc, gNB_RRC_UE_t *U break; } - if (UE->xids[xid] == RRC_PDUSESSION_ESTABLISH) - UE->ongoing_pdusession_setup_request = false; - UE->xids[xid] = RRC_ACTION_NONE; for (int i = 0; i < NR_RRC_TRANSACTION_IDENTIFIER_NUMBER; ++i) { if (UE->xids[i] != RRC_ACTION_NONE) { @@ -3005,7 +3095,8 @@ void *rrc_gnb_task(void *args_p) { break; case NGAP_PDUSESSION_SETUP_REQ: - rrc_gNB_process_NGAP_PDUSESSION_SETUP_REQ(msg_p, instance); + if (!rrc_delay_transaction(instance, msg_p)) + rrc_gNB_process_NGAP_PDUSESSION_SETUP_REQ(msg_p, instance); break; case NGAP_PDUSESSION_MODIFY_REQ: @@ -3013,7 +3104,8 @@ void *rrc_gnb_task(void *args_p) { break; case NGAP_PDUSESSION_RELEASE_COMMAND: - rrc_gNB_process_NGAP_PDUSESSION_RELEASE_COMMAND(&NGAP_PDUSESSION_RELEASE_COMMAND(msg_p), RC.nrrrc[instance]); + if (!rrc_delay_transaction(instance, msg_p)) + rrc_gNB_process_NGAP_PDUSESSION_RELEASE_COMMAND(&NGAP_PDUSESSION_RELEASE_COMMAND(msg_p), RC.nrrrc[instance]); break; case NGAP_DL_RAN_STATUS_TRANSFER: diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index 7bcecb2533..8ba7b3e950 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -789,29 +789,6 @@ void rrc_gNB_send_NGAP_PDUSESSION_SETUP_RESP(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE return; } -/* \brief checks if any transaction is ongoing for any xid of this UE */ -static bool transaction_ongoing(const gNB_RRC_UE_t *UE) -{ - for (int xid = 0; xid < NR_RRC_TRANSACTION_IDENTIFIER_NUMBER; ++xid) { - if (UE->xids[xid] != RRC_ACTION_NONE) - return true; - } - return false; -} - -/* \brief delays the ongoing transaction (in msg_p) by setting a timer to wait - * 10ms; upon expiry, delivers to RRC, which sends the message to itself */ -static void delay_transaction(MessageDef *msg_p, int wait_us) -{ - MessageDef *new = itti_alloc_new_message(TASK_RRC_GNB, 0, NGAP_PDUSESSION_SETUP_REQ); - ngap_pdusession_setup_req_t *n = &NGAP_PDUSESSION_SETUP_REQ(new); - *n = NGAP_PDUSESSION_SETUP_REQ(msg_p); - - int instance = msg_p->ittiMsgHeader.originInstance; - long timer_id; - timer_setup(0, wait_us, TASK_RRC_GNB, instance, TIMER_ONE_SHOT, new, &timer_id); -} - /** * @brief Fill PDU Session Resource Setup Response with a list of PDU Session Resources Failed to Setup * and send ITTI message to TASK_NGAP @@ -883,25 +860,6 @@ void rrc_gNB_process_NGAP_PDUSESSION_SETUP_REQ(MessageDef *msg_p, instance_t ins UE->amf_ue_ngap_id = msg->amf_ue_ngap_id; - /* This is a hack. We observed that with some UEs, PDU session requests might - * come in quick succession, faster than the RRC reconfiguration for the PDU - * session requests can be carried out (UE is doing reconfig, and second PDU - * session request arrives). We don't have currently the means to "queue up" - * these transactions, which would probably involve some rework of the RRC. - * To still allow these requests to come in and succeed, we below check and delay transactions - * for 10ms. However, to not accidentally end up in infinite loops, the - * maximum number is capped on a per-UE basis as indicated in variable - * max_delays_pdu_session. */ - if (!UE->ongoing_pdusession_setup_request) - UE->max_delays_pdu_session = 100; - - if (UE->max_delays_pdu_session > 0 && (transaction_ongoing(UE) || UE->ongoing_pdusession_setup_request)) { - int wait_us = 10000; - LOG_I(RRC, "UE %d: delay PDU session setup by %d us, pending %d retries\n", UE->rrc_ue_id, wait_us, UE->max_delays_pdu_session); - delay_transaction(msg_p, wait_us); - UE->max_delays_pdu_session--; - return; - } pdusession_t to_setup[NGAP_MAX_PDU_SESSION] = {0}; for (int i = 0; i < msg->nb_pdusessions_tosetup; ++i) @@ -916,7 +874,8 @@ void rrc_gNB_process_NGAP_PDUSESSION_SETUP_REQ(MessageDef *msg_p, instance_t ins send_ngap_pdu_session_setup_resp_fail(instance, msg, cause); rrc_forward_ue_nas_message(rrc, UE); } else { - UE->ongoing_pdusession_setup_request = true; + // Set ongoing_transaction flag to true + init_delayed_action(&UE->delayed_action); } } @@ -1630,6 +1589,7 @@ int rrc_gNB_process_NGAP_PDUSESSION_RELEASE_COMMAND(ngap_pdusession_release_comm sctp_assoc_t assoc_id = get_existing_cuup_for_ue(rrc, UE); rrc->cucp_cuup.bearer_context_mod(assoc_id, &req); } + init_delayed_action(&UE->delayed_action); return 0; } diff --git a/openair2/RRC/NR/rrc_gNB_UE_context.c b/openair2/RRC/NR/rrc_gNB_UE_context.c index 9a2c3fb6f6..86838927ab 100644 --- a/openair2/RRC/NR/rrc_gNB_UE_context.c +++ b/openair2/RRC/NR/rrc_gNB_UE_context.c @@ -218,8 +218,6 @@ rrc_gNB_ue_context_t *rrc_gNB_create_ue_context(sctp_assoc_t assoc_id, ue->rrc_ue_id); bool success = cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data); DevAssert(success); - ue->max_delays_pdu_session = 20; /* see rrc_gNB_process_NGAP_PDUSESSION_SETUP_REQ() */ - ue->ongoing_pdusession_setup_request = false; // Initialise setup PDU Sessions list seq_arr_init(&ue->pduSessions, sizeof(rrc_pdu_session_param_t));