Merge "conman: Remove temporary connection_check routes."
diff --git a/conman/interface.py b/conman/interface.py
index 53fe0eb..2d0fbbe 100755
--- a/conman/interface.py
+++ b/conman/interface.py
@@ -17,7 +17,6 @@
METRIC_5GHZ = 20
METRIC_24GHZ_5GHZ = 21
METRIC_24GHZ = 22
-METRIC_TEMPORARY_CONNECTION_CHECK = 99
RFC2385_MULTICAST_ROUTE = '239.0.0.0/8'
@@ -83,21 +82,7 @@
' (ACS)' if check_acs else '', self.name)
return False
- # Temporarily add a route to make sure the connection check can be run.
- # Give it a high metric so that it won't interfere with normal default
- # routes.
- added_temporary_route = False
- if 'default' not in self.current_routes():
- logging.debug('Adding temporary connection check routes for dev %s',
- self.name)
- self._ip_route('add', self._gateway_ip,
- 'dev', self.name,
- 'metric', str(METRIC_TEMPORARY_CONNECTION_CHECK))
- self._ip_route('add', 'default',
- 'via', self._gateway_ip,
- 'dev', self.name,
- 'metric', str(METRIC_TEMPORARY_CONNECTION_CHECK))
- added_temporary_route = True
+ self.add_routes()
cmd = [self.CONNECTION_CHECK, '-I', self.name]
if check_acs:
@@ -110,17 +95,6 @@
self.name,
'passed' if result else 'failed')
- # Delete the temporary route.
- if added_temporary_route:
- logging.debug('Deleting temporary connection check routes for dev %s',
- self.name)
- self._ip_route('del', 'default',
- 'dev', self.name,
- 'metric', str(METRIC_TEMPORARY_CONNECTION_CHECK))
- self._ip_route('del', self._gateway_ip,
- 'dev', self.name,
- 'metric', str(METRIC_TEMPORARY_CONNECTION_CHECK))
-
return result
def gateway(self):
@@ -147,10 +121,6 @@
logging.info('Cannot add route for %s without a metric.', self.name)
return
- if self._gateway_ip is None:
- logging.info('Cannot add route for %s without a gateway IP.', self.name)
- return
-
# If the current routes are the same, there is nothing to do. If either
# exists but is different, delete it before adding an updated one.
current = self.current_routes()
@@ -158,21 +128,28 @@
to_add = []
subnet = current.get('subnet', {})
- if (self._subnet and
- (subnet.get('route', None), subnet.get('metric', None)) !=
- (self._subnet, str(self.metric))):
- logging.debug('Adding subnet route for dev %s', self.name)
- to_add.append(('subnet', ('add', self._subnet, 'dev', self.name,
- 'metric', str(self.metric))))
- subnet = self._subnet
+ if self._subnet:
+ if ((subnet.get('route', None), subnet.get('metric', None)) !=
+ (self._subnet, str(self.metric))):
+ logging.debug('Adding subnet route for dev %s', self.name)
+ to_add.append(('subnet', ('add', self._subnet, 'dev', self.name,
+ 'metric', str(self.metric))))
+ subnet = self._subnet
+ else:
+ subnet = None
+ self.delete_route('default', 'subnet')
default = current.get('default', {})
- if (subnet and
- (default.get('via', None), default.get('metric', None)) !=
- (self._gateway_ip, str(self.metric))):
- logging.debug('Adding default route for dev %s', self.name)
- to_add.append(('default', ('add', 'default', 'via', self._gateway_ip,
- 'dev', self.name, 'metric', str(self.metric))))
+ if self._gateway_ip:
+ if (subnet and
+ (default.get('via', None), default.get('metric', None)) !=
+ (self._gateway_ip, str(self.metric))):
+ logging.debug('Adding default route for dev %s', self.name)
+ to_add.append(('default',
+ ('add', 'default', 'via', self._gateway_ip,
+ 'dev', self.name, 'metric', str(self.metric))))
+ else:
+ self.delete_route('default')
# RFC2365 multicast route.
if current.get('multicast', {}).get('metric', None) != str(self.metric):
@@ -248,17 +225,17 @@
' '.join(self.IP_ROUTE), ' '.join(args))
return ''
- return self._really_ip_route(*args)
-
- def _really_ip_route(self, *args):
try:
logging.debug('%s calling ip route %s', self.name, ' '.join(args))
- return subprocess.check_output(self.IP_ROUTE + list(args))
+ return self._really_ip_route(*args)
except subprocess.CalledProcessError as e:
logging.error('Failed to call "ip route" with args %r: %s', args,
e.message)
return ''
+ def _really_ip_route(self, *args):
+ return subprocess.check_output(self.IP_ROUTE + list(args))
+
def _ip_addr_show(self):
try:
return subprocess.check_output(self.IP_ADDR_SHOW + [self.name])
diff --git a/conman/interface_test.py b/conman/interface_test.py
index 45714a8..79c58ae 100755
--- a/conman/interface_test.py
+++ b/conman/interface_test.py
@@ -7,6 +7,7 @@
import shutil
import socket
import struct
+import subprocess
import tempfile
import time
@@ -40,6 +41,13 @@
self.routing_table = {}
self.ip_testonly = None
+ def _connection_check(self, *args, **kwargs):
+ result = super(FakeInterfaceMixin, self)._connection_check(*args, **kwargs)
+ if (self.current_routes().get('default', {}).get('via', None) !=
+ self._gateway_ip):
+ return False
+ return result
+
def set_connection_check_result(self, result):
if result in ['succeed', 'fail', 'restricted']:
# pylint: disable=invalid-name
@@ -87,8 +95,9 @@
key = (self.name, route, metric)
if args[0] == 'add' and key not in self.routing_table:
if not can_add_route():
- raise ValueError('Tried to add default route without subnet route: %r',
- self.routing_table)
+ raise subprocess.CalledProcessError(
+ 'Tried to add default route without subnet route: %r',
+ self.routing_table)
logging.debug('Adding route for %r', key)
self.routing_table[key] = ' '.join(args[1:])
elif args[0] == 'del':
@@ -431,8 +440,40 @@
b.ip_testonly = '192.168.1.100'
wvtest.WVPASSEQ(b.get_ip_address(), '192.168.1.100')
+ # Get a new gateway/subnet (e.g. due to joining a new network).
# Not on the subnet; adding IP should fail.
- wvtest.WVEXCEPT(ValueError, b.set_gateway_ip, '192.168.2.1')
+ b.set_gateway_ip('192.168.2.1')
+ wvtest.WVFAIL('default' in b.current_routes())
+ wvtest.WVPASS('subnet' in b.current_routes())
+ # Without a default route, the connection check should fail.
+ wvtest.WVFAIL(b.acs())
+
+ # Now we get the subnet and should add updated subnet and gateway routes.
+ b.set_subnet('192.168.2.0/24')
+ wvtest.WVPASSEQ(b.current_routes()['default']['via'], '192.168.2.1')
+ wvtest.WVPASSLE(int(b.current_routes()['default']['metric']), 50)
+ wvtest.WVPASSEQ(b.current_routes()['subnet']['route'], '192.168.2.0/24')
+ wvtest.WVPASSLE(int(b.current_routes()['subnet']['metric']), 50)
+ wvtest.WVPASS(b.acs())
+
+ # If we have no subnet, make sure that both subnet and default routes are
+ # removed.
+ b.set_subnet(None)
+ wvtest.WVFAIL('subnet' in b.current_routes())
+ wvtest.WVFAIL('default' in b.current_routes())
+
+ # Now repeat the new-network test, but with a faulty connection. Make sure
+ # the metrics are set appropriately.
+ b.set_connection_check_result('fail')
+ b.set_subnet('192.168.3.0/24')
+ b.set_gateway_ip('192.168.3.1')
+ wvtest.WVPASSGE(int(b.current_routes()['default']['metric']), 50)
+ wvtest.WVPASSGE(int(b.current_routes()['subnet']['metric']), 50)
+
+ # Now test deleting only the gateway IP.
+ b.set_gateway_ip(None)
+ wvtest.WVPASS('subnet' in b.current_routes())
+ wvtest.WVFAIL('default' in b.current_routes())
finally:
shutil.rmtree(tmp_dir)
@@ -536,6 +577,7 @@
b = Bridge('br0', '10', acs_autoprovisioning_filepath=autoprov_filepath)
b.add_moca_station(0)
b.set_gateway_ip('192.168.1.1')
+ b.set_subnet('192.168.1.0/24')
b.set_connection_check_result('succeed')
b.initialize()
