Make nic.spoofing and nic.promiscuous atomic Bools to avoid lock contention.

Also add atomicbitops.Bool which is just a Uint32, but behaves as a boolean.

PiperOrigin-RevId: 495103870

pkg/atomicbitops/BUILD

@@ -14,6 +14,7 @@ go_library(
         "atomicbitops_arm64.go",
         "atomicbitops_arm64.s",
         "atomicbitops_noasm.go",
+        "bool.go",
     ],
     visibility = ["//:sandbox"],
     deps = [

pkg/atomicbitops/bool.go

@@ -0,0 +1,71 @@
+// Copyright 2022 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 atomicbitops
+
+import "sync/atomic"
+
+// Bool is an atomic Boolean.
+//
+// It is implemented by a Uint32, with value 0 indicating false, and 1
+// indicating true.
+//
+// +stateify savable
+type Bool struct {
+	Uint32
+}
+
+// FromBool returns an Bool initialized to value val.
+//
+//go:nosplit
+func FromBool(val bool) Bool {
+	var u uint32
+	if val {
+		u = 1
+	}
+	return Bool{
+		Uint32{
+			value: u,
+		},
+	}
+}
+
+// Load is analogous to atomic.LoadBool, if such a thing existed.
+//
+//go:nosplit
+func (b *Bool) Load() bool {
+	return atomic.LoadUint32(&b.value) == 1
+}
+
+// Store is analogous to atomic.StoreBool, if such a thing existed.
+//
+//go:nosplit
+func (b *Bool) Store(val bool) {
+	var u uint32
+	if val {
+		u = 1
+	}
+	atomic.StoreUint32(&b.value, u)
+}
+
+// Swap is analogous to atomic.SwapBool, if such a thing existed.
+//
+//go:nosplit
+func (b *Bool) Swap(val bool) bool {
+	var u uint32
+	if val {
+		u = 1
+	}
+	return atomic.SwapUint32(&b.value, u) == 1
+}

pkg/tcpip/stack/nic.go

@@ -44,28 +44,28 @@ type nic struct {
 
 	stats sharedStats
 
+	// mu protects annotated fields below.
+	mu nicRWMutex
+
 	// The network endpoints themselves may be modified by calling the interface's
 	// methods, but the map reference and entries must be constant.
 	networkEndpoints          map[tcpip.NetworkProtocolNumber]NetworkEndpoint
 	linkAddrResolvers         map[tcpip.NetworkProtocolNumber]*linkResolver
 	duplicateAddressDetectors map[tcpip.NetworkProtocolNumber]DuplicateAddressDetector
 
-	// enabled is set to 1 when the NIC is enabled and 0 when it is disabled.
-	enabled atomicbitops.Uint32
+	// enabled indicates whether the NIC is enabled.
+	enabled atomicbitops.Bool
+
+	// spoofing indicates whether the NIC is spoofing.
+	spoofing atomicbitops.Bool
+
+	// promiscuous indicates whether the NIC is promiscuous.
+	promiscuous atomicbitops.Bool
 
 	// linkResQueue holds packets that are waiting for link resolution to
 	// complete.
 	linkResQueue packetsPendingLinkResolution
 
-	// mu protects annotated fields below.
-	mu nicRWMutex
-
-	// +checklocks:mu
-	spoofing bool
-
-	// +checklocks:mu
-	promiscuous bool
-
 	// packetEPsMu protects annotated fields below.
 	packetEPsMu packetEPsRWMutex
 
@@ -212,17 +212,14 @@ func (n *nic) getNetworkEndpoint(proto tcpip.NetworkProtocolNumber) NetworkEndpo
 
 // Enabled implements NetworkInterface.
 func (n *nic) Enabled() bool {
-	return n.enabled.Load() == 1
+	return n.enabled.Load()
 }
 
 // setEnabled sets the enabled status for the NIC.
 //
 // Returns true if the enabled status was updated.
 func (n *nic) setEnabled(v bool) bool {
-	if v {
-		return n.enabled.Swap(1) == 0
-	}
-	return n.enabled.Swap(0) == 1
+	return n.enabled.Swap(v) != v
 }
 
 // disable disables n.
@@ -322,17 +319,12 @@ func (n *nic) remove() tcpip.Error {
 
 // setPromiscuousMode enables or disables promiscuous mode.
 func (n *nic) setPromiscuousMode(enable bool) {
-	n.mu.Lock()
-	n.promiscuous = enable
-	n.mu.Unlock()
+	n.promiscuous.Store(enable)
 }
 
 // Promiscuous implements NetworkInterface.
 func (n *nic) Promiscuous() bool {
-	n.mu.RLock()
-	rv := n.promiscuous
-	n.mu.RUnlock()
-	return rv
+	return n.promiscuous.Load()
 }
 
 // IsLoopback implements NetworkInterface.
@@ -403,16 +395,12 @@ func (n *nic) writeRawPacket(pkt PacketBufferPtr) tcpip.Error {
 
 // setSpoofing enables or disables address spoofing.
 func (n *nic) setSpoofing(enable bool) {
-	n.mu.Lock()
-	n.spoofing = enable
-	n.mu.Unlock()
+	n.spoofing.Store(enable)
 }
 
 // Spoofing implements NetworkInterface.
 func (n *nic) Spoofing() bool {
-	n.mu.RLock()
-	defer n.mu.RUnlock()
-	return n.spoofing
+	return n.spoofing.Load()
 }
 
 // primaryAddress returns an address that can be used to communicate with
@@ -428,11 +416,7 @@ func (n *nic) primaryEndpoint(protocol tcpip.NetworkProtocolNumber, remoteAddr t
 		return nil
 	}
 
-	n.mu.RLock()
-	spoofing := n.spoofing
-	n.mu.RUnlock()
-
-	return addressableEndpoint.AcquireOutgoingPrimaryAddress(remoteAddr, spoofing)
+	return addressableEndpoint.AcquireOutgoingPrimaryAddress(remoteAddr, n.Spoofing())
 }
 
 type getAddressBehaviour int
@@ -476,15 +460,13 @@ func (n *nic) findEndpoint(protocol tcpip.NetworkProtocolNumber, address tcpip.A
 // If the address is the IPv4 broadcast address for an endpoint's network, that
 // endpoint will be returned.
 func (n *nic) getAddressOrCreateTemp(protocol tcpip.NetworkProtocolNumber, address tcpip.Address, peb PrimaryEndpointBehavior, tempRef getAddressBehaviour) AssignableAddressEndpoint {
-	n.mu.RLock()
 	var spoofingOrPromiscuous bool
 	switch tempRef {
 	case spoofing:
-		spoofingOrPromiscuous = n.spoofing
+		spoofingOrPromiscuous = n.Spoofing()
 	case promiscuous:
-		spoofingOrPromiscuous = n.promiscuous
+		spoofingOrPromiscuous = n.Promiscuous()
 	}
-	n.mu.RUnlock()
 	return n.getAddressOrCreateTempInner(protocol, address, spoofingOrPromiscuous, peb)
 }
 
@@ -961,10 +943,7 @@ func (n *nic) unregisterPacketEndpoint(netProto tcpip.NetworkProtocolNumber, ep
 // packet. It requires the endpoint to not be marked expired (i.e., its address
 // has been removed) unless the NIC is in spoofing mode, or temporary.
 func (n *nic) isValidForOutgoing(ep AssignableAddressEndpoint) bool {
-	n.mu.RLock()
-	spoofing := n.spoofing
-	n.mu.RUnlock()
-	return n.Enabled() && ep.IsAssigned(spoofing)
+	return n.Enabled() && ep.IsAssigned(n.Spoofing())
 }
 
 // HandleNeighborProbe implements NetworkInterface.

pkg/tcpip/stack/nic_test.go

@@ -204,7 +204,7 @@ func TestDisabledRxStatsWhenNICDisabled(t *testing.T) {
 func TestPacketWithUnknownNetworkProtocolNumber(t *testing.T) {
 	nic := nic{
 		stats:   makeNICStats(tcpip.NICStats{}.FillIn()),
-		enabled: atomicbitops.FromUint32(1),
+		enabled: atomicbitops.FromBool(true),
 	}
 	// IPv4 isn't recognized since we haven't initialized the NIC with an IPv4
 	// endpoint.
@@ -224,7 +224,7 @@ func TestPacketWithUnknownTransportProtocolNumber(t *testing.T) {
 	nic := nic{
 		stack:   &Stack{},
 		stats:   makeNICStats(tcpip.NICStats{}.FillIn()),
-		enabled: atomicbitops.FromUint32(1),
+		enabled: atomicbitops.FromBool(true),
 	}
 	// UDP isn't recognized since we haven't initialized the NIC with a UDP
 	// protocol.