[SAIP4] Remove yellow actions and add annotations specifying meter ty…

…pe to direct meters, Add multicast reference annotations to p4 programs, Increase the guaranteed size of QoS and mirror & redirect ACL tables, Add match on ipv6_dst in mirror_and_redirect to middleblock as well, Add entry restrictions of mirror_and_redirect to middleblock as well & Add dst_ipv4 match to mirror_and_redirect. (#624)

Co-authored-by: kishanps <[email protected]>

sai_p4/fixed/packet_rewrites.p4

@@ -64,12 +64,10 @@ control multicast_rewrites(inout local_metadata_t local_metadata,
   table multicast_router_interface_table {
     key = {
       multicast_replica_port : exact
-        // TODO: Add this once supported by PDPI and its customers.
-        // @referenced_by(multicast_group_table, replica.port)
+        @referenced_by(builtin::multicast_group_table, replica.port)
         @id(1);
       multicast_replica_instance : exact
-        // TODO: Add this once supported by PDPI and its customers.
-        // @referenced_by(multicast_group_table, replica.instance)
+        @referenced_by(builtin::multicast_group_table, replica.instance)
         @id(2);
     }
     actions = {

sai_p4/fixed/routing.p4

@@ -175,8 +175,7 @@ control routing_lookup(in headers_t headers,
   ")
   action set_multicast_group_id(
       @id(1)
-      // TODO: Add this once supported by PDPI and its customers.
-      // @refers_to(multicast_group_table, multicast_group_id)
+      @refers_to(builtin::multicast_group_table, multicast_group_id)
       multicast_group_id_t multicast_group_id) {
     standard_metadata.mcast_grp = multicast_group_id;
   }

sai_p4/instantiations/google/acl_ingress.p4

@@ -24,9 +24,11 @@ control acl_ingress(in headers_t headers,
   bool cancel_copy = false;
 
   @id(ACL_INGRESS_METER_ID)
+  @mode(single_rate_two_color)
   direct_meter<MeterColor_t>(MeterType.bytes) acl_ingress_meter;
 
   @id(ACL_INGRESS_QOS_METER_ID)
+  @mode(single_rate_two_color)
   direct_meter<MeterColor_t>(MeterType.bytes) acl_ingress_qos_meter;
 
   @id(ACL_INGRESS_COUNTER_ID)
@@ -53,7 +55,6 @@ control acl_ingress(in headers_t headers,
   }
 #else
   @sai_action(SAI_PACKET_ACTION_COPY, SAI_PACKET_COLOR_GREEN)
-  @sai_action(SAI_PACKET_ACTION_FORWARD, SAI_PACKET_COLOR_YELLOW)
   @sai_action(SAI_PACKET_ACTION_FORWARD, SAI_PACKET_COLOR_RED)
   action acl_copy(@sai_action_param(QOS_QUEUE) @id(1) qos_queue_t qos_queue) {
     acl_ingress_counter.count();
@@ -73,7 +74,6 @@ control acl_ingress(in headers_t headers,
   @sai_action(SAI_PACKET_ACTION_TRAP)
 #else
   @sai_action(SAI_PACKET_ACTION_TRAP, SAI_PACKET_COLOR_GREEN)
-  @sai_action(SAI_PACKET_ACTION_DROP, SAI_PACKET_COLOR_YELLOW)
   @sai_action(SAI_PACKET_ACTION_DROP, SAI_PACKET_COLOR_RED)
 #endif
   action acl_trap(@sai_action_param(QOS_QUEUE) @id(1) qos_queue_t qos_queue) {
@@ -94,7 +94,6 @@ control acl_ingress(in headers_t headers,
   }
 #else
   @sai_action(SAI_PACKET_ACTION_FORWARD, SAI_PACKET_COLOR_GREEN)
-  @sai_action(SAI_PACKET_ACTION_DROP, SAI_PACKET_COLOR_YELLOW)
   @sai_action(SAI_PACKET_ACTION_DROP, SAI_PACKET_COLOR_RED)
   action acl_forward() {
     acl_ingress_meter.read(local_metadata.color);
@@ -124,7 +123,6 @@ control acl_ingress(in headers_t headers,
 
   @id(ACL_INGRESS_SET_QOS_QUEUE_AND_CANCEL_COPY_ABOVE_RATE_LIMIT_ACTION_ID)
   @sai_action(SAI_PACKET_ACTION_FORWARD, SAI_PACKET_COLOR_GREEN)
-  @sai_action(SAI_PACKET_ACTION_COPY_CANCEL, SAI_PACKET_COLOR_YELLOW)
   @sai_action(SAI_PACKET_ACTION_COPY_CANCEL, SAI_PACKET_COLOR_RED)
   // TODO: Rename qos queue to cpu queue, as per action below.
   action set_qos_queue_and_cancel_copy_above_rate_limit(
@@ -143,17 +141,15 @@ control acl_ingress(in headers_t headers,
   // set depending on packet color.
   @id(ACL_INGRESS_SET_CPU_AND_MULTICAST_QUEUES_AND_DENY_ABOVE_RATE_LIMIT_ACTION_ID)
   @sai_action(SAI_PACKET_ACTION_FORWARD, SAI_PACKET_COLOR_GREEN)
-  @sai_action(SAI_PACKET_ACTION_DENY, SAI_PACKET_COLOR_YELLOW)
   @sai_action(SAI_PACKET_ACTION_DENY, SAI_PACKET_COLOR_RED)
   // TODO: Remove @unsupported annotation.
   @unsupported
   action set_cpu_and_multicast_queues_and_deny_above_rate_limit(
       @id(1) @sai_action_param(QOS_QUEUE) qos_queue_t cpu_queue,
       @id(2) @sai_action_param(MULTICAST_QOS_QUEUE, SAI_PACKET_COLOR_GREEN)
         qos_queue_t green_multicast_queue,
-      @id(3) @sai_action_param(MULTICAST_QOS_QUEUE, SAI_PACKET_COLOR_YELLOW)
-      @sai_action_param(MULTICAST_QOS_QUEUE, SAI_PACKET_COLOR_RED)
-        qos_queue_t red_and_yellow_multicast_queue) {
+      @id(3) @sai_action_param(MULTICAST_QOS_QUEUE, SAI_PACKET_COLOR_RED)
+        qos_queue_t red_multicast_queue) {
     acl_ingress_qos_meter.read(local_metadata.color);
     // We model the behavior for GREEN packes only here.
     // TODO: Branch on color and model behavior for all colors.
@@ -163,7 +159,6 @@ control acl_ingress(in headers_t headers,
   // they are not copied to the CPU.
   @id(ACL_INGRESS_SET_CPU_QUEUE_AND_DENY_ABOVE_RATE_LIMIT_ACTION_ID)
   @sai_action(SAI_PACKET_ACTION_FORWARD, SAI_PACKET_COLOR_GREEN)
-  @sai_action(SAI_PACKET_ACTION_DENY, SAI_PACKET_COLOR_YELLOW)
   @sai_action(SAI_PACKET_ACTION_DENY, SAI_PACKET_COLOR_RED)
   action set_cpu_queue_and_deny_above_rate_limit(
       @id(1) @sai_action_param(QOS_QUEUE) qos_queue_t cpu_queue) {
@@ -391,7 +386,7 @@ control acl_ingress(in headers_t headers,
     const default_action = NoAction;
     meters = acl_ingress_qos_meter;
     counters = acl_ingress_qos_counter;
-    size = ACL_INGRESS_TABLE_MINIMUM_GUARANTEED_SIZE;
+    size = ACL_INGRESS_QOS_TABLE_MINIMUM_GUARANTEED_SIZE;
   }
 
   @p4runtime_role(P4RUNTIME_ROLE_SDN_CONTROLLER)
@@ -471,10 +466,10 @@ control acl_ingress(in headers_t headers,
   @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;
+    dst_ip::mask != 0 -> is_ipv4 == 1;
+    dst_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);
@@ -483,7 +478,6 @@ 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)
@@ -492,6 +486,17 @@ control acl_ingress(in headers_t headers,
         @sai_field(SAI_ACL_TABLE_ATTR_FIELD_IN_PORT)
         @id(1);
 
+      local_metadata.acl_metadata : ternary
+        @name("acl_metadata")
+        @sai_field(SAI_ACL_TABLE_ATTR_FIELD_ACL_USER_META)
+        @id(6);
+
+      local_metadata.vlan_id : ternary
+        @name("vlan_id")
+        @sai_field(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)
+        @id(7);
+#endif
+
       headers.ipv4.isValid() || headers.ipv6.isValid() : optional
         @name("is_ip")
         @sai_field(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE/IP)
@@ -507,6 +512,12 @@ control acl_ingress(in headers_t headers,
         @sai_field(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE/IPV6ANY)
         @id(4);
 
+      headers.ipv4.dst_addr : ternary
+        @name("dst_ip")
+        @sai_field(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)
+        @format(IPV4_ADDRESS)
+        @id(10);
+
       headers.ipv6.dst_addr[127:64] : ternary
         @name("dst_ipv6")
         @composite_field(
@@ -515,22 +526,6 @@ control acl_ingress(in headers_t headers,
         @format(IPV6_ADDRESS)
         @id(5);
 
-      local_metadata.acl_metadata : ternary
-        @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 without `@unsupported`
-      // annotation is required.
-      // TODO: Make this field TOR-only once the
-      // middleblock  match fields are no longer `@unsupported`.
-      local_metadata.vlan_id : ternary
-        @name("vlan_id")
-        @sai_field(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)
-        @id(7);
-
       local_metadata.vrf_id : optional
         @name("vrf_id")
         @sai_field(SAI_ACL_TABLE_ATTR_FIELD_VRF_ID)

sai_p4/instantiations/google/fabric_border_router.p4info.pb.txt

@@ -857,6 +857,7 @@ tables {
   match_fields {
     id: 1
     name: "multicast_replica_port"
+    annotations: "@referenced_by(builtin : : multicast_group_table , replica . port)"
     match_type: EXACT
     type_name {
       name: "port_id_t"
@@ -865,6 +866,7 @@ tables {
   match_fields {
     id: 2
     name: "multicast_replica_instance"
+    annotations: "@referenced_by(builtin : : multicast_group_table , replica . instance)"
     bitwidth: 16
     match_type: EXACT
   }
@@ -1127,6 +1129,7 @@ actions {
   params {
     id: 1
     name: "multicast_group_id"
+    annotations: "@refers_to(builtin : : multicast_group_table , multicast_group_id)"
     bitwidth: 16
   }
 }
@@ -1136,7 +1139,6 @@ actions {
     name: "ingress.acl_ingress.acl_copy"
     alias: "acl_copy"
     annotations: "@sai_action(SAI_PACKET_ACTION_COPY , SAI_PACKET_COLOR_GREEN)"
-    annotations: "@sai_action(SAI_PACKET_ACTION_FORWARD , SAI_PACKET_COLOR_YELLOW)"
     annotations: "@sai_action(SAI_PACKET_ACTION_FORWARD , SAI_PACKET_COLOR_RED)"
   }
   params {
@@ -1154,7 +1156,6 @@ actions {
     name: "ingress.acl_ingress.acl_trap"
     alias: "acl_trap"
     annotations: "@sai_action(SAI_PACKET_ACTION_TRAP , SAI_PACKET_COLOR_GREEN)"
-    annotations: "@sai_action(SAI_PACKET_ACTION_DROP , SAI_PACKET_COLOR_YELLOW)"
     annotations: "@sai_action(SAI_PACKET_ACTION_DROP , SAI_PACKET_COLOR_RED)"
   }
   params {
@@ -1172,7 +1173,6 @@ actions {
     name: "ingress.acl_ingress.acl_forward"
     alias: "acl_forward"
     annotations: "@sai_action(SAI_PACKET_ACTION_FORWARD , SAI_PACKET_COLOR_GREEN)"
-    annotations: "@sai_action(SAI_PACKET_ACTION_DROP , SAI_PACKET_COLOR_YELLOW)"
     annotations: "@sai_action(SAI_PACKET_ACTION_DROP , SAI_PACKET_COLOR_RED)"
   }
 }
@@ -1519,6 +1519,7 @@ direct_meters {
     id: 352321792
     name: "ingress.acl_ingress.acl_ingress_meter"
     alias: "acl_ingress_meter"
+    annotations: "@mode(single_rate_two_color)"
   }
   spec {
     unit: BYTES

sai_p4/instantiations/google/middleblock.p4info.pb.txt

@@ -553,12 +553,43 @@ tables {
     alias: "acl_ingress_mirror_and_redirect_table"
     annotations: "@sai_acl(INGRESS)"
     annotations: "@p4runtime_role(\"sdn_controller\")"
+    annotations: "@entry_restriction(\"\n    // Only allow IP field matches for IP packets.\n    dst_ip::mask != 0 -> is_ipv4 == 1;\n    dst_ipv6::mask != 0 -> 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: 7
-    name: "vlan_id"
-    annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)"
-    bitwidth: 12
+    id: 2
+    name: "is_ip"
+    annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE / IP)"
+    bitwidth: 1
+    match_type: OPTIONAL
+  }
+  match_fields {
+    id: 3
+    name: "is_ipv4"
+    annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE / IPV4ANY)"
+    bitwidth: 1
+    match_type: OPTIONAL
+  }
+  match_fields {
+    id: 4
+    name: "is_ipv6"
+    annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE / IPV6ANY)"
+    bitwidth: 1
+    match_type: OPTIONAL
+  }
+  match_fields {
+    id: 10
+    name: "dst_ip"
+    annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)"
+    annotations: "@format(IPV4_ADDRESS)"
+    bitwidth: 32
+    match_type: TERNARY
+  }
+  match_fields {
+    id: 5
+    name: "dst_ipv6"
+    annotations: "@composite_field(@ sai_field ( SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6_WORD3 ) , @ sai_field ( SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6_WORD2 ))"
+    annotations: "@format(IPV6_ADDRESS)"
+    bitwidth: 64
     match_type: TERNARY
   }
   match_fields {
@@ -920,6 +951,7 @@ tables {
   match_fields {
     id: 1
     name: "multicast_replica_port"
+    annotations: "@referenced_by(builtin : : multicast_group_table , replica . port)"
     match_type: EXACT
     type_name {
       name: "port_id_t"
@@ -928,6 +960,7 @@ tables {
   match_fields {
     id: 2
     name: "multicast_replica_instance"
+    annotations: "@referenced_by(builtin : : multicast_group_table , replica . instance)"
     bitwidth: 16
     match_type: EXACT
   }
@@ -1110,6 +1143,7 @@ actions {
   params {
     id: 1
     name: "multicast_group_id"
+    annotations: "@refers_to(builtin : : multicast_group_table , multicast_group_id)"
     bitwidth: 16
   }
 }
@@ -1119,7 +1153,6 @@ actions {
     name: "ingress.acl_ingress.acl_copy"
     alias: "acl_copy"
     annotations: "@sai_action(SAI_PACKET_ACTION_COPY , SAI_PACKET_COLOR_GREEN)"
-    annotations: "@sai_action(SAI_PACKET_ACTION_FORWARD , SAI_PACKET_COLOR_YELLOW)"
     annotations: "@sai_action(SAI_PACKET_ACTION_FORWARD , SAI_PACKET_COLOR_RED)"
   }
   params {
@@ -1137,7 +1170,6 @@ actions {
     name: "ingress.acl_ingress.acl_trap"
     alias: "acl_trap"
     annotations: "@sai_action(SAI_PACKET_ACTION_TRAP , SAI_PACKET_COLOR_GREEN)"
-    annotations: "@sai_action(SAI_PACKET_ACTION_DROP , SAI_PACKET_COLOR_YELLOW)"
     annotations: "@sai_action(SAI_PACKET_ACTION_DROP , SAI_PACKET_COLOR_RED)"
   }
   params {
@@ -1155,7 +1187,6 @@ actions {
     name: "ingress.acl_ingress.acl_forward"
     alias: "acl_forward"
     annotations: "@sai_action(SAI_PACKET_ACTION_FORWARD , SAI_PACKET_COLOR_GREEN)"
-    annotations: "@sai_action(SAI_PACKET_ACTION_DROP , SAI_PACKET_COLOR_YELLOW)"
     annotations: "@sai_action(SAI_PACKET_ACTION_DROP , SAI_PACKET_COLOR_RED)"
   }
 }
@@ -1522,6 +1553,7 @@ direct_meters {
     id: 352321792
     name: "ingress.acl_ingress.acl_ingress_meter"
     alias: "acl_ingress_meter"
+    annotations: "@mode(single_rate_two_color)"
   }
   spec {
     unit: BYTES

sai_p4/instantiations/google/minimum_guaranteed_sizes.p4

@@ -89,9 +89,15 @@
 #define ACL_TOR_PRE_INGRESS_TABLE_MINIMUM_GUARANTEED_SIZE 127
 
 
+#define ACL_INGRESS_QOS_TABLE_MINIMUM_GUARANTEED_SIZE 511
+
 #define ACL_INGRESS_COUNTING_TABLE_MINIMUM_GUARANTEED_SIZE 255
 
+#ifdef SAI_INSTANTIATION_EXPERIMENTAL_TOR
+#define ACL_INGRESS_MIRROR_AND_REDIRECT_TABLE_MINIMUM_GUARANTEED_SIZE 511
+#else
 #define ACL_INGRESS_MIRROR_AND_REDIRECT_TABLE_MINIMUM_GUARANTEED_SIZE 255
+#endif
 
 #define ACL_EGRESS_TABLE_MINIMUM_GUARANTEED_SIZE 127
 

sai_p4/instantiations/google/sai_pd.proto

@@ -724,7 +724,8 @@ message AclIngressSecurityTableEntry {
 
 // Table entry restrictions:
 // ## Only allow IP field matches for IP packets.
-//   is_ipv6::mask != 0 -> is_ipv6 == 1;
+//   dst_ip::mask != 0 -> is_ipv4 == 1;
+//   dst_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,7 +746,8 @@ message AclIngressMirrorAndRedirectTableEntry {
     Optional vrf_id = 8;  // optional match / Format::STRING
     // CAUTION: This match field is not (yet) supported.
     Optional ipmc_table_hit =
-        9;  // optional match / Format::HEX_STRING / 1 bits
+        9;                // optional match / Format::HEX_STRING / 1 bits
+    Ternary dst_ip = 10;  // ternary match / Format::IPV4
   }
   Match match = 1;
   message Action {
@@ -948,9 +950,9 @@ message SetCpuQueueAction {
 
 // CAUTION: This action is not (yet) supported.
 message SetCpuAndMulticastQueuesAndDenyAboveRateLimitAction {
-  string cpu_queue = 1;                       // Format::STRING
-  string green_multicast_queue = 2;           // Format::STRING
-  string red_and_yellow_multicast_queue = 3;  // Format::STRING
+  string cpu_queue = 1;              // Format::STRING
+  string green_multicast_queue = 2;  // Format::STRING
+  string red_multicast_queue = 3;    // Format::STRING
 }
 
 message RedirectToNexthopAction {