From 6e4d4a3684914d071a9b9249217bb6222aeb1d24 Mon Sep 17 00:00:00 2001 From: Marco Varlese Date: Mon, 12 Mar 2018 12:36:59 +0100 Subject: 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 --- src/vnet/sctp/sctp.c | 11 +++++++++-- src/vnet/sctp/sctp.h | 13 ++++++++++++- src/vnet/sctp/sctp_input.c | 26 +++++++++++++++----------- src/vnet/sctp/sctp_output.c | 9 ++++++++- 4 files changed, 44 insertions(+), 15 deletions(-) diff --git a/src/vnet/sctp/sctp.c b/src/vnet/sctp/sctp.c index 42cc2acae91..76a1bf41eeb 100644 --- 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 32d3ab96862..f0ce5949285 100644 --- 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 962534c1a6b..5691f0f44e6 100644 --- 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 a4ba960789e..9ac3feb8aab 100644 --- 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; -- cgit 1.2.3-korg