QUIC ACKM: Clarify the role of is_inflight
authorHugo Landau <hlandau@openssl.org>
Tue, 6 Jun 2023 15:25:10 +0000 (16:25 +0100)
committerPauli <pauli@openssl.org>
Sun, 16 Jul 2023 22:17:57 +0000 (08:17 +1000)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21135)

include/internal/quic_ackm.h
ssl/quic/quic_ackm.c

index 7574c97a964b57a0a269fa158d869a4efb33d591..ca0c1aab1f0e7487941062384329dd3f94312a92 100644 (file)
@@ -67,10 +67,19 @@ struct ossl_ackm_tx_pkt_st {
      */
     unsigned int pkt_space :2;
 
-    /* 1 if the packet is in flight. */
+    /*
+     * 1 if the packet is in flight. A packet is considered 'in flight' if it is
+     * counted for purposes of congestion control and 'bytes in flight' counts.
+     * Most packets are considered in flight. The only circumstance where a
+     * numbered packet is not considered in flight is if it contains only ACK
+     * frames (not even PADDING frames), as these frames can bypass CC.
+     */
     unsigned int is_inflight :1;
 
-    /* 1 if the packet has one or more ACK-eliciting frames. */
+    /*
+     * 1 if the packet has one or more ACK-eliciting frames.
+     * Note that if this is set, is_inflight must be set.
+     */
     unsigned int is_ack_eliciting :1;
 
     /* 1 if the packet is a PTO probe. */
index 777d71ce53a6d16a92c0c9e9707e1094cc1f0bbe..a6c8ebef1fc88eb9a7d0d5f1cf345fb07f73ebc5 100644 (file)
@@ -1068,6 +1068,10 @@ int ossl_ackm_on_tx_packet(OSSL_ACKM *ackm, OSSL_ACKM_TX_PKT *pkt)
     if (pkt->num_bytes == 0)
         return 0;
 
+    /* Does not make any sense for a non-in-flight packet to be ACK-eliciting. */
+    if (!pkt->is_inflight && pkt->is_ack_eliciting)
+        return 0;
+
     if (tx_pkt_history_add(h, pkt) == 0)
         return 0;