From da5db205cacbacefffd8e344215f2d1e2e2e13c4 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 1 Apr 2014 22:54:20 +0900 Subject: [PATCH] Make group weight range [1, 256], inclusive We do -+1 on serialization and deserialization since the field is 1 byte. This change also parameterized range so that we can change it easily. --- lib/includes/nghttp2/nghttp2.h | 29 +++++++++++++++++++------- lib/nghttp2_frame.c | 6 +++--- lib/nghttp2_outbound_item.h | 6 +++--- lib/nghttp2_priority_spec.c | 2 +- lib/nghttp2_session.c | 4 ++-- lib/nghttp2_submit.c | 37 +++++++++++++++++++++++++--------- 6 files changed, 58 insertions(+), 26 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index e4b865e6..ba4729ef 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -123,7 +123,14 @@ typedef struct { * * The maximum weight of priority group. */ -#define NGHTTP2_MAX_WEIGHT 255 +#define NGHTTP2_MAX_WEIGHT 256 + +/** + * @macro + * + * The minimum weight of priority group. + */ +#define NGHTTP2_MIN_WEIGHT 1 /** * @macro @@ -717,7 +724,7 @@ typedef struct { /** * The weight of the priority group */ - uint8_t weight; + int32_t weight; } nghttp2_priority_group; /** @@ -2073,13 +2080,21 @@ const char* nghttp2_strerror(int lib_error_code); * @function * * Initializes |pri_spec| with priority group ID |pri_group_id| and - * its weight |weight|. To specify weight for the default priority - * group (which is the same as the stream ID of the stream) in - * `nghttp2_submit_request()` and `nghttp2_submit_headers()` and its - * stream ID is not known in advance, specify -1 to |pri_group_id|. + * its weight |weight|. + * + * The |weight| must be in [:enum:`NGHTTP2_MIN_WEIGHT`, + * :enum:`NGHTTP2_MAX_WEIGHT`], inclusive. If |weight| is strictly + * less than :enum:`NGHTTP2_MIN_WEIGHT`, it becomes + * :enum:`NGHTTP2_MIN_WEIGHT`. If it is strictly greater than + * :enum:`NGHTTP2_MAX_WEIGHT`, it becomes :enum:`NGHTTP2_MAX_WEIGHT`. + * + * To specify weight for the default priority group (which is the same + * as the stream ID of the stream) in `nghttp2_submit_request()` and + * `nghttp2_submit_headers()` and its stream ID is not known in + * advance, specify -1 to |pri_group_id|. */ void nghttp2_priority_spec_group_init(nghttp2_priority_spec *pri_spec, - int32_t pri_group_id, uint8_t weight); + int32_t pri_group_id, int32_t weight); /** * @function diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 1ef92de0..621d63f6 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -390,7 +390,7 @@ void nghttp2_frame_pack_priority_spec(uint8_t *buf, case NGHTTP2_PRIORITY_TYPE_GROUP: nghttp2_put_uint32be(buf, pri_spec->group.pri_group_id); - buf[4] = pri_spec->group.weight; + buf[4] = pri_spec->group.weight - 1; return; case NGHTTP2_PRIORITY_TYPE_DEP: @@ -413,10 +413,10 @@ void nghttp2_frame_unpack_priority_spec(nghttp2_priority_spec *pri_spec, { if(flags & NGHTTP2_FLAG_PRIORITY_GROUP) { int32_t pri_group_id; - uint8_t weight; + int32_t weight; pri_group_id = nghttp2_get_uint32(payload) & NGHTTP2_PRI_GROUP_ID_MASK; - weight = payload[4]; + weight = payload[4] + 1; nghttp2_priority_spec_group_init(pri_spec, pri_group_id, weight); } else if(flags & NGHTTP2_FLAG_PRIORITY_DEPENDENCY) { diff --git a/lib/nghttp2_outbound_item.h b/lib/nghttp2_outbound_item.h index 37245f64..50cbb4f3 100644 --- a/lib/nghttp2_outbound_item.h +++ b/lib/nghttp2_outbound_item.h @@ -33,11 +33,11 @@ #include "nghttp2_frame.h" /* A bit higher weight for non-DATA frames */ -#define NGHTTP2_OB_EX_WEIGHT 256 +#define NGHTTP2_OB_EX_WEIGHT 300 /* Higher weight for SETTINGS */ -#define NGHTTP2_OB_SETTINGS_WEIGHT 257 +#define NGHTTP2_OB_SETTINGS_WEIGHT 301 /* Highest weight for PING */ -#define NGHTTP2_OB_PING_WEIGHT 258 +#define NGHTTP2_OB_PING_WEIGHT 302 typedef struct { nghttp2_data_provider *data_prd; diff --git a/lib/nghttp2_priority_spec.c b/lib/nghttp2_priority_spec.c index 9c9028e7..8fcfba85 100644 --- a/lib/nghttp2_priority_spec.c +++ b/lib/nghttp2_priority_spec.c @@ -25,7 +25,7 @@ #include "nghttp2_priority_spec.h" void nghttp2_priority_spec_group_init(nghttp2_priority_spec *pri_spec, - int32_t pri_group_id, uint8_t weight) + int32_t pri_group_id, int32_t weight) { pri_spec->pri_type = NGHTTP2_PRIORITY_TYPE_GROUP; pri_spec->group.pri_group_id = pri_group_id; diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 911c520a..2bf1e4d8 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -748,7 +748,7 @@ nghttp2_stream* nghttp2_session_open_stream(nghttp2_session *session, nghttp2_stream *dep_stream; nghttp2_stream *root_stream; int32_t pri_group_id; - uint8_t weight; + int32_t weight; nghttp2_stream_group *stream_group; dep_stream = NULL; @@ -1883,7 +1883,7 @@ static int session_call_on_frame_send(nghttp2_session *session, static void outbound_item_cycle_weight(nghttp2_outbound_item *item, int32_t ini_weight) { - if(item->weight == 0 || item->weight > ini_weight) { + if(item->weight == NGHTTP2_MIN_WEIGHT || item->weight > ini_weight) { item->weight = ini_weight; } else { --item->weight; diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index 117fda44..fd77c907 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -102,6 +102,17 @@ static int nghttp2_submit_headers_shared return rv; } +static void adjust_priority_spec_group_weight(nghttp2_priority_spec *pri_spec) +{ + assert(pri_spec->pri_type == NGHTTP2_PRIORITY_TYPE_GROUP); + + if(pri_spec->group.weight < NGHTTP2_MIN_WEIGHT) { + pri_spec->group.weight = NGHTTP2_MIN_WEIGHT; + } else if(pri_spec->group.weight > NGHTTP2_MAX_WEIGHT) { + pri_spec->group.weight = NGHTTP2_MAX_WEIGHT; + } +} + static int nghttp2_submit_headers_shared_nva (nghttp2_session *session, uint8_t flags, @@ -114,24 +125,25 @@ static int nghttp2_submit_headers_shared_nva { ssize_t rv; nghttp2_nv *nva_copy; - nghttp2_priority_spec pri_spec_none = { + nghttp2_priority_spec copy_pri_spec = { NGHTTP2_PRIORITY_TYPE_NONE }; - const nghttp2_priority_spec *pri_spec_ptr; rv = nghttp2_nv_array_copy(&nva_copy, nva, nvlen); if(rv < 0) { return rv; } - if(pri_spec == NULL) { - pri_spec_ptr = &pri_spec_none; - } else { - pri_spec_ptr = pri_spec; + if(pri_spec) { + copy_pri_spec = *pri_spec; + + if(copy_pri_spec.pri_type == NGHTTP2_PRIORITY_TYPE_GROUP) { + adjust_priority_spec_group_weight(©_pri_spec); + } } return nghttp2_submit_headers_shared(session, flags, stream_id, - pri_spec_ptr, nva_copy, rv, data_prd, + ©_pri_spec, nva_copy, rv, data_prd, stream_user_data); } @@ -176,16 +188,21 @@ int nghttp2_submit_priority(nghttp2_session *session, uint8_t flags, { int rv; nghttp2_frame *frame; + nghttp2_priority_spec copy_pri_spec; if(pri_spec == NULL) { return NGHTTP2_ERR_INVALID_ARGUMENT; } - switch(pri_spec->pri_type) { + copy_pri_spec = *pri_spec; + + switch(copy_pri_spec.pri_type) { case NGHTTP2_PRIORITY_TYPE_GROUP: + adjust_priority_spec_group_weight(©_pri_spec); + break; case NGHTTP2_PRIORITY_TYPE_DEP: - if(stream_id == pri_spec->dep.stream_id) { + if(stream_id == copy_pri_spec.dep.stream_id) { return NGHTTP2_ERR_INVALID_ARGUMENT; } @@ -200,7 +217,7 @@ int nghttp2_submit_priority(nghttp2_session *session, uint8_t flags, return NGHTTP2_ERR_NOMEM; } - nghttp2_frame_priority_init(&frame->priority, stream_id, pri_spec); + nghttp2_frame_priority_init(&frame->priority, stream_id, ©_pri_spec); rv = nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, NULL);