From 3e8e2cad881978674737ee3f9ac58b780d172187 Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Thu, 4 Mar 2021 11:37:12 -0800 Subject: Make stack.DADResult an interface While I'm here, update NDPDispatcher.OnDuplicateAddressDetectionStatus to take a DADResult and rename it to OnDuplicateAddressDetectionResult. Fixes #5606. PiperOrigin-RevId: 360965416 --- pkg/tcpip/network/arp/arp.go | 2 +- .../internal/ip/duplicate_address_detection.go | 17 ++--- .../ip/duplicate_address_detection_test.go | 22 +++---- pkg/tcpip/network/ipv6/icmp.go | 2 +- pkg/tcpip/network/ipv6/ipv6.go | 47 ++++++-------- pkg/tcpip/network/ipv6/ndp.go | 56 ++++++++-------- pkg/tcpip/network/ipv6/ndp_test.go | 6 +- pkg/tcpip/stack/ndp_test.go | 64 +++++++++---------- pkg/tcpip/stack/registration.go | 40 +++++++++--- pkg/tcpip/stack/stack_test.go | 4 +- .../tests/integration/link_resolution_test.go | 74 +++++++++++----------- pkg/tcpip/tests/integration/loopback_test.go | 2 +- 12 files changed, 176 insertions(+), 160 deletions(-) diff --git a/pkg/tcpip/network/arp/arp.go b/pkg/tcpip/network/arp/arp.go index 3fcdea119..98ded525c 100644 --- a/pkg/tcpip/network/arp/arp.go +++ b/pkg/tcpip/network/arp/arp.go @@ -232,7 +232,7 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) { linkAddr := tcpip.LinkAddress(h.HardwareAddressSender()) e.mu.Lock() - e.mu.dad.StopLocked(addr, false /* aborted */) + e.mu.dad.StopLocked(addr, &stack.DADDupAddrDetected{}) e.mu.Unlock() // The solicited, override, and isRouter flags are not available for ARP; diff --git a/pkg/tcpip/network/internal/ip/duplicate_address_detection.go b/pkg/tcpip/network/internal/ip/duplicate_address_detection.go index 6f89a6a16..0053646ee 100644 --- a/pkg/tcpip/network/internal/ip/duplicate_address_detection.go +++ b/pkg/tcpip/network/internal/ip/duplicate_address_detection.go @@ -126,9 +126,12 @@ func (d *DAD) CheckDuplicateAddressLocked(addr tcpip.Address, h stack.DADComplet s.timer.Stop() delete(d.addresses, addr) - r := stack.DADResult{Resolved: dadDone, Err: err} + var res stack.DADResult = &stack.DADSucceeded{} + if err != nil { + res = &stack.DADError{Err: err} + } for _, h := range s.completionHandlers { - h(r) + h(res) } }), } @@ -142,7 +145,7 @@ func (d *DAD) CheckDuplicateAddressLocked(addr tcpip.Address, h stack.DADComplet // StopLocked stops a currently running DAD process. // // Precondition: d.protocolMU must be locked. -func (d *DAD) StopLocked(addr tcpip.Address, aborted bool) { +func (d *DAD) StopLocked(addr tcpip.Address, reason stack.DADResult) { s, ok := d.addresses[addr] if !ok { return @@ -152,14 +155,8 @@ func (d *DAD) StopLocked(addr tcpip.Address, aborted bool) { s.timer.Stop() delete(d.addresses, addr) - var err tcpip.Error - if aborted { - err = &tcpip.ErrAborted{} - } - - r := stack.DADResult{Resolved: false, Err: err} for _, h := range s.completionHandlers { - h(r) + h(reason) } } diff --git a/pkg/tcpip/network/internal/ip/duplicate_address_detection_test.go b/pkg/tcpip/network/internal/ip/duplicate_address_detection_test.go index 18c357b56..e00aa4678 100644 --- a/pkg/tcpip/network/internal/ip/duplicate_address_detection_test.go +++ b/pkg/tcpip/network/internal/ip/duplicate_address_detection_test.go @@ -78,10 +78,10 @@ func (m *mockDADProtocol) checkDuplicateAddress(addr tcpip.Address, h stack.DADC return m.mu.dad.CheckDuplicateAddressLocked(addr, h) } -func (m *mockDADProtocol) stop(addr tcpip.Address, aborted bool) { +func (m *mockDADProtocol) stop(addr tcpip.Address, reason stack.DADResult) { m.mu.Lock() defer m.mu.Unlock() - m.mu.dad.StopLocked(addr, aborted) + m.mu.dad.StopLocked(addr, reason) } func (m *mockDADProtocol) setConfigs(c stack.DADConfigurations) { @@ -175,7 +175,7 @@ func TestDADCheckDuplicateAddress(t *testing.T) { } clock.Advance(delta) for i := 0; i < 2; i++ { - if diff := cmp.Diff(dadResult{Addr: addr1, R: stack.DADResult{Resolved: true, Err: nil}}, <-ch); diff != "" { + if diff := cmp.Diff(dadResult{Addr: addr1, R: &stack.DADSucceeded{}}, <-ch); diff != "" { t.Errorf("(i=%d) dad result mismatch (-want +got):\n%s", i, diff) } } @@ -189,7 +189,7 @@ func TestDADCheckDuplicateAddress(t *testing.T) { default: } clock.Advance(delta) - if diff := cmp.Diff(dadResult{Addr: addr2, R: stack.DADResult{Resolved: true, Err: nil}}, <-ch); diff != "" { + if diff := cmp.Diff(dadResult{Addr: addr2, R: &stack.DADSucceeded{}}, <-ch); diff != "" { t.Errorf("dad result mismatch (-want +got):\n%s", diff) } @@ -202,7 +202,7 @@ func TestDADCheckDuplicateAddress(t *testing.T) { t.Errorf("dad check mismatch (-want +got):\n%s", diff) } clock.Advance(dadConfig2Duration) - if diff := cmp.Diff(dadResult{Addr: addr2, R: stack.DADResult{Resolved: true, Err: nil}}, <-ch); diff != "" { + if diff := cmp.Diff(dadResult{Addr: addr2, R: &stack.DADSucceeded{}}, <-ch); diff != "" { t.Errorf("dad result mismatch (-want +got):\n%s", diff) } @@ -241,19 +241,19 @@ func TestDADStop(t *testing.T) { t.Errorf("dad check mismatch (-want +got):\n%s", diff) } - dad.stop(addr1, true /* aborted */) - if diff := cmp.Diff(dadResult{Addr: addr1, R: stack.DADResult{Resolved: false, Err: &tcpip.ErrAborted{}}}, <-ch); diff != "" { + dad.stop(addr1, &stack.DADAborted{}) + if diff := cmp.Diff(dadResult{Addr: addr1, R: &stack.DADAborted{}}, <-ch); diff != "" { t.Errorf("dad result mismatch (-want +got):\n%s", diff) } - dad.stop(addr2, false /* aborted */) - if diff := cmp.Diff(dadResult{Addr: addr2, R: stack.DADResult{Resolved: false, Err: nil}}, <-ch); diff != "" { + dad.stop(addr2, &stack.DADDupAddrDetected{}) + if diff := cmp.Diff(dadResult{Addr: addr2, R: &stack.DADDupAddrDetected{}}, <-ch); diff != "" { t.Errorf("dad result mismatch (-want +got):\n%s", diff) } dadResolutionDuration := time.Duration(dadConfigs.DupAddrDetectTransmits) * dadConfigs.RetransmitTimer clock.Advance(dadResolutionDuration) - if diff := cmp.Diff(dadResult{Addr: addr3, R: stack.DADResult{Resolved: true, Err: nil}}, <-ch); diff != "" { + if diff := cmp.Diff(dadResult{Addr: addr3, R: &stack.DADSucceeded{}}, <-ch); diff != "" { t.Errorf("dad result mismatch (-want +got):\n%s", diff) } @@ -266,7 +266,7 @@ func TestDADStop(t *testing.T) { t.Errorf("dad check mismatch (-want +got):\n%s", diff) } clock.Advance(dadResolutionDuration) - if diff := cmp.Diff(dadResult{Addr: addr1, R: stack.DADResult{Resolved: true, Err: nil}}, <-ch); diff != "" { + if diff := cmp.Diff(dadResult{Addr: addr1, R: &stack.DADSucceeded{}}, <-ch); diff != "" { t.Errorf("dad result mismatch (-want +got):\n%s", diff) } diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go index e80e681da..1cebe75ec 100644 --- a/pkg/tcpip/network/ipv6/icmp.go +++ b/pkg/tcpip/network/ipv6/icmp.go @@ -564,7 +564,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r targetAddr := na.TargetAddress() e.dad.mu.Lock() - e.dad.mu.dad.StopLocked(targetAddr, false /* aborted */) + e.dad.mu.dad.StopLocked(targetAddr, &stack.DADDupAddrDetected{}) e.dad.mu.Unlock() if e.hasTentativeAddr(targetAddr) { diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go index 544717678..d9ab240b6 100644 --- a/pkg/tcpip/network/ipv6/ipv6.go +++ b/pkg/tcpip/network/ipv6/ipv6.go @@ -363,7 +363,7 @@ func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address) tcpip.Error { // If the address is a SLAAC address, do not invalidate its SLAAC prefix as an // attempt will be made to generate a new address for it. - if err := e.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, true /* dadFailure */); err != nil { + if err := e.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, &stack.DADDupAddrDetected{}); err != nil { return err } @@ -536,8 +536,20 @@ func (e *endpoint) disableLocked() { } e.mu.ndp.stopSolicitingRouters() + // Stop DAD for all the tentative unicast addresses. + e.mu.addressableEndpointState.ForEachEndpoint(func(addressEndpoint stack.AddressEndpoint) bool { + if addressEndpoint.GetKind() != stack.PermanentTentative { + return true + } + + addr := addressEndpoint.AddressWithPrefix().Address + if header.IsV6UnicastAddress(addr) { + e.mu.ndp.stopDuplicateAddressDetection(addr, &stack.DADAborted{}) + } + + return true + }) e.mu.ndp.cleanupState(false /* hostOnly */) - e.stopDADForPermanentAddressesLocked() // The endpoint may have already left the multicast group. switch err := e.leaveGroupLocked(header.IPv6AllNodesMulticastAddress); err.(type) { @@ -555,25 +567,6 @@ func (e *endpoint) disableLocked() { } } -// stopDADForPermanentAddressesLocked stops DAD for all permaneent addresses. -// -// Precondition: e.mu must be write locked. -func (e *endpoint) stopDADForPermanentAddressesLocked() { - // Stop DAD for all the tentative unicast addresses. - e.mu.addressableEndpointState.ForEachEndpoint(func(addressEndpoint stack.AddressEndpoint) bool { - if addressEndpoint.GetKind() != stack.PermanentTentative { - return true - } - - addr := addressEndpoint.AddressWithPrefix().Address - if header.IsV6UnicastAddress(addr) { - e.mu.ndp.stopDuplicateAddressDetection(addr, false /* failed */) - } - - return true - }) -} - // DefaultTTL is the default hop limit for this endpoint. func (e *endpoint) DefaultTTL() uint8 { return e.protocol.DefaultTTL() @@ -1384,8 +1377,6 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { func (e *endpoint) Close() { e.mu.Lock() e.disableLocked() - e.mu.ndp.removeSLAACAddresses(false /* keepLinkLocal */) - e.stopDADForPermanentAddressesLocked() e.mu.addressableEndpointState.Cleanup() e.mu.Unlock() @@ -1451,14 +1442,14 @@ func (e *endpoint) RemovePermanentAddress(addr tcpip.Address) tcpip.Error { return &tcpip.ErrBadLocalAddress{} } - return e.removePermanentEndpointLocked(addressEndpoint, true /* allowSLAACInvalidation */, false /* dadFailure */) + return e.removePermanentEndpointLocked(addressEndpoint, true /* allowSLAACInvalidation */, &stack.DADAborted{}) } // removePermanentEndpointLocked is like removePermanentAddressLocked except // it works with a stack.AddressEndpoint. // // Precondition: e.mu must be write locked. -func (e *endpoint) removePermanentEndpointLocked(addressEndpoint stack.AddressEndpoint, allowSLAACInvalidation, dadFailure bool) tcpip.Error { +func (e *endpoint) removePermanentEndpointLocked(addressEndpoint stack.AddressEndpoint, allowSLAACInvalidation bool, dadResult stack.DADResult) tcpip.Error { addr := addressEndpoint.AddressWithPrefix() // If we are removing an address generated via SLAAC, cleanup // its SLAAC resources and notify the integrator. @@ -1469,16 +1460,16 @@ func (e *endpoint) removePermanentEndpointLocked(addressEndpoint stack.AddressEn e.mu.ndp.cleanupTempSLAACAddrResourcesAndNotify(addr) } - return e.removePermanentEndpointInnerLocked(addressEndpoint, dadFailure) + return e.removePermanentEndpointInnerLocked(addressEndpoint, dadResult) } // 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 { +func (e *endpoint) removePermanentEndpointInnerLocked(addressEndpoint stack.AddressEndpoint, dadResult stack.DADResult) tcpip.Error { addr := addressEndpoint.AddressWithPrefix() - e.mu.ndp.stopDuplicateAddressDetection(addr.Address, dadFailure) + e.mu.ndp.stopDuplicateAddressDetection(addr.Address, dadResult) if err := e.mu.addressableEndpointState.RemovePermanentEndpoint(addressEndpoint); err != nil { return err diff --git a/pkg/tcpip/network/ipv6/ndp.go b/pkg/tcpip/network/ipv6/ndp.go index c22f60709..d9b728878 100644 --- a/pkg/tcpip/network/ipv6/ndp.go +++ b/pkg/tcpip/network/ipv6/ndp.go @@ -208,16 +208,12 @@ const ( // NDPDispatcher is the interface integrators of netstack must implement to // receive and handle NDP related events. type NDPDispatcher interface { - // OnDuplicateAddressDetectionStatus is called when the DAD process for an - // address (addr) on a NIC (with ID nicID) completes. resolved is set to true - // if DAD completed successfully (no duplicate addr detected); false otherwise - // (addr was detected to be a duplicate on the link the NIC is a part of, or - // it was stopped for some other reason, such as the address being removed). - // If an error occured during DAD, err is set and resolved must be ignored. + // OnDuplicateAddressDetectionResult is called when the DAD process for an + // address on a NIC completes. // // This function is not permitted to block indefinitely. This function // is also not permitted to call into the stack. - OnDuplicateAddressDetectionStatus(nicID tcpip.NICID, addr tcpip.Address, resolved bool, err tcpip.Error) + OnDuplicateAddressDetectionResult(tcpip.NICID, tcpip.Address, stack.DADResult) // OnDefaultRouterDiscovered is called when a new default router is // discovered. Implementations must return true if the newly discovered @@ -225,14 +221,14 @@ type NDPDispatcher interface { // // This function is not permitted to block indefinitely. This function // is also not permitted to call into the stack. - OnDefaultRouterDiscovered(nicID tcpip.NICID, addr tcpip.Address) bool + OnDefaultRouterDiscovered(tcpip.NICID, tcpip.Address) bool // OnDefaultRouterInvalidated is called when a discovered default router that // was remembered is invalidated. // // This function is not permitted to block indefinitely. This function // is also not permitted to call into the stack. - OnDefaultRouterInvalidated(nicID tcpip.NICID, addr tcpip.Address) + OnDefaultRouterInvalidated(tcpip.NICID, tcpip.Address) // OnOnLinkPrefixDiscovered is called when a new on-link prefix is discovered. // Implementations must return true if the newly discovered on-link prefix @@ -240,14 +236,14 @@ type NDPDispatcher interface { // // This function is not permitted to block indefinitely. This function // is also not permitted to call into the stack. - OnOnLinkPrefixDiscovered(nicID tcpip.NICID, prefix tcpip.Subnet) bool + OnOnLinkPrefixDiscovered(tcpip.NICID, tcpip.Subnet) bool // OnOnLinkPrefixInvalidated is called when a discovered on-link prefix that // was remembered is invalidated. // // This function is not permitted to block indefinitely. This function // is also not permitted to call into the stack. - OnOnLinkPrefixInvalidated(nicID tcpip.NICID, prefix tcpip.Subnet) + OnOnLinkPrefixInvalidated(tcpip.NICID, tcpip.Subnet) // OnAutoGenAddress is called when a new prefix with its autonomous address- // configuration flag set is received and SLAAC was performed. Implementations @@ -280,12 +276,12 @@ type NDPDispatcher interface { // It is up to the caller to use the DNS Servers only for their valid // lifetime. OnRecursiveDNSServerOption may be called for new or // already known DNS servers. If called with known DNS servers, their - // valid lifetimes must be refreshed to lifetime (it may be increased, - // decreased, or completely invalidated when lifetime = 0). + // valid lifetimes must be refreshed to the lifetime (it may be increased, + // decreased, or completely invalidated when the lifetime = 0). // // This function is not permitted to block indefinitely. It must not // call functions on the stack itself. - OnRecursiveDNSServerOption(nicID tcpip.NICID, addrs []tcpip.Address, lifetime time.Duration) + OnRecursiveDNSServerOption(tcpip.NICID, []tcpip.Address, time.Duration) // OnDNSSearchListOption is called when the stack learns of DNS search lists // through NDP. @@ -293,9 +289,9 @@ type NDPDispatcher interface { // It is up to the caller to use the domain names in the search list // for only their valid lifetime. OnDNSSearchListOption may be called // with new or already known domain names. If called with known domain - // names, their valid lifetimes must be refreshed to lifetime (it may - // be increased, decreased or completely invalidated when lifetime = 0. - OnDNSSearchListOption(nicID tcpip.NICID, domainNames []string, lifetime time.Duration) + // names, their valid lifetimes must be refreshed to the lifetime (it may + // be increased, decreased or completely invalidated when the lifetime = 0. + OnDNSSearchListOption(tcpip.NICID, []string, time.Duration) // OnDHCPv6Configuration is called with an updated configuration that is // available via DHCPv6 for the passed NIC. @@ -587,15 +583,25 @@ func (ndp *ndpState) startDuplicateAddressDetection(addr tcpip.Address, addressE panic(fmt.Sprintf("ndpdad: addr %s is no longer tentative on NIC(%d)", addr, ndp.ep.nic.ID())) } - if r.Resolved { + var dadSucceeded bool + switch r.(type) { + case *stack.DADAborted, *stack.DADError, *stack.DADDupAddrDetected: + dadSucceeded = false + case *stack.DADSucceeded: + dadSucceeded = true + default: + panic(fmt.Sprintf("unrecognized DAD result = %T", r)) + } + + if dadSucceeded { addressEndpoint.SetKind(stack.Permanent) } if ndpDisp := ndp.ep.protocol.options.NDPDisp; ndpDisp != nil { - ndpDisp.OnDuplicateAddressDetectionStatus(ndp.ep.nic.ID(), addr, r.Resolved, r.Err) + ndpDisp.OnDuplicateAddressDetectionResult(ndp.ep.nic.ID(), addr, r) } - if r.Resolved { + if dadSucceeded { if addressEndpoint.ConfigType() == stack.AddressConfigSlaac { // Reset the generation attempts counter as we are starting the // generation of a new address for the SLAAC prefix. @@ -616,7 +622,7 @@ func (ndp *ndpState) startDuplicateAddressDetection(addr tcpip.Address, addressE // Consider DAD to have resolved even if no DAD messages were actually // transmitted. if ndpDisp := ndp.ep.protocol.options.NDPDisp; ndpDisp != nil { - ndpDisp.OnDuplicateAddressDetectionStatus(ndp.ep.nic.ID(), addr, true, nil) + ndpDisp.OnDuplicateAddressDetectionResult(ndp.ep.nic.ID(), addr, &stack.DADSucceeded{}) } ndp.ep.onAddressAssignedLocked(addr) @@ -633,8 +639,8 @@ func (ndp *ndpState) startDuplicateAddressDetection(addr tcpip.Address, addressE // of this function to handle such a scenario. // // The IPv6 endpoint that ndp belongs to MUST be locked. -func (ndp *ndpState) stopDuplicateAddressDetection(addr tcpip.Address, failed bool) { - ndp.dad.StopLocked(addr, !failed) +func (ndp *ndpState) stopDuplicateAddressDetection(addr tcpip.Address, reason stack.DADResult) { + ndp.dad.StopLocked(addr, reason) } // handleRA handles a Router Advertisement message that arrived on the NIC @@ -1501,7 +1507,7 @@ func (ndp *ndpState) invalidateSLAACPrefix(prefix tcpip.Subnet, state slaacPrefi ndpDisp.OnAutoGenAddressInvalidated(ndp.ep.nic.ID(), addressEndpoint.AddressWithPrefix()) } - if err := ndp.ep.removePermanentEndpointInnerLocked(addressEndpoint, false /* dadFailure */); err != nil { + if err := ndp.ep.removePermanentEndpointInnerLocked(addressEndpoint, &stack.DADAborted{}); err != nil { panic(fmt.Sprintf("ndp: error removing stable SLAAC address %s: %s", addressEndpoint.AddressWithPrefix(), err)) } } @@ -1560,7 +1566,7 @@ func (ndp *ndpState) cleanupSLAACPrefixResources(prefix tcpip.Subnet, state slaa func (ndp *ndpState) invalidateTempSLAACAddr(tempAddrs map[tcpip.Address]tempSLAACAddrState, tempAddr tcpip.Address, tempAddrState tempSLAACAddrState) { ndp.cleanupTempSLAACAddrResourcesAndNotifyInner(tempAddrs, tempAddr, tempAddrState) - if err := ndp.ep.removePermanentEndpointInnerLocked(tempAddrState.addressEndpoint, false /* dadFailure */); err != nil { + if err := ndp.ep.removePermanentEndpointInnerLocked(tempAddrState.addressEndpoint, &stack.DADAborted{}); err != nil { panic(fmt.Sprintf("error removing temporary SLAAC address %s: %s", tempAddrState.addressEndpoint.AddressWithPrefix(), err)) } } diff --git a/pkg/tcpip/network/ipv6/ndp_test.go b/pkg/tcpip/network/ipv6/ndp_test.go index 0d53c260d..6e850fd46 100644 --- a/pkg/tcpip/network/ipv6/ndp_test.go +++ b/pkg/tcpip/network/ipv6/ndp_test.go @@ -90,7 +90,7 @@ type testNDPDispatcher struct { addr tcpip.Address } -func (*testNDPDispatcher) OnDuplicateAddressDetectionStatus(tcpip.NICID, tcpip.Address, bool, tcpip.Error) { +func (*testNDPDispatcher) OnDuplicateAddressDetectionResult(tcpip.NICID, tcpip.Address, stack.DADResult) { } func (t *testNDPDispatcher) OnDefaultRouterDiscovered(_ tcpip.NICID, addr tcpip.Address) bool { @@ -1314,10 +1314,10 @@ func TestCheckDuplicateAddress(t *testing.T) { t.Fatalf("got s.CheckDuplicateAddress(%d, %d, %s, _) = %d, want = %d", nicID, ProtocolNumber, lladdr0, res, stack.DADAlreadyRunning) } - // Wait for DAD to resolve. + // Wait for DAD to complete. clock.Advance(time.Duration(dadConfigs.DupAddrDetectTransmits) * dadConfigs.RetransmitTimer) for i := 0; i < dadRequestsMade; i++ { - if diff := cmp.Diff(stack.DADResult{Resolved: true}, <-ch); diff != "" { + if diff := cmp.Diff(&stack.DADSucceeded{}, <-ch); diff != "" { t.Errorf("(i=%d) DAD result mismatch (-want +got):\n%s", i, diff) } } diff --git a/pkg/tcpip/stack/ndp_test.go b/pkg/tcpip/stack/ndp_test.go index 78a4cb072..e7669de69 100644 --- a/pkg/tcpip/stack/ndp_test.go +++ b/pkg/tcpip/stack/ndp_test.go @@ -99,12 +99,11 @@ func prefixSubnetAddr(offset uint8, linkAddr tcpip.LinkAddress) (tcpip.AddressWi } // ndpDADEvent is a set of parameters that was passed to -// ndpDispatcher.OnDuplicateAddressDetectionStatus. +// ndpDispatcher.OnDuplicateAddressDetectionResult. type ndpDADEvent struct { - nicID tcpip.NICID - addr tcpip.Address - resolved bool - err tcpip.Error + nicID tcpip.NICID + addr tcpip.Address + res stack.DADResult } type ndpRouterEvent struct { @@ -173,14 +172,13 @@ type ndpDispatcher struct { dhcpv6ConfigurationC chan ndpDHCPv6Event } -// Implements ipv6.NDPDispatcher.OnDuplicateAddressDetectionStatus. -func (n *ndpDispatcher) OnDuplicateAddressDetectionStatus(nicID tcpip.NICID, addr tcpip.Address, resolved bool, err tcpip.Error) { +// Implements ipv6.NDPDispatcher.OnDuplicateAddressDetectionResult. +func (n *ndpDispatcher) OnDuplicateAddressDetectionResult(nicID tcpip.NICID, addr tcpip.Address, res stack.DADResult) { if n.dadC != nil { n.dadC <- ndpDADEvent{ nicID, addr, - resolved, - err, + res, } } } @@ -311,8 +309,8 @@ func (l *channelLinkWithHeaderLength) MaxHeaderLength() uint16 { // Check e to make sure that the event is for addr on nic with ID 1, and the // resolved flag set to resolved with the specified err. -func checkDADEvent(e ndpDADEvent, nicID tcpip.NICID, addr tcpip.Address, resolved bool, err tcpip.Error) string { - return cmp.Diff(ndpDADEvent{nicID: nicID, addr: addr, resolved: resolved, err: err}, e, cmp.AllowUnexported(e)) +func checkDADEvent(e ndpDADEvent, nicID tcpip.NICID, addr tcpip.Address, res stack.DADResult) string { + return cmp.Diff(ndpDADEvent{nicID: nicID, addr: addr, res: res}, e, cmp.AllowUnexported(e)) } // TestDADDisabled tests that an address successfully resolves immediately @@ -344,7 +342,7 @@ func TestDADDisabled(t *testing.T) { // DAD on it. select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr1, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr1, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } default: @@ -491,7 +489,7 @@ func TestDADResolve(t *testing.T) { case <-time.After(defaultAsyncPositiveEventTimeout): t.Fatal("timed out waiting for DAD resolution") case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr1, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr1, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } } @@ -691,7 +689,7 @@ func TestDADFail(t *testing.T) { // something is wrong. t.Fatal("timed out waiting for DAD failure") case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr1, false, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr1, &stack.DADDupAddrDetected{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } } @@ -790,7 +788,7 @@ func TestDADStop(t *testing.T) { // time + extra 1s buffer, something is wrong. t.Fatal("timed out waiting for DAD failure") case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr1, false, &tcpip.ErrAborted{}); diff != "" { + if diff := checkDADEvent(e, nicID, addr1, &stack.DADAborted{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } } @@ -852,7 +850,7 @@ func TestSetNDPConfigurations(t *testing.T) { expectDADEvent := func(nicID tcpip.NICID, addr tcpip.Address) { select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } default: @@ -944,7 +942,7 @@ func TestSetNDPConfigurations(t *testing.T) { // means something is wrong. t.Fatal("timed out waiting for DAD resolution") case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID1, addr1, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID1, addr1, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } } @@ -1963,7 +1961,7 @@ func TestAutoGenTempAddr(t *testing.T) { select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } case <-time.After(time.Duration(test.dupAddrTransmits)*test.retransmitTimer + defaultAsyncPositiveEventTimeout): @@ -2169,7 +2167,7 @@ func TestNoAutoGenTempAddrForLinkLocal(t *testing.T) { } select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, llAddr1, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, llAddr1, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } case <-time.After(time.Duration(test.dupAddrTransmits)*test.retransmitTimer + defaultAsyncPositiveEventTimeout): @@ -2257,7 +2255,7 @@ func TestNoAutoGenTempAddrWithoutStableAddr(t *testing.T) { // address to be generated. select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr.Address, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } case <-time.After(dadTransmits*retransmitTimer + defaultAsyncPositiveEventTimeout): @@ -2723,7 +2721,7 @@ func TestMixedSLAACAddrConflictRegen(t *testing.T) { t.Helper() clock.Advance(dupAddrTransmits * retransmitTimer) - if diff := checkDADEvent(<-ndpDisp.dadC, nicID, addr, true, nil); diff != "" { + if diff := checkDADEvent(<-ndpDisp.dadC, nicID, addr, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } } @@ -2754,7 +2752,7 @@ func TestMixedSLAACAddrConflictRegen(t *testing.T) { rxNDPSolicit(e, addr.Address) select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr.Address, false, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADDupAddrDetected{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } default: @@ -3853,12 +3851,12 @@ func TestAutoGenAddrInResponseToDADConflicts(t *testing.T) { } } - expectDADEvent := func(t *testing.T, ndpDisp *ndpDispatcher, addr tcpip.Address, resolved bool, err tcpip.Error) { + expectDADEvent := func(t *testing.T, ndpDisp *ndpDispatcher, addr tcpip.Address, res stack.DADResult) { t.Helper() select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr, resolved, err); diff != "" { + if diff := checkDADEvent(e, nicID, addr, res); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } default: @@ -3866,12 +3864,12 @@ func TestAutoGenAddrInResponseToDADConflicts(t *testing.T) { } } - expectDADEventAsync := func(t *testing.T, ndpDisp *ndpDispatcher, addr tcpip.Address, resolved bool) { + expectDADEventAsync := func(t *testing.T, ndpDisp *ndpDispatcher, addr tcpip.Address, res stack.DADResult) { t.Helper() select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr, resolved, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr, res); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } case <-time.After(dadTransmits*retransmitTimer + defaultAsyncPositiveEventTimeout): @@ -3929,7 +3927,7 @@ func TestAutoGenAddrInResponseToDADConflicts(t *testing.T) { // generated. e.InjectInbound(header.IPv6ProtocolNumber, raBufWithPI(llAddr2, 0, prefix, true, true, 100, 100)) expectAutoGenAddrEvent(t, ndpDisp, stableAddrForTempAddrTest, newAddr) - expectDADEventAsync(t, ndpDisp, stableAddrForTempAddrTest.Address, true) + expectDADEventAsync(t, ndpDisp, stableAddrForTempAddrTest.Address, &stack.DADSucceeded{}) // The stable address will be assigned throughout the test. return []tcpip.AddressWithPrefix{stableAddrForTempAddrTest} @@ -4004,7 +4002,7 @@ func TestAutoGenAddrInResponseToDADConflicts(t *testing.T) { // Simulate a DAD conflict. rxNDPSolicit(e, addr.Address) expectAutoGenAddrEvent(t, &ndpDisp, addr, invalidatedAddr) - expectDADEvent(t, &ndpDisp, addr.Address, false, nil) + expectDADEvent(t, &ndpDisp, addr.Address, &stack.DADDupAddrDetected{}) // Attempting to add the address manually should not fail if the // address's state was cleaned up when DAD failed. @@ -4014,7 +4012,7 @@ func TestAutoGenAddrInResponseToDADConflicts(t *testing.T) { if err := s.RemoveAddress(nicID, addr.Address); err != nil { t.Fatalf("RemoveAddress(%d, %s) = %s", nicID, addr.Address, err) } - expectDADEvent(t, &ndpDisp, addr.Address, false, &tcpip.ErrAborted{}) + expectDADEvent(t, &ndpDisp, addr.Address, &stack.DADAborted{}) } // Should not have any new addresses assigned to the NIC. @@ -4027,7 +4025,7 @@ func TestAutoGenAddrInResponseToDADConflicts(t *testing.T) { if maxRetries+1 > numFailures { addr := addrType.addrGenFn(numFailures, tempIIDHistory[:]) expectAutoGenAddrEventAsync(t, &ndpDisp, addr, newAddr) - expectDADEventAsync(t, &ndpDisp, addr.Address, true) + expectDADEventAsync(t, &ndpDisp, addr.Address, &stack.DADSucceeded{}) if mismatch := addressCheck(s.NICInfo()[nicID].ProtocolAddresses, append(stableAddrs, addr), nil); mismatch != "" { t.Fatal(mismatch) } @@ -4144,7 +4142,7 @@ func TestAutoGenAddrWithEUI64IIDNoDADRetries(t *testing.T) { expectAutoGenAddrEvent(addr, invalidatedAddr) select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr.Address, false, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADDupAddrDetected{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } default: @@ -4243,7 +4241,7 @@ func TestAutoGenAddrContinuesLifetimesAfterRetry(t *testing.T) { expectAutoGenAddrEvent(addr, invalidatedAddr) select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr.Address, false, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADDupAddrDetected{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } default: @@ -4255,7 +4253,7 @@ func TestAutoGenAddrContinuesLifetimesAfterRetry(t *testing.T) { expectAutoGenAddrEvent(addr, newAddr) select { case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr.Address, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } case <-time.After(dadTransmits*retransmitTimer + defaultAsyncPositiveEventTimeout): diff --git a/pkg/tcpip/stack/registration.go b/pkg/tcpip/stack/registration.go index 43e9e4beb..426342fd2 100644 --- a/pkg/tcpip/stack/registration.go +++ b/pkg/tcpip/stack/registration.go @@ -852,18 +852,42 @@ type InjectableLinkEndpoint interface { InjectOutbound(dest tcpip.Address, packet []byte) tcpip.Error } -// DADResult is the result of a duplicate address detection process. -type DADResult struct { - // Resolved is true when DAD completed without detecting a duplicate address - // on the link. - // - // Ignored when Err is non-nil. - Resolved bool +// DADResult is a marker interface for the result of a duplicate address +// detection process. +type DADResult interface { + isDADResult() +} + +var _ DADResult = (*DADSucceeded)(nil) + +// DADSucceeded indicates DAD completed without finding any duplicate addresses. +type DADSucceeded struct{} - // Err is an error encountered while performing DAD. +func (*DADSucceeded) isDADResult() {} + +var _ DADResult = (*DADError)(nil) + +// DADError indicates DAD hit an error. +type DADError struct { Err tcpip.Error } +func (*DADError) isDADResult() {} + +var _ DADResult = (*DADAborted)(nil) + +// DADAborted indicates DAD was aborted. +type DADAborted struct{} + +func (*DADAborted) isDADResult() {} + +var _ DADResult = (*DADDupAddrDetected)(nil) + +// DADDupAddrDetected indicates DAD detected a duplicate address. +type DADDupAddrDetected struct{} + +func (*DADDupAddrDetected) isDADResult() {} + // DADCompletionHandler is a handler for DAD completion. type DADCompletionHandler func(DADResult) diff --git a/pkg/tcpip/stack/stack_test.go b/pkg/tcpip/stack/stack_test.go index f45cf5fdf..880219007 100644 --- a/pkg/tcpip/stack/stack_test.go +++ b/pkg/tcpip/stack/stack_test.go @@ -2605,7 +2605,7 @@ func TestNICAutoGenAddrDoesDAD(t *testing.T) { // means something is wrong. t.Fatal("timed out waiting for DAD resolution") case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, linkLocalAddr, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, linkLocalAddr, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } } @@ -3289,7 +3289,7 @@ func TestDoDADWhenNICEnabled(t *testing.T) { case <-time.After(dadTransmits*retransmitTimer + defaultAsyncPositiveEventTimeout): t.Fatal("timed out waiting for DAD resolution") case e := <-ndpDisp.dadC: - if diff := checkDADEvent(e, nicID, addr.AddressWithPrefix.Address, true, nil); diff != "" { + if diff := checkDADEvent(e, nicID, addr.AddressWithPrefix.Address, &stack.DADSucceeded{}); diff != "" { t.Errorf("dad event mismatch (-want +got):\n%s", diff) } } diff --git a/pkg/tcpip/tests/integration/link_resolution_test.go b/pkg/tcpip/tests/integration/link_resolution_test.go index 405c74c65..759cee4ef 100644 --- a/pkg/tcpip/tests/integration/link_resolution_test.go +++ b/pkg/tcpip/tests/integration/link_resolution_test.go @@ -1167,53 +1167,53 @@ func TestDAD(t *testing.T) { } tests := []struct { - name string - netProto tcpip.NetworkProtocolNumber - dadNetProto tcpip.NetworkProtocolNumber - remoteAddr tcpip.Address - expectedResolved bool + name string + netProto tcpip.NetworkProtocolNumber + dadNetProto tcpip.NetworkProtocolNumber + remoteAddr tcpip.Address + expectedResult stack.DADResult }{ { - name: "IPv4 own address", - netProto: ipv4.ProtocolNumber, - dadNetProto: arp.ProtocolNumber, - remoteAddr: utils.Ipv4Addr1.AddressWithPrefix.Address, - expectedResolved: true, + name: "IPv4 own address", + netProto: ipv4.ProtocolNumber, + dadNetProto: arp.ProtocolNumber, + remoteAddr: utils.Ipv4Addr1.AddressWithPrefix.Address, + expectedResult: &stack.DADSucceeded{}, }, { - name: "IPv6 own address", - netProto: ipv6.ProtocolNumber, - dadNetProto: ipv6.ProtocolNumber, - remoteAddr: utils.Ipv6Addr1.AddressWithPrefix.Address, - expectedResolved: true, + name: "IPv6 own address", + netProto: ipv6.ProtocolNumber, + dadNetProto: ipv6.ProtocolNumber, + remoteAddr: utils.Ipv6Addr1.AddressWithPrefix.Address, + expectedResult: &stack.DADSucceeded{}, }, { - name: "IPv4 duplicate address", - netProto: ipv4.ProtocolNumber, - dadNetProto: arp.ProtocolNumber, - remoteAddr: utils.Ipv4Addr2.AddressWithPrefix.Address, - expectedResolved: false, + name: "IPv4 duplicate address", + netProto: ipv4.ProtocolNumber, + dadNetProto: arp.ProtocolNumber, + remoteAddr: utils.Ipv4Addr2.AddressWithPrefix.Address, + expectedResult: &stack.DADDupAddrDetected{}, }, { - name: "IPv6 duplicate address", - netProto: ipv6.ProtocolNumber, - dadNetProto: ipv6.ProtocolNumber, - remoteAddr: utils.Ipv6Addr2.AddressWithPrefix.Address, - expectedResolved: false, + name: "IPv6 duplicate address", + netProto: ipv6.ProtocolNumber, + dadNetProto: ipv6.ProtocolNumber, + remoteAddr: utils.Ipv6Addr2.AddressWithPrefix.Address, + expectedResult: &stack.DADDupAddrDetected{}, }, { - name: "IPv4 no duplicate address", - netProto: ipv4.ProtocolNumber, - dadNetProto: arp.ProtocolNumber, - remoteAddr: utils.Ipv4Addr3.AddressWithPrefix.Address, - expectedResolved: true, + name: "IPv4 no duplicate address", + netProto: ipv4.ProtocolNumber, + dadNetProto: arp.ProtocolNumber, + remoteAddr: utils.Ipv4Addr3.AddressWithPrefix.Address, + expectedResult: &stack.DADSucceeded{}, }, { - name: "IPv6 no duplicate address", - netProto: ipv6.ProtocolNumber, - dadNetProto: ipv6.ProtocolNumber, - remoteAddr: utils.Ipv6Addr3.AddressWithPrefix.Address, - expectedResolved: true, + name: "IPv6 no duplicate address", + netProto: ipv6.ProtocolNumber, + dadNetProto: ipv6.ProtocolNumber, + remoteAddr: utils.Ipv6Addr3.AddressWithPrefix.Address, + expectedResult: &stack.DADSucceeded{}, }, } @@ -1260,7 +1260,7 @@ func TestDAD(t *testing.T) { } expectResults := 1 - if test.expectedResolved { + if _, ok := test.expectedResult.(*stack.DADSucceeded); ok { const delta = time.Nanosecond clock.Advance(time.Duration(dadConfigs.DupAddrDetectTransmits)*dadConfigs.RetransmitTimer - delta) select { @@ -1285,7 +1285,7 @@ func TestDAD(t *testing.T) { } for i := 0; i < expectResults; i++ { - if diff := cmp.Diff(stack.DADResult{Resolved: test.expectedResolved}, <-ch); diff != "" { + if diff := cmp.Diff(test.expectedResult, <-ch); diff != "" { t.Errorf("(i=%d) DAD result mismatch (-want +got):\n%s", i, diff) } } diff --git a/pkg/tcpip/tests/integration/loopback_test.go b/pkg/tcpip/tests/integration/loopback_test.go index c56155ea2..80afc2825 100644 --- a/pkg/tcpip/tests/integration/loopback_test.go +++ b/pkg/tcpip/tests/integration/loopback_test.go @@ -38,7 +38,7 @@ var _ ipv6.NDPDispatcher = (*ndpDispatcher)(nil) type ndpDispatcher struct{} -func (*ndpDispatcher) OnDuplicateAddressDetectionStatus(tcpip.NICID, tcpip.Address, bool, tcpip.Error) { +func (*ndpDispatcher) OnDuplicateAddressDetectionResult(tcpip.NICID, tcpip.Address, stack.DADResult) { } func (*ndpDispatcher) OnDefaultRouterDiscovered(tcpip.NICID, tcpip.Address) bool { -- cgit v1.2.3