Merge pull request #3643 from anarkiwi/noact

Do not ever output a no-op apply actions instruction, and make FlowMod() and instruction generation cacheable.

clib/fakeoftable.py

@@ -519,6 +519,8 @@ def get_table_output(self, match, table_id, trace=False):
                     else:
                         raise FakeOFTableException('goto to lower table ID')
                 elif instruction.type == ofp.OFPIT_APPLY_ACTIONS:
+                    if not instruction.actions:
+                        raise FakeOFTableException('no-op instruction actions')
                     instruction_outputs, packet_dict, pending_actions = self._process_instruction(
                         packet_dict, instruction)
                     for out_port, out_pkts in instruction_outputs.items():
@@ -1050,11 +1052,8 @@ def __str__(self):
             if isinstance(instruction, parser.OFPInstructionGotoTable):
                 result += ' goto {}'.format(instruction.table_id)
             elif isinstance(instruction, parser.OFPInstructionActions):
-                if instruction.actions:
-                    for action in instruction.actions:
-                        result += " {},".format(self._pretty_action_str(action))
-                else:
-                    result += ' drop'
+                for action in instruction.actions:
+                    result += " {},".format(self._pretty_action_str(action))
             else:
                 result += str(instruction)
         result = result.rstrip(',')

clib/valve_test_lib.py

@@ -833,7 +833,7 @@ def connect_dp(self, dp_id=None):
             connect_msgs = (
                 valve.switch_features(None) +
                 valve.datapath_connect(self.mock_time(10), discovered_up_ports))
-            self.apply_ofmsgs(connect_msgs, dp_id)
+            connect_msgs = self.apply_ofmsgs(connect_msgs, dp_id)
             self.valves_manager.update_config_applied(sent={dp_id: True})
             self.assertEqual(1, int(self.get_prom('dp_status', dp_id=dp_id)))
             self.assertTrue(valve.dp.to_conf())

faucet/valve.py

@@ -340,7 +340,7 @@ def _add_default_drop_flows(self):
                 miss_table = self.dp.tables[miss_table_name]
                 ofmsgs.append(table.flowmod(
                     priority=self.dp.lowest_priority,
-                    inst=[table.goto_miss(miss_table)]))
+                    inst=(table.goto_miss(miss_table),)))
             else:
                 ofmsgs.append(table.flowdrop(
                     priority=self.dp.lowest_priority))
@@ -562,7 +562,7 @@ def _send_lldp_beacon_on_port(self, port, now):
             system_name=system_name,
             port_descr=port.lldp_beacon['port_descr'])
         port.dyn_last_lldp_beacon_time = now
-        return valve_of.packetout(port.number, lldp_beacon_pkt.data)
+        return valve_of.packetout(port.number, bytes(lldp_beacon_pkt.data))
 
     def fast_advertise(self, now, _other_valves):
         """Called periodically to send LLDP/LACP packets."""

faucet/valve_acl.py

@@ -221,7 +221,7 @@ def build_tunnel_ofmsgs(rule_conf, acl_table, priority,
     if vlan_vid is not None:
         acl_match_dict['vlan_vid'] = valve_of.vid_present(vlan_vid)
     acl_match = valve_of.match_from_dict(acl_match_dict)
-    flowmod = acl_table.flowmod(acl_match, priority=priority, inst=acl_inst)
+    flowmod = acl_table.flowmod(acl_match, priority=priority, inst=tuple(acl_inst))
     if flowdel:
         ofmsgs.append(acl_table.flowdel(match=acl_match, priority=priority, strict=False))
     ofmsgs.append(flowmod)
@@ -244,7 +244,7 @@ def build_rule_ofmsgs(rule_conf, acl_table,
     if exact_match:
         priority = highest_priority
     flowmod = acl_table.flowmod(
-        acl_match, priority=priority, inst=acl_inst, cookie=acl_cookie)
+        acl_match, priority=priority, inst=tuple(acl_inst), cookie=acl_cookie)
     if flowdel:
         ofmsgs.append(acl_table.flowdel(match=acl_match, priority=priority))
     ofmsgs.append(flowmod)
@@ -342,7 +342,7 @@ def add_port(self, port):
             ofmsgs.append(self.port_acl_table.flowmod(
                 in_port_match,
                 priority=self.acl_priority,
-                inst=acl_allow_inst))
+                inst=tuple(acl_allow_inst)))
         return ofmsgs
 
     def cold_start_port(self, port):
@@ -376,7 +376,7 @@ def add_vlan(self, vlan, cold_start):
                 ofmsgs.append(self.egress_acl_table.flowmod(
                     self.egress_acl_table.match(vlan=vlan),
                     priority=self.acl_priority,
-                    inst=egress_acl_allow_inst
+                    inst=tuple(egress_acl_allow_inst),
                     ))
         return ofmsgs
 
@@ -385,7 +385,7 @@ def add_authed_mac(self, port_num, mac):
         return [self.port_acl_table.flowmod(
             self.port_acl_table.match(in_port=port_num, eth_src=mac),
             priority=self.auth_priority,
-            inst=self.pipeline.accept_to_vlan())]
+            inst=tuple(self.pipeline.accept_to_vlan()))]
 
     def del_authed_mac(self, port_num, mac=None, strict=True):
         """remove authed mac address"""
@@ -436,21 +436,21 @@ def create_dot1x_flow_pair(self, port_num, nfv_sw_port_num, mac):
                     in_port=port_num,
                     eth_type=valve_packet.ETH_EAPOL),
                 priority=self.override_priority,
-                inst=[valve_of.apply_actions([
+                inst=(valve_of.apply_actions([
                     self.port_acl_table.set_field(eth_dst=mac),
-                    valve_of.output_port(nfv_sw_port_num)])],
+                    valve_of.output_port(nfv_sw_port_num)]),),
             ),
             self.port_acl_table.flowmod(
                 match=self.port_acl_table.match(
                     in_port=nfv_sw_port_num,
                     eth_type=valve_packet.ETH_EAPOL,
                     eth_src=mac),
                 priority=self.override_priority,
-                inst=[valve_of.apply_actions([
+                inst=(valve_of.apply_actions([
                     self.port_acl_table.set_field(
                         eth_src=valve_packet.EAPOL_ETH_DST),
                     valve_of.output_port(port_num)
-                ])],
+                ]),),
             )
         ]
 
@@ -495,9 +495,9 @@ def create_mab_flow(self, port_num, nfv_sw_port_num, mac):
                 udp_dst=67,
             ),
             priority=self.low_priority,
-            inst=[valve_of.apply_actions([
+            inst=(valve_of.apply_actions([
                 self.port_acl_table.set_field(eth_dst=mac),
-                valve_of.output_port(nfv_sw_port_num)])],
+                valve_of.output_port(nfv_sw_port_num)]),),
         )
 
     def del_mab_flow(self, port_num, nfv_sw_port_num, mac):

faucet/valve_coprocessor.py

@@ -42,17 +42,17 @@ def add_port(self, port):
             ofmsgs.append(self.vlan_table.flowmod(
                 self.vlan_table.match(in_port=port.number),
                 priority=self.low_priority,
-                inst=[self.vlan_table.goto(self.copro_table)]))
+                inst=(self.vlan_table.goto(self.copro_table),)))
             ofmsgs.append(self.eth_src_table.flowmod(
                 match=self.eth_src_table.match(in_port=port.number),
                 priority=self.high_priority,
-                inst=[self.eth_src_table.goto(self.output_table)]))
+                inst=(self.eth_src_table.goto(self.output_table),)))
             # TODO: add additional output port strategies (eg. MPLS) and tagged ports
             vlan_vid_base = port.coprocessor.get('vlan_vid_base', 0)
             for port_number in self.ports:
-                inst = [valve_of.apply_actions([
+                inst = (valve_of.apply_actions((
                     valve_of.pop_vlan(),
-                    valve_of.output_port(port_number)])]
+                    valve_of.output_port(port_number))),)
                 vid = vlan_vid_base + port_number
                 vlan = OFVLAN(str(vid), vid)
                 match = self.copro_table.match(vlan=vlan)

faucet/valve_of.py

@@ -376,6 +376,11 @@ def apply_meter(meter_id):
     return parser.OFPInstructionMeter(meter_id, ofp.OFPIT_METER)
 
 
+@functools.lru_cache()
+def _apply_actions(actions):
+    return parser.OFPInstructionActions(ofp.OFPIT_APPLY_ACTIONS, actions)
+
+
 def apply_actions(actions):
     """Return instruction that applies action list.
 
@@ -384,9 +389,10 @@ def apply_actions(actions):
     Returns:
         ryu.ofproto.ofproto_v1_3_parser.OFPInstruction: instruction of actions.
     """
-    return parser.OFPInstructionActions(ofp.OFPIT_APPLY_ACTIONS, actions)
+    return _apply_actions(tuple(actions))
 
 
+@functools.lru_cache()
 def goto_table(table):
     """Return instruction to goto table.
 
@@ -413,6 +419,7 @@ def metadata_goto_table(metadata, mask, table):
         ]
 
 
+@functools.lru_cache()
 def set_field(**kwds):
     """Return action to set any field.
 
@@ -461,6 +468,7 @@ def push_vlan_act(table, vlan_vid, eth_type=ether.ETH_TYPE_8021Q):
     ]
 
 
+@functools.lru_cache()
 def dec_ip_ttl():
     """Return OpenFlow action to decrement IP TTL.
 
@@ -585,6 +593,7 @@ def packetouts(port_nums, data):
         data=data)
 
 
+@functools.lru_cache()
 def packetout(port_num, data):
     """Return OpenFlow action to packet out to dataplane from controller.
 
@@ -747,6 +756,7 @@ def build_match_dict(in_port=None, vlan=None, eth_type=None, eth_src=None,
     return match_dict
 
 
+@functools.lru_cache()
 def flowmod(cookie, command, table_id, priority, out_port, out_group,
             match_fields, inst, hard_timeout, idle_timeout, flags=0):
     return parser.OFPFlowMod(

faucet/valve_pipeline.py

@@ -18,10 +18,12 @@
 # limitations under the License.
 
 import copy
+import functools
 import faucet.faucet_metadata as faucet_md
 from faucet import valve_of
 from faucet.valve_manager_base import ValveManagerBase
 
+
 class ValvePipeline(ValveManagerBase):
     """Responsible for maintaing the integrity of the Faucet pipeline for a
     single valve.
@@ -46,12 +48,14 @@ def __init__(self, dp):
         self.select_priority = self._HIGH_PRIORITY
 
     @staticmethod
+    @functools.lru_cache()
     def _accept_to_table(table, actions):
         inst = [table.goto_this()]
-        if actions is not None:
+        if actions:
             inst.append(valve_of.apply_actions(actions))
-        return inst
+        return tuple(inst)
 
+    @functools.lru_cache()
     def accept_to_vlan(self, actions=None):
         """Get instructions to forward packet through the pipeline to
         vlan table.
@@ -62,6 +66,7 @@ def accept_to_vlan(self, actions=None):
         """
         return self._accept_to_table(self.vlan_table, actions)
 
+    @functools.lru_cache()
     def accept_to_classification(self, actions=None):
         """Get instructions to forward packet through the pipeline to
         classification table.
@@ -72,6 +77,7 @@ def accept_to_classification(self, actions=None):
         """
         return self._accept_to_table(self.classification_table, actions)
 
+    @functools.lru_cache()
     def accept_to_l2_forwarding(self, actions=None):
         """Get instructions to forward packet through the pipeline to l2
         forwarding.
@@ -82,6 +88,7 @@ def accept_to_l2_forwarding(self, actions=None):
         """
         return self._accept_to_table(self.output_table, actions)
 
+    @functools.lru_cache()
     def accept_to_egress(self, actions=None):
         """Get instructions to forward packet through the pipeline to egress
         table
@@ -123,7 +130,7 @@ def output(self, port, vlan, hairpin=False, external_forwarding_requested=None):
             instructions.append(valve_of.apply_actions(vlan.output_port(
                 port, hairpin=hairpin, output_table=self.output_table,
                 external_forwarding_requested=external_forwarding_requested)))
-        return instructions
+        return tuple(instructions)
 
     def initialise_tables(self):
         """Install rules to initialise the classification_table"""
@@ -167,7 +174,7 @@ def _add_egress_table_rule(self, port, vlan, pop_vlan=True):
         if pop_vlan:
             actions.append(valve_of.pop_vlan())
         actions.append(valve_of.output_port(port.number))
-        inst = [valve_of.apply_actions(actions)]
+        inst = (valve_of.apply_actions(tuple(actions)),)
         return self.egress_table.flowmod(
             self.egress_table.match(
                 vlan=vlan,
@@ -220,7 +227,7 @@ def select_packets(self, target_table, match_dict, actions=None,
         return [self.classification_table.flowmod(
             self.classification_table.match(**match_dict),
             priority=self.select_priority + priority_offset,
-            inst=inst)]
+            inst=tuple(inst))]
 
     def remove_filter(self, match_dict, strict=True, priority_offset=0):
         """retrieve flow mods to remove a filter from the classification table

faucet/valve_route.py

@@ -184,10 +184,10 @@ def _flood_stack_links(self, pkt_builder, vlan, multi_out=True, *args):
                 if running_port_nos:
                     random.shuffle(running_port_nos)
                     if multi_out:
-                        ofmsgs.append(valve_of.packetouts(running_port_nos, pkt.data))
+                        ofmsgs.append(valve_of.packetouts(running_port_nos, bytes(pkt.data)))
                     else:
                         ofmsgs.extend(
-                            [valve_of.packetout(port_no, pkt.data) for port_no in running_port_nos])
+                            [valve_of.packetout(port_no, bytes(pkt.data)) for port_no in running_port_nos])
         return ofmsgs
 
     def _resolve_gw_on_vlan(self, vlan, faucet_vip, ip_gw):
@@ -214,7 +214,7 @@ def _resolve_gw_on_port(self, vlan, port, faucet_vip, ip_gw, eth_dst):
     def _controller_and_flood(self):
         """Return instructions to forward packet to l2-forwarding"""
         return self.pipeline.accept_to_l2_forwarding(
-            actions=[valve_of.output_controller(max_len=self.MAX_PACKET_IN_SIZE)])
+            actions=(valve_of.output_controller(max_len=self.MAX_PACKET_IN_SIZE),))
 
     def _resolve_vip_response(self, pkt_meta, solicited_ip, now):
         """Learn host requesting for router, and return packet-out ofmsgs router response"""
@@ -297,7 +297,7 @@ def _nexthop_actions(self, eth_dst, vlan):
             self.fib_table.set_field(eth_dst=eth_dst)])
         if self.dec_ttl:
             actions.append(valve_of.dec_ip_ttl())
-        return actions
+        return tuple(actions)
 
     def _route_match(self, vlan, ip_dst):
         """Return vid, dst, eth_type flowrule match for fib entry"""
@@ -357,14 +357,14 @@ def _add_faucet_fib_to_vip(self, vlan, priority, faucet_vip, faucet_vip_host):
         ofmsgs.append(self.fib_table.flowmod(
             self._route_match(vlan, faucet_vip_host),
             priority=priority,
-            inst=[self.fib_table.goto(self.vip_table)]))
+            inst=(self.fib_table.goto(self.vip_table),)))
         if self.proactive_learn and not faucet_vip.ip.is_link_local:
             routed_vlans = self._routed_vlans(vlan)
             for routed_vlan in routed_vlans:
                 ofmsgs.append(self.fib_table.flowmod(
                     self._route_match(routed_vlan, faucet_vip),
                     priority=learn_connected_priority,
-                    inst=[self.fib_table.goto(self.vip_table)]))
+                    inst=(self.fib_table.goto(self.vip_table),)))
             # Unicast ICMP to us.
             priority -= 1
             ofmsgs.append(self.vip_table.flowcontroller(
@@ -1011,7 +1011,7 @@ def _add_faucet_fib_to_vip(self, vlan, priority, faucet_vip, faucet_vip_host):
         ofmsgs.append(self.fib_table.flowmod(
             self._route_match(vlan, faucet_vip_broadcast),
             priority=priority,
-            inst=[self.fib_table.goto(self.vip_table)]))
+            inst=(self.fib_table.goto(self.vip_table),)))
         return ofmsgs
 
     def _nd_solicit_handler(self, now, pkt_meta, _ipv6_pkt, icmpv6_pkt):

faucet/valve_switch_stack.py

@@ -171,7 +171,7 @@ def _learn_host_intervlan_routing_flows(self, port, vlan, eth_src, eth_dst):
         (src_rule_idle_timeout, src_rule_hard_timeout, _) = self._learn_host_timeouts(port, eth_src)
         src_match = self.eth_src_table.match(vlan=vlan, eth_src=eth_src, eth_dst=eth_dst)
         src_priority = self.host_priority - 1
-        inst = [self.eth_src_table.goto(self.output_table)]
+        inst = (self.eth_src_table.goto(self.output_table),)
         ofmsgs.extend([self.eth_src_table.flowmod(
             match=src_match,
             priority=src_priority,