dhcp-server: T3936: Added support for DHCP Option 82 #4665

cblackburn-igl · 2025-08-17T16:48:03Z

Change summary

NB: This is a replacement pull request for one which I had to close as I was unable to rebase the commits into a single commit, as is required by the coding standard of VyOS. I have linked the previous PR in the Related PR's section so the history can be seen.

This commit adds support in both the CLI and the underlying code for DHCP Option 82 to be used to filter/route DHCP address assignments.

The primary use case for this is to support enterprise switches which can "tag" DHCP requests with physical real world informaiton such as which switch first saw the request and which port it originated from (known in this context as remote-id and circuit-id). Once client-classes have been defined they can be assigned to subnets or ranges so that only certain addresses get assigned to specific requests.

There is also a corresponding documentation update which pairs with this code change.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Code style update (formatting, renaming)
Refactoring (no functional changes)
Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
Other (please describe):

Related Task(s)

https://vyos.dev/T3936

Related PR(s)

vyos/vyos-documentation#1666
#4654 <-- Old Pull request this replaces

How to test / Smoketest result

I have tested these changes by adding new smoketests into the pre existing smoketest file. These new tests specifically check the new functionality including verifying that the generated config files are as expected and that the CLI refuses to take an incorrect commit.

I have also done integration testing in our lab which is set up with a switch which injects DHCP Option 82 information into live DHCP requests and these are processed as expected by VyOS.

Checklist:

I have read the CONTRIBUTING document
I have linked this PR to one or more Phabricator Task(s)
I have run the components SMOKETESTS if applicable
My commit headlines contain a valid Task id
My change requires a change to the documentation
I have updated the documentation accordingly

github-actions · 2025-08-17T16:48:14Z

👍
No issues in PR Title / Commit Title

github-actions · 2025-08-19T14:26:18Z

❌ Conflicts Found. This pull request has conflicts. Please resolve them before we can evaluate the pull request.

cblackburn-igl · 2025-08-19T14:55:06Z

This is quite a significant conflict that I have to resolve that's going to require some merging commits. These will mean it won't be possible to rebase the commits down into a single commit, similar to the previous pull request that I had to abandon. Do I absolutely have to revert everything and redo it so it can be in one commit or is it acceptable to have a few commits in this case?

cblackburn-igl · 2025-08-28T13:42:33Z

As I've not heard anything about this I'm going to assume that I can continue with multiple commits in the PR.

github-actions · 2025-08-28T15:05:17Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

dmbaturin

It looks good overall but I added a few suggestions that I think are worth discussing.

interface-definitions/include/dhcp/dhcp-server-common-config.xml.i

python/vyos/kea.py

interface-definitions/include/dhcp/dhcp-server-common-config.xml.i

sarthurdev · 2025-09-29T21:55:18Z

Thanks for the PR. Implementation looks really good, I'll aim to test it locally this week and approve if all checks out.

Only nitpick is option82 as CLI node doesn't feel right having digits in the node name, but not a blocker.

interface-definitions/include/dhcp/dhcp-server-common-config.xml.i

Copilot

Pull Request Overview

This PR adds support for DHCP Option 82 (relay agent information) filtering in the VyOS DHCP server. This feature allows DHCP client classes to be defined based on Option 82 circuit-id and remote-id values, enabling address assignment filtering based on physical network information provided by enterprise switches.

Added client class configuration with Option 82 circuit-id and remote-id filtering
Added client class assignment to subnets and address ranges
Added validation for client class references and Option 82 hex format

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/conf_mode/service_dhcp-server.py	Added validation for client class references and Option 82 hex format
smoketest/scripts/cli/test_service_dhcp-server.py	Added comprehensive tests for client class functionality and validation
python/vyos/template.py	Added Jinja2 filter for rendering client class JSON configuration
python/vyos/kea.py	Added functions to parse client classes and build Option 82 test expressions
interface-definitions/include/dhcp/dhcp-server-common-config.xml.i	Added CLI definitions for client classes and Option 82 configuration
data/templates/dhcp-server/kea-dhcp4.conf.j2	Added client class rendering to Kea DHCP configuration template

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/conf_mode/service_dhcp-server.py

smoketest/scripts/cli/test_service_dhcp-server.py

python/vyos/kea.py

cblackburn-igl · 2025-10-02T17:34:23Z

sigh so, I have absolutely no idea how this PR has now managed to collect an extra 44 commits from seemingly unrelated pull requests.

If it's ok to accept this and everything will work out then please accept the PR. If not then I'll extract all the changes out into a new branch and resubmit the PR.

Git is both the best and the worst.

sever-sever · 2025-10-02T22:36:27Z

I see 68 commits and most of them unexpected

cblackburn-igl · 2025-10-05T19:45:14Z

well, in the spirit of this XKCD: https://xkcd.com/1597/ I think we're going to have to have a 3rd attempt at this pull request.

If any of the developers think that it's fine then please merge this one and I'll stop. Otherwise, I'll try and extract all of the changes and put them in a new branch and resubmit.

dmbaturin · 2025-10-09T09:55:55Z

@cblackburn-igl No, I'd prefer to avoid merging it with unrelated commits. Please clean up the unrelared history,

natali-rs1985

The code looks good but needs a minor changes to stay consistent

Also the history is still not cleaned

natali-rs1985 · 2025-10-22T11:14:06Z

interface-definitions/include/dhcp/dhcp-server-common-config.xml.i

+        </leafNode>
+        <leafNode name="remote-id">
+          <properties>
+            <help>Filters on the contents of the remote-id sub option.</help>


Suggested change

<help>Filters on the contents of the remote-id sub option.</help>

<help>Filters on the contents of the remote-id sub option</help>

The same as with circuit-id help string

natali-rs1985 · 2025-10-22T11:27:37Z

src/conf_mode/service_dhcp-server.py

+                if 'client_class' not in dhcp:
+                    raise ConfigError(f'Client class "{client_class}" set in subnet "{subnet}" but does not exist')
+
+                if client_class not in dhcp['client_class'].keys():
+                    raise ConfigError(f'Client class "{client_class}" set in subnet "{subnet}" but does not exist')


Suggested change

if 'client_class' not in dhcp:

raise ConfigError(f'Client class "{client_class}" set in subnet "{subnet}" but does not exist')

if client_class not in dhcp['client_class'].keys():

raise ConfigError(f'Client class "{client_class}" set in subnet "{subnet}" but does not exist')

if client_class not in dhcp.get('client_class', {}):

raise ConfigError(f'Client class "{client_class}" set in subnet "{subnet}" but does not exist')

This part can by simplified

natali-rs1985 · 2025-10-22T11:31:21Z

src/conf_mode/service_dhcp-server.py

+                        if 'client_class' not in dhcp:
+                            raise ConfigError(f'Client class "{client_class}" set in range "{range}" but does not exist')
+
+                        if client_class not in dhcp['client_class'].keys():
+                            raise ConfigError(f'Client class "{client_class}" set in range "{range}" but does not exist')


Suggested change

if 'client_class' not in dhcp:

raise ConfigError(f'Client class "{client_class}" set in range "{range}" but does not exist')

if client_class not in dhcp['client_class'].keys():

raise ConfigError(f'Client class "{client_class}" set in range "{range}" but does not exist')

if client_class not in dhcp.get('client_class', {}):

raise ConfigError(f'Client class "{client_class}" set in range "{range}" but does not exist')

The same is here

This commit adds support in both the CLI and the underlying code for DHCP Option 82 to be used to filter/route DHCP address assignments. The primary use case for this is to support enterprise switches which can "tag" DHCP requests with physical real world informaiton such as which switch first saw the request and which port it originated from (known in this context as remote-id and circuit-id). Once client-classes have been defined they can be assigned to subnets or ranges so that only certain addresses get assigned to specific requests. There is also a corresponding documentation update which pairs with this code change. (cherry picked from commit 326b5e7)

sarthurdev · 2025-10-25T22:31:36Z

Cleaned up commits and squashed

github-actions · 2025-10-26T00:05:22Z

CI integration 👍 passed!

Details

CI logs

CLI Smoketests (no interfaces) 👍 passed
CLI Smoketests VPP 👍 passed
CLI Smoketests (interfaces only) 👍 passed
Config tests 👍 passed
Config tests VPP 👍 passed
RAID1 tests 👍 passed
TPM tests 👍 passed

github-actions bot assigned cblackburn-igl Aug 17, 2025

github-actions bot added current conflicts labels Aug 17, 2025

cblackburn-igl closed this Aug 28, 2025

cblackburn-igl force-pushed the T3936-DHCP-Option-82-Support branch from 326b5e7 to 133cb0c Compare August 28, 2025 14:49

cblackburn-igl reopened this Aug 28, 2025

github-actions bot removed the conflicts label Aug 28, 2025

sever-sever requested review from c-po, dmbaturin and sarthurdev August 31, 2025 05:44

dmbaturin requested changes Sep 15, 2025

View reviewed changes

github-actions bot added the rebase label Sep 15, 2025

cblackburn-igl requested a review from dmbaturin September 28, 2025 14:39

cblackburn-igl force-pushed the T3936-DHCP-Option-82-Support branch from 65b020b to 43538d7 Compare September 28, 2025 22:04

sarthurdev reviewed Sep 29, 2025

View reviewed changes

interface-definitions/include/dhcp/dhcp-server-common-config.xml.i Outdated Show resolved Hide resolved

dmbaturin requested changes Oct 1, 2025

View reviewed changes

interface-definitions/include/dhcp/dhcp-server-common-config.xml.i Outdated Show resolved Hide resolved

interface-definitions/include/dhcp/dhcp-server-common-config.xml.i Outdated Show resolved Hide resolved

sever-sever requested a review from Copilot October 1, 2025 21:00

Copilot AI reviewed Oct 1, 2025

View reviewed changes

src/conf_mode/service_dhcp-server.py Show resolved Hide resolved

smoketest/scripts/cli/test_service_dhcp-server.py Show resolved Hide resolved

python/vyos/kea.py Outdated Show resolved Hide resolved

python/vyos/kea.py Outdated Show resolved Hide resolved

sever-sever requested a review from natali-rs1985 October 22, 2025 11:00

natali-rs1985 requested changes Oct 22, 2025

View reviewed changes

sarthurdev force-pushed the T3936-DHCP-Option-82-Support branch from 093340b to 2e86aea Compare October 25, 2025 22:31

	<help>Filters on the contents of the remote-id sub option.</help>
	<help>Filters on the contents of the remote-id sub option</help>

Uh oh!

dhcp-server: T3936: Added support for DHCP Option 82 #4665

Are you sure you want to change the base?

dhcp-server: T3936: Added support for DHCP Option 82 #4665

Conversation

cblackburn-igl commented Aug 17, 2025

Change summary

Types of changes

Related Task(s)

Related PR(s)

How to test / Smoketest result

Checklist:

Uh oh!

github-actions bot commented Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 19, 2025

Uh oh!

cblackburn-igl commented Aug 19, 2025

Uh oh!

cblackburn-igl commented Aug 28, 2025

Uh oh!

github-actions bot commented Aug 28, 2025

Uh oh!

dmbaturin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sarthurdev commented Sep 29, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cblackburn-igl commented Oct 2, 2025

Uh oh!

sever-sever commented Oct 2, 2025

Uh oh!

cblackburn-igl commented Oct 5, 2025

Uh oh!

dmbaturin commented Oct 9, 2025

Uh oh!

natali-rs1985 left a comment

Choose a reason for hiding this comment

Uh oh!

natali-rs1985 Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

natali-rs1985 Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

natali-rs1985 Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

sarthurdev commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 26, 2025

Details

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Aug 17, 2025 •

edited

Loading

sarthurdev commented Oct 25, 2025 •

edited

Loading