ackhandler: don't fail ECN validation if less than 10 testing packets are lost (#4088)

* ackhandler: don't fail ECN validation less than 10 testing packets lost

* ackhandler: simplify checks for mangling and loss of all testing packets
This commit is contained in:
Marten Seemann 2023-09-17 23:00:05 +04:00 committed by GitHub
parent 9010cfd2bb
commit c12f425803
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 29 additions and 9 deletions

View file

@ -125,6 +125,10 @@ func (e *ecnTracker) LostPacket(pn protocol.PacketNumber) {
return
}
e.numLostTesting++
// Only proceed if we have sent all 10 testing packets.
if e.state != ecnStateUnknown {
return
}
if e.numLostTesting >= e.numSentTesting {
e.logger.Debugf("Disabling ECN. All testing packets were lost.")
if e.tracer != nil && e.tracer.ECNStateUpdated != nil {
@ -223,16 +227,16 @@ func (e *ecnTracker) HandleNewlyAcked(packets []*packet, ect0, ect1, ecnce int64
e.numAckedECT1 = ect1
e.numAckedECNCE = ecnce
if e.state == ecnStateTesting || e.state == ecnStateUnknown {
// Detect mangling (a path remarking all ECN-marked testing packets as CE).
if e.numSentECT0+e.numSentECT1 == e.numAckedECNCE && e.numAckedECNCE >= numECNTestingPackets {
if e.tracer != nil && e.tracer.ECNStateUpdated != nil {
e.tracer.ECNStateUpdated(logging.ECNStateFailed, logging.ECNFailedManglingDetected)
}
e.state = ecnStateFailed
return false
// Detect mangling (a path remarking all ECN-marked testing packets as CE),
// once all 10 testing packets have been sent out.
if e.state == ecnStateUnknown && e.numSentECT0+e.numSentECT1 == e.numAckedECNCE {
if e.tracer != nil && e.tracer.ECNStateUpdated != nil {
e.tracer.ECNStateUpdated(logging.ECNStateFailed, logging.ECNFailedManglingDetected)
}
e.state = ecnStateFailed
return false
}
if e.state == ecnStateTesting || e.state == ecnStateUnknown {
var ackedTestingPacket bool
for _, p := range packets {
if e.isTestingPacket(p.PacketNumber) {

View file

@ -71,6 +71,22 @@ var _ = Describe("ECN tracker", func() {
ecnTracker.LostPacket(16)
})
It("only detects ECN mangling after sending all testing packets", func() {
tracer.EXPECT().ECNStateUpdated(logging.ECNStateTesting, logging.ECNTriggerNoTrigger)
for i := 0; i < 9; i++ {
Expect(ecnTracker.Mode()).To(Equal(protocol.ECT0))
ecnTracker.SentPacket(protocol.PacketNumber(i), protocol.ECT0)
ecnTracker.LostPacket(protocol.PacketNumber(i))
}
// Send the last testing packet, and receive a
tracer.EXPECT().ECNStateUpdated(logging.ECNStateUnknown, logging.ECNTriggerNoTrigger)
Expect(ecnTracker.Mode()).To(Equal(protocol.ECT0))
ecnTracker.SentPacket(9, protocol.ECT0)
// Now lose the last testing packet.
tracer.EXPECT().ECNStateUpdated(logging.ECNStateFailed, logging.ECNFailedLostAllTestingPackets)
ecnTracker.LostPacket(9)
})
It("passes ECN validation when a testing packet is acknowledged, while still in testing state", func() {
tracer.EXPECT().ECNStateUpdated(logging.ECNStateTesting, logging.ECNTriggerNoTrigger)
for i := 0; i < 5; i++ {