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

[Daniel Gröber]

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.

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

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?

> 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.

> 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.

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

--Daniel