Merge pull request #2548 from anarkiwi/master

 Implement idle_eth workaround for broken idle time reset on flow refresh, and test.

faucet/dp.py

@@ -142,6 +142,8 @@ class DP(Conf):
         # Apply strict packet in checking to all packet ins.
         'multi_out': True,
         # Have OFA copy packet outs to multiple ports.
+        'idle_dst': True,
+        # If False, workaround for flow idle timer not reset on flow refresh.
         }
 
     defaults_types = {
@@ -193,6 +195,7 @@ class DP(Conf):
         'strict_packet_in_cookie': bool,
         'multi_out': bool,
         'lacp_timeout': int,
+        'idle_dst': bool,
     }
 
     default_table_sizes_types = {
@@ -295,6 +298,7 @@ def __init__(self, _id, dp_id, conf):
         self.use_classification = None
         self.strict_packet_in_cookie = None
         self.multi_out = None
+        self.idle_dst = None
 
         self.acls = {}
         self.vlans = {}
@@ -329,7 +333,7 @@ def check_config(self):
         test_config_condition(not (self.arp_neighbor_timeout < (self.timeout / 2)), (
             'L2 timeout must be > ARP timeout * 2'))
         test_config_condition(not (self.nd_neighbor_timeout < (self.timeout / 2)), (
-            'L2 timeout must be > ARP timeout * 2'))
+            'L2 timeout must be > ND timeout * 2'))
         if self.cache_update_guard_time == 0:
             self.cache_update_guard_time = int(self.timeout / 2)
         if self.learn_jitter == 0:

faucet/valve.py

@@ -210,7 +210,7 @@ def dp_init(self):
             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.low_priority, self.dp.highest_priority,
-            self.dp.cache_update_guard_time)
+            self.dp.cache_update_guard_time, self.dp.idle_dst)
         table_configs = sorted([
             (table.table_id, str(table.table_config)) for table in self.dp.tables.values()])
         for table_id, table_config in table_configs:
@@ -1343,6 +1343,9 @@ def state_expire(self, now, _other_valves):
         if self.dp.dyn_running:
             for vlan in self.dp.vlans.values():
                 expired_hosts = self.host_manager.expire_hosts_from_vlan(vlan, now)
+                if not self.dp.idle_dst:
+                    for entry in expired_hosts:
+                         ofmsgs.extend(self.host_manager.delete_host_from_vlan(entry.eth_src, vlan))
                 for entry in expired_hosts:
                     self._notify(
                         {'L2_EXPIRE': {

faucet/valve_host.py

@@ -30,7 +30,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, low_priority, host_priority,
-                 cache_update_guard_time):
+                 cache_update_guard_time, idle_dst):
         self.logger = logger
         self.ports = ports
         self.vlans = vlans
@@ -45,6 +45,7 @@ def __init__(self, logger, ports, vlans, eth_src_table, eth_dst_table,
         self.host_priority = host_priority
         self.cache_update_guard_time = cache_update_guard_time
         self.output_table = self.eth_dst_table
+        self.idle_dst = idle_dst
         if self.eth_dst_hairpin_table:
             self.output_table = self.eth_dst_hairpin_table
 
@@ -146,6 +147,8 @@ def learn_host_timeouts(self, port):
         src_rule_idle_timeout = 0
         src_rule_hard_timeout = learn_timeout
         dst_rule_idle_timeout = learn_timeout + self.cache_update_guard_time
+        if not self.idle_dst:
+            dst_rule_idle_timeout = 0
         return (src_rule_idle_timeout, src_rule_hard_timeout, dst_rule_idle_timeout)
 
     def learn_host_on_vlan_port_flows(self, port, vlan, eth_src,

tests/integration/mininet_tests.py

@@ -1894,6 +1894,7 @@ class FaucetSingleHostsTimeoutPrometheusTest(FaucetUntaggedTest):
         nd_neighbor_timeout: 4
         ignore_learn_ins: 0
         learn_jitter: 0
+        cache_update_guard_time: 1
         interfaces:
             %(port_1)d:
                 native_vlan: 100
@@ -1967,17 +1968,54 @@ def learn_then_down_hosts(base, count):
         # make sure at least one host still learned
         learned_macs = self.hosts_learned(all_learned_mac_ports)
         self.assertTrue(learned_macs)
+        before_expiry_learned_macs = learned_macs
 
         # make sure they all eventually expire
-        for _ in range(self.TIMEOUT * 2):
+        for _ in range(self.TIMEOUT * 3):
             learned_macs = self.hosts_learned(all_learned_mac_ports)
             self.verify_learn_counters(
                 100, list(range(1, len(self.net.hosts) + 1)))
             if not learned_macs:
-                return
+                break
             time.sleep(1)
 
-        self.fail('MACs did not expire: %s' % learned_macs)
+        self.assertFalse(learned_macs, msg='MACs did not expire: %s' % learned_macs)
+
+        self.assertTrue(before_expiry_learned_macs)
+        for mac in before_expiry_learned_macs:
+            self.assertFalse(
+                self.get_matching_flow(
+                    match={'eth_dst': mac}, table_id=self._ETH_DST_TABLE))
+
+
+class FaucetSingleHostsNoIdleTimeoutPrometheusTest(FaucetSingleHostsTimeoutPrometheusTest):
+
+    """Test broken reset idle timer on flow refresh workaround."""
+
+    CONFIG = """
+        timeout: 10
+        arp_neighbor_timeout: 4
+        nd_neighbor_timeout: 4
+        ignore_learn_ins: 0
+        learn_jitter: 0
+        cache_update_guard_time: 1
+        idle_dst: False
+        interfaces:
+            %(port_1)d:
+                native_vlan: 100
+                description: "b1"
+            %(port_2)d:
+                native_vlan: 100
+                description: "b2"
+            %(port_3)d:
+                native_vlan: 100
+                description: "b3"
+            %(port_4)d:
+                native_vlan: 100
+                description: "b4"
+"""
+
+
 
 
 class FaucetSingleL3LearnMACsOnPortTest(FaucetUntaggedTest):