QUIC ACKM: Rework probe reporting to allow use for bookkeeping

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19925)

diff --git a/doc/designs/quic-design/connection-state-machine.md b/doc/designs/quic-design/connection-state-machine.md
index a148f71c62e631301d62073bb716ed5a23697c9c..beb560935461db0444016a9eb186496ba9c8a33a 100644 (file)
--- a/doc/designs/quic-design/connection-state-machine.md
+++ b/doc/designs/quic-design/connection-state-machine.md
@@ -225,7 +225,7 @@ The above states and their substates are defined as follows:
 We express this state machine in more concrete form in the form of a table,
 which makes the available transitions clear:
 
-† Except where superceded by a more specific transition
+† Except where superseded by a more specific transition
 
 ε means “where no other transition is applicable”.
 

diff --git a/include/internal/quic_ackm.h b/include/internal/quic_ackm.h
index 2d5de7fd6412f7f2bbcd7d738f867eb000cd1426..4bdf15ff1e28e7743fadb4a738abfbea951a2bf3 100644 (file)
--- a/include/internal/quic_ackm.h
+++ b/include/internal/quic_ackm.h
@@ -227,8 +227,21 @@ typedef struct ossl_ackm_probe_info_st {
     uint32_t pto[QUIC_PN_SPACE_NUM];
 } OSSL_ACKM_PROBE_INFO;
 
-int ossl_ackm_get_probe_request(OSSL_ACKM *ackm, int clear,
-                                OSSL_ACKM_PROBE_INFO *info);
+/*
+ * Returns a pointer to a structure counting any pending probe requests which
+ * have been generated by the ACKM. The fields in the structure are incremented
+ * by one every time the ACKM wants another probe of the given type to be sent.
+ * If the ACKM thinks two packets should be generated for a probe, it will
+ * increment the field twice (TODO).
+ *
+ * It is permissible for the caller to decrement or zero these fields to keep
+ * track of when it has generated a probe as asked. The returned structure
+ * has the same lifetime as the ACKM.
+ *
+ * This function should be called after calling e.g. ossl_ackm_on_timeout
+ * to determine if any probe requests have been generated.
+ */
+OSSL_ACKM_PROBE_INFO *ossl_ackm_get_probe_request(OSSL_ACKM *ackm);
 
 int ossl_ackm_get_largest_unacked(OSSL_ACKM *ackm, int pkt_space, QUIC_PN *pn);
 

diff --git a/ssl/quic/quic_ackm.c b/ssl/quic/quic_ackm.c
index 2c01757494096abe36c0b2fdb324bac48c5ef892..7e692a27ee2fc7d7ada981f2f8d23ba6051ae44d 100644 (file)
--- a/ssl/quic/quic_ackm.c
+++ b/ssl/quic/quic_ackm.c
@@ -943,7 +943,7 @@ static void ackm_on_pkts_lost(OSSL_ACKM *ackm, int pkt_space,
 
     if (pseudo)
         /*
-         * If this is psuedo-loss (e.g. during connection retry) we do not
+         * If this is pseudo-loss (e.g. during connection retry) we do not
          * inform the CC as it is not a real loss and not reflective of network
          * conditions.
          */
@@ -1316,15 +1316,9 @@ OSSL_TIME ossl_ackm_get_loss_detection_deadline(OSSL_ACKM *ackm)
     return ackm->loss_detection_deadline;
 }
 
-int ossl_ackm_get_probe_request(OSSL_ACKM *ackm, int clear,
-                                OSSL_ACKM_PROBE_INFO *info)
+OSSL_ACKM_PROBE_INFO *ossl_ackm_get_probe_request(OSSL_ACKM *ackm)
 {
-    *info = ackm->pending_probe;
-
-    if (clear != 0)
-        memset(&ackm->pending_probe, 0, sizeof(ackm->pending_probe));
-
-    return 1;
+    return &ackm->pending_probe;
 }
 
 int ossl_ackm_get_largest_unacked(OSSL_ACKM *ackm, int pkt_space, QUIC_PN *pn)

diff --git a/test/quic_ackm_test.c b/test/quic_ackm_test.c
index 6036d0d0a912eed8c7a02724514fc5ed8ccad9e5..aee9f1c40ae033581fb19cb1d122b0b2eab0248c 100644 (file)
--- a/test/quic_ackm_test.c
+++ b/test/quic_ackm_test.c
@@ -415,7 +415,7 @@ static int test_tx_ack_case_actual(int tidx, int space, int mode)
         }
     } else if (mode == MODE_PTO) {
         OSSL_TIME deadline = ossl_ackm_get_loss_detection_deadline(h.ackm);
-        OSSL_ACKM_PROBE_INFO probe = {0};
+        OSSL_ACKM_PROBE_INFO probe;
 
         if (!TEST_int_eq(ossl_time_compare(deadline, loss_detection_deadline), 0))
             goto err;
@@ -425,9 +425,7 @@ static int test_tx_ack_case_actual(int tidx, int space, int mode)
             goto err;
 
         /* Should not have any probe requests yet. */
-        if (!TEST_int_eq(ossl_ackm_get_probe_request(h.ackm, 0, &probe), 1))
-            goto err;
-
+        probe = *ossl_ackm_get_probe_request(h.ackm);
         if (!TEST_int_eq(test_probe_counts(&probe, 0, 0, 0, 0, 0), 1))
             goto err;
 
@@ -447,8 +445,9 @@ static int test_tx_ack_case_actual(int tidx, int space, int mode)
 
         /* Should have a probe request. Not cleared by first call. */
         for (i = 0; i < 3; ++i) {
-            if (!TEST_int_eq(ossl_ackm_get_probe_request(h.ackm, i > 0, &probe), 1))
-                goto err;
+            probe = *ossl_ackm_get_probe_request(h.ackm);
+            if (i > 0)
+                memset(ossl_ackm_get_probe_request(h.ackm), 0, sizeof(probe));
 
             if (i == 2) {
                 if (!TEST_int_eq(test_probe_counts(&probe, 0, 0, 0, 0, 0), 1))