added net client to consolidate bridge and interface creation

.editorconfig

@@ -11,6 +11,7 @@ insert_final_newline = true
 [*.py]
 indent_style = space
 indent_size = 4
+max_line_length = 88
 
 [*.am]
 indent_style = tab

daemon/core/emulator/coreemu.py

@@ -44,7 +44,7 @@ class CoreEmu(object):
         os.umask(0)
 
         # configuration
-        if not config:
+        if config is None:
             config = {}
         self.config = config
 

daemon/core/nodes/base.py

@@ -19,6 +19,7 @@ from core.emulator.data import LinkData, NodeData
 from core.emulator.enumerations import LinkTypes, NodeTypes
 from core.nodes import client, ipaddress
 from core.nodes.interface import CoreInterface, TunTap, Veth
+from core.nodes.netclient import BrctlClient, OvsClient
 
 _DEFAULT_MTU = 1500
 
@@ -1064,6 +1065,10 @@ class CoreNetworkBase(NodeBase):
         super(CoreNetworkBase, self).__init__(session, _id, name, start=start)
         self._linked = {}
         self._linked_lock = threading.Lock()
+        if session.options.get_config("ovs"):
+            self.net_client = OvsClient()
+        else:
+            self.net_client = BrctlClient()
 
     def startup(self):
         """

daemon/core/nodes/netclient.py

@@ -0,0 +1,74 @@
+"""
+Clients for dealing with bridge and interface commands.
+"""
+
+import abc
+import os
+
+from core import constants, utils
+
+
+class NetworkClient(object):
+    __metaclass__ = abc.ABCMeta
+
+    @abc.abstractmethod
+    def create_bridge(self, name):
+        return NotImplemented
+
+
+class BrctlClient(object):
+    def create_bridge(self, name):
+        utils.check_cmd([constants.BRCTL_BIN, "addbr", name])
+        # disable spanning tree protocol and set forward delay to 0
+        utils.check_cmd([constants.BRCTL_BIN, "stp", name, "off"])
+        utils.check_cmd([constants.BRCTL_BIN, "setfd", name, "0"])
+        utils.check_cmd([constants.IP_BIN, "link", "set", name, "up"])
+
+        # turn off multicast snooping so forwarding occurs w/o IGMP joins
+        snoop = "/sys/devices/virtual/net/%s/bridge/multicast_snooping" % name
+        if os.path.exists(snoop):
+            with open(snoop, "w") as f:
+                f.write("0")
+
+    def delete_bridge(self, name):
+        utils.check_cmd([constants.IP_BIN, "link", "set", name, "down"])
+        utils.check_cmd([constants.BRCTL_BIN, "delbr", name])
+
+    def create_interface(self, bridge_name, interface_name):
+        utils.check_cmd([constants.BRCTL_BIN, "addif", bridge_name, interface_name])
+        utils.check_cmd([constants.IP_BIN, "link", "set", interface_name, "up"])
+
+    def delete_interface(self, bridge_name, interface_name):
+        utils.check_cmd([constants.BRCTL_BIN, "delif", bridge_name, interface_name])
+
+    def get_bridges(self):
+        return utils.check_cmd([constants.OVS_BIN, "list-br"])
+
+    def disable_mac_learning(self, name):
+        utils.check_cmd([constants.BRCTL_BIN, "setageing", name, "0"])
+
+
+class OvsClient(object):
+    def create_bridge(self, name):
+        # turn off spanning tree protocol and forwarding delay
+        # TODO: verify stp and rstp are always off by default
+        # TODO: ovs only supports rstp forward delay and again it's off by default
+        utils.check_cmd([constants.OVS_BIN, "add-br", name])
+        utils.check_cmd([constants.IP_BIN, "link", "set", name, "up"])
+
+    def delete_bridge(self, name):
+        utils.check_cmd([constants.IP_BIN, "link", "set", name, "down"])
+        utils.check_cmd([constants.OVS_BIN, "del-br", name])
+
+    def create_interface(self, bridge_name, interface_name):
+        utils.check_cmd([constants.OVS_BIN, "add-port", bridge_name, interface_name])
+        utils.check_cmd([constants.IP_BIN, "link", "set", interface_name, "up"])
+
+    def delete_interface(self, bridge_name, interface_name):
+        utils.check_cmd([constants.OVS_BIN, "del-port", bridge_name, interface_name])
+
+    def get_bridges(self):
+        utils.check_cmd([constants.BRCTL_BIN, "show"])
+
+    def disable_mac_learning(self, name):
+        pass

daemon/core/nodes/network.py

@@ -314,12 +314,8 @@ class CoreNetwork(CoreNetworkBase):
         :return: nothing
         :raises CoreCommandError: when there is a command exception
         """
-        utils.check_cmd([constants.BRCTL_BIN, "addbr", self.brname])
+        self.net_client.create_bridge(self.brname)
 
-        # turn off spanning tree protocol and forwarding delay
-        utils.check_cmd([constants.BRCTL_BIN, "stp", self.brname, "off"])
-        utils.check_cmd([constants.BRCTL_BIN, "setfd", self.brname, "0"])
-        utils.check_cmd([constants.IP_BIN, "link", "set", self.brname, "up"])
         # create a new ebtables chain for this bridge
         ebtablescmds(
             utils.check_cmd,
@@ -336,11 +332,6 @@ class CoreNetwork(CoreNetworkBase):
                 ],
             ],
         )
-        # turn off multicast snooping so mcast forwarding occurs w/o IGMP joins
-        snoop = "/sys/devices/virtual/net/%s/bridge/multicast_snooping" % self.brname
-        if os.path.exists(snoop):
-            with open(snoop, "w") as snoop_file:
-                snoop_file.write("0")
 
         self.up = True
 
@@ -356,8 +347,7 @@ class CoreNetwork(CoreNetworkBase):
         ebq.stopupdateloop(self)
 
         try:
-            utils.check_cmd([constants.IP_BIN, "link", "set", self.brname, "down"])
+            self.net_client.delete_bridge(self.brname)
-            utils.check_cmd([constants.BRCTL_BIN, "delbr", self.brname])
             ebtablescmds(
                 utils.check_cmd,
                 [
@@ -385,7 +375,8 @@ class CoreNetwork(CoreNetworkBase):
         del self.session
         self.up = False
 
-    # TODO: this depends on a subtype with localname defined, seems like the wrong place for this to live
+    # TODO: this depends on a subtype with localname defined, seems like the
+    #  wrong place for this to live
     def attach(self, netif):
         """
         Attach a network interface.
@@ -394,10 +385,7 @@ class CoreNetwork(CoreNetworkBase):
         :return: nothing
         """
         if self.up:
-            utils.check_cmd(
+            self.net_client.create_interface(self.brname, netif.localname)
-                [constants.BRCTL_BIN, "addif", self.brname, netif.localname]
-            )
-            utils.check_cmd([constants.IP_BIN, "link", "set", netif.localname, "up"])
 
         CoreNetworkBase.attach(self, netif)
 
@@ -409,9 +397,7 @@ class CoreNetwork(CoreNetworkBase):
         :return: nothing
         """
         if self.up:
-            utils.check_cmd(
+            self.net_client.delete_interface(self.brname, netif.localname)
-                [constants.BRCTL_BIN, "delif", self.brname, netif.localname]
-            )
 
         CoreNetworkBase.detach(self, netif)
 
@@ -610,10 +596,8 @@ class CoreNetwork(CoreNetworkBase):
         )
         self.attach(netif)
         if net.up:
-            # this is similar to net.attach() but uses netif.name instead
+            # this is similar to net.attach() but uses netif.name instead of localname
-            # of localname
+            self.net_client.create_interface(net.brname, netif.name)
-            utils.check_cmd([constants.BRCTL_BIN, "addif", net.brname, netif.name])
-            utils.check_cmd([constants.IP_BIN, "link", "set", netif.name, "up"])
         i = net.newifindex()
         net._netif[i] = netif
         with net._linked_lock:
@@ -848,41 +832,32 @@ class CtrlNet(CoreNetwork):
             utils.check_cmd([self.updown_script, self.brname, "startup"])
 
         if self.serverintf:
-            # sets the interface as a port of the bridge
+            self.net_client.create_interface(self.brname, self.serverintf)
-            utils.check_cmd(
-                [constants.BRCTL_BIN, "addif", self.brname, self.serverintf]
-            )
-
-            # bring interface up
-            utils.check_cmd([constants.IP_BIN, "link", "set", self.serverintf, "up"])
 
     def detectoldbridge(self):
         """
-        Occasionally, control net bridges from previously closed sessions are not cleaned up.
+        Occasionally, control net bridges from previously closed sessions are not
-        Check if there are old control net bridges and delete them
+        cleaned up. Check if there are old control net bridges and delete them
 
         :return: True if an old bridge was detected, False otherwise
         :rtype: bool
         """
-        status, output = utils.cmd_output([constants.BRCTL_BIN, "show"])
+        output = self.net_client.get_bridges()
-        if status != 0:
+        lines = output.split("\n")
-            logging.error("Unable to retrieve list of installed bridges")
+        for line in lines[1:]:
-        else:
+            cols = line.split("\t")
-            lines = output.split("\n")
+            oldbr = cols[0]
-            for line in lines[1:]:
+            flds = cols[0].split(".")
-                cols = line.split("\t")
+            if len(flds) == 3:
-                oldbr = cols[0]
+                if flds[0] == "b" and flds[1] == self.id:
-                flds = cols[0].split(".")
+                    logging.error(
-                if len(flds) == 3:
+                        "error: An active control net bridge (%s) found. "
-                    if flds[0] == "b" and flds[1] == self.id:
+                        "An older session might still be running. "
-                        logging.error(
+                        "Stop all sessions and, if needed, delete %s to continue.",
-                            "error: An active control net bridge (%s) found. "
+                        oldbr,
-                            "An older session might still be running. "
+                        oldbr,
-                            "Stop all sessions and, if needed, delete %s to continue.",
+                    )
-                            oldbr,
+                    return True
-                            oldbr,
-                        )
-                        return True
         return False
 
     def shutdown(self):
@@ -893,9 +868,7 @@ class CtrlNet(CoreNetwork):
         """
         if self.serverintf is not None:
             try:
-                utils.check_cmd(
+                self.net_client.delete_interface(self.brname, self.serverintf)
-                    [constants.BRCTL_BIN, "delif", self.brname, self.serverintf]
-                )
             except CoreCommandError:
                 logging.exception(
                     "error deleting server interface %s from bridge %s",
@@ -1100,7 +1073,7 @@ class HubNode(CoreNetwork):
 
         # TODO: move to startup method
         if start:
-            utils.check_cmd([constants.BRCTL_BIN, "setageing", self.brname, "0"])
+            self.net_client.disable_mac_learning(self.brname)
 
 
 class WlanNode(CoreNetwork):
@@ -1131,7 +1104,7 @@ class WlanNode(CoreNetwork):
 
         # TODO: move to startup method
         if start:
-            utils.check_cmd([constants.BRCTL_BIN, "setageing", self.brname, "0"])
+            self.net_client.disable_mac_learning(self.brname)
 
     def attach(self, netif):
         """

daemon/scripts/core-daemon

@@ -44,12 +44,11 @@ def start_udp(mainserver, server_address):
     mainserver.udpthread.start()
 
 
-def cored(cfg, use_ovs):
+def cored(cfg):
     """
     Start the CoreServer object and enter the server loop.
 
     :param dict cfg: core configuration
-    :param bool use_ovs: flag to determine if ovs nodes should be used
     :return: nothing
     """
     host = cfg["listenaddr"]
@@ -153,11 +152,8 @@ def main():
     cfg = get_merged_config("%s/core.conf" % constants.CORE_CONF_DIR)
     banner()
 
-    # check if ovs flag was provided
-    use_ovs = len(sys.argv) == 2 and sys.argv[1] == "ovs"
-
     try:
-        cored(cfg, use_ovs)
+        cored(cfg)
     except KeyboardInterrupt:
         logging.info("keyboard interrupt, stopping core daemon")