From 8140e62a7f318f0f1330a150e7f4a8fae429691d Mon Sep 17 00:00:00 2001 From: Xavier Arteaga Date: Fri, 11 Feb 2022 15:17:50 +0100 Subject: [PATCH 01/28] GNB-PHY: fix max PUSCH LDPC iterations --- lib/src/phy/gnb/gnb_ul.c | 2 +- srsenb/src/phy/nr/slot_worker.cc | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/src/phy/gnb/gnb_ul.c b/lib/src/phy/gnb/gnb_ul.c index 13fd7a0cc..f3f77c4bb 100644 --- a/lib/src/phy/gnb/gnb_ul.c +++ b/lib/src/phy/gnb/gnb_ul.c @@ -359,4 +359,4 @@ uint32_t srsran_gnb_ul_pusch_info(srsran_gnb_ul_t* q, len += srsran_csi_meas_info_short(&q->dmrs.csi, &str[len], str_len - len); return len; -} \ No newline at end of file +} diff --git a/srsenb/src/phy/nr/slot_worker.cc b/srsenb/src/phy/nr/slot_worker.cc index 0dba28c1b..c887a1d1a 100644 --- a/srsenb/src/phy/nr/slot_worker.cc +++ b/srsenb/src/phy/nr/slot_worker.cc @@ -80,13 +80,14 @@ bool slot_worker::init(const args_t& args) } // Prepare UL arguments - srsran_gnb_ul_args_t ul_args = {}; - ul_args.pusch.measure_time = true; - ul_args.pusch.measure_evm = true; - ul_args.pusch.max_layers = args.nof_rx_ports; - ul_args.pusch.max_prb = args.nof_max_prb; - ul_args.nof_max_prb = args.nof_max_prb; - ul_args.pusch_min_snr_dB = args.pusch_min_snr_dB; + srsran_gnb_ul_args_t ul_args = {}; + ul_args.pusch.measure_time = true; + ul_args.pusch.measure_evm = true; + ul_args.pusch.max_layers = args.nof_rx_ports; + ul_args.pusch.sch.max_nof_iter = args.pusch_max_its; + ul_args.pusch.max_prb = args.nof_max_prb; + ul_args.nof_max_prb = args.nof_max_prb; + ul_args.pusch_min_snr_dB = args.pusch_min_snr_dB; // Initialise UL if (srsran_gnb_ul_init(&gnb_ul, rx_buffer[0], &ul_args) < SRSRAN_SUCCESS) { From 866fe55c42091adaf5ae3fb2012f0901056cc40b Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Sun, 13 Feb 2022 10:43:46 +0100 Subject: [PATCH 02/28] enb,phy: reduce log level to info when RNTI isn't found this can happen during user removal 2022-02-13T00:47:49.195816 [RRC ] [I] Activity timer for rnti=0x50 expired after 80 ms 2022-02-13T00:47:49.195818 [GTPU ] [I] Removing user - rnti=0x50 not found. 2022-02-13T00:47:49.195818 [STCK ] [I] Bearers: No EPS bearer registered for rnti=0x50 2022-02-13T00:47:49.195827 [RRC ] [I] Removed user rnti=0x50 2022-02-13T00:47:49.199697 [PHY ] [E] [ 7234] Error setting grant for rnti=0x0, cc=0 2022-02-13T00:47:49.199698 [PHY0 ] [I] [ 7233] Failed setting UL grants. Some grant's RNTI does not exist. 2022-02-13T00:47:49.209172 [MAC ] [I] [ 7242] User rnti=0x4f removed from MAC/PHY 2022-02-13T00:47:49.223248 [MAC ] [I] [ 7256] User rnti=0x50 removed from MAC/PHY --- srsenb/src/phy/phy_ue_db.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srsenb/src/phy/phy_ue_db.cc b/srsenb/src/phy/phy_ue_db.cc index 9d6761a8f..9090e0984 100644 --- a/srsenb/src/phy/phy_ue_db.cc +++ b/srsenb/src/phy/phy_ue_db.cc @@ -784,7 +784,7 @@ int phy_ue_db::set_ul_grant_available(uint32_t tti, const stack_interface_phy_lt // Check that eNb Cell/Carrier is active for the given RNTI if (_assert_active_enb_cc(rnti, enb_cc_idx) != SRSRAN_SUCCESS) { ret = SRSRAN_ERROR; - srslog::fetch_basic_logger("PHY").error("Error setting grant for rnti=0x%x, cc=%d\n", rnti, enb_cc_idx); + srslog::fetch_basic_logger("PHY").info("Error setting grant for rnti=0x%x, cc=%d", rnti, enb_cc_idx); continue; } // Rise Grant available flag From a2174a57142fb7416856977a021260c4a9a33315 Mon Sep 17 00:00:00 2001 From: Francisco Date: Tue, 15 Feb 2022 10:01:14 +0000 Subject: [PATCH 03/28] Fix type storage copy and move assignment function helper Previously, in the case both the lhs and rhs optionals were present, the copy/move assignments were erroneously destroying the lhs object. --- lib/include/srsran/adt/detail/type_storage.h | 12 ++++-------- lib/test/adt/optional_test.cc | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/lib/include/srsran/adt/detail/type_storage.h b/lib/include/srsran/adt/detail/type_storage.h index 5e7ba2803..d52ee97a5 100644 --- a/lib/include/srsran/adt/detail/type_storage.h +++ b/lib/include/srsran/adt/detail/type_storage.h @@ -70,11 +70,9 @@ void copy_if_present_helper(type_storage& lhs, { if (lhs_present and rhs_present) { lhs.get() = rhs.get(); - } - if (lhs_present) { + } else if (lhs_present) { lhs.destroy(); - } - if (rhs_present) { + } else if (rhs_present) { lhs.copy_ctor(rhs); } } @@ -87,11 +85,9 @@ void move_if_present_helper(type_storage& lhs, { if (lhs_present and rhs_present) { lhs.move_assign(std::move(rhs)); - } - if (lhs_present) { + } else if (lhs_present) { lhs.destroy(); - } - if (rhs_present) { + } else if (rhs_present) { lhs.move_ctor(std::move(rhs)); } } diff --git a/lib/test/adt/optional_test.cc b/lib/test/adt/optional_test.cc index bcfe19063..ea8eb388b 100644 --- a/lib/test/adt/optional_test.cc +++ b/lib/test/adt/optional_test.cc @@ -29,7 +29,26 @@ void test_optional_int() TESTASSERT(opt == opt2); } +struct C { + std::unique_ptr val; + + C(int val = 0) : val(std::make_unique(val)) {} +}; + +void test_optional_move_only() +{ + optional a, b; + a.emplace(C{}); + TESTASSERT(a.has_value()); + TESTASSERT_EQ(0, *a.value().val); + TESTASSERT(not b.has_value()); + b.emplace(C{5}); + a = std::move(b); + TESTASSERT_EQ(5, *a.value().val); +} + int main() { test_optional_int(); + test_optional_move_only(); } \ No newline at end of file From 581a99c616dde3833828bcd67874c8ee3c7b979d Mon Sep 17 00:00:00 2001 From: Francisco Date: Tue, 15 Feb 2022 12:44:14 +0000 Subject: [PATCH 04/28] nr,gnb: use memory pool to allocate scheduler UEs --- .../srsran/adt/pool/circular_stack_pool.h | 16 ++++++++++++++++ srsgnb/hdr/stack/mac/sched_nr.h | 5 ++++- srsgnb/hdr/stack/mac/sched_nr_ue.h | 4 +++- srsgnb/src/stack/mac/sched_nr.cc | 10 ++++++++-- 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/include/srsran/adt/pool/circular_stack_pool.h b/lib/include/srsran/adt/pool/circular_stack_pool.h index 2228e8d29..640ff6b13 100644 --- a/lib/include/srsran/adt/pool/circular_stack_pool.h +++ b/lib/include/srsran/adt/pool/circular_stack_pool.h @@ -107,6 +107,22 @@ private: srslog::basic_logger& logger; }; +template +unique_pool_ptr make_pool_obj_with_fallback(circular_stack_pool& pool, size_t key, Args&&... args) +{ + void* block = pool.allocate(key, sizeof(T), alignof(T)); + if (block == nullptr) { + // allocated with "new" as a fallback + return unique_pool_ptr(new T(std::forward(args)...), std::default_delete()); + } + // allocation using memory pool was successful + new (block) T(std::forward(args)...); + return unique_pool_ptr(static_cast(block), [key, &pool](T* ptr) { + ptr->~T(); + pool.deallocate(key, ptr); + }); +} + } // namespace srsran #endif // SRSRAN_CIRCULAR_MAP_STACK_POOL_H diff --git a/srsgnb/hdr/stack/mac/sched_nr.h b/srsgnb/hdr/stack/mac/sched_nr.h index 2041d09d4..144d9261d 100644 --- a/srsgnb/hdr/stack/mac/sched_nr.h +++ b/srsgnb/hdr/stack/mac/sched_nr.h @@ -17,6 +17,7 @@ #include "sched_nr_interface.h" #include "sched_nr_ue.h" #include "srsran/adt/pool/cached_alloc.h" +#include "srsran/adt/pool/circular_stack_pool.h" #include "srsran/common/slot_point.h" #include extern "C" { @@ -61,7 +62,7 @@ public: private: int ue_cfg_impl(uint16_t rnti, const ue_cfg_t& cfg); - int add_ue_impl(uint16_t rnti, std::unique_ptr u); + int add_ue_impl(uint16_t rnti, sched_nr_impl::unique_ue_ptr u); // args sched_nr_impl::sched_params_t cfg; @@ -74,6 +75,8 @@ private: using slot_cc_worker = sched_nr_impl::cc_worker; std::vector > cc_workers; + // UE Database + std::unique_ptr > ue_pool; using ue_map_t = sched_nr_impl::ue_map_t; ue_map_t ue_db; diff --git a/srsgnb/hdr/stack/mac/sched_nr_ue.h b/srsgnb/hdr/stack/mac/sched_nr_ue.h index b6cd93699..0fd4eb123 100644 --- a/srsgnb/hdr/stack/mac/sched_nr_ue.h +++ b/srsgnb/hdr/stack/mac/sched_nr_ue.h @@ -22,6 +22,7 @@ #include "srsran/adt/circular_map.h" #include "srsran/adt/move_callback.h" #include "srsran/adt/pool/cached_alloc.h" +#include "srsran/adt/pool/pool_interface.h" namespace srsenb { @@ -205,7 +206,8 @@ private: ue_carrier* ue = nullptr; }; -using ue_map_t = rnti_map_t >; +using unique_ue_ptr = srsran::unique_pool_ptr; +using ue_map_t = rnti_map_t; using slot_ue_map_t = rnti_map_t; } // namespace sched_nr_impl diff --git a/srsgnb/src/stack/mac/sched_nr.cc b/srsgnb/src/stack/mac/sched_nr.cc index 9176611a3..0c95a0739 100644 --- a/srsgnb/src/stack/mac/sched_nr.cc +++ b/srsgnb/src/stack/mac/sched_nr.cc @@ -295,6 +295,9 @@ int sched_nr::config(const sched_args_t& sched_cfg, srsran::const_span(8, sizeof(ue), 4)); + // Initiate Common Sched Configuration cfg.cells.reserve(cell_list.size()); for (uint32_t cc = 0; cc < cell_list.size(); ++cc) { @@ -333,7 +336,7 @@ void sched_nr::ue_rem(uint16_t rnti) }); } -int sched_nr::add_ue_impl(uint16_t rnti, std::unique_ptr u) +int sched_nr::add_ue_impl(uint16_t rnti, sched_nr_impl::unique_ue_ptr u) { logger->info("SCHED: New user rnti=0x%x, cc=%d", rnti, cfg.cells[0].cc); return ue_db.insert(rnti, std::move(u)).has_value() ? SRSRAN_SUCCESS : SRSRAN_ERROR; @@ -342,6 +345,8 @@ int sched_nr::add_ue_impl(uint16_t rnti, std::unique_ptr u) int sched_nr::ue_cfg_impl(uint16_t rnti, const ue_cfg_t& uecfg) { if (not ue_db.contains(rnti)) { + // create user object + unique_ue_ptr u = srsran::make_pool_obj_with_fallback(*ue_pool, rnti, rnti, uecfg, cfg); return add_ue_impl(rnti, std::make_unique(rnti, uecfg, cfg)); } ue_db[rnti]->set_cfg(uecfg); @@ -416,7 +421,8 @@ void sched_nr::get_metrics(mac_metrics_t& metrics) int sched_nr::dl_rach_info(const rar_info_t& rar_info) { // create user object outside of sched main thread - std::unique_ptr u = std::make_unique(rar_info.temp_crnti, rar_info.cc, cfg); + unique_ue_ptr u = + srsran::make_pool_obj_with_fallback(*ue_pool, rar_info.temp_crnti, rar_info.temp_crnti, rar_info.cc, cfg); // enqueue UE creation event + RACH handling auto add_ue = [this, rar_info, u = std::move(u)](event_manager::logger& ev_logger) mutable { From b1a33a07a1b4a341b37908e08822f648ac425dd5 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Wed, 19 Jan 2022 12:14:50 +0000 Subject: [PATCH 05/28] lib,rlc_am_nr: starting to add test for segmenting retx. Changed sdu under segmentation to only hold the SN. The actual SDU already exists in the pdu stored in the tx_window. --- lib/include/srsran/rlc/rlc_am_nr.h | 10 +- lib/src/rlc/rlc_am_nr.cc | 129 ++++++++++++++++--------- lib/test/rlc/rlc_am_nr_test.cc | 147 ++++++++++++++++++++++++++++- srsgnb/hdr/stack/rrc/rrc_nr.h | 2 +- 4 files changed, 235 insertions(+), 53 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_nr.h b/lib/include/srsran/rlc/rlc_am_nr.h index a5f052d85..95e08c6f3 100644 --- a/lib/include/srsran/rlc/rlc_am_nr.h +++ b/lib/include/srsran/rlc/rlc_am_nr.h @@ -74,7 +74,7 @@ struct rlc_amd_tx_pdu_nr { const uint32_t rlc_sn = INVALID_RLC_SN; const uint32_t pdcp_sn = INVALID_RLC_SN; rlc_am_nr_pdu_header_t header = {}; - unique_byte_buffer_t buf = nullptr; + unique_byte_buffer_t sdu_buf = nullptr; uint32_t retx_count = 0; struct pdu_segment { uint32_t so = 0; @@ -104,10 +104,8 @@ public: void empty_queue() final; // Data PDU helpers - int build_new_sdu_segment(unique_byte_buffer_t tx_sdu, - rlc_amd_tx_pdu_nr& tx_pdu, - uint8_t* payload, - uint32_t nof_bytes); + int build_new_pdu(uint8_t* payload, uint32_t nof_bytes); + int build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); int build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); int build_retx_pdu(unique_byte_buffer_t& tx_pdu, uint32_t nof_bytes); @@ -149,7 +147,7 @@ private: // Queues and buffers pdu_retx_queue retx_queue; - rlc_amd_tx_sdu_nr_t sdu_under_segmentation; + uint32_t sdu_under_segmentation_sn = INVALID_RLC_SN; // SN of the SDU currently being segmented. // Helper constants uint32_t min_hdr_size = 2; diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index ba1e30328..3ee441e4e 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -67,6 +67,18 @@ bool rlc_am_nr_tx::has_data() tx_sdu_queue.get_n_sdus() != 1; // or if there is a SDU queued up for transmission } +/** + * Builds the RLC PDU. + * + * Called by the MAC, trough the STACK thread. + * + * \param [payload] is a pointer to the buffer that will hold the PDU. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This will be called multiple times from the MAC, + * while there is something to TX and enough space in the TB. + */ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) { std::lock_guard lock(mutex); @@ -106,14 +118,14 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) } // Send remaining segment, if it exists - if (sdu_under_segmentation.rlc_sn != INVALID_RLC_SN) { - if (not tx_window.has_sn(sdu_under_segmentation.rlc_sn)) { - sdu_under_segmentation.rlc_sn = INVALID_RLC_SN; + if (sdu_under_segmentation_sn != INVALID_RLC_SN) { + if (not tx_window.has_sn(sdu_under_segmentation_sn)) { + sdu_under_segmentation_sn = INVALID_RLC_SN; RlcError("SDU currently being segmented does not exist in tx_window. Aborting segmentation SN=%d", - sdu_under_segmentation.rlc_sn); + sdu_under_segmentation_sn); return 0; } - return build_continuation_sdu_segment(tx_window[sdu_under_segmentation.rlc_sn], payload, nof_bytes); + return build_continuation_sdu_segment(tx_window[sdu_under_segmentation_sn], payload, nof_bytes); } // Check whether there is something to TX @@ -122,6 +134,26 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) return 0; } + return build_new_pdu(payload, nof_bytes); +} + +/** + * Builds a new RLC PDU, which contains the full SDU. + * + * Called by the MAC, trough the STACK thread. + * This will be called after checking whether control, retransmission, + * or segment PDUs needed to be transmitted first. + * + * This will read an SDU from the SDU queue, build a new PDU, and add it to the tx_window. + * Segmentation will be done if necessary. + * + * \param [payload] is a pointer to the buffer that will hold the PDU. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + */ +int rlc_am_nr_tx::build_new_pdu(uint8_t* payload, uint32_t nof_bytes) +{ // Read new SDU from TX queue unique_byte_buffer_t tx_sdu; RlcDebug("reading from RLC SDU queue. Queue size %d", tx_sdu_queue.size()); @@ -139,21 +171,22 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) // insert newly assigned SN into window and use reference for in-place operations // NOTE: from now on, we can't return from this function anymore before increasing tx_next rlc_amd_tx_pdu_nr& tx_pdu = tx_window.add_pdu(st.tx_next); - tx_pdu.buf = srsran::make_byte_buffer(); - if (tx_pdu.buf == nullptr) { + tx_pdu.sdu_buf = srsran::make_byte_buffer(); + if (tx_pdu.sdu_buf == nullptr) { RlcError("couldn't allocate PDU in %s().", __FUNCTION__); return 0; } + // Copy SDU into PDU info + memcpy(tx_pdu.sdu_buf->msg, tx_sdu->msg, tx_sdu->N_bytes); + tx_pdu.sdu_buf->N_bytes = tx_sdu->N_bytes; + // Segment new SDU if necessary if (tx_sdu->N_bytes + min_hdr_size > nof_bytes) { RlcInfo("trying to build PDU segment from SDU."); - return build_new_sdu_segment(std::move(tx_sdu), tx_pdu, payload, nof_bytes); + return build_new_sdu_segment(tx_pdu, payload, nof_bytes); } - memcpy(tx_pdu.buf->msg, tx_sdu->msg, tx_sdu->N_bytes); - tx_pdu.buf->N_bytes = tx_sdu->N_bytes; - // Prepare header rlc_am_nr_pdu_header_t hdr = {}; hdr.dc = RLC_DC_FIELD_DATA_PDU; @@ -179,18 +212,27 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) return tx_sdu->N_bytes; } -int rlc_am_nr_tx::build_new_sdu_segment(unique_byte_buffer_t tx_sdu, - rlc_amd_tx_pdu_nr& tx_pdu, - uint8_t* payload, - uint32_t nof_bytes) +/** + * Builds a new RLC PDU segment. + * + * Called by the MAC, trough the STACK thread. + * + * \param [tx_pdu] is a pointer to the buffer that will hold the PDU. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. + */ +int rlc_am_nr_tx::build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes) { - RlcInfo("creating new SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_sdu->N_bytes, nof_bytes); + RlcInfo("creating new SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); // Sanity check: can this SDU be sent this in a single PDU? - if ((tx_sdu->N_bytes + min_hdr_size) < nof_bytes) { + if ((tx_pdu.sdu_buf->N_bytes + min_hdr_size) < nof_bytes) { RlcError("calling build_new_sdu_segment(), but there are enough bytes to tx in a single PDU. Tx SDU (%d B), " "nof_bytes=%d B ", - tx_sdu->N_bytes, + tx_pdu.sdu_buf->N_bytes, nof_bytes); return 0; } @@ -223,11 +265,10 @@ int rlc_am_nr_tx::build_new_sdu_segment(unique_byte_buffer_t tx_sdu, // Copy PDU to payload uint32_t segment_payload_len = nof_bytes - hdr_len; srsran_assert((hdr_len + segment_payload_len) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); - memcpy(&payload[hdr_len], tx_pdu.buf->msg, segment_payload_len); + memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, segment_payload_len); // Save SDU currently being segmented - sdu_under_segmentation.rlc_sn = st.tx_next; - sdu_under_segmentation.buf = std::move(tx_sdu); + sdu_under_segmentation_sn = st.tx_next; // Store Segment Info rlc_amd_tx_pdu_nr::pdu_segment segment_info; @@ -239,19 +280,18 @@ int rlc_am_nr_tx::build_new_sdu_segment(unique_byte_buffer_t tx_sdu, int rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes) { RlcInfo("continuing SDU segment. SN=%d, Tx SDU (%d B), nof_bytes=%d B ", - sdu_under_segmentation.rlc_sn, - sdu_under_segmentation.buf->N_bytes, + sdu_under_segmentation_sn, + tx_pdu.sdu_buf->N_bytes, nof_bytes); // Sanity check: is there an initial SDU segment? if (tx_pdu.segment_list.empty()) { RlcError("build_continuation_sdu_segment was called, but there was no initial segment. SN=%d, Tx SDU (%d B), " "nof_bytes=%d B ", - sdu_under_segmentation.rlc_sn, - sdu_under_segmentation.buf->N_bytes, + sdu_under_segmentation_sn, + tx_pdu.sdu_buf->N_bytes, nof_bytes); - sdu_under_segmentation.rlc_sn = INVALID_RLC_SN; - sdu_under_segmentation.buf = nullptr; + sdu_under_segmentation_sn = INVALID_RLC_SN; return 0; } @@ -267,15 +307,16 @@ int rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint uint32_t last_byte = seg.so + seg.payload_len; RlcDebug("continuing SDU segment. SN=%d, last byte transmitted %d", tx_pdu.rlc_sn, last_byte); - // Sanity check: last byte must be smaller than SDU - if (sdu_under_segmentation.buf->N_bytes < last_byte) { - RlcError("last byte transmitted larger than SDU len. SDU len=%d B, last_byte=%d B", tx_pdu.buf->N_bytes, last_byte); + // Sanity check: last byte must be smaller than SDU size + if (last_byte > tx_pdu.sdu_buf->N_bytes) { + RlcError( + "last byte transmitted larger than SDU len. SDU len=%d B, last_byte=%d B", tx_pdu.sdu_buf->N_bytes, last_byte); return 0; } - uint32_t segment_payload_full_len = sdu_under_segmentation.buf->N_bytes - last_byte + max_hdr_size; // SO is included - uint32_t segment_payload_len = sdu_under_segmentation.buf->N_bytes - last_byte; - rlc_nr_si_field_t si = {}; + uint32_t segment_payload_full_len = tx_pdu.sdu_buf->N_bytes - last_byte + max_hdr_size; // SO is included + uint32_t segment_payload_len = tx_pdu.sdu_buf->N_bytes - last_byte; + rlc_nr_si_field_t si = {}; if (segment_payload_full_len > nof_bytes) { RlcInfo("grant is not large enough for full SDU. " @@ -313,7 +354,7 @@ int rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint // Copy PDU to payload srsran_assert((hdr_len + segment_payload_len) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); - memcpy(&payload[hdr_len], &tx_pdu.buf->msg[last_byte], segment_payload_len); + memcpy(&payload[hdr_len], &tx_pdu.sdu_buf->msg[last_byte], segment_payload_len); // Store PDU segment info into tx_window rlc_amd_tx_pdu_nr::pdu_segment segment_info = {}; @@ -327,8 +368,7 @@ int rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint } else { RlcInfo("grant is large enough for full SDU." "Removing current SDU info"); - sdu_under_segmentation.rlc_sn = INVALID_RLC_SN; - sdu_under_segmentation.buf = nullptr; + sdu_under_segmentation_sn = INVALID_RLC_SN; } return hdr_len + segment_payload_len; @@ -361,22 +401,21 @@ int rlc_am_nr_tx::build_retx_pdu(unique_byte_buffer_t& tx_pdu, uint32_t nof_byte uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(new_header, tx_pdu.get()); // Check if we exceed allocated number of bytes - if (hdr_len + tx_window[retx.sn].buf->N_bytes > nof_bytes) { - RlcWarning("segmentation not supported yet. Cannot provide retx PDU"); + if (hdr_len + tx_window[retx.sn].sdu_buf->N_bytes > nof_bytes) { + RlcWarning("Trying to segment retx PDU"); return SRSRAN_ERROR; } - // TODO Consider re-segmentation - memcpy(&tx_pdu->msg[hdr_len], tx_window[retx.sn].buf->msg, tx_window[retx.sn].buf->N_bytes); - tx_pdu->N_bytes += tx_window[retx.sn].buf->N_bytes; + memcpy(&tx_pdu->msg[hdr_len], tx_window[retx.sn].sdu_buf->msg, tx_window[retx.sn].sdu_buf->N_bytes); + tx_pdu->N_bytes += tx_window[retx.sn].sdu_buf->N_bytes; retx_queue.pop(); - RlcHexInfo(tx_window[retx.sn].buf->msg, - tx_window[retx.sn].buf->N_bytes, + RlcHexInfo(tx_window[retx.sn].sdu_buf->msg, + tx_window[retx.sn].sdu_buf->N_bytes, "Original SDU SN=%d (%d B) (attempt %d/%d)", retx.sn, - tx_window[retx.sn].buf->N_bytes, + tx_window[retx.sn].sdu_buf->N_bytes, tx_window[retx.sn].retx_count + 1, cfg.max_retx_thresh); RlcHexInfo(tx_pdu->msg, tx_pdu->N_bytes, "retx PDU SN=%d (%d B)", retx.sn, tx_pdu->N_bytes); @@ -463,7 +502,7 @@ void rlc_am_nr_tx::handle_control_pdu(uint8_t* payload, uint32_t nof_bytes) retx.sn = nack_sn; retx.is_segment = false; retx.so_start = 0; - retx.so_end = pdu.buf->N_bytes; + retx.so_end = pdu.sdu_buf->N_bytes; } } } diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index 673887b32..1136c8830 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -410,7 +410,151 @@ int basic_segmentation_test() return SRSRAN_SUCCESS; } -int main(int argc, char** argv) +int segment_retx_test() +{ + rlc_am_tester tester; + timer_handler timers(8); + byte_buffer_t pdu_bufs[NBUFS]; + + auto& test_logger = srslog::fetch_basic_logger("TESTER "); + rlc_am rlc1(srsran_rat_t::nr, srslog::fetch_basic_logger("RLC_AM_1"), 1, &tester, &tester, &timers); + rlc_am rlc2(srsran_rat_t::nr, srslog::fetch_basic_logger("RLC_AM_2"), 1, &tester, &tester, &timers); + test_delimit_logger delimiter("segment retx PDU"); + + // before configuring entity + TESTASSERT(0 == rlc1.get_buffer_state()); + + if (not rlc1.configure(rlc_config_t::default_rlc_am_nr_config())) { + return -1; + } + + if (not rlc2.configure(rlc_config_t::default_rlc_am_nr_config())) { + return -1; + } + + // Push 5 SDUs into RLC1 + unique_byte_buffer_t sdu_bufs[NBUFS]; + for (int i = 0; i < NBUFS; i++) { + sdu_bufs[i] = srsran::make_byte_buffer(); + sdu_bufs[i]->msg[0] = i; // Write the index into the buffer + sdu_bufs[i]->N_bytes = 3; // Give each buffer a size of 3 bytes + sdu_bufs[i]->md.pdcp_sn = i; // PDCP SN for notifications + rlc1.write_sdu(std::move(sdu_bufs[i])); + } + + TESTASSERT(25 == rlc1.get_buffer_state()); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) = 25 + + // Read 5 PDUs from RLC1 (1 byte each) + for (int i = 0; i < NBUFS; i++) { + uint32_t len = rlc1.read_pdu(pdu_bufs[i].msg, 5); // 2 bytes for header + 3 byte payload + pdu_bufs[i].N_bytes = len; + TESTASSERT_EQ(5, len); + } + + TESTASSERT(0 == rlc1.get_buffer_state()); + + // Write 5 PDUs into RLC2 + for (int i = 0; i < NBUFS; i++) { + if (i != 3) { + rlc2.write_pdu(pdu_bufs[i].msg, pdu_bufs[i].N_bytes); // Don't write RLC_SN=3. + } + } + + // Only after t-reassembly has expired, will the status report include NACKs. + TESTASSERT(3 == rlc2.get_buffer_state()); + { + // Read status PDU from RLC2 + byte_buffer_t status_buf; + int len = rlc2.read_pdu(status_buf.msg, 5); + status_buf.N_bytes = len; + + TESTASSERT(0 == rlc2.get_buffer_state()); + + // Assert status is correct + rlc_am_nr_status_pdu_t status_check = {}; + rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); + TESTASSERT(status_check.ack_sn == 3); // 3 is the next expected SN (i.e. the lost packet.) + + // Write status PDU to RLC1 + rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); + } + + // Step timers until reassambly timeout expires + for (int cnt = 0; cnt < 35; cnt++) { + timers.step_all(); + } + + // t-reassembly has expired. There should be a NACK in the status report. + TESTASSERT(5 == rlc2.get_buffer_state()); + { + // Read status PDU from RLC2 + byte_buffer_t status_buf; + int len = rlc2.read_pdu(status_buf.msg, 5); + status_buf.N_bytes = len; + + TESTASSERT(0 == rlc2.get_buffer_state()); + + // Assert status is correct + rlc_am_nr_status_pdu_t status_check = {}; + rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); + TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. + TESTASSERT(status_check.N_nack == 1); // We lost one PDU. + TESTASSERT(status_check.nacks[0].nack_sn == 3); // Lost PDU SN=3. + + // Write status PDU to RLC1 + rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); + + // Check there is an Retx of SN=3 + TESTASSERT(5 == rlc1.get_buffer_state()); + } + + { + // Re-transmit PDU in 3 segments + for (int i = 0; i < 3; i++) { + byte_buffer_t retx_buf; + int len = rlc1.read_pdu(retx_buf.msg, 3); + retx_buf.N_bytes = len; + TESTASSERT(3 == len); + + rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); + } + TESTASSERT(0 == rlc1.get_buffer_state()); + } + + // Check statistics + rlc_bearer_metrics_t metrics1 = rlc1.get_metrics(); + rlc_bearer_metrics_t metrics2 = rlc2.get_metrics(); + + // SDU metrics + TESTASSERT_EQ(5, metrics1.num_tx_sdus); + TESTASSERT_EQ(0, metrics1.num_rx_sdus); + TESTASSERT_EQ(15, metrics1.num_tx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_lost_sdus); + // PDU metrics + TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions + TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU + TESTASSERT_EQ(18, + metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + 3 rext (3 * 3) = 34 + TESTASSERT_EQ(3 + 5, metrics1.num_rx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs + + // PDU metrics + TESTASSERT_EQ(0, metrics2.num_tx_sdus); + TESTASSERT_EQ(5, metrics2.num_rx_sdus); + TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); + TESTASSERT_EQ(5, metrics2.num_rx_sdu_bytes); + TESTASSERT_EQ(0, metrics2.num_lost_sdus); + // SDU metrics + TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs + TESTASSERT_EQ(5, metrics2.num_rx_pdus); // 5 PDUs (6 tx'ed, but one was lost) + TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(15, metrics2.num_rx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS (data) = 15 + TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs + return SRSRAN_SUCCESS; +} + +int main() { // Setup the log message spy to intercept error and warning log entries from RLC if (!srslog::install_custom_sink(srsran::log_sink_message_spy::name(), @@ -439,6 +583,7 @@ int main(int argc, char** argv) TESTASSERT(basic_test() == SRSRAN_SUCCESS); TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS); TESTASSERT(basic_segmentation_test() == SRSRAN_SUCCESS); + TESTASSERT(segment_retx_test() == SRSRAN_SUCCESS); return SRSRAN_SUCCESS; } diff --git a/srsgnb/hdr/stack/rrc/rrc_nr.h b/srsgnb/hdr/stack/rrc/rrc_nr.h index 85508d457..a82aafdc7 100644 --- a/srsgnb/hdr/stack/rrc/rrc_nr.h +++ b/srsgnb/hdr/stack/rrc/rrc_nr.h @@ -71,7 +71,7 @@ public: int read_pdu_bcch_bch(const uint32_t tti, srsran::byte_buffer_t& buffer) final; int read_pdu_bcch_dlsch(uint32_t sib_index, srsran::byte_buffer_t& buffer) final; - /// User manegement + /// User management int add_user(uint16_t rnti, uint32_t pcell_cc_idx) final; void rem_user(uint16_t rnti); int update_user(uint16_t new_rnti, uint16_t old_rnti) final; From e4a72de3429254a5addbf553da14b492e04cc7b9 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Wed, 19 Jan 2022 17:11:56 +0000 Subject: [PATCH 06/28] lib,rlc_am_nr: refactored build_pdu helpers to receive the payload pointer for consistency. Added function to segment retx. Added some comments to build PDU helper functions. --- lib/include/srsran/rlc/rlc_am_nr.h | 15 ++- lib/src/rlc/rlc_am_nr.cc | 148 ++++++++++++++++++++++++----- lib/test/rlc/rlc_am_nr_test.cc | 4 + 3 files changed, 136 insertions(+), 31 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_nr.h b/lib/include/srsran/rlc/rlc_am_nr.h index 95e08c6f3..0df3870f1 100644 --- a/lib/include/srsran/rlc/rlc_am_nr.h +++ b/lib/include/srsran/rlc/rlc_am_nr.h @@ -104,10 +104,11 @@ public: void empty_queue() final; // Data PDU helpers - int build_new_pdu(uint8_t* payload, uint32_t nof_bytes); - int build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); - int build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); - int build_retx_pdu(unique_byte_buffer_t& tx_pdu, uint32_t nof_bytes); + uint32_t build_new_pdu(uint8_t* payload, uint32_t nof_bytes); + uint32_t build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); + uint32_t build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); + uint32_t build_retx_pdu(uint8_t* payload, uint32_t nof_bytes); + uint32_t build_retx_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); // Buffer State bool has_data() final; @@ -147,7 +148,8 @@ private: // Queues and buffers pdu_retx_queue retx_queue; - uint32_t sdu_under_segmentation_sn = INVALID_RLC_SN; // SN of the SDU currently being segmented. + uint32_t sdu_under_segmentation_sn = INVALID_RLC_SN; // SN of the SDU currently being segmented. + uint32_t retx_under_segmentation_sn = INVALID_RLC_SN; // SN of RETX currently being segmented. // Helper constants uint32_t min_hdr_size = 2; @@ -159,6 +161,9 @@ public: void set_tx_state(const rlc_am_nr_tx_state_t& st_) { st = st_; } // This should only be used for testing. rlc_am_nr_tx_state_t get_tx_state() { return st; } // This should only be used for testing. uint32_t get_tx_window_size() { return tx_window.size(); } // This should only be used for testing. + + // Debug Helper + void debug_state(); }; /**************************************************************************** diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 3ee441e4e..14b3b406d 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -105,16 +105,7 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) // Retransmit if required if (not retx_queue.empty()) { RlcInfo("re-transmission required. Retransmission queue size: %d", retx_queue.size()); - unique_byte_buffer_t tx_pdu = srsran::make_byte_buffer(); - if (tx_pdu == nullptr) { - RlcError("couldn't allocate PDU in %s().", __FUNCTION__); - return 0; - } - int retx_err = build_retx_pdu(tx_pdu, nof_bytes); - if (retx_err >= 0 && tx_pdu->N_bytes <= nof_bytes) { - memcpy(payload, tx_pdu->msg, tx_pdu->N_bytes); - return tx_pdu->N_bytes; - } + return build_retx_pdu(payload, nof_bytes); } // Send remaining segment, if it exists @@ -152,7 +143,7 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) * * \returns the number of bytes written to the payload buffer. */ -int rlc_am_nr_tx::build_new_pdu(uint8_t* payload, uint32_t nof_bytes) +uint32_t rlc_am_nr_tx::build_new_pdu(uint8_t* payload, uint32_t nof_bytes) { // Read new SDU from TX queue unique_byte_buffer_t tx_sdu; @@ -217,14 +208,14 @@ int rlc_am_nr_tx::build_new_pdu(uint8_t* payload, uint32_t nof_bytes) * * Called by the MAC, trough the STACK thread. * - * \param [tx_pdu] is a pointer to the buffer that will hold the PDU. + * \param [tx_pdu] is the tx_pdu info contained in the tx_window. * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. * * \returns the number of bytes written to the payload buffer. * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. */ -int rlc_am_nr_tx::build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes) +uint32_t rlc_am_nr_tx::build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes) { RlcInfo("creating new SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); @@ -277,7 +268,19 @@ int rlc_am_nr_tx::build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payl return hdr_len + segment_payload_len; } -int rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes) +/** + * Build PDU segment for an RLC SDU that is already on-going segmentation. + * + * Called by the MAC, trough the STACK thread. + * + * \param [tx_pdu] is the tx_pdu info contained in the tx_window. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. + */ +uint32_t rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes) { RlcInfo("continuing SDU segment. SN=%d, Tx SDU (%d B), nof_bytes=%d B ", sdu_under_segmentation_sn, @@ -374,12 +377,25 @@ int rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint return hdr_len + segment_payload_len; } -int rlc_am_nr_tx::build_retx_pdu(unique_byte_buffer_t& tx_pdu, uint32_t nof_bytes) +/** + * Builds a retx RLC PDU. + * + * Called by the MAC, trough the STACK thread. + * This will segment the RETX PDU if necessary + * + * \param [tx_pdu] is the tx_pdu info contained in the tx_window. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. + */ +uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) { // Check there is at least 1 element before calling front() if (retx_queue.empty()) { RlcError("in build_retx_pdu(): retx_queue is empty"); - return SRSRAN_ERROR; + return 0; } rlc_amd_retx_t retx = retx_queue.front(); @@ -392,22 +408,30 @@ int rlc_am_nr_tx::build_retx_pdu(unique_byte_buffer_t& tx_pdu, uint32_t nof_byte retx = retx_queue.front(); } else { RlcWarning("empty retx queue, cannot provide retx PDU"); - return SRSRAN_ERROR; + return 0; } } + // Get tx_pdu info from tx_window + rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; // Update & write header - rlc_am_nr_pdu_header_t new_header = tx_window[retx.sn].header; + rlc_am_nr_pdu_header_t new_header = tx_pdu.header; new_header.p = 0; - uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(new_header, tx_pdu.get()); + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(new_header, payload); // Check if we exceed allocated number of bytes if (hdr_len + tx_window[retx.sn].sdu_buf->N_bytes > nof_bytes) { + RlcInfo("Trying to segment retx PDU. SN=%d", retx.sn); + log_rlc_am_nr_pdu_header_to_string(logger.debug, new_header); + if (tx_pdu.header.si == rlc_nr_si_field_t::full_sdu) { + return build_new_sdu_segment(tx_pdu, payload, nof_bytes); + } RlcWarning("Trying to segment retx PDU"); - return SRSRAN_ERROR; + return 0; } - memcpy(&tx_pdu->msg[hdr_len], tx_window[retx.sn].sdu_buf->msg, tx_window[retx.sn].sdu_buf->N_bytes); - tx_pdu->N_bytes += tx_window[retx.sn].sdu_buf->N_bytes; + // RETX full PDU + memcpy(&payload[hdr_len], tx_window[retx.sn].sdu_buf->msg, tx_window[retx.sn].sdu_buf->N_bytes); + uint32_t pdu_bytes = hdr_len + tx_window[retx.sn].sdu_buf->N_bytes; retx_queue.pop(); @@ -418,11 +442,70 @@ int rlc_am_nr_tx::build_retx_pdu(unique_byte_buffer_t& tx_pdu, uint32_t nof_byte tx_window[retx.sn].sdu_buf->N_bytes, tx_window[retx.sn].retx_count + 1, cfg.max_retx_thresh); - RlcHexInfo(tx_pdu->msg, tx_pdu->N_bytes, "retx PDU SN=%d (%d B)", retx.sn, tx_pdu->N_bytes); + RlcHexInfo(payload, pdu_bytes, "retx PDU SN=%d (%d B)", retx.sn, pdu_bytes); log_rlc_am_nr_pdu_header_to_string(logger.debug, new_header); - // debug_state(); - return SRSRAN_SUCCESS; + debug_state(); + return pdu_bytes; +} + +/** + * Builds RLC PDU segment from an RETX. + * + * Called by the MAC, trough the STACK thread. + * + * \param [tx_pdu] is the tx_pdu info contained in the tx_window. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. + */ +uint32_t rlc_am_nr_tx::build_retx_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes) +{ + RlcInfo("creating SDU segment from retx. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); + + // Sanity check: can this SDU be sent this in a single PDU? + if ((tx_pdu.sdu_buf->N_bytes + min_hdr_size) < nof_bytes) { + RlcError("calling build_new_sdu_segment(), but there are enough bytes to tx in a single PDU. Tx SDU (%d B), " + "nof_bytes=%d B ", + tx_pdu.sdu_buf->N_bytes, + nof_bytes); + return 0; + } + + // Sanity check: can this SDU be sent considering header overhead? + if (nof_bytes <= min_hdr_size) { // Small header as SO is not present + RlcError("cannot build new sdu_segment, there are not enough bytes allocated to tx header plus data. nof_bytes=%d", + nof_bytes); + return 0; + } + + // Prepare header + rlc_am_nr_pdu_header_t hdr = tx_pdu.header; + hdr.si = rlc_nr_si_field_t::first_segment; + log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); + + // Write header + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); + if (hdr_len >= nof_bytes || hdr_len != min_hdr_size) { + RlcError("error writing AMD PDU header"); + return 0; + } + + // Copy PDU to payload + uint32_t segment_payload_len = nof_bytes - hdr_len; + srsran_assert((hdr_len + segment_payload_len) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); + memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, segment_payload_len); + + // Save SDU currently being segmented + retx_under_segmentation_sn = hdr.sn; + + // Store Segment Info + rlc_amd_tx_pdu_nr::pdu_segment segment_info; + segment_info.payload_len = segment_payload_len; + tx_pdu.segment_list.push_back(segment_info); + return hdr_len + segment_payload_len; } uint32_t rlc_am_nr_tx::build_status_pdu(byte_buffer_t* payload, uint32_t nof_bytes) @@ -492,8 +575,8 @@ void rlc_am_nr_tx::handle_control_pdu(uint8_t* payload, uint32_t nof_bytes) if (not retx_queue.has_sn(nack_sn)) { // increment Retx counter and inform upper layers if needed pdu.retx_count++; - // check_sn_reached_max_retx(nack_sn); + // check_sn_reached_max_retx(nack_sn); rlc_amd_retx_t& retx = retx_queue.push(); srsran_expect(tx_window[nack_sn].rlc_sn == nack_sn, "Incorrect RLC SN=%d!=%d being accessed", @@ -605,6 +688,7 @@ bool rlc_am_nr_tx::sdu_queue_is_full() void rlc_am_nr_tx::empty_queue() {} void rlc_am_nr_tx::stop() {} + /* * Window helpers */ @@ -619,6 +703,18 @@ bool rlc_am_nr_tx::inside_tx_window(uint32_t sn) return tx_mod_base_nr(sn) < RLC_AM_NR_WINDOW_SIZE; } +/* + * Debug Helpers + */ +void rlc_am_nr_tx::debug_state() +{ + RlcDebug("TX entity state: Tx_Next %d, Rx_Next_Ack %d, POLL_SN %d, PDU_WITHOUT_POLL %d, BYTE_WITHOUT_POLL %d", + st.tx_next, + st.tx_next_ack, + st.poll_sn, + st.pdu_without_poll, + st.byte_without_poll); +} /**************************************************************************** * Rx subclass implementation ***************************************************************************/ diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index 1136c8830..abbc7d18c 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -516,6 +516,10 @@ int segment_retx_test() retx_buf.N_bytes = len; TESTASSERT(3 == len); + rlc_am_nr_pdu_header_t header_check = {}; + uint32_t hdr_len = rlc_am_nr_read_data_pdu_header(&retx_buf, rlc_am_nr_sn_size_t::size12bits, &header_check); + TESTASSERT(header_check.sn == 3); // Double check RETX SN + rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); } TESTASSERT(0 == rlc1.get_buffer_state()); From 65d5df8b6e947d631d46d759052c2f23501911d0 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Thu, 20 Jan 2022 18:46:16 +0000 Subject: [PATCH 07/28] lib,rlc_am_nr: Continue to add functionality to provide segments of retx'es. Started to add function to re-segment already existing SDU segment --- lib/include/srsran/rlc/rlc_am_nr.h | 10 +- lib/src/rlc/rlc_am_nr.cc | 226 ++++++++++++++++++++++++----- lib/test/rlc/rlc_am_nr_test.cc | 20 ++- 3 files changed, 215 insertions(+), 41 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_nr.h b/lib/include/srsran/rlc/rlc_am_nr.h index 0df3870f1..be230ed08 100644 --- a/lib/include/srsran/rlc/rlc_am_nr.h +++ b/lib/include/srsran/rlc/rlc_am_nr.h @@ -108,8 +108,14 @@ public: uint32_t build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); uint32_t build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); uint32_t build_retx_pdu(uint8_t* payload, uint32_t nof_bytes); - uint32_t build_retx_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); - + uint32_t build_retx_pdu_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); + uint32_t build_retx_segment_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); + uint32_t build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); + uint32_t build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); + /* + * uint32_t build_retx_sdu_segment(rlc_amd_retx_t& retx, rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t + * nof_bytes); + */ // Buffer State bool has_data() final; uint32_t get_buffer_state() final; diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 14b3b406d..16a3d4368 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -380,8 +380,9 @@ uint32_t rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, /** * Builds a retx RLC PDU. * - * Called by the MAC, trough the STACK thread. - * This will segment the RETX PDU if necessary + * This will use the retx_queue to get information about the RLC PDU + * being retx'ed. The retx may have been previously transmitted as + * a full SDU or an SDU segment. * * \param [tx_pdu] is the tx_pdu info contained in the tx_window. * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. @@ -398,7 +399,7 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) return 0; } - rlc_amd_retx_t retx = retx_queue.front(); + rlc_amd_retx_t& retx = retx_queue.front(); // Sanity check - drop any retx SNs not present in tx_window while (not tx_window.has_sn(retx.sn)) { @@ -411,6 +412,29 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) return 0; } } + + if (retx.is_segment) { + return build_retx_pdu_from_sdu_segment(retx, payload, nof_bytes); + } + return build_retx_pdu_from_full_sdu(retx, payload, nof_bytes); +} + +/** + * Builds a retx RLC PDU from a full SDU. + * + * The RETX of the full SDU may be further segmented if necessary. + * + * \param [tx_pdu] is the tx_pdu info contained in the tx_window. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. + */ +uint32_t rlc_am_nr_tx::build_retx_pdu_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) +{ + srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); + // Get tx_pdu info from tx_window rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; // Update & write header @@ -422,11 +446,7 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) if (hdr_len + tx_window[retx.sn].sdu_buf->N_bytes > nof_bytes) { RlcInfo("Trying to segment retx PDU. SN=%d", retx.sn); log_rlc_am_nr_pdu_header_to_string(logger.debug, new_header); - if (tx_pdu.header.si == rlc_nr_si_field_t::full_sdu) { - return build_new_sdu_segment(tx_pdu, payload, nof_bytes); - } - RlcWarning("Trying to segment retx PDU"); - return 0; + return build_retx_segment_from_full_sdu(retx, payload, nof_bytes); } // RETX full PDU @@ -434,7 +454,6 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) uint32_t pdu_bytes = hdr_len + tx_window[retx.sn].sdu_buf->N_bytes; retx_queue.pop(); - RlcHexInfo(tx_window[retx.sn].sdu_buf->msg, tx_window[retx.sn].sdu_buf->N_bytes, "Original SDU SN=%d (%d B) (attempt %d/%d)", @@ -450,9 +469,12 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) } /** - * Builds RLC PDU segment from an RETX. + * Builds a retx RLC PDU from a full SDU. * - * Called by the MAC, trough the STACK thread. + * This function will further segment the previously transmitted SDU. + * There should not be enough bytes granted to transmit the previous + * SDU full; this should have been checked previous to calling this + * function. * * \param [tx_pdu] is the tx_pdu info contained in the tx_window. * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. @@ -461,51 +483,183 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) * \returns the number of bytes written to the payload buffer. * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. */ -uint32_t rlc_am_nr_tx::build_retx_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes) +uint32_t rlc_am_nr_tx::build_retx_segment_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) { - RlcInfo("creating SDU segment from retx. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); + // Get tx_pdu info from tx_window + srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); + rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; - // Sanity check: can this SDU be sent this in a single PDU? - if ((tx_pdu.sdu_buf->N_bytes + min_hdr_size) < nof_bytes) { - RlcError("calling build_new_sdu_segment(), but there are enough bytes to tx in a single PDU. Tx SDU (%d B), " - "nof_bytes=%d B ", - tx_pdu.sdu_buf->N_bytes, - nof_bytes); + RlcInfo("creating SDU segment from full SDU. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); + + // Sanity check: is this an SDU segment retx? + if (retx.is_segment) { + RlcError("called %s, but retx contains a SDU segment. SN=%d, so_start=%d, so_end=%d", + __FUNCTION__, + retx.sn, + retx.so_start, + retx.so_end); return 0; } - // Sanity check: can this SDU be sent considering header overhead? - if (nof_bytes <= min_hdr_size) { // Small header as SO is not present - RlcError("cannot build new sdu_segment, there are not enough bytes allocated to tx header plus data. nof_bytes=%d", - nof_bytes); + // Sanity check: are there enough bytes for header plus data? + if (nof_bytes <= min_hdr_size) { + RlcError("called %s, but there are not enough bytes for data plus header. SN=%d", __FUNCTION__, retx.sn); return 0; } - // Prepare header - rlc_am_nr_pdu_header_t hdr = tx_pdu.header; - hdr.si = rlc_nr_si_field_t::first_segment; - log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); + // Sanity check: could this have been transmitted without segmentation? + if (nof_bytes > (tx_pdu.sdu_buf->N_bytes + min_hdr_size)) { + RlcError("called %s, but there are enough bytes to avoid segmentation. SN=%d", __FUNCTION__, retx.sn); + return 0; + } + + // Can the RETX PDU be transmitted in a single PDU? + uint32_t retx_pdu_payload_size = nof_bytes - min_hdr_size; // Write header - uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); + rlc_am_nr_pdu_header_t hdr = tx_pdu.header; + hdr.si = rlc_nr_si_field_t::first_segment; + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); if (hdr_len >= nof_bytes || hdr_len != min_hdr_size) { RlcError("error writing AMD PDU header"); return 0; } + log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); // Copy PDU to payload - uint32_t segment_payload_len = nof_bytes - hdr_len; - srsran_assert((hdr_len + segment_payload_len) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); - memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, segment_payload_len); - - // Save SDU currently being segmented - retx_under_segmentation_sn = hdr.sn; + srsran_assert((hdr_len + retx_pdu_payload_size) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); + memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); // Store Segment Info rlc_amd_tx_pdu_nr::pdu_segment segment_info; - segment_info.payload_len = segment_payload_len; + segment_info.payload_len = retx_pdu_payload_size; tx_pdu.segment_list.push_back(segment_info); - return hdr_len + segment_payload_len; + + // Update retx queue. Next SDU segment will be the rest of the PDU + retx.is_segment = true; + retx.so_start = retx_pdu_payload_size; + retx.so_end = tx_pdu.sdu_buf->N_bytes; + + return hdr_len + retx_pdu_payload_size; +} + +/** + * Builds a retx RLC PDU from an RETX SDU segment. + * + * The + * \param [tx_pdu] is the tx_pdu info contained in the tx_window. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. + */ +uint32_t rlc_am_nr_tx::build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) +{ + // Get tx_pdu info from tx_window + srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); + rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; + + RlcInfo("creating RETX PDU from SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); + + // Sanity check: is this an SDU segment retx? + if (not retx.is_segment) { + RlcError("called %s, but retx contains a SDU segment. SN=%d, so_start=%d, so_end=%d", + __FUNCTION__, + retx.sn, + retx.so_start, + retx.so_end); + return 0; + } + + uint32_t expected_hdr_len = min_hdr_size; + if (retx.so_start != 0) { + expected_hdr_len = max_hdr_size; + } + + // Sanity check: are there enough bytes for header plus data? + if (nof_bytes <= expected_hdr_len) { + RlcError("called %s, but there are not enough bytes for data plus header. SN=%d", __FUNCTION__, retx.sn); + return 0; + } + + // Can this have be transmitted without segmentation? + if (nof_bytes < (retx.so_end - retx.so_start) + expected_hdr_len) { + RlcInfo("called %s, but there are enough bytes to avoid segmentation. SN=%d", __FUNCTION__, retx.sn); + return build_retx_segment_from_sdu_segment(retx, payload, nof_bytes); + } + + // Can the RETX PDU be transmitted in a single PDU? + uint32_t retx_pdu_payload_size = (retx.so_end - retx.so_start); + + // Write header + rlc_am_nr_pdu_header_t hdr = tx_pdu.header; + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); + if (hdr_len >= nof_bytes || hdr_len != expected_hdr_len) { + RlcError("error writing AMD PDU header"); + return 0; + } + log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); + + // Copy SDU segment into payload + srsran_assert((hdr_len + retx_pdu_payload_size) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); + memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); + + // Update retx queue + retx.so_start = retx.so_end; + retx.so_end += retx_pdu_payload_size; + + if (retx.so_end == tx_pdu.sdu_buf->N_bytes) { + // Last segment to retx, remove retx from queue + retx_queue.pop(); + } + + // Update SDU segment info + // TODO + return hdr_len + retx_pdu_payload_size; +} + +/** + * Builds a retx RLC PDU from an SDU SDU segment. + * + * \param [tx_pdu] is the tx_pdu info contained in the tx_window. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. + */ +uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) +{ + // Get tx_pdu info from tx_window + srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); + rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; + + RlcInfo("creating SDU segment from full SDU. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); + + // Sanity check: is this an SDU segment retx? + if (not retx.is_segment) { + RlcError("called %s, but retx contains a SDU segment. SN=%d, so_start=%d, so_end=%d", + __FUNCTION__, + retx.sn, + retx.so_start, + retx.so_end); + return 0; + } + + // Sanity check: are there enough bytes for header plus data? + if (nof_bytes <= min_hdr_size) { + RlcError("called %s, but there are not enough bytes for data plus header. SN=%d", __FUNCTION__, retx.sn); + return 0; + } + + // Sanity check: could this have been transmitted without segmentation? + if (nof_bytes > (tx_pdu.sdu_buf->N_bytes + min_hdr_size)) { + RlcError("called %s, but there are enough bytes to avoid segmentation. SN=%d", __FUNCTION__, retx.sn); + return 0; + } + + return 0; } uint32_t rlc_am_nr_tx::build_status_pdu(byte_buffer_t* payload, uint32_t nof_bytes) diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index abbc7d18c..8f8edff10 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -512,13 +512,27 @@ int segment_retx_test() // Re-transmit PDU in 3 segments for (int i = 0; i < 3; i++) { byte_buffer_t retx_buf; - int len = rlc1.read_pdu(retx_buf.msg, 3); - retx_buf.N_bytes = len; - TESTASSERT(3 == len); + uint32_t len = 0; + if (i == 0) { + len = rlc1.read_pdu(retx_buf.msg, 3); + TESTASSERT(3 == len); + } else { + len = rlc1.read_pdu(retx_buf.msg, 5); + TESTASSERT(5 == len); + } + retx_buf.N_bytes = len; rlc_am_nr_pdu_header_t header_check = {}; uint32_t hdr_len = rlc_am_nr_read_data_pdu_header(&retx_buf, rlc_am_nr_sn_size_t::size12bits, &header_check); + // Double check header. TESTASSERT(header_check.sn == 3); // Double check RETX SN + if (i == 0) { + TESTASSERT(header_check.si == rlc_nr_si_field_t::first_segment); + } else if (i == 1) { + TESTASSERT(header_check.si == rlc_nr_si_field_t::neither_first_nor_last_segment); + } else { + TESTASSERT(header_check.si == rlc_nr_si_field_t::last_segment); + } rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); } From 5fa72b2b3d5ed0fa791a7a33877fe1e53c2aeb9f Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Mon, 24 Jan 2022 18:21:15 +0000 Subject: [PATCH 08/28] lib,rlc_am_nr: fix creation of retx segement from existing segment. --- lib/src/rlc/rlc_am_nr.cc | 66 ++++++++++++++++++++++++++++++---- lib/test/rlc/rlc_am_nr_test.cc | 4 +++ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 16a3d4368..a782c5d44 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -579,14 +579,32 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uin // Sanity check: are there enough bytes for header plus data? if (nof_bytes <= expected_hdr_len) { - RlcError("called %s, but there are not enough bytes for data plus header. SN=%d", __FUNCTION__, retx.sn); + RlcError("called %s, but there are not enough bytes for data plus header. SN=%d, nof_bytes=%d, expected_hdr_len=%d", + __FUNCTION__, + retx.sn, + nof_bytes, + expected_hdr_len); return 0; } - // Can this have be transmitted without segmentation? + // Can be transmitted without segmentation? if (nof_bytes < (retx.so_end - retx.so_start) + expected_hdr_len) { - RlcInfo("called %s, but there are enough bytes to avoid segmentation. SN=%d", __FUNCTION__, retx.sn); + RlcDebug("It is necessary to further segment the SDU segment. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " + "so_start=%d, so_end=%d", + retx.sn, + nof_bytes, + expected_hdr_len, + retx.so_start, + retx.so_end); return build_retx_segment_from_sdu_segment(retx, payload, nof_bytes); + } else { + RlcDebug("SDU segment can be fully transmitted. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " + "so_start=%d, so_end=%d", + retx.sn, + nof_bytes, + expected_hdr_len, + retx.so_start, + retx.so_end); } // Can the RETX PDU be transmitted in a single PDU? @@ -609,7 +627,7 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uin retx.so_start = retx.so_end; retx.so_end += retx_pdu_payload_size; - if (retx.so_end == tx_pdu.sdu_buf->N_bytes) { + if (retx.so_end >= tx_pdu.sdu_buf->N_bytes) { // Last segment to retx, remove retx from queue retx_queue.pop(); } @@ -635,7 +653,7 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; - RlcInfo("creating SDU segment from full SDU. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); + RlcInfo("creating SDU segment from SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); // Sanity check: is this an SDU segment retx? if (not retx.is_segment) { @@ -647,18 +665,52 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, return 0; } + uint32_t expected_hdr_len = min_hdr_size; + rlc_nr_si_field_t si = rlc_nr_si_field_t::first_segment; + if (retx.so_start != 0) { + si = rlc_nr_si_field_t::neither_first_nor_last_segment; + expected_hdr_len = max_hdr_size; + } + // Sanity check: are there enough bytes for header plus data? - if (nof_bytes <= min_hdr_size) { + if (nof_bytes <= expected_hdr_len) { RlcError("called %s, but there are not enough bytes for data plus header. SN=%d", __FUNCTION__, retx.sn); return 0; } // Sanity check: could this have been transmitted without segmentation? - if (nof_bytes > (tx_pdu.sdu_buf->N_bytes + min_hdr_size)) { + if (nof_bytes > (tx_pdu.sdu_buf->N_bytes + expected_hdr_len)) { RlcError("called %s, but there are enough bytes to avoid segmentation. SN=%d", __FUNCTION__, retx.sn); return 0; } + // Can the RETX PDU be transmitted in a single PDU? + uint32_t retx_pdu_payload_size = nof_bytes - expected_hdr_len; + + // Write header + rlc_am_nr_pdu_header_t hdr = tx_pdu.header; + hdr.so = retx.so_start; + hdr.si = si; + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); + if (hdr_len >= nof_bytes || hdr_len != expected_hdr_len) { + log_rlc_am_nr_pdu_header_to_string(logger.error, hdr); + RlcError("Error writing AMD PDU header. nof_bytes=%d, hdr_len=%d", nof_bytes, hdr_len); + return 0; + } + log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); + + // Copy SDU segment into payload + srsran_assert((hdr_len + retx_pdu_payload_size) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); + memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); + + // Update retx queue + retx.so_start = retx.so_end; + retx.so_end += retx_pdu_payload_size; + + // Update SDU segment info + // TODO + return hdr_len + retx_pdu_payload_size; + return 0; } diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index 8f8edff10..c0bdc11cc 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -513,6 +513,10 @@ int segment_retx_test() for (int i = 0; i < 3; i++) { byte_buffer_t retx_buf; uint32_t len = 0; + test_logger.info("Looping!!! i=%d", i); + test_logger.info("Looping!!! i=%d", i); + test_logger.info("Looping!!! i=%d", i); + test_logger.info("Looping!!! i=%d", i); if (i == 0) { len = rlc1.read_pdu(retx_buf.msg, 3); TESTASSERT(3 == len); From 38e4c47f34bc2b4f294dfe63d5f70222cc2b739c Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Wed, 26 Jan 2022 14:00:39 +0000 Subject: [PATCH 09/28] lib,rlc_am_nr: added current SO to keep track of the SO for the next RETX. --- lib/include/srsran/rlc/rlc_am_data_structs.h | 1 + lib/src/rlc/rlc_am_nr.cc | 125 ++++++++++++++----- 2 files changed, 93 insertions(+), 33 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_data_structs.h b/lib/include/srsran/rlc/rlc_am_data_structs.h index 35107383d..a7fa667d0 100644 --- a/lib/include/srsran/rlc/rlc_am_data_structs.h +++ b/lib/include/srsran/rlc/rlc_am_data_structs.h @@ -305,6 +305,7 @@ struct rlc_amd_retx_t { bool is_segment; uint32_t so_start; uint32_t so_end; + uint32_t current_so; }; template diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index a782c5d44..f5aec7808 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -93,18 +93,18 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) if (do_status()) { unique_byte_buffer_t tx_pdu = srsran::make_byte_buffer(); if (tx_pdu == nullptr) { - RlcError("couldn't allocate PDU in %s().", __FUNCTION__); + RlcError("Couldn't allocate PDU in %s().", __FUNCTION__); return 0; } build_status_pdu(tx_pdu.get(), nof_bytes); memcpy(payload, tx_pdu->msg, tx_pdu->N_bytes); - RlcDebug("status PDU built - %d bytes", tx_pdu->N_bytes); + RlcDebug("Status PDU built - %d bytes", tx_pdu->N_bytes); return tx_pdu->N_bytes; } // Retransmit if required if (not retx_queue.empty()) { - RlcInfo("re-transmission required. Retransmission queue size: %d", retx_queue.size()); + RlcInfo("Re-transmission required. Retransmission queue size: %d", retx_queue.size()); return build_retx_pdu(payload, nof_bytes); } @@ -121,7 +121,7 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) // Check whether there is something to TX if (tx_sdu_queue.is_empty()) { - RlcInfo("no data available to be sent"); + RlcInfo("No data available to be sent"); return 0; } @@ -147,15 +147,15 @@ uint32_t rlc_am_nr_tx::build_new_pdu(uint8_t* payload, uint32_t nof_bytes) { // Read new SDU from TX queue unique_byte_buffer_t tx_sdu; - RlcDebug("reading from RLC SDU queue. Queue size %d", tx_sdu_queue.size()); + RlcDebug("Reading from RLC SDU queue. Queue size %d", tx_sdu_queue.size()); do { tx_sdu = tx_sdu_queue.read(); } while (tx_sdu == nullptr && tx_sdu_queue.size() != 0); if (tx_sdu != nullptr) { - RlcDebug("read RLC SDU - %d bytes", tx_sdu->N_bytes); + RlcDebug("Read RLC SDU - %d bytes", tx_sdu->N_bytes); } else { - RlcDebug("no SDUs left in the tx queue."); + RlcDebug("No SDUs left in the tx queue."); return 0; } @@ -164,7 +164,7 @@ uint32_t rlc_am_nr_tx::build_new_pdu(uint8_t* payload, uint32_t nof_bytes) rlc_amd_tx_pdu_nr& tx_pdu = tx_window.add_pdu(st.tx_next); tx_pdu.sdu_buf = srsran::make_byte_buffer(); if (tx_pdu.sdu_buf == nullptr) { - RlcError("couldn't allocate PDU in %s().", __FUNCTION__); + RlcError("Couldn't allocate PDU in %s().", __FUNCTION__); return 0; } @@ -413,6 +413,11 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) } } + RlcDebug("RETX - is_segment=%s, current_so=%d, so_start=%d, so_end=%d", + retx.is_segment ? "true" : "false", + retx.current_so, + retx.so_start, + retx.so_end); if (retx.is_segment) { return build_retx_pdu_from_sdu_segment(retx, payload, nof_bytes); } @@ -493,9 +498,10 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_full_sdu(rlc_amd_retx_t& retx, ui // Sanity check: is this an SDU segment retx? if (retx.is_segment) { - RlcError("called %s, but retx contains a SDU segment. SN=%d, so_start=%d, so_end=%d", + RlcError("called %s, but retx contains a SDU segment. SN=%d, current_so=%d, so_start=%d, so_end=%d", __FUNCTION__, retx.sn, + retx.current_so, retx.so_start, retx.so_end); return 0; @@ -537,8 +543,13 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_full_sdu(rlc_amd_retx_t& retx, ui // Update retx queue. Next SDU segment will be the rest of the PDU retx.is_segment = true; - retx.so_start = retx_pdu_payload_size; - retx.so_end = tx_pdu.sdu_buf->N_bytes; + retx.current_so = retx_pdu_payload_size; + + RlcDebug("Updated RETX info. is_segment=%s, current_so=%d, so_start=%d, so_end=%d", + retx.is_segment ? "true" : "false", + retx.current_so, + retx.so_start, + retx.so_end); return hdr_len + retx_pdu_payload_size; } @@ -560,26 +571,30 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uin srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; - RlcInfo("creating RETX PDU from SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); + RlcInfo("Creating RETX PDU from SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); // Sanity check: is this an SDU segment retx? if (not retx.is_segment) { - RlcError("called %s, but retx contains a SDU segment. SN=%d, so_start=%d, so_end=%d", + RlcError("Called %s, but retx contains a SDU segment. SN=%d, current_so=%d, so_start=%d, current_so=%d, so_end=%d", __FUNCTION__, retx.sn, + retx.current_so, retx.so_start, + retx.current_so, retx.so_end); return 0; } - uint32_t expected_hdr_len = min_hdr_size; - if (retx.so_start != 0) { + uint32_t expected_hdr_len = min_hdr_size; + rlc_nr_si_field_t si = rlc_nr_si_field_t::first_segment; + if (retx.current_so != 0) { + si = rlc_nr_si_field_t::neither_first_nor_last_segment; expected_hdr_len = max_hdr_size; } // Sanity check: are there enough bytes for header plus data? if (nof_bytes <= expected_hdr_len) { - RlcError("called %s, but there are not enough bytes for data plus header. SN=%d, nof_bytes=%d, expected_hdr_len=%d", + RlcError("Called %s, but there are not enough bytes for data plus header. SN=%d, nof_bytes=%d, expected_hdr_len=%d", __FUNCTION__, retx.sn, nof_bytes, @@ -588,33 +603,37 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uin } // Can be transmitted without segmentation? - if (nof_bytes < (retx.so_end - retx.so_start) + expected_hdr_len) { + if (nof_bytes < (retx.so_end - retx.current_so) + expected_hdr_len) { RlcDebug("It is necessary to further segment the SDU segment. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " - "so_start=%d, so_end=%d", + "current_so=%d, so_start=%d, so_end=%d", retx.sn, nof_bytes, expected_hdr_len, + retx.current_so, retx.so_start, retx.so_end); return build_retx_segment_from_sdu_segment(retx, payload, nof_bytes); } else { RlcDebug("SDU segment can be fully transmitted. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " - "so_start=%d, so_end=%d", + "current_so=%d, so_start=%d, so_end=%d", retx.sn, nof_bytes, expected_hdr_len, + retx.current_so, retx.so_start, retx.so_end); + si = rlc_nr_si_field_t::last_segment; } - // Can the RETX PDU be transmitted in a single PDU? - uint32_t retx_pdu_payload_size = (retx.so_end - retx.so_start); + uint32_t retx_pdu_payload_size = (retx.so_end - retx.current_so); // Write header - rlc_am_nr_pdu_header_t hdr = tx_pdu.header; - uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); + rlc_am_nr_pdu_header_t hdr = tx_pdu.header; + hdr.si = si; + hdr.so = retx.current_so; + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); if (hdr_len >= nof_bytes || hdr_len != expected_hdr_len) { - RlcError("error writing AMD PDU header"); + RlcError("Error writing AMD PDU header. nof_bytes=%d, hdr_len=%d", nof_bytes, hdr_len); return 0; } log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); @@ -624,12 +643,27 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uin memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); // Update retx queue - retx.so_start = retx.so_end; - retx.so_end += retx_pdu_payload_size; + retx.current_so = retx.current_so + retx_pdu_payload_size; - if (retx.so_end >= tx_pdu.sdu_buf->N_bytes) { + RlcDebug("Updated RETX info. is_segment=%s, current_so=%d, so_start=%d, so_end=%d", + retx.is_segment ? "true" : "false", + retx.current_so, + retx.so_start, + retx.so_end); + + if (retx.current_so == tx_pdu.sdu_buf->N_bytes) { // Last segment to retx, remove retx from queue retx_queue.pop(); + } else if (retx.current_so > tx_pdu.sdu_buf->N_bytes) { + RlcError("Current SO larger than SDU size. SO_end=%d", retx.so_end); + } + + if (retx.current_so > retx.so_end) { + RlcError("SO end larger than SO start. current_so=%d, SO_start=%d, SO_end=%d", + retx.current_so, + retx.so_start, + retx.so_end); + return 0; } // Update SDU segment info @@ -653,13 +687,14 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; - RlcInfo("creating SDU segment from SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); + RlcDebug("Creating SDU segment from SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); // Sanity check: is this an SDU segment retx? if (not retx.is_segment) { - RlcError("called %s, but retx contains a SDU segment. SN=%d, so_start=%d, so_end=%d", + RlcError("called %s, but retx contains a SDU segment. SN=%d, current_so=%d, so_start=%d, so_end=%d", __FUNCTION__, retx.sn, + retx.current_so, retx.so_start, retx.so_end); return 0; @@ -667,7 +702,7 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, uint32_t expected_hdr_len = min_hdr_size; rlc_nr_si_field_t si = rlc_nr_si_field_t::first_segment; - if (retx.so_start != 0) { + if (retx.current_so != 0) { si = rlc_nr_si_field_t::neither_first_nor_last_segment; expected_hdr_len = max_hdr_size; } @@ -689,7 +724,7 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, // Write header rlc_am_nr_pdu_header_t hdr = tx_pdu.header; - hdr.so = retx.so_start; + hdr.so = retx.current_so; hdr.si = si; uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); if (hdr_len >= nof_bytes || hdr_len != expected_hdr_len) { @@ -704,8 +739,32 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); // Update retx queue - retx.so_start = retx.so_end; - retx.so_end += retx_pdu_payload_size; + retx.current_so = retx.current_so + retx_pdu_payload_size; + + RlcDebug("Updated RETX info. is_segment=%s, current_so=%d, so_start=%d, so_end=%d", + retx.is_segment ? "true" : "false", + retx.current_so, + retx.so_start, + retx.so_end); + + if (retx.current_so >= tx_pdu.sdu_buf->N_bytes) { + RlcError("Current SO larger or equal to SDU size when creating SDU segment. SN=%d, current SO=%d, SO_start=%d, " + "SO_end=%d", + retx.sn, + retx.current_so, + retx.so_start, + retx.so_end); + return 0; + } + + if (retx.current_so >= retx.so_end) { + RlcError("Current SO larger than SO end. SN=%d, current SO=%d, SO_start=%d, SO_end=%s", + retx.sn, + retx.current_so, + retx.so_start, + retx.so_end); + return 0; + } // Update SDU segment info // TODO From 801eddf899d2d16bad19e54b0e2e3665883160a6 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Thu, 27 Jan 2022 14:33:31 +0000 Subject: [PATCH 10/28] lib,rlc_am_nr: fix up test regarding retx segmentation --- lib/test/rlc/rlc_am_nr_test.cc | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index c0bdc11cc..f870babe7 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -421,6 +421,11 @@ int segment_retx_test() rlc_am rlc2(srsran_rat_t::nr, srslog::fetch_basic_logger("RLC_AM_2"), 1, &tester, &tester, &timers); test_delimit_logger delimiter("segment retx PDU"); + rlc_am_nr_tx* tx1 = dynamic_cast(rlc1.get_tx()); + rlc_am_nr_rx* rx1 = dynamic_cast(rlc1.get_rx()); + rlc_am_nr_tx* tx2 = dynamic_cast(rlc2.get_tx()); + rlc_am_nr_rx* rx2 = dynamic_cast(rlc2.get_rx()); + // before configuring entity TESTASSERT(0 == rlc1.get_buffer_state()); @@ -513,10 +518,6 @@ int segment_retx_test() for (int i = 0; i < 3; i++) { byte_buffer_t retx_buf; uint32_t len = 0; - test_logger.info("Looping!!! i=%d", i); - test_logger.info("Looping!!! i=%d", i); - test_logger.info("Looping!!! i=%d", i); - test_logger.info("Looping!!! i=%d", i); if (i == 0) { len = rlc1.read_pdu(retx_buf.msg, 3); TESTASSERT(3 == len); @@ -554,10 +555,10 @@ int segment_retx_test() TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); TESTASSERT_EQ(0, metrics1.num_lost_sdus); // PDU metrics - TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions - TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU - TESTASSERT_EQ(18, - metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + 3 rext (3 * 3) = 34 + TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions + TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU + TESTASSERT_EQ(38, metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 38 TESTASSERT_EQ(3 + 5, metrics1.num_rx_pdu_bytes); // Two status PDU (one with a NACK) TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs @@ -565,14 +566,19 @@ int segment_retx_test() TESTASSERT_EQ(0, metrics2.num_tx_sdus); TESTASSERT_EQ(5, metrics2.num_rx_sdus); TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); - TESTASSERT_EQ(5, metrics2.num_rx_sdu_bytes); + TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each TESTASSERT_EQ(0, metrics2.num_lost_sdus); // SDU metrics TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs - TESTASSERT_EQ(5, metrics2.num_rx_pdus); // 5 PDUs (6 tx'ed, but one was lost) + TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) - TESTASSERT_EQ(15, metrics2.num_rx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS (data) = 15 + TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs + + // Check state + rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); + TESTASSERT_EQ(5, state2_rx.rx_next); return SRSRAN_SUCCESS; } From 720651784610cc3b781419f6afbac87e64786225 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Fri, 28 Jan 2022 12:35:40 +0000 Subject: [PATCH 11/28] lib,rlc_am_nr: starting to add unit test for retx'ing segments. Fixed issue in updating tx_next when segmenting the SDU. --- lib/src/rlc/rlc_am_nr.cc | 2 + lib/test/rlc/rlc_am_nr_test.cc | 202 ++++++++++++++++++++++++++++++++- 2 files changed, 203 insertions(+), 1 deletion(-) diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index f5aec7808..de5856002 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -372,6 +372,8 @@ uint32_t rlc_am_nr_tx::build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, RlcInfo("grant is large enough for full SDU." "Removing current SDU info"); sdu_under_segmentation_sn = INVALID_RLC_SN; + // SDU is fully TX'ed. Increment TX_NEXt + st.tx_next++; } return hdr_len + segment_payload_len; diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index f870babe7..711b5a277 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -350,6 +350,11 @@ int basic_segmentation_test() rlc_am rlc1(srsran_rat_t::nr, srslog::fetch_basic_logger("RLC_AM_1"), 1, &tester, &tester, &timers); rlc_am rlc2(srsran_rat_t::nr, srslog::fetch_basic_logger("RLC_AM_2"), 1, &tester, &tester, &timers); + rlc_am_nr_tx* tx1 = dynamic_cast(rlc1.get_tx()); + rlc_am_nr_rx* rx1 = dynamic_cast(rlc1.get_rx()); + rlc_am_nr_tx* tx2 = dynamic_cast(rlc2.get_tx()); + rlc_am_nr_rx* rx2 = dynamic_cast(rlc2.get_rx()); + // before configuring entity TESTASSERT(0 == rlc1.get_buffer_state()); @@ -407,6 +412,10 @@ int basic_segmentation_test() TESTASSERT_EQ(13, metrics2.num_rx_pdu_bytes); // 1 PDU (No SO) + 2 PDUs (with SO) = 3 + 2*5 TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs + // Check state + rlc_am_nr_tx_state_t state1_tx = tx1->get_tx_state(); + TESTASSERT_EQ(1, state1_tx.tx_next); + return SRSRAN_SUCCESS; } @@ -582,6 +591,197 @@ int segment_retx_test() return SRSRAN_SUCCESS; } +int retx_segment_test() +{ + rlc_am_tester tester; + timer_handler timers(8); + + auto& test_logger = srslog::fetch_basic_logger("TESTER "); + rlc_am rlc1(srsran_rat_t::nr, srslog::fetch_basic_logger("RLC_AM_1"), 1, &tester, &tester, &timers); + rlc_am rlc2(srsran_rat_t::nr, srslog::fetch_basic_logger("RLC_AM_2"), 1, &tester, &tester, &timers); + test_delimit_logger delimiter("retx segment PDU"); + + rlc_am_nr_tx* tx1 = dynamic_cast(rlc1.get_tx()); + rlc_am_nr_rx* rx1 = dynamic_cast(rlc1.get_rx()); + rlc_am_nr_tx* tx2 = dynamic_cast(rlc2.get_tx()); + rlc_am_nr_rx* rx2 = dynamic_cast(rlc2.get_rx()); + + // before configuring entity + TESTASSERT(0 == rlc1.get_buffer_state()); + + if (not rlc1.configure(rlc_config_t::default_rlc_am_nr_config())) { + return -1; + } + + if (not rlc2.configure(rlc_config_t::default_rlc_am_nr_config())) { + return -1; + } + + int n_sdu_bufs = 5; + int n_pdu_bufs = 15; + + // Push 5 SDUs into RLC1 + std::vector sdu_bufs(n_sdu_bufs); + for (int i = 0; i < n_sdu_bufs; i++) { + sdu_bufs[i] = srsran::make_byte_buffer(); + sdu_bufs[i]->msg[0] = i; // Write the index into the buffer + sdu_bufs[i]->N_bytes = 3; // Give each buffer a size of 3 bytes + sdu_bufs[i]->md.pdcp_sn = i; // PDCP SN for notifications + rlc1.write_sdu(std::move(sdu_bufs[i])); + } + + TESTASSERT(25 == rlc1.get_buffer_state()); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) = 25 + + // Read 15 PDUs from RLC1 + std::vector pdu_bufs(n_pdu_bufs); + for (int i = 0; i < n_pdu_bufs; i++) { + pdu_bufs[i] = srsran::make_byte_buffer(); + if (i == 0 || i == 3 || i == 6 || i == 9 || i == 12) { + // First segment, no SO + uint32_t len = rlc1.read_pdu(pdu_bufs[i]->msg, 3); // 2 bytes for header + 1 byte payload + pdu_bufs[i]->N_bytes = len; + TESTASSERT_EQ(3, len); + } else { + // Middle or last segment, SO present + uint32_t len = rlc1.read_pdu(pdu_bufs[i]->msg, 5); // 4 bytes for header + 1 byte payload + pdu_bufs[i]->N_bytes = len; + TESTASSERT_EQ(5, len); + } + } + + TESTASSERT(0 == rlc1.get_buffer_state()); + return SRSRAN_SUCCESS; + // Write 15 - 3 PDUs into RLC2 + for (int i = 0; i < n_pdu_bufs; i++) { + if (i != 3) { + rlc2.write_pdu(pdu_bufs[i]->msg, pdu_bufs[i]->N_bytes); // Lose first segment of RLC_SN=1. + } + if (i != 3) { + rlc2.write_pdu(pdu_bufs[i]->msg, pdu_bufs[i]->N_bytes); // Lose middle segment of RLC_SN=2. + } + if (i != 3) { + rlc2.write_pdu(pdu_bufs[i]->msg, pdu_bufs[i]->N_bytes); // Lose last segment of RLC_SN=3. + } + } + /* + // Only after t-reassembly has expired, will the status report include NACKs. + TESTASSERT(3 == rlc2.get_buffer_state()); + { + // Read status PDU from RLC2 + byte_buffer_t status_buf; + int len = rlc2.read_pdu(status_buf.msg, 5); + status_buf.N_bytes = len; + + TESTASSERT(0 == rlc2.get_buffer_state()); + + // Assert status is correct + rlc_am_nr_status_pdu_t status_check = {}; + rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); + TESTASSERT(status_check.ack_sn == 3); // 3 is the next expected SN (i.e. the lost packet.) + + // Write status PDU to RLC1 + rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); + } + + // Step timers until reassambly timeout expires + for (int cnt = 0; cnt < 35; cnt++) { + timers.step_all(); + } + + // t-reassembly has expired. There should be a NACK in the status report. + TESTASSERT(5 == rlc2.get_buffer_state()); + { + // Read status PDU from RLC2 + byte_buffer_t status_buf; + int len = rlc2.read_pdu(status_buf.msg, 5); + status_buf.N_bytes = len; + + TESTASSERT(0 == rlc2.get_buffer_state()); + + // Assert status is correct + rlc_am_nr_status_pdu_t status_check = {}; + rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); + TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. + TESTASSERT(status_check.N_nack == 1); // We lost one PDU. + TESTASSERT(status_check.nacks[0].nack_sn == 3); // Lost PDU SN=3. + + // Write status PDU to RLC1 + rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); + + // Check there is an Retx of SN=3 + TESTASSERT(5 == rlc1.get_buffer_state()); + } + + { + // Re-transmit PDU in 3 segments + for (int i = 0; i < 3; i++) { + byte_buffer_t retx_buf; + uint32_t len = 0; + if (i == 0) { + len = rlc1.read_pdu(retx_buf.msg, 3); + TESTASSERT(3 == len); + } else { + len = rlc1.read_pdu(retx_buf.msg, 5); + TESTASSERT(5 == len); + } + retx_buf.N_bytes = len; + + rlc_am_nr_pdu_header_t header_check = {}; + uint32_t hdr_len = rlc_am_nr_read_data_pdu_header(&retx_buf, rlc_am_nr_sn_size_t::size12bits, &header_check); + // Double check header. + TESTASSERT(header_check.sn == 3); // Double check RETX SN + if (i == 0) { + TESTASSERT(header_check.si == rlc_nr_si_field_t::first_segment); + } else if (i == 1) { + TESTASSERT(header_check.si == rlc_nr_si_field_t::neither_first_nor_last_segment); + } else { + TESTASSERT(header_check.si == rlc_nr_si_field_t::last_segment); + } + + rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); + } + TESTASSERT(0 == rlc1.get_buffer_state()); + } + + // Check statistics + rlc_bearer_metrics_t metrics1 = rlc1.get_metrics(); + rlc_bearer_metrics_t metrics2 = rlc2.get_metrics(); + + // SDU metrics + TESTASSERT_EQ(5, metrics1.num_tx_sdus); + TESTASSERT_EQ(0, metrics1.num_rx_sdus); + TESTASSERT_EQ(15, metrics1.num_tx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_lost_sdus); + // PDU metrics + TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions + TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU + TESTASSERT_EQ(38, metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 38 + TESTASSERT_EQ(3 + 5, metrics1.num_rx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs + + // PDU metrics + TESTASSERT_EQ(0, metrics2.num_tx_sdus); + TESTASSERT_EQ(5, metrics2.num_rx_sdus); + TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); + TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each + TESTASSERT_EQ(0, metrics2.num_lost_sdus); + // SDU metrics + TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs + TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) + TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 + TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs + + // Check state + rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); + TESTASSERT_EQ(5, state2_rx.rx_next); + */ + return SRSRAN_SUCCESS; +} + int main() { // Setup the log message spy to intercept error and warning log entries from RLC @@ -612,6 +812,6 @@ int main() TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS); TESTASSERT(basic_segmentation_test() == SRSRAN_SUCCESS); TESTASSERT(segment_retx_test() == SRSRAN_SUCCESS); - + TESTASSERT(retx_segment_test() == SRSRAN_SUCCESS); return SRSRAN_SUCCESS; } From 12e530a8003154c23bcb6a800697f91563486e3b Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Fri, 28 Jan 2022 17:21:53 +0000 Subject: [PATCH 12/28] lib,rlc_am_nr: fix generation of status report when NACKs of lost SDU segments are present. --- lib/src/rlc/rlc_am_nr.cc | 39 +++++- lib/test/rlc/rlc_am_nr_test.cc | 239 ++++++++++++++++----------------- 2 files changed, 151 insertions(+), 127 deletions(-) diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index de5856002..7e5647804 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -24,6 +24,8 @@ namespace srsran { const static uint32_t max_tx_queue_size = 256; +const static uint32_t so_end_of_sdu = 0xFFFF; + /**************************************************************************** * RLC AM NR entity ***************************************************************************/ @@ -1254,12 +1256,39 @@ uint32_t rlc_am_nr_rx::get_status_pdu(rlc_am_nr_status_pdu_t* status, uint32_t m uint32_t i = status->ack_sn; while (rx_mod_base_nr(i) <= rx_mod_base_nr(st.rx_highest_status)) { - if (rx_window.has_sn(i) || i == st.rx_highest_status) { - // only update ACK_SN if this SN has been received, or if we reached the maximum possible SN + if ((rx_window.has_sn(i) && rx_window[i].fully_received) || i == st.rx_highest_status) { + // only update ACK_SN if this SN has been fully received, or if we reached the maximum possible SN status->ack_sn = i; } else { - status->nacks[status->N_nack].nack_sn = i; - status->N_nack++; + if (not rx_window.has_sn(i)) { + // No segment received, NACK the whole SDU + status->nacks[status->N_nack].nack_sn = i; + status->N_nack++; + } else if (not rx_window[i].fully_received) { + // Some segments were received, but not all. + // NACK non consecutive missing bytes + uint32_t last_so = 0; + bool last_segment_rx = false; + for (auto segm = rx_window[i].segments.begin(); segm != rx_window[i].segments.end(); segm++) { + if (segm->header.so != last_so) { + // Some bytes were not received + status->nacks[status->N_nack].nack_sn = i; + status->nacks[status->N_nack].so_start = last_so; + status->nacks[status->N_nack].so_end = segm->header.so; + status->N_nack++; + } + if (segm->header.si == rlc_nr_si_field_t::last_segment) { + last_segment_rx = true; + } + last_so = segm->header.so + segm->buf->N_bytes; + } + if (not last_segment_rx) { + status->nacks[status->N_nack].nack_sn = i; + status->nacks[status->N_nack].so_start = last_so; + status->nacks[status->N_nack].so_end = so_end_of_sdu; + status->N_nack++; + } + } } // make sure we don't exceed grant size (FIXME) @@ -1313,7 +1342,7 @@ void rlc_am_nr_rx::timer_expired(uint32_t timeout_id) */ for (uint32_t tmp_sn = st.rx_next_status_trigger; tmp_sn < st.rx_next_status_trigger + RLC_AM_WINDOW_SIZE; tmp_sn++) { - if (not rx_window.has_sn(tmp_sn) || not rx_window[tmp_sn].fully_received) { + if (not rx_window.has_sn(tmp_sn)) { st.rx_highest_status = tmp_sn; break; } diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index 711b5a277..d8518a800 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -650,135 +650,130 @@ int retx_segment_test() } TESTASSERT(0 == rlc1.get_buffer_state()); - return SRSRAN_SUCCESS; + // Write 15 - 3 PDUs into RLC2 for (int i = 0; i < n_pdu_bufs; i++) { - if (i != 3) { + if (i != 3 && i != 7 && i != 11) { rlc2.write_pdu(pdu_bufs[i]->msg, pdu_bufs[i]->N_bytes); // Lose first segment of RLC_SN=1. } - if (i != 3) { - rlc2.write_pdu(pdu_bufs[i]->msg, pdu_bufs[i]->N_bytes); // Lose middle segment of RLC_SN=2. - } - if (i != 3) { - rlc2.write_pdu(pdu_bufs[i]->msg, pdu_bufs[i]->N_bytes); // Lose last segment of RLC_SN=3. - } + } + + // Only after t-reassembly has expired, will the status report include NACKs. + TESTASSERT(3 == rlc2.get_buffer_state()); + { + // Read status PDU from RLC2 + byte_buffer_t status_buf; + int len = rlc2.read_pdu(status_buf.msg, 5); + status_buf.N_bytes = len; + + TESTASSERT(0 == rlc2.get_buffer_state()); + + // Assert status is correct + rlc_am_nr_status_pdu_t status_check = {}; + rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); + TESTASSERT(status_check.ack_sn == 1); // 1 is the next expected SN (i.e. the first lost packet.) + + // Write status PDU to RLC1 + rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); + } + + // Step timers until reassambly timeout expires + for (int cnt = 0; cnt < 35; cnt++) { + timers.step_all(); + } + + // t-reassembly has expired. There should be a NACK in the status report. + // There should be 3 NACKs with SO_start and SO_end + TESTASSERT(20 == rlc2.get_buffer_state()); // 2 bytes for ACK + 3 * 6 for NACK with SO = 20. + { + // Read status PDU from RLC2 + byte_buffer_t status_buf; + int len = rlc2.read_pdu(status_buf.msg, 20); + status_buf.N_bytes = len; + + TESTASSERT(0 == rlc2.get_buffer_state()); + + // Assert status is correct + rlc_am_nr_status_pdu_t status_check = {}; + rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); + TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. + TESTASSERT(status_check.N_nack == 3); // We lost one PDU. + TESTASSERT(status_check.nacks[0].nack_sn == 3); // Lost PDU SN=3. + + // Write status PDU to RLC1 + rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); + + // Check there is an Retx of SN=3 + TESTASSERT(5 == rlc1.get_buffer_state()); } /* - // Only after t-reassembly has expired, will the status report include NACKs. - TESTASSERT(3 == rlc2.get_buffer_state()); - { - // Read status PDU from RLC2 - byte_buffer_t status_buf; - int len = rlc2.read_pdu(status_buf.msg, 5); - status_buf.N_bytes = len; - - TESTASSERT(0 == rlc2.get_buffer_state()); - - // Assert status is correct - rlc_am_nr_status_pdu_t status_check = {}; - rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 3); // 3 is the next expected SN (i.e. the lost packet.) - - // Write status PDU to RLC1 - rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); - } - - // Step timers until reassambly timeout expires - for (int cnt = 0; cnt < 35; cnt++) { - timers.step_all(); - } - - // t-reassembly has expired. There should be a NACK in the status report. - TESTASSERT(5 == rlc2.get_buffer_state()); - { - // Read status PDU from RLC2 - byte_buffer_t status_buf; - int len = rlc2.read_pdu(status_buf.msg, 5); - status_buf.N_bytes = len; - - TESTASSERT(0 == rlc2.get_buffer_state()); - - // Assert status is correct - rlc_am_nr_status_pdu_t status_check = {}; - rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. - TESTASSERT(status_check.N_nack == 1); // We lost one PDU. - TESTASSERT(status_check.nacks[0].nack_sn == 3); // Lost PDU SN=3. - - // Write status PDU to RLC1 - rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); - - // Check there is an Retx of SN=3 - TESTASSERT(5 == rlc1.get_buffer_state()); - } - - { - // Re-transmit PDU in 3 segments - for (int i = 0; i < 3; i++) { - byte_buffer_t retx_buf; - uint32_t len = 0; - if (i == 0) { - len = rlc1.read_pdu(retx_buf.msg, 3); - TESTASSERT(3 == len); - } else { - len = rlc1.read_pdu(retx_buf.msg, 5); - TESTASSERT(5 == len); - } - retx_buf.N_bytes = len; - - rlc_am_nr_pdu_header_t header_check = {}; - uint32_t hdr_len = rlc_am_nr_read_data_pdu_header(&retx_buf, rlc_am_nr_sn_size_t::size12bits, &header_check); - // Double check header. - TESTASSERT(header_check.sn == 3); // Double check RETX SN - if (i == 0) { - TESTASSERT(header_check.si == rlc_nr_si_field_t::first_segment); - } else if (i == 1) { - TESTASSERT(header_check.si == rlc_nr_si_field_t::neither_first_nor_last_segment); - } else { - TESTASSERT(header_check.si == rlc_nr_si_field_t::last_segment); - } - - rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); + { + // Re-transmit PDU in 3 segments + for (int i = 0; i < 3; i++) { + byte_buffer_t retx_buf; + uint32_t len = 0; + if (i == 0) { + len = rlc1.read_pdu(retx_buf.msg, 3); + TESTASSERT(3 == len); + } else { + len = rlc1.read_pdu(retx_buf.msg, 5); + TESTASSERT(5 == len); } - TESTASSERT(0 == rlc1.get_buffer_state()); + retx_buf.N_bytes = len; + + rlc_am_nr_pdu_header_t header_check = {}; + uint32_t hdr_len = rlc_am_nr_read_data_pdu_header(&retx_buf, rlc_am_nr_sn_size_t::size12bits, &header_check); + // Double check header. + TESTASSERT(header_check.sn == 3); // Double check RETX SN + if (i == 0) { + TESTASSERT(header_check.si == rlc_nr_si_field_t::first_segment); + } else if (i == 1) { + TESTASSERT(header_check.si == rlc_nr_si_field_t::neither_first_nor_last_segment); + } else { + TESTASSERT(header_check.si == rlc_nr_si_field_t::last_segment); + } + + rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); } + TESTASSERT(0 == rlc1.get_buffer_state()); + } - // Check statistics - rlc_bearer_metrics_t metrics1 = rlc1.get_metrics(); - rlc_bearer_metrics_t metrics2 = rlc2.get_metrics(); + // Check statistics + rlc_bearer_metrics_t metrics1 = rlc1.get_metrics(); + rlc_bearer_metrics_t metrics2 = rlc2.get_metrics(); - // SDU metrics - TESTASSERT_EQ(5, metrics1.num_tx_sdus); - TESTASSERT_EQ(0, metrics1.num_rx_sdus); - TESTASSERT_EQ(15, metrics1.num_tx_sdu_bytes); - TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); - TESTASSERT_EQ(0, metrics1.num_lost_sdus); - // PDU metrics - TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions - TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU - TESTASSERT_EQ(38, metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + - // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 38 - TESTASSERT_EQ(3 + 5, metrics1.num_rx_pdu_bytes); // Two status PDU (one with a NACK) - TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs + // SDU metrics + TESTASSERT_EQ(5, metrics1.num_tx_sdus); + TESTASSERT_EQ(0, metrics1.num_rx_sdus); + TESTASSERT_EQ(15, metrics1.num_tx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_lost_sdus); + // PDU metrics + TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions + TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU + TESTASSERT_EQ(38, metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 38 + TESTASSERT_EQ(3 + 5, metrics1.num_rx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs - // PDU metrics - TESTASSERT_EQ(0, metrics2.num_tx_sdus); - TESTASSERT_EQ(5, metrics2.num_rx_sdus); - TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); - TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each - TESTASSERT_EQ(0, metrics2.num_lost_sdus); - // SDU metrics - TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs - TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) - TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) - TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) - // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 - TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs + // PDU metrics + TESTASSERT_EQ(0, metrics2.num_tx_sdus); + TESTASSERT_EQ(5, metrics2.num_rx_sdus); + TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); + TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each + TESTASSERT_EQ(0, metrics2.num_lost_sdus); + // SDU metrics + TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs + TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) + TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 + TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs - // Check state - rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); - TESTASSERT_EQ(5, state2_rx.rx_next); - */ + // Check state + rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); + TESTASSERT_EQ(5, state2_rx.rx_next); + */ return SRSRAN_SUCCESS; } @@ -807,11 +802,11 @@ int main() // start log back-end srslog::init(); - TESTASSERT(window_checker_test() == SRSRAN_SUCCESS); - TESTASSERT(basic_test() == SRSRAN_SUCCESS); - TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS); - TESTASSERT(basic_segmentation_test() == SRSRAN_SUCCESS); - TESTASSERT(segment_retx_test() == SRSRAN_SUCCESS); + // TESTASSERT(window_checker_test() == SRSRAN_SUCCESS); + // TESTASSERT(basic_test() == SRSRAN_SUCCESS); + // TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS); + // TESTASSERT(basic_segmentation_test() == SRSRAN_SUCCESS); + // TESTASSERT(segment_retx_test() == SRSRAN_SUCCESS); TESTASSERT(retx_segment_test() == SRSRAN_SUCCESS); return SRSRAN_SUCCESS; } From 047831e17825d4d729196b17c656cb7fe2dfd209 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Mon, 31 Jan 2022 18:23:24 +0000 Subject: [PATCH 13/28] lib,rlc_am_nr: fixup compilation of rlc_am_nr_pdu_test when saving pcaps --- lib/test/rlc/CMakeLists.txt | 4 +-- lib/test/rlc/rlc_am_nr_pdu_test.cc | 46 +++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/lib/test/rlc/CMakeLists.txt b/lib/test/rlc/CMakeLists.txt index dc57c224e..7768eb3be 100644 --- a/lib/test/rlc/CMakeLists.txt +++ b/lib/test/rlc/CMakeLists.txt @@ -25,8 +25,8 @@ target_link_libraries(rlc_am_nr_test srsran_rlc srsran_phy srsran_common) add_nr_test(rlc_am_nr_test rlc_am_nr_test) add_executable(rlc_am_nr_pdu_test rlc_am_nr_pdu_test.cc) -target_link_libraries(rlc_am_nr_pdu_test srsran_rlc srsran_phy) -add_nr_test(rlc_am_nr_pdu_test rlc_am_nr_pdu_test) +target_link_libraries(rlc_am_nr_pdu_test srsran_rlc srsran_phy srsran_mac srsran_common ) +add_nr_test(rlc_am_nr_pdu_test rlc_am_nr_pdu_test ) add_executable(rlc_stress_test rlc_stress_test.cc) target_link_libraries(rlc_stress_test srsran_rlc srsran_mac srsran_phy srsran_common ${Boost_LIBRARIES} ${ATOMIC_LIBS}) diff --git a/lib/test/rlc/rlc_am_nr_pdu_test.cc b/lib/test/rlc/rlc_am_nr_pdu_test.cc index a2257c9d9..445722134 100644 --- a/lib/test/rlc/rlc_am_nr_pdu_test.cc +++ b/lib/test/rlc/rlc_am_nr_pdu_test.cc @@ -27,16 +27,17 @@ } \ } -#define PCAP 0 +#define PCAP 1 #define PCAP_CRNTI (0x1001) #define PCAP_TTI (666) using namespace srsran; #if PCAP -#include "srsran/common/mac_nr_pcap.h" -#include "srsran/mac/mac_nr_pdu.h" -static std::unique_ptr pcap_handle = nullptr; +#include "srsran/common/mac_pcap.h" +#include "srsran/mac/mac_rar_pdu_nr.h" +#include "srsran/mac/mac_sch_pdu_nr.h" +static std::unique_ptr pcap_handle = nullptr; #endif int write_pdu_to_pcap(const uint32_t lcid, const uint8_t* payload, const uint32_t len) @@ -44,11 +45,11 @@ int write_pdu_to_pcap(const uint32_t lcid, const uint8_t* payload, const uint32_ #if PCAP if (pcap_handle) { byte_buffer_t tx_buffer; - srsran::mac_nr_sch_pdu tx_pdu; + srsran::mac_sch_pdu_nr tx_pdu; tx_pdu.init_tx(&tx_buffer, len + 10); tx_pdu.add_sdu(lcid, payload, len); tx_pdu.pack(); - pcap_handle->write_dl_crnti(tx_buffer.msg, tx_buffer.N_bytes, PCAP_CRNTI, true, PCAP_TTI); + pcap_handle->write_dl_crnti_nr(tx_buffer.msg, tx_buffer.N_bytes, PCAP_CRNTI, true, PCAP_TTI); return SRSRAN_SUCCESS; } #endif @@ -285,10 +286,41 @@ int rlc_am_nr_control_pdu_test2() return SRSRAN_SUCCESS; } +// Status PDU for 12bit SN with ACK_SN=2065, NACK_SN=273, SO_START=2, SO_END=5, NACK_SN=275, SO_START=5, SO_END=0xFF +// E1 bit set) +int rlc_am_nr_control_pdu_test3() +{ + const int len = 5; + std::array tv = {0x08, 0x11, 0x80, 0x11, 0x10}; + srsran::byte_buffer_t pdu = make_pdu_and_log(tv); + + TESTASSERT(rlc_am_is_control_pdu(pdu.msg) == true); + + // unpack PDU + rlc_am_nr_status_pdu_t status_pdu = {}; + TESTASSERT(rlc_am_nr_read_status_pdu(&pdu, srsran::rlc_am_nr_sn_size_t::size12bits, &status_pdu) == SRSRAN_SUCCESS); + TESTASSERT(status_pdu.ack_sn == 2065); + TESTASSERT(status_pdu.N_nack == 1); + TESTASSERT(status_pdu.nacks[0].nack_sn == 273); + + // reset status PDU + pdu.clear(); + + // pack again + TESTASSERT(rlc_am_nr_write_status_pdu(status_pdu, srsran::rlc_am_nr_sn_size_t::size12bits, &pdu) == SRSRAN_SUCCESS); + // TESTASSERT(pdu.N_bytes == tv.size()); + + write_pdu_to_pcap(4, pdu.msg, pdu.N_bytes); + + TESTASSERT(memcmp(pdu.msg, tv.data(), pdu.N_bytes) == 0); + + return SRSRAN_SUCCESS; +} + int main(int argc, char** argv) { #if PCAP - pcap_handle = std::unique_ptr(new srsran::mac_nr_pcap()); + pcap_handle = std::unique_ptr(new srsran::mac_pcap()); pcap_handle->open("rlc_am_nr_pdu_test.pcap"); #endif From 876c45c534d531288114b968fe09f6175e62d578 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Wed, 2 Feb 2022 12:50:29 +0000 Subject: [PATCH 14/28] lib,rlc_am_nr: added ability to pack/unpack SO_start and SO_end to rlc am nr NACKs. Added unit test for this feature. --- lib/src/rlc/rlc_am_nr_packing.cc | 49 +++++++++++++++++--- lib/test/rlc/rlc_am_nr_pdu_test.cc | 74 ++++++++++++++++++++---------- 2 files changed, 91 insertions(+), 32 deletions(-) diff --git a/lib/src/rlc/rlc_am_nr_packing.cc b/lib/src/rlc/rlc_am_nr_packing.cc index 359ebcfff..709310245 100644 --- a/lib/src/rlc/rlc_am_nr_packing.cc +++ b/lib/src/rlc/rlc_am_nr_packing.cc @@ -185,15 +185,30 @@ uint32_t rlc_am_nr_read_status_pdu(const uint8_t* payload, // reset number of acks status->N_nack = 0; - if (e1) { + while (e1 != 0) { // E1 flag set, read a NACK_SN rlc_status_nack_t nack = {}; nack.nack_sn = (*ptr & 0xff) << 4; ptr++; + + e1 = *ptr & 0x08; + uint8_t e2 = *ptr & 0x04; + // uint8_t len2 = (*ptr & 0xF0) >> 4; nack.nack_sn |= (*ptr & 0xF0) >> 4; status->nacks[status->N_nack] = nack; + ptr++; + if (e2 != 0) { + status->nacks[status->N_nack].so_start = (*ptr) << 8; + ptr++; + status->nacks[status->N_nack].so_start |= (*ptr); + ptr++; + status->nacks[status->N_nack].so_end = (*ptr) << 8; + ptr++; + status->nacks[status->N_nack].so_end |= (*ptr); + ptr++; + } status->N_nack++; } } @@ -228,13 +243,33 @@ int32_t rlc_am_nr_write_status_pdu(const rlc_am_nr_status_pdu_t& status_pdu, ptr++; if (status_pdu.N_nack > 0) { - // write first 8 bit of NACK_SN - *ptr = (status_pdu.nacks[0].nack_sn >> 4) & 0xff; - ptr++; + for (uint32_t i = 0; i < status_pdu.N_nack; i++) { + if (status_pdu.nacks[i].so_start != 0 || status_pdu.nacks[i].so_end != 0) { + // write first 8 bit of NACK_SN + *ptr = (status_pdu.nacks[i].nack_sn >> 4) & 0xff; + ptr++; - // write remaining 4 bits of NACK_SN - *ptr = (status_pdu.nacks[0].nack_sn & 0x0f) << 4; - ptr++; + // write remaining 4 bits of NACK_SN + *ptr = (status_pdu.nacks[i].nack_sn & 0x0f) << 4; + + // Set E1 if necessary + if (i < (uint32_t)(status_pdu.N_nack - 1)) { + *ptr |= 0x08; + } + // Set E2 + *ptr |= 0x04; + + ptr++; + (*ptr) = status_pdu.nacks[i].so_start >> 8; + ptr++; + (*ptr) = status_pdu.nacks[i].so_start; + ptr++; + (*ptr) = status_pdu.nacks[i].so_end >> 8; + ptr++; + (*ptr) = status_pdu.nacks[i].so_end; + ptr++; + } + } } } else { // 18bit SN diff --git a/lib/test/rlc/rlc_am_nr_pdu_test.cc b/lib/test/rlc/rlc_am_nr_pdu_test.cc index 445722134..dff22f9d4 100644 --- a/lib/test/rlc/rlc_am_nr_pdu_test.cc +++ b/lib/test/rlc/rlc_am_nr_pdu_test.cc @@ -10,39 +10,29 @@ * */ +#include "srsran/common/test_common.h" #include "srsran/config.h" #include "srsran/rlc/rlc.h" #include "srsran/rlc/rlc_am_nr_packing.h" #include +#include #include #include #include -#define TESTASSERT(cond) \ - { \ - if (!(cond)) { \ - std::cout << "[" << __FUNCTION__ << "][Line " << __LINE__ << "]: FAIL at " << (#cond) << std::endl; \ - return -1; \ - } \ - } - -#define PCAP 1 #define PCAP_CRNTI (0x1001) #define PCAP_TTI (666) using namespace srsran; -#if PCAP #include "srsran/common/mac_pcap.h" #include "srsran/mac/mac_rar_pdu_nr.h" #include "srsran/mac/mac_sch_pdu_nr.h" static std::unique_ptr pcap_handle = nullptr; -#endif int write_pdu_to_pcap(const uint32_t lcid, const uint8_t* payload, const uint32_t len) { -#if PCAP if (pcap_handle) { byte_buffer_t tx_buffer; srsran::mac_sch_pdu_nr tx_pdu; @@ -52,7 +42,6 @@ int write_pdu_to_pcap(const uint32_t lcid, const uint8_t* payload, const uint32_ pcap_handle->write_dl_crnti_nr(tx_buffer.msg, tx_buffer.N_bytes, PCAP_CRNTI, true, PCAP_TTI); return SRSRAN_SUCCESS; } -#endif return SRSRAN_ERROR; } @@ -79,6 +68,7 @@ void corrupt_pdu_header(srsran::byte_buffer_t& pdu, const uint32_t header_len, c // RLC AM PDU 12bit with complete SDU int rlc_am_nr_pdu_test1() { + test_delimit_logger delimiter("PDU test 1"); const int header_len = 2, payload_len = 4; std::array tv = {0x80, 0x00, 0x11, 0x22, 0x33, 0x44}; srsran::byte_buffer_t pdu = make_pdu_and_log(tv); @@ -102,6 +92,7 @@ int rlc_am_nr_pdu_test1() // RLC AM PDU 12bit first segment of SDU with P flag and SN 511 int rlc_am_nr_pdu_test2() { + test_delimit_logger delimiter("PDU test 2"); const int header_len = 2, payload_len = 4; std::array tv = {0xd1, 0xff, 0x11, 0x22, 0x33, 0x44}; srsran::byte_buffer_t pdu = make_pdu_and_log(tv); @@ -130,6 +121,7 @@ int rlc_am_nr_pdu_test2() // RLC AM PDU 12bit last segment of SDU without P flag and SN 0x0404 and SO 0x0404 (1028) int rlc_am_nr_pdu_test3() { + test_delimit_logger delimiter("PDU test 3"); const int header_len = 4, payload_len = 4; std::array tv = {0xa4, 0x04, 0x04, 0x04, 0x11, 0x22, 0x33, 0x44}; srsran::byte_buffer_t pdu = make_pdu_and_log(tv); @@ -158,6 +150,7 @@ int rlc_am_nr_pdu_test3() // RLC AM PDU 18bit full SDU with P flag and SN 0x100000001000000010 (131586) int rlc_am_nr_pdu_test4() { + test_delimit_logger delimiter("PDU test 4"); const int header_len = 3, payload_len = 4; std::array tv = {0xc2, 0x02, 0x02, 0x11, 0x22, 0x33, 0x44}; srsran::byte_buffer_t pdu = make_pdu_and_log(tv); @@ -186,6 +179,7 @@ int rlc_am_nr_pdu_test4() // RLC AM PDU 18bit middle part of SDU (SO 514) without P flag and SN 131327 int rlc_am_nr_pdu_test5() { + test_delimit_logger delimiter("PDU test 5"); const int header_len = 5, payload_len = 4; std::array tv = {0xb2, 0x00, 0xff, 0x02, 0x02, 0x11, 0x22, 0x33, 0x44}; srsran::byte_buffer_t pdu = make_pdu_and_log(tv); @@ -214,6 +208,7 @@ int rlc_am_nr_pdu_test5() // Malformed RLC AM PDU 18bit with reserved bits set int rlc_am_nr_pdu_test6() { + test_delimit_logger delimiter("PDU test 6"); const int header_len = 5, payload_len = 4; std::array tv = {0xb7, 0x00, 0xff, 0x02, 0x02, 0x11, 0x22, 0x33, 0x44}; srsran::byte_buffer_t pdu = make_pdu_and_log(tv); @@ -230,6 +225,7 @@ int rlc_am_nr_pdu_test6() // Status PDU for 12bit SN with ACK_SN=2065 and no further NACK_SN (E1 bit not set) int rlc_am_nr_control_pdu_test1() { + test_delimit_logger delimiter("Control PDU test 1"); const int len = 3; std::array tv = {0x08, 0x11, 0x00}; srsran::byte_buffer_t pdu = make_pdu_and_log(tv); @@ -259,6 +255,7 @@ int rlc_am_nr_control_pdu_test1() // Status PDU for 12bit SN with ACK_SN=2065 and NACK_SN=273 (E1 bit set) int rlc_am_nr_control_pdu_test2() { + test_delimit_logger delimiter("Control PDU test 2"); const int len = 5; std::array tv = {0x08, 0x11, 0x80, 0x11, 0x10}; srsran::byte_buffer_t pdu = make_pdu_and_log(tv); @@ -286,13 +283,14 @@ int rlc_am_nr_control_pdu_test2() return SRSRAN_SUCCESS; } -// Status PDU for 12bit SN with ACK_SN=2065, NACK_SN=273, SO_START=2, SO_END=5, NACK_SN=275, SO_START=5, SO_END=0xFF -// E1 bit set) +// Status PDU for 12bit SN with ACK_SN=2065, NACK_SN=273, SO_START=2, SO_END=5, NACK_SN=275, SO_START=5, SO_END=0xFFFF +// E1 and E2 bit set on first NACK, only E2 on second. int rlc_am_nr_control_pdu_test3() { - const int len = 5; - std::array tv = {0x08, 0x11, 0x80, 0x11, 0x10}; - srsran::byte_buffer_t pdu = make_pdu_and_log(tv); + const int len = 15; + std::array tv = { + 0x08, 0x11, 0x80, 0x11, 0x1c, 0x00, 0x02, 0x00, 0x05, 0x11, 0x34, 0x00, 0x05, 0xFF, 0xFF}; + srsran::byte_buffer_t pdu = make_pdu_and_log(tv); TESTASSERT(rlc_am_is_control_pdu(pdu.msg) == true); @@ -300,18 +298,22 @@ int rlc_am_nr_control_pdu_test3() rlc_am_nr_status_pdu_t status_pdu = {}; TESTASSERT(rlc_am_nr_read_status_pdu(&pdu, srsran::rlc_am_nr_sn_size_t::size12bits, &status_pdu) == SRSRAN_SUCCESS); TESTASSERT(status_pdu.ack_sn == 2065); - TESTASSERT(status_pdu.N_nack == 1); + TESTASSERT(status_pdu.N_nack == 2); TESTASSERT(status_pdu.nacks[0].nack_sn == 273); + TESTASSERT(status_pdu.nacks[0].so_start == 2); + TESTASSERT(status_pdu.nacks[0].so_end == 5); + TESTASSERT(status_pdu.nacks[1].nack_sn == 275); + TESTASSERT(status_pdu.nacks[1].so_start == 5); + TESTASSERT(status_pdu.nacks[1].so_end == 0xFFFF); // reset status PDU pdu.clear(); // pack again TESTASSERT(rlc_am_nr_write_status_pdu(status_pdu, srsran::rlc_am_nr_sn_size_t::size12bits, &pdu) == SRSRAN_SUCCESS); - // TESTASSERT(pdu.N_bytes == tv.size()); + TESTASSERT(pdu.N_bytes == tv.size()); write_pdu_to_pcap(4, pdu.msg, pdu.N_bytes); - TESTASSERT(memcmp(pdu.msg, tv.data(), pdu.N_bytes) == 0); return SRSRAN_SUCCESS; @@ -319,10 +321,27 @@ int rlc_am_nr_control_pdu_test3() int main(int argc, char** argv) { -#if PCAP - pcap_handle = std::unique_ptr(new srsran::mac_pcap()); - pcap_handle->open("rlc_am_nr_pdu_test.pcap"); -#endif + static const struct option long_options[] = {{"pcap", no_argument, nullptr, 'p'}, {nullptr, 0, nullptr, 0}}; + + // Parse arguments + while (true) { + int option_index = 0; + int c = getopt_long(argc, argv, "p", long_options, &option_index); + if (c == -1) { + break; + } + + switch (c) { + case 'p': + printf("Setting up PCAP\n"); + pcap_handle = std::unique_ptr(new srsran::mac_pcap()); + pcap_handle->open("rlc_am_nr_pdu_test.pcap"); + break; + default: + fprintf(stderr, "error parsing arguments\n"); + return SRSRAN_ERROR; + } + } srslog::init(); @@ -366,5 +385,10 @@ int main(int argc, char** argv) return SRSRAN_ERROR; } + if (rlc_am_nr_control_pdu_test3()) { + fprintf(stderr, "rlc_am_nr_control_pdu_test3() failed.\n"); + return SRSRAN_ERROR; + } + return SRSRAN_SUCCESS; } From b13d0ca84af303919f501989a4d1abeaec12d64f Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Wed, 2 Feb 2022 17:03:41 +0000 Subject: [PATCH 15/28] lib,rlc_am_nr: fixed generation of status report when SOs are present. Starting to test retx of segments. --- lib/src/rlc/rlc_am_nr.cc | 2 + lib/src/rlc/rlc_am_nr_packing.cc | 3 +- lib/test/rlc/rlc_am_nr_pdu_test.cc | 1 + lib/test/rlc/rlc_am_nr_test.cc | 98 +++++++++++++++++------------- 4 files changed, 60 insertions(+), 44 deletions(-) diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 7e5647804..519a4157e 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -1273,6 +1273,7 @@ uint32_t rlc_am_nr_rx::get_status_pdu(rlc_am_nr_status_pdu_t* status, uint32_t m if (segm->header.so != last_so) { // Some bytes were not received status->nacks[status->N_nack].nack_sn = i; + status->nacks[status->N_nack].has_so = true; status->nacks[status->N_nack].so_start = last_so; status->nacks[status->N_nack].so_end = segm->header.so; status->N_nack++; @@ -1284,6 +1285,7 @@ uint32_t rlc_am_nr_rx::get_status_pdu(rlc_am_nr_status_pdu_t* status, uint32_t m } if (not last_segment_rx) { status->nacks[status->N_nack].nack_sn = i; + status->nacks[status->N_nack].has_so = true; status->nacks[status->N_nack].so_start = last_so; status->nacks[status->N_nack].so_end = so_end_of_sdu; status->N_nack++; diff --git a/lib/src/rlc/rlc_am_nr_packing.cc b/lib/src/rlc/rlc_am_nr_packing.cc index 709310245..031aaa406 100644 --- a/lib/src/rlc/rlc_am_nr_packing.cc +++ b/lib/src/rlc/rlc_am_nr_packing.cc @@ -200,6 +200,7 @@ uint32_t rlc_am_nr_read_status_pdu(const uint8_t* payload, ptr++; if (e2 != 0) { + status->nacks[status->N_nack].has_so = true; status->nacks[status->N_nack].so_start = (*ptr) << 8; ptr++; status->nacks[status->N_nack].so_start |= (*ptr); @@ -244,7 +245,7 @@ int32_t rlc_am_nr_write_status_pdu(const rlc_am_nr_status_pdu_t& status_pdu, if (status_pdu.N_nack > 0) { for (uint32_t i = 0; i < status_pdu.N_nack; i++) { - if (status_pdu.nacks[i].so_start != 0 || status_pdu.nacks[i].so_end != 0) { + if (status_pdu.nacks[i].has_so) { // write first 8 bit of NACK_SN *ptr = (status_pdu.nacks[i].nack_sn >> 4) & 0xff; ptr++; diff --git a/lib/test/rlc/rlc_am_nr_pdu_test.cc b/lib/test/rlc/rlc_am_nr_pdu_test.cc index dff22f9d4..2b456e0a4 100644 --- a/lib/test/rlc/rlc_am_nr_pdu_test.cc +++ b/lib/test/rlc/rlc_am_nr_pdu_test.cc @@ -287,6 +287,7 @@ int rlc_am_nr_control_pdu_test2() // E1 and E2 bit set on first NACK, only E2 on second. int rlc_am_nr_control_pdu_test3() { + test_delimit_logger delimiter("Control PDU test 3"); const int len = 15; std::array tv = { 0x08, 0x11, 0x80, 0x11, 0x1c, 0x00, 0x02, 0x00, 0x05, 0x11, 0x34, 0x00, 0x05, 0xFF, 0xFF}; diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index d8518a800..ebb9b5ae0 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -684,11 +684,11 @@ int retx_segment_test() // t-reassembly has expired. There should be a NACK in the status report. // There should be 3 NACKs with SO_start and SO_end - TESTASSERT(20 == rlc2.get_buffer_state()); // 2 bytes for ACK + 3 * 6 for NACK with SO = 20. + TESTASSERT(21 == rlc2.get_buffer_state()); // 3 bytes for fixed header (ACK+E1) + 3 * 6 for NACK with SO = 21. { // Read status PDU from RLC2 byte_buffer_t status_buf; - int len = rlc2.read_pdu(status_buf.msg, 20); + int len = rlc2.read_pdu(status_buf.msg, 21); status_buf.N_bytes = len; TESTASSERT(0 == rlc2.get_buffer_state()); @@ -696,9 +696,20 @@ int retx_segment_test() // Assert status is correct rlc_am_nr_status_pdu_t status_check = {}; rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. - TESTASSERT(status_check.N_nack == 3); // We lost one PDU. - TESTASSERT(status_check.nacks[0].nack_sn == 3); // Lost PDU SN=3. + TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. + TESTASSERT(status_check.N_nack == 3); // We lost one PDU. + TESTASSERT(status_check.nacks[0].nack_sn == 1); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[0].has_so == true); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[0].so_start == 0); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[0].so_end == 1); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[1].nack_sn == 2); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[1].has_so == true); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[1].so_start == 1); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[1].so_end == 2); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[2].nack_sn == 3); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[2].has_so == true); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[2].so_start == 2); // Lost SDU on SN=1. + TESTASSERT(status_check.nacks[2].so_end == 0xFFFF); // Lost SDU on SN=1. // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); @@ -706,9 +717,9 @@ int retx_segment_test() // Check there is an Retx of SN=3 TESTASSERT(5 == rlc1.get_buffer_state()); } - /* + { - // Re-transmit PDU in 3 segments + // Re-transmit the 3 lost segments for (int i = 0; i < 3; i++) { byte_buffer_t retx_buf; uint32_t len = 0; @@ -724,12 +735,14 @@ int retx_segment_test() rlc_am_nr_pdu_header_t header_check = {}; uint32_t hdr_len = rlc_am_nr_read_data_pdu_header(&retx_buf, rlc_am_nr_sn_size_t::size12bits, &header_check); // Double check header. - TESTASSERT(header_check.sn == 3); // Double check RETX SN if (i == 0) { + TESTASSERT(header_check.sn == 1); // Double check RETX SN TESTASSERT(header_check.si == rlc_nr_si_field_t::first_segment); } else if (i == 1) { + TESTASSERT(header_check.sn == 2); // Double check RETX SN TESTASSERT(header_check.si == rlc_nr_si_field_t::neither_first_nor_last_segment); } else { + TESTASSERT(header_check.sn == 3); // Double check RETX SN TESTASSERT(header_check.si == rlc_nr_si_field_t::last_segment); } @@ -737,43 +750,43 @@ int retx_segment_test() } TESTASSERT(0 == rlc1.get_buffer_state()); } + /* + // Check statistics + rlc_bearer_metrics_t metrics1 = rlc1.get_metrics(); + rlc_bearer_metrics_t metrics2 = rlc2.get_metrics(); - // Check statistics - rlc_bearer_metrics_t metrics1 = rlc1.get_metrics(); - rlc_bearer_metrics_t metrics2 = rlc2.get_metrics(); + // SDU metrics + TESTASSERT_EQ(5, metrics1.num_tx_sdus); + TESTASSERT_EQ(0, metrics1.num_rx_sdus); + TESTASSERT_EQ(15, metrics1.num_tx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_lost_sdus); + // PDU metrics + TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions + TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU + TESTASSERT_EQ(38, metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 38 + TESTASSERT_EQ(3 + 5, metrics1.num_rx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs - // SDU metrics - TESTASSERT_EQ(5, metrics1.num_tx_sdus); - TESTASSERT_EQ(0, metrics1.num_rx_sdus); - TESTASSERT_EQ(15, metrics1.num_tx_sdu_bytes); - TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); - TESTASSERT_EQ(0, metrics1.num_lost_sdus); - // PDU metrics - TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions - TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU - TESTASSERT_EQ(38, metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + - // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 38 - TESTASSERT_EQ(3 + 5, metrics1.num_rx_pdu_bytes); // Two status PDU (one with a NACK) - TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs + // PDU metrics + TESTASSERT_EQ(0, metrics2.num_tx_sdus); + TESTASSERT_EQ(5, metrics2.num_rx_sdus); + TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); + TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each + TESTASSERT_EQ(0, metrics2.num_lost_sdus); + // SDU metrics + TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs + TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) + TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 + TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs - // PDU metrics - TESTASSERT_EQ(0, metrics2.num_tx_sdus); - TESTASSERT_EQ(5, metrics2.num_rx_sdus); - TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); - TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each - TESTASSERT_EQ(0, metrics2.num_lost_sdus); - // SDU metrics - TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs - TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) - TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) - TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) - // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 - TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs - - // Check state - rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); - TESTASSERT_EQ(5, state2_rx.rx_next); - */ + // Check state + rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); + TESTASSERT_EQ(5, state2_rx.rx_next); + */ return SRSRAN_SUCCESS; } @@ -801,7 +814,6 @@ int main() // start log back-end srslog::init(); - // TESTASSERT(window_checker_test() == SRSRAN_SUCCESS); // TESTASSERT(basic_test() == SRSRAN_SUCCESS); // TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS); From 525d2db454f6a920edf2692a19f04a036f54607f Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Wed, 2 Feb 2022 17:48:50 +0000 Subject: [PATCH 16/28] lib,rlc_am_nr: re-enable all tests --- lib/test/rlc/rlc_am_nr_test.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index ebb9b5ae0..b91d0e38a 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -814,11 +814,11 @@ int main() // start log back-end srslog::init(); - // TESTASSERT(window_checker_test() == SRSRAN_SUCCESS); - // TESTASSERT(basic_test() == SRSRAN_SUCCESS); - // TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS); - // TESTASSERT(basic_segmentation_test() == SRSRAN_SUCCESS); - // TESTASSERT(segment_retx_test() == SRSRAN_SUCCESS); + TESTASSERT(window_checker_test() == SRSRAN_SUCCESS); + TESTASSERT(basic_test() == SRSRAN_SUCCESS); + TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS); + TESTASSERT(basic_segmentation_test() == SRSRAN_SUCCESS); + TESTASSERT(segment_retx_test() == SRSRAN_SUCCESS); TESTASSERT(retx_segment_test() == SRSRAN_SUCCESS); return SRSRAN_SUCCESS; } From 8c53c74c86470b01677795543d3d6002a693e76c Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Wed, 2 Feb 2022 18:43:19 +0000 Subject: [PATCH 17/28] lib,rlc_am_nr: fix ptr increment when there is no SO when writting status report without SO. --- lib/src/rlc/rlc_am_nr_packing.cc | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/src/rlc/rlc_am_nr_packing.cc b/lib/src/rlc/rlc_am_nr_packing.cc index 031aaa406..ac6303a91 100644 --- a/lib/src/rlc/rlc_am_nr_packing.cc +++ b/lib/src/rlc/rlc_am_nr_packing.cc @@ -240,23 +240,27 @@ int32_t rlc_am_nr_write_status_pdu(const rlc_am_nr_status_pdu_t& status_pdu, ptr++; // write E1 flag in octet 3 - *ptr = (status_pdu.N_nack > 0) ? 0x80 : 0x00; + if (status_pdu.N_nack > 0) { + *ptr = 0x80; + } else { + *ptr = 0x00; + } ptr++; if (status_pdu.N_nack > 0) { for (uint32_t i = 0; i < status_pdu.N_nack; i++) { + // write first 8 bit of NACK_SN + *ptr = (status_pdu.nacks[i].nack_sn >> 4) & 0xff; + ptr++; + + // write remaining 4 bits of NACK_SN + *ptr = (status_pdu.nacks[i].nack_sn & 0x0f) << 4; + // Set E1 if necessary + if (i < (uint32_t)(status_pdu.N_nack - 1)) { + *ptr |= 0x08; + } + if (status_pdu.nacks[i].has_so) { - // write first 8 bit of NACK_SN - *ptr = (status_pdu.nacks[i].nack_sn >> 4) & 0xff; - ptr++; - - // write remaining 4 bits of NACK_SN - *ptr = (status_pdu.nacks[i].nack_sn & 0x0f) << 4; - - // Set E1 if necessary - if (i < (uint32_t)(status_pdu.N_nack - 1)) { - *ptr |= 0x08; - } // Set E2 *ptr |= 0x04; @@ -268,8 +272,8 @@ int32_t rlc_am_nr_write_status_pdu(const rlc_am_nr_status_pdu_t& status_pdu, (*ptr) = status_pdu.nacks[i].so_end >> 8; ptr++; (*ptr) = status_pdu.nacks[i].so_end; - ptr++; } + ptr++; } } } else { From 5e8b7b5ebeeb5143d950d022da85c880654c149a Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Fri, 4 Feb 2022 16:56:35 +0000 Subject: [PATCH 18/28] lib,rlc_am_nr: re-factored code for retx'ing with and without segmentation. --- lib/include/srsran/rlc/rlc_am_data_structs.h | 1 + lib/include/srsran/rlc/rlc_am_nr.h | 4 + lib/src/rlc/rlc_am_nr.cc | 289 +++++++++++++++++-- 3 files changed, 274 insertions(+), 20 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_data_structs.h b/lib/include/srsran/rlc/rlc_am_data_structs.h index a7fa667d0..b23d64cb3 100644 --- a/lib/include/srsran/rlc/rlc_am_data_structs.h +++ b/lib/include/srsran/rlc/rlc_am_data_structs.h @@ -302,6 +302,7 @@ private: struct rlc_amd_retx_t { uint32_t sn; + uint32_t sdu_end; bool is_segment; uint32_t so_start; uint32_t so_end; diff --git a/lib/include/srsran/rlc/rlc_am_nr.h b/lib/include/srsran/rlc/rlc_am_nr.h index be230ed08..3cb6845b1 100644 --- a/lib/include/srsran/rlc/rlc_am_nr.h +++ b/lib/include/srsran/rlc/rlc_am_nr.h @@ -108,10 +108,14 @@ public: uint32_t build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); uint32_t build_continuation_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t nof_bytes); uint32_t build_retx_pdu(uint8_t* payload, uint32_t nof_bytes); + uint32_t build_retx_pdu_without_segmentation(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); + uint32_t build_retx_pdu_with_segmentation(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); uint32_t build_retx_pdu_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); uint32_t build_retx_segment_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); uint32_t build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); uint32_t build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); + bool is_retx_segmentation_required(const rlc_amd_retx_t& retx, uint32_t nof_bytes); + uint32_t get_retx_expected_hdr_len(const rlc_amd_retx_t& retx); /* * uint32_t build_retx_sdu_segment(rlc_amd_retx_t& retx, rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t * nof_bytes); diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 519a4157e..bb8c1cdc5 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -417,17 +417,218 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) } } - RlcDebug("RETX - is_segment=%s, current_so=%d, so_start=%d, so_end=%d", + RlcDebug("RETX - SDU len=%d, is_segment=%s, current_so=%d, so_start=%d, so_end=%d", + retx.sdu_end, retx.is_segment ? "true" : "false", retx.current_so, retx.so_start, retx.so_end); - if (retx.is_segment) { - return build_retx_pdu_from_sdu_segment(retx, payload, nof_bytes); + + // Is segmentation/re-segmentation required? + bool segmentation_required = is_retx_segmentation_required(retx, nof_bytes); + + if (segmentation_required) { + return build_retx_pdu_with_segmentation(retx, payload, nof_bytes); } - return build_retx_pdu_from_full_sdu(retx, payload, nof_bytes); + return build_retx_pdu_without_segmentation(retx, payload, nof_bytes); } +/** + * Builds a retx RLC PDU, without requiring (re-)segmentation. + * + * The RETX PDU may be transporting a full SDU or an SDU segment. + * + * \param [retx] is the retx info contained in the retx_queue. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark this function will not update the SI. This means that if the retx is of the last + * SDU segment, the SI should already be of the `last_segment` type. + */ +uint32_t rlc_am_nr_tx::build_retx_pdu_without_segmentation(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) +{ + srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); + srsran_assert(not is_retx_segmentation_required(retx, nof_bytes), + "Called %s without checking if segmentation was required", + __FUNCTION__); + + // Get tx_pdu info from tx_window + rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; + + // Get expected header and payload len + uint32_t expected_hdr_len = get_retx_expected_hdr_len(retx); + uint32_t retx_payload_len = retx.is_segment ? (retx.so_end - retx.current_so) : tx_window[retx.sn].sdu_buf->N_bytes; + srsran_assert(nof_bytes >= (expected_hdr_len + retx_payload_len), + "Called %s but segmentation is required. nof_bytes=%d, expeced_hdr_len=%d, retx_payload_len=%d", + __FUNCTION__, + nof_bytes, + expected_hdr_len, + retx_payload_len); + + // Update SI, if necessary + rlc_nr_si_field_t si = rlc_nr_si_field_t::full_sdu; + if (retx.is_segment) { + RlcDebug("SDU segment can be fully transmitted. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " + "current_so=%d, so_start=%d, so_end=%d", + retx.sn, + nof_bytes, + expected_hdr_len, + retx.current_so, + retx.so_start, + retx.so_end); + if (retx.current_so == 0) { + si = rlc_nr_si_field_t::first_segment; + } else if ((retx.current_so + retx_payload_len) < tx_pdu.sdu_buf->N_bytes) { + si = rlc_nr_si_field_t::neither_first_nor_last_segment; + } else { + si = rlc_nr_si_field_t::last_segment; + } + } + + // Update & write header + rlc_am_nr_pdu_header_t new_header = tx_pdu.header; + new_header.si = si; + new_header.p = 0; + if (retx.is_segment) { + new_header.so = retx.current_so; + } + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(new_header, payload); + + uint32_t pdu_bytes = 0; + if (not retx.is_segment) { + // RETX full SDU + memcpy(&payload[hdr_len], tx_window[retx.sn].sdu_buf->msg, tx_window[retx.sn].sdu_buf->N_bytes); + pdu_bytes = hdr_len + tx_window[retx.sn].sdu_buf->N_bytes; + } else { + // RETX SDU segment + uint32_t retx_pdu_payload_size = (retx.so_end - retx.current_so); + pdu_bytes = hdr_len + retx_pdu_payload_size; + memcpy(&payload[hdr_len], &tx_pdu.sdu_buf->msg[retx.current_so], retx_pdu_payload_size); + } + + retx_queue.pop(); + RlcHexInfo(tx_window[retx.sn].sdu_buf->msg, + tx_window[retx.sn].sdu_buf->N_bytes, + "Original SDU SN=%d (%d B) (attempt %d/%d)", + retx.sn, + tx_window[retx.sn].sdu_buf->N_bytes, + tx_window[retx.sn].retx_count + 1, + cfg.max_retx_thresh); + RlcHexInfo(payload, nof_bytes, "retx PDU SN=%d (%d B)", retx.sn, nof_bytes); + log_rlc_am_nr_pdu_header_to_string(logger.debug, new_header); + + debug_state(); + return pdu_bytes; +} + +/** + * Builds a retx RLC PDU that requires (re-)segmentation. + * + * \param [tx_pdu] is the tx_pdu info contained in the tx_window. + * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. + * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. + * + * \returns the number of bytes written to the payload buffer. + * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. + */ +uint32_t rlc_am_nr_tx::build_retx_pdu_with_segmentation(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) +{ + // Get tx_pdu info from tx_window + srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); + srsran_assert(is_retx_segmentation_required(retx, nof_bytes), + "Called %s without checking if segmentation was not required", + __FUNCTION__); + + rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; + + // Is this an SDU segment or a full SDU? + if (not retx.is_segment) { + RlcDebug("Creating SDU segment from full SDU. SN=%d Tx SDU (%d B), nof_bytes=%d B ", + retx.sn, + tx_pdu.sdu_buf->N_bytes, + nof_bytes); + + } else { + RlcDebug("Creating SDU segment from SDU segment. SN=%d, current_so=%d, so_start=%d, so_end=%d", + __FUNCTION__, + retx.sn, + retx.current_so, + retx.so_start, + retx.so_end); + } + + uint32_t expected_hdr_len = min_hdr_size; + rlc_nr_si_field_t si = rlc_nr_si_field_t::first_segment; + if (retx.current_so != 0) { + si = rlc_nr_si_field_t::neither_first_nor_last_segment; + expected_hdr_len = max_hdr_size; + } + + // Sanity check: are there enough bytes for header plus data? + if (nof_bytes <= expected_hdr_len) { + RlcError("called %s, but there are not enough bytes for data plus header. SN=%d", __FUNCTION__, retx.sn); + return 0; + } + + // Sanity check: could this have been transmitted without segmentation? + if (nof_bytes > (tx_pdu.sdu_buf->N_bytes + expected_hdr_len)) { + RlcError("called %s, but there are enough bytes to avoid segmentation. SN=%d", __FUNCTION__, retx.sn); + return 0; + } + + // Can the RETX PDU be transmitted in a single PDU? + uint32_t retx_pdu_payload_size = nof_bytes - expected_hdr_len; + + // Write header + rlc_am_nr_pdu_header_t hdr = tx_pdu.header; + hdr.so = retx.current_so; + hdr.si = si; + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); + if (hdr_len >= nof_bytes || hdr_len != expected_hdr_len) { + log_rlc_am_nr_pdu_header_to_string(logger.error, hdr); + RlcError("Error writing AMD PDU header. nof_bytes=%d, hdr_len=%d", nof_bytes, hdr_len); + return 0; + } + log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); + + // Copy SDU segment into payload + srsran_assert((hdr_len + retx_pdu_payload_size) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); + memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); + + // Update retx queue + retx.is_segment = true; + retx.current_so = retx.current_so + retx_pdu_payload_size; + + RlcDebug("Updated RETX info. is_segment=%s, current_so=%d, so_start=%d, so_end=%d", + retx.is_segment ? "true" : "false", + retx.current_so, + retx.so_start, + retx.so_end); + + if (retx.current_so >= tx_pdu.sdu_buf->N_bytes) { + RlcError("Current SO larger or equal to SDU size when creating SDU segment. SN=%d, current SO=%d, SO_start=%d, " + "SO_end=%d", + retx.sn, + retx.current_so, + retx.so_start, + retx.so_end); + return 0; + } + + if (retx.current_so >= retx.so_end) { + RlcError("Current SO larger than SO end. SN=%d, current SO=%d, SO_start=%d, SO_end=%s", + retx.sn, + retx.current_so, + retx.so_start, + retx.so_end); + return 0; + } + + // Update SDU segment info + // TODO + return hdr_len + retx_pdu_payload_size; +} /** * Builds a retx RLC PDU from a full SDU. * @@ -777,6 +978,33 @@ uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, return 0; } +bool rlc_am_nr_tx::is_retx_segmentation_required(const rlc_amd_retx_t& retx, uint32_t nof_bytes) +{ + bool segmentation_required = false; + if (retx.is_segment) { + uint32_t expected_hdr_size = retx.current_so == 0 ? min_hdr_size : max_hdr_size; + if (nof_bytes < ((retx.so_end - retx.current_so) + expected_hdr_size)) { + RlcInfo("Re-segmentation required for RETX. SN=%d", retx.sn); + segmentation_required = true; + } + } else { + if (nof_bytes < (tx_window[retx.sn].sdu_buf->N_bytes + min_hdr_size)) { + RlcInfo("Segmentation required for RETX. SN=%d", retx.sn); + segmentation_required = true; + } + } + return segmentation_required; +} + +uint32_t rlc_am_nr_tx::get_retx_expected_hdr_len(const rlc_amd_retx_t& retx) +{ + uint32_t expected_hdr_len = min_hdr_size; + if (retx.is_segment && retx.current_so != 0) { + expected_hdr_len = max_hdr_size; + } + return expected_hdr_len; +} + uint32_t rlc_am_nr_tx::build_status_pdu(byte_buffer_t* payload, uint32_t nof_bytes) { RlcInfo("generating status PDU. Bytes available:%d", nof_bytes); @@ -817,7 +1045,7 @@ void rlc_am_nr_tx::handle_control_pdu(uint8_t* payload, uint32_t nof_bytes) // Process ACKs uint32_t stop_sn = status.N_nack == 0 ? status.ack_sn - : status.nacks[0].nack_sn - 1; // Stop processing ACKs at the first NACK, if it exists. + : status.nacks[0].nack_sn; // Stop processing ACKs at the first NACK, if it exists. if (stop_sn > st.tx_next) { RlcError("Received ACK or NACK larger than TX_NEXT. Ignoring status report"); return; @@ -836,25 +1064,46 @@ void rlc_am_nr_tx::handle_control_pdu(uint8_t* payload, uint32_t nof_bytes) // Process N_acks for (uint32_t nack_idx = 0; nack_idx < status.N_nack; nack_idx++) { if (st.tx_next_ack <= status.nacks[nack_idx].nack_sn && status.nacks[nack_idx].nack_sn <= st.tx_next) { - uint32_t nack_sn = status.nacks[nack_idx].nack_sn; + auto nack = status.nacks[nack_idx]; + uint32_t nack_sn = nack.nack_sn; if (tx_window.has_sn(nack_sn)) { auto& pdu = tx_window[nack_sn]; - // add to retx queue if it's not already there - if (not retx_queue.has_sn(nack_sn)) { - // increment Retx counter and inform upper layers if needed - pdu.retx_count++; + if (nack.has_so) { + // NACK'ing missing bytes in SDU segment. + // Retransmit all SDU segments within those missing bytes. + if (pdu.segment_list.empty()) { + RlcError("Received NACK with SO, but there is no segment information"); + } + for (std::list::iterator segm = pdu.segment_list.begin(); + segm != pdu.segment_list.end(); + segm++) { + if (segm->so >= nack.so_start && nack.so_end <= (segm->so + segm->payload_len)) { + rlc_amd_retx_t& retx = retx_queue.push(); + retx.sn = nack_sn; + retx.sdu_end = pdu.sdu_buf->N_bytes; + retx.is_segment = true; + retx.so_start = segm->so; + retx.current_so = segm->so; + retx.so_end = segm->so + segm->payload_len; + } + } + } else { + // NACK'ing full SDU. + // add to retx queue if it's not already there + if (not retx_queue.has_sn(nack_sn)) { + // increment Retx counter and inform upper layers if needed + pdu.retx_count++; - // check_sn_reached_max_retx(nack_sn); - rlc_amd_retx_t& retx = retx_queue.push(); - srsran_expect(tx_window[nack_sn].rlc_sn == nack_sn, - "Incorrect RLC SN=%d!=%d being accessed", - tx_window[nack_sn].rlc_sn, - nack_sn); - retx.sn = nack_sn; - retx.is_segment = false; - retx.so_start = 0; - retx.so_end = pdu.sdu_buf->N_bytes; + // check_sn_reached_max_retx(nack_sn); + rlc_amd_retx_t& retx = retx_queue.push(); + retx.sn = nack_sn; + retx.sdu_end = pdu.sdu_buf->N_bytes; + retx.is_segment = false; + retx.so_start = 0; + retx.current_so = 0; + retx.so_end = pdu.sdu_buf->N_bytes; + } } } } From 4bbbc8ffde0ff52d2268798f83bc2f136731447a Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Fri, 4 Feb 2022 17:14:21 +0000 Subject: [PATCH 19/28] lib,rlc_am_nr: fixed incorrectly adding to many segments when receiving a status report with an SO. --- lib/src/rlc/rlc_am_nr.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index bb8c1cdc5..6fd9404ae 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -1078,7 +1078,7 @@ void rlc_am_nr_tx::handle_control_pdu(uint8_t* payload, uint32_t nof_bytes) for (std::list::iterator segm = pdu.segment_list.begin(); segm != pdu.segment_list.end(); segm++) { - if (segm->so >= nack.so_start && nack.so_end <= (segm->so + segm->payload_len)) { + if (segm->so >= nack.so_start && segm->so < nack.so_end) { rlc_amd_retx_t& retx = retx_queue.push(); retx.sn = nack_sn; retx.sdu_end = pdu.sdu_buf->N_bytes; From 9bb3b1f18fc0d84fca5a34bb73d1d4645480c8dd Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Mon, 7 Feb 2022 11:09:41 +0000 Subject: [PATCH 20/28] lib,rlc_am_nr: remove unused code after re-factor --- lib/include/srsran/rlc/rlc_am_nr.h | 4 - lib/src/rlc/rlc_am_nr.cc | 348 ----------------------------- 2 files changed, 352 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_nr.h b/lib/include/srsran/rlc/rlc_am_nr.h index 3cb6845b1..0f042ab73 100644 --- a/lib/include/srsran/rlc/rlc_am_nr.h +++ b/lib/include/srsran/rlc/rlc_am_nr.h @@ -110,10 +110,6 @@ public: uint32_t build_retx_pdu(uint8_t* payload, uint32_t nof_bytes); uint32_t build_retx_pdu_without_segmentation(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); uint32_t build_retx_pdu_with_segmentation(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); - uint32_t build_retx_pdu_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); - uint32_t build_retx_segment_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); - uint32_t build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); - uint32_t build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); bool is_retx_segmentation_required(const rlc_amd_retx_t& retx, uint32_t nof_bytes); uint32_t get_retx_expected_hdr_len(const rlc_amd_retx_t& retx); /* diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 6fd9404ae..62ffa9958 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -629,354 +629,6 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_with_segmentation(rlc_amd_retx_t& retx, ui // TODO return hdr_len + retx_pdu_payload_size; } -/** - * Builds a retx RLC PDU from a full SDU. - * - * The RETX of the full SDU may be further segmented if necessary. - * - * \param [tx_pdu] is the tx_pdu info contained in the tx_window. - * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. - * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. - * - * \returns the number of bytes written to the payload buffer. - * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. - */ -uint32_t rlc_am_nr_tx::build_retx_pdu_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) -{ - srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); - - // Get tx_pdu info from tx_window - rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; - // Update & write header - rlc_am_nr_pdu_header_t new_header = tx_pdu.header; - new_header.p = 0; - uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(new_header, payload); - - // Check if we exceed allocated number of bytes - if (hdr_len + tx_window[retx.sn].sdu_buf->N_bytes > nof_bytes) { - RlcInfo("Trying to segment retx PDU. SN=%d", retx.sn); - log_rlc_am_nr_pdu_header_to_string(logger.debug, new_header); - return build_retx_segment_from_full_sdu(retx, payload, nof_bytes); - } - - // RETX full PDU - memcpy(&payload[hdr_len], tx_window[retx.sn].sdu_buf->msg, tx_window[retx.sn].sdu_buf->N_bytes); - uint32_t pdu_bytes = hdr_len + tx_window[retx.sn].sdu_buf->N_bytes; - - retx_queue.pop(); - RlcHexInfo(tx_window[retx.sn].sdu_buf->msg, - tx_window[retx.sn].sdu_buf->N_bytes, - "Original SDU SN=%d (%d B) (attempt %d/%d)", - retx.sn, - tx_window[retx.sn].sdu_buf->N_bytes, - tx_window[retx.sn].retx_count + 1, - cfg.max_retx_thresh); - RlcHexInfo(payload, pdu_bytes, "retx PDU SN=%d (%d B)", retx.sn, pdu_bytes); - log_rlc_am_nr_pdu_header_to_string(logger.debug, new_header); - - debug_state(); - return pdu_bytes; -} - -/** - * Builds a retx RLC PDU from a full SDU. - * - * This function will further segment the previously transmitted SDU. - * There should not be enough bytes granted to transmit the previous - * SDU full; this should have been checked previous to calling this - * function. - * - * \param [tx_pdu] is the tx_pdu info contained in the tx_window. - * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. - * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. - * - * \returns the number of bytes written to the payload buffer. - * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. - */ -uint32_t rlc_am_nr_tx::build_retx_segment_from_full_sdu(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) -{ - // Get tx_pdu info from tx_window - srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); - rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; - - RlcInfo("creating SDU segment from full SDU. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); - - // Sanity check: is this an SDU segment retx? - if (retx.is_segment) { - RlcError("called %s, but retx contains a SDU segment. SN=%d, current_so=%d, so_start=%d, so_end=%d", - __FUNCTION__, - retx.sn, - retx.current_so, - retx.so_start, - retx.so_end); - return 0; - } - - // Sanity check: are there enough bytes for header plus data? - if (nof_bytes <= min_hdr_size) { - RlcError("called %s, but there are not enough bytes for data plus header. SN=%d", __FUNCTION__, retx.sn); - return 0; - } - - // Sanity check: could this have been transmitted without segmentation? - if (nof_bytes > (tx_pdu.sdu_buf->N_bytes + min_hdr_size)) { - RlcError("called %s, but there are enough bytes to avoid segmentation. SN=%d", __FUNCTION__, retx.sn); - return 0; - } - - // Can the RETX PDU be transmitted in a single PDU? - uint32_t retx_pdu_payload_size = nof_bytes - min_hdr_size; - - // Write header - rlc_am_nr_pdu_header_t hdr = tx_pdu.header; - hdr.si = rlc_nr_si_field_t::first_segment; - uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); - if (hdr_len >= nof_bytes || hdr_len != min_hdr_size) { - RlcError("error writing AMD PDU header"); - return 0; - } - log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); - - // Copy PDU to payload - srsran_assert((hdr_len + retx_pdu_payload_size) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); - memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); - - // Store Segment Info - rlc_amd_tx_pdu_nr::pdu_segment segment_info; - segment_info.payload_len = retx_pdu_payload_size; - tx_pdu.segment_list.push_back(segment_info); - - // Update retx queue. Next SDU segment will be the rest of the PDU - retx.is_segment = true; - retx.current_so = retx_pdu_payload_size; - - RlcDebug("Updated RETX info. is_segment=%s, current_so=%d, so_start=%d, so_end=%d", - retx.is_segment ? "true" : "false", - retx.current_so, - retx.so_start, - retx.so_end); - - return hdr_len + retx_pdu_payload_size; -} - -/** - * Builds a retx RLC PDU from an RETX SDU segment. - * - * The - * \param [tx_pdu] is the tx_pdu info contained in the tx_window. - * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. - * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. - * - * \returns the number of bytes written to the payload buffer. - * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. - */ -uint32_t rlc_am_nr_tx::build_retx_pdu_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) -{ - // Get tx_pdu info from tx_window - srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); - rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; - - RlcInfo("Creating RETX PDU from SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); - - // Sanity check: is this an SDU segment retx? - if (not retx.is_segment) { - RlcError("Called %s, but retx contains a SDU segment. SN=%d, current_so=%d, so_start=%d, current_so=%d, so_end=%d", - __FUNCTION__, - retx.sn, - retx.current_so, - retx.so_start, - retx.current_so, - retx.so_end); - return 0; - } - - uint32_t expected_hdr_len = min_hdr_size; - rlc_nr_si_field_t si = rlc_nr_si_field_t::first_segment; - if (retx.current_so != 0) { - si = rlc_nr_si_field_t::neither_first_nor_last_segment; - expected_hdr_len = max_hdr_size; - } - - // Sanity check: are there enough bytes for header plus data? - if (nof_bytes <= expected_hdr_len) { - RlcError("Called %s, but there are not enough bytes for data plus header. SN=%d, nof_bytes=%d, expected_hdr_len=%d", - __FUNCTION__, - retx.sn, - nof_bytes, - expected_hdr_len); - return 0; - } - - // Can be transmitted without segmentation? - if (nof_bytes < (retx.so_end - retx.current_so) + expected_hdr_len) { - RlcDebug("It is necessary to further segment the SDU segment. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " - "current_so=%d, so_start=%d, so_end=%d", - retx.sn, - nof_bytes, - expected_hdr_len, - retx.current_so, - retx.so_start, - retx.so_end); - return build_retx_segment_from_sdu_segment(retx, payload, nof_bytes); - } else { - RlcDebug("SDU segment can be fully transmitted. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " - "current_so=%d, so_start=%d, so_end=%d", - retx.sn, - nof_bytes, - expected_hdr_len, - retx.current_so, - retx.so_start, - retx.so_end); - si = rlc_nr_si_field_t::last_segment; - } - - uint32_t retx_pdu_payload_size = (retx.so_end - retx.current_so); - - // Write header - rlc_am_nr_pdu_header_t hdr = tx_pdu.header; - hdr.si = si; - hdr.so = retx.current_so; - uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); - if (hdr_len >= nof_bytes || hdr_len != expected_hdr_len) { - RlcError("Error writing AMD PDU header. nof_bytes=%d, hdr_len=%d", nof_bytes, hdr_len); - return 0; - } - log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); - - // Copy SDU segment into payload - srsran_assert((hdr_len + retx_pdu_payload_size) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); - memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); - - // Update retx queue - retx.current_so = retx.current_so + retx_pdu_payload_size; - - RlcDebug("Updated RETX info. is_segment=%s, current_so=%d, so_start=%d, so_end=%d", - retx.is_segment ? "true" : "false", - retx.current_so, - retx.so_start, - retx.so_end); - - if (retx.current_so == tx_pdu.sdu_buf->N_bytes) { - // Last segment to retx, remove retx from queue - retx_queue.pop(); - } else if (retx.current_so > tx_pdu.sdu_buf->N_bytes) { - RlcError("Current SO larger than SDU size. SO_end=%d", retx.so_end); - } - - if (retx.current_so > retx.so_end) { - RlcError("SO end larger than SO start. current_so=%d, SO_start=%d, SO_end=%d", - retx.current_so, - retx.so_start, - retx.so_end); - return 0; - } - - // Update SDU segment info - // TODO - return hdr_len + retx_pdu_payload_size; -} - -/** - * Builds a retx RLC PDU from an SDU SDU segment. - * - * \param [tx_pdu] is the tx_pdu info contained in the tx_window. - * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. - * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. - * - * \returns the number of bytes written to the payload buffer. - * \remark: This functions assumes that the SDU has already been copied to tx_pdu.sdu_buf. - */ -uint32_t rlc_am_nr_tx::build_retx_segment_from_sdu_segment(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes) -{ - // Get tx_pdu info from tx_window - srsran_assert(tx_window.has_sn(retx.sn), "Called %s without checking retx SN", __FUNCTION__); - rlc_amd_tx_pdu_nr& tx_pdu = tx_window[retx.sn]; - - RlcDebug("Creating SDU segment from SDU segment. Tx SDU (%d B), nof_bytes=%d B ", tx_pdu.sdu_buf->N_bytes, nof_bytes); - - // Sanity check: is this an SDU segment retx? - if (not retx.is_segment) { - RlcError("called %s, but retx contains a SDU segment. SN=%d, current_so=%d, so_start=%d, so_end=%d", - __FUNCTION__, - retx.sn, - retx.current_so, - retx.so_start, - retx.so_end); - return 0; - } - - uint32_t expected_hdr_len = min_hdr_size; - rlc_nr_si_field_t si = rlc_nr_si_field_t::first_segment; - if (retx.current_so != 0) { - si = rlc_nr_si_field_t::neither_first_nor_last_segment; - expected_hdr_len = max_hdr_size; - } - - // Sanity check: are there enough bytes for header plus data? - if (nof_bytes <= expected_hdr_len) { - RlcError("called %s, but there are not enough bytes for data plus header. SN=%d", __FUNCTION__, retx.sn); - return 0; - } - - // Sanity check: could this have been transmitted without segmentation? - if (nof_bytes > (tx_pdu.sdu_buf->N_bytes + expected_hdr_len)) { - RlcError("called %s, but there are enough bytes to avoid segmentation. SN=%d", __FUNCTION__, retx.sn); - return 0; - } - - // Can the RETX PDU be transmitted in a single PDU? - uint32_t retx_pdu_payload_size = nof_bytes - expected_hdr_len; - - // Write header - rlc_am_nr_pdu_header_t hdr = tx_pdu.header; - hdr.so = retx.current_so; - hdr.si = si; - uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(hdr, payload); - if (hdr_len >= nof_bytes || hdr_len != expected_hdr_len) { - log_rlc_am_nr_pdu_header_to_string(logger.error, hdr); - RlcError("Error writing AMD PDU header. nof_bytes=%d, hdr_len=%d", nof_bytes, hdr_len); - return 0; - } - log_rlc_am_nr_pdu_header_to_string(logger.info, hdr); - - // Copy SDU segment into payload - srsran_assert((hdr_len + retx_pdu_payload_size) <= nof_bytes, "Error calculating hdr_len and segment_payload_len"); - memcpy(&payload[hdr_len], tx_pdu.sdu_buf->msg, retx_pdu_payload_size); - - // Update retx queue - retx.current_so = retx.current_so + retx_pdu_payload_size; - - RlcDebug("Updated RETX info. is_segment=%s, current_so=%d, so_start=%d, so_end=%d", - retx.is_segment ? "true" : "false", - retx.current_so, - retx.so_start, - retx.so_end); - - if (retx.current_so >= tx_pdu.sdu_buf->N_bytes) { - RlcError("Current SO larger or equal to SDU size when creating SDU segment. SN=%d, current SO=%d, SO_start=%d, " - "SO_end=%d", - retx.sn, - retx.current_so, - retx.so_start, - retx.so_end); - return 0; - } - - if (retx.current_so >= retx.so_end) { - RlcError("Current SO larger than SO end. SN=%d, current SO=%d, SO_start=%d, SO_end=%s", - retx.sn, - retx.current_so, - retx.so_start, - retx.so_end); - return 0; - } - - // Update SDU segment info - // TODO - return hdr_len + retx_pdu_payload_size; - - return 0; -} bool rlc_am_nr_tx::is_retx_segmentation_required(const rlc_amd_retx_t& retx, uint32_t nof_bytes) { From 832d65057498f0ef0999bf52af4ef724de63d3a2 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Mon, 7 Feb 2022 11:57:35 +0000 Subject: [PATCH 21/28] lib,rlc_am_nr: remove unused variables. Fixed up some comments. --- lib/include/srsran/rlc/rlc_am_data_structs.h | 1 - lib/include/srsran/rlc/rlc_am_nr.h | 8 ++------ lib/src/rlc/rlc_am_nr.cc | 19 ++++++------------- 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_data_structs.h b/lib/include/srsran/rlc/rlc_am_data_structs.h index b23d64cb3..a7fa667d0 100644 --- a/lib/include/srsran/rlc/rlc_am_data_structs.h +++ b/lib/include/srsran/rlc/rlc_am_data_structs.h @@ -302,7 +302,6 @@ private: struct rlc_amd_retx_t { uint32_t sn; - uint32_t sdu_end; bool is_segment; uint32_t so_start; uint32_t so_end; diff --git a/lib/include/srsran/rlc/rlc_am_nr.h b/lib/include/srsran/rlc/rlc_am_nr.h index 0f042ab73..d06aa943e 100644 --- a/lib/include/srsran/rlc/rlc_am_nr.h +++ b/lib/include/srsran/rlc/rlc_am_nr.h @@ -112,10 +112,7 @@ public: uint32_t build_retx_pdu_with_segmentation(rlc_amd_retx_t& retx, uint8_t* payload, uint32_t nof_bytes); bool is_retx_segmentation_required(const rlc_amd_retx_t& retx, uint32_t nof_bytes); uint32_t get_retx_expected_hdr_len(const rlc_amd_retx_t& retx); - /* - * uint32_t build_retx_sdu_segment(rlc_amd_retx_t& retx, rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* payload, uint32_t - * nof_bytes); - */ + // Buffer State bool has_data() final; uint32_t get_buffer_state() final; @@ -154,8 +151,7 @@ private: // Queues and buffers pdu_retx_queue retx_queue; - uint32_t sdu_under_segmentation_sn = INVALID_RLC_SN; // SN of the SDU currently being segmented. - uint32_t retx_under_segmentation_sn = INVALID_RLC_SN; // SN of RETX currently being segmented. + uint32_t sdu_under_segmentation_sn = INVALID_RLC_SN; // SN of the SDU currently being segmented. // Helper constants uint32_t min_hdr_size = 2; diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 62ffa9958..961610194 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -72,7 +72,7 @@ bool rlc_am_nr_tx::has_data() /** * Builds the RLC PDU. * - * Called by the MAC, trough the STACK thread. + * Called by the MAC, trough one of the PHY worker threads. * * \param [payload] is a pointer to the buffer that will hold the PDU. * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. @@ -131,14 +131,13 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes) } /** - * Builds a new RLC PDU, which contains the full SDU. + * Builds a new RLC PDU. * - * Called by the MAC, trough the STACK thread. * This will be called after checking whether control, retransmission, * or segment PDUs needed to be transmitted first. * * This will read an SDU from the SDU queue, build a new PDU, and add it to the tx_window. - * Segmentation will be done if necessary. + * SDU segmentation will be done if necessary. * * \param [payload] is a pointer to the buffer that will hold the PDU. * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. @@ -206,9 +205,7 @@ uint32_t rlc_am_nr_tx::build_new_pdu(uint8_t* payload, uint32_t nof_bytes) } /** - * Builds a new RLC PDU segment. - * - * Called by the MAC, trough the STACK thread. + * Builds a new RLC PDU segment, from a RLC SDU. * * \param [tx_pdu] is the tx_pdu info contained in the tx_window. * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. @@ -273,8 +270,6 @@ uint32_t rlc_am_nr_tx::build_new_sdu_segment(rlc_amd_tx_pdu_nr& tx_pdu, uint8_t* /** * Build PDU segment for an RLC SDU that is already on-going segmentation. * - * Called by the MAC, trough the STACK thread. - * * \param [tx_pdu] is the tx_pdu info contained in the tx_window. * \param [payload] is a pointer to the MAC buffer that will hold the PDU segment. * \param [nof_bytes] is the number of bytes the RLC is allowed to fill. @@ -417,8 +412,8 @@ uint32_t rlc_am_nr_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_bytes) } } - RlcDebug("RETX - SDU len=%d, is_segment=%s, current_so=%d, so_start=%d, so_end=%d", - retx.sdu_end, + RlcDebug("RETX - SN=%d, is_segment=%s, current_so=%d, so_start=%d, so_end=%d", + retx.sn, retx.is_segment ? "true" : "false", retx.current_so, retx.so_start, @@ -733,7 +728,6 @@ void rlc_am_nr_tx::handle_control_pdu(uint8_t* payload, uint32_t nof_bytes) if (segm->so >= nack.so_start && segm->so < nack.so_end) { rlc_amd_retx_t& retx = retx_queue.push(); retx.sn = nack_sn; - retx.sdu_end = pdu.sdu_buf->N_bytes; retx.is_segment = true; retx.so_start = segm->so; retx.current_so = segm->so; @@ -750,7 +744,6 @@ void rlc_am_nr_tx::handle_control_pdu(uint8_t* payload, uint32_t nof_bytes) // check_sn_reached_max_retx(nack_sn); rlc_amd_retx_t& retx = retx_queue.push(); retx.sn = nack_sn; - retx.sdu_end = pdu.sdu_buf->N_bytes; retx.is_segment = false; retx.so_start = 0; retx.current_so = 0; From 3b9ad84bdaddff5e54251d0fa0db631aedd74f60 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Mon, 7 Feb 2022 16:38:35 +0000 Subject: [PATCH 22/28] lib,rlc_am_nr: fix checking some statistics in unit test --- lib/test/rlc/rlc_am_nr_test.cc | 73 ++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index b91d0e38a..f500a28a8 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -750,43 +750,46 @@ int retx_segment_test() } TESTASSERT(0 == rlc1.get_buffer_state()); } + + // Check statistics + rlc_bearer_metrics_t metrics1 = rlc1.get_metrics(); + rlc_bearer_metrics_t metrics2 = rlc2.get_metrics(); + + // SDU metrics + TESTASSERT_EQ(5, metrics1.num_tx_sdus); + TESTASSERT_EQ(0, metrics1.num_rx_sdus); + TESTASSERT_EQ(15, metrics1.num_tx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); + TESTASSERT_EQ(0, metrics1.num_lost_sdus); + + // PDU metrics + TESTASSERT_EQ(15 + 3, metrics1.num_tx_pdus); // 15 PDUs + 3 re-transmissions + TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU + TESTASSERT_EQ(78, metrics1.num_tx_pdu_bytes); // 3 Bytes * 5 (5 PDUs without SO) + 10 * 5 (10 PDUs with SO) + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 78 + TESTASSERT_EQ(24, metrics1.num_rx_pdu_bytes); // Two status PDU. One with just an ack (3 bytes) + // Another with 3 NACKs all with SO. (3 + 3*6 bytes) + TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs + + // PDU metrics + TESTASSERT_EQ(0, metrics2.num_tx_sdus); + TESTASSERT_EQ(5, metrics2.num_rx_sdus); /* - // Check statistics - rlc_bearer_metrics_t metrics1 = rlc1.get_metrics(); - rlc_bearer_metrics_t metrics2 = rlc2.get_metrics(); + TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); + TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each + TESTASSERT_EQ(0, metrics2.num_lost_sdus); + // SDU metrics + TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs + TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) + TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) + TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) + // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 + TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs - // SDU metrics - TESTASSERT_EQ(5, metrics1.num_tx_sdus); - TESTASSERT_EQ(0, metrics1.num_rx_sdus); - TESTASSERT_EQ(15, metrics1.num_tx_sdu_bytes); - TESTASSERT_EQ(0, metrics1.num_rx_sdu_bytes); - TESTASSERT_EQ(0, metrics1.num_lost_sdus); - // PDU metrics - TESTASSERT_EQ(5 + 3, metrics1.num_tx_pdus); // 3 re-transmissions - TESTASSERT_EQ(2, metrics1.num_rx_pdus); // Two status PDU - TESTASSERT_EQ(38, metrics1.num_tx_pdu_bytes); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) + - // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 38 - TESTASSERT_EQ(3 + 5, metrics1.num_rx_pdu_bytes); // Two status PDU (one with a NACK) - TESTASSERT_EQ(0, metrics1.num_lost_sdus); // No lost SDUs - - // PDU metrics - TESTASSERT_EQ(0, metrics2.num_tx_sdus); - TESTASSERT_EQ(5, metrics2.num_rx_sdus); - TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); - TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each - TESTASSERT_EQ(0, metrics2.num_lost_sdus); - // SDU metrics - TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs - TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) - TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) - TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) - // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 - TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs - - // Check state - rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); - TESTASSERT_EQ(5, state2_rx.rx_next); - */ + // Check state + rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); + TESTASSERT_EQ(5, state2_rx.rx_next); + */ return SRSRAN_SUCCESS; } From 634c9ea3af436ea77cac6dbdbf23ea5e4db774ce Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Mon, 7 Feb 2022 17:51:55 +0000 Subject: [PATCH 23/28] lib,rlc_am_nr: starting to order segments to be able to receive them out-of-order --- lib/include/srsran/rlc/rlc_am_nr.h | 4 +++- lib/include/srsran/rlc/rlc_am_nr_packing.h | 13 +++++++++---- lib/src/rlc/rlc_am_nr.cc | 15 +++++++++++---- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_nr.h b/lib/include/srsran/rlc/rlc_am_nr.h index d06aa943e..cc336cbe5 100644 --- a/lib/include/srsran/rlc/rlc_am_nr.h +++ b/lib/include/srsran/rlc/rlc_am_nr.h @@ -221,7 +221,9 @@ public: int handle_segment_data_sdu(const rlc_am_nr_pdu_header_t& header, const uint8_t* payload, uint32_t nof_bytes); bool inside_rx_window(uint32_t sn); void write_to_upper_layers(uint32_t lcid, unique_byte_buffer_t sdu); - bool have_all_segments_been_received(const std::list& segment_list); + void insert_received_segment(rlc_amd_rx_pdu_nr segment, + std::set& segment_list); + bool have_all_segments_been_received(const std::set& segment_list); // Metrics uint32_t get_sdu_rx_latency_ms() final; diff --git a/lib/include/srsran/rlc/rlc_am_nr_packing.h b/lib/include/srsran/rlc/rlc_am_nr_packing.h index 44592755d..f75c60e30 100644 --- a/lib/include/srsran/rlc/rlc_am_nr_packing.h +++ b/lib/include/srsran/rlc/rlc_am_nr_packing.h @@ -15,6 +15,7 @@ #include "srsran/common/string_helpers.h" #include "srsran/rlc/rlc_am_base.h" +#include namespace srsran { @@ -52,11 +53,15 @@ struct rlc_amd_rx_pdu_nr { explicit rlc_amd_rx_pdu_nr(uint32_t rlc_sn_) : rlc_sn(rlc_sn_) {} }; +struct rlc_amd_rx_pdu_nr_cmp { + bool operator()(const rlc_amd_rx_pdu_nr& a, const rlc_amd_rx_pdu_nr& b) const { return a.header.so < b.header.so; } +}; + struct rlc_amd_rx_sdu_nr_t { - uint32_t rlc_sn = 0; - bool fully_received = false; - unique_byte_buffer_t buf; - std::list segments; + uint32_t rlc_sn = 0; + bool fully_received = false; + unique_byte_buffer_t buf; + std::set segments; rlc_amd_rx_sdu_nr_t() = default; explicit rlc_amd_rx_sdu_nr_t(uint32_t rlc_sn_) : rlc_sn(rlc_sn_) {} diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 961610194..b86821bc0 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -1112,8 +1112,8 @@ int rlc_am_nr_rx::handle_segment_data_sdu(const rlc_am_nr_pdu_header_t& header, memcpy(pdu_segment.buf->msg, payload + hdr_len, nof_bytes - hdr_len); // Don't copy header pdu_segment.buf->N_bytes = nof_bytes - hdr_len; - // Store SDU segment. TODO sort by SO and check for duplicate bytes. - rx_sdu.segments.push_back(std::move(pdu_segment)); + // Store SDU segment. Sort by SO and check for duplicate bytes. + insert_received_segment(std::move(pdu_segment), rx_sdu.segments); // Check weather all segments have been received rx_sdu.fully_received = have_all_segments_been_received(rx_sdu.segments); @@ -1301,14 +1301,21 @@ uint32_t rlc_am_nr_rx::get_rx_buffered_bytes() return 0; } -bool rlc_am_nr_rx::have_all_segments_been_received(const std::list& segment_list) +void rlc_am_nr_rx::insert_received_segment(rlc_amd_rx_pdu_nr segment, + std::set& segment_list) +{ + segment_list.insert(std::move(segment)); +} + +bool rlc_am_nr_rx::have_all_segments_been_received( + const std::set& segment_list) { if (segment_list.empty()) { return false; } // Check if we have received the last segment - if ((--segment_list.end())->header.si != rlc_nr_si_field_t::last_segment) { + if (segment_list.rbegin()->header.si != rlc_nr_si_field_t::last_segment) { return false; } From 871142b722a8262328b7a3c531eda723dbc1cb83 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Tue, 8 Feb 2022 15:05:25 +0000 Subject: [PATCH 24/28] lib,rlc_am_nr: fix up the last metric checks in the retx_segment_test() --- lib/test/rlc/rlc_am_nr_test.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index f500a28a8..4443a7797 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -774,22 +774,23 @@ int retx_segment_test() // PDU metrics TESTASSERT_EQ(0, metrics2.num_tx_sdus); TESTASSERT_EQ(5, metrics2.num_rx_sdus); - /* TESTASSERT_EQ(0, metrics2.num_tx_sdu_bytes); TESTASSERT_EQ(15, metrics2.num_rx_sdu_bytes); // 5 SDUs, 3 bytes each TESTASSERT_EQ(0, metrics2.num_lost_sdus); // SDU metrics - TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs - TESTASSERT_EQ(7, metrics2.num_rx_pdus); // 7 PDUs (8 tx'ed, but one was lost) - TESTASSERT_EQ(5 + 3, metrics2.num_tx_pdu_bytes); // Two status PDU (one with a NACK) - TESTASSERT_EQ(33, metrics2.num_rx_pdu_bytes); // 2 Bytes * (NBUFFS-1) (header size) + (NBUFFS-1) * 3 (data) - // 3 (1 retx no SO) + 2 * 5 (2 retx with SO) = 33 - TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs + TESTASSERT_EQ(2, metrics2.num_tx_pdus); // Two status PDUs + TESTASSERT_EQ(15, metrics2.num_rx_pdus); // 15 PDUs (18 tx'ed, but three were lost) + TESTASSERT_EQ(24, metrics2.num_tx_pdu_bytes); // Two status PDU. One with just an ack (3 bytes) + // Another with 3 NACKs all with SO. (3 + 3*6 bytes) + TESTASSERT_EQ(65, metrics2.num_rx_pdu_bytes); // 3 Bytes (header + data size, without SO) * 5 (N PDUs without SO) + // 5 bytes (header + data size, with SO) * 10 (N PDUs with SO) + // = 81 bytes + TESTASSERT_EQ(0, metrics2.num_lost_sdus); // No lost SDUs // Check state rlc_am_nr_rx_state_t state2_rx = rx2->get_rx_state(); TESTASSERT_EQ(5, state2_rx.rx_next); - */ + return SRSRAN_SUCCESS; } From c47be649f4adbfba12d035ba156a0f6dd30a34cb Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Wed, 9 Feb 2022 11:17:10 +0100 Subject: [PATCH 25/28] lib,rlc_am_nr: added assert to double check nof_bytes before memcopying into the payload on build_retx_pdu_without_segmentation() --- lib/src/rlc/rlc_am_nr.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index b86821bc0..0364bff18 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -490,17 +490,19 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_without_segmentation(rlc_amd_retx_t& retx, } uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(new_header, payload); - uint32_t pdu_bytes = 0; + uint32_t pdu_bytes = 0; + uint32_t retx_pdu_payload_size = 0; if (not retx.is_segment) { // RETX full SDU - memcpy(&payload[hdr_len], tx_window[retx.sn].sdu_buf->msg, tx_window[retx.sn].sdu_buf->N_bytes); - pdu_bytes = hdr_len + tx_window[retx.sn].sdu_buf->N_bytes; + retx_pdu_payload_size = tx_window[retx.sn].sdu_buf->N_bytes; + pdu_bytes = hdr_len + tx_window[retx.sn].sdu_buf->N_bytes; } else { // RETX SDU segment uint32_t retx_pdu_payload_size = (retx.so_end - retx.current_so); pdu_bytes = hdr_len + retx_pdu_payload_size; - memcpy(&payload[hdr_len], &tx_pdu.sdu_buf->msg[retx.current_so], retx_pdu_payload_size); } + srsran_assert(pdu_bytes <= nof_bytes, "Error calculating hdr_len and pdu_payload_len"); + memcpy(&payload[hdr_len], &tx_pdu.sdu_buf->msg[retx.current_so], retx_pdu_payload_size); retx_queue.pop(); RlcHexInfo(tx_window[retx.sn].sdu_buf->msg, From c8d15135c6fd033ae68ad5cd33051166f4d5e4d1 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Mon, 14 Feb 2022 11:13:44 +0000 Subject: [PATCH 26/28] lib,rlc_am_nr: addressing reveier comments: * made some methods const * changed from TESTASSERT to TESTASSERT_EQ * fix up comment * removed superfulous if * used typedef for list of segments received * added unit test for is_retx_required() * added unit test for malformed status PDU --- lib/include/srsran/rlc/rlc_am_nr.h | 11 +- lib/include/srsran/rlc/rlc_am_nr_packing.h | 9 +- lib/src/rlc/rlc_am_nr.cc | 50 ++--- lib/src/rlc/rlc_am_nr_packing.cc | 4 + lib/test/rlc/rlc_am_nr_pdu_test.cc | 66 +++++++ lib/test/rlc/rlc_am_nr_test.cc | 214 ++++++++++++++------- 6 files changed, 254 insertions(+), 100 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_am_nr.h b/lib/include/srsran/rlc/rlc_am_nr.h index cc336cbe5..fc9172910 100644 --- a/lib/include/srsran/rlc/rlc_am_nr.h +++ b/lib/include/srsran/rlc/rlc_am_nr.h @@ -127,7 +127,7 @@ public: void stop() final; - bool inside_tx_window(uint32_t sn); + bool inside_tx_window(uint32_t sn) const; private: rlc_am* parent = nullptr; @@ -165,7 +165,7 @@ public: uint32_t get_tx_window_size() { return tx_window.size(); } // This should only be used for testing. // Debug Helper - void debug_state(); + void debug_state() const; }; /**************************************************************************** @@ -221,9 +221,8 @@ public: int handle_segment_data_sdu(const rlc_am_nr_pdu_header_t& header, const uint8_t* payload, uint32_t nof_bytes); bool inside_rx_window(uint32_t sn); void write_to_upper_layers(uint32_t lcid, unique_byte_buffer_t sdu); - void insert_received_segment(rlc_amd_rx_pdu_nr segment, - std::set& segment_list); - bool have_all_segments_been_received(const std::set& segment_list); + void insert_received_segment(rlc_amd_rx_pdu_nr segment, rlc_amd_rx_sdu_nr_t::segment_list_t& segment_list) const; + bool have_all_segments_been_received(const rlc_amd_rx_sdu_nr_t::segment_list_t& segment_list) const; // Metrics uint32_t get_sdu_rx_latency_ms() final; @@ -233,7 +232,7 @@ public: void timer_expired(uint32_t timeout_id); // Helpers - void debug_state(); + void debug_state() const; private: rlc_am* parent = nullptr; diff --git a/lib/include/srsran/rlc/rlc_am_nr_packing.h b/lib/include/srsran/rlc/rlc_am_nr_packing.h index f75c60e30..adf6eb6ca 100644 --- a/lib/include/srsran/rlc/rlc_am_nr_packing.h +++ b/lib/include/srsran/rlc/rlc_am_nr_packing.h @@ -58,10 +58,11 @@ struct rlc_amd_rx_pdu_nr_cmp { }; struct rlc_amd_rx_sdu_nr_t { - uint32_t rlc_sn = 0; - bool fully_received = false; - unique_byte_buffer_t buf; - std::set segments; + uint32_t rlc_sn = 0; + bool fully_received = false; + unique_byte_buffer_t buf; + using segment_list_t = std::set; + segment_list_t segments; rlc_amd_rx_sdu_nr_t() = default; explicit rlc_amd_rx_sdu_nr_t(uint32_t rlc_sn_) : rlc_sn(rlc_sn_) {} diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 0364bff18..8b06b7dee 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -169,7 +169,7 @@ uint32_t rlc_am_nr_tx::build_new_pdu(uint8_t* payload, uint32_t nof_bytes) return 0; } - // Copy SDU into PDU info + // Copy SDU into TX window SDU info memcpy(tx_pdu.sdu_buf->msg, tx_sdu->msg, tx_sdu->N_bytes); tx_pdu.sdu_buf->N_bytes = tx_sdu->N_bytes; @@ -461,17 +461,21 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_without_segmentation(rlc_amd_retx_t& retx, expected_hdr_len, retx_payload_len); - // Update SI, if necessary - rlc_nr_si_field_t si = rlc_nr_si_field_t::full_sdu; + // Log RETX info + RlcDebug("SDU%scan be fully re-transmitted. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " + "current_so=%d, so_start=%d, so_end=%d", + retx.is_segment ? " segment " : " ", + retx.sn, + nof_bytes, + expected_hdr_len, + retx.current_so, + retx.so_start, + retx.so_end); + + // Update & write header + uint32_t current_so = 0; + rlc_nr_si_field_t si = rlc_nr_si_field_t::full_sdu; if (retx.is_segment) { - RlcDebug("SDU segment can be fully transmitted. SN=%d, nof_bytes=%d, expected_hdr_len=%d, " - "current_so=%d, so_start=%d, so_end=%d", - retx.sn, - nof_bytes, - expected_hdr_len, - retx.current_so, - retx.so_start, - retx.so_end); if (retx.current_so == 0) { si = rlc_nr_si_field_t::first_segment; } else if ((retx.current_so + retx_payload_len) < tx_pdu.sdu_buf->N_bytes) { @@ -479,17 +483,15 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_without_segmentation(rlc_amd_retx_t& retx, } else { si = rlc_nr_si_field_t::last_segment; } + current_so = retx.current_so; } - - // Update & write header rlc_am_nr_pdu_header_t new_header = tx_pdu.header; - new_header.si = si; new_header.p = 0; - if (retx.is_segment) { - new_header.so = retx.current_so; - } - uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(new_header, payload); + new_header.si = si; + new_header.so = current_so; + uint32_t hdr_len = rlc_am_nr_write_data_pdu_header(new_header, payload); + // Write payload into PDU uint32_t pdu_bytes = 0; uint32_t retx_pdu_payload_size = 0; if (not retx.is_segment) { @@ -504,6 +506,7 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_without_segmentation(rlc_amd_retx_t& retx, srsran_assert(pdu_bytes <= nof_bytes, "Error calculating hdr_len and pdu_payload_len"); memcpy(&payload[hdr_len], &tx_pdu.sdu_buf->msg[retx.current_so], retx_pdu_payload_size); + // Update RETX queue and log retx_queue.pop(); RlcHexInfo(tx_window[retx.sn].sdu_buf->msg, tx_window[retx.sn].sdu_buf->N_bytes, @@ -548,7 +551,6 @@ uint32_t rlc_am_nr_tx::build_retx_pdu_with_segmentation(rlc_amd_retx_t& retx, ui } else { RlcDebug("Creating SDU segment from SDU segment. SN=%d, current_so=%d, so_start=%d, so_end=%d", - __FUNCTION__, retx.sn, retx.current_so, retx.so_start, @@ -862,7 +864,7 @@ uint32_t rlc_am_nr_tx::tx_mod_base_nr(uint32_t sn) const return (sn - st.tx_next_ack) % mod_nr; } -bool rlc_am_nr_tx::inside_tx_window(uint32_t sn) +bool rlc_am_nr_tx::inside_tx_window(uint32_t sn) const { // TX_Next_Ack <= SN < TX_Next_Ack + AM_Window_Size return tx_mod_base_nr(sn) < RLC_AM_NR_WINDOW_SIZE; @@ -871,7 +873,7 @@ bool rlc_am_nr_tx::inside_tx_window(uint32_t sn) /* * Debug Helpers */ -void rlc_am_nr_tx::debug_state() +void rlc_am_nr_tx::debug_state() const { RlcDebug("TX entity state: Tx_Next %d, Rx_Next_Ack %d, POLL_SN %d, PDU_WITHOUT_POLL %d, BYTE_WITHOUT_POLL %d", st.tx_next, @@ -1304,13 +1306,13 @@ uint32_t rlc_am_nr_rx::get_rx_buffered_bytes() } void rlc_am_nr_rx::insert_received_segment(rlc_amd_rx_pdu_nr segment, - std::set& segment_list) + std::set& segment_list) const { segment_list.insert(std::move(segment)); } bool rlc_am_nr_rx::have_all_segments_been_received( - const std::set& segment_list) + const std::set& segment_list) const { if (segment_list.empty()) { return false; @@ -1335,7 +1337,7 @@ bool rlc_am_nr_rx::have_all_segments_been_received( /* * Debug Helpers */ -void rlc_am_nr_rx::debug_state() +void rlc_am_nr_rx::debug_state() const { RlcDebug("RX entity state: Rx_Next %d, Rx_Next_Status_Trigger %d, Rx_Highest_Status %d, Rx_Next_Highest", st.rx_next, diff --git a/lib/src/rlc/rlc_am_nr_packing.cc b/lib/src/rlc/rlc_am_nr_packing.cc index ac6303a91..88565d2ef 100644 --- a/lib/src/rlc/rlc_am_nr_packing.cc +++ b/lib/src/rlc/rlc_am_nr_packing.cc @@ -211,6 +211,10 @@ uint32_t rlc_am_nr_read_status_pdu(const uint8_t* payload, ptr++; } status->N_nack++; + if ((ptr - payload) > nof_bytes) { + fprintf(stderr, "Malformed PDU, trying to read more bytes than it is available\n"); + return 0; + } } } diff --git a/lib/test/rlc/rlc_am_nr_pdu_test.cc b/lib/test/rlc/rlc_am_nr_pdu_test.cc index 2b456e0a4..df299ecdc 100644 --- a/lib/test/rlc/rlc_am_nr_pdu_test.cc +++ b/lib/test/rlc/rlc_am_nr_pdu_test.cc @@ -320,6 +320,62 @@ int rlc_am_nr_control_pdu_test3() return SRSRAN_SUCCESS; } +// Status PDU for 12bit SN with ACK_SN=2065, NACK_SN=273, SO_START=2, SO_END=5, NACK_SN=275 +// E1 and E2 bit set on first NACK, neither E1 or E2 on the second. +int rlc_am_nr_control_pdu_test4() +{ + test_delimit_logger delimiter("Control PDU test 4"); + const int len = 11; + std::array tv = {0x08, 0x11, 0x80, 0x11, 0x1c, 0x00, 0x02, 0x00, 0x05, 0x11, 0x30}; + srsran::byte_buffer_t pdu = make_pdu_and_log(tv); + + TESTASSERT(rlc_am_is_control_pdu(pdu.msg) == true); + + // unpack PDU + rlc_am_nr_status_pdu_t status_pdu = {}; + TESTASSERT(rlc_am_nr_read_status_pdu(&pdu, srsran::rlc_am_nr_sn_size_t::size12bits, &status_pdu) == SRSRAN_SUCCESS); + TESTASSERT(status_pdu.ack_sn == 2065); + TESTASSERT(status_pdu.N_nack == 2); + TESTASSERT(status_pdu.nacks[0].nack_sn == 273); + TESTASSERT(status_pdu.nacks[0].has_so == true); + TESTASSERT(status_pdu.nacks[0].so_start == 2); + TESTASSERT(status_pdu.nacks[0].so_end == 5); + TESTASSERT(status_pdu.nacks[1].nack_sn == 275); + TESTASSERT(status_pdu.nacks[1].has_so == false); + + // reset status PDU + pdu.clear(); + + // pack again + TESTASSERT(rlc_am_nr_write_status_pdu(status_pdu, srsran::rlc_am_nr_sn_size_t::size12bits, &pdu) == SRSRAN_SUCCESS); + TESTASSERT(pdu.N_bytes == tv.size()); + + write_pdu_to_pcap(4, pdu.msg, pdu.N_bytes); + TESTASSERT(memcmp(pdu.msg, tv.data(), pdu.N_bytes) == 0); + + return SRSRAN_SUCCESS; +} + +// Malformed Status PDU, with E1 still set at the end of the PDU +// 12bit SN with ACK_SN=2065, NACK_SN=273, SO_START=2, SO_END=5, NACK_SN=275, SO_START=5, SO_END=0xFFFF +// E1 and E2 bit set on first NACK, only E2 on second. +int rlc_am_nr_control_pdu_test5() +{ + test_delimit_logger delimiter("Control PDU test 5"); + const int len = 15; + std::array tv = { + 0x08, 0x11, 0x80, 0x11, 0x1c, 0x00, 0x02, 0x00, 0x05, 0x11, 0x3c, 0x00, 0x05, 0xFF, 0xFF}; + srsran::byte_buffer_t pdu = make_pdu_and_log(tv); + + TESTASSERT(rlc_am_is_control_pdu(pdu.msg) == true); + + // unpack PDU + rlc_am_nr_status_pdu_t status_pdu = {}; + TESTASSERT(rlc_am_nr_read_status_pdu(&pdu, srsran::rlc_am_nr_sn_size_t::size12bits, &status_pdu) == 0); + + return SRSRAN_SUCCESS; +} + int main(int argc, char** argv) { static const struct option long_options[] = {{"pcap", no_argument, nullptr, 'p'}, {nullptr, 0, nullptr, 0}}; @@ -391,5 +447,15 @@ int main(int argc, char** argv) return SRSRAN_ERROR; } + if (rlc_am_nr_control_pdu_test4()) { + fprintf(stderr, "rlc_am_nr_control_pdu_test4() failed.\n"); + return SRSRAN_ERROR; + } + + if (rlc_am_nr_control_pdu_test5()) { + fprintf(stderr, "rlc_am_nr_control_pdu_test5() failed.\n"); + return SRSRAN_ERROR; + } + return SRSRAN_SUCCESS; } diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index 4443a7797..fb08b94b2 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -38,7 +38,7 @@ int basic_test_tx(rlc_am* rlc, byte_buffer_t pdu_bufs[NBUFS]) rlc->write_sdu(std::move(sdu_bufs[i])); } - TESTASSERT(15 == rlc->get_buffer_state()); // 2 Bytes * NBUFFS (header size) + NBUFFS (data) = 15 + TESTASSERT_EQ(15, rlc->get_buffer_state()); // 2 Bytes * NBUFFS (header size) + NBUFFS (data) = 15 // Read 5 PDUs from RLC1 (1 byte each) for (int i = 0; i < NBUFS; i++) { @@ -47,14 +47,13 @@ int basic_test_tx(rlc_am* rlc, byte_buffer_t pdu_bufs[NBUFS]) TESTASSERT_EQ(3, len); } - TESTASSERT(0 == rlc->get_buffer_state()); + TESTASSERT_EQ(0, rlc->get_buffer_state()); return SRSRAN_SUCCESS; } /* * Test the limits of the TX/RX window checkers * - * This will test */ int window_checker_test() { @@ -114,6 +113,88 @@ int window_checker_test() return SRSRAN_SUCCESS; } +/* + * Test is retx_segmentation required + * + */ +int retx_segmentation_required_checker_test() +{ + rlc_am_tester tester; + timer_handler timers(8); + + auto& test_logger = srslog::fetch_basic_logger("TESTER "); + test_delimit_logger delimiter("retx segmentation required checkers"); + rlc_am rlc1(srsran_rat_t::nr, srslog::fetch_basic_logger("RLC_AM_1"), 1, &tester, &tester, &timers); + + rlc_am_nr_tx* tx = dynamic_cast(rlc1.get_tx()); + rlc_am_nr_rx* rx = dynamic_cast(rlc1.get_rx()); + + if (not rlc1.configure(rlc_config_t::default_rlc_am_nr_config())) { + return SRSRAN_ERROR; + } + + unique_byte_buffer_t sdu_bufs[NBUFS]; + unique_byte_buffer_t pdu_bufs[NBUFS]; + for (int i = 0; i < NBUFS; i++) { + sdu_bufs[i] = srsran::make_byte_buffer(); + pdu_bufs[i] = srsran::make_byte_buffer(); + sdu_bufs[i]->msg[0] = i; // Write the index into the buffer + sdu_bufs[i]->N_bytes = 5; // Give each buffer a size of 1 byte + sdu_bufs[i]->md.pdcp_sn = i; // PDCP SN for notifications + rlc1.write_sdu(std::move(sdu_bufs[i])); + rlc1.read_pdu(pdu_bufs[i]->msg, 8); + } + + // Test full SDU retx + { + uint32_t nof_bytes = 8; + rlc_amd_retx_t retx = {}; + retx.sn = 0; + retx.is_segment = false; + + tx->is_retx_segmentation_required(retx, nof_bytes); + TESTASSERT_EQ(false, tx->is_retx_segmentation_required(retx, nof_bytes)); + } + + // Test SDU retx segmentation required + { + uint32_t nof_bytes = 4; + rlc_amd_retx_t retx; + retx.sn = 0; + retx.is_segment = false; + + tx->is_retx_segmentation_required(retx, nof_bytes); + TESTASSERT_EQ(true, tx->is_retx_segmentation_required(retx, nof_bytes)); + } + + // Test full SDU segment retx + { + uint32_t nof_bytes = 40; + rlc_amd_retx_t retx = {}; + retx.sn = 0; + retx.is_segment = true; + retx.so_start = 4; + retx.so_end = 6; + + tx->is_retx_segmentation_required(retx, nof_bytes); + TESTASSERT_EQ(false, tx->is_retx_segmentation_required(retx, nof_bytes)); + } + + // Test SDU segment retx segmentation required + { + uint32_t nof_bytes = 4; + rlc_amd_retx_t retx = {}; + retx.sn = 0; + retx.is_segment = true; + retx.so_start = 4; + retx.so_end = 6; + + tx->is_retx_segmentation_required(retx, nof_bytes); + TESTASSERT_EQ(true, tx->is_retx_segmentation_required(retx, nof_bytes)); + } + return SRSRAN_SUCCESS; +} + /* * Test the transmission and acknowledgement of 5 SDUs. * @@ -139,7 +220,7 @@ int basic_test() rlc_am_nr_rx* rx2 = dynamic_cast(rlc2.get_rx()); // before configuring entity - TESTASSERT(0 == rlc1.get_buffer_state()); + TESTASSERT_EQ(0, rlc1.get_buffer_state()); if (not rlc1.configure(rlc_config_t::default_rlc_am_nr_config())) { return -1; @@ -156,18 +237,18 @@ int basic_test() rlc2.write_pdu(pdu_bufs[i].msg, pdu_bufs[i].N_bytes); } - TESTASSERT(3 == rlc2.get_buffer_state()); + TESTASSERT_EQ(3, rlc2.get_buffer_state()); // Read status PDU from RLC2 byte_buffer_t status_buf; int len = rlc2.read_pdu(status_buf.msg, 3); status_buf.N_bytes = len; - TESTASSERT(0 == rlc2.get_buffer_state()); + TESTASSERT_EQ(0, rlc2.get_buffer_state()); // Assert status is correct rlc_am_nr_status_pdu_t status_check = {}; rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 5); // 5 is the last SN that was not received. + TESTASSERT_EQ(5, status_check.ack_sn); // 5 is the last SN that was not received. // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); @@ -246,7 +327,7 @@ int lost_pdu_test() } // Only after t-reassembly has expired, will the status report include NACKs. - TESTASSERT(3 == rlc2.get_buffer_state()); + TESTASSERT_EQ(3, rlc2.get_buffer_state()); { // Read status PDU from RLC2 byte_buffer_t status_buf; @@ -258,7 +339,7 @@ int lost_pdu_test() // Assert status is correct rlc_am_nr_status_pdu_t status_check = {}; rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 3); // 3 is the next expected SN (i.e. the lost packet.) + TESTASSERT_EQ(3, status_check.ack_sn); // 3 is the next expected SN (i.e. the lost packet.) // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); @@ -270,27 +351,27 @@ int lost_pdu_test() } // t-reassembly has expired. There should be a NACK in the status report. - TESTASSERT(5 == rlc2.get_buffer_state()); + TESTASSERT_EQ(5, rlc2.get_buffer_state()); { // Read status PDU from RLC2 byte_buffer_t status_buf; int len = rlc2.read_pdu(status_buf.msg, 5); status_buf.N_bytes = len; - TESTASSERT(0 == rlc2.get_buffer_state()); + TESTASSERT_EQ(0, rlc2.get_buffer_state()); // Assert status is correct rlc_am_nr_status_pdu_t status_check = {}; rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. - TESTASSERT(status_check.N_nack == 1); // We lost one PDU. - TESTASSERT(status_check.nacks[0].nack_sn == 3); // Lost PDU SN=3. + TESTASSERT_EQ(5, status_check.ack_sn); // 5 is the next expected SN. + TESTASSERT_EQ(1, status_check.N_nack); // We lost one PDU. + TESTASSERT_EQ(3, status_check.nacks[0].nack_sn); // Lost PDU SN=3. // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); // Check there is an Retx of SN=3 - TESTASSERT(3 == rlc1.get_buffer_state()); + TESTASSERT_EQ(3, rlc1.get_buffer_state()); } { @@ -298,11 +379,11 @@ int lost_pdu_test() byte_buffer_t retx_buf; int len = rlc1.read_pdu(retx_buf.msg, 3); retx_buf.N_bytes = len; - TESTASSERT(3 == len); + TESTASSERT_EQ(3, len); rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); - TESTASSERT(0 == rlc2.get_buffer_state()); + TESTASSERT_EQ(0, rlc2.get_buffer_state()); } // Check statistics @@ -356,7 +437,7 @@ int basic_segmentation_test() rlc_am_nr_rx* rx2 = dynamic_cast(rlc2.get_rx()); // before configuring entity - TESTASSERT(0 == rlc1.get_buffer_state()); + TESTASSERT_EQ(0, rlc1.get_buffer_state()); if (not rlc1.configure(rlc_config_t::default_rlc_am_nr_config())) { return -1; @@ -436,7 +517,7 @@ int segment_retx_test() rlc_am_nr_rx* rx2 = dynamic_cast(rlc2.get_rx()); // before configuring entity - TESTASSERT(0 == rlc1.get_buffer_state()); + TESTASSERT_EQ(0, rlc1.get_buffer_state()); if (not rlc1.configure(rlc_config_t::default_rlc_am_nr_config())) { return -1; @@ -456,7 +537,7 @@ int segment_retx_test() rlc1.write_sdu(std::move(sdu_bufs[i])); } - TESTASSERT(25 == rlc1.get_buffer_state()); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) = 25 + TESTASSERT_EQ(25, rlc1.get_buffer_state()); // 2 Bytes * NBUFFS (header size) + NBUFFS * 3 (data) = 25 // Read 5 PDUs from RLC1 (1 byte each) for (int i = 0; i < NBUFS; i++) { @@ -465,7 +546,7 @@ int segment_retx_test() TESTASSERT_EQ(5, len); } - TESTASSERT(0 == rlc1.get_buffer_state()); + TESTASSERT_EQ(0, rlc1.get_buffer_state()); // Write 5 PDUs into RLC2 for (int i = 0; i < NBUFS; i++) { @@ -475,19 +556,19 @@ int segment_retx_test() } // Only after t-reassembly has expired, will the status report include NACKs. - TESTASSERT(3 == rlc2.get_buffer_state()); + TESTASSERT_EQ(3, rlc2.get_buffer_state()); { // Read status PDU from RLC2 byte_buffer_t status_buf; int len = rlc2.read_pdu(status_buf.msg, 5); status_buf.N_bytes = len; - TESTASSERT(0 == rlc2.get_buffer_state()); + TESTASSERT_EQ(0, rlc2.get_buffer_state()); // Assert status is correct rlc_am_nr_status_pdu_t status_check = {}; rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 3); // 3 is the next expected SN (i.e. the lost packet.) + TESTASSERT_EQ(3, status_check.ack_sn); // 3 is the next expected SN (i.e. the lost packet.) // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); @@ -499,27 +580,27 @@ int segment_retx_test() } // t-reassembly has expired. There should be a NACK in the status report. - TESTASSERT(5 == rlc2.get_buffer_state()); + TESTASSERT_EQ(5, rlc2.get_buffer_state()); { // Read status PDU from RLC2 byte_buffer_t status_buf; int len = rlc2.read_pdu(status_buf.msg, 5); status_buf.N_bytes = len; - TESTASSERT(0 == rlc2.get_buffer_state()); + TESTASSERT_EQ(0, rlc2.get_buffer_state()); // Assert status is correct rlc_am_nr_status_pdu_t status_check = {}; rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. - TESTASSERT(status_check.N_nack == 1); // We lost one PDU. - TESTASSERT(status_check.nacks[0].nack_sn == 3); // Lost PDU SN=3. + TESTASSERT_EQ(5, status_check.ack_sn); // 5 is the next expected SN. + TESTASSERT_EQ(1, status_check.N_nack); // We lost one PDU. + TESTASSERT_EQ(3, status_check.nacks[0].nack_sn); // Lost PDU SN=3. // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); // Check there is an Retx of SN=3 - TESTASSERT(5 == rlc1.get_buffer_state()); + TESTASSERT_EQ(5, rlc1.get_buffer_state()); } { @@ -529,23 +610,23 @@ int segment_retx_test() uint32_t len = 0; if (i == 0) { len = rlc1.read_pdu(retx_buf.msg, 3); - TESTASSERT(3 == len); + TESTASSERT_EQ(3, len); } else { len = rlc1.read_pdu(retx_buf.msg, 5); - TESTASSERT(5 == len); + TESTASSERT_EQ(5, len); } retx_buf.N_bytes = len; rlc_am_nr_pdu_header_t header_check = {}; uint32_t hdr_len = rlc_am_nr_read_data_pdu_header(&retx_buf, rlc_am_nr_sn_size_t::size12bits, &header_check); // Double check header. - TESTASSERT(header_check.sn == 3); // Double check RETX SN + TESTASSERT_EQ(3, header_check.sn); // Double check RETX SN if (i == 0) { - TESTASSERT(header_check.si == rlc_nr_si_field_t::first_segment); + TESTASSERT_EQ(rlc_nr_si_field_t::first_segment, header_check.si); } else if (i == 1) { - TESTASSERT(header_check.si == rlc_nr_si_field_t::neither_first_nor_last_segment); + TESTASSERT_EQ(rlc_nr_si_field_t::neither_first_nor_last_segment, header_check.si); } else { - TESTASSERT(header_check.si == rlc_nr_si_field_t::last_segment); + TESTASSERT_EQ(rlc_nr_si_field_t::last_segment, header_check.si); } rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); @@ -649,7 +730,7 @@ int retx_segment_test() } } - TESTASSERT(0 == rlc1.get_buffer_state()); + TESTASSERT_EQ(0, rlc1.get_buffer_state()); // Write 15 - 3 PDUs into RLC2 for (int i = 0; i < n_pdu_bufs; i++) { @@ -659,19 +740,19 @@ int retx_segment_test() } // Only after t-reassembly has expired, will the status report include NACKs. - TESTASSERT(3 == rlc2.get_buffer_state()); + TESTASSERT_EQ(3, rlc2.get_buffer_state()); { // Read status PDU from RLC2 byte_buffer_t status_buf; int len = rlc2.read_pdu(status_buf.msg, 5); status_buf.N_bytes = len; - TESTASSERT(0 == rlc2.get_buffer_state()); + TESTASSERT_EQ(0, rlc2.get_buffer_state()); // Assert status is correct rlc_am_nr_status_pdu_t status_check = {}; rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 1); // 1 is the next expected SN (i.e. the first lost packet.) + TESTASSERT_EQ(1, status_check.ack_sn); // 1 is the next expected SN (i.e. the first lost packet.) // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); @@ -684,38 +765,38 @@ int retx_segment_test() // t-reassembly has expired. There should be a NACK in the status report. // There should be 3 NACKs with SO_start and SO_end - TESTASSERT(21 == rlc2.get_buffer_state()); // 3 bytes for fixed header (ACK+E1) + 3 * 6 for NACK with SO = 21. + TESTASSERT_EQ(21, rlc2.get_buffer_state()); // 3 bytes for fixed header (ACK+E1) + 3 * 6 for NACK with SO = 21. { // Read status PDU from RLC2 byte_buffer_t status_buf; int len = rlc2.read_pdu(status_buf.msg, 21); status_buf.N_bytes = len; - TESTASSERT(0 == rlc2.get_buffer_state()); + TESTASSERT_EQ(0, rlc2.get_buffer_state()); // Assert status is correct rlc_am_nr_status_pdu_t status_check = {}; rlc_am_nr_read_status_pdu(&status_buf, rlc_am_nr_sn_size_t::size12bits, &status_check); - TESTASSERT(status_check.ack_sn == 5); // 5 is the next expected SN. - TESTASSERT(status_check.N_nack == 3); // We lost one PDU. - TESTASSERT(status_check.nacks[0].nack_sn == 1); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[0].has_so == true); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[0].so_start == 0); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[0].so_end == 1); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[1].nack_sn == 2); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[1].has_so == true); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[1].so_start == 1); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[1].so_end == 2); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[2].nack_sn == 3); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[2].has_so == true); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[2].so_start == 2); // Lost SDU on SN=1. - TESTASSERT(status_check.nacks[2].so_end == 0xFFFF); // Lost SDU on SN=1. + TESTASSERT_EQ(5, status_check.ack_sn); // 5 is the next expected SN. + TESTASSERT_EQ(3, status_check.N_nack); // We lost one PDU. + TESTASSERT_EQ(1, status_check.nacks[0].nack_sn); // Lost SDU on SN=1. + TESTASSERT_EQ(true, status_check.nacks[0].has_so); // Lost SDU on SN=1. + TESTASSERT_EQ(0, status_check.nacks[0].so_start); // Lost SDU on SN=1. + TESTASSERT_EQ(1, status_check.nacks[0].so_end); // Lost SDU on SN=1. + TESTASSERT_EQ(2, status_check.nacks[1].nack_sn); // Lost SDU on SN=1. + TESTASSERT_EQ(true, status_check.nacks[1].has_so); // Lost SDU on SN=1. + TESTASSERT_EQ(1, status_check.nacks[1].so_start); // Lost SDU on SN=1. + TESTASSERT_EQ(2, status_check.nacks[1].so_end); // Lost SDU on SN=1. + TESTASSERT_EQ(3, status_check.nacks[2].nack_sn); // Lost SDU on SN=1. + TESTASSERT_EQ(true, status_check.nacks[2].has_so); // Lost SDU on SN=1. + TESTASSERT_EQ(2, status_check.nacks[2].so_start); // Lost SDU on SN=1. + TESTASSERT_EQ(0xFFFF, status_check.nacks[2].so_end); // Lost SDU on SN=1. // Write status PDU to RLC1 rlc1.write_pdu(status_buf.msg, status_buf.N_bytes); // Check there is an Retx of SN=3 - TESTASSERT(5 == rlc1.get_buffer_state()); + TESTASSERT_EQ(5, rlc1.get_buffer_state()); } { @@ -725,10 +806,10 @@ int retx_segment_test() uint32_t len = 0; if (i == 0) { len = rlc1.read_pdu(retx_buf.msg, 3); - TESTASSERT(3 == len); + TESTASSERT_EQ(3, len); } else { len = rlc1.read_pdu(retx_buf.msg, 5); - TESTASSERT(5 == len); + TESTASSERT_EQ(5, len); } retx_buf.N_bytes = len; @@ -736,19 +817,19 @@ int retx_segment_test() uint32_t hdr_len = rlc_am_nr_read_data_pdu_header(&retx_buf, rlc_am_nr_sn_size_t::size12bits, &header_check); // Double check header. if (i == 0) { - TESTASSERT(header_check.sn == 1); // Double check RETX SN - TESTASSERT(header_check.si == rlc_nr_si_field_t::first_segment); + TESTASSERT_EQ(1, header_check.sn); // Double check RETX SN + TESTASSERT_EQ(rlc_nr_si_field_t::first_segment, header_check.si); } else if (i == 1) { - TESTASSERT(header_check.sn == 2); // Double check RETX SN - TESTASSERT(header_check.si == rlc_nr_si_field_t::neither_first_nor_last_segment); + TESTASSERT_EQ(2, header_check.sn); // Double check RETX SN + TESTASSERT_EQ(rlc_nr_si_field_t::neither_first_nor_last_segment, header_check.si); } else { - TESTASSERT(header_check.sn == 3); // Double check RETX SN - TESTASSERT(header_check.si == rlc_nr_si_field_t::last_segment); + TESTASSERT_EQ(3, header_check.sn); // Double check RETX SN + TESTASSERT_EQ(rlc_nr_si_field_t::last_segment, header_check.si); } rlc2.write_pdu(retx_buf.msg, retx_buf.N_bytes); } - TESTASSERT(0 == rlc1.get_buffer_state()); + TESTASSERT_EQ(0, rlc1.get_buffer_state()); } // Check statistics @@ -819,6 +900,7 @@ int main() // start log back-end srslog::init(); TESTASSERT(window_checker_test() == SRSRAN_SUCCESS); + TESTASSERT(retx_segmentation_required_checker_test() == SRSRAN_SUCCESS); TESTASSERT(basic_test() == SRSRAN_SUCCESS); TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS); TESTASSERT(basic_segmentation_test() == SRSRAN_SUCCESS); From 9205ede8c1fc32c601485b63fa557908e5e347ee Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Tue, 15 Feb 2022 18:17:02 +0000 Subject: [PATCH 27/28] lib,rlc_am_nr: enabled RLC AM NR stress tests --- lib/test/rlc/CMakeLists.txt | 1 + lib/test/rlc/rlc_stress_test.cc | 2 ++ 2 files changed, 3 insertions(+) diff --git a/lib/test/rlc/CMakeLists.txt b/lib/test/rlc/CMakeLists.txt index 7768eb3be..e9956147f 100644 --- a/lib/test/rlc/CMakeLists.txt +++ b/lib/test/rlc/CMakeLists.txt @@ -36,6 +36,7 @@ add_lte_test(rlc_tm_stress_test rlc_stress_test --mode=TM --loglevel 1 --random_ add_nr_test(rlc_um6_nr_stress_test rlc_stress_test --rat NR --mode=UM6 --loglevel 1) add_nr_test(rlc_um12_nr_stress_test rlc_stress_test --rat NR --mode=UM12 --loglevel 1) +add_nr_test(rlc_am12_nr_stress_test rlc_stress_test --rat NR --mode=AM12 --loglevel 1) add_executable(rlc_um_data_test rlc_um_data_test.cc) target_link_libraries(rlc_um_data_test srsran_rlc srsran_phy srsran_common) diff --git a/lib/test/rlc/rlc_stress_test.cc b/lib/test/rlc/rlc_stress_test.cc index cc6478be1..b8e7def3a 100644 --- a/lib/test/rlc/rlc_stress_test.cc +++ b/lib/test/rlc/rlc_stress_test.cc @@ -488,6 +488,8 @@ void stress_test(stress_test_args_t args) cnfg_ = rlc_config_t::default_rlc_um_nr_config(6); } else if (args.mode == "UM12") { cnfg_ = rlc_config_t::default_rlc_um_nr_config(12); + } else if (args.mode == "AM12") { + cnfg_ = rlc_config_t::default_rlc_am_nr_config(); } else { cout << "Unsupported RLC mode " << args.mode << ", exiting." << endl; exit(-1); From 2156c319d2905315d7e34512deb9367b28775970 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Thu, 17 Feb 2022 11:31:24 +0000 Subject: [PATCH 28/28] lib,rlc_am_nr: temporarily disable RLC AM NR stress test. --- lib/test/rlc/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/test/rlc/CMakeLists.txt b/lib/test/rlc/CMakeLists.txt index e9956147f..9efd2dd13 100644 --- a/lib/test/rlc/CMakeLists.txt +++ b/lib/test/rlc/CMakeLists.txt @@ -36,7 +36,7 @@ add_lte_test(rlc_tm_stress_test rlc_stress_test --mode=TM --loglevel 1 --random_ add_nr_test(rlc_um6_nr_stress_test rlc_stress_test --rat NR --mode=UM6 --loglevel 1) add_nr_test(rlc_um12_nr_stress_test rlc_stress_test --rat NR --mode=UM12 --loglevel 1) -add_nr_test(rlc_am12_nr_stress_test rlc_stress_test --rat NR --mode=AM12 --loglevel 1) +#add_nr_test(rlc_am12_nr_stress_test rlc_stress_test --rat NR --mode=AM12 --loglevel 1) add_executable(rlc_um_data_test rlc_um_data_test.cc) target_link_libraries(rlc_um_data_test srsran_rlc srsran_phy srsran_common)