diff --git a/pkg/tcpip/transport/tcp/connect.go b/pkg/tcpip/transport/tcp/connect.go index cc34e74b2..7b80f7519 100644 --- a/pkg/tcpip/transport/tcp/connect.go +++ b/pkg/tcpip/transport/tcp/connect.go @@ -289,8 +289,9 @@ func (h *handshake) synRcvdState(s *segment) *tcpip.Error { } // Update timestamp if required. See RFC7323, section-4.3. - h.ep.updateRecentTimestamp(s.parsedOptions.TSVal, h.ackNum, s.sequenceNumber) - + if h.ep.sendTSOk && s.parsedOptions.TS { + h.ep.updateRecentTimestamp(s.parsedOptions.TSVal, h.ackNum, s.sequenceNumber) + } h.state = handshakeCompleted return nil } @@ -740,16 +741,6 @@ func (e *endpoint) handleSegments() *tcpip.Error { // send window scale. s.window <<= e.snd.sndWndScale - // If the timestamp option is negotiated and the segment - // does not carry a timestamp option then the segment - // must be dropped as per - // https://tools.ietf.org/html/rfc7323#section-3.2. - if e.sendTSOk && !s.parsedOptions.TS { - e.stack.Stats().DroppedPackets.Increment() - s.decRef() - continue - } - // RFC 793, page 41 states that "once in the ESTABLISHED // state all segments must carry current acknowledgment // information." diff --git a/pkg/tcpip/transport/tcp/snd.go b/pkg/tcpip/transport/tcp/snd.go index 122c52f32..5c9e76fa1 100644 --- a/pkg/tcpip/transport/tcp/snd.go +++ b/pkg/tcpip/transport/tcp/snd.go @@ -635,14 +635,15 @@ func (s *sender) checkDuplicateAck(seg *segment) (rtx bool) { // updating the send-related state. func (s *sender) handleRcvdSegment(seg *segment) { // Check if we can extract an RTT measurement from this ack. - if !s.ep.sendTSOk && s.rttMeasureSeqNum.LessThan(seg.ackNumber) { + if !seg.parsedOptions.TS && s.rttMeasureSeqNum.LessThan(seg.ackNumber) { s.updateRTO(time.Now().Sub(s.rttMeasureTime)) s.rttMeasureSeqNum = s.sndNxt } // Update Timestamp if required. See RFC7323, section-4.3. - s.ep.updateRecentTimestamp(seg.parsedOptions.TSVal, s.maxSentAck, seg.sequenceNumber) - + if s.ep.sendTSOk && seg.parsedOptions.TS { + s.ep.updateRecentTimestamp(seg.parsedOptions.TSVal, s.maxSentAck, seg.sequenceNumber) + } // Count the duplicates and do the fast retransmit if needed. rtx := s.checkDuplicateAck(seg) diff --git a/pkg/tcpip/transport/tcp/tcp_timestamp_test.go b/pkg/tcpip/transport/tcp/tcp_timestamp_test.go index b08df0fec..87c640967 100644 --- a/pkg/tcpip/transport/tcp/tcp_timestamp_test.go +++ b/pkg/tcpip/transport/tcp/tcp_timestamp_test.go @@ -255,7 +255,7 @@ func TestSendGreaterThanMTUWithOptions(t *testing.T) { testBrokenUpWrite(t, c, maxPayload) } -func TestSegmentDropWhenTimestampMissing(t *testing.T) { +func TestSegmentNotDroppedWhenTimestampMissing(t *testing.T) { const maxPayload = 100 c := context.New(t, uint32(header.TCPMinimumSize+header.IPv4MinimumSize+maxPayload)) defer c.Cleanup() @@ -270,37 +270,16 @@ func TestSegmentDropWhenTimestampMissing(t *testing.T) { droppedPacketsStat := c.Stack().Stats().DroppedPackets droppedPackets := droppedPacketsStat.Value() data := []byte{1, 2, 3} - // Save the sequence number as we will reset it later down - // in the test. - savedSeqNum := rep.NextSeqNum + // Send a packet with no TCP options/timestamp. rep.SendPacket(data, nil) - select { - case <-ch: - t.Fatalf("Got data to read when we expect packet to be dropped") - case <-time.After(1 * time.Second): - // We expect that no data will be available to read. - } - - // Assert that DroppedPackets was incremented by 1. - if got, want := droppedPacketsStat.Value(), droppedPackets+1; got != want { - t.Fatalf("incorrect number of dropped packets, got: %v, want: %v", got, want) - } - - droppedPackets = droppedPacketsStat.Value() - // Reset the sequence number so that the other endpoint accepts - // this segment and does not treat it like an out of order delivery. - rep.NextSeqNum = savedSeqNum - // Now send a packet with timestamp and we should get the data. - rep.SendPacketWithTS(data, rep.TSVal+1) - select { case <-ch: case <-time.After(1 * time.Second): t.Fatalf("Timed out waiting for data to arrive") } - // Assert that DroppedPackets was not incremented by 1. + // Assert that DroppedPackets was not incremented. if got, want := droppedPacketsStat.Value(), droppedPackets; got != want { t.Fatalf("incorrect number of dropped packets, got: %v, want: %v", got, want) }