From 79bb3a9bd3ebe4ffc218d3f1ead87759ff1f16ff Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 7 Aug 2017 15:50:49 +0700 Subject: [PATCH] force sending of a retransmittable packet every 20 packets --- ackhandler/interfaces.go | 1 + ackhandler/sent_packet_handler.go | 9 +++++++++ ackhandler/sent_packet_handler_test.go | 28 ++++++++++++++++++++++++++ protocol/server_parameters.go | 3 +++ session.go | 4 ++++ session_test.go | 22 ++++++++++++++++---- 6 files changed, 63 insertions(+), 4 deletions(-) diff --git a/ackhandler/interfaces.go b/ackhandler/interfaces.go index ac14e201..cb1c1a0e 100644 --- a/ackhandler/interfaces.go +++ b/ackhandler/interfaces.go @@ -15,6 +15,7 @@ type SentPacketHandler interface { SendingAllowed() bool GetStopWaitingFrame(force bool) *frames.StopWaitingFrame + ShouldSendRetransmittablePacket() bool DequeuePacketForRetransmission() (packet *Packet) GetLeastUnacked() protocol.PacketNumber diff --git a/ackhandler/sent_packet_handler.go b/ackhandler/sent_packet_handler.go index 300b665e..4ffaea2c 100644 --- a/ackhandler/sent_packet_handler.go +++ b/ackhandler/sent_packet_handler.go @@ -40,6 +40,8 @@ type sentPacketHandler struct { lastSentPacketNumber protocol.PacketNumber skippedPackets []protocol.PacketNumber + numNonRetransmittablePackets int // number of non-retransmittable packets since the last retransmittable packet + LargestAcked protocol.PacketNumber largestReceivedPacketWithAck protocol.PacketNumber @@ -89,6 +91,10 @@ func (h *sentPacketHandler) largestInOrderAcked() protocol.PacketNumber { return h.LargestAcked } +func (h *sentPacketHandler) ShouldSendRetransmittablePacket() bool { + return h.numNonRetransmittablePackets >= protocol.MaxNonRetransmittablePackets +} + func (h *sentPacketHandler) SentPacket(packet *Packet) error { if packet.PacketNumber <= h.lastSentPacketNumber { return errPacketNumberNotIncreasing @@ -116,6 +122,9 @@ func (h *sentPacketHandler) SentPacket(packet *Packet) error { packet.SendTime = now h.bytesInFlight += packet.Length h.packetHistory.PushBack(*packet) + h.numNonRetransmittablePackets = 0 + } else { + h.numNonRetransmittablePackets++ } h.congestion.OnPacketSent( diff --git a/ackhandler/sent_packet_handler_test.go b/ackhandler/sent_packet_handler_test.go index 22dfbf87..f68369f6 100644 --- a/ackhandler/sent_packet_handler_test.go +++ b/ackhandler/sent_packet_handler_test.go @@ -61,6 +61,10 @@ func retransmittablePacket(num protocol.PacketNumber) *Packet { return &Packet{PacketNumber: num, Length: 1, Frames: []frames.Frame{&frames.PingFrame{}}} } +func nonRetransmittablePacket(num protocol.PacketNumber) *Packet { + return &Packet{PacketNumber: num, Length: 1, Frames: []frames.Frame{&frames.AckFrame{}}} +} + var _ = Describe("SentPacketHandler", func() { var ( handler *sentPacketHandler @@ -222,6 +226,30 @@ var _ = Describe("SentPacketHandler", func() { }) }) + Context("forcing retransmittable packets", func() { + It("says that every 20th packet should be retransmittable", func() { + // send 19 non-retransmittable packets + for i := 1; i <= protocol.MaxNonRetransmittablePackets; i++ { + Expect(handler.ShouldSendRetransmittablePacket()).To(BeFalse()) + err := handler.SentPacket(nonRetransmittablePacket(protocol.PacketNumber(i))) + Expect(err).ToNot(HaveOccurred()) + } + Expect(handler.ShouldSendRetransmittablePacket()).To(BeTrue()) + }) + + It("resets the counter when a retransmittable packet is sent", func() { + // send 19 non-retransmittable packets + for i := 1; i <= protocol.MaxNonRetransmittablePackets; i++ { + Expect(handler.ShouldSendRetransmittablePacket()).To(BeFalse()) + err := handler.SentPacket(nonRetransmittablePacket(protocol.PacketNumber(i))) + Expect(err).ToNot(HaveOccurred()) + } + err := handler.SentPacket(retransmittablePacket(20)) + Expect(err).ToNot(HaveOccurred()) + Expect(handler.ShouldSendRetransmittablePacket()).To(BeFalse()) + }) + }) + Context("DoS mitigation", func() { It("checks the size of the packet history, for unacked packets", func() { i := protocol.PacketNumber(1) diff --git a/protocol/server_parameters.go b/protocol/server_parameters.go index 8e632cc1..c2ab602b 100644 --- a/protocol/server_parameters.go +++ b/protocol/server_parameters.go @@ -99,6 +99,9 @@ const MaxTrackedReceivedAckRanges = DefaultMaxCongestionWindow // MaxPacketsReceivedBeforeAckSend is the number of packets that can be received before an ACK frame is sent const MaxPacketsReceivedBeforeAckSend = 20 +// MaxNonRetransmittablePackets is the maximum number of non-retransmittable packets that we send in a row +const MaxNonRetransmittablePackets = 19 + // RetransmittablePacketsBeforeAck is the number of retransmittable that an ACK is sent for const RetransmittablePacketsBeforeAck = 2 diff --git a/session.go b/session.go index b299beac..4a4d89b2 100644 --- a/session.go +++ b/session.go @@ -667,6 +667,10 @@ func (s *session) sendPacket() error { s.packer.QueueControlFrame(swf) } } + // add a retransmittable frame + if s.sentPacketHandler.ShouldSendRetransmittablePacket() { + s.packer.QueueControlFrame(&frames.PingFrame{}) + } packet, err := s.packer.PackPacket() if err != nil || packet == nil { return err diff --git a/session_test.go b/session_test.go index 3556c54d..994c40ac 100644 --- a/session_test.go +++ b/session_test.go @@ -60,10 +60,11 @@ func (m *mockUnpacker) Unpack(publicHeaderBinary []byte, hdr *PublicHeader, data } type mockSentPacketHandler struct { - retransmissionQueue []*ackhandler.Packet - sentPackets []*ackhandler.Packet - congestionLimited bool - requestedStopWaiting bool + retransmissionQueue []*ackhandler.Packet + sentPackets []*ackhandler.Packet + congestionLimited bool + requestedStopWaiting bool + shouldSendRetransmittablePacket bool } func (h *mockSentPacketHandler) SentPacket(packet *ackhandler.Packet) error { @@ -79,6 +80,11 @@ func (h *mockSentPacketHandler) GetLeastUnacked() protocol.PacketNumber { return func (h *mockSentPacketHandler) GetAlarmTimeout() time.Time { panic("not implemented") } func (h *mockSentPacketHandler) OnAlarm() { panic("not implemented") } func (h *mockSentPacketHandler) SendingAllowed() bool { return !h.congestionLimited } +func (h *mockSentPacketHandler) ShouldSendRetransmittablePacket() bool { + b := h.shouldSendRetransmittablePacket + h.shouldSendRetransmittablePacket = false + return b +} func (h *mockSentPacketHandler) GetStopWaitingFrame(force bool) *frames.StopWaitingFrame { h.requestedStopWaiting = true @@ -949,6 +955,14 @@ var _ = Describe("Session", func() { Expect(mconn.written[0]).To(ContainSubstring(string([]byte{0x5E, 0x03}))) }) + It("sends a retransmittable packet when required by the SentPacketHandler", func() { + sess.sentPacketHandler = &mockSentPacketHandler{shouldSendRetransmittablePacket: true} + err := sess.sendPacket() + Expect(err).ToNot(HaveOccurred()) + Expect(mconn.written).To(HaveLen(1)) + Expect(sess.sentPacketHandler.(*mockSentPacketHandler).sentPackets[0].Frames).To(ContainElement(&frames.PingFrame{})) + }) + It("sends two WindowUpdate frames", func() { _, err := sess.GetOrOpenStream(5) Expect(err).ToNot(HaveOccurred())