Re: [PATCH 1/2] tcp: leverage support of SO_PEEK_OFF socket option when available

On Wed, 24 Apr 2024 10:48:05 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  On Tue, Apr 23, 2024 at 07:50:10PM +0200, Stefano
Brivio wrote:
  On Sat, 20 Apr 2024 15:19:19 -0400
 Jon Maloy <jmaloy(a)redhat.com> wrote:   
 [snip]
 
 
+	set_peek_offset(s, 0);   
 
 Do we really need to initialise it to zero on a new connection? Extra
 system calls on this path matter for latency of connection
 establishment.   
 
 Sort of, yes: we need to enable the SO_PEEK_OFF behaviour by setting
 it to 0, rather than the default -1. 
By the way of which, this is not documented at this point -- a man page
patch (linux-man and linux-api lists) would be nice.

...
  We could lazily enable it, but
 we'd need either to a) do it later in the handshake (maybe when we set
 ESTABLISHED), but we'd need to be careful it is always set before the
 first MSG_PEEK 
I was actually thinking that we could set it only as we receive data
(not every connection will receive data), and keep this out of the
handshake (which we want to keep "faster", I think).

And setting it as we mark a connection as ESTABLISHED should have the
same effect on latency as setting it on a new connection -- that's not
really lazy. So, actually:

...
  or b) keep track of whether it's set on a
per-socket
 basis (this would have the advantage of robustness if we ever
 encountered a kernel that weirdly allows it for some but not all TCP
 sockets). 
...this could be done as we receive data in tcp_data_from_sock(), with
a new flag in tcp_tap_conn::flags, to avoid adding latency to the
handshake. It also looks more robust to me, and done/checked in a
single place where we need it.

We have just three bits left there which isn't great, but if we need to
save one at a later point, we can drop this new flag easily.

-- 
Stefano