From 253fffe7fb699c39c34d6c13f3b9fed93f95a07a Mon Sep 17 00:00:00 2001
From: Robert Schmidt <robert.schmidt@openairinterface.org>
Date: Thu, 16 Jan 2025 15:15:47 +0100
Subject: [PATCH 1/7] F1AP IDs: refactor common code

---
 openair2/F1AP/f1ap_ids.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/openair2/F1AP/f1ap_ids.c b/openair2/F1AP/f1ap_ids.c
index 64d4b0b938..7dba047a1a 100644
--- a/openair2/F1AP/f1ap_ids.c
+++ b/openair2/F1AP/f1ap_ids.c
@@ -29,6 +29,14 @@
 #include "common/utils/assertions.h"
 
 
+static f1_ue_data_t *get_hashtable_data(hash_table_t *ht, uint64_t ue_id)
+{
+  void *data = NULL;
+  hashtable_rc_t ret = hashtable_get(ht, ue_id, &data);
+  AssertFatal(ret == HASH_TABLE_OK && data != NULL, "element for ue_id %ld not found\n", ue_id);
+  return data;
+}
+
 /* we have separate versions for CU and DU, as both CU&DU might coexist in the
  * same process */
 static hash_table_t *cu2du_ue_mapping;
@@ -74,11 +82,7 @@ f1_ue_data_t cu_get_f1_ue_data(uint32_t ue_id)
 {
   pthread_mutex_lock(&cu2du_mutex);
   DevAssert(cu2du_ue_mapping != NULL);
-  uint64_t key = ue_id;
-  void *data = NULL;
-  hashtable_rc_t ret = hashtable_get(cu2du_ue_mapping, key, &data);
-  AssertFatal(ret == HASH_TABLE_OK && data != NULL, "element for ue_id %d not found\n", ue_id);
-  f1_ue_data_t ued = *(f1_ue_data_t *)data;
+  f1_ue_data_t ued = *get_hashtable_data(cu2du_ue_mapping, ue_id);
   pthread_mutex_unlock(&cu2du_mutex);
   return ued;
 }
@@ -137,11 +141,7 @@ f1_ue_data_t du_get_f1_ue_data(uint32_t ue_id)
 {
   pthread_mutex_lock(&du2cu_mutex);
   DevAssert(du2cu_ue_mapping != NULL);
-  uint64_t key = ue_id;
-  void *data = NULL;
-  hashtable_rc_t ret = hashtable_get(du2cu_ue_mapping, key, &data);
-  AssertFatal(ret == HASH_TABLE_OK && data != NULL, "element for ue_id %d not found\n", ue_id);
-  f1_ue_data_t ued = *(f1_ue_data_t *)data;
+  f1_ue_data_t ued = *get_hashtable_data(du2cu_ue_mapping, ue_id);
   pthread_mutex_unlock(&du2cu_mutex);
   return ued;
 }

From e2bf9fb0a4dabb84a118b25808ab15e921a1ad1e Mon Sep 17 00:00:00 2001
From: Robert Schmidt <robert.schmidt@openairinterface.org>
Date: Thu, 16 Jan 2025 15:21:52 +0100
Subject: [PATCH 2/7] F1AP IDs: refactor common insert

---
 openair2/F1AP/f1ap_ids.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/openair2/F1AP/f1ap_ids.c b/openair2/F1AP/f1ap_ids.c
index 7dba047a1a..009517510e 100644
--- a/openair2/F1AP/f1ap_ids.c
+++ b/openair2/F1AP/f1ap_ids.c
@@ -37,6 +37,15 @@ static f1_ue_data_t *get_hashtable_data(hash_table_t *ht, uint64_t ue_id)
   return data;
 }
 
+static bool add_hashtable_data(hash_table_t *ht, uint64_t ue_id, const f1_ue_data_t *data)
+{
+  f1_ue_data_t *idata = malloc(sizeof(*idata));
+  AssertFatal(idata, "cannot allocate memory\n");
+  *idata = *data;
+  hashtable_rc_t ret = hashtable_insert(ht, ue_id, idata);
+  return ret == HASH_TABLE_OK;
+}
+
 /* we have separate versions for CU and DU, as both CU&DU might coexist in the
  * same process */
 static hash_table_t *cu2du_ue_mapping;
@@ -60,12 +69,9 @@ bool cu_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
     pthread_mutex_unlock(&cu2du_mutex);
     return false;
   }
-  f1_ue_data_t *idata = malloc(sizeof(*idata));
-  AssertFatal(idata, "cannot allocate memory\n");
-  *idata = *data;
-  hashtable_rc_t ret = hashtable_insert(cu2du_ue_mapping, key, idata);
+  bool ret = add_hashtable_data(cu2du_ue_mapping, key, data);
   pthread_mutex_unlock(&cu2du_mutex);
-  return ret == HASH_TABLE_OK;
+  return ret;
 }
 
 bool cu_exists_f1_ue_data(uint32_t ue_id)
@@ -119,12 +125,9 @@ bool du_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
     pthread_mutex_unlock(&du2cu_mutex);
     return false;
   }
-  f1_ue_data_t *idata = malloc(sizeof(*idata));
-  AssertFatal(idata, "cannot allocate memory\n");
-  *idata = *data;
-  hashtable_rc_t ret = hashtable_insert(du2cu_ue_mapping, key, idata);
+  bool ret = add_hashtable_data(du2cu_ue_mapping, key, data);
   pthread_mutex_unlock(&du2cu_mutex);
-  return ret == HASH_TABLE_OK;
+  return ret;
 }
 
 bool du_exists_f1_ue_data(uint32_t ue_id)

From c7dc71e66d5bd31dfdd48dd75ef8a6e4efb57b1f Mon Sep 17 00:00:00 2001
From: Robert Schmidt <robert.schmidt@openairinterface.org>
Date: Thu, 16 Jan 2025 15:38:20 +0100
Subject: [PATCH 3/7] Add test for CU F1AP ID data

---
 openair2/F1AP/f1ap_ids_test.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/openair2/F1AP/f1ap_ids_test.c b/openair2/F1AP/f1ap_ids_test.c
index 3007db566a..8c1860a946 100644
--- a/openair2/F1AP/f1ap_ids_test.c
+++ b/openair2/F1AP/f1ap_ids_test.c
@@ -25,7 +25,7 @@
 #include "common/utils/assertions.h"
 #include "f1ap_ids.h"
 
-int main()
+void du_data_test()
 {
   du_init_f1_ue_data();
   int rnti = 13;
@@ -38,5 +38,26 @@ int main()
   DevAssert(exists);
   f1_ue_data_t rdata = du_get_f1_ue_data(rnti);
   DevAssert(rdata.secondary_ue == data.secondary_ue);
+}
+
+void cu_data_test()
+{
+  cu_init_f1_ue_data();
+  int ue_id = 13;
+  f1_ue_data_t data = {.secondary_ue = 1};
+  bool ret = cu_add_f1_ue_data(ue_id, &data);
+  DevAssert(ret);
+  ret = cu_add_f1_ue_data(ue_id, &data);
+  DevAssert(!ret);
+  bool exists = cu_exists_f1_ue_data(ue_id);
+  DevAssert(exists);
+  f1_ue_data_t rdata = cu_get_f1_ue_data(ue_id);
+  DevAssert(rdata.secondary_ue == data.secondary_ue);
+}
+
+int main()
+{
+  du_data_test();
+  cu_data_test();
   return 0;
 }

From 6d5e29f5aee3961775e73096c40c5069c1308ce4 Mon Sep 17 00:00:00 2001
From: Robert Schmidt <robert.schmidt@openairinterface.org>
Date: Thu, 16 Jan 2025 16:50:58 +0100
Subject: [PATCH 4/7] F1AP IDs: Allow AddMod, fix concurrency bug

Modify xu_add_f1_ue_data() to be xu_addmod_f1_ue_data(), i.e., "addmod"
behavior in which it is possible to also update f1_ue_data. I do it this
way since an xu_updade_f1_ue_data() function would be almost the same as
the add function (only deleting existing data, whereas the original
add() was asserting in that case).

Modify tests to test for this behavior.

This fixes a concurrency bug. For instance, during reestablishment, the
CU needs to update the DU UE ID under which the UE is identified in the
DU. Previously, the CU would remove, then add the DU UE ID info.
At the same time, the PDCP thread might look up the information. This
can lead to asserts. Use the above addmod functionality to do the
remove-add under a mutex.
---
 openair2/F1AP/f1ap_ids.c                      | 12 +++++-----
 openair2/F1AP/f1ap_ids.h                      |  4 ++--
 openair2/F1AP/f1ap_ids_test.c                 | 24 ++++++++++++++-----
 .../LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c    |  4 ++--
 openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c   |  2 +-
 openair2/RRC/NR/rrc_gNB.c                     |  9 +++----
 openair2/RRC/NR/rrc_gNB_UE_context.c          |  2 +-
 openair2/RRC/NR/rrc_gNB_cuup.c                |  3 +--
 openair2/RRC/NR/rrc_gNB_du.c                  |  3 +--
 openair2/RRC/NR/rrc_gNB_nsa.c                 |  4 ++--
 10 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/openair2/F1AP/f1ap_ids.c b/openair2/F1AP/f1ap_ids.c
index 009517510e..a69c21d050 100644
--- a/openair2/F1AP/f1ap_ids.c
+++ b/openair2/F1AP/f1ap_ids.c
@@ -60,14 +60,14 @@ void cu_init_f1_ue_data(void)
   pthread_mutex_unlock(&cu2du_mutex);
 }
 
-bool cu_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
+bool cu_addmod_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
 {
   pthread_mutex_lock(&cu2du_mutex);
   DevAssert(cu2du_ue_mapping != NULL);
   uint64_t key = ue_id;
   if (hashtable_is_key_exists(cu2du_ue_mapping, key) == HASH_TABLE_OK) {
-    pthread_mutex_unlock(&cu2du_mutex);
-    return false;
+    hashtable_rc_t ret = hashtable_remove(cu2du_ue_mapping, key);
+    DevAssert(ret == HASH_TABLE_OK);
   }
   bool ret = add_hashtable_data(cu2du_ue_mapping, key, data);
   pthread_mutex_unlock(&cu2du_mutex);
@@ -116,14 +116,14 @@ void du_init_f1_ue_data(void)
   pthread_mutex_unlock(&du2cu_mutex);
 }
 
-bool du_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
+bool du_addmod_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
 {
   pthread_mutex_lock(&du2cu_mutex);
   DevAssert(du2cu_ue_mapping != NULL);
   uint64_t key = ue_id;
   if (hashtable_is_key_exists(du2cu_ue_mapping, key) == HASH_TABLE_OK) {
-    pthread_mutex_unlock(&du2cu_mutex);
-    return false;
+    hashtable_rc_t ret = hashtable_remove(du2cu_ue_mapping, key);
+    DevAssert(ret == HASH_TABLE_OK);
   }
   bool ret = add_hashtable_data(du2cu_ue_mapping, key, data);
   pthread_mutex_unlock(&du2cu_mutex);
diff --git a/openair2/F1AP/f1ap_ids.h b/openair2/F1AP/f1ap_ids.h
index 54d17243c5..0847816124 100644
--- a/openair2/F1AP/f1ap_ids.h
+++ b/openair2/F1AP/f1ap_ids.h
@@ -38,13 +38,13 @@ typedef struct f1_ue_data_t {
 } f1_ue_data_t;
 
 void cu_init_f1_ue_data(void);
-bool cu_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
+bool cu_addmod_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
 bool cu_exists_f1_ue_data(uint32_t ue_id);
 f1_ue_data_t cu_get_f1_ue_data(uint32_t ue_id);
 bool cu_remove_f1_ue_data(uint32_t ue_id);
 
 void du_init_f1_ue_data(void);
-bool du_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
+bool du_addmod_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
 bool du_exists_f1_ue_data(uint32_t ue_id);
 f1_ue_data_t du_get_f1_ue_data(uint32_t ue_id);
 bool du_remove_f1_ue_data(uint32_t ue_id);
diff --git a/openair2/F1AP/f1ap_ids_test.c b/openair2/F1AP/f1ap_ids_test.c
index 8c1860a946..47e85ba777 100644
--- a/openair2/F1AP/f1ap_ids_test.c
+++ b/openair2/F1AP/f1ap_ids_test.c
@@ -30,10 +30,11 @@ void du_data_test()
   du_init_f1_ue_data();
   int rnti = 13;
   f1_ue_data_t data = {.secondary_ue = 1};
-  bool ret = du_add_f1_ue_data(rnti, &data);
+  bool ret = du_addmod_f1_ue_data(rnti, &data);
+  DevAssert(ret);
+  // we can overwrite, no problem
+  ret = du_addmod_f1_ue_data(rnti, &data);
   DevAssert(ret);
-  ret = du_add_f1_ue_data(rnti, &data);
-  DevAssert(!ret);
   bool exists = du_exists_f1_ue_data(rnti);
   DevAssert(exists);
   f1_ue_data_t rdata = du_get_f1_ue_data(rnti);
@@ -45,14 +46,25 @@ void cu_data_test()
   cu_init_f1_ue_data();
   int ue_id = 13;
   f1_ue_data_t data = {.secondary_ue = 1};
-  bool ret = cu_add_f1_ue_data(ue_id, &data);
+  bool ret = cu_addmod_f1_ue_data(ue_id, &data);
+  DevAssert(ret);
+  // we can overwrite, no problem
+  data.secondary_ue = 2;
+  ret = cu_addmod_f1_ue_data(ue_id, &data);
   DevAssert(ret);
-  ret = cu_add_f1_ue_data(ue_id, &data);
-  DevAssert(!ret);
   bool exists = cu_exists_f1_ue_data(ue_id);
   DevAssert(exists);
   f1_ue_data_t rdata = cu_get_f1_ue_data(ue_id);
   DevAssert(rdata.secondary_ue == data.secondary_ue);
+
+  rdata.secondary_ue = 3;
+  // overwrite once more
+  ret = cu_addmod_f1_ue_data(ue_id, &rdata);
+  DevAssert(ret);
+  exists = cu_exists_f1_ue_data(ue_id);
+  DevAssert(exists);
+  f1_ue_data_t r2data = cu_get_f1_ue_data(ue_id);
+  DevAssert(r2data.secondary_ue == rdata.secondary_ue);
 }
 
 int main()
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 9c6f5f4b27..4e185e0223 100644
--- a/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c
+++ b/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c
@@ -482,7 +482,7 @@ static NR_UE_info_t *create_new_UE(gNB_MAC_INST *mac, uint32_t cu_id)
     return NULL;
 
   f1_ue_data_t new_ue_data = {.secondary_ue = cu_id};
-  du_add_f1_ue_data(rnti, &new_ue_data);
+  du_addmod_f1_ue_data(rnti, &new_ue_data);
 
   const NR_ServingCellConfigCommon_t *scc = mac->common_channels[CC_id].ServingCellConfigCommon;
   const NR_ServingCellConfig_t *sccd = mac->common_channels[CC_id].pre_ServingCellConfig;
@@ -826,7 +826,7 @@ void dl_rrc_message_transfer(const f1ap_dl_rrc_message_t *dl_rrc)
   if (!du_exists_f1_ue_data(dl_rrc->gNB_DU_ue_id)) {
     LOG_D(NR_MAC, "No CU UE ID stored for UE RNTI %04x, adding CU UE ID %d\n", dl_rrc->gNB_DU_ue_id, dl_rrc->gNB_CU_ue_id);
     f1_ue_data_t new_ue_data = {.secondary_ue = dl_rrc->gNB_CU_ue_id};
-    du_add_f1_ue_data(dl_rrc->gNB_DU_ue_id, &new_ue_data);
+    du_addmod_f1_ue_data(dl_rrc->gNB_DU_ue_id, &new_ue_data);
   }
 
   if (UE->expect_reconfiguration && dl_rrc->srb_id == 1) {
diff --git a/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c b/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
index 1d3394e788..8a7ca4c278 100644
--- a/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
+++ b/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
@@ -150,7 +150,7 @@ void e1_bearer_context_setup(const e1ap_bearer_setup_req_t *req)
   uint32_t cu_up_ue_id = req->gNB_cu_cp_ue_id;
   f1_ue_data_t ued = {.secondary_ue = req->gNB_cu_cp_ue_id};
   if (need_ue_id_mgmt) {
-    cu_add_f1_ue_data(cu_up_ue_id, &ued);
+    cu_addmod_f1_ue_data(cu_up_ue_id, &ued);
     LOG_I(E1AP, "adding UE with CU-CP UE ID %d and CU-UP UE ID %d\n", req->gNB_cu_cp_ue_id, cu_up_ue_id);
   }
 
diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c
index 2456a851de..efe4a53a13 100644
--- a/openair2/RRC/NR/rrc_gNB.c
+++ b/openair2/RRC/NR/rrc_gNB.c
@@ -1181,8 +1181,7 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc,
     /* update to old DU assoc id -- RNTI + secondary DU UE ID further below */
     f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
     ue_data.du_assoc_id = source_ctx->du->assoc_id;
-    cu_remove_f1_ue_data(UE->rrc_ue_id);
-    cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    cu_addmod_f1_ue_data(UE->rrc_ue_id, &ue_data);
     nr_rrc_finalize_ho(UE);
   } else if (physCellId != cell_info->nr_pci) {
     /* UE was moving from previous cell so quickly that RRCReestablishment for previous cell was received in this cell */
@@ -1212,8 +1211,7 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc,
   UE->nr_cellid = msg->nr_cellid;
   f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
   ue_data.secondary_ue = msg->gNB_DU_ue_id;
-  cu_remove_f1_ue_data(UE->rrc_ue_id);
-  cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+  cu_addmod_f1_ue_data(UE->rrc_ue_id, &ue_data);
 
   rrc_gNB_generate_RRCReestablishment(ue_context_p, msg->du2cu_rrc_container, old_rnti, du);
   return;
@@ -2125,8 +2123,7 @@ static void rrc_CU_process_ue_context_modification_response(MessageDef *msg_p, i
     f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
     ue_data.secondary_ue = target_ctx->du_ue_id;
     ue_data.du_assoc_id = target_ctx->du->assoc_id;
-    cu_remove_f1_ue_data(UE->rrc_ue_id);
-    cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    cu_addmod_f1_ue_data(UE->rrc_ue_id, &ue_data);
     LOG_I(NR_RRC, "UE %d handover: update RNTI from %04x to %04x\n", UE->rrc_ue_id, UE->rnti, target_ctx->new_rnti);
     nr_ho_source_cu_t *source_ctx = UE->ho_context->source;
     DevAssert(source_ctx->old_rnti == UE->rnti);
diff --git a/openair2/RRC/NR/rrc_gNB_UE_context.c b/openair2/RRC/NR/rrc_gNB_UE_context.c
index eae5c0706e..5f3ac385c9 100644
--- a/openair2/RRC/NR/rrc_gNB_UE_context.c
+++ b/openair2/RRC/NR/rrc_gNB_UE_context.c
@@ -204,7 +204,7 @@ rrc_gNB_ue_context_t *rrc_gNB_create_ue_context(sctp_assoc_t assoc_id,
   AssertFatal(!cu_exists_f1_ue_data(ue->rrc_ue_id),
               "UE F1 Context for ID %d already exists, logic bug\n",
               ue->rrc_ue_id);
-  cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  cu_addmod_f1_ue_data(ue->rrc_ue_id, &ue_data);
 
   RB_INSERT(rrc_nr_ue_tree_s, &rrc_instance_pP->rrc_ue_head, ue_context_p);
   LOG_UE_EVENT(ue,
diff --git a/openair2/RRC/NR/rrc_gNB_cuup.c b/openair2/RRC/NR/rrc_gNB_cuup.c
index 621ebca56e..f6fb8750a3 100644
--- a/openair2/RRC/NR/rrc_gNB_cuup.c
+++ b/openair2/RRC/NR/rrc_gNB_cuup.c
@@ -132,8 +132,7 @@ sctp_assoc_t get_new_cuup_for_ue(const gNB_RRC_INST *rrc, const gNB_RRC_UE_t *ue
 
   /* update the association for the UE so it will be picked up later */
   ue_data.e1_assoc_id = selected->assoc_id;
-  cu_remove_f1_ue_data(ue->rrc_ue_id);
-  cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  cu_addmod_f1_ue_data(ue->rrc_ue_id, &ue_data);
   LOG_I(RRC, "UE %d associating to CU-UP assoc_id %d out of %ld CU-UPs\n", ue->rrc_ue_id, ue_data.e1_assoc_id, rrc->num_cuups);
 
   return ue_data.e1_assoc_id;
diff --git a/openair2/RRC/NR/rrc_gNB_du.c b/openair2/RRC/NR/rrc_gNB_du.c
index 654ef04be1..8900d183b4 100644
--- a/openair2/RRC/NR/rrc_gNB_du.c
+++ b/openair2/RRC/NR/rrc_gNB_du.c
@@ -426,9 +426,8 @@ static int invalidate_du_connections(gNB_RRC_INST *rrc, sctp_assoc_t assoc_id)
     if (ue_data.du_assoc_id == assoc_id) {
       /* this UE belongs to the DU that disconnected, set du_assoc_id to 0,
        * meaning DU is offline, then trigger release request */
-      cu_remove_f1_ue_data(ue_id);
       ue_data.du_assoc_id = 0;
-      cu_add_f1_ue_data(ue_id, &ue_data);
+      cu_addmod_f1_ue_data(ue_id, &ue_data);
       rrc_gNB_send_NGAP_UE_CONTEXT_RELEASE_REQ(0,
                                                ue_context_p,
                                                NGAP_CAUSE_RADIO_NETWORK,
diff --git a/openair2/RRC/NR/rrc_gNB_nsa.c b/openair2/RRC/NR/rrc_gNB_nsa.c
index bae7c45180..515776f2a1 100644
--- a/openair2/RRC/NR/rrc_gNB_nsa.c
+++ b/openair2/RRC/NR/rrc_gNB_nsa.c
@@ -355,9 +355,9 @@ void rrc_add_nsa_user(gNB_RRC_INST *rrc, rrc_gNB_ue_context_t *ue_context_p, x2a
   uint32_t du_ue_id = ue_context_p->ue_context.rnti;
   uint32_t rrc_ue_id = ue_context_p->ue_context.rrc_ue_id;
   f1_ue_data_t du_ue_data = {.secondary_ue = rrc_ue_id};
-  du_add_f1_ue_data(du_ue_id, &du_ue_data);
+  du_addmod_f1_ue_data(du_ue_id, &du_ue_data);
   f1_ue_data_t cu_ue_data = {.secondary_ue = du_ue_id};
-  cu_add_f1_ue_data(rrc_ue_id, &cu_ue_data);
+  cu_addmod_f1_ue_data(rrc_ue_id, &cu_ue_data);
   LOG_I(RRC, "Assign CU UE ID %d and DU UE ID %d to UE RNTI %04x\n", rrc_ue_id, du_ue_id, ue_context_p->ue_context.rnti);
 
   // configure MAC and RLC

From f0ab268393e926d95964c468d986d60d91813911 Mon Sep 17 00:00:00 2001
From: Robert Schmidt <robert.schmidt@openairinterface.org>
Date: Fri, 24 Jan 2025 15:23:06 +0100
Subject: [PATCH 5/7] Revert "F1AP IDs: Allow AddMod, fix concurrency bug"

This reverts commit 6d5e29f5aee3961775e73096c40c5069c1308ce4.
---
 openair2/F1AP/f1ap_ids.c                      | 12 +++++-----
 openair2/F1AP/f1ap_ids.h                      |  4 ++--
 openair2/F1AP/f1ap_ids_test.c                 | 24 +++++--------------
 .../LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c    |  4 ++--
 openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c   |  2 +-
 openair2/RRC/NR/rrc_gNB.c                     |  9 ++++---
 openair2/RRC/NR/rrc_gNB_UE_context.c          |  2 +-
 openair2/RRC/NR/rrc_gNB_cuup.c                |  3 ++-
 openair2/RRC/NR/rrc_gNB_du.c                  |  3 ++-
 openair2/RRC/NR/rrc_gNB_nsa.c                 |  4 ++--
 10 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/openair2/F1AP/f1ap_ids.c b/openair2/F1AP/f1ap_ids.c
index a69c21d050..009517510e 100644
--- a/openair2/F1AP/f1ap_ids.c
+++ b/openair2/F1AP/f1ap_ids.c
@@ -60,14 +60,14 @@ void cu_init_f1_ue_data(void)
   pthread_mutex_unlock(&cu2du_mutex);
 }
 
-bool cu_addmod_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
+bool cu_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
 {
   pthread_mutex_lock(&cu2du_mutex);
   DevAssert(cu2du_ue_mapping != NULL);
   uint64_t key = ue_id;
   if (hashtable_is_key_exists(cu2du_ue_mapping, key) == HASH_TABLE_OK) {
-    hashtable_rc_t ret = hashtable_remove(cu2du_ue_mapping, key);
-    DevAssert(ret == HASH_TABLE_OK);
+    pthread_mutex_unlock(&cu2du_mutex);
+    return false;
   }
   bool ret = add_hashtable_data(cu2du_ue_mapping, key, data);
   pthread_mutex_unlock(&cu2du_mutex);
@@ -116,14 +116,14 @@ void du_init_f1_ue_data(void)
   pthread_mutex_unlock(&du2cu_mutex);
 }
 
-bool du_addmod_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
+bool du_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
 {
   pthread_mutex_lock(&du2cu_mutex);
   DevAssert(du2cu_ue_mapping != NULL);
   uint64_t key = ue_id;
   if (hashtable_is_key_exists(du2cu_ue_mapping, key) == HASH_TABLE_OK) {
-    hashtable_rc_t ret = hashtable_remove(du2cu_ue_mapping, key);
-    DevAssert(ret == HASH_TABLE_OK);
+    pthread_mutex_unlock(&du2cu_mutex);
+    return false;
   }
   bool ret = add_hashtable_data(du2cu_ue_mapping, key, data);
   pthread_mutex_unlock(&du2cu_mutex);
diff --git a/openair2/F1AP/f1ap_ids.h b/openair2/F1AP/f1ap_ids.h
index 0847816124..54d17243c5 100644
--- a/openair2/F1AP/f1ap_ids.h
+++ b/openair2/F1AP/f1ap_ids.h
@@ -38,13 +38,13 @@ typedef struct f1_ue_data_t {
 } f1_ue_data_t;
 
 void cu_init_f1_ue_data(void);
-bool cu_addmod_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
+bool cu_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
 bool cu_exists_f1_ue_data(uint32_t ue_id);
 f1_ue_data_t cu_get_f1_ue_data(uint32_t ue_id);
 bool cu_remove_f1_ue_data(uint32_t ue_id);
 
 void du_init_f1_ue_data(void);
-bool du_addmod_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
+bool du_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
 bool du_exists_f1_ue_data(uint32_t ue_id);
 f1_ue_data_t du_get_f1_ue_data(uint32_t ue_id);
 bool du_remove_f1_ue_data(uint32_t ue_id);
diff --git a/openair2/F1AP/f1ap_ids_test.c b/openair2/F1AP/f1ap_ids_test.c
index 47e85ba777..8c1860a946 100644
--- a/openair2/F1AP/f1ap_ids_test.c
+++ b/openair2/F1AP/f1ap_ids_test.c
@@ -30,11 +30,10 @@ void du_data_test()
   du_init_f1_ue_data();
   int rnti = 13;
   f1_ue_data_t data = {.secondary_ue = 1};
-  bool ret = du_addmod_f1_ue_data(rnti, &data);
-  DevAssert(ret);
-  // we can overwrite, no problem
-  ret = du_addmod_f1_ue_data(rnti, &data);
+  bool ret = du_add_f1_ue_data(rnti, &data);
   DevAssert(ret);
+  ret = du_add_f1_ue_data(rnti, &data);
+  DevAssert(!ret);
   bool exists = du_exists_f1_ue_data(rnti);
   DevAssert(exists);
   f1_ue_data_t rdata = du_get_f1_ue_data(rnti);
@@ -46,25 +45,14 @@ void cu_data_test()
   cu_init_f1_ue_data();
   int ue_id = 13;
   f1_ue_data_t data = {.secondary_ue = 1};
-  bool ret = cu_addmod_f1_ue_data(ue_id, &data);
-  DevAssert(ret);
-  // we can overwrite, no problem
-  data.secondary_ue = 2;
-  ret = cu_addmod_f1_ue_data(ue_id, &data);
+  bool ret = cu_add_f1_ue_data(ue_id, &data);
   DevAssert(ret);
+  ret = cu_add_f1_ue_data(ue_id, &data);
+  DevAssert(!ret);
   bool exists = cu_exists_f1_ue_data(ue_id);
   DevAssert(exists);
   f1_ue_data_t rdata = cu_get_f1_ue_data(ue_id);
   DevAssert(rdata.secondary_ue == data.secondary_ue);
-
-  rdata.secondary_ue = 3;
-  // overwrite once more
-  ret = cu_addmod_f1_ue_data(ue_id, &rdata);
-  DevAssert(ret);
-  exists = cu_exists_f1_ue_data(ue_id);
-  DevAssert(exists);
-  f1_ue_data_t r2data = cu_get_f1_ue_data(ue_id);
-  DevAssert(r2data.secondary_ue == rdata.secondary_ue);
 }
 
 int main()
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 4e185e0223..9c6f5f4b27 100644
--- a/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c
+++ b/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c
@@ -482,7 +482,7 @@ static NR_UE_info_t *create_new_UE(gNB_MAC_INST *mac, uint32_t cu_id)
     return NULL;
 
   f1_ue_data_t new_ue_data = {.secondary_ue = cu_id};
-  du_addmod_f1_ue_data(rnti, &new_ue_data);
+  du_add_f1_ue_data(rnti, &new_ue_data);
 
   const NR_ServingCellConfigCommon_t *scc = mac->common_channels[CC_id].ServingCellConfigCommon;
   const NR_ServingCellConfig_t *sccd = mac->common_channels[CC_id].pre_ServingCellConfig;
@@ -826,7 +826,7 @@ void dl_rrc_message_transfer(const f1ap_dl_rrc_message_t *dl_rrc)
   if (!du_exists_f1_ue_data(dl_rrc->gNB_DU_ue_id)) {
     LOG_D(NR_MAC, "No CU UE ID stored for UE RNTI %04x, adding CU UE ID %d\n", dl_rrc->gNB_DU_ue_id, dl_rrc->gNB_CU_ue_id);
     f1_ue_data_t new_ue_data = {.secondary_ue = dl_rrc->gNB_CU_ue_id};
-    du_addmod_f1_ue_data(dl_rrc->gNB_DU_ue_id, &new_ue_data);
+    du_add_f1_ue_data(dl_rrc->gNB_DU_ue_id, &new_ue_data);
   }
 
   if (UE->expect_reconfiguration && dl_rrc->srb_id == 1) {
diff --git a/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c b/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
index 8a7ca4c278..1d3394e788 100644
--- a/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
+++ b/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
@@ -150,7 +150,7 @@ void e1_bearer_context_setup(const e1ap_bearer_setup_req_t *req)
   uint32_t cu_up_ue_id = req->gNB_cu_cp_ue_id;
   f1_ue_data_t ued = {.secondary_ue = req->gNB_cu_cp_ue_id};
   if (need_ue_id_mgmt) {
-    cu_addmod_f1_ue_data(cu_up_ue_id, &ued);
+    cu_add_f1_ue_data(cu_up_ue_id, &ued);
     LOG_I(E1AP, "adding UE with CU-CP UE ID %d and CU-UP UE ID %d\n", req->gNB_cu_cp_ue_id, cu_up_ue_id);
   }
 
diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c
index efe4a53a13..2456a851de 100644
--- a/openair2/RRC/NR/rrc_gNB.c
+++ b/openair2/RRC/NR/rrc_gNB.c
@@ -1181,7 +1181,8 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc,
     /* update to old DU assoc id -- RNTI + secondary DU UE ID further below */
     f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
     ue_data.du_assoc_id = source_ctx->du->assoc_id;
-    cu_addmod_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    cu_remove_f1_ue_data(UE->rrc_ue_id);
+    cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
     nr_rrc_finalize_ho(UE);
   } else if (physCellId != cell_info->nr_pci) {
     /* UE was moving from previous cell so quickly that RRCReestablishment for previous cell was received in this cell */
@@ -1211,7 +1212,8 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc,
   UE->nr_cellid = msg->nr_cellid;
   f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
   ue_data.secondary_ue = msg->gNB_DU_ue_id;
-  cu_addmod_f1_ue_data(UE->rrc_ue_id, &ue_data);
+  cu_remove_f1_ue_data(UE->rrc_ue_id);
+  cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
 
   rrc_gNB_generate_RRCReestablishment(ue_context_p, msg->du2cu_rrc_container, old_rnti, du);
   return;
@@ -2123,7 +2125,8 @@ static void rrc_CU_process_ue_context_modification_response(MessageDef *msg_p, i
     f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
     ue_data.secondary_ue = target_ctx->du_ue_id;
     ue_data.du_assoc_id = target_ctx->du->assoc_id;
-    cu_addmod_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    cu_remove_f1_ue_data(UE->rrc_ue_id);
+    cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
     LOG_I(NR_RRC, "UE %d handover: update RNTI from %04x to %04x\n", UE->rrc_ue_id, UE->rnti, target_ctx->new_rnti);
     nr_ho_source_cu_t *source_ctx = UE->ho_context->source;
     DevAssert(source_ctx->old_rnti == UE->rnti);
diff --git a/openair2/RRC/NR/rrc_gNB_UE_context.c b/openair2/RRC/NR/rrc_gNB_UE_context.c
index 5f3ac385c9..eae5c0706e 100644
--- a/openair2/RRC/NR/rrc_gNB_UE_context.c
+++ b/openair2/RRC/NR/rrc_gNB_UE_context.c
@@ -204,7 +204,7 @@ rrc_gNB_ue_context_t *rrc_gNB_create_ue_context(sctp_assoc_t assoc_id,
   AssertFatal(!cu_exists_f1_ue_data(ue->rrc_ue_id),
               "UE F1 Context for ID %d already exists, logic bug\n",
               ue->rrc_ue_id);
-  cu_addmod_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
 
   RB_INSERT(rrc_nr_ue_tree_s, &rrc_instance_pP->rrc_ue_head, ue_context_p);
   LOG_UE_EVENT(ue,
diff --git a/openair2/RRC/NR/rrc_gNB_cuup.c b/openair2/RRC/NR/rrc_gNB_cuup.c
index f6fb8750a3..621ebca56e 100644
--- a/openair2/RRC/NR/rrc_gNB_cuup.c
+++ b/openair2/RRC/NR/rrc_gNB_cuup.c
@@ -132,7 +132,8 @@ sctp_assoc_t get_new_cuup_for_ue(const gNB_RRC_INST *rrc, const gNB_RRC_UE_t *ue
 
   /* update the association for the UE so it will be picked up later */
   ue_data.e1_assoc_id = selected->assoc_id;
-  cu_addmod_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  cu_remove_f1_ue_data(ue->rrc_ue_id);
+  cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
   LOG_I(RRC, "UE %d associating to CU-UP assoc_id %d out of %ld CU-UPs\n", ue->rrc_ue_id, ue_data.e1_assoc_id, rrc->num_cuups);
 
   return ue_data.e1_assoc_id;
diff --git a/openair2/RRC/NR/rrc_gNB_du.c b/openair2/RRC/NR/rrc_gNB_du.c
index 8900d183b4..654ef04be1 100644
--- a/openair2/RRC/NR/rrc_gNB_du.c
+++ b/openair2/RRC/NR/rrc_gNB_du.c
@@ -426,8 +426,9 @@ static int invalidate_du_connections(gNB_RRC_INST *rrc, sctp_assoc_t assoc_id)
     if (ue_data.du_assoc_id == assoc_id) {
       /* this UE belongs to the DU that disconnected, set du_assoc_id to 0,
        * meaning DU is offline, then trigger release request */
+      cu_remove_f1_ue_data(ue_id);
       ue_data.du_assoc_id = 0;
-      cu_addmod_f1_ue_data(ue_id, &ue_data);
+      cu_add_f1_ue_data(ue_id, &ue_data);
       rrc_gNB_send_NGAP_UE_CONTEXT_RELEASE_REQ(0,
                                                ue_context_p,
                                                NGAP_CAUSE_RADIO_NETWORK,
diff --git a/openair2/RRC/NR/rrc_gNB_nsa.c b/openair2/RRC/NR/rrc_gNB_nsa.c
index 515776f2a1..bae7c45180 100644
--- a/openair2/RRC/NR/rrc_gNB_nsa.c
+++ b/openair2/RRC/NR/rrc_gNB_nsa.c
@@ -355,9 +355,9 @@ void rrc_add_nsa_user(gNB_RRC_INST *rrc, rrc_gNB_ue_context_t *ue_context_p, x2a
   uint32_t du_ue_id = ue_context_p->ue_context.rnti;
   uint32_t rrc_ue_id = ue_context_p->ue_context.rrc_ue_id;
   f1_ue_data_t du_ue_data = {.secondary_ue = rrc_ue_id};
-  du_addmod_f1_ue_data(du_ue_id, &du_ue_data);
+  du_add_f1_ue_data(du_ue_id, &du_ue_data);
   f1_ue_data_t cu_ue_data = {.secondary_ue = du_ue_id};
-  cu_addmod_f1_ue_data(rrc_ue_id, &cu_ue_data);
+  cu_add_f1_ue_data(rrc_ue_id, &cu_ue_data);
   LOG_I(RRC, "Assign CU UE ID %d and DU UE ID %d to UE RNTI %04x\n", rrc_ue_id, du_ue_id, ue_context_p->ue_context.rnti);
 
   // configure MAC and RLC

From ffb3354d4933fb96edfd90417bfab57be010f0f7 Mon Sep 17 00:00:00 2001
From: Robert Schmidt <robert.schmidt@openairinterface.org>
Date: Fri, 24 Jan 2025 15:39:43 +0100
Subject: [PATCH 6/7] Verify cu/du_add_f1_ue_data() success in stack

---
 .../LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c     |  6 ++++--
 openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c    |  3 ++-
 openair2/RRC/NR/rrc_gNB.c                      | 18 ++++++++++++------
 openair2/RRC/NR/rrc_gNB_UE_context.c           |  3 ++-
 openair2/RRC/NR/rrc_gNB_cuup.c                 |  6 ++++--
 openair2/RRC/NR/rrc_gNB_du.c                   |  6 ++++--
 openair2/RRC/NR/rrc_gNB_nsa.c                  |  6 ++++--
 7 files changed, 32 insertions(+), 16 deletions(-)

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 9c6f5f4b27..124d63d293 100644
--- a/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c
+++ b/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c
@@ -482,7 +482,8 @@ static NR_UE_info_t *create_new_UE(gNB_MAC_INST *mac, uint32_t cu_id)
     return NULL;
 
   f1_ue_data_t new_ue_data = {.secondary_ue = cu_id};
-  du_add_f1_ue_data(rnti, &new_ue_data);
+  bool success = du_add_f1_ue_data(rnti, &new_ue_data);
+  DevAssert(success);
 
   const NR_ServingCellConfigCommon_t *scc = mac->common_channels[CC_id].ServingCellConfigCommon;
   const NR_ServingCellConfig_t *sccd = mac->common_channels[CC_id].pre_ServingCellConfig;
@@ -826,7 +827,8 @@ void dl_rrc_message_transfer(const f1ap_dl_rrc_message_t *dl_rrc)
   if (!du_exists_f1_ue_data(dl_rrc->gNB_DU_ue_id)) {
     LOG_D(NR_MAC, "No CU UE ID stored for UE RNTI %04x, adding CU UE ID %d\n", dl_rrc->gNB_DU_ue_id, dl_rrc->gNB_CU_ue_id);
     f1_ue_data_t new_ue_data = {.secondary_ue = dl_rrc->gNB_CU_ue_id};
-    du_add_f1_ue_data(dl_rrc->gNB_DU_ue_id, &new_ue_data);
+    bool success = du_add_f1_ue_data(dl_rrc->gNB_DU_ue_id, &new_ue_data);
+    DevAssert(success);
   }
 
   if (UE->expect_reconfiguration && dl_rrc->srb_id == 1) {
diff --git a/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c b/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
index 1d3394e788..380ae1f60a 100644
--- a/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
+++ b/openair2/LAYER2/nr_pdcp/cucp_cuup_handler.c
@@ -150,7 +150,8 @@ void e1_bearer_context_setup(const e1ap_bearer_setup_req_t *req)
   uint32_t cu_up_ue_id = req->gNB_cu_cp_ue_id;
   f1_ue_data_t ued = {.secondary_ue = req->gNB_cu_cp_ue_id};
   if (need_ue_id_mgmt) {
-    cu_add_f1_ue_data(cu_up_ue_id, &ued);
+    bool success = cu_add_f1_ue_data(cu_up_ue_id, &ued);
+    DevAssert(success);
     LOG_I(E1AP, "adding UE with CU-CP UE ID %d and CU-UP UE ID %d\n", req->gNB_cu_cp_ue_id, cu_up_ue_id);
   }
 
diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c
index 2456a851de..76f902ffb8 100644
--- a/openair2/RRC/NR/rrc_gNB.c
+++ b/openair2/RRC/NR/rrc_gNB.c
@@ -1181,8 +1181,10 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc,
     /* update to old DU assoc id -- RNTI + secondary DU UE ID further below */
     f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
     ue_data.du_assoc_id = source_ctx->du->assoc_id;
-    cu_remove_f1_ue_data(UE->rrc_ue_id);
-    cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    bool success = cu_remove_f1_ue_data(UE->rrc_ue_id);
+    DevAssert(success);
+    success = cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    DevAssert(success);
     nr_rrc_finalize_ho(UE);
   } else if (physCellId != cell_info->nr_pci) {
     /* UE was moving from previous cell so quickly that RRCReestablishment for previous cell was received in this cell */
@@ -1212,8 +1214,10 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc,
   UE->nr_cellid = msg->nr_cellid;
   f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
   ue_data.secondary_ue = msg->gNB_DU_ue_id;
-  cu_remove_f1_ue_data(UE->rrc_ue_id);
-  cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+  bool success = cu_remove_f1_ue_data(UE->rrc_ue_id);
+  DevAssert(success);
+  success = cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+  DevAssert(success);
 
   rrc_gNB_generate_RRCReestablishment(ue_context_p, msg->du2cu_rrc_container, old_rnti, du);
   return;
@@ -2125,8 +2129,10 @@ static void rrc_CU_process_ue_context_modification_response(MessageDef *msg_p, i
     f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
     ue_data.secondary_ue = target_ctx->du_ue_id;
     ue_data.du_assoc_id = target_ctx->du->assoc_id;
-    cu_remove_f1_ue_data(UE->rrc_ue_id);
-    cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    bool success = cu_remove_f1_ue_data(UE->rrc_ue_id);
+    DevAssert(success);
+    success = cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    DevAssert(success);
     LOG_I(NR_RRC, "UE %d handover: update RNTI from %04x to %04x\n", UE->rrc_ue_id, UE->rnti, target_ctx->new_rnti);
     nr_ho_source_cu_t *source_ctx = UE->ho_context->source;
     DevAssert(source_ctx->old_rnti == UE->rnti);
diff --git a/openair2/RRC/NR/rrc_gNB_UE_context.c b/openair2/RRC/NR/rrc_gNB_UE_context.c
index eae5c0706e..9b02eaa1f4 100644
--- a/openair2/RRC/NR/rrc_gNB_UE_context.c
+++ b/openair2/RRC/NR/rrc_gNB_UE_context.c
@@ -204,7 +204,8 @@ rrc_gNB_ue_context_t *rrc_gNB_create_ue_context(sctp_assoc_t assoc_id,
   AssertFatal(!cu_exists_f1_ue_data(ue->rrc_ue_id),
               "UE F1 Context for ID %d already exists, logic bug\n",
               ue->rrc_ue_id);
-  cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  bool success = cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  DevAssert(success);
 
   RB_INSERT(rrc_nr_ue_tree_s, &rrc_instance_pP->rrc_ue_head, ue_context_p);
   LOG_UE_EVENT(ue,
diff --git a/openair2/RRC/NR/rrc_gNB_cuup.c b/openair2/RRC/NR/rrc_gNB_cuup.c
index 621ebca56e..2b007e490b 100644
--- a/openair2/RRC/NR/rrc_gNB_cuup.c
+++ b/openair2/RRC/NR/rrc_gNB_cuup.c
@@ -132,8 +132,10 @@ sctp_assoc_t get_new_cuup_for_ue(const gNB_RRC_INST *rrc, const gNB_RRC_UE_t *ue
 
   /* update the association for the UE so it will be picked up later */
   ue_data.e1_assoc_id = selected->assoc_id;
-  cu_remove_f1_ue_data(ue->rrc_ue_id);
-  cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  bool success = cu_remove_f1_ue_data(ue->rrc_ue_id);
+  DevAssert(success);
+  success = cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  DevAssert(success);
   LOG_I(RRC, "UE %d associating to CU-UP assoc_id %d out of %ld CU-UPs\n", ue->rrc_ue_id, ue_data.e1_assoc_id, rrc->num_cuups);
 
   return ue_data.e1_assoc_id;
diff --git a/openair2/RRC/NR/rrc_gNB_du.c b/openair2/RRC/NR/rrc_gNB_du.c
index 654ef04be1..64f96efbee 100644
--- a/openair2/RRC/NR/rrc_gNB_du.c
+++ b/openair2/RRC/NR/rrc_gNB_du.c
@@ -426,9 +426,11 @@ static int invalidate_du_connections(gNB_RRC_INST *rrc, sctp_assoc_t assoc_id)
     if (ue_data.du_assoc_id == assoc_id) {
       /* this UE belongs to the DU that disconnected, set du_assoc_id to 0,
        * meaning DU is offline, then trigger release request */
-      cu_remove_f1_ue_data(ue_id);
+      bool success = cu_remove_f1_ue_data(ue_id);
+      DevAssert(success);
       ue_data.du_assoc_id = 0;
-      cu_add_f1_ue_data(ue_id, &ue_data);
+      success = cu_add_f1_ue_data(ue_id, &ue_data);
+      DevAssert(success);
       rrc_gNB_send_NGAP_UE_CONTEXT_RELEASE_REQ(0,
                                                ue_context_p,
                                                NGAP_CAUSE_RADIO_NETWORK,
diff --git a/openair2/RRC/NR/rrc_gNB_nsa.c b/openair2/RRC/NR/rrc_gNB_nsa.c
index bae7c45180..595e380846 100644
--- a/openair2/RRC/NR/rrc_gNB_nsa.c
+++ b/openair2/RRC/NR/rrc_gNB_nsa.c
@@ -355,9 +355,11 @@ void rrc_add_nsa_user(gNB_RRC_INST *rrc, rrc_gNB_ue_context_t *ue_context_p, x2a
   uint32_t du_ue_id = ue_context_p->ue_context.rnti;
   uint32_t rrc_ue_id = ue_context_p->ue_context.rrc_ue_id;
   f1_ue_data_t du_ue_data = {.secondary_ue = rrc_ue_id};
-  du_add_f1_ue_data(du_ue_id, &du_ue_data);
+  bool success = du_add_f1_ue_data(du_ue_id, &du_ue_data);
+  DevAssert(success);
   f1_ue_data_t cu_ue_data = {.secondary_ue = du_ue_id};
-  cu_add_f1_ue_data(rrc_ue_id, &cu_ue_data);
+  success = cu_add_f1_ue_data(rrc_ue_id, &cu_ue_data);
+  DevAssert(success);
   LOG_I(RRC, "Assign CU UE ID %d and DU UE ID %d to UE RNTI %04x\n", rrc_ue_id, du_ue_id, ue_context_p->ue_context.rnti);
 
   // configure MAC and RLC

From febeaa71817edc446a498e3050cf0b89ea3a168a Mon Sep 17 00:00:00 2001
From: Robert Schmidt <robert.schmidt@openairinterface.org>
Date: Fri, 24 Jan 2025 16:09:10 +0100
Subject: [PATCH 7/7] F1AP IDs: Add update method to fix concurrency bug

Introduce function cu_update_f1_ue_data() which does remove+add under
one lock. This fixes a concurrency bug. For instance, during
reestablishment, the CU needs to update the DU UE ID under which the UE
is identified in the DU. Previously, the CU would remove, then add the
DU UE ID info.  At the same time, the PDCP thread might look up the
information. This can lead to asserts. Use the above addmod
functionality to do the remove-add under a mutex.

Modify test to verify this works as expected.
---
 openair2/F1AP/f1ap_ids.c       | 12 ++++++++++++
 openair2/F1AP/f1ap_ids.h       |  1 +
 openair2/F1AP/f1ap_ids_test.c  | 11 +++++++++++
 openair2/RRC/NR/rrc_gNB.c      | 12 +++---------
 openair2/RRC/NR/rrc_gNB_cuup.c |  4 +---
 openair2/RRC/NR/rrc_gNB_du.c   |  4 +---
 6 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/openair2/F1AP/f1ap_ids.c b/openair2/F1AP/f1ap_ids.c
index 009517510e..5569d60c48 100644
--- a/openair2/F1AP/f1ap_ids.c
+++ b/openair2/F1AP/f1ap_ids.c
@@ -84,6 +84,18 @@ bool cu_exists_f1_ue_data(uint32_t ue_id)
   return ret == HASH_TABLE_OK;
 }
 
+bool cu_update_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data)
+{
+  pthread_mutex_lock(&cu2du_mutex);
+  DevAssert(cu2du_ue_mapping != NULL);
+  uint64_t key = ue_id;
+  hashtable_rc_t retrm = hashtable_remove(cu2du_ue_mapping, key);
+  AssertFatal(retrm == HASH_TABLE_OK, "could not remove UE %d, ret %d\n", ue_id, retrm);
+  bool ret = add_hashtable_data(cu2du_ue_mapping, key, data);
+  pthread_mutex_unlock(&cu2du_mutex);
+  return ret;
+}
+
 f1_ue_data_t cu_get_f1_ue_data(uint32_t ue_id)
 {
   pthread_mutex_lock(&cu2du_mutex);
diff --git a/openair2/F1AP/f1ap_ids.h b/openair2/F1AP/f1ap_ids.h
index 54d17243c5..585ed33322 100644
--- a/openair2/F1AP/f1ap_ids.h
+++ b/openair2/F1AP/f1ap_ids.h
@@ -40,6 +40,7 @@ typedef struct f1_ue_data_t {
 void cu_init_f1_ue_data(void);
 bool cu_add_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
 bool cu_exists_f1_ue_data(uint32_t ue_id);
+bool cu_update_f1_ue_data(uint32_t ue_id, const f1_ue_data_t *data);
 f1_ue_data_t cu_get_f1_ue_data(uint32_t ue_id);
 bool cu_remove_f1_ue_data(uint32_t ue_id);
 
diff --git a/openair2/F1AP/f1ap_ids_test.c b/openair2/F1AP/f1ap_ids_test.c
index 8c1860a946..a58059339d 100644
--- a/openair2/F1AP/f1ap_ids_test.c
+++ b/openair2/F1AP/f1ap_ids_test.c
@@ -49,10 +49,21 @@ void cu_data_test()
   DevAssert(ret);
   ret = cu_add_f1_ue_data(ue_id, &data);
   DevAssert(!ret);
+  data.secondary_ue = 2;
+  ret = cu_update_f1_ue_data(ue_id, &data);
   bool exists = cu_exists_f1_ue_data(ue_id);
   DevAssert(exists);
   f1_ue_data_t rdata = cu_get_f1_ue_data(ue_id);
   DevAssert(rdata.secondary_ue == data.secondary_ue);
+
+  rdata.secondary_ue = 3;
+  // overwrite once more
+  ret = cu_update_f1_ue_data(ue_id, &rdata);
+  DevAssert(ret);
+  exists = cu_exists_f1_ue_data(ue_id);
+  DevAssert(exists);
+  f1_ue_data_t r2data = cu_get_f1_ue_data(ue_id);
+  DevAssert(r2data.secondary_ue == rdata.secondary_ue);
 }
 
 int main()
diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c
index 76f902ffb8..8630b4006c 100644
--- a/openair2/RRC/NR/rrc_gNB.c
+++ b/openair2/RRC/NR/rrc_gNB.c
@@ -1181,9 +1181,7 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc,
     /* update to old DU assoc id -- RNTI + secondary DU UE ID further below */
     f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
     ue_data.du_assoc_id = source_ctx->du->assoc_id;
-    bool success = cu_remove_f1_ue_data(UE->rrc_ue_id);
-    DevAssert(success);
-    success = cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    bool success = cu_update_f1_ue_data(UE->rrc_ue_id, &ue_data);
     DevAssert(success);
     nr_rrc_finalize_ho(UE);
   } else if (physCellId != cell_info->nr_pci) {
@@ -1214,9 +1212,7 @@ static void rrc_handle_RRCReestablishmentRequest(gNB_RRC_INST *rrc,
   UE->nr_cellid = msg->nr_cellid;
   f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
   ue_data.secondary_ue = msg->gNB_DU_ue_id;
-  bool success = cu_remove_f1_ue_data(UE->rrc_ue_id);
-  DevAssert(success);
-  success = cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+  bool success = cu_update_f1_ue_data(UE->rrc_ue_id, &ue_data);
   DevAssert(success);
 
   rrc_gNB_generate_RRCReestablishment(ue_context_p, msg->du2cu_rrc_container, old_rnti, du);
@@ -2129,9 +2125,7 @@ static void rrc_CU_process_ue_context_modification_response(MessageDef *msg_p, i
     f1_ue_data_t ue_data = cu_get_f1_ue_data(UE->rrc_ue_id);
     ue_data.secondary_ue = target_ctx->du_ue_id;
     ue_data.du_assoc_id = target_ctx->du->assoc_id;
-    bool success = cu_remove_f1_ue_data(UE->rrc_ue_id);
-    DevAssert(success);
-    success = cu_add_f1_ue_data(UE->rrc_ue_id, &ue_data);
+    bool success = cu_update_f1_ue_data(UE->rrc_ue_id, &ue_data);
     DevAssert(success);
     LOG_I(NR_RRC, "UE %d handover: update RNTI from %04x to %04x\n", UE->rrc_ue_id, UE->rnti, target_ctx->new_rnti);
     nr_ho_source_cu_t *source_ctx = UE->ho_context->source;
diff --git a/openair2/RRC/NR/rrc_gNB_cuup.c b/openair2/RRC/NR/rrc_gNB_cuup.c
index 2b007e490b..67aa14bbbb 100644
--- a/openair2/RRC/NR/rrc_gNB_cuup.c
+++ b/openair2/RRC/NR/rrc_gNB_cuup.c
@@ -132,9 +132,7 @@ sctp_assoc_t get_new_cuup_for_ue(const gNB_RRC_INST *rrc, const gNB_RRC_UE_t *ue
 
   /* update the association for the UE so it will be picked up later */
   ue_data.e1_assoc_id = selected->assoc_id;
-  bool success = cu_remove_f1_ue_data(ue->rrc_ue_id);
-  DevAssert(success);
-  success = cu_add_f1_ue_data(ue->rrc_ue_id, &ue_data);
+  bool success = cu_update_f1_ue_data(ue->rrc_ue_id, &ue_data);
   DevAssert(success);
   LOG_I(RRC, "UE %d associating to CU-UP assoc_id %d out of %ld CU-UPs\n", ue->rrc_ue_id, ue_data.e1_assoc_id, rrc->num_cuups);
 
diff --git a/openair2/RRC/NR/rrc_gNB_du.c b/openair2/RRC/NR/rrc_gNB_du.c
index 64f96efbee..3c28040d16 100644
--- a/openair2/RRC/NR/rrc_gNB_du.c
+++ b/openair2/RRC/NR/rrc_gNB_du.c
@@ -426,10 +426,8 @@ static int invalidate_du_connections(gNB_RRC_INST *rrc, sctp_assoc_t assoc_id)
     if (ue_data.du_assoc_id == assoc_id) {
       /* this UE belongs to the DU that disconnected, set du_assoc_id to 0,
        * meaning DU is offline, then trigger release request */
-      bool success = cu_remove_f1_ue_data(ue_id);
-      DevAssert(success);
       ue_data.du_assoc_id = 0;
-      success = cu_add_f1_ue_data(ue_id, &ue_data);
+      bool success = cu_update_f1_ue_data(ue_id, &ue_data);
       DevAssert(success);
       rrc_gNB_send_NGAP_UE_CONTEXT_RELEASE_REQ(0,
                                                ue_context_p,