aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVratko Polak <vrpolak@cisco.com>2019-10-07 14:52:53 +0200
committerDave Barach <openvpp@barachs.net>2020-02-18 13:28:25 +0000
commit72ab26ca8f1f4d28c41b39f994ac82f60552f41a (patch)
tree806a0c23fa300c37d025b8dfd587dd5edcb1b8d7
parent05c2f5b73e67994da1d4f5b61731608ee08c379d (diff)
api: comment, simplify and fix api socket read
The function vl_socket_read_ready did contain some comments already, but as they stated, the logic has to be tricky to cover multiple cases. Comment: + Add function-level comment + Add comments to describe some of local variables + Add many comments to describe internal state at particular lines. Simplify: + Remov mbp_set as it is never needed. + Replace msg_len with msgbuf_len to save "+ sizeof (msgbuf_t)". Improve: + Early exit on EAGAIN. Fix: + "n" now only tracks input_buffer. Previously, it was entering the detection of additional messages even for unprocessed_input. + Set up msg_buffer (including appending to unprocessed_input) outside full-message-detection loop now, so it cannot be executed multiple times as before. Type: fix Ticket: VPP-1785 Change-Id: I256e34b435be06844458744a13ea37a0e86a96f9 Signed-off-by: Vratko Polak <vrpolak@cisco.com> (cherry picked from commit 6a6af6ea1a77b5818e717047b5d01251ef6d024a)
-rw-r--r--src/vlibmemory/socket_api.c128
1 files changed, 80 insertions, 48 deletions
diff --git a/src/vlibmemory/socket_api.c b/src/vlibmemory/socket_api.c
index c423c526625..edc0f28ff34 100644
--- a/src/vlibmemory/socket_api.c
+++ b/src/vlibmemory/socket_api.c
@@ -203,43 +203,68 @@ vl_socket_process_api_msg (clib_file_t * uf, vl_api_registration_t * rp,
socket_main.current_rp = 0;
}
+/*
+ * Read function for API socket.
+ *
+ * Read data from socket, invoke SOCKET_READ_EVENT
+ * for each fully read API message, return 0.
+ * Store incomplete data for next invocation to continue.
+ *
+ * On severe read error, the file is closed.
+ *
+ * As reading is single threaded,
+ * socket_main.input_buffer is used temporarily.
+ * Even its length is modified, but always restored before return.
+ *
+ * Incomplete data is copied into a vector,
+ * pointer saved in registration's unprocessed_input.
+ */
clib_error_t *
vl_socket_read_ready (clib_file_t * uf)
{
clib_file_main_t *fm = &file_main;
vlib_main_t *vm = vlib_get_main ();
vl_api_registration_t *rp;
+ /* n is the size of data read to input_buffer */
int n;
+ /* msg_buffer vector can point to input_buffer or unprocessed_input */
i8 *msg_buffer = 0;
+ /* data_for_process is a vector containing one full message, incl msgbuf_t */
u8 *data_for_process;
- u32 msg_len;
+ /* msgbuf_len is the size of one message, including sizeof (msgbuf_t) */
+ u32 msgbuf_len;
u32 save_input_buffer_length = vec_len (socket_main.input_buffer);
vl_socket_args_for_process_t *a;
- msgbuf_t *mbp;
- int mbp_set = 0;
rp = pool_elt_at_index (socket_main.registration_pool, uf->private_data);
+ /* Ignore unprocessed_input for now, n describes input_buffer for now. */
n = read (uf->file_descriptor, socket_main.input_buffer,
vec_len (socket_main.input_buffer));
- if (n <= 0 && errno != EAGAIN)
+ if (n <= 0)
{
- clib_file_del (fm, uf);
-
- if (!pool_is_free (socket_main.registration_pool, rp))
- {
- u32 index = rp - socket_main.registration_pool;
- vl_socket_free_registration_index (index);
- }
- else
+ if (errno != EAGAIN)
{
- clib_warning ("client index %d already free?",
- rp->vl_api_registration_pool_index);
+ /* Severe error, close the file. */
+ clib_file_del (fm, uf);
+
+ if (!pool_is_free (socket_main.registration_pool, rp))
+ {
+ u32 index = rp - socket_main.registration_pool;
+ vl_socket_free_registration_index (index);
+ }
+ else
+ {
+ clib_warning ("client index %d already free?",
+ rp->vl_api_registration_pool_index);
+ }
}
+ /* EAGAIN means we do not close the file, but no data to process anyway. */
return 0;
}
+ /* Fake smaller length teporarily, so input_buffer can be used as msg_buffer. */
_vec_len (socket_main.input_buffer) = n;
/*
@@ -248,37 +273,38 @@ vl_socket_read_ready (clib_file_t * uf)
* boundaries. In the case of a long message (>4K bytes)
* we have to do (at least) 2 reads, etc.
*/
+ /* Determine msg_buffer. */
+ if (vec_len (rp->unprocessed_input))
+ {
+ vec_append (rp->unprocessed_input, socket_main.input_buffer);
+ msg_buffer = rp->unprocessed_input;
+ }
+ else
+ {
+ msg_buffer = socket_main.input_buffer;
+ }
+ /* Loop to process any full messages. */
+ ASSERT (vec_len (msg_buffer) > 0);
do
{
- if (vec_len (rp->unprocessed_input))
- {
- vec_append (rp->unprocessed_input, socket_main.input_buffer);
- msg_buffer = rp->unprocessed_input;
- }
- else
- {
- msg_buffer = socket_main.input_buffer;
- mbp_set = 0;
- }
-
- if (mbp_set == 0)
- {
- /* Any chance that we have a complete message? */
- if (vec_len (msg_buffer) <= sizeof (msgbuf_t))
- goto save_and_split;
-
- mbp = (msgbuf_t *) msg_buffer;
- msg_len = ntohl (mbp->data_len);
- mbp_set = 1;
- }
-
- /* We don't have the entire message yet. */
- if (mbp_set == 0
- || (msg_len + sizeof (msgbuf_t)) > vec_len (msg_buffer))
+ /* Here, we are not sure how big a chunk of message we have left. */
+ /* Do we at least know how big the full message will be? */
+ if (vec_len (msg_buffer) <= sizeof (msgbuf_t))
+ /* No, so fragment is not a full message. */
+ goto save_and_split;
+
+ /* Now we know how big the full message will be. */
+ msgbuf_len =
+ ntohl (((msgbuf_t *) msg_buffer)->data_len) + sizeof (msgbuf_t);
+
+ /* But do we have a full message? */
+ if (msgbuf_len > vec_len (msg_buffer))
{
save_and_split:
- /* if we were using the input buffer save the fragment */
+ /* We don't have the entire message yet. */
+ /* If msg_buffer is unprocessed_input, nothing needs to be done. */
if (msg_buffer == socket_main.input_buffer)
+ /* But if we were using the input buffer, save the fragment. */
{
ASSERT (vec_len (rp->unprocessed_input) == 0);
vec_validate (rp->unprocessed_input, vec_len (msg_buffer) - 1);
@@ -286,12 +312,19 @@ vl_socket_read_ready (clib_file_t * uf)
vec_len (msg_buffer));
_vec_len (rp->unprocessed_input) = vec_len (msg_buffer);
}
+ /* No more full messages, restore original input_buffer length. */
_vec_len (socket_main.input_buffer) = save_input_buffer_length;
return 0;
}
+ /*
+ * We have at least one full message.
+ * But msg_buffer can contain more data, so copy one message data
+ * so we can overwrite its length to what single message has.
+ */
data_for_process = (u8 *) vec_dup (msg_buffer);
- _vec_len (data_for_process) = (msg_len + sizeof (msgbuf_t));
+ _vec_len (data_for_process) = msgbuf_len;
+ /* Everything is ready to signal the SOCKET_READ_EVENT. */
pool_get (socket_main.process_args, a);
a->clib_file = uf;
a->regp = rp;
@@ -300,18 +333,17 @@ vl_socket_read_ready (clib_file_t * uf)
vlib_process_signal_event (vm, vl_api_clnt_node.index,
SOCKET_READ_EVENT,
a - socket_main.process_args);
- if (n > (msg_len + sizeof (*mbp)))
- vec_delete (msg_buffer, msg_len + sizeof (*mbp), 0);
+ if (vec_len (msg_buffer) > msgbuf_len)
+ /* There are some fragments left. Shrink the msg_buffer to simplify logic. */
+ vec_delete (msg_buffer, msgbuf_len, 0);
else
+ /* We are done with msg_buffer. */
_vec_len (msg_buffer) = 0;
- n -= msg_len + sizeof (msgbuf_t);
- msg_len = 0;
- mbp_set = 0;
}
- while (n > 0);
+ while (vec_len (msg_buffer) > 0);
+ /* Restore input_buffer, it could have been msg_buffer. */
_vec_len (socket_main.input_buffer) = save_input_buffer_length;
-
return 0;
}