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 #658 Co-authored-by: herlesupreeth <[email protected]> Co-authored-by: Francisco <[email protected]>
srsran · May 12, 2021 · d81788e · d81788e
1 parent ace8ff2
commit d81788e
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 14 deletions.
diff --git a/lib/include/srsran/common/common_lte.h b/lib/include/srsran/common/common_lte.h
@@ -59,11 +59,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
@@ -172,7 +172,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
@@ -78,7 +78,8 @@ class bearer_cfg_handler
       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
@@ -64,7 +64,7 @@ class gtpu_tunnel_manager
 
   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
@@ -233,8 +233,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) {
@@ -250,6 +268,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;
@@ -385,7 +404,7 @@ srsran::expected<uint32_t> bearer_cfg_handler::add_gtpu_bearer(uint32_t
   erab_t::gtpu_tunnel bearer;
   bearer.teid_out                   = teid_out;
   bearer.addr                       = addr;
-  srsran::expected<uint32_t> teidin = gtpu->add_bearer(rnti, erab.id - 2, addr, teid_out, props);
+  srsran::expected<uint32_t> 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();
@@ -397,7 +416,12 @@ srsran::expected<uint32_t> 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
@@ -582,7 +582,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
@@ -20,10 +20,10 @@
  */
 
 #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"
@@ -1339,7 +1339,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
@@ -1361,7 +1361,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) {
@@ -1389,7 +1389,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) {