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

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 24 Apr 2024 10:48:05 +1000
David Gibson <david@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@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