Skip to content

Commit

Permalink
Keep track of connection credit as we add stream data
Browse files Browse the repository at this point in the history 
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 openssl#22706
  • Loading branch information
@mattcaswell
mattcaswell committed Nov 13, 2023
1 parent 9e75a0b commit daf7e7c
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 28 deletions.
10 changes: 6 additions & 4 deletions include/internal/quic_fc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions ssl/quic/quic_fc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion ssl/quic/quic_stream_map.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
18 changes: 12 additions & 6 deletions ssl/quic/quic_txp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

/*
Expand Down Expand Up @@ -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;) {
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 10 additions & 10 deletions test/quic_fc_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -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)
Expand All @@ -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)))
Expand Down

0 comments on commit daf7e7c

Please sign in to comment.