From 6c4548c243d66eb1627473760fd625db61b87d27 Mon Sep 17 00:00:00 2001 From: faluco Date: Tue, 5 Oct 2021 18:15:23 +0200 Subject: [PATCH] Fix several data races in LTE and NR harq classes. --- srsue/hdr/stack/mac/ul_harq.h | 1 + srsue/hdr/stack/mac_nr/dl_harq_nr.h | 7 ++++--- srsue/hdr/stack/mac_nr/ul_harq_nr.h | 9 +++++---- srsue/src/stack/mac/ul_harq.cc | 12 ++++++++---- srsue/src/stack/mac_nr/dl_harq_nr.cc | 7 +++++-- srsue/src/stack/mac_nr/ul_harq_nr.cc | 9 ++++++++- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/srsue/hdr/stack/mac/ul_harq.h b/srsue/hdr/stack/mac/ul_harq.h index b0726d161..ae51ec542 100644 --- a/srsue/hdr/stack/mac/ul_harq.h +++ b/srsue/hdr/stack/mac/ul_harq.h @@ -99,6 +99,7 @@ private: ue_rnti* rntis = nullptr; srsran::ul_harq_cfg_t harq_cfg = {}; + std::mutex config_mutex; std::atomic average_retx{0}; std::atomic nof_pkts{0}; diff --git a/srsue/hdr/stack/mac_nr/dl_harq_nr.h b/srsue/hdr/stack/mac_nr/dl_harq_nr.h index 9754cb6a6..c0475fba7 100644 --- a/srsue/hdr/stack/mac_nr/dl_harq_nr.h +++ b/srsue/hdr/stack/mac_nr/dl_harq_nr.h @@ -42,7 +42,7 @@ public: ~dl_harq_entity_nr(); int32_t set_config(const srsran::dl_harq_cfg_nr_t& cfg_); - void reset(); + void reset(); /// PHY->MAC interface for DL processes void new_grant_dl(const mac_nr_grant_dl_t& grant, mac_interface_phy_nr::tb_action_dl_t* action); @@ -69,7 +69,7 @@ private: uint8_t get_ndi(); void - new_grant_dl(const mac_nr_grant_dl_t& grant, const bool& ndi_toggled, mac_interface_phy_nr::tb_action_dl_t* action); + new_grant_dl(const mac_nr_grant_dl_t& grant, const bool& ndi_toggled, mac_interface_phy_nr::tb_action_dl_t* action); void tb_decoded(const mac_nr_grant_dl_t& grant, mac_interface_phy_nr::tb_action_dl_result_t result); private: @@ -96,7 +96,8 @@ private: srslog::basic_logger& logger; uint16_t last_temporal_crnti = SRSRAN_INVALID_RNTI; dl_harq_metrics_t metrics = {}; - uint8_t cc_idx = 0; + std::mutex metrics_mutex; + uint8_t cc_idx = 0; pthread_rwlock_t rwlock; }; diff --git a/srsue/hdr/stack/mac_nr/ul_harq_nr.h b/srsue/hdr/stack/mac_nr/ul_harq_nr.h index 918e51eae..4bb5ac21a 100644 --- a/srsue/hdr/stack/mac_nr/ul_harq_nr.h +++ b/srsue/hdr/stack/mac_nr/ul_harq_nr.h @@ -54,9 +54,9 @@ private: ul_harq_process_nr(); ~ul_harq_process_nr(); - bool init(uint32_t pid_, ul_harq_entity_nr* entity_); - void reset(); - void reset_ndi(); + bool init(uint32_t pid_, ul_harq_entity_nr* entity_); + void reset(); + void reset_ndi(); uint8_t get_ndi(); bool has_grant(); @@ -75,7 +75,7 @@ private: mac_interface_phy_nr::tb_action_ul_t* action); private: - mac_interface_phy_nr::mac_nr_grant_ul_t current_grant = {}; + mac_interface_phy_nr::mac_nr_grant_ul_t current_grant = {}; bool grant_configured = false; uint32_t pid = 0; @@ -103,6 +103,7 @@ private: srsran::ul_harq_cfg_t harq_cfg = {}; ul_harq_metrics_t metrics = {}; + std::mutex metrics_mutex; }; typedef std::unique_ptr ul_harq_entity_nr_ptr; diff --git a/srsue/src/stack/mac/ul_harq.cc b/srsue/src/stack/mac/ul_harq.cc index de968b511..6951b2a34 100644 --- a/srsue/src/stack/mac/ul_harq.cc +++ b/srsue/src/stack/mac/ul_harq.cc @@ -57,6 +57,7 @@ void ul_harq_entity::reset_ndi() void ul_harq_entity::set_config(srsran::ul_harq_cfg_t& harq_cfg_) { + std::lock_guard lock(config_mutex); harq_cfg = harq_cfg_; } @@ -174,10 +175,13 @@ void ul_harq_entity::ul_harq_process::new_grant_ul(mac_interface_phy_lte::mac_gr // Get maximum retransmissions uint32_t max_retx; - if (grant.rnti == harq_entity->rntis->get_temp_rnti()) { - max_retx = harq_entity->harq_cfg.max_harq_msg3_tx; - } else { - max_retx = harq_entity->harq_cfg.max_harq_tx; + { + std::lock_guard lock(harq_entity->config_mutex); + if (grant.rnti == harq_entity->rntis->get_temp_rnti()) { + max_retx = harq_entity->harq_cfg.max_harq_msg3_tx; + } else { + max_retx = harq_entity->harq_cfg.max_harq_tx; + } } // Check maximum retransmissions, do not consider last retx ACK diff --git a/srsue/src/stack/mac_nr/dl_harq_nr.cc b/srsue/src/stack/mac_nr/dl_harq_nr.cc index eca971ad6..b1bfd9d70 100644 --- a/srsue/src/stack/mac_nr/dl_harq_nr.cc +++ b/srsue/src/stack/mac_nr/dl_harq_nr.cc @@ -126,8 +126,9 @@ void dl_harq_entity_nr::reset() dl_harq_entity_nr::dl_harq_metrics_t dl_harq_entity_nr::get_metrics() { - dl_harq_metrics_t tmp = metrics; - metrics = {}; + std::lock_guard lock(metrics_mutex); + dl_harq_metrics_t tmp = metrics; + metrics = {}; return tmp; } @@ -230,9 +231,11 @@ void dl_harq_entity_nr::dl_harq_process_nr::tb_decoded(const mac_nr_grant_dl_t& } } + std::lock_guard lock(harq_entity->metrics_mutex); harq_entity->metrics.rx_ok++; harq_entity->metrics.rx_brate += grant.tbs * 8; } else { + std::lock_guard lock(harq_entity->metrics_mutex); harq_entity->metrics.rx_ko++; } diff --git a/srsue/src/stack/mac_nr/ul_harq_nr.cc b/srsue/src/stack/mac_nr/ul_harq_nr.cc index 3e101c82d..1f4ae8c49 100644 --- a/srsue/src/stack/mac_nr/ul_harq_nr.cc +++ b/srsue/src/stack/mac_nr/ul_harq_nr.cc @@ -106,6 +106,7 @@ int ul_harq_entity_nr::get_current_tbs(uint32_t pid) ul_harq_entity_nr::ul_harq_metrics_t ul_harq_entity_nr::get_metrics() { + std::lock_guard lock(metrics_mutex); ul_harq_entity_nr::ul_harq_metrics_t tmp = metrics; metrics = {}; return tmp; @@ -240,6 +241,7 @@ void ul_harq_entity_nr::ul_harq_process_nr::generate_new_tx(const mac_interface_ generate_tx(action); + std::lock_guard lock(harq_entity->metrics_mutex); harq_entity->metrics.tx_ok++; } @@ -255,6 +257,7 @@ void ul_harq_entity_nr::ul_harq_process_nr::generate_retx(const mac_interface_ph generate_tx(action); // increment Tx error count + std::lock_guard lock(harq_entity->metrics_mutex); harq_entity->metrics.tx_ko++; } @@ -262,7 +265,11 @@ void ul_harq_entity_nr::ul_harq_process_nr::generate_retx(const mac_interface_ph void ul_harq_entity_nr::ul_harq_process_nr::generate_tx(mac_interface_phy_nr::tb_action_ul_t* action) { nof_retx++; - harq_entity->metrics.tx_brate += current_grant.tbs * 8; + + { + std::lock_guard lock(harq_entity->metrics_mutex); + harq_entity->metrics.tx_brate += current_grant.tbs * 8; + } action->tb.rv = current_grant.rv; action->tb.enabled = true;