From 08578a7331c5592fb4b44ebf4ec08327ffad188d Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Tue, 16 Jun 2020 19:51:10 +0100 Subject: [PATCH] Switched to use a struct to hold the PDCP entity state variables. Deleted some unecessary setters. --- .../srslte/interfaces/pdcp_interface_types.h | 3 +- lib/include/srslte/upper/pdcp_entity_lte.h | 19 +-- lib/src/upper/pdcp_entity_lte.cc | 139 +++++++++--------- lib/test/upper/pdcp_lte_test.h | 26 +--- lib/test/upper/pdcp_lte_test_rx.cc | 8 +- 5 files changed, 84 insertions(+), 111 deletions(-) diff --git a/lib/include/srslte/interfaces/pdcp_interface_types.h b/lib/include/srslte/interfaces/pdcp_interface_types.h index 468b2192f..ecc215635 100644 --- a/lib/include/srslte/interfaces/pdcp_interface_types.h +++ b/lib/include/srslte/interfaces/pdcp_interface_types.h @@ -142,12 +142,13 @@ public: enum srslte_direction_t { DIRECTION_NONE = 0, DIRECTION_TX, DIRECTION_RX, DIRECTION_TXRX, DIRECTION_N_ITEMS }; static const char* srslte_direction_text[DIRECTION_N_ITEMS] = {"none", "tx", "rx", "tx/rx"}; -// PDCP LTE internal state +// PDCP LTE internal state variables, as defined in TS 36 323, section 7.1 struct pdcp_lte_state_t { uint32_t tx_count; uint32_t rx_hfn; uint32_t next_pdcp_rx_sn; uint32_t last_submitted_pdcp_rx_sn; + uint32_t reordering_pdcp_rx_count; }; } // namespace srslte diff --git a/lib/include/srslte/upper/pdcp_entity_lte.h b/lib/include/srslte/upper/pdcp_entity_lte.h index 0903dd4e3..c6251e202 100644 --- a/lib/include/srslte/upper/pdcp_entity_lte.h +++ b/lib/include/srslte/upper/pdcp_entity_lte.h @@ -65,15 +65,6 @@ public: // RLC interface void write_pdu(unique_byte_buffer_t pdu); - // State variable setters (should be used only for testing) - void set_tx_count(uint32_t tx_count_) { tx_count = tx_count_; } - void set_rx_hfn(uint32_t rx_hfn_) { rx_hfn = rx_hfn_; } - void set_next_pdcp_rx_sn(uint32_t next_pdcp_rx_sn_) { next_pdcp_rx_sn = next_pdcp_rx_sn_; } - void set_last_submitted_pdcp_rx_sn(uint32_t last_submitted_pdcp_rx_sn_) - { - last_submitted_pdcp_rx_sn = last_submitted_pdcp_rx_sn_; - } - // Config helpers bool check_valid_config(); @@ -82,13 +73,11 @@ private: srsue::rrc_interface_pdcp* rrc = nullptr; srsue::gw_interface_pdcp* gw = nullptr; - uint32_t tx_count = 0; + // State variables, as defined in TS 36 323, section 7.1 + pdcp_lte_state_t st = {}; - uint32_t rx_hfn = 0; - uint32_t next_pdcp_rx_sn = 0; - uint32_t reordering_window = 0; - uint32_t last_submitted_pdcp_rx_sn = 0; - uint32_t maximum_pdcp_sn = 0; + uint32_t reordering_window = 0; + uint32_t maximum_pdcp_sn = 0; void handle_srb_pdu(srslte::unique_byte_buffer_t pdu); void handle_um_drb_pdu(srslte::unique_byte_buffer_t pdu); diff --git a/lib/src/upper/pdcp_entity_lte.cc b/lib/src/upper/pdcp_entity_lte.cc index 47337d580..ebd7480fb 100644 --- a/lib/src/upper/pdcp_entity_lte.cc +++ b/lib/src/upper/pdcp_entity_lte.cc @@ -39,10 +39,10 @@ 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; + lcid = lcid_; + cfg = cfg_; + active = true; + integrity_direction = DIRECTION_NONE; encryption_direction = DIRECTION_NONE; @@ -52,10 +52,12 @@ void pdcp_entity_lte::init(uint32_t lcid_, pdcp_config_t cfg_) reordering_window = 2048; } - rx_hfn = 0; - next_pdcp_rx_sn = 0; - maximum_pdcp_sn = (1 << cfg.sn_len) - 1; - last_submitted_pdcp_rx_sn = maximum_pdcp_sn; + st.tx_count = 0; + st.rx_hfn = 0; + st.next_pdcp_rx_sn = 0; + maximum_pdcp_sn = (1 << cfg.sn_len) - 1; + st.last_submitted_pdcp_rx_sn = maximum_pdcp_sn; + log->info("Init %s with bearer ID: %d\n", rrc->get_rb_name(lcid).c_str(), cfg.bearer_id); log->info("SN len bits: %d, SN len bytes: %d, reordering window: %d, Maximum SN %d\n", cfg.sn_len, @@ -75,15 +77,15 @@ void pdcp_entity_lte::reestablish() log->info("Re-establish %s with bearer ID: %d\n", rrc->get_rb_name(lcid).c_str(), cfg.bearer_id); // For SRBs if (is_srb()) { - tx_count = 0; - rx_hfn = 0; - next_pdcp_rx_sn = 0; + st.tx_count = 0; + st.rx_hfn = 0; + st.next_pdcp_rx_sn = 0; } else { // Only reset counter in RLC-UM if (rlc->rb_is_um(lcid)) { - tx_count = 0; - rx_hfn = 0; - next_pdcp_rx_sn = 0; + st.tx_count = 0; + st.rx_hfn = 0; + st.next_pdcp_rx_sn = 0; } } } @@ -101,7 +103,7 @@ void pdcp_entity_lte::reset() void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, bool blocking) { // check for pending security config in transmit direction - if (enable_security_tx_sn != -1 && enable_security_tx_sn == static_cast(tx_count)) { + if (enable_security_tx_sn != -1 && enable_security_tx_sn == static_cast(st.tx_count)) { enable_integrity(DIRECTION_TX); enable_encryption(DIRECTION_TX); enable_security_tx_sn = -1; @@ -111,17 +113,17 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, bool blocking) sdu->N_bytes, "TX %s SDU, SN=%d, integrity=%s, encryption=%s", rrc->get_rb_name(lcid).c_str(), - tx_count, + st.tx_count, srslte_direction_text[integrity_direction], srslte_direction_text[encryption_direction]); - write_data_header(sdu, tx_count); + write_data_header(sdu, st.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); + integrity_generate(sdu->msg, sdu->N_bytes, st.tx_count, mac); } if (is_srb()) { @@ -130,10 +132,10 @@ void pdcp_entity_lte::write_sdu(unique_byte_buffer_t sdu, bool blocking) 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]); + &sdu->msg[cfg.hdr_len_bytes], sdu->N_bytes - cfg.hdr_len_bytes, st.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++; + st.tx_count++; rlc->write_sdu(lcid, std::move(sdu), blocking); } @@ -187,14 +189,14 @@ void pdcp_entity_lte::handle_srb_pdu(srslte::unique_byte_buffer_t pdu) // Read SN from header uint32_t sn = read_data_header(pdu); - log->debug("RX SRB PDU. Next_PDCP_RX_SN %d, SN %d", next_pdcp_rx_sn, sn); + log->debug("RX SRB PDU. Next_PDCP_RX_SN %d, SN %d", st.next_pdcp_rx_sn, sn); // Estimate COUNT for integrity check and decryption uint32_t count; - if (sn < next_pdcp_rx_sn) { - count = COUNT(rx_hfn + 1, sn); + if (sn < st.next_pdcp_rx_sn) { + count = COUNT(st.rx_hfn + 1, sn); } else { - count = COUNT(rx_hfn, sn); + count = COUNT(st.rx_hfn, sn); } // Perform decryption @@ -219,14 +221,14 @@ void pdcp_entity_lte::handle_srb_pdu(srslte::unique_byte_buffer_t pdu) discard_data_header(pdu); // Update state variables - if (sn < next_pdcp_rx_sn) { - rx_hfn++; + if (sn < st.next_pdcp_rx_sn) { + st.rx_hfn++; } - next_pdcp_rx_sn = sn + 1; + st.next_pdcp_rx_sn = sn + 1; - if (next_pdcp_rx_sn > maximum_pdcp_sn) { - next_pdcp_rx_sn = 0; - rx_hfn++; + if (st.next_pdcp_rx_sn > maximum_pdcp_sn) { + st.next_pdcp_rx_sn = 0; + st.rx_hfn++; } // Pass to upper layers @@ -240,20 +242,20 @@ void pdcp_entity_lte::handle_um_drb_pdu(srslte::unique_byte_buffer_t pdu) uint32_t sn = read_data_header(pdu); discard_data_header(pdu); - if (sn < next_pdcp_rx_sn) { - rx_hfn++; + if (sn < st.next_pdcp_rx_sn) { + st.rx_hfn++; } - uint32_t count = (rx_hfn << cfg.sn_len) | sn; + uint32_t count = (st.rx_hfn << cfg.sn_len) | sn; if (encryption_direction == DIRECTION_RX || encryption_direction == DIRECTION_TXRX) { cipher_decrypt(pdu->msg, pdu->N_bytes, count, pdu->msg); log->debug_hex(pdu->msg, pdu->N_bytes, "%s Rx PDU (decrypted)", rrc->get_rb_name(lcid).c_str()); } - next_pdcp_rx_sn = sn + 1; - if (next_pdcp_rx_sn > maximum_pdcp_sn) { - next_pdcp_rx_sn = 0; - rx_hfn++; + st.next_pdcp_rx_sn = sn + 1; + if (st.next_pdcp_rx_sn > maximum_pdcp_sn) { + st.next_pdcp_rx_sn = 0; + st.rx_hfn++; } // Pass to upper layers @@ -267,15 +269,15 @@ void pdcp_entity_lte::handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu) uint32_t sn = read_data_header(pdu); discard_data_header(pdu); - int32_t last_submit_diff_sn = last_submitted_pdcp_rx_sn - sn; - int32_t sn_diff_last_submit = sn - last_submitted_pdcp_rx_sn; - int32_t sn_diff_next_pdcp_rx_sn = sn - next_pdcp_rx_sn; + int32_t last_submit_diff_sn = st.last_submitted_pdcp_rx_sn - sn; + int32_t sn_diff_last_submit = sn - st.last_submitted_pdcp_rx_sn; + int32_t sn_diff_next_pdcp_rx_sn = sn - st.next_pdcp_rx_sn; log->debug("RX HFN: %d, SN=%d, Last_Submitted_PDCP_RX_SN=%d, Next_PDCP_RX_SN=%d\n", - rx_hfn, + st.rx_hfn, sn, - last_submitted_pdcp_rx_sn, - next_pdcp_rx_sn); + st.last_submitted_pdcp_rx_sn, + st.next_pdcp_rx_sn); // Handle PDU uint32_t count = 0; @@ -288,25 +290,25 @@ void pdcp_entity_lte::handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu) last_submit_diff_sn, reordering_window); return; // Discard - } else if ((int32_t)(next_pdcp_rx_sn - sn) > (int32_t)reordering_window) { + } else if ((int32_t)(st.next_pdcp_rx_sn - sn) > (int32_t)reordering_window) { log->debug("(Next_PDCP_RX_SN - SN) is larger than re-ordering window.\n"); - rx_hfn++; - count = (rx_hfn << cfg.sn_len) | sn; - next_pdcp_rx_sn = sn + 1; + st.rx_hfn++; + count = (st.rx_hfn << cfg.sn_len) | sn; + st.next_pdcp_rx_sn = sn + 1; } else if (sn_diff_next_pdcp_rx_sn >= (int32_t)reordering_window) { log->debug("(SN - Next_PDCP_RX_SN) is larger or equal than re-ordering window.\n"); - count = ((rx_hfn - 1) << cfg.sn_len) | sn; - } else if (sn >= next_pdcp_rx_sn) { + count = ((st.rx_hfn - 1) << cfg.sn_len) | sn; + } else if (sn >= st.next_pdcp_rx_sn) { log->debug("SN is larger or equal than Next_PDCP_RX_SN.\n"); - count = (rx_hfn << cfg.sn_len) | sn; - next_pdcp_rx_sn = sn + 1; - if (next_pdcp_rx_sn > maximum_pdcp_sn) { - next_pdcp_rx_sn = 0; - rx_hfn++; + count = (st.rx_hfn << cfg.sn_len) | sn; + st.next_pdcp_rx_sn = sn + 1; + if (st.next_pdcp_rx_sn > maximum_pdcp_sn) { + st.next_pdcp_rx_sn = 0; + st.rx_hfn++; } - } else if (sn < next_pdcp_rx_sn) { + } else if (sn < st.next_pdcp_rx_sn) { log->debug("SN is smaller than Next_PDCP_RX_SN.\n"); - count = (rx_hfn << cfg.sn_len) | sn; + count = (st.rx_hfn << cfg.sn_len) | sn; } // Decrypt @@ -314,7 +316,7 @@ void pdcp_entity_lte::handle_am_drb_pdu(srslte::unique_byte_buffer_t pdu) log->debug_hex(pdu->msg, pdu->N_bytes, "%s Rx PDU (decrypted)", rrc->get_rb_name(lcid).c_str()); // Update info on last PDU submitted to upper layers - last_submitted_pdcp_rx_sn = sn; + st.last_submitted_pdcp_rx_sn = sn; // Pass to upper layers log->info_hex(pdu->msg, pdu->N_bytes, "%s Rx PDU SN=%d", rrc->get_rb_name(lcid).c_str(), sn); @@ -328,18 +330,18 @@ void pdcp_entity_lte::get_bearer_status(uint16_t* dlsn, uint16_t* dlhfn, uint16_ { if (cfg.rb_type == PDCP_RB_IS_DRB) { if (12 == cfg.sn_len) { - *dlsn = (uint16_t)(tx_count & 0xFFFu); - *dlhfn = (uint16_t)((tx_count - *dlsn) >> 12u); + *dlsn = (uint16_t)(st.tx_count & 0xFFFu); + *dlhfn = (uint16_t)((st.tx_count - *dlsn) >> 12u); } else { - *dlsn = (uint16_t)(tx_count & 0x7Fu); - *dlhfn = (uint16_t)((tx_count - *dlsn) >> 7u); + *dlsn = (uint16_t)(st.tx_count & 0x7Fu); + *dlhfn = (uint16_t)((st.tx_count - *dlsn) >> 7u); } } else { // is control - *dlsn = (uint16_t)(tx_count & 0x1Fu); - *dlhfn = (uint16_t)((tx_count - *dlsn) >> 5u); + *dlsn = (uint16_t)(st.tx_count & 0x1Fu); + *dlhfn = (uint16_t)((st.tx_count - *dlsn) >> 5u); } - *ulsn = (uint16_t)next_pdcp_rx_sn; - *ulhfn = (uint16_t)rx_hfn; + *ulsn = (uint16_t)st.next_pdcp_rx_sn; + *ulhfn = (uint16_t)st.rx_hfn; } /**************************************************************************** @@ -371,15 +373,12 @@ bool pdcp_entity_lte::check_valid_config() ***************************************************************************/ void pdcp_entity_lte::get_state(pdcp_lte_state_t* state) { - *state = {tx_count, rx_hfn, next_pdcp_rx_sn, last_submitted_pdcp_rx_sn}; + *state = st; } void pdcp_entity_lte::set_state(const pdcp_lte_state_t& state) { - tx_count = state.tx_count; - rx_hfn = state.rx_hfn; - next_pdcp_rx_sn = state.next_pdcp_rx_sn; - last_submitted_pdcp_rx_sn = state.last_submitted_pdcp_rx_sn; + st = state; } } // namespace srslte diff --git a/lib/test/upper/pdcp_lte_test.h b/lib/test/upper/pdcp_lte_test.h index 281862c4f..89aa2c2c4 100644 --- a/lib/test/upper/pdcp_lte_test.h +++ b/lib/test/upper/pdcp_lte_test.h @@ -26,13 +26,6 @@ #include "srslte/test/ue_test_interfaces.h" #include "srslte/upper/pdcp_entity_lte.h" -struct pdcp_lte_initial_state { - uint32_t tx_count; - uint32_t rx_hfn; - uint32_t next_pdcp_rx_sn; - uint32_t last_submitted_pdcp_rx_sn; -}; - // Helper struct to hold a packet and the number of clock // ticks to run after writing the packet to test timeouts. struct pdcp_test_event_t { @@ -66,7 +59,7 @@ uint8_t sdu1[] = {0x18, 0xe2}; uint8_t sdu2[] = {0xde, 0xad}; // This is the normal initial state. All state variables are set to zero -pdcp_lte_initial_state normal_init_state = {}; +srslte::pdcp_lte_state_t normal_init_state = {}; /* * Helper classes to reduce copy / pasting in setting up tests @@ -76,10 +69,7 @@ class pdcp_lte_test_helper { public: pdcp_lte_test_helper(srslte::pdcp_config_t cfg, srslte::as_security_config_t sec_cfg, srslte::log_ref log) : - rlc(log), - rrc(log), - gw(log), - pdcp(&rlc, &rrc, &gw, &stack, log) + rlc(log), rrc(log), gw(log), pdcp(&rlc, &rrc, &gw, &stack, log) { pdcp.init(0, cfg); pdcp.config_security(sec_cfg); @@ -87,13 +77,7 @@ public: pdcp.enable_encryption(srslte::DIRECTION_TXRX); } - void set_pdcp_initial_state(pdcp_lte_initial_state init_state) - { - pdcp.set_tx_count(init_state.tx_count); - pdcp.set_rx_hfn(init_state.rx_hfn); - pdcp.set_next_pdcp_rx_sn(init_state.next_pdcp_rx_sn); - pdcp.set_last_submitted_pdcp_rx_sn(init_state.last_submitted_pdcp_rx_sn); - } + void set_pdcp_initial_state(const srslte::pdcp_lte_state_t& init_state) { pdcp.set_state(init_state); } rlc_dummy rlc; rrc_dummy rrc; @@ -123,8 +107,8 @@ srslte::unique_byte_buffer_t gen_expected_pdu(const srslte::unique_byte_buffer_t srslte::pdcp_entity_lte* pdcp = &pdcp_hlp.pdcp; rlc_dummy* rlc = &pdcp_hlp.rlc; - pdcp_lte_initial_state init_state = {}; - init_state.tx_count = count; + srslte::pdcp_lte_state_t init_state = {}; + init_state.tx_count = count; pdcp_hlp.set_pdcp_initial_state(init_state); srslte::unique_byte_buffer_t sdu = srslte::allocate_unique_buffer(*pool); diff --git a/lib/test/upper/pdcp_lte_test_rx.cc b/lib/test/upper/pdcp_lte_test_rx.cc index 9f667349a..6dfaee256 100644 --- a/lib/test/upper/pdcp_lte_test_rx.cc +++ b/lib/test/upper/pdcp_lte_test_rx.cc @@ -25,7 +25,7 @@ * Genric function to test reception of in-sequence packets */ int test_rx(std::vector events, - const pdcp_lte_initial_state& init_state, + const srslte::pdcp_lte_state_t& init_state, uint8_t pdcp_sn_len, srslte::pdcp_rb_type_t rb_type, uint32_t n_sdus_exp, @@ -96,7 +96,7 @@ int test_rx_all(srslte::byte_buffer_pool* pool, srslte::log_ref log) std::iota(test1_counts.begin(), test1_counts.end(), 31); // Starting at COUNT 31 std::vector test1_pdus = gen_expected_pdus_vector( tst_sdu1, test1_counts, srslte::PDCP_SN_LEN_5, srslte::PDCP_RB_IS_SRB, sec_cfg, pool, log); - pdcp_lte_initial_state test1_init_state = { + srslte::pdcp_lte_state_t test1_init_state = { .tx_count = 0, .rx_hfn = 0, .next_pdcp_rx_sn = 31, .last_submitted_pdcp_rx_sn = 30}; TESTASSERT(test_rx(std::move(test1_pdus), test1_init_state, @@ -118,7 +118,7 @@ int test_rx_all(srslte::byte_buffer_pool* pool, srslte::log_ref log) std::iota(test_counts.begin(), test_counts.end(), 4095); // Starting at COUNT 4095 std::vector test_pdus = gen_expected_pdus_vector( tst_sdu1, test_counts, srslte::PDCP_SN_LEN_12, srslte::PDCP_RB_IS_DRB, sec_cfg, pool, log); - pdcp_lte_initial_state test_init_state = { + srslte::pdcp_lte_state_t test_init_state = { .tx_count = 0, .rx_hfn = 0, .next_pdcp_rx_sn = 4095, .last_submitted_pdcp_rx_sn = 4094}; TESTASSERT(test_rx(std::move(test_pdus), test_init_state, @@ -139,7 +139,7 @@ int test_rx_all(srslte::byte_buffer_pool* pool, srslte::log_ref log) std::iota(test_counts.begin(), test_counts.end(), 31); // Starting at COUNT 31 std::vector test_pdus = gen_expected_pdus_vector( tst_sdu1, test_counts, srslte::PDCP_SN_LEN_12, srslte::PDCP_RB_IS_DRB, sec_cfg, pool, log); - pdcp_lte_initial_state test_init_state = { + srslte::pdcp_lte_state_t test_init_state = { .tx_count = 0, .rx_hfn = 0, .next_pdcp_rx_sn = 32, .last_submitted_pdcp_rx_sn = 31}; TESTASSERT(test_rx(std::move(test_pdus), test_init_state,