From 1da144365d7b7317105a907000aaea3fa00d9954 Mon Sep 17 00:00:00 2001 From: huitema Date: Mon, 25 Nov 2024 11:54:30 -0800 Subject: [PATCH 1/4] Better test of spinbit code --- CMakeLists.txt | 1 + UnitTest1/unittest1.cpp | 38 +++- picoquic/picoquic.h | 1 + picoquic/quicctx.c | 15 ++ picoquic_t/picoquic_t.c | 7 +- picoquictest/picoquictest.h | 7 +- picoquictest/picoquictest.vcxproj | 1 + picoquictest/picoquictest.vcxproj.filters | 3 + picoquictest/spinbit_test.c | 216 ++++++++++++++++++++++ picoquictest/tls_api_test.c | 130 ------------- 10 files changed, 285 insertions(+), 134 deletions(-) create mode 100644 picoquictest/spinbit_test.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 69b013a2a..4ae1afe86 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -178,6 +178,7 @@ set(PICOQUIC_TEST_LIBRARY_FILES picoquictest/skip_frame_test.c picoquictest/socket_test.c picoquictest/sockloop_test.c + picoquictest/spinbit_test.c picoquictest/splay_test.c picoquictest/stream0_frame_test.c picoquictest/stresstest.c diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index dae430f75..4bde538ea 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -1398,13 +1398,47 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } - TEST_METHOD(test_spin_bit) + TEST_METHOD(spinbit) { - int ret = spin_bit_test(); + int ret = spinbit_test(); Assert::AreEqual(ret, 0); } + TEST_METHOD(spinbit_bad) + { + int ret = spinbit_bad_test(); + + Assert::AreEqual(ret, 0); + } + + TEST_METHOD(spinbit_on) + { + int ret = spinbit_on_test(); + + Assert::AreEqual(ret, 0); + } + + TEST_METHOD(spinbit_null) + { + int ret = spinbit_null_test(); + + Assert::AreEqual(ret, 0); + } + + TEST_METHOD(spinbit_random) + { + int ret = spinbit_random_test(); + + Assert::AreEqual(ret, 0); + } + + TEST_METHOD(spinbit_randclient) + { + int ret = spinbit_randclient_test(); + + Assert::AreEqual(ret, 0); + } TEST_METHOD(loss_bit) { int ret = loss_bit_test(); diff --git a/picoquic/picoquic.h b/picoquic/picoquic.h index e1f338ad5..22cec8c22 100644 --- a/picoquic/picoquic.h +++ b/picoquic/picoquic.h @@ -640,6 +640,7 @@ void picoquic_set_default_padding(picoquic_quic_t* quic, uint32_t padding_multip /* Set default spin bit policy for the context */ void picoquic_set_default_spinbit_policy(picoquic_quic_t * quic, picoquic_spinbit_version_enum default_spinbit_policy); +void picoquic_set_spinbit_policy(picoquic_cnx_t* cnx, picoquic_spinbit_version_enum spinbit_policy); /* Set default loss bit policy for the context */ void picoquic_set_default_lossbit_policy(picoquic_quic_t* quic, picoquic_lossbit_version_enum default_lossbit_policy); diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c index c1ba92fa6..e0db07d73 100644 --- a/picoquic/quicctx.c +++ b/picoquic/quicctx.c @@ -803,6 +803,21 @@ void picoquic_set_default_spinbit_policy(picoquic_quic_t * quic, picoquic_spinbi quic->default_spin_policy = default_spinbit_policy; } +void picoquic_set_spinbit_policy(picoquic_cnx_t* cnx, picoquic_spinbit_version_enum spinbit_policy) +{ + if (spinbit_policy == picoquic_spinbit_on) { + /* This policy forces using basic all the time! */ + cnx->spin_policy = picoquic_spinbit_basic; + } + else if (spinbit_policy < picoquic_spinbit_on) { + cnx->spin_policy = spinbit_policy; + } + else + { + cnx->spin_policy = picoquic_spinbit_null; + } +} + void picoquic_set_default_lossbit_policy(picoquic_quic_t* quic, picoquic_lossbit_version_enum default_lossbit_policy) { quic->default_lossbit_policy = default_lossbit_policy; diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c index a3a02d0cd..449cbb17b 100644 --- a/picoquic_t/picoquic_t.c +++ b/picoquic_t/picoquic_t.c @@ -237,7 +237,12 @@ static const picoquic_test_def_t test_table[] = { { "nat_rebinding_zero", nat_rebinding_zero_test }, { "nat_rebinding_latency", nat_rebinding_latency_test }, { "nat_rebinding_fast", fast_nat_rebinding_test}, - { "spin_bit", spin_bit_test}, + { "spinbit", spinbit_test }, + { "spinbit_bad", spinbit_bad_test }, + { "spinbit_on", spinbit_on_test }, + { "spinbit_null", spinbit_null_test }, + { "spinbit_randclient", spinbit_randclient_test }, + { "spinbit_random", spinbit_random_test }, { "loss_bit", loss_bit_test}, { "client_error", client_error_test }, { "client_only", client_only_test }, diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h index 5870cc0fe..7e9f4f6b8 100644 --- a/picoquictest/picoquictest.h +++ b/picoquictest/picoquictest.h @@ -154,7 +154,12 @@ int nat_rebinding_test(); int nat_rebinding_loss_test(); int nat_rebinding_zero_test(); int nat_rebinding_latency_test(); -int spin_bit_test(); +int spinbit_test(); +int spinbit_random_test(); +int spinbit_randclient_test(); +int spinbit_null_test(); +int spinbit_on_test(); +int spinbit_bad_test(); int loss_bit_test(); int client_error_test(); int client_only_test(); diff --git a/picoquictest/picoquictest.vcxproj b/picoquictest/picoquictest.vcxproj index 374b82896..053ad3ad9 100644 --- a/picoquictest/picoquictest.vcxproj +++ b/picoquictest/picoquictest.vcxproj @@ -188,6 +188,7 @@ + diff --git a/picoquictest/picoquictest.vcxproj.filters b/picoquictest/picoquictest.vcxproj.filters index 46bd56a5e..7dc357148 100644 --- a/picoquictest/picoquictest.vcxproj.filters +++ b/picoquictest/picoquictest.vcxproj.filters @@ -180,6 +180,9 @@ Source Files + + Source Files + diff --git a/picoquictest/spinbit_test.c b/picoquictest/spinbit_test.c new file mode 100644 index 000000000..f3cffeae4 --- /dev/null +++ b/picoquictest/spinbit_test.c @@ -0,0 +1,216 @@ +/* +* Author: Christian Huitema +* Copyright (c) 2017, 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 "tls_api.h" +#include "picoquic_internal.h" +#include "picoquictest_internal.h" +#include "picoquic.h" +#include +#include + +#include "logreader.h" +#include "picoquic_binlog.h" +#include "picoquic_logger.h" +#include "qlog.h" + + +/* + * Spin bit test. Verify that the bit does spin, and that the number + * of rotations is plausible given the duration and the min delay + * for various the spin policies. + */ + +static test_api_stream_desc_t test_scenario_spin[] = { + { 4, 0, 257, 1000000 } +}; + +int spinbit_test_one(picoquic_spinbit_version_enum spin_policy, picoquic_spinbit_version_enum spin_policy_server) +{ + uint64_t simulated_time = 0; + uint64_t loss_mask = 0; + uint64_t spin_duration = 0; + picoquic_test_tls_api_ctx_t* test_ctx = NULL; + int spin_count = 0; + int ret = tls_api_init_ctx(&test_ctx, PICOQUIC_INTERNAL_TEST_VERSION_1, + PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, &simulated_time, NULL, NULL, 0, 1, 0); + + if (ret != 0) + { + DBG_PRINTF("%s", "Could not create the QUIC test contexts\n"); + } + + if (ret == 0) { + /* force spinbit policy as specified, then start */ + picoquic_set_default_spinbit_policy(test_ctx->qserver, spin_policy_server); + picoquic_set_spinbit_policy(test_ctx->cnx_client, spin_policy); + + ret = picoquic_start_client_cnx(test_ctx->cnx_client); + if (ret != 0) + { + DBG_PRINTF("%s", "Could not initialize stream zero for the client\n"); + } + + } + + if (ret == 0) { + ret = tls_api_connection_loop(test_ctx, &loss_mask, 0, &simulated_time); + + if (ret != 0) + { + DBG_PRINTF("Connection loop returns error %d\n", ret); + } + } + + /* Prepare to send data */ + if (ret == 0) { + ret = test_api_init_send_recv_scenario(test_ctx, test_scenario_spin, sizeof(test_scenario_spin)); + + if (ret != 0) + { + DBG_PRINTF("Init send receive scenario returns %d\n", ret); + } + } + + /* Explore the data sending loop so we can observe the spin bit */ + if (ret == 0) { + uint64_t spin_begin_time = simulated_time; + uint64_t next_time = simulated_time + 10000000; + int ret = 0; + int nb_trials = 0; + int nb_inactive = 0; + int max_trials = 100000; + int current_spin = test_ctx->cnx_client->path[0]->current_spin; + + test_ctx->c_to_s_link->loss_mask = &loss_mask; + test_ctx->s_to_c_link->loss_mask = &loss_mask; + + while (ret == 0 && nb_trials < max_trials && simulated_time < next_time && nb_inactive < 256 && TEST_CLIENT_READY && TEST_SERVER_READY) { + int was_active = 0; + + nb_trials++; + + ret = tls_api_one_sim_round(test_ctx, &simulated_time, next_time, &was_active); + + if (ret < 0) + { + break; + } + + if (test_ctx->cnx_client->path[0]->current_spin != current_spin) { + spin_count++; + current_spin = test_ctx->cnx_client->path[0]->current_spin; + } + + if (was_active) { + nb_inactive = 0; + } + else { + nb_inactive++; + } + + if (test_ctx->test_finished) { + if (picoquic_is_cnx_backlog_empty(test_ctx->cnx_client) && picoquic_is_cnx_backlog_empty(test_ctx->cnx_server)) { + break; + } + } + } + + spin_duration = simulated_time - spin_begin_time; + + if (ret != 0) + { + DBG_PRINTF("Data sending loop fails with ret = %d\n", ret); + } + } + + if (ret == 0) { + ret = picoquic_close(test_ctx->cnx_client, 0); + if (ret != 0) + { + DBG_PRINTF("Picoquic close returns %d\n", ret); + } + } + + if (ret == 0) { + if (spin_policy == picoquic_spinbit_basic) { + if (spin_policy_server == picoquic_spinbit_basic) { + 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; + } + } + else if (spin_policy_server == picoquic_spinbit_random) { + if (spin_count < 100) { + 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; + } + } + else if (spin_policy_server == picoquic_spinbit_null) { + if (spin_count > 0) { + 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; + } + } + } + } + + if (test_ctx != NULL) { + tls_api_delete_ctx(test_ctx); + test_ctx = NULL; + } + + return ret; +} + +int spinbit_test() +{ + return spinbit_test_one(picoquic_spinbit_basic, picoquic_spinbit_basic); +} + +int spinbit_random_test() +{ + return spinbit_test_one(picoquic_spinbit_basic, picoquic_spinbit_random); +} + +int spinbit_randclient_test() +{ + return spinbit_test_one(picoquic_spinbit_random, picoquic_spinbit_basic); +} + +int spinbit_null_test() +{ + return spinbit_test_one(picoquic_spinbit_basic, picoquic_spinbit_null); +} + +int spinbit_on_test() +{ + return spinbit_test_one(picoquic_spinbit_on, picoquic_spinbit_on); +} + +int spinbit_bad_test() +{ + return spinbit_test_one(picoquic_spinbit_on, 123456); +} + + diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 2191c25fd..2a6dc06e9 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -5872,136 +5872,6 @@ int fast_nat_rebinding_test() return ret; } -/* - * Spin bit test. Verify that the bit does spin, and that the number - * of rotations is plausible given the duration and the min delay. - */ - -int spin_bit_test() -{ - uint64_t simulated_time = 0; - uint64_t loss_mask = 0; - uint64_t spin_duration = 0; - picoquic_test_tls_api_ctx_t* test_ctx = NULL; - int spin_count = 0; - int ret = tls_api_init_ctx(&test_ctx, PICOQUIC_INTERNAL_TEST_VERSION_1, - PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, &simulated_time, NULL, NULL, 0, 1, 0); - - if (ret != 0) - { - DBG_PRINTF("%s", "Could not create the QUIC test contexts\n"); - } - - if (ret == 0) { - /* force spinbit policy to basic, then start */ - test_ctx->cnx_client->spin_policy = picoquic_spinbit_basic; - - ret = picoquic_start_client_cnx(test_ctx->cnx_client); - if (ret != 0) - { - DBG_PRINTF("%s", "Could not initialize stream zero for the client\n"); - } - - } - - if (ret == 0) { - ret = tls_api_connection_loop(test_ctx, &loss_mask, 0, &simulated_time); - - if (ret != 0) - { - DBG_PRINTF("Connection loop returns error %d\n", ret); - } - } - - /* Prepare to send data */ - if (ret == 0) { - /* force the server spin bit policy to basic, then init the scenario */ - test_ctx->cnx_server->spin_policy = picoquic_spinbit_basic; - - ret = test_api_init_send_recv_scenario(test_ctx, test_scenario_very_long, sizeof(test_scenario_very_long)); - - if (ret != 0) - { - DBG_PRINTF("Init send receive scenario returns %d\n", ret); - } - } - - /* Explore the data sending loop so we can observe the spin bit */ - if (ret == 0) { - uint64_t spin_begin_time = simulated_time; - uint64_t next_time = simulated_time + 10000000; - int ret = 0; - int nb_trials = 0; - int nb_inactive = 0; - int max_trials = 100000; - int current_spin = test_ctx->cnx_client->path[0]->current_spin; - - test_ctx->c_to_s_link->loss_mask = &loss_mask; - test_ctx->s_to_c_link->loss_mask = &loss_mask; - - while (ret == 0 && nb_trials < max_trials && simulated_time < next_time && nb_inactive < 256 && TEST_CLIENT_READY && TEST_SERVER_READY) { - int was_active = 0; - - nb_trials++; - - ret = tls_api_one_sim_round(test_ctx, &simulated_time, next_time, &was_active); - - if (ret < 0) - { - break; - } - - if (test_ctx->cnx_client->path[0]->current_spin != current_spin) { - spin_count++; - current_spin = test_ctx->cnx_client->path[0]->current_spin; - } - - if (was_active) { - nb_inactive = 0; - } - else { - nb_inactive++; - } - - if (test_ctx->test_finished) { - if (picoquic_is_cnx_backlog_empty(test_ctx->cnx_client) && picoquic_is_cnx_backlog_empty(test_ctx->cnx_server)) { - break; - } - } - } - - spin_duration = simulated_time - spin_begin_time; - - if (ret != 0) - { - DBG_PRINTF("Data sending loop fails with ret = %d\n", ret); - } - } - - if (ret == 0) { - ret = picoquic_close(test_ctx->cnx_client, 0); - if (ret != 0) - { - DBG_PRINTF("Picoquic close returns %d\n", ret); - } - } - - if (ret == 0) { - 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; - } - } - - if (test_ctx != NULL) { - tls_api_delete_ctx(test_ctx); - test_ctx = NULL; - } - - return ret; -} - /* * Test whether the loss bit reporting can be enabled without breaking the connection */ From f1ed0ae940a58538fe6f4f51631c585532e7f338 Mon Sep 17 00:00:00 2001 From: huitema Date: Mon, 25 Nov 2024 12:21:57 -0800 Subject: [PATCH 2/4] Fix spin policy checks. --- picoquic/picoquic.h | 11 ++++++++--- picoquic/quicctx.c | 25 ++++++++++++++++--------- picoquictest/multipath_test.c | 4 ++-- picoquictest/skip_frame_test.c | 2 +- picoquictest/spinbit_test.c | 19 ++++++++++++++----- picoquictest/tls_api_test.c | 4 ++-- 6 files changed, 43 insertions(+), 22 deletions(-) diff --git a/picoquic/picoquic.h b/picoquic/picoquic.h index 22cec8c22..165545d76 100644 --- a/picoquic/picoquic.h +++ b/picoquic/picoquic.h @@ -638,9 +638,14 @@ void picoquic_enforce_client_only(picoquic_quic_t* quic, int do_enforce); /* Set default padding policy for the context */ void picoquic_set_default_padding(picoquic_quic_t* quic, uint32_t padding_multiple, uint32_t padding_minsize); -/* Set default spin bit policy for the context */ -void picoquic_set_default_spinbit_policy(picoquic_quic_t * quic, picoquic_spinbit_version_enum default_spinbit_policy); -void picoquic_set_spinbit_policy(picoquic_cnx_t* cnx, picoquic_spinbit_version_enum spinbit_policy); +/* Set default spin bit policy for the context +* return 0 if OK, -1 if the policy was invalid. +* Note that "picoquic_spinbit_on" is only allowed as a default policy, +* translating to unconditional setup when connections are created for +* the context. As a per conection setup, it is invalid. + */ +int picoquic_set_default_spinbit_policy(picoquic_quic_t * quic, picoquic_spinbit_version_enum default_spinbit_policy); +int picoquic_set_spinbit_policy(picoquic_cnx_t* cnx, picoquic_spinbit_version_enum spinbit_policy); /* Set default loss bit policy for the context */ void picoquic_set_default_lossbit_policy(picoquic_quic_t* quic, picoquic_lossbit_version_enum default_lossbit_policy); diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c index e0db07d73..300ab72d8 100644 --- a/picoquic/quicctx.c +++ b/picoquic/quicctx.c @@ -798,24 +798,31 @@ void picoquic_set_default_padding(picoquic_quic_t* quic, uint32_t padding_multip quic->padding_multiple_default = padding_multiple; } -void picoquic_set_default_spinbit_policy(picoquic_quic_t * quic, picoquic_spinbit_version_enum default_spinbit_policy) +int picoquic_set_default_spinbit_policy(picoquic_quic_t * quic, picoquic_spinbit_version_enum default_spinbit_policy) { - quic->default_spin_policy = default_spinbit_policy; + int ret = 0; + + if (default_spinbit_policy <= picoquic_spinbit_on) { + quic->default_spin_policy = default_spinbit_policy; + } + else { + ret = -1; + } + return ret; } -void picoquic_set_spinbit_policy(picoquic_cnx_t* cnx, picoquic_spinbit_version_enum spinbit_policy) +int picoquic_set_spinbit_policy(picoquic_cnx_t* cnx, picoquic_spinbit_version_enum spinbit_policy) { - if (spinbit_policy == picoquic_spinbit_on) { - /* This policy forces using basic all the time! */ - cnx->spin_policy = picoquic_spinbit_basic; - } - else if (spinbit_policy < picoquic_spinbit_on) { + int ret = 0; + + if (spinbit_policy < picoquic_spinbit_on) { cnx->spin_policy = spinbit_policy; } else { - cnx->spin_policy = picoquic_spinbit_null; + ret = -1; } + return ret; } void picoquic_set_default_lossbit_policy(picoquic_quic_t* quic, picoquic_lossbit_version_enum default_lossbit_policy) diff --git a/picoquictest/multipath_test.c b/picoquictest/multipath_test.c index c5029902d..54634ec94 100644 --- a/picoquictest/multipath_test.c +++ b/picoquictest/multipath_test.c @@ -1647,8 +1647,8 @@ int multipath_trace_test_one() * current working directory, and run a basic test scenario */ if (ret == 0) { picoquic_set_binlog(test_ctx->qserver, "."); - picoquic_set_default_spinbit_policy(test_ctx->qserver, picoquic_spinbit_on); - picoquic_set_default_spinbit_policy(test_ctx->qclient, picoquic_spinbit_on); + (void)picoquic_set_default_spinbit_policy(test_ctx->qserver, picoquic_spinbit_on); + (void)picoquic_set_default_spinbit_policy(test_ctx->qclient, picoquic_spinbit_on); picoquic_set_default_lossbit_policy(test_ctx->qserver, picoquic_lossbit_send_receive); picoquic_set_default_lossbit_policy(test_ctx->qclient, picoquic_lossbit_send_receive); test_ctx->qserver->cnx_id_callback_ctx = (void*)&cnxfn_data_server; diff --git a/picoquictest/skip_frame_test.c b/picoquictest/skip_frame_test.c index 5a2cc5db1..44a320d45 100644 --- a/picoquictest/skip_frame_test.c +++ b/picoquictest/skip_frame_test.c @@ -2131,7 +2131,7 @@ int binlog_test() } else { picoquic_set_binlog(quic, "."); - picoquic_set_default_spinbit_policy(quic, picoquic_spinbit_null); + (void)picoquic_set_default_spinbit_policy(quic, picoquic_spinbit_null); struct sockaddr_in saddr; memset(&saddr, 0, sizeof(struct sockaddr_in)); diff --git a/picoquictest/spinbit_test.c b/picoquictest/spinbit_test.c index f3cffeae4..6ce9ffc95 100644 --- a/picoquictest/spinbit_test.c +++ b/picoquictest/spinbit_test.c @@ -59,15 +59,19 @@ int spinbit_test_one(picoquic_spinbit_version_enum spin_policy, picoquic_spinbit if (ret == 0) { /* force spinbit policy as specified, then start */ - picoquic_set_default_spinbit_policy(test_ctx->qserver, spin_policy_server); - picoquic_set_spinbit_policy(test_ctx->cnx_client, spin_policy); - + if (picoquic_set_default_spinbit_policy(test_ctx->qserver, spin_policy_server) != 0 || + picoquic_set_spinbit_policy(test_ctx->cnx_client, spin_policy) != 0) + { + DBG_PRINTF("Invalid policies: %d, %d\n", spin_policy_server, spin_policy); + ret = -1; + } + } + if (ret == 0) { ret = picoquic_start_client_cnx(test_ctx->cnx_client); if (ret != 0) { DBG_PRINTF("%s", "Could not initialize stream zero for the client\n"); } - } if (ret == 0) { @@ -210,7 +214,12 @@ int spinbit_on_test() int spinbit_bad_test() { - return spinbit_test_one(picoquic_spinbit_on, 123456); + int ret = 0; + if (spinbit_test_one(picoquic_spinbit_on, 123456) == 0 || + spinbit_test_one(123455, picoquic_spinbit_null) == 0) { + ret = -1; + } + return ret; } diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 2a6dc06e9..28028b897 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -8515,8 +8515,8 @@ int qlog_trace_test_one(int auto_qlog, int keep_binlog, uint8_t recv_ecn) if (keep_binlog) { picoquic_set_binlog(test_ctx->qserver, "."); } - picoquic_set_default_spinbit_policy(test_ctx->qserver, picoquic_spinbit_on); - picoquic_set_default_spinbit_policy(test_ctx->qclient, picoquic_spinbit_on); + (void)picoquic_set_default_spinbit_policy(test_ctx->qserver, picoquic_spinbit_on); + (void)picoquic_set_default_spinbit_policy(test_ctx->qclient, picoquic_spinbit_on); picoquic_set_default_lossbit_policy(test_ctx->qserver, picoquic_lossbit_send_receive); picoquic_set_default_lossbit_policy(test_ctx->qclient, picoquic_lossbit_send_receive); test_ctx->qserver->cnx_id_callback_ctx = (void*)&cnxfn_data_server; From 1277b9c0508dff19e1ad2fd81b99ae63cf90270f Mon Sep 17 00:00:00 2001 From: huitema Date: Mon, 25 Nov 2024 12:45:35 -0800 Subject: [PATCH 3/4] Fix spin-on test --- UnitTest1/unittest1.cpp | 7 ------- picoquic_t/picoquic_t.c | 1 - picoquictest/picoquictest.h | 1 - picoquictest/spinbit_test.c | 9 ++------- 4 files changed, 2 insertions(+), 16 deletions(-) diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index 4bde538ea..be30b6e8f 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -1412,13 +1412,6 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } - TEST_METHOD(spinbit_on) - { - int ret = spinbit_on_test(); - - Assert::AreEqual(ret, 0); - } - TEST_METHOD(spinbit_null) { int ret = spinbit_null_test(); diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c index 449cbb17b..9b1e3d329 100644 --- a/picoquic_t/picoquic_t.c +++ b/picoquic_t/picoquic_t.c @@ -239,7 +239,6 @@ static const picoquic_test_def_t test_table[] = { { "nat_rebinding_fast", fast_nat_rebinding_test}, { "spinbit", spinbit_test }, { "spinbit_bad", spinbit_bad_test }, - { "spinbit_on", spinbit_on_test }, { "spinbit_null", spinbit_null_test }, { "spinbit_randclient", spinbit_randclient_test }, { "spinbit_random", spinbit_random_test }, diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h index 7e9f4f6b8..4c38c9c2b 100644 --- a/picoquictest/picoquictest.h +++ b/picoquictest/picoquictest.h @@ -158,7 +158,6 @@ int spinbit_test(); int spinbit_random_test(); int spinbit_randclient_test(); int spinbit_null_test(); -int spinbit_on_test(); int spinbit_bad_test(); int loss_bit_test(); int client_error_test(); diff --git a/picoquictest/spinbit_test.c b/picoquictest/spinbit_test.c index 6ce9ffc95..7e4a47ffb 100644 --- a/picoquictest/spinbit_test.c +++ b/picoquictest/spinbit_test.c @@ -155,7 +155,7 @@ int spinbit_test_one(picoquic_spinbit_version_enum spin_policy, picoquic_spinbit if (ret == 0) { if (spin_policy == picoquic_spinbit_basic) { - if (spin_policy_server == picoquic_spinbit_basic) { + if (spin_policy_server == picoquic_spinbit_on) { 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); @@ -189,7 +189,7 @@ int spinbit_test_one(picoquic_spinbit_version_enum spin_policy, picoquic_spinbit int spinbit_test() { - return spinbit_test_one(picoquic_spinbit_basic, picoquic_spinbit_basic); + return spinbit_test_one(picoquic_spinbit_on, picoquic_spinbit_basic); } int spinbit_random_test() @@ -207,11 +207,6 @@ int spinbit_null_test() return spinbit_test_one(picoquic_spinbit_basic, picoquic_spinbit_null); } -int spinbit_on_test() -{ - return spinbit_test_one(picoquic_spinbit_on, picoquic_spinbit_on); -} - int spinbit_bad_test() { int ret = 0; From 6caebd692de96698e18cf45cf63add35b1a7726d Mon Sep 17 00:00:00 2001 From: huitema Date: Mon, 25 Nov 2024 12:50:35 -0800 Subject: [PATCH 4/4] One last spinbit fix --- picoquictest/spinbit_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picoquictest/spinbit_test.c b/picoquictest/spinbit_test.c index 7e4a47ffb..a99ea2a7b 100644 --- a/picoquictest/spinbit_test.c +++ b/picoquictest/spinbit_test.c @@ -189,7 +189,7 @@ int spinbit_test_one(picoquic_spinbit_version_enum spin_policy, picoquic_spinbit int spinbit_test() { - return spinbit_test_one(picoquic_spinbit_on, picoquic_spinbit_basic); + return spinbit_test_one(picoquic_spinbit_basic, picoquic_spinbit_on); } int spinbit_random_test()