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

WIP: multi source port #497

Closed
wants to merge 4 commits into from
Closed

Conversation

demmer
Copy link

@demmer demmer commented Jul 15, 2021

Description

WIP: send outbound packets using one of N bound source ports

Motivation

The goal of this work is to send packets between two hosts using more than one 5-tuple. When running on networks like AWS where the underlying network driver and overlay fabric makes routing, load balancing, and failover decisions based on the flow hash, this enables more than one flow between pairs of hosts.

Proposal

With this change, Nebula will optionally bind to multiple additional UDP sockets that are only used for sending, not for receiving.

Add a new optional configuration "send.ports" which specifies how many ports to use per thread for sending (default is 1). Notably, the destination port is always fixed for a given host.

If this is set to a value > 1, then nebula binds to additional sockets and then picks the one to send based on a "hash" of source of the src and dst ports. This is deliberately just a random scrambling of the bits, I'm sure there's something better we could do.

After some internal testing revealed that the original commit triggered the roaming behavior, I also added a modification to hostDidRoam so that it ignores the ports and only pays attention to the IP addresses.

After some more discussion I thought of a different approach: Tag packets sent from the "non-canonical" interface, i.e. using a source port that the sender is not listening on, with a subtype that indicates to the receiver that it should ignore packet for roaming detection.

Caveats

This PR is the first time I've ever touched the nebula code, I have only a cursory understanding of the implications of this change on the handshaking / roaming / etc.

In particular, I have absolutely 0 understanding of roaming or what my change does or would do.
I now kinda maybe get this a bit.

@CLAassistant
Copy link

CLAassistant commented Jul 15, 2021

CLA assistant check
All committers have signed the CLA.

wadey added a commit that referenced this pull request Apr 27, 2022
We are currently testing changes for multiport (related to #497) that
use fields 6 and 7 in the protobuf. Reserved these fields so that when
we eventually open the PR we are backwards compatible with any future
changes.
wadey added a commit that referenced this pull request Jun 27, 2022
We are currently testing changes for multiport (related to #497) that
use fields 6 and 7 in the protobuf. Reserved these fields so that when
we eventually open the PR we are backwards compatible with any future
changes.
@wadey
Copy link
Member

wadey commented Oct 17, 2022

We reworked this PR into #768, thus I'm closing this older one.

@wadey wadey closed this Oct 17, 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.

3 participants