From 7842e3bff3a784be8205b9f7cce600d4598b606e Mon Sep 17 00:00:00 2001 From: Francisco Date: Mon, 12 Apr 2021 16:16:27 +0100 Subject: [PATCH] s1ap - add unit test to check correct handling of invalid E-RAB ids in modification request. Fix s1ap in order for the test to pass --- .../srsran/interfaces/enb_rrc_interfaces.h | 4 +- srsenb/hdr/stack/rrc/rrc.h | 7 +- srsenb/hdr/stack/rrc/rrc_ue.h | 3 + srsenb/src/stack/rrc/rrc.cc | 17 +++-- srsenb/src/stack/upper/s1ap.cc | 68 ++++++++++------- srsenb/test/common/dummy_classes.h | 7 +- srsenb/test/upper/s1ap_test.cc | 76 +++++++++++++------ 7 files changed, 120 insertions(+), 62 deletions(-) diff --git a/lib/include/srsran/interfaces/enb_rrc_interfaces.h b/lib/include/srsran/interfaces/enb_rrc_interfaces.h index 1a1e88ad8..c39b56a89 100644 --- a/lib/include/srsran/interfaces/enb_rrc_interfaces.h +++ b/lib/include/srsran/interfaces/enb_rrc_interfaces.h @@ -30,8 +30,8 @@ public: virtual bool setup_ue_erabs(uint16_t rnti, const asn1::s1ap::erab_setup_request_s& msg) = 0; virtual void modify_erabs(uint16_t rnti, - srsran::const_span erabs_to_modify, - std::vector* erabs_failed_to_modify) = 0; + srsran::const_span erabs_to_modify) = 0; + virtual bool has_erab(uint16_t rnti, uint32_t erab_id) const = 0; virtual bool release_erabs(uint32_t rnti) = 0; virtual void release_erabs(uint32_t rnti, const asn1::s1ap::erab_release_cmd_s& msg, diff --git a/srsenb/hdr/stack/rrc/rrc.h b/srsenb/hdr/stack/rrc/rrc.h index 8fc4b8bf4..bda0a43a6 100644 --- a/srsenb/hdr/stack/rrc/rrc.h +++ b/srsenb/hdr/stack/rrc/rrc.h @@ -82,9 +82,10 @@ public: bool setup_ue_ctxt(uint16_t rnti, const asn1::s1ap::init_context_setup_request_s& msg) override; bool modify_ue_ctxt(uint16_t rnti, const asn1::s1ap::ue_context_mod_request_s& msg) override; bool setup_ue_erabs(uint16_t rnti, const asn1::s1ap::erab_setup_request_s& msg) override; - void modify_erabs(uint16_t rnti, - srsran::const_span erabs_to_modify, - std::vector* erabs_failed_to_modify) override; + bool has_erab(uint16_t rnti, uint32_t erab_id) const override; + void modify_erabs( + uint16_t rnti, + srsran::const_span erabs_to_modify) override; bool modify_ue_erab(uint16_t rnti, uint8_t erab_id, const asn1::s1ap::erab_level_qos_params_s& qos_params, diff --git a/srsenb/hdr/stack/rrc/rrc_ue.h b/srsenb/hdr/stack/rrc/rrc_ue.h index c6e21f5d7..ffa1cc7f3 100644 --- a/srsenb/hdr/stack/rrc/rrc_ue.h +++ b/srsenb/hdr/stack/rrc/rrc_ue.h @@ -53,6 +53,9 @@ public: ///< Helper to access a cell cfg based on ue_cc_idx enb_cell_common* get_ue_cc_cfg(uint32_t ue_cc_idx); + /// Helper to check UE ERABs + bool has_erab(uint32_t erab_id) const { return bearer_list.get_erabs().count(erab_id) > 0; } + /// List of results a RRC procedure may produce. enum class procedure_result_code { none, diff --git a/srsenb/src/stack/rrc/rrc.cc b/srsenb/src/stack/rrc/rrc.cc index 3171e8bb3..0c9390026 100644 --- a/srsenb/src/stack/rrc/rrc.cc +++ b/srsenb/src/stack/rrc/rrc.cc @@ -386,9 +386,18 @@ void rrc::release_erabs(uint32_t rnti, user_it->second->send_connection_reconf(nullptr, false, nas_pdu); } +bool rrc::has_erab(uint16_t rnti, uint32_t erab_id) const +{ + auto user_it = users.find(rnti); + if (user_it == users.end()) { + logger.warning("Unrecognised rnti: 0x%x", rnti); + return false; + } + return user_it->second->has_erab(erab_id); +} + void rrc::modify_erabs(uint16_t rnti, - srsran::const_span erabs_to_modify, - std::vector* erabs_failed_to_modify) + srsran::const_span erabs_to_modify) { logger.info("Modifying E-RABs for 0x%x", rnti); auto user_it = users.find(rnti); @@ -401,9 +410,7 @@ void rrc::modify_erabs(uint16_t for (const auto* erab_ptr : erabs_to_modify) { // Attempt to modify E-RAB bool ret = modify_ue_erab(rnti, erab_ptr->erab_id, erab_ptr->erab_level_qos_params, &erab_ptr->nas_pdu); - if (not ret) { - erabs_failed_to_modify->push_back(erab_ptr->erab_id); - } + srsran_expect(ret, "modify_erabs should not called for valid E-RAB Ids"); } } diff --git a/srsenb/src/stack/upper/s1ap.cc b/srsenb/src/stack/upper/s1ap.cc index 6b7f1f62c..04cede167 100644 --- a/srsenb/src/stack/upper/s1ap.cc +++ b/srsenb/src/stack/upper/s1ap.cc @@ -736,39 +736,53 @@ bool s1ap::handle_erabmodifyrequest(const erab_modify_request_s& msg) } // make a copy, sort by ERAB ID - using erab_t = erab_to_be_modified_item_bearer_mod_req_s; + using erab_t = erab_to_be_modified_item_bearer_mod_req_s; + using failed_erab_t = std::pair; srsran::bounded_vector erab_mod_list; - for (const auto& erab : msg.protocol_ies.erab_to_be_modified_list_bearer_mod_req.value) { - erab_mod_list.push_back(&erab.value.erab_to_be_modified_item_bearer_mod_req()); - } - auto lower_erab = [](const erab_t* lhs, const erab_t* rhs) { return lhs->erab_id < rhs->erab_id; }; - std::sort(erab_mod_list.begin(), erab_mod_list.end(), lower_erab); + std::vector erabs_failed_to_modify; + auto& msg_erabs = msg.protocol_ies.erab_to_be_modified_list_bearer_mod_req.value; - // Find repeated ERAB-IDs and add them to list of ERABs failed to modify - std::vector > erabs_failed_to_modify; - auto new_end = std::unique(erab_mod_list.begin(), erab_mod_list.end()); - for (auto it = new_end; it != erab_mod_list.end(); ++it) { - cause_c cause; - cause.set_radio_network().value = cause_radio_network_opts::multiple_erab_id_instances; - erabs_failed_to_modify.emplace_back((*it)->erab_id, cause); + for (const auto& msg_erab : msg_erabs) { + const erab_t& e = msg_erab.value.erab_to_be_modified_item_bearer_mod_req(); + + // Check if E-RAB exists. If not, add to "erabs_failed_to_modify" with "unknown_erab_id" cause + if (not rrc->has_erab(u->ctxt.rnti, e.erab_id)) { + cause_c cause; + cause.set_radio_network().value = cause_radio_network_opts::unknown_erab_id; + erabs_failed_to_modify.emplace_back(e.erab_id, cause); + continue; + } + + // Check Repeated E-RABs in the modification list. If repeated, add to the list of "erabs_failed_to_modify" + // with cause "multiple_erab_id_instances" + for (const auto& msg_erab2 : msg_erabs) { + const erab_t& e2 = msg_erab2.value.erab_to_be_modified_item_bearer_mod_req(); + if (&msg_erab2 != &msg_erab and e2.erab_id == e.erab_id) { + cause_c cause; + cause.set_radio_network().value = cause_radio_network_opts::multiple_erab_id_instances; + erabs_failed_to_modify.emplace_back(e.erab_id, cause); + break; + } + } + if (not erabs_failed_to_modify.empty() and erabs_failed_to_modify.back().first == e.erab_id) { + continue; + } + + // Add to the list to modify + erab_mod_list.push_back(&e); } - erab_mod_list.erase(new_end, erab_mod_list.end()); // Modify E-RABs from RRC - std::vector unknown_erabids; - rrc->modify_erabs(u->ctxt.rnti, erab_mod_list, &unknown_erabids); + rrc->modify_erabs(u->ctxt.rnti, erab_mod_list); - // Add Unknown E-RAB to the list of failed to modify - for (uint16_t erab : unknown_erabids) { - cause_c cause; - cause.set_radio_network().value = cause_radio_network_opts::unknown_erab_id; - erabs_failed_to_modify.emplace_back(erab, cause); - auto lower_erab2 = [](const erab_t* lhs, uint16_t erab) { return lhs->erab_id < erab; }; - auto it = std::lower_bound(erab_mod_list.begin(), erab_mod_list.end(), erab, lower_erab2); - if (it != erab_mod_list.end() and (*it)->erab_id == erab) { - erab_mod_list.erase(it); - } - } + // Sort by E-RAB id, and remove duplicates + auto lower_erab = [](const erab_t* lhs, const erab_t* rhs) { return lhs->erab_id < rhs->erab_id; }; + std::sort(erab_mod_list.begin(), erab_mod_list.end(), lower_erab); + auto lower_erab2 = [](const failed_erab_t& lhs, const failed_erab_t& rhs) { return lhs.first < rhs.first; }; + auto equal_erab2 = [](const failed_erab_t& lhs, const failed_erab_t& rhs) { return lhs.first == rhs.first; }; + std::sort(erabs_failed_to_modify.begin(), erabs_failed_to_modify.end(), lower_erab2); + erabs_failed_to_modify.erase(std::unique(erabs_failed_to_modify.begin(), erabs_failed_to_modify.end(), equal_erab2), + erabs_failed_to_modify.end()); // Send E-RAB modify response back to the MME if (not u->send_erab_modify_response(erab_mod_list, erabs_failed_to_modify)) { diff --git a/srsenb/test/common/dummy_classes.h b/srsenb/test/common/dummy_classes.h index 08088070e..5e5b62cdd 100644 --- a/srsenb/test/common/dummy_classes.h +++ b/srsenb/test/common/dummy_classes.h @@ -168,10 +168,11 @@ public: bool setup_ue_ctxt(uint16_t rnti, const asn1::s1ap::init_context_setup_request_s& msg) override { return true; } bool modify_ue_ctxt(uint16_t rnti, const asn1::s1ap::ue_context_mod_request_s& msg) override { return true; } bool setup_ue_erabs(uint16_t rnti, const asn1::s1ap::erab_setup_request_s& msg) override { return true; } - void modify_erabs(uint16_t rnti, - srsran::const_span erabs_to_modify, - std::vector* erabs_failed_to_modify) override + void modify_erabs( + uint16_t rnti, + srsran::const_span erabs_to_modify) override {} + bool has_erab(uint16_t rnti, uint32_t erab_id) const override { return true; } bool release_erabs(uint32_t rnti) override { return true; } void release_erabs(uint32_t rnti, const asn1::s1ap::erab_release_cmd_s& msg, diff --git a/srsenb/test/upper/s1ap_test.cc b/srsenb/test/upper/s1ap_test.cc index ad41e1f61..aa67162f1 100644 --- a/srsenb/test/upper/s1ap_test.cc +++ b/srsenb/test/upper/s1ap_test.cc @@ -91,16 +91,18 @@ struct dummy_socket_manager : public srsran::socket_manager_itf { }; struct rrc_tester : public rrc_dummy { - void modify_erabs(uint16_t rnti, - srsran::const_span erabs_to_modify, - std::vector* erabs_failed_to_modify) override + void modify_erabs( + uint16_t rnti, + srsran::const_span erabs_to_modify) override { + last_erabs_modified.clear(); for (auto& erab : erabs_to_modify) { - if (std::count(next_erabs_failed_to_modify.begin(), next_erabs_failed_to_modify.end(), erab->erab_id) == 0) { - last_erabs_modified.push_back(erab->erab_id); - } + last_erabs_modified.push_back(erab->erab_id); } - *erabs_failed_to_modify = next_erabs_failed_to_modify; + } + bool has_erab(uint16_t rnti, uint32_t erab_id) const override + { + return std::count(next_erabs_failed_to_modify.begin(), next_erabs_failed_to_modify.end(), erab_id) == 0; } void release_ue(uint16_t rnti) override { last_released_rnti = rnti; } @@ -187,7 +189,7 @@ void add_rnti(s1ap& s1ap_obj, mme_dummy& mme) TESTASSERT(s1ap_pdu.successful_outcome().proc_code == ASN1_S1AP_ID_INIT_CONTEXT_SETUP); } -enum class test_event { success, wrong_erabid_mod, wrong_mme_s1ap_id }; +enum class test_event { success, wrong_erabid_mod, wrong_mme_s1ap_id, repeated_erabid_mod }; void test_s1ap_erab_setup(test_event event) { @@ -220,22 +222,37 @@ void test_s1ap_erab_setup(test_event event) add_rnti(s1ap_obj, mme); // E-RAB Modify Request - sockaddr_in mme_addr = {}; - sctp_sndrcvinfo rcvinfo = {}; - int flags = 0; - uint8_t mod_req_msg[] = {0x00, 0x06, 0x00, 0x1E, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x02, 0x00, - 0x01, 0x00, 0x08, 0x00, 0x02, 0x00, 0x01, 0x00, 0x1E, 0x00, 0x0B, 0x00, - 0x00, 0x24, 0x00, 0x06, 0x0A, 0x00, 0x09, 0x3C, 0x01, 0x00}; - // 00 06 00 1E 00 00 03 00 00 00 02 00 01 00 08 00 02 00 01 00 1E 00 0B 00 00 24 00 06 0A 00 09 3C 01 00 + sockaddr_in mme_addr = {}; + sctp_sndrcvinfo rcvinfo = {}; + int flags = 0; + asn1::s1ap::s1ap_pdu_c mod_req_pdu; + mod_req_pdu.set_init_msg().load_info_obj(ASN1_S1AP_ID_ERAB_MODIFY); + auto& protocols = mod_req_pdu.init_msg().value.erab_modify_request().protocol_ies; + protocols.enb_ue_s1ap_id.value = 1; + protocols.mme_ue_s1ap_id.value = event == test_event::wrong_mme_s1ap_id ? 2 : 1; + auto& erab_list = protocols.erab_to_be_modified_list_bearer_mod_req.value; + erab_list.resize(2); + erab_list[0].load_info_obj(ASN1_S1AP_ID_ERAB_TO_BE_MODIFIED_ITEM_BEARER_MOD_REQ); + auto* erab_ptr = &erab_list[0].value.erab_to_be_modified_item_bearer_mod_req(); + erab_ptr->erab_id = 5; + erab_ptr->erab_level_qos_params.qci = 9; + erab_ptr->erab_level_qos_params.alloc_retention_prio.prio_level = 15; + erab_ptr->erab_level_qos_params.alloc_retention_prio.pre_emption_cap.value = + asn1::s1ap::pre_emption_cap_opts::shall_not_trigger_pre_emption; + erab_ptr->erab_level_qos_params.alloc_retention_prio.pre_emption_vulnerability.value = + asn1::s1ap::pre_emption_vulnerability_opts::not_pre_emptable; + erab_ptr->nas_pdu.resize(1); + erab_list[1] = erab_list[0]; + erab_ptr = &erab_list[1].value.erab_to_be_modified_item_bearer_mod_req(); + erab_ptr->erab_id = event == test_event::repeated_erabid_mod ? 5 : 6; if (event == test_event::wrong_erabid_mod) { - mod_req_msg[sizeof(mod_req_msg) - 6] = 0x0C; // E-RAB id = 6 rrc.next_erabs_failed_to_modify.push_back(6); - } else if (event == test_event::wrong_mme_s1ap_id) { - mod_req_msg[12] = 0x02; // MME-UE-S1AP-ID = 2 } sdu = srsran::make_byte_buffer(); - memcpy(sdu->msg, mod_req_msg, sizeof(mod_req_msg)); - sdu->N_bytes = sizeof(mod_req_msg); + asn1::bit_ref bref(sdu->msg, sdu->get_tailroom()); + TESTASSERT(mod_req_pdu.pack(bref) == SRSRAN_SUCCESS); + sdu->N_bytes = bref.distance_bytes(); + TESTASSERT(rrc.last_released_rnti == SRSRAN_INVALID_RNTI); TESTASSERT(s1ap_obj.handle_mme_rx_msg(std::move(sdu), mme_addr, rcvinfo, flags)); sdu = mme.read_msg(); @@ -258,7 +275,10 @@ void test_s1ap_erab_setup(test_event event) TESTASSERT(s1ap_pdu.successful_outcome().proc_code == ASN1_S1AP_ID_ERAB_MODIFY); auto& protocol_ies = s1ap_pdu.successful_outcome().value.erab_modify_resp().protocol_ies; if (event == test_event::wrong_erabid_mod) { - TESTASSERT(not protocol_ies.erab_modify_list_bearer_mod_res_present); + TESTASSERT(protocol_ies.erab_modify_list_bearer_mod_res_present); + TESTASSERT(protocol_ies.erab_modify_list_bearer_mod_res.value.size() == 1); + TESTASSERT(protocol_ies.erab_modify_list_bearer_mod_res.value[0].value.erab_modify_item_bearer_mod_res().erab_id == + 5); TESTASSERT(protocol_ies.erab_failed_to_modify_list_present); TESTASSERT(protocol_ies.erab_failed_to_modify_list.value.size() == 1); auto& erab_item = protocol_ies.erab_failed_to_modify_list.value[0].value.erab_item(); @@ -267,10 +287,21 @@ void test_s1ap_erab_setup(test_event event) TESTASSERT(erab_item.cause.radio_network().value == asn1::s1ap::cause_radio_network_opts::unknown_erab_id); return; } + if (event == test_event::repeated_erabid_mod) { + TESTASSERT(not protocol_ies.erab_modify_list_bearer_mod_res_present); + TESTASSERT(protocol_ies.erab_failed_to_modify_list_present); + TESTASSERT(protocol_ies.erab_failed_to_modify_list.value.size() == 1); + auto& erab_item = protocol_ies.erab_failed_to_modify_list.value[0].value.erab_item(); + TESTASSERT(erab_item.erab_id == 5); + TESTASSERT(erab_item.cause.type().value == asn1::s1ap::cause_c::types_opts::radio_network); + TESTASSERT(erab_item.cause.radio_network().value == + asn1::s1ap::cause_radio_network_opts::multiple_erab_id_instances); + return; + } TESTASSERT(protocol_ies.erab_modify_list_bearer_mod_res_present); TESTASSERT(not protocol_ies.erab_failed_to_modify_list_present); - TESTASSERT(protocol_ies.erab_modify_list_bearer_mod_res.value.size() == 1); + TESTASSERT(protocol_ies.erab_modify_list_bearer_mod_res.value.size() == 2); auto& erab_item = protocol_ies.erab_modify_list_bearer_mod_res.value[0].value.erab_modify_item_bearer_mod_res(); TESTASSERT(erab_item.erab_id == 5); } @@ -288,4 +319,5 @@ int main(int argc, char** argv) test_s1ap_erab_setup(test_event::success); test_s1ap_erab_setup(test_event::wrong_erabid_mod); test_s1ap_erab_setup(test_event::wrong_mme_s1ap_id); + test_s1ap_erab_setup(test_event::repeated_erabid_mod); } \ No newline at end of file