Page MenuHomeFreeBSD
Differential D56898

sysctl.conf(5): Add support for sysctl.conf.d
Needs ReviewPublic
Actions

Authored by agh_riseup.net on Sat, May 9, 7:24 AM.
Tags
None
Referenced Files
F158003043: D56898.id177521.diff
Wed, May 27, 9:27 AM
F158002422: D56898.id177521.diff
Wed, May 27, 9:18 AM
F157947707: D56898.id177491.diff
Tue, May 26, 7:33 PM
F157947320: D56898.id177491.diff
Tue, May 26, 7:24 PM
F157944241: D56898.id177519.diff
Tue, May 26, 6:23 PM
Unknown Object (File)
Tue, May 26, 6:15 PM
Unknown Object (File)
Tue, May 26, 4:44 AM
Unknown Object (File)
Sun, May 24, 8:14 PM
View All 52 Files
Subscribers
antranigv_freebsd.am
des
imp
ivy
ziaee
Src Committers
srcmgr

Details

Reviewers
des
antranigv_freebsd.am
Group Reviewers
Contributor Reviews (src)
Summary

Currently the sysctl(8) rc(8) service is limited to searching only
/etc/sysctl.conf, and /etc/sysctl.conf.local to configure kernel state
(Management Information Base (MIB)). The current limitation leads to
polluting /etc (for users' who prefer to leave /etc/ read-only), and
restricts users from codifying MIB namespace scope in sysctl.conf(5),
this prohibits a cleaner, and modular design to managing host MIB
information. Adding the ability to separate a machines MIB state across
any number of configuration files, under a user's chosen namespace
overcomes this limitation

  • Add /etc/sysctl.conf.d to the sysctl(8) rc(8) service
  • Add /etc/sysctl.conf.d to ${SRC}/etc/mtree/BSD.root.dist to tag the directory for the pkgbase FreeBSD-rc package
  • Update stock sysctl.conf referring to /etc/sysctl.conf.d
  • Update the sysctl.conf(5) man page to reflect the addition of /etc/sysctl.conf.d for modular MIB customizations, and complete the history

This patch is based on the original work (reviews.freebsd.org/D32128) by
Antranig Vartanian <antranigv@freebsd.am>

Relnotes: Yes

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 72957
Build 69840: arc lint + arc unit

Event Timeline

agh_riseup.net created this revision.Sat, May 9, 7:24 AM
Herald added subscribers: ivy, ziaee, imp. · View Herald TranscriptSat, May 9, 7:24 AM
agh_riseup.net requested review of this revision.Sat, May 9, 7:24 AM
agh_riseup.net updated this revision to Diff 177480.

Updated diff with D${num}

agh_riseup.net added a comment.Sat, May 9, 7:27 AM

I am not sure on the sorting in the man page.

agh_riseup.net mentioned this in D32128: Add support for sysctl.conf.d.Sat, May 9, 7:28 AM
des added a subscriber: des.Sat, May 9, 8:27 AM

Can you please regenerate this with context, preferably using devel/freebsd-git-arc?

agh_riseup.net added a comment.Sat, May 9, 8:42 AM

Sure I will give it a shot. Is this because my git formatted-patch was striped of the preamble?

des added a comment.Sat, May 9, 8:54 AM

No, it's because you uploaded a unidiff with only three lines of context.

agh_riseup.net added a comment.EditedSat, May 9, 9:10 AM

Should the directory be tagged as rc, similar to https://reviews.freebsd.org/D56900?

des added a comment.Sat, May 9, 9:14 AM

Yes please, and remove the leading zero in the date.

agh_riseup.net updated this revision to Diff 177491.EditedSat, May 9, 12:35 PM

Uploaded git-arc patch.

Harbormaster completed remote builds in B72945: Diff 177491.Sat, May 9, 12:35 PM
agh_riseup.net edited the summary of this revision. (Show Details)Sat, May 9, 12:39 PM
agh_riseup.net retitled this revision from Add support for sysctl.conf.d to sysctl.conf(5): Add support for sysctl.conf.d.
agh_riseup.net edited the summary of this revision. (Show Details)
antranigv_freebsd.am added a subscriber: antranigv_freebsd.am.Sat, May 9, 2:51 PM

👍 +1! I Approve! Thank you! directorize all the things!

des added a comment.Sat, May 9, 6:27 PM

Should we add a comment to the stock sysctl.conf?

des edited reviewers, added: des; removed: Src Committers, srcmgr.Sat, May 9, 6:27 PM
ziaee added a comment.Sat, May 9, 6:51 PM

Please add Relnotes: yes to the commit message.

agh_riseup.net added a comment.Sat, May 9, 11:05 PM
In D56898#1303538, @des wrote:

Should we add a comment to the stock sysctl.conf?

Yes, I think that is a great idea.

agh_riseup.net added a comment.Sat, May 9, 11:06 PM
In D56898#1303540, @ziaee wrote:

Please add Relnotes: yes to the commit message.

Will do.

agh_riseup.net added a comment.Sun, May 10, 2:18 AM
In D56898#1303540, @ziaee wrote:

Please add Relnotes: yes to the commit message.

In D56898#1303619, @agh_riseup.net wrote:
In D56898#1303540, @ziaee wrote:

Please add Relnotes: yes to the commit message.

Will do.

Wait, that has to be done when/if the diff is merged to the Tree?

imp added a comment.Sun, May 10, 2:21 AM
In D56898#1303625, @agh_riseup.net wrote:
In D56898#1303540, @ziaee wrote:

Please add Relnotes: yes to the commit message.

In D56898#1303619, @agh_riseup.net wrote:
In D56898#1303540, @ziaee wrote:

Please add Relnotes: yes to the commit message.

Will do.

Wait, that has to be done when/if the diff is merged to the Tree?

You can add it now

imp added a comment.Sun, May 10, 2:22 AM

Just update the phabricator summary

agh_riseup.net updated this revision to Diff 177519.Sun, May 10, 2:26 AM
agh_riseup.net edited the summary of this revision. (Show Details)
  • Update stock sysctl.conf
  • Update man page date
Harbormaster completed remote builds in B72957: Diff 177519.Sun, May 10, 2:26 AM
agh_riseup.net edited the summary of this revision. (Show Details)Sun, May 10, 2:33 AM
agh_riseup.net updated this revision to Diff 177521.EditedSun, May 10, 2:46 AM

Correct typo in sbin/sysctl/sysctl.conf:
"to through to" becomes "through to"

Harbormaster completed remote builds in B72959: Diff 177521.Sun, May 10, 2:46 AM
agh_riseup.net foisted this revision upon antranigv_freebsd.am.Sun, May 10, 2:49 AM
agh_riseup.net added a reviewer: agh_riseup.net.
agh_riseup.net edited the summary of this revision. (Show Details)
agh_riseup.net added a comment.Sun, May 10, 2:53 AM
In D56898#1303487, @antranigv_freebsd.am wrote:

👍 +1! I Approve! Thank you! directorize all the things!

And then UCL all the things?

I noticed that could I edit the Summary to correctly attribute the author, you.

des added inline comments.Sun, May 10, 8:07 PM
sbin/sysctl/sysctl.conf
7

They're not “piped through” to sysctl(8), they're read directly by sysctl(8).

share/man/man5/sysctl.conf.5
36

I think we should add a sentence about /etc/sysctl.conf.d either here or before the paragraph about /etc/sysctl.kld.d.

agh_riseup.net added inline comments.Sun, May 17, 11:50 AM
sbin/sysctl/sysctl.conf
7

OK, this is easily rectified.

share/man/man5/sysctl.conf.5
36

What about a paragraph similar to the 2nd paragraph in sbin/sysctl/sysctl.conf above?

The files /etc/sysctl.conf, /etc/sysctl.conf.local and any .conf file(s) in the directory /etc/sysctl.conf.d are read when the system goes into multi-user mode to set default settings for the kernel. The structure of these configuration files is in the format of the sysctl(8) command, i.e.,

Then above sysctl.kld.d.,
The /etc/sysctl.conf.d exists to reduce management burden of kernel settings cluttering a single configuration file. Possible scenarios are for performance tuning or investigation, or separating out MIB node specific settings into separate files, or sharing common settings across a number of hosts.

agh_riseup.net added a comment.Sun, May 17, 11:52 AM

Another solution, move /etc/sysctl.conf.d to EXAMPLES, and provide two or three examples there?

des added inline comments.Sun, May 17, 12:26 PM
share/man/man5/sysctl.conf.5
36

These both seem fine to me.

des added a comment.Sun, May 17, 12:28 PM
In D56898#1303648, @agh_riseup.net wrote:

I noticed that could I edit the Summary to correctly attribute the author, you.

There's very little of Antranig's work in this patch. Crediting him as inspiration in the commit message is fine, attributing the commit to him is not.

agh_riseup.net commandeered this revision from antranigv_freebsd.am.Mon, May 18, 5:40 AM
agh_riseup.net edited reviewers, added: antranigv_freebsd.am; removed: agh_riseup.net.
agh_riseup.net updated this revision to Diff 177995.Mon, May 18, 5:41 AM

sysctl.conf(5): Add support for sysctl.conf.d

Update with new changes to man page and stock sysctl.conf

Differential Revision: https://reviews.freebsd.org/D56898

Harbormaster completed remote builds in B73180: Diff 177995.Mon, May 18, 5:41 AM
agh_riseup.net edited the summary of this revision. (Show Details)Mon, May 18, 5:43 AM
agh_riseup.net added a comment.EditedMon, May 18, 5:49 AM

Man page was checked with mandoc -T lint, igor, and aspell check --lang=en --mode=nroff as per the Documentation Project Primer for New Contributors, the only issue was a lack of SYNPOSIS

Revision Contents
Changeset List

PathSize
etc/
mtree/
2 lines
libexec/
rc/
rc.d/
2 lines
sbin/
sysctl/
7 lines
share/
man/
man5/
6 lines

Diff 177519

View Options

etc/mtree/BSD.root.dist

Loading...
View Options

libexec/rc/rc.d/sysctl

Loading...
View Options

sbin/sysctl/sysctl.conf

Loading...
View Options

share/man/man5/sysctl.conf.5

Loading...