rlc v2: implement SOstart/SOend in NACK reporting

The necessity of SOstart/SOend was understood after email exchanges with
David Breaux <davidbreaux67@gmail.com>.

See openair2/LAYER2/rlc_v2/tests/test46.h for some details.

Basically if we NACK without SOstart/SOend then the other end of the RLC
channel will consider all the bytes as NACKed and will have to resend them
all. If another status is then reported before all the bytes are sent,
and because we don't use SOstart/SOend this status will NACK all the bytes
again, then all the bytes are to be sent again. And the cycle repeats.
It will end with a max retx reached (which triggers RLF, Radio Link Failure).

The function put_bit in openair2/LAYER2/rlc_v2/rlc_pdu.c has been changed
too, maybe introducing some performance issues.

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,