From fcd4ff4fca31e3c594b30aff6b008e7af4c7c1a5 Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Wed, 24 Feb 2021 12:30:20 -0800 Subject: Cleanup temp SLAAC address jobs on DAD conflicts Previously, when DAD would detect a conflict for a temporary address, the address would be removed but its timers would not be stopped, resulting in a panic when the removed address's invalidation timer fired. While I'm here, remove the check for unicast-ness on removed address endpoints since multicast addresses are no longer stored in the same structure as unicast addresses as of 27ee4fe76ad586ac8751951a842b3681f93. Test: stack_test.TestMixedSLAACAddrConflictRegen PiperOrigin-RevId: 359344849 --- pkg/tcpip/network/ipv6/ipv6.go | 34 ++++++++++++++++++---------------- pkg/tcpip/network/ipv6/ndp.go | 42 ++++++++++++++++++++---------------------- 2 files changed, 38 insertions(+), 38 deletions(-) (limited to 'pkg/tcpip/network') diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go index c5c3ef882..b16a2d322 100644 --- a/pkg/tcpip/network/ipv6/ipv6.go +++ b/pkg/tcpip/network/ipv6/ipv6.go @@ -1441,28 +1441,30 @@ func (e *endpoint) RemovePermanentAddress(addr tcpip.Address) tcpip.Error { // Precondition: e.mu must be write locked. func (e *endpoint) removePermanentEndpointLocked(addressEndpoint stack.AddressEndpoint, allowSLAACInvalidation, dadFailure bool) tcpip.Error { addr := addressEndpoint.AddressWithPrefix() - unicast := header.IsV6UnicastAddress(addr.Address) - if unicast { - e.mu.ndp.stopDuplicateAddressDetection(addr.Address, dadFailure) - - // If we are removing an address generated via SLAAC, cleanup - // its SLAAC resources and notify the integrator. - switch addressEndpoint.ConfigType() { - case stack.AddressConfigSlaac: - e.mu.ndp.cleanupSLAACAddrResourcesAndNotify(addr, allowSLAACInvalidation) - case stack.AddressConfigSlaacTemp: - e.mu.ndp.cleanupTempSLAACAddrResourcesAndNotify(addr, allowSLAACInvalidation) - } + // If we are removing an address generated via SLAAC, cleanup + // its SLAAC resources and notify the integrator. + switch addressEndpoint.ConfigType() { + case stack.AddressConfigSlaac: + e.mu.ndp.cleanupSLAACAddrResourcesAndNotify(addr, allowSLAACInvalidation) + case stack.AddressConfigSlaacTemp: + e.mu.ndp.cleanupTempSLAACAddrResourcesAndNotify(addr) } + return e.removePermanentEndpointInnerLocked(addressEndpoint, dadFailure) +} + +// removePermanentEndpointInnerLocked is like removePermanentEndpointLocked +// except it does not cleanup SLAAC address state. +// +// Precondition: e.mu must be write locked. +func (e *endpoint) removePermanentEndpointInnerLocked(addressEndpoint stack.AddressEndpoint, dadFailure bool) tcpip.Error { + addr := addressEndpoint.AddressWithPrefix() + e.mu.ndp.stopDuplicateAddressDetection(addr.Address, dadFailure) + if err := e.mu.addressableEndpointState.RemovePermanentEndpoint(addressEndpoint); err != nil { return err } - if !unicast { - return nil - } - snmc := header.SolicitedNodeAddr(addr.Address) err := e.leaveGroupLocked(snmc) // The endpoint may have already left the multicast group. diff --git a/pkg/tcpip/network/ipv6/ndp.go b/pkg/tcpip/network/ipv6/ndp.go index 53c043dcd..721269c58 100644 --- a/pkg/tcpip/network/ipv6/ndp.go +++ b/pkg/tcpip/network/ipv6/ndp.go @@ -1497,9 +1497,11 @@ func (ndp *ndpState) invalidateSLAACPrefix(prefix tcpip.Subnet, state slaacPrefi ndp.cleanupSLAACPrefixResources(prefix, state) if addressEndpoint := state.stableAddr.addressEndpoint; addressEndpoint != nil { - // Since we are already invalidating the prefix, do not invalidate the - // prefix when removing the address. - if err := ndp.ep.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, false /* dadFailure */); err != nil { + if ndpDisp := ndp.ep.protocol.options.NDPDisp; ndpDisp != nil { + ndpDisp.OnAutoGenAddressInvalidated(ndp.ep.nic.ID(), addressEndpoint.AddressWithPrefix()) + } + + if err := ndp.ep.removePermanentEndpointInnerLocked(addressEndpoint, false /* dadFailure */); err != nil { panic(fmt.Sprintf("ndp: error removing stable SLAAC address %s: %s", addressEndpoint.AddressWithPrefix(), err)) } } @@ -1556,28 +1558,19 @@ func (ndp *ndpState) cleanupSLAACPrefixResources(prefix tcpip.Subnet, state slaa // // The IPv6 endpoint that ndp belongs to MUST be locked. func (ndp *ndpState) invalidateTempSLAACAddr(tempAddrs map[tcpip.Address]tempSLAACAddrState, tempAddr tcpip.Address, tempAddrState tempSLAACAddrState) { - // Since we are already invalidating the address, do not invalidate the - // address when removing the address. - if err := ndp.ep.removePermanentEndpointLocked(tempAddrState.addressEndpoint, false /* allowSLAACInvalidation */, false /* dadFailure */); err != nil { + ndp.cleanupTempSLAACAddrResourcesAndNotifyInner(tempAddrs, tempAddr, tempAddrState) + + if err := ndp.ep.removePermanentEndpointInnerLocked(tempAddrState.addressEndpoint, false /* dadFailure */); err != nil { panic(fmt.Sprintf("error removing temporary SLAAC address %s: %s", tempAddrState.addressEndpoint.AddressWithPrefix(), err)) } - - ndp.cleanupTempSLAACAddrResources(tempAddrs, tempAddr, tempAddrState) } // cleanupTempSLAACAddrResourcesAndNotify cleans up an invalidated temporary -// SLAAC address's resources from ndp. +// SLAAC address's resources from ndp and notifies the NDP dispatcher that the +// address was invalidated. // // The IPv6 endpoint that ndp belongs to MUST be locked. -func (ndp *ndpState) cleanupTempSLAACAddrResourcesAndNotify(addr tcpip.AddressWithPrefix, invalidateAddr bool) { - if ndpDisp := ndp.ep.protocol.options.NDPDisp; ndpDisp != nil { - ndpDisp.OnAutoGenAddressInvalidated(ndp.ep.nic.ID(), addr) - } - - if !invalidateAddr { - return - } - +func (ndp *ndpState) cleanupTempSLAACAddrResourcesAndNotify(addr tcpip.AddressWithPrefix) { prefix := addr.Subnet() state, ok := ndp.slaacPrefixes[prefix] if !ok { @@ -1589,14 +1582,19 @@ func (ndp *ndpState) cleanupTempSLAACAddrResourcesAndNotify(addr tcpip.AddressWi panic(fmt.Sprintf("ndp: must have a tempAddr entry to clean up temp addr %s resources", addr)) } - ndp.cleanupTempSLAACAddrResources(state.tempAddrs, addr.Address, tempAddrState) + ndp.cleanupTempSLAACAddrResourcesAndNotifyInner(state.tempAddrs, addr.Address, tempAddrState) } -// cleanupTempSLAACAddrResourcesAndNotify cleans up a temporary SLAAC address's -// jobs and entry. +// cleanupTempSLAACAddrResourcesAndNotifyInner is like +// cleanupTempSLAACAddrResourcesAndNotify except it does not lookup the +// temporary address's state in ndp - it assumes the passed state is valid. // // The IPv6 endpoint that ndp belongs to MUST be locked. -func (ndp *ndpState) cleanupTempSLAACAddrResources(tempAddrs map[tcpip.Address]tempSLAACAddrState, tempAddr tcpip.Address, tempAddrState tempSLAACAddrState) { +func (ndp *ndpState) cleanupTempSLAACAddrResourcesAndNotifyInner(tempAddrs map[tcpip.Address]tempSLAACAddrState, tempAddr tcpip.Address, tempAddrState tempSLAACAddrState) { + if ndpDisp := ndp.ep.protocol.options.NDPDisp; ndpDisp != nil { + ndpDisp.OnAutoGenAddressInvalidated(ndp.ep.nic.ID(), tempAddrState.addressEndpoint.AddressWithPrefix()) + } + tempAddrState.addressEndpoint.DecRef() tempAddrState.addressEndpoint = nil tempAddrState.deprecationJob.Cancel() -- cgit v1.2.3