From 69bb8307eb03255ad60fa20ba5d8a148bd726e91 Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Tue, 21 Feb 2023 10:56:54 -0700 Subject: [PATCH] network: do not accept route metric > 20000 Network manager can create routes at metric aka priority above 20000. These can stick around if they are not the best choice, or they may disappear quickly. Do not consider one of these routes as a valid default route for has_network purposes. --- subiquitycore/controllers/network.py | 13 ++++++- .../controllers/tests/test_network.py | 39 ++++++++++++++++++- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py index d2aac0da..6a578278 100644 --- a/subiquitycore/controllers/network.py +++ b/subiquitycore/controllers/network.py @@ -91,8 +91,17 @@ class SubiquityNetworkEventReceiver(NetworkEventReceiver): self.controller.update_has_default_route(self.has_default_route) def _default_route_exists(self, routes): - return any(route['table'] == 254 and not route['dst'] - for route in routes) + for route in routes: + if int(route['table']) != 254: + continue + if route['dst']: + continue + if int(route['priority']) >= 20000: + # network manager probes routes by creating one at 20000 + + # the real metric, but those aren't necessarily valid. + continue + return True + return False def probe_default_routes(self): with pyroute2.NDB() as ndb: diff --git a/subiquitycore/controllers/tests/test_network.py b/subiquitycore/controllers/tests/test_network.py index e8970af6..aec6d17e 100644 --- a/subiquitycore/controllers/tests/test_network.py +++ b/subiquitycore/controllers/tests/test_network.py @@ -34,12 +34,13 @@ class TestRoutes(unittest.IsolatedAsyncioTestCase): "ifname": "ens3", "dst": "", "dst_len": 0, + "priority": 100, "gateway": "10.0.2.2" }] self.assertTrue(self.er._default_route_exists(routes)) - def test_one_good_one_other(self): + def test_mix(self): routes = [{ "target": "localhost", "tflags": 0, @@ -47,6 +48,7 @@ class TestRoutes(unittest.IsolatedAsyncioTestCase): "ifname": "ens3", "dst": "", "dst_len": 0, + "priority": 100, "gateway": "10.0.2.2" }, { "target": "localhost", @@ -55,6 +57,25 @@ class TestRoutes(unittest.IsolatedAsyncioTestCase): "ifname": "ens3", "dst": "10.0.2.0", "dst_len": 24, + "priority": 100, + "gateway": None + }, { + "target": "localhost", + "tflags": 0, + "table": 255, + "ifname": "ens3", + "dst": "10.0.2.0", + "dst_len": 24, + "priority": 100, + "gateway": None + }, { + "target": "localhost", + "tflags": 0, + "table": 254, + "ifname": "ens3", + "dst": "10.0.2.0", + "dst_len": 24, + "priority": 20100, "gateway": None }] @@ -68,6 +89,7 @@ class TestRoutes(unittest.IsolatedAsyncioTestCase): "ifname": "ens3", "dst": "10.0.2.0", "dst_len": 24, + "priority": 100, "gateway": None }] @@ -81,6 +103,21 @@ class TestRoutes(unittest.IsolatedAsyncioTestCase): "ifname": "ens3", "dst": "", "dst_len": 0, + "priority": 100, + "gateway": "10.0.2.2" + }] + + self.assertFalse(self.er._default_route_exists(routes)) + + def test_wrong_priority(self): + routes = [{ + "target": "localhost", + "tflags": 0, + "table": 254, + "ifname": "ens3", + "dst": "", + "dst_len": 0, + "priority": 20100, "gateway": "10.0.2.2" }]