removed the uneeded locks from the pdcp

lib/include/srslte/upper/pdcp.h

@@ -74,8 +74,8 @@ private:
   srslte::timer_handler* timers   = nullptr;
   log*                   pdcp_log = nullptr;
   pdcp_map_t             pdcp_array, pdcp_array_mrb;
-  pthread_rwlock_t       rwlock;
 
+  // cache valid lcids to be checked from separate thread
   std::mutex         cache_mutex;
   std::set<uint32_t> valid_lcids_cached;
 

lib/include/srslte/upper/pdcp_entity_base.h

@@ -30,7 +30,6 @@
 #include "srslte/common/threads.h"
 #include "srslte/common/timers.h"
 #include "srslte/upper/pdcp_config.h"
-#include <mutex>
 
 namespace srslte {
 
@@ -128,8 +127,6 @@ protected:
                        pdcp_t_reordering_t::ms500,
                        pdcp_discard_timer_t::infinity};
 
-  std::mutex mutex;
-
   srslte::as_security_config_t sec_cfg = {};
 
   // Security functions

lib/src/upper/pdcp.cc

@@ -23,10 +23,7 @@
 
 namespace srslte {
 
-pdcp::pdcp(srslte::timer_handler* timers_, srslte::log* log_) : timers(timers_), pdcp_log(log_)
+pdcp::pdcp(srslte::timer_handler* timers_, srslte::log* log_) : timers(timers_), pdcp_log(log_) {}
-{
-  pthread_rwlock_init(&rwlock, NULL);
-}
 
 pdcp::~pdcp()
 {
@@ -35,7 +32,6 @@ pdcp::~pdcp()
     valid_lcids_cached.clear();
   }
   // destroy all remaining entities
-  pthread_rwlock_wrlock(&rwlock);
   for (pdcp_map_t::iterator it = pdcp_array.begin(); it != pdcp_array.end(); ++it) {
     delete (it->second);
   }
@@ -45,9 +41,6 @@ pdcp::~pdcp()
     delete (it->second);
   }
   pdcp_array_mrb.clear();
-
-  pthread_rwlock_unlock(&rwlock);
-  pthread_rwlock_destroy(&rwlock);
 }
 
 void pdcp::init(srsue::rlc_interface_pdcp* rlc_, srsue::rrc_interface_pdcp* rrc_, srsue::gw_interface_pdcp* gw_)
@@ -61,20 +54,16 @@ void pdcp::stop() {}
 
 void pdcp::reestablish()
 {
-  pthread_rwlock_rdlock(&rwlock);
   for (pdcp_map_t::iterator it = pdcp_array.begin(); it != pdcp_array.end(); ++it) {
     it->second->reestablish();
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::reestablish(uint32_t lcid)
 {
-  pthread_rwlock_rdlock(&rwlock);
   if (valid_lcid(lcid)) {
     pdcp_array.at(lcid)->reestablish();
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::reset()
@@ -84,13 +73,11 @@ void pdcp::reset()
     valid_lcids_cached.clear();
   }
   // destroy all bearers
-  pthread_rwlock_wrlock(&rwlock);
   for (pdcp_map_t::iterator it = pdcp_array.begin(); it != pdcp_array.end(); /* post increment in erase */) {
     it->second->reset();
     delete (it->second);
     pdcp_array.erase(it++);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 /*******************************************************************************
@@ -106,31 +93,26 @@ bool pdcp::is_lcid_enabled(uint32_t lcid)
 
 void pdcp::write_sdu(uint32_t lcid, unique_byte_buffer_t sdu, bool blocking)
 {
-  pthread_rwlock_rdlock(&rwlock);
   if (valid_lcid(lcid)) {
     pdcp_array.at(lcid)->write_sdu(std::move(sdu), blocking);
   } else {
     pdcp_log->warning("Writing sdu: lcid=%d. Deallocating sdu\n", lcid);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::write_sdu_mch(uint32_t lcid, unique_byte_buffer_t sdu)
 {
-  pthread_rwlock_rdlock(&rwlock);
   if (valid_mch_lcid(lcid)) {
     pdcp_array_mrb.at(lcid)->write_sdu(std::move(sdu), true);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::add_bearer(uint32_t lcid, pdcp_config_t cfg)
 {
-  pthread_rwlock_wrlock(&rwlock);
   if (not valid_lcid(lcid)) {
     if (not pdcp_array.insert(pdcp_map_pair_t(lcid, new pdcp_entity_lte(rlc, rrc, gw, timers, pdcp_log))).second) {
       pdcp_log->error("Error inserting PDCP entity in to array\n.");
-      goto unlock_and_exit;
+      return;
     }
     pdcp_array.at(lcid)->init(lcid, cfg);
     pdcp_log->info("Add %s (lcid=%d, bearer_id=%d, sn_len=%dbits)\n",
@@ -145,17 +127,14 @@ void pdcp::add_bearer(uint32_t lcid, pdcp_config_t cfg)
   } else {
     pdcp_log->warning("Bearer %s already configured. Reconfiguration not supported\n", rrc->get_rb_name(lcid).c_str());
   }
-unlock_and_exit:
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::add_bearer_mrb(uint32_t lcid, pdcp_config_t cfg)
 {
-  pthread_rwlock_wrlock(&rwlock);
   if (not valid_mch_lcid(lcid)) {
     if (not pdcp_array_mrb.insert(pdcp_map_pair_t(lcid, new pdcp_entity_lte(rlc, rrc, gw, timers, pdcp_log))).second) {
       pdcp_log->error("Error inserting PDCP entity in to array\n.");
-      goto unlock_and_exit;
+      return;
     }
     pdcp_array_mrb.at(lcid)->init(lcid, cfg);
     pdcp_log->info("Add %s (lcid=%d, bearer_id=%d, sn_len=%dbits)\n",
@@ -166,8 +145,6 @@ void pdcp::add_bearer_mrb(uint32_t lcid, pdcp_config_t cfg)
   } else {
     pdcp_log->warning("Bearer %s already configured. Reconfiguration not supported\n", rrc->get_rb_name(lcid).c_str());
   }
-unlock_and_exit:
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::del_bearer(uint32_t lcid)
@@ -176,7 +153,6 @@ void pdcp::del_bearer(uint32_t lcid)
     std::lock_guard<std::mutex> lock(cache_mutex);
     valid_lcids_cached.erase(lcid);
   }
-  pthread_rwlock_wrlock(&rwlock);
   if (valid_lcid(lcid)) {
     pdcp_map_t::iterator it = pdcp_array.find(lcid);
     delete (it->second);
@@ -185,13 +161,10 @@ void pdcp::del_bearer(uint32_t lcid)
   } else {
     pdcp_log->warning("Can't delete bearer %s. Bearer doesn't exist.\n", rrc->get_rb_name(lcid).c_str());
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::change_lcid(uint32_t old_lcid, uint32_t new_lcid)
 {
-  pthread_rwlock_wrlock(&rwlock);
-
   // make sure old LCID exists and new LCID is still free
   if (valid_lcid(old_lcid) && not valid_lcid(new_lcid)) {
     // insert old PDCP entity into new LCID
@@ -199,7 +172,7 @@ void pdcp::change_lcid(uint32_t old_lcid, uint32_t new_lcid)
     pdcp_entity_lte*     pdcp_entity = it->second;
     if (not pdcp_array.insert(pdcp_map_pair_t(new_lcid, pdcp_entity)).second) {
       pdcp_log->error("Error inserting PDCP entity into array\n.");
-      goto exit;
+      return;
     }
     {
       std::lock_guard<std::mutex> lock(cache_mutex);
@@ -216,44 +189,34 @@ void pdcp::change_lcid(uint32_t old_lcid, uint32_t new_lcid)
         old_lcid,
         new_lcid);
   }
-exit:
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::config_security(uint32_t lcid, as_security_config_t sec_cfg)
 {
-  pthread_rwlock_rdlock(&rwlock);
   if (valid_lcid(lcid)) {
     pdcp_array.at(lcid)->config_security(sec_cfg);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::config_security_all(as_security_config_t sec_cfg)
 {
-  pthread_rwlock_rdlock(&rwlock);
   for (pdcp_map_t::iterator it = pdcp_array.begin(); it != pdcp_array.end(); ++it) {
     it->second->config_security(sec_cfg);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::enable_integrity(uint32_t lcid, srslte_direction_t direction)
 {
-  pthread_rwlock_rdlock(&rwlock);
   if (valid_lcid(lcid)) {
     pdcp_array.at(lcid)->enable_integrity(direction);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::enable_encryption(uint32_t lcid, srslte_direction_t direction)
 {
-  pthread_rwlock_rdlock(&rwlock);
   if (valid_lcid(lcid)) {
     pdcp_array.at(lcid)->enable_encryption(direction);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 bool pdcp::get_bearer_status(uint32_t lcid, uint16_t* dlsn, uint16_t* dlhfn, uint16_t* ulsn, uint16_t* ulhfn)
@@ -270,13 +233,11 @@ bool pdcp::get_bearer_status(uint32_t lcid, uint16_t* dlsn, uint16_t* dlhfn, uin
 *******************************************************************************/
 void pdcp::write_pdu(uint32_t lcid, unique_byte_buffer_t pdu)
 {
-  pthread_rwlock_rdlock(&rwlock);
   if (valid_lcid(lcid)) {
     pdcp_array.at(lcid)->write_pdu(std::move(pdu));
   } else {
     pdcp_log->warning("Writing pdu: lcid=%d. Deallocating pdu\n", lcid);
   }
-  pthread_rwlock_unlock(&rwlock);
 }
 
 void pdcp::write_pdu_bcch_bch(unique_byte_buffer_t sdu)

lib/src/upper/pdcp_entity_lte.cc

@@ -40,11 +40,11 @@ pdcp_entity_lte::~pdcp_entity_lte() {}
 
 void pdcp_entity_lte::init(uint32_t lcid_, pdcp_config_t cfg_)
 {
-  lcid          = lcid_;
+  lcid                 = lcid_;
-  cfg           = cfg_;
+  cfg                  = cfg_;
-  active        = true;
+  active               = true;
-  tx_count      = 0;
+  tx_count             = 0;
-  rx_count      = 0;
+  rx_count             = 0;
   integrity_direction  = DIRECTION_NONE;
   encryption_direction = DIRECTION_NONE;
 
@@ -107,29 +107,25 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, bool blocking)
                 srslte_direction_text[integrity_direction],
                 srslte_direction_text[encryption_direction]);
 
-  {
+  write_data_header(sdu, tx_count);
-    std::unique_lock<std::mutex> lock(mutex);
 
-    write_data_header(sdu, tx_count);
+  // Append MAC (SRBs only)
-
+  uint8_t mac[4]       = {};
-    // Append MAC (SRBs only)
+  bool    do_integrity = integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX;
-    uint8_t mac[4] = {};
+  if (do_integrity && is_srb()) {
-    bool do_integrity = integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX;
+    integrity_generate(sdu->msg, sdu->N_bytes, tx_count, mac);
-    if (do_integrity && is_srb()) { 
+  }
-      integrity_generate(sdu->msg, sdu->N_bytes, tx_count, mac);
-    }
 
-    if (is_srb()) {
+  if (is_srb()) {
-      append_mac(sdu, mac);
+    append_mac(sdu, mac);
-    }
+  }
 
-    if (encryption_direction == DIRECTION_TX || encryption_direction == DIRECTION_TXRX) {
+  if (encryption_direction == DIRECTION_TX || encryption_direction == DIRECTION_TXRX) {
-      cipher_encrypt(
+    cipher_encrypt(
-          &sdu->msg[cfg.hdr_len_bytes], sdu->N_bytes - cfg.hdr_len_bytes, tx_count, &sdu->msg[cfg.hdr_len_bytes]);
+        &sdu->msg[cfg.hdr_len_bytes], sdu->N_bytes - cfg.hdr_len_bytes, tx_count, &sdu->msg[cfg.hdr_len_bytes]);
-      log->info_hex(sdu->msg, sdu->N_bytes, "TX %s SDU (encrypted)", rrc->get_rb_name(lcid).c_str());
+    log->info_hex(sdu->msg, sdu->N_bytes, "TX %s SDU (encrypted)", rrc->get_rb_name(lcid).c_str());
-    }
-    tx_count++;
   }
+  tx_count++;
 
   rlc->write_sdu(lcid, std::move(sdu), blocking);
 }
@@ -151,7 +147,6 @@ void pdcp_entity_lte::write_pdu(unique_byte_buffer_t pdu)
     return;
   }
 
-  std::unique_lock<std::mutex> lock(mutex);
   if (is_srb()) {
     handle_srb_pdu(std::move(pdu));
   } else if (is_drb() && rlc->rb_is_um(lcid)) {
@@ -267,7 +262,7 @@ void pdcp_entity_lte::handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu)
              next_pdcp_rx_sn);
 
   // Handle PDU
-  uint32_t count;
+  uint32_t count = 0;
   if ((0 <= sn_diff_last_submit && sn_diff_last_submit > (int32_t)reordering_window) ||
       (0 <= last_submit_diff_sn && last_submit_diff_sn < (int32_t)reordering_window)) {
     log->warning("|SN - last_submitted_sn| is larger than re-ordering window.\n");