Details

Reviewers

• ian
imp
kib
rgrimes
jhb

Commits

rS352865: MFC r352304, r352540
rS352305: Add RELNOTES comment for r352304 discussing removing default mlock()
rS352304: No longer mlock() ntpd pages by default in memory thus allowing its

Summary

As per the discussion on -current@ between Sept 6 - 9, disable ntpd mlock() of pages in memory by default. As ntpd can account for possible (though unlikely) jitter, the impact to ntpd is minimal.

Additionally, disabling ntpd's use of mlock() is default on Linux.

See the discussion on freebsd-current@.

A corresponding change will be made to ports/net/ntp and ports/net/ntp-devel.

Users may enable memory locking through the rlimit memlock statement in ntp.conf.

Test Plan

Testing has shown no effect so far.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 26442

Event Timeline

cy created this revision.Sep 10 2019, 2:44 AM

Harbormaster completed remote builds in B26372: Diff 61870.Sep 10 2019, 2:45 AM

kib added inline comments.Sep 10 2019, 10:11 AM

usr.sbin/ntp/ntpd/ntp.conf
110	'will result' seems to be too strong. Might be mention stack limit and stack gap randomization. Or just say that rlimit 0 is incompatible with default settings for ASLR and enabled stack gap randomization.

rgrimes accepted this revision.Sep 10 2019, 5:23 PM

This revision is now accepted and ready to land.Sep 10 2019, 5:23 PM

Reworded the result of rlimit memlock 0 a little less harshly.

This revision now requires review to proceed.Sep 10 2019, 8:34 PM

cy marked an inline comment as done.Sep 10 2019, 8:34 PM

• ian accepted this revision.Sep 10 2019, 8:38 PM

• ian added inline comments.

usr.sbin/ntp/ntpd/ntp.conf
110	Typo in the new text: "will may result". Also, that sentence needs a period.

This revision is now accepted and ready to land.Sep 10 2019, 8:38 PM

pfft.

This revision now requires review to proceed.Sep 10 2019, 8:57 PM

oops

kib accepted this revision.Sep 11 2019, 4:44 AM

This revision is now accepted and ready to land.Sep 11 2019, 4:44 AM

Does anyone think we need UPDATING or RELNOTES entries? Probably UPDATING. Not sure about RELNOTES.

In D21581#470863, @cy wrote:

Does anyone think we need UPDATING or RELNOTES entries? Probably UPDATING. Not sure about RELNOTES.

My feedback is to do both.

Added UPDATING and RELNOTES. RELNOTES will be committed separately in order to fill in a revision number.

This revision now requires review to proceed.Sep 12 2019, 3:04 AM

UPDATING and RELNOTES added. RELNOTES will be committed after a revision number for the main commit is known.

• ian added inline comments.Sep 12 2019, 4:28 PM

UPDATING
37	I think this gives the misleading impression that locking ntpd into memory is required for high-stratum servers, and I don't believe that is true. I would prefer to see words that make that clear, something like Locking ntpd into memory is not believed to be required for proper operation. Administrators wishing to preserve the historic behavior can do so by adding 'rlimit memlock 32' to ntp.conf. See the NTP chapter of the FreeBSD handbook for more details. Then we should update the handbook with some details about all this, which is something I can do. I just now glanced at that section of the handbook, maybe for the first time in 25 years :) and it seems a bit thin to me, so I can look into updating it with some modern tips and best practices stuff.

cy added inline comments.Sep 12 2019, 7:47 PM

UPDATING
37	How about a compromise? There are differences of opinion as noted in the email thread on -current@. Rather than making a judgement that it is believed or not believed to make a difference, let's simply leave the first sentence out. ntpd no longer by default locks its pages in memory, allowing them to be paged out by the kernel. Use rlimit memlock to restore historic BSD behaviour. For example, adding "rlimit memlock 32" to ntp.conf to lock up to 32 MB of ntpd address space in memory. Does this verbiage work for you? I don't want to make a judgement in the UPGRADING statement. Agreed we should put something in the handbook. Memory locking is historic BSD behaviour. Linux hasn't locked ntpd into memory for as long as I can remember. (This change brings us more in line with Linux behaviour.)

Reworded the UPDATING comment to only describe the change and how to restore historic BSD behaviour. No recommendation is made.

I do not think a reference to the handbook should be made here. However we do need to update the handbook. I think that is where we can work on recommendations and ntp best practices.

Harbormaster completed remote builds in B26442: Diff 62002.Sep 12 2019, 7:54 PM

• ian added inline comments.Sep 12 2019, 7:58 PM