From da0d5a83e68781257eea16992d26e0e52eaf72e1 Mon Sep 17 00:00:00 2001 From: Riff Date: Wed, 10 Jan 2024 22:21:38 -0800 Subject: [PATCH] [sai-gen] Add counter id generation support for counters. (#503) ## Problem Currently, DASH only supports generating counters as SAI attributes upon DASH APIs, such as metering buckets. This might be ok for new things that has dedicated DASH concepts, i.e., metering buckets. However, for other more generic counters used for collecting telemetry, this approach could leads to feature missing and not fully utilizing what we already have in SAI and SONiC. For example, no get-and-reset operation support and need to build dedicated pipeline for pulling the counters and feeds them into the telemetry. ## What are we doing in this change This change adds support to generate the counters as [SAI generic counters](https://github.com/opencomputeproject/SAI/blob/master/doc/SAI-Proposal-Generic-Counters.md). Instead of generate the counters as readonly attributes, we generate a counter id for it. Then, we can leverage all the infrastructure already built for counters. As the screenshot shows below, after removing `as_attr="true"` for metering_bucket_outbound, a counter id will be generated instead of an readonly counter attribute. It also supports handling the packets-and-bytes counter too, because generic counter already collects both information. ![image](https://github.com/sonic-net/DASH/assets/1533278/aa40224b-8b45-48c1-8233-14345dff6bb8) --- dash-pipeline/SAI/sai_api_gen.py | 55 ++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/dash-pipeline/SAI/sai_api_gen.py b/dash-pipeline/SAI/sai_api_gen.py index 69c5dc955..3ce783aa5 100755 --- a/dash-pipeline/SAI/sai_api_gen.py +++ b/dash-pipeline/SAI/sai_api_gen.py @@ -501,6 +501,11 @@ def link_ip_is_v6_vars(vars: List['SAIAPITableAttribute']) -> List['SAIAPITableA # Delete all vars with *_is_v6 in their names. return [v for v in vars if '_is_v6' not in v.name] + def set_sai_type(self, sai_type_info: SAITypeInfo) -> None: + self.type = sai_type_info.name + self.field = sai_type_info.sai_attribute_value_field + if self.default == None: + self.default = sai_type_info.default @sai_parser_from_p4rt class SAICounter(SAIAPITableAttribute): @@ -536,9 +541,20 @@ def parse_p4rt(self, p4rt_counter: Dict[str, Any], var_ref_graph: P4VarRefGraph) print("Parsing counter: " + self.name) self.__parse_sai_counter_annotation(p4rt_counter) - counter_storage_type = SAITypeSolver.get_object_sai_type(self.bitwidth) - self.type = counter_storage_type.name - self.field = counter_storage_type.sai_attribute_value_field + # If this counter is marked as SAI attributes, then we need to generate dedicated SAI attributes for this counter. + # In this case, the type needs to be created based on the size. + if self.as_attr: + counter_storage_type = SAITypeSolver.get_object_sai_type(self.bitwidth) + + # Otherwise, this counter should be linked to a SAI counter using an object ID. + # In this case, the type needs to be sai_object_id_t. + else: + counter_storage_type = SAITypeSolver.get_sai_type("sai_object_id_t") + self.name = f"{self.name}_counter_id" + self.isreadonly = "false" + self.object_name = "counter" + + self.set_sai_type(counter_storage_type) counter_unit = str(p4rt_counter['spec']['unit']).lower() if counter_unit in ['bytes', 'packets', 'both']: @@ -586,16 +602,22 @@ def __parse_sai_counter_annotation(self, p4rt_counter: Dict[str, Any]) -> None: raise ValueError("Unknown attr annotation " + kv['key']) def generate_final_counter_on_type(self) -> 'Iterator[SAICounter]': - if self.counter_type != 'both': - self.name = f"{self.name}_{self.counter_type}_counter" + # If this counter is not used as an attribute, then we need to treat it as a counter object id. + if not self.as_attr: yield self + + # Otherwise, we need to generate dedicated SAI attributes for this counter. else: - packets_counter = copy.deepcopy(self) - packets_counter.name = f"{packets_counter.name}_packets_counter" - yield packets_counter + if self.counter_type != 'both': + self.name = f"{self.name}_{self.counter_type}_counter" + yield self + else: + packets_counter = copy.deepcopy(self) + packets_counter.name = f"{packets_counter.name}_packets_counter" + yield packets_counter - self.name = f"{self.name}_bytes_counter" - yield self + self.name = f"{self.name}_bytes_counter" + yield self @sai_parser_from_p4rt @@ -645,11 +667,8 @@ def parse_p4rt(self, p4rt_table_key: Dict[str, Any]) -> None: sai_type_info = SAITypeSolver.get_sai_type(self.type) else: sai_type_info = SAITypeSolver.get_match_key_sai_type(self.match_type, self.bitwidth) - self.type = sai_type_info.name - self.field = sai_type_info.sai_attribute_value_field - if self.default == None: - self.default = sai_type_info.default + self.set_sai_type(sai_type_info) return @@ -725,11 +744,8 @@ def parse_p4rt(self, p4rt_table_action_param: Dict[str, Any]) -> None: sai_type_info = SAITypeSolver.get_sai_type(self.type) else: sai_type_info = SAITypeSolver.get_object_sai_type(self.bitwidth) - self.type = sai_type_info.name - self.field = sai_type_info.sai_attribute_value_field - if self.default == None: - self.default = sai_type_info.default + self.set_sai_type(sai_type_info) return @@ -934,8 +950,7 @@ def __build_sai_attributes_after_parsing(self): 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) + sai_attributes_by_order.setdefault(counter.order, []).append(counter) # Merge all attributes into a single list. self.sai_attributes = []