From 9bde6d442cc9a0b23971d104988ab93b2efca0c3 Mon Sep 17 00:00:00 2001 From: Francisco Paisana Date: Mon, 23 Mar 2020 14:00:12 +0000 Subject: [PATCH] removed the uneeded locks from the pdcp --- lib/include/srslte/upper/pdcp.h | 2 +- lib/include/srslte/upper/pdcp_entity_base.h | 3 -- lib/src/upper/pdcp.cc | 47 ++------------------- lib/src/upper/pdcp_entity_lte.cc | 47 +++++++++------------ 4 files changed, 26 insertions(+), 73 deletions(-) diff --git a/lib/include/srslte/upper/pdcp.h b/lib/include/srslte/upper/pdcp.h index 5aefe5be9..1ec4d08e2 100644 --- a/lib/include/srslte/upper/pdcp.h +++ b/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 valid_lcids_cached; diff --git a/lib/include/srslte/upper/pdcp_entity_base.h b/lib/include/srslte/upper/pdcp_entity_base.h index 241eaee85..b1c169652 100644 --- a/lib/include/srslte/upper/pdcp_entity_base.h +++ b/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 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 diff --git a/lib/src/upper/pdcp.cc b/lib/src/upper/pdcp.cc index 6129bde20..004446d3f 100644 --- a/lib/src/upper/pdcp.cc +++ b/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_) -{ - pthread_rwlock_init(&rwlock, NULL); -} +pdcp::pdcp(srslte::timer_handler* timers_, srslte::log* log_) : timers(timers_), pdcp_log(log_) {} 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 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 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) diff --git a/lib/src/upper/pdcp_entity_lte.cc b/lib/src/upper/pdcp_entity_lte.cc index 31b5a680e..e7fdda3d4 100644 --- a/lib/src/upper/pdcp_entity_lte.cc +++ b/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_; - cfg = cfg_; - active = true; - tx_count = 0; - rx_count = 0; + lcid = lcid_; + cfg = cfg_; + active = true; + tx_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]); - { - std::unique_lock lock(mutex); + write_data_header(sdu, tx_count); - write_data_header(sdu, tx_count); + // Append MAC (SRBs only) + uint8_t mac[4] = {}; + bool do_integrity = integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX; + if (do_integrity && is_srb()) { + integrity_generate(sdu->msg, sdu->N_bytes, tx_count, mac); + } - // Append MAC (SRBs only) - uint8_t mac[4] = {}; - bool do_integrity = integrity_direction == DIRECTION_TX || integrity_direction == DIRECTION_TXRX; - if (do_integrity && is_srb()) { - integrity_generate(sdu->msg, sdu->N_bytes, tx_count, mac); - } + if (is_srb()) { + append_mac(sdu, mac); + } - if (is_srb()) { - append_mac(sdu, mac); - } - - if (encryption_direction == DIRECTION_TX || encryption_direction == DIRECTION_TXRX) { - cipher_encrypt( - &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()); - } - tx_count++; + if (encryption_direction == DIRECTION_TX || encryption_direction == DIRECTION_TXRX) { + cipher_encrypt( + &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()); } + 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 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");