use the packet receive time in the receivedPacketHandler

By passing the packet receive time to the receivedPacketHandler we can
get rid of two time.Now() syscalls.
This commit is contained in:
Marten Seemann 2018-01-12 11:30:39 +07:00
parent 53dd697adf
commit 9b3139a1d6
6 changed files with 67 additions and 55 deletions

View file

@ -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

View file

@ -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)
}
}
}

View file

@ -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)

View file

@ -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)
}

View file

@ -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
}

View file

@ -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()