TCP ooo reception fixes

- Improve svm fifo handling of out-of-order segments
- Ensure tsval_recent is updated only if rcv_las falls withing the
  segments's sequence space
- Avoid directly dropping old ACKs
- Improve debugging

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

10 files changed, 275 insertions, 115 deletions

diff --git a/src/svm/svm_fifo.c b/src/svm/svm_fifo.c
index 8f2ed0c9d95..9b09d0c20f4 100644
--- a/src/svm/svm_fifo.c
+++ b/src/svm/svm_fifo.c
@@ -17,6 +17,8 @@
 
 #define offset_lt(_a, _b) ((i32)((_a)-(_b)) < 0)
 #define offset_leq(_a, _b) ((i32)((_a)-(_b)) <= 0)
+#define offset_gt(_a, _b) ((i32)((_a)-(_b)) > 0)
+#define offset_geq(_a, _b) ((i32)((_a)-(_b)) >= 0)
 
 u8 *
 format_ooo_segment (u8 * s, va_list * args)
@@ -160,14 +162,23 @@ ooo_segment_add (svm_fifo_t * f, u32 offset, u32 length)
 	 && offset_leq (ooo_segment_offset (f, s), offset))
     s = pool_elt_at_index (f->ooo_segments, s->next);
 
+  /* If we have a previous and we overlap it, use it as starting point */
+  prev = ooo_segment_get_prev (f, s);
+  if (prev && offset_leq (offset, ooo_segment_end_offset (f, prev)))
+    {
+      s = prev;
+      prev = ooo_segment_get_prev (f, s);
+      s_sof = ooo_segment_offset (f, s);
+      s_eof = ooo_segment_end_offset (f, s);
+      goto merge;
+    }
+
   s_index = s - f->ooo_segments;
   s_sof = ooo_segment_offset (f, s);
   s_eof = ooo_segment_end_offset (f, s);
-  prev = ooo_segment_get_prev (f, s);
 
   /* No overlap, add before current segment */
-  if (offset_lt (end_offset, s_sof)
-      && (!prev || offset_lt (prev->start + prev->length, offset)))
+  if (offset_lt (end_offset, s_sof))
     {
       new_s = ooo_segment_new (f, offset, length);
       new_index = new_s - f->ooo_segments;
@@ -192,7 +203,7 @@ ooo_segment_add (svm_fifo_t * f, u32 offset, u32 length)
       return;
     }
   /* No overlap, add after current segment */
-  else if (offset_lt (s_eof, offset))
+  else if (offset_gt (offset, s_eof))
     {
       new_s = ooo_segment_new (f, offset, length);
       new_index = new_s - f->ooo_segments;
@@ -218,62 +229,16 @@ ooo_segment_add (svm_fifo_t * f, u32 offset, u32 length)
    * Merge needed
    */
 
+merge:
+
   /* Merge at head */
-  if (offset_leq (offset, s_sof))
+  if (offset_lt (offset, s_sof))
     {
-      /* If we have a previous, check if we overlap */
-      if (s->prev != OOO_SEGMENT_INVALID_INDEX)
-	{
-	  prev = pool_elt_at_index (f->ooo_segments, s->prev);
-
-	  /* New segment merges prev and current. Remove previous and
-	   * update position of current. */
-	  if (offset_leq (offset, ooo_segment_end_offset (f, prev)))
-	    {
-	      s->start = prev->start;
-	      s->length = s_eof - ooo_segment_offset (f, prev);
-	      ooo_segment_del (f, s->prev);
-	    }
-	  else
-	    {
-	      s->start = offset;
-	      s->length = s_eof - ooo_segment_offset (f, s);
-	    }
-	}
-      else
-	{
-	  s->start = offset;
-	  s->length = s_eof - ooo_segment_offset (f, s);
-	}
-
-      /* The new segment's tail may cover multiple smaller ones */
-      if (offset_lt (s_eof, end_offset))
-	{
-	  /* Remove segments completely covered */
-	  it = (s->next != OOO_SEGMENT_INVALID_INDEX) ?
-	    pool_elt_at_index (f->ooo_segments, s->next) : 0;
-	  while (it && offset_lt (ooo_segment_end_offset (f, it), end_offset))
-	    {
-	      next = (it->next != OOO_SEGMENT_INVALID_INDEX) ?
-		pool_elt_at_index (f->ooo_segments, it->next) : 0;
-	      ooo_segment_del (f, it - f->ooo_segments);
-	      it = next;
-	    }
-
-	  /* Update length. Segment's start might have changed. */
-	  s->length = end_offset - ooo_segment_offset (f, s);
-
-	  /* If partial overlap with last, merge */
-	  if (it && offset_lt (ooo_segment_offset (f, it), end_offset))
-	    {
-	      s->length +=
-		it->length - (ooo_segment_offset (f, it) - end_offset);
-	      ooo_segment_del (f, it - f->ooo_segments);
-	    }
-	}
+      s->start = offset;
+      s->length = s_eof - ooo_segment_offset (f, s);
     }
   /* Last but overlapping previous */
-  else if (offset_leq (s_eof, end_offset))
+  else if (offset_gt (end_offset, s_eof))
     {
       s->length = end_offset - ooo_segment_offset (f, s);
     }
@@ -281,8 +246,33 @@ ooo_segment_add (svm_fifo_t * f, u32 offset, u32 length)
   else
     {
       /* Do Nothing */
+      goto done;
+    }
+
+  /* The new segment's tail may cover multiple smaller ones */
+  if (offset_geq (end_offset, s_eof))
+    {
+      /* Remove the completely overlapped segments */
+      it = (s->next != OOO_SEGMENT_INVALID_INDEX) ?
+	pool_elt_at_index (f->ooo_segments, s->next) : 0;
+      while (it && offset_leq (ooo_segment_end_offset (f, it), end_offset))
+	{
+	  next = (it->next != OOO_SEGMENT_INVALID_INDEX) ?
+	    pool_elt_at_index (f->ooo_segments, it->next) : 0;
+	  ooo_segment_del (f, it - f->ooo_segments);
+	  it = next;
+	}
+
+      /* If partial overlap with last, merge */
+      if (it && offset_leq (ooo_segment_offset (f, it), end_offset))
+	{
+	  s->length = ooo_segment_end_offset (f, it) -
+	    ooo_segment_offset (f, s);
+	  ooo_segment_del (f, it - f->ooo_segments);
+	}
     }
 
+done:
   /* Most recently updated segment */
   f->ooos_newest = s - f->ooo_segments;
 }
@@ -296,14 +286,17 @@ ooo_segment_try_collect (svm_fifo_t * f, u32 n_bytes_enqueued)
 {
   ooo_segment_t *s;
   u32 index, bytes = 0, diff;
-  u32 cursize;
+  u32 cursize, norm_start, nitems;
 
   /* current size has not yet been updated */
   cursize = svm_fifo_max_dequeue (f) + n_bytes_enqueued;
+  nitems = f->nitems;
 
   s = pool_elt_at_index (f->ooo_segments, f->ooos_list_head);
 
-  diff = (f->nitems + (i32) (f->tail - s->start)) % f->nitems;
+  norm_start = s->start % nitems;
+  diff = (f->nitems + (i32) (f->tail - norm_start)) % nitems;
+
   if (diff > cursize)
     return 0;
 
@@ -326,7 +319,8 @@ ooo_segment_try_collect (svm_fifo_t * f, u32 n_bytes_enqueued)
       if (s->next != OOO_SEGMENT_INVALID_INDEX)
 	{
 	  s = pool_elt_at_index (f->ooo_segments, s->next);
-	  diff = (f->nitems + (i32) (f->tail - s->start)) % f->nitems;
+	  norm_start = s->start % nitems;
+	  diff = (f->nitems + (i32) (f->tail - norm_start)) % nitems;
 	  ooo_segment_del (f, index);
 	}
       /* End of search */
@@ -340,11 +334,11 @@ ooo_segment_try_collect (svm_fifo_t * f, u32 n_bytes_enqueued)
   /* If tail is adjacent to an ooo segment, 'consume' it */
   if (diff == 0)
     {
-      bytes = ((f->nitems - cursize) >= s->length) ? s->length :
-	f->nitems - cursize;
+      bytes = ((nitems - cursize) >= s->length) ? s->length :
+	nitems - cursize;
 
       f->tail += bytes;
-      f->tail %= f->nitems;
+      f->tail %= nitems;
 
       ooo_segment_del (f, s - f->ooo_segments);
     }
@@ -430,31 +424,22 @@ svm_fifo_enqueue_with_offset_internal (svm_fifo_t * f,
 {
   u32 total_copy_bytes, first_copy_bytes, second_copy_bytes;
   u32 cursize, nitems;
-  u32 normalized_offset;
-  int rv;
-
-  /* Users would do well to avoid this */
-  if (PREDICT_FALSE (f->tail == (offset % f->nitems)))
-    {
-      rv = svm_fifo_enqueue_internal (f, required_bytes, copy_from_here);
-      if (rv > 0)
-	return 0;
-      return -1;
-    }
+  u32 normalized_offset, offset_from_tail;
 
   /* read cursize, which can only increase while we're working */
   cursize = svm_fifo_max_dequeue (f);
   nitems = f->nitems;
+  normalized_offset = offset % nitems;
 
   /* Will this request fit? */
-  if ((required_bytes + (offset - f->tail) % nitems) > (nitems - cursize))
+  offset_from_tail = (nitems + normalized_offset - f->tail) % nitems;
+  if ((required_bytes + offset_from_tail) > (nitems - cursize))
     return -1;
 
   ooo_segment_add (f, offset, required_bytes);
 
   /* Number of bytes we're going to copy */
   total_copy_bytes = required_bytes;
-  normalized_offset = offset % nitems;
 
   /* Number of bytes in first copy segment */
   first_copy_bytes = ((nitems - normalized_offset) < total_copy_bytes)
@@ -631,6 +616,15 @@ svm_fifo_first_ooo_segment (svm_fifo_t * f)
   return pool_elt_at_index (f->ooo_segments, f->ooos_list_head);
 }
 
+/**
+ * Set fifo pointers to requested offset
+ */
+void
+svm_fifo_init_pointers (svm_fifo_t * f, u32 pointer)
+{
+  f->head = f->tail = pointer % f->nitems;
+}
+
 /*
  * fd.io coding-style-patch-verification: ON
  *
diff --git a/src/svm/svm_fifo.h b/src/svm/svm_fifo.h
index d67237c69b0..36158dc5db7 100644
--- a/src/svm/svm_fifo.h
+++ b/src/svm/svm_fifo.h
@@ -119,6 +119,7 @@ int svm_fifo_peek (svm_fifo_t * f, u32 offset, u32 max_bytes, u8 * copy_here);
 int svm_fifo_dequeue_drop (svm_fifo_t * f, u32 max_bytes);
 u32 svm_fifo_number_ooo_segments (svm_fifo_t * f);
 ooo_segment_t *svm_fifo_first_ooo_segment (svm_fifo_t * f);
+void svm_fifo_init_pointers (svm_fifo_t * f, u32 pointer);
 
 format_function_t format_svm_fifo;
 
diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c
index d17c93f8f69..e92bb440601 100644
--- a/src/vnet/session/session.c
+++ b/src/vnet/session/session.c
@@ -609,6 +609,21 @@ session_manager_flush_enqueue_events (u32 thread_index)
   return errors;
 }
 
+/**
+ * Init fifo tail and head pointers
+ *
+ * Useful if transport uses absolute offsets for tracking ooo segments.
+ */
+void
+stream_session_init_fifos_pointers (transport_connection_t * tc,
+				    u32 rx_pointer, u32 tx_pointer)
+{
+  stream_session_t *s;
+  s = stream_session_get (tc->s_index, tc->thread_index);
+  svm_fifo_init_pointers (s->server_rx_fifo, rx_pointer);
+  svm_fifo_init_pointers (s->server_tx_fifo, tx_pointer);
+}
+
 void
 stream_session_connect_notify (transport_connection_t * tc, u8 sst,
 			       u8 is_fail)
diff --git a/src/vnet/session/session.h b/src/vnet/session/session.h
index 8cd72f3577a..f41a8a96eb5 100644
--- a/src/vnet/session/session.h
+++ b/src/vnet/session/session.h
@@ -352,9 +352,10 @@ stream_session_peek_bytes (transport_connection_t * tc, u8 * buffer,
 			   u32 offset, u32 max_bytes);
 u32 stream_session_dequeue_drop (transport_connection_t * tc, u32 max_bytes);
 
-void
-stream_session_connect_notify (transport_connection_t * tc, u8 sst,
-			       u8 is_fail);
+void stream_session_connect_notify (transport_connection_t * tc, u8 sst,
+				    u8 is_fail);
+void stream_session_init_fifos_pointers (transport_connection_t * tc,
+					 u32 rx_pointer, u32 tx_pointer);
 
 void stream_session_accept_notify (transport_connection_t * tc);
 void stream_session_disconnect_notify (transport_connection_t * tc);
diff --git a/src/vnet/tcp/tcp_debug.h b/src/vnet/tcp/tcp_debug.h
index ecbf78874fb..b4497a3b8d8 100755
--- a/src/vnet/tcp/tcp_debug.h
+++ b/src/vnet/tcp/tcp_debug.h
@@ -50,6 +50,7 @@
   _(CC_EVT, "cc event")			\
   _(CC_PACK, "cc partial ack")		\
   _(SEG_INVALID, "invalid segment")	\
+  _(PAWS_FAIL, "failed paws check")	\
   _(ACK_RCV_ERR, "invalid ack")		\
   _(RCV_WND_SHRUNK, "shrunk rcv_wnd")	\
 
@@ -382,6 +383,20 @@ typedef enum _tcp_dbg_evt
   ed->data[4] = _tc->rcv_wnd;						\
 }
 
+#define TCP_EVT_PAWS_FAIL_HANDLER(_tc, _seq, _end, ...)			\
+{									\
+  ELOG_TYPE_DECLARE (_e) =						\
+  {									\
+    .format = "paws fail: seq %u end %u tsval %u tsval_recent %u",	\
+    .format_args = "i4i4i4i4",						\
+  };									\
+  DECLARE_ETD(_tc, _e, 4);						\
+  ed->data[0] = _seq - _tc->irs;					\
+  ed->data[1] = _end - _tc->irs;					\
+  ed->data[2] = _tc->opt.tsval;						\
+  ed->data[3] = _tc->tsval_recent;					\
+}
+
 #define TCP_EVT_ACK_RCV_ERR_HANDLER(_tc, _type, _ack, ...)		\
 {									\
   ELOG_TYPE_DECLARE (_e) =						\
diff --git a/src/vnet/tcp/tcp_error.def b/src/vnet/tcp/tcp_error.def
index 0d75d9751d7..a4e46d64629 100644
--- a/src/vnet/tcp/tcp_error.def
+++ b/src/vnet/tcp/tcp_error.def
@@ -37,4 +37,5 @@ tcp_error (PKTS_SENT, "Packets sent")
 tcp_error (FILTERED_DUPACKS, "Filtered duplicate ACKs")
 tcp_error (RST_SENT, "Resets sent")
 tcp_error (INVALID_CONNECTION, "Invalid connection")
-tcp_error (NO_WND, "No window")
\ No newline at end of file
+tcp_error (NO_WND, "No window")
+tcp_error (CONNECTION_CLOSED, "Connection closed")
\ No newline at end of file
diff --git a/src/vnet/tcp/tcp_format.c b/src/vnet/tcp/tcp_format.c
index 3148fd406c9..4de99235934 100644
--- a/src/vnet/tcp/tcp_format.c
+++ b/src/vnet/tcp/tcp_format.c
@@ -131,11 +131,13 @@ format_tcp_header (u8 * s, va_list * args)
 u8 *
 format_tcp_sacks (u8 * s, va_list * args)
 {
-  sack_block_t *sacks = va_arg (*args, sack_block_t *);
+  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, block->end);
+    s = format (s, " start %u end %u\n", block->start - tc->irs,
+		block->end - tc->irs);
   }
   return s;
 }
diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c
index 3c65a5ea73d..0030cfe24ab 100644
--- a/src/vnet/tcp/tcp_input.c
+++ b/src/vnet/tcp/tcp_input.c
@@ -208,6 +208,15 @@ tcp_options_parse (tcp_header_t * th, tcp_options_t * to)
     }
 }
 
+/**
+ * RFC1323: Check against wrapped sequence numbers (PAWS). If we have
+ * timestamp to echo and it's less than tsval_recent, drop segment
+ * but still send an ACK in order to retain TCP's mechanism for detecting
+ * and recovering from half-open connections
+ *
+ * Or at least that's what the theory says. It seems that this might not work
+ * very well with packet reordering and fast retransmit. XXX
+ */
 always_inline int
 tcp_segment_check_paws (tcp_connection_t * tc)
 {
@@ -216,6 +225,27 @@ tcp_segment_check_paws (tcp_connection_t * tc)
 }
 
 /**
+ * Update tsval recent
+ */
+always_inline void
+tcp_update_timestamp (tcp_connection_t * tc, u32 seq, u32 seq_end)
+{
+  /*
+   * RFC1323: If Last.ACK.sent falls within the range of sequence numbers
+   * of an incoming segment:
+   *    SEG.SEQ <= Last.ACK.sent < SEG.SEQ + SEG.LEN
+   * then the TSval from the segment is copied to TS.Recent;
+   * otherwise, the TSval is ignored.
+   */
+  if (tcp_opts_tstamp (&tc->opt) && tc->tsval_recent
+      && seq_leq (seq, tc->rcv_las) && seq_leq (tc->rcv_las, seq_end))
+    {
+      tc->tsval_recent = tc->opt.tsval;
+      tc->tsval_recent_age = tcp_time_now ();
+    }
+}
+
+/**
  * Validate incoming segment as per RFC793 p. 69 and RFC1323 p. 19
  *
  * It first verifies if segment has a wrapped sequence number (PAWS) and then
@@ -228,21 +258,16 @@ static int
 tcp_segment_validate (vlib_main_t * vm, tcp_connection_t * tc0,
 		      vlib_buffer_t * b0, tcp_header_t * th0, u32 * next0)
 {
-  u8 paws_failed;
-
   if (PREDICT_FALSE (!tcp_ack (th0) && !tcp_rst (th0) && !tcp_syn (th0)))
     return -1;
 
   tcp_options_parse (th0, &tc0->opt);
 
-  /* RFC1323: Check against wrapped sequence numbers (PAWS). If we have
-   * timestamp to echo and it's less than tsval_recent, drop segment
-   * but still send an ACK in order to retain TCP's mechanism for detecting
-   * and recovering from half-open connections */
-  paws_failed = tcp_segment_check_paws (tc0);
-  if (paws_failed)
+  if (tcp_segment_check_paws (tc0))
     {
       clib_warning ("paws failed");
+      TCP_EVT_DBG (TCP_EVT_PAWS_FAIL, tc0, vnet_buffer (b0)->tcp.seq_number,
+		   vnet_buffer (b0)->tcp.seq_end);
 
       /* If it just so happens that a segment updates tsval_recent for a
        * segment over 24 days old, invalidate tsval_recent. */
@@ -251,6 +276,7 @@ tcp_segment_validate (vlib_main_t * vm, tcp_connection_t * tc0,
 	{
 	  /* Age isn't reset until we get a valid tsval (bsd inspired) */
 	  tc0->tsval_recent = 0;
+	  clib_warning ("paws failed - really old segment. REALLY?");
 	}
       else
 	{
@@ -305,12 +331,9 @@ tcp_segment_validate (vlib_main_t * vm, tcp_connection_t * tc0,
       return -1;
     }
 
-  /* If PAWS passed and segment in window, save timestamp */
-  if (!paws_failed)
-    {
-      tc0->tsval_recent = tc0->opt.tsval;
-      tc0->tsval_recent_age = tcp_time_now ();
-    }
+  /* If segment in window, save timestamp */
+  tcp_update_timestamp (tc0, vnet_buffer (b0)->tcp.seq_number,
+			vnet_buffer (b0)->tcp.seq_end);
 
   return 0;
 }
@@ -835,7 +858,8 @@ tcp_rcv_ack (tcp_connection_t * tc, vlib_buffer_t * b,
 	  TCP_EVT_DBG (TCP_EVT_DUPACK_RCVD, tc);
 	  tcp_cc_rcv_dupack (tc, vnet_buffer (b)->tcp.ack_number);
 	}
-      return -1;
+      /* Don't drop yet */
+      return 0;
     }
 
   if (tcp_opts_sack_permitted (&tc->opt))
@@ -932,10 +956,6 @@ tcp_update_sack_list (tcp_connection_t * tc, u32 start, u32 end)
 	{
 	  vec_add1 (new_list, tc->snd_sacks[i]);
 	}
-      else
-	{
-	  clib_warning ("dropped sack blocks");
-	}
     }
 
   ASSERT (vec_len (new_list) <= TCP_MAX_SACK_BLOCKS);
@@ -1011,7 +1031,6 @@ tcp_session_enqueue_ooo (tcp_connection_t * tc, vlib_buffer_t * b,
 			 u16 data_len)
 {
   stream_session_t *s0;
-  u32 offset;
   int rv;
 
   /* Pure ACK. Do nothing */
@@ -1021,12 +1040,11 @@ tcp_session_enqueue_ooo (tcp_connection_t * tc, vlib_buffer_t * b,
     }
 
   s0 = stream_session_get (tc->c_s_index, tc->c_thread_index);
-  offset = vnet_buffer (b)->tcp.seq_number - tc->irs;
 
-  clib_warning ("ooo: offset %d len %d", offset, data_len);
-
-  rv = svm_fifo_enqueue_with_offset (s0->server_rx_fifo, offset, data_len,
-				     vlib_buffer_get_current (b));
+  /* Enqueue out-of-order data with absolute offset */
+  rv = svm_fifo_enqueue_with_offset (s0->server_rx_fifo,
+				     vnet_buffer (b)->tcp.seq_number,
+				     data_len, vlib_buffer_get_current (b));
 
   /* Nothing written */
   if (rv)
@@ -1542,6 +1560,9 @@ tcp46_syn_sent_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	      /* Notify app that we have connection */
 	      stream_session_connect_notify (&new_tc0->connection, sst, 0);
 
+	      stream_session_init_fifos_pointers (&new_tc0->connection,
+						  new_tc0->irs + 1,
+						  new_tc0->iss + 1);
 	      /* Make sure after data segment processing ACK is sent */
 	      new_tc0->flags |= TCP_CONN_SNDACK;
 	    }
@@ -1552,7 +1573,9 @@ tcp46_syn_sent_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 
 	      /* Notify app that we have connection */
 	      stream_session_connect_notify (&new_tc0->connection, sst, 0);
-
+	      stream_session_init_fifos_pointers (&new_tc0->connection,
+						  new_tc0->irs + 1,
+						  new_tc0->iss + 1);
 	      tcp_make_synack (new_tc0, b0);
 	      next0 = tcp_next_output (is_ip4);
 
@@ -2139,6 +2162,10 @@ tcp46_listen_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	  tcp_make_synack (child0, b0);
 	  next0 = tcp_next_output (is_ip4);
 
+	  /* Init fifo pointers after we have iss */
+	  stream_session_init_fifos_pointers (&child0->connection,
+					      child0->irs + 1,
+					      child0->iss + 1);
 	drop:
 	  if (PREDICT_FALSE (b0->flags & VLIB_BUFFER_IS_TRACED))
 	    {
@@ -2474,6 +2501,7 @@ do {                                                       	\
   _(LISTEN, TCP_FLAG_SYN, TCP_INPUT_NEXT_LISTEN, TCP_ERROR_NONE);
   /* ACK for for a SYN-ACK -> tcp-rcv-process. */
   _(SYN_RCVD, TCP_FLAG_ACK, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE);
+  _(SYN_RCVD, TCP_FLAG_RST, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE);
   /* SYN-ACK for a SYN */
   _(SYN_SENT, TCP_FLAG_SYN | TCP_FLAG_ACK, TCP_INPUT_NEXT_SYN_SENT,
     TCP_ERROR_NONE);
@@ -2499,6 +2527,7 @@ do {                                                       	\
   _(FIN_WAIT_2, TCP_FLAG_FIN | TCP_FLAG_ACK, TCP_INPUT_NEXT_RCV_PROCESS,
     TCP_ERROR_NONE);
   _(LAST_ACK, TCP_FLAG_ACK, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE);
+  _(CLOSED, TCP_FLAG_ACK, TCP_INPUT_NEXT_DROP, TCP_ERROR_CONNECTION_CLOSED);
 #undef _
 }
 
diff --git a/src/vnet/tcp/tcp_output.c b/src/vnet/tcp/tcp_output.c
index 4e1a7aa5ef4..a85d30daddb 100644
--- a/src/vnet/tcp/tcp_output.c
+++ b/src/vnet/tcp/tcp_output.c
@@ -359,7 +359,8 @@ tcp_make_established_options (tcp_connection_t * tc, tcp_options_t * opts)
 	{
 	  opts->flags |= TCP_OPTS_FLAG_SACK;
 	  opts->sacks = tc->snd_sacks;
-	  opts->n_sack_blocks = vec_len (tc->snd_sacks);
+	  opts->n_sack_blocks = clib_min (vec_len (tc->snd_sacks),
+					  TCP_OPTS_MAX_SACK_BLOCKS);
 	  len += 2 + TCP_OPTION_LEN_SACK_BLOCK * opts->n_sack_blocks;
 	}
     }
@@ -917,6 +918,7 @@ tcp_push_hdr_i (tcp_connection_t * tc, vlib_buffer_t * b,
   vnet_buffer (b)->tcp.connection_index = tc->c_c_index;
 
   tc->snd_nxt += data_len;
+  tc->rcv_las = tc->rcv_nxt;
 
   /* TODO this is updated in output as well ... */
   if (tc->snd_nxt > tc->snd_una_max)
diff --git a/src/vnet/tcp/tcp_test.c b/src/vnet/tcp/tcp_test.c
index ed0322062bd..a457ac8f7fa 100644
--- a/src/vnet/tcp/tcp_test.c
+++ b/src/vnet/tcp/tcp_test.c
@@ -231,7 +231,7 @@ tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input)
   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);
+		     format_tcp_sacks, tc);
   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),
@@ -244,7 +244,7 @@ tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input)
   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);
+		     format_tcp_sacks, tc);
   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),
@@ -257,7 +257,7 @@ tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input)
   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);
+		     format_tcp_sacks, tc);
 
   TCP_TEST ((vec_len (tc->snd_sacks) == 3), "sack blocks %d expected %d",
 	    vec_len (tc->snd_sacks), 3);
@@ -271,8 +271,7 @@ tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input)
   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);
+    vlib_cli_output (vm, "advance rcv_nxt to 1200\n%U", format_tcp_sacks, tc);
   TCP_TEST ((vec_len (tc->snd_sacks) == 0), "sack blocks %d expected %d",
 	    vec_len (tc->snd_sacks), 0);
   return 0;
@@ -502,7 +501,13 @@ tcp_test_fifo1 (vlib_main_t * vm, unformat_input_t * input)
     {
       offset = (2 * i + 1) * sizeof (u32);
       data = (u8 *) (test_data + (2 * i + 1));
-      rv = svm_fifo_enqueue_with_offset (f, offset, sizeof (u32), data);
+      if (i == 0)
+	{
+	  rv = svm_fifo_enqueue_nowait (f, sizeof (u32), data);
+	  rv = rv > 0 ? 0 : rv;
+	}
+      else
+	rv = svm_fifo_enqueue_with_offset (f, offset, sizeof (u32), data);
       if (verbose)
 	vlib_cli_output (vm, "add [%d] [%d, %d]", 2 * i + 1, offset,
 			 offset + sizeof (u32));
@@ -517,6 +522,26 @@ tcp_test_fifo1 (vlib_main_t * vm, unformat_input_t * input)
     vlib_cli_output (vm, "fifo after odd segs: %U", format_svm_fifo, f, 1);
 
   TCP_TEST ((f->tail == 8), "fifo tail %u", f->tail);
+  TCP_TEST ((svm_fifo_number_ooo_segments (f) == 2),
+	    "number of ooo segments %u", svm_fifo_number_ooo_segments (f));
+
+  /*
+   * Try adding a completely overlapped segment
+   */
+  offset = 3 * sizeof (u32);
+  data = (u8 *) (test_data + 3);
+  rv = svm_fifo_enqueue_with_offset (f, offset, sizeof (u32), data);
+  if (rv)
+    {
+      clib_warning ("enqueue returned %d", rv);
+      goto err;
+    }
+
+  if (verbose)
+    vlib_cli_output (vm, "fifo after overlap seg: %U", format_svm_fifo, f, 1);
+
+  TCP_TEST ((svm_fifo_number_ooo_segments (f) == 2),
+	    "number of ooo segments %u", svm_fifo_number_ooo_segments (f));
 
   /*
    * Make sure format functions are not buggy
@@ -887,7 +912,7 @@ tcp_test_fifo3 (vlib_main_t * vm, unformat_input_t * input)
   f->head = fifo_initial_offset;
   f->tail = fifo_initial_offset;
 
-  for (i = 0; i < vec_len (generate); i++)
+  for (i = !randomize; i < vec_len (generate); i++)
     {
       tp = generate + i;
       svm_fifo_enqueue_with_offset (f, fifo_initial_offset + tp->offset,
@@ -895,6 +920,10 @@ tcp_test_fifo3 (vlib_main_t * vm, unformat_input_t * input)
 				    (u8 *) data_pattern + tp->offset);
     }
 
+  /* Add the first segment in order for non random data */
+  if (!randomize)
+    svm_fifo_enqueue_nowait (f, generate[0].len, (u8 *) data_pattern);
+
   /*
    * Expected result: one big fat chunk at offset 1 if randomize == 1
    */
@@ -965,6 +994,73 @@ tcp_test_fifo3 (vlib_main_t * vm, unformat_input_t * input)
 }
 
 static int
+tcp_test_fifo4 (vlib_main_t * vm, unformat_input_t * input)
+{
+  svm_fifo_t *f;
+  u32 fifo_size = 6 << 10;
+  u32 fifo_initial_offset = 1000000000;
+  u32 test_n_bytes = 5000, j;
+  u8 *test_data = 0, *data_buf = 0;
+  int i, rv, verbose = 0;
+
+  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
+    {
+      if (unformat (input, "verbose"))
+	verbose = 1;
+      else
+	{
+	  clib_error_t *e = clib_error_return
+	    (0, "unknown input `%U'", format_unformat_error, input);
+	  clib_error_report (e);
+	  return -1;
+	}
+    }
+
+  /*
+   * Create a fifo and add segments
+   */
+  f = fifo_prepare (fifo_size);
+
+  /* Set head and tail pointers */
+  fifo_initial_offset = fifo_initial_offset % fifo_size;
+  svm_fifo_init_pointers (f, fifo_initial_offset);
+
+  vec_validate (test_data, test_n_bytes - 1);
+  for (i = 0; i < vec_len (test_data); i++)
+    test_data[i] = i;
+
+  for (i = test_n_bytes - 1; i > 0; i--)
+    {
+      rv = svm_fifo_enqueue_with_offset (f, fifo_initial_offset + i,
+					 sizeof (u8), &test_data[i]);
+      if (verbose)
+	vlib_cli_output (vm, "add [%d] [%d, %d]", i, i, i + sizeof (u8));
+      if (rv)
+	{
+	  clib_warning ("enqueue returned %d", rv);
+	  svm_fifo_free (f);
+	  vec_free (test_data);
+	  return -1;
+	}
+    }
+
+  svm_fifo_enqueue_nowait (f, sizeof (u8), &test_data[0]);
+
+  vec_validate (data_buf, vec_len (test_data));
+
+  svm_fifo_dequeue_nowait (f, vec_len (test_data), data_buf);
+  rv = compare_data (data_buf, test_data, 0, vec_len (test_data), &j);
+  if (rv)
+    vlib_cli_output (vm, "[%d] dequeued %u expected %u", j, data_buf[j],
+		     test_data[j]);
+  TCP_TEST ((rv == 0), "dequeued compared to original returned %d", rv);
+
+  svm_fifo_free (f);
+  vec_free (test_data);
+  return 0;
+}
+
+static int
 tcp_test_fifo (vlib_main_t * vm, unformat_input_t * input)
 {
   int res = 0;
@@ -1028,6 +1124,10 @@ tcp_test_fifo (vlib_main_t * vm, unformat_input_t * input)
 	{
 	  res = tcp_test_fifo1 (vm, input);
 	}
+      else if (unformat (input, "fifo4"))
+	{
+	  res = tcp_test_fifo4 (vm, input);
+	}
     }
 
   return res;