Skip to content

[Nexthop] Add BGP thrift definitions#1038

Closed
manoharan-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:manoharan.bgp-show-thrift
Closed

[Nexthop] Add BGP thrift definitions#1038
manoharan-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:manoharan.bgp-show-thrift

Conversation

@manoharan-nexthop
Copy link
Copy Markdown
Contributor

@manoharan-nexthop manoharan-nexthop commented Mar 26, 2026

Pre-submission checklist

  • I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running pip install -r requirements-dev.txt && pre-commit install
  • pre-commit run

Summary

Adds the thrift interface definitions for BGP (Border Gateway Protocol) functionality. These thrift files define the data structures and service interfaces needed for BGP operations.

Thrift Files Added:

  • fboss/bgp/if/bgp_thrift.thrift - Main BGP service interface (TBgpService) defining RPC methods for BGP operations, session management, RIB queries, policy management, and monitoring
  • fboss/cli/fboss2/commands/show/bgp/summary/bgp_summary.thrift - BGP summary data model for CLI commands
  • fboss/routing/policy/if/policy_thrift.thrift - Routing policy statistics stub

Build System Updates:

  • CMakeLists.txt - Added thrift compilation targets for all BGP thrift files
  • BUCK files - Added thrift library definitions for Meta-internal builds

Test Plan

@manoharan-nexthop manoharan-nexthop requested review from a team as code owners March 26, 2026 18:25
@meta-cla meta-cla Bot added the CLA Signed label Mar 26, 2026
@manoharan-nexthop manoharan-nexthop changed the title Add BGP thrift definitions [Nexthop] Add BGP thrift definitions Mar 26, 2026
Comment thread fboss/bgp/if/bgp_config.thrift
Comment thread fboss/cli/fboss2/commands/show/bgp/summary/BUCK Outdated
Comment thread fboss/routing/if/BUCK
Comment thread fboss/bgp_policy/if/bgp_policy.thrift Outdated
Copy link
Copy Markdown
Contributor

@induvsuresh induvsuresh left a comment

Choose a reason for hiding this comment

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

Thank you for the changes, overall lgtm! pls address the inline comments

@manoharan-nexthop manoharan-nexthop force-pushed the manoharan.bgp-show-thrift branch from f7abcb4 to 09c95df Compare April 17, 2026 16:38
Copy link
Copy Markdown
Contributor

@induvsuresh induvsuresh left a comment

Choose a reason for hiding this comment

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

We were able to publish the OSS version of bgp_policy.thrift, rib_policy.thrift etc - https://github.com/facebook/fboss/tree/main/configerator/structs/neteng

Could we update the PR to drop these changes and use them directly
Thank you in advance!

@manoharan-nexthop manoharan-nexthop force-pushed the manoharan.bgp-show-thrift branch 2 times, most recently from 55ac725 to f0a6513 Compare April 28, 2026 18:12
@joseph5wu
Copy link
Copy Markdown
Contributor

@manoharan-nexthop : I recently synced our BGP open-source ready thrift files in this folder in fboss repo:
https://github.com/facebook/fboss/tree/main/configerator/structs/neteng
Can you use these thrift files in cmake to support bgp cli?

@manoharan-nexthop manoharan-nexthop force-pushed the manoharan.bgp-show-thrift branch from f0a6513 to 82eab0e Compare April 30, 2026 02:32
@manoharan-nexthop
Copy link
Copy Markdown
Contributor Author

@manoharan-nexthop : I recently synced our BGP open-source ready thrift files in this folder in fboss repo: https://github.com/facebook/fboss/tree/main/configerator/structs/neteng Can you use these thrift files in cmake to support bgp cli?

@joseph5wu, The bgp_config.thrift includes configerator/structs/neteng/config/vip_service_config.thrift which is not available yet. Will you be adding that as well?

Also, the main bgp service thrift file is not available yet which this PR introduces, should we leave that as is, or will you be bringing in that as well. Please let me know.

@induvsuresh
Copy link
Copy Markdown
Contributor

@manoharan-nexthop : I recently synced our BGP open-source ready thrift files in this folder in fboss repo: https://github.com/facebook/fboss/tree/main/configerator/structs/neteng Can you use these thrift files in cmake to support bgp cli?

@joseph5wu, The bgp_config.thrift includes configerator/structs/neteng/config/vip_service_config.thrift which is not available yet. Will you be adding that as well?
vip_service_config will not be OSS'ed, @manoharan-nexthop could you help clean up the bgp_config.thrift and any other usages in the thrift and update

Also, the main bgp service thrift file is not available yet which this PR introduces, should we leave that as is, or will you be bringing in that as well. Please let me know.
When you say main thrift file do you mean bgp_thrift.thrift?

@manoharan-nexthop manoharan-nexthop force-pushed the manoharan.bgp-show-thrift branch 4 times, most recently from c22ca75 to afce7c0 Compare May 6, 2026 00:57
This commit adds the core BGP service thrift interface along with
supporting thrift definitions and build configuration.

Files Added:
- fboss/bgp/if/bgp_thrift.thrift - Main BGP service interface (TBgpService)
  defining RPC methods for BGP operations, session management, RIB queries,
  policy management, and monitoring
- fboss/cli/fboss2/commands/show/bgp/summary/bgp_summary.thrift - BGP summary
  data model for CLI commands
- fboss/routing/policy/if/policy_thrift.thrift - Routing policy statistics stub

Build System Updates:
- CMakeLists.txt - Added thrift compilation targets for BGP thrift files
  with dependencies on configerator BGP policy and attribute definitions
- fboss/bgp/if/BUCK - Thrift library definition for bgp_thrift
- fboss/cli/fboss2/commands/show/bgp/summary/BUCK - Thrift library for bgp_summary
- fboss/routing/policy/if/BUCK - Thrift library for policy_thrift

Dependencies:
- Requires configerator BGP policy and attribute thrift definitions
- Integrates with fb303 for service monitoring
@manoharan-nexthop manoharan-nexthop force-pushed the manoharan.bgp-show-thrift branch from afce7c0 to 1ce3372 Compare May 7, 2026 14:22
@induvsuresh
Copy link
Copy Markdown
Contributor

bgp_thrift.thrift and policy_thrift have been published to the fboss OSS repo already, could we re-use that and that way this PR can have just the bgp_summary , its related BUCK and cmake file

@manoharan-nexthop
Copy link
Copy Markdown
Contributor Author

Closing this PR, as #1039 already has this change (only this PR now has one thrift file change).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants