From 51b7c6a535ceb5702c9d75213dee02878371a500 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Fri, 15 Dec 2023 19:19:46 -0800 Subject: [PATCH 01/13] Test reproes race condition error --- UnitTest1/unittest1.cpp | 7 ++ picoquic_t/picoquic_t.c | 1 + picoquictest/edge_cases.c | 159 ++++++++++++++++++++++++++++++++++++ picoquictest/picoquictest.h | 1 + 4 files changed, 168 insertions(+) diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index 04648b242..7c651654c 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -1621,6 +1621,13 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } + TEST_METHOD(reset_repeat_ack) + { + int ret = reset_repeat_ack_test(); + + Assert::AreEqual(ret, 0); + } + TEST_METHOD(ready_to_send) { int ret = ready_to_send_test(); diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c index 294acb5b1..a84f183b0 100644 --- a/picoquic_t/picoquic_t.c +++ b/picoquic_t/picoquic_t.c @@ -273,6 +273,7 @@ static const picoquic_test_def_t test_table[] = { { "error_reason", error_reason_test }, { "idle_server", idle_server_test }, { "idle_timeout", idle_timeout_test }, + { "reset_repeat_ack", reset_repeat_ack_test }, { "ready_to_send", ready_to_send_test }, { "ready_to_skip", ready_to_skip_test }, { "ready_to_zfin", ready_to_zfin_test }, diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 08da8beee..9df3ab7d0 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -857,4 +857,163 @@ int idle_server_test() DBG_PRINTF("%s", "All idle timeout tests pass.\n"); } return ret; +} + +/* testing the ack_of_ack issue with reset stream. The sequence is: +* +* - Connection starts. +* - Stream starts. +* - Stream is reset. +* - Reset is acked, and stream context is deleted. +* - then some new event happens: +* +* 1) simulate that a repeated reset packet is acked, +* calling picoquic_process_ack_of_reset_stream_frame +* +* 2) simulate arrival of a extra "reset" frame. +* 3) simulate receit of delayed "stop sending" frame +* 4) simulate receit of delayed "stream blocked" frame + */ + +static test_api_stream_desc_t test_scenario_edge_reset[] = { + { 4, 0, 128, 1000000 } + }; + +int picoquic_process_ack_of_reset_stream_frame(picoquic_cnx_t* cnx, const uint8_t* bytes, size_t bytes_size, size_t* consumed); + +int reset_repeat_test_one(uint8_t test_id) +{ + picoquic_test_tls_api_ctx_t* test_ctx = NULL; + uint64_t simulated_time = 0; + uint64_t loss_mask = 0; + uint64_t data_stream_id = 4; + uint64_t loop1_time = 15000; + uint64_t loop2_time = 100000; + picoquic_connection_id_t initial_cid = { { 0x8e, 0x5e, 0x48, 0xe9, 0, 0, 0, 0}, 8 }; + int ret = 0; + + initial_cid.id[4] = test_id; + + /* Create the test context */ + if (ret == 0) { + ret = tls_api_init_ctx_ex(&test_ctx, PICOQUIC_INTERNAL_TEST_VERSION_1, + PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, &simulated_time, NULL, NULL, 0, 1, 0, &initial_cid); + } + + /* Set the binlog */ + if (ret == 0) { + picoquic_set_binlog(test_ctx->qclient, "."); + } + + /* Prepare to send data */ + if (ret == 0) { + ret = test_api_init_send_recv_scenario(test_ctx, test_scenario_edge_reset, sizeof(test_scenario_edge_reset)); + + if (ret != 0) + { + DBG_PRINTF("Init send receive scenario returns %d\n", ret); + } + } + + if (ret == 0) { + ret = tls_api_one_scenario_body_connect(test_ctx, &simulated_time, 0, 0, 0); + if (ret != 0) + { + DBG_PRINTF("Connection loop returns %d\n", ret); + } + } + + /* Run for a short time, so the stream is created and the transfer started */ + if (ret == 0) { + int was_active = 0; + int nb_inactive = 0; + uint64_t time_out = simulated_time + loop1_time; + + while (simulated_time < time_out && + TEST_CLIENT_READY && + TEST_SERVER_READY && + nb_inactive < 64 && + ret == 0) { + int was_active = 0; + picoquic_stream_head_t* stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + if (stream != NULL && stream->sent_offset > 10000) { + break; + } + + ret = tls_api_one_sim_round(test_ctx, &simulated_time, 0, &was_active); + + if (was_active) { + nb_inactive = 0; + } + else { + nb_inactive++; + } + } + } + + /* Verify that the stream #4 is present, and the + * transmission has not stopped. + */ + if (ret == 0) { + picoquic_stream_head_t* stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + if (stream == NULL || stream->fin_sent) { + DBG_PRINTF("Waited too long, stream is %s\n", (stream == NULL) ? "deleted" : "finished"); + ret = -1; + } + } + /* Reset the stream, then run the connection for a short time. + */ + if (ret == 0) { + ret = picoquic_reset_stream(test_ctx->cnx_server, data_stream_id, 0); + } + if (ret == 0) { + ret = tls_api_wait_for_timeout(test_ctx, &simulated_time, loop2_time); + } + /* Verify that the stream #4 is now gone. + */ + if (ret == 0) { + picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); + if (s_stream != NULL || c_stream != NULL) { + DBG_PRINTF("%s", "Did not wait long enough, stream is still there."); + ret = -1; + } + } + /* Perform the specified test. + */ + switch (test_id) { + case 1: /* spurious ack of reset frame. */ { + picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + uint8_t reset_frame[4] = { + (uint8_t)picoquic_frame_type_reset_stream, + (uint8_t)data_stream_id, + 1, + 1 }; + size_t consumed = 0; + ret = picoquic_process_ack_of_reset_stream_frame(test_ctx->cnx_server, reset_frame, sizeof(reset_frame), &consumed); + + if (ret != 0 || test_ctx->cnx_server->cnx_state != picoquic_state_ready) { + DBG_PRINTF("Test %d. Error after ack of reset, ret = 0x%x.", test_id, ret); + ret = -1; + } + break; + } + default: + DBG_PRINTF("What test is that: %d.", test_id); + ret = -1; + break; + } + + /* Clean up */ + if (test_ctx != NULL) { + tls_api_delete_ctx(test_ctx); + test_ctx = NULL; + } + + return ret; +} + +int reset_repeat_ack_test() +{ + return reset_repeat_test_one(1); } \ No newline at end of file diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h index 8a03ce731..b0d99dc1a 100644 --- a/picoquictest/picoquictest.h +++ b/picoquictest/picoquictest.h @@ -224,6 +224,7 @@ int ec9a_preemptive_amok_test(); int error_reason_test(); int idle_server_test(); int idle_timeout_test(); +int reset_repeat_ack_test(); int ready_to_send_test(); int ready_to_skip_test(); int ready_to_zero_test(); From 16ab83943e5c32a452426d9ee155915b3239889c Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Fri, 15 Dec 2023 19:53:37 -0800 Subject: [PATCH 02/13] Corrected repro --- picoquic/frames.c | 2 +- picoquictest/edge_cases.c | 49 +++++++++++++++++++++++++++------------ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/picoquic/frames.c b/picoquic/frames.c index 2c0947998..2a7d559d7 100644 --- a/picoquic/frames.c +++ b/picoquic/frames.c @@ -320,7 +320,7 @@ int picoquic_process_ack_of_reset_stream_frame(picoquic_cnx_t * cnx, const uint8 } else { *consumed = bytes - byte_first; /* Find the stream */ - if ((stream = picoquic_find_or_create_stream(cnx, stream_id, 0)) != NULL) { + if ((stream = picoquic_find_or_create_stream(cnx, stream_id, !IS_LOCAL_STREAM_ID(stream_id, cnx->client_mode))) != NULL) { /* mark reset as acked by peer */ stream->reset_acked = 1; /* Delete stream if closed. */ diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 9df3ab7d0..5ed5e82c4 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -876,7 +876,7 @@ int idle_server_test() */ static test_api_stream_desc_t test_scenario_edge_reset[] = { - { 4, 0, 128, 1000000 } + { 4, 0, 1000000, 1000000 } }; int picoquic_process_ack_of_reset_stream_frame(picoquic_cnx_t* cnx, const uint8_t* bytes, size_t bytes_size, size_t* consumed); @@ -888,7 +888,7 @@ int reset_repeat_test_one(uint8_t test_id) uint64_t loss_mask = 0; uint64_t data_stream_id = 4; uint64_t loop1_time = 15000; - uint64_t loop2_time = 100000; + uint64_t loop2_time = 1000000; picoquic_connection_id_t initial_cid = { { 0x8e, 0x5e, 0x48, 0xe9, 0, 0, 0, 0}, 8 }; int ret = 0; @@ -935,8 +935,9 @@ int reset_repeat_test_one(uint8_t test_id) nb_inactive < 64 && ret == 0) { int was_active = 0; - picoquic_stream_head_t* stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); - if (stream != NULL && stream->sent_offset > 10000) { + picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); + picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + if (c_stream != NULL && c_stream->sent_offset > 10000 && s_stream != NULL) { break; } @@ -955,7 +956,7 @@ int reset_repeat_test_one(uint8_t test_id) * transmission has not stopped. */ if (ret == 0) { - picoquic_stream_head_t* stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + picoquic_stream_head_t* stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); if (stream == NULL || stream->fin_sent) { DBG_PRINTF("Waited too long, stream is %s\n", (stream == NULL) ? "deleted" : "finished"); ret = -1; @@ -964,17 +965,36 @@ int reset_repeat_test_one(uint8_t test_id) /* Reset the stream, then run the connection for a short time. */ if (ret == 0) { - ret = picoquic_reset_stream(test_ctx->cnx_server, data_stream_id, 0); + ret = picoquic_reset_stream(test_ctx->cnx_client, data_stream_id, 0); } if (ret == 0) { - ret = tls_api_wait_for_timeout(test_ctx, &simulated_time, loop2_time); + ret = picoquic_reset_stream(test_ctx->cnx_server, data_stream_id, 0); } - /* Verify that the stream #4 is now gone. - */ if (ret == 0) { - picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); - picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); - if (s_stream != NULL || c_stream != NULL) { + int was_active = 0; + int nb_inactive = 0; + int client_stream_gone = 0; + uint64_t time_out = simulated_time + loop2_time; + while (ret == 0 && simulated_time < time_out && + TEST_CLIENT_READY && + TEST_SERVER_READY && + nb_inactive < 64) { + int was_active = 0; + + ret = tls_api_one_sim_round(test_ctx, &simulated_time, time_out, &was_active); + + if (was_active) { + nb_inactive = 0; + } + else { + nb_inactive++; + } + if (picoquic_find_stream(test_ctx->cnx_client, data_stream_id) == NULL) { + client_stream_gone = 1; + break; + } + } + if (!client_stream_gone) { DBG_PRINTF("%s", "Did not wait long enough, stream is still there."); ret = -1; } @@ -983,16 +1003,15 @@ int reset_repeat_test_one(uint8_t test_id) */ switch (test_id) { case 1: /* spurious ack of reset frame. */ { - picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); uint8_t reset_frame[4] = { (uint8_t)picoquic_frame_type_reset_stream, (uint8_t)data_stream_id, 1, 1 }; size_t consumed = 0; - ret = picoquic_process_ack_of_reset_stream_frame(test_ctx->cnx_server, reset_frame, sizeof(reset_frame), &consumed); + ret = picoquic_process_ack_of_reset_stream_frame(test_ctx->cnx_client, reset_frame, sizeof(reset_frame), &consumed); - if (ret != 0 || test_ctx->cnx_server->cnx_state != picoquic_state_ready) { + if (ret != 0 || test_ctx->cnx_client->cnx_state > picoquic_state_ready) { DBG_PRINTF("Test %d. Error after ack of reset, ret = 0x%x.", test_id, ret); ret = -1; } From 648fd46900b9cd36aeecc937ecce420831558be4 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 18:50:14 -0800 Subject: [PATCH 03/13] Tests of stream related frames after reset --- CMakeLists.txt | 2 +- UnitTest1/unittest1.cpp | 50 ++++++- picoquic/frames.c | 49 ++++++- picoquic/picoquic.h | 2 +- picoquic/tls_api.h | 1 + picoquic_t/picoquic_t.c | 9 +- picoquictest/cleartext_aead_test.c | 5 +- picoquictest/edge_cases.c | 208 ++++++++++++++++++++++++++--- picoquictest/picoquictest.h | 9 +- 9 files changed, 301 insertions(+), 34 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2f8e1c291..642ecf411 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ else() endif() project(picoquic - VERSION 1.1.16.1 + VERSION 1.1.16.2 DESCRIPTION "picoquic library" LANGUAGES C CXX) diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index 7c651654c..46eb505b2 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -1621,9 +1621,55 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } - TEST_METHOD(reset_repeat_ack) + TEST_METHOD(reset_ack_max) { - int ret = reset_repeat_ack_test(); + int ret = reset_ack_max_test(); + + Assert::AreEqual(ret, 0); + } + TEST_METHOD(reset_ack_reset) + { + int ret = reset_ack_reset_test(); + + Assert::AreEqual(ret, 0); + } + + TEST_METHOD(reset_extra_max) + { + int ret = reset_extra_max_test(); + + Assert::AreEqual(ret, 0); + } + + TEST_METHOD(reset_extra_reset) + { + int ret = reset_extra_reset_test(); + + Assert::AreEqual(ret, 0); + } + TEST_METHOD(reset_extra_stop) + { + int ret = reset_extra_stop_test(); + + Assert::AreEqual(ret, 0); + } + + TEST_METHOD(reset_need_max) + { + int ret = reset_need_max_test(); + + Assert::AreEqual(ret, 0); + } + TEST_METHOD(reset_need_reset) + { + int ret = reset_need_reset_test(); + + Assert::AreEqual(ret, 0); + } + + TEST_METHOD(reset_need_stop) + { + int ret = reset_need_stop_test(); Assert::AreEqual(ret, 0); } diff --git a/picoquic/frames.c b/picoquic/frames.c index 2a7d559d7..562bfb2ad 100644 --- a/picoquic/frames.c +++ b/picoquic/frames.c @@ -267,8 +267,15 @@ const uint8_t* picoquic_decode_stream_reset_frame(picoquic_cnx_t* cnx, const uin picoquic_connection_error(cnx, PICOQUIC_TRANSPORT_FRAME_FORMAT_ERROR, picoquic_frame_type_reset_stream); } else if ((stream = picoquic_find_or_create_stream(cnx, stream_id, 1)) == NULL) { - bytes = NULL; // error already signaled - + /* Not finding the stream is only an error if the stream + * was expected to be present, or created on demand. If the + * stream was already created and then deleted, there is no harm. + * If the "return NULL" is in a normal scenario, the connection state + * will remain "ready" or "almost ready" + */ + if (cnx->cnx_state > picoquic_state_ready) { + bytes = NULL; /* error already signaled */ + } } else if ((stream->fin_received || stream->reset_received) && final_offset != stream->fin_offset) { picoquic_connection_error(cnx, PICOQUIC_TRANSPORT_FINAL_OFFSET_ERROR, picoquic_frame_type_reset_stream); @@ -319,8 +326,8 @@ int picoquic_process_ack_of_reset_stream_frame(picoquic_cnx_t * cnx, const uint8 ret = -1; } else { *consumed = bytes - byte_first; - /* Find the stream */ - if ((stream = picoquic_find_or_create_stream(cnx, stream_id, !IS_LOCAL_STREAM_ID(stream_id, cnx->client_mode))) != NULL) { + /* Find the stream, if it exists. If it was already deleted, do nothing. */ + if ((stream = picoquic_find_stream(cnx, stream_id)) != NULL) { /* mark reset as acked by peer */ stream->reset_acked = 1; /* Delete stream if closed. */ @@ -347,7 +354,7 @@ int picoquic_check_reset_stream_needs_repeat(picoquic_cnx_t* cnx, const uint8_t* /* Internal error -- cannot parse the stored packet */ ret = -1; } - else if ((stream = picoquic_find_or_create_stream(cnx, stream_id, 0)) == NULL || + else if ((stream = picoquic_find_stream(cnx, stream_id)) == NULL || stream->reset_acked) { *no_need_to_repeat = 1; } @@ -837,6 +844,35 @@ const uint8_t* picoquic_skip_stop_sending_frame(const uint8_t* bytes, const uint } +int picoquic_check_stop_sending_needs_repeat(picoquic_cnx_t* cnx, const uint8_t* bytes, size_t bytes_size, int* no_need_to_repeat) +{ + uint64_t stream_id = 0; + uint64_t error_code = 0; + const uint8_t* bytes_max = bytes + bytes_size; + picoquic_stream_head_t* stream; + int ret = 0; + + *no_need_to_repeat = 0; + + if ((bytes = picoquic_frames_varint_decode(bytes+1, bytes_max, &stream_id)) == NULL || + (bytes = picoquic_frames_varint_decode(bytes, bytes_max, &error_code)) == NULL) + { + /* If the frame cannot be decoded, do not repeat it */ + *no_need_to_repeat = 1; + } + else if ((stream = picoquic_find_stream(cnx, stream_id)) == NULL) { + /* If the stream is deleted, no need to repeat this frame. */ + *no_need_to_repeat = 1; + } + else if (stream->fin_received || stream->reset_received) { + /* No point repeating if the stream is closed by the peer */ + *no_need_to_repeat = 1; + } + + return ret; +} + + /* * STREAM frames implicitly create a stream and carry stream data. */ @@ -3123,6 +3159,9 @@ int picoquic_check_frame_needs_repeat(picoquic_cnx_t* cnx, const uint8_t* bytes, case picoquic_frame_type_reset_stream: ret = picoquic_check_reset_stream_needs_repeat(cnx, bytes, bytes_max, no_need_to_repeat); break; + case picoquic_frame_type_stop_sending: + ret = picoquic_check_stop_sending_needs_repeat(cnx, bytes, bytes_max, no_need_to_repeat); + break; default: { uint64_t frame_id64; *no_need_to_repeat = 0; diff --git a/picoquic/picoquic.h b/picoquic/picoquic.h index eb78b665c..a2c61658c 100644 --- a/picoquic/picoquic.h +++ b/picoquic/picoquic.h @@ -40,7 +40,7 @@ extern "C" { #endif -#define PICOQUIC_VERSION "1.1.16.1" +#define PICOQUIC_VERSION "1.1.16.2" #define PICOQUIC_ERROR_CLASS 0x400 #define PICOQUIC_ERROR_DUPLICATE (PICOQUIC_ERROR_CLASS + 1) #define PICOQUIC_ERROR_AEAD_CHECK (PICOQUIC_ERROR_CLASS + 3) diff --git a/picoquic/tls_api.h b/picoquic/tls_api.h index efb3b12d9..8bd47b9d9 100644 --- a/picoquic/tls_api.h +++ b/picoquic/tls_api.h @@ -193,6 +193,7 @@ void picoquic_aes128_ecb_free(void* v_aesecb); void picoquic_aes128_ecb_encrypt(void* v_aesecb, uint8_t* output, const uint8_t* input, size_t len); +void picoquic_tls_api_init(); void picoquic_tls_api_unload(); void picoquic_tls_api_reset(uint64_t init_flags); diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c index a84f183b0..6eadd7138 100644 --- a/picoquic_t/picoquic_t.c +++ b/picoquic_t/picoquic_t.c @@ -273,7 +273,14 @@ static const picoquic_test_def_t test_table[] = { { "error_reason", error_reason_test }, { "idle_server", idle_server_test }, { "idle_timeout", idle_timeout_test }, - { "reset_repeat_ack", reset_repeat_ack_test }, + { "reset_ack_max", reset_ack_max_test }, + { "reset_ack_reset", reset_ack_reset_test }, + { "reset_extra_max", reset_extra_max_test }, + { "reset_extra_reset", reset_extra_reset_test }, + { "reset_extra_stop", reset_extra_stop_test }, + { "reset_need_max", reset_need_max_test }, + { "reset_need_reset", reset_need_reset_test }, + { "reset_need_stop", reset_need_stop_test }, { "ready_to_send", ready_to_send_test }, { "ready_to_skip", ready_to_skip_test }, { "ready_to_zfin", ready_to_zfin_test }, diff --git a/picoquictest/cleartext_aead_test.c b/picoquictest/cleartext_aead_test.c index d95daf73c..a3281b219 100644 --- a/picoquictest/cleartext_aead_test.c +++ b/picoquictest/cleartext_aead_test.c @@ -329,8 +329,11 @@ int pn_ctr_test() uint8_t in_bytes[16]; uint8_t out_bytes[16]; uint8_t decoded[16]; + + picoquic_tls_api_init(); + ptls_aead_algorithm_t* aead = (ptls_aead_algorithm_t*)picoquic_get_aes128gcm_v(0); - ptls_cipher_context_t *pn_enc = ptls_cipher_new(aead->ctr_cipher, 1, key); + ptls_cipher_context_t *pn_enc = (aead == NULL)?NULL:ptls_cipher_new(aead->ctr_cipher, 1, key); if (pn_enc == NULL) { ret = -1; diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 5ed5e82c4..de0fd39b4 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -859,27 +859,91 @@ int idle_server_test() return ret; } -/* testing the ack_of_ack issue with reset stream. The sequence is: +/* +* Testing isuses caused by frame events after a stream is reset. +* +* We are concerned with possible errors when copies of "old" +* frames cause processing of a stream after the stream has been +* closed. This includes: * -* - Connection starts. -* - Stream starts. -* - Stream is reset. -* - Reset is acked, and stream context is deleted. -* - then some new event happens: +* - reset stream frames, +* - stop sending frames, +* - max stream data frames. * -* 1) simulate that a repeated reset packet is acked, -* calling picoquic_process_ack_of_reset_stream_frame +* We are concerned with three events: * -* 2) simulate arrival of a extra "reset" frame. -* 3) simulate receit of delayed "stop sending" frame -* 4) simulate receit of delayed "stream blocked" frame +* - processing the ACK of a packet that contained the frame, because +* the ACK may be received after the reset after the stream context +* was deleted, +* - receiving extra copies of the frames after the stream is deleted. +* These extra copies shall be ignored with no side effect. +* - processing of frames in packets detected as lost after the +* stream was deleted. The stack whether the packets needs to +* be repeated. +* +* The combination of "ack/extra/need" with three frame types produces +* 9 possible tests. However, there is no acking processing for +* stop sending frames, so we do not implement a test for +* "reset_ack_stop_sending". */ +typedef enum { + reset_ack_max_stream = 0, + reset_ack_reset, + reset_ack_stop_sending, + reset_extra_max_stream, + reset_extra_reset, + reset_extra_stop_sending, + reset_need_max_stream, + reset_need_reset, + reset_need_stop_sending +} reset_test_enum; + static test_api_stream_desc_t test_scenario_edge_reset[] = { - { 4, 0, 1000000, 1000000 } - }; + { 4, 0, 1000000, 1000000 } +}; int picoquic_process_ack_of_reset_stream_frame(picoquic_cnx_t* cnx, const uint8_t* bytes, size_t bytes_size, size_t* consumed); +int picoquic_process_ack_of_max_stream_data_frame(picoquic_cnx_t* cnx, const uint8_t* bytes, + size_t bytes_size, size_t* consumed); + +int reset_repeat_test_receive_frame(int test_id, picoquic_cnx_t * cnx, const uint8_t * frame, size_t frame_size, + uint64_t simulated_time, uint64_t stream_id, int do_not_create) +{ + picoquic_stream_data_node_t dn; + int ret = picoquic_decode_frames(cnx, cnx->path[0], frame, frame_size, + &dn, picoquic_epoch_1rtt, + (struct sockaddr*)&cnx->path[0]->peer_addr, + (struct sockaddr*)&cnx->path[0]->local_addr, + 123, 0, simulated_time); + + if (ret != 0 || cnx->cnx_state > picoquic_state_ready) { + DBG_PRINTF("Test %d. Error after stop sending, ret = 0x%x.", test_id, ret); + ret = -1; + } + else if (ret == 0 && stream_id != UINT64_MAX && do_not_create && picoquic_find_stream(cnx, stream_id) != NULL) { + DBG_PRINTF("Test %d. Stream %" PRIu64 " was created.", test_id, stream_id); + ret = -1; + } + return ret; +} + +int reset_repeat_test_need_repeat(int test_id, picoquic_cnx_t* cnx, const uint8_t* frame, size_t frame_size, + uint64_t simulated_time, uint64_t stream_id, int do_not_create) +{ + int no_need_to_repeat = 0; + int do_not_detect_spurious = 0; + int is_preemptive_needed = 0; + + int ret = picoquic_check_frame_needs_repeat(cnx, frame, frame_size, picoquic_packet_1rtt_protected, + &no_need_to_repeat, &do_not_detect_spurious, &is_preemptive_needed); + + if (ret != 0 || cnx->cnx_state > picoquic_state_ready || !no_need_to_repeat) { + DBG_PRINTF("Test %d. Error after ack of frame 0x%x, ret = 0x%x.", test_id, frame[0], ret); + ret = -1; + } + return ret; +} int reset_repeat_test_one(uint8_t test_id) { @@ -891,6 +955,19 @@ int reset_repeat_test_one(uint8_t test_id) uint64_t loop2_time = 1000000; picoquic_connection_id_t initial_cid = { { 0x8e, 0x5e, 0x48, 0xe9, 0, 0, 0, 0}, 8 }; int ret = 0; + uint8_t stop_sending_frame[3] = { + (uint8_t)picoquic_frame_type_stop_sending, + (uint8_t)data_stream_id, + 0x17 }; + uint8_t max_stream_data_frame[3] = { + (uint8_t)picoquic_frame_type_max_stream_data, + (uint8_t)data_stream_id, + 63 }; + uint8_t reset_frame[4] = { + (uint8_t)picoquic_frame_type_reset_stream, + (uint8_t)data_stream_id, + 1, + 1 }; initial_cid.id[4] = test_id; @@ -1002,12 +1079,17 @@ int reset_repeat_test_one(uint8_t test_id) /* Perform the specified test. */ switch (test_id) { - case 1: /* spurious ack of reset frame. */ { - uint8_t reset_frame[4] = { - (uint8_t)picoquic_frame_type_reset_stream, - (uint8_t)data_stream_id, - 1, - 1 }; + case reset_ack_max_stream: { + size_t consumed = 0; + ret = picoquic_process_ack_of_max_stream_data_frame(test_ctx->cnx_client, reset_frame, sizeof(reset_frame), &consumed); + + if (ret != 0 || test_ctx->cnx_client->cnx_state > picoquic_state_ready) { + DBG_PRINTF("Test %d. Error after ack of max stream, ret = 0x%x.", test_id, ret); + ret = -1; + } + break; + } + case reset_ack_reset:/* spurious ack of reset frame. */ { size_t consumed = 0; ret = picoquic_process_ack_of_reset_stream_frame(test_ctx->cnx_client, reset_frame, sizeof(reset_frame), &consumed); @@ -1017,8 +1099,55 @@ int reset_repeat_test_one(uint8_t test_id) } break; } + case reset_ack_stop_sending: + /* TODO: there is no code yet for processing acks of stop sending frame. */ + ret = -1; + break; + case reset_extra_max_stream: + /* arrival of stream related frame on a non existing stream. + * this is a bit more subtle than the ACK test. + * - if this is a remotely created stream: + * - if it is bidir, this could be an out of order request to + * not send response, or it could be an old stream that was + * already deleted. + * - if it is monodir, it does not make sense. + * - if it is locally created: + * - if it is closed, just ignore it. + * - if it is not created yet, this is a protocol error. + */ + ret = reset_repeat_test_receive_frame(test_id, test_ctx->cnx_client, max_stream_data_frame, sizeof(max_stream_data_frame), + simulated_time, data_stream_id, 1); + break; + case reset_extra_reset: + /* arrival of extra reset frame. + * see, arrival of stop sending. + */ + ret = reset_repeat_test_receive_frame(test_id, test_ctx->cnx_client, reset_frame, sizeof(reset_frame), + simulated_time, data_stream_id, 1); + break; + case reset_extra_stop_sending: + ret = reset_repeat_test_receive_frame(test_id, test_ctx->cnx_client, stop_sending_frame, sizeof(stop_sending_frame), + simulated_time, data_stream_id, 1); + break; + case reset_need_max_stream: + /* Check whether a frame needs to be repeated. + * this should never cause an error! If the stream is not + * there any more, this means the original reset was + * successful, there is no need to resend it. + */ + ret = reset_repeat_test_need_repeat(test_id, test_ctx->cnx_client, max_stream_data_frame, sizeof(max_stream_data_frame), + simulated_time, data_stream_id, 1); + break; + case reset_need_reset: + ret = reset_repeat_test_need_repeat(test_id, test_ctx->cnx_client, reset_frame, sizeof(reset_frame), + simulated_time, data_stream_id, 1); + break; + case reset_need_stop_sending: + ret = reset_repeat_test_need_repeat(test_id, test_ctx->cnx_client, stop_sending_frame, sizeof(stop_sending_frame), + simulated_time, data_stream_id, 1); + break; default: - DBG_PRINTF("What test is that: %d.", test_id); + DBG_PRINTF("What test is that: %d?", test_id); ret = -1; break; } @@ -1032,7 +1161,42 @@ int reset_repeat_test_one(uint8_t test_id) return ret; } -int reset_repeat_ack_test() +int reset_ack_max_test() +{ + return reset_repeat_test_one(reset_ack_max_stream); +} + +int reset_ack_reset_test() +{ + return reset_repeat_test_one(reset_ack_reset); +} + +int reset_extra_max_test() +{ + return reset_repeat_test_one(reset_extra_max_stream); +} + +int reset_extra_reset_test() +{ + return reset_repeat_test_one(reset_extra_reset); +} + +int reset_extra_stop_test() +{ + return reset_repeat_test_one(reset_extra_stop_sending); +} + +int reset_need_max_test() +{ + return reset_repeat_test_one(reset_need_max_stream); +} + +int reset_need_reset_test() +{ + return reset_repeat_test_one(reset_need_reset); +} + +int reset_need_stop_test() { - return reset_repeat_test_one(1); + return reset_repeat_test_one(reset_need_stop_sending); } \ No newline at end of file diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h index b0d99dc1a..ad46a7031 100644 --- a/picoquictest/picoquictest.h +++ b/picoquictest/picoquictest.h @@ -224,7 +224,14 @@ int ec9a_preemptive_amok_test(); int error_reason_test(); int idle_server_test(); int idle_timeout_test(); -int reset_repeat_ack_test(); +int reset_ack_max_test(); +int reset_ack_reset_test(); +int reset_extra_max_test(); +int reset_extra_reset_test(); +int reset_extra_stop_test(); +int reset_need_max_test(); +int reset_need_reset_test(); +int reset_need_stop_test(); int ready_to_send_test(); int ready_to_skip_test(); int ready_to_zero_test(); From 0cb7fcc88e32d28be4440c93b884710860f2c91f Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 19:18:45 -0800 Subject: [PATCH 04/13] Fix memory access in simulation. --- picoquictest/edge_cases.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index de0fd39b4..8d216d9e2 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -1013,7 +1013,8 @@ int reset_repeat_test_one(uint8_t test_id) ret == 0) { int was_active = 0; picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); - picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + picoquic_stream_head_t* s_stream = (test_ctx->cnx_server == NULL)?NULL: + picoquic_find_stream(test_ctx->cnx_server, data_stream_id); if (c_stream != NULL && c_stream->sent_offset > 10000 && s_stream != NULL) { break; } From 7b2ebee0e213357a37d23349b6102c0191188568 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 20:59:38 -0800 Subject: [PATCH 05/13] Fix memory issue in test varints --- UnitTest1/unittest1.cpp | 2 +- picoquictest/edge_cases.c | 7 +++++-- picoquictest/intformattest.c | 12 ++++++++---- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index 46eb505b2..361cc2a1d 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -160,7 +160,7 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } - TEST_METHOD(test_varints) + TEST_METHOD(varints) { int ret = varint_test(); diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 8d216d9e2..3fc3aad09 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -1013,8 +1013,7 @@ int reset_repeat_test_one(uint8_t test_id) ret == 0) { int was_active = 0; picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); - picoquic_stream_head_t* s_stream = (test_ctx->cnx_server == NULL)?NULL: - picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); if (c_stream != NULL && c_stream->sent_offset > 10000 && s_stream != NULL) { break; } @@ -1033,6 +1032,10 @@ int reset_repeat_test_one(uint8_t test_id) /* Verify that the stream #4 is present, and the * transmission has not stopped. */ + if (!(TEST_CLIENT_READY) || !(TEST_SERVER_READY)) { + DBG_PRINTF("%s", "Sevrer or client not ready!"); + ret = -1; + } if (ret == 0) { picoquic_stream_head_t* stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); if (stream == NULL || stream->fin_sent) { diff --git a/picoquictest/intformattest.c b/picoquictest/intformattest.c index 460d1d64a..39781ef8f 100644 --- a/picoquictest/intformattest.c +++ b/picoquictest/intformattest.c @@ -249,21 +249,25 @@ static size_t nb_varint_test_cases = sizeof(varint_test_cases) / sizeof(picoquic int varint_test() { int ret = 0; + uint8_t test_buf[16]; const picoquic_varintformat_test_t* max_test = varint_test_cases + nb_varint_test_cases; + memset(test_buf, 0xcc, 16); for (picoquic_varintformat_test_t* test = varint_test_cases; ret == 0 && test < max_test; test++) { for (int is_new_decode = 0; ret == 0 && is_new_decode <= 1; is_new_decode++) { - for (size_t buf_size = 0; ret == 0 && buf_size <= test->length + 2; buf_size++) { + for (size_t buf_size = 0; ret == 0 && buf_size <= test->length + 2 && buf_size < 16; buf_size++) { int test_ret = 0; uint64_t n64 = 0; size_t length; + memcpy(test_buf, test->encoding, test->length); + if (is_new_decode) { - const uint8_t* bytes = picoquic_frames_varint_decode(test->encoding, test->encoding + buf_size, &n64); - length = bytes != NULL ? bytes - test->encoding : 0; + const uint8_t* bytes = picoquic_frames_varint_decode(test_buf, test_buf + buf_size, &n64); + length = bytes != NULL ? bytes - test_buf : 0; } else { - length = picoquic_varint_decode(test->encoding, buf_size, &n64); + length = picoquic_varint_decode(test_buf, buf_size, &n64); } if (length != (buf_size < test->length ? 0 : test->length)) { From 39f0621350f65f03629833053565fcb6c8322546 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 21:08:46 -0800 Subject: [PATCH 06/13] fix compile and ubsan warning --- picoquic/quicctx.c | 2 +- picoquictest/edge_cases.c | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c index 463398016..e3a310e96 100644 --- a/picoquic/quicctx.c +++ b/picoquic/quicctx.c @@ -539,7 +539,7 @@ static picosplay_node_t* picoquic_registered_token_create(void* value) static void* picoquic_registered_token_value(picosplay_node_t* node) { - return (void*)((char*)node - offsetof(struct st_picoquic_registered_token_t, registered_token_node)); + return (void*)((node == NULL)?NULL:((char*)node - offsetof(struct st_picoquic_registered_token_t, registered_token_node))); } static void picoquic_registered_token_delete(void* tree, picosplay_node_t* node) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 3fc3aad09..018ff00f3 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -748,7 +748,6 @@ int idle_server_test_one(uint8_t test_id, uint64_t client_timeout, uint64_t hand picoquic_test_tls_api_ctx_t* test_ctx = NULL; uint64_t simulated_time = 0; uint64_t target_timeout; - uint64_t loss_mask = 0; picoquic_connection_id_t initial_cid = { { 0x41, 0x9e, 0xc0, 0x99, 0, 0, 0, 0}, 8 }; uint8_t send_buffer[PICOQUIC_MAX_PACKET_SIZE]; int ret = 0; @@ -949,7 +948,6 @@ int reset_repeat_test_one(uint8_t test_id) { picoquic_test_tls_api_ctx_t* test_ctx = NULL; uint64_t simulated_time = 0; - uint64_t loss_mask = 0; uint64_t data_stream_id = 4; uint64_t loop1_time = 15000; uint64_t loop2_time = 1000000; @@ -1011,12 +1009,12 @@ int reset_repeat_test_one(uint8_t test_id) TEST_SERVER_READY && nb_inactive < 64 && ret == 0) { - int was_active = 0; picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); if (c_stream != NULL && c_stream->sent_offset > 10000 && s_stream != NULL) { break; } + was_active = 0; ret = tls_api_one_sim_round(test_ctx, &simulated_time, 0, &was_active); @@ -1060,7 +1058,7 @@ int reset_repeat_test_one(uint8_t test_id) TEST_CLIENT_READY && TEST_SERVER_READY && nb_inactive < 64) { - int was_active = 0; + was_active = 0; ret = tls_api_one_sim_round(test_ctx, &simulated_time, time_out, &was_active); From 09d71ae9db9ea09cbfb3a049d30d50f1772d4f38 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 21:25:01 -0800 Subject: [PATCH 07/13] Isolate first wait loop --- picoquictest/edge_cases.c | 51 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 018ff00f3..f1fd1aa1a 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -859,7 +859,7 @@ int idle_server_test() } /* -* Testing isuses caused by frame events after a stream is reset. +* Testing issues caused by frame events after a stream is reset. * * We are concerned with possible errors when copies of "old" * frames cause processing of a stream after the stream has been @@ -944,12 +944,49 @@ int reset_repeat_test_need_repeat(int test_id, picoquic_cnx_t* cnx, const uint8_ return ret; } +int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, + uint64_t* simulated_time, uint64_t data_stream_id, uint64_t loop1_time) +{ + int ret = 0; + int was_active = 0; + int nb_inactive = 0; + uint64_t time_out = *simulated_time + loop1_time; + int is_opened = 0; + + while (*simulated_time < time_out && + TEST_CLIENT_READY && + TEST_SERVER_READY && + nb_inactive < 64 && + ret == 0) { + picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); + picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + if (c_stream != NULL && c_stream->sent_offset > 10000 && s_stream != NULL) { + is_opened = 1; + break; + } + was_active = 0; + + ret = tls_api_one_sim_round(test_ctx, simulated_time, 0, &was_active); + + if (was_active) { + nb_inactive = 0; + } + else { + nb_inactive++; + } + } + if (!is_opened) { + ret = -1; + } + return ret; +} + int reset_repeat_test_one(uint8_t test_id) { picoquic_test_tls_api_ctx_t* test_ctx = NULL; uint64_t simulated_time = 0; uint64_t data_stream_id = 4; - uint64_t loop1_time = 15000; + uint64_t loop1_time = 1000000; uint64_t loop2_time = 1000000; picoquic_connection_id_t initial_cid = { { 0x8e, 0x5e, 0x48, 0xe9, 0, 0, 0, 0}, 8 }; int ret = 0; @@ -1000,6 +1037,13 @@ int reset_repeat_test_one(uint8_t test_id) /* Run for a short time, so the stream is created and the transfer started */ if (ret == 0) { +#if 1 + ret = reset_loop_wait_stream_opened(test_ctx, &simulated_time, data_stream_id, loop1_time); + if (ret != 0) { + DBG_PRINTF("Test #%d. Loop wait stream failed!"); + ret = -1; + } +#else int was_active = 0; int nb_inactive = 0; uint64_t time_out = simulated_time + loop1_time; @@ -1025,13 +1069,14 @@ int reset_repeat_test_one(uint8_t test_id) nb_inactive++; } } +#endif } /* Verify that the stream #4 is present, and the * transmission has not stopped. */ if (!(TEST_CLIENT_READY) || !(TEST_SERVER_READY)) { - DBG_PRINTF("%s", "Sevrer or client not ready!"); + DBG_PRINTF("%s", "Server or client not ready!"); ret = -1; } if (ret == 0) { From c41e8c5d827a29203d320c04e2749a106a252e82 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 21:26:58 -0800 Subject: [PATCH 08/13] Fix typo. --- picoquictest/edge_cases.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index f1fd1aa1a..7ddc97365 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -1040,7 +1040,7 @@ int reset_repeat_test_one(uint8_t test_id) #if 1 ret = reset_loop_wait_stream_opened(test_ctx, &simulated_time, data_stream_id, loop1_time); if (ret != 0) { - DBG_PRINTF("Test #%d. Loop wait stream failed!"); + DBG_PRINTF("Test #%d. Loop wait stream failed!", test_id); ret = -1; } #else @@ -1075,7 +1075,7 @@ int reset_repeat_test_one(uint8_t test_id) /* Verify that the stream #4 is present, and the * transmission has not stopped. */ - if (!(TEST_CLIENT_READY) || !(TEST_SERVER_READY)) { + if (ret == 0 && (!(TEST_CLIENT_READY) || !(TEST_SERVER_READY))) { DBG_PRINTF("%s", "Server or client not ready!"); ret = -1; } From 7bde821724c7a9d7951ef1b3896a1d313d180e81 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 21:35:34 -0800 Subject: [PATCH 09/13] Decompose stream context verification --- picoquictest/edge_cases.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 7ddc97365..81fb45c27 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -958,11 +958,15 @@ int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, TEST_SERVER_READY && nb_inactive < 64 && ret == 0) { - picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); - picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); - if (c_stream != NULL && c_stream->sent_offset > 10000 && s_stream != NULL) { - is_opened = 1; - break; + if (test_ctx->cnx_client != NULL && test_ctx->cnx_server != NULL) { + picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); + if (c_stream != NULL && c_stream->sent_offset > 10000) { + picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + if (s_stream != NULL) { + is_opened = 1; + break; + } + } } was_active = 0; From 2d215b8a093555b9e04950cf50bf8b37b4f0f444 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 21:39:09 -0800 Subject: [PATCH 10/13] Tweaking the wait opened loop --- picoquictest/edge_cases.c | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 81fb45c27..a60e8aa24 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -948,7 +948,6 @@ int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, uint64_t* simulated_time, uint64_t data_stream_id, uint64_t loop1_time) { int ret = 0; - int was_active = 0; int nb_inactive = 0; uint64_t time_out = *simulated_time + loop1_time; int is_opened = 0; @@ -958,6 +957,7 @@ int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, TEST_SERVER_READY && nb_inactive < 64 && ret == 0) { + int was_active = 0; if (test_ctx->cnx_client != NULL && test_ctx->cnx_server != NULL) { picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); if (c_stream != NULL && c_stream->sent_offset > 10000) { @@ -968,7 +968,6 @@ int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, } } } - was_active = 0; ret = tls_api_one_sim_round(test_ctx, simulated_time, 0, &was_active); @@ -1041,39 +1040,11 @@ int reset_repeat_test_one(uint8_t test_id) /* Run for a short time, so the stream is created and the transfer started */ if (ret == 0) { -#if 1 ret = reset_loop_wait_stream_opened(test_ctx, &simulated_time, data_stream_id, loop1_time); if (ret != 0) { DBG_PRINTF("Test #%d. Loop wait stream failed!", test_id); ret = -1; } -#else - int was_active = 0; - int nb_inactive = 0; - uint64_t time_out = simulated_time + loop1_time; - - while (simulated_time < time_out && - TEST_CLIENT_READY && - TEST_SERVER_READY && - nb_inactive < 64 && - ret == 0) { - picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); - picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); - if (c_stream != NULL && c_stream->sent_offset > 10000 && s_stream != NULL) { - break; - } - was_active = 0; - - ret = tls_api_one_sim_round(test_ctx, &simulated_time, 0, &was_active); - - if (was_active) { - nb_inactive = 0; - } - else { - nb_inactive++; - } - } -#endif } /* Verify that the stream #4 is present, and the From f8b97abfb4b0330487adb7a62306be566cf89694 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 21:46:17 -0800 Subject: [PATCH 11/13] Decompose file open check --- picoquictest/edge_cases.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index a60e8aa24..4a99e5fc9 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -944,6 +944,21 @@ int reset_repeat_test_need_repeat(int test_id, picoquic_cnx_t* cnx, const uint8_ return ret; } +int reset_loop_check_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, uint64_t data_stream_id) +{ + int is_opened = 0; + if (test_ctx->cnx_client != NULL && test_ctx->cnx_server != NULL) { + picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); + if (c_stream != NULL && c_stream->sent_offset > 10000) { + picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); + if (s_stream != NULL) { + is_opened = 1; + } + } + } + return is_opened; +} + int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, uint64_t* simulated_time, uint64_t data_stream_id, uint64_t loop1_time) { @@ -958,15 +973,10 @@ int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, nb_inactive < 64 && ret == 0) { int was_active = 0; - if (test_ctx->cnx_client != NULL && test_ctx->cnx_server != NULL) { - picoquic_stream_head_t* c_stream = picoquic_find_stream(test_ctx->cnx_client, data_stream_id); - if (c_stream != NULL && c_stream->sent_offset > 10000) { - picoquic_stream_head_t* s_stream = picoquic_find_stream(test_ctx->cnx_server, data_stream_id); - if (s_stream != NULL) { - is_opened = 1; - break; - } - } + + if (reset_loop_check_stream_opened(test_ctx, data_stream_id)) { + is_opened = 1; + break; } ret = tls_api_one_sim_round(test_ctx, simulated_time, 0, &was_active); From 15744a32651148256436263909f38dc16f67edd4 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 22:19:39 -0800 Subject: [PATCH 12/13] Fix test context init. --- picoquictest/edge_cases.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 4a99e5fc9..1b44c65ad 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -965,20 +965,23 @@ int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, int ret = 0; int nb_inactive = 0; uint64_t time_out = *simulated_time + loop1_time; + int guard1 = 0xdeadbeef; + int was_active = 0; int is_opened = 0; + int guard2 = 0xdeadbeef; - while (*simulated_time < time_out && + while (ret == 0 && *simulated_time < time_out && TEST_CLIENT_READY && TEST_SERVER_READY && - nb_inactive < 64 && - ret == 0) { - int was_active = 0; + nb_inactive < 64) { if (reset_loop_check_stream_opened(test_ctx, data_stream_id)) { is_opened = 1; break; } + was_active = 0; + ret = tls_api_one_sim_round(test_ctx, simulated_time, 0, &was_active); if (was_active) { @@ -998,8 +1001,9 @@ int reset_repeat_test_one(uint8_t test_id) { picoquic_test_tls_api_ctx_t* test_ctx = NULL; uint64_t simulated_time = 0; + uint64_t loss_mask = 0; uint64_t data_stream_id = 4; - uint64_t loop1_time = 1000000; + uint64_t loop1_time = 50000; uint64_t loop2_time = 1000000; picoquic_connection_id_t initial_cid = { { 0x8e, 0x5e, 0x48, 0xe9, 0, 0, 0, 0}, 8 }; int ret = 0; @@ -1028,6 +1032,12 @@ int reset_repeat_test_one(uint8_t test_id) /* Set the binlog */ if (ret == 0) { picoquic_set_binlog(test_ctx->qclient, "."); + ret = picoquic_start_client_cnx(test_ctx->cnx_client); + } + + /* set the connection */ + if (ret == 0) { + ret = tls_api_connection_loop(test_ctx, &loss_mask, 0, &simulated_time); } /* Prepare to send data */ @@ -1040,14 +1050,6 @@ int reset_repeat_test_one(uint8_t test_id) } } - if (ret == 0) { - ret = tls_api_one_scenario_body_connect(test_ctx, &simulated_time, 0, 0, 0); - if (ret != 0) - { - DBG_PRINTF("Connection loop returns %d\n", ret); - } - } - /* Run for a short time, so the stream is created and the transfer started */ if (ret == 0) { ret = reset_loop_wait_stream_opened(test_ctx, &simulated_time, data_stream_id, loop1_time); From 81b97261fe01a21d4ba0e830ce41f0f00d74e101 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 16 Dec 2023 22:21:38 -0800 Subject: [PATCH 13/13] Remove debug variables --- picoquictest/edge_cases.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/picoquictest/edge_cases.c b/picoquictest/edge_cases.c index 1b44c65ad..2cf359698 100644 --- a/picoquictest/edge_cases.c +++ b/picoquictest/edge_cases.c @@ -965,10 +965,8 @@ int reset_loop_wait_stream_opened(picoquic_test_tls_api_ctx_t* test_ctx, int ret = 0; int nb_inactive = 0; uint64_t time_out = *simulated_time + loop1_time; - int guard1 = 0xdeadbeef; int was_active = 0; int is_opened = 0; - int guard2 = 0xdeadbeef; while (ret == 0 && *simulated_time < time_out && TEST_CLIENT_READY &&