TrunkManager for the OVS agent

This patch introduces the TrunkManager for the OVS
agent. This class is responsible for wiring the trunk
and the subports.

Partially-implements: blueprint vlan-aware-vms
Co-Authored-By: Jakub Libosvar <libosvar@redhat.com>

Change-Id: I498560798983177ce7b64e1a8f32f1a157558897

neutron/services/trunk/drivers/openvswitch/agent/exceptions.py

@@ -0,0 +1,19 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from neutron_lib import exceptions as n_exc
+
+from neutron._i18n import _
+
+
+class TrunkBridgeNotFound(n_exc.NotFound):
+    message = _("Trunk bridge %(bridge)s could not be found.")

neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py

@@ -0,0 +1,254 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import contextlib
+
+from neutron_lib import constants
+from oslo_log import log as logging
+
+from neutron.agent.common import ovs_lib
+from neutron.services.trunk.drivers.openvswitch.agent import exceptions as exc
+from neutron.services.trunk import utils
+
+LOG = logging.getLogger(__name__)
+
+
+def get_br_int_port_name(prefix, port_id):
+    """Return the OVS port name for the given port ID.
+
+    The port name is the one that plumbs into the integration bridge.
+    """
+    return ("%si-%s" % (prefix, port_id))[:constants.DEVICE_NAME_MAX_LEN]
+
+
+def get_br_trunk_port_name(prefix, port_id):
+    """Return the OVS port name for the given port ID.
+
+    The port name is the one that plumbs into the trunk bridge.
+    """
+    return ("%st-%s" % (prefix, port_id))[:constants.DEVICE_NAME_MAX_LEN]
+
+
+def get_patch_peer_attrs(peer_name, port_mac=None, port_id=None):
+    external_ids = {}
+    if port_mac:
+        external_ids['attached-mac'] = port_mac
+    if port_id:
+        external_ids['iface-id'] = port_id
+    attrs = [('type', 'patch'),
+            ('options', {'peer': peer_name})]
+    if external_ids:
+        attrs.append(
+            ('external_ids', external_ids))
+    return attrs
+
+
+class TrunkBridge(ovs_lib.OVSBridge):
+
+    def __init__(self, trunk_id):
+        name = utils.gen_trunk_br_name(trunk_id)
+        super(TrunkBridge, self).__init__(name)
+
+    def exists(self):
+        return self.bridge_exists(self.br_name)
+
+
+class TrunkParentPort(object):
+    DEV_PREFIX = 'tp'
+
+    def __init__(self, trunk_id, port_id, port_mac=None):
+        self.trunk_id = trunk_id
+        self.port_id = port_id
+        self.port_mac = port_mac
+        self.bridge = TrunkBridge(self.trunk_id)
+        # The name has form of tpi-<hash>
+        self.patch_port_int_name = get_br_int_port_name(
+            self.DEV_PREFIX, port_id)
+        # The name has form of tpt-<hash>
+        self.patch_port_trunk_name = get_br_trunk_port_name(
+            self.DEV_PREFIX, port_id)
+        self._transaction = None
+
+    # TODO(jlibosva): Move nested transaction to ovs_lib
+    @contextlib.contextmanager
+    def ovsdb_transaction(self):
+        """Context manager for ovsdb transaction.
+
+        The object caches whether its already in transaction and if it is, the
+        original transaction is returned.  This behavior enables calling
+        manager several times while always getting the same transaction.
+        """
+        if self._transaction:
+            yield self._transaction
+        else:
+            with self.bridge.ovsdb.transaction() as txn:
+                self._transaction = txn
+                try:
+                    yield txn
+                finally:
+                    self._transaction = None
+
+    def plug(self, br_int):
+        """Create patch ports between trunk bridge and given bridge.
+
+        The method creates one patch port on the given bridge side using
+        port mac and id as external ids.  The other endpoint of patch port is
+        attached to the trunk bridge.  Everything is done in a single
+        ovsdb transaction so either all operations succeed or fail.
+
+        :param br_int: An integration bridge where peer endpoint of patch port
+                       will be created.
+
+        """
+        # NOTE(jlibosva): osvdb is an api so it doesn't matter whether we
+        # use self.bridge or br_int
+        ovsdb = self.bridge.ovsdb
+        patch_int_attrs = get_patch_peer_attrs(
+            self.patch_port_trunk_name, self.port_mac, self.port_id)
+        patch_trunk_attrs = get_patch_peer_attrs(self.patch_port_int_name)
+
+        with self.ovsdb_transaction() as txn:
+            txn.add(ovsdb.add_port(br_int.br_name,
+                                   self.patch_port_int_name))
+            txn.add(ovsdb.db_set('Interface', self.patch_port_int_name,
+                                 *patch_int_attrs))
+            txn.add(ovsdb.add_port(self.bridge.br_name,
+                                   self.patch_port_trunk_name))
+            txn.add(ovsdb.db_set('Interface', self.patch_port_trunk_name,
+                                 *patch_trunk_attrs))
+
+    def unplug(self, bridge):
+        """Unplug the trunk from bridge.
+
+        Method deletes in single ovsdb transaction the trunk bridge and patch
+        port on provided bridge.
+
+        :param bridge: Bridge that has peer side of patch port for this
+                       subport.
+        """
+        ovsdb = self.bridge.ovsdb
+        with self.ovsdb_transaction() as txn:
+            txn.add(ovsdb.del_br(self.bridge.br_name))
+            txn.add(ovsdb.del_port(self.patch_port_int_name,
+                                   bridge.br_name))
+
+
+class SubPort(TrunkParentPort):
+    # Patch port names have form of spi-<hash> or spt-<hash> respectively.
+    DEV_PREFIX = 'sp'
+
+    def __init__(self, trunk_id, port_id, port_mac=None, segmentation_id=None):
+        super(SubPort, self).__init__(trunk_id, port_id, port_mac)
+        self.segmentation_id = segmentation_id
+
+    def plug(self, br_int):
+        """Create patch ports between trunk bridge and given bridge.
+
+        The method creates one patch port on the given bridge side using
+        port mac and id as external ids.  The other endpoint of patch port is
+        attached to the trunk bridge.  Then it sets vlan tag represented by
+        segmentation_id.  Everything is done in a single ovsdb transaction so
+        either all operations succeed or fail.
+
+        :param br_int: An integration bridge where peer endpoint of patch port
+                       will be created.
+
+        """
+        ovsdb = self.bridge.ovsdb
+        with self.ovsdb_transaction() as txn:
+            super(SubPort, self).plug(br_int)
+            txn.add(ovsdb.db_set(
+                "Port", self.patch_port_trunk_name,
+                ("tag", self.segmentation_id)))
+
+    def unplug(self, bridge):
+        """Unplug the sub port from the bridge.
+
+        Method deletes in single ovsdb transaction both endpoints of patch
+        ports that represents the subport.
+
+        :param bridge: Bridge that has peer side of patch port for this
+                       subport.
+        """
+        ovsdb = self.bridge.ovsdb
+        with self.ovsdb_transaction() as txn:
+            txn.add(ovsdb.del_port(self.patch_port_trunk_name,
+                                   self.bridge.br_name))
+            txn.add(ovsdb.del_port(self.patch_port_int_name,
+                                   bridge.br_name))
+
+
+class TrunkManager(object):
+
+    def __init__(self, br_int):
+        self.br_int = br_int
+
+    def create_trunk(self, trunk_id, port_id, port_mac):
+        """Create the trunk.
+
+        This patches the bridge for trunk_id with the integration bridge
+        by means of parent port identified by port_id.
+
+        :param trunk_id: ID of the trunk.
+        :param port_id: ID of the parent port.
+        :param port_mac: the MAC address of the parent port.
+        :raises: TrunkBridgeNotFound -- In case trunk bridge doesn't exist.
+
+        """
+        trunk = TrunkParentPort(trunk_id, port_id, port_mac)
+        if not trunk.bridge.exists():
+            raise exc.TrunkBridgeNotFound(bridge=trunk.bridge.br_name)
+        # Once the bridges are connected with the following patch ports,
+        # the ovs agent will recognize the ports for processing and it will
+        # take over the wiring process and everything that entails.
+        # REVISIT(rossella_s): revisit this integration part, should tighter
+        # control over the wiring logic for trunk ports be required.
+        trunk.plug(self.br_int)
+
+    def remove_trunk(self, trunk_id, port_id):
+        """Remove the trunk bridge."""
+        trunk = TrunkParentPort(trunk_id, port_id)
+        if trunk.bridge.exists():
+            trunk.unplug(self.br_int)
+        else:
+            LOG.debug("Trunk bridge with ID %s doesn't exist.", trunk_id)
+
+    def add_sub_port(self, trunk_id, port_id, port_mac, segmentation_id):
+        """Create a sub_port.
+
+        :param trunk_id: ID of the trunk
+        :param port_id: ID of the child port
+        :param segmentation_id: segmentation ID associated with this sub-port
+        :param port_mac: MAC address of the child port
+
+        """
+        sub_port = SubPort(trunk_id, port_id, port_mac, segmentation_id)
+        # If creating of parent trunk bridge takes longer than API call for
+        # creating subport then bridge doesn't exist yet.
+        if not sub_port.bridge.exists():
+            raise exc.TrunkBridgeNotFound(bridge=sub_port.bridge.br_name)
+        sub_port.plug(self.br_int)
+
+    def remove_sub_port(self, trunk_id, port_id):
+        """Remove a sub_port.
+
+        :param trunk_id: ID of the trunk
+        :param port_id: ID of the child port
+        """
+        sub_port = SubPort(trunk_id, port_id)
+
+        # Trunk bridge might have been deleted by calling delete_trunk() before
+        # remove_sub_port().
+        if sub_port.bridge.exists():
+            sub_port.unplug(self.br_int)
+        else:
+            LOG.debug("Trunk bridge with ID %s doesn't exist.", trunk_id)

neutron/tests/common/conn_testers.py

@@ -64,6 +64,7 @@ class ConnectionTester(fixtures.Fixture):
     ARP = n_consts.ETHERTYPE_NAME_ARP
     INGRESS = firewall.INGRESS_DIRECTION
     EGRESS = firewall.EGRESS_DIRECTION
+    ICMP_COUNT = 1
 
     def __init__(self, ip_cidr):
         self.ip_cidr = ip_cidr
@@ -157,7 +158,8 @@ class ConnectionTester(fixtures.Fixture):
         icmp_timeout = ICMP_VERSION_TIMEOUTS[ip_version]
         try:
             net_helpers.assert_ping(src_namespace, ip_address,
-                                    timeout=icmp_timeout)
+                                    timeout=icmp_timeout,
+                                    count=self.ICMP_COUNT)
         except RuntimeError:
             raise ConnectionTesterException(
                 "ICMP packets can't get from %s namespace to %s address" % (
@@ -317,7 +319,28 @@ class ConnectionTester(fixtures.Fixture):
                 'sending icmp packets to %s' % destination)
 
 
-class OVSConnectionTester(ConnectionTester):
+class OVSBaseConnectionTester(ConnectionTester):
+
+    @property
+    def peer_port_id(self):
+        return self._peer.port.id
+
+    @property
+    def vm_port_id(self):
+        return self._vm.port.id
+
+    @staticmethod
+    def set_tag(port_name, bridge, tag):
+        bridge.set_db_attribute('Port', port_name, 'tag', tag)
+        other_config = bridge.db_get_val(
+            'Port', port_name, 'other_config')
+        other_config['tag'] = tag
+        bridge.set_db_attribute(
+            'Port', port_name, 'other_config', other_config)
+
+
+class OVSConnectionTester(OVSBaseConnectionTester):
+
     """Tester with OVS bridge in the middle
 
     The endpoints are created as OVS ports attached to the OVS bridge.
@@ -347,27 +370,103 @@ class OVSConnectionTester(ConnectionTester):
         for column, value in attrs:
             self.bridge.set_db_attribute('Interface', port.name, column, value)
 
-    @property
-    def peer_port_id(self):
-        return self._peer.port.id
+    def set_vm_tag(self, tag):
+        self.set_tag(self._vm.port.name, self.bridge, tag)
 
-    @property
-    def vm_port_id(self):
-        return self._vm.port.id
+    def set_peer_tag(self, tag):
+        self.set_tag(self._peer.port.name, self.bridge, tag)
 
-    def set_tag(self, port_name, tag):
-        self.bridge.set_db_attribute('Port', port_name, 'tag', tag)
-        other_config = self.bridge.db_get_val(
-            'Port', port_name, 'other_config')
-        other_config['tag'] = tag
-        self.bridge.set_db_attribute(
-            'Port', port_name, 'other_config', other_config)
+
+class OVSTrunkConnectionTester(OVSBaseConnectionTester):
+    """Tester with OVS bridge and a trunk bridge
+
+    Two endpoints: one is a VM that is connected to a port associated with a
+    trunk (the port is created  on the trunk bridge), the other is a VM on the
+    same network (the port is on the integration bridge).
+
+    NOTE: The OVS ports are connected from the namespace. This connection is
+    currently not supported in OVS and may lead to unpredicted behavior:
+    https://bugzilla.redhat.com/show_bug.cgi?id=1160340
+
+    """
+    ICMP_COUNT = 3
+
+    def __init__(self, ip_cidr, br_trunk_name):
+        super(OVSTrunkConnectionTester, self).__init__(ip_cidr)
+        self._br_trunk_name = br_trunk_name
+
+    def _setUp(self):
+        super(OVSTrunkConnectionTester, self)._setUp()
+        self.bridge = self.useFixture(
+            net_helpers.OVSBridgeFixture()).bridge
+        self.br_trunk = self.useFixture(
+            net_helpers.OVSTrunkBridgeFixture(self._br_trunk_name)).bridge
+        self._peer = self.useFixture(machine_fixtures.FakeMachine(
+                self.bridge, self.ip_cidr))
+        ip_cidr = net_helpers.increment_ip_cidr(self.ip_cidr, 1)
+
+        self._vm = self.useFixture(machine_fixtures.FakeMachine(
+            self.br_trunk, ip_cidr))
+
+    def add_vlan_interface_and_peer(self, vlan, ip_cidr):
+        """Create a sub_port and a peer
+
+        We create a sub_port that uses vlan as segmentation ID. In the vm
+        namespace we create a vlan subinterface on the same vlan.
+        A peer on the same network is created. When pinging from the peer
+        to the sub_port packets will be tagged using the internal vlan ID
+        of the network. The sub_port will remove that vlan tag and push the
+        vlan specified in the segmentation ID. The packets will finally reach
+        the vlan subinterface in the vm namespace.
+
+        """
+
+        ip_wrap = ip_lib.IPWrapper(self._vm.namespace)
+        dev_name = self._vm.port.name + ".%d" % vlan
+        ip_wrap.add_vlan(dev_name, self._vm.port.name, vlan)
+        dev = ip_wrap.device(dev_name)
+        dev.addr.add(ip_cidr)
+        dev.link.set_up()
+        self._ip_vlan = ip_cidr.partition('/')[0]
+        ip_cidr = net_helpers.increment_ip_cidr(ip_cidr, 1)
+        self._peer2 = self.useFixture(machine_fixtures.FakeMachine(
+            self.bridge, ip_cidr))
 
     def set_vm_tag(self, tag):
-        self.set_tag(self._vm.port.name, tag)
+        self.set_tag(self._vm.port.name, self.br_trunk, tag)
 
     def set_peer_tag(self, tag):
-        self.set_tag(self._peer.port.name, tag)
+        self.set_tag(self._peer.port.name, self.bridge, tag)
+
+    def _get_subport_namespace_and_address(self, direction):
+        if direction == self.INGRESS:
+            return self._peer2.namespace, self._ip_vlan
+        return self._vm.namespace, self._peer2.ip
+
+    def test_sub_port_icmp_connectivity(self, direction):
+
+        src_namespace, ip_address = self._get_subport_namespace_and_address(
+            direction)
+        ip_version = ip_lib.get_ip_version(ip_address)
+        icmp_timeout = ICMP_VERSION_TIMEOUTS[ip_version]
+        try:
+            net_helpers.assert_ping(src_namespace, ip_address,
+                                    timeout=icmp_timeout,
+                                    count=self.ICMP_COUNT)
+        except RuntimeError:
+            raise ConnectionTesterException(
+                "ICMP packets can't get from %s namespace to %s address" % (
+                    src_namespace, ip_address))
+
+    def test_sub_port_icmp_no_connectivity(self, direction):
+        try:
+            self.test_sub_port_icmp_connectivity(direction)
+        except ConnectionTesterException:
+            pass
+        else:
+            raise ConnectionTesterException(
+                'Established %s connection with protocol ICMP, ' % (
+                    direction))
 
 
 class LinuxBridgeConnectionTester(ConnectionTester):

neutron/tests/common/helpers.py

@@ -14,6 +14,7 @@
 
 import datetime
 import os
+import random
 
 from neutron_lib import constants
 from oslo_utils import timeutils
@@ -212,3 +213,11 @@ def requires_py2(testcase):
 
 def requires_py3(testcase):
     return testtools.skipUnless(six.PY3, "requires python 3.x")(testcase)
+
+
+def get_not_used_vlan(bridge, vlan_range):
+    port_vlans = bridge.ovsdb.db_find(
+        'Port', ('tag', '!=', []), columns=['tag']).execute()
+    used_vlan_tags = {val['tag'] for val in port_vlans}
+    available_vlans = vlan_range - used_vlan_tags
+    return random.choice(list(available_vlans))

neutron/tests/common/net_helpers.py

@@ -685,6 +685,14 @@ class OVSBridgeFixture(fixtures.Fixture):
         self.addCleanup(self.bridge.destroy)
 
 
+class OVSTrunkBridgeFixture(OVSBridgeFixture):
+    """This bridge doesn't generate the name."""
+    def _setUp(self):
+        ovs = ovs_lib.BaseOVS()
+        self.bridge = ovs.add_bridge(self.prefix)
+        self.addCleanup(self.bridge.destroy)
+
+
 class OVSPortFixture(PortFixture):
     NIC_NAME_LEN = 14
 

neutron/tests/functional/agent/test_firewall.py

@@ -19,7 +19,6 @@
 
 import copy
 import functools
-import random
 
 import netaddr
 from neutron_lib import constants
@@ -33,8 +32,10 @@ from neutron.agent.linux import openvswitch_firewall
 from neutron.agent import securitygroups_rpc as sg_cfg
 from neutron.cmd.sanity import checks
 from neutron.tests.common import conn_testers
+from neutron.tests.common import helpers
 from neutron.tests.functional.agent.linux import base as linux_base
 from neutron.tests.functional import base
+from neutron.tests.functional import constants as test_constants
 
 LOG = logging.getLogger(__name__)
 
@@ -50,7 +51,6 @@ reverse_transport_protocol = {
     conn_testers.ConnectionTester.UDP: conn_testers.ConnectionTester.TCP}
 
 DEVICE_OWNER_COMPUTE = constants.DEVICE_OWNER_COMPUTE_PREFIX + 'fake'
-VLAN_COUNT = 4096
 
 
 def skip_if_firewall(firewall_name):
@@ -91,7 +91,7 @@ class BaseFirewallTestCase(base.BaseSudoTestCase):
     scenarios = scenarios_iptables + scenarios_ovs_fw_interfaces
 
     ip_cidr = None
-    vlan_range = set(range(VLAN_COUNT))
+    vlan_range = set(range(test_constants.VLAN_COUNT))
 
     def setUp(self):
         cfg.CONF.register_opts(sg_cfg.security_group_opts, 'SECURITYGROUP')
@@ -133,18 +133,12 @@ class BaseFirewallTestCase(base.BaseSudoTestCase):
         return tester, firewall_drv
 
     def assign_vlan_to_peers(self):
-        vlan = self.get_not_used_vlan()
+        vlan = helpers.get_not_used_vlan(self.firewall.int_br.br,
+                                         self.vlan_range)
         LOG.debug("Using %d vlan tag for this test", vlan)
         self.tester.set_vm_tag(vlan)
         self.tester.set_peer_tag(vlan)
 
-    def get_not_used_vlan(self):
-        port_vlans = self.firewall.int_br.br.ovsdb.db_find(
-            'Port', ('tag', '!=', []), columns=['tag']).execute()
-        used_vlan_tags = {val['tag'] for val in port_vlans}
-        available_vlans = self.vlan_range - used_vlan_tags
-        return random.choice(list(available_vlans))
-
     @staticmethod
     def _create_port_description(port_id, ip_addresses, mac_address, sg_ids):
         return {'admin_state_up': True,

neutron/tests/functional/constants.py

@@ -0,0 +1,13 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+VLAN_COUNT = 4096

neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py

@@ -0,0 +1,231 @@
+# All Rights Reserved.
+
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import mock
+
+from oslo_log import log as logging
+from oslo_utils import uuidutils
+import testtools
+
+from neutron.common import utils as common_utils
+from neutron.services.trunk.drivers.openvswitch.agent import trunk_manager
+from neutron.services.trunk import utils
+from neutron.tests.common import conn_testers
+from neutron.tests.common import helpers
+from neutron.tests.common import net_helpers
+from neutron.tests.functional import base
+from neutron.tests.functional import constants as test_constants
+
+LOG = logging.getLogger(__name__)
+
+VLAN_RANGE = set(range(test_constants.VLAN_COUNT))
+
+
+class FakeOVSDBException(Exception):
+    pass
+
+
+class TrunkParentPortTestCase(base.BaseSudoTestCase):
+    def setUp(self):
+        super(TrunkParentPortTestCase, self).setUp()
+        trunk_id = uuidutils.generate_uuid()
+        port_id = uuidutils.generate_uuid()
+        port_mac = common_utils.get_random_mac('fa:16:3e:00:00:00'.split(':'))
+        self.trunk = trunk_manager.TrunkParentPort(trunk_id, port_id, port_mac)
+        self.trunk.bridge = self.useFixture(
+            net_helpers.OVSTrunkBridgeFixture(
+                self.trunk.bridge.br_name)).bridge
+        self.br_int = self.useFixture(net_helpers.OVSBridgeFixture()).bridge
+
+    def test_plug(self):
+        self.trunk.plug(self.br_int)
+        self.assertIn(self.trunk.patch_port_trunk_name,
+                      self.trunk.bridge.get_port_name_list())
+        self.assertIn(self.trunk.patch_port_int_name,
+                      self.br_int.get_port_name_list())
+
+    def test_plug_failure_doesnt_create_ports(self):
+        with mock.patch.object(
+                self.trunk.bridge.ovsdb, 'db_set',
+                side_effect=FakeOVSDBException):
+            with testtools.ExpectedException(FakeOVSDBException):
+                self.trunk.plug(self.br_int)
+        self.assertNotIn(self.trunk.patch_port_trunk_name,
+                         self.trunk.bridge.get_port_name_list())
+        self.assertNotIn(self.trunk.patch_port_int_name,
+                         self.br_int.get_port_name_list())
+
+    def test_unplug(self):
+        self.trunk.plug(self.br_int)
+        self.trunk.unplug(self.br_int)
+        self.assertFalse(
+            self.trunk.bridge.bridge_exists(self.trunk.bridge.br_name))
+        self.assertNotIn(self.trunk.patch_port_int_name,
+                         self.br_int.get_port_name_list())
+
+    def test_unplug_failure_doesnt_delete_bridge(self):
+        self.trunk.plug(self.br_int)
+        with mock.patch.object(
+                self.trunk.bridge.ovsdb, 'del_port',
+                side_effect=FakeOVSDBException):
+            with testtools.ExpectedException(FakeOVSDBException):
+                self.trunk.unplug(self.br_int)
+        self.assertTrue(
+            self.trunk.bridge.bridge_exists(self.trunk.bridge.br_name))
+        self.assertIn(self.trunk.patch_port_trunk_name,
+                      self.trunk.bridge.get_port_name_list())
+        self.assertIn(self.trunk.patch_port_int_name,
+                      self.br_int.get_port_name_list())
+
+
+class SubPortTestCase(base.BaseSudoTestCase):
+    def setUp(self):
+        super(SubPortTestCase, self).setUp()
+        trunk_id = uuidutils.generate_uuid()
+        port_id = uuidutils.generate_uuid()
+        port_mac = common_utils.get_random_mac('fa:16:3e:00:00:00'.split(':'))
+        trunk_bridge_name = utils.gen_trunk_br_name(trunk_id)
+        trunk_bridge = self.useFixture(
+            net_helpers.OVSTrunkBridgeFixture(trunk_bridge_name)).bridge
+        segmentation_id = helpers.get_not_used_vlan(
+            trunk_bridge, VLAN_RANGE)
+        self.subport = trunk_manager.SubPort(
+            trunk_id, port_id, port_mac, segmentation_id)
+        self.subport.bridge = trunk_bridge
+        self.br_int = self.useFixture(net_helpers.OVSBridgeFixture()).bridge
+
+    def test_plug(self):
+        self.subport.plug(self.br_int)
+        self.assertIn(self.subport.patch_port_trunk_name,
+                      self.subport.bridge.get_port_name_list())
+        self.assertIn(self.subport.patch_port_int_name,
+                      self.br_int.get_port_name_list())
+        self.assertEqual(
+            self.subport.segmentation_id,
+            self.subport.bridge.db_get_val(
+                'Port', self.subport.patch_port_trunk_name, 'tag'))
+
+    def test_plug_failure_doesnt_create_ports(self):
+        with mock.patch.object(
+                self.subport.bridge.ovsdb, 'db_set',
+                side_effect=FakeOVSDBException):
+            with testtools.ExpectedException(FakeOVSDBException):
+                self.subport.plug(self.br_int)
+        self.assertNotIn(self.subport.patch_port_trunk_name,
+                         self.subport.bridge.get_port_name_list())
+        self.assertNotIn(self.subport.patch_port_int_name,
+                         self.br_int.get_port_name_list())
+
+    def test_unplug(self):
+        self.subport.plug(self.br_int)
+        self.subport.unplug(self.br_int)
+        self.assertNotIn(self.subport.patch_port_trunk_name,
+                         self.subport.bridge.get_port_name_list())
+        self.assertNotIn(self.subport.patch_port_int_name,
+                         self.br_int.get_port_name_list())
+
+    def test_unplug_failure(self):
+        self.subport.plug(self.br_int)
+        with mock.patch.object(
+                self.subport.bridge.ovsdb, 'del_port',
+                side_effect=FakeOVSDBException):
+            with testtools.ExpectedException(FakeOVSDBException):
+                self.subport.unplug(self.br_int)
+        self.assertIn(self.subport.patch_port_trunk_name,
+                      self.subport.bridge.get_port_name_list())
+        self.assertIn(self.subport.patch_port_int_name,
+                      self.br_int.get_port_name_list())
+
+
+class TrunkManagerTestCase(base.BaseSudoTestCase):
+    net1_cidr = '192.178.0.1/24'
+    net2_cidr = '192.168.0.1/24'
+
+    def setUp(self):
+        super(TrunkManagerTestCase, self).setUp()
+        trunk_id = uuidutils.generate_uuid()
+        self.tester = self.useFixture(
+            conn_testers.OVSTrunkConnectionTester(
+                self.net1_cidr, utils.gen_trunk_br_name(trunk_id)))
+        self.trunk_manager = trunk_manager.TrunkManager(
+            self.tester.bridge)
+        self.trunk = trunk_manager.TrunkParentPort(
+            trunk_id, uuidutils.generate_uuid())
+
+    # TODO(jlibosva): Replace all tester methods with more robust tests
+    def test_connectivity(self):
+        """Test connectivity with trunk and sub ports.
+
+        In this test we create a vm that has a trunk on net1 and a vm peer on
+        the same network. We check connectivity between the peer and the vm.
+        We create a sub port on net2 and a peer, check connectivity again.
+
+        """
+        vlan_net1 = helpers.get_not_used_vlan(self.tester.bridge, VLAN_RANGE)
+        vlan_net2 = helpers.get_not_used_vlan(self.tester.bridge, VLAN_RANGE)
+        trunk_mac = common_utils.get_random_mac('fa:16:3e:00:00:00'.split(':'))
+        sub_port_mac = common_utils.get_random_mac(
+            'fa:16:3e:00:00:00'.split(':'))
+        sub_port_segmentation_id = helpers.get_not_used_vlan(
+            self.tester.bridge, VLAN_RANGE)
+        LOG.debug("Using %(n1)d vlan tag as local vlan ID for net1 and %(n2)d "
+                  "for local vlan ID for net2", {
+                      'n1': vlan_net1, 'n2': vlan_net2})
+        self.tester.set_peer_tag(vlan_net1)
+        self.trunk_manager.create_trunk(self.trunk.trunk_id,
+                                        self.trunk.port_id,
+                                        trunk_mac)
+
+        # tag the patch port, this should be done by the ovs agent but we mock
+        # it for this test
+        conn_testers.OVSBaseConnectionTester.set_tag(
+            self.trunk.patch_port_int_name, self.tester.bridge, vlan_net1)
+
+        self.tester.assert_connection(protocol=self.tester.ICMP,
+                                      direction=self.tester.INGRESS)
+        self.tester.assert_connection(protocol=self.tester.ICMP,
+                                      direction=self.tester.EGRESS)
+
+        self.tester.add_vlan_interface_and_peer(sub_port_segmentation_id,
+                                                self.net2_cidr)
+        conn_testers.OVSBaseConnectionTester.set_tag(
+            self.tester._peer2.port.name, self.tester.bridge, vlan_net2)
+
+        sub_port = trunk_manager.SubPort(self.trunk.trunk_id,
+                                         uuidutils.generate_uuid(),
+                                         sub_port_mac,
+                                         sub_port_segmentation_id)
+
+        self.trunk_manager.add_sub_port(sub_port.trunk_id,
+                                        sub_port.port_id,
+                                        sub_port.port_mac,
+                                        sub_port.segmentation_id)
+        # tag the patch port, this should be done by the ovs agent but we mock
+        # it for this test
+        conn_testers.OVSBaseConnectionTester.set_tag(
+            sub_port.patch_port_int_name, self.tester.bridge, vlan_net2)
+
+        self.tester.test_sub_port_icmp_connectivity(self.tester.INGRESS)
+        self.tester.test_sub_port_icmp_connectivity(self.tester.EGRESS)
+
+        self.trunk_manager.remove_sub_port(sub_port.trunk_id,
+                                           sub_port.port_id)
+        self.tester.test_sub_port_icmp_no_connectivity(self.tester.INGRESS)
+        self.tester.test_sub_port_icmp_no_connectivity(self.tester.EGRESS)
+
+        self.trunk_manager.remove_trunk(self.trunk.trunk_id,
+                                        self.trunk.port_id)
+        self.tester.assert_no_connection(protocol=self.tester.ICMP,
+                                         direction=self.tester.INGRESS)

neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py

@@ -0,0 +1,67 @@
+# Copyright (c) 2016 Red Hat
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import mock
+
+from oslo_utils import uuidutils
+import testtools
+
+from neutron.common import utils as common_utils
+from neutron.services.trunk.drivers.openvswitch.agent import trunk_manager
+from neutron.tests import base
+
+NATIVE_OVSDB_CONNECTION = (
+    'neutron.agent.ovsdb.impl_idl.OvsdbIdl.ovsdb_connection')
+
+
+class TrunkParentPortTestCase(base.BaseTestCase):
+    def setUp(self):
+        super(TrunkParentPortTestCase, self).setUp()
+        # Mock out connecting to ovsdb
+        mock.patch(NATIVE_OVSDB_CONNECTION).start()
+        trunk_id = uuidutils.generate_uuid()
+        port_id = uuidutils.generate_uuid()
+        trunk_mac = common_utils.get_random_mac('fa:16:3e:00:00:00'.split(':'))
+        self.trunk = trunk_manager.TrunkParentPort(
+            trunk_id, port_id, trunk_mac)
+
+    def test_multiple_transactions(self):
+        def method_inner(trunk):
+            with trunk.ovsdb_transaction() as txn:
+                return id(txn)
+
+        def method_outer(trunk):
+            with trunk.ovsdb_transaction() as txn:
+                return method_inner(trunk), id(txn)
+
+        with self.trunk.ovsdb_transaction() as txn1:
+            mock_commit = mock.patch.object(txn1, 'commit').start()
+            txn_inner_id, txn_outer_id = method_outer(self.trunk)
+            self.assertFalse(mock_commit.called)
+        self.assertTrue(mock_commit.called)
+        self.assertTrue(id(txn1) == txn_inner_id == txn_outer_id)
+
+    def test_transaction_raises_error(self):
+        class MyException(Exception):
+            pass
+
+        with testtools.ExpectedException(MyException):
+            with self.trunk.ovsdb_transaction() as txn1:
+                mock.patch.object(txn1, 'commit').start()
+                raise MyException()
+        self.assertIsNone(self.trunk._transaction)
+        with self.trunk.ovsdb_transaction() as txn2:
+            mock.patch.object(txn2, 'commit').start()
+            self.assertIsNot(txn1, txn2)