Release 2.7.2

- [BUGFIX] Send controller: update scheduled bytes when DCID length changes (IETF client). - [BUGFIX] Drop alarm check from sanity test. It no longer works now that we use loss chains. - [PORTABILITY] Fix build on Alpine Linux. - [PORTABILITY] Fix build using XCode. - Client initial DCID length: use RAND_bytes() instead of rand(3). - Add unit tests for connection min heap. - [DEBUG] Log CID in gQUIC handshake module - [DEBUG] Turn on extra checks for IETF client send controller. - [DEBUG] Dedup next advisory tick messages when reason is IDLE timer. - [DEBUG] QPACK decoder handler: log header error code.
litespeedtech · Dec 11, 2019 · a137764 · a137764
1 parent d6937dd
commit a137764
Show file tree

Hide file tree

Showing 22 changed files with 369 additions and 76 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,18 @@
+2019-12-11
+    - 2.7.2
+    - [BUGFIX] Send controller: update scheduled bytes when DCID length
+      changes (IETF client).
+    - [BUGFIX] Drop alarm check from sanity test.  It no longer works now
+      that we use loss chains.
+    - [PORTABILITY] Fix build on Alpine Linux.
+    - [PORTABILITY] Fix build using XCode.
+    - Client initial DCID length: use RAND_bytes() instead of rand(3).
+    - Add unit tests for connection min heap.
+    - [DEBUG] Log CID in gQUIC handshake module
+    - [DEBUG] Turn on extra checks for IETF client send controller.
+    - [DEBUG] Dedup next advisory tick messages when reason is IDLE timer.
+    - [DEBUG] QPACK decoder handler: log header error code.
+
 2019-12-05
     - 2.7.1
     - [BUGFIX] client: don't call ignore_init() in middle of batch send.

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
@@ -7,6 +7,8 @@ to the LiteSpeed Client Library:
     - Alexis La Goutte -- Travis-CI integration
     - Bernhard Jaeger -- DNS Resolution
     - Zhang Chi -- libevent build fix on Darwin
+    - Omar Roth -- Alpine Linux build and Crystal language bindings
+    - initlife (?) -- XCode build
 
 Thank you!
 

diff --git a/include/lsquic.h b/include/lsquic.h
@@ -25,7 +25,7 @@ extern "C" {
 
 #define LSQUIC_MAJOR_VERSION 2
 #define LSQUIC_MINOR_VERSION 7
-#define LSQUIC_PATCH_VERSION 1
+#define LSQUIC_PATCH_VERSION 2
 
 /**
  * Engine flags:

diff --git a/src/liblsquic/lsquic_conn.c b/src/liblsquic/lsquic_conn.c
@@ -3,7 +3,6 @@
 #include <inttypes.h>
 #include <string.h>
 #include <sys/queue.h>
-#include <stdlib.h>
 
 #include <openssl/rand.h>
 
@@ -238,8 +237,12 @@ void
 lsquic_generate_cid (lsquic_cid_t *cid, size_t len)
 {
     if (!len)
+    {
         /* If not set, generate ID between 8 and MAX_CID_LEN bytes in length */
-        len = 8 + rand() % (MAX_CID_LEN - 7);
+        RAND_bytes((uint8_t *) &len, sizeof(len));
+        len %= MAX_CID_LEN - 7;
+        len += 8;
+    }
     RAND_bytes(cid->idbuf, len);
     cid->len = len;
 }

diff --git a/src/liblsquic/lsquic_enc_sess.h b/src/liblsquic/lsquic_enc_sess.h
@@ -99,6 +99,9 @@ struct enc_session_funcs_common
     int
     (*esf_alg_keysize) (enc_session_t *);
 
+    /* Need to pass lconn in encrypt and decrypt methods because enc_session
+     * is allowed to be NULL for gQUIC.
+     */
     enum enc_packout
     (*esf_encrypt_packet) (enc_session_t *, const struct lsquic_engine_public *,
         struct lsquic_conn *, struct lsquic_packet_out *);
@@ -119,6 +122,9 @@ struct enc_session_funcs_common
     int
     (*esf_is_zero_rtt_enabled) (enc_session_t *);
 
+    void
+    (*esf_set_conn) (enc_session_t *, struct lsquic_conn *);
+
     unsigned
     esf_tag_len;
 };
@@ -147,7 +153,8 @@ struct enc_session_funcs_gquic
 
     /* Create server session */
     enc_session_t *
-    (*esf_create_server) (lsquic_cid_t cid, const struct lsquic_engine_public *);
+    (*esf_create_server) (struct lsquic_conn *,
+                        lsquic_cid_t cid, const struct lsquic_engine_public *);
 
     /* out_len should have init value as the max length of out */
     enum handshake_error
@@ -208,7 +215,8 @@ struct enc_session_funcs_gquic
 
     /* Create client session */
     enc_session_t *
-    (*esf_create_client) (const char *domain, lsquic_cid_t cid,
+    (*esf_create_client) (struct lsquic_conn *, const char *domain,
+                            lsquic_cid_t cid,
                                     const struct lsquic_engine_public *,
                                     const unsigned char *, size_t);
 
@@ -229,7 +237,6 @@ struct enc_session_funcs_gquic
      * call it after the "handshake is done" callback is called.
      */
     void (*esf_maybe_dispatch_zero_rtt) (enc_session_t *,
-            struct lsquic_conn *conn,
             void (*cb)(struct lsquic_conn *, const unsigned char *, size_t));
 
     void (*esf_reset_cid) (enc_session_t *, const lsquic_cid_t *);
@@ -279,9 +286,6 @@ struct enc_session_funcs_iquic
     int
     (*esfi_init_server) (enc_session_t *);
 
-    void
-    (*esfi_set_conn) (enc_session_t *, struct lsquic_conn *);
-
     void
     (*esfi_set_streams) (enc_session_t *, void *(crypto_streams)[4],
                            const struct crypto_stream_if *);

diff --git a/src/liblsquic/lsquic_enc_sess_ietf.c b/src/liblsquic/lsquic_enc_sess_ietf.c
@@ -1064,7 +1064,7 @@ iquic_lookup_cert (SSL *ssl, void *arg)
 
 
 static void
-iquic_esfi_set_conn (enc_session_t *enc_session_p, struct lsquic_conn *lconn)
+iquic_esf_set_conn (enc_session_t *enc_session_p, struct lsquic_conn *lconn)
 {
     struct enc_sess_iquic *const enc_sess = enc_session_p;
     enc_sess->esi_conn = lconn;
@@ -1657,10 +1657,11 @@ static const enum enc_level pns2enc_level[] =
 
 static enum enc_packout
 iquic_esf_encrypt_packet (enc_session_t *enc_session_p,
-    const struct lsquic_engine_public *enpub, struct lsquic_conn *lconn,
+    const struct lsquic_engine_public *enpub, struct lsquic_conn *lconn_UNUSED,
     struct lsquic_packet_out *packet_out)
 {
     struct enc_sess_iquic *const enc_sess = enc_session_p;
+    struct lsquic_conn *const lconn = enc_sess->esi_conn;
     unsigned char *dst;
     const struct crypto_ctx_pair *pair;
     const struct crypto_ctx *crypto_ctx;
@@ -1676,8 +1677,6 @@ iquic_esf_encrypt_packet (enc_session_t *enc_session_p,
     enum packnum_space pns;
     char errbuf[ERR_ERROR_STRING_BUF_LEN];
 
-    assert(lconn == enc_sess->esi_conn);
-
     if (packet_out->po_flags & PO_MINI)
     {
         /* XXX TODO Don't rely on PO_MINI, generalize this logic */
@@ -2232,7 +2231,6 @@ const struct enc_session_funcs_iquic lsquic_enc_session_iquic_ietf_v1 =
                          = iquic_esfi_get_peer_transport_params,
     .esfi_reset_dcid     = iquic_esfi_reset_dcid,
     .esfi_init_server    = iquic_esfi_init_server,
-    .esfi_set_conn       = iquic_esfi_set_conn,
     .esfi_set_streams    = iquic_esfi_set_streams,
     .esfi_create_server  = iquic_esfi_create_server,
     .esfi_shake_stream   = iquic_esfi_shake_stream,
@@ -2253,6 +2251,7 @@ const struct enc_session_funcs_common lsquic_enc_session_common_ietf_v1 =
     .esf_keysize         = iquic_esf_keysize,
     .esf_alg_keysize     = iquic_esf_alg_keysize,
     .esf_is_zero_rtt_enabled = iquic_esf_zero_rtt_enabled,
+    .esf_set_conn        = iquic_esf_set_conn,
 };
 
 

diff --git a/src/liblsquic/lsquic_engine.c b/src/liblsquic/lsquic_engine.c
@@ -84,6 +84,10 @@
 #define LSQUIC_DEBUG_NEXT_ADV_TICK 1
 #endif
 
+#if LSQUIC_DEBUG_NEXT_ADV_TICK
+#include "lsquic_alarmset.h"
+#endif
+
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
 
 /* The batch of outgoing packets grows and shrinks dynamically */
@@ -248,6 +252,9 @@ struct lsquic_engine
 #if LSQUIC_COUNT_ENGINE_CALLS
     unsigned long                      n_engine_calls;
 #endif
+#if LSQUIC_DEBUG_NEXT_ADV_TICK
+    uintptr_t                          last_logged_idle_conn;
+#endif
 };
 
 
@@ -2543,16 +2550,14 @@ process_connections (lsquic_engine_t *engine, conn_iter_f next_conn,
         (void) engine_decref_conn(engine, conn, LSCONN_CLOSING);
     }
 
-    /* TODO Heapification can be optimized by switching to the Floyd method:
-     * https://en.wikipedia.org/wiki/Binary_heap#Building_a_heap
-     */
     while ((conn = TAILQ_FIRST(&ticked_conns)))
     {
         TAILQ_REMOVE(&ticked_conns, conn, cn_next_ticked);
         engine_decref_conn(engine, conn, LSCONN_TICKED);
         if (!(conn->cn_flags & LSCONN_TICKABLE)
             && conn->cn_if->ci_is_tickable(conn))
         {
+            /* Floyd heapification is not faster, don't bother. */
             lsquic_mh_insert(&engine->conns_tickable, conn, conn->cn_last_ticked);
             engine_incref_conn(conn, LSCONN_TICKABLE);
         }
@@ -2709,6 +2714,7 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff)
     {
 #if LSQUIC_DEBUG_NEXT_ADV_TICK
         conn = lsquic_mh_peek(&engine->conns_out);
+        engine->last_logged_idle_conn = 0;
         LSQ_LOGC(L, "next advisory tick is now: went past deadline last time "
             "and have %u outgoing connection%.*s (%"CID_FMT" first)",
             lsquic_mh_count(&engine->conns_out),
@@ -2722,6 +2728,7 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff)
     if (engine->pr_queue && prq_have_pending(engine->pr_queue))
     {
 #if LSQUIC_DEBUG_NEXT_ADV_TICK
+        engine->last_logged_idle_conn = 0;
         LSQ_LOG(L, "next advisory tick is now: have pending PRQ elements");
 #endif
         *diff = 0;
@@ -2732,6 +2739,7 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff)
     {
 #if LSQUIC_DEBUG_NEXT_ADV_TICK
         conn = lsquic_mh_peek(&engine->conns_tickable);
+        engine->last_logged_idle_conn = 0;
         LSQ_LOGC(L, "next advisory tick is now: have %u tickable "
             "connection%.*s (%"CID_FMT" first)",
             lsquic_mh_count(&engine->conns_tickable),
@@ -2769,9 +2777,21 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff)
     *diff = (int) ((int64_t) next_time - (int64_t) now);
 #if LSQUIC_DEBUG_NEXT_ADV_TICK
     if (next_attq)
-        LSQ_LOGC(L, "next advisory tick is %d usec away: conn %"CID_FMT
-            ": %s", *diff, CID_BITS(lsquic_conn_log_cid(next_attq->ae_conn)),
-            lsquic_attq_why2str(next_attq->ae_why));
+    {
+        /* Deduplicate consecutive log messages about IDLE timer for the
+         * same connection.
+         */
+        if (!((unsigned) next_attq->ae_why == (unsigned) (N_AEWS + AL_IDLE)
+                    && (uintptr_t) next_attq->ae_conn
+                                            == engine->last_logged_idle_conn))
+        {
+            if ((unsigned) next_attq->ae_why == (unsigned) (N_AEWS + AL_IDLE))
+                engine->last_logged_idle_conn = (uintptr_t) next_attq->ae_conn;
+            LSQ_LOGC(L, "next advisory tick is %d usec away: conn %"CID_FMT
+                ": %s", *diff, CID_BITS(lsquic_conn_log_cid(next_attq->ae_conn)),
+                lsquic_attq_why2str(next_attq->ae_why));
+        }
+    }
     else
         LSQ_LOG(L, "next advisory tick is %d usec away: resume sending", *diff);
 #endif

diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c
@@ -708,8 +708,8 @@ lsquic_gquic_full_conn_client_new (struct lsquic_engine_public *enpub,
     conn->fc_conn.cn_esf_c = select_esf_common_by_ver(version);
     conn->fc_conn.cn_esf.g = esf_g;
     conn->fc_conn.cn_enc_session =
-        conn->fc_conn.cn_esf.g->esf_create_client(hostname, cid, conn->fc_enpub,
-                                                    zero_rtt, zero_rtt_len);
+        conn->fc_conn.cn_esf.g->esf_create_client(&conn->fc_conn, hostname,
+                                cid, conn->fc_enpub, zero_rtt, zero_rtt_len);
     if (!conn->fc_conn.cn_enc_session)
     {
         LSQ_WARN("could not create enc session: %s", strerror(errno));
@@ -818,6 +818,8 @@ lsquic_gquic_full_conn_server_new (struct lsquic_engine_public *enpub,
     assert(lconn_full->cn_enc_session == NULL);
     lconn_full->cn_enc_session = lconn_mini->cn_enc_session;
     lconn_mini->cn_enc_session = NULL;
+    lconn_full->cn_esf_c->esf_set_conn(lconn_full->cn_enc_session,
+                                                            &conn->fc_conn);
 
     lsquic_send_ctl_verneg_done(&conn->fc_send_ctl);
     conn->fc_send_ctl.sc_cur_packno = mc->mc_cur_packno;
@@ -3640,7 +3642,7 @@ full_conn_ci_hsk_done (lsquic_conn_t *lconn, enum lsquic_hsk_status status)
     {
         if (conn->fc_stream_ifs[STREAM_IF_STD].stream_if->on_zero_rtt_info)
             conn->fc_conn.cn_esf.g->esf_maybe_dispatch_zero_rtt(
-                conn->fc_conn.cn_enc_session, &conn->fc_conn,
+                conn->fc_conn.cn_enc_session,
                 conn->fc_stream_ifs[STREAM_IF_STD].stream_if->on_zero_rtt_info);
         if (conn->fc_n_delayed_streams)
             create_delayed_streams(conn);

diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c
@@ -1209,7 +1209,7 @@ lsquic_ietf_full_conn_server_new (struct lsquic_engine_public *enpub,
 
     conn->ifc_conn.cn_enc_session = mini_conn->cn_enc_session;
     mini_conn->cn_enc_session     = NULL;
-    conn->ifc_conn.cn_esf.i->esfi_set_conn(conn->ifc_conn.cn_enc_session,
+    conn->ifc_conn.cn_esf_c->esf_set_conn(conn->ifc_conn.cn_enc_session,
                                                             &conn->ifc_conn);
     conn->ifc_process_incoming_packet = process_incoming_packet_fast;
 
@@ -5672,6 +5672,21 @@ sockaddr_eq (const struct sockaddr *a, const struct sockaddr *b)
 }
 
 
+static void
+record_dcid (struct ietf_full_conn *conn,
+                                    const struct lsquic_packet_in *packet_in)
+{
+    unsigned orig_cid_len;
+
+    orig_cid_len = CUR_DCID(conn)->len;
+    conn->ifc_flags |= IFC_DCID_SET;
+    lsquic_scid_from_packet_in(packet_in, CUR_DCID(conn));
+    LSQ_DEBUGC("set DCID to %"CID_FMT, CID_BITS(CUR_DCID(conn)));
+    lsquic_send_ctl_cidlen_change(&conn->ifc_send_ctl, orig_cid_len,
+                                                        CUR_DCID(conn)->len);
+}
+
+
 static int
 process_regular_packet (struct ietf_full_conn *conn,
                                         struct lsquic_packet_in *packet_in)
@@ -5732,8 +5747,8 @@ process_regular_packet (struct ietf_full_conn *conn,
     if (0 == (packet_in->pi_flags & PI_DECRYPTED))
     {
         dec_packin = conn->ifc_conn.cn_esf_c->esf_decrypt_packet(
-                        conn->ifc_conn.cn_enc_session, conn->ifc_enpub,
-                        &conn->ifc_conn, packet_in);
+                            conn->ifc_conn.cn_enc_session, conn->ifc_enpub,
+                            &conn->ifc_conn, packet_in);
         switch (dec_packin)
         {
         case DECPI_BADCRYPT:
@@ -5784,12 +5799,7 @@ process_regular_packet (struct ietf_full_conn *conn,
     case REC_ST_OK:
         if (!(conn->ifc_flags & (IFC_SERVER|IFC_DCID_SET))
                                                 && (packet_in->pi_scid_len))
-        {
-            conn->ifc_flags |= IFC_DCID_SET;
-            lsquic_scid_from_packet_in(packet_in, CUR_DCID(conn));
-            LSQ_DEBUGC("set DCID to %"CID_FMT,
-                                        CID_BITS(CUR_DCID(conn)));
-        }
+            record_dcid(conn, packet_in);
         saved_path_id = conn->ifc_cur_path_id;
         parse_regular_packet(conn, packet_in);
         if (saved_path_id == conn->ifc_cur_path_id)