From 6a38f37e85e85b56439189dffde07a4d43a5f3e4 Mon Sep 17 00:00:00 2001 From: Josh Bailey Date: Fri, 31 Jul 2020 10:10:09 +1200 Subject: [PATCH 1/4] Return deduped connect msgs. --- clib/valve_test_lib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clib/valve_test_lib.py b/clib/valve_test_lib.py index b39b9fff38..0c4c5c0df9 100644 --- a/clib/valve_test_lib.py +++ b/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()) From 00dc7e5f24b1e2a121407c11a82d2fcadd676090 Mon Sep 17 00:00:00 2001 From: Josh Bailey Date: Fri, 31 Jul 2020 10:52:09 +1200 Subject: [PATCH 2/4] Do not ever output a no-op apply actions instruction. --- clib/fakeoftable.py | 9 ++++----- faucet/valve_table.py | 24 +++++++++++++++--------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/clib/fakeoftable.py b/clib/fakeoftable.py index 660c9595fd..b3b747e1c1 100644 --- a/clib/fakeoftable.py +++ b/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(',') diff --git a/faucet/valve_table.py b/faucet/valve_table.py index 49cf8241f4..4af12c8e89 100644 --- a/faucet/valve_table.py +++ b/faucet/valve_table.py @@ -147,16 +147,22 @@ def _trim_actions(self, actions): return new_actions def _trim_inst(self, inst): - """Discard actions on packets that are not output and not goto another table.""" + """Discard empty/actions on packets that are not output and not goto another table.""" inst_types = {instruction.type for instruction in inst} - if valve_of.ofp.OFPIT_GOTO_TABLE in inst_types: - return inst - new_inst = [] - for instruction in inst: - if instruction.type == valve_of.ofp.OFPIT_APPLY_ACTIONS: - instruction.actions = self._trim_actions(instruction.actions) - new_inst.append(instruction) - return new_inst + if valve_of.ofp.OFPIT_APPLY_ACTIONS in inst_types: + goto_present = valve_of.ofp.OFPIT_GOTO_TABLE in inst_types + new_inst = [] + for instruction in inst: + if instruction.type == valve_of.ofp.OFPIT_APPLY_ACTIONS: + # If no goto present, this is the last set of actions that can take place + if not goto_present: + instruction.actions = self._trim_actions(instruction.actions) + # Always drop an apply actions instruction with no actions. + if not instruction.actions: + continue + new_inst.append(instruction) + return new_inst + return inst def flowmod(self, match=None, priority=None, # pylint: disable=too-many-arguments inst=None, command=valve_of.ofp.OFPFC_ADD, out_port=0, From 14c473ed3df3492a439b715a4d93be09840449e4 Mon Sep 17 00:00:00 2001 From: Josh Bailey Date: Fri, 31 Jul 2020 11:42:38 +1200 Subject: [PATCH 3/4] Make more OF instruction operations cacheable and verify cache is used. --- faucet/valve.py | 2 +- faucet/valve_of.py | 11 ++++++++++- faucet/valve_route.py | 4 ++-- faucet/valve_switch_standalone.py | 2 +- faucet/valve_table.py | 5 +++-- faucet/vlan.py | 2 +- tests/unit/faucet/test_valve_config.py | 4 ++++ 7 files changed, 22 insertions(+), 8 deletions(-) diff --git a/faucet/valve.py b/faucet/valve.py index 5f836163eb..d688b21466 100644 --- a/faucet/valve.py +++ b/faucet/valve.py @@ -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.""" diff --git a/faucet/valve_of.py b/faucet/valve_of.py index 25bf50e0d4..9e65d40fc1 100644 --- a/faucet/valve_of.py +++ b/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. diff --git a/faucet/valve_route.py b/faucet/valve_route.py index 01510496c7..d202e72aba 100644 --- a/faucet/valve_route.py +++ b/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): diff --git a/faucet/valve_switch_standalone.py b/faucet/valve_switch_standalone.py index d63c3ad747..72cd8dc30e 100644 --- a/faucet/valve_switch_standalone.py +++ b/faucet/valve_switch_standalone.py @@ -820,7 +820,7 @@ def lacp_req_reply(self, lacp_pkt, port): actor_state_collecting=actor_state_col, actor_state_distributing=actor_state_dist) self.logger.debug('Sending LACP %s on %s activity %s' % (pkt, port, actor_state_activity)) - return [valve_of.packetout(port.number, pkt.data)] + return [valve_of.packetout(port.number, bytes(pkt.data))] def get_lacp_dpid_nomination(self, lacp_id, valve, other_valves): # pylint: disable=unused-argument """Chooses the DP for a given LAG. diff --git a/faucet/valve_table.py b/faucet/valve_table.py index 4af12c8e89..3ab187cf25 100644 --- a/faucet/valve_table.py +++ b/faucet/valve_table.py @@ -146,6 +146,7 @@ def _trim_actions(self, actions): 'unexpected set fields %s configured %s in %s' % (set_fields, self.set_fields, self.name)) return new_actions + @functools.lru_cache() def _trim_inst(self, inst): """Discard empty/actions on packets that are not output and not goto another table.""" inst_types = {instruction.type for instruction in inst} @@ -173,14 +174,14 @@ def flowmod(self, match=None, priority=None, # pylint: disable=too-many-argument if not match: match = self.match() if inst is None: - inst = [] + inst = () if cookie is None: cookie = self.flow_cookie flags = 0 if self.notify_flow_removed: flags = valve_of.ofp.OFPFF_SEND_FLOW_REM if inst: - inst = self._trim_inst(inst) + inst = self._trim_inst(tuple(inst)) flowmod = valve_of.flowmod( cookie, command, diff --git a/faucet/vlan.py b/faucet/vlan.py index 652d984a94..959cc982d7 100644 --- a/faucet/vlan.py +++ b/faucet/vlan.py @@ -554,7 +554,7 @@ def pkt_out_port(self, packet_builder, port, *args): if self.port_is_tagged(port): vid = self.vid pkt = packet_builder(vid, *args) - return valve_of.packetout(port.number, pkt.data) + return valve_of.packetout(port.number, bytes(pkt.data)) def flood_pkt(self, packet_builder, multi_out=True, *args): """Return Packet-out actions via flooding""" diff --git a/tests/unit/faucet/test_valve_config.py b/tests/unit/faucet/test_valve_config.py index 4fd8119023..eef1ccb357 100755 --- a/tests/unit/faucet/test_valve_config.py +++ b/tests/unit/faucet/test_valve_config.py @@ -860,6 +860,10 @@ def load_orig_config(): total_tt_prop = pstats_out.total_tt / self.baseline_total_tt # pytype: disable=attribute-error # must not be 15x slower, to ingest config for 100 interfaces than 1. if total_tt_prop < 15: + for valve in self.valves_manager.valves.values(): + for table in valve.dp.tables.values(): + cache_info = table._trim_inst.cache_info() + self.assertGreater(cache_info.hits, cache_info.misses, msg=cache_info) return time.sleep(i) From 7b341faa01b3bed6960924d7d4d9987054dcf825 Mon Sep 17 00:00:00 2001 From: Josh Bailey Date: Fri, 31 Jul 2020 11:53:12 +1200 Subject: [PATCH 4/4] Make flowmod creation cacheable. --- faucet/valve.py | 2 +- faucet/valve_acl.py | 22 +++++++++++----------- faucet/valve_coprocessor.py | 8 ++++---- faucet/valve_of.py | 1 + faucet/valve_pipeline.py | 17 ++++++++++++----- faucet/valve_route.py | 10 +++++----- faucet/valve_switch_stack.py | 2 +- faucet/valve_switch_standalone.py | 15 ++++++--------- faucet/valve_table.py | 14 +++++++------- tests/unit/faucet/test_valve_of.py | 4 ++-- tests/unit/faucet/test_valve_stack.py | 2 +- 11 files changed, 51 insertions(+), 46 deletions(-) diff --git a/faucet/valve.py b/faucet/valve.py index d688b21466..36d77493d7 100644 --- a/faucet/valve.py +++ b/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)) diff --git a/faucet/valve_acl.py b/faucet/valve_acl.py index bde89b1750..edf5829306 100644 --- a/faucet/valve_acl.py +++ b/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,9 +436,9 @@ 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( @@ -446,11 +446,11 @@ def create_dot1x_flow_pair(self, port_num, nfv_sw_port_num, mac): 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): diff --git a/faucet/valve_coprocessor.py b/faucet/valve_coprocessor.py index 25f31451b8..cd633d5154 100644 --- a/faucet/valve_coprocessor.py +++ b/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) diff --git a/faucet/valve_of.py b/faucet/valve_of.py index 9e65d40fc1..41d52dd8e0 100644 --- a/faucet/valve_of.py +++ b/faucet/valve_of.py @@ -756,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( diff --git a/faucet/valve_pipeline.py b/faucet/valve_pipeline.py index b903c5f62b..da6aa3864c 100644 --- a/faucet/valve_pipeline.py +++ b/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 diff --git a/faucet/valve_route.py b/faucet/valve_route.py index d202e72aba..7f2895b9fc 100644 --- a/faucet/valve_route.py +++ b/faucet/valve_route.py @@ -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): diff --git a/faucet/valve_switch_stack.py b/faucet/valve_switch_stack.py index c79728276a..77804f62a4 100644 --- a/faucet/valve_switch_stack.py +++ b/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, diff --git a/faucet/valve_switch_standalone.py b/faucet/valve_switch_standalone.py index 72cd8dc30e..f499b9911e 100644 --- a/faucet/valve_switch_standalone.py +++ b/faucet/valve_switch_standalone.py @@ -155,7 +155,7 @@ def _build_flood_rule(self, match, command, flood_acts, flood_priority): return self.flood_table.flowmod( match=match, command=command, - inst=[valve_of.apply_actions(flood_acts)], + inst=(valve_of.apply_actions(flood_acts),), priority=flood_priority) @functools.lru_cache(maxsize=1024) @@ -309,7 +309,7 @@ def _build_group_flood_rules(self, vlan, modify, command): ofmsgs.append(self.flood_table.flowmod( match=match, command=command, - inst=[valve_of.apply_actions([valve_of.group_act(group.group_id)])], + inst=(valve_of.apply_actions((valve_of.group_act(group.group_id),)),), priority=flood_priority)) return ofmsgs @@ -318,7 +318,7 @@ def add_vlan(self, vlan, cold_start): ofmsgs.append(self.eth_src_table.flowcontroller( match=self.eth_src_table.match(vlan=vlan), priority=self.low_priority, - inst=[self.eth_src_table.goto(self.output_table)])) + inst=(self.eth_src_table.goto(self.output_table),))) ofmsgs.extend(self._build_flood_rules(vlan, cold_start)) return ofmsgs @@ -346,9 +346,9 @@ def _port_add_vlan_rules(self, port, vlan, mirror_act, push_vlan=True): actions.append(self.vlan_table.set_no_external_forwarding_requested()) else: actions.append(self.vlan_table.set_external_forwarding_requested()) - inst = [ + inst = ( valve_of.apply_actions(actions), - self.vlan_table.goto(self._find_forwarding_table(vlan))] + self.vlan_table.goto(self._find_forwarding_table(vlan))) return self.vlan_table.flowmod( self.vlan_table.match(in_port=port.number, vlan=match_vlan), priority=self.low_priority, inst=inst) @@ -559,10 +559,7 @@ def learn_host_on_vlan_port_flows(self, port, vlan, eth_src, in_port=port.number, vlan=vlan, eth_src=eth_src) src_priority = self.host_priority - 1 - inst = [] - - inst.append(self.eth_src_table.goto(self.output_table)) - + inst = (self.eth_src_table.goto(self.output_table),) ofmsgs.append(self.eth_src_table.flowmod( match=src_match, priority=src_priority, diff --git a/faucet/valve_table.py b/faucet/valve_table.py index 3ab187cf25..9c8bba2b69 100644 --- a/faucet/valve_table.py +++ b/faucet/valve_table.py @@ -162,7 +162,7 @@ def _trim_inst(self, inst): if not instruction.actions: continue new_inst.append(instruction) - return new_inst + return tuple(new_inst) return inst def flowmod(self, match=None, priority=None, # pylint: disable=too-many-arguments @@ -181,7 +181,7 @@ def flowmod(self, match=None, priority=None, # pylint: disable=too-many-argument if self.notify_flow_removed: flags = valve_of.ofp.OFPFF_SEND_FLOW_REM if inst: - inst = self._trim_inst(tuple(inst)) + inst = self._trim_inst(inst) flowmod = valve_of.flowmod( cookie, command, @@ -190,7 +190,7 @@ def flowmod(self, match=None, priority=None, # pylint: disable=too-many-argument out_port, out_group, match, - inst, + tuple(inst), hard_timeout, idle_timeout, flags) @@ -212,17 +212,17 @@ def flowdrop(self, match=None, priority=None, hard_timeout=0): match=match, priority=priority, hard_timeout=hard_timeout, - inst=[]) + inst=()) def flowcontroller(self, match=None, priority=None, inst=None, max_len=96): """Add flow outputting to controller.""" if inst is None: - inst = [] + inst = () return self.flowmod( match=match, priority=priority, - inst=[valve_of.apply_actions( - [valve_of.output_controller(max_len)])] + inst) + inst=(valve_of.apply_actions( + (valve_of.output_controller(max_len),)),) + inst) class ValveGroupEntry: diff --git a/tests/unit/faucet/test_valve_of.py b/tests/unit/faucet/test_valve_of.py index 71a9dac42e..137336e599 100755 --- a/tests/unit/faucet/test_valve_of.py +++ b/tests/unit/faucet/test_valve_of.py @@ -39,11 +39,11 @@ def test_delete_order(self): global_groupdel = valve_of.groupdel(group_id=valve_of.ofp.OFPG_ALL) global_flowdel = valve_of.flowmod( cookie=None, hard_timeout=None, idle_timeout=None, match_fields=None, out_port=None, - table_id=valve_of.ofp.OFPTT_ALL, inst=[], priority=0, command=valve_of.ofp.OFPFC_DELETE, + table_id=valve_of.ofp.OFPTT_ALL, inst=(), priority=0, command=valve_of.ofp.OFPFC_DELETE, out_group=valve_of.ofp.OFPG_ANY) flowdel = valve_of.flowmod( cookie=None, hard_timeout=None, idle_timeout=None, match_fields=None, out_port=None, - table_id=9, inst=[], priority=0, command=valve_of.ofp.OFPFC_DELETE, + table_id=9, inst=(), priority=0, command=valve_of.ofp.OFPFC_DELETE, out_group=valve_of.ofp.OFPG_ANY) flow = valve_of.output_port(1) flows = [flowdel, flow, flow, flow, global_flowdel, global_groupdel] diff --git a/tests/unit/faucet/test_valve_stack.py b/tests/unit/faucet/test_valve_stack.py index 7b687e49f7..b411e48a79 100755 --- a/tests/unit/faucet/test_valve_stack.py +++ b/tests/unit/faucet/test_valve_stack.py @@ -2532,7 +2532,7 @@ def test_groupdel_exists(self): global_flowmod = valve_of.flowmod( 0, ofp.OFPFC_DELETE, ofp.OFPTT_ALL, 0, ofp.OFPP_CONTROLLER, ofp.OFPP_CONTROLLER, - valve_of.match_from_dict({}), [], 0, 0, 0) + valve_of.match_from_dict({}), (), 0, 0, 0) self.check_groupmods_exist( valve_of.valve_flowreorder(ofmsgs + [global_flowmod])) global_metermod = valve_of.meterdel()