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

[Toke Høiland-Jørgensen]

dxld at darkboxed.org writes:

> Hi Toke,
>
> On Tue, Feb 28, 2023 at 12:20:22PM +0100, Toke Høiland-Jørgensen wrote:
>> > 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 :)
>
> Hmm, I suppose that is a valid use-case. I'd rather expose a boolean flag
> for that rather than the smoothed metric itself.

No strong opinion on this, I just generally skew towards exposing things
for people to do interesting things with :)

>> > 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()?
>
> I think you're looking at this from the wrong angle. By not having a
> smoothed metric attribute we have to update to begin with we save on all
> this timer logic that would constantly re-announce routes to core. This way
> the number of core announcements stays exactly the same as before and we
> don't really loose anything.

Right, sure, that's a nice property, but I'm not actually sure how what
you sketched out above accomplishes that?

>> > 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.
>
> Simpler, yes, but I do want to be able to maintain internet scale routing
> tables through babel eventually so slashing every little bit helps :)

Heh, yeah, I would like to eventually be able to do that as well, but I
think there are other optimisations we need to do first. For instance,
walking the entire routing table every second is not going to work in
the first place in this case :)

>> 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.
>
> I feel like that's an artifact specific to the "metric smoothing" approach
> to route dampening not a feature though. The way I see it the behaviour we
> really want is to delay any change in selected route for a time related to
> the metric difference.
>
> Think back to what the purpose of the metric smoothing is in the first
> place: to limit oscillations of the selected route, which this will do just
> as well.

I'm OK with finding another solution, but I think you're going to have
to explain in more detail how what you propose actually represents such
a solution, then :)

>> >> 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 ;)
>
> I don't agree with that. It's not as if I want per-hop information. Just a
> sum of RTTs along the path and a sum of administrative metric along the
> path rather than have those jumbled together into one number.
>
> Since babel is quite flexible in the actual metric math that would allow
> interesting ways of weighing each metric component rather than just having
> everything be linear.

It also introduces dependencies, though. I.e., with the current approach
you can have a subset of the routers speak the RTT extension, and other
parts of the network will just have that incorporated into the metric.
Whereas if it is carried as a separate metric your entire network has to
know about the extension for it to be useful.

> For debugging this would be useful as you can see that this path in front
> of you actually has a crazy RTT rather than someone just having fiddled
> with their rxcost.

Meh, not convinced that the routing protocol is the right place to get
such debugging information. I'd rather just monitor the actual traffic :)

>
>> > Also see above for how "lovely" all the necessary lookups are going to
>> > be if we go down this path, haha.
>
> Whoops, that was from an earlier draft. This was about what it would look
> like if we update the smoothed metric in rte_better. Something like this:
>
>   struct babel_proto
>     *p_new = (struct babel_proto*)new->src->proto,
>     *p_old = (struct babel_proto*)old->src->proto;
>   struct babel_iface *ifa_new, *ifa_old;
>   struct babel_neighbor *nbr_new, *nbr_old;
>   struct babel_entry *ent_new, *ent_old;
>   struct babel_route *route_new, *route_old;
>
>   ifa_new = babel_find_iface(p_new, new->attrs->nh.iface);
>   ifa_old = babel_find_iface(p_old, old->attrs->nh.iface);
>
>   nbr_new = babel_find_neighbor(ifa_new, new->attrs->from);
>   nbr_old = babel_find_neighbor(ifa_old, old->attrs->from);
>
>   ent_new = babel_find_entry(p_new, new->net->n.addr);
>   ent_old = babel_find_entry(p_old, old->net->n.addr);
>
>   route_new = babel_find_route(ent_new, nbr_new);
>   route_old = babel_find_route(ent_old, nbr_old);
>
>   babel_update_smoothed_metric(p_new, route_new);
>   babel_update_smoothed_metric(p_old, route_old);
>
> yikes. Don't want to go down that road, got enough of these lookups in
> rt_notify already :)

Right, but then we do need to put the smoothed metric into an attribute
if it's to be used in the comparison. But maybe you can explain how
that's not really need cf the above.

-Toke