Release 2.10.1

- [BUGFIX] Coalesced packets could get longer than normal packet size.
- Add spin bit configuration option es_spin (-o spin=[01]).
- Disable spin bit in 1/16 of connections.
- Improve logging a bit.

CHANGELOG

@@ -1,3 +1,11 @@
+2020-01-29
+    - 2.10.1
+    - [BUGFIX] Coalesced packets could get longer than normal packet
+      size.
+    - Add spin bit configuration option es_spin (-o spin=[01]).
+    - Disable spin bit in 1/16 of connections.
+    - Improve logging a bit.
+
 2020-01-28
     - 2.10.0
     - [FEATURE] QUIC and HTTP/3 Internet Draft 25 support.

include/lsquic.h

@@ -25,7 +25,7 @@ extern "C" {
 
 #define LSQUIC_MAJOR_VERSION 2
 #define LSQUIC_MINOR_VERSION 10
-#define LSQUIC_PATCH_VERSION 0
+#define LSQUIC_PATCH_VERSION 1
 
 /**
  * Engine flags:
@@ -381,6 +381,9 @@ typedef struct ssl_ctx_st * (*lsquic_lookup_cert_f)(
 /** Use QL loss bits by default */
 #define LSQUIC_DF_QL_BITS 2
 
+/** Turn spin bit on by default */
+#define LSQUIC_DF_SPIN 1
+
 /* 1: Cubic; 2: BBR */
 #define LSQUIC_DF_CC_ALGO 1
 
@@ -759,6 +762,13 @@ struct lsquic_engine_settings {
      * Default value is @ref LSQUIC_DF_QL_BITS
      */
     int             es_ql_bits;
+
+    /**
+     * Enable spin bit.  Allowed values are 0 and 1.
+     *
+     * Default value is @ref LSQUIC_DF_SPIN
+     */
+    int             es_spin;
 };
 
 /* Initialize `settings' to default values */

src/liblsquic/lsquic_engine.c

@@ -337,6 +337,7 @@ lsquic_engine_init_settings (struct lsquic_engine_settings *settings,
     settings->es_qpack_enc_max_blocked = LSQUIC_DF_QPACK_ENC_MAX_BLOCKED;
     settings->es_allow_migration = LSQUIC_DF_ALLOW_MIGRATION;
     settings->es_ql_bits         = LSQUIC_DF_QL_BITS;
+    settings->es_spin            = LSQUIC_DF_SPIN;
 }
 
 
@@ -412,6 +413,14 @@ lsquic_engine_check_settings (const struct lsquic_engine_settings *settings,
         return -1;
     }
 
+    if (!(settings->es_spin == 0 || settings->es_spin == 1))
+    {
+        if (err_buf)
+            snprintf(err_buf, err_buf_sz, "Invalid spin value %d",
+                settings->es_spin);
+        return -1;
+    }
+
     return 0;
 }
 
@@ -2048,13 +2057,13 @@ sockaddr2str (const struct sockaddr *addr, char *buf, size_t sz)
     switch (addr->sa_family)
     {
     case AF_INET:
-        port = ((struct sockaddr_in *) addr)->sin_port;
+        port = ntohs(((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;
+        port = ntohs(((struct sockaddr_in6 *) addr)->sin6_port);
         if (!inet_ntop(AF_INET6, &((struct sockaddr_in6 *) addr)->sin6_addr,
                                                                     buf, sz))
             buf[0] = '\0';

src/liblsquic/lsquic_full_conn_ietf.c

@@ -131,6 +131,7 @@ enum ifull_conn_flags
     IFC_IGNORE_HSK    = 1 << 25,
     IFC_PROC_CRYPTO   = 1 << 26,
     IFC_MIGRA         = 1 << 27,
+    IFC_SPIN          = 1 << 28, /* Spin bits are enabled */
 };
 
 
@@ -973,6 +974,43 @@ ietf_full_conn_add_scid (struct ietf_full_conn *conn,
 }
 
 
+/* From [draft-ietf-quic-transport-25] Section 17.3.1:
+ *  " endpoints MUST disable their use of the spin bit for a random selection
+ *  " of at least one in every 16 network paths, or for one in every 16
+ *  " connection IDs.
+ */
+static void
+maybe_enable_spin (struct ietf_full_conn *conn)
+{
+    uint8_t nyb;
+
+    if (!conn->ifc_settings->es_spin)
+    {
+        conn->ifc_flags &= ~IFC_SPIN;
+        LSQ_DEBUG("spin bit disabled via settings");
+    }
+    else if (lsquic_crand_get_nybble(conn->ifc_enpub->enp_crand))
+    {
+        conn->ifc_flags |= IFC_SPIN;
+        conn->ifc_spin_bit = 0;
+        LSQ_DEBUG("spin bit enabled");
+    }
+    else
+    {
+        /* " It is RECOMMENDED that endpoints set the spin bit to a random
+         * " value either chosen independently for each packet or chosen
+         * " independently for each connection ID.
+         * (ibid.)
+         */
+        conn->ifc_flags &= ~IFC_SPIN;
+        nyb = lsquic_crand_get_nybble(conn->ifc_enpub->enp_crand);
+        conn->ifc_spin_bit = nyb & 1;
+        LSQ_DEBUG("spin bit randomly disabled; random spin bit value is %d",
+            conn->ifc_spin_bit);
+    }
+}
+
+
 static int
 ietf_full_conn_init (struct ietf_full_conn *conn,
            struct lsquic_engine_public *enpub, unsigned flags, int ecn)
@@ -1035,6 +1073,7 @@ ietf_full_conn_init (struct ietf_full_conn *conn,
 #define valid_stream_id(v) ((v) <= VINT_MAX_VALUE)
     conn->ifc_max_req_id = VINT_MAX_VALUE + 1;
     conn->ifc_ping_unretx_thresh = 20;
+    maybe_enable_spin(conn);
     return 0;
 }
 
@@ -5678,7 +5717,10 @@ process_retry_packet (struct ietf_full_conn *conn,
     }
 
     if (0 != verify_retry_packet(conn, packet_in))
+    {
+        LSQ_DEBUG("cannot verify retry packet: ignore it");
         return 0;
+    }
 
     if (0 != lsquic_send_ctl_retry(&conn->ifc_send_ctl,
                     packet_in->pi_data + packet_in->pi_token,
@@ -5767,7 +5809,7 @@ on_dcid_change (struct ietf_full_conn *conn, const lsquic_cid_t *dcid_in)
     LOG_SCIDS(conn);
 
     /* Reset spin bit, see [draft-ietf-quic-transport-20] Section 17.3.1 */
-    conn->ifc_spin_bit = 0;
+    maybe_enable_spin(conn);
 
     return 0;
 }
@@ -5996,7 +6038,7 @@ process_regular_packet (struct ietf_full_conn *conn,
         conn->ifc_incoming_ecn |=
                             lsquic_packet_in_ecn(packet_in) != ECN_NOT_ECT;
         ++conn->ifc_ecn_counts_in[pns][ lsquic_packet_in_ecn(packet_in) ];
-        if (packno_increased && PNS_APP == pns)
+        if (packno_increased && PNS_APP == pns && (conn->ifc_flags & IFC_SPIN))
         {
             if (conn->ifc_flags & IFC_SERVER)
                 conn->ifc_spin_bit = lsquic_packet_in_spin_bit(packet_in);

src/liblsquic/lsquic_send_ctl.c

@@ -1608,18 +1608,21 @@ send_ctl_maybe_zero_pad (struct lsquic_send_ctl *ctl,
 
     cum_size = packet_out_total_sz(initial_packet);
     if (cum_size >= limit)
+    {
+        LSQ_DEBUG("packet size %zu larger than %zu-byte limit: not "
+            "zero-padding", cum_size, limit);
         return;
+    }
 
     TAILQ_FOREACH(packet_out, &ctl->sc_scheduled_packets, po_next)
     {
         size = packet_out_total_sz(packet_out);
-        if (cum_size + size > SC_PACK_SIZE(ctl))
+        if (cum_size + size > limit)
             break;
         cum_size += size;
-        if (cum_size >= limit)
-            return;
     }
 
+    LSQ_DEBUG("cum_size: %zu; limit: %zu", cum_size, limit);
     assert(cum_size < limit);
     size = limit - cum_size;
     if (size > lsquic_packet_out_avail(initial_packet))
@@ -1676,8 +1679,9 @@ lsquic_send_ctl_next_packet_to_send (struct lsquic_send_ctl *ctl, size_t size)
     {
         if (packet_out_total_sz(packet_out) + size > SC_PACK_SIZE(ctl))
             return NULL;
-        LSQ_DEBUG("packet %"PRIu64" will be tacked on to previous packet "
+        LSQ_DEBUG("packet %"PRIu64" (%zu bytes) will be tacked on to "
-                                    "(coalescing)", packet_out->po_packno);
+            "previous packet(s) (%zu bytes) (coalescing)",
+            packet_out->po_packno, packet_out_total_sz(packet_out), size);
     }
     send_ctl_sched_remove(ctl, packet_out);
 
@@ -1690,9 +1694,10 @@ lsquic_send_ctl_next_packet_to_send (struct lsquic_send_ctl *ctl, size_t size)
         packet_out->po_flags &= ~PO_LIMITED;
 
     if (UNLIKELY(packet_out->po_header_type == HETY_INITIAL)
-                    && !(ctl->sc_conn_pub->lconn->cn_flags & LSCONN_SERVER))
+                    && !(ctl->sc_conn_pub->lconn->cn_flags & LSCONN_SERVER)
+                    && size < 1200)
     {
-        send_ctl_maybe_zero_pad(ctl, packet_out, size ? size : 1200);
+        send_ctl_maybe_zero_pad(ctl, packet_out, 1200 - size);
     }
 
     if (ctl->sc_flags & SC_QL_BITS)

test/test_common.c

@@ -1729,6 +1729,11 @@ set_engine_option (struct lsquic_engine_settings *settings,
             settings->es_sfcw = atoi(val);
             return 0;
         }
+        if (0 == strncmp(name, "spin", 4))
+        {
+            settings->es_spin = atoi(val);
+            return 0;
+        }
         break;
     case 7:
         if (0 == strncmp(name, "version", 7))