From 2499d3bdc6007308bf282cb44462990a4cd03b0e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 19 Dec 2017 10:10:15 +0100 Subject: [PATCH] core: ensure that the default route-metric bumps at most 50 points First check that the limit of 50 metric points is not surpassed. Otherwise, if you have an ethernet device (aspired 100, effective 130) and a MACSec devic (aspired 125, effective 155), activating a new ethernet device would bump it's metric to 155 -- more then the 50 points limit. It doesn't matter too much, because the cases where the limit of 50 could have been surpassed were very specific. Still, change it to ensure that the limit is always honored as one would expect. Fixes: 6a32c64d8fb2a9c1cfb78ab7e2f0bb3a269c81d7 --- src/nm-manager.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 1bdd4e5591..7161e87ca9 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -473,7 +473,11 @@ initited: /* unfortunately, there is no stright forward way to lookup all reserved metrics. * Note, that we don't only have to know which metrics are currently reserved, * but also, which metrics are now seemingly un-used but caused another reserved - * metric to be bumped. Hence, the naive O(n^2) search :( */ + * metric to be bumped. Hence, the naive O(n^2) search :( + * + * Well, technically, since we limit bumping the metric to 50, this entire + * loop runs at most 50 times, so it's still O(n). Let's just say, it's not + * very efficient. */ again: g_hash_table_iter_init (&h_iter, priv->device_route_metrics); while (g_hash_table_iter_next (&h_iter, NULL, (gpointer *) &d2)) { @@ -489,24 +493,30 @@ again: g_hash_table_iter_remove (&h_iter); continue; } - data->effective_metric = d2->effective_metric; - if (data->effective_metric == G_MAXUINT32) { - /* we cannot bump any further. Done. */ + + if (d2->effective_metric == G_MAXUINT32) { + /* we cannot bump the metric any further. Done. + * + * Actually, this can currently not happen because the aspired_metric + * are small numbers and we limit the bumping to 50. Still, for + * completeness... */ + data->effective_metric = G_MAXUINT32; break; } - if (data->effective_metric - data->aspired_metric > 50) { + if (d2->effective_metric - data->aspired_metric >= 50) { /* as one active interface reserves an entire range of metrics * (from aspired_metric to effective_metric), that means if you * alternatingly activate two interfaces, their metric will - * juggle up. + * bump each other. * - * Limit this, don't bump the metric more then 50 times. */ + * Limit this, bump the metric at most 50 points. */ + data->effective_metric = data->aspired_metric + 50; break; } /* bump the metric, and search again. */ - data->effective_metric++; + data->effective_metric = d2->effective_metric + 1; goto again; }