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

@@ -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);

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);
 

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()

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;

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);
 

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,