From fb4dd3ba7bc274f987377e6aa1b5dba8fe243bf4 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Tue, 1 Jun 2021 20:45:08 +0200 Subject: [PATCH] ttcn3: fix various races between SS and Stack thread detected by TSAN fixed through the right usage of mutexes in both TTCN PHY and syssim. nested mutex locking is solved by calling SS from the PHY after releaseing the PHY lock again. --- srsue/src/test/ttcn3/hdr/lte_ttcn3_phy.h | 2 +- srsue/src/test/ttcn3/hdr/ttcn3_syssim.h | 5 +- srsue/src/test/ttcn3/src/lte_ttcn3_phy.cc | 77 ++++++++++++++--------- srsue/src/test/ttcn3/src/ttcn3_dut.cc | 1 + srsue/src/test/ttcn3/src/ttcn3_syssim.cc | 13 +++- 5 files changed, 66 insertions(+), 32 deletions(-) diff --git a/srsue/src/test/ttcn3/hdr/lte_ttcn3_phy.h b/srsue/src/test/ttcn3/hdr/lte_ttcn3_phy.h index 72ebf41ed..6a38708ba 100644 --- a/srsue/src/test/ttcn3/hdr/lte_ttcn3_phy.h +++ b/srsue/src/test/ttcn3/hdr/lte_ttcn3_phy.h @@ -122,7 +122,7 @@ private: int sr_tx_tti = -1; bool sr_pending = false; - std::mutex mutex; + std::mutex phy_mutex; srsran::task_scheduler task_sched; diff --git a/srsue/src/test/ttcn3/hdr/ttcn3_syssim.h b/srsue/src/test/ttcn3/hdr/ttcn3_syssim.h index 029f41dd9..1c18195d7 100644 --- a/srsue/src/test/ttcn3/hdr/ttcn3_syssim.h +++ b/srsue/src/test/ttcn3/hdr/ttcn3_syssim.h @@ -242,7 +242,7 @@ private: uint32_t prach_preamble_index = 0; uint16_t dl_rnti = 0; int force_lcid = -1; - srsue::stack_test_dummy stack; + srsue::stack_test_dummy stack; // helper to run queue pending tasks bool last_dl_ndi[SRSRAN_FDD_NOF_HARQ] = {}; bool last_ul_ndi[SRSRAN_FDD_NOF_HARQ] = {}; @@ -276,6 +276,9 @@ private: std::vector cells; int32_t pcell_idx = -1; + // Main mutex to protect access from syssim's main thread (epoll handlers) and calls from UE's stack thread + std::mutex syssim_mutex; + // Internal function void update_cell_map(); bool syssim_has_cell(std::string cell_name); diff --git a/srsue/src/test/ttcn3/src/lte_ttcn3_phy.cc b/srsue/src/test/ttcn3/src/lte_ttcn3_phy.cc index 05b8f31a4..2d0fcf6fd 100644 --- a/srsue/src/test/ttcn3/src/lte_ttcn3_phy.cc +++ b/srsue/src/test/ttcn3/src/lte_ttcn3_phy.cc @@ -55,7 +55,7 @@ void lte_ttcn3_phy::get_metrics(const srsran::srsran_rat_t& rat, phy_metrics_t* // The interface for the SS void lte_ttcn3_phy::set_cell_map(const cell_list_t& cells_) { - std::lock_guard lock(mutex); + std::lock_guard lock(phy_mutex); cells = cells_; } @@ -95,7 +95,7 @@ void lte_ttcn3_phy::meas_stop() {} // are actually visible though. bool lte_ttcn3_phy::cell_search() { - std::lock_guard lock(mutex); + std::lock_guard lock(phy_mutex); logger.info("Running cell search in PHY"); @@ -135,28 +135,44 @@ bool lte_ttcn3_phy::cell_search() return true; } +// Called from RRC/Stack thread bool lte_ttcn3_phy::cell_select(phy_cell_t rrc_cell) { - // try to find RRC cell in current cell map - for (auto& cell : cells) { - if (cell.info.id == rrc_cell.pci && cell.earfcn == rrc_cell.earfcn) { - if (cell.power >= SUITABLE_CELL_RS_EPRE) { - pcell = cell; - pcell_set = true; - syssim->select_cell(pcell.info); - logger.info("Select PCell with %.2f on PCI=%d on EARFCN=%d.", cell.power, rrc_cell.pci, rrc_cell.earfcn); - } else { - pcell_set = false; - logger.error("Power of selected cell too low (%.2f < %.2f)", cell.power, SUITABLE_CELL_RS_EPRE); - } + bool ret = false; - stack->cell_select_complete(pcell_set); - return true; + { + std::lock_guard lock(phy_mutex); + // try to find RRC cell in current cell map + for (auto& cell : cells) { + if (cell.info.id == rrc_cell.pci && cell.earfcn == rrc_cell.earfcn) { + if (cell.power >= SUITABLE_CELL_RS_EPRE) { + pcell = cell; + pcell_set = true; + logger.info("Select PCell with %.2f on PCI=%d on EARFCN=%d.", cell.power, rrc_cell.pci, rrc_cell.earfcn); + } else { + pcell_set = false; + logger.error("Power of selected cell too low (%.2f < %.2f)", cell.power, SUITABLE_CELL_RS_EPRE); + } + // update return value + ret = pcell_set; + break; + } } } - logger.error("Couldn't find RRC cell with PCI=%d on EARFCN=%d in cell map.", rrc_cell.pci, rrc_cell.earfcn); - return false; + if (ret) { + // cell has been selected + syssim->select_cell(pcell.info); + } else { + logger.error( + "Couldn't find (suitable) RRC cell with PCI=%d on EARFCN=%d in cell map.", rrc_cell.pci, rrc_cell.earfcn); + } + + // inform stack about result asynchronously + task_sched.defer_task([this, ret]() { stack->cell_select_complete(ret); }); + + // regardless of actual result, return True to tell RRC that we entered the cell select state at PHY + return true; } bool lte_ttcn3_phy::cell_is_camping() @@ -169,15 +185,19 @@ bool lte_ttcn3_phy::cell_is_camping() } // The interface for MAC (called from Stack thread context) - void lte_ttcn3_phy::prach_send(uint32_t preamble_idx, int allowed_subframe, float target_power_dbm, float ta_base_sec) { - std::lock_guard lock(mutex); - logger.info("Sending PRACH with preamble %d on PCID=%d", preamble_idx, pcell.info.id); - prach_tti_tx = current_tti; - ra_trans_cnt++; + uint32_t pcell_pci = 0; + { + std::lock_guard lock(phy_mutex); + pcell_pci = pcell.info.id; + + logger.info("Sending PRACH with preamble %d on PCID=%d", preamble_idx, pcell_pci); + prach_tti_tx = current_tti; + ra_trans_cnt++; + } - syssim->prach_indication(preamble_idx, pcell.info.id); + syssim->prach_indication(preamble_idx, pcell_pci); }; std::string lte_ttcn3_phy::get_type() @@ -187,7 +207,7 @@ std::string lte_ttcn3_phy::get_type() phy_interface_mac_lte::prach_info_t lte_ttcn3_phy::prach_get_info() { - std::lock_guard lock(mutex); + std::lock_guard lock(phy_mutex); prach_info_t info = {}; if (prach_tti_tx != -1) { info.is_transmitted = true; @@ -231,7 +251,7 @@ void lte_ttcn3_phy::set_rar_grant(uint8_t grant_payload[SRSRAN_RAR_GRANT_LEN], u // Called from the SYSSIM to configure the current TTI void lte_ttcn3_phy::set_current_tti(uint32_t tti) { - std::lock_guard lock(mutex); + std::lock_guard lock(phy_mutex); current_tti = tti; run_tti(); @@ -240,6 +260,7 @@ void lte_ttcn3_phy::set_current_tti(uint32_t tti) // Called from MAC to retrieve the current TTI uint32_t lte_ttcn3_phy::get_current_tti() { + std::lock_guard lock(phy_mutex); return current_tti; } @@ -259,7 +280,7 @@ float lte_ttcn3_phy::get_pathloss_db() // Calling function hold mutex void lte_ttcn3_phy::new_grant_ul(mac_interface_phy_lte::mac_grant_ul_t ul_mac_grant) { - std::lock_guard lock(mutex); + std::lock_guard lock(phy_mutex); mac_interface_phy_lte::tb_action_ul_t ul_action = {}; @@ -275,7 +296,7 @@ void lte_ttcn3_phy::new_grant_ul(mac_interface_phy_lte::mac_grant_ul_t ul_mac_gr // Provides DL grant, copy data into DL action and pass up to MAC void lte_ttcn3_phy::new_tb(const srsue::mac_interface_phy_lte::mac_grant_dl_t dl_grant, const uint8_t* data) { - std::lock_guard lock(mutex); + std::lock_guard lock(phy_mutex); if (data == nullptr) { logger.error("Invalid data buffer passed"); diff --git a/srsue/src/test/ttcn3/src/ttcn3_dut.cc b/srsue/src/test/ttcn3/src/ttcn3_dut.cc index a39f8d64c..f50e2d64a 100644 --- a/srsue/src/test/ttcn3/src/ttcn3_dut.cc +++ b/srsue/src/test/ttcn3/src/ttcn3_dut.cc @@ -11,6 +11,7 @@ */ #include "srsran/build_info.h" +#include "srsran/common/tsan_options.h" #include "srsran/srslog/srslog.h" #include "srsue/hdr/ue.h" #include "swappable_sink.h" diff --git a/srsue/src/test/ttcn3/src/ttcn3_syssim.cc b/srsue/src/test/ttcn3/src/ttcn3_syssim.cc index 698411149..71c92cfca 100644 --- a/srsue/src/test/ttcn3/src/ttcn3_syssim.cc +++ b/srsue/src/test/ttcn3/src/ttcn3_syssim.cc @@ -52,7 +52,9 @@ ttcn3_syssim::ttcn3_syssim(ttcn3_ue* ue_) : signal_handler(running), timer_handler(create_tti_timer(), [&](uint64_t res) { new_tti_indication(res); }) { - if (ue->init(all_args_t{}, this, "INIT_TEST") != SRSRAN_SUCCESS) { + all_args_t args = {}; + args.stack.sync_queue_size = MULTIQUEUE_DEFAULT_CAPACITY; + if (ue->init(args, this, "INIT_TEST") != SRSRAN_SUCCESS) { ue->stop(); fprintf(stderr, "Couldn't initialize UE.\n"); } @@ -193,6 +195,8 @@ int ttcn3_syssim::add_port_handler() ///< Function called by epoll timer handler when TTI timer expires void ttcn3_syssim::new_tti_indication(uint64_t res) { + std::lock_guard lock(syssim_mutex); + tti = (tti + 1) % 10240; logger.set_context(tti); @@ -384,6 +388,7 @@ void ttcn3_syssim::stop() void ttcn3_syssim::reset() { + std::lock_guard lock(syssim_mutex); logger.info("Resetting SS"); cells.clear(); pcell_idx = -1; @@ -480,9 +485,11 @@ void ttcn3_syssim::disable_data() event_queue.push(DISABLE_DATA); } -// Called from PHY but always from the SS main thread with lock being hold +// Called from PHY through RA procedure running on Stack thread void ttcn3_syssim::prach_indication(uint32_t preamble_index_, const uint32_t& cell_id) { + std::lock_guard lock(syssim_mutex); + // verify that UE intends to send PRACH on current Pcell if (cells[pcell_idx]->config.phy_cell.id != cell_id) { logger.error( @@ -1261,8 +1268,10 @@ void ttcn3_syssim::release_as_security_impl(const std::string cell_name) cell->pending_bearer_config.clear(); } +// Called from PHY in Stack context void ttcn3_syssim::select_cell(srsran_cell_t phy_cell) { + std::lock_guard lock(syssim_mutex); // find matching cell in SS cell list for (uint32_t i = 0; i < cells.size(); ++i) { if (cells[i]->config.phy_cell.id == phy_cell.id) {