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.

Sign up for GitHub

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

Fix reliability and time consumption of testInitialConnectionTimings test #5160

Merged
merged 1 commit into from
Sep 19, 2023
Merged

Fix reliability and time consumption of testInitialConnectionTimings test #5160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions ios/MullvadVPN.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,16 +393,17 @@
7A11DD0B2A9495D400098CD8 /* AppRoutes.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5802EBC42A8E44AC00E5CE4C /* AppRoutes.swift */; };
7A1A26432A2612AE00B978AA /* PaymentAlertPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A1A26422A2612AE00B978AA /* PaymentAlertPresenter.swift */; };
7A21DACF2A30AA3700A787A9 /* UITextField+Appearance.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A21DACE2A30AA3700A787A9 /* UITextField+Appearance.swift */; };
7A2960F62A963F7500389B82 /* AlertCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A2960F52A963F7500389B82 /* AlertCoordinator.swift */; };
7A2960FD2A964BB700389B82 /* AlertPresentation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A2960FC2A964BB700389B82 /* AlertPresentation.swift */; };
7A307AD92A8CD8DA0017618B /* Duration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A307AD82A8CD8DA0017618B /* Duration.swift */; };
7A307ADB2A8F56DF0017618B /* Duration+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A307ADA2A8F56DF0017618B /* Duration+Extensions.swift */; };
7A33538F2AA9FF1600F0A71C /* SimulatorTunnelProviderManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A33538E2AA9FF1600F0A71C /* SimulatorTunnelProviderManager.swift */; };
7A3353912AAA014400F0A71C /* SimulatorVPNConnection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A3353902AAA014400F0A71C /* SimulatorVPNConnection.swift */; };
7A3353932AAA089000F0A71C /* SimulatorTunnelInfo.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A3353922AAA089000F0A71C /* SimulatorTunnelInfo.swift */; };
7A3353972AAA0F8600F0A71C /* OperationBlockObserverSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A3353962AAA0F8600F0A71C /* OperationBlockObserverSupport.swift */; };
7A2960F62A963F7500389B82 /* AlertCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A2960F52A963F7500389B82 /* AlertCoordinator.swift */; };
7A2960FD2A964BB700389B82 /* AlertPresentation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A2960FC2A964BB700389B82 /* AlertPresentation.swift */; };
7A42DEC92A05164100B209BE /* SettingsInputCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A42DEC82A05164100B209BE /* SettingsInputCell.swift */; };
7A42DECD2A09064C00B209BE /* SelectableSettingsCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A42DECC2A09064C00B209BE /* SelectableSettingsCell.swift */; };
7A6B4F592AB8412E00123853 /* TunnelMonitorTimings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6B4F582AB8412E00123853 /* TunnelMonitorTimings.swift */; };
7A7AD28D29DC677800480EF1 /* FirstTimeLaunch.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */; };
7A7AD28F29DEDB1C00480EF1 /* SettingsHeaderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7AD28E29DEDB1C00480EF1 /* SettingsHeaderView.swift */; };
7A818F1F29F0305800C7F0F4 /* RootConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A818F1E29F0305800C7F0F4 /* RootConfiguration.swift */; };
Expand Down Expand Up @@ -1318,16 +1319,17 @@
7A0C0F622A979C4A0058EFCE /* Coordinator+Router.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Coordinator+Router.swift"; sourceTree = "<group>"; };
7A1A26422A2612AE00B978AA /* PaymentAlertPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentAlertPresenter.swift; sourceTree = "<group>"; };
7A21DACE2A30AA3700A787A9 /* UITextField+Appearance.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "UITextField+Appearance.swift"; sourceTree = "<group>"; };
7A2960F52A963F7500389B82 /* AlertCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlertCoordinator.swift; sourceTree = "<group>"; };
7A2960FC2A964BB700389B82 /* AlertPresentation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlertPresentation.swift; sourceTree = "<group>"; };
7A307AD82A8CD8DA0017618B /* Duration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Duration.swift; sourceTree = "<group>"; };
7A307ADA2A8F56DF0017618B /* Duration+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Duration+Extensions.swift"; sourceTree = "<group>"; };
7A33538E2AA9FF1600F0A71C /* SimulatorTunnelProviderManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SimulatorTunnelProviderManager.swift; sourceTree = "<group>"; };
7A3353902AAA014400F0A71C /* SimulatorVPNConnection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SimulatorVPNConnection.swift; sourceTree = "<group>"; };
7A3353922AAA089000F0A71C /* SimulatorTunnelInfo.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SimulatorTunnelInfo.swift; sourceTree = "<group>"; };
7A3353962AAA0F8600F0A71C /* OperationBlockObserverSupport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OperationBlockObserverSupport.swift; sourceTree = "<group>"; };
7A2960F52A963F7500389B82 /* AlertCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlertCoordinator.swift; sourceTree = "<group>"; };
7A2960FC2A964BB700389B82 /* AlertPresentation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlertPresentation.swift; sourceTree = "<group>"; };
7A42DEC82A05164100B209BE /* SettingsInputCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsInputCell.swift; sourceTree = "<group>"; };
7A42DECC2A09064C00B209BE /* SelectableSettingsCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SelectableSettingsCell.swift; sourceTree = "<group>"; };
7A6B4F582AB8412E00123853 /* TunnelMonitorTimings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelMonitorTimings.swift; sourceTree = "<group>"; };
7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FirstTimeLaunch.swift; sourceTree = "<group>"; };
7A7AD28E29DEDB1C00480EF1 /* SettingsHeaderView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsHeaderView.swift; sourceTree = "<group>"; };
7A818F1E29F0305800C7F0F4 /* RootConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RootConfiguration.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2458,9 +2460,9 @@
58D1560C29C0B27600749324 /* Root */ = {
isa = PBXGroup;
children = (
587425C02299833500CA2045 /* RootContainerViewController.swift */,
58F3C0A3249CB069003E76BE /* HeaderBarView.swift */,
7A818F1E29F0305800C7F0F4 /* RootConfiguration.swift */,
587425C02299833500CA2045 /* RootContainerViewController.swift */,
);
path = Root;
sourceTree = "<group>";
Expand Down Expand Up @@ -2520,6 +2522,7 @@
582403162A821FD700163DE8 /* TunnelDeviceInfoProtocol.swift */,
58FC040927B3EE03001C21F0 /* TunnelMonitor.swift */,
58C7A42C2A85067A0060C66F /* TunnelMonitorProtocol.swift */,
7A6B4F582AB8412E00123853 /* TunnelMonitorTimings.swift */,
58A3BDAF28A1821A00C8C2C6 /* WgStats.swift */,
);
path = TunnelMonitor;
Expand Down Expand Up @@ -3735,6 +3738,7 @@
58C7A4552A863FB90060C66F /* TunnelMonitor.swift in Sources */,
58C7A4512A863FB50060C66F /* PingerProtocol.swift in Sources */,
58C7A4592A863FB90060C66F /* WgStats.swift in Sources */,
7A6B4F592AB8412E00123853 /* TunnelMonitorTimings.swift in Sources */,
58C7A4582A863FB90060C66F /* TunnelMonitorProtocol.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down
3 changes: 2 additions & 1 deletion ios/PacketTunnel/PacketTunnelProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
eventQueue: dispatchQueue,
pinger: Pinger(replyQueue: dispatchQueue),
tunnelDeviceInfo: WgAdapterDeviceInfo(adapter: adapter),
defaultPathObserver: PacketTunnelPathObserver(packetTunnelProvider: self)
defaultPathObserver: PacketTunnelPathObserver(packetTunnelProvider: self),
timings: TunnelMonitorTimings()
)
tunnelMonitor.onEvent = { [weak self] event in
self?.handleTunnelMonitorEvent(event)
Expand Down
76 changes: 24 additions & 52 deletions ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,6 @@ import MullvadTypes
import protocol Network.IPAddress
import struct Network.IPv4Address

/// Interval for periodic heartbeat ping issued when traffic is flowing.
/// Should help to detect connectivity issues on networks that drop traffic in one of directions,
/// regardless if tx/rx counters are being updated.
private let heartbeatPingInterval: Duration = .seconds(10)

/// Heartbeat timeout that once exceeded triggers next heartbeat to be sent.
private let heartbeatReplyTimeout: Duration = .seconds(3)

/// Timeout used to determine if there was a network activity lately.
private let trafficFlowTimeout: Duration = heartbeatPingInterval * 0.5

/// Ping timeout.
private let pingTimeout: Duration = .seconds(15)

/// Interval to wait before sending next ping.
private let pingDelay: Duration = .seconds(3)

/// Initial timeout when establishing connection.
private let initialEstablishTimeout: Duration = .seconds(4)

/// Multiplier applied to `establishTimeout` on each failed connection attempt.
private let establishTimeoutMultiplier: UInt32 = 2

/// Maximum timeout when establishing connection.
private let maxEstablishTimeout = pingTimeout

/// Connectivity check periodicity.
private let connectivityCheckInterval: Duration = .seconds(1)

/// Inbound traffic timeout used when outbound traffic was registered prior to inbound traffic.
private let inboundTrafficTimeout: Duration = .seconds(5)

/// Traffic timeout applied when both tx/rx counters remain stale, i.e no traffic flowing.
/// Ping is issued after that timeout is exceeded.s
private let trafficTimeout: Duration = .minutes(2)

public final class TunnelMonitor: TunnelMonitorProtocol {
/// Connection state.
private enum ConnectionState {
Expand Down Expand Up @@ -98,6 +62,9 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
/// Retry attempt.
var retryAttempt: UInt32 = 0

// Timings and timeouts.
let timings: TunnelMonitorTimings

func evaluateConnection(now: Date, pingTimeout: Duration) -> ConnectionEvaluation {
switch connectionState {
case .connecting:
Expand All @@ -112,17 +79,17 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
func getPingTimeout() -> Duration {
switch connectionState {
case .connecting:
let multiplier = establishTimeoutMultiplier.saturatingPow(retryAttempt)
let nextTimeout = initialEstablishTimeout * Double(multiplier)
let multiplier = timings.establishTimeoutMultiplier.saturatingPow(retryAttempt)
let nextTimeout = timings.initialEstablishTimeout * Double(multiplier)

if nextTimeout.isFinite, nextTimeout < maxEstablishTimeout {
if nextTimeout.isFinite, nextTimeout < timings.maxEstablishTimeout {
return nextTimeout
} else {
return maxEstablishTimeout
return timings.maxEstablishTimeout
}

case .pendingStart, .connected, .waitingConnectivity, .stopped, .recovering:
return pingTimeout
return timings.pingTimeout
}
}

Expand Down Expand Up @@ -163,7 +130,7 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
return .sendInitialPing
}

if now.timeIntervalSince(lastRequestDate) >= pingDelay {
if now.timeIntervalSince(lastRequestDate) >= timings.pingDelay {
return .sendNextPing
}

Expand All @@ -181,8 +148,8 @@ public final class TunnelMonitor: TunnelMonitorProtocol {

let timeSinceLastPing = now.timeIntervalSince(lastRequestDate)
if let lastReplyDate = pingStats.lastReplyDate,
lastRequestDate.timeIntervalSince(lastReplyDate) >= heartbeatReplyTimeout,
timeSinceLastPing >= pingDelay, !isHeartbeatSuspended {
lastRequestDate.timeIntervalSince(lastReplyDate) >= timings.heartbeatReplyTimeout,
timeSinceLastPing >= timings.pingDelay, !isHeartbeatSuspended {
return .retryHeartbeatPing
}

Expand All @@ -191,9 +158,9 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
let rxTimeElapsed = now.timeIntervalSince(lastSeenRx)
let txTimeElapsed = now.timeIntervalSince(lastSeenTx)

if timeSinceLastPing >= heartbeatPingInterval {
if timeSinceLastPing >= timings.heartbeatPingInterval {
// Send heartbeat if traffic is flowing.
if rxTimeElapsed <= trafficFlowTimeout || txTimeElapsed <= trafficFlowTimeout {
if rxTimeElapsed <= timings.trafficFlowTimeout || txTimeElapsed <= timings.trafficFlowTimeout {
return .sendHeartbeatPing
}

Expand All @@ -202,12 +169,12 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
}
}

if timeSinceLastPing >= pingDelay {
if txTimeElapsed >= trafficTimeout || rxTimeElapsed >= trafficTimeout {
if timeSinceLastPing >= timings.pingDelay {
if txTimeElapsed >= timings.trafficTimeout || rxTimeElapsed >= timings.trafficTimeout {
return .trafficTimeout
}

if lastSeenTx > lastSeenRx, rxTimeElapsed >= inboundTrafficTimeout {
if lastSeenTx > lastSeenRx, rxTimeElapsed >= timings.inboundTrafficTimeout {
return .inboundTrafficTimeout
}
}
Expand All @@ -233,13 +200,14 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
private let nslock = NSLock()
private let timerQueue = DispatchQueue(label: "TunnelMonitor-timerQueue")
private let eventQueue: DispatchQueue
private let timings: TunnelMonitorTimings

private var pinger: PingerProtocol
private var defaultPathObserver: DefaultPathObserverProtocol
private var isObservingDefaultPath = false
private var timer: DispatchSourceTimer?

private var state = State()
private var state: State
private var probeAddress: IPv4Address?

private let logger = Logger(label: "TunnelMonitor")
Expand All @@ -262,12 +230,16 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
eventQueue: DispatchQueue,
pinger: PingerProtocol,
tunnelDeviceInfo: TunnelDeviceInfoProtocol,
defaultPathObserver: DefaultPathObserverProtocol
defaultPathObserver: DefaultPathObserverProtocol,
timings: TunnelMonitorTimings
) {
self.eventQueue = eventQueue
self.tunnelDeviceInfo = tunnelDeviceInfo
self.defaultPathObserver = defaultPathObserver

self.timings = timings
state = State(timings: timings)

self.pinger = pinger
self.pinger.onReply = { [weak self] reply in
guard let self else { return }
Expand Down Expand Up @@ -584,7 +556,7 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
timer.setEventHandler { [weak self] in
self?.checkConnectivity()
}
timer.schedule(wallDeadline: .now(), repeating: connectivityCheckInterval.timeInterval)
timer.schedule(wallDeadline: .now(), repeating: timings.connectivityCheckInterval.timeInterval)
timer.activate()

self.timer?.cancel()
Expand Down
69 changes: 69 additions & 0 deletions ios/PacketTunnelCore/TunnelMonitor/TunnelMonitorTimings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//
// TunnelMonitorTimings.swift
// PacketTunnelCore
//
// Created by Jon Petersson on 2023-09-18.
// Copyright © 2023 Mullvad VPN AB. All rights reserved.
//

import MullvadTypes

public struct TunnelMonitorTimings {
/// Interval for periodic heartbeat ping issued when traffic is flowing.
/// Should help to detect connectivity issues on networks that drop traffic in one of directions,
/// regardless if tx/rx counters are being updated.
let heartbeatPingInterval: Duration

/// Heartbeat timeout that once exceeded triggers next heartbeat to be sent.
let heartbeatReplyTimeout: Duration

/// Timeout used to determine if there was a network activity lately.
var trafficFlowTimeout: Duration { heartbeatPingInterval * 0.5 }

/// Ping timeout.
let pingTimeout: Duration

/// Interval to wait before sending next ping.
let pingDelay: Duration

/// Initial timeout when establishing connection.
let initialEstablishTimeout: Duration

/// Multiplier applied to `establishTimeout` on each failed connection attempt.
let establishTimeoutMultiplier: UInt32

/// Maximum timeout when establishing connection.
var maxEstablishTimeout: Duration { pingTimeout }

/// Connectivity check periodicity.
let connectivityCheckInterval: Duration

/// Inbound traffic timeout used when outbound traffic was registered prior to inbound traffic.
let inboundTrafficTimeout: Duration

/// Traffic timeout applied when both tx/rx counters remain stale, i.e no traffic flowing.
/// Ping is issued after that timeout is exceeded.s
let trafficTimeout: Duration

public init(
heartbeatPingInterval: Duration = .seconds(10),
heartbeatReplyTimeout: Duration = .seconds(3),
pingTimeout: Duration = .seconds(15),
pingDelay: Duration = .seconds(3),
initialEstablishTimeout: Duration = .seconds(4),
establishTimeoutMultiplier: UInt32 = 2,
connectivityCheckInterval: Duration = .seconds(1),
inboundTrafficTimeout: Duration = .seconds(5),
trafficTimeout: Duration = .minutes(2)
) {
self.heartbeatPingInterval = heartbeatPingInterval
self.heartbeatReplyTimeout = heartbeatReplyTimeout
self.pingTimeout = pingTimeout
self.pingDelay = pingDelay
self.initialEstablishTimeout = initialEstablishTimeout
self.establishTimeoutMultiplier = establishTimeoutMultiplier
self.connectivityCheckInterval = connectivityCheckInterval
self.inboundTrafficTimeout = inboundTrafficTimeout
self.trafficTimeout = trafficTimeout
}
}
Loading
Loading