Re: [pve-devel] [PATCH V3 qemu-server 1/3] tap_plug: add support for bridge disable learning

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 24/08/2022 um 18:26 schrieb Alexandre Derumier:
> This disabling mac learning && unicast flood for the tap interface
> 
> for vmstart, we don't add mac directly to fdb.
> We set it latter if it's a migration or a fresh start.
> 
> for nic hotplug, we directly add mac to fdb
> 
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  PVE/QemuServer.pm             | 8 +++++++-
>  vm-network-scripts/pve-bridge | 6 +++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index c706653..0114d06 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5137,8 +5137,14 @@ sub vmconfig_update_net {
>  
>  		if ($have_sdn) {
>  		    PVE::Network::SDN::Zones::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate});
> +		    PVE::Network::SDN::Zones::add_bridge_fdb($iface, $newnet->{macaddr}, $newnet->{bridge}, $newnet->{firewall});
>  		} else {
> -		    PVE::Network::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate});
> +		    my $interfaces_config = PVE::INotify::read_file('interfaces');
> +		    my $bridge = $newnet->{bridge};
> +		    my $opts = {};
> +		    $opts->{learning} = 0 if $interfaces_config->{ifaces}->{$bridge} && $interfaces_config->{ifaces}->{$bridge}->{'bridge-disable-mac-learning'};
> +		    PVE::Network::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate}, $opts);
> +		    PVE::Network::add_bridge_fdb($iface, $newnet->{macaddr}, $newnet->{firewall}) if defined($opts->{learning}) && !$opts->{learning};
>  		}
>  	    } elsif (safe_num_ne($oldnet->{rate}, $newnet->{rate})) {
>  		# Rate can be applied on its own but any change above needs to
> diff --git a/vm-network-scripts/pve-bridge b/vm-network-scripts/pve-bridge
> index d37ce33..38cf2f6 100755
> --- a/vm-network-scripts/pve-bridge
> +++ b/vm-network-scripts/pve-bridge
> @@ -47,8 +47,12 @@ if ($have_sdn) {
>      PVE::Network::SDN::Zones::tap_create($iface, $net->{bridge});
>      PVE::Network::SDN::Zones::tap_plug($iface, $net->{bridge}, $net->{tag}, $net->{firewall}, $net->{trunks}, $net->{rate});
>  } else {
> +    my $interfaces_config = PVE::INotify::read_file('interfaces');
> +    my $bridge = $net->{bridge};
> +    my $opts = {};
> +    $opts->{learning} = 0 if $interfaces_config->{ifaces}->{$bridge} && $interfaces_config->{ifaces}->{$bridge}->{'bridge-disable-mac-learning'};
>      PVE::Network::tap_create($iface, $net->{bridge});
> -    PVE::Network::tap_plug($iface, $net->{bridge}, $net->{tag}, $net->{firewall}, $net->{trunks}, $net->{rate});
> +    PVE::Network::tap_plug($iface, $net->{bridge}, $net->{tag}, $net->{firewall}, $net->{trunks}, $net->{rate}, $opts);
>  }
>  
>  exit 0;

what about moving this into pve-common instead? IOW. something like:

diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index c468e40..cc2403c 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -460,7 +460,12 @@ sub tap_plug {
 
     $opts = {} if !defined($opts);
 
-    my $no_learning = defined($opts->{learning}) && !$opts->{learning}; # default to learning on
+    if (!defined($opts->{learning})) { # auto-detect
+       my $interfaces_config = PVE::INotify::read_file('interfaces');
+       my $bridge = $interfaces_config->{ifaces}->{$bridge};
+       $opts->{learning} = !($bridge && $bridge->{'bridge-disable-mac-learning'}); # default learning to on
+    }
+    my $no_learning = !$opts->{learning};
 
     # cleanup old port config from any openvswitch bridge
     eval {

That way we'd not need to touch all call sites and avoid forgetting it on new ones.