http: fix client receiving large data
HTTP client was relying on synchronous rx notifications to the client app when moving lage data from underlying transport proto. Recent change in session layer made such notifications asynchronous making http client not working. This patch fixes the issue. Type: fix Change-Id: I4b24c6185a594a0fe8d5d87c149c53d3b40d7110 Signed-off-by: Filip Tehlar <ftehlar@cisco.com> Signed-off-by: Matus Fabian <matfabia@cisco.com>
This commit is contained in:

committed by
Florin Coras

parent
1db11b7d8d
commit
d894438f04
@ -49,7 +49,7 @@ func HttpCliTest(s *VethsSuite) {
|
||||
uri := "http://" + serverVeth.ip4AddressString() + "/80"
|
||||
|
||||
o := clientContainer.vppInstance.vppctl("http cli client" +
|
||||
" uri " + uri + " query /show/version")
|
||||
" uri " + uri + " query /show/vlib/graph")
|
||||
|
||||
s.log(o)
|
||||
s.assertContains(o, "<html>", "<html> not found in the result!")
|
||||
|
@ -370,7 +370,7 @@ hcc_connect ()
|
||||
}
|
||||
|
||||
static clib_error_t *
|
||||
hcc_run (vlib_main_t *vm)
|
||||
hcc_run (vlib_main_t *vm, int print_output)
|
||||
{
|
||||
vlib_thread_main_t *vtm = vlib_get_thread_main ();
|
||||
hcc_main_t *hcm = &hcc_main;
|
||||
@ -407,7 +407,8 @@ hcc_run (vlib_main_t *vm)
|
||||
goto cleanup;
|
||||
|
||||
case HCC_REPLY_RECEIVED:
|
||||
vlib_cli_output (vm, "%v", hcm->http_response);
|
||||
if (print_output)
|
||||
vlib_cli_output (vm, "%v", hcm->http_response);
|
||||
vec_free (hcm->http_response);
|
||||
break;
|
||||
default:
|
||||
@ -448,7 +449,7 @@ hcc_command_fn (vlib_main_t *vm, unformat_input_t *input,
|
||||
u64 seg_size;
|
||||
u8 *appns_id = 0;
|
||||
clib_error_t *err = 0;
|
||||
int rv;
|
||||
int rv, print_output = 1;
|
||||
|
||||
hcm->prealloc_fifos = 0;
|
||||
hcm->private_segment_size = 0;
|
||||
@ -472,6 +473,8 @@ hcc_command_fn (vlib_main_t *vm, unformat_input_t *input,
|
||||
hcm->fifo_size <<= 10;
|
||||
else if (unformat (line_input, "uri %s", &hcm->uri))
|
||||
;
|
||||
else if (unformat (line_input, "no-output"))
|
||||
print_output = 0;
|
||||
else if (unformat (line_input, "appns %_%v%_", &appns_id))
|
||||
;
|
||||
else if (unformat (line_input, "secret %lu", &hcm->appns_secret))
|
||||
@ -506,7 +509,7 @@ hcc_command_fn (vlib_main_t *vm, unformat_input_t *input,
|
||||
vnet_session_enable_disable (vm, 1 /* turn on TCP, etc. */);
|
||||
vlib_worker_thread_barrier_release (vm);
|
||||
|
||||
err = hcc_run (vm);
|
||||
err = hcc_run (vm, print_output);
|
||||
|
||||
if (hcc_detach ())
|
||||
{
|
||||
@ -526,7 +529,7 @@ done:
|
||||
VLIB_CLI_COMMAND (hcc_command, static) = {
|
||||
.path = "http cli client",
|
||||
.short_help = "[appns <app-ns> secret <appns-secret>] uri http://<ip-addr> "
|
||||
"query <query-string>",
|
||||
"query <query-string> [no-output]",
|
||||
.function = hcc_command_fn,
|
||||
.is_mp_safe = 1,
|
||||
};
|
||||
|
@ -74,14 +74,14 @@ format_http_state (u8 *s, va_list *va)
|
||||
return format (s, "unknown");
|
||||
}
|
||||
|
||||
static inline void
|
||||
http_state_change (http_conn_t *hc, http_state_t state)
|
||||
{
|
||||
HTTP_DBG (1, "changing http state %U -> %U", format_http_state,
|
||||
hc->http_state, format_http_state, state);
|
||||
ASSERT (hc->http_state != state);
|
||||
hc->http_state = state;
|
||||
}
|
||||
#define http_state_change(_hc, _state) \
|
||||
do \
|
||||
{ \
|
||||
HTTP_DBG (1, "changing http state %U -> %U", format_http_state, \
|
||||
(_hc)->http_state, format_http_state, _state); \
|
||||
(_hc)->http_state = _state; \
|
||||
} \
|
||||
while (0)
|
||||
|
||||
static inline http_worker_t *
|
||||
http_worker_get (u32 thread_index)
|
||||
@ -577,7 +577,7 @@ http_state_wait_server_reply (http_conn_t *hc, transport_send_params_t *sp)
|
||||
{
|
||||
hc->rx_buf_offset = 0;
|
||||
vec_reset_length (hc->rx_buf);
|
||||
http_state_change (hc, HTTP_STATE_WAIT_CLIENT_METHOD);
|
||||
http_state_change (hc, HTTP_STATE_WAIT_APP_METHOD);
|
||||
}
|
||||
else
|
||||
{
|
||||
@ -585,7 +585,8 @@ http_state_wait_server_reply (http_conn_t *hc, transport_send_params_t *sp)
|
||||
}
|
||||
|
||||
app_wrk = app_worker_get_if_valid (as->app_wrk_index);
|
||||
app_worker_rx_notify (app_wrk, as);
|
||||
if (app_wrk)
|
||||
app_worker_rx_notify (app_wrk, as);
|
||||
return HTTP_SM_STOP;
|
||||
}
|
||||
else
|
||||
@ -594,7 +595,6 @@ http_state_wait_server_reply (http_conn_t *hc, transport_send_params_t *sp)
|
||||
ec = HTTP_STATUS_METHOD_NOT_ALLOWED;
|
||||
goto error;
|
||||
}
|
||||
return HTTP_SM_STOP;
|
||||
|
||||
error:
|
||||
|
||||
@ -791,7 +791,6 @@ error:
|
||||
static http_sm_result_t
|
||||
http_state_wait_app_method (http_conn_t *hc, transport_send_params_t *sp)
|
||||
{
|
||||
http_status_code_t sc;
|
||||
http_msg_t msg;
|
||||
session_t *as;
|
||||
u8 *buf = 0, *request;
|
||||
@ -806,19 +805,15 @@ http_state_wait_app_method (http_conn_t *hc, transport_send_params_t *sp)
|
||||
if (msg.data.type > HTTP_MSG_DATA_PTR)
|
||||
{
|
||||
clib_warning ("no data");
|
||||
sc = HTTP_STATUS_INTERNAL_ERROR;
|
||||
goto error;
|
||||
}
|
||||
|
||||
if (msg.type != HTTP_MSG_REQUEST)
|
||||
{
|
||||
clib_warning ("unexpected message type %d", msg.type);
|
||||
sc = HTTP_STATUS_INTERNAL_ERROR;
|
||||
goto error;
|
||||
}
|
||||
|
||||
sc = msg.code;
|
||||
|
||||
vec_validate (buf, msg.data.len - 1);
|
||||
rv = svm_fifo_dequeue (as->tx_fifo, msg.data.len, buf);
|
||||
ASSERT (rv == msg.data.len);
|
||||
@ -828,7 +823,6 @@ http_state_wait_app_method (http_conn_t *hc, transport_send_params_t *sp)
|
||||
if (offset != vec_len (request))
|
||||
{
|
||||
clib_warning ("sending request failed!");
|
||||
sc = HTTP_STATUS_INTERNAL_ERROR;
|
||||
goto error;
|
||||
}
|
||||
|
||||
@ -837,83 +831,76 @@ http_state_wait_app_method (http_conn_t *hc, transport_send_params_t *sp)
|
||||
vec_free (buf);
|
||||
vec_free (request);
|
||||
|
||||
return HTTP_SM_CONTINUE;
|
||||
return HTTP_SM_STOP;
|
||||
|
||||
error:
|
||||
clib_warning ("unexpected msg type from app %u", msg.type);
|
||||
http_send_error (hc, sc);
|
||||
session_transport_closing_notify (&hc->connection);
|
||||
http_disconnect_transport (hc);
|
||||
return HTTP_SM_STOP;
|
||||
}
|
||||
|
||||
static void
|
||||
http_app_enqueue (http_conn_t *hc, session_t *as)
|
||||
{
|
||||
app_worker_t *app_wrk;
|
||||
u32 dlen, max_enq, n_enq;
|
||||
int rv;
|
||||
|
||||
dlen = vec_len (hc->rx_buf) - hc->rx_buf_offset;
|
||||
if (!dlen)
|
||||
return;
|
||||
|
||||
max_enq = svm_fifo_max_enqueue (as->rx_fifo);
|
||||
n_enq = clib_min (max_enq, dlen);
|
||||
rv = svm_fifo_enqueue (as->rx_fifo, n_enq, &hc->rx_buf[hc->rx_buf_offset]);
|
||||
if (rv < 0)
|
||||
return;
|
||||
|
||||
hc->rx_buf_offset += rv;
|
||||
if (hc->rx_buf_offset >= vec_len (hc->rx_buf))
|
||||
{
|
||||
vec_reset_length (hc->rx_buf);
|
||||
hc->rx_buf_offset = 0;
|
||||
}
|
||||
|
||||
app_wrk = app_worker_get_if_valid (as->app_wrk_index);
|
||||
ASSERT (app_wrk);
|
||||
app_worker_rx_notify (app_wrk, as);
|
||||
return HTTP_SM_ERROR;
|
||||
}
|
||||
|
||||
static http_sm_result_t
|
||||
http_state_client_io_more_data (http_conn_t *hc, transport_send_params_t *sp)
|
||||
{
|
||||
session_t *as, *ts;
|
||||
u32 max_deq;
|
||||
int n_read;
|
||||
app_worker_t *app_wrk;
|
||||
svm_fifo_seg_t _seg, *seg = &_seg;
|
||||
u32 max_len, max_deq, max_enq, n_segs = 1;
|
||||
int rv, len;
|
||||
|
||||
as = session_get_from_handle (hc->h_pa_session_handle);
|
||||
ts = session_get_from_handle (hc->h_tc_session_handle);
|
||||
|
||||
http_app_enqueue (hc, as);
|
||||
|
||||
if (hc->to_recv == 0)
|
||||
max_deq = svm_fifo_max_dequeue (ts->rx_fifo);
|
||||
if (max_deq == 0)
|
||||
{
|
||||
http_state_change (hc, HTTP_STATE_WAIT_CLIENT_METHOD);
|
||||
HTTP_DBG (1, "no data to deq");
|
||||
return HTTP_SM_STOP;
|
||||
}
|
||||
|
||||
max_deq = svm_fifo_max_dequeue (ts->rx_fifo);
|
||||
if (max_deq > 0)
|
||||
max_enq = svm_fifo_max_enqueue (as->rx_fifo);
|
||||
if (max_enq == 0)
|
||||
{
|
||||
vec_validate (hc->rx_buf, max_deq - 1);
|
||||
n_read = svm_fifo_dequeue (ts->rx_fifo, max_deq, hc->rx_buf);
|
||||
ASSERT (n_read == max_deq);
|
||||
|
||||
if (svm_fifo_is_empty (ts->rx_fifo))
|
||||
svm_fifo_unset_event (ts->rx_fifo);
|
||||
|
||||
hc->to_recv -= n_read;
|
||||
vec_set_len (hc->rx_buf, n_read);
|
||||
HTTP_DBG (1, "app's rx fifo full");
|
||||
svm_fifo_add_want_deq_ntf (as->rx_fifo, SVM_FIFO_WANT_DEQ_NOTIF);
|
||||
return HTTP_SM_STOP;
|
||||
}
|
||||
|
||||
if (hc->rx_buf_offset < vec_len (hc->rx_buf) ||
|
||||
svm_fifo_max_dequeue_cons (ts->rx_fifo))
|
||||
max_len = clib_min (max_enq, max_deq);
|
||||
len = svm_fifo_segments (ts->rx_fifo, 0, seg, &n_segs, max_len);
|
||||
if (len < 0)
|
||||
{
|
||||
session_enqueue_notify (ts);
|
||||
HTTP_DBG (1, "svm_fifo_segments() len %d", len);
|
||||
return HTTP_SM_STOP;
|
||||
}
|
||||
return HTTP_SM_CONTINUE;
|
||||
|
||||
rv = svm_fifo_enqueue_segments (as->rx_fifo, seg, 1, 0 /* allow partial */);
|
||||
if (rv < 0)
|
||||
{
|
||||
clib_warning ("data enqueue failed, rv: %d", rv);
|
||||
return HTTP_SM_ERROR;
|
||||
}
|
||||
|
||||
svm_fifo_dequeue_drop (ts->rx_fifo, rv);
|
||||
if (rv > hc->to_recv)
|
||||
{
|
||||
clib_warning ("http protocol error: received more data than expected");
|
||||
session_transport_closing_notify (&hc->connection);
|
||||
http_disconnect_transport (hc);
|
||||
http_state_change (hc, HTTP_STATE_WAIT_APP_METHOD);
|
||||
return HTTP_SM_ERROR;
|
||||
}
|
||||
hc->to_recv -= rv;
|
||||
HTTP_DBG (1, "drained %d from ts; remains %d", rv, hc->to_recv);
|
||||
|
||||
app_wrk = app_worker_get_if_valid (as->app_wrk_index);
|
||||
if (app_wrk)
|
||||
app_worker_rx_notify (app_wrk, as);
|
||||
|
||||
if (svm_fifo_max_dequeue_cons (ts->rx_fifo))
|
||||
session_enqueue_notify (ts);
|
||||
|
||||
return HTTP_SM_STOP;
|
||||
}
|
||||
|
||||
static http_sm_result_t
|
||||
@ -983,6 +970,7 @@ static void
|
||||
http_req_run_state_machine (http_conn_t *hc, transport_send_params_t *sp)
|
||||
{
|
||||
http_sm_result_t res;
|
||||
|
||||
do
|
||||
{
|
||||
res = state_funcs[hc->http_state](hc, sp);
|
||||
@ -1010,6 +998,12 @@ http_ts_rx_callback (session_t *ts)
|
||||
return -1;
|
||||
}
|
||||
|
||||
if (hc->state == HTTP_CONN_STATE_CLOSED)
|
||||
{
|
||||
svm_fifo_dequeue_drop_all (ts->tx_fifo);
|
||||
return 0;
|
||||
}
|
||||
|
||||
http_req_run_state_machine (hc, 0);
|
||||
|
||||
if (hc->state == HTTP_CONN_STATE_TRANSPORT_CLOSED)
|
||||
|
Reference in New Issue
Block a user