allow to set individual peer group policies and compute of them once#3161
Open
myaut wants to merge 2 commits intoosrg:masterfrom
Open
allow to set individual peer group policies and compute of them once#3161myaut wants to merge 2 commits intoosrg:masterfrom
myaut wants to merge 2 commits intoosrg:masterfrom
Conversation
8129693 to
9eb31d0
Compare
added 2 commits
September 24, 2025 21:05
Idea of per-peer-group policy is described in docs/sources/policy.md. Basically: - Instead of sending update to an individual *peer and computing policy for each of it, GoBGP will now compute policy and other path changes for a receiver, which can be either a *peer, or a peer-group (thus computing changes once even if there are 10 peers in a peer-group) - Similarly, when receiving update, per-peer-group policy is checked (stored internally as pg:PGNAME) instead of global which allows to skip neighbor set check. - Default Config/State values in BGP configs now computed both for oc.Peer and oc.PeerGroup table code now relies on PeerInfo structure that contains all relevant fields needed to change path when sending it to a peer. Per-peer-group policy for various types of servers (route-reflector, route-server) are checked by TestPeerStarTopology test. The last change revealed important problem: code which was checking presence of Local AS in received routes was checking for AS configured in Global config section even if LocalAS override was supplied for a peer group. Per https://www.cisco.com/c/en/us/support/docs/ip/border-gateway-protocol-bgp/13761-39.html idea of LocalAS was migratory period when two AS belong to a same router, thus both AS numbers should be checked. So now both checks are performed, but for a new check (LocalAS override) path is imported while warning is issued which is subject to change in future releases. Various invariants of LocalAS config are now checked by TestBgpServerLocalASOverride test. In our tests we reduced propagaging time of 1.2M routes to ~100 peers in route reflector setup from 7 minutes to 2 minutes.
9eb31d0 to
30af8e4
Compare
Author
This is kinda strange test failure since drainChannel is written in a manner it can't stuck. I belive I've seen similar in my manual runs with some race tracking function going wild, so look like a possible runtime issue. Be aware. |
Author
|
@fujita have you had the chance to look at it? I know PR is large, but conflicts are growing too |
Contributor
|
Hello, @fujita! Could you take a look at this PR, please? Maybe I can split it for several PRs for your convenience |
This file contains hidden or 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
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.
Idea of per-peer-group policy is described in docs/sources/policy.md. Basically:
*peerand computing policy for each of it, GoBGP will now compute policy and other path changes for a receiver, which can be either a*peer, or a peer-group (thus computing changes once even if there are 10 peers in a peer-group)oc.Peerandoc.PeerGrouptable code now relies on PeerInfo structure that contains all relevant fields needed to change path when sending it to a peer.Per-peer-group policy for various types of servers (route-reflector, route-server) are checked by TestPeerStarTopology test.
The last change revealed important problem: code which was checking presence of Local AS in received routes was checking for AS configured in Global config section even if LocalAS override was supplied for a peer group.
Per https://www.cisco.com/c/en/us/support/docs/ip/border-gateway-protocol-bgp/13761-39.html idea of LocalAS was migratory period when two AS belong to a same router, thus both AS numbers should be checked. So now both checks are performed, but for a new check (LocalAS override) path is imported while warning is issued which is subject to change in future releases.
Various invariants of LocalAS config are now checked by TestBgpServerLocalASOverride test.
In our tests we reduced propagaging time of 1.2M routes to ~100 peers in route reflector setup from 7 minutes to 2 minutes.