Did blame.
commit afeabf3c90edf6699d7e0d058593835ec258be46
Author: Hangbin Liu <liuhang...@gmail.com>
Date: Wed May 25 14:46:16 2022 +0800
ptp4l: add VLAN over bond support
Perhaps Hangbin Liu can assist?
Erez
On Tue, 2 May 2023 at 00:08, Erez <erezge...@gmail.com> wrote:
>
>
> On Mon, 1 May 2023 at 18:30, Martin Pecka <pecka...@fel.cvut.cz> wrote:
>
>>
>> This code in hwts_init is wrong:
>>
>> cfg.flags = HWTSTAMP_FLAG_BONDED_PHC_INDEX;
>> /* Fall back without flag if user run new build on old kernel */
>> if (ioctl(fd, SIOCGHWTSTAMP, &ifreq) == -EINVAL)
>> init_ifreq(&ifreq, &cfg, device);
>>
>> As `man ioctl` says:
>>
>> RETURN VALUE
>> Usually, on success zero is returned. A few ioctl() requests use
>> the
>> return value as an output parameter and return a nonnegative value
>> on
>> success. On error, -1 is returned, and errno is set appropriately.
>>
>> So I think what you meant to write is this:
>>
>> cfg.flags = HWTSTAMP_FLAG_BONDED_PHC_INDEX;
>> err = ioctl(fd, SIOCGHWTSTAMP, &ifreq);
>> if (err < 0) {
>> /* Fall back without flag if user run new build on old kernel */
>> if (errno == EINVAL) {
>> init_ifreq(&ifreq, &cfg, device);
>> } else {
>> pr_err("ioctl SIOCGHWTSTAMP failed: %m");
>> return err;
>> }
>> }
>>
>>
>> Agree. Applying this fix reveals the real error - EOPNOTSUPP.
>>
>> @Martin would that also fix your issue?
>>
>> No. Getting the real error code, I went after the source of EOPNOTSUPP,
>> and...
>>
>> $ sudo hwstamp_ctl -i eth0
>> Device driver does not have support for non-destructive SIOCGHWTSTAMP.
>>
>> This is it! The nvidia eqos driver does not support SIOCGHWTSTAMP (but it
>> does support SIOCSHWTSTAMP). Evidence can be found in
>> https://github.com/OE4T/linux-tegra-4.9/blob/oe4t-patches-l4t-r32.7.3/nvidia/drivers/net/ethernet/nvidia/eqos/drv.c#L3979
>> (function eqos_ioctl).
>>
>> According to
>> https://www.kernel.org/doc/Documentation/networking/timestamping.txt,
>> section 3.1:
>>
>> A driver which supports hardware time stamping must support the
>> SIOCSHWTSTAMP ioctl and update the supplied struct hwtstamp_config with
>> the actual values as described in the section on SIOCSHWTSTAMP. It
>> should also support SIOCGHWTSTAMP.
>>
>> The support for SIOCGHWTSTAMP is optional.
>>
>> So it seems to me wrong to test for the bonded PHC support using this
>> ioctl, which is only optional.
>>
>> To add importance to this issue, I've found out it not only affects the
>> pretty old Jetson TX2 I used for the test, but all Jetson models up to the
>> newest AGX Orin (included). Meaning no Jetson can run ptp4l from master
>> branch now. I really wonder I'm the first one complaining...
>>
>> Regarding possible solutions I can think of:
>>
>> 1. Check errno not only for EINVAL, but also for EOPNOTSUPP. This would
>> solve the issue for me (and Jetsons in general), but would probably leave
>> some users with bonded PHCs whose drivers do not support SIOCGHWTSTAMP
>> without the possibility to use the bonded PHC.
>>
>
> On a second review.
> I think you have a point.
>
> This get ioctl (SIOCGHWTSTAMP) was added for VLAN over bond support.
>
> On normal run, the get ioctl is only used with HWTS_FILTER_CHECK.
> So the user can switch to HWTS_FILTER_FULL or HWTS_FILTER_NORMAL and skip
> the get ioctl.
>
> But the get ioctl with bonded PHC index, can not be skipped.
>
>
>> 2. Find another way to test for the bonded PHC feature.
>>
> Perhaps a flag?
>
> Erez
>
>
>> Martin
>>
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
