From df9cef5c0938347b0c5189b9ab2cfbd9c46f25fa Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Sun, 17 Mar 2024 21:02:29 +1000 Subject: [PATCH 01/16] it works? --- include/quicly.h | 5 +- include/quicly/constants.h | 3 + include/quicly/frame.h | 19 +- lib/quicly.c | 368 +++++++++++++++++++++++++++---------- 4 files changed, 289 insertions(+), 106 deletions(-) diff --git a/include/quicly.h b/include/quicly.h index a4b9a6cd..bf46a647 100644 --- a/include/quicly.h +++ b/include/quicly.h @@ -528,7 +528,7 @@ struct st_quicly_conn_streamgroup_state_t { uint64_t padding, ping, ack, reset_stream, stop_sending, crypto, new_token, stream, max_data, max_stream_data, \ max_streams_bidi, max_streams_uni, data_blocked, stream_data_blocked, streams_blocked, new_connection_id, \ retire_connection_id, path_challenge, path_response, transport_close, application_close, handshake_done, datagram, \ - ack_frequency; \ + ack_frequency, qs_transport_parameters; \ } num_frames_sent, num_frames_received; \ /** \ * Total number of PTOs observed during the connection. \ @@ -1355,6 +1355,9 @@ extern const quicly_stream_callbacks_t quicly_stream_noop_callbacks; }); \ } while (0) +int quicly_qos_send(quicly_conn_t *conn, void *buf, size_t *bufsize); +quicly_conn_t *quicly_qos_new(quicly_context_t *ctx, int is_client, void *appdata); + /* inline definitions */ inline int quicly_is_supported_version(uint32_t version) diff --git a/include/quicly/constants.h b/include/quicly/constants.h index 151af360..f6de2406 100644 --- a/include/quicly/constants.h +++ b/include/quicly/constants.h @@ -65,6 +65,8 @@ extern "C" { #define QUICLY_EPOCH_HANDSHAKE 2 #define QUICLY_EPOCH_1RTT 3 #define QUICLY_NUM_EPOCHS 4 +#define QUICLY_EPOCH_ON_STREAMS_TP 4 /* used internally */ +#define QUICLY_EPOCH_ON_STREAMS_OTHER 5 /* coexists with picotls error codes, assuming that int is at least 32-bits */ #define QUICLY_ERROR_IS_QUIC(e) (((e) & ~0x1ffff) == 0x20000) @@ -108,6 +110,7 @@ extern "C" { #define QUICLY_ERROR_STATE_EXHAUSTION 0xff07 #define QUICLY_ERROR_INVALID_INITIAL_VERSION 0xff08 #define QUICLY_ERROR_DECRYPTION_FAILED 0xff09 +#define QUICLY_ERROR_PARTIAL_FRAME 0xff0a typedef int64_t quicly_stream_id_t; diff --git a/include/quicly/frame.h b/include/quicly/frame.h index 6689be6e..db4df7fc 100644 --- a/include/quicly/frame.h +++ b/include/quicly/frame.h @@ -60,6 +60,7 @@ extern "C" { #define QUICLY_FRAME_TYPE_DATAGRAM_NOLEN 48 #define QUICLY_FRAME_TYPE_DATAGRAM_WITHLEN 49 #define QUICLY_FRAME_TYPE_ACK_FREQUENCY 0xaf +#define QUICLY_FRAME_TYPE_QS_TRANSPORT_PARAMETERS 0x3f5153300d0a0d0a #define QUICLY_FRAME_TYPE_STREAM_BITS 0x7 #define QUICLY_FRAME_TYPE_STREAM_BIT_OFF 0x4 @@ -401,7 +402,7 @@ inline int quicly_decode_stream_frame(uint8_t type_flags, const uint8_t **src, c return 0; Error: - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; } inline uint8_t *quicly_encode_crypto_frame_header(uint8_t *dst, uint8_t *dst_end, uint64_t offset, size_t *data_len) @@ -470,7 +471,7 @@ inline int quicly_decode_reset_stream_frame(const uint8_t **src, const uint8_t * frame->final_size = quicly_decodev(src, end); return 0; Error: - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; } inline int quicly_decode_application_close_frame(const uint8_t **src, const uint8_t *end, quicly_application_close_frame_t *frame) @@ -508,7 +509,7 @@ inline int quicly_decode_transport_close_frame(const uint8_t **src, const uint8_ *src += reason_len; return 0; Error: - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; } inline size_t quicly_close_frame_capacity(uint64_t error_code, uint64_t offending_frame_type, const char *reason_phrase) @@ -526,7 +527,7 @@ inline uint8_t *quicly_encode_max_data_frame(uint8_t *dst, uint64_t max_data) inline int quicly_decode_max_data_frame(const uint8_t **src, const uint8_t *end, quicly_max_data_frame_t *frame) { if ((frame->max_data = quicly_decodev(src, end)) == UINT64_MAX) - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; return 0; } @@ -546,7 +547,7 @@ inline int quicly_decode_max_stream_data_frame(const uint8_t **src, const uint8_ goto Error; return 0; Error: - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; } inline uint8_t *quicly_encode_max_streams_frame(uint8_t *dst, int uni, uint64_t count) @@ -559,7 +560,7 @@ inline uint8_t *quicly_encode_max_streams_frame(uint8_t *dst, int uni, uint64_t inline int quicly_decode_max_streams_frame(const uint8_t **src, const uint8_t *end, quicly_max_streams_frame_t *frame) { if ((frame->count = quicly_decodev(src, end)) == UINT64_MAX) - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; if (frame->count > (uint64_t)1 << 60) return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; return 0; @@ -588,7 +589,7 @@ inline uint8_t *quicly_encode_data_blocked_frame(uint8_t *dst, uint64_t offset) inline int quicly_decode_data_blocked_frame(const uint8_t **src, const uint8_t *end, quicly_data_blocked_frame_t *frame) { if ((frame->offset = quicly_decodev(src, end)) == UINT64_MAX) - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; return 0; } @@ -622,7 +623,7 @@ inline uint8_t *quicly_encode_streams_blocked_frame(uint8_t *dst, int uni, uint6 inline int quicly_decode_streams_blocked_frame(const uint8_t **src, const uint8_t *end, quicly_streams_blocked_frame_t *frame) { if ((frame->count = quicly_decodev(src, end)) == UINT64_MAX) - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; if (frame->count > (uint64_t)1 << 60) return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; return 0; @@ -727,7 +728,7 @@ inline int quicly_decode_stop_sending_frame(const uint8_t **src, const uint8_t * frame->app_error_code = (uint16_t)error_code; return 0; Error: - return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + return QUICLY_ERROR_PARTIAL_FRAME; } inline size_t quicly_new_token_frame_capacity(ptls_iovec_t token) diff --git a/lib/quicly.c b/lib/quicly.c index c2dbb9a6..e131bf36 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -350,6 +350,10 @@ struct st_quicly_conn_t { * */ uint8_t try_jumpstart : 1; + /** + * + */ + uint8_t is_on_streams : 1; /** * pending RETIRE_CONNECTION_ID frames to be sent */ @@ -2184,6 +2188,35 @@ static int collect_transport_parameters(ptls_t *tls, struct st_ptls_handshake_pr return type == get_transport_parameters_extension_id(conn->super.version); } +static void init_connection_core(quicly_conn_t *conn, int is_client) +{ + conn->super.state = QUICLY_STATE_FIRSTFLIGHT; + conn->super.remote.transport_params = default_transport_params; + if (is_client) { + conn->super.local.bidi.next_stream_id = 0; + conn->super.local.uni.next_stream_id = 2; + conn->super.remote.bidi.next_stream_id = 1; + conn->super.remote.uni.next_stream_id = 3; + } else { + conn->super.local.bidi.next_stream_id = 1; + conn->super.local.uni.next_stream_id = 3; + conn->super.remote.bidi.next_stream_id = 0; + conn->super.remote.uni.next_stream_id = 2; + } + quicly_linklist_init(&conn->super._default_scheduler.active); + quicly_linklist_init(&conn->super._default_scheduler.blocked); + conn->streams = kh_init(quicly_stream_t); + quicly_maxsender_init(&conn->ingress.max_data.sender, conn->super.ctx->transport_params.max_data); + quicly_maxsender_init(&conn->ingress.max_streams.uni, conn->super.ctx->transport_params.max_streams_uni); + quicly_maxsender_init(&conn->ingress.max_streams.bidi, conn->super.ctx->transport_params.max_streams_bidi); + init_max_streams(&conn->egress.max_streams.uni); + init_max_streams(&conn->egress.max_streams.bidi); + 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); + conn->stash.on_ack_stream.active_acked_cache.stream_id = INT64_MIN; +} + static quicly_conn_t *create_connection(quicly_context_t *ctx, uint32_t protocol_version, const char *server_name, struct sockaddr *remote_addr, struct sockaddr *local_addr, ptls_iovec_t *remote_cid, const quicly_cid_plaintext_t *local_cid, ptls_handshake_properties_t *handshake_properties, @@ -2227,35 +2260,15 @@ static quicly_conn_t *create_connection(quicly_context_t *ctx, uint32_t protocol quicly_local_cid_init_set(&conn->super.local.cid_set, ctx->cid_encryptor, local_cid); conn->super.local.long_header_src_cid = conn->super.local.cid_set.cids[0].cid; quicly_remote_cid_init_set(&conn->super.remote.cid_set, remote_cid, ctx->tls->random_bytes); - conn->super.state = QUICLY_STATE_FIRSTFLIGHT; - if (server_name != NULL) { - conn->super.local.bidi.next_stream_id = 0; - conn->super.local.uni.next_stream_id = 2; - conn->super.remote.bidi.next_stream_id = 1; - conn->super.remote.uni.next_stream_id = 3; - } else { - conn->super.local.bidi.next_stream_id = 1; - conn->super.local.uni.next_stream_id = 3; - conn->super.remote.bidi.next_stream_id = 0; - conn->super.remote.uni.next_stream_id = 2; - } - conn->super.remote.transport_params = default_transport_params; conn->super.version = protocol_version; conn->super.remote.largest_retire_prior_to = 0; - quicly_linklist_init(&conn->super._default_scheduler.active); - quicly_linklist_init(&conn->super._default_scheduler.blocked); - conn->streams = kh_init(quicly_stream_t); - quicly_maxsender_init(&conn->ingress.max_data.sender, conn->super.ctx->transport_params.max_data); - quicly_maxsender_init(&conn->ingress.max_streams.uni, conn->super.ctx->transport_params.max_streams_uni); - quicly_maxsender_init(&conn->ingress.max_streams.bidi, conn->super.ctx->transport_params.max_streams_bidi); + init_connection_core(conn, server_name != NULL); quicly_loss_init(&conn->egress.loss, &conn->super.ctx->loss, conn->super.ctx->loss.default_initial_rtt /* FIXME remember initial_rtt in session ticket */, &conn->super.remote.transport_params.max_ack_delay, &conn->super.remote.transport_params.ack_delay_exponent); conn->egress.next_pn_to_skip = calc_next_pn_to_skip(conn->super.ctx->tls, 0, initcwnd, conn->super.ctx->initial_egress_max_udp_payload_size); conn->egress.max_udp_payload_size = conn->super.ctx->initial_egress_max_udp_payload_size; - init_max_streams(&conn->egress.max_streams.uni); - init_max_streams(&conn->egress.max_streams.bidi); conn->egress.ack_frequency.update_at = INT64_MAX; conn->egress.send_ack_at = INT64_MAX; conn->super.ctx->init_cc->cb(conn->super.ctx->init_cc, &conn->egress.cc, initcwnd, conn->stash.now); @@ -2265,9 +2278,6 @@ static quicly_conn_t *create_connection(quicly_context_t *ctx, uint32_t protocol } 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); quicly_ratemeter_init(&conn->egress.ratemeter); if (server_name == NULL && conn->super.ctx->use_pacing && conn->egress.cc.type->cc_jumpstart != NULL && (conn->super.ctx->default_jumpstart_cwnd_packets != 0 || conn->super.ctx->max_jumpstart_cwnd_packets != 0)) @@ -2285,7 +2295,6 @@ static quicly_conn_t *create_connection(quicly_context_t *ctx, uint32_t protocol conn->retry_scid.len = UINT8_MAX; conn->idle_timeout.at = INT64_MAX; conn->idle_timeout.should_rearm_on_send = 1; - conn->stash.on_ack_stream.active_acked_cache.stream_id = INT64_MIN; *ptls_get_data_ptr(tls) = conn; @@ -3246,6 +3255,12 @@ struct st_quicly_send_context_t { * first packet number to be used within the lifetime of this send context */ uint64_t first_packet_number; + /** + * + */ + struct { + quicly_sent_t sent; /* if acked is set to non-NULL, it might be called */ + } on_streams; }; static int commit_send_packet(quicly_conn_t *conn, quicly_send_context_t *s, int coalesced) @@ -3376,6 +3391,7 @@ static int do_allocate_frame(quicly_conn_t *conn, quicly_send_context_t *s, size { int coalescible, ret; + assert(conn->crypto.tls != NULL && "cannot be QoS"); assert((s->current.first_byte & QUICLY_QUIC_BIT) != 0); /* allocate and setup the new packet if necessary */ @@ -3505,11 +3521,36 @@ static int do_allocate_frame(quicly_conn_t *conn, quicly_send_context_t *s, size return 0; } +static int qs_call_acked(quicly_conn_t *conn, quicly_send_context_t *s) +{ + if (s->on_streams.sent.acked == NULL) + return 0; + + static const quicly_sent_packet_t dummy_sent_packet = {}; + int ret = s->on_streams.sent.acked(&conn->egress.loss.sentmap, &dummy_sent_packet, 1, &s->on_streams.sent); + s->on_streams.sent.acked = NULL; + + assert(conn->stash.on_ack_stream.active_acked_cache.stream_id == INT64_MIN); + + return ret; +} + static int allocate_ack_eliciting_frame(quicly_conn_t *conn, quicly_send_context_t *s, size_t min_space, quicly_sent_t **sent, quicly_sent_acked_cb acked) { int ret; + if (conn->crypto.tls == NULL) { + /* QoS */ + if ((ret = qs_call_acked(conn, s)) != 0) + return ret; + if (min_space > s->dst_end - s->dst) + return QUICLY_ERROR_SENDBUF_FULL; + *sent = &s->on_streams.sent; + (*sent)->acked = acked; + return 0; + } + if ((ret = do_allocate_frame(conn, s, min_space, ALLOCATE_FRAME_TYPE_ACK_ELICITING)) != 0) return ret; if ((*sent = quicly_sentmap_allocate(&conn->egress.loss.sentmap, acked)) == NULL) @@ -4822,6 +4863,29 @@ static int send_other_control_frames(quicly_conn_t *conn, quicly_send_context_t return 0; } +static int do_send_core(quicly_conn_t *conn, quicly_send_context_t *s) +{ + int ret; + + /* send stream-level control frames */ + if ((ret = send_stream_control_frames(conn, s)) != 0) + goto Exit; + /* send STREAM frames */ + if ((ret = conn->super.ctx->stream_scheduler->do_send(conn->super.ctx->stream_scheduler, conn, s)) != 0) + goto Exit; + /* once more, send control frames related to streams, as the state might have changed */ + if ((ret = send_stream_control_frames(conn, s)) != 0) + goto Exit; + if ((conn->egress.pending_flows & QUICLY_PENDING_FLOW_OTHERS_BIT) != 0) { + if ((ret = send_other_control_frames(conn, s)) != 0) + goto Exit; + conn->egress.pending_flows &= ~QUICLY_PENDING_FLOW_OTHERS_BIT; + } + +Exit: + return ret; +} + static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) { int restrict_sending = 0, ack_only = 0, ret; @@ -4983,20 +5047,9 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) (ret = send_resumption_token(conn, s)) != 0) goto Exit; } - /* send stream-level control frames */ - if ((ret = send_stream_control_frames(conn, s)) != 0) - goto Exit; - /* send STREAM frames */ - if ((ret = conn->super.ctx->stream_scheduler->do_send(conn->super.ctx->stream_scheduler, conn, s)) != 0) + /* send streams and flow control information */ + if ((ret = do_send_core(conn, s)) != 0) goto Exit; - /* once more, send control frames related to streams, as the state might have changed */ - if ((ret = send_stream_control_frames(conn, s)) != 0) - goto Exit; - if ((conn->egress.pending_flows & QUICLY_PENDING_FLOW_OTHERS_BIT) != 0) { - if ((ret = send_other_control_frames(conn, s)) != 0) - goto Exit; - conn->egress.pending_flows &= ~QUICLY_PENDING_FLOW_OTHERS_BIT; - } /* stream operations might have requested emission of NEW_TOKEN at the tail; if so, try to bundle it */ if ((conn->egress.pending_flows & QUICLY_PENDING_FLOW_NEW_TOKEN_BIT) != 0) { assert(conn->application->one_rtt_writable); @@ -6209,8 +6262,35 @@ static int handle_ack_frequency_frame(quicly_conn_t *conn, struct st_quicly_hand return 0; } -static int handle_payload(quicly_conn_t *conn, size_t epoch, const uint8_t *_src, size_t _len, uint64_t *offending_frame_type, - int *is_ack_only) +static int handle_qs_transport_parameters_frame(quicly_conn_t *conn, struct st_quicly_handle_payload_state_t *state) +{ + uint64_t len; + int ret; + + if ((len = quicly_decodev(&state->src, state->end)) == UINT64_MAX) + return QUICLY_ERROR_PARTIAL_FRAME; + if (state->end - state->src < len) + return QUICLY_ERROR_PARTIAL_FRAME; + if ((ret = quicly_decode_transport_parameter_list(&conn->super.remote.transport_params, NULL, NULL, NULL, NULL, state->src, + state->src + len)) != 0) + return ret; + state->src += len; + + if ((ret = apply_remote_transport_params(conn)) != 0) + return ret; + + state->epoch = QUICLY_EPOCH_ON_STREAMS_OTHER; + + if (conn->super.remote.transport_params.max_streams_uni != 0) + open_blocked_streams(conn, 0); + if (conn->super.remote.transport_params.max_streams_bidi != 0) + open_blocked_streams(conn, 1); + + return 0; +} + +static int handle_payload(quicly_conn_t *conn, size_t _epoch, const uint8_t *_src, size_t _len, size_t *decoded_len, + uint64_t *offending_frame_type, int *is_ack_only) { /* clang-format off */ @@ -6221,81 +6301,84 @@ static int handle_payload(quicly_conn_t *conn, size_t epoch, const uint8_t *_src uint8_t ack_eliciting; /* boolean indicating if the frame is ack-eliciting */ size_t counter_offset; /* offset of corresponding `conn->super.stats.num_frames_received.type` within quicly_conn_t */ } frame_handlers[] = { -#define FRAME(n, i, z, h, o, ae) \ +#define FRAME(n, i, z, h, o, qs0, qs1, ae) \ { \ handle_##n##_frame, \ - (i << QUICLY_EPOCH_INITIAL) | (z << QUICLY_EPOCH_0RTT) | (h << QUICLY_EPOCH_HANDSHAKE) | (o << QUICLY_EPOCH_1RTT), \ + (i << QUICLY_EPOCH_INITIAL) | (z << QUICLY_EPOCH_0RTT) | (h << QUICLY_EPOCH_HANDSHAKE) | (o << QUICLY_EPOCH_1RTT) | \ + (qs0 << QUICLY_EPOCH_ON_STREAMS_TP) | (qs1 << QUICLY_EPOCH_ON_STREAMS_OTHER), \ ae, \ offsetof(quicly_conn_t, super.stats.num_frames_received.n) \ } - /* +----------------------+-------------------+---------------+ - * | | permitted epochs | | - * | frame +----+----+----+----+ ack-eliciting | - * | | IN | 0R | HS | 1R | | - * +----------------------+----+----+----+----+---------------+ */ - FRAME( padding , 1 , 1 , 1 , 1 , 0 ), /* 0 */ - FRAME( ping , 1 , 1 , 1 , 1 , 1 ), - FRAME( ack , 1 , 0 , 1 , 1 , 0 ), - FRAME( ack , 1 , 0 , 1 , 1 , 0 ), - FRAME( reset_stream , 0 , 1 , 0 , 1 , 1 ), - FRAME( stop_sending , 0 , 1 , 0 , 1 , 1 ), - FRAME( crypto , 1 , 0 , 1 , 1 , 1 ), - FRAME( new_token , 0 , 0 , 0 , 1 , 1 ), - FRAME( stream , 0 , 1 , 0 , 1 , 1 ), /* 8 */ - FRAME( stream , 0 , 1 , 0 , 1 , 1 ), - FRAME( stream , 0 , 1 , 0 , 1 , 1 ), - FRAME( stream , 0 , 1 , 0 , 1 , 1 ), - FRAME( stream , 0 , 1 , 0 , 1 , 1 ), - FRAME( stream , 0 , 1 , 0 , 1 , 1 ), - FRAME( stream , 0 , 1 , 0 , 1 , 1 ), - FRAME( stream , 0 , 1 , 0 , 1 , 1 ), - FRAME( max_data , 0 , 1 , 0 , 1 , 1 ), /* 16 */ - FRAME( max_stream_data , 0 , 1 , 0 , 1 , 1 ), - FRAME( max_streams_bidi , 0 , 1 , 0 , 1 , 1 ), - FRAME( max_streams_uni , 0 , 1 , 0 , 1 , 1 ), - FRAME( data_blocked , 0 , 1 , 0 , 1 , 1 ), - FRAME( stream_data_blocked , 0 , 1 , 0 , 1 , 1 ), - FRAME( streams_blocked , 0 , 1 , 0 , 1 , 1 ), - FRAME( streams_blocked , 0 , 1 , 0 , 1 , 1 ), - FRAME( new_connection_id , 0 , 1 , 0 , 1 , 1 ), /* 24 */ - FRAME( retire_connection_id , 0 , 0 , 0 , 1 , 1 ), - FRAME( path_challenge , 0 , 1 , 0 , 1 , 1 ), - FRAME( path_response , 0 , 0 , 0 , 1 , 1 ), - FRAME( transport_close , 1 , 1 , 1 , 1 , 0 ), - FRAME( application_close , 0 , 1 , 0 , 1 , 0 ), - FRAME( handshake_done , 0, 0 , 0 , 1 , 1 ), - /* +----------------------+----+----+----+----+---------------+ */ + /* +----------------------+-------------------------------+---------------+ + * | | permitted epochs | | + * | frame +----+----+----+----+-----+-----+ ack-eliciting | + * | | IN | 0R | HS | 1R | QS0 | QS1 | | + * +----------------------+----+----+----+----+-----+-----+---------------+ */ + FRAME( padding , 1 , 1 , 1 , 1 , 0 , 1 , 0 ), /* 0 */ + FRAME( ping , 1 , 1 , 1 , 1 , 0 , 0 , 1 ), + FRAME( ack , 1 , 0 , 1 , 1 , 0 , 0 , 0 ), + FRAME( ack , 1 , 0 , 1 , 1 , 0 , 0 , 0 ), + FRAME( reset_stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( stop_sending , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( crypto , 1 , 0 , 1 , 1 , 0 , 0 , 1 ), + FRAME( new_token , 0 , 0 , 0 , 1 , 0 , 0 , 1 ), + FRAME( stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), /* 8 */ + FRAME( stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( stream , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( max_data , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), /* 16 */ + FRAME( max_stream_data , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( max_streams_bidi , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( max_streams_uni , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( data_blocked , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( stream_data_blocked , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( streams_blocked , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( streams_blocked , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( new_connection_id , 0 , 1 , 0 , 1 , 0 , 0 , 1 ), /* 24 */ + FRAME( retire_connection_id , 0 , 0 , 0 , 1 , 0 , 0 , 1 ), + FRAME( path_challenge , 0 , 1 , 0 , 1 , 0 , 0 , 1 ), + FRAME( path_response , 0 , 0 , 0 , 1 , 0 , 0 , 1 ), + FRAME( transport_close , 1 , 1 , 1 , 1 , 0 , 1 , 0 ), + FRAME( application_close , 0 , 1 , 0 , 1 , 0 , 1 , 0 ), + FRAME( handshake_done , 0, 0 , 0 , 1 , 0 , 0 , 1 ), + /* +----------------------+----+----+----+----+-----+-----+---------------+ */ #undef FRAME }; static const struct { uint64_t type; struct st_quicly_frame_handler_t _; } ex_frame_handlers[] = { -#define FRAME(uc, lc, i, z, h, o, ae) \ +#define FRAME(uc, lc, i, z, h, o, qs0, qs1, ae) \ { \ QUICLY_FRAME_TYPE_##uc, \ { \ handle_##lc##_frame, \ - (i << QUICLY_EPOCH_INITIAL) | (z << QUICLY_EPOCH_0RTT) | (h << QUICLY_EPOCH_HANDSHAKE) | (o << QUICLY_EPOCH_1RTT), \ + (i << QUICLY_EPOCH_INITIAL) | (z << QUICLY_EPOCH_0RTT) | (h << QUICLY_EPOCH_HANDSHAKE) | (o << QUICLY_EPOCH_1RTT) | \ + (qs0 << QUICLY_EPOCH_ON_STREAMS_TP) | (qs1 << QUICLY_EPOCH_ON_STREAMS_OTHER), \ ae, \ offsetof(quicly_conn_t, super.stats.num_frames_received.lc) \ }, \ } - /* +----------------------------------+-------------------+---------------+ - * | frame | permitted epochs | | - * |------------------+---------------+----+----+----+----+ ack-eliciting | - * | upper-case | lower-case | IN | 0R | HS | 1R | | - * +------------------+---------------+----+----+----+----+---------------+ */ - FRAME( DATAGRAM_NOLEN , datagram , 0 , 1, 0, 1 , 1 ), - FRAME( DATAGRAM_WITHLEN , datagram , 0 , 1, 0, 1 , 1 ), - FRAME( ACK_FREQUENCY , ack_frequency , 0 , 0 , 0 , 1 , 1 ), - /* +------------------+---------------+-------------------+---------------+ */ + /* +----------------------------------------------------+-------------------------------+---------------+ + * | frame | permitted epochs | | + * |-------------------------+--------------------------+----+----+----+----+-----+-----+ ack-eliciting | + * | upper-case | lower-case | IN | 0R | HS | 1R | QS0 | QS1 | | + * +-------------------------+--------------------------+----+----+----+----+-----+-----+---------------+ */ + FRAME( DATAGRAM_NOLEN , datagram , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( DATAGRAM_WITHLEN , datagram , 0 , 1 , 0 , 1 , 0 , 1 , 1 ), + FRAME( ACK_FREQUENCY , ack_frequency , 0 , 0 , 0 , 1 , 0 , 0 , 1 ), + FRAME( QS_TRANSPORT_PARAMETERS , qs_transport_parameters , 0 , 0 , 0 , 0 , 1 , 0 , 1 ), + /* +-------------------------+--------------------------+----+----+----+----+-----+-----+---------------+ */ #undef FRAME {UINT64_MAX}, }; /* clang-format on */ - struct st_quicly_handle_payload_state_t state = {_src, _src + _len, epoch}; + struct st_quicly_handle_payload_state_t state = {_src, _src + _len, _epoch}; size_t num_frames_ack_eliciting = 0; int ret; @@ -6311,7 +6394,7 @@ static int handle_payload(quicly_conn_t *conn, size_t epoch, const uint8_t *_src if ((state.frame_type = quicly_decodev(&state.src, state.end)) == UINT64_MAX) { state.frame_type = QUICLY_FRAME_TYPE_PADDING; /* we cannot signal the offending frame type when failing to decode the frame type */ - ret = QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + ret = QUICLY_ERROR_PARTIAL_FRAME; break; } size_t i; @@ -6324,7 +6407,7 @@ static int handle_payload(quicly_conn_t *conn, size_t epoch, const uint8_t *_src frame_handler = &ex_frame_handlers[i]._; } /* check if frame is allowed, then process */ - if ((frame_handler->permitted_epochs & (1 << epoch)) == 0) { + if ((frame_handler->permitted_epochs & (1 << state.epoch)) == 0) { ret = QUICLY_TRANSPORT_ERROR_PROTOCOL_VIOLATION; break; } @@ -6335,8 +6418,19 @@ static int handle_payload(quicly_conn_t *conn, size_t epoch, const uint8_t *_src } while (state.src != state.end); *is_ack_only = num_frames_ack_eliciting == 0; - if (ret != 0) + + if (ret == QUICLY_ERROR_PARTIAL_FRAME && decoded_len == NULL) + ret = QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; + switch (ret) { + case 0: + case QUICLY_ERROR_PARTIAL_FRAME: + if (decoded_len != NULL) + *decoded_len = _len - (state.end - state.src); + break; + default: *offending_frame_type = state.frame_type; + break; + } return ret; } @@ -6459,7 +6553,8 @@ int quicly_accept(quicly_conn_t **conn, quicly_context_t *ctx, struct sockaddr * if (packet->ecn != 0) (*conn)->super.stats.num_packets.received_ecn_counts[get_ecn_index_from_bits(packet->ecn)] += 1; (*conn)->super.stats.num_bytes.received += packet->datagram_size; - if ((ret = handle_payload(*conn, QUICLY_EPOCH_INITIAL, payload.base, payload.len, &offending_frame_type, &is_ack_only)) != 0) + if ((ret = handle_payload(*conn, QUICLY_EPOCH_INITIAL, payload.base, payload.len, NULL, &offending_frame_type, &is_ack_only)) != + 0) goto Exit; if ((ret = record_receipt(&(*conn)->initial->super, pn, packet->ecn, 0, (*conn)->stash.now, &(*conn)->egress.send_ack_at, &(*conn)->super.stats.num_packets.received_out_of_order)) != 0) @@ -6698,7 +6793,7 @@ int quicly_receive(quicly_conn_t *conn, struct sockaddr *dest_addr, struct socka } /* handle the payload */ - if ((ret = handle_payload(conn, epoch, payload.base, payload.len, &offending_frame_type, &is_ack_only)) != 0) + if ((ret = handle_payload(conn, epoch, payload.base, payload.len, NULL, &offending_frame_type, &is_ack_only)) != 0) goto Exit; if (*space != NULL && conn->super.state < QUICLY_STATE_CLOSING) { if ((ret = record_receipt(*space, pn, packet->ecn, is_ack_only, conn->stash.now, &conn->egress.send_ack_at, @@ -7215,5 +7310,86 @@ void quicly__debug_printf(quicly_conn_t *conn, const char *function, int line, c #endif } +static int emit_qs_transport_parameters(quicly_conn_t *conn, quicly_send_context_t *s) +{ + quicly_sent_t *sent; + ptls_buffer_t buf; + int ret; + + if ((ret = allocate_ack_eliciting_frame(conn, s, 100, &sent, NULL)) != 0) + return ret; + ptls_buffer_init(&buf, s->dst, 100); + + ptls_buffer_push_quicint(&buf, QUICLY_FRAME_TYPE_QS_TRANSPORT_PARAMETERS); + ptls_buffer_push_block(&buf, -1, { + if ((ret = quicly_encode_transport_parameter_list(&buf, &conn->super.ctx->transport_params, NULL, NULL, NULL, NULL, 0)) != + 0) + goto Exit; + }); + + assert(!buf.is_allocated); + s->dst += buf.off; + +Exit: + return ret; +} + +int quicly_qos_send(quicly_conn_t *conn, void *buf, size_t *bufsize) +{ + quicly_send_context_t s = {.dst = buf, .dst_end = (uint8_t *)buf + *bufsize, .max_datagrams = 1}; + int ret; + + lock_now(conn, 0); + + if (conn->super.state == QUICLY_STATE_FIRSTFLIGHT) { + if ((ret = emit_qs_transport_parameters(conn, &s)) != 0) + goto Exit; + conn->super.state = QUICLY_STATE_CONNECTED; + } + if ((ret = do_send_core(conn, &s)) != 0 && ret != QUICLY_ERROR_SENDBUF_FULL) + goto Exit; + if ((ret = qs_call_acked(conn, &s)) != 0) + goto Exit; + +Exit: + if (ret == 0) + *bufsize = s.dst - (uint8_t *)buf; + unlock_now(conn); + return ret; +} + +int quicly_qos_receive(quicly_conn_t *conn, const void *src, size_t *len) +{ + size_t epoch = conn->super.stats.num_frames_received.qs_transport_parameters == 0 ? QUICLY_EPOCH_ON_STREAMS_TP + : QUICLY_EPOCH_ON_STREAMS_OTHER; + uint64_t offending_frame_type = QUICLY_FRAME_TYPE_PADDING; + int is_ack_only, ret; + + lock_now(conn, 0); + + ret = handle_payload(conn, epoch, src, *len, len, &offending_frame_type, &is_ack_only); + + unlock_now(conn); + + return ret; +} + +quicly_conn_t *quicly_qos_new(quicly_context_t *ctx, int is_client, void *appdata) +{ + quicly_conn_t *conn; + + if ((conn = malloc(sizeof(*conn))) == NULL) + return NULL; + + memset(conn, 0, sizeof(*conn)); + conn->super.ctx = ctx; + conn->super.data = appdata; + lock_now(conn, 0); + init_connection_core(conn, is_client); + unlock_now(conn); + + return conn; +} + const uint32_t quicly_supported_versions[] = {QUICLY_PROTOCOL_VERSION_1, QUICLY_PROTOCOL_VERSION_DRAFT29, QUICLY_PROTOCOL_VERSION_DRAFT27, 0}; From c598fde6947699afc586f824d882d3f1f3dac478 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Sun, 17 Mar 2024 21:04:07 +1000 Subject: [PATCH 02/16] add test --- t/test.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/t/test.c b/t/test.c index 5be11562..c60d036c 100644 --- a/t/test.c +++ b/t/test.c @@ -740,6 +740,41 @@ static void test_jumpstart_cwnd(void) ok(derive_jumpstart_cwnd(&bounded_max, 250, 1000000, 250) == 80000); } +static void test_on_streams(void) +{ + quicly_conn_t *client_conn = quicly_qos_new(&quic_ctx, 1, NULL), *server_conn = quicly_qos_new(&quic_ctx, 0, NULL); + quicly_stream_t *client_stream = NULL, *server_stream; + quicly_streambuf_t *server_streambuf; + char buf[10480]; + size_t bufsize; + int ret; + + bufsize = sizeof(buf); + ret = quicly_qos_send(server_conn, buf, &bufsize); + ok(ret == 0); + + ret = quicly_qos_receive(client_conn, buf, &bufsize); + ok(ret == 0); + + ret = quicly_open_stream(client_conn, &client_stream, 0); + ok(ret == 0); + ret = quicly_streambuf_egress_write(client_stream, "hello", 5); + ok(ret == 0); + + bufsize = sizeof(buf); + ret = quicly_qos_send(client_conn, buf, &bufsize); + ok(ret == 0); + + ret = quicly_qos_receive(server_conn, buf, &bufsize); + ok(ret == 0); + + server_stream = quicly_get_stream(server_conn, client_stream->stream_id); + ok(server_stream != NULL); + server_streambuf = server_stream->data; + ok(server_streambuf->ingress.off == 5); + ok(memcmp(server_streambuf->ingress.base, "hello", 5) == 0); +} + int main(int argc, char **argv) { static ptls_iovec_t cert; @@ -819,6 +854,7 @@ int main(int argc, char **argv) subtest("ecn-index-from-bits", test_ecn_index_from_bits); subtest("jumpstart-cwnd", test_jumpstart_cwnd); subtest("jumpstart", test_jumpstart); + subtest("on-streams", test_on_streams); return done_testing(); } From 34524abd26e182e076a007882398cd906cc1263f Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 18 Mar 2024 07:41:29 +1000 Subject: [PATCH 03/16] missing proto --- include/quicly.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/quicly.h b/include/quicly.h index bf46a647..fcdf6fd9 100644 --- a/include/quicly.h +++ b/include/quicly.h @@ -1356,6 +1356,7 @@ extern const quicly_stream_callbacks_t quicly_stream_noop_callbacks; } while (0) int quicly_qos_send(quicly_conn_t *conn, void *buf, size_t *bufsize); +int quicly_qos_receive(quicly_conn_t *conn, const void *src, size_t *len); quicly_conn_t *quicly_qos_new(quicly_context_t *ctx, int is_client, void *appdata); /* inline definitions */ From 1255c32db31419867a139114a4887fee0b11d470 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 18 Mar 2024 15:33:43 +1000 Subject: [PATCH 04/16] let `quicly_get_first_timeout` be usable with QoS --- include/quicly.h | 12 ++++++++++++ lib/quicly.c | 11 ++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/include/quicly.h b/include/quicly.h index fcdf6fd9..4423076b 100644 --- a/include/quicly.h +++ b/include/quicly.h @@ -166,6 +166,10 @@ QUICLY_CALLBACK_TYPE(void, update_open_count, ssize_t delta); * is complete. */ QUICLY_CALLBACK_TYPE(void, async_handshake, ptls_t *tls); +/** + * + */ +QUICLY_CALLBACK_TYPE(int, qos_is_writing, quicly_conn_t *conn); /** * crypto offload API @@ -400,6 +404,10 @@ struct st_quicly_context_t { * */ quicly_async_handshake_t *async_handshake; + /** + * + */ + quicly_qos_is_writing_t *qos_is_writing; }; /** @@ -967,6 +975,10 @@ static const quicly_cid_t *quicly_get_remote_cid(quicly_conn_t *conn); * */ static const quicly_transport_parameters_t *quicly_get_remote_transport_parameters(quicly_conn_t *conn); +/** + * + */ +int quicly_is_on_streams(quicly_conn_t *conn); /** * */ diff --git a/lib/quicly.c b/lib/quicly.c index e131bf36..5934990d 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -856,6 +856,11 @@ static int update_max_streams(struct st_quicly_max_streams_t *m, uint64_t count) return 0; } +int quicly_is_on_streams(quicly_conn_t *conn) +{ + return conn->crypto.tls == NULL; +} + int quicly_connection_is_ready(quicly_conn_t *conn) { return conn->application != NULL; @@ -3092,6 +3097,9 @@ static inline uint64_t calc_amplification_limit_allowance(quicly_conn_t *conn) static size_t calc_send_window(quicly_conn_t *conn, size_t min_bytes_to_send, uint64_t amp_window, uint64_t pacer_window, int restrict_sending) { + if (quicly_is_on_streams(conn)) + return conn->super.ctx->qos_is_writing->cb(conn->super.ctx->qos_is_writing, conn) ? 0 : SIZE_MAX; + uint64_t window = 0; if (restrict_sending) { /* Send min_bytes_to_send on PTO */ @@ -3152,7 +3160,8 @@ int64_t quicly_get_first_timeout(quicly_conn_t *conn) if (conn->egress.pending_flows != 0) { /* crypto streams (as indicated by lower 4 bits) can be sent whenever CWND is available; other flows need application * packet number space */ - if ((conn->application != NULL && conn->application->cipher.egress.key.header_protection != NULL) || + if (quicly_is_on_streams(conn) || + (conn->application != NULL && conn->application->cipher.egress.key.header_protection != NULL) || (conn->egress.pending_flows & 0xf) != 0) at = pacer_at; } From 30429d0073dab129dcf19dd1aa2cff530c2001cc Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 18 Mar 2024 15:36:57 +1000 Subject: [PATCH 05/16] QUICv1 payload cannot be empty but `quicly_qos_receive` should accept zero-byte input --- lib/quicly.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index 5934990d..156dd4b7 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -7372,11 +7372,12 @@ int quicly_qos_receive(quicly_conn_t *conn, const void *src, size_t *len) size_t epoch = conn->super.stats.num_frames_received.qs_transport_parameters == 0 ? QUICLY_EPOCH_ON_STREAMS_TP : QUICLY_EPOCH_ON_STREAMS_OTHER; uint64_t offending_frame_type = QUICLY_FRAME_TYPE_PADDING; - int is_ack_only, ret; + int is_ack_only, ret = 0; lock_now(conn, 0); - ret = handle_payload(conn, epoch, src, *len, len, &offending_frame_type, &is_ack_only); + if (*len != 0) + ret = handle_payload(conn, epoch, src, *len, len, &offending_frame_type, &is_ack_only); unlock_now(conn); From ffdc224d2c112577b05c8e347d10c94ae032b506 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 18 Mar 2024 15:38:56 +1000 Subject: [PATCH 06/16] otherwise QoS connections cannot be freed --- lib/quicly.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/quicly.c b/lib/quicly.c index 156dd4b7..6767ecc2 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -1731,7 +1731,7 @@ void quicly_free(quicly_conn_t *conn) if (conn->crypto.async_in_progress) { /* When async signature generation is inflight, `ptls_free` will be called from `quicly_resume_handshake` laterwards. */ *ptls_get_data_ptr(conn->crypto.tls) = NULL; - } else { + } else if (conn->crypto.tls != NULL) { ptls_free(conn->crypto.tls); } From 35a4dcef17b4f809109a640db1efde89dc36f4af Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 18 Mar 2024 15:39:06 +1000 Subject: [PATCH 07/16] QoS uses idle timeout --- lib/quicly.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/quicly.c b/lib/quicly.c index 6767ecc2..8a91dab0 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -2219,6 +2219,7 @@ static void init_connection_core(quicly_conn_t *conn, int is_client) 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); + conn->idle_timeout.at = INT64_MAX; conn->stash.on_ack_stream.active_acked_cache.stream_id = INT64_MIN; } @@ -2298,7 +2299,6 @@ static quicly_conn_t *create_connection(quicly_context_t *ctx, uint32_t protocol } conn->crypto.handshake_properties.collect_extension = collect_transport_parameters; conn->retry_scid.len = UINT8_MAX; - conn->idle_timeout.at = INT64_MAX; conn->idle_timeout.should_rearm_on_send = 1; *ptls_get_data_ptr(tls) = conn; From a3bc946b81017fc7bc7f467d3f062d6a7b5e7ff4 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 18 Mar 2024 15:48:16 +1000 Subject: [PATCH 08/16] [QoS] data can be sent without application space --- lib/quicly.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/quicly.c b/lib/quicly.c index 8a91dab0..7390c214 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -1356,7 +1356,7 @@ static int scheduler_can_send(quicly_conn_t *conn) } /* scheduler would never have data to send, until application keys become available */ - if (conn->application == NULL) + if (!quicly_is_on_streams(conn) && conn->application == NULL) return 0; int conn_is_saturated = !(conn->egress.max_data.sent < conn->egress.max_data.permitted); From 0920322f4640e836f57c07c889f610ee9589779c Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 18 Mar 2024 15:50:00 +1000 Subject: [PATCH 09/16] remove unused property --- lib/quicly.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index 7390c214..f4913bcb 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -350,10 +350,6 @@ struct st_quicly_conn_t { * */ uint8_t try_jumpstart : 1; - /** - * - */ - uint8_t is_on_streams : 1; /** * pending RETIRE_CONNECTION_ID frames to be sent */ From 939684e28550c5f3c54dc6dc1670fdec7c4262db Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 18 Mar 2024 15:58:06 +1000 Subject: [PATCH 10/16] respect idle timeout in QoS --- lib/quicly.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/quicly.c b/lib/quicly.c index f4913bcb..352f1604 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -7346,6 +7346,12 @@ int quicly_qos_send(quicly_conn_t *conn, void *buf, size_t *bufsize) lock_now(conn, 0); + if (conn->idle_timeout.at <= conn->stash.now) { + conn->super.state = QUICLY_STATE_DRAINING; + destroy_all_streams(conn, 0, 0); + return QUICLY_ERROR_FREE_CONNECTION; + } + if (conn->super.state == QUICLY_STATE_FIRSTFLIGHT) { if ((ret = emit_qs_transport_parameters(conn, &s)) != 0) goto Exit; From 04c84157de94fbe9bfb1b2f94d0ec131a5fff2f2 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 22 Jul 2024 14:03:41 -0700 Subject: [PATCH 11/16] If the STREAM frame is emitted as the 2nd frame of the buffer provided in QoS mode, available space could be smaller than max_frame_size. But because the frame is being written up to the end of the buffer, Length field is omitted. Emitting Length field alaways is a tentative fix. --- lib/quicly.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/quicly.c b/lib/quicly.c index 352f1604..a8c2eb8c 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -3836,6 +3836,12 @@ static inline void adjust_stream_frame_layout(uint8_t **dst, uint8_t *const dst_ } else { /* STREAM frame: insert length if space can be left for more frames. Otherwise, retain STREAM frame header omitting the * length field, prepending PADDING if necessary. */ +#if 1 /* for the time being always emit the length field */ + if (space_left < len_of_len) { + *len = dst_end - *dst - len_of_len; + *wrote_all = 0; + } +#else if (space_left <= len_of_len) { if (space_left != 0) { memmove(*frame_at + space_left, *frame_at, *dst + *len - *frame_at); @@ -3846,6 +3852,7 @@ static inline void adjust_stream_frame_layout(uint8_t **dst, uint8_t *const dst_ *dst += *len; return; } +#endif **frame_at |= QUICLY_FRAME_TYPE_STREAM_BIT_LEN; } From 46b62ad5f80b5f3fe6b17c7d611476778578f366 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Sun, 4 Aug 2024 10:03:38 +0900 Subject: [PATCH 12/16] revert 04c8415 to add failing test --- lib/quicly.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index 686407a6..b470f418 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -4213,12 +4213,6 @@ static inline void adjust_stream_frame_layout(uint8_t **dst, uint8_t *const dst_ } else { /* STREAM frame: insert length if space can be left for more frames. Otherwise, retain STREAM frame header omitting the * length field, prepending PADDING if necessary. */ -#if 1 /* for the time being always emit the length field */ - if (space_left < len_of_len) { - *len = dst_end - *dst - len_of_len; - *wrote_all = 0; - } -#else if (space_left <= len_of_len) { if (space_left != 0) { memmove(*frame_at + space_left, *frame_at, *dst + *len - *frame_at); @@ -4229,7 +4223,6 @@ static inline void adjust_stream_frame_layout(uint8_t **dst, uint8_t *const dst_ *dst += *len; return; } -#endif **frame_at |= QUICLY_FRAME_TYPE_STREAM_BIT_LEN; } From 1f531c348ad343ab7d69bcac9d2f291a9e0c55c7 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Sun, 4 Aug 2024 11:04:16 +0900 Subject: [PATCH 13/16] [QoS] when decoding STREAM frames, respect max_frame_size currently hard-coded to 16384 --- include/quicly/frame.h | 31 +++++++++++++++++++++---------- lib/quicly.c | 5 ++++- t/frame.c | 2 +- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/include/quicly/frame.h b/include/quicly/frame.h index db4df7fc..8294dddb 100644 --- a/include/quicly/frame.h +++ b/include/quicly/frame.h @@ -106,7 +106,8 @@ typedef struct st_quicly_stream_frame_t { ptls_iovec_t data; } quicly_stream_frame_t; -static int quicly_decode_stream_frame(uint8_t type_flags, const uint8_t **src, const uint8_t *end, quicly_stream_frame_t *frame); +static int quicly_decode_stream_frame(uint8_t type_flags, uint64_t max_frame_size, const uint8_t **src, const uint8_t *end, + quicly_stream_frame_t *frame); static uint8_t *quicly_encode_crypto_frame_header(uint8_t *dst, uint8_t *dst_end, uint64_t offset, size_t *data_len); static int quicly_decode_crypto_frame(const uint8_t **src, const uint8_t *end, quicly_stream_frame_t *frame); @@ -369,16 +370,17 @@ inline unsigned quicly_clz64(uint64_t v) return v != 0 ? __builtin_clzll(v) : 64; } -inline int quicly_decode_stream_frame(uint8_t type_flags, const uint8_t **src, const uint8_t *end, quicly_stream_frame_t *frame) +inline int quicly_decode_stream_frame(uint8_t type_flags, uint64_t max_frame_size, const uint8_t **src, const uint8_t *end, + quicly_stream_frame_t *frame) { /* obtain stream id */ if ((frame->stream_id = quicly_decodev(src, end)) == UINT64_MAX) - goto Error; + return QUICLY_ERROR_PARTIAL_FRAME; /* obtain offset */ if ((type_flags & QUICLY_FRAME_TYPE_STREAM_BIT_OFF) != 0) { if ((frame->offset = quicly_decodev(src, end)) == UINT64_MAX) - goto Error; + return QUICLY_ERROR_PARTIAL_FRAME; } else { frame->offset = 0; } @@ -387,22 +389,31 @@ inline int quicly_decode_stream_frame(uint8_t type_flags, const uint8_t **src, c if ((type_flags & QUICLY_FRAME_TYPE_STREAM_BIT_LEN) != 0) { uint64_t len; if ((len = quicly_decodev(src, end)) == UINT64_MAX) - goto Error; + return QUICLY_ERROR_PARTIAL_FRAME; if ((uint64_t)(end - *src) < len) - goto Error; + return QUICLY_ERROR_PARTIAL_FRAME; + if (len > max_frame_size) + return QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; frame->data = ptls_iovec_init(*src, len); *src += len; } else { - frame->data = ptls_iovec_init(*src, end - *src); - *src = end; + if (max_frame_size == SIZE_MAX) { + /* QUIC v1 */ + frame->data = ptls_iovec_init(*src, end - *src); + *src = end; + } else { + /* QUIC on Streams */ + if (end - *src < max_frame_size) + return QUICLY_ERROR_PARTIAL_FRAME; + frame->data = ptls_iovec_init(*src, max_frame_size); + *src += max_frame_size; + } } /* fin bit */ frame->is_fin = (type_flags & QUICLY_FRAME_TYPE_STREAM_BIT_FIN) != 0; return 0; -Error: - return QUICLY_ERROR_PARTIAL_FRAME; } inline uint8_t *quicly_encode_crypto_frame_header(uint8_t *dst, uint8_t *dst_end, uint64_t offset, size_t *data_len) diff --git a/lib/quicly.c b/lib/quicly.c index b470f418..16072b78 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -5881,7 +5881,10 @@ static int handle_stream_frame(quicly_conn_t *conn, struct st_quicly_handle_payl quicly_stream_t *stream; int ret; - if ((ret = quicly_decode_stream_frame(state->frame_type, &state->src, state->end, &frame)) != 0) + if ((ret = quicly_decode_stream_frame( + state->frame_type, + quicly_is_on_streams(conn) ? 16384 /* hard-coded, until TP ID of max_frame_size is defined */ : SIZE_MAX, &state->src, + state->end, &frame)) != 0) return ret; QUICLY_PROBE(QUICTRACE_RECV_STREAM, conn, conn->stash.now, frame.stream_id, frame.offset, frame.data.len, (int)frame.is_fin); if ((ret = quicly_get_or_open_stream(conn, frame.stream_id, &stream)) != 0 || stream == NULL) diff --git a/t/frame.c b/t/frame.c index 2e53fc95..d99f4dea 100644 --- a/t/frame.c +++ b/t/frame.c @@ -177,7 +177,7 @@ static void test_mozquic(void) quicly_stream_frame_t frame; static const char *mess = "\xc5\0\0\0\0\0\0\xb6\x16\x03"; const uint8_t *p = (void *)mess, type_flags = *p++; - quicly_decode_stream_frame(type_flags, &p, p + 9, &frame); + quicly_decode_stream_frame(type_flags, SIZE_MAX, &p, p + 9, &frame); } void test_frame(void) From 1de8f38a9f5be843e90c64e224ef4dd2f6846b32 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Sun, 4 Aug 2024 11:09:26 +0900 Subject: [PATCH 14/16] add test --- t/test.c | 64 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/t/test.c b/t/test.c index cf1cc1b8..a4b97657 100644 --- a/t/test.c +++ b/t/test.c @@ -742,37 +742,69 @@ static void test_jumpstart_cwnd(void) static void test_on_streams(void) { - quicly_conn_t *client_conn = quicly_qos_new(&quic_ctx, 1, NULL), *server_conn = quicly_qos_new(&quic_ctx, 0, NULL); - quicly_stream_t *client_stream = NULL, *server_stream; - quicly_streambuf_t *server_streambuf; - char buf[10480]; - size_t bufsize; + static char message16k[16384]; + + if (message16k[0] == '\0') { + for (size_t i = 0; i < sizeof(message16k); i += 16) + memcpy(message16k + i, "helloworldhello\n", 16); + } + + quicly_conn_t *cc = quicly_qos_new(&quic_ctx, 1, NULL), *sc = quicly_qos_new(&quic_ctx, 0, NULL); + quicly_stream_t *cs1 = NULL, *cs2 = NULL, *ss1, *ss2; + quicly_streambuf_t *ss1buf, *ss2buf; + char buf[16384]; + size_t bufsize, decoded_len; int ret; bufsize = sizeof(buf); - ret = quicly_qos_send(server_conn, buf, &bufsize); + ret = quicly_qos_send(sc, buf, &bufsize); ok(ret == 0); - ret = quicly_qos_receive(client_conn, buf, &bufsize); + ret = quicly_qos_receive(cc, buf, &bufsize); + ok(ret == 0); + + ret = quicly_open_stream(cc, &cs1, 0); + ok(ret == 0); + ret = quicly_streambuf_egress_write(cs1, "hello", 5); + ok(ret == 0); + ret = quicly_open_stream(cc, &cs2, 0); + ok(ret == 0); + ret = quicly_streambuf_egress_write(cs2, message16k, sizeof(message16k)); ok(ret == 0); - ret = quicly_open_stream(client_conn, &client_stream, 0); + bufsize = sizeof(buf); + ret = quicly_qos_send(cc, buf, &bufsize); ok(ret == 0); - ret = quicly_streambuf_egress_write(client_stream, "hello", 5); + + decoded_len = bufsize; /* TODO add test for partial frame receive */ + ret = quicly_qos_receive(sc, buf, &decoded_len); ok(ret == 0); + ok(decoded_len == bufsize); + + ss1 = quicly_get_stream(sc, cs1->stream_id); + ok(ss1 != NULL); + ss1buf = ss1->data; + ok(ss1buf->ingress.off == 5); + ok(memcmp(ss1buf->ingress.base, "hello", 5) == 0); + + ss2 = quicly_get_stream(sc, cs2->stream_id); + ok(ss2 != NULL); + ss2buf = ss2->data; + ok(ss2buf->ingress.off >= sizeof(message16k) - 400); + ok(ss2buf->ingress.off < sizeof(message16k)); + ok(memcmp(ss2buf->ingress.base, message16k, ss2buf->ingress.off) == 0); bufsize = sizeof(buf); - ret = quicly_qos_send(client_conn, buf, &bufsize); + ret = quicly_qos_send(cc, buf, &bufsize); ok(ret == 0); - ret = quicly_qos_receive(server_conn, buf, &bufsize); + decoded_len = bufsize; + ret = quicly_qos_receive(sc, buf, &decoded_len); ok(ret == 0); + ok(decoded_len == bufsize); - server_stream = quicly_get_stream(server_conn, client_stream->stream_id); - ok(server_stream != NULL); - server_streambuf = server_stream->data; - ok(server_streambuf->ingress.off == 5); - ok(memcmp(server_streambuf->ingress.base, "hello", 5) == 0); + ok(ss2buf->ingress.off == sizeof(message16k)); + ok(memcmp(ss2buf->ingress.base, message16k, ss2buf->ingress.off) == 0); } int main(int argc, char **argv) From 02300e996681dd46ff7ed0241fdec586e1d2f3d6 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 5 Aug 2024 10:07:25 +0900 Subject: [PATCH 15/16] When sending QUIC frames on QoS, always use STREAM frame with the length field. In addition, as of this commit, FIN-only frame will always carries the length field, even if the protocol is QUIC v1. We anticipate the downside of the change to be negligible as such a frame can be used only at the tail of the packet payload. --- lib/quicly.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index 16072b78..44105c3d 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -4204,9 +4204,11 @@ static inline void adjust_stream_frame_layout(uint8_t **dst, uint8_t *const dst_ { size_t space_left = (dst_end - *dst) - *len, len_of_len = quicly_encodev_capacity(*len); - if (**frame_at == QUICLY_FRAME_TYPE_CRYPTO) { - /* CRYPTO frame: adjust payload length to make space for the length field, if necessary. */ + if (**frame_at == QUICLY_FRAME_TYPE_CRYPTO || (**frame_at & QUICLY_FRAME_TYPE_STREAM_BIT_LEN) != 0) { + /* CRYPTO frame or QoS, in which case we always prepend length: adjust payload length to make space for the length field, if + * necessary. */ if (space_left < len_of_len) { + assert(dst_end - *dst >= len_of_len); *len = dst_end - *dst - len_of_len; *wrote_all = 0; } @@ -4263,13 +4265,10 @@ int quicly_send_stream(quicly_stream_t *stream, quicly_send_context_t *s) if (off == stream->sendstate.final_size) { assert(!quicly_sendstate_is_open(&stream->sendstate)); /* special case for emitting FIN only */ - header[0] |= QUICLY_FRAME_TYPE_STREAM_BIT_FIN; + header[0] |= QUICLY_FRAME_TYPE_STREAM_BIT_LEN | QUICLY_FRAME_TYPE_STREAM_BIT_FIN; + hp = quicly_encodev(hp, 0); /* length=0 */ if ((ret = allocate_ack_eliciting_frame(stream->conn, s, hp - header, &sent, on_ack_stream)) != 0) return ret; - if (hp - header != s->dst_end - s->dst) { - header[0] |= QUICLY_FRAME_TYPE_STREAM_BIT_LEN; - *hp++ = 0; /* empty length */ - } memcpy(s->dst, header, hp - header); s->dst += hp - header; len = 0; @@ -4277,6 +4276,8 @@ int quicly_send_stream(quicly_stream_t *stream, quicly_send_context_t *s) is_fin = 1; goto UpdateState; } + if (quicly_is_on_streams(stream->conn)) + header[0] |= QUICLY_FRAME_TYPE_STREAM_BIT_LEN; if ((ret = allocate_ack_eliciting_frame(stream->conn, s, hp - header + 1, &sent, on_ack_stream)) != 0) return ret; dst = s->dst; From 557aad4eb00cc179292f4bd0bcd601fcd07bd7ae Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Mon, 12 Aug 2024 14:54:10 +0900 Subject: [PATCH 16/16] to suppress signed comparison warning, follow the convention of casting ptrdiff_t to unsigned --- include/quicly/frame.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/quicly/frame.h b/include/quicly/frame.h index 8294dddb..e546d8c5 100644 --- a/include/quicly/frame.h +++ b/include/quicly/frame.h @@ -403,7 +403,7 @@ inline int quicly_decode_stream_frame(uint8_t type_flags, uint64_t max_frame_siz *src = end; } else { /* QUIC on Streams */ - if (end - *src < max_frame_size) + if ((uint64_t)(end - *src) < max_frame_size) return QUICLY_ERROR_PARTIAL_FRAME; frame->data = ptls_iovec_init(*src, max_frame_size); *src += max_frame_size;