diff --git a/CHANGELOG b/CHANGELOG index 369f93839..a87d48d24 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,9 @@ +2019-05-06 + - 1.21.1 + - [BUGFIX] If FIN or RST not received, don't delay stream destruction. + - [OPTIMIZATION] Speed up HPACK encoder by using same hash value to + search static and dynamic tables. + 2019-04-12 - 1.21.0 - [FEATURE] Add qlog log module. diff --git a/include/lsquic.h b/include/lsquic.h index 029a69375..2a8fbd27c 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 1 #define LSQUIC_MINOR_VERSION 21 -#define LSQUIC_PATCH_VERSION 0 +#define LSQUIC_PATCH_VERSION 1 /** * Engine flags: diff --git a/src/liblsquic/lsquic_stream.c b/src/liblsquic/lsquic_stream.c index 8f599aa79..94c82cf19 100644 --- a/src/liblsquic/lsquic_stream.c +++ b/src/liblsquic/lsquic_stream.c @@ -378,8 +378,7 @@ stream_is_finished (const lsquic_stream_t *stream) */ && 0 == (stream->stream_flags & STREAM_SEND_RST) && ((stream->stream_flags & STREAM_FORCE_FINISH) - || ((stream->stream_flags & (STREAM_FIN_SENT |STREAM_RST_SENT)) - && (stream->stream_flags & (STREAM_FIN_RECVD|STREAM_RST_RECVD)))); + || (stream->stream_flags & (STREAM_FIN_SENT |STREAM_RST_SENT))); } diff --git a/src/lshpack/lshpack.c b/src/lshpack/lshpack.c index aa15e830b..2858c5cb7 100644 --- a/src/lshpack/lshpack.c +++ b/src/lshpack/lshpack.c @@ -5411,325 +5411,119 @@ lshpack_enc_cleanup (struct lshpack_enc *enc) } -//not find return 0, otherwise return the index -unsigned -lshpack_enc_get_stx_tab_id (const char *name, lshpack_strlen_t name_len, - const char *val, lshpack_strlen_t val_len, int *val_matched) +#define LSHPACK_XXH_SEED 0 +#define XXH_NAMEVAL_WIDTH 9 +#define XXH_NAMEVAL_SHIFT 2 +#define XXH_NAME_WIDTH 9 +#define XXH_NAME_SHIFT 9 + +static const unsigned char nameval2id[ 1 << XXH_NAMEVAL_WIDTH ] = { - if (name_len < 3) - return 0; + [475] = 2, [361] = 3, [149] = 4, [219] = 5, [511] = 6, + [299] = 7, [298] = 8, [19] = 9, [370] = 10, [95] = 11, + [158] = 12, [281] = 13, [423] = 14, [501] = 16, +}; - *val_matched = 0; - - //check value first - int i = -1; - switch (*val) - { - case 'G': - i = 1; - break; - case 'P': - i = 2; - break; - case '/': - if (val_len == 1) - i = 3; - else if (val_len == 11) - i = 4; - break; - case 'h': - if (val_len == 4) - i = 5; - else if (val_len == 5) - i = 6; - break; - case '2': - if (val_len == 3) - { - switch (*(val + 2)) - { - case '0': - i = 7; - break; - case '4': - i = 8; - break; - case '6': - i = 9; - break; - default: - break; - } - } - break; - case '3': - i = 10; - break; - case '4': - if (val_len == 3) - { - switch (*(val + 2)) - { - case '0': - i = 11; - break; - case '4': - i = 12; - default: - break; - } - } - break; - case '5': - i = 13; - break; - case 'g': - i = 15; - break; - default: - break; +static const unsigned char name2id[ 1 << XXH_NAME_WIDTH ] = +{ + [215] = 1, [207] = 2, [286] = 4, [321] = 6, [289] = 8, + [63] = 15, [122] = 16, [209] = 17, [194] = 18, [331] = 19, + [273] = 20, [278] = 21, [431] = 22, [232] = 23, [182] = 24, + [483] = 25, [59] = 26, [52] = 27, [422] = 28, [62] = 29, + [381] = 30, [233] = 31, [110] = 32, [15] = 33, [85] = 34, + [452] = 35, [28] = 36, [414] = 37, [82] = 38, [345] = 39, + [467] = 40, [501] = 41, [299] = 42, [168] = 43, [326] = 44, + [199] = 45, [360] = 46, [36] = 47, [349] = 48, [347] = 49, + [126] = 50, [208] = 51, [416] = 52, [373] = 53, [477] = 54, + [33] = 55, [316] = 56, [156] = 57, [386] = 58, [116] = 59, + [450] = 60, [297] = 61, +}; + + +//not find return 0, otherwise return the index +#if !LS_HPACK_EMIT_TEST_CODE +static +#endif + unsigned +lshpack_enc_get_static_nameval (uint32_t nameval_hash, const char *name, + lshpack_strlen_t name_len, const char *val, lshpack_strlen_t val_len) +{ + unsigned i; + + i = (nameval_hash >> XXH_NAMEVAL_SHIFT) & ((1 << XXH_NAMEVAL_WIDTH) - 1); + if (nameval2id[i]) + { + i = nameval2id[i] - 1; + if (static_table[i].name_len == name_len + && static_table[i].val_len == val_len + && memcmp(name, static_table[i].name, name_len) == 0 + && memcmp(val, static_table[i].val, val_len) == 0) + { + return i + 1; + } } - if (i > 0 && static_table[i].val_len == val_len - && static_table[i].name_len == name_len - && memcmp(val, static_table[i].val, val_len) == 0 - && memcmp(name, static_table[i].name, name_len) == 0) + return 0; +} + + +#if !LS_HPACK_EMIT_TEST_CODE +static +#endif + unsigned +lshpack_enc_get_static_name (uint32_t name_hash, const char *name, + lshpack_strlen_t name_len) +{ + unsigned i; + + i = (name_hash >> XXH_NAME_SHIFT) & ((1 << XXH_NAME_WIDTH) - 1); + if (name2id[i]) { - *val_matched = 1; - return i + 1; + i = name2id[i] - 1; + if (static_table[i].name_len == name_len + && memcmp(name, static_table[i].name, name_len) == 0) + { + return i + 1; + } } - //macth name only checking - i = -1; - switch (*name) + return 0; +} + + +unsigned +lshpack_enc_get_stx_tab_id (const char *name, lshpack_strlen_t name_len, + const char *val, lshpack_strlen_t val_len) +{ + uint32_t name_hash, nameval_hash; + unsigned i; + + name_hash = XXH32(name, name_len, 0); + nameval_hash = XXH32(val, val_len, name_hash); + + i = (nameval_hash >> XXH_NAMEVAL_SHIFT) & ((1 << XXH_NAMEVAL_WIDTH) - 1); + if (nameval2id[i]) { - case ':': - switch (*(name + 1)) - { - case 'a': - i = 0; - break; - case 'm': - i = 1; - break; - case 'p': - i = 3; - break; - case 's': - if (*(name + 2) == 'c') //:scheme - i = 5; - else - i = 7; - break; - default: - break; - } - break; - case 'a': - switch (name_len) - { - case 3: - i = 20; //age - break; - case 5: - i = 21; //allow - break; - case 6: - i = 18; //accept - break; - case 13: - if (*(name + 1) == 'u') - i = 22; //authorization - else - i = 17; //accept-ranges - break; - case 14: - i = 14; //accept-charset - break; - case 15: - if (*(name + 7) == 'l') - i = 16; //accept-language, - else - i = 15;// accept-encoding - break; - case 27: - i = 19;//access-control-allow-origin - break; - default: - break; - } - break; - case 'c': - switch (name_len) - { - case 6: - i = 31; //cookie - break; - case 12: - i = 30; //content-type - break; - case 13: - if (*(name + 1) == 'a') - i = 23; //cache-control - else - i = 29; //content-range - break; - case 14: - i = 27; //content-length - break; - case 16: - switch (*(name + 9)) - { - case 'n': - i = 25 ;//content-encoding - break; - case 'a': - i = 26; //content-language - break; - case 'o': - i = 28; //content-location - default: - break; - } - break; - case 19: - i = 24; //content-disposition - break; - } - break; - case 'd': - i = 32 ;//date - break; - case 'e': - switch (name_len) - { - case 4: - i = 33; //etag - break; - case 6: - i = 34; - break; - case 7: - i = 35; - break; - default: - break; - } - break; - case 'f': - i = 36; //from - break; - case 'h': - i = 37; //host - break; - case 'i': - switch (name_len) - { - case 8: - if (*(name + 3) == 'm') - i = 38; //if-match - else - i = 41; //if-range - break; - case 13: - i = 40; //if-none-match - break; - case 17: - i = 39; //if-modified-since - break; - case 19: - i = 42; //if-unmodified-since - break; - default: - break; - } - break; - case 'l': - switch (name_len) - { - case 4: - i = 44; //link - break; - case 8: - i = 45; //location - break; - case 13: - i = 43; //last-modified - break; - default: - break; - } - break; - case 'm': - i = 46; //max-forwards - break; - case 'p': - if (name_len == 18) - i = 47; //proxy-authenticate - else - i = 48; //proxy-authorization - break; - case 'r': - if (name_len >= 5) - { - switch (*(name + 4)) - { - case 'e': - if (name_len == 5) - i = 49; //range - else - i = 51; //refresh - break; - case 'r': - i = 50; //referer - break; - case 'y': - i = 52; //retry-after - break; - default: - break; - } - } - break; - case 's': - switch (name_len) - { - case 6: - i = 53; //server - break; - case 10: - i = 54; //set-cookie - break; - case 25: - i = 55; //strict-transport-security - break; - default: - break; - } - break; - case 't': - i = 56;//transfer-encoding - break; - case 'u': - i = 57; //user-agent - break; - case 'v': - if (name_len == 4) - i = 58; - else - i = 59; - break; - case 'w': - i = 60; - break; - default: - break; + i = nameval2id[i] - 1; + if (static_table[i].name_len == name_len + && static_table[i].val_len == val_len + && memcmp(name, static_table[i].name, name_len) == 0 + && memcmp(val, static_table[i].val, val_len) == 0) + { + return i + 1; + } } - if (i >= 0 - && static_table[i].name_len == name_len + i = (name_hash >> XXH_NAME_SHIFT) & ((1 << XXH_NAME_WIDTH) - 1); + if (name2id[i]) + { + i = name2id[i] - 1; + if (static_table[i].name_len == name_len && memcmp(name, static_table[i].name, name_len) == 0) - return i + 1; + { + return i + 1; + } + } return 0; } @@ -5747,28 +5541,23 @@ henc_calc_table_id (const struct lshpack_enc *enc, static unsigned -henc_find_table_id (struct lshpack_enc *enc, const char *name, +henc_find_table_id (struct lshpack_enc *enc, uint32_t name_hash, + uint32_t nameval_hash, const char *name, lshpack_strlen_t name_len, const char *value, lshpack_strlen_t value_len, int *val_matched) { struct lshpack_enc_table_entry *entry; - unsigned name_hash, nameval_hash, buckno, static_table_id; - XXH32_state_t hash_state; + unsigned buckno, static_table_id; /* First, look for a match in the static table: */ - static_table_id = lshpack_enc_get_stx_tab_id(name, name_len, value, - value_len, val_matched); - if (static_table_id > 0 && *val_matched) + static_table_id = lshpack_enc_get_static_nameval(nameval_hash, name, + name_len, value, value_len); + if (static_table_id > 0) + { + *val_matched = 1; return static_table_id; + } - /* Search by name and value: */ - XXH32_reset(&hash_state, (uintptr_t) enc); - XXH32_update(&hash_state, &name_len, sizeof(name_len)); - XXH32_update(&hash_state, name, name_len); - name_hash = XXH32_digest(&hash_state); - XXH32_update(&hash_state, &value_len, sizeof(value_len)); - XXH32_update(&hash_state, value, value_len); - nameval_hash = XXH32_digest(&hash_state); buckno = BUCKNO(enc->hpe_nbits, nameval_hash); STAILQ_FOREACH(entry, &enc->hpe_buckets[buckno].by_nameval, ete_next_nameval) @@ -5782,11 +5571,12 @@ henc_find_table_id (struct lshpack_enc *enc, const char *name, return henc_calc_table_id(enc, entry); } - /* Name/value match is not found, but if the caller found a matching - * static table entry, no need to continue to search: - */ + static_table_id = lshpack_enc_get_static_name(name_hash, name, name_len); if (static_table_id > 0) + { + *val_matched = 0; return static_table_id; + } /* Search by name only: */ buckno = BUCKNO(enc->hpe_nbits, name_hash); @@ -6037,13 +5827,12 @@ henc_grow_tables (struct lshpack_enc *enc) static #endif int -lshpack_enc_push_entry (struct lshpack_enc *enc, const char *name, - lshpack_strlen_t name_len, const char *value, - lshpack_strlen_t value_len) +lshpack_enc_push_entry (struct lshpack_enc *enc, uint32_t name_hash, + uint32_t nameval_hash, const char *name, lshpack_strlen_t name_len, + const char *value, lshpack_strlen_t value_len) { - unsigned name_hash, nameval_hash, buckno; struct lshpack_enc_table_entry *entry; - XXH32_state_t hash_state; + unsigned buckno; size_t size; if (enc->hpe_nelem >= N_BUCKETS(enc->hpe_nbits) / 2 && @@ -6055,14 +5844,6 @@ lshpack_enc_push_entry (struct lshpack_enc *enc, const char *name, if (!entry) return -1; - XXH32_reset(&hash_state, (uintptr_t) enc); - XXH32_update(&hash_state, &name_len, sizeof(name_len)); - XXH32_update(&hash_state, name, name_len); - name_hash = XXH32_digest(&hash_state); - XXH32_update(&hash_state, &value_len, sizeof(value_len)); - XXH32_update(&hash_state, value, value_len); - nameval_hash = XXH32_digest(&hash_state); - entry->ete_name_hash = name_hash; entry->ete_nameval_hash = nameval_hash; entry->ete_name_len = name_len; @@ -6094,16 +5875,24 @@ lshpack_enc_encode2 (struct lshpack_enc *enc, unsigned char *dst, //indexed_type: 0, Add, 1,: without, 2: never static const char indexed_prefix_number[] = {0x40, 0x00, 0x10}; unsigned char *const dst_org = dst; + uint32_t name_hash, nameval_hash; int val_matched, rc; unsigned table_id; + XXH32_state_t hash_state; assert(indexed_type >= 0 && indexed_type <= 2); if (dst_end <= dst) return dst_org; - table_id = henc_find_table_id(enc, name, name_len, value, value_len, - &val_matched); + XXH32_reset(&hash_state, LSHPACK_XXH_SEED); + XXH32_update(&hash_state, name, name_len); + name_hash = XXH32_digest(&hash_state); + XXH32_update(&hash_state, value, value_len); + nameval_hash = XXH32_digest(&hash_state); + + table_id = henc_find_table_id(enc, name_hash, nameval_hash, name, + name_len, value, value_len, &val_matched); if (table_id > 0) { if (val_matched) @@ -6142,7 +5931,8 @@ lshpack_enc_encode2 (struct lshpack_enc *enc, unsigned char *dst, if (indexed_type == 0) { - rc = lshpack_enc_push_entry(enc, name, name_len, value, value_len); + rc = lshpack_enc_push_entry(enc, name_hash, nameval_hash, name, + name_len, value, value_len); if (rc != 0) return dst_org; //Failed to enc this header, return unchanged ptr. } diff --git a/src/lshpack/lshpack.h b/src/lshpack/lshpack.h index 3d57db53a..907f03ef8 100644 --- a/src/lshpack/lshpack.h +++ b/src/lshpack/lshpack.h @@ -262,7 +262,7 @@ struct lshpack_dec unsigned lshpack_enc_get_stx_tab_id (const char *name, lshpack_strlen_t name_len, - const char *val, lshpack_strlen_t val_len, int *val_matched); + const char *val, lshpack_strlen_t val_len); #ifdef __cplusplus } diff --git a/test/unittests/test_stream.c b/test/unittests/test_stream.c index 55e0c0225..fd1f6af0a 100644 --- a/test/unittests/test_stream.c +++ b/test/unittests/test_stream.c @@ -658,7 +658,7 @@ test_rem_FIN_loc_FIN (struct test_objs *tobjs) * DOES NOT result in stream being reset. */ static void -test_rem_data_loc_close (struct test_objs *tobjs) +test_rem_data_loc_close_and_rst_in (struct test_objs *tobjs) { lsquic_stream_t *stream; char buf[0x100]; @@ -695,6 +695,8 @@ test_rem_data_loc_close (struct test_objs *tobjs) s = lsquic_stream_rst_in(stream, 100, 1); assert(0 == s); + assert(stream->stream_flags & STREAM_FREE_STREAM); + lsquic_stream_destroy(stream); /* This simply checks that the stream got removed from the queue: */ assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); @@ -704,6 +706,57 @@ test_rem_data_loc_close (struct test_objs *tobjs) } +/* Server: we read data and close the read side before reading FIN. No + * FIN or RST arrive from peer. This should still place the stream on + * the "streams to be freed" list. + */ +static void +test_rem_data_loc_close (struct test_objs *tobjs) +{ + lsquic_stream_t *stream; + char buf[0x100]; + ssize_t n; + int s; + + stream = new_stream(tobjs, 345); + + s = lsquic_stream_frame_in(stream, new_frame_in(tobjs, 0, 100, 0)); + assert(0 == s); + + n = lsquic_stream_read(stream, buf, 60); + assert(60 == n); + + s = lsquic_stream_shutdown(stream, 0); + assert(0 == s); + assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); + assert(!((stream->stream_flags & (STREAM_SERVICE_FLAGS)) + == STREAM_CALL_ONCLOSE)); + + n = lsquic_stream_read(stream, buf, 60); + assert(n == -1); /* Cannot read from closed stream */ + + /* Close write side */ + s = lsquic_stream_shutdown(stream, 1); + assert(0 == s); + + assert(1 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl)); /* Shutdown performs a flush */ + + assert(!TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); + assert((stream->stream_flags & (STREAM_SERVICE_FLAGS)) + == STREAM_CALL_ONCLOSE); + + assert(!(stream->stream_flags & STREAM_FREE_STREAM)); + lsquic_stream_acked(stream); + assert(stream->stream_flags & STREAM_FREE_STREAM); + + lsquic_stream_destroy(stream); + /* This simply checks that the stream got removed from the queue: */ + assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); + + assert(100 == tobjs->conn_pub.cfcw.cf_max_recv_off); + assert(100 == tobjs->conn_pub.cfcw.cf_read_off); +} + /* Client: we send some data and FIN, but remote end sends some data and * then resets the stream. The client gets an error when it reads from @@ -1197,6 +1250,7 @@ test_termination (void) void (*const test_funcs[])(struct test_objs *) = { test_loc_FIN_rem_FIN, test_rem_FIN_loc_FIN, + test_rem_data_loc_close_and_rst_in, test_rem_data_loc_close, test_loc_FIN_rem_RST, test_loc_data_rem_RST,