openstack/neutron
Code Issues Proposed changes

Revert "DVR: Fix allowed_address_pair IP, ARP table update by neutron agent"

Browse Source 
This reverts commit fbe308bdc1.

This does not help the ARP update for the unbound Allowed-address-pair
IP, since the temporary ARP update (NUD: reachable) goes to incomplete
state when the router tries to re-ARP for the IP, before it responds to
a VM, since DVR routers does not allow the ARP requests to flow through
the br-tun.

Closes-bug: #1773999

Change-Id: I9977c8cbbbc1e68565249e7f80c59319fe967300
This commit is contained in:
Swaminathan Vasudevan 2018-06-04 16:57:00 +00:00 committed by Brian Haley
parent fbe308bdc1
commit f98f239a15
7 changed files with 17 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
neutron/agent/l3/dvr.py
View File

@ -53,8 +53,8 @@ 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']
nud_state = arp_table.get('nud_state')
ri._update_arp_entry(ip, mac, subnet_id, action, nud_state=nud_state) ri._update_arp_entry(ip, mac, subnet_id, action)
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."""

9
neutron/agent/l3/dvr_local_router.py
View File

@ -239,8 +239,7 @@ 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( def _update_arp_entry(self, ip, mac, subnet_id, operation):
self, ip, mac, subnet_id, operation, nud_state='permanent'):
"""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) port = self._get_internal_port(subnet_id)
# update arp entry only if the subnet is attached to the router # update arp entry only if the subnet is attached to the router
@ -253,7 +252,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
device = ip_lib.IPDevice(interface_name, namespace=self.ns_name) device = ip_lib.IPDevice(interface_name, namespace=self.ns_name)
if device.exists(): if device.exists():
if operation == 'add': if operation == 'add':
device.neigh.add(ip, mac, nud_state=nud_state) device.neigh.add(ip, mac)
elif operation == 'delete': elif operation == 'delete':
device.neigh.delete(ip, mac) device.neigh.delete(ip, mac)
return True return True
@ -280,14 +279,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
tuple(common_utils.get_dvr_allowed_address_pair_device_owners())) tuple(common_utils.get_dvr_allowed_address_pair_device_owners()))
for p in subnet_ports: for p in subnet_ports:
nud_state = 'permanent' if p.get('device_owner') else 'reachable'
if p['device_owner'] not in ignored_device_owners: if p['device_owner'] not in ignored_device_owners:
for fixed_ip in p['fixed_ips']: for fixed_ip in p['fixed_ips']:
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')
nud_state=nud_state)
self._process_arp_cache_for_internal_port(subnet_id) self._process_arp_cache_for_internal_port(subnet_id)
@staticmethod @staticmethod

12
neutron/db/l3_dvr_db.py
View File

@ -892,7 +892,7 @@ class _DVRAgentInterfaceMixin(object):
return f_port return f_port
def _generate_arp_table_and_notify_agent( def _generate_arp_table_and_notify_agent(
self, context, fixed_ip, mac_address, notifier, nud_state='permanent'): self, context, fixed_ip, mac_address, notifier):
"""Generates the arp table entry and notifies the l3 agent.""" """Generates the arp table entry and notifies the l3 agent."""
ip_address = fixed_ip['ip_address'] ip_address = fixed_ip['ip_address']
subnet = fixed_ip['subnet_id'] subnet = fixed_ip['subnet_id']
@ -904,8 +904,7 @@ class _DVRAgentInterfaceMixin(object):
return return
arp_table = {'ip_address': ip_address, arp_table = {'ip_address': ip_address,
'mac_address': mac_address, 'mac_address': mac_address,
'subnet_id': subnet, 'subnet_id': subnet}
'nud_state': nud_state}
notifier(context, router_id, arp_table) notifier(context, router_id, arp_table)
def _get_subnet_id_for_given_fixed_ip( def _get_subnet_id_for_given_fixed_ip(
@ -949,14 +948,11 @@ class _DVRAgentInterfaceMixin(object):
return return
allowed_address_pair_fixed_ips = ( allowed_address_pair_fixed_ips = (
self._get_allowed_address_pair_fixed_ips(context, port_dict)) self._get_allowed_address_pair_fixed_ips(context, port_dict))
for fixed_ip in fixed_ips: changed_fixed_ips = fixed_ips + allowed_address_pair_fixed_ips
for fixed_ip in changed_fixed_ips:
self._generate_arp_table_and_notify_agent( self._generate_arp_table_and_notify_agent(
context, fixed_ip, port_dict['mac_address'], context, fixed_ip, port_dict['mac_address'],
self.l3_rpc_notifier.add_arp_entry) self.l3_rpc_notifier.add_arp_entry)
for fixed_ip in allowed_address_pair_fixed_ips:
self._generate_arp_table_and_notify_agent(
context, fixed_ip, port_dict['mac_address'],
self.l3_rpc_notifier.add_arp_entry, nud_state='reachable')
def delete_arp_entry_for_dvr_service_port( def delete_arp_entry_for_dvr_service_port(
self, context, port_dict, fixed_ips_to_delete=None): self, context, port_dict, fixed_ips_to_delete=None):

3
neutron/privileged/agent/linux/ip_lib.py
View File

@ -176,14 +176,13 @@ def add_neigh_entry(ip_version, ip_address, mac_address, device, namespace,
:param namespace: The name of the namespace in which to add the entry :param namespace: The name of the namespace in which to add the entry
""" """
family = _IP_VERSION_FAMILY_MAP[ip_version] family = _IP_VERSION_FAMILY_MAP[ip_version]
state = kwargs.get('nud_state', 'permanent')
_run_iproute_neigh('replace', _run_iproute_neigh('replace',
device, device,
namespace, namespace,
dst=ip_address, dst=ip_address,
lladdr=mac_address, lladdr=mac_address,
family=family, family=family,
state=ndmsg.states[state], state=ndmsg.states['permanent'],
**kwargs) **kwargs)

12
neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py
View File

@ -1045,8 +1045,7 @@ class L3DvrTestCase(L3DvrTestCaseBase):
vm_arp_table = { vm_arp_table = {
'ip_address': vm_port_fixed_ips[0]['ip_address'], 'ip_address': vm_port_fixed_ips[0]['ip_address'],
'mac_address': vm_port_mac, 'mac_address': vm_port_mac,
'subnet_id': vm_port_subnet_id, 'subnet_id': vm_port_subnet_id}
'nud_state': 'permanent'}
vm_port2 = self.core_plugin.update_port( vm_port2 = self.core_plugin.update_port(
self.context, int_port2['port']['id'], self.context, int_port2['port']['id'],
{'port': {portbindings.HOST_ID: HOST2}}) {'port': {portbindings.HOST_ID: HOST2}})
@ -1098,8 +1097,7 @@ class L3DvrTestCase(L3DvrTestCaseBase):
vrrp_arp_table1 = { vrrp_arp_table1 = {
'ip_address': vrrp_port_fixed_ips[0]['ip_address'], 'ip_address': vrrp_port_fixed_ips[0]['ip_address'],
'mac_address': vm_port_mac, 'mac_address': vm_port_mac,
'subnet_id': vrrp_port_subnet_id, 'subnet_id': vrrp_port_subnet_id}
'nud_state': 'reachable'}
expected_calls = [ expected_calls = [
mock.call(self.context, mock.call(self.context,
@ -1214,8 +1212,7 @@ class L3DvrTestCase(L3DvrTestCaseBase):
vm_arp_table = { vm_arp_table = {
'ip_address': vm_port_fixed_ips[0]['ip_address'], 'ip_address': vm_port_fixed_ips[0]['ip_address'],
'mac_address': vm_port_mac, 'mac_address': vm_port_mac,
'subnet_id': vm_port_subnet_id, 'subnet_id': vm_port_subnet_id}
'nud_state': 'permanent'}
self.assertEqual(1, l3_notifier.add_arp_entry.call_count) self.assertEqual(1, l3_notifier.add_arp_entry.call_count)
floating_ip = {'floating_network_id': ext_net['network']['id'], floating_ip = {'floating_network_id': ext_net['network']['id'],
'router_id': router['id'], 'router_id': router['id'],
@ -1244,8 +1241,7 @@ class L3DvrTestCase(L3DvrTestCaseBase):
vrrp_arp_table1 = { vrrp_arp_table1 = {
'ip_address': vrrp_port_fixed_ips[0]['ip_address'], 'ip_address': vrrp_port_fixed_ips[0]['ip_address'],
'mac_address': vm_port_mac, 'mac_address': vm_port_mac,
'subnet_id': vrrp_port_subnet_id, 'subnet_id': vrrp_port_subnet_id}
'nud_state': 'reachable'}
expected_calls = [ expected_calls = [
mock.call(self.context, mock.call(self.context,

67
neutron/tests/unit/agent/l3/test_dvr_local_router.py
View File

@ -521,49 +521,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri._set_subnet_arp_info(subnet_id) ri._set_subnet_arp_info(subnet_id)
self.assertEqual(1, parp.call_count) self.assertEqual(1, parp.call_count)
self.mock_ip_dev.neigh.add.assert_called_once_with( self.mock_ip_dev.neigh.add.assert_called_once_with(
'1.2.3.4', '00:11:22:33:44:55', nud_state='permanent') '1.2.3.4', '00:11:22:33:44:55')
# Test negative case
router['distributed'] = False
ri._set_subnet_arp_info(subnet_id)
self.mock_ip_dev.neigh.add.never_called()
def test__set_subnet_arp_info_with_allowed_address_pair_port(self):
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
router = l3_test_common.prepare_router_data(num_internal_ports=2)
router['distributed'] = True
self._set_ri_kwargs(agent, router['id'], router)
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])
test_ports = [{'mac_address': '00:11:22:33:44:55',
'device_owner': '',
'fixed_ips': [{'ip_address': '1.2.3.4',
'prefixlen': 24,
'subnet_id': subnet_id}]},
{'mac_address': '11:22:33:44:55:66',
'device_owner': lib_constants.DEVICE_OWNER_LOADBALANCER,
'fixed_ips': [{'ip_address': '1.2.3.5',
'prefixlen': 24,
'subnet_id': subnet_id}]},
{'mac_address': '22:33:44:55:66:77',
'device_owner':
lib_constants.DEVICE_OWNER_LOADBALANCERV2,
'fixed_ips': [{'ip_address': '1.2.3.6',
'prefixlen': 24,
'subnet_id': subnet_id}]}]
self.plugin_api.get_ports_by_subnet.return_value = test_ports
# Test basic case
ports[0]['subnets'] = [{'id': subnet_id,
'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)
self.assertEqual(1, parp.call_count)
self.mock_ip_dev.neigh.add.assert_called_once_with(
'1.2.3.4', '00:11:22:33:44:55', nud_state='reachable')
# Test negative case # Test negative case
router['distributed'] = False router['distributed'] = False
@ -578,33 +536,14 @@ class TestDvrRouterOperations(base.BaseTestCase):
router[lib_constants.INTERFACE_KEY][0]) router[lib_constants.INTERFACE_KEY][0])
arp_table = {'ip_address': '1.7.23.11', arp_table = {'ip_address': '1.7.23.11',
'mac_address': '00:11:22:33:44:55', 'mac_address': '00:11:22:33:44:55',
'subnet_id': subnet_id, 'subnet_id': subnet_id}
'nud_state': 'permanent'}
payload = {'arp_table': arp_table, 'router_id': router['id']} payload = {'arp_table': arp_table, 'router_id': router['id']}
agent._router_added(router['id'], router) agent._router_added(router['id'], router)
agent.add_arp_entry(None, payload) agent.add_arp_entry(None, payload)
agent.router_deleted(None, router['id']) agent.router_deleted(None, router['id'])
self.mock_ip_dev.neigh.add.assert_called_once_with( self.mock_ip_dev.neigh.add.assert_called_once_with(
'1.7.23.11', '00:11:22:33:44:55', nud_state='permanent') '1.7.23.11', '00:11:22:33:44:55')
def test_add_arp_entry_with_nud_state_reachable(self):
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
router = l3_test_common.prepare_router_data(num_internal_ports=2)
router['distributed'] = True
subnet_id = l3_test_common.get_subnet_id(
router[lib_constants.INTERFACE_KEY][0])
arp_table = {'ip_address': '1.7.23.11',
'mac_address': '00:11:22:33:44:55',
'subnet_id': subnet_id,
'nud_state': 'reachable'}
payload = {'arp_table': arp_table, 'router_id': router['id']}
agent._router_added(router['id'], router)
agent.add_arp_entry(None, payload)
agent.router_deleted(None, router['id'])
self.mock_ip_dev.neigh.add.assert_called_once_with(
'1.7.23.11', '00:11:22:33:44:55', nud_state='reachable')
def test_add_arp_entry_no_routerinfo(self): def test_add_arp_entry_no_routerinfo(self):
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)

17
neutron/tests/unit/agent/linux/test_ip_lib.py
View File

@ -1681,23 +1681,6 @@ class TestIpNeighCommand(TestIPCmdBase):
ifindex=1, ifindex=1,
state=ndmsg.states['permanent']) state=ndmsg.states['permanent'])
@mock.patch.object(pyroute2, 'NetNS')
def test_add_entry_with_state_override(self, mock_netns):
mock_netns_instance = mock_netns.return_value
mock_netns_enter = mock_netns_instance.__enter__.return_value
mock_netns_enter.link_lookup.return_value = [1]
self.neigh_cmd.add(
'192.168.45.100', 'cc:dd:ee:ff:ab:cd', nud_state='reachable')
mock_netns_enter.link_lookup.assert_called_once_with(ifname='tap0')
mock_netns_enter.neigh.assert_called_once_with(
'replace',
dst='192.168.45.100',
lladdr='cc:dd:ee:ff:ab:cd',
family=2,
ifindex=1,
state=ndmsg.states['reachable'],
nud_state='reachable')
@mock.patch.object(pyroute2, 'NetNS') @mock.patch.object(pyroute2, 'NetNS')
def test_add_entry_nonexistent_namespace(self, mock_netns): def test_add_entry_nonexistent_namespace(self, mock_netns):
mock_netns.side_effect = OSError(errno.ENOENT, None) mock_netns.side_effect = OSError(errno.ENOENT, None)