From d019790fe436b72cb05b8d0ff1f3a62ebd9e9bee 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 --- 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 f211adea7b7..9b2812805a8 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 3de6a023a2a..9199bd8b6e7 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 @@ -420,16 +420,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 85a4c863a25..efd49f59169 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 @@ -2442,29 +2442,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' @@ -2487,7 +2491,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=[]),\ @@ -2521,7 +2530,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, @@ -2554,7 +2564,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' @@ -2577,7 +2588,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=[]),\ @@ -2610,7 +2626,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) @@ -2645,6 +2662,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) @@ -2738,7 +2765,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=[]),\ @@ -2802,7 +2830,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=[]),\ @@ -2907,7 +2940,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=[]),\ @@ -3022,7 +3060,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=[]),\