From 167200f5cd338783ddbd3b98dcc5766be5fc90a7 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Wed, 5 May 2021 17:21:24 +0200 Subject: [PATCH] rrc_bearer_cfg: replace fixed E-RAB to DRB mapping also make sure we don't assign LCIDs beyond the possible number. possible fix for https://github.com/srsran/srsRAN/issues/658 Co-authored-by: herlesupreeth Co-authored-by: Francisco --- lib/include/srsran/common/common_lte.h | 5 ++-- lib/src/upper/pdcp.cc | 2 +- srsenb/hdr/stack/rrc/rrc_bearer_cfg.h | 3 ++- srsenb/hdr/stack/upper/gtpu.h | 2 +- srsenb/src/stack/rrc/rrc_bearer_cfg.cc | 32 ++++++++++++++++++++++---- srsenb/src/stack/rrc/rrc_mobility.cc | 2 +- srsenb/src/stack/rrc/rrc_ue.cc | 11 ++++----- 7 files changed, 41 insertions(+), 16 deletions(-) diff --git a/lib/include/srsran/common/common_lte.h b/lib/include/srsran/common/common_lte.h index f650dc2e3..0152a692b 100644 --- a/lib/include/srsran/common/common_lte.h +++ b/lib/include/srsran/common/common_lte.h @@ -50,11 +50,12 @@ enum class lte_srb { srb0, srb1, srb2, count }; const uint32_t MAX_LTE_SRB_ID = 2; enum class lte_drb { drb1 = 1, drb2, drb3, drb4, drb5, drb6, drb7, drb8, drb9, drb10, drb11, invalid }; const uint32_t MAX_LTE_DRB_ID = 11; -const uint32_t MAX_NOF_BEARERS = 14; +const uint32_t MAX_LTE_LCID = 10; // logicalChannelIdentity 3..10 in TS 36.331 v15.3 +const uint32_t INVALID_LCID = 99; // random invalid LCID constexpr bool is_lte_rb(uint32_t lcid) { - return lcid < MAX_NOF_BEARERS; + return lcid <= MAX_LTE_LCID; } constexpr bool is_lte_srb(uint32_t lcid) diff --git a/lib/src/upper/pdcp.cc b/lib/src/upper/pdcp.cc index a1a68ae8b..9eec6080a 100644 --- a/lib/src/upper/pdcp.cc +++ b/lib/src/upper/pdcp.cc @@ -163,7 +163,7 @@ void pdcp::del_bearer(uint32_t lcid) } if (valid_lcid(lcid)) { pdcp_array.erase(lcid); - logger.warning("Deleted PDCP bearer %s", rrc->get_rb_name(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)); } diff --git a/srsenb/hdr/stack/rrc/rrc_bearer_cfg.h b/srsenb/hdr/stack/rrc/rrc_bearer_cfg.h index 639672418..5e8d1eea3 100644 --- a/srsenb/hdr/stack/rrc/rrc_bearer_cfg.h +++ b/srsenb/hdr/stack/rrc/rrc_bearer_cfg.h @@ -69,7 +69,8 @@ public: uint32_t teid_in = 0; uint32_t addr = 0; }; - uint8_t id = 0; + uint8_t id = 0; + uint8_t lcid = 0; asn1::s1ap::erab_level_qos_params_s qos_params; asn1::bounded_bitstring<1, 160, true, true> address; uint32_t teid_out = 0; diff --git a/srsenb/hdr/stack/upper/gtpu.h b/srsenb/hdr/stack/upper/gtpu.h index e1426ba01..2206a8c33 100644 --- a/srsenb/hdr/stack/upper/gtpu.h +++ b/srsenb/hdr/stack/upper/gtpu.h @@ -54,7 +54,7 @@ public: struct tunnel { uint16_t rnti = SRSRAN_INVALID_RNTI; - uint32_t lcid = srsran::MAX_NOF_BEARERS; + uint32_t lcid = srsran::INVALID_LCID; uint32_t teid_in = 0; uint32_t teid_out = 0; uint32_t spgw_addr = 0; diff --git a/srsenb/src/stack/rrc/rrc_bearer_cfg.cc b/srsenb/src/stack/rrc/rrc_bearer_cfg.cc index e0fc3d266..0414c7fe6 100644 --- a/srsenb/src/stack/rrc/rrc_bearer_cfg.cc +++ b/srsenb/src/stack/rrc/rrc_bearer_cfg.cc @@ -224,8 +224,26 @@ int bearer_cfg_handler::add_erab(uint8_t cause.set_radio_network().value = asn1::s1ap::cause_radio_network_opts::unknown_erab_id; return SRSRAN_ERROR; } - uint8_t lcid = erab_id - 2; // Map e.g. E-RAB 5 to LCID 3 (==DRB1) - uint8_t drbid = erab_id - 4; + + uint8_t lcid = 3; // first E-RAB with DRB1 gets LCID3 + for (const auto& drb : current_drbs) { + if (drb.lc_ch_id == lcid) { + lcid++; + } + } + if (lcid > srsran::MAX_LTE_LCID) { + logger->error("Can't allocate LCID for ERAB id=%d", erab_id); + cause.set_radio_network().value = asn1::s1ap::cause_radio_network_opts::radio_res_not_available; + return SRSRAN_ERROR; + } + + // We currently have this static mapping between LCID->DRB ID + uint8_t drbid = lcid - 2; + if (drbid > srsran::MAX_LTE_DRB_ID) { + logger->error("Can't allocate DRB ID for ERAB id=%d", erab_id); + cause.set_radio_network().value = asn1::s1ap::cause_radio_network_opts::radio_res_not_available; + return SRSRAN_ERROR; + } auto qci_it = cfg->qci_cfg.find(qos.qci); if (qci_it == cfg->qci_cfg.end() or not qci_it->second.configured) { @@ -241,6 +259,7 @@ int bearer_cfg_handler::add_erab(uint8_t const rrc_cfg_qci_t& qci_cfg = qci_it->second; erabs[erab_id].id = erab_id; + erabs[erab_id].lcid = lcid; erabs[erab_id].qos_params = qos; erabs[erab_id].address = addr; erabs[erab_id].teid_out = teid_out; @@ -376,7 +395,7 @@ srsran::expected bearer_cfg_handler::add_gtpu_bearer(uint32_t erab_t::gtpu_tunnel bearer; bearer.teid_out = teid_out; bearer.addr = addr; - srsran::expected teidin = gtpu->add_bearer(rnti, erab.id - 2, addr, teid_out, props); + srsran::expected teidin = gtpu->add_bearer(rnti, erab.lcid, addr, teid_out, props); if (teidin.is_error()) { logger->error("Adding erab_id=%d to GTPU", erab_id); return srsran::default_error_t(); @@ -388,7 +407,12 @@ srsran::expected bearer_cfg_handler::add_gtpu_bearer(uint32_t void bearer_cfg_handler::rem_gtpu_bearer(uint32_t erab_id) { - gtpu->rem_bearer(rnti, erab_id - 2); + auto it = erabs.find(erab_id); + if (it == erabs.end()) { + logger->error("Removing erab_id=%d from GTPU", erab_id); + return; + } + gtpu->rem_bearer(rnti, it->second.lcid); } void bearer_cfg_handler::fill_pending_nas_info(asn1::rrc::rrc_conn_recfg_r8_ies_s* msg) diff --git a/srsenb/src/stack/rrc/rrc_mobility.cc b/srsenb/src/stack/rrc/rrc_mobility.cc index 16b9a4e77..d2207a5a9 100644 --- a/srsenb/src/stack/rrc/rrc_mobility.cc +++ b/srsenb/src/stack/rrc/rrc_mobility.cc @@ -573,7 +573,7 @@ rrc::ue::rrc_mobility::s1_source_ho_st::start_enb_status_transfer(const asn1::s1 for (const auto& erab_pair : rrc_ue->bearer_list.get_erabs()) { s1ap_interface_rrc::bearer_status_info b = {}; - uint8_t lcid = erab_pair.second.id - 2u; + uint8_t lcid = erab_pair.second.lcid; b.erab_id = erab_pair.second.id; srsran::pdcp_lte_state_t pdcp_state = {}; if (not rrc_enb->pdcp->get_bearer_state(rrc_ue->rnti, lcid, &pdcp_state)) { diff --git a/srsenb/src/stack/rrc/rrc_ue.cc b/srsenb/src/stack/rrc/rrc_ue.cc index aed5cbe57..809b6e4ff 100644 --- a/srsenb/src/stack/rrc/rrc_ue.cc +++ b/srsenb/src/stack/rrc/rrc_ue.cc @@ -11,13 +11,12 @@ */ #include "srsenb/hdr/stack/rrc/rrc_ue.h" +#include "srsenb/hdr/common/common_enb.h" #include "srsenb/hdr/stack/rrc/mac_controller.h" #include "srsenb/hdr/stack/rrc/rrc_mobility.h" #include "srsenb/hdr/stack/rrc/ue_rr_cfg.h" -#include "srsran/adt/pool/mem_pool.h" #include "srsran/asn1/rrc_utils.h" #include "srsran/common/enb_events.h" -#include "srsran/common/int_helpers.h" #include "srsran/common/standard_streams.h" #include "srsran/interfaces/enb_pdcp_interfaces.h" #include "srsran/interfaces/enb_rlc_interfaces.h" @@ -575,7 +574,7 @@ void rrc::ue::handle_rrc_con_reest_req(rrc_conn_reest_request_s* msg) // Get PDCP entity state (required when using RLC AM) for (const auto& erab_pair : old_ue->bearer_list.get_erabs()) { - uint16_t lcid = erab_pair.second.id - 2; + uint16_t lcid = erab_pair.second.lcid; old_reest_pdcp_state[lcid] = {}; parent->pdcp->get_bearer_state(old_rnti, lcid, &old_reest_pdcp_state[lcid]); @@ -1327,7 +1326,7 @@ void rrc::ue::apply_pdcp_srb_updates(const rr_cfg_ded_s& pending_rr_cfg) void rrc::ue::apply_pdcp_drb_updates(const rr_cfg_ded_s& pending_rr_cfg) { for (uint8_t drb_id : pending_rr_cfg.drb_to_release_list) { - parent->pdcp->del_bearer(rnti, drb_id + 2); + parent->pdcp->del_bearer(rnti, drb_to_lcid((lte_drb)drb_id)); } for (const drb_to_add_mod_s& drb : pending_rr_cfg.drb_to_add_mod_list) { // Configure DRB1 in PDCP @@ -1349,7 +1348,7 @@ void rrc::ue::apply_pdcp_drb_updates(const rr_cfg_ded_s& pending_rr_cfg) // If reconf due to reestablishment, recover PDCP state if (state == RRC_STATE_REESTABLISHMENT_COMPLETE) { for (const auto& erab_pair : bearer_list.get_erabs()) { - uint16_t lcid = erab_pair.second.id - 2; + uint16_t lcid = erab_pair.second.lcid; bool is_am = parent->cfg.qci_cfg[erab_pair.second.qos_params.qci].rlc_cfg.type().value == asn1::rrc::rlc_cfg_c::types_opts::am; if (is_am) { @@ -1377,7 +1376,7 @@ void rrc::ue::apply_rlc_rb_updates(const rr_cfg_ded_s& pending_rr_cfg) } if (pending_rr_cfg.drb_to_release_list.size() > 0) { for (uint8_t drb_id : pending_rr_cfg.drb_to_release_list) { - parent->rlc->del_bearer(rnti, drb_id + 2); + parent->rlc->del_bearer(rnti, drb_to_lcid((lte_drb)drb_id)); } } for (const drb_to_add_mod_s& drb : pending_rr_cfg.drb_to_add_mod_list) {