From ade634090de244f8c3cbd08059928db36a761dd9 Mon Sep 17 00:00:00 2001
From: madhanmellanox <62459540+madhanmellanox@users.noreply.github.com>
Date: Wed, 15 Jul 2020 10:24:44 -0700
Subject: [PATCH] [caclmgrd] Log error message if IPv4 ACL table contains IPv6
 rule and vice-versa (#4498)

* Defect 2082949: Handling Control Plane ACLs so that IPv4 rules and IPv6 rules are not added to the same ACL table

* Previous code review comments of coming up with functions for is_ipv4_rule and is_ipv6_rule is addressed and also raising Exceptions instead of simply aborting when the conflict occurs is handled

* Addressed code review comment to replace duplicate code with already existing functions

* removed raising Exception when rule conflict in Control plane ACLs are found

* added code to remove the rule_props if it is conflicting ACL table versioning rule

* addressed review comment to add ignoring rule in the error statement

Co-authored-by: Madhan Babu <madhan@arc-build-server.mtr.labs.mlnx>
---
 files/image_config/caclmgrd/caclmgrd | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/files/image_config/caclmgrd/caclmgrd b/files/image_config/caclmgrd/caclmgrd
index b6d5cbe713..3f0c7f93ff 100755
--- a/files/image_config/caclmgrd/caclmgrd
+++ b/files/image_config/caclmgrd/caclmgrd
@@ -168,6 +168,19 @@ class ControlPlaneAclManager(object):
 
         return block_ip2me_cmds
 
+    def is_rule_ipv4(self, rule_props):
+        if (("SRC_IP" in rule_props and rule_props["SRC_IP"]) or
+            ("DST_IP" in rule_props and rule_props["DST_IP"])):
+            return True
+        else:
+            return False
+
+    def is_rule_ipv6(self, rule_props):
+        if (("SRC_IPV6" in rule_props and rule_props["SRC_IPV6"]) or
+            ("DST_IPV6" in rule_props and rule_props["DST_IPV6"])):
+            return True
+        else:
+            return False
 
     def get_acl_rules_and_translate_to_iptables_commands(self):
         """
@@ -295,13 +308,20 @@ class ControlPlaneAclManager(object):
                         # try to do it now. We attempt to determine heuristically based on
                         # whether the src or dst IP of this rule is an IPv4 or IPv6 address.
                         if not table_ip_version:
-                            if (("SRC_IPV6" in rule_props and rule_props["SRC_IPV6"]) or
-                                ("DST_IPV6" in rule_props and rule_props["DST_IPV6"])):
+                            if self.is_rule_ipv6(rule_props):
                                 table_ip_version = 6
-                            elif (("SRC_IP" in rule_props and rule_props["SRC_IP"]) or
-                                  ("DST_IP" in rule_props and rule_props["DST_IP"])):
+                            elif self.is_rule_ipv4(rule_props):
                                 table_ip_version = 4
 
+                        if (self.is_rule_ipv6(rule_props) and (table_ip_version == 4)):
+                            log_error("CtrlPlane ACL table {} is a IPv4 based table and rule {} is a IPV6 rule! Ignoring rule."
+                                                          .format(table_name, rule_id))
+                            acl_rules.pop(rule_props["PRIORITY"])
+                        elif (self.is_rule_ipv4(rule_props) and (table_ip_version == 6)):
+                            log_error("CtrlPlane ACL table {} is a IPv6 based table and rule {} is a IPV4 rule! Ignroing rule."
+                                                          .format(table_name, rule_id))
+                            acl_rules.pop(rule_props["PRIORITY"])
+
                 # If we were unable to determine whether this ACL table contains
                 # IPv4 or IPv6 rules, log a message and skip processing this table.
                 if not table_ip_version: