Re: [pve-devel] [PATCH common 2/2] daemon: explicitly bind to wildcard address.

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, May 04, 2021 at 12:12:16PM +0200, Stoiko Ivanov wrote:
> with the recent change in pve-manager pveproxy (and spiceproxy)
> try binding to '::' per default. This fails for hosts having disabled
> ipv6 via kernel commandline.
> 
> Our desired behavior of binding on '::' and only falling back to
> '0.0.0.0' in case this is not supported is not directly possible with
> IO::Socket::IP->new (or rather by Socket::GetAddrInfo, which at least
> on my system always returns the v4 wildcard-address first).
> 
> the code now binds to:
> * the provided $host if not undef
> * '::' if $host is not set
> * '0.0.0.0' if $host is not set and binding on '::' yields an error
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PVE/Daemon.pm | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/src/PVE/Daemon.pm b/src/PVE/Daemon.pm
> index 79b90ad..f4786b8 100644
> --- a/src/PVE/Daemon.pm
> +++ b/src/PVE/Daemon.pm
> @@ -819,14 +819,24 @@ sub create_reusable_socket {
>  	$socket->fcntl(Fcntl::F_SETFD(), Fcntl::FD_CLOEXEC);
>      } else {
>  
> -	$socket = IO::Socket::IP->new(
> -	    LocalHost => $host,
> +	my %sockargs = (
>  	    LocalPort => $port,
>  	    Listen => SOMAXCONN,
>  	    Proto  => 'tcp',
>  	    GetAddrInfoFlags => 0,
> -	    ReuseAddr => 1) ||
> -	    die "unable to create socket - $@\n";
> +	    ReuseAddr => 1,
> +	);
> +	if (defined($host)) {
> +	    $socket = IO::Socket::IP->new( LocalHost => $host, %sockargs) ||
> +		die "unable to create socket - $@\n";
> +	} else {
> +	    # disabling AF_INET6 (by adding ipv6.disable=1 to the kernel cmdline)
> +	    # causes bind on :: to fail, try 0.0.0.0 in that case
> +	    $socket = IO::Socket::IP->new( LocalHost => '::', %sockargs);
> +
> +	    $socket = IO::Socket::IP->new( LocalHost => '0.0.0.0', %sockargs) if $@;

So apparently this doesn't *die* but just returns `undef` on error. But
we also should not rely on internal implementation details, so we cannot
assume that $@ will be cleared on success.
So we should probably bind the two calls together via `//` (or clear $@
before the first call?)

    $socket = IO::Socket::IP->new( LocalHost => '::', %sockargs)
        // IO::Socket::IP->new( LocalHost => '0.0.0.0', %sockargs);

> +	    die "unable to create socket - $@\n" if !$socket;
> +	}
>  
>  	# we often observe delays when using Nagle algorithm,
>  	# so we disable that to maximize performance
> -- 
> 2.20.1