diff --git a/intra/core/expiringmap.go b/intra/core/expiringmap.go index c89ce3f6..eb77960e 100644 --- a/intra/core/expiringmap.go +++ b/intra/core/expiringmap.go @@ -107,13 +107,6 @@ func (m *ExpMap[P, Q]) Get(key P) uint32 { return v.hits } -func (m *ExpMap[P, Q]) SetMin(key P) uint32 { - if done(m.ctx) || m.minlife <= 0 { - return 0 - } - return m.Set(key, m.minlife) -} - // Set sets the expiry for the given key and returns the number of hits. // expiry is clamped to minlife. If the key was expired, its hit window // is reset to 0 before returning. Value is set to Q's zero value. diff --git a/intra/icmp.go b/intra/icmp.go index 844b08b1..e8e1c83b 100644 --- a/intra/icmp.go +++ b/intra/icmp.go @@ -24,23 +24,13 @@ import ( type icmpHandler struct { *baseHandler - staller *core.ExpMap[netip.AddrPort, string] // src(addr:port) -> stallSecs } var _ netstack.GICMPHandler = (*icmpHandler)(nil) -// pings allowed per source within icmpFloodTrackTTL, after -// which each ping is stalled for up to icmptarpitMaxSecs. -const ( - icmpFloodHits = 10 // pings allowed per source per window - icmptarpitMaxSecs = 5 // max stall per ping - icmpFloodTrackTTL = 10 * time.Second -) - func NewICMPHandler(pctx context.Context, resolver dnsx.Resolver, prox ipn.ProxyProvider, listener Listener) netstack.GICMPHandler { h := &icmpHandler{ baseHandler: newBaseHandler(pctx, "icmp", resolver, prox, listener), - staller: core.NewExpiringMapLifetime[netip.AddrPort, string](pctx, "icmp.staller", icmpFloodTrackTTL), } core.Gx("icmp.ps", h.processSummaries) @@ -49,14 +39,6 @@ func NewICMPHandler(pctx context.Context, resolver dnsx.Resolver, prox ipn.Proxy return h } -func (h *icmpHandler) maybeStall(src netip.AddrPort) (secs uint32) { - if n := h.staller.Get(src); n > icmpFloodHits { - secs = icmptarpitMaxSecs - } - h.staller.SetMin(src) // track for icmpFloodTrackTTL - return -} - // Ping implements netstack.GICMPHandler. Takes ownership of msg. // Nb: to send icmp pings, root access is required; and so, // send "unprivileged" icmp pings via udp reqs; which do @@ -112,12 +94,6 @@ func (h *icmpHandler) Ping(msg []byte, source, target netip.AddrPort) (echoed bo return false // denied } - // delay flooders; this fn is async, so stalling doesn't block dispatchers - if secs := h.maybeStall(source); secs > 0 { - log.I("t.icmp: flood: stalled %s => %s for %ds", source, target, secs) - time.Sleep(time.Duration(secs) * time.Second) - } - if px, err = h.prox.ProxyTo(cid, dst, "icmp", uid, pids); err != nil || px == nil { err = log.EE("t.icmp: egress: no proxy(%s); err %v", pids, err) return false // denied diff --git a/intra/ipn/proxies.go b/intra/ipn/proxies.go index 797941b4..f752fd3b 100644 --- a/intra/ipn/proxies.go +++ b/intra/ipn/proxies.go @@ -929,7 +929,26 @@ func (px *proxifier) proxyFor(id string) (Proxy, error) { // Ingress (dummy): no fast path, fall through to general lookup } - timeout := time.Duration(minWaitPeriodSec/2) * time.Second + // Regression fix: this used to be getproxytimeout (5s) and was + // inadvertently shortened to minWaitPeriodSec/2 (1s) in 8677a52c + // ("core/volatile: cr by muse spark" era commit chain). proxyFor is + // called for every proxy id, including non-wellknown, app-registered + // ids (see isWellknown/ProxyFor above) for which there is NO retry/ + // wait fallback -- ProxyFor returns immediately with errProxyNotFound + // for those ids, so this is the *only* window a caller gets to find + // a just-registered proxy. The lookup itself is a cheap RLock'd map + // read (see below), but on loaded/low-RAM devices the paired Lock() + // in AddProxy/RemoveProxy can legitimately hold the mutex for longer + // than 1s during proxy setup/teardown, especially for proxies that do + // real I/O in their constructor. Shortening this guard to 1s turns a + // rare, recoverable stall into a hard, unretried lookup failure for + // any non-wellknown proxy id registered right around this window -- + // observed in production as a permanently-failing custom local proxy + // route until the next reconnect. Restoring getproxytimeout (5s) + // keeps this a deadlock-recovery guard (its original documented + // purpose, see the ProxyFor doc-comment above) rather than a + // register-race timeout. + timeout := getproxytimeout // go.dev/play/p/xCug1W3OcMH p, completed := core.Grx("pxr.ProxyFor: "+id, func(_ context.Context) (Proxy, error) { px.RLock() diff --git a/intra/ipn/proxy.go b/intra/ipn/proxy.go index cc22cd88..86c4b918 100644 --- a/intra/ipn/proxy.go +++ b/intra/ipn/proxy.go @@ -766,9 +766,6 @@ func healthy(p Proxy) error { age := now - stat.LastOpen oldEnough := age > ageThreshold.Milliseconds() - - lastGoodRx := now - stat.LastGoodRx - lastGoodTx := now - stat.LastGoodTx lastOK := stat.LastOK lastOKNeverOK := lastOK <= 0 lastOKBeyondThres := lastOK > 0 && now-lastOK > lastOKThreshold.Milliseconds() @@ -778,10 +775,8 @@ func healthy(p Proxy) error { pid, core.FmtMillis(age), pxstatus(status), lastOKNeverOK, lastOKBeyondThres) } else if now-lastOK > tzzTimeout.Milliseconds() { core.Gx("proxy.health.TZZ."+pid, func() { p.Ping() }) - } else if lastGoodTx > tzzTimeout.Milliseconds() || lastGoodRx > tzzTimeout.Milliseconds() { - core.Gx("proxy.health.TxRx."+pid, func() { p.Ping() }) } else if status != TOK { - core.Gx("proxy.health.TNOK."+pid, func() { p.Ping() }) + core.Gx("proxy.health.TOK."+pid, func() { p.Ping() }) } return nil // ok diff --git a/intra/netstack/icmp.go b/intra/netstack/icmp.go index 6670e36e..5abe7987 100644 --- a/intra/netstack/icmp.go +++ b/intra/netstack/icmp.go @@ -84,12 +84,6 @@ func (f *icmpForwarder) reply4(id stack.TransportEndpointID, pkt *stack.PacketBu log.D("icmp: v4: %s: type %v passthrough", f.o, hdr.Type()) return // not handled } - // consult the stack-wide ICMP rate limiter; see: stackopts.go:SetNetstackOpts - // github.com/google/gvisor/blob/738e1d995f/pkg/tcpip/network/ipv4/icmp.go - if !f.s.AllowICMPMessage() { - log.V("icmp: v4: %s: rate limited; dropping echo %s => %s", f.o, src, dst) - return true // handled (silently dropped) - } ipHdr := header.IPv4(l3hdr) replyData := stack.PayloadSince(pkt.TransportHeader()) localAddressBroadcast := pkt.NetworkPacketInfo.LocalAddressBroadcast @@ -203,14 +197,6 @@ func (f *icmpForwarder) reply6(id stack.TransportEndpointID, pkt *stack.PacketBu } l3 := pkt.Network() // l3.Dst == id.LocalAddr and l3.Src == id.RemoteAddr - - // consult the stack-wide ICMP rate limiter before; see: stackopts.go:SetNetstackOpts - // github.com/google/gvisor/blob/738e1d995f/pkg/tcpip/network/ipv6/icmp.go - if !f.s.AllowICMPMessage() { - log.V("icmp: v6: %s: rate limited; dropping echo %s => %s", f.o, l3.DestinationAddress(), l3.SourceAddress()) - return true // handled (silently dropped) - } - route, err := f.s.FindRoute(pkt.NICID, l3.DestinationAddress(), l3.SourceAddress(), pkt.NetworkProtocolNumber, false) if err != nil { log.W("icmp: v6: %s: no route on %v to %s <= %s", f.o, pkt.NICID, l3.DestinationAddress(), l3.SourceAddress()) diff --git a/intra/netstack/icmpecho.go b/intra/netstack/icmpecho.go index ba156c8a..4b2c28f4 100644 --- a/intra/netstack/icmpecho.go +++ b/intra/netstack/icmpecho.go @@ -217,12 +217,6 @@ func (r *icmpResponder) process(h *icmpForwarder, nic tcpip.NICID, pkt *wire.Par return } - // consult the stack-wide ICMP rate limiter before; see: stackopts.go:SetNetstackOpts - if h.s != nil && !h.s.AllowICMPMessage() { - logwv(true)("icmp: responder: rate limited; dropping ping %s => %s", src, dst) - return - } - pinged := h.h.Ping(icmpMsg, src, dst) resp, proto, l4proto, tag, err := r.echoReply(pkt, payload, pinged) diff --git a/intra/netstack/stackopts.go b/intra/netstack/stackopts.go index 9cad91f3..e301ec2d 100644 --- a/intra/netstack/stackopts.go +++ b/intra/netstack/stackopts.go @@ -7,7 +7,6 @@ package netstack import ( - "golang.org/x/time/rate" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/network/ipv4" "gvisor.dev/gvisor/pkg/tcpip/network/ipv6" @@ -15,20 +14,7 @@ import ( "gvisor.dev/gvisor/pkg/tcpip/transport/tcp" ) -const ( - // icmpPingLimit caps generated ICMP messages per second: - // Firestack client code must consult Stack.AllowICMPMessage() - // but it auto-applies to gVisor generated ICMP errors (ipv4.go:allowICMPReply) - icmpPingLimit = rate.Limit(10) - // icmpPingBurst caps the initial burst: - // Firestack client code must consult Stack.AllowICMPMessage() - icmpPingBurst = 7 -) - func SetNetstackOpts(s *stack.Stack) { - s.SetICMPLimit(icmpPingLimit) - s.SetICMPBurst(icmpPingBurst) - // TODO: other stack otps? // github.com/xjasonlyu/tun2socks/blob/31468620e/core/option/option.go#L69