From f65e7ba05d0502bdc7163a6221ed8138ac183055 Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Fri, 23 Feb 2018 16:22:33 -0800 Subject: [PATCH] DVR: Inter Tenant Traffic between networks not possible with shared net Inter Tenant Traffic between two different networks that belong to two different Tenants is not possible when connected through a shared network that are internally connected through DVR routers. This issue can be seen in multinode environment where there is network isolation. The issue is, we have two different IP for the ports that are connecting the two routers and DVR does not expose the router interfaces outside a compute and is blocked by ovs tunnel bridge rules. This patch fixes the issue by not applying the DVR specific rules in the tunnel-bridge to the shared network ports that are connecting the routers. Closes-Bug: #1751396 Change-Id: I0717f29209f1354605d2f4128949ddbaefd99629 (cherry picked from commit d019790fe436b72cb05b8d0ff1f3a62ebd9e9bee) --- neutron/api/rpc/handlers/dvr_rpc.py | 14 +++ .../agent/ovs_dvr_neutron_agent.py | 29 ++++-- .../unit/api/rpc/handlers/test_dvr_rpc.py | 7 ++ .../agent/test_ovs_neutron_agent.py | 97 +++++++++++++------ 4 files changed, 111 insertions(+), 36 deletions(-) diff --git a/neutron/api/rpc/handlers/dvr_rpc.py b/neutron/api/rpc/handlers/dvr_rpc.py index b85f4c69204..6518da4aadb 100644 --- a/neutron/api/rpc/handlers/dvr_rpc.py +++ b/neutron/api/rpc/handlers/dvr_rpc.py @@ -59,6 +59,13 @@ class DVRServerRpcApi(object): return cctxt.call(context, 'get_ports_on_host_by_subnet', host=host, subnet=subnet) + @log_helpers.log_method_call + def get_network_info_for_id(self, context, network_id): + """Get network info for DVR router ports.""" + cctxt = self.client.prepare() + return cctxt.call(context, 'get_network_info_for_id', + network_id=network_id) + @log_helpers.log_method_call def get_subnet_for_dvr(self, context, subnet, fixed_ips): cctxt = self.client.prepare() @@ -105,6 +112,13 @@ class DVRServerRpcCallback(object): return self.plugin.get_ports_on_host_by_subnet(context, host, subnet) + def get_network_info_for_id(self, context, **kwargs): + """Get network info for DVR port.""" + network_id = kwargs.get('network_id') + LOG.debug("DVR Agent requests network info for id %s", network_id) + net_filter = {'id': [network_id]} + return self.plugin.get_networks(context, filters=net_filter) + def get_subnet_for_dvr(self, context, **kwargs): fixed_ips = kwargs.get('fixed_ips') subnet = kwargs.get('subnet') diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py index edecbc51e38..6b8e6f9a0b6 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py @@ -425,16 +425,27 @@ class OVSDVRNeutronAgent(object): br = self.tun_br # TODO(vivek) remove the IPv6 related flows once SNAT is not # used for IPv6 DVR. - if ip_version == 4: - if subnet_info['gateway_ip']: - br.install_dvr_process_ipv4( - vlan_tag=lvm.vlan, gateway_ip=subnet_info['gateway_ip']) + port_net_info = ( + self.plugin_rpc.get_network_info_for_id( + self.context, subnet_info.get('network_id'))) + net_shared_only = ( + port_net_info[0]['shared'] and + not port_net_info[0]['router:external']) + if net_shared_only: + LOG.debug("Not applying DVR rules to tunnel bridge because %s " + "is a shared network", subnet_info.get('network_id')) else: - br.install_dvr_process_ipv6( - vlan_tag=lvm.vlan, gateway_mac=subnet_info['gateway_mac']) - br.install_dvr_process( - vlan_tag=lvm.vlan, vif_mac=port.vif_mac, - dvr_mac_address=self.dvr_mac_address) + if ip_version == 4: + if subnet_info['gateway_ip']: + br.install_dvr_process_ipv4( + vlan_tag=lvm.vlan, + gateway_ip=subnet_info['gateway_ip']) + else: + br.install_dvr_process_ipv6( + vlan_tag=lvm.vlan, gateway_mac=subnet_info['gateway_mac']) + br.install_dvr_process( + vlan_tag=lvm.vlan, vif_mac=port.vif_mac, + dvr_mac_address=self.dvr_mac_address) # the dvr router interface is itself a port, so capture it # queue this subnet to that port. A subnet appears only once as diff --git a/neutron/tests/unit/api/rpc/handlers/test_dvr_rpc.py b/neutron/tests/unit/api/rpc/handlers/test_dvr_rpc.py index 0931604db7b..b8a3fb4b618 100644 --- a/neutron/tests/unit/api/rpc/handlers/test_dvr_rpc.py +++ b/neutron/tests/unit/api/rpc/handlers/test_dvr_rpc.py @@ -46,6 +46,13 @@ class DVRServerRpcApiTestCase(base.BaseTestCase): self.ctxt, 'get_ports_on_host_by_subnet', host='foo_host', subnet='foo_subnet') + def test_get_network_info_for_id(self): + self.rpc.get_network_info_for_id( + self.ctxt, 'fake-network-id') + self.mock_cctxt.call.assert_called_with( + self.ctxt, 'get_network_info_for_id', + network_id='fake-network-id') + def test_get_subnet_for_dvr(self): self.rpc.get_subnet_for_dvr( self.ctxt, 'foo_subnet', fixed_ips='foo_fixed_ips') diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 0ce32f6cf34..c2802bef402 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -2357,29 +2357,33 @@ class TestOvsDvrNeutronAgent(object): return resp def _expected_install_dvr_process(self, lvid, port, ip_version, - gateway_ip, gateway_mac): - if ip_version == 4: - ipvx_calls = [ - mock.call.install_dvr_process_ipv4( + gateway_ip, gateway_mac, shared=False): + if not shared: + if ip_version == 4: + ipvx_calls = [ + mock.call.install_dvr_process_ipv4( + vlan_tag=lvid, + gateway_ip=gateway_ip), + ] + else: + ipvx_calls = [ + mock.call.install_dvr_process_ipv6( + vlan_tag=lvid, + gateway_mac=gateway_mac), + ] + return ipvx_calls + [ + mock.call.install_dvr_process( vlan_tag=lvid, - gateway_ip=gateway_ip), + dvr_mac_address=self.agent.dvr_agent.dvr_mac_address, + vif_mac=port.vif_mac, + ), ] else: - ipvx_calls = [ - mock.call.install_dvr_process_ipv6( - vlan_tag=lvid, - gateway_mac=gateway_mac), - ] - return ipvx_calls + [ - mock.call.install_dvr_process( - vlan_tag=lvid, - dvr_mac_address=self.agent.dvr_agent.dvr_mac_address, - vif_mac=port.vif_mac, - ), - ] + return [] def _test_port_bound_for_dvr_on_vlan_network(self, device_owner, - ip_version=4): + ip_version=4, + shared=False): self._setup_for_dvr_test() if ip_version == 4: gateway_ip = '1.1.1.1' @@ -2402,7 +2406,12 @@ class TestOvsDvrNeutronAgent(object): return_value={'gateway_ip': gateway_ip, 'cidr': cidr, 'ip_version': ip_version, - 'gateway_mac': gateway_mac}),\ + 'gateway_mac': gateway_mac, + 'network_id': 'fake-id'}),\ + mock.patch.object(self.agent.dvr_agent.plugin_rpc, + 'get_network_info_for_id', + return_value=[{'shared': shared, + 'router:external': False}]),\ mock.patch.object(self.agent.dvr_agent.plugin_rpc, 'get_ports_on_host_by_subnet', return_value=[]),\ @@ -2436,7 +2445,8 @@ class TestOvsDvrNeutronAgent(object): lvid=lvid, ip_version=ip_version, gateway_ip=gateway_ip, - gateway_mac=gateway_mac) + gateway_mac=gateway_mac, + shared=shared) expected_on_int_br = [ mock.call.provision_local_vlan( port=int_ofp, @@ -2469,7 +2479,8 @@ class TestOvsDvrNeutronAgent(object): self.assertFalse([], phys_br.mock_calls) def _test_port_bound_for_dvr_on_vxlan_network(self, device_owner, - ip_version=4): + ip_version=4, + shared=False): self._setup_for_dvr_test() if ip_version == 4: gateway_ip = '1.1.1.1' @@ -2492,7 +2503,12 @@ class TestOvsDvrNeutronAgent(object): return_value={'gateway_ip': gateway_ip, 'cidr': cidr, 'ip_version': ip_version, - 'gateway_mac': gateway_mac}),\ + 'gateway_mac': gateway_mac, + 'network_id': 'fake-id'}),\ + mock.patch.object(self.agent.dvr_agent.plugin_rpc, + 'get_network_info_for_id', + return_value=[{'shared': shared, + 'router:external': False}]),\ mock.patch.object(self.agent.dvr_agent.plugin_rpc, 'get_ports_on_host_by_subnet', return_value=[]),\ @@ -2525,7 +2541,8 @@ class TestOvsDvrNeutronAgent(object): lvid=lvid, ip_version=ip_version, gateway_ip=gateway_ip, - gateway_mac=gateway_mac) + gateway_mac=gateway_mac, + shared=shared) self.assertEqual(expected_on_int_br, int_br.mock_calls) self.assertEqual(expected_on_tun_br, tun_br.mock_calls) self.assertEqual([], phys_br.mock_calls) @@ -2560,6 +2577,16 @@ class TestOvsDvrNeutronAgent(object): self._test_port_bound_for_dvr_on_vxlan_network( device_owner=DEVICE_OWNER_COMPUTE, ip_version=6) + def test_port_bound_for_dvr_with_compute_ports_on_shared_network(self): + self._test_port_bound_for_dvr_on_vlan_network( + device_owner=DEVICE_OWNER_COMPUTE, shared=True) + self._test_port_bound_for_dvr_on_vlan_network( + device_owner=DEVICE_OWNER_COMPUTE, ip_version=6, shared=True) + self._test_port_bound_for_dvr_on_vxlan_network( + device_owner=DEVICE_OWNER_COMPUTE, shared=True) + self._test_port_bound_for_dvr_on_vxlan_network( + device_owner=DEVICE_OWNER_COMPUTE, ip_version=6, shared=True) + def test_port_bound_for_dvr_with_lbaas_vip_ports(self): self._test_port_bound_for_dvr_on_vlan_network( device_owner=n_const.DEVICE_OWNER_LOADBALANCER) @@ -2653,7 +2680,8 @@ class TestOvsDvrNeutronAgent(object): return_value={'gateway_ip': '1.1.1.1', 'cidr': '1.1.1.0/24', 'ip_version': 4, - 'gateway_mac': 'aa:bb:cc:11:22:33'}),\ + 'gateway_mac': 'aa:bb:cc:11:22:33', + 'network_id': 'faked-id'}),\ mock.patch.object(self.agent.dvr_agent.plugin_rpc, 'get_ports_on_host_by_subnet', return_value=[]),\ @@ -2717,7 +2745,12 @@ class TestOvsDvrNeutronAgent(object): return_value={'gateway_ip': gateway_ip, 'cidr': cidr, 'ip_version': ip_version, - 'gateway_mac': gateway_mac}),\ + 'gateway_mac': gateway_mac, + 'network_id': 'fake-id'}),\ + mock.patch.object(self.agent.dvr_agent.plugin_rpc, + 'get_network_info_for_id', + return_value=[{'shared': False, + 'router:external': False}]),\ mock.patch.object(self.agent.dvr_agent.plugin_rpc, 'get_ports_on_host_by_subnet', return_value=[]),\ @@ -2822,7 +2855,12 @@ class TestOvsDvrNeutronAgent(object): return_value={'gateway_ip': gateway_ip, 'cidr': cidr, 'ip_version': ip_version, - 'gateway_mac': gateway_mac}),\ + 'gateway_mac': gateway_mac, + 'network_id': 'faked-id'}),\ + mock.patch.object(self.agent.dvr_agent.plugin_rpc, + 'get_network_info_for_id', + return_value=[{'shared': False, + 'router:external': False}]),\ mock.patch.object(self.agent.dvr_agent.plugin_rpc, 'get_ports_on_host_by_subnet', return_value=[]),\ @@ -2937,7 +2975,12 @@ class TestOvsDvrNeutronAgent(object): return_value={'gateway_ip': '1.1.1.1', 'cidr': '1.1.1.0/24', 'ip_version': 4, - 'gateway_mac': gateway_mac}),\ + 'gateway_mac': gateway_mac, + 'network_id': 'fake-id'}),\ + mock.patch.object(self.agent.dvr_agent.plugin_rpc, + 'get_network_info_for_id', + return_value=[{'shared': False, + 'router:external': False}]),\ mock.patch.object(self.agent.dvr_agent.plugin_rpc, 'get_ports_on_host_by_subnet', return_value=[]),\