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

[bgp] Add explictly configure bgp router id for multi-asic #18764

Merged
merged 11 commits into from
May 16, 2024

Conversation

yaqiangz
Copy link
Contributor

@yaqiangz yaqiangz commented Apr 23, 2024

Why I did it

HLD: sonic-net/SONiC#1643
Remove hard coupling between bgp router-id and IPv4 address of Loopback4096
Add support to explictly specify bgp router id for multi asic device

Work item tracking
  • Microsoft ADO (number only): 27801007

How I did it

When bgp_router_id configured in DEVICE_METADATA, use it as bgp router-id and originator-id.
Remove the hard dependency on loopback4096 ipv4 address when adding ibgp peer.
Add UTs.

Behavior of set bgp router-id

To be clarified that when bgp router-id hasn't been explicitly set, bgp actions would totally like previous

Loopback4096 IPv4 address exists Loopback4096 IPv4 address doesn't exist
bgp_router_id configured Honor bgp_router_id Honor bgp_router_id
bgp_router_id doesn't be configured Honor Loopback4096 IPv4 address FRR default router ID value is selected as the largest IP address of the device. When router zebra is not enabled bgpd can’t get interface information, so router-id is set to 0.0.0.0

Behavior of add bgp peer

To be clarified that when bgp router-id hasn't been explicitly set, bgp actions would totally like previous

Loopback4096 IPv4 address exists Loopback4096 IPv4 address doesn't exist
bgp_router_id configured Add BGP peer Add BGP peer
bgp_router_id doesn't be configured Add BGP peer Do not add iBGP peer

How to verify it

UT passed.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@mssonicbld
Copy link
Collaborator

@yaqiangz PR: #18764 is conflict with MS internal repo
Please push fix commit to sonicbld/precheck/head/18764 and approve
https://msazure.visualstudio.com/One/_git/Networking-acs-buildimage/pullrequest/9949534
After ms PR is merged, comment "/azpw ms_conflict" to rerun PR checker.

@yaqiangz
Copy link
Contributor Author

yaqiangz commented May 7, 2024

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yaqiangz yaqiangz force-pushed the master_bgp_router_id_multiasic branch from e97714b to c4ce9a8 Compare May 7, 2024 08:48
@yaqiangz yaqiangz force-pushed the master_bgp_router_id_multiasic branch from c4ce9a8 to 656dd5a Compare May 7, 2024 08:52
@yaqiangz yaqiangz force-pushed the master_bgp_router_id_multiasic branch from 58304af to ecdbf44 Compare May 7, 2024 13:57
@yaqiangz yaqiangz marked this pull request as ready for review May 8, 2024 05:13
@yaqiangz
Copy link
Contributor Author

yaqiangz commented May 8, 2024

@StormLiangMS Could you please help to review this PR?

@StormLiangMS StormLiangMS requested a review from abdosi May 8, 2024 08:44
@yaqiangz
Copy link
Contributor Author

yaqiangz commented May 9, 2024

Hi @abdosi could you please help to review this PR?

@yaqiangz yaqiangz force-pushed the master_bgp_router_id_multiasic branch from 68f1d81 to d5e07d4 Compare May 13, 2024 13:00
@StormLiangMS StormLiangMS merged commit c48cc1e into sonic-net:master May 16, 2024
18 checks passed
StormLiangMS pushed a commit that referenced this pull request May 21, 2024
Why I did it
Remove unnecessary print introduced by #18764

Work item tracking
Microsoft ADO (number only): 27801007
How I did it
Remove unnecessary print introduced by #18764

How to verify it
UT passed
StormLiangMS pushed a commit that referenced this pull request May 28, 2024
Why I did it
These 2 dependencies were removed by #18764 and #18727 for decoupling hard dependency between Loopback IPv4 address and BGP.
Actually, these dependencies are to make sure existence of Loopback interface, no need to remove.

Work item tracking
Microsoft ADO (number only): 28210625
How I did it
Add dependency back

How to verify it
UT passed.
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.

4 participants