dhcp-server: T3936: Added support for DHCP Option 82 #4665
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.
|
❌ |
|
❌ Conflicts Found. This pull request has conflicts. Please resolve them before we can evaluate the pull request. |
|
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? |
|
As I've not heard anything about this I'm going to assume that I can continue with multiple commits in the PR. |
cblackburn-igl
force-pushed
the
T3936-DHCP-Option-82-Support
branch
from
326b5e7 to
133cb0c
Compare
…ackburn-igl/vyos-1x into T3936-DHCP-Option-82-Support
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
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
interface-definitions/include/dhcp/dhcp-server-common-config.xml.i
Outdated
Show resolved
Hide resolved
…ml.i Co-authored-by: Daniil Baturin <[email protected]>
…ml.i Co-authored-by: Daniil Baturin <[email protected]>
Co-authored-by: Daniil Baturin <[email protected]>
…a bug so that was corrected.
cblackburn-igl
force-pushed
the
T3936-DHCP-Option-82-Support
branch
from
65b020b to
43538d7
Compare
interface-definitions/include/dhcp/dhcp-server-common-config.xml.i
Outdated
Show resolved
Hide resolved
|
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 |
Some platforms do not have `vendor_id` for the CPU information This causes of `KeyError: 'vendor_id'` errors while commiting system option kernel memory settings. Fix this.
Move encrypted volume check before key input Unmount any conflicting config bind mounts
write_file_sync does an explicit fsync of file and directory on write. write_file_atomic calls write_file_sync on a temp file and uses os.rename to provide an 'atomic' write.
Config save is provided by the helper script in both CLI and configsession (hence also in the http api). Use utilities write_file_sync and write_file_atomic, in accordance with permissions and location: If the target is in /opt/vyatta/etc/config or /config, use write_file_sync; if, moreover, the caller has permissions, use write_file_atomic. Otherwise, fall back to util write_file.
Take advantage of the helper script in the vyconf context.
Check that write_file_sync reaches disk, by using util vmtouch to evict cache.
After commit 85fe32f ("bgp: T7760: remove per vrf instance system-as node") BGP isntances running in a VRF will no longer have a system-as node set. This results in "set vrf name <name> protocols bgp" becomeing a valid CLI path. When reading in the config dict - we now might see {'protocols': {'bgp': {}} as a valid entry. We do need to account for this empty dictionary.
Ensures rendered FRR config passes sanity checks before applying. Prevents issues like T7089 where bad template syntax broke routing. Improves robustness and minimizes risk of config-induced outages.
…a bug so that was corrected.
…ml.i Co-authored-by: Daniil Baturin <[email protected]>
Co-authored-by: Copilot <[email protected]>
…ackburn-igl/vyos-1x into T3936-DHCP-Option-82-Support
|
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. |
|
CI integration 👍 passed! Details
|
|
I see 68 commits and most of them unexpected |
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
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: