Re: [pve-devel] [PATCH V3 qemu-server 3/3] migration : add del_nets_bridge_fdb

2022-11-10 Thread DERUMIER, Alexandre 
Le lundi 07 novembre 2022 à 13:41 +0100, Mira Limbeck a écrit :
> 
> I'm currently going through this patch series
Thanks for looking at it ! 

>  and I'm wondering. 
> Wouldn't it make more sense to delete the bridge fdb entries as part
> of 
> the VM shutdown instead?
> 
> This way those would be cleaned up whenever the VM is stopped (and
> the 
> tap device gets destroyed). No special handling for migrations
> needed, 
> unless I'm missing something?
> 

Well, the fdb entry is already auto-deleted when the tap in unplug from
the bridge. (so when qemu process is stopped too)

We want the fdb entries only present in the bridge on active vm.
(some advanced controlplane like evpn, also blocking the write of
duplicated mac on multiple nodes)

as we resume the targetvm before stopping the sourcevm, we need to
remove fdb entries on source bridge manually.




The workflow is:


1) original vm is started, fbd entries are written in bridge

2) start migration, destination vm is started paused without writting
fdb entries   (I think I don't implement this currently, I need to
verify)

3) At the end of the migration, when sourcevm is stopped, before the
resume of target vm:

  - fdb entries are removed on source vm bridge

  - fdb entries are added in destination vm bridge

  - targetvm is resumed

  - sourcevm is stopped





I'll review and retest the full patch series tomorrow
> 
> 
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=Zk92VEFKaGQ4Ums4cnZEUWMTpfHaXFQGRw1_CnOoOH0=bHA1dGV3NWJQVUloaWNFUZPm0fiiBviaiy_RDav2GQ1U4uy6lsDDv3uBszpvvWYQN5FqKqFD6WPYupfAUP1c9g=SlhDbE9uS2laS2JaZFpNWvmsxai1zlJP9llgnl5HIv-4jAji8Dh2BQawzxID5bzr6Uv-3EQd-eluQbsPfcUOTg=https%3A//lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel=XRKU
> 

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

Re: [pve-devel] [PATCH V3 qemu-server 3/3] migration : add del_nets_bridge_fdb

2022-11-07 Thread Mira Limbeck 

On 8/24/22 18:26, Alexandre Derumier wrote:

at the end of a live migration, we need to remove old mac entries
on source host (vm is not yet stopped), before resume vm on target host

Signed-off-by: Alexandre Derumier 
---
  PVE/QemuMigrate.pm|  1 +
  PVE/QemuServer.pm | 20 
  test/MigrationTest/QemuMigrateMock.pm |  3 +++
  3 files changed, 24 insertions(+)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index d52dc8d..b72a3fe 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -1157,6 +1157,7 @@ sub phase3_cleanup {
  
  # transfer replication state before move config

  $self->transfer_replication_state() if $self->{is_replicated};
+PVE::QemuServer::del_nets_bridge_fdb($conf, $vmid);


I'm currently going through this patch series, and I'm wondering. 
Wouldn't it make more sense to delete the bridge fdb entries as part of 
the VM shutdown instead?


This way those would be cleaned up whenever the VM is stopped (and the 
tap device gets destroyed). No special handling for migrations needed, 
unless I'm missing something?




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

[pve-devel] [PATCH V3 qemu-server 3/3] migration : add del_nets_bridge_fdb

2022-08-24 Thread Alexandre Derumier 
at the end of a live migration, we need to remove old mac entries
on source host (vm is not yet stopped), before resume vm on target host

Signed-off-by: Alexandre Derumier 
---
 PVE/QemuMigrate.pm|  1 +
 PVE/QemuServer.pm | 20 
 test/MigrationTest/QemuMigrateMock.pm |  3 +++
 3 files changed, 24 insertions(+)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index d52dc8d..b72a3fe 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -1157,6 +1157,7 @@ sub phase3_cleanup {
 
 # transfer replication state before move config
 $self->transfer_replication_state() if $self->{is_replicated};
+PVE::QemuServer::del_nets_bridge_fdb($conf, $vmid);
 PVE::QemuConfig->move_config_to_node($vmid, $self->{node});
 $self->switch_replication_job_target() if $self->{is_replicated};
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6d71006..47a4882 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -8258,4 +8258,24 @@ sub add_nets_bridge_fdb {
}
 }
 }
+
+sub del_nets_bridge_fdb {
+my ($conf, $vmid) = @_;
+
+foreach my $opt (keys %$conf) {
+   if ($opt =~  m/^net(\d+)$/) {
+   my $net = parse_net($conf->{$opt});
+   next if !$net;
+   next if !$net->{macaddr};
+
+   my $iface = "tap${vmid}i$1";
+   if ($have_sdn) {
+   PVE::Network::SDN::Zones::del_bridge_fdb($iface, 
$net->{macaddr}, $net->{bridge}, $net->{firewall});
+   } else {
+   PVE::Network::del_bridge_fdb($iface, $net->{macaddr}, 
$net->{firewall});
+   }
+   }
+}
+}
+
 1;
diff --git a/test/MigrationTest/QemuMigrateMock.pm 
b/test/MigrationTest/QemuMigrateMock.pm
index f2c0281..f00b974 100644
--- a/test/MigrationTest/QemuMigrateMock.pm
+++ b/test/MigrationTest/QemuMigrateMock.pm
@@ -158,6 +158,9 @@ $MigrationTest::Shared::qemu_server_module->mock(
$vm_stop_executed = 1;
delete $expected_calls->{'vm_stop'};
 },
+del_nets_bridge_fdb => sub {
+   return;
+},
 );
 
 my $qemu_server_cpuconfig_module = 
Test::MockModule->new("PVE::QemuServer::CPUConfig");
-- 
2.30.2


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