Add support for additional architectures in ntp.

[cy]

Summary

This patch adds support for additional architectures in ntp.

[imp]

I wonder why you need these #defines and can't use sizeof(type) instead...

usr.sbin/ntp/config.h

1472 ↗

(On Diff #5901)

#ifdef LP64

covers this.

1482 ↗

(On Diff #5901)

Same as above.

1501 ↗

(On Diff #5901)

I think the more proper thing here is #if !defined(i386) && !defined(powerpc)

[andrew]

usr.sbin/ntp/config.h

1545 ↗

(On Diff #5901)

I think you mean __aarch64__

[kib]

And what about SIZEOF_PTHREAD_T ?
Similarly CHAR_UNSIGNED, which should be true for ppc* and probably arms.

IMO, you cannot use single config.h on different arches without loosing the sanity. Better to generate per-arch config.<arch>.h on the native machine and check that in.

Did you looked how similar MD bits are handled for config.h's from e.g. gnu/* ?

[roberto]

To all: do not forget that originally this file is automatically generated during configure. We are able to modify it because we are not using configure in the freebsd context but keep in mind that this change will have to be merged again during the next update.

As ntp seems to be back evolving regularely, maybe clean up the changes as expected in config.h.in (focus on type sizes instead of just list of architecture-related symbols) and submit upstream? The undermydesk changes I could not care less though :)

[jmg]

usr.sbin/ntp/config.h

1472 ↗

(On Diff #5901)

_LP64 or

__LP64__

from cc -E -dM - < /dev/null

1501 ↗

(On Diff #5901)

or swap the orders and make it (reads easier to me):
#if defined(i386) || defined(powerpc)
4
#else
8
#endif

[cy]

Roberto@, I agree. This should be submitted upline.

usr.sbin/ntp/config.h

1501 ↗

(On Diff #5901)

Wouldn't #ifdef LP64 work here too?

[cy]

Updated diff attached.

[cy]

Correctly handle time_t with arm too.

[imp]

usr.sbin/ntp/config.h

1501 ↗

(On Diff #5943)

My code was correct, but this ifdef is wrong.
All mips, no matter the flavor is 8 byte.
i386 and powerpc are the only two that are still 4 byte.
We don't follow LP64-ness here at all.

So no, LP64 isn't appropriate here.

[cy]

It's been corrected. Thoughts?

[kib]

So you ignored my feedback.

[imp]

In D2720#52014, @cy wrote:

It's been corrected. Thoughts?

It hasn't been corrected. time_t is still wrong.

[imp]

In D2720#51478, @kib wrote:

And what about SIZEOF_PTHREAD_T ?
Similarly CHAR_UNSIGNED, which should be true for ppc* and probably arms

These suggestions are not implemented, but need to be. This is required for things to work properly on these systems.

IMO, you cannot use single config.h on different arches without loosing the sanity. Better to generate per-arch config.<arch>.h on the native machine and check that in.

Did you looked how similar MD bits are handled for config.h's from e.g. gnu/* ?

These are good suggestions, but are optional. I agree with them, but what you've done here will also work, even if it is a less optimal solution.

[• ian]

This diff lacks full context, making it difficult to comment on things that were not changed that should have been.

I think these changes are also needed. There may be a better way to do the big-endian detection.

 /* Default location of crypto key info */
-#define NTP_KEYSDIR "/usr/local/etc"
+#define NTP_KEYSDIR "/etc/ntp"

/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most
    significant byte first (like Motorola and SPARC, unlike Intel). */
-#if defined AC_APPLE_UNIVERSAL_BUILD
-# if defined __BIG_ENDIAN__
-#  define WORDS_BIGENDIAN 1
-# endif
-#else
-# ifndef WORDS_BIGENDIAN
-/* #  undef WORDS_BIGENDIAN */
-# endif
+#if defined(__ARMEB__) || defined(__MIPSEB__) || defined(__powerpc__) || \
+    defined(__powerpc64__) || defined(__sparc64__)
+#define WORDS_BIGENDIAN 1
 #endif
 
 /* routine worker child proc uses to exit. */

[cy]

Agreed. I've been pondering kib's comment for a while now. Interesting. I think this is more of a long term target, requiring detailed documentation (probably written by me) on which files to generate and how to. The more we can automate the better. Then it would be a matter of getting the target names correct.

I'll upload the latest diff now. At least people can have a look at it while I'm out today. I'll have time to work on it tonight.

[cy]

This is the full file with diff.

[cy]

After having considered kib's suggestion, I'd like to submit his idea.

[imp]

config.h

1624

Don't you think you'd be happier with an #endif here?
:)

[• ian]

Looks good. I've tested this on arm (after adding the missing #endif), and discovered that it works properly on armv4/5 systems, but ntpd still fails to keep time properly on armv6.

At this point I'm beginning to suspect the timekeeping problem is either an armv6 bug (like some floating point support error), or it's an ntpd bug that's harmless on other systems but triggers on armv6 (there's one of those in the old ntpd due to an unitialized var that's harmless on x86 and only "mostly harmless" on arm).

config.h

1623

missing #endif here

[andrew]

Was there anything holding this up now? There have been reports this fixes ntp on MIPS.

[• lidl_pix.net]

I just tested this patch on a MIPS64 machine (ERL), running r284544M.

Without this patch, ntpdate reports a large offset when starting, but
the clock is left at 1970. With this patch, ntpdate reports a large
offset when starting, and the date is properly set when the machine
goes multi-user.

[cy]

I like kib's suggestion. Unfortunately qemu-sbruno with poudriere doesn't support all architectures yet. What if we commit the proposed config for now with the long fee goal of one config file per architecture?

[kib]

In D2720#55992, @cy wrote:

I like kib's suggestion. Unfortunately qemu-sbruno with poudriere doesn't support all architectures yet. What if we commit the proposed config for now with the long fee goal of one config file per architecture?

I think something must be done now. If there is nothing better than committing the existing patch, so be it.

But, IMO, you could already provide the per-arch config.h. For some arches, where you have access to the machine and can generate the genuine config.h, it would be the real file. For other arches, it could be hand-edited. If people pop up who complain about non-working ntpd on such arches, that means they have both machine and interest, so they should be able to follow your instructions to provide you with the right config.h for commit.

[cy]

Makes sense. Not all arches are supported by qemu-sbruno. I'll include what I can gen a config file for. In the mean time I'll include a new config.h until I gen arch specific config.h files.

[cy]

Makes sense. Not all arches are supported by qemu-sbruno. I'll include what I can gen a config file for. In the mean time I'll include a new config.h until I gen arch specific config.h files.

[cy]

This should cover it.