[PATCH qemu-server v2 0/2] fix #6490: network: bridge fdb entries on nic hotplug

[PATCH qemu-server v2 0/2] fix #6490: network: bridge fdb entries on nic hotplug

[Rémi PAETA]

Thank you Gabriel and Fiona for your feedback on v1! This v2 takes it
into account.

On bridges with MAC learning disabled, hotplugged or re-plugged NICs
get no forwarding database entries, so the guest loses connectivity on
that NIC, and a later live migration fails when deleting the missing
entries.

v1: https://lore.proxmox.com/pve-devel/20261013095852.2586609-1-remi.paeta@comaite.com/

Changes since v1, following your feedback:
- new first patch: only warn when deleting the entries fails at the end
  of a migration, instead of failing the migration after the VM state
  was transferred
- only warn when adding the entries fails after plugging the NIC, so
  the configuration stays in sync with the running VM
- only add the entries on hotplug if the NIC has a bridge, to not warn
  about NICs without one

Note that with the synchronous config update of the web UI, such a
warning only ends up in the syslog, as there is no task log.

Tested on a Linux bridge with bridge-disable-mac-learning, with a
failure of 'bridge fdb' injected where noted:
- toggling the firewall of a NIC and hotplugging a NIC: entries added,
  connectivity kept
- same with an injected failure: warning, configuration updated (v1
  failed the update and left the new value pending, while the NIC was
  already re-plugged)
- live migration with missing entries: warnings, migration succeeds,
  entries re-added on the target on resume, no QEMU left on the source
  (without the first patch, the VM stayed paused and locked on the
  target)

Rémi PAETA (2):
  network: do not fail on missing bridge fdb entries when migrating
  fix #6490: network: add bridge fdb entries when hotplugging a nic

 src/PVE/QemuServer.pm         | 8 ++++++++
 src/PVE/QemuServer/Network.pm | 3 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.47.3

[PATCH qemu-server v2 1/2] network: do not fail on missing bridge fdb entries when migrating

[Rémi PAETA]

del_nets_bridge_fdb() is called at the end of a live migration, to
remove the forwarding database entries of the NICs on the source
before the VM resumes on the target. If an entry is missing, for
example after a NIC got re-plugged (see bug #6490), 'bridge fdb del'
fails and so does the migration, after the VM state was transferred:
the VM stays paused and locked on the target, and the paused QEMU stays
on the source.

The source tap devices go away with the source QEMU anyway, so only
warn about entries that could not be deleted.

Suggested-by: Gabriel Goller <g.goller@proxmox.com>
Signed-off-by: Rémi PAETA <remi.paeta@comaite.com>
---
 src/PVE/QemuServer/Network.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer/Network.pm b/src/PVE/QemuServer/Network.pm
index b4893c01..01b21f86 100644
--- a/src/PVE/QemuServer/Network.pm
+++ b/src/PVE/QemuServer/Network.pm
@@ -309,7 +309,8 @@ sub del_nets_bridge_fdb {
         my $mac = $net->{macaddr} or next;
 
         my $bridge = $net->{bridge};
-        PVE::Network::SDN::Zones::del_bridge_fdb($iface, $mac, $bridge);
+        eval { PVE::Network::SDN::Zones::del_bridge_fdb($iface, $mac, $bridge) };
+        log_warn("failed to delete forwarding database entries of '$iface' - $@") if $@;
     }
 }
 
-- 
2.47.3

[PATCH qemu-server v2 2/2] fix #6490: network: add bridge fdb entries when hotplugging a nic

[Rémi PAETA]

On bridges with MAC learning disabled, the forwarding database entries
of the guest NICs are added on VM start and on resume after a
migration, but not when a NIC gets hotplugged or re-plugged because its
bridge, VLAN tag, trunks or firewall setting changed. Re-plugging even
drops the existing entries.

The guest then loses connectivity on that NIC until its next start or
migration, for example after enabling or disabling the firewall of the
NIC.

Add the forwarding database entries of the NIC after plugging it. On
bridges with MAC learning enabled, this is a no-op. As the NIC is
already plugged at that point, only warn if adding them fails, instead
of failing the update and leaving the configuration out of sync with
the running VM.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6490
Signed-off-by: Rémi PAETA <remi.paeta@comaite.com>
---
 src/PVE/QemuServer.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index b4a1a38d..626dc78a 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -4994,6 +4994,9 @@ sub vmconfig_update_net {
                     $newnet->{trunks},
                     $newnet->{rate},
                 );
+                # unplugging the interface dropped its forwarding database entries
+                eval { PVE::QemuServer::Network::add_nets_bridge_fdb({ $opt => $value }, $vmid) };
+                log_warn("failed to add forwarding database entries of '$iface' - $@") if $@;
 
             } elsif (safe_num_ne($oldnet->{rate}, $newnet->{rate})) {
                 # Rate can be applied on its own but any change above needs to
@@ -5023,6 +5026,11 @@ sub vmconfig_update_net {
             $newnet->{bridge}, $newnet->{macaddr}, $vmid, $conf->{name},
         );
         vm_deviceplug($storecfg, $conf, $vmid, $opt, $newnet, $arch, $machine_type);
+        if ($newnet->{bridge}) {
+            # the NIC is plugged already, so do not fail the hotplug
+            eval { PVE::QemuServer::Network::add_nets_bridge_fdb({ $opt => $value }, $vmid) };
+            log_warn("failed to add forwarding database entries of '$opt' - $@") if $@;
+        }
     } else {
         die "skip\n";
     }
-- 
2.47.3

Re: [PATCH qemu-server v2 0/2] fix #6490: network: bridge fdb entries on nic hotplug

[Gabriel Goller]

Thanks, LGTM.

Consider:
Reviewed-by: Gabriel Goller <g.goller@proxmox.com>