From f7aef3ffc175fd0f0ec80df84520fa905291e040 Mon Sep 17 00:00:00 2001 From: Francisco Date: Tue, 1 Jun 2021 18:05:15 +0100 Subject: [PATCH] sched,bugfix: fix nof_retx update when UL HARQs are resumed --- lib/include/srsran/common/srsran_assert.h | 2 +- .../hdr/stack/mac/sched_ue_ctrl/sched_harq.h | 9 ++-- srsenb/src/stack/mac/sched_grid.cc | 41 ++++++++++++------- .../src/stack/mac/sched_ue_ctrl/sched_harq.cc | 36 ++++++++-------- srsenb/test/mac/sched_common_test_suite.cc | 4 +- srsenb/test/mac/sched_sim_ue.cc | 18 +++++--- srsenb/test/mac/sched_sim_ue.h | 2 +- srsenb/test/mac/sched_ue_ded_test_suite.cc | 27 +++++++----- 8 files changed, 84 insertions(+), 55 deletions(-) diff --git a/lib/include/srsran/common/srsran_assert.h b/lib/include/srsran/common/srsran_assert.h index aaad80874..45eaa4a6d 100644 --- a/lib/include/srsran/common/srsran_assert.h +++ b/lib/include/srsran/common/srsran_assert.h @@ -19,8 +19,8 @@ #define srsran_unlikely(expr) __builtin_expect(!!(expr), 0) #define srsran_terminate(fmt, ...) \ - std::fprintf(stderr, "%s:%d: " fmt "\n", __FILE__, __LINE__, ##__VA_ARGS__); \ srslog::flush(); \ + std::fprintf(stderr, "%s:%d: " fmt "\n", __FILE__, __LINE__, ##__VA_ARGS__); \ std::abort() #ifdef ASSERTS_ENABLED diff --git a/srsenb/hdr/stack/mac/sched_ue_ctrl/sched_harq.h b/srsenb/hdr/stack/mac/sched_ue_ctrl/sched_harq.h index 7805a8b87..5bd5278ca 100644 --- a/srsenb/hdr/stack/mac/sched_ue_ctrl/sched_harq.h +++ b/srsenb/hdr/stack/mac/sched_ue_ctrl/sched_harq.h @@ -106,14 +106,15 @@ public: uint32_t get_pending_data() const; bool has_pending_phich() const; bool pop_pending_phich(); - void phich_alloc_failed(); + void request_pdcch(); + void retx_skipped(); private: prb_interval allocation; int pending_data; - bool pending_phich = false; - bool is_msg3_ = false; - bool pdcch_required = false; + bool pending_phich = false; + bool is_msg3_ = false; + bool pdcch_requested = false; }; class harq_entity diff --git a/srsenb/src/stack/mac/sched_grid.cc b/srsenb/src/stack/mac/sched_grid.cc index 0f2fcff0c..c68df774f 100644 --- a/srsenb/src/stack/mac/sched_grid.cc +++ b/srsenb/src/stack/mac/sched_grid.cc @@ -637,17 +637,20 @@ alloc_result sf_sched::alloc_phich(sched_ue* user) auto* ul_sf_result = &cc_results->get_cc(cc_cfg->enb_cc_idx)->ul_sched_result; if (ul_sf_result->phich.full()) { - logger.warning("SCHED: Maximum number of PHICH allocations has been reached"); - h->phich_alloc_failed(); + logger.warning( + "SCHED: UL skipped retx rnti=0x%x, pid=%d. Cause: No PHICH space left", user->get_rnti(), h->get_id()); + h->pop_pending_phich(); return alloc_result::no_grant_space; } if (not user->phich_enabled(get_tti_rx(), cc_cfg->enb_cc_idx)) { // PHICH falls in measGap. PHICH hi=1 is assumed by UE. In case of NACK, the HARQ is going to be resumed later on. - logger.debug("SCHED: HARQ pid=%d for rnti=0x%x is being resumed due to PHICH - MeasGap collision", - h->get_id(), - user->get_rnti()); - h->phich_alloc_failed(); + logger.info( + "SCHED: UL skipped retx rnti=0x%x, pid=%d. Cause: PHICH-measGap collision", user->get_rnti(), h->get_id()); + h->pop_pending_phich(); // empty pending PHICH + // Note: Given that the UE assumes PHICH hi=1, it is not expecting PUSCH grants for tti_tx_ul. Requesting PDCCH + // for the UL Harq has the effect of forbidding PUSCH grants, since phich_tti == pdcch_tti. + h->request_pdcch(); return alloc_result::no_cch_space; } @@ -906,15 +909,23 @@ void sf_sched::generate_sched_results(sched_ue_list& ue_db) /* Resume UL HARQs with pending retxs that did not get allocated */ using phich_t = sched_interface::ul_sched_phich_t; auto& phich_list = cc_result->ul_sched_result.phich; - for (uint32_t i = 0; i < cc_result->ul_sched_result.phich.size(); ++i) { - auto& phich = phich_list[i]; - if (phich.phich == phich_t::NACK) { - auto& ue = *ue_db[phich.rnti]; - ul_harq_proc* h = ue.get_ul_harq(get_tti_tx_ul(), cc_cfg->enb_cc_idx); - if (not is_ul_alloc(ue.get_rnti()) and h != nullptr and not h->is_empty()) { - // There was a missed UL harq retx. Halt+Resume the HARQ - phich.phich = phich_t::ACK; - logger.debug("SCHED: rnti=0x%x UL harq pid=%d is being resumed", ue.get_rnti(), h->get_id()); + for (auto& ue_pair : ue_db) { + auto& ue = *ue_pair.second; + uint16_t rnti = ue.get_rnti(); + ul_harq_proc* h = ue.get_ul_harq(get_tti_tx_ul(), cc_cfg->enb_cc_idx); + if (h != nullptr and not h->is_empty() and not is_ul_alloc(rnti)) { + // There was a missed UL harq retx. Halt+Resume the HARQ + h->retx_skipped(); + auto same_rnti = [rnti](const phich_t& p) { return p.rnti == rnti; }; + phich_t* phich = std::find_if(phich_list.begin(), phich_list.end(), same_rnti); + if (phich != phich_list.end()) { + srsran_assert(phich->phich == phich_t::NACK, "Expected hi=0 in case of active UL HARQ that was not retx"); + logger.info("SCHED: UL skipped retx rnti=0x%x, pid=%d. Cause: %s", + ue.get_rnti(), + h->get_id(), + ue.pusch_enabled(get_tti_rx(), cc_cfg->enb_cc_idx, false) ? "lack of PHY resources" + : "PUSCH-measGap collision"); + phich->phich = phich_t::ACK; } } } diff --git a/srsenb/src/stack/mac/sched_ue_ctrl/sched_harq.cc b/srsenb/src/stack/mac/sched_ue_ctrl/sched_harq.cc index fc7ab9c8d..23826d26c 100644 --- a/srsenb/src/stack/mac/sched_ue_ctrl/sched_harq.cc +++ b/srsenb/src/stack/mac/sched_ue_ctrl/sched_harq.cc @@ -247,10 +247,10 @@ void ul_harq_proc::new_tx(tti_point tti_, int mcs, int tbs, prb_interval alloc, { allocation = alloc; new_tx_common(0, tti_point{tti_}, mcs, tbs, max_retx_); - pending_data = tbs; - pending_phich = true; - is_msg3_ = is_msg3; - pdcch_required = false; + pending_data = tbs; + pending_phich = true; + is_msg3_ = is_msg3; + pdcch_requested = false; } void ul_harq_proc::new_retx(tti_point tti_, int* mcs, int* tbs, prb_interval alloc) @@ -258,15 +258,15 @@ void ul_harq_proc::new_retx(tti_point tti_, int* mcs, int* tbs, prb_interval all // If PRBs changed, or there was no tx in last oportunity (e.g. HARQ is being resumed) allocation = alloc; new_retx_common(0, tti_point{tti_}, mcs, tbs); - pending_phich = true; - pdcch_required = false; + pending_phich = true; + pdcch_requested = false; } bool ul_harq_proc::retx_requires_pdcch(tti_point tti_, prb_interval alloc) const { // Adaptive retx if: (1) PRBs changed, (2) HARQ resumed due to last PUSCH retx being skipped (3) HARQ resumed due to // last PHICH alloc being skipped (e.g. due to measGaps) - return alloc != allocation or tti_ != to_tx_ul(tti) or pdcch_required; + return alloc != allocation or pdcch_requested; } bool ul_harq_proc::set_ack(uint32_t tb_idx, bool ack_) @@ -283,13 +283,17 @@ bool ul_harq_proc::has_pending_phich() const return pending_phich; } -void ul_harq_proc::phich_alloc_failed() +void ul_harq_proc::request_pdcch() { - pop_pending_phich(); - if (not is_empty(0)) { - // HARQ needs to be resumed. This is signalled by pending_phich flag - pdcch_required = true; - } + pdcch_requested = true; +} + +void ul_harq_proc::retx_skipped() +{ + // Note: This function should be called in case of PHICH allocation is successful + // Flagging "PDCCH required" for next retx, as HARQ is being resumed + pdcch_requested = true; + n_rtx[0]++; } bool ul_harq_proc::pop_pending_phich() @@ -307,9 +311,9 @@ bool ul_harq_proc::pop_pending_phich() void ul_harq_proc::reset_pending_data() { reset_pending_data_common(); - pending_data = 0; - is_msg3_ = false; - pdcch_required = false; + pending_data = 0; + is_msg3_ = false; + pdcch_requested = false; } uint32_t ul_harq_proc::get_pending_data() const diff --git a/srsenb/test/mac/sched_common_test_suite.cc b/srsenb/test/mac/sched_common_test_suite.cc index d04999f9a..71cc82213 100644 --- a/srsenb/test/mac/sched_common_test_suite.cc +++ b/srsenb/test/mac/sched_common_test_suite.cc @@ -266,7 +266,9 @@ int test_dci_content_common(const sf_output_res_t& sf_out, uint32_t enb_cc_idx) CONDERROR(pusch.tbs == 0, "Allocated PUSCH with invalid TBS=%d", pusch.tbs); CONDERROR(alloc_rntis.count(rnti) > 0, "The user rnti=0x%x got allocated multiple times in UL", rnti); alloc_rntis.insert(pusch.dci.rnti); - CONDERROR(not((pusch.current_tx_nb == 0) xor (pusch.dci.tb.rv != 0)), "Number of txs incorrectly set"); + CONDERROR(not(((pusch.current_tx_nb % 4) == 0) xor (pusch.dci.tb.rv != 0)), + "[rnti=0x%x] Number of txs incorrectly set", + rnti); if (not pusch.needs_pdcch) { // In case of non-adaptive retx or Msg3 continue; diff --git a/srsenb/test/mac/sched_sim_ue.cc b/srsenb/test/mac/sched_sim_ue.cc index 4ce4dc4ac..4606ba400 100644 --- a/srsenb/test/mac/sched_sim_ue.cc +++ b/srsenb/test/mac/sched_sim_ue.cc @@ -116,6 +116,9 @@ void ue_sim::update_ul_harqs(const sf_output_res_t& sf_out) // Update UL harqs with PHICH info bool found_phich = false; + bool is_msg3 = h.nof_txs == h.nof_retxs + 1 and ctxt.msg3_tti_rx.is_valid() and h.first_tti_rx == ctxt.msg3_tti_rx; + uint32_t max_retxs = is_msg3 ? sf_out.cc_params[0].cfg.maxharq_msg3tx : ctxt.ue_cfg.maxharq_tx; + bool last_retx = h.nof_retxs + 1 >= max_retxs; for (uint32_t i = 0; i < sf_out.ul_cc_result[cc].phich.size(); ++i) { const auto& phich = sf_out.ul_cc_result[cc].phich[i]; if (phich.rnti != ctxt.rnti) { @@ -124,25 +127,23 @@ void ue_sim::update_ul_harqs(const sf_output_res_t& sf_out) found_phich = true; bool is_ack = phich.phich == phich_t::ACK; - bool is_msg3 = - h.nof_txs == h.nof_retxs + 1 and ctxt.msg3_tti_rx.is_valid() and h.first_tti_rx == ctxt.msg3_tti_rx; - bool last_retx = h.nof_retxs + 1 >= (is_msg3 ? sf_out.cc_params[0].cfg.maxharq_msg3tx : ctxt.ue_cfg.maxharq_tx); if (is_ack or last_retx) { h.active = false; } } - if (h.active and not found_phich) { - // HARQ being resumed. Possibly due to measGap, PHICH may not be allocated. - logger.info("TESTER: rnti=0x%x, HARQ pid=%d being resumed.", ctxt.rnti, pid); + if (not found_phich and h.active) { + // There can be missing PHICH due to measGap collisions. In such case, we deactivate the harq and assume hi=1 h.active = false; } // Update UL harqs with PUSCH grants + bool pusch_found = false; for (uint32_t i = 0; i < sf_out.ul_cc_result[cc].pusch.size(); ++i) { const auto& data = sf_out.ul_cc_result[cc].pusch[i]; if (data.dci.rnti != ctxt.rnti) { continue; } + pusch_found = true; if (h.nof_txs == 0 or h.ndi != data.dci.tb.ndi) { // newtx @@ -158,6 +159,11 @@ void ue_sim::update_ul_harqs(const sf_output_res_t& sf_out) h.riv = data.dci.type2_alloc.riv; h.nof_txs++; } + if (not pusch_found and h.nof_retxs < max_retxs) { + // PUSCH *may* be skipped due to measGap. nof_retxs keeps getting incremented + h.nof_retxs++; + h.nof_txs++; + } } } diff --git a/srsenb/test/mac/sched_sim_ue.h b/srsenb/test/mac/sched_sim_ue.h index 34a84b88f..8a39426f5 100644 --- a/srsenb/test/mac/sched_sim_ue.h +++ b/srsenb/test/mac/sched_sim_ue.h @@ -26,7 +26,7 @@ struct ue_harq_ctxt_t { bool ndi = false; uint32_t pid = 0; uint32_t nof_txs = 0; - uint32_t nof_retxs = 0; + uint32_t nof_retxs = std::numeric_limits::max(); uint32_t riv = 0; srsran_dci_location_t dci_loc = {}; uint32_t tbs = 0; diff --git a/srsenb/test/mac/sched_ue_ded_test_suite.cc b/srsenb/test/mac/sched_ue_ded_test_suite.cc index 63fc56b87..dad865ebe 100644 --- a/srsenb/test/mac/sched_ue_ded_test_suite.cc +++ b/srsenb/test/mac/sched_ue_ded_test_suite.cc @@ -189,13 +189,16 @@ int test_ul_sched_result(const sim_enb_ctxt_t& enb_ctxt, const sf_output_res_t& continue; } - const auto& h = ue.cc_list[ue_cc_idx].ul_harqs[pid]; - bool phich_ack = phich_ptr != nullptr and phich_ptr->phich == phich_t::ACK; - bool is_msg3 = h.first_tti_rx == ue.msg3_tti_rx and h.nof_txs == h.nof_retxs + 1; - bool last_retx = h.nof_retxs + 1 >= (is_msg3 ? sf_out.cc_params[0].cfg.maxharq_msg3tx : ue.ue_cfg.maxharq_tx); - tti_point tti_tx_phich = to_tx_dl(sf_out.tti_rx); - bool phich_in_meas_gap = is_in_measgap(tti_tx_phich, ue.ue_cfg.measgap_period, ue.ue_cfg.measgap_offset); - bool h_inactive_now = (not h.active) or (phich_ack or last_retx or phich_in_meas_gap); + const auto& h = ue.cc_list[ue_cc_idx].ul_harqs[pid]; + bool phich_ack = phich_ptr != nullptr and phich_ptr->phich == phich_t::ACK; + bool is_msg3 = h.first_tti_rx == ue.msg3_tti_rx and h.nof_txs == h.nof_retxs + 1; + uint32_t max_nof_retxs = is_msg3 ? sf_out.cc_params[0].cfg.maxharq_msg3tx : ue.ue_cfg.maxharq_tx; + bool last_retx = h.nof_retxs + 1 >= max_nof_retxs; + tti_point tti_tx_phich = to_tx_dl(sf_out.tti_rx); + bool phich_in_meas_gap = is_in_measgap(tti_tx_phich, ue.ue_cfg.measgap_period, ue.ue_cfg.measgap_offset); + bool pusch_in_meas_gap = + is_in_measgap(to_tx_ul(sf_out.tti_rx), ue.ue_cfg.measgap_period, ue.ue_cfg.measgap_offset); + bool h_cleared = (not h.active) or (phich_ack or last_retx); // TEST: Already active UL HARQs have to receive PHICH (unless MeasGap collision) CONDERROR(h.active and phich_ptr == nullptr and not phich_in_meas_gap, @@ -209,7 +212,7 @@ int test_ul_sched_result(const sim_enb_ctxt_t& enb_ctxt, const sf_output_res_t& // TEST: absent PUSCH grants for active UL HARQs must be either ACKs, last retx, or interrupted HARQs if (phich_ptr != nullptr) { - CONDERROR(not h_inactive_now and pusch_ptr == nullptr, + CONDERROR(not h_cleared and pusch_ptr == nullptr and not pusch_in_meas_gap, "PHICH NACK received for rnti=0x%x but no PUSCH retx reallocated", rnti); } @@ -224,11 +227,13 @@ int test_ul_sched_result(const sim_enb_ctxt_t& enb_ctxt, const sf_output_res_t& // newtx CONDERROR(nof_retx != 0, "Invalid rv index for new UL tx"); CONDERROR(pusch_ptr->current_tx_nb != 0, "UL HARQ retxs need to have been previously transmitted"); - CONDERROR(not h_inactive_now, "New tx for already active UL HARQ"); + CONDERROR(not h_cleared, "New tx for already active UL HARQ"); CONDERROR(not pusch_ptr->needs_pdcch and ue.msg3_tti_rx.is_valid() and sf_out.tti_rx > ue.msg3_tti_rx, "In case of newtx, PDCCH allocation is required, unless it is Msg3"); } else { CONDERROR(pusch_ptr->current_tx_nb == 0, "UL retx has to have nof tx > 0"); + CONDERROR(h.nof_retxs >= max_nof_retxs, "UL max nof retxs exceeded"); + CONDERROR(pusch_ptr->current_tx_nb != h.nof_retxs + 1, "UL HARQ nof_retx mismatch"); if (not h.active) { // the HARQ is being resumed. PDCCH must be active with the exception of Msg3 CONDERROR(ue.msg4_tti_rx.is_valid() and not pusch_ptr->needs_pdcch, @@ -239,11 +244,11 @@ int test_ul_sched_result(const sim_enb_ctxt_t& enb_ctxt, const sf_output_res_t& } else { // non-adaptive retx CONDERROR(pusch_ptr->dci.type2_alloc.riv != h.riv, "Non-adaptive retx must keep the same riv"); - CONDERROR(to_tx_ul(h.last_tti_rx) > sf_out.tti_rx, "UL harq pid=%d was reused too soon", h.pid); } } - CONDERROR(get_rvidx(h.nof_retxs + 1) != (uint32_t)pusch_ptr->dci.tb.rv, "Invalid rv index for retx"); + CONDERROR(get_rvidx(h.nof_retxs + 1) != (uint32_t)pusch_ptr->dci.tb.rv, "Invalid rv index for UL retx"); CONDERROR(h.tbs != pusch_ptr->tbs, "TBS changed during HARQ retx"); + CONDERROR(to_tx_ul(h.last_tti_rx) > sf_out.tti_rx, "UL harq pid=%d was reused too soon", h.pid); } } }