0
0
mirror of https://gitlab.nic.cz/labs/bird.git synced 2024-12-22 09:41:54 +00:00

Nest: Improve export counter handling

One of previous workarounds for phantom route avoidance breaks export
counters by expanding sending of spurious withdraws, which are send when
we are not sure whether we have advertised that routes in the past.
If not, then export counter is decreased, but it was not increased
before, so it overflows under zero.

The patch fixes that by sendung spurious withdraws, but not counting them
on export counter. That may lead to error in the other direction, but that
happens only as a race condition (i.e., in normal operation filters
return proper values about old route export state).
This commit is contained in:
Ondrej Zajicek (work) 2019-02-05 19:00:43 +01:00
parent 52fdd1cb76
commit 6e8fb66859

View File

@ -564,34 +564,47 @@ rt_notify_basic(struct channel *c, net *net, rte *new0, rte *old0, int refeed)
* and the end of refeed - if a newly filtered route disappears during this * and the end of refeed - if a newly filtered route disappears during this
* period, proper withdraw is not sent (because old would be also filtered) * period, proper withdraw is not sent (because old would be also filtered)
* and the route is not refeeded (because it disappeared before that). * and the route is not refeeded (because it disappeared before that).
* Therefore, we also do not try to run the filter on old routes that are * This is handled below as a special case.
* older than the last filter change.
*/ */
if (new) if (new)
new = export_filter(c, new, &new_free, 0); new = export_filter(c, new, &new_free, 0);
if (old && !(refeed || ((old->lastmod <= c->last_tx_filter_change) && if (old && !refeed)
(p != old->sender->proto))))
old = export_filter(c, old, &old_free, 1); old = export_filter(c, old, &old_free, 1);
if (!new && !old) if (!new && !old)
{ {
/* /*
* As mentioned above, 'old' value may be incorrect in some race conditions. * As mentioned above, 'old' value may be incorrect in some race conditions.
* We generally ignore it with the exception of withdraw to pipe protocol. * We generally ignore it with two exceptions:
* In that case we rather propagate unfiltered withdraws regardless of *
* export filters to ensure that when a protocol is flushed, its routes are * First, withdraw to pipe protocol. In that case we rather propagate
* removed from all tables. Possible spurious unfiltered withdraws are not * unfiltered withdraws regardless of export filters to ensure that when a
* problem here as they are ignored if there is no corresponding route at * protocol is flushed, its routes are removed from all tables. Possible
* the other end of the pipe. We directly call rt_notify() hook instead of * spurious unfiltered withdraws are not problem here as they are ignored if
* there is no corresponding route at the other end of the pipe.
*
* Second, recent filter change. If old route is older than filter change,
* then it was previously evaluated by a different filter and we do not know
* whether it was really propagated. In that case we rather send spurious
* withdraw than do nothing and possibly cause phantom routes.
*
* In both cases wqe directly call rt_notify() hook instead of
* do_rt_notify() to avoid logging and stat counters. * do_rt_notify() to avoid logging and stat counters.
*/ */
int pipe_withdraw = 0, filter_change = 0;
#ifdef CONFIG_PIPE #ifdef CONFIG_PIPE
if ((p->proto == &proto_pipe) && !new0 && (p != old0->sender->proto)) pipe_withdraw = (p->proto == &proto_pipe) && !new0;
p->rt_notify(p, c, net, NULL, old0);
#endif #endif
filter_change = old0 && (old0->lastmod <= c->last_tx_filter_change);
if ((pipe_withdraw || filter_change) && (p != old0->sender->proto))
{
c->stats.exp_withdraws_accepted++;
p->rt_notify(p, c, net, NULL, old0);
}
return; return;
} }