Merge remote-tracking branch 'origin/rlc-v2-bugfix-status-reporting' into integration_2021_wk02

openair2/LAYER2/rlc_v2/TODO

@@ -13,6 +13,3 @@ RLC AM
 - 36.322 5.1.3.2.3 Actions when a RLC data PDU is placed in the reception
   buffer
   [...] and in-sequence byte segments of the AMD PDU with SN = VR(R) [...]
-
-- use SOstart/SOend in NACK reporting, do not NACK full PDU if
-  parts of it have been received

openair2/LAYER2/rlc_v2/rlc_entity_am.c

@@ -898,6 +898,64 @@ static tx_pdu_size_t compute_new_pdu_size(rlc_entity_am_t *entity, int maxsize)
   return ret;
 }
 
+/* return number of missing parts of a sn
+ * if everything is missing (nothing received), return 0
+ */
+static int count_nack_missing_parts(rlc_entity_am_t *entity, int sn)
+{
+  rlc_rx_pdu_segment_t *l = entity->rx_list;
+  int last_byte;
+  int new_last_byte;
+  int count;
+
+  while (l != NULL) {
+    if (l->sn == sn)
+      break;
+    l = l->next;
+  }
+  /* nothing received, everything is missing, return 0 */
+  if (l == NULL)
+    return 0;
+
+  last_byte = -1;
+  count = 0;
+  while (l != NULL && l->sn == sn) {
+    if (l->so > last_byte + 1)
+      /* missing part detected */
+      count++;
+    if (l->is_last)
+      /* end of PDU reached - no more missing part to add */
+      return count;
+    new_last_byte = l->so + l->size - l->data_offset - 1;
+    if (new_last_byte > last_byte)
+      last_byte = new_last_byte;
+    l = l->next;
+  }
+  /* at this point: end of PDU not received, one more missing part */
+  count++;
+
+  return count;
+}
+
+/* return size of nack reporting for sn, in bits */
+static int segment_nack_size(rlc_entity_am_t *entity, int sn)
+{
+  /* nack + e1 + e2 = 12 bits
+   * SOstart + SOend = 30 bits
+   */
+  int count;
+
+  count = count_nack_missing_parts(entity, sn);
+
+  /* count_nack_missing_parts returns 0 when everything is missing
+   * in which case we don't have SOstart/SOend, so only 12 bits
+   */
+  if (count == 0)
+    return 12;
+
+  return count * (12 + 30);
+}
+
 static int status_size(rlc_entity_am_t *entity, int maxsize)
 {
   /* let's count bits */
@@ -912,11 +970,16 @@ static int status_size(rlc_entity_am_t *entity, int maxsize)
     return 0;
   }
 
-  /* each NACK adds 12 bits */
+  /* add size of NACKs */
   sn = entity->vr_r;
-  while (bits + 12 <= maxsize && sn_compare_rx(entity, sn, entity->vr_ms) < 0) {
-    if (!(rlc_am_segment_full(entity, sn)))
-      bits += 12;
+  while (sn_compare_rx(entity, sn, entity->vr_ms) < 0) {
+    if (!(rlc_am_segment_full(entity, sn))) {
+      int nack_size = segment_nack_size(entity, sn);
+      /* stop there if not enough room */
+      if (bits + nack_size > maxsize)
+        break;
+      bits += nack_size;
+    }
     sn = (sn + 1) % 1024;
   }
 
@@ -929,7 +992,8 @@ static int generate_status(rlc_entity_am_t *entity, char *buffer, int size)
   int bits = 15;               /* minimum size is 15 (header+ack_sn+e1) */
   int sn;
   rlc_pdu_encoder_t encoder;
-  int has_nack = 0;
+  rlc_pdu_encoder_t encoder_ack;
+  rlc_pdu_encoder_t previous_e1;
   int ack;
 
   rlc_pdu_encoder_init(&encoder, buffer, size);
@@ -947,24 +1011,68 @@ static int generate_status(rlc_entity_am_t *entity, char *buffer, int size)
   rlc_pdu_encoder_put_bits(&encoder, 0, 3);   /* CPT */
 
   /* reserve room for ACK (it will be set after putting the NACKs) */
+  encoder_ack = encoder;
   rlc_pdu_encoder_put_bits(&encoder, 0, 10);
+  /* put 0 for e1, will be set to 1 later in the code if needed */
+  previous_e1 = encoder;
+  rlc_pdu_encoder_put_bits(&encoder, 0, 1);
 
   /* at this point, ACK is VR(R) */
   ack = entity->vr_r;
 
-  /* each NACK adds 12 bits */
   sn = entity->vr_r;
-  while (bits + 12 <= size && sn_compare_rx(entity, sn, entity->vr_ms) < 0) {
+  while (sn_compare_rx(entity, sn, entity->vr_ms) < 0) {
     if (!(rlc_am_segment_full(entity, sn))) {
-      /* put previous e1 (is 1) */
-      rlc_pdu_encoder_put_bits(&encoder, 1, 1);
-      /* if previous was NACK, put previous e2 (0, we don't do 'so' thing) */
-      if (has_nack)
-        rlc_pdu_encoder_put_bits(&encoder, 0, 1);
-      /* put NACKed sn */
-      rlc_pdu_encoder_put_bits(&encoder, sn, 10);
-      has_nack = 1;
-      bits += 12;
+      rlc_rx_pdu_segment_t *l;
+      int i;
+      int count     = count_nack_missing_parts(entity, sn);
+      int nack_bits = count == 0 ? 12 : count * (12 + 30);
+      int last_byte;
+      int new_last_byte;
+      int so_start;
+      int so_end;
+      /* if not enough room, stop putting NACKs */
+      if (bits + nack_bits > size)
+        break;
+      /* set previous e1 to 1 */
+      rlc_pdu_encoder_put_bits(&previous_e1, 1, 1);
+
+      if (count == 0) {
+        /* simply NACK, no SOstart/SOend */
+        rlc_pdu_encoder_put_bits(&encoder, sn, 10);                 /* nack */
+        previous_e1 = encoder;
+        rlc_pdu_encoder_put_bits(&encoder, 0, 2);                 /* e1, e2 */
+      } else {
+        /* count is not 0, so we have 'count' NACK+e1+e2+SOstart+SOend */
+        l = entity->rx_list;
+        last_byte = -1;
+        while (l->sn != sn) l = l->next;
+        for (i = 0; i < count; i++) {
+          rlc_pdu_encoder_put_bits(&encoder, sn, 10);               /* nack */
+          previous_e1 = encoder;
+          /* all NACKs but the last have a following NACK, set e1 for them */
+          rlc_pdu_encoder_put_bits(&encoder, i != count - 1, 1);      /* e1 */
+          rlc_pdu_encoder_put_bits(&encoder, 1, 1);                   /* e2 */
+          /* look for the next segment with missing data before it */
+          while (l != NULL && l->sn == sn && !(l->so > last_byte + 1)) {
+            new_last_byte = l->so + l->size - l->data_offset - 1;
+            if (new_last_byte > last_byte)
+              last_byte = new_last_byte;
+            l = l->next;
+          }
+          so_start = last_byte + 1;
+          if (l == NULL)
+            so_end = 0x7fff;
+          else
+            so_end = l->so - 1;
+          rlc_pdu_encoder_put_bits(&encoder, so_start, 15);
+          rlc_pdu_encoder_put_bits(&encoder, so_end, 15);
+          if (l != NULL)
+            last_byte = l->so + l->size - l->data_offset - 1;
+        }
+      }
+
+      bits += nack_bits;
     } else {
       /* this sn is full and we put all NACKs before it, use it for ACK */
       ack = (sn + 1) % 1024;
@@ -972,24 +1080,10 @@ static int generate_status(rlc_entity_am_t *entity, char *buffer, int size)
     sn = (sn + 1) % 1024;
   }
 
-  /* go to highest full sn+1 for ACK, VR(MS) is the limit */
-  while (sn_compare_rx(entity, sn, entity->vr_ms) < 0 &&
-         rlc_am_segment_full(entity, sn)) {
-    ack = (sn + 1) % 1024;
-    sn = (sn + 1) % 1024;
-  }
-
-  /* at this point, if last put was NACK then put 2 bits else put 1 bit */
-  if (has_nack)
-    rlc_pdu_encoder_put_bits(&encoder, 0, 2);
-  else
-    rlc_pdu_encoder_put_bits(&encoder, 0, 1);
-
   rlc_pdu_encoder_align(&encoder);
 
   /* let's put the ACK */
-  buffer[0] |= ack >> 6;
-  buffer[1] |= (ack & 0x3f) << 2;
+  rlc_pdu_encoder_put_bits(&encoder_ack, ack, 10);
 
   /* reset the trigger */
   entity->status_triggered = 0;

openair2/LAYER2/rlc_v2/rlc_pdu.c

@@ -233,9 +233,10 @@ static void put_bit(rlc_pdu_encoder_t *encoder, int bit)
     exit(1);
   }
 
-  encoder->buffer[encoder->byte] <<= 1;
   if (bit)
-    encoder->buffer[encoder->byte] |= 1;
+    encoder->buffer[encoder->byte] |= 1 << (7 - encoder->bit);
+  else
+    encoder->buffer[encoder->byte] &= ~(1 << (7 - encoder->bit));
 
   encoder->bit++;
   if (encoder->bit == 8) {

openair2/LAYER2/rlc_v2/tests/run_tests.sh

 #!/bin/sh
 
-test_count=45
+test_count=49
 
 for i in `seq $test_count`
 do

openair2/LAYER2/rlc_v2/tests/test46.h

+/*
+ * rlc am test ack/nack reporting with segmented SDUs
+ *   eNB sends SDU in big PDU, not received
+ *   eNB retx with smaller PDUs, not received except a few
+ *   after some time, RX works again, UE should NACK with SOstart/SOend
+ *
+ * this test was written because in the past we didn't generate NACKs with
+ * SOstart/SOend, potentially leading to infinite retransmissions, even with
+ * good radio conditions (well, there will be max ReTX reached at some point).
+ * The scenario is as follows:
+ * the eNB transmits many many PDUs segments and asks for status
+ * report at some point (but before sending all the segments). The UE then
+ * generates a (wrong) NACK without SOstart/SOend, and then the eNB has to
+ * resend everything from start (all bytes of the PDU have been NACKed because
+ * there is no SOstart/SOend). Then the eNB will again ask for status report
+ * before sending everything and the process repeats all over again.
+ */
+TIME, 1,
+    ENB_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,
+    UE_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,
+    UE_RECV_FAILS, 1,
+    ENB_RECV_FAILS, 1,
+    ENB_SDU, 0, 300,
+    ENB_PDU_SIZE, 500,
+    UE_PDU_SIZE, 1000,
+TIME, 2,
+    ENB_PDU_SIZE, 7,
+TIME, 50,
+    UE_RECV_FAILS, 0,
+TIME, 51,
+    UE_RECV_FAILS, 1,
+TIME, 60,
+    UE_RECV_FAILS, 0,
+TIME, 61,
+    UE_RECV_FAILS, 1,
+TIME, 70,
+    UE_RECV_FAILS, 0,
+TIME, 71,
+    UE_RECV_FAILS, 1,
+TIME, 80,
+    UE_RECV_FAILS, 0,
+TIME, 81,
+    UE_RECV_FAILS, 1,
+TIME, 140,
+    UE_RECV_FAILS, 0,
+    ENB_RECV_FAILS, 0,
+TIME, 150,
+    ENB_PDU_SIZE, 200,
+TIME, -1

openair2/LAYER2/rlc_v2/tests/test47.h

+/*
+ * am: test function segment_nack_size, case "if (count == 0) return 12;"
+ * The eNB sends 5 PDUs, only first and last received by the UE. Then we
+ * ask for the UE's buffer status to trigger the case.
+ */
+TIME, 1,
+    ENB_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,
+    UE_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,
+    ENB_RECV_FAILS, 1,
+    ENB_PDU_SIZE, 100,
+    UE_PDU_SIZE, 100,
+TIME, 2,
+    ENB_SDU, 0, 10,
+TIME, 3,
+    UE_RECV_FAILS, 1,
+    ENB_SDU, 1, 10,
+TIME, 4,
+    ENB_SDU, 2, 10,
+TIME, 5,
+    ENB_SDU, 3, 10,
+TIME, 6,
+    UE_RECV_FAILS, 0,
+    ENB_RECV_FAILS, 0,
+    ENB_SDU, 4, 10,
+TIME, 42,
+    UE_BUFFER_STATUS,
+TIME, -1

openair2/LAYER2/rlc_v2/tests/test48.h

+/*
+ * am: test case "if (bits + nack_size > maxsize)" in functions status_size
+ * and generate_status.
+ * The eNB sends 5 PDUs, only first and last received by the UE. Then we
+ * ask for the UE's buffer status to trigger the case. UE PDU size is
+ * limited to 6 to trigger the case.
+ */
+TIME, 1,
+    ENB_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,
+    UE_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,
+    ENB_RECV_FAILS, 1,
+    ENB_PDU_SIZE, 100,
+    UE_PDU_SIZE, 6,
+TIME, 2,
+    ENB_SDU, 0, 10,
+TIME, 3,
+    UE_RECV_FAILS, 1,
+    ENB_SDU, 1, 10,
+TIME, 4,
+    ENB_SDU, 2, 10,
+TIME, 5,
+    ENB_SDU, 3, 10,
+TIME, 6,
+    UE_RECV_FAILS, 0,
+    ENB_RECV_FAILS, 0,
+    ENB_SDU, 4, 10,
+TIME, 42,
+    UE_BUFFER_STATUS,
+TIME, -1

openair2/LAYER2/rlc_v2/tests/test49.h

+/*
+ * rlc am test ack/nack reporting with segmented SDUs
+ *
+ * 5 SDUs put into 6 PDUs, all PDUs received expect the 3rd. Then
+ * retransmission happens with smaller PDU size.
+ */
+TIME, 1,
+    ENB_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,
+    UE_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,
+    ENB_SDU, 0, 60,
+    ENB_SDU, 1, 160,
+    ENB_SDU, 2, 130,
+    ENB_SDU, 3, 120,
+    ENB_SDU, 4, 100,
+    ENB_PDU_SIZE, 100,
+    UE_PDU_SIZE, 1000,
+TIME, 3,
+    UE_RECV_FAILS, 1,
+TIME, 4,
+    UE_RECV_FAILS, 0,
+TIME, 7,
+    ENB_PDU_SIZE, 40,
+TIME, 42,
+    UE_RECV_FAILS, 1,
+TIME, 80,
+    UE_RECV_FAILS, 0,
+TIME, -1

openair2/LAYER2/rlc_v2/tests/test6.h

@@ -5,9 +5,6 @@
  *   [1..600] is received but ACK/NACK not
  *   eNB retx with still smaller PDUs [1..400] [401..600] [601..900]
  *   all is received, ACKs/NACKs go through
- *
- * this test will fail if NACK mechanism uses SOstart/SOend
- * (not implemented for the moment)
  */
 TIME, 1,
     ENB_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,

openair2/LAYER2/rlc_v2/tests/test7.h

@@ -6,9 +6,6 @@
  *   eNB retx with still smaller PDUs [1..400] [401..600] [601..900]
  *   [401..600] received, ACK goes through
  *   link clean, all goes through
- *
- * this test will fail if NACK mechanism uses SOstart/SOend
- * (not implemented for the moment)
  */
 TIME, 1,
     ENB_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,

openair2/LAYER2/rlc_v2/tests/test9.h

@@ -11,9 +11,6 @@
  * In the current RLC implementation, this is impossible. If we send [12..21]
  * it means [1..21] has been split and so we won't sent it later on.
  * Maybe with HARQ retransmissions in PHY/MAC in bad radio conditions?
- *
- * this test will fail if NACK mechanism uses SOstart/SOend
- * (not implemented for the moment)
  */
 TIME, 1,
     ENB_AM, 100000, 100000, 35, 0, 45, -1, -1, 4,