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

LLT-5886: Protected RTT QoS pings #1058

Merged
merged 1 commit into from
Jan 21, 2025
Merged

LLT-5886: Protected RTT QoS pings #1058

merged 1 commit into from
Jan 21, 2025

Conversation

tomasz-grz
Copy link
Contributor

@tomasz-grz tomasz-grz commented Jan 13, 2025

Problem

Due to the nature of Apple's NetworkExtension control protocol, packets originating from the NetworkExtension process are routed outside of the tunnel.
Because of this on macOS AppStore and iOS platforms, raw ICMP packets from QoS RTT and Link detection components destined for the tunnel interface, leave the main interface instead.

Solution

Use the SocketPool component in order to bind the ping raw socket to the tunnel interface. The Pinger component had to moved into it's own crate in order to avoid circular dependencies between Utils and Sockets.
LinkDetection component was also updated to use the new Pinger.

☑️ Definition of Done checklist

  • Commit history is clean (requirements)
  • README.md is updated
  • Functionality is covered by unit or integration tests

@tomasz-grz tomasz-grz self-assigned this Jan 13, 2025
@tomasz-grz tomasz-grz marked this pull request as ready for review January 15, 2025 12:07
@tomasz-grz tomasz-grz requested a review from a team as a code owner January 15, 2025 12:07
@tomasz-grz tomasz-grz force-pushed the protected_pinger branch 2 times, most recently from 7312c7f to 24ab2d8 Compare January 15, 2025 14:34
Copy link
Contributor

@LukasPukenis LukasPukenis left a comment

Choose a reason for hiding this comment

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

Very nice PR.

I checked out the telio-sockets/protector/* and it doesn't contain the information of what it does except the apple.rs which mentions what it tries to do.

While we're at it, would it be possible to add a brief comment into protector.rs about it's purpose?

Anyway, that's not a blocker :) +1

@tomasz-grz
Copy link
Contributor Author

Also added rust docs to the protector module.

In order to add SocketPool, Pinger had to be broken out of telio-utils
because of circular dependencies. LinkDetection had to be also updated
to use the new Pinger.
Copy link
Contributor

@Jauler Jauler left a comment

Choose a reason for hiding this comment

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

+1.0

@tomasz-grz tomasz-grz merged commit 3e6b4f4 into main Jan 21, 2025
64 checks passed
@tomasz-grz tomasz-grz deleted the protected_pinger branch January 21, 2025 13:23
@tomasz-grz tomasz-grz restored the protected_pinger branch January 23, 2025 10:56
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