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

Log if Same IP is being used or not #5236

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

faern
Copy link
Member

@faern faern commented Oct 5, 2023

Desktop+Android equivalent of #5228. Logs when the known Same IP IPs are being used for a WireGuard tunnel. Should aid in debugging any issues that might arise during the rollout of Same IP.


This change is Reviewable

@faern faern requested a review from dlon October 5, 2023 08:08
@linear
Copy link

linear bot commented Oct 5, 2023

DES-386 Log when Same IP is being used

According to the latest plans with how we roll out Same IP, the app will not know about it at all. The API will just start handing out Same IP IPs to apps. But since there is substantial risk of bugs and issues with it (tricky kernel changes server side) we need good ways of debugging this.

Currently if the app sends a problem report, it's not possible/easy to figure out if the user has been assigned Same IP or not. The logs do not contain account or device identifiers, nor IPs. It would be much easier for us to debug Same IP issues if app logs could reveal whether or not Same IP was used.

Solution

Hardcode the currently known Same IP IPs. And if the device has those IPs assigned to it, log on each connection attempt that Same IP is being used.

if device.ips.contains(SAME_IP_IP) {
    log::debug!("Same IP is being used");
}

This will likely be removed a while after the Same IP rollout reaches 100% and that should be noted in the code.

@faern faern force-pushed the log-when-same-ip-is-being-used-des-386 branch from 52f382c to dea72b8 Compare October 5, 2023 08:23
@faern faern force-pushed the log-when-same-ip-is-being-used-des-386 branch from dea72b8 to 2d7ff6c Compare October 5, 2023 08:37
Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @faern)


mullvad-daemon/src/tunnel.rs line 33 at r2 (raw file):

/// peer.
static SAME_IP_V4: Lazy<IpAddr> =
    Lazy::new(|| Ipv4Addr::from_str("10.127.255.254").unwrap().into());

Nit: Might be a bit nicer to just use Ipv{4,6}Addr::new here since they're const.

@faern faern merged commit 226134e into main Oct 5, 2023
@faern faern deleted the log-when-same-ip-is-being-used-des-386 branch October 5, 2023 09:10
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.

2 participants