Prefix Compression feature addition to SAI #2045

nadershinouda · 2024-07-08T19:49:38Z

This pull requests adds a new feature, prefix compression, to the SAI repository. Prefix Compression allows mapping an IP prefix/mask to a bincode (Binary Code). These prefix/bincode mapping can be grouped together to form prefix compression table. Tables can have both IPV4 and IPV6 entries.

This pull request also adds extending ACL spec to add support for bincode field matching. Bincode is part of prefix compression, where an IP prefix is mapped to Bincode.

Files changed:
SAI-Proposal-ACL-Bincode.md -- ACL extension documentation. This extension allows ACL to do lookups based on a bincode

SAI-Proposal-Prefix-Compression.md -- Prefix Compression documentation.

sah.h -- Addition of a new SAI API (prefix compression)

saiacl.h -- Adding extenstions to SAI ACL to support Bincode lookups

saiobject.h -- Adding a new entry type. Prefix compression Entry is the entry used to add a new prefix/bincode mapping to a prefix compression table

saiprefixcompression.h -- The SAI header file that defines Prefix Compression

saitypes.h -- Addition of new types to support prefix compression feature. Table and entry types

acronyms.txt -- Adding a defination for BINCODE

tjchadaga · 2024-07-18T16:57:36Z

@nadershinouda - please look into the metadata check failure. Adding @kcudnik

nadershinouda · 2024-07-18T17:19:46Z

@nadershinouda - please look into the metadata check failure. Adding @kcudnik

I had this passing but there was an update in master that I picked up, which is causing this failure. It looks because how I merged the updates:

This is the error I see:
WARNING: Both enums have the same value SAI_API_ICMP_ECHO and SAI_API_PREFIX_COMPRESSION = 0x00000052
WARNING: Both enums have the same value SAI_OBJECT_TYPE_ICMP_ECHO_SESSION and SAI_OBJECT_TYPE_PREFIX_COMPRESSION_TABLE = 0x00000111
processing commit 5eaebb9
ERROR: check ! SAI_API_PREFIX_COMPRESSION value is 0x00000053, but on was 0x00000052 on commit 1ee867c
ERROR: check ! SAI_OBJECT_TYPE_PREFIX_COMPRESSION_TABLE value is 0x00000112, but on was 0x00000111 on commit 1ee867c
ERROR: check ! SAI_OBJECT_TYPE_PREFIX_COMPRESSION_ENTRY value is 0x00000113, but on was 0x00000112 on commit 1ee867c
processing commit 12c952e
ERROR: please correct all 3 error(s) and all 2 warnings before continue
make: *** [Makefile:90: all] Error 1

So yes it was originally the SAI_API value was 52, but a commit went in that made me go and update my code to be 53. The same with SAI_OBJECT. What I have now is correct.

kcudnik · 2024-07-19T07:28:03Z

You need to squash changes and force push instead to merge

nadershinouda · 2024-07-19T13:15:05Z

You need to squash changes and force push instead to merge

@kcudnik, how do I do that now that I already merged the changes? Is there an easy way to do that or do I need to revert the changes, then do as you suggested?

kcudnik · 2024-07-19T14:29:49Z

You can rebase locally and then force push to this branch

nadershinouda · 2024-07-19T19:00:37Z

You can rebase locally and then force push to this branch

thank you

rck-innovium · 2024-07-25T16:05:59Z

inc/saiprefixcompression.h

+     * @flags MANDATORY_ON_CREATE | CREATE_AND_SET
+     */
+    SAI_PREFIX_COMPRESSION_ENTRY_ATTR_BINCODE = SAI_PREFIX_COMPRESSION_ENTRY_ATTR_START,
+


We should have an attribute to let user specify the priority of the prefix compression entry, say

SAI_PREFIX_COMPRESSION_ENTRY_ATTR_PRIORITY

A priority does not apply in this feature. the concept of this feature is having a longest prefix entry added. The prefix table that is created is based on the longest prefix match. When creating this table if it is designed with the concept of longest prefix entries then that longest prefix would always match.

We usually need a priority when we deal with TCAM, if a vendor plans to implement this using TCAM then this feature does not add value and ACLs could be used instead.

In the discussion, the idea was that PREFIX compression is generic, and that ACL table is one type of consumer of the compressed metadata. So, the assumption is more objects will be using the SAI_PREFIX_COMPRESSION_ENTRY_ATTR_META and it could be different from ACL meta.

We should add the PRIORITY attribute, and LPM based implementations should return NOT_SUPPORTED in the sai_capability_query() for this attribute. I don't see why we should not keep it generic.

Yes this feature is generic but generic with the use of LPM capabilities to optimize the correlation between a LPM address and a meta value. The entries are a LPM entry. Can you give a situation where you would want a more specific entry to take place over a longest prefix entry? And why would we do that?

If the intention of the feature is not to use LPM then you could use ACLs instead to match on a specific set of entries with a specific priority. This feature is really intended to allow the consolidation of addresses within a single LPM address. Any other feature that plans to use these tables would be a feature that would benefit from a LPM match and not an ACL style match

rck-innovium · 2024-07-25T16:07:32Z

inc/saiprefixcompression.h

+     * @type sai_uint32_t
+     * @flags MANDATORY_ON_CREATE | CREATE_AND_SET
+     */
+    SAI_PREFIX_COMPRESSION_ENTRY_ATTR_BINCODE = SAI_PREFIX_COMPRESSION_ENTRY_ATTR_START,


It would be better to use META instead of BINCODE to align with other existing SAI attributes.
SAI_PREFIX_COMPRESSION_ENTRY_ATTR_USER_META

I don't mind changing this to META, the reason I used BINCODE because it would be a unique name and won't get confused of the meta in ACL. I don't mind discussing this and if this is a strong ask, then I can change it.

BINCODE is NOT aligning to the terms already used in SAI. Today SAI already uses META for the different qualifiers (route, neighbor, FDB etc) that can be matched in ACL and disambiguates them well. For e.g., SAI_ACL_ENTRY_ATTR_FIELD_SRC_PREFIX_COMPRESSION_META clearly disambiguates it from other META that can be matched in the ACL table.

I have no problem with that, I can change all BINCODE to META in this PR. Thank you for pointing that out, that is a good point. I will try to have the changes done in the next two days

I pushed a change to replace BINCODE with META. Let me know if I can resolve this comment

@rck-innovium can you please resolve this if you are ok with the changes I pushed up

rck-innovium · 2024-07-25T16:10:47Z

inc/saiprefixcompression.h

+     * @type sai_uint32_t
+     * @flags MANDATORY_ON_CREATE | CREATE_AND_SET
+     */
+    SAI_PREFIX_COMPRESSION_TABLE_ATTR_DEFAULT_ENTRY_BINCODE,


Can't this be modelled by creating a prefix entry with a prefix length of 0?

This attribute allows the user to specific what Bincode they want to associate with the default entry which is a pefix/mask of zero. The default BINCODE value does not have to be zero just the prefix/mask value.

Per discussion today will make changes as requested. I will remove this attribute

@rck-innovium I removed the default entry attribute, please close if you are ok with the changes

rck-innovium · 2024-07-25T16:13:41Z

inc/saiprefixcompression.h

+    SAI_PREFIX_COMPRESSION_TABLE_ATTR_DEFAULT_ENTRY_BINCODE,
+
+    /**
+     * @brief End of attributes


Need the below two attributes:

SAI_PREFIX_COMPRESSION_TABLE_ATTR_TYPE to distinguish between Source IP and Destination IP address

SAI_PREFIX_COMPRESSION_TABLE_ATTR_ACL_STAGE of type sai_acl_stage_t

For the comment below:

We should not have indicate an attribute for source ip or destination IP. From creating the actual prefix tables, they should not be labeled on creation. The user can give them a unique label. The user can decide to mix source/destination values. In ACL the distinction is made because at that point we would need to know if we are matching on source address/destination. However, during table creation there is no value to add that distinction.

I think it is important to seperate the feature Prefix compression from other features that use it. As a prefix compression feature, the main purpose of it is to create tables of prefixes based on longest prefix match. There are no rules on how you create these tables. The issue comes in which feature uses these tables. If we are using these tables with ACL, then ACL would need to know which one is Source IP/DST IP and stage. ACL as a feature has these requirements. Another feature in the future that might use prefix tables, might not care.

There could be value to this, currently when you apply a table to ACL you would indicate the stage, however, I am open to discussing this next time we can bring up this topic in a meeting.

As discussed in meeting today:

Will wait to see if @rck-innovium has a use case that will require this

Will add the stage to the table

@rck-innovium I added stage to table creation (ingress/egress), please close if you are ok with the changes

I am ok with using a new sai_prefix_compression_stage_t instead of sai_acl_stage_t. Prefix compression is generic and can be used by features other than ACL.

I still believe we need SAI_PREFIX_COMPRESSION_TABLE_ATTR_TYPE with values Source, Destination and Both. This lets the SAI application query the enum capability and use accordingly.

Is there a use case in your mind that would require this? since the feature that would be using prefix compression would indicate which table type it would expect. it would be up to the user to pass in the correct information.

I don't mind adding this as a topic that we need to revisit and discuss

Simple use-case is when the grouping of IP addresses is different for Source IP vs Dest IP. A quick example below where DIP grouping is 10.1.X.X, and SIP grouping is different for 10.1.1.X from 10.1.2.X.

SIP DIP L4-Dst-Port Syn Action

* 10.1.X.X 80 True Permit

* 10.1.X.X 90 True Permit

* 10.1.X.X * True Drop

10.1.1.X 100.X.X.X 80 True Allow

10.1.1.X 100.X.X.X 90 True Allow

10.1.2.X 200.X.X.X 80 True Allow

10.1.2.X 200.X.X.X 90 True Allow

There are multiple possible implementations for assigning the meta-data. In one implementation, the hardware uses the same packet metadata bits to store both the Source IP metadata and the Dest IP metadata. In another implementation, the hardware can use separate set of metadata bits to store the Source IP metadata, and another set of bits for the Dest IP metadata.

Having a SAI_PREFIX_COMPRESSION_TABLE_ATTR_TYPE allows users to identify the supported type(s) using SAI enum capability query.

would this work:

create prefix compression table

add label SIP

direction ingress

Create entry for table SIP

Add entry 10.1.1.0/24 --> meta value 10

Add entry 10.1.2.0/24 --> meta value 20

Create Prefix compression table

Add label DIP

Add direction Ingress

Add entries to table called DIP

Entry: 10.1.0.0/16 --> meta 30

Entry: 100.0.0.0/8 --> meta 40

Entry: 200.0.0.0/8 --> meata 50

Create ACL TABLE

Table Attribute: SAI_ACL_TABLE_ATTR_SRC_PREFIX_COMPRESSION_TABLE = SIP Table

Table Attribue: SAI_ACL_TABLE_ATTR_DST_PREFIX_COMPRESSION_TABLE = DIP Table

Field support: SAI_ACL_TABLE_ATTR_FIELD_SRC_PREFIX_BINCODE

Field support: SAI_ACL_TABLE_ATTR_FIELD_DST_PREFIX_BINCODE

Add entry match on SAI_ACL_ENTRY_ATTR_FIELD_SRC_PREFIX_BINCODE = 10 (Add action of forward)

Add entry match on SAI_ACL_ENTRY_ATTR_FIELD_DST_PREFIX_BINCODE = 30 (Add action of drop)

When adding the ACL entries if you don't specify a priority then it is up to the vendor on how it is implemented. If you wanted to guarantee that the drop happens before the forward then I would add a higher priority on the drop rule.

I also want to add two points:

When creating the Prefix Table we are using LPM so if multiple entries with the same subnet will be treated the same. In the rule you gave above for SIP you would probably want to add only one entry which is 10.1.x.x/16 and the same for the DST.

in ACL these are treated differently we are not matching on both SRC and DST in a single entry. you are only matching on a source address or destination LPM address in a single entry

@rck-innovium / @helloanandhi I added a table type to table attribute. I believe this meets all the changes that were requested. Can you please let me know if you have any other requests, if not can you please approve

There need to be is_source flag on this attribute

rck-innovium · 2024-07-25T16:19:53Z

inc/saiacl.h

+    /**
+     * @brief SRC BINCODE
+     *
+     * @type sai_acl_field_data_t sai_uint32_t


To make reference counting easier, will it be better to model this as: * @type sai_acl_field_data_t sai_object_id_t of type sai_prefix_compression_entry

I don't think this needs to be an entry, this is a bincode(META) value that we are sending in to ACL to match on. A prefix compression entry is the actual key that would be used. We want to match on the result of the key. So we should match on either BINCODE or META if we decide to go with that name.

The reason we are not passing in the entry structure here, because the BINCODE or META is not part of that entry. This is similar to sairoute.cpp. in SAI route we have a route entry and the result of that entry is the next hop.

if this is still confusing we can discuss it next time we have a meeting about this or let me know and I can try to give a better explanation.

@rck-innovium Can you please resolve this since we are not going to make this change as discussed in today's meeting

rck-innovium · 2024-07-25T16:20:38Z

inc/saiacl.h

+     * @flags CREATE_AND_SET
+     * @default disabled
+     */
+    SAI_ACL_ENTRY_ATTR_FIELD_SRC_PREFIX_BINCODE = SAI_ACL_ENTRY_ATTR_FIELD_START + 0x160,


Please rename to SAI_ACL_ENTRY_ATTR_FIELD_SRC_PREFIX_META

I pushed a change to replace BINCODE with META. Let me know if I can resolve this comment

@rck-innovium Can you please close this, based on my latest commit of switch BINCODE to META

rck-innovium · 2024-07-25T16:22:41Z

doc/SAI-Proposal-Prefix-Compression.md

+    sai_entries_attribute_list,
+    SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR,
+    bulk_status);
+


Please consider including the ACL table and ACL entry usage of the prefix table and prefix entry respectively.

I created a separate document in ACL on ACL using this feature. However, this feature standalone does not depend on ACL. For that reason we should not be listing an example of it in this part of the documenations.

I want prefix compression to be though off as a separate feature than ACL. This is a standalone feature that other features can use. In this case I am using it with ACL, in the future there might be another use of it with another feature.

please refer to [doc/ACL/SAI-Proposal-ACL-Bincode.md]

@rck-innovium, I updated the documentation. I combined both in a single document. Please let me know if this meets your request and resolve if it does

helloanandhi · 2024-07-25T17:01:38Z

doc/SAI-Proposal-Prefix-Compression.md

+
+## 1.0  Introduction ##
+
+This spec adds Prefix Compression. Prefix Compression allows mapping an IP prefix/mask to a bincode. These prefix/bincode mapping can be grouped together to form prefix compression table.  Tables can have both IPV4 and IPV6 entries.


Consider modelling this as a generic specialized table, to assign meta data (aka bincode). This table should have stage and other qualification parameters (like prefix, SIP, DIP etc in this case) as an attribute. Depending on these attributes the generic table entries needs to be configured.
This provides means to extension of the hardware based tables to be extended to other packet parameters if needed in future.

The point of prefix compression is to create a key/value lookup based on longest prefix match. I am not sure what you mean but adding other qualification params. The entry is the prefix which is always the longest prefix and the table has to be created with that in mind. The result of the lookup is a unique code. In this case I am calling bincode but can discuss about changing it to META if needed based on other comments from the PR.

In terms of stage, i would like to discuss that in the next meeting where I have an opening. I am open to that, if we feel there is a benefit. As a standalone feature the stage gets added with ACL when we apply it to ACL, ACL would indicate which stage it is using that table. But again I am open to adding a stage to it

@rck-innovium, I updated the documentation. I combined both in a single document. Please let me know if this meets your request and resolve if it does

helloanandhi · 2024-07-25T17:06:20Z

doc/ACL/SAI-Proposal-ACL-Bincode.md

+     * @flags CREATE_AND_SET
+     * @default disabled
+     */
+    SAI_ACL_ENTRY_ATTR_FIELD_DST_PREFIX_BINCODE,


ACL qualification in your case can be either or DST_PREFIX_BINCODE or SRC_PREFIX_BINCODE or can both be qualified simultaneously ? If former is true a generic name like META_DATA should suffice.

@rck-innovium, I updated the documentation. I combined both in a single document. Please let me know if this meets your request and resolve if it does

helloanandhi · 2024-07-25T17:07:39Z

doc/SAI-Proposal-Prefix-Compression.md

+Prefix Compression tables can be used in features such as ACL to match on a specific bincode. This allows additional functionality to ACL or any feature that can take advantage of such groupings.
+
+## 1.1.0 Function Requirement of Prefix Compression
+- Enable the creation of a prefix compresison table as a SAI object


SAI infra already has META data assignment in multiple hardware entity like FDB, ROUTE entry etc, Do review them and see if you can add an additional attribute to provide a hint to program the prefix compression table instead. This would minimize the changes in your SAI modelling.

I looked at the specs and we should not be touching ROUTE because it will affect the forwarding path. This is unique because it allows the user to create a table based on longest prefix and use that output in other features for lookup. In this case I am using it with ACL but in the future there could be another feature that benefit from this.

I do agree any time we can minimize SAI changes it would be great, but this is a unique use case and it will add new functionality to SAI. In this case it will add functionality to ACL but again in the future we might find another feature to use this.

@rck-innovium, I updated the documentation. I combined both in a single document. Please let me know if this meets your request and resolve if it does

I still don't understand the explanation why we can't reuse existing ROUTE Meta data.

Today one can assign a META data to any route/FDB/.. entry and it won't affect forwarding path. Later this metadata can be used in other features for additional lookup.

Why proposed feature is different?
What functionality SAI will miss if it will continue to be modeled as a META data in ROUTE entry?

@ilyav11

SAI_ROUTE_ENTRY_ATTR_META_DATA cannot be assigned based on Source IP address. Similarly, FDB metadata cannot be assigned based on the Source MAC address.

For a given packet, a single route entry is matched based on LPM. So, we cannot have separate route entries for forwarding and meta-data.

Can you plz elaborate about (2)?
Do you mean that this table:

is not used for forwarding, but only to match on pattern that is e.g. SIP/DIP and set meta data - similar to ACL?

How match in this table done?

@ilyav11

No it is not used for forwarding but a feature that would need to do a lookup based on a LPM. You can do this in ACL today but it will require lots of entries. This allows a customer lets say has a finance pool of addresses, HR pool of addresses and printers pool of addresses. You can use an LPM address for finance, one for HR and one for Printers as an example to represent each department. If you are using ACL then you can easily decide with meta value can access other meta values. Each can be done in a single entry

This table can be done in HW or SW depending on how the vendor supports. This table would be beneficial for any feature that could take advantage of this LPM mapping to a meta value

We are having a meeting tomorrow to discuss this. Please me know if you want to be included. I can have @tjchadaga add you to the meeting.

helloanandhi · 2024-07-29T08:35:39Z

inc/saiprefixcompression.h

+    /**
+     * @brief IP Prefix Destination
+     */
+    sai_ip_prefix_t prefix;


VRF is not being considered here. Is this intentional ?

Yes. Prefix Compression is a table of longest prefix match. The entries in the table should indicate the long prefix which are mapped to a unique value. This should not be confused with sairoute.cpp or such features. This has to standalone outside the forwarding table in order not to affect the forwarding path.

@helloanandhi, do you still want to discuss this after the discussion we had about this feature on 8/8/2024 meeting?

@nadershinouda, these bincodes are applied only to routed packets or also to layer 2 bridged packets ?

@helloanandhi This is only dealing with L3 routed packets. The table entries are prefix/mask of addresses that get mapped to a meta (bincode) value. The idea is to add longest prefix addresses as entries to this table, that get mapped to this meta value. Packets would need to have either IPV4 address or IPV6 address for ACL to be able to match on the meta value

If this is not clear I don't mind adding it to the list of things that we need to discuss.

@helloanandhi can I resolve this conversation?

I think the question is not whether the packet is IP or non-IP, rather more on the forwarding operation. Let us say, the switch receives an IP packet with DIP as 10.1.1.3 on a VLAN. The DMAC of the packet does not match the router's MAC and the packet will get bridged.
Now a prefix-compression entry for 10.1.1.x to assign meta-data X should still apply for this packet and meta-data X should be associated with the bridged packet. Is this true?

the packet would be dropped before it reaches your ACL, in that case the packet will not be affected. If the packet reaches the ACL for any reason, prefix compression only applies to IPV4/IPV6 rules, so it would not apply to this packet.

I can request for time next week for us to discuss this if needed. Might be easier over a meeting than on this review

Hi Nader,

Can you please explicitly clarify whether the below statements are true.

"Prefix compression applies to a packet with IP header irrespective of whether the packet undergoes L2 forwarding or L3 forwarding. For e.g., a packet with IP header that undergoes bridging operation, will still get the metadata assigned by the prefix compression table".

I am not sure if I understand the question but I will attempt to answer it in the terms of ACL:

if you send the following packet:

ETHERNET|VLAN-TAG|IP|

Create a prefix compression table that includes longest prefix match of the IP of the packet. Create an ACL match on that meta value. Then yes it would match.

What type of forwarding (L2/L3) you do does not affect the Prefix compression table. it is based on the feature that will be using that table

rck-innovium · 2024-07-30T04:58:36Z

doc/SAI-Proposal-Prefix-Compression.md

@@ -0,0 +1,271 @@
+#  Prefix Compression #
+-------------------------------------------------------------------------------


Please consider merging both the documents. One doc explains how the meta-data is set, and the other doc explains how it is used. Each doc is incomplete on its own.

The reason I made two documents because Prefix Compression should be a standalone feature. In this case I am using the feature for ACL and added extensions in ACL.

The first document explains how Prefix Compression works as a feature. The feature allows adding entries of longest prefix and mapping those to a unique META value.

The second document is an ACL specific document that explains how ACL would use that feature. In that document i do give a full example of creating a table/entry, and applying it to ACL

I want prefix compression not to be though of as an extension to ACL. It is a separate feature that ACL uses. Similar to other features that ACL uses like mirroring or sample packet. Sample packet stand alone is a feature but it can be used with ACL. Sample packet has its own documentation that is not associated with ACL.

@rck-innovium, I updated the documentation. I combined both in a single document. Please let me know if this meets your request and resolve if it does

nadershinouda · 2024-09-19T15:12:30Z

@rck-innovium and @helloanandhi, I made all the changes that you asked for. Can you please look over my latest changes and approve if you are don't have anymore comments.

rck-innovium · 2024-10-07T14:59:50Z

doc/SAI-Proposal-Prefix-Compression.md

+## 2.0 Specification ##
+
+A prefix compression tables is represented by an object of SAI_PREFIX_COMPRESSION_TABLE. The creation of this object will allocate an empty table that can be used later to add specific entries to it. Table creation requires a stage to be associated with the table and also requires the type of table that is getting created: source addresses, destionation addresses or both.
+


Please describe the changes to saiacl.h.

rlhui · 2024-10-07T15:49:00Z

There is only one .md file in this PR so far. I don't see "SAI-Proposal-ACL-Bincode.md" as mentioned in the PR description.

rck-innovium · 2024-10-07T15:53:41Z

inc/saiprefixcompression.h

+     *
+     * @type sai_prefix_compression_type_t
+     * @flags MANDATORY_ON_CREATE | CREATE_ONLY
+     */


pls add @isresource

rck-innovium · 2024-10-07T16:02:34Z

doc/SAI-Proposal-Prefix-Compression.md

+sai_remove_prefix_compression_table_fn(
+    src_prefix_compression_table_id);
+
+```


Please consider adding the workflow to identify the platforms of the below types using sai_object_type_get_availability() and using filters on SAI_PREFIX_COMPRESSION_TYPE:

A single compression table of type SAI_PREFIX_COMPRESSION_TYPE_BOTH,

A compression table or type SAI_PREFIX_COMPRESSION_TYPE_SRC and another SAI_PREFIX_COMPRESSION_TYPE_DST

nadershinouda · 2024-11-07T14:47:15Z

@JaiOCP when you get a chance can you review the updated document and give your feedback or approve

JaiOCP · 2024-11-08T19:46:57Z

doc/SAI-Proposal-Prefix-Compression.md

+When performing meta-data entry lookup in an ACL, the prefix compression object ID is retrieved from the ACL table and dereferenced for a separate lookup in the prefix compression table to obtain the meta-data. Ideally, this lookup is performed in hardware and occurs simultaneously with the ACL table lookup.
+
+Example: Two prefix compression tables, Table_1 and Table_2, are created. ACL Table_1 is then configured with the attributes SAI_ACL_TABLE_ATTR_SRC_PREFIX_COMPRESSION_TABLE set to Prefix Compression Table_1 and SAI_ACL_TABLE_ATTR_DST_PREFIX_COMPRESSION_TABLE set to Prefix Compression Table_2. Under ACL Table_1, two entries are created. Entry_1 references meta-data from Prefix Compression Table_1, and Entry_2 references meta-data from Prefix Compression Table_2. During the pipeline process for Entry_1, the ID of Prefix Compression Table_1 is dereferenced from ACL Table_1, and a lookup is performed on Prefix Compression Table_1, which is a separate table in the pipeline. The result of this lookup determines whether Entry_1 hits or misses in the ACL. The same process is applied to Entry_2, but the lookup is performed in Prefix Compression Table_2.
+


Just for my understanding.
In your example
ACL_Table1 points to SRC_PREFIX_COMPRESSION_TABLE
ACL_Table1->Entry has match on the result of SRC_PREFIX_COMPRESSION_TABLE as well

Now pipeline is something like this.
packet derives the associated compression table based on the ACL_Table1 configured. Note there is no match criteria to derive the src compression table i.e. it is not an action.
Subsequently lookup is performed on the src compression table and result is some "metadata".
Next "metadata is used as match criteria in ACL_Table1 and result may be miss or hit. Normal ACL actions of hit or miss will take effect.

Is this understanding correct?

Yes that is correct. If you feel my wording is not clear enough, I can tweak that part to make it better. Maybe the way you described it provides a better picture.

Yes, if you can capture the language so that pipeline behavior is clear it will be very helpful later on.

I have one more comment. In any pipeline typically you don't derive entire table from table, it is usually an entry into another table based on some lookup criteria.

Having said that, I don't see this as an issue, since all you are doing is leveraging the stages of ACL tables/groups. In hardware truly speaking there is no such derivation from the ACL tables and there is an implicit pipeline behavior such that prefix compression table is always before the ACL table. This is for the reason that metadata from compression table is looked up in the ACL table.

Can you please confirm this understanding as well.

Yes, I updated the docs:

Moved that section to a new section to make it clear

Used some of your verbiage to make the wording clear

I hope that is better but I don't mind giving it another try. I know this is something new and I want to make sure it is clear for future users

Thanks, Can you please capture following in your own words as well
....
I have one more comment. In any pipeline typically you don't derive entire table from table, it is usually an entry into another table based on some lookup criteria.

Having said that, I don't see this as an issue, since all you are doing is leveraging the stages of ACL tables/groups. In hardware truly speaking there is no such derivation from the ACL tables and there is an implicit pipeline behavior such that prefix compression table is always before the ACL table. This is for the reason that metadata from compression table is looked up in the ACL table.

...

@JaiOCP, not a problem at all. Again please free free to point out anything, this has to be clear for future users. Especially since this has not been done before. Thank you for your help with this

Thanks. Approved the PR

Signed-off-by: nshinoud <nshinoud@cisco.com> Change BINCODE to META - PR change request Signed-off-by: nshinoud <nshinoud@cisco.com> Additional changes based on requests from git review - Removed default meta attribute - Added stage for table creation - updated docs Signed-off-by: nshinoud <nshinoud@cisco.com> Fix typo: Changes SAI_ACL_TABLE_ATTR_PREFIX_COMPRESSION_STAGE to SAI_TABLE_ATTR_PREFIX_COMPRESSION_STAGE Signed-off-by: nshinoud <nshinoud@cisco.com> Fix metadata script error Signed-off-by: nshinoud <nshinoud@cisco.com> Another attempt to fix metadata issue. For some reason this passes on my local build Signed-off-by: nshinoud <nshinoud@cisco.com> Add support for Prefix Compression Table type: SRC, DST, BOTH Signed-off-by: nshinoud <nshinoud@cisco.com> Minor fix (change bincode to meta) Signed-off-by: nshinoud <nshinoud@cisco.com> Update comment Signed-off-by: nshinoud <nshinoud@cisco.com>

Signed-off-by: nshinoud <nshinoud@cisco.com>

nadershinouda force-pushed the master branch from d379320 to 1ee867c Compare July 8, 2024 20:13

nadershinouda marked this pull request as ready for review July 11, 2024 17:44

nadershinouda force-pushed the master branch from 5eaebb9 to bdf9e01 Compare July 19, 2024 16:20

rck-innovium suggested changes Jul 25, 2024

View reviewed changes

helloanandhi reviewed Jul 25, 2024

View reviewed changes

helloanandhi reviewed Jul 29, 2024

View reviewed changes

rck-innovium reviewed Jul 30, 2024

View reviewed changes

nadershinouda force-pushed the master branch from cc9aa4c to 2afbc1c Compare August 13, 2024 18:26

nadershinouda force-pushed the master branch 2 times, most recently from a862dc8 to 3cbb5d9 Compare September 16, 2024 22:47

rlhui added the reviewed PR is discussed in SAI Meeting label Sep 19, 2024

rck-innovium reviewed Oct 7, 2024

View reviewed changes

rck-innovium approved these changes Oct 7, 2024

View reviewed changes

tjchadaga requested a review from kcudnik October 7, 2024 16:15

nadershinouda force-pushed the master branch 2 times, most recently from b07decb to 6715150 Compare November 1, 2024 15:20

JaiOCP reviewed Nov 8, 2024

View reviewed changes

nadershinouda added 3 commits November 8, 2024 17:08

PR changes: Update an attribute and documentation

2f20ac1

Signed-off-by: nshinoud <nshinoud@cisco.com>

Update Docs: Packet pipeline

49101b8

Signed-off-by: nshinoud <nshinoud@cisco.com>

nadershinouda force-pushed the master branch from 28ddbad to 49101b8 Compare November 8, 2024 22:09

Update Prefix Compression Documentation: Pipeline

c2c0a22

Signed-off-by: nshinoud <nshinoud@cisco.com>

JaiOCP approved these changes Nov 8, 2024

View reviewed changes

kcudnik approved these changes Nov 9, 2024

View reviewed changes

SIP	DIP	L4-Dst-Port	Syn	Action
*	10.1.X.X	80	True	Permit
*	10.1.X.X	90	True	Permit
*	10.1.X.X	*	True	Drop
10.1.1.X	100.X.X.X	80	True	Allow
10.1.1.X	100.X.X.X	90	True	Allow
10.1.2.X	200.X.X.X	80	True	Allow
10.1.2.X	200.X.X.X	90	True	Allow


		## 1.0 Introduction ##

		This spec adds Prefix Compression. Prefix Compression allows mapping an IP prefix/mask to a bincode. These prefix/bincode mapping can be grouped together to form prefix compression table. Tables can have both IPV4 and IPV6 entries.

		@@ -0,0 +1,271 @@
		# Prefix Compression #
		-------------------------------------------------------------------------------

		## 2.0 Specification ##

		A prefix compression tables is represented by an object of SAI_PREFIX_COMPRESSION_TABLE. The creation of this object will allocate an empty table that can be used later to add specific entries to it. Table creation requires a stage to be associated with the table and also requires the type of table that is getting created: source addresses, destionation addresses or both.

		When performing meta-data entry lookup in an ACL, the prefix compression object ID is retrieved from the ACL table and dereferenced for a separate lookup in the prefix compression table to obtain the meta-data. Ideally, this lookup is performed in hardware and occurs simultaneously with the ACL table lookup.

		Example: Two prefix compression tables, Table_1 and Table_2, are created. ACL Table_1 is then configured with the attributes SAI_ACL_TABLE_ATTR_SRC_PREFIX_COMPRESSION_TABLE set to Prefix Compression Table_1 and SAI_ACL_TABLE_ATTR_DST_PREFIX_COMPRESSION_TABLE set to Prefix Compression Table_2. Under ACL Table_1, two entries are created. Entry_1 references meta-data from Prefix Compression Table_1, and Entry_2 references meta-data from Prefix Compression Table_2. During the pipeline process for Entry_1, the ID of Prefix Compression Table_1 is dereferenced from ACL Table_1, and a lookup is performed on Prefix Compression Table_1, which is a separate table in the pipeline. The result of this lookup determines whether Entry_1 hits or misses in the ACL. The same process is applied to Entry_2, but the lookup is performed in Prefix Compression Table_2.

Prefix Compression feature addition to SAI #2045

Are you sure you want to change the base?

Prefix Compression feature addition to SAI #2045

Conversation

nadershinouda commented Jul 8, 2024

tjchadaga commented Jul 18, 2024

nadershinouda commented Jul 18, 2024

kcudnik commented Jul 19, 2024

nadershinouda commented Jul 19, 2024

kcudnik commented Jul 19, 2024

nadershinouda commented Jul 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadershinouda Sep 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadershinouda Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadershinouda commented Sep 19, 2024

Choose a reason for hiding this comment

rlhui commented Oct 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadershinouda commented Nov 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadershinouda Sep 22, 2024 •

edited

Loading

nadershinouda Aug 27, 2024 •

edited

Loading