rrc-nr: prevent master_cell_group to be sent if...

...there are no changes in the radio_bearer_configuration

In addition:
i)  Force RRCReconfiguration to be sent after receiving
    SecurityModeComplete
ii) Prevent RRCReconfiguration to be sent after establish_rrc_bearer()

Signed-off-by: Carlo Galiotto <carlo@srs.io>

srsgnb/src/stack/rrc/rrc_nr.cc

@@ -610,8 +610,6 @@ int rrc_nr::establish_rrc_bearer(uint16_t rnti, uint16_t pdu_session_id, srsran:
 
   users[rnti]->establish_eps_bearer(pdu_session_id, nas_pdu, lcid);
 
-  // TODO: verify whether this is the best place where to call the RRCReconfig
-  users[rnti]->send_rrc_reconfiguration();
   return SRSRAN_SUCCESS;
 }
 

srsgnb/src/stack/rrc/rrc_nr_ue.cc

@@ -1021,6 +1021,7 @@ void rrc_nr::ue::handle_security_mode_complete(const asn1::rrc_nr::security_mode
   parent->logger.info("SecurityModeComplete transaction ID: %d", msg.rrc_transaction_id);
   parent->pdcp->enable_encryption(rnti, srb_to_lcid(srsran::nr_srb::srb1));
 
+  send_rrc_reconfiguration();
   // Note: Skip UE capabilities
 
   // Send RRCReconfiguration if necessary
@@ -1040,50 +1041,57 @@ void rrc_nr::ue::send_rrc_reconfiguration()
   ies.radio_bearer_cfg_present =
       compute_diff_radio_bearer_cfg(parent->cfg, radio_bearer_cfg, next_radio_bearer_cfg, ies.radio_bearer_cfg);
 
-  ies.non_crit_ext_present                   = true;
+  // If no bearer to add/mod/remove, do not include master_cell_group
-  ies.non_crit_ext.master_cell_group_present = true;
+  // Set ies.non_crit_ext_present only if master_cell_group_present = true or
+  if (ies.radio_bearer_cfg_present){
 
-  // Fill masterCellGroup
+    ies.non_crit_ext.master_cell_group_present = true;
-  cell_group_cfg_s master_cell_group;
-  master_cell_group.cell_group_id = 0;
-  fill_cellgroup_with_radio_bearer_cfg(parent->cfg, ies.radio_bearer_cfg, master_cell_group);
 
-  // Pack masterCellGroup into container
+    // Fill masterCellGroup
-  srsran::unique_byte_buffer_t pdu = parent->pack_into_pdu(master_cell_group, __FUNCTION__);
+    cell_group_cfg_s master_cell_group;
-  if (pdu == nullptr) {
+    master_cell_group.cell_group_id = 0;
-    send_rrc_release();
+    fill_cellgroup_with_radio_bearer_cfg(parent->cfg, ies.radio_bearer_cfg, master_cell_group);
-    return;
-  }
-  ies.non_crit_ext.master_cell_group.resize(pdu->N_bytes);
-  memcpy(ies.non_crit_ext.master_cell_group.data(), pdu->data(), pdu->N_bytes);
-  if (logger.debug.enabled()) {
-    asn1::json_writer js;
-    master_cell_group.to_json(js);
-    logger.debug("Containerized MasterCellGroup: %s", js.to_string().c_str());
-  }
 
-  // Pass stored NAS PDUs
+    // Pack masterCellGroup into container
-  ies.non_crit_ext.ded_nas_msg_list_present = true;
+    srsran::unique_byte_buffer_t pdu = parent->pack_into_pdu(master_cell_group, __FUNCTION__);
-  ies.non_crit_ext.ded_nas_msg_list.resize(nas_pdu_queue.size());
+    if (pdu == nullptr) {
-  for (uint32_t i = 0; i < nas_pdu_queue.size(); ++i) {
+      send_rrc_release();
-    ies.non_crit_ext.ded_nas_msg_list[i].resize(nas_pdu_queue[i]->size());
+      return;
-    memcpy(ies.non_crit_ext.ded_nas_msg_list[i].data(), nas_pdu_queue[i]->data(), nas_pdu_queue[i]->size());
+    }
-  }
+    ies.non_crit_ext.master_cell_group.resize(pdu->N_bytes);
-  ies.non_crit_ext.ded_nas_msg_list_present = nas_pdu_queue.size() > 0;
+    memcpy(ies.non_crit_ext.master_cell_group.data(), pdu->data(), pdu->N_bytes);
-  nas_pdu_queue.clear();
+    if (logger.debug.enabled()) {
+      asn1::json_writer js;
+      master_cell_group.to_json(js);
+      logger.debug("Containerized MasterCellGroup: %s", js.to_string().c_str());
+    }
+
+    // Update lower layers
+    if (ies.radio_bearer_cfg_present) {
+      // add PDCP bearers
+      update_pdcp_bearers(ies.radio_bearer_cfg, master_cell_group);
 
-  // Update lower layers
+      // add RLC bearers
-  if (ies.radio_bearer_cfg_present) {
+      update_rlc_bearers(master_cell_group);
-    // add PDCP bearers
-    update_pdcp_bearers(ies.radio_bearer_cfg, master_cell_group);
 
-    // add RLC bearers
+      // add MAC bearers
-    update_rlc_bearers(master_cell_group);
+      update_mac(master_cell_group, false);
+    }
+  }
 
-    // add MAC bearers
+  if (nas_pdu_queue.size() > 0) {
-    update_mac(master_cell_group, false);
+    // Pass stored NAS PDUs
+    ies.non_crit_ext.ded_nas_msg_list_present = true;
+    ies.non_crit_ext.ded_nas_msg_list.resize(nas_pdu_queue.size());
+    for (uint32_t i = 0; i < nas_pdu_queue.size(); ++i) {
+      ies.non_crit_ext.ded_nas_msg_list[i].resize(nas_pdu_queue[i]->size());
+      memcpy(ies.non_crit_ext.ded_nas_msg_list[i].data(), nas_pdu_queue[i]->data(), nas_pdu_queue[i]->size());
+    }
+    nas_pdu_queue.clear();
   }
 
+  ies.non_crit_ext_present = ies.non_crit_ext.master_cell_group_present or ies.non_crit_ext.ded_nas_msg_list_present;
+
   if (send_dl_dcch(srsran::nr_srb::srb1, dl_dcch_msg) != SRSRAN_SUCCESS) {
     send_rrc_release();
   }

srsgnb/src/stack/rrc/test/rrc_nr_test_helpers.cc

@@ -258,7 +258,57 @@ void test_rrc_nr_reconfiguration(srsran::task_scheduler& task_sched,
   NAS_msg.from_string("c574defc80ba722bffb8eacb6f8a163e3222cf1542ac529f6980bb15e0bf12d9f2b29f11fb458ec9");
 
   // STEP 2 -  Trigger and send RRCReconfiguration command (gNB -> UE)
-  rrc_obj.establish_rrc_bearer(rnti, 1, NAS_msg, srsran::srb_to_lcid(srsran::nr_srb::srb1));
+  //rrc_obj.start_rrc_reconfiguration(rnti);
+
+  // Test whether there exists the SRB1 initiated in the Connection Establishment
+  // We test this as the SRB1 was set up in a different function
+  TESTASSERT_EQ(rnti, pdcp.last_sdu_rnti);
+  TESTASSERT_EQ(srsran::srb_to_lcid(srsran::nr_srb::srb1), pdcp.last_sdu_lcid);
+
+  dl_dcch_msg_s dl_dcch_msg;
+  {
+    asn1::cbit_ref bref{pdcp.last_sdu->data(), pdcp.last_sdu->size()};
+    TESTASSERT_SUCCESS(dl_dcch_msg.unpack(bref));
+  }
+
+  // Test whether the unpacked message is correct
+  TESTASSERT_EQ(dl_dcch_msg_type_c::types_opts::c1, dl_dcch_msg.msg.type().value);
+  TESTASSERT_EQ(dl_dcch_msg_type_c::c1_c_::types_opts::rrc_recfg, dl_dcch_msg.msg.c1().type().value);
+  TESTASSERT_EQ(rrc_recfg_s::crit_exts_c_::types_opts::rrc_recfg,
+                dl_dcch_msg.msg.c1().rrc_recfg().crit_exts.type().value);
+  const rrc_recfg_ies_s& reconf_ies = dl_dcch_msg.msg.c1().rrc_recfg().crit_exts.rrc_recfg();
+
+  // STEP 2 - Send RRCReconfiguration (UE -> gNB)
+  ul_dcch_msg_s ul_dcch_msg;
+  auto&         RRC_recfg_complete  = ul_dcch_msg.msg.set_c1().set_rrc_recfg_complete();
+  RRC_recfg_complete.rrc_transaction_id = dl_dcch_msg.msg.c1().rrc_recfg().rrc_transaction_id;
+  RRC_recfg_complete.crit_exts.set_rrc_recfg_complete();
+
+  srsran::unique_byte_buffer_t pdu;
+  {
+    pdu = srsran::make_byte_buffer();
+    asn1::bit_ref bref{pdu->data(), pdu->get_tailroom()};
+    TESTASSERT_SUCCESS(ul_dcch_msg.pack(bref));
+    pdu->N_bytes = bref.distance_bytes();
+  }
+
+  // send message to RRC
+  rrc_obj.write_pdu(rnti, 1, std::move(pdu));
+}
+
+#if 0
+void test_rrc_nr_reconfiguration(srsran::task_scheduler& task_sched,
+                                 rrc_nr&                 rrc_obj,
+                                 pdcp_nr_rrc_tester&     pdcp,
+                                 uint16_t                rnti)
+{
+  TESTASSERT_EQ(srsran::srb_to_lcid(srsran::nr_srb::srb1), pdcp.last_sdu_lcid);
+
+  // create an unbounded_octstring object that contains a random NAS message (we simulate a NAS message)
+  asn1::unbounded_octstring<false> NAS_msg;
+  NAS_msg.from_string("c574defc80ba722bffb8eacb6f8a163e3222cf1542ac529f6980bb15e0bf12d9f2b29f11fb458ec9");
+
+  // STEP 2 -  Trigger and send RRCReconfiguration command (gNB -> UE)
   //rrc_obj.start_rrc_reconfiguration(rnti);
 
   // Test whether there exists the SRB1 initiated in the Connection Establishment
@@ -326,5 +376,6 @@ void test_rrc_nr_reconfiguration(srsran::task_scheduler& task_sched,
   // send message to RRC
   rrc_obj.write_pdu(rnti, 1, std::move(pdu));
 }
+#endif
 
 } // namespace srsenb