Refactor pdcp ctor seperate the configure function

lib/include/srsran/upper/pdcp_entity_base.h

@@ -56,6 +56,7 @@ public:
   pdcp_entity_base(task_sched_handle task_sched_, srslog::basic_logger& logger);
   pdcp_entity_base(pdcp_entity_base&&) = default;
   virtual ~pdcp_entity_base();
+  virtual bool configure(const pdcp_config_t& cnfg_) = 0;
   virtual void reset()       = 0;
   virtual void reestablish() = 0;
 

lib/include/srsran/upper/pdcp_entity_lte.h

@@ -108,9 +108,9 @@ public:
                   srsue::gw_interface_pdcp*  gw_,
                   srsran::task_sched_handle  task_sched_,
                   srslog::basic_logger&      logger,
-                  uint32_t                   lcid_,
-                  pdcp_config_t              cfg_);
+                  uint32_t                   lcid_);
   ~pdcp_entity_lte() override;
+  bool configure(const pdcp_config_t& cnfg_) override;
   void reset() override;
   void reestablish() override;
 

lib/include/srsran/upper/pdcp_entity_nr.h

@@ -39,9 +39,9 @@ public:
                  srsue::gw_interface_pdcp*  gw_,
                  srsran::task_sched_handle  task_sched_,
                  srslog::basic_logger&      logger,
-                 uint32_t                   lcid,
-                 pdcp_config_t              cfg_);
+                 uint32_t                   lcid);
   ~pdcp_entity_nr() final;
+  bool configure(const pdcp_config_t& cnfg_) final;
   void reset() final;
   void reestablish() final;
 

lib/src/upper/pdcp.cc

@@ -95,7 +95,12 @@ void pdcp::add_bearer(uint32_t lcid, pdcp_config_t cfg)
   if (not valid_lcid(lcid)) {
     std::unique_ptr<pdcp_entity_base> entity;
     // For now we create an pdcp entity lte for nr due to it's maturity
-    entity.reset(new pdcp_entity_lte{rlc, rrc, gw, task_sched, logger, lcid, cfg});
+    entity.reset(new pdcp_entity_lte{rlc, rrc, gw, task_sched, logger, lcid});
+    if (not entity->configure(cfg)) {
+      logger.error("Can not configure PDCP entity");
+      return;
+    }
+    
     if (not pdcp_array.insert(std::make_pair(lcid, std::move(entity))).second) {
       logger.error("Error inserting PDCP entity in to array.");
       return;
@@ -117,10 +122,17 @@ void pdcp::add_bearer(uint32_t lcid, pdcp_config_t cfg)
 void pdcp::add_bearer_mrb(uint32_t lcid, pdcp_config_t cfg)
 {
   if (not valid_mch_lcid(lcid)) {
+    std::unique_ptr<pdcp_entity_lte> entity;
+    entity.reset(new pdcp_entity_lte{rlc, rrc, gw, task_sched, logger, lcid});    
+    if(not entity->configure(cfg)){
+      logger.error("Can not configure PDCP entity");
+      return; 
+    }
+
     if (not pdcp_array_mrb
                 .insert(std::make_pair(
                     lcid,
-                    std::unique_ptr<pdcp_entity_lte>(new pdcp_entity_lte(rlc, rrc, gw, task_sched, logger, lcid, cfg))))
+                    std::move(entity)))
                 .second) {
       logger.error("Error inserting PDCP entity in to array.");
       return;

lib/src/upper/pdcp_entity_lte.cc

@@ -29,29 +29,36 @@ pdcp_entity_lte::pdcp_entity_lte(srsue::rlc_interface_pdcp* rlc_,
                                  srsue::gw_interface_pdcp*  gw_,
                                  srsran::task_sched_handle  task_sched_,
                                  srslog::basic_logger&      logger,
-                                 uint32_t                   lcid_,
-                                 pdcp_config_t              cfg_) :
+                                 uint32_t                   lcid_) :
   pdcp_entity_base(task_sched_, logger), rlc(rlc_), rrc(rrc_), gw(gw_)
 {
-  lcid                 = lcid_;
-  cfg                  = cfg_;
-  active               = true;
+  // Initial state
   integrity_direction  = DIRECTION_NONE;
   encryption_direction = DIRECTION_NONE;
 
-  if (is_srb()) {
-    reordering_window = 0;
-  } else if (is_drb()) {
-    reordering_window = 2048;
-  }
-
-  // Initial state
   st.next_pdcp_tx_sn = 0;
   st.tx_hfn          = 0;
   st.rx_hfn          = 0;
   st.next_pdcp_rx_sn = 0;
+
+  lcid = lcid_;
+}
+
+pdcp_entity_lte::~pdcp_entity_lte()
+{
+  reset();
+}
+
+bool pdcp_entity_lte::configure(const pdcp_config_t& cnfg_)
+{
+  cfg                          = cnfg_;
   maximum_pdcp_sn              = (1u << cfg.sn_len) - 1u;
   st.last_submitted_pdcp_rx_sn = maximum_pdcp_sn;
+  if (is_srb()) {
+    reordering_window = 0;
+  } else if (is_drb()) {
+    reordering_window = 2048;
+  }
 
   if (is_drb() && not rlc->rb_is_um(lcid) && cfg.discard_timer == pdcp_discard_timer_t::infinity) {
     logger.warning(
@@ -80,14 +87,11 @@ pdcp_entity_lte::pdcp_entity_lte(srsue::rlc_interface_pdcp* rlc_,
   // Check supported config
   if (!check_valid_config()) {
     srsran::console("Warning: Invalid PDCP config.\n");
+    return false;
   }
+  active = true;
+  return true;
 }
-
-pdcp_entity_lte::~pdcp_entity_lte()
-{
-  reset();
-}
-
 // Reestablishment procedure: 36.323 5.2
 void pdcp_entity_lte::reestablish()
 {

lib/src/upper/pdcp_entity_nr.cc

@@ -20,8 +20,7 @@ pdcp_entity_nr::pdcp_entity_nr(srsue::rlc_interface_pdcp* rlc_,
                                srsue::gw_interface_pdcp*  gw_,
                                srsran::task_sched_handle  task_sched_,
                                srslog::basic_logger&      logger,
-                               uint32_t                   lcid_,
-                               pdcp_config_t              cfg_) :
+                               uint32_t                   lcid_) :
   pdcp_entity_base(task_sched_, logger),
   rlc(rlc_),
   rrc(rrc_),
@@ -29,11 +28,22 @@ pdcp_entity_nr::pdcp_entity_nr(srsue::rlc_interface_pdcp* rlc_,
   reordering_fnc(new pdcp_entity_nr::reordering_callback(this))
 {
   lcid                 = lcid_;
-  cfg                  = cfg_;
-  active               = true;
   integrity_direction  = DIRECTION_NONE;
   encryption_direction = DIRECTION_NONE;
+}
+
+pdcp_entity_nr::~pdcp_entity_nr() {}
+
+// Reestablishment procedure: 38.323 5.2
+void pdcp_entity_nr::reestablish()
+{
+  logger.info("Re-establish %s with bearer ID: %d", rrc->get_rb_name(lcid).c_str(), cfg.bearer_id);
+  // TODO
+}
 
+bool pdcp_entity_nr::configure(const pdcp_config_t& cnfg_)
+{
+  cfg         = cnfg_;
   window_size = 1 << (cfg.sn_len - 1);
 
   // Timers
@@ -43,15 +53,8 @@ pdcp_entity_nr::pdcp_entity_nr(srsue::rlc_interface_pdcp* rlc_,
   if (static_cast<uint32_t>(cfg.t_reordering) > 0) {
     reordering_timer.set(static_cast<uint32_t>(cfg.t_reordering), *reordering_fnc);
   }
-}
-
-pdcp_entity_nr::~pdcp_entity_nr() {}
-
-// Reestablishment procedure: 38.323 5.2
-void pdcp_entity_nr::reestablish()
-{
-  logger.info("Re-establish %s with bearer ID: %d", rrc->get_rb_name(lcid).c_str(), cfg.bearer_id);
-  // TODO
+  active               = true;
+  return true;
 }
 
 // Used to stop/pause the entity (called on RRC conn release)

lib/test/upper/pdcp_lte_test.h

@@ -60,8 +60,9 @@ class pdcp_lte_test_helper
 {
 public:
   pdcp_lte_test_helper(srsran::pdcp_config_t cfg, srsran::as_security_config_t sec_cfg_, srslog::basic_logger& logger) :
-    rlc(logger), rrc(logger), gw(logger), pdcp(&rlc, &rrc, &gw, &stack.task_sched, logger, 0, cfg)
+    rlc(logger), rrc(logger), gw(logger), pdcp(&rlc, &rrc, &gw, &stack.task_sched, logger, 0)
   {
+    pdcp.configure(cfg);
     pdcp.config_security(sec_cfg_);
     pdcp.enable_integrity(srsran::DIRECTION_TXRX);
     pdcp.enable_encryption(srsran::DIRECTION_TXRX);

lib/test/upper/pdcp_nr_test.h

@@ -88,8 +88,9 @@ class pdcp_nr_test_helper
 {
 public:
   pdcp_nr_test_helper(srsran::pdcp_config_t cfg, srsran::as_security_config_t sec_cfg_, srslog::basic_logger& logger) :
-    rlc(logger), rrc(logger), gw(logger), pdcp(&rlc, &rrc, &gw, &stack.task_sched, logger, 0, cfg)
+    rlc(logger), rrc(logger), gw(logger), pdcp(&rlc, &rrc, &gw, &stack.task_sched, logger, 0)
   {
+    pdcp.configure(cfg);
     pdcp.config_security(sec_cfg_);
     pdcp.enable_integrity(srsran::DIRECTION_TXRX);
     pdcp.enable_encryption(srsran::DIRECTION_TXRX);