dhcp-server: T3936: Added support for DHCP Option 82 #4654
Conversation
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.
|
❌ |
python/vyos/kea.py
Outdated
| inet, | ||
| ip_address, | ||
| host_name=None, | ||
| mac_address=None, | ||
| iaid=None, | ||
| duid=None, | ||
| subnet_id=None, |
There was a problem hiding this comment.
Why did you touch this code?
The indents were correct.
There was a problem hiding this comment.
I honestly didn't mean to. It must have been done automatically by the autoformatter. I can't remember using it but I must have out of muscle memory. I'll see if I can work out how to undo it in the PR. I might have to cancel and resubmit.
python/vyos/kea.py
Outdated
| data_lease['remaining'] != '' | ||
| and data_lease['pool'] in pools | ||
| and data_lease['state'] != 'free' | ||
| and (not state or state == 'all' or data_lease['state'] in state) |
There was a problem hiding this comment.
Why did you touch this code?
The indents were correct.
There was a problem hiding this comment.
I honestly didn't mean to. It must have been done automatically by the autoformatter. I can't remember using it but I must have out of muscle memory. I'll see if I can work out how to undo it in the PR. I might have to cancel and resubmit.
| <children> | ||
| <leafNode name="circuit-id"> | ||
| <properties> | ||
| <help>Filters on the contents of the circuit-id sub option. Assumes ASCII text unless input starts with 0x in which case it is interpreted as raw hex</help> |
There was a problem hiding this comment.
This help string is way too long. To let the user know that both text and hex values are accepted, I'd use <valueHelp> tags like in here in VRRP.
| </leafNode> | ||
| <leafNode name="remote-id"> | ||
| <properties> | ||
| <help>Filters on the contents of the remote-id sub option. Assumes ASCII text unless input starts with 0x in which case it is interpreted as raw hex</help> |
There was a problem hiding this comment.
Same issue as above — a very long string that should be split into a short help string and value help items.
python/vyos/kea.py
Outdated
|
|
||
| return out | ||
|
|
||
|
|
There was a problem hiding this comment.
To add to @sever-sever's review: please do not mix new logic and formatting changes.
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. Co-Authored-By: Daniil Baturin <[email protected]>
cblackburn-igl
force-pushed
the
origin/T3936-DHCP-Option-82-Support
branch
from
aeb5878 to
59b9ceb
Compare
|
Why is this line included, I mean what is the purpose? |
…ort' into origin/T3936-DHCP-Option-82-Support # Conflicts: # interface-definitions/service_dhcp-server.xml.in
cblackburn-igl
force-pushed
the
origin/T3936-DHCP-Option-82-Support
branch
from
7dae904 to
487d5fe
Compare
|
ok, despite wrestling for hours with Git I can't find a way to rebase this PR down into a single commit now because it contains a merge. I'm going to have to close and abandon this pull request and recreae in a new branch which is squashed down to one commit. I will then create a new pull request |
|
CI integration 👍 passed! Details
|
Change summary
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 information 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
Related Task(s)
https://vyos.dev/T3936
Related PR(s)
vyos/vyos-documentation#1666
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: