Bgpbrg - #339
Merged
Merged
Conversation
crypticC0der
force-pushed
the
bgpbrg
branch
2 times, most recently
from
June 25, 2026 14:32
f1fd19c to
3d10b2c
Compare
Copilot stopped reviewing on behalf of
crypticC0der due to an error
June 25, 2026 14:32
crypticC0der
force-pushed
the
bgpbrg
branch
2 times, most recently
from
July 6, 2026 20:40
043019d to
ad76b0d
Compare
Expands microovn enable bgp to accept a bridge as the argument instead of a network interface. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com>
Added as part of other work that ended up not being used, but it expands our library so I thought it's still worth committing. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com>
This allows us to avoid creating a default interface on the bridge and makes the naming easier, no real functional changes. Part of larger ongoing BGP work. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com>
frr_start_bgp_unnumbered only takes one interface which is a little annoying if you want to have connections with multiple neighbors. So I have just made it take as many interfaces as you give it. This also meant i had to move some test arguments around. Also cleaned up the data plane test a bit. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com>
Contributor
Author
|
Currently a failed join means that node cannot be added to the cluster, however there is an open issue and PR canonical/microcluster#784 |
Adds microovn setup bgp as explained in the spec. This sets up the bgp service with the same configuration on each node of the cluster, and when new nodes join they are made to join. However if they do not have the proper interface the prejoin hook fails. This should allow you to fix it and then rejoin but there is an ongoing microcluster issue wherin the join hook does not fail cleanly. This also adds setup_bgp tests, which test various argument validation and then tests the actual functionality. All microovn setup bgp is, is a way of easily and safely doing microovn enable bgp on a whole cluster at once. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com> Assisted-by: Deepseek V4 Pro
We need ParseASNRange in a few places, this should be then be an exported function to avoid code reuse. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com>
We need to be doing lots of ovs-vsctl gets in the external-ids enable bgp work so we should allow this function to be used elsewhere as well. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com>
crypticC0der
force-pushed
the
bgpbrg
branch
2 times, most recently
from
July 16, 2026 12:09
d1a2d39 to
afd042e
Compare
matperin
reviewed
Jul 17, 2026
|
|
||
| const BgpConfigKeyAsnRange = "microovn-bgp-arg-asn_range" | ||
|
|
||
| const BgpConfigKeyConnectionIP4Range = "microovn-bgp-arg-connection-ip4-range" |
Contributor
There was a problem hiding this comment.
Maybe it is better to wait for the numbered BGP implementation before introducing the --connection-ipv4-range configuration value?
Right now this key seems to be never used and its addition would semantically belong to the proper commit making use of it.
matperin
reviewed
Jul 17, 2026
| brgInterface := getBgpRedirectIfacePeerName(extConnection.Iface) | ||
| mac := generateLrpMac(getLrpName(s, extConnection.Iface)) | ||
| for _, brg := range bridges { | ||
| bgpInterface := getBgpRedirectIfaceName(brg.Bridge) |
Contributor
There was a problem hiding this comment.
It is worth nothing that net iface names are capped at 15 chars in the linux kernel.
Since veth names are derived as v<bridge>-bgp / v<bridge>-brg that would make only bridges with names of <=10 chars acceptable.
Not a problem with the implementation per-se as it is a hard limit on the kernel, but something we may want to document or flag somewhere.
matperin
reviewed
Jul 17, 2026
matperin
reviewed
Jul 17, 2026
matperin
requested changes
Jul 17, 2026
matperin
left a comment
matperin
left a comment
Contributor
There was a problem hiding this comment.
Overall amazing work @crypticC0der!
Also good job on the test coverage, I appreciate it.
I left some minor things to consider in some comments, but overall the PR seems to be in very good shape already.
On join or bootstrap, if there is a OVS bridge with microovn-bgp-enabled=true. It should set up BGP on it, it should also inspect the bridge for any config options and use them in the enablement. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com> Assisted-by: Deepseek V4 Pro
matperin
approved these changes
Jul 22, 2026
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.
Expands microovn enable bgp to accept a bridge as the argument instead of a network interface.