-
Notifications
You must be signed in to change notification settings - Fork 2
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
ddm tunnel routing #77
Conversation
2684168
to
1f29a8a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny nit: most (all?) of the files have 2022 Copyrights that should be updated.
I pointed out a few specific examples, but in general there are a lot of pub struct
types that could use top-level and per-field comments.
It seems like there is a fair bit of boilerplate code converting between structs that are identical, other than their names (e.g., TunnelVector
and TunnelOrigin
). It's not clear to me why we're not just sharing a common structure.
dd10dc5
to
f2fc0dd
Compare
49bc8f2
to
295107c
Compare
1b20200
to
88b33ab
Compare
This has been fixed. |
TunnelVector has been squashed into TunnelOrigin |
88b33ab
to
4c749fd
Compare
@Nieuwejaar this is ready for another look. The primary things that have changed since you last looked are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like I've only been able to do a tree-level review of this, and it looks good. There's enough here, and it's been long enough since my first look, that I want to revisit the RFD and make another pass to make sure I also have a forest-level view.
Introduced tunnel routing as described in RFD 404. It's strongly encouraged to read that short RFD before reviewing this PR.
Depends on
The primary thing this PR does is add a new tunnel route type and associated advertisement mechanics. Support for adding tunnel routes to OPTE from ddmd is also added.