FRRouting / frr

The FRRouting Protocol Suite
https://frrouting.org/
Other
3.29k stars 1.25k forks source link

bgpd: avoid clearing routes for peers that were never established #16271

Closed lsang6WIND closed 3 months ago

lsang6WIND commented 3 months ago

Under heavy system load with many peers in passive mode and a large number of routes, bgpd can enter an infinite loop. This occurs while processing timeout BGP_OPEN messages, which prevents it from accepting new connections. The following log entries illustrate the issue:

bgpd[6151]: [VX6SM-8YE5W][EC 33554460] 3.3.2.224: nexthop_set failed, resetting connection - intf 0x0 bgpd[6151]: [P790V-THJKS][EC 100663299] bgp_open_receive: bgp_getsockname() failed for peer: 3.3.2.224 bgpd[6151]: [HTQD2-0R1WR][EC 33554451] bgp_process_packet: BGP OPEN receipt failed for peer: 3.3.2.224 ... repeating

The issue occurs when bgpd handles a massive number of routes in the RIB while receiving numerous BGP_OPEN packets. If bgpd is overloaded, it fails to process these packets promptly, leading the remote peer to close the connection and resend BGP_OPEN packets.

When bgpd eventually starts processing these timeout BGP_OPEN packets, it finds the TCP connection closed by the remote peer, resulting in "bgp_stop()" being called. For each timeout peer, bgpd must iterate through the routing table, which is time-consuming and causes new incoming BGP_OPEN packets to timeout, perpetuating the infinite loop.

To address this issue, the code is modified to check if the peer has been established at least once before calling "bgp_clear_route_all()". This ensures that routes are only cleared for peers that had a successful session, preventing unnecessary iterations over the routing table for peers that never established a connection.

With this change, BGP_OPEN timeout messages may still occur, but in the worst case, bgpd will stabilize. Before this patch, bgpd could enter a loop where it was unable to accpet any new connections.

fdumontet6WIND commented 3 months ago

improve performance

ton31337 commented 3 months ago

@Mergifyio backport dev/10.1

mergify[bot] commented 3 months ago

backport dev/10.1

✅ Backports have been created

* [#16302 bgpd: avoid clearing routes for peers that were never established (backport #16271)](https://github.com/FRRouting/frr/pull/16302) has been created for branch `dev/10.1`
riw777 commented 3 months ago

lint errors need to be fixed ... still trying to get ci to pass (it's failing in ospf)

lsang6WIND commented 3 months ago

Previous checks are all okay except for the linter.

The following topotest failures are not related to this PR: bgp_gr_functionality_topo1.test_bgp_gr_functionality_topo1-3 test_BGP_GR_TC_31_1_p1 bgp_peer_type_multipath_relax.test_bgp_peer-type_multipath-relax test_bgp_peer_type_multipath_relax_test10