diff --git a/pkg/tcpip/stack/iptables.go b/pkg/tcpip/stack/iptables.go index 27da52d7b..da79f2bb5 100644 --- a/pkg/tcpip/stack/iptables.go +++ b/pkg/tcpip/stack/iptables.go @@ -180,7 +180,6 @@ func DefaultTables(clock tcpip.Clock, rand *rand.Rand) *IPTables { clock: clock, rand: rand, }, - reaperDone: make(chan struct{}, 1), } } @@ -545,7 +544,7 @@ func (it *IPTables) check(table Table, hook Hook, pkt *PacketBuffer, r *Route, a // beforeSave is invoked by stateify. func (it *IPTables) beforeSave() { // Ensure the reaper exits cleanly. - it.reaperDone <- struct{}{} + it.reaper.Stop() // Prevent others from modifying the connection table. it.connections.mu.Lock() } @@ -555,21 +554,13 @@ func (it *IPTables) afterLoad() { it.startReaper(reaperDelay) } -// startReaper starts a goroutine that wakes up periodically to reap timed out -// connections. +// startReaper periodically reaps timed out connections. func (it *IPTables) startReaper(interval time.Duration) { - go func() { // S/R-SAFE: reaperDone is signalled when iptables is saved. - bucket := 0 - for { - select { - case <-it.reaperDone: - return - // TODO(gvisor.dev/issue/5939): do not use the ambient clock. - case <-time.After(interval): - bucket, interval = it.connections.reapUnused(bucket, interval) - } - } - }() + bucket := 0 + it.reaper = it.connections.clock.AfterFunc(interval, func() { + bucket, interval = it.connections.reapUnused(bucket, interval) + it.reaper.Reset(interval) + }) } // Preconditions: diff --git a/pkg/tcpip/stack/iptables_test.go b/pkg/tcpip/stack/iptables_test.go index fd84bf525..57948ad50 100644 --- a/pkg/tcpip/stack/iptables_test.go +++ b/pkg/tcpip/stack/iptables_test.go @@ -121,7 +121,9 @@ func TestNATedConnectionReap(t *testing.T) { // Stop the reaper if it is running so we can reap manually as it is started // on the first change to IPTables. - iptables.reaperDone <- struct{}{} + if !iptables.reaper.Stop() { + t.Fatal("failed to stop reaper") + } pkt := v6PacketBuffer() @@ -224,7 +226,7 @@ func TestNATedConnectionReap(t *testing.T) { bkt.mu.RLock() defer bkt.mu.RUnlock() for tuple := bkt.tuples.Front(); tuple != nil; tuple = tuple.Next() { - if tuple.id() == originalTID { + if tuple.id() == tid { t.Errorf("unexpectedly found tuple with ID = %#v; reply = %t", tid, reply) } } diff --git a/pkg/tcpip/stack/iptables_types.go b/pkg/tcpip/stack/iptables_types.go index 25b51acaa..d77ed1c3a 100644 --- a/pkg/tcpip/stack/iptables_types.go +++ b/pkg/tcpip/stack/iptables_types.go @@ -83,8 +83,7 @@ const ( type IPTables struct { connections ConnTrack - // reaperDone can be signaled to stop the reaper goroutine. - reaperDone chan struct{} + reaper tcpip.Timer mu sync.RWMutex // v4Tables and v6tables map tableIDs to tables. They hold builtin