[PATCH 0/3] babel: Add support for the RTT extension

[Toke Høiland-Jørgensen]

Daniel Gröber <dxld at darkboxed.org> writes:

> Hi Toke,
>
> On Mon, Feb 27, 2023 at 12:16:01PM +0100, Toke Høiland-Jørgensen wrote:
>> >> - Add the smoothed metric as a new route attribute (so it's also
>> >>   available to filters)
>> >
>> > I think doing that is a bad idea. If we keep filters from changing this we
>> > might just be able to optimize by only announcing smooted metric updates
>> > when the resulting route would actually be better than the currently
>> > selected one.
>> >
>> > If we let filters meddle with this however that becomes impossible. Unless
>> > you were thinking of a r/o attribute?
>> 
>> I meant read-only. I agree that the filters shouldn't be able to modify
>> the smoothed metric. 
>
> I've thought about this some more, I think we absolutely shouldn't expose
> the smooted metric to filters. It's an implementation detail. There's a
> bunch of other valid ways to implement this sort of dampening/hysteresis no
> reason to expose it to filter users really.
>
> How would you even use this for any practical filtering anyway? Reject
> routes based on `smoothed_metric - metric > threshold` or even better,
> increase metric based on smoothed_metric difference (oscillations galore
> haha). Hardly seems useful to me :)
>
> AFAICT it's perfectly fine to have EAs that filters can't access though,
> that's the situation we're currently in for the router-id after all so
> that's not really a blocker.

My thinking was that filters may want to do something like:

if (metric == smoothed_metric)
  metric += 100; /* route is stable, we can apply our policy */

but I honestly don't know if that's useful for anything in reality :)

> I've been playing with rebasing your smoothing patch on mine now and I see
> a couple of options. My favorite so far is strategically replacing relevant
> babel_announce_rte calls with this:
>
>     static void
>     babel_sched_announce_rte(struct babel_proto *p, struct babel_route *r)
>     {
>       struct babel_config *cf = (void *) p->p.cf;
>     
>       if (!cf->metric_decay || !e->selected ||
>           e->selected->metric < r->metric)
>           /* When best route's metric is less than the one we're about to announce
>            * it's safe to do so immediately even if metric smoothing is on. Note
>            * e->selected takes export filters into account. */
>         babel_announce_rte(p, r->e, r);
>       else
>         e->scheduled = r;
>     }
>
> The clever bit is that announcing a route that's no better than the
> selected one will have no impact on route selection. Same applies if
> another protocol's route is active (i.e. when e->selected is NULL).

Why is it better to do this pre-filtering instead of just having the
smoothed metric be part of the check in babel_rte_better()?

> Only think I'm still missing is a way to reframe the metric smoothing
> math such that I can install a timer for when the next scheduled route
> update should go through, though perhaps polling in the global timer
> hook is fine?

I think it's probably simpler to just re-announce any route that's still
converging every time we go through the routing table. Bear in mind that
the currently selected route can also be converging, so predicting when
two routes "cross" gets complicated quickly. Simpler to just do a
periodic update and redo the comparison every time this update happens.

>> Another question is if the smoothing should incorporate any changes to
>> the metric that the filters do? It probably should, right?
>
> I don't think that'll give us very useful behaviour TBH. Perhaps the way to
> look at it is that we're not trying to smooth the (administrative) metric
> but rather only the link cost component which is the bit that is actually
> fluctuating? Problem is we only have these as seperate numbers for our
> local metrics/costs not the ones from other nodes.
>
> Maybe we need a protocol extension to seperate these two concepts, hmm. I
> have been thinking it would be nice to have seperate rtt and administrative
> metric numbers just for network debugging, perhaps it would make sense for
> general operation too.

I'm not sure this is a good idea from a protocol perspective. If you
want a global view of the network, you should really be running a
link-state protocol ;)

>> That may be OK, but I'm not actually sure how the nest comparison works
>> (when we announce a new route, does the rte_better function get called
>> pairwise against every other candidate with the same prefix, or just
>> against the current best?).
>
> From adding some quick tracing code it certainly looks like the
> comparison is only against the current best. There's only ever a
> couple of rte_better calls per prefix once things are initialized.

Right, that's what I thought, thanks for confirming.

> Also see above for how "lovely" all the necessary lookups are going to
> be if we go down this path, haha.

What path? :)

-Toke