* [client] Add tests for the ICEBind open and close races
Running many embedded clients in one process intermittently wedges interface
creation. A goroutine dump taken from 50 clients shows ten of them parked for
seven minutes in Device.IpcSet, in closeBindLocked waiting on
device.net.stopping.Wait, holding device.net while every other device
goroutine queues behind it on Device.Up.
Open writes s.closed and Close reads it with no synchronisation, and Close
also closes s.closedChan without the mutex that Open swaps it under. Two
Closes can both pass the check and close the same channel, and a Close racing
an Open can mark the bind closed while a live channel and live receive
functions remain, after which every later Close takes its early return and
runs neither close(closedChan) nor StdNetBind.Close. The receive functions
never stop, so stopping.Wait never returns.
These tests do not fix that. The first pins the contract closeBindLocked
depends on and passes today. The other two fail under -race, reporting the
races at the three sites above, and pass again once closed and closedChan are
guarded consistently.
* [client] Release parked receivers so reopening a bind cannot stall
receiveRelayed held closedChanMu for the whole of its blocking select, so a
parked receiver kept the read lock indefinitely and Open could never take the
write lock it needs to install a fresh closedChan. wireguard-go reaches Open
from Device.IpcSet and Device.Up with device.net held, so the stall took the
device lock with it: interface creation never finished, every other device
goroutine queued behind Device.Up, and Engine.Start never returned.
Callers now copy the channel under a short read lock and select on the copy.
Copying alone would stand a new trap in the same place, because an Open that
follows an Open leaves the previous generation parked on a channel no later
Close can reach, so Open now closes the outgoing channel before swapping it.
closed and closedChan are also updated together under that mutex. Read and
written apart, Close could see a stale closed and skip both close(closedChan)
and StdNetBind.Close, leaving every receive function running and wedging
closeBindLocked on device.net.stopping.Wait, or two Close calls could pass the
check together and close the same channel twice.
TestICEBindOpenDoesNotBlockOnParkedReceiver fails without this change, without
needing the race detector. The other three cover the surrounding contract and
report the state races under -race.
* [client] Make the bind lifecycle transition atomic and tighten its tests
Review caught that the previous commit moved the torn transition rather than
removing it. Open published the new generation before calling StdNetBind.Open,
so an Open rejected because the bind was already open had already signalled the
outgoing generation, and a Close arriving in that window could mark the bind
closed while the same call went on to install live sockets. Every later Close
then returned early and never shut them down.
Open now calls StdNetBind.Open first, so a failure leaves the current
generation untouched, and both Open and Close hold the lock across the whole
transition. Ordering is safe: StdNetBind.Open reaches muUDPMux through
createReceiverFn, and no path takes muUDPMux before closedChanMu.
The tests were also weaker than they read. The stress test claimed to cover a
stale channel but only ever raced two Closes, and the concurrency test left
overlap to goroutine start order. Both now gate their goroutines on a common
start, the stress test races an Open against the Closes, and both assert the
surviving generation channel is actually closed. Waiting on receive functions
to be entered replaces part of the sleep in the reopen probe, and teardown
bounds its Close so a regression fails the assertion instead of hanging.
Two of the four now fail without the fix and no race detector, the stress test
by reproducing close of a closed channel at the Close early return.
* [client] Fail the reopen probe when its teardown does not complete
closeBounded swallowed its timeout and the cleanup discarded what
receiversStopped returned, so the bounds added in the previous commit only
stopped teardown hanging. A wedged Close or a parked receiver would have left
the test green with a leaked goroutine, which is the failure this test exists
to catch.
closeBounded now reports whether Close returned, and cleanup fails the test on
either bound.
The test writes 500 packets per family and asserted exact-count
delivery within a 5s window, even though its own comment says "Some
packet loss is acceptable for UDP". On FreeBSD/QEMU runners the writer
loops cannot always finish all 500 before the 5s deadline closes the
readers (we have seen 411/500 in CI).
The real assertion of this test is the routing check — IPv4 peer only
gets v4- packets, IPv6 peer only gets v6- packets — which remains
strict. Replace the exact-count assertions with a >=80% delivery
threshold so runner speed variance no longer causes false failures.
The Relayed connection setup is optimistic. It does not have any confirmation of an established end-to-end connection. Peers start sending WireGuard handshake packets immediately after the successful offer-answer handshake.
Meanwhile, for successful P2P connection negotiation, we change the WireGuard endpoint address, but this change does not trigger new handshake initiation. Because the peer switched from Relayed connection to P2P, the packets from the Relay server are dropped and must wait for the next WireGuard handshake via P2P.
To avoid this scenario, the relayed WireGuard proxy no longer drops the packets. Instead, it rewrites the source address to the new P2P endpoint and continues forwarding the packets.
We still have one corner case: if the Relayed server negotiation chooses a server that has not been used before. In this case, one side of the peer connection will be slower to reach the Relay server, and the Relay server will drop the handshake packet.
If everything goes well we should see exactly 5 seconds improvements between the WireGuard configuration time and the handshake time.
- Move `util/grpc` and `util/net` to `client` so `internal` packages can be accessed
- Add methods to return the next best interface after the NetBird interface.
- Use `IP_UNICAST_IF` sock opt to force the outgoing interface for the NetBird `net.Dialer` and `net.ListenerConfig` to avoid routing loops. The interface is picked by the new route lookup method.
- Some refactoring to avoid import cycles
- Old behavior is available through `NB_USE_LEGACY_ROUTING=true` env var
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.
This PR introduces a new inactivity package responsible for monitoring peer activity and notifying when peers become inactive.
Introduces a new Signal message type to close the peer connection after the idle timeout is reached.
Periodically checks the last activity of registered peers via a Bind interface.
Notifies via a channel when peers exceed a configurable inactivity threshold.
Default settings
DefaultInactivityThreshold is set to 15 minutes, with a minimum allowed threshold of 1 minute.
Limitations
This inactivity check does not support kernel WireGuard integration. In kernel–user space communication, the user space side will always be responsible for closing the connection.
updates the logging implementation to use the netbird logger for both ICE and gRPC components. The key changes include:
- Introducing a gRPC logger configuration in util/log.go that integrates with the netbird logging setup.
- Updating the log hook in formatter/hook/hook.go to ensure a default caller is used when not set.
- Refactoring ICE agent and UDP multiplexers to use a unified logger via the new getLogger() method.
- When a peer is connected with TURN and a Relay connection is established, do not force switching to Relay. Keep using TURN until disconnection.
-In the proxy preparation phase, the Bind Proxy does not set the remote conn as a fake address for Bind. When running the Work() function, the proper proxy instance updates the conn inside the Bind.
Code cleaning around the util/net package. The goal was to write a more understandable source code but modify nothing on the logic.
Protect the WireGuard UDP listeners with marks.
The implementation can support the VPN permission revocation events in thread safe way. It will be important if we start to support the running time route and DNS update features.
- uniformize the file name convention: [struct_name] _ [functions] _ [os].go
- code cleaning in net_linux.go
- move env variables to env.go file
In the case of user space WireGuard mode, use in-memory proxy between the TURN/Relay connection and the WireGuard Bind. We keep the UDP proxy and eBPF proxy for kernel mode.
The key change is the new wgproxy/bind and the iface/bind/ice_bind changes. Everything else is just to fulfill the dependencies.
