From 729470c84386b147154df1d7beed74f38c391ec8 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Wed, 4 Sep 2024 13:43:35 +0900 Subject: [PATCH 1/2] fix assertion failures --- lib/quicly.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index c797ef71..38fecdd2 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -6932,7 +6932,9 @@ int quicly_accept(quicly_conn_t **conn, quicly_context_t *ctx, struct sockaddr * Exit: if (*conn != NULL) { if (ret == 0) { - (*conn)->super.state = QUICLY_STATE_CONNECTED; + /* if CONNECTION_CLOSE was found and the state advanced to DRAINING, we need to retain that state */ + if ((*conn)->super.state < QUICLY_STATE_CONNECTED) + (*conn)->super.state = QUICLY_STATE_CONNECTED; } else { initiate_close(*conn, ret, offending_frame_type, ""); ret = 0; @@ -6997,8 +6999,10 @@ int quicly_receive(quicly_conn_t *conn, struct sockaddr *dest_addr, struct socka if (conn->paths[path_index] != NULL && compare_socket_address(src_addr, &conn->paths[path_index]->address.remote.sa) == 0) break; if (path_index == PTLS_ELEMENTSOF(conn->paths) && - conn->super.stats.num_paths.validation_failed >= conn->super.ctx->max_path_validation_failures) - return QUICLY_ERROR_PACKET_IGNORED; + conn->super.stats.num_paths.validation_failed >= conn->super.ctx->max_path_validation_failures) { + ret = QUICLY_ERROR_PACKET_IGNORED; + goto Exit; + } /* add unconditionally, as packet->datagram_size is set only for the first packet within the UDP datagram */ conn->super.stats.num_bytes.received += packet->datagram_size; From d31b349e93bda82a1159f022e3bb23d76d137bb1 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Tue, 8 Oct 2024 14:37:33 +0900 Subject: [PATCH 2/2] more fixes --- lib/quicly.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index 38fecdd2..cb70e442 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -1919,6 +1919,9 @@ static int promote_path(quicly_conn_t *conn, size_t path_index) do_delete_path(conn, path); + /* rearm the loss timer, now that the RTT estimate has been changed */ + setup_next_send(conn); + return 0; } @@ -5194,7 +5197,7 @@ static int send_other_control_frames(quicly_conn_t *conn, quicly_send_context_t static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) { - int restrict_sending = 0, ack_only = 0, ret; + int restrict_sending = 0, ack_only = 0, ret = 0; size_t min_packets_to_send = 0, orig_bytes_inflight = 0; /* handle timeouts */ @@ -5283,11 +5286,13 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) * * quicly running as a client sends either a Handshake probe (or data) if the handshake keys are available, or else an * Initial probe (or data). * * quicly running as a server sends both Initial and Handshake probes (or data) if the corresponding keys are available. */ - if ((ret = send_handshake_flow(conn, QUICLY_EPOCH_INITIAL, s, ack_only, - min_packets_to_send != 0 && (!quicly_is_client(conn) || conn->handshake == NULL))) != 0) - goto Exit; - if ((ret = send_handshake_flow(conn, QUICLY_EPOCH_HANDSHAKE, s, ack_only, min_packets_to_send != 0)) != 0) - goto Exit; + if (s->path_index == 0) { + if ((ret = send_handshake_flow(conn, QUICLY_EPOCH_INITIAL, s, ack_only, + min_packets_to_send != 0 && (!quicly_is_client(conn) || conn->handshake == NULL))) != 0) + goto Exit; + if ((ret = send_handshake_flow(conn, QUICLY_EPOCH_HANDSHAKE, s, ack_only, min_packets_to_send != 0)) != 0) + goto Exit; + } /* setup 0-RTT or 1-RTT send context (as the availability of the two epochs are mutually exclusive, we can try 1-RTT first as an * optimization), then send application data if that succeeds */ @@ -5447,7 +5452,7 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) if (conn->application == NULL || conn->application->super.unacked_count == 0) conn->egress.send_ack_at = INT64_MAX; /* we have sent ACKs for every epoch (or before address validation) */ int can_send_stream_data = scheduler_can_send(conn); - update_send_alarm(conn, can_send_stream_data, 1); + update_send_alarm(conn, can_send_stream_data, s->path_index == 0); update_ratemeter(conn, can_send_stream_data && conn->super.remote.address_validation.validated && (s->num_datagrams == s->max_datagrams || conn->egress.loss.sentmap.bytes_in_flight >= conn->egress.cc.cwnd || @@ -5553,7 +5558,8 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s * more than one path at a time) */ if (conn->egress.send_probe_at <= conn->stash.now) { for (s.path_index = 1; s.path_index < PTLS_ELEMENTSOF(conn->paths); ++s.path_index) { - if (conn->paths[s.path_index] == NULL || conn->stash.now < conn->paths[s.path_index]->path_challenge.send_at) + if (conn->paths[s.path_index] == NULL || !(conn->stash.now >= conn->paths[s.path_index]->path_challenge.send_at || + 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); @@ -5569,6 +5575,7 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s } if ((ret = do_send(conn, &s)) != 0) goto Exit; + assert(conn->stash.now < conn->paths[s.path_index]->path_challenge.send_at); if (s.num_datagrams != 0) break; } @@ -5582,7 +5589,7 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s ret = 0; } - assert_consistency(conn, 1); + assert_consistency(conn, s.path_index == 0); Exit: if (s.path_index == 0) @@ -6231,6 +6238,7 @@ static int handle_path_response_frame(quicly_conn_t *conn, struct st_quicly_hand if (ptls_mem_equal(path->path_challenge.data, frame.data, QUICLY_PATH_CHALLENGE_DATA_LEN)) { /* Path validation succeeded, stop sending PATH_CHALLENGEs. Active path might become changed in `quicly_receive`. */ path->path_challenge.send_at = INT64_MAX; + recalc_send_probe_at(conn); conn->super.stats.num_paths.validated += 1; }