Re: [PATCH net-next v2 1/6] net/tls: Bound consecutive no-data records in tls_sw_read_sock()
From: "Chuck Lever" <cel@kernel.org>
Date: 2026-07-23 14:30:22
Also in:
linux-kselftest, linux-nfs
On Thu, Jul 23, 2026, at 10:23 AM, Sabrina Dubroca wrote:
2026-07-23, 09:24:47 -0400, Chuck Lever wrote:quoted
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_sockWould 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
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
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?
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