Thread (4 messages) flat view 4 messages, 2 authors, 3d ago

Re: [PATCH net v2] tcp: prevent stale tx timestamp keys on rtx fallback

From: Jason Xing <hidden>
Date: 2026-09-18 05:06:08
Also in: bpf

On Fri, Sep 18, 2026 at 12:35 PM David Wei [off-list ref] wrote:
On 2026-09-17 18:59, Jason Xing wrote:
quoted
On Fri, Sep 18, 2026 at 1:20 AM David Wei [off-list ref] wrote:
quoted
tcp_tx_timestamp() can select skbs from the rtx queue when all the
copied data has been sent in tcp_sendmsg_locked(). These skbs may be
already cloned, sharing the same shinfo, and handed off into the lower
tx layers.

tcp_tx_timestamp() sets tx_flags before setting skb tskey, racing with
any reader of both. It is possible to observe a valid tx_flag, but an
uninitialized tskey, which produces a large underflow after subtracting
the socket tskey.

Reorder the writes in tcp_tx_timestamp() and
bpf_sock_ops_enable_tx_tstamp() to write the tskey first, followed by
publishing tx_flags via store-release.

Readers perform a symmetric load-acquire on the tx_flags, followed by a
relaxed read of tskey.

Fixes: 838eb9687691 ("tcp: tcp_tx_timestamp() must look at the rtx queue")
Assisted-by: LLM
Signed-off-by: David Wei <redacted>
Thanks for the work.

Yesterday I remarked on your V1 at
https://lore.kernel.org/all/CAL+tcoDMR87sxJftp7T0JX-+NUz8VkXU67M3oNZy8Tk88sDPOA@mail.gmail.com/ (local)

I'm still doubtful if we really need to introduce this much code churn
just to fix the problem of this best-effort behavior. Even if the
patch is fixed thoroughly, it's still a best-effort attempt since
there are a few report points where it misses generating timestamps.
It has nothing to do with the patch itself, but rather the design of
net timestamping.
Hi Jason, I saw your response yesterday but it seemed like you were
discussing with Eric and not me. I had this code ready anyway before you
+ Willem

Oh, well, my true intention is to discuss this with all of you :)
responded, so I sent it should Eric decide to keep 838eb9687691 and
review this approach.

If you decide to revert 838eb9687691, that is fine too and I can abandon
this patch. The main thing for me is to prevent userspace from seeing
underflows, but missing timestamps are okay.
I see. I'm still struggling, to be honest...

Thanks,
Jason
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help