Merge pull request #2940 from anarkiwi/tunnelsamedp

Handle tunnels with src and dst on same DP, and move tunnel rule generation to stack state changes.

faucet/acl.py

@@ -326,8 +326,9 @@ def update_tunnel_acl_conf(self, dp):
                 output_rule = tunnel_rule['actions']['output']
                 orig_output_rule = copy.deepcopy(output_rule)
                 if dp == dst_dp:
-                    if not 'pop_vlans' in output_rule:
-                        output_rule['pop_vlans'] = 1
+                    if src_dp != dst_dp:
+                        if not 'pop_vlans' in output_rule:
+                            output_rule['pop_vlans'] = 1
                     if not 'port' in output_rule:
                         output_rule['port'] = dst_port
                 else:

faucet/valve.py

@@ -552,18 +552,20 @@ def _next_stack_link_state(self, port, now):
 
     def _update_stack_link_state(self, port, now, other_valves):
         next_state = self._next_stack_link_state(port, now)
-        if next_state is None:
-            return
-        next_state()
-        self._set_var(
-            'port_stack_state',
-            port.dyn_stack_current_state,
-            labels=self.dp.port_labels(port.number))
-        if port.is_stack_up() or port.is_stack_down():
-            port_stack_up = port.is_stack_up()
-            for valve in [self] + other_valves:
-                valve.flood_manager.update_stack_topo(port_stack_up, self.dp, port)
-                valve.update_tunnel_flowrules()
+        ofmsgs_by_valve = {}
+        if next_state is not None:
+            next_state()
+            self._set_var(
+                'port_stack_state',
+                port.dyn_stack_current_state,
+                labels=self.dp.port_labels(port.number))
+            if port.is_stack_up() or port.is_stack_down():
+                port_stack_up = port.is_stack_up()
+                for valve in [self] + other_valves:
+                    valve.flood_manager.update_stack_topo(port_stack_up, self.dp, port)
+                    valve.update_tunnel_flowrules()
+                ofmsgs_by_valve[self] = self.get_tunnel_flowmods()
+        return ofmsgs_by_valve
 
     def update_tunnel_flowrules(self):
         """Update tunnel ACL rules because the stack topology has changed"""
@@ -582,9 +584,10 @@ def get_tunnel_flowmods(self):
 
     def fast_state_expire(self, now, other_valves):
         """Called periodically to verify the state of stack ports."""
+        ofmsgs_by_valve = {}
         for port in self.dp.stack_ports:
-            self._update_stack_link_state(port, now, other_valves)
-        return {}
+            ofmsgs_by_valve.update(self._update_stack_link_state(port, now, other_valves))
+        return ofmsgs_by_valve
 
     def _reset_dp_status(self):
         if self.dp.dyn_running:
@@ -959,7 +962,7 @@ def _verify_stack_lldp(self, port, now, other_valves,
                            remote_dp_id, remote_dp_name,
                            remote_port_id, remote_port_state):
         if not port.stack:
-            return
+            return {}
         remote_dp = port.stack['dp']
         remote_port = port.stack['port']
         stack_correct = True
@@ -986,7 +989,7 @@ def _verify_stack_lldp(self, port, now, other_valves,
             'remote_port_id': remote_port_id,
             'remote_port_state': remote_port_state
         }
-        self._update_stack_link_state(port, now, other_valves)
+        return self._update_stack_link_state(port, now, other_valves)
 
     def lldp_handler(self, now, pkt_meta, other_valves):
         """Handle an LLDP packet.
@@ -995,25 +998,27 @@ def lldp_handler(self, now, pkt_meta, other_valves):
             pkt_meta (PacketMeta): packet for control plane.
         """
         if pkt_meta.eth_type != valve_of.ether.ETH_TYPE_LLDP:
-            return
+            return {}
         pkt_meta.reparse_all()
         lldp_pkt = valve_packet.parse_lldp(pkt_meta.pkt)
         if not lldp_pkt:
-            return
+            return {}
 
         port = pkt_meta.port
         (remote_dp_id, remote_dp_name,
          remote_port_id, remote_port_state) = valve_packet.parse_faucet_lldp(
              lldp_pkt, self.dp.faucet_dp_mac)
 
+        ofmsgs_by_valve = {}
         if remote_dp_id and remote_port_id:
             self.logger.info('FAUCET LLDP from %s (remote %s, port %u)' % (
                 pkt_meta.log(), valve_util.dpid_log(remote_dp_id), remote_port_id))
-            self._verify_stack_lldp(
+            ofmsgs_by_valve.update(self._verify_stack_lldp(
                 port, now, other_valves,
                 remote_dp_id, remote_dp_name,
-                remote_port_id, remote_port_state)
+                remote_port_id, remote_port_state))
         self.logger.debug('LLDP from %s: %s' % (pkt_meta.log(), str(lldp_pkt)))
+        return ofmsgs_by_valve
 
     @staticmethod
     def _control_plane_handler(now, pkt_meta, route_manager):
@@ -1252,11 +1257,7 @@ def _non_vlan_rcv_packet(self, now, other_valves, pkt_meta):
             if lacp_ofmsgs_by_valve:
                 return lacp_ofmsgs_by_valve
         # TODO: verify LLDP message (e.g. org-specific authenticator TLV)
-        self.lldp_handler(now, pkt_meta, other_valves)
-        tunnel_ofmsgs = self.get_tunnel_flowmods()
-        if tunnel_ofmsgs:
-            return {self: tunnel_ofmsgs}
-        return {}
+        return self.lldp_handler(now, pkt_meta, other_valves)
 
     def _router_rcv_packet(self, now, _other_valves, pkt_meta):
         if not pkt_meta.vlan.faucet_vips:

tests/integration/mininet_tests.py

@@ -7166,6 +7166,76 @@ def test_port5002_notblocked(self):
         self.verify_no_cable_errors()
 
 
+class FaucetTunnelSameDpTest(FaucetStringOfDPTest):
+
+    NUM_DPS = 2
+    NUM_HOSTS = 2
+    SWITCH_TO_SWITCH_LINKS = 2
+    VID = 100
+    ACLS = {
+        1: [
+            {'rule': {
+                'dl_type': IPV4_ETH,
+                'ip_proto': 1,
+                'actions': {
+                    'allow': 0,
+                    'output': {
+                        'tunnel': {
+                            'type': 'vlan',
+                            'tunnel_id': 200,
+                            'dp': 'faucet-1',
+                            'port': mininet_test_topo.SWITCH_START_PORT + 1}
+                    }
+                }
+            }}
+        ]
+    }
+
+    # DP-to-acl_in port mapping.
+    ACL_IN_DP = {
+        'faucet-1': {
+            # First port 1, acl_in = 1
+            mininet_test_topo.SWITCH_START_PORT: 1,
+        }
+    }
+
+    def setUp(self): # pylint: disable=invalid-name
+        super(FaucetTunnelSameDpTest, self).setUp()
+        self.build_net(
+            stack=True,
+            n_dps=self.NUM_DPS,
+            n_untagged=self.NUM_HOSTS,
+            untagged_vid=self.VID,
+            acls=self.ACLS,
+            acl_in_dp=self.ACL_IN_DP,
+            switch_to_switch_links=self.SWITCH_TO_SWITCH_LINKS,
+            hw_dpid=self.hw_dpid,
+        )
+        self.start_net()
+
+    def verify_tunnel_established(self, src_host, dst_host, other_host, packets=1):
+        """Verify ICMP packets tunnelled from src to dst."""
+        icmp_match = {'eth_type': IPV4_ETH, 'ip_proto': 1, 'in_port': self.port_map['port_1']}
+        self.wait_until_matching_flow(icmp_match, table_id=self._PORT_ACL_TABLE)
+        tcpdump_text = self.tcpdump_helper(
+            dst_host, 'icmp', [
+                # need to set static ARP as only ICMP is tunnelled.
+                lambda: src_host.cmd('arp -s %s %s' % (other_host.IP(), other_host.MAC())),
+                lambda: src_host.cmd('ping -c%u -t1 %s' % (packets, other_host.IP()))
+            ],
+        )
+        self.wait_nonzero_packet_count_flow(icmp_match, table_id=self._PORT_ACL_TABLE)
+        self.assertTrue(re.search(
+            '%s: ICMP echo request' % other_host.IP(), tcpdump_text
+        ), 'Tunnel was not established')
+
+    def test_tunnel_established(self):
+        """Test a tunnel path can be created."""
+        self.verify_all_stack_up()
+        src_host, dst_host, other_host = self.net.hosts[:3]
+        self.verify_tunnel_established(src_host, dst_host, other_host)
+
+
 class FaucetTunnelTest(FaucetStringOfDPTest):
 
     NUM_DPS = 2