From 157e626cf7e12fd41f2972e45e87818a5ee30ef1 Mon Sep 17 00:00:00 2001 From: huitema <huitema@huitema.net> Date: Fri, 12 Oct 2018 18:44:00 -0700 Subject: [PATCH] Add API for multiple spin bit variants --- CMakeLists.txt | 1 + picoquic/packet.c | 4 ++ picoquic/picoquic.h | 44 ++++++------- picoquic/picoquic.vcxproj | 1 + picoquic/picoquic.vcxproj.filters | 3 + picoquic/picoquic_internal.h | 73 ++++++++++++++------- picoquic/quicctx.c | 12 ++-- picoquic/sender.c | 5 ++ picoquic/spinbit.c | 104 ++++++++++++++++++++++++++++++ picoquictest/sacktest.c | 6 +- picoquictest/tls_api_test.c | 2 +- 11 files changed, 200 insertions(+), 55 deletions(-) create mode 100644 picoquic/spinbit.c diff --git a/CMakeLists.txt b/CMakeLists.txt index cdd8f60e..f6d216be 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,6 +30,7 @@ SET(PICOQUIC_LIBRARY_FILES picoquic/quicctx.c picoquic/sacks.c picoquic/sender.c + picoquic/spinbit.c picoquic/ticket_store.c picoquic/tls_api.c picoquic/transport.c diff --git a/picoquic/packet.c b/picoquic/packet.c index 84ff96d0..eabc5c82 100644 --- a/picoquic/packet.c +++ b/picoquic/packet.c @@ -1424,6 +1424,7 @@ int picoquic_incoming_encrypted( if (ret == 0) { picoquic_path_t * path_x = cnx->path[path_id]; +#if 0 /* Process the spin bit on this path */ if (ph->pn64 > cnx->pkt_ctx[pc].first_sack_item.end_of_sack_range) { path_x->current_spin = ph->spin ^ cnx->client_mode; @@ -1436,6 +1437,9 @@ int picoquic_incoming_encrypted( path_x->spin_last_trigger = picoquic_get_quic_time(cnx->quic); } } +#else + picoquic_spin_function_table[picoquic_supported_versions[cnx->version_index].spinbit_version].spinbit_incoming(cnx, path_x, ph); +#endif /* Accept the incoming frames */ ret = picoquic_decode_frames(cnx, cnx->path[path_id], bytes + ph->offset, ph->payload_length, ph->epoch, current_time); diff --git a/picoquic/picoquic.h b/picoquic/picoquic.h index 282d0789..e870af5d 100644 --- a/picoquic/picoquic.h +++ b/picoquic/picoquic.h @@ -134,6 +134,28 @@ typedef enum { picoquic_context_server_busy = 8 } picoquic_context_flags; +/* +* Nominal packet types. These are the packet types used internally by the +* implementation. The wire encoding depends on the version. +*/ +typedef enum { + picoquic_packet_error = 0, + picoquic_packet_version_negotiation, + picoquic_packet_initial, + picoquic_packet_retry, + picoquic_packet_handshake, + picoquic_packet_0rtt_protected, + picoquic_packet_1rtt_protected, + picoquic_packet_type_max +} picoquic_packet_type_enum; + +typedef enum { + picoquic_packet_context_application = 0, + picoquic_packet_context_handshake = 1, + picoquic_packet_context_initial = 2, + picoquic_nb_packet_context = 3 +} picoquic_packet_context_enum; + /* * Provisional definition of the connection ID. */ @@ -163,28 +185,6 @@ typedef struct st_picoquic_stateless_packet_t { uint8_t bytes[PICOQUIC_MAX_PACKET_SIZE]; } picoquic_stateless_packet_t; -/* -* Nominal packet types. These are the packet types used internally by the -* implementation. The wire encoding depends on the version. -*/ -typedef enum { - picoquic_packet_error = 0, - picoquic_packet_version_negotiation, - picoquic_packet_initial, - picoquic_packet_retry, - picoquic_packet_handshake, - picoquic_packet_0rtt_protected, - picoquic_packet_1rtt_protected, - picoquic_packet_type_max -} picoquic_packet_type_enum; - -typedef enum { - picoquic_packet_context_application = 0, - picoquic_packet_context_handshake = 1, - picoquic_packet_context_initial = 2, - picoquic_nb_packet_context = 3 -} picoquic_packet_context_enum; - /* * The simple packet structure is used to store packets that * have been sent but are not yet acknowledged. diff --git a/picoquic/picoquic.vcxproj b/picoquic/picoquic.vcxproj index aece6a6a..70d3e320 100644 --- a/picoquic/picoquic.vcxproj +++ b/picoquic/picoquic.vcxproj @@ -148,6 +148,7 @@ <ClCompile Include="picohash.c" /> <ClCompile Include="sacks.c" /> <ClCompile Include="sender.c" /> + <ClCompile Include="spinbit.c" /> <ClCompile Include="ticket_store.c" /> <ClCompile Include="tls_api.c" /> <ClCompile Include="transport.c" /> diff --git a/picoquic/picoquic.vcxproj.filters b/picoquic/picoquic.vcxproj.filters index 740662eb..f2e4c9e7 100644 --- a/picoquic/picoquic.vcxproj.filters +++ b/picoquic/picoquic.vcxproj.filters @@ -69,6 +69,9 @@ <ClCompile Include="picosplay.c"> <Filter>Source Files</Filter> </ClCompile> + <ClCompile Include="spinbit.c"> + <Filter>Source Files</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <ClInclude Include="picoquic.h"> diff --git a/picoquic/picoquic_internal.h b/picoquic/picoquic_internal.h index ed58183f..e6688e66 100644 --- a/picoquic/picoquic_internal.h +++ b/picoquic/picoquic_internal.h @@ -95,6 +95,28 @@ typedef enum { picoquic_frame_type_ack_ecn = 0x1b } picoquic_frame_type_enum_t; +typedef struct st_picoquic_packet_header_t { + picoquic_connection_id_t dest_cnx_id; + picoquic_connection_id_t srce_cnx_id; + uint32_t pn; + uint32_t vn; + uint32_t offset; + uint32_t pn_offset; + picoquic_packet_type_enum ptype; + uint64_t pnmask; + uint64_t pn64; + uint16_t payload_length; + int version_index; + int epoch; + picoquic_packet_context_enum pc; + unsigned int key_phase : 1; + unsigned int spin : 1; + unsigned int spin_vec : 2; + unsigned int has_spin_bit : 1; + uint32_t token_length; + uint32_t token_offset; +} picoquic_packet_header; + /* * Efficient range operations that assume range containing bitfields. * Namely, it assumes max&min==min, min&bits==0, max&bits==bits. @@ -121,6 +143,8 @@ typedef enum { #define PICOQUIC_INTEROP_VERSION_INDEX 1 + + /* * Flags used to describe the capabilities of different versions. */ @@ -129,8 +153,30 @@ typedef enum { picoquic_version_no_flag = 0 } picoquic_version_feature_flags; +/* + * Spin bit management + * There is a spinbit version attached to the description of each QUIC version + * The table of functions is defined in the module "spinbit.c" + */ + +typedef enum { + picoquic_spinbit_basic = 0, + picoquic_spinbit_vec = 1, + picoquic_spinbit_null = 2 +} picoquic_spinbit_version_enum; + +typedef void (*picoquic_spinbit_incoming_fn)(picoquic_cnx_t * cnx, picoquic_path_t * path_x, picoquic_packet_header * ph); +typedef uint8_t (*picoquic_spinbit_outgoing_fn)(picoquic_cnx_t * cnx); + +typedef struct st_picoquic_spinbit_def_t { + picoquic_spinbit_incoming_fn spinbit_incoming; + picoquic_spinbit_outgoing_fn spinbit_outgoing; +} picoquic_spinbit_def_t; + +extern picoquic_spinbit_def_t picoquic_spin_function_table[]; + /* - * Codes used for representing the various types of packet encodings + * Codes used for representing the various types of packet encodings. */ typedef enum { picoquic_version_header_13 @@ -138,14 +184,15 @@ typedef enum { typedef struct st_picoquic_version_parameters_t { uint32_t version; - uint32_t version_flags; picoquic_version_header_encoding version_header_encoding; + picoquic_spinbit_version_enum spinbit_version; size_t version_aead_key_length; uint8_t* version_aead_key; } picoquic_version_parameters_t; extern const picoquic_version_parameters_t picoquic_supported_versions[]; extern const size_t picoquic_nb_supported_versions; + int picoquic_get_version_index(uint32_t proposed_version); /* @@ -741,28 +788,6 @@ char* picoquic_string_duplicate(const char* original); /* Packet parsing */ -typedef struct _picoquic_packet_header { - picoquic_connection_id_t dest_cnx_id; - picoquic_connection_id_t srce_cnx_id; - uint32_t pn; - uint32_t vn; - uint32_t offset; - uint32_t pn_offset; - picoquic_packet_type_enum ptype; - uint64_t pnmask; - uint64_t pn64; - uint16_t payload_length; - int version_index; - int epoch; - picoquic_packet_context_enum pc; - unsigned int key_phase : 1; - unsigned int spin : 1; - unsigned int spin_vec : 2; - unsigned int has_spin_bit : 1; - uint32_t token_length; - uint32_t token_offset; -} picoquic_packet_header; - int picoquic_parse_packet_header( picoquic_quic_t* quic, uint8_t* bytes, diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c index c77394bc..cf68eff1 100644 --- a/picoquic/quicctx.c +++ b/picoquic/quicctx.c @@ -143,20 +143,24 @@ static uint8_t picoquic_cleartext_draft_10_salt[] = { /* Support for draft 13! */ const picoquic_version_parameters_t picoquic_supported_versions[] = { - { PICOQUIC_INTERNAL_TEST_VERSION_1, 0, + { PICOQUIC_INTERNAL_TEST_VERSION_1, picoquic_version_header_13, + picoquic_spinbit_vec, sizeof(picoquic_cleartext_internal_test_1_salt), picoquic_cleartext_internal_test_1_salt }, - { PICOQUIC_NINTH_INTEROP_VERSION, 0, + { PICOQUIC_NINTH_INTEROP_VERSION, picoquic_version_header_13, + picoquic_spinbit_vec, sizeof(picoquic_cleartext_draft_10_salt), picoquic_cleartext_draft_10_salt }, - { PICOQUIC_EIGHT_INTEROP_VERSION, 0, + { PICOQUIC_EIGHT_INTEROP_VERSION, picoquic_version_header_13, + picoquic_spinbit_vec, sizeof(picoquic_cleartext_draft_10_salt), picoquic_cleartext_draft_10_salt }, - { PICOQUIC_SEVENTH_INTEROP_VERSION, 0, + { PICOQUIC_SEVENTH_INTEROP_VERSION, picoquic_version_header_13, + picoquic_spinbit_vec, sizeof(picoquic_cleartext_draft_10_salt), picoquic_cleartext_draft_10_salt } }; diff --git a/picoquic/sender.c b/picoquic/sender.c index f5a4bbec..a68f333e 100644 --- a/picoquic/sender.c +++ b/picoquic/sender.c @@ -239,6 +239,7 @@ uint32_t picoquic_create_packet_header( /* Create a short packet -- using 32 bit sequence numbers for now */ uint8_t K = (cnx->key_phase_enc) ? 0x40 : 0; const uint8_t C = 0x30; +#if 0 uint8_t spin_vec = (uint8_t)(cnx->path[0]->spin_vec); uint8_t spin_bit = (uint8_t)((cnx->path[0]->current_spin) << 2); @@ -255,6 +256,10 @@ uint32_t picoquic_create_packet_header( length = 0; bytes[length++] = (K | C | spin_bit | spin_vec); +#else + length = 0; + bytes[length++] = (K | C | picoquic_spin_function_table[picoquic_supported_versions[cnx->version_index].spinbit_version].spinbit_outgoing(cnx)); +#endif length += picoquic_format_connection_id(&bytes[length], PICOQUIC_MAX_PACKET_SIZE - length, dest_cnx_id); *pn_offset = length; diff --git a/picoquic/spinbit.c b/picoquic/spinbit.c new file mode 100644 index 00000000..aa0c1855 --- /dev/null +++ b/picoquic/spinbit.c @@ -0,0 +1,104 @@ +/* +* Author: Christian Huitema +* Copyright (c) 2018, Private Octopus, Inc. +* All rights reserved. +* +* Permission to use, copy, modify, and distribute this software for any +* purpose with or without fee is hereby granted, provided that the above +* copyright notice and this permission notice appear in all copies. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND +* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +* DISCLAIMED. IN NO EVENT SHALL Private Octopus, Inc. BE LIABLE FOR ANY +* DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES +* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +* ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ + +#include "picoquic_internal.h" +#include "tls_api.h" + +#ifndef UNREFERENCED_PARAMETER +#define UNREFERENCED_PARAMETER(x) (void)(x) +#endif + +/* + * Two procedures defining the spin bit basic variant + */ +void picoquic_spinbit_basic_incoming(picoquic_cnx_t * cnx, picoquic_path_t * path_x, picoquic_packet_header * ph) +{ + path_x->current_spin = ph->spin ^ cnx->client_mode; +} + +uint8_t picoquic_spinbit_basic_outgoing(picoquic_cnx_t * cnx) +{ + uint8_t spin_bit = (uint8_t)((cnx->path[0]->current_spin) << 2); + + spin_bit |= (uint8_t)(picoquic_public_random_64() & 3); + + return spin_bit; +} + +/* + * Two procedures defining the spin bit VEC variant + */ + +void picoquic_spinbit_vec_incoming(picoquic_cnx_t * cnx, picoquic_path_t * path_x, picoquic_packet_header * ph) +{ + path_x->current_spin = ph->spin ^ cnx->client_mode; + + if (ph->has_spin_bit && path_x->current_spin != path_x->prev_spin) { + // got an edge + path_x->prev_spin = path_x->current_spin; + path_x->spin_edge = 1; + path_x->spin_vec = (ph->spin_vec == 3) ? 3 : (ph->spin_vec + 1); + path_x->spin_last_trigger = picoquic_get_quic_time(cnx->quic); + } +} + +uint8_t picoquic_spinbit_vec_outgoing(picoquic_cnx_t * cnx) +{ + uint8_t spin_vec = (uint8_t)(cnx->path[0]->spin_vec); + + if (!cnx->path[0]->spin_edge) { + spin_vec = 0; + } else { + cnx->path[0]->spin_edge = 0; + uint64_t dt = picoquic_get_quic_time(cnx->quic) - cnx->path[0]->spin_last_trigger; + if (dt > PICOQUIC_SPIN_VEC_LATE) { // DELAYED + spin_vec = 1; + } + } + + return spin_vec | (uint8_t)((cnx->path[0]->current_spin) << 2); +} + +/* + * Two procedures defining the null spin bit randomized variant + */ + +void picoquic_spinbit_null_incoming(picoquic_cnx_t * cnx, picoquic_path_t * path_x, picoquic_packet_header * ph) +{ + UNREFERENCED_PARAMETER(cnx); + UNREFERENCED_PARAMETER(path_x); + UNREFERENCED_PARAMETER(ph); +} + +uint8_t picoquic_spinbit_null_outgoing(picoquic_cnx_t * cnx) +{ + UNREFERENCED_PARAMETER(cnx); + return 0; +} + +/* + * Table of spin bit functions + */ +picoquic_spinbit_def_t picoquic_spin_function_table[] = { + {picoquic_spinbit_basic_incoming, picoquic_spinbit_basic_outgoing}, + {picoquic_spinbit_vec_incoming, picoquic_spinbit_vec_outgoing}, + {picoquic_spinbit_null_incoming, picoquic_spinbit_null_outgoing} +}; \ No newline at end of file diff --git a/picoquictest/sacktest.c b/picoquictest/sacktest.c index ef74e9c7..815776fb 100644 --- a/picoquictest/sacktest.c +++ b/picoquictest/sacktest.c @@ -158,8 +158,7 @@ static void ack_range_mask(uint64_t* mask, uint64_t highest, uint64_t range) } static int basic_ack_parse(uint8_t* bytes, size_t bytes_max, - struct expected_ack_t* expected_ack, uint64_t expected_mask, - uint32_t version_flags) + struct expected_ack_t* expected_ack, uint64_t expected_mask) { int ret = 0; size_t byte_index = 1; @@ -297,8 +296,7 @@ int sendacktest() received_mask |= 1ull << (test_pn64[i] & 63); if (ret == 0) { - ret = basic_ack_parse(bytes, consumed, &expected_ack[i], received_mask, - picoquic_supported_versions[cnx.version_index].version_flags); + ret = basic_ack_parse(bytes, consumed, &expected_ack[i], received_mask); } if (ret != 0) { diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 4c6b841c..faf4dbe5 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -3028,7 +3028,7 @@ int spin_bit_test() if (ret == 0) { uint64_t spin_time_prediction = spin_count * test_ctx->cnx_client->path[0]->smoothed_rtt; - if (5 * spin_time_prediction < spin_duration || 5 * spin_duration < spin_time_prediction) { + if (spin_count < 6) { DBG_PRINTF("Unplausible spin bit: %d rotations, rtt_min = %d, duration = %d\n", spin_count, (int)test_ctx->cnx_client->path[0]->rtt_min, (int)spin_duration); ret = -1; -- GitLab