[PATCH] babel: Use acknowledged retractions when losing a prefix

[Toke Høiland-Jørgensen]

Ondrej Zajicek <santiago at crfreenet.org> writes:

> On Thu, Mar 08, 2018 at 10:25:56PM +0100, Toke Høiland-Jørgensen wrote:
>> Toke Høiland-Jørgensen <toke at toke.dk> writes:
>> 
>> > In order to prevent routing loops, Babel installs temporary blackhole
>> > routes to prefixes that expire or are otherwise lost. These blackhole
>> > routes are maintained for a while to ensure the route has been flushed
>> > from the whole network; the default hold time is 64 seconds.
>> >
>> > In the updated IETF version of Babel (rfc6126bis), an alternative
>> > procedure was added which replaces this hold time with an explicit
>> > acknowledgement procedure. This procedure simply consists of sending a
>> > retraction along with an acknowledgement request to all neighbours, and
>> > removing the blackhole route once all neighbours have acknowledged the
>> > retraction. As this usually happens pretty quickly, the hold time during
>> > which the route is blackholed is almost completely avoided.
>> >
>> > This patch implements the above behaviour. It adds a generic facility to
>> > the Babel protocol which allows for sending sequences of acknowledgement
>> > requests to a number of neighbours, and executing a callback function
>> > once all the requests have been ACKed. This facility is then used to
>> > implement the acknowledgement scheme when a route is lost.
>> >
>> > Signed-off-by: Toke Høiland-Jørgensen <toke at toke.dk>
>> 
>> Did anyone have a chance to look at this? :)
>
> Hi
>
> Finally, i get some time to read through the patch. I have some questions:
>
> 1) Why the retractions with ACK req are sent as unicast? IMHO the
> specification allows both unicast and multicast, so it makes sense
> to me send retractions as multicast like before (but with ACK req
> TLV).

Well, I eventually want to support sending everything over unicast to
better support links with no or limited multicast support; but maybe I
was getting ahead of myself here, and implementing this to work over
multicast as well is a good idea. Or maybe I should just do both in the
same patch series. I'll take a look at that.

> 2) If i understand it correctly, now each retracted route is sent in
> separate packet instead of being packed together?
>
> 3) If i understand it correctly, the sequence is not properly split if it does
> not fit to one packet. Currently it is not a big issue due to 2).

Yup; I was planning to implement batching and packet splitting as part
of the general unicast handling.

> 4) What happens if the route becames reachable again before ack sequence
> for retraction is processed? i guess it will retransmit the retractions
> again, which does not make sense. IMHO router should announce the current
> state, not the obsolete one.

Good point; I'll fix that :)


Thank you for taking a look; I'll return with an updated version once I
get some more Bird hacking time...

-Toke