Changed notification mechanisms to use map of acked SNs instead of acked bytes.

This was required due to an issue where bytes were counted twice when a
packet had been lost.

lib/include/srslte/upper/rlc_am_lte.h

@@ -54,10 +54,17 @@ struct rlc_amd_retx_t {
   uint32_t so_end;
 };
 
+struct rlc_sn_info_t {
+  uint32_t sn;
+  bool     is_acked;
+};
+
 struct pdcp_sdu_info_t {
   uint32_t                   sn;
-  uint32_t acked_bytes;
+  bool                       fully_txed;       // Boolean indicating if the SDU is fully transmitted.
-  uint32_t total_bytes;
+  uint32_t                   acked_bytes;      // For metrics
+  uint32_t                   total_bytes;      // For metrics
+  std::vector<rlc_sn_info_t> rlc_sn_info_list; // List of RLC PDUs in transit and whether they have been acked or not.
 };
 
 class rlc_am_lte : public rlc_common

lib/src/upper/rlc_am_lte.cc

@@ -384,10 +384,13 @@ int rlc_am_lte::rlc_am_lte_tx::write_sdu(unique_byte_buffer_t sdu)
   if (info_count != 0) {
     log->error("PDCP SDU info alreay exists\n");
     return SRSLTE_ERROR;
-  } else if (undelivered_sdu_info_queue.size() >= pdcp_info_queue_capacity) {
+  }
+
+  if (undelivered_sdu_info_queue.size() >= pdcp_info_queue_capacity) {
     log->error("PDCP SDU info exceeds maximum queue capacity\n");
     return SRSLTE_ERROR;
   }
+
   undelivered_sdu_info_queue[info.sn] = info;
   return SRSLTE_SUCCESS;
 }
@@ -858,8 +861,10 @@ int rlc_am_lte::rlc_am_lte_tx::build_data_pdu(uint8_t* payload, uint32_t nof_byt
     tx_sdu->N_bytes -= to_move;
     tx_sdu->msg += to_move;
     pdcp_tx_counts[0] = tx_sdu->md.pdcp_sn;
+    undelivered_sdu_info_queue[tx_sdu->md.pdcp_sn].rlc_sn_info_list.push_back({header.sn, false});
     if (tx_sdu->N_bytes == 0) {
       log->debug("%s Complete SDU scheduled for tx.\n", RB_NAME);
+      undelivered_sdu_info_queue[tx_sdu->md.pdcp_sn].fully_txed = true;
       tx_sdu.reset();
     }
     if (pdu_space > to_move) {
@@ -898,8 +903,10 @@ int rlc_am_lte::rlc_am_lte_tx::build_data_pdu(uint8_t* payload, uint32_t nof_byt
     tx_sdu->N_bytes -= to_move;
     tx_sdu->msg += to_move;
     pdcp_tx_counts[header.N_li] = tx_sdu->md.pdcp_sn;
+    undelivered_sdu_info_queue[tx_sdu->md.pdcp_sn].rlc_sn_info_list.push_back({header.sn, false});
     if (tx_sdu->N_bytes == 0) {
-      log->debug("%s Complete SDU scheduled for tx.\n", RB_NAME);
+      log->debug("%s Complete SDU scheduled for tx. PDCP SN=%d\n", RB_NAME, tx_sdu->md.pdcp_sn);
+      undelivered_sdu_info_queue[tx_sdu->md.pdcp_sn].fully_txed = true;
       tx_sdu.reset();
     }
     if (pdu_space > to_move) {
@@ -1081,33 +1088,33 @@ void rlc_am_lte::rlc_am_lte_tx::update_notification_ack_info(const rlc_amd_tx_pd
   // Notify PDCP of the number of bytes succesfully delivered
   uint32_t total_acked_bytes = 0;
   uint32_t nof_bytes         = 0;
-  uint32_t pdcp_sn           = 0;
+
-  for (uint32_t pdcp_notify_it = 0; pdcp_notify_it < tx_pdu.header.N_li; pdcp_notify_it++) {
+  // Iterate over all undelivered SDUs
-    nof_bytes = tx_pdu.header.li[pdcp_notify_it];
+  for (auto& info_it : undelivered_sdu_info_queue) {
-    pdcp_sn   = tx_pdu.pdcp_tx_counts[pdcp_notify_it];
+    // Iterate over all SNs that
-    if (undelivered_sdu_info_queue.find(pdcp_sn) == undelivered_sdu_info_queue.end()) {
+    uint32_t pdcp_sn = info_it.first;
-      log->debug("Could not find notification info, perhaps SDU was already ACK'ed.\n");
+    auto&    info    = info_it.second;
-      continue;
+    for (auto& rlc_sn_info : info.rlc_sn_info_list) {
-    }
+      // Mark this SN as acked, if necessary
-    undelivered_sdu_info_queue[pdcp_sn].acked_bytes += nof_bytes;
+      if (rlc_sn_info.is_acked == false && rlc_sn_info.sn == tx_pdu.header.sn) {
-    total_acked_bytes += nof_bytes;
+        rlc_sn_info.is_acked = true;
-    if (undelivered_sdu_info_queue[pdcp_sn].acked_bytes >= undelivered_sdu_info_queue[pdcp_sn].total_bytes) {
+      }
-      undelivered_sdu_info_queue.erase(pdcp_sn);
+    }
+    // Check wether the SDU was fully acked
+    if (info.fully_txed) {
+      // Iterate over all SNs that
+      bool fully_acked = std::all_of(info.rlc_sn_info_list.begin(),
+                                     info.rlc_sn_info_list.end(),
+                                     [](rlc_sn_info_t rlc_sn_info) { return rlc_sn_info.is_acked; });
+      if (fully_acked) {
         notify_info_vec.push_back(pdcp_sn);
-      log->debug("Reporting to PDCP: PDCP SN=%d, nof_bytes=%d\n", pdcp_sn, nof_bytes);
       }
     }
-  pdcp_sn   = tx_pdu.pdcp_tx_counts[tx_pdu.header.N_li];
-  nof_bytes = tx_pdu.buf->N_bytes - total_acked_bytes; // Notify last SDU
-  if (undelivered_sdu_info_queue.find(pdcp_sn) == undelivered_sdu_info_queue.end()) {
-    log->debug("Could not find notification info, perhaps SDU was already ACK'ed.\n");
-    return;
   }
-  undelivered_sdu_info_queue[pdcp_sn].acked_bytes += nof_bytes;
+
-  if (undelivered_sdu_info_queue[pdcp_sn].acked_bytes >= undelivered_sdu_info_queue[pdcp_sn].total_bytes) {
+  // Remove all SDUs that were fully acked
-    undelivered_sdu_info_queue.erase(pdcp_sn);
+  for (uint32_t acked_pdcp_sn : notify_info_vec) {
-    notify_info_vec.push_back(pdcp_sn);
+    undelivered_sdu_info_queue.erase(acked_pdcp_sn);
-    log->debug("Reporting to PDCP: PDCP SN=%d, nof_bytes=%d\n", pdcp_sn, nof_bytes);
   }
 }
 

lib/test/upper/rlc_am_notify_pdcp_test.cc

@@ -357,8 +357,8 @@ int rtxed_sdu_notify_test()
   return SRSLTE_SUCCESS;
 }
 
-// Test out of order ACK for SDU.
+// Test out of order ACK for PDUs.
-// Two sdus are transmitted, and ack arrives out of order
+// Two PDDs are transmitted for one SDU, and ack arrives out of order
 int two_sdus_out_of_order_ack_notify_test()
 {
   srslte::byte_buffer_pool* pool = srslte::byte_buffer_pool::get_instance();
@@ -395,16 +395,9 @@ int two_sdus_out_of_order_ack_notify_test()
   pdu2_buf->N_bytes = rlc.read_pdu(pdu2_buf->msg, 4); // 2 bytes for header + 2 for payload
   TESTASSERT(0 == rlc.get_buffer_state());
 
-  // Feed ack of PDU1 to RLC
-  srslte::rlc_status_pdu_t s1;
-  s1.ack_sn = 1;
-  s1.N_nack = 0;
-
-  // Intentionally do not write first ack to RLC
-
   // Ack of PDU2 to RLC, with PDU1 with NACK
   srslte::rlc_status_nack_t nack = {};
-  nack.nack_sn                   = 1;
+  nack.nack_sn                   = 0;
 
   srslte::rlc_status_pdu_t s2;
   s2.ack_sn   = 2;
@@ -415,18 +408,22 @@ int two_sdus_out_of_order_ack_notify_test()
   sta_buf->N_bytes = srslte::rlc_am_write_status_pdu(&s2, sta_buf->msg);
   rlc.write_pdu(sta_buf->msg, sta_buf->N_bytes);
 
-  // Check PDCP notifications
+  // Check PDCP notifications. No notification should be available yet.
-  TESTASSERT(pdcp.notified_counts.size() == 1);
+  TESTASSERT(pdcp.notified_counts.size() == 0);
-  TESTASSERT(pdcp.notified_counts.find(10) != pdcp.notified_counts.end());
+
-  TESTASSERT(pdcp.notified_counts[10] == 1);
+  // Ack all of PDUs to RLC
+  srslte::rlc_status_pdu_t s1;
+  s1.ack_sn = 2;
+  s1.N_nack = 0;
 
-  // Write first ack (out of order)
+  // Write ack for all PDUs
   sta_buf->N_bytes = srslte::rlc_am_write_status_pdu(&s1, sta_buf->msg);
   rlc.write_pdu(sta_buf->msg, sta_buf->N_bytes);
 
   // Check PDCP notifications
   TESTASSERT(pdcp.notified_counts.size() == 1);
   TESTASSERT(pdcp.notified_counts.find(10) != pdcp.notified_counts.end());
+  std::cout << pdcp.notified_counts[10] << std::endl;
   TESTASSERT(pdcp.notified_counts[10] == 1);
   return SRSLTE_SUCCESS;
 }
@@ -497,10 +494,11 @@ int main(int argc, char** argv)
 {
   srslte::byte_buffer_pool::get_instance();
   TESTASSERT(simple_sdu_notify_test() == SRSLTE_SUCCESS);
-  TESTASSERT(two_pdus_notify_test() == SRSLTE_SUCCESS);
   TESTASSERT(two_sdus_notify_test() == SRSLTE_SUCCESS);
   TESTASSERT(three_sdus_notify_test() == SRSLTE_SUCCESS);
+  TESTASSERT(two_pdus_notify_test() == SRSLTE_SUCCESS);
   TESTASSERT(rtxed_sdu_notify_test() == SRSLTE_SUCCESS);
+  TESTASSERT(two_pdus_out_of_order_ack_notify_test() == SRSLTE_SUCCESS);
   TESTASSERT(two_sdus_out_of_order_ack_notify_test() == SRSLTE_SUCCESS);
   srslte::byte_buffer_pool::cleanup();
   return SRSLTE_SUCCESS;