On 2/3/26 5:01 PM, Gabriel Goller wrote:
> + # Initialize router if not already configured
> + if (!keys %{$bgp_router}) {
> + $bgp_router->{asn} = $asn;
> + $bgp_router->{router_id} = $routerid;
> + $bgp_router->{default_ipv4_unicast} = 1;
This default setting here is wrong, previously we always set it to 0 [1].
Tests didn't catch this, because this depends on the order of names of
the controllers due to sorting [2] and in the test cases that would
catch this, the BGP controller generates the config *after* the EVPN
controller (where it is correctly set to 0 in this patch series).
This led me a bit deeper down the rabbit hole: We generate different
options for the BGP router depending on whether an EVPN or BGP
controller is evaluated first, because the default controller options
differ in the two plugins [3] [4].
I guess we should at least introduce a test-case that triggers this for
this patch series and evaluate which default options make sense with
which configuration in a later patch series?
Because the way it is now you can get different FRR configurations
depending on the name of the controllers...
[1]
https://git.proxmox.com/?p=pve-network.git;a=blob;f=src/PVE/Network/SDN/Controllers/BgpPlugin.pm;h=447ebf1ba74492898b7692988e6d5fb230f55552;hb=HEAD#l86
[2]
https://git.proxmox.com/?p=pve-network.git;a=blob;f=src/PVE/Network/SDN/Controllers.pm;h=3c1855253c5d4cb26d2d62871a7176d10c3ab2cb;hb=HEAD#l106
[3]
https://git.proxmox.com/?p=pve-network.git;a=blob;f=src/PVE/Network/SDN/Controllers/BgpPlugin.pm;h=447ebf1ba74492898b7692988e6d5fb230f55552;hb=HEAD#l86
[4]
https://git.proxmox.com/?p=pve-network.git;a=blob;f=src/PVE/Network/SDN/Controllers/EvpnPlugin.pm;h=cc217126607fddb8be39353aa18be6b68112e3ef;hb=HEAD#l131