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

On Thu, Jul 23, 2026, at 10:23 AM, Sabrina Dubroca wrote:
> 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 <[email protected]>
>> >> ---
>> >>   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?

It sounds like a change in presentation strategy might be
needed: I have the other patches you suggested before but
was planning on submitting them after this series goes in.

But it could be sensible to post those first, along with
1/6 of this series, destined for "net". Thoughts?


-- 
Chuck Lever