Add order support.

dash-pipeline/SAI/sai_api_gen.py

@@ -6,7 +6,7 @@
     import argparse
     import copy
     from jinja2 import Template, Environment, FileSystemLoader
-    from typing import (Any, Dict, List, Optional)
+    from typing import (Type, Any, Dict, List, Optional)
 except ImportError as ie:
     print("Import failed for " + ie.name)
     exit(1)
@@ -37,12 +37,11 @@
 SAI_VAL_TAG: str = 'SaiVal'
 SAI_COUNTER_TAG: str = 'SaiCounter'
 SAI_TABLE_TAG: str = 'SaiTable'
-SAI_API_ORDER_TAG: str = 'api_order'
 
 #
 # SAI parser decorators:
 #
-def sai_parser_from_p4rt(cls: type['SAIObject']):
+def sai_parser_from_p4rt(cls: Type['SAIObject']):
     @staticmethod
     def create(p4rt_value, *args, **kwargs):
         sai_object = cls()
@@ -218,6 +217,7 @@ def __init__(self):
         self.name: str = ''
         self.id: int = 0
         self.alias: str = ''
+        self.order: int = 0
 
     def parse_basic_info_if_exists(self, p4rt_object: Dict[str, Any]) -> None:
         '''
@@ -258,6 +258,9 @@ def _parse_sai_common_annotation(self, p4rt_anno: Dict[str, Any]) -> None:
         if p4rt_anno['key'] == 'name':
             self.name = p4rt_anno['value']['stringValue']
             return True
+        elif p4rt_anno['key'] == 'order':
+            self.order = p4rt_anno['value']['int64Value']
+            return True
 
         return False
 
@@ -322,6 +325,8 @@ def parse_p4rt(self, p4rt_enum: Dict[str, Any]) -> None:
 
 class SAIAPITableAttribute(SAIObject):
     def __init__(self):
+        super().__init__()
+
         # Properties from SAI annotations
         self.type: Optional[str] = None
         self.field: Optional[str] = None
@@ -618,7 +623,6 @@ def __init__(self):
         # Extra properties from annotations
         self.stage: Optional[str] = None
         self.is_object: Optional[str] = None
-        self.api_order: int = 0
         self.api_type: Optional[str] = None
 
     def parse_p4rt(self,
@@ -696,8 +700,6 @@ def __parse_sai_table_annotations(self, p4rt_table_preamble: Dict[str, Any]) ->
                         self.api_name = kv['value']['stringValue']
                     elif kv['key'] == 'api_type':
                         self.api_type = kv['value']['stringValue']
-                    elif kv['key'] == 'api_order':
-                        self.api_order = kv['value']['int64Value']
 
         if self.is_object == None:
             self.is_object = 'false'
@@ -793,12 +795,21 @@ def __update_table_param_object_name_reference(self, all_table_names) -> None:
                             key.object_name = table_name
 
     def __build_sai_attributes_after_parsing(self):
-        # TODO:
-        # We need to build a mechanism to ensure the order of the attributes, because the attributes can come from
-        # multiple actions as well as counters. Adding a new parameter to one action may cause the new parameter
-        # being inserted in the middle of the list and break the ABI compatibility.
-        self.sai_attributes = [action_param for action_param in self.action_params if action_param.skipattr != "true"] \
-            + [counter for counter in self.counters if counter.as_attr]
+        # Group all actions parameters and counters with as_attr set by order with sequence kept the same.
+        # Then merge them into a single list.
+        sai_attributes_by_order = {}
+        for action_param in self.action_params:
+            if action_param.skipattr != "true":
+                sai_attributes_by_order.setdefault(action_param.order, []).append(action_param)
+
+        for counter in self.counters:
+            if counter.as_attr:
+                sai_attributes_by_order.setdefault(counter.order, []).append(counter)
+
+        # Merge all attributes into a single list.
+        self.sai_attributes = []
+        for order in sorted(sai_attributes_by_order.keys()):
+            self.sai_attributes.extend(sai_attributes_by_order[order])
 
 
 class DASHAPISet(SAIObject):
@@ -885,7 +896,7 @@ def __parse_sai_apis_from_p4rt(self, program: Dict[str, Any], ignore_tables: Lis
 
         # Sort all parsed tables by API order, so we can always generate the APIs in the same order for keeping ABI compatibility.
         for sai_api in self.sai_apis:
-            sai_api.tables.sort(key=lambda x: x.api_order)
+            sai_api.tables.sort(key=lambda x: x.order)
 
     def __parse_sai_table_action(self, p4rt_actions: Dict[str, Any], sai_enums: List[SAIEnum], counters_by_action_name: Dict[str, List[SAICounter]]) -> Dict[int, SAIAPITableAction]:
         action_data = {}

dash-pipeline/bmv2/README.md

@@ -30,6 +30,7 @@ Available tags are:
 - `default_value`: Override the default value for this key or action parameter.
 - `isresourcetype`: When set to "true", we generate a corresponding SAI tag in SAI APIs: `@isresourcetype true`.
 - `objects`: Space separated list of SAI object type this value accepts. When set, we force this value to be a SAI object id, and generate a corresponding SAI tag in SAI APIs: `@objects <list>`.
+- `order`: Specify the order of the generated attributes in the SAI API header file. This will be particularly useful, when a table has multiple actions, and we add parameters to one of them. The new attributes might be generated in the middle of existing attributes, which breaks ABI compatibility with older version of SAI APIs.
 - `isreadonly`: When set to "true", we generate force this value to be read-only in SAI API using: `@flags READ_ONLY`, otherwise, we generate `@flags CREATE_AND_SET`.
 - `skipattr`: When set to "true", we skip this attribute in SAI API generation.
 
@@ -42,6 +43,7 @@ Available tags are:
 - `name`: Specify the preferred counter name in SAI API generation, e.g. `outbound_bytes_counter`.
 - `action_names`: The counters are usually updated in actions whenever a table is matched. However, v1model doesn't support conditional statements (if-else) in action blocks. Hence, to workaround, sometimes counters should be updated in the actions are updated in the control blocks after the action is called. This tag is used to specify the name of the actions that was supposed to update this counter. e.g. `action1,action2,...`
 - `as_attr`: When set to "true", the counters will be generated as an attribute of the SAI object. This is not a normal behavior in SAI, since SAI usually either use get stats APIs or directly use counter IDs. Currently, generating get stats APIs is not supported yet, hence when this is not set, the attribute will be ignored.
+- `order`: Specify the order of the generated attributes in the SAI API header file. This will be particularly useful, when a table has multiple actions, and we add parameters to one of them. The new attributes might be generated in the middle of existing attributes, which breaks ABI compatibility with older version of SAI APIs. When `as_attr` is set, it will be compared with the order of other attributes from match keys and action parameters in the same object too.
 
 #### `@SaiTable`: Tables
 
@@ -52,7 +54,7 @@ Available tags are:
 - `name`: Specify the preferred table name in SAI API generation, e.g. `dash_acl_rule`.
 - `api`: Specify which SAI API should be used in generation, e.g. `dash_acl`.
 - `api_type`: The type of the API. DASH contains certain tables for handling underlay actions, such as route table. We should not generate header files for these tables but only the lib files without experimental prefix. To enable this behavior, please set the API type to `underlay`.
-- `api_order`: Specify the order of the generated API in the SAI API header file. When multiple tables generates API entries in the same API set, e.g., acl group and acl rules. This explicit attribute helps us keep the order of the generated APIs to keep ABI compatibility.
+- `order`: Specify the order of the generated API in the SAI API header file. When multiple tables generates API entries in the same API set, e.g., acl group and acl rules. This explicit attribute helps us keep the order of the generated APIs to keep ABI compatibility.
 - `stage`: Specify which stage this table represents for the matching stage type, e.g. `acl.stage1`.
 - `isobject`: When set to "true", a top level objects in SAI that attached to switch will be generated. Otherwise, a new type of entry will be generated, if nothing else helps us to determine this table is an object table.
 - `ignored`: When set to "true", we skip this table in SAI API generation.

dash-pipeline/bmv2/dash_acl.p4

@@ -33,7 +33,7 @@ match_kind {
 #ifdef TARGET_BMV2_V1MODEL
 #define ACL_STAGE(stage_index) \
     direct_counter(CounterType.packets_and_bytes) ## stage ## stage_index ##_counter; \
-    @SaiTable[name="dash_acl_rule", stage=str(acl.stage ## stage_index), api="dash_acl", api_order=1, isobject="true"] \
+    @SaiTable[name="dash_acl_rule", stage=str(acl.stage ## stage_index), api="dash_acl", order=1, isobject="true"] \
     table stage ## stage_index { \
         key = { \
             meta.stage ## stage_index ##_dash_acl_group_id : exact \

dash-pipeline/bmv2/dash_pipeline.p4

@@ -153,7 +153,7 @@ control dash_ingress(
         }
     }
 
-    @SaiTable[name = "eni", api = "dash_eni", api_order=1, isobject="true"]
+    @SaiTable[name = "eni", api = "dash_eni", order=1, isobject="true"]
     table eni {
         key = {
             meta.eni_id : exact @SaiVal[type="sai_object_id_t"];
@@ -255,7 +255,7 @@ control dash_ingress(
         }
     }
 
-    @SaiTable[name = "meter_policy", api = "dash_meter", api_order = 1, isobject="true"]
+    @SaiTable[name = "meter_policy", api = "dash_meter", order = 1, isobject="true"]
     table meter_policy {
         key = {
             meta.meter_policy_id : exact;
@@ -269,7 +269,7 @@ control dash_ingress(
         meta.policy_meter_class = meter_class;
     }
 
-    @SaiTable[name = "meter_rule", api = "dash_meter", api_order = 2, isobject="true"]
+    @SaiTable[name = "meter_rule", api = "dash_meter", order = 2, isobject="true"]
     table meter_rule {
         key = {
             meta.meter_policy_id: exact @SaiVal[type="sai_object_id_t", isresourcetype="true", objects="METER_POLICY"];
@@ -295,7 +295,7 @@ control dash_ingress(
         meta.meter_bucket_index = meter_bucket_index;
     }
 
-    @SaiTable[name = "meter_bucket", api = "dash_meter", api_order = 0, isobject="true"]
+    @SaiTable[name = "meter_bucket", api = "dash_meter", order = 0, isobject="true"]
     table meter_bucket {
         key = {
             meta.eni_id: exact @SaiVal[type="sai_object_id_t"];
@@ -312,7 +312,7 @@ control dash_ingress(
         meta.eni_id = eni_id;
     }
 
-    @SaiTable[name = "eni_ether_address_map", api = "dash_eni", api_order=0]
+    @SaiTable[name = "eni_ether_address_map", api = "dash_eni", order=0]
     table eni_ether_address_map {
         key = {
             meta.eni_addr : exact @SaiVal[name = "address", type = "sai_mac_t"];
@@ -337,7 +337,7 @@ control dash_ingress(
         }
     }
 
-    @SaiTable[name = "dash_acl_group", api = "dash_acl", api_order = 0, isobject="true"]
+    @SaiTable[name = "dash_acl_group", api = "dash_acl", order = 0, isobject="true"]
     table acl_group {
         key = {
             meta.stage1_dash_acl_group_id : exact @SaiVal[name = "dash_acl_group_id"];