Re: [PATCH net v1 3/3] tcp: use skb_get_hwtstamp() for hardware timestamps

Willem de Bruijn
2026-04-29T21:09:09Z

Kohei Enju wrote:
> Since commit 97dc7cd92ac6 ("ptp: Support late timestamp determination"),
> skb_shared_hwtstamps may contain netdev_data instead of hwtstamp. TCP
> receive timestamping can then interpret the stored value as a ktime_t
> and report bogus hardware timestamps to userspace.
>
> Use skb_get_hwtstamp() instead of reading hwtstamp directly, so TCP
> sockets follow the same hardware timestamp resolution path as the socket
> layer. When coalescing SKBs, resolve late timestamps before copying them
> to the merged skb.

Why? Does this preclude supporting SOF_TIMESTAMPING_BIND_PHC for such
sockets at a later time?

It is just as easy to coalesce the cookie as the htwtstamp.

That also avoids the need to mask out SKBTX_HW_TSTAMP_NETDEV.

> Additionally, recognize SKBTX_HW_TSTAMP_NETDEV as
> indicating a receive timestamp is present.

What is the reason for this extra condition?

> Note that skb_get_hwtstamp() is called with cycles == false, since TCP
> hasn't honored SOF_TIMESTAMPING_BIND_PHC so far, and this patch doesn't
> change that behavior.
>
> Fixes: 97dc7cd92ac6 ("ptp: Support late timestamp determination")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> ---
> include/net/tcp.h | 2 +-
> net/ipv4/tcp_input.c | 3 ++-
> net/ipv4/tcp_ipv4.c | 6 ++++--
> net/ipv6/tcp_ipv6.c | 3 ++-
> 4 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index ecbadcb3a744..7b5fcee97079 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -524,7 +524,7 @@ tcp_update_recv_tstamps(struct sk_buff *skb,
> struct scm_timestamping_internal *tss)
> {
> tss->ts[0] = skb->tstamp;
> - tss->ts[2] = skb_hwtstamps(skb)->hwtstamp;
> + tss->ts[2] = skb_get_hwtstamp(skb, false, NULL);
> }
>
> void tcp_recv_timestamp(struct msghdr *msg, const struct sock *sk,
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index d5c9e65d9760..9fd473559b58 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5237,7 +5237,8 @@ static bool tcp_try_coalesce(struct sock *sk,
> if (TCP_SKB_CB(from)->has_rxtstamp) {
> TCP_SKB_CB(to)->has_rxtstamp = true;
> to->tstamp = from->tstamp;
> - skb_hwtstamps(to)->hwtstamp = skb_hwtstamps(from)->hwtstamp;
> + skb_hwtstamps(to)->hwtstamp = skb_get_hwtstamp(from, false, NULL);
> + skb_shinfo(to)->tx_flags &= ~SKBTX_HW_TSTAMP_NETDEV;
> }
>
> return true;
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 8fc24c3743c5..c35d82317764 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1993,7 +1993,8 @@ enum skb_drop_reason tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
> if (TCP_SKB_CB(skb)->has_rxtstamp) {
> TCP_SKB_CB(tail)->has_rxtstamp = true;
> tail->tstamp = skb->tstamp;
> - skb_hwtstamps(tail)->hwtstamp = skb_hwtstamps(skb)->hwtstamp;
> + skb_hwtstamps(tail)->hwtstamp = skb_get_hwtstamp(skb, false, NULL);
> + skb_shinfo(tail)->tx_flags &= ~SKBTX_HW_TSTAMP_NETDEV;
> }
>
> /* Not as strict as GRO. We only need to carry mss max value */
> @@ -2062,7 +2063,8 @@ static void tcp_v4_fill_cb(struct sk_buff *skb, const struct iphdr *iph,
> TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> TCP_SKB_CB(skb)->sacked = 0;
> TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp ||
> + (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV);
> }
>
> /*
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 2c3f7a739709..3361ed7f94de 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1704,7 +1704,8 @@ static void tcp_v6_fill_cb(struct sk_buff *skb, const struct ipv6hdr *hdr,
> TCP_SKB_CB(skb)->ip_dsfield = ipv6_get_dsfield(hdr);
> TCP_SKB_CB(skb)->sacked = 0;
> TCP_SKB_CB(skb)->has_rxtstamp =
> - skb->tstamp || skb_hwtstamps(skb)->hwtstamp;
> + skb->tstamp || skb_hwtstamps(skb)->hwtstamp ||
> + (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV);
> }
>
> INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
> --
> 2.53.0
>