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
--- a/src/vnet/sctp/sctp.c
+++ b/src/vnet/sctp/sctp.c
@@ -615,14 +615,21 @@
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
--- a/src/vnet/sctp/sctp.h
+++ b/src/vnet/sctp/sctp.h
@@ -93,7 +93,9 @@
{
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 @@
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 @@
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 @@
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
--- a/src/vnet/sctp/sctp_input.c
+++ b/src/vnet/sctp/sctp_input.c
@@ -375,7 +375,6 @@
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 @@
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 @@
return 0;
}
+ sctp_conn->ack_state += 1;
if (sctp_conn->ack_state >= MAX_ENQUEABLE_SACKS)
{
SCTP_CONN_TRACKING_DBG
@@ -741,8 +741,6 @@
return 0;
}
- sctp_conn->ack_state += 1;
-
return 1;
}
@@ -826,12 +824,15 @@
}
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 @@
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_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 @@
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
--- a/src/vnet/sctp/sctp_output.c
+++ b/src/vnet/sctp/sctp_output.c
@@ -463,6 +463,8 @@
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_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;