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

North bound interface (CLI/REST clients) support for configuring FRR BGP #544

Merged
merged 12 commits into from
Mar 9, 2021

Conversation

venkatmahalingam
Copy link
Collaborator

  1. Extend and provide unified configuration and management capability for FRR-BGP features used in SONiC.
  2. Allow the user to configure & manage FRR-BGP using SONiC Management Framework with Open Config data models via REST, gNMI and also provides access via SONiC Management Framework CLI as well.

@msftclas
Copy link

msftclas commented Dec 19, 2019

CLA assistant check
All CLA requirements met.

@venkatmahalingam venkatmahalingam changed the title FRR BGP NBI support North bound interface (CLI/REST clients) support configuring FRR BGP Dec 19, 2019
@venkatmahalingam venkatmahalingam changed the title North bound interface (CLI/REST clients) support configuring FRR BGP North bound interface (CLI/REST clients) support for configuring FRR BGP Dec 19, 2019
Copy link

@pavel-shirshov pavel-shirshov left a comment

Choose a reason for hiding this comment

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

  1. Please convert all pictures to .png. jpeg doesn't work good for diagrams

  2. Suggest to split the proposal to two parts: north-bound to DB, DB to SONiC FRR

  3. Suggest to use special utility to generate the database schema from YANG model automatically. Don't write the schema manually.
    .4. Suggest to rewrite bgpcfgd in go

  4. Suggest to introduce validation loop: first northbound interface check syntax rules, then bgpcfgd uses frr to validate generated bgp configuration.

  5. Suggest to return validation errors from bgpcfgd back to the requested. syslog message is not enough

  6. What if some parameter is already configured and there is conflict of something being already in the database. What should we do? overwrite? remove old parameter and create new parameter?

  7. What if some parameters are depends on the parameters which is changed by some rest request. How we can be sure we work with the parameters successfully?

@venkatmahalingam
Copy link
Collaborator Author

venkatmahalingam commented Jan 7, 2020

  1. Please convert all pictures to .png. jpeg doesn't work good for diagrams
    Sure.
  2. Suggest to split the proposal to two parts: north-bound to DB, DB to SONiC FRR
  3. Suggest to use special utility to generate the database schema from YANG model automatically. Don't write the schema manually.
    .4. Suggest to rewrite bgpcfgd in go
  4. Suggest to introduce validation loop: first northbound interface check syntax rules, then bgpcfgd uses frr to validate generated bgp configuration.
  5. Suggest to return validation errors from bgpcfgd back to the requested. syslog message is not enough

Thanks for all your suggestions, will discuss internally and take up if required now.

  1. What if some parameter is already configured and there is conflict of something being already in the database. What should we do? overwrite? remove old parameter and create new parameter?

If there are multiple config sources, it's hard to keep the configs in sync, for now, the short answer is, it is beyond our scope of the work.

  1. What if some parameters are depends on the parameters which is changed by some rest request. How we can be sure we work with the parameters successfully?

We handle the dependencies in CVL layer (mgmt-framework), if there are any specific example, let us know, will try to see if that is already handled.

Signed-off-by: Venkatesan Mahalingam <[email protected]>
Signed-off-by: Venkatesan Mahalingam <[email protected]>
Signed-off-by: Venkatesan Mahalingam <[email protected]>
Signed-off-by: Venkatesan Mahalingam <[email protected]>
@venkatmahalingam venkatmahalingam removed the request for review from pavel-shirshov March 5, 2021 19:49
@venkatmahalingam
Copy link
Collaborator Author

@lguohan Please review the changes again, if everything looks good, merge the PR. Thanks.

@lguohan
Copy link
Contributor

lguohan commented Mar 9, 2021

merge this as it is. the yang model discussion for this is still ongoing in the community.

@lguohan lguohan merged commit 7747e2f into sonic-net:master Mar 9, 2021
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.

5 participants