SCTP: cumulative SACK fix

A bug was found affecting the cumulative sending of SACK messages.
Because the next0 was never assigned to the next_output the SACK message
was never leaving the peer.
Further, two new flags have been added to distinguish when a peer is
AWAITING a SACK message (e.g. DATA is inflight and waiting to be
acknowledged).

Change-Id: Ibb5a98f7e5fed15cdc76710b74195cac031d59ed
Signed-off-by: Marco Varlese <marco.varlese@suse.com>

diff --git a/src/vnet/sctp/sctp.c b/src/vnet/sctp/sctp.c
index 42cc2ac..76a1bf4 100644 (file)
--- a/src/vnet/sctp/sctp.c
+++ b/src/vnet/sctp/sctp.c
@@ -615,14 +615,21 @@ sctp_session_send_mss (transport_connection_t * trans_conn)
   update_cwnd (sctp_conn);
   update_smallest_pmtu_idx (sctp_conn);
 
-  return sctp_conn->sub_conn[sctp_conn->smallest_PMTU_idx].cwnd;
+  u8 idx = sctp_data_subconn_select (sctp_conn);
+
+  return sctp_conn->sub_conn[idx].cwnd;
 }
 
 u16
 sctp_snd_space (sctp_connection_t * sctp_conn)
 {
+  /* RFC 4096 Section 6.1; point (A) */
+  if (sctp_conn->peer_rwnd == 0)
+    return 0;
+
+  u8 idx = sctp_data_subconn_select (sctp_conn);
   /* Finally, let's subtract the DATA chunk headers overhead */
-  return sctp_conn->sub_conn[sctp_conn->smallest_PMTU_idx].cwnd -
+  return sctp_conn->sub_conn[idx].cwnd -
     sizeof (sctp_payload_data_chunk_t) - sizeof (sctp_full_hdr_t);
 }
 

diff --git a/src/vnet/sctp/sctp.h b/src/vnet/sctp/sctp.h
index 32d3ab9..f0ce594 100644 (file)
--- a/src/vnet/sctp/sctp.h
+++ b/src/vnet/sctp/sctp.h
@@ -93,7 +93,9 @@ enum _sctp_subconn_state
 {
   SCTP_SUBCONN_STATE_DOWN = 0,
   SCTP_SUBCONN_STATE_UP,
-  SCTP_SUBCONN_STATE_ALLOW_HB
+  SCTP_SUBCONN_STATE_ALLOW_HB,
+  SCTP_SUBCONN_AWAITING_SACK,
+  SCTP_SUBCONN_SACK_RECEIVED
 };
 
 #define SCTP_INITIAL_SSHTRESH 65535
@@ -920,6 +922,8 @@ sctp_in_cong_recovery (sctp_connection_t * sctp_conn, u8 idx)
 always_inline u8
 cwnd_fully_utilized (sctp_connection_t * sctp_conn, u8 idx)
 {
+  if (sctp_conn->sub_conn[idx].cwnd == 0)
+    return 1;
   return 0;
 }
 
@@ -928,6 +932,7 @@ always_inline void
 update_cwnd (sctp_connection_t * sctp_conn)
 {
   u8 i;
+  u32 inflight = sctp_conn->next_tsn - sctp_conn->last_unacked_tsn;
 
   for (i = 0; i < MAX_SCTP_CONNECTIONS; i++)
     {
@@ -960,6 +965,12 @@ update_cwnd (sctp_connection_t * sctp_conn)
          sctp_conn->sub_conn[i].cwnd =
            clib_min (sctp_conn->sub_conn[i].PMTU, 1);
        }
+
+      /* Section 6.1; point (D) */
+      if ((inflight + SCTP_RTO_BURST * sctp_conn->sub_conn[i].PMTU) <
+         sctp_conn->sub_conn[i].cwnd)
+       sctp_conn->sub_conn[i].cwnd =
+         inflight + SCTP_RTO_BURST * sctp_conn->sub_conn[i].PMTU;
     }
 }
 

diff --git a/src/vnet/sctp/sctp_input.c b/src/vnet/sctp/sctp_input.c
index 962534c..5691f0f 100644 (file)
--- a/src/vnet/sctp/sctp_input.c
+++ b/src/vnet/sctp/sctp_input.c
@@ -375,7 +375,6 @@ sctp_handle_init (sctp_header_t * sctp_hdr,
                          sctp_conn->remote_initial_tsn);
 
   sctp_conn->peer_rwnd = clib_net_to_host_u32 (init_chunk->a_rwnd);
-
   /*
    * If the length specified in the INIT message is bigger than the size in bytes of our structure it means that
    * optional parameters have been sent with the INIT chunk and we need to parse them.
@@ -725,7 +724,7 @@ sctp_is_sack_delayable (sctp_connection_t * sctp_conn, u8 idx, u8 is_gapping)
       return 0;
     }
 
-  if (is_gapping != 0)
+  if (is_gapping)
     {
       SCTP_CONN_TRACKING_DBG
        ("gapping != 0: CONN_INDEX = %u, sctp_conn->ack_state = %u",
@@ -733,6 +732,7 @@ sctp_is_sack_delayable (sctp_connection_t * sctp_conn, u8 idx, u8 is_gapping)
       return 0;
     }
 
+  sctp_conn->ack_state += 1;
   if (sctp_conn->ack_state >= MAX_ENQUEABLE_SACKS)
     {
       SCTP_CONN_TRACKING_DBG
@@ -741,8 +741,6 @@ sctp_is_sack_delayable (sctp_connection_t * sctp_conn, u8 idx, u8 is_gapping)
       return 0;
     }
 
-  sctp_conn->ack_state += 1;
-
   return 1;
 }
 
@@ -826,12 +824,15 @@ sctp_handle_data (sctp_payload_data_chunk_t * sctp_data_chunk,
     }
   sctp_conn->last_rcvd_tsn = tsn;
 
-  *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4);
-
   SCTP_ADV_DBG ("POINTER_WITH_DATA = %p", b->data);
 
   if (!sctp_is_sack_delayable (sctp_conn, idx, is_gapping))
-    sctp_prepare_sack_chunk (sctp_conn, idx, b);
+    {
+      *next0 = sctp_next_output (sctp_conn->sub_conn[idx].c_is_ip4);
+      sctp_prepare_sack_chunk (sctp_conn, idx, b);
+    }
+  else
+    *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4);
 
   sctp_conn->sub_conn[idx].enqueue_state = error;
 
@@ -1022,13 +1023,13 @@ sctp46_rcv_phase_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 
                  sctp_connection_timers_init (new_sctp_conn);
 
+                 sctp_init_cwnd (new_sctp_conn);
+
                  error0 =
                    sctp_handle_init_ack (sctp_hdr, sctp_chunk_hdr,
                                          new_sctp_conn, idx, b0,
                                          sctp_implied_length);
 
-                 sctp_init_cwnd (new_sctp_conn);
-
                  if (session_stream_connect_notify
                      (&new_sctp_conn->sub_conn[idx].connection, 0))
                    {
@@ -1505,12 +1506,15 @@ sctp_handle_sack (sctp_selective_ack_chunk_t * sack_chunk,
                  sctp_connection_t * sctp_conn, u8 idx, vlib_buffer_t * b0,
                  u16 * next0)
 {
+
   /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */
   if (sctp_conn->local_tag != sack_chunk->sctp_hdr.verification_tag)
     {
       return SCTP_ERROR_INVALID_TAG;
     }
 
+  sctp_conn->sub_conn[idx].state = SCTP_SUBCONN_SACK_RECEIVED;
+
   sctp_conn->sub_conn[idx].last_seen = sctp_time_now ();
 
   /* Section 7.2.2; point (2) */
@@ -1710,12 +1714,12 @@ sctp46_listen_process_inline (vlib_main_t * vm,
 
              sctp_init_snd_vars (child_conn);
 
+             sctp_init_cwnd (child_conn);
+
              error0 =
                sctp_handle_init (sctp_hdr, sctp_chunk_hdr, child_conn, b0,
                                  sctp_implied_length);
 
-             sctp_init_cwnd (child_conn);
-
              if (error0 == SCTP_ERROR_NONE)
                {
                  if (stream_session_accept

diff --git a/src/vnet/sctp/sctp_output.c b/src/vnet/sctp/sctp_output.c
index a4ba960..9ac3feb 100644 (file)
--- a/src/vnet/sctp/sctp_output.c
+++ b/src/vnet/sctp/sctp_output.c
@@ -463,6 +463,8 @@ sctp_prepare_init_chunk (sctp_connection_t * sctp_conn, u8 idx,
   vnet_sctp_set_chunk_length (&init_chunk->chunk_hdr, chunk_len);
   vnet_sctp_common_hdr_params_host_to_net (&init_chunk->chunk_hdr);
 
+  sctp_init_cwnd (sctp_conn);
+
   init_chunk->a_rwnd = clib_host_to_net_u32 (sctp_conn->sub_conn[idx].cwnd);
   init_chunk->initiate_tag = clib_host_to_net_u32 (random_u32 (&random_seed));
   init_chunk->inboud_streams_count =
@@ -1405,7 +1407,12 @@ sctp_push_hdr_i (sctp_connection_t * sctp_conn, vlib_buffer_t * b,
   SCTP_ADV_DBG_OUTPUT ("POINTER_WITH_DATA = %p, DATA_OFFSET = %u",
                       b->data, b->current_data);
 
-  sctp_conn->last_unacked_tsn = sctp_conn->next_tsn;
+  if (sctp_conn->sub_conn[idx].state != SCTP_SUBCONN_AWAITING_SACK)
+    {
+      sctp_conn->sub_conn[idx].state = SCTP_SUBCONN_AWAITING_SACK;
+      sctp_conn->last_unacked_tsn = sctp_conn->next_tsn;
+    }
+
   sctp_conn->next_tsn += data_len;
 
   u32 inflight = sctp_conn->next_tsn - sctp_conn->last_unacked_tsn;