Netdev List
 help / color / mirror / Atom feed
* [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS
@ 2026-10-16  2:44 Rishikesh Jethwani
  2026-10-16  2:44 ` [PATCH net v2 1/2] kcm: defer receive parsing when the lower socket's read_sock may sleep Rishikesh Jethwani
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Rishikesh Jethwani @ 2026-10-16  2:44 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Rishikesh Jethwani

When a TCP socket with software kTLS RX is attached to a KCM mux,
psock_data_ready() calls strp_data_ready() from softirq, which calls the
socket's read_sock(). For kTLS that is tls_sw_read_sock(), which can
sleep, and CONFIG_DEBUG_ATOMIC_SLEEP reports it on every run of a
loopback reproducer.

Patch 1 is the fix: KCM parses inline only when read_sock is
tcp_read_sock() and otherwise defers to the strparser work, which runs
under lock_sock(). It applies and builds on its own, including on
stable kernels.

Patch 2 addresses review feedback on v1 and is needed by patch 1.
kstrp, the workqueue that deferred work runs on, is an ordered
workqueue shared by every strparser user (KCM, sockmap, espintcp,
ovpn). With patch 1 alone, a KCM psock decrypting kTLS records in
software on kstrp would delay parsing and message timeouts for every
other strparser socket on the system. Patch 2 makes kstrp per-CPU. It
has no Fixes tag: the ordered workqueue is as old as strparser, and it
only becomes a real problem for KCM over kTLS with patch 1.

Other strparser users are not affected by the bug fixed in patch 1:
espintcp is itself a TCP ULP and cannot be combined with kTLS, ovpn
installs its own ops built from inet_stream_ops, and sockmap reads
through tcp_bpf_strp_read_sock() and is mutually exclusive with kTLS
RX (do_tls_setsockopt_conf() rejects sockets with a psock).

Testing: with patch 1, 5/5 runs of the reproducer produce no splat and
full datagram delivery; without it every run hits "BUG: sleeping
function called from invalid context" in strp_data_ready ->
tls_sw_read_sock. Multi-record messages (>16 KiB each) also deliver
completely with kTLS RX installed before the KCM attach and the whole
stream queued before the reader starts. With patch 2, test_sockmap and the ovpn TCP selftests
pass, strparser work runs concurrently across sockets, and dmesg shows
no warnings.

v2:
 - add patch 2 (kstrp per-CPU), per review
 - read sock->ops with READ_ONCE(), pairing with update_sk_prot()

v1: https://lore.kernel.org/netdev/20261013033920.2069285-1-rjethwani@purestorage.com/

Rishikesh Jethwani (2):
  kcm: defer receive parsing when the lower socket's read_sock may sleep
  strparser: don't serialize all parser work on one ordered workqueue

 net/kcm/kcmsock.c         | 33 +++++++++++++++++++++++++++++++--
 net/strparser/strparser.c |  2 +-
 2 files changed, 32 insertions(+), 3 deletions(-)

-- 
2.50.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net v2 1/2] kcm: defer receive parsing when the lower socket's read_sock may sleep
  2026-10-16  2:44 [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS Rishikesh Jethwani
@ 2026-10-16  2:44 ` Rishikesh Jethwani
  2026-10-20  3:13   ` netdev-bot+sashiko
  2026-10-16  2:44 ` [PATCH net v2 2/2] strparser: don't serialize all parser work on one ordered workqueue Rishikesh Jethwani
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Rishikesh Jethwani @ 2026-10-16  2:44 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Rishikesh Jethwani

psock_data_ready() is the lower socket's data_ready callback and can
run in softirq context, under read_lock_bh(&sk->sk_callback_lock). It calls
strp_data_ready(), which, when the socket is not owned by a user,
invokes sock->ops->read_sock() inline to pull records out of the
receive queue. For a plain TCP socket that is tcp_read_sock(), which
does not block.

A ULP may install a different read_sock. kTLS installs
tls_sw_read_sock(), which can sleep: it waits for an asynchronous
AEAD to complete via crypto_wait_req(), flushes the socket backlog
through __sk_flush_backlog(), allocates with sk->sk_allocation, and,
on reader contention in tls_rx_reader_acquire(), enters
sk_wait_event()'s socket release/reacquire path despite the zero
timeout. It passes nonblock, so it does not wait for data, but these
paths can require sleepable context, which softirq and a BH read lock
do not provide. Attaching a kTLS socket to a KCM multiplexor can
therefore sleep in atomic context.

With CONFIG_DEBUG_ATOMIC_SLEEP, a loopback TCP connection using software
kTLS RX, with the receiving socket attached to a KCM mux, reports:

  BUG: sleeping function called from invalid context at include/linux/sched/mm.h:321
  in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 189, name: ksoftirqd/34
  Call Trace:
   __might_resched
   kmem_cache_alloc_node_noprof
   __alloc_skb
   alloc_skb_with_frags
   tls_rx_one_record
   tls_sw_read_sock
   strp_read_sock
   strp_data_ready
   psock_data_ready [kcm]

Keep the inline fast path for tcp_read_sock() and hand any other
read_sock to the parser work via strp_check_rcv(). The work runs
do_strp_work() under lock_sock(), where a sleeping read_sock is fine.
sk->sk_socket should not be NULL here, since KCM holds the lower
socket's file reference while a psock is attached; if it is,
defer as well, as strp_read_sock() would not read without a socket.

The deferred path also skips strp_data_ready()'s need_bytes early
return. That check counts bytes in the TCP receive queue, which for
kTLS is ciphertext rather than parsed records, so it would not be
meaningful there. Without it, a data_ready event can queue work that
finds no complete record and returns; queue_work() does nothing if the
work is already pending.

kTLS installs tls_sw_read_sock() by replacing sk_socket->ops with
WRITE_ONCE() in update_sk_prot() when TLS_RX is configured. That happens
under lock_sock() only, and can happen after the socket is attached to
KCM, so read it with READ_ONCE().

The deferred work runs on kstrp, which is currently an ordered workqueue
shared by all strparser users. The next patch makes it per-CPU, so a
psock whose read_sock does software decryption does not hold up parsing
for other strparser users.

Tested with the reproducer above: 5/5 runs produce no splat and full
datagram delivery; without this change every run hits "BUG: sleeping
function called from invalid context" in strp_data_ready ->
tls_sw_read_sock.

Multi-record messages (>16 KiB each) also deliver completely when kTLS
RX is installed before the KCM attach and the entire stream is queued
before the reader starts: the parser correctly pauses on KCM flow
control (psock RxPause/RdyRx) and resumes on read, with no strparser
need_bytes stall.

Fixes: 662fbcec32f4 ("net/tls: implement ->read_sock()")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 net/kcm/kcmsock.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 71af69d442f2..ec7aaba17fe6 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -30,6 +30,7 @@
 #include <net/kcm.h>
 #include <net/netns/generic.h>
 #include <net/sock.h>
+#include <net/tcp.h>
 #include <uapi/linux/kcm.h>
 #include <trace/events/sock.h>
 
@@ -348,6 +349,30 @@ static void unreserve_rx_kcm(struct kcm_psock *psock,
 	spin_unlock_bh(&mux->rx_lock);
 }
 
+/* strp_data_ready() calls the lower socket's read_sock() inline, from
+ * softirq and under sk_callback_lock. That is only safe for TCP's own
+ * tcp_read_sock(); anything else goes to the parser work, which runs
+ * under lock_sock(). Called with sk_callback_lock held.
+ */
+static bool psock_can_parse_inline(struct sock *sk)
+{
+	struct socket *sock = sk->sk_socket;
+	const struct proto_ops *ops;
+
+	/* KCM holds the lower socket's file reference while a psock is
+	 * attached, so sk_socket should not be NULL here. Defer if it is,
+	 * as strp_read_sock() would not read without a socket anyway.
+	 */
+	if (!sock)
+		return false;
+
+	/* A ULP's read_sock, such as kTLS's tls_sw_read_sock(), may sleep.
+	 * READ_ONCE() pairs with the WRITE_ONCE() in kTLS's update_sk_prot().
+	 */
+	ops = READ_ONCE(sock->ops);
+	return ops->read_sock == tcp_read_sock;
+}
+
 /* Lower sock lock held */
 static void psock_data_ready(struct sock *sk)
 {
@@ -358,8 +383,12 @@ static void psock_data_ready(struct sock *sk)
 	read_lock_bh(&sk->sk_callback_lock);
 
 	psock = (struct kcm_psock *)sk->sk_user_data;
-	if (likely(psock))
-		strp_data_ready(&psock->strp);
+	if (likely(psock)) {
+		if (psock_can_parse_inline(sk))
+			strp_data_ready(&psock->strp);
+		else
+			strp_check_rcv(&psock->strp);
+	}
 
 	read_unlock_bh(&sk->sk_callback_lock);
 }
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net v2 2/2] strparser: don't serialize all parser work on one ordered workqueue
  2026-10-16  2:44 [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS Rishikesh Jethwani
  2026-10-16  2:44 ` [PATCH net v2 1/2] kcm: defer receive parsing when the lower socket's read_sock may sleep Rishikesh Jethwani
@ 2026-10-16  2:44 ` Rishikesh Jethwani
  2026-10-20  3:13   ` netdev-bot+sashiko
  2026-10-16  2:54 ` [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS netdev-bot+sinfo
  2026-10-16 10:13 ` Jiayuan Chen
  3 siblings, 1 reply; 8+ messages in thread
From: Rishikesh Jethwani @ 2026-10-16  2:44 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Rishikesh Jethwani

strp_wq has been created with create_singlethread_workqueue() since
strparser was introduced, which makes it an ordered workqueue: at most
one strparser work item runs at a time, system-wide. Every strparser
user shares it: KCM, sockmap, espintcp and ovpn's TCP transport, and
every instance's msg_timer_work.

strp_work() takes cb.lock before reading, and every in-tree user leaves
cb.lock at its default, lock_sock(). The single kstrp worker therefore
already stalls for as long as any process holds the socket lock of any
strparser-attached socket, delaying parsing and message timeouts for
every other socket using strparser. The same applies to work that is
slow for other reasons, such as a lower socket's read_sock() doing
software decryption, which is what KCM over kTLS does now that the
previous patch moves its parsing to kstrp.

Nothing relies on the ordering:

 - The workqueue core never runs one work item on two workers at
   once, so a given strparser's work stays serialized with itself.
 - strp_work() and strp_msg_timeout() both run under cb.lock, so they
   are serialized against each other per socket by the socket lock,
   and strp_abort_strp() is idempotent through strp->stopped.
 - strp_done() cancels both with the _sync variants, which work on any
   workqueue.
 - Different strparsers were never ordered against each other: the
   common strp_data_ready() path runs inline in softirq on whichever
   CPU received the data, bypassing the workqueue entirely.

Allocate kstrp as a per-CPU, WQ_MEM_RECLAIM workqueue instead. kTLS's
tls-strp workqueue has the same flags, via create_workqueue(), but
caps max_active at 1 per CPU; kstrp uses the default limit (1024 per
CPU), so a work item blocked on a socket lock does not hold up others
queued on the same CPU. A per-CPU workqueue keeps the work local to
the CPU that received the data, like tls-strp; work that stays
CPU-bound for long is excluded from concurrency management by the
workqueue core's CPU-intensive detection.

This lets strparser work for different sockets run concurrently for
every user of strparser, not only KCM. Without it, the previous patch
would let one KCM psock doing software kTLS decryption on kstrp delay
parsing and message timeouts for every other strparser socket.

Tested with test_sockmap and the ovpn TCP selftests; strparser work now
runs concurrently across sockets with no dmesg warnings.

Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 net/strparser/strparser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index d5b17d099a0e..32fe96ba10d6 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -542,7 +542,7 @@ static int __init strp_dev_init(void)
 	BUILD_BUG_ON(sizeof(struct sk_skb_cb) >
 		     sizeof_field(struct sk_buff, cb));
 
-	strp_wq = create_singlethread_workqueue("kstrp");
+	strp_wq = alloc_workqueue("kstrp", WQ_PERCPU | WQ_MEM_RECLAIM, 0);
 	if (unlikely(!strp_wq))
 		return -ENOMEM;
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS
  2026-10-16  2:44 [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS Rishikesh Jethwani
  2026-10-16  2:44 ` [PATCH net v2 1/2] kcm: defer receive parsing when the lower socket's read_sock may sleep Rishikesh Jethwani
  2026-10-16  2:44 ` [PATCH net v2 2/2] strparser: don't serialize all parser work on one ordered workqueue Rishikesh Jethwani
@ 2026-10-16  2:54 ` netdev-bot+sinfo
  2026-10-16  4:29   ` Rishikesh Jethwani
  2026-10-16 10:13 ` Jiayuan Chen
  3 siblings, 1 reply; 8+ messages in thread
From: netdev-bot+sinfo @ 2026-10-16  2:54 UTC (permalink / raw)
  To: Rishikesh Jethwani
  Cc: netdev, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS
  2026-10-16  2:54 ` [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS netdev-bot+sinfo
@ 2026-10-16  4:29   ` Rishikesh Jethwani
  0 siblings, 0 replies; 8+ messages in thread
From: Rishikesh Jethwani @ 2026-10-16  4:29 UTC (permalink / raw)
  To: netdev-bot+sinfo
  Cc: netdev, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

On Thu, Oct 15, 2026 at 7:54 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

The issue fixed in patch 1 was found by manual code inspection: reading
the kTLS ->read_sock() path, I noticed that tls_sw_read_sock() can sleep
while KCM's psock_data_ready() reaches it from softirq through
strp_data_ready(). I then confirmed it with the CONFIG_DEBUG_ATOMIC_SLEEP
loopback reproducer described in the commit message, which hits the
splat on every run without the patch.

Patch 2 comes from review feedback on v1, which pointed out that kstrp
is an ordered workqueue shared by every strparser user.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS
  2026-10-16  2:44 [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS Rishikesh Jethwani
                   ` (2 preceding siblings ...)
  2026-10-16  2:54 ` [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS netdev-bot+sinfo
@ 2026-10-16 10:13 ` Jiayuan Chen
  3 siblings, 0 replies; 8+ messages in thread
From: Jiayuan Chen @ 2026-10-16 10:13 UTC (permalink / raw)
  To: Rishikesh Jethwani, netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman


On 10/16/26 10:44 AM, Rishikesh Jethwani wrote:
> When a TCP socket with software kTLS RX is attached to a KCM mux,


Can we just reject such combo ?


> psock_data_ready() calls strp_data_ready() from softirq, which calls the
> socket's read_sock(). For kTLS that is tls_sw_read_sock(), which can
> sleep, and CONFIG_DEBUG_ATOMIC_SLEEP reports it on every run of a
> loopback reproducer.
>
> Patch 1 is the fix: KCM parses inline only when read_sock is
> tcp_read_sock() and otherwise defers to the strparser work, which runs
> under lock_sock(). It applies and builds on its own, including on
> stable kernels.
>
> Patch 2 addresses review feedback on v1 and is needed by patch 1.
> kstrp, the workqueue that deferred work runs on, is an ordered
> workqueue shared by every strparser user (KCM, sockmap, espintcp,
> ovpn). With patch 1 alone, a KCM psock decrypting kTLS records in
> software on kstrp would delay parsing and message timeouts for every
> other strparser socket on the system. Patch 2 makes kstrp per-CPU. It
> has no Fixes tag: the ordered workqueue is as old as strparser, and it
> only becomes a real problem for KCM over kTLS with patch 1.
>
> Other strparser users are not affected by the bug fixed in patch 1:
> espintcp is itself a TCP ULP and cannot be combined with kTLS, ovpn
> installs its own ops built from inet_stream_ops, and sockmap reads
> through tcp_bpf_strp_read_sock() and is mutually exclusive with kTLS
> RX (do_tls_setsockopt_conf() rejects sockets with a psock).
>
> Testing: with patch 1, 5/5 runs of the reproducer produce no splat and
> full datagram delivery; without it every run hits "BUG: sleeping
> function called from invalid context" in strp_data_ready ->
> tls_sw_read_sock. Multi-record messages (>16 KiB each) also deliver
> completely with kTLS RX installed before the KCM attach and the whole
> stream queued before the reader starts. With patch 2, test_sockmap and the ovpn TCP selftests
> pass, strparser work runs concurrently across sockets, and dmesg shows
> no warnings.
>
> v2:
>   - add patch 2 (kstrp per-CPU), per review
>   - read sock->ops with READ_ONCE(), pairing with update_sk_prot()
>
> v1: https://lore.kernel.org/netdev/20261013033920.2069285-1-rjethwani@purestorage.com/
>
> Rishikesh Jethwani (2):
>    kcm: defer receive parsing when the lower socket's read_sock may sleep
>    strparser: don't serialize all parser work on one ordered workqueue
>
>   net/kcm/kcmsock.c         | 33 +++++++++++++++++++++++++++++++--
>   net/strparser/strparser.c |  2 +-
>   2 files changed, 32 insertions(+), 3 deletions(-)
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net v2 1/2] kcm: defer receive parsing when the lower socket's read_sock may sleep
  2026-10-16  2:44 ` [PATCH net v2 1/2] kcm: defer receive parsing when the lower socket's read_sock may sleep Rishikesh Jethwani
@ 2026-10-20  3:13   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-20  3:13 UTC (permalink / raw)
  To: rjethwani; +Cc: netdev, davem, edumazet, kuba, pabeni, horms

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] RX can stall indefinitely when kTLS RX is installed before the
  TCP socket is attached to KCM.

--

kcm: defer receive parsing when the lower socket's read_sock may sleep

This patch changes psock_data_ready() so that it only calls
strp_data_ready() inline when the lower socket's read_sock is
tcp_read_sock(). Any other read_sock, such as kTLS's tls_sw_read_sock(),
is handed to the strparser work through strp_check_rcv(), so it runs
under lock_sock() and not in softirq context.

> diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
> index 71af69d442f21..ec7aaba17fe60 100644
> --- a/net/kcm/kcmsock.c
> +++ b/net/kcm/kcmsock.c

[ ... ]

> @@ -358,8 +383,12 @@ static void psock_data_ready(struct sock *sk)
>  	read_lock_bh(&sk->sk_callback_lock);
>  
>  	psock = (struct kcm_psock *)sk->sk_user_data;
> -	if (likely(psock))
> -		strp_data_ready(&psock->strp);
> +	if (likely(psock)) {
> +		if (psock_can_parse_inline(sk))
> +			strp_data_ready(&psock->strp);
> +		else
> +			strp_check_rcv(&psock->strp);
> +	}

[Severity: Medium]
Can receive stall on a kTLS psock when TLS_RX is configured before
kcm_attach()? The commit message tests this ordering and says there is
"no strparser need_bytes stall".

After this change, every read on a kTLS psock goes through do_strp_work().
That work is only queued again when psock_data_ready() runs, which means
when new TCP data arrives. do_strp_work() itself only requeues on -ENOMEM:

net/strparser/strparser.c:do_strp_work() {
    ...
	if (strp_read_sock(strp) == -ENOMEM)
		queue_work(strp_wq, &strp->work);
    ...
}

When a KCM message spans multiple TLS records, tls_sw_read_sock() calls
tls_rx_rec_done() before read_actor(). The ciphertext of the current
record has therefore already been consumed from the TCP queue:

net/tls/tls_sw.c:tls_sw_read_sock() {
    ...
			tls_rx_rec_done(ctx);
		}
    ...
		used = read_actor(desc, skb, rxm->offset, rxm->full_len);
    ...
}

kTLS does not override peek_len, so in __strp_recv() strp_peek_len() is
tcp_inq(). The current record is no longer counted in it:

net/strparser/strparser.c:__strp_recv() {
    ...
			if (stm->strp.full_len - stm->accum_len >
			    strp_peek_len(strp)) {
				...
				strp->need_bytes = stm->strp.full_len -
						       stm->accum_len;
				...
				desc->count = 0; /* Stop reading socket */
				break;
    ...
}

Won't this stop the read pass even when the next TLS records are already
fully queued in TCP? The commit message says the need_bytes check in
strp_data_ready() isn't meaningful for kTLS because it counts ciphertext.
This check uses the same TCP byte count, and it is still in place.

A second trigger looks possible as well. If tls_strp_read_sock() returns
-ENOMEM during tls_rx_rec_wait(), tls_strp_check_rcv() queues
tls_strp_work. The nonblocking reader returns -EAGAIN, desc.error stays 0
and kcm_read_sock_done() returns 0, so do_strp_work() does not requeue.

In both cases the next-record notification goes only through this path:

tls_rx_reader_release() or tls_strp_work()
  tls_rx_msg_maybe_announce()
    ctx->saved_data_ready(strp->sk)

tls_sw_strparser_arm() captured saved_data_ready before kcm_attach()
installed psock_data_ready:

net/tls/tls_sw.c:tls_sw_strparser_arm() {
    ...
	rx_ctx->saved_data_ready = sk->sk_data_ready;
	sk->sk_data_ready = tls_data_ready;
    ...
}

So saved_data_ready is sock_def_readable(), which never reaches the KCM
strparser. Would the last multi-record message of a burst then stay
undelivered until the peer sends more data or a FIN?

The peek check in __strp_recv() and the stale saved_data_ready both
predate this patch. With this patch, though, all progress depends on
psock_data_ready() being called again. Deferring everything to the
workqueue also means segments that arrive while the work is pending get
merged into one read pass, because queue_work() does nothing while the
work is already queued.

>  
>  	read_unlock_bh(&sk->sk_callback_lock);
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016025059.289202-1-rjethwani%40purestorage.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net v2 2/2] strparser: don't serialize all parser work on one ordered workqueue
  2026-10-16  2:44 ` [PATCH net v2 2/2] strparser: don't serialize all parser work on one ordered workqueue Rishikesh Jethwani
@ 2026-10-20  3:13   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-20  3:13 UTC (permalink / raw)
  To: rjethwani; +Cc: netdev, davem, edumazet, kuba, pabeni, horms

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Switching kstrp from an ordered workqueue to a per-CPU one
  lets strp_work() and strp_msg_timeout() for the same strparser run on…

--

strparser: don't serialize all parser work on one ordered workqueue

This patch changes the kstrp workqueue from an ordered workqueue (made
with create_singlethread_workqueue()) to a per-CPU WQ_MEM_RECLAIM
workqueue with the default max_active. The aim is to let strparser work
for different sockets run at the same time, so that every strparser
user no longer waits on a single worker.

> diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
> index d5b17d099a0e7..32fe96ba10d61 100644
> --- a/net/strparser/strparser.c
> +++ b/net/strparser/strparser.c
> @@ -542,7 +542,7 @@ static int __init strp_dev_init(void)
>  	BUILD_BUG_ON(sizeof(struct sk_skb_cb) >
>  		     sizeof_field(struct sk_buff, cb));
>  
> -	strp_wq = create_singlethread_workqueue("kstrp");
> +	strp_wq = alloc_workqueue("kstrp", WQ_PERCPU | WQ_MEM_RECLAIM, 0);

[Severity: Medium]
With this change, strp->work and strp->msg_timer_work for the same
strparser can run on two different kworkers at once. Can a message that
has already completed then be followed by a timeout abort?

The commit message says:

    Nothing relies on the ordering:
    [ ... ]
     - strp_work() and strp_msg_timeout() both run under cb.lock, so they
       are serialized against each other per socket by the socket lock,

The socket lock gives mutual exclusion. However, the cancel in
__strp_recv() when a message completes is not a sync cancel:

net/strparser/strparser.c:__strp_recv() {
    ...
		/* Hurray, we have a new message! */
		cancel_delayed_work(&strp->msg_timer_work);
		strp->skb_head = NULL;
    ...
}

strp_msg_timeout() then aborts unconditionally once it gets the lock:

net/strparser/strparser.c:strp_msg_timeout() {
    ...
	STRP_STATS_INCR(strp->stats.msg_timeouts);
	strp->cb.lock(strp);
	strp->cb.abort_parser(strp, -ETIMEDOUT);
	strp->cb.unlock(strp);
}

Assume SO_RCVTIMEO is set on the lower socket, so strp_start_timer()
arms the timer. Assume also that the timer expires while strp->work is
holding lock_sock(), for example during kTLS decryption for KCM:

kworker B (strp->work)               kworker A (msg_timer_work)
do_strp_work()
  strp->cb.lock()
  read_sock() ...
                                     strp_msg_timeout()
                                       msg_timeouts++
                                       strp->cb.lock()  /* sleeps */
  __strp_recv()
    cancel_delayed_work()  /* not pending, no-op */
    strp->cb.rcv_msg()
    new message started
    strp_start_timer()
      mod_delayed_work()
  strp->cb.unlock()
                                       strp->cb.abort_parser(-ETIMEDOUT)

No in-tree user overrides abort_parser, so this calls strp_abort_strp().
That function sets strp->stopped, frees the newly started skb_head, and
reports ETIMEDOUT on the lower socket. The KCM psock, sockmap or
espintcp stream is torn down right after a successful delivery.

With the old ordered workqueue, the expired timeout work stayed on the
pwq inactive list behind strp->work. The cancel_delayed_work() in
__strp_recv() removed it, because try_to_grab_pending() handles
WORK_STRUCT_INACTIVE. So on the workqueue path the message survived.

A similar but much narrower window, spinning on sk_lock.slock, already
exists on the inline softirq strp_data_ready() path. This patch widens
it to the kstrp path, which after the previous patch in the series is
the only path KCM over kTLS uses.

Should strp_msg_timeout() check under cb.lock that the message it was
timing is still in progress before it aborts? A generation counter, or
a check that skb_head and accum_len still match, would do this.

>  	if (unlikely(!strp_wq))
>  		return -ENOMEM;
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016025059.289202-1-rjethwani%40purestorage.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-20  3:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16  2:44 [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS Rishikesh Jethwani
2026-10-16  2:44 ` [PATCH net v2 1/2] kcm: defer receive parsing when the lower socket's read_sock may sleep Rishikesh Jethwani
2026-10-20  3:13   ` netdev-bot+sashiko
2026-10-16  2:44 ` [PATCH net v2 2/2] strparser: don't serialize all parser work on one ordered workqueue Rishikesh Jethwani
2026-10-20  3:13   ` netdev-bot+sashiko
2026-10-16  2:54 ` [PATCH net v2 0/2] kcm: don't sleep in softirq when the lower socket uses kTLS netdev-bot+sinfo
2026-10-16  4:29   ` Rishikesh Jethwani
2026-10-16 10:13 ` Jiayuan Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox