wgengine/magicsock: keep advertising endpoints after we stop discovering them #7877
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danderson
approved these changes
Apr 14, 2023
Member
danderson
left a comment
danderson
left a comment
There was a problem hiding this comment.
The data structures in endpointTracker feel like they could be more efficient, but I couldn't work out the exact shape of them during code review, and that's easy to improve later on. LGTM.
bradfitz
reviewed
Apr 14, 2023
bradfitz
reviewed
Apr 14, 2023
andrew-d
added a commit
that referenced
this pull request
Apr 14, 2023
For use in #7877 Signed-off-by: Andrew Dunham <andrew@du.nham.ca> Change-Id: I712d09c6d1a180c6633abe3acf8feb59b27e2866
andrew-d
added a commit
that referenced
this pull request
Apr 14, 2023
This is an exact copy of the files misc/set/set{,_test}.go from corp as
of the time this commit was created, plus the license headers.
For use in #7877
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I712d09c6d1a180c6633abe3acf8feb59b27e2866
andrew-d
force-pushed
the
andrew/endpoints-advertise-timeout
branch
from
a360f19 to
057605e
Compare
andrew-d
added a commit
that referenced
this pull request
Apr 14, 2023
This is an exact copy of the files misc/set/set{,_test}.go from
tailscale/corp@a5415da, plus the
license headers.
For use in #7877
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I712d09c6d1a180c6633abe3acf8feb59b27e2866
andrew-d
added a commit
that referenced
this pull request
Apr 14, 2023
This is an exact copy of the files misc/set/set{,_test}.go from
tailscale/corp@a5415da, plus the
license headers.
For use in #7877
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I712d09c6d1a180c6633abe3acf8feb59b27e2866
andrew-d
force-pushed
the
andrew/endpoints-advertise-timeout
branch
2 times, most recently
from
bb6c04e to
531e20a
Compare
bradfitz
reviewed
Apr 15, 2023
bradfitz
approved these changes
Apr 15, 2023
…ing them Previously, when updating endpoints we would immediately stop advertising any endpoint that wasn't discovered during determineEndpoints. This could result in, for example, a case where we performed an incremental netcheck, didn't get any of our three STUN packets back, and then dropped our STUN endpoint from the set of advertised endpoints... which would result in clients falling back to a DERP connection until the next call to determineEndpoints. Instead, let's cache endpoints that we've discovered and continue reporting them to clients until a timeout expires. In the above case where we temporarily don't have a discovered STUN endpoint, we would continue reporting the old value, then re-discover the STUN endpoint again and continue reporting it as normal, so clients never see a withdrawal. Updates tailscale/coral#108 Signed-off-by: Andrew Dunham <andrew@du.nham.ca> Change-Id: I42de72e7418ab328a6c732bdefc74549708cf8b9
andrew-d
force-pushed
the
andrew/endpoints-advertise-timeout
branch
from
531e20a to
4760e84
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, when updating endpoints we would immediately stop advertising any endpoint that wasn't discovered during
determineEndpoints. This could result in, for example, a case where we performed an incremental netcheck, didn't get any of our three STUN packets back, and then dropped our STUN endpoint from the set of advertised endpoints... which would result in clients falling back to a DERP connection until the next call todetermineEndpoints.Instead, let's cache endpoints that we've discovered and continue reporting them to clients until a timeout expires. In the above case where we temporarily don't have a discovered STUN endpoint, we would continue reporting the old value, then re-discover the STUN endpoint again and continue reporting it as normal, so clients never see a withdrawal.
Updates tailscale/coral#108
Change-Id: I42de72e7418ab328a6c732bdefc74549708cf8b9