Merge release-20210112.0-25-ge57ebcd37 (automated)

5 files changed, 88 insertions, 65 deletions

diff --git a/pkg/tcpip/network/arp/arp.go b/pkg/tcpip/network/arp/arp.go
index 3d5c0d270..3259d052f 100644
--- a/pkg/tcpip/network/arp/arp.go
+++ b/pkg/tcpip/network/arp/arp.go
@@ -119,21 +119,28 @@ func (*endpoint) WriteHeaderIncludedPacket(*stack.Route, *stack.PacketBuffer) *t
 }
 
 func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) {
+	stats := e.protocol.stack.Stats().ARP
+	stats.PacketsReceived.Increment()
+
 	if !e.isEnabled() {
+		stats.DisabledPacketsReceived.Increment()
 		return
 	}
 
 	h := header.ARP(pkt.NetworkHeader().View())
 	if !h.IsValid() {
+		stats.MalformedPacketsReceived.Increment()
 		return
 	}
 
 	switch h.Op() {
 	case header.ARPRequest:
+		stats.RequestsReceived.Increment()
 		localAddr := tcpip.Address(h.ProtocolAddressTarget())
 
 		if e.nud == nil {
 			if e.linkAddrCache.CheckLocalAddress(e.nic.ID(), header.IPv4ProtocolNumber, localAddr) == 0 {
+				stats.RequestsReceivedUnknownTargetAddress.Increment()
 				return // we have no useful answer, ignore the request
 			}
 
@@ -142,6 +149,7 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) {
 			e.linkAddrCache.AddLinkAddress(e.nic.ID(), addr, linkAddr)
 		} else {
 			if e.protocol.stack.CheckLocalAddress(e.nic.ID(), header.IPv4ProtocolNumber, localAddr) == 0 {
+				stats.RequestsReceivedUnknownTargetAddress.Increment()
 				return // we have no useful answer, ignore the request
 			}
 
@@ -177,9 +185,14 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) {
 		//
 		//   Send the packet to the (new) target hardware address on the same
 		//   hardware on which the request was received.
-		_ = e.nic.WritePacketToRemote(tcpip.LinkAddress(origSender), nil /* gso */, ProtocolNumber, respPkt)
+		if err := e.nic.WritePacketToRemote(tcpip.LinkAddress(origSender), nil /* gso */, ProtocolNumber, respPkt); err != nil {
+			stats.OutgoingRepliesDropped.Increment()
+		} else {
+			stats.OutgoingRepliesSent.Increment()
+		}
 
 	case header.ARPReply:
+		stats.RepliesReceived.Increment()
 		addr := tcpip.Address(h.ProtocolAddressSender())
 		linkAddr := tcpip.LinkAddress(h.HardwareAddressSender())
 
@@ -233,6 +246,8 @@ func (*protocol) LinkAddressProtocol() tcpip.NetworkProtocolNumber {
 
 // LinkAddressRequest implements stack.LinkAddressResolver.LinkAddressRequest.
 func (p *protocol) LinkAddressRequest(targetAddr, localAddr tcpip.Address, remoteLinkAddr tcpip.LinkAddress, nic stack.NetworkInterface) *tcpip.Error {
+	stats := p.stack.Stats().ARP
+
 	if len(remoteLinkAddr) == 0 {
 		remoteLinkAddr = header.EthernetBroadcastAddress
 	}
@@ -241,15 +256,18 @@ func (p *protocol) LinkAddressRequest(targetAddr, localAddr tcpip.Address, remot
 	if len(localAddr) == 0 {
 		addr, err := p.stack.GetMainNICAddress(nicID, header.IPv4ProtocolNumber)
 		if err != nil {
+			stats.OutgoingRequestInterfaceHasNoLocalAddressErrors.Increment()
 			return err
 		}
 
 		if len(addr.Address) == 0 {
+			stats.OutgoingRequestNetworkUnreachableErrors.Increment()
 			return tcpip.ErrNetworkUnreachable
 		}
 
 		localAddr = addr.Address
 	} else if p.stack.CheckLocalAddress(nicID, header.IPv4ProtocolNumber, localAddr) == 0 {
+		stats.OutgoingRequestBadLocalAddressErrors.Increment()
 		return tcpip.ErrBadLocalAddress
 	}
 
@@ -269,7 +287,12 @@ func (p *protocol) LinkAddressRequest(targetAddr, localAddr tcpip.Address, remot
 	if n := copy(h.ProtocolAddressTarget(), targetAddr); n != header.IPv4AddressSize {
 		panic(fmt.Sprintf("copied %d bytes, expected %d bytes", n, header.IPv4AddressSize))
 	}
-	return nic.WritePacketToRemote(remoteLinkAddr, nil /* gso */, ProtocolNumber, pkt)
+	if err := nic.WritePacketToRemote(remoteLinkAddr, nil /* gso */, ProtocolNumber, pkt); err != nil {
+		stats.OutgoingRequestsDropped.Increment()
+		return err
+	}
+	stats.OutgoingRequestsSent.Increment()
+	return nil
 }
 
 // ResolveStaticAddress implements stack.LinkAddressResolver.ResolveStaticAddress.
diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go
index 002ddaf67..49d4912ad 100644
--- a/pkg/tcpip/tcpip.go
+++ b/pkg/tcpip/tcpip.go
@@ -1591,6 +1591,59 @@ type IPStats struct {
 	OptionUnknownReceived *StatCounter
 }
 
+// ARPStats collects ARP-specific stats.
+type ARPStats struct {
+	// PacketsReceived is the number of ARP packets received from the link layer.
+	PacketsReceived *StatCounter
+
+	// DisabledPacketsReceived is the number of ARP packets received from the link
+	// layer when the ARP layer is disabled.
+	DisabledPacketsReceived *StatCounter
+
+	// MalformedPacketsReceived is the number of ARP packets that were dropped due
+	// to being malformed.
+	MalformedPacketsReceived *StatCounter
+
+	// RequestsReceived is the number of ARP requests received.
+	RequestsReceived *StatCounter
+
+	// RequestsReceivedUnknownTargetAddress is the number of ARP requests that
+	// were targeted to an interface different from the one it was received on.
+	RequestsReceivedUnknownTargetAddress *StatCounter
+
+	// OutgoingRequestInterfaceHasNoLocalAddressErrors is the number of failures
+	// to send an ARP request because the interface has no network address
+	// assigned to it.
+	OutgoingRequestInterfaceHasNoLocalAddressErrors *StatCounter
+
+	// OutgoingRequestBadLocalAddressErrors is the number of failures to send an
+	// ARP request with a bad local address.
+	OutgoingRequestBadLocalAddressErrors *StatCounter
+
+	// OutgoingRequestNetworkUnreachableErrors is the number of failures to send
+	// an ARP request with a network unreachable error.
+	OutgoingRequestNetworkUnreachableErrors *StatCounter
+
+	// OutgoingRequestsDropped is the number of ARP requests which failed to write
+	// to a link-layer endpoint.
+	OutgoingRequestsDropped *StatCounter
+
+	// OutgoingRequestSent is the number of ARP requests successfully written to a
+	// link-layer endpoint.
+	OutgoingRequestsSent *StatCounter
+
+	// RepliesReceived is the number of ARP replies received.
+	RepliesReceived *StatCounter
+
+	// OutgoingRepliesDropped is the number of ARP replies which failed to write
+	// to a link-layer endpoint.
+	OutgoingRepliesDropped *StatCounter
+
+	// OutgoingRepliesSent is the number of ARP replies successfully written to a
+	// link-layer endpoint.
+	OutgoingRepliesSent *StatCounter
+}
+
 // TCPStats collects TCP-specific stats.
 type TCPStats struct {
 	// ActiveConnectionOpenings is the number of connections opened
@@ -1743,6 +1796,9 @@ type Stats struct {
 	// IP breaks out IP-specific stats (both v4 and v6).
 	IP IPStats
 
+	// ARP breaks out ARP-specific stats.
+	ARP ARPStats
+
 	// TCP breaks out TCP-specific stats.
 	TCP TCPStats
 
diff --git a/pkg/tcpip/transport/tcp/accept.go b/pkg/tcpip/transport/tcp/accept.go
index 9e8872fc9..6921de0f1 100644
--- a/pkg/tcpip/transport/tcp/accept.go
+++ b/pkg/tcpip/transport/tcp/accept.go
@@ -305,10 +305,7 @@ func (l *listenContext) startHandshake(s *segment, opts *header.TCPSynOptions, q
 
 	// Initialize and start the handshake.
 	h := ep.newPassiveHandshake(isn, irs, opts, deferAccept)
-	if err := h.start(); err != nil {
-		l.cleanupFailedHandshake(h)
-		return nil, err
-	}
+	h.start()
 	return h, nil
 }
 
diff --git a/pkg/tcpip/transport/tcp/connect.go b/pkg/tcpip/transport/tcp/connect.go
index f45d26a87..6cdbb8bee 100644
--- a/pkg/tcpip/transport/tcp/connect.go
+++ b/pkg/tcpip/transport/tcp/connect.go
@@ -53,7 +53,6 @@ const (
 	wakerForNotification = iota
 	wakerForNewSegment
 	wakerForResend
-	wakerForResolution
 )
 
 const (
@@ -460,9 +459,9 @@ func (h *handshake) processSegments() *tcpip.Error {
 	return nil
 }
 
-// start resolves the route if necessary and sends the first
-// SYN/SYN-ACK.
-func (h *handshake) start() *tcpip.Error {
+// start sends the first SYN/SYN-ACK. It does not block, even if link address
+// resolution is required.
+func (h *handshake) start() {
 	h.startTime = time.Now()
 	h.ep.amss = calculateAdvertisedMSS(h.ep.userMSS, h.ep.route)
 	var sackEnabled tcpip.TCPSACKEnabled
@@ -503,7 +502,6 @@ func (h *handshake) start() *tcpip.Error {
 		ack:    h.ackNum,
 		rcvWnd: h.rcvWnd,
 	}, synOpts)
-	return nil
 }
 
 // complete completes the TCP 3-way handshake initiated by h.start().
diff --git a/pkg/tcpip/transport/tcp/endpoint.go b/pkg/tcpip/transport/tcp/endpoint.go
index ddbed7e46..a4508e871 100644
--- a/pkg/tcpip/transport/tcp/endpoint.go
+++ b/pkg/tcpip/transport/tcp/endpoint.go
@@ -2325,68 +2325,17 @@ func (e *endpoint) connect(addr tcpip.FullAddress, handshake bool, run bool) *tc
 	}
 
 	if run {
-		if err := e.startMainLoop(handshake); err != nil {
-			return err
-		}
-	}
-
-	return tcpip.ErrConnectStarted
-}
-
-// startMainLoop sends the initial SYN and starts the main loop for the
-// endpoint.
-func (e *endpoint) startMainLoop(handshake bool) *tcpip.Error {
-	preloop := func() *tcpip.Error {
 		if handshake {
 			h := e.newHandshake()
 			e.setEndpointState(StateSynSent)
-			if err := h.start(); err != nil {
-				e.lastErrorMu.Lock()
-				e.lastError = err
-				e.lastErrorMu.Unlock()
-
-				e.setEndpointState(StateError)
-				e.hardError = err
-
-				// Call cleanupLocked to free up any reservations.
-				e.cleanupLocked()
-				return err
-			}
+			h.start()
 		}
 		e.stack.Stats().TCP.ActiveConnectionOpenings.Increment()
-		return nil
-	}
-
-	if e.route.IsResolutionRequired() {
-		// If the endpoint is closed between releasing e.mu and the goroutine below
-		// acquiring it, make sure that cleanup is deferred to the new goroutine.
 		e.workerRunning = true
-
-		// Sending the initial SYN may block due to route resolution; do it in a
-		// separate goroutine to avoid blocking the syscall goroutine.
-		go func() { // S/R-SAFE: will be drained before save.
-			e.mu.Lock()
-			if err := preloop(); err != nil {
-				e.workerRunning = false
-				e.mu.Unlock()
-				return
-			}
-			e.mu.Unlock()
-			_ = e.protocolMainLoop(handshake, nil)
-		}()
-		return nil
+		go e.protocolMainLoop(handshake, nil) // S/R-SAFE: will be drained before save.
 	}
 
-	// No route resolution is required, so we can send the initial SYN here without
-	// blocking. This will hopefully reduce overall latency by overlapping time
-	// spent waiting for a SYN-ACK and time spent spinning up a new goroutine
-	// for the main loop.
-	if err := preloop(); err != nil {
-		return err
-	}
-	e.workerRunning = true
-	go e.protocolMainLoop(handshake, nil) // S/R-SAFE: will be drained before save.
-	return nil
+	return tcpip.ErrConnectStarted
 }
 
 // ConnectEndpoint is not supported.