From 2e0cc62d827d8fa72c1abe3e0fc6615720753ce0 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Wed, 7 Dec 2022 17:10:49 -0800 Subject: [PATCH] tcpip/stack: use lockdep mutexes PiperOrigin-RevId: 493749454 --- pkg/tcpip/BUILD | 1 + pkg/tcpip/stack/BUILD | 162 ++++++++++++++++++ pkg/tcpip/stack/addressable_endpoint_state.go | 5 +- pkg/tcpip/stack/conntrack.go | 8 +- pkg/tcpip/stack/iptables_types.go | 3 +- pkg/tcpip/stack/neighbor_cache.go | 3 +- pkg/tcpip/stack/neighbor_entry.go | 3 +- pkg/tcpip/stack/nic.go | 7 +- pkg/tcpip/stack/pending_packets.go | 3 +- pkg/tcpip/stack/route.go | 3 +- pkg/tcpip/stack/stack.go | 7 +- pkg/tcpip/stack/transport_demuxer.go | 7 +- 12 files changed, 183 insertions(+), 29 deletions(-) diff --git a/pkg/tcpip/BUILD b/pkg/tcpip/BUILD index 405d03a81..a47d49876 100644 --- a/pkg/tcpip/BUILD +++ b/pkg/tcpip/BUILD @@ -64,6 +64,7 @@ deps_test( "//pkg/state", "//pkg/state/wire", "//pkg/sync", + "//pkg/sync/locking", "//pkg/waiter", "//pkg/xdp", diff --git a/pkg/tcpip/stack/BUILD b/pkg/tcpip/stack/BUILD index 6248ff237..00c889386 100644 --- a/pkg/tcpip/stack/BUILD +++ b/pkg/tcpip/stack/BUILD @@ -1,8 +1,149 @@ load("//tools:defs.bzl", "go_library", "go_test", "most_shards") load("//tools/go_generics:defs.bzl", "go_template_instance") +load("//pkg/sync/locking:locking.bzl", "declare_mutex", "declare_rwmutex") package(licenses = ["notice"]) +declare_rwmutex( + name = "addressable_endpoint_state_mutex", + out = "addressable_endpoint_state_mutex.go", + package = "stack", + prefix = "addressableEndpointState", +) + +declare_rwmutex( + name = "address_state_mutex", + out = "address_state_mutex.go", + package = "stack", + prefix = "addressState", +) + +declare_rwmutex( + name = "route_mutex", + out = "route_mutex.go", + package = "stack", + prefix = "route", +) + +declare_rwmutex( + name = "route_stack_mutex", + out = "route_stack_mutex.go", + package = "stack", + prefix = "routeStack", +) + +declare_rwmutex( + name = "stack_mutex", + out = "stack_mutex.go", + package = "stack", + prefix = "stack", +) + +declare_rwmutex( + name = "nic_mutex", + out = "nic_mutex.go", + package = "stack", + prefix = "nic", +) + +declare_rwmutex( + name = "packet_eps_mutex", + out = "packet_eps_mutex.go", + package = "stack", + prefix = "packetEPs", +) + +declare_rwmutex( + name = "packet_endpoint_list_mutex", + out = "packet_endpoint_list_mutex.go", + package = "stack", + prefix = "packetEndpointList", +) + +declare_rwmutex( + name = "transport_endpoints_mutex", + out = "transport_endpoints_mutex.go", + package = "stack", + prefix = "transportEndpoints", +) + +declare_rwmutex( + name = "endpoints_by_nic_mutex", + out = "endpoints_by_nic_mutex.go", + package = "stack", + prefix = "endpointsByNIC", +) + +declare_rwmutex( + name = "multi_port_endpoint_mutex", + out = "multi_port_endpoint_mutex.go", + package = "stack", + prefix = "multiPortEndpoint", +) + +declare_rwmutex( + name = "neighbor_entry_mutex", + out = "neighbor_entry_mutex.go", + package = "stack", + prefix = "neighborEntry", +) + +declare_rwmutex( + name = "neighbor_cache_mutex", + out = "neighbor_cache_mutex.go", + package = "stack", + prefix = "neighborCache", +) + +declare_rwmutex( + name = "conn_mutex", + out = "conn_mutex.go", + package = "stack", + prefix = "conn", +) + +declare_rwmutex( + name = "state_conn_mutex", + out = "state_conn_mutex.go", + package = "stack", + prefix = "stateConn", +) + +declare_rwmutex( + name = "bucket_mutex", + out = "bucket_mutex.go", + package = "stack", + prefix = "bucket", +) + +declare_rwmutex( + name = "conn_track_mutex", + out = "conn_track_mutex.go", + package = "stack", + prefix = "connTrack", +) + +declare_rwmutex( + name = "iptables_mutex", + out = "iptables_mutex.go", + package = "stack", + prefix = "ipTables", +) + +declare_mutex( + name = "cleanup_endpoints_mutex", + out = "cleanup_endpoints_mutex.go", + package = "stack", + prefix = "cleanupEndpoints", +) + +declare_mutex( + name = "packets_pending_link_resolution_mutex", + out = "packets_pending_link_resolution_mutex.go", + package = "stack", + prefix = "packetsPendingLinkResolution", +) + go_template_instance( name = "neighbor_entry_list", out = "neighbor_entry_list.go", @@ -53,36 +194,56 @@ go_template_instance( go_library( name = "stack", srcs = [ + "address_state_mutex.go", "addressable_endpoint_state.go", + "addressable_endpoint_state_mutex.go", + "bucket_mutex.go", + "cleanup_endpoints_mutex.go", + "conn_mutex.go", + "conn_track_mutex.go", "conntrack.go", + "endpoints_by_nic_mutex.go", "gro.go", "gro_packet_list.go", "headertype_string.go", "hook_string.go", "icmp_rate_limit.go", "iptables.go", + "iptables_mutex.go", "iptables_targets.go", "iptables_types.go", + "multi_port_endpoint_mutex.go", "neighbor_cache.go", + "neighbor_cache_mutex.go", "neighbor_entry.go", "neighbor_entry_list.go", + "neighbor_entry_mutex.go", "neighborstate_string.go", "nic.go", + "nic_mutex.go", "nic_stats.go", "nud.go", "packet_buffer.go", "packet_buffer_list.go", "packet_buffer_refs.go", "packet_buffer_unsafe.go", + "packet_endpoint_list_mutex.go", + "packet_eps_mutex.go", + "packets_pending_link_resolution_mutex.go", "pending_packets.go", "rand.go", "registration.go", "route.go", + "route_mutex.go", + "route_stack_mutex.go", "stack.go", "stack_global_state.go", + "stack_mutex.go", "stack_options.go", + "state_conn_mutex.go", "tcp.go", "transport_demuxer.go", + "transport_endpoints_mutex.go", "tuple_list.go", ], visibility = ["//visibility:public"], @@ -95,6 +256,7 @@ go_library( "//pkg/refs", "//pkg/sleep", "//pkg/sync", + "//pkg/sync/locking", "//pkg/tcpip", "//pkg/tcpip/checksum", "//pkg/tcpip/hash/jenkins", diff --git a/pkg/tcpip/stack/addressable_endpoint_state.go b/pkg/tcpip/stack/addressable_endpoint_state.go index df9953375..22658a133 100644 --- a/pkg/tcpip/stack/addressable_endpoint_state.go +++ b/pkg/tcpip/stack/addressable_endpoint_state.go @@ -17,7 +17,6 @@ package stack import ( "fmt" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" ) @@ -38,7 +37,7 @@ type AddressableEndpointState struct { // // AddressableEndpointState.mu // addressState.mu - mu sync.RWMutex + mu addressableEndpointStateRWMutex // +checklocks:mu endpoints map[tcpip.Address]*addressState // +checklocks:mu @@ -701,7 +700,7 @@ type addressState struct { // // AddressableEndpointState.mu // addressState.mu - mu sync.RWMutex + mu addressStateRWMutex // checklocks:mu refs uint32 // checklocks:mu diff --git a/pkg/tcpip/stack/conntrack.go b/pkg/tcpip/stack/conntrack.go index e2d6ad52c..0618d899b 100644 --- a/pkg/tcpip/stack/conntrack.go +++ b/pkg/tcpip/stack/conntrack.go @@ -139,7 +139,7 @@ type conn struct { // Holds a finalizeResult. finalizeResult atomicbitops.Uint32 - mu sync.RWMutex `state:"nosave"` + mu connRWMutex `state:"nosave"` // sourceManip indicates the source manipulation type. // // +checklocks:mu @@ -149,7 +149,7 @@ type conn struct { // +checklocks:mu destinationManip manipType - stateMu sync.RWMutex `state:"nosave"` + stateMu stateConnRWMutex `state:"nosave"` // tcb is TCB control block. It is used to keep track of states // of tcp connection. // @@ -230,7 +230,7 @@ type ConnTrack struct { clock tcpip.Clock rand *rand.Rand - mu sync.RWMutex `state:"nosave"` + mu connTrackRWMutex `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. // @@ -240,7 +240,7 @@ type ConnTrack struct { // +stateify savable type bucket struct { - mu sync.RWMutex `state:"nosave"` + mu bucketRWMutex `state:"nosave"` // +checklocks:mu tuples tupleList } diff --git a/pkg/tcpip/stack/iptables_types.go b/pkg/tcpip/stack/iptables_types.go index 99b5c9684..ff1c4270a 100644 --- a/pkg/tcpip/stack/iptables_types.go +++ b/pkg/tcpip/stack/iptables_types.go @@ -18,7 +18,6 @@ import ( "fmt" "strings" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/header" ) @@ -85,7 +84,7 @@ type IPTables struct { reaper tcpip.Timer - mu sync.RWMutex + mu ipTablesRWMutex // v4Tables and v6tables map tableIDs to tables. They hold builtin // tables only, not user tables. // diff --git a/pkg/tcpip/stack/neighbor_cache.go b/pkg/tcpip/stack/neighbor_cache.go index be5c66bfe..c63740074 100644 --- a/pkg/tcpip/stack/neighbor_cache.go +++ b/pkg/tcpip/stack/neighbor_cache.go @@ -17,7 +17,6 @@ package stack import ( "fmt" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" ) @@ -48,7 +47,7 @@ type neighborCache struct { linkRes LinkAddressResolver mu struct { - sync.RWMutex + neighborCacheRWMutex cache map[tcpip.Address]*neighborEntry dynamic struct { diff --git a/pkg/tcpip/stack/neighbor_entry.go b/pkg/tcpip/stack/neighbor_entry.go index 0b67acf30..68fe0b21e 100644 --- a/pkg/tcpip/stack/neighbor_entry.go +++ b/pkg/tcpip/stack/neighbor_entry.go @@ -16,7 +16,6 @@ package stack import ( "fmt" - "sync" "time" "gvisor.dev/gvisor/pkg/tcpip" @@ -97,7 +96,7 @@ type neighborEntry struct { nudState *NUDState mu struct { - sync.RWMutex + neighborEntryRWMutex neigh NeighborEntry diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index 7990219d2..1ab92bc5e 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -19,7 +19,6 @@ import ( "reflect" "gvisor.dev/gvisor/pkg/atomicbitops" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/header" ) @@ -59,7 +58,7 @@ type nic struct { linkResQueue packetsPendingLinkResolution // mu protects annotated fields below. - mu sync.RWMutex + mu nicRWMutex // +checklocks:mu spoofing bool @@ -68,7 +67,7 @@ type nic struct { promiscuous bool // packetEPsMu protects annotated fields below. - packetEPsMu sync.RWMutex + packetEPsMu packetEPsRWMutex // eps is protected by the mutex, but the values contained in it are not. // @@ -90,7 +89,7 @@ func makeNICStats(global tcpip.NICStats) sharedStats { } type packetEndpointList struct { - mu sync.RWMutex + mu packetEndpointListRWMutex // eps is protected by mu, but the contained PacketEndpoint values are not. // diff --git a/pkg/tcpip/stack/pending_packets.go b/pkg/tcpip/stack/pending_packets.go index 8faa5b60a..0627fb812 100644 --- a/pkg/tcpip/stack/pending_packets.go +++ b/pkg/tcpip/stack/pending_packets.go @@ -17,7 +17,6 @@ package stack import ( "fmt" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" ) @@ -40,7 +39,7 @@ type packetsPendingLinkResolution struct { nic *nic mu struct { - sync.Mutex + packetsPendingLinkResolutionMutex // The packets to send once the resolver completes. // diff --git a/pkg/tcpip/stack/route.go b/pkg/tcpip/stack/route.go index cd97df596..9a3714cee 100644 --- a/pkg/tcpip/stack/route.go +++ b/pkg/tcpip/stack/route.go @@ -17,7 +17,6 @@ package stack import ( "fmt" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/header" ) @@ -34,7 +33,7 @@ type Route struct { localAddressNIC *nic // mu protects annotated fields below. - mu sync.RWMutex + mu routeRWMutex // localAddressEndpoint is the local address this route is associated with. // +checklocks:mu diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 6df93ff8c..a7f5986b9 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -32,7 +32,6 @@ import ( "gvisor.dev/gvisor/pkg/bufferv2" "gvisor.dev/gvisor/pkg/log" cryptorand "gvisor.dev/gvisor/pkg/rand" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/header" "gvisor.dev/gvisor/pkg/tcpip/ports" @@ -85,18 +84,18 @@ type Stack struct { stats tcpip.Stats // routeMu protects annotated fields below. - routeMu sync.RWMutex + routeMu routeStackRWMutex // +checklocks:routeMu routeTable []tcpip.Route - mu sync.RWMutex + mu stackRWMutex // +checklocks:mu nics map[tcpip.NICID]*nic defaultForwardingEnabled map[tcpip.NetworkProtocolNumber]struct{} // cleanupEndpointsMu protects cleanupEndpoints. - cleanupEndpointsMu sync.Mutex + cleanupEndpointsMu cleanupEndpointsMutex // +checklocks:cleanupEndpointsMu cleanupEndpoints map[TransportEndpoint]struct{} diff --git a/pkg/tcpip/stack/transport_demuxer.go b/pkg/tcpip/stack/transport_demuxer.go index a9602ce1f..adb54ffe4 100644 --- a/pkg/tcpip/stack/transport_demuxer.go +++ b/pkg/tcpip/stack/transport_demuxer.go @@ -17,7 +17,6 @@ package stack import ( "fmt" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/hash/jenkins" "gvisor.dev/gvisor/pkg/tcpip/header" @@ -32,7 +31,7 @@ type protocolIDs struct { // transportEndpoints manages all endpoints of a given protocol. It has its own // mutex so as to reduce interference between protocols. type transportEndpoints struct { - mu sync.RWMutex + mu transportEndpointsRWMutex // +checklocks:mu endpoints map[TransportEndpointID]*endpointsByNIC // rawEndpoints contains endpoints for raw sockets, which receive all @@ -138,7 +137,7 @@ type endpointsByNIC struct { // seed is a random secret for a jenkins hash. seed uint32 - mu sync.RWMutex + mu endpointsByNICRWMutex // +checklocks:mu endpoints map[tcpip.NICID]*multiPortEndpoint } @@ -346,7 +345,7 @@ type multiPortEndpoint struct { flags ports.FlagCounter - mu sync.RWMutex `state:"nosave"` + mu multiPortEndpointRWMutex `state:"nosave"` // endpoints stores the transport endpoints in the order in which they // were bound. This is required for UDP SO_REUSEADDR. //