Release 2.16.1

- [FEATURE] Use "no-progress timeout" after which connection is closed. - [BUGFIX] Select new SCID when current SCID is retired. - [BUGFIX] Don't warn about dropped Initial packet sequence gaps during mini/full handoff. - [BUGFIX] Send correct conn error when HTTP/3 frame is truncated. - [BUGFIX] Mini conn: consider amplification when deciding to return TICK_SEND. - [BUGFIX] Don't double-count tag length in amplification logic. - [BUGFIX] Don't squeeze out lone path challenges. - [BUGFIX] Log messages dealing with scheduled packet queue squeezing. - [BUGFIX] don't wipe current path if no path challenge responses come back. - [BUGFIX] When path is reset, don't lose path_id which is used for logging. - Downgrade flow control violations to info log level from warnings. - Fix connection cap extra check, avoid checks in nested calls. - Fix some unit tests when extra checks are enabled. - Use ls-hpack 2.2.1. - Turn off unconditional extra checks for IETF clients. - Extra checks: don't verify sent size of hello packets. Client changes DCID length and this check will fail.
litespeedtech · Jun 9, 2020 · 8ae5ecb · 8ae5ecb
1 parent 6bca16f
commit 8ae5ecb
Show file tree

Hide file tree

Showing 22 changed files with 368 additions and 53 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,27 @@
+2020-06-09
+    - 2.16.1
+    - [FEATURE] Use "no-progress timeout" after which connection is closed.
+    - [BUGFIX] Select new SCID when current SCID is retired.
+    - [BUGFIX] Don't warn about dropped Initial packet sequence gaps during
+      mini/full handoff.
+    - [BUGFIX] Send correct conn error when HTTP/3 frame is truncated.
+    - [BUGFIX] Mini conn: consider amplification when deciding to return
+      TICK_SEND.
+    - [BUGFIX] Don't double-count tag length in amplification logic.
+    - [BUGFIX] Don't squeeze out lone path challenges.
+    - [BUGFIX] Log messages dealing with scheduled packet queue squeezing.
+    - [BUGFIX] don't wipe current path if no path challenge responses
+      come back.
+    - [BUGFIX] When path is reset, don't lose path_id which is used for
+      logging.
+    - Downgrade flow control violations to info log level from warnings.
+    - Fix connection cap extra check, avoid checks in nested calls.
+    - Fix some unit tests when extra checks are enabled.
+    - Use ls-hpack 2.2.1.
+    - Turn off unconditional extra checks for IETF clients.
+    - Extra checks: don't verify sent size of hello packets.  Client
+      changes DCID length and this check will fail.
+
 2020-06-03
     - 2.16.0
     - [API] Use lsxpack_header v206.

diff --git a/bin/test_common.c b/bin/test_common.c
@@ -1939,6 +1939,11 @@ set_engine_option (struct lsquic_engine_settings *settings,
             settings->es_qpack_dec_max_size = atoi(val);
             return 0;
         }
+        if (0 == strncmp(name, "noprogress_timeout", 18))
+        {
+            settings->es_noprogress_timeout = atoi(val);
+            return 0;
+        }
         break;
     case 20:
         if (0 == strncmp(name, "max_header_list_size", 20))

diff --git a/docs/apiref.rst b/docs/apiref.rst
@@ -46,13 +46,13 @@ developed by the IETF.  Both types are included in a single enum:
 
         Google QUIC version Q050
 
-    .. member:: LSQVER_ID25
+    .. member:: LSQVER_ID27
 
-        IETF QUIC version ID (Internet-Draft) 25
+        IETF QUIC version ID (Internet-Draft) 27
 
-    .. member:: LSQVER_ID27
+    .. member:: LSQVER_ID28
 
-        IETF QUIC version ID 27
+        IETF QUIC version ID 28
 
     .. member:: N_LSQVER
 
@@ -703,6 +703,19 @@ settings structure:
 
        Default value is :macro:`LSQUIC_DF_MAX_UDP_PAYLOAD_SIZE_RX`
 
+    .. member:: unsigned        es_noprogress_timeout
+
+       No progress timeout.
+
+       If connection does not make progress for this number of seconds, the
+       connection is dropped.  Here, progress is defined as user streams
+       being written to or read from.
+
+       If this value is zero, this timeout is disabled.
+
+       Default value is :macro:`LSQUIC_DF_NOPROGRESS_TIMEOUT_SERVER` in server
+       mode and :macro:`LSQUIC_DF_NOPROGRESS_TIMEOUT_CLIENT` in client mode.
+
 To initialize the settings structure to library defaults, use the following
 convenience function:
 
@@ -881,6 +894,14 @@ out of date.  Please check your :file:`lsquic.h` for actual values.*
 
     By default, incoming packet size is not limited.
 
+.. macro:: LSQUIC_DF_NOPROGRESS_TIMEOUT_SERVER
+
+    By default, drop no-progress connections after 60 seconds on the server.
+
+.. macro:: LSQUIC_DF_NOPROGRESS_TIMEOUT_CLIENT
+
+    By default, do not use no-progress timeout on the client.
+
 Receiving Packets
 -----------------
 

diff --git a/docs/conf.py b/docs/conf.py
@@ -26,7 +26,7 @@
 # The short X.Y version
 version = u'2.16'
 # The full version, including alpha/beta/rc tags
-release = u'2.16.0'
+release = u'2.16.1'
 
 
 # -- General configuration ---------------------------------------------------

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 16
-#define LSQUIC_PATCH_VERSION 0
+#define LSQUIC_PATCH_VERSION 1
 
 /**
  * Engine flags:
@@ -343,6 +343,12 @@ typedef struct ssl_ctx_st * (*lsquic_lookup_cert_f)(
 /** By default, incoming packet size is not limited. */
 #define LSQUIC_DF_MAX_UDP_PAYLOAD_SIZE_RX 0
 
+/** By default, drop no-progress connections after 60 seconds on the server */
+#define LSQUIC_DF_NOPROGRESS_TIMEOUT_SERVER 60
+
+/** By default, do not use no-progress timeout on the client */
+#define LSQUIC_DF_NOPROGRESS_TIMEOUT_CLIENT 0
+
 struct lsquic_engine_settings {
     /**
      * This is a bit mask wherein each bit corresponds to a value in
@@ -752,6 +758,20 @@ struct lsquic_engine_settings {
      * Default value is @ref LSQUIC_DF_MAX_UDP_PAYLOAD_SIZE_RX
      */
     unsigned short  es_max_udp_payload_size_rx;
+
+    /**
+     * No progress timeout.
+     *
+     * If connection does not make progress for this number of seconds, the
+     * connection is dropped.  Here, progress is defined as user streams
+     * being written to or read from.
+     *
+     * If this value is zero, this timeout is disabled.
+     *
+     * Default value is @ref LSQUIC_DF_NOPROGRESS_TIMEOUT_SERVER in server
+     * mode and @ref LSQUIC_DF_NOPROGRESS_TIMEOUT_CLIENT in client mode.
+     */
+    unsigned        es_noprogress_timeout;
 };
 
 /* Initialize `settings' to default values */

diff --git a/src/liblsquic/lsquic_cfcw.c b/src/liblsquic/lsquic_cfcw.c
@@ -99,7 +99,7 @@ lsquic_cfcw_incr_max_recv_off (struct lsquic_cfcw *fc, uint64_t incr)
     }
     else
     {
-        LSQ_WARN("flow control violation: received at offset %"PRIu64", while "
+        LSQ_INFO("flow control violation: received at offset %"PRIu64", while "
             "flow control receive offset is %"PRIu64,
             fc->cf_max_recv_off + incr, fc->cf_recv_off);
         return 0;

diff --git a/src/liblsquic/lsquic_conn_public.h b/src/liblsquic/lsquic_conn_public.h
@@ -56,12 +56,15 @@ struct lsquic_conn_public {
     const struct network_path      *path;
 #if LSQUIC_EXTRA_CHECKS
     unsigned long                   stream_frame_bytes;
+    unsigned                        wtp_level;  /* wtp: Write To Packets */
 #endif
     /* "unsigned" is wide enough: these values are only used for amplification
      * limit before initial path is validated.
      */
     unsigned                        bytes_in;   /* successfully processed */
     unsigned                        bytes_out;
+    /* Used for no-progress timeout */
+    lsquic_time_t                   last_tick, last_prog;
 };
 
 #endif
diff --git a/src/liblsquic/lsquic_engine.c b/src/liblsquic/lsquic_engine.c
@@ -295,6 +295,8 @@ lsquic_engine_init_settings (struct lsquic_engine_settings *settings,
         settings->es_init_max_streams_uni
                          = LSQUIC_DF_INIT_MAX_STREAMS_UNI_SERVER;
         settings->es_ping_period = 0;
+        settings->es_noprogress_timeout
+                         = LSQUIC_DF_NOPROGRESS_TIMEOUT_SERVER;
     }
     else
     {
@@ -311,6 +313,8 @@ lsquic_engine_init_settings (struct lsquic_engine_settings *settings,
         settings->es_init_max_streams_uni
                          = LSQUIC_DF_INIT_MAX_STREAMS_UNI_CLIENT;
         settings->es_ping_period = LSQUIC_DF_PING_PERIOD;
+        settings->es_noprogress_timeout
+                         = LSQUIC_DF_NOPROGRESS_TIMEOUT_CLIENT;
     }
     settings->es_max_streams_in  = LSQUIC_DF_MAX_STREAMS_IN;
     settings->es_idle_conn_to    = LSQUIC_DF_IDLE_CONN_TO;
@@ -601,6 +605,9 @@ lsquic_engine_new (unsigned flags,
     if (!engine->pub.enp_tokgen)
         return NULL;
     engine->pub.enp_crand = &engine->crand;
+    if (engine->pub.enp_settings.es_noprogress_timeout)
+        engine->pub.enp_noprog_timeout
+            = engine->pub.enp_settings.es_noprogress_timeout * 1000000;
     if (flags & ENG_SERVER)
     {
         engine->pr_queue = lsquic_prq_create(

diff --git a/src/liblsquic/lsquic_engine_public.h b/src/liblsquic/lsquic_engine_public.h
@@ -66,6 +66,8 @@ struct lsquic_engine_public {
     struct crand                   *enp_crand;
     struct evp_aead_ctx_st         *enp_retry_aead_ctx;
     unsigned char                  *enp_alpn;   /* May be set if not HTTP */
+    /* es_noprogress_timeout converted to microseconds for speed */
+    lsquic_time_t                   enp_noprog_timeout;
 };
 
 /* Put connection onto the Tickable Queue if it is not already on it.  If

diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c
@@ -112,6 +112,7 @@ enum full_conn_flags {
     FC_ABORT_COMPLAINED
                       = (1 <<23),
     FC_GOT_SREJ       = (1 <<24),   /* Don't schedule ACK alarm */
+    FC_NOPROG_TIMEOUT = (1 <<25),
 };
 
 #define FC_IMMEDIATE_CLOSE_FLAGS \
@@ -686,6 +687,8 @@ new_conn_common (lsquic_cid_t cid, struct lsquic_engine_public *enpub,
     if (conn->fc_settings->es_support_push)
         conn->fc_flags |= FC_SUPPORT_PUSH;
     conn->fc_conn.cn_n_cces = sizeof(conn->fc_cces) / sizeof(conn->fc_cces[0]);
+    if (conn->fc_settings->es_noprogress_timeout)
+        conn->fc_flags |= FC_NOPROG_TIMEOUT;
     return conn;
 
   cleanup_on_error:
@@ -2481,9 +2484,22 @@ idle_alarm_expired (enum alarm_id al_id, void *ctx, lsquic_time_t expiry,
                                                             lsquic_time_t now)
 {
     struct full_conn *conn = ctx;
-    LSQ_DEBUG("connection timed out");
-    EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "connection timed out");
-    conn->fc_flags |= FC_TIMED_OUT;
+
+    if ((conn->fc_flags & FC_NOPROG_TIMEOUT)
+        && conn->fc_pub.last_prog + conn->fc_enpub->enp_noprog_timeout < now)
+    {
+        LSQ_DEBUG("connection timed out due to lack of progress");
+        EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "connection timed out due to "
+                                                            "lack of progress");
+        /* Different flag so that CONNECTION_CLOSE frame is sent */
+        conn->fc_flags |= FC_ABORTED;
+    }
+    else
+    {
+        LSQ_DEBUG("connection timed out");
+        EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "connection timed out");
+        conn->fc_flags |= FC_TIMED_OUT;
+    }
 }
 
 
@@ -3242,6 +3258,31 @@ full_conn_ci_can_write_ack (struct lsquic_conn *lconn)
 }
 
 
+/* This should be called before lsquic_alarmset_ring_expired() */
+static void
+maybe_set_noprogress_alarm (struct full_conn *conn, lsquic_time_t now)
+{
+    lsquic_time_t exp;
+
+    if (conn->fc_flags & FC_NOPROG_TIMEOUT)
+    {
+        if (conn->fc_pub.last_tick)
+        {
+            exp = conn->fc_pub.last_prog + conn->fc_enpub->enp_noprog_timeout;
+            if (!lsquic_alarmset_is_set(&conn->fc_alset, AL_IDLE)
+                                    || exp < conn->fc_alset.as_expiry[AL_IDLE])
+                lsquic_alarmset_set(&conn->fc_alset, AL_IDLE, exp);
+            conn->fc_pub.last_tick = now;
+        }
+        else
+        {
+            conn->fc_pub.last_tick = now;
+            conn->fc_pub.last_prog = now;
+        }
+    }
+}
+
+
 static enum tick_st
 full_conn_ci_tick (lsquic_conn_t *lconn, lsquic_time_t now)
 {
@@ -3296,6 +3337,8 @@ full_conn_ci_tick (lsquic_conn_t *lconn, lsquic_time_t now)
         conn->fc_flags &= ~FC_HAVE_SAVED_ACK;
     }
 
+    maybe_set_noprogress_alarm(conn, now);
+
     lsquic_send_ctl_tick_in(&conn->fc_send_ctl, now);
     lsquic_send_ctl_set_buffer_stream_packets(&conn->fc_send_ctl, 1);
     CLOSE_IF_NECESSARY();
@@ -3520,6 +3563,20 @@ full_conn_ci_tick (lsquic_conn_t *lconn, lsquic_time_t now)
 }
 
 
+static void
+set_earliest_idle_alarm (struct full_conn *conn, lsquic_time_t idle_conn_to)
+{
+    lsquic_time_t exp;
+
+    if (conn->fc_pub.last_prog
+        && (assert(conn->fc_flags & FC_NOPROG_TIMEOUT),
+            exp = conn->fc_pub.last_prog + conn->fc_enpub->enp_noprog_timeout,
+            exp < idle_conn_to))
+        idle_conn_to = exp;
+    lsquic_alarmset_set(&conn->fc_alset, AL_IDLE, idle_conn_to);
+}
+
+
 static void
 full_conn_ci_packet_in (lsquic_conn_t *lconn, lsquic_packet_in_t *packet_in)
 {
@@ -3528,7 +3585,7 @@ full_conn_ci_packet_in (lsquic_conn_t *lconn, lsquic_packet_in_t *packet_in)
 #if LSQUIC_CONN_STATS
     conn->fc_stats.in.bytes += packet_in->pi_data_sz;
 #endif
-    lsquic_alarmset_set(&conn->fc_alset, AL_IDLE,
+    set_earliest_idle_alarm(conn,
                 packet_in->pi_received + conn->fc_settings->es_idle_conn_to);
     if (0 == (conn->fc_flags & FC_ERROR))
         if (0 != process_incoming_packet(conn, packet_in))