[PATCH pve-network 07/16] evpn controller: add bgp-mode setting

[Stefan Hanreich <s.hanreich@proxmox.com>]

This setting allows to define the type of BGP session (iBGP or eBGP)
in the EVPN controller, analogous to the BGP controller.

Previously, the EVPN controller always inherited the type of BGP
session of the BGP controller, if one was defined. This made it
impossible to have EVPN over iBGP and the underlay via eBGP. By
decoupling the session type of the EVPN controller from the session
type of the BGP controller, a great deal of flexibility is added for
users that want to configure certain setups that were not possible
before.

Additionally, with the previous patches that introduce defining
multiple EVPN controllers, it is now possible to have multiple EVPN
sessions, each with their own BGP semantics. This allows defining EVPN
controllers that handle announcing EVPN routes inside the cluster via
iBGP and then have a different EVPN BGP session for interconnects /
uplinks that utilize eBGP.

To achieve this, introduce a new helper function that determines the
ASN of a node. The new behavior is explained in the documentation of
the function.

In order to preserve backwards-compatibility, the bgp-mode setting
defaults to 'legacy' which keeps the old behavior. The new behavior is
completely opt-in and needs to be explicitly set in the EVPN
controller. If any EVPN controller on a node is defined with legacy
behavior, then the old logic of inheriting the BGP session always has
precendence.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN/Controllers.pm       |  7 ++
 src/PVE/Network/SDN/Controllers/BgpPlugin.pm  | 17 +++-
 src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 54 +++++++++++--
 src/PVE/Network/SDN/Controllers/Plugin.pm     | 77 +++++++++++++++++++
 4 files changed, 149 insertions(+), 6 deletions(-)

diff --git a/src/PVE/API2/Network/SDN/Controllers.pm b/src/PVE/API2/Network/SDN/Controllers.pm
index 49951a3..e26e0a5 100644
--- a/src/PVE/API2/Network/SDN/Controllers.pm
+++ b/src/PVE/API2/Network/SDN/Controllers.pm
@@ -99,6 +99,13 @@ my $CONTROLLER_PROPERTIES = {
         type => 'string',
         optional => 1,
     },
+    'bgp-mode' => {
+        description =>
+            "Whether to use eBGP or iBGP. Legacy mode chooses depending on BGP controller or falls back to iBGP.",
+        type => 'string',
+        enum => ['legacy', 'external', 'internal'],
+        optional => 1,
+    },
 };
 
 __PACKAGE__->register_method({
diff --git a/src/PVE/Network/SDN/Controllers/BgpPlugin.pm b/src/PVE/Network/SDN/Controllers/BgpPlugin.pm
index 7cbd436..5edbae6 100644
--- a/src/PVE/Network/SDN/Controllers/BgpPlugin.pm
+++ b/src/PVE/Network/SDN/Controllers/BgpPlugin.pm
@@ -83,7 +83,9 @@ sub generate_frr_config {
 
     # Initialize router if not already configured
     if (!keys %{$bgp_router}) {
-        $bgp_router->{asn} = $asn;
+        $bgp_router->{asn} =
+            PVE::Network::SDN::Controllers::Plugin::get_default_router_asn($local_node,
+                $controller);
         $bgp_router->{router_id} = $routerid;
         $bgp_router->{default_ipv4_unicast} = 0;
         $bgp_router->{coalesce_time} = 1000;
@@ -104,8 +106,21 @@ sub generate_frr_config {
             ips => \@peers,
             interfaces => [],
         };
+
         $neighbor_group->{ebgp_multihop} = int($ebgp_multihop) if $ebgp && $ebgp_multihop;
 
+        if ($asn != int($bgp_router->{asn})) {
+            # should never trigger due to validation, but asserting it here nonetheless
+            die
+                "cannot set local_as to $asn - since this is the default router ASN and therefore an iBGP session"
+                if !$ebgp;
+
+            $neighbor_group->{local_as} = {
+                asn => $asn,
+                mode => 'no-prepend replace-as',
+            };
+        }
+
         push @{ $bgp_router->{neighbor_groups} }, $neighbor_group;
 
         # Configure address-family unicast
diff --git a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
index a683dde..db844ff 100644
--- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
@@ -44,6 +44,14 @@ sub properties {
             optional => 1,
             default => 'VTEP',
         },
+        'bgp-mode' => {
+            description =>
+                "Whether to use eBGP or iBGP. Legacy mode chooses depending on BGP controller or falls back to iBGP.",
+            type => 'string',
+            enum => ['legacy', 'external', 'internal'],
+            optional => 1,
+            default => 'legacy',
+        },
     };
 }
 
@@ -56,6 +64,7 @@ sub options {
         'route-map-out' => { optional => 1 },
         'nodes' => { optional => 1 },
         'peer-group-name' => { optional => 1 },
+        'bgp-mode' => { optional => 1 },
     };
 }
 
@@ -77,6 +86,7 @@ sub generate_frr_config {
     my $autortas = undef;
     my $ifaceip = undef;
     my $routerid = undef;
+    my $bgp_mode = $plugin_config->{'bgp-mode'} // 'legacy';
 
     my $bgp_controller = find_bgp_controller($local_node, $controller_cfg);
     my $isis_controller = find_isis_controller($local_node, $controller_cfg);
@@ -132,10 +142,12 @@ sub generate_frr_config {
         return;
     }
 
-    if ($bgp_controller) {
+    if ($bgp_controller && $bgp_mode eq 'legacy') {
         $ebgp = 1 if $plugin_config->{'asn'} ne $bgp_controller->{asn};
         $asn = int($bgp_controller->{asn}) if $bgp_controller->{asn};
         $autortas = $plugin_config->{'asn'} if $ebgp;
+    } else {
+        $ebgp = $bgp_mode eq 'external';
     }
 
     return if !$asn || !$routerid;
@@ -144,7 +156,9 @@ sub generate_frr_config {
 
     # Initialize router if not already configured
     if (!keys %{$bgp_router}) {
-        $bgp_router->{asn} = $asn;
+        $bgp_router->{asn} = PVE::Network::SDN::Controllers::Plugin::get_default_router_asn(
+            $local_node, $controller_cfg,
+        );
         $bgp_router->{router_id} = $routerid;
         $bgp_router->{default_ipv4_unicast} = 0;
         $bgp_router->{hard_administrative_reset} = 0;
@@ -166,7 +180,21 @@ sub generate_frr_config {
         ips => \@vtep_ips,
         interfaces => [],
     };
-    $neighbor_group->{ebgp_multihop} = 10 if $ebgp && $loopback;
+
+    $neighbor_group->{ebgp_multihop} = 10 if $ebgp && $loopback && $bgp_mode eq 'legacy';
+
+    if ($asn != int($bgp_router->{asn})) {
+        # should never trigger due to validation, but asserting it here nonetheless
+        die
+            "cannot set local_as to $asn - since this is the default router ASN and therefore an iBGP session"
+            if !$ebgp;
+
+        $neighbor_group->{local_as} = {
+            asn => $asn,
+            mode => 'no-prepend replace-as',
+        };
+    }
+
     $neighbor_group->{update_source} = $loopback if $loopback;
 
     push @{ $bgp_router->{neighbor_groups} }, $neighbor_group;
@@ -299,7 +327,8 @@ sub generate_zone_frr_config {
 
     # Configure VRF
     my $vrf_router = $config->{frr}->{bgp}->{vrf_router}->{$vrf} //= {};
-    $vrf_router->{asn} = $asn;
+    $vrf_router->{asn} = PVE::Network::SDN::Controllers::Plugin::get_default_router_asn($local_node,
+        $controller_cfg);
     $vrf_router->{router_id} = $routerid;
     $vrf_router->{hard_administrative_reset} = 0;
     $vrf_router->{graceful_restart_notification} = 0;
@@ -343,7 +372,7 @@ sub generate_zone_frr_config {
     $vrf_router->{address_families} = {};
 
     # Configure L2VPN EVPN address family with route targets
-    if ($autortas) {
+    if ($autortas && $autortas ne $vrf_router->{asn}) {
         $vrf_router->{address_families}->{l2vpn_evpn} //= {};
         $vrf_router->{address_families}->{l2vpn_evpn}->{route_targets} = {
             import => ["$autortas:$vrfvxlan"],
@@ -519,6 +548,21 @@ sub on_update_hook {
 
     my $controller = $controller_cfg->{ids}->{$controllerid};
 
+    my @nodes;
+    if (defined($controller->{nodes})) {
+        @nodes = PVE::Tools::split_list($controller->{nodes});
+    } else {
+        @nodes = PVE::Cluster::get_nodelist()->@*;
+    }
+
+    # check if there is a unambiguous default router ASN on every node with the
+    # updated controller - this method dies if there isn't one and we can use
+    # that behavior for validation purposes to avoid re-implementing the same
+    # logic here. For more information see the documentation of the method.
+    for my $node (@nodes) {
+        PVE::Network::SDN::Controllers::Plugin::get_default_router_asn($node, $controller_cfg);
+    }
+
     foreach my $id (keys %{ $controller_cfg->{ids} }) {
         next if $id eq $controllerid;
         my $other_controller = $controller_cfg->{ids}->{$id};
diff --git a/src/PVE/Network/SDN/Controllers/Plugin.pm b/src/PVE/Network/SDN/Controllers/Plugin.pm
index 1068b5d..f9301a3 100644
--- a/src/PVE/Network/SDN/Controllers/Plugin.pm
+++ b/src/PVE/Network/SDN/Controllers/Plugin.pm
@@ -136,4 +136,81 @@ sub get_router_id {
         . hex($mac_bytes[5]);
 }
 
+=head3 get_default_router_asn($node_name, \%controller_config)
+
+This function determines the ASN that should be used for the BGP router
+definition in the FRR configuration on node $node_name with the given controller
+configuration \%controller_config.
+
+For backwards-compatibility reasons, this function checks if there is any EVPN
+controller in legacy mode. The initial SDN implementation *always* uses the ASN
+in the BGP controller for its router defintion, if it exists, so return the ASN
+of the BGP controller if one is configured and any EVPN controller uses the
+legacy mode.
+
+The ASN in the router definition defines the ASN of the local node, and is used
+for deriving e.g. Route Targets in EVPN setups. Therefore the configuration
+needs to always use the EVPN ASN in its router definition to ensure correct
+generation for Route Targets (if not using the autort patch).
+
+The FRR config generation logic utilizes the local-as directive for specifying
+alternate ASN numbers. Since local-as is only applicable for eBGP sessions, the
+internal ASN number always needs to be used for the router definition. So if
+there are no EVPN controllers, but iBGP BGP sessions, utilize the ASN configured
+there.
+
+In other cases fallback to the BGP controller ASN, if there is no EVPN
+controller.
+
+Any configuration that has two iBGP sessions with different ASNs is rejected and
+an error thrown, since it is by definition not possible to have two iBGP
+sessions with different ASNs on the same BGP instance, as one instance can only
+have one local ASN.
+
+=cut
+
+sub get_default_router_asn {
+    my ($node_name, $controller_config) = @_;
+
+    my $legacy_asn = undef;
+    my $evpn_asn = undef;
+    my $ibgp_asn = undef;
+
+    my $bgp_controller = PVE::Network::SDN::Controllers::EvpnPlugin::find_bgp_controller(
+        $node_name, $controller_config,
+    );
+
+    if ($bgp_controller && !$bgp_controller->{ebgp}) {
+        $ibgp_asn = $bgp_controller->{asn};
+    }
+
+    for my $controller_id (sort keys $controller_config->{ids}->%*) {
+        my $controller = $controller_config->{ids}->{$controller_id};
+
+        next if $controller->{type} ne 'evpn';
+
+        if (defined($controller->{nodes})) {
+            my @nodes = PVE::Tools::split_list($controller->{nodes});
+            next if !grep { $_ eq $node_name } @nodes;
+        }
+
+        die "all EVPN controllers on a node must have the same ASN configured"
+            if defined($evpn_asn) && $evpn_asn ne $controller->{asn};
+
+        $evpn_asn = $controller->{asn};
+
+        my $bgp_mode = $controller->{'bgp-mode'} // 'legacy';
+        $legacy_asn = $bgp_controller->{asn} if $bgp_mode eq 'legacy' && $bgp_controller;
+
+        next if $bgp_mode eq 'external';
+
+        die "cannot have two different ASNs for iBGP sessions configured"
+            if defined($ibgp_asn) && $ibgp_asn ne $controller->{asn};
+
+        $ibgp_asn = $controller->{asn};
+    }
+
+    return $legacy_asn // $evpn_asn // $ibgp_asn // $bgp_controller->{asn};
+}
+
 1;
-- 
2.47.3