From cc2d71183e100abef2625411f87c404ea84070cf Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Tue, 14 Sep 2021 21:10:40 +0200 Subject: [PATCH] ue,phy,nr: fix race condition when setting UL grant for Msg3 the Msg3 UL grant requires the TTI in which the RAR has been received to calcualte the correct timing. There was a race between PHY and Stack thread. This patch circumvents the issue by removing a PHY state member that only holds the RAR Rx timing. In the new interface the RA proc passes the Rx TTI to the PHY again when setting the UL grant so the PHY can calculate the correct timing without any state. --- lib/include/srsran/interfaces/ue_nr_interfaces.h | 8 +++++--- srsue/hdr/phy/nr/state.h | 2 -- srsue/hdr/phy/nr/worker_pool.h | 3 ++- srsue/hdr/phy/phy.h | 3 ++- srsue/src/phy/nr/cc_worker.cc | 9 +++------ srsue/src/phy/nr/worker_pool.cc | 13 +++++++++---- srsue/src/phy/phy.cc | 5 +++-- srsue/src/stack/mac_nr/proc_ra_nr.cc | 2 +- srsue/src/stack/mac_nr/test/mac_nr_test.cc | 5 ++++- srsue/src/stack/mac_nr/test/proc_ra_nr_test.cc | 5 ++++- srsue/src/stack/mac_nr/test/proc_sr_nr_test.cc | 5 ++++- 11 files changed, 37 insertions(+), 23 deletions(-) diff --git a/lib/include/srsran/interfaces/ue_nr_interfaces.h b/lib/include/srsran/interfaces/ue_nr_interfaces.h index 04fc73ee5..030082010 100644 --- a/lib/include/srsran/interfaces/ue_nr_interfaces.h +++ b/lib/include/srsran/interfaces/ue_nr_interfaces.h @@ -54,8 +54,9 @@ public: } mac_nr_grant_dl_t; typedef struct { - srsran::unique_byte_buffer_t payload; // TB when decoded successfully, nullptr otherwise - bool ack; // HARQ information + uint32_t rx_slot_idx; // Slot when DL TB has been decoded + srsran::unique_byte_buffer_t payload; // TB when decoded successfully, nullptr otherwise + bool ack; // HARQ information } tb_action_dl_result_t; // UL grant as conveyed between PHY and MAC @@ -212,7 +213,8 @@ public: } tx_request_t; // MAC informs PHY about UL grant included in RAR PDU - virtual int set_ul_grant(std::array packed_ul_grant, + virtual int set_ul_grant(uint32_t rar_slot_idx, + std::array packed_ul_grant, uint16_t rnti, srsran_rnti_type_t rnti_type) = 0; diff --git a/srsue/hdr/phy/nr/state.h b/srsue/hdr/phy/nr/state.h index f804b8a6a..c6f8c3186 100644 --- a/srsue/hdr/phy/nr/state.h +++ b/srsue/hdr/phy/nr/state.h @@ -90,8 +90,6 @@ public: /// Semaphore for aligning UL work srsran::tti_semaphore dl_ul_semaphore; - srsran_slot_cfg_t rar_grant_slot = {}; - state() { // Hard-coded values, this should be set when the measurements take place diff --git a/srsue/hdr/phy/nr/worker_pool.h b/srsue/hdr/phy/nr/worker_pool.h index c0d96232e..b49b90dfe 100644 --- a/srsue/hdr/phy/nr/worker_pool.h +++ b/srsue/hdr/phy/nr/worker_pool.h @@ -41,7 +41,8 @@ public: const int preamble_index, const float preamble_received_target_power, const float ta_base_sec = 0.0f) override; - int set_ul_grant(std::array array, + int set_ul_grant(uint32_t rx_tti, + std::array array, uint16_t rnti, srsran_rnti_type_t rnti_type) override; bool set_config(const srsran::phy_cfg_nr_t& cfg) override; diff --git a/srsue/hdr/phy/phy.h b/srsue/hdr/phy/phy.h index 96a98611d..2a4ec37c6 100644 --- a/srsue/hdr/phy/phy.h +++ b/srsue/hdr/phy/phy.h @@ -165,7 +165,8 @@ public: int init(const phy_args_nr_t& args_, stack_interface_phy_nr* stack_, srsran::radio_interface_phy* radio_) final; bool set_config(const srsran::phy_cfg_nr_t& cfg) final; - int set_ul_grant(std::array packed_ul_grant, + int set_ul_grant(uint32_t rx_tti, + std::array packed_ul_grant, uint16_t rnti, srsran_rnti_type_t rnti_type) final; void send_prach(const uint32_t prach_occasion, diff --git a/srsue/src/phy/nr/cc_worker.cc b/srsue/src/phy/nr/cc_worker.cc index 74210d093..337baa17e 100644 --- a/srsue/src/phy/nr/cc_worker.cc +++ b/srsue/src/phy/nr/cc_worker.cc @@ -336,14 +336,11 @@ bool cc_worker::decode_pdsch_dl() // Notify MAC about PDSCH decoding result mac_interface_phy_nr::tb_action_dl_result_t mac_dl_result = {}; - mac_dl_result.ack = pdsch_res.tb[0].crc; - mac_dl_result.payload = mac_dl_result.ack ? std::move(data) : nullptr; // only pass data when successful + mac_dl_result.rx_slot_idx = dl_slot_cfg.idx; // Rx TTI for this TB (required for correct Msg3 timing) + mac_dl_result.ack = pdsch_res.tb[0].crc; + mac_dl_result.payload = mac_dl_result.ack ? std::move(data) : nullptr; // only pass data when successful phy.stack->tb_decoded(cc_idx, mac_dl_grant, std::move(mac_dl_result)); - if (pdsch_cfg.grant.rnti_type == srsran_rnti_type_ra) { - phy.rar_grant_slot = dl_slot_cfg; - } - if (pdsch_res.tb[0].crc) { // Generate DL metrics dl_metrics_t dl_m = {}; diff --git a/srsue/src/phy/nr/worker_pool.cc b/srsue/src/phy/nr/worker_pool.cc index 00a2fb446..d41183d4b 100644 --- a/srsue/src/phy/nr/worker_pool.cc +++ b/srsue/src/phy/nr/worker_pool.cc @@ -99,7 +99,9 @@ void worker_pool::send_prach(const uint32_t prach_occasion, prach_buffer->prepare_to_send(preamble_index); } -int worker_pool::set_ul_grant(std::array packed_ul_grant, +// called from Stack thread when processing RAR PDU +int worker_pool::set_ul_grant(uint32_t rar_slot_idx, + std::array packed_ul_grant, uint16_t rnti, srsran_rnti_type_t rnti_type) { @@ -113,21 +115,24 @@ int worker_pool::set_ul_grant(std::array pac srsran_vec_u8_copy(dci_msg.payload, packed_ul_grant.data(), SRSRAN_RAR_UL_GRANT_NBITS); srsran_dci_ul_nr_t dci_ul = {}; - if (srsran_dci_nr_ul_unpack(NULL, &dci_msg, &dci_ul) < SRSRAN_SUCCESS) { logger.error("Couldn't unpack UL grant"); return SRSRAN_ERROR; } + // initialize with Rx TTI of RAR + srsran_slot_cfg_t msg3_slot_cfg = {}; + msg3_slot_cfg.idx = rar_slot_idx; + if (logger.info.enabled()) { std::array str; srsran_dci_nr_t dci = {}; srsran_dci_ul_nr_to_str(&dci, &dci_ul, str.data(), str.size()); - logger.set_context(phy_state.rar_grant_slot.idx); + logger.set_context(msg3_slot_cfg.idx); logger.info("Setting RAR Grant: %s", str.data()); } - phy_state.set_ul_pending_grant(phy_state.rar_grant_slot, dci_ul); + phy_state.set_ul_pending_grant(msg3_slot_cfg, dci_ul); return SRSRAN_SUCCESS; } diff --git a/srsue/src/phy/phy.cc b/srsue/src/phy/phy.cc index ede466fd1..c6fb51f3a 100644 --- a/srsue/src/phy/phy.cc +++ b/srsue/src/phy/phy.cc @@ -622,11 +622,12 @@ int phy::init(const phy_args_nr_t& args_, stack_interface_phy_nr* stack_, srsran return SRSRAN_SUCCESS; } -int phy::set_ul_grant(std::array packed_ul_grant, +int phy::set_ul_grant(uint32_t rar_slot_idx, + std::array packed_ul_grant, uint16_t rnti, srsran_rnti_type_t rnti_type) { - return nr_workers.set_ul_grant(packed_ul_grant, rnti, rnti_type); + return nr_workers.set_ul_grant(rar_slot_idx, packed_ul_grant, rnti, rnti_type); } void phy::send_prach(const uint32_t prach_occasion, diff --git a/srsue/src/stack/mac_nr/proc_ra_nr.cc b/srsue/src/stack/mac_nr/proc_ra_nr.cc index 4177df8b0..402bec56b 100644 --- a/srsue/src/stack/mac_nr/proc_ra_nr.cc +++ b/srsue/src/stack/mac_nr/proc_ra_nr.cc @@ -207,7 +207,7 @@ void proc_ra_nr::ra_response_reception(const mac_interface_phy_nr::tb_action_dl_ temp_crnti = subpdu.get_temp_crnti(); // Set Temporary-C-RNTI if provided, otherwise C-RNTI is ok - phy->set_ul_grant(subpdu.get_ul_grant(), temp_crnti, srsran_rnti_type_ra); + phy->set_ul_grant(tb.rx_slot_idx, subpdu.get_ul_grant(), temp_crnti, srsran_rnti_type_ra); // reset all parameters that are used before rar rar_rnti = SRSRAN_INVALID_RNTI; diff --git a/srsue/src/stack/mac_nr/test/mac_nr_test.cc b/srsue/src/stack/mac_nr/test/mac_nr_test.cc index 17de4be67..b7899bb10 100644 --- a/srsue/src/stack/mac_nr/test/mac_nr_test.cc +++ b/srsue/src/stack/mac_nr/test/mac_nr_test.cc @@ -36,7 +36,10 @@ public: preamble_received_target_power = preamble_received_target_power_; } int tx_request(const tx_request_t& request) override { return 0; } - int set_ul_grant(std::array, uint16_t rnti, srsran_rnti_type_t rnti_type) override + int set_ul_grant(uint32_t rar_slot_idx, + std::array, + uint16_t rnti, + srsran_rnti_type_t rnti_type) override { return 0; } diff --git a/srsue/src/stack/mac_nr/test/proc_ra_nr_test.cc b/srsue/src/stack/mac_nr/test/proc_ra_nr_test.cc index 4867c6edb..fabf2632e 100644 --- a/srsue/src/stack/mac_nr/test/proc_ra_nr_test.cc +++ b/srsue/src/stack/mac_nr/test/proc_ra_nr_test.cc @@ -30,7 +30,10 @@ public: preamble_received_target_power = preamble_received_target_power_; } int tx_request(const tx_request_t& request) override { return 0; } - int set_ul_grant(std::array, uint16_t rnti, srsran_rnti_type_t rnti_type) override + int set_ul_grant(uint32_t rar_slot_idx, + std::array, + uint16_t rnti, + srsran_rnti_type_t rnti_type) override { return 0; } diff --git a/srsue/src/stack/mac_nr/test/proc_sr_nr_test.cc b/srsue/src/stack/mac_nr/test/proc_sr_nr_test.cc index 666bce515..83ecb1fad 100644 --- a/srsue/src/stack/mac_nr/test/proc_sr_nr_test.cc +++ b/srsue/src/stack/mac_nr/test/proc_sr_nr_test.cc @@ -31,7 +31,10 @@ public: preamble_received_target_power = preamble_received_target_power_; } int tx_request(const tx_request_t& request) override { return 0; } - int set_ul_grant(std::array, uint16_t rnti, srsran_rnti_type_t rnti_type) override + int set_ul_grant(uint32_t rar_slot_idx, + std::array, + uint16_t rnti, + srsran_rnti_type_t rnti_type) override { return 0; }