openstack/neutron
Code Issues Proposed changes

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

Browse Source 
Allowed_address_pair IP when associated with a network port will
inherit the services MAC.
Right now the ARP entry is updated with the last MAC that it is
associated with. But when allowed_address_pair IPs are used in
the context of VRRP the MAC keeps switching between the MASTER
and SLAVE. VRRP instance sends out GARP, but the ARP entry in the
router namespace is not getting updated based on the GARP.

This might cause the VRRP IP and the service using the IP to fail.

Since we having been adding the ARP entry with NUD state as
PERMANENT, the ARP entries are set for ever and does not adopt the
GARP sent out by the VRRP instance.

This will cause instances associated with DVR routers to have a
service interruption.

So the proposed patch will add the ARP entry for the Allowed address
pair with NUD for 'REACHABLE'.
This allows the Allowed_address_pair IP MAC to be updated on the
fly.

Change-Id: I43c3471f5d259e8c2ee1685398a06a4680c0bfcd
Closes-Bug: #1608400
(cherry-picked from commit fbe308bdc1)
This commit is contained in:
Swaminathan Vasudevan 2018-03-07 19:03:42 -08:00
parent 40ac93b806
commit 34b7213ee3
7 changed files with 96 additions and 17 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']
mac = arp_table['mac_address']
subnet_id = arp_table['subnet_id']
ri._update_arp_entry(ip, mac, subnet_id, action)
nud_state = arp_table.get('nud_state')
ri._update_arp_entry(ip, mac, subnet_id, action, nud_state=nud_state)
def add_arp_entry(self, context, payload):
"""Add arp entry into router namespace. Called from RPC."""

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

@ -239,7 +239,8 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
arp_delete.add(arp_entry)
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, nud_state='permanent'):
"""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
@ -252,7 +253,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
device = ip_lib.IPDevice(interface_name, namespace=self.ns_name)
if device.exists():
if operation == 'add':
device.neigh.add(ip, mac)
device.neigh.add(ip, mac, nud_state=nud_state)
elif operation == 'delete':
device.neigh.delete(ip, mac)
return True
@ -276,12 +277,14 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
subnet_ports = self.agent.get_ports_by_subnet(subnet_id)
for p in subnet_ports:
nud_state = 'permanent' if p.get('device_owner') else 'reachable'
if p['device_owner'] not in lib_constants.ROUTER_INTERFACE_OWNERS:
for fixed_ip in p['fixed_ips']:
self._update_arp_entry(fixed_ip['ip_address'],
p['mac_address'],
subnet_id,
'add')
'add',
nud_state=nud_state)
self._process_arp_cache_for_internal_port(subnet_id)
@staticmethod

12
neutron/db/l3_dvr_db.py
View File

@ -902,7 +902,7 @@ class _DVRAgentInterfaceMixin(object):
return f_port
def _generate_arp_table_and_notify_agent(
self, context, fixed_ip, mac_address, notifier):
self, context, fixed_ip, mac_address, notifier, nud_state='permanent'):
"""Generates the arp table entry and notifies the l3 agent."""
ip_address = fixed_ip['ip_address']
subnet = fixed_ip['subnet_id']
@ -914,7 +914,8 @@ class _DVRAgentInterfaceMixin(object):
return
arp_table = {'ip_address': ip_address,
'mac_address': mac_address,
'subnet_id': subnet}
'subnet_id': subnet,
'nud_state': nud_state}
notifier(context, router_id, arp_table)
def _get_subnet_id_for_given_fixed_ip(
@ -958,11 +959,14 @@ class _DVRAgentInterfaceMixin(object):
return
allowed_address_pair_fixed_ips = (
self._get_allowed_address_pair_fixed_ips(context, port_dict))
changed_fixed_ips = fixed_ips + allowed_address_pair_fixed_ips
for fixed_ip in changed_fixed_ips:
for fixed_ip in fixed_ips:
self._generate_arp_table_and_notify_agent(
context, fixed_ip, port_dict['mac_address'],
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(
self, context, port_dict, fixed_ips_to_delete=None):

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

@ -112,13 +112,14 @@ 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
"""
family = _IP_VERSION_FAMILY_MAP[ip_version]
state = kwargs.get('nud_state', 'permanent')
_run_iproute('replace',
device,
namespace,
dst=ip_address,
lladdr=mac_address,
family=family,
state=ndmsg.states['permanent'],
state=ndmsg.states[state],
**kwargs)

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

@ -1045,7 +1045,8 @@ class L3DvrTestCase(L3DvrTestCaseBase):
vm_arp_table = {
'ip_address': vm_port_fixed_ips[0]['ip_address'],
'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(
self.context, int_port2['port']['id'],
{'port': {portbindings.HOST_ID: HOST2}})
@ -1097,7 +1098,8 @@ class L3DvrTestCase(L3DvrTestCaseBase):
vrrp_arp_table1 = {
'ip_address': vrrp_port_fixed_ips[0]['ip_address'],
'mac_address': vm_port_mac,
'subnet_id': vrrp_port_subnet_id}
'subnet_id': vrrp_port_subnet_id,
'nud_state': 'reachable'}
expected_calls = [
mock.call(self.context,
@ -1212,7 +1214,8 @@ class L3DvrTestCase(L3DvrTestCaseBase):
vm_arp_table = {
'ip_address': vm_port_fixed_ips[0]['ip_address'],
'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)
floating_ip = {'floating_network_id': ext_net['network']['id'],
'router_id': router['id'],
@ -1241,7 +1244,8 @@ class L3DvrTestCase(L3DvrTestCaseBase):
vrrp_arp_table1 = {
'ip_address': vrrp_port_fixed_ips[0]['ip_address'],
'mac_address': vm_port_mac,
'subnet_id': vrrp_port_subnet_id}
'subnet_id': vrrp_port_subnet_id,
'nud_state': 'reachable'}
expected_calls = [
mock.call(self.context,

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

@ -510,7 +510,38 @@ class TestDvrRouterOperations(base.BaseTestCase):
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')
'1.2.3.4', '00:11:22:33:44:55', nud_state='permanent')
# 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}]}]
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
router['distributed'] = False
@ -525,14 +556,33 @@ class TestDvrRouterOperations(base.BaseTestCase):
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}
'subnet_id': subnet_id,
'nud_state': 'permanent'}
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')
'1.7.23.11', '00:11:22:33:44:55', nud_state='permanent')
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):
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)

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

@ -1643,6 +1643,23 @@ class TestIpNeighCommand(TestIPCmdBase):
ifindex=1,
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')
def test_add_entry_nonexistent_namespace(self, mock_netns):
mock_netns.side_effect = OSError(errno.ENOENT, None)