From 0c9703fa2c7181497e1b0a63947076d3ae3d9dfc Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 18 Aug 2013 22:04:09 +0900 Subject: [PATCH] Remove NGHTTP2_FLAG_END_FLOW_CONTROL --- lib/includes/nghttp2/nghttp2.h | 12 +++---- lib/nghttp2_session.c | 37 +++------------------ lib/nghttp2_submit.c | 16 ++------- src/HttpServer.cc | 19 ++--------- src/HttpServer.h | 3 +- src/app_helper.cc | 5 --- src/nghttp.cc | 40 ++++++----------------- src/nghttpd.cc | 19 ++++------- tests/nghttp2_frame_test.c | 4 +-- tests/nghttp2_session_test.c | 60 +++++----------------------------- 10 files changed, 41 insertions(+), 174 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 3ba1fe57..b17017d4 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -337,11 +337,7 @@ typedef enum { /** * The PONG flag. */ - NGHTTP2_FLAG_PONG = 0x1, - /** - * The END_FLOW_CONTROL flag. - */ - NGHTTP2_FLAG_END_FLOW_CONTROL = 0x1 + NGHTTP2_FLAG_PONG = 0x1 } nghttp2_flag; /** @@ -1738,6 +1734,8 @@ int nghttp2_submit_goaway(nghttp2_session *session, * * Submits WINDOW_UPDATE frame. * + * The |flags| is currently ignored. + * * If the |window_size_increment| is positive, the WINDOW_UPDATE with * that value as window_size_increment is queued. If the * |window_size_increment| is larger than the received bytes from the @@ -1756,9 +1754,7 @@ int nghttp2_submit_goaway(nghttp2_session *session, * negative error codes: * * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` - * The |delta_window_size| is 0 and - * :enum:`NGHTTP2_FLAG_END_FLOW_CONTROL` bit is not set in - * |flags|. + * The |delta_window_size| is 0. * :enum:`NGHTTP2_ERR_FLOW_CONTROL` * The local window size overflow or gets negative. * :enum:`NGHTTP2_ERR_STREAM_CLOSED` diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index cf18a393..6ddea60d 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -2268,24 +2268,13 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, if(frame->hd.stream_id == 0) { /* Handle connection-level flow control */ if(session->remote_flow_control == 0) { - /* The sepc says receiving WINDOW_UPDATE from peer when flow - control is disabled is error, but disabling flow control and - receiving WINDOW_UPDATE are asynchronous, so it is hard to - determine that the peer is misbehaving or not without - measuring RTT. For now, we just ignore such frames. */ + /* Disabling flow control by sending SETTINGS and receiving + WINDOW_UPDATE are asynchronous, so it is hard to determine + that the peer is misbehaving or not without measuring + RTT. For now, we just ignore such frames. */ nghttp2_session_call_on_frame_received(session, frame); return 0; } - if(frame->hd.flags & NGHTTP2_FLAG_END_FLOW_CONTROL) { - if(session->remote_flow_control) { - /* Disable connection-level flow control and push back - deferred DATA frame if any */ - session->remote_flow_control = 0; - nghttp2_session_call_on_frame_received(session, frame); - return nghttp2_session_push_back_deferred_data(session); - } - return 0; - } if(NGHTTP2_MAX_WINDOW_SIZE - frame->window_update.window_size_increment < session->remote_window_size) { return nghttp2_session_handle_invalid_connection @@ -2309,24 +2298,6 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, nghttp2_session_call_on_frame_received(session, frame); return 0; } - if(frame->hd.flags & NGHTTP2_FLAG_END_FLOW_CONTROL) { - stream->remote_flow_control = 0; - if(stream->remote_flow_control && - stream->deferred_data != NULL && - (stream->deferred_flags & NGHTTP2_DEFERRED_FLOW_CONTROL)) { - int r; - r = nghttp2_pq_push(&session->ob_pq, stream->deferred_data); - if(r == 0) { - nghttp2_stream_detach_deferred_data(stream); - } else if(r < 0) { - /* FATAL */ - assert(r < NGHTTP2_ERR_FATAL); - return r; - } - } - nghttp2_session_call_on_frame_received(session, frame); - return 0; - } if(NGHTTP2_MAX_WINDOW_SIZE - frame->window_update.window_size_increment < stream->remote_window_size) { int r; diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index 32ffdd79..ba8867e3 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -238,22 +238,10 @@ int nghttp2_submit_window_update(nghttp2_session *session, uint8_t flags, { int rv; nghttp2_stream *stream; - flags &= NGHTTP2_FLAG_END_FLOW_CONTROL; - if(flags & NGHTTP2_FLAG_END_FLOW_CONTROL) { - if(stream_id == 0) { - session->local_flow_control = 0; - } else { - stream = nghttp2_session_get_stream(session, stream_id); - if(stream) { - stream->local_flow_control = 0; - } else { - return NGHTTP2_ERR_STREAM_CLOSED; - } - } - return nghttp2_session_add_window_update(session, flags, stream_id, 0); - } else if(window_size_increment == 0) { + if(window_size_increment == 0) { return NGHTTP2_ERR_INVALID_ARGUMENT; } + flags = 0; if(stream_id == 0) { if(!session->local_flow_control) { return NGHTTP2_ERR_INVALID_ARGUMENT; diff --git a/src/HttpServer.cc b/src/HttpServer.cc index c9d8f9ff..a12bebd7 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -71,8 +71,7 @@ Config::Config() data_ptr(nullptr), verify_client(false), no_tls(false), - no_connection_flow_control(false), - no_stream_flow_control(false), + no_flow_control(false), output_upper_thres(1024*1024) {} @@ -353,8 +352,7 @@ int Http2Handler::on_connect() size_t niv = 1; entry[0].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; entry[0].value = 100; - if(sessions_->get_config()->no_connection_flow_control && - sessions_->get_config()->no_stream_flow_control) { + if(sessions_->get_config()->no_flow_control) { entry[niv].settings_id = NGHTTP2_SETTINGS_FLOW_CONTROL_OPTIONS; entry[niv].value = 1; ++niv; @@ -363,14 +361,6 @@ int Http2Handler::on_connect() if(r != 0) { return r; } - if(sessions_->get_config()->no_connection_flow_control && - !sessions_->get_config()->no_stream_flow_control) { - r = nghttp2_submit_window_update(session_, NGHTTP2_FLAG_END_FLOW_CONTROL, - 0, 0); - if(r != 0) { - return r; - } - } return on_write(); } @@ -717,11 +707,6 @@ void hd_on_frame_recv_callback auto req = util::make_unique(stream_id); append_nv(req.get(), frame->headers.nva, frame->headers.nvlen); hd->add_stream(stream_id, std::move(req)); - if(!hd->get_config()->no_connection_flow_control && - hd->get_config()->no_stream_flow_control) { - nghttp2_submit_window_update(session, NGHTTP2_FLAG_END_FLOW_CONTROL, - stream_id, 0); - } break; } case NGHTTP2_HCAT_HEADERS: diff --git a/src/HttpServer.h b/src/HttpServer.h index ed4aba0e..fe1d9568 100644 --- a/src/HttpServer.h +++ b/src/HttpServer.h @@ -57,8 +57,7 @@ struct Config { void *data_ptr; bool verify_client; bool no_tls; - bool no_connection_flow_control; - bool no_stream_flow_control; + bool no_flow_control; size_t output_upper_thres; Config(); }; diff --git a/src/app_helper.cc b/src/app_helper.cc index 17ea59bb..d2440db2 100644 --- a/src/app_helper.cc +++ b/src/app_helper.cc @@ -217,11 +217,6 @@ void print_flags(const nghttp2_frame_hd& hd) s += "PONG"; } break; - case NGHTTP2_WINDOW_UPDATE: - if(hd.flags & NGHTTP2_FLAG_END_FLOW_CONTROL) { - s += "END_FLOW_CONTROL"; - } - break; } printf("; %s\n", s.c_str()); } diff --git a/src/nghttp.cc b/src/nghttp.cc index d559509d..26e7ec3e 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -79,8 +79,7 @@ struct Config { bool verbose; bool get_assets; bool stat; - bool no_connection_flow_control; - bool no_stream_flow_control; + bool no_flow_control; bool upgrade; int32_t pri; int multiply; @@ -98,8 +97,7 @@ struct Config { verbose(false), get_assets(false), stat(false), - no_connection_flow_control(false), - no_stream_flow_control(false), + no_flow_control(false), upgrade(false), pri(NGHTTP2_PRI_DEFAULT), multiply(1), @@ -334,7 +332,7 @@ size_t populate_settings(nghttp2_settings_entry *iv) } else { iv[1].value = NGHTTP2_INITIAL_WINDOW_SIZE; } - if(config.no_connection_flow_control && config.no_stream_flow_control) { + if(config.no_flow_control) { iv[niv].settings_id = NGHTTP2_SETTINGS_FLOW_CONTROL_OPTIONS; iv[niv].value = 1; ++niv; @@ -653,13 +651,6 @@ struct HttpClient { return -1; } } - if(config.no_connection_flow_control && !config.no_stream_flow_control) { - rv = nghttp2_submit_window_update(session, NGHTTP2_FLAG_END_FLOW_CONTROL, - 0, 0); - if(rv != 0) { - return -1; - } - } // Adjust first request depending on the existence of the upload // data for(auto i = std::begin(reqvec)+(need_upgrade() && !reqvec[0]->data_prd); @@ -983,12 +974,6 @@ void check_stream_id(nghttp2_session *session, int32_t stream_id, stream_id); client->streams[stream_id] = req; req->record_syn_stream_time(); - - if(!config.no_connection_flow_control && config.no_stream_flow_control) { - nghttp2_submit_window_update(session, - NGHTTP2_FLAG_END_FLOW_CONTROL, - stream_id, 0); - } } } // namespace @@ -1421,7 +1406,7 @@ int run(char **uris, int n) void print_usage(std::ostream& out) { - out << "Usage: nghttp [-FOafnsuv] [-t ] [-w ] [--cert=]\n" + out << "Usage: nghttp [-Oafnsuv] [-t ] [-w ] [--cert=]\n" << " [--key=] [-d ] [-m ] [-p ]\n" << " ..." << std::endl; @@ -1458,10 +1443,9 @@ void print_help(std::ostream& out) << " -m, --multiply= Request each URI times. By default, same\n" << " URI is not requested twice. This option\n" << " disables it too.\n" - << " -F, --no-connection-flow-control\n" - << " Disables connection level flow control.\n" - << " -f, --no-stream-flow-control\n" - << " Disables stream level flow control.\n" + << " -f, --no-low-control\n" + << " Disables connection and stream level flow\n" + << " controls.\n" << " -u, --upgrade Perform HTTP Upgrade for HTTP/2.0. This\n" << " option is ignored if the request URI has\n" << " https scheme.\n" @@ -1491,27 +1475,23 @@ int main(int argc, char **argv) {"header", required_argument, 0, 'H' }, {"data", required_argument, 0, 'd' }, {"multiply", required_argument, 0, 'm' }, - {"no-connection-flow-control", no_argument, 0, 'F'}, - {"no-stream-flow-control", no_argument, 0, 'f'}, + {"no-flow-control", no_argument, 0, 'f'}, {"upgrade", no_argument, 0, 'u'}, {"pri", required_argument, 0, 'p'}, {0, 0, 0, 0 } }; int option_index = 0; - int c = getopt_long(argc, argv, "FOad:fm:np:hH:vst:uw:", long_options, + int c = getopt_long(argc, argv, "Oad:fm:np:hH:vst:uw:", long_options, &option_index); if(c == -1) { break; } switch(c) { - case 'F': - config.no_connection_flow_control = true; - break; case 'O': config.remote_name = true; break; case 'f': - config.no_stream_flow_control = true; + config.no_flow_control = true; break; case 'h': print_help(std::cout); diff --git a/src/nghttpd.cc b/src/nghttpd.cc index 28ab2c12..ca534c15 100644 --- a/src/nghttpd.cc +++ b/src/nghttpd.cc @@ -45,7 +45,7 @@ namespace nghttp2 { namespace { void print_usage(std::ostream& out) { - out << "Usage: nghttpd [-FDVfhv] [-d ] [--no-tls] [ ]" + out << "Usage: nghttpd [-DVfhv] [-d ] [--no-tls] [ ]" << std::endl; } } // namespace @@ -73,10 +73,9 @@ void print_help(std::ostream& out) << " -v, --verbose Print debug information such as reception/\n" << " transmission of frames and name/value pairs.\n" << " --no-tls Disable SSL/TLS.\n" - << " -F, --no-connection-flow-control\n" - << " Disables connection level flow control.\n" - << " -f, --no-stream-flow-control\n" - << " Disables stream level flow control.\n" + << " -f, --no-flow-control\n" + << " Disables connection and stream level flow\n" + << " controls.\n" << " -h, --help Print this help.\n" << std::endl; } @@ -94,19 +93,15 @@ int main(int argc, char **argv) {"verbose", no_argument, 0, 'v' }, {"verify-client", no_argument, 0, 'V' }, {"no-tls", no_argument, &flag, 1 }, - {"no-connection-flow-control", no_argument, 0, 'F'}, - {"no-stream-flow-control", no_argument, 0, 'f'}, + {"no-flow-control", no_argument, 0, 'f'}, {0, 0, 0, 0 } }; int option_index = 0; - int c = getopt_long(argc, argv, "FDVd:fhv", long_options, &option_index); + int c = getopt_long(argc, argv, "DVd:fhv", long_options, &option_index); if(c == -1) { break; } switch(c) { - case 'F': - config.no_connection_flow_control = true; - break; case 'D': config.daemon = true; break; @@ -117,7 +112,7 @@ int main(int argc, char **argv) config.htdocs = optarg; break; case 'f': - config.no_stream_flow_control = true; + config.no_flow_control = true; break; case 'h': print_help(std::cout); diff --git a/tests/nghttp2_frame_test.c b/tests/nghttp2_frame_test.c index 8d4cc7fa..beb1ec1b 100644 --- a/tests/nghttp2_frame_test.c +++ b/tests/nghttp2_frame_test.c @@ -368,7 +368,7 @@ void test_nghttp2_frame_pack_window_update(void) uint8_t *buf = NULL; size_t buflen = 0; ssize_t framelen; - nghttp2_frame_window_update_init(&frame, NGHTTP2_FLAG_END_FLOW_CONTROL, + nghttp2_frame_window_update_init(&frame, NGHTTP2_FLAG_NONE, 1000000007, 4096); framelen = nghttp2_frame_pack_window_update(&buf, &buflen, &frame); @@ -377,7 +377,7 @@ void test_nghttp2_frame_pack_window_update(void) &buf[0], NGHTTP2_FRAME_HEAD_LENGTH, &buf[NGHTTP2_FRAME_HEAD_LENGTH], framelen - NGHTTP2_FRAME_HEAD_LENGTH)); - check_frame_header(4, NGHTTP2_WINDOW_UPDATE, NGHTTP2_FLAG_END_FLOW_CONTROL, + check_frame_header(4, NGHTTP2_WINDOW_UPDATE, NGHTTP2_FLAG_NONE, 1000000007, &oframe.hd); CU_ASSERT(4096 == oframe.window_size_increment); free(buf); diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 42bd983b..ab69a552 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1158,25 +1158,6 @@ void test_nghttp2_session_on_window_update_received(void) nghttp2_frame_window_update_free(&frame.window_update); - /* Check END_FLOW_CONTROL flag */ - user_data.frame_recv_cb_called = 0; - nghttp2_frame_window_update_init(&frame.window_update, - NGHTTP2_FLAG_END_FLOW_CONTROL, 1, 0); - CU_ASSERT(0 == nghttp2_session_on_window_update_received(session, &frame)); - CU_ASSERT(1 == user_data.frame_recv_cb_called); - CU_ASSERT(0 == stream->remote_flow_control); - - nghttp2_frame_window_update_free(&frame.window_update); - - user_data.frame_recv_cb_called = 0; - nghttp2_frame_window_update_init(&frame.window_update, - NGHTTP2_FLAG_END_FLOW_CONTROL, 0, 0); - CU_ASSERT(0 == nghttp2_session_on_window_update_received(session, &frame)); - CU_ASSERT(1 == user_data.frame_recv_cb_called); - CU_ASSERT(0 == session->remote_flow_control); - - nghttp2_frame_window_update_free(&frame.window_update); - nghttp2_session_del(session); } @@ -2114,23 +2095,10 @@ void test_nghttp2_submit_window_update(void) CU_ASSERT(0 == nghttp2_session_send(session)); CU_ASSERT(0 == stream->recv_window_size); - /* Disable stream-level flow control */ - CU_ASSERT(0 == nghttp2_submit_window_update(session, - NGHTTP2_FLAG_END_FLOW_CONTROL, - 2, 0)); - CU_ASSERT(0 == nghttp2_session_send(session)); - CU_ASSERT(0 == stream->local_flow_control); - - /* Disable connection-level flow control */ - CU_ASSERT(0 == nghttp2_submit_window_update(session, - NGHTTP2_FLAG_END_FLOW_CONTROL, - 0, 0)); - CU_ASSERT(0 == nghttp2_session_send(session)); - CU_ASSERT(0 == session->local_flow_control); - - CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 2, 0)); + /* Disable local flow control */ + stream->local_flow_control = 0; CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 2, -1)); CU_ASSERT(NGHTTP2_ERR_STREAM_CLOSED == @@ -2734,6 +2702,7 @@ void test_nghttp2_session_flow_control_disable_remote(void) nghttp2_data_provider data_prd; nghttp2_frame frame; size_t data_size = 128*1024; + nghttp2_settings_entry iv = { NGHTTP2_SETTINGS_FLOW_CONTROL_OPTIONS, 0x1 }; memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks.send_callback = null_send_callback; @@ -2751,31 +2720,20 @@ void test_nghttp2_session_flow_control_disable_remote(void) CU_ASSERT(0 == nghttp2_session_send(session)); CU_ASSERT(data_size - NGHTTP2_INITIAL_WINDOW_SIZE == ud.data_source_length); - /* Disable stream flow control */ - nghttp2_frame_window_update_init(&frame.window_update, - NGHTTP2_FLAG_END_FLOW_CONTROL, 1, 0); - nghttp2_session_on_window_update_received(session, &frame); + /* Disable flow control entirely */ + nghttp2_frame_settings_init(&frame.settings, dup_iv(&iv, 1), 1); + nghttp2_session_on_settings_received(session, &frame); - /* Check stream-level remote_flow_control is disabled */ + /* Check both connection and stream-level remote_flow_control is + disabled */ CU_ASSERT(0 == nghttp2_session_get_stream(session, 1)->remote_flow_control); - - /* Still nothing is sent because of connection-level flow control */ - CU_ASSERT(0 == nghttp2_session_send(session)); - CU_ASSERT(data_size - NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE == - ud.data_source_length); - - /* Disable connection-level flow control */ - frame.hd.stream_id = 0; - nghttp2_session_on_window_update_received(session, &frame); - - /* Check connection-level remote_flow_control is disabled */ CU_ASSERT(0 == session->remote_flow_control); /* Sends remaining data */ CU_ASSERT(0 == nghttp2_session_send(session)); CU_ASSERT(0 == ud.data_source_length); - nghttp2_frame_window_update_free(&frame.window_update); + nghttp2_frame_settings_free(&frame.settings); nghttp2_session_del(session); }