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

Jump to bottom

Updates to SRv6 programming model and related objects/attributes #1231

Merged
merged 10 commits into from Jun 24, 2021
Merged

Updates to SRv6 programming model and related objects/attributes #1231

merged 10 commits into from Jun 24, 2021

Conversation

ashutosh-agrawal
Copy link
Contributor

@ashutosh-agrawal ashutosh-agrawal commented Apr 30, 2021 •
edited

Changes are listed in the attached md file.

Following items were deferred and will be addressed in subsequent PRs:

  • Additional capability attributes to specify number of SIDs supported by the platform
  • Need for shift-only behavior in case of uSID
  • Consider specifying SID structure in a separate container (object/struct) and reuse as necessary
  • Split the headend behavior from SID list and add it as a separate attribute for nexthop so SID lists can be shared across different behaviors

kcudnik
kcudnik reviewed Apr 30, 2021
View reviewed changes
meta/parse.pl Outdated Show resolved Hide resolved
@ashutosh-agrawal
Initial SRv6 Changes
338fb59 
Signed-off-by: Ashutosh Agrawal <ashutosh.agrawal@intel.com>
@ashutosh-agrawal
Added behavioral model diagrams and example configuration
ab0f580 
Signed-off-by: Ashutosh Agrawal <ashutosh.agrawal@intel.com>
@ashutosh-agrawal
changed default for endpoint_flavor
150e4b1 
Signed-off-by: Ashutosh Agrawal <ashutosh.agrawal@intel.com>
@ashutosh-agrawal
1) Updated reference list in the document 2) Added tunnel_id to local…
21d7047 
…_sid attribute list to get tunnel termination properties like TTL/DSCP mode 3) Changed IPv6 address to IP prefix in Local SID entry based on review feedback

Signed-off-by: Ashutosh Agrawal <ashutosh.agrawal@intel.com>
pcamaril
pcamaril reviewed May 12, 2021
View reviewed changes
inc/saisegmentroute.h Outdated Show resolved Hide resolved
pcamaril
pcamaril reviewed May 12, 2021
View reviewed changes
inc/saisegmentroute.h Outdated Show resolved Hide resolved
@ashutosh-agrawal
Addressed review comments regarding locator length and reference for …
3e74fa8 
…innsertion function

Signed-off-by: Ashutosh Agrawal <ashutosh.agrawal@intel.com>
@rlhui
Copy link
Collaborator

rlhui commented May 18, 2021

@rck-innovium , @marian-pritsak - would you like to review this? Thanks.

@rlhui
Copy link
Collaborator

rlhui commented May 19, 2021

@Israel-Meilik - please help review/approve. Thanks.

@rlhui
Copy link
Collaborator

rlhui commented May 21, 2021

@skraza - please help review, thanks.

skraza
skraza reviewed May 25, 2021
View reviewed changes
inc/saisegmentroute.h Outdated Show resolved Hide resolved
@ashutosh-agrawal
Addressed review comments.
2ee234d 
Signed-off-by: Ashutosh Agrawal <ashutosh.agrawal@intel.com>
caizhenghui-juniper
caizhenghui-juniper reviewed May 26, 2021
View reviewed changes
Copy link

@caizhenghui-juniper caizhenghui-juniper left a comment

Choose a reason for hiding this comment

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

Tunnel id is for encapsulation not decapsulation

@ashutosh-agrawal
Copy link
Contributor Author

Tunnel id is for encapsulation not decapsulation

@caizhenghui-juniper local_sid entry has tunnel id for decap and nexthop entry will have tunnel id for encap. SAI application can use appropriate tunnel_id to set both encap/decap attributes for features like ttl/qos modes.

@caizhenghui-juniper
Copy link

@ashutosh-agrawal ok, got it.

pcamaril
pcamaril approved these changes Jun 3, 2021
View reviewed changes
Copy link

@pcamaril pcamaril left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks.

skraza
skraza approved these changes Jun 8, 2021
View reviewed changes
Copy link

@skraza skraza left a comment

Choose a reason for hiding this comment

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

PR#1 approved. Some comments to be discussed/addressed in PR#2.

Israel-Meilik
Israel-Meilik approved these changes Jun 8, 2021
View reviewed changes
Copy link

@Israel-Meilik Israel-Meilik left a comment

Choose a reason for hiding this comment

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

PR#1 reviewed by Broadcom team and approved.
Thanks.

@ashutosh-agrawal
Added new behaviors to support uSID instruction
02e58d9 
Signed-off-by: Ashutosh Agrawal <ashutosh.agrawal@intel.com>
@ashutosh-agrawal
Renamed segmentroute to srv6 to avoid any confusion with sr-mpls. Als…
2c16a18 
…o renamed local_sid to my_sid based on community feedback.

Signed-off-by: Ashutosh Agrawal <ashutosh.agrawal@intel.com>
@abdosi
Copy link
Collaborator

abdosi commented Jun 15, 2021

Based on community meeting discussion there were 2 major points:

1> No need to address Backward Compatibility for SRv6 since feature has evolve lot when SAI api were design intially.
2> All the TODO enhancements will be done as part of subsequent PR's

There were agreement to approve/merge this PR as there was no outstanding issue.

@abdosi
Copy link
Collaborator

abdosi commented Jun 15, 2021

@ashutosh-agrawal Can you please add details in PR description above. We can merge after that.

@ashutosh-agrawal
Copy link
Contributor Author

@ashutosh-agrawal Can you please add details in PR description above. We can merge after that.

@abdosi In the PR description, I've added a list of items which were deferred (as discussed in the SRv6 WG).

@ashutosh-agrawal
Copy link
Contributor Author

@abdosi Please let me know if anything else needs to be done to get this PR to merge.

@abdosi
Copy link
Collaborator

abdosi commented Jun 24, 2021

thanks . will merge this when PR check passes

@abdosi abdosi merged commit 2e934ff into opencomputeproject:master Jun 24, 2021
@caizhenghui-juniper
Copy link

@ashutosh-agrawal do we have vslib code change which uses old segmentroute sid list in the code before moving the pointer of SAI in sonic to avoid build break?

@zhangyanzhao
Copy link

@abdosi are all the code PRs merged? Can you please update the code PRs by following example sonic-net/SONiC#806?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caizhenghui-juniper caizhenghui-juniper caizhenghui-juniper left review comments

@kcudnik kcudnik kcudnik approved these changes

@pcamaril pcamaril pcamaril approved these changes

@skraza skraza skraza approved these changes

@abdosi abdosi abdosi approved these changes

@Israel-Meilik Israel-Meilik Israel-Meilik approved these changes

@reshmaintel reshmaintel reshmaintel approved these changes

Assignees

@abdosi abdosi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@ashutosh-agrawal @rlhui @caizhenghui-juniper @abdosi @zhangyanzhao @kcudnik @pcamaril @skraza @Israel-Meilik @reshmaintel