Merge pull request #2512 from anarkiwi/master

Move active LACP to fast_advertise() service.

Author: anarkiwi

README.rst

@@ -1,7 +1,7 @@
 Faucet
 ======
 
-:version: 1.8.21
+:version: 1.8.22
 
 .. image:: https://travis-ci.com/faucetsdn/faucet.svg?branch=master
     :target: https://travis-ci.com/faucetsdn/faucet

faucet/dp.py

@@ -101,7 +101,9 @@ class DP(Conf):
         'learn_ban_timeout': 0,
         # When banning/limiting learning, wait this many seconds before learning can be retried
         'advertise_interval': 30,
-        # How often to advertise (eg. IPv6 RAs)
+        # How often to slow advertise (eg. IPv6 RAs)
+        'fast_advertise_interval': 5,
+        # How often to fast advertise (eg. LACP)
         'proactive_learn_v4': True,
         # whether proactive learning is enabled for IPv4 nexthops
         'proactive_learn_v6': True,
@@ -171,6 +173,7 @@ class DP(Conf):
         'learn_jitter': int,
         'learn_ban_timeout': int,
         'advertise_interval': int,
+        'fast_advertise_interval': int,
         'proactive_learn_v4': bool,
         'proactive_learn_v6': bool,
         'use_idle_timeout': bool,
@@ -230,6 +233,7 @@ def __init__(self, _id, dp_id, conf):
         self.acls = None
         self.acls_in = None
         self.advertise_interval = None
+        self.fast_advertise_interval = None
         self.arp_neighbor_timeout = None
         self.nd_neighbor_timeout = None
         self.bgp_local_address = None

faucet/faucet.py

@@ -47,6 +47,11 @@ class EventFaucetExperimentalAPIRegistered(event.EventBase): # pylint: disable=t
     pass
 
 
+class EventFaucetMetricUpdate(event.EventBase): # pylint: disable=too-few-public-methods
+    """Event used to trigger update of metrics."""
+    pass
+
+
 class EventFaucetResolveGateways(event.EventBase): # pylint: disable=too-few-public-methods
     """Event used to trigger gateway re/resolution."""
     pass
@@ -56,9 +61,8 @@ class EventFaucetStateExpire(event.EventBase): # pylint: disable=too-few-public-
     """Event used to trigger expiration of state in controller."""
     pass
 
-
-class EventFaucetMetricUpdate(event.EventBase): # pylint: disable=too-few-public-methods
-    """Event used to trigger update of metrics."""
+class EventFaucetFastStateExpire(event.EventBase): # pylint: disable=too-few-public-methods
+    """Event used to trigger fast expiration of state in controller."""
     pass
 
 
@@ -67,15 +71,11 @@ class EventFaucetAdvertise(event.EventBase): # pylint: disable=too-few-public-me
     pass
 
 
-class EventFaucetLLDPAdvertise(event.EventBase): # pylint: disable=too-few-public-methods
-    """Event used to trigger periodic LLDP beacons."""
+class EventFaucetFastAdvertise(event.EventBase): # pylint: disable=too-few-public-methods
+    """Event used to trigger periodic fast network advertisements (eg LACP)."""
     pass
 
 
-class EventFaucetStackLinkStates(event.EventBase): # pylint: disable=too-few-public-methods
-    """Event used to update link stack states."""
-    pass
-
 
 class Faucet(RyuAppBase):
     """A RyuApp that implements an L2/L3 learning VLAN switch.
@@ -92,9 +92,9 @@ class Faucet(RyuAppBase):
         EventFaucetMetricUpdate: (None, 5),
         EventFaucetResolveGateways: ('resolve_gateways', 2),
         EventFaucetStateExpire: ('state_expire', 5),
-        EventFaucetAdvertise: ('advertise', 5),
-        EventFaucetLLDPAdvertise: ('send_lldp_beacons', 5),
-        EventFaucetStackLinkStates: ('update_stack_link_states', 2),
+        EventFaucetFastStateExpire: ('fast_state_expire', 2),
+        EventFaucetAdvertise: ('advertise', 15),
+        EventFaucetFastAdvertise: ('fast_advertise', 5),
     }
     logname = 'faucet'
     exc_logname = logname + '.exception'
@@ -199,9 +199,9 @@ def metric_update(self, _):
 
     @set_ev_cls(EventFaucetResolveGateways, MAIN_DISPATCHER)
     @set_ev_cls(EventFaucetStateExpire, MAIN_DISPATCHER)
+    @set_ev_cls(EventFaucetFastStateExpire, MAIN_DISPATCHER)
     @set_ev_cls(EventFaucetAdvertise, MAIN_DISPATCHER)
-    @set_ev_cls(EventFaucetLLDPAdvertise, MAIN_DISPATCHER)
-    @set_ev_cls(EventFaucetStackLinkStates, MAIN_DISPATCHER)
+    @set_ev_cls(EventFaucetFastAdvertise, MAIN_DISPATCHER)
     @kill_on_exception(exc_logname)
     def _valve_flow_services(self, ryu_event):
         """Call a method on all Valves and send any resulting flows."""

faucet/valve.py

@@ -87,6 +87,7 @@ class Valve:
         'ofchannel_logger',
         'recent_ofmsgs',
         '_last_advertise_sec',
+        '_last_fast_advertise_sec',
         '_last_packet_in_sec',
         '_last_pipeline_flows',
         '_packet_in_count_sec',
@@ -110,11 +111,7 @@ def __init__(self, dp, logname, metrics, notifier, dot1x):
         self.ofchannel_logger = None
         self.logger = None
         self.recent_ofmsgs = deque(maxlen=32)
-        self._last_advertise_sec = 0
         self._last_pipeline_flows = []
-        self._last_packet_in_sec = 0
-        self._packet_in_count_sec = 0
-        self._port_highwater = {}
         self.dp_init()
 
     def port_labels(self, port):
@@ -153,6 +150,7 @@ def dp_init(self):
         self._packet_in_count_sec = 0
         self._last_packet_in_sec = 0
         self._last_advertise_sec = 0
+        self._last_fast_advertise_sec = 0
         self._route_manager_by_ipv = {}
         self._route_manager_by_eth_type = {}
         self._port_highwater = {}
@@ -517,16 +515,15 @@ def _decode_port_status(reason):
     def advertise(self, now, _other_values):
         """Called periodically to advertise services (eg. IPv6 RAs)."""
         ofmsgs = []
-        if (self.dp.advertise_interval and
-                now - self._last_advertise_sec > self.dp.advertise_interval):
-            for route_manager in list(self._route_manager_by_ipv.values()):
-                for vlan in list(self.dp.vlans.values()):
-                    ofmsgs.extend(route_manager.advertise(vlan))
-            for port in list(self.dp.lacp_active_ports):
-                if port.running():
-                    pkt = self._lacp_pkt(port.dyn_last_lacp_pkt, port)
-                    ofmsgs.append(valve_of.packetout(port.number, pkt.data))
-            self._last_advertise_sec = now
+        if (not self.dp.advertise_interval or
+                now - self._last_advertise_sec < self.dp.advertise_interval):
+            return ofmsgs
+        self._last_advertise_sec = now
+
+        for route_manager in list(self._route_manager_by_ipv.values()):
+            for vlan in list(self.dp.vlans.values()):
+                ofmsgs.extend(route_manager.advertise(vlan))
+
         return ofmsgs
 
     def _send_lldp_beacon_on_port(self, port, now):
@@ -549,19 +546,28 @@ def _send_lldp_beacon_on_port(self, port, now):
         port.dyn_last_lldp_beacon_time = now
         return valve_of.packetout(port.number, lldp_beacon_pkt.data)
 
-    def send_lldp_beacons(self, now, _other_valves):
-        """Called periodically to send LLDP beacon packets."""
+    def fast_advertise(self, now, _other_valves):
+        """Called periodically to send LLDP/LACP packets."""
         # TODO: the beacon service is specifically NOT to support conventional R/STP.
         # It is intended to facilitate physical troubleshooting (e.g.
         # a standard cable tester can display OF port information).
         # It is used also by stacking to verify stacking links.
         # TODO: in the stacking case, provide an authentication scheme for the probes
         # so they cannot be forged.
         ofmsgs = []
+        if (not self.dp.fast_advertise_interval or
+                now - self._last_fast_advertise_sec < self.dp.fast_advertise_interval):
+            return ofmsgs
+        self._last_fast_advertise_sec = now
+
+        for port in list(self.dp.lacp_active_ports):
+            if port.running():
+                pkt = self._lacp_pkt(port.dyn_last_lacp_pkt, port)
+                ofmsgs.append(valve_of.packetout(port.number, pkt.data))
+
         ports = self.dp.lldp_beacon_send_ports(now)
-        if ports:
-            self.logger.debug('sending LLDP beacons on %s' % ports)
-            ofmsgs = [self._send_lldp_beacon_on_port(port, now) for port in ports]
+        ofmsgs.extend([self._send_lldp_beacon_on_port(port, now) for port in ports])
+
         return ofmsgs
 
     def _next_stack_link_state(self, port, now):
@@ -617,7 +623,7 @@ def _update_stack_link_state(self, port, now, other_valves):
             for valve in [self] + other_valves:
                 valve.flood_manager.update_stack_topo(port_stack_up, self.dp, port)
 
-    def update_stack_link_states(self, now, other_valves):
+    def fast_state_expire(self, now, other_valves):
         """Called periodically to verify the state of stack ports."""
         for port in self.dp.stack_ports:
             self._update_stack_link_state(port, now, other_valves)

tests/unit/faucet/test_valve.py

@@ -603,8 +603,7 @@ def rcv_packet(self, port, vid, match):
             rcv_packet_ofmsgs = self.last_flows_to_dp[self.DP_ID]
             self.table.apply_ofmsgs(rcv_packet_ofmsgs)
             for valve_service in (
-                    'resolve_gateways', 'advertise',
-                    'send_lldp_beacons', 'state_expire'):
+                    'resolve_gateways', 'advertise', 'fast_advertise', 'state_expire'):
                 self.valves_manager.valve_flow_services(
                     now, valve_service)
             self.valves_manager.update_metrics(now)
@@ -1285,7 +1284,7 @@ def test_port_acl_deny(self):
         def test_lldp_beacon(self):
             """Test LLDP beacon service."""
             # TODO: verify LLDP packet content.
-            self.assertTrue(self.valve.send_lldp_beacons(time.time(), None))
+            self.assertTrue(self.valve.fast_advertise(time.time(), None))
 
         def test_unknown_port(self):
             """Test port status change for unknown port handled."""
@@ -1887,7 +1886,7 @@ def test_stack_probe(self):
         other_dp = self.valves_manager.valves[2].dp
         other_port = other_dp.ports[1]
         other_valves = self.valves_manager._other_running_valves(self.valve)
-        self.valve.update_stack_link_states(time.time(), other_valves)
+        self.valve.fast_state_expire(time.time(), other_valves)
         self.assertTrue(stack_port.is_stack_down())
         for change_func, check_func in [
                 ('stack_init', 'is_stack_init'),
@@ -1920,7 +1919,7 @@ def test_stack_lost_lldp(self):
         self.rcv_lldp(stack_port, other_dp, other_port)
         self.assertTrue(stack_port.is_stack_init())
         other_valves = self.valves_manager._other_running_valves(self.valve)
-        self.valve.update_stack_link_states(time.time() + 300, other_valves) # simulate packet loss
+        self.valve.fast_state_expire(time.time() + 300, other_valves) # simulate packet loss
         self.assertTrue(stack_port.is_stack_down())
 
 
@@ -1947,7 +1946,7 @@ def down_stack_port(port):
             peer_port.stack_down()
             self.valves_manager.valve_flow_services(
                 time.time() + 600,
-                'update_stack_link_states')
+                'fast_state_expire')
             self.assertTrue(port.is_stack_down())
 
         def verify_stack_learn_edges(num_edges, edge=None, test_func=None):