From 399199e4b7c89ff9d670e37ed8ae93bbca0defb4 Mon Sep 17 00:00:00 2001 From: Bhasker Hariharan Date: Mon, 4 Apr 2022 13:50:47 -0700 Subject: [PATCH] Call ConfirmReachable only once per batch of packets. ConfirmReachable does not need to be called on every packet processed as we process packets in batches. Its more efficient to do so once for every batch in handleSegmentsLocked. PiperOrigin-RevId: 439402884 --- pkg/tcpip/transport/tcp/connect.go | 11 +++++++++++ pkg/tcpip/transport/tcp/snd.go | 12 ------------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/pkg/tcpip/transport/tcp/connect.go b/pkg/tcpip/transport/tcp/connect.go index 22e37f1a4..dbc3ea599 100644 --- a/pkg/tcpip/transport/tcp/connect.go +++ b/pkg/tcpip/transport/tcp/connect.go @@ -1142,6 +1142,7 @@ func (e *endpoint) handleReset(s *segment) (ok bool, err tcpip.Error) { // +checklocksalias:e.snd.ep.mu=e.mu func (e *endpoint) handleSegmentsLocked(fastPath bool) tcpip.Error { checkRequeue := true + sndUna := e.snd.SndUna for i := 0; i < maxSegmentsPerWake; i++ { if state := e.EndpointState(); state.closed() || state == StateTimeWait { return nil @@ -1162,6 +1163,16 @@ func (e *endpoint) handleSegmentsLocked(fastPath bool) tcpip.Error { } } + // The remote ACK-ing at least 1 byte is an indication that we have a + // full-duplex connection to the remote as the only way we will receive an + // ACK is if the remote received data that we previously sent. + // + // As of writing, Linux seems to only confirm a route as reachable when + // forward progress is made which is indicated by an ACK that removes data + // from the retransmit queue, i.e. sender makes forward progress. + if sndUna.LessThan(e.snd.SndUna) { + e.route.ConfirmReachable() + } // When fastPath is true we don't want to wake up the worker // goroutine. If the endpoint has more segments to process the // dispatcher will call handleSegments again anyway. diff --git a/pkg/tcpip/transport/tcp/snd.go b/pkg/tcpip/transport/tcp/snd.go index 4e22faf0a..e518799a3 100644 --- a/pkg/tcpip/transport/tcp/snd.go +++ b/pkg/tcpip/transport/tcp/snd.go @@ -1496,18 +1496,6 @@ func (s *sender) handleRcvdSegment(rcvdSeg *segment) { // Remove all acknowledged data from the write list. acked := s.SndUna.Size(ack) s.SndUna = ack - - // The remote ACK-ing at least 1 byte is an indication that we have a - // full-duplex connection to the remote as the only way we will receive an - // ACK is if the remote received data that we previously sent. - // - // As of writing, linux seems to only confirm a route as reachable when - // forward progress is made which is indicated by an ACK that removes data - // from the retransmit queue. - if acked > 0 { - s.ep.route.ConfirmReachable() - } - ackLeft := acked originalOutstanding := s.Outstanding for ackLeft > 0 {