rlc: revisit interface for MAC and RRC

* add locked and unlocked version of has_data() since one is
  called from stack and one from PHY threads

* add comments in each interface section as to why locking
  is required or not

* remove RLC rwlock when not required

* move calls only used by RRC to RRC section

lib/include/srslte/interfaces/ue_interfaces.h

@@ -305,7 +305,7 @@ class rlc_interface_mac : public srslte::read_pdu_interface
 public:
   /* MAC calls has_data() to query whether a logical channel has data to transmit (without
    * knowing how much. This function should return quickly. */
-  virtual bool has_data(const uint32_t lcid) = 0;
+  virtual bool has_data_locked(const uint32_t lcid) = 0;
 
   /* MAC calls RLC to get the buffer state for a logical channel. */
   virtual uint32_t get_buffer_state(const uint32_t lcid) = 0;

lib/include/srslte/test/ue_test_interfaces.h

@@ -55,7 +55,7 @@ public:
 class rlc_dummy_interface : public rlc_interface_mac
 {
 public:
-  bool     has_data(const uint32_t lcid) override { return false; }
+  bool     has_data_locked(const uint32_t lcid) override { return false; }
   uint32_t get_buffer_state(const uint32_t lcid) override { return 0; }
   int      read_pdu(uint32_t lcid, uint8_t* payload, uint32_t nof_bytes) override { return 0; }
   void     write_pdu(uint32_t lcid, uint8_t* payload, uint32_t nof_bytes) override {}

lib/include/srslte/upper/rlc.h

@@ -63,8 +63,7 @@ public:
   bool sdu_queue_is_full(uint32_t lcid);
 
   // MAC interface
-  bool     has_data(const uint32_t lcid);
+  bool     has_data_locked(const uint32_t lcid);
-  bool     is_suspended(const uint32_t lcid);
   uint32_t get_buffer_state(const uint32_t lcid);
   uint32_t get_total_mch_buffer_state(uint32_t lcid);
   int      read_pdu(uint32_t lcid, uint8_t* payload, uint32_t nof_bytes);
@@ -77,6 +76,8 @@ public:
   void     write_pdu_mch(uint32_t lcid, uint8_t* payload, uint32_t nof_bytes);
 
   // RRC interface
+  bool is_suspended(const uint32_t lcid);
+  bool has_data(const uint32_t lcid);
   void reestablish();
   void reestablish(uint32_t lcid);
   void reset();

lib/src/upper/rlc.cc

@@ -182,7 +182,7 @@ void rlc::empty_queue()
 }
 
 /*******************************************************************************
-  PDCP interface
+  PDCP interface (called from Stack thread and therefore no lock required)
 *******************************************************************************/
 
 void rlc::write_sdu(uint32_t lcid, unique_byte_buffer_t sdu)
@@ -244,27 +244,12 @@ bool rlc::sdu_queue_is_full(uint32_t lcid)
 }
 
 /*******************************************************************************
-  MAC interface
+  MAC interface (mostly called from PHY workers, lock needs to be hold)
 *******************************************************************************/
-bool rlc::has_data(uint32_t lcid)
+bool rlc::has_data_locked(const uint32_t lcid)
-{
-  bool has_data = false;
-
-  if (valid_lcid(lcid)) {
-    has_data = rlc_array.at(lcid)->has_data();
-  }
-
-  return has_data;
-}
-bool rlc::is_suspended(const uint32_t lcid)
 {
-  bool ret = false;
+  rwlock_read_guard lock(rwlock);
-
+  return has_data(lcid);
-  if (valid_lcid(lcid)) {
-    ret = rlc_array.at(lcid)->is_suspended();
-  }
-
-  return ret;
 }
 
 uint32_t rlc::get_buffer_state(uint32_t lcid)
@@ -288,7 +273,6 @@ uint32_t rlc::get_total_mch_buffer_state(uint32_t lcid)
   uint32_t ret = 0;
 
   rwlock_read_guard lock(rwlock);
-
   if (valid_lcid_mrb(lcid)) {
     ret = rlc_array_mrb.at(lcid)->get_buffer_state();
   }
@@ -326,6 +310,7 @@ int rlc::read_pdu_mch(uint32_t lcid, uint8_t* payload, uint32_t nof_bytes)
   return ret;
 }
 
+// Write PDU methods are called from Stack thread context, no need to acquire the lock
 void rlc::write_pdu(uint32_t lcid, uint8_t* payload, uint32_t nof_bytes)
 {
   if (valid_lcid(lcid)) {
@@ -373,9 +358,31 @@ void rlc::write_pdu_mch(uint32_t lcid, uint8_t* payload, uint32_t nof_bytes)
 }
 
 /*******************************************************************************
-  RRC interface
+  RRC interface (write-lock ONLY needs to be hold for all calls modifying the RLC array)
 *******************************************************************************/
+bool rlc::is_suspended(const uint32_t lcid)
+{
+  bool ret = false;
+
+  if (valid_lcid(lcid)) {
+    ret = rlc_array.at(lcid)->is_suspended();
+  }
 
+  return ret;
+}
+
+bool rlc::has_data(uint32_t lcid)
+{
+  bool has_data = false;
+
+  if (valid_lcid(lcid)) {
+    has_data = rlc_array.at(lcid)->has_data();
+  }
+
+  return has_data;
+}
+
+// Methods modifying the RLC array need to acquire the write-lock
 void rlc::add_bearer(uint32_t lcid, const rlc_config_t& cnfg)
 {
   rwlock_write_guard lock(rwlock);
@@ -533,6 +540,7 @@ void rlc::change_lcid(uint32_t old_lcid, uint32_t new_lcid)
   }
 }
 
+// Further RRC calls executed from Stack thread, no need to hold lock
 void rlc::suspend_bearer(uint32_t lcid)
 {
   if (valid_lcid(lcid)) {
@@ -569,7 +577,6 @@ bool rlc::has_bearer(uint32_t lcid)
 /*******************************************************************************
   Helpers (Lock must be hold when calling those)
 *******************************************************************************/
-
 bool rlc::valid_lcid(uint32_t lcid)
 {
   if (lcid >= SRSLTE_N_RADIO_BEARERS) {

srsue/src/stack/mac/mux.cc

@@ -74,7 +74,7 @@ void mux::step()
 bool mux::is_pending_any_sdu()
 {
   for (auto& channel : logical_channels) {
-    if (rlc->has_data(channel.lcid)) {
+    if (rlc->has_data_locked(channel.lcid)) {
       return true;
     }
   }

srsue/test/mac_test.cc

@@ -48,7 +48,7 @@ class rlc_dummy : public srsue::rlc_dummy_interface
 {
 public:
   rlc_dummy(srslte::log_filter* log_) : received_bytes(0), log(log_) {}
-  bool     has_data(const uint32_t lcid) final { return ul_queues[lcid] > 0; }
+  bool     has_data_locked(const uint32_t lcid) final { return ul_queues[lcid] > 0; }
   uint32_t get_buffer_state(const uint32_t lcid) final { return ul_queues[lcid]; }
   int      read_pdu(uint32_t lcid, uint8_t* payload, uint32_t nof_bytes) final
   {