From 0d3d88707054b7b1d0c7d8f545554de32a6a58c5 Mon Sep 17 00:00:00 2001 From: r12f Date: Fri, 27 Sep 2024 23:43:44 -0700 Subject: [PATCH 1/9] Add tunnel member and next hop table. --- dash-pipeline/SAI/templates/saiapi.cpp.j2 | 22 ++-- .../SAI/utils/dash_p4/dash_p4_table.py | 10 +- .../utils/dash_p4/dash_p4_table_attribute.py | 3 + dash-pipeline/bmv2/dash_metadata.p4 | 3 + dash-pipeline/bmv2/stages/tunnel_stage.p4 | 101 +++++++++++++++--- 5 files changed, 114 insertions(+), 25 deletions(-) diff --git a/dash-pipeline/SAI/templates/saiapi.cpp.j2 b/dash-pipeline/SAI/templates/saiapi.cpp.j2 index 573aabbe3..b2cd171d0 100644 --- a/dash-pipeline/SAI/templates/saiapi.cpp.j2 +++ b/dash-pipeline/SAI/templates/saiapi.cpp.j2 @@ -59,15 +59,16 @@ static sai_status_t dash_sai_create_{{ table.name }}( matchActionEntry->set_table_id(tableId); - {% if table['keys'] | length== 1 %} - // SAI object table with single P4 key - object ID itself is the P4 table key - // Generate a SAI object ID and fill it as the P4 table key - auto mf = matchActionEntry->add_match(); - mf->set_field_id({{table['keys'][0].id}}); - auto mf_exact = mf->mutable_exact(); - //{{table['keys'][0].field}}SetVal(objId, mf_exact, {{table['keys'][0].bitwidth}}); - {{table['keys'][0].field}}SetVal(static_cast(objId), mf_exact, {{ table['keys'][0].bitwidth }}); - {% else %} + {% for key in table['keys'] %} + {% if key.is_object_key %} + auto key_mf = matchActionEntry->add_match(); + key_mf->set_field_id({{key.id}}); + auto key_mf_exact = key_mf->mutable_exact(); + // {{key.field}}SetVal(objId, key_mf_exact, {{key.bitwidth}}); + {{key.field}}SetVal(static_cast(objId), key_mf_exact, {{ key.bitwidth }}); + {% endif %} + {% endfor %} + // SAI object table with multiple P4 table keys // Copy P4 table keys from appropriate SAI attributes for (uint32_t i = 0; i < attr_count; i++) @@ -79,6 +80,7 @@ static sai_status_t dash_sai_create_{{ table.name }}( switch(attr_list[i].id) { {% for key in table['keys'] %} + {% if not key.is_object_key %} case SAI_{{ table.name | upper }}_ATTR_{{ key.name | upper }}: { auto mf = matchActionEntry->add_match(); @@ -138,6 +140,7 @@ static sai_status_t dash_sai_create_{{ table.name }}( {% endif %} break; } + {% endif%} {% endfor %} {% if table['keys'] | selectattr('match_type', 'ne', 'exact') | list | length > 0 %} {% if table['keys'] | selectattr('match_type', 'eq', 'lpm') | list | length == 0 %} @@ -154,7 +157,6 @@ static sai_status_t dash_sai_create_{{ table.name }}( break; } } - {% endif %} // If there is only one action, simply set it. // Else, search in the attrs. diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py index 103a2332c..67a7c5424 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py @@ -372,9 +372,13 @@ def create_sai_stats(self, sai_api: SaiApi) -> None: def create_sai_attributes(self, sai_api: SaiApi) -> None: # If the table is an object with more one key (table entry id), we need to add all the keys into the attributes. - if self.is_object == "true" and len(self.keys) > 1: - for key in self.keys: - sai_api.attributes.extend(key.to_sai_attribute(self.name, create_only=True)) + if self.is_object == "true": + if len(self.keys) == 1: + self.keys[0].is_object_key = True + elif len(self.keys) > 1: + for key in self.keys: + if not key.is_object_key: + sai_api.attributes.extend(key.to_sai_attribute(self.name, create_only=True)) # Add all the action parameters into the attributes. for attr in self.sai_attributes: diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py index 79bd50bee..86699cb4e 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py @@ -19,6 +19,7 @@ def __init__(self): self.skipattr: Optional[str] = None self.match_type: str = "" self.validonly: Optional[str] = None + self.is_object_key: bool = False def _parse_sai_table_attribute_annotation( self, p4rt_anno_list: Dict[str, Any] @@ -66,6 +67,8 @@ def _parse_sai_table_attribute_annotation( self.match_type = str(kv["value"]["stringValue"]) elif kv["key"] == "validonly": self.validonly = str(kv["value"]["stringValue"]) + elif kv["key"] == "is_object_key": + self.is_object_key = kv["value"]["stringValue"] == "true" else: raise ValueError("Unknown attr annotation " + kv["key"]) diff --git a/dash-pipeline/bmv2/dash_metadata.p4 b/dash-pipeline/bmv2/dash_metadata.p4 index 044aa1c52..2defa9610 100644 --- a/dash-pipeline/bmv2/dash_metadata.p4 +++ b/dash-pipeline/bmv2/dash_metadata.p4 @@ -322,6 +322,9 @@ struct metadata_t { encap_data_t tunnel_data; overlay_rewrite_data_t overlay_data; bit<16> dash_tunnel_id; + bit<16> dash_tunnel_member_index; + bit<16> dash_tunnel_member_id; + bit<16> dash_tunnel_next_hop_id; bit<32> meter_class; bit<8> local_region_id; } diff --git a/dash-pipeline/bmv2/stages/tunnel_stage.p4 b/dash-pipeline/bmv2/stages/tunnel_stage.p4 index bf8b878ab..e1b7bfe9c 100644 --- a/dash-pipeline/bmv2/stages/tunnel_stage.p4 +++ b/dash-pipeline/bmv2/stages/tunnel_stage.p4 @@ -6,18 +6,19 @@ control tunnel_stage( inout metadata_t meta) { action set_tunnel_attrs( - @SaiVal[type="sai_ip_address_t"] - IPv4Address dip, - @SaiVal[type="sai_dash_encapsulation_t", default_value="SAI_DASH_ENCAPSULATION_VXLAN"] - dash_encapsulation_t dash_encapsulation, - bit<24> tunnel_key) { - push_action_static_encap(hdr = hdr, - meta = meta, - encap = dash_encapsulation, - vni = tunnel_key, - underlay_sip = hdr.u0_ipv4.src_addr, - underlay_dip = dip, - overlay_dmac = hdr.u0_ethernet.dst_addr); + @SaiVal[type="sai_ip_address_t"] + IPv4Address dip, + @SaiVal[type="sai_dash_encapsulation_t", default_value="SAI_DASH_ENCAPSULATION_VXLAN"] + dash_encapsulation_t dash_encapsulation, + bit<24> tunnel_key) + { + push_action_static_encap(hdr = hdr, + meta = meta, + encap = dash_encapsulation, + vni = tunnel_key, + underlay_sip = hdr.u0_ipv4.src_addr, + underlay_dip = dip, + overlay_dmac = hdr.u0_ethernet.dst_addr); } @SaiTable[name = "dash_tunnel", api = "dash_tunnel", order = 0, isobject="true"] @@ -31,8 +32,84 @@ control tunnel_stage( } } + action select_tunnel_member( + bit<16> dash_tunnel_member_id) + { + meta.dash_tunnel_member_id = dash_tunnel_member_id; + } + + // This table is a helper table that used to select the tunnel member based on the index. + // The entry of this table is created by DASH data plane app, when the tunnel member is created. + @SaiTable[ignored = "true"] + table tunnel_member_select { + key = { + meta.dash_tunnel_member_index : exact @SaiVal[type="sai_object_id_t", is_object_key="true"]; + meta.dash_tunnel_id : exact @SaiVal[type="sai_object_id_t"]; + } + + actions = { + select_tunnel_member; + } + } + + action set_tunnel_member_attrs( + @SaiVal[type="sai_object_id_t"] bit<16> dash_tunnel_id, + @SaiVal[type="sai_object_id_t"] bit<16> dash_tunnel_next_hop_id) + { + // dash_tunnel_id in tunnel member must match the metadata + assert(meta.dash_tunnel_id == dash_tunnel_id); + + meta.dash_tunnel_next_hop_id = dash_tunnel_next_hop_id; + } + + @SaiTable[name = "dash_tunnel_member", api = "dash_tunnel", order = 1, isobject="true"] + table tunnel_member { + key = { + meta.dash_tunnel_member_id : exact @SaiVal[type="sai_object_id_t", is_object_key="true"]; + } + + actions = { + set_tunnel_member_attrs; + } + } + + action set_tunnel_next_hop_attrs( + @SaiVal[type="sai_ip_address_t"] + IPv4Address dip, + @SaiVal[type="sai_dash_encapsulation_t", default_value="SAI_DASH_ENCAPSULATION_VXLAN"] + dash_encapsulation_t dash_encapsulation, + bit<24> tunnel_key) + { + push_action_static_encap(hdr = hdr, + meta = meta, + encap = dash_encapsulation, + vni = tunnel_key, + underlay_sip = hdr.u0_ipv4.src_addr, + underlay_dip = dip, + overlay_dmac = hdr.u0_ethernet.dst_addr); + } + + @SaiTable[name = "dash_tunnel_next_hop", api = "dash_tunnel", order = 2, isobject="true"] + table tunnel_next_hop{ + key = { + meta.dash_tunnel_next_hop_id : exact @SaiVal[type="sai_object_id_t"]; + } + + actions = { + set_tunnel_next_hop_attrs; + } + } + apply { tunnel.apply(); + + // TODO: Calculate based on packet and tunnel member size. + // Currently, we will have to use 0 here, because we don't know how many tunnel members we will have. + meta.dash_tunnel_member_index = 0; + tunnel_member_select.apply(); + + tunnel_member.apply(); + tunnel_next_hop.apply(); } } From d308722d4865fef0339751a1663652c586137054 Mon Sep 17 00:00:00 2001 From: r12f Date: Tue, 1 Oct 2024 20:12:32 -0700 Subject: [PATCH 2/9] Remove tunnel common setting from next hop. --- dash-pipeline/bmv2/stages/tunnel_stage.p4 | 67 +++++++++++++---------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/dash-pipeline/bmv2/stages/tunnel_stage.p4 b/dash-pipeline/bmv2/stages/tunnel_stage.p4 index e1b7bfe9c..23578e63a 100644 --- a/dash-pipeline/bmv2/stages/tunnel_stage.p4 +++ b/dash-pipeline/bmv2/stages/tunnel_stage.p4 @@ -6,19 +6,21 @@ control tunnel_stage( inout metadata_t meta) { action set_tunnel_attrs( - @SaiVal[type="sai_ip_address_t"] - IPv4Address dip, @SaiVal[type="sai_dash_encapsulation_t", default_value="SAI_DASH_ENCAPSULATION_VXLAN"] dash_encapsulation_t dash_encapsulation, - bit<24> tunnel_key) + + bit<24> tunnel_key, + + @SaiVal[type="sai_ip_address_t"] + IPv4Address dip, + + @SaiVal[type="sai_ip_address_t"] + IPv4Address sip) { - push_action_static_encap(hdr = hdr, - meta = meta, - encap = dash_encapsulation, - vni = tunnel_key, - underlay_sip = hdr.u0_ipv4.src_addr, - underlay_dip = dip, - overlay_dmac = hdr.u0_ethernet.dst_addr); + meta.tunnel_data.dash_encapsulation = dash_encapsulation; + meta.tunnel_data.vni = tunnel_key; + meta.tunnel_data.underlay_sip = sip == 0 ? hdr.u0_ipv4.src_addr : sip; + meta.tunnel_data.underlay_dip = dip; } @SaiTable[name = "dash_tunnel", api = "dash_tunnel", order = 0, isobject="true"] @@ -57,7 +59,7 @@ control tunnel_stage( @SaiVal[type="sai_object_id_t"] bit<16> dash_tunnel_next_hop_id) { // dash_tunnel_id in tunnel member must match the metadata - assert(meta.dash_tunnel_id == dash_tunnel_id); + REQUIRES(meta.dash_tunnel_id == dash_tunnel_id); meta.dash_tunnel_next_hop_id = dash_tunnel_next_hop_id; } @@ -75,22 +77,13 @@ control tunnel_stage( action set_tunnel_next_hop_attrs( @SaiVal[type="sai_ip_address_t"] - IPv4Address dip, - @SaiVal[type="sai_dash_encapsulation_t", default_value="SAI_DASH_ENCAPSULATION_VXLAN"] - dash_encapsulation_t dash_encapsulation, - bit<24> tunnel_key) + IPv4Address dip) { - push_action_static_encap(hdr = hdr, - meta = meta, - encap = dash_encapsulation, - vni = tunnel_key, - underlay_sip = hdr.u0_ipv4.src_addr, - underlay_dip = dip, - overlay_dmac = hdr.u0_ethernet.dst_addr); + meta.tunnel_data.underlay_dip = dip == 0 ? meta.tunnel_data.underlay_dip : dip; } @SaiTable[name = "dash_tunnel_next_hop", api = "dash_tunnel", order = 2, isobject="true"] - table tunnel_next_hop{ + table tunnel_next_hop { key = { meta.dash_tunnel_next_hop_id : exact @SaiVal[type="sai_object_id_t"]; } @@ -101,15 +94,31 @@ control tunnel_stage( } apply { + if (meta.dash_tunnel_id == 0) { + return; + } + tunnel.apply(); - // TODO: Calculate based on packet and tunnel member size. - // Currently, we will have to use 0 here, because we don't know how many tunnel members we will have. - meta.dash_tunnel_member_index = 0; - tunnel_member_select.apply(); + if (meta.tunnel_data.underlay_dip == 0) { + // If underlay_dip is not set by the tunnel, we are using the tunnel as ECMP group. - tunnel_member.apply(); - tunnel_next_hop.apply(); + // TODO: Calculate based on packet and tunnel member size. + // Currently, we will have to use 0 here, because we don't know how many tunnel members we will have. + meta.dash_tunnel_member_index = 0; + tunnel_member_select.apply(); + + tunnel_member.apply(); + tunnel_next_hop.apply(); + } + + push_action_static_encap(hdr = hdr, + meta = meta, + encap = meta.tunnel_data.dash_encapsulation, + vni = meta.tunnel_data.vni, + underlay_sip = meta.tunnel_data.underlay_sip, + underlay_dip = meta.tunnel_data.underlay_dip, + overlay_dmac = hdr.u0_ethernet.dst_addr); } } From 78b1e3a709c37131508de52e13a4a8b72840429b Mon Sep 17 00:00:00 2001 From: r12f Date: Thu, 3 Oct 2024 11:02:03 -0700 Subject: [PATCH 3/9] Add max size. --- dash-pipeline/SAI/templates/saiapi.h.j2 | 2 ++ .../SAI/utils/dash_p4/dash_p4_table.py | 5 ++++- .../utils/dash_p4/dash_p4_table_attribute.py | 15 +++++++++---- .../SAI/utils/dash_p4/dash_p4_table_key.py | 1 + dash-pipeline/bmv2/README.md | 3 ++- dash-pipeline/bmv2/dash_metadata.p4 | 1 + dash-pipeline/bmv2/stages/tunnel_stage.p4 | 22 ++++++++++++++----- 7 files changed, 38 insertions(+), 11 deletions(-) diff --git a/dash-pipeline/SAI/templates/saiapi.h.j2 b/dash-pipeline/SAI/templates/saiapi.h.j2 index 36c942415..bafb639c5 100644 --- a/dash-pipeline/SAI/templates/saiapi.h.j2 +++ b/dash-pipeline/SAI/templates/saiapi.h.j2 @@ -166,6 +166,8 @@ typedef enum _sai_{{ table.name }}_attr_t * @type {{ sai_attr.type }} {% if sai_attr.isreadonly == 'true' %} * @flags READ_ONLY +{% else if sai_attr.iscreateonly == 'true' %} + * @flags CREATE_ONLY {% else %} * @flags CREATE_AND_SET {% endif %} diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py index 67a7c5424..1d03687fe 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py @@ -127,6 +127,9 @@ def __table_with_counters(self, program: Dict[str, Any]) -> None: def __parse_table_keys(self, p4rt_table: Dict[str, Any]) -> None: for p4rt_table_key in p4rt_table[MATCH_FIELDS_TAG]: table_key = DashP4TableKey.from_p4rt(p4rt_table_key) + + # For table keys, we currently require the values to be explicitly set. + table_key.default = None if self.is_object != "false": table_key.is_entry_key = False @@ -378,7 +381,7 @@ def create_sai_attributes(self, sai_api: SaiApi) -> None: elif len(self.keys) > 1: for key in self.keys: if not key.is_object_key: - sai_api.attributes.extend(key.to_sai_attribute(self.name, create_only=True)) + sai_api.attributes.extend(key.to_sai_attribute(self.name)) # Add all the action parameters into the attributes. for attr in self.sai_attributes: diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py index 86699cb4e..bdde67de3 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py @@ -15,6 +15,7 @@ def __init__(self): self.bitwidth: int = 0 self.isresourcetype: Optional[str] = None self.isreadonly: Optional[str] = None + self.iscreateonly: Optional[str] = None self.object_name: Optional[str] = None self.skipattr: Optional[str] = None self.match_type: str = "" @@ -59,6 +60,8 @@ def _parse_sai_table_attribute_annotation( self.isresourcetype = str(kv["value"]["stringValue"]) elif kv["key"] == "isreadonly": self.isreadonly = str(kv["value"]["stringValue"]) + elif kv["key"] == "iscreateonly": + self.iscreateonly = str(kv["value"]["stringValue"]) elif kv["key"] == "objects": self.object_name = str(kv["value"]["stringValue"]) elif kv["key"] == "skipattr": @@ -125,7 +128,7 @@ def to_sai_struct_entry(self, table_name: str) -> List[SaiStructEntry]: return entries - def to_sai_attribute(self, table_name: str, create_only: bool = False, add_action_valid_only_check: bool = False) -> List[SaiAttribute]: + def to_sai_attribute(self, table_name: str, add_action_valid_only_check: bool = False) -> List[SaiAttribute]: name = self.get_sai_name(table_name) description = self.get_sai_description(table_name) @@ -136,9 +139,13 @@ def to_sai_attribute(self, table_name: str, create_only: bool = False, add_actio if self.isreadonly == "true": sai_flags = "READ_ONLY" default_value = None - elif create_only: - sai_flags = "MANDATORY_ON_CREATE | CREATE_ONLY" - default_value = None + elif self.iscreateonly == "true": + if self.default == None: + sai_flags = "MANDATORY_ON_CREATE | CREATE_ONLY" + else: + sai_flags = "CREATE_ONLY" + + default_value = self.default allow_null = False else: sai_flags = "CREATE_AND_SET" diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py index a360f1268..758497dd3 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py @@ -11,6 +11,7 @@ class DashP4TableKey(DashP4TableAttribute): def __init__(self): super().__init__() + self.iscreateonly: str = "true" self.ip_is_v6_field_id: int = 0 def parse_p4rt(self, p4rt_table_key: Dict[str, Any]) -> None: diff --git a/dash-pipeline/bmv2/README.md b/dash-pipeline/bmv2/README.md index d3949ce11..9b0baac57 100644 --- a/dash-pipeline/bmv2/README.md +++ b/dash-pipeline/bmv2/README.md @@ -30,7 +30,8 @@ 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 `. -- `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`. +- `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`. It cannot be used with `iscreateonly`. +- `iscreateonly`: When set to "true", we generate force this value to be create-only in SAI API using: `@flags CREATE_ONLY`, otherwise, we generate `@flags CREATE_AND_SET`. It cannot be used with `isreadonly`. - `skipattr`: When set to "true", we skip this attribute in SAI API generation. #### `@SaiCounter`: Counters diff --git a/dash-pipeline/bmv2/dash_metadata.p4 b/dash-pipeline/bmv2/dash_metadata.p4 index 2defa9610..62668b23b 100644 --- a/dash-pipeline/bmv2/dash_metadata.p4 +++ b/dash-pipeline/bmv2/dash_metadata.p4 @@ -322,6 +322,7 @@ struct metadata_t { encap_data_t tunnel_data; overlay_rewrite_data_t overlay_data; bit<16> dash_tunnel_id; + bit<32> dash_tunnel_max_member_size; bit<16> dash_tunnel_member_index; bit<16> dash_tunnel_member_id; bit<16> dash_tunnel_next_hop_id; diff --git a/dash-pipeline/bmv2/stages/tunnel_stage.p4 b/dash-pipeline/bmv2/stages/tunnel_stage.p4 index 23578e63a..c9d824d1a 100644 --- a/dash-pipeline/bmv2/stages/tunnel_stage.p4 +++ b/dash-pipeline/bmv2/stages/tunnel_stage.p4 @@ -11,12 +11,17 @@ control tunnel_stage( bit<24> tunnel_key, + @SaiVal[default_value="1", iscreateonly="true"] + bit<32> max_member_size, + @SaiVal[type="sai_ip_address_t"] IPv4Address dip, @SaiVal[type="sai_ip_address_t"] IPv4Address sip) { + meta.dash_tunnel_max_member_size = max_member_size; + meta.tunnel_data.dash_encapsulation = dash_encapsulation; meta.tunnel_data.vni = tunnel_key; meta.tunnel_data.underlay_sip = sip == 0 ? hdr.u0_ipv4.src_addr : sip; @@ -100,12 +105,19 @@ control tunnel_stage( tunnel.apply(); - if (meta.tunnel_data.underlay_dip == 0) { - // If underlay_dip is not set by the tunnel, we are using the tunnel as ECMP group. - - // TODO: Calculate based on packet and tunnel member size. - // Currently, we will have to use 0 here, because we don't know how many tunnel members we will have. + // If max member size is greater than 1, the tunnel is programmed with multiple members. + if (meta.dash_tunnel_max_member_size > 1) { +#if defined(TARGET_BMV2_V1MODEL) + // Select tunnel member based on the hash of the packet tuples. + hash(meta.dash_tunnel_member_index, HashAlgorithm.crc32, (bit<32>)0, { + meta.dst_ip_addr, + meta.src_ip_addr, + meta.src_l4_port, + meta.dst_l4_port + }, meta.dash_tunnel_max_member_size); +#else meta.dash_tunnel_member_index = 0; +#endif tunnel_member_select.apply(); tunnel_member.apply(); From 2ec8597186d403c0e2f30e1e62c818e5975f12be Mon Sep 17 00:00:00 2001 From: r12f Date: Thu, 3 Oct 2024 11:14:58 -0700 Subject: [PATCH 4/9] Fix tunnel member attribute flags. --- .../SAI/utils/dash_p4/dash_p4_table.py | 3 --- .../utils/dash_p4/dash_p4_table_attribute.py | 18 ++++++++++++++---- .../SAI/utils/dash_p4/dash_p4_table_key.py | 4 ++++ dash-pipeline/bmv2/README.md | 1 + dash-pipeline/bmv2/stages/tunnel_stage.p4 | 4 ++-- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py index 1d03687fe..f931d57cb 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table.py @@ -128,9 +128,6 @@ def __parse_table_keys(self, p4rt_table: Dict[str, Any]) -> None: for p4rt_table_key in p4rt_table[MATCH_FIELDS_TAG]: table_key = DashP4TableKey.from_p4rt(p4rt_table_key) - # For table keys, we currently require the values to be explicitly set. - table_key.default = None - if self.is_object != "false": table_key.is_entry_key = False diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py index bdde67de3..537c8b30a 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py @@ -16,6 +16,7 @@ def __init__(self): self.isresourcetype: Optional[str] = None self.isreadonly: Optional[str] = None self.iscreateonly: Optional[str] = None + self.ismandatory: Optional[str] = None self.object_name: Optional[str] = None self.skipattr: Optional[str] = None self.match_type: str = "" @@ -62,6 +63,8 @@ def _parse_sai_table_attribute_annotation( self.isreadonly = str(kv["value"]["stringValue"]) elif kv["key"] == "iscreateonly": self.iscreateonly = str(kv["value"]["stringValue"]) + elif kv["key"] == "ismandatory": + self.ismandatory = str(kv["value"]["stringValue"]) elif kv["key"] == "objects": self.object_name = str(kv["value"]["stringValue"]) elif kv["key"] == "skipattr": @@ -140,16 +143,23 @@ def to_sai_attribute(self, table_name: str, add_action_valid_only_check: bool = sai_flags = "READ_ONLY" default_value = None elif self.iscreateonly == "true": - if self.default == None: + if self.default == None or self.ismandatory == "true": sai_flags = "MANDATORY_ON_CREATE | CREATE_ONLY" + default_value = None + allow_null = False else: sai_flags = "CREATE_ONLY" + default_value = self.default - default_value = self.default allow_null = False else: - sai_flags = "CREATE_AND_SET" - default_value = self.default + if self.ismandatory == "true": + sai_flags = "MANDATORY_ON_CREATE | CREATE_AND_SET" + default_value = None + allow_null = False + else: + sai_flags = "CREATE_AND_SET" + default_value = self.default valid_only_checks = [] if add_action_valid_only_check: diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py index 758497dd3..1432c0a61 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py @@ -12,6 +12,7 @@ class DashP4TableKey(DashP4TableAttribute): def __init__(self): super().__init__() self.iscreateonly: str = "true" + self.ismandatory: str = "true" self.ip_is_v6_field_id: int = 0 def parse_p4rt(self, p4rt_table_key: Dict[str, Any]) -> None: @@ -56,6 +57,9 @@ def parse_p4rt(self, p4rt_table_key: Dict[str, Any]) -> None: self.set_sai_type(sai_type_info) + # For table keys, we currently require the values to be explicitly set. + self.default = None + return # diff --git a/dash-pipeline/bmv2/README.md b/dash-pipeline/bmv2/README.md index 9b0baac57..1d2927a2e 100644 --- a/dash-pipeline/bmv2/README.md +++ b/dash-pipeline/bmv2/README.md @@ -32,6 +32,7 @@ Available tags are: - `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 `. - `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`. It cannot be used with `iscreateonly`. - `iscreateonly`: When set to "true", we generate force this value to be create-only in SAI API using: `@flags CREATE_ONLY`, otherwise, we generate `@flags CREATE_AND_SET`. It cannot be used with `isreadonly`. +- `ismandatory`: When set to "true", we generate force this value to be mandatory in SAI API using: `@flags MANDATORY_ON_CREATE`. It cannot be used with `isreadonly`. - `skipattr`: When set to "true", we skip this attribute in SAI API generation. #### `@SaiCounter`: Counters diff --git a/dash-pipeline/bmv2/stages/tunnel_stage.p4 b/dash-pipeline/bmv2/stages/tunnel_stage.p4 index c9d824d1a..3f218cb2e 100644 --- a/dash-pipeline/bmv2/stages/tunnel_stage.p4 +++ b/dash-pipeline/bmv2/stages/tunnel_stage.p4 @@ -60,8 +60,8 @@ control tunnel_stage( } action set_tunnel_member_attrs( - @SaiVal[type="sai_object_id_t"] bit<16> dash_tunnel_id, - @SaiVal[type="sai_object_id_t"] bit<16> dash_tunnel_next_hop_id) + @SaiVal[type="sai_object_id_t", ismandatory="true", iscreateonly="true"] bit<16> dash_tunnel_id, + @SaiVal[type="sai_object_id_t", ismandatory="true"] bit<16> dash_tunnel_next_hop_id) { // dash_tunnel_id in tunnel member must match the metadata REQUIRES(meta.dash_tunnel_id == dash_tunnel_id); From c1b8186e3d5a496311633bb74e2e09b119544104 Mon Sep 17 00:00:00 2001 From: r12f Date: Thu, 3 Oct 2024 11:15:08 -0700 Subject: [PATCH 5/9] Update SAI spec. --- dash-pipeline/SAI/specs/dash_tunnel.yaml | 99 ++++++++++++++++++++++++ dash-pipeline/SAI/specs/sai_spec.yaml | 2 + 2 files changed, 101 insertions(+) diff --git a/dash-pipeline/SAI/specs/dash_tunnel.yaml b/dash-pipeline/SAI/specs/dash_tunnel.yaml index b73273e6b..02d6cc272 100644 --- a/dash-pipeline/SAI/specs/dash_tunnel.yaml +++ b/dash-pipeline/SAI/specs/dash_tunnel.yaml @@ -49,6 +49,32 @@ sai_apis: valid_only: null is_vlan: false deprecated: false + - !!python/object:utils.sai_spec.sai_attribute.SaiAttribute + name: SAI_DASH_TUNNEL_ATTR_MAX_MEMBER_SIZE + description: Action parameter max member size + type: sai_uint32_t + attr_value_field: u32 + default: '1' + isresourcetype: false + flags: CREATE_ONLY + object_name: null + allow_null: false + valid_only: null + is_vlan: false + deprecated: false + - !!python/object:utils.sai_spec.sai_attribute.SaiAttribute + name: SAI_DASH_TUNNEL_ATTR_SIP + description: Action parameter sip + type: sai_ip_address_t + attr_value_field: ipaddr + default: 0.0.0.0 + isresourcetype: false + flags: CREATE_AND_SET + object_name: null + allow_null: false + valid_only: null + is_vlan: false + deprecated: false stats: [] p4_meta: !!python/object:utils.sai_spec.sai_api_p4_meta.SaiApiP4Meta tables: @@ -59,3 +85,76 @@ sai_apis: name: default id: 27891720 attr_param_id: {} +- !!python/object:utils.sai_spec.sai_api.SaiApi + name: dash_tunnel_member + description: DASH tunnel member + is_object: true + enums: [] + structs: [] + attributes: + - !!python/object:utils.sai_spec.sai_attribute.SaiAttribute + name: SAI_DASH_TUNNEL_MEMBER_ATTR_DASH_TUNNEL_ID + description: Action parameter DASH tunnel id + type: sai_object_id_t + attr_value_field: u16 + default: null + isresourcetype: false + flags: MANDATORY_ON_CREATE | CREATE_ONLY + object_name: SAI_OBJECT_TYPE_DASH_TUNNEL + allow_null: false + valid_only: null + is_vlan: false + deprecated: false + - !!python/object:utils.sai_spec.sai_attribute.SaiAttribute + name: SAI_DASH_TUNNEL_MEMBER_ATTR_DASH_TUNNEL_NEXT_HOP_ID + description: Action parameter DASH tunnel next hop id + type: sai_object_id_t + attr_value_field: u16 + default: null + isresourcetype: false + flags: MANDATORY_ON_CREATE | CREATE_AND_SET + object_name: SAI_OBJECT_TYPE_DASH_TUNNEL_NEXT_HOP + allow_null: false + valid_only: null + is_vlan: false + deprecated: false + stats: [] + p4_meta: !!python/object:utils.sai_spec.sai_api_p4_meta.SaiApiP4Meta + tables: + - !!python/object:utils.sai_spec.sai_api_p4_meta.SaiApiP4MetaTable + id: 40012474 + actions: + default: !!python/object:utils.sai_spec.sai_api_p4_meta.SaiApiP4MetaAction + name: default + id: 23738116 + attr_param_id: {} +- !!python/object:utils.sai_spec.sai_api.SaiApi + name: dash_tunnel_next_hop + description: DASH tunnel next hop + is_object: true + enums: [] + structs: [] + attributes: + - !!python/object:utils.sai_spec.sai_attribute.SaiAttribute + name: SAI_DASH_TUNNEL_NEXT_HOP_ATTR_DIP + description: Action parameter dip + type: sai_ip_address_t + attr_value_field: ipaddr + default: 0.0.0.0 + isresourcetype: false + flags: CREATE_AND_SET + object_name: null + allow_null: false + valid_only: null + is_vlan: false + deprecated: false + stats: [] + p4_meta: !!python/object:utils.sai_spec.sai_api_p4_meta.SaiApiP4Meta + tables: + - !!python/object:utils.sai_spec.sai_api_p4_meta.SaiApiP4MetaTable + id: 34229686 + actions: + default: !!python/object:utils.sai_spec.sai_api_p4_meta.SaiApiP4MetaAction + name: default + id: 31749609 + attr_param_id: {} diff --git a/dash-pipeline/SAI/specs/sai_spec.yaml b/dash-pipeline/SAI/specs/sai_spec.yaml index e770e98a1..a5163cb75 100644 --- a/dash-pipeline/SAI/specs/sai_spec.yaml +++ b/dash-pipeline/SAI/specs/sai_spec.yaml @@ -41,6 +41,8 @@ object_types: - SAI_OBJECT_TYPE_FLOW_ENTRY_BULK_GET_SESSION - SAI_OBJECT_TYPE_METER_BUCKET_ENTRY - SAI_OBJECT_TYPE_DASH_APPLIANCE +- SAI_OBJECT_TYPE_DASH_TUNNEL_MEMBER +- SAI_OBJECT_TYPE_DASH_TUNNEL_NEXT_HOP object_entries: - !!python/object:utils.sai_spec.sai_struct_entry.SaiStructEntry name: direction_lookup_entry From 6fdca326674507a81b5d3489d7f01de9f16d64d9 Mon Sep 17 00:00:00 2001 From: r12f Date: Sun, 20 Oct 2024 13:33:23 -0700 Subject: [PATCH 6/9] Making tunnel encap and key create only. --- .../utils/dash_p4/dash_p4_table_attribute.py | 18 +++++++++--------- dash-pipeline/bmv2/stages/tunnel_stage.p4 | 9 +++++---- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py index 537c8b30a..8e3bceef0 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_attribute.py @@ -15,8 +15,8 @@ def __init__(self): self.bitwidth: int = 0 self.isresourcetype: Optional[str] = None self.isreadonly: Optional[str] = None - self.iscreateonly: Optional[str] = None - self.ismandatory: Optional[str] = None + self.is_create_only: Optional[str] = None + self.is_mandatory: Optional[str] = None self.object_name: Optional[str] = None self.skipattr: Optional[str] = None self.match_type: str = "" @@ -61,10 +61,10 @@ def _parse_sai_table_attribute_annotation( self.isresourcetype = str(kv["value"]["stringValue"]) elif kv["key"] == "isreadonly": self.isreadonly = str(kv["value"]["stringValue"]) - elif kv["key"] == "iscreateonly": - self.iscreateonly = str(kv["value"]["stringValue"]) - elif kv["key"] == "ismandatory": - self.ismandatory = str(kv["value"]["stringValue"]) + elif kv["key"] == "create_only": + self.is_create_only = str(kv["value"]["stringValue"]) + elif kv["key"] == "mandatory": + self.is_mandatory = str(kv["value"]["stringValue"]) elif kv["key"] == "objects": self.object_name = str(kv["value"]["stringValue"]) elif kv["key"] == "skipattr": @@ -142,8 +142,8 @@ def to_sai_attribute(self, table_name: str, add_action_valid_only_check: bool = if self.isreadonly == "true": sai_flags = "READ_ONLY" default_value = None - elif self.iscreateonly == "true": - if self.default == None or self.ismandatory == "true": + elif self.is_create_only == "true": + if self.default == None or self.is_mandatory == "true": sai_flags = "MANDATORY_ON_CREATE | CREATE_ONLY" default_value = None allow_null = False @@ -153,7 +153,7 @@ def to_sai_attribute(self, table_name: str, add_action_valid_only_check: bool = allow_null = False else: - if self.ismandatory == "true": + if self.is_mandatory == "true": sai_flags = "MANDATORY_ON_CREATE | CREATE_AND_SET" default_value = None allow_null = False diff --git a/dash-pipeline/bmv2/stages/tunnel_stage.p4 b/dash-pipeline/bmv2/stages/tunnel_stage.p4 index 3f218cb2e..66194c667 100644 --- a/dash-pipeline/bmv2/stages/tunnel_stage.p4 +++ b/dash-pipeline/bmv2/stages/tunnel_stage.p4 @@ -6,12 +6,13 @@ control tunnel_stage( inout metadata_t meta) { action set_tunnel_attrs( - @SaiVal[type="sai_dash_encapsulation_t", default_value="SAI_DASH_ENCAPSULATION_VXLAN"] + @SaiVal[type="sai_dash_encapsulation_t", default_value="SAI_DASH_ENCAPSULATION_VXLAN", create_only="true"] dash_encapsulation_t dash_encapsulation, + @SaiVal[create_only="true"] bit<24> tunnel_key, - @SaiVal[default_value="1", iscreateonly="true"] + @SaiVal[default_value="1", create_only="true"] bit<32> max_member_size, @SaiVal[type="sai_ip_address_t"] @@ -60,8 +61,8 @@ control tunnel_stage( } action set_tunnel_member_attrs( - @SaiVal[type="sai_object_id_t", ismandatory="true", iscreateonly="true"] bit<16> dash_tunnel_id, - @SaiVal[type="sai_object_id_t", ismandatory="true"] bit<16> dash_tunnel_next_hop_id) + @SaiVal[type="sai_object_id_t", mandatory="true", create_only="true"] bit<16> dash_tunnel_id, + @SaiVal[type="sai_object_id_t", mandatory="true"] bit<16> dash_tunnel_next_hop_id) { // dash_tunnel_id in tunnel member must match the metadata REQUIRES(meta.dash_tunnel_id == dash_tunnel_id); From 53f946d1b0d8e8750ce1b5e8736a87ac82525fb3 Mon Sep 17 00:00:00 2001 From: r12f Date: Sun, 20 Oct 2024 13:33:44 -0700 Subject: [PATCH 7/9] Minor refactor. --- dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py index 1432c0a61..d60a646cb 100644 --- a/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py +++ b/dash-pipeline/SAI/utils/dash_p4/dash_p4_table_key.py @@ -11,8 +11,8 @@ class DashP4TableKey(DashP4TableAttribute): def __init__(self): super().__init__() - self.iscreateonly: str = "true" - self.ismandatory: str = "true" + self.is_create_only: str = "true" + self.is_mandatory: str = "true" self.ip_is_v6_field_id: int = 0 def parse_p4rt(self, p4rt_table_key: Dict[str, Any]) -> None: From 08abadbd00a047aced647fc7e1f4b48432ef822d Mon Sep 17 00:00:00 2001 From: r12f Date: Sun, 20 Oct 2024 13:34:25 -0700 Subject: [PATCH 8/9] Update SAI spec. --- dash-pipeline/SAI/specs/dash_tunnel.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dash-pipeline/SAI/specs/dash_tunnel.yaml b/dash-pipeline/SAI/specs/dash_tunnel.yaml index 02d6cc272..d90724899 100644 --- a/dash-pipeline/SAI/specs/dash_tunnel.yaml +++ b/dash-pipeline/SAI/specs/dash_tunnel.yaml @@ -30,7 +30,7 @@ sai_apis: attr_value_field: s32 default: SAI_DASH_ENCAPSULATION_VXLAN isresourcetype: false - flags: CREATE_AND_SET + flags: CREATE_ONLY object_name: null allow_null: false valid_only: null @@ -43,7 +43,7 @@ sai_apis: attr_value_field: u32 default: '0' isresourcetype: false - flags: CREATE_AND_SET + flags: CREATE_ONLY object_name: null allow_null: false valid_only: null From 91c1389160917f90f4a25823546d53346ebefaa3 Mon Sep 17 00:00:00 2001 From: r12f Date: Sun, 20 Oct 2024 13:34:49 -0700 Subject: [PATCH 9/9] Add missing template update. --- dash-pipeline/SAI/templates/saiapi.h.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dash-pipeline/SAI/templates/saiapi.h.j2 b/dash-pipeline/SAI/templates/saiapi.h.j2 index bafb639c5..ad8c94d10 100644 --- a/dash-pipeline/SAI/templates/saiapi.h.j2 +++ b/dash-pipeline/SAI/templates/saiapi.h.j2 @@ -166,7 +166,7 @@ typedef enum _sai_{{ table.name }}_attr_t * @type {{ sai_attr.type }} {% if sai_attr.isreadonly == 'true' %} * @flags READ_ONLY -{% else if sai_attr.iscreateonly == 'true' %} +{% else if sai_attr.is_create_only == 'true' %} * @flags CREATE_ONLY {% else %} * @flags CREATE_AND_SET