mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-21 04:49:10 +02:00
36fe467aba4239cb3df9d00b2f3feb75d86bbc91
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Zoltan Papp
|
28657607c1 |
[client] Keep interface discovery out of the bind open path (#8226)
* [client] Keep interface discovery out of the bind open path Since 0.81.0 a userspace-mode agent whose interface name is not covered by the interface blacklist deadlocks while bringing the device up, and the daemon, the GUI and the service stop all hang. wireguard-go holds the device net lock across bind.Open, our bind builds the UDP mux there, the mux enumerated interfaces in its constructor, and the interface filter probed every unlisted name through wgctrl. For the agent's own interface that probe connected to the daemon's own UAPI endpoint and waited, with no deadline, for a reply the handler could only give after taking the same lock. #7346 moved the first enumeration from the stdnet constructor, which ran before the device existed, to this call site. The mux no longer discovers interfaces when it is constructed. It records whether its socket is bound to an unspecified address and GetConn uses that instead of the length of the discovered list, while GetListenAddresses keeps refreshing on demand, which pion calls during candidate gathering outside the device locks. The WireGuard detector also answers for the agent's own interface without probing, so no filter asks the daemon about itself. * [client] Take the agent's own interface name in the detector constructor Every device implementation reports the configured name, so there is no later name to follow and the setter was state without a reader that needed it. The detector takes the name once and compares without a lock. * [client] Create the default mux Net in the constructor With discovery moved to GetListenAddresses, the nil Net fallback ran there as well, and the first concurrent callers raced on writing it. stdnet.NewNet does no discovery, so the constructor can create it before the mux is shared and the address refresh only reads it. * [client] Follow the created interface name in the detector Android ignores the configured interface name and keeps the TUN name returned by CreateUnmonitoredTUNFromFD, so the detector built from the configuration would still probe the agent's real interface. The engine updates the detector with wgInterface.Name() once the interface exists. * [client] Keep the detector's own interface name immutable On Android the kernel names the TUN and the name can change again on renew, while the UAPI socket cannot be opened there at all, so neither the probe nor a name set after creation can identify the agent's own interface reliably. Excluding that interface on Android belongs to the interface discovery the app provides. Everywhere else the configured name is the created name, so the constructor argument is enough and the detector needs no setter and no lock on the hot path. |
||
|
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
|
||
|
Viktor Liu
|
9cc9462cd5 | [client] Use stdnet with a context to avoid DNS deadlocks (#4781) | ||
|
Zoltan Papp
|
9e81e782e5 |
[client] Fix/v4 stun routing (#4430)
Deduplicate STUN package sending. Originally, because every peer shared the same UDP address, the library could not distinguish which STUN message was associated with which candidate. As a result, the Pion library responded from all candidates for every STUN message. |