Initial version of the `firezone-connection` crate. To begin with, we
only establish a connection in a LAN, i.e. no hole-punching, no STUN or
TURN servers, just host candidates. As such, a lot of this PR is just
scaffolding for setting up the test environment and the actual
`ConnectionPool` implementation.
For the curious, I've left some TODOs where I am going to attempt
extending the implementation once we start dealing with STUN and TURN
servers.
I also extended CI to run these tests.
... and move its methods into ResourceDescription.
This was a TODO from some pull request in the last few days. I assume
the goal is to share this function between all clients if needed. It
doesn't reduce the number of lines of code, since I could have removed
ResourceDisplay and done this on-the-fly when building the systray menu,
as an alternative.
Fix for #2956 this is achieved by refreshing access to every resource
every 5 minutes.
There's still an open question for this PR:
When the gateway resolves an ip the gateway allows access to a DNS
resource it resolves the address and allow access to that ip for that
client.
Right now, until the access for that resource doesn't expire that access
isn't revoked.
We could change it so that we require the client to refresh such
access(with this PR those refresh queries are already being made every 5
minutes) every x minutes on top of the `expires_at` or we can keep
`expires_at` as to mean "allow access until `expires_at` for whatever
this resource resolves to".
cc @jamilbk @AndrewDryga
Previously, we just assumed that the domain in the query is a subdomain
of the resource but a malicious actor can hijack that field to access
domains that doesn't correspond to that resource.
With this patch we don't even resolve the address for unrelated domains.
Fixes#2470, now for linux it looks like:
```
Alpine Linux/3.19.0 (x86_64;5.15.133.1-microsoft-standard-WSL2;) connlib/1.0.0
```
For macos it looks like:
```
Mac OS/13.4.1 (arm64;22.5.0;) connlib/1.0.0
```
and this is how it looks on android:
```
Android/Unknown 6.1.23-android14-4-00257-g7e35917775b8-ab9964412 connlib/1.0.0
```
note: seems like in android emulator at least we can't get the
architecture so easily
Should fix#2880
The way I do it is after ~10 seconds dropping the
`gateway_awaiting_connection` and let the client try the connection
again, depending on upper layer, I think this is fine since the cases
where this happens is unlikely.
It's hard to test thoroughly but I'll test with bad-condition
simulators, [pumba](https://github.com/alexei-led/pumba) seems
promising. In the meantime I'm still creating the PR so that I can have
it reviewed.
Edit: Using Pumba with different % of packet loss things seems to go
well, and connections are actually established even if the packets are
loss. (Making a note that we should integrate pumba with our CI)
Partially fixes#2920
As explained in
https://github.com/firezone/firezone/issues/2920#issuecomment-1861642550
in the future we should change the way we resolve DNS queries in the
gateway to properly handle HTTPS record types.
With this patch this is what happens to an HTTPS query while firezone is
running:
```
kdig -t HTTPS ifconfig.net
;; ->>HEADER<<- opcode: QUERY; status: NXDOMAIN; id: 15773
;; Flags: qr rd; QUERY: 1; ANSWER: 0; AUTHORITY: 0; ADDITIONAL: 0
;; QUESTION SECTION:
;; ifconfig.net. IN HTTPS
;; Received 30 B
;; Time 2023-12-18 18:34:23 -03
;; From 100.100.111.1@53(UDP) in 0.6 ms
```
Fixes#2948
So it seems that it's easiest just to use an old-fashioned semver
string. This means we'll need to keep a version matrix in the docs of
which components are supported and for how long, but it's better than
having different version schemes for different Firezone components
altogether.
Automatically write the wintun.dll file on startup and then detect
whether we need to elevate to admin privileges.
I check for privileges by making a test tunnel, so I did #2758 as part
of this, which bundles the DLL inside the exe, and then the exe deploys
it.
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
Prevent the edge case where our DNS sentinel could be used as a fallback
resolver. I didn't observe this in the wild, but we should avoid it in
case.
---------
Co-authored-by: Gabi <gabrielalejandro7@gmail.com>
This fixes#2503
Also:
* decouples data-plane and control-plane on the gateway
* fixes a thing were a client would stop retrying connecting to a
resource if it failed too many times
* add all routes on start instead of on a per-route basis
This reduces the failover time by depending on webrtc's keepalive
instead of wireguard's.
We have much more control over that, since boringtun doesn't bubble up
any of the keepalives timeout(only a trace warning).
In the a next commit, when things are more stable, we should just get
rid of wireguard's keep alive. When we remove webrtc we will build our
own.
Events based on `keepalive` timeouts are key to our failover system, so
we **need** it.
Draft because it's built on top of #2891 (which is completely separate
code but without that the failover just doesn't work correctly)
<img width="1552" alt="Screenshot 2023-12-12 at 11 29 43 PM"
src="https://github.com/firezone/firezone/assets/167144/d517c830-64a8-462d-8cb5-c41835fa2059">
Found a reliable way to return default system DNS resolvers on iOS and
macOS. Even if this method is not perfect, I think it's still worth
pursuing because:
* Many administrators will set an upstream resolver in the portal anyway
(bypassing client system resolvers)
* It unifies our Split DNS approach across platforms (assuming we can
query the default system resolvers on Windows), allowing connlib to
intercept all DNS queries on all platforms. This opens the door for some
interesting feature possibilities in the area of malicious query
blocking. This also makes DNS bugs easier to investigate because there's
only one codepath for packets to take. See
https://github.com/firezone/firezone/issues/2859
Draft because it needs more testing and I need to figure out the
`RustVec<RustString>` type for the Swift -> Rust FFI.
Refs #2713
When a peer expired the os might have cached the old internal ips that
we used, then with a new peer we were assigning new ips and that cached
ip might have been wrong, then the tunnel would be in state where it
would send the wrong response to this ips.
With this PR we try to always reuse the old ip if there's any available.
Previously, we just expected the portal to disconnects us and 401 on the
retry, right now we harden that behaviour by also just disconnecting
when token expiration.
This seems to work, there's another part to this which is not only
handling the replies but also handling the message generated by the
portal, I'll implement that when I can easily test expirying tokens, for
now this makes the client much more stable.
just silly but important mistake 😛fixes#2858 and #2859 (though there might be an additional edge case in
#2859 where the upstream server is set as a dns, though it seems to work
some further testing would be good)
At present, the definition of `Device` is heavily nested with
conditional code. I've found this hard to understand and navigate.
Recent refactorings now made it possible to remove a lot of these layers
so we primarily deal with two concepts:
- A `Device` which offers async read and non-blocking write functions
- A `Tun` abstraction which is platform-specific
Instead of dedicated modules, I chose to feature-flag individual
functions on `Device` with `#[cfg(target_family = "unix")]` and
`#[cfg(target_family = "windows")]`. I find this easier to understand
because the code is right next to each other.
In addition, changing the module hierarchy of `Device` allows us to
remove `async` from the public API which is only introduced by the use
of `rtnetlink` in Linux. Instead of making functions across all `Tun`
implementations `async`, we embed a "worker" within the `linux::Tun`
implementation that gets polled before `poll_read`.
---------
Co-authored-by: Gabi <gabrielalejandro7@gmail.com>
Fixes: #2854.
Note: this is ready for review but reproducing the bug that triggered
the fix takes ~1 hour or so, so I would like to wait to check that's
fixed.
Can be reviewed meanwhile.
This PR changes the protocol and adds support for DNS subdomains, now
when a DNS resource is added all its subdomains are automatically
tunneled too. Later we will add support for `*.domain` or `?.domain` but
currently there is an Apple split tunnel implementation limitation which
is too labor-intensive to fix right away.
Fixes#2661
Co-authored-by: Andrew Dryga <andrew@dryga.com>
Bumps
[org.jetbrains.kotlin:kotlin-stdlib](https://github.com/JetBrains/kotlin)
from 1.9.20 to 1.9.21.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/JetBrains/kotlin/releases">org.jetbrains.kotlin:kotlin-stdlib's
releases</a>.</em></p>
<blockquote>
<h2>Kotlin 1.9.21</h2>
<h2>Changelog</h2>
<h3>Compiler</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62885"><code>KT-62885</code></a>
Introduce a language feature entry for expect actual classes for easier
configuration of MPP projects</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63081"><code>KT-63081</code></a>
Optimize new native caches:
CachedLibraries.computeVersionedCacheDirectory()</li>
</ul>
<h3>Docs & Examples</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-55619"><code>KT-55619</code></a>
Document <code>String.format</code> function</li>
</ul>
<h3>IDE. Gradle Integration</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62877"><code>KT-62877</code></a>
Artifact files collecting for project configuration was finished.
Resolution for configuration configuration X will be skipped</li>
</ul>
<h3>IDE. Gradle. Script</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-60813"><code>KT-60813</code></a>
Scripts: NoSuchMethodError: 'void
org.slf4j.Logger.error(java.lang.String, java.lang.Object)' when
dependency uses Slf4j API</li>
</ul>
<h3>JavaScript</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-60785"><code>KT-60785</code></a>
KJS: Destructured value class in suspend function fails with Uncaught
TypeError: can't convert to primitive type error</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63207"><code>KT-63207</code></a>
KMP / JS: "TypeError: <!-- raw HTML omitted --> is not a
function" with 1.9.20</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62778"><code>KT-62778</code></a>
package.json "main" field has .js extension when the result
files have .mjs extension</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-61795"><code>KT-61795</code></a>
KJS: Incremental Cache is not invalidated if <code>useEsClasses</code>
compiler argument was changed</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-61957"><code>KT-61957</code></a>
KJS: "Uncaught ReferenceError: entries is not defined" caused
by enum class with <code>@JsExport</code> and Enum.entries call</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62444"><code>KT-62444</code></a>
KJS with commonJS modules should re-export in 1.9.20</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63184"><code>KT-63184</code></a>
KJS / Serialization: JsExport on serializable interface creates
erroneous TypeScript</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62190"><code>KT-62190</code></a>
KJS: "IllegalStateException: Expect to have either super call or
partial linkage stub inside constructor" caused by Compose and
useEsModules()</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-58685"><code>KT-58685</code></a>
KJS: "IllegalStateException: Not locked" cused by
"unlock" called twice</li>
</ul>
<h3>Klibs</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62515"><code>KT-62515</code></a>
Interop klib of concurrent version is not accepted when building
dependent project: "The library versions don't match"</li>
</ul>
<h3>Tools. CLI</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63139"><code>KT-63139</code></a>
Incorrect kotlin implementation version (1.9.255-SNAPSHOT) in metadata
info</li>
</ul>
<h3>Tools. Gradle</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63499"><code>KT-63499</code></a>
Gradle: Source sets conventions are still registered</li>
</ul>
<h3>Tools. Gradle. JS</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-59523"><code>KT-59523</code></a>
MPP / KJS: ESM modules uses incorrect file extension on package.json
(.mjs)</li>
</ul>
<h3>Tools. Gradle. Kapt</h3>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md">org.jetbrains.kotlin:kotlin-stdlib's
changelog</a>.</em></p>
<blockquote>
<h2>1.9.21</h2>
<h3>Compiler</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62885"><code>KT-62885</code></a>
Introduce a language feature entry for expect actual classes for easier
configuration of MPP projects</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63081"><code>KT-63081</code></a>
Optimize new native caches:
CachedLibraries.computeVersionedCacheDirectory()</li>
</ul>
<h3>Docs & Examples</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-55619"><code>KT-55619</code></a>
Document <code>String.format</code> function</li>
</ul>
<h3>IDE. Gradle Integration</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62877"><code>KT-62877</code></a>
Artifact files collecting for project configuration was finished.
Resolution for configuration configuration X will be skipped</li>
</ul>
<h3>IDE. Gradle. Script</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-60813"><code>KT-60813</code></a>
Scripts: NoSuchMethodError: 'void
org.slf4j.Logger.error(java.lang.String, java.lang.Object)' when
dependency uses Slf4j API</li>
</ul>
<h3>JavaScript</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-60785"><code>KT-60785</code></a>
KJS: Destructured value class in suspend function fails with Uncaught
TypeError: can't convert to primitive type error</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63207"><code>KT-63207</code></a>
KMP / JS: "TypeError: <!-- raw HTML omitted --> is not a
function" with 1.9.20</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62778"><code>KT-62778</code></a>
package.json "main" field has .js extension when the result
files have .mjs extension</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-61795"><code>KT-61795</code></a>
KJS: Incremental Cache is not invalidated if <code>useEsClasses</code>
compiler argument was changed</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-61957"><code>KT-61957</code></a>
KJS: "Uncaught ReferenceError: entries is not defined" caused
by enum class with <code>@JsExport</code> and Enum.entries call</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62444"><code>KT-62444</code></a>
KJS with commonJS modules should re-export in 1.9.20</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63184"><code>KT-63184</code></a>
KJS / Serialization: JsExport on serializable interface creates
erroneous TypeScript</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62190"><code>KT-62190</code></a>
KJS: "IllegalStateException: Expect to have either super call or
partial linkage stub inside constructor" caused by Compose and
useEsModules()</li>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-58685"><code>KT-58685</code></a>
KJS: "IllegalStateException: Not locked" cused by
"unlock" called twice</li>
</ul>
<h3>Klibs</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-62515"><code>KT-62515</code></a>
Interop klib of concurrent version is not accepted when building
dependent project: "The library versions don't match"</li>
</ul>
<h3>Tools. CLI</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63139"><code>KT-63139</code></a>
Incorrect kotlin implementation version (1.9.255-SNAPSHOT) in metadata
info</li>
</ul>
<h3>Tools. Gradle</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63499"><code>KT-63499</code></a>
Gradle: Source sets conventions are still registered</li>
</ul>
<h3>Tools. Gradle. JS</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-59523"><code>KT-59523</code></a>
MPP / KJS: ESM modules uses incorrect file extension on package.json
(.mjs)</li>
</ul>
<h3>Tools. Gradle. Kapt</h3>
<ul>
<li><a
href="https://youtrack.jetbrains.com/issue/KT-63366"><code>KT-63366</code></a>
Kapt processing fails with custom source sets</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="b2740d8957"><code>b2740d8</code></a>
Add changelog for 1.9.21</li>
<li><a
href="5d6520c9fb"><code>5d6520c</code></a>
[K/JS] Fix coroutines but turn back the fix for coroutines intrinsics
`interc...</li>
<li><a
href="8293c8f538"><code>8293c8f</code></a>
KAPT: Use reflection to access code, changed in JDK 21</li>
<li><a
href="e459a6d5d2"><code>e459a6d</code></a>
KAPT: Always print parens on empty annotation parameters</li>
<li><a
href="705c8047ae"><code>705c804</code></a>
KAPT: Run tests on JDK 21</li>
<li><a
href="120c99d455"><code>120c99d</code></a>
Fix ClassCastException in JPS statistics ad update log messages</li>
<li><a
href="68f26183f8"><code>68f2618</code></a>
Fix ClassCastException in JPS statistics ad update log messages</li>
<li><a
href="c66b789f93"><code>c66b789</code></a>
KAPT3: Use another class in com.sun.tools.javac.main</li>
<li><a
href="d4ce8c7262"><code>d4ce8c7</code></a>
Temporarily disable Kapt4IT and KaptIncrementalIT and the
descendants.</li>
<li><a
href="f559dd55ab"><code>f559dd5</code></a>
Stop using IJ's maps in graph implementation of JPS</li>
<li>Additional commits viewable in <a
href="https://github.com/JetBrains/kotlin/compare/v1.9.20...v1.9.21">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
In some cases we were obvserving that connections between clients and
gateways couldn't be established.
This happened even when candidates where being found on both ends.
This usually was obvserved when ipv6 isn't working on the relays and
it's still used as one of the viable candidates.
To reproduce this more easily I created an iface with 50 ips using this
script:
```bash
#!/bin/bash
# Generate 10 IPv6 addresses
for i in {1..10}
do
for j in {1..5}
do
# Generate a random IPv6 address
ipv6_address=$(openssl rand -hex 5 | sed 's/\(..\)/\1:/g; s/.$//' | awk '{print "fd00::"$1}')
# Add IPv6 address to lo0
sudo ifconfig lo0 inet6 alias $ipv6_address
echo "Added IPv6 address $ipv6_address to lo0"
done
done
```
This behavior was almost consistently obvserved, as it depended on the
order candidates were used.
I tried modifying timeouts and the limits to channel binding requests
that are internal to webrtc but the connections were still not
consistent, the only thing that worked was limiting the number of host
candidates.
This is okay since even if we can't stablish the local connection (no
hairpin nat) relayed connection will still happen.
But this is not a good long-term solution. In the future we should be
smarter how we sort and ping candidates, prioritizing srflx to srflx or
srflx to relay and leave host candidates for last. Will be easier to
improve on after refactoring webrtc out.
Yesterday, during some portion of the day connections between clients
and resources were impossible.
While I couldn't pinpoint the exact cause I found some issues with
cleanup. This PR fixes those.
Furthermore, I increased the default log level for tunnels in the
clients so that if this happens again we have better logs to triage.
~~Furthermore, I found out about #2705 so, I removed the limit of relays
from connlib since the portal already limits it to 2 (4 if you count
per-ip), that way we make sure that we always use both ipv4 and ipv6.
The connection start up time seems to slow down due to this but I think
this is better. We might want to go to only 2 urls again later on to
speed this up, if the portal can ensure it's a working relay
load-balanced relay there might not be a point in using more than a
single server~~. cc @AndrewDryga
Edit: we always get an ipv4 and ipv6 address for the same relay as the
first two relays in the relay list, save the case where only one of the
ip types is supported. We should be safe limiting it to 2.
---------
Signed-off-by: Gabi <gabrielalejandro7@gmail.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
This PR started as part of a degradation in performance for the
gateways.
The way to test performance in a realistic enviroment is using a GCP vm
as a client and an AWS vm as a gateway with a single iperf server behind
the gateway.
Then the `iperf` results with current main:
```
Connecting to host 172.31.92.238, port 5201
Reverse mode, remote host 172.31.92.238 is sending
[ 5] local 100.83.194.77 port 58426 connected to 172.31.92.238 port 5201
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 1.01 MBytes 8.50 Mbits/sec
[ 5] 1.00-2.00 sec 1.14 MBytes 9.59 Mbits/sec
[ 5] 2.00-3.00 sec 699 KBytes 5.73 Mbits/sec
[ 5] 3.00-4.00 sec 1.11 MBytes 9.31 Mbits/sec
[ 5] 4.00-5.00 sec 664 KBytes 5.44 Mbits/sec
[ 5] 5.00-6.00 sec 591 KBytes 4.84 Mbits/sec
[ 5] 6.00-7.00 sec 722 KBytes 5.91 Mbits/sec
[ 5] 7.00-8.00 sec 833 KBytes 6.83 Mbits/sec
[ 5] 8.00-9.00 sec 738 KBytes 6.04 Mbits/sec
[ 5] 9.00-10.00 sec 836 KBytes 6.85 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.06 sec 8.78 MBytes 7.32 Mbits/sec 3 sender
[ 5] 0.00-10.00 sec 8.23 MBytes 6.90 Mbits/sec receiver
iperf Done.
```
Most of the performance problems were due to using SCTP and DTLS.
So I created a
[fork](https://github.com/firezone/webrtc/tree/expose-new-endpoint) of
webrtc that let us circumvent those, since we don't need them because we
are depending on wireguard for encryption.
With those changes much better throughput is achieved:
```
gabriel@cloudshell:~ (firezone-personal-instances)$ iperf3 -R -c 172.31.92.238
Connecting to host 172.31.92.238, port 5201
Reverse mode, remote host 172.31.92.238 is sending
[ 5] local 100.83.194.77 port 51206 connected to 172.31.92.238 port 5201
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 5.60 MBytes 47.0 Mbits/sec
[ 5] 1.00-2.00 sec 17.2 MBytes 144 Mbits/sec
[ 5] 2.00-3.00 sec 15.8 MBytes 132 Mbits/sec
[ 5] 3.00-4.00 sec 14.8 MBytes 125 Mbits/sec
[ 5] 4.00-5.00 sec 15.9 MBytes 133 Mbits/sec
[ 5] 5.00-6.00 sec 15.8 MBytes 133 Mbits/sec
[ 5] 6.00-7.00 sec 15.3 MBytes 128 Mbits/sec
[ 5] 7.00-8.00 sec 15.6 MBytes 131 Mbits/sec
[ 5] 8.00-9.00 sec 15.6 MBytes 131 Mbits/sec
[ 5] 9.00-10.00 sec 16.0 MBytes 134 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.05 sec 151 MBytes 126 Mbits/sec 74 sender
[ 5] 0.00-10.00 sec 148 MBytes 124 Mbits/sec receiver
iperf Done
```
However, this is still worse than it was achieved with a previous
commit(`21afdf0a9a113c996d60a63b2e8c8f32d3aeb87`):
```
gabriel@cloudshell:~ (firezone-personal-instances)$ iperf3 -R -c 172.31.92.238
Connecting to host 172.31.92.238, port 5201
Reverse mode, remote host 172.31.92.238 is sending
[ 5] local 100.100.68.41 port 49762 connected to 172.31.92.238 port 5201
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 6.14 MBytes 51.5 Mbits/sec
[ 5] 1.00-2.00 sec 17.1 MBytes 144 Mbits/sec
[ 5] 2.00-3.00 sec 22.8 MBytes 191 Mbits/sec
[ 5] 3.00-4.00 sec 23.5 MBytes 197 Mbits/sec
[ 5] 4.00-5.00 sec 23.0 MBytes 193 Mbits/sec
[ 5] 5.00-6.00 sec 22.1 MBytes 185 Mbits/sec
[ 5] 6.00-7.00 sec 23.0 MBytes 193 Mbits/sec
[ 5] 7.00-8.00 sec 22.7 MBytes 190 Mbits/sec
[ 5] 8.00-9.00 sec 21.0 MBytes 176 Mbits/sec
[ 5] 9.00-10.00 sec 19.9 MBytes 167 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.05 sec 204 MBytes 170 Mbits/sec 127 sender
[ 5] 0.00-10.00 sec 201 MBytes 169 Mbits/sec receiver
```
My profiling suggested that this is due to reading/writing packets
happening in its own dedicated tasks. So much so that maybe in the
future we should even consider spawning their own dedicated runtime so
that those loops have a dedicated OS thread.
Also, probably using a multi-queue interface will give us huge gains if
we have a dedicated task for each queue(currently the interface is
started as a multi-queue but a single file descriptor is used) for
handling multiple concurrent clients.
However, the changes proposed in this PR are good enough for now as long
as performance don't degrade.
In that line I will create a CI that reports the throughput using the
local `docker-compose.yml` file that we should always check before
merging, that is not the be all end all of the performance story but for
smaller PRs the correlation to real world throughput should be enough.
For bigger PRs we should manually test before merging for now, until we
have a way in CI to spin up some realistic tests(note that vms should be
in separate cloud enviroments, the same-cloud links are so reliable that
we miss actual performance degradation due to dropped packets). On this
note I'll write a small manual on how to conduct those tests with full
current results that we should use always before merging new PRs that
affect the hot-path. cc @thomaseizinger
Finally, when testing these changes I found some flakiness regarding the
re-connection path. So I changed things so that we cleanup connections
only using wireguard's error(connection expiration). This is quite slow
for now (~120 seconds) but in the future we can issue an ice restart
each time wireguard keepalive expires(rekey timeout) so that we can
restart connection each ~30 seconds and we can reduce the keepalive time
out from the portal to accelerate it even more. And in the future we can
get smarter about it.
---------
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
We encapsulate the internals of `Device` by providing high-level
functions on `Device` itself and make all the fields private. From the
outside, each consumer this only has an `Arc<Device>` that they can
interact with.
To achieve this, we use the `arc-swap` crate to atomically swap out the
reference to the `Arc<Device>` instead of relying on an `RwLock`. Note
that the _reference_ to this `ArcSwapOption` is also wrapped in an `Arc`
because we need to share this pointer across many `peer_handler`s.
Once we get rid of `Arc<Tunnel>`, this will become a lot simpler.
Bumps [tokio-util](https://github.com/tokio-rs/tokio) from 0.7.9 to
0.7.10.
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a
href="https://github.com/tokio-rs/tokio/commits">compare view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
