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] Update ACL tables type and match type annotations for SAI header generation #486

Merged
merged 6 commits into from
Dec 20, 2023

Conversation

r12f
Copy link
Collaborator

@r12f r12f commented Dec 14, 2023

This PR changes 3 things:

  1. Update the type annotation of ACL table match keys, so we can generate the right types for ACLs.
  2. Add match_type annotation for match keys, so we can fix the optional match kind that we use to make P4 compiler happy.
  3. Update SAI header generation script to handle the new annotation.

The end results is shown as below. The comment and type are now updated to the same as opencompute SAI:

image
(no changes showing up in git diff)

To compare with previous generated content:

  • Header files:

    r12f@r12f-dl380:~/data/code/sonic/DASH/dash-pipeline
    $ diff SAI/SAI/experimental/ ~/data/code/sonic/DASH-exp/dash-pipeline/SAI/SAI/experimental/
    diff SAI/SAI/experimental/saiexperimentaldashacl.h /home/r12f/data/code/sonic/DASH-exp/dash-pipeline/SAI/SAI/experimental/saiexperimentaldashacl.h
    114c114
    <      * @brief List matched key dip
    ---
    >      * @brief Optional matched key dip
    ...

    image

  • libsai (creating adapter that converts list / range list to optional for bmv2):

    image

@r12f
Copy link
Collaborator Author

r12f commented Dec 14, 2023

PR is not ready for review. Need to discuss what is the right way for libsai changes. My feeling is to convert the input to optional, because this is what BMv2 supports.

@r12f
Copy link
Collaborator Author

r12f commented Dec 14, 2023

discussed in today's DASH BM meeting today. For BMv2, we will convert the ACL input to optional by taking the first item of the list.

@r12f
Copy link
Collaborator Author

r12f commented Dec 19, 2023

OK, the tests are fixed now. this PR is ready for review. @marian-pritsak @kcudnik

@r12f r12f requested a review from chrispsommers December 20, 2023 16:44
@r12f r12f merged commit fb9e205 into sonic-net:main Dec 20, 2023
5 checks passed
@r12f r12f deleted the user/r12f/acl branch December 20, 2023 19:40
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.

3 participants