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

v4-over-v6 #56

Merged
merged 10 commits into from
Mar 31, 2022
Merged

v4-over-v6 #56

merged 10 commits into from
Mar 31, 2022

Conversation

tobast
Copy link
Contributor

@tobast tobast commented Apr 1, 2020

This merge request implements v4-over-v6.

Copy link
Contributor

@MisterDA MisterDA left a comment

Choose a reason for hiding this comment

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

Hi! Some little suggestions, mostly on the code style, so nothing important.

message.c Outdated Show resolved Hide resolved
message.c Outdated Show resolved Hide resolved
rc = network_prefix(message[2], message[4], message[5],
message + 12,
message[2] == 1 ? v4_prefix : v6_prefix,
(message[2] == AE_IPV4 ? v4_prefix :
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use parentheses surrounding the ternary expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly for visual grouping, as this spans on multiple lines. If you'd rather not have those, I could remove it.

message.c Show resolved Hide resolved
message.c Outdated Show resolved Hide resolved
Copy link
Contributor Author

@tobast tobast left a comment

Choose a reason for hiding this comment

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

Thanks for the review! I'm not yet super clear with Babel coding style, and my C might be a bit rusty :)

Except on the commented bit, I agree with everything and will make the changes.

@tobast tobast force-pushed the tobast/v6tethered-reimplem branch from 9776642 to 43ac425 Compare April 4, 2020 10:32
@tobast tobast force-pushed the tobast/v6tethered-reimplem branch 5 times, most recently from dbb13a4 to 6b7bfd5 Compare April 19, 2020 11:00
@tobast tobast force-pushed the tobast/v6tethered-reimplem branch from 6b7bfd5 to 6ffcc56 Compare April 20, 2020 09:19
util.c Outdated
int
ae_is_v4(int ae)
{
return ae == 1 || ae == 240;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren’t you using the AE_ constants here? If you’re only using ae_is_v4 in message.c you could put the function there, and you wouldn’t have to include message.h or to put the constant definitions in a weird place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not using it anywhere else, but this looks like a function that might be needed somewhere else later on. I might be wrong here, though.

I did that a while ago, but I think that including message.h in util broke things and there was no trivial way to fix this, so it seemed easier to just do it this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t have a hard opinion on this, though it feels wrong not to use the AE_ constants everywhere.

that including message.h in util broke things

Does not surprise me 🙂 :

Copy link
Owner

Choose a reason for hiding this comment

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

This simply indicates that this function doesn't belong in util. Util is for generic utility functions that are not closely tied to Babel.

@MisterDA
Copy link
Contributor

Oh, you can also add yourself to the CREDITS file!

tobast added 10 commits May 14, 2020 15:34
Detection is based on the kernel version, and can be overridden by
the configuration option `v4-over-v6 {true|false}`. If v4-over-v6 is not
supported, ignores v4-over-v6 routes early.
In the following topology

A --[v4]-- B --[v6]-- C

if B does not support v4ov6 (eg. because of an old kernel), it can
nevertheless announce a v4-over-v6 route to C, since the route in its
own routing table is pure v4.
@tobast tobast force-pushed the tobast/v6tethered-reimplem branch from 46ca871 to a4aebc7 Compare May 14, 2020 13:37
@jech jech merged commit 3646ba9 into jech:master Mar 31, 2022
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