Netdev List
 help / color / mirror / Atom feed
* [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework
@ 2026-10-15 20:11 Rishikesh Jethwani
  2026-10-15 20:11 ` [PATCH net 1/2] tls: device: fix record start in the tls_device_decrypted tracepoint Rishikesh Jethwani
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Rishikesh Jethwani @ 2026-10-15 20:11 UTC (permalink / raw)
  To: netdev; +Cc: John Fastabend, Jakub Kicinski, Sabrina Dubroca,
	Rishikesh Jethwani

Since commit 84c61fe1a75b ("tls: rx: do not use the standard
strparser"), the record being processed stays in the TCP receive queue
in the strparser's normal mode, so tcp_sk(sk)->copied_seq points at the
start of the current record rather than past it. In copy mode it is past
the record, plus one if TCP consumed a FIN with it.

Two users in tls_device.c still assume the old meaning:

Patch 1 fixes the record start reported by the tls_device_decrypted
tracepoint. It saves the record start when the strparser enters copy
mode and adds tls_device_rx_rec_start() to return it.

Patch 2 uses that helper to fix the next-record TCP sequence passed to
the device by core-driven RX resync (TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT,
used only by nfp). In normal mode the device was told that the next
record starts where the current one does.

Patch 1 was tested on mlx5. Patch 2 is untested on hardware; review of
the device-side effect from anyone with nfp kTLS RX would be welcome.

Rishikesh Jethwani (2):
  tls: device: fix record start in the tls_device_decrypted tracepoint
  tls: device: fix next-record sequence in core-driven RX resync

 include/net/tls.h    |  2 ++
 net/tls/tls_device.c | 24 +++++++++++++++++++++---
 net/tls/tls_strp.c   |  2 ++
 3 files changed, 25 insertions(+), 3 deletions(-)

-- 
2.50.1


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

* [PATCH net 1/2] tls: device: fix record start in the tls_device_decrypted tracepoint
  2026-10-15 20:11 [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework Rishikesh Jethwani
@ 2026-10-15 20:11 ` Rishikesh Jethwani
  2026-10-15 20:11 ` [PATCH net 2/2] tls: device: fix next-record sequence in core-driven RX resync Rishikesh Jethwani
  2026-10-15 20:14 ` [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework netdev-bot+sinfo
  2 siblings, 0 replies; 5+ messages in thread
From: Rishikesh Jethwani @ 2026-10-15 20:11 UTC (permalink / raw)
  To: netdev; +Cc: John Fastabend, Jakub Kicinski, Sabrina Dubroca,
	Rishikesh Jethwani

tls_device_decrypted() reports the TCP sequence of the record it is
classifying as tcp_sk(sk)->copied_seq - rxm->full_len. The original
strparser read every record out of the TCP receive queue before the TLS
layer looked at it, so copied_seq had already moved past the record.

The TLS-specific strparser leaves the record in the TCP receive queue in
its normal mode and only calls tcp_read_done() from
tls_strp_msg_consume(), after decryption. When the tracepoint fires,
copied_seq still points at the first byte of the record, and the
reported tcp_seq is one record length too low.

In copy mode (e.g. when the receive queue is short or the record spans
non-contiguous or mixed decrypted/undecrypted skbs), tcp_read_sock()
has already advanced copied_seq past the record. It can also consume a
FIN that ends the record's last skb, which adds one byte of sequence
space absent from the record length, so subtracting the length reports
a start one byte too high. copied_seq cannot tell whether a FIN was
consumed, so the start must be saved before the read.

Save the record start in the strparser when it enters copy mode, and
add tls_device_rx_rec_start() to return it, or copied_seq while the
record remains queued. tls_strp_msg_cow() also enters copy mode, via
tcp_read_done(); save the start there too, so the saved value is valid
whenever copy_mode is set regardless of which path set it.

No functional change beyond the tracepoint argument.

Fixes: 84c61fe1a75b ("tls: rx: do not use the standard strparser")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
Tested on mlx5 (ConnectX-6 Dx) with TLS RX device offload: the
tls_device_decrypted tracepoint now reports the record's starting TCP
sequence. Also build-tested with CONFIG_TLS_DEVICE=n.

 include/net/tls.h    |  2 ++
 net/tls/tls_device.c | 17 ++++++++++++++++-
 net/tls/tls_strp.c   |  2 ++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index e57bef58851e..aebe21e8917d 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -123,6 +123,8 @@ struct tls_strparser {
 	bool msg_ready;
 
 	struct strp_msg stm;
+	/* TCP sequence of the record start, saved before entering copy mode. */
+	u32 copy_seq;
 
 	struct sk_buff *anchor;
 	struct work_struct work;
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index f11d0528fc43..33e6dc3d4164 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -835,6 +835,21 @@ void tls_device_rx_resync_new_rec(struct sock *sk, u32 rcd_len, u32 seq)
 	tls_device_resync_rx(tls_ctx, sk, seq, rcd_sn);
 }
 
+/* TCP sequence of the first byte of the record the strparser holds. In
+ * normal mode the record stays in the TCP receive queue until
+ * tls_strp_msg_consume(), so copied_seq still points at its start. Copy
+ * mode saves the start before TCP reads advance copied_seq, possibly
+ * over a FIN that is not part of the record.
+ */
+static u32 tls_device_rx_rec_start(struct sock *sk,
+				   struct tls_sw_context_rx *sw_ctx)
+{
+	if (sw_ctx->strp.copy_mode)
+		return sw_ctx->strp.copy_seq;
+
+	return tcp_sk(sk)->copied_seq;
+}
+
 static void tls_device_core_ctrl_rx_resync(struct tls_context *tls_ctx,
 					   struct tls_offload_context_rx *ctx,
 					   struct sock *sk, struct sk_buff *skb)
@@ -990,7 +1005,7 @@ int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx)
 		is_encrypted = 0;
 	}
 
-	trace_tls_device_decrypted(sk, tcp_sk(sk)->copied_seq - rxm->full_len,
+	trace_tls_device_decrypted(sk, tls_device_rx_rec_start(sk, sw_ctx),
 				   tls_ctx->rx.rec_seq, rxm->full_len,
 				   is_encrypted, is_decrypted);
 
diff --git a/net/tls/tls_strp.c b/net/tls/tls_strp.c
index 6cc222008d95..ad8b5150ac97 100644
--- a/net/tls/tls_strp.c
+++ b/net/tls/tls_strp.c
@@ -132,6 +132,7 @@ int tls_strp_msg_cow(struct tls_sw_context_rx *ctx)
 	tls_strp_anchor_free(strp);
 	strp->anchor = skb;
 
+	strp->copy_seq = tcp_sk(strp->sk)->copied_seq;
 	tcp_read_done(strp->sk, strp->stm.full_len);
 	strp->copy_mode = 1;
 
@@ -418,6 +419,7 @@ static int tls_strp_read_copy(struct tls_strparser *strp, bool qshort)
 
 	shinfo->frag_list = NULL;
 
+	strp->copy_seq = tcp_sk(strp->sk)->copied_seq;
 	strp->copy_mode = 1;
 	strp->stm.offset = 0;
 
-- 
2.50.1


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

* [PATCH net 2/2] tls: device: fix next-record sequence in core-driven RX resync
  2026-10-15 20:11 [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework Rishikesh Jethwani
  2026-10-15 20:11 ` [PATCH net 1/2] tls: device: fix record start in the tls_device_decrypted tracepoint Rishikesh Jethwani
@ 2026-10-15 20:11 ` Rishikesh Jethwani
  2026-10-15 20:14 ` [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework netdev-bot+sinfo
  2 siblings, 0 replies; 5+ messages in thread
From: Rishikesh Jethwani @ 2026-10-15 20:11 UTC (permalink / raw)
  To: netdev; +Cc: John Fastabend, Jakub Kicinski, Sabrina Dubroca,
	Rishikesh Jethwani

tls_device_core_ctrl_rx_resync() asks the device to resync at the next
record, passing rec_seq + 1 together with tcp_sk(sk)->copied_seq as that
record's TCP sequence. With the original strparser the current record
had already been read out of the TCP receive queue, so copied_seq was
the start of the next record.

The TLS-specific strparser leaves the record queued in its normal mode
until tls_strp_msg_consume(), so copied_seq is still the start of the
current record. The resync branch is taken when tcp_inq() does not
exceed the record length, i.e. typically when the current record is the
only data queued, and the device is told that record rec_seq + 1 begins
where record rec_seq begins. The device is pointed at the wrong record
boundary, so records keep being decrypted in software until a later
attempt, retried with exponential backoff, goes through the
parser-driven path instead.

In copy mode copied_seq is already past the record, so the hint is
correct there; the only exception is a FIN consumed along with the
record, after which no further record can follow.

Use the record start from tls_device_rx_rec_start() plus the record
length instead. That is the start of the next record in both modes, so
the hint no longer depends on which mode the strparser is in.

Only drivers using TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT are affected.

Fixes: 84c61fe1a75b ("tls: rx: do not use the standard strparser")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
Not tested on nfp hardware; I do not have a NIC that uses
TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT. The fix follows the contract in
Documentation/networking/tls-offload.rst ("informs the device about the
next expected record number and its TCP sequence number"). Build-tested
with CONFIG_TLS_DEVICE=y and =n.

 net/tls/tls_device.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 33e6dc3d4164..7dfbead19d3e 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -854,6 +854,7 @@ static void tls_device_core_ctrl_rx_resync(struct tls_context *tls_ctx,
 					   struct tls_offload_context_rx *ctx,
 					   struct sock *sk, struct sk_buff *skb)
 {
+	struct tls_sw_context_rx *sw_ctx = tls_sw_ctx_rx(tls_ctx);
 	struct strp_msg *rxm;
 
 	/* device will request resyncs by itself based on stream scan */
@@ -892,8 +893,10 @@ static void tls_device_core_ctrl_rx_resync(struct tls_context *tls_ctx,
 		memcpy(rcd_sn, tls_ctx->rx.rec_seq, prot->rec_seq_size);
 		tls_bigint_increment(rcd_sn, prot->rec_seq_size);
 
-		tls_device_resync_rx(tls_ctx, sk, tcp_sk(sk)->copied_seq,
-				     rcd_sn);
+		/* the next record starts right after the current one */
+		tls_device_resync_rx(tls_ctx, sk,
+				     tls_device_rx_rec_start(sk, sw_ctx) +
+				     rxm->full_len, rcd_sn);
 	}
 }
 
-- 
2.50.1


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

* Re: [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework
  2026-10-15 20:11 [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework Rishikesh Jethwani
  2026-10-15 20:11 ` [PATCH net 1/2] tls: device: fix record start in the tls_device_decrypted tracepoint Rishikesh Jethwani
  2026-10-15 20:11 ` [PATCH net 2/2] tls: device: fix next-record sequence in core-driven RX resync Rishikesh Jethwani
@ 2026-10-15 20:14 ` netdev-bot+sinfo
  2026-10-15 20:55   ` Rishikesh Jethwani
  2 siblings, 1 reply; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-10-15 20:14 UTC (permalink / raw)
  To: Rishikesh Jethwani
  Cc: netdev, John Fastabend, Jakub Kicinski, Sabrina Dubroca

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] 5+ messages in thread

* Re: [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework
  2026-10-15 20:14 ` [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework netdev-bot+sinfo
@ 2026-10-15 20:55   ` Rishikesh Jethwani
  0 siblings, 0 replies; 5+ messages in thread
From: Rishikesh Jethwani @ 2026-10-15 20:55 UTC (permalink / raw)
  To: netdev-bot+sinfo; +Cc: netdev, John Fastabend, Jakub Kicinski, Sabrina Dubroca

On Thu, Oct 15, 2026 at 1:14 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.

Both issues were found by code inspection, while
working on the TLS 1.3 hardware offload series.

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

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

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15 20:11 [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework Rishikesh Jethwani
2026-10-15 20:11 ` [PATCH net 1/2] tls: device: fix record start in the tls_device_decrypted tracepoint Rishikesh Jethwani
2026-10-15 20:11 ` [PATCH net 2/2] tls: device: fix next-record sequence in core-driven RX resync Rishikesh Jethwani
2026-10-15 20:14 ` [PATCH net 0/2] tls: device: fix users of copied_seq after the strparser rework netdev-bot+sinfo
2026-10-15 20:55   ` Rishikesh Jethwani

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