[PATCH net] tcp: preserve timestamps across receive queue collapse

[PATCH net] tcp: preserve timestamps across receive queue collapse

[Jason Xing]

When tcp_collapse() rebuilds skbs under memory pressure, the copy
process doesn't include the right tstamp and hwtstamp from the
old skb.

In net timestamping case, if such an skb happens to be the last
one consumed in a recvmsg() call, the application receives no RX
timestamp for that call.

Fix this by copying both tstamp and hwtstamp to the new skb right
after the initialization.

Fixes: 98aaa913b4ed ("tcp: Extend SOF_TIMESTAMPING_RX_SOFTWARE to TCP recvmsg")
Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
---
 net/ipv4/tcp_input.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0f60a1dbf927..c5756db892f2 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5835,6 +5835,8 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 
 		memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
 		skb_copy_decrypted(nskb, skb);
+		nskb->tstamp = skb->tstamp;
+		skb_hwtstamps(nskb)->hwtstamp = skb_hwtstamps(skb)->hwtstamp;
 		TCP_SKB_CB(nskb)->seq = TCP_SKB_CB(nskb)->end_seq = start;
 		if (list)
 			__skb_queue_before(list, skb, nskb);
-- 
2.43.5

Re: [PATCH net] tcp: preserve timestamps across receive queue collapse

[Eric Dumazet]

On Wed, Sep 16, 2026 at 5:42 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> When tcp_collapse() rebuilds skbs under memory pressure, the copy
> process doesn't include the right tstamp and hwtstamp from the
> old skb.
>

Good catch, the problem is real: memcpy(nskb->cb, skb->cb, ...) copies
has_rxtstamp, but nskb->tstamp and hwtstamps are left at zero, so
tcp_recv_timestamp() ends up emitting no cmsg at all.


> In net timestamping case, if such an skb happens to be the last
> one consumed in a recvmsg() call, the application receives no RX
> timestamp for that call.
>
> Fix this by copying both tstamp and hwtstamp to the new skb right
> after the initialization.
>
> Fixes: 98aaa913b4ed ("tcp: Extend SOF_TIMESTAMPING_RX_SOFTWARE to TCP recvmsg")
> Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
> ---
>  net/ipv4/tcp_input.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 0f60a1dbf927..c5756db892f2 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5835,6 +5835,8 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>
>                 memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
>                 skb_copy_decrypted(nskb, skb);
> +               nskb->tstamp = skb->tstamp;
> +               skb_hwtstamps(nskb)->hwtstamp = skb_hwtstamps(skb)->hwtstamp;
>                 TCP_SKB_CB(nskb)->seq = TCP_SKB_CB(nskb)->end_seq = start;
>                 if (list)
>                         __skb_queue_before(list, skb, nskb);

However this samples the wrong skb. nskb aggregates data from several
skbs in the loop below, and you are copying the timestamp of the first
one, while 98aaa913b4ed (the commit you are fixing) established the
opposite invariant:

  "When skbs are coalesced either in the sk_receive_queue or the
   out_of_order_queue always choose the timestamp of the appended skb
   to maintain the invariant of returning the timestamp of the last
   byte in the recvmsg buffer."

tcp_try_coalesce() and tcp_add_backlog() both implement "last one
wins", please do the same here, something like:

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0f60a1dbf927..81d587654d3d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5854,6 +5854,12 @@ tcp_collapse(struct sock *sk, struct
sk_buff_head *list, struct rb_root *root,
                                if (skb_copy_bits(skb, offset,
skb_put(nskb, size), size))
                                        BUG();
                                TCP_SKB_CB(nskb)->end_seq += size;
+                               if (TCP_SKB_CB(skb)->has_rxtstamp) {
+                                       TCP_SKB_CB(nskb)->has_rxtstamp = true;
+                                       nskb->tstamp = skb->tstamp;
+                                       skb_hwtstamps(nskb)->hwtstamp =
+                                               skb_hwtstamps(skb)->hwtstamp;
+                               }
                                copy -= size;
                                start += size;
                        }

This would be the fourth copy of these lines, but let's keep the fix
minimal for net, a helper shared by the three call sites can come
later in net-next.

This is testable from user space (small SO_RCVBUF + many small
segments + delayed read hits tcp_prune_queue(), see
LINUX_MIB_TCPRCVCOLLAPSED); a selftest addition to
tools/testing/selftests/net/rxtimestamp.c would be welcome.

Thanks.

pw-bot: cr

Re: [PATCH net] tcp: preserve timestamps across receive queue collapse

[Jason Xing]

On Wed, Sep 16, 2026 at 9:06 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Sep 16, 2026 at 5:42 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > When tcp_collapse() rebuilds skbs under memory pressure, the copy
> > process doesn't include the right tstamp and hwtstamp from the
> > old skb.
> >
>
> Good catch, the problem is real: memcpy(nskb->cb, skb->cb, ...) copies
> has_rxtstamp, but nskb->tstamp and hwtstamps are left at zero, so
> tcp_recv_timestamp() ends up emitting no cmsg at all.
>
>
> > In net timestamping case, if such an skb happens to be the last
> > one consumed in a recvmsg() call, the application receives no RX
> > timestamp for that call.
> >
> > Fix this by copying both tstamp and hwtstamp to the new skb right
> > after the initialization.
> >
> > Fixes: 98aaa913b4ed ("tcp: Extend SOF_TIMESTAMPING_RX_SOFTWARE to TCP recvmsg")
> > Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
> > ---
> >  net/ipv4/tcp_input.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 0f60a1dbf927..c5756db892f2 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -5835,6 +5835,8 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
> >
> >                 memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
> >                 skb_copy_decrypted(nskb, skb);
> > +               nskb->tstamp = skb->tstamp;
> > +               skb_hwtstamps(nskb)->hwtstamp = skb_hwtstamps(skb)->hwtstamp;
> >                 TCP_SKB_CB(nskb)->seq = TCP_SKB_CB(nskb)->end_seq = start;
> >                 if (list)
> >                         __skb_queue_before(list, skb, nskb);
>
> However this samples the wrong skb. nskb aggregates data from several
> skbs in the loop below, and you are copying the timestamp of the first
> one, while 98aaa913b4ed (the commit you are fixing) established the
> opposite invariant:
>
>   "When skbs are coalesced either in the sk_receive_queue or the
>    out_of_order_queue always choose the timestamp of the appended skb
>    to maintain the invariant of returning the timestamp of the last
>    byte in the recvmsg buffer."
>
> tcp_try_coalesce() and tcp_add_backlog() both implement "last one
> wins", please do the same here, something like:
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 0f60a1dbf927..81d587654d3d 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5854,6 +5854,12 @@ tcp_collapse(struct sock *sk, struct
> sk_buff_head *list, struct rb_root *root,
>                                 if (skb_copy_bits(skb, offset,
> skb_put(nskb, size), size))
>                                         BUG();
>                                 TCP_SKB_CB(nskb)->end_seq += size;
> +                               if (TCP_SKB_CB(skb)->has_rxtstamp) {
> +                                       TCP_SKB_CB(nskb)->has_rxtstamp = true;
> +                                       nskb->tstamp = skb->tstamp;
> +                                       skb_hwtstamps(nskb)->hwtstamp =
> +                                               skb_hwtstamps(skb)->hwtstamp;
> +                               }
>                                 copy -= size;
>                                 start += size;
>                         }
>

Right, it works and adheres to the same semantics as before. But
honestly I feel a bit strange because using the last skb loses the
latency delta between the first one and the last one. Even though the
delta should be so small as to be negligible.

> This would be the fourth copy of these lines, but let's keep the fix
> minimal for net, a helper shared by the three call sites can come
> later in net-next.

Sure, I will cook it after the patch gets merged in the -next branch.

Thanks,
Jason

>
> This is testable from user space (small SO_RCVBUF + many small
> segments + delayed read hits tcp_prune_queue(), see
> LINUX_MIB_TCPRCVCOLLAPSED); a selftest addition to
> tools/testing/selftests/net/rxtimestamp.c would be welcome.
>
> Thanks.
>
> pw-bot: cr