Adding unit test for lost SDUs. Fixing way rx_next and rx_status_highest are updated.

lib/src/rlc/rlc_am_nr.cc

@@ -184,7 +184,7 @@ void rlc_am_nr_tx::get_buffer_state(uint32_t& n_bytes_new, uint32_t& n_bytes_pri
   */
 
   // Bytes needed for status report
-  if (do_status() /* && not TODO status_prohibit_timer*/) {
+  if (do_status()) {
     n_bytes_prio += rx->get_status_pdu_length();
     logger->debug("%s Buffer state - total status report: %d bytes", rb_name, n_bytes_prio);
   }
@@ -311,12 +311,20 @@ void rlc_am_nr_rx::handle_data_pdu(uint8_t* payload, uint32_t nof_bytes)
     do_status = true;
   }
 
-  // Update reordering variables and timers (36.322 v10.0.0 Section 5.1.3.2.3)
-  // TODO
-
   debug_state();
 
-  // Iterate over Rx Window and write any finished PDUs
+  // 5.2.3.2.3 Actions when an AMD PDU is placed in the reception buffer
+  // Update Rx_Next_Highest
+  if (RX_MOD_BASE_NR(header.sn) >= RX_MOD_BASE_NR(rx_next_highest)) {
+    rx_next_highest = (header.sn + 1) % MOD;
+  }
+
+  // Check if all bytes of the RLC SDU with SN = x are received:
+  bool sdu_fully_received = false;
+  if (header.si == rlc_nr_si_field_t::full_sdu) { // Check whether it's a full SDU
+    parent->pdcp->write_pdu(parent->lcid, std::move(rx_window[header.sn].buf));
+    sdu_fully_received = true;
+  } else {
     for (uint32_t i = rx_next; i < rx_next + RLC_AM_WINDOW_SIZE; i++) {
       if (rx_window.has_sn(i)) {
         if (rx_window[i].header.si == rlc_nr_si_field_t::full_sdu) {
@@ -327,21 +335,57 @@ void rlc_am_nr_rx::handle_data_pdu(uint8_t* payload, uint32_t nof_bytes)
         break;
       }
     }
-
-  // Update Rx_Next_Highest
-  if (RX_MOD_BASE_NR(header.sn) >= RX_MOD_BASE_NR(rx_next_highest)) {
-    rx_next_highest = (header.sn + 1) % MOD;
   }
 
+  if (sdu_fully_received) {
+    // Remove SDU from RX Window
+    rx_window.remove_pdu(header.sn);
+
     // Update RX_Highest_Status
+    /*
+     * - if x = RX_Highest_Status,
+     *   - update RX_Highest_Status to the SN of the first RLC SDU with SN > current RX_Highest_Status for which not all
+     *     bytes have been received.
+     */
     if (RX_MOD_BASE_NR(header.sn) == RX_MOD_BASE_NR(rx_highest_status)) {
-    rx_highest_status = (header.sn + 1) % MOD;
+      bool has_sn_with_missing_bytes = false;
+      for (uint32_t sn_tmp = rx_highest_status; sn_tmp < rx_highest_status + RLC_AM_WINDOW_SIZE; sn_tmp++) {
+        if (rx_window.has_sn(sn_tmp)) {
+          rx_highest_status         = sn_tmp;
+          has_sn_with_missing_bytes = true;
+          break;
+        }
+      }
+      if (not has_sn_with_missing_bytes) {
+        // There was no SN with missing bytes on the RX window.
+        // Updating RX_Highest_Status to RX_Highest_Status + 1
+        rx_highest_status = header.sn + 1 % MOD;
+      }
     }
 
-  // Update RX_Next (FIXME should only be updated if all segments are received)
+    /*
+     * - if x = RX_Next:
+     *   - update RX_Next to the SN of the first RLC SDU with SN > current RX_Next for which not all bytes
+     *     have been received.
+     */
     if (RX_MOD_BASE_NR(header.sn) == RX_MOD_BASE_NR(rx_next)) {
+      bool has_sn_with_missing_bytes = false;
+      for (uint32_t sn_tmp = rx_next; sn_tmp < rx_next + RLC_AM_WINDOW_SIZE; sn_tmp++) {
+        if (rx_window.has_sn(sn_tmp)) {
+          rx_next                   = sn_tmp;
+          has_sn_with_missing_bytes = true;
+          break;
+        }
+      }
+      if (not has_sn_with_missing_bytes) {
+        // There was no SN with missing bytes on the RX window.
+        // Updating RX_Next to RX_Next + 1
         rx_next = header.sn + 1 % MOD;
       }
+    }
+  }
+  // if t-Reassembly is running: (TODO)
+  // if t-Reassembly is not running (includes the case t-Reassembly is stopped due to actions above): (TODO)
 }
 
 bool rlc_am_nr_rx::inside_rx_window(uint32_t sn)
@@ -404,8 +448,6 @@ void rlc_am_nr_rx::timer_expired(uint32_t timeout_id)
   if (status_prohibit_timer.is_valid() && status_prohibit_timer.id() == timeout_id) {
     logger->debug("%s Status prohibit timer expired after %dms", parent->rb_name, status_prohibit_timer.duration());
   }
-
-  lock.unlock();
 }
 
 /*

lib/test/rlc/rlc_am_nr_test.cc

@@ -57,6 +57,10 @@ int basic_test()
   timer_handler timers(8);
   byte_buffer_t pdu_bufs[NBUFS];
 
+  auto& test_logger = srslog::fetch_basic_logger("TESTER  ");
+  test_logger.info("====================");
+  test_logger.info("==== Basic Test ====");
+  test_logger.info("====================");
   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);
 
@@ -102,6 +106,68 @@ int basic_test()
   return SRSRAN_SUCCESS;
 }
 
+/*
+ * Test the loss of a single PDU.
+ * NACK should be visible in the status report.
+ * Retx after NACK should be present too.
+ */
+int lost_pdu_test()
+{
+  rlc_am_tester tester;
+  timer_handler timers(8);
+  byte_buffer_t pdu_bufs[NBUFS];
+
+  auto& test_logger = srslog::fetch_basic_logger("TESTER  ");
+  test_logger.info("=======================");
+  test_logger.info("==== Lost PDU Test ====");
+  test_logger.info("=======================");
+  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);
+
+  // 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;
+  }
+
+  basic_test_tx(&rlc1, pdu_bufs);
+
+  // 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.
+    }
+  }
+
+  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 last SN that was not received.
+                                        // TESTASSERT(rlc_am_is_valid_status_pdu(status_check));
+
+  // Write status PDU to RLC1
+  rlc1.write_pdu(status_buf.msg, status_buf.N_bytes);
+  // Check PDCP notifications
+  // TODO
+
+  // Check statistics
+  TESTASSERT(rx_is_tx(rlc1.get_metrics(), rlc2.get_metrics()));
+  return SRSRAN_SUCCESS;
+}
+
 int main(int argc, char** argv)
 {
   // Setup the log message spy to intercept error and warning log entries from RLC
@@ -128,6 +194,7 @@ int main(int argc, char** argv)
   srslog::init();
 
   TESTASSERT(basic_test() == SRSRAN_SUCCESS);
+  TESTASSERT(lost_pdu_test() == SRSRAN_SUCCESS);
 
   return SRSRAN_SUCCESS;
 }