Import dhcpcd(8) into FreeBSD base.

[woodsb02]

Summary

Import dhcpcd(8) into FreeBSD base.

This introduces DHCPv6 functionality to FreeBSD.
This exists in parallel with existing tools dhclient(8) and rtsol(8),
which continue to be the default.

To use dhcpcd(8) for dynamic address allocation of IPv4 and IPv6,
make the following changes to /etc/rc.conf:

• set dhcpcd_enable="YES"
• remove "DHCP" from the ifconfig_<iface> line
• remove rtsold_enable
• remove ifconfig_<iface>_ipv6="inet6 accept_rtadv"

Test Plan

dhcpcd_enable="YES" enables both DHCPv4 and DHCPv6 on all interfaces.

In addition to the normal review cycle, given I am only a ports committer (I don’t have a src commit bit), I will need this to be endorsed and approved by a src committer.

[driesm]

Thanks for the good work Ben.

libexec/rc/rc.d/dhcpcd

7

I'm wondering how we tackle the rc script when both dhcpcd exists in base and in ports (and both are installed).
I know for unbound, we made it "local_unbound" for the in-base version, and "unbound" for the port version.
I'm asking this as what happens when dhcpcd is installed from ports and is present in the base system with the same rc identifier, probably will be started twice with one dhcpcd_enable line?

15

Ditto

[moonlapse81_gmail.com]

In D22012#826303, @woodsb02 wrote:

Implement review comments:

• Add dhcpcd as a separate package (not in "runtime" package)
• Allow dhcpcd to be compiled without IPv4 or IPv6 support
• Remove duplicate system directory configuration
• Bump manpage dates
• Simplify rc.conf(5) language describing master mode

how to specifically disable ipv6 ( or ipv4 ) when dhcpcd is in /usr/src?

[pauamma_chezdork.com]

Some or all of this needs pushing upstream instead of/in addition to local changes. Since the upstream maintainer is subscribed, I haven't made the distinction.

contrib/dhcpcd/README.md

5–7

I believe Wikipedia has been redirecting http to https for 6 years or so. Let's save a round trip.

9

Per https://books.google.com/ngrams/graph?content=layman_INF%2Clayperson_INF&year_start=1800&year_end=2019&corpus=28&smoothing=3, "laym{ae]n" steeply declined in frequency of use starting around 1960 and is now very close to the gender-agnostic forms layperson(s) and laypeople. So I'd future-proof this.

97

After checking it works.

contrib/dhcpcd/hooks/dhcpcd-run-hooks.8

72

73

Spurious line?

80

89

Make clearer to non-native speakers it's "initialize now", not "initialization should already be complete".

105

108–109

Is "REBOOT" correct? I can't connect the dots between rebooting and getting a leased address for the first time.

148

For consistency, either remove .Nm here or add it to all list items above.

185

A SSID *is* a name.

196

What does that mean?

203

Same question as above.

211

217–221

Commafy list to make it clearer that "lexical order" applies to /libexec/dhcpcd-hooks/* only.

228

Since the https version works.

contrib/dhcpcd/src/dhcpcd.8

175

187

Or "The ... option"

325

455–458

Unclear which configuration "and configuration" refers to.

794

830

886

contrib/dhcpcd/src/dhcpcd.conf.5

1005

[emaste]

I'd suggest updating this review to be a diff against the copy of dhcpcd now in vendor/ merged into contrib/, so that changes if any to existing dhcpcd files are clear

[roy_marples.name]

In D22012#826669, @emaste wrote:

I'd suggest updating this review to be a diff against the copy of dhcpcd now in vendor/ merged into contrib/, so that changes if any to existing dhcpcd files are clear

That would help me greatly bringing any changes requested here upstream.

[imp]

In D22012#818455, @emaste wrote:

As far as I can tell the objections that were raised over the last few years have all been addressed, and it doesn't seem like much progress has been made on the proposed alternatives. It seems reasonable to me to import dhcpcd initially controlled by a build knob (i.e., WITH_DHCPCD) to allow for further development and experimentation.

This seems reasonable. Just add DHCPCD to DEFAULT_NO and we should be good to go :)

In D22012#826671, @roy_marples.name wrote:

In D22012#826669, @emaste wrote:

I'd suggest updating this review to be a diff against the copy of dhcpcd now in vendor/ merged into contrib/, so that changes if any to existing dhcpcd files are clear

That would help me greatly bringing any changes requested here upstream.

and hopefully it will be easier to review here too :)
Win win

[woodsb02]

Fix build WITHOUT_INET=yes: move SRCS+=privsep-bpf.c inside .if ${MK_INET_SUPPORT}

[woodsb02]

In D22012#826551, @moonlapse81_gmail.com wrote:

how to specifically disable ipv6 ( or ipv4 ) when dhcpcd is in /usr/src?

As per the rest of the FreeBSD src branch - add one of the following to your /etc/src.conf file before running make buildworld:

• WITHOUT_INET=yes
• WITHOUT_INET6=yes

[woodsb02]

In D22012#826669, @emaste wrote:

I'd suggest updating this review to be a diff against the copy of dhcpcd now in vendor/ merged into contrib/, so that changes if any to existing dhcpcd files are clear

No changes have been made in this review from the copy in the vendor/dhcpcd branch.
The only difference if that rather than being a direct copy/paste, I have used the mechanism built into dhcpcd to load the src into contrib/dhcpcd - this is described in contrib/dhcpcd/README.FREEBSD - extract below for convenience.

The source is imported via a Makefile target rather than by hand.
There is no README.DELETED for this import as it's all automated.

Use "git diff ../vendor/dhcpcd contrib/dhcpcd" to see local modifications.

The program, hook scripts and configuration file are installed by 'sbin/dhcpcd'.

Upgrade notes
-------------

1. Configure
2. Import
3. Copy config.h to sbin/dhcpcd
4. Tailor Makefile in sbin/dhcpcd to import

$ ./configure
$ make import-src DESTDIR=/usr/src/contrib/dhcpcd
$ cp config.h /usr/src/sbin/dhcpcd
$ vi /usr/src/sbin/dhcpcd/Makefile

The following files are only in contrib/dhcpcd:

• hooks/30-hostname
• hooks/50-ypbind
• hooks/dhcpcd-run-hooks
• hooks/dhcpcd-run-hooks.8
• src/dhcpcd-embedded.c
• src/dhcpcd-embedded.h
• src/dhcpcd.8
• src/dhcpcd.conf.5
• README.FREEBSD

The following files are only in vendor/dhcpcd (not brought into contrib/dhcpcd by the "make import-src"):

• .git/*
• compat/crypt/md5.c
• compat/crypt/md5.h
• compat/crypt/sha256.c
• compat/crypt/sha256.h
• compat/arc4random.c
• compat/arc4random.h
• compat/arc4random_uniform.c
• compat/arc4random_uniform.h
• compat/dprintf.c
• compat/dprintf.h
• compat/endian.h
• compat/queue.h
• compat/reallocarray.c
• compat/reallocarray.h
• compat/setproctitle.c
• compat/setproctitle.h
• compat/strlcpy.c
• compat/strlcpy.h
• hooks/30-hostname.in
• hooks/50-dhcpcd-compat
• hooks/50-yp.conf
• hooks/50-ypbind.in
• hooks/Makefile
• hooks/dhcpcd-run-hooks.8.in
• hooks/dhcpcd-run-hooks.in
• src/dev/*
• src/GNUmakefile
• src/Makefile
• src/dev.c
• src/dhcpcd-definitions-small.conf
• src/dhcpcd-definitions.conf
• src/dhcpcd-embedded.c.in
• src/dhcpcd-embedded.h.in
• src/dhcpcd.8.in
• src/dhcpcd.conf.5.in
• src/genembedc
• src/genembedh
• src/if-linux-wext.c
• src/if-linux.c
• src/if-sun.c
• src/privsep-linux.c
• src/privsep-sun.c
• tests/*
• .gitignore
• BUILDING.md
• Makefile
• Makefile.inc
• config-null.mk
• configure
• iconfig.mk

[woodsb02]

In D22012#826694, @imp wrote:

In D22012#826671, @roy_marples.name wrote:

In D22012#826669, @emaste wrote:

I'd suggest updating this review to be a diff against the copy of dhcpcd now in vendor/ merged into contrib/, so that changes if any to existing dhcpcd files are clear

That would help me greatly bringing any changes requested here upstream.

and hopefully it will be easier to review here too :)
Win win

Given I have not made any changes to the contents of contrib/dhcpcd (other than adding the new README.FreeBSD file), suggest reviewers could exclude contrib/dhcpcd (focus on other changes in the review)?

[woodsb02]

In D22012#826644, @pauamma wrote:

Some or all of this needs pushing upstream instead of/in addition to local changes. Since the upstream maintainer is subscribed, I haven't made the distinction.

Thanks for the detailed review Pau!

I am trying to keep the contrib/dhcpcd code exactly as per upstream, so rather than fix them here, I have submitted a pull request to upstream on your behalf:
https://github.com/NetworkConfiguration/dhcpcd/pull/123

The only comments of yours I didnt include were:

• Your question about what was meant by IFF_UP - will leave it to @roy_marples.name to clarify wording
• Your question about whether REBOOT was correct - will leave it to @roy_marples.name to clarify wording
• Removing the .Nm from "dhcpcd" at the beginning of the ENVIRONMENT section of the manpage - as I suspect it is either referring to the name of the binary when executed, or is for consistency showing the bold name at the beginning of a new section. @roy_marples.name?

[roy_marples.name]

I've addressed the IFF_UP and REBOOT comments with hopefully suitable answers but not marked them as done as I think the asker needs to be satisified and close themselves?

contrib/dhcpcd/hooks/dhcpcd-run-hooks.8

108–109

REBOOT is when a previously aquired lease is acked by the server but not yet on the interface.
Such as when the host initially starts up or when the carrier goes down or comes back up again.

If there is no previouisly acquired lease, or it has expired then the BOUND event is raised instead.

Realistically there is no difference between the two and mainly exists as to be compatible with dhclient-script(8)

196

For example the link could be down or could be in some other transitive state.
I'm deliberately being vague because it's highly dependent on the interface state, what the OS supports and the state of various protocols running on it.

I think I'm trying to say that even if the interface is marked as up, dhcpcd might mark it as not up.

203

Same answer as above, but note that if_down is seperate from if_up - it's not possible both would be true, but it is possible for both to be false.

[pauamma_chezdork.com]

In D22012#826793, @roy_marples.name wrote:

I've addressed the IFF_UP and REBOOT comments with hopefully suitable answers but not marked them as done as I think the asker needs to be satisified and close themselves?

I'm fine with them being marked "done", but I think only the review author (@woodsb02) can do that.

[pauamma_chezdork.com]

In D22012#826786, @woodsb02 wrote:

In D22012#826644, @pauamma wrote:

Some or all of this needs pushing upstream instead of/in addition to local changes. Since the upstream maintainer is subscribed, I haven't made the distinction.

Thanks for the detailed review Pau!

I am trying to keep the contrib/dhcpcd code exactly as per upstream, so rather than fix them here, I have submitted a pull request to upstream on your behalf:
https://github.com/NetworkConfiguration/dhcpcd/pull/123

Thanks for that. It looks like that was committed upstream meanwhile.

[freebsd_steffen.cc]

@woodsb02 / everyone: What's the state of this?

I've recently had issues with both dhclient and rtsold and decided to try dhcpcd as it also adds dhcpv6 client support.
Dhcpcd from ports works great and I could put that into our downstream images but I'd like to avoid doing that and then having a version in base a couple months later.

Conversation on the mailing list seems to have stalled and it doesn't seem like there would be an issue as long as dhclient is still available.

[cy]

In D22012#862131, @freebsd_steffen.cc wrote:

@woodsb02 / everyone: What's the state of this?

I've recently had issues with both dhclient and rtsold and decided to try dhcpcd as it also adds dhcpv6 client support.
Dhcpcd from ports works great and I could put that into our downstream images but I'd like to avoid doing that and then having a version in base a couple months later.

Conversation on the mailing list seems to have stalled and it doesn't seem like there would be an issue as long as dhclient is still available.

I suppose as long as,

A) There is a build knob to enable/disable building this.
B) In addition it has to have its own rc script and that the new rc script and the older dhclient rc script are enabled differently in ifconfig_IF= differently in rc.conf to allow users to select the old and new way. For example, DHCP used for the old dhclient script while NDHCP (or something else) used for this new dhcpd,
C) At some point should it be decided to deprecate the old dhclient DHCP for the dhclient way would be renamed to ODHCP while the NDHCP renamed to DHCP.

In other words we need a plan for how this will be implemented and its future in FreeBSD and how its future will affect the dhclient future in FreeBSD.

If this is to be imported into FreeBSD, sample IPv4 and IPv6 DHCP configurations must be provided.

sbin/Makefile

15

This should be optionalized.

[emaste]

One related note, the dhclient in the FreeBSD base system no longer has an upstream -- it is OpenBSD's modified version of ISC dhclient, but OpenBSD has subsequently renamed it and reworked it extensively.

[woodsb02]

Sorry for not providing an update on this sooner.
The summary is I was waiting on the next release of dhcpcd before proceeding with importing it into base.
The reason was a regression related to capsicum was noticed with FreeBSD-14 starting in August 2022 where the privilged process could no longer write to the routing table. This issue isn't occurring on FreeBSD-13, nor FreeBSD-14 from 2021.
@roy_marples.name found and fixed the issue in dhcpcd, but only in what will become dhcpcd-10 (it's too invasive for dhcpcd-9 at this point which is the stable branch).
The dhcpcd-10 has been delayed a bit, since Roy has a few health issues that are his priority right now.
That said, I'm still hoping this can be merged into FreeBSD-14 before the code slush begins on 25th April 2023.

[melifaro]

In D22012#862257, @woodsb02 wrote:

Sorry for not providing an update on this sooner.
The summary is I was waiting on the next release of dhcpcd before proceeding with importing it into base.
The reason was a regression related to capsicum was noticed with FreeBSD-14 starting in August 2022 where the privilged process could no longer write to the routing table. This issue isn't occurring on FreeBSD-13, nor FreeBSD-14 from 2021.

Thank you for the clarification! Just to check: the relevant change Is commit @ 30 Aug, right? This looks like a regression, but the commit description looks a bit vague. I wasn't able to find any bug report or message in the ML. Did you / @roy_marples.name reported it? If that's a regression, maybe something can be change on the kernel side, so dhcp 10 is not exactly a blocker.

@roy_marples.name found and fixed the issue in dhcpcd, but only in what will become dhcpcd-10 (it's too invasive for dhcpcd-9 at this point which is the stable branch).
The dhcpcd-10 has been delayed a bit, since Roy has a few health issues that are his priority right now.
That said, I'm still hoping this can be merged into FreeBSD-14 before the code slush begins on 25th April 2023.

[roy_marples.name]

In D22012#862327, @melifaro wrote:

Thank you for the clarification! Just to check: the relevant change Is commit @ 30 Aug, right? This looks like a regression, but the commit description looks a bit vague. I wasn't able to find any bug report or message in the ML. Did you / @roy_marples.name reported it? If that's a regression, maybe something can be change on the kernel side, so dhcp 10 is not exactly a blocker.

It is a regression in FreeBSD-14. I updated my VM to the latest FreeBSD-current and the issue is still there.
I didn't report it to any ML as I'm happier with the new code in dhcpcd-10.
In a nutshell it seems like the dhcpcd privileged process seems to be under capsicum control for *some* operations but not others and some in kernel message buffers seem to shrink as writev reports message too long.
The same code works fine on FreeBSD-13.

Happy to discuss this elsewhere?

[melifaro]

In D22012#862511, @roy_marples.name wrote:

In D22012#862327, @melifaro wrote:

Thank you for the clarification! Just to check: the relevant change Is commit @ 30 Aug, right? This looks like a regression, but the commit description looks a bit vague. I wasn't able to find any bug report or message in the ML. Did you / @roy_marples.name reported it? If that's a regression, maybe something can be change on the kernel side, so dhcp 10 is not exactly a blocker.

It is a regression in FreeBSD-14. I updated my VM to the latest FreeBSD-current and the issue is still there.
I didn't report it to any ML as I'm happier with the new code in dhcpcd-10.
In a nutshell it seems like the dhcpcd privileged process seems to be under capsicum control for *some* operations but not others and some in kernel message buffers seem to shrink as writev reports message too long.
The same code works fine on FreeBSD-13.

Happy to discuss this elsewhere?

Sure! I guess the best-suited platform for that would be our bugzilla - as I guess we need to get sort of a repro (or at least more detailed description) and go from here. Do you mind submitting something?

[woodsb02]

dhcpcd-10 will be released in the next few week or so. Following that, I intend to update this diff to target importing dhcpcd into base ahead of the FreeBSD-14 code slush.

[roy_marples.name]

In D22012#894877, @woodsb02 wrote:

dhcpcd-10 will be released in the next few week or so. Following that, I intend to update this diff to target importing dhcpcd into base ahead of the FreeBSD-14 code slush.

Just a reminder that dhcpcd-10 has been released and is ready for import into FreeBSD-14.
Well, from my perspective anyway.

[bcr]

@woodsb02: Are you working on a newer patch for dhcpcd-10 for inclusion into FreeBSD14?
Would really be great to have. If this needs any shepherding from core, I'll see what I can do.

[meka_tilda.center]

Is there something we can do to make dhcpcd part of 14.0-RELEASE besides testing this patch?

[• grahamperrin]

I don't have any IPv6-enabled network, but the test plan above looks simple enough for me to test with eduroam and one or two other environments.

Re: the schedule at https://www.freebsd.org/releases/14.0R/, I can probably find time to test before the first beta.

[meka_tilda.center]

This patch fails to apply to CURRENT. I am using dhcpcd-10 since it was released (and used previous versions) for DHCP, SLAAC and DHCPv6, all on the same network. That's the closest to testing I can get in current state. If this patch is updated I would love to test it. Of course, if updating patch is required, I would love to do it, but at this point I'm not sure I know how.

[driesm]

Would be good to get this in main before stable/14 branches. Should we look for someone else to do the work if @woodsb02 doesnt have the cycles right now?

[roy_marples.name]

I just pushed out dhcpcd-10.0.2 which fixes some privsep issues.

[roy_marples.name]

dhcpcd-10.0.3 just released which also fixes some important stuff.

Is there anything I can do to help accelerate this ticket?

[driesm]

I plan on taking this further during the holidays.

[editor_callfortesting.org]

This clearly has quite a bit of attention!
Is it PkgBase-friendly?
Are there any blockers other than the usual re-basing as time moves on?
If a vendor-branch import, how difficult is the maintenance burden?

[pauamma_chezdork.com]

In D22012#1018620, @editor_callfortesting.org wrote:

This clearly has quite a bit of attention!
Is it PkgBase-friendly?

@freebsd_igalic.co Is that something you have (time for) an opinion on?

[emaste]

contrib/dhcpcd/src/dhcpcd-embedded.c

2

Should we not generate this as part of the build process? It looks like it would be no trouble to invoke src/genembedh and src/genembedc as part of a bespoke bmake build.

[emaste]

contrib/dhcpcd/src/dhcpcd-embedded.c

2

generated files:

hooks/30-hostname
hooks/50-ypbind
hooks/dhcpcd-run-hooks
hooks/dhcpcd-run-hooks.8
src/dhcpcd-embedded.c
src/dhcpcd-embedded.h
src/dhcpcd.8
src/dhcpcd.conf.5

[roy_marples.name]

contrib/dhcpcd/src/dhcpcd-embedded.c

2

As upstream I agree with this as it makes upstreaming any patches easier.

However, someone in this thread said that this should be part of the import process and some wanted the same in other OS's.
I have created an import-src target which I use for this purpose into Dragonfly BSD and NetBSD as I'm lazy and like to automate.
See here: https://gitweb.dragonflybsd.org/dragonfly.git/blob/HEAD:/contrib/dhcpcd/README.DRAGONFLY

I also have the import target which just extracts the dist tarball to a directory.

Hope this helps.

[ziaee]

contrib/dhcpcd/compat/consttime_memequal.h

3

I think according to the licensing guide this needs to be relicensed or get an exemption from core@, many companies and countries don't accept "public domain dedications" as open source, e.g. I am forbidden by my company from touching this file.

https://docs.freebsd.org/en/articles/license-guide/

[roy_marples.name]

contrib/dhcpcd/compat/consttime_memequal.h

3

I think according to the licensing guide this needs to be relicensed or get an exemption from core@

I can probably use FreeBSDs timingsafe_memcmp(3) in a macro instead to avoid including this compat file if that’s a problem.

I will look into that next week when I get back to my desk.

[imp]

contrib/dhcpcd/compat/consttime_memequal.h

3

If you can re-use other bits of FreeBSD, that's likely good to do. It would avoid the issue. And I think it makes good sense here to have that small variation from the upstream, since it can be handled easily enough in all the canonical ways. I'd go so far as to say reusing the FreeBSD standard thing is safer and better.

Now, were it not the case that there's a great FreeBSD alternative, getting an exemption from core@ (more likely srcmgr@) for the public domain license wouldn't be a problem. It's only an issue, really, when it's something like the yacc-shaving-plenipotentiary-license-v1.58 (eg, something weird and against the goals of the project to make it easier to share).

[roy_marples.name]

contrib/dhcpcd/compat/consttime_memequal.h

3

I think according to the licensing guide this needs to be relicensed or get an exemption from core@

I can probably use FreeBSDs timingsafe_memcmp(3) in a macro instead to avoid including this compat file if that’s a problem.

Fixed upstream here: https://github.com/NetworkConfiguration/dhcpcd/commit/46b6a361bb0444c4e93bc565bbee2ea17e65d2d1
I used timingsafe_bcmp instead as it's equivalent aside from the return value being the opposite.
I've just pushed out dhcpcd-10.2.4 with this change in.

To get the same fix in this version if you want, simply change the generated config.h file to use

- include                        "compat/consttime_memequal.h"
+ #define consttime_memequal      !timingsafe_bcmp

[rcm]

I have submitted a draft PR upstream to port https://reviews.freebsd.org/D31263 over to dhcpcd.