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

[Mira Limbeck <m.limbeck@proxmox.com>]

On 8/24/22 18:26, Alexandre Derumier wrote:
> 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);
>   }

Why not add the bridge in the pve-bridge script as well? This way there 
would be no need for patch 2 for qemu-server since we always add the MAC 
address to the FDB whenever the tap device is plugged.

If we then remove it in the pve-bridgedown script, there should be no 
need for patch 3, right?