bgpd: Ensure send order is 100% consistent #14468
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When BGP is sending updates to peers on a neighbor up event it was noticed that the bgp updates being sent were in reverse order being sent to the first peer. Imagine r1 -- r2 -- r3. r1 and r2 are ebgp peers and r2 and r3 are ebgp peers. r1's interface to r2 is currently shutdown. Prior to this fix the send order would look like this: r1 -> r2 send of routes to r2 and then they would be installed in order received: 10.0.0.12 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.11 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.10 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.9 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.8 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.7 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.6 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.5 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.4 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.3 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.2 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.1 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 r2 would then send these routes to r3 and then they would be installed in order received: 10.0.0.1 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.2 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.3 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.4 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.5 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.6 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.7 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.8 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.9 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.10 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.11 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.12 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 Not that big of a deal right? Well imagine a situation where r1 is originating several ten's of thousands of routes. It sends routes to r2 r2 is processing routes but in reverse order and at the same time it is sending routes to r3, in the correct order of the bgp table. r3 will have the early 10.0.0.1/32 routes installed and start forwarding while r2 will not have those routes installed yet( since they were at the end and zebra is slightly slower for processing routes than bgp is ). Ensure that the order sent is a true FIFO. What is happening is that there is an update fifo which stores all routes. And off that FIFO is a bgp advertise attribute list which stores the list of prefixes which share the same attribute that allow for more efficient packing this list was being stored in reverse order causing the problem for the initial send. When adding items to this list put them at the end so we keep the fifo order that is traversed when we walk through the bgp table. After the fix: r2 installation order: 10.0.0.0 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.1 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.2 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.3 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.4 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.5 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.6 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.7 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.8 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.9 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.10 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.11 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.12 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 r3 installation order: 10.0.0.0 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.1 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.2 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.3 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.4 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.5 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.6 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.7 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.8 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.9 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.10 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.11 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.12 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 Signed-off-by: Donald Sharp <[email protected]>
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO8U18I386-14292/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 8 Successful on other platforms/tests
|
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-PULLREQ2-14292/ This is a comment from an automated CI system. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When BGP is sending updates to peers on a neighbor up event it was noticed that the bgp updates being sent were in reverse order being sent to the first peer.
Imagine r1 -- r2 -- r3. r1 and r2 are ebgp peers and r2 and r3 are ebgp peers. r1's interface to r2 is currently shutdown. Prior to this fix the send order would look like this:
r1 -> r2 send of routes to r2 and then they would be installed in order received:
10.0.0.12 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.11 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.10 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.9 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.8 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.7 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.6 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.5 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.4 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.3 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.2 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.1 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20
r2 would then send these routes to r3 and then they would be installed in order received:
10.0.0.1 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.2 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.3 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.4 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.5 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.6 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.7 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.8 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.9 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.10 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.11 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.12 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20
Not that big of a deal right? Well imagine a situation where r1 is originating several ten's of thousands of routes. It sends routes to r2 r2 is processing routes but in reverse order and at the same time it is sending routes to r3, in the correct order of the bgp table.
r3 will have the early 10.0.0.1/32 routes installed and start forwarding while r2 will not have those routes installed yet( since they were at the end and zebra is slightly slower for processing routes than bgp is ).
Ensure that the order sent is a true FIFO. What is happening is that there is an update fifo which stores all routes. And off that FIFO is a bgp advertise attribute list which stores the list of prefixes which share the same attribute that allow for more efficient packing this list was being stored in reverse order causing the problem for the initial send. When adding items to this list put them at the end so we keep the fifo order that is traversed when we walk through the bgp table.
After the fix:
r2 installation order:
10.0.0.0 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.1 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.2 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.3 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.4 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.5 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.6 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.7 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.8 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.9 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.10 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.11 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20 10.0.0.12 nhid 39 via 192.168.8.2 dev leaf2-eth5 proto bgp metric 20
r3 installation order:
10.0.0.0 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.1 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.2 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.3 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.4 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.5 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.6 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.7 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.8 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.9 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.10 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.11 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20 10.0.0.12 nhid 12 via 192.168.61.2 dev spine2-eth1 proto bgp metric 20