make Path MTU Discovery resilient to random packet loss #4545
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,16 +25,80 @@ const ( | |
| maxMTUDiff = 20 | ||
| // send a probe packet every mtuProbeDelay RTTs | ||
| mtuProbeDelay = 5 | ||
| // Once maxLostMTUProbes MTU probe packets larger than a certain size are lost, | ||
| // MTU discovery won't probe for larger MTUs than this size. | ||
| // The algorithm used here is resilient to packet loss of (maxLostMTUProbes - 1) packets. | ||
| maxLostMTUProbes = 3 | ||
| ) | ||
|
|
||
| // The Path MTU is found by sending a larger packet every now and then. | ||
| // If the packet is acknowledged, we conclude that the path supports this larger packet size. | ||
| // If the packet is lost, this can mean one of two things: | ||
| // 1. The path doesn't support this larger packet size, or | ||
| // 2. The packet was lost due to packet loss, independent of its size. | ||
| // The algorithm used here is resilient to packet loss of (maxLostMTUProbes - 1) packets. | ||
| // For simplicty, the following example use maxLostMTUProbes = 2. | ||
| // | ||
| // Initialization: | ||
| // |------------------------------------------------------------------------------| | ||
| // min max | ||
| // | ||
| // The first MTU probe packet will have size (min+max)/2. | ||
| // Assume that this packet is acknowledged. We can now move the min marker, | ||
| // and continue the search in the resulting interval. | ||
| // | ||
| // If 1st probe packet acknowledged: | ||
| // |---------------------------------------|--------------------------------------| | ||
| // min max | ||
| // | ||
| // If 1st probe packet lost: | ||
| // |---------------------------------------|--------------------------------------| | ||
| // min lost[0] max | ||
| // | ||
| // We can't conclude that the path doesn't support this packet size, since the loss of the probe | ||
| // packet could have been unrelated to the packet size. A larger probe packet will be sent later on. | ||
| // After a loss, the next probe packet has size (min+lost[0])/2. | ||
|
There was a problem hiding this comment. Recommendation from the RFC:
Maybe we could select the nearest common PMTU value for the result of this calculation? There was a problem hiding this comment. I'm not sure if there's much to be gained from this. We definitely don't want to jump to a higher value than we have confirmed, just because it's a common value. This might blackhole the entire connection. And if I understand https://www.hb.fh-muenster.de/opus4/frontdoor/deliver/index/docId/14965/file/dplpmtudQuicPaper.pdf correctly, there's not too much benefit, compared to a binary search. Also keep in mind that as more and more traffic is proxied (via MASQUE and friends), the value of "common values" will decrease further. There was a problem hiding this comment.
True. But they also mention limiting the precision to a multiple of four bytes:
There was a problem hiding this comment. That's a good idea. Very similar, we could skip steps that would lead to increases smaller than 4 bytes. There was a problem hiding this comment. I'll go ahead and merge this PR. We can do this as a follow up. |
||
| // Now assume this probe packet is acknowledged: | ||
| // | ||
| // 2nd probe packet acknowledged: | ||
| // |------------------|--------------------|--------------------------------------| | ||
| // min lost[0] max | ||
| // | ||
| // First of all, we conclude that the path supports at least this MTU. That's progress! | ||
| // Second, we probe a bit more aggressively with the next probe packet: | ||
| // After an acknowledgement, the next probe packet has size (min+max)/2. | ||
| // This means we'll send a packet larger than the first probe packet (which was lost). | ||
| // | ||
| // If 3rd probe packet acknowledged: | ||
| // |-------------------------------------------------|----------------------------| | ||
| // min max | ||
| // | ||
| // We can conclude that the loss of the 1st probe packet was not due to its size, and | ||
| // continue searching in a much smaller interval now. | ||
| // | ||
| // If 3rd probe packet lost: | ||
| // |------------------|--------------------|---------|----------------------------| | ||
| // min lost[0] max | ||
| // | ||
| // Since in our example numPTOProbes = 2, and we lost 2 packets smaller than max, we | ||
| // conclude that this packet size is not supported on the path, and reduce the maximum | ||
| // value of the search interval. | ||
| // | ||
| // MTU discovery concludes once the interval min and max has been narrowed down to maxMTUDiff. | ||
|
|
||
| type mtuFinder struct { | ||
| lastProbeTime time.Time | ||
| mtuIncreased func(protocol.ByteCount) | ||
|
|
||
| rttStats *utils.RTTStats | ||
|
|
||
| inFlight protocol.ByteCount // the size of the probe packet currently in flight. InvalidByteCount if none is in flight | ||
| min protocol.ByteCount | ||
| limit protocol.ByteCount | ||
|
|
||
| // on initialization, we treat the maximum size as the first "lost" packet | ||
| lost [maxLostMTUProbes]protocol.ByteCount | ||
| lastProbeWasLost bool | ||
|
|
||
| tracer *logging.ConnectionTracer | ||
| } | ||
|
|
@@ -47,29 +111,43 @@ func newMTUDiscoverer( | |
| mtuIncreased func(protocol.ByteCount), | ||
| tracer *logging.ConnectionTracer, | ||
| ) *mtuFinder { | ||
| f := &mtuFinder{ | ||
| inFlight: protocol.InvalidByteCount, | ||
| min: start, | ||
| limit: max, | ||
| rttStats: rttStats, | ||
| mtuIncreased: mtuIncreased, | ||
| tracer: tracer, | ||
| } | ||
| for i := range f.lost { | ||
| if i == 0 { | ||
| f.lost[i] = max | ||
| continue | ||
| } | ||
| f.lost[i] = protocol.InvalidByteCount | ||
| } | ||
| return f | ||
| } | ||
|
|
||
| func (f *mtuFinder) done() bool { | ||
| return f.max()-f.min <= maxMTUDiff+1 | ||
| } | ||
|
|
||
| func (f *mtuFinder) max() protocol.ByteCount { | ||
| for i, v := range f.lost { | ||
| if v == protocol.InvalidByteCount { | ||
| return f.lost[i-1] | ||
| } | ||
| } | ||
| return f.lost[len(f.lost)-1] | ||
| } | ||
|
|
||
| func (f *mtuFinder) Start() { | ||
| f.lastProbeTime = time.Now() // makes sure the first probe packet is not sent immediately | ||
| } | ||
|
|
||
| func (f *mtuFinder) ShouldSendProbe(now time.Time) bool { | ||
| if f.lastProbeTime.IsZero() { | ||
| return false | ||
| } | ||
| if f.inFlight != protocol.InvalidByteCount || f.done() { | ||
|
|
@@ -79,7 +157,12 @@ func (f *mtuFinder) ShouldSendProbe(now time.Time) bool { | |
| } | ||
|
|
||
| func (f *mtuFinder) GetPing() (ackhandler.Frame, protocol.ByteCount) { | ||
| var size protocol.ByteCount | ||
| if f.lastProbeWasLost { | ||
| size = (f.min + f.lost[0]) / 2 | ||
| } else { | ||
| size = (f.min + f.max()) / 2 | ||
| } | ||
| f.lastProbeTime = time.Now() | ||
| f.inFlight = size | ||
| return ackhandler.Frame{ | ||
|
|
@@ -89,7 +172,7 @@ func (f *mtuFinder) GetPing() (ackhandler.Frame, protocol.ByteCount) { | |
| } | ||
|
|
||
| func (f *mtuFinder) CurrentSize() protocol.ByteCount { | ||
| return f.min | ||
| } | ||
|
|
||
| type mtuFinderAckHandler struct { | ||
|
|
@@ -104,7 +187,25 @@ func (h *mtuFinderAckHandler) OnAcked(wire.Frame) { | |
| panic("OnAcked callback called although there's no MTU probe packet in flight") | ||
| } | ||
| h.inFlight = protocol.InvalidByteCount | ||
| h.min = size | ||
| h.lastProbeWasLost = false | ||
| // remove all values smaller than size from the lost array | ||
| var j int | ||
| for i, v := range h.lost { | ||
| if size < v { | ||
| j = i | ||
| break | ||
| } | ||
| } | ||
| if j > 0 { | ||
| for i := 0; i < len(h.lost); i++ { | ||
| if i+j < len(h.lost) { | ||
| h.lost[i] = h.lost[i+j] | ||
| } else { | ||
| h.lost[i] = protocol.InvalidByteCount | ||
| } | ||
| } | ||
| } | ||
| if h.tracer != nil && h.tracer.UpdatedMTU != nil { | ||
| h.tracer.UpdatedMTU(size, h.done()) | ||
| } | ||
|
|
@@ -116,6 +217,13 @@ func (h *mtuFinderAckHandler) OnLost(wire.Frame) { | |
| if size == protocol.InvalidByteCount { | ||
| panic("OnLost callback called although there's no MTU probe packet in flight") | ||
| } | ||
| h.lastProbeWasLost = true | ||
| h.inFlight = protocol.InvalidByteCount | ||
| for i, v := range h.lost { | ||
| if size < v { | ||
| copy(h.lost[i+1:], h.lost[i:]) | ||
| h.lost[i] = size | ||
| break | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This follows the
BinaryCandidate Sequence as outlined in the QUIC Path MTU paper. While it performs good, the authors conclude that theOptBinaryvariant is preferable: