Release 2.19.8

- [FEATURE] Update the timestamp extension to latest version. - [FEATURE] Cope with appearance of ECN blackholes. - [OPTIMIZATION] return packno offset and size when header is generated. - [BUGFIX] ignore old ACK frames in mini conns. - [BUGFIX] Mark initial server path as initialized. - [BUGFIX] Do not merge ACK if ECN counts do not match. - Turn incoming packet number history in mini conn back on. - Record mini conn event history again when compiled in debug mode. - IETF mini conn: log when ACK is queued. - Clean up and refactor code in several places.
litespeedtech · Sep 2, 2020 · 49f1f4f · 49f1f4f
1 parent 792df05
commit 49f1f4f
Show file tree

Hide file tree

Showing 26 changed files with 201 additions and 209 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,16 @@
+2020-09-02
+    - 2.19.8
+    - [FEATURE] Update the timestamp extension to latest version.
+    - [FEATURE] Cope with appearance of ECN blackholes.
+    - [OPTIMIZATION] return packno offset and size when header is generated.
+    - [BUGFIX] ignore old ACK frames in mini conns.
+    - [BUGFIX] Mark initial server path as initialized.
+    - [BUGFIX] Do not merge ACK if ECN counts do not match.
+    - Turn incoming packet number history in mini conn back on.
+    - Record mini conn event history again when compiled in debug mode.
+    - IETF mini conn: log when ACK is queued.
+    - Clean up and refactor code in several places.
+
 2020-08-26
     - 2.19.7
     - Handle ECT-CE event: issue a loss event.

diff --git a/docs/conf.py b/docs/conf.py
@@ -26,7 +26,7 @@
 # The short X.Y version
 version = u'2.19'
 # The full version, including alpha/beta/rc tags
-release = u'2.19.7'
+release = u'2.19.8'
 
 
 # -- 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 19
-#define LSQUIC_PATCH_VERSION 7
+#define LSQUIC_PATCH_VERSION 8
 
 /**
  * Engine flags:

diff --git a/src/liblsquic/lsquic_enc_sess_ietf.c b/src/liblsquic/lsquic_enc_sess_ietf.c
@@ -623,7 +623,10 @@ gen_trans_params (struct enc_sess_iquic *enc_sess, unsigned char *buf,
         params.tp_set |= 1 << TPI_MIN_ACK_DELAY;
     }
     if (settings->es_timestamps)
+    {
+        params.tp_numerics[TPI_TIMESTAMPS] = TS_GENERATE_THEM;
         params.tp_set |= 1 << TPI_TIMESTAMPS;
+    }
 
     len = (version == LSQVER_ID27 ? lsquic_tp_encode_27 : lsquic_tp_encode)(
                         &params, enc_sess->esi_flags & ESI_SERVER, buf, bufsz);
@@ -1570,15 +1573,6 @@ get_crypto_params (const struct enc_sess_iquic *enc_sess,
         return -1;
     }
 
-    /* FIXME: figure out why this duplicate check is here and either fix it
-     * or get rid of it.
-     */
-    if (key_sz > EVP_MAX_KEY_LENGTH)
-    {
-        LSQ_DEBUG("PN size %u is too large", key_sz);
-        return -1;
-    }
-
     return 0;
 }
 
@@ -1987,11 +1981,8 @@ iquic_esf_encrypt_packet (enc_session_t *enc_session_p,
 #endif
     *((uint64_t *) begin_xor) ^= packno;
 
-    /* TODO: have this call return packno_off and packno_len to avoid
-     * another function call.
-     */
     header_sz = lconn->cn_pf->pf_gen_reg_pkt_header(lconn, packet_out, dst,
-                                                                        dst_sz);
+                                            dst_sz, &packno_off, &packno_len);
     if (header_sz < 0)
         goto err;
     if (enc_level == ENC_LEV_FORW)
@@ -2017,7 +2008,6 @@ 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);
 #ifndef NDEBUG
     const unsigned sample_off = packno_off + 4;
     assert(sample_off + IQUIC_TAG_LEN <= dst_sz);

diff --git a/src/liblsquic/lsquic_frame_reader.c b/src/liblsquic/lsquic_frame_reader.c
@@ -561,8 +561,7 @@ skip_headers (struct lsquic_frame_reader *fr)
 }
 
 
-/* TODO: this function always returns 0.  Make it void */
-static int
+static void
 decode_and_pass_payload (struct lsquic_frame_reader *fr)
 {
     struct headers_state *hs = &fr->fr_state.by_type.headers_state;
@@ -588,7 +587,7 @@ decode_and_pass_payload (struct lsquic_frame_reader *fr)
         if (!target_stream)
         {
             skip_headers(fr);
-            return 0;
+            return;
         }
     }
     hset = fr->fr_hsi_if->hsi_create_header_set(fr->fr_hsi_ctx, target_stream,
@@ -688,14 +687,13 @@ decode_and_pass_payload (struct lsquic_frame_reader *fr)
     fr->fr_conn_stats->in.headers_comp += fr->fr_header_block_sz;
 #endif
 
-    return 0;
+    return;
 
   stream_error:
     LSQ_INFO("%s: stream error %u", __func__, err);
     if (hset)
         fr->fr_hsi_if->hsi_discard_header_set(hset);
     fr->fr_callbacks->frc_on_error(fr->fr_cb_ctx, fr_get_stream_id(fr), err);
-    return 0;
 }
 
 
@@ -725,13 +723,12 @@ read_headers_block_fragment (struct lsquic_frame_reader *fr)
     if (hs->nread == payload_length &&
                 (fr->fr_state.header.hfh_flags & HFHF_END_HEADERS))
     {
-        int rv = decode_and_pass_payload(fr);
+        decode_and_pass_payload(fr);
         free(fr->fr_header_block);
         fr->fr_header_block = NULL;
-        return rv;
     }
-    else
-        return 0;
+
+    return 0;
 }
 
 
@@ -865,20 +862,14 @@ read_contin (struct lsquic_frame_reader *fr)
     {
         if (fr->fr_state.header.hfh_flags & HFHF_END_HEADERS)
         {
-            int rv = decode_and_pass_payload(fr);
+            decode_and_pass_payload(fr);
             free(fr->fr_header_block);
             fr->fr_header_block = NULL;
-            reset_state(fr);
-            return rv;
-        }
-        else
-        {
-            reset_state(fr);
-            return 0;
         }
+        reset_state(fr);
     }
-    else
-        return 0;
+
+    return 0;
 }
 
 

diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c
@@ -145,6 +145,7 @@ enum more_flags
     MF_CHECK_MTU_PROBE  = 1 << 2,
     MF_IGNORE_MISSING   = 1 << 3,
     MF_CONN_CLOSE_PACK  = 1 << 4,   /* CONNECTION_CLOSE has been packetized */
+    MF_SEND_WRONG_COUNTS= 1 << 5,   /* Send wrong ECN counts to peer */
 };
 
 
@@ -1400,7 +1401,7 @@ lsquic_ietf_full_conn_server_new (struct lsquic_engine_public *enpub,
         conn->ifc_flags |= IFC_IGNORE_INIT;
 
     conn->ifc_paths[0].cop_path = imc->imc_path;
-    conn->ifc_paths[0].cop_flags = COP_VALIDATED;
+    conn->ifc_paths[0].cop_flags = COP_VALIDATED|COP_INITIALIZED;
     conn->ifc_used_paths = 1 << 0;
     if (imc->imc_flags & IMC_ADDR_VALIDATED)
         lsquic_send_ctl_path_validated(&conn->ifc_send_ctl);
@@ -1540,8 +1541,6 @@ lsquic_ietf_full_conn_server_new (struct lsquic_engine_public *enpub,
     conn->ifc_conn_ctx = conn->ifc_enpub->enp_stream_if->on_new_conn(
                         conn->ifc_enpub->enp_stream_if_ctx, &conn->ifc_conn);
 
-    /* TODO: do something if there is outgoing ACK */
-
     if (0 != handshake_ok(&conn->ifc_conn))
         goto err3;
 
@@ -1666,6 +1665,14 @@ generate_ack_frame_for_pns (struct ietf_full_conn *conn,
     if (conn->ifc_incoming_ecn
                         && lsquic_send_ctl_ecn_turned_on(&conn->ifc_send_ctl))
         ecn_counts = conn->ifc_ecn_counts_in[pns];
+    else if ((conn->ifc_mflags & MF_SEND_WRONG_COUNTS) && pns == PNS_APP)
+    {
+        /* We try once.  A more advanced version would wait until we get a
+         * packet from peer and only then stop.
+         */
+        conn->ifc_mflags &= ~MF_SEND_WRONG_COUNTS;
+        ecn_counts = conn->ifc_ecn_counts_in[pns];
+    }
     else
         ecn_counts = NULL;
 
@@ -3262,9 +3269,10 @@ handshake_ok (struct lsquic_conn *lconn)
         conn->ifc_flags |= IFC_DELAYED_ACKS;
     }
     if (conn->ifc_settings->es_timestamps
-            && (params->tp_set & (1 << TPI_TIMESTAMPS)))
+            && (params->tp_set & (1 << TPI_TIMESTAMPS))
+                && (params->tp_numerics[TPI_TIMESTAMPS] & TS_WANT_THEM))
     {
-        LSQ_DEBUG("timestamps enabled");
+        LSQ_DEBUG("timestamps enabled: will send TIMESTAMP frames");
         conn->ifc_flags |= IFC_TIMESTAMPS;
     }
 
@@ -5909,29 +5917,9 @@ static unsigned
 process_timestamp_frame (struct ietf_full_conn *conn,
     struct lsquic_packet_in *packet_in, const unsigned char *p, size_t len)
 {
-    uint64_t timestamp;
-    int parsed_len;
-
-    if (!(conn->ifc_flags & IFC_TIMESTAMPS))
-    {
-        ABORT_QUIETLY(0, TEC_PROTOCOL_VIOLATION,
+    ABORT_QUIETLY(0, TEC_PROTOCOL_VIOLATION,
             "Received unexpected TIMESTAMP frame (not negotiated)");
-        return 0;
-    }
-
-    parsed_len = conn->ifc_conn.cn_pf->pf_parse_timestamp_frame(p, len,
-                                                                &timestamp);
-    if (parsed_len < 0)
-        return 0;
-
-    timestamp <<= conn->ifc_cfg.ack_exp;
-    EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "TIMESTAMP(%"PRIu64" us)", timestamp);
-    LSQ_DEBUG("TIMESTAMP(%"PRIu64" us) (%"PRIu64" << %"PRIu8")", timestamp,
-        timestamp >> conn->ifc_cfg.ack_exp, conn->ifc_cfg.ack_exp);
-
-    /* We don't do anything with the timestamp */
-
-    return parsed_len;
+    return 0;
 }
 
 
@@ -6167,6 +6155,16 @@ many_in_and_will_write (struct ietf_full_conn *conn)
 }
 
 
+static void
+force_queueing_ack_app (struct ietf_full_conn *conn)
+{
+    lsquic_alarmset_unset(&conn->ifc_alset, AL_ACK_APP);
+    lsquic_send_ctl_sanity_check(&conn->ifc_send_ctl);
+    conn->ifc_flags |= IFC_ACK_QUED_APP;
+    LSQ_DEBUG("force-queued ACK");
+}
+
+
 static void
 try_queueing_ack_app (struct ietf_full_conn *conn,
                                 int was_missing, int ecn, lsquic_time_t now)
@@ -7184,6 +7182,22 @@ ietf_full_conn_ci_retx_timeout (struct lsquic_conn *lconn)
         lsquic_send_ctl_resize(&conn->ifc_send_ctl);
     else
         LSQ_DEBUG("RTO occurred, but no MTUs to reset");
+
+    if (lsquic_send_ctl_ecn_turned_on(&conn->ifc_send_ctl))
+    {
+        LSQ_INFO("RTO occurred, disable ECN");
+        lsquic_send_ctl_disable_ecn(&conn->ifc_send_ctl);
+        if (lsquic_rechist_first(&conn->ifc_rechist[PNS_APP]))
+        {
+            LSQ_DEBUG("Send wrong ECN counts to peer so that it turns off "
+                                                                "ECN as well");
+            memset(conn->ifc_ecn_counts_in[PNS_APP], 0,
+                                    sizeof(conn->ifc_ecn_counts_in[PNS_APP]));
+            conn->ifc_mflags |= MF_SEND_WRONG_COUNTS;
+            force_queueing_ack_app(conn);
+            conn->ifc_send_flags |= SF_SEND_PING;
+        }
+    }
 }
 
 
@@ -7878,13 +7892,13 @@ cancel_push_promise (struct ietf_full_conn *conn, struct push_promise *promise)
     LSQ_DEBUG("cancel promise %"PRIu64, promise->pp_id);
     /* Remove promise from hash to prevent multiple cancellations */
     lsquic_hash_erase(conn->ifc_pub.u.ietf.promises, &promise->pp_hash_id);
-    lsquic_pp_put(promise, conn->ifc_pub.u.ietf.promises);
     /* But let stream dtor free the promise object as sm_promise may yet
      * be used by the stream in some ways.
      */
     lsquic_stream_shutdown_internal(promise->pp_pushed_stream);
     if (0 != lsquic_hcso_write_cancel_push(&conn->ifc_hcso, promise->pp_id))
         ABORT_WARN("cannot write CANCEL_PUSH");
+    lsquic_pp_put(promise, conn->ifc_pub.u.ietf.promises);
 }
 
 

diff --git a/src/liblsquic/lsquic_handshake.c b/src/liblsquic/lsquic_handshake.c
@@ -3625,7 +3625,7 @@ gquic_really_encrypt_packet (struct lsquic_enc_session *enc_session,
     unsigned char header_buf[GQUIC_MAX_PUBHDR_SZ];
 
     header_sz = lconn->cn_pf->pf_gen_reg_pkt_header(lconn, packet_out,
-                                            header_buf, sizeof(header_buf));
+                                header_buf, sizeof(header_buf), NULL, NULL);
     if (header_sz < 0)
         return -1;
 
@@ -3963,7 +3963,7 @@ gquic2_esf_encrypt_packet (enc_session_t *enc_session_p,
     *((uint64_t *) begin_xor) ^= packno;
 
     header_sz = lconn->cn_pf->pf_gen_reg_pkt_header(lconn, packet_out, dst,
-                                                                        dst_sz);
+                                            dst_sz, &packno_off, &packno_len);
     if (header_sz < 0)
         goto err;
 
@@ -3987,7 +3987,6 @@ gquic2_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);
     if (!packet_out->po_nonce)
         divers_nonce_len = 0;
     else