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.
Thanks,
Petr
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
