From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 3EEB01FF0AE for ; Tue, 13 Oct 2026 14:32:53 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 499922151E; Tue, 13 Oct 2026 14:32:37 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.10.3 smtp.infra.comaite.com 38D8562C0A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comaite.com; s=377A38C6-3A8D-11E5-A0EC-E97757B27CB3; t=1791893673; bh=pn9bJWhitCrCjQ/vhexrWYdcXDgTMMyDg0sJH9oMHss=; h=From:To:Date:Message-ID:MIME-Version; b=SFzC7Qj1ufCz/ddGH24jNI2O7nw6cYcZiq2jgsn9ze1S6HfzKfE3/r9Bwv/IZjW0N 32+4Jj8z8LRUlq8/jHJOE9pdA4UlLHM0H5DrhulkYXS17mBhuTMUgb2ajpmlxRE979 /uEHxMKqrnFl3PniuAYhPOktA+qkpX2ufhhlcsfIeZ4J7tgdSkDiZfaKUzJ89fqbc0 1LYPmLsPr9JTQfDaOGsJrYey/gbeSM5QUuxQ1VeXTx/n2NseeiYlrb1AwDi3DZTvSa 1XfsM3pfNc3xUazs9E5Znq6IjurH50lzZB8rO5UPN8dxQcnmyjmrNrlW61a4Dq37Gp aJF7Vu/xg1Pmw== From: =?UTF-8?q?R=C3=A9mi=20PAETA?= To: pve-devel@lists.proxmox.com Subject: [RFC PATCH network 04/16] sdn: ovn: sync the northbound database on reload Date: Tue, 13 Oct 2026 14:14:20 +0200 Message-ID: <20261013121432.2642979-5-remi.paeta@comaite.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261013121432.2642979-1-remi.paeta@comaite.com> References: <1116090294.1714263.1791537779868.JavaMail.zimbra@comaite.com> <20261013121432.2642979-1-remi.paeta@comaite.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 2 AWL -2.500 Adjusted score from AWL reputation of From: address DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_DBL_SPAM 5 Contains a spam URL listed in the Spamhaus DBL blocklist [sdn.pm] Message-ID-Hash: W6TBUH24XI35PHZN2CELIK4NASYO3FRI X-Message-ID-Hash: W6TBUH24XI35PHZN2CELIK4NASYO3FRI X-MailFrom: remi.paeta@comaite.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The logical switches of the vnets get created, updated and deleted when the SDN configuration is applied. Since the northbound database is shared by all nodes, this is done once for the whole cluster in the reload API call, before the per-node reload workers get started. Only objects tagged with the pve-sdn-* external_ids are touched, so an OVN deployment can be shared with other consumers. A logical switch named like a vnet but not managed by PVE is refused instead of taken over. Ports of guests that do not exist anymore get removed, as do all managed objects of an OVN controller that was removed from the configuration. Ports whose interface is not on their vnet in the guest configuration anymore, because it got removed or moved to another bridge, get removed too, unless they are still up: on hotplug, the port gets created before the interface is written to the guest configuration. Signed-off-by: Rémi PAETA --- src/PVE/API2/Network/SDN.pm | 8 ++ src/PVE/Network/SDN.pm | 16 +++ src/PVE/Network/SDN/Ovn.pm | 268 ++++++++++++++++++++++++++++++++++++ src/test/run_test_ovn.pl | 206 +++++++++++++++++++++++++++ 4 files changed, 498 insertions(+) diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm index e3c8d9d..c4522ad 100644 --- a/src/PVE/API2/Network/SDN.pm +++ b/src/PVE/API2/Network/SDN.pm @@ -318,9 +318,11 @@ __PACKAGE__->register_method({ my $previous_config_has_frr; my $new_config_has_frr; + my $previous_running_config; PVE::Network::SDN::lock_sdn_config( sub { + $previous_running_config = PVE::Network::SDN::running_config(); $previous_config_has_frr = PVE::Network::SDN::running_config_has_frr(); PVE::Network::SDN::commit_config(); $new_config_has_frr = PVE::Network::SDN::running_config_has_frr(); @@ -338,6 +340,12 @@ __PACKAGE__->register_method({ my $code = sub { $rpcenv->{type} = 'priv'; # to start tasks in background PVE::Cluster::check_cfs_quorum(); + + # the OVN northbound database is shared by all nodes, so it is + # synced once for the whole cluster + eval { PVE::Network::SDN::sync_ovn_northbound($previous_running_config) }; + warn "syncing OVN northbound database failed - $@" if $@; + my $nodelist = PVE::Cluster::get_nodelist(); for my $node (@$nodelist) { my $pid = eval { $create_reload_network_worker->($node, $regenerate_frr) }; diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm index aeb9b4c..d601cd0 100644 --- a/src/PVE/Network/SDN.pm +++ b/src/PVE/Network/SDN.pm @@ -25,6 +25,7 @@ use PVE::Network::SDN::Subnets; use PVE::Network::SDN::Dhcp; use PVE::Network::SDN::Frr; use PVE::Network::SDN::Fabrics; +use PVE::Network::SDN::Ovn; use PVE::Network::SDN::RouteMaps; use PVE::Network::SDN::PrefixLists; @@ -503,6 +504,21 @@ sub generate_dhcp_config { PVE::Network::SDN::Dhcp::regenerate_config($reload); } +=head3 sync_ovn_northbound(\%previous_running_config) + +Reconciles the OVN northbound databases with the current running +configuration. C<\%previous_running_config> is the running configuration before +the last commit, so that the objects of removed OVN controllers get cleaned up. + +=cut + +sub sync_ovn_northbound { + my ($previous_running_config) = @_; + + my $running_config = PVE::Network::SDN::running_config(); + PVE::Network::SDN::Ovn::sync_northbound($previous_running_config, $running_config); +} + sub encode_value { my ($type, $key, $value) = @_; diff --git a/src/PVE/Network/SDN/Ovn.pm b/src/PVE/Network/SDN/Ovn.pm index 93235e8..142477b 100644 --- a/src/PVE/Network/SDN/Ovn.pm +++ b/src/PVE/Network/SDN/Ovn.pm @@ -2,6 +2,9 @@ package PVE::Network::SDN::Ovn; use v5.36; +use JSON; + +use PVE::Cluster; use PVE::Network; use PVE::Tools qw(run_command); @@ -19,6 +22,180 @@ my $EXT_ZONE = 'pve-sdn-zone'; my $EXT_VNET = 'pve-sdn-vnet'; my $EXT_VMID = 'pve-sdn-vmid'; +=head3 ovn_controllers(\%running_cfg) + +Returns a hash of all controllers of type C in C<\%running_cfg>, keyed by +their ID. + +=cut + +sub ovn_controllers { + my ($running_cfg) = @_; + + my $controllers = $running_cfg->{controllers}->{ids} // {}; + + return { + map { $_ => $controllers->{$_} } + grep { $controllers->{$_}->{type} eq 'ovn' } keys %$controllers + }; +} + +=head3 desired_state(\%running_cfg) + +Computes the northbound objects that should exist for C<\%running_cfg>, keyed by +OVN controller ID. Every vnet of an C zone maps to a logical switch of the +same name. + +=cut + +sub desired_state { + my ($running_cfg) = @_; + + my $zones = $running_cfg->{zones}->{ids} // {}; + my $vnets = $running_cfg->{vnets}->{ids} // {}; + + my $state = {}; + $state->{$_} = { switches => {} } for keys ovn_controllers($running_cfg)->%*; + + for my $vnetid (sort keys %$vnets) { + my $zoneid = $vnets->{$vnetid}->{zone} or next; + my $zone = $zones->{$zoneid} or next; + next if $zone->{type} ne 'ovn'; + + my $controllerid = $zone->{controller}; + next if !$controllerid || !$state->{$controllerid}; + + $state->{$controllerid}->{switches}->{$vnetid} = { + external_ids => { + $EXT_CONTROLLER => $controllerid, + $EXT_ZONE => $zoneid, + $EXT_VNET => $vnetid, + }, + }; + } + + return $state; +} + +=head3 sync_commands($controllerid, \%desired, \@switches, \@ports, \%vmlist, \%nics) + +Returns the list of C commands (each an array reference) that turn +the current northbound content (C<\@switches> and C<\@ports>, as returned by +C) into C<\%desired>, the entry of C for +C<$controllerid>. + +Ports of guests that are not in C<\%vmlist> anymore get removed too. + +If C<\%nics> is given, as returned by C, ports of guest +interfaces that are not on their vnet anymore get removed too, unless they are +still up, as a port gets created right before its interface is written to the +guest config on hotplug. + +=cut + +sub sync_commands { + my ($controllerid, $desired, $switches, $ports, $vmlist, $nics) = @_; + + my $wanted = $desired->{switches} // {}; + + my $managed = {}; + for my $switch (@$switches) { + my $name = $switch->{name}; + my $external_ids = $switch->{external_ids} // {}; + + if (($external_ids->{$EXT_CONTROLLER} // '') ne $controllerid) { + if ($wanted->{$name}) { + die "logical switch '$name' already exists in the OVN northbound database, but" + . " is not managed by SDN controller '$controllerid'\n"; + } + next; + } + + $managed->{$name} = $external_ids; + } + + my $commands = []; + + for my $name (sort keys %$managed) { + push @$commands, ['ls-del', $name] if !$wanted->{$name}; + } + + for my $name (sort keys %$wanted) { + my $external_ids = $wanted->{$name}->{external_ids}; + + if (!$managed->{$name}) { + push @$commands, ['ls-add', $name]; + } elsif ( + !grep { ($managed->{$name}->{$_} // '') ne $external_ids->{$_} } + keys %$external_ids + ) { + next; + } + + push @$commands, + [ + 'set', + 'Logical_Switch', + $name, + map { "external_ids:$_=$external_ids->{$_}" } sort keys %$external_ids, + ]; + } + + for my $port (sort { $a->{name} cmp $b->{name} } @$ports) { + my $external_ids = $port->{external_ids} // {}; + next if ($external_ids->{$EXT_CONTROLLER} // '') ne $controllerid; + + # ports of deleted switches are removed together with their switch + my $vnetid = $external_ids->{$EXT_VNET}; + next if defined($vnetid) && $managed->{$vnetid} && !$wanted->{$vnetid}; + + my $vmid = $external_ids->{$EXT_VMID}; + if (defined($vmid) && !$vmlist->{$vmid}) { + push @$commands, ['lsp-del', $port->{name}]; + next; + } + + if (defined($vmid) && $nics && $port->{name} =~ m/^(?:tap|veth)\d+i(\d+)$/) { + my $bridge = $nics->{$vmid}->{"net$1"}; + if ((!defined($bridge) || $bridge ne ($vnetid // '')) && !ovsdb_true($port->{up})) { + push @$commands, ['lsp-del', $port->{name}]; + next; + } + } + } + + return $commands; +} + +# optional booleans are an empty set when unset +sub ovsdb_true { + my ($value) = @_; + + $value = $value->[0] if ref($value) eq 'ARRAY'; + return $value ? 1 : 0; +} + +=head3 guest_nics() + +Returns the bridge of every network interface of all guests, keyed by VMID and +interface (C, C, ...). + +=cut + +sub guest_nics { + my $properties = PVE::Cluster::get_guest_config_properties([map { "net$_" } 0 .. 31]); + + my $nics = {}; + for my $vmid (keys %$properties) { + for my $net (keys $properties->{$vmid}->%*) { + my ($bridge) = $properties->{$vmid}->{$net} =~ m/(?:^|,)bridge=([^,]+)/; + $nics->{$vmid}->{$net} = $bridge if defined($bridge); + } + } + + return $nics; +} + sub nbctl { my ($controller, @args) = @_; @@ -47,6 +224,97 @@ sub nbctl_transaction { nbctl($controller, map { ('--', @$_) } @$commands); } +# converts an OVSDB JSON value (RFC 7047) to plain perl data +sub ovsdb_to_perl { + my ($value) = @_; + + return $value if ref($value) ne 'ARRAY'; + + my ($type, $data) = @$value; + return { map { $_->[0] => $_->[1] } @$data } if $type eq 'map'; + return [map { ovsdb_to_perl($_) } @$data] if $type eq 'set'; + return $data; # uuid +} + +=head3 list_rows($controller, $table, @columns) + +Returns all rows of the northbound C<$table> as list of hashes containing +C<@columns>. + +=cut + +sub list_rows { + my ($controller, $table, @columns) = @_; + + my $output = nbctl( + $controller, + '--format=json', + '--data=json', + '--columns=' . join(',', @columns), + 'list', + $table, + ); + my $result = decode_json($output); + + my @headings = $result->{headings}->@*; + + my $rows = []; + for my $data ($result->{data}->@*) { + push @$rows, { map { $headings[$_] => ovsdb_to_perl($data->[$_]) } 0 .. $#headings }; + } + + return $rows; +} + +=head3 sync_northbound(\%previous_cfg, \%running_cfg) + +Reconciles the northbound database of every OVN controller with +C<\%running_cfg>. Controllers only present in C<\%previous_cfg> get all their +managed objects removed. + +Each controller is synced independently, errors are collected and raised at the +end. + +=cut + +sub sync_northbound { + my ($previous_cfg, $running_cfg) = @_; + + my $desired = desired_state($running_cfg); + my $controllers = ovn_controllers($running_cfg); + + my $previous_controllers = ovn_controllers($previous_cfg // {}); + for my $id (keys %$previous_controllers) { + next if $controllers->{$id}; + $controllers->{$id} = $previous_controllers->{$id}; + $desired->{$id} = { switches => {} }; + } + + return if !%$controllers; + + my $vmlist = PVE::Cluster::get_vmlist()->{ids} // {}; + my $guest_nics = guest_nics(); + + my @errors; + for my $id (sort keys %$controllers) { + my $controller = $controllers->{$id}; + + eval { + my $switches = list_rows($controller, 'Logical_Switch', 'name', 'external_ids'); + my $ports = list_rows( + $controller, 'Logical_Switch_Port', 'name', 'external_ids', 'up', + ); + + my $commands = + sync_commands($id, $desired->{$id}, $switches, $ports, $vmlist, $guest_nics); + nbctl_transaction($controller, $commands); + }; + push @errors, "OVN controller '$id': $@" if $@; + } + + die join('', @errors) if @errors; +} + =head3 port_external_ids($controllerid, $zoneid, $vnetid, $iface) Returns the external_ids identifying the logical switch port of the guest diff --git a/src/test/run_test_ovn.pl b/src/test/run_test_ovn.pl index 411c60e..b12ca0b 100755 --- a/src/test/run_test_ovn.pl +++ b/src/test/run_test_ovn.pl @@ -5,7 +5,9 @@ use warnings; use lib qw(..); +use JSON; use Test::More; +use Test::MockModule; use PVE::Network::SDN; use PVE::Network::SDN::Ovn; @@ -57,6 +59,163 @@ sub ext { return $ext; } +sub set_switch { + my ($vnet, $zone) = @_; + return [ + 'set', + 'Logical_Switch', + $vnet, + 'external_ids:pve-sdn-controller=ovnctl', + "external_ids:pve-sdn-vnet=$vnet", + "external_ids:pve-sdn-zone=$zone", + ]; +} + +# desired_state + +my $desired = PVE::Network::SDN::Ovn::desired_state($running_cfg); +is_deeply( + $desired, + { + ovnctl => { + switches => { + vnet1 => { external_ids => ext('zone1', 'vnet1') }, + vnet2 => { external_ids => ext('zone2', 'vnet2') }, + }, + }, + }, + 'desired_state: one logical switch per ovn vnet', +); + +is_deeply( + PVE::Network::SDN::Ovn::desired_state({}), {}, 'desired_state: empty running config', +); + +# sync_commands + +is_deeply( + PVE::Network::SDN::Ovn::sync_commands('ovnctl', $desired->{ovnctl}, [], [], {}), + [ + ['ls-add', 'vnet1'], + set_switch('vnet1', 'zone1'), + ['ls-add', 'vnet2'], + set_switch('vnet2', 'zone2'), + ], + 'sync_commands: create missing switches', +); + +my $switches = [ + { name => 'vnet1', external_ids => ext('zone1', 'vnet1') }, + { name => 'vnet2', external_ids => ext('oldzone', 'vnet2') }, + { name => 'oldvnet', external_ids => ext('zone1', 'oldvnet') }, + { name => 'foreign', external_ids => { 'neutron:network_name' => 'foo' } }, +]; +my $ports = [ + { name => 'tap100i0', external_ids => ext('zone1', 'vnet1', 100) }, + { name => 'tap101i0', external_ids => ext('zone1', 'vnet1', 101) }, + { name => 'tap102i0', external_ids => ext('zone1', 'oldvnet', 102) }, + { name => 'manual', external_ids => ext('zone1', 'vnet1') }, + { name => 'foreignport', external_ids => {} }, +]; + +is_deeply( + PVE::Network::SDN::Ovn::sync_commands( + 'ovnctl', + $desired->{ovnctl}, + $switches, + $ports, + { 100 => {} }, + ), + [['ls-del', 'oldvnet'], set_switch('vnet2', 'zone2'), ['lsp-del', 'tap101i0']], + 'sync_commands: delete, update and garbage collect, leave the rest alone', +); + +eval { + PVE::Network::SDN::Ovn::sync_commands( + 'ovnctl', + $desired->{ovnctl}, + [{ name => 'vnet1', external_ids => {} }], + [], + {}, + ); +}; +like($@, qr/logical switch 'vnet1' already exists/, 'sync_commands: refuse unmanaged switch'); + +is_deeply( + PVE::Network::SDN::Ovn::sync_commands( + 'ovnctl', + { switches => {} }, + [{ name => 'vnet1', external_ids => ext('zone1', 'vnet1') }], + [{ name => 'tap100i0', external_ids => ext('zone1', 'vnet1', 100) }], + {}, + ), + [['ls-del', 'vnet1']], + 'sync_commands: deleted switch takes its ports along', +); + +# sync_northbound, with ovn-nbctl mocked + +my $nb_switches = { + ovnctl => [[ + 'vnet1', + [ + 'map', + [map { [$_, ext('zone1', 'vnet1')->{$_}] } sort keys ext('zone1', 'vnet1')->%*], + ], + ]], + gonectl => [['gone', ['map', [['pve-sdn-controller', 'gonectl']]]]], +}; + +my $nbctl_calls = []; +my $ovn_module = Test::MockModule->new('PVE::Network::SDN::Ovn'); +$ovn_module->mock( + run_command => sub { + my ($cmd, %param) = @_; + + die "unexpected command @$cmd\n" if $cmd->[0] ne $PVE::Network::SDN::Ovn::NBCTL; + my ($db) = $cmd->[1] =~ m/^--db=tcp:192\.168\.0\.(\d+)/; + my $id = $db == 1 ? 'ovnctl' : 'gonectl'; + + my @args = $cmd->@[3 .. $#$cmd]; + if ($args[-2] eq 'list') { + my $data = $args[-1] eq 'Logical_Switch' ? $nb_switches->{$id} : []; + $param{outfunc} + ->(encode_json({ headings => ['name', 'external_ids'], data => $data })); + } else { + push @$nbctl_calls, [$id, @args]; + } + }, +); +my $cluster_module = Test::MockModule->new('PVE::Cluster'); +$cluster_module->mock( + get_vmlist => sub { return { ids => {} } }, + get_guest_config_properties => sub { return {} }, +); + +my $previous_cfg = { + controllers => { + ids => { + gonectl => { + type => 'ovn', + 'nb-connection' => 'tcp:192.168.0.9:6641', + 'sb-connection' => 'tcp:192.168.0.9:6642', + }, + }, + }, +}; + +PVE::Network::SDN::Ovn::sync_northbound($previous_cfg, $running_cfg); +is_deeply( + $nbctl_calls, + [ + ['gonectl', '--', 'ls-del', 'gone'], + ['ovnctl', '--', 'ls-add', 'vnet2', '--', set_switch('vnet2', 'zone2')->@*], + ], + 'sync_northbound: one transaction per controller, removed controllers get cleaned up', +); + +$ovn_module->unmock('run_command'); + # ports is_deeply( @@ -130,6 +289,53 @@ is( 'firewall_port_name: container', ); +# guest interfaces removed from their vnet + +is_deeply( + PVE::Network::SDN::Ovn::sync_commands( + 'ovnctl', + $desired->{ovnctl}, + [{ name => 'vnet1', external_ids => ext('zone1', 'vnet1') }], + [ + { + name => 'tap100i0', + external_ids => ext('zone1', 'vnet1', 100), + up => ['set', []]->[1], + }, + { name => 'tap100i1', external_ids => ext('zone1', 'vnet1', 100), up => 0 }, + { name => 'tap100i2', external_ids => ext('zone1', 'vnet1', 100), up => 0 }, + { name => 'tap100i3', external_ids => ext('zone1', 'vnet1', 100), up => 1 }, + ], + { 100 => {} }, + { 100 => { net0 => 'vnet1', net1 => 'vmbr0' } }, + ), + [ + ['ls-add', 'vnet2'], + set_switch('vnet2', 'zone2'), + ['lsp-del', 'tap100i1'], + ['lsp-del', 'tap100i2'], + ], + 'sync_commands: remove ports of interfaces moved or removed, unless still up', +); + +ok(!PVE::Network::SDN::Ovn::ovsdb_true([]), 'ovsdb_true: unset optional boolean'); +ok(PVE::Network::SDN::Ovn::ovsdb_true(JSON::true), 'ovsdb_true: true'); + +$cluster_module->mock( + get_guest_config_properties => sub { + return { + 100 => { net0 => 'virtio=BC:24:11:00:00:01,bridge=vnet1,firewall=1' }, + 101 => { net0 => 'name=eth0,bridge=vmbr0,hwaddr=BC:24:11:00:00:02,type=veth' }, + 102 => { net1 => 'virtio=BC:24:11:00:00:03' }, + }; + }, +); +is_deeply( + PVE::Network::SDN::Ovn::guest_nics(), + { 100 => { net0 => 'vnet1' }, 101 => { net0 => 'vmbr0' } }, + 'guest_nics: bridges of the guest interfaces', +); + # unsupported options eval { -- 2.47.3