fix (sdap): select RX/TX SDAP headers on per-DRB RRC role

A DRB may have an uplink SDAP header but no downlink header (valid TS 38.331).
The old entity->enable_sdap flag was true if either direction had a header, so UE
downlink wrongly read byte 0 as QFI. IPv4 payloads start with 0x45 -> bogus QFI 5
and dropped traffic before the TUN (PR !158).

Header presence must follow TS 37.324 §5.2.1/§5.2.2 per DRB and direction, not a
single PDU-session-wide enable-sdap.

Also, store the default DRB as a `qfi2drb_t` mapping in the SDAP entity and use
it directly for fallback paths.

Changes:
- Add nr_sdap_drb_lookup() to return first qfi2drb_table row for a DRB
- nr_sdap_rx_entity(): parse QFI only when the receiving direction has a header,
  drop if the DRB has no table row
- nr_sdap_tx_entity(): UL/DL TX header bits from qfi2drb_map() (drb_id + entity_role),
  including default-DRB fallback
- nr_sdap_qfi2drb(): const qfi2drb_t *, remove SDAP_MAP_RULE_EMPTY
- nr_sdap_map_ctrl_pdu(): handle NULL from qfi2drb_map
- nr_sdap_qfi2drb_map_update(): sync entity_role on
  all QFIs for the DRB. At most one DRB when both headers absent.
- nr_sdap_entity_update_qos_flows(): keep per-DRB role from nr_sdap_drb_lookup(),
  skip update when DRB has no row
- Cleanup entity->enable_sdap
- nr_sdap.c: remove TUN-thread QFI drop, tx_entity owns unmappable-QFI handling
- replace default_drb with qfi2drb_t default_drb in SDAP entity and init
  default DRB id+role at SDAP entity creation

Refs:
- PR #158
- TS 37.324 §5.2.1, §5.2.2, §6.2.2.1
- TS 38.331 SDAP-Config (sdap-HeaderUL / sdap-HeaderDL)

Signed-off-by: Guido Casati <guido.casati@openairinterface.org>

openair2/SDAP/nr_sdap/nr_sdap.c

@@ -213,14 +213,6 @@ static void *sdap_tun_read_thread(void *arg)
 
     LOG_D(SDAP, "read data of size %d\n", len);
 
-    if (!entity->is_gnb && entity->enable_sdap && (entity->qfi < 0 || entity->qfi >= SDAP_MAX_QFI)) {
-      LOG_W(SDAP,
-            "Dropping UL SDU for UE %ld PDU session %d: no QoS rule QFI available for SDAP header\n",
-            entity->ue_id,
-            entity->pdusession_id);
-      continue;
-    }
-
     protocol_ctxt_t ctxt = {.enb_flag = entity->is_gnb, .rntiMaybeUEid = entity->ue_id};
 
     bool dc = entity->is_gnb ? false : SDAP_HDR_UL_DATA_PDU;

openair2/SDAP/nr_sdap/nr_sdap_entity.c

@@ -43,14 +43,35 @@ static int get_sdap_role(bool is_gnb, const NR_SDAP_Config_t *sdap_config)
 {
   sdap_role_t role_ul = is_gnb ? SDAP_UL_RX : SDAP_UL_TX;
   sdap_role_t role_dl = is_gnb ? SDAP_DL_TX : SDAP_DL_RX;
+  const bool ul_hdr = sdap_config->sdap_HeaderUL == NR_SDAP_Config__sdap_HeaderUL_present;
+  const bool dl_hdr = sdap_config->sdap_HeaderDL == NR_SDAP_Config__sdap_HeaderDL_present;
   int role = NO_SDAP_HEADER;
-  if (sdap_config->sdap_HeaderUL == NR_SDAP_Config__sdap_HeaderUL_present)
+  if (ul_hdr)
     role |= role_ul;
-  if (sdap_config->sdap_HeaderDL == NR_SDAP_Config__sdap_HeaderDL_present)
+  if (dl_hdr)
     role |= role_dl;
+  LOG_D(SDAP,
+        "PDU session %ld %s: sdap-HeaderUL %s, sdap-HeaderDL %s (role 0x%x)\n",
+        sdap_config->pdu_Session,
+        is_gnb ? "gNB" : "UE",
+        ul_hdr ? "present" : "absent",
+        dl_hdr ? "present" : "absent",
+        role);
   return role;
 }
 
+/** Return the first table row for drb_id (all QFIs on a DRB share entity_role) */
+static const qfi2drb_t *nr_sdap_drb_lookup(const nr_sdap_entity_t *entity, int drb_id)
+{
+  for (int qfi = 0; qfi < SDAP_MAX_QFI; qfi++) {
+    if (entity->qfi2drb_table[qfi].drb_id == drb_id)
+      return &entity->qfi2drb_table[qfi];
+  }
+  if (entity->default_drb.drb_id == drb_id)
+    return &entity->default_drb;
+  return NULL;
+}
+
 void nr_pdcp_submit_sdap_ctrl_pdu(ue_id_t ue_id, int sdap_ctrl_pdu_drb, nr_sdap_ul_hdr_t ctrl_pdu)
 {
 
@@ -94,19 +115,20 @@ static bool nr_sdap_tx_entity(nr_sdap_entity_t *entity,
   }
 
   uint8_t sdap_buf[SDAP_MAX_PDU];
-  int drb_id = entity->qfi2drb_map(entity, qfi);
+  const qfi2drb_t *map = entity->qfi2drb_map(entity, qfi);
-
+  if (!map) {
-  if (drb_id != SDAP_MAP_RULE_EMPTY) {
+    LOG_W(SDAP, "Dropping TX SDAP SDU: no DRB mapping for QFI %u (pdu_session=%d)\n", qfi, entity->pdusession_id);
-    sdap_ul_tx = entity->qfi2drb_table[qfi].entity_role & SDAP_UL_TX; // UE TX entity
-    sdap_dl_tx = entity->qfi2drb_table[qfi].entity_role & SDAP_DL_TX; // gNB TX entity
-    LOG_D(SDAP, "TX - QFI: %u is mapped to DRB ID: %d\n", qfi, entity->qfi2drb_table[qfi].drb_id);
-  } else {
-    LOG_E(SDAP, "QFI %u not mapped to any DRB - dropping SDU of size %d\n", qfi, sdu_buffer_size);
     return false;
   }
+  const int drb_id = map->drb_id;
+  const int drb_role = map->entity_role;
+
+  sdap_ul_tx = drb_role & SDAP_UL_TX;
+  sdap_dl_tx = drb_role & SDAP_DL_TX;
+  LOG_D(SDAP, "TX - QFI: %u is mapped to DRB ID: %d\n", qfi, drb_id);
 
   if (!sdap_ul_tx && !sdap_dl_tx) {
-    LOG_D(SDAP, "TX - DRB ID: %d does not have SDAP header\n", entity->qfi2drb_table[qfi].drb_id);
+    LOG_D(SDAP, "TX - DRB ID: %d does not have SDAP header\n", drb_id);
     ret = nr_pdcp_data_req_drb(ctxt_p,
                                srb_flag,
                                drb_id,
@@ -200,12 +222,17 @@ static void nr_sdap_rx_entity(nr_sdap_entity_t *entity,
                               char *buf,
                               int size)
 {
-  bool sdap_ul_rx = false;
-  bool sdap_dl_rx = false;
   int qfi = -1;
-  /* If SDAP header is disabled for this entity, bypass header parsing */
+  const qfi2drb_t *map = nr_sdap_drb_lookup(entity, pdcp_entity);
-  if (entity->enable_sdap) {
+  if (!map) {
-    /** Extract QFI from SDAP header for UL direction
+    LOG_W(SDAP, "Dropping RX payload: DRB %d has no qfi2drb_table entry (ue=%ld, pdu_session=%d)\n", pdcp_entity, ue_id, pdusession_id);
+    return;
+  }
+  const int drb_role = map->entity_role;
+  bool sdap_header_rx = is_gnb ? (drb_role & SDAP_UL_RX) : (drb_role & SDAP_DL_RX);
+
+  if (is_gnb && sdap_header_rx) {
+    /** Extract QFI from SDAP header for gNB UL RX
      * Per TS 37.324 §6.2.2, QFI is carried in SDAP header when SDAP header is present
      * This QFI will be used in GTP-U extension header (PDU Session Container)
      * for N3-U tunnel (the first 6 bits in the first octet) */
@@ -227,15 +254,12 @@ static void nr_sdap_rx_entity(nr_sdap_entity_t *entity,
             pdcp_entity);
       return;
     }
-    sdap_ul_rx = mapping->entity_role & SDAP_UL_RX; // gNB RX entity
-    sdap_dl_rx = mapping->entity_role & SDAP_DL_RX; // UE RX entity
   }
 
-
   if (is_gnb) { // gNB
     /** QFI extracted from SDAP header for UL direction (N3-U tunnel)
      * QFI range: 0..63 (6 bits) */
-    if (sdap_ul_rx) { // UL Data/Control PDU with SDAP header
+    if (sdap_header_rx) { // UL Data/Control PDU with SDAP header
       DevAssert(qfi >= 0 && qfi < SDAP_MAX_QFI);
       int offset = SDAP_HDR_LENGTH;
       nr_sdap_ul_hdr_t *sdap_hdr = (nr_sdap_ul_hdr_t *)buf;
@@ -279,7 +303,7 @@ static void nr_sdap_rx_entity(nr_sdap_entity_t *entity,
      * if the DRB from which this SDAP data PDU is received is configured by RRC with the presence of SDAP header.
      */
     int offset = 0;
-    if (sdap_dl_rx) { // DL Data/Control PDU with SDAP header
+    if (sdap_header_rx) { // DL Data/Control PDU with SDAP header
       offset = SDAP_HDR_LENGTH;
       /*
        * TS 37.324 5.2 Data transfer
@@ -303,7 +327,7 @@ static void nr_sdap_rx_entity(nr_sdap_entity_t *entity,
          * 5.3.2 Reflective mapping
          * If there is no stored QoS flow to DRB mapping rule for the QoS flow and a default DRB is configured.
          */
-        if(!entity->qfi2drb_table[sdap_hdr->QFI].drb_id && entity->default_drb){
+        if (!entity->qfi2drb_table[sdap_hdr->QFI].drb_id && entity->default_drb.drb_id) {
           nr_sdap_ul_hdr_t sdap_ctrl_pdu = entity->sdap_construct_ctrl_pdu(sdap_hdr->QFI);
           int sdap_ctrl_pdu_drb = entity->sdap_map_ctrl_pdu(entity, SDAP_CTRL_PDU_MAP_DEF_DRB, sdap_hdr->QFI);
           entity->sdap_submit_ctrl_pdu(ue_id, sdap_ctrl_pdu_drb, sdap_ctrl_pdu);
@@ -382,31 +406,31 @@ static void nr_sdap_qfi2drb_map_del(nr_sdap_entity_t *entity, const uint8_t qfi)
 
 /**
  * @brief   get the DRB ID mapped to the QFI, for both DL and UL
- * @return  DRB that is mapped to the QFI
+ * @return  qfi2drb_table entry for the QFI {drb_id, entity_role},
- *          or the default DRB if no mapping rule exists
+ *          or the default DRB row if no mapping rule exists,
- *          or 0 if no mapping and no default DRB exists for that QFI
+ *          or NULL if no mapping and no default DRB exists for that QFI
  * @ref     TS 37.324, 5.2.1 Uplink
  *          If there is no stored QoS flow to DRB mapping rule for the
  *          QoS flow as specified in the subclause 5.3, and a default DRB is configured,
  *          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. */
-static int nr_sdap_qfi2drb(nr_sdap_entity_t *entity, uint8_t qfi)
+static const qfi2drb_t *nr_sdap_qfi2drb(const nr_sdap_entity_t *entity, uint8_t qfi)
 {
   /* Fetch DRB ID mapped to QFI */
-  int drb_id = entity->qfi2drb_table[qfi].drb_id;
+  const qfi2drb_t *row = &entity->qfi2drb_table[qfi];
-  if (drb_id) {
+  if (row->drb_id) {
     /* QoS flow to DRB mapping rule exists, return corresponding DRB ID */
-    LOG_D(SDAP, "Existing QoS flow to DRB mapping rule: QFI %u to DRB %d\n", qfi, drb_id);
+    LOG_D(SDAP, "Existing QoS flow to DRB mapping rule: QFI %u to DRB %d\n", qfi, row->drb_id);
-    return drb_id;
+    return row;
-  } else if (entity->default_drb) {
+  } else if (entity->default_drb.drb_id) {
-    /* QoS flow to DRB mapping rule does not exist, map SDAP SDU to default DRB, e.g. return default DRB of the SDAP entity */
+    /* QoS flow to DRB mapping rule does not exist: map SDAP SDU to default DRB */
-    LOG_D(SDAP, "QoS flow to DRB mapping rule does not exists! mapping SDU to Default DRB: %d\n", entity->default_drb);
+    LOG_D(SDAP, "QoS flow to DRB mapping rule does not exists! mapping SDU to Default DRB: %d\n", entity->default_drb.drb_id);
-    return entity->default_drb;
+    return &entity->default_drb;
   } else {
     /* Note: UE undefined behaviour when neither a default DRB
        nor a stored QoS flow to DRB mapping rule exists */
     LOG_E(SDAP, "Mapping rule and default DRB do not exist for QFI:%u\n", qfi);
-    return SDAP_MAP_RULE_EMPTY;
+    return NULL;
   }
 }
 
@@ -425,11 +449,16 @@ int nr_sdap_map_ctrl_pdu(nr_sdap_entity_t *entity, int map_type, uint8_t dl_qfi)
 {
   int drb_of_endmarker = 0;
   if(map_type == SDAP_CTRL_PDU_MAP_DEF_DRB){
-    drb_of_endmarker = entity->default_drb;
+    drb_of_endmarker = entity->default_drb.drb_id;
     LOG_D(SDAP, "Mapping Control PDU QFI: %u to Default DRB: %d\n", dl_qfi, drb_of_endmarker);
   }
   if(map_type == SDAP_CTRL_PDU_MAP_RULE_DRB){
-    drb_of_endmarker = entity->qfi2drb_map(entity, dl_qfi);
+    const qfi2drb_t *map = entity->qfi2drb_map(entity, dl_qfi);
+    if (!map) {
+      LOG_E(SDAP, "No DRB mapping for control PDU QFI %u\n", dl_qfi);
+      return 0;
+    }
+    drb_of_endmarker = map->drb_id;
     LOG_D(SDAP, "Mapping Control PDU QFI: %u to DRB: %d\n", dl_qfi, drb_of_endmarker);
   }
   return drb_of_endmarker;
@@ -454,7 +483,7 @@ static void nr_sdap_ue_control_pdu_config(nr_sdap_entity_t *entity, const ue_id_
   for (int i = 0; i < sdap->mappedQFIs2AddCount; i++) {
     uint8_t qfi = sdap->mappedQFIs2Add[i];
     /* a default DRB exists and there is no stored QFI to DRB mapping rule for the QFI */
-    if (entity->default_drb && entity->qfi2drb_table[qfi].drb_id == SDAP_NO_MAPPING_RULE) {
+    if (entity->default_drb.drb_id && entity->qfi2drb_table[qfi].drb_id == SDAP_NO_MAPPING_RULE) {
       // construct an end-marker control PDU (6.2.3 TS 37.324)
       nr_sdap_ul_hdr_t sdap_ctrl_pdu = entity->sdap_construct_ctrl_pdu(qfi);
       // map the end-marker control PDU to the default DRB
@@ -481,7 +510,7 @@ static void nr_sdap_add_qos_flows_to_drb(nr_sdap_entity_t *entity, const sdap_co
   for (int i = 0; i < sdap->mappedQFIs2AddCount; i++) {
     uint8_t qfi = sdap->mappedQFIs2Add[i];
     LOG_D(SDAP, "Adding QFI to DRB mapping rules: %d mapped QFIs for DRB %d\n", sdap->mappedQFIs2AddCount, sdap->drb_id);
-    if (qfi < SDAP_MAX_QFI && sdap->drb_id > SDAP_MAP_RULE_EMPTY && sdap->drb_id <= MAX_DRBS_PER_UE) {
+    if (qfi < SDAP_MAX_QFI && sdap->drb_id > SDAP_NO_MAPPING_RULE && sdap->drb_id <= MAX_DRBS_PER_UE) {
       entity->qfi2drb_map_add(entity, qfi, sdap->drb_id, sdap->role);
     } else {
       LOG_E(SDAP, "Failed to add qfi2drb mapping: QFI=%d, DRB=%d\n", qfi, sdap->drb_id);
@@ -504,24 +533,31 @@ static void nr_sdap_rm_qos_flows_from_drb(nr_sdap_entity_t *entity, const sdap_c
  * @param drb the DRB ID to be mapped */
 static void nr_sdap_qfi2drb_map_update(nr_sdap_entity_t *entity, const sdap_config_t *sdap)
 {
-  /* disabled SDAP: only one DRB per PDU session existing qfi2drb_table entries must match the DRB to be updated */
-  if (!entity->enable_sdap) {
-    for (int i = 0; i < SDAP_MAX_QFI; i++) {
-      int mapped = entity->qfi2drb_table[i].drb_id;
-      if (mapped != SDAP_NO_MAPPING_RULE) {
-        AssertFatal(mapped == sdap->drb_id,
-                    "PDU session %d: disabled SDAP allows only one DRB per PDU session (qfi2drb_table[%d] maps to DRB %d)\n",
-                    entity->pdusession_id,
-                    i,
-                    mapped);
-      }
-    }
-  }
   if (!entity->is_gnb) { // UE control PDU configuration
     nr_sdap_ue_control_pdu_config(entity, entity->ue_id, sdap);
   }
   nr_sdap_add_qos_flows_to_drb(entity, sdap);
   nr_sdap_rm_qos_flows_from_drb(entity, sdap);
+
+  /* TS 38.331: all QFIs on a DRB share the same sdap-HeaderUL/DL role */
+  for (int i = 0; i < SDAP_MAX_QFI; i++) {
+    if (entity->qfi2drb_table[i].drb_id == sdap->drb_id)
+      entity->qfi2drb_table[i].entity_role = sdap->role;
+  }
+
+  if (sdap->role == NO_SDAP_HEADER) {
+    /* TS 37.324 §6.2.2.1: with both headers absent, only one DRB per PDU session is allowed */
+    int mapped_drbs = 0;
+    for (int drb = 1; drb <= MAX_DRBS_PER_UE; drb++) {
+      for (int qfi = 0; qfi < SDAP_MAX_QFI; qfi++) {
+        if (entity->qfi2drb_table[qfi].drb_id == drb) {
+          mapped_drbs++;
+          break;
+        }
+      }
+    }
+    AssertFatal(mapped_drbs <= 1, "PDU session %d: disabled SDAP but %d DRBs mapped\n", entity->pdusession_id, mapped_drbs);
+  }
 }
 
 /**
@@ -540,7 +576,6 @@ static void nr_sdap_add_entity(const int is_gnb, const ue_id_t ue_id, const sdap
   sdap_entity->ue_id = ue_id;
   sdap_entity->pdusession_id = sdap->pdusession_id;
   sdap_entity->is_gnb = is_gnb;
-  sdap_entity->enable_sdap = (sdap->role != NO_SDAP_HEADER);
 
   // rx/tx entities
   sdap_entity->tx_entity = nr_sdap_tx_entity;
@@ -560,13 +595,14 @@ static void nr_sdap_add_entity(const int is_gnb, const ue_id_t ue_id, const sdap
 
   // set default DRB
   if (sdap->defaultDRB) {
-    sdap_entity->default_drb = sdap->drb_id;
+    sdap_entity->default_drb.drb_id = sdap->drb_id;
-    LOG_I(SDAP, "Default DRB for the created SDAP entity: DRB %d \n", sdap_entity->default_drb);
+    sdap_entity->default_drb.entity_role = sdap->role;
+    LOG_I(SDAP, "Default DRB for the created SDAP entity: DRB %d \n", sdap_entity->default_drb.drb_id);
   }
 
-  if (!sdap_entity->enable_sdap) {
+  if (sdap->role == NO_SDAP_HEADER) {
     AssertFatal(sdap->mappedQFIs2AddCount <= 1,
-                "PDU session %d: disabled SDAP allows at most one QoS flow to add at entity creation\n",
+                "PDU session %d: both SDAP headers absent: at most one QoS flow at entity creation\n",
                 sdap->pdusession_id);
   }
 
@@ -657,22 +693,15 @@ void nr_sdap_entity_update_qos_flows(ue_id_t ue_id, int pdusession_id, int drb_i
     return;
   }
 
-  /* Derive SDAP role from entity configuration */
+  const qfi2drb_t *drb_map = nr_sdap_drb_lookup(entity, drb_id);
-  uint8_t role = NO_SDAP_HEADER;
+  if (!drb_map) {
-  if (entity->enable_sdap) {
+    LOG_W(SDAP, "DRB %d: no qfi2drb_table entry, skip QoS-flow mapping update (pdu_session=%d)\n", drb_id, pdusession_id);
-    if (entity->is_gnb) {
+    return;
-      role |= SDAP_UL_RX;
-      role |= SDAP_DL_TX;
-    } else {
-      role |= SDAP_UL_TX;
-      role |= SDAP_DL_RX;
-    }
   }
-
   sdap_config_t sdap = {.pdusession_id = pdusession_id,
                         .drb_id = drb_id,
-                        .role = role,
+                        .role = drb_map->entity_role,
-                        .defaultDRB = (entity->default_drb == drb_id),
+                        .defaultDRB = (entity->default_drb.drb_id == drb_id),
                         .mappedQFIs2AddCount = n_qfis};
 
   /* Build list of QFIs to be released */
@@ -783,7 +812,6 @@ sdap_config_t nr_sdap_get_config(const int is_gnb, const NR_SDAP_Config_t *sdap_
   sdap_config_t sdapConfig = {0};
   sdapConfig.drb_id = drb_id;
   sdapConfig.role = get_sdap_role(is_gnb, sdap_Config);
-  LOG_D(SDAP, "SDAP headers %s\n", sdapConfig.role == NO_SDAP_HEADER ? "absent" : "present");
   sdapConfig.defaultDRB = sdap_Config->defaultDRB;
   // 3GPP TS 38.331 The network sets sdap-HeaderUL to present if the field defaultDRB is set to true
   if (sdapConfig.defaultDRB && (sdap_Config->sdap_HeaderUL != NR_SDAP_Config__sdap_HeaderUL_present))

openair2/SDAP/nr_sdap/nr_sdap_entity.h

@@ -20,7 +20,6 @@
 #define SDAP_HDR_UL_CTRL_PDU        (0)
 #define SDAP_HDR_LENGTH             (1)
 #define SDAP_MAX_QFI                (64)
-#define SDAP_MAP_RULE_EMPTY         (0)
 #define SDAP_NO_MAPPING_RULE        (0)
 #define SDAP_REFLECTIVE_MAPPING     (1)
 #define SDAP_RQI_HANDLING           (1)
@@ -78,17 +77,16 @@ typedef struct sdap_configuration_s {
 
 typedef struct nr_sdap_entity_s {
   ue_id_t ue_id;
-  int default_drb;
   /// sdap_tun_read_thread needs to know if we are gNB/UE, so for noS1 mode,
   /// store which one we are
   bool is_gnb;
-  bool enable_sdap;
   int pdusession_id;
   int pdusession_sock;
   pthread_t pdusession_thread;
   bool stop_thread;
   int qfi;
 
+  qfi2drb_t default_drb;
   qfi2drb_t qfi2drb_table[SDAP_MAX_QFI];
 
   void (*qfi2drb_map_update)(struct nr_sdap_entity_s *entity, const sdap_config_t *sdap);
@@ -97,7 +95,7 @@ typedef struct nr_sdap_entity_s {
                           const uint8_t qfi,
                           const uint8_t drb_id,
                           const uint8_t role);
-  int (*qfi2drb_map)(struct nr_sdap_entity_s *entity, uint8_t qfi);
+  const qfi2drb_t *(*qfi2drb_map)(const struct nr_sdap_entity_s *entity, uint8_t qfi);
 
   nr_sdap_ul_hdr_t (*sdap_construct_ctrl_pdu)(uint8_t qfi);
   int (*sdap_map_ctrl_pdu)(struct nr_sdap_entity_s *entity, int map_type, uint8_t dl_qfi);