Merge pull request #3504 from mab68/groupdel

Only remove delete ofmsgs that have a global delete of the same type
faucetsdn · Mar 29, 2020 · c8c1fd5 · c8c1fd5
2 parents 5f67e50 + 13c656b
commit c8c1fd5
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 2 deletions.
diff --git a/faucet/valve_of.py b/faucet/valve_of.py
@@ -983,10 +983,17 @@ def valve_flowreorder(input_ofmsgs, use_barriers=True):
     output_ofmsgs = []
     by_kind = _partition_ofmsgs(input_ofmsgs)
 
-    # Suppress all other deletes if a global delete is present.
+    # Suppress all other relevant deletes if a global delete is present.
     delete_global_ofmsgs = by_kind.get('deleteglobal', [])
     if delete_global_ofmsgs:
-        by_kind['delete'] = []
+        global_types = []
+        for ofmsg in delete_global_ofmsgs:
+            global_types.append(type(ofmsg))
+        new_delete = []
+        for ofmsg in by_kind.get('delete', []):
+            if type(ofmsg) not in global_types:
+                new_delete.append(ofmsg)
+        by_kind['delete'] = new_delete
 
     for kind, random_order, suggest_barrier in _OFMSG_ORDER:
         ofmsgs = dedupe_ofmsgs(by_kind.get(kind, []), random_order)

diff --git a/tests/unit/faucet/test_valve_stack.py b/tests/unit/faucet/test_valve_stack.py
@@ -2232,5 +2232,75 @@ def test_topo(self):
         self.assertTrue(dp_obj.is_stack_edge())
 
 
+class GroupDeleteACLTestCase(ValveTestBases.ValveTestSmall):
+    """Test that a group ACL creates a groupdel for the group_id"""
+
+    CONFIG = """
+acls:
+    group-acl:
+        - rule:
+            dl_dst: "0e:00:00:00:02:02"
+            actions:
+                output:
+                    failover:
+                        group_id: 1001
+                        ports: [2, 3]
+vlans:
+    vlan100:
+        vid: 100
+dps:
+    s1:
+        dp_id: 0x1
+        hardware: 'GenericTFM'
+        interfaces:
+            1:
+                native_vlan: vlan100
+                acls_in: [group-acl]
+            2:
+                native_vlan: vlan100
+            3:
+                native_vlan: vlan100
+"""
+
+    def setUp(self):
+        self.setup_valve(self.CONFIG)
+
+    def check_groupmods_exist(self, ofmsgs, groupdel_exists=True):
+        """Test that the ACL groupmods exist when expected"""
+        groupdel = None
+        groupmod = None
+        for ofmsg in ofmsgs:
+            if valve_of.is_groupdel(ofmsg) and not valve_of.is_global_groupdel(ofmsg):
+                groupdel = ofmsg
+            elif valve_of.is_groupmod(ofmsg):
+                groupmod = ofmsg
+        self.assertIsNotNone(groupmod)
+        if groupdel_exists:
+            self.assertIsNotNone(groupdel)
+            self.assertTrue(groupdel.group_id, 1001)
+        else:
+            self.assertIsNone(groupdel)
+
+    def test_groupdel_exists(self):
+        """Test valve_flowreorder doesn't remove groupmods unless expected"""
+        valve = self.valves_manager.valves[0x1]
+        port = valve.dp.ports[1]
+        ofmsgs = valve.acl_manager.add_port(port)
+        self.check_groupmods_exist(valve_of.valve_flowreorder(ofmsgs))
+        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)
+        self.check_groupmods_exist(
+            valve_of.valve_flowreorder(ofmsgs + [global_flowmod]))
+        global_metermod = valve_of.meterdel()
+        self.check_groupmods_exist(
+            valve_of.valve_flowreorder(ofmsgs + [global_flowmod, global_metermod]))
+        global_groupmod = valve_of.groupdel()
+        self.check_groupmods_exist(
+            valve_of.valve_flowreorder(
+                ofmsgs + [global_flowmod, global_metermod, global_groupmod]), False)
+
+
 if __name__ == "__main__":
     unittest.main()  # pytype: disable=module-attr