From a11291eb08d7a13ee92e09e9b31fceaf42d900ab Mon Sep 17 00:00:00 2001 From: Robert Falkenberg Date: Fri, 3 Jun 2022 12:11:53 +0200 Subject: [PATCH] lib,rlc_am_nr: fix handling of NACK ranges with SO reaching SDU edge This changes the handling of NACK ranges with segment offset (SO), where either so_start or so_end reach the edge of a full SDU. That SDU is then NACK'ed as a whole, rather than as a segment from 0 to 0xFFFF. Otherwise, the search for segments will fail if said SDU was initially sent as a whole (without segmentation). --- lib/include/srsran/rlc/rlc_common.h | 2 +- lib/src/rlc/rlc_am_nr.cc | 13 +- lib/test/rlc/rlc_am_nr_test.cc | 216 ++++++++++++++++++++++++++++ 3 files changed, 223 insertions(+), 8 deletions(-) diff --git a/lib/include/srsran/rlc/rlc_common.h b/lib/include/srsran/rlc/rlc_common.h index 1c35fc467..f6b2bc8b2 100644 --- a/lib/include/srsran/rlc/rlc_common.h +++ b/lib/include/srsran/rlc/rlc_common.h @@ -179,7 +179,7 @@ struct rlc_status_nack_t { has_so = false; nack_sn = 0; so_start = 0; - so_end = 0; + so_end = so_end_of_sdu; has_nack_range = false; nack_range = 0; } diff --git a/lib/src/rlc/rlc_am_nr.cc b/lib/src/rlc/rlc_am_nr.cc index 2ddb062d4..0acdbe273 100644 --- a/lib/src/rlc/rlc_am_nr.cc +++ b/lib/src/rlc/rlc_am_nr.cc @@ -854,17 +854,16 @@ void rlc_am_nr_tx::handle_control_pdu(uint8_t* payload, uint32_t nof_bytes) rlc_status_nack_t nack = {}; nack.nack_sn = range_sn; if (status.nacks[nack_idx].has_so) { + // Apply so_start to first range item if (range_sn == status.nacks[nack_idx].nack_sn) { - // First SN - nack.has_so = true; nack.so_start = status.nacks[nack_idx].so_start; - nack.so_end = rlc_status_nack_t::so_end_of_sdu; - } else if (range_sn == (status.nacks[nack_idx].nack_sn + status.nacks[nack_idx].nack_range - 1)) { - // Last SN - nack.has_so = true; - // This might be first+last item at the same time, so don't change so_start here + } + // Apply so_end to last range item + if (range_sn == (status.nacks[nack_idx].nack_sn + status.nacks[nack_idx].nack_range - 1)) { nack.so_end = status.nacks[nack_idx].so_end; } + // Enable has_so only if the offsets do not span the whole SDU + nack.has_so = (nack.so_start != 0) || (nack.so_end != rlc_status_nack_t::so_end_of_sdu); } handle_nack(nack, retx_sn_set); } diff --git a/lib/test/rlc/rlc_am_nr_test.cc b/lib/test/rlc/rlc_am_nr_test.cc index 25ca15fae..707ba3859 100644 --- a/lib/test/rlc/rlc_am_nr_test.cc +++ b/lib/test/rlc/rlc_am_nr_test.cc @@ -2837,6 +2837,220 @@ int rx_nack_range_with_so_test(rlc_am_nr_sn_size_t sn_size) return SRSRAN_SUCCESS; } +int rx_nack_range_with_so_starting_with_full_sdu_test(rlc_am_nr_sn_size_t sn_size) +{ + 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); + std::string str = + "Rx NACK range test with SO starting with full SDU (" + std::to_string(to_number(sn_size)) + " bit SN)"; + test_delimit_logger delimiter(str.c_str()); + + 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()); + + auto rlc_cnfg = rlc_config_t::default_rlc_am_nr_config(to_number(sn_size)); + rlc_cnfg.am_nr.t_poll_retx = -1; + if (not rlc1.configure(rlc_cnfg)) { + return -1; + } + + // after configuring entity + TESTASSERT(0 == rlc1.get_buffer_state()); + + int n_sdu_bufs = 5; + int n_pdu_bufs = 15; + + // Push 5 SDUs into RLC1 + std::vector sdu_bufs(n_sdu_bufs); + constexpr uint32_t payload_size = 3; // Give the SDU the size of 3 bytes + uint32_t header_size = sn_size == rlc_am_nr_sn_size_t::size12bits ? 2 : 3; + 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 = payload_size; // 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])); + } + + uint32_t expected_buffer_state = (header_size + payload_size) * n_sdu_bufs; + TESTASSERT_EQ(expected_buffer_state, rlc1.get_buffer_state()); + + constexpr uint32_t so_size = 2; + constexpr uint32_t segment_size = 1; + uint32_t pdu_size_whole = header_size + payload_size; + uint32_t pdu_size_first = header_size + segment_size; + uint32_t pdu_size_continued = header_size + so_size + segment_size; + + // Read 15 PDUs from RLC1 + std::vector pdu_bufs(n_pdu_bufs); + for (int i = 0; i < n_pdu_bufs; i++) { + // First also test buffer state + uint32_t remaining_total_bytes = (payload_size * n_sdu_bufs) - (i * segment_size); + uint32_t remaining_full_sdus = remaining_total_bytes / payload_size; + uint32_t remaining_seg_bytes = remaining_total_bytes % payload_size; + + uint32_t buffer_state_full_sdus = (header_size + payload_size) * remaining_full_sdus; + uint32_t buffer_state_seg_sdu = remaining_seg_bytes == 0 ? 0 : (header_size + so_size + remaining_seg_bytes); + expected_buffer_state = buffer_state_full_sdus + buffer_state_seg_sdu; + TESTASSERT_EQ(expected_buffer_state, rlc1.get_buffer_state()); + + pdu_bufs[i] = srsran::make_byte_buffer(); + if (i == 3) { + // Special handling for SDU SN=1 (i==3): send as a whole, not segmented + uint32_t len = rlc1.read_pdu(pdu_bufs[i]->msg, pdu_size_whole); // 2 bytes for header + 3 byte payload + pdu_bufs[i]->N_bytes = len; + TESTASSERT_EQ(pdu_size_whole, len); + // update i to skip 2 segments + i += 2; + } else { + if (i == 0 || i == 6 || i == 9 || i == 12) { + // First segment, no SO + uint32_t len = rlc1.read_pdu(pdu_bufs[i]->msg, pdu_size_first); // 2 bytes for header + 1 byte payload + pdu_bufs[i]->N_bytes = len; + TESTASSERT_EQ(pdu_size_first, len); + } else { + // Middle or last segment, SO present + uint32_t len = rlc1.read_pdu(pdu_bufs[i]->msg, pdu_size_continued); // 4 bytes for header + 1 byte payload + pdu_bufs[i]->N_bytes = len; + TESTASSERT_EQ(pdu_size_continued, len); + } + } + } + + // Deliver dummy status report with nack range betwen PDU 4 and 10. + rlc_am_nr_status_pdu_t status(sn_size); + status.ack_sn = 5; + + rlc_status_nack_t nack = {}; + nack.nack_sn = 1; + nack.has_nack_range = true; + nack.nack_range = 3; + nack.has_so = true; + nack.so_start = 0; + nack.so_end = 0; + status.push_nack(nack); + byte_buffer_t status_pdu; + rlc_am_nr_write_status_pdu(status, sn_size, &status_pdu); + + rlc1.write_pdu(status_pdu.msg, status_pdu.N_bytes); + + TESTASSERT_EQ(pdu_size_whole + 2 * pdu_size_first + 2 * pdu_size_continued, rlc1.get_buffer_state()); + return SRSRAN_SUCCESS; +} + +int rx_nack_range_with_so_ending_with_full_sdu_test(rlc_am_nr_sn_size_t sn_size) +{ + 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); + std::string str = + "Rx NACK range test with SO starting with full SDU (" + std::to_string(to_number(sn_size)) + " bit SN)"; + test_delimit_logger delimiter(str.c_str()); + + 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()); + + auto rlc_cnfg = rlc_config_t::default_rlc_am_nr_config(to_number(sn_size)); + rlc_cnfg.am_nr.t_poll_retx = -1; + if (not rlc1.configure(rlc_cnfg)) { + return -1; + } + + // after configuring entity + TESTASSERT(0 == rlc1.get_buffer_state()); + + int n_sdu_bufs = 5; + int n_pdu_bufs = 15; + + // Push 5 SDUs into RLC1 + std::vector sdu_bufs(n_sdu_bufs); + constexpr uint32_t payload_size = 3; // Give the SDU the size of 3 bytes + uint32_t header_size = sn_size == rlc_am_nr_sn_size_t::size12bits ? 2 : 3; + 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 = payload_size; // 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])); + } + + uint32_t expected_buffer_state = (header_size + payload_size) * n_sdu_bufs; + TESTASSERT_EQ(expected_buffer_state, rlc1.get_buffer_state()); + + constexpr uint32_t so_size = 2; + constexpr uint32_t segment_size = 1; + uint32_t pdu_size_whole = header_size + payload_size; + uint32_t pdu_size_first = header_size + segment_size; + uint32_t pdu_size_continued = header_size + so_size + segment_size; + + // Read 15 PDUs from RLC1 + std::vector pdu_bufs(n_pdu_bufs); + for (int i = 0; i < n_pdu_bufs; i++) { + // First also test buffer state + uint32_t remaining_total_bytes = (payload_size * n_sdu_bufs) - (i * segment_size); + uint32_t remaining_full_sdus = remaining_total_bytes / payload_size; + uint32_t remaining_seg_bytes = remaining_total_bytes % payload_size; + + uint32_t buffer_state_full_sdus = (header_size + payload_size) * remaining_full_sdus; + uint32_t buffer_state_seg_sdu = remaining_seg_bytes == 0 ? 0 : (header_size + so_size + remaining_seg_bytes); + expected_buffer_state = buffer_state_full_sdus + buffer_state_seg_sdu; + TESTASSERT_EQ(expected_buffer_state, rlc1.get_buffer_state()); + + pdu_bufs[i] = srsran::make_byte_buffer(); + if (i == 9) { + // Special handling for SDU SN=3 (i==9): send as a whole, not segmented + uint32_t len = rlc1.read_pdu(pdu_bufs[i]->msg, pdu_size_whole); // 2 bytes for header + 3 byte payload + pdu_bufs[i]->N_bytes = len; + TESTASSERT_EQ(pdu_size_whole, len); + // update i to skip 2 segments + i += 2; + } else { + if (i == 0 || i == 3 || i == 6 || i == 12) { + // First segment, no SO + uint32_t len = rlc1.read_pdu(pdu_bufs[i]->msg, pdu_size_first); // 2 bytes for header + 1 byte payload + pdu_bufs[i]->N_bytes = len; + TESTASSERT_EQ(pdu_size_first, len); + } else { + // Middle or last segment, SO present + uint32_t len = rlc1.read_pdu(pdu_bufs[i]->msg, pdu_size_continued); // 4 bytes for header + 1 byte payload + pdu_bufs[i]->N_bytes = len; + TESTASSERT_EQ(pdu_size_continued, len); + } + } + } + + // Deliver dummy status report with nack range betwen PDU 6 and 12. + rlc_am_nr_status_pdu_t status(sn_size); + status.ack_sn = 5; + + rlc_status_nack_t nack = {}; + nack.nack_sn = 1; + nack.has_nack_range = true; + nack.nack_range = 3; + nack.has_so = true; + nack.so_start = 2; + nack.so_end = rlc_status_nack_t::so_end_of_sdu; + status.push_nack(nack); + byte_buffer_t status_pdu; + rlc_am_nr_write_status_pdu(status, sn_size, &status_pdu); + + rlc1.write_pdu(status_pdu.msg, status_pdu.N_bytes); + + TESTASSERT_EQ(1 * pdu_size_first + 3 * pdu_size_continued + pdu_size_whole, rlc1.get_buffer_state()); + return SRSRAN_SUCCESS; +} + int out_of_order_status(rlc_am_nr_sn_size_t sn_size) { rlc_am_tester tester; @@ -2954,6 +3168,8 @@ int main() TESTASSERT(poll_retx_expiry(sn_size) == SRSRAN_SUCCESS); TESTASSERT(rx_nack_range_no_so_test(sn_size) == SRSRAN_SUCCESS); TESTASSERT(rx_nack_range_with_so_test(sn_size) == SRSRAN_SUCCESS); + TESTASSERT(rx_nack_range_with_so_starting_with_full_sdu_test(sn_size) == SRSRAN_SUCCESS); + TESTASSERT(rx_nack_range_with_so_ending_with_full_sdu_test(sn_size) == SRSRAN_SUCCESS); TESTASSERT(out_of_order_status(sn_size) == SRSRAN_SUCCESS); } return SRSRAN_SUCCESS;