Code review comment for lp:~martin-lp/hipl/update_ack_handling

On Thu, 11 Aug 2011 15:43:33 +0200, David Martin
<email address hidden> wrote:

> On Wed, Aug 10, 2011 at 7:40 PM, Christof Mroz
> <email address hidden> wrote:
>> You introduce the lower bound in this branch, but other than
>> initializing it to zero it's not updated. Don't you need to adjust it
>> upon succesfully processing the update tagged with the smallest ID?
>
> It gets updated in check_and_update_ack_id_bounds() with every incoming
> valid ACK.

You are right of course. It's hard to spot where the if(...) condition
ends and the body starts IMHO. You may consider adding an empty line after
the opening brace for readability.

> The problems that may be as it is:
> - it gets updated before the HMAC check is performed, a retransmission
> would still be accepted though so this should not make trouble.
> - out-of-order ACKs get dropped. Say you send two update packets with
> SEQ x and x+1 and you receive the ACK for x+1 first, it would update the
> lower bound and ACK for x would be dropped. This is non-trivial to fix
> though. Seems to be ok with the RFC in any case: "The semantics are such
> that the receiver MUST acknowledge the UPDATE, but the sender MAY choose
> to not care about receiving the ACK."

Yes, don't bother as long as the branch solves the original problem (which
it does as far as I can tell).