use ethertype to decide matching against SAI_ACL_ENTRY_ATTR_FIELD_ICM…

…P_TYPE or SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_TYPE Summary: Currently, ACL icmp type/code match would be ignored if proto is not specified. On J3, proto does not work together with icmp type/code right now, see CS00012373216. Before we got brcm-sai enhancement, use ethertype to decide matching against SAI_ACL_ENTRY_ATTR_FIELD_ICMP_TYPE or SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_TYPE Reviewed By: shri-khare Differential Revision: D64490281 Privacy Context Container: L1125642 fbshipit-source-id: c1e17725c6b7833752009f7f92800677ed3d3a1f
facebook · Oct 17, 2024 · bcc0107 · bcc0107
1 parent c790437
commit bcc0107
Showing 1 changed file with 25 additions and 17 deletions.
diff --git a/fboss/agent/hw/sai/switch/SaiAclTableManager.cpp b/fboss/agent/hw/sai/switch/SaiAclTableManager.cpp
@@ -743,26 +743,34 @@ AclEntrySaiId SaiAclTableManager::addAclEntry(
   std::optional<SaiAclEntryTraits::Attributes::FieldIcmpV6Code> fieldIcmpV6Code{
       std::nullopt};
   if (addedAclEntry->getIcmpType()) {
-    if (addedAclEntry->getProto()) {
-      if (addedAclEntry->getProto().value() == AclEntry::kProtoIcmp) {
-        fieldIcmpV4Type = SaiAclEntryTraits::Attributes::FieldIcmpV4Type{
+    if ((addedAclEntry->getProto() &&
+         addedAclEntry->getProto().value() == AclEntry::kProtoIcmp) ||
+        (addedAclEntry->getEtherType() &&
+         addedAclEntry->getEtherType().value() == cfg::EtherType::IPv4)) {
+      fieldIcmpV4Type = SaiAclEntryTraits::Attributes::FieldIcmpV4Type{
+          AclEntryFieldU8(std::make_pair(
+              addedAclEntry->getIcmpType().value(), kIcmpTypeMask))};
+      if (addedAclEntry->getIcmpCode()) {
+        fieldIcmpV4Code = SaiAclEntryTraits::Attributes::FieldIcmpV4Code{
             AclEntryFieldU8(std::make_pair(
-                addedAclEntry->getIcmpType().value(), kIcmpTypeMask))};
-        if (addedAclEntry->getIcmpCode()) {
-          fieldIcmpV4Code = SaiAclEntryTraits::Attributes::FieldIcmpV4Code{
-              AclEntryFieldU8(std::make_pair(
-                  addedAclEntry->getIcmpCode().value(), kIcmpCodeMask))};
-        }
-      } else if (addedAclEntry->getProto().value() == AclEntry::kProtoIcmpv6) {
-        fieldIcmpV6Type = SaiAclEntryTraits::Attributes::FieldIcmpV6Type{
+                addedAclEntry->getIcmpCode().value(), kIcmpCodeMask))};
+      }
+    } else if (
+        (addedAclEntry->getProto() &&
+         addedAclEntry->getProto().value() == AclEntry::kProtoIcmpv6) ||
+        (addedAclEntry->getEtherType() &&
+         addedAclEntry->getEtherType().value() == cfg::EtherType::IPv6)) {
+      fieldIcmpV6Type = SaiAclEntryTraits::Attributes::FieldIcmpV6Type{
+          AclEntryFieldU8(std::make_pair(
+              addedAclEntry->getIcmpType().value(), kIcmpTypeMask))};
+      if (addedAclEntry->getIcmpCode()) {
+        fieldIcmpV6Code = SaiAclEntryTraits::Attributes::FieldIcmpV6Code{
             AclEntryFieldU8(std::make_pair(
-                addedAclEntry->getIcmpType().value(), kIcmpTypeMask))};
-        if (addedAclEntry->getIcmpCode()) {
-          fieldIcmpV6Code = SaiAclEntryTraits::Attributes::FieldIcmpV6Code{
-              AclEntryFieldU8(std::make_pair(
-                  addedAclEntry->getIcmpCode().value(), kIcmpCodeMask))};
-        }
+                addedAclEntry->getIcmpCode().value(), kIcmpCodeMask))};
       }
+    } else {
+      throw FbossError(
+          "proto or etherType not sepcified in ACL when matching icmp type/code");
     }
   }