Re: [PATCH RFC net-next 7/9] tcp: propagate the start time onto every skb in the tx path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] bpf: MPTCP subflows leave sk_start_time uninitialized causing hardware timestamp leak
- [High] net: global pskb_copy performance regression for hardware timestamping
- [Medium] bpf: start_time leaks to receiver as hardware timestamp over loopback
- [Medium] tcp: TFO SYN-data segments bypass sk_start_time assignment
--

commit 1c42878ae739db740438d18e58461996447a2443
Author: Jason Xing <kerneljasonxing@gmail.com>

tcp: propagate the start time onto every skb in the tx path
    
Now that sendmsg() records a start time in sk->sk_start_time within
the socket lock protection, copy that value into the shared hwtstamps
of each skb.

The timing is tcp_skb_entail() runs once per newly allocated skb
in the sendmsg() loop, every segment produced by a single sendmsg()
carries the same start time.

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index f07abd53ed27f..040f612608476 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -705,6 +705,9 @@ void tcp_skb_entail(struct sock *sk, struct sk_buff *skb)
>  	tcb->tcp_flags = TCPHDR_ACK;
>  	__skb_header_release(skb);
>  	psp_enqueue_set_decrypted(sk, skb);
> +	if (static_branch_unlikely(&bpfts_v2_needed_key) &&
> +	    SK_BPF_CB_FLAG_TEST(sk, SK_BPF_CB_TIMESTAMPING_V2))
> +		skb_hwtstamps(skb)->start_time = sk->sk_start_time;

[Severity: Medium]
Does this leak the internal timestamp to the receiver as a hardware timestamp
over local virtual interfaces?

When sending traffic over loopback or veth, the SKB enters the receive path.
skb_scrub_packet() clears skb->tstamp but not skb_shinfo(skb)->hwtstamps.

Since start_time and hwtstamp share a union, does the receiving socket
erroneously read the sender's software start_time as a hardware RX timestamp?

[Severity: Medium]
Does this code miss assigning sk_start_time for Fast Open segments?

tcp_sendmsg_fastopen() delegates to tcp_connect() to allocate and queue the
SYN segment carrying payload:

net/ipv4/tcp_output.c:tcp_connect() {
    ...
    buff = tcp_stream_alloc_skb(sk, sk->sk_allocation, true);
    ...
    tcp_init_nondata_skb(buff, sk, tp->write_seq, TCPHDR_SYN);
    ...
    tcp_connect_queue_skb(sk, buff);
}

Both tcp_connect() and tcp_send_syn_data() bypass tcp_skb_entail().
Will this result in a loss of BPF timestamping functionality for any
application using MSG_FASTOPEN?

[Severity: High]
Can this corrupt the shared SKB in the write queue for MPTCP sockets?

MPTCP bypasses tcp_sendmsg_locked(), which leaves the subflow socket's
sk_start_time uninitialized (0). When __mptcp_alloc_tx_skb() calls
tcp_skb_entail(), the SKB's start_time is set to 0.

Upon TX completion with a hardware timestamp, the callback in
skb_tstamp_tx_report_bpf_timestamping() reads the 0:

net/core/skbuff.c:skb_tstamp_tx_report_bpf_timestamping() {
    ...
    if (static_branch_unlikely(&bpfts_v2_needed_key) && start)
        skb_hwtstamps(skb)->start_time = start;
}

Since the check fails when start is 0, does this permanently leak the
hardware timestamp into the shared orig_skb in the transmit queue?

[Severity: High]
Does this series introduce a system-wide performance regression for all
hardware TX timestamping users?

When the static key is enabled, all hardware timestamps fall into this path:

net/core/skbuff.c:__skb_tstamp_tx() {
    ...
    if (static_branch_unlikely(&bpfts_v2_needed_key) && hwtstamps)
        skb = pskb_copy(orig_skb, GFP_ATOMIC);
    else
        skb = skb_clone(orig_skb, GFP_ATOMIC);
}

Because the static key is globally enabled when BPF timestamping is used,
will unrelated sockets using normal SO_TIMESTAMPING suffer the high overhead
of copying the entire SKB head via pskb_copy() instead of skb_clone()?

>  	tcp_add_write_queue_tail(sk, skb);
>  	sk_wmem_queued_add(sk, skb->truesize);
>  	sk_mem_charge(sk, skb->truesize);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919143732.11772-1-kerneljasonxing@gmail.com?part=7