RE: [RFC PATCH 0/7] Extend socket timestamping API

["Keller, Jacob E" <jacob.e.keller@intel.com>]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Miroslav Lichvar
> Sent: Thursday, April 13, 2017 2:54 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>;
> Willem de Bruijn <willemb@google.com>; Soheil Hassas Yeganeh
> <soheil@google.com>; Denny Page <dennypage@me.com>; Jiri Benc
> <jbenc@redhat.com>
> Subject: Re: [RFC PATCH 0/7] Extend socket timestamping API
> 
> On Thu, Apr 13, 2017 at 09:08:08AM +0000, Keller, Jacob E wrote:
> > > -----Original Message-----
> > > This patchset adds new options to the timestamping API that will be
> > > useful for NTP implementations and possibly other applications.
> 
> > I think this looks pretty good, straight forward and useful. I agree with Richard
> about the one commit message, but the rest looks good modulo missing updates
> for all the other drivers.
> 
> Thanks for the review.
> 
> > I didn't see any code to update the ethtool get_ts_info data for _NTP_ALL
> either.
> 
> My understanding was that ethtool should list only filters that are
> actually supported by the HW and are not handled just by switching to
> a more general filter. I think that's what drivers I've looked were
> doing. The phyter driver would be the only one that could list the
> filter as supported. Is that correct?
> 
> --
> Miroslav Lichvar

I'm not sure whether that's how all drivers are implemented, or whether that makes sense. Suppose an application wants to know if a particular filter is supported, they might ask using get_ts_info, but now the application must encode the rules for which filters are more general. I suppose this logic isn't all that complicated and is straight forward deductions based on the name and generally we prefer if driver or hardware supports timestamping all frames anyways.

In this regard I guess you'd be right.

Thanks,
Jake