Merge "Remove deprecated use_veth_interconnection option"

doc/source/contributor/internals/openvswitch_agent.rst

@@ -466,8 +466,7 @@ Implementation Trunk Bridge (Option C)
 
 This implementation is based on this `etherpad <https://etherpad.openstack.org/p/trunk-bridge-tagged-patch-experiment>`_.
 Credits to Bence Romsics.
-The option use_veth_interconnection=true won't be supported, it is deprecated since Victoria,
+The IDs used for bridge and port names are truncated.
-see [1]. The IDs used for bridge and port names are truncated.
 
 ::
 
@@ -499,8 +498,6 @@ tpi-parent-id: int bridge side of the patch port that implements a trunk.
 spt-subport-id: trunk bridge side of the patch port that implements a subport.
 spi-subport-id: int bridge side of the patch port that implements a subport.
 
-[1] https://bugs.launchpad.net/neutron/+bug/1587296
-
 Trunk creation
 ++++++++++++++
 

neutron/cmd/sanity_check.py

@@ -296,7 +296,7 @@ OPTS = [
                     help=_('Check for OVS Geneve support')),
     BoolOptCallback('iproute2_vxlan', check_iproute2_vxlan, default=False,
                     help=_('Check for iproute2 vxlan support')),
-    BoolOptCallback('ovs_patch', check_ovs_patch, default=False,
+    BoolOptCallback('ovs_patch', check_ovs_patch, default=True,
                     help=_('Check for patch port support')),
     BoolOptCallback('nova_notify', check_nova_notify,
                     help=_('Check for nova notification support')),
@@ -366,10 +366,6 @@ def enable_tests_from_config():
     if ('vxlan' in cfg.CONF.ml2.type_drivers or
             cfg.CONF.VXLAN.enable_vxlan):
         cfg.CONF.set_default('iproute2_vxlan', True)
-    if cfg.CONF.AGENT.tunnel_types:
-        cfg.CONF.set_default('ovs_patch', True)
-    if not cfg.CONF.OVS.use_veth_interconnection:
-        cfg.CONF.set_default('ovs_patch', True)
     if (cfg.CONF.notify_nova_on_port_status_changes or
             cfg.CONF.notify_nova_on_port_data_changes):
         cfg.CONF.set_default('nova_notify', True)

neutron/conf/plugins/ml2/drivers/ovs_conf.py

@@ -100,15 +100,6 @@ ovs_opts = [
                        "See also: "
                        "https://docs.openstack.org/api-ref/placement/"
                        "#update-resource-provider-inventories")),
-    cfg.BoolOpt('use_veth_interconnection', default=False,
-                deprecated_for_removal=True,
-                deprecated_since="Victoria",
-                deprecated_reason="Patch ports should be used to provide "
-                                  "bridges interconnection.",
-                help=_("Use veths instead of patch ports to interconnect the "
-                       "integration bridge to physical networks. "
-                       "Support kernel without Open vSwitch patch port "
-                       "support so long as it is set to True.")),
     cfg.StrOpt('datapath_type', default=constants.OVS_DATAPATH_SYSTEM,
                choices=[constants.OVS_DATAPATH_SYSTEM,
                         constants.OVS_DATAPATH_NETDEV],

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

@@ -168,12 +168,6 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
                               datapath_type=ovs_conf.datapath_type)
             for b in ('br_int', 'br_phys', 'br_tun'))
 
-        self.use_veth_interconnection = ovs_conf.use_veth_interconnection
-        if self.use_veth_interconnection:
-            LOG.warning("Usage of veth instead of patch ports for bridges "
-                        "interconnection is deprecated in Victoria and will "
-                        "be removed in W release. Please use patch ports "
-                        "instead.")
         self.veth_mtu = agent_conf.veth_mtu
         self.available_local_vlans = set(range(n_const.MIN_VLAN_TAG,
                                                n_const.MAX_VLAN_TAG + 1))
@@ -1484,7 +1478,6 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
         self.int_ofports = {}
         self.phys_ofports = {}
         datapath_ids_set = set()
-        ip_wrapper = ip_lib.IPWrapper()
         ovs = ovs_lib.BaseOVS()
         ovs_bridges = ovs.get_bridges()
         for physical_network, bridge in bridge_mappings.items():
@@ -1522,45 +1515,31 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
             # be same, so check only one of them.
             # Not logging error here, as the interface may not exist yet.
             # Type check is done to cleanup wrong interface if any.
+
+            # TODO(slaweq) In X release we can remove code which is here just
+            # to move from old "veth" interconnection between bridges to the
+            # patch ports (L1527 - L1547)
             int_type = self.int_br.db_get_val("Interface", int_if_name, "type",
                                               log_errors=False)
-            if self.use_veth_interconnection:
+            # Drop ports if the interface type doesn't match the
-                # Drop ports if the interface types doesn't match the
+            # configuration value
-                # configuration value.
+            if int_type == 'veth':
-                if int_type == 'patch':
+                self.int_br.delete_port(int_if_name)
-                    self.int_br.delete_port(int_if_name)
+                br.delete_port(phys_if_name)
-                    br.delete_port(phys_if_name)
-                device = ip_lib.IPDevice(int_if_name)
-                if device.exists():
-                    device.link.delete()
-                    # Give udev a chance to process its rules here, to avoid
-                    # race conditions between commands launched by udev rules
-                    # and the subsequent call to ip_wrapper.add_veth
-                    utils.execute(['udevadm', 'settle', '--timeout=10'])
-                int_veth, phys_veth = ip_wrapper.add_veth(int_if_name,
-                                                          phys_if_name)
-                int_ofport = self.int_br.add_port(int_if_name)
-                phys_ofport = br.add_port(phys_if_name)
-            else:
-                # Drop ports if the interface type doesn't match the
-                # configuration value
-                if int_type == 'veth':
-                    self.int_br.delete_port(int_if_name)
-                    br.delete_port(phys_if_name)
 
-                # Setup int_br to physical bridge patches.  If they already
+            # Setup int_br to physical bridge patches.  If they already
-                # exist we leave them alone, otherwise we create them but don't
+            # exist we leave them alone, otherwise we create them but don't
-                # connect them until after the drop rules are in place.
+            # connect them until after the drop rules are in place.
-                if self.int_br.port_exists(int_if_name):
+            if self.int_br.port_exists(int_if_name):
-                    int_ofport = self.int_br.get_port_ofport(int_if_name)
+                int_ofport = self.int_br.get_port_ofport(int_if_name)
-                else:
+            else:
-                    int_ofport = self.int_br.add_patch_port(
+                int_ofport = self.int_br.add_patch_port(
-                        int_if_name, constants.NONEXISTENT_PEER)
+                    int_if_name, constants.NONEXISTENT_PEER)
-                if br.port_exists(phys_if_name):
+            if br.port_exists(phys_if_name):
-                    phys_ofport = br.get_port_ofport(phys_if_name)
+                phys_ofport = br.get_port_ofport(phys_if_name)
-                else:
+            else:
-                    phys_ofport = br.add_patch_port(
+                phys_ofport = br.add_patch_port(
-                        phys_if_name, constants.NONEXISTENT_PEER)
+                    phys_if_name, constants.NONEXISTENT_PEER)
 
             self.int_ofports[physical_network] = int_ofport
             self.phys_ofports[physical_network] = phys_ofport
@@ -1574,20 +1553,11 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
             if not self.enable_distributed_routing:
                 br.drop_port(in_port=phys_ofport)
 
-            if self.use_veth_interconnection:
+            # associate patch ports to pass traffic
-                # enable veth to pass traffic
+            self.int_br.set_db_attribute('Interface', int_if_name,
-                int_veth.link.set_up()
+                                         'options', {'peer': phys_if_name})
-                phys_veth.link.set_up()
+            br.set_db_attribute('Interface', phys_if_name,
-                if self.veth_mtu:
+                                'options', {'peer': int_if_name})
-                    # set up mtu size for veth interfaces
-                    int_veth.link.set_mtu(self.veth_mtu)
-                    phys_veth.link.set_mtu(self.veth_mtu)
-            else:
-                # associate patch ports to pass traffic
-                self.int_br.set_db_attribute('Interface', int_if_name,
-                                             'options', {'peer': phys_if_name})
-                br.set_db_attribute('Interface', phys_if_name,
-                                    'options', {'peer': int_if_name})
 
     def update_stale_ofport_rules(self):
         # ARP spoofing rules and drop-flow upon port-delete

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

@@ -1609,46 +1609,6 @@ class TestOvsNeutronAgent(object):
     def test_setup_physical_bridges_dvr_enabled(self):
         self._test_setup_physical_bridges(dvr_enabled=True)
 
-    def test_setup_physical_bridges_using_veth_interconnection(self):
-        self.agent.use_veth_interconnection = True
-        with mock.patch.object(ip_lib.IPDevice, "exists") as devex_fn,\
-                mock.patch.object(sys, "exit"),\
-                mock.patch.object(utils, "execute") as utilsexec_fn,\
-                mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\
-                mock.patch.object(self.agent, 'int_br') as int_br,\
-                mock.patch.object(self.agent, '_check_bridge_datapath_id'),\
-                mock.patch.object(ip_lib.IPWrapper, "add_veth") as addveth_fn,\
-                mock.patch.object(ip_lib.IpLinkCommand,
-                                  "delete") as linkdel_fn,\
-                mock.patch.object(ip_lib.IpLinkCommand, "set_up"),\
-                mock.patch.object(ip_lib.IpLinkCommand, "set_mtu"),\
-                mock.patch.object(ovs_lib.BaseOVS, "get_bridges") as get_br_fn:
-            devex_fn.return_value = True
-            parent = mock.MagicMock()
-            parent.attach_mock(utilsexec_fn, 'utils_execute')
-            parent.attach_mock(linkdel_fn, 'link_delete')
-            parent.attach_mock(addveth_fn, 'add_veth')
-            addveth_fn.return_value = (ip_lib.IPDevice("int-br-eth1"),
-                                       ip_lib.IPDevice("phy-br-eth1"))
-            phys_br = phys_br_cls()
-            phys_br.add_port.return_value = "phys_veth_ofport"
-            int_br.add_port.return_value = "int_veth_ofport"
-            get_br_fn.return_value = ["br-eth"]
-            self.agent.setup_physical_bridges({"physnet1": "br-eth"})
-            expected_calls = [mock.call.link_delete(),
-                              mock.call.utils_execute(['udevadm',
-                                                       'settle',
-                                                       '--timeout=10']),
-                              mock.call.add_veth('int-br-eth',
-                                                 'phy-br-eth')]
-            parent.assert_has_calls(expected_calls, any_order=False)
-            self.assertEqual("int_veth_ofport",
-                             self.agent.int_ofports["physnet1"])
-            self.assertEqual("phys_veth_ofport",
-                             self.agent.phys_ofports["physnet1"])
-            int_br.add_port.assert_called_with("int-br-eth")
-            phys_br.add_port.assert_called_with("phy-br-eth")
-
     def _test_setup_physical_bridges_change_from_veth_to_patch_conf(
             self, port_exists=False):
         with mock.patch.object(sys, "exit"),\

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

@@ -66,7 +66,6 @@ class DummyVlanBinding(object):
 
 
 class TunnelTest(object):
-    USE_VETH_INTERCONNECTION = False
     VETH_MTU = None
 
     def setUp(self):
@@ -312,8 +311,6 @@ class TunnelTest(object):
         cfg.CONF.set_override('tunnel_types', ['gre'], 'AGENT')
         cfg.CONF.set_override('veth_mtu', self.VETH_MTU, 'AGENT')
         cfg.CONF.set_override('minimize_polling', False, 'AGENT')
-        cfg.CONF.set_override('use_veth_interconnection',
-                              self.USE_VETH_INTERCONNECTION, 'OVS')
 
         for k, v in config_opts_agent.items():
             cfg.CONF.set_override(k, v, 'AGENT')
@@ -344,7 +341,6 @@ class TunnelTest(object):
         self._verify_mock_call(self.mock_aux_bridge,
                                self.mock_aux_bridge_expected)
         self._verify_mock_call(self.ipdevice, self.ipdevice_expected)
-        self._verify_mock_call(self.ipwrapper, self.ipwrapper_expected)
         self._verify_mock_call(self.get_bridges, self.get_bridges_expected)
         self._verify_mock_call(self.inta, self.inta_expected)
         self._verify_mock_call(self.intb, self.intb_expected)
@@ -673,127 +669,3 @@ class TunnelTest(object):
 
 class TunnelTestOSKen(TunnelTest, ovs_test_base.OVSOSKenTestBase):
     pass
-
-
-class TunnelTestUseVethInterco(TunnelTest):
-    USE_VETH_INTERCONNECTION = True
-
-    def _define_expected_calls(self, arp_responder=False, igmp_snooping=False):
-        self.mock_int_bridge_cls_expected = [
-            mock.call(self.INT_BRIDGE,
-                      datapath_type=mock.ANY),
-        ]
-        self.mock_phys_bridge_cls_expected = [
-            mock.call(self.MAP_TUN_BRIDGE,
-                      datapath_type=mock.ANY),
-        ]
-        self.mock_tun_bridge_cls_expected = [
-            mock.call(self.TUN_BRIDGE,
-                      datapath_type=mock.ANY),
-        ]
-
-        self.mock_int_bridge_expected = [
-            mock.call.create(),
-            mock.call.set_secure_mode(),
-            mock.call.setup_controllers(mock.ANY),
-            mock.call.set_igmp_snooping_state(igmp_snooping),
-            mock.call.setup_default_table(),
-        ]
-
-        self.mock_map_tun_bridge_expected = [
-            mock.call.create(),
-            mock.call.set_secure_mode(),
-            mock.call.setup_controllers(mock.ANY),
-            mock.call.setup_default_table(),
-            mock.call.add_port('phy-%s' % self.MAP_TUN_BRIDGE),
-        ]
-        self.mock_int_bridge_expected += [
-            mock.call.db_get_val('Interface', 'int-%s' % self.MAP_TUN_BRIDGE,
-                                 'type', log_errors=False),
-            mock.call.add_port('int-%s' % self.MAP_TUN_BRIDGE)
-        ]
-
-        self.mock_int_bridge_expected += [
-            mock.call.drop_port(in_port=self.MAP_TUN_INT_OFPORT),
-        ]
-        self.mock_map_tun_bridge_expected += [
-            mock.call.drop_port(in_port=self.MAP_TUN_PHY_OFPORT),
-        ]
-
-        self.mock_aux_bridge = self.ovs_bridges[self.AUX_BRIDGE]
-        self.mock_aux_bridge_expected = [
-        ]
-
-        self.mock_tun_bridge_expected = [
-            mock.call.create(secure_mode=True),
-            mock.call.setup_controllers(mock.ANY),
-            mock.call.port_exists('patch-int'),
-            mock.ANY,
-            mock.call.add_patch_port('patch-int', 'patch-tun'),
-        ]
-        self.mock_int_bridge_expected += [
-            mock.call.port_exists('patch-tun'),
-            mock.call.add_patch_port('patch-tun', 'patch-int')
-        ]
-        self.mock_int_bridge_expected += [
-            mock.call.get_vif_ports((ovs_lib.INVALID_OFPORT,
-                                     ovs_lib.UNASSIGNED_OFPORT)),
-            mock.call.get_ports_attributes(
-                'Port', columns=['name', 'other_config', 'tag'], ports=[])
-        ]
-        self.mock_tun_bridge_expected += [
-            mock.call.setup_default_table(self.INT_OFPORT, arp_responder),
-        ]
-
-        self.ipdevice_expected = [
-            mock.call('int-%s' % self.MAP_TUN_BRIDGE),
-            mock.call().exists(),
-            mock.ANY,
-            mock.call().link.delete()
-        ]
-        self.ipwrapper_expected = [
-            mock.call(),
-            mock.call().add_veth('int-%s' % self.MAP_TUN_BRIDGE,
-                                 'phy-%s' % self.MAP_TUN_BRIDGE)
-        ]
-
-        self.get_bridges_expected = [mock.call(), mock.call()]
-
-        self.inta_expected = [mock.call.link.set_up()]
-        self.intb_expected = [mock.call.link.set_up()]
-        self.execute_expected = [mock.call(['udevadm', 'settle',
-                                            '--timeout=10'])]
-
-        self.mock_int_bridge_expected += [
-            mock.call.install_goto(
-                dest_table_id=constants.LOCAL_MAC_DIRECT,
-                in_port=self.MAP_TUN_INT_OFPORT,
-                priority=4, table_id=constants.TRANSIENT_TABLE),
-            mock.call.install_goto(
-                dest_table_id=constants.LOCAL_MAC_DIRECT,
-                in_port=self.TUN_OFPORT,
-                priority=4, table_id=constants.TRANSIENT_TABLE),
-            mock.call.install_goto(
-                dest_table_id=constants.TRANSIENT_EGRESS_TABLE,
-                table_id=constants.LOCAL_MAC_DIRECT),
-        ]
-
-
-class TunnelTestUseVethIntercoOSKen(TunnelTestUseVethInterco,
-                                  ovs_test_base.OVSOSKenTestBase):
-    pass
-
-
-class TunnelTestWithMTU(TunnelTestUseVethInterco):
-    VETH_MTU = 1500
-
-    def _define_expected_calls(self, arp_responder=False, igmp_snooping=False):
-        super(TunnelTestWithMTU, self)._define_expected_calls(
-            arp_responder, igmp_snooping)
-        self.inta_expected.append(mock.call.link.set_mtu(self.VETH_MTU))
-        self.intb_expected.append(mock.call.link.set_mtu(self.VETH_MTU))
-
-
-class TunnelTestWithMTUOSKen(TunnelTestWithMTU,
-                           ovs_test_base.OVSOSKenTestBase):
-    pass