Repository: faucetsdn/faucet
Issue: Updating config variables for some dp variables won't send flow mods (change won't be reflected)

Updating most config variables (excluding ones that will force a cold restart such as dot1x) won't send a correct flowmod when sighup is sent.

Some examples are 

drop_broadcast_source_address, drop_spoofed_faucet_mac, priority_offset, packetin_pps...etc.

Guessing this happens because dp cold start doesn't happen when these values are changed, and warm start won't trigger changes to these variables.

Changed files:
faucet/valve.py, faucet/valve_host.py, tests/unit/faucet/test_valve_config.py, faucet/valves_manager.py, tests/unit/faucet/test_config.py, faucet/port.py, faucet/dp.py, faucet/conf.py, tests/integration/mininet_tests.py, tests/unit/faucet/test_valve_stack.py, clib/mininet_test_base.py

diff --git a/clib/mininet_test_base.py b/clib/mininet_test_base.py
index c9b69963..966d6a9a 100644
--- a/clib/mininet_test_base.py
+++ b/clib/mininet_test_base.py
@@ -2080,6 +2080,7 @@ dbs:
             self.assertEqual(
                 old_count, new_count,
                 msg='%s incremented: %u' % (var, new_count))
+        self.wait_for_prometheus_var('faucet_config_applied', 1, dpid=None, timeout=30)
         self.wait_dp_status(1)
 
     def force_faucet_reload(self, new_config):
diff --git a/faucet/conf.py b/faucet/conf.py
index e1e0ce4f..c6179340 100644
--- a/faucet/conf.py
+++ b/faucet/conf.py
@@ -24,7 +24,6 @@ from collections import OrderedDict
 
 class InvalidConfigError(Exception):
     """This error is thrown when the config file is not valid."""
-    pass
 
 
 def test_config_condition(cond, msg):
@@ -37,8 +36,8 @@ class Conf:
     """Base class for FAUCET configuration."""
 
     mutable_attrs = frozenset() # type: frozenset
-    defaults = None # type: dict
-    defaults_types = None # type: dict
+    defaults = {}  # type: dict
+    defaults_types = {}  # type: dict
     dyn_finalized = False
     dyn_hash = None
 
@@ -55,6 +54,7 @@ class Conf:
             self.update(conf)
             self.set_defaults()
         self.check_config()
+        self.orig_conf = {k: self.__dict__[k] for k in self.defaults}
 
     def __setattr__(self, name, value):
         if not self.dyn_finalized or name.startswith('dyn') or name in self.mutable_attrs:
@@ -116,21 +116,22 @@ class Conf:
         self._check_unknown_conf(conf)
         self._check_conf_types(conf, self.defaults_types)
 
-    def check_config(self):
+    @staticmethod
+    def check_config():
         """Check config at instantiation time for errors, typically via assert."""
-        pass
+        return
 
-    @staticmethod
-    def _conf_keys(conf, dyn=False, subconf=True, ignore_keys=None):
+    def _conf_keys(self, conf, subconf=True, ignore_keys=None):
         """Return a list of key/values of attributes with dyn/Conf attributes/filtered."""
         conf_keys = []
-        for key, value in sorted(conf.__dict__.items()):
-            if not dyn and key.startswith('dyn'):
-                continue
-            if not subconf and isinstance(value, Conf):
-                continue
+        for key, value in sorted(((key, value) for key, value in conf.orig_conf.items() if key in self.defaults)):
             if ignore_keys and key in ignore_keys:
                 continue
+            if not subconf and value:
+                if isinstance(value, Conf):
+                    continue
+                if isinstance(value, (tuple, list, set)) and isinstance(value[0], Conf):
+                    continue
             conf_keys.append((key, value))
         return conf_keys
 
@@ -162,7 +163,7 @@ class Conf:
     def to_conf(self):
         """Return configuration as a dict."""
         conf = {
-            k: self.__dict__[str(k)] for k in self.defaults.keys() if k != 'name'}
+            k: self.orig_conf[str(k)] for k in self.defaults if k != 'name'}
         return json.dumps(self._str_conf(conf), sort_keys=True, indent=4, separators=(',', ': '))
 
     def conf_diff(self, other):
@@ -171,15 +172,15 @@ class Conf:
         return '\\n'.join(differ.compare(
             self.to_conf().splitlines(), other.to_conf().splitlines()))
 
-    def conf_hash(self, dyn=False, subconf=True, ignore_keys=None):
+    def conf_hash(self, subconf=True, ignore_keys=None):
         """Return hash of keys configurably filtering attributes."""
         return hash(frozenset(list(map(
-            str, self._conf_keys(self, dyn=dyn, subconf=subconf, ignore_keys=ignore_keys)))))
+            str, self._conf_keys(self, subconf=subconf, ignore_keys=ignore_keys)))))
 
     def __hash__(self):
         if self.dyn_hash is not None:
             return self.dyn_hash
-        dyn_hash = self.conf_hash(dyn=False, subconf=True)
+        dyn_hash = self.conf_hash(subconf=True)
         if self.dyn_finalized:
             self.dyn_hash = dyn_hash
         return dyn_hash
@@ -205,8 +206,9 @@ class Conf:
 
     def ignore_subconf(self, other, ignore_keys=None):
         """Return True if this config same as other, ignoring sub config."""
-        return (self.conf_hash(dyn=False, subconf=False, ignore_keys=ignore_keys)
-                == other.conf_hash(dyn=False, subconf=False, ignore_keys=ignore_keys))
+        return (self.conf_hash(
+            subconf=False, ignore_keys=ignore_keys) == other.conf_hash(
+                subconf=False, ignore_keys=ignore_keys))
 
     def __eq__(self, other):
         return self.__hash__() == other.__hash__()
@@ -220,8 +222,7 @@ class Conf:
             # bool type is deprecated by the library ipaddress
             if not isinstance(ip_str, bool):
                 return ip_method(ip_str)
-            else:
-                raise InvalidConfigError('Invalid IP address %s: IP address of type bool' % (ip_str))
+            raise InvalidConfigError('Invalid IP address %s: IP address of type bool' % (ip_str))
         except (ValueError, AttributeError, TypeError) as err:
             raise InvalidConfigError('Invalid IP address %s: %s' % (ip_str, err))
 
diff --git a/faucet/dp.py b/faucet/dp.py
index 905b532f..b7d5fe51 100644
--- a/faucet/dp.py
+++ b/faucet/dp.py
@@ -329,6 +329,7 @@ configuration.
         self.table_sizes = {}
         self.dyn_up_port_nos = set()
         self.has_externals = None
+        self.stack_graph = None
 
         #tunnel_id: int
         #   ID of the tunnel, for now this will be the VLAN ID
@@ -390,8 +391,6 @@ configuration.
         if self.learn_ban_timeout == 0:
             self.learn_ban_timeout = self.learn_jitter
         if self.stack:
-            if 'graph' in self.stack:
-                del self.stack['graph']
             self._check_conf_types(self.stack, self.stack_defaults_types)
         if self.lldp_beacon:
             self._lldp_defaults()
@@ -864,7 +863,7 @@ configuration.
         if graph.size() and self.name in graph:
             if self.stack is None:
                 self.stack = {}
-            self.stack.update({'graph': graph})
+            self.stack_graph = graph
             for dp in graph.nodes():
                 path_to_root_len = len(self.shortest_path(self.stack_root_name, src_dp=dp))
                 test_config_condition(
@@ -878,19 +877,15 @@ configuration.
 
     def get_node_link_data(self):
         """Return network stacking graph as a node link representation"""
-        graph = self.stack.get('graph', None)
-        return networkx.json_graph.node_link_data(graph)
+        return networkx.json_graph.node_link_data(self.stack_graph)
 
     def stack_longest_path_to_root_len(self):
         """Return length of the longest path to root in the stack."""
-        if not self.stack or not self.stack_root_name:
-            return None
-        graph = self.stack.get('graph', None)
-        if not graph:
+        if not self.stack_graph or not self.stack_root_name:
             return None
         len_paths_to_root = [
             len(self.shortest_path(self.stack_root_name, src_dp=dp))
-            for dp in graph.nodes()]
+            for dp in self.stack_graph.nodes()]
         if len_paths_to_root:
             return max(len_paths_to_root)
         return None
@@ -924,13 +919,11 @@ configuration.
         """Return shortest path to a DP, as a list of DPs."""
         if src_dp is None:
             src_dp = self.name
-        if self.stack:
-            graph = self.stack.get('graph', None)
-            if graph:
-                try:
-                    return sorted(networkx.all_shortest_paths(graph, src_dp, dest_dp))[0]
-                except (networkx.exception.NetworkXNoPath, networkx.exception.NodeNotFound):
-                    pass
+        if self.stack_graph:
+            try:
+                return sorted(networkx.all_shortest_paths(self.stack_graph, src_dp, dest_dp))[0]
+            except (networkx.exception.NetworkXNoPath, networkx.exception.NodeNotFound):
+                pass
         return []
 
     def shortest_path_to_root(self, src_dp=None):
@@ -1360,9 +1353,11 @@ configuration.
         for conf_id, new_conf in new_subconf.items():
             old_conf = subconf.get(conf_id, None)
             if old_conf:
-                if old_conf.ignore_subconf(new_conf, ignore_keys=ignore_keys):
+                if old_conf.ignore_subconf(
+                        new_conf, ignore_keys=ignore_keys):
                     same_confs.add(conf_id)
-                elif old_conf.ignore_subconf(new_conf, ignore_keys=(ignore_keys.union(['description']))):
+                elif old_conf.ignore_subconf(
+                        new_conf, ignore_keys=(ignore_keys.union(['description']))):
                     same_confs.add(conf_id)
                     description_only_confs.add(conf_id)
                     logger.info('%s %s description only changed' % (
@@ -1393,7 +1388,8 @@ configuration.
         else:
             logger.info('no %s changes' % conf_name)
 
-        return (changes, deleted_confs, added_confs, changed_confs, same_confs, description_only_confs)
+        return (
+            changes, deleted_confs, added_confs, changed_confs, same_confs, description_only_confs)
 
     def _get_acl_config_changes(self, logger, new_dp):
         """Detect any config changes to ACLs.
@@ -1423,20 +1419,23 @@ configuration.
             logger, 'VLAN', self.vlans, new_dp.vlans)
         return (deleted_vlans, added_vlans.union(changed_vlans))
 
-    def _get_port_config_changes(self, logger, new_dp, changed_vlans, changed_acls):
+    def _get_port_config_changes(self, logger, new_dp, changed_vlans, deleted_vlans, changed_acls):
         """Detect any config changes to ports.
 
         Args:
             logger (ValveLogger): logger instance.
             new_dp (DP): new dataplane configuration.
             changed_vlans (set): changed/added VLAN IDs.
+            deleted_vlans (set): deleted VLAN IDs.
             changed_acls (set): changed/added ACL IDs.
         Returns:
             changes (tuple) of:
                 all_ports_changed (bool): True if all ports changed.
                 deleted_ports (set): deleted port numbers.
-                changed_ports (set): changed/added port numbers.
+                changed_ports (set): changed port numbers.
+                added_ports (set): added port numbers.
                 changed_acl_ports (set): changed ACL only port numbers.
+                changed_vlans (set): changed/added VLAN IDs.
         """
         _, deleted_ports, added_ports, changed_ports, same_ports, _ = self._get_conf_changes(
             logger, 'port', self.ports, new_dp.ports,
@@ -1445,20 +1444,58 @@ configuration.
         changed_acl_ports = set()
         all_ports_changed = False
 
+        if not same_ports:
+            all_ports_changed = True
         # TODO: optimize case where only VLAN ACL changed.
-        if changed_vlans:
+        elif changed_vlans:
             all_ports = frozenset(new_dp.ports.keys())
-            for vid in changed_vlans:
-                changed_port_nums = {port.number for port in new_dp.vlans[vid].get_ports()}
+            new_changed_vlans = {
+                vlan for vlan in new_dp.vlans.values() if vlan.vid in changed_vlans}
+            for vlan in new_changed_vlans:
+                changed_port_nums = {port.number for port in vlan.get_ports()}
                 changed_ports.update(changed_port_nums)
             all_ports_changed = changed_ports == all_ports
 
-        # TODO: optimize for only mirror options changed on a port
-        # Optimize for case where only the ACL changed on a port.
+        # Detect changes to VLANs and ACLs based on port changes.
         if not all_ports_changed:
+            def _add_changed_vlan_port(port):
+                if port.native_vlan:
+                    changed_vlans.add(port.native_vlan.vid)
+                if port.tagged_vlans:
+                    changed_vlans.update({vlan.vid for vlan in port.tagged_vlans})
+
+            for port_no in changed_ports:
+                old_port = self.ports[port_no]
+                new_port = new_dp.ports[port_no]
+                # native_vlan changed.
+                if old_port.native_vlan != new_port.native_vlan:
+                    for port in (old_port, new_port):
+                        if port.native_vlan:
+                            changed_vlans.add(port.native_vlan.vid)
+                # tagged vlans changed.
+                if old_port.tagged_vlans != new_port.tagged_vlans:
+                    for port in (old_port, new_port):
+                        if port.tagged_vlans:
+                            changed_vlans.update({vlan.vid for vlan in port.tagged_vlans})
+
+            # ports deleted or added.
+            for port_no in deleted_ports:
+                port = self.ports[port_no]
+                _add_changed_vlan_port(port)
+            for port_no in added_ports:
+                port = new_dp.ports[port_no]
+                _add_changed_vlan_port(port)
             for port_no in same_ports:
                 old_port = self.ports[port_no]
                 new_port = new_dp.ports[port_no]
+                # mirror options changed.
+                if old_port.mirror != new_port.mirror:
+                    logger.info('port %s mirror options changed: %s' % (
+                        port_no, new_port.mirror))
+                    for mirrored_port in (old_port, new_port):
+                        if mirrored_port.mirror:
+                            _add_changed_vlan_port(mirrored_port)
+                # ACL changes
                 new_acl_ids = new_port.acls_in
                 port_acls_changed = set()
                 if new_acl_ids:
@@ -1479,8 +1516,20 @@ configuration.
             if changed_acl_ports:
                 same_ports -= changed_acl_ports
                 logger.info('ports where ACL only changed: %s' % changed_acl_ports)
+
+        changed_vlans -= deleted_vlans
+        # TODO: limit scope to only routers that have affected VLANs.
+        changed_vlans_with_vips = []
+        for vid in changed_vlans:
+            vlan = new_dp.vlans[vid]
+            if vlan.faucet_vips:
+                changed_vlans_with_vips.append(vlan)
+        if changed_vlans_with_vips:
+            logger.info('forcing cold start because %s has routing' % changed_vlans_with_vips)
+            all_ports_changed = True
+
         return (all_ports_changed, deleted_ports,
-                added_ports.union(changed_ports), changed_acl_ports)
+                changed_ports, added_ports, changed_acl_ports, changed_vlans)
 
     def _get_meter_config_changes(self, logger, new_dp):
         """Detect any config changes to meters.
@@ -1497,6 +1546,9 @@ configuration.
 
         return (all_meters_changed, deleted_meters, added_meters, changed_meters)
 
+    def _table_configs(self):
+        return frozenset([table.table_config for table in self.tables.values()])
+
     def get_config_changes(self, logger, new_dp):
         """Detect any config changes.
 
@@ -1507,7 +1559,8 @@ configuration.
             (tuple): changes tuple containing:
 
                 deleted_ports (set): deleted port numbers.
-                changed_ports (set): changed/added port numbers.
+                changed_ports (set): changed port numbers.
+                added_ports (set): added port numbers.
                 changed_acl_ports (set): changed ACL only port numbers.
                 deleted_vlans (set): deleted VLAN IDs.
                 changed_vlans (set): changed/added VLAN IDs.
@@ -1515,32 +1568,28 @@ configuration.
                 deleted_meters (set): deleted meter numbers
                 changed_meters (set): changed/added meter numbers
         """
-        def _table_configs(dp):
-            return frozenset([
-                table.table_config for table in dp.tables.values()])
-
-        if self.ignore_subconf(new_dp):
-            logger.info('DP base level config changed - requires cold start')
-        elif _table_configs(self) != _table_configs(new_dp):
+        if new_dp._table_configs() != self._table_configs():
             logger.info('pipeline table config change - requires cold start')
-        elif new_dp.routers != self.routers:
-            logger.info('DP routers config changed - requires cold start')
         elif new_dp.stack_root_name != self.stack_root_name:
             logger.info('Stack root change - requires cold start')
+        elif new_dp.routers != self.routers:
+            logger.info('DP routers config changed - requires cold start')
+        elif not self.ignore_subconf(new_dp, ignore_keys=['interfaces', 'interfaces_range', 'routers']):
+            logger.info('DP config changed - requires cold start: %s' % self.conf_diff(new_dp))
         else:
             changed_acls = self._get_acl_config_changes(logger, new_dp)
             deleted_vlans, changed_vlans = self._get_vlan_config_changes(logger, new_dp)
-            (all_ports_changed, deleted_ports, changed_ports,
-             changed_acl_ports) = self._get_port_config_changes(
-                 logger, new_dp, changed_vlans, changed_acls)
             (all_meters_changed, deleted_meters,
              added_meters, changed_meters) = self._get_meter_config_changes(logger, new_dp)
-            return (deleted_ports, changed_ports, changed_acl_ports,
+            (all_ports_changed, deleted_ports, changed_ports, added_ports,
+             changed_acl_ports, changed_vlans) = self._get_port_config_changes(
+                 logger, new_dp, changed_vlans, deleted_vlans, changed_acls)
+            return (deleted_ports, changed_ports, added_ports, changed_acl_ports,
                     deleted_vlans, changed_vlans, all_ports_changed,
                     all_meters_changed, deleted_meters,
                     added_meters, changed_meters)
         # default cold start
-        return (set(), set(), set(), set(), set(), True, True, set(), set(), set())
+        return (set(), set(), set(), set(), set(), set(), True, True, set(), set(), set())
 
     def get_tables(self):
         """Return tables as dict for API call."""
diff --git a/faucet/port.py b/faucet/port.py
index 8297b3ee..23b18be2 100644
--- a/faucet/port.py
+++ b/faucet/port.py
@@ -133,7 +133,7 @@ class Port(Conf):
         # If true, this port cannot send non-ARP/IPv6 ND broadcasts to other restricted_bcast_arpnd ports.
         'coprocessor': {},
         # If defined, this port is attached to a packet coprocessor.
-        'count_untag_vlan_miss': {},
+        'count_untag_vlan_miss': False,
         # If defined, this port will explicitly count unconfigured native VLAN packets.
     }
 
diff --git a/faucet/valve.py b/faucet/valve.py
index 4c2c6d8e..6645882e 100644
--- a/faucet/valve.py
+++ b/faucet/valve.py
@@ -171,7 +171,7 @@ class Valve:
             for port_number in self.dp.ports.keys():
                 self._port_highwater[vlan_vid][port_number] = 0
         restricted_bcast_arpnd = bool(self.dp.restricted_bcast_arpnd_ports())
-        if self.dp.stack:
+        if self.dp.stack_graph:
             flood_class = valve_flood.ValveFloodStackManagerNoReflection
             if self.dp.stack_root_flood_reflection:
                 flood_class = valve_flood.ValveFloodStackManagerReflection
@@ -186,7 +186,7 @@ class Valve:
                 self.dp.stack_ports, self.dp.has_externals,
                 self.dp.shortest_path_to_root, self.dp.shortest_path_port,
                 self.dp.is_stack_root, self.dp.is_stack_root_candidate,
-                self.dp.is_stack_edge, self.dp.stack.get('graph', None))
+                self.dp.is_stack_edge, self.dp.stack_graph)
         else:
             self.flood_manager = valve_flood.ValveFloodManager(
                 self.logger, self.dp.tables['flood'], self.pipeline,
@@ -202,7 +202,7 @@ class Valve:
             fib_table = self.dp.tables[fib_table_name]
             proactive_learn = getattr(self.dp, 'proactive_learn_v%u' % ipv)
             valve_flood_manager = None
-            if self.dp.stack:
+            if self.dp.stack_graph:
                 valve_flood_manager = self.flood_manager
             route_manager = route_manager_class(
                 self.logger, self.notify, self.dp.global_vlan, neighbor_timeout,
@@ -229,7 +229,7 @@ class Valve:
             self.dp.vlans, self.dp.tables['eth_src'],
             self.dp.tables['eth_dst'], eth_dst_hairpin_table, self.pipeline,
             self.dp.timeout, self.dp.learn_jitter, self.dp.learn_ban_timeout,
-            self.dp.cache_update_guard_time, self.dp.idle_dst, self.dp.stack,
+            self.dp.cache_update_guard_time, self.dp.idle_dst, self.dp.stack_graph,
             self.dp.has_externals, self.dp.stack_root_flood_reflection)
         self.acl_manager = None
         if self.dp.has_acls:
@@ -661,12 +661,11 @@ class Valve:
 
             # Find the first valve with a valid stack and trigger notification.
             for valve in stacked_valves:
-                graph = valve.dp.get_node_link_data()
-                if graph:
+                if valve.dp.stack_graph:
                     self.notify(
                         {'STACK_TOPO_CHANGE': {
                             'stack_root': valve.dp.stack_root_name,
-                            'graph': graph,
+                            'graph': valve.dp.stack_graph,
                             'dps': notify_dps
                             }})
                     break
@@ -1030,7 +1029,7 @@ class Valve:
             bool: True if port state changed
         """
         nominated_dpid = self.dp.dp_id
-        if self.dp.stack:
+        if self.dp.stack_graph:
             nominated_dpid, _ = self.get_lacp_dpid_nomination(port.lacp, other_valves)
         prev_state = port.lacp_port_state()
         new_state = port.lacp_port_update(self.dp.dp_id == nominated_dpid, cold_start=cold_start)
@@ -1470,7 +1469,7 @@ class Valve:
                 'packet with all zeros eth_src %s port %u' % (
                     pkt_meta.eth_src, in_port))
             return None
-        if self.dp.stack is not None:
+        if self.dp.stack_graph:
             if (not pkt_meta.port.stack and
                     pkt_meta.vlan and
                     pkt_meta.vlan not in pkt_meta.port.tagged_vlans and
@@ -1758,7 +1757,8 @@ class Valve:
             new_dp: (DP): new dataplane configuration.
             changes (tuple) of:
                 deleted_ports (set): deleted port numbers.
-                changed_ports (set): changed/added port numbers.
+                changed_ports (set): changed port numbers.
+                added_ports (set): added port numbers.
                 changed_acl_ports (set): changed ACL only port numbers.
                 deleted_vids (set): deleted VLAN IDs.
                 changed_vids (set): changed/added VLAN IDs.
@@ -1772,7 +1772,7 @@ class Valve:
                 cold_start (bool): whether cold starting.
                 ofmsgs (list): OpenFlow messages.
         """
-        (deleted_ports, changed_ports, changed_acl_ports,
+        (deleted_ports, changed_ports, added_ports, changed_acl_ports,
          deleted_vids, changed_vids, all_ports_changed,
          _, deleted_meters, added_meters, changed_meters) = changes
 
@@ -1836,6 +1836,8 @@ class Valve:
             ofmsgs.extend(self.add_vlans(changed_vlans))
         if changed_ports:
             ofmsgs.extend(self.ports_add(all_up_port_nos))
+        if added_ports:
+            ofmsgs.extend(self.ports_add(added_ports))
         if self.acl_manager and changed_acl_ports:
             for port_num in changed_acl_ports:
                 port = self.dp.ports[port_num]
diff --git a/faucet/valve_host.py b/faucet/valve_host.py
index 29d4ab2e..0bf9fef0 100644
--- a/faucet/valve_host.py
+++ b/faucet/valve_host.py
@@ -26,7 +26,7 @@ class ValveHostManager(ValveManagerBase):
 
     def __init__(self, logger, ports, vlans, eth_src_table, eth_dst_table,
                  eth_dst_hairpin_table, pipeline, learn_timeout, learn_jitter,
-                 learn_ban_timeout, cache_update_guard_time, idle_dst, stack,
+                 learn_ban_timeout, cache_update_guard_time, idle_dst, stack_graph,
                  has_externals, stack_root_flood_reflection):
         self.logger = logger
         self.ports = ports
@@ -44,7 +44,7 @@ class ValveHostManager(ValveManagerBase):
         self.cache_update_guard_time = cache_update_guard_time
         self.output_table = self.eth_dst_table
         self.idle_dst = idle_dst
-        self.stack = stack
+        self.stack_graph = stack_graph
         self.has_externals = has_externals
         self.stack_root_flood_reflection = stack_root_flood_reflection
         if self.eth_dst_hairpin_table:
@@ -242,7 +242,7 @@ class ValveHostManager(ValveManagerBase):
         if self.has_externals:
             match_dict.update({
                 valve_of.EXTERNAL_FORWARDING_FIELD: valve_of.PCP_EXT_PORT_FLAG})
-            if port.tagged_vlans and port.loop_protect_external and self.stack:
+            if port.tagged_vlans and port.loop_protect_external and self.stack_graph:
                 external_forwarding_requested = False
             elif not port.stack:
                 external_forwarding_requested = True
diff --git a/faucet/valves_manager.py b/faucet/valves_manager.py
index 3373052d..a5a34790 100644
--- a/faucet/valves_manager.py
+++ b/faucet/valves_manager.py
@@ -248,7 +248,7 @@ class ValvesManager:
                 valve = self.valves[dp_id]
                 ofmsgs = valve.reload_config(now, new_dp)
                 self.send_flows_to_dp_by_id(valve, ofmsgs)
-                sent[dp_id] = True
+                sent[dp_id] = valve.dp.dyn_running
             else:
                 self.logger.info('Add new datapath %s', dpid_log(new_dp.dp_id))
                 valve = self.new_valve(new_dp)
@@ -362,4 +362,5 @@ class ValvesManager:
     def datapath_connect(self, now, valve, discovered_up_ports):
         """Handle connection from DP."""
         self.meta_dp_state.dp_last_live_time[valve.dp.name] = now
+        self.update_config_applied({valve.dp.dp_id: True})
         return valve.datapath_connect(now, discovered_up_ports)
diff --git a/tests/integration/mininet_tests.py b/tests/integration/mininet_tests.py
index 802afe0a..6d7e485d 100644
--- a/tests/integration/mininet_tests.py
+++ b/tests/integration/mininet_tests.py
@@ -3128,7 +3128,7 @@ class FaucetConfigReloadTest(FaucetConfigReloadTestBase):
             actions=['OUTPUT:CONTROLLER', 'GOTO_TABLE:%u' % self._ETH_DST_TABLE])
         self.change_port_config(
             self.port_map['port_2'], 'native_vlan', 200,
-            restart=True, cold_start=True)
+            restart=True, cold_start=False)
         for port_name in ('port_1', 'port_2'):
             self.wait_until_matching_flow(
                 {'in_port': int(self.port_map[port_name])},
diff --git a/tests/unit/faucet/test_config.py b/tests/unit/faucet/test_config.py
index fa5e61aa..83fe6fd2 100755
--- a/tests/unit/faucet/test_config.py
+++ b/tests/unit/faucet/test_config.py
@@ -181,7 +181,7 @@ dps:
             self.assertEqual(
                 dp.stack_roots_names, ('t1-1', 't1-2'), 'root_dps configured incorrectly')
             self.assertEqual(
-                len(dp.stack['graph'].nodes),
+                len(dp.stack_graph.nodes),
                 3,
                 'stack graph has incorrect nodes'
                 )
diff --git a/tests/unit/faucet/test_valve_config.py b/tests/unit/faucet/test_valve_config.py
index 62b9cf34..68848361 100755
--- a/tests/unit/faucet/test_valve_config.py
+++ b/tests/unit/faucet/test_valve_config.py
@@ -180,7 +180,7 @@ dps:
 
     def test_port_delete(self):
         """Test port can be deleted."""
-        self.update_config(self.LESS_CONFIG, reload_type='cold')
+        self.update_config(self.LESS_CONFIG, reload_type='warm')
 
 
 class ValveAddPortTestCase(ValveTestBases.ValveTestSmall):
@@ -226,7 +226,7 @@ dps:
 
     def test_port_add(self):
         """Test port can be added."""
-        reload_ofmsgs = self.update_config(self.MORE_CONFIG, reload_type='cold')
+        reload_ofmsgs = self.update_config(self.MORE_CONFIG, reload_type='warm')
         self.assertTrue(self._inport_flows(3, reload_ofmsgs))
 
 
@@ -332,6 +332,45 @@ dps:
         self.update_config(self.LESS_CONFIG, reload_type='cold')
 
 
+class ValveChangeDPTestCase(ValveTestBases.ValveTestSmall):
+    """Test changing DP."""
+
+    CONFIG = """
+dps:
+    s1:
+%s
+        priority_offset: 4321
+        interfaces:
+            p1:
+                number: 1
+                native_vlan: 0x100
+            p2:
+                number: 2
+                native_vlan: 0x100
+""" % DP1_CONFIG
+
+    NEW_CONFIG = """
+dps:
+    s1:
+%s
+        priority_offset: 1234
+        interfaces:
+            p1:
+                number: 1
+                native_vlan: 0x100
+            p2:
+                number: 2
+                native_vlan: 0x100
+""" % DP1_CONFIG
+
+    def setUp(self):
+        self.setup_valve(self.CONFIG)
+
+    def test_change_dp(self):
+        """Test DP changed."""
+        self.update_config(self.NEW_CONFIG, reload_type='cold')
+
+
 class ValveAddVLANTestCase(ValveTestBases.ValveTestSmall):
     """Test adding VLAN."""
 
@@ -493,6 +532,9 @@ dps:
             p2:
                 number: 2
                 output_only: True
+            p3:
+                number: 3
+                native_vlan: 0x200
 """ % DP1_CONFIG
 
     MIRROR_CONFIG = """
@@ -506,6 +548,9 @@ dps:
             p2:
                 number: 2
                 mirror: p1
+            p3:
+                number: 3
+                native_vlan: 0x200
 """ % DP1_CONFIG
 
     def setUp(self):
@@ -531,6 +576,12 @@ dps:
             1, self.get_prom('vlan_hosts_learned', labels=vlan_labels))
         self.assertEqual(
             1, self.get_prom('port_vlan_hosts_learned', labels=port_labels))
+        # Now unmirror again.
+        self.update_config(self.CONFIG, reload_type='warm')
+        self.assertEqual(
+            1, self.get_prom('vlan_hosts_learned', labels=vlan_labels))
+        self.assertEqual(
+            1, self.get_prom('port_vlan_hosts_learned', labels=port_labels))
 
 
 class ValveACLTestCase(ValveTestBases.ValveTestSmall):
@@ -899,7 +950,7 @@ dps:
                 number: 2
                 native_vlan: 0x100
         """
-        self.update_config(more_config, reload_expected=True, error_expected=0)
+        self.update_config(more_config, reload_expected=True, reload_type='warm', error_expected=0)
 
 
 class ValveTestConfigRevertBootstrap(ValveTestBases.ValveTestSmall):
diff --git a/tests/unit/faucet/test_valve_stack.py b/tests/unit/faucet/test_valve_stack.py
index 3110d37e..87b05f63 100755
--- a/tests/unit/faucet/test_valve_stack.py
+++ b/tests/unit/faucet/test_valve_stack.py
@@ -950,7 +950,7 @@ class ValveStackGraphUpdateTestCase(ValveTestBases.ValveTestSmall):
                 valve = self.valves_manager.valves[dpid]
                 if not valve.dp.stack:
                     continue
-                graph = valve.dp.stack['graph']
+                graph = valve.dp.stack_graph
                 self.assertEqual(num_edges, len(graph.edges()))
                 if test_func and edge:
                     test_func(edge in graph.edges(keys=True))