From c9962258d82470cfe3ed46c5550718632db18be9 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 8 Apr 2024 18:59:54 +0900 Subject: [PATCH 1/8] improve handling of CID retirement, by: * change the capacity of CID retirement queue from max_cids to 2 * max_cids * upon overflow, emit PROTOCOL_VIOLATION rather than dropping RETIRE_CID frames * merge remote_cid and retire_cid structs for simplicity --- CMakeLists.txt | 2 - include/quicly/remote_cid.h | 31 ++++++++-- include/quicly/retire_cid.h | 64 --------------------- lib/quicly.c | 77 ++++++++++++------------- lib/remote_cid.c | 61 ++++++++++++++------ lib/retire_cid.c | 55 ------------------ quicly.xcodeproj/project.pbxproj | 18 ------ t/remote_cid.c | 73 +++++++++++++----------- t/retire_cid.c | 97 -------------------------------- t/test.c | 1 - t/test.h | 1 - 11 files changed, 146 insertions(+), 334 deletions(-) delete mode 100644 include/quicly/retire_cid.h delete mode 100644 lib/retire_cid.c delete mode 100644 t/retire_cid.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 4d00892a0..a038e2289 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,7 +59,6 @@ SET(QUICLY_LIBRARY_FILES lib/rate.c lib/recvstate.c lib/remote_cid.c - lib/retire_cid.c lib/sendstate.c lib/sentmap.c lib/streambuf.c @@ -77,7 +76,6 @@ SET(UNITTEST_SOURCE_FILES t/ranges.c t/rate.c t/remote_cid.c - t/retire_cid.c t/sentmap.c t/simple.c t/stream-concurrency.c diff --git a/include/quicly/remote_cid.h b/include/quicly/remote_cid.h index 3be8c180d..e848098cd 100644 --- a/include/quicly/remote_cid.h +++ b/include/quicly/remote_cid.h @@ -84,6 +84,13 @@ typedef struct st_quicly_remote_cid_set_t { * we expect to receive CIDs with sequence number smaller than or equal to this number */ uint64_t _largest_sequence_expected; + /** + * queue containing CID sequence numbers that should be sent using RETIRE_CONNECTION_ID frames + */ + struct { + uint64_t cids[QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT * 2]; + size_t count; + } retired; } quicly_remote_cid_set_t; /** @@ -93,16 +100,30 @@ typedef struct st_quicly_remote_cid_set_t { */ void quicly_remote_cid_init_set(quicly_remote_cid_set_t *set, ptls_iovec_t *initial_cid, void (*random_bytes)(void *, size_t)); /** - * registers received connection ID + * registers received connection ID at the same time pushing CIDs to the retired queue, if any * returns 0 if successful (registered or ignored because of duplication/stale information), transport error code otherwise */ int quicly_remote_cid_register(quicly_remote_cid_set_t *set, uint64_t sequence, const uint8_t *cid, size_t cid_len, - const uint8_t srt[QUICLY_STATELESS_RESET_TOKEN_LEN], uint64_t retire_prior_to, - uint64_t unregistered_seqs[QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT], size_t *num_unregistered_seqs); + const uint8_t srt[QUICLY_STATELESS_RESET_TOKEN_LEN], uint64_t retire_prior_to); /** - * unregisters specified CID from the store + * unregisters specified CID from the store; the unregistered CID is pushed onto the retired queue */ -void quicly_remote_cid_unregister(quicly_remote_cid_set_t *set, uint64_t sequence); +int quicly_remote_cid_unregister(quicly_remote_cid_set_t *set, uint64_t sequence); +/** + * pushes a CID sequence number to the retired queue + */ +int quicly_remote_cid_push_retired(quicly_remote_cid_set_t *set, uint64_t sequence); +/** + * flushes the retire queue + */ +static void quicly_remote_cid_clear_retired(quicly_remote_cid_set_t *set); + +/* inline definitions */ + +inline void quicly_remote_cid_clear_retired(quicly_remote_cid_set_t *set) +{ + set->retired.count = 0; +} #ifdef __cplusplus } diff --git a/include/quicly/retire_cid.h b/include/quicly/retire_cid.h deleted file mode 100644 index 72ef60865..000000000 --- a/include/quicly/retire_cid.h +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright (c) 2020 Fastly, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to - * deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or - * sell copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ -#ifndef quicly_retire_cid_h -#define quicly_retire_cid_h - -#include "quicly/cid.h" - -#ifdef __cplusplus -extern "C" { -#endif - -/** - * up to how many RETIRE_CONNECTION_IDs to keep for retransmission - */ -#define QUICLY_RETIRE_CONNECTION_ID_LIMIT (QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT * 2) - -typedef struct st_quicly_retire_cid_set_t quicly_retire_cid_set_t; - -struct st_quicly_retire_cid_set_t { - /** - * sequence numbers to ask for retirement - * Valid entries are packed in the front of the array with FIFO manner. - */ - uint64_t sequences[QUICLY_RETIRE_CONNECTION_ID_LIMIT]; - /** - * number of pending sequence numbers - */ - size_t _num_pending; -}; - -void quicly_retire_cid_init(quicly_retire_cid_set_t *set); -void quicly_retire_cid_push(quicly_retire_cid_set_t *set, uint64_t sequence); -void quicly_retire_cid_shift(quicly_retire_cid_set_t *set, size_t num_shift); -static size_t quicly_retire_cid_get_num_pending(const quicly_retire_cid_set_t *set); - -inline size_t quicly_retire_cid_get_num_pending(const quicly_retire_cid_set_t *set) -{ - return set->_num_pending; -} - -#ifdef __cplusplus -} -#endif - -#endif diff --git a/lib/quicly.c b/lib/quicly.c index 5cc2685bd..67d62559f 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -42,7 +42,6 @@ #if QUICLY_USE_DTRACE #include "quicly-probes.h" #endif -#include "quicly/retire_cid.h" #define QUICLY_TLS_EXTENSION_TYPE_TRANSPORT_PARAMETERS_FINAL 0x39 #define QUICLY_TLS_EXTENSION_TYPE_TRANSPORT_PARAMETERS_DRAFT 0xffa5 @@ -406,10 +405,6 @@ struct st_quicly_conn_t { * */ uint8_t try_jumpstart : 1; - /** - * pending RETIRE_CONNECTION_ID frames to be sent - */ - quicly_retire_cid_set_t retire_cid; /** * payload of DATAGRAM frames to be sent */ @@ -1015,27 +1010,17 @@ static size_t local_cid_size(const quicly_conn_t *conn) } /** - * set up an internal record to send RETIRE_CONNECTION_ID frame later + * Resets CIDs associated to paths if they are being retired. To maximize the chance of having enough number of CIDs to run all + * paths when new CIDs are provided through multiple NCID frames possibly scattered over multiple packets, CIDs are reassigned to + * the paths lazily. */ -static void schedule_retire_connection_id_frame(quicly_conn_t *conn, uint64_t sequence) +static void dissociate_cid(quicly_conn_t *conn, uint64_t sequence) { - quicly_retire_cid_push(&conn->egress.retire_cid, sequence); - conn->egress.pending_flows |= QUICLY_PENDING_FLOW_OTHERS_BIT; -} - -static void retire_connection_id(quicly_conn_t *conn, uint64_t sequence) -{ - /* Reset path CIDs that are being retired. To maximize the chance of having enough number of CIDs to run all paths when new CIDs - * are provided through multiple NCID frames possibly scattered over multiple packets, CIDs are reassigned to the paths lazily. - */ for (size_t i = 0; i < PTLS_ELEMENTSOF(conn->paths); ++i) { struct st_quicly_conn_path_t *path = conn->paths[i]; if (path != NULL && path->dcid == sequence) path->dcid = UINT64_MAX; } - - quicly_remote_cid_unregister(&conn->super.remote.cid_set, sequence); - schedule_retire_connection_id_frame(conn, sequence); } static int write_crypto_data(quicly_conn_t *conn, ptls_buffer_t *tlsbuf, size_t epoch_offsets[5]) @@ -1840,9 +1825,10 @@ static int new_path(quicly_conn_t *conn, size_t path_index, struct sockaddr *rem * if is_promote is set, paths[0] (the default path) is freed and the path specified by `path_index` is promoted * if is_promote is not_set, paths[path_index] is freed */ -static void delete_path(quicly_conn_t *conn, int is_promote, size_t path_index) +static int delete_path(quicly_conn_t *conn, int is_promote, size_t path_index) { struct st_quicly_conn_path_t *path; + int ret; /* fetch and detatch the path object to be freed */ if (is_promote) { @@ -1862,9 +1848,18 @@ static void delete_path(quicly_conn_t *conn, int is_promote, size_t path_index) } /* deinstantiate */ - if (path->dcid != UINT64_MAX && conn->super.remote.cid_set.cids[0].cid.len != 0) - retire_connection_id(conn, path->dcid); + if (path->dcid != UINT64_MAX && conn->super.remote.cid_set.cids[0].cid.len != 0) { + uint64_t cid = path->dcid; + dissociate_cid(conn, cid); + if ((ret = quicly_remote_cid_unregister(&conn->super.remote.cid_set, cid)) != 0) + return ret; + assert(conn->super.remote.cid_set.retired.count != 0); + conn->egress.pending_flows |= QUICLY_PENDING_FLOW_OTHERS_BIT; + } + free(path); + + return 0; } static int open_path(quicly_conn_t *conn, size_t *path_index, struct sockaddr *remote_addr, struct sockaddr *local_addr) @@ -1888,8 +1883,8 @@ static int open_path(quicly_conn_t *conn, size_t *path_index, struct sockaddr *r return QUICLY_ERROR_PACKET_IGNORED; /* free existing path info */ - if (conn->paths[*path_index] != NULL) - delete_path(conn, 0, *path_index); + if (conn->paths[*path_index] != NULL && (ret = delete_path(conn, 0, *path_index)) != 0) + return ret; /* initialize new path info */ if ((ret = new_path(conn, *path_index, remote_addr, local_addr)) != 0) @@ -2513,7 +2508,6 @@ static quicly_conn_t *create_connection(quicly_context_t *ctx, uint32_t protocol quicly_pacer_reset(conn->egress.pacer); } conn->egress.ecn.state = conn->super.ctx->enable_ecn ? QUICLY_ECN_PROBING : QUICLY_ECN_OFF; - quicly_retire_cid_init(&conn->egress.retire_cid); quicly_linklist_init(&conn->egress.pending_streams.blocked.uni); quicly_linklist_init(&conn->egress.pending_streams.blocked.bidi); quicly_linklist_init(&conn->egress.pending_streams.control); @@ -3281,9 +3275,13 @@ static int on_ack_retire_connection_id(quicly_sentmap_t *map, const quicly_sent_ { quicly_conn_t *conn = (quicly_conn_t *)((char *)map - offsetof(quicly_conn_t, egress.loss.sentmap)); uint64_t sequence = sent->data.retire_connection_id.sequence; + int ret; - if (!acked) - schedule_retire_connection_id_frame(conn, sequence); + if (!acked) { + if ((ret = quicly_remote_cid_push_retired(&conn->super.remote.cid_set, sequence)) != 0) + return ret; + conn->egress.pending_flows |= QUICLY_PENDING_FLOW_OTHERS_BIT; + } return 0; } @@ -5115,15 +5113,12 @@ static int send_other_control_frames(quicly_conn_t *conn, quicly_send_context_t } { /* RETIRE_CONNECTION_ID */ - size_t i, size = quicly_retire_cid_get_num_pending(&conn->egress.retire_cid); - for (i = 0; i < size; i++) { - uint64_t sequence = conn->egress.retire_cid.sequences[i]; + for (size_t i = 0; i < conn->super.remote.cid_set.retired.count; ++i) { + uint64_t sequence = conn->super.remote.cid_set.retired.cids[i]; if ((ret = send_retire_connection_id(conn, s, sequence)) != 0) - break; + return ret; } - quicly_retire_cid_shift(&conn->egress.retire_cid, i); - if (ret != 0) - return ret; + quicly_remote_cid_clear_retired(&conn->super.remote.cid_set); } return 0; @@ -6478,15 +6473,15 @@ static int handle_new_connection_id_frame(quicly_conn_t *conn, struct st_quicly_ PTLS_LOG_ELEMENT_HEXDUMP(stateless_reset_token, frame.stateless_reset_token, QUICLY_STATELESS_RESET_TOKEN_LEN); }); - uint64_t unregistered_seqs[QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT]; - size_t num_unregistered_seqs; + size_t orig_num_retired = conn->super.remote.cid_set.retired.count; if ((ret = quicly_remote_cid_register(&conn->super.remote.cid_set, frame.sequence, frame.cid.base, frame.cid.len, - frame.stateless_reset_token, frame.retire_prior_to, unregistered_seqs, - &num_unregistered_seqs)) != 0) + frame.stateless_reset_token, frame.retire_prior_to)) != 0) return ret; - - for (size_t i = 0; i < num_unregistered_seqs; i++) - retire_connection_id(conn, unregistered_seqs[i]); + if (orig_num_retired != conn->super.remote.cid_set.retired.count) { + for (size_t i = orig_num_retired; i < conn->super.remote.cid_set.retired.count; ++i) + dissociate_cid(conn, conn->super.remote.cid_set.retired.cids[i]); + conn->egress.pending_flows |= QUICLY_PENDING_FLOW_OTHERS_BIT; + } return 0; } diff --git a/lib/remote_cid.c b/lib/remote_cid.c index ff7f57c37..06c771081 100644 --- a/lib/remote_cid.c +++ b/lib/remote_cid.c @@ -20,6 +20,7 @@ * IN THE SOFTWARE. */ #include +#include #include "quicly/constants.h" #include "quicly/remote_cid.h" @@ -44,6 +45,7 @@ void quicly_remote_cid_init_set(quicly_remote_cid_set_t *set, ptls_iovec_t *init }; set->_largest_sequence_expected = PTLS_ELEMENTSOF(set->cids) - 1; + memset(&set->retired, 0, sizeof(set->retired)); } static int do_register(quicly_remote_cid_set_t *set, uint64_t sequence, const uint8_t *cid, size_t cid_len, @@ -91,41 +93,44 @@ static int do_register(quicly_remote_cid_set_t *set, uint64_t sequence, const ui return 0; } -static void do_unregister(quicly_remote_cid_set_t *set, size_t idx_to_unreg) +static int do_unregister(quicly_remote_cid_set_t *set, size_t idx_to_unreg) { + int ret; + + if ((ret = quicly_remote_cid_push_retired(set, set->cids[idx_to_unreg].sequence)) != 0) + return ret; + set->cids[idx_to_unreg].state = QUICLY_REMOTE_CID_UNAVAILABLE; set->cids[idx_to_unreg].sequence = ++set->_largest_sequence_expected; + + return 0; } -void quicly_remote_cid_unregister(quicly_remote_cid_set_t *set, uint64_t sequence) +int quicly_remote_cid_unregister(quicly_remote_cid_set_t *set, uint64_t sequence) { for (size_t i = 0; i < PTLS_ELEMENTSOF(set->cids); i++) { - if (sequence == set->cids[i].sequence) { - do_unregister(set, i); - return; - } + if (sequence == set->cids[i].sequence) + return do_unregister(set, i); } assert(!"invalid CID sequence number"); } -static size_t unregister_prior_to(quicly_remote_cid_set_t *set, uint64_t seq_unreg_prior_to, - uint64_t unregistered_seqs[QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT]) +static int unregister_prior_to(quicly_remote_cid_set_t *set, uint64_t seq_unreg_prior_to) { - size_t num_unregistered = 0; + int ret; for (size_t i = 0; i < PTLS_ELEMENTSOF(set->cids); i++) { - if (set->cids[i].sequence < seq_unreg_prior_to) { - unregistered_seqs[num_unregistered++] = set->cids[i].sequence; - do_unregister(set, i); + while (set->cids[i].sequence < seq_unreg_prior_to) { + if ((ret = do_unregister(set, i)) != 0) + return ret; } } - return num_unregistered; + return 0; } int quicly_remote_cid_register(quicly_remote_cid_set_t *set, uint64_t sequence, const uint8_t *cid, size_t cid_len, - const uint8_t srt[QUICLY_STATELESS_RESET_TOKEN_LEN], uint64_t retire_prior_to, - uint64_t unregistered_seqs[QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT], size_t *num_unregistered_seqs) + const uint8_t srt[QUICLY_STATELESS_RESET_TOKEN_LEN], uint64_t retire_prior_to) { quicly_remote_cid_set_t backup = *set; /* preserve current state so that it can be restored to notify protocol violation */ int ret; @@ -134,7 +139,8 @@ int quicly_remote_cid_register(quicly_remote_cid_set_t *set, uint64_t sequence, /* First, handle retire_prior_to. This order is important as it is possible to receive a NEW_CONNECTION_ID frame such that it * retires active_connection_id_limit CIDs and then installs one new CID. */ - *num_unregistered_seqs = unregister_prior_to(set, retire_prior_to, unregistered_seqs); + if ((ret = unregister_prior_to(set, retire_prior_to)) != 0) + return ret; /* Then, register given value. If an error occurs, restore the backup and send the error. */ if ((ret = do_register(set, sequence, cid, cid_len, srt)) != 0) { @@ -144,3 +150,26 @@ int quicly_remote_cid_register(quicly_remote_cid_set_t *set, uint64_t sequence, return ret; } + +int quicly_remote_cid_push_retired(quicly_remote_cid_set_t *set, uint64_t sequence) +{ + /* do nothing if given sequence is already registered */ + for (size_t i = 0; i < set->retired.count; ++i) { + if (set->retired.cids[i] == sequence) + return 0; + } + + if (set->retired.count >= PTLS_ELEMENTSOF(set->retired.cids)) + return QUICLY_TRANSPORT_ERROR_PROTOCOL_VIOLATION; + set->retired.cids[set->retired.count++] = sequence; + return 0; +} + +void quicly_remote_cid_shift_retired(quicly_remote_cid_set_t *set, size_t count) +{ + assert(count <= set->retired.count); + + set->retired.count -= count; + for (size_t i = 0; i < count; ++i) + set->retired.cids[i] = set->retired.cids[i + count]; +} diff --git a/lib/retire_cid.c b/lib/retire_cid.c deleted file mode 100644 index b0f291207..000000000 --- a/lib/retire_cid.c +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright (c) 2020 Fastly, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to - * deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or - * sell copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ - -#include "quicly/retire_cid.h" - -void quicly_retire_cid_init(quicly_retire_cid_set_t *set) -{ - set->_num_pending = 0; -} - -void quicly_retire_cid_push(quicly_retire_cid_set_t *set, uint64_t sequence) -{ - if (set->_num_pending == PTLS_ELEMENTSOF(set->sequences)) { - /* in case we don't find an empty slot, we'll just drop this sequence (never send RETIRE_CONNECTION_ID frame) */ - return; - } - - for (size_t i = 0; i < set->_num_pending; i++) { - if (set->sequences[i] == sequence) { - /* already scheduled */ - return; - } - } - - set->sequences[set->_num_pending] = sequence; - set->_num_pending++; -} - -void quicly_retire_cid_shift(quicly_retire_cid_set_t *set, size_t num_shift) -{ - assert(num_shift <= PTLS_ELEMENTSOF(set->sequences)); - assert(num_shift <= set->_num_pending); - /* move the remaining pending sequence numbers to the front */ - memmove(set->sequences, set->sequences + num_shift, sizeof(set->sequences[0]) * (set->_num_pending - num_shift)); - set->_num_pending -= num_shift; -} diff --git a/quicly.xcodeproj/project.pbxproj b/quicly.xcodeproj/project.pbxproj index 2b63dfed7..26acd3902 100644 --- a/quicly.xcodeproj/project.pbxproj +++ b/quicly.xcodeproj/project.pbxproj @@ -11,7 +11,6 @@ 082195092683498900E3EFCF /* cc-pico.c in Sources */ = {isa = PBXBuildFile; fileRef = 082195072683498900E3EFCF /* cc-pico.c */; }; 0821950A2683498900E3EFCF /* cc-pico.c in Sources */ = {isa = PBXBuildFile; fileRef = 082195072683498900E3EFCF /* cc-pico.c */; }; 0829876726D372B70053638F /* cc-cubic.c in Sources */ = {isa = PBXBuildFile; fileRef = E9DF012324E4BAC20002EEC7 /* cc-cubic.c */; }; - 0829876826D372B70053638F /* retire_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736528246FD3AC0039AA49 /* retire_cid.c */; }; 0829876926D372B70053638F /* picotls-probes.d in Sources */ = {isa = PBXBuildFile; fileRef = E95E953A2290498E00215ACD /* picotls-probes.d */; }; 0829876A26D372B70053638F /* ranges.c in Sources */ = {isa = PBXBuildFile; fileRef = E9F6A41F1F3C0B6D0083F0B2 /* ranges.c */; }; 0829876B26D372B70053638F /* cc-reno.c in Sources */ = {isa = PBXBuildFile; fileRef = E98041C522383C62008B9745 /* cc-reno.c */; }; @@ -80,19 +79,14 @@ E9736523246FD3890039AA49 /* cid.h in Headers */ = {isa = PBXBuildFile; fileRef = E973651F246FD3880039AA49 /* cid.h */; }; E9736524246FD3890039AA49 /* local_cid.h in Headers */ = {isa = PBXBuildFile; fileRef = E9736520246FD3880039AA49 /* local_cid.h */; }; E9736525246FD3890039AA49 /* remote_cid.h in Headers */ = {isa = PBXBuildFile; fileRef = E9736521246FD3890039AA49 /* remote_cid.h */; }; - E9736526246FD3890039AA49 /* retire_cid.h in Headers */ = {isa = PBXBuildFile; fileRef = E9736522246FD3890039AA49 /* retire_cid.h */; }; E973652A246FD3AC0039AA49 /* remote_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736527246FD3AC0039AA49 /* remote_cid.c */; }; - E973652B246FD3AC0039AA49 /* retire_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736528246FD3AC0039AA49 /* retire_cid.c */; }; E973652C246FD3AC0039AA49 /* local_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736529246FD3AC0039AA49 /* local_cid.c */; }; E973652D246FD3B40039AA49 /* local_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736529246FD3AC0039AA49 /* local_cid.c */; }; E973652E246FD3B40039AA49 /* remote_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736527246FD3AC0039AA49 /* remote_cid.c */; }; - E973652F246FD3B40039AA49 /* retire_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736528246FD3AC0039AA49 /* retire_cid.c */; }; E9736530246FD3B50039AA49 /* local_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736529246FD3AC0039AA49 /* local_cid.c */; }; E9736531246FD3B50039AA49 /* remote_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736527246FD3AC0039AA49 /* remote_cid.c */; }; - E9736532246FD3B50039AA49 /* retire_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736528246FD3AC0039AA49 /* retire_cid.c */; }; E9736536246FD3DA0039AA49 /* local_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736533246FD3DA0039AA49 /* local_cid.c */; }; E9736537246FD3DA0039AA49 /* remote_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736534246FD3DA0039AA49 /* remote_cid.c */; }; - E9736538246FD3DA0039AA49 /* retire_cid.c in Sources */ = {isa = PBXBuildFile; fileRef = E9736535246FD3DA0039AA49 /* retire_cid.c */; }; E98041C422383C5C008B9745 /* cc.h in Headers */ = {isa = PBXBuildFile; fileRef = E98041C322383C5C008B9745 /* cc.h */; }; E98041C622383C62008B9745 /* cc-reno.c in Sources */ = {isa = PBXBuildFile; fileRef = E98041C522383C62008B9745 /* cc-reno.c */; }; E98041C722383C7A008B9745 /* cc-reno.c in Sources */ = {isa = PBXBuildFile; fileRef = E98041C522383C62008B9745 /* cc-reno.c */; }; @@ -221,13 +215,10 @@ E973651F246FD3880039AA49 /* cid.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = cid.h; sourceTree = ""; }; E9736520246FD3880039AA49 /* local_cid.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = local_cid.h; sourceTree = ""; }; E9736521246FD3890039AA49 /* remote_cid.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = remote_cid.h; sourceTree = ""; }; - E9736522246FD3890039AA49 /* retire_cid.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = retire_cid.h; sourceTree = ""; }; E9736527246FD3AC0039AA49 /* remote_cid.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = remote_cid.c; sourceTree = ""; }; - E9736528246FD3AC0039AA49 /* retire_cid.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = retire_cid.c; sourceTree = ""; }; E9736529246FD3AC0039AA49 /* local_cid.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = local_cid.c; sourceTree = ""; }; E9736533246FD3DA0039AA49 /* local_cid.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = local_cid.c; sourceTree = ""; }; E9736534246FD3DA0039AA49 /* remote_cid.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = remote_cid.c; sourceTree = ""; }; - E9736535246FD3DA0039AA49 /* retire_cid.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = retire_cid.c; sourceTree = ""; }; E98041C322383C5C008B9745 /* cc.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = cc.h; sourceTree = ""; }; E98041C522383C62008B9745 /* cc-reno.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = "cc-reno.c"; sourceTree = ""; }; E980421A223952D2008B9745 /* echo.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = echo.c; sourceTree = ""; }; @@ -400,7 +391,6 @@ E9F6A41F1F3C0B6D0083F0B2 /* ranges.c */, 0829878D26E03D4D0053638F /* rate.c */, E920D21D1F43E05000799777 /* recvstate.c */, - E9736528246FD3AC0039AA49 /* retire_cid.c */, E9F6A42D1F41375B0083F0B2 /* sendstate.c */, E920D22A1F49533800799777 /* sentmap.c */, E9D3CCCE21D22F4300516202 /* streambuf.c */, @@ -489,7 +479,6 @@ E9F6A4291F3C3B7B0083F0B2 /* ranges.c */, 0829879126E0A9DF0053638F /* rate.c */, E9736534246FD3DA0039AA49 /* remote_cid.c */, - E9736535246FD3DA0039AA49 /* retire_cid.c */, E920D22D1F4981E500799777 /* sentmap.c */, E920D2241F49412900799777 /* simple.c */, E99B75E81F5D259400CF503E /* stream-concurrency.c */, @@ -527,7 +516,6 @@ E9F6A4261F3C3B050083F0B2 /* ranges.h */, 0829878C26DF775B0053638F /* rate.h */, E920D21B1F43DE4100799777 /* recvstate.h */, - E9736522246FD3890039AA49 /* retire_cid.h */, E92D43EC1F41DCF5002AC767 /* sendstate.h */, E920D2281F4951BA00799777 /* sentmap.h */, E9D3CCCC21D22E7E00516202 /* streambuf.h */, @@ -558,7 +546,6 @@ E98041C422383C5C008B9745 /* cc.h in Headers */, E9CC44111EB85E0B00DC7D3E /* khash.h in Headers */, E920D2231F4536CB00799777 /* maxsender.h in Headers */, - E9736526246FD3890039AA49 /* retire_cid.h in Headers */, 085AB4D726427E9C00DF5209 /* pacer.h in Headers */, E920D2291F4951BA00799777 /* sentmap.h in Headers */, E984482C1EA48D1200390927 /* picotls.h in Headers */, @@ -773,7 +760,6 @@ buildActionMask = 2147483647; files = ( 0829876726D372B70053638F /* cc-cubic.c in Sources */, - 0829876826D372B70053638F /* retire_cid.c in Sources */, 0829876926D372B70053638F /* picotls-probes.d in Sources */, 0829876A26D372B70053638F /* ranges.c in Sources */, 0829876B26D372B70053638F /* cc-reno.c in Sources */, @@ -818,7 +804,6 @@ E9F6A4201F3C0B6D0083F0B2 /* ranges.c in Sources */, E98041C622383C62008B9745 /* cc-reno.c in Sources */, E95E95392290497900215ACD /* quicly-probes.d in Sources */, - E973652B246FD3AC0039AA49 /* retire_cid.c in Sources */, E920D21E1F43E05000799777 /* recvstate.c in Sources */, E98042362244A5D7008B9745 /* defaults.c in Sources */, E904233D24AED0410072C5B7 /* loss.c in Sources */, @@ -840,7 +825,6 @@ buildActionMask = 2147483647; files = ( E9DF012524E4BAC90002EEC7 /* cc-cubic.c in Sources */, - E973652F246FD3B40039AA49 /* retire_cid.c in Sources */, E95E953C22904A4C00215ACD /* picotls-probes.d in Sources */, E941428F23B0B84F002D3CE0 /* ranges.c in Sources */, E941428C23B0B831002D3CE0 /* cc-reno.c in Sources */, @@ -878,7 +862,6 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - E9736532246FD3B50039AA49 /* retire_cid.c in Sources */, E9736537246FD3DA0039AA49 /* remote_cid.c in Sources */, E99F8C291F4EAEF800C26B3D /* frame.c in Sources */, 085AB4D9264283D600DF5209 /* pacer.c in Sources */, @@ -893,7 +876,6 @@ E9F6A42F1F41375B0083F0B2 /* sendstate.c in Sources */, E904234124AEFB980072C5B7 /* loss.c in Sources */, E920D2251F49412900799777 /* simple.c in Sources */, - E9736538246FD3DA0039AA49 /* retire_cid.c in Sources */, E920D21F1F43E05000799777 /* recvstate.c in Sources */, E9CC44251EC1962700DC7D3E /* test.c in Sources */, E9F6A4211F3C0B6D0083F0B2 /* ranges.c in Sources */, diff --git a/t/remote_cid.c b/t/remote_cid.c index fbe37ea41..52709c464 100644 --- a/t/remote_cid.c +++ b/t/remote_cid.c @@ -60,8 +60,6 @@ static uint8_t srts[][QUICLY_STATELESS_RESET_TOKEN_LEN] = { void test_received_cid(void) { quicly_remote_cid_set_t set; - uint64_t unregistered_seqs[QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT]; - size_t num_unregistered; #define TEST_SET(...) \ do { \ @@ -84,90 +82,97 @@ void test_received_cid(void) } while (0) quicly_remote_cid_init_set(&set, NULL, quic_ctx.tls->random_bytes); + ok(set._largest_sequence_expected == QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT - 1); + /* fill CIDs */ for (int i = 1; i < 4; i++) { - ok(quicly_remote_cid_register(&set, i, cids[i], CID_LEN, srts[i], 0, unregistered_seqs, &num_unregistered) == 0); - ok(num_unregistered == 0); + ok(quicly_remote_cid_register(&set, i, cids[i], CID_LEN, srts[i], 0) == 0); + ok(set.retired.count == 0); } /* CIDs = {0, 1, 2, 3} */ /* dup */ - ok(quicly_remote_cid_register(&set, 1, cids[1], CID_LEN, srts[1], 0, unregistered_seqs, &num_unregistered) == 0); - ok(num_unregistered == 0); + ok(quicly_remote_cid_register(&set, 1, cids[1], CID_LEN, srts[1], 0) == 0); + ok(set.retired.count == 0); /* same CID with different sequence number */ - ok(quicly_remote_cid_register(&set, 0, cids[1], CID_LEN, srts[1], 0, unregistered_seqs, &num_unregistered) != 0); - ok(num_unregistered == 0); + ok(quicly_remote_cid_register(&set, 0, cids[1], CID_LEN, srts[1], 0) != 0); + ok(set.retired.count == 0); /* already full */ - ok(quicly_remote_cid_register(&set, 4, cids[4], CID_LEN, srts[4], 0, unregistered_seqs, &num_unregistered) == - QUICLY_TRANSPORT_ERROR_CONNECTION_ID_LIMIT); + ok(quicly_remote_cid_register(&set, 4, cids[4], CID_LEN, srts[4], 0) == QUICLY_TRANSPORT_ERROR_CONNECTION_ID_LIMIT); TEST_SET({0, QUICLY_REMOTE_CID_IN_USE}, /* we have CID to send error */ {1, QUICLY_REMOTE_CID_AVAILABLE}, {2, QUICLY_REMOTE_CID_AVAILABLE}, {3, QUICLY_REMOTE_CID_AVAILABLE}); /* retire seq=0 */ quicly_remote_cid_unregister(&set, 0); + ok(set.retired.count == 1); + ok(set.retired.cids[0] == 0); + /* CIDs = {(4), 1, 2, 3} */ TEST_SET({4, QUICLY_REMOTE_CID_UNAVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {2, QUICLY_REMOTE_CID_AVAILABLE}, {3, QUICLY_REMOTE_CID_AVAILABLE}); + ok(set._largest_sequence_expected == 4); /* sequence number out of current acceptable window */ - ok(quicly_remote_cid_register(&set, 255, cids[4], CID_LEN, srts[4], 0, unregistered_seqs, &num_unregistered) == - QUICLY_TRANSPORT_ERROR_CONNECTION_ID_LIMIT); + ok(quicly_remote_cid_register(&set, 255, cids[4], CID_LEN, srts[4], 0) == QUICLY_TRANSPORT_ERROR_CONNECTION_ID_LIMIT); ok(set.cids[1].state == QUICLY_REMOTE_CID_AVAILABLE && "we have CID to send error"); /* ignore already retired CID */ - ok(quicly_remote_cid_register(&set, 0, cids[0], CID_LEN, srts[0], 0, unregistered_seqs, &num_unregistered) == 0); - ok(num_unregistered == 0); + ok(quicly_remote_cid_register(&set, 0, cids[0], CID_LEN, srts[0], 0) == 0); + ok(set.retired.count == 1); /* register 5th CID */ - ok(quicly_remote_cid_register(&set, 4, cids[4], CID_LEN, srts[4], 0, unregistered_seqs, &num_unregistered) == 0); - ok(num_unregistered == 0); + ok(quicly_remote_cid_register(&set, 4, cids[4], CID_LEN, srts[4], 0) == 0); /* active CIDs = {4, 1, 2, 3} */ TEST_SET({4, QUICLY_REMOTE_CID_AVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {2, QUICLY_REMOTE_CID_AVAILABLE}, {3, QUICLY_REMOTE_CID_AVAILABLE}); + ok(set.retired.count == 1); /* unregister seq=2 */ quicly_remote_cid_unregister(&set, 2); /* active CIDs = {4, 1, (5), 3} */ TEST_SET({4, QUICLY_REMOTE_CID_AVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {5, QUICLY_REMOTE_CID_UNAVAILABLE}, {3, QUICLY_REMOTE_CID_AVAILABLE}); + ok(set.retired.count == 2); + ok(set.retired.cids[1] == 2); /* register 5, unregister prior to 5 -- seq=1,3,4 should be unregistered at this moment */ - ok(quicly_remote_cid_register(&set, 5, cids[5], CID_LEN, srts[5], 5, unregistered_seqs, &num_unregistered) == 0); - ok(num_unregistered == 3); - /* check unregistered_seqs */ - ok(unregistered_seqs[0] == 4); - ok(unregistered_seqs[1] == 1); - ok(unregistered_seqs[2] == 3); + ok(quicly_remote_cid_register(&set, 5, cids[5], CID_LEN, srts[5], 5) == 0); /* active CIDs = {(6), (7), 5, (8)} */ TEST_SET({6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_UNAVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, {8, QUICLY_REMOTE_CID_UNAVAILABLE}); + ok(set.retired.count == 5); + ok(set.retired.cids[2] == 4); + ok(set.retired.cids[3] == 1); + ok(set.retired.cids[4] == 3); /* install CID with out-of-order sequence */ - ok(quicly_remote_cid_register(&set, 8, cids[8], CID_LEN, srts[8], 5, unregistered_seqs, &num_unregistered) == 0); - ok(num_unregistered == 0); + ok(quicly_remote_cid_register(&set, 8, cids[8], CID_LEN, srts[8], 5) == 0); + ok(set.retired.count == 5); /* active CIDs = {(6), (7), 5, 8} */ TEST_SET({6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_UNAVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, {8, QUICLY_REMOTE_CID_AVAILABLE}); - ok(quicly_remote_cid_register(&set, 7, cids[7], CID_LEN, srts[7], 5, unregistered_seqs, &num_unregistered) == 0); + ok(quicly_remote_cid_register(&set, 7, cids[7], CID_LEN, srts[7], 5) == 0); + ok(set.retired.count == 5); /* active CIDs = {(6), 7, 5, 8} */ TEST_SET({6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_AVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, {8, QUICLY_REMOTE_CID_AVAILABLE}); /* unregister prior to 8 -- seq=5-7 should be unregistered at this moment */ - ok(quicly_remote_cid_register(&set, 8, cids[8], CID_LEN, srts[8], 8, unregistered_seqs, &num_unregistered) == 0); - /* active CIDs = {*8} */ - ok(num_unregistered == 3); - /* check unregistered_seqs */ - ok(unregistered_seqs[0] == 6); - ok(unregistered_seqs[1] == 7); - ok(unregistered_seqs[2] == 5); + ok(quicly_remote_cid_register(&set, 8, cids[8], CID_LEN, srts[8], 8) == 0); /* active CIDs = {(9), (10), (11), 8} */ TEST_SET({9, QUICLY_REMOTE_CID_UNAVAILABLE}, {10, QUICLY_REMOTE_CID_UNAVAILABLE}, {11, QUICLY_REMOTE_CID_UNAVAILABLE}, {8, QUICLY_REMOTE_CID_AVAILABLE}); + ok(set.retired.count == 8); + ok(set.retired.cids[5] == 6); + ok(set.retired.cids[6] == 7); + ok(set.retired.cids[7] == 5); /* register 11 */ - ok(quicly_remote_cid_register(&set, 11, cids[11], CID_LEN, srts[11], 8, unregistered_seqs, &num_unregistered) == 0); - ok(num_unregistered == 0); + ok(quicly_remote_cid_register(&set, 11, cids[11], CID_LEN, srts[11], 8) == 0); + ok(set.retired.count == 8); /* active CIDs = {(9), (10), (11), 8} */ TEST_SET({9, QUICLY_REMOTE_CID_UNAVAILABLE}, {10, QUICLY_REMOTE_CID_UNAVAILABLE}, {11, QUICLY_REMOTE_CID_AVAILABLE}, {8, QUICLY_REMOTE_CID_AVAILABLE}); + + /* too many outstanding CIDs in retire queue */ + ok(quicly_remote_cid_register(&set, 11, cids[0], CID_LEN, srts[0], 9) == QUICLY_TRANSPORT_ERROR_PROTOCOL_VIOLATION); } diff --git a/t/retire_cid.c b/t/retire_cid.c deleted file mode 100644 index 5fd695c76..000000000 --- a/t/retire_cid.c +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright (c) 2020 Fastly, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to - * deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or - * sell copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ - -#include "test.h" -#include "picotls.h" -#include "quicly/retire_cid.h" - -/** - * verifies that expected sequence numbers are in that order at the front of the array. - * Returns zero on success. - */ -static int verify(const quicly_retire_cid_set_t *set, const uint64_t expected_seqs[], size_t num_seqs) -{ - size_t i; - - if (set->_num_pending != num_seqs) - return 1; - - for (i = 0; i < num_seqs && i < PTLS_ELEMENTSOF(set->sequences); i++) { - if (set->sequences[i] != expected_seqs[i]) - return 1; - } - - return 0; -} - -void test_retire_cid(void) -{ - uint64_t sequence = 0; - - quicly_retire_cid_set_t set; - quicly_retire_cid_init(&set); - - /* should be empty */ - ok(verify(&set, NULL, 0) == 0); - - /* push one sequence number */ - quicly_retire_cid_push(&set, sequence); - { - uint64_t seqs[] = {sequence}; - ok(verify(&set, seqs, PTLS_ELEMENTSOF(seqs)) == 0); - } - - sequence++; - - /* shift one -- back to empty */ - quicly_retire_cid_shift(&set, 1); - ok(verify(&set, NULL, 0) == 0); - - { - /* make the array full */ - uint64_t seqs[PTLS_ELEMENTSOF(set.sequences)]; - for (size_t i = 0; i < PTLS_ELEMENTSOF(set.sequences); i++) { - seqs[i] = sequence; - quicly_retire_cid_push(&set, sequence); - sequence++; - } - ok(verify(&set, seqs, PTLS_ELEMENTSOF(seqs)) == 0); - /* make sure duplicated push is ignored */ - quicly_retire_cid_push(&set, seqs[0]); - ok(verify(&set, seqs, PTLS_ELEMENTSOF(seqs)) == 0); - /* make sure push is ignored when the array is already full */ - quicly_retire_cid_push(&set, sequence + 1); - ok(verify(&set, seqs, PTLS_ELEMENTSOF(seqs)) == 0); - /* zero shift from a full array */ - quicly_retire_cid_shift(&set, 0); - /* test partial removal */ - size_t num_shift = PTLS_ELEMENTSOF(seqs) / 2; - quicly_retire_cid_shift(&set, num_shift); - ok(verify(&set, seqs + num_shift, PTLS_ELEMENTSOF(seqs) - num_shift) == 0); - /* test zero shift */ - quicly_retire_cid_shift(&set, 0); - ok(verify(&set, seqs + num_shift, PTLS_ELEMENTSOF(seqs) - num_shift) == 0); - /* remove remaining sequence numbers */ - quicly_retire_cid_shift(&set, PTLS_ELEMENTSOF(seqs) - num_shift); - ok(verify(&set, NULL, 0) == 0); - } -} diff --git a/t/test.c b/t/test.c index 5c786f5f9..875ea4a7a 100644 --- a/t/test.c +++ b/t/test.c @@ -587,7 +587,6 @@ static void test_cid(void) { subtest("received cid", test_received_cid); subtest("local cid", test_local_cid); - subtest("retire cid", test_retire_cid); } /** diff --git a/t/test.h b/t/test.h index e3ea36c64..c8ead692a 100644 --- a/t/test.h +++ b/t/test.h @@ -60,7 +60,6 @@ void test_lossy(void); void test_stream_concurrency(void); void test_received_cid(void); void test_local_cid(void); -void test_retire_cid(void); void test_jumpstart(void); #endif From 3813fe96929e54724e923b86206f1f95da08f87d Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Tue, 9 Apr 2024 15:50:45 +0900 Subject: [PATCH 2/8] more tests --- t/remote_cid.c | 79 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/t/remote_cid.c b/t/remote_cid.c index 52709c464..c618e241d 100644 --- a/t/remote_cid.c +++ b/t/remote_cid.c @@ -39,6 +39,11 @@ static uint8_t cids[][CID_LEN] = { {9, 10, 11, 12, 13, 14, 15, 16}, {10, 11, 12, 13, 14, 15, 16, 17}, {11, 12, 13, 14, 15, 16, 17, 18}, + {12, 13, 14, 15, 16, 17, 18, 19}, + {13, 14, 15, 16, 17, 18, 19, 20}, + {14, 15, 16, 17, 18, 19, 20, 21}, + {15, 16, 17, 18, 19, 20, 21, 22}, + {16, 17, 18, 19, 20, 21, 22, 23}, }; static uint8_t srts[][QUICLY_STATELESS_RESET_TOKEN_LEN] = { @@ -54,6 +59,11 @@ static uint8_t srts[][QUICLY_STATELESS_RESET_TOKEN_LEN] = { {9}, {10}, {11}, + {12}, + {13}, + {14}, + {15}, + {16}, }; /* clang-format on */ @@ -61,8 +71,9 @@ void test_received_cid(void) { quicly_remote_cid_set_t set; -#define TEST_SET(...) \ +#define TEST_SET(largest_expected, ...) \ do { \ + ok(set._largest_sequence_expected == largest_expected); \ static const struct { \ uint64_t seq; \ quicly_remote_cid_state_t state; \ @@ -80,16 +91,22 @@ void test_received_cid(void) } \ } \ } while (0) +#define TEST_RETIRED(...) \ + do { \ + static uint64_t expected[] = {__VA_ARGS__}; \ + ok(set.retired.count == PTLS_ELEMENTSOF(expected)); \ + for (size_t i = 0; i < PTLS_ELEMENTSOF(expected); ++i) \ + ok(set.retired.cids[i] == expected[i]); \ + } while (0) quicly_remote_cid_init_set(&set, NULL, quic_ctx.tls->random_bytes); - ok(set._largest_sequence_expected == QUICLY_LOCAL_ACTIVE_CONNECTION_ID_LIMIT - 1); /* fill CIDs */ for (int i = 1; i < 4; i++) { ok(quicly_remote_cid_register(&set, i, cids[i], CID_LEN, srts[i], 0) == 0); - ok(set.retired.count == 0); } /* CIDs = {0, 1, 2, 3} */ + ok(set.retired.count == 0); /* dup */ ok(quicly_remote_cid_register(&set, 1, cids[1], CID_LEN, srts[1], 0) == 0); @@ -99,80 +116,80 @@ void test_received_cid(void) ok(set.retired.count == 0); /* already full */ ok(quicly_remote_cid_register(&set, 4, cids[4], CID_LEN, srts[4], 0) == QUICLY_TRANSPORT_ERROR_CONNECTION_ID_LIMIT); - TEST_SET({0, QUICLY_REMOTE_CID_IN_USE}, /* we have CID to send error */ + TEST_SET(3, {0, QUICLY_REMOTE_CID_IN_USE}, /* we have CID to send error */ {1, QUICLY_REMOTE_CID_AVAILABLE}, {2, QUICLY_REMOTE_CID_AVAILABLE}, {3, QUICLY_REMOTE_CID_AVAILABLE}); /* retire seq=0 */ quicly_remote_cid_unregister(&set, 0); - ok(set.retired.count == 1); - ok(set.retired.cids[0] == 0); + TEST_RETIRED(0); /* CIDs = {(4), 1, 2, 3} */ - TEST_SET({4, QUICLY_REMOTE_CID_UNAVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {2, QUICLY_REMOTE_CID_AVAILABLE}, + TEST_SET(4, {4, QUICLY_REMOTE_CID_UNAVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {2, QUICLY_REMOTE_CID_AVAILABLE}, {3, QUICLY_REMOTE_CID_AVAILABLE}); - ok(set._largest_sequence_expected == 4); /* sequence number out of current acceptable window */ ok(quicly_remote_cid_register(&set, 255, cids[4], CID_LEN, srts[4], 0) == QUICLY_TRANSPORT_ERROR_CONNECTION_ID_LIMIT); ok(set.cids[1].state == QUICLY_REMOTE_CID_AVAILABLE && "we have CID to send error"); /* ignore already retired CID */ ok(quicly_remote_cid_register(&set, 0, cids[0], CID_LEN, srts[0], 0) == 0); - ok(set.retired.count == 1); + TEST_RETIRED(0); /* register 5th CID */ ok(quicly_remote_cid_register(&set, 4, cids[4], CID_LEN, srts[4], 0) == 0); /* active CIDs = {4, 1, 2, 3} */ - TEST_SET({4, QUICLY_REMOTE_CID_AVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {2, QUICLY_REMOTE_CID_AVAILABLE}, + TEST_SET(4, {4, QUICLY_REMOTE_CID_AVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {2, QUICLY_REMOTE_CID_AVAILABLE}, {3, QUICLY_REMOTE_CID_AVAILABLE}); - ok(set.retired.count == 1); + TEST_RETIRED(0); /* unregister seq=2 */ quicly_remote_cid_unregister(&set, 2); /* active CIDs = {4, 1, (5), 3} */ - TEST_SET({4, QUICLY_REMOTE_CID_AVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {5, QUICLY_REMOTE_CID_UNAVAILABLE}, + TEST_SET(5, {4, QUICLY_REMOTE_CID_AVAILABLE}, {1, QUICLY_REMOTE_CID_AVAILABLE}, {5, QUICLY_REMOTE_CID_UNAVAILABLE}, {3, QUICLY_REMOTE_CID_AVAILABLE}); - ok(set.retired.count == 2); - ok(set.retired.cids[1] == 2); + TEST_RETIRED(0, 2); /* register 5, unregister prior to 5 -- seq=1,3,4 should be unregistered at this moment */ ok(quicly_remote_cid_register(&set, 5, cids[5], CID_LEN, srts[5], 5) == 0); /* active CIDs = {(6), (7), 5, (8)} */ - TEST_SET({6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_UNAVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, + TEST_SET(8, {6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_UNAVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, {8, QUICLY_REMOTE_CID_UNAVAILABLE}); - ok(set.retired.count == 5); - ok(set.retired.cids[2] == 4); - ok(set.retired.cids[3] == 1); - ok(set.retired.cids[4] == 3); + TEST_RETIRED(0, 2, 4, 1, 3); /* install CID with out-of-order sequence */ ok(quicly_remote_cid_register(&set, 8, cids[8], CID_LEN, srts[8], 5) == 0); - ok(set.retired.count == 5); /* active CIDs = {(6), (7), 5, 8} */ - TEST_SET({6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_UNAVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, + TEST_SET(8, {6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_UNAVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, {8, QUICLY_REMOTE_CID_AVAILABLE}); + TEST_RETIRED(0, 2, 4, 1, 3); ok(quicly_remote_cid_register(&set, 7, cids[7], CID_LEN, srts[7], 5) == 0); - ok(set.retired.count == 5); /* active CIDs = {(6), 7, 5, 8} */ - TEST_SET({6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_AVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, + TEST_SET(8, {6, QUICLY_REMOTE_CID_UNAVAILABLE}, {7, QUICLY_REMOTE_CID_AVAILABLE}, {5, QUICLY_REMOTE_CID_AVAILABLE}, {8, QUICLY_REMOTE_CID_AVAILABLE}); + TEST_RETIRED(0, 2, 4, 1, 3); /* unregister prior to 8 -- seq=5-7 should be unregistered at this moment */ ok(quicly_remote_cid_register(&set, 8, cids[8], CID_LEN, srts[8], 8) == 0); /* active CIDs = {(9), (10), (11), 8} */ - TEST_SET({9, QUICLY_REMOTE_CID_UNAVAILABLE}, {10, QUICLY_REMOTE_CID_UNAVAILABLE}, {11, QUICLY_REMOTE_CID_UNAVAILABLE}, + TEST_SET(11, {9, QUICLY_REMOTE_CID_UNAVAILABLE}, {10, QUICLY_REMOTE_CID_UNAVAILABLE}, {11, QUICLY_REMOTE_CID_UNAVAILABLE}, {8, QUICLY_REMOTE_CID_AVAILABLE}); - ok(set.retired.count == 8); - ok(set.retired.cids[5] == 6); - ok(set.retired.cids[6] == 7); - ok(set.retired.cids[7] == 5); /* register 11 */ ok(quicly_remote_cid_register(&set, 11, cids[11], CID_LEN, srts[11], 8) == 0); - ok(set.retired.count == 8); /* active CIDs = {(9), (10), (11), 8} */ - TEST_SET({9, QUICLY_REMOTE_CID_UNAVAILABLE}, {10, QUICLY_REMOTE_CID_UNAVAILABLE}, {11, QUICLY_REMOTE_CID_AVAILABLE}, + TEST_SET(11, {9, QUICLY_REMOTE_CID_UNAVAILABLE}, {10, QUICLY_REMOTE_CID_UNAVAILABLE}, {11, QUICLY_REMOTE_CID_AVAILABLE}, {8, QUICLY_REMOTE_CID_AVAILABLE}); + TEST_RETIRED(0, 2, 4, 1, 3, 6, 7, 5); /* too many outstanding CIDs in retire queue */ - ok(quicly_remote_cid_register(&set, 11, cids[0], CID_LEN, srts[0], 9) == QUICLY_TRANSPORT_ERROR_PROTOCOL_VIOLATION); + ok(quicly_remote_cid_register(&set, 11, cids[11], CID_LEN, srts[11], 9) == QUICLY_TRANSPORT_ERROR_PROTOCOL_VIOLATION); + + /* pretend as if RCID frames carrying the retired ones were sent and that all have been acked */ + set.retired.count = 0; + + /* retire more than ACTIVE_CID_LIMITs at once */ + ok(quicly_remote_cid_register(&set, 16, cids[16], CID_LEN, srts[16], 16) == 0); + /* active CIDs = {16, (17), (18), (19)} */ + TEST_SET(19, {16, QUICLY_REMOTE_CID_AVAILABLE}, {17, QUICLY_REMOTE_CID_UNAVAILABLE}, {18, QUICLY_REMOTE_CID_UNAVAILABLE}, + {19, QUICLY_REMOTE_CID_UNAVAILABLE}); + TEST_RETIRED(9, 12, 13, 14, 15, 10, 11, 8); } From cf2e0a557ae8c78a7678a6d18d1c2eceb8048acd Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 23 Dec 2024 11:50:59 +0900 Subject: [PATCH 3/8] to avoid resource leak inside `quicly_free`, call `free(path)` even when the retired CID queue is full --- include/quicly/remote_cid.h | 3 ++- lib/quicly.c | 7 +++---- lib/remote_cid.c | 7 ++----- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/include/quicly/remote_cid.h b/include/quicly/remote_cid.h index e848098cd..f5849b5f3 100644 --- a/include/quicly/remote_cid.h +++ b/include/quicly/remote_cid.h @@ -106,7 +106,8 @@ void quicly_remote_cid_init_set(quicly_remote_cid_set_t *set, ptls_iovec_t *init int quicly_remote_cid_register(quicly_remote_cid_set_t *set, uint64_t sequence, const uint8_t *cid, size_t cid_len, const uint8_t srt[QUICLY_STATELESS_RESET_TOKEN_LEN], uint64_t retire_prior_to); /** - * unregisters specified CID from the store; the unregistered CID is pushed onto the retired queue + * Unregisters specified CID from the store, pushing the unregistered CID onto the retired queue. The former always succeeds, while + * the latter might fail due to overflow. In case of the latter, an error is returned. */ int quicly_remote_cid_unregister(quicly_remote_cid_set_t *set, uint64_t sequence); /** diff --git a/lib/quicly.c b/lib/quicly.c index e2979e9bd..7669fefdd 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -1841,20 +1841,19 @@ static int new_path(quicly_conn_t *conn, size_t path_index, struct sockaddr *rem static int do_delete_path(quicly_conn_t *conn, struct st_quicly_conn_path_t *path) { - int ret; + int ret = 0; if (path->dcid != UINT64_MAX && conn->super.remote.cid_set.cids[0].cid.len != 0) { uint64_t cid = path->dcid; dissociate_cid(conn, cid); - if ((ret = quicly_remote_cid_unregister(&conn->super.remote.cid_set, cid)) != 0) - return ret; + ret = quicly_remote_cid_unregister(&conn->super.remote.cid_set, cid); assert(conn->super.remote.cid_set.retired.count != 0); conn->egress.pending_flows |= QUICLY_PENDING_FLOW_OTHERS_BIT; } free(path); - return 0; + return ret; } static int delete_path(quicly_conn_t *conn, size_t path_index) diff --git a/lib/remote_cid.c b/lib/remote_cid.c index 06c771081..0948762a5 100644 --- a/lib/remote_cid.c +++ b/lib/remote_cid.c @@ -95,15 +95,12 @@ static int do_register(quicly_remote_cid_set_t *set, uint64_t sequence, const ui static int do_unregister(quicly_remote_cid_set_t *set, size_t idx_to_unreg) { - int ret; - - if ((ret = quicly_remote_cid_push_retired(set, set->cids[idx_to_unreg].sequence)) != 0) - return ret; + uint64_t seq_to_unreg = set->cids[idx_to_unreg].sequence; set->cids[idx_to_unreg].state = QUICLY_REMOTE_CID_UNAVAILABLE; set->cids[idx_to_unreg].sequence = ++set->_largest_sequence_expected; - return 0; + return quicly_remote_cid_push_retired(set, seq_to_unreg); } int quicly_remote_cid_unregister(quicly_remote_cid_set_t *set, uint64_t sequence) From c0451b064971240a3f2f3ccf4e442e90f177f8c6 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 23 Dec 2024 14:07:00 +0900 Subject: [PATCH 4/8] restore state whenever handling of NCID fails --- lib/remote_cid.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/remote_cid.c b/lib/remote_cid.c index 0948762a5..d7812b204 100644 --- a/lib/remote_cid.c +++ b/lib/remote_cid.c @@ -135,15 +135,11 @@ int quicly_remote_cid_register(quicly_remote_cid_set_t *set, uint64_t sequence, assert(sequence >= retire_prior_to); /* First, handle retire_prior_to. This order is important as it is possible to receive a NEW_CONNECTION_ID frame such that it - * retires active_connection_id_limit CIDs and then installs one new CID. */ - if ((ret = unregister_prior_to(set, retire_prior_to)) != 0) - return ret; - - /* Then, register given value. If an error occurs, restore the backup and send the error. */ - if ((ret = do_register(set, sequence, cid, cid_len, srt)) != 0) { + * retires active_connection_id_limit CIDs and then installs one new CID. Then, the new CID is registered. If either of the two + * fails, the state is restored to the original so that we can send an error to the peer. */ + if ((ret = unregister_prior_to(set, retire_prior_to)) != 0 || + (ret = do_register(set, sequence, cid, cid_len, srt)) != 0) *set = backup; - return ret; - } return ret; } From ed80eca4ef52028df9d5915c049fa1ad66b0f0f7 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 23 Dec 2024 15:07:54 +0900 Subject: [PATCH 5/8] retire cid queue can overflow while buidling packets --- lib/quicly.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index 7669fefdd..de155510e 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -5544,6 +5544,7 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s if (conn->super.state >= QUICLY_STATE_CLOSING) { quicly_sentmap_iter_t iter; + DoClose: if ((ret = init_acks_iter(conn, &iter)) != 0) goto Exit; /* check if the connection can be closed now (after 3 pto) */ @@ -5583,13 +5584,17 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s conn->paths[s.path_index]->path_response.send_)) continue; if (conn->paths[s.path_index]->path_challenge.num_sent > conn->super.ctx->max_probe_packets) { - delete_path(conn, s.path_index); + if ((ret = delete_path(conn, s.path_index)) != 0) { + initiate_close(conn, ret, QUICLY_FRAME_TYPE_PADDING, NULL); + goto DoClose; + } s.recalc_send_probe_at = 1; continue; } /* determine DCID to be used, if not yet been done; upon failure, this path (being secondary) is discarded */ if (conn->paths[s.path_index]->dcid == UINT64_MAX && !setup_path_dcid(conn, s.path_index)) { - delete_path(conn, s.path_index); + ret = delete_path(conn, s.path_index); + assert(ret == 0 && "path->dcid is UINT64_MAX and therefore does not trigger an error"); s.recalc_send_probe_at = 1; conn->super.stats.num_paths.closed_no_dcid += 1; continue; From a0849c6563108f16b77c1c54ab9428a5763e22a0 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 23 Dec 2024 15:18:50 +0900 Subject: [PATCH 6/8] refactor as a separate function --- lib/quicly.c | 77 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index de155510e..a846e5827 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -5504,6 +5504,48 @@ int quicly_set_cc(quicly_conn_t *conn, quicly_cc_type_t *cc) return cc->cc_switch(&conn->egress.cc); } +static int do_send_closed(quicly_conn_t *conn, quicly_send_context_t *s) +{ + quicly_sentmap_iter_t iter; + int ret; + + if ((ret = init_acks_iter(conn, &iter)) != 0) + goto Exit; + + /* check if the connection can be closed now (after 3 pto) */ + if (conn->super.state == QUICLY_STATE_DRAINING || + conn->super.stats.num_frames_sent.transport_close + conn->super.stats.num_frames_sent.application_close != 0) { + if (quicly_sentmap_get(&iter)->packet_number == UINT64_MAX) { + assert(quicly_num_streams(conn) == 0); + ret = QUICLY_ERROR_FREE_CONNECTION; + goto Exit; + } + } + + if (conn->super.state == QUICLY_STATE_CLOSING && conn->egress.send_ack_at <= conn->stash.now) { + /* destroy all streams; doing so is delayed until the emission of CONNECTION_CLOSE frame to allow quicly_close to be called + * from a stream handler */ + destroy_all_streams(conn, 0, 0); + /* send CONNECTION_CLOSE in all possible epochs */ + s->dcid = get_dcid(conn, 0); + for (size_t epoch = 0; epoch < QUICLY_NUM_EPOCHS; ++epoch) { + if ((ret = send_connection_close(conn, epoch, s)) != 0) + goto Exit; + } + if ((ret = commit_send_packet(conn, s, 0)) != 0) + goto Exit; + } + + /* wait at least 1ms */ + if ((conn->egress.send_ack_at = quicly_sentmap_get(&iter)->sent_at + get_sentmap_expiration_time(conn)) <= conn->stash.now) + conn->egress.send_ack_at = conn->stash.now + 1; + + ret = 0; + +Exit: + return ret; +} + int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *src, struct iovec *datagrams, size_t *num_datagrams, void *buf, size_t bufsize) { @@ -5543,36 +5585,7 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s } if (conn->super.state >= QUICLY_STATE_CLOSING) { - quicly_sentmap_iter_t iter; - DoClose: - if ((ret = init_acks_iter(conn, &iter)) != 0) - goto Exit; - /* check if the connection can be closed now (after 3 pto) */ - if (conn->super.state == QUICLY_STATE_DRAINING || - conn->super.stats.num_frames_sent.transport_close + conn->super.stats.num_frames_sent.application_close != 0) { - if (quicly_sentmap_get(&iter)->packet_number == UINT64_MAX) { - assert(quicly_num_streams(conn) == 0); - ret = QUICLY_ERROR_FREE_CONNECTION; - goto Exit; - } - } - if (conn->super.state == QUICLY_STATE_CLOSING && conn->egress.send_ack_at <= conn->stash.now) { - /* destroy all streams; doing so is delayed until the emission of CONNECTION_CLOSE frame to allow quicly_close to be - * called from a stream handler */ - destroy_all_streams(conn, 0, 0); - /* send CONNECTION_CLOSE in all possible epochs */ - s.dcid = get_dcid(conn, 0); - for (size_t epoch = 0; epoch < QUICLY_NUM_EPOCHS; ++epoch) { - if ((ret = send_connection_close(conn, epoch, &s)) != 0) - goto Exit; - } - if ((ret = commit_send_packet(conn, &s, 0)) != 0) - goto Exit; - } - /* wait at least 1ms */ - if ((conn->egress.send_ack_at = quicly_sentmap_get(&iter)->sent_at + get_sentmap_expiration_time(conn)) <= conn->stash.now) - conn->egress.send_ack_at = conn->stash.now + 1; - ret = 0; + ret = do_send_closed(conn, &s); goto Exit; } @@ -5586,7 +5599,9 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s if (conn->paths[s.path_index]->path_challenge.num_sent > conn->super.ctx->max_probe_packets) { if ((ret = delete_path(conn, s.path_index)) != 0) { initiate_close(conn, ret, QUICLY_FRAME_TYPE_PADDING, NULL); - goto DoClose; + assert(conn->super.state >= QUICLY_STATE_CLOSING); + ret = do_send_closed(conn, &s); + goto Exit; } s.recalc_send_probe_at = 1; continue; From c9cc65e8deee72e0e97932e20807bf348db0ae4d Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 23 Dec 2024 15:19:48 +0900 Subject: [PATCH 7/8] connection close happens on path 0 --- lib/quicly.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/quicly.c b/lib/quicly.c index a846e5827..3bafff890 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -5506,6 +5506,8 @@ int quicly_set_cc(quicly_conn_t *conn, quicly_cc_type_t *cc) static int do_send_closed(quicly_conn_t *conn, quicly_send_context_t *s) { + assert(s->path_index == 0); + quicly_sentmap_iter_t iter; int ret; @@ -5600,6 +5602,7 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s if ((ret = delete_path(conn, s.path_index)) != 0) { initiate_close(conn, ret, QUICLY_FRAME_TYPE_PADDING, NULL); assert(conn->super.state >= QUICLY_STATE_CLOSING); + s.path_index = 0; ret = do_send_closed(conn, &s); goto Exit; } From c52800edbbcd5d24fb59815276c0bc3445d9aa49 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 23 Dec 2024 18:57:06 +0900 Subject: [PATCH 8/8] remove those sent to avoid unnecessary retransmission --- include/quicly/remote_cid.h | 11 ++--------- lib/quicly.c | 9 ++++++--- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/include/quicly/remote_cid.h b/include/quicly/remote_cid.h index f5849b5f3..7517968cc 100644 --- a/include/quicly/remote_cid.h +++ b/include/quicly/remote_cid.h @@ -115,16 +115,9 @@ int quicly_remote_cid_unregister(quicly_remote_cid_set_t *set, uint64_t sequence */ int quicly_remote_cid_push_retired(quicly_remote_cid_set_t *set, uint64_t sequence); /** - * flushes the retire queue + * removed the first `count` sequence numbers from the retired queue */ -static void quicly_remote_cid_clear_retired(quicly_remote_cid_set_t *set); - -/* inline definitions */ - -inline void quicly_remote_cid_clear_retired(quicly_remote_cid_set_t *set) -{ - set->retired.count = 0; -} +void quicly_remote_cid_shift_retired(quicly_remote_cid_set_t *set, size_t count); #ifdef __cplusplus } diff --git a/lib/quicly.c b/lib/quicly.c index 3bafff890..26356a395 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -5201,12 +5201,15 @@ static int send_other_control_frames(quicly_conn_t *conn, quicly_send_context_t } { /* RETIRE_CONNECTION_ID */ - for (size_t i = 0; i < conn->super.remote.cid_set.retired.count; ++i) { + size_t i; + for (i = 0; i < conn->super.remote.cid_set.retired.count; ++i) { uint64_t sequence = conn->super.remote.cid_set.retired.cids[i]; if ((ret = send_retire_connection_id(conn, s, sequence)) != 0) - return ret; + break; } - quicly_remote_cid_clear_retired(&conn->super.remote.cid_set); + quicly_remote_cid_shift_retired(&conn->super.remote.cid_set, i); + if (ret != 0) + return ret; } return 0;