diff --git a/CHANGELOG b/CHANGELOG index ddf5317..21e96b4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,11 @@ +2020-01-30 + - 2.10.2 + - [BUGFIX] Do not delay ACKs for Initial and Handshake packets. + - [BUGFIX] Send PATH_CHALLENGE if path changed before mini conn + promotion. + - Logging improvements. + - http_client: discard data faster. + 2020-01-29 - 2.10.1 - [BUGFIX] Coalesced packets could get longer than normal packet diff --git a/include/lsquic.h b/include/lsquic.h index 46939b2..99c864d 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 2 #define LSQUIC_MINOR_VERSION 10 -#define LSQUIC_PATCH_VERSION 1 +#define LSQUIC_PATCH_VERSION 2 /** * Engine flags: diff --git a/src/liblsquic/lsquic_alarmset.h b/src/liblsquic/lsquic_alarmset.h index b00eef7..aaa5c21 100644 --- a/src/liblsquic/lsquic_alarmset.h +++ b/src/liblsquic/lsquic_alarmset.h @@ -27,6 +27,7 @@ enum alarm_id { AL_RETX_APP = AL_RETX_INIT + PNS_APP, AL_PING, AL_IDLE, + /* TODO: remove AL_ACK_INIT and AL_ACK_HSK when LSQVER_ID24 is removed */ AL_ACK_INIT, AL_ACK_HSK = AL_ACK_INIT + PNS_HSK, AL_ACK_APP = AL_ACK_INIT + PNS_APP, diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c index 5c680d6..b71eee7 100644 --- a/src/liblsquic/lsquic_full_conn_ietf.c +++ b/src/liblsquic/lsquic_full_conn_ietf.c @@ -481,10 +481,10 @@ ack_alarm_expired (enum alarm_id al_id, void *ctx, lsquic_time_t expiry, lsquic_time_t now) { struct ietf_full_conn *conn = ctx; - enum packnum_space pns = al_id - AL_ACK_INIT; + assert(al_id == AL_ACK_APP); LSQ_DEBUG("%s ACK timer expired (%"PRIu64" < %"PRIu64"): ACK queued", - lsquic_pns2str[pns], expiry, now); - conn->ifc_flags |= IFC_ACK_QUED_INIT << pns; + lsquic_pns2str[PNS_APP], expiry, now); + conn->ifc_flags |= IFC_ACK_QUED_APP; } @@ -671,7 +671,7 @@ log_scids (const struct ietf_full_conn *conn) if (cce->cce_flags & CCE_REG) flags[fi++] = 'r'; if (cce->cce_flags & CCE_SEQNO) flags[fi++] = 's'; if (cce->cce_flags & CCE_USED) flags[fi++] = 'u'; - flags[fi] = '\0'; + flags[fi] = '\0'; if (lconn->cn_cces_mask & (1 << idx)) { if (cce->cce_flags & CCE_PORT) @@ -1038,8 +1038,6 @@ ietf_full_conn_init (struct ietf_full_conn *conn, lsquic_alarmset_init(&conn->ifc_alset, &conn->ifc_conn); lsquic_alarmset_init_alarm(&conn->ifc_alset, AL_IDLE, idle_alarm_expired, conn); lsquic_alarmset_init_alarm(&conn->ifc_alset, AL_ACK_APP, ack_alarm_expired, conn); - lsquic_alarmset_init_alarm(&conn->ifc_alset, AL_ACK_INIT, ack_alarm_expired, conn); - lsquic_alarmset_init_alarm(&conn->ifc_alset, AL_ACK_HSK, ack_alarm_expired, conn); lsquic_alarmset_init_alarm(&conn->ifc_alset, AL_PING, ping_alarm_expired, conn); lsquic_alarmset_init_alarm(&conn->ifc_alset, AL_HANDSHAKE, handshake_alarm_expired, conn); lsquic_alarmset_init_alarm(&conn->ifc_alset, AL_CID_THROT, cid_throt_alarm_expired, conn); @@ -1279,6 +1277,11 @@ lsquic_ietf_full_conn_server_new (struct lsquic_engine_public *enpub, conn->ifc_paths[0].cop_path = imc->imc_path; conn->ifc_paths[0].cop_flags = COP_VALIDATED; conn->ifc_used_paths = 1 << 0; + if (imc->imc_flags & IMC_PATH_CHANGED) + { + LSQ_DEBUG("path changed during mini conn: schedule PATH_CHALLENGE"); + conn->ifc_send_flags |= SF_SEND_PATH_CHAL_PATH_0; + } #ifndef NDEBUG if (getenv("LSQUIC_CN_PACK_SIZE")) { @@ -1524,10 +1527,14 @@ generate_ack_frame_for_pns (struct ietf_full_conn *conn, } conn->ifc_n_slack_akbl[pns] = 0; - if (PNS_APP == pns) - conn->ifc_n_slack_all = 0; conn->ifc_flags &= ~(IFC_ACK_QUED_INIT << pns); - lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_INIT + pns); + if (pns == PNS_APP) + { + conn->ifc_n_slack_all = 0; + lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_APP); + } + else + assert(!lsquic_alarmset_is_set(&conn->ifc_alset, AL_ACK_INIT + pns)); lsquic_send_ctl_sanity_check(&conn->ifc_send_ctl); LSQ_DEBUG("%s ACK state reset", lsquic_pns2str[pns]); @@ -1968,6 +1975,8 @@ generate_max_stream_data_frame (struct ietf_full_conn *conn, ABORT_ERROR("Generating MAX_STREAM_DATA frame failed"); return 0; } + EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "generated %d-byte MAX_STREAM_DATA " + "frame; stream_id: %"PRIu64"; offset: %"PRIu64, sz, stream->id, off); lsquic_send_ctl_incr_pack_sz(&conn->ifc_send_ctl, packet_out, sz); packet_out->po_frame_types |= 1 << QUIC_FRAME_MAX_STREAM_DATA; lsquic_stream_max_stream_data_sent(stream); @@ -1998,6 +2007,8 @@ generate_stream_blocked_frame (struct ietf_full_conn *conn, ABORT_ERROR("Generating STREAM_BLOCKED frame failed"); return 0; } + EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "generated %d-byte STREAM_BLOCKED " + "frame; stream_id: %"PRIu64"; offset: %"PRIu64, sz, stream->id, off); lsquic_send_ctl_incr_pack_sz(&conn->ifc_send_ctl, packet_out, sz); packet_out->po_frame_types |= 1 << QUIC_FRAME_STREAM_BLOCKED; lsquic_stream_blocked_frame_sent(stream); @@ -4860,6 +4871,8 @@ process_max_data_frame (struct ietf_full_conn *conn, if (parsed_len < 0) return 0; + EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "MAX_DATA frame in; offset: %"PRIu64, + max_data); if (max_data > conn->ifc_pub.conn_cap.cc_max) { LSQ_DEBUG("max data goes from %"PRIu64" to %"PRIu64, @@ -4888,6 +4901,8 @@ process_max_stream_data_frame (struct ietf_full_conn *conn, if (parsed_len < 0) return 0; + EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "MAX_STREAM_DATA frame in; " + "stream_id: %"PRIu64"; offset: %"PRIu64, stream_id, max_data); if (conn_is_receive_only_stream(conn, stream_id)) { ABORT_QUIETLY(0, TEC_STREAM_STATE_ERROR, @@ -5585,26 +5600,26 @@ many_in_and_will_write (struct ietf_full_conn *conn) static void -try_queueing_ack (struct ietf_full_conn *conn, enum packnum_space pns, +try_queueing_ack_app (struct ietf_full_conn *conn, int was_missing, lsquic_time_t now) { lsquic_time_t srtt, ack_timeout; - if (conn->ifc_n_slack_akbl[pns] >= MAX_RETR_PACKETS_SINCE_LAST_ACK + if (conn->ifc_n_slack_akbl[PNS_APP] >= MAX_RETR_PACKETS_SINCE_LAST_ACK || ((conn->ifc_flags & IFC_ACK_HAD_MISS) - && was_missing && conn->ifc_n_slack_akbl[pns] > 0) - || (PNS_APP == pns && many_in_and_will_write(conn))) + && was_missing && conn->ifc_n_slack_akbl[PNS_APP] > 0) + || many_in_and_will_write(conn)) { - lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_INIT + pns); + lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_APP); lsquic_send_ctl_sanity_check(&conn->ifc_send_ctl); - conn->ifc_flags |= IFC_ACK_QUED_INIT << pns; + conn->ifc_flags |= IFC_ACK_QUED_APP; LSQ_DEBUG("%s ACK queued: ackable: %u; all: %u; had_miss: %d; " "was_missing: %d", - lsquic_pns2str[pns], conn->ifc_n_slack_akbl[pns], + lsquic_pns2str[PNS_APP], conn->ifc_n_slack_akbl[PNS_APP], conn->ifc_n_slack_all, !!(conn->ifc_flags & IFC_ACK_HAD_MISS), was_missing); } - else if (conn->ifc_n_slack_akbl[pns] > 0) + else if (conn->ifc_n_slack_akbl[PNS_APP] > 0) { /* See https://github.com/quicwg/base-drafts/issues/3304 for more */ srtt = lsquic_rtt_stats_get_srtt(&conn->ifc_pub.rtt_stats); @@ -5612,14 +5627,38 @@ try_queueing_ack (struct ietf_full_conn *conn, enum packnum_space pns, ack_timeout = MAX(1000, MIN(ACK_TIMEOUT, srtt / 4)); else ack_timeout = ACK_TIMEOUT; - lsquic_alarmset_set(&conn->ifc_alset, AL_ACK_INIT + pns, + lsquic_alarmset_set(&conn->ifc_alset, AL_ACK_APP, now + ack_timeout); - LSQ_DEBUG("%s ACK alarm set to %"PRIu64, lsquic_pns2str[pns], + LSQ_DEBUG("%s ACK alarm set to %"PRIu64, lsquic_pns2str[PNS_APP], now + ack_timeout); } } +static void +try_queueing_ack_init_or_hsk (struct ietf_full_conn *conn, + enum packnum_space pns) +{ + if (conn->ifc_n_slack_akbl[pns] > 0) + { + conn->ifc_flags |= IFC_ACK_QUED_INIT << pns; + LSQ_DEBUG("%s ACK queued: ackable: %u", + lsquic_pns2str[pns], conn->ifc_n_slack_akbl[pns]); + } +} + + +static void +try_queueing_ack (struct ietf_full_conn *conn, enum packnum_space pns, + int was_missing, lsquic_time_t now) +{ + if (PNS_APP == pns) + try_queueing_ack_app(conn, was_missing, now); + else + try_queueing_ack_init_or_hsk(conn, pns); +} + + static int maybe_queue_opp_ack (struct ietf_full_conn *conn) { @@ -5736,9 +5775,6 @@ process_retry_packet (struct ietf_full_conn *conn, lsquic_alarmset_unset(&conn->ifc_alset, AL_RETX_INIT); lsquic_alarmset_unset(&conn->ifc_alset, AL_RETX_HSK); lsquic_alarmset_unset(&conn->ifc_alset, AL_RETX_APP); - lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_INIT); - lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_HSK); - lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_APP); LSQ_INFO("Received a retry packet. Will retry."); conn->ifc_flags |= IFC_RETRIED; @@ -5821,7 +5857,6 @@ ignore_init (struct ietf_full_conn *conn) LSQ_DEBUG("henceforth, no Initial packets shall be sent or received"); conn->ifc_flags |= IFC_IGNORE_INIT; conn->ifc_flags &= ~(IFC_ACK_QUED_INIT << PNS_INIT); - lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_INIT + PNS_INIT); lsquic_send_ctl_empty_pns(&conn->ifc_send_ctl, PNS_INIT); lsquic_rechist_cleanup(&conn->ifc_rechist[PNS_INIT]); if (!(conn->ifc_flags & IFC_SERVER)) @@ -5842,7 +5877,6 @@ ignore_hsk (struct ietf_full_conn *conn) LSQ_DEBUG("henceforth, no Handshake packets shall be sent or received"); conn->ifc_flags |= IFC_IGNORE_HSK; conn->ifc_flags &= ~(IFC_ACK_QUED_INIT << PNS_HSK); - lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_HSK); lsquic_send_ctl_empty_pns(&conn->ifc_send_ctl, PNS_HSK); lsquic_rechist_cleanup(&conn->ifc_rechist[PNS_HSK]); if (!(conn->ifc_flags & IFC_SERVER)) @@ -5854,28 +5888,6 @@ ignore_hsk (struct ietf_full_conn *conn) } -/* Returns true if socket addresses are equal, false otherwise. Only - * families, IP addresses, and ports are compared. - */ -static int -sockaddr_eq (const struct sockaddr *a, const struct sockaddr *b) -{ - if (a->sa_family == AF_INET) - return a->sa_family == b->sa_family - && ((struct sockaddr_in *) a)->sin_addr.s_addr - == ((struct sockaddr_in *) b)->sin_addr.s_addr - && ((struct sockaddr_in *) a)->sin_port - == ((struct sockaddr_in *) b)->sin_port; - else - return a->sa_family == b->sa_family - && ((struct sockaddr_in6 *) a)->sin6_port == - ((struct sockaddr_in6 *) b)->sin6_port - && 0 == memcmp(&((struct sockaddr_in6 *) a)->sin6_addr, - &((struct sockaddr_in6 *) b)->sin6_addr, - sizeof(((struct sockaddr_in6 *) b)->sin6_addr)); -} - - static void record_dcid (struct ietf_full_conn *conn, const struct lsquic_packet_in *packet_in) @@ -5929,12 +5941,12 @@ process_regular_packet (struct ietf_full_conn *conn, NP_IS_IPv6(&conn->ifc_paths[packet_in->pi_path_id].cop_path)) { case (1 << 1) | 1: /* IPv6 */ - if (sockaddr_eq(NP_PEER_SA(CUR_NPATH(conn)), NP_PEER_SA( + if (lsquic_sockaddr_eq(NP_PEER_SA(CUR_NPATH(conn)), NP_PEER_SA( &conn->ifc_paths[packet_in->pi_path_id].cop_path))) goto known_peer_addr; break; case (0 << 1) | 0: /* IPv4 */ - if (sockaddr_eq(NP_PEER_SA(CUR_NPATH(conn)), NP_PEER_SA( + if (lsquic_sockaddr_eq(NP_PEER_SA(CUR_NPATH(conn)), NP_PEER_SA( &conn->ifc_paths[packet_in->pi_path_id].cop_path))) goto known_peer_addr; break; @@ -6710,7 +6722,7 @@ static int path_matches_local_sa (const struct network_path *path, const struct sockaddr *local_sa) { - return sockaddr_eq(NP_LOCAL_SA(path), local_sa); + return lsquic_sockaddr_eq(NP_LOCAL_SA(path), local_sa); } @@ -6758,8 +6770,8 @@ path_matches (const struct network_path *path, const struct sockaddr *local_sa, const struct sockaddr *peer_sa) { return local_sa->sa_family == NP_LOCAL_SA(path)->sa_family - && sockaddr_eq(local_sa, NP_LOCAL_SA(path)) - && sockaddr_eq(peer_sa, NP_PEER_SA(path)); + && lsquic_sockaddr_eq(local_sa, NP_LOCAL_SA(path)) + && lsquic_sockaddr_eq(peer_sa, NP_PEER_SA(path)); } diff --git a/src/liblsquic/lsquic_mini_conn_ietf.c b/src/liblsquic/lsquic_mini_conn_ietf.c index e7a2a75..5533b94 100644 --- a/src/liblsquic/lsquic_mini_conn_ietf.c +++ b/src/liblsquic/lsquic_mini_conn_ietf.c @@ -1766,6 +1766,7 @@ ietf_mini_conn_ci_record_addrs (struct lsquic_conn *lconn, void *peer_ctx, const struct sockaddr *local_sa, const struct sockaddr *peer_sa) { struct ietf_mini_conn *conn = (struct ietf_mini_conn *) lconn; + const struct sockaddr *orig_peer_sa; struct lsquic_packet_out *packet_out; size_t len; @@ -1774,6 +1775,12 @@ ietf_mini_conn_ci_record_addrs (struct lsquic_conn *lconn, void *peer_ctx, if ((packet_out->po_flags & (PO_SENT|PO_ENCRYPTED)) == PO_ENCRYPTED) imico_return_enc_data(conn, packet_out); + orig_peer_sa = NP_PEER_SA(&conn->imc_path); + if (orig_peer_sa->sa_family != 0 + && !(lsquic_sockaddr_eq(NP_PEER_SA(&conn->imc_path), peer_sa) + && lsquic_sockaddr_eq(NP_LOCAL_SA(&conn->imc_path), local_sa))) + conn->imc_flags |= IMC_PATH_CHANGED; + len = local_sa->sa_family == AF_INET ? sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6); diff --git a/src/liblsquic/lsquic_mini_conn_ietf.h b/src/liblsquic/lsquic_mini_conn_ietf.h index 0852efe..d9ec65e 100644 --- a/src/liblsquic/lsquic_mini_conn_ietf.h +++ b/src/liblsquic/lsquic_mini_conn_ietf.h @@ -56,6 +56,7 @@ struct ietf_mini_conn IMC_HSK_PACKET_SENT = 1 << 18, IMC_CLOSE_RECVD = 1 << 19, IMC_PARSE_FAILED = 1 << 20, + IMC_PATH_CHANGED = 1 << 21, } imc_flags; struct mini_crypto_stream imc_streams[N_ENC_LEVS]; void *imc_stream_ps[N_ENC_LEVS]; diff --git a/src/liblsquic/lsquic_send_ctl.c b/src/liblsquic/lsquic_send_ctl.c index 5f5bca1..876fa3a 100644 --- a/src/liblsquic/lsquic_send_ctl.c +++ b/src/liblsquic/lsquic_send_ctl.c @@ -1498,6 +1498,7 @@ lsquic_send_ctl_expire_all (lsquic_send_ctl_t *ctl) } +#ifndef NDEBUG void lsquic_send_ctl_do_sanity_check (const struct lsquic_send_ctl *ctl) { @@ -1540,6 +1541,7 @@ lsquic_send_ctl_do_sanity_check (const struct lsquic_send_ctl *ctl) assert(count == ctl->sc_n_scheduled); assert(bytes == ctl->sc_bytes_scheduled); } +#endif void diff --git a/src/liblsquic/lsquic_send_ctl.h b/src/liblsquic/lsquic_send_ctl.h index b45fdd0..b2fe6c4 100644 --- a/src/liblsquic/lsquic_send_ctl.h +++ b/src/liblsquic/lsquic_send_ctl.h @@ -180,10 +180,14 @@ lsquic_send_ctl_expire_all (lsquic_send_ctl_t *ctl); void lsquic_send_ctl_do_sanity_check (const struct lsquic_send_ctl *ctl); +#ifndef NDEBUG #define lsquic_send_ctl_sanity_check(ctl) do { \ if ((ctl)->sc_flags & SC_SANITY_CHECK) \ lsquic_send_ctl_do_sanity_check(ctl); \ } while (0) +#else +#define lsquic_send_ctl_sanity_check(ctl) +#endif int lsquic_send_ctl_have_outgoing_stream_frames (const lsquic_send_ctl_t *); diff --git a/src/liblsquic/lsquic_trans_params.c b/src/liblsquic/lsquic_trans_params.c index 8edf512..bfa9e1f 100644 --- a/src/liblsquic/lsquic_trans_params.c +++ b/src/liblsquic/lsquic_trans_params.c @@ -128,7 +128,7 @@ lsquic_tp_encode (const struct transport_params *params, else { LSQ_DEBUG("numeric value is too large (%"PRIu64" vs maximum " - "of %"PRIu64, params->tp_numerics_u.a[tpi2idx[tpi]], + "of %"PRIu64")", params->tp_numerics_u.a[tpi2idx[tpi]], max_vals[tpi]); return -1; } diff --git a/src/liblsquic/lsquic_util.c b/src/liblsquic/lsquic_util.c index 7c099a4..cc8c9fc 100644 --- a/src/liblsquic/lsquic_util.c +++ b/src/liblsquic/lsquic_util.c @@ -14,6 +14,7 @@ #else #include #endif +#include #if !(defined(_POSIX_TIMERS) && _POSIX_TIMERS > 0) && defined(__APPLE__) #include @@ -254,3 +255,25 @@ lsquic_hexdump (const void *src_void, size_t src_sz, char *out, size_t out_sz) return out + out_sz - out_end; } + + +/* Returns true if socket addresses are equal, false otherwise. Only + * families, IP addresses, and ports are compared. + */ +int +lsquic_sockaddr_eq (const struct sockaddr *a, const struct sockaddr *b) +{ + if (a->sa_family == AF_INET) + return a->sa_family == b->sa_family + && ((struct sockaddr_in *) a)->sin_addr.s_addr + == ((struct sockaddr_in *) b)->sin_addr.s_addr + && ((struct sockaddr_in *) a)->sin_port + == ((struct sockaddr_in *) b)->sin_port; + else + return a->sa_family == b->sa_family + && ((struct sockaddr_in6 *) a)->sin6_port == + ((struct sockaddr_in6 *) b)->sin6_port + && 0 == memcmp(&((struct sockaddr_in6 *) a)->sin6_addr, + &((struct sockaddr_in6 *) b)->sin6_addr, + sizeof(((struct sockaddr_in6 *) b)->sin6_addr)); +} diff --git a/src/liblsquic/lsquic_util.h b/src/liblsquic/lsquic_util.h index c4b21f0..555579e 100644 --- a/src/liblsquic/lsquic_util.h +++ b/src/liblsquic/lsquic_util.h @@ -10,6 +10,8 @@ extern "C" { #endif +struct sockaddr; + lsquic_time_t lsquic_time_now (void); @@ -38,6 +40,9 @@ lsquic_hexstr (const unsigned char *buf, size_t bufsz, char *out, size_t outsz); #define HEXSTR(buf, bufsz, out) \ (lsquic_hexstr(buf, bufsz, out, sizeof(out)), out) +int +lsquic_sockaddr_eq (const struct sockaddr *a, const struct sockaddr *b); + #ifdef __cplusplus } #endif diff --git a/test/http_client.c b/test/http_client.c index 1cffc09..8ef47b9 100644 --- a/test/http_client.c +++ b/test/http_client.c @@ -659,6 +659,13 @@ http_client_on_write (lsquic_stream_t *stream, lsquic_stream_ctx_t *st_h) } +static size_t +discard (void *ctx, const unsigned char *buf, size_t sz, int fin) +{ + return sz; +} + + static void http_client_on_read (lsquic_stream_t *stream, lsquic_stream_ctx_t *st_h) { @@ -691,7 +698,10 @@ http_client_on_read (lsquic_stream_t *stream, lsquic_stream_ctx_t *st_h) hset_destroy(hset); st_h->sh_flags |= PROCESSED_HEADERS; } - else if (nread = lsquic_stream_read(stream, buf, sizeof(buf)), nread > 0) + else if (nread = (s_discard_response + ? lsquic_stream_readf(stream, discard, NULL) + : lsquic_stream_read(stream, buf, sizeof(buf))), + nread > 0) { s_stat_downloaded_bytes += nread; /* test stream_reset after some number of read bytes */