Merge remote-tracking branch 'origin/bugfix-nr-rlc-control-pdu' into integration_2023_w37

openair2/LAYER2/nr_rlc/nr_rlc_entity_am.c

@@ -327,22 +327,16 @@ static void process_control_pdu(nr_rlc_entity_am_t *entity,
   nr_rlc_sdu_segment_t *cur_retransmit_list;
   nr_rlc_sdu_segment_t *new_retransmit_list;
   nr_rlc_sdu_segment_t head_retransmit_list;
+  int cmp;
 
-  head_wait_list.next = entity->wait_list;
-  cur_wait_list       = entity->wait_list;
-  prev_wait_list      = &head_wait_list;
-
-  head_retransmit_list.next = NULL;
-  cur_retransmit_list = entity->retransmit_list;
-  new_retransmit_list = &head_retransmit_list;
+  /* validate the control PDU: read it, check the values of ACK and NACKs */
 
   nr_rlc_pdu_decoder_init(&decoder, buffer, size);
   nr_rlc_pdu_decoder_get_bits(&decoder, 1); R(decoder); /* dc */
 
   cpt = nr_rlc_pdu_decoder_get_bits(&decoder, 3); R(decoder);
   if (cpt != 0) {
-    LOG_E(RLC, "%s:%d:%s: warning: discard PDU, CPT not 0 (%d)\n",
-          __FILE__, __LINE__, __FUNCTION__, cpt);
+    LOG_E(RLC, "discard PDU, CPT not 0 (%d)\n", cpt);
     goto err;
   }
   ack_sn = nr_rlc_pdu_decoder_get_bits(&decoder, entity->sn_field_length); R(decoder);
@@ -354,12 +348,20 @@ static void process_control_pdu(nr_rlc_entity_am_t *entity,
     nr_rlc_pdu_decoder_get_bits(&decoder, 7); R(decoder);
   }
 
-  /* 38.322 5.3.3.3 says to stop t_poll_retransmit if a ACK or NACK is
-   * received for the SN 'poll_sn' - check ACK case (NACK done below)
+  /* discard the whole control PDU if ack_sn is invalid, that is
+   * if it does not satisfy tx_next_ack <= ack_sn <= tx_next
+   * (no need to test tx_next_ack <= ack_sn, this is always true since
+   * tx_next_ack is the modulus base)
    */
-  if (sn_compare_tx(entity, entity->poll_sn, ack_sn) < 0)
-    entity->t_poll_retransmit_start = 0;
+  if (sn_compare_tx(entity, ack_sn, entity->tx_next) > 0) {
+    LOG_W(RLC, "ack_sn (%d) not valid (tx_next_ack %d tx_next %d), discard control PDU\n",
+          ack_sn, entity->tx_next_ack, entity->tx_next);
+    return;
+  }
 
+  /* discard the whole control PDU if NACKs are bad (not <= ack_sn, not in
+   * increasing order)
+   */
   while (e1) {
     nack_sn = nr_rlc_pdu_decoder_get_bits(&decoder, entity->sn_field_length); R(decoder);
     e1 = nr_rlc_pdu_decoder_get_bits(&decoder, 1); R(decoder);
@@ -387,35 +389,98 @@ static void process_control_pdu(nr_rlc_entity_am_t *entity,
     if (so_end == 0xffff)
       so_end = -1;
 
-    /* process nacks */
     for (i = 0; i < range; i++) {
       int cur_nack_sn  = (nack_sn + i) % entity->sn_modulus;
       int cur_so_start = i == 0 ?         so_start : 0;
       int cur_so_end   = i == range - 1 ? so_end : -1;
 
       /* check that current nack is > previous nack and <= ack
-       * if not then skip it and all following nacks, and
-       * do not touch t_poll_retransmit
+       * if not then reject the control PDU
        */
       if (prev_nack_sn != -1) {
-        int cmp = sn_compare_tx(entity, cur_nack_sn, prev_nack_sn);
+        cmp = sn_compare_tx(entity, cur_nack_sn, prev_nack_sn);
         if (cmp < 0
             || (cmp == 0
                 && (prev_so_end == -1
                     || cur_so_start <= prev_so_end))) {
-          LOG_E(RLC, "%s:%d:%s: bad NACK (nack sn %d so start/end %d/%d, previous nack sn %d so start/end %d/%d), skip it and all following NACKs\n",
-                __FILE__, __LINE__, __FUNCTION__,
+          LOG_E(RLC, "bad NACK, not bigger than previous NACK (nack sn %d so start/end %d/%d, previous nack sn %d so start/end %d/%d)\n",
                 cur_nack_sn, cur_so_start, cur_so_end,
                 prev_nack_sn, prev_so_start, prev_so_end);
-          goto nacks_done;
+          goto err;
         }
       }
       if (sn_compare_tx(entity, cur_nack_sn, ack_sn) > 0) {
-        LOG_E(RLC, "%s:%d:%s: bad NACK (nack %d ack %d), skip it and all following NACKs\n",
-              __FILE__, __LINE__, __FUNCTION__,
-              cur_nack_sn, ack_sn);
-        goto nacks_done;
+        LOG_E(RLC, "bad NACK, bigger than ACK (nack %d ack %d) (tx_next_ack %d tx_next %d)\n",
+              cur_nack_sn, ack_sn, entity->tx_next_ack, entity->tx_next);
+        goto err;
       }
+    }
+  }
+
+  /* process the control PDU for real (checks not needed now, previous steps
+   * validated everything)
+   */
+
+  head_wait_list.next = entity->wait_list;
+  cur_wait_list       = entity->wait_list;
+  prev_wait_list      = &head_wait_list;
+
+  head_retransmit_list.next = NULL;
+  cur_retransmit_list = entity->retransmit_list;
+  new_retransmit_list = &head_retransmit_list;
+
+  nr_rlc_pdu_decoder_init(&decoder, buffer, size);
+  nr_rlc_pdu_decoder_get_bits(&decoder, 1);             /* dc */
+
+  cpt = nr_rlc_pdu_decoder_get_bits(&decoder, 3);
+  ack_sn = nr_rlc_pdu_decoder_get_bits(&decoder, entity->sn_field_length);
+  e1 = nr_rlc_pdu_decoder_get_bits(&decoder, 1);
+  /* r bits */
+  if (entity->sn_field_length == 18) {
+    nr_rlc_pdu_decoder_get_bits(&decoder, 1);
+  } else {
+    nr_rlc_pdu_decoder_get_bits(&decoder, 7);
+  }
+
+  /* 38.322 5.3.3.3 says to stop t_poll_retransmit if a ACK or NACK is
+   * received for the SN 'poll_sn' - check ACK case (NACK done below)
+   */
+  if (sn_compare_tx(entity, entity->poll_sn, ack_sn) < 0)
+    entity->t_poll_retransmit_start = 0;
+
+  while (e1) {
+    nack_sn = nr_rlc_pdu_decoder_get_bits(&decoder, entity->sn_field_length);
+    e1 = nr_rlc_pdu_decoder_get_bits(&decoder, 1);
+    e2 = nr_rlc_pdu_decoder_get_bits(&decoder, 1);
+    e3 = nr_rlc_pdu_decoder_get_bits(&decoder, 1);
+    /* r bits */
+    if (entity->sn_field_length == 18) {
+      nr_rlc_pdu_decoder_get_bits(&decoder, 3);
+    } else {
+      nr_rlc_pdu_decoder_get_bits(&decoder, 1);
+    }
+    if (e2) {
+      so_start = nr_rlc_pdu_decoder_get_bits(&decoder, 16);
+      so_end = nr_rlc_pdu_decoder_get_bits(&decoder, 16);
+    } else {
+      so_start = 0;
+      so_end = 0xffff;
+    }
+    if (e3) {
+      range = nr_rlc_pdu_decoder_get_bits(&decoder, 8);
+    } else {
+      range = 1;
+    }
+    /* special value 0xffff indicates 'all bytes to the end' */
+    if (so_end == 0xffff)
+      so_end = -1;
+
+    /* process nacks */
+    for (i = 0; i < range; i++) {
+      int cur_nack_sn  = (nack_sn + i) % entity->sn_modulus;
+      int cur_so_start = i == 0 ?         so_start : 0;
+      int cur_so_end   = i == range - 1 ? so_end : -1;
+
 
 process_next_pdu:
       /* process smallest SN either from wait_list or retransmit list */
@@ -425,9 +490,10 @@ process_next_pdu:
         goto process_retransmit_list_head;
       if (cur_retransmit_list == NULL)
         goto process_wait_list_head;
-      if (cur_wait_list->sdu->sn < cur_retransmit_list->sdu->sn
-          || (cur_wait_list->sdu->sn == cur_retransmit_list->sdu->sn &&
-              cur_wait_list->so < cur_retransmit_list->so))
+      cmp = sn_compare_tx(entity, cur_wait_list->sdu->sn, cur_retransmit_list->sdu->sn);
+      if (cmp < 0
+          || (cmp == 0
+              && cur_wait_list->so < cur_retransmit_list->so))
         goto process_wait_list_head;
       goto process_retransmit_list_head;
 
@@ -468,8 +534,9 @@ process_wait_list_head:
       }
 
       /* if current segment SN > current NACK, we can't classify it yet */
-      if (sn_compare_tx(entity, cur_wait_list->sdu->sn, cur_nack_sn) > 0
-          || (cur_wait_list->sdu->sn == cur_nack_sn
+      cmp = sn_compare_tx(entity, cur_wait_list->sdu->sn, cur_nack_sn);
+      if (cmp > 0
+          || (cmp == 0
               && cur_wait_list->so > cur_so_start))
         goto done_nack;
 
@@ -528,8 +595,9 @@ process_retransmit_list_head:
       }
 
       /* if current segment SN > current NACK, we can't classify it yet */
-      if (sn_compare_tx(entity, cur_retransmit_list->sdu->sn, cur_nack_sn) > 0
-          || (cur_retransmit_list->sdu->sn == cur_nack_sn
+      cmp = sn_compare_tx(entity, cur_retransmit_list->sdu->sn, cur_nack_sn);
+      if (cmp > 0
+          || (cmp == 0
               && cur_retransmit_list->so > cur_so_start))
         goto done_nack;
 
@@ -584,7 +652,6 @@ lists_over:
       entity->t_poll_retransmit_start = 0;
   } /* while (e1) */
 
-nacks_done:
   /* nacks done, finish with ack */
   /* we may report successful delivery out of order, if it's a problem
    * then we can have a single loop and deal with the smallest sn of
@@ -664,8 +731,7 @@ nacks_done:
   return;
 
 err:
-  LOG_E(RLC, "%s:%d:%s: error decoding PDU, NR RLC entity in inconsistent state\n",
-        __FILE__, __LINE__, __FUNCTION__);
+  LOG_E(RLC, "error decoding control PDU, discarding\n");
 
 #undef R
 }

openair2/LAYER2/nr_rlc/tests/LOG/log.h

@@ -3,8 +3,8 @@
 
 #include <stdio.h>
 
-#define LOG_E(x, ...) printf(__VA_ARGS__)
-#define LOG_D(x, ...) printf(__VA_ARGS__)
-#define LOG_W(x, ...) printf(__VA_ARGS__)
+#define LOG_E(x, ...) do { printf("LOG_E: "); printf(__VA_ARGS__); } while (0)
+#define LOG_D(x, ...) do { printf("LOG_D: "); printf(__VA_ARGS__); } while (0)
+#define LOG_W(x, ...) do { printf("LOG_W: "); printf(__VA_ARGS__); } while (0)
 
 #endif /* _NR_RLC_TESTS_LOG_H_ */