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] Enable BGP Graceful Restart based on device role #9486

Merged
merged 24 commits into from
Dec 13, 2021

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Dec 9, 2021

What I did:
Updated Jinja Template to enable BGP Graceful Restart based on device role. By default it will be enable only if the device role type is TorRouter.

Why I did:-
By default FRR is configured in Graceful Helper mode. Graceful Restart is needed on T0/TorRouter only since the device can go for warm-reboot. For T1/LeafRouter it need to be in Helper mode only. For T2 and above we can disable Graceful Restart feature completely support by FRR 7.4 onwards (FRRouting/frr#5210). However As per FRR current code it send BGP EOR packet only if Peer has advertised Graceful Restart or Helper capability. And as mention in BGP RFC https://datatracker.ietf.org/doc/html/rfc4724 sending of BGP EOR is better for overall network convergance so we are having BGP Helper mode on for all role other than T0/TorRouter

Trigger for this change was eg:
a) if we cold-reboot the T1 router T0 will not withdraw T1 routes till Graceful Timer is not expired and this causes traffic blackhole for that duration.
b) For Chassis iBGP session to detect BGP peer down and withdraw routes immediately

How I verify:

  1. Updated Unit Test case.
  2. Manual Verification.
  3. We will create test for this in sonic-mgmt
On Instance0 GR commands configured with Restart Timer as 240 sec and Instance1 has no GR configuration 

1.	We can see Instance1 by default goes into helper mode

•	Instance 0 : show ip bgp neighbor O/P
                                                                                                                                                                               
  Graceful restart information:
    End-of-RIB send: IPv4 Unicast
    End-of-RIB received: IPv4 Unicast
    Local GR Mode: Restart*
    Remote GR Mode: Helper
    R bit: False
    Timers:
      Configured Restart Time(sec): 240
      Received Restart Time(sec): 120
    IPv4 Unicast:
      F bit: False
      End-of-RIB sent: Yes
      End-of-RIB sent after update: No
      End-of-RIB received: Yes
      Timers:
        Configured Stale Path Time(sec): 360
        Configured Selection Deferral Time(sec): 45

•	Instance 1 : show ip bgp neighbor O/P

  Graceful restart information:
    End-of-RIB send: IPv4 Unicast
    End-of-RIB received: IPv4 Unicast
    Local GR Mode: Helper*
    Remote GR Mode: Restart
    R bit: True
    Timers:
      Configured Restart Time(sec): 120
      Received Restart Time(sec): 240
    IPv4 Unicast:
      F bit: True
      End-of-RIB sent: Yes
      End-of-RIB sent after update: Yes
      End-of-RIB received: Yes
      Timers:
        Configured Stale Path Time(sec): 360

2.	When the Helper BGP instance goes down (Instance 1) Restart BGP Instance (Instance0) remove the route immediately.

3.	When the Restart BGP instance goes down (Instance 0) Helper BGP Instance (Instance1) remove the route after 4 min (240 sec as configured).

abdosi added 24 commits April 3, 2021 01:41
df46ed418e661a9bccdb2639d8873def356f8ba0 (HEAD -> master, origin/master, origin/HEAD) Fix the LLDP_LOC_CHASSIS not getting populated if no remote neighbors are present (sonic-net#39)
e487532e11cc0e97cfce573b6b997fdd0beeb660 [CI] Set up CI&PR with Azure Pipelines (sonic-net#38)
3c9f488490a1dbded20dbf2d8a88a5ab4dbda8df Replace swsssdk's SonicV2Connector with swsscommon's implementation (sonic-net#35)

Signed-off-by: Abhishek Dosi <[email protected]>
•	Internal BGP Peering between LCx-ASICy is done using Loopback4096
•	Internal BGP Peer will be classified as BGP_INTERNAL_NEIGHBOR with source updated as Loopback4096
•	DEVICE_METADATA will have “switch_type” define as “chassis-packet”.  We already have “voq” define. This will be used in BGP template.
•	Static Routes are define using SONiC schema.
*       Fixed Vlan Sub Interface Parsing in Graph when configured on Port-Channel.
*       Added Unit test cases of graph and BGP template changes.

Signed-off-by: Abhishek Dosi <[email protected]>
Signed-off-by: Abhishek Dosi <[email protected]>
Also address review comments for PR:sonic-net#8966

Signed-off-by: Abhishek Dosi <[email protected]>
1. For Packet Chassis do not advertise Loopback4096 address into BGP
   as there is Static Route for same.
2. Advertise only P2P Connected IP's into BGP

Signed-off-by: Abhishek Dosi <[email protected]>
By default FRR will be in Graceful Helper mode

Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi abdosi requested a review from lguohan as a code owner December 9, 2021 21:06
@abdosi abdosi requested review from arlakshm, yxieca and rlhui December 9, 2021 21:06
Copy link
Contributor

@arlakshm arlakshm left a comment

Choose a reason for hiding this comment

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

For T2 role, should we add bgp graceful-restart-disable config to disable helper mode as well?

@abdosi
Copy link
Contributor Author

abdosi commented Dec 9, 2021

For T2 role, should we add bgp graceful-restart-disable config to disable helper mode as well?

@arlakshm PR description has reason of not doing that as per current understanding. In future we can reference this change again.

@abdosi
Copy link
Contributor Author

abdosi commented Dec 10, 2021

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@abdosi
Copy link
Contributor Author

abdosi commented Dec 12, 2021

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@abdosi abdosi merged commit 6c0da4b into sonic-net:master Dec 13, 2021
@abdosi abdosi deleted the chassis-packet branch December 13, 2021 18:14
@abdosi abdosi added Chassis 🤖 Modular chassis support Request for 202111 Branch For PRs being requested for 202111 branch Request for 202106 Branch labels Dec 13, 2021
judyjoseph pushed a commit that referenced this pull request Dec 27, 2021
What I did:
Updated Jinja Template to enable BGP Graceful Restart based on device role. By default it will be enable only if the device role type is TorRouter.

Why I did:-
By default FRR is configured in Graceful Helper mode. Graceful Restart is needed on T0/TorRouter only since the device can go for warm-reboot. For T1/LeafRouter it need to be in Helper mode only
abdosi added a commit that referenced this pull request Apr 1, 2022
What I did:
Updated Jinja Template to enable BGP Graceful Restart based on device role. By default it will be enable only if the device role type is TorRouter.

Why I did:-
By default FRR is configured in Graceful Helper mode. Graceful Restart is needed on T0/TorRouter only since the device can go for warm-reboot. For T1/LeafRouter it need to be in Helper mode only
abdosi added a commit that referenced this pull request Apr 2, 2022
[bgp] Enable BGP Graceful Restart based on device role (#9486)

Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi
Copy link
Contributor Author

abdosi commented Sep 1, 2022

@yxieca @qiluo-msft can we take this for 202012.

abdosi added a commit to abdosi/sonic-buildimage that referenced this pull request Dec 8, 2022
What I did:
Updated Jinja Template to enable BGP Graceful Restart based on device role. By default it will be enable only if the device role type is TorRouter.

Why I did:-
By default FRR is configured in Graceful Helper mode. Graceful Restart is needed on T0/TorRouter only since the device can go for warm-reboot. For T1/LeafRouter it need to be in Helper mode only
@abdosi
Copy link
Contributor Author

abdosi commented Dec 8, 2022

@qiluo-msft created 202012 PR: #12990

lguohan pushed a commit that referenced this pull request Dec 8, 2022
What I did:
Updated Jinja Template to enable BGP Graceful Restart based on device role. By default it will be enable only if the device role type is TorRouter.

Why I did:-
By default FRR is configured in Graceful Helper mode. Graceful Restart is needed on T0/TorRouter only since the device can go for warm-reboot. For T1/LeafRouter it need to be in Helper mode only
github-actions bot pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Aug 29, 2023
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.

6 participants