Add a method for inspecting assigned addresses.

This new method allows checking for the existence of assigned addresses without
taking an extra reference that needs to be DecRefed. DecRef takes exclusive
locks. Contention on the addressState lock causes performance issues when
multiple goroutines are processing IP packets simultaneously. This isn't the
case today since IP processing is single threaded, but will be eventually.

PiperOrigin-RevId: 623567408
This commit is contained in:
Lucas Manning
2024-04-10 12:12:23 -07:00
committed by gVisor bot
parent 87d8df37c7
commit 52fc5b60f7
10 changed files with 46 additions and 44 deletions
+1 -2
View File
@@ -232,8 +232,7 @@ func (e *endpoint) checkLocalAddress(addr tcpip.Address) bool {
return true
}
if addressEndpoint := e.AcquireAssignedAddress(addr, false, stack.NeverPrimaryEndpoint); addressEndpoint != nil {
addressEndpoint.DecRef()
if addressEndpoint := e.AcquireAssignedAddress(addr, false, stack.NeverPrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
return true
}
return false
+7 -13
View File
@@ -193,8 +193,7 @@ func (p *protocol) findEndpointWithAddress(addr tcpip.Address) *endpoint {
defer p.mu.RUnlock()
for _, e := range p.eps {
if addressEndpoint := e.AcquireAssignedAddress(addr, false /* allowTemp */, stack.NeverPrimaryEndpoint); addressEndpoint != nil {
addressEndpoint.DecRef()
if addressEndpoint := e.AcquireAssignedAddress(addr, false /* allowTemp */, stack.NeverPrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
return e
}
}
@@ -846,10 +845,8 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) {
}
if e.protocol.stack.HandleLocal() {
addressEndpoint := e.AcquireAssignedAddress(header.IPv4(pkt.NetworkHeader().Slice()).SourceAddress(), e.nic.Promiscuous(), stack.CanBePrimaryEndpoint)
addressEndpoint := e.AcquireAssignedAddress(header.IPv4(pkt.NetworkHeader().Slice()).SourceAddress(), e.nic.Promiscuous(), stack.CanBePrimaryEndpoint, true /* readOnly */)
if addressEndpoint != nil {
addressEndpoint.DecRef()
// The source address is one of our own, so we never should have gotten
// a packet like this unless HandleLocal is false or our NIC is the
// loopback interface.
@@ -1118,9 +1115,8 @@ func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer,
return
}
// Make sure the source address is not a subnet-local broadcast address.
if addressEndpoint := e.AcquireAssignedAddress(srcAddr, false /* createTemp */, stack.NeverPrimaryEndpoint); addressEndpoint != nil {
if addressEndpoint := e.AcquireAssignedAddress(srcAddr, false /* createTemp */, stack.NeverPrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
subnet := addressEndpoint.Subnet()
addressEndpoint.DecRef()
if subnet.IsBroadcast(srcAddr) {
stats.ip.InvalidSourceAddressesReceived.Increment()
return
@@ -1157,9 +1153,8 @@ func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer,
//
// If the packet is destined for this device, then it should be delivered
// locally. Otherwise, if forwarding is enabled, it should be forwarded.
if addressEndpoint := e.AcquireAssignedAddress(dstAddr, e.nic.Promiscuous(), stack.CanBePrimaryEndpoint); addressEndpoint != nil {
if addressEndpoint := e.AcquireAssignedAddress(dstAddr, e.nic.Promiscuous(), stack.CanBePrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
subnet := addressEndpoint.AddressWithPrefix().Subnet()
addressEndpoint.DecRef()
pkt.NetworkPacketInfo.LocalAddressBroadcast = subnet.IsBroadcast(dstAddr) || dstAddr == header.IPv4Broadcast
e.deliverPacketLocally(h, pkt, inNICName)
} else if e.Forwarding() {
@@ -1409,7 +1404,7 @@ func (e *endpoint) MainAddress() tcpip.AddressWithPrefix {
}
// AcquireAssignedAddress implements stack.AddressableEndpoint.
func (e *endpoint) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB stack.PrimaryEndpointBehavior) stack.AddressEndpoint {
func (e *endpoint) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB stack.PrimaryEndpointBehavior, readOnly bool) stack.AddressEndpoint {
e.mu.RLock()
defer e.mu.RUnlock()
@@ -1419,7 +1414,7 @@ func (e *endpoint) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp boo
// IPv4 has a notion of a subnet broadcast address and considers the
// loopback interface bound to an address's whole subnet (on linux).
return subnet.IsBroadcast(localAddr) || (loopback && subnet.Contains(localAddr))
}, allowTemp, tempPEB)
}, allowTemp, tempPEB, readOnly)
}
// AcquireOutgoingPrimaryAddress implements stack.AddressableEndpoint.
@@ -1758,9 +1753,8 @@ func (p *protocol) isSubnetLocalBroadcastAddress(addr tcpip.Address) bool {
defer p.mu.RUnlock()
for _, e := range p.eps {
if addressEndpoint := e.AcquireAssignedAddress(addr, false /* createTemp */, stack.NeverPrimaryEndpoint); addressEndpoint != nil {
if addressEndpoint := e.AcquireAssignedAddress(addr, false /* createTemp */, stack.NeverPrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
subnet := addressEndpoint.Subnet()
addressEndpoint.DecRef()
if subnet.IsBroadcast(addr) {
return true
}
+1 -2
View File
@@ -153,8 +153,7 @@ func (e *endpoint) checkLocalAddress(addr tcpip.Address) bool {
return true
}
if addressEndpoint := e.AcquireAssignedAddress(addr, false, stack.NeverPrimaryEndpoint); addressEndpoint != nil {
addressEndpoint.DecRef()
if addressEndpoint := e.AcquireAssignedAddress(addr, false, stack.NeverPrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
return true
}
return false
+7 -11
View File
@@ -1105,10 +1105,8 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) {
}
if e.protocol.stack.HandleLocal() {
addressEndpoint := e.AcquireAssignedAddress(header.IPv6(pkt.NetworkHeader().Slice()).SourceAddress(), e.nic.Promiscuous(), stack.CanBePrimaryEndpoint)
addressEndpoint := e.AcquireAssignedAddress(header.IPv6(pkt.NetworkHeader().Slice()).SourceAddress(), e.nic.Promiscuous(), stack.CanBePrimaryEndpoint, true /* readOnly */)
if addressEndpoint != nil {
addressEndpoint.DecRef()
// The source address is one of our own, so we never should have gotten
// a packet like this unless HandleLocal is false or our NIC is the
// loopback interface.
@@ -1348,8 +1346,7 @@ func (e *endpoint) handleValidatedPacket(h header.IPv6, pkt *stack.PacketBuffer,
// The destination address should be an address we own for us to receive the
// packet. Otherwise, attempt to forward the packet.
if addressEndpoint := e.AcquireAssignedAddress(dstAddr, e.nic.Promiscuous(), stack.CanBePrimaryEndpoint); addressEndpoint != nil {
addressEndpoint.DecRef()
if addressEndpoint := e.AcquireAssignedAddress(dstAddr, e.nic.Promiscuous(), stack.CanBePrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
e.deliverPacketLocally(h, pkt, inNICName)
} else if e.Forwarding() {
e.handleForwardingError(e.forwardUnicastPacket(pkt))
@@ -2036,18 +2033,18 @@ func (e *endpoint) MainAddress() tcpip.AddressWithPrefix {
}
// AcquireAssignedAddress implements stack.AddressableEndpoint.
func (e *endpoint) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB stack.PrimaryEndpointBehavior) stack.AddressEndpoint {
func (e *endpoint) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB stack.PrimaryEndpointBehavior, readOnly bool) stack.AddressEndpoint {
e.mu.RLock()
defer e.mu.RUnlock()
return e.acquireAddressOrCreateTempLocked(localAddr, allowTemp, tempPEB)
return e.acquireAddressOrCreateTempLocked(localAddr, allowTemp, tempPEB, readOnly)
}
// acquireAddressOrCreateTempLocked is like AcquireAssignedAddress but with
// locking requirements.
//
// Precondition: e.mu must be write locked.
func (e *endpoint) acquireAddressOrCreateTempLocked(localAddr tcpip.Address, allowTemp bool, tempPEB stack.PrimaryEndpointBehavior) stack.AddressEndpoint {
return e.mu.addressableEndpointState.AcquireAssignedAddress(localAddr, allowTemp, tempPEB)
func (e *endpoint) acquireAddressOrCreateTempLocked(localAddr tcpip.Address, allowTemp bool, tempPEB stack.PrimaryEndpointBehavior, readOnly bool) stack.AddressEndpoint {
return e.mu.addressableEndpointState.AcquireAssignedAddress(localAddr, allowTemp, tempPEB, readOnly)
}
// AcquireOutgoingPrimaryAddress implements stack.AddressableEndpoint.
@@ -2369,8 +2366,7 @@ func (p *protocol) findEndpointWithAddress(addr tcpip.Address) *endpoint {
defer p.mu.RUnlock()
for _, e := range p.mu.eps {
if addressEndpoint := e.AcquireAssignedAddress(addr, false /* allowTemp */, stack.NeverPrimaryEndpoint); addressEndpoint != nil {
addressEndpoint.DecRef()
if addressEndpoint := e.AcquireAssignedAddress(addr, false /* allowTemp */, stack.NeverPrimaryEndpoint, true /* readOnly */); addressEndpoint != nil {
return e
}
}
+22 -5
View File
@@ -537,16 +537,20 @@ func (a *AddressableEndpointState) acquirePrimaryAddressRLocked(remoteAddr, srcH
// If there is no matching address, a temporary address will be returned if
// allowTemp is true.
//
// If readOnly is true, the address will be returned without an extra reference.
// In this case it is not safe to modify the endpoint, only read attributes like
// subnet.
//
// 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 {
func (a *AddressableEndpointState) AcquireAssignedAddressOrMatching(localAddr tcpip.Address, f func(AddressEndpoint) bool, allowTemp bool, tempPEB PrimaryEndpointBehavior, readOnly bool) AddressEndpoint {
lookup := func() *addressState {
if addrState, ok := a.endpoints[localAddr]; ok {
if !addrState.IsAssigned(allowTemp) {
return nil
}
if !addrState.TryIncRef() {
if !readOnly && !addrState.TryIncRef() {
panic(fmt.Sprintf("failed to increase the reference count for address = %s", addrState.addr))
}
@@ -555,7 +559,10 @@ func (a *AddressableEndpointState) AcquireAssignedAddressOrMatching(localAddr tc
if f != nil {
for _, addrState := range a.endpoints {
if addrState.IsAssigned(allowTemp) && f(addrState) && addrState.TryIncRef() {
if addrState.IsAssigned(allowTemp) && f(addrState) {
if !readOnly && !addrState.TryIncRef() {
continue
}
return addrState
}
}
@@ -614,12 +621,22 @@ func (a *AddressableEndpointState) AcquireAssignedAddressOrMatching(localAddr tc
if ep == nil {
return nil
}
if readOnly {
if ep.addressableEndpointState == a {
// Checklocks doesn't understand that we are logically guaranteed to have
// ep.mu locked already. We need to use checklocksignore to appease the
// analyzer.
ep.addressableEndpointState.decAddressRefLocked(ep) // +checklocksignore
} else {
ep.DecRef()
}
}
return ep
}
// AcquireAssignedAddress implements AddressableEndpoint.
func (a *AddressableEndpointState) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB PrimaryEndpointBehavior) AddressEndpoint {
return a.AcquireAssignedAddressOrMatching(localAddr, nil, allowTemp, tempPEB)
func (a *AddressableEndpointState) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB PrimaryEndpointBehavior, readOnly bool) AddressEndpoint {
return a.AcquireAssignedAddressOrMatching(localAddr, nil, allowTemp, tempPEB, readOnly)
}
// AcquireOutgoingPrimaryAddress implements AddressableEndpoint.
@@ -47,16 +47,14 @@ func TestAddressableEndpointStateCleanup(t *testing.T) {
ep.DecRef()
}
{
ep := s.AcquireAssignedAddress(addr.Address, false /* allowTemp */, stack.NeverPrimaryEndpoint)
ep := s.AcquireAssignedAddress(addr.Address, false /* allowTemp */, stack.NeverPrimaryEndpoint, true /* readOnly */)
if ep == nil {
t.Fatalf("got s.AcquireAssignedAddress(%s, false, NeverPrimaryEndpoint) = nil, want = non-nil", addr.Address)
}
ep.DecRef()
}
s.Cleanup()
if ep := s.AcquireAssignedAddress(addr.Address, false /* allowTemp */, stack.NeverPrimaryEndpoint); ep != nil {
ep.DecRef()
if ep := s.AcquireAssignedAddress(addr.Address, false /* allowTemp */, stack.NeverPrimaryEndpoint, true /* readOnly */); ep != nil {
t.Fatalf("got s.AcquireAssignedAddress(%s, false, NeverPrimaryEndpoint) = %s, want = nil", addr.Address, ep.AddressWithPrefix())
}
}
+1 -2
View File
@@ -88,9 +88,8 @@ func (f *fwdTestNetworkEndpoint) HandlePacket(pkt *PacketBuffer) {
netHdr := pkt.NetworkHeader().Slice()
_, dst := f.proto.ParseAddresses(netHdr)
addressEndpoint := f.AcquireAssignedAddress(dst, f.nic.Promiscuous(), CanBePrimaryEndpoint)
addressEndpoint := f.AcquireAssignedAddress(dst, f.nic.Promiscuous(), CanBePrimaryEndpoint, true /* readOnly */)
if addressEndpoint != nil {
addressEndpoint.DecRef()
// Dispatch the packet to the transport protocol.
f.dispatcher.DeliverTransportPacket(tcpip.TransportProtocolNumber(netHdr[protocolNumberOffset]), pkt)
return
+1 -1
View File
@@ -505,7 +505,7 @@ func (n *nic) getAddressOrCreateTempInner(protocol tcpip.NetworkProtocolNumber,
return nil
}
return addressableEndpoint.AcquireAssignedAddress(address, createTemp, peb)
return addressableEndpoint.AcquireAssignedAddress(address, createTemp, peb, false)
}
// addAddress adds a new address to n, so that it starts accepting packets
+3 -2
View File
@@ -668,10 +668,11 @@ type AddressableEndpoint interface {
// that is considered bound to the endpoint, optionally creating a temporary
// endpoint if requested and no existing address exists.
//
// The returned endpoint's reference count is incremented.
// The returned endpoint's reference count is incremented if readOnly is
// false.
//
// Returns nil if the specified address is not local to this endpoint.
AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB PrimaryEndpointBehavior) AddressEndpoint
AcquireAssignedAddress(localAddr tcpip.Address, allowTemp bool, tempPEB PrimaryEndpointBehavior, readOnly bool) AddressEndpoint
// AcquireOutgoingPrimaryAddress returns a primary address that may be used as
// a source address when sending packets to the passed remote address.
+1 -2
View File
@@ -130,11 +130,10 @@ func (f *fakeNetworkEndpoint) HandlePacket(pkt *stack.PacketBuffer) {
netHdr := pkt.NetworkHeader().Slice()
dst := tcpip.AddrFromSlice(netHdr[dstAddrOffset:][:header.IPv4AddressSize])
addressEndpoint := f.AcquireAssignedAddress(dst, f.nic.Promiscuous(), stack.CanBePrimaryEndpoint)
addressEndpoint := f.AcquireAssignedAddress(dst, f.nic.Promiscuous(), stack.CanBePrimaryEndpoint, true /* readOnly */)
if addressEndpoint == nil {
return
}
addressEndpoint.DecRef()
f.proto.packetCount[int(dst.AsSlice()[0])%len(f.proto.packetCount)]++