Change AcquireAssignedAddress to use RLock.

This is a hot path for all incoming packets and we don't
need an exclusive lock here as we are not modifying any
of the fields protected by mu here.

PiperOrigin-RevId: 373181254
This commit is contained in:
Bhasker Hariharan
2021-05-11 10:30:33 -07:00
committed by gVisor bot
parent 60bdf7ed31
commit ebebb3059f
3 changed files with 50 additions and 28 deletions
+6 -6
View File
@@ -990,8 +990,8 @@ func (e *endpoint) Close() {
// AddAndAcquirePermanentAddress implements stack.AddressableEndpoint.
func (e *endpoint) AddAndAcquirePermanentAddress(addr tcpip.AddressWithPrefix, peb stack.PrimaryEndpointBehavior, configType stack.AddressConfigType, deprecated bool) (stack.AddressEndpoint, tcpip.Error) {
e.mu.Lock()
defer e.mu.Unlock()
e.mu.RLock()
defer e.mu.RUnlock()
ep, err := e.mu.addressableEndpointState.AddAndAcquirePermanentAddress(addr, peb, configType, deprecated)
if err == nil {
@@ -1002,8 +1002,8 @@ func (e *endpoint) AddAndAcquirePermanentAddress(addr tcpip.AddressWithPrefix, p
// RemovePermanentAddress implements stack.AddressableEndpoint.
func (e *endpoint) RemovePermanentAddress(addr tcpip.Address) tcpip.Error {
e.mu.Lock()
defer e.mu.Unlock()
e.mu.RLock()
defer e.mu.RUnlock()
return e.mu.addressableEndpointState.RemovePermanentAddress(addr)
}
@@ -1016,8 +1016,8 @@ func (e *endpoint) MainAddress() tcpip.AddressWithPrefix {
// AcquireAssignedAddress implements stack.AddressableEndpoint.
func (e *endpoint) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB stack.PrimaryEndpointBehavior) stack.AddressEndpoint {
e.mu.Lock()
defer e.mu.Unlock()
e.mu.RLock()
defer e.mu.RUnlock()
loopback := e.nic.IsLoopback()
return e.mu.addressableEndpointState.AcquireAssignedAddressOrMatching(localAddr, func(addressEndpoint stack.AddressEndpoint) bool {
+6 -6
View File
@@ -1546,8 +1546,8 @@ func (e *endpoint) NetworkProtocolNumber() tcpip.NetworkProtocolNumber {
func (e *endpoint) AddAndAcquirePermanentAddress(addr tcpip.AddressWithPrefix, peb stack.PrimaryEndpointBehavior, configType stack.AddressConfigType, deprecated bool) (stack.AddressEndpoint, tcpip.Error) {
// TODO(b/169350103): add checks here after making sure we no longer receive
// an empty address.
e.mu.Lock()
defer e.mu.Unlock()
e.mu.RLock()
defer e.mu.RUnlock()
return e.addAndAcquirePermanentAddressLocked(addr, peb, configType, deprecated)
}
@@ -1588,8 +1588,8 @@ func (e *endpoint) addAndAcquirePermanentAddressLocked(addr tcpip.AddressWithPre
// RemovePermanentAddress implements stack.AddressableEndpoint.
func (e *endpoint) RemovePermanentAddress(addr tcpip.Address) tcpip.Error {
e.mu.Lock()
defer e.mu.Unlock()
e.mu.RLock()
defer e.mu.RUnlock()
addressEndpoint := e.getAddressRLocked(addr)
if addressEndpoint == nil || !addressEndpoint.GetKind().IsPermanent() {
@@ -1666,8 +1666,8 @@ func (e *endpoint) MainAddress() tcpip.AddressWithPrefix {
// AcquireAssignedAddress implements stack.AddressableEndpoint.
func (e *endpoint) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB stack.PrimaryEndpointBehavior) stack.AddressEndpoint {
e.mu.Lock()
defer e.mu.Unlock()
e.mu.RLock()
defer e.mu.RUnlock()
return e.acquireAddressOrCreateTempLocked(localAddr, allowTemp, tempPEB)
}
+38 -16
View File
@@ -440,33 +440,54 @@ func (a *AddressableEndpointState) acquirePrimaryAddressRLocked(isValid func(*ad
// Regardless how the address was obtained, it will be acquired before it is
// returned.
func (a *AddressableEndpointState) AcquireAssignedAddressOrMatching(localAddr tcpip.Address, f func(AddressEndpoint) bool, allowTemp bool, tempPEB PrimaryEndpointBehavior) AddressEndpoint {
a.mu.Lock()
defer a.mu.Unlock()
lookup := func() *addressState {
if addrState, ok := a.mu.endpoints[localAddr]; ok {
if !addrState.IsAssigned(allowTemp) {
return nil
}
if addrState, ok := a.mu.endpoints[localAddr]; ok {
if !addrState.IsAssigned(allowTemp) {
return nil
if !addrState.IncRef() {
panic(fmt.Sprintf("failed to increase the reference count for address = %s", addrState.addr))
}
return addrState
}
if !addrState.IncRef() {
panic(fmt.Sprintf("failed to increase the reference count for address = %s", addrState.addr))
}
return addrState
}
if f != nil {
for _, addrState := range a.mu.endpoints {
if addrState.IsAssigned(allowTemp) && f(addrState) && addrState.IncRef() {
return addrState
if f != nil {
for _, addrState := range a.mu.endpoints {
if addrState.IsAssigned(allowTemp) && f(addrState) && addrState.IncRef() {
return addrState
}
}
}
return nil
}
// Avoid exclusive lock on mu unless we need to add a new address.
a.mu.RLock()
ep := lookup()
a.mu.RUnlock()
if ep != nil {
return ep
}
if !allowTemp {
return nil
}
// Acquire state lock in exclusive mode as we need to add a new temporary
// endpoint.
a.mu.Lock()
defer a.mu.Unlock()
// Do the lookup again in case another goroutine added the address in the time
// we released and acquired the lock.
ep = lookup()
if ep != nil {
return ep
}
// Proceed to add a new temporary endpoint.
addr := localAddr.WithPrefix()
ep, err := a.addAndAcquireAddressLocked(addr, tempPEB, AddressConfigStatic, false /* deprecated */, false /* permanent */)
if err != nil {
@@ -475,6 +496,7 @@ func (a *AddressableEndpointState) AcquireAssignedAddressOrMatching(localAddr tc
// expect no error.
panic(fmt.Sprintf("a.addAndAcquireAddressLocked(%s, %d, %d, false, false): %s", addr, tempPEB, AddressConfigStatic, err))
}
// From https://golang.org/doc/faq#nil_error:
//
// Under the covers, interfaces are implemented as two elements, a type T and