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

[David Wei <dw@davidwei.uk>]

On 2026-09-17 18:59, Jason Xing wrote:
> On Fri, Sep 18, 2026 at 1:20 AM David Wei <dw@davidwei.uk> wrote:
>>
>> 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 <dw@davidwei.uk>
> 
> Thanks for the work.
> 
> Yesterday I remarked on your V1 at
> https://lore.kernel.org/all/CAL+tcoDMR87sxJftp7T0JX-+NUz8VkXU67M3oNZy8Tk88sDPOA@mail.gmail.com/
> 
> 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
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.

> 
> Thanks,
> Jason
>