Release 2.3.0

- [FEATURE] BBR congestion control is on by default
- [BUGFIX] BBR app-limited logic
- [BUGFIX] Fix uninitialized warnings in IETF
- [BUGFIX] Update ls-qpack to v0.9.14
- [CLEANUP] Code cleanup

CHANGELOG

@@ -1,3 +1,11 @@
+2019-09-12
+    - 2.3.0
+    - [FEATURE] BBR congestion control is on by default
+    - [BUGFIX] BBR app-limited logic
+    - [BUGFIX] Fix uninitialized warnings in IETF
+    - [BUGFIX] Update ls-qpack to v0.9.14
+    - [CLEANUP] Code cleanup
+
 2019-09-11
     - 2.2.0
     - [FEATURE] Server code is included in the library

include/lsquic.h

@@ -24,7 +24,7 @@ extern "C" {
 #endif
 
 #define LSQUIC_MAJOR_VERSION 2
-#define LSQUIC_MINOR_VERSION 2
+#define LSQUIC_MINOR_VERSION 3
 #define LSQUIC_PATCH_VERSION 0
 
 /**
@@ -371,7 +371,7 @@ typedef struct ssl_ctx_st * (*lsquic_lookup_cert_f)(
 #define LSQUIC_DF_ALLOW_MIGRATION 1
 
 /* 1: Cubic; 2: BBR */
-#define LSQUIC_DF_CC_ALGO 1
+#define LSQUIC_DF_CC_ALGO 2
 
 struct lsquic_engine_settings {
     /**

src/liblsquic/lsquic_bbr.c

@@ -282,15 +282,21 @@ is_pipe_sufficiently_full (struct lsquic_bbr *bbr, uint64_t bytes_in_flight)
 }
 
 
-/* See BbrSender::OnApplicationLimited */
 static void
-lsquic_bbr_was_quiet (void *cong_ctl, lsquic_time_t now,
+lsquic_bbr_was_quiet (void *cong_ctl, lsquic_time_t now, uint64_t in_flight)
-                                                    uint64_t bytes_in_flight)
 {
     struct lsquic_bbr *const bbr = cong_ctl;
+    LSQ_DEBUG("was quiet");         /* Do nothing */
+}
+
+
+/* See BbrSender::OnApplicationLimited */
+static void
+bbr_app_limited (struct lsquic_bbr *bbr, uint64_t bytes_in_flight)
+{
     uint64_t cwnd;
 
-    cwnd = lsquic_bbr_get_cwnd(cong_ctl);
+    cwnd = lsquic_bbr_get_cwnd(bbr);
     if (bytes_in_flight >= cwnd)
         return;
     if ((bbr->bbr_flags & BBR_FLAG_FLEXIBLE_APP_LIMITED)
@@ -328,7 +334,7 @@ lsquic_bbr_ack (void *cong_ctl, struct lsquic_packet_out *packet_out,
 
 static void
 lsquic_bbr_sent (void *cong_ctl, struct lsquic_packet_out *packet_out,
-                                                        uint64_t in_flight)
+                                        uint64_t in_flight, int app_limited)
 {
     struct lsquic_bbr *const bbr = cong_ctl;
 
@@ -340,6 +346,9 @@ lsquic_bbr_sent (void *cong_ctl, struct lsquic_packet_out *packet_out,
      * increasing.
      */
     bbr->bbr_last_sent_packno = packet_out->po_packno;
+
+    if (app_limited)
+        bbr_app_limited(bbr, in_flight);
 }
 
 

src/liblsquic/lsquic_cong_ctl.h

@@ -42,7 +42,7 @@ struct cong_ctl_if
     /* Optional method */
     void
     (*cci_sent) (void *cong_ctl, struct lsquic_packet_out *,
-                                                        uint64_t in_flight);
+                                        uint64_t in_flight, int app_limited);
 
     /* Optional method */
     void

src/liblsquic/lsquic_conn.h

@@ -242,8 +242,6 @@ struct conn_iface
     (*ci_get_log_cid) (const struct lsquic_conn *);
 };
 
-struct cert_susp_head;
-
 #define LSCONN_CCE_BITS 3
 #define LSCONN_MAX_CCES (1 << LSCONN_CCE_BITS)
 
@@ -292,7 +290,6 @@ struct lsquic_conn
     const struct conn_iface     *cn_if;
     const struct parse_funcs    *cn_pf;
     struct attq_elem            *cn_attq_elem;
-    struct cert_susp_head        *cn_cert_susp_head;
     lsquic_time_t                cn_last_sent;
     lsquic_time_t                cn_last_ticked;
     struct conn_cid_elem        *cn_cces;   /* At least one is available */

src/liblsquic/lsquic_data_in_if.h

@@ -64,6 +64,13 @@ struct data_in_iface
     /* Return number of bytes readable starting at offset `read_offset' */
     uint64_t
     (*di_readable_bytes) (struct data_in *, uint64_t read_offset);
+
+    /* If set, this means that when di_insert_frame() returns INS_FRAME_OK,
+     * the data_in handler has taken ownership of the frame.  Otherwise, it
+     * is up to the caller to free it.
+     */
+    const int
+    di_own_on_ok;
 };
 
 

src/liblsquic/lsquic_di_error.c

@@ -96,6 +96,7 @@ static const struct data_in_iface di_if_error = {
     .di_get_frame    = error_di_get_frame,
     .di_insert_frame = error_di_insert_frame,
     .di_mem_used     = error_di_mem_used,
+    .di_own_on_ok    = 0,   /* Never returns INS_FRAME_OK, but anyway */
     .di_readable_bytes
                      = error_di_readable_bytes,
     .di_switch_impl  = error_di_switch_impl,

src/liblsquic/lsquic_di_hash.c

@@ -440,6 +440,7 @@ hash_di_insert_frame (struct data_in *data_in,
     ins = data_in_hash_insert_data_frame(data_in, data_frame, read_offset);
     assert(ins != INS_FRAME_OVERLAP);
     lsquic_packet_in_put(hdi->hdi_conn_pub->mm, new_frame->packet_in);
+    if (ins != INS_FRAME_OK)
         lsquic_malo_put(new_frame);
     return ins;
 }
@@ -678,6 +679,7 @@ static const struct data_in_iface di_if_hash = {
     .di_get_frame    = hash_di_get_frame,
     .di_insert_frame = hash_di_insert_frame,
     .di_mem_used     = hash_di_mem_used,
+    .di_own_on_ok    = 0,
     .di_readable_bytes
                      = hash_di_readable_bytes,
     .di_switch_impl  = hash_di_switch_impl,

src/liblsquic/lsquic_di_nocopy.c

@@ -551,6 +551,7 @@ static const struct data_in_iface di_if_nocopy = {
     .di_get_frame    = nocopy_di_get_frame,
     .di_insert_frame = nocopy_di_insert_frame,
     .di_mem_used     = nocopy_di_mem_used,
+    .di_own_on_ok    = 1,
     .di_readable_bytes
                      = nocopy_di_readable_bytes,
     .di_switch_impl  = nocopy_di_switch_impl,

src/liblsquic/lsquic_enc_sess_ietf.c

@@ -1625,7 +1625,7 @@ iquic_esf_encrypt_packet (enc_session_t *enc_session_p,
     size_t out_sz, dst_sz;
     int header_sz;
     int ipv6;
-    unsigned packno_off, packno_len, sample_off, cliser;
+    unsigned packno_off, packno_len, cliser;
     enum packnum_space pns;
     char errbuf[ERR_ERROR_STRING_BUF_LEN];
 
@@ -1739,8 +1739,10 @@ iquic_esf_encrypt_packet (enc_session_t *enc_session_p,
     assert(out_sz == dst_sz - header_sz);
 
     lconn->cn_pf->pf_packno_info(lconn, packet_out, &packno_off, &packno_len);
-    sample_off = packno_off + 4;
+#ifndef NDEBUG
+    const unsigned sample_off = packno_off + 4;
     assert(sample_off + IQUIC_TAG_LEN <= dst_sz);
+#endif
     apply_hp(enc_sess, hp, cliser, dst, packno_off, packno_len);
 
     packet_out->po_enc_data    = dst;
@@ -1876,6 +1878,7 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p,
     }
     else
     {
+        key_phase = 0;
         assert(enc_sess->esi_hsk_pairs);
         pair = &enc_sess->esi_hsk_pairs[ enc_level ];
         crypto_ctx = &pair->ykp_ctx[ cliser ];
@@ -1976,6 +1979,9 @@ 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;

src/liblsquic/lsquic_engine.c

@@ -165,12 +165,6 @@ force_close_conn (lsquic_engine_t *engine, lsquic_conn_t *conn);
                         |LSCONN_ATTQ)
 
 
-struct cert_susp_head
-{
-    TAILQ_HEAD(, lsquic_conn)   csh_conns;
-    struct lsquic_hash_elem     csh_hash_el;
-    char                        csh_sni[0];
-};
 
 
 struct cid_update_batch
@@ -237,7 +231,6 @@ struct lsquic_engine
     lsquic_time_t                      deadline;
     lsquic_time_t                      resume_sending_at;
     unsigned                           mini_conns_count;
-    struct lsquic_hash                *suspended_sni_heads;
     struct lsquic_purga               *purga;
 #if LSQUIC_CONN_STATS
     struct {
@@ -581,7 +574,6 @@ lsquic_engine_new (unsigned flags,
     engine->attq = attq_create();
     eng_hist_init(&engine->history);
     engine->batch_size = INITIAL_OUT_BATCH_SIZE;
-    engine->suspended_sni_heads = lsquic_hash_create();
     if (engine->pub.enp_settings.es_honor_prst)
     {
         engine->pub.enp_srst_hash = lsquic_hash_create();
@@ -659,20 +651,6 @@ shrink_batch_size (struct lsquic_engine *engine)
 }
 
 
-static void
-delete_susp_head (struct lsquic_engine *engine, struct cert_susp_head *head)
-{
-    struct lsquic_hash_elem *el;
-
-    el = lsquic_hash_find(engine->suspended_sni_heads, head->csh_sni,
-                          strlen(head->csh_sni));
-    assert(el);
-    assert(head == lsquic_hashelem_getdata(el));
-    lsquic_hash_erase(engine->suspended_sni_heads, el);
-    free(head);
-}
-
-
 struct cce_cid_iter
 {
     const struct lsquic_conn   *conn;
@@ -744,14 +722,6 @@ destroy_conn (struct lsquic_engine *engine, struct lsquic_conn *conn,
     lsquic_time_t drain_time;
     struct purga_el *puel;
 
-    if (conn->cn_cert_susp_head)
-    {
-        TAILQ_REMOVE(&conn->cn_cert_susp_head->csh_conns, conn,
-                     cn_next_susp_cert);
-        if (TAILQ_EMPTY(&conn->cn_cert_susp_head->csh_conns))
-            delete_susp_head(engine, conn->cn_cert_susp_head);
-        conn->cn_cert_susp_head = NULL;
-    }
     engine->mini_conns_count -= !!(conn->cn_flags & LSCONN_MINI);
     if (engine->purga
         /* Blacklist all CIDs except for promoted mini connections */
@@ -1411,7 +1381,6 @@ lsquic_engine_destroy (lsquic_engine_t *engine)
     if (engine->flags & ENG_LOSE_PACKETS)
         regfree(&engine->lose_packets_re);
 #endif
-    lsquic_hash_destroy(engine->suspended_sni_heads);
     if (engine->pub.enp_tokgen)
         lsquic_tg_destroy(engine->pub.enp_tokgen);
 #if LSQUIC_CONN_STATS

src/liblsquic/lsquic_full_conn.c

@@ -3470,6 +3470,8 @@ full_conn_ci_tick (lsquic_conn_t *lconn, lsquic_time_t now)
     if (!TAILQ_EMPTY(&conn->fc_pub.write_streams))
         process_streams_write_events(conn, 0);
 
+    lsquic_send_ctl_maybe_app_limited(&conn->fc_send_ctl, &conn->fc_path);
+
   end_write:
 
   skip_write:

src/liblsquic/lsquic_full_conn_ietf.c

@@ -6027,6 +6027,8 @@ ietf_full_conn_ci_tick (struct lsquic_conn *lconn, lsquic_time_t now)
     if (!TAILQ_EMPTY(&conn->ifc_pub.write_streams))
         process_streams_write_events(conn, 0, &highest_non_crit);
 
+    lsquic_send_ctl_maybe_app_limited(&conn->ifc_send_ctl, CUR_NPATH(conn));
+
   end_write:
     if ((conn->ifc_flags & IFC_CLOSING) && conn_ok_to_close(conn))
     {

src/liblsquic/lsquic_mini_conn.c

@@ -1940,14 +1940,19 @@ mini_conn_ci_record_addrs (struct lsquic_conn *lconn, void *peer_ctx,
 {
     struct mini_conn *mc = (struct mini_conn *) lconn;
     struct lsquic_packet_out *packet_out;
+    size_t len;
+
 
     if (NP_IS_IPv6(&mc->mc_path) != (AF_INET6 == peer_sa->sa_family))
         TAILQ_FOREACH(packet_out, &mc->mc_packets_out, po_next)
             if ((packet_out->po_flags & (PO_SENT|PO_ENCRYPTED)) == PO_ENCRYPTED)
                 return_enc_data(mc, packet_out);
 
-    memcpy(mc->mc_path.np_peer_addr, peer_sa, sizeof(mc->mc_path.np_peer_addr));
+    len = local_sa->sa_family == AF_INET ? sizeof(struct sockaddr_in)
-    memcpy(mc->mc_path.np_local_addr, local_sa, sizeof(mc->mc_path.np_local_addr));
+                                                : sizeof(struct sockaddr_in6);
+
+    memcpy(mc->mc_path.np_peer_addr, peer_sa, len);
+    memcpy(mc->mc_path.np_local_addr, local_sa, len);
     mc->mc_path.np_peer_ctx = peer_ctx;
     return 0;
 }

src/liblsquic/lsquic_mini_conn_ietf.c

@@ -1426,16 +1426,18 @@ ietf_mini_conn_ci_record_addrs (struct lsquic_conn *lconn, void *peer_ctx,
 {
     struct ietf_mini_conn *conn = (struct ietf_mini_conn *) lconn;
     struct lsquic_packet_out *packet_out;
+    size_t len;
 
     if (NP_IS_IPv6(&conn->imc_path) != (AF_INET6 == peer_sa->sa_family))
         TAILQ_FOREACH(packet_out, &conn->imc_packets_out, po_next)
             if ((packet_out->po_flags & (PO_SENT|PO_ENCRYPTED)) == PO_ENCRYPTED)
                 imico_return_enc_data(conn, packet_out);
 
-    memcpy(conn->imc_path.np_peer_addr, peer_sa,
+    len = local_sa->sa_family == AF_INET ? sizeof(struct sockaddr_in)
-                                        sizeof(conn->imc_path.np_peer_addr));
+                                                : sizeof(struct sockaddr_in6);
-    memcpy(conn->imc_path.np_local_addr, local_sa,
+
-                                        sizeof(conn->imc_path.np_local_addr));
+    memcpy(conn->imc_path.np_peer_addr, peer_sa, len);
+    memcpy(conn->imc_path.np_local_addr, local_sa, len);
     conn->imc_path.np_peer_ctx = peer_ctx;
     return 0;
 }

src/liblsquic/lsquic_send_ctl.c

@@ -591,7 +591,8 @@ lsquic_send_ctl_sent_packet (lsquic_send_ctl_t *ctl,
     ++ctl->sc_stats.n_total_sent;
 #endif
     if (ctl->sc_ci->cci_sent)
-        ctl->sc_ci->cci_sent(CGP(ctl), packet_out, ctl->sc_n_in_flight_all);
+        ctl->sc_ci->cci_sent(CGP(ctl), packet_out, ctl->sc_n_in_flight_all,
+                                            ctl->sc_flags & SC_APP_LIMITED);
     lsquic_send_ctl_sanity_check(ctl);
     return 0;
 }
@@ -1291,6 +1292,38 @@ lsquic_send_ctl_can_send (lsquic_send_ctl_t *ctl)
 }
 
 
+/* Like lsquic_send_ctl_can_send(), but no mods */
+static int
+send_ctl_could_send (const struct lsquic_send_ctl *ctl)
+{
+    uint64_t cwnd;
+    unsigned n_out;
+
+    if ((ctl->sc_flags & SC_PACE) && pacer_delayed(&ctl->sc_pacer))
+        return 0;
+
+    cwnd = ctl->sc_ci->cci_get_cwnd(CGP(ctl));
+    n_out = send_ctl_all_bytes_out(ctl);
+    return n_out < cwnd;
+}
+
+
+void
+lsquic_send_ctl_maybe_app_limited (struct lsquic_send_ctl *ctl,
+                                            const struct network_path *path)
+{
+    const struct lsquic_packet_out *packet_out;
+
+    packet_out = lsquic_send_ctl_last_scheduled(ctl, PNS_APP, path, 0);
+    if ((packet_out && lsquic_packet_out_avail(packet_out) > 10)
+                                                || send_ctl_could_send(ctl))
+    {
+        LSQ_DEBUG("app-limited");
+        ctl->sc_flags |= SC_APP_LIMITED;
+    }
+}
+
+
 static void
 send_ctl_expire (struct lsquic_send_ctl *ctl, enum packnum_space pns,
                                                     enum expire_filter filter)

src/liblsquic/lsquic_send_ctl.h

@@ -42,6 +42,7 @@ enum send_ctl_flags {
     SC_LOST_ACK_HSK = SC_LOST_ACK_INIT << PNS_HSK,
     SC_LOST_ACK_APP = SC_LOST_ACK_INIT << PNS_APP,
     SC_1RTT_ACKED   =  1 << 11,
+    SC_APP_LIMITED  =  1 << 12,
 };
 
 typedef struct lsquic_send_ctl {
@@ -255,6 +256,7 @@ lsquic_send_ctl_drop_scheduled (lsquic_send_ctl_t *);
         (ctl)->sc_flags |= SC_SCHED_TICK;                   \
         pacer_tick_in(&(ctl)->sc_pacer, now);               \
     }                                                       \
+    (ctl)->sc_flags &= ~SC_APP_LIMITED;                     \
 } while (0)
 
 #define lsquic_send_ctl_tick_out(ctl) do {                  \
@@ -353,4 +355,8 @@ lsquic_send_ctl_return_enc_data (struct lsquic_send_ctl *);
 
 #define lsquic_send_ctl_1rtt_acked(ctl) ((ctl)->sc_flags & SC_1RTT_ACKED)
 
+void
+lsquic_send_ctl_maybe_app_limited (struct lsquic_send_ctl *,
+                                            const struct network_path *);
+
 #endif

src/liblsquic/lsquic_stream.c

@@ -846,7 +846,7 @@ int
 lsquic_stream_frame_in (lsquic_stream_t *stream, stream_frame_t *frame)
 {
     uint64_t max_off;
-    int got_next_offset;
+    int got_next_offset, rv, free_frame;
     enum ins_frame ins_frame;
 
     assert(frame->packet_in);
@@ -871,9 +871,11 @@ lsquic_stream_frame_in (lsquic_stream_t *stream, stream_frame_t *frame)
         /* Update maximum offset in the flow controller and check for flow
          * control violation:
          */
+        rv = -1;
+        free_frame = !stream->data_in->di_if->di_own_on_ok;
         max_off = frame->data_frame.df_offset + frame->data_frame.df_size;
         if (0 != lsquic_stream_update_sfcw(stream, max_off))
-            return -1;
+            goto end_ok;
         if (frame->data_frame.df_fin)
         {
             SM_HISTORY_APPEND(stream, SHE_FIN_IN);
@@ -889,13 +891,17 @@ lsquic_stream_frame_in (lsquic_stream_t *stream, stream_frame_t *frame)
             if (!stream->data_in)
             {
                 stream->data_in = data_in_error_new();
-                return -1;
+                goto end_ok;
             }
         }
         if (got_next_offset)
             /* Checking the offset saves di_get_frame() call */
             maybe_conn_to_tickable_if_readable(stream);
-        return 0;
+        rv = 0;
+  end_ok:
+        if (free_frame)
+            lsquic_malo_put(frame);
+        return rv;
     }
     else if (INS_FRAME_DUP == ins_frame)
     {

test/unittests/test_bw_sampler.c

@@ -412,8 +412,8 @@ test_not_congestion_controlled (void)
         {
             sample = sampler_test_ack_packet(&stest, packets[i]);
             assert(sample);
-            lsquic_malo_put(sample);
             assert(expected_bw == BW_VALUE(&sample->bandwidth));
+            lsquic_malo_put(sample);
         }
         stest.time += time_between_packets;
     }
@@ -436,6 +436,7 @@ test_compressed_ack (void)
     const lsquic_time_t time_between_packets = ms(1),
                         ridiculously_small_time_delta = us(20);
     uint64_t expected_bw = FromKBytesPerSecond(kRegularPacketSize);
+    uint64_t bw;
     unsigned i;
     struct bw_sample *sample;
     struct lsquic_packet_out *packets[41];
@@ -453,10 +454,11 @@ test_compressed_ack (void)
         sample = sampler_test_ack_packet(&stest, packets[i]);
         assert(sample);
         stest.time += ridiculously_small_time_delta;
+        bw = BW_VALUE(&sample->bandwidth);
         lsquic_malo_put(sample);
     }
 
-    assert(BW_VALUE(&sample->bandwidth) == expected_bw);
+    assert(bw == expected_bw);
     assert(lsquic_bw_sampler_entry_count(&stest.sampler) == 0);
     assert(stest.bytes_in_flight == 0);