Include duplicate address holder info in DADResult

The integrator may be interested in who owns a duplicate address so
pass this information (if available) along.

Fixes #5605.

PiperOrigin-RevId: 361213556

6 files changed, 54 insertions, 41 deletions

diff --git a/pkg/tcpip/network/arp/arp.go b/pkg/tcpip/network/arp/arp.go
index 98ded525c..ae0461a6d 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, &stack.DADDupAddrDetected{})
+		e.mu.dad.StopLocked(addr, &stack.DADDupAddrDetected{HolderLinkAddress: linkAddr})
 		e.mu.Unlock()
 
 		// The solicited, override, and isRouter flags are not available for ARP;
diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go
index 1cebe75ec..6344a3e09 100644
--- a/pkg/tcpip/network/ipv6/icmp.go
+++ b/pkg/tcpip/network/ipv6/icmp.go
@@ -382,6 +382,10 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 			// stack know so it can handle such a scenario and do nothing further with
 			// the NS.
 			if srcAddr == header.IPv6Any {
+				// Since this is a DAD message we know the sender does not actually hold
+				// the target address so there is no "holder".
+				var holderLinkAddress tcpip.LinkAddress
+
 				// We would get an error if the address no longer exists or the address
 				// is no longer tentative (DAD resolved between the call to
 				// hasTentativeAddr and this point). Both of these are valid scenarios:
@@ -393,7 +397,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 				//
 				// TODO(gvisor.dev/issue/4046): Handle the scenario when a duplicate
 				// address is detected for an assigned address.
-				switch err := e.dupTentativeAddrDetected(targetAddr); err.(type) {
+				switch err := e.dupTentativeAddrDetected(targetAddr, holderLinkAddress); err.(type) {
 				case nil, *tcpip.ErrBadAddress, *tcpip.ErrInvalidEndpointState:
 				default:
 					panic(fmt.Sprintf("unexpected error handling duplicate tentative address: %s", err))
@@ -561,10 +565,24 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 		// 5, NDP messages cannot be fragmented. Also note that in the common case
 		// NDP datagrams are very small and AsView() will not incur allocations.
 		na := header.NDPNeighborAdvert(payload.AsView())
+
+		it, err := na.Options().Iter(false /* check */)
+		if err != nil {
+			// If we have a malformed NDP NA option, drop the packet.
+			received.invalid.Increment()
+			return
+		}
+
+		targetLinkAddr, ok := getTargetLinkAddr(it)
+		if !ok {
+			received.invalid.Increment()
+			return
+		}
+
 		targetAddr := na.TargetAddress()
 
 		e.dad.mu.Lock()
-		e.dad.mu.dad.StopLocked(targetAddr, &stack.DADDupAddrDetected{})
+		e.dad.mu.dad.StopLocked(targetAddr, &stack.DADDupAddrDetected{HolderLinkAddress: targetLinkAddr})
 		e.dad.mu.Unlock()
 
 		if e.hasTentativeAddr(targetAddr) {
@@ -584,7 +602,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 			//
 			// TODO(gvisor.dev/issue/4046): Handle the scenario when a duplicate
 			// address is detected for an assigned address.
-			switch err := e.dupTentativeAddrDetected(targetAddr); err.(type) {
+			switch err := e.dupTentativeAddrDetected(targetAddr, targetLinkAddr); err.(type) {
 			case nil, *tcpip.ErrBadAddress, *tcpip.ErrInvalidEndpointState:
 				return
 			default:
@@ -592,13 +610,6 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 			}
 		}
 
-		it, err := na.Options().Iter(false /* check */)
-		if err != nil {
-			// If we have a malformed NDP NA option, drop the packet.
-			received.invalid.Increment()
-			return
-		}
-
 		// At this point we know that the target address is not tentative on the
 		// NIC. However, the target address may still be assigned to the NIC but not
 		// tentative (it could be permanent). Such a scenario is beyond the scope of
@@ -608,11 +619,6 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 		// TODO(b/143147598): Handle the scenario described above. Also inform the
 		// netstack integration that a duplicate address was detected outside of
 		// DAD.
-		targetLinkAddr, ok := getTargetLinkAddr(it)
-		if !ok {
-			received.invalid.Increment()
-			return
-		}
 
 		// As per RFC 4861 section 7.1.2:
 		//   A node MUST silently discard any received Neighbor Advertisement
diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go
index d9ab240b6..46b6cc41a 100644
--- a/pkg/tcpip/network/ipv6/ipv6.go
+++ b/pkg/tcpip/network/ipv6/ipv6.go
@@ -348,7 +348,7 @@ func (e *endpoint) hasTentativeAddr(addr tcpip.Address) bool {
 // dupTentativeAddrDetected removes the tentative address if it exists. If the
 // address was generated via SLAAC, an attempt is made to generate a new
 // address.
-func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address) tcpip.Error {
+func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address, holderLinkAddr tcpip.LinkAddress) tcpip.Error {
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
@@ -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 */, &stack.DADDupAddrDetected{}); err != nil {
+	if err := e.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, &stack.DADDupAddrDetected{HolderLinkAddress: holderLinkAddr}); err != nil {
 		return err
 	}
 
diff --git a/pkg/tcpip/stack/ndp_test.go b/pkg/tcpip/stack/ndp_test.go
index e7669de69..47796a6ba 100644
--- a/pkg/tcpip/stack/ndp_test.go
+++ b/pkg/tcpip/stack/ndp_test.go
@@ -343,7 +343,7 @@ func TestDADDisabled(t *testing.T) {
 	select {
 	case e := <-ndpDisp.dadC:
 		if diff := checkDADEvent(e, nicID, addr1, &stack.DADSucceeded{}); diff != "" {
-			t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+			t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 		}
 	default:
 		t.Fatal("expected DAD event")
@@ -490,7 +490,7 @@ func TestDADResolve(t *testing.T) {
 				t.Fatal("timed out waiting for DAD resolution")
 			case e := <-ndpDisp.dadC:
 				if diff := checkDADEvent(e, nicID, addr1, &stack.DADSucceeded{}); diff != "" {
-					t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+					t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 				}
 			}
 			if err := checkGetMainNICAddress(s, nicID, header.IPv6ProtocolNumber, addrWithPrefix); err != nil {
@@ -596,9 +596,10 @@ func TestDADFail(t *testing.T) {
 	const nicID = 1
 
 	tests := []struct {
-		name    string
-		rxPkt   func(e *channel.Endpoint, tgt tcpip.Address)
-		getStat func(s tcpip.ICMPv6ReceivedPacketStats) *tcpip.StatCounter
+		name                      string
+		rxPkt                     func(e *channel.Endpoint, tgt tcpip.Address)
+		getStat                   func(s tcpip.ICMPv6ReceivedPacketStats) *tcpip.StatCounter
+		expectedHolderLinkAddress tcpip.LinkAddress
 	}{
 		{
 			name:  "RxSolicit",
@@ -606,6 +607,7 @@ func TestDADFail(t *testing.T) {
 			getStat: func(s tcpip.ICMPv6ReceivedPacketStats) *tcpip.StatCounter {
 				return s.NeighborSolicit
 			},
+			expectedHolderLinkAddress: "",
 		},
 		{
 			name: "RxAdvert",
@@ -640,6 +642,7 @@ func TestDADFail(t *testing.T) {
 			getStat: func(s tcpip.ICMPv6ReceivedPacketStats) *tcpip.StatCounter {
 				return s.NeighborAdvert
 			},
+			expectedHolderLinkAddress: linkAddr1,
 		},
 	}
 
@@ -689,8 +692,8 @@ 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, &stack.DADDupAddrDetected{}); diff != "" {
-					t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+				if diff := checkDADEvent(e, nicID, addr1, &stack.DADDupAddrDetected{HolderLinkAddress: test.expectedHolderLinkAddress}); diff != "" {
+					t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 				}
 			}
 			if err := checkGetMainNICAddress(s, nicID, header.IPv6ProtocolNumber, tcpip.AddressWithPrefix{}); err != nil {
@@ -789,7 +792,7 @@ func TestDADStop(t *testing.T) {
 				t.Fatal("timed out waiting for DAD failure")
 			case e := <-ndpDisp.dadC:
 				if diff := checkDADEvent(e, nicID, addr1, &stack.DADAborted{}); diff != "" {
-					t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+					t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 				}
 			}
 
@@ -851,7 +854,7 @@ func TestSetNDPConfigurations(t *testing.T) {
 				select {
 				case e := <-ndpDisp.dadC:
 					if diff := checkDADEvent(e, nicID, addr, &stack.DADSucceeded{}); diff != "" {
-						t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+						t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 					}
 				default:
 					t.Fatalf("expected DAD event for %s", addr)
@@ -943,7 +946,7 @@ func TestSetNDPConfigurations(t *testing.T) {
 				t.Fatal("timed out waiting for DAD resolution")
 			case e := <-ndpDisp.dadC:
 				if diff := checkDADEvent(e, nicID1, addr1, &stack.DADSucceeded{}); diff != "" {
-					t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+					t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 				}
 			}
 			if err := checkGetMainNICAddress(s, nicID1, header.IPv6ProtocolNumber, addrWithPrefix1); err != nil {
@@ -1962,7 +1965,7 @@ func TestAutoGenTempAddr(t *testing.T) {
 					select {
 					case e := <-ndpDisp.dadC:
 						if diff := checkDADEvent(e, nicID, addr, &stack.DADSucceeded{}); diff != "" {
-							t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+							t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 						}
 					case <-time.After(time.Duration(test.dupAddrTransmits)*test.retransmitTimer + defaultAsyncPositiveEventTimeout):
 						t.Fatal("timed out waiting for DAD event")
@@ -2168,7 +2171,7 @@ func TestNoAutoGenTempAddrForLinkLocal(t *testing.T) {
 				select {
 				case e := <-ndpDisp.dadC:
 					if diff := checkDADEvent(e, nicID, llAddr1, &stack.DADSucceeded{}); diff != "" {
-						t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+						t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 					}
 				case <-time.After(time.Duration(test.dupAddrTransmits)*test.retransmitTimer + defaultAsyncPositiveEventTimeout):
 					t.Fatal("timed out waiting for DAD event")
@@ -2256,7 +2259,7 @@ func TestNoAutoGenTempAddrWithoutStableAddr(t *testing.T) {
 	select {
 	case e := <-ndpDisp.dadC:
 		if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADSucceeded{}); diff != "" {
-			t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+			t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 		}
 	case <-time.After(dadTransmits*retransmitTimer + defaultAsyncPositiveEventTimeout):
 		t.Fatal("timed out waiting for DAD event")
@@ -2722,7 +2725,7 @@ func TestMixedSLAACAddrConflictRegen(t *testing.T) {
 
 				clock.Advance(dupAddrTransmits * retransmitTimer)
 				if diff := checkDADEvent(<-ndpDisp.dadC, nicID, addr, &stack.DADSucceeded{}); diff != "" {
-					t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+					t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 				}
 			}
 
@@ -2753,7 +2756,7 @@ func TestMixedSLAACAddrConflictRegen(t *testing.T) {
 			select {
 			case e := <-ndpDisp.dadC:
 				if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADDupAddrDetected{}); diff != "" {
-					t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+					t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 				}
 			default:
 				t.Fatal("expected DAD event")
@@ -3857,7 +3860,7 @@ func TestAutoGenAddrInResponseToDADConflicts(t *testing.T) {
 		select {
 		case e := <-ndpDisp.dadC:
 			if diff := checkDADEvent(e, nicID, addr, res); diff != "" {
-				t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+				t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 			}
 		default:
 			t.Fatal("expected DAD event")
@@ -3870,7 +3873,7 @@ func TestAutoGenAddrInResponseToDADConflicts(t *testing.T) {
 		select {
 		case e := <-ndpDisp.dadC:
 			if diff := checkDADEvent(e, nicID, addr, res); diff != "" {
-				t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+				t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 			}
 		case <-time.After(dadTransmits*retransmitTimer + defaultAsyncPositiveEventTimeout):
 			t.Fatal("timed out waiting for DAD event")
@@ -4143,7 +4146,7 @@ func TestAutoGenAddrWithEUI64IIDNoDADRetries(t *testing.T) {
 			select {
 			case e := <-ndpDisp.dadC:
 				if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADDupAddrDetected{}); diff != "" {
-					t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+					t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 				}
 			default:
 				t.Fatal("expected DAD event")
@@ -4242,7 +4245,7 @@ func TestAutoGenAddrContinuesLifetimesAfterRetry(t *testing.T) {
 	select {
 	case e := <-ndpDisp.dadC:
 		if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADDupAddrDetected{}); diff != "" {
-			t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+			t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 		}
 	default:
 		t.Fatal("expected DAD event")
@@ -4254,7 +4257,7 @@ func TestAutoGenAddrContinuesLifetimesAfterRetry(t *testing.T) {
 	select {
 	case e := <-ndpDisp.dadC:
 		if diff := checkDADEvent(e, nicID, addr.Address, &stack.DADSucceeded{}); diff != "" {
-			t.Errorf("dad event mismatch (-want +got):\n%s", diff)
+			t.Errorf("DAD event mismatch (-want +got):\n%s", diff)
 		}
 	case <-time.After(dadTransmits*retransmitTimer + defaultAsyncPositiveEventTimeout):
 		t.Fatal("timed out waiting for DAD event")
diff --git a/pkg/tcpip/stack/registration.go b/pkg/tcpip/stack/registration.go
index 426342fd2..85f0f471a 100644
--- a/pkg/tcpip/stack/registration.go
+++ b/pkg/tcpip/stack/registration.go
@@ -884,7 +884,11 @@ func (*DADAborted) isDADResult() {}
 var _ DADResult = (*DADDupAddrDetected)(nil)
 
 // DADDupAddrDetected indicates DAD detected a duplicate address.
-type DADDupAddrDetected struct{}
+type DADDupAddrDetected struct {
+	// HolderLinkAddress is the link address of the node that holds the duplicate
+	// address.
+	HolderLinkAddress tcpip.LinkAddress
+}
 
 func (*DADDupAddrDetected) isDADResult() {}
 
diff --git a/pkg/tcpip/tests/integration/link_resolution_test.go b/pkg/tcpip/tests/integration/link_resolution_test.go
index 759cee4ef..095623789 100644
--- a/pkg/tcpip/tests/integration/link_resolution_test.go
+++ b/pkg/tcpip/tests/integration/link_resolution_test.go
@@ -1192,14 +1192,14 @@ func TestDAD(t *testing.T) {
 			netProto:       ipv4.ProtocolNumber,
 			dadNetProto:    arp.ProtocolNumber,
 			remoteAddr:     utils.Ipv4Addr2.AddressWithPrefix.Address,
-			expectedResult: &stack.DADDupAddrDetected{},
+			expectedResult: &stack.DADDupAddrDetected{HolderLinkAddress: utils.LinkAddr2},
 		},
 		{
 			name:           "IPv6 duplicate address",
 			netProto:       ipv6.ProtocolNumber,
 			dadNetProto:    ipv6.ProtocolNumber,
 			remoteAddr:     utils.Ipv6Addr2.AddressWithPrefix.Address,
-			expectedResult: &stack.DADDupAddrDetected{},
+			expectedResult: &stack.DADDupAddrDetected{HolderLinkAddress: utils.LinkAddr2},
 		},
 		{
 			name:           "IPv4 no duplicate address",