From 7ee4152504cba5b0a4b79ed9eab2cf8134128975 Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Wed, 27 Nov 2019 15:24:18 -0500 Subject: [PATCH] Release 2.7.0 - [API, FEATURE] Close connection immediately when ea_packets_out() fails with errno != EAGAIN. The API change is that errno is now examined. Make sure to set it if using something other than sendmsg() to send packets. - [CLEANUP] Immediate close logic in IETF full conn. - [CLEANUP] Fix bogus warning about uninitialized `pair' variable. --- APIs.txt | 4 +- CHANGELOG | 9 ++ include/lsquic.h | 18 ++-- src/liblsquic/lsquic_conn.h | 6 +- src/liblsquic/lsquic_enc_sess_ietf.c | 9 +- src/liblsquic/lsquic_engine.c | 131 +++++++++++++++++++++----- src/liblsquic/lsquic_full_conn_ietf.c | 7 +- src/liblsquic/lsquic_pr_queue.c | 46 +++++++-- src/liblsquic/lsquic_pr_queue.h | 3 + 9 files changed, 179 insertions(+), 54 deletions(-) diff --git a/APIs.txt b/APIs.txt index c5b8878..d3a5e2b 100644 --- a/APIs.txt +++ b/APIs.txt @@ -83,7 +83,7 @@ The streams have four callbacks: on_close Stream has been closed. For both connections and streams, the "on new" callback return value can -be use to specify user-supplied data. This data pointer is optional and +be used to specify user-supplied data. This data pointer is optional and can be NULL. It can also refer to the same data for the connection and its streams. "on close" callbacks should be used to free user-supplied data. @@ -133,7 +133,7 @@ drive QUIC connections: Connection Management --------------------- -A connections needs to be processed once in a while. It needs to be +A connection needs to be processed once in a while. It needs to be processed when one of the following is true: - There are incoming packets; diff --git a/CHANGELOG b/CHANGELOG index efb5757..e4f8463 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,12 @@ +2019-11-27 + - 2.7.0 + - [API, FEATURE] Close connection immediately when ea_packets_out() + fails with errno != EAGAIN. The API change is that errno is now + examined. Make sure to set it if using something other than + sendmsg() to send packets. + - [CLEANUP] Immediate close logic in IETF full conn. + - [CLEANUP] Fix bogus warning about uninitialized `pair' variable. + 2019-11-22 - 2.6.7 - [FEATURE] Implement the QL extension (offered by default). diff --git a/include/lsquic.h b/include/lsquic.h index 9f82f5a..d7d9052 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -24,8 +24,8 @@ extern "C" { #endif #define LSQUIC_MAJOR_VERSION 2 -#define LSQUIC_MINOR_VERSION 6 -#define LSQUIC_PATCH_VERSION 7 +#define LSQUIC_MINOR_VERSION 7 +#define LSQUIC_PATCH_VERSION 0 /** * Engine flags: @@ -788,8 +788,14 @@ struct lsquic_out_spec * Returns number of packets successfully sent out or -1 on error. -1 should * only be returned if no packets were sent out. If -1 is returned or if the * return value is smaller than `n_packets_out', this indicates that sending - * of packets is not possible No packets will be attempted to be sent out - * until @ref lsquic_engine_send_unsent_packets() is called. + * of packets is not possible. + * + * If not all packets could be sent out, errno is examined. If it is not + * EAGAIN or EWOULDBLOCK, the connection whose packet cause the error is + * closed forthwith. + * + * No packets will be attempted to be sent out until + * @ref lsquic_engine_send_unsent_packets() is called. */ typedef int (*lsquic_packets_out_f)( void *packets_out_ctx, @@ -1282,7 +1288,7 @@ int lsquic_stream_close(lsquic_stream_t *s); /** * Get certificate chain returned by the server. This can be used for - * server certificate verifiction. + * server certificate verification. * * If server certificate cannot be verified, the connection can be closed * using lsquic_conn_cert_verification_failed(). @@ -1523,7 +1529,7 @@ enum lsquic_version lsquic_alpn2ver (const char *alpn, size_t len); /** - * This function closes all mini connections and marks all full connection + * This function closes all mini connections and marks all full connections * as going away. In server mode, this also causes the engine to stop * creating new connections. */ diff --git a/src/liblsquic/lsquic_conn.h b/src/liblsquic/lsquic_conn.h index 78f4f16..93c3e94 100644 --- a/src/liblsquic/lsquic_conn.h +++ b/src/liblsquic/lsquic_conn.h @@ -35,7 +35,7 @@ enum lsquic_conn_flags { LSCONN_HAS_OUTGOING = (1 << 1), LSCONN_HASHED = (1 << 2), LSCONN_MINI = (1 << 3), /* This is a mini connection */ - LSCONN_UNUSED_4 = (1 << 4), + LSCONN_IMMED_CLOSE = (1 << 4), LSCONN_UNUSED_5 = (1 << 5), LSCONN_HANDSHAKE_DONE = (1 << 6), LSCONN_CLOSING = (1 << 7), @@ -288,9 +288,7 @@ struct lsquic_conn STAILQ_ENTRY(lsquic_conn) cn_next_new_full; TAILQ_ENTRY(lsquic_conn) cn_next_ticked; TAILQ_ENTRY(lsquic_conn) cn_next_out; - TAILQ_ENTRY(lsquic_conn) cn_next_susp_cert; - /* Reuse this entry, as evanecsent connections are never suspended: */ -#define cn_next_pr cn_next_susp_cert + TAILQ_ENTRY(lsquic_conn) cn_next_pr; const struct conn_iface *cn_if; const struct parse_funcs *cn_pf; struct attq_elem *cn_attq_elem; diff --git a/src/liblsquic/lsquic_enc_sess_ietf.c b/src/liblsquic/lsquic_enc_sess_ietf.c index d581edb..6dfaa83 100644 --- a/src/liblsquic/lsquic_enc_sess_ietf.c +++ b/src/liblsquic/lsquic_enc_sess_ietf.c @@ -1894,11 +1894,9 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, if (enc_level == ENC_LEV_FORW) { key_phase = (dst[0] & 0x04) > 0; + pair = &enc_sess->esi_pairs[ key_phase ]; if (key_phase == enc_sess->esi_key_phase) - { - pair = &enc_sess->esi_pairs[ key_phase ]; crypto_ctx = &pair->ykp_ctx[ cliser ]; - } else if (!is_valid_packno( enc_sess->esi_pairs[enc_sess->esi_key_phase].ykp_thresh) || packet_in->pi_packno @@ -1929,7 +1927,6 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, } else { - pair = &enc_sess->esi_pairs[ key_phase ]; crypto_ctx = &pair->ykp_ctx[ cliser ]; if (UNLIKELY(0 == (crypto_ctx->yk_flags & YK_INITED))) { @@ -2014,7 +2011,6 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, LSQ_DEBUG("decryption in the new key phase %u successful, rotate " "keys", key_phase); const struct ku_label kl = select_ku_label(enc_sess); - pair = &enc_sess->esi_pairs[ key_phase ]; pair->ykp_thresh = packet_in->pi_packno; pair->ykp_ctx[ cliser ] = crypto_ctx_buf; memcpy(enc_sess->esi_traffic_secrets[ cliser ], new_secret, @@ -2052,9 +2048,6 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, pns = lsquic_enclev2pns[enc_level]; if (packet_in->pi_packno > enc_sess->esi_max_packno[pns]) enc_sess->esi_max_packno[pns] = packet_in->pi_packno; - /* XXX Compiler complains that `pair' may be uninitialized here, but this - * variable is set in `if (crypto_ctx == &crypto_ctx_buf)' above. - */ if (is_valid_packno(pair->ykp_thresh) && packet_in->pi_packno > pair->ykp_thresh) pair->ykp_thresh = packet_in->pi_packno; diff --git a/src/liblsquic/lsquic_engine.c b/src/liblsquic/lsquic_engine.c index f9c12ed..56e5a6b 100644 --- a/src/liblsquic/lsquic_engine.c +++ b/src/liblsquic/lsquic_engine.c @@ -13,6 +13,7 @@ #include #include #include +#include #ifndef WIN32 #include #include @@ -714,7 +715,8 @@ destroy_conn (struct lsquic_engine *engine, struct lsquic_conn *conn, && (conn->cn_flags & (LSCONN_MINI|LSCONN_PROMOTED)) != (LSCONN_MINI|LSCONN_PROMOTED)) { - if (conn->cn_if->ci_drain_time && + if (!(conn->cn_flags & LSCONN_IMMED_CLOSE) + && conn->cn_if->ci_drain_time && (drain_time = conn->cn_if->ci_drain_time(conn), drain_time)) { for (cce = cce_iter_first(&citer, conn); cce; @@ -1942,7 +1944,8 @@ coi_reheap (struct conns_out_iter *iter, lsquic_engine_t *engine) { TAILQ_REMOVE(&iter->coi_active_list, conn, cn_next_out); conn->cn_flags &= ~LSCONN_COI_ACTIVE; - if ((conn->cn_flags & CONN_REF_FLAGS) != LSCONN_HAS_OUTGOING) + if ((conn->cn_flags & CONN_REF_FLAGS) != LSCONN_HAS_OUTGOING + && !(conn->cn_flags & LSCONN_IMMED_CLOSE)) lsquic_mh_insert(iter->coi_heap, conn, conn->cn_last_sent); else /* Closed connection gets one shot at sending packets */ (void) engine_decref_conn(engine, conn, LSCONN_HAS_OUTGOING); @@ -1992,14 +1995,99 @@ lose_matching_packets (const lsquic_engine_t *engine, struct out_batch *batch, #define CONST_BATCH #endif -static unsigned -send_batch (lsquic_engine_t *engine, struct conns_out_iter *conns_iter, - CONST_BATCH struct out_batch *batch, unsigned n_to_send) + +static void +sockaddr2str (const struct sockaddr *addr, char *buf, size_t sz) { - int n_sent, i; + unsigned short port; + int len; + + switch (addr->sa_family) + { + case AF_INET: + port = ((struct sockaddr_in *) addr)->sin_port; + if (!inet_ntop(AF_INET, &((struct sockaddr_in *) addr)->sin_addr, + buf, sz)) + buf[0] = '\0'; + break; + case AF_INET6: + port = ((struct sockaddr_in6 *) addr)->sin6_port; + if (!inet_ntop(AF_INET6, &((struct sockaddr_in6 *) addr)->sin6_addr, + buf, sz)) + buf[0] = '\0'; + break; + default: + port = 0; + (void) snprintf(buf, sz, "", addr->sa_family); + break; + } + + len = strlen(buf); + if (len < (int) sz) + snprintf(buf + len, sz - (size_t) len, ":%hu", port); +} + + +struct send_batch_ctx { + struct conns_stailq *closed_conns; + struct conns_tailq *ticked_conns; + struct conns_out_iter *conns_iter; + CONST_BATCH struct out_batch *batch; +}; + + +static void +close_conn_immediately (struct lsquic_engine *engine, + const struct send_batch_ctx *sb_ctx, struct lsquic_conn *conn) +{ + conn->cn_flags |= LSCONN_IMMED_CLOSE; + if (!(conn->cn_flags & LSCONN_CLOSING)) + { + STAILQ_INSERT_TAIL(sb_ctx->closed_conns, conn, cn_next_closed_conn); + engine_incref_conn(conn, LSCONN_CLOSING); + if (conn->cn_flags & LSCONN_HASHED) + remove_conn_from_hash(engine, conn); + } + if (conn->cn_flags & LSCONN_TICKED) + { + TAILQ_REMOVE(sb_ctx->ticked_conns, conn, cn_next_ticked); + engine_decref_conn(engine, conn, LSCONN_TICKED); + } +} + + +static void +close_conn_on_send_error (struct lsquic_engine *engine, + const struct send_batch_ctx *sb_ctx, int n, int e_val) +{ + const struct out_batch *batch = sb_ctx->batch; + struct lsquic_conn *const conn = batch->conns[n]; + char buf[2][INET6_ADDRSTRLEN + sizeof(":65535")]; + + LSQ_WARNC("error sending packet for %s connection %"CID_FMT" - close it; " + "src: %s; dst: %s; errno: %d", + conn->cn_flags & LSCONN_EVANESCENT ? "evanecsent" : + conn->cn_flags & LSCONN_MINI ? "mini" : "regular", + CID_BITS(lsquic_conn_log_cid(conn)), + (sockaddr2str(batch->outs[n].local_sa, buf[0], sizeof(buf[0])), buf[0]), + (sockaddr2str(batch->outs[n].dest_sa, buf[1], sizeof(buf[1])), buf[1]), + e_val); + if (conn->cn_flags & LSCONN_EVANESCENT) + lsquic_prq_drop(conn); + else + close_conn_immediately(engine, sb_ctx, conn); +} + + +static unsigned +send_batch (lsquic_engine_t *engine, const struct send_batch_ctx *sb_ctx, + unsigned n_to_send) +{ + int n_sent, i, e_val; lsquic_time_t now; unsigned off; size_t count; + CONST_BATCH struct out_batch *const batch = sb_ctx->batch; struct lsquic_packet_out *CONST_BATCH *packet_out, *CONST_BATCH *end; #ifndef NDEBUG @@ -2021,18 +2109,22 @@ send_batch (lsquic_engine_t *engine, struct conns_out_iter *conns_iter, } n_sent = engine->packets_out(engine->packets_out_ctx, batch->outs, n_to_send); + e_val = errno; if (n_sent < (int) n_to_send) { engine->pub.enp_flags &= ~ENPUB_CAN_SEND; engine->resume_sending_at = now + 1000000; LSQ_DEBUG("cannot send packets"); EV_LOG_GENERIC_EVENT("cannot send packets"); + if (!(EAGAIN == e_val || EWOULDBLOCK == e_val)) + close_conn_on_send_error(engine, sb_ctx, + n_sent < 0 ? 0 : n_sent, e_val); } if (n_sent >= 0) LSQ_DEBUG("packets out returned %d (out of %u)", n_sent, n_to_send); else { - LSQ_DEBUG("packets out returned an error: %s", strerror(errno)); + LSQ_DEBUG("packets out returned an error: %s", strerror(e_val)); n_sent = 0; } if (n_sent > 0) @@ -2097,7 +2189,7 @@ send_batch (lsquic_engine_t *engine, struct conns_out_iter *conns_iter, *packet_out); while (--packet_out > end); if (!(batch->conns[i]->cn_flags & (LSCONN_COI_ACTIVE|LSCONN_EVANESCENT))) - coi_reactivate(conns_iter, batch->conns[i]); + coi_reactivate(sb_ctx->conns_iter, batch->conns[i]); } return n_sent; } @@ -2153,6 +2245,12 @@ send_packets_out (struct lsquic_engine *engine, struct iovec *iov, *packet_iov; struct conns_out_iter conns_iter; int shrink, deadline_exceeded; + const struct send_batch_ctx sb_ctx = { + closed_conns, + ticked_conns, + &conns_iter, + &engine->out_batch, + }; coi_init(&conns_iter, engine); n_batches_sent = 0; @@ -2192,19 +2290,8 @@ send_packets_out (struct lsquic_engine *engine, CID_BITS(lsquic_conn_log_cid(conn))); if (!(conn->cn_flags & LSCONN_EVANESCENT)) { - if (!(conn->cn_flags & LSCONN_CLOSING)) - { - STAILQ_INSERT_TAIL(closed_conns, conn, cn_next_closed_conn); - engine_incref_conn(conn, LSCONN_CLOSING); - if (conn->cn_flags & LSCONN_HASHED) - remove_conn_from_hash(engine, conn); - } + close_conn_immediately(engine, &sb_ctx, conn); coi_deactivate(&conns_iter, conn); - if (conn->cn_flags & LSCONN_TICKED) - { - TAILQ_REMOVE(ticked_conns, conn, cn_next_ticked); - engine_decref_conn(engine, conn, LSCONN_TICKED); - } } continue; case ENCPA_OK: @@ -2263,7 +2350,7 @@ send_packets_out (struct lsquic_engine *engine, if (n == engine->batch_size || iov >= batch->iov + sizeof(batch->iov) / sizeof(batch->iov[0])) { - w = send_batch(engine, &conns_iter, batch, n); + w = send_batch(engine, &sb_ctx, n); n = 0; iov = batch->iov; packet = batch->packets; @@ -2283,7 +2370,7 @@ send_packets_out (struct lsquic_engine *engine, end_for: if (n > 0) { - w = send_batch(engine, &conns_iter, batch, n); + w = send_batch(engine, &sb_ctx, n); n_sent += w; shrink = w < n; ++n_batches_sent; diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c index ef3f27b..8844eb4 100644 --- a/src/liblsquic/lsquic_full_conn_ietf.c +++ b/src/liblsquic/lsquic_full_conn_ietf.c @@ -3328,7 +3328,7 @@ immediate_close (struct ietf_full_conn *conn) return TICK_CLOSE; } - assert(conn->ifc_flags & (IFC_ERROR|IFC_ABORTED|IFC_TIMED_OUT|IFC_HSK_FAILED)); + assert(conn->ifc_flags & (IFC_ERROR|IFC_ABORTED|IFC_HSK_FAILED)); if (conn->ifc_error.u.err != 0) { conn_err = conn->ifc_error; @@ -3344,11 +3344,6 @@ immediate_close (struct ietf_full_conn *conn) conn_err = CONN_ERR(0, TEC_NO_ERROR); error_reason = "user aborted connection"; } - else if (conn->ifc_flags & IFC_TIMED_OUT) - { - conn_err = CONN_ERR(0, TEC_NO_ERROR); - error_reason = "connection timed out"; - } else if (conn->ifc_flags & IFC_HSK_FAILED) { conn_err = CONN_ERR(0, TEC_NO_ERROR); diff --git a/src/liblsquic/lsquic_pr_queue.c b/src/liblsquic/lsquic_pr_queue.c index ee50fae..5e7ee9a 100644 --- a/src/liblsquic/lsquic_pr_queue.c +++ b/src/liblsquic/lsquic_pr_queue.c @@ -68,6 +68,9 @@ struct evanescent_conn struct pr_queue *evc_queue; struct lsquic_packet_out evc_packet_out; struct conn_cid_elem evc_cces[1]; + enum { + EVC_DROP = 1 << 0, + } evc_flags; unsigned char evc_buf[0]; }; @@ -398,7 +401,9 @@ get_evconn (struct pr_queue *prq) if (lconn) { TAILQ_REMOVE(&prq->prq_free_conns, lconn, cn_next_pr); - return (struct evanescent_conn *) lconn; + evconn = (struct evanescent_conn *) lconn; + evconn->evc_flags = 0; + return evconn; } bufsz = max_bufsz(prq); @@ -549,6 +554,17 @@ evanescent_conn_ci_next_packet_to_send (struct lsquic_conn *lconn, size_t size) } +static void +prq_free_conn (struct pr_queue *prq, struct lsquic_conn *lconn) +{ + struct evanescent_conn *const evconn = (struct evanescent_conn *) lconn; + + TAILQ_INSERT_HEAD(&prq->prq_free_conns, lconn, cn_next_pr); + put_req(prq, evconn->evc_req); + --prq->prq_nconns; +} + + static void evanescent_conn_ci_packet_sent (struct lsquic_conn *lconn, struct lsquic_packet_out *packet_out) @@ -562,9 +578,7 @@ evanescent_conn_ci_packet_sent (struct lsquic_conn *lconn, LSQ_DEBUGC("sent %s packet for connection %"CID_FMT"; free resources", lsquic_preqt2str[ evconn->evc_req->pr_type ], CID_BITS(&evconn->evc_req->pr_dcid)); - TAILQ_INSERT_HEAD(&prq->prq_free_conns, lconn, cn_next_pr); - put_req(prq, evconn->evc_req); - --prq->prq_nconns; + prq_free_conn(prq, lconn); } @@ -578,8 +592,17 @@ evanescent_conn_ci_packet_not_sent (struct lsquic_conn *lconn, assert(packet_out == &evconn->evc_packet_out); assert(prq->prq_nconns > 0); - LSQ_DEBUG("packet not sent; put connection onto used list"); - TAILQ_INSERT_HEAD(&prq->prq_returned_conns, lconn, cn_next_pr); + if (evconn->evc_flags & EVC_DROP) + { + LSQ_DEBUGC("packet not sent; drop connection %"CID_FMT, + CID_BITS(&evconn->evc_req->pr_dcid)); + prq_free_conn(prq, lconn); + } + else + { + LSQ_DEBUG("packet not sent; put connection onto used list"); + TAILQ_INSERT_HEAD(&prq->prq_returned_conns, lconn, cn_next_pr); + } } @@ -668,3 +691,14 @@ const char *const lsquic_preqt2str[] = [PACKET_REQ_VERNEG] = "version negotiation", [PACKET_REQ_PUBRES] = "stateless reset", }; + + +void +lsquic_prq_drop (struct lsquic_conn *lconn) +{ + struct evanescent_conn *const evconn = (void *) lconn; + + evconn->evc_flags |= EVC_DROP; + LSQ_DEBUGC("mark for connection %"CID_FMT" for dropping", + CID_BITS(&evconn->evc_req->pr_dcid)); +} diff --git a/src/liblsquic/lsquic_pr_queue.h b/src/liblsquic/lsquic_pr_queue.h index 34b606b..349ae55 100644 --- a/src/liblsquic/lsquic_pr_queue.h +++ b/src/liblsquic/lsquic_pr_queue.h @@ -75,4 +75,7 @@ prq_next_conn (struct pr_queue *); int prq_have_pending (const struct pr_queue *); +void +lsquic_prq_drop (struct lsquic_conn *); + #endif