[pve-devel] [PATCH pve-network v5 04/21] refactor: controller: move frr methods into helper

[Gabriel Goller <g.goller@proxmox.com>]

From: Stefan Hanreich <s.hanreich@proxmox.com>

Up until now the EVPN controller contained all the helper methods as
well as the configuration generation logic for FRR. Since we need to
write FRR configuration with the fabrics as well, move the FRR helper
files into its own FRR module, so they can be used by the EVPN plugin
as well as the future fabrics plugins.

The fact that the EVPN controller was solely responsible for
generating the FRR config also meant, that FRR configuration was only
generated if you had an EVPN controller defined.

In the process of generating an FRR configuration, we used mainly two
formats, which I'll refer to by the following names:

frr_config: This is a perl hash, that loosely resembles the structure
of the FRR configuration file and was later converted into the
raw_config format before writing it.

raw_config: This is an array, that contains strings, where each string
is a line in the FRR configuration. So the finished FRR configuration
consists of all the strings in the array joined by newlines.

Controllers used the frr_config format for generating FRR
configuration. The local configuration in /etc/frr/frr.conf.local also
gets parsed into this format. The fabrics perlmod module, returns the
raw_config format. This was behind the intention to make this split
more clear and handle the FRR config generation in two steps from now
on:

* generate a frr_config in all plugins that utilize that format
* convert it to the raw_config format
* append the configuration obtained via perlmod
* write the finished configuration to frr.conf

This process was already in place, but the distinction wasn't that
clear. During this process I renamed all methods to make clear which
format they accept / return.

Some functions have been split to make them more granular, so we can
use intermediate results. Most namely the
generate_controller_rawconfig function has been split into multiple
functions.

Added documentation to all public FRR functions, so it is clearer
which format they expect, as well as which operations they perform on
the respective passed configurations.

For the future it might make sense to further split the FRR config
generation for zones and vnets into the respective Zone / VNet
Plugins, instead of in the EVPN controller, but this was beyond the
scope of this already quite large patch series.

Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 291 ---------------
 src/PVE/Network/SDN/Frr.pm                    | 335 ++++++++++++++++++
 src/PVE/Network/SDN/Makefile                  |   2 +-
 3 files changed, 336 insertions(+), 292 deletions(-)
 create mode 100644 src/PVE/Network/SDN/Frr.pm

diff --git a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
index f4a4b552ec44..4e71306499d2 100644
--- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
@@ -423,295 +423,4 @@ sub find_isis_controller {
     return $res;
 }
 
-sub generate_frr_recurse {
-    my ($final_config, $content, $parentkey, $level) = @_;
-
-    my $keylist = {};
-    $keylist->{'address-family'} = 1;
-    $keylist->{router} = 1;
-
-    my $exitkeylist = {};
-    $exitkeylist->{'address-family'} = 1;
-
-    my $simple_exitkeylist = {};
-    $simple_exitkeylist->{router} = 1;
-
-    # FIXME: make this generic
-    my $paddinglevel = undef;
-    if ($level == 1 || $level == 2) {
-        $paddinglevel = $level - 1;
-    } elsif ($level == 3 || $level == 4) {
-        $paddinglevel = $level - 2;
-    }
-
-    my $padding = "";
-    $padding = ' ' x ($paddinglevel) if $paddinglevel;
-
-    if (ref $content eq 'HASH') {
-        foreach my $key (sort keys %$content) {
-            next if $key eq 'vrf';
-            if ($parentkey && defined($keylist->{$parentkey})) {
-                push @{$final_config}, $padding . "!";
-                push @{$final_config}, $padding . "$parentkey $key";
-            } elsif ($key ne '' && !defined($keylist->{$key})) {
-                push @{$final_config}, $padding . "$key";
-            }
-
-            my $option = $content->{$key};
-            generate_frr_recurse($final_config, $option, $key, $level + 1);
-
-            push @{$final_config}, $padding . "exit-$parentkey"
-                if $parentkey && defined($exitkeylist->{$parentkey});
-            push @{$final_config}, $padding . "exit"
-                if $parentkey && defined($simple_exitkeylist->{$parentkey});
-        }
-    }
-
-    if (ref $content eq 'ARRAY') {
-        push @{$final_config}, map { $padding . "$_" } @$content;
-    }
-}
-
-sub generate_frr_vrf {
-    my ($final_config, $vrfs) = @_;
-
-    return if !$vrfs;
-
-    my @config = ();
-
-    foreach my $id (sort keys %$vrfs) {
-        my $vrf = $vrfs->{$id};
-        push @config, "!";
-        push @config, "vrf $id";
-        foreach my $rule (@$vrf) {
-            push @config, " $rule";
-
-        }
-        push @config, "exit-vrf";
-    }
-
-    push @{$final_config}, @config;
-}
-
-sub generate_frr_simple_list {
-    my ($final_config, $rules) = @_;
-
-    return if !$rules;
-
-    my @config = ();
-    push @{$final_config}, "!";
-    foreach my $rule (sort @$rules) {
-        push @{$final_config}, $rule;
-    }
-}
-
-sub generate_frr_interfaces {
-    my ($final_config, $interfaces) = @_;
-
-    foreach my $k (sort keys %$interfaces) {
-        my $iface = $interfaces->{$k};
-        push @{$final_config}, "!";
-        push @{$final_config}, "interface $k";
-        foreach my $rule (sort @$iface) {
-            push @{$final_config}, " $rule";
-        }
-    }
-}
-
-sub generate_frr_routemap {
-    my ($final_config, $routemaps) = @_;
-
-    foreach my $id (sort keys %$routemaps) {
-
-        my $routemap = $routemaps->{$id};
-        my $order = 0;
-        foreach my $seq (@$routemap) {
-            $order++;
-            next if !defined($seq->{action});
-            my @config = ();
-            push @config, "!";
-            push @config, "route-map $id $seq->{action} $order";
-            my $rule = $seq->{rule};
-            push @config, map { " $_" } @$rule;
-            push @{$final_config}, @config;
-            push @{$final_config}, "exit";
-        }
-    }
-}
-
-sub generate_frr_list {
-    my ($final_config, $lists, $type) = @_;
-
-    my $config = [];
-
-    for my $id (sort keys %$lists) {
-        my $list = $lists->{$id};
-
-        for my $seq (sort keys %$list) {
-            my $rule = $list->{$seq};
-            push @$config, "$type $id seq $seq $rule";
-        }
-    }
-
-    if (@$config > 0) {
-        push @{$final_config}, "!", @$config;
-    }
-}
-
-sub read_local_frr_config {
-    if (-e "/etc/frr/frr.conf.local") {
-        return file_get_contents("/etc/frr/frr.conf.local");
-    }
-}
-
-sub generate_controller_rawconfig {
-    my ($class, $plugin_config, $config) = @_;
-
-    my $nodename = PVE::INotify::nodename();
-
-    my $final_config = [];
-    push @{$final_config}, "frr version 8.5.2";
-    push @{$final_config}, "frr defaults datacenter";
-    push @{$final_config}, "hostname $nodename";
-    push @{$final_config}, "log syslog informational";
-    push @{$final_config}, "service integrated-vtysh-config";
-    push @{$final_config}, "!";
-
-    my $local_conf = read_local_frr_config();
-    if ($local_conf) {
-        parse_merge_frr_local_config($config, $local_conf);
-    }
-
-    generate_frr_vrf($final_config, $config->{frr}->{vrf});
-    generate_frr_interfaces($final_config, $config->{frr_interfaces});
-    generate_frr_recurse($final_config, $config->{frr}, undef, 0);
-    generate_frr_list($final_config, $config->{frr_access_list}, "access-list");
-    generate_frr_list($final_config, $config->{frr_prefix_list}, "ip prefix-list");
-    generate_frr_list($final_config, $config->{frr_prefix_list_v6}, "ipv6 prefix-list");
-    generate_frr_simple_list($final_config, $config->{frr_bgp_community_list});
-    generate_frr_routemap($final_config, $config->{frr_routemap});
-    generate_frr_simple_list($final_config, $config->{frr_ip_protocol});
-
-    push @{$final_config}, "!";
-    push @{$final_config}, "line vty";
-    push @{$final_config}, "!";
-
-    my $rawconfig = join("\n", @{$final_config});
-
-    return if !$rawconfig;
-    return $rawconfig;
-}
-
-sub parse_merge_frr_local_config {
-    my ($config, $local_conf) = @_;
-
-    my $section = \$config->{""};
-    my $router = undef;
-    my $routemap = undef;
-    my $routemap_config = ();
-    my $routemap_action = undef;
-
-    while ($local_conf =~ /^\s*(.+?)\s*$/gm) {
-        my $line = $1;
-        $line =~ s/^\s+|\s+$//g;
-
-        if ($line =~ m/^router (.+)$/) {
-            $router = $1;
-            $section = \$config->{'frr'}->{'router'}->{$router}->{""};
-            next;
-        } elsif ($line =~ m/^vrf (.+)$/) {
-            $section = \$config->{'frr'}->{'vrf'}->{$1};
-            next;
-        } elsif ($line =~ m/^interface (.+)$/) {
-            $section = \$config->{'frr_interfaces'}->{$1};
-            next;
-        } elsif ($line =~ m/^bgp community-list (.+)$/) {
-            push(@{ $config->{'frr_bgp_community_list'} }, $line);
-            next;
-        } elsif ($line =~ m/address-family (.+)$/) {
-            $section = \$config->{'frr'}->{'router'}->{$router}->{'address-family'}->{$1};
-            next;
-        } elsif ($line =~ m/^route-map (.+) (permit|deny) (\d+)/) {
-            $routemap = $1;
-            $routemap_config = ();
-            $routemap_action = $2;
-            $section = \$config->{'frr_routemap'}->{$routemap};
-            next;
-        } elsif ($line =~ m/^access-list (.+) seq (\d+) (.+)$/) {
-            $config->{'frr_access_list'}->{$1}->{$2} = $3;
-            next;
-        } elsif ($line =~ m/^ip prefix-list (.+) seq (\d+) (.*)$/) {
-            $config->{'frr_prefix_list'}->{$1}->{$2} = $3;
-            next;
-        } elsif ($line =~ m/^ipv6 prefix-list (.+) seq (\d+) (.*)$/) {
-            $config->{'frr_prefix_list_v6'}->{$1}->{$2} = $3;
-            next;
-        } elsif ($line =~ m/^exit-address-family$/) {
-            next;
-        } elsif ($line =~ m/^exit$/) {
-            if ($router) {
-                $section = \$config->{''};
-                $router = undef;
-            } elsif ($routemap) {
-                push(@{$$section}, { rule => $routemap_config, action => $routemap_action });
-                $section = \$config->{''};
-                $routemap = undef;
-                $routemap_action = undef;
-                $routemap_config = ();
-            }
-            next;
-        } elsif ($line =~ m/!/) {
-            next;
-        }
-
-        next if !$section;
-        if ($routemap) {
-            push(@{$routemap_config}, $line);
-        } else {
-            push(@{$$section}, $line);
-        }
-    }
-}
-
-sub write_controller_config {
-    my ($class, $plugin_config, $config) = @_;
-
-    my $rawconfig = $class->generate_controller_rawconfig($plugin_config, $config);
-    return if !$rawconfig;
-    return if !-d "/etc/frr";
-
-    file_set_contents("/etc/frr/frr.conf", $rawconfig);
-}
-
-sub reload_controller {
-    my ($class) = @_;
-
-    my $conf_file = "/etc/frr/frr.conf";
-    my $bin_path = "/usr/lib/frr/frr-reload.py";
-
-    if (!-e $bin_path) {
-        log_warn("missing $bin_path. Please install frr-pythontools package");
-        return;
-    }
-
-    run_command(['systemctl', 'enable', '--now', 'frr'])
-        if !-e "/etc/systemd/system/multi-user.target.wants/frr.service";
-
-    my $err = sub {
-        my $line = shift;
-        if ($line =~ /ERROR:/) {
-            warn "$line \n";
-        }
-    };
-
-    if (-e $conf_file && -e $bin_path) {
-        eval { run_command([$bin_path, '--stdout', '--reload', $conf_file], errfunc => $err); };
-        if ($@) {
-            warn "frr reload command fail. Restarting frr.";
-            eval { run_command(['systemctl', 'restart', 'frr']); };
-        }
-    }
-}
-
 1;
-
diff --git a/src/PVE/Network/SDN/Frr.pm b/src/PVE/Network/SDN/Frr.pm
new file mode 100644
index 000000000000..9c6e91c94967
--- /dev/null
+++ b/src/PVE/Network/SDN/Frr.pm
@@ -0,0 +1,335 @@
+package PVE::Network::SDN::Frr;
+
+use strict;
+use warnings;
+
+=head1 NAME
+
+C<PVE::Network::SDN::Frr> - Helper module for FRR
+
+=head1 DESCRIPTION
+
+This module contains helpers for handling the various intermediate FRR
+configuration formats.
+
+We currently mainly use two different intermediate formats throughout the SDN
+module:
+
+=head2 frr config
+
+An frr config represented as a perl hash. The controller plugins generate their
+frr configuration in this format. This format is also used for merging the local
+FRR config (a user-defined configuration file) with the controller-generated
+configuration.
+
+=head2 raw config
+
+This is generated from the frr config. It is an array where every entry is a
+string that is a FRR configuration line.
+
+=cut
+
+use PVE::RESTEnvironment qw(log_warn);
+use PVE::Tools qw(file_get_contents file_set_contents run_command);
+
+=head3 read_local_frr_config
+
+Returns the contents of `/etc/frr/frr.conf.local` as a string if it exists, otherwise undef.
+
+=cut
+
+sub read_local_frr_config {
+    if (-e "/etc/frr/frr.conf.local") {
+        return file_get_contents("/etc/frr/frr.conf.local");
+    }
+}
+
+=head3 to_raw_config(\%frr_config)
+
+Converts a given C<\%frr_config> to the raw config format.
+
+=cut
+
+sub to_raw_config {
+    my ($frr_config) = @_;
+
+    my $raw_config = [];
+
+    generate_frr_vrf($raw_config, $frr_config->{frr}->{vrf});
+    generate_frr_interfaces($raw_config, $frr_config->{frr_interfaces});
+    generate_frr_recurse($raw_config, $frr_config->{frr}, undef, 0);
+    generate_frr_list($raw_config, $frr_config->{frr_access_list}, "access-list");
+    generate_frr_list($raw_config, $frr_config->{frr_prefix_list}, "ip prefix-list");
+    generate_frr_list($raw_config, $frr_config->{frr_prefix_list_v6}, "ipv6 prefix-list");
+    generate_frr_simple_list($raw_config, $frr_config->{frr_bgp_community_list});
+    generate_frr_routemap($raw_config, $frr_config->{frr_routemap});
+    generate_frr_simple_list($raw_config, $frr_config->{frr_ip_protocol});
+
+    return $raw_config;
+}
+
+=head3 raw_config_to_string(\@raw_config)
+
+Converts a given C<\@raw_config> to a string representing a complete frr
+configuration, ready to be written to /etc/frr/frr.conf. If raw_config is empty,
+returns only the FRR config skeleton.
+
+=cut
+
+sub raw_config_to_string {
+    my ($raw_config) = @_;
+
+    my $nodename = PVE::INotify::nodename();
+
+    my @final_config = (
+        "frr version 8.5.2",
+        "frr defaults datacenter",
+        "hostname $nodename",
+        "log syslog informational",
+        "service integrated-vtysh-config",
+        "!",
+    );
+
+    push @final_config, @$raw_config;
+
+    push @final_config, (
+        "!", "line vty", "!",
+    );
+
+    return join("\n", @final_config);
+}
+
+=head3 raw_config_to_string(\@raw_config)
+
+Writes a given C<\@raw_config> to /etc/frr/frr.conf.
+
+=cut
+
+sub write_raw_config {
+    my ($raw_config) = @_;
+
+    return if !-d "/etc/frr";
+    return if !$raw_config;
+
+    file_set_contents("/etc/frr/frr.conf", raw_config_to_string($raw_config));
+
+}
+
+=head3 append_local_config(\%frr_config, $local_config)
+
+Takes an existing C<\%frr_config> and C<$local_config> (as a string). It parses
+the local configuration and appends the values to the existing C<\%frr_config>
+in-place.
+
+=cut
+
+sub append_local_config {
+    my ($frr_config, $local_config) = @_;
+
+    $local_config = read_local_frr_config() if !$local_config;
+    return if !$local_config;
+
+    my $section = \$frr_config->{""};
+    my $router = undef;
+    my $routemap = undef;
+    my $routemap_config = ();
+    my $routemap_action = undef;
+
+    while ($local_config =~ /^\s*(.+?)\s*$/gm) {
+        my $line = $1;
+        $line =~ s/^\s+|\s+$//g;
+
+        if ($line =~ m/^router (.+)$/) {
+            $router = $1;
+            $section = \$frr_config->{'frr'}->{'router'}->{$router}->{""};
+            next;
+        } elsif ($line =~ m/^vrf (.+)$/) {
+            $section = \$frr_config->{'frr'}->{'vrf'}->{$1};
+            next;
+        } elsif ($line =~ m/^interface (.+)$/) {
+            $section = \$frr_config->{'frr_interfaces'}->{$1};
+            next;
+        } elsif ($line =~ m/^bgp community-list (.+)$/) {
+            push(@{ $frr_config->{'frr_bgp_community_list'} }, $line);
+            next;
+        } elsif ($line =~ m/address-family (.+)$/) {
+            $section = \$frr_config->{'frr'}->{'router'}->{$router}->{'address-family'}->{$1};
+            next;
+        } elsif ($line =~ m/^route-map (.+) (permit|deny) (\d+)/) {
+            $routemap = $1;
+            $routemap_config = ();
+            $routemap_action = $2;
+            $section = \$frr_config->{'frr_routemap'}->{$routemap};
+            next;
+        } elsif ($line =~ m/^access-list (.+) seq (\d+) (.+)$/) {
+            $frr_config->{'frr_access_list'}->{$1}->{$2} = $3;
+            next;
+        } elsif ($line =~ m/^ip prefix-list (.+) seq (\d+) (.*)$/) {
+            $frr_config->{'frr_prefix_list'}->{$1}->{$2} = $3;
+            next;
+        } elsif ($line =~ m/^ipv6 prefix-list (.+) seq (\d+) (.*)$/) {
+            $frr_config->{'frr_prefix_list_v6'}->{$1}->{$2} = $3;
+            next;
+        } elsif ($line =~ m/^exit-address-family$/) {
+            next;
+        } elsif ($line =~ m/^exit$/) {
+            if ($router) {
+                $section = \$frr_config->{''};
+                $router = undef;
+            } elsif ($routemap) {
+                push(@{$$section}, { rule => $routemap_config, action => $routemap_action });
+                $section = \$frr_config->{''};
+                $routemap = undef;
+                $routemap_action = undef;
+                $routemap_config = ();
+            }
+            next;
+        } elsif ($line =~ m/!/) {
+            next;
+        }
+
+        next if !$section;
+        if ($routemap) {
+            push(@{$routemap_config}, $line);
+        } else {
+            push(@{$$section}, $line);
+        }
+    }
+}
+
+sub generate_frr_recurse {
+    my ($final_config, $content, $parentkey, $level) = @_;
+
+    my $keylist = {};
+    $keylist->{'address-family'} = 1;
+    $keylist->{router} = 1;
+
+    my $exitkeylist = {};
+    $exitkeylist->{'address-family'} = 1;
+
+    my $simple_exitkeylist = {};
+    $simple_exitkeylist->{router} = 1;
+
+    # FIXME: make this generic
+    my $paddinglevel = undef;
+    if ($level == 1 || $level == 2) {
+        $paddinglevel = $level - 1;
+    } elsif ($level == 3 || $level == 4) {
+        $paddinglevel = $level - 2;
+    }
+
+    my $padding = "";
+    $padding = ' ' x ($paddinglevel) if $paddinglevel;
+
+    if (ref $content eq 'HASH') {
+        foreach my $key (sort keys %$content) {
+            next if $key eq 'vrf';
+            if ($parentkey && defined($keylist->{$parentkey})) {
+                push @{$final_config}, $padding . "!";
+                push @{$final_config}, $padding . "$parentkey $key";
+            } elsif ($key ne '' && !defined($keylist->{$key})) {
+                push @{$final_config}, $padding . "$key";
+            }
+
+            my $option = $content->{$key};
+            generate_frr_recurse($final_config, $option, $key, $level + 1);
+
+            push @{$final_config}, $padding . "exit-$parentkey"
+                if $parentkey && defined($exitkeylist->{$parentkey});
+            push @{$final_config}, $padding . "exit"
+                if $parentkey && defined($simple_exitkeylist->{$parentkey});
+        }
+    }
+
+    if (ref $content eq 'ARRAY') {
+        push @{$final_config}, map { $padding . "$_" } @$content;
+    }
+}
+
+sub generate_frr_vrf {
+    my ($final_config, $vrfs) = @_;
+
+    return if !$vrfs;
+
+    my @config = ();
+
+    foreach my $id (sort keys %$vrfs) {
+        my $vrf = $vrfs->{$id};
+        push @config, "!";
+        push @config, "vrf $id";
+        foreach my $rule (@$vrf) {
+            push @config, " $rule";
+
+        }
+        push @config, "exit-vrf";
+    }
+
+    push @{$final_config}, @config;
+}
+
+sub generate_frr_simple_list {
+    my ($final_config, $rules) = @_;
+
+    return if !$rules;
+
+    my @config = ();
+    push @{$final_config}, "!";
+    foreach my $rule (sort @$rules) {
+        push @{$final_config}, $rule;
+    }
+}
+
+sub generate_frr_list {
+    my ($final_config, $lists, $type) = @_;
+
+    my $config = [];
+
+    for my $id (sort keys %$lists) {
+        my $list = $lists->{$id};
+
+        for my $seq (sort keys %$list) {
+            my $rule = $list->{$seq};
+            push @$config, "$type $id seq $seq $rule";
+        }
+    }
+
+    if (@$config > 0) {
+        push @{$final_config}, "!", @$config;
+    }
+}
+
+sub generate_frr_interfaces {
+    my ($final_config, $interfaces) = @_;
+
+    foreach my $k (sort keys %$interfaces) {
+        my $iface = $interfaces->{$k};
+        push @{$final_config}, "!";
+        push @{$final_config}, "interface $k";
+        foreach my $rule (sort @$iface) {
+            push @{$final_config}, " $rule";
+        }
+    }
+}
+
+sub generate_frr_routemap {
+    my ($final_config, $routemaps) = @_;
+
+    foreach my $id (sort keys %$routemaps) {
+
+        my $routemap = $routemaps->{$id};
+        my $order = 0;
+        foreach my $seq (@$routemap) {
+            $order++;
+            next if !defined($seq->{action});
+            my @config = ();
+            push @config, "!";
+            push @config, "route-map $id $seq->{action} $order";
+            my $rule = $seq->{rule};
+            push @config, map { " $_" } @$rule;
+            push @{$final_config}, @config;
+            push @{$final_config}, "exit";
+        }
+    }
+}
+
+1;
diff --git a/src/PVE/Network/SDN/Makefile b/src/PVE/Network/SDN/Makefile
index a256642e3044..d1ffef9eebe7 100644
--- a/src/PVE/Network/SDN/Makefile
+++ b/src/PVE/Network/SDN/Makefile
@@ -1,4 +1,4 @@
-SOURCES=Vnets.pm VnetPlugin.pm Zones.pm Controllers.pm Subnets.pm SubnetPlugin.pm Ipams.pm Dns.pm Dhcp.pm Fabrics.pm
+SOURCES=Vnets.pm VnetPlugin.pm Zones.pm Controllers.pm Subnets.pm SubnetPlugin.pm Ipams.pm Dns.pm Dhcp.pm Fabrics.pm Frr.pm
 
 
 PERL5DIR=${DESTDIR}/usr/share/perl5
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel