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

SRv6 VPN HLD for 202305 release #1252

Merged
merged 10 commits into from
Jun 26, 2023

Conversation

eddieruan-alibaba
Copy link
Contributor

@eddieruan-alibaba eddieruan-alibaba commented Feb 3, 2023

SRv6 VPN HLD for 202305 release

The PR would be merged to 202305 release

Repo PR title State
sonic-swss [202305][SRv6]: SRv6 VPN SID #2765 GitHub issue/pull request detail
sonic-swss [202305][SRv6]: SRv6 VPN Unit Tests #2766 GitHub issue/pull request detail
sonic-swss [fpmsyncd] Add support for SRv6 GitHub issue/pull request detail

The PR would be outstanding for 202305 release, a.k.a coming in 202311
These PRs are either missed FRR train or need to finalize in routing WG. The purposes for these PRs are for people to use 202305 for SRv6 VPN PoC.

  • FRR outstanding PRs
Repo PR title State
FRR bgpd: Add support for BGP vrf route copying
FRR zebra: Add the missing Source Addr param for SRv6 Encapsulation
FRR zebra: Extend the FPM module to push the missing SRv6 information
  • SONiC workaround PRs
Repo PR title State
SONiC [add cli(frr) design doc for srv6_vpn] (#1343) GitHub issue/pull request detail

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 3, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@eddieruan-alibaba eddieruan-alibaba mentioned this pull request Feb 3, 2023
@eddieruan-alibaba
Copy link
Contributor Author

@zhangyanzhao
Copy link
Collaborator

Copy link

@reshmaintel reshmaintel left a comment

Choose a reason for hiding this comment

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

In current SRv6 implementation the SRv6Orch creates the nexthop. This is because we did not have SRv6 support in FRR then and assumption is configuration is controller driven path which adds the SRv6 routes into APP_DB. As we integrate the new FRR SRv6 support into SONiC as in this PR, the nexthop/group creation can now be leveraged from the FRR route addition path RouteOrch. SRv6Orch will continue to do other tasks for MY_SID and SRv6 policy -processing,

doc/srv6/srv6_vpn.md Outdated Show resolved Hide resolved
doc/srv6/srv6_vpn.md Outdated Show resolved Hide resolved
doc/srv6/srv6_vpn.md Outdated Show resolved Hide resolved
doc/srv6/srv6_vpn.md Show resolved Hide resolved
doc/srv6/srv6_vpn.md Show resolved Hide resolved
doc/srv6/srv6_vpn.md Show resolved Hide resolved
doc/srv6/srv6_vpn.md Show resolved Hide resolved
doc/srv6/srv6_vpn.md Show resolved Hide resolved
doc/srv6/srv6_vpn.md Show resolved Hide resolved
doc/srv6/srv6_vpn.md Outdated Show resolved Hide resolved
segment = SRV6_SID_LIST.key ; New optional field. List of segment names, separated by ','
seg_src = address ; New optional field. Source addrs for sid encap
vpn_sid = vpn_sid ; New optional field to add vpn_sid to learnt BGP routes
policy = policy ; New optional field to add policy name to learnt BGP routes
Copy link
Contributor

Choose a reason for hiding this comment

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

as mentioned above, I am not sure if policy should really belong to orchagent.. There is no policy notion in orchagent for regular routing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I quote @reshmaintel 's previous comments here "SRv6Orch will continue to do other tasks for MY_SID and SRv6 policy -processing," . The current thought process is to continue with current design. We could discuss this topic in routing WG if you want.

For 202205, we will upstream SRv6 VPN only. Policy will come in the future release.

doc/srv6/srv6_vpn.md Show resolved Hide resolved
@shay-zadok
Copy link
Collaborator

Hi @eddieruan-alibaba,
We would like to review the SBFD
Thanks

reshmaintel
reshmaintel previously approved these changes Feb 28, 2023
@zhangyanzhao
Copy link
Collaborator

@prsunny @shay-zadok @venkatmahalingam @reshmaintel @kafka-soda @svshah-intel @kperumalbfn reviewers, would you please take time to finish your review? It is not mandatory to get approval from all reviewers to merge the PR. Thanks.

@venkatmahalingam
Copy link
Collaborator

@prsunny @shay-zadok @venkatmahalingam @reshmaintel @kafka-soda @svshah-intel @kperumalbfn reviewers, would you please take time to finish your review? It is not mandatory to get approval from all reviewers to merge the PR. Thanks.

Wondering if we need to approve as is or discuss further to understand the workarounds/deviations mentioned in this HLD in the separate work-group and then update this document as needed for final approval.

@zhangyanzhao
Copy link
Collaborator

@eddieruan-alibaba can you please list the code PRs to this HLD by referring to #806

Copy link
Collaborator

@zhangyanzhao zhangyanzhao left a comment

Choose a reason for hiding this comment

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

Please list the code PRs by referring to #806

@zhangyanzhao
Copy link
Collaborator

@eddieruan-alibaba can you please help to add the code PRs to this HLD by referring to #806 ? Thanks.

@eddieruan-alibaba
Copy link
Contributor Author

@eddieruan-alibaba can you please list the code PRs to this HLD by referring to #806

Added. But how to add a url to display PR status here? Any suggestions? @zhangyanzhao

@zhangyanzhao
Copy link
Collaborator

Can this PR be merged? Also please help on the code PR review.

@eddieruan-alibaba
Copy link
Contributor Author

** zhangyanzhao ** requested changes

I saw you added "
zhangyanzhao requested changes"

I have added code PRs in this PR.

@zhangyanzhao
Copy link
Collaborator

@eddieruan-alibaba the FRR PRs need be reviewed in FRR community, are those required by your feature?

@lguohan
Copy link
Contributor

lguohan commented May 26, 2023

@eddieruan-alibaba , do we have sonic-mgmt test plan for this feature?

@eddieruan-alibaba
Copy link
Contributor Author

@eddieruan-alibaba the FRR PRs need be reviewed in FRR community, are those required by your feature?

They have missed the train to go into 202305 release. These PRs could be patched in 202305 for the POC purpose. These FRR PRs would go to FRR community and come in to 202311 once we move FRR ref point for 202311.

@eddieruan-alibaba
Copy link
Contributor Author

@eddieruan-alibaba , do we have sonic-mgmt test plan for this feature?

Hanyu from my team has created a PR for SONiC CLI changes and explained the information to check.
#1343

The headache is that we don't have software or hardware platform for the test. I am working with Praveen from Cisco to see if we could add Cisco C8K as a hardware platform for this feature in public.

@eddieruan-alibaba eddieruan-alibaba merged commit c6e27b1 into sonic-net:master Jun 26, 2023
@zhangyanzhao
Copy link
Collaborator

code PRs are not merged yet

@zhangyanzhao
Copy link
Collaborator

code PRs are not merged. move to backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: MovedToBacklog
Development

Successfully merging this pull request may close these issues.

9 participants