From 16a9b66aaa2fdb12f3c57d1cf801937a179e73d7 Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Fri, 9 Mar 2018 14:17:39 -0500 Subject: [PATCH] Latest changes - [OPTIMIZATION] Merge series of ACKs if possible Parsed single-range ACK frames (that is the majority of frames) are saved in the connection and their processing is deferred until the connection is ticked. If several ACKs come in a series between adjacent ticks, we check whether the latest ACK is a strict superset of the saved ACK. If it is, the older ACK is not processed. If ACK frames can be merged, they are merged and only one of them is either processed or saved. - [OPTIMIZATION] Speed up ACK verification by simplifying send history. Never generate a gap in the sent packet number sequence. This reduces the send history to a single number instead of potentially a series of packet ranges and thereby speeds up ACK verification. By default, detecting a gap in the send history is not fatal: only a single warning is generated per connection. The connection can continue to operate even if the ACK verification code is not able to detect some inconsistencies. - [OPTIMIZATION] Rearrange the lsquic_send_ctl struct The first part of struct lsquic_send_ctl now consists of members that are used in lsquic_send_ctl_got_ack() (in the absense of packet loss, which is the normal case). To speed up reads and writes, we no longer try to save space by using 8- and 16-bit integers. Use regular integer width for everything. - [OPTIMIZATION] Cache size of sent packet. - [OPTIMIZATION] Keep track of the largest ACKed in packet_out Instead of parsing our own ACK frames when packet has been acked, use the value saved in the packet_out structure when the ACK frame was generated. - [OPTIMIZATION] Take RTT sampling conditional out of ACK loop - [OPTIMIZATION] ACK processing: only call clock_gettime() if needed - [OPTIMIZATION] Several code-level optimizations to ACK processing. - Fix: http_client: fix -I flag; switch assert() to abort() --- CHANGELOG | 50 +++- CMakeLists.txt | 4 + src/liblsquic/lsquic_engine.c | 7 +- src/liblsquic/lsquic_full_conn.c | 242 ++++++++++++++++-- src/liblsquic/lsquic_packet_out.h | 22 ++ src/liblsquic/lsquic_parse.h | 11 +- src/liblsquic/lsquic_parse_gquic_Q041.c | 27 +- src/liblsquic/lsquic_parse_gquic_be.c | 25 +- src/liblsquic/lsquic_parse_gquic_be.h | 2 +- src/liblsquic/lsquic_parse_gquic_le.c | 23 +- src/liblsquic/lsquic_send_ctl.c | 297 ++++++++++++---------- src/liblsquic/lsquic_send_ctl.h | 62 +++-- src/liblsquic/lsquic_senhist.c | 116 +-------- src/liblsquic/lsquic_senhist.h | 66 +++-- test/http_client.c | 9 +- test/unittests/test_ackgen_gquic_be.c | 38 +-- test/unittests/test_ackgen_gquic_ietf.c | 38 +-- test/unittests/test_ackgen_gquic_le.c | 38 +-- test/unittests/test_ackparse_gquic_be.c | 14 +- test/unittests/test_ackparse_gquic_ietf.c | 14 +- test/unittests/test_ackparse_gquic_le.c | 14 +- test/unittests/test_senhist.c | 35 +-- 22 files changed, 647 insertions(+), 507 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 76ded86..6bb1965 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,50 @@ -2018-02-28 - - Fix unit test regression: enable them correctly in cmake. - - Simplify connection has interface +2018-03-09 + + - [OPTIMIZATION] Merge series of ACKs if possible + + Parsed single-range ACK frames (that is the majority of frames) are + saved in the connection and their processing is deferred until the + connection is ticked. If several ACKs come in a series between + adjacent ticks, we check whether the latest ACK is a strict superset + of the saved ACK. If it is, the older ACK is not processed. + + If ACK frames can be merged, they are merged and only one of them is + either processed or saved. + + - [OPTIMIZATION] Speed up ACK verification by simplifying send history. + + Never generate a gap in the sent packet number sequence. This reduces + the send history to a single number instead of potentially a series of + packet ranges and thereby speeds up ACK verification. + + By default, detecting a gap in the send history is not fatal: only a + single warning is generated per connection. The connection can continue + to operate even if the ACK verification code is not able to detect some + inconsistencies. + + - [OPTIMIZATION] Rearrange the lsquic_send_ctl struct + + The first part of struct lsquic_send_ctl now consists of members that + are used in lsquic_send_ctl_got_ack() (in the absense of packet loss, + which is the normal case). To speed up reads and writes, we no longer + try to save space by using 8- and 16-bit integers. Use regular integer + width for everything. + + - [OPTIMIZATION] Cache size of sent packet. + + - [OPTIMIZATION] Keep track of the largest ACKed in packet_out + + Instead of parsing our own ACK frames when packet has been acked, + use the value saved in the packet_out structure when the ACK frame + was generated. + + - [OPTIMIZATION] Take RTT sampling conditional out of ACK loop + + - [OPTIMIZATION] ACK processing: only call clock_gettime() if needed + + - [OPTIMIZATION] Several code-level optimizations to ACK processing. + + - Fix: http_client: fix -I flag; switch assert() to abort() 2018-02-26 - [API Change] lsquic_engine_connect() returns pointer to the connection diff --git a/CMakeLists.txt b/CMakeLists.txt index 10f0f6d..d08ac6a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,6 +52,10 @@ IF(LSQUIC_PROFILE EQUAL 1) SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -g -pg") ENDIF() +IF(LSQUIC_COVERAGE EQUAL 1) + SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -fprofile-arcs -ftest-coverage") +ENDIF() + IF(MY_CMAKE_FLAGS MATCHES "fsanitize=address") MESSAGE(STATUS "AddressSanitizer is ON") ELSE() diff --git a/src/liblsquic/lsquic_engine.c b/src/liblsquic/lsquic_engine.c index eb3e837..8cff5c3 100644 --- a/src/liblsquic/lsquic_engine.c +++ b/src/liblsquic/lsquic_engine.c @@ -1118,6 +1118,7 @@ encrypt_packet (lsquic_engine_t *engine, const lsquic_conn_t *conn, { ssize_t enc_sz; size_t bufsz; + unsigned sent_sz; unsigned char *buf; bufsz = lsquic_po_header_length(packet_out->po_flags) + @@ -1130,7 +1131,10 @@ encrypt_packet (lsquic_engine_t *engine, const lsquic_conn_t *conn, return ENCPA_NOMEM; } + { enc_sz = really_encrypt_packet(conn, packet_out, buf, bufsz); + sent_sz = enc_sz; + } if (enc_sz < 0) { @@ -1140,7 +1144,8 @@ encrypt_packet (lsquic_engine_t *engine, const lsquic_conn_t *conn, packet_out->po_enc_data = buf; packet_out->po_enc_data_sz = enc_sz; - packet_out->po_flags |= PO_ENCRYPTED; + packet_out->po_sent_sz = sent_sz; + packet_out->po_flags |= PO_ENCRYPTED|PO_SENT_SZ; return ENCPA_OK; } diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c index 0cfa13c..8e43bf8 100644 --- a/src/liblsquic/lsquic_full_conn.c +++ b/src/liblsquic/lsquic_full_conn.c @@ -92,6 +92,7 @@ enum full_conn_flags { FC_FIRST_TICK = (1 <<19), FC_TICK_CLOSE = (1 <<20), /* We returned TICK_CLOSE */ FC_HSK_FAILED = (1 <<21), + FC_HAVE_SAVED_ACK = (1 <<22), }; #define FC_IMMEDIATE_CLOSE_FLAGS \ @@ -188,6 +189,10 @@ struct full_conn n_dup_packets, n_err_packets; unsigned long stream_data_sz; + unsigned long n_ticks; + unsigned n_acks_in, + n_acks_proc, + n_acks_merged[2]; } fc_stats; #endif #if KEEP_CLOSED_STREAM_HISTORY @@ -203,6 +208,8 @@ struct full_conn #endif STAILQ_HEAD(, stream_id_to_reset) fc_stream_ids_to_reset; + struct short_ack_info fc_saved_ack_info; + lsquic_time_t fc_saved_ack_received; }; @@ -757,6 +764,7 @@ full_conn_ci_destroy (lsquic_conn_t *lconn) conn->fc_conn.cn_esf->esf_destroy(conn->fc_conn.cn_enc_session); lsquic_malo_destroy(conn->fc_pub.packet_out_malo); #if FULL_CONN_STATS + LSQ_NOTICE("# ticks: %lu", conn->fc_stats.n_ticks); LSQ_NOTICE("received %u packets, of which %u were not decryptable, %u were " "dups and %u were errors; sent %u packets, avg stream data per outgoing" " packet is %lu bytes", @@ -764,6 +772,9 @@ full_conn_ci_destroy (lsquic_conn_t *lconn) conn->fc_stats.n_dup_packets, conn->fc_stats.n_err_packets, conn->fc_stats.n_packets_out, conn->fc_stats.stream_data_sz / conn->fc_stats.n_packets_out); + LSQ_NOTICE("ACKs: in: %u; processed: %u; merged to: new %u, old %u", + conn->fc_stats.n_acks_in, conn->fc_stats.n_acks_proc, + conn->fc_stats.n_acks_merged[0], conn->fc_stats.n_acks_merged[1]); #endif while ((sitr = STAILQ_FIRST(&conn->fc_stream_ids_to_reset))) { @@ -1259,34 +1270,214 @@ log_invalid_ack_frame (struct full_conn *conn, const unsigned char *p, } +static int +process_ack (struct full_conn *conn, struct ack_info *acki, + lsquic_time_t received) +{ +#if FULL_CONN_STATS + ++conn->fc_stats.n_acks_proc; +#endif + LSQ_DEBUG("Processing ACK"); + if (0 == lsquic_send_ctl_got_ack(&conn->fc_send_ctl, acki, received)) + { + if (lsquic_send_ctl_largest_ack2ed(&conn->fc_send_ctl)) + lsquic_rechist_stop_wait(&conn->fc_rechist, + lsquic_send_ctl_largest_ack2ed(&conn->fc_send_ctl) + 1); + return 0; + } + else + { + ABORT_ERROR("Received invalid ACK"); + return -1; + } +} + + +static int +process_saved_ack (struct full_conn *conn, int restore_parsed_ack) +{ + struct ack_info *const acki = conn->fc_pub.mm->acki; + struct lsquic_packno_range range; + unsigned n_ranges, n_timestamps; + lsquic_time_t lack_delta; + int retval; + + if (restore_parsed_ack) + { + n_ranges = acki->n_ranges; + n_timestamps = acki->n_timestamps; + lack_delta = acki->lack_delta; + range = acki->ranges[0]; + } + + acki->n_ranges = 1; + acki->n_timestamps = conn->fc_saved_ack_info.sai_n_timestamps; + acki->lack_delta = conn->fc_saved_ack_info.sai_lack_delta; + acki->ranges[0] = conn->fc_saved_ack_info.sai_range; + + retval = process_ack(conn, acki, conn->fc_saved_ack_received); + + if (restore_parsed_ack) + { + acki->n_ranges = n_ranges; + acki->n_timestamps = n_timestamps; + acki->lack_delta = lack_delta; + acki->ranges[0] = range; + } + + return retval; +} + + +static int +new_ack_is_superset (const struct short_ack_info *old, const struct ack_info *new) +{ + const struct lsquic_packno_range *new_range; + + new_range = &new->ranges[ new->n_ranges - 1 ]; + return new_range->low <= old->sai_range.low + && new_range->high >= old->sai_range.high; +} + + +static int +merge_saved_to_new (const struct short_ack_info *old, struct ack_info *new) +{ + struct lsquic_packno_range *smallest_range; + + assert(new->n_ranges > 1); + smallest_range = &new->ranges[ new->n_ranges - 1 ]; + if (old->sai_range.high <= smallest_range->high + && old->sai_range.high >= smallest_range->low + && old->sai_range.low < smallest_range->low) + { + smallest_range->low = old->sai_range.low; + return 1; + } + else + return 0; +} + + +static int +merge_new_to_saved (struct short_ack_info *old, const struct ack_info *new) +{ + const struct lsquic_packno_range *new_range; + + assert(new->n_ranges == 1); + new_range = &new->ranges[0]; + /* Only merge if new is higher, for simplicity. This is also the + * expected case. + */ + if (new_range->high > old->sai_range.high + && new_range->low > old->sai_range.low) + { + old->sai_range.high = new_range->high; + return 1; + } + else + return 0; +} + + static unsigned process_ack_frame (struct full_conn *conn, lsquic_packet_in_t *packet_in, const unsigned char *p, size_t len) { - const int parsed_len = conn->fc_conn.cn_pf->pf_parse_ack_frame(p, len, - conn->fc_pub.mm->acki); + struct ack_info *const new_acki = conn->fc_pub.mm->acki; + int parsed_len; + +#if FULL_CONN_STATS + ++conn->fc_stats.n_acks_in; +#endif + + parsed_len = conn->fc_conn.cn_pf->pf_parse_ack_frame(p, len, new_acki); if (parsed_len < 0) - return 0; - if (packet_in->pi_packno > conn->fc_max_ack_packno) + goto err; + + if (packet_in->pi_packno <= conn->fc_max_ack_packno) { - EV_LOG_ACK_FRAME_IN(LSQUIC_LOG_CONN_ID, conn->fc_pub.mm->acki); - if (0 == lsquic_send_ctl_got_ack(&conn->fc_send_ctl, - conn->fc_pub.mm->acki, packet_in->pi_received)) - { - conn->fc_max_ack_packno = packet_in->pi_packno; - if (lsquic_send_ctl_largest_ack2ed(&conn->fc_send_ctl)) - lsquic_rechist_stop_wait(&conn->fc_rechist, - lsquic_send_ctl_largest_ack2ed(&conn->fc_send_ctl) + 1); - } - else - { - log_invalid_ack_frame(conn, p, parsed_len, conn->fc_pub.mm->acki); - ABORT_ERROR("Received invalid ACK"); + LSQ_DEBUG("Ignore old ack (max %"PRIu64")", conn->fc_max_ack_packno); + return parsed_len; + } + + EV_LOG_ACK_FRAME_IN(LSQUIC_LOG_CONN_ID, new_acki); + conn->fc_max_ack_packno = packet_in->pi_packno; + + if (conn->fc_flags & FC_HAVE_SAVED_ACK) + { + LSQ_DEBUG("old ack [%"PRIu64"-%"PRIu64"]", + conn->fc_saved_ack_info.sai_range.high, + conn->fc_saved_ack_info.sai_range.low); + const int is_superset = new_ack_is_superset(&conn->fc_saved_ack_info, + new_acki); + const int is_1range = new_acki->n_ranges == 1; + switch ( + (is_superset << 1) + | (is_1range << 0)) + /* | | + | | + V V */ { + case (0 << 1) | (0 << 0): + if (merge_saved_to_new(&conn->fc_saved_ack_info, new_acki)) + { +#if FULL_CONN_STATS + ++conn->fc_stats.n_acks_merged[0] +#endif + ; + } + else + process_saved_ack(conn, 1); + conn->fc_flags &= ~FC_HAVE_SAVED_ACK; + if (0 != process_ack(conn, new_acki, packet_in->pi_received)) + goto err; + break; + case (0 << 1) | (1 << 0): + if (merge_new_to_saved(&conn->fc_saved_ack_info, new_acki)) + { +#if FULL_CONN_STATS + ++conn->fc_stats.n_acks_merged[1] +#endif + ; + } + else + { + process_saved_ack(conn, 1); + conn->fc_saved_ack_info.sai_n_timestamps = new_acki->n_timestamps; + conn->fc_saved_ack_info.sai_range = new_acki->ranges[0]; + } + conn->fc_saved_ack_info.sai_lack_delta = new_acki->lack_delta; + conn->fc_saved_ack_received = packet_in->pi_received; + break; + case (1 << 1) | (0 << 0): + conn->fc_flags &= ~FC_HAVE_SAVED_ACK; + if (0 != process_ack(conn, new_acki, packet_in->pi_received)) + goto err; + break; + case (1 << 1) | (1 << 0): + conn->fc_saved_ack_info.sai_n_timestamps = new_acki->n_timestamps; + conn->fc_saved_ack_info.sai_lack_delta = new_acki->lack_delta; + conn->fc_saved_ack_info.sai_range = new_acki->ranges[0]; + conn->fc_saved_ack_received = packet_in->pi_received; + break; } } - else - LSQ_DEBUG("Ignore old ack (max %"PRIu64")", conn->fc_max_ack_packno); + else if (new_acki->n_ranges == 1) + { + conn->fc_saved_ack_info.sai_n_timestamps = new_acki->n_timestamps; + conn->fc_saved_ack_info.sai_lack_delta = new_acki->lack_delta; + conn->fc_saved_ack_info.sai_range = new_acki->ranges[0]; + conn->fc_saved_ack_received = packet_in->pi_received; + conn->fc_flags |= FC_HAVE_SAVED_ACK; + } + else if (0 != process_ack(conn, new_acki, packet_in->pi_received)) + goto err; + return parsed_len; + + err: + log_invalid_ack_frame(conn, p, parsed_len, new_acki); + return 0; } @@ -2315,7 +2506,7 @@ generate_ack_frame (struct full_conn *conn) (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &conn->fc_rechist, now, &has_missing); + &conn->fc_rechist, now, &has_missing, &packet_out->po_ack2ed); if (w < 0) { ABORT_ERROR("generating ACK frame failed: %d", errno); return; @@ -2470,6 +2661,10 @@ full_conn_ci_tick (lsquic_conn_t *lconn, lsquic_time_t now) } \ } while (0) +#if FULL_CONN_STATS + ++conn->fc_stats.n_ticks; +#endif + if (LSQ_LOG_ENABLED(LSQ_LOG_DEBUG) && conn->fc_mem_logged_last + 1000000 <= now) { @@ -2479,6 +2674,13 @@ full_conn_ci_tick (lsquic_conn_t *lconn, lsquic_time_t now) assert(!(conn->fc_conn.cn_flags & LSCONN_RW_PENDING)); + if (conn->fc_flags & FC_HAVE_SAVED_ACK) + { + (void) /* If there is an error, we'll fail shortly */ + process_saved_ack(conn, 0); + conn->fc_flags &= ~FC_HAVE_SAVED_ACK; + } + lsquic_send_ctl_tick(&conn->fc_send_ctl, now); lsquic_send_ctl_set_buffer_stream_packets(&conn->fc_send_ctl, 1); CLOSE_IF_NECESSARY(); diff --git a/src/liblsquic/lsquic_packet_out.h b/src/liblsquic/lsquic_packet_out.h index d5e6089..089c5e6 100644 --- a/src/liblsquic/lsquic_packet_out.h +++ b/src/liblsquic/lsquic_packet_out.h @@ -83,10 +83,12 @@ typedef struct lsquic_packet_out * further writes are allowed. */ PO_SCHED = (1 <<14), /* On scheduled queue */ + PO_SENT_SZ = (1 <<15), } po_flags:16; enum quic_ft_bit po_frame_types:16; /* Bitmask of QUIC_FRAME_* */ unsigned short po_data_sz; /* Number of usable bytes in data */ unsigned short po_enc_data_sz; /* Number of usable bytes in data */ + unsigned short po_sent_sz; /* If PO_SENT_SZ is set, real size of sent buffer. */ unsigned short po_regen_sz; /* Number of bytes at the beginning * of data containing bytes that are * not to be retransmitted, e.g. ACK @@ -94,6 +96,9 @@ typedef struct lsquic_packet_out */ unsigned short po_n_alloc; /* Total number of bytes allocated in po_data */ unsigned char *po_data; + lsquic_packno_t po_ack2ed; /* If packet has ACK frame, value of + * largest acked in it. + */ /* A lot of packets contain data belonging to only one stream. Thus, * `one' is used first. If this is not enough, any number of @@ -142,6 +147,23 @@ typedef struct lsquic_packet_out ((p)->po_data_sz + lsquic_po_header_length((p)->po_flags) \ + QUIC_PACKET_HASH_SZ) +#if __GNUC__ +#if LSQUIC_EXTRA_CHECKS +#define lsquic_packet_out_sent_sz(p) ( \ + __builtin_expect(((p)->po_flags & PO_SENT_SZ), 1) ? \ + (assert((p)->po_sent_sz == lsquic_packet_out_total_sz(p)), \ + (p)->po_sent_sz) : lsquic_packet_out_total_sz(p)) +# else +#define lsquic_packet_out_sent_sz(p) ( \ + __builtin_expect(((p)->po_flags & PO_SENT_SZ), 1) ? \ + (p)->po_sent_sz : lsquic_packet_out_total_sz(p)) +#endif +#else +# define lsquic_packet_out_sent_sz(p) ( \ + (p)->po_flags & PO_SENT_SZ ? \ + (p)->po_sent_sz : lsquic_packet_out_total_sz(p)) +#endif + #define lsquic_packet_out_verneg(p) \ (((p)->po_flags & (PO_NOENCRYPT|PO_VERNEG)) == (PO_NOENCRYPT|PO_VERNEG)) diff --git a/src/liblsquic/lsquic_parse.h b/src/liblsquic/lsquic_parse.h index 9f5ebe4..3e5b004 100644 --- a/src/liblsquic/lsquic_parse.h +++ b/src/liblsquic/lsquic_parse.h @@ -31,6 +31,13 @@ typedef struct ack_info #endif } ack_info_t; +struct short_ack_info +{ + unsigned sai_n_timestamps; + lsquic_time_t sai_lack_delta; + struct lsquic_packno_range sai_range; +}; + #define largest_acked(acki) (+(acki)->ranges[0].high) #define smallest_acked(acki) (+(acki)->ranges[(acki)->n_ranges - 1].low) @@ -86,13 +93,11 @@ struct parse_funcs int (*pf_parse_ack_frame) (const unsigned char *buf, size_t buf_len, ack_info_t *ack_info); - lsquic_packno_t - (*pf_parse_ack_high) (const unsigned char *buf, size_t buf_len); int (*pf_gen_ack_frame) (unsigned char *outbuf, size_t outbuf_sz, gaf_rechist_first_f, gaf_rechist_next_f, gaf_rechist_largest_recv_f, void *rechist, lsquic_time_t now, - int *has_missing); + int *has_missing, lsquic_packno_t *largest_received); int (*pf_gen_stop_waiting_frame) (unsigned char *buf, size_t buf_len, lsquic_packno_t cur_packno, enum lsquic_packno_bits, diff --git a/src/liblsquic/lsquic_parse_gquic_Q041.c b/src/liblsquic/lsquic_parse_gquic_Q041.c index d4b59ff..483a1e3 100644 --- a/src/liblsquic/lsquic_parse_gquic_Q041.c +++ b/src/liblsquic/lsquic_parse_gquic_Q041.c @@ -263,28 +263,6 @@ gquic_ietf_gen_stream_frame (unsigned char *buf, size_t buf_len, uint32_t stream } -/* This is a special function: it is used to extract the largest observed - * packet number from ACK frame that we ourselves generated. This allows - * us to skip some checks. - */ -lsquic_packno_t -gquic_ietf_parse_ack_high (const unsigned char *buf, size_t buf_len) -{ - unsigned char type; - unsigned largest_obs_len; - unsigned n_blocks_len; - lsquic_packno_t packno; - - type = buf[0]; - largest_obs_len = twobit_to_1248((type >> 2) & 3); - n_blocks_len = !!(type & 0x10); - assert(parse_frame_type_gquic_Q041(type) == QUIC_FRAME_ACK); - assert(buf_len >= 1 + n_blocks_len + 1 + largest_obs_len); - READ_UINT(packno, 64, buf + 1 + n_blocks_len + 1, largest_obs_len); - return packno; -} - - int gquic_ietf_parse_ack_frame (const unsigned char *buf, size_t buf_len, ack_info_t *ack) @@ -379,7 +357,8 @@ int gquic_ietf_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz, gaf_rechist_first_f rechist_first, gaf_rechist_next_f rechist_next, gaf_rechist_largest_recv_f rechist_largest_recv, - void *rechist, lsquic_time_t now, int *has_missing) + void *rechist, lsquic_time_t now, int *has_missing, + lsquic_packno_t *largest_received) { lsquic_packno_t tmp_packno; const struct lsquic_packno_range *const first = rechist_first(rechist); @@ -533,6 +512,7 @@ gquic_ietf_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz, p += ack_block_len; } + *largest_received = maxno; return p - (unsigned char *) outbuf; #undef CHECKOUT @@ -549,7 +529,6 @@ const struct parse_funcs lsquic_parse_funcs_gquic_Q041 = .pf_parse_stream_frame_header_sz = gquic_ietf_parse_stream_frame_header_sz, .pf_parse_stream_frame = gquic_ietf_parse_stream_frame, .pf_parse_ack_frame = gquic_ietf_parse_ack_frame, - .pf_parse_ack_high = gquic_ietf_parse_ack_high, .pf_gen_ack_frame = gquic_ietf_gen_ack_frame, .pf_gen_stop_waiting_frame = gquic_be_gen_stop_waiting_frame, .pf_parse_stop_waiting_frame = gquic_be_parse_stop_waiting_frame, diff --git a/src/liblsquic/lsquic_parse_gquic_be.c b/src/liblsquic/lsquic_parse_gquic_be.c index ab2ae86..31ba037 100644 --- a/src/liblsquic/lsquic_parse_gquic_be.c +++ b/src/liblsquic/lsquic_parse_gquic_be.c @@ -363,26 +363,6 @@ gquic_be_parse_stream_frame (const unsigned char *buf, size_t rem_packet_sz, } -/* This is a special function: it is used to extract the largest observed - * packet number from ACK frame that we ourselves generated. This allows - * us to skip some checks. - */ -lsquic_packno_t -gquic_be_parse_ack_high (const unsigned char *buf, size_t buf_len) -{ - unsigned char type; - unsigned largest_obs_len; - lsquic_packno_t packno; - - type = buf[0]; - largest_obs_len = twobit_to_1246((type >> 2) & 3); - assert(parse_frame_type_gquic_Q035_thru_Q039(type) == QUIC_FRAME_ACK); - assert(buf_len >= 1 + largest_obs_len); - READ_UINT(packno, 64, buf + 1, largest_obs_len); - return packno; -} - - static int parse_ack_frame_without_blocks (const unsigned char *buf, size_t buf_len, ack_info_t *ack) @@ -809,7 +789,8 @@ int gquic_be_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz, gaf_rechist_first_f rechist_first, gaf_rechist_next_f rechist_next, gaf_rechist_largest_recv_f rechist_largest_recv, - void *rechist, lsquic_time_t now, int *has_missing) + void *rechist, lsquic_time_t now, int *has_missing, + lsquic_packno_t *largest_received) { lsquic_packno_t tmp_packno; const struct lsquic_packno_range *const first = rechist_first(rechist); @@ -960,6 +941,7 @@ gquic_be_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz, *p = 0; ++p; + *largest_received = maxno; return p - (unsigned char *) outbuf; #undef CHECKOUT @@ -976,7 +958,6 @@ const struct parse_funcs lsquic_parse_funcs_gquic_Q039 = .pf_parse_stream_frame_header_sz = parse_stream_frame_header_sz_gquic, .pf_parse_stream_frame = gquic_be_parse_stream_frame, .pf_parse_ack_frame = gquic_be_parse_ack_frame, - .pf_parse_ack_high = gquic_be_parse_ack_high, .pf_gen_ack_frame = gquic_be_gen_ack_frame, .pf_gen_stop_waiting_frame = gquic_be_gen_stop_waiting_frame, .pf_parse_stop_waiting_frame = gquic_be_parse_stop_waiting_frame, diff --git a/src/liblsquic/lsquic_parse_gquic_be.h b/src/liblsquic/lsquic_parse_gquic_be.h index ab2df5c..dfda24a 100644 --- a/src/liblsquic/lsquic_parse_gquic_be.h +++ b/src/liblsquic/lsquic_parse_gquic_be.h @@ -135,6 +135,6 @@ int gquic_be_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz, gaf_rechist_first_f rechist_first, gaf_rechist_next_f rechist_next, gaf_rechist_largest_recv_f rechist_largest_recv, - void *rechist, lsquic_time_t now, int *has_missing); + void *rechist, lsquic_time_t now, int *has_missing, lsquic_packno_t *); #endif diff --git a/src/liblsquic/lsquic_parse_gquic_le.c b/src/liblsquic/lsquic_parse_gquic_le.c index 544a906..151056a 100644 --- a/src/liblsquic/lsquic_parse_gquic_le.c +++ b/src/liblsquic/lsquic_parse_gquic_le.c @@ -333,24 +333,6 @@ gquic_le_parse_stream_frame (const unsigned char *buf, size_t rem_packet_sz, } -/* This is a special function: it is used to extract the largest observed - * packet number from ACK frame that we ourselves generated. This allows - * us to skip some checks. - */ -static lsquic_packno_t -gquic_le_parse_ack_high (const unsigned char *buf, size_t buf_len) -{ - unsigned char type; - unsigned largest_obs_len; - - type = buf[0]; - largest_obs_len = flag_to_pkt_num_len(type >> 2); - assert(parse_frame_type_gquic_Q035_thru_Q039(type) == QUIC_FRAME_ACK); - assert(buf_len >= 1 + largest_obs_len); - return get_vary_len_int64(buf + 1, largest_obs_len); -} - - /* Return parsed (used) buffer length. * If parsing failed, negative value is returned. */ @@ -701,7 +683,8 @@ static int gquic_le_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz, gaf_rechist_first_f rechist_first, gaf_rechist_next_f rechist_next, gaf_rechist_largest_recv_f rechist_largest_recv, - void *rechist, lsquic_time_t now, int *has_missing) + void *rechist, lsquic_time_t now, int *has_missing, + lsquic_packno_t *largest_received) { const struct lsquic_packno_range *const first = rechist_first(rechist); if (!first) @@ -835,6 +818,7 @@ gquic_le_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz, *p = 0; ++p; + *largest_received = maxno; return p - (unsigned char *) outbuf; #undef CHECKOUT @@ -851,7 +835,6 @@ const struct parse_funcs lsquic_parse_funcs_gquic_le = .pf_parse_stream_frame_header_sz = parse_stream_frame_header_sz_gquic, .pf_parse_stream_frame = gquic_le_parse_stream_frame, .pf_parse_ack_frame = gquic_le_parse_ack_frame, - .pf_parse_ack_high = gquic_le_parse_ack_high, .pf_gen_ack_frame = gquic_le_gen_ack_frame, .pf_gen_stop_waiting_frame = gquic_le_gen_stop_waiting_frame, .pf_parse_stop_waiting_frame = gquic_le_parse_stop_waiting_frame, diff --git a/src/liblsquic/lsquic_send_ctl.c b/src/liblsquic/lsquic_send_ctl.c index 413d6e5..6e86424 100644 --- a/src/liblsquic/lsquic_send_ctl.c +++ b/src/liblsquic/lsquic_send_ctl.c @@ -410,12 +410,9 @@ send_ctl_unacked_append (struct lsquic_send_ctl *ctl, static void send_ctl_unacked_remove (struct lsquic_send_ctl *ctl, - struct lsquic_packet_out *packet_out) + struct lsquic_packet_out *packet_out, unsigned packet_sz) { - unsigned packet_sz; - TAILQ_REMOVE(&ctl->sc_unacked_packets, packet_out, po_next); - packet_sz = lsquic_packet_out_total_sz(packet_out); assert(ctl->sc_bytes_unacked_all >= packet_sz); ctl->sc_bytes_unacked_all -= packet_sz; ctl->sc_n_in_flight_all -= 1; @@ -479,47 +476,56 @@ lsquic_send_ctl_sent_packet (lsquic_send_ctl_t *ctl, sizeof(frames), packet_out->po_frame_types)); if (account) ctl->sc_bytes_out -= lsquic_packet_out_total_sz(packet_out); - if (0 == lsquic_senhist_add(&ctl->sc_senhist, packet_out->po_packno)) + lsquic_senhist_add(&ctl->sc_senhist, packet_out->po_packno); + send_ctl_unacked_append(ctl, packet_out); + if (packet_out->po_frame_types & QFRAME_RETRANSMITTABLE_MASK) { - send_ctl_unacked_append(ctl, packet_out); - if (packet_out->po_frame_types & QFRAME_RETRANSMITTABLE_MASK) - { - if (!lsquic_alarmset_is_set(ctl->sc_alset, AL_RETX)) - set_retx_alarm(ctl); - if (ctl->sc_n_in_flight_retx == 1) - ctl->sc_flags |= SC_WAS_QUIET; - } - /* TODO: Do we really want to use those for RTT info? Revisit this. */ - /* Hold on to packets that are not retransmittable because we need them - * to sample RTT information. They are released when ACK is received. - */ -#if LSQUIC_SEND_STATS - ++ctl->sc_stats.n_total_sent; -#endif - return 0; + if (!lsquic_alarmset_is_set(ctl->sc_alset, AL_RETX)) + set_retx_alarm(ctl); + if (ctl->sc_n_in_flight_retx == 1) + ctl->sc_flags |= SC_WAS_QUIET; } - else - return -1; + /* TODO: Do we really want to use those for RTT info? Revisit this. */ + /* Hold on to packets that are not retransmittable because we need them + * to sample RTT information. They are released when ACK is received. + */ +#if LSQUIC_SEND_STATS + ++ctl->sc_stats.n_total_sent; +#endif + lsquic_send_ctl_sanity_check(ctl); + return 0; } static void -take_rtt_sample (lsquic_send_ctl_t *ctl, const lsquic_packet_out_t *packet_out, +take_rtt_sample (lsquic_send_ctl_t *ctl, lsquic_time_t now, lsquic_time_t lack_delta) { - assert(packet_out->po_sent); - lsquic_time_t measured_rtt = now - packet_out->po_sent; - if (packet_out->po_packno > ctl->sc_max_rtt_packno && lack_delta < measured_rtt) + const lsquic_packno_t packno = ctl->sc_largest_acked_packno; + const lsquic_time_t sent = ctl->sc_largest_acked_sent_time; + const lsquic_time_t measured_rtt = now - sent; + if (packno > ctl->sc_max_rtt_packno && lack_delta < measured_rtt) { - ctl->sc_max_rtt_packno = packet_out->po_packno; + ctl->sc_max_rtt_packno = packno; lsquic_rtt_stats_update(&ctl->sc_conn_pub->rtt_stats, measured_rtt, lack_delta); LSQ_DEBUG("packno %"PRIu64"; rtt: %"PRIu64"; delta: %"PRIu64"; " - "new srtt: %"PRIu64, packet_out->po_packno, measured_rtt, lack_delta, + "new srtt: %"PRIu64, packno, measured_rtt, lack_delta, lsquic_rtt_stats_get_srtt(&ctl->sc_conn_pub->rtt_stats)); } } +static void +send_ctl_release_enc_data (struct lsquic_send_ctl *ctl, + struct lsquic_packet_out *packet_out) +{ + ctl->sc_enpub->enp_pmi->pmi_release(ctl->sc_enpub->enp_pmi_ctx, + packet_out->po_enc_data); + packet_out->po_flags &= ~PO_ENCRYPTED; + packet_out->po_enc_data = NULL; +} + + /* Returns true if packet was rescheduled, false otherwise. In the latter * case, you should not dereference packet_out after the function returns. */ @@ -527,14 +533,13 @@ static int send_ctl_handle_lost_packet (lsquic_send_ctl_t *ctl, lsquic_packet_out_t *packet_out) { + unsigned packet_sz; + assert(ctl->sc_n_in_flight_all); - send_ctl_unacked_remove(ctl, packet_out); - if (packet_out->po_flags & PO_ENCRYPTED) { - ctl->sc_enpub->enp_pmi->pmi_release(ctl->sc_enpub->enp_pmi_ctx, - packet_out->po_enc_data); - packet_out->po_flags &= ~PO_ENCRYPTED; - packet_out->po_enc_data = NULL; - } + packet_sz = lsquic_packet_out_sent_sz(packet_out); + send_ctl_unacked_remove(ctl, packet_out, packet_sz); + if (packet_out->po_flags & PO_ENCRYPTED) + send_ctl_release_enc_data(ctl, packet_out); if (packet_out->po_frame_types & (1 << QUIC_FRAME_ACK)) { ctl->sc_flags |= SC_LOST_ACK; @@ -652,102 +657,110 @@ lsquic_send_ctl_got_ack (lsquic_send_ctl_t *ctl, { struct lsquic_packets_tailq acked_acks = TAILQ_HEAD_INITIALIZER(acked_acks); -#if !LSQUIC_CAN_REORDER const struct lsquic_packno_range *range = &acki->ranges[ acki->n_ranges - 1 ]; -#endif lsquic_packet_out_t *packet_out, *next; - lsquic_time_t now = lsquic_time_now(); - lsquic_packno_t high; - int rtt_updated = 0; + lsquic_time_t now = 0; + lsquic_packno_t smallest_unacked; + lsquic_packno_t ack2ed[2]; + unsigned packet_sz; int app_limited; - unsigned n; + signed char do_rtt, skip_checks; - LSQ_DEBUG("Got ACK frame, largest acked: %"PRIu64"; delta: %"PRIu64, - largest_acked(acki), acki->lack_delta); + packet_out = TAILQ_FIRST(&ctl->sc_unacked_packets); +#if __GNUC__ + __builtin_prefetch(packet_out); +#endif + +#if __GNUC__ +# define UNLIKELY(cond) __builtin_expect(cond, 0) +#else +# define UNLIKELY(cond) cond +#endif + +#if __GNUC__ + if (UNLIKELY(LSQ_LOG_ENABLED(LSQ_LOG_DEBUG))) +#endif + LSQ_DEBUG("Got ACK frame, largest acked: %"PRIu64"; delta: %"PRIu64, + largest_acked(acki), acki->lack_delta); /* Validate ACK first: */ - for (n = 0; n < acki->n_ranges; ++n) - if (!lsquic_senhist_sent_range(&ctl->sc_senhist, acki->ranges[n].low, - acki->ranges[n].high)) - { - LSQ_INFO("at least one packet in ACK range [%"PRIu64" - %"PRIu64"] " - "was never sent", acki->ranges[n].low, acki->ranges[n].high); - return -1; - } + if (UNLIKELY(largest_acked(acki) + > lsquic_senhist_largest(&ctl->sc_senhist))) + { + LSQ_INFO("at least one packet in ACK range [%"PRIu64" - %"PRIu64"] " + "was never sent", acki->ranges[0].low, acki->ranges[0].high); + return -1; + } - if (ctl->sc_flags & SC_WAS_QUIET) + if (UNLIKELY(ctl->sc_flags & SC_WAS_QUIET)) { ctl->sc_flags &= ~SC_WAS_QUIET; LSQ_DEBUG("ACK comes after a period of quiescence"); + if (!now) + now = lsquic_time_now(); lsquic_cubic_was_quiet(&ctl->sc_cubic, now); } - /* Peer is acking packets that have been acked already. Schedule ACK - * and STOP_WAITING frame to chop the range if we get two of these in - * a row. - */ - if (lsquic_send_ctl_smallest_unacked(ctl) > smallest_acked(acki)) - ++ctl->sc_n_stop_waiting; - else - ctl->sc_n_stop_waiting = 0; + if (UNLIKELY(!packet_out)) + goto no_unacked_packets; - app_limited = send_ctl_retx_bytes_out(ctl) + 3 * ctl->sc_pack_size /* This - is the "maximum burst" parameter */ - < lsquic_cubic_get_cwnd(&ctl->sc_cubic); + smallest_unacked = packet_out->po_packno; + ack2ed[1] = 0; - for (packet_out = TAILQ_FIRST(&ctl->sc_unacked_packets); - packet_out -#if !LSQUIC_CAN_REORDER - && packet_out->po_packno <= largest_acked(acki) -#endif - ; - packet_out = next) + if (packet_out->po_packno > largest_acked(acki)) + goto detect_losses; + + do_rtt = 0, skip_checks = 0; + app_limited = -1; + do { next = TAILQ_NEXT(packet_out, po_next); -#if LSQUIC_CAN_REORDER - if (!in_acked_range(acki, packet_out->po_packno)) - continue; -#else - /* This is faster than binary search in the normal case when the number - * of ranges is not much larger than the number of unacked packets. - */ - while (range->high < packet_out->po_packno) - --range; - if (range->low > packet_out->po_packno) - continue; -#endif - ctl->sc_largest_acked_packno = packet_out->po_packno; - ctl->sc_largest_acked_sent_time = packet_out->po_sent; - if (packet_out->po_packno == largest_acked(acki)) - { - take_rtt_sample(ctl, packet_out, ack_recv_time, acki->lack_delta); - ++rtt_updated; - } - lsquic_cubic_ack(&ctl->sc_cubic, now, now - packet_out->po_sent, - app_limited, lsquic_packet_out_total_sz(packet_out)); - LSQ_DEBUG("Got ACK for packet %"PRIu64", remove from unacked queue", - packet_out->po_packno); - assert(ctl->sc_n_in_flight_all); - send_ctl_unacked_remove(ctl, packet_out); - lsquic_packet_out_ack_streams(packet_out); #if __GNUC__ __builtin_prefetch(next); #endif - if ((ctl->sc_flags & SC_NSTP) && - (packet_out->po_frame_types & (1 << QUIC_FRAME_ACK))) - TAILQ_INSERT_TAIL(&acked_acks, packet_out, po_next); - else + if (skip_checks) + goto after_checks; + /* This is faster than binary search in the normal case when the number + * of ranges is not much larger than the number of unacked packets. + */ + while (UNLIKELY(range->high < packet_out->po_packno)) + --range; + if (range->low <= packet_out->po_packno) + { + skip_checks = range == acki->ranges; + if (app_limited < 0) + app_limited = send_ctl_retx_bytes_out(ctl) + 3 * ctl->sc_pack_size /* This + is the "maximum burst" parameter */ + < lsquic_cubic_get_cwnd(&ctl->sc_cubic); + if (!now) + now = lsquic_time_now(); + after_checks: + packet_sz = lsquic_packet_out_sent_sz(packet_out); + ctl->sc_largest_acked_packno = packet_out->po_packno; + ctl->sc_largest_acked_sent_time = packet_out->po_sent; + send_ctl_unacked_remove(ctl, packet_out, packet_sz); + ack2ed[!!(packet_out->po_frame_types & (1 << QUIC_FRAME_ACK))] + = packet_out->po_ack2ed; + do_rtt |= packet_out->po_packno == largest_acked(acki); + lsquic_cubic_ack(&ctl->sc_cubic, now, now - packet_out->po_sent, + app_limited, packet_sz); + lsquic_packet_out_ack_streams(packet_out); lsquic_packet_out_destroy(packet_out, ctl->sc_enpub); + } + packet_out = next; } + while (packet_out && packet_out->po_packno <= largest_acked(acki)); - if (rtt_updated) + if (do_rtt) { + take_rtt_sample(ctl, ack_recv_time, acki->lack_delta); ctl->sc_n_consec_rtos = 0; ctl->sc_n_hsk = 0; ctl->sc_n_tlp = 0; } + detect_losses: send_ctl_detect_losses(ctl, ack_recv_time); if (send_ctl_first_unacked_retx_packet(ctl)) set_retx_alarm(ctl); @@ -758,29 +771,28 @@ lsquic_send_ctl_got_ack (lsquic_send_ctl_t *ctl, } lsquic_send_ctl_sanity_check(ctl); - /* Processing of packets that contain acked ACK frames is deferred because - * we only need to process one of them: the last one, which we know to - * contain the largest value. - */ - packet_out = TAILQ_LAST(&acked_acks, lsquic_packets_tailq); - if (packet_out) - { - high = ctl->sc_conn_pub->lconn->cn_pf->pf_parse_ack_high( - packet_out->po_data, packet_out->po_data_sz); - if (high > ctl->sc_largest_ack2ed) - ctl->sc_largest_ack2ed = high; - do - { - next = TAILQ_PREV(packet_out, lsquic_packets_tailq, po_next); - lsquic_packet_out_destroy(packet_out, ctl->sc_enpub); - } - while ((packet_out = next)); - } + if ((ctl->sc_flags & SC_NSTP) && ack2ed[1] > ctl->sc_largest_ack2ed) + ctl->sc_largest_ack2ed = ack2ed[1]; if (ctl->sc_n_in_flight_retx == 0) ctl->sc_flags |= SC_WAS_QUIET; + update_n_stop_waiting: + if (smallest_unacked > smallest_acked(acki)) + /* Peer is acking packets that have been acked already. Schedule ACK + * and STOP_WAITING frame to chop the range if we get two of these in + * a row. + */ + ++ctl->sc_n_stop_waiting; + else + ctl->sc_n_stop_waiting = 0; + lsquic_send_ctl_sanity_check(ctl); return 0; + + no_unacked_packets: + smallest_unacked = lsquic_senhist_largest(&ctl->sc_senhist) + 1; + ctl->sc_flags |= SC_WAS_QUIET; + goto update_n_stop_waiting; } @@ -793,7 +805,7 @@ lsquic_send_ctl_smallest_unacked (lsquic_send_ctl_t *ctl) * on purpose). Thus, the first packet on the unacked packets list has * the smallest packet number of all packets on that list. */ - if ((packet_out = TAILQ_FIRST(&ctl->sc_unacked_packets))) + if ((packet_out = TAILQ_FIRST(&ctl->sc_unacked_packets))) return packet_out->po_packno; else return lsquic_senhist_largest(&ctl->sc_senhist) + 1; @@ -964,7 +976,7 @@ void lsquic_send_ctl_sanity_check (const lsquic_send_ctl_t *ctl) { const struct lsquic_packet_out *packet_out; - unsigned count, sched_bytes; + unsigned count, bytes; assert(!send_ctl_first_unacked_retx_packet(ctl) || lsquic_alarmset_is_set(ctl->sc_alset, AL_RETX)); @@ -974,20 +986,24 @@ lsquic_send_ctl_sanity_check (const lsquic_send_ctl_t *ctl) assert(lsquic_time_now() < ctl->sc_alset->as_expiry[AL_RETX] + MAX_RTO_DELAY); } - count = 0; + count = 0, bytes = 0; TAILQ_FOREACH(packet_out, &ctl->sc_unacked_packets, po_next) + { + bytes += lsquic_packet_out_sent_sz(packet_out); ++count; + } assert(count == ctl->sc_n_in_flight_all); + assert(bytes == ctl->sc_bytes_unacked_all); - count = 0, sched_bytes = 0; + count = 0, bytes = 0; TAILQ_FOREACH(packet_out, &ctl->sc_scheduled_packets, po_next) { assert(packet_out->po_flags & PO_SCHED); - sched_bytes += lsquic_packet_out_total_sz(packet_out); + bytes += lsquic_packet_out_total_sz(packet_out); ++count; } assert(count == ctl->sc_n_scheduled); - assert(sched_bytes == ctl->sc_bytes_scheduled); + assert(bytes == ctl->sc_bytes_scheduled); } @@ -1202,6 +1218,7 @@ update_for_resending (lsquic_send_ctl_t *ctl, lsquic_packet_out_t *packet_out) oldno = packet_out->po_packno; packno = send_ctl_next_packno(ctl); + packet_out->po_flags &= ~PO_SENT_SZ; packet_out->po_frame_types &= ~QFRAME_REGEN_MASK; assert(packet_out->po_frame_types); packet_out->po_packno = packno; @@ -1283,8 +1300,10 @@ void lsquic_send_ctl_elide_stream_frames (lsquic_send_ctl_t *ctl, uint32_t stream_id) { struct lsquic_packet_out *packet_out, *next; + struct lsquic_packet_out *pre_dropped; unsigned n, adj; + pre_dropped = NULL; for (packet_out = TAILQ_FIRST(&ctl->sc_scheduled_packets); packet_out; packet_out = next) { @@ -1298,6 +1317,9 @@ lsquic_send_ctl_elide_stream_frames (lsquic_send_ctl_t *ctl, uint32_t stream_id) ctl->sc_bytes_scheduled -= adj; if (0 == packet_out->po_frame_types) { + if (!pre_dropped) + pre_dropped = TAILQ_PREV(packet_out, lsquic_packets_tailq, + po_next); LSQ_DEBUG("cancel packet %"PRIu64" after eliding frames for " "stream %"PRIu32, packet_out->po_packno, stream_id); send_ctl_sched_remove(ctl, packet_out); @@ -1306,6 +1328,21 @@ lsquic_send_ctl_elide_stream_frames (lsquic_send_ctl_t *ctl, uint32_t stream_id) } } + /* Need to assign new packet numbers to all packets following the first + * dropped packet to eliminate packet number gap. + */ + if (pre_dropped) + { + ctl->sc_cur_packno = lsquic_senhist_largest(&ctl->sc_senhist); + for (packet_out = TAILQ_NEXT(pre_dropped, po_next); packet_out; + packet_out = TAILQ_NEXT(packet_out, po_next)) + { + packet_out->po_flags |= PO_REPACKNO; + if (packet_out->po_flags & PO_ENCRYPTED) + send_ctl_release_enc_data(ctl, packet_out); + } + } + for (n = 0; n < sizeof(ctl->sc_buffered_packets) / sizeof(ctl->sc_buffered_packets[0]); ++n) { @@ -1416,12 +1453,7 @@ lsquic_send_ctl_squeeze_sched (lsquic_send_ctl_t *ctl) if (packet_out->po_regen_sz < packet_out->po_data_sz) { if (packet_out->po_flags & PO_ENCRYPTED) - { - ctl->sc_enpub->enp_pmi->pmi_release(ctl->sc_enpub->enp_pmi_ctx, - packet_out->po_enc_data); - packet_out->po_enc_data = NULL; - packet_out->po_flags &= ~PO_ENCRYPTED; - } + send_ctl_release_enc_data(ctl, packet_out); } else { @@ -1486,6 +1518,7 @@ lsquic_send_ctl_drop_scheduled (lsquic_send_ctl_t *ctl) lsquic_packet_out_destroy(packet_out, ctl->sc_enpub); } assert(0 == ctl->sc_n_scheduled); + ctl->sc_cur_packno = lsquic_senhist_largest(&ctl->sc_senhist); LSQ_DEBUG("dropped %u scheduled packet%s", n, n != 0 ? "s" : ""); } diff --git a/src/liblsquic/lsquic_send_ctl.h b/src/liblsquic/lsquic_send_ctl.h index d80c793..b15c5b5 100644 --- a/src/liblsquic/lsquic_send_ctl.h +++ b/src/liblsquic/lsquic_send_ctl.h @@ -28,23 +28,51 @@ struct buf_packet_q unsigned bpq_count; }; +enum send_ctl_flags { + SC_TCID0 = (1 << 0), + SC_LOST_ACK = (1 << 1), + SC_NSTP = (1 << 2), + SC_PACE = (1 << 3), + SC_SCHED_TICK = (1 << 4), + SC_BUFFER_STREAM= (1 << 5), + SC_WAS_QUIET = (1 << 6), +}; + typedef struct lsquic_send_ctl { + /* The first section consists of struct members which are used in the + * time-critical lsquic_send_ctl_got_ack() in the approximate order + * of usage. + */ + lsquic_senhist_t sc_senhist; + enum send_ctl_flags sc_flags; + unsigned sc_n_stop_waiting; + struct lsquic_packets_tailq sc_unacked_packets; + lsquic_packno_t sc_largest_acked_packno; + lsquic_time_t sc_largest_acked_sent_time; + unsigned sc_bytes_out; + unsigned sc_bytes_unacked_retx; + unsigned sc_bytes_scheduled; + unsigned sc_pack_size; + struct lsquic_cubic sc_cubic; + struct lsquic_engine_public *sc_enpub; + unsigned sc_bytes_unacked_all; + unsigned sc_n_in_flight_all; + unsigned sc_n_in_flight_retx; + unsigned sc_n_consec_rtos; + unsigned sc_n_hsk; + unsigned sc_n_tlp; + struct lsquic_alarmset *sc_alset; + + /* Second section: everything else. */ struct lsquic_packets_tailq sc_scheduled_packets, - sc_unacked_packets, sc_lost_packets; struct buf_packet_q sc_buffered_packets[BPT_OTHER_PRIO + 1]; - struct lsquic_engine_public *sc_enpub; - struct lsquic_alarmset *sc_alset; const struct ver_neg *sc_ver_neg; struct lsquic_conn_public *sc_conn_pub; - struct lsquic_cubic sc_cubic; struct pacer sc_pacer; - lsquic_senhist_t sc_senhist; lsquic_packno_t sc_cur_packno; lsquic_packno_t sc_largest_sent_at_cutback; lsquic_packno_t sc_max_rtt_packno; - lsquic_packno_t sc_largest_acked_packno; - lsquic_time_t sc_largest_acked_sent_time; /* sc_largest_ack2ed is the packet number sent by peer that we acked and * we know that our ACK was received by peer. This is used to determine * the receive history cutoff point for the purposes of generating ACK @@ -59,28 +87,8 @@ typedef struct lsquic_send_ctl { uint32_t stream_id; enum buf_packet_type packet_type; } sc_cached_bpt; - unsigned sc_bytes_out; - unsigned sc_bytes_unacked_all; - unsigned sc_bytes_unacked_retx; - unsigned sc_n_consec_rtos; unsigned sc_next_limit; - unsigned sc_n_in_flight_all; - unsigned sc_n_in_flight_retx; unsigned sc_n_scheduled; - unsigned sc_bytes_scheduled; - unsigned sc_n_stop_waiting; - unsigned short sc_pack_size; - enum { - SC_TCID0 = (1 << 0), - SC_LOST_ACK = (1 << 1), - SC_NSTP = (1 << 2), - SC_PACE = (1 << 3), - SC_SCHED_TICK = (1 << 4), - SC_BUFFER_STREAM= (1 << 5), - SC_WAS_QUIET = (1 << 6), - } sc_flags:8; - unsigned char sc_n_hsk; - unsigned char sc_n_tlp; #if LSQUIC_SEND_STATS struct { unsigned n_total_sent, diff --git a/src/liblsquic/lsquic_senhist.c b/src/liblsquic/lsquic_senhist.c index 28e38d5..63997b4 100644 --- a/src/liblsquic/lsquic_senhist.c +++ b/src/liblsquic/lsquic_senhist.c @@ -3,126 +3,18 @@ * lsquic_senhist.c -- Sent history implementation */ -#include #include #include -#include -#include #include "lsquic_int_types.h" #include "lsquic_senhist.h" -void -lsquic_senhist_init (lsquic_senhist_t *hist) -{ - lsquic_packints_init(&hist->sh_pints); -} - - -void -lsquic_senhist_cleanup (lsquic_senhist_t *hist) -{ - lsquic_packints_cleanup(&hist->sh_pints); -} - - -/* At the time of this writing, the only reason the sequence of sent - * packet numbers could contain a hole is elision of stream frames from - * scheduled, but delayed packets. If such packet becomes empty after - * elision, it is dropped from the queue. - */ - - -/* The fast insert is used in the normal case, when packets are sent - * out in the same order in which they are scheduled: that is, their - * packet numbers are always increasing. - */ -static int -senhist_add_fast (lsquic_senhist_t *hist, lsquic_packno_t packno) -{ - struct packet_interval *pi; - - pi = TAILQ_FIRST(&hist->sh_pints.pk_intervals); - if (pi) - { - /* Check that packet numbers are always increasing */ - assert(packno > pi->range.high); - if (packno == pi->range.high + 1) - { - ++pi->range.high; - return 0; - } - } - - pi = malloc(sizeof(*pi)); - if (!pi) - return -1; - pi->range.high = packno; - pi->range.low = packno; - TAILQ_INSERT_HEAD(&hist->sh_pints.pk_intervals, pi, next_pi); - return 0; -} - - -int -lsquic_senhist_add (lsquic_senhist_t *hist, lsquic_packno_t packno) -{ - return senhist_add_fast(hist, packno); -} - - -int -lsquic_senhist_sent_range (lsquic_senhist_t *hist, lsquic_packno_t low, - lsquic_packno_t high) -{ - const struct lsquic_packno_range *range; - for (range = lsquic_packints_first(&hist->sh_pints); range; - range = lsquic_packints_next(&hist->sh_pints)) - if (range->low <= low && range->high >= high) - return 1; - return 0; -} - - -lsquic_packno_t -lsquic_senhist_largest (lsquic_senhist_t *hist) -{ - const struct lsquic_packno_range *range; - range = lsquic_packints_first(&hist->sh_pints); - if (range) - return range->high; - else - return 0; -} - - void lsquic_senhist_tostr (lsquic_senhist_t *hist, char *buf, size_t bufsz) { - const struct lsquic_packno_range *range; - size_t off; - int n; - for (off = 0, range = lsquic_packints_first(&hist->sh_pints); - range && off < bufsz; - off += n, range = lsquic_packints_next(&hist->sh_pints)) - { - n = snprintf(buf + off, bufsz - off, "[%"PRIu64"-%"PRIu64"]", - range->high, range->low); - if (n < 0 || (size_t) n >= bufsz - off) - break; - } - if (bufsz > 0) - buf[off] = '\0'; + if (hist->sh_last_sent) + snprintf(buf, bufsz, "[1-%"PRIu64"]", hist->sh_last_sent); + else + snprintf(buf, bufsz, "[]"); } - - -size_t -lsquic_senhist_mem_used (const struct lsquic_senhist *hist) -{ - return sizeof(*hist) - - sizeof(hist->sh_pints) - + lsquic_packints_mem_used(&hist->sh_pints); -} - - diff --git a/src/liblsquic/lsquic_senhist.h b/src/liblsquic/lsquic_senhist.h index 5e5a7bb..20e9aa7 100644 --- a/src/liblsquic/lsquic_senhist.h +++ b/src/liblsquic/lsquic_senhist.h @@ -2,49 +2,61 @@ /* * lsquic_senhist.h -- History sent packets. * - * We only keep track of packet numbers in order to verify ACKs. + * We need to keep track of packet numbers in order to verify ACKs. To + * speed up processing, we make sure that there is never a gap in the + * packet number sequence we generate. */ #ifndef LSQUIC_SENHIST_H #define LSQUIC_SENHIST_H 1 -#include "lsquic_packints.h" +#ifndef LSQUIC_SENHIST_FATAL +# define LSQUIC_SENHIST_FATAL 0 +#endif typedef struct lsquic_senhist { - /* These ranges are ordered from high to low. While searching this - * structure is O(n), I expect that in practice, a very long search - * could only happen once before the connection is terminated, - * because: - * a) either the packet number far away is real, but it was so long - * ago that it would have timed out by now (RTO); or - * b) the peer sends an invalid ACK. - */ - struct packints sh_pints; + lsquic_packno_t sh_last_sent; +#if !LSQUIC_SENHIST_FATAL + enum { + SH_WARNED = 1 << 0, /* Warn once */ + } sh_flags; +#endif } lsquic_senhist_t; -void -lsquic_senhist_init (lsquic_senhist_t *); +#define lsquic_senhist_init(hist) do { \ + (hist)->sh_last_sent = 0; \ +} while (0) -void -lsquic_senhist_cleanup (lsquic_senhist_t *); +#define lsquic_senhist_cleanup(hist) -int -lsquic_senhist_add (lsquic_senhist_t *, lsquic_packno_t); - -/* Returns true if history contains all packets numbers in this range. - */ -int -lsquic_senhist_sent_range (lsquic_senhist_t *, lsquic_packno_t low, - lsquic_packno_t high); +#if LSQUIC_SENHIST_FATAL +#define lsquic_senhist_add(hist, packno) do { \ + assert((hist)->sh_last_sent == packno - 1); \ + (hist)->sh_last_sent = packno; \ +} while (0) +#else +#define lsquic_senhist_add(hist, packno) do { \ + if ((hist)->sh_last_sent == packno - 1) \ + (hist)->sh_last_sent = packno; \ + else \ + { \ + if (!((hist)->sh_flags & SH_WARNED)) \ + { \ + LSQ_WARN("send history gap %"PRIu64" - %"PRIu64, \ + (hist)->sh_last_sent, packno); \ + (hist)->sh_flags |= SH_WARNED; \ + } \ + (hist)->sh_last_sent = packno; \ + } \ +} while (0) +#endif /* Returns 0 if no packets have been sent yet */ -lsquic_packno_t -lsquic_senhist_largest (lsquic_senhist_t *hist); +#define lsquic_senhist_largest(hist) (+(hist)->sh_last_sent) void lsquic_senhist_tostr (lsquic_senhist_t *hist, char *buf, size_t bufsz); -size_t -lsquic_senhist_mem_used (const struct lsquic_senhist *); +#define lsquic_senhist_mem_used(hist) (sizeof(*(hist))) #endif diff --git a/test/http_client.c b/test/http_client.c index 05486df..87782cc 100644 --- a/test/http_client.c +++ b/test/http_client.c @@ -118,10 +118,11 @@ http_client_on_conn_closed (lsquic_conn_t *conn) status = lsquic_conn_status(conn, errmsg, sizeof(errmsg)); LSQ_INFO("Connection closed. Status: %d. Message: %s", status, errmsg[0] ? errmsg : ""); -#ifndef NDEBUG if (conn_h->client_ctx->hcc_flags & HCC_ABORT_ON_INCOMPLETE) - assert(conn_h->client_ctx->hcc_flags & HCC_SEEN_FIN); -#endif + { + if (!(conn_h->client_ctx->hcc_flags & HCC_SEEN_FIN)) + abort(); + } TAILQ_REMOVE(&conn_h->client_ctx->conn_ctxs, conn_h, next_ch); --conn_h->client_ctx->hcc_n_open_conns; create_connections(conn_h->client_ctx); @@ -432,7 +433,7 @@ main (int argc, char **argv) prog_init(&prog, LSENG_HTTP, &sports, &http_client_if, &client_ctx); - while (-1 != (opt = getopt(argc, argv, PROG_OPTS "r:R:Ku:EP:M:n:H:p:h"))) + while (-1 != (opt = getopt(argc, argv, PROG_OPTS "r:R:IKu:EP:M:n:H:p:h"))) { switch (opt) { case 'I': diff --git a/test/unittests/test_ackgen_gquic_be.c b/test/unittests/test_ackgen_gquic_be.c index baea00d..30041af 100644 --- a/test/unittests/test_ackgen_gquic_be.c +++ b/test/unittests/test_ackgen_gquic_be.c @@ -24,6 +24,7 @@ test1 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock */ { lsquic_rechist_t rechist; lsquic_time_t now = lsquic_time_now(); + lsquic_packno_t largest = 0; lsquic_rechist_init(&rechist, 0); @@ -45,14 +46,13 @@ test1 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock */ (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now + 0x7FF8000, &has_missing); + &rechist, now + 0x7FF8000, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has no missing packets", has_missing == 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(0x1234 == ack_high); + assert(largest == 0x1234); lsquic_rechist_cleanup(&rechist); } @@ -102,18 +102,18 @@ test2 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock, minus unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(0x1234 == ack_high); + assert(largest == 0x1234); lsquic_rechist_cleanup(&rechist); } @@ -147,18 +147,18 @@ test3 (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(3 == ack_high); + assert(largest == 0x03); lsquic_rechist_cleanup(&rechist); } @@ -185,18 +185,18 @@ test4 (void) }; unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has no missing packets", has_missing == 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(1 == ack_high); + assert(largest == 1); } for (i = 3; i <= 5; ++i) @@ -215,18 +215,18 @@ test4 (void) }; unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(5 == ack_high); + assert(largest == 5); } lsquic_rechist_cleanup(&rechist); @@ -267,16 +267,18 @@ test_4byte_packnos (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); + assert(largest == 0x23456789); lsquic_rechist_cleanup(&rechist); } @@ -316,16 +318,18 @@ test_6byte_packnos (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); + assert(largest == 0xABCD23456789ULL); lsquic_rechist_cleanup(&rechist); } diff --git a/test/unittests/test_ackgen_gquic_ietf.c b/test/unittests/test_ackgen_gquic_ietf.c index c168b95..bf113b6 100644 --- a/test/unittests/test_ackgen_gquic_ietf.c +++ b/test/unittests/test_ackgen_gquic_ietf.c @@ -24,6 +24,7 @@ test1 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock */ { lsquic_rechist_t rechist; lsquic_time_t now = lsquic_time_now(); + lsquic_packno_t largest = 0; lsquic_rechist_init(&rechist, 0); @@ -46,14 +47,13 @@ test1 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock */ (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now + 0x7FF8000, &has_missing); + &rechist, now + 0x7FF8000, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has no missing packets", has_missing == 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(0x1234 == ack_high); + assert(largest == 0x1234); lsquic_rechist_cleanup(&rechist); } @@ -104,18 +104,18 @@ test2 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock, minus unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(0x1234 == ack_high); + assert(largest == 0x1234); lsquic_rechist_cleanup(&rechist); } @@ -150,18 +150,18 @@ test3 (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(3 == ack_high); + assert(largest == 0x03); lsquic_rechist_cleanup(&rechist); } @@ -189,18 +189,18 @@ test4 (void) }; unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has no missing packets", has_missing == 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(1 == ack_high); + assert(largest == 1); } for (i = 3; i <= 5; ++i) @@ -220,18 +220,18 @@ test4 (void) }; unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(5 == ack_high); + assert(largest == 5); } lsquic_rechist_cleanup(&rechist); @@ -270,16 +270,18 @@ test_4byte_packnos (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); + assert(largest == 0x23456789); lsquic_rechist_cleanup(&rechist); } @@ -317,16 +319,18 @@ test_8byte_packnos (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); + assert(largest == 0xABCD23456789ULL); lsquic_rechist_cleanup(&rechist); } diff --git a/test/unittests/test_ackgen_gquic_le.c b/test/unittests/test_ackgen_gquic_le.c index 47244eb..3c0a14d 100644 --- a/test/unittests/test_ackgen_gquic_le.c +++ b/test/unittests/test_ackgen_gquic_le.c @@ -24,6 +24,7 @@ test1 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock */ { lsquic_rechist_t rechist; lsquic_time_t now = lsquic_time_now(); + lsquic_packno_t largest = 0; lsquic_rechist_init(&rechist, 0); @@ -45,14 +46,13 @@ test1 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock */ (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now + 0x7FF8000, &has_missing); + &rechist, now + 0x7FF8000, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has no missing packets", has_missing == 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(0x1234 == ack_high); + assert(largest == 0x1234); lsquic_rechist_cleanup(&rechist); } @@ -102,18 +102,18 @@ test2 (void) /* Inverse of quic_framer_test.cc -- NewAckFrameOneAckBlock, minus unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(0x1234 == ack_high); + assert(largest == 0x1234); lsquic_rechist_cleanup(&rechist); } @@ -147,18 +147,18 @@ test3 (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(3 == ack_high); + assert(largest == 0x03); lsquic_rechist_cleanup(&rechist); } @@ -185,18 +185,18 @@ test4 (void) }; unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has no missing packets", has_missing == 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(1 == ack_high); + assert(largest == 1); } for (i = 3; i <= 5; ++i) @@ -215,18 +215,18 @@ test4 (void) }; unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(outbuf, sizeof(outbuf)); - assert(5 == ack_high); + assert(largest == 5); } lsquic_rechist_cleanup(&rechist); @@ -267,16 +267,18 @@ test_4byte_packnos (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); + assert(largest == 0x23456789); lsquic_rechist_cleanup(&rechist); } @@ -316,16 +318,18 @@ test_6byte_packnos (void) unsigned char outbuf[0x100]; int has_missing = -1; + lsquic_packno_t largest = 0; int w = pf->pf_gen_ack_frame(outbuf, sizeof(outbuf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(("ACK frame generation successful", w > 0)); assert(("ACK frame length is correct", w == sizeof(expected_ack_frame))); assert(("ACK frame contents are as expected", 0 == memcmp(outbuf, expected_ack_frame, sizeof(expected_ack_frame)))); assert(("ACK frame has missing packets", has_missing > 0)); + assert(largest == 0xABCD23456789ULL); lsquic_rechist_cleanup(&rechist); } diff --git a/test/unittests/test_ackparse_gquic_be.c b/test/unittests/test_ackparse_gquic_be.c index 2749800..76879d8 100644 --- a/test/unittests/test_ackparse_gquic_be.c +++ b/test/unittests/test_ackparse_gquic_be.c @@ -48,8 +48,6 @@ test1 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 0x1234", n == 0x1234)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(0x1234 == ack_high); { size_t sz; @@ -111,8 +109,6 @@ test2 (void) assert(("Number of timestamps is 2", acki.n_timestamps == 2)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 4254", n == 4254)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(0x1234 == ack_high); for (n = 0; n < 4; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -164,8 +160,6 @@ test3 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 4", n == 4)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(6 == ack_high); for (n = 0; n < 2; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -216,8 +210,6 @@ test4 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 2", n == 2)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(3 == ack_high); for (n = 0; n < 2; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -359,11 +351,12 @@ test_max_ack (void) memset(buf, 0xAA, sizeof(buf)); + lsquic_packno_t largest = 0; sz[0] = pf->pf_gen_ack_frame(buf, sizeof(buf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(sz[0] > 0); assert(sz[0] <= (int) sizeof(buf)); assert(has_missing); @@ -412,11 +405,12 @@ test_ack_truncation (void) for (bufsz = 200; bufsz < 210; ++bufsz) { memset(buf, 0xAA, sizeof(buf)); + lsquic_packno_t largest = 0; sz[0] = pf->pf_gen_ack_frame(buf, bufsz, (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(sz[0] > 0); assert(sz[0] <= (int) bufsz); assert(has_missing); diff --git a/test/unittests/test_ackparse_gquic_ietf.c b/test/unittests/test_ackparse_gquic_ietf.c index 11bb4df..85bcdba 100644 --- a/test/unittests/test_ackparse_gquic_ietf.c +++ b/test/unittests/test_ackparse_gquic_ietf.c @@ -49,8 +49,6 @@ test1 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 0x1234", n == 0x1234)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(0x1234 == ack_high); { size_t sz; @@ -113,8 +111,6 @@ test2 (void) assert(("Number of timestamps is 2", acki.n_timestamps == 2)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 4254", n == 4254)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(0x1234 == ack_high); for (n = 0; n < 4; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -167,8 +163,6 @@ test3 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 4", n == 4)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(6 == ack_high); for (n = 0; n < 2; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -220,8 +214,6 @@ test4 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 2", n == 2)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(3 == ack_high); for (n = 0; n < 2; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -359,11 +351,12 @@ test_max_ack (void) memset(buf, 0xAA, sizeof(buf)); + lsquic_packno_t largest = 0; sz[0] = pf->pf_gen_ack_frame(buf, sizeof(buf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(sz[0] > 0); assert(sz[0] <= (int) sizeof(buf)); assert(has_missing); @@ -412,11 +405,12 @@ test_ack_truncation (void) for (bufsz = 200; bufsz < 210; ++bufsz) { memset(buf, 0xAA, sizeof(buf)); + lsquic_packno_t largest = 0; sz[0] = pf->pf_gen_ack_frame(buf, bufsz, (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(sz[0] > 0); assert(sz[0] <= (int) bufsz); assert(has_missing); diff --git a/test/unittests/test_ackparse_gquic_le.c b/test/unittests/test_ackparse_gquic_le.c index b501271..819e18f 100644 --- a/test/unittests/test_ackparse_gquic_le.c +++ b/test/unittests/test_ackparse_gquic_le.c @@ -47,8 +47,6 @@ test1 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 0x1234", n == 0x1234)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(0x1234 == ack_high); { size_t sz; @@ -110,8 +108,6 @@ test2 (void) assert(("Number of timestamps is 2", acki.n_timestamps == 2)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 4254", n == 4254)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(0x1234 == ack_high); for (n = 0; n < 4; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -163,8 +159,6 @@ test3 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 4", n == 4)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(6 == ack_high); for (n = 0; n < 2; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -215,8 +209,6 @@ test4 (void) assert(("Number of timestamps is 0", acki.n_timestamps == 0)); unsigned n = n_acked(&acki); assert(("Number of acked packets is 2", n == 2)); - lsquic_packno_t ack_high = pf->pf_parse_ack_high(ack_buf, sizeof(ack_buf)); - assert(3 == ack_high); for (n = 0; n < 2; ++n) assert(("Range checks out", ranges[n].high == acki.ranges[n].high @@ -358,11 +350,12 @@ test_max_ack (void) memset(buf, 0xAA, sizeof(buf)); + lsquic_packno_t largest = 0; sz[0] = pf->pf_gen_ack_frame(buf, sizeof(buf), (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(sz[0] > 0); assert(sz[0] <= (int) sizeof(buf)); assert(has_missing); @@ -411,11 +404,12 @@ test_ack_truncation (void) for (bufsz = 200; bufsz < 210; ++bufsz) { memset(buf, 0xAA, sizeof(buf)); + lsquic_packno_t largest = 0; sz[0] = pf->pf_gen_ack_frame(buf, bufsz, (gaf_rechist_first_f) lsquic_rechist_first, (gaf_rechist_next_f) lsquic_rechist_next, (gaf_rechist_largest_recv_f) lsquic_rechist_largest_recv, - &rechist, now, &has_missing); + &rechist, now, &has_missing, &largest); assert(sz[0] > 0); assert(sz[0] <= (int) bufsz); assert(has_missing); diff --git a/test/unittests/test_senhist.c b/test/unittests/test_senhist.c index 1a89531..53766ee 100644 --- a/test/unittests/test_senhist.c +++ b/test/unittests/test_senhist.c @@ -3,9 +3,11 @@ #include #include #include +#include #include "lsquic_int_types.h" #include "lsquic_senhist.h" +#include "lsquic_logger.h" int @@ -13,42 +15,15 @@ main (void) { struct lsquic_senhist hist; lsquic_packno_t packno; - int s; lsquic_senhist_init(&hist); + assert(0 == lsquic_senhist_largest(&hist)); + for (packno = 1; packno < 100; ++packno) lsquic_senhist_add(&hist, packno); - for (packno = 1; packno < 100; ++packno) - { - s = lsquic_senhist_sent_range(&hist, packno, packno); - assert(s); - } - - /* Note break in the sequence at 100 */ - for (packno = 101; packno < 200; ++packno) - lsquic_senhist_add(&hist, packno); - - for (packno = 1; packno < 100; ++packno) - { - s = lsquic_senhist_sent_range(&hist, packno, packno); - assert(s); - } - s = lsquic_senhist_sent_range(&hist, 100, 100); - assert(0 == s); - for (packno = 101; packno < 200; ++packno) - { - s = lsquic_senhist_sent_range(&hist, packno, packno); - assert(s); - } - - s = lsquic_senhist_sent_range(&hist, 1, 99); - assert(s); - s = lsquic_senhist_sent_range(&hist, 101, 199); - assert(s); - s = lsquic_senhist_sent_range(&hist, 1, 199); - assert(0 == s); + assert(99 == lsquic_senhist_largest(&hist)); lsquic_senhist_cleanup(&hist);