[SAIP4] Add redirect to multicast action to ingress ACL & Add "mirror…

… and redirect" ACL ingress table to middleblock. (sonic-net#615) Co-authored-by: smolkaj <[email protected]> Co-authored-by: kheradmandG <[email protected]> Co-authored-by: kishanps <[email protected]>
divyagayathri-hcl · Oct 15, 2024 · 38d80b7 · 38d80b7
1 parent 78dbb2c
commit 38d80b7
Show file tree

Hide file tree

Showing 13 changed files with 537 additions and 60 deletions.
diff --git a/sai_p4/fixed/packet_rewrites.p4 b/sai_p4/fixed/packet_rewrites.p4
@@ -90,50 +90,36 @@ control packet_rewrites(inout headers_t headers,
                         inout standard_metadata_t standard_metadata) {
   apply {
     if (standard_metadata.instance_type == PKT_INSTANCE_TYPE_REPLICATION) {
-      // TODO: Remove guard once p4-symbolic suports assertions.
-#ifndef PLATFORM_P4SYMBOLIC
-      assert(local_metadata.admit_to_l3);
-#endif
       local_metadata.enable_decrement_ttl = true;
       multicast_rewrites.apply(local_metadata, standard_metadata);
     }
-
-    if (local_metadata.admit_to_l3){
+    if (local_metadata.enable_src_mac_rewrite) {
+      headers.ethernet.src_addr = local_metadata.packet_rewrites.src_mac;
+    }
+    if (local_metadata.enable_dst_mac_rewrite) {
+      headers.ethernet.dst_addr = local_metadata.packet_rewrites.dst_mac;
+    }
+    if (local_metadata.enable_vlan_rewrite) {
       // VLAN id is kept in local_metadata until the end of egress pipeline
       // where depending on the value of VLAN id and VLAN configuration the
       // packet might potentially get VLAN tagged with that VLAN id.
-      // TODO: For now rewriting unconditionaly since disabling
-      // VLAN rewrite is not modeled yet. When modeled, this rewrite should
-      // become conditional too.
-//      local_metadata.vlan_id = local_metadata.packet_rewrites.vlan_id;
-      if (local_metadata.enable_src_mac_rewrite) {
-        headers.ethernet.src_addr = local_metadata.packet_rewrites.src_mac;
-      }
-      if (local_metadata.enable_dst_mac_rewrite) {
-        headers.ethernet.dst_addr = local_metadata.packet_rewrites.dst_mac;
-      }
-      if (local_metadata.enable_vlan_rewrite) {
-        // VLAN id is kept in local_metadata until the end of egress pipeline
-        // where depending on the value of VLAN id and VLAN configuration the
-        // packet might potentially get VLAN tagged with that VLAN id.
-        local_metadata.vlan_id = local_metadata.packet_rewrites.vlan_id;
-      }
-      if (headers.ipv4.isValid()) {
-        if (headers.ipv4.ttl > 0 && local_metadata.enable_decrement_ttl) {
-          headers.ipv4.ttl = headers.ipv4.ttl - 1;
-        }
-        // TODO: Verify this is accurate when TTL rewrite is
-        // disabled and update this code if not.
-        if (headers.ipv4.ttl == 0) mark_to_drop(standard_metadata);
+      local_metadata.vlan_id = local_metadata.packet_rewrites.vlan_id;
+    }
+    if (headers.ipv4.isValid()) {
+      if (headers.ipv4.ttl > 0 && local_metadata.enable_decrement_ttl) {
+        headers.ipv4.ttl = headers.ipv4.ttl - 1;
       }
-      if (headers.ipv6.isValid()) {
-        if (headers.ipv6.hop_limit > 0 && local_metadata.enable_decrement_ttl) {
-          headers.ipv6.hop_limit = headers.ipv6.hop_limit - 1;
-        }
-        // TODO: Verify this is accurate when TTL rewrite is
-        // disabled and update this code if not.
-        if (headers.ipv6.hop_limit == 0) mark_to_drop(standard_metadata);
+      // TODO: Verify this is accurate when TTL rewrite is
+      // disabled and update this code if not.
+      if (headers.ipv4.ttl == 0) mark_to_drop(standard_metadata);
+    }
+    if (headers.ipv6.isValid()) {
+      if (headers.ipv6.hop_limit > 0 && local_metadata.enable_decrement_ttl) {
+        headers.ipv6.hop_limit = headers.ipv6.hop_limit - 1;
       }
+      // TODO: Verify this is accurate when TTL rewrite is
+      // disabled and update this code if not.
+      if (headers.ipv6.hop_limit == 0) mark_to_drop(standard_metadata);
     }
   }
 }  // control packet_rewrites

diff --git a/sai_p4/fixed/routing.p4 b/sai_p4/fixed/routing.p4
@@ -171,9 +171,8 @@ control routing_lookup(in headers_t headers,
   @id(ROUTING_SET_MULTICAST_GROUP_ID_ACTION_ID)
   @action_restriction("
     // Disallow 0 since it encodes 'no multicast' in V1Model.
-    multicast_group_id != 0"
-  )
-
+    multicast_group_id != 0;
+  ")
   action set_multicast_group_id(
       @id(1)
       // TODO: Add this once supported by PDPI and its customers.

diff --git a/sai_p4/instantiations/google/acl_ingress.p4 b/sai_p4/instantiations/google/acl_ingress.p4
@@ -398,6 +398,7 @@ control acl_ingress(in headers_t headers,
   @id(ACL_INGRESS_COUNTING_TABLE_ID)
   @sai_acl(INGRESS)
   @entry_restriction("
+    // Only allow IP field matches for IP packets.
     dscp::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);
     // Forbid illegal combinations of IP_TYPE fields.
     is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);
@@ -433,6 +434,7 @@ control acl_ingress(in headers_t headers,
   @id(ACL_INGRESS_REDIRECT_TO_NEXTHOP_ACTION_ID)
   action redirect_to_nexthop(
     @sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)
+    @refers_to(nexthop_table, nexthop_id)
     nexthop_id_t nexthop_id) {
 
     // Set nexthop id.
@@ -444,11 +446,35 @@ control acl_ingress(in headers_t headers,
     standard_metadata.mcast_grp = 0;
   }
 
+  // TODO: Remove `@unsupported` annotation once the switch stack
+  // supports multicast.
+  @unsupported
+  @id(ACL_INGRESS_REDIRECT_TO_IPMC_GROUP_ACTION_ID)
+  @action_restriction("
+    // Disallow 0 since it encodes 'no multicast' in V1Model.
+    multicast_group_id != 0;
+  ")
+  action redirect_to_ipmc_group(
+    @sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)
+    // TODO: Add this once supported by PDPI and its customers.
+    // @refers_to(multicast_group_table, multicast_group_id)
+    multicast_group_id_t multicast_group_id) {
+    standard_metadata.mcast_grp = multicast_group_id;
+
+    // Cancel other forwarding decisions (if any).
+    local_metadata.nexthop_id_valid = false;
+    local_metadata.wcmp_group_id_valid = false;
+  }
+
+
   // ACL table that mirrors and redirects packets.
   @id(ACL_INGRESS_MIRROR_AND_REDIRECT_TABLE_ID)
   @sai_acl(INGRESS)
   @p4runtime_role(P4RUNTIME_ROLE_SDN_CONTROLLER)
+#if defined(SAI_INSTANTIATION_TOR)
   @entry_restriction("
+    // Only allow IP field matches for IP packets.
+    is_ipv6::mask != 0 ->  is_ipv6 == 1;
     // Forbid illegal combinations of IP_TYPE fields.
     is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);
     is_ipv4::mask != 0 -> (is_ip::mask == 0 && is_ipv6::mask == 0);
@@ -457,8 +483,10 @@ control acl_ingress(in headers_t headers,
     is_ipv4::mask != 0 -> (is_ipv4 == 1);
     is_ipv6::mask != 0 -> (is_ipv6 == 1);
   ")
+#endif
   table acl_ingress_mirror_and_redirect_table {
     key = {
+#if defined(SAI_INSTANTIATION_TOR)
       local_metadata.ingress_port : optional
         @name("in_port")
         @sai_field(SAI_ACL_TABLE_ATTR_FIELD_IN_PORT)
@@ -491,7 +519,13 @@ control acl_ingress(in headers_t headers,
         @name("acl_metadata")
         @sai_field(SAI_ACL_TABLE_ATTR_FIELD_ACL_USER_META)
         @id(6);
+#endif
 
+      // This field is technically only needed on ToR and only included
+      // for middleblock because at least 1 match field is required and the
+      // other middleblock match fields are TBD.
+      // TODO: Make this field TOR-only when we add
+      // middleblock  match fields.
       local_metadata.vlan_id : ternary
         @name("vlan_id")
         @sai_field(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)
@@ -502,6 +536,7 @@ control acl_ingress(in headers_t headers,
     actions = {
       @proto_id(1) acl_mirror();
       @proto_id(2) redirect_to_nexthop();
+      @proto_id(3) redirect_to_ipmc_group();
       @defaultonly NoAction;
     }
     const default_action = NoAction;
@@ -613,6 +648,7 @@ control acl_ingress(in headers_t headers,
 
 #if defined(SAI_INSTANTIATION_MIDDLEBLOCK)
     acl_ingress_table.apply();
+    acl_ingress_mirror_and_redirect_table.apply();
     acl_ingress_security_table.apply();
 #elif defined(SAI_INSTANTIATION_FABRIC_BORDER_ROUTER)
     acl_ingress_table.apply();
@@ -623,6 +659,7 @@ control acl_ingress(in headers_t headers,
     // additional parts of SAI in the future.
     acl_ingress_table.apply();
     acl_ingress_qos_table.apply();
+    acl_ingress_mirror_and_redirect_table.apply();
 #endif
 
     if (cancel_copy) {

diff --git a/sai_p4/instantiations/google/fabric_border_router.p4info.pb.txt b/sai_p4/instantiations/google/fabric_border_router.p4info.pb.txt
@@ -568,7 +568,7 @@ tables {
     alias: "acl_ingress_counting_table"
     annotations: "@p4runtime_role(\"sdn_controller\")"
     annotations: "@sai_acl(INGRESS)"
-    annotations: "@entry_restriction(\"\n    dscp::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n    // Forbid illegal combinations of IP_TYPE fields.\n    is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);\n    is_ipv4::mask != 0 -> (is_ip::mask == 0 && is_ipv6::mask == 0);\n    is_ipv6::mask != 0 -> (is_ip::mask == 0 && is_ipv4::mask == 0);\n    // Forbid unsupported combinations of IP_TYPE fields.\n    is_ipv4::mask != 0 -> (is_ipv4 == 1);\n    is_ipv6::mask != 0 -> (is_ipv6 == 1);\n  \")"
+    annotations: "@entry_restriction(\"\n    // Only allow IP field matches for IP packets.\n    dscp::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n    // Forbid illegal combinations of IP_TYPE fields.\n    is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);\n    is_ipv4::mask != 0 -> (is_ip::mask == 0 && is_ipv6::mask == 0);\n    is_ipv6::mask != 0 -> (is_ip::mask == 0 && is_ipv4::mask == 0);\n    // Forbid unsupported combinations of IP_TYPE fields.\n    is_ipv4::mask != 0 -> (is_ipv4 == 1);\n    is_ipv6::mask != 0 -> (is_ipv6 == 1);\n  \")"
   }
   match_fields {
     id: 1
@@ -1122,7 +1122,7 @@ actions {
     name: "ingress.routing_lookup.set_multicast_group_id"
     alias: "set_multicast_group_id"
     annotations: "@unsupported"
-    annotations: "@action_restriction(\"\n    // Disallow 0 since it encodes \'no multicast\' in V1Model.\n    multicast_group_id != 0\")"
+    annotations: "@action_restriction(\"\n    // Disallow 0 since it encodes \'no multicast\' in V1Model.\n    multicast_group_id != 0;\n  \")"
   }
   params {
     id: 1

diff --git a/sai_p4/instantiations/google/ids.h b/sai_p4/instantiations/google/ids.h
@@ -50,14 +50,15 @@
   0x0100010E                                            // 16777486
 #define ACL_INGRESS_SET_CPU_QUEUE_ACTION_ID 0x01000110  // 16777488
 #define ACL_INGRESS_SET_CPU_AND_MULTICAST_QUEUES_AND_DENY_ABOVE_RATE_LIMIT_ACTION_ID \
-  0x01000111                                                  // 16777489
-#define ACL_INGRESS_DENY_ACTION_ID 0x0100010F                 // 16777487
-#define ACL_INGRESS_REDIRECT_TO_NEXTHOP_ACTION_ID 0x01000112  // 16777490
-#define ACL_EGRESS_FORWARD_ACTION_ID 0x0100010D               // 16777485
-#define ACL_WBB_INGRESS_COPY_ACTION_ID 0x01000107             // 16777479
-#define ACL_WBB_INGRESS_TRAP_ACTION_ID 0x01000108             // 16777480
-#define ACL_DROP_ACTION_ID 0x01000109                         // 16777481
-// Next available action id: 0x01000113 (16777491)
+  0x01000111                                                     // 16777489
+#define ACL_INGRESS_DENY_ACTION_ID 0x0100010F                    // 16777487
+#define ACL_INGRESS_REDIRECT_TO_NEXTHOP_ACTION_ID 0x01000112     // 16777490
+#define ACL_INGRESS_REDIRECT_TO_IPMC_GROUP_ACTION_ID 0x01000113  // 16777491
+#define ACL_EGRESS_FORWARD_ACTION_ID 0x0100010D                  // 16777485
+#define ACL_WBB_INGRESS_COPY_ACTION_ID 0x01000107                // 16777479
+#define ACL_WBB_INGRESS_TRAP_ACTION_ID 0x01000108                // 16777480
+#define ACL_DROP_ACTION_ID 0x01000109                            // 16777481
+// Next available action id: 0x01000113 (16777492)
 
 
 // NOLINTEND

diff --git a/sai_p4/instantiations/google/middleblock.p4info.pb.txt b/sai_p4/instantiations/google/middleblock.p4info.pb.txt
@@ -546,6 +546,41 @@ tables {
   direct_resource_ids: 352321792
   size: 256
 }
+tables {
+  preamble {
+    id: 33554699
+    name: "ingress.acl_ingress.acl_ingress_mirror_and_redirect_table"
+    alias: "acl_ingress_mirror_and_redirect_table"
+    annotations: "@sai_acl(INGRESS)"
+    annotations: "@p4runtime_role(\"sdn_controller\")"
+  }
+  match_fields {
+    id: 7
+    name: "vlan_id"
+    annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)"
+    bitwidth: 12
+    match_type: TERNARY
+  }
+  action_refs {
+    id: 16777476
+    annotations: "@proto_id(1)"
+  }
+  action_refs {
+    id: 16777490
+    annotations: "@proto_id(2)"
+  }
+  action_refs {
+    id: 16777491
+    annotations: "@proto_id(3)"
+  }
+  action_refs {
+    id: 21257015
+    annotations: "@defaultonly"
+    scope: DEFAULT_ONLY
+  }
+  const_default_action_id: 21257015
+  size: 255
+}
 tables {
   preamble {
     id: 33554698
@@ -1052,7 +1087,7 @@ actions {
     name: "ingress.routing_lookup.set_multicast_group_id"
     alias: "set_multicast_group_id"
     annotations: "@unsupported"
-    annotations: "@action_restriction(\"\n    // Disallow 0 since it encodes \'no multicast\' in V1Model.\n    multicast_group_id != 0\")"
+    annotations: "@action_restriction(\"\n    // Disallow 0 since it encodes \'no multicast\' in V1Model.\n    multicast_group_id != 0;\n  \")"
   }
   params {
     id: 1
@@ -1131,6 +1166,37 @@ actions {
     annotations: "@sai_action(SAI_PACKET_ACTION_DENY)"
   }
 }
+actions {
+  preamble {
+    id: 16777490
+    name: "ingress.acl_ingress.redirect_to_nexthop"
+    alias: "redirect_to_nexthop"
+  }
+  params {
+    id: 1
+    name: "nexthop_id"
+    annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+    annotations: "@refers_to(nexthop_table , nexthop_id)"
+    type_name {
+      name: "nexthop_id_t"
+    }
+  }
+}
+actions {
+  preamble {
+    id: 16777491
+    name: "ingress.acl_ingress.redirect_to_ipmc_group"
+    alias: "redirect_to_ipmc_group"
+    annotations: "@unsupported"
+    annotations: "@action_restriction(\"\n    // Disallow 0 since it encodes \'no multicast\' in V1Model.\n    multicast_group_id != 0;\n  \")"
+  }
+  params {
+    id: 1
+    name: "multicast_group_id"
+    annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+    bitwidth: 16
+  }
+}
 actions {
   preamble {
     id: 16777217

diff --git a/sai_p4/instantiations/google/sai_pd.proto b/sai_p4/instantiations/google/sai_pd.proto
@@ -649,6 +649,7 @@ message AclEgressDhcpToHostTableEntry {
 }
 
 // Table entry restrictions:
+// ## Only allow IP field matches for IP packets.
 //   dscp::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);
 // ## Forbid illegal combinations of IP_TYPE fields.
 //   is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);
@@ -723,6 +724,8 @@ message AclIngressSecurityTableEntry {
 }
 
 // Table entry restrictions:
+// ## Only allow IP field matches for IP packets.
+//   is_ipv6::mask != 0 -> is_ipv6 == 1;
 // ## Forbid illegal combinations of IP_TYPE fields.
 //   is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);
 //   is_ipv4::mask != 0 -> (is_ip::mask == 0 && is_ipv6::mask == 0);
@@ -745,6 +748,8 @@ message AclIngressMirrorAndRedirectTableEntry {
     oneof action {
       AclMirrorAction acl_mirror = 1;
       RedirectToNexthopAction redirect_to_nexthop = 2;
+      // CAUTION: This action is not (yet) supported.
+      RedirectToIpmcGroupAction redirect_to_ipmc_group = 3;
     }
   }
   Action action = 2;
@@ -946,9 +951,15 @@ message SetCpuAndMulticastQueuesAndDenyAboveRateLimitAction {
 }
 
 message RedirectToNexthopAction {
+  // Refers to 'nexthop_table.nexthop_id'.
   string nexthop_id = 1;  // Format::STRING
 }
 
+// CAUTION: This action is not (yet) supported.
+message RedirectToIpmcGroupAction {
+  string multicast_group_id = 1;  // Format::HEX_STRING / 16 bits
+}
+
 message NoAction {}
 
 // This action is unique to `MulticastGroupTableEntry` and is not explicitly

diff --git a/sai_p4/instantiations/google/test_tools/test_entries.cc b/sai_p4/instantiations/google/test_tools/test_entries.cc
@@ -540,6 +540,22 @@ EntryBuilder& EntryBuilder::AddIngressAclEntryRedirectingToNexthop(
   return *this;
 }
 
+EntryBuilder& EntryBuilder::AddIngressAclEntryRedirectingToMulticastGroup(
+    int multicast_group_id, std::optional<absl::string_view> in_port_match) {
+  sai::AclIngressMirrorAndRedirectTableEntry& entry =
+      *entries_.add_entries()
+           ->mutable_acl_ingress_mirror_and_redirect_table_entry();
+  if (in_port_match.has_value()) {
+    entry.mutable_match()->mutable_in_port()->set_value(*in_port_match);
+  }
+  entry.mutable_action()
+      ->mutable_redirect_to_ipmc_group()
+      ->set_multicast_group_id(pdpi::BitsetToHexString<16>(multicast_group_id));
+  entry.set_priority(1);
+
+  return *this;
+}
+
 EntryBuilder& EntryBuilder::AddMirrorSessionTableEntry(
     const MirrorSessionParams& params) {
   sai::TableEntry pd_entry;