]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Keep track of connection credit as we add stream data
authorMatt Caswell <matt@openssl.org>
Mon, 13 Nov 2023 14:16:57 +0000 (14:16 +0000)
committerTomas Mraz <tomas@openssl.org>
Wed, 15 Nov 2023 08:08:16 +0000 (09:08 +0100)
If a single packet contains data from multiple streams we need to keep track
of the cummulative connection level credit consumed across all of the
streams. Once the connection level credit has been consumed we must stop
adding stream data.

Fixes #22706

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22718)

include/internal/quic_fc.h
ssl/quic/quic_fc.c
ssl/quic/quic_stream_map.c
ssl/quic/quic_txp.c
test/quic_fc_test.c

index 7a8273d54288f03215e2e51596a1d7aa668a76e1..49b448a3a489e72da0b4128529e10d7e98e0841a 100644 (file)
@@ -61,16 +61,18 @@ int ossl_quic_txfc_bump_cwm(QUIC_TXFC *txfc, uint64_t cwm);
  *
  * If called on a stream-level TXFC, ossl_quic_txfc_get_credit is called on
  * the connection-level TXFC as well, and the lesser of the two values is
- * returned.
+ * returned. The consumed value is the amount already consumed on the connection
+ * level TXFC.
  */
-uint64_t ossl_quic_txfc_get_credit(QUIC_TXFC *txfc);
+uint64_t ossl_quic_txfc_get_credit(QUIC_TXFC *txfc, uint64_t consumed);
 
 /*
  * Like ossl_quic_txfc_get_credit(), but when called on a stream-level TXFC,
  * retrieves only the stream-level credit value and does not clamp it based on
- * connection-level flow control.
+ * connection-level flow control. Any credit value is reduced by the consumed
+ * amount.
  */
-uint64_t ossl_quic_txfc_get_credit_local(QUIC_TXFC *txfc);
+uint64_t ossl_quic_txfc_get_credit_local(QUIC_TXFC *txfc, uint64_t consumed);
 
 /*
  * Consume num_bytes of credit. This is the 'On TX' operation. This should be
index 1a9c5890f80a985c81dafbb6f0206d1db29e97b1..750e896306f7e9d123c00f12652ae019342cebe0 100644 (file)
@@ -46,21 +46,21 @@ int ossl_quic_txfc_bump_cwm(QUIC_TXFC *txfc, uint64_t cwm)
     return 1;
 }
 
-uint64_t ossl_quic_txfc_get_credit_local(QUIC_TXFC *txfc)
+uint64_t ossl_quic_txfc_get_credit_local(QUIC_TXFC *txfc, uint64_t consumed)
 {
-    assert(txfc->swm <= txfc->cwm);
-    return txfc->cwm - txfc->swm;
+    assert((txfc->swm + consumed) <= txfc->cwm);
+    return txfc->cwm - (consumed + txfc->swm);
 }
 
-uint64_t ossl_quic_txfc_get_credit(QUIC_TXFC *txfc)
+uint64_t ossl_quic_txfc_get_credit(QUIC_TXFC *txfc, uint64_t consumed)
 {
     uint64_t r, conn_r;
 
-    r = ossl_quic_txfc_get_credit_local(txfc);
+    r = ossl_quic_txfc_get_credit_local(txfc, 0);
 
     if (txfc->parent != NULL) {
         assert(txfc->parent->parent == NULL);
-        conn_r = ossl_quic_txfc_get_credit_local(txfc->parent);
+        conn_r = ossl_quic_txfc_get_credit_local(txfc->parent, consumed);
         if (conn_r < r)
             r = conn_r;
     }
@@ -71,7 +71,7 @@ uint64_t ossl_quic_txfc_get_credit(QUIC_TXFC *txfc)
 int ossl_quic_txfc_consume_credit_local(QUIC_TXFC *txfc, uint64_t num_bytes)
 {
     int ok = 1;
-    uint64_t credit = ossl_quic_txfc_get_credit_local(txfc);
+    uint64_t credit = ossl_quic_txfc_get_credit_local(txfc, 0);
 
     if (num_bytes > credit) {
         ok = 0;
index 0f41b03da58d61d2ad44ade4aa99ccee5c245107..f8278c9913239c537863e6b06b42e4081b5cd515 100644 (file)
@@ -269,7 +269,7 @@ static int stream_has_data_to_send(QUIC_STREAM *s)
                                             &num_iov))
         return 0;
 
-    fc_credit = ossl_quic_txfc_get_credit(&s->txfc);
+    fc_credit = ossl_quic_txfc_get_credit(&s->txfc, 0);
     fc_swm    = ossl_quic_txfc_get_swm(&s->txfc);
     fc_limit  = fc_swm + fc_credit;
 
index 5500c9b3f60fcac8768e3c4167ea0e9d5e5a242e..f26f1e81a1bf6e2d3b5023a2fc1284878fb8d3a1 100644 (file)
@@ -2111,7 +2111,8 @@ static int txp_plan_stream_chunk(OSSL_QUIC_TX_PACKETISER *txp,
                                  QUIC_SSTREAM *sstream,
                                  QUIC_TXFC *stream_txfc,
                                  size_t skip,
-                                 struct chunk_info *chunk)
+                                 struct chunk_info *chunk,
+                                 uint64_t consumed)
 {
     uint64_t fc_credit, fc_swm, fc_limit;
 
@@ -2130,7 +2131,7 @@ static int txp_plan_stream_chunk(OSSL_QUIC_TX_PACKETISER *txp,
     chunk->orig_len = chunk->shdr.len;
 
     /* Clamp according to connection and stream-level TXFC. */
-    fc_credit   = ossl_quic_txfc_get_credit(stream_txfc);
+    fc_credit   = ossl_quic_txfc_get_credit(stream_txfc, consumed);
     fc_swm      = ossl_quic_txfc_get_swm(stream_txfc);
     fc_limit    = fc_swm + fc_credit;
 
@@ -2166,7 +2167,8 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
                                       QUIC_STREAM *next_stream,
                                       int *have_ack_eliciting,
                                       int *packet_full,
-                                      uint64_t *new_credit_consumed)
+                                      uint64_t *new_credit_consumed,
+                                      uint64_t conn_consumed)
 {
     int rc = 0;
     struct chunk_info chunks[2] = {0};
@@ -2194,7 +2196,8 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
      * determining when we can use an implicit length in a STREAM frame.
      */
     for (i = 0; i < 2; ++i) {
-        if (!txp_plan_stream_chunk(txp, h, sstream, stream_txfc, i, &chunks[i]))
+        if (!txp_plan_stream_chunk(txp, h, sstream, stream_txfc, i, &chunks[i],
+                                   conn_consumed))
             goto err;
 
         if (i == 0 && !chunks[i].valid) {
@@ -2232,7 +2235,7 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
         if (i > 0)
             /* Load next chunk for lookahead. */
             if (!txp_plan_stream_chunk(txp, h, sstream, stream_txfc, i + 1,
-                                       &chunks[(i + 1) % 2]))
+                                       &chunks[(i + 1) % 2], conn_consumed))
                 goto err;
 
         /*
@@ -2382,6 +2385,7 @@ static int txp_generate_stream_related(OSSL_QUIC_TX_PACKETISER *txp,
     uint64_t cwm;
     QUIC_STREAM *stream, *snext;
     struct tx_helper *h = &pkt->h;
+    uint64_t conn_consumed = 0;
 
     for (ossl_quic_stream_iter_init(&it, txp->args.qsm, 1);
          it.stream != NULL;) {
@@ -2517,11 +2521,13 @@ static int txp_generate_stream_related(OSSL_QUIC_TX_PACKETISER *txp,
                                             snext,
                                             have_ack_eliciting,
                                             &packet_full,
-                                            &stream->txp_txfc_new_credit_consumed)) {
+                                            &stream->txp_txfc_new_credit_consumed,
+                                            conn_consumed)) {
                 /* Fatal error (allocation, etc.) */
                 txp_enlink_tmp(tmp_head, stream);
                 return 0;
             }
+            conn_consumed += stream->txp_txfc_new_credit_consumed;
 
             if (packet_full) {
                 txp_enlink_tmp(tmp_head, stream);
index e624d81b7344c1614f092d9c49f4749f05688cbc..d2797667569a0b15a260f43a734f710c58b2924f 100644 (file)
@@ -37,10 +37,10 @@ static int test_txfc(int is_stream)
     if (!TEST_uint64_t_eq(ossl_quic_txfc_get_cwm(txfc), 2000))
         goto err;
 
-    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc), 2000))
+    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc, 0), 2000))
         goto err;
 
-    if (is_stream && !TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc),
+    if (is_stream && !TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc, 0),
                                        2000))
         goto err;
 
@@ -50,10 +50,10 @@ static int test_txfc(int is_stream)
     if (!TEST_true(ossl_quic_txfc_consume_credit(txfc, 500)))
         goto err;
 
-    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc), 1500))
+    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc, 0), 1500))
         goto err;
 
-    if (is_stream && !TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc),
+    if (is_stream && !TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc, 0),
                                        1500))
         goto err;
 
@@ -69,10 +69,10 @@ static int test_txfc(int is_stream)
     if (!TEST_uint64_t_eq(ossl_quic_txfc_get_swm(txfc), 600))
         goto err;
 
-    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc), 1400))
+    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc, 0), 1400))
         goto err;
 
-    if (is_stream && !TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc),
+    if (is_stream && !TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc, 0),
                                        1400))
         goto err;
 
@@ -82,10 +82,10 @@ static int test_txfc(int is_stream)
     if (!TEST_true(ossl_quic_txfc_consume_credit(txfc, 1400)))
         goto err;
 
-    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc), 0))
+    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc, 0), 0))
         goto err;
 
-    if (is_stream && !TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc),
+    if (is_stream && !TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc, 0),
                                        0))
         goto err;
 
@@ -131,7 +131,7 @@ static int test_txfc(int is_stream)
     if (!TEST_uint64_t_eq(ossl_quic_txfc_get_swm(txfc), 2000))
         goto err;
 
-    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc), 500))
+    if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit_local(txfc, 0), 500))
         goto err;
 
     if (is_stream)
@@ -144,7 +144,7 @@ static int test_txfc(int is_stream)
         if (!TEST_false(ossl_quic_txfc_has_become_blocked(txfc, 0)))
             goto err;
 
-        if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc), 1))
+        if (!TEST_uint64_t_eq(ossl_quic_txfc_get_credit(txfc, 0), 1))
             goto err;
 
         if (!TEST_true(ossl_quic_txfc_consume_credit(txfc, 1)))