Merge "[DVR] Set arp entries only for IPs from the correct subnet"

This commit is contained in:
Zuul 2021-11-02 03:24:50 +00:00 committed by Gerrit Code Review
commit 81f8524527
5 changed files with 43 additions and 27 deletions

View File

@ -42,7 +42,7 @@ class DvrEdgeHaRouter(dvr_edge_router.DvrEdgeRouter,
router_info.RouterInfo.internal_network_added(self, port)
for subnet in port['subnets']:
self._set_subnet_arp_info(subnet['id'])
self._set_subnet_arp_info(subnet)
self._snat_redirect_add_from_port(port)
if not self.get_ex_gw_port() or not self._is_this_snat_host():

View File

@ -346,52 +346,56 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
device_exists = device.exists()
return device, device_exists
def _set_subnet_arp_info(self, subnet_id):
def _set_subnet_arp_info(self, subnet):
"""Set ARP info retrieved from Plugin for existing ports."""
# TODO(Carl) Can we eliminate the need to make this RPC while
# processing a router.
subnet_ports = self.agent.get_ports_by_subnet(subnet_id)
subnet_ports = self.agent.get_ports_by_subnet(subnet['id'])
ignored_device_owners = (
lib_constants.ROUTER_INTERFACE_OWNERS +
tuple(common_utils.get_dvr_allowed_address_pair_device_owners()))
device, device_exists = self.get_arp_related_dev(subnet_id)
device, device_exists = self.get_arp_related_dev(subnet['id'])
subnet_ip_version = netaddr.IPNetwork(subnet['cidr']).version
for p in subnet_ports:
if p['device_owner'] not in ignored_device_owners:
for fixed_ip in p['fixed_ips']:
self._update_arp_entry(fixed_ip['ip_address'],
p['mac_address'],
subnet_id,
'add',
device=device,
device_exists=device_exists)
if fixed_ip['subnet_id'] == subnet['id']:
self._update_arp_entry(fixed_ip['ip_address'],
p['mac_address'],
subnet['id'],
'add',
device=device,
device_exists=device_exists)
for allowed_address_pair in p.get('allowed_address_pairs', []):
if ('/' not in str(allowed_address_pair['ip_address']) or
common_utils.is_cidr_host(
allowed_address_pair['ip_address'])):
ip_address = common_utils.cidr_to_ip(
allowed_address_pair['ip_address'])
self._update_arp_entry(
ip_address,
allowed_address_pair['mac_address'],
subnet_id,
'add',
device=device,
device_exists=device_exists)
ip_version = common_utils.get_ip_version(ip_address)
if ip_version == subnet_ip_version:
self._update_arp_entry(
ip_address,
allowed_address_pair['mac_address'],
subnet['id'],
'add',
device=device,
device_exists=device_exists)
# subnet_ports does not have snat port if the port is still unbound
# by the time this function is called. So ensure to add arp entry
# for snat port if port details are updated in router info.
for p in self.get_snat_interfaces():
for fixed_ip in p['fixed_ips']:
if fixed_ip['subnet_id'] == subnet_id:
if fixed_ip['subnet_id'] == subnet['id']:
self._update_arp_entry(fixed_ip['ip_address'],
p['mac_address'],
subnet_id,
subnet['id'],
'add',
device=device,
device_exists=device_exists)
self._process_arp_cache_for_internal_port(subnet_id)
self._process_arp_cache_for_internal_port(subnet['id'])
@staticmethod
def _get_snat_idx(ip_cidr):
@ -508,7 +512,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
# external_gateway port or the agent_mode.
ex_gw_port = self.get_ex_gw_port()
for subnet in port['subnets']:
self._set_subnet_arp_info(subnet['id'])
self._set_subnet_arp_info(subnet)
if ex_gw_port:
# Check for address_scopes here if gateway exists.
address_scopes_match = self._check_if_address_scopes_match(

View File

@ -1039,8 +1039,11 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
router_info = self.generate_dvr_router_info(enable_snat=True)
expected_neighbors = ['35.4.1.10', '10.0.0.10', '10.200.0.3']
allowed_address_net = netaddr.IPNetwork('10.100.0.0/30')
subnet_id = router_info['_interfaces'][0]['fixed_ips'][0]['subnet_id']
port_data = {
'fixed_ips': [{'ip_address': expected_neighbors[0]}],
'fixed_ips': [
{'ip_address': expected_neighbors[0],
'subnet_id': subnet_id}],
'mac_address': 'fa:3e:aa:bb:cc:dd',
'device_owner': DEVICE_OWNER_COMPUTE,
'allowed_address_pairs': [

View File

@ -692,7 +692,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
ri._add_interface_route_to_fip_ns = mock.Mock()
ri.internal_network_added(port)
self.assertEqual(2, ri._internal_network_added.call_count)
ri._set_subnet_arp_info.assert_called_once_with(subnet_id)
ri._set_subnet_arp_info.assert_called_once_with({'id': subnet_id})
ri._internal_network_added.assert_called_with(
dvr_snat_ns.SnatNamespace.get_snat_ns_name(ri.router['id']),
sn_port['network_id'],

View File

@ -559,10 +559,19 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri = dvr_router.DvrLocalRouter(HOSTNAME, **self.ri_kwargs)
ports = ri.router.get(lib_constants.INTERFACE_KEY, [])
subnet_id = l3_test_common.get_subnet_id(ports[0])
subnet = {
'id': subnet_id,
'cidr': '1.2.3.0/24'
}
ri.router['_snat_router_interfaces'] = [{
'mac_address': 'fa:16:3e:80:8d:80',
'fixed_ips': [{'subnet_id': subnet_id,
'ip_address': '1.2.3.10'}]}]
'fixed_ips': [
{'subnet_id': subnet_id,
'ip_address': '1.2.3.10'},
{'subnet_id': _uuid(),
'ip_address': '2001:db8::1'}
]
}]
test_ports = [{'mac_address': '00:11:22:33:44:55',
'device_owner': lib_constants.DEVICE_OWNER_DHCP,
@ -591,7 +600,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
'cidr': '1.2.3.0/24'}]
with mock.patch.object(ri,
'_process_arp_cache_for_internal_port') as parp:
ri._set_subnet_arp_info(subnet_id)
ri._set_subnet_arp_info(subnet)
self.assertEqual(1, parp.call_count)
self.mock_ip_dev.neigh.add.assert_has_calls([
mock.call('1.2.3.4', '00:11:22:33:44:55'),
@ -600,7 +609,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
# Test negative case
router['distributed'] = False
ri._set_subnet_arp_info(subnet_id)
ri._set_subnet_arp_info(subnet)
self.mock_ip_dev.neigh.add.never_called()
def test_add_arp_entry(self):