From 7a9b83ff9dc272b0b4610911c74e2295e13dd014 Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Fri, 18 May 2018 10:24:20 -0400 Subject: [PATCH] Latest changes - Improve checks of number of incoming streams limit and associated error reporting. - Small improvements to the recent DNS resolution code. --- CHANGELOG | 5 ++ src/liblsquic/lsquic_full_conn.c | 82 +++++++++++++++++++++++++++----- test/prog.c | 5 +- test/test_common.c | 2 + 4 files changed, 81 insertions(+), 13 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 21308ec..03969a5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,8 @@ +2018-05-18 + + - Improve checks of number of incoming streams limit and associated + error reporting. + 2018-05-16 - [FEATURE] DNS resolution diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c index 8421fcc..13d11a4 100644 --- a/src/liblsquic/lsquic_full_conn.c +++ b/src/liblsquic/lsquic_full_conn.c @@ -95,6 +95,8 @@ enum full_conn_flags { FC_TICK_CLOSE = (1 <<20), /* We returned TICK_CLOSE */ FC_HSK_FAILED = (1 <<21), FC_HAVE_SAVED_ACK = (1 <<22), + FC_ABORT_COMPLAINED + = (1 <<23), }; #define FC_IMMEDIATE_CLOSE_FLAGS \ @@ -224,15 +226,17 @@ struct full_conn snprintf((conn)->fc_errmsg, MAX_ERRMSG, __VA_ARGS__); \ } while (0) -#define ABORT_WITH_FLAG(conn, flag, ...) do { \ +#define ABORT_WITH_FLAG(conn, log_level, flag, ...) do { \ SET_ERRMSG(conn, __VA_ARGS__); \ - (conn)->fc_flags |= flag; \ - LSQ_ERROR("Abort connection: " __VA_ARGS__); \ + if (!((conn)->fc_flags & FC_ABORT_COMPLAINED)) \ + LSQ_LOG(log_level, "Abort connection: " __VA_ARGS__); \ + (conn)->fc_flags |= flag|FC_ABORT_COMPLAINED; \ } while (0) -#define ABORT_ERROR(...) ABORT_WITH_FLAG(conn, FC_ERROR, __VA_ARGS__) - -#define ABORT_TIMEOUT(...) ABORT_WITH_FLAG(conn, FC_TIMED_OUT, __VA_ARGS__) +#define ABORT_ERROR(...) \ + ABORT_WITH_FLAG(conn, LSQ_LOG_ERROR, FC_ERROR, __VA_ARGS__) +#define ABORT_WARN(...) \ + ABORT_WITH_FLAG(conn, LSQ_LOG_WARN, FC_ERROR, __VA_ARGS__) static void idle_alarm_expired (void *ctx, lsquic_time_t expiry, lsquic_time_t now); @@ -725,13 +729,51 @@ count_streams (const struct full_conn *conn, int peer) stream = lsquic_hashelem_getdata(el); ours = (1 & stream->id) ^ is_server; if (ours ^ peer) - count += !lsquic_stream_is_closed(stream); + count += !(lsquic_stream_is_closed(stream) + /* When counting peer-initiated streams, do not + * include those that have been reset: + */ + || (peer && lsquic_stream_is_reset(stream))); } return count; } +enum stream_count { SCNT_ALL, SCNT_PEER, SCNT_CLOSED, SCNT_RESET, + SCNT_RES_UNCLO /* reset and not closed */, N_SCNTS }; + +static void +collect_stream_counts (const struct full_conn *conn, int peer, + unsigned counts[N_SCNTS]) +{ + const lsquic_stream_t *stream; + int ours; + int is_server; + struct lsquic_hash_elem *el; + + peer = !!peer; + is_server = !!(conn->fc_flags & FC_SERVER); + memset(counts, 0, N_SCNTS * sizeof(counts[0])); + + for (el = lsquic_hash_first(conn->fc_pub.all_streams); el; + el = lsquic_hash_next(conn->fc_pub.all_streams)) + { + ++counts[SCNT_ALL]; + stream = lsquic_hashelem_getdata(el); + ours = (1 & stream->id) ^ is_server; + if (ours ^ peer) + { + ++counts[SCNT_PEER]; + counts[SCNT_CLOSED] += lsquic_stream_is_closed(stream); + counts[SCNT_RESET] += lsquic_stream_is_reset(stream); + counts[SCNT_RES_UNCLO] += lsquic_stream_is_reset(stream) + && !lsquic_stream_is_closed(stream); + } + } +} + + static void full_conn_ci_destroy (lsquic_conn_t *lconn) { @@ -1120,8 +1162,17 @@ process_stream_frame (struct full_conn *conn, lsquic_packet_in_t *packet_in, LSQ_DEBUG("number of peer-initiated streams: %u", in_count); if (in_count >= conn->fc_cfg.max_streams_in) { - ABORT_ERROR("incoming stream would exceed limit: %u", - conn->fc_cfg.max_streams_in); + if (!(conn->fc_flags & FC_ABORT_COMPLAINED)) + { + enum stream_count counts[N_SCNTS]; + collect_stream_counts(conn, 1, counts); + ABORT_WARN("incoming stream would exceed limit: %u. " + "all: %u; peer: %u; closed: %u; reset: %u; reset " + "and not closed: %u", conn->fc_cfg.max_streams_in, + counts[SCNT_ALL], counts[SCNT_PEER], + counts[SCNT_CLOSED], counts[SCNT_RESET], + counts[SCNT_RES_UNCLO]); + } lsquic_malo_put(stream_frame); return 0; } @@ -3081,8 +3132,17 @@ find_stream_on_non_stream_frame (struct full_conn *conn, uint32_t stream_id, LSQ_DEBUG("number of peer-initiated streams: %u", in_count); if (in_count >= conn->fc_cfg.max_streams_in) { - ABORT_ERROR("incoming %s for stream %u would exceed " - "limit: %u", what, stream_id, conn->fc_cfg.max_streams_in); + if (!(conn->fc_flags & FC_ABORT_COMPLAINED)) + { + enum stream_count counts[N_SCNTS]; + collect_stream_counts(conn, 1, counts); + ABORT_WARN("incoming %s for stream %u would exceed " + "limit: %u. all: %u; peer: %u; closed: %u; reset: %u; reset " + "and not closed: %u", + what, stream_id, conn->fc_cfg.max_streams_in, counts[SCNT_ALL], + counts[SCNT_PEER], counts[SCNT_CLOSED], counts[SCNT_RESET], + counts[SCNT_RES_UNCLO]); + } return NULL; } if ((conn->fc_flags & FC_GOING_AWAY) && diff --git a/test/prog.c b/test/prog.c index b4afa45..593ffa2 100644 --- a/test/prog.c +++ b/test/prog.c @@ -92,8 +92,7 @@ prog_print_common_options (const struct prog *prog, FILE *out) #else " -s SERVER Server address. Takes on the form of host:port or host.\n" " If host is not an IPv4 or IPv6 address, it is resolved.\n" -" If port is not set, the default is 443. If -s is not\n" -" specified, the value of SNI is used (see the -H flag).\n" +" If port is not set, the default is 443.\n" #endif " Examples:\n" " 127.0.0.1:12345\n" @@ -103,6 +102,8 @@ prog_print_common_options (const struct prog *prog, FILE *out) #if HAVE_REGEX " 8443\n" #endif +" If -s is not specified, the value of SNI is used (see\n" +" the -H flag).\n" #if LSQUIC_DONTFRAG_SUPPORTED " -D Set `do not fragment' flag on outgoing UDP packets\n" #endif diff --git a/test/test_common.c b/test/test_common.c index 7e1a967..9a0dc3e 100644 --- a/test/test_common.c +++ b/test/test_common.c @@ -28,6 +28,8 @@ #include #include #include + +#include "test_config.h" #if HAVE_REGEX #include #endif