From e2212b20bcf96c62c17a5e124c3bd61a98b8fcfd Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Fri, 16 Dec 2022 10:53:02 +0000 Subject: [PATCH] QUIC ACKM: Rework probe reporting to allow use for bookkeeping Reviewed-by: Matt Caswell Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/19925) --- .../quic-design/connection-state-machine.md | 2 +- include/internal/quic_ackm.h | 17 +++++++++++++++-- ssl/quic/quic_ackm.c | 12 +++--------- test/quic_ackm_test.c | 11 +++++------ 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/doc/designs/quic-design/connection-state-machine.md b/doc/designs/quic-design/connection-state-machine.md index a148f71c62e..beb56093546 100644 --- 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 2d5de7fd641..4bdf15ff1e2 100644 --- 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 2c017574940..7e692a27ee2 100644 --- 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 6036d0d0a91..aee9f1c40ae 100644 --- 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)) -- 2.47.2