From 3cd08251cae2e4858abc8b127b2773d64e350f3d Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Sun, 24 Aug 2014 11:32:44 -0400 Subject: [PATCH] Send window size API extension Motivation: The send window size is currently fixed by a macro at compile time. In order for users of the library to impact the send window size they would have to change a macro at compile time. The window size may be dynamic depending on the environment and deployment scheme. The library users currently have no way to change this parameter. Modifications: Add a new optional callback method which is called before data is sent to obtain the desired send window size. The callback return value will be subject to a range check for the current session, stream, and settings limits defined by flow control. Result: Library users have control over their send sizes. --- lib/includes/nghttp2/nghttp2.h | 32 ++++++++++- lib/nghttp2_frame.h | 8 ++- lib/nghttp2_session.c | 81 +++++++++++++++++++------- lib/nghttp2_submit.c | 2 +- tests/main.c | 4 ++ tests/nghttp2_session_test.c | 101 ++++++++++++++++++++++++++++++++- tests/nghttp2_session_test.h | 2 + 7 files changed, 202 insertions(+), 28 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 0031789b..421b58d9 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -598,7 +598,7 @@ typedef struct { */ int32_t stream_id; /** - * The type of this frame. See `nghttp2_frame`. + * The type of this frame. See `nghttp2_frame_type`. */ uint8_t type; /** @@ -642,6 +642,30 @@ typedef enum { NGHTTP2_DATA_FLAG_EOF = 0x01 } nghttp2_data_flag; +/** + * @functypedef + * + * Callback function invoked when |session| wants to get max |length| + * of data to send data to the remote peer. The implementation of this + * function should return a value in the following range. + * [1, min(session window, stream window, settings remote max frame size)]. + * If a window size greater than this range is returned than the max allow + * value will be used. Returning a window size smaller than this range is + * a callback error. The frame_type is provided for future extensibility + * and identifies the type of frame (see nghttp2_frame_type) for which to + * get the |length| for. Currently supported frame types are: NGHTTP2_DATA. + * + * This callback can be used to control the |length| in bytes + * for which `nghttp2_data_source_read_callback()` is allowed to send to the + * remote endpoint. This callback is optional. + * Returning :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` will signal the entire session + * failure. + */ +typedef ssize_t (*nghttp2_data_source_read_length_callback) +(nghttp2_session *session, int32_t stream_id, int32_t session_remote_window_size, + int32_t stream_remote_window_size, uint32_t remote_max_frame_size, uint8_t frame_type, + void *user_data); + /** * @functypedef * @@ -1475,6 +1499,11 @@ typedef struct { * frame. */ nghttp2_select_padding_callback select_padding_callback; + /** + * The callback function used to determine the |length| allowed + * in `nghttp2_data_source_read_callback()` + */ + nghttp2_data_source_read_length_callback read_length_callback; } nghttp2_session_callbacks; struct nghttp2_option; @@ -1985,7 +2014,6 @@ int32_t nghttp2_session_get_effective_local_window_size int32_t nghttp2_session_get_stream_remote_window_size(nghttp2_session* session, int32_t stream_id); - /** * @function * diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index 4361fb20..6fa86af8 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -55,8 +55,9 @@ /* Number of inbound buffer */ #define NGHTTP2_FRAMEBUF_MAX_NUM 5 -/* The maximum length of DATA frame payload. */ -#define NGHTTP2_DATA_PAYLOADLEN 4096 +/* The default length of DATA frame payload. This should be small enough + * for the data payload and the header to fit into 1 TLS record */ +#define NGHTTP2_DATA_PAYLOADLEN ((NGHTTP2_MAX_FRAME_SIZE_MIN) - (NGHTTP2_FRAME_HDLEN)) /* Maximum headers payload length, calculated in compressed form. This applies to transmission only. */ @@ -79,6 +80,9 @@ NGHTTP2_ALTSVC_FIXED_PARTLEN + Host-Len. */ #define NGHTTP2_ALTSVC_MINLEN 8 +/* Maximum length of padding in bytes. */ +#define NGHTTP2_MAX_PADLEN 256 + /* Category of frames. */ typedef enum { /* non-DATA frame */ diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 21a8b55e..dadd4188 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1323,33 +1323,41 @@ static int nghttp2_session_predicate_settings_send(nghttp2_session *session, return 0; } -/* - * Returns the maximum length of next data read. If the - * connection-level and/or stream-wise flow control are enabled, the - * return value takes into account those current window sizes. - */ -static size_t nghttp2_session_next_data_read(nghttp2_session *session, - nghttp2_stream *stream) +/* Take into account settings max frame size and both connection-level flow control here */ +static ssize_t nghttp2_session_enforce_flow_control_limits(nghttp2_session *session, + nghttp2_stream *stream, + ssize_t requested_window_size) { - int32_t window_size = NGHTTP2_DATA_PAYLOADLEN; - DEBUGF(fprintf(stderr, - "send: remote windowsize connection=%d, " + "send: remote windowsize connection=%d, remote maxframsize=%u, " "stream(id %d)=%d\n", session->remote_window_size, + session->remote_settings.max_frame_size, stream->stream_id, stream->remote_window_size)); - /* Take into account both connection-level flow control here */ - window_size = nghttp2_min(window_size, session->remote_window_size); - window_size = nghttp2_min(window_size, stream->remote_window_size); + return nghttp2_min( + nghttp2_min(nghttp2_min(requested_window_size, stream->remote_window_size), + session->remote_window_size), + session->remote_settings.max_frame_size); +} - DEBUGF(fprintf(stderr, "send: available window=%d\n", window_size)); +/* + * Returns the maximum length of next data read. If the + * connection-level and/or stream-wise flow control are enabled, the + * return value takes into account those current window sizes. The remote + * settings for max frame size is also taken into account. + */ +static size_t nghttp2_session_next_data_read(nghttp2_session *session, + nghttp2_stream *stream) +{ + ssize_t window_size = nghttp2_session_enforce_flow_control_limits(session, + stream, + NGHTTP2_DATA_PAYLOADLEN); - if(window_size > 0) { - return window_size; - } - return 0; + DEBUGF(fprintf(stderr, "send: available window=%zd\n", window_size)); + + return window_size > 0 ? (size_t) window_size : 0; } /* @@ -1414,8 +1422,7 @@ static ssize_t session_call_select_padding(nghttp2_session *session, if(session->callbacks.select_padding_callback) { size_t max_paddedlen; - /* 256 is maximum padding size */ - max_paddedlen = nghttp2_min(frame->hd.length + 256, max_payloadlen); + max_paddedlen = nghttp2_min(frame->hd.length + NGHTTP2_MAX_PADLEN, max_payloadlen); rv = session->callbacks.select_padding_callback(session, frame, max_paddedlen, @@ -1444,7 +1451,7 @@ static int session_headers_add_pad(nghttp2_session *session, aob = &session->aob; framebufs = &aob->framebufs; - max_payloadlen = nghttp2_min(NGHTTP2_MAX_PAYLOADLEN, frame->hd.length + 256); + max_payloadlen = nghttp2_min(NGHTTP2_MAX_PAYLOADLEN, frame->hd.length + NGHTTP2_MAX_PADLEN); padded_payloadlen = session_call_select_padding(session, frame, max_payloadlen); @@ -5568,6 +5575,36 @@ int nghttp2_session_pack_data(nghttp2_session *session, buf = &bufs->cur->buf; + if(session->callbacks.read_length_callback) { + nghttp2_stream *stream = nghttp2_session_get_stream(session, frame->hd.stream_id); + if(!stream) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + + payloadlen = session->callbacks.read_length_callback(session, stream->stream_id, + session->remote_window_size, stream->remote_window_size, + session->remote_settings.max_frame_size, frame->hd.type, session->user_data); + DEBUGF(fprintf(stderr, "send: read_length_callback=%zd\n", payloadlen)); + payloadlen = nghttp2_session_enforce_flow_control_limits(session, stream, payloadlen); + DEBUGF(fprintf(stderr, "send: read_length_callback after flow control=%zd\n", payloadlen)); + if(payloadlen <= 0) { + return NGHTTP2_ERR_CALLBACK_FAILURE; + } else if(payloadlen > nghttp2_buf_avail(buf)) { + // Resize the current buffer(s) + nghttp2_bufs_free(&session->aob.framebufs); + + // The reason why we do +1 for buffer size is for possible padding field. + rv = nghttp2_bufs_init3(&session->aob.framebufs, + NGHTTP2_FRAME_HDLEN + 1 + payloadlen, + NGHTTP2_FRAMEBUF_MAX_NUM, + 1, NGHTTP2_FRAME_HDLEN + 1); + if(rv != 0) { + return rv; + } + } + datamax = (size_t) payloadlen; + } + /* Current max DATA length is less then buffer chunk size */ assert(nghttp2_buf_avail(buf) >= (ssize_t)datamax); @@ -5611,7 +5648,7 @@ int nghttp2_session_pack_data(nghttp2_session *session, data_frame.hd.flags = flags; data_frame.data.padlen = 0; - max_payloadlen = nghttp2_min(datamax, data_frame.hd.length + 256); + max_payloadlen = nghttp2_min(datamax, data_frame.hd.length + NGHTTP2_MAX_PADLEN); padded_payloadlen = session_call_select_padding(session, &data_frame, max_payloadlen); diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index eea748b9..c89dcc54 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -333,7 +333,7 @@ int nghttp2_submit_window_update(nghttp2_session *session, uint8_t flags, int32_t window_size_increment) { int rv; - nghttp2_stream *stream; + nghttp2_stream *stream = 0; if(window_size_increment == 0) { return 0; } diff --git a/tests/main.c b/tests/main.c index 6db64cbd..ad66c36f 100644 --- a/tests/main.c +++ b/tests/main.c @@ -142,6 +142,8 @@ int main(int argc, char* argv[]) !CU_add_test(pSuite, "session_reprioritize_stream", test_nghttp2_session_reprioritize_stream) || !CU_add_test(pSuite, "submit_data", test_nghttp2_submit_data) || + !CU_add_test(pSuite, "submit_data_read_length_too_large", + test_nghttp2_submit_data_read_length_too_large) || !CU_add_test(pSuite, "submit_request_with_data", test_nghttp2_submit_request_with_data) || !CU_add_test(pSuite, "submit_request_without_data", @@ -237,6 +239,8 @@ int main(int argc, char* argv[]) test_nghttp2_frame_pack_headers) || !CU_add_test(pSuite, "frame_pack_headers_frame_too_large", test_nghttp2_frame_pack_headers_frame_too_large) || + !CU_add_test(pSuite, "frame_pack_headers_frame_smallest", + test_nghttp2_submit_data_read_length_smallest) || !CU_add_test(pSuite, "frame_pack_priority", test_nghttp2_frame_pack_priority) || !CU_add_test(pSuite, "frame_pack_rst_stream", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 0e3b13bc..a6aa10f6 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -228,6 +228,20 @@ static ssize_t select_padding_callback(nghttp2_session *session, return nghttp2_min(max_payloadlen, frame->hd.length + ud->padlen); } +static ssize_t too_large_data_source_length_callback +(nghttp2_session *session, int32_t stream_id, + int32_t session_remote_window_size, int32_t stream_remote_window_size, + uint32_t remote_max_frame_size, uint8_t frame_type, void *user_data) { + return NGHTTP2_MAX_FRAME_SIZE_MAX + 1; +} + +static ssize_t smallest_length_data_source_length_callback +(nghttp2_session *session, int32_t stream_id, + int32_t session_remote_window_size, int32_t stream_remote_window_size, + uint32_t remote_max_frame_size, uint8_t frame_type, void *user_data) { + return 1; +} + static ssize_t fixed_length_data_source_read_callback (nghttp2_session *session, int32_t stream_id, uint8_t *buf, size_t len, uint32_t *data_flags, @@ -3005,6 +3019,92 @@ void test_nghttp2_submit_data(void) nghttp2_session_del(session); } +void test_nghttp2_submit_data_read_length_too_large(void) +{ + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_data_provider data_prd; + my_user_data ud; + nghttp2_private_data *data_frame; + nghttp2_frame_hd hd; + nghttp2_active_outbound_item *aob; + nghttp2_bufs *framebufs; + nghttp2_buf *buf; + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.send_callback = block_count_send_callback; + callbacks.read_length_callback = too_large_data_source_length_callback; + + data_prd.read_callback = fixed_length_data_source_read_callback; + ud.data_source_length = NGHTTP2_DATA_PAYLOADLEN * 2; + CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, &ud)); + aob = &session->aob; + framebufs = &aob->framebufs; + + nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE, + &pri_spec_default, NGHTTP2_STREAM_OPENING, + NULL); + CU_ASSERT(0 == nghttp2_submit_data(session, + NGHTTP2_FLAG_END_STREAM, 1, &data_prd)); + + ud.block_count = 0; + CU_ASSERT(0 == nghttp2_session_send(session)); + data_frame = nghttp2_outbound_item_get_data_frame(aob->item); + + buf = &framebufs->head->buf; + nghttp2_frame_unpack_frame_hd(&hd, buf->pos); + + CU_ASSERT(NGHTTP2_FLAG_NONE == hd.flags); + CU_ASSERT(16384 == hd.length) + /* frame->hd.flags has these flags */ + CU_ASSERT(NGHTTP2_FLAG_END_STREAM == data_frame->hd.flags); + + nghttp2_session_del(session); +} + +void test_nghttp2_submit_data_read_length_smallest(void) +{ + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_data_provider data_prd; + my_user_data ud; + nghttp2_private_data *data_frame; + nghttp2_frame_hd hd; + nghttp2_active_outbound_item *aob; + nghttp2_bufs *framebufs; + nghttp2_buf *buf; + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.send_callback = block_count_send_callback; + callbacks.read_length_callback = smallest_length_data_source_length_callback; + + data_prd.read_callback = fixed_length_data_source_read_callback; + ud.data_source_length = NGHTTP2_DATA_PAYLOADLEN * 2; + CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, &ud)); + aob = &session->aob; + framebufs = &aob->framebufs; + + nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE, + &pri_spec_default, NGHTTP2_STREAM_OPENING, + NULL); + CU_ASSERT(0 == nghttp2_submit_data(session, + NGHTTP2_FLAG_END_STREAM, 1, &data_prd)); + + ud.block_count = 0; + CU_ASSERT(0 == nghttp2_session_send(session)); + data_frame = nghttp2_outbound_item_get_data_frame(aob->item); + + buf = &framebufs->head->buf; + nghttp2_frame_unpack_frame_hd(&hd, buf->pos); + + CU_ASSERT(NGHTTP2_FLAG_NONE == hd.flags); + CU_ASSERT(1 == hd.length) + /* frame->hd.flags has these flags */ + CU_ASSERT(NGHTTP2_FLAG_END_STREAM == data_frame->hd.flags); + + nghttp2_session_del(session); +} + void test_nghttp2_submit_request_with_data(void) { nghttp2_session *session; @@ -4303,7 +4403,6 @@ void test_nghttp2_session_defer_data(void) CU_ASSERT(ud.data_source_length == NGHTTP2_DATA_PAYLOADLEN * 2); /* Resume deferred DATA */ - CU_ASSERT(0 == nghttp2_session_resume_data(session, 1)); item = nghttp2_session_get_ob_pq_top(session); OB_DATA(item)->data_prd.read_callback = diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 0cdd7394..92c1abde 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -62,6 +62,8 @@ void test_nghttp2_session_is_my_stream_id(void); void test_nghttp2_session_upgrade(void); void test_nghttp2_session_reprioritize_stream(void); void test_nghttp2_submit_data(void); +void test_nghttp2_submit_data_read_length_too_large(void); +void test_nghttp2_submit_data_read_length_smallest(void); void test_nghttp2_submit_request_with_data(void); void test_nghttp2_submit_request_without_data(void); void test_nghttp2_submit_response_with_data(void);