From 211bbf82ad2f490ed7215568c2065d76dfa682ca Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Mon, 18 Oct 2021 15:07:06 -0700 Subject: conntrack: use tcpip.Clock instead of time.Time - We should be using a monotonic clock - This will make future testing easier Updates #6748. PiperOrigin-RevId: 404072318 --- pkg/sentry/socket/netfilter/netfilter.go | 4 +-- pkg/tcpip/stack/BUILD | 1 - pkg/tcpip/stack/conntrack.go | 25 +++++++++--------- pkg/tcpip/stack/iptables.go | 5 ++-- pkg/tcpip/stack/iptables_state.go | 44 -------------------------------- pkg/tcpip/stack/stack.go | 4 +-- 6 files changed, 20 insertions(+), 63 deletions(-) delete mode 100644 pkg/tcpip/stack/iptables_state.go diff --git a/pkg/sentry/socket/netfilter/netfilter.go b/pkg/sentry/socket/netfilter/netfilter.go index e3eade180..8d9e73243 100644 --- a/pkg/sentry/socket/netfilter/netfilter.go +++ b/pkg/sentry/socket/netfilter/netfilter.go @@ -58,8 +58,8 @@ var nameToID = map[string]stack.TableID{ // DefaultLinuxTables returns the rules of stack.DefaultTables() wrapped for // compatibility with netfilter extensions. -func DefaultLinuxTables(seed uint32) *stack.IPTables { - tables := stack.DefaultTables(seed) +func DefaultLinuxTables(seed uint32, clock tcpip.Clock) *stack.IPTables { + tables := stack.DefaultTables(seed, clock) tables.VisitTargets(func(oldTarget stack.Target) stack.Target { switch val := oldTarget.(type) { case *stack.AcceptTarget: diff --git a/pkg/tcpip/stack/BUILD b/pkg/tcpip/stack/BUILD index 6c42ab29b..6999add78 100644 --- a/pkg/tcpip/stack/BUILD +++ b/pkg/tcpip/stack/BUILD @@ -48,7 +48,6 @@ go_library( "hook_string.go", "icmp_rate_limit.go", "iptables.go", - "iptables_state.go", "iptables_targets.go", "iptables_types.go", "neighbor_cache.go", diff --git a/pkg/tcpip/stack/conntrack.go b/pkg/tcpip/stack/conntrack.go index eee0fc20c..3583d93c6 100644 --- a/pkg/tcpip/stack/conntrack.go +++ b/pkg/tcpip/stack/conntrack.go @@ -122,14 +122,12 @@ type conn struct { // lastUsed is the last time the connection saw a relevant packet, and // is updated by each packet on the connection. // - // TODO(gvisor.dev/issue/5939): do not use the ambient clock. - // // +checklocks:mu - lastUsed time.Time `state:".(unixTime)"` + lastUsed tcpip.MonotonicTime } // timedOut returns whether the connection timed out based on its state. -func (cn *conn) timedOut(now time.Time) bool { +func (cn *conn) timedOut(now tcpip.MonotonicTime) bool { const establishedTimeout = 5 * 24 * time.Hour const defaultTimeout = 120 * time.Second cn.mu.RLock() @@ -190,6 +188,9 @@ type ConnTrack struct { // It is immutable. seed uint32 + // clock provides timing used to determine conntrack reapings. + clock tcpip.Clock + mu sync.RWMutex `state:"nosave"` // mu protects the buckets slice, but not buckets' contents. Only take // the write lock if you are modifying the slice or saving for S/R. @@ -248,7 +249,7 @@ func (ct *ConnTrack) getConnOrMaybeInsertNoop(pkt *PacketBuffer) *tuple { bkt := &ct.buckets[bktID] ct.mu.RUnlock() - now := time.Now() + now := ct.clock.NowMonotonic() if t := bkt.connForTID(tid, now); t != nil { return t } @@ -294,17 +295,17 @@ func (ct *ConnTrack) connForTID(tid tupleID) *tuple { bkt := &ct.buckets[bktID] ct.mu.RUnlock() - return bkt.connForTID(tid, time.Now()) + return bkt.connForTID(tid, ct.clock.NowMonotonic()) } -func (bkt *bucket) connForTID(tid tupleID, now time.Time) *tuple { +func (bkt *bucket) connForTID(tid tupleID, now tcpip.MonotonicTime) *tuple { bkt.mu.RLock() defer bkt.mu.RUnlock() return bkt.connForTIDRLocked(tid, now) } // +checklocksread:bkt.mu -func (bkt *bucket) connForTIDRLocked(tid tupleID, now time.Time) *tuple { +func (bkt *bucket) connForTIDRLocked(tid tupleID, now tcpip.MonotonicTime) *tuple { for other := bkt.tuples.Front(); other != nil; other = other.Next() { if tid == other.id() && !other.conn.timedOut(now) { return other @@ -324,7 +325,7 @@ func (ct *ConnTrack) finalize(cn *conn) { bkt.mu.Lock() defer bkt.mu.Unlock() - if t := bkt.connForTIDRLocked(tid, time.Now()); t != nil { + if t := bkt.connForTIDRLocked(tid, ct.clock.NowMonotonic()); t != nil { // Another connection for the reply already exists. We can't do much about // this so we leave the connection cn represents in a state where it can // send packets but its responses will be mapped to some other connection. @@ -476,7 +477,7 @@ func (cn *conn) handlePacket(pkt *PacketBuffer, hook Hook, r *Route) bool { } // Mark the connection as having been used recently so it isn't reaped. - cn.lastUsed = time.Now() + cn.lastUsed = cn.ct.clock.NowMonotonic() // Update connection state. cn.updateLocked(pkt, reply) @@ -548,7 +549,7 @@ func (ct *ConnTrack) reapUnused(start int, prevInterval time.Duration) (int, tim const minInterval = 10 * time.Millisecond const maxInterval = maxFullTraversal / fractionPerReaping - now := time.Now() + now := ct.clock.NowMonotonic() checked := 0 expired := 0 var idx int @@ -592,7 +593,7 @@ func (ct *ConnTrack) reapUnused(start int, prevInterval time.Duration) (int, tim // Precondition: ct.mu is read locked and bkt.mu is write locked. // +checklocksread:ct.mu // +checklocks:bkt.mu -func (ct *ConnTrack) reapTupleLocked(tuple *tuple, bktID int, bkt *bucket, now time.Time) bool { +func (ct *ConnTrack) reapTupleLocked(tuple *tuple, bktID int, bkt *bucket, now tcpip.MonotonicTime) bool { if !tuple.conn.timedOut(now) { return false } diff --git a/pkg/tcpip/stack/iptables.go b/pkg/tcpip/stack/iptables.go index 0baa378ea..fd61387bf 100644 --- a/pkg/tcpip/stack/iptables.go +++ b/pkg/tcpip/stack/iptables.go @@ -42,7 +42,7 @@ const reaperDelay = 5 * time.Second // DefaultTables returns a default set of tables. Each chain is set to accept // all packets. -func DefaultTables(seed uint32) *IPTables { +func DefaultTables(seed uint32, clock tcpip.Clock) *IPTables { return &IPTables{ v4Tables: [NumTables]Table{ NATID: { @@ -182,7 +182,8 @@ func DefaultTables(seed uint32) *IPTables { Postrouting: {MangleID, NATID}, }, connections: ConnTrack{ - seed: seed, + seed: seed, + clock: clock, }, reaperDone: make(chan struct{}, 1), } diff --git a/pkg/tcpip/stack/iptables_state.go b/pkg/tcpip/stack/iptables_state.go deleted file mode 100644 index 3d3c39c20..000000000 --- a/pkg/tcpip/stack/iptables_state.go +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2020 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package stack - -import ( - "time" -) - -// +stateify savable -type unixTime struct { - second int64 - nano int64 -} - -// saveLastUsed is invoked by stateify. -func (cn *conn) saveLastUsed() unixTime { - cn.mu.Lock() - defer cn.mu.Unlock() - return unixTime{cn.lastUsed.Unix(), cn.lastUsed.UnixNano()} -} - -// loadLastUsed is invoked by stateify. -func (cn *conn) loadLastUsed(unix unixTime) { - cn.mu.Lock() - defer cn.mu.Unlock() - cn.lastUsed = time.Unix(unix.second, unix.nano) -} - -// beforeSave is invoked by stateify. -func (ct *ConnTrack) beforeSave() { - ct.mu.Lock() -} diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 428350f31..ee6767654 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -238,7 +238,7 @@ type Options struct { // DefaultIPTables is an optional iptables rules constructor that is called // if IPTables is nil. If both fields are nil, iptables will allow all // traffic. - DefaultIPTables func(uint32) *IPTables + DefaultIPTables func(seed uint32, clock tcpip.Clock) *IPTables // SecureRNG is a cryptographically secure random number generator. SecureRNG io.Reader @@ -358,7 +358,7 @@ func New(opts Options) *Stack { if opts.DefaultIPTables == nil { opts.DefaultIPTables = DefaultTables } - opts.IPTables = opts.DefaultIPTables(seed) + opts.IPTables = opts.DefaultIPTables(seed, clock) } opts.NUDConfigs.resetInvalidFields() -- cgit v1.2.3