* [PATCH net v1 0/3] af_packet/tcp: fix late hardware timestamp handling
@ 2026-04-29 9:16 Kohei Enju
2026-04-29 9:16 ` [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb Kohei Enju
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Kohei Enju @ 2026-04-29 9:16 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran,
Kohei Enju
Since commit 97dc7cd92ac6 ("ptp: Support late timestamp determination"),
skb_shared_hwtstamps may carry netdev_data instead of a resolved
hwtstamp. AF_PACKET and TCP could still read that storage as a ktime_t
and report bogus hardware timestamps to userspace.
This series factors the late timestamp resolution logic into a common
helper, then switches AF_PACKET and TCP to use it.
Notes on SOF_TIMESTAMPING_BIND_PHC:
The generic socket receive timestamping path honors it, but the
AF_PACKET and TCP receive paths touched here haven't implemented that
behavior.
This series doesn't change that; those paths always resolve timestamps
with cycles == false and preserve their timestamp-domain semantics.
Kohei Enju (3):
net: introduce helper to resolve hardware timestamps from skb
af_packet: use skb_get_hwtstamp() for hardware timestamps
tcp: use skb_get_hwtstamp() for hardware timestamps
include/linux/skbuff.h | 11 +++++++++++
include/net/tcp.h | 2 +-
net/core/skbuff.c | 27 +++++++++++++++++++++++++++
net/ipv4/tcp_input.c | 3 ++-
net/ipv4/tcp_ipv4.c | 6 ++++--
net/ipv6/tcp_ipv6.c | 3 ++-
net/packet/af_packet.c | 2 +-
net/socket.c | 27 +++------------------------
8 files changed, 51 insertions(+), 30 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb 2026-04-29 9:16 [PATCH net v1 0/3] af_packet/tcp: fix late hardware timestamp handling Kohei Enju @ 2026-04-29 9:16 ` Kohei Enju 2026-04-29 21:04 ` Willem de Bruijn 2026-05-01 20:25 ` Gerhard Engleder 2026-04-29 9:16 ` [PATCH net v1 2/3] af_packet: use skb_get_hwtstamp() for hardware timestamps Kohei Enju 2026-04-29 9:16 ` [PATCH net v1 3/3] tcp: " Kohei Enju 2 siblings, 2 replies; 11+ messages in thread From: Kohei Enju @ 2026-04-29 9:16 UTC (permalink / raw) To: netdev Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran, Kohei Enju Move the logic that resolves a hardware timestamp from an skb, including late timestamp resolution via netdev_get_tstamp(), from net/socket.c to a common helper. Let's allow other networking code to reuse the same resolution path. Signed-off-by: Kohei Enju <kohei@enjuk.jp> --- include/linux/skbuff.h | 11 +++++++++++ net/core/skbuff.c | 27 +++++++++++++++++++++++++++ net/socket.c | 27 +++------------------------ 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 2bcf78a4de7b..651a5ae8b11c 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -4731,6 +4731,17 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb, const struct sk_buff *ack_skb, void skb_tstamp_tx(struct sk_buff *orig_skb, struct skb_shared_hwtstamps *hwtstamps); +/** + * skb_get_hwtstamp - resolve a hardware timestamp from an skb + * @skb: skb carrying the timestamp + * @cycles: true to request the free-running cycle-based timestamp + * @if_index: optional return pointer for the originating netdev ifindex + * + * Return: resolved hardware timestamp, or the stored skb hwtstamp when no + * device-specific late timestamp resolution is needed. + */ +ktime_t skb_get_hwtstamp(struct sk_buff *skb, bool cycles, int *if_index); + /** * skb_tx_timestamp() - Driver hook for transmit timestamping * diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 7dad68e3b518..d11f4e2e9391 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -5729,6 +5729,33 @@ void skb_tstamp_tx(struct sk_buff *orig_skb, } EXPORT_SYMBOL_GPL(skb_tstamp_tx); +ktime_t skb_get_hwtstamp(struct sk_buff *skb, bool cycles, int *if_index) +{ + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); + struct net_device *orig_dev; + ktime_t hwtstamp; + + if (if_index) + *if_index = 0; + + if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV)) + return shhwtstamps->hwtstamp; + + rcu_read_lock(); + orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); + if (orig_dev) { + if (if_index) + *if_index = orig_dev->ifindex; + hwtstamp = netdev_get_tstamp(orig_dev, shhwtstamps, cycles); + } else { + hwtstamp = shhwtstamps->hwtstamp; + } + rcu_read_unlock(); + + return hwtstamp; +} +EXPORT_SYMBOL_GPL(skb_get_hwtstamp); + #ifdef CONFIG_WIRELESS void skb_complete_wifi_ack(struct sk_buff *skb, bool acked) { diff --git a/net/socket.c b/net/socket.c index 22a412fdec07..95b21b16a0fc 100644 --- a/net/socket.c +++ b/net/socket.c @@ -876,21 +876,7 @@ static bool skb_is_swtx_tstamp(const struct sk_buff *skb, int false_tstamp) static ktime_t get_timestamp(struct sock *sk, struct sk_buff *skb, int *if_index) { bool cycles = READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_BIND_PHC; - struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); - struct net_device *orig_dev; - ktime_t hwtstamp; - - rcu_read_lock(); - orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); - if (orig_dev) { - *if_index = orig_dev->ifindex; - hwtstamp = netdev_get_tstamp(orig_dev, shhwtstamps, cycles); - } else { - hwtstamp = shhwtstamps->hwtstamp; - } - rcu_read_unlock(); - - return hwtstamp; + return skb_get_hwtstamp(skb, cycles, if_index); } static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb, @@ -940,7 +926,6 @@ int skb_get_tx_timestamp(struct sk_buff *skb, struct sock *sk, { u32 tsflags = READ_ONCE(sk->sk_tsflags); ktime_t hwtstamp; - int if_index = 0; if ((tsflags & SOF_TIMESTAMPING_SOFTWARE) && ktime_to_timespec64_cond(skb->tstamp, ts)) @@ -950,10 +935,7 @@ int skb_get_tx_timestamp(struct sk_buff *skb, struct sock *sk, skb_is_swtx_tstamp(skb, false)) return -ENOENT; - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV) - hwtstamp = get_timestamp(sk, skb, &if_index); - else - hwtstamp = skb_hwtstamps(skb)->hwtstamp; + hwtstamp = get_timestamp(sk, skb, NULL); if (tsflags & SOF_TIMESTAMPING_BIND_PHC) hwtstamp = ptp_convert_timestamp(&hwtstamp, @@ -1033,10 +1015,7 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk, !(tsflags & SOF_TIMESTAMPING_OPT_RX_FILTER))) && !skb_is_swtx_tstamp(skb, false_tstamp)) { if_index = 0; - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV) - hwtstamp = get_timestamp(sk, skb, &if_index); - else - hwtstamp = shhwtstamps->hwtstamp; + hwtstamp = get_timestamp(sk, skb, &if_index); if (tsflags & SOF_TIMESTAMPING_BIND_PHC) hwtstamp = ptp_convert_timestamp(&hwtstamp, -- 2.53.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb 2026-04-29 9:16 ` [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb Kohei Enju @ 2026-04-29 21:04 ` Willem de Bruijn 2026-04-30 4:50 ` Kohei Enju 2026-05-01 20:25 ` Gerhard Engleder 1 sibling, 1 reply; 11+ messages in thread From: Willem de Bruijn @ 2026-04-29 21:04 UTC (permalink / raw) To: Kohei Enju, netdev Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran, Kohei Enju Kohei Enju wrote: > Move the logic that resolves a hardware timestamp from an skb, including > late timestamp resolution via netdev_get_tstamp(), from net/socket.c to > a common helper. > > Let's allow other networking code to reuse the same resolution path. > > Signed-off-by: Kohei Enju <kohei@enjuk.jp> Thanks for the fix series. Fixes require a Fixes tag. See also https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html. I suggest merging this and the second patch, as this is not a standalone fix, and the other is a one-line change. > --- > include/linux/skbuff.h | 11 +++++++++++ > net/core/skbuff.c | 27 +++++++++++++++++++++++++++ > net/socket.c | 27 +++------------------------ > 3 files changed, 41 insertions(+), 24 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 2bcf78a4de7b..651a5ae8b11c 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -4731,6 +4731,17 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb, const struct sk_buff *ack_skb, > void skb_tstamp_tx(struct sk_buff *orig_skb, > struct skb_shared_hwtstamps *hwtstamps); > > +/** > + * skb_get_hwtstamp - resolve a hardware timestamp from an skb > + * @skb: skb carrying the timestamp > + * @cycles: true to request the free-running cycle-based timestamp > + * @if_index: optional return pointer for the originating netdev ifindex > + * > + * Return: resolved hardware timestamp, or the stored skb hwtstamp when no > + * device-specific late timestamp resolution is needed. > + */ > +ktime_t skb_get_hwtstamp(struct sk_buff *skb, bool cycles, int *if_index); > + > /** > * skb_tx_timestamp() - Driver hook for transmit timestamping > * > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 7dad68e3b518..d11f4e2e9391 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -5729,6 +5729,33 @@ void skb_tstamp_tx(struct sk_buff *orig_skb, > } > EXPORT_SYMBOL_GPL(skb_tstamp_tx); > > +ktime_t skb_get_hwtstamp(struct sk_buff *skb, bool cycles, int *if_index) > +{ > + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > + struct net_device *orig_dev; > + ktime_t hwtstamp; > + > + if (if_index) > + *if_index = 0; > + > + if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV)) > + return shhwtstamps->hwtstamp; > + > + rcu_read_lock(); > + orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); > + if (orig_dev) { > + if (if_index) > + *if_index = orig_dev->ifindex; > + hwtstamp = netdev_get_tstamp(orig_dev, shhwtstamps, cycles); > + } else { > + hwtstamp = shhwtstamps->hwtstamp; > + } > + rcu_read_unlock(); > + > + return hwtstamp; > +} > +EXPORT_SYMBOL_GPL(skb_get_hwtstamp); > + > #ifdef CONFIG_WIRELESS > void skb_complete_wifi_ack(struct sk_buff *skb, bool acked) > { > diff --git a/net/socket.c b/net/socket.c > index 22a412fdec07..95b21b16a0fc 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -876,21 +876,7 @@ static bool skb_is_swtx_tstamp(const struct sk_buff *skb, int false_tstamp) > static ktime_t get_timestamp(struct sock *sk, struct sk_buff *skb, int *if_index) > { > bool cycles = READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_BIND_PHC; > - struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > - struct net_device *orig_dev; > - ktime_t hwtstamp; > - > - rcu_read_lock(); > - orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); > - if (orig_dev) { > - *if_index = orig_dev->ifindex; > - hwtstamp = netdev_get_tstamp(orig_dev, shhwtstamps, cycles); > - } else { > - hwtstamp = shhwtstamps->hwtstamp; > - } > - rcu_read_unlock(); > - > - return hwtstamp; > + return skb_get_hwtstamp(skb, cycles, if_index); > } At this point simpler to remove get_timestamp entirely. It's an unnecessary layer of indirection. Perhaps pass sk_tsflags rather than bool to skb_get_hwtstamp. > static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb, > @@ -940,7 +926,6 @@ int skb_get_tx_timestamp(struct sk_buff *skb, struct sock *sk, > { > u32 tsflags = READ_ONCE(sk->sk_tsflags); > ktime_t hwtstamp; > - int if_index = 0; > > if ((tsflags & SOF_TIMESTAMPING_SOFTWARE) && > ktime_to_timespec64_cond(skb->tstamp, ts)) > @@ -950,10 +935,7 @@ int skb_get_tx_timestamp(struct sk_buff *skb, struct sock *sk, > skb_is_swtx_tstamp(skb, false)) > return -ENOENT; > > - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV) > - hwtstamp = get_timestamp(sk, skb, &if_index); > - else > - hwtstamp = skb_hwtstamps(skb)->hwtstamp; > + hwtstamp = get_timestamp(sk, skb, NULL); > > if (tsflags & SOF_TIMESTAMPING_BIND_PHC) > hwtstamp = ptp_convert_timestamp(&hwtstamp, > @@ -1033,10 +1015,7 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk, > !(tsflags & SOF_TIMESTAMPING_OPT_RX_FILTER))) && > !skb_is_swtx_tstamp(skb, false_tstamp)) { > if_index = 0; > - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV) > - hwtstamp = get_timestamp(sk, skb, &if_index); > - else > - hwtstamp = shhwtstamps->hwtstamp; > + hwtstamp = get_timestamp(sk, skb, &if_index); > > if (tsflags & SOF_TIMESTAMPING_BIND_PHC) > hwtstamp = ptp_convert_timestamp(&hwtstamp, > -- > 2.53.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb 2026-04-29 21:04 ` Willem de Bruijn @ 2026-04-30 4:50 ` Kohei Enju 0 siblings, 0 replies; 11+ messages in thread From: Kohei Enju @ 2026-04-30 4:50 UTC (permalink / raw) To: Willem de Bruijn Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran On 04/29 17:04, Willem de Bruijn wrote: > Kohei Enju wrote: > > Move the logic that resolves a hardware timestamp from an skb, including > > late timestamp resolution via netdev_get_tstamp(), from net/socket.c to > > a common helper. > > > > Let's allow other networking code to reuse the same resolution path. > > > > Signed-off-by: Kohei Enju <kohei@enjuk.jp> > > Thanks for the fix series. > > Fixes require a Fixes tag. See also https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html. > > I suggest merging this and the second patch, as this is not a > standalone fix, and the other is a one-line change. Thank you for the suggestion, and it looks good to me. I'll do so in v2. > > > --- > > include/linux/skbuff.h | 11 +++++++++++ > > net/core/skbuff.c | 27 +++++++++++++++++++++++++++ > > net/socket.c | 27 +++------------------------ > > 3 files changed, 41 insertions(+), 24 deletions(-) > > > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > > index 2bcf78a4de7b..651a5ae8b11c 100644 > > --- a/include/linux/skbuff.h > > +++ b/include/linux/skbuff.h > > @@ -4731,6 +4731,17 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb, const struct sk_buff *ack_skb, > > void skb_tstamp_tx(struct sk_buff *orig_skb, > > struct skb_shared_hwtstamps *hwtstamps); > > > > +/** > > + * skb_get_hwtstamp - resolve a hardware timestamp from an skb > > + * @skb: skb carrying the timestamp > > + * @cycles: true to request the free-running cycle-based timestamp > > + * @if_index: optional return pointer for the originating netdev ifindex > > + * > > + * Return: resolved hardware timestamp, or the stored skb hwtstamp when no > > + * device-specific late timestamp resolution is needed. > > + */ > > +ktime_t skb_get_hwtstamp(struct sk_buff *skb, bool cycles, int *if_index); > > + > > /** > > * skb_tx_timestamp() - Driver hook for transmit timestamping > > * > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > > index 7dad68e3b518..d11f4e2e9391 100644 > > --- a/net/core/skbuff.c > > +++ b/net/core/skbuff.c > > @@ -5729,6 +5729,33 @@ void skb_tstamp_tx(struct sk_buff *orig_skb, > > } > > EXPORT_SYMBOL_GPL(skb_tstamp_tx); > > > > +ktime_t skb_get_hwtstamp(struct sk_buff *skb, bool cycles, int *if_index) > > +{ > > + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > > + struct net_device *orig_dev; > > + ktime_t hwtstamp; > > + > > + if (if_index) > > + *if_index = 0; > > + > > + if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV)) > > + return shhwtstamps->hwtstamp; > > + > > + rcu_read_lock(); > > + orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); > > + if (orig_dev) { > > + if (if_index) > > + *if_index = orig_dev->ifindex; > > + hwtstamp = netdev_get_tstamp(orig_dev, shhwtstamps, cycles); > > + } else { > > + hwtstamp = shhwtstamps->hwtstamp; > > + } > > + rcu_read_unlock(); > > + > > + return hwtstamp; > > +} > > +EXPORT_SYMBOL_GPL(skb_get_hwtstamp); > > + > > #ifdef CONFIG_WIRELESS > > void skb_complete_wifi_ack(struct sk_buff *skb, bool acked) > > { > > diff --git a/net/socket.c b/net/socket.c > > index 22a412fdec07..95b21b16a0fc 100644 > > --- a/net/socket.c > > +++ b/net/socket.c > > @@ -876,21 +876,7 @@ static bool skb_is_swtx_tstamp(const struct sk_buff *skb, int false_tstamp) > > static ktime_t get_timestamp(struct sock *sk, struct sk_buff *skb, int *if_index) > > { > > bool cycles = READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_BIND_PHC; > > - struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > > - struct net_device *orig_dev; > > - ktime_t hwtstamp; > > - > > - rcu_read_lock(); > > - orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); > > - if (orig_dev) { > > - *if_index = orig_dev->ifindex; > > - hwtstamp = netdev_get_tstamp(orig_dev, shhwtstamps, cycles); > > - } else { > > - hwtstamp = shhwtstamps->hwtstamp; > > - } > > - rcu_read_unlock(); > > - > > - return hwtstamp; > > + return skb_get_hwtstamp(skb, cycles, if_index); > > } > > At this point simpler to remove get_timestamp entirely. It's an > unnecessary layer of indirection. > > Perhaps pass sk_tsflags rather than bool to skb_get_hwtstamp. Good points. I'll rework this in v2. Thanks, Kohei > > > static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb, > > @@ -940,7 +926,6 @@ int skb_get_tx_timestamp(struct sk_buff *skb, struct sock *sk, > > { > > u32 tsflags = READ_ONCE(sk->sk_tsflags); > > ktime_t hwtstamp; > > - int if_index = 0; > > > > if ((tsflags & SOF_TIMESTAMPING_SOFTWARE) && > > ktime_to_timespec64_cond(skb->tstamp, ts)) > > @@ -950,10 +935,7 @@ int skb_get_tx_timestamp(struct sk_buff *skb, struct sock *sk, > > skb_is_swtx_tstamp(skb, false)) > > return -ENOENT; > > > > - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV) > > - hwtstamp = get_timestamp(sk, skb, &if_index); > > - else > > - hwtstamp = skb_hwtstamps(skb)->hwtstamp; > > + hwtstamp = get_timestamp(sk, skb, NULL); > > > > if (tsflags & SOF_TIMESTAMPING_BIND_PHC) > > hwtstamp = ptp_convert_timestamp(&hwtstamp, > > @@ -1033,10 +1015,7 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk, > > !(tsflags & SOF_TIMESTAMPING_OPT_RX_FILTER))) && > > !skb_is_swtx_tstamp(skb, false_tstamp)) { > > if_index = 0; > > - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV) > > - hwtstamp = get_timestamp(sk, skb, &if_index); > > - else > > - hwtstamp = shhwtstamps->hwtstamp; > > + hwtstamp = get_timestamp(sk, skb, &if_index); > > > > if (tsflags & SOF_TIMESTAMPING_BIND_PHC) > > hwtstamp = ptp_convert_timestamp(&hwtstamp, > > -- > > 2.53.0 > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb 2026-04-29 9:16 ` [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb Kohei Enju 2026-04-29 21:04 ` Willem de Bruijn @ 2026-05-01 20:25 ` Gerhard Engleder 1 sibling, 0 replies; 11+ messages in thread From: Gerhard Engleder @ 2026-05-01 20:25 UTC (permalink / raw) To: Kohei Enju, netdev Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Jonathan Lemon, Richard Cochran On 29.04.26 11:16, Kohei Enju wrote: > Move the logic that resolves a hardware timestamp from an skb, including > late timestamp resolution via netdev_get_tstamp(), from net/socket.c to > a common helper. > > Let's allow other networking code to reuse the same resolution path. > > Signed-off-by: Kohei Enju <kohei@enjuk.jp> > --- > include/linux/skbuff.h | 11 +++++++++++ > net/core/skbuff.c | 27 +++++++++++++++++++++++++++ > net/socket.c | 27 +++------------------------ > 3 files changed, 41 insertions(+), 24 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 2bcf78a4de7b..651a5ae8b11c 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -4731,6 +4731,17 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb, const struct sk_buff *ack_skb, > void skb_tstamp_tx(struct sk_buff *orig_skb, > struct skb_shared_hwtstamps *hwtstamps); > > +/** > + * skb_get_hwtstamp - resolve a hardware timestamp from an skb > + * @skb: skb carrying the timestamp > + * @cycles: true to request the free-running cycle-based timestamp > + * @if_index: optional return pointer for the originating netdev ifindex > + * > + * Return: resolved hardware timestamp, or the stored skb hwtstamp when no > + * device-specific late timestamp resolution is needed. > + */ > +ktime_t skb_get_hwtstamp(struct sk_buff *skb, bool cycles, int *if_index); > + > /** > * skb_tx_timestamp() - Driver hook for transmit timestamping > * > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 7dad68e3b518..d11f4e2e9391 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -5729,6 +5729,33 @@ void skb_tstamp_tx(struct sk_buff *orig_skb, > } > EXPORT_SYMBOL_GPL(skb_tstamp_tx); > > +ktime_t skb_get_hwtstamp(struct sk_buff *skb, bool cycles, int *if_index) > +{ > + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > + struct net_device *orig_dev; > + ktime_t hwtstamp; > + > + if (if_index) > + *if_index = 0; if_index is set to 0 here the second time when called from __sock_recv_timestamp(). IMO you can remove these two lines, because in skb_get_tx_timestamp() if_index has been removed by your commit. > + > + if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV)) > + return shhwtstamps->hwtstamp; > + > + rcu_read_lock(); > + orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); > + if (orig_dev) { > + if (if_index) > + *if_index = orig_dev->ifindex; > + hwtstamp = netdev_get_tstamp(orig_dev, shhwtstamps, cycles); > + } else { > + hwtstamp = shhwtstamps->hwtstamp; > + } > + rcu_read_unlock(); > + > + return hwtstamp; > +} > +EXPORT_SYMBOL_GPL(skb_get_hwtstamp); > + > #ifdef CONFIG_WIRELESS > void skb_complete_wifi_ack(struct sk_buff *skb, bool acked) > { > diff --git a/net/socket.c b/net/socket.c > index 22a412fdec07..95b21b16a0fc 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -876,21 +876,7 @@ static bool skb_is_swtx_tstamp(const struct sk_buff *skb, int false_tstamp) > static ktime_t get_timestamp(struct sock *sk, struct sk_buff *skb, int *if_index) > { > bool cycles = READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_BIND_PHC; > - struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); > - struct net_device *orig_dev; > - ktime_t hwtstamp; > - > - rcu_read_lock(); > - orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); > - if (orig_dev) { > - *if_index = orig_dev->ifindex; > - hwtstamp = netdev_get_tstamp(orig_dev, shhwtstamps, cycles); > - } else { > - hwtstamp = shhwtstamps->hwtstamp; > - } > - rcu_read_unlock(); > - > - return hwtstamp; > + return skb_get_hwtstamp(skb, cycles, if_index); > } > > static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb, > @@ -940,7 +926,6 @@ int skb_get_tx_timestamp(struct sk_buff *skb, struct sock *sk, > { > u32 tsflags = READ_ONCE(sk->sk_tsflags); > ktime_t hwtstamp; > - int if_index = 0; if_index is useless in this function, so it can be removed. This was introduced with 2410251cde0b. Maybe removing if_index here could be a separate commit, as this clean up makes sense on its own. > > if ((tsflags & SOF_TIMESTAMPING_SOFTWARE) && > ktime_to_timespec64_cond(skb->tstamp, ts)) > @@ -950,10 +935,7 @@ int skb_get_tx_timestamp(struct sk_buff *skb, struct sock *sk, > skb_is_swtx_tstamp(skb, false)) > return -ENOENT; > > - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV) > - hwtstamp = get_timestamp(sk, skb, &if_index); > - else > - hwtstamp = skb_hwtstamps(skb)->hwtstamp; > + hwtstamp = get_timestamp(sk, skb, NULL); > > if (tsflags & SOF_TIMESTAMPING_BIND_PHC) > hwtstamp = ptp_convert_timestamp(&hwtstamp, > @@ -1033,10 +1015,7 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk, > !(tsflags & SOF_TIMESTAMPING_OPT_RX_FILTER))) && > !skb_is_swtx_tstamp(skb, false_tstamp)) { > if_index = 0; > - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV) > - hwtstamp = get_timestamp(sk, skb, &if_index); > - else > - hwtstamp = shhwtstamps->hwtstamp; > + hwtstamp = get_timestamp(sk, skb, &if_index); > > if (tsflags & SOF_TIMESTAMPING_BIND_PHC) > hwtstamp = ptp_convert_timestamp(&hwtstamp, Gerhard ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net v1 2/3] af_packet: use skb_get_hwtstamp() for hardware timestamps 2026-04-29 9:16 [PATCH net v1 0/3] af_packet/tcp: fix late hardware timestamp handling Kohei Enju 2026-04-29 9:16 ` [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb Kohei Enju @ 2026-04-29 9:16 ` Kohei Enju 2026-04-29 9:16 ` [PATCH net v1 3/3] tcp: " Kohei Enju 2 siblings, 0 replies; 11+ messages in thread From: Kohei Enju @ 2026-04-29 9:16 UTC (permalink / raw) To: netdev Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran, Kohei Enju Since commit 97dc7cd92ac6 ("ptp: Support late timestamp determination"), skb_shared_hwtstamps may contain netdev_data instead of hwtstamp. tpacket_get_timestamp() unconditionally interprets skb_shared_hwtstamps as a resolved hardware timestamp, and can report bogus hardware timestamps to userspace. Use skb_get_hwtstamp() instead of reading hwtstamp directly, so packet sockets follow the same hardware timestamp resolution path as the socket layer. Note that skb_get_hwtstamp() is called with cycles == false, since AF_PACKET 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> --- net/packet/af_packet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 8e6f3a734ba0..e88bc3f1156f 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -458,7 +458,7 @@ static __u32 tpacket_get_timestamp(struct sk_buff *skb, struct timespec64 *ts, if (shhwtstamps && (flags & SOF_TIMESTAMPING_RAW_HARDWARE) && - ktime_to_timespec64_cond(shhwtstamps->hwtstamp, ts)) + ktime_to_timespec64_cond(skb_get_hwtstamp(skb, false, NULL), ts)) return TP_STATUS_TS_RAW_HARDWARE; if ((flags & SOF_TIMESTAMPING_SOFTWARE) && -- 2.53.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net v1 3/3] tcp: use skb_get_hwtstamp() for hardware timestamps 2026-04-29 9:16 [PATCH net v1 0/3] af_packet/tcp: fix late hardware timestamp handling Kohei Enju 2026-04-29 9:16 ` [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb Kohei Enju 2026-04-29 9:16 ` [PATCH net v1 2/3] af_packet: use skb_get_hwtstamp() for hardware timestamps Kohei Enju @ 2026-04-29 9:16 ` Kohei Enju 2026-04-29 21:09 ` Willem de Bruijn 2 siblings, 1 reply; 11+ messages in thread From: Kohei Enju @ 2026-04-29 9:16 UTC (permalink / raw) To: netdev Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran, Kohei Enju 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. Additionally, recognize SKBTX_HW_TSTAMP_NETDEV as indicating a receive timestamp is present. 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net v1 3/3] tcp: use skb_get_hwtstamp() for hardware timestamps 2026-04-29 9:16 ` [PATCH net v1 3/3] tcp: " Kohei Enju @ 2026-04-29 21:09 ` Willem de Bruijn 2026-04-30 5:07 ` Kohei Enju 0 siblings, 1 reply; 11+ messages in thread From: Willem de Bruijn @ 2026-04-29 21:09 UTC (permalink / raw) To: Kohei Enju, netdev Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran, Kohei Enju 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v1 3/3] tcp: use skb_get_hwtstamp() for hardware timestamps 2026-04-29 21:09 ` Willem de Bruijn @ 2026-04-30 5:07 ` Kohei Enju 2026-04-30 6:09 ` Eric Dumazet 0 siblings, 1 reply; 11+ messages in thread From: Kohei Enju @ 2026-04-30 5:07 UTC (permalink / raw) To: Willem de Bruijn Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran On 04/29 17:09, Willem de Bruijn wrote: > 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? You're right. If we want to keep the door open for SOF_TIMESTAMPING_BIND_PHC on coalesced skbs, resolving the timestamp at coalescing time is the wrong approach. > > It is just as easy to coalesce the cookie as the htwtstamp. > > That also avoids the need to mask out SKBTX_HW_TSTAMP_NETDEV. Indeed. We should also carry the matching @napi_id, since skb_get_hwtstamp() uses it for late resolution. I'll update this in v2. > > > Additionally, recognize SKBTX_HW_TSTAMP_NETDEV as > > indicating a receive timestamp is present. > > What is the reason for this extra condition? With late timestamp determination, skb_shared_hwtstamps may hold netdev_data instead of a resolved hwtstamp, and 0 can be a valid cookie value. So checking only skb_hwtstamps(skb)->hwtstamp is not enough to detect the presence of an RX hardware timestamp. > > > 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> > > --- [...] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v1 3/3] tcp: use skb_get_hwtstamp() for hardware timestamps 2026-04-30 5:07 ` Kohei Enju @ 2026-04-30 6:09 ` Eric Dumazet 2026-04-30 6:52 ` Kohei Enju 0 siblings, 1 reply; 11+ messages in thread From: Eric Dumazet @ 2026-04-30 6:09 UTC (permalink / raw) To: Kohei Enju Cc: Willem de Bruijn, netdev, David S. Miller, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran On Wed, Apr 29, 2026 at 10:07 PM Kohei Enju <kohei@enjuk.jp> wrote: > > On 04/29 17:09, Willem de Bruijn wrote: > > 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? > > You're right. If we want to keep the door open for > SOF_TIMESTAMPING_BIND_PHC on coalesced skbs, resolving the timestamp at > coalescing time is the wrong approach. > > > > > It is just as easy to coalesce the cookie as the htwtstamp. > > > > That also avoids the need to mask out SKBTX_HW_TSTAMP_NETDEV. > > Indeed. We should also carry the matching @napi_id, since > skb_get_hwtstamp() uses it for late resolution. > > I'll update this in v2. Do you plan adding a selftest ? I am confused by your series, a test would really help clarify the issue at hand. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v1 3/3] tcp: use skb_get_hwtstamp() for hardware timestamps 2026-04-30 6:09 ` Eric Dumazet @ 2026-04-30 6:52 ` Kohei Enju 0 siblings, 0 replies; 11+ messages in thread From: Kohei Enju @ 2026-04-30 6:52 UTC (permalink / raw) To: Eric Dumazet Cc: Willem de Bruijn, netdev, David S. Miller, Jakub Kicinski, Paolo Abeni, Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern, Neal Cardwell, Gerhard Engleder, Jonathan Lemon, Richard Cochran On 04/29 23:09, Eric Dumazet wrote: > On Wed, Apr 29, 2026 at 10:07 PM Kohei Enju <kohei@enjuk.jp> wrote: > > > > On 04/29 17:09, Willem de Bruijn wrote: > > > 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? > > > > You're right. If we want to keep the door open for > > SOF_TIMESTAMPING_BIND_PHC on coalesced skbs, resolving the timestamp at > > coalescing time is the wrong approach. > > > > > > > > It is just as easy to coalesce the cookie as the htwtstamp. > > > > > > That also avoids the need to mask out SKBTX_HW_TSTAMP_NETDEV. > > > > Indeed. We should also carry the matching @napi_id, since > > skb_get_hwtstamp() uses it for late resolution. > > > > I'll update this in v2. > > Do you plan adding a selftest ? > > I am confused by your series, a test would really help clarify the > issue at hand. Yes, I agree a selftest would help. Since this issue is only triggered with drivers that use late RX timestamp determination (SKBTX_HW_TSTAMP_NETDEV / ndo_get_tstamp), it would likely need to live under tools/testing/selftests/drivers/net/hw/ rather than as a generic net selftest. I'm looking into whether I can add such a test. Either way, I'll expand the cover letter to explain the issue in more detail. Sorry for the confusion. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-05-01 20:47 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-29 9:16 [PATCH net v1 0/3] af_packet/tcp: fix late hardware timestamp handling Kohei Enju 2026-04-29 9:16 ` [PATCH net v1 1/3] net: introduce helper to resolve hardware timestamps from skb Kohei Enju 2026-04-29 21:04 ` Willem de Bruijn 2026-04-30 4:50 ` Kohei Enju 2026-05-01 20:25 ` Gerhard Engleder 2026-04-29 9:16 ` [PATCH net v1 2/3] af_packet: use skb_get_hwtstamp() for hardware timestamps Kohei Enju 2026-04-29 9:16 ` [PATCH net v1 3/3] tcp: " Kohei Enju 2026-04-29 21:09 ` Willem de Bruijn 2026-04-30 5:07 ` Kohei Enju 2026-04-30 6:09 ` Eric Dumazet 2026-04-30 6:52 ` Kohei Enju
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox