Combine v4 and v6 L3 ACL rules on optimized platforms sonic-net#1267 (s…

…onic-net#2735)

* Combine v4 and v6 L3 ACL rules on optimized platforms sonic-net#1267

orchagent/aclorch.cpp

@@ -34,6 +34,7 @@ extern CrmOrch *gCrmOrch;
 
 #define STATE_DB_ACL_ACTION_FIELD_IS_ACTION_LIST_MANDATORY "is_action_list_mandatory"
 #define STATE_DB_ACL_ACTION_FIELD_ACTION_LIST "action_list"
+#define STATE_DB_ACL_L3V4V6_SUPPORTED "supported_L3V4V6"
 #define COUNTERS_ACL_COUNTER_RULE_MAP "ACL_COUNTER_RULE_MAP"
 
 #define ACL_COUNTER_DEFAULT_POLLING_INTERVAL_MS 10000 // ms
@@ -202,6 +203,26 @@ static acl_table_action_list_lookup_t defaultAclActionList =
  }
  }
  },
+ {
+ // L3V4V6
+ TABLE_TYPE_L3V4V6,
+ {
+ {
+ ACL_STAGE_INGRESS,
+ {
+ SAI_ACL_ACTION_TYPE_PACKET_ACTION,
+ SAI_ACL_ACTION_TYPE_REDIRECT
+ }
+ },
+ {
+ ACL_STAGE_EGRESS,
+ {
+ SAI_ACL_ACTION_TYPE_PACKET_ACTION,
+ SAI_ACL_ACTION_TYPE_REDIRECT
+ }
+ }
+ }
+ },
  {
  // MIRROR
  TABLE_TYPE_MIRROR,
@@ -2303,6 +2324,19 @@ bool AclTable::validate()
  return false;
  }
 
+ if (type.getName() == TABLE_TYPE_L3V4V6)
+ {
+ if (!m_pAclOrch->isAclL3V4V6TableSupported(stage))
+ {
+
+ SWSS_LOG_ERROR("Table %s: table type %s in stage %d not supported on this platform.",
+ id.c_str(), type.getName().c_str(), stage);
+ return false;
+ }
+ }
+
+
+
  if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage))
  {
  if (type.getActions().empty())
@@ -3060,11 +3094,36 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
  };
  }
 
+ if ( platform == MRVL_PLATFORM_SUBSTRING ||
+ platform == INVM_PLATFORM_SUBSTRING ||
+ platform == VS_PLATFORM_SUBSTRING)
+ {
+ m_L3V4V6Capability =
+ {
+ {ACL_STAGE_INGRESS, true},
+ {ACL_STAGE_EGRESS, true},
+ };
+ }
+ else
+ {
+ m_L3V4V6Capability =
+ {
+ {ACL_STAGE_INGRESS, false},
+ {ACL_STAGE_EGRESS, false},
+ };
+
+ }
+
+
  SWSS_LOG_NOTICE("%s switch capability:", platform.c_str());
  SWSS_LOG_NOTICE(" TABLE_TYPE_MIRROR: %s",
  m_mirrorTableCapabilities[TABLE_TYPE_MIRROR] ? "yes" : "no");
  SWSS_LOG_NOTICE(" TABLE_TYPE_MIRRORV6: %s",
  m_mirrorTableCapabilities[TABLE_TYPE_MIRRORV6] ? "yes" : "no");
+ SWSS_LOG_NOTICE(" TABLE_TYPE_L3V4V6: Ingress [%s], Egress [%s]",
+ m_L3V4V6Capability[ACL_STAGE_INGRESS] ? "yes" : "no",
+ m_L3V4V6Capability[ACL_STAGE_EGRESS] ? "yes" : "no");
+
 
  // In Mellanox platform, V4 and V6 rules are stored in different tables
  // In Broadcom DNX platform also, V4 and V6 rules are stored in different tables
@@ -3179,6 +3238,31 @@ void AclOrch::initDefaultTableTypes()
  .build()
  );
 
+
+ addAclTableType(
+ builder.withName(TABLE_TYPE_L3V4V6)
+ .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
+ .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG)
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_DST_IP))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS))
+ .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS))
+ .build()
+ );
+
  addAclTableType(
  builder.withName(TABLE_TYPE_MCLAG)
  .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
@@ -3420,10 +3504,21 @@ void AclOrch::putAclActionCapabilityInDB(acl_stage_type_t stage)
  }
  }
 
+
  is_action_list_mandatory_stream << boolalpha << capabilities.isActionListMandatoryOnTableCreation;
 
  fvVector.emplace_back(STATE_DB_ACL_ACTION_FIELD_IS_ACTION_LIST_MANDATORY, is_action_list_mandatory_stream.str());
  fvVector.emplace_back(STATE_DB_ACL_ACTION_FIELD_ACTION_LIST, acl_action_value_stream.str());
+
+ for (auto const& it : m_L3V4V6Capability)
+ {
+ string value = it.second ? "true" : "false";
+ if (it.first == stage)
+ {
+ fvVector.emplace_back(STATE_DB_ACL_L3V4V6_SUPPORTED, value);
+ }
+ }
+
  m_aclStageCapabilityTable.set(stage_str, fvVector);
 }
 
@@ -4231,6 +4326,16 @@ bool AclOrch::isAclActionListMandatoryOnTableCreation(acl_stage_type_t stage) co
  return it->second.isActionListMandatoryOnTableCreation;
 }
 
+bool AclOrch::isAclL3V4V6TableSupported(acl_stage_type_t stage) const
+{
+ const auto& it = m_L3V4V6Capability.find(stage);
+ if (it == m_L3V4V6Capability.cend())
+ {
+ return false;
+ }
+ return it->second;
+}
+
 bool AclOrch::isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const
 {
  const auto& it = m_aclCapabilities.find(stage);
@@ -4484,6 +4589,8 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
  }
  bool bHasTCPFlag = false;
  bool bHasIPProtocol = false;
+ bool bHasIPV4 = false;
+ bool bHasIPV6 = false;
  for (const auto& itr : kfvFieldsValues(t))
  {
  string attr_name = to_upper(fvField(itr));
@@ -4494,6 +4601,14 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
  {
  bHasTCPFlag = true;
  }
+ if (attr_name == MATCH_SRC_IP || attr_name == MATCH_DST_IP)
+ {
+ bHasIPV4 = true;
+ }
+ if (attr_name == MATCH_SRC_IPV6 || attr_name == MATCH_DST_IPV6)
+ {
+ bHasIPV6 = true;
+ }
  if (attr_name == MATCH_IP_PROTOCOL || attr_name == MATCH_NEXT_HEADER)
  {
  bHasIPProtocol = true;
@@ -4542,6 +4657,15 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
  }
  }
 
+ if (bHasIPV4 && bHasIPV6)
+ {
+ if (type == TABLE_TYPE_L3V4V6)
+ {
+ SWSS_LOG_ERROR("Rule '%s' is invalid since it has both v4 and v6 matchfields.", rule_id.c_str());
+ bAllAttributesOk = false;
+ }
+ }
+
  // validate and create ACL rule
  if (bAllAttributesOk && newRule->validate())
  {

orchagent/aclorch.h

@@ -502,12 +502,14 @@ class AclOrch : public Orch, public Observer
  bool isAclMirrorV6Supported() const;
  bool isAclMirrorV4Supported() const;
  bool isAclMirrorTableSupported(string type) const;
+ bool isAclL3V4V6TableSupported(acl_stage_type_t stage) const;
  bool isAclActionListMandatoryOnTableCreation(acl_stage_type_t stage) const;
  bool isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const;
  bool isAclActionEnumValueSupported(sai_acl_action_type_t action, sai_acl_action_parameter_t param) const;
 
  bool m_isCombinedMirrorV6Table = true;
  map<string, bool> m_mirrorTableCapabilities;
+ map<acl_stage_type_t, bool> m_L3V4V6Capability;
 
  void registerFlexCounter(const AclRule& rule);
  void deregisterFlexCounter(const AclRule& rule);

orchagent/acltable.h

@@ -25,6 +25,7 @@ extern "C" {
 
 #define TABLE_TYPE_L3 "L3"
 #define TABLE_TYPE_L3V6 "L3V6"
+#define TABLE_TYPE_L3V4V6 "L3V4V6"
 #define TABLE_TYPE_MIRROR "MIRROR"
 #define TABLE_TYPE_MIRRORV6 "MIRRORV6"
 #define TABLE_TYPE_MIRROR_DSCP "MIRROR_DSCP"

tests/test_acl_l3v4v6.py

@@ -0,0 +1,99 @@
+import pytest
+from requests import request
+
+L3V4V6_TABLE_TYPE = "L3V4V6"
+L3V4V6_TABLE_NAME = "L3_V4V6_TEST"
+L3V4V6_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8"]
+L3V4V6_RULE_NAME = "L3V4V6_TEST_RULE"
+
+class TestAcl:
+ @pytest.fixture
+ def l3v4v6_acl_table(self, dvs_acl):
+ try:
+ dvs_acl.create_acl_table(L3V4V6_TABLE_NAME,
+ L3V4V6_TABLE_TYPE,
+ L3V4V6_BIND_PORTS)
+ yield dvs_acl.get_acl_table_ids(1)[0]
+ finally:
+ dvs_acl.remove_acl_table(L3V4V6_TABLE_NAME)
+ dvs_acl.verify_acl_table_count(0)
+
+ @pytest.fixture
+ def setup_teardown_neighbor(self, dvs):
+ try:
+ # NOTE: set_interface_status has a dependency on cdb within dvs,
+ # so we still need to setup the db. This should be refactored.
+ dvs.setup_db()
+
+ # Bring up an IP interface with a neighbor
+ dvs.set_interface_status("Ethernet4", "up")
+ dvs.add_ip_address("Ethernet4", "10.0.0.1/24")
+ dvs.add_neighbor("Ethernet4", "10.0.0.2", "00:01:02:03:04:05")
+
+ yield dvs.get_asic_db().wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", 1)[0]
+ finally:
+ # Clean up the IP interface and neighbor
+ dvs.remove_neighbor("Ethernet4", "10.0.0.2")
+ dvs.remove_ip_address("Ethernet4", "10.0.0.1/24")
+ dvs.set_interface_status("Ethernet4", "down")
+
+ def test_L3V4V6AclTableCreationDeletion(self, dvs_acl):
+ try:
+ dvs_acl.create_acl_table(L3V4V6_TABLE_NAME, L3V4V6_TABLE_TYPE, L3V4V6_BIND_PORTS)
+
+ acl_table_id = dvs_acl.get_acl_table_ids(1)[0]
+ acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(L3V4V6_BIND_PORTS))
+
+ dvs_acl.verify_acl_table_group_members(acl_table_id, acl_table_group_ids, 1)
+ dvs_acl.verify_acl_table_port_binding(acl_table_id, L3V4V6_BIND_PORTS, 1)
+ # Verify status is written into STATE_DB
+ dvs_acl.verify_acl_table_status(L3V4V6_TABLE_NAME, "Active")
+ finally:
+ dvs_acl.remove_acl_table(L3V4V6_TABLE_NAME)
+ dvs_acl.verify_acl_table_count(0)
+ # Verify the STATE_DB entry is removed
+ dvs_acl.verify_acl_table_status(L3V4V6_TABLE_NAME, None)
+
+ def test_ValidAclRuleCreation_sip_dip(self, dvs_acl, l3v4v6_acl_table):
+ config_qualifiers = {"DST_IP": "20.0.0.1/32",
+ "SRC_IP": "10.0.0.0/32"};
+
+ dvs_acl.create_acl_rule(L3V4V6_TABLE_NAME, "VALID_RULE", config_qualifiers)
+ # Verify status is written into STATE_DB
+ dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "VALID_RULE", "Active")
+
+ dvs_acl.remove_acl_rule(L3V4V6_TABLE_NAME, "VALID_RULE")
+ # Verify the STATE_DB entry is removed
+ dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "VALID_RULE", None)
+ dvs_acl.verify_no_acl_rules()
+
+ def test_InvalidAclRuleCreation_sip_sipv6(self, dvs_acl, l3v4v6_acl_table):
+ config_qualifiers = {"SRC_IPV6": "2777::0/64",
+ "SRC_IP": "10.0.0.0/32"};
+
+ dvs_acl.create_acl_rule(L3V4V6_TABLE_NAME, "INVALID_RULE", config_qualifiers)
+ # Verify status is written into STATE_DB
+ dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "INVALID_RULE", "Inactive")
+
+ dvs_acl.remove_acl_rule(L3V4V6_TABLE_NAME, "INVALID_RULE")
+ # Verify the STATE_DB entry is removed
+ dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "INVALID_RULE", None)
+ dvs_acl.verify_no_acl_rules()
+
+ def test_InvalidAclRuleCreation_dip_sipv6(self, dvs_acl, l3v4v6_acl_table):
+ config_qualifiers = {"SRC_IPV6": "2777::0/64",
+ "DST_IP": "10.0.0.0/32"};
+
+ dvs_acl.create_acl_rule(L3V4V6_TABLE_NAME, "INVALID_RULE", config_qualifiers)
+ # Verify status is written into STATE_DB
+ dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "INVALID_RULE", "Inactive")
+
+ dvs_acl.remove_acl_rule(L3V4V6_TABLE_NAME, "INVALID_RULE")
+ # Verify the STATE_DB entry is removed
+ dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "INVALID_RULE", None)
+ dvs_acl.verify_no_acl_rules()
+
+# Add Dummy always-pass test at end as workaroud
+# for issue when Flaky fail on final test it invokes module tear-down before retrying
+def test_nonflaky_dummy():
+ pass