DVR: Fix allowed_address_pair IP, ARP table update by neutron agent
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
This commit is contained in:
parent
c542ec801a
commit
fbe308bdc1
|
@ -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."""
|
||||
|
|
|
@ -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
|
||||
|
@ -279,12 +280,14 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
|
|||
tuple(common_utils.get_dvr_allowed_address_pair_device_owners()))
|
||||
|
||||
for p in subnet_ports:
|
||||
nud_state = 'permanent' if p.get('device_owner') else 'reachable'
|
||||
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')
|
||||
'add',
|
||||
nud_state=nud_state)
|
||||
self._process_arp_cache_for_internal_port(subnet_id)
|
||||
|
||||
@staticmethod
|
||||
|
|
|
@ -892,7 +892,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']
|
||||
|
@ -904,7 +904,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(
|
||||
|
@ -948,11 +949,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):
|
||||
|
|
|
@ -176,13 +176,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_neigh('replace',
|
||||
device,
|
||||
namespace,
|
||||
dst=ip_address,
|
||||
lladdr=mac_address,
|
||||
family=family,
|
||||
state=ndmsg.states['permanent'],
|
||||
state=ndmsg.states[state],
|
||||
**kwargs)
|
||||
|
||||
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -521,7 +521,49 @@ 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}]},
|
||||
{'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
|
||||
router['distributed'] = False
|
||||
|
@ -536,14 +578,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)
|
||||
|
|
|
@ -1681,6 +1681,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)
|
||||
|
|
Loading…
Reference in New Issue