Do not drop packets w/ missing TCP timestamps.
RFC7323 recommends that if the timestamp option was negotiated then all packets should carry a TCP Timestamp and any packets that do not should be dropped. Netstack implemented this behaviour. Linux OTOH does not and will accept such packets. This change makes Netstack behaviour compatible with Linux. Also now that we allow such packets, we do need to update RTO calculations based on these packets even if timestamp option is enabled. PiperOrigin-RevId: 233432268 Change-Id: I9f4742ae6b63930ac3b5e37d8c238761e6a4b29f
This commit is contained in:
parent
85d53d81d9
commit
efe5e737d7
|
@ -289,8 +289,9 @@ func (h *handshake) synRcvdState(s *segment) *tcpip.Error {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Update timestamp if required. See RFC7323, section-4.3.
|
// Update timestamp if required. See RFC7323, section-4.3.
|
||||||
|
if h.ep.sendTSOk && s.parsedOptions.TS {
|
||||||
h.ep.updateRecentTimestamp(s.parsedOptions.TSVal, h.ackNum, s.sequenceNumber)
|
h.ep.updateRecentTimestamp(s.parsedOptions.TSVal, h.ackNum, s.sequenceNumber)
|
||||||
|
}
|
||||||
h.state = handshakeCompleted
|
h.state = handshakeCompleted
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
@ -740,16 +741,6 @@ func (e *endpoint) handleSegments() *tcpip.Error {
|
||||||
// send window scale.
|
// send window scale.
|
||||||
s.window <<= e.snd.sndWndScale
|
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
|
// RFC 793, page 41 states that "once in the ESTABLISHED
|
||||||
// state all segments must carry current acknowledgment
|
// state all segments must carry current acknowledgment
|
||||||
// information."
|
// information."
|
||||||
|
|
|
@ -635,14 +635,15 @@ func (s *sender) checkDuplicateAck(seg *segment) (rtx bool) {
|
||||||
// updating the send-related state.
|
// updating the send-related state.
|
||||||
func (s *sender) handleRcvdSegment(seg *segment) {
|
func (s *sender) handleRcvdSegment(seg *segment) {
|
||||||
// Check if we can extract an RTT measurement from this ack.
|
// 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.updateRTO(time.Now().Sub(s.rttMeasureTime))
|
||||||
s.rttMeasureSeqNum = s.sndNxt
|
s.rttMeasureSeqNum = s.sndNxt
|
||||||
}
|
}
|
||||||
|
|
||||||
// Update Timestamp if required. See RFC7323, section-4.3.
|
// Update Timestamp if required. See RFC7323, section-4.3.
|
||||||
|
if s.ep.sendTSOk && seg.parsedOptions.TS {
|
||||||
s.ep.updateRecentTimestamp(seg.parsedOptions.TSVal, s.maxSentAck, seg.sequenceNumber)
|
s.ep.updateRecentTimestamp(seg.parsedOptions.TSVal, s.maxSentAck, seg.sequenceNumber)
|
||||||
|
}
|
||||||
// Count the duplicates and do the fast retransmit if needed.
|
// Count the duplicates and do the fast retransmit if needed.
|
||||||
rtx := s.checkDuplicateAck(seg)
|
rtx := s.checkDuplicateAck(seg)
|
||||||
|
|
||||||
|
|
|
@ -255,7 +255,7 @@ func TestSendGreaterThanMTUWithOptions(t *testing.T) {
|
||||||
testBrokenUpWrite(t, c, maxPayload)
|
testBrokenUpWrite(t, c, maxPayload)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSegmentDropWhenTimestampMissing(t *testing.T) {
|
func TestSegmentNotDroppedWhenTimestampMissing(t *testing.T) {
|
||||||
const maxPayload = 100
|
const maxPayload = 100
|
||||||
c := context.New(t, uint32(header.TCPMinimumSize+header.IPv4MinimumSize+maxPayload))
|
c := context.New(t, uint32(header.TCPMinimumSize+header.IPv4MinimumSize+maxPayload))
|
||||||
defer c.Cleanup()
|
defer c.Cleanup()
|
||||||
|
@ -270,37 +270,16 @@ func TestSegmentDropWhenTimestampMissing(t *testing.T) {
|
||||||
droppedPacketsStat := c.Stack().Stats().DroppedPackets
|
droppedPacketsStat := c.Stack().Stats().DroppedPackets
|
||||||
droppedPackets := droppedPacketsStat.Value()
|
droppedPackets := droppedPacketsStat.Value()
|
||||||
data := []byte{1, 2, 3}
|
data := []byte{1, 2, 3}
|
||||||
// Save the sequence number as we will reset it later down
|
// Send a packet with no TCP options/timestamp.
|
||||||
// in the test.
|
|
||||||
savedSeqNum := rep.NextSeqNum
|
|
||||||
rep.SendPacket(data, nil)
|
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 {
|
select {
|
||||||
case <-ch:
|
case <-ch:
|
||||||
case <-time.After(1 * time.Second):
|
case <-time.After(1 * time.Second):
|
||||||
t.Fatalf("Timed out waiting for data to arrive")
|
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 {
|
if got, want := droppedPacketsStat.Value(), droppedPackets; got != want {
|
||||||
t.Fatalf("incorrect number of dropped packets, got: %v, want: %v", got, want)
|
t.Fatalf("incorrect number of dropped packets, got: %v, want: %v", got, want)
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue