Re: [PATCH net-next v2 1/6] net/tls: Bound consecutive no-data records in tls_sw_read_sock()

[Sabrina Dubroca <sd@queasysnail.net>]

2026-07-23, 09:24:47 -0400, Chuck Lever wrote:
> 
> 
> On Thu, Jul 23, 2026, at 3:11 AM, Hannes Reinecke wrote:
> > On 7/20/26 4:27 PM, Chuck Lever wrote:
> >> A record that delivers no payload -- an empty TLS 1.3 data record
> >> today, a control record once read_sock_rectype() lands -- leaves
> >> tls_sw_read_sock() in its loop without advancing the caller's read
> >> descriptor. A peer that streams such records keeps the receive loop
> >> running, and the socket lock held, for as long as the records
> >> arrive.
> >> 
> >> Cap the number of consecutive no-data records consumed per call. The
> >> count resets on any record that delivers bytes, so a normal stream
> >> is unaffected; a peer supplying only empty records is bounded to
> >> TLS_RX_NODATA_LIMIT iterations before the call returns 0. read_sock

Would a time-based limit be "better" than a packet count? For example
that's what net_rx_action() (net/core/dev.c) does.


> >> consumers treat that as "no progress, re-poll" rather than EOF, so
> >> the connection stays up and makes progress once real data arrives.
> >> 
> >> Only tls_sw_read_sock() needs this cap. Its consumers drive the receive
> >> loop from kernel context -- a work item or service thread holding the
> >> socket lock across the whole call with no return to userspace -- so an
> >> unbounded empty-record stream keeps that context and the lock pinned
> >> for as long as the flood lasts. The cap supplies the return boundary
> >> that a system call would otherwise provide. tls_sw_splice_read()
> >> and tls_sw_recvmsg() already have one: they run in the calling task's
> >> context, reschedule while draining the socket backlog (cond_resched()
> >> in __release_sock()),

tls_sw_read_sock() would also do that via tls_rx_rec_wait(), no?

> >> and drop the socket lock when the call returns. A
> >> flood there costs the caller only its own scheduler time, so the cap
> >> would add nothing.
> >> 
> >> Signed-off-by: Chuck Lever <cel@kernel.org>
> >> ---
> >>   net/tls/tls_sw.c | 13 +++++++++++++
> >>   1 file changed, 13 insertions(+)
> >> 
> > This is technically a fix, so it might be worthwhile sending it
> > on its own.
> 
> My impression is that the issue this patch addresses is not
> reachable until the subsequent patches in this series have
> been applied. Thus I positioned it as a pre-requisite patch
> in this series, and not as part of the earlier "fixes" series.

It looks to me like a follow-up for 3be28e2c9cd0 ("net/tls: Consume
empty data records in tls_sw_read_sock()"). Control records only
become a problem after the rest of the series, but TLS 1.3 empty data
records would get there with the current code, no?

> But light taps with a clue bat are welcome, as always.
> 
> 
> -- 
> Chuck Lever

-- 
Sabrina