Skip to content

Conversation

@valscale
Copy link
Contributor

@valscale valscale commented Oct 4, 2023

disco,net/tstun,wgengine/magicsock: probe and record peer MTU

Automatically probe and record the path MTU to a peer when peer MTU is enabled,
but do not use the MTU information for anything other than metrics yet.

Updates #311

@valscale valscale requested review from icio, knyar and raggi October 4, 2023 14:28
@valscale
Copy link
Contributor Author

valscale commented Oct 4, 2023

I think this could probably use more (any) tests but wanted to get this out there for review while I worked on them.

Copy link
Contributor

@icio icio left a comment

Choose a reason for hiding this comment

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

init stuff looks good to me ☺️

Copy link
Contributor

@knyar knyar left a comment

Choose a reason for hiding this comment

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

Looks good overall, I would suggest moving out of draft after you address pending comments.

@valscale valscale marked this pull request as ready for review October 5, 2023 18:32
@valscale valscale force-pushed the valscale/probeMTU branch 4 times, most recently from b72bddd to 8827cc3 Compare October 6, 2023 15:05
@valscale
Copy link
Contributor Author

valscale commented Oct 6, 2023

Update: I removed the entire concept of max probed MTU from this PR since that won't be necessary until the PTB generation PR. I removed any modification of the MTUs to probe - it's such a tiny optimization and we can add it later if necessary. And I split the current usage of default TUN MTU into "what to set the tailscale TUN to" and "what is the safe value to assume that the wire MTU is if we have no information about the path."

Automatically probe the path MTU to a peer when peer MTU is enabled, but do not
use the MTU information for anything yet.

Updates #311

Signed-off-by: Val <valerie@tailscale.com>
Record the number of MTU probes sent, the total bytes sent, the number of times
we got a successful return from an MTU probe of a particular size, and the max
MTU recorded.

Updates #311

Signed-off-by: Val <valerie@tailscale.com>
@valscale
Copy link
Contributor Author

valscale commented Oct 7, 2023

Quick update to use @raggi's nice new syncs.LoadOrInit().

@valscale valscale requested a review from raggi October 7, 2023 06:14
@valscale valscale merged commit 249edaa into main Oct 9, 2023
@valscale valscale deleted the valscale/probeMTU branch October 9, 2023 08:57
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.

5 participants