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

[Ondrej Zajicek]

On Wed, Apr 04, 2018 at 05:12:47PM +0200, Toke Høiland-Jørgensen wrote:
> Ondrej Zajicek <santiago at crfreenet.org> writes:
> > 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.

I think that these are independent issues.

For unicast messages, you could have per-neighbor msg_queue list, which
should work in the same way how per-iface msg_queue list works - TLVs are
enqueued to appropriate list(s), send_event is triggered and its hook
packs and sends packets. The only difference is whether per-iface
multicast or (multiple) per-neighbor unicast queue is used.

So perhaps these should be independent patches / patch series.

The acked retraction code then could use the same sending path
(msg_queue and send_event) as the regular triggered update code.


> > 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 :)

I think fixing this could be problematic with the current design of the patch,
esp. keeping the whole sequence of babel_msg_nodes in babel_ack_sequence.
IMHO it would be better to handle these retractions in similar manner as
triggered updates. Not keep sent TLVs and in case of retransmissions due
to ack timeout just regenerate the list by table walk.


-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santiago at crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."