commit 0d77fd78aecfa586cc363000837188d3b222de11
author Scott Graham <scottmg@google.com> Tue May 28 14:30:32 2019 -0700
committer Scott Graham <scottmg@google.com> Tue May 28 14:32:39 2019 -0700
tree 29ec7ea2596b04f0589b610a3346a348d5e7aa11
parent c56ec786c73f21f64efdcc21a8b87dde71b6ab17

[botanist][tftp] Fix over-send causing timeout during paving

First, for reference, window size is always 256.

For a given window, botanist is currently sending blocks in the range of
the window base (called "seq" in tftp.go), let's say 100 here, so from
[101..357] inclusive (see the changed line in this CL).

This is one more than seems to be required by the protocol. Typically
this is fine, the last block (index 357 here) would typically either be
consumed anyway, or if it arrives out of order, it'll just be dropped.

In one very specific situation though, it causes problems. If the target
is receiving blocks in order and advancing through the window normally,
it will eventually get to a point where it receives block 356 (i.e. the
second last one that botanist will send in this attempt).

At that point, the target has received a full window of 256 (that is,
blocks 101..356 have been received), so it replies with an Ack saying
it's OK to move to the next window. Botanist has to wait on this Ack to
move to the next window, and otherwise continues repeating its send from
[101..357]. If this Ack for block 356 is dropped (this is all UDP so
eventually this will happen), AND the "bonus" 357 block is subsequently
received on the target, then the target will have moved on to the next
window block.

The target is now sitting waiting for block 358, thinking that both
sides have moved on to the next window. Botanist is still sending
[101..357] waiting for an Ack, but the target will never resend an Ack
for an old window. On a timeout, the target Acks 357 to attempt to
resync, but that will be ignored by botanist, because it does not fall
within the range check in window advancement (an indeed it is in the
next window).

So, two fixes are possible: botanist could accept that resync ack that's
one-past the end of the window (but I think that's wrong). Instead, it
should just not be sending block 357, and the loop should be [0..256),
that is, exclusive at the end of the range.

I've tried testing this by artificially making the target drop Acks and
paving now recovers and continues as expected even when that happens,
after this botanist change.

ZX-4146 #comment [botanist][tftp] Fix over-send causing timeout during paving

Change-Id: I85faccf45a1a7b173db5d357ba427a22c4b9205c