[pve-devel] [PATCH pve-common 2/5] Inotify: forbid ip address on bridged interface.

[Thomas Lamprecht]

On 1/8/20 4:31 AM, Alexandre Derumier wrote:
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> ---
>  src/PVE/INotify.pm | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
> index 867da30..5c15926 100644
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -1512,6 +1512,8 @@ sub __write_etc_network_interfaces {
>  		my $n = $ifaces->{$p};
>  		die "bridge '$iface' - unable to find bridge port '$p'\n"
>  		    if !$n;
> +		die "iface $p - ip address can't be set on interface if bridged in $iface\n" if ($n->{method} eq 'static' || $n->{method6} eq 'static') && $n->{address} ne '0.0.0.0' && $n->{address6} ne '0.0.0.0';

1. the IPv6 "zero address" isn't 0.0.0.0, so you'd need to check for '::' (and normalize?)
2. this check is too long IMO, can we do

if (($n->{method} eq 'static && $n->{address} ne '0.0.0.0') ||
    ($n->{method6} eq 'static' && $n->{address6} ne '::')) {
    die "...";
}

or move the check to a sub for readability, e.g:
sub iface_has_address {
    my $if = shift;

    ($if->{method} ...) ||
    ($if...);
}

or at least move the post-if to a new line:

die "..."
    if (...);


thanks!

> +
>  		&$check_mtu($ifaces, $iface, $p);
>  		$bridgeports->{$p} = $iface;
>  	    }
>