On Wed, 18 Dec 2024, Hal Murray wrote:
Fred Wright said:
I'm always in favor of maximum compatibility.
There is a tradeoff between broader compatibility and clutter in the code.
Indeed, though a good design keeps aspects dependent on the OS or other
outside packages well-segregated.
(though OpenBSD requires the same patches as I use for older OSX
versions).
How big is the patch for OSX? I think that's much more interesting that
ancient versions of OpenSSL. Can we set it up with all the extra code in
a compatibility module that just implements ntp_adjtime() and only gets
linked in if needed?
The changes break down into four categories. In roughly chronological
order by introduction:
1) Fallbacks for clock_gettime() and clock_settime().
2) Restoration of the optionality of timex and KERNEL_PLL.
3) Various warning fixes.
4) Packet timestamp fixes for OSX 10.5 x86_64.
For #1, the issue arose when Eric was trying to implement some kind of
fallback, apparently in some strange way, when he got frustrated by
mismatches between documentation and actual behavior, threw in the towel,
and declared macOS <10.12 to be unsupported. This is bizarre, since
fallbacks for those functions are quite straightforward, and in fact gpsd
has had clock_gettime() fallbacks for ages. In this case, I did them as
inline functions residing entirely in ntp_machine.h. The only ongoing
complication is that every time you add a new attic program that uses
clock_gettime(), I have to add the ntp_machine include to unbreak it.
For #2, the issue arose when Eric added two commits to make those features
mandatory, initially described as a trial balloon, but left in place.
Since macOS doesn't provide those capabilities until 10.13, it further
raised the bar on OS versions. My fix was simply to revert the two
commits, though resolving related conflicts is an ongoing problem. As is,
these patches are clutter on steroids, so I don't think you'd want them.
In theory, the code could probably be refactored to better localize that
dependence, but I've never done that for a couple of reasons:
1) At any given point, fixing the conflicts is less work than refactoring
the code.
2) I think this kind of incestuous relationship between ntpd and the
kernel is a bad idea, anyway. A better approach would be to create a
timescale entirely in userspace that represents the best estimate of the
correct time, and then separately and optionally steer the system time to
match. The only kernel support needed for a userspace timescale is
CLOCK_MONOTONIC_RAW or equivalent.
For #3, warnings pop up at various times and usually have simple fixes,
though since they're only warnings, the fixes aren't mandatory. In
general, they actually relate to the compiler version rather than the OS
version, though there tends to be coupling between the two.
For #4, I only got around to tracking that one down several months ago.
The symptom was that everything would build and pass all the tests, but
ntpd didn't actually work. It turned out to be due to two separate issues
related to packet timestamps, which were harder to track down than they
should have been, since dropping packets due to bad timestamps doesn't
even produce a debug message, let alone a log message. The bugs were:
1) A bad definition of CMSG_DATA in the OSX 10.5 headers resulted in a bad
payload pointer in 64-bit builds.
2) If the bitness of the kernel's time_t doesn't match userspace's, then
packet timestamps may not have the expected format, resulting in garbling.
The fix is to pay attention to the cmsg length to see which kind of
timestamp is being provided. Although this problem has only been observed
in an old OSX case, it's not conceptually Mac-specific, and I just
submitted MR !1414 with the fix (a cleaned up version of my original).
The main reason I haven't bothered submitting any of this (except as
noted) is that the actual range of supported macOS versions wouldn't be
increased without #2, which I'm pretty sure is unacceptable as is. Though
I guess getting some acceptable changes incorporated would at least reduce
the size of the patches I have to apply in MacPorts. That might be worth
looking at after the release.
Fred Wright
_______________________________________________
devel mailing list
devel@ntpsec.org
https://lists.ntpsec.org/mailman/listinfo/devel
