On Tue, Feb 4, 2025 at 7:14 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 1/28/25 12:46 AM, Jason Xing wrote:
> > No functional changes here. I add skb_enable_app_tstamp() to test
> > if the orig_skb matches the usage of application SO_TIMESTAMPING
> > and skb_sw_tstamp_tx() to distinguish the software and hardware
>
> There is no skb_sw_tstamp_tx() in the code. An outdated commit message?
Thanks. I'll update it and double check before reposting.
>
> > timestamp when tsflag is SCM_TSTAMP_SND.
> >
> > Also, I deliberately distinguish the 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.
> >
> > After this patch, I will soon add checks about bpf SO_TIMESTAMPING.
>
> This needs to be updated also. BPF does not use the SO_TIMESTAMPING socket option.
>
> > In this way, we can support two modes parallelly.
>
> s/parallely/in parallel/
Will fix it.
>
> >
> > 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
> > * @orig_skb: the original outgoing packet
> > * @hwtstamps: hardware time stamps, may be NULL if not available
> > *
> > * If the skb has a socket associated, then this function clones the
> > * skb (thus sharing the actual data and optional structures), stores
> > - * the optional hardware time stamping information (if non NULL) or
> > - * generates a software time stamp (otherwise), then queues the clone
>
> This line is removed. Does it mean no software timestamp now after this change?
Right, _software_ timestamp will enter skb_tx_timestamp() then call
__skb_tstamp_tx() instead of this skb_tx_timestamp().
>
> > - * to the error queue of the socket. Errors are silently ignored.
> > + * the optional hardware time stamping information (if non NULL) then
> > + * queues the clone to the error queue of the socket. Errors are
> > + * silently ignored.
> > */
> > void skb_tstamp_tx(struct sk_buff *orig_skb,
> > struct skb_shared_hwtstamps *hwtstamps);
> > @@ -4565,7 +4566,7 @@ static inline void skb_tx_timestamp(struct sk_buff *skb)
> > {
> > skb_clone_tx_timestamp(skb);
> > if (skb_shinfo(skb)->tx_flags & SKBTX_SW_TSTAMP)
> > - skb_tstamp_tx(skb, NULL);
> > + __skb_tstamp_tx(skb, NULL, NULL, skb->sk, true, SCM_TSTAMP_SND);
> > }
>
>
