[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().
This commit is contained in:
riccardom
2026-08-28 12:07:36 +02:00
parent 89c6e84a41
commit 48a2a7a5b7
16 changed files with 196 additions and 128 deletions

View File

@@ -44,10 +44,7 @@ func TestWGIface_UpdateAddr(t *testing.T) {
ifaceName := fmt.Sprintf("utun%d", WgIntNumber+4)
addr := "100.64.0.1/8"
wgPort := 33100
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := WGIFaceOpts{
IFaceName: ifaceName,
@@ -127,10 +124,7 @@ func getIfaceAddrs(ifaceName string) ([]net.Addr, error) {
func Test_CreateInterface(t *testing.T) {
ifaceName := fmt.Sprintf("utun%d", WgIntNumber+1)
wgIP := "10.99.99.1/32"
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := WGIFaceOpts{
IFaceName: ifaceName,
Address: wgaddr.MustParseWGAddress(wgIP),
@@ -170,10 +164,7 @@ func Test_Close(t *testing.T) {
ifaceName := fmt.Sprintf("utun%d", WgIntNumber+2)
wgIP := "10.99.99.2/32"
wgPort := 33100
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := WGIFaceOpts{
IFaceName: ifaceName,
@@ -215,10 +206,7 @@ func TestRecreation(t *testing.T) {
ifaceName := fmt.Sprintf("utun%d", WgIntNumber+2)
wgIP := "10.99.99.2/32"
wgPort := 33100
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := WGIFaceOpts{
IFaceName: ifaceName,
@@ -288,10 +276,7 @@ func Test_ConfigureInterface(t *testing.T) {
ifaceName := fmt.Sprintf("utun%d", WgIntNumber+3)
wgIP := "10.99.99.5/30"
wgPort := 33100
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := WGIFaceOpts{
IFaceName: ifaceName,
Address: wgaddr.MustParseWGAddress(wgIP),
@@ -343,10 +328,7 @@ func Test_ConfigureInterface(t *testing.T) {
func Test_UpdatePeer(t *testing.T) {
ifaceName := fmt.Sprintf("utun%d", WgIntNumber+4)
wgIP := "10.99.99.9/30"
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := WGIFaceOpts{
IFaceName: ifaceName,
@@ -413,10 +395,7 @@ func Test_UpdatePeer(t *testing.T) {
func Test_RemovePeer(t *testing.T) {
ifaceName := fmt.Sprintf("utun%d", WgIntNumber+4)
wgIP := "10.99.99.13/30"
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := WGIFaceOpts{
IFaceName: ifaceName,
@@ -477,10 +456,7 @@ func Test_ConnectPeers(t *testing.T) {
peer2wgPort := 33200
keepAlive := 1 * time.Second
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
guid := fmt.Sprintf("{%s}", uuid.New().String())
device.CustomWindowsGUIDString = strings.ToLower(guid)
@@ -516,10 +492,7 @@ func Test_ConnectPeers(t *testing.T) {
guid = fmt.Sprintf("{%s}", uuid.New().String())
device.CustomWindowsGUIDString = strings.ToLower(guid)
newNet, err = stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet = stdnet.NewNet(context.Background(), nil)
optsPeer2 := WGIFaceOpts{
IFaceName: peer2ifaceName,

View File

@@ -200,10 +200,7 @@ func (m *SingleSocketUDPMux) updateLocalAddresses() {
}
if len(networks) > 0 {
if m.params.Net == nil {
var err error
if m.params.Net, err = stdnet.NewNet(context.Background(), nil); err != nil {
m.params.Logger.Errorf("failed to get create network: %v", err)
}
m.params.Net = stdnet.NewNet(context.Background(), nil)
}
ips, err := localInterfaces(m.params.Net, m.params.InterfaceFilter, nil, networks, true)

View File

@@ -243,10 +243,7 @@ func TestUpdateDNSServer(t *testing.T) {
for n, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
privKey, _ := wgtypes.GenerateKey()
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := iface.WGIFaceOpts{
IFaceName: fmt.Sprintf("utun230%d", n),
@@ -348,11 +345,7 @@ func TestDNSFakeResolverHandleUpdates(t *testing.T) {
defer t.Setenv("NB_WG_KERNEL_DISABLED", ov)
t.Setenv("NB_WG_KERNEL_DISABLED", "true")
newNet, err := stdnet.NewNet(context.Background(), []string{"utun2301"})
if err != nil {
t.Errorf("create stdnet: %v", err)
return
}
newNet := stdnet.NewNet(context.Background(), []string{"utun2301"})
privKey, _ := wgtypes.GeneratePrivateKey()
opts := iface.WGIFaceOpts{

View File

@@ -394,11 +394,7 @@ func createWgInterfaceWithBind(t *testing.T) (*iface.WGIface, error) {
defer t.Setenv("NB_WG_KERNEL_DISABLED", ov)
t.Setenv("NB_WG_KERNEL_DISABLED", "true")
newNet, err := stdnet.NewNet(context.Background(), []string{"utun2301"})
if err != nil {
t.Fatalf("create stdnet: %v", err)
return nil, err
}
newNet := stdnet.NewNet(context.Background(), []string{"utun2301"})
privKey, _ := wgtypes.GeneratePrivateKey()

View File

@@ -2110,10 +2110,7 @@ func (e *Engine) close() {
}
func (e *Engine) newWgIface() (*iface.WGIface, error) {
transportNet, err := e.newStdNet()
if err != nil {
log.Errorf("failed to create pion's stdnet: %s", err)
}
transportNet := e.newStdNet()
opts := iface.WGIFaceOpts{
IFaceName: e.config.WgIfaceName,

View File

@@ -6,6 +6,6 @@ import (
"github.com/netbirdio/netbird/client/internal/stdnet"
)
func (e *Engine) newStdNet() (*stdnet.Net, error) {
func (e *Engine) newStdNet() *stdnet.Net {
return stdnet.NewNet(e.clientCtx, e.config.IFaceBlackList)
}

View File

@@ -2,6 +2,6 @@ package internal
import "github.com/netbirdio/netbird/client/internal/stdnet"
func (e *Engine) newStdNet() (*stdnet.Net, error) {
func (e *Engine) newStdNet() *stdnet.Net {
return stdnet.NewNetWithDiscover(e.clientCtx, e.mobileDep.IFaceDiscover, e.config.IFaceBlackList)
}

View File

@@ -582,10 +582,7 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
StatusRecorder: peer.NewRecorder("https://mgm"),
}, MobileDependency{})
engine.ctx = ctx
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := iface.WGIFaceOpts{
IFaceName: wgIfaceName,
@@ -790,10 +787,7 @@ func TestEngine_UpdateNetworkMapWithDNSUpdate(t *testing.T) {
}, MobileDependency{})
engine.ctx = ctx
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := iface.WGIFaceOpts{
IFaceName: wgIfaceName,
Address: wgaddr.MustParseWGAddress(wgAddr),

View File

@@ -39,10 +39,7 @@ func NewAgent(ctx context.Context, iFaceDiscover stdnet.ExternalIFaceDiscover, c
iceFailedTimeout := iceFailedTimeout()
iceRelayAcceptanceMinWait := iceRelayAcceptanceMinWait()
transportNet, err := newStdNet(ctx, iFaceDiscover, config.InterfaceBlackList)
if err != nil {
log.Errorf("failed to create pion's stdnet: %s", err)
}
transportNet := newStdNet(ctx, iFaceDiscover, config.InterfaceBlackList)
fac := logging.NewDefaultLoggerFactory()

View File

@@ -8,6 +8,6 @@ import (
"github.com/netbirdio/netbird/client/internal/stdnet"
)
func newStdNet(ctx context.Context, _ stdnet.ExternalIFaceDiscover, ifaceBlacklist []string) (*stdnet.Net, error) {
func newStdNet(ctx context.Context, _ stdnet.ExternalIFaceDiscover, ifaceBlacklist []string) *stdnet.Net {
return stdnet.NewNet(ctx, ifaceBlacklist)
}

View File

@@ -6,6 +6,6 @@ import (
"github.com/netbirdio/netbird/client/internal/stdnet"
)
func newStdNet(ctx context.Context, iFaceDiscover stdnet.ExternalIFaceDiscover, ifaceBlacklist []string) (*stdnet.Net, error) {
func newStdNet(ctx context.Context, iFaceDiscover stdnet.ExternalIFaceDiscover, ifaceBlacklist []string) *stdnet.Net {
return stdnet.NewNetWithDiscover(ctx, iFaceDiscover, ifaceBlacklist)
}

View File

@@ -201,11 +201,7 @@ func (p *StunTurnProbe) probeSTUN(ctx context.Context, uri *stun.URI) (addr stri
}
}()
net, err := stdnet.NewNet(ctx, nil)
if err != nil {
probeErr = fmt.Errorf("new net: %w", err)
return
}
net := stdnet.NewNet(ctx, nil)
client, err := stun.DialURI(uri, &stun.DialConfig{
Net: net,
@@ -290,11 +286,7 @@ func (p *StunTurnProbe) probeTURN(ctx context.Context, uri *stun.URI) (addr stri
}
}()
net, err := stdnet.NewNet(ctx, nil)
if err != nil {
probeErr = fmt.Errorf("new net: %w", err)
return
}
net := stdnet.NewNet(ctx, nil)
cfg := &turn.ClientConfig{
STUNServerAddr: turnServerAddr,
TURNServerAddr: turnServerAddr,

View File

@@ -406,10 +406,7 @@ func TestManagerUpdateRoutes(t *testing.T) {
for n, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
peerPrivateKey, _ := wgtypes.GeneratePrivateKey()
newNet, err := stdnet.NewNet(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
newNet := stdnet.NewNet(context.Background(), nil)
opts := iface.WGIFaceOpts{
IFaceName: fmt.Sprintf("utun43%d", n),
Address: wgaddr.MustParseWGAddress("100.65.65.2/24"),

View File

@@ -436,8 +436,7 @@ func createWGInterface(t *testing.T, interfaceName, ipAddressCIDR string, listen
peerPrivateKey, err := wgtypes.GeneratePrivateKey()
require.NoError(t, err)
newNet, err := stdnet.NewNet(context.Background(), nil)
require.NoError(t, err)
newNet := stdnet.NewNet(context.Background(), nil)
opts := iface.WGIFaceOpts{
IFaceName: interfaceName,

View File

@@ -45,7 +45,7 @@ type Net struct {
}
// NewNetWithDiscover creates a new StdNet instance.
func NewNetWithDiscover(ctx context.Context, iFaceDiscover ExternalIFaceDiscover, disallowList []string) (*Net, error) {
func NewNetWithDiscover(ctx context.Context, iFaceDiscover ExternalIFaceDiscover, disallowList []string) *Net {
if ctx == nil {
ctx = context.Background()
}
@@ -60,20 +60,19 @@ func NewNetWithDiscover(ctx context.Context, iFaceDiscover ExternalIFaceDiscover
} else {
n.iFaceDiscover = newMobileIFaceDiscover(iFaceDiscover)
}
return n, n.UpdateInterfaces()
return n
}
// NewNet creates a new StdNet instance.
func NewNet(ctx context.Context, disallowList []string) (*Net, error) {
func NewNet(ctx context.Context, disallowList []string) *Net {
if ctx == nil {
ctx = context.Background()
}
n := &Net{
return &Net{
iFaceDiscover: pionDiscover{},
interfaceFilter: InterfaceFilter(disallowList),
ctx: ctx,
}
return n, n.UpdateInterfaces()
}
// resolveAddr performs DNS resolution with context support and timeout.
@@ -122,45 +121,18 @@ func (n *Net) resolveAddr(network, address string) (netip.AddrPort, error) {
return netip.AddrPortFrom(addrs[0], uint16(port)), nil
}
// UpdateInterfaces updates the internal list of network interfaces
// and associated addresses filtering them by name.
// The interfaces are discovered by an external iFaceDiscover function or by a default discoverer if the external one
// wasn't specified.
func (n *Net) UpdateInterfaces() (err error) {
n.mu.Lock()
defer n.mu.Unlock()
return n.updateInterfaces()
}
func (n *Net) updateInterfaces() (err error) {
allIfaces, err := n.iFaceDiscover.iFaces()
if err != nil {
return err
}
n.interfaces = n.filterInterfaces(allIfaces)
n.lastUpdate = time.Now()
return nil
}
// Interfaces returns a slice of interfaces which are available on the
// system
func (n *Net) Interfaces() ([]*transport.Interface, error) {
n.mu.Lock()
defer n.mu.Unlock()
if time.Since(n.lastUpdate) < updateInterval {
return slices.Clone(n.interfaces), nil
iFaces, err := n.freshInterfacesLocked()
if err != nil {
return nil, err
}
if err := n.updateInterfaces(); err != nil {
return nil, fmt.Errorf("update interfaces: %w", err)
}
return slices.Clone(n.interfaces), nil
return slices.Clone(iFaces), nil
}
// InterfaceByIndex returns the interface specified by index.
@@ -171,7 +143,13 @@ func (n *Net) Interfaces() ([]*transport.Interface, error) {
func (n *Net) InterfaceByIndex(index int) (*transport.Interface, error) {
n.mu.Lock()
defer n.mu.Unlock()
for _, ifc := range n.interfaces {
iFaces, err := n.freshInterfacesLocked()
if err != nil {
return nil, err
}
for _, ifc := range iFaces {
if ifc.Index == index {
return ifc, nil
}
@@ -184,7 +162,13 @@ func (n *Net) InterfaceByIndex(index int) (*transport.Interface, error) {
func (n *Net) InterfaceByName(name string) (*transport.Interface, error) {
n.mu.Lock()
defer n.mu.Unlock()
for _, ifc := range n.interfaces {
iFaces, err := n.freshInterfacesLocked()
if err != nil {
return nil, err
}
for _, ifc := range iFaces {
if ifc.Name == name {
return ifc, nil
}
@@ -193,6 +177,31 @@ func (n *Net) InterfaceByName(name string) (*transport.Interface, error) {
return nil, fmt.Errorf("%w: %s", transport.ErrInterfaceNotFound, name)
}
func (n *Net) freshInterfacesLocked() ([]*transport.Interface, error) {
if time.Since(n.lastUpdate) < updateInterval {
return n.interfaces, nil
}
if err := n.updateInterfacesLocked(); err != nil {
return nil, fmt.Errorf("update interfaces: %w", err)
}
return n.interfaces, nil
}
func (n *Net) updateInterfacesLocked() error {
allIFaces, err := n.iFaceDiscover.iFaces()
if err != nil {
return err
}
n.interfaces = n.filterInterfaces(allIFaces)
n.lastUpdate = time.Now()
return nil
}
func (n *Net) filterInterfaces(interfaces []*transport.Interface) []*transport.Interface {
if n.interfaceFilter == nil {
return interfaces

View File

@@ -0,0 +1,124 @@
package stdnet
import (
"context"
"errors"
"net"
"testing"
"github.com/pion/transport/v3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
type countingDiscover struct {
calls int
list []*transport.Interface
err error
}
func (d *countingDiscover) iFaces() ([]*transport.Interface, error) {
d.calls++
if d.err != nil {
return nil, d.err
}
return d.list, nil
}
func newTestNet(t *testing.T, d iFaceDiscover) *Net {
t.Helper()
return &Net{
iFaceDiscover: d,
ctx: context.Background(),
}
}
func testIFace(index int, name string) *transport.Interface {
return transport.NewInterface(net.Interface{Index: index, Name: name})
}
func TestNet_InterfacesDiscoversLazilyAndCaches(t *testing.T) {
d := &countingDiscover{list: []*transport.Interface{testIFace(1, "eth0")}}
n := newTestNet(t, d)
require.Zero(t, d.calls, "construction must not discover interfaces")
iFaces, err := n.Interfaces()
require.NoError(t, err)
require.Len(t, iFaces, 1)
assert.Equal(t, 1, d.calls)
_, err = n.Interfaces()
require.NoError(t, err)
assert.Equal(t, 1, d.calls)
}
func TestNet_InterfacesRetryAfterDiscoveryFailure(t *testing.T) {
discoverErr := errors.New("discover failed")
d := &countingDiscover{err: discoverErr}
n := newTestNet(t, d)
_, err := n.Interfaces()
require.ErrorIs(t, err, discoverErr)
d.err = nil
d.list = []*transport.Interface{testIFace(1, "eth0")}
iFaces, err := n.Interfaces()
require.NoError(t, err)
require.Len(t, iFaces, 1)
assert.Equal(t, 2, d.calls)
}
func TestNet_InterfaceByNameRefreshes(t *testing.T) {
d := &countingDiscover{list: []*transport.Interface{testIFace(3, "eth0")}}
n := newTestNet(t, d)
ifc, err := n.InterfaceByName("eth0")
require.NoError(t, err)
assert.Equal(t, "eth0", ifc.Name)
assert.Equal(t, 1, d.calls)
_, err = n.InterfaceByName("nope")
require.ErrorIs(t, err, transport.ErrInterfaceNotFound)
}
func TestNet_InterfaceByIndexRefreshes(t *testing.T) {
d := &countingDiscover{list: []*transport.Interface{testIFace(3, "eth0")}}
n := newTestNet(t, d)
ifc, err := n.InterfaceByIndex(3)
require.NoError(t, err)
assert.Equal(t, "eth0", ifc.Name)
assert.Equal(t, 1, d.calls)
_, err = n.InterfaceByIndex(99)
require.ErrorIs(t, err, transport.ErrInterfaceNotFound)
}
func TestNet_InterfaceLookupPropagatesDiscoveryError(t *testing.T) {
discoverErr := errors.New("discover failed")
n := newTestNet(t, &countingDiscover{err: discoverErr})
_, err := n.InterfaceByName("eth0")
require.ErrorIs(t, err, discoverErr)
_, err = n.InterfaceByIndex(1)
require.ErrorIs(t, err, discoverErr)
}
func TestNet_InterfacesReturnsCopy(t *testing.T) {
d := &countingDiscover{list: []*transport.Interface{testIFace(1, "eth0")}}
n := newTestNet(t, d)
iFaces, err := n.Interfaces()
require.NoError(t, err)
require.Len(t, iFaces, 1)
iFaces[0] = testIFace(2, "tampered")
iFaces, err = n.Interfaces()
require.NoError(t, err)
require.Len(t, iFaces, 1)
assert.Equal(t, "eth0", iFaces[0].Name)
}