Improve sack bytes accounting and testing

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

diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c
index e365fa0..36d85e4 100644 (file)
--- a/src/vnet/tcp/tcp.c
+++ b/src/vnet/tcp/tcp.c
@@ -565,6 +565,48 @@ format_tcp_half_open_session (u8 * s, va_list * args)
   return format (s, "%U", format_tcp_connection, tc);
 }
 
+u8 *
+format_tcp_sacks (u8 * s, va_list * args)
+{
+  tcp_connection_t *tc = va_arg (*args, tcp_connection_t *);
+  sack_block_t *sacks = tc->snd_sacks;
+  sack_block_t *block;
+  vec_foreach (block, sacks)
+  {
+    s = format (s, " start %u end %u\n", block->start - tc->irs,
+               block->end - tc->irs);
+  }
+  return s;
+}
+
+u8 *
+format_tcp_sack_hole (u8 * s, va_list * args)
+{
+  sack_scoreboard_hole_t *hole = va_arg (*args, sack_scoreboard_hole_t *);
+  s = format (s, "[%u, %u]", hole->start, hole->end);
+  return s;
+}
+
+u8 *
+format_tcp_scoreboard (u8 * s, va_list * args)
+{
+  sack_scoreboard_t *sb = va_arg (*args, sack_scoreboard_t *);
+  sack_scoreboard_hole_t *hole;
+  s = format (s, "head %u tail %u snd_una_adv %u\n", sb->head, sb->tail,
+             sb->snd_una_adv);
+  s = format (s, "sacked_bytes %u last_sacked_bytes %u", sb->sacked_bytes,
+             sb->last_sacked_bytes);
+  s = format (s, " max_byte_sacked %u\n", sb->max_byte_sacked);
+  s = format (s, "holes:\n");
+  hole = scoreboard_first_hole (sb);
+  while (hole)
+    {
+      s = format (s, "%U", format_tcp_sack_hole, hole);
+      hole = scoreboard_next_hole (sb, hole);
+    }
+  return s;
+}
+
 transport_connection_t *
 tcp_session_get_transport (u32 conn_index, u32 thread_index)
 {

diff --git a/src/vnet/tcp/tcp.h b/src/vnet/tcp/tcp.h
index 8212ada..8d24a70 100644 (file)
--- a/src/vnet/tcp/tcp.h
+++ b/src/vnet/tcp/tcp.h
@@ -389,6 +389,7 @@ void tcp_connection_reset (tcp_connection_t * tc);
 
 u8 *format_tcp_connection (u8 * s, va_list * args);
 u8 *format_tcp_connection_verbose (u8 * s, va_list * args);
+u8 *format_tcp_scoreboard (u8 * s, va_list * args);
 
 always_inline tcp_connection_t *
 tcp_listener_get (u32 tli)

diff --git a/src/vnet/tcp/tcp_format.c b/src/vnet/tcp/tcp_format.c
index 4de9923..1ca2f58 100644 (file)
--- a/src/vnet/tcp/tcp_format.c
+++ b/src/vnet/tcp/tcp_format.c
@@ -128,20 +128,6 @@ format_tcp_header (u8 * s, va_list * args)
   return s;
 }
 
-u8 *
-format_tcp_sacks (u8 * s, va_list * args)
-{
-  tcp_connection_t *tc = va_arg (*args, tcp_connection_t *);
-  sack_block_t *sacks = tc->snd_sacks;
-  sack_block_t *block;
-  vec_foreach (block, sacks)
-  {
-    s = format (s, " start %u end %u\n", block->start - tc->irs,
-               block->end - tc->irs);
-  }
-  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 ddee41e..9d3f4cc 100644 (file)
--- a/src/vnet/tcp/tcp_input.c
+++ b/src/vnet/tcp/tcp_input.c
@@ -533,12 +533,13 @@ tcp_rcv_sacks (tcp_connection_t * tc, u32 ack)
   sack_scoreboard_t *sb = &tc->sack_sb;
   sack_block_t *blk, tmp;
   sack_scoreboard_hole_t *hole, *next_hole, *last_hole, *new_hole;
-  u32 blk_index = 0, old_sacked_bytes, hole_index;
+  u32 blk_index = 0, old_sacked_bytes, delivered_bytes, hole_index;
   int i, j;
 
   sb->last_sacked_bytes = 0;
   sb->snd_una_adv = 0;
   old_sacked_bytes = sb->sacked_bytes;
+  delivered_bytes = 0;
 
   if (!tcp_opts_sack (&tc->opt) && sb->head == TCP_INVALID_SACK_HOLE_INDEX)
     return;
@@ -584,6 +585,8 @@ tcp_rcv_sacks (tcp_connection_t * tc, u32 ack)
       last_hole = scoreboard_insert_hole (sb, TCP_INVALID_SACK_HOLE_INDEX,
                                          tc->snd_una, tc->snd_una_max);
       sb->tail = scoreboard_hole_index (sb, last_hole);
+      tmp = tc->opt.sacks[vec_len (tc->opt.sacks) - 1];
+      sb->max_byte_sacked = tmp.end;
     }
   else
     {
@@ -614,37 +617,43 @@ tcp_rcv_sacks (tcp_connection_t * tc, u32 ack)
                {
                  /* Bytes lost because snd_wnd left edge advances */
                  if (next_hole && seq_leq (next_hole->start, ack))
-                   sb->sacked_bytes -= next_hole->start - hole->end;
+                   delivered_bytes += next_hole->start - hole->end;
                  else
-                   sb->sacked_bytes -= ack - hole->end;
+                   delivered_bytes += ack - hole->end;
                }
              else
                {
                  sb->sacked_bytes += scoreboard_hole_bytes (hole);
                }
 
-             /* snd_una needs to be advanced */
-             if (seq_geq (ack, hole->end))
-               {
-                 if (next_hole && seq_lt (ack, next_hole->start))
-                   sb->snd_una_adv = next_hole->start - ack;
-                 else
-                   sb->snd_una_adv = sb->max_byte_sacked - ack;
-
-                 /* all these can be delivered */
-                 sb->sacked_bytes -= sb->snd_una_adv;
-               }
-
              /* About to remove last hole */
              if (hole == last_hole)
                {
                  sb->tail = hole->prev;
                  last_hole = scoreboard_last_hole (sb);
-                 /* keep track of max byte sacked in case the last hole
+                 /* keep track of max byte sacked for when the last hole
                   * is acked */
                  if (seq_gt (hole->end, sb->max_byte_sacked))
                    sb->max_byte_sacked = hole->end;
                }
+
+             /* snd_una needs to be advanced */
+             if (blk->end == ack && seq_geq (ack, hole->end))
+               {
+                 if (next_hole && seq_lt (ack, next_hole->start))
+                   {
+                     sb->snd_una_adv = next_hole->start - ack;
+
+                     /* all these can be delivered */
+                     delivered_bytes += sb->snd_una_adv;
+                   }
+                 else if (!next_hole)
+                   {
+                     sb->snd_una_adv = sb->max_byte_sacked - ack;
+                     delivered_bytes += sb->snd_una_adv;
+                   }
+               }
+
              scoreboard_remove_hole (sb, hole);
              hole = next_hole;
            }
@@ -693,8 +702,8 @@ tcp_rcv_sacks (tcp_connection_t * tc, u32 ack)
        }
     }
 
-  sb->last_sacked_bytes = sb->sacked_bytes + sb->snd_una_adv
-    - old_sacked_bytes;
+  sb->last_sacked_bytes = sb->sacked_bytes - old_sacked_bytes;
+  sb->sacked_bytes -= delivered_bytes;
 }
 
 /** Update snd_wnd

diff --git a/src/vnet/tcp/tcp_test.c b/src/vnet/tcp/tcp_test.c
index a457ac8..2af3848 100644 (file)
--- a/src/vnet/tcp/tcp_test.c
+++ b/src/vnet/tcp/tcp_test.c
@@ -35,13 +35,19 @@
 }
 
 static int
-tcp_test_sack_rx ()
+tcp_test_sack_rx (vlib_main_t * vm, unformat_input_t * input)
 {
   tcp_connection_t _tc, *tc = &_tc;
   sack_scoreboard_t *sb = &tc->sack_sb;
   sack_block_t *sacks = 0, block;
   sack_scoreboard_hole_t *hole;
-  int i;
+  int i, verbose = 0;
+
+  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
+    {
+      if (unformat (input, "verbose"))
+       verbose = 1;
+    }
 
   memset (tc, 0, sizeof (*tc));
 
@@ -69,6 +75,10 @@ tcp_test_sack_rx ()
   tc->opt.n_sack_blocks = vec_len (tc->opt.sacks);
   tcp_rcv_sacks (tc, 0);
 
+  if (verbose)
+    vlib_cli_output (vm, "sb after even blocks:\n%U", format_tcp_scoreboard,
+                    sb);
+
   TCP_TEST ((pool_elts (sb->holes) == 5),
            "scoreboard has %d elements", pool_elts (sb->holes));
 
@@ -83,7 +93,8 @@ tcp_test_sack_rx ()
   TCP_TEST ((sb->snd_una_adv == 0), "snd_una_adv %u", sb->snd_una_adv);
   TCP_TEST ((sb->last_sacked_bytes == 400),
            "last sacked bytes %d", sb->last_sacked_bytes);
-
+  TCP_TEST ((sb->max_byte_sacked == 900),
+           "max byte sacked %u", sb->max_byte_sacked);
   /*
    * Inject odd blocks
    */
@@ -96,6 +107,10 @@ tcp_test_sack_rx ()
   tc->opt.n_sack_blocks = vec_len (tc->opt.sacks);
   tcp_rcv_sacks (tc, 0);
 
+  if (verbose)
+    vlib_cli_output (vm, "sb after odd blocks:\n%U", format_tcp_scoreboard,
+                    sb);
+
   hole = scoreboard_first_hole (sb);
   TCP_TEST ((pool_elts (sb->holes) == 1),
            "scoreboard has %d holes", pool_elts (sb->holes));
@@ -112,6 +127,9 @@ tcp_test_sack_rx ()
    *  Ack until byte 100, all bytes are now acked + sacked
    */
   tcp_rcv_sacks (tc, 100);
+  if (verbose)
+    vlib_cli_output (vm, "ack until byte 100:\n%U", format_tcp_scoreboard,
+                    sb);
 
   TCP_TEST ((pool_elts (sb->holes) == 0),
            "scoreboard has %d elements", pool_elts (sb->holes));
@@ -133,11 +151,17 @@ tcp_test_sack_rx ()
   block.end = 1300;
   vec_add1 (tc->opt.sacks, block);
 
+  if (verbose)
+    vlib_cli_output (vm, "add [1200, 1300]:\n%U", format_tcp_scoreboard, sb);
   tc->snd_una_max = 1500;
   tc->snd_una = 1000;
   tc->snd_nxt = 1500;
   tcp_rcv_sacks (tc, 1000);
 
+  if (verbose)
+    vlib_cli_output (vm, "sb snd_una_max 1500, snd_una 1000:\n%U",
+                    format_tcp_scoreboard, sb);
+
   TCP_TEST ((sb->snd_una_adv == 0),
            "snd_una_adv after ack %u", sb->snd_una_adv);
   TCP_TEST ((pool_elts (sb->holes) == 2),
@@ -145,6 +169,10 @@ tcp_test_sack_rx ()
   hole = scoreboard_first_hole (sb);
   TCP_TEST ((hole->start == 1000 && hole->end == 1200),
            "first hole start %u end %u", hole->start, hole->end);
+  TCP_TEST ((sb->snd_una_adv == 0),
+           "snd_una_adv after ack %u", sb->snd_una_adv);
+  TCP_TEST ((sb->max_byte_sacked == 1300),
+           "max sacked byte %u", sb->max_byte_sacked);
   hole = scoreboard_last_hole (sb);
   TCP_TEST ((hole->start == 1300 && hole->end == 1500),
            "last hole start %u end %u", hole->start, hole->end);
@@ -157,6 +185,10 @@ tcp_test_sack_rx ()
   vec_reset_length (tc->opt.sacks);
   tcp_rcv_sacks (tc, 1200);
 
+  if (verbose)
+    vlib_cli_output (vm, "sb ack up to byte 1200:\n%U", format_tcp_scoreboard,
+                    sb);
+
   TCP_TEST ((sb->snd_una_adv == 100),
            "snd_una_adv after ack %u", sb->snd_una_adv);
   TCP_TEST ((sb->sacked_bytes == 0), "sacked bytes %d", sb->sacked_bytes);
@@ -168,8 +200,41 @@ tcp_test_sack_rx ()
    */
 
   scoreboard_clear (sb);
+  if (verbose)
+    vlib_cli_output (vm, "sb cleared all:\n%U", format_tcp_scoreboard, sb);
+
   TCP_TEST ((pool_elts (sb->holes) == 0),
            "number of holes %d", pool_elts (sb->holes));
+  /*
+   * Re-inject odd blocks and ack them all
+   */
+
+  tc->snd_una = 0;
+  tc->snd_una_max = 1000;
+  tc->snd_nxt = 1000;
+  for (i = 0; i < 5; i++)
+    {
+      vec_add1 (tc->opt.sacks, sacks[i * 2 + 1]);
+    }
+  tc->opt.n_sack_blocks = vec_len (tc->opt.sacks);
+  tcp_rcv_sacks (tc, 0);
+  if (verbose)
+    vlib_cli_output (vm, "sb added odd blocks and ack [0, 950]:\n%U",
+                    format_tcp_scoreboard, sb);
+
+  tcp_rcv_sacks (tc, 950);
+
+  if (verbose)
+    vlib_cli_output (vm, "sb added odd blocks and ack [0, 950]:\n%U",
+                    format_tcp_scoreboard, sb);
+
+  TCP_TEST ((pool_elts (sb->holes) == 0),
+           "scoreboard has %d elements", pool_elts (sb->holes));
+  TCP_TEST ((sb->snd_una_adv == 50), "snd_una_adv %u", sb->snd_una_adv);
+  TCP_TEST ((sb->sacked_bytes == 0), "sacked bytes %d", sb->sacked_bytes);
+  TCP_TEST ((sb->last_sacked_bytes == 0),
+           "last sacked bytes %d", sb->last_sacked_bytes);
+
   return 0;
 }
 
@@ -290,7 +355,7 @@ tcp_test_sack (vlib_main_t * vm, unformat_input_t * input)
          return -1;
        }
 
-      if (tcp_test_sack_rx ())
+      if (tcp_test_sack_rx (vm, input))
        {
          return -1;
        }
@@ -303,7 +368,7 @@ tcp_test_sack (vlib_main_t * vm, unformat_input_t * input)
        }
       else if (unformat (input, "rx"))
        {
-         res = tcp_test_sack_rx ();
+         res = tcp_test_sack_rx (vm, input);
        }
     }