> -----Original Message----- > From: Petr Machata [mailto:pe...@mellanox.com] > Sent: Monday, June 10, 2019 9:42 AM > To: Keller, Jacob E <jacob.e.kel...@intel.com> > Cc: linuxptp-devel@lists.sourceforge.net > Subject: Re: [PATCH] sk: Recognize HWTSTAMP_FILTER_SOME > > > Keller, Jacob E <jacob.e.kel...@intel.com> writes: > > >> index 93ba77a..416d784 100644 > >> --- a/sk.c > >> +++ b/sk.c > >> @@ -105,7 +105,8 @@ static int hwts_init(int fd, const char *device, int > >> rx_filter, > >> if (cfg.tx_type != tx_type || > >> (cfg.rx_filter != rx_filter && > >> cfg.rx_filter != rx_filter2 && > >> - cfg.rx_filter != HWTSTAMP_FILTER_ALL)) { > >> + cfg.rx_filter != HWTSTAMP_FILTER_ALL && > >> + cfg.rx_filter != HWTSTAMP_FILTER_SOME)) { > >> pr_debug("tx_type %d not %d", cfg.tx_type, tx_type); > >> pr_debug("rx_filter %d not %d or %d", cfg.rx_filter, rx_filter, > >> rx_filter2); > > > > You can't accept HWTSTAMP_FILTER_SOME from the "get hwtstamp config" > > ioctl, because it's unclear what packets "SOME" actually refers to. > > I.e. one caller could set it to filter V1 packets, the driver could > > report SOME, and then ptp4l could "accept" this filter even though > > it's not valid. > > You are right, I missed the FILTER_CHECK scenario in the same function. > I'll try to come up with a way to encode the check that's not three > layers of and-or nesting. > > (If the goal of the patch is generally acceptable.) > > > You can accept it in response to the "set hwtstamp config", I suppose, > > but it is a tad confusing. I never really liked that return value, and > > just wish the API had been "you may timestamp more than requested > > without reporting it if there is no valid filter matching what you > > timestamp". > > It looks like there's currently only one driver doing this in the > vanilla kernel, Intel e1000e. I'm writing PTP support for Mellanox > Spectrum switches, and the HW can be configured to timestamp by PTP > message type, but not by layer or PTP version. I think SOME is exactly > appropriate here. > > Having a major PTP stack on Linux reject the configuration is something > of a bummer though, even if it should be fixed in the next version, so I > might reconsider. But it should be fixed, the interface is what it is.
Right. We should accept it, as it is documented. Thanks, Jake > > Thanks, > Petr _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
