diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index a654e6af7..15a93cd86 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -2532,13 +2532,11 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } -#if 0 TEST_METHOD(h09_multi_file_preemptive) { int ret = h09_multi_file_preemptive_test(); Assert::AreEqual(ret, 0); } -#endif TEST_METHOD(h3_multi_file) { int ret = h3_multi_file_test(); @@ -2552,13 +2550,11 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } -#if 0 TEST_METHOD(h3_multi_file_preemptive) { int ret = h3_multi_file_preemptive_test(); Assert::AreEqual(ret, 0); } -#endif TEST_METHOD(h3zero_settings) { int ret = h3zero_settings_test(); diff --git a/picohttp_t/picohttp_t.c b/picohttp_t/picohttp_t.c index bd56bbe30..92eb6570e 100644 --- a/picohttp_t/picohttp_t.c +++ b/picohttp_t/picohttp_t.c @@ -74,12 +74,12 @@ static const picoquic_test_def_t test_table[] = { { "h3_long_file_name", h3_long_file_name_test }, { "h3_multi_file", h3_multi_file_test }, { "h3_multi_file_loss", h3_multi_file_loss_test }, -#if 0 +#if 1 { "h3_multi_file_preemptive", h3_multi_file_preemptive_test }, #endif { "h09_multi_file", h09_multi_file_test }, { "h09_multi_file_loss", h09_multi_file_loss_test }, -#if 0 +#if 1 { "h09_multi_file_preemptive", h09_multi_file_preemptive_test }, #endif { "h3zero_settings", h3zero_settings_test }, diff --git a/picoquic/bbr.c b/picoquic/bbr.c index 7ef8fbec0..70f838462 100644 --- a/picoquic/bbr.c +++ b/picoquic/bbr.c @@ -184,6 +184,30 @@ Hystart instead of startup if the RTT is above the Reno target of * timeout, the timeout will still be handled. */ +/* +* Handling of suspension +* +* After a timeout, the path is suspended, and the congestion window is +* immediately reduced. If do not do anything in particular, the +* suspended state will be cleared on the first next acknowledgement, +* and the congestion window will be restored gradually. +* +* This is correct in general, when the timeout is due to some series +* of packet loss events. It is not so good in the particular case of +* Wi-Fi suspension, when the timeout is caused by the Wi-Fi link +* being "suspended" for the time needed to scan other channels. In that +* case, the code will receive a "spurious time out" notification, +* typically triggered when an ACK queued "in the network" is delivered +* when transmission resume. Waiting for the next ACK has two +* downsides: +* +* - it comes some times later, something like 1/2 RTT to 1 full RTT. +* - the CWIND is lower than if the suspension had not happened. +* +* The reasonable solution is to exit the suspended state upon +* notification of spurious reset, and restore the prior cwin. +*/ + typedef enum { picoquic_bbr_alg_startup = 0, picoquic_bbr_alg_drain, @@ -252,9 +276,9 @@ typedef struct st_picoquic_bbr_state_t { uint64_t previous_sampling_lost; uint64_t loss_interval_start; /* Time in microsec when last loss considered */ uint64_t congestion_sequence; /* sequence number after congestion notification */ + uint64_t cwin_before_suspension; /* So it can be restored if suspension stops. */ uint64_t wifi_shadow_rtt; /* Shadow RTT used for wifi connections. */ - unsigned int filled_pipe : 1; unsigned int round_start : 1; unsigned int rt_prop_expired : 1; @@ -266,6 +290,7 @@ typedef struct st_picoquic_bbr_state_t { unsigned int lt_is_sampling : 1; unsigned int last_loss_was_timeout : 1; unsigned int cycle_on_loss : 1; + unsigned int is_suspended; } picoquic_bbr_state_t; @@ -1022,6 +1047,10 @@ void picoquic_bbr_notify_congestion( } path_x->cwin = path_x->cwin / 2; if (is_timeout || path_x->cwin < PICOQUIC_CWIN_MINIMUM) { + if (!bbr_state->is_suspended) { + bbr_state->is_suspended = 1; + bbr_state->cwin_before_suspension = path_x->cwin; + } path_x->cwin = PICOQUIC_CWIN_MINIMUM; } @@ -1042,6 +1071,25 @@ void picoquic_bbr_notify_congestion( } } +/* +* Exit from suspension, after notification of spurious repeat. +*/ +void picoquic_bbr_exit_suspension( + picoquic_bbr_state_t* bbr_state, + picoquic_cnx_t * cnx, + picoquic_path_t* path_x, + uint64_t lost_packet_number) +{ + if (bbr_state->is_suspended && + bbr_state->cwin_before_suspension > 0) { + if (bbr_state->congestion_sequence >= lost_packet_number) { + bbr_state->is_suspended = 0; + path_x->cwin = bbr_state->cwin_before_suspension; + /* Set the pacing rate in picoquic sender */ + picoquic_update_pacing_rate(cnx, path_x, bbr_state->pacing_rate, bbr_state->send_quantum); + } + } +} /* * In order to implement BBR, we map generic congestion notification @@ -1067,6 +1115,9 @@ static void picoquic_bbr_notify( switch (notification) { case picoquic_congestion_notification_acknowledgement: /* sum the amount of data acked per packet */ + if (bbr_state->is_suspended) { + bbr_state->is_suspended = 0; + } bbr_state->bytes_delivered += nb_bytes_acknowledged; break; case picoquic_congestion_notification_ecn_ec: @@ -1085,6 +1136,9 @@ static void picoquic_bbr_notify( } break; case picoquic_congestion_notification_spurious_repeat: + if (bbr_state->is_suspended) { + picoquic_bbr_exit_suspension(bbr_state, cnx, path_x, lost_packet_number); + } break; case picoquic_congestion_notification_rtt_measurement: if (bbr_state->state == picoquic_bbr_alg_startup && path_x->rtt_min > BBR_HYSTART_THRESHOLD_RTT) { diff --git a/picoquic/loss_recovery.c b/picoquic/loss_recovery.c index 8ecd3c6c2..e2e6edbe4 100644 --- a/picoquic/loss_recovery.c +++ b/picoquic/loss_recovery.c @@ -922,7 +922,7 @@ static void picoquic_count_and_notify_loss( if (cnx->congestion_alg != NULL && cnx->cnx_state >= picoquic_state_ready && old_p->send_path != NULL) { cnx->congestion_alg->alg_notify(cnx, old_p->send_path, (timer_based_retransmit == 0) ? picoquic_congestion_notification_repeat : picoquic_congestion_notification_timeout, - 0, 0, 0, old_p->sequence_number, current_time); + 0, 0, 0, old_p->path_packet_number, current_time); } } } diff --git a/picoquictest/app_limited.c b/picoquictest/app_limited.c index b36453a26..d71b33f0f 100644 --- a/picoquictest/app_limited.c +++ b/picoquictest/app_limited.c @@ -607,8 +607,8 @@ int app_limited_rpr_test() config.ccalgo = picoquic_cubic_algorithm; config.do_preemptive_repeat = 1; config.loss_mask = 0x1482481224818214ull; - config.completion_target = 39000000; - config.nb_losses_max = 1960; + config.completion_target = 44000000; + config.nb_losses_max = 1980; config.rtt_max = 275000; return app_limited_test_one(&config); diff --git a/picoquictest/h3zerotest.c b/picoquictest/h3zerotest.c index 2baec2d75..cc26d92fd 100644 --- a/picoquictest/h3zerotest.c +++ b/picoquictest/h3zerotest.c @@ -2577,15 +2577,17 @@ int h3_multi_file_test() return http_multi_file_test_one(PICOHTTP_ALPN_H3_LATEST, h3zero_callback, 0, 0); } +#define H3ZERO_MULTI_LOSS_PATTERN 0xa242EDB710000ull + int h3_multi_file_loss_test() { - uint64_t loss_pattern = 0xa243FFB700000ull; + uint64_t loss_pattern = H3ZERO_MULTI_LOSS_PATTERN; return http_multi_file_test_one(PICOHTTP_ALPN_H3_LATEST, h3zero_callback, loss_pattern, 0); } int h3_multi_file_preemptive_test() { - uint64_t loss_pattern = 0xa243FFB700000ull; + uint64_t loss_pattern = H3ZERO_MULTI_LOSS_PATTERN; return http_multi_file_test_one(PICOHTTP_ALPN_H3_LATEST, h3zero_callback, loss_pattern, 1); } @@ -2596,14 +2598,14 @@ int h09_multi_file_test() int h09_multi_file_loss_test() { - uint64_t loss_pattern = 0xa243FFB700000ull; + uint64_t loss_pattern = H3ZERO_MULTI_LOSS_PATTERN; return http_multi_file_test_one(PICOHTTP_ALPN_HQ_LATEST, picoquic_h09_server_callback, loss_pattern, 0); } int h09_multi_file_preemptive_test() { - uint64_t loss_pattern = 0xa243FFB700000ull; + uint64_t loss_pattern = H3ZERO_MULTI_LOSS_PATTERN; return http_multi_file_test_one(PICOHTTP_ALPN_HQ_LATEST, picoquic_h09_server_callback, loss_pattern, 1); } diff --git a/picoquictest/path_quality_server_ref.txt b/picoquictest/path_quality_server_ref.txt index a8a4607c0..67b1667f1 100644 --- a/picoquictest/path_quality_server_ref.txt +++ b/picoquictest/path_quality_server_ref.txt @@ -59,61 +59,71 @@ Time, Path-ID, Event, Pacing_rate, Receive Rate, CWIN, RTT 158446, 1, 23, 3614021, 40507, 77262, 26723 163050, 1, 23, 3665388, 40507, 82958, 28291 168805, 1, 23, 2941418, 40507, 90078, 30624 -169956, 0, 23, 533321, 42902, 10666, 39455 171109, 1, 23, 2858869, 40507, 90123, 31524 173413, 1, 23, 2808740, 40507, 90169, 32103 174564, 1, 23, 2753587, 40507, 90191, 32754 +174564, 0, 23, 1056260, 42902, 42130, 39886 176866, 1, 23, 2656764, 40507, 90237, 33965 179164, 1, 23, 2567601, 40507, 90282, 35162 181466, 1, 23, 2485088, 40507, 90328, 36348 184443, 1, 23, 2416002, 40507, 90373, 37406 -186745, 1, 23, 595605, 40507, 22689, 38094 -199250, 0, 23, 533321, 42902, 6977, 33994 -287383, 0, 23, 392761, 12023, 12024, 30614 -299345, 1, 23, 595605, 11296, 10918, 32831 -319353, 1, 23, 360238, 8069, 11827, 32831 -319353, 0, 23, 449047, 8481, 13017, 28988 -345568, 1, 23, 410899, 8595, 12991, 31616 -345568, 0, 23, 505148, 7927, 14228, 28166 -369590, 0, 23, 561438, 9321, 15343, 27328 -374800, 1, 23, 484828, 8595, 14205, 29299 -391794, 0, 23, 616592, 9321, 16381, 26567 -403600, 1, 23, 545438, 9432, 15455, 28335 -409378, 0, 23, 667756, 9731, 17357, 25993 -431882, 0, 23, 728704, 9445, 18504, 25393 -431882, 1, 23, 605624, 9548, 16734, 27631 -447334, 0, 23, 789339, 9445, 19267, 24409 -447334, 1, 23, 669222, 9723, 17456, 26084 -457389, 1, 23, 723233, 9723, 18036, 24938 -468865, 0, 23, 851817, 10406, 20407, 23957 -480128, 1, 23, 781688, 10781, 19142, 24488 -485406, 0, 23, 910895, 10054, 21294, 23377 -490684, 1, 23, 833902, 9815, 19776, 23715 -506343, 1, 23, 894211, 9815, 20591, 23027 -506343, 0, 23, 968222, 12401, 22425, 23161 -522446, 0, 23, 1022761, 12401, 23411, 22890 -527510, 1, 23, 958408, 10490, 21661, 22601 -548661, 0, 23, 1077080, 10490, 24775, 23002 -553725, 1, 23, 1010882, 10490, 23129, 22880 -585235, 1, 23, 1064717, 10490, 24842, 23332 -585235, 0, 23, 1127672, 10490, 26692, 23670 -616481, 1, 23, 1122839, 10490, 26444, 23551 -616481, 0, 23, 1187942, 10490, 28336, 23853 -647974, 1, 23, 1173473, 10476, 28100, 23946 -647974, 0, 23, 1242678, 10490, 29956, 24106 -679736, 1, 23, 1223901, 10490, 29802, 24350 -679736, 0, 23, 1297093, 10490, 31688, 24430 -711015, 1, 23, 1274115, 10490, 31542, 24756 -782580, 1, 23, 1327607, 5087, 33623, 25326 -782580, 0, 23, 1356542, 13157, 34836, 25680 -799845, 1, 23, 1381838, 9556, 34528, 24987 -799845, 0, 23, 1411590, 9556, 35709, 25297 -817110, 1, 23, 1433812, 9556, 35408, 24695 -914945, 0, 23, 1463688, 9556, 41055, 28049 -955230, 1, 23, 1485795, 9556, 41788, 28125 -960985, 0, 23, 1520684, 9556, 43008, 28282 -972495, 0, 23, 1466014, 9556, 43482, 29660 -995515, 0, 23, 1409047, 11149, 44416, 31522 -1007025, 1, 23, 1431288, 9556, 43942, 30701 -1270480, 1, 22, -1270480, 0, 22, +186745, 1, 23, 1187667, 40507, 45243, 38094 +194931, 0, 23, 1130382, 42902, 43002, 38042 +196082, 0, 23, 1191832, 42902, 43049, 36120 +198384, 0, 23, 1247614, 42902, 43145, 34582 +201842, 0, 23, 1333015, 48819, 43287, 32473 +206451, 0, 23, 1384630, 48819, 43476, 31399 +209904, 1, 23, 1136925, 46044, 46141, 40584 +211053, 0, 23, 1435371, 48819, 43664, 30420 +215649, 0, 23, 1491818, 48819, 43852, 29395 +221404, 0, 23, 1548417, 48819, 44085, 28471 +224857, 1, 23, 1187816, 46044, 46718, 39331 +225290, 1, 23, 1252162, 46044, 46762, 37345 +226441, 1, 23, 1309194, 40006, 46805, 35751 +228743, 1, 23, 1364914, 40006, 46893, 34356 +228743, 0, 23, 1599048, 48819, 44364, 27744 +240287, 1, 23, 1429244, 40006, 47328, 33114 +257552, 1, 23, 1501063, 40006, 47974, 31960 +269062, 0, 23, 1545451, 9545, 45954, 29735 +274817, 1, 23, 1573324, 11933, 48611, 30897 +286327, 0, 23, 1489139, 9545, 46619, 31306 +292082, 1, 23, 1631273, 11933, 49240, 30185 +309390, 1, 23, 1577430, 9556, 49861, 31609 +326698, 1, 23, 1520331, 9556, 50475, 33200 +338208, 1, 23, 1463288, 9533, 50880, 34771 +361314, 1, 23, 1408393, 9533, 51681, 36695 +378622, 1, 23, 699846, 9536, 26429, 37764 +401685, 0, 23, 1437873, 9533, 50836, 35355 +407234, 1, 23, 771265, 9536, 28307, 36702 +430252, 1, 23, 835477, 9585, 29164, 34907 +430252, 0, 23, 1383927, 9536, 51835, 37455 +441762, 1, 23, 887392, 9585, 29583, 33337 +458819, 1, 23, 947001, 9557, 30269, 31963 +469594, 1, 23, 1037510, 9557, 30674, 29565 +469594, 0, 23, 697099, 9606, 26869, 38544 +479953, 1, 23, 1108328, 10024, 31072, 28035 +490312, 1, 23, 1159948, 10024, 31597, 27240 +490312, 0, 23, 748203, 10156, 28211, 37705 +500947, 0, 23, 807188, 10156, 28790, 35667 +500947, 1, 23, 1210409, 10024, 32048, 26477 +511349, 0, 23, 872848, 10156, 29003, 33228 +521708, 0, 23, 950232, 10156, 29003, 30522 +527212, 1, 23, 1261732, 10464, 32048, 25400 +558135, 0, 23, 1014932, 12976, 29635, 29199 +580947, 1, 23, 1322551, 5957, 33856, 25599 +580947, 0, 23, 1082180, 12976, 30656, 28328 +598212, 1, 23, 1378127, 9626, 34755, 25219 +603967, 0, 23, 1151482, 9626, 31774, 27594 +620375, 0, 23, 1205620, 9626, 32477, 26938 +620375, 1, 23, 1428554, 9626, 35861, 25103 +630734, 0, 23, 1258417, 9626, 33041, 26256 +653797, 0, 23, 1328916, 10194, 34201, 25736 +676609, 0, 23, 1395504, 9626, 35323, 25312 +699629, 0, 23, 1460801, 9542, 36466, 24963 +751424, 1, 23, 1481518, 9556, 41884, 28271 +808974, 1, 23, 1423298, 9556, 44266, 31101 +837792, 0, 23, 1512989, 11468, 42689, 28215 +866610, 0, 23, 1450110, 9542, 43876, 30257 +895428, 0, 23, 1395920, 9533, 45031, 32259 +1064552, 1, 22, +1064552, 0, 22, diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index a13b4ebf7..607b2a530 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -4744,7 +4744,7 @@ int mtu_drop_test() picoquic_bbr_algorithm }; uint64_t algo_time[5] = { - 11100000, + 11600000, 9450000, 9200000, 11400000,