Add sack tx unit test

Change-Id: Ib91db6e531231bdc52b0104673a912bee024872f
Signed-off-by: Florin Coras <fcoras@cisco.com>

4 files changed, 185 insertions, 18 deletions

diff --git a/src/vnet/tcp/tcp.h b/src/vnet/tcp/tcp.h
index 2ac6a9b8d55..40fb3515635 100644
--- a/src/vnet/tcp/tcp.h
+++ b/src/vnet/tcp/tcp.h
@@ -59,6 +59,7 @@ typedef enum _tcp_state
 
 format_function_t format_tcp_state;
 format_function_t format_tcp_flags;
+format_function_t format_tcp_sacks;
 
 /** TCP timers */
 #define foreach_tcp_timer               \
@@ -470,11 +471,13 @@ tcp_available_snd_space (const tcp_connection_t * tc)
 void tcp_update_rcv_wnd (tcp_connection_t * tc);
 
 void tcp_retransmit_first_unacked (tcp_connection_t * tc);
-
 void tcp_fast_retransmit (tcp_connection_t * tc);
 void tcp_cc_congestion (tcp_connection_t * tc);
 void tcp_cc_recover (tcp_connection_t * tc);
 
+/* Made public for unit testing only */
+void tcp_update_sack_list (tcp_connection_t * tc, u32 start, u32 end);
+
 always_inline u32
 tcp_time_now (void)
 {
@@ -496,7 +499,6 @@ tcp_prepare_retransmit_segment (tcp_connection_t * tc, vlib_buffer_t * b,
 
 void tcp_connection_timers_init (tcp_connection_t * tc);
 void tcp_connection_timers_reset (tcp_connection_t * tc);
-
 void tcp_connection_init_vars (tcp_connection_t * tc);
 
 always_inline void
diff --git a/src/vnet/tcp/tcp_format.c b/src/vnet/tcp/tcp_format.c
index 1ca2f58ea5d..3148fd406c9 100644
--- a/src/vnet/tcp/tcp_format.c
+++ b/src/vnet/tcp/tcp_format.c
@@ -128,6 +128,18 @@ format_tcp_header (u8 * s, va_list * args)
   return s;
 }
 
+u8 *
+format_tcp_sacks (u8 * s, va_list * args)
+{
+  sack_block_t *sacks = va_arg (*args, sack_block_t *);
+  sack_block_t *block;
+  vec_foreach (block, sacks)
+  {
+    s = format (s, " start %u end %u\n", block->start, block->end);
+  }
+  return s;
+}
+
 /*
  * fd.io coding-style-patch-verification: ON
  *
diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c
index bfe3665ab00..e184a4d69e3 100644
--- a/src/vnet/tcp/tcp_input.c
+++ b/src/vnet/tcp/tcp_input.c
@@ -894,37 +894,51 @@ tcp_rcv_ack (tcp_connection_t * tc, vlib_buffer_t * b,
  * @param start Start sequence number of the newest SACK block
  * @param end End sequence of the newest SACK block
  */
-static void
+void
 tcp_update_sack_list (tcp_connection_t * tc, u32 start, u32 end)
 {
-  sack_block_t *new_list = 0, block;
+  sack_block_t *new_list = 0, *block = 0;
   int i;
 
   /* If the first segment is ooo add it to the list. Last write might've moved
    * rcv_nxt over the first segment. */
   if (seq_lt (tc->rcv_nxt, start))
     {
-      block.start = start;
-      block.end = end;
-      vec_add1 (new_list, block);
+      vec_add2 (new_list, block, 1);
+      block->start = start;
+      block->end = end;
     }
 
   /* Find the blocks still worth keeping. */
   for (i = 0; i < vec_len (tc->snd_sacks); i++)
     {
-      /* Discard if:
-       * 1) rcv_nxt advanced beyond current block OR
-       * 2) Segment overlapped by the first segment, i.e., it has been merged
-       *    into it.*/
-      if (seq_leq (tc->snd_sacks[i].start, tc->rcv_nxt)
-	  || seq_leq (tc->snd_sacks[i].start, end))
+      /* Discard if rcv_nxt advanced beyond current block */
+      if (seq_leq (tc->snd_sacks[i].start, tc->rcv_nxt))
 	continue;
 
-      /* Save to new SACK list. */
-      vec_add1 (new_list, tc->snd_sacks[i]);
+      /* Merge or drop if segment overlapped by the new segment */
+      if (block && (seq_geq (tc->snd_sacks[i].end, new_list[0].start)
+		    && seq_leq (tc->snd_sacks[i].start, new_list[0].end)))
+	{
+	  if (seq_lt (tc->snd_sacks[i].start, new_list[0].start))
+	    new_list[0].start = tc->snd_sacks[i].start;
+	  if (seq_lt (new_list[0].end, tc->snd_sacks[i].end))
+	    new_list[0].end = tc->snd_sacks[i].end;
+	  continue;
+	}
+
+      /* Save to new SACK list if we have space. */
+      if (vec_len (new_list) < TCP_MAX_SACK_BLOCKS)
+	{
+	  vec_add1 (new_list, tc->snd_sacks[i]);
+	}
+      else
+	{
+	  clib_warning ("dropped sack blocks");
+	}
     }
 
-  ASSERT (vec_len (new_list) < TCP_MAX_SACK_BLOCKS);
+  ASSERT (vec_len (new_list) <= TCP_MAX_SACK_BLOCKS);
 
   /* Replace old vector with new one */
   vec_free (tc->snd_sacks);
diff --git a/src/vnet/tcp/tcp_test.c b/src/vnet/tcp/tcp_test.c
index d65ce1beeba..bca5795af12 100644
--- a/src/vnet/tcp/tcp_test.c
+++ b/src/vnet/tcp/tcp_test.c
@@ -35,7 +35,7 @@
 }
 
 static int
-tcp_test_sack ()
+tcp_test_sack_rx ()
 {
   tcp_connection_t _tc, *tc = &_tc;
   sack_scoreboard_t *sb = &tc->sack_sb;
@@ -173,6 +173,145 @@ tcp_test_sack ()
   return 0;
 }
 
+static int
+tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input)
+{
+  tcp_connection_t _tc, *tc = &_tc;
+  sack_block_t *sacks;
+  int i, verbose = 0;
+
+  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
+    {
+      if (unformat (input, "verbose"))
+	verbose = 1;
+      else
+	{
+	  vlib_cli_output (vm, "parse error: '%U'", format_unformat_error,
+			   input);
+	  return -1;
+	}
+    }
+
+  memset (tc, 0, sizeof (*tc));
+
+  /*
+   * Add odd sack block pairs
+   */
+  for (i = 1; i < 10; i += 2)
+    {
+      tcp_update_sack_list (tc, i * 100, (i + 1) * 100);
+    }
+
+  TCP_TEST ((vec_len (tc->snd_sacks) == 5), "sack blocks %d expected %d",
+	    vec_len (tc->snd_sacks), 5);
+  TCP_TEST ((tc->snd_sacks[0].start = 900),
+	    "first sack block start %u expected %u", tc->snd_sacks[0].start,
+	    900);
+
+  /*
+   * Try to add one extra
+   */
+  sacks = vec_dup (tc->snd_sacks);
+
+  tcp_update_sack_list (tc, 1100, 1200);
+  TCP_TEST ((vec_len (tc->snd_sacks) == 5), "sack blocks %d expected %d",
+	    vec_len (tc->snd_sacks), 5);
+  TCP_TEST ((tc->snd_sacks[0].start == 1100),
+	    "first sack block start %u expected %u", tc->snd_sacks[0].start,
+	    1100);
+
+  /* restore */
+  vec_free (tc->snd_sacks);
+  tc->snd_sacks = sacks;
+
+  /*
+   * Overlap first 2 segment
+   */
+  tc->rcv_nxt = 300;
+  tcp_update_sack_list (tc, 300, 300);
+  if (verbose)
+    vlib_cli_output (vm, "overlap first 2 segments:\n%U",
+		     format_tcp_sacks, tc->snd_sacks);
+  TCP_TEST ((vec_len (tc->snd_sacks) == 3), "sack blocks %d expected %d",
+	    vec_len (tc->snd_sacks), 3);
+  TCP_TEST ((tc->snd_sacks[0].start == 900),
+	    "first sack block start %u expected %u", tc->snd_sacks[0].start,
+	    500);
+
+  /*
+   * Add a new segment
+   */
+  tcp_update_sack_list (tc, 1100, 1200);
+  if (verbose)
+    vlib_cli_output (vm, "add new segment [1100, 1200]\n%U",
+		     format_tcp_sacks, tc->snd_sacks);
+  TCP_TEST ((vec_len (tc->snd_sacks) == 4), "sack blocks %d expected %d",
+	    vec_len (tc->snd_sacks), 4);
+  TCP_TEST ((tc->snd_sacks[0].start == 1100),
+	    "first sack block start %u expected %u", tc->snd_sacks[0].start,
+	    1100);
+
+  /*
+   * Join middle segments
+   */
+  tcp_update_sack_list (tc, 800, 900);
+  if (verbose)
+    vlib_cli_output (vm, "join middle segments [800, 900]\n%U",
+		     format_tcp_sacks, tc->snd_sacks);
+
+  TCP_TEST ((vec_len (tc->snd_sacks) == 3), "sack blocks %d expected %d",
+	    vec_len (tc->snd_sacks), 3);
+  TCP_TEST ((tc->snd_sacks[0].start == 700),
+	    "first sack block start %u expected %u", tc->snd_sacks[0].start,
+	    1100);
+
+  /*
+   * Advance rcv_nxt to overlap all
+   */
+  tc->rcv_nxt = 1200;
+  tcp_update_sack_list (tc, 1200, 1200);
+  if (verbose)
+    vlib_cli_output (vm, "advance rcv_nxt to 1200\n%U",
+		     format_tcp_sacks, tc->snd_sacks);
+  TCP_TEST ((vec_len (tc->snd_sacks) == 0), "sack blocks %d expected %d",
+	    vec_len (tc->snd_sacks), 0);
+  return 0;
+}
+
+static int
+tcp_test_sack (vlib_main_t * vm, unformat_input_t * input)
+{
+  int res = 0;
+
+  /* Run all tests */
+  if (unformat_check_input (input) == UNFORMAT_END_OF_INPUT)
+    {
+      if (tcp_test_sack_tx (vm, input))
+	{
+	  return -1;
+	}
+
+      if (tcp_test_sack_rx ())
+	{
+	  return -1;
+	}
+    }
+  else
+    {
+      if (unformat (input, "tx"))
+	{
+	  res = tcp_test_sack_tx (vm, input);
+	}
+      else if (unformat (input, "rx"))
+	{
+	  res = tcp_test_sack_rx ();
+	}
+    }
+
+  return res;
+}
+
+
 typedef struct
 {
   u32 offset;
@@ -967,7 +1106,7 @@ tcp_test (vlib_main_t * vm,
     {
       if (unformat (input, "sack"))
 	{
-	  res = tcp_test_sack ();
+	  res = tcp_test_sack (vm, input);
 	}
       else if (unformat (input, "fifo"))
 	{