diff --git a/ackhandler/interfaces.go b/ackhandler/interfaces.go index 914ee146..8bda9588 100644 --- a/ackhandler/interfaces.go +++ b/ackhandler/interfaces.go @@ -27,7 +27,7 @@ type SentPacketHandler interface { // ReceivedPacketHandler handles ACKs needed to send for incoming packets type ReceivedPacketHandler interface { - ReceivedPacket(packetNumber protocol.PacketNumber, shouldInstigateAck bool) error + ReceivedPacket(packetNumber protocol.PacketNumber, rcvTime time.Time, shouldInstigateAck bool) error IgnoreBelow(protocol.PacketNumber) GetAlarmTimeout() time.Time diff --git a/ackhandler/received_packet_handler.go b/ackhandler/received_packet_handler.go index 97410f37..c316af4a 100644 --- a/ackhandler/received_packet_handler.go +++ b/ackhandler/received_packet_handler.go @@ -34,10 +34,10 @@ func NewReceivedPacketHandler(version protocol.VersionNumber) ReceivedPacketHand } } -func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumber, shouldInstigateAck bool) error { +func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumber, rcvTime time.Time, shouldInstigateAck bool) error { if packetNumber > h.largestObserved { h.largestObserved = packetNumber - h.largestObservedReceivedTime = time.Now() + h.largestObservedReceivedTime = rcvTime } if packetNumber < h.ignoreBelow { @@ -47,7 +47,7 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe if err := h.packetHistory.ReceivedPacket(packetNumber); err != nil { return err } - h.maybeQueueAck(packetNumber, shouldInstigateAck) + h.maybeQueueAck(packetNumber, rcvTime, shouldInstigateAck) return nil } @@ -58,7 +58,7 @@ func (h *receivedPacketHandler) IgnoreBelow(p protocol.PacketNumber) { h.packetHistory.DeleteBelow(p) } -func (h *receivedPacketHandler) maybeQueueAck(packetNumber protocol.PacketNumber, shouldInstigateAck bool) { +func (h *receivedPacketHandler) maybeQueueAck(packetNumber protocol.PacketNumber, rcvTime time.Time, shouldInstigateAck bool) { h.packetsReceivedSinceLastAck++ if shouldInstigateAck { @@ -86,7 +86,7 @@ func (h *receivedPacketHandler) maybeQueueAck(packetNumber protocol.PacketNumber h.ackQueued = true } else { if h.ackAlarm.IsZero() { - h.ackAlarm = time.Now().Add(h.ackSendDelay) + h.ackAlarm = rcvTime.Add(h.ackSendDelay) } } } diff --git a/ackhandler/received_packet_handler_test.go b/ackhandler/received_packet_handler_test.go index 4a87c72d..10246bd8 100644 --- a/ackhandler/received_packet_handler_test.go +++ b/ackhandler/received_packet_handler_test.go @@ -21,34 +21,36 @@ var _ = Describe("receivedPacketHandler", func() { Context("accepting packets", func() { It("handles a packet that arrives late", func() { - err := handler.ReceivedPacket(protocol.PacketNumber(1), true) + err := handler.ReceivedPacket(protocol.PacketNumber(1), time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(protocol.PacketNumber(3), true) + err = handler.ReceivedPacket(protocol.PacketNumber(3), time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(protocol.PacketNumber(2), true) + err = handler.ReceivedPacket(protocol.PacketNumber(2), time.Time{}, true) Expect(err).ToNot(HaveOccurred()) }) It("saves the time when each packet arrived", func() { - err := handler.ReceivedPacket(protocol.PacketNumber(3), true) + err := handler.ReceivedPacket(protocol.PacketNumber(3), time.Now(), true) Expect(err).ToNot(HaveOccurred()) Expect(handler.largestObservedReceivedTime).To(BeTemporally("~", time.Now(), 10*time.Millisecond)) }) It("updates the largestObserved and the largestObservedReceivedTime", func() { + now := time.Now() handler.largestObserved = 3 - handler.largestObservedReceivedTime = time.Now().Add(-1 * time.Second) - err := handler.ReceivedPacket(5, true) + handler.largestObservedReceivedTime = now.Add(-1 * time.Second) + err := handler.ReceivedPacket(5, now, true) Expect(err).ToNot(HaveOccurred()) Expect(handler.largestObserved).To(Equal(protocol.PacketNumber(5))) - Expect(handler.largestObservedReceivedTime).To(BeTemporally("~", time.Now(), 10*time.Millisecond)) + Expect(handler.largestObservedReceivedTime).To(Equal(now)) }) It("doesn't update the largestObserved and the largestObservedReceivedTime for a belated packet", func() { - timestamp := time.Now().Add(-1 * time.Second) + now := time.Now() + timestamp := now.Add(-1 * time.Second) handler.largestObserved = 5 handler.largestObservedReceivedTime = timestamp - err := handler.ReceivedPacket(4, true) + err := handler.ReceivedPacket(4, now, true) Expect(err).ToNot(HaveOccurred()) Expect(handler.largestObserved).To(Equal(protocol.PacketNumber(5))) Expect(handler.largestObservedReceivedTime).To(Equal(timestamp)) @@ -57,7 +59,7 @@ var _ = Describe("receivedPacketHandler", func() { It("passes on errors from receivedPacketHistory", func() { var err error for i := protocol.PacketNumber(0); i < 5*protocol.MaxTrackedReceivedAckRanges; i++ { - err = handler.ReceivedPacket(2*i+1, true) + err = handler.ReceivedPacket(2*i+1, time.Time{}, true) // this will eventually return an error // details about when exactly the receivedPacketHistory errors are tested there if err != nil { @@ -72,7 +74,7 @@ var _ = Describe("receivedPacketHandler", func() { Context("queueing ACKs", func() { receiveAndAck10Packets := func() { for i := 1; i <= 10; i++ { - err := handler.ReceivedPacket(protocol.PacketNumber(i), true) + err := handler.ReceivedPacket(protocol.PacketNumber(i), time.Time{}, true) Expect(err).ToNot(HaveOccurred()) } Expect(handler.GetAckFrame()).ToNot(BeNil()) @@ -80,14 +82,14 @@ var _ = Describe("receivedPacketHandler", func() { } It("always queues an ACK for the first packet", func() { - err := handler.ReceivedPacket(1, false) + err := handler.ReceivedPacket(1, time.Time{}, false) Expect(err).ToNot(HaveOccurred()) Expect(handler.ackQueued).To(BeTrue()) Expect(handler.GetAlarmTimeout()).To(BeZero()) }) It("works with packet number 0", func() { - err := handler.ReceivedPacket(0, false) + err := handler.ReceivedPacket(0, time.Time{}, false) Expect(err).ToNot(HaveOccurred()) Expect(handler.ackQueued).To(BeTrue()) Expect(handler.GetAlarmTimeout()).To(BeZero()) @@ -95,11 +97,11 @@ var _ = Describe("receivedPacketHandler", func() { It("queues an ACK for every second retransmittable packet, if they are arriving fast", func() { receiveAndAck10Packets() - err := handler.ReceivedPacket(11, true) + err := handler.ReceivedPacket(11, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) Expect(handler.ackQueued).To(BeFalse()) Expect(handler.GetAlarmTimeout()).NotTo(BeZero()) - err = handler.ReceivedPacket(12, true) + err = handler.ReceivedPacket(12, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) Expect(handler.ackQueued).To(BeTrue()) Expect(handler.GetAlarmTimeout()).To(BeZero()) @@ -107,11 +109,11 @@ var _ = Describe("receivedPacketHandler", func() { It("only sets the timer when receiving a retransmittable packets", func() { receiveAndAck10Packets() - err := handler.ReceivedPacket(11, false) + err := handler.ReceivedPacket(11, time.Time{}, false) Expect(err).ToNot(HaveOccurred()) Expect(handler.ackQueued).To(BeFalse()) Expect(handler.ackAlarm).To(BeZero()) - err = handler.ReceivedPacket(12, true) + err = handler.ReceivedPacket(12, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) Expect(handler.ackQueued).To(BeFalse()) Expect(handler.ackAlarm).ToNot(BeZero()) @@ -120,15 +122,15 @@ var _ = Describe("receivedPacketHandler", func() { It("queues an ACK if it was reported missing before", func() { receiveAndAck10Packets() - err := handler.ReceivedPacket(11, true) + err := handler.ReceivedPacket(11, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(13, true) + err = handler.ReceivedPacket(13, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() // ACK: 1 and 3, missing: 2 Expect(ack).ToNot(BeNil()) Expect(ack.HasMissingRanges()).To(BeTrue()) Expect(handler.ackQueued).To(BeFalse()) - err = handler.ReceivedPacket(12, false) + err = handler.ReceivedPacket(12, time.Time{}, false) Expect(err).ToNot(HaveOccurred()) Expect(handler.ackQueued).To(BeTrue()) }) @@ -136,10 +138,10 @@ var _ = Describe("receivedPacketHandler", func() { It("queues an ACK if it creates a new missing range", func() { receiveAndAck10Packets() for i := 11; i < 16; i++ { - err := handler.ReceivedPacket(protocol.PacketNumber(i), true) + err := handler.ReceivedPacket(protocol.PacketNumber(i), time.Time{}, true) Expect(err).ToNot(HaveOccurred()) } - err := handler.ReceivedPacket(20, true) // we now know that packets 16 to 19 are missing + err := handler.ReceivedPacket(20, time.Time{}, true) // we now know that packets 16 to 19 are missing Expect(err).ToNot(HaveOccurred()) Expect(handler.ackQueued).To(BeTrue()) ack := handler.GetAckFrame() @@ -154,9 +156,9 @@ var _ = Describe("receivedPacketHandler", func() { }) It("generates a simple ACK frame", func() { - err := handler.ReceivedPacket(1, true) + err := handler.ReceivedPacket(1, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(2, true) + err = handler.ReceivedPacket(2, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() Expect(ack).ToNot(BeNil()) @@ -166,7 +168,7 @@ var _ = Describe("receivedPacketHandler", func() { }) It("generates an ACK for packet number 0", func() { - err := handler.ReceivedPacket(0, true) + err := handler.ReceivedPacket(0, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() Expect(ack).ToNot(BeNil()) @@ -176,12 +178,12 @@ var _ = Describe("receivedPacketHandler", func() { }) It("saves the last sent ACK", func() { - err := handler.ReceivedPacket(1, true) + err := handler.ReceivedPacket(1, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() Expect(ack).ToNot(BeNil()) Expect(handler.lastAck).To(Equal(ack)) - err = handler.ReceivedPacket(2, true) + err = handler.ReceivedPacket(2, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) handler.ackQueued = true ack = handler.GetAckFrame() @@ -190,9 +192,9 @@ var _ = Describe("receivedPacketHandler", func() { }) It("generates an ACK frame with missing packets", func() { - err := handler.ReceivedPacket(1, true) + err := handler.ReceivedPacket(1, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(4, true) + err = handler.ReceivedPacket(4, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() Expect(ack).ToNot(BeNil()) @@ -205,11 +207,11 @@ var _ = Describe("receivedPacketHandler", func() { }) It("generates an ACK for packet number 0 and other packets", func() { - err := handler.ReceivedPacket(0, true) + err := handler.ReceivedPacket(0, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(1, true) + err = handler.ReceivedPacket(1, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(3, true) + err = handler.ReceivedPacket(3, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() Expect(ack).ToNot(BeNil()) @@ -223,15 +225,15 @@ var _ = Describe("receivedPacketHandler", func() { It("accepts packets below the lower limit", func() { handler.IgnoreBelow(6) - err := handler.ReceivedPacket(2, true) + err := handler.ReceivedPacket(2, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) }) It("doesn't add delayed packets to the packetHistory", func() { handler.IgnoreBelow(7) - err := handler.ReceivedPacket(4, true) + err := handler.ReceivedPacket(4, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(10, true) + err = handler.ReceivedPacket(10, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() Expect(ack).ToNot(BeNil()) @@ -241,7 +243,7 @@ var _ = Describe("receivedPacketHandler", func() { It("deletes packets from the packetHistory when a lower limit is set", func() { for i := 1; i <= 12; i++ { - err := handler.ReceivedPacket(protocol.PacketNumber(i), true) + err := handler.ReceivedPacket(protocol.PacketNumber(i), time.Time{}, true) Expect(err).ToNot(HaveOccurred()) } handler.IgnoreBelow(7) @@ -256,7 +258,7 @@ var _ = Describe("receivedPacketHandler", func() { // TODO: remove this test when dropping support for STOP_WAITINGs It("handles a lower limit of 0", func() { handler.IgnoreBelow(0) - err := handler.ReceivedPacket(1337, true) + err := handler.ReceivedPacket(1337, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) ack := handler.GetAckFrame() Expect(ack).ToNot(BeNil()) @@ -264,7 +266,7 @@ var _ = Describe("receivedPacketHandler", func() { }) It("resets all counters needed for the ACK queueing decision when sending an ACK", func() { - err := handler.ReceivedPacket(1, true) + err := handler.ReceivedPacket(1, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) handler.ackAlarm = time.Now().Add(-time.Minute) Expect(handler.GetAckFrame()).ToNot(BeNil()) @@ -275,7 +277,7 @@ var _ = Describe("receivedPacketHandler", func() { }) It("doesn't generate an ACK when none is queued and the timer is not set", func() { - err := handler.ReceivedPacket(1, true) + err := handler.ReceivedPacket(1, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) handler.ackQueued = false handler.ackAlarm = time.Time{} @@ -283,7 +285,7 @@ var _ = Describe("receivedPacketHandler", func() { }) It("doesn't generate an ACK when none is queued and the timer has not yet expired", func() { - err := handler.ReceivedPacket(1, true) + err := handler.ReceivedPacket(1, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) handler.ackQueued = false handler.ackAlarm = time.Now().Add(time.Minute) @@ -291,7 +293,7 @@ var _ = Describe("receivedPacketHandler", func() { }) It("generates an ACK when the timer has expired", func() { - err := handler.ReceivedPacket(1, true) + err := handler.ReceivedPacket(1, time.Time{}, true) Expect(err).ToNot(HaveOccurred()) handler.ackQueued = false handler.ackAlarm = time.Now().Add(-time.Minute) diff --git a/internal/mocks/ackhandler/received_packet_handler.go b/internal/mocks/ackhandler/received_packet_handler.go index 03e0ce73..670d12d3 100644 --- a/internal/mocks/ackhandler/received_packet_handler.go +++ b/internal/mocks/ackhandler/received_packet_handler.go @@ -71,13 +71,13 @@ func (mr *MockReceivedPacketHandlerMockRecorder) IgnoreBelow(arg0 interface{}) * } // ReceivedPacket mocks base method -func (m *MockReceivedPacketHandler) ReceivedPacket(arg0 protocol.PacketNumber, arg1 bool) error { - ret := m.ctrl.Call(m, "ReceivedPacket", arg0, arg1) +func (m *MockReceivedPacketHandler) ReceivedPacket(arg0 protocol.PacketNumber, arg1 time.Time, arg2 bool) error { + ret := m.ctrl.Call(m, "ReceivedPacket", arg0, arg1, arg2) ret0, _ := ret[0].(error) return ret0 } // ReceivedPacket indicates an expected call of ReceivedPacket -func (mr *MockReceivedPacketHandlerMockRecorder) ReceivedPacket(arg0, arg1 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReceivedPacket", reflect.TypeOf((*MockReceivedPacketHandler)(nil).ReceivedPacket), arg0, arg1) +func (mr *MockReceivedPacketHandlerMockRecorder) ReceivedPacket(arg0, arg1, arg2 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReceivedPacket", reflect.TypeOf((*MockReceivedPacketHandler)(nil).ReceivedPacket), arg0, arg1, arg2) } diff --git a/session.go b/session.go index 09af8c09..53487d58 100644 --- a/session.go +++ b/session.go @@ -532,7 +532,7 @@ func (s *session) handlePacketImpl(p *receivedPacket) error { s.largestRcvdPacketNumber = utils.MaxPacketNumber(s.largestRcvdPacketNumber, hdr.PacketNumber) isRetransmittable := ackhandler.HasRetransmittableFrames(packet.frames) - if err = s.receivedPacketHandler.ReceivedPacket(hdr.PacketNumber, isRetransmittable); err != nil { + if err = s.receivedPacketHandler.ReceivedPacket(hdr.PacketNumber, p.rcvTime, isRetransmittable); err != nil { return err } diff --git a/session_test.go b/session_test.go index 6438b45d..47eff4dc 100644 --- a/session_test.go +++ b/session_test.go @@ -532,6 +532,16 @@ var _ = Describe("Session", func() { Expect(sess.largestRcvdPacketNumber).To(Equal(protocol.PacketNumber(5))) }) + It("informs the ReceivedPacketHandler", func() { + now := time.Now().Add(time.Hour) + rph := mockackhandler.NewMockReceivedPacketHandler(mockCtrl) + rph.EXPECT().ReceivedPacket(protocol.PacketNumber(5), now, false) + sess.receivedPacketHandler = rph + hdr.PacketNumber = 5 + err := sess.handlePacketImpl(&receivedPacket{header: hdr, rcvTime: now}) + Expect(err).ToNot(HaveOccurred()) + }) + It("closes when handling a packet fails", func(done Done) { streamManager.EXPECT().CloseWithError(gomock.Any()) testErr := errors.New("unpack error") @@ -609,7 +619,7 @@ var _ = Describe("Session", func() { It("sends ACK frames", func() { packetNumber := protocol.PacketNumber(0x035e) - err := sess.receivedPacketHandler.ReceivedPacket(packetNumber, true) + err := sess.receivedPacketHandler.ReceivedPacket(packetNumber, time.Now(), true) Expect(err).ToNot(HaveOccurred()) sent, err := sess.sendPacket() Expect(err).NotTo(HaveOccurred()) @@ -782,7 +792,7 @@ var _ = Describe("Session", func() { }) sess.sentPacketHandler = sph sess.packer.packetNumberGenerator.next = 0x1338 - sess.receivedPacketHandler.ReceivedPacket(1, true) + sess.receivedPacketHandler.ReceivedPacket(1, time.Now(), true) done := make(chan struct{}) go func() { defer GinkgoRecover() @@ -810,7 +820,7 @@ var _ = Describe("Session", func() { }) sess.sentPacketHandler = sph sess.packer.packetNumberGenerator.next = 0x1338 - sess.receivedPacketHandler.ReceivedPacket(1, true) + sess.receivedPacketHandler.ReceivedPacket(1, time.Now(), true) go func() { defer GinkgoRecover() sess.run()