From 985846e3bd3a7e5655d23debb9270afe51f16ad8 Mon Sep 17 00:00:00 2001 From: Francisco Date: Mon, 12 Apr 2021 14:51:58 +0100 Subject: [PATCH] s1ap - set multiple erab ids cause in replay when s1ap erab modify request has repeated ids. --- lib/include/srsran/asn1/s1ap_utils.h | 1 + .../srsran/interfaces/enb_rrc_interfaces.h | 14 ++--- srsenb/hdr/stack/rrc/rrc.h | 7 +-- srsenb/hdr/stack/upper/s1ap.h | 5 +- srsenb/src/stack/rrc/rrc.cc | 38 +++---------- srsenb/src/stack/upper/s1ap.cc | 57 ++++++++++++++----- srsenb/test/common/dummy_classes.h | 7 +-- srsenb/test/upper/s1ap_test.cc | 17 +++--- 8 files changed, 75 insertions(+), 71 deletions(-) diff --git a/lib/include/srsran/asn1/s1ap_utils.h b/lib/include/srsran/asn1/s1ap_utils.h index 613ebbf52..6627f7852 100644 --- a/lib/include/srsran/asn1/s1ap_utils.h +++ b/lib/include/srsran/asn1/s1ap_utils.h @@ -38,6 +38,7 @@ struct bearers_subject_to_status_transfer_item_ies_o; struct erab_level_qos_params_s; struct ho_cmd_s; struct erab_admitted_item_s; +struct erab_to_be_modified_item_bearer_mod_req_s; template struct protocol_ie_single_container_s; diff --git a/lib/include/srsran/interfaces/enb_rrc_interfaces.h b/lib/include/srsran/interfaces/enb_rrc_interfaces.h index c5cb87957..1a1e88ad8 100644 --- a/lib/include/srsran/interfaces/enb_rrc_interfaces.h +++ b/lib/include/srsran/interfaces/enb_rrc_interfaces.h @@ -28,16 +28,16 @@ public: virtual bool setup_ue_ctxt(uint16_t rnti, const asn1::s1ap::init_context_setup_request_s& msg) = 0; virtual bool modify_ue_ctxt(uint16_t rnti, const asn1::s1ap::ue_context_mod_request_s& msg) = 0; virtual bool setup_ue_erabs(uint16_t rnti, const asn1::s1ap::erab_setup_request_s& msg) = 0; - virtual void modify_erabs(uint16_t rnti, - const asn1::s1ap::erab_modify_request_s& msg, - std::vector* erabs_modified, - std::vector* erabs_failed_to_modify) = 0; - virtual bool release_erabs(uint32_t rnti) = 0; + virtual void + modify_erabs(uint16_t rnti, + srsran::const_span erabs_to_modify, + std::vector* erabs_failed_to_modify) = 0; + virtual bool release_erabs(uint32_t rnti) = 0; virtual void release_erabs(uint32_t rnti, const asn1::s1ap::erab_release_cmd_s& msg, std::vector* erabs_released, - std::vector* erabs_failed_to_release) = 0; - virtual void add_paging_id(uint32_t ueid, const asn1::s1ap::ue_paging_id_c& ue_paging_id) = 0; + std::vector* erabs_failed_to_release) = 0; + virtual void add_paging_id(uint32_t ueid, const asn1::s1ap::ue_paging_id_c& ue_paging_id) = 0; /** * Reports the reception of S1 HandoverCommand / HandoverPreparationFailure or abnormal conditions during diff --git a/srsenb/hdr/stack/rrc/rrc.h b/srsenb/hdr/stack/rrc/rrc.h index 0d38cd8a4..8fc4b8bf4 100644 --- a/srsenb/hdr/stack/rrc/rrc.h +++ b/srsenb/hdr/stack/rrc/rrc.h @@ -82,10 +82,9 @@ 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, - const asn1::s1ap::erab_modify_request_s& msg, - std::vector* erabs_modified, - std::vector* erabs_failed_to_modify) override; + void modify_erabs(uint16_t rnti, + srsran::const_span erabs_to_modify, + std::vector* erabs_failed_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/upper/s1ap.h b/srsenb/hdr/stack/upper/s1ap.h index 9506c709b..a365876a0 100644 --- a/srsenb/hdr/stack/upper/s1ap.h +++ b/srsenb/hdr/stack/upper/s1ap.h @@ -239,8 +239,9 @@ private: bool send_erab_setup_response(const asn1::s1ap::erab_setup_resp_s& res_); bool send_erab_release_response(const std::vector& erabs_successfully_released, const std::vector& erabs_failed_to_release); - bool send_erab_modify_response(const std::vector& erabs_successfully_released, - const std::vector& erabs_failed_to_release); + bool send_erab_modify_response( + srsran::const_span erabs_modified, + srsran::const_span > erabs_failed_to_modify); bool send_erab_release_indication(const std::vector& erabs_successfully_released); bool send_ue_cap_info_indication(srsran::unique_byte_buffer_t ue_radio_cap); diff --git a/srsenb/src/stack/rrc/rrc.cc b/srsenb/src/stack/rrc/rrc.cc index 36b4d72bf..3171e8bb3 100644 --- a/srsenb/src/stack/rrc/rrc.cc +++ b/srsenb/src/stack/rrc/rrc.cc @@ -386,10 +386,9 @@ void rrc::release_erabs(uint32_t rnti, user_it->second->send_connection_reconf(nullptr, false, nas_pdu); } -void rrc::modify_erabs(uint16_t rnti, - const asn1::s1ap::erab_modify_request_s& msg, - std::vector* erabs_modified, - std::vector* erabs_failed_to_modify) +void rrc::modify_erabs(uint16_t rnti, + srsran::const_span erabs_to_modify, + std::vector* erabs_failed_to_modify) { logger.info("Modifying E-RABs for 0x%x", rnti); auto user_it = users.find(rnti); @@ -399,36 +398,13 @@ void rrc::modify_erabs(uint16_t rnti, return; } - const auto& erab_mod_list = msg.protocol_ies.erab_to_be_modified_list_bearer_mod_req.value; - - for (const auto* it = erab_mod_list.begin(); it != erab_mod_list.end(); ++it) { - const auto& erab_to_mod = it->value.erab_to_be_modified_item_bearer_mod_req(); - - for (const auto* it2 = it + 1; it2 != erab_mod_list.end(); ++it2) { - // Detect repeated E-RAB IDs - if (it2->value.erab_to_be_modified_item_bearer_mod_req().erab_id == erab_to_mod.erab_id) { - erabs_failed_to_modify->push_back(it->id); - continue; - } - } - if (std::find(erabs_failed_to_modify->begin(), erabs_failed_to_modify->end(), erab_to_mod.erab_id) != - erabs_failed_to_modify->end()) { - // Already added to the list of E-RABs that modification as failed - continue; - } - + for (const auto* erab_ptr : erabs_to_modify) { // Attempt to modify E-RAB - bool ret = modify_ue_erab(rnti, erab_to_mod.erab_id, erab_to_mod.erab_level_qos_params, &erab_to_mod.nas_pdu); - if (ret) { - erabs_modified->push_back(erab_to_mod.erab_id); - } else { - erabs_failed_to_modify->push_back(erab_to_mod.erab_id); + 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); } } - - // Sort output vectors by ERAB ID - std::sort(erabs_modified->begin(), erabs_modified->end()); - std::sort(erabs_failed_to_modify->begin(), erabs_failed_to_modify->end()); } bool rrc::modify_ue_erab(uint16_t rnti, diff --git a/srsenb/src/stack/upper/s1ap.cc b/srsenb/src/stack/upper/s1ap.cc index ae12ad8ec..6b7f1f62c 100644 --- a/srsenb/src/stack/upper/s1ap.cc +++ b/srsenb/src/stack/upper/s1ap.cc @@ -726,9 +726,6 @@ bool s1ap::handle_erabsetuprequest(const erab_setup_request_s& msg) bool s1ap::handle_erabmodifyrequest(const erab_modify_request_s& msg) { - std::vector erab_successful_modified = {}; - std::vector erab_failed_to_modify = {}; - if (msg.ext) { logger.warning("Not handling S1AP message extension"); } @@ -738,11 +735,43 @@ bool s1ap::handle_erabmodifyrequest(const erab_modify_request_s& msg) return false; } + // make a copy, sort by ERAB ID + using erab_t = erab_to_be_modified_item_bearer_mod_req_s; + 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); + + // 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); + } + erab_mod_list.erase(new_end, erab_mod_list.end()); + // Modify E-RABs from RRC - rrc->modify_erabs(u->ctxt.rnti, msg, &erab_successful_modified, &erab_failed_to_modify); + std::vector unknown_erabids; + rrc->modify_erabs(u->ctxt.rnti, erab_mod_list, &unknown_erabids); + + // 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); + } + } // Send E-RAB modify response back to the MME - if (not u->send_erab_modify_response(erab_successful_modified, erab_failed_to_modify)) { + if (not u->send_erab_modify_response(erab_mod_list, erabs_failed_to_modify)) { logger.info("Failed to send ERABReleaseResponse"); return false; } @@ -1390,8 +1419,9 @@ bool s1ap::ue::send_erab_release_response(const std::vector& erabs_suc return s1ap_ptr->sctp_send_s1ap_pdu(tx_pdu, ctxt.rnti, "E-RABReleaseResponse"); } -bool s1ap::ue::send_erab_modify_response(const std::vector& erabs_successfully_modified, - const std::vector& erabs_failed_to_modify) +bool s1ap::ue::send_erab_modify_response( + srsran::const_span erabs_modified, + srsran::const_span > erabs_failed_to_modify) { if (not s1ap_ptr->mme_connected) { return false; @@ -1405,13 +1435,13 @@ bool s1ap::ue::send_erab_modify_response(const std::vector& erabs_succ container.mme_ue_s1ap_id.value = ctxt.mme_ue_s1ap_id.value(); // Fill in which E-RABs were successfully released - if (not erabs_successfully_modified.empty()) { + if (not erabs_modified.empty()) { container.erab_modify_list_bearer_mod_res_present = true; - container.erab_modify_list_bearer_mod_res.value.resize(erabs_successfully_modified.size()); + container.erab_modify_list_bearer_mod_res.value.resize(erabs_modified.size()); for (uint32_t i = 0; i < container.erab_modify_list_bearer_mod_res.value.size(); i++) { container.erab_modify_list_bearer_mod_res.value[i].load_info_obj(ASN1_S1AP_ID_ERAB_MODIFY_ITEM_BEARER_MOD_RES); container.erab_modify_list_bearer_mod_res.value[i].value.erab_modify_item_bearer_mod_res().erab_id = - erabs_successfully_modified[i]; + erabs_modified[i]->erab_id; } } @@ -1421,11 +1451,8 @@ bool s1ap::ue::send_erab_modify_response(const std::vector& erabs_succ container.erab_failed_to_modify_list.value.resize(erabs_failed_to_modify.size()); for (uint32_t i = 0; i < container.erab_failed_to_modify_list.value.size(); i++) { container.erab_failed_to_modify_list.value[i].load_info_obj(ASN1_S1AP_ID_ERAB_ITEM); - container.erab_failed_to_modify_list.value[i].value.erab_item().erab_id = erabs_failed_to_modify[i]; - container.erab_failed_to_modify_list.value[i].value.erab_item().cause.set( - asn1::s1ap::cause_c::types_opts::radio_network); - container.erab_failed_to_modify_list.value[i].value.erab_item().cause.radio_network().value = - cause_radio_network_opts::unknown_erab_id; + container.erab_failed_to_modify_list.value[i].value.erab_item().erab_id = erabs_failed_to_modify[i].first; + container.erab_failed_to_modify_list.value[i].value.erab_item().cause = erabs_failed_to_modify[i].second; } } diff --git a/srsenb/test/common/dummy_classes.h b/srsenb/test/common/dummy_classes.h index 45235babb..08088070e 100644 --- a/srsenb/test/common/dummy_classes.h +++ b/srsenb/test/common/dummy_classes.h @@ -168,10 +168,9 @@ 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, - const asn1::s1ap::erab_modify_request_s& msg, - std::vector* erabs_modified, - std::vector* erabs_failed_to_modify) override + void modify_erabs(uint16_t rnti, + srsran::const_span erabs_to_modify, + std::vector* erabs_failed_to_modify) override {} bool release_erabs(uint32_t rnti) override { return true; } void release_erabs(uint32_t rnti, diff --git a/srsenb/test/upper/s1ap_test.cc b/srsenb/test/upper/s1ap_test.cc index b4adf6205..ad41e1f61 100644 --- a/srsenb/test/upper/s1ap_test.cc +++ b/srsenb/test/upper/s1ap_test.cc @@ -91,18 +91,21 @@ struct dummy_socket_manager : public srsran::socket_manager_itf { }; struct rrc_tester : public rrc_dummy { - void modify_erabs(uint16_t rnti, - const asn1::s1ap::erab_modify_request_s& msg, - std::vector* erabs_modified, - std::vector* erabs_failed_to_modify) override + void modify_erabs(uint16_t rnti, + srsran::const_span erabs_to_modify, + std::vector* erabs_failed_to_modify) override { - *erabs_modified = next_erabs_modified; + 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); + } + } *erabs_failed_to_modify = next_erabs_failed_to_modify; } void release_ue(uint16_t rnti) override { last_released_rnti = rnti; } uint16_t last_released_rnti = SRSRAN_INVALID_RNTI; - std::vector next_erabs_modified, next_erabs_failed_to_modify; + std::vector next_erabs_failed_to_modify, last_erabs_modified; }; void run_s1_setup(s1ap& s1ap_obj, mme_dummy& mme) @@ -229,8 +232,6 @@ void test_s1ap_erab_setup(test_event event) 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 - } else { - rrc.next_erabs_modified.push_back(5); } sdu = srsran::make_byte_buffer(); memcpy(sdu->msg, mod_req_msg, sizeof(mod_req_msg));