On Wed, Feb 5, 2025 at 11:34 PM Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote: > > Jason Xing wrote: > > No functional changes here, only add skb_enable_app_tstamp() to test > > if the orig_skb matches the usage of application SO_TIMESTAMPING > > or its bpf extension. And it's good to support two modes in > > parallel later in this series. > > > > Also, this patch deliberately distinguish the software and > > hardware SCM_TSTAMP_SND timestamp by passing 'sw' parameter in order > > to avoid such a case where hardware may go wrong and pass a NULL > > hwstamps, which is even though unlikely to happen. If it really > > happens, bpf prog will finally consider it as a software timestamp. > > It will be hardly recognized. Let's make the timestamping part > > more robust. > > Disagree. Don't add a crutch that has not shown to be necessary for > all this time. > > Just infer hw from hwtstamps != NULL. I can surely modify this part as you said, but may I ask why? I cannot find a good reason to absolutely trust the hardware behaviour. If that corner case happens, it would be very hard to trace the root cause... > > > Signed-off-by: Jason Xing <kerneljasonxing@xxxxxxxxx> > > --- > > include/linux/skbuff.h | 13 +++++++------ > > net/core/dev.c | 2 +- > > net/core/skbuff.c | 32 ++++++++++++++++++++++++++++++-- > > net/ipv4/tcp_input.c | 3 ++- > > 4 files changed, 40 insertions(+), 10 deletions(-) > > > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > > index bb2b751d274a..dfc419281cc9 100644 > > --- a/include/linux/skbuff.h > > +++ b/include/linux/skbuff.h > > @@ -39,6 +39,7 @@ > > #include <net/net_debug.h> > > #include <net/dropreason-core.h> > > #include <net/netmem.h> > > +#include <uapi/linux/errqueue.h> > > > > /** > > * DOC: skb checksums > > @@ -4533,18 +4534,18 @@ void skb_complete_tx_timestamp(struct sk_buff *skb, > > > > void __skb_tstamp_tx(struct sk_buff *orig_skb, const struct sk_buff *ack_skb, > > struct skb_shared_hwtstamps *hwtstamps, > > - struct sock *sk, int tstype); > > + struct sock *sk, bool sw, int tstype); > > > > /** > > - * skb_tstamp_tx - queue clone of skb with send time stamps > > + * skb_tstamp_tx - queue clone of skb with send HARDWARE timestamps > > Unfortunately this cannot be modified to skb_tstamp_tx_hw, as that > would require updating way too many callers. I didn't change the name, only the description and usage of skb_tstamp_tx(). It always gets called in the hardware timestamp situation except skb_tx_timestamp() that is modified. Thanks, Jason
