From b7fdcaabc502c0524b0ae83dff2dc1c97a39b9e6 Mon Sep 17 00:00:00 2001 From: Pedro Alvarez Date: Thu, 3 Oct 2019 16:58:15 +0100 Subject: [PATCH] Starting to add reordering timers at PDCP NR entity. Timeout seems to be triggered correctly and pass the SDUs to the upper layers when it expires. First tests passing. --- lib/include/srslte/upper/pdcp_entity_base.h | 7 +- lib/include/srslte/upper/pdcp_entity_nr.h | 26 +++++++ lib/include/srslte/upper/rlc_am.h | 32 ++++----- lib/src/upper/pdcp_entity_nr.cc | 77 +++++++++++++++------ lib/test/upper/pdcp_nr_test.cc | 50 +++++++++---- srsenb/src/stack/rrc/rrc.cc | 59 +++++++++------- srsue/src/stack/rrc/rrc.cc | 8 ++- 7 files changed, 183 insertions(+), 76 deletions(-) diff --git a/lib/include/srslte/upper/pdcp_entity_base.h b/lib/include/srslte/upper/pdcp_entity_base.h index cdcb742fd..0bf6bc3c0 100644 --- a/lib/include/srslte/upper/pdcp_entity_base.h +++ b/lib/include/srslte/upper/pdcp_entity_base.h @@ -98,7 +98,12 @@ protected: bool do_integrity = false; bool do_encryption = false; - pdcp_config_t cfg = {1, PDCP_RB_IS_DRB, SECURITY_DIRECTION_DOWNLINK, SECURITY_DIRECTION_UPLINK, PDCP_SN_LEN_12}; + pdcp_config_t cfg = {1, + PDCP_RB_IS_DRB, + SECURITY_DIRECTION_DOWNLINK, + SECURITY_DIRECTION_UPLINK, + PDCP_SN_LEN_12, + pdcp_t_reordering_t::ms500}; std::mutex mutex; diff --git a/lib/include/srslte/upper/pdcp_entity_nr.h b/lib/include/srslte/upper/pdcp_entity_nr.h index fd2e1b1f0..cd9cae6b4 100644 --- a/lib/include/srslte/upper/pdcp_entity_nr.h +++ b/lib/include/srslte/upper/pdcp_entity_nr.h @@ -85,7 +85,33 @@ private: void write_data_header(const unique_byte_buffer_t& sdu, uint32_t sn); void extract_mac(const unique_byte_buffer_t& sdu, uint8_t* mac); void append_mac(const unique_byte_buffer_t& sdu, uint8_t* mac); + + // Pass to Upper Layers Helper function + void deliver_all_consecutive_counts(); + void pass_to_upper_layers(unique_byte_buffer_t pdu); + + // Reordering callback (t-Reordering) + class reordering_callback : public timer_callback + { + public: + reordering_callback(pdcp_entity_nr* parent_) { parent = parent_; }; + virtual void timer_expired(uint32_t timer_id) final; + + private: + pdcp_entity_nr* parent; + }; + + reordering_callback reordering_fnc; }; +inline void pdcp_entity_nr::pass_to_upper_layers(unique_byte_buffer_t sdu) +{ + if (is_srb()) { + rrc->write_pdu(lcid, std::move(sdu)); + } else { + gw->write_pdu(lcid, std::move(sdu)); + } +} + } // namespace srslte #endif // SRSLTE_PDCP_ENTITY_NR_H diff --git a/lib/include/srslte/upper/rlc_am.h b/lib/include/srslte/upper/rlc_am.h index 3f84a8edb..cb725138c 100644 --- a/lib/include/srslte/upper/rlc_am.h +++ b/lib/include/srslte/upper/rlc_am.h @@ -290,24 +290,24 @@ private: * Header pack/unpack helper functions * Ref: 3GPP TS 36.322 v10.0.0 Section 6.2.1 ***************************************************************************/ -void rlc_am_read_data_pdu_header(byte_buffer_t *pdu, rlc_amd_pdu_header_t *header); -void rlc_am_read_data_pdu_header(uint8_t **payload, uint32_t *nof_bytes, rlc_amd_pdu_header_t *header); -void rlc_am_write_data_pdu_header(rlc_amd_pdu_header_t *header, byte_buffer_t *pdu); -void rlc_am_write_data_pdu_header(rlc_amd_pdu_header_t *header, uint8_t **payload); -void rlc_am_read_status_pdu(byte_buffer_t *pdu, rlc_status_pdu_t *status); -void rlc_am_read_status_pdu(uint8_t *payload, uint32_t nof_bytes, rlc_status_pdu_t *status); -void rlc_am_write_status_pdu(rlc_status_pdu_t *status, byte_buffer_t *pdu ); -int rlc_am_write_status_pdu(rlc_status_pdu_t *status, uint8_t *payload); - -uint32_t rlc_am_packed_length(rlc_amd_pdu_header_t *header); -uint32_t rlc_am_packed_length(rlc_status_pdu_t *status); +void rlc_am_read_data_pdu_header(byte_buffer_t* pdu, rlc_amd_pdu_header_t* header); +void rlc_am_read_data_pdu_header(uint8_t** payload, uint32_t* nof_bytes, rlc_amd_pdu_header_t* header); +void rlc_am_write_data_pdu_header(rlc_amd_pdu_header_t* header, byte_buffer_t* pdu); +void rlc_am_write_data_pdu_header(rlc_amd_pdu_header_t* header, uint8_t** payload); +void rlc_am_read_status_pdu(byte_buffer_t* pdu, rlc_status_pdu_t* status); +void rlc_am_read_status_pdu(uint8_t* payload, uint32_t nof_bytes, rlc_status_pdu_t* status); +void rlc_am_write_status_pdu(rlc_status_pdu_t* status, byte_buffer_t* pdu); +int rlc_am_write_status_pdu(rlc_status_pdu_t* status, uint8_t* payload); + +uint32_t rlc_am_packed_length(rlc_amd_pdu_header_t* header); +uint32_t rlc_am_packed_length(rlc_status_pdu_t* status); uint32_t rlc_am_packed_length(rlc_amd_retx_t retx); bool rlc_am_is_valid_status_pdu(const rlc_status_pdu_t& status); -bool rlc_am_is_control_pdu(byte_buffer_t *pdu); -bool rlc_am_is_control_pdu(uint8_t *payload); -bool rlc_am_is_pdu_segment(uint8_t *payload); -std::string rlc_am_status_pdu_to_string(rlc_status_pdu_t *status); -std::string rlc_amd_pdu_header_to_string(const rlc_amd_pdu_header_t &header); +bool rlc_am_is_control_pdu(byte_buffer_t* pdu); +bool rlc_am_is_control_pdu(uint8_t* payload); +bool rlc_am_is_pdu_segment(uint8_t* payload); +std::string rlc_am_status_pdu_to_string(rlc_status_pdu_t* status); +std::string rlc_amd_pdu_header_to_string(const rlc_amd_pdu_header_t& header); bool rlc_am_start_aligned(const uint8_t fi); bool rlc_am_end_aligned(const uint8_t fi); bool rlc_am_is_unaligned(const uint8_t fi); diff --git a/lib/src/upper/pdcp_entity_nr.cc b/lib/src/upper/pdcp_entity_nr.cc index bee94442b..03d6a5dc9 100644 --- a/lib/src/upper/pdcp_entity_nr.cc +++ b/lib/src/upper/pdcp_entity_nr.cc @@ -25,7 +25,7 @@ namespace srslte { -pdcp_entity_nr::pdcp_entity_nr() {} +pdcp_entity_nr::pdcp_entity_nr() : reordering_fnc(this) {} pdcp_entity_nr::~pdcp_entity_nr() {} @@ -53,7 +53,7 @@ void pdcp_entity_nr::init(srsue::rlc_interface_pdcp* rlc_, // Timers reordering_timer_id = timers->get_unique_id(); reordering_timer = timers->get(reordering_timer_id); - + reordering_timer->set(&reordering_fnc, (uint32_t)cfg.t_reordering); } // Reestablishment procedure: 38.323 5.2 @@ -173,29 +173,24 @@ void pdcp_entity_nr::write_pdu(unique_byte_buffer_t pdu) if (rcvd_count == rx_deliv) { // Deliver to upper layers in ascending order of associeted COUNT - log->debug("Delivering SDU(s) to upper layers\n"); - for (std::map::iterator it = reorder_queue.begin(); - it != reorder_queue.end() && it->first == rx_deliv;) { - log->debug("Delivering SDU with RCVD_COUNT %d\n", it->first); - - // Pass to upper layers - if (is_srb()) { - rrc->write_pdu(lcid, std::move(it->second)); - } else { - gw->write_pdu(lcid, std::move(it->second)); - } + deliver_all_consecutive_counts(); + } - // Remove from queue - reorder_queue.erase(it++); + // Handle reordering timers + if(reordering_timer->is_running() and rx_deliv >= rx_reord){ + reordering_timer->stop(); + reordering_timer->reset(); + } - // Update RX_DELIV - rx_deliv = rx_deliv + 1; // TODO needs to be corrected when queueing is implemented - } + if (not reordering_timer->is_running() and rx_deliv < rx_next) { + rx_reord = rx_next; + reordering_timer->run(); } - - // TODO handle reordering timers } +/* + * Packing / Unpacking Helpers + */ uint32_t pdcp_entity_nr::read_data_header(const unique_byte_buffer_t& pdu) { // Check PDU is long enough to extract header @@ -278,4 +273,46 @@ void pdcp_entity_nr::append_mac(const unique_byte_buffer_t& sdu, uint8_t* mac) memcpy(&sdu->msg[sdu->N_bytes], mac, 4); sdu->N_bytes += 4; } + +/* + * Reordering Helpers + */ +// Deliver all consecutivly associated COUNTs. +// Update RX_NEXT after submitting to higher layers +void pdcp_entity_nr::deliver_all_consecutive_counts() +{ + for (std::map::iterator it = reorder_queue.begin(); + it != reorder_queue.end() && it->first == rx_deliv; + reorder_queue.erase(it++)) + { + log->debug("Delivering SDU with RCVD_COUNT %d\n", it->first); + + // Pass PDCP SDU to the next layers + pass_to_upper_layers(std::move(it->second)); + + // Update RX_DELIV + rx_deliv = rx_deliv + 1; // TODO needs to be corrected when queueing is implemented + } +} + +void pdcp_entity_nr::reordering_callback::timer_expired(uint32_t timer_id) +{ + parent->log->debug("Reordering timer expired\n"); + + // Deliver all PDCP SDU(s) with associeted COUNT value(s) < RX_REORD + for (std::map::iterator it = parent->reorder_queue.begin(); + it != parent->reorder_queue.end() && it->first < parent->rx_reord; + parent->reorder_queue.erase(it++)) { + // Deliver to upper layers + parent->pass_to_upper_layers(std::move(it->second)); + } + + // Deliver all PDCP SDU(s) consecutivly associeted COUNT value(s) starting from RX_REORD + parent->deliver_all_consecutive_counts(); + + if (parent->rx_deliv < parent->rx_next){ + + } + return; +} } // namespace srslte diff --git a/lib/test/upper/pdcp_nr_test.cc b/lib/test/upper/pdcp_nr_test.cc index 65a4fdf8d..38bdcb714 100644 --- a/lib/test/upper/pdcp_nr_test.cc +++ b/lib/test/upper/pdcp_nr_test.cc @@ -148,8 +148,12 @@ int test_tx(uint32_t n_packets, srslte::log* log) { srslte::pdcp_entity_nr pdcp; - srslte::pdcp_config_t cfg = { - 1, srslte::PDCP_RB_IS_DRB, srslte::SECURITY_DIRECTION_UPLINK, srslte::SECURITY_DIRECTION_DOWNLINK, pdcp_sn_len}; + srslte::pdcp_config_t cfg = {1, + srslte::PDCP_RB_IS_DRB, + srslte::SECURITY_DIRECTION_UPLINK, + srslte::SECURITY_DIRECTION_DOWNLINK, + pdcp_sn_len, + srslte::pdcp_t_reordering_t::ms500}; rlc_dummy rlc(log); rrc_dummy rrc(log); @@ -273,10 +277,20 @@ int test_rx_in_sequence(uint64_t n_packets, uint8_t pdcp_sn_len, srslte::byte_bu { srslte::pdcp_entity_nr pdcp_tx; srslte::pdcp_entity_nr pdcp_rx; - srslte::pdcp_config_t cfg_tx = { - 1, srslte::PDCP_RB_IS_DRB, srslte::SECURITY_DIRECTION_UPLINK, srslte::SECURITY_DIRECTION_DOWNLINK, pdcp_sn_len}; - srslte::pdcp_config_t cfg_rx = { - 1, srslte::PDCP_RB_IS_DRB, srslte::SECURITY_DIRECTION_DOWNLINK, srslte::SECURITY_DIRECTION_UPLINK, pdcp_sn_len}; + + srslte::pdcp_config_t cfg_tx = {1, + srslte::PDCP_RB_IS_DRB, + srslte::SECURITY_DIRECTION_UPLINK, + srslte::SECURITY_DIRECTION_DOWNLINK, + pdcp_sn_len, + srslte::pdcp_t_reordering_t::ms500}; + + srslte::pdcp_config_t cfg_rx = {1, + srslte::PDCP_RB_IS_DRB, + srslte::SECURITY_DIRECTION_DOWNLINK, + srslte::SECURITY_DIRECTION_UPLINK, + pdcp_sn_len, + srslte::pdcp_t_reordering_t::ms500}; rlc_dummy rlc_tx(log); rrc_dummy rrc_tx(log); @@ -334,8 +348,12 @@ int test_rx_in_sequence(uint64_t n_packets, uint8_t pdcp_sn_len, srslte::byte_bu int test_rx_out_of_order(uint8_t pdcp_sn_len, srslte::byte_buffer_pool* pool, srslte::log* log) { srslte::pdcp_entity_nr pdcp_rx; - srslte::pdcp_config_t cfg_rx = { - 1, srslte::PDCP_RB_IS_DRB, srslte::SECURITY_DIRECTION_DOWNLINK, srslte::SECURITY_DIRECTION_UPLINK, pdcp_sn_len}; + srslte::pdcp_config_t cfg_rx = {1, + srslte::PDCP_RB_IS_DRB, + srslte::SECURITY_DIRECTION_DOWNLINK, + srslte::SECURITY_DIRECTION_UPLINK, + pdcp_sn_len, + srslte::pdcp_t_reordering_t::ms500}; rlc_dummy rlc_rx(log); rrc_dummy rrc_rx(log); @@ -380,8 +398,12 @@ int test_rx_out_of_order(uint8_t pdcp_sn_len, srslte::byte_buffer_pool* pool, sr int test_rx_out_of_order_timeout(uint8_t pdcp_sn_len, srslte::byte_buffer_pool* pool, srslte::log* log) { srslte::pdcp_entity_nr pdcp_rx; - srslte::pdcp_config_t cfg_rx = { - 1, srslte::PDCP_RB_IS_DRB, srslte::SECURITY_DIRECTION_DOWNLINK, srslte::SECURITY_DIRECTION_UPLINK, pdcp_sn_len}; + srslte::pdcp_config_t cfg_rx = {1, + srslte::PDCP_RB_IS_DRB, + srslte::SECURITY_DIRECTION_DOWNLINK, + srslte::SECURITY_DIRECTION_UPLINK, + pdcp_sn_len, + srslte::pdcp_t_reordering_t::ms500}; rlc_dummy rlc_rx(log); rrc_dummy rrc_rx(log); @@ -406,11 +428,15 @@ int test_rx_out_of_order_timeout(uint8_t pdcp_sn_len, srslte::byte_buffer_pool* // decript and check matching SDUs (out of order) pdcp_rx.write_pdu(std::move(rx_pdu7)); - gw_rx.get_last_pdu(sdu_act); // Make sure out of order is not received until time out TESTASSERT(gw_rx.rx_count == 0); - + for (uint16_t i = 0; i < 500; ++i){ + timers_rx.step_all(); + } + //Trigger timer + TESTASSERT(gw_rx.rx_count == 1); + gw_rx.get_last_pdu(sdu_act); TESTASSERT(sdu_exp->N_bytes == sdu_act->N_bytes); for (uint32_t j = 0; j < sdu_act->N_bytes; ++j) { TESTASSERT(sdu_exp->msg[j] == sdu_act->msg[j]); diff --git a/srsenb/src/stack/rrc/rrc.cc b/srsenb/src/stack/rrc/rrc.cc index e9ba02e34..03969aa36 100644 --- a/srsenb/src/stack/rrc/rrc.cc +++ b/srsenb/src/stack/rrc/rrc.cc @@ -218,11 +218,12 @@ void rrc::add_user(uint16_t rnti) } if (rnti == SRSLTE_MRNTI) { - srslte::pdcp_config_t cfg_{1, - srslte::PDCP_RB_IS_DRB, - srslte::SECURITY_DIRECTION_DOWNLINK, - srslte::SECURITY_DIRECTION_UPLINK, - srslte::PDCP_SN_LEN_12}; + srslte::pdcp_config_t cfg = {.bearer_id = 1, + .rb_type = srslte::PDCP_RB_IS_DRB, + .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, + .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, + .sn_len = srslte::PDCP_SN_LEN_12, + .t_reorderding = srslte::pdcp_t_reordering_t::ms500}; uint32_t teid_in = 1; @@ -1646,11 +1647,13 @@ void rrc::ue::send_connection_setup(bool is_setup) parent->rlc->add_bearer(rnti, 1, srslte::rlc_config_t::srb_config(1)); // Configure SRB1 in PDCP - srslte::pdcp_config_t pdcp_cnfg{.bearer_id = 1, - .rb_type = srslte::PDCP_RB_IS_SRB, - .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, - .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, - .sn_len = srslte::PDCP_SN_LEN_5}; + srslte::pdcp_config_t pdcp_cnfg{.bearer_id = 1, + .rb_type = srslte::PDCP_RB_IS_SRB, + .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, + .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, + .sn_len = srslte::PDCP_SN_LEN_5, + .t_reorderding = srslte::pdcp_t_reordering_t::ms500}; + parent->pdcp->add_bearer(rnti, 1, pdcp_cnfg); // Configure PHY layer @@ -1865,11 +1868,13 @@ void rrc::ue::send_connection_reconf(srslte::unique_byte_buffer_t pdu) parent->rlc->add_bearer(rnti, 2, srslte::rlc_config_t::srb_config(2)); // Configure SRB2 in PDCP - srslte::pdcp_config_t pdcp_cnfg_srb = {.bearer_id = 2, - .rb_type = srslte::PDCP_RB_IS_SRB, - .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, - .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, - .sn_len = srslte::PDCP_SN_LEN_5}; + srslte::pdcp_config_t pdcp_cnfg_srb = {.bearer_id = 2, + .rb_type = srslte::PDCP_RB_IS_SRB, + .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, + .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, + .sn_len = srslte::PDCP_SN_LEN_5, + .t_reorderding = srslte::pdcp_t_reordering_t::ms500}; + parent->pdcp->add_bearer(rnti, 2, pdcp_cnfg_srb); parent->pdcp->config_security(rnti, 2, k_rrc_enc, k_rrc_int, k_up_enc, cipher_algo, integ_algo); parent->pdcp->enable_integrity(rnti, 2); @@ -1879,11 +1884,13 @@ void rrc::ue::send_connection_reconf(srslte::unique_byte_buffer_t pdu) parent->rlc->add_bearer(rnti, 3, srslte::make_rlc_config_t(conn_reconf->rr_cfg_ded.drb_to_add_mod_list[0].rlc_cfg)); // Configure DRB1 in PDCP - srslte::pdcp_config_t pdcp_cnfg_drb = {.bearer_id = 1, - .rb_type = srslte::PDCP_RB_IS_DRB, - .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, - .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, - .sn_len = srslte::PDCP_SN_LEN_12}; + srslte::pdcp_config_t pdcp_cnfg_drb = {.bearer_id = 1, + .rb_type = srslte::PDCP_RB_IS_DRB, + .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, + .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, + .sn_len = srslte::PDCP_SN_LEN_12, + .t_reorderding = srslte::pdcp_t_reordering_t::ms500}; + if (conn_reconf->rr_cfg_ded.drb_to_add_mod_list[0].pdcp_cfg.rlc_um_present) { if (conn_reconf->rr_cfg_ded.drb_to_add_mod_list[0].pdcp_cfg.rlc_um.pdcp_sn_size.value == pdcp_cfg_s::rlc_um_s_::pdcp_sn_size_e_::len7bits) { @@ -1952,11 +1959,13 @@ void rrc::ue::send_connection_reconf_new_bearer(LIBLTE_S1AP_E_RABTOBESETUPLISTBE // Configure DRB in PDCP srslte::pdcp_config_t pdcp_config = { - .bearer_id = (uint8_t)(drb_item.drb_id - 1), // TODO: Review all ID mapping LCID DRB ERAB EPSBID Mapping - .rb_type = srslte::PDCP_RB_IS_DRB, - .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, - .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, - .sn_len = srslte::PDCP_SN_LEN_12}; + .bearer_id = (uint8_t)(drb_item.drb_id - 1), // TODO: Review all ID mapping LCID DRB ERAB EPSBID Mapping + .rb_type = srslte::PDCP_RB_IS_DRB, + .tx_direction = srslte::SECURITY_DIRECTION_DOWNLINK, + .rx_direction = srslte::SECURITY_DIRECTION_UPLINK, + .sn_len = srslte::PDCP_SN_LEN_12, + .t_reorderding = srslte::pdcp_t_reordering_t::ms500}; + parent->pdcp->add_bearer(rnti, lcid, pdcp_config); // DRB has already been configured in GTPU through bearer setup diff --git a/srsue/src/stack/rrc/rrc.cc b/srsue/src/stack/rrc/rrc.cc index d15e8d331..76565c070 100644 --- a/srsue/src/stack/rrc/rrc.cc +++ b/srsue/src/stack/rrc/rrc.cc @@ -2684,7 +2684,9 @@ void rrc::add_srb(srb_to_add_mod_s* srb_cnfg) .rb_type = PDCP_RB_IS_SRB, .tx_direction = SECURITY_DIRECTION_UPLINK, .rx_direction = SECURITY_DIRECTION_DOWNLINK, - .sn_len = PDCP_SN_LEN_5}; + .sn_len = PDCP_SN_LEN_5, + .t_reordering = pdcp_t_reordering_t::ms500}; + pdcp->add_bearer(srb_cnfg->srb_id, pdcp_cfg); if (RB_ID_SRB2 == srb_cnfg->srb_id) { pdcp->config_security(srb_cnfg->srb_id, k_rrc_enc, k_rrc_int, k_up_enc, cipher_algo, integ_algo); @@ -2761,7 +2763,9 @@ void rrc::add_drb(drb_to_add_mod_s* drb_cnfg) .rb_type = PDCP_RB_IS_DRB, .tx_direction = SECURITY_DIRECTION_UPLINK, .rx_direction = SECURITY_DIRECTION_DOWNLINK, - .sn_len = PDCP_SN_LEN_12}; + .sn_len = PDCP_SN_LEN_12, + .t_reordering = pdcp_t_reordering_t::ms500}; + if (drb_cnfg->pdcp_cfg.rlc_um_present) { if (drb_cnfg->pdcp_cfg.rlc_um.pdcp_sn_size == pdcp_cfg_s::rlc_um_s_::pdcp_sn_size_e_::len7bits) { pdcp_cfg.sn_len = 7;