Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sai-gen] Move acl table to use SaiTable annotation. #478

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

r12f
Copy link
Collaborator

@r12f r12f commented Dec 12, 2023

This PR moves acl table to use @SaiTable annotation instead of @name.

After the change, SAI headers will be generated exactly the same:

r12f@r12f-dl380:~/data/code/sonic/DASH/dash-pipeline
$ diff SAI/SAI/experimental/ ~/data/code/sonic/DASH-exp/dash-pipeline/SAI/SAI/experimental/

However, due to name is changed in P4, the table id will be changed, but it only affects the libsai generated from DASH for testing, not any providers' implementation.

r12f@r12f-dl380:~/data/code/sonic/DASH/dash-pipeline
$ diff SAI/lib ~/data/code/sonic/DASH-exp/dash-pipeline/SAI/lib              
diff SAI/lib/saidashacl.cpp /home/r12f/data/code/sonic/DASH-exp/dash-pipeline/SAI/lib/saidashacl.cpp
211c211
<     // For stage acl.stage1
---
>     // For stage inbound_acl_stage1
213c213
<     tableId = 41950136;
---
>     tableId = 33901322;
366c366
<     // For stage acl.stage2
---
>     // For stage inbound_acl_stage2
368c368
<     tableId = 43016664;
---
>     tableId = 44184066;
521c521
<     // For stage acl.stage3
---
>     // For stage inbound_acl_stage3
523c523
<     tableId = 49695908;
---
>     tableId = 46150034;
676c676
<     // For stage acl.stage1
---
>     // For stage outbound_acl_stage1
678c678
<     tableId = 49209582;
---
>     tableId = 33810473;
831c831
<     // For stage acl.stage2
---
>     // For stage outbound_acl_stage2
833c833
<     tableId = 36478314;
---
>     tableId = 49812549;
986c986
<     // For stage acl.stage3
---
>     // For stage outbound_acl_stage3
988c988
<     tableId = 39012793;
---
>     tableId = 40782112;

The naming change in p4rt:

$ diff bmv2/dash_pipeline.bmv2/dash_pipeline_p4rt.json ~/data/code/sonic/DASH-exp/dash-pipeline/bmv2/dash_pipeline.bmv2/dash_pipeline_p4rt.json 
8,37c8,10
<     "id": 49209582,
<     "name": "dash_ingress.outbound.acl.stage1",
<     "alias": "outbound.acl.stage1",
<     "structuredAnnotations": [
...
---
>     "id": 33810473,
>     "name": "dash_ingress.outbound.acl.stage1:dash_acl_rule|dash_acl",
>     "alias": "outbound.acl.stage1:dash_acl_rule|dash_acl"

@r12f
Copy link
Collaborator Author

r12f commented Dec 12, 2023

@chrispsommers , I believe ACL is the worst. If this looks good, I am going to move all other tables, which will contain very similar changes as this one.

@r12f
Copy link
Collaborator Author

r12f commented Dec 12, 2023

thanks Kamil! now, I am way more confident with your blessing! :D

@r12f r12f merged commit e6a8e98 into sonic-net:main Dec 12, 2023
4 checks passed
@r12f r12f deleted the user/r12f/acl branch December 12, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants