On Wed, Jun 05, 2024 at 05:21:22PM +0200, Laurent
Vivier wrote:
This commit isolates the internal data structure
management used for storing
data (e.g., tcp4_l2_flags_iov[], tcp6_l2_flags_iov[], tcp4_flags_ip[],
tcp4_flags[], ...) from the tcp_send_flag() function. The extracted
functionality is relocated to a new function named tcp_fill_flag_header().
tcp_fill_flag_header() is now a generic function that accepts parameters such
as struct tcphdr and a data pointer. tcp_send_flag() utilizes this parameter to
pass memory pointers from tcp4_l2_flags_iov[] and tcp6_l2_flags_iov[].
This separation sets the stage for utilizing tcp_fill_flag_header() to
set the memory provided by the guest via vhost-user in future developments.
Thanks for the commit message, it makes this much clearer.
I have a number of comments below, but they're basically all cosmetic.
Signed-off-by: Laurent Vivier
<lvivier(a)redhat.com>
---
tcp.c | 63 ++++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/tcp.c b/tcp.c
index 06acb41e4d90..68d4afa05a36 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1549,24 +1549,25 @@ static void tcp_update_seqack_from_tap(const struct ctx *c,
}
/**
- * tcp_send_flag() - Send segment with flags to tap (no payload)
+ * tcp_fill_flag_header() - Prepare header for flags-only segment (no payload)
I don't love the name tcp_fill_flag_header(), although it's not
terrible. Maybe tcp_prepare_flags() would be better?
* @c: Execution context
* @conn: Connection pointer
* @flags: TCP flags: if not set, send segment only if ACK is due
+ * @th: TCP header to update
+ * @data: buffer to store TCP option
+ * @optlen: size of the TCP option buffer
Worth noting this is an output parameter here...
*
- * Return: negative error code on connection reset, 0 otherwise
+ * Return: < 0 error code on connection reset,
+ * 0 if there is no flag to send
+ * 1 otherwise
.. or, since optlen will always be positive on success cases, you
could just return it.