PTP Clock Rate Adjustment Does Not Validate Acceptable Rate Bounds #93804
Description
Describe the bug
The method void ptp_clock_synchronize(uint64_t ingress, uint64_t egress) in subsys/net/lib/ptp/clock.c uses the driver-specific API call to ptp_clock_rate_adjust without checking for the acceptable range of the PTP clock rate adjustment.
It can therefore occur that ptp_clock_rate_adjust returns -EINVAL (e.g., drivers/ptp_clock/ptp_clock_nxp_enet.c:ptp_clock_nxp_enet_rate_adjust) without adjusting the clock rate.
Regression
- This is a regression.
Steps to reproduce
A minimal example to reproduce the error would be to call ptp_clock_synchronize directly, e.g., via
#include <clock.h>
int main(void) {
ptp_clock_synchronize(0, 999999999);
return 0;
}
and by including zephyr_include_directories(${ZEPHYR_BASE}/subsys/net/lib/ptp) in CMakeLists.txt.
In this case, the ptp_clock_synchronize needs to be modified slightly by commenting out this check
if (!ptp_clk.current_ds.mean_delay) {
return;
}
To reproduce the log below, I printed the return code of ptp_clock_rate_adjust via
int err = ptp_clock_rate_adjust(ptp_clk.phc, 1.0 + (ppb / 1000000000.0));
LOG_INF("Return %d", err);
Relevant log output
[00:00:03.123,000] <inf> phy_mc_ksz8081: PHY 0 is up
[00:00:03.123,000] <inf> phy_mc_ksz8081: PHY (0) Link speed 100 Mb, full duplex
[00:00:03.124,000] <inf> eth_nxp_enet_mac: Link is up
*** Booting Zephyr OS build v4.1.0-5652-gcef712f2d1e1 ***
[00:00:03.125,000] <inf> net_config: Initializing network
[00:00:03.125,000] <inf> net_config: IPv4 address: 192.0.0.1
[00:00:03.126,000] <inf> ptp_clock: Return -22Impact
Not sure
Environment
Zephyr Commit: 6d77442
Tested with ENET driver
Additional Context
The impact of this bug is difficult to assess. While hard to reproduce (that's why the above example directly calls ptp_clock_synchronize with custom ingress and egress timestamps), it's not hard to imagine a case where this could lead to time synchronization inaccuracies in the range of hundreds of milliseconds.
At the same time, the admissible rate adjustments are driver-specific (e.g., drivers/ethernet/eth_stm32_hal.c has a different range of acceptable rate adjustments than drivers/ptp_clock/ptp_clock_nxp_enet.c).