summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFilip Tehlar <ftehlar@cisco.com>2023-11-27 13:28:36 +0100
committerFlorin Coras <florin.coras@gmail.com>2024-05-01 05:07:26 +0000
commitd894438f0499db83a3182453547bd3accea4776f (patch)
tree4924878a04b2b2a044cc04ebbe00baf18facfd99
parent1db11b7d8de5306555e1ce1dd398fb4a9eaf55ba (diff)
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>
Diffstat
-rw-r--r--extras/hs-test/http_test.go2
-rw-r--r--src/plugins/hs_apps/http_client_cli.c13
-rw-r--r--src/plugins/http/http.c132
3 files changed, 72 insertions, 75 deletions
diff --git a/extras/hs-test/http_test.go b/extras/hs-test/http_test.go
index 4277d432e72..acd026d484f 100644
--- a/extras/hs-test/http_test.go
+++ b/extras/hs-test/http_test.go
@@ -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!")
diff --git a/src/plugins/hs_apps/http_client_cli.c b/src/plugins/hs_apps/http_client_cli.c
index 8de9ff8fd37..085a2b69bf7 100644
--- a/src/plugins/hs_apps/http_client_cli.c
+++ b/src/plugins/hs_apps/http_client_cli.c
@@ -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,
};
diff --git a/src/plugins/http/http.c b/src/plugins/http/http.c
index 036e6929987..0fa113c8155 100644
--- a/src/plugins/http/http.c
+++ b/src/plugins/http/http.c
@@ -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);
+ 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 (svm_fifo_is_empty (ts->rx_fifo))
- svm_fifo_unset_event (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)
+ {
+ HTTP_DBG (1, "svm_fifo_segments() len %d", len);
+ return HTTP_SM_STOP;
+ }
- hc->to_recv -= n_read;
- vec_set_len (hc->rx_buf, n_read);
+ 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;
}
- if (hc->rx_buf_offset < vec_len (hc->rx_buf) ||
- svm_fifo_max_dequeue_cons (ts->rx_fifo))
+ svm_fifo_dequeue_drop (ts->rx_fifo, rv);
+ if (rv > hc->to_recv)
{
- session_enqueue_notify (ts);
+ 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;
}
- return HTTP_SM_CONTINUE;
+ 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)