RLC failure to deliver notifications and discard timer limits to 1500ms (#2368)

* Added interfaces for the RLC to notify the PDCP of failure to transmit SDU * Limit discard timer to 1500ms, to avoid issues of lingering SDUs in the undeliverd_sdus_queue. * Fix bug in early exit of notify_delivery and notify_failure * fix compilation issue in rlc-pdcp notification Co-authored-by: Francisco <francisco.paisana@softwareradiosystems.com>
4 years ago · d8f3878ce1
parent 2f75abcc7a
commit d8f3878ce1
23 changed files with 102 additions and 17 deletions
--- a/lib/include/srslte/interfaces/enb_pdcp_interfaces.h
+++ b/lib/include/srslte/interfaces/enb_pdcp_interfaces.h
@ -54,6 +54,7 @@ public:
  /* RLC calls PDCP to push a PDCP PDU. */
  virtual void write_pdu(uint16_t rnti, uint32_t lcid, srslte::unique_byte_buffer_t pdu)            = 0;
  virtual void notify_delivery(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& pdcp_sns) = 0;
+  virtual void notify_failure(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& pdcp_sns)  = 0;
 };

 } // namespace srsenb
--- a/lib/include/srslte/interfaces/gnb_interfaces.h
+++ b/lib/include/srslte/interfaces/gnb_interfaces.h
@ -84,6 +84,7 @@ public:
  /* RLC calls PDCP to push a PDCP PDU. */
  virtual void write_pdu(uint16_t rnti, uint32_t lcid, srslte::unique_byte_buffer_t sdu)            = 0;
  virtual void notify_delivery(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& tx_count) = 0;
+  virtual void notify_failure(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& tx_count)  = 0;
 };

 class pdcp_interface_rrc_nr
--- a/lib/include/srslte/interfaces/ue_interfaces.h
+++ b/lib/include/srslte/interfaces/ue_interfaces.h
@ -306,6 +306,7 @@ public:
  virtual void write_pdu_pcch(srslte::unique_byte_buffer_t sdu)                     = 0;
  virtual void write_pdu_mch(uint32_t lcid, srslte::unique_byte_buffer_t sdu)       = 0;
  virtual void notify_delivery(uint32_t lcid, const std::vector<uint32_t>& pdcp_sn) = 0;
+  virtual void notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sn)  = 0;
 };

 class pdcp_interface_gw
--- a/lib/include/srslte/upper/pdcp.h
+++ b/lib/include/srslte/upper/pdcp.h
@ -59,6 +59,7 @@ public:
  void write_pdu_bcch_dlsch(unique_byte_buffer_t sdu) override;
  void write_pdu_pcch(unique_byte_buffer_t sdu) override;
  void notify_delivery(uint32_t lcid, const std::vector<uint32_t>& pdcp_sn) override;
+  void notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sn) override;

  // eNB-only methods
  std::map<uint32_t, srslte::unique_byte_buffer_t> get_buffered_pdus(uint32_t lcid);
--- a/lib/include/srslte/upper/pdcp_entity_base.h
+++ b/lib/include/srslte/upper/pdcp_entity_base.h
@ -113,7 +113,8 @@ public:

  // RLC interface
  virtual void write_pdu(unique_byte_buffer_t pdu)                    = 0;
-  virtual void notify_delivery(const std::vector<uint32_t>& tx_count) = 0;
+  virtual void notify_delivery(const std::vector<uint32_t>& pdcp_sns) = 0;
+  virtual void notify_failure(const std::vector<uint32_t>& pdcp_sns)  = 0;

  virtual void get_bearer_state(pdcp_lte_state_t* state)       = 0;
  virtual void set_bearer_state(const pdcp_lte_state_t& state) = 0;
--- a/lib/include/srslte/upper/pdcp_entity_lte.h
+++ b/lib/include/srslte/upper/pdcp_entity_lte.h
@ -53,6 +53,7 @@ public:

  // RLC interface
  void write_pdu(unique_byte_buffer_t pdu) override;
+  void notify_failure(const std::vector<uint32_t>& pdcp_sns) override;
  void notify_delivery(const std::vector<uint32_t>& pdcp_sns) override;

  // Config helpers
--- a/lib/include/srslte/upper/pdcp_entity_nr.h
+++ b/lib/include/srslte/upper/pdcp_entity_nr.h
@ -50,6 +50,7 @@ public:
  // RLC interface
  void write_pdu(unique_byte_buffer_t pdu) final;
  void notify_delivery(const std::vector<uint32_t>& tx_count) final;
+  void notify_failure(const std::vector<uint32_t>& tx_count) final;

  // State variable setters (should be used only for testing)
  void set_tx_next(uint32_t tx_next_) { tx_next = tx_next_; }
--- a/lib/src/upper/pdcp.cc
+++ b/lib/src/upper/pdcp.cc
@ -306,6 +306,15 @@ void pdcp::notify_delivery(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns)
  }
 }

+void pdcp::notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns)
+{
+  if (valid_lcid(lcid)) {
+    pdcp_array.at(lcid)->notify_failure(pdcp_sns);
+  } else {
+    logger.warning("Could not notify failure: lcid=%d, nof_sn=%ld.", lcid, pdcp_sns.size());
+  }
+}
+
 bool pdcp::valid_lcid(uint32_t lcid)
 {
  if (lcid >= SRSLTE_N_RADIO_BEARERS) {
--- a/lib/src/upper/pdcp_entity_lte.cc
+++ b/lib/src/upper/pdcp_entity_lte.cc
@ -60,6 +60,13 @@ pdcp_entity_lte::pdcp_entity_lte(srsue::rlc_interface_pdcp* rlc_,
  // Queue Helpers
  maximum_allocated_sns_window = (1 << cfg.sn_len) / 2;

+  if (is_drb() && not rlc->rb_is_um(lcid) && cfg.discard_timer == pdcp_discard_timer_t::infinity) {
+    logger.warning(
+        "Setting discard timer to 1500ms, to avoid issues with lingering SDUs in the Unacknowledged SDUs map. LCID=%d",
+        lcid);
+    cfg.discard_timer = pdcp_discard_timer_t::ms1500;
+  }
+
  logger.info("Init %s with bearer ID: %d", rrc->get_rb_name(lcid).c_str(), cfg.bearer_id);
  logger.info("SN len bits: %d, SN len bytes: %d, reordering window: %d, Maximum SN: %d, discard timer: %d ms",
              cfg.sn_len,
@ -642,31 +649,48 @@ void pdcp_entity_lte::discard_callback::operator()(uint32_t timer_id)
 }

 /****************************************************************************
- * Handle delivery notifications from RLC
+ * Handle delivery/failure notifications from RLC
 ***************************************************************************/
 void pdcp_entity_lte::notify_delivery(const std::vector<uint32_t>& pdcp_sns)
 {
  logger.info("Received delivery notification from RLC. Number of PDU notified=%ld", pdcp_sns.size());

  for (uint32_t sn : pdcp_sns) {
-    logger.debug("Received delivery notification for SN=%d", sn);
+    logger.debug("Delivery notification received for PDU with SN=%d", sn);
    // Find undelivered PDU info
    std::map<uint32_t, unique_byte_buffer_t>::iterator it = undelivered_sdus_queue.find(sn);
    if (it == undelivered_sdus_queue.end()) {
      logger.warning("Could not find PDU for delivery notification. Notified SN=%d", sn);
-      return;
+    } else {
+      // Metrics
+      tx_pdu_ack_latency_ms.push(std::chrono::duration_cast<std::chrono::milliseconds>(
+                                     std::chrono::high_resolution_clock::now() - it->second->get_timestamp())
+                                     .count());
+      metrics.num_tx_acked_bytes += it->second->N_bytes;
+      metrics.num_tx_buffered_pdus_bytes -= it->second->N_bytes;
+
+      // Remove PDU and disarm timer.
+      undelivered_sdus_queue.erase(sn);
+      stop_discard_timer(sn);
    }
+  }
+}

-    // Metrics
-    tx_pdu_ack_latency_ms.push(std::chrono::duration_cast<std::chrono::milliseconds>(
-                                   std::chrono::high_resolution_clock::now() - it->second->get_timestamp())
-                                   .count());
-    metrics.num_tx_acked_bytes += it->second->N_bytes;
-    metrics.num_tx_buffered_pdus_bytes -= it->second->N_bytes;
+void pdcp_entity_lte::notify_failure(const std::vector<uint32_t>& pdcp_sns)
+{
+  logger.info("Received failure notification from RLC. Number of PDU notified=%ld", pdcp_sns.size());

-    // If ACK'ed bytes are equal to (or exceed) PDU size, remove PDU and disarm timer.
-    undelivered_sdus_queue.erase(sn);
-    stop_discard_timer(sn);
+  for (uint32_t sn : pdcp_sns) {
+    logger.info("Failure notification received for PDU with SN=%d", sn);
+    // Find undelivered PDU info
+    std::map<uint32_t, unique_byte_buffer_t>::iterator it = undelivered_sdus_queue.find(sn);
+    if (it == undelivered_sdus_queue.end()) {
+      logger.info("Could not find PDU for failure notification. Notified SN=%d", sn);
+    } else {
+      // Remove PDU and disarm timer.
+      undelivered_sdus_queue.erase(sn);
+      stop_discard_timer(sn);
+    }
  }
 }

--- a/lib/src/upper/pdcp_entity_nr.cc
+++ b/lib/src/upper/pdcp_entity_nr.cc
@ -202,12 +202,17 @@ void pdcp_entity_nr::write_pdu(unique_byte_buffer_t pdu)
  }
 }

-// Notification of delivery
+// Notification of delivery/failure
 void pdcp_entity_nr::notify_delivery(const std::vector<uint32_t>& pdcp_sns)
 {
  logger.debug("Received delivery notification from RLC. Nof SNs=%ld", pdcp_sns.size());
 }

+void pdcp_entity_nr::notify_failure(const std::vector<uint32_t>& pdcp_sns)
+{
+  logger.debug("Received failure notification from RLC. Nof SNs=%ld", pdcp_sns.size());
+}
+
 /*
 * Packing / Unpacking Helpers
 */
--- a/lib/src/upper/rlc_am_lte.cc
+++ b/lib/src/upper/rlc_am_lte.cc
@ -626,6 +626,7 @@ int rlc_am_lte::rlc_am_lte_tx::build_retx_pdu(uint8_t* payload, uint32_t nof_byt
  if (tx_window[retx.sn].retx_count >= cfg.max_retx_thresh) {
    logger.warning("%s Signaling max number of reTx=%d for for SN=%d", RB_NAME, tx_window[retx.sn].retx_count, retx.sn);
    parent->rrc->max_retx_attempted();
+    parent->pdcp->notify_failure(parent->lcid, tx_window[retx.sn].pdcp_sns);
  }

  logger.info(payload,
@ -873,11 +874,11 @@ int rlc_am_lte::rlc_am_lte_tx::build_data_pdu(uint8_t* payload, uint32_t nof_byt
      logger.error("Could not find PDCP SN in SDU info queue (segment). PDCP_SN=%d", tx_sdu->md.pdcp_sn);
      return 0;
    }
-    undelivered_sdu_info_queue.at(tx_sdu->md.pdcp_sn).rlc_sn_info_list.push_back({header.sn, false});
+    info_it->second.rlc_sn_info_list.push_back({header.sn, false});
    pdcp_sns.push_back(tx_sdu->md.pdcp_sn);
    if (tx_sdu->N_bytes == 0) {
      logger.debug("%s Complete SDU scheduled for tx.", RB_NAME);
-      undelivered_sdu_info_queue[tx_sdu->md.pdcp_sn].fully_txed = true;
+      info_it->second.fully_txed = true;
      tx_sdu.reset();
    }
    if (pdu_space > to_move) {
--- a/lib/test/upper/rlc_am_test.cc
+++ b/lib/test/upper/rlc_am_test.cc
@ -64,6 +64,11 @@ public:
      notified_counts[pdcp_sn] += 1;
    }
  }
+  void notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sn_vec)
+  {
+    assert(lcid == 1);
+    // TODO
+  }

  // RRC interface
  void        max_retx_attempted() {}
--- a/lib/test/upper/rlc_common_test.cc
+++ b/lib/test/upper/rlc_common_test.cc
@ -47,6 +47,7 @@ public:
    sdus[n_sdus++] = std::move(sdu);
  }
  void notify_delivery(uint32_t lcid, const std::vector<uint32_t>& pdcp_sn) {}
+  void notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sn) {}
  void write_pdu_bcch_bch(unique_byte_buffer_t sdu) {}
  void write_pdu_bcch_dlsch(unique_byte_buffer_t sdu) {}
  void write_pdu_pcch(unique_byte_buffer_t sdu) {}
--- a/lib/test/upper/rlc_stress_test.cc
+++ b/lib/test/upper/rlc_stress_test.cc
@ -352,6 +352,7 @@ public:
  void write_pdu_pcch(unique_byte_buffer_t sdu) {}
  void write_pdu_mch(uint32_t lcid, srslte::unique_byte_buffer_t sdu) {}
  void notify_delivery(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns) {}
+  void notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns) {}

  // RRC interface
  void        max_retx_attempted() {}
--- a/lib/test/upper/rlc_test_common.h
+++ b/lib/test/upper/rlc_test_common.h
@ -50,6 +50,7 @@ public:
  void write_pdu_pcch(unique_byte_buffer_t sdu) {}
  void write_pdu_mch(uint32_t lcid, srslte::unique_byte_buffer_t sdu) { sdus.push_back(std::move(sdu)); }
  void notify_delivery(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns) {}
+  void notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns) {}

  // RRC interface
  void        max_retx_attempted() {}
--- a/srsenb/hdr/stack/upper/pdcp.h
+++ b/srsenb/hdr/stack/upper/pdcp.h
@ -38,6 +38,7 @@ public:
  // pdcp_interface_rlc
  void write_pdu(uint16_t rnti, uint32_t lcid, srslte::unique_byte_buffer_t sdu) override;
  void notify_delivery(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& pdcp_sn) override;
+  void notify_failure(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& pdcp_sn) override;
  void write_pdu_mch(uint32_t lcid, srslte::unique_byte_buffer_t sdu) {}

  // pdcp_interface_rrc
--- a/srsenb/hdr/stack/upper/pdcp_nr.h
+++ b/srsenb/hdr/stack/upper/pdcp_nr.h
@ -41,6 +41,7 @@ public:
  // pdcp_interface_rlc_nr
  void write_pdu(uint16_t rnti, uint32_t lcid, srslte::unique_byte_buffer_t sdu);
  void notify_delivery(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& tx_count);
+  void notify_failure(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& tx_count);
  void write_pdu_mch(uint32_t lcid, srslte::unique_byte_buffer_t sdu) {}

  // pdcp_interface_rrc_nr
--- a/srsenb/hdr/stack/upper/rlc.h
+++ b/srsenb/hdr/stack/upper/rlc.h
@ -38,7 +38,7 @@ class rlc : public rlc_interface_mac, public rlc_interface_rrc, public rlc_inter
 public:
  explicit rlc(srslog::basic_logger& logger) : logger(logger) {}
  void
-       init(pdcp_interface_rlc* pdcp_, rrc_interface_rlc* rrc_, mac_interface_rlc* mac_, srslte::timer_handler* timers_);
+  init(pdcp_interface_rlc* pdcp_, rrc_interface_rlc* rrc_, mac_interface_rlc* mac_, srslte::timer_handler* timers_);
  void stop();
  void get_metrics(rlc_metrics_t& m, const uint32_t nof_tti);

@ -72,6 +72,7 @@ private:
  public:
    void        write_pdu(uint32_t lcid, srslte::unique_byte_buffer_t sdu);
    void        notify_delivery(uint32_t lcid, const std::vector<uint32_t>& tx_count);
+    void        notify_failure(uint32_t lcid, const std::vector<uint32_t>& tx_count);
    void        write_pdu_bcch_bch(srslte::unique_byte_buffer_t sdu);
    void        write_pdu_bcch_dlsch(srslte::unique_byte_buffer_t sdu);
    void        write_pdu_pcch(srslte::unique_byte_buffer_t sdu);
--- a/srsenb/hdr/stack/upper/rlc_nr.h
+++ b/srsenb/hdr/stack/upper/rlc_nr.h
@ -64,6 +64,7 @@ private:
  public:
    void        write_pdu(uint32_t lcid, srslte::unique_byte_buffer_t sdu);
    void        notify_delivery(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns);
+    void        notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns);
    void        write_pdu_bcch_bch(srslte::unique_byte_buffer_t sdu);
    void        write_pdu_bcch_dlsch(srslte::unique_byte_buffer_t sdu);
    void        write_pdu_pcch(srslte::unique_byte_buffer_t sdu);
--- a/srsenb/src/stack/upper/pdcp.cc
+++ b/srsenb/src/stack/upper/pdcp.cc
@ -149,6 +149,14 @@ void pdcp::notify_delivery(uint16_t rnti, uint32_t lcid, const std::vector<uint3
    users[rnti].pdcp->notify_delivery(lcid, pdcp_sns);
  }
 }
+
+void pdcp::notify_failure(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& pdcp_sns)
+{
+  if (users.count(rnti)) {
+    users[rnti].pdcp->notify_failure(lcid, pdcp_sns);
+  }
+}
+
 void pdcp::write_sdu(uint16_t rnti, uint32_t lcid, srslte::unique_byte_buffer_t sdu, int pdcp_sn)
 {
  if (users.count(rnti)) {
--- a/srsenb/src/stack/upper/pdcp_nr.cc
+++ b/srsenb/src/stack/upper/pdcp_nr.cc
@ -108,6 +108,15 @@ void pdcp_nr::notify_delivery(uint16_t rnti, uint32_t lcid, const std::vector<ui
  }
 }

+void pdcp_nr::notify_failure(uint16_t rnti, uint32_t lcid, const std::vector<uint32_t>& pdcp_sns)
+{
+  if (users.count(rnti)) {
+    users[rnti].pdcp->notify_failure(lcid, pdcp_sns);
+  } else {
+    m_log->error("Can't notify Ack of PDU. RNTI=0x%X doesn't exist.\n", rnti);
+  }
+}
+
 void pdcp_nr::write_sdu(uint16_t rnti, uint32_t lcid, srslte::unique_byte_buffer_t sdu)
 {
  if (users.count(rnti)) {
--- a/srsenb/src/stack/upper/rlc.cc
+++ b/srsenb/src/stack/upper/rlc.cc
@ -274,6 +274,11 @@ void rlc::user_interface::notify_delivery(uint32_t lcid, const std::vector<uint3
  pdcp->notify_delivery(rnti, lcid, pdcp_sns);
 }

+void rlc::user_interface::notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns)
+{
+  pdcp->notify_failure(rnti, lcid, pdcp_sns);
+}
+
 void rlc::user_interface::write_pdu_bcch_bch(srslte::unique_byte_buffer_t sdu)
 {
  ERROR("Error: Received BCCH from ue=%d", rnti);
--- a/srsenb/src/stack/upper/rlc_nr.cc
+++ b/srsenb/src/stack/upper/rlc_nr.cc
@ -212,4 +212,9 @@ void rlc_nr::user_interface::notify_delivery(uint32_t lcid, const std::vector<ui
  m_pdcp->notify_delivery(rnti, lcid, pdcp_sns);
 }

+void rlc_nr::user_interface::notify_failure(uint32_t lcid, const std::vector<uint32_t>& pdcp_sns)
+{
+  m_pdcp->notify_failure(rnti, lcid, pdcp_sns);
+}
+
 } // namespace srsenb