Skip to content

Commit

Permalink
Fix reliability and time consumption of testInitialConnectionTimings …
Browse files Browse the repository at this point in the history
…test
  • Loading branch information
Jon Petersson committed Sep 19, 2023
1 parent a91c5d9 commit 56f7b23
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 101 deletions.
14 changes: 9 additions & 5 deletions ios/MullvadVPN.xcodeproj/project.pbxproj
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
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
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
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

0 comments on commit 56f7b23

Please sign in to comment.