Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic DHCP server support #887

Merged
merged 28 commits into from
Jan 31, 2025
Merged

Add basic DHCP server support #887

merged 28 commits into from
Jan 31, 2025

Conversation

troglobit
Copy link
Contributor

@troglobit troglobit commented Jan 14, 2025

Description

This work fixes #703 for Infix v25.01. Further work, as laid out in #446, is planned after that.

Based on the work by @sgsx3 for @minexn

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Test changes
    • Checked in changed Readme.adoc (make test-spec)
    • Added new test to group Readme.adoc and yaml file
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
    • ChangeLog updated (for major changes)
  • Other (please describe):

@troglobit troglobit added the enhancement New feature or request label Jan 14, 2025
@troglobit troglobit added this to the Infix v25.01 milestone Jan 14, 2025
@troglobit troglobit self-assigned this Jan 14, 2025
@troglobit troglobit force-pushed the dhcp-server branch 5 times, most recently from 4db9499 to 34f7141 Compare January 26, 2025 17:54
@troglobit troglobit force-pushed the dhcp-server branch 7 times, most recently from d30f4b8 to 52f5df9 Compare January 30, 2025 04:41
@troglobit troglobit requested review from mattiaswal and wkz January 30, 2025 04:42
@troglobit troglobit marked this pull request as ready for review January 30, 2025 04:42
@troglobit troglobit requested a review from jovatn January 30, 2025 04:42
@troglobit troglobit force-pushed the dhcp-server branch 2 times, most recently from d9ac0ad to e0c18bd Compare January 30, 2025 09:12
Copy link
Contributor

@jovatn jovatn left a comment

Choose a reason for hiding this comment

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

Great work!!
In have some comments inline - use as you judge best.
In addition, I wonder if there are some problems with formatting the output when running "help" inside "subnet" and "host" contexts in DHCP server. I'll paste some output below.

admin@ix-00-00-00:/config/dhcp-server/subnet/192.168.2.0/24/> help
  description                       Additional information about this subnet (e.g., purpose, location, or notes).
  enabled                           Enable or disable DHCP server on this subnet.
  if-name                           Optional interface to bind this subnet to.
  option                              pool                                host                              List of static host entries.
admin@ix-00-00-00:/config/dhcp-server/subnet/192.168.2.0/24/> 


admin@ix-00-00-00:/config/dhcp-server/subnet/192.168.2.0/24/host/192.168.2.10/> help
  description                       Additional information about this entry, e.g., owner's name, equipment details, or location.
  match                               hostname                          Optional hostname to assign with the lease.
  lease-time                        Lease time, or 'infinite'.
  option                            
admin@ix-00-00-00:/config/dhcp-server/subnet/192.168.2.0/24/host/192.168.2.10/> 

@troglobit
Copy link
Contributor Author

Great work!! In have some comments inline - use as you judge best.

Thanks! 😃

In addition, I wonder if there are some problems with formatting the output when running "help" inside "subnet" and "host" contexts in DHCP server. I'll paste some output below.

Ouch, seems to be missing descriptions for nodes/lists/etc. Thanks!

admin@ix-00-00-00:/config/dhcp-server/subnet/192.168.2.0/24/> help
  description                       Additional information about this subnet (e.g., purpose, location, or notes).
  enabled                           Enable or disable DHCP server on this subnet.
  if-name                           Optional interface to bind this subnet to.
  option                              pool                                host                              List of static host entries.
admin@ix-00-00-00:/config/dhcp-server/subnet/192.168.2.0/24/> 


admin@ix-00-00-00:/config/dhcp-server/subnet/192.168.2.0/24/host/192.168.2.10/> help
  description                       Additional information about this entry, e.g., owner's name, equipment details, or location.
  match                               hostname                          Optional hostname to assign with the lease.
  lease-time                        Lease time, or 'infinite'.
  option                            
admin@ix-00-00-00:/config/dhcp-server/subnet/192.168.2.0/24/host/192.168.2.10/> 

Copy link
Contributor

@wkz wkz left a comment

Choose a reason for hiding this comment

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

Absolutely brilliant! 💎

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.

LGTM. Great addition for Infix! 🚀 🏅 ❤️

troglobit and others added 27 commits January 31, 2025 13:55
Similar to the run_multiline() method, sometimes files contain more than
one line and a common read_multiline() quickly becomes useful.  This is
currently only used by ietf-system for reading /etc/resolv.conf.head.

Also, fix pylint warning for missing encoding to all open() calls.

Signed-off-by: Joachim Wiberg <[email protected]>
Instead of 'show ntp' =>

    Error, top level "ietf-system:system-state" missing

Try something nicer:

    NTP client not enabled.

Signed-off-by: Joachim Wiberg <[email protected]>
The sr_get_item() function logs an error: "No data found for ..." in
case the referenced XPatjh does not exist, causing unnecessarily scary
log message about missing genkey elements in /ietf-keystore:keystore/

Signed-off-by: Joachim Wiberg <[email protected]>
Skip the password node in operational data for entries that have locked
or empty password, otherwise the YANG validation (regexp) will fail.

Signed-off-by: Joachim Wiberg <[email protected]>
This patch introduces a new yang yang model for DHCP server and relay,
using dnsmasq to provide the functionality on a per-interface basis.

Available DHCP options will be queried by confd on startup and can
then be configured in a flexible manner.

The DHCP option 82 feature has been realized by patching dnsmasq
(see patches/dnsmasq/2.90/0000-relay-agent-info.patch).

Signed-off-by: Stefan Schlosser <[email protected]>
For consistency with the new dhcp-server model, which has an option
value 'name', options are now referred to as id's or identifiers.

This means bumping the confd syntax version 1.3 -> 1.4 and adding a
migration script for existing dhcp-client configuration files.

Signed-off-by: Joachim Wiberg <[email protected]>
A DHCP client should be able to request a DHCP server assigned hostname,
*and* advertise its current hostname (as a client-id).  This change lets
the client do either, instead of always assuming the latter.

Alt 1)
     option id:hostname                        => request hostname

Alt 2)
     option id:hostname, value:<STRING | auto> => advertise hostname

This also fixes a bug where the hostname sent to the server could be the
old hostname.  The fix picks up any new hostname directly from sysrepo.

Signed-off-by: Joachim Wiberg <[email protected]>
The hostname leaf in ietf-system.yang defines it as an fqdn.  This patch
adds support for splitting the host and domain parts to set them in
Linux according to hosts(5).

Signed-off-by: Joachim Wiberg <[email protected]>
 - dnsmasq:
   - allow listening to all interfaces again to serve DHCP leases
   - disable most of the default DHCP options, keep broadcast and
     netmask because when dnsmasq can infer these they are better
     than breaking networking for clients -- note, they can still
     be overridden from global/subnet/host scope -- and for relay
     setups they need to be supplied anyway
 - confd:
   - drop per-interface dnsmasq, although practical for many use-cases,
     having a per-subnet focused server means improved integration with
     future stand-alone dhcp relay setup
   - implement suggested, simplified, yang model

Fixes #703

Signed-off-by: Joachim Wiberg <[email protected]>
Example status output:

    "infix-system:dns-resolver": {
      "options": {
        "timeout": 3,
        "attempts": 5
      },
      "search": [
        "example.com",
        "foo.com"
      ],
      "server": [
        {
          "address": "1.2.3.4",
          "origin": "static"
        },
        {
          "address": "192.168.2.1",
          "origin": "dhcp",
          "interface": "e5"
        }
      ]
    }

Fixes #510

Signed-off-by: Joachim Wiberg <[email protected]>
With support for dns resolver in place, the ietf-system output needs to
be complemented with additional data collected from both the dnsmasq and
openresolv subsystems.

System date taken from 'client3' in the Infix DHCP server-subnets test.

Signed-off-by: Joachim Wiberg <[email protected]>
Instead of encoding hex data in regular strings, using an 'id:' prefix,
or guessing based on pattern in C code, we have decided for a breaking
change in the DHCP client model.

This commit introduces string and hex values for client-id in the models
for both DHCP client and server.  The *breaking* part of the changes are
strictly related to how a user pass binary data as colon-separated hex
digits.  The dedicated leaf node `client-id` is now reserved for string
values and the generic `list option` can be used to input user defined
string or hex values.  Even non-conformant client-id options can now be
encoded.  All described in detail in infix-dhcp-client.yang

The server model has been updated to be able to send generic options in
hex format, and the static host lease matcher for client-id now require
a 'str' or 'hex' modifier.

Note: dnsmasq is RFC compliant and will not match non-conforming option
      61 (client-id) even though the client can send such payload.

To increase test coverage, the server_host test has been updated to
check both string and hex client-id.  Matching on hostname is already
covered by other test(s).

Signed-off-by: Joachim Wiberg <[email protected]>
Counters should follow the format from ieee802-ethernet-interface
model, which use:

 - in/out for Rx/Tx
 - plural from

Signed-off-by: Joachim Wiberg <[email protected]>
@mattiaswal mattiaswal merged commit 5e89d8e into main Jan 31, 2025
6 checks passed
@mattiaswal mattiaswal deleted the dhcp-server branch January 31, 2025 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
5 participants