From f0b5801b799db3e45066f2fc8238539c9546837a Mon Sep 17 00:00:00 2001 From: Aloys Augustin Date: Thu, 2 Apr 2020 12:54:17 +0200 Subject: [PATCH 1/3] prevent cwnd overflow --- lib/cc-reno.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/cc-reno.c b/lib/cc-reno.c index 93a47684a..1a4b179e6 100644 --- a/lib/cc-reno.c +++ b/lib/cc-reno.c @@ -43,7 +43,10 @@ void quicly_cc_on_acked(quicly_cc_t *cc, uint32_t bytes, uint64_t largest_acked, // slow start if (cc->cwnd < cc->ssthresh) { - cc->cwnd += bytes; + if (cc->cwnd > UINT32_MAX - bytes) + cc->cwnd = UINT32_MAX; + else + cc->cwnd += bytes; return; } // congestion avoidance @@ -53,7 +56,11 @@ void quicly_cc_on_acked(quicly_cc_t *cc, uint32_t bytes, uint64_t largest_acked, // increase cwnd by 1 MSS per cwnd acked uint32_t count = cc->stash / cc->cwnd; cc->stash -= count * cc->cwnd; - cc->cwnd += count * QUICLY_MAX_PACKET_SIZE; + uint32_t increase = count * QUICLY_MAX_PACKET_SIZE; + if (cc->cwnd < UINT32_MAX - increase) + cc->cwnd += increase; + else + cc->cwnd = UINT32_MAX; } void quicly_cc_on_lost(quicly_cc_t *cc, uint32_t bytes, uint64_t lost_pn, uint64_t next_pn) From e6c7895f74103b811dee3a3c9e2660e5e97c2ba4 Mon Sep 17 00:00:00 2001 From: Aloys Augustin Date: Thu, 2 Apr 2020 12:44:33 +0200 Subject: [PATCH 2/3] Use a dlist of packets for the sentmap. This improves the throughput of quicly by 5-7% in benchmarks. The discard code used to be a bottleneck. This structure allows removing packets from the sentmap in O(1) time. --- include/quicly/sentmap.h | 159 +++++++++++++-------------------- lib/quicly.c | 54 +++++------ lib/sentmap.c | 188 ++++++++++++++++++--------------------- t/sentmap.c | 33 ++++--- 4 files changed, 192 insertions(+), 242 deletions(-) diff --git a/include/quicly/sentmap.h b/include/quicly/sentmap.h index 9082dfeb9..a7773f213 100644 --- a/include/quicly/sentmap.h +++ b/include/quicly/sentmap.h @@ -32,31 +32,12 @@ extern "C" { #include "quicly/maxsender.h" #include "quicly/sendstate.h" +#define QUICLY_SENTMAP_DEFAULT_FRAMES_PER_PACKET 8 + struct st_quicly_conn_t; -typedef struct st_quicly_sent_t quicly_sent_t; -typedef struct st_quicly_sent_packet_t { - /** - * - */ - uint64_t packet_number; - /** - * - */ - int64_t sent_at; - /** - * epoch to be acked in - */ - uint8_t ack_epoch; - /** - * - */ - uint8_t ack_eliciting : 1; - /** - * number of bytes in-flight for the packet (becomes zero once deemed lost) - */ - uint16_t bytes_in_flight; -} quicly_sent_packet_t; +typedef struct st_quicly_sent_packet_t quicly_sent_packet_t; +typedef struct st_quicly_sent_frame_t quicly_sent_frame_t; typedef enum en_quicly_sentmap_event_t { /** @@ -73,13 +54,12 @@ typedef enum en_quicly_sentmap_event_t { QUICLY_SENTMAP_EVENT_EXPIRED } quicly_sentmap_event_t; -typedef int (*quicly_sent_acked_cb)(struct st_quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *data, +typedef int (*quicly_sent_acked_cb)(struct st_quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *data, quicly_sentmap_event_t event); -struct st_quicly_sent_t { +struct st_quicly_sent_frame_t { quicly_sent_acked_cb acked; union { - quicly_sent_packet_t packet; struct { quicly_range_t range; } ack; @@ -112,23 +92,45 @@ struct st_quicly_sent_t { } data; }; -struct st_quicly_sent_block_t { +typedef struct st_dlist_t { + struct st_dlist_t *prev; + struct st_dlist_t *next; +} dlist_t; + +struct st_quicly_sent_packet_t { + dlist_t pkt_list; /** - * next block if exists (or NULL) + * */ - struct st_quicly_sent_block_t *next; + uint64_t packet_number; /** - * number of entries in the block + * */ - size_t num_entries; + int64_t sent_at; /** - * insertion index within `entries` + * epoch to be acked in */ - size_t next_insert_at; + uint8_t ack_epoch; /** - * slots + * + */ + uint8_t ack_eliciting : 1; + /** + * number of bytes in-flight for the packet (becomes zero once deemed lost) + */ + uint16_t bytes_in_flight; + /** + * number of frame slots available + */ + uint16_t frame_capacity; + /** + * number of used frame slots */ - quicly_sent_t entries[16]; + uint16_t used_frames; + /** + * variable length array of frames composing the packet + */ + quicly_sent_frame_t frames[]; }; /** @@ -152,9 +154,10 @@ struct st_quicly_sent_block_t { */ typedef struct st_quicly_sentmap_t { /** + * "fake" packet, used for its linked list, and as a valid end value for the iterator * the linked list includes entries that are deemed lost (up to 3*SRTT) as well */ - struct st_quicly_sent_block_t *head, *tail; + quicly_sent_packet_t _; /** * bytes in-flight */ @@ -162,23 +165,20 @@ typedef struct st_quicly_sentmap_t { /** * is non-NULL between prepare and commit, pointing to the packet header that is being written to */ - quicly_sent_t *_pending_packet; + uint8_t is_open; + } quicly_sentmap_t; typedef struct st_quicly_sentmap_iter_t { - quicly_sent_t *p; - size_t count; - struct st_quicly_sent_block_t **ref; + dlist_t *p; } quicly_sentmap_iter_t; -extern const quicly_sent_t quicly_sentmap__end_iter; - /** * initializes the sentmap */ -static void quicly_sentmap_init(quicly_sentmap_t *map); +void quicly_sentmap_init(quicly_sentmap_t *map); /** - * + * frees all the data contained in the sentmap */ void quicly_sentmap_dispose(quicly_sentmap_t *map); @@ -193,12 +193,11 @@ int quicly_sentmap_prepare(quicly_sentmap_t *map, uint64_t packet_number, int64_ /** * commits a write */ -static void quicly_sentmap_commit(quicly_sentmap_t *map, uint16_t bytes_in_flight); +void quicly_sentmap_commit(quicly_sentmap_t *map, uint16_t bytes_in_flight); /** * Allocates a slot to contain a callback for a frame. The function MUST be called after _prepare but before _commit. */ -static quicly_sent_t *quicly_sentmap_allocate(quicly_sentmap_t *map, quicly_sent_acked_cb acked); - +quicly_sent_frame_t *quicly_sentmap_allocate_frame(quicly_sentmap_t *map, quicly_sent_acked_cb acked); /** * initializes the iterator */ @@ -210,77 +209,43 @@ static const quicly_sent_packet_t *quicly_sentmap_get(quicly_sentmap_iter_t *ite /** * advances the iterator to the next packet */ -void quicly_sentmap_skip(quicly_sentmap_iter_t *iter); +static void quicly_sentmap_skip(quicly_sentmap_iter_t *iter); +/** + * checks wether the iterator reached the end of the sentmap + */ +static int8_t quicly_sentmap_iter_is_end(quicly_sentmap_iter_t *iter); /** * updates the state of the packet being pointed to by the iterator, _and advances to the next packet_ */ int quicly_sentmap_update(quicly_sentmap_t *map, quicly_sentmap_iter_t *iter, quicly_sentmap_event_t event, struct st_quicly_conn_t *conn); -struct st_quicly_sent_block_t *quicly_sentmap__new_block(quicly_sentmap_t *map); -int quicly_sentmap__type_packet(struct st_quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, - quicly_sentmap_event_t event); - /* inline definitions */ -inline void quicly_sentmap_init(quicly_sentmap_t *map) +static inline const quicly_sent_packet_t *quicly_sentmap_get(quicly_sentmap_iter_t *iter) { - *map = (quicly_sentmap_t){NULL}; + assert(iter->p); + return (quicly_sent_packet_t *)iter->p; } -inline int quicly_sentmap_is_open(quicly_sentmap_t *map) +static inline int8_t quicly_sentmap_iter_is_end(quicly_sentmap_iter_t *iter) { - return map->_pending_packet != NULL; + return quicly_sentmap_get(iter)->packet_number == UINT64_MAX; } -inline void quicly_sentmap_commit(quicly_sentmap_t *map, uint16_t bytes_in_flight) +static inline int quicly_sentmap_is_open(quicly_sentmap_t *map) { - assert(quicly_sentmap_is_open(map)); - - if (bytes_in_flight != 0) { - map->_pending_packet->data.packet.ack_eliciting = 1; - map->_pending_packet->data.packet.bytes_in_flight = bytes_in_flight; - map->bytes_in_flight += bytes_in_flight; - } - map->_pending_packet = NULL; -} - -inline quicly_sent_t *quicly_sentmap_allocate(quicly_sentmap_t *map, quicly_sent_acked_cb acked) -{ - struct st_quicly_sent_block_t *block; - - if ((block = map->tail) == NULL || block->next_insert_at == sizeof(block->entries) / sizeof(block->entries[0])) { - if ((block = quicly_sentmap__new_block(map)) == NULL) - return NULL; - } - - quicly_sent_t *sent = block->entries + block->next_insert_at++; - ++block->num_entries; - - sent->acked = acked; - - return sent; + return map->is_open; } -inline void quicly_sentmap_init_iter(quicly_sentmap_t *map, quicly_sentmap_iter_t *iter) +static inline void quicly_sentmap_init_iter(quicly_sentmap_t *map, quicly_sentmap_iter_t *iter) { - iter->ref = &map->head; - if (map->head != NULL) { - assert(map->head->num_entries != 0); - for (iter->p = map->head->entries; iter->p->acked == NULL; ++iter->p) - ; - assert(iter->p->acked == quicly_sentmap__type_packet); - iter->count = map->head->num_entries; - } else { - iter->p = (quicly_sent_t *)&quicly_sentmap__end_iter; - iter->count = 0; - } + iter->p = map->_.pkt_list.next; } -inline const quicly_sent_packet_t *quicly_sentmap_get(quicly_sentmap_iter_t *iter) +static inline void quicly_sentmap_skip(quicly_sentmap_iter_t *iter) { - assert(iter->p->acked == quicly_sentmap__type_packet); - return &iter->p->data.packet; + iter->p = iter->p->next; } #ifdef __cplusplus diff --git a/lib/quicly.c b/lib/quicly.c index 6e5fe06d4..480538dbf 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -628,7 +628,7 @@ static void assert_consistency(quicly_conn_t *conn, int timer_must_be_in_future) assert(now < conn->egress.loss.alarm_at); } -static int on_invalid_ack(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_invalid_ack(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { if (event == QUICLY_SENTMAP_EVENT_ACKED) @@ -2104,7 +2104,8 @@ static int decrypt_packet(ptls_cipher_context_t *header_protection, return 0; } -static int on_ack_ack(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, quicly_sentmap_event_t event) +static int on_ack_ack(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, + quicly_sentmap_event_t event) { /* TODO log */ @@ -2141,7 +2142,8 @@ static int on_ack_ack(quicly_conn_t *conn, const quicly_sent_packet_t *packet, q return 0; } -static int on_ack_stream(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, quicly_sentmap_event_t event) +static int on_ack_stream(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, + quicly_sentmap_event_t event) { quicly_stream_t *stream; int ret; @@ -2184,7 +2186,7 @@ static int on_ack_stream(quicly_conn_t *conn, const quicly_sent_packet_t *packet return 0; } -static int on_ack_max_stream_data(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_ack_max_stream_data(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { quicly_stream_t *stream; @@ -2211,7 +2213,7 @@ static int on_ack_max_stream_data(quicly_conn_t *conn, const quicly_sent_packet_ return 0; } -static int on_ack_max_data(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_ack_max_data(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { switch (event) { @@ -2228,7 +2230,7 @@ static int on_ack_max_data(quicly_conn_t *conn, const quicly_sent_packet_t *pack return 0; } -static int on_ack_max_streams(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_ack_max_streams(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { quicly_maxsender_t *maxsender = sent->data.max_streams.uni ? conn->ingress.max_streams.uni : conn->ingress.max_streams.bidi; @@ -2253,7 +2255,7 @@ static void on_ack_stream_state_sender(quicly_sender_state_t *sender_state, int *sender_state = acked ? QUICLY_SENDER_STATE_ACKED : QUICLY_SENDER_STATE_SEND; } -static int on_ack_reset_stream(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_ack_reset_stream(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { if (event != QUICLY_SENTMAP_EVENT_EXPIRED) { @@ -2268,7 +2270,7 @@ static int on_ack_reset_stream(quicly_conn_t *conn, const quicly_sent_packet_t * return 0; } -static int on_ack_stop_sending(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_ack_stop_sending(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { if (event != QUICLY_SENTMAP_EVENT_EXPIRED) { @@ -2283,7 +2285,7 @@ static int on_ack_stop_sending(quicly_conn_t *conn, const quicly_sent_packet_t * return 0; } -static int on_ack_streams_blocked(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_ack_streams_blocked(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { struct st_quicly_max_streams_t *m = @@ -2303,7 +2305,7 @@ static int on_ack_streams_blocked(quicly_conn_t *conn, const quicly_sent_packet_ return 0; } -static int on_ack_handshake_done(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_ack_handshake_done(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { /* When HANDSHAKE_DONE is deemed lost, schedule retransmission. */ @@ -2312,7 +2314,7 @@ static int on_ack_handshake_done(quicly_conn_t *conn, const quicly_sent_packet_t return 0; } -static int on_ack_new_token(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_ack_new_token(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { if (sent->data.new_token.is_inflight) { @@ -2527,7 +2529,7 @@ static int commit_send_packet(quicly_conn_t *conn, quicly_send_context_t *s, int int ret; if ((ret = quicly_sentmap_prepare(&conn->egress.sentmap, conn->egress.packet_number, now, QUICLY_EPOCH_1RTT)) != 0) return ret; - if (quicly_sentmap_allocate(&conn->egress.sentmap, on_invalid_ack) == NULL) + if (quicly_sentmap_allocate_frame(&conn->egress.sentmap, on_invalid_ack) == NULL) return PTLS_ERROR_NO_MEMORY; quicly_sentmap_commit(&conn->egress.sentmap, 0); ++conn->egress.packet_number; @@ -2674,14 +2676,14 @@ static int allocate_frame(quicly_conn_t *conn, quicly_send_context_t *s, size_t return _do_allocate_frame(conn, s, min_space, 0); } -static int allocate_ack_eliciting_frame(quicly_conn_t *conn, quicly_send_context_t *s, size_t min_space, quicly_sent_t **sent, +static int allocate_ack_eliciting_frame(quicly_conn_t *conn, quicly_send_context_t *s, size_t min_space, quicly_sent_frame_t **sent, quicly_sent_acked_cb acked) { int ret; if ((ret = _do_allocate_frame(conn, s, min_space, 1)) != 0) return ret; - if ((*sent = quicly_sentmap_allocate(&conn->egress.sentmap, acked)) == NULL) + if ((*sent = quicly_sentmap_allocate_frame(&conn->egress.sentmap, acked)) == NULL) return PTLS_ERROR_NO_MEMORY; /* TODO return the remaining window that the sender can use */ @@ -2734,8 +2736,8 @@ static int send_ack(quicly_conn_t *conn, struct st_quicly_pn_space_t *space, qui { /* save what's inflight */ size_t i; for (i = 0; i != space->ack_queue.num_ranges; ++i) { - quicly_sent_t *sent; - if ((sent = quicly_sentmap_allocate(&conn->egress.sentmap, on_ack_ack)) == NULL) + quicly_sent_frame_t *sent; + if ((sent = quicly_sentmap_allocate_frame(&conn->egress.sentmap, on_ack_ack)) == NULL) return PTLS_ERROR_NO_MEMORY; sent->data.ack.range = space->ack_queue.ranges[i]; } @@ -2749,7 +2751,7 @@ static int send_ack(quicly_conn_t *conn, struct st_quicly_pn_space_t *space, qui static int prepare_stream_state_sender(quicly_stream_t *stream, quicly_sender_state_t *sender, quicly_send_context_t *s, size_t min_space, quicly_sent_acked_cb ack_cb) { - quicly_sent_t *sent; + quicly_sent_frame_t *sent; int ret; if ((ret = allocate_ack_eliciting_frame(stream->conn, s, min_space, &sent, ack_cb)) != 0) @@ -2776,7 +2778,7 @@ static int send_stream_control_frames(quicly_stream_t *stream, quicly_send_conte /* send MAX_STREAM_DATA if necessary */ if (should_send_max_stream_data(stream)) { uint64_t new_value = stream->recvstate.data_off + stream->_recv_aux.window; - quicly_sent_t *sent; + quicly_sent_frame_t *sent; /* prepare */ if ((ret = allocate_ack_eliciting_frame(stream->conn, s, QUICLY_MAX_STREAM_DATA_FRAME_CAPACITY, &sent, on_ack_max_stream_data)) != 0) @@ -2814,7 +2816,7 @@ int quicly_can_send_stream_data(quicly_conn_t *conn, quicly_send_context_t *s) int quicly_send_stream(quicly_stream_t *stream, quicly_send_context_t *s) { uint64_t off = stream->sendstate.pending.ranges[0].start, end_off; - quicly_sent_t *sent; + quicly_sent_frame_t *sent; uint8_t *frame_type_at; size_t capacity, len; int ret, wrote_all, is_fin; @@ -3080,7 +3082,7 @@ static int send_max_streams(quicly_conn_t *conn, int uni, quicly_send_context_t (uni ? conn->super.ctx->transport_params.max_streams_uni : conn->super.ctx->transport_params.max_streams_bidi) - group->num_streams; - quicly_sent_t *sent; + quicly_sent_frame_t *sent; if ((ret = allocate_ack_eliciting_frame(conn, s, QUICLY_MAX_STREAMS_FRAME_CAPACITY, &sent, on_ack_max_streams)) != 0) return ret; s->dst = quicly_encode_max_streams_frame(s->dst, uni, new_count); @@ -3108,7 +3110,7 @@ static int send_streams_blocked(quicly_conn_t *conn, int uni, quicly_send_contex if (!quicly_maxsender_should_send_blocked(&max_streams->blocked_sender, max_streams->count)) return 0; - quicly_sent_t *sent; + quicly_sent_frame_t *sent; if ((ret = allocate_ack_eliciting_frame(conn, s, QUICLY_STREAMS_BLOCKED_FRAME_CAPACITY, &sent, on_ack_streams_blocked)) != 0) return ret; s->dst = quicly_encode_streams_blocked_frame(s->dst, uni, max_streams->count); @@ -3151,7 +3153,7 @@ static void open_blocked_streams(quicly_conn_t *conn, int uni) static int send_handshake_done(quicly_conn_t *conn, quicly_send_context_t *s) { - quicly_sent_t *sent; + quicly_sent_frame_t *sent; int ret; if ((ret = allocate_ack_eliciting_frame(conn, s, 1, &sent, on_ack_handshake_done)) != 0) @@ -3170,7 +3172,7 @@ static int send_resumption_token(quicly_conn_t *conn, quicly_send_context_t *s) quicly_address_token_plaintext_t token; ptls_buffer_t tokenbuf; uint8_t tokenbuf_small[128]; - quicly_sent_t *sent; + quicly_sent_frame_t *sent; int ret; ptls_buffer_init(&tokenbuf, tokenbuf_small, sizeof(tokenbuf_small)); @@ -3602,7 +3604,7 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) goto Exit; /* send connection-level flow control frame */ if (should_send_max_data(conn)) { - quicly_sent_t *sent; + quicly_sent_frame_t *sent; if ((ret = allocate_ack_eliciting_frame(conn, s, QUICLY_MAX_DATA_FRAME_CAPACITY, &sent, on_ack_max_data)) != 0) goto Exit; uint64_t new_value = conn->ingress.max_data.bytes_consumed + conn->super.ctx->transport_params.max_data; @@ -3805,7 +3807,7 @@ int quicly_send_resumption_token(quicly_conn_t *conn) return 0; } -static int on_end_closing(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_end_closing(quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { /* we stop accepting frames by the time this ack callback is being registered */ @@ -3824,7 +3826,7 @@ static int enter_close(quicly_conn_t *conn, int host_is_initiating, int wait_dra return ret; if ((ret = quicly_sentmap_prepare(&conn->egress.sentmap, conn->egress.packet_number, now, QUICLY_EPOCH_INITIAL)) != 0) return ret; - if (quicly_sentmap_allocate(&conn->egress.sentmap, on_end_closing) == NULL) + if (quicly_sentmap_allocate_frame(&conn->egress.sentmap, on_end_closing) == NULL) return PTLS_ERROR_NO_MEMORY; quicly_sentmap_commit(&conn->egress.sentmap, 0); ++conn->egress.packet_number; diff --git a/lib/sentmap.c b/lib/sentmap.c index 1fd106805..8bdf2e920 100644 --- a/lib/sentmap.c +++ b/lib/sentmap.c @@ -21,156 +21,140 @@ */ #include #include +#include #include "picotls.h" #include "quicly/sentmap.h" -const quicly_sent_t quicly_sentmap__end_iter = {quicly_sentmap__type_packet, {{UINT64_MAX, INT64_MAX}}}; - -static void next_entry(quicly_sentmap_iter_t *iter) +void quicly_sentmap_init(quicly_sentmap_t *map) { - if (--iter->count != 0) { - ++iter->p; - } else if (*(iter->ref = &(*iter->ref)->next) == NULL) { - iter->p = (quicly_sent_t *)&quicly_sentmap__end_iter; - iter->count = 0; - return; - } else { - assert((*iter->ref)->num_entries != 0); - iter->count = (*iter->ref)->num_entries; - iter->p = (*iter->ref)->entries; - } - while (iter->p->acked == NULL) - ++iter->p; + *map = (quicly_sentmap_t){NULL}; + map->_.pkt_list.next = &(map->_.pkt_list); + map->_.pkt_list.prev = &(map->_.pkt_list); + map->_.packet_number = UINT64_MAX; + map->_.sent_at = INT64_MAX; } -static struct st_quicly_sent_block_t **free_block(quicly_sentmap_t *map, struct st_quicly_sent_block_t **ref) +void quicly_sentmap_commit(quicly_sentmap_t *map, uint16_t bytes_in_flight) { - static const struct st_quicly_sent_block_t dummy = {NULL}; - static const struct st_quicly_sent_block_t *const dummy_ref = &dummy; - struct st_quicly_sent_block_t *block = *ref; - - if (block->next != NULL) { - *ref = block->next; - assert((*ref)->num_entries != 0); - } else { - assert(block == map->tail); - if (ref == &map->head) { - map->head = NULL; - map->tail = NULL; - } else { - map->tail = (void *)((char *)ref - offsetof(struct st_quicly_sent_block_t, next)); - map->tail->next = NULL; - } - ref = (struct st_quicly_sent_block_t **)&dummy_ref; - } + assert(quicly_sentmap_is_open(map)); - free(block); - return ref; + if (bytes_in_flight != 0) { + quicly_sent_packet_t *p = (quicly_sent_packet_t *)map->_.pkt_list.prev; + p->ack_eliciting = 1; + p->bytes_in_flight = bytes_in_flight; + map->bytes_in_flight += bytes_in_flight; + } + map->is_open = 0; } -static void discard_entry(quicly_sentmap_t *map, quicly_sentmap_iter_t *iter) +static inline quicly_sent_packet_t *allocate_packet(uint16_t frames) { - assert(iter->p->acked != NULL); - iter->p->acked = NULL; - - struct st_quicly_sent_block_t *block = *iter->ref; - if (--block->num_entries == 0) { - iter->ref = free_block(map, iter->ref); - block = *iter->ref; - iter->p = block->entries - 1; - iter->count = block->num_entries + 1; - } + quicly_sent_packet_t *packet = calloc(1, offsetof(quicly_sent_packet_t, frames) + frames * sizeof(quicly_sent_frame_t)); + if (packet == NULL) + return NULL; + packet->frame_capacity = frames; + return packet; } -void quicly_sentmap_dispose(quicly_sentmap_t *map) +quicly_sent_frame_t *quicly_sentmap_allocate_frame(quicly_sentmap_t *map, quicly_sent_acked_cb acked) { - struct st_quicly_sent_block_t *block; - - while ((block = map->head) != NULL) { - map->head = block->next; - free(block); + assert(quicly_sentmap_is_open(map)); + + quicly_sent_packet_t *p = (quicly_sent_packet_t *)map->_.pkt_list.prev; + /* grow the packet if it is full */ + if (p->used_frames == p->frame_capacity) { + quicly_sent_packet_t *new_p = allocate_packet(p->frame_capacity * 2); + if (!new_p) + return NULL; + memcpy(new_p, p, offsetof(quicly_sent_packet_t, frames) + p->frame_capacity * sizeof(quicly_sent_frame_t)); + new_p->frame_capacity = p->frame_capacity * 2; + p->pkt_list.prev->next = &(new_p->pkt_list); + p->pkt_list.next->prev = &(new_p->pkt_list); + p = new_p; } + + assert(p->used_frames < p->frame_capacity); + quicly_sent_frame_t *frame = &p->frames[p->used_frames]; + p->used_frames++; + frame->acked = acked; + return frame; } int quicly_sentmap_prepare(quicly_sentmap_t *map, uint64_t packet_number, int64_t now, uint8_t ack_epoch) { - assert(map->_pending_packet == NULL); + assert(!quicly_sentmap_is_open(map)); - if ((map->_pending_packet = quicly_sentmap_allocate(map, quicly_sentmap__type_packet)) == NULL) + quicly_sent_packet_t *new_packet = allocate_packet(QUICLY_SENTMAP_DEFAULT_FRAMES_PER_PACKET); + if (new_packet == NULL) { return PTLS_ERROR_NO_MEMORY; - map->_pending_packet->data.packet = (quicly_sent_packet_t){packet_number, now, ack_epoch}; + } + new_packet->packet_number = packet_number; + new_packet->sent_at = now; + new_packet->ack_epoch = ack_epoch; + + map->_.pkt_list.prev->next = &(new_packet->pkt_list); + new_packet->pkt_list.prev = map->_.pkt_list.prev; + map->_.pkt_list.prev = &(new_packet->pkt_list); + new_packet->pkt_list.next = &(map->_.pkt_list); + + map->is_open = 1; return 0; } -struct st_quicly_sent_block_t *quicly_sentmap__new_block(quicly_sentmap_t *map) +static inline void discard_packet(quicly_sentmap_t *map, quicly_sent_packet_t *packet) { - struct st_quicly_sent_block_t *block; + assert(packet); + assert(&(packet->pkt_list) != &(map->_.pkt_list)); /* not the end of the list */ - if ((block = malloc(sizeof(*block))) == NULL) - return NULL; + packet->pkt_list.prev->next = packet->pkt_list.next; + packet->pkt_list.next->prev = packet->pkt_list.prev; - block->next = NULL; - block->num_entries = 0; - block->next_insert_at = 0; - if (map->tail != NULL) { - map->tail->next = block; - map->tail = block; - } else { - map->head = map->tail = block; - } - - return block; -} - -void quicly_sentmap_skip(quicly_sentmap_iter_t *iter) -{ - do { - next_entry(iter); - } while (iter->p->acked != quicly_sentmap__type_packet); + free(packet); } int quicly_sentmap_update(quicly_sentmap_t *map, quicly_sentmap_iter_t *iter, quicly_sentmap_event_t event, struct st_quicly_conn_t *conn) { - quicly_sent_packet_t packet; - int notify_lost = 0, ret = 0; + quicly_sent_packet_t *packet; + int notify_lost = 0, ret = 0, i = 0; - assert(iter->p != &quicly_sentmap__end_iter); - assert(iter->p->acked == quicly_sentmap__type_packet); + assert(!quicly_sentmap_iter_is_end(iter)); - /* copy packet info */ - packet = iter->p->data.packet; + /* save packet pointer */ + packet = (quicly_sent_packet_t *)iter->p; /* update packet-level metrics (make adjustments to notify the loss when discarding a packet that is still deemed inflight) */ - if (packet.bytes_in_flight != 0) { + if (packet->bytes_in_flight != 0) { if (event == QUICLY_SENTMAP_EVENT_EXPIRED) notify_lost = 1; - assert(map->bytes_in_flight >= packet.bytes_in_flight); - map->bytes_in_flight -= packet.bytes_in_flight; + assert(map->bytes_in_flight >= packet->bytes_in_flight); + map->bytes_in_flight -= packet->bytes_in_flight; } - iter->p->data.packet.bytes_in_flight = 0; + packet->bytes_in_flight = 0; - /* Remove entry from sentmap, unless packet is deemed lost. If lost, then hold on to this packet until removed by a - * QUICLY_SENTMAP_EVENT_EXPIRED event. */ - if (event != QUICLY_SENTMAP_EVENT_LOST) - discard_entry(map, iter); + /* move iterator to next packet */ + quicly_sentmap_skip(iter); /* iterate through the frames */ - for (next_entry(iter); iter->p->acked != quicly_sentmap__type_packet; next_entry(iter)) { + for (i = 0; i < packet->used_frames; i++) { + quicly_sent_frame_t *frame = &packet->frames[i]; if (notify_lost && ret == 0) - ret = iter->p->acked(conn, &packet, iter->p, QUICLY_SENTMAP_EVENT_LOST); + ret = frame->acked(conn, packet, frame, QUICLY_SENTMAP_EVENT_LOST); if (ret == 0) - ret = iter->p->acked(conn, &packet, iter->p, event); - if (event != QUICLY_SENTMAP_EVENT_LOST) - discard_entry(map, iter); + ret = frame->acked(conn, packet, frame, event); } + /* Remove packet from sentmap, unless it is deemed lost. If lost, then hold on to this packet until removed by a + * QUICLY_SENTMAP_EVENT_EXPIRED event. */ + if (event != QUICLY_SENTMAP_EVENT_LOST) + discard_packet(map, packet); + return ret; } -int quicly_sentmap__type_packet(struct st_quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, - quicly_sentmap_event_t event) +void quicly_sentmap_dispose(quicly_sentmap_t *map) { - assert(!"quicly_sentmap__type_packet cannot be called"); - return QUICLY_TRANSPORT_ERROR_INTERNAL; + while (map->_.pkt_list.next != &(map->_.pkt_list)) { + discard_packet(map, (quicly_sent_packet_t *)map->_.pkt_list.next); + } } diff --git a/t/sentmap.c b/t/sentmap.c index f51a4516e..0f2ee0bec 100644 --- a/t/sentmap.c +++ b/t/sentmap.c @@ -24,7 +24,7 @@ static int on_acked_callcnt, on_acked_ackcnt; -static int on_acked(struct st_quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_t *sent, +static int on_acked(struct st_quicly_conn_t *conn, const quicly_sent_packet_t *packet, quicly_sent_frame_t *sent, quicly_sentmap_event_t event) { ++on_acked_callcnt; @@ -33,17 +33,6 @@ static int on_acked(struct st_quicly_conn_t *conn, const quicly_sent_packet_t *p return 0; } -static size_t num_blocks(quicly_sentmap_t *map) -{ - struct st_quicly_sent_block_t *block; - size_t n = 0; - - for (block = map->head; block != NULL; block = block->next) - ++n; - - return n; -} - void test_sentmap(void) { quicly_sentmap_t map; @@ -58,8 +47,8 @@ void test_sentmap(void) for (at = 0; at < 10; ++at) { for (i = 1; i <= 5; ++i) { quicly_sentmap_prepare(&map, at * 5 + i, at, 0); - quicly_sentmap_allocate(&map, on_acked); - quicly_sentmap_allocate(&map, on_acked); + quicly_sentmap_allocate_frame(&map, on_acked); + quicly_sentmap_allocate_frame(&map, on_acked); quicly_sentmap_commit(&map, 1); } } @@ -76,8 +65,7 @@ void test_sentmap(void) quicly_sentmap_skip(&iter); } } - ok(quicly_sentmap_get(&iter)->packet_number == UINT64_MAX); - ok(num_blocks(&map) == 150 / 16 + 1); + ok(quicly_sentmap_iter_is_end(&iter)); /* pop acks between 11 <= packet_number <= 40 */ quicly_sentmap_init_iter(&map, &iter); @@ -97,7 +85,18 @@ void test_sentmap(void) ++cnt; } ok(cnt == 20); - ok(num_blocks(&map) == 30 / 16 + 1 + 1 + 30 / 16 + 1); quicly_sentmap_dispose(&map); + + /* save one packet and delete it */ + quicly_sentmap_init(&map); + quicly_sentmap_prepare(&map, at * 5 + i, at, 0); + quicly_sentmap_allocate_frame(&map, on_acked); + quicly_sentmap_commit(&map, 1); + quicly_sentmap_init_iter(&map, &iter); + quicly_sentmap_update(&map, &iter, QUICLY_SENTMAP_EVENT_EXPIRED, NULL); + ok(quicly_sentmap_iter_is_end(&iter)); + quicly_sentmap_init_iter(&map, &iter); + ok(quicly_sentmap_iter_is_end(&iter)); + quicly_sentmap_dispose(&map); } From d89d28dbe20a655820bcb19ba85aa4b5a324a290 Mon Sep 17 00:00:00 2001 From: Aloys Augustin Date: Sun, 5 Apr 2020 14:56:38 +0200 Subject: [PATCH 3/3] Fix comment, remove unnecessary parentheses --- include/quicly/sentmap.h | 3 +-- lib/sentmap.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/include/quicly/sentmap.h b/include/quicly/sentmap.h index a7773f213..e60cf7504 100644 --- a/include/quicly/sentmap.h +++ b/include/quicly/sentmap.h @@ -163,7 +163,7 @@ typedef struct st_quicly_sentmap_t { */ size_t bytes_in_flight; /** - * is non-NULL between prepare and commit, pointing to the packet header that is being written to + * whether a new packet has been allocated and frames can be added to it */ uint8_t is_open; @@ -181,7 +181,6 @@ void quicly_sentmap_init(quicly_sentmap_t *map); * frees all the data contained in the sentmap */ void quicly_sentmap_dispose(quicly_sentmap_t *map); - /** * if transaction is open (i.e. between prepare and commit) */ diff --git a/lib/sentmap.c b/lib/sentmap.c index 8bdf2e920..7efc993f7 100644 --- a/lib/sentmap.c +++ b/lib/sentmap.c @@ -28,8 +28,8 @@ void quicly_sentmap_init(quicly_sentmap_t *map) { *map = (quicly_sentmap_t){NULL}; - map->_.pkt_list.next = &(map->_.pkt_list); - map->_.pkt_list.prev = &(map->_.pkt_list); + map->_.pkt_list.next = &map->_.pkt_list; + map->_.pkt_list.prev = &map->_.pkt_list; map->_.packet_number = UINT64_MAX; map->_.sent_at = INT64_MAX; } @@ -68,8 +68,8 @@ quicly_sent_frame_t *quicly_sentmap_allocate_frame(quicly_sentmap_t *map, quicly return NULL; memcpy(new_p, p, offsetof(quicly_sent_packet_t, frames) + p->frame_capacity * sizeof(quicly_sent_frame_t)); new_p->frame_capacity = p->frame_capacity * 2; - p->pkt_list.prev->next = &(new_p->pkt_list); - p->pkt_list.next->prev = &(new_p->pkt_list); + p->pkt_list.prev->next = &new_p->pkt_list; + p->pkt_list.next->prev = &new_p->pkt_list; p = new_p; } @@ -92,10 +92,10 @@ int quicly_sentmap_prepare(quicly_sentmap_t *map, uint64_t packet_number, int64_ new_packet->sent_at = now; new_packet->ack_epoch = ack_epoch; - map->_.pkt_list.prev->next = &(new_packet->pkt_list); + map->_.pkt_list.prev->next = &new_packet->pkt_list; new_packet->pkt_list.prev = map->_.pkt_list.prev; - map->_.pkt_list.prev = &(new_packet->pkt_list); - new_packet->pkt_list.next = &(map->_.pkt_list); + map->_.pkt_list.prev = &new_packet->pkt_list; + new_packet->pkt_list.next = &map->_.pkt_list; map->is_open = 1; return 0; @@ -104,7 +104,7 @@ int quicly_sentmap_prepare(quicly_sentmap_t *map, uint64_t packet_number, int64_ static inline void discard_packet(quicly_sentmap_t *map, quicly_sent_packet_t *packet) { assert(packet); - assert(&(packet->pkt_list) != &(map->_.pkt_list)); /* not the end of the list */ + assert(&packet->pkt_list != &map->_.pkt_list); /* not the end of the list */ packet->pkt_list.prev->next = packet->pkt_list.next; packet->pkt_list.next->prev = packet->pkt_list.prev; @@ -154,7 +154,7 @@ int quicly_sentmap_update(quicly_sentmap_t *map, quicly_sentmap_iter_t *iter, qu void quicly_sentmap_dispose(quicly_sentmap_t *map) { - while (map->_.pkt_list.next != &(map->_.pkt_list)) { + while (map->_.pkt_list.next != &map->_.pkt_list) { discard_packet(map, (quicly_sent_packet_t *)map->_.pkt_list.next); } }