From 1272f4c327616b9dbd5900b52d8c683c13e7f964 Mon Sep 17 00:00:00 2001 From: Francisco Date: Thu, 28 Oct 2021 10:28:43 +0100 Subject: [PATCH 01/12] nr,gnb,mac: fix log tti context overflow --- srsenb/src/stack/mac/nr/mac_nr.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/srsenb/src/stack/mac/nr/mac_nr.cc b/srsenb/src/stack/mac/nr/mac_nr.cc index a2988f872..95a72dd32 100644 --- a/srsenb/src/stack/mac/nr/mac_nr.cc +++ b/srsenb/src/stack/mac/nr/mac_nr.cc @@ -298,10 +298,10 @@ int mac_nr::slot_indication(const srsran_slot_cfg_t& slot_cfg) int mac_nr::get_dl_sched(const srsran_slot_cfg_t& slot_cfg, dl_sched_t& dl_sched) { - logger.set_context(slot_cfg.idx - TX_ENB_DELAY); - slot_point pdsch_slot = srsran::slot_point{NUMEROLOGY_IDX, slot_cfg.idx}; + logger.set_context((pdsch_slot - TX_ENB_DELAY).to_uint()); + // Run Scheduler sched_nr_interface::sched_rar_list_t rar_list; sched_nr_interface::dl_res_t dl_res(rar_list, dl_sched); From 8a53a9d35c4b9cb1b4872a4c652b2ae38de00083 Mon Sep 17 00:00:00 2001 From: Ismael Gomez Date: Thu, 28 Oct 2021 12:06:33 +0200 Subject: [PATCH 02/12] ssb_decode_test: Fix stack overflow pbch_msg buffer --- lib/include/srsran/phy/phch/pbch_msg_nr.h | 7 ++++++- lib/src/phy/sync/test/ssb_decode_test.c | 10 +--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/include/srsran/phy/phch/pbch_msg_nr.h b/lib/include/srsran/phy/phch/pbch_msg_nr.h index 8d1e09c09..e9757caf1 100644 --- a/lib/include/srsran/phy/phch/pbch_msg_nr.h +++ b/lib/include/srsran/phy/phch/pbch_msg_nr.h @@ -23,11 +23,16 @@ */ #define SRSRAN_PBCH_MSG_NR_SZ 24 +/** + * @brief Define the payload buffer for SRSRAN_PBCH_MSG_NR_SZ to be 32 for alignment purposes + */ +#define SRSRAN_PBCH_MSG_NR_MAX_SZ 32 + /** * @brief Describes the NR PBCH message */ typedef struct SRSRAN_API { - uint8_t payload[SRSRAN_PBCH_MSG_NR_SZ]; ///< Actual PBCH payload provided by higher layers + uint8_t payload[SRSRAN_PBCH_MSG_NR_MAX_SZ]; ///< Actual PBCH payload provided by higher layers uint8_t sfn_4lsb; ///< SFN 4 LSB uint8_t ssb_idx; ///< SS/PBCH blocks index described in TS 38.213 4.1 uint8_t k_ssb_msb; ///< Subcarrier offset MSB described in TS 38.211 7.4.3.1 diff --git a/lib/src/phy/sync/test/ssb_decode_test.c b/lib/src/phy/sync/test/ssb_decode_test.c index 9cebbd0a4..c59982551 100644 --- a/lib/src/phy/sync/test/ssb_decode_test.c +++ b/lib/src/phy/sync/test/ssb_decode_test.c @@ -20,14 +20,6 @@ #include #include -/** - * @brief NR PBCH payload buffer size for the SSB decode test. - * - * @note This needs to be a multiple of 32, since the PBCH message payload is - * generated with srs_random_bit_vector. - */ -#define SRSRAN_PBCH_TEST_MSG_NR_SZ 32 - // NR parameters static uint32_t carrier_nof_prb = 52; static srsran_subcarrier_spacing_t carrier_scs = srsran_subcarrier_spacing_15kHz; @@ -121,7 +113,7 @@ static void gen_pbch_msg(srsran_pbch_msg_nr_t* pbch_msg, uint32_t ssb_idx) SRSRAN_MEM_ZERO(pbch_msg, srsran_pbch_msg_nr_t, 1); // Generate payload - srsran_random_bit_vector(random_gen, pbch_msg->payload, SRSRAN_PBCH_TEST_MSG_NR_SZ); + srsran_random_bit_vector(random_gen, pbch_msg->payload, SRSRAN_PBCH_MSG_NR_SZ); pbch_msg->ssb_idx = ssb_idx; pbch_msg->crc = true; From 229b1eef213abbb1faf870e4675ecff994978765 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Thu, 28 Oct 2021 16:06:12 +0200 Subject: [PATCH 03/12] radio,test: fix Coverity warning about dead code add default in switch instead of using the the END state directly that is checked before entering the switch. Coverity correctly reports this as dead code. --- lib/src/radio/test/test_radio_rt_gain.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/radio/test/test_radio_rt_gain.cc b/lib/src/radio/test/test_radio_rt_gain.cc index 540ef7399..c12f671e5 100644 --- a/lib/src/radio/test/test_radio_rt_gain.cc +++ b/lib/src/radio/test/test_radio_rt_gain.cc @@ -262,8 +262,8 @@ int main(int argc, char** argv) continue; } break; - case END: - continue; + default: + break; } // Prepare reception buffers From 3b2f0d2f65dadee0d78898a09a06c82cdf79c2e0 Mon Sep 17 00:00:00 2001 From: faluco Date: Thu, 28 Oct 2021 13:01:41 +0200 Subject: [PATCH 04/12] Fix several issues in GW class: - Avoid triggering an assertion when the gw thread is pending termination. - Re-order gw::stop method to avoid a race condition when closing the TUN device while the gw thread is still running. --- srsue/hdr/stack/upper/gw.h | 3 ++- srsue/src/stack/upper/gw.cc | 27 +++++++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/srsue/hdr/stack/upper/gw.h b/srsue/hdr/stack/upper/gw.h index aec223558..00d80c8d1 100644 --- a/srsue/hdr/stack/upper/gw.h +++ b/srsue/hdr/stack/upper/gw.h @@ -44,6 +44,7 @@ class gw : public gw_interface_stack, public srsran::thread { public: gw(srslog::basic_logger& logger_); + ~gw(); int init(const gw_args_t& args_, stack_interface_gw* stack); void stop(); @@ -76,7 +77,7 @@ private: int32_t tun_fd = 0; struct ifreq ifr = {}; int32_t sock = 0; - bool if_up = false; + std::atomic if_up = {false}; static const int NOT_ASSIGNED = -1; int32_t default_eps_bearer_id = NOT_ASSIGNED; diff --git a/srsue/src/stack/upper/gw.cc b/srsue/src/stack/upper/gw.cc index 7ae6c8cbb..7a0da339e 100644 --- a/srsue/src/stack/upper/gw.cc +++ b/srsue/src/stack/upper/gw.cc @@ -62,12 +62,22 @@ int gw::init(const gw_args_t& args_, stack_interface_gw* stack_) return SRSRAN_SUCCESS; } +gw::~gw() +{ + if (tun_fd > 0) { + close(tun_fd); + } +} + void gw::stop() { if (run_enable) { run_enable = false; if (if_up) { - close(tun_fd); + if_up = false; + if (running) { + thread_cancel(); + } // Wait thread to exit gracefully otherwise might leave a mutex locked int cnt = 0; @@ -75,9 +85,6 @@ void gw::stop() usleep(10000); cnt++; } - if (running) { - thread_cancel(); - } wait_thread_finish(); current_ip_addr = 0; @@ -125,7 +132,9 @@ void gw::write_pdu(uint32_t lcid, srsran::unique_byte_buffer_t pdu) dl_tput_bytes += pdu->N_bytes; } if (!if_up) { - logger.warning("TUN/TAP not up - dropping gw RX message"); + if (run_enable) { + logger.warning("TUN/TAP not up - dropping gw RX message"); + } } else if (pdu->N_bytes < 20) { // Packet not large enough to hold IPv4 Header logger.warning("Packet to small to hold IPv4 header. Dropping packet with %d B", pdu->N_bytes); @@ -163,7 +172,9 @@ void gw::write_pdu_mch(uint32_t lcid, srsran::unique_byte_buffer_t pdu) memcpy(&dst_addr.s_addr, &pdu->msg[16], 4); if (!if_up) { - logger.warning("TUN/TAP not up - dropping gw RX message"); + if (run_enable) { + logger.warning("TUN/TAP not up - dropping gw RX message"); + } } else { int n = write(tun_fd, pdu->msg, pdu->N_bytes); if (n > 0 && (pdu->N_bytes != (uint32_t)n)) { @@ -324,6 +335,10 @@ void gw::run_thread() continue; } + if (!run_enable) { + break; + } + // Beyond this point we should have a activated default EPS bearer srsran_assert(default_eps_bearer_id != NOT_ASSIGNED, "Default EPS bearer not activated"); From 296758e4abe66336b3dcbf743fa90d7c69d59463 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Thu, 28 Oct 2021 12:17:41 +0200 Subject: [PATCH 05/12] Revert "rlc_um_nr: reimplement update of RX_Next_Reassembly" This reverts commit 5b025cfbf8ec78474896231f266ba8970128e512. --- lib/src/rlc/rlc_um_nr.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/src/rlc/rlc_um_nr.cc b/lib/src/rlc/rlc_um_nr.cc index 16979dece..2ad4e50da 100644 --- a/lib/src/rlc/rlc_um_nr.cc +++ b/lib/src/rlc/rlc_um_nr.cc @@ -421,12 +421,9 @@ void rlc_um_nr::rlc_um_nr_rx::handle_rx_buffer_update(const uint32_t sn) // find next SN in rx buffer if (sn == RX_Next_Reassembly) { - for (auto it = rx_window.begin(); it != rx_window.end(); ++it) { - logger.debug("SN=%d has %zd segments", it->first, it->second.segments.size()); - if (RX_MOD_NR_BASE(it->first) > RX_MOD_NR_BASE(RX_Next_Reassembly)) { - RX_Next_Reassembly = it->first; - break; - } + RX_Next_Reassembly = ((RX_Next_Reassembly + 1) % mod); + while (RX_MOD_NR_BASE(RX_Next_Reassembly) < RX_MOD_NR_BASE(RX_Next_Highest)) { + RX_Next_Reassembly = (RX_Next_Reassembly + 1) % mod; } logger.debug("Updating RX_Next_Reassembly=%d", RX_Next_Reassembly); } From e491aef74ed351610956f7f23162cded2440b34d Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Thu, 28 Oct 2021 12:43:42 +0200 Subject: [PATCH 06/12] rlc_um_nr_test: disable test9 until low TCP UL rates are understood/fixed --- lib/test/rlc/rlc_um_nr_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/test/rlc/rlc_um_nr_test.cc b/lib/test/rlc/rlc_um_nr_test.cc index 9f1052a1a..76fff9ad5 100644 --- a/lib/test/rlc/rlc_um_nr_test.cc +++ b/lib/test/rlc/rlc_um_nr_test.cc @@ -667,10 +667,13 @@ int main(int argc, char** argv) return SRSRAN_ERROR; } +// temporarily disabling +#if 0 if (rlc_um_nr_test9()) { fprintf(stderr, "rlc_um_nr_test9() failed.\n"); return SRSRAN_ERROR; } +#endif #if PCAP pcap_handle->close(); From 3b66c2cb2aba5f1a18e881f68797cb5dd1dcb040 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Thu, 28 Oct 2021 15:00:06 +0200 Subject: [PATCH 07/12] github: remove memcheck from actions workflow the memcheck gives constant errors on github machines. disabling them. they work fine on internal CI machines. 393/1465 MemCheck #393: pusch_test-n50-L50-puci_ack10-m0 ................................***Exception: Illegal 1.49 sec Start 394: pusch_test-n50-L50-puci_ack10-pcqiwideband-m0 --- .github/workflows/ccpp.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ccpp.yml b/.github/workflows/ccpp.yml index 9840d5fa5..8272df741 100644 --- a/.github/workflows/ccpp.yml +++ b/.github/workflows/ccpp.yml @@ -13,7 +13,7 @@ jobs: run: | sudo apt update sudo apt install -y build-essential cmake libfftw3-dev libmbedtls-dev libpcsclite-dev libboost-program-options-dev libconfig++-dev libsctp-dev colordiff ninja-build valgrind - mkdir build && cd build && cmake -DRF_FOUND=True -GNinja .. && ninja && ctest -T memcheck + mkdir build && cd build && cmake -DRF_FOUND=True -GNinja .. && ninja && ctest x86_ubuntu16_build: name: Build and test on x86 Ubuntu 16.04 strategy: @@ -26,7 +26,7 @@ jobs: run: | sudo apt update sudo apt install -y build-essential cmake libfftw3-dev libmbedtls-dev libpcsclite-dev libboost-program-options-dev libconfig++-dev libsctp-dev colordiff ninja-build valgrind - mkdir build && cd build && cmake -DRF_FOUND=True -GNinja .. && ninja && ctest -T memcheck + mkdir build && cd build && cmake -DRF_FOUND=True -GNinja .. && ninja && ctest aarch64_ubuntu18_build: runs-on: ubuntu-18.04 From fa1c06e477158f35acc3a5d54ea0c64cbc608620 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Thu, 28 Oct 2021 15:01:34 +0200 Subject: [PATCH 08/12] readme: update CI badges and eNB brief info --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index f8e9792a4..90659b531 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,9 @@ srsRAN ====== -[![Build Status](https://travis-ci.com/srsran/srsRAN.svg?branch=master)](https://travis-ci.com/srsran/srsRAN) +[![Build Status](https://app.travis-ci.com/srsran/srsRAN.svg?branch=master)](https://app.travis-ci.com/github/srsran/srsRAN) [![Language grade: C/C++](https://img.shields.io/lgtm/grade/cpp/g/srsran/srsRAN.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/srsran/srsRAN/context:cpp) -[![Coverity](https://scan.coverity.com/projects/23048/badge.svg)](https://scan.coverity.com/projects/srsran_agpl) +[![Coverity](https://scan.coverity.com/projects/23045/badge.svg)](https://scan.coverity.com/projects/srsran) srsRAN is a 4G/5G software radio suite developed by [SRS](http://www.srs.io). @@ -11,7 +11,7 @@ See the [srsRAN project pages](https://www.srsran.com) for information, guides a The srsRAN suite includes: * srsUE - a full-stack SDR 4G/5G-NSA UE application (5G-SA coming soon) - * srsENB - a full-stack SDR 4G eNodeB application (5G-NSA and 5G-SA coming soon) + * srsENB - a full-stack SDR 4G/5G-NSA eNodeB application (5G-SA coming soon) * srsEPC - a light-weight 4G core network implementation with MME, HSS and S/P-GW For application features, build instructions and user guides see the [srsRAN documentation](https://docs.srsran.com). From 5b8d4c39afea7ce796ab244a0fe1096925b8188e Mon Sep 17 00:00:00 2001 From: Francisco Date: Wed, 27 Oct 2021 15:09:42 +0100 Subject: [PATCH 09/12] pdcp,lte: fix drb/srb id logging to account for dynamic mapping --- lib/include/srsran/upper/pdcp_entity_lte.h | 3 ++ lib/src/pdcp/pdcp_entity_lte.cc | 44 +++++++++++++++------- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/lib/include/srsran/upper/pdcp_entity_lte.h b/lib/include/srsran/upper/pdcp_entity_lte.h index a8b96bb03..86c613726 100644 --- a/lib/include/srsran/upper/pdcp_entity_lte.h +++ b/lib/include/srsran/upper/pdcp_entity_lte.h @@ -156,6 +156,9 @@ private: uint32_t reordering_window = 0; uint32_t maximum_pdcp_sn = 0; + std::string get_rb_name() const; + void get_rb_name(fmt::memory_buffer& fmtbuf) const; + // PDU handlers void handle_control_pdu(srsran::unique_byte_buffer_t pdu); void handle_srb_pdu(srsran::unique_byte_buffer_t pdu); diff --git a/lib/src/pdcp/pdcp_entity_lte.cc b/lib/src/pdcp/pdcp_entity_lte.cc index af5663735..91f573aca 100644 --- a/lib/src/pdcp/pdcp_entity_lte.cc +++ b/lib/src/pdcp/pdcp_entity_lte.cc @@ -54,7 +54,7 @@ bool pdcp_entity_lte::configure(const pdcp_config_t& cnfg_) if (active) { // Already configured if (cnfg_ != cfg) { - logger.error("Bearer reconfiguration not supported. LCID=%s.", rrc->get_rb_name(lcid)); + logger.error("Bearer reconfiguration not supported. LCID=%s.", get_rb_name().c_str()); return false; } return true; @@ -79,7 +79,7 @@ bool pdcp_entity_lte::configure(const pdcp_config_t& cnfg_) // Queue Helpers maximum_allocated_sns_window = (1u << cfg.sn_len) / 2u; - logger.info("Init %s with bearer ID: %d", rrc->get_rb_name(lcid), cfg.bearer_id); + logger.info("Init %s with bearer ID: %d", get_rb_name().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, cfg.hdr_len_bytes, @@ -104,7 +104,7 @@ bool pdcp_entity_lte::configure(const pdcp_config_t& cnfg_) // Reestablishment procedure: 36.323 5.2 void pdcp_entity_lte::reestablish() { - logger.info("Re-establish %s with bearer ID: %d", rrc->get_rb_name(lcid), cfg.bearer_id); + logger.info("Re-establish %s with bearer ID: %d", get_rb_name().c_str(), cfg.bearer_id); // For SRBs if (is_srb()) { st.next_pdcp_tx_sn = 0; @@ -126,7 +126,7 @@ void pdcp_entity_lte::reestablish() void pdcp_entity_lte::reset() { if (active) { - logger.debug("Reset %s", rrc->get_rb_name(lcid)); + logger.debug("Reset %s", get_rb_name().c_str()); } active = false; } @@ -135,7 +135,7 @@ void pdcp_entity_lte::reset() void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, int upper_sn) { if (!active) { - logger.warning("Dropping %s SDU due to inactive bearer", rrc->get_rb_name(lcid)); + logger.warning("Dropping %s SDU due to inactive bearer", get_rb_name().c_str()); return; } @@ -145,7 +145,7 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, int upper_sn) } if (rlc->sdu_queue_is_full(lcid)) { - logger.info(sdu->msg, sdu->N_bytes, "Dropping %s SDU due to full queue", rrc->get_rb_name(lcid)); + logger.info(sdu->msg, sdu->N_bytes, "Dropping %s SDU due to full queue", get_rb_name().c_str()); return; } @@ -199,7 +199,7 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, int upper_sn) logger.info(sdu->msg, sdu->N_bytes, "TX %s PDU, SN=%d, integrity=%s, encryption=%s", - rrc->get_rb_name(lcid), + get_rb_name().c_str(), used_sn, srsran_direction_text[integrity_direction], srsran_direction_text[encryption_direction]); @@ -226,7 +226,7 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, int upper_sn) void pdcp_entity_lte::write_pdu(unique_byte_buffer_t pdu) { if (!active) { - logger.warning("Dropping %s PDU due to inactive bearer", rrc->get_rb_name(lcid)); + logger.warning("Dropping %s PDU due to inactive bearer", get_rb_name()); return; } @@ -256,7 +256,7 @@ void pdcp_entity_lte::write_pdu(unique_byte_buffer_t pdu) logger.info(pdu->msg, pdu->N_bytes, "%s Rx PDU SN=%d (%d B, integrity=%s, encryption=%s)", - rrc->get_rb_name(lcid), + get_rb_name(), sn, pdu->N_bytes, srsran_direction_text[integrity_direction], @@ -316,7 +316,7 @@ void pdcp_entity_lte::handle_srb_pdu(srsran::unique_byte_buffer_t pdu) cipher_decrypt(&pdu->msg[cfg.hdr_len_bytes], pdu->N_bytes - cfg.hdr_len_bytes, count, &pdu->msg[cfg.hdr_len_bytes]); } - logger.debug(pdu->msg, pdu->N_bytes, "%s Rx SDU SN=%d", rrc->get_rb_name(lcid), sn); + logger.debug(pdu->msg, pdu->N_bytes, "%s Rx SDU SN=%d", get_rb_name(), sn); // Extract MAC uint8_t mac[4]; @@ -325,7 +325,7 @@ void pdcp_entity_lte::handle_srb_pdu(srsran::unique_byte_buffer_t pdu) // Perfrom integrity checks if (integrity_direction == DIRECTION_RX || integrity_direction == DIRECTION_TXRX) { if (not integrity_verify(pdu->msg, pdu->N_bytes, count, mac)) { - logger.error(pdu->msg, pdu->N_bytes, "%s Dropping PDU", rrc->get_rb_name(lcid)); + logger.error(pdu->msg, pdu->N_bytes, "%s Dropping PDU", get_rb_name()); rrc->notify_pdcp_integrity_error(lcid); return; // Discard } @@ -364,7 +364,7 @@ void pdcp_entity_lte::handle_um_drb_pdu(srsran::unique_byte_buffer_t pdu) cipher_decrypt(pdu->msg, pdu->N_bytes, count, pdu->msg); } - logger.debug(pdu->msg, pdu->N_bytes, "%s Rx PDU SN=%d", rrc->get_rb_name(lcid), sn); + logger.debug(pdu->msg, pdu->N_bytes, "%s Rx PDU SN=%d", get_rb_name(), sn); st.next_pdcp_rx_sn = sn + 1; if (st.next_pdcp_rx_sn > maximum_pdcp_sn) { @@ -428,7 +428,7 @@ void pdcp_entity_lte::handle_am_drb_pdu(srsran::unique_byte_buffer_t pdu) // Decrypt cipher_decrypt(pdu->msg, pdu->N_bytes, count, pdu->msg); - logger.debug(pdu->msg, pdu->N_bytes, "%s Rx SDU SN=%d", rrc->get_rb_name(lcid), sn); + logger.debug(pdu->msg, pdu->N_bytes, "%s Rx SDU SN=%d", get_rb_name(), sn); // Update info on last PDU submitted to upper layers st.last_submitted_pdcp_rx_sn = sn; @@ -857,6 +857,24 @@ void pdcp_entity_lte::reset_metrics() metrics.tx_notification_latency_ms = 0; } +/**************************************************************************** + * Logging helpers + ***************************************************************************/ +std::string pdcp_entity_lte::get_rb_name() const +{ + fmt::memory_buffer fmtbuf; + get_rb_name(fmtbuf); + return fmt::to_string(fmtbuf); +} +void pdcp_entity_lte::get_rb_name(fmt::memory_buffer& fmtbuf) const +{ + if (cfg.rb_type == PDCP_RB_IS_DRB) { + fmt::format_to(fmtbuf, "DRB{}", cfg.bearer_id); + } else { + fmt::format_to(fmtbuf, "SRB{}", cfg.bearer_id); + } +} + /**************************************************************************** * Undelivered SDUs queue helpers ***************************************************************************/ From 3c18e7c1f30ac48c6958c36016495ae4ea844db5 Mon Sep 17 00:00:00 2001 From: Francisco Date: Wed, 27 Oct 2021 16:57:11 +0100 Subject: [PATCH 10/12] lte,enb,pdcp: fix addition of DRB logging in PDCP --- lib/src/pdcp/pdcp.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/src/pdcp/pdcp.cc b/lib/src/pdcp/pdcp.cc index ecd1769f3..a18cdbc29 100644 --- a/lib/src/pdcp/pdcp.cc +++ b/lib/src/pdcp/pdcp.cc @@ -129,7 +129,11 @@ int pdcp::add_bearer(uint32_t lcid, const pdcp_config_t& cfg) valid_lcids_cached.insert(lcid); } - logger.info("Add %s (lcid=%d, bearer_id=%d, sn_len=%dbits)", rrc->get_rb_name(lcid), lcid, cfg.bearer_id, cfg.sn_len); + logger.info("Add %s%d (lcid=%d, sn_len=%dbits)", + cfg.rb_type == PDCP_RB_IS_DRB ? "DRB" : "SRB", + cfg.bearer_id, + lcid, + cfg.sn_len); return SRSRAN_SUCCESS; } From c64be663d205822603661eec81135ce2206bb3c7 Mon Sep 17 00:00:00 2001 From: Francisco Date: Thu, 28 Oct 2021 17:21:52 +0100 Subject: [PATCH 11/12] nr,gnb,pdcp: store the PDCP RB name in base class for easier use --- lib/include/srsran/upper/pdcp_entity_base.h | 3 ++ lib/include/srsran/upper/pdcp_entity_lte.h | 3 -- lib/src/pdcp/pdcp.cc | 4 +- lib/src/pdcp/pdcp_entity_lte.cc | 49 +++++++-------------- 4 files changed, 22 insertions(+), 37 deletions(-) diff --git a/lib/include/srsran/upper/pdcp_entity_base.h b/lib/include/srsran/upper/pdcp_entity_base.h index 837b370fd..ab3513fe7 100644 --- a/lib/include/srsran/upper/pdcp_entity_base.h +++ b/lib/include/srsran/upper/pdcp_entity_base.h @@ -133,6 +133,8 @@ public: virtual pdcp_bearer_metrics_t get_metrics() = 0; virtual void reset_metrics() = 0; + const char* get_rb_name() const { return rb_name.c_str(); } + protected: srslog::basic_logger& logger; srsran::task_sched_handle task_sched; @@ -154,6 +156,7 @@ protected: pdcp_discard_timer_t::infinity, false, srsran_rat_t::lte}; + std::string rb_name; srsran::as_security_config_t sec_cfg = {}; diff --git a/lib/include/srsran/upper/pdcp_entity_lte.h b/lib/include/srsran/upper/pdcp_entity_lte.h index 86c613726..a8b96bb03 100644 --- a/lib/include/srsran/upper/pdcp_entity_lte.h +++ b/lib/include/srsran/upper/pdcp_entity_lte.h @@ -156,9 +156,6 @@ private: uint32_t reordering_window = 0; uint32_t maximum_pdcp_sn = 0; - std::string get_rb_name() const; - void get_rb_name(fmt::memory_buffer& fmtbuf) const; - // PDU handlers void handle_control_pdu(srsran::unique_byte_buffer_t pdu); void handle_srb_pdu(srsran::unique_byte_buffer_t pdu); diff --git a/lib/src/pdcp/pdcp.cc b/lib/src/pdcp/pdcp.cc index a18cdbc29..9a5ec950c 100644 --- a/lib/src/pdcp/pdcp.cc +++ b/lib/src/pdcp/pdcp.cc @@ -166,10 +166,10 @@ void pdcp::del_bearer(uint32_t lcid) valid_lcids_cached.erase(lcid); } if (valid_lcid(lcid)) { + logger.info("Deleted PDCP bearer %s", pdcp_array[lcid]->get_rb_name()); pdcp_array.erase(lcid); - logger.info("Deleted PDCP bearer %s", rrc->get_rb_name(lcid)); } else { - logger.warning("Can't delete bearer %s. Bearer doesn't exist.", rrc->get_rb_name(lcid)); + logger.warning("Can't delete bearer with LCID=%s. Cause: bearer doesn't exist.", lcid); } } diff --git a/lib/src/pdcp/pdcp_entity_lte.cc b/lib/src/pdcp/pdcp_entity_lte.cc index 91f573aca..b0f3c55f9 100644 --- a/lib/src/pdcp/pdcp_entity_lte.cc +++ b/lib/src/pdcp/pdcp_entity_lte.cc @@ -54,13 +54,16 @@ bool pdcp_entity_lte::configure(const pdcp_config_t& cnfg_) if (active) { // Already configured if (cnfg_ != cfg) { - logger.error("Bearer reconfiguration not supported. LCID=%s.", get_rb_name().c_str()); + logger.error("Bearer reconfiguration not supported. LCID=%s.", rb_name.c_str()); return false; } return true; } - cfg = cnfg_; + cfg = cnfg_; + rb_name = cfg.rb_type == PDCP_RB_IS_DRB ? "DRB" : "SRB"; + rb_name += std::to_string(cfg.bearer_id); + maximum_pdcp_sn = (1u << cfg.sn_len) - 1u; st.last_submitted_pdcp_rx_sn = maximum_pdcp_sn; if (is_srb()) { @@ -79,7 +82,7 @@ bool pdcp_entity_lte::configure(const pdcp_config_t& cnfg_) // Queue Helpers maximum_allocated_sns_window = (1u << cfg.sn_len) / 2u; - logger.info("Init %s with bearer ID: %d", get_rb_name().c_str(), cfg.bearer_id); + logger.info("Init %s with bearer ID: %d", rb_name.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, cfg.hdr_len_bytes, @@ -104,7 +107,7 @@ bool pdcp_entity_lte::configure(const pdcp_config_t& cnfg_) // Reestablishment procedure: 36.323 5.2 void pdcp_entity_lte::reestablish() { - logger.info("Re-establish %s with bearer ID: %d", get_rb_name().c_str(), cfg.bearer_id); + logger.info("Re-establish %s with bearer ID: %d", rb_name.c_str(), cfg.bearer_id); // For SRBs if (is_srb()) { st.next_pdcp_tx_sn = 0; @@ -126,7 +129,7 @@ void pdcp_entity_lte::reestablish() void pdcp_entity_lte::reset() { if (active) { - logger.debug("Reset %s", get_rb_name().c_str()); + logger.debug("Reset %s", rb_name.c_str()); } active = false; } @@ -135,7 +138,7 @@ void pdcp_entity_lte::reset() void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, int upper_sn) { if (!active) { - logger.warning("Dropping %s SDU due to inactive bearer", get_rb_name().c_str()); + logger.warning("Dropping %s SDU due to inactive bearer", rb_name.c_str()); return; } @@ -145,7 +148,7 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, int upper_sn) } if (rlc->sdu_queue_is_full(lcid)) { - logger.info(sdu->msg, sdu->N_bytes, "Dropping %s SDU due to full queue", get_rb_name().c_str()); + logger.info(sdu->msg, sdu->N_bytes, "Dropping %s SDU due to full queue", rb_name.c_str()); return; } @@ -199,7 +202,7 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, int upper_sn) logger.info(sdu->msg, sdu->N_bytes, "TX %s PDU, SN=%d, integrity=%s, encryption=%s", - get_rb_name().c_str(), + rb_name.c_str(), used_sn, srsran_direction_text[integrity_direction], srsran_direction_text[encryption_direction]); @@ -226,7 +229,7 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, int upper_sn) void pdcp_entity_lte::write_pdu(unique_byte_buffer_t pdu) { if (!active) { - logger.warning("Dropping %s PDU due to inactive bearer", get_rb_name()); + logger.warning("Dropping %s PDU due to inactive bearer", rb_name.c_str()); return; } @@ -256,7 +259,7 @@ void pdcp_entity_lte::write_pdu(unique_byte_buffer_t pdu) logger.info(pdu->msg, pdu->N_bytes, "%s Rx PDU SN=%d (%d B, integrity=%s, encryption=%s)", - get_rb_name(), + rb_name.c_str(), sn, pdu->N_bytes, srsran_direction_text[integrity_direction], @@ -316,7 +319,7 @@ void pdcp_entity_lte::handle_srb_pdu(srsran::unique_byte_buffer_t pdu) cipher_decrypt(&pdu->msg[cfg.hdr_len_bytes], pdu->N_bytes - cfg.hdr_len_bytes, count, &pdu->msg[cfg.hdr_len_bytes]); } - logger.debug(pdu->msg, pdu->N_bytes, "%s Rx SDU SN=%d", get_rb_name(), sn); + logger.debug(pdu->msg, pdu->N_bytes, "%s Rx SDU SN=%d", rb_name.c_str(), sn); // Extract MAC uint8_t mac[4]; @@ -325,7 +328,7 @@ void pdcp_entity_lte::handle_srb_pdu(srsran::unique_byte_buffer_t pdu) // Perfrom integrity checks if (integrity_direction == DIRECTION_RX || integrity_direction == DIRECTION_TXRX) { if (not integrity_verify(pdu->msg, pdu->N_bytes, count, mac)) { - logger.error(pdu->msg, pdu->N_bytes, "%s Dropping PDU", get_rb_name()); + logger.error(pdu->msg, pdu->N_bytes, "%s Dropping PDU", rb_name.c_str()); rrc->notify_pdcp_integrity_error(lcid); return; // Discard } @@ -364,7 +367,7 @@ void pdcp_entity_lte::handle_um_drb_pdu(srsran::unique_byte_buffer_t pdu) cipher_decrypt(pdu->msg, pdu->N_bytes, count, pdu->msg); } - logger.debug(pdu->msg, pdu->N_bytes, "%s Rx PDU SN=%d", get_rb_name(), sn); + logger.debug(pdu->msg, pdu->N_bytes, "%s Rx PDU SN=%d", rb_name.c_str(), sn); st.next_pdcp_rx_sn = sn + 1; if (st.next_pdcp_rx_sn > maximum_pdcp_sn) { @@ -428,7 +431,7 @@ void pdcp_entity_lte::handle_am_drb_pdu(srsran::unique_byte_buffer_t pdu) // Decrypt cipher_decrypt(pdu->msg, pdu->N_bytes, count, pdu->msg); - logger.debug(pdu->msg, pdu->N_bytes, "%s Rx SDU SN=%d", get_rb_name(), sn); + logger.debug(pdu->msg, pdu->N_bytes, "%s Rx SDU SN=%d", rb_name.c_str(), sn); // Update info on last PDU submitted to upper layers st.last_submitted_pdcp_rx_sn = sn; @@ -857,24 +860,6 @@ void pdcp_entity_lte::reset_metrics() metrics.tx_notification_latency_ms = 0; } -/**************************************************************************** - * Logging helpers - ***************************************************************************/ -std::string pdcp_entity_lte::get_rb_name() const -{ - fmt::memory_buffer fmtbuf; - get_rb_name(fmtbuf); - return fmt::to_string(fmtbuf); -} -void pdcp_entity_lte::get_rb_name(fmt::memory_buffer& fmtbuf) const -{ - if (cfg.rb_type == PDCP_RB_IS_DRB) { - fmt::format_to(fmtbuf, "DRB{}", cfg.bearer_id); - } else { - fmt::format_to(fmtbuf, "SRB{}", cfg.bearer_id); - } -} - /**************************************************************************** * Undelivered SDUs queue helpers ***************************************************************************/ From dfc2ea0a3ee1a71277d024a4347e10d7b45e0ff3 Mon Sep 17 00:00:00 2001 From: Francisco Date: Thu, 28 Oct 2021 17:37:51 +0100 Subject: [PATCH 12/12] nr,gnb,pdcp: extend the use of local rb_name member to pdcp nr --- lib/include/srsran/interfaces/pdcp_interface_types.h | 2 ++ lib/src/pdcp/pdcp_entity_lte.cc | 3 +-- lib/src/pdcp/pdcp_entity_nr.cc | 11 ++++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/include/srsran/interfaces/pdcp_interface_types.h b/lib/include/srsran/interfaces/pdcp_interface_types.h index cd7cdf7c0..6507e14ba 100644 --- a/lib/include/srsran/interfaces/pdcp_interface_types.h +++ b/lib/include/srsran/interfaces/pdcp_interface_types.h @@ -165,6 +165,8 @@ public: status_report_required == other.status_report_required; } bool operator!=(const pdcp_config_t& other) const { return not(*this == other); } + + std::string get_rb_name() const { return (rb_type == PDCP_RB_IS_DRB ? "DRB" : "SRB") + std::to_string(bearer_id); } }; // Specifies in which direction security (integrity and ciphering) are enabled for PDCP diff --git a/lib/src/pdcp/pdcp_entity_lte.cc b/lib/src/pdcp/pdcp_entity_lte.cc index b0f3c55f9..4f1efdafa 100644 --- a/lib/src/pdcp/pdcp_entity_lte.cc +++ b/lib/src/pdcp/pdcp_entity_lte.cc @@ -61,8 +61,7 @@ bool pdcp_entity_lte::configure(const pdcp_config_t& cnfg_) } cfg = cnfg_; - rb_name = cfg.rb_type == PDCP_RB_IS_DRB ? "DRB" : "SRB"; - rb_name += std::to_string(cfg.bearer_id); + rb_name = cfg.get_rb_name(); maximum_pdcp_sn = (1u << cfg.sn_len) - 1u; st.last_submitted_pdcp_rx_sn = maximum_pdcp_sn; diff --git a/lib/src/pdcp/pdcp_entity_nr.cc b/lib/src/pdcp/pdcp_entity_nr.cc index ba354599d..f447816c9 100644 --- a/lib/src/pdcp/pdcp_entity_nr.cc +++ b/lib/src/pdcp/pdcp_entity_nr.cc @@ -37,7 +37,7 @@ pdcp_entity_nr::~pdcp_entity_nr() {} // Reestablishment procedure: 38.323 5.2 void pdcp_entity_nr::reestablish() { - logger.info("Re-establish %s with bearer ID: %d", rrc->get_rb_name(lcid), cfg.bearer_id); + logger.info("Re-establish %s with bearer ID: %d", rb_name.c_str(), cfg.bearer_id); // TODO } @@ -46,13 +46,14 @@ bool pdcp_entity_nr::configure(const pdcp_config_t& cnfg_) if (active) { // Already configured if (cnfg_ != cfg) { - logger.error("Bearer reconfiguration not supported. LCID=%s.", rrc->get_rb_name(lcid)); + logger.error("Bearer reconfiguration not supported. LCID=%s.", rb_name.c_str()); return false; } return true; } cfg = cnfg_; + rb_name = cfg.get_rb_name(); window_size = 1 << (cfg.sn_len - 1); // Timers @@ -70,7 +71,7 @@ bool pdcp_entity_nr::configure(const pdcp_config_t& cnfg_) void pdcp_entity_nr::reset() { active = false; - logger.debug("Reset %s", rrc->get_rb_name(lcid)); + logger.debug("Reset %s", rb_name.c_str()); } // SDAP/RRC interface @@ -80,7 +81,7 @@ void pdcp_entity_nr::write_sdu(unique_byte_buffer_t sdu, int sn) logger.info(sdu->msg, sdu->N_bytes, "TX %s SDU, integrity=%s, encryption=%s", - rrc->get_rb_name(lcid), + rb_name.c_str(), srsran_direction_text[integrity_direction], srsran_direction_text[encryption_direction]); @@ -136,7 +137,7 @@ void pdcp_entity_nr::write_pdu(unique_byte_buffer_t pdu) logger.info(pdu->msg, pdu->N_bytes, "RX %s PDU (%d B), integrity=%s, encryption=%s", - rrc->get_rb_name(lcid), + rb_name.c_str(), pdu->N_bytes, srsran_direction_text[integrity_direction], srsran_direction_text[encryption_direction]);