From 7cf919e2af84dd687e47a30ce9439b0879daf5ce Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Mon, 9 Nov 2020 10:50:02 +0100 Subject: [PATCH] Fix MAC PDU packing after sending Truncated BSR (#2004) * mac_test: add extended TBSR unit test unit test to MAC UL packing after sending a TBSR this fixes the MAC issues described in issue #2002 * mux: fix updating of LCG buffer state after packing PDU we've previously lowered the buffer state of the LCG according to the bytes that have been scheduled, but not according to those that have been actually included in the PDU. * proc_bsr: fix LCG buffer state updating for TBSR when sending a TBSR do not update the internal buffer state of the BSR proc. This caused issues because the buffer state for all LCG that are not included in the TBSR are set to zero, although at least one LCG does have data to transmit. * rlc_am: include LCID when logging retx of SN --- lib/src/upper/rlc_am_lte.cc | 2 +- srsue/hdr/stack/mac/mux.h | 2 +- srsue/src/stack/mac/mux.cc | 18 +-- srsue/src/stack/mac/proc_bsr.cc | 6 + srsue/test/mac_test.cc | 259 +++++++++++++++++++++++--------- 5 files changed, 201 insertions(+), 86 deletions(-) diff --git a/lib/src/upper/rlc_am_lte.cc b/lib/src/upper/rlc_am_lte.cc index c51a706a7..132990ec0 100644 --- a/lib/src/upper/rlc_am_lte.cc +++ b/lib/src/upper/rlc_am_lte.cc @@ -463,7 +463,7 @@ void rlc_am_lte::rlc_am_lte_tx::retransmit_random_pdu() // randomly select PDU in tx window for retransmission std::map::iterator it = tx_window.begin(); std::advance(it, rand() % tx_window.size()); - log->info("Schedule SN=%d for reTx.\n", it->first); + log->info("%s Schedule SN=%d for reTx.\n", RB_NAME, it->first); rlc_amd_retx_t retx = {}; retx.is_segment = false; retx.so_start = 0; diff --git a/srsue/hdr/stack/mac/mux.h b/srsue/hdr/stack/mac/mux.h index 8a9bb4227..72e9b0ed5 100644 --- a/srsue/hdr/stack/mac/mux.h +++ b/srsue/hdr/stack/mac/mux.h @@ -81,7 +81,7 @@ public: private: bool has_logical_channel(const uint32_t& lcid); bool pdu_move_to_msg3(uint32_t pdu_sz); - bool allocate_sdu(uint32_t lcid, srslte::sch_pdu* pdu, int max_sdu_sz); + uint32_t allocate_sdu(uint32_t lcid, srslte::sch_pdu* pdu, int max_sdu_sz); bool sched_sdu(logical_channel_config_t* ch, int* sdu_space, int max_sdu_sz); const static int MAX_NOF_SUBHEADERS = 20; diff --git a/srsue/src/stack/mac/mux.cc b/srsue/src/stack/mac/mux.cc index cb75b3617..937c6270e 100644 --- a/srsue/src/stack/mac/mux.cc +++ b/srsue/src/stack/mac/mux.cc @@ -272,10 +272,10 @@ uint8_t* mux::pdu_get(srslte::byte_buffer_t* payload, uint32_t pdu_sz) for (auto& channel : logical_channels) { if (channel.sched_len != 0) { - allocate_sdu(channel.lcid, &pdu_msg, channel.sched_len); + uint32_t sdu_len = allocate_sdu(channel.lcid, &pdu_msg, channel.sched_len); - // update BSR according to allocation - bsr.buff_size[channel.lcg] -= channel.sched_len; + // update BSR according to allocation (may be smaller than sched_len) + bsr.buff_size[channel.lcg] -= sdu_len; } } @@ -334,11 +334,11 @@ bool mux::sched_sdu(logical_channel_config_t* ch, int* sdu_space, int max_sdu_sz return false; } -bool mux::allocate_sdu(uint32_t lcid, srslte::sch_pdu* pdu_msg, int max_sdu_sz) +uint32_t mux::allocate_sdu(uint32_t lcid, srslte::sch_pdu* pdu_msg, int max_sdu_sz) { - bool sdu_added = false; - int sdu_space = max_sdu_sz; - int buffer_state = rlc->get_buffer_state(lcid); + uint32_t total_sdu_len = 0; + int32_t sdu_space = max_sdu_sz; + int32_t buffer_state = rlc->get_buffer_state(lcid); while (buffer_state > 0 && sdu_space > 0) { // there is pending SDU to allocate int requested_sdu_len = SRSLTE_MIN(buffer_state, sdu_space); @@ -356,7 +356,7 @@ bool mux::allocate_sdu(uint32_t lcid, srslte::sch_pdu* pdu_msg, int max_sdu_sz) max_sdu_sz, pdu_msg->rem_size()); sdu_space -= sdu_len; - sdu_added = true; + total_sdu_len += sdu_len; buffer_state = rlc->get_buffer_state(lcid); } else { @@ -382,7 +382,7 @@ bool mux::allocate_sdu(uint32_t lcid, srslte::sch_pdu* pdu_msg, int max_sdu_sz) break; } } - return sdu_added; + return total_sdu_len; } void mux::msg3_flush() diff --git a/srsue/src/stack/mac/proc_bsr.cc b/srsue/src/stack/mac/proc_bsr.cc index b15f2a6d1..36fb77a39 100644 --- a/srsue/src/stack/mac/proc_bsr.cc +++ b/srsue/src/stack/mac/proc_bsr.cc @@ -277,6 +277,12 @@ bool bsr_proc::generate_bsr(bsr_t* bsr, uint32_t pdu_space) void bsr_proc::update_bsr_tti_end(const bsr_t* bsr) { std::lock_guard lock(mutex); + + // Don't handle TBSR as it would reset old state for all non-reported LCGs, which might be wrong. + if (bsr->format == TRUNC_BSR) { + return; + } + for (uint32_t i = 0; i < NOF_LCG; i++) { if (bsr->buff_size[i] == 0) { for (std::map::iterator iter = lcgs[i].begin(); iter != lcgs[i].end(); ++iter) { diff --git a/srsue/test/mac_test.cc b/srsue/test/mac_test.cc index 44f7b3752..073e36c4e 100644 --- a/srsue/test/mac_test.cc +++ b/srsue/test/mac_test.cc @@ -1599,6 +1599,173 @@ int mac_ul_sch_periodic_bsr_test() return SRSLTE_SUCCESS; } +/** + * Test handling of TBSR + * + * with { data available for two LCGs, small grant arrives and TBSR is sent for highest-priorit LCG } + * ensure that { + * when { bigger grant becomes available } + * then { No further (L)BSR is included, only SDUs are sent } + * + * with { data available for two LCGs } + * ensure that { + * when { small grant arrives but RLC can't generate PDU } + * then { TBSR is generated with highest priority LCG } + */ +int mac_ul_sch_trunc_bsr_test2() +{ + // 1st and 3rd PDU, TBSR + const uint8_t tv1[] = {0x3f, 0x1c, 0x01}; + + // Following PDU includes only SDUs for highest priority LCID + const uint8_t tv2[] = {0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01}; + + srslte::log_filter rlc_log("RLC"); + rlc_log.set_level(srslte::LOG_LEVEL_DEBUG); + rlc_log.set_hex_limit(100000); + + // dummy layers + phy_dummy phy; + rlc_dummy rlc(&rlc_log); + rrc_dummy rrc; + stack_dummy stack; + + // the actual MAC + mac mac("MAC", &stack.task_sched); + stack.init(&mac, &phy); + mac.init(&phy, &rlc, &rrc); + + mac_cfg_t mac_cfg = {}; + mac_cfg.bsr_cfg.periodic_timer = 20; + mac.set_config(mac_cfg); + + const uint16_t crnti = 0x1001; + mac.set_ho_rnti(crnti, 0); + + // generate configs for two LCIDs with different priority and PBR + std::vector lcids; + logical_channel_config_t config = {}; + // The config of SRB1 + config.lcid = 1; + config.lcg = 0; + config.PBR = 8; // 8 kByte/s + config.BSD = 100; // 100ms + config.priority = 7; + lcids.push_back(config); + + // DRB + config.lcid = 3; + config.lcg = 2; + config.PBR = 0; // no PBR + config.priority = 15; // lower prio than SRB1 + lcids.push_back(config); + + // setup LCIDs in MAC + for (auto& channel : lcids) { + mac.setup_lcid(channel.lcid, channel.lcg, channel.priority, channel.PBR, channel.BSD); + } + + // generate SRB and DRB data + rlc.write_sdu(1, 9); + rlc.write_sdu(3, 100); + + // generate TTI + uint32 tti = 0; + stack.run_tti(tti++); + usleep(100); + + bool ndi = true; + + // create UL action and grant and push MAC PDU + { + mac_interface_phy_lte::tb_action_ul_t ul_action = {}; + mac_interface_phy_lte::mac_grant_ul_t mac_grant = {}; + + mac_grant.rnti = crnti; // make sure MAC picks it up as valid UL grant + mac_grant.tb.ndi_present = true; + mac_grant.tb.ndi = ndi; + mac_grant.tb.tbs = 3; // only space for TBSR + int cc_idx = 0; + + // Send grant to MAC and get action for this TB, then call tb_decoded to unlock MAC + mac.new_grant_ul(cc_idx, mac_grant, &ul_action); + + // print generated PDU + mac_log->info_hex(ul_action.tb.payload, mac_grant.tb.tbs, "Generated PDU (%d B)\n", mac_grant.tb.tbs); +#if HAVE_PCAP + pcap_handle->write_ul_crnti(ul_action.tb.payload, mac_grant.tb.tbs, 0x1001, true, 1, 0); +#endif + + TESTASSERT(memcmp(ul_action.tb.payload, tv1, sizeof(tv1)) == 0); + } + + // trigger next TTI + stack.run_tti(tti++); + usleep(100); + + // toggle NDI + ndi = !ndi; + + { + mac_interface_phy_lte::tb_action_ul_t ul_action = {}; + mac_interface_phy_lte::mac_grant_ul_t mac_grant = {}; + + mac_grant.rnti = crnti; // make sure MAC picks it up as valid UL grant + mac_grant.tb.ndi_present = true; + mac_grant.tb.ndi = ndi; + mac_grant.tb.tbs = 9; // room SDU + int cc_idx = 0; + + // Send grant to MAC and get action for this TB, then call tb_decoded to unlock MAC + mac.new_grant_ul(cc_idx, mac_grant, &ul_action); + + // print generated PDU + mac_log->info_hex(ul_action.tb.payload, mac_grant.tb.tbs, "Generated PDU (%d B)\n", mac_grant.tb.tbs); +#if HAVE_PCAP + pcap_handle->write_ul_crnti(ul_action.tb.payload, mac_grant.tb.tbs, 0x1001, true, 1, 0); +#endif + + TESTASSERT(memcmp(ul_action.tb.payload, tv2, sizeof(tv2)) == 0); + } + + // trigger next TTI + stack.run_tti(tti++); + usleep(100); + + ndi = !ndi; + + // turn of RLC read (simulate RLC can't generate PDU) + rlc.disable_read(); + + { + mac_interface_phy_lte::tb_action_ul_t ul_action = {}; + mac_interface_phy_lte::mac_grant_ul_t mac_grant = {}; + + mac_grant.rnti = crnti; // make sure MAC picks it up as valid UL grant + mac_grant.tb.ndi_present = true; + mac_grant.tb.ndi = ndi; // toggled NDI + mac_grant.tb.tbs = 3; // again only room for TBSR + int cc_idx = 0; + + // Send grant to MAC and get action for this TB, then call tb_decoded to unlock MAC + mac.new_grant_ul(cc_idx, mac_grant, &ul_action); + + // print generated PDU + mac_log->info_hex(ul_action.tb.payload, mac_grant.tb.tbs, "Generated PDU (%d B)\n", mac_grant.tb.tbs); +#if HAVE_PCAP + pcap_handle->write_ul_crnti(ul_action.tb.payload, mac_grant.tb.tbs, 0x1001, true, 1, 0); +#endif + + TESTASSERT(memcmp(ul_action.tb.payload, tv1, sizeof(tv1)) == 0); + } + + // make sure MAC PDU thread picks up before stopping + stack.run_tti(tti); + mac.stop(); + + return SRSLTE_SUCCESS; +} + // Single byte MAC PDU int mac_ul_sch_pdu_one_byte_test() { @@ -2175,80 +2342,22 @@ int main(int argc, char** argv) mac_log->set_level(srslte::LOG_LEVEL_DEBUG); mac_log->set_hex_limit(100000); - if (mac_unpack_test()) { - printf("MAC PDU unpack test failed.\n"); - return -1; - } - - if (mac_ul_sch_pdu_test1()) { - printf("mac_ul_sch_pdu_test1() test failed.\n"); - return -1; - } - - if (mac_ul_logical_channel_prioritization_test1()) { - printf("mac_ul_logical_channel_prioritization_test1() test failed.\n"); - return -1; - } - - if (mac_ul_logical_channel_prioritization_test2()) { - printf("mac_ul_logical_channel_prioritization_test2() test failed.\n"); - return -1; - } - - if (mac_ul_logical_channel_prioritization_test3()) { - printf("mac_ul_logical_channel_prioritization_test3() test failed.\n"); - return -1; - } - - if (mac_ul_logical_channel_prioritization_test4()) { - printf("mac_ul_logical_channel_prioritization_test4() test failed.\n"); - return -1; - } - - if (mac_ul_sch_pdu_with_short_bsr_test()) { - printf("mac_ul_sch_pdu_with_short_bsr_test() test failed.\n"); - return -1; - } - - if (mac_ul_sch_pdu_with_padding_long_bsr_test()) { - printf("mac_ul_sch_pdu_with_padding_long_bsr_test() test failed.\n"); - return -1; - } - - if (mac_ul_sch_pdu_with_padding_trunc_bsr_test()) { - printf("mac_ul_sch_pdu_with_padding_trunc_bsr_test() test failed.\n"); - return -1; - } - - if (mac_ul_sch_regular_bsr_retx_test()) { - printf("mac_ul_sch_regular_bsr_retx_test() test failed.\n"); - return -1; - } - - if (mac_ul_sch_periodic_bsr_test()) { - printf("mac_ul_sch_periodic_bsr_test() test failed.\n"); - return -1; - } - - if (mac_ul_sch_pdu_one_byte_test()) { - printf("mac_ul_sch_pdu_one_byte_test() test failed.\n"); - return -1; - } + TESTASSERT(mac_unpack_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_pdu_test1() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_logical_channel_prioritization_test1() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_logical_channel_prioritization_test2() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_logical_channel_prioritization_test3() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_logical_channel_prioritization_test4() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_pdu_with_short_bsr_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_pdu_with_padding_long_bsr_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_pdu_with_padding_trunc_bsr_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_regular_bsr_retx_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_periodic_bsr_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_trunc_bsr_test2() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_pdu_one_byte_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_pdu_two_byte_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_ul_sch_pdu_three_byte_test() == SRSLTE_SUCCESS); + TESTASSERT(mac_random_access_test() == SRSLTE_SUCCESS); - if (mac_ul_sch_pdu_two_byte_test()) { - printf("mac_ul_sch_pdu_two_byte_test() test failed.\n"); - return -1; - } - - if (mac_ul_sch_pdu_three_byte_test()) { - printf("mac_ul_sch_pdu_three_byte_test() test failed.\n"); - return -1; - } - - if (mac_random_access_test()) { - printf("mac_random_access_test() test failed.\n"); - return -1; - } - - return 0; + return SRSLTE_SUCCESS; }