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

backend/vxlan: Add support for "direct routing" #787

Merged
merged 1 commit into from
Sep 6, 2017

Conversation

tomdee
Copy link
Contributor

@tomdee tomdee commented Jul 31, 2017

This skips vxlan encapsulation if the hosts are on the same subnet

@tomdee tomdee mentioned this pull request Aug 7, 2017
2 tasks
@tomdee tomdee requested a review from gunjan5 August 14, 2017 20:45
@tomdee tomdee force-pushed the vxlan-direct-routing branch 2 times, most recently from 74b4346 to 802887f Compare August 14, 2017 21:18
This skips vxlan encapsulation if the hosts are on the same subnet
Copy link
Contributor

@gunjan5 gunjan5 left a comment

Choose a reason for hiding this comment

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

a question about cleanup during subnet.EventRemoved

continue
}
if len(routes) == 1 && routes[0].Gw == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

wonder if you can use any of netlink.Route's attributes to determine if it's a directly connected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't that what I'm doing with my .Gw == nil test?

I could also check to see if the FLAG_ONLINK flag is set on the flags attribute if you think that would be better?

Copy link
Contributor

Choose a reason for hiding this comment

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

that would be nice but not necessary since your comments are pretty clear

continue
}
if len(routes) == 1 && routes[0].Gw == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

how would this condition set directRoutingOK to true for subnet.EventRemoved event? maybe I'm missing something, but when we get the EventAdded we program the directRoute which has Gw: attrs.PublicIP.ToIP(), then we get EventRemoved for the same one and if we check this condition where we see if Gw==nil, so the clean up in L173 won't happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The directRoutingOK check is seeing if there's a direct route to the public IP address of other host. If there is, then we add a route to the IP range for that host using the public IP address as the gateway. The "route" to the public IP address for that host is unchanged; it's still directly connected.

So when the remove comes in, the public IP is still directly connected and we can remove the route to the network which uses the public IP as the gateway. Make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah that makes sense, I misunderstood the direct link checking logic and thought directRoutingOk flag will stay false for delete, but it doesn't, so all good now :)

Copy link
Contributor

@gunjan5 gunjan5 left a comment

Choose a reason for hiding this comment

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

LGTM!

@tomdee tomdee merged commit cde204d into flannel-io:master Sep 6, 2017
@tomdee tomdee deleted the vxlan-direct-routing branch September 6, 2017 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants