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

[dash-p4] Add tunnel member table and tunnel next hop table. #622

Merged
merged 9 commits into from
Oct 22, 2024

Conversation

r12f
Copy link
Collaborator

@r12f r12f commented Sep 28, 2024

To support the ECMP group in the tunnel, this change adds the tunnel member and tunnel next hop tables using the same way as how next hop group works today in SAI.

The detailed design is following the HLD defined here: https://github.com/sonic-net/DASH/blob/main/documentation/private-link-service/private-link-service.md.

@r12f
Copy link
Collaborator Author

r12f commented Sep 28, 2024

hi @prsunny , here is the preview of the tunnel member and next hop change. please let me know, if you have any concerns. The API should be ok, and I will get the P4 part a bit more stable in the meantime.

Copy link
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mukeshmv
Copy link
Collaborator

mukeshmv commented Oct 1, 2024

@r12f , @prsunny this ECMP API design with tunnel nexthops and tunnel members being pushed iteratively is very hard to implement in the pipeline. Ideally we need to be able to pre-allocate the resources for each ECMP group when the group is created.

@r12f
Copy link
Collaborator Author

r12f commented Oct 1, 2024

hi @mukeshmv , the tunnel design follows exactly the same pattern as the current SAI next hop group and next hop design. If preallocation is required, we can ensure the tunnel next hops are all created before bind the tunnel to any mappings. And after that, the next hop list will not be change. This should solve the problem.

However, we do have case there tunnel next hops being changed, say a destination being removed. Without certain support, this can fundamentally break certain scenarios. So let's touch base offline and see how to get this supported.

@r12f
Copy link
Collaborator Author

r12f commented Oct 2, 2024

Actually, after second thought, this might be related to the hash distribution thing, not the actual element count. Adding a size should help. Yea, we can add that. I will push a new PR late today.

@KrisNey-MSFT
Copy link
Collaborator

Looks ready to merge

@mukeshmv
Copy link
Collaborator

mukeshmv commented Oct 8, 2024

@prsunny Can you please specify the scale parameters for these objects in the HLD
number of DASH_TUNNEL objects,
number of members per DASH_TUNNEL object.

@prsunny
Copy link
Collaborator

prsunny commented Oct 8, 2024

@prsunny Can you please specify the scale parameters for these objects in the HLD number of DASH_TUNNEL objects, number of members per DASH_TUNNEL object.

@mukeshmv , the following are the scale we are thinking:

  1. DASH_TUNNEL - 1k per dpu
  2. MEMBER scale - 128

Will update the sonic-hld for this scale numbers

Copy link
Collaborator

@mukeshmv mukeshmv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@prsunny prsunny merged commit 8dd75ce into sonic-net:main Oct 22, 2024
5 checks passed
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.

4 participants