From b725ebb3f47302c416e41c7be3f5a8bb3c9fe547 Mon Sep 17 00:00:00 2001 From: Aloys Augustin Date: Sun, 14 Jul 2019 23:48:36 +0200 Subject: quic: Refactor connections closing and deletion This code should handle the 3 following cases: - Active close quic_proto_on_close sets state to ACTIVE_CLOSING send packets eventually returns an error, calling quic_connection_closed which deletes the connection - Passive close quic_on_closed_by_peer -> set state to PASSIVE_CLOSING "race" between app confirmation (calling quic_proto_on_close) and quicly signalling that it's done (triggers call to quic_connection_closed). If quic_connection_closed is called first, it sets the state to PASSIVE CLOSING QUIC CLOSED, then when quic_proto_on_close is called it frees the connection. If quic_proto_on_close is called first, it sets the state to PASSIVE CLOSING APP CLOSED, then when quic_connection_closed is called it frees the connection - Error close (reset) quic_connection_closed is called in state READY. This means a timeout or protocol error happened. This calls session_transport_reset_notify, the app should confirm the deletion and quic_proto_on_close will be called to delete the connection. Change-Id: I3acbf9b079ed2439bdbb447197c428c78915d8c0 Signed-off-by: Aloys Augustin Type: feature --- src/plugins/quic/quic.c | 99 ++++++++++++++++++++++++++++++++++++++----------- src/plugins/quic/quic.h | 3 ++ 2 files changed, 81 insertions(+), 21 deletions(-) (limited to 'src/plugins/quic') diff --git a/src/plugins/quic/quic.c b/src/plugins/quic/quic.c index 29cfaf3e8aa..8bb94221002 100644 --- a/src/plugins/quic/quic.c +++ b/src/plugins/quic/quic.c @@ -412,42 +412,88 @@ quic_disconnect_transport (quic_ctx_t * ctx) } static void -quic_connection_closed (u32 ctx_index, u32 thread_index, u8 notify_transport) +quic_connection_delete (quic_ctx_t * ctx) { - QUIC_DBG (2, "QUIC connection closed"); tw_timer_wheel_1t_3w_1024sl_ov_t *tw; clib_bihash_kv_16_8_t kv; quicly_conn_t *conn; - quic_ctx_t *ctx; - ctx = quic_ctx_get (ctx_index, thread_index); + QUIC_DBG (2, "Deleting connection %u", ctx->c_c_index); + ASSERT (!quic_ctx_is_stream (ctx)); - /* TODO if connection is not established, just delete the session? */ /* Stop the timer */ if (ctx->timer_handle != QUIC_TIMER_HANDLE_INVALID) { - tw = &quic_main.wrk_ctx[thread_index].timer_wheel; + tw = &quic_main.wrk_ctx[ctx->c_thread_index].timer_wheel; tw_timer_stop_1t_3w_1024sl_ov (tw, ctx->timer_handle); } /* Delete the connection from the connection map */ conn = ctx->c_quic_ctx_id.conn; quic_make_connection_key (&kv, quicly_get_master_id (conn)); - QUIC_DBG (2, "Deleting conn with id %lu %lu", kv.key[0], kv.key[1]); + QUIC_DBG (2, "Deleting conn with id %lu %lu from map", kv.key[0], + kv.key[1]); clib_bihash_add_del_16_8 (&quic_main.connection_hash, &kv, 0 /* is_add */ ); quic_disconnect_transport (ctx); - if (notify_transport) - session_transport_closing_notify (&ctx->connection); - else - session_transport_delete_notify (&ctx->connection); - /* Do not try to send anything anymore */ - quicly_free (ctx->c_quic_ctx_id.conn); + + if (ctx->c_quic_ctx_id.conn) + quicly_free (ctx->c_quic_ctx_id.conn); ctx->c_quic_ctx_id.conn = NULL; + + session_transport_delete_notify (&ctx->connection); quic_ctx_free (ctx); } +/** + * Called when quicly return an error + * This function interacts tightly with quic_proto_on_close + */ +static void +quic_connection_closed (quic_ctx_t * ctx) +{ + QUIC_DBG (2, "QUIC connection %u/%u closed", ctx->c_thread_index, + ctx->c_c_index); + + /* TODO if connection is not established, just delete the session? */ + /* Actually should send connect or accept error */ + + switch (ctx->c_quic_ctx_id.conn_state) + { + case QUIC_CONN_STATE_READY: + /* Error on an opened connection (timeout...) + This puts the session in closing state, we should receive a notification + when the app has closed its session */ + session_transport_reset_notify (&ctx->connection); + /* This ensures we delete the connection when the app confirms the close */ + ctx->c_quic_ctx_id.conn_state = + QUIC_CONN_STATE_PASSIVE_CLOSING_QUIC_CLOSED; + break; + case QUIC_CONN_STATE_PASSIVE_CLOSING: + ctx->c_quic_ctx_id.conn_state = + QUIC_CONN_STATE_PASSIVE_CLOSING_QUIC_CLOSED; + /* quic_proto_on_close will eventually be called when the app confirms the close + , we delete the connection at that point */ + break; + case QUIC_CONN_STATE_PASSIVE_CLOSING_APP_CLOSED: + /* App already confirmed close, we can delete the connection */ + session_transport_delete_notify (&ctx->connection); + quic_connection_delete (ctx); + break; + case QUIC_CONN_STATE_PASSIVE_CLOSING_QUIC_CLOSED: + QUIC_DBG (0, "BUG"); + break; + case QUIC_CONN_STATE_ACTIVE_CLOSING: + session_transport_delete_notify (&ctx->connection); + quic_connection_delete (ctx); + break; + default: + QUIC_DBG (0, "BUG"); + break; + } +} + static int quic_send_datagram (session_t * udp_session, quicly_datagram_t * packet) { @@ -544,8 +590,7 @@ quic_send_packets (quic_ctx_t * ctx) { clib_warning ("Tried to send packets on non existing app worker %u", ctx->parent_app_wrk_id); - quic_connection_closed (ctx->c_c_index, ctx->c_thread_index, - 1 /* notify_transport */ ); + quic_connection_delete (ctx); return 1; } app = application_get (app_wrk->app_index); @@ -587,8 +632,7 @@ quicly_error: if (err && err != QUICLY_ERROR_PACKET_IGNORED && err != QUICLY_ERROR_FREE_CONNECTION) clib_warning ("Quic error '%U'.", quic_format_err, err); - quic_connection_closed (ctx->c_c_index, ctx->c_thread_index, - 1 /* notify_transport */ ); + quic_connection_closed (ctx); return 1; } @@ -1520,17 +1564,30 @@ quic_proto_on_close (u32 ctx_index, u32 thread_index) quicly_reset_stream (stream, QUIC_APP_ERROR_CLOSE_NOTIFY); quic_send_packets (ctx); } - else if (ctx->c_quic_ctx_id.conn_state == QUIC_CONN_STATE_PASSIVE_CLOSING) - quic_connection_closed (ctx->c_c_index, ctx->c_thread_index, - 0 /* notify_transport */ ); - else + + switch (ctx->c_quic_ctx_id.conn_state) { + case QUIC_CONN_STATE_READY: + ctx->c_quic_ctx_id.conn_state = QUIC_CONN_STATE_ACTIVE_CLOSING; quicly_conn_t *conn = ctx->c_quic_ctx_id.conn; /* Start connection closing. Keep sending packets until quicly_send returns QUICLY_ERROR_FREE_CONNECTION */ quicly_close (conn, QUIC_APP_ERROR_CLOSE_NOTIFY, "Closed by peer"); /* This also causes all streams to be closed (and the cb called) */ quic_send_packets (ctx); + break; + case QUIC_CONN_STATE_PASSIVE_CLOSING: + ctx->c_quic_ctx_id.conn_state = + QUIC_CONN_STATE_PASSIVE_CLOSING_APP_CLOSED; + /* send_packets will eventually return an error, we delete the conn at + that point */ + break; + case QUIC_CONN_STATE_PASSIVE_CLOSING_QUIC_CLOSED: + quic_connection_delete (ctx); + break; + default: + QUIC_DBG (0, "BUG"); + break; } } diff --git a/src/plugins/quic/quic.h b/src/plugins/quic/quic.h index 80f2664a6ec..de27f2ac7d5 100644 --- a/src/plugins/quic/quic.h +++ b/src/plugins/quic/quic.h @@ -71,6 +71,9 @@ typedef enum quic_ctx_conn_state_ QUIC_CONN_STATE_HANDSHAKE, QUIC_CONN_STATE_READY, QUIC_CONN_STATE_PASSIVE_CLOSING, + QUIC_CONN_STATE_PASSIVE_CLOSING_APP_CLOSED, + QUIC_CONN_STATE_PASSIVE_CLOSING_QUIC_CLOSED, + QUIC_CONN_STATE_ACTIVE_CLOSING, } quic_ctx_conn_state_t; -- cgit 1.2.3-korg