Skip to content

NM-363: prevent restart loop on repeated IPv6 endpoint detection #1335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
abhishek9686 wants to merge 1 commit into
base: release-v1.6.0
Choose a base branch
from
Draft

NM-363: prevent restart loop on repeated IPv6 endpoint detection #1335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions functions/daemon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ func startGoRoutines(wg *sync.WaitGroup) context.CancelFunc {
}

if !netclientCfg.IsStatic {
if netclientCfg.EndpointIP != nil {
config.HostPublicIP = netclientCfg.EndpointIP
}
if netclientCfg.EndpointIPv6 != nil {
config.HostPublicIP6 = netclientCfg.EndpointIPv6
}
Comment on lines +215 to +220

@tenki-reviewer tenki-reviewer Bot Jun 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Dead EndpointIP→HostPublicIP/HostPublicIP6 seeding code in daemon.go startGoRoutines (bug)

The PR adds seeding of config.HostPublicIP and config.HostPublicIP6 from stored netclientCfg.EndpointIP/EndpointIPv6 in daemon.go:startGoRoutines (lines 215-220). These assignments are immediately overwritten by synchronous holePunchWgPort() calls at line 222 (IPv4) and lines 239-254 (IPv6), before any goroutine or downstream code can read the seeded values. The existing fallback logic in the else branches (lines 229-232 for IPv4, lines 250-253 for IPv6) already restores HostPublicIP/HostPublicIP6 from the stored endpoints when hole punching fails, making the new lines 215-220 entirely dead code. In contrast, the same seeding pattern in mqpublish.go:Checkin (lines 69-76) is effective because it precedes hostServerUpdate without an immediate synchronous overwrite.

💡 Suggestion: Remove the dead seeding block at daemon.go:215-220. The fallback logic at lines 229-232 (IPv4) and 250-253 (IPv6) already handles seeding HostPublicIP/HostPublicIP6 from stored endpoints when hole punching fails.

📋 Prompt for AI Agents

In functions/daemon.go, remove lines 215-220 (the if blocks that seed config.HostPublicIP and config.HostPublicIP6 from netclientCfg.EndpointIP and netclientCfg.EndpointIPv6). These assignments are dead code: line 222 unconditionally overwrites config.HostPublicIP with the holePunchWgPort result, and the existing else branches at lines 229-232 and 250-253 already restore these values from stored endpoints on hole-punch failure.

// IPV4
config.HostPublicIP, config.WgPublicListenPort, config.HostNatType = holePunchWgPort(4, netclientCfg.ListenPort)
slog.Info("wireguard public listen port: ", "port", config.WgPublicListenPort)
Expand Down
40 changes: 32 additions & 8 deletions functions/mqpublish.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ func Checkin(ctx context.Context, wg *sync.WaitGroup) {

ipTicker = time.NewTicker(time.Second * time.Duration(ipTickerIntervalSec))
defer ipTicker.Stop()
if !config.Netclient().IsStatic && config.Netclient().CurrGwNmIP == nil {
if config.HostPublicIP == nil && config.Netclient().EndpointIP != nil {
config.HostPublicIP = config.Netclient().EndpointIP
}
if config.HostPublicIP6 == nil && config.Netclient().EndpointIPv6 != nil {
config.HostPublicIP6 = config.Netclient().EndpointIPv6
}
}
err := hostServerUpdate(models.HostUpdate{Action: models.UpdateHost})
if err != nil {
logger.Log(0, "could not publish endpoint change", err.Error())
Expand Down Expand Up @@ -102,20 +110,36 @@ func Checkin(ctx context.Context, wg *sync.WaitGroup) {
if ip4 == nil && ip6 == nil {
continue
}
if ip4 != nil && ip4.To4() != nil && !ip4.IsUnspecified() && !config.HostPublicIP.Equal(ip4) {
slog.Debug("IP CHECKIN 1", "ipv4", ip4, "HostPublicIP", config.HostPublicIP)
config.HostPublicIP = ip4
restart = true
knownIP4 := config.HostPublicIP
if knownIP4 == nil {
knownIP4 = config.Netclient().EndpointIP
}
if ip4 != nil && ip4.To4() != nil && !ip4.IsUnspecified() {
if !ncutils.IPsEqual(knownIP4, ip4) {
slog.Debug("IP CHECKIN 1", "ipv4", ip4, "HostPublicIP", config.HostPublicIP)
config.HostPublicIP = ip4
restart = true
} else if config.HostPublicIP == nil {
config.HostPublicIP = ip4
}
} else if ip4 == nil && config.HostPublicIP != nil {
slog.Debug("IP CHECKIN 2", "ipv4", ip4, "HostPublicIP", config.HostPublicIP)
config.HostPublicIP = nil
restart = true
}

if ip6 != nil && ip6.To16() != nil && !ip6.IsUnspecified() && !config.HostPublicIP6.Equal(ip6) {
slog.Debug("IP CHECKIN 1", "ipv6", ip6, "HostPublicIP6", config.HostPublicIP6)
config.HostPublicIP6 = ip6
restart = true
knownIP6 := config.HostPublicIP6
if knownIP6 == nil {
knownIP6 = config.Netclient().EndpointIPv6
}
if ip6 != nil && ip6.To16() != nil && !ip6.IsUnspecified() {
if !ncutils.IPsEqual(knownIP6, ip6) {
slog.Debug("IP CHECKIN 1", "ipv6", ip6, "HostPublicIP6", config.HostPublicIP6)
config.HostPublicIP6 = ip6
restart = true
} else if config.HostPublicIP6 == nil {
config.HostPublicIP6 = ip6
}
} else if ip6 == nil && config.HostPublicIP6 != nil {
slog.Debug("IP CHECKIN 2", "ipv6", ip6, "HostPublicIP6", config.HostPublicIP6)
config.HostPublicIP6 = nil
Expand Down
15 changes: 15 additions & 0 deletions ncutils/netclientutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,21 @@ func SetVerbosity(logLevel int) {

}

// IPsEqual compares two IP addresses, normalizing IPv4 and IPv6 representations.
func IPsEqual(a, b net.IP) bool {
if a == nil && b == nil {
return true
}
if a == nil || b == nil {
return false
}
a16, b16 := a.To16(), b.To16()
if a16 == nil || b16 == nil {
return a.Equal(b)
}
return a16.Equal(b16)
}

func TraceCaller() {
// Skip 1 frame to get the caller of this function
pc, file, line, ok := runtime.Caller(2)
Expand Down