lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 14 Apr 2020 11:46:49 +0200
From:   Julien Beraud <julien.beraud@...lia.com>
To:     Jose Abreu <Jose.Abreu@...opsys.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 2/2] net: stmmac: Fix sub-second increment

Jose

On 14/04/2020 11:15, Jose Abreu wrote:
> From: Julien Beraud <julien.beraud@...lia.com>
> Date: Apr/14/2020, 10:10:03 (UTC+00:00)
> 
>> This will also
>> work for frequencies below 50MHz (for instance using the internal osc1
>> clock at 25MHz for socfpga platforms).
> 
> No please. Per HW specifications the minimum value for PTP reference clock
> is 50MHz. If this does not work in your setup you'll need to add some kind
> of quirk to stmmac instead of changing the existing logic which works fine
> in all other setups.

The numbers I have in the documentation say that the minimum clock 
frequency for PTP is determined by "3 * PTP clock period + 4 * GMII/MII 
clock period <= Minimum gap between two SFDs". The example values say 
5MHz for 1000-Mbps Full Duplex. Is this documentation incorrect ?

If this clock is really limited to 50MHz minimum, we could explicitely 
forbid values under 50MHz but silently making the ptp clock not 
increment doesn't look right.

Apart from that, the existing logic doesn't work. The calculation is off 
by a factor 2, making the ptp clock increment twice slower as it should, 
at least on socfpga platform but I expect that it is the same on other 
platforms. Please check commit 19d857c, which has kind of been reverted 
since for more explanation on the sub-seconds + addend calculation.
Also, it artificially sets the increment to a value while we should 
allow it to be as small as posible for higher frequencies, in order to 
gain accuracy in timestamping.

Regards,
Julien

Powered by blists - more mailing lists