Re: [PATCH bpf-next v8 05/12] net-timestamp: prepare for isolating two modes of SO_TIMESTAMPING

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Jason Xing wrote:
> On Wed, Feb 5, 2025 at 11:34 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> 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...

A NULL pointer exception is easy to find.

It's not a hardware bug, but a driver bug. Given the small number of
drivers implementing this API, it could even be found through code
inspection.

As a general rule of thumb we don't add protection mechanisms to paper
over bugs elsewhere in the kernel. But detect and fix the bugs. An
exception to the general rule is when buggy code is hard to find. That
is not the case here.