From: Hugo Landau Date: Fri, 21 Apr 2023 09:56:48 +0000 (+0100) Subject: QUIC CC: Use OSSL_PARAM X-Git-Tag: openssl-3.2.0-alpha1~929 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=878df9be67df14c90ef584e5762a8c1f5c8f9749;p=thirdparty%2Fopenssl.git QUIC CC: Use OSSL_PARAM Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/20423) --- diff --git a/include/internal/quic_cc.h b/include/internal/quic_cc.h index af68e6dd08d..d26ea55da55 100644 --- a/include/internal/quic_cc.h +++ b/include/internal/quic_cc.h @@ -41,19 +41,19 @@ typedef struct ossl_cc_ecn_info_st { } OSSL_CC_ECN_INFO; /* Parameter (read-write): Maximum datagram payload length in bytes. */ -#define OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN 1 +#define OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN "max_dgram_payload_len" /* Diagnostic (read-only): current congestion window size in bytes. */ -#define OSSL_CC_OPTION_CUR_CWND_SIZE 2 +#define OSSL_CC_OPTION_CUR_CWND_SIZE "cur_cwnd_size" /* Diagnostic (read-only): minimum congestion window size in bytes. */ -#define OSSL_CC_OPTION_MIN_CWND_SIZE 3 +#define OSSL_CC_OPTION_MIN_CWND_SIZE "min_cwnd_size" /* Diagnostic (read-only): current net bytes in flight. */ -#define OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT 4 +#define OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT "bytes_in_flight" /* Diagnostic (read-only): method-specific state value. */ -#define OSSL_CC_OPTION_CUR_STATE 5 +#define OSSL_CC_OPTION_CUR_STATE "cur_state" /* * Congestion control abstract interface. @@ -77,7 +77,6 @@ typedef struct ossl_cc_ecn_info_st { * * All of these changes are intended to avoid having a congestion controller * have to access ACKM internal state. - * */ #define OSSL_CC_LOST_FLAG_PERSISTENT_CONGESTION (1U << 0) @@ -98,23 +97,41 @@ typedef struct ossl_cc_method_st { /* * Escape hatch for option configuration. * - * option_id: One of OSSL_CC_OPTION_*. + * params is an array of OSSL_PARAM structures. * - * value: The option value to set. + * Returns 1 on success and 0 on failure. + */ + int (*set_input_params)(OSSL_CC_DATA *ccdata, + const OSSL_PARAM *params); + + /* + * (Re)bind output (diagnostic) information. + * + * params is an array of OSSL_PARAM structures used to output values. The + * storage locations associated with each parameter are stored internally + * and updated whenever the state of the congestion controller is updated; + * thus, the storage locations associated with the OSSL_PARAMs passed in the + * call to this function must remain valid until the congestion controller + * is freed or those parameters are unbound. A given parameter name may be + * bound to only one location at a time. The params structures themselves + * do not need to remain allocated after this call returns. * * Returns 1 on success and 0 on failure. */ - int (*set_option_uint)(OSSL_CC_DATA *ccdata, - uint32_t option_id, - uint64_t value); + int (*bind_diagnostics)(OSSL_CC_DATA *ccdata, + OSSL_PARAM *params); /* - * On success, returns 1 and writes the current value of the given option to - * *value. Otherwise, returns 0. + * Unbind diagnostic information. The parameters with the given names are + * unbound, cancelling the effects of a previous call to bind_diagnostic(). + * params is an array of OSSL_PARAMs. The values of the parameters are + * ignored. If a parameter is already unbound, there is no effect for that + * parameter but other parameters are still unbound. + * + * Returns 1 on success or 0 on failure. */ - int (*get_option_uint)(OSSL_CC_DATA *ccdata, - uint32_t option_id, - uint64_t *value); + int (*unbind_diagnostics)(OSSL_CC_DATA *ccdata, + OSSL_PARAM *params); /* * Returns the amount of additional data (above and beyond the data diff --git a/ssl/quic/cc_newreno.c b/ssl/quic/cc_newreno.c index c23b8d69215..850ff9899a8 100644 --- a/ssl/quic/cc_newreno.c +++ b/ssl/quic/cc_newreno.c @@ -25,6 +25,13 @@ typedef struct ossl_cc_newreno_st { /* Diagnostic state. */ int in_congestion_recovery; + + /* Diagnostic output locations. */ + size_t *p_diag_max_dgram_payload_len; + uint64_t *p_diag_cur_cwnd_size; + uint64_t *p_diag_min_cwnd_size; + uint64_t *p_diag_cur_bytes_in_flight; + uint32_t *p_diag_cur_state; } OSSL_CC_NEWRENO; #define MIN_MAX_INIT_WND_SIZE 14720 /* RFC 9002 s. 7.2 */ @@ -33,6 +40,7 @@ typedef struct ossl_cc_newreno_st { static void newreno_set_max_dgram_size(OSSL_CC_NEWRENO *nr, size_t max_dgram_size); +static void newreno_update_diag(OSSL_CC_NEWRENO *nr); static void newreno_reset(OSSL_CC_DATA *cc); @@ -78,6 +86,8 @@ static void newreno_set_max_dgram_size(OSSL_CC_NEWRENO *nr, if (is_reduced) nr->cong_wnd = nr->k_init_wnd; + + newreno_update_diag(nr); } static void newreno_reset(OSSL_CC_DATA *cc) @@ -99,57 +109,130 @@ static void newreno_reset(OSSL_CC_DATA *cc) nr->in_congestion_recovery = 0; } -static int newreno_set_option_uint(OSSL_CC_DATA *cc, uint32_t option_id, - uint64_t value) +static int newreno_set_input_params(OSSL_CC_DATA *cc, const OSSL_PARAM *params) { OSSL_CC_NEWRENO *nr = (OSSL_CC_NEWRENO *)cc; + const OSSL_PARAM *p; + size_t value; - switch (option_id) { - case OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN: - if (value > SIZE_MAX || value < QUIC_MIN_INITIAL_DGRAM_LEN) + p = OSSL_PARAM_locate_const(params, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN); + if (p != NULL) { + if (!OSSL_PARAM_get_size_t(p, &value)) + return 0; + if (value < QUIC_MIN_INITIAL_DGRAM_LEN) return 0; - newreno_set_max_dgram_size(nr, (size_t)value); + newreno_set_max_dgram_size(nr, value); + } + + return 1; +} + +static int bind_diag(OSSL_PARAM *params, const char *param_name, size_t len, + void **pp) +{ + const OSSL_PARAM *p = OSSL_PARAM_locate_const(params, param_name); + + *pp = NULL; + + if (p == NULL) return 1; - default: + if (p->data_type != OSSL_PARAM_UNSIGNED_INTEGER + || p->data_size != len) return 0; - } + + *pp = p->data; + return 1; } -static int newreno_get_option_uint(OSSL_CC_DATA *cc, uint32_t option_id, - uint64_t *value) +static int newreno_bind_diagnostic(OSSL_CC_DATA *cc, OSSL_PARAM *params) { OSSL_CC_NEWRENO *nr = (OSSL_CC_NEWRENO *)cc; + size_t *new_p_max_dgram_payload_len; + uint64_t *new_p_cur_cwnd_size; + uint64_t *new_p_min_cwnd_size; + uint64_t *new_p_cur_bytes_in_flight; + uint32_t *new_p_cur_state; + + if (!bind_diag(params, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN, + sizeof(size_t), (void **)&new_p_max_dgram_payload_len) + || !bind_diag(params, OSSL_CC_OPTION_CUR_CWND_SIZE, + sizeof(uint64_t), (void **)&new_p_cur_cwnd_size) + || !bind_diag(params, OSSL_CC_OPTION_MIN_CWND_SIZE, + sizeof(uint64_t), (void **)&new_p_min_cwnd_size) + || !bind_diag(params, OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT, + sizeof(uint64_t), (void **)&new_p_cur_bytes_in_flight) + || !bind_diag(params, OSSL_CC_OPTION_CUR_STATE, + sizeof(uint32_t), (void **)&new_p_cur_state)) + return 0; - switch (option_id) { - case OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN: - *value = (uint64_t)nr->max_dgram_size; - return 1; + if (new_p_max_dgram_payload_len != NULL) + nr->p_diag_max_dgram_payload_len = new_p_max_dgram_payload_len; - case OSSL_CC_OPTION_CUR_CWND_SIZE: - *value = nr->cong_wnd; - return 1; + if (new_p_cur_cwnd_size != NULL) + nr->p_diag_cur_cwnd_size = new_p_cur_cwnd_size; - case OSSL_CC_OPTION_MIN_CWND_SIZE: - *value = nr->k_min_wnd; - return 1; + if (new_p_min_cwnd_size != NULL) + nr->p_diag_min_cwnd_size = new_p_min_cwnd_size; - case OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT: - *value = nr->bytes_in_flight; - return 1; + if (new_p_cur_bytes_in_flight != NULL) + nr->p_diag_cur_bytes_in_flight = new_p_cur_bytes_in_flight; + + if (new_p_cur_state != NULL) + nr->p_diag_cur_state = new_p_cur_state; + + newreno_update_diag(nr); + return 1; +} + +static void unbind_diag(OSSL_PARAM *params, const char *param_name, + void **pp) +{ + const OSSL_PARAM *p = OSSL_PARAM_locate_const(params, param_name); + + if (p != NULL) + *pp = NULL; +} + +static int newreno_unbind_diagnostic(OSSL_CC_DATA *cc, OSSL_PARAM *params) +{ + OSSL_CC_NEWRENO *nr = (OSSL_CC_NEWRENO *)cc; + + unbind_diag(params, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN, + (void **)&nr->p_diag_max_dgram_payload_len); + unbind_diag(params, OSSL_CC_OPTION_CUR_CWND_SIZE, + (void **)&nr->p_diag_cur_cwnd_size); + unbind_diag(params, OSSL_CC_OPTION_MIN_CWND_SIZE, + (void **)&nr->p_diag_min_cwnd_size); + unbind_diag(params, OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT, + (void **)&nr->p_diag_cur_bytes_in_flight); + unbind_diag(params, OSSL_CC_OPTION_CUR_STATE, + (void **)&nr->p_diag_cur_state); + return 1; +} + +static void newreno_update_diag(OSSL_CC_NEWRENO *nr) +{ + if (nr->p_diag_max_dgram_payload_len != NULL) + *nr->p_diag_max_dgram_payload_len = nr->max_dgram_size; + + if (nr->p_diag_cur_cwnd_size != NULL) + *nr->p_diag_cur_cwnd_size = nr->cong_wnd; - case OSSL_CC_OPTION_CUR_STATE: + if (nr->p_diag_min_cwnd_size != NULL) + *nr->p_diag_min_cwnd_size = nr->k_min_wnd; + + if (nr->p_diag_cur_bytes_in_flight != NULL) + *nr->p_diag_cur_bytes_in_flight = nr->bytes_in_flight; + + if (nr->p_diag_cur_state != NULL) { if (nr->in_congestion_recovery) - *value = 'R'; + *nr->p_diag_cur_state = 'R'; else if (nr->cong_wnd < nr->slow_start_thresh) - *value = 'S'; + *nr->p_diag_cur_state = 'S'; else - *value = 'A'; - return 1; - - default: - return 0; + *nr->p_diag_cur_state = 'A'; } } @@ -198,6 +281,7 @@ static void newreno_flush(OSSL_CC_NEWRENO *nr, uint32_t flags) } nr->processing_loss = 0; + newreno_update_diag(nr); } static uint64_t newreno_get_tx_allowance(OSSL_CC_DATA *cc) @@ -224,6 +308,7 @@ static int newreno_on_data_sent(OSSL_CC_DATA *cc, uint64_t num_bytes) OSSL_CC_NEWRENO *nr = (OSSL_CC_NEWRENO *)cc; nr->bytes_in_flight += num_bytes; + newreno_update_diag(nr); return 1; } @@ -268,7 +353,7 @@ static int newreno_on_data_acked(OSSL_CC_DATA *cc, * congestion window. */ if (!newreno_is_cong_limited(nr)) - return 1; + goto out; /* * We can handle acknowledgement of a packet in one of three ways @@ -286,12 +371,10 @@ static int newreno_on_data_acked(OSSL_CC_DATA *cc, */ if (newreno_in_cong_recovery(nr, info->tx_time)) { /* Congestion recovery, do nothing. */ - return 1; } else if (nr->cong_wnd < nr->slow_start_thresh) { /* When this condition is true we are in the Slow Start state. */ nr->cong_wnd += info->tx_size; nr->in_congestion_recovery = 0; - return 1; } else { /* Otherwise, we are in the Congestion Avoidance state. */ nr->bytes_acked += info->tx_size; @@ -305,8 +388,11 @@ static int newreno_on_data_acked(OSSL_CC_DATA *cc, } nr->in_congestion_recovery = 0; - return 1; } + +out: + newreno_update_diag(nr); + return 1; } static int newreno_on_data_lost(OSSL_CC_DATA *cc, @@ -328,7 +414,7 @@ static int newreno_on_data_lost(OSSL_CC_DATA *cc, * packet at a time s < t, as we've effectively already signalled * congestion on loss of that and subsequent packets. */ - return 1; + goto out; nr->processing_loss = 1; @@ -340,6 +426,9 @@ static int newreno_on_data_lost(OSSL_CC_DATA *cc, nr->tx_time_of_last_loss = ossl_time_max(nr->tx_time_of_last_loss, info->tx_time); + +out: + newreno_update_diag(nr); return 1; } @@ -357,6 +446,7 @@ static int newreno_on_data_invalidated(OSSL_CC_DATA *cc, OSSL_CC_NEWRENO *nr = (OSSL_CC_NEWRENO *)cc; nr->bytes_in_flight -= num_bytes; + newreno_update_diag(nr); return 1; } @@ -376,8 +466,9 @@ const OSSL_CC_METHOD ossl_cc_newreno_method = { newreno_new, newreno_free, newreno_reset, - newreno_set_option_uint, - newreno_get_option_uint, + newreno_set_input_params, + newreno_bind_diagnostic, + newreno_unbind_diagnostic, newreno_get_tx_allowance, newreno_get_wakeup_deadline, newreno_on_data_sent, diff --git a/test/cc_dummy.c b/test/cc_dummy.c index af0d5483791..0331a7cdcf5 100644 --- a/test/cc_dummy.c +++ b/test/cc_dummy.c @@ -12,8 +12,11 @@ typedef struct ossl_cc_dummy_st { size_t max_dgram_len; + size_t *p_diag_max_dgram_len; } OSSL_CC_DUMMY; +static void dummy_update_diag(OSSL_CC_DUMMY *d); + static OSSL_CC_DATA *dummy_new(OSSL_TIME (*now_cb)(void *arg), void *now_cb_arg) { @@ -36,39 +39,59 @@ static void dummy_reset(OSSL_CC_DATA *cc) } -static int dummy_set_option_uint(OSSL_CC_DATA *cc, - uint32_t option_id, - uint64_t value) +static int dummy_set_input_params(OSSL_CC_DATA *cc, const OSSL_PARAM *params) { OSSL_CC_DUMMY *d = (OSSL_CC_DUMMY *)cc; + const OSSL_PARAM *p; + size_t value; - switch (option_id) { - case OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN: - if (value > SIZE_MAX) + p = OSSL_PARAM_locate_const(params, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN); + if (p != NULL) { + if (!OSSL_PARAM_get_size_t(p, &value)) + return 0; + if (value < QUIC_MIN_INITIAL_DGRAM_LEN) return 0; - d->max_dgram_len = (size_t)value; - return 1; - - default: - return 0; + d->max_dgram_len = value; + dummy_update_diag(d); } + + return 1; } -static int dummy_get_option_uint(OSSL_CC_DATA *cc, - uint32_t option_id, - uint64_t *value) +static int dummy_bind_diagnostic(OSSL_CC_DATA *cc, OSSL_PARAM *params) { OSSL_CC_DUMMY *d = (OSSL_CC_DUMMY *)cc; + const OSSL_PARAM *p; - switch (option_id) { - case OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN: - *value = (uint64_t)d->max_dgram_len; - return 1; + p = OSSL_PARAM_locate_const(params, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN); + if (p != NULL) { + if (p->data_type != OSSL_PARAM_UNSIGNED_INTEGER + || p->data_size != sizeof(size_t)) + return 0; - default: - return 0; + d->p_diag_max_dgram_len = p->data; } + + dummy_update_diag(d); + return 1; +} + +static int dummy_unbind_diagnostic(OSSL_CC_DATA *cc, OSSL_PARAM *params) +{ + OSSL_CC_DUMMY *d = (OSSL_CC_DUMMY *)cc; + + if (OSSL_PARAM_locate_const(params, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN) + != NULL) + d->p_diag_max_dgram_len = NULL; + + return 1; +} + +static void dummy_update_diag(OSSL_CC_DUMMY *d) +{ + if (d->p_diag_max_dgram_len != NULL) + *d->p_diag_max_dgram_len = d->max_dgram_len; } static uint64_t dummy_get_tx_allowance(OSSL_CC_DATA *cc) @@ -115,8 +138,9 @@ const OSSL_CC_METHOD ossl_cc_dummy_method = { dummy_new, dummy_free, dummy_reset, - dummy_set_option_uint, - dummy_get_option_uint, + dummy_set_input_params, + dummy_bind_diagnostic, + dummy_unbind_diagnostic, dummy_get_tx_allowance, dummy_get_wakeup_deadline, dummy_on_data_sent, diff --git a/test/quic_cc_test.c b/test/quic_cc_test.c index 87f9e268d27..ce2507e68d3 100644 --- a/test/quic_cc_test.c +++ b/test/quic_cc_test.c @@ -328,11 +328,14 @@ static int test_simulate(void) int have_sim = 0; const OSSL_CC_METHOD *ccm = &ossl_cc_newreno_method; OSSL_CC_DATA *cc = NULL; - uint64_t mdpl = 1472; + size_t mdpl = 1472; uint64_t total_sent = 0, total_to_send, allowance; uint64_t actual_capacity = 16000; /* B/s - 128kb/s */ uint64_t cwnd_sample_sum = 0, cwnd_sample_count = 0; + uint64_t diag_cur_bytes_in_flight = UINT64_MAX; + uint64_t diag_cur_cwnd_size = UINT64_MAX; struct net_sim sim; + OSSL_PARAM params[3], *p = params; fake_time = TIME_BASE; @@ -344,8 +347,21 @@ static int test_simulate(void) have_sim = 1; - if (!TEST_true(ccm->set_option_uint(cc, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN, - mdpl))) + *p++ = OSSL_PARAM_construct_size_t(OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN, + &mdpl); + *p++ = OSSL_PARAM_construct_end(); + + if (!TEST_true(ccm->set_input_params(cc, params))) + goto err; + + p = params; + *p++ = OSSL_PARAM_construct_uint64(OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT, + &diag_cur_bytes_in_flight); + *p++ = OSSL_PARAM_construct_uint64(OSSL_CC_OPTION_CUR_CWND_SIZE, + &diag_cur_cwnd_size); + *p++ = OSSL_PARAM_construct_end(); + + if (!TEST_true(ccm->bind_diagnostics(cc, params))) goto err; ccm->reset(cc); @@ -399,13 +415,7 @@ static int test_simulate(void) * have at least one MDPL's worth of allowance as nothing is in flight. */ if (rc == 3) { - uint64_t v = 1; - - if (!TEST_true(ccm->get_option_uint(cc, OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT, - &v))) - goto err; - - if (!TEST_uint64_t_eq(v, 0)) + if (!TEST_uint64_t_eq(diag_cur_bytes_in_flight, 0)) goto err; if (!TEST_uint64_t_ge(ccm->get_tx_allowance(cc), mdpl)) @@ -416,8 +426,8 @@ static int test_simulate(void) { uint64_t v = 1; - if (!TEST_true(ccm->get_option_uint(cc, OSSL_CC_OPTION_CUR_CWND_SIZE, - &v))) + if (!TEST_uint64_t_ne(diag_cur_bytes_in_flight, UINT64_MAX) + || !TEST_uint64_t_ne(diag_cur_cwnd_size, UINT64_MAX)) goto err; cwnd_sample_sum += v; @@ -471,7 +481,10 @@ static int test_sanity(void) const OSSL_CC_METHOD *ccm = &ossl_cc_newreno_method; OSSL_CC_LOSS_INFO loss_info = {0}; OSSL_CC_ACK_INFO ack_info = {0}; - uint64_t allowance, allowance2, v = 1; + uint64_t allowance, allowance2; + OSSL_PARAM params[3], *p = params; + size_t mdpl = 1472, diag_mdpl = SIZE_MAX; + uint64_t diag_cur_bytes_in_flight = UINT64_MAX; fake_time = TIME_BASE; @@ -479,15 +492,24 @@ static int test_sanity(void) goto err; /* Test configuration of options. */ - if (!TEST_true(ccm->set_option_uint(cc, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN, - 1472))) + *p++ = OSSL_PARAM_construct_size_t(OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN, + &mdpl); + *p++ = OSSL_PARAM_construct_end(); + + if (!TEST_true(ccm->set_input_params(cc, params))) goto err; ccm->reset(cc); - if (!TEST_true(ccm->get_option_uint(cc, OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN, - &v)) - || !TEST_uint64_t_eq(v, 1472)) + p = params; + *p++ = OSSL_PARAM_construct_size_t(OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN, + &diag_mdpl); + *p++ = OSSL_PARAM_construct_uint64(OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT, + &diag_cur_bytes_in_flight); + *p++ = OSSL_PARAM_construct_end(); + + if (!TEST_true(ccm->bind_diagnostics(cc, params)) + || !TEST_size_t_eq(diag_mdpl, 1472)) goto err; if (!TEST_uint64_t_ge(allowance = ccm->get_tx_allowance(cc), 1472)) @@ -501,9 +523,7 @@ static int test_sanity(void) goto err; /* No bytes should currently be in flight. */ - if (!TEST_true(ccm->get_option_uint(cc, OSSL_CC_OPTION_CUR_BYTES_IN_FLIGHT, - &v)) - || !TEST_uint64_t_eq(v, 0)) + if (!TEST_uint64_t_eq(diag_cur_bytes_in_flight, 0)) goto err; /* Tell the CC we have sent some data. */