[pve-devel] applied: [PATCH pve-common] network_interfaces: use allow-ovs for OVSBridge

[Thomas Lamprecht]

On 20.09.19 08:45, Alexandre Derumier wrote:
> new openvswitch-switch service use "ifup --allow-ovs",
> and this race with "auto ..." used by networking service
> 
> bug reported here:
> https://forum.proxmox.com/threads/so-is-openvswitch-bonding-just-broken-on-pve-6-whats-going-on.58020/
> 
> 
> This is also fixing ovs with ifupdown2
> 
> Openvswitch github have a note about this:
> https://github.com/openvswitch/ovs/commit/e0dfd67b456c8ea36cc2e2f23039a300dbbedfed#diff-555e6da95251766b76e83867900bd8fe
> 

Thanks for this, applied, with some small cleanup followups (see inline).

> Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> ---
>  src/PVE/INotify.pm                            | 20 ++++++++++---------
>  .../t.list-interfaces.pl                      |  2 +-
>  .../t.ovs_bridge_allow.pl                     |  6 +++---
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
> index 6a7f110..b3e5ab0 100644
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -1142,7 +1142,9 @@ sub __read_etc_network_interfaces {
>      # from the {options} hash for them to be removed correctly.
>      @$options = grep {defined($_)} map {
>  	my ($pri, $line) = @$_;
> -	if ($line =~ /^allow-(\S+)\s+(.*)$/) {
> +	if ($line =~ /^allow-ovs\s+(.*)$/) {
> +	    undef;
> +	} elsif ($line =~ /^allow-(\S+)\s+(.*)$/) {
>  	    my $bridge = $1;
>  	    my @ports = split(/\s+/, $2);
>  	    if (defined(my $br = $ifaces->{$bridge})) {
> @@ -1254,6 +1256,7 @@ sub __interface_to_string {
>  	$done->{ovs_type} = 1;
>  
>  	$raw .= "\tovs_ports $d->{ovs_ports}\n" if $d->{ovs_ports};
> +
>  	$done->{ovs_ports} = 1;
>      } elsif ($d->{type} eq 'OVSPort' || $d->{type} eq 'OVSIntPort' ||
>  	     $d->{type} eq 'OVSBond') {
> @@ -1293,12 +1296,7 @@ sub __interface_to_string {
>  
>  	if ($d->{ovs_bridge}) {

used
	if (my $bridge = $d->{ovs_bridge}) {
above and then $bridge respectively below.

>  
> -	    if ($ifupdown2) {
> -		$raw = "auto $iface\n$raw";
> -	    } else {
> -		$raw = "allow-$d->{ovs_bridge} $iface\n$raw";
> -	    }
> -
> +	    $raw = "allow-$d->{ovs_bridge} $iface\n$raw";
>  	    $raw .= "\tovs_bridge $d->{ovs_bridge}\n";
>  	    $done->{ovs_bridge} = 1;
>  	}
> @@ -1592,7 +1590,6 @@ NETWORKDOC
>  	return $a cmp $b;
>      } keys %$ifaces) {
>  	next if $printed->{$iface};
> -
>  	my $d = $ifaces->{$iface};
>  	my $pri = $d->{priority} // 0;
>  	if (@options && $options[0]->[0] < $pri) {
> @@ -1603,7 +1600,12 @@ NETWORKDOC
>  	}
>  
>  	$printed->{$iface} = 1;
> -	$raw .= "auto $iface\n" if $d->{autostart};
> +        if ($d->{type} eq 'OVSBridge') {

indentation error above

> +	    $raw .= "allow-ovs $iface\n";
> +	} elsif ($d->{autostart}) {
> +	    $raw .= "auto $iface\n" if $d->{autostart};

double check for "if $d->{autostart}" above

> +	}
> +
>  	my $i = 0; # some options should be printed only once
>  	$raw .= __interface_to_string($iface, $d, $_, !$i++, $ifupdown2) foreach @{$d->{families}};
>      }
> diff --git a/test/etc_network_interfaces/t.list-interfaces.pl b/test/etc_network_interfaces/t.list-interfaces.pl
> index 41907ae..7e37afa 100644
> --- a/test/etc_network_interfaces/t.list-interfaces.pl
> +++ b/test/etc_network_interfaces/t.list-interfaces.pl
> @@ -53,7 +53,7 @@ iface vmbr0 inet6 static
>  
>  source-directory before-ovs.d
>  
> -auto vmbr1
> +allow-ovs vmbr1
>  iface vmbr1 inet static
>  	address  $wanted{vmbr1}->{address}
>  	netmask  $wanted{vmbr1}->{netmask}
> diff --git a/test/etc_network_interfaces/t.ovs_bridge_allow.pl b/test/etc_network_interfaces/t.ovs_bridge_allow.pl
> index 6d22243..45fad2e 100644
> --- a/test/etc_network_interfaces/t.ovs_bridge_allow.pl
> +++ b/test/etc_network_interfaces/t.ovs_bridge_allow.pl
> @@ -39,7 +39,7 @@ iface eth2 inet manual
>  
>  iface eth3 inet manual
>  
> -auto vmbr0
> +allow-ovs vmbr0
>  iface vmbr0 inet static
>  	address  $ip
>  	netmask  $nm
> @@ -67,7 +67,7 @@ iface eth2 inet manual
>  
>  iface eth3 inet manual
>  
> -auto vmbr0
> +allow-ovs vmbr0
>  iface vmbr0 inet static
>  	address  $ip
>  	netmask  $nm
> @@ -102,7 +102,7 @@ iface eth3 inet manual
>  
>  iface eth2 inet manual
>  
> -auto vmbr0
> +allow-ovs vmbr0
>  iface vmbr0 inet static
>  	address  $ip
>  	netmask  $nm
>