-
Notifications
You must be signed in to change notification settings - Fork 93
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-Sonic] Introduce route group, pa validation tables #520
Conversation
prsunny
commented
Feb 21, 2024
- Add Route group table for grouping routes and binding to an ENI
- Introduce PA validation table for special validation for certain scenarios
|
||
``` | ||
DASH_ROUTE_GROUP_TABLE:{{group_id}} | ||
"guid": {{string}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still need the guid here? @sharmasushant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
guid can be optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a use for guid field, but I am okay to leave it there.
"DASH_ROUTE_TABLE:F4939FEFC47E:50.1.2.0/24": { | ||
"DASH_ENI_ROUTE_TABLE:F4939FEFC47E": { | ||
"group_id":"group_id_2" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different ENI ids for group_id_1, group_id_2 and group_id_3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a reference example. it can be any group_id
Currently the route table is modified w/routes belonging to an ENI (per ENI). What if libsai has no resources to support an atomic update? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Are all comments resolved? IF Y, I can merge :) |
hi @r12f , @kperumalbfn , and @pranjal-git - would you mind reviewing for Prince please? |
Align DASH API with newest HLD changes: sonic-net/DASH#520 and sonic-net/SONiC#1658 - Route group API changes - Metering implementation changes - PL/PL-NSG - Deprecate/rename some fields for clarity