From 1be486a7535ff6dc424fc19e97c23e3571c52807 Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Wed, 24 Apr 2024 12:46:34 +0100 Subject: [PATCH] QUIC REACTOR: Allow ticks to schedule notifications of other threads Reviewed-by: Matt Caswell Reviewed-by: Neil Horman (Merged from https://github.com/openssl/openssl/pull/24971) --- include/internal/quic_reactor.h | 38 +++++++++++---------------------- ssl/quic/quic_engine.c | 10 ++++----- ssl/quic/quic_impl.c | 3 +-- ssl/quic/quic_port.c | 7 +++--- ssl/quic/quic_reactor.c | 30 ++++++++++++++++++++------ 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/include/internal/quic_reactor.h b/include/internal/quic_reactor.h index aa55d378647..8d20a073431 100644 --- a/include/internal/quic_reactor.h +++ b/include/internal/quic_reactor.h @@ -73,6 +73,7 @@ struct quic_tick_result_st { char net_read_desired; char net_write_desired; + char notify_other_threads; OSSL_TIME tick_deadline; }; @@ -80,9 +81,10 @@ static ossl_inline ossl_unused void ossl_quic_tick_result_merge_into(QUIC_TICK_RESULT *r, const QUIC_TICK_RESULT *src) { - r->net_read_desired = r->net_read_desired || src->net_read_desired; - r->net_write_desired = r->net_write_desired || src->net_write_desired; - r->tick_deadline = ossl_time_min(r->tick_deadline, src->tick_deadline); + r->net_read_desired = r->net_read_desired || src->net_read_desired; + r->net_write_desired = r->net_write_desired || src->net_write_desired; + r->notify_other_threads = r->notify_other_threads || src->notify_other_threads; + r->tick_deadline = ossl_time_min(r->tick_deadline, src->tick_deadline); } struct quic_reactor_st { @@ -100,6 +102,9 @@ struct quic_reactor_st { void (*tick_cb)(QUIC_TICK_RESULT *res, void *arg, uint32_t flags); void *tick_cb_arg; + /* The mutex used for ticking. Not owned by the reactor. */ + CRYPTO_MUTEX *mutex; + /* Used to notify other threads. Valid only if have_notifier is set. */ RIO_NOTIFIER notifier; @@ -143,6 +148,7 @@ int ossl_quic_reactor_init(QUIC_REACTOR *rtor, void (*tick_cb)(QUIC_TICK_RESULT *res, void *arg, uint32_t flags), void *tick_cb_arg, + CRYPTO_MUTEX *mutex, OSSL_TIME initial_tick_deadline, uint64_t flags); @@ -218,34 +224,14 @@ RIO_NOTIFIER *ossl_quic_reactor_get0_notifier(QUIC_REACTOR *rtor); * mutex is non-NULL, it must be a lock currently held for write; it will be * unlocked during any sleep, and then relocked for write afterwards. * - * Precondition: mutex is NULL or is held for write (unchecked) - * Postcondition: mutex is NULL or is held for write (unless - * CRYPTO_THREAD_write_lock fails) + * Precondition: If a reactor mutex is being used, it must be held (unchecked) + * Postcondition: If a reactor mutex is being used, it is held */ #define SKIP_FIRST_TICK (1U << 0) int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor, int (*pred)(void *arg), void *pred_arg, - uint32_t flags, - CRYPTO_MUTEX *mutex); - -/* - * ossl_quic_reactor_notify_other_threads - * -------------------------------------- - * - * Notify other threads currently blocking in - * ossl_quic_reactor_block_until_pred() calls that a predicate they are using - * might now be met due to state changes. - * - * This function must be called after state changes which might cause a - * predicate in another thread to now be met (i.e., ticking). It is a no-op if - * inter-thread notification is not being used. - * - * mutex is required and must be held. - */ -void ossl_quic_reactor_notify_other_threads(QUIC_REACTOR *rtor, - CRYPTO_MUTEX *mutex); - + uint32_t flags); # endif #endif diff --git a/ssl/quic/quic_engine.c b/ssl/quic/quic_engine.c index c6ecde24a3f..6272f4b7614 100644 --- a/ssl/quic/quic_engine.c +++ b/ssl/quic/quic_engine.c @@ -56,6 +56,7 @@ void ossl_quic_engine_free(QUIC_ENGINE *qeng) static int qeng_init(QUIC_ENGINE *qeng, uint64_t reactor_flags) { return ossl_quic_reactor_init(&qeng->rtor, qeng_tick, qeng, + qeng->mutex, ossl_time_zero(), reactor_flags); } @@ -152,9 +153,10 @@ static void qeng_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags) QUIC_ENGINE *qeng = arg; QUIC_PORT *port; - res->net_read_desired = 0; - res->net_write_desired = 0; - res->tick_deadline = ossl_time_infinite(); + res->net_read_desired = 0; + res->net_write_desired = 0; + res->notify_other_threads = 0; + res->tick_deadline = ossl_time_infinite(); if (qeng->inhibit_tick) return; @@ -166,6 +168,4 @@ static void qeng_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags) ossl_quic_port_subtick(port, &subr, flags); ossl_quic_tick_result_merge_into(res, &subr); } - - ossl_quic_reactor_notify_other_threads(&qeng->rtor, qeng->mutex); } diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c index cb52eb949df..faf4cdb10c3 100644 --- a/ssl/quic/quic_impl.c +++ b/ssl/quic/quic_impl.c @@ -549,8 +549,7 @@ static int block_until_pred(QCTX *ctx, ossl_quic_engine_set_inhibit_tick(qeng, 0); rtor = ossl_quic_engine_get0_reactor(qeng); - return ossl_quic_reactor_block_until_pred(rtor, pred, pred_arg, flags, - ossl_quic_engine_get0_mutex(qeng)); + return ossl_quic_reactor_block_until_pred(rtor, pred, pred_arg, flags); } /* diff --git a/ssl/quic/quic_port.c b/ssl/quic/quic_port.c index 594d37cb01a..e4bafabd805 100644 --- a/ssl/quic/quic_port.c +++ b/ssl/quic/quic_port.c @@ -480,9 +480,10 @@ void ossl_quic_port_subtick(QUIC_PORT *port, QUIC_TICK_RESULT *res, { QUIC_CHANNEL *ch; - res->net_read_desired = ossl_quic_port_is_running(port); - res->net_write_desired = 0; - res->tick_deadline = ossl_time_infinite(); + res->net_read_desired = ossl_quic_port_is_running(port); + res->net_write_desired = 0; + res->notify_other_threads = 0; + res->tick_deadline = ossl_time_infinite(); if (!port->engine->inhibit_tick) { /* Handle any incoming data from network. */ diff --git a/ssl/quic/quic_reactor.c b/ssl/quic/quic_reactor.c index cdc90fefd88..142818923ec 100644 --- a/ssl/quic/quic_reactor.c +++ b/ssl/quic/quic_reactor.c @@ -15,10 +15,13 @@ * Core I/O Reactor Framework * ========================== */ +static void rtor_notify_other_threads(QUIC_REACTOR *rtor); + int ossl_quic_reactor_init(QUIC_REACTOR *rtor, void (*tick_cb)(QUIC_TICK_RESULT *res, void *arg, uint32_t flags), void *tick_cb_arg, + CRYPTO_MUTEX *mutex, OSSL_TIME initial_tick_deadline, uint64_t flags) { @@ -32,6 +35,7 @@ int ossl_quic_reactor_init(QUIC_REACTOR *rtor, rtor->tick_cb = tick_cb; rtor->tick_cb_arg = tick_cb_arg; + rtor->mutex = mutex; rtor->cur_blocking_waiters = 0; @@ -144,6 +148,9 @@ int ossl_quic_reactor_tick(QUIC_REACTOR *rtor, uint32_t flags) rtor->net_read_desired = res.net_read_desired; rtor->net_write_desired = res.net_write_desired; rtor->tick_deadline = res.tick_deadline; + if (res.notify_other_threads) + rtor_notify_other_threads(rtor); + return 1; } @@ -387,8 +394,18 @@ static int poll_two_descriptors(const BIO_POLL_DESCRIPTOR *r, int r_want_read, notify_rfd, deadline, mutex); } -void ossl_quic_reactor_notify_other_threads(QUIC_REACTOR *rtor, - CRYPTO_MUTEX *mutex) +/* + * Notify other threads currently blocking in + * ossl_quic_reactor_block_until_pred() calls that a predicate they are using + * might now be met due to state changes. + * + * This function must be called after state changes which might cause a + * predicate in another thread to now be met (i.e., ticking). It is a no-op if + * inter-thread notification is not being used. + * + * The reactor mutex must be held while calling this function. + */ +static void rtor_notify_other_threads(QUIC_REACTOR *rtor) { if (!rtor->have_notifier) return; @@ -421,7 +438,7 @@ void ossl_quic_reactor_notify_other_threads(QUIC_REACTOR *rtor, * unsignalling the notifier. */ while (rtor->signalled_notifier) - ossl_crypto_condvar_wait(rtor->notifier_cv, mutex); + ossl_crypto_condvar_wait(rtor->notifier_cv, rtor->mutex); } /* @@ -437,8 +454,7 @@ void ossl_quic_reactor_notify_other_threads(QUIC_REACTOR *rtor, */ int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor, int (*pred)(void *arg), void *pred_arg, - uint32_t flags, - CRYPTO_MUTEX *mutex) + uint32_t flags) { int res, net_read_desired, net_write_desired, notifier_fd; OSSL_TIME tick_deadline; @@ -472,7 +488,7 @@ int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor, net_write_desired, notifier_fd, tick_deadline, - mutex); + rtor->mutex); assert(rtor->cur_blocking_waiters > 0); --rtor->cur_blocking_waiters; @@ -530,7 +546,7 @@ int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor, } else { /* We are not the last waiter out - so wait for that one. */ while (rtor->signalled_notifier) - ossl_crypto_condvar_wait(rtor->notifier_cv, mutex); + ossl_crypto_condvar_wait(rtor->notifier_cv, rtor->mutex); } } -- 2.47.2