Add support for ptp_kvm to coreos-platform-chrony system-generator

[karelvanhecke]

This PR will allow CoreOS to make use of ptp_kvm when the underlying hypervisor supports it.

[jlebon]

Thank you for opening this! I'm wondering though if we should do this by default on QEMU. Unlike on the other cloud platforms supported there, there's no guarantee that the host hypervisor clock is itself accurate. Maybe instead we should just document it in this section and leave it up to the user? I could also imagine Butane sugar for this to make it easier.

[jlebon]

Some past discussions about ptp_kvm in https://bugzilla.redhat.com/show_bug.cgi?id=1780165#c10 and following. Quoting:

I don't think the ptp_kvm module is guaranteed to work. It's currently specific to the x86 arch and it works only if the host is using the tsc clocksource. I'm not sure how exactly it works in OpenStack.

There are few downsides to using the PHC refclock for guest synchronization. One is that it relies on the host to be properly synchronized. If you manage both, that's ok. There is also an issue that the NTP error estimates (root delay and root dispersion) are lost in the NTP->PHC->refclock conversion, so the guests underestimate the maximum error of their clock, which might be a problem in some applications.

[karelvanhecke]

I don't think the ptp_kvm module is guaranteed to work. It's currently specific to the x86 arch and it works only if the host is using the tsc clocksource. I'm not sure how exactly it works in OpenStack.

It is true that ptp_kvm isn't guaranteed to work, though the module would simply fail to load and we can fall back to the default Chrony configuration. For example, deploying the image in a nested virtualization environment triggers this behavior.
Support has also improved since that discussion. If i'm not mistaken, arm/arm64 support for ptp_kvm has been implemented in the Linux kernel.

There are few downsides to using the PHC refclock for guest synchronization. One is that it relies on the host to be properly synchronized. If you manage both, that's ok.

A similar point could be made about the default Chrony behavior, which by default relies on the accuracy & availability of the NTP pool project.

There is also an issue that the NTP error estimates (root delay and root dispersion) are lost in the NTP->PHC->refclock conversion, so the guests underestimate the maximum error of their clock, which might be a problem in some applications.

The image shipped for Azure would suffer from a similar issue:

https://learn.microsoft.com/en-us/azure/virtual-machines/linux/time-sync

Azure hosts are synchronized to internal Microsoft time servers that take their time from Microsoft-owned Stratum 1 devices, with GPS antennas. Virtual machines in Azure can either depend on their host to pass the accurate time (host time) on to the VM or the VM can directly get time from a time server, or a combination of both.

Stratum information isn't automatically conveyed from the Azure host to the Linux guest.

Out of all these possible issues, the fact that we can't guarantee the accuracy of the hypervisor might be the biggest blocking factor. What if we deploy ptp_kvm + NTP pool servers combined? Chrony would detect the ptp_kvm clock as a falseticker when the hypervisor is incorrectly synchronized.

[jlebon]

A similar point could be made about the default Chrony behavior, which by default relies on the accuracy & availability of the NTP pool project.

That's true, but IIUC there's no reason to think that the hypervisor clock has a higher chance of being accurate. The user may know it's the case for their particular environment, so it makes sense to let them opt into it.

What if we deploy ptp_kvm + NTP pool servers combined? Chrony would detect the ptp_kvm clock as a falseticker when the hypervisor is incorrectly synchronized.

That's interesting. To make sure I understand, what would be the advantage of this over only relying on the default NTP pool?

[karelvanhecke]

That's interesting. To make sure I understand, what would be the advantage of this over only relying on the default NTP pool?

If the ptp_kvm source is deemed accurate, It'll keep polling all sources, but only use the results of ptp_kvm (from what I've seen when I tested this). All CoreOS instances on that virtualization platform will be synchronized with sub-millisecond accuracy with each other.

[cgwalters]

Thanks so much for submitting this patch!

My PoV is by default we should trust the hypervisor. Anyone who doesn't want to would need to configure things.

[jlebon]

That's interesting. To make sure I understand, what would be the advantage of this over only relying on the default NTP pool?

If the ptp_kvm source is deemed accurate, It'll keep polling all sources, but only use the results of ptp_kvm (from what I've seen when I tested this). All CoreOS instances on that virtualization platform will be synchronized with sub-millisecond accuracy with each other.

Thanks, that sounds reasonable.

My PoV is by default we should trust the hypervisor. Anyone who doesn't want to would need to configure things.

Are you talking about "trust" from a security perspective or as in "trust that the hypervisor clock is accurate"? I agree with the former but the latter is not clear to me. Though @karelvanhecke's latest suggestion is an interesting middle ground.

Interested to see what others think. Maybe we can discuss this in the next community meeting. I'll create a tracker ticket for it so it shows up in the filter (edit: filed coreos/fedora-coreos-tracker#1433).

@karelvanhecke BTW, do you have more information on the context around this? Is this fixing an issue for you?

[jlebon]

@karelvanhecke We discussed this today in the community meeting (coreos/fedora-coreos-tracker#1433 (comment)). Would you be able to update this PR to implement:

What if we deploy ptp_kvm + NTP pool servers combined? Chrony would detect the ptp_kvm clock as a falseticker when the hypervisor is incorrectly synchronized.

?

[karelvanhecke]

@jlebon rebased against latest testing-devel branch + enabled ntp pool in combination with ptp_kvm.

[jlebon]

Thanks! LGTM, but I'd like to manually test this before merging to sanity-check the falseticker case. Will try to do that next week.

[jlebon]

I've verified this works as expected. Output of chronyc sources when the host clock is synchronized:

[root@cosa-devsh ~]# chronyc sources
MS Name/IP address         Stratum Poll Reach LastRx Last sample
===============================================================================
#* PHC0                          0   2   377     2  +8193ns[  -41us] +/-   34us
^- up2.com                       2   6   377    14   -413us[ -579us] +/-   70ms
^- 68-69-221-61.regn.hsdb.s>     1   6   377    13  -1281us[-1446us] +/-   18ms
^- ntp.nyy.ca                    1   6   377    13   +500us[ +334us] +/-   22ms
^- time.cloudflare.com           3   6   377    15   -532us[ -698us] +/-   16ms

The * means the source is selected for synchronization.

Output of chronyc sources when the host clock is inconsistent:

[root@cosa-devsh ~]# chronyc sources
MS Name/IP address         Stratum Poll Reach LastRx Last sample
===============================================================================
#x PHC0                          0   2   377     4  +91106h[+91106h] +/-   69us
^- up2.com                       2   6    17    35   -817us[ -817us] +/-   92ms
^* 68-69-221-61.regn.hsdb.s>     1   6    17    35   -169us[ -389us] +/-   19ms
^+ ntp.nyy.ca                    1   6    17    35   -306us[ -526us] +/-   20ms
^+ time.cloudflare.com           3   6    17    35   -218us[ -438us] +/-   17ms

The x means the source is flagged as a falseticker.

There's something I want to call out related to an additional change I pushed here. The ext.config.ntp.chrony.dhcp-propagation test was failing because ptp_kvm is supported in our CI builders, and so the chrony script neuters DHCP-provided NTP servers. This is consistent with other platforms which support platform-native clocks (AWS, Azure, GCP), but on QEMU something other than the default DHCP server could be used, and users may have relied on their own configured DHCP servers to pass in NTP servers. Now, those will be ignored. It's still possible to pick up DHCP-provided NTP servers, but would now require explicit configuration (as shown in the changes I made to the test).

[jlebon]

Updated! I've added more context to the commit message and removed the changes to the ext.config.ntp.chrony.dhcp-propagation test since they're no longer necessary.

[dustymabe]

LGTM