Merge "Fix removal of dvr-src mac flows when non-gateway port on router is deleted" into stable/rocky

neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import collections
 import sys
 
 import netaddr
@@ -36,6 +37,8 @@ class LocalDVRSubnetMapping(object):
     def __init__(self, subnet, csnat_ofport=constants.OFPORT_INVALID):
         # set of compute ports on this dvr subnet
         self.compute_ports = {}
+        # set of dvr router interfaces on this subnet
+        self.dvr_ports = {}
         self.subnet = subnet
         self.csnat_ofport = csnat_ofport
         self.dvr_owned = False
@@ -73,6 +76,15 @@ class LocalDVRSubnetMapping(object):
     def get_csnat_ofport(self):
         return self.csnat_ofport
 
+    def add_dvr_ofport(self, vif_id, ofport):
+        self.dvr_ports[vif_id] = ofport
+
+    def remove_dvr_ofport(self, vif_id):
+        self.dvr_ports.pop(vif_id, 0)
+
+    def get_dvr_ofports(self):
+        return self.dvr_ports
+
 
 class OVSPort(object):
     def __init__(self, id, ofport, mac, device_owner):
@@ -81,21 +93,30 @@ class OVSPort(object):
         self.ofport = ofport
         self.subnets = set()
         self.device_owner = device_owner
+        # Currently, this is updated only for DVR router interfaces
+        self.ips = collections.defaultdict(list)
 
     def __str__(self):
         return ("OVSPort: id = %s, ofport = %s, mac = %s, "
-                "device_owner = %s, subnets = %s" %
+                "device_owner = %s, subnets = %s, ips = %s" %
                 (self.id, self.ofport, self.mac,
-                 self.device_owner, self.subnets))
+                 self.device_owner, self.subnets,
+                 self.ips))
 
-    def add_subnet(self, subnet_id):
+    def add_subnet(self, subnet_id, fixed_ip=None):
         self.subnets.add(subnet_id)
+        if fixed_ip is None:
+            return
+
+        self.ips[subnet_id].append(fixed_ip)
 
     def remove_subnet(self, subnet_id):
         self.subnets.remove(subnet_id)
+        self.ips.pop(subnet_id, None)
 
     def remove_all_subnets(self):
         self.subnets.clear()
+        self.ips.clear()
 
     def get_subnets(self):
         return self.subnets
@@ -109,6 +130,9 @@ class OVSPort(object):
     def get_ofport(self):
         return self.ofport
 
+    def get_ip(self, subnet_id):
+        return self.ips.get(subnet_id)
+
 
 @profiler.trace_cls("ovs_dvr_agent")
 class OVSDVRNeutronAgent(object):
@@ -458,8 +482,9 @@ class OVSDVRNeutronAgent(object):
         # a router interface on any given router
         ovsport = OVSPort(port.vif_id, port.ofport,
                           port.vif_mac, device_owner)
-        ovsport.add_subnet(subnet_uuid)
+        ovsport.add_subnet(subnet_uuid, fixed_ip['ip_address'])
         self.local_ports[port.vif_id] = ovsport
+        ldm.add_dvr_ofport(port.vif_id, port.ofport)
 
     def _bind_port_on_dvr_subnet(self, port, lvm, fixed_ips,
                                  device_owner):
@@ -617,22 +642,37 @@ class OVSDVRNeutronAgent(object):
             ldm = self.local_dvr_map[sub_uuid]
             subnet_info = ldm.get_subnet_info()
             ip_version = subnet_info['ip_version']
-            # DVR is no more owner
-            ldm.set_dvr_owned(False)
-            # remove all vm rules for this dvr subnet
-            # clear of compute_ports altogether
-            compute_ports = ldm.get_compute_ofports()
-            for vif_id in compute_ports:
-                comp_port = self.local_ports[vif_id]
-                self.int_br.delete_dvr_to_src_mac(
-                    network_type=network_type,
-                    vlan_tag=vlan_to_use, dst_mac=comp_port.get_mac())
-            ldm.remove_all_compute_ofports()
-
-            if ldm.get_csnat_ofport() == constants.OFPORT_INVALID:
-                # if there is no csnat port for this subnet, remove
-                # this subnet from local_dvr_map, as no dvr (or) csnat
-                # ports available on this agent anymore
+
+            fixed_ip = ovsport.get_ip(sub_uuid)
+            is_dvr_gateway_port = False
+            subnet_gateway = subnet_info.get('gateway_ip')
+            # since distributed router port must have only one fixed IP,
+            # directly use fixed_ip[0]
+            if fixed_ip and fixed_ip[0] == subnet_gateway:
+                is_dvr_gateway_port = True
+
+            # remove vm dvr src mac rules only if the ovsport
+            # is gateway for the subnet or if the gateway is
+            # not set on the subnet
+            if is_dvr_gateway_port or not subnet_gateway:
+                # DVR is no more owner
+                ldm.set_dvr_owned(False)
+                # remove all vm rules for this dvr subnet
+                # clear of compute_ports altogether
+                compute_ports = ldm.get_compute_ofports()
+                for vif_id in compute_ports:
+                    comp_port = self.local_ports[vif_id]
+                    self.int_br.delete_dvr_to_src_mac(
+                        network_type=network_type,
+                        vlan_tag=vlan_to_use, dst_mac=comp_port.get_mac())
+                ldm.remove_all_compute_ofports()
+
+            if (ldm.get_csnat_ofport() == constants.OFPORT_INVALID and
+                    len(ldm.get_dvr_ofports()) <= 1):
+                # if there is no csnat port for this subnet and if this is
+                # the last dvr port in the subnet, remove this subnet from
+                # local_dvr_map, as no dvr (or) csnat ports available on this
+                # agent anymore
                 self.local_dvr_map.pop(sub_uuid, None)
             if network_type == n_const.TYPE_VLAN:
                 br = self.phys_brs[physical_network]
@@ -647,6 +687,7 @@ class OVSDVRNeutronAgent(object):
                 br.delete_dvr_process_ipv6(
                     vlan_tag=lvm.vlan, gateway_mac=subnet_info['gateway_mac'])
             ovsport.remove_subnet(sub_uuid)
+            ldm.remove_dvr_ofport(port.vif_id)
 
             if self.firewall and isinstance(self.firewall,
                                             ovs_firewall.OVSFirewallDriver):

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py

@@ -2690,6 +2690,15 @@ class TestOvsDvrNeutronAgent(object):
         self._compute_fixed_ips = [{'subnet_id': 'my-subnet-uuid',
                                     'ip_address': '1.1.1.3'}]
 
+    def _setup_extra_port_for_dvr(self):
+        self._port_nongateway = mock.Mock()
+        self._port_nongateway.ofport = 11
+        self._port_nongateway.vif_id = "1234-5678-99"
+        self._port_nongateway.vif_mac = 'aa:bb:cc:11:22:44'
+        self._port_nongateway.dvr_mac = self.agent.dvr_agent.dvr_mac_address
+        self._fixed_ips_nongateway = [{'subnet_id': 'my-subnet-uuid',
+                                       'ip_address': '1.1.1.11'}]
+
     @staticmethod
     def _expected_port_bound(port, lvid, is_dvr=True):
         resp = [
@@ -3061,6 +3070,152 @@ class TestOvsDvrNeutronAgent(object):
                 False)
             self.assertFalse(int_br.install_dvr_to_src_mac.called)
 
+    def _test_port_unbound_dvr_router_port_on_vxlan_network(
+            self, port_type='gateway', ip_version=n_const.IP_VERSION_4):
+        """Test DVR ovs flows when router port is unbound.
+
+        Setup 2 ports from same subnet to a DVR router. Add an instance port.
+        Remove one of the router ports based on port_type passed to the
+        function. port_type='gateway' removes the gateway port, port_type
+        nongateway removes the non-gateway port. port_type='all' removes
+        both the ports. Verify if the corresponding ovs flows are updated.
+        """
+        self._setup_for_dvr_test()
+        self._setup_extra_port_for_dvr()
+
+        if ip_version == n_const.IP_VERSION_4:
+            gateway_ip = '1.1.1.1'
+            cidr = '1.1.1.0/24'
+        else:
+            gateway_ip = '2001:db8:100::1'
+            cidr = '2001:db8:100::0/64'
+            self._fixed_ips = [{'subnet_id': 'my-subnet-uuid',
+                               'ip_address': '2001:db8:100::1'}]
+            self._fixed_ips_nongateway = [{'subnet_id': 'my-subnet-uuid',
+                                          'ip_address': '2001:db8:100::10'}]
+        network_type = n_const.TYPE_VXLAN
+        self._port.vif_mac = gateway_mac = 'aa:bb:cc:11:22:33'
+        self._port.dvr_mac = self.agent.dvr_agent.dvr_mac_address
+        self._compute_port.vif_mac = '77:88:99:00:11:22'
+        physical_network = self._physical_network
+        segmentation_id = self._segmentation_id
+
+        int_br = mock.create_autospec(self.agent.int_br)
+        tun_br = mock.create_autospec(self.agent.tun_br)
+        phys_br = mock.create_autospec(self.br_phys_cls('br-phys'))
+        int_br.set_db_attribute.return_value = True
+        int_br.db_get_val.return_value = {}
+        with mock.patch.object(self.agent.dvr_agent.plugin_rpc,
+                               'get_subnet_for_dvr',
+                               return_value={'gateway_ip': gateway_ip,
+                                             'cidr': cidr,
+                                             'ip_version': ip_version,
+                                             'gateway_mac': gateway_mac}),\
+                mock.patch.object(self.agent.dvr_agent.plugin_rpc,
+                                  'get_ports_on_host_by_subnet',
+                                  return_value=[]),\
+                mock.patch.object(self.agent.dvr_agent.int_br,
+                                  'get_vif_port_by_id',
+                                  return_value=self._port),\
+                mock.patch.object(self.agent, 'int_br', new=int_br),\
+                mock.patch.object(self.agent, 'tun_br', new=tun_br),\
+                mock.patch.dict(self.agent.phys_brs,
+                                {physical_network: phys_br}),\
+                mock.patch.object(self.agent.dvr_agent, 'int_br', new=int_br),\
+                mock.patch.object(self.agent.dvr_agent, 'tun_br', new=tun_br),\
+                mock.patch.dict(self.agent.dvr_agent.phys_brs,
+                                {physical_network: phys_br}):
+            self.agent.port_bound(
+                self._port, self._net_uuid, network_type,
+                physical_network, segmentation_id, self._fixed_ips,
+                n_const.DEVICE_OWNER_DVR_INTERFACE, False)
+
+            lvid = self.agent.vlan_manager.get(self._net_uuid).vlan
+
+            # Bound non-gateway port
+            self.agent.port_bound(
+                self._port_nongateway, self._net_uuid, network_type,
+                physical_network, segmentation_id, self._fixed_ips_nongateway,
+                n_const.DEVICE_OWNER_DVR_INTERFACE, False)
+
+            # Bound compute port
+            self.agent.port_bound(self._compute_port, self._net_uuid,
+                                  network_type, physical_network,
+                                  segmentation_id,
+                                  self._compute_fixed_ips,
+                                  DEVICE_OWNER_COMPUTE, False)
+
+            int_br.reset_mock()
+            tun_br.reset_mock()
+            phys_br.reset_mock()
+
+            ports_to_unbind = {}
+            expected_br_int_mock_calls_gateway_port = 1
+            expected_br_int_mock_calls_nongateway_port = 0
+            if port_type == 'gateway':
+                ports_to_unbind = {
+                    self._port: expected_br_int_mock_calls_gateway_port
+                }
+            elif port_type == 'nongateway':
+                ports_to_unbind = {
+                    self._port_nongateway:
+                    expected_br_int_mock_calls_nongateway_port
+                }
+            else:
+                ports_to_unbind = {
+                    self._port:
+                    expected_br_int_mock_calls_gateway_port,
+                    self._port_nongateway:
+                    expected_br_int_mock_calls_nongateway_port
+                }
+
+            for port_to_unbind, br_int_mock_calls in ports_to_unbind.items():
+                self.agent.port_unbound(port_to_unbind.vif_id)
+
+                if ip_version == n_const.IP_VERSION_4:
+                    expected_on_tun_br = [
+                        mock.call.delete_dvr_process_ipv4(
+                            vlan_tag=lvid,
+                            gateway_ip=gateway_ip),
+                    ]
+                else:
+                    expected_on_tun_br = [
+                        mock.call.delete_dvr_process_ipv6(
+                            vlan_tag=lvid,
+                            gateway_mac=gateway_mac),
+                    ]
+                expected_on_tun_br.extend([
+                    mock.call.delete_dvr_process(
+                        vlan_tag=lvid,
+                        vif_mac=port_to_unbind.vif_mac),
+                ])
+
+                self.assertEqual(br_int_mock_calls, len(int_br.mock_calls))
+                tun_br.assert_has_calls(expected_on_tun_br)
+                int_br.reset_mock()
+                tun_br.reset_mock()
+                phys_br.assert_not_called()
+                phys_br.reset_mock()
+
+    def test_port_unbound_dvr_router_port(self):
+        self._test_port_unbound_dvr_router_port_on_vxlan_network(
+            port_type='gateway')
+        self._test_port_unbound_dvr_router_port_on_vxlan_network(
+            port_type='nongateway')
+        # Unbound all the dvr router ports
+        self._test_port_unbound_dvr_router_port_on_vxlan_network(
+            port_type='all')
+        self._test_port_unbound_dvr_router_port_on_vxlan_network(
+            port_type='gateway',
+            ip_version=n_const.IP_VERSION_6)
+        self._test_port_unbound_dvr_router_port_on_vxlan_network(
+            port_type='nongateway',
+            ip_version=n_const.IP_VERSION_6)
+        # Unbound all the dvr router ports
+        self._test_port_unbound_dvr_router_port_on_vxlan_network(
+            port_type='all',
+            ip_version=n_const.IP_VERSION_6)
+
     def test_treat_devices_removed_for_dvr_interface(self):
         self._test_treat_devices_removed_for_dvr_interface()
         self._test_treat_devices_removed_for_dvr_interface(ip_version=6)