-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
bgpd: Rework BGP dampening to be per AFI/SAFI #5307
bgpd: Rework BGP dampening to be per AFI/SAFI #5307
Conversation
Before we had: ! router bgp 65031 bgp dampening 1 2 3 4 ! exit2-debian-9(config)# router bgp 65031 exit2-debian-9(config-router)# address-family ipv4 multicast exit2-debian-9(config-router-af)# bgp dampening 5 6 7 8 exit2-debian-9(config-router-af)# end exit2-debian-9# show running-config ! router bgp 65031 bgp dampening 1 2 3 4 ! After fix: ! router bgp 65031 neighbor 192.168.1.2 remote-as 100 ! address-family ipv4 unicast bgp dampening 1 2 3 4 exit-address-family ! address-family ipv4 multicast bgp dampening 5 6 7 8 exit-address-family ! exit2-debian-9# show ip bgp ipv4 unicast dampening parameters Half-life time: 1 min Reuse penalty: 2 Suppress penalty: 3 Max suppress time: 4 min Max suppress penalty: 32 exit2-debian-9# show ip bgp ipv4 multicast dampening parameters Half-life time: 5 min Reuse penalty: 6 Suppress penalty: 7 Max suppress time: 8 min Max suppress penalty: 18 Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9652/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
does the documenation need to be updated? |
Am I reading the code right? This is configuration per afi/safi. So if I have router bgp 99 router bgp 99 vrf blue I can only have dampening per afi/safi not per bgp instance afi safi? |
@donaldsharp, unfortunately, this does not implement per VRF. What do you think about implementing per VRF as an enhancement with further commits?
I'll update it 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me. No logic/functionality change except replacing the global damp variable with address family level bdc.
One thing to note, This will break the existing functionality of having the bgp dampening for BGP VRF instances.
@donaldsharp @srimohans actually, the previous version before this change does not work with VRF as well, neither with per AFI/SAFI, only for AFI_IP/SAFI_UNICAST. This change allows dampening work per AFI/SAFI, at least for AFI_IP/SAFI_UNICAST and AFI_IP/SAFI_MULTICAST. |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9669/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
agreed this is a change in behavior. I was merely asking if this behavior needed to be tracked per |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopology tests on Ubuntu 16.04 amd64: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1604-9670/test Topology Tests failed for Topology tests on Ubuntu 16.04 amd64:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9670/artifact/TOPOU1604/ErrorLog/log_topotests.txt Topotest tests on Ubuntu 16.04 i386: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-9670/test Topology Tests failed for Topotest tests on Ubuntu 16.04 i386:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9670/artifact/TOPOI386/ErrorLog/log_topotests.txt Topology tests on Ubuntu 18.04 amd64: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1804-9670/test Topology Tests failed for Topology tests on Ubuntu 18.04 amd64:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9670/artifact/TOPOU1804/ErrorLog/log_topotests.txt Successful on other platforms
Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9670/artifact/TOPOU1604/MemoryLeaks/ Warnings Generated during build:Checkout code: Successful with additional warningsTopology tests on Ubuntu 16.04 amd64: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1604-9670/test Topology Tests failed for Topology tests on Ubuntu 16.04 amd64:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9670/artifact/TOPOU1604/ErrorLog/log_topotests.txt Topotest tests on Ubuntu 16.04 i386: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-9670/test Topology Tests failed for Topotest tests on Ubuntu 16.04 i386:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9670/artifact/TOPOI386/ErrorLog/log_topotests.txt Topology tests on Ubuntu 18.04 amd64: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1804-9670/test Topology Tests failed for Topology tests on Ubuntu 18.04 amd64:
see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9670/artifact/TOPOU1804/ErrorLog/log_topotests.txt
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9674/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
Before we had:
!
router bgp 65031
bgp dampening 1 2 3 4
!
exit2-debian-9(config)# router bgp 65031
exit2-debian-9(config-router)# address-family ipv4 multicast
exit2-debian-9(config-router-af)# bgp dampening 5 6 7 8
exit2-debian-9(config-router-af)# end
exit2-debian-9# show running-config
!
router bgp 65031
bgp dampening 1 2 3 4
!
After fix:
!
router bgp 65031
neighbor 192.168.1.2 remote-as 100
!
address-family ipv4 unicast
bgp dampening 1 2 3 4
exit-address-family
!
address-family ipv4 multicast
bgp dampening 5 6 7 8
exit-address-family
!
exit2-debian-9# show ip bgp ipv4 unicast dampening parameters
Half-life time: 1 min
Reuse penalty: 2
Suppress penalty: 3
Max suppress time: 4 min
Max suppress penalty: 32
exit2-debian-9# show ip bgp ipv4 multicast dampening parameters
Half-life time: 5 min
Reuse penalty: 6
Suppress penalty: 7
Max suppress time: 8 min
Max suppress penalty: 18
Closes #3193
Signed-off-by: Donatas Abraitis donatas.abraitis@gmail.com