Re: [PATCH net-next v2 2/6] net: Introduce read_sock_rectype proto_ops for control record delivery

2026-07-30, 20:12:22 -0400, Chuck Lever wrote:
> 
> 
> On Thu, Jul 30, 2026, at 5:35 PM, Jakub Kicinski wrote:
> > On Thu, 30 Jul 2026 16:16:33 +0200 Sabrina Dubroca wrote:
> >> 2026-07-29, 16:31:42 -0700, Jakub Kicinski wrote:
> 
> >> > Off the top of my head I think a setsockopt which pre-seeds the content
> >> > type so that the read returns an errno if the queued content type is
> >> > different could be a simple fix. You'd configure that on your sockets
> >> > to DATA and once you see a EWHATEVER you'd assume that some special
> >> > record arrived and the socket has to be handed back over to the TLS
> >> > control path. This is literally the first thing that comes to mind,
> >> > IDK how ugly it will look in reality so no promises.  
> >> 
> >> But then you're back to "read_sock stopped, caller has to take some
> >> special action to handle the next bit of payload". It's not better
> >> than "read_sock, and do a recvmsg when read_sock says it's not DATA".
> 
> But the “recover the control type with a separate operation” is strictly
> better than “pass a CMSG buffer to every I/O operation just in case” ;-)

That's up to every user to decide :)

> > My bad, I replied without looking at the code.
> > We already constrain control records in the way I proposed.
> > rcvmsg (w/o cmsg) and read_sock will error out if the next
> > record is control.
> 
> Almost.
> 
> There is no API contract for ->read_sock, but the TLS read_sock
> implementation itself will return -EINVAL for two unrelated reasons:
> 
>   - net/tls/tls_sw.c:2068-2072 — entry gate: sk_psock_get(sk) returns
>     non-NULL, so the socket is under sockmap/BPF. Drop the ref and
>     refuse before even acquiring the reader.

You can ignore this one, it's just a leftover that 79511603a65b ("tls:
remove dead sockmap (psock) handling from the SW path") missed. I'll
get back to cleaning all this up once I'm not drowning in reviews
(I'm sure being on holidays next week will help with that).

>   - net/tls/tls_sw.c:2111-2115 — per-record: tlm->control !=
>     TLS_RECORD_TYPE_DATA. The record is requeued rather than consumed.
> 
> As far as I can tell, no other socket provider that implements read_sock
> will return -EINVAL. But this isn’t a documented guarantee that a
> socket consumer can depend on, currently.
> 
> What would make this just a little friendlier is having distinct errnos
> for these two conditions, and a kdoc API contract that documents them.

+1 on the API contract doc. I have no idea what read_actor does to
desc, how that relates to the returned value, and if it modifies the
skb it gets in any way. That makes it hard to figure out the correct
behavior for tls_sw_read_sock().

Maybe we should even come up with some new internal errno (like one of
the >=512 in include/linux/errno.h). And maybe this -errno return
should be desc->error instead. I really don't know how read_sock is
supposed to behave.

-- 
Sabrina