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

2026-07-23, 09:24:47 -0400, Chuck Lever wrote:

On Thu, Jul 23, 2026, at 3:11 AM, Hannes Reinecke wrote:

quoted

On 7/20/26 4:27 PM, Chuck Lever wrote:

quoted

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.

quoted

quoted

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?

quoted

quoted

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