Merge lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl

[David Martin]

Update:
Remarks on the code have been addressed in revisions 6224ff.

Previous description:
This branch fixes and improves the currently partially broken retransmissions in HIPL.

In short:
- buffer up to three retransmissions in simple round-robin style
 -> this does not affect regular management packets during BEX but makes it possible to retransmit the locators of the second UPDATE packet

- we remove buffered retransmission based on incoming packets from the peer
 -> hip_update_retransmissions() in input.c takes care of that
 -> for example an incoming R1 invalidates or acknowledges our buffered I1 retransmission
 -> it's the same for UPDATE packets. incoming U2 invalidates buffered U1, and U3 invalidates U2 respectively
 -> the rationale: if we do not get a response from the peer we can assume our or their packet was lost, so we retransmit. A received packet from the peer acts as acknowledgment and we remove the retransmissions. For this reason R2 and U3 packets do not get buffered for retransmission. Their retransmission is triggered by the peers retransmission of I2 or U2 respectively.

I have relatively thoroughly tested this and in the normal use case (no relay, opportunistic or what else we offer in special use cases) it works fine. Would be great if someone had a look at the other cases.

How to test these changes:
- decrease retransmission timeout define HIP_RETRANSMIT_WAIT in hipd.h (retransmission waiting time is still awfully long at the moment but out of scope of this branch to be changed)

- modify the packet loss defines in output.c (thanks Miika for the hint!)
 -> if it does not compile cast the random() call to uint64_t

[Miika Komu (miika-iki)]

Seems good to me from the view point of functionality. May I ask few questions though:

1. Did you test that memory is deallocated (valgrind) - optional, but extra good karma if you do this :)
2. Does the scheme ignore retransmission of R1 messages (not really supposed to be resent)
3. Why to buffer to three messages?

Regarding to buffering, the state may change during retransmission and this causes the implementation to resend old messages in addition to new ones? If we're lucky, the peer will ignore UPDATE packets with old SPI numbers. However, the situation may be different in the BEX because there's no state.

A special (and supported) use case from base exchange: try to trigger the BEX simultaneously from both sides (i.e. break point both sides with gdb just after sending I1). I know the state machine can handle this but what happens to the retransmissions?

review: Needs Information

[Diego Biurrun (diego-biurrun)]

 review needs-fixing

On Tue, Dec 20, 2011 at 11:43:26AM +0000, David Martin wrote:
> David Martin has proposed merging lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
>
> --- hipd/hadb.c 2011-11-17 11:15:47 +0000
> +++ hipd/hadb.c 2011-12-20 11:41:33 +0000
> @@ -693,17 +693,25 @@
> }
>
> - entry->hip_msg_retrans.count = 0;
>
> /* Initialize module states */

You should remove an excess empty line here.

> --- hipd/input.c 2011-12-09 09:32:56 +0000
> +++ hipd/input.c 2011-12-20 11:41:33 +0000
> @@ -1877,3 +1873,117 @@
> +
> +/**
> + * Clear the given retransmission.
> + * i.e. set the remaining retransmissions to zero and zero the buffer.
> + *
> + * @param retrans The retransmission to be cleared.
> + */
> +void hip_clear_retransmission(struct hip_msg_retrans *const retrans)
> +{
> + if (!retrans) {
> + return;
> + }
> +
> + retrans->count = 0;
> + if (retrans->buf) {
> + memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);

sizeof(retrans->buf) does not work?

> +/**
> + * Update our buffered retransmissions after receiving the given packet.
> + * This functions removes invalid retransmissions after a state change caused
> + * by a received packet. For example after successfully receiving and handling
> + * an I2 or R2 packet all our buffered packets are dropped.
> + *
> + * Call this function after successful handling of the incoming packet.
> + *
> + * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
> + * @param ha_state The host association state (RFC 5201, 4.4.1.)
> + * @param ctx The packet context of the incoming transmission.
> + *
> + * @return always 0
> + */

Why always return 0?

> +int hip_update_retransmissions(UNUSED const uint8_t packet_type,
> + UNUSED const enum hip_state ha_state,
> + struct hip_packet_context *const ctx)
> +{
> + struct hip_msg_retrans *retrans;
> +
> + HIP_ASSERT(ctx);
> + HIP_ASSERT(ctx->input_msg);
> +
> + /* When receiving an I1 there usually is no established state yet. */
> + if (!ctx->hadb_entry) {
> + return 0;
> + }
> +
> + for (unsigned int i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
> + retrans = &ctx->hadb_entry->hip_msg_retrans[i];
> +
> + if (retrans->count == 0) {
> + continue;
> + }
> +
> + switch (hip_get_msg_type(ctx->input_msg)) {
> + case HIP_I1:
> + switch (hip_get_msg_type(retrans->buf)) {
> + case HIP_I1:
> + /* We only remove our sent I1s when we have got a greater HIT
> + * than the peer (ref. RFC 5201, 4.4.2). */
> + if (hip_hit_is_bigger(&ctx->hadb_entry->hit_our, &ctx->hadb_entry->hit_peer)) {
> + hip_clear_retransmission(retrans);
> + }
> + break;
> + }
> + break;

This nested switch statement for only one case looks pretty weird IMO.

> --- hipd/maintenance.c 2011-12-16 13:37:33 +0000
> +++ hipd/maintenance.c 2011-12-20 11:41:33 +0000
> @@ -92,53 +93,48 @@
> static int handle_retransmission(struct hip_hadb_state *entry,
> ...

review: Needs Fixing

[David Martin (martin-lp)]

Hi,

On Tue, Dec 20, 2011 at 6:50 PM, Miika Komu <email address hidden> wrote:
> Review: Needs Information
>
> 1. Did you test that memory is deallocated (valgrind) - optional, but extra good karma if you do this :)

Actually I did and it looks fine. There are only HIP_RETRANSMIT_QUEUE_SIZE - 1 more calls to calloc compared to before so there is not that much to go wrong. Static memory ftw.

> 2. Does the scheme ignore retransmission of R1 messages (not really supposed to be resent)

R1s don't get buffered (hip_send_r1() sets the retransmit flag to zero when sending the packet) so they are not retransmitted as well. I did not touch this or to answer your question: yes, they are ignored.

What I did change though is R2 and U3. They are now not retransmitted anymore as they don't get explicitly acknowledged. A retransmission is basically triggered by the peer when they send I2 / U2 again.

> 3. Why to buffer to three messages?

It's more than one to retransmit the locators during UPDATEs. And three is more or less arbitrarily chosen by me. Two did not seem worth the effort of looping through it and four seems not needed? How many locators are common when using HIPL? On my test setup it's always only two... This can easily be changed by changing HIP_RETRANSMIT_QUEUE_SIZE though.

> Regarding to buffering, the state may change during retransmission and this causes the implementation to resend old messages in addition to new ones? If we're lucky, the peer will ignore UPDATE packets with old SPI numbers. However, the situation may be different in the BEX because there's no state.

I'm not sure whether I fully get your question. You mean for example when you trigger three UPDATEs in a row you would have three retransmissions? Once one of the UPDATEs has been processed the peer would ignore the older retransmissions (check_update_freshness() would fail) and on our side the buffered retransmissions would be removed after receiving the U2 from the peer.

Can you trigger another BEX during BEX? Can I force hipd to send another I1 when I'm in state R1_SENT for example? I did not test this.

In general the behaviour during BEX is basically unchanged though in this branch. When a I2, R1, R2, CLOSE or CLOSE_ACK comes in from the peer we remove all our retransmissions. On the host side a retransmission does not cause a state change. Retransmissions are simply putting a packet again on the wire, not more.

> A special (and supported) use case from base exchange: try to trigger the BEX simultaneously from both sides (i.e. break point both sides with gdb just after sending I1). I know the state machine can handle this but what happens to the retransmissions?

Actually I did try this and when I let two machines ping each other virtually at the same time the BEX sort of gets stuck and neither side can get any traffic through (this is not specific to this branch as it is the same on trunk).

In theory this situation is dealt with though. When receiving an I1 the host with the greater HIT removes its own I1 retransmissions.

[Diego Biurrun (diego-biurrun)]

On Wed, Dec 21, 2011 at 11:32:26AM +0100, Diego Biurrun wrote:
> On Tue, Dec 20, 2011 at 11:43:26AM +0000, David Martin wrote:
> > David Martin has proposed merging lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
> >
> > --- hipd/output.c 2011-11-10 10:53:21 +0000
> > +++ hipd/output.c 2011-12-20 11:41:33 +0000
> > @@ -1118,22 +1118,25 @@
> > const struct hip_common *msg,
> > struct hip_hadb_state *entry)
> > {
> > - int len = hip_get_msg_total_len(msg);
> > + int len = hip_get_msg_total_len(msg);
> > + struct hip_msg_retrans *retrans;
> >
> > if (!entry) {
> > return 0;
> > }
> >
> > + retrans = &entry->hip_msg_retrans[entry->next_retrans_slot];
> > +
> > /* Not reusing the old entry as the new packet may have different length */
> > + memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);
>
> see above
>
> > + memcpy(retrans->buf, msg, len);
> > + memcpy(&retrans->saddr, src_addr, sizeof(struct in6_addr));
> > + memcpy(&retrans->daddr, peer_addr, sizeof(struct in6_addr));

The comment I intended to add here was that this looks as if it can be
done by assignments instead of memcpy.

Diego

[Miika Komu (miika-iki)]

Hi,

On 22/12/11 00:22, Diego Biurrun wrote:
> On Wed, Dec 21, 2011 at 11:32:26AM +0100, Diego Biurrun wrote:
>> On Tue, Dec 20, 2011 at 11:43:26AM +0000, David Martin wrote:
>>> David Martin has proposed merging lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
>>>
>>> --- hipd/output.c 2011-11-10 10:53:21 +0000
>>> +++ hipd/output.c 2011-12-20 11:41:33 +0000
>>> @@ -1118,22 +1118,25 @@
>>> const struct hip_common *msg,
>>> struct hip_hadb_state *entry)
>>> {
>>> - int len = hip_get_msg_total_len(msg);
>>> + int len = hip_get_msg_total_len(msg);
>>> + struct hip_msg_retrans *retrans;
>>>
>>> if (!entry) {
>>> return 0;
>>> }
>>>
>>> + retrans =&entry->hip_msg_retrans[entry->next_retrans_slot];
>>> +
>>> /* Not reusing the old entry as the new packet may have different length */
>>> + memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);
>>
>> see above
>>
>>> + memcpy(retrans->buf, msg, len);
>>> + memcpy(&retrans->saddr, src_addr, sizeof(struct in6_addr));
>>> + memcpy(&retrans->daddr, peer_addr, sizeof(struct in6_addr));
>
> The comment I intended to add here was that this looks as if it can be
> done by assignments instead of memcpy.

if you refer to IPv6 addresses, they have to be copied (no such thing as
128-bit integer). Anyway, the most elegant way is to use
ipv6_addr_copy() instead of memcpy().

[David Martin (martin-lp)]

Hi,

On Wed, Dec 21, 2011 at 11:32 AM, Diego Biurrun <email address hidden> wrote:
> review needs-fixing
>
> On Tue, Dec 20, 2011 at 11:43:26AM +0000, David Martin wrote:
>> David Martin has proposed merging lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
>>
>> --- hipd/hadb.c 2011-11-17 11:15:47 +0000
>> +++ hipd/hadb.c 2011-12-20 11:41:33 +0000
>> @@ -693,17 +693,25 @@
>> }
>>
>> - entry->hip_msg_retrans.count = 0;
>>
>> /* Initialize module states */
>
> You should remove an excess empty line here.

fixed.

>> --- hipd/input.c 2011-12-09 09:32:56 +0000
>> +++ hipd/input.c 2011-12-20 11:41:33 +0000
>> @@ -1877,3 +1873,117 @@
>> +
>> +/**
>> + * Clear the given retransmission.
>> + * i.e. set the remaining retransmissions to zero and zero the buffer.
>> + *
>> + * @param retrans The retransmission to be cleared.
>> + */
>> +void hip_clear_retransmission(struct hip_msg_retrans *const retrans)
>> +{
>> + if (!retrans) {
>> + return;
>> + }
>> +
>> + retrans->count = 0;
>> + if (retrans->buf) {
>> + memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);
>
> sizeof(retrans->buf) does not work?

Nope. It's just a pointer so that would not make much sense. But even referencing the struct does not seem right:

debug(hipd/input.c:1891@hip_clear_retransmission): HIP_MAX_NETWORK_PACKET: 2048
debug(hipd/input.c:1892@hip_clear_retransmission): sizeof(retrans->buf): 8
debug(hipd/input.c:1893@hip_clear_retransmission): sizeof(*retrans->buf): 40

>> +/**
>> + * Update our buffered retransmissions after receiving the given packet.
>> + * This functions removes invalid retransmissions after a state change caused
>> + * by a received packet. For example after successfully receiving and handling
>> + * an I2 or R2 packet all our buffered packets are dropped.
>> + *
>> + * Call this function after successful handling of the incoming packet.
>> + *
>> + * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
>> + * @param ha_state The host association state (RFC 5201, 4.4.1.)
>> + * @param ctx The packet context of the incoming transmission.
>> + *
>> + * @return always 0
>> + */
>
> Why always return 0?

Nothing unforeseen may happen that I deem important enough to break the handling of packets. I would make it void but the handle functions have to return an int.

>> +int hip_update_retransmissions(UNUSED const uint8_t packet_type,
>> + UNUSED const enum hip_state ha_state,
>> + struct hip_packet_context *const ctx)
>> +{
>> + struct hip_msg_retrans *retrans;
>> +
>> + HIP_ASSERT(ctx);
>> + HIP_ASSERT(ctx->input_msg);
>> +
>> + /* When receiving an I1 there usually is no established state yet. */
>> + if (!ctx->hadb_entry) {
>> + return 0;
>> + }
>> +
>> + for (unsigned int i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
>> + retrans = &ctx->hadb_entry->hip_msg_retrans[i];
>> +
>> + if (retrans->count == 0) {
>> + continue;
>> + }
>> +
>> + switch (hip_get_msg_type(ctx->input_msg)) {
>> + case HIP_I1:
>> + switch (hip_get_msg_type...

[David Martin (martin-lp)]

Hi,

On Thu, Dec 22, 2011 at 8:42 AM, Miika Komu <email address hidden> wrote:
> On 22/12/11 00:22, Diego Biurrun wrote:
>>
>> On Wed, Dec 21, 2011 at 11:32:26AM +0100, Diego Biurrun wrote:
>>>
>>> On Tue, Dec 20, 2011 at 11:43:26AM +0000, David Martin wrote:
>>>>
>>>> David Martin has proposed merging
>>>> lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
>>>>
>>>> --- hipd/output.c 2011-11-10 10:53:21 +0000
>>>> +++ hipd/output.c 2011-12-20 11:41:33 +0000
>>>> @@ -1118,22 +1118,25 @@
>>>> const struct hip_common *msg,
>>>> struct hip_hadb_state *entry)
>>>> {
>>>> - int len = hip_get_msg_total_len(msg);
>>>> + int len = hip_get_msg_total_len(msg);
>>>> + struct hip_msg_retrans *retrans;
>>>>
>>>> if (!entry) {
>>>> return 0;
>>>> }
>>>>
>>>> + retrans =&entry->hip_msg_retrans[entry->next_retrans_slot];
>>>>
>>>> +
>>>> /* Not reusing the old entry as the new packet may have different
>>>> length */
>>>> + memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);
>>>
>>>
>>> see above
>>>
>>>> + memcpy(retrans->buf, msg, len);
>>>> + memcpy(&retrans->saddr, src_addr, sizeof(struct in6_addr));
>>>> + memcpy(&retrans->daddr, peer_addr, sizeof(struct in6_addr));
>>
>>
>> The comment I intended to add here was that this looks as if it can be
>> done by assignments instead of memcpy.
>
>
> if you refer to IPv6 addresses, they have to be copied (no such thing as
> 128-bit integer). Anyway, the most elegant way is to use ipv6_addr_copy()
> instead of memcpy().

I don't think you can assign the buffer as they are independent. The retrans buffer slots are allocated at startup and the message comes wherever the caller allocates it from.

I've removed the needless variable indirection of the int len though and replaced the address memcpy() calls with ipv6_addr_copy().

[Miika Komu (miika-iki)]

Hi,

On 21/12/11 12:46, David Martin wrote:
> Hi,
>
> On Tue, Dec 20, 2011 at 6:50 PM, Miika Komu<email address hidden> wrote:
>> Review: Needs Information
>>
>> 1. Did you test that memory is deallocated (valgrind) - optional,
>> but extra good karma if you do this :)
>
> Actually I did and it looks fine. There are only
> HIP_RETRANSMIT_QUEUE_SIZE - 1 more calls to calloc compared to before
> so there is not that much to go wrong. Static memory ftw.
>
>> 2. Does the scheme ignore retransmission of R1 messages (not really
>> supposed to be resent)
>
> R1s don't get buffered (hip_send_r1() sets the retransmit flag to
> zero when sending the packet) so they are not retransmitted as well.
> I did not touch this or to answer your question: yes, they are
> ignored.
>
> What I did change though is R2 and U3. They are now not retransmitted
> anymore as they don't get explicitly acknowledged. A retransmission
> is basically triggered by the peer when they send I2 / U2 again.
>
>
>> 3. Why to buffer to three messages?
>
> It's more than one to retransmit the locators during UPDATEs. And
> three is more or less arbitrarily chosen by me. Two did not seem
> worth the effort of looping through it and four seems not needed? How
> many locators are common when using HIPL? On my test setup it's
> always only two... This can easily be changed by changing
> HIP_RETRANSMIT_QUEUE_SIZE though.

maybe I failed to understand something or should dig into the code
better. Are you saying that a buffered packet is a) retransmitted three
times or b) three packets can be queued?

Option a) makes sense, option b) doesn't make sense because the
retransmitted packets are identical (in most cases, see below).

>> Regarding to buffering, the state may change during retransmission
>> and this causes the implementation to resend old messages in
>> addition to new ones? If we're lucky, the peer will ignore UPDATE
>> packets with old SPI numbers. However, the situation may be
>> different in the BEX because there's no state.
>
> I'm not sure whether I fully get your question. You mean for example
> when you trigger three UPDATEs in a row you would have three
> retransmissions? Once one of the UPDATEs has been processed the peer
> would ignore the older retransmissions (check_update_freshness()
> would fail) and on our side the buffered retransmissions would be
> removed after receiving the U2 from the peer.

Regarding to option b), the packets can be different e.g. with UPDATEs.
It occurs that a host obtains new locators every now and then. If the
host has a queue of length of more than two, then it keeps sending an
old buffered set of locators in addition to the new set. While this is
not a big issue, it's completely unnecessary and should be avoided IMHO.
The easiest way to avoid this is to have queue of size one.

> Can you trigger another BEX during BEX? Can I force hipd to send
> another I1 when I'm in state R1_SENT for example? I did not test
> this.

No.

> In general the behaviour during BEX is basically unchanged though in
> this branch. When a I2, R1, R2, CLOSE or CLOSE_ACK comes in from the
> peer we remove all our retransmissions. On the host side a
> retransmissi...

[Diego Biurrun (diego-biurrun)]

On Thu, Dec 22, 2011 at 10:05:21AM +0000, David Martin wrote:
> On Wed, Dec 21, 2011 at 11:32 AM, Diego Biurrun <email address hidden> wrote:
> > On Tue, Dec 20, 2011 at 11:43:26AM +0000, David Martin wrote:
> >> David Martin has proposed merging lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
> >>
> >> --- hipd/input.c 2011-12-09 09:32:56 +0000
> >> +++ hipd/input.c 2011-12-20 11:41:33 +0000
> >> @@ -1877,3 +1873,117 @@
> >> +
> >> +/**
> >> + * Clear the given retransmission.
> >> + * i.e. set the remaining retransmissions to zero and zero the buffer.
> >> + *
> >> + * @param retrans The retransmission to be cleared.
> >> + */
> >> +void hip_clear_retransmission(struct hip_msg_retrans *const retrans)
> >> +{
> >> + if (!retrans) {
> >> + return;
> >> + }
> >> +
> >> + retrans->count = 0;
> >> + if (retrans->buf) {
> >> + memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);
> >
> > sizeof(retrans->buf) does not work?
>
> Nope. It's just a pointer so that would not make much sense.

Sorry, sizeof(*retrans->buf) of course, I make this mistake all the time :-/

> But even referencing the struct does not seem right:
>
> debug(hipd/input.c:1891@hip_clear_retransmission): HIP_MAX_NETWORK_PACKET: 2048
> debug(hipd/input.c:1892@hip_clear_retransmission): sizeof(retrans->buf): 8
> debug(hipd/input.c:1893@hip_clear_retransmission): sizeof(*retrans->buf): 40

buf is a pointer to struct hip_common, which is

  struct hip_common {
      uint8_t payload_proto;
      uint8_t payload_len;
      uint8_t type_hdr;
      uint8_t ver_res;
      uint16_t checksum;
      uint16_t control;
      struct in6_addr hits; /**< Sender HIT */
      struct in6_addr hitr; /**< Receiver HIT */
  } __attribute__((packed));

so I guess the 40 bytes is about right.

So why 2048? It looks as though you are zeroing way beyond the field
you intend to initialize ...

> >> +/**
> >> + * Update our buffered retransmissions after receiving the given packet.
> >> + * This functions removes invalid retransmissions after a state change caused
> >> + * by a received packet. For example after successfully receiving and handling
> >> + * an I2 or R2 packet all our buffered packets are dropped.
> >> + *
> >> + * Call this function after successful handling of the incoming packet.
> >> + *
> >> + * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
> >> + * @param ha_state The host association state (RFC 5201, 4.4.1.)
> >> + * @param ctx The packet context of the incoming transmission.
> >> + *
> >> + * @return always 0
> >> + */
> >
> > Why always return 0?
>
> Nothing unforeseen may happen that I deem important enough to break
> the handling of packets. I would make it void but the handle functions
> have to return an int.

OK, a short comment explaining this would be nice.

> >> --- hipd/maintenance.c 2011-12-16 13:37:33 +0000
> >> +++ hipd/maintenance.c 2011-12-20 11:41:33 +0000
> >> @@ -92,53 +93,48 @@
> >> static int handle_retransmission(struct hip_hadb_state *entry,
> >> void *current_time)
> >> {
> >> + ...

[David Martin (martin-lp)]

Hi,

On Thu, Dec 22, 2011 at 1:31 PM, Diego Biurrun <email address hidden> wrote:
> On Thu, Dec 22, 2011 at 10:05:21AM +0000, David Martin wrote:
>> On Wed, Dec 21, 2011 at 11:32 AM, Diego Biurrun <email address hidden> wrote:
>> > On Tue, Dec 20, 2011 at 11:43:26AM +0000, David Martin wrote:
>> >> David Martin has proposed merging lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
>> >>
>> >> --- hipd/input.c 2011-12-09 09:32:56 +0000
>> >> +++ hipd/input.c 2011-12-20 11:41:33 +0000
>> >> @@ -1877,3 +1873,117 @@
>> >> +
>> >> +/**
>> >> + * Clear the given retransmission.
>> >> + * i.e. set the remaining retransmissions to zero and zero the buffer.
>> >> + *
>> >> + * @param retrans The retransmission to be cleared.
>> >> + */
>> >> +void hip_clear_retransmission(struct hip_msg_retrans *const retrans)
>> >> +{
>> >> + if (!retrans) {
>> >> + return;
>> >> + }
>> >> +
>> >> + retrans->count = 0;
>> >> + if (retrans->buf) {
>> >> + memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);
>> >
>> > sizeof(retrans->buf) does not work?
>>
>> Nope. It's just a pointer so that would not make much sense.
>
> Sorry, sizeof(*retrans->buf) of course, I make this mistake all the time :-/
>
>> But even referencing the struct does not seem right:
>>
>> debug(hipd/input.c:1891@hip_clear_retransmission): HIP_MAX_NETWORK_PACKET: 2048
>> debug(hipd/input.c:1892@hip_clear_retransmission): sizeof(retrans->buf): 8
>> debug(hipd/input.c:1893@hip_clear_retransmission): sizeof(*retrans->buf): 40
>
> buf is a pointer to struct hip_common, which is
>
> struct hip_common {
> uint8_t payload_proto;
> uint8_t payload_len;
> uint8_t type_hdr;
> uint8_t ver_res;
> uint16_t checksum;
> uint16_t control;
> struct in6_addr hits; /**< Sender HIT */
> struct in6_addr hitr; /**< Receiver HIT */
> } __attribute__((packed));
>
> so I guess the 40 bytes is about right.
>
> So why 2048? It looks as though you are zeroing way beyond the field
> you intend to initialize ...

You are right and wrong. 40 bytes is wrong as this would only be the size of the message header. You have to keep in mind that there's payload as well. You are right as 2048 is the maximum size and a sort of pessimistic approach. I've changed it so that it zeros only the actual message size. The queuing function uses hip_get_msg_total_len() to determine how many bytes to copy into the buffer. I'm using the same now to determine how many bytes to zero. They are the only two functions actually changing the buffer so this is good now.

>> >> +/**
>> >> + * Update our buffered retransmissions after receiving the given packet.
>> >> + * This functions removes invalid retransmissions after a state change caused
>> >> + * by a received packet. For example after successfully receiving and handling
>> >> + * an I2 or R2 packet all our buffered packets are dropped.
>> >> + *
>> >> + * Call this function after successful handling of the incoming packet.
>> >> + *
>> >> + * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
>> >> + * @param ha_state The host association sta...

[David Martin (martin-lp)]

Hi,

On Thu, Dec 22, 2011 at 11:45 AM, Miika Komu <email address hidden> wrote:
> On 21/12/11 12:46, David Martin wrote:
>> On Tue, Dec 20, 2011 at 6:50 PM, Miika Komu<email address hidden> wrote:

>>> 3. Why to buffer to three messages?
>>
>>
>> It's more than one to retransmit the locators during UPDATEs. And
>> three is more or less arbitrarily chosen by me. Two did not seem
>> worth the effort of looping through it and four seems not needed? How
>> many locators are common when using HIPL? On my test setup it's
>> always only two... This can easily be changed by changing
>> HIP_RETRANSMIT_QUEUE_SIZE though.
>
>
> maybe I failed to understand something or should dig into the code better.
> Are you saying that a buffered packet is a) retransmitted three times or b)
> three packets can be queued?
>
> Option a) makes sense, option b) doesn't make sense because the
> retransmitted packets are identical (in most cases, see below).

We are doing option b) in this branch. Well, are they identical? The destination of the buffered U2 packets differ because of the locators so this is not true, no?

>>> Regarding to buffering, the state may change during retransmission
>>> and this causes the implementation to resend old messages in
>>> addition to new ones? If we're lucky, the peer will ignore UPDATE
>>> packets with old SPI numbers. However, the situation may be
>>> different in the BEX because there's no state.
>>
>>
>> I'm not sure whether I fully get your question. You mean for example
>> when you trigger three UPDATEs in a row you would have three
>> retransmissions? Once one of the UPDATEs has been processed the peer
>> would ignore the older retransmissions (check_update_freshness()
>> would fail) and on our side the buffered retransmissions would be
>> removed after receiving the U2 from the peer.
>
>
> Regarding to option b), the packets can be different e.g. with UPDATEs. It
> occurs that a host obtains new locators every now and then. If the host has
> a queue of length of more than two, then it keeps sending an old buffered
> set of locators in addition to the new set. While this is not a big issue,
> it's completely unnecessary and should be avoided IMHO. The easiest way to
> avoid this is to have queue of size one.

Setting the queue to size to one would completely defeat its point though. How often does such a locator change happen? I guess in theory it may happen that an UPDATE is triggered and lost, the host obtains a new locator and another UPDATE is triggered which is lost again. Then you would actually retransmit both UPDATEs with different locators.
A solution may be to remove older buffered U1 packets when a new UPDATE is triggered. I may change that if necessary or desired.

>> Can you trigger another BEX during BEX? Can I force hipd to send
>> another I1 when I'm in state R1_SENT for example? I did not test
>> this.
>
>
> No.

Ok, so we do not have a problem here.

>>> A special (and supported) use case from base exchange: try to
>>> trigger the BEX simultaneously from both sides (i.e. break point
>>> both sides with gdb just after sending I1). I know the state
>>> machine can handle this but what happens to the retransmissions?
>>...

[Miika Komu (miika-iki)]

Hi,

On 12/29/2011 01:01 PM, David Martin wrote:
> Hi,
>
> On Thu, Dec 22, 2011 at 11:45 AM, Miika Komu<email address hidden>
> wrote:
>> On 21/12/11 12:46, David Martin wrote:
>>> On Tue, Dec 20, 2011 at 6:50 PM, Miika Komu<email address hidden>
>>> wrote:
>
>>>> 3. Why to buffer to three messages?
>>>
>>>
>>> It's more than one to retransmit the locators during UPDATEs.
>>> And three is more or less arbitrarily chosen by me. Two did not
>>> seem worth the effort of looping through it and four seems not
>>> needed? How many locators are common when using HIPL? On my test
>>> setup it's always only two... This can easily be changed by
>>> changing HIP_RETRANSMIT_QUEUE_SIZE though.
>>
>>
>> maybe I failed to understand something or should dig into the code
>> better. Are you saying that a buffered packet is a) retransmitted
>> three times or b) three packets can be queued?
>>
>> Option a) makes sense, option b) doesn't make sense because the
>> retransmitted packets are identical (in most cases, see below).
>
> We are doing option b) in this branch. Well, are they identical? The
> destination of the buffered U2 packets differ because of the locators
> so this is not true, no?

the retransmitted packages are different only in exceptional
circumstances. Similarly, the destination is usually the same because
there is one retransmission queue per host association - right?

Note: the destination can actually change with the shotgun extension
because it sends an UPDATE to all known peer addresses.

>>>> Regarding to buffering, the state may change during
>>>> retransmission and this causes the implementation to resend old
>>>> messages in addition to new ones? If we're lucky, the peer will
>>>> ignore UPDATE packets with old SPI numbers. However, the
>>>> situation may be different in the BEX because there's no
>>>> state.
>>>
>>>
>>> I'm not sure whether I fully get your question. You mean for
>>> example when you trigger three UPDATEs in a row you would have
>>> three retransmissions? Once one of the UPDATEs has been processed
>>> the peer would ignore the older retransmissions
>>> (check_update_freshness() would fail) and on our side the
>>> buffered retransmissions would be removed after receiving the U2
>>> from the peer.
>>
>>
>> Regarding to option b), the packets can be different e.g. with
>> UPDATEs. It occurs that a host obtains new locators every now and
>> then. If the host has a queue of length of more than two, then it
>> keeps sending an old buffered set of locators in addition to the
>> new set. While this is not a big issue, it's completely unnecessary
>> and should be avoided IMHO. The easiest way to avoid this is to
>> have queue of size one.
>
> Setting the queue to size to one would completely defeat its point
> though.

What is the point of having a queue size of more than one actually? Or
is that that the buffer size determines the number of retransmissions?

> How often does such a locator change happen?

Seldom by default because hipd reacts to locator changes with an
intended delay to accumulate the changes.

> I guess in
> theory it may happen that an UPDATE is triggered and lost, the host
> obtains a new locator and another UPDA...

[David Martin (martin-lp)]

Hey,

On Thu, Dec 29, 2011 at 12:29 PM, Miika Komu <email address hidden> wrote:
> On 12/29/2011 01:01 PM, David Martin wrote:
>> On Thu, Dec 22, 2011 at 11:45 AM, Miika Komu<email address hidden>
>>> On 21/12/11 12:46, David Martin wrote:
>>>> On Tue, Dec 20, 2011 at 6:50 PM, Miika Komu<email address hidden>

>>>>> 3. Why to buffer to three messages?
>>>>
>>>> It's more than one to retransmit the locators during UPDATEs.
>>>> And three is more or less arbitrarily chosen by me. Two did not
>>>> seem worth the effort of looping through it and four seems not
>>>> needed? How many locators are common when using HIPL? On my test
>>>> setup it's always only two... This can easily be changed by
>>>> changing HIP_RETRANSMIT_QUEUE_SIZE though.
>>>
>>> maybe I failed to understand something or should dig into the code
>>> better. Are you saying that a buffered packet is a) retransmitted
>>> three times or b) three packets can be queued?
>>>
>>> Option a) makes sense, option b) doesn't make sense because the
>>> retransmitted packets are identical (in most cases, see below).
>>
>>
>> We are doing option b) in this branch. Well, are they identical? The
>> destination of the buffered U2 packets differ because of the locators
>> so this is not true, no?
>
> the retransmitted packages are different only in exceptional
> circumstances. Similarly, the destination is usually the same because
> there is one retransmission queue per host association - right?

I'm not sure whether we are talking at cross-purposes right now. :) In principle it's quite simple:
Yes, retransmissions are handled on a per host association basis. At the moment in trunk we have got a single maximum message sized buffer for each host association. When we send a message (which is supposed to be retransmitted when it does not reach its destination) it gets copied to this buffer. Every subsequent message gets copied into the same buffer overwriting the previous one. This is fine for BEX as packets are send on their own and we do not care about previous ones. As far as I understand it it is different for UPDATEs. The first UPDATE packet has got a list of locators. The second UPDATE packet from the peer is send to all of these locators. In the case we send to 3 locators, the first and second transmission would never be retransmitted as they get overwritten by the subsequent ones.

Now what this branch does is instead of having at max a single message per HA buffered we now can buffer up to three (set by a #define and open for discussion which value is best) messages per HA. This means that in case of the second UPDATE packet all transmissions to the different locators are retransmitted and not just the last one. This change for UPDATE retransmissions is the real benefit of increasing the queue size. It does not make a difference for normal BEX.

> Note: the destination can actually change with the shotgun extension because
> it sends an UPDATE to all known peer addresses.

If I see it right in hip_send_pkt() in output.c all packets get transmitted to all known addresses when the shotgun extension is used. This would benefit of a greater queue size as well then as then all packets to the different addresses...

[Miika Komu (miika-iki)]

Thanks, I understood now. Good work!

review: Approve

[Diego Biurrun (diego-biurrun)]

 review abstain

I leave the decision to Miika, who still had some concerns.
Below are some minor issues I noticed; at least the typo
should be addressed before merging.

On Thu, Dec 29, 2011 at 11:15:31AM +0000, David Martin wrote:
> David Martin has proposed merging lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
>
> Requested reviews:
> Diego Biurrun (diego-biurrun)
> Miika Komu (miika-iki)
>
> --- hipd/input.c 2011-12-09 09:32:56 +0000
> +++ hipd/input.c 2011-12-29 11:14:24 +0000
> @@ -1877,3 +1873,121 @@
>
> +/**
> + * Update our buffered retransmissions after receiving the given packet.
> + * This functions removes invalid retransmissions after a state change caused
> + * by a received packet. For example after successfully receiving and handling
> + * an I2 or R2 packet all our buffered packets are dropped.
> + *
> + * Call this function after successful handling of the incoming packet.
> + *
> + * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
> + * @param ha_state The host association state (RFC 5201, 4.4.1.)
> + * @param ctx The packet context of the incoming transmission.
> + *
> + * @return Always 0. Nothing onforeseen may happen here which is important

_u_nforeseen

> --- hipd/maintenance.c 2011-12-21 10:54:26 +0000
> +++ hipd/maintenance.c 2011-12-29 11:14:24 +0000
> @@ -89,55 +90,50 @@
> +static int handle_retransmissions(struct hip_hadb_state *entry,
> + void *current_time)
> {
> +
> + for (i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
> + retrans = &entry->hip_msg_retrans[(entry->next_retrans_slot + i) %
> + HIP_RETRANSMIT_QUEUE_SIZE];
> +
> + /* check if the last transmission was at least RETRANSMIT_WAIT seconds ago */
> + if (*now - HIP_RETRANSMIT_WAIT > retrans->last_transmit) {
> + if (retrans->count > 0) {
> + /* @todo: verify that this works over slow ADSL line */
> + if (hip_send_pkt(&retrans->saddr,
> + &retrans->daddr,
> + entry->nat_mode ? hip_get_local_nat_udp_port() : 0,
> + entry->peer_udp_port,
> + retrans->buf,
> + entry, 0) < 0) {
> + HIP_ERROR("Failed to retransmit packet of type %d.\n",
> + hip_get_msg_type(retrans->buf));
> + err = -1;
> + }
> +
> + /* Set entry state, if previous state was unassociated
> + * and type is I1. */
> + if (!err && hip_get_msg_type(retrans->buf) == HIP_I1 &&
> + entry->state == HIP_STATE_UNASSOCIATED) {
> + HIP_DEBUG("Resent I1 succcesfully\n");
> + entry->state = HIP_STATE_I1_SENT;
> + }

Instead of testing err you could move this into the previous if-block.
That would read more natural to me.

> + retrans->count--;
> + time(&retrans->last_transmit);
> } else {
> + if (hip_get_m...

review: Abstain

[lp:~martin-lp/hipl/hipl_retransmissions]

6234. By David Martin on 2011-12-29

Fix typo in hip_update_retransmissions() doxygen documentation.

6235. By David Martin on 2011-12-29

Make if-conditions in handle_retransmissions() more reasonable.

Do not check for the err value twice and use else if when there is only
a single if condition in the else case.

[David Martin (martin-lp)]

I've fixed the two issues mentioned by Diego in revisions 6234f. Should I start another review or is it good as is?

[Miika Komu (miika-iki)]

In my opinion, go ahead and merge.

[David Martin (martin-lp)]

Ok, done.

=== modified file 'hipd/hadb.c'
--- hipd/hadb.c	2011-12-28 15:52:14 +0000
+++ hipd/hadb.c	2011-12-29 15:06:25 +0000
@@ -693,17 +693,24 @@
     get_random_bytes(&entry->spi_inbound_current,
                      sizeof(entry->spi_inbound_current));
 
-    if (!(entry->hip_msg_retrans.buf = calloc(1, HIP_MAX_NETWORK_PACKET))) {
-        return -ENOMEM;
+    entry->next_retrans_slot = 0;
+    for (unsigned int i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
+        if (!(entry->hip_msg_retrans[i].buf = calloc(1, HIP_MAX_NETWORK_PACKET))) {
+            for (unsigned int j = 0; j < i; j++) {
+                free(entry->hip_msg_retrans[j].buf);
+            }
+            return -ENOMEM;
+        }
+        entry->hip_msg_retrans[i].count = 0;
     }
 
-    entry->hip_msg_retrans.count = 0;
-
     /* Initialize module states */
     entry->hip_modular_state = lmod_init_state();
     if (entry->hip_modular_state == NULL) {
         HIP_ERROR("Failed to initialize modular state.\n");
-        free(entry->hip_msg_retrans.buf);
+        for (unsigned int i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
+            free(entry->hip_msg_retrans[i].buf);
+        }
         return -1;
     }
     lmod_init_state_items(entry->hip_modular_state);
@@ -823,7 +830,9 @@
     /* Delete SAs */
 
     free(ha->dh_shared_key);
-    free(ha->hip_msg_retrans.buf);
+    for (i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
+        free(ha->hip_msg_retrans[i].buf);
+    }
     if (ha->peer_pub) {
         switch (hip_get_host_id_algo(ha->peer_pub)) {
         case HIP_HI_RSA:

=== modified file 'hipd/init.c'
--- hipd/init.c	2011-12-12 14:30:43 +0000
+++ hipd/init.c	2011-12-29 15:06:25 +0000
@@ -561,31 +561,40 @@
 
     hip_register_handle_function(HIP_I1, HIP_STATE_UNASSOCIATED, &hip_check_i1,  20000);
     hip_register_handle_function(HIP_I1, HIP_STATE_UNASSOCIATED, &hip_handle_i1, 30000);
+    hip_register_handle_function(HIP_I1, HIP_STATE_UNASSOCIATED, &hip_update_retransmissions, 35000);
     hip_register_handle_function(HIP_I1, HIP_STATE_UNASSOCIATED, &hip_send_r1,   40000);
     hip_register_handle_function(HIP_I1, HIP_STATE_I1_SENT, &hip_check_i1,  20000);
     hip_register_handle_function(HIP_I1, HIP_STATE_I1_SENT, &hip_handle_i1, 30000);
+    hip_register_handle_function(HIP_I1, HIP_STATE_I1_SENT, &hip_update_retransmissions, 35000);
     hip_register_handle_function(HIP_I1, HIP_STATE_I1_SENT, &hip_send_r1,   40000);
     hip_register_handle_function(HIP_I1, HIP_STATE_I2_SENT, &hip_check_i1,  20000);
     hip_register_handle_function(HIP_I1, HIP_STATE_I2_SENT, &hip_handle_i1, 30000);
+    hip_register_handle_function(HIP_I1, HIP_STATE_I2_SENT, &hip_update_retransmissions, 35000);
     hip_register_handle_function(HIP_I1, HIP_STATE_I2_SENT, &hip_send_r1,   40000);
     hip_register_handle_function(HIP_I1, HIP_STATE_R2_SENT, &hip_check_i1,  20000);
     hip_register_handle_function(HIP_I1, HIP_STATE_R2_SENT, &hip_handle_i1, 30000);
+    hip_register_handle_function(HIP_I1, HIP_STATE_R2_SENT, &hip_update_retransmissions, 35000);
     hip_register_handle_function(HIP_I1, HIP_STATE_R2_SENT, &hip_send_r1,   40000);
     hip_register_handle_function(HIP_I1, HIP_STATE_ESTABLISHED, &hip_check_i1,  20000);
     hip_register_handle_function(HIP_I1, HIP_STATE_ESTABLISHED, &hip_handle_i1, 30000);
+    hip_register_handle_function(HIP_I1, HIP_STATE_ESTABLISHED, &hip_update_retransmissions, 35000);
     hip_register_handle_function(HIP_I1, HIP_STATE_ESTABLISHED, &hip_send_r1,   40000);
     hip_register_handle_function(HIP_I1, HIP_STATE_CLOSING, &hip_check_i1,  20000);
     hip_register_handle_function(HIP_I1, HIP_STATE_CLOSING, &hip_handle_i1, 30000);
+    hip_register_handle_function(HIP_I1, HIP_STATE_CLOSING, &hip_update_retransmissions, 35000);
     hip_register_handle_function(HIP_I1, HIP_STATE_CLOSING, &hip_send_r1,   40000);
     hip_register_handle_function(HIP_I1, HIP_STATE_CLOSED, &hip_check_i1,  20000);
     hip_register_handle_function(HIP_I1, HIP_STATE_CLOSED, &hip_handle_i1, 30000);
+    hip_register_handle_function(HIP_I1, HIP_STATE_CLOSED, &hip_update_retransmissions, 35000);
     hip_register_handle_function(HIP_I1, HIP_STATE_CLOSED, &hip_send_r1,   40000);
     hip_register_handle_function(HIP_I1, HIP_STATE_NONE, &hip_check_i1,  20000);
     hip_register_handle_function(HIP_I1, HIP_STATE_NONE, &hip_handle_i1, 30000);
+    hip_register_handle_function(HIP_I1, HIP_STATE_NONE, &hip_update_retransmissions, 35000);
     hip_register_handle_function(HIP_I1, HIP_STATE_NONE, &hip_send_r1,   40000);
 
     hip_register_handle_function(HIP_I2, HIP_STATE_UNASSOCIATED, &hip_check_i2,  20000);
     hip_register_handle_function(HIP_I2, HIP_STATE_UNASSOCIATED, &hip_handle_i2, 30000);
+    hip_register_handle_function(HIP_I2, HIP_STATE_UNASSOCIATED, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_I2, HIP_STATE_UNASSOCIATED, &hip_setup_ipsec_sa, 30500);
     hip_register_handle_function(HIP_I2, HIP_STATE_UNASSOCIATED, &hip_create_r2, 40000);
     hip_register_handle_function(HIP_I2, HIP_STATE_UNASSOCIATED, &hip_add_rvs_reg_from, 41000);
@@ -594,6 +603,7 @@
     hip_register_handle_function(HIP_I2, HIP_STATE_UNASSOCIATED, &hip_send_r2, 50000);
     hip_register_handle_function(HIP_I2, HIP_STATE_I1_SENT, &hip_check_i2,  20000);
     hip_register_handle_function(HIP_I2, HIP_STATE_I1_SENT, &hip_handle_i2, 30000);
+    hip_register_handle_function(HIP_I2, HIP_STATE_I1_SENT, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_I2, HIP_STATE_I1_SENT, &hip_setup_ipsec_sa, 30500);
     hip_register_handle_function(HIP_I2, HIP_STATE_I1_SENT, &hip_create_r2, 40000);
     hip_register_handle_function(HIP_I2, HIP_STATE_I1_SENT, &hip_add_rvs_reg_from, 41000);
@@ -603,6 +613,7 @@
     hip_register_handle_function(HIP_I2, HIP_STATE_I2_SENT, &hip_check_i2,             20000);
     hip_register_handle_function(HIP_I2, HIP_STATE_I2_SENT, &hip_handle_i2_in_i2_sent, 21000);
     hip_register_handle_function(HIP_I2, HIP_STATE_I2_SENT, &hip_handle_i2,            30000);
+    hip_register_handle_function(HIP_I2, HIP_STATE_I2_SENT, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_I2, HIP_STATE_I2_SENT, &hip_setup_ipsec_sa, 30500);
     hip_register_handle_function(HIP_I2, HIP_STATE_I2_SENT, &hip_create_r2, 40000);
     hip_register_handle_function(HIP_I2, HIP_STATE_I2_SENT, &hip_add_rvs_reg_from, 41000);
@@ -611,6 +622,7 @@
     hip_register_handle_function(HIP_I2, HIP_STATE_I2_SENT, &hip_send_r2, 50000);
     hip_register_handle_function(HIP_I2, HIP_STATE_R2_SENT, &hip_check_i2,  20000);
     hip_register_handle_function(HIP_I2, HIP_STATE_R2_SENT, &hip_handle_i2, 30000);
+    hip_register_handle_function(HIP_I2, HIP_STATE_R2_SENT, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_I2, HIP_STATE_R2_SENT, &hip_setup_ipsec_sa, 30500);
     hip_register_handle_function(HIP_I2, HIP_STATE_R2_SENT, &hip_create_r2, 40000);
     hip_register_handle_function(HIP_I2, HIP_STATE_R2_SENT, &hip_add_rvs_reg_from, 41000);
@@ -619,6 +631,7 @@
     hip_register_handle_function(HIP_I2, HIP_STATE_R2_SENT, &hip_send_r2, 50000);
     hip_register_handle_function(HIP_I2, HIP_STATE_ESTABLISHED, &hip_check_i2,  20000);
     hip_register_handle_function(HIP_I2, HIP_STATE_ESTABLISHED, &hip_handle_i2, 30000);
+    hip_register_handle_function(HIP_I2, HIP_STATE_ESTABLISHED, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_I2, HIP_STATE_ESTABLISHED, &hip_setup_ipsec_sa, 30500);
     hip_register_handle_function(HIP_I2, HIP_STATE_ESTABLISHED, &hip_create_r2, 40000);
     hip_register_handle_function(HIP_I2, HIP_STATE_ESTABLISHED, &hip_add_rvs_reg_from, 41000);
@@ -627,6 +640,7 @@
     hip_register_handle_function(HIP_I2, HIP_STATE_ESTABLISHED, &hip_send_r2, 50000);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSING, &hip_check_i2,  20000);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSING, &hip_handle_i2, 30000);
+    hip_register_handle_function(HIP_I2, HIP_STATE_CLOSING, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSING, &hip_setup_ipsec_sa, 30500);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSING, &hip_create_r2, 40000);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSING, &hip_add_rvs_reg_from, 41000);
@@ -635,6 +649,7 @@
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSING, &hip_send_r2, 50000);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSED, &hip_check_i2,  20000);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSED, &hip_handle_i2, 30000);
+    hip_register_handle_function(HIP_I2, HIP_STATE_CLOSED, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSED, &hip_setup_ipsec_sa, 30500);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSED, &hip_create_r2, 40000);
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSED, &hip_add_rvs_reg_from, 41000);
@@ -643,6 +658,7 @@
     hip_register_handle_function(HIP_I2, HIP_STATE_CLOSED, &hip_send_r2, 50000);
     hip_register_handle_function(HIP_I2, HIP_STATE_NONE, &hip_check_i2,  20000);
     hip_register_handle_function(HIP_I2, HIP_STATE_NONE, &hip_handle_i2, 30000);
+    hip_register_handle_function(HIP_I2, HIP_STATE_NONE, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_I2, HIP_STATE_NONE, &hip_setup_ipsec_sa, 30500);
     hip_register_handle_function(HIP_I2, HIP_STATE_NONE, &hip_create_r2, 40000);
     hip_register_handle_function(HIP_I2, HIP_STATE_NONE, &hip_add_rvs_reg_from, 41000);
@@ -652,6 +668,7 @@
 
     hip_register_handle_function(HIP_R1, HIP_STATE_I1_SENT, &hip_check_r1,  20000);
     hip_register_handle_function(HIP_R1, HIP_STATE_I1_SENT, &hip_handle_r1, 30000);
+    hip_register_handle_function(HIP_R1, HIP_STATE_I1_SENT, &hip_update_retransmissions, 30500);
     hip_register_handle_function(HIP_R1, HIP_STATE_I1_SENT, &hip_build_esp_info, 31000);
     hip_register_handle_function(HIP_R1, HIP_STATE_I1_SENT, &hip_build_solution, 32000);
     hip_register_handle_function(HIP_R1, HIP_STATE_I1_SENT, &hip_handle_diffie_hellman, 33000);
@@ -663,6 +680,7 @@
     hip_register_handle_function(HIP_R1, HIP_STATE_I1_SENT, &hip_send_i2,   50000);
     hip_register_handle_function(HIP_R1, HIP_STATE_I2_SENT, &hip_check_r1,  20000);
     hip_register_handle_function(HIP_R1, HIP_STATE_I2_SENT, &hip_handle_r1, 30000);
+    hip_register_handle_function(HIP_R1, HIP_STATE_I2_SENT, &hip_update_retransmissions, 30500);
     hip_register_handle_function(HIP_R1, HIP_STATE_I2_SENT, &hip_build_esp_info, 31000);
     hip_register_handle_function(HIP_R1, HIP_STATE_I2_SENT, &hip_build_solution, 32000);
     hip_register_handle_function(HIP_R1, HIP_STATE_I2_SENT, &hip_handle_diffie_hellman, 33000);
@@ -674,6 +692,7 @@
     hip_register_handle_function(HIP_R1, HIP_STATE_I2_SENT, &hip_send_i2,   50000);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSING, &hip_check_r1,  20000);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSING, &hip_handle_r1, 30000);
+    hip_register_handle_function(HIP_R1, HIP_STATE_CLOSING, &hip_update_retransmissions, 30500);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSING, &hip_build_esp_info, 31000);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSING, &hip_build_solution, 32000);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSING, &hip_handle_diffie_hellman, 33000);
@@ -685,6 +704,7 @@
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSING, &hip_send_i2,   50000);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSED, &hip_check_r1,  20000);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSED, &hip_handle_r1, 30000);
+    hip_register_handle_function(HIP_R1, HIP_STATE_CLOSED, &hip_update_retransmissions, 30500);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSED, &hip_build_esp_info, 31000);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSED, &hip_build_solution, 32000);
     hip_register_handle_function(HIP_R1, HIP_STATE_CLOSED, &hip_handle_diffie_hellman, 33000);
@@ -697,6 +717,7 @@
 
     hip_register_handle_function(HIP_R2, HIP_STATE_I2_SENT, &hip_check_r2,  20000);
     hip_register_handle_function(HIP_R2, HIP_STATE_I2_SENT, &hip_handle_r2, 30000);
+    hip_register_handle_function(HIP_R2, HIP_STATE_I2_SENT, &hip_update_retransmissions, 30250);
     hip_register_handle_function(HIP_R2, HIP_STATE_I2_SENT, &hip_setup_ipsec_sa, 30500);
 
     hip_register_handle_function(HIP_NOTIFY, HIP_STATE_I1_SENT, &hip_check_notify,  20000);
@@ -712,18 +733,22 @@
     hip_register_handle_function(HIP_NOTIFY, HIP_STATE_CLOSED, &hip_check_notify,  20000);
     hip_register_handle_function(HIP_NOTIFY, HIP_STATE_CLOSED, &hip_handle_notify, 30000);
 
-    hip_register_handle_function(HIP_CLOSE, HIP_STATE_ESTABLISHED,  &hip_close_check_packet,    20000);
-    hip_register_handle_function(HIP_CLOSE, HIP_STATE_ESTABLISHED,  &hip_close_create_response, 30000);
-    hip_register_handle_function(HIP_CLOSE, HIP_STATE_ESTABLISHED,  &hip_close_send_response,   40000);
+    hip_register_handle_function(HIP_CLOSE, HIP_STATE_ESTABLISHED,  &hip_close_check_packet,     20000);
+    hip_register_handle_function(HIP_CLOSE, HIP_STATE_ESTABLISHED,  &hip_update_retransmissions, 25000);
+    hip_register_handle_function(HIP_CLOSE, HIP_STATE_ESTABLISHED,  &hip_close_create_response,  30000);
+    hip_register_handle_function(HIP_CLOSE, HIP_STATE_ESTABLISHED,  &hip_close_send_response,    40000);
 
-    hip_register_handle_function(HIP_CLOSE, HIP_STATE_CLOSING,  &hip_close_check_packet,    20000);
-    hip_register_handle_function(HIP_CLOSE, HIP_STATE_CLOSING,  &hip_close_create_response, 30000);
-    hip_register_handle_function(HIP_CLOSE, HIP_STATE_CLOSING,  &hip_close_send_response,   40000);
+    hip_register_handle_function(HIP_CLOSE, HIP_STATE_CLOSING,  &hip_close_check_packet,     20000);
+    hip_register_handle_function(HIP_CLOSE, HIP_STATE_CLOSING,  &hip_update_retransmissions, 25000);
+    hip_register_handle_function(HIP_CLOSE, HIP_STATE_CLOSING,  &hip_close_create_response,  30000);
+    hip_register_handle_function(HIP_CLOSE, HIP_STATE_CLOSING,  &hip_close_send_response,    40000);
 
     hip_register_handle_function(HIP_CLOSE_ACK, HIP_STATE_CLOSING, &hip_close_ack_check_packet,  20000);
+    hip_register_handle_function(HIP_CLOSE_ACK, HIP_STATE_CLOSING, &hip_update_retransmissions,  25000);
     hip_register_handle_function(HIP_CLOSE_ACK, HIP_STATE_CLOSING, &hip_close_ack_handle_packet, 30000);
 
     hip_register_handle_function(HIP_CLOSE_ACK, HIP_STATE_CLOSED,  &hip_close_ack_check_packet,  20000);
+    hip_register_handle_function(HIP_CLOSE_ACK, HIP_STATE_CLOSED,  &hip_update_retransmissions,  25000);
     hip_register_handle_function(HIP_CLOSE_ACK, HIP_STATE_CLOSED,  &hip_close_ack_handle_packet, 30000);
 
     hip_register_handle_function(HIP_LUPDATE, HIP_STATE_ESTABLISHED, &esp_prot_handle_light_update, 20000);

=== modified file 'hipd/input.c'
--- hipd/input.c	2011-12-09 09:32:56 +0000
+++ hipd/input.c	2011-12-29 15:06:25 +0000
@@ -58,6 +58,7 @@
 #include "lib/core/state.h"
 #include "lib/core/transform.h"
 #include "lib/tool/xfrmapi.h"
+#include "modules/update/hipd/update.h"
 #include "config.h"
 #include "cookie.h"
 #include "dh.h"
@@ -1113,8 +1114,6 @@
     hip_perf_stop_benchmark(perf_set, PERF_BASE);
     hip_perf_write_benchmark(perf_set, PERF_BASE);
 #endif
-    ctx->hadb_entry->hip_msg_retrans.count = 0;
-    memset(ctx->hadb_entry->hip_msg_retrans.buf, 0, HIP_MAX_NETWORK_PACKET);
 
     if (ctx->hadb_entry->state == HIP_STATE_ESTABLISHED) {
         HIP_DEBUG("Send response to firewall.\n");
@@ -1695,9 +1694,6 @@
     ctx->hadb_entry->state = HIP_STATE_ESTABLISHED;
     HIP_INFO("Reached %s state\n", hip_state_str(ctx->hadb_entry->state));
 
-    ctx->hadb_entry->hip_msg_retrans.count = 0;
-    memset(ctx->hadb_entry->hip_msg_retrans.buf, 0, HIP_MAX_NETWORK_PACKET);
-
 out_err:
     if (err) {
         ctx->error = err;
@@ -1877,3 +1873,121 @@
 
     return err;
 }
+
+/**
+ * Clear the given retransmission.
+ * i.e. set the remaining retransmissions to zero and zero the buffer.
+ *
+ * @param retrans The retransmission to be cleared.
+ */
+void hip_clear_retransmission(struct hip_msg_retrans *const retrans)
+{
+    if (!retrans) {
+        return;
+    }
+
+    retrans->count = 0;
+    if (retrans->buf) {
+        memset(retrans->buf, 0, hip_get_msg_total_len(retrans->buf));
+    }
+}
+
+/**
+ * Decide whether the given UPDATE retransmission is still valid with regard to
+ * the incoming message. For example a buffered U1 packet is invalidated when a
+ * fresh U2 comes in.
+ *
+ * @param ctx     The packet context of the incoming transmission.
+ * @param retrans The retransmission in question.
+ * @return True  if given retransmission is invalid.
+ *         False else.
+ */
+static bool update_retrans_is_invalid(struct hip_packet_context *const ctx,
+                                      struct hip_msg_retrans *const retrans)
+{
+    if (!ctx || !retrans) {
+        return false;
+    }
+
+    switch (hip_classify_update_type(ctx->input_msg)) {
+    case SECOND_UPDATE_PACKET:
+        return hip_classify_update_type(retrans->buf) == FIRST_UPDATE_PACKET;
+
+    case THIRD_UPDATE_PACKET:
+        return hip_classify_update_type(retrans->buf) == SECOND_UPDATE_PACKET;
+
+    default:
+        return false;
+    }
+}
+
+/**
+ * Update our buffered retransmissions after receiving the given packet.
+ * This functions removes invalid retransmissions after a state change caused
+ * by a received packet. For example after successfully receiving and handling
+ * an I2 or R2 packet all our buffered packets are dropped.
+ *
+ * Call this function after successful handling of the incoming packet.
+ *
+ * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
+ * @param ha_state    The host association state (RFC 5201, 4.4.1.)
+ * @param ctx         The packet context of the incoming transmission.
+ *
+ * @return Always 0. Nothing unforeseen may happen here which is important
+ *         enough to break the packet processing. Unhandled incoming message
+ *         types are simply ignored. Handle functions have to return an int
+ *         though so always 0 it is.
+ */
+int hip_update_retransmissions(UNUSED const uint8_t packet_type,
+                               UNUSED const enum hip_state ha_state,
+                               struct hip_packet_context *const ctx)
+{
+    struct hip_msg_retrans *retrans;
+
+    HIP_ASSERT(ctx);
+    HIP_ASSERT(ctx->input_msg);
+
+    /* When receiving an I1 there usually is no established state yet. */
+    if (!ctx->hadb_entry) {
+        return 0;
+    }
+
+    for (unsigned int i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
+        retrans = &ctx->hadb_entry->hip_msg_retrans[i];
+
+        if (retrans->count == 0) {
+            continue;
+        }
+
+        switch (hip_get_msg_type(ctx->input_msg)) {
+        case HIP_I1:
+            /* We only remove our sent I1s when we have got a greater HIT
+             * than the peer (ref. RFC 5201, 4.4.2). */
+            if (hip_get_msg_type(retrans->buf) == HIP_I1 &&
+                hip_hit_is_bigger(&ctx->hadb_entry->hit_our, &ctx->hadb_entry->hit_peer)) {
+                hip_clear_retransmission(retrans);
+            }
+            break;
+
+        case HIP_I2:
+        case HIP_R1:
+        case HIP_R2:
+        case HIP_CLOSE:
+        case HIP_CLOSE_ACK:
+            hip_clear_retransmission(retrans);
+            break;
+
+        case HIP_UPDATE:
+            if (update_retrans_is_invalid(ctx, retrans)) {
+                hip_clear_retransmission(retrans);
+            }
+            break;
+
+        /* We do not act on other message types like Light UPDATE for example. */
+        default:
+            break;
+        }
+    }
+
+    return 0;
+}

=== modified file 'hipd/input.h'
--- hipd/input.h	2011-11-25 17:56:24 +0000
+++ hipd/input.h	2011-12-29 15:06:25 +0000
@@ -127,4 +127,10 @@
                               UNUSED const enum hip_state ha_state,
                               struct hip_packet_context *ctx);
 
+void hip_clear_retransmission(struct hip_msg_retrans *const retrans);
+
+int hip_update_retransmissions(UNUSED const uint8_t packet_type,
+                               UNUSED const enum hip_state ha_state,
+                               struct hip_packet_context *const ctx);
+
 #endif /* HIPL_HIPD_INPUT_H */

=== modified file 'hipd/maintenance.c'
--- hipd/maintenance.c	2011-12-21 10:54:26 +0000
+++ hipd/maintenance.c	2011-12-29 15:06:25 +0000
@@ -60,6 +60,7 @@
 #include "hidb.h"
 #include "hipd.h"
 #include "init.h"
+#include "input.h"
 #include "output.h"
 #include "maintenance.h"
 
@@ -89,55 +90,48 @@
  * @param current_time current time
  * @return zero on success or negative on failure
  */
-static int handle_retransmission(struct hip_hadb_state *entry,
-                                 void *current_time)
+static int handle_retransmissions(struct hip_hadb_state *entry,
+                                  void *current_time)
 {
-    int     err = 0;
-    time_t *now = current_time;
-
-    if (entry->hip_msg_retrans.count == 0) {
-        goto out_err;
-    }
-
-    /* check if the last transmission was at least RETRANSMIT_WAIT seconds ago */
-    if (*now - HIP_RETRANSMIT_WAIT > entry->hip_msg_retrans.last_transmit) {
-        if (entry->hip_msg_retrans.count > 0  &&
-            ((entry->state != HIP_STATE_ESTABLISHED && entry->retrans_state != entry->state) ||
-             entry->light_update_retrans == 1)) {
-            HIP_DEBUG("state=%d, retrans_state=%d\n", entry->state, entry->retrans_state);
-
-            /* @todo: verify that this works over slow ADSL line */
-            err = hip_send_pkt(&entry->hip_msg_retrans.saddr,
-                               &entry->hip_msg_retrans.daddr,
-                               entry->nat_mode ? hip_get_local_nat_udp_port() : 0,
-                               entry->peer_udp_port,
-                               entry->hip_msg_retrans.buf,
-                               entry, 0);
-
-            /* Set entry state, if previous state was unassociated
-             * and type is I1. */
-            if (!err && hip_get_msg_type(entry->hip_msg_retrans.buf)
-                == HIP_I1 && entry->state == HIP_STATE_UNASSOCIATED) {
-                HIP_DEBUG("Resent I1 succcesfully\n");
-                entry->state = HIP_STATE_I1_SENT;
-            }
-
-            entry->hip_msg_retrans.count--;
-            time(&entry->hip_msg_retrans.last_transmit);
-        } else {
-            entry->hip_msg_retrans.count = 0;
-            memset(entry->hip_msg_retrans.buf, 0, HIP_MAX_NETWORK_PACKET);
-
-            if (entry->state == HIP_STATE_ESTABLISHED) {
-                entry->retrans_state = HIP_STATE_NONE;
-            } else {
-                entry->retrans_state = entry->state;
+    int                     err = 0, i = 0;
+    struct hip_msg_retrans *retrans;
+    time_t                 *now = current_time;
+
+    for (i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
+        retrans = &entry->hip_msg_retrans[(entry->next_retrans_slot + i) %
+                                          HIP_RETRANSMIT_QUEUE_SIZE];
+
+        /* check if the last transmission was at least RETRANSMIT_WAIT seconds ago */
+        if (*now - HIP_RETRANSMIT_WAIT > retrans->last_transmit) {
+            if (retrans->count > 0) {
+                /* @todo: verify that this works over slow ADSL line */
+                if (hip_send_pkt(&retrans->saddr,
+                                 &retrans->daddr,
+                                 entry->nat_mode ? hip_get_local_nat_udp_port() : 0,
+                                 entry->peer_udp_port,
+                                 retrans->buf,
+                                 entry, 0) == 0) {
+                    /* Set entry state, if previous state was unassociated
+                     * and type is I1. */
+                    if (hip_get_msg_type(retrans->buf) == HIP_I1 &&
+                        entry->state == HIP_STATE_UNASSOCIATED) {
+                        HIP_DEBUG("Resent I1 succcesfully\n");
+                        entry->state = HIP_STATE_I1_SENT;
+                    }
+                } else {
+                    HIP_ERROR("Failed to retransmit packet of type %d.\n",
+                              hip_get_msg_type(retrans->buf));
+                    err = -1;
+                }
+
+                retrans->count--;
+                time(&retrans->last_transmit);
+            } else if (hip_get_msg_type(retrans->buf)) {
+                hip_clear_retransmission(retrans);
             }
         }
     }
 
-out_err:
-
     return err;
 }
 
@@ -151,7 +145,7 @@
     time_t current_time;
     time(&current_time);
 
-    if (hip_for_each_ha(handle_retransmission, &current_time)) {
+    if (hip_for_each_ha(handle_retransmissions, &current_time)) {
         return -1;
     }
     return 0;

=== modified file 'hipd/netdev.c'
--- hipd/netdev.c	2011-12-09 09:32:56 +0000
+++ hipd/netdev.c	2011-12-29 15:06:25 +0000
@@ -921,12 +921,15 @@
 
 send_i1:
 
-    if (entry->hip_msg_retrans.count == 0) {
-        HIP_DEBUG("Expired retransmissions, sending i1\n");
-    } else {
-        HIP_DEBUG("I1 was already sent, ignoring\n");
-        goto out_err;
+    HIP_DEBUG("Checking for older queued I1 transmissions.\n");
+    for (unsigned int i = 0; i < HIP_RETRANSMIT_QUEUE_SIZE; i++) {
+        if (entry->hip_msg_retrans[i].count > 0
+            && hip_get_msg_type(entry->hip_msg_retrans[i].buf) == HIP_I1) {
+            HIP_DEBUG("I1 was already sent, ignoring\n");
+            goto out_err;
+        }
     }
+    HIP_DEBUG("Expired retransmissions, sending i1\n");
 
     is_ipv4_locator = IN6_IS_ADDR_V4MAPPED(&entry->peer_addr);
 

=== modified file 'hipd/output.c'
--- hipd/output.c	2011-11-10 10:53:21 +0000
+++ hipd/output.c	2011-12-29 15:06:25 +0000
@@ -1066,7 +1066,7 @@
     err = hip_send_pkt(&ctx->dst_addr, &ctx->src_addr,
                        ctx->hadb_entry->nat_mode ? hip_get_local_nat_udp_port() : 0,
                        ctx->hadb_entry->peer_udp_port, ctx->output_msg,
-                       ctx->hadb_entry, 1);
+                       ctx->hadb_entry, 0);
 
     HIP_IFEL(err, -ECOMM, "Sending R2 packet failed.\n");
 
@@ -1111,29 +1111,30 @@
  *                  destination HITs.
  * @param entry     a pointer to the current host association database state.
  * @return          zero
- * @note currently the queue length is one and new packets replace old ones
  */
 static int queue_packet(const struct in6_addr *src_addr,
                         const struct in6_addr *peer_addr,
                         const struct hip_common *msg,
                         struct hip_hadb_state *entry)
 {
-    int len = hip_get_msg_total_len(msg);
+    struct hip_msg_retrans *retrans;
 
     if (!entry) {
         return 0;
     }
 
+    retrans = &entry->hip_msg_retrans[entry->next_retrans_slot];
+
     /* Not reusing the old entry as the new packet may have different length */
-    memset(entry->hip_msg_retrans.buf, 0, HIP_MAX_NETWORK_PACKET);
-
-    memcpy(entry->hip_msg_retrans.buf, msg, len);
-    memcpy(&entry->hip_msg_retrans.saddr, src_addr,
-           sizeof(struct in6_addr));
-    memcpy(&entry->hip_msg_retrans.daddr, peer_addr,
-           sizeof(struct in6_addr));
-    entry->hip_msg_retrans.count = HIP_RETRANSMIT_MAX;
-    time(&entry->hip_msg_retrans.last_transmit);
+    memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);
+
+    memcpy(retrans->buf, msg, hip_get_msg_total_len(msg));
+    ipv6_addr_copy(&retrans->saddr, src_addr);
+    ipv6_addr_copy(&retrans->daddr, peer_addr);
+    retrans->count = HIP_RETRANSMIT_MAX;
+    time(&retrans->last_transmit);
+
+    entry->next_retrans_slot = (entry->next_retrans_slot + 1) % HIP_RETRANSMIT_QUEUE_SIZE;
 
     return 0;
 }

=== modified file 'lib/core/state.h'
--- lib/core/state.h	2011-12-16 13:37:33 +0000
+++ lib/core/state.h	2011-12-29 15:06:25 +0000
@@ -100,6 +100,9 @@
     HIP_HA_STATE_VALID   = 1,
 };
 
+/* The maximum number of retransmissions to queue. */
+#define HIP_RETRANSMIT_QUEUE_SIZE 3
+
 /**
  * A data structure for handling retransmission. Used inside host association
  * database entries.
@@ -157,9 +160,6 @@
     int purge_timeout;
     /** The state of this host association. */
     enum hip_state state;
-    /** This guarantees that retransmissions work properly also in
-     *  non-established state.*/
-    enum hip_state retrans_state;
     /** Our control values related to this host association.
      *  @see hip_ha_controls */
     hip_controls local_controls;
@@ -282,8 +282,11 @@
     unsigned char echo_data[4];
 
     HIP_HASHTABLE *peer_addr_list_to_be_added;
+    /** The slot in our struct hip_msg_retrans array which will be used to save
+     *  the next retransmission. */
+    unsigned int next_retrans_slot;
     /** For storing retransmission related data. */
-    struct hip_msg_retrans hip_msg_retrans;
+    struct hip_msg_retrans hip_msg_retrans[HIP_RETRANSMIT_QUEUE_SIZE];
     /** peer hostname */
     uint8_t peer_hostname[HIP_HOST_ID_HOSTNAME_LEN_MAX];
     /** Counters of heartbeats (ICMPv6s) */

=== modified file 'modules/update/hipd/update.c'
--- modules/update/hipd/update.c	2011-12-16 13:37:33 +0000
+++ modules/update/hipd/update.c	2011-12-29 15:06:25 +0000
@@ -364,7 +364,7 @@
                            ctx->hadb_entry->peer_udp_port,
                            ctx->output_msg,
                            ctx->hadb_entry,
-                           1);
+                           0);
     }
 
     return err;
@@ -810,6 +810,11 @@
              -1, "Error on registering UPDATE handle function.\n");
     HIP_IFEL(hip_register_handle_function(HIP_UPDATE,
                                           HIP_STATE_R2_SENT,
+                                          &hip_update_retransmissions,
+                                          20150),
+             -1, "Error on registering UPDATE handle function.\n");
+    HIP_IFEL(hip_register_handle_function(HIP_UPDATE,
+                                          HIP_STATE_R2_SENT,
                                           &prepare_update_response,
                                           20200),
              -1, "Error on registering UPDATE handle function.\n");
@@ -896,6 +901,11 @@
              -1, "Error on registering UPDATE handle function.\n");
     HIP_IFEL(hip_register_handle_function(HIP_UPDATE,
                                           HIP_STATE_ESTABLISHED,
+                                          &hip_update_retransmissions,
+                                          20150),
+             -1, "Error on registering UPDATE handle function.\n");
+    HIP_IFEL(hip_register_handle_function(HIP_UPDATE,
+                                          HIP_STATE_ESTABLISHED,
                                           &prepare_update_response,
                                           20200),
              -1, "Error on registering UPDATE handle function.\n");