tcp: fix fin in syn-rcvd
[vpp.git] / src / vnet / tcp / tcp.c
index 5fadef0..b1ed99a 100644 (file)
@@ -246,7 +246,7 @@ tcp_connection_del (tcp_connection_t * tc)
 }
 
 tcp_connection_t *
-tcp_connection_new (u8 thread_index)
+tcp_connection_alloc (u8 thread_index)
 {
   tcp_main_t *tm = vnet_get_tcp_main ();
   tcp_connection_t *tc;
@@ -258,6 +258,15 @@ tcp_connection_new (u8 thread_index)
   return tc;
 }
 
+void
+tcp_connection_free (tcp_connection_t * tc)
+{
+  tcp_main_t *tm = &tcp_main;
+  pool_put (tm->connections[tc->c_thread_index], tc);
+  if (CLIB_DEBUG > 0)
+    clib_memset (tc, 0xFA, sizeof (*tc));
+}
+
 /** Notify session that connection has been reset.
  *
  * Switch state to closed and wait for session to call cleanup.
@@ -281,21 +290,21 @@ tcp_connection_reset (tcp_connection_t * tc)
       tcp_connection_timers_reset (tc);
       /* Set the cleanup timer, in case the session layer/app don't
        * cleanly close the connection */
-      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
+      tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLOSEWAIT_TIME);
       stream_session_reset_notify (&tc->connection);
       break;
     case TCP_STATE_CLOSE_WAIT:
     case TCP_STATE_FIN_WAIT_1:
     case TCP_STATE_FIN_WAIT_2:
     case TCP_STATE_CLOSING:
-      tc->state = TCP_STATE_CLOSED;
-      TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc);
       tcp_connection_timers_reset (tc);
-      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
+      tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLOSEWAIT_TIME);
       break;
     case TCP_STATE_CLOSED:
       return;
     }
+  tc->state = TCP_STATE_CLOSED;
+  TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc);
 }
 
 /**
@@ -326,7 +335,7 @@ tcp_connection_close (tcp_connection_t * tc)
       tcp_connection_timers_reset (tc);
       tcp_send_fin (tc);
       tc->state = TCP_STATE_FIN_WAIT_1;
-      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
+      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_FINWAIT1_TIME);
       break;
     case TCP_STATE_ESTABLISHED:
       if (!session_tx_fifo_max_dequeue (&tc->connection))
@@ -334,6 +343,9 @@ tcp_connection_close (tcp_connection_t * tc)
       else
        tc->flags |= TCP_CONN_FINPNDG;
       tc->state = TCP_STATE_FIN_WAIT_1;
+      /* Set a timer in case the peer stops responding. Otherwise the
+       * connection will be stuck here forever. */
+      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_FINWAIT1_TIME);
       break;
     case TCP_STATE_CLOSE_WAIT:
       if (!session_tx_fifo_max_dequeue (&tc->connection))
@@ -349,16 +361,21 @@ tcp_connection_close (tcp_connection_t * tc)
     case TCP_STATE_FIN_WAIT_1:
       tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
       break;
+    case TCP_STATE_CLOSED:
+      tcp_connection_timers_reset (tc);
+      break;
     default:
       TCP_DBG ("state: %u", tc->state);
     }
 
   TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc);
 
-  /* If in CLOSED and WAITCLOSE timer is not set, delete connection now */
+  /* If in CLOSED and WAITCLOSE timer is not set, delete connection.
+   * But instead of doing it now wait until next dispatch cycle to give
+   * the session layer a chance to clear unhandled events */
   if (!tcp_timer_is_active (tc, TCP_TIMER_WAITCLOSE)
       && tc->state == TCP_STATE_CLOSED)
-    tcp_connection_del (tc);
+    tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
 }
 
 static void
@@ -448,10 +465,10 @@ tcp_connection_select_lb_bucket (tcp_connection_t * tc, const dpo_id_t * dpo,
       ip6_tcp_hdr_t hdr;
       clib_memset (&hdr, 0, sizeof (hdr));
       hdr.ip.protocol = IP_PROTOCOL_TCP;
-      clib_memcpy (&hdr.ip.src_address, &tc->c_lcl_ip.ip6,
-                  sizeof (ip6_address_t));
-      clib_memcpy (&hdr.ip.dst_address, &tc->c_rmt_ip.ip6,
-                  sizeof (ip6_address_t));
+      clib_memcpy_fast (&hdr.ip.src_address, &tc->c_lcl_ip.ip6,
+                       sizeof (ip6_address_t));
+      clib_memcpy_fast (&hdr.ip.dst_address, &tc->c_rmt_ip.ip6,
+                       sizeof (ip6_address_t));
       hdr.tcp.src_port = tc->c_lcl_port;
       hdr.tcp.dst_port = tc->c_rmt_port;
       hash = ip6_compute_flow_hash (&hdr.ip, lb->lb_hash_config);
@@ -466,7 +483,7 @@ tcp_lookup_rmt_in_fib (tcp_connection_t * tc)
   fib_prefix_t prefix;
   u32 fib_index;
 
-  clib_memcpy (&prefix.fp_addr, &tc->c_rmt_ip, sizeof (prefix.fp_addr));
+  clib_memcpy_fast (&prefix.fp_addr, &tc->c_rmt_ip, sizeof (prefix.fp_addr));
   prefix.fp_proto = tc->c_is_ip4 ? FIB_PROTOCOL_IP4 : FIB_PROTOCOL_IP6;
   prefix.fp_len = tc->c_is_ip4 ? 32 : 128;
   fib_index = fib_table_find (prefix.fp_proto, tc->c_fib_index);
@@ -510,7 +527,7 @@ tcp_connection_fib_attach (tcp_connection_t * tc)
 static void
 tcp_cc_init (tcp_connection_t * tc)
 {
-  tc->cc_algo = tcp_cc_algo_get (TCP_CC_NEWRENO);
+  tc->cc_algo = tcp_cc_algo_get (tcp_main.cc_algo);
   tc->cc_algo->init (tc);
 }
 
@@ -546,22 +563,24 @@ tcp_init_snd_vars (tcp_connection_t * tc)
    * handshake may make it look as if time has flown in the opposite
    * direction for us.
    */
-  tcp_set_time_now (vlib_get_thread_index ());
+  tcp_set_time_now (tcp_get_worker (vlib_get_thread_index ()));
   time_now = tcp_time_now ();
 
   tc->iss = random_u32 (&time_now);
   tc->snd_una = tc->iss;
   tc->snd_nxt = tc->iss + 1;
   tc->snd_una_max = tc->snd_nxt;
+  tc->srtt = 0;
 }
 
 void
 tcp_enable_pacing (tcp_connection_t * tc)
 {
-  u32 max_burst, byte_rate;
-  max_burst = 16 * tc->snd_mss;
+  u32 initial_bucket, byte_rate;
+  initial_bucket = 16 * tc->snd_mss;
   byte_rate = 2 << 16;
-  transport_connection_tx_pacer_init (&tc->connection, byte_rate, max_burst);
+  transport_connection_tx_pacer_init (&tc->connection, byte_rate,
+                                     initial_bucket);
   tc->mrtt_us = (u32) ~ 0;
 }
 
@@ -606,8 +625,8 @@ tcp_alloc_custom_local_endpoint (tcp_main_t * tm, ip46_address_t * lcl_addr,
       index = tm->last_v6_address_rotor++;
       if (tm->last_v6_address_rotor >= vec_len (tm->ip6_src_addresses))
        tm->last_v6_address_rotor = 0;
-      clib_memcpy (&lcl_addr->ip6, &tm->ip6_src_addresses[index],
-                  sizeof (ip6_address_t));
+      clib_memcpy_fast (&lcl_addr->ip6, &tm->ip6_src_addresses[index],
+                       sizeof (ip6_address_t));
     }
   port = transport_alloc_local_port (TRANSPORT_PROTO_TCP, lcl_addr);
   if (port < 1)
@@ -620,7 +639,7 @@ tcp_alloc_custom_local_endpoint (tcp_main_t * tm, ip46_address_t * lcl_addr,
 }
 
 static int
-tcp_connection_open (transport_endpoint_t * rmt)
+tcp_session_open (transport_endpoint_cfg_t * rmt)
 {
   tcp_main_t *tm = vnet_get_tcp_main ();
   tcp_connection_t *tc;
@@ -666,12 +685,6 @@ tcp_connection_open (transport_endpoint_t * rmt)
   return tc->c_c_index;
 }
 
-static int
-tcp_session_open (transport_endpoint_t * tep)
-{
-  return tcp_connection_open (tep);
-}
-
 const char *tcp_dbg_evt_str[] = {
 #define _(sym, str) str,
   foreach_tcp_dbg_evt
@@ -807,9 +820,10 @@ format_tcp_vars (u8 * s, va_list * args)
              tcp_rcv_wnd_available (tc));
   s = format (s, " tsval_recent %u tsval_recent_age %u\n", tc->tsval_recent,
              tcp_time_now () - tc->tsval_recent_age);
-  s = format (s, " rto %u rto_boff %u srtt %u rttvar %u rtt_ts %2.5f ",
-             tc->rto, tc->rto_boff, tc->srtt, tc->rttvar, tc->rtt_ts);
-  s = format (s, "rtt_seq %u\n", tc->rtt_seq - tc->iss);
+  s = format (s, " rto %u rto_boff %u srtt %u us %.3f rttvar %u rtt_ts %x",
+             tc->rto, tc->rto_boff, tc->srtt, tc->mrtt_us * 1000, tc->rttvar,
+             tc->rtt_ts);
+  s = format (s, " rtt_seq %u\n", tc->rtt_seq - tc->iss);
   s = format (s, " cong:   %U", format_tcp_congestion, tc);
 
   if (tc->state >= TCP_STATE_ESTABLISHED)
@@ -977,13 +991,13 @@ format_tcp_scoreboard (u8 * s, va_list * args)
 
   hole = scoreboard_first_hole (sb);
   if (hole)
-    s = format (s, "\n%Uhead %u tail %u %u holes:\n", format_white_space,
-               indent, sb->head, sb->tail, pool_elts (sb->holes));
+    s = format (s, "\n%Uhead %u tail %u %u holes:\n%U", format_white_space,
+               indent, sb->head, sb->tail, pool_elts (sb->holes),
+               format_white_space, indent);
 
   while (hole)
     {
-      s = format (s, "%U%U", format_white_space, indent, format_tcp_sack_hole,
-                 hole, tc);
+      s = format (s, "%U", format_tcp_sack_hole, hole, tc);
       hole = scoreboard_next_hole (sb, hole);
     }
 
@@ -1056,38 +1070,26 @@ tcp_snd_space_inline (tcp_connection_t * tc)
 {
   int snd_space, snt_limited;
 
-  if (PREDICT_TRUE (!tcp_in_fastrecovery (tc)))
-    {
-      snd_space = tcp_available_output_snd_space (tc);
+  if (PREDICT_FALSE (tcp_in_fastrecovery (tc)
+                    || tc->state == TCP_STATE_CLOSED))
+    return 0;
 
-      /* If we haven't gotten dupacks or if we did and have gotten sacked
-       * bytes then we can still send as per Limited Transmit (RFC3042) */
-      if (PREDICT_FALSE (tc->rcv_dupacks != 0
-                        && (tcp_opts_sack_permitted (tc)
-                            && tc->sack_sb.last_sacked_bytes == 0)))
-       {
-         if (tc->rcv_dupacks == 1 && tc->limited_transmit != tc->snd_nxt)
-           tc->limited_transmit = tc->snd_nxt;
-         ASSERT (seq_leq (tc->limited_transmit, tc->snd_nxt));
+  snd_space = tcp_available_output_snd_space (tc);
 
-         snt_limited = tc->snd_nxt - tc->limited_transmit;
-         snd_space = clib_max (2 * tc->snd_mss - snt_limited, 0);
-       }
-      return tcp_round_snd_space (tc, snd_space);
-    }
-
-  /* RFC 5681: When previously unsent data is available and the new value of
-   * cwnd and the receiver's advertised window allow, a TCP SHOULD send 1*SMSS
-   * bytes of previously unsent data. */
-  if (tcp_in_fastrecovery (tc) && !tcp_fastrecovery_sent_1_smss (tc))
+  /* If we haven't gotten dupacks or if we did and have gotten sacked
+   * bytes then we can still send as per Limited Transmit (RFC3042) */
+  if (PREDICT_FALSE (tc->rcv_dupacks != 0
+                    && (tcp_opts_sack_permitted (tc)
+                        && tc->sack_sb.last_sacked_bytes == 0)))
     {
-      if (tcp_available_cc_snd_space (tc) < tc->snd_mss)
-       return 0;
-      tcp_fastrecovery_1_smss_on (tc);
-      return tc->snd_mss;
-    }
+      if (tc->rcv_dupacks == 1 && tc->limited_transmit != tc->snd_nxt)
+       tc->limited_transmit = tc->snd_nxt;
+      ASSERT (seq_leq (tc->limited_transmit, tc->snd_nxt));
 
-  return 0;
+      snt_limited = tc->snd_nxt - tc->limited_transmit;
+      snd_space = clib_max (2 * tc->snd_mss - snt_limited, 0);
+    }
+  return tcp_round_snd_space (tc, snd_space);
 }
 
 u32
@@ -1118,12 +1120,13 @@ tcp_session_tx_fifo_offset (transport_connection_t * trans_conn)
 static void
 tcp_update_time (f64 now, u8 thread_index)
 {
-  tcp_set_time_now (thread_index);
-  tw_timer_expire_timers_16t_2w_512sl (&tcp_main.
-                                      wrk_ctx[thread_index].timer_wheel,
-                                      now);
-  tcp_do_fastretransmits (thread_index);
-  tcp_flush_frames_to_output (thread_index);
+  tcp_worker_ctx_t *wrk = tcp_get_worker (thread_index);
+
+  tcp_set_time_now (wrk);
+  tw_timer_expire_timers_16t_2w_512sl (&wrk->timer_wheel, now);
+  tcp_do_fastretransmits (wrk);
+  tcp_send_acks (wrk);
+  tcp_flush_frames_to_output (wrk);
 }
 
 static u32
@@ -1133,6 +1136,16 @@ tcp_session_push_header (transport_connection_t * tconn, vlib_buffer_t * b)
   return tcp_push_header (tc, b);
 }
 
+static void
+tcp_session_flush_data (transport_connection_t * tconn)
+{
+  tcp_connection_t *tc = (tcp_connection_t *) tconn;
+  if (tc->flags & TCP_CONN_PSH_PENDING)
+    return;
+  tc->flags |= TCP_CONN_PSH_PENDING;
+  tc->psh_seq = tc->snd_una_max + transport_max_tx_dequeue (tconn) - 1;
+}
+
 /* *INDENT-OFF* */
 const static transport_proto_vft_t tcp_proto = {
   .enable = vnet_tcp_enable_disable,
@@ -1149,6 +1162,7 @@ const static transport_proto_vft_t tcp_proto = {
   .send_space = tcp_session_send_space,
   .update_time = tcp_update_time,
   .tx_fifo_offset = tcp_session_tx_fifo_offset,
+  .flush_data = tcp_session_flush_data,
   .format_connection = format_tcp_session,
   .format_listener = format_tcp_listener_session,
   .format_half_open = format_tcp_half_open_session,
@@ -1158,16 +1172,30 @@ const static transport_proto_vft_t tcp_proto = {
 /* *INDENT-ON* */
 
 void
-tcp_update_pacer (tcp_connection_t * tc)
+tcp_connection_tx_pacer_update (tcp_connection_t * tc)
 {
   f64 srtt;
+  u64 rate;
 
   if (!transport_connection_is_tx_paced (&tc->connection))
     return;
 
   srtt = clib_min ((f64) tc->srtt * TCP_TICK, tc->mrtt_us);
-  transport_connection_tx_pacer_update (&tc->connection,
-                                       ((f64) tc->cwnd) / srtt);
+  /* TODO should constrain to interface's max throughput but
+   * we don't have link speeds for sw ifs ..*/
+  rate = tc->cwnd / srtt;
+  transport_connection_tx_pacer_update (&tc->connection, rate);
+}
+
+void
+tcp_connection_tx_pacer_reset (tcp_connection_t * tc, u32 window,
+                              u32 start_bucket)
+{
+  tcp_worker_ctx_t *wrk = tcp_get_worker (tc->c_thread_index);
+  u32 byte_rate = window / ((f64) TCP_TICK * tc->srtt);
+  u64 last_time = wrk->vm->clib_time.last_cpu_time;
+  transport_connection_tx_pacer_reset (&tc->connection, byte_rate,
+                                      start_bucket, last_time);
 }
 
 static void
@@ -1223,13 +1251,8 @@ tcp_timer_waitclose_handler (u32 conn_index)
 
   /* Session didn't come back with a close(). Send FIN either way
    * and switch to LAST_ACK. */
-  if (tc->state == TCP_STATE_CLOSE_WAIT)
+  if (tc->state == TCP_STATE_CLOSE_WAIT && (tc->flags & TCP_CONN_FINPNDG))
     {
-      if (tc->flags & TCP_CONN_FINSNT)
-       {
-         clib_warning ("FIN was sent and still in CLOSE WAIT. Weird!");
-       }
-
       /* Make sure we don't try to send unsent data */
       tcp_connection_timers_reset (tc);
       tcp_cong_recovery_off (tc);
@@ -1243,6 +1266,14 @@ tcp_timer_waitclose_handler (u32 conn_index)
       /* Don't delete the connection yet */
       return;
     }
+  else if (tc->state == TCP_STATE_FIN_WAIT_1)
+    {
+      tcp_connection_timers_reset (tc);
+      tc->state = TCP_STATE_CLOSED;
+      /* Wait for session layer to clean up tx events */
+      tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
+      return;
+    }
 
   tcp_connection_del (tc);
 }
@@ -1296,13 +1327,12 @@ tcp_initialize_timer_wheels (tcp_main_t * tm)
 static clib_error_t *
 tcp_main_enable (vlib_main_t * vm)
 {
-  tcp_main_t *tm = vnet_get_tcp_main ();
   vlib_thread_main_t *vtm = vlib_get_thread_main ();
+  u32 num_threads, n_workers, prealloc_conn_per_wrk;
+  tcp_connection_t *tc __attribute__ ((unused));
+  tcp_main_t *tm = vnet_get_tcp_main ();
   clib_error_t *error = 0;
-  u32 num_threads;
   int thread;
-  tcp_connection_t *tc __attribute__ ((unused));
-  u32 preallocated_connections_per_thread;
 
   if ((error = vlib_call_init_function (vm, ip_main_init)))
     return error;
@@ -1324,27 +1354,32 @@ tcp_main_enable (vlib_main_t * vm)
 
   num_threads = 1 /* main thread */  + vtm->n_threads;
   vec_validate (tm->connections, num_threads - 1);
+  vec_validate (tm->wrk_ctx, num_threads - 1);
+  n_workers = num_threads == 1 ? 1 : vtm->n_threads;
+  prealloc_conn_per_wrk = tm->preallocated_connections / n_workers;
 
-  /*
-   * Preallocate connections. Assume that thread 0 won't
-   * use preallocated threads when running multi-core
-   */
-  if (num_threads == 1)
-    {
-      thread = 0;
-      preallocated_connections_per_thread = tm->preallocated_connections;
-    }
-  else
-    {
-      thread = 1;
-      preallocated_connections_per_thread =
-       tm->preallocated_connections / (num_threads - 1);
-    }
-  for (; thread < num_threads; thread++)
+  for (thread = 0; thread < num_threads; thread++)
     {
-      if (preallocated_connections_per_thread)
-       pool_init_fixed (tm->connections[thread],
-                        preallocated_connections_per_thread);
+      vec_validate (tm->wrk_ctx[thread].pending_fast_rxt, 255);
+      vec_validate (tm->wrk_ctx[thread].ongoing_fast_rxt, 255);
+      vec_validate (tm->wrk_ctx[thread].postponed_fast_rxt, 255);
+      vec_validate (tm->wrk_ctx[thread].pending_deq_acked, 255);
+      vec_validate (tm->wrk_ctx[thread].pending_acks, 255);
+      vec_validate (tm->wrk_ctx[thread].pending_disconnects, 255);
+      vec_reset_length (tm->wrk_ctx[thread].pending_fast_rxt);
+      vec_reset_length (tm->wrk_ctx[thread].ongoing_fast_rxt);
+      vec_reset_length (tm->wrk_ctx[thread].postponed_fast_rxt);
+      vec_reset_length (tm->wrk_ctx[thread].pending_deq_acked);
+      vec_reset_length (tm->wrk_ctx[thread].pending_acks);
+      vec_reset_length (tm->wrk_ctx[thread].pending_disconnects);
+      tm->wrk_ctx[thread].vm = vlib_mains[thread];
+
+      /*
+       * Preallocate connections. Assume that thread 0 won't
+       * use preallocated threads when running multi-core
+       */
+      if ((thread > 0 || num_threads == 1) && prealloc_conn_per_wrk)
+       pool_init_fixed (tm->connections[thread], prealloc_conn_per_wrk);
     }
 
   /*
@@ -1364,7 +1399,6 @@ tcp_main_enable (vlib_main_t * vm)
       clib_spinlock_init (&tm->half_open_lock);
     }
 
-  vec_validate (tm->wrk_ctx, num_threads - 1);
   tcp_initialize_timer_wheels (tm);
 
   tm->bytes_per_buffer = vlib_buffer_free_list_buffer_size
@@ -1425,11 +1459,51 @@ tcp_init (vlib_main_t * vm)
                               FIB_PROTOCOL_IP6, tcp6_output_node.index);
 
   tcp_api_reference ();
+  tm->tx_pacing = 1;
+  tm->cc_algo = TCP_CC_NEWRENO;
   return 0;
 }
 
 VLIB_INIT_FUNCTION (tcp_init);
 
+uword
+unformat_tcp_cc_algo (unformat_input_t * input, va_list * va)
+{
+  uword *result = va_arg (*va, uword *);
+
+  if (unformat (input, "newreno"))
+    *result = TCP_CC_NEWRENO;
+  else if (unformat (input, "cubic"))
+    *result = TCP_CC_CUBIC;
+  else
+    return 0;
+
+  return 1;
+}
+
+uword
+unformat_tcp_cc_algo_cfg (unformat_input_t * input, va_list * va)
+{
+  tcp_main_t *tm = vnet_get_tcp_main ();
+  tcp_cc_algorithm_t *cc_alg;
+  unformat_input_t sub_input;
+  int found = 0;
+
+  vec_foreach (cc_alg, tm->cc_algos)
+  {
+    if (!unformat (input, cc_alg->name))
+      continue;
+
+    if (cc_alg->unformat_cfg
+       && unformat (input, "%U", unformat_vlib_cli_sub_input, &sub_input))
+      {
+       if (cc_alg->unformat_cfg (&sub_input))
+         found = 1;
+      }
+  }
+  return found;
+}
+
 static clib_error_t *
 tcp_config_fn (vlib_main_t * vm, unformat_input_t * input)
 {
@@ -1449,8 +1523,13 @@ tcp_config_fn (vlib_main_t * vm, unformat_input_t * input)
       else if (unformat (input, "max-rx-fifo %U", unformat_memory_size,
                         &tm->max_rx_fifo))
        ;
-      else if (unformat (input, "tx-pacing"))
-       tm->tx_pacing = 1;
+      else if (unformat (input, "no-tx-pacing"))
+       tm->tx_pacing = 0;
+      else if (unformat (input, "cc-algo %U", unformat_tcp_cc_algo,
+                        &tm->cc_algo))
+       ;
+      else if (unformat (input, "%U", unformat_tcp_cc_algo_cfg))
+       ;
       else
        return clib_error_return (0, "unknown input `%U'",
                                  format_unformat_error, input);
@@ -1795,6 +1874,12 @@ tcp_scoreboard_replay (u8 * s, tcp_connection_t * tc, u8 verbose)
   tcp_connection_t _dummy_tc, *dummy_tc = &_dummy_tc;
   sack_block_t *block;
 
+  if (!TCP_SCOREBOARD_TRACE)
+    {
+      s = format (s, "scoreboard tracing not enabled");
+      return s;
+    }
+
   if (!tc)
     return s;
 
@@ -1803,13 +1888,7 @@ tcp_scoreboard_replay (u8 * s, tcp_connection_t * tc, u8 verbose)
   scoreboard_init (&dummy_tc->sack_sb);
   dummy_tc->rcv_opts.flags |= TCP_OPTS_FLAG_SACK;
 
-/* Since this is also accessible via decl. in tcp.h.
- * Otherwise, it is gated earlier by cli parser.
- */
-#if (!TCP_SCOREBOARD_TRACE)
-  s = format (0, "scoreboard tracing not enabled");
-  return s;
-#else
+#if TCP_SCOREBOARD_TRACE
   trace = tc->sack_sb.trace;
   trace_len = vec_len (tc->sack_sb.trace);
 #endif