Skip to content

Improvements to DHCP client#249

Merged
mattiaswal merged 28 commits intomainfrom
dhcp-client-features
Dec 14, 2023
Merged

Improvements to DHCP client#249
mattiaswal merged 28 commits intomainfrom
dhcp-client-features

Conversation

@troglobit
Copy link
Contributor

@troglobit troglobit commented Dec 14, 2023

This PR extends the functionality of the DHCPv4 client by adding support for:

  • Changing the default request options
  • Acting on more options: NTP servers, routes, DNS servers
  • Configurable ARP:ing of acquired lease, default: enabled
  • Configurable route preference (metric) per DHCP interface. Applies to options 3, 121, 249

All routes, including default route, is by default set with proto dhcp and metric 100. This ensure that routes from other routing protocols, as well as statically set routes, have priority over DHCP acquired routes.

DNS servers, and search/domain settings, are saved per interface and enabled globally in the system using the dnsmasq resolver functionality. A statically set DNS server will always take precedence.

NTP servers are added as non-preferred sources, and will be selected according to the NTPv4 protocol (strata), unless there is a statically configured NTP server with the prefer flag set.

A set of basic DHCP tests have been added to verify that leases are accepted and routes are installed properly (according to option precedence rules defined in RFC). There are currently no regression tests for DNS or NTP yet, these can easily be added later when we have operational support for querying system DNS and NTP servers. (Manual testing has of course been performed.)

Note: for NTP servers to actually take, the ietf-system NTP client must be enabled. (It can be enabled without any static servers set, but is by default disabled.)

@troglobit
cli: add 'show ntp [sources]' command to admin-exec
8fae788 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
confd: refactor ntp client setup
0706287 
Move from everything in a single /etc/chrony.conf to a split up with
configuration and server snippets.  The latter comes in the form of
configured (static) and DHCP client (dynamic) server setup.

To accomodate this new scheme we need to detect when serves are removed
from the configuration, so not only have the whole change_ntp() been
refactored, it has been extended with a new pass.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
board/common: add support for acquiring NTP server from DHCP server
726745f 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
board/common: major refactor of udhcpc script
29bdc84 
 - New log() function replaces stdout logging
 - New set_dhcp_routes() and clr_dhcp_routes() functions
   - Set all option 121 routes with same metric
   - Set all option 3 routers with increasing metric (this is what the
     reference udhcpc scripts do, and RFC says the routers should be
     listed in order of preference ...)
   - Clearing routes must, like IP addresses, be done both by interface
     and protocol.  This refactor makes sure to delete any DHCP routes
     set on the given interface (in case options change)
 - Use resolvconf per-interface search+nameserver
 - Cache IP lease so we can ask for it back later
 - On deconfig|leasefail|nak, make sure to clean up anything that might
   be lingering from this interface.  E.g., we can get leasefail when a
   server denies our request to prolong a lease.
 - On renew|bound, refresh routes, and set search+dns + NTP servers

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
confd: initial enhancements to DHCP client
b2fb75c 
 - Add support for option 12, provide current hostname to server for
   registering the lease with -- this allows registering in local DNS
   for some DHCP servers.
 - Add support for option 50, request any previously cached IP address
 - Override option 60, vendor class identifier, with Infix vYY.MM
 - Adjust timers and retry options to be more persistant
 - Include initial metric as environment variable to client
 - Disable all default options, set a hard-coded subset, which will be
   replaced in a later commit by a generated list.
 - Ensure udhcpc creates a pid file, in case we may need to start any
   other service/task in sync with the DHCP client

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
doc: list supported DHCP client options
814490a 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
confd: update DHCP YANG model, new options, arping, and route metric
f29966a 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
confd: add support for arping (1 ARP w/ 1 sec timeout)
49477e3 
This should be longer, there are devices out there (in the industry)
that may drop the ball on a single ARP, or may be in deep sleep (IoT),
so we should send like 5 ARP with at least 1 sec between them before
timing out.

However, this requires patching BusyBox a bit, so skipping for now.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
confd: initial support for extended dhcp client settings
40472c7 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
confd: refactor infix-dhcp-client options from leaf-list to list
12c8735 
This commit changes the DHCP options from plain flags to customizable
options with optional values.  This works for most key:value options,
but not for the more complex ones, e.g. option 81.  For this we have
dedicated handling to use `udhcpc -F fqdn` instad.

Additionally, the inference a default option list has been removed in
favor of a set of generic default options: router, dns, domain, address,
broadcast, ntpsrv, search, staticroutes, msstaticroutes.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
cli: add 'show dns' command to display DNS client status
b51bbc4 
This includes locally configured DNS and any learned from DHCP.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
patches/finit: drop old v4.4 patches (on v4.6 now)
ae0e334 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
patches/finit: backport silencing of noisy log messages
6f34e1c 
When an optional service doesn't exist, and the run/task/service stanza
clearly list `nowarn`, we should not log missing/skipping messages. This
patch-set is backported from the upcoming Finit v4.7

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
confd: add netlink condition to dhcp-client, <net/IFNAME/running>
b860a12 
This patch ensures that the DHCP client is not started until the
interface is up and running (link).  If the interface goes down,
or loses link, the client is stopped and everything learned from
the server (address, routes, DNS/NTP servers, etc.) is remvoed.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit troglobit added this to the Infix v23.12 milestone Dec 14, 2023
@troglobit troglobit requested review from mattiaswal and wkz December 14, 2023 12:11
@troglobit troglobit force-pushed the dhcp-client-features branch from 3d76b95 to 51e0c02 Compare December 14, 2023 14:11
@troglobit
Copy link
Contributor Author

troglobit commented Dec 14, 2023

(Re: force push: just squashed and moved around a few commits to reduce the amount of cruft.)

mattiaswal
mattiaswal reviewed Dec 14, 2023
View reviewed changes
Copy link
Contributor

@mattiaswal mattiaswal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One documentation change and one question.

src/confd/src/infix-dhcp.c

static const struct srx_module_requirement infix_dhcp_reqs[] = {
{ .dir = YANG_PATH_, .name = "infix-dhcp-client", .rev = "2023-05-22" },
static const struct srx_module_requirement reqs[] = {
Copy link
Contributor

@mattiaswal mattiaswal Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why check if the module is loaded? will it ever not be?

Copy link
Contributor Author

@troglobit troglobit Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just the auto-loader, the dependency-order handling is done by the bootstrap script. for models we don't have any dependencies (or need factory settings) we can load using the auto-loader (srx_require_modules()) in each .C file's init function.

@troglobit
cli: for consistency with YANG model, rename METRIC --> PREF(ERENCE)
75d3c3d 
In review discussions we have decided, for resonsons of consistency over
correctness, to rename this column to match the YANG model node name
rather than the Linux kernel name.

Also, right align the values.  Numbers should always be right-aligned.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
board/netconf: add 'cfg' alias for 'sysrepocfg -f json'
5fd0043 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: fix regression in starting qeneth from e644d87
42bfe96 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: minor, update module description
6c2bba3 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: rename method
44f9570 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: add method, print all data for a given interface
ae7b2de 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
confd: refactor, let default options to use same code path
bb484a7 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: two new methods to fetch ietf-ip ipv4 addresses
3117aa0 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: disable yang validation before sending config to target
c21600d 
This should probably be an optional argument to this method instead.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: new infamy DHCP server class
e70f04f 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: new test, basic DHCP client
97c609f 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: minor refactor to check for nexthop (gw) in routes
3828846 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: new test, verify dhcp option 3 (router)
91ee8c1 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
test: new test, verify dhcp option 121 is preferred over option 3
4145a64 
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit troglobit force-pushed the dhcp-client-features branch from 51e0c02 to 4145a64 Compare December 14, 2023 15:36
@mattiaswal mattiaswal merged commit 95c3e5c into main Dec 14, 2023
@mattiaswal mattiaswal deleted the dhcp-client-features branch December 14, 2023 15:40
@troglobit troglobit modified the milestones: Infix v23.12, Infix v24.01 Jan 16, 2024
@troglobit troglobit mentioned this pull request Jul 2, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattiaswal mattiaswal mattiaswal approved these changes

@wkz wkz Awaiting requested review from wkz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Infix v24.02

Development

Successfully merging this pull request may close these issues.

2 participants

@troglobit @mattiaswal