From 9385aac35c44161537527660fa2d74972964886f Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 21 Apr 2016 16:36:47 +0700 Subject: [PATCH] ack ACK handling for ACKs without NACKs --- ackhandler/interfaces.go | 2 +- ackhandler/outgoing_packet_ack_handler.go | 33 +++++++++- .../outgoing_packet_ack_handler_test.go | 66 +++++++++++++++++++ frames/ack_frame.go | 2 +- frames/ack_frame_test.go | 6 +- 5 files changed, 102 insertions(+), 7 deletions(-) diff --git a/ackhandler/interfaces.go b/ackhandler/interfaces.go index 72a321f3c..849d43687 100644 --- a/ackhandler/interfaces.go +++ b/ackhandler/interfaces.go @@ -7,7 +7,7 @@ import ( type OutgoingPacketAckHandler interface { SentPacket(packet *Packet) error - ReceivedAck(ackFrame *frames.AckFrame) + ReceivedAck(ackFrame *frames.AckFrame) error DequeuePacketForRetransmission() (packet *Packet) } diff --git a/ackhandler/outgoing_packet_ack_handler.go b/ackhandler/outgoing_packet_ack_handler.go index 3c2e64e6f..67ab085fa 100644 --- a/ackhandler/outgoing_packet_ack_handler.go +++ b/ackhandler/outgoing_packet_ack_handler.go @@ -52,8 +52,37 @@ func (h *outgoingPacketAckHandler) SentPacket(packet *Packet) error { return nil } -func (h *outgoingPacketAckHandler) ReceivedAck(ackFrame *frames.AckFrame) { - return +func (h *outgoingPacketAckHandler) ReceivedAck(ackFrame *frames.AckFrame) error { + if ackFrame.LargestObserved > h.lastSentPacketNumber { + return errors.New("OutgoingPacketAckHandler: Received ACK for an unsent package") + } + + entropyError := errors.New("OutgoingPacketAckHandler: Wrong entropy") + + h.packetHistoryMutex.Lock() + defer h.packetHistoryMutex.Unlock() + + highestInOrderAckedEntropy := h.highestInOrderAckedEntropy + highestInOrderAckedPacketNumber := ackFrame.GetHighestInOrderPacketNumber() + for i := h.highestInOrderAckedPacketNumber + 1; i <= highestInOrderAckedPacketNumber; i++ { + highestInOrderAckedEntropy.Add(h.packetHistory[i].PacketNumber, h.packetHistory[i].EntropyBit) + } + + if !ackFrame.HasNACK() { + if ackFrame.Entropy != byte(h.packetHistory[ackFrame.LargestObserved].Entropy) { + return entropyError + } + } + // ToDo: check entropy for ACKs with NACKs + + // Entropy ok. Now actually process the ACK packet + for i := h.highestInOrderAckedPacketNumber; i <= highestInOrderAckedPacketNumber; i++ { + delete(h.packetHistory, i) + } + + h.highestInOrderAckedPacketNumber = highestInOrderAckedPacketNumber + h.highestInOrderAckedEntropy = highestInOrderAckedEntropy + return nil } func (h *outgoingPacketAckHandler) DequeuePacketForRetransmission() (packet *Packet) { diff --git a/ackhandler/outgoing_packet_ack_handler_test.go b/ackhandler/outgoing_packet_ack_handler_test.go index 0befccb8b..680e13c16 100644 --- a/ackhandler/outgoing_packet_ack_handler_test.go +++ b/ackhandler/outgoing_packet_ack_handler_test.go @@ -1,6 +1,7 @@ package ackhandler import ( + "github.com/lucas-clemente/quic-go/frames" "github.com/lucas-clemente/quic-go/protocol" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -56,4 +57,69 @@ var _ = Describe("AckHandler", func() { Expect(handler.packetHistory).ToNot(HaveKey(protocol.PacketNumber(2))) }) }) + + Context("ACK handling", func() { + var ( + packets []*Packet + ) + BeforeEach(func() { + packets = []*Packet{ + &Packet{PacketNumber: 1, Plaintext: []byte{0x13, 0x37}, EntropyBit: true}, + &Packet{PacketNumber: 2, Plaintext: []byte{0xBE, 0xEF}, EntropyBit: false}, + &Packet{PacketNumber: 3, Plaintext: []byte{0xCA, 0xFE}, EntropyBit: true}, + &Packet{PacketNumber: 4, Plaintext: []byte{0x54, 0x32}, EntropyBit: true}, + &Packet{PacketNumber: 5, Plaintext: []byte{0x54, 0x32}, EntropyBit: false}, + &Packet{PacketNumber: 6, Plaintext: []byte{0x54, 0x32}, EntropyBit: true}, + } + for _, packet := range packets { + handler.SentPacket(packet) + } + }) + + It("rejects ACKs with a too high LargestObserved packet number", func() { + ack := frames.AckFrame{ + LargestObserved: 1337, + } + err := handler.ReceivedAck(&ack) + Expect(err).To(HaveOccurred()) + Expect(handler.highestInOrderAckedPacketNumber).To(Equal(protocol.PacketNumber(0))) + }) + + Context("ACKs without NACK ranges", func() { + It("handles an ACK with the correct entropy", func() { + expectedEntropy := EntropyAccumulator(0) + largestObserved := 4 + for i := 0; i < largestObserved; i++ { + expectedEntropy.Add(packets[i].PacketNumber, packets[i].EntropyBit) + } + ack := frames.AckFrame{ + LargestObserved: protocol.PacketNumber(largestObserved), + Entropy: byte(expectedEntropy), + } + err := handler.ReceivedAck(&ack) + Expect(err).ToNot(HaveOccurred()) + Expect(handler.highestInOrderAckedPacketNumber).To(Equal(protocol.PacketNumber(largestObserved))) + Expect(handler.highestInOrderAckedEntropy).To(Equal(expectedEntropy)) + // all packets with packetNumbers smaller or equal largestObserved should be deleted + Expect(handler.packetHistory).ToNot(HaveKey(protocol.PacketNumber(1))) + Expect(handler.packetHistory).ToNot(HaveKey(protocol.PacketNumber(4))) + Expect(handler.packetHistory).To(HaveKey(protocol.PacketNumber(5))) + Expect(handler.packetHistory).To(HaveKey(protocol.PacketNumber(6))) + }) + + It("rejects an ACK with incorrect entropy", func() { + ack := frames.AckFrame{ + LargestObserved: 3, + Entropy: 0, + } + err := handler.ReceivedAck(&ack) + Expect(err).To(HaveOccurred()) + Expect(handler.highestInOrderAckedPacketNumber).To(Equal(protocol.PacketNumber(0))) + Expect(handler.highestInOrderAckedEntropy).To(Equal(EntropyAccumulator(0))) + // nothing should be deleted from the packetHistory map + Expect(handler.packetHistory).To(HaveKey(protocol.PacketNumber(1))) + Expect(handler.packetHistory).To(HaveKey(protocol.PacketNumber(3))) + }) + }) + }) }) diff --git a/frames/ack_frame.go b/frames/ack_frame.go index 388a01607..dad0bd1a1 100644 --- a/frames/ack_frame.go +++ b/frames/ack_frame.go @@ -84,7 +84,7 @@ func (f *AckFrame) HasNACK() bool { } // GetHighestInOrderPacket gets the highest in order packet number that is confirmed by this ACK -func (f *AckFrame) GetHighestInOrderPacket() protocol.PacketNumber { +func (f *AckFrame) GetHighestInOrderPacketNumber() protocol.PacketNumber { if f.HasNACK() { return (f.NackRanges[len(f.NackRanges)-1].FirstPacketNumber - 1) } diff --git a/frames/ack_frame_test.go b/frames/ack_frame_test.go index b677388c2..54097172e 100644 --- a/frames/ack_frame_test.go +++ b/frames/ack_frame_test.go @@ -76,7 +76,7 @@ var _ = Describe("AckFrame", func() { It("gets the highest in order packet number for an ACK without NACK ranges", func() { frame := AckFrame{LargestObserved: 5} - Expect(frame.GetHighestInOrderPacket()).To(Equal(protocol.PacketNumber(5))) + Expect(frame.GetHighestInOrderPacketNumber()).To(Equal(protocol.PacketNumber(5))) }) It("gets the highest in order packet number for an ACK with one NACK ranges", func() { @@ -85,7 +85,7 @@ var _ = Describe("AckFrame", func() { LargestObserved: 6, NackRanges: []NackRange{nackRange}, } - Expect(frame.GetHighestInOrderPacket()).To(Equal(protocol.PacketNumber(2))) + Expect(frame.GetHighestInOrderPacketNumber()).To(Equal(protocol.PacketNumber(2))) }) It("gets the highest in order packet number for an ACK with one NACK ranges", func() { @@ -98,7 +98,7 @@ var _ = Describe("AckFrame", func() { LargestObserved: 15, NackRanges: nackRanges, } - Expect(frame.GetHighestInOrderPacket()).To(Equal(protocol.PacketNumber(3))) + Expect(frame.GetHighestInOrderPacketNumber()).To(Equal(protocol.PacketNumber(3))) }) })