Merge "Move arp device check out of loop"
This commit is contained in:
commit
bbcaed7f34
@ -53,8 +53,10 @@ class AgentMixin(object):
|
|||||||
ip = arp_table['ip_address']
|
ip = arp_table['ip_address']
|
||||||
mac = arp_table['mac_address']
|
mac = arp_table['mac_address']
|
||||||
subnet_id = arp_table['subnet_id']
|
subnet_id = arp_table['subnet_id']
|
||||||
|
device, device_exists = ri.get_arp_related_dev(subnet_id)
|
||||||
ri._update_arp_entry(ip, mac, subnet_id, action)
|
ri._update_arp_entry(ip, mac, subnet_id, action,
|
||||||
|
device,
|
||||||
|
device_exists=device_exists)
|
||||||
|
|
||||||
def add_arp_entry(self, context, payload):
|
def add_arp_entry(self, context, payload):
|
||||||
"""Add arp entry into router namespace. Called from RPC."""
|
"""Add arp entry into router namespace. Called from RPC."""
|
||||||
|
@ -223,12 +223,15 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
|
|||||||
def _process_arp_cache_for_internal_port(self, subnet_id):
|
def _process_arp_cache_for_internal_port(self, subnet_id):
|
||||||
"""Function to process the cached arp entries."""
|
"""Function to process the cached arp entries."""
|
||||||
arp_remove = set()
|
arp_remove = set()
|
||||||
|
device, device_exists = self.get_arp_related_dev(subnet_id)
|
||||||
for arp_entry in self._pending_arp_set:
|
for arp_entry in self._pending_arp_set:
|
||||||
if subnet_id == arp_entry.subnet_id:
|
if subnet_id == arp_entry.subnet_id:
|
||||||
try:
|
try:
|
||||||
state = self._update_arp_entry(
|
state = self._update_arp_entry(
|
||||||
arp_entry.ip, arp_entry.mac,
|
arp_entry.ip, arp_entry.mac,
|
||||||
arp_entry.subnet_id, arp_entry.operation)
|
arp_entry.subnet_id, arp_entry.operation,
|
||||||
|
device=device,
|
||||||
|
device_exists=device_exists)
|
||||||
except Exception:
|
except Exception:
|
||||||
state = False
|
state = False
|
||||||
if state:
|
if state:
|
||||||
@ -246,18 +249,13 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
|
|||||||
arp_delete.add(arp_entry)
|
arp_delete.add(arp_entry)
|
||||||
self._pending_arp_set -= arp_delete
|
self._pending_arp_set -= arp_delete
|
||||||
|
|
||||||
def _update_arp_entry(self, ip, mac, subnet_id, operation):
|
def _update_arp_entry(
|
||||||
|
self, ip, mac, subnet_id, operation, device,
|
||||||
|
device_exists=True):
|
||||||
"""Add or delete arp entry into router namespace for the subnet."""
|
"""Add or delete arp entry into router namespace for the subnet."""
|
||||||
port = self._get_internal_port(subnet_id)
|
|
||||||
# update arp entry only if the subnet is attached to the router
|
|
||||||
if not port:
|
|
||||||
return False
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# TODO(mrsmith): optimize the calls below for bulk calls
|
if device_exists:
|
||||||
interface_name = self.get_internal_device_name(port['id'])
|
|
||||||
device = ip_lib.IPDevice(interface_name, namespace=self.ns_name)
|
|
||||||
if device.exists():
|
|
||||||
if operation == 'add':
|
if operation == 'add':
|
||||||
device.neigh.add(ip, mac)
|
device.neigh.add(ip, mac)
|
||||||
elif operation == 'delete':
|
elif operation == 'delete':
|
||||||
@ -276,6 +274,16 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
|
|||||||
with excutils.save_and_reraise_exception():
|
with excutils.save_and_reraise_exception():
|
||||||
LOG.exception("DVR: Failed updating arp entry")
|
LOG.exception("DVR: Failed updating arp entry")
|
||||||
|
|
||||||
|
def get_arp_related_dev(self, subnet_id):
|
||||||
|
port = self._get_internal_port(subnet_id)
|
||||||
|
# update arp entry only if the subnet is attached to the router
|
||||||
|
if not port:
|
||||||
|
return None, False
|
||||||
|
interface_name = self.get_internal_device_name(port['id'])
|
||||||
|
device = ip_lib.IPDevice(interface_name, namespace=self.ns_name)
|
||||||
|
device_exists = device.exists()
|
||||||
|
return device, device_exists
|
||||||
|
|
||||||
def _set_subnet_arp_info(self, subnet_id):
|
def _set_subnet_arp_info(self, subnet_id):
|
||||||
"""Set ARP info retrieved from Plugin for existing ports."""
|
"""Set ARP info retrieved from Plugin for existing ports."""
|
||||||
# TODO(Carl) Can we eliminate the need to make this RPC while
|
# TODO(Carl) Can we eliminate the need to make this RPC while
|
||||||
@ -284,6 +292,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
|
|||||||
ignored_device_owners = (
|
ignored_device_owners = (
|
||||||
lib_constants.ROUTER_INTERFACE_OWNERS +
|
lib_constants.ROUTER_INTERFACE_OWNERS +
|
||||||
tuple(common_utils.get_dvr_allowed_address_pair_device_owners()))
|
tuple(common_utils.get_dvr_allowed_address_pair_device_owners()))
|
||||||
|
device, device_exists = self.get_arp_related_dev(subnet_id)
|
||||||
|
|
||||||
for p in subnet_ports:
|
for p in subnet_ports:
|
||||||
if p['device_owner'] not in ignored_device_owners:
|
if p['device_owner'] not in ignored_device_owners:
|
||||||
@ -291,7 +300,9 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
|
|||||||
self._update_arp_entry(fixed_ip['ip_address'],
|
self._update_arp_entry(fixed_ip['ip_address'],
|
||||||
p['mac_address'],
|
p['mac_address'],
|
||||||
subnet_id,
|
subnet_id,
|
||||||
'add')
|
'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
|
@staticmethod
|
||||||
@ -525,10 +536,14 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
|
|||||||
self.enable_snat_redirect_rules(ex_gw_port)
|
self.enable_snat_redirect_rules(ex_gw_port)
|
||||||
for port in self.get_snat_interfaces():
|
for port in self.get_snat_interfaces():
|
||||||
for ip in port['fixed_ips']:
|
for ip in port['fixed_ips']:
|
||||||
|
subnet_id = ip['subnet_id']
|
||||||
|
device, device_exists = self.get_arp_related_dev(subnet_id)
|
||||||
self._update_arp_entry(ip['ip_address'],
|
self._update_arp_entry(ip['ip_address'],
|
||||||
port['mac_address'],
|
port['mac_address'],
|
||||||
ip['subnet_id'],
|
subnet_id,
|
||||||
'add')
|
'add',
|
||||||
|
device=device,
|
||||||
|
device_exists=device_exists)
|
||||||
|
|
||||||
def external_gateway_updated(self, ex_gw_port, interface_name):
|
def external_gateway_updated(self, ex_gw_port, interface_name):
|
||||||
pass
|
pass
|
||||||
|
@ -561,14 +561,14 @@ class TestDvrRouterOperations(base.BaseTestCase):
|
|||||||
payload = {'arp_table': arp_table, 'router_id': router['id']}
|
payload = {'arp_table': arp_table, 'router_id': router['id']}
|
||||||
agent.add_arp_entry(None, payload)
|
agent.add_arp_entry(None, payload)
|
||||||
|
|
||||||
def test__update_arp_entry_with_no_subnet(self):
|
def test_get_arp_related_dev_no_subnet(self):
|
||||||
self._set_ri_kwargs(mock.sentinel.agent,
|
self._set_ri_kwargs(mock.sentinel.agent,
|
||||||
'foo_router_id',
|
'foo_router_id',
|
||||||
{'distributed': True, 'gw_port_host': HOSTNAME})
|
{'distributed': True, 'gw_port_host': HOSTNAME})
|
||||||
ri = dvr_router.DvrLocalRouter(HOSTNAME, **self.ri_kwargs)
|
ri = dvr_router.DvrLocalRouter(HOSTNAME, **self.ri_kwargs)
|
||||||
ri.get_internal_device_name = mock.Mock()
|
with mock.patch('neutron.agent.linux.ip_lib.IPDevice') as f:
|
||||||
ri._update_arp_entry(mock.ANY, mock.ANY, 'foo_subnet_id', 'add')
|
ri.get_arp_related_dev('foo_subnet_id')
|
||||||
self.assertFalse(ri.get_internal_device_name.call_count)
|
self.assertFalse(f.call_count)
|
||||||
|
|
||||||
def _setup_test_for_arp_entry_cache(self):
|
def _setup_test_for_arp_entry_cache(self):
|
||||||
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
|
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
|
||||||
@ -585,9 +585,9 @@ class TestDvrRouterOperations(base.BaseTestCase):
|
|||||||
state = True
|
state = True
|
||||||
with mock.patch('neutron.agent.linux.ip_lib.IPDevice') as rtrdev,\
|
with mock.patch('neutron.agent.linux.ip_lib.IPDevice') as rtrdev,\
|
||||||
mock.patch.object(ri, '_cache_arp_entry') as arp_cache:
|
mock.patch.object(ri, '_cache_arp_entry') as arp_cache:
|
||||||
rtrdev.return_value.exists.return_value = False
|
|
||||||
state = ri._update_arp_entry(
|
state = ri._update_arp_entry(
|
||||||
mock.ANY, mock.ANY, subnet_id, 'add')
|
mock.ANY, mock.ANY, subnet_id, 'add',
|
||||||
|
mock.ANY, device_exists=False)
|
||||||
self.assertFalse(state)
|
self.assertFalse(state)
|
||||||
self.assertTrue(arp_cache.called)
|
self.assertTrue(arp_cache.called)
|
||||||
arp_cache.assert_called_once_with(mock.ANY, mock.ANY,
|
arp_cache.assert_called_once_with(mock.ANY, mock.ANY,
|
||||||
|
Loading…
Reference in New Issue
Block a user