don't send recv errors for packets outside the connection window anymore (#1463)

* don't send recv errors for packets outside the connection window anymore

* Pull in fix from #1459, add my opinion on maxRecvError

* remove recv_error counter entirely

hostmap.go

@@ -17,12 +17,10 @@ import (
 	"github.com/slackhq/nebula/header"
 )
 
-// const ProbeLen = 100
 const defaultPromoteEvery = 1000       // Count of packets sent before we try moving a tunnel to a preferred underlay ip address
 const defaultReQueryEvery = 5000       // Count of packets sent before re-querying a hostinfo to the lighthouse
 const defaultReQueryWait = time.Minute // Minimum amount of seconds to wait before re-querying a hostinfo the lighthouse. Evaluated every ReQueryEvery
 const MaxRemotes = 10
-const maxRecvError = 4
 
 // MaxHostInfosPerVpnIp is the max number of hostinfos we will track for a given vpn ip
 // 5 allows for an initial handshake and each host pair re-handshaking twice
@@ -226,7 +224,6 @@ type HostInfo struct {
 	// The host may have other vpn addresses that are outside our
 	// vpn networks but were removed because they are not usable
 	vpnAddrs []netip.Addr
-	recvError atomic.Uint32
 
 	// networks are both all vpn and unsafe networks assigned to this host
 	networks   *bart.Lite
@@ -733,13 +730,6 @@ func (i *HostInfo) SetRemoteIfPreferred(hm *HostMap, newRemote netip.AddrPort) b
 	return false
 }
 
-func (i *HostInfo) RecvErrorExceeded() bool {
-	if i.recvError.Add(1) >= maxRecvError {
-		return true
-	}
-	return true
-}
-
 func (i *HostInfo) buildNetworks(networks, unsafeNetworks []netip.Prefix) {
 	if len(networks) == 1 && len(unsafeNetworks) == 0 {
 		// Simple case, no CIDRTree needed

outside.go

@@ -254,16 +254,18 @@ func (f *Interface) handleHostRoaming(hostinfo *HostInfo, udpAddr netip.AddrPort
 
 }
 
+// handleEncrypted returns true if a packet should be processed, false otherwise
 func (f *Interface) handleEncrypted(ci *ConnectionState, addr netip.AddrPort, h *header.H) bool {
-	// If connectionstate exists and the replay protector allows, process packet
+	// If connectionstate does not exist, send a recv error, if possible, to encourage a fast reconnect
-	// Else, send recv errors for 300 seconds after a restart to allow fast reconnection.
+	if ci == nil {
-	if ci == nil || !ci.window.Check(f.l, h.MessageCounter) {
 		if addr.IsValid() {
 			f.maybeSendRecvError(addr, h.RemoteIndex)
-			return false
+		}
-		} else {
 		return false
 	}
+	// If the window check fails, refuse to process the packet, but don't send a recv error
+	if !ci.window.Check(f.l, h.MessageCounter) {
+		return false
 	}
 
 	return true
@@ -537,10 +539,6 @@ func (f *Interface) handleRecvError(addr netip.AddrPort, h *header.H) {
 		return
 	}
 
-	if !hostinfo.RecvErrorExceeded() {
-		return
-	}
-
 	if hostinfo.remote.IsValid() && hostinfo.remote != addr {
 		f.l.Infoln("Someone spoofing recv_errors? ", addr, hostinfo.remote)
 		return