Minor fixes and clang formatting in SDAP procedures

- added descriptive comments to the code
- removed redundant function prototype
- removed unused function argument

openair2/LAYER2/nr_pdcp/nr_pdcp_oai_api.c

@@ -921,8 +921,16 @@ void add_drb(int is_gnb,
     nr_pdcp_ue_add_drb_pdcp_entity(ue, drb_id, pdcp_drb);
 
     LOG_I(PDCP, "added drb %d to UE ID %ld\n", drb_id, UEid);
-
-    new_nr_sdap_entity(is_gnb, has_sdap_rx, has_sdap_tx, UEid, pdusession_id, is_sdap_DefaultDRB, drb_id, mappedQFIs2Add, mappedQFIs2AddCount);
+    /* add new SDAP entity for the PDU session the DRB belongs to */
+    new_nr_sdap_entity(is_gnb,
+                       has_sdap_rx,
+                       has_sdap_tx,
+                       UEid,
+                       pdusession_id,
+                       is_sdap_DefaultDRB,
+                       drb_id,
+                       mappedQFIs2Add,
+                       mappedQFIs2AddCount);
   }
   nr_pdcp_manager_unlock(nr_pdcp_ue_manager);
 }

openair2/RRC/NR_UE/rrc_UE.c

@@ -1159,6 +1159,10 @@ void nr_rrc_ue_process_measConfig(rrcPerNB_t *rrc, NR_MeasConfig_t *const measCo
   }
 }
 
+/**
+ * @brief add, modify and release SRBs and/or DRBs
+ * @ref   3GPP TS 38.331
+ */
 static void nr_rrc_ue_process_RadioBearerConfig(NR_UE_RRC_INST_t *ue_rrc,
                                                 rrcPerNB_t *rrcNB,
                                                 NR_RadioBearerConfig_t *const radioBearerConfig)
@@ -1218,15 +1222,21 @@ static void nr_rrc_ue_process_RadioBearerConfig(NR_UE_RRC_INST_t *ue_rrc,
     }
   }
 
-  // Establish DRBs if present
+  /**
+   * Establish/reconfig DRBs if DRB-ToAddMod is present
+   * according to 3GPP TS 38.331 clause 5.3.5.6.5 DRB addition/modification
+   */
   if (radioBearerConfig->drb_ToAddModList != NULL) {
     for (int cnt = 0; cnt < radioBearerConfig->drb_ToAddModList->list.count; cnt++) {
       struct NR_DRB_ToAddMod *drb = radioBearerConfig->drb_ToAddModList->list.array[cnt];
       int DRB_id = drb->drb_Identity;
+      /* DRB is already established and configured */
       if (rrcNB->status_DRBs[DRB_id] == RB_ESTABLISHED) {
         AssertFatal(drb->reestablishPDCP == NULL, "reestablishPDCP not yet implemented\n");
         AssertFatal(drb->recoverPDCP == NULL, "recoverPDCP not yet implemented\n");
+        /* sdap-Config is included (SA mode) */
         NR_SDAP_Config_t *sdap_Config = drb->cnAssociation ? drb->cnAssociation->choice.sdap_Config : NULL;
+        /* PDCP and SDAP reconfiguration */
         if (drb->pdcp_Config || sdap_Config)
           nr_pdcp_reconfigure_drb(ue_rrc->ue_id, DRB_id, drb->pdcp_Config, sdap_Config);
         if (drb->cnAssociation)

openair2/SDAP/nr_sdap/nr_sdap_entity.c

@@ -110,7 +110,7 @@ static bool nr_sdap_tx_entity(nr_sdap_entity_t *entity,
   }
 
   uint8_t sdap_buf[SDAP_MAX_PDU];
-  rb_id_t pdcp_entity = entity->qfi2drb_map(entity, qfi, rb_id);
+  rb_id_t pdcp_entity = entity->qfi2drb_map(entity, qfi);
 
   if(pdcp_entity){
     sdap_drb_id = pdcp_entity;
@@ -212,7 +212,8 @@ static void nr_sdap_rx_entity(nr_sdap_entity_t *entity,
                               int pdusession_id,
                               ue_id_t ue_id,
                               char *buf,
-                              int size) {
+                              int size)
+{
   /* The offset of the SDAP header, it might be 0 if has_sdap_rx is not true in the pdcp entity. */
   int offset=0;
 
@@ -338,6 +339,9 @@ static void nr_sdap_rx_entity(nr_sdap_entity_t *entity,
   }
 }
 
+/**
+ * @brief update QFI to DRB mapping rules
+*/
 void nr_sdap_qfi2drb_map_update(nr_sdap_entity_t *entity, uint8_t qfi, rb_id_t drb, bool has_sdap_rx, bool has_sdap_tx)
 {
   if(qfi < SDAP_MAX_QFI &&
@@ -358,7 +362,11 @@ void nr_sdap_qfi2drb_map_del(nr_sdap_entity_t *entity, uint8_t qfi){
   LOG_D(SDAP, "Deleted mapping for QFI: %u \n", qfi);
 }
 
-rb_id_t nr_sdap_qfi2drb_map(nr_sdap_entity_t *entity, uint8_t qfi, rb_id_t upper_layer_rb_id){
+/**
+ * @brief   maps the QFIs to the default DRB if not mapping rule exists
+ * @return  DRB that is mapped to the QFI, 0 if no mapping and no default DRB exists for that QFI
+*/
+rb_id_t nr_sdap_qfi2drb_map(nr_sdap_entity_t *entity, uint8_t qfi){
   rb_id_t pdcp_entity;
 
   pdcp_entity = entity->qfi2drb_table[qfi].drb_id;
@@ -396,12 +404,15 @@ rb_id_t nr_sdap_map_ctrl_pdu(nr_sdap_entity_t *entity, rb_id_t pdcp_entity, int
     LOG_D(SDAP, "Mapping Control PDU QFI: %u to Default DRB: %ld\n", dl_qfi, drb_of_endmarker);
   }
   if(map_type == SDAP_CTRL_PDU_MAP_RULE_DRB){
-    drb_of_endmarker = entity->qfi2drb_map(entity, dl_qfi, pdcp_entity);
+    drb_of_endmarker = entity->qfi2drb_map(entity, dl_qfi);
     LOG_D(SDAP, "Mapping Control PDU QFI: %u to DRB: %ld\n", dl_qfi, drb_of_endmarker);
   }
   return drb_of_endmarker;
 }
 
+/**
+ * @brief Submit the end-marker control PDU to PDCP according to TS 37.324, clause 5.3
+ */
 void nr_sdap_submit_ctrl_pdu(ue_id_t ue_id, rb_id_t sdap_ctrl_pdu_drb, nr_sdap_ul_hdr_t ctrl_pdu)
 {
   if(sdap_ctrl_pdu_drb){
@@ -410,28 +421,55 @@ void nr_sdap_submit_ctrl_pdu(ue_id_t ue_id, rb_id_t sdap_ctrl_pdu_drb, nr_sdap_u
   }
 }
 
-void nr_sdap_ue_qfi2drb_config(nr_sdap_entity_t *existing_sdap_entity, rb_id_t pdcp_entity, ue_id_t ue_id, NR_QFI_t *mapped_qfi_2_add, uint8_t mappedQFIs2AddCount, uint8_t drb_identity, bool has_sdap_rx, bool has_sdap_tx)
+/**
+ * @brief UL QoS flow to DRB mapping configuration for an existing SDAP entity
+ *        according to TS 37.324, 5.3 QoS flow to DRB Mapping, clause 5.3.1 Configuration Procedures
+ */
+static void nr_sdap_ue_qfi2drb_config(nr_sdap_entity_t *existing_sdap_entity,
+                                      rb_id_t pdcp_entity,
+                                      ue_id_t ue_id,
+                                      NR_QFI_t *mapped_qfi_2_add,
+                                      uint8_t mappedQFIs2AddCount,
+                                      uint8_t drb_identity,
+                                      bool has_sdap_rx,
+                                      bool has_sdap_tx)
 {
   LOG_D(SDAP, "RRC Configuring SDAP Entity\n");
-  uint8_t qfi = 0;
-
   for(int i = 0; i < mappedQFIs2AddCount; i++){
-    qfi = mapped_qfi_2_add[i];
+    uint8_t qfi = mapped_qfi_2_add[i];
+    /* a default DRB exists and there is no QFI to DRB mapping rule for the QFI */
     if (existing_sdap_entity->default_drb && existing_sdap_entity->qfi2drb_table[qfi].drb_id == SDAP_NO_MAPPING_RULE) {
       nr_sdap_ul_hdr_t sdap_ctrl_pdu = existing_sdap_entity->sdap_construct_ctrl_pdu(qfi);
-      rb_id_t sdap_ctrl_pdu_drb = existing_sdap_entity->sdap_map_ctrl_pdu(existing_sdap_entity, pdcp_entity, SDAP_CTRL_PDU_MAP_DEF_DRB, qfi);
+      rb_id_t sdap_ctrl_pdu_drb =
+          existing_sdap_entity->sdap_map_ctrl_pdu(existing_sdap_entity, pdcp_entity, SDAP_CTRL_PDU_MAP_DEF_DRB, qfi);
       existing_sdap_entity->sdap_submit_ctrl_pdu(ue_id, sdap_ctrl_pdu_drb, sdap_ctrl_pdu);
     }
+    /* the stored UL QFI to DRB mapping rule is different from the configured one and has UL SDAP header*/
     if (existing_sdap_entity->qfi2drb_table[qfi].drb_id != drb_identity && existing_sdap_entity->qfi2drb_table[qfi].has_sdap_tx) {
       nr_sdap_ul_hdr_t sdap_ctrl_pdu = existing_sdap_entity->sdap_construct_ctrl_pdu(qfi);
-      rb_id_t sdap_ctrl_pdu_drb = existing_sdap_entity->sdap_map_ctrl_pdu(existing_sdap_entity, pdcp_entity, SDAP_CTRL_PDU_MAP_RULE_DRB, qfi);
+      rb_id_t sdap_ctrl_pdu_drb =
+          existing_sdap_entity->sdap_map_ctrl_pdu(existing_sdap_entity, pdcp_entity, SDAP_CTRL_PDU_MAP_RULE_DRB, qfi);
       existing_sdap_entity->sdap_submit_ctrl_pdu(ue_id, sdap_ctrl_pdu_drb, sdap_ctrl_pdu);
     }
+    /* update QFI to DRB mapping rules */
     LOG_D(SDAP, "Storing the configured QoS flow to DRB mapping rule\n");
     existing_sdap_entity->qfi2drb_map_update(existing_sdap_entity, qfi, drb_identity, has_sdap_rx, has_sdap_tx);
   }
 }
 
+/**
+ * @brief   add a new SDAP entity according to 5.1.1. of 3GPP TS 37.324
+ * @note    there is one SDAP entity per PDU session
+ *
+ * @param   is_gnb, indicates whether it is for gNB or UE
+ * @param   has_sdap_rx, indicates whether it is a receiving SDAP entity
+ * @param   has_sdap_tx, indicates whether it is a transmitting SDAP entity
+ * @param   ue_id, UE ID
+ * @param   pdusession_id, PDU session ID
+ * @param   is_defaultDRB, indicates whether the entity has a default DRB
+ * @param   mapped_qfi_2_add, list of QoS flows to add/update
+ * @param   mappedQFIs2AddCount, number of QoS flows to add/update
+ */
 nr_sdap_entity_t *new_nr_sdap_entity(int is_gnb,
                                      bool has_sdap_rx,
                                      bool has_sdap_tx,
@@ -442,12 +480,21 @@ nr_sdap_entity_t *new_nr_sdap_entity(int is_gnb,
                                      NR_QFI_t *mapped_qfi_2_add,
                                      uint8_t mappedQFIs2AddCount)
 {
+  /* check whether the SDAP entity already exists and
+     update QFI to DRB mapping rules in that case */
   if (nr_sdap_get_entity(ue_id, pdusession_id)) {
     LOG_E(SDAP, "SDAP Entity for UE already exists with RNTI/UE ID: %lu and PDU SESSION ID: %d\n", ue_id, pdusession_id);
     nr_sdap_entity_t *existing_sdap_entity = nr_sdap_get_entity(ue_id, pdusession_id);
     rb_id_t pdcp_entity = existing_sdap_entity->default_drb;
     if(!is_gnb)
-      nr_sdap_ue_qfi2drb_config(existing_sdap_entity, pdcp_entity, ue_id, mapped_qfi_2_add, mappedQFIs2AddCount, drb_identity, has_sdap_rx, has_sdap_tx);
+      nr_sdap_ue_qfi2drb_config(existing_sdap_entity,
+                                pdcp_entity,
+                                ue_id,
+                                mapped_qfi_2_add,
+                                mappedQFIs2AddCount,
+                                drb_identity,
+                                has_sdap_rx,
+                                has_sdap_tx);
     return existing_sdap_entity;
   }
 
@@ -486,6 +533,11 @@ nr_sdap_entity_t *new_nr_sdap_entity(int is_gnb,
   return sdap_entity;
 }
 
+/**
+ * @brief   Fetches the SDAP entity for the give PDU session ID.
+ * @note    There is one SDAP entity per PDU session.
+ * @return  The pointer to the SDAP entity if existing, NULL otherwise
+ */
 nr_sdap_entity_t *nr_sdap_get_entity(ue_id_t ue_id, int pdusession_id)
 {
   nr_sdap_entity_t *sdap_entity;
@@ -515,8 +567,7 @@ void nr_sdap_release_drb(ue_id_t ue_id, int drb_id, int pdusession_id)
     }
   }
   else
-    LOG_E(SDAP, "Couldn't find a SDAP entity associated with PDU session ID %d\n",
-          pdusession_id);
+    LOG_E(SDAP, "Couldn't find a SDAP entity associated with PDU session ID %d\n", pdusession_id);
 }
 
 bool nr_sdap_delete_entity(ue_id_t ue_id, int pdusession_id)

openair2/SDAP/nr_sdap/nr_sdap_entity.h

@@ -84,7 +84,7 @@ typedef struct nr_sdap_entity_s {
 
   void (*qfi2drb_map_update)(struct nr_sdap_entity_s *entity, uint8_t qfi, rb_id_t drb, bool has_sdap_rx, bool has_sdap_tx);
   void (*qfi2drb_map_delete)(struct nr_sdap_entity_s *entity, uint8_t qfi);
-  rb_id_t (*qfi2drb_map)(struct nr_sdap_entity_s *entity, uint8_t qfi, rb_id_t upper_layer_rb_id);
+  rb_id_t (*qfi2drb_map)(struct nr_sdap_entity_s *entity, uint8_t qfi);
 
   nr_sdap_ul_hdr_t (*sdap_construct_ctrl_pdu)(uint8_t qfi);
   rb_id_t (*sdap_map_ctrl_pdu)(struct nr_sdap_entity_s *entity, rb_id_t pdcp_entity, int map_type, uint8_t dl_qfi);
@@ -132,7 +132,7 @@ void nr_sdap_qfi2drb_map_del(nr_sdap_entity_t *entity, uint8_t qfi);
  * If there is no stored QoS flow to DRB mapping rule for the QoS flow as specified in the subclause 5.3, map the SDAP SDU to the default DRB
  * else, map the SDAP SDU to the DRB according to the stored QoS flow to DRB mapping rule.
  */
-rb_id_t nr_sdap_qfi2drb_map(nr_sdap_entity_t *entity, uint8_t qfi, rb_id_t upper_layer_rb_id);
+rb_id_t nr_sdap_qfi2drb_map(nr_sdap_entity_t *entity, uint8_t qfi);
 
 /*
  * TS 37.324 5.3 QoS flow to DRB Mapping 
@@ -154,17 +154,19 @@ rb_id_t nr_sdap_map_ctrl_pdu(nr_sdap_entity_t *entity, rb_id_t pdcp_entity, int
  */
 void nr_sdap_submit_ctrl_pdu(ue_id_t ue_id, rb_id_t sdap_ctrl_pdu_drb, nr_sdap_ul_hdr_t ctrl_pdu);
 
-/*
- * TS 37.324 5.3 QoS flow to DRB Mapping 
- * 5.3.1 Configuration Procedures
- */
-void nr_sdap_ue_qfi2drb_config(nr_sdap_entity_t *existing_sdap_entity, rb_id_t pdcp_entity, ue_id_t ue_id, NR_QFI_t *mapped_qfi_2_add, uint8_t mappedQFIs2AddCount, uint8_t drb_identity, bool has_sdap_rx, bool has_sdap_tx);
-
 /*
  * TS 37.324 4.4 5.1.1 SDAP entity establishment
  * Establish an SDAP entity.
  */
-nr_sdap_entity_t *new_nr_sdap_entity(int is_gnb, bool has_sdap_rx, bool has_sdap_tx, ue_id_t ue_id, int pdusession_id, bool is_defaultDRB, uint8_t default_DRB, NR_QFI_t *mapped_qfi_2_add, uint8_t mappedQFIs2AddCount);
+nr_sdap_entity_t *new_nr_sdap_entity(int is_gnb,
+                                     bool has_sdap_rx,
+                                     bool has_sdap_tx,
+                                     ue_id_t ue_id,
+                                     int pdusession_id,
+                                     bool is_defaultDRB,
+                                     uint8_t default_DRB,
+                                     NR_QFI_t *mapped_qfi_2_add,
+                                     uint8_t mappedQFIs2AddCount);
 
 /* Entity Handling Related Functions */
 nr_sdap_entity_t *nr_sdap_get_entity(ue_id_t ue_id, int pdusession_id);