summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNathan Skrzypczak <nathan.skrzypczak@gmail.com>2019-12-02 17:01:40 +0100
committerDave Wallace <dwallacelf@gmail.com>2019-12-04 18:08:10 +0000
commitd9577b4aa3a325e46c09796835d22122bec9e3d0 (patch)
treeff3a635fd7430e976756336ff2a99f2806252661
parent3f5594d89f583d12c0fcf586f2c3c7e2b008ea7d (diff)
quic: clean accept/connect error codepath
Type: fix First attempt to clean the leftover state when accept_notify / connect_notify fails due to mq size constraints. vpp should now be left in a state such that clean state will eventually be reached when timers fire. Change-Id: I9e1166dab2778bf05d5af42d437769651369cae0 Signed-off-by: Nathan Skrzypczak <nathan.skrzypczak@gmail.com>
-rw-r--r--src/plugins/quic/quic.c99
1 files changed, 51 insertions, 48 deletions
diff --git a/src/plugins/quic/quic.c b/src/plugins/quic/quic.c
index 6c2dd6af316..d1f188443d6 100644
--- a/src/plugins/quic/quic.c
+++ b/src/plugins/quic/quic.c
@@ -38,7 +38,11 @@ static char *quic_error_strings[] = {
static quic_main_t quic_main;
static void quic_update_timer (quic_ctx_t * ctx);
-static int quic_check_quic_session_connected (quic_ctx_t * ctx);
+static void quic_check_quic_session_connected (quic_ctx_t * ctx);
+static int quic_reset_connection (u64 udp_session_handle,
+ quic_rx_packet_ctx_t * pctx);
+static void quic_proto_on_close (u32 ctx_index, u32 thread_index);
+
static quicly_stream_open_t on_stream_open;
static quicly_closed_by_peer_t on_closed_by_peer;
static quicly_now_t quicly_vpp_now_cb;
@@ -383,9 +387,8 @@ quic_connection_closed (quic_ctx_t * ctx)
/* App already confirmed close, we can delete the connection */
quic_connection_delete (ctx);
break;
- case QUIC_CONN_STATE_PASSIVE_CLOSING_QUIC_CLOSED:
- QUIC_DBG (0, "BUG");
- break;
+ case QUIC_CONN_STATE_OPENED:
+ case QUIC_CONN_STATE_HANDSHAKE:
case QUIC_CONN_STATE_ACTIVE_CLOSING:
quic_connection_delete (ctx);
break;
@@ -677,9 +680,9 @@ int
quic_fifo_egress_emit (quicly_stream_t * stream, size_t off, void *dst,
size_t * len, int *wrote_all)
{
+ u32 deq_max, first_deq, max_rd_chunk, rem_offset;
session_t *stream_session;
svm_fifo_t *f;
- u32 deq_max, first_deq, max_rd_chunk, rem_offset;
stream_session = get_stream_session_from_stream (stream);
f = stream_session->tx_fifo;
@@ -731,6 +734,11 @@ static const quicly_stream_callbacks_t quic_stream_callbacks = {
static int
quic_on_stream_open (quicly_stream_open_t * self, quicly_stream_t * stream)
{
+ /* Return code for this function ends either
+ * - in quicly_receive : if not QUICLY_ERROR_PACKET_IGNORED, will close connection
+ * - in quicly_open_stream, returned directly
+ */
+
session_t *stream_session, *quic_session;
quic_stream_data_t *stream_data;
app_worker_t *app_wrk;
@@ -786,7 +794,6 @@ quic_on_stream_open (quicly_stream_open_t * self, quicly_stream_t * stream)
if ((rv = app_worker_init_connected (app_wrk, stream_session)))
{
QUIC_ERR ("failed to allocate fifos");
- session_free (stream_session);
quicly_reset_stream (stream, QUIC_APP_ALLOCATION_ERROR);
return 0; /* Frame is still valid */
}
@@ -797,7 +804,6 @@ quic_on_stream_open (quicly_stream_open_t * self, quicly_stream_t * stream)
if ((rv = app_worker_accept_notify (app_wrk, stream_session)))
{
QUIC_ERR ("failed to notify accept worker app");
- session_free_w_fifos (stream_session);
quicly_reset_stream (stream, QUIC_APP_ACCEPT_NOTIFY_ERROR);
return 0; /* Frame is still valid */
}
@@ -1014,13 +1020,17 @@ quic_connect_stream (session_t * quic_session, u32 opaque)
session_type_from_proto_and_ip (TRANSPORT_PROTO_QUIC, qctx->udp_is_ip4);
sctx->c_s_index = stream_session->session_index;
+ stream_data = (quic_stream_data_t *) stream->data;
+ stream_data->ctx_id = sctx->c_c_index;
+ stream_data->thread_index = sctx->c_thread_index;
+ stream_data->app_rx_data_len = 0;
+ stream_session->session_state = SESSION_STATE_READY;
+ /* For now we only reset streams. Cleanup will be triggered by timers */
if (app_worker_init_connected (app_wrk, stream_session))
{
QUIC_ERR ("failed to app_worker_init_connected");
- quicly_reset_stream (stream, QUIC_APP_ALLOCATION_ERROR);
- session_free_w_fifos (stream_session);
- quic_ctx_free (sctx);
+ quicly_reset_stream (stream, QUIC_APP_CONNECT_NOTIFY_ERROR);
return app_worker_connect_notify (app_wrk, NULL, opaque);
}
@@ -1028,19 +1038,13 @@ quic_connect_stream (session_t * quic_session, u32 opaque)
SVM_FIFO_WANT_DEQ_NOTIF_IF_FULL |
SVM_FIFO_WANT_DEQ_NOTIF_IF_EMPTY);
- stream_session->session_state = SESSION_STATE_READY;
if (app_worker_connect_notify (app_wrk, stream_session, opaque))
{
QUIC_ERR ("failed to notify app");
quicly_reset_stream (stream, QUIC_APP_CONNECT_NOTIFY_ERROR);
- session_free_w_fifos (stream_session);
- quic_ctx_free (sctx);
return -1;
}
- stream_data = (quic_stream_data_t *) stream->data;
- stream_data->ctx_id = sctx->c_c_index;
- stream_data->thread_index = sctx->c_thread_index;
- stream_data->app_rx_data_len = 0;
+
return 0;
}
@@ -1134,6 +1138,8 @@ quic_proto_on_close (u32 ctx_index, u32 thread_index)
switch (ctx->conn_state)
{
+ case QUIC_CONN_STATE_OPENED:
+ case QUIC_CONN_STATE_HANDSHAKE:
case QUIC_CONN_STATE_READY:
ctx->conn_state = QUIC_CONN_STATE_ACTIVE_CLOSING;
quicly_conn_t *conn = ctx->conn;
@@ -1151,8 +1157,10 @@ quic_proto_on_close (u32 ctx_index, u32 thread_index)
case QUIC_CONN_STATE_PASSIVE_CLOSING_QUIC_CLOSED:
quic_connection_delete (ctx);
break;
+ case QUIC_CONN_STATE_ACTIVE_CLOSING:
+ break;
default:
- QUIC_DBG (0, "BUG");
+ QUIC_ERR ("Trying to close conn in state %d", ctx->conn_state);
break;
}
}
@@ -1348,7 +1356,7 @@ quic_build_sockaddr (struct sockaddr *sa, socklen_t * salen,
}
}
-static int
+static void
quic_on_quic_session_connected (quic_ctx_t * ctx)
{
session_t *quic_session;
@@ -1357,13 +1365,6 @@ quic_on_quic_session_connected (quic_ctx_t * ctx)
u32 thread_index = ctx->c_thread_index;
int rv;
- app_wrk = app_worker_get_if_valid (ctx->parent_app_wrk_id);
- if (!app_wrk)
- {
- quic_disconnect_transport (ctx);
- return 0;
- }
-
quic_session = session_alloc (thread_index);
QUIC_DBG (2, "Allocated quic session 0x%lx", session_handle (quic_session));
@@ -1374,11 +1375,14 @@ quic_on_quic_session_connected (quic_ctx_t * ctx)
quic_session->session_type =
session_type_from_proto_and_ip (TRANSPORT_PROTO_QUIC, ctx->udp_is_ip4);
+ /* If quic session connected fails, immediatly close connection */
+ app_wrk = app_worker_get (ctx->parent_app_wrk_id);
if (app_worker_init_connected (app_wrk, quic_session))
{
QUIC_ERR ("failed to app_worker_init_connected");
quic_proto_on_close (ctx_id, thread_index);
- return app_worker_connect_notify (app_wrk, NULL, ctx->client_opaque);
+ app_worker_connect_notify (app_wrk, NULL, ctx->client_opaque);
+ return;
}
quic_session->session_state = SESSION_STATE_CONNECTING;
@@ -1387,7 +1391,7 @@ quic_on_quic_session_connected (quic_ctx_t * ctx)
{
QUIC_ERR ("failed to notify app %d", rv);
quic_proto_on_close (ctx_id, thread_index);
- return -1;
+ return;
}
/* If the app opens a stream in its callback it may invalidate ctx */
@@ -1398,11 +1402,9 @@ quic_on_quic_session_connected (quic_ctx_t * ctx)
*/
quic_session = session_get (ctx->c_s_index, thread_index);
quic_session->session_state = SESSION_STATE_LISTENING;
-
- return 0;
}
-static int
+static void
quic_check_quic_session_connected (quic_ctx_t * ctx)
{
/* Called when we need to trigger quic session connected
@@ -1411,13 +1413,13 @@ quic_check_quic_session_connected (quic_ctx_t * ctx)
/* Conn may be set to null if the connection is terminated */
if (!ctx->conn || ctx->conn_state != QUIC_CONN_STATE_HANDSHAKE)
- return 0;
+ return;
if (!quicly_connection_is_ready (ctx->conn))
- return 0;
+ return;
ctx->conn_state = QUIC_CONN_STATE_READY;
if (!quicly_is_client (ctx->conn))
- return 0;
- return quic_on_quic_session_connected (ctx);
+ return;
+ quic_on_quic_session_connected (ctx);
}
static void
@@ -1776,7 +1778,7 @@ quic_accept_connection (u32 ctx_index, quic_rx_packet_ctx_t * pctx)
if (ctx->c_s_index != QUIC_SESSION_INVALID)
{
QUIC_DBG (2, "already accepted ctx 0x%x", ctx_index);
- return -1;
+ return 0;
}
quicly_ctx = quic_get_quicly_ctx_from_ctx (ctx);
@@ -1795,7 +1797,6 @@ quic_accept_connection (u32 ctx_index, quic_rx_packet_ctx_t * pctx)
/* Save ctx handle in quicly connection */
quic_store_conn_ctx (conn, ctx);
ctx->conn = conn;
- ctx->conn_state = QUIC_CONN_STATE_HANDSHAKE;
quic_session = session_alloc (ctx->c_thread_index);
QUIC_DBG (2, "Allocated quic_session, 0x%lx ctx %u",
@@ -1811,26 +1812,28 @@ quic_accept_connection (u32 ctx_index, quic_rx_packet_ctx_t * pctx)
session_type_from_proto_and_ip (TRANSPORT_PROTO_QUIC, ctx->udp_is_ip4);
quic_session->listener_handle = lctx->c_s_index;
- /* TODO: don't alloc fifos when we don't transfer data on this session
- * but we still need fifos for the events? */
+ /* Register connection in connections map */
+ quic_make_connection_key (&kv, quicly_get_master_id (conn));
+ kv.value = ((u64) thread_index) << 32 | (u64) ctx_index;
+ clib_bihash_add_del_16_8 (&quic_main.connection_hash, &kv, 1 /* is_add */ );
+ QUIC_DBG (2, "Registering conn with id %lu %lu", kv.key[0], kv.key[1]);
+
+ /* If notify fails, reset connection immediatly */
if ((rv = app_worker_init_accepted (quic_session)))
{
QUIC_ERR ("failed to allocate fifos");
- session_free (quic_session);
+ quic_proto_on_close (ctx_index, thread_index);
return rv;
}
+
app_wrk = app_worker_get (quic_session->app_wrk_index);
if ((rv = app_worker_accept_notify (app_wrk, quic_session)))
{
QUIC_ERR ("failed to notify accept worker app");
+ quic_proto_on_close (ctx_index, thread_index);
return rv;
}
-
- /* Register connection in connections map */
- quic_make_connection_key (&kv, quicly_get_master_id (conn));
- kv.value = ((u64) thread_index) << 32 | (u64) ctx_index;
- clib_bihash_add_del_16_8 (&quic_main.connection_hash, &kv, 1 /* is_add */ );
- QUIC_DBG (2, "Registering conn with id %lu %lu", kv.key[0], kv.key[1]);
+ ctx->conn_state = QUIC_CONN_STATE_HANDSHAKE;
return quic_send_packets (ctx);
}
@@ -1994,9 +1997,9 @@ rx_start:
ctx = quic_ctx_get (packets_ctx[i].ctx_index, thread_index);
rv = quicly_receive (ctx->conn, NULL, &packets_ctx[i].sa,
&packets_ctx[i].packet);
- if (rv)
+ if (rv && rv != QUICLY_ERROR_PACKET_IGNORED)
{
- QUIC_DBG (1, "quicly_receive return error %U",
+ QUIC_ERR ("quicly_receive return error %U",
quic_format_err, rv);
}
break;