From 6671a42d605d681e6aa63b610617523ab632e95a Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Mon, 8 Feb 2021 21:39:29 -0800 Subject: Remove unnecessary locking The thing the lock protects will never be accessed concurrently. PiperOrigin-RevId: 356423331 --- pkg/tcpip/network/ipv6/ndp.go | 32 +++++--------------------------- pkg/tcpip/stack/neighbor_entry.go | 38 ++++++-------------------------------- 2 files changed, 11 insertions(+), 59 deletions(-) diff --git a/pkg/tcpip/network/ipv6/ndp.go b/pkg/tcpip/network/ipv6/ndp.go index a55330b7e..53c043dcd 100644 --- a/pkg/tcpip/network/ipv6/ndp.go +++ b/pkg/tcpip/network/ipv6/ndp.go @@ -466,20 +466,6 @@ type ndpState struct { temporaryAddressDesyncFactor time.Duration } -type remainingCounter struct { - mu struct { - sync.Mutex - - remaining uint8 - } -} - -func (r *remainingCounter) init(max uint8) { - r.mu.Lock() - defer r.mu.Unlock() - r.mu.remaining = max -} - // defaultRouterState holds data associated with a default router discovered by // a Router Advertisement (RA). type defaultRouterState struct { @@ -1687,7 +1673,8 @@ func (ndp *ndpState) startSolicitingRouters() { return } - if ndp.configs.MaxRtrSolicitations == 0 { + remaining := ndp.configs.MaxRtrSolicitations + if remaining == 0 { return } @@ -1698,9 +1685,6 @@ func (ndp *ndpState) startSolicitingRouters() { delay = time.Duration(rand.Int63n(int64(ndp.configs.MaxRtrSolicitationDelay))) } - var remaining remainingCounter - remaining.init(ndp.configs.MaxRtrSolicitations) - // Protected by ndp.ep.mu. done := false @@ -1754,19 +1738,13 @@ func (ndp *ndpState) startSolicitingRouters() { panic(fmt.Sprintf("failed to add IP header: %s", err)) } - // Okay to hold this lock while writing packets since we use a different - // lock per router solicitaiton timer so there will not be any lock - // contention. - remaining.mu.Lock() - defer remaining.mu.Unlock() - if err := ndp.ep.nic.WritePacketToRemote(header.EthernetAddressFromMulticastIPv6Address(header.IPv6AllRoutersMulticastAddress), nil /* gso */, ProtocolNumber, pkt); err != nil { sent.dropped.Increment() // Don't send any more messages if we had an error. - remaining.mu.remaining = 0 + remaining = 0 } else { sent.routerSolicit.Increment() - remaining.mu.remaining-- + remaining-- } ndp.ep.mu.Lock() @@ -1777,7 +1755,7 @@ func (ndp *ndpState) startSolicitingRouters() { return } - if remaining.mu.remaining == 0 { + if remaining == 0 { // We are done soliciting routers. ndp.stopSolicitingRouters() return diff --git a/pkg/tcpip/stack/neighbor_entry.go b/pkg/tcpip/stack/neighbor_entry.go index 78c6cb681..4ed149ee8 100644 --- a/pkg/tcpip/stack/neighbor_entry.go +++ b/pkg/tcpip/stack/neighbor_entry.go @@ -146,20 +146,6 @@ func newStaticNeighborEntry(cache *neighborCache, addr tcpip.Address, linkAddr t return n } -type remainingCounter struct { - mu struct { - sync.Mutex - - remaining uint32 - } -} - -func (r *remainingCounter) init(max uint32) { - r.mu.Lock() - defer r.mu.Unlock() - r.mu.remaining = max -} - // notifyCompletionLocked notifies those waiting for address resolution, with // the link address if resolution completed successfully. // @@ -298,8 +284,7 @@ func (e *neighborEntry) setStateLocked(next NeighborState) { // Protected by e.mu. done := false - var remaining remainingCounter - remaining.init(config.MaxUnicastProbes) + remaining := config.MaxUnicastProbes addr := e.mu.neigh.Addr linkAddr := e.mu.neigh.LinkAddr @@ -310,13 +295,8 @@ func (e *neighborEntry) setStateLocked(next NeighborState) { e.mu.timer = timer{ done: &done, timer: e.cache.nic.stack.Clock().AfterFunc(0, func() { - // Okay to hold this lock while writing packets since we use a different - // lock per probe timer so there will not be any lock contention. - remaining.mu.Lock() - defer remaining.mu.Unlock() - var err tcpip.Error - timedoutResolution := remaining.mu.remaining == 0 + timedoutResolution := remaining == 0 if !timedoutResolution { err = e.cache.linkRes.LinkAddressRequest(addr, "" /* localAddr */, linkAddr) } @@ -335,7 +315,7 @@ func (e *neighborEntry) setStateLocked(next NeighborState) { return } - remaining.mu.remaining-- + remaining-- e.mu.timer.timer.Reset(config.RetransmitTimer) }), } @@ -373,8 +353,7 @@ func (e *neighborEntry) handlePacketQueuedLocked(localAddr tcpip.Address) { // Protected by e.mu. done := false - var remaining remainingCounter - remaining.init(config.MaxMulticastProbes) + remaining := config.MaxMulticastProbes addr := e.mu.neigh.Addr // Send a probe in another gorountine to free this thread of execution @@ -384,13 +363,8 @@ func (e *neighborEntry) handlePacketQueuedLocked(localAddr tcpip.Address) { e.mu.timer = timer{ done: &done, timer: e.cache.nic.stack.Clock().AfterFunc(0, func() { - // Okay to hold this lock while writing packets since we use a different - // lock per probe timer so there will not be any lock contention. - remaining.mu.Lock() - defer remaining.mu.Unlock() - var err tcpip.Error - timedoutResolution := remaining.mu.remaining == 0 + timedoutResolution := remaining == 0 if !timedoutResolution { // As per RFC 4861 section 7.2.2: // @@ -416,7 +390,7 @@ func (e *neighborEntry) handlePacketQueuedLocked(localAddr tcpip.Address) { return } - remaining.mu.remaining-- + remaining-- e.mu.timer.timer.Reset(config.RetransmitTimer) }), } -- cgit v1.2.3