rlc_am_nr: Enabled sending NACKs on status PDUs

- Added some logic to only poll when PDU_WITHOUT_POLL > PollPDU
  - Fixed initalization of t-Reassembly
  - Fixed a bug in the packing of the status buffer.
Nacks now work for a single lost PDU.

lib/include/srsran/interfaces/rlc_interface_types.h

@@ -225,6 +225,8 @@ public:
     rlc_cnfg.rat                     = srsran_rat_t::nr;
     rlc_cnfg.rlc_mode                = rlc_mode_t::am;
     rlc_cnfg.am_nr.t_status_prohibit = 8;
+    rlc_cnfg.am_nr.t_reassembly      = 35;
+    rlc_cnfg.am_nr.poll_pdu          = 4;
     return rlc_cnfg;
   }
   static rlc_config_t default_rlc_um_nr_config(uint32_t sn_size = 6)

lib/src/rlc/rlc_am_nr.cc

@@ -112,9 +112,20 @@ uint32_t rlc_am_nr_tx::read_pdu(uint8_t* payload, uint32_t nof_bytes)
     return 0;
   }
 
+  // Check wether polling is required
+  uint8_t poll = 0;
+  if (cfg.poll_pdu > 0) {
+    if (st.pdu_without_poll >= (uint32_t)cfg.poll_pdu) {
+      poll                = 1;
+      st.pdu_without_poll = 0;
+    } else {
+      st.pdu_without_poll++;
+    }
+  }
+
   rlc_am_nr_pdu_header_t hdr = {};
   hdr.dc                     = RLC_DC_FIELD_DATA_PDU;
-  hdr.p                      = 1; // FIXME
+  hdr.p                      = poll; // FIXME
   hdr.si                     = rlc_nr_si_field_t::full_sdu;
   hdr.sn_size                = rlc_am_nr_sn_size_t::size12bits;
   hdr.sn                     = st.tx_next;
@@ -234,6 +245,7 @@ rlc_am_nr_rx::rlc_am_nr_rx(rlc_am* parent_) :
   parent(parent_),
   pool(byte_buffer_pool::get_instance()),
   status_prohibit_timer(parent->timers->get_unique_timer()),
+  reassembly_timer(parent->timers->get_unique_timer()),
   rlc_am_base_rx(parent_, &parent_->logger)
 {}
 
@@ -250,6 +262,7 @@ bool rlc_am_nr_rx::configure(const rlc_config_t& cfg_)
   // Configure t_reassembly timer
   if (cfg.t_reassembly > 0) {
     reassembly_timer.set(static_cast<uint32_t>(cfg.t_reassembly), [this](uint32_t timerid) { timer_expired(timerid); });
+    logger->info("Configured reassembly timer. t-Reassembly=%d ms", cfg.t_reassembly);
   }
 
   return true;
@@ -398,7 +411,21 @@ void rlc_am_nr_rx::handle_data_pdu(uint8_t* payload, uint32_t nof_bytes)
      * - if RX_Next_Highest> RX_Next +1; or
      * - if RX_Next_Highest = RX_Next + 1 and there is at least one missing byte segment of the SDU associated
      *   with SN = RX_Next before the last byte of all received segments of this SDU:
+     *   - start t-Reassembly;
+     *   - set RX_Next_Status_Trigger to RX_Next_Highest.
      */
+    bool restart_reassembly_timer = false;
+    if (rx_next_highest > rx_next + 1) {
+      restart_reassembly_timer = true;
+    }
+    if (rx_next_highest == rx_next + 1 &&
+        rx_window[rx_next + 1].fully_received == false) { // TODO: does the last by need to be received?
+      restart_reassembly_timer = true;
+    }
+    if (restart_reassembly_timer) {
+      reassembly_timer.run();
+      rx_next_status_trigger = rx_next_highest;
+    }
   }
 }
 
@@ -419,7 +446,7 @@ uint32_t rlc_am_nr_rx::get_status_pdu(rlc_am_nr_status_pdu_t* status, uint32_t m
   }
 
   status->N_nack = 0;
-  status->ack_sn = rx_highest_status; // ACK RX_Highest_Status
+  status->ack_sn = rx_next; // Start with the lower end of the window
   byte_buffer_t tmp_buf;
   uint32_t      len;
 
@@ -438,6 +465,7 @@ uint32_t rlc_am_nr_rx::get_status_pdu(rlc_am_nr_status_pdu_t* status, uint32_t m
     // TODO
     i = (i + 1) % MOD;
   }
+
   if (max_len != UINT32_MAX) {
     status_prohibit_timer.run(); // UINT32_MAX is used just to querry the status PDU length
   }

lib/src/rlc/rlc_am_nr_packing.cc

@@ -229,7 +229,7 @@ int32_t rlc_am_nr_write_status_pdu(const rlc_am_nr_status_pdu_t& status_pdu,
       ptr++;
 
       // write remaining 4 bits of NACK_SN
-      *ptr = status_pdu.nacks[0].nack_sn & 0xf0;
+      *ptr = (status_pdu.nacks[0].nack_sn & 0x0f) << 4;
       ptr++;
     }
   } else {

lib/test/rlc/rlc_am_nr_test.cc

@@ -53,9 +53,11 @@ int basic_test_tx(rlc_am* rlc, byte_buffer_t pdu_bufs[NBUFS])
 
 /*
  * Test the transmission and acknowledgement of 5 SDUs.
- * These are transmitted as single PDUs.
- * There is no lost PDUs, and the byte size is small, so the Poll_PDU configuration
+ *
+ * Each SDU is transmitted as a single PDU.
+ * There are no lost PDUs, and the byte size is small, so the Poll_PDU configuration
  * will trigger the status report.
+ * Poll PDU is configured to 4, so the 5th PDU should set the polling bit.
  */
 int basic_test()
 {
@@ -100,12 +102,9 @@ int basic_test()
   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()));
@@ -150,24 +149,52 @@ int lost_pdu_test()
     }
   }
 
-  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());
+  // 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);
+  }
 
-  // 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));
+  // Step timers until reassambly timeout expires
+  for (int cnt = 0; cnt < 35; cnt++) {
+    timers.step_all();
+  }
 
-  // Write status PDU to RLC1
-  rlc1.write_pdu(status_buf.msg, status_buf.N_bytes);
-  // Check PDCP notifications
-  // TODO
+  // 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(0 == rlc1.get_buffer_state());
+  }
 
   // Check statistics
   TESTASSERT(rx_is_tx(rlc1.get_metrics(), rlc2.get_metrics()));