mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-09 23:19:11 +02:00
3e85e40be27e9cae8f35c7d163d322ea73f65d3d
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Riccardo Manfrin
|
3e85e40be2 |
[client] Cache the WireGuard interface check shared by ICE agents (#8001)
* [client] Take a WireGuard detector through the interface filter The interface filter answers whether an interface is a WireGuard device by opening a wgctrl client and asking for it, and it does that for every interface it is given. Nothing about that call is tied to the caller, so it can be answered by a shared object instead of being repeated, but the filter has no way to receive one. InterfaceFilter and the constructors that build one now take a detector, and the ICE config carries it so that every agent can be handed the same one. Nobody supplies a detector yet: a nil one probes on every call, which is what the filter did before, so this changes no behaviour. * [client] Share one WireGuard detector across every ICE agent Creating an ICE agent builds two interface filters, one for the agent and one for the transport net it sits on, and each is asked about every host interface. For an interface the disallow list does not settle, answering means opening a wgctrl client, which builds a kernel and a userspace client and resolves the netlink family, and then a round trip that usually just reports the device does not exist. An agent is created per peer connection attempt, so on a large network that runs constantly: on a routing peer with ~16000 peers it measured 2.40s of a 66.59s CPU profile, 3.6%, split evenly between opening the client and the round trip. The engine now owns a detector and passes it to every agent through the ICE config, so the answer for an interface is reused instead of being asked again for each agent. It is kept for a second, short enough that a WireGuard interface appearing is picked up before ICE settles on candidates over it. The callers that build one filter and keep it, the relay and the UDP mux, keep passing nil and so keep probing, which costs them nothing at their rate. * [client] Recheck the WireGuard cache inside the singleflight group A caller that saw an expired entry could enter the singleflight group after another caller had already refreshed the entry and left it, and probe the interface a second time. Read the cache again inside the group before probing. This also makes the concurrent probe test independent of scheduling: a late caller finds the fresh entry instead of starting a new probe. * [client] Drop expired WireGuard detector entries The detector lives as long as the engine and kept an entry for every interface name it was ever asked about. On hosts that churn interfaces, such as container veths, the map only grew. Remove expired entries when a new answer is stored; the map holds a few dozen names at most, so the sweep is cheap and runs at most once per interface per TTL. * [client] Skip the disallow-list filter test on iOS InterfaceFilter does not apply the disallow list on iOS, so the subtest reaches the probe there and its no-probe assertion cannot hold. |
||
|
Riccardo Manfrin
|
9f8ddc7131 |
[client] Discover interfaces lazily in stdnet instead of at construction (#7346)
* [client] Discover interfaces lazily in stdnet instead of at construction
stdnet.NewNet and NewNetWithDiscover ended with
return n, n.UpdateInterfaces()
handing back a non-nil *Net together with the discovery error. Three of the
five call sites (Engine.newWgIface, ice.NewAgent, SingleSocketUDPMux) logged
the error and kept using the instance, which is only safe as long as the
instance still works after a failed discovery.
That stopped being true when Interfaces() gained a lazily refreshed cache:
updateInterfaces sets lastUpdate only on success, so after a failed
construction the 30s cache guard never holds and Interfaces() returns an
error rather than the empty list it used to return. Feeding such an instance
to pion is worse than passing nothing at all - ice.NewAgent falls back to its
own stdnet when Net is nil, and the interface blacklist is applied separately
through AgentConfig.InterfaceFilter, so the fallback loses nothing. Instead,
a transient discovery failure (the Android bridge at boot, or an interface
disappearing between net.Interfaces() and Interface.Addrs()) turned into a
hard "error getting local interfaces" from ice.NewAgent, and aborted the STUN
and TURN probes, which never even need the interface list.
Since the accessors already refresh a stale cache on demand, the eager
discovery in the constructors is redundant: drop it, make both constructors
infallible, and let the discovery error surface at the call that actually
needs the interfaces. UpdateInterfaces had no callers left and is not part of
transport.Net, so it is removed along with it.
InterfaceByIndex and InterfaceByName read the cached slice directly and never
refreshed it, so they would have kept reporting ErrInterfaceNotFound forever
on an instance whose first discovery failed. They now go through the same
refresh path as Interfaces().
* [client] Warm the stdnet interface cache at construction
Moving discovery to first use regressed the privileged suites on the three
platforms that always build an ICE bind: Darwin, FreeBSD and Windows time out
in TestWGIface_UpdateAddr, TestRecreation, TestEngine_SSH and
TestEngine_MultiplePeers, while Linux stays green because a host with the
WireGuard kernel module takes the kernel-device branch and never drives the
mux that asks for interfaces.
interfaceFilter probes with wgctrl every interface the disallow list does not
already exclude. Discovering at construction ran that probe before the caller
had an overlay interface of its own; discovering at first use runs it after,
so on a userspace WireGuard platform the probe reaches the UAPI socket of the
same process. The tests reach it because they construct with a nil disallow
list, where the client passes DefaultInterfaceBlacklist and its own interface
is excluded by prefix.
Restore the original timing with an explicit warm-up. The constructors stay
infallible and the error is still reported by the accessor that needs the
interfaces, so the contract this branch is about is unchanged.
* Revert "[client] Warm the stdnet interface cache at construction"
This reverts commit
|
||
|
dmitri-netbird
|
a144e8c144 |
[client, management] switch to go.uber.org/mock (#7253)
* switch to go.uber.org/mock/gomock Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> * updated go:generate commands + regenerated mocks Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> * update go:generate mockgen commands Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> * removed duplicate import Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> * fix go:generate Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> --------- Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> |
||
|
Zoltan Pappandcoderabbitai[bot]
|
2d7b309004 |
[client] Categorize privileged tests behind a build tag and run them in Docker (#6425)
* [client] categorize root/system-mutating tests behind a privileged build tag Tests that need root or mutate host state (nftables/iptables/DNS, TUN/WireGuard interfaces, routes, eBPF, SSH/service install) are now gated behind a //go:build privileged tag. The default `go test ./client/...` runs as a non-root user with no sudo and leaves host networking untouched; mixed files were split so pure-logic tests stay in the default suite. A self-hosting ory/dockertest/v4 harness (client/testutil/privileged) runs the privileged suite inside a --privileged --cap-add=NET_ADMIN container via `make test-privileged`; a DOCKER_CI=true guard skips the spawn when already inside the container. Added `make test-unit` for the host-safe run. * [client] add PRIV_RUN/PRIV_PKGS filters to the privileged test harness The dockertest harness now reads two optional env vars when building the in-container `go test` command: PRIV_RUN adds a -run test-name filter and PRIV_PKGS overrides the package list. Both empty reproduce the full privileged suite, so CI and `make test-privileged` behave as before. Lets a developer run a single privileged test in the container, e.g.: PRIV_RUN=TestNftablesManager PRIV_PKGS=./client/firewall/nftables/... make test-privileged * [client] fix unused-helper lint after the privileged test split Splitting privileged tests into *_privileged_test.go left their shared helpers in the untagged files, so in the default (no-tag) build they had no callers and golangci-lint flagged them as unused. Moved the privileged-only helpers into the privileged files next to their callers (generateDummyHandler; createEngine/startSignal/startManagement/getConnectedPeers/ getPeers + kaep/kasp; (*mockDaemon).setJWTToken). Annotated the shared routing-test fixtures that must stay untagged for cross-platform compilation with //nolint:unused (systemops_bsd expected* vars, ensureIPv6DefaultRoute on bsd/windows, loopbackIfaceWindows), matching the existing linux variant. * [client] fix privileged test CI failures and run the harness on macOS The host-safe unit run dropped sudo but two privileged test groups were never tagged, and the Docker privileged job silently never ran the suite: - Gate the ssh/server PrivilegeDropper command-construction tests behind the privileged tag (they require root to target a different UID); split them into executor_unix_privileged_test.go. - Tag sharedsock raw-socket tests privileged (need CAP_NET_RAW). - Fix the Docker job command: nested single quotes around the build tags closed the sh -c wrapper early, dropping the go list package set and the privileged tag, so go test ran on the empty repo root. Use double quotes. Make the self-hosting harness usable from a dev Mac: - Build it on darwin as well as linux; it only drives Docker. - Resolve the active docker context endpoint into DOCKER_HOST when the default /var/run/docker.sock is absent (Docker Desktop, Colima, OrbStack). - Rename the misspelled containerGoModache constant to containerGoModCache. * Update client/internal/engine_privileged_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update client/internal/routemanager/systemops/systemops_linux_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update client/internal/routemanager/systemops/systemops_windows_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update client/server/server_privileged_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * [ci] Run privileged-tagged tests on darwin, windows and freebsd The privileged build tag split moved root/system-mutating tests behind //go:build privileged, but only the linux docker job was given the tag. The native darwin (sudo), windows (PsExec64 -s) and freebsd VM runners already have the required privileges, so add the privileged tag there too to keep CI running the same set of tests as before the split. * [ci] Exclude dockertest harness from the darwin privileged run The privileged tag now compiles client/testutil/privileged on darwin, whose TestRunPrivilegedSuiteInDocker spawns a container the macOS runner has no Docker for. Exclude the harness package from the darwin list, matching the linux job, so the privileged tests run in place without a container spawn. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |