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

Implement DHCPv6 Server Identifier Generation #46

Merged
merged 20 commits into from
Nov 27, 2023
Merged

Implement DHCPv6 Server Identifier Generation #46

merged 20 commits into from
Nov 27, 2023

Conversation

chenwanqq
Copy link
Contributor

Hi, I have been working on a plan to enhance DHCPv6 support of this project for a long time. The generation of Server Identifier is the first step of this plan, and also the very first step of a DHCPv6 serving process.

I got some inspiration from kea, a high-performance DHCP server engine, and I implemented the support of 4 types of server identifier: LLT, LL, EN and DUID. Some major points of my codes are:
(1) The definition of certain configuration of server identifier. These can be found in various config yaml files I added.
(2) The fix of finding the local link layer address if we never define the interfaces in config.
(3) The implementation of reusing the DUID generated at the first time DHCPv6 is used. The DUID will only be reused if the configuration has not changed since the last time.
(4) Some tests.

@chenwanqq chenwanqq changed the title Implement server_id generation Implement DHCPv6 Server Identifier Generation Nov 18, 2023
Copy link
Contributor Author

@chenwanqq chenwanqq left a comment

Choose a reason for hiding this comment

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

If DUID-EN identifier not specified, will randomly generate a 6-byte-long identifier, like Kea

@chenwanqq
Copy link
Contributor Author

Some tests I added that involve adding and deleting local files (/var/lib/dora/server_id), which cannot pass the workflow test. Do I need to modify them?

@leshow
Copy link
Collaborator

leshow commented Nov 20, 2023

Great! I'll take a more thorough look tomorrow but yes I would like to get the tests to pass in the workflow.

Copy link
Collaborator

@leshow leshow left a comment

Choose a reason for hiding this comment

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

Thanks so much for working on this!

libs/config/sample/config_v6_LL.yaml Outdated Show resolved Hide resolved
libs/config/src/lib.rs Outdated Show resolved Hide resolved
libs/config/src/lib.rs Outdated Show resolved Hide resolved
example.yaml Outdated Show resolved Hide resolved
libs/config/src/v6.rs Outdated Show resolved Hide resolved
libs/config/src/v4.rs Outdated Show resolved Hide resolved
libs/config/src/wire/v6.rs Show resolved Hide resolved
example.yaml Outdated Show resolved Hide resolved
libs/config/src/v6.rs Outdated Show resolved Hide resolved
libs/config/src/lib.rs Outdated Show resolved Hide resolved
@chenwanqq
Copy link
Contributor Author

chenwanqq commented Nov 21, 2023

OK, I will modify them(a lot…) However kind of busy in other things in recent few days🤪

@chenwanqq
Copy link
Contributor Author

Thank you so much for these reviews in detail🙏

@leshow
Copy link
Collaborator

leshow commented Nov 21, 2023

No rush! Thanks again for picking this up

@chenwanqq
Copy link
Contributor Author

  • I have made some changes to the code and submitted a few commits in the past few days. These changes are quite big, and I need to check them more carefully. It would be better to review them after I finish checking. Thank you very much for your review!

@chenwanqq chenwanqq marked this pull request as draft November 25, 2023 16:35
@chenwanqq chenwanqq marked this pull request as ready for review November 26, 2023 09:03
@chenwanqq
Copy link
Contributor Author

Maybe.. LGTM?

@leshow
Copy link
Collaborator

leshow commented Nov 27, 2023

LGTM. I may push a few small changes on top of the branch before merging if that's OK.

I want to thank you again for your persistence in working on this. When the v6 changes in dhcproto we can make the update here also. I can take care of that if you want to work on something more exciting, have you thought about what to look at next for v6?

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 71.73% (-0.6%) from 72.319%
when pulling d2dac96 on chenwanqq:ipv6
into f510bcb on bluecatengineering:master.

@chenwanqq
Copy link
Contributor Author

Yeah. If v6 Htype is updated in dhcproto, there is also a part that needs to be modified here. I will add TODO tags in the corresponding places.
The next plan is to continue to improve the various functions of DHCPv6, and I think the first step is to add appropriate test support, such as adding a blank_ctx_v6. Still thinking for it!

@leshow
Copy link
Collaborator

leshow commented Nov 27, 2023

I pushed some small changes, thanks again!

@leshow leshow merged commit cd4e175 into bluecatengineering:master Nov 27, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants