public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: Christoph Heiss <c.heiss@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [PATCH installer 3/8] gui: use run_env->{network} instead of old run_env->{ipconf}
Date: Mon, 11 May 2026 11:09:30 +0200	[thread overview]
Message-ID: <97ad2754-0db5-493c-94e3-2367ab5d539d@proxmox.com> (raw)
In-Reply-To: <20260508184546.113293-4-c.heiss@proxmox.com>

looks like this is missing an update to the `device_change_handler`
closure in `sub create_ipconf_view` (proxinstall)

On 2026-05-08 20:44, Christoph Heiss wrote:
> The TUI started out with using the newer network configuration. That
> information is parsed from the iproute2 JSON output, instead of the
> human-readable output - making it more robust.
> 
> Reworking the network address/gateway/DNS server selection also allows
> for properly handling IPv4 and/or IPv6 setups, by always selecting the
> correct IP address for the "active" network, based on the gateway.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  Proxmox/Sys/Net.pm |  6 +++++
>  proxinstall        | 66 ++++++++++++++++++++++++++++++----------------
>  2 files changed, 50 insertions(+), 22 deletions(-)
> 
> diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
> index 9571236..10144e0 100644
> --- a/Proxmox/Sys/Net.pm
> +++ b/Proxmox/Sys/Net.pm
> @@ -10,6 +10,7 @@ use JSON qw(from_json);
>  
>  use base qw(Exporter);
>  our @EXPORT_OK = qw(
> +    ip_get_version
>      parse_ip_address
>      parse_ip_mask
>      parse_fqdn
> @@ -134,6 +135,11 @@ sub parse_ip_address {
>      return (undef, undef);
>  }
>  
> +sub ip_get_version {
> +    my ($ip, $ver) = parse_ip_address($_[0]);
> +    return $ver;
> +}
> +
>  sub parse_ip_mask {
>      my ($text, $ip_version) = @_;
>      $text =~ s/^\s+//;
> diff --git a/proxinstall b/proxinstall
> index f5e1555..8291a46 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -37,7 +37,9 @@ use Proxmox::Sys;
>  use Proxmox::Sys::Block qw(get_cached_disks);
>  use Proxmox::Sys::Command qw(syscmd);
>  use Proxmox::Sys::File qw(file_read_all file_write_all);
> -use Proxmox::Sys::Net qw(parse_ip_address parse_ip_mask validate_link_pin_map DEFAULT_PIN_PREFIX);
> +use Proxmox::Sys::Net qw(
> +    parse_ip_address ip_get_version parse_ip_mask validate_link_pin_map DEFAULT_PIN_PREFIX
> +);
>  use Proxmox::UI;
>  
>  my $step_number = 0; # Init number for global function list
> @@ -430,7 +432,21 @@ sub create_ipconf_view {
>      Proxmox::UI::display_html('ipconf.htm');
>  
>      my $run_env = Proxmox::Install::RunEnv::get();
> -    my $ipconf = $run_env->{ipconf};
> +    my $network = $run_env->{network};
> +
> +    # prefer ipv4 gateway and fallback to ipv6
> +    my $default_gateway = $network->{routes}->{gateway4} // $network->{routes}->{gateway6};
> +
> +    my $default_gateway_ipversion = ip_get_version($default_gateway->{gateway})
> +        if defined($default_gateway->{gateway});
> +
> +    my $default_dns = undef;
> +    if (defined($default_gateway)) {
> +        # try to retrieve the first dns server matching the IP version of the gateway
> +        ($default_dns) = grep {
> +            ip_get_version($_) == $default_gateway_ipversion
> +        } @{ $network->{dns}->{dns} };
> +    }
>  
>      my $grid = &$create_basic_grid();
>      $grid->set_row_spacing(10);
> @@ -463,20 +479,20 @@ sub create_ipconf_view {
>  
>          my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
>          my $i = 0;
> -        for my $index (sort keys $ipconf->{ifaces}->%*) {
> -            my $iface = $ipconf->{ifaces}->{$index};
> +        for my $name (sort keys $network->{interfaces}->%*) {
> +            my $iface = $network->{interfaces}->{$name};
>              my $iter = $device_model->append();
>  
>              my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
> -            my $name =
> +            my $label =
>                  $gtk_state->{network_pinning_enabled} && defined($mapping->{ $iface->{mac} })
>                  ? $mapping->{ $iface->{mac} }
> -                : $iface->{name};
> +                : $name;
>  
>              $device_model->set(
>                  $iter,
>                  0 => $symbol,
> -                1 => "$name - $iface->{mac} ($iface->{driver})",
> +                1 => "$label - $iface->{mac} ($iface->{driver})",
>              );
>              $i++;
>          }
> @@ -513,7 +529,7 @@ sub create_ipconf_view {
>          return if $current->get_active() == -1;
>  
>          my $new = $device_active_map->{ $current->get_active() };
> -        my $iface = $ipconf->{ifaces}->{$new};
> +        my $iface = $network->{interfaces}->{$new};
>  
>          my $selected = Proxmox::Install::Config::get_mngmt_nic();
>          return if defined($selected) && $iface->{name} eq $selected;
> @@ -529,15 +545,22 @@ sub create_ipconf_view {
>      my ($initial_active_device_pos, $initial_addr, $initial_mask) = (0, undef, undef);
>  
>      my $i = 0;
> -    for my $index (sort keys $ipconf->{ifaces}->%*) {
> -        my $iface = $ipconf->{ifaces}->{$index};
> -        $device_active_map->{$i} = $index;
> -        $device_active_reverse_map->{ $iface->{name} } = $i;
> +    for my $name (sort keys $network->{interfaces}->%*) {
> +        my $iface = $network->{interfaces}->{$name};
> +        $device_active_map->{$i} = $name;
> +        $device_active_reverse_map->{$name} = $i;
>  
> -        if (defined($ipconf->{default}) && $index == $ipconf->{default}) {
> +        if (defined($default_gateway) && $name eq $default_gateway->{dev}) {
> +            # the chosen gateway is on this interface
>              $initial_active_device_pos = $i;
> -            $initial_addr = $iface->{inet}->{addr} || $iface->{inet6}->{addr};
> -            $initial_mask = $iface->{inet}->{prefix} || $iface->{inet6}->{prefix};
> +
> +            # now find an address that matches the IP version of the gateway
> +            my ($addr) = grep {
> +                ip_get_version($_->{address}) == $default_gateway_ipversion
> +            } @{ $iface->{addresses} };
> +
> +            $initial_addr = $addr->{address};
> +            $initial_mask = $addr->{prefix};
>          }
>          $i++;
>      }
> @@ -548,9 +571,8 @@ sub create_ipconf_view {
>      if (my $nic = Proxmox::Install::Config::get_mngmt_nic()) {
>          $initial_active_device_pos = $device_active_reverse_map->{$nic};
>      } else {
> -        my $iface_id = $device_active_map->{$initial_active_device_pos};
> -        my $iface = $ipconf->{ifaces}->{$iface_id};
> -        Proxmox::Install::Config::set_mngmt_nic($iface->{name});
> +        my $iface_name = $device_active_map->{$initial_active_device_pos};
> +        Proxmox::Install::Config::set_mngmt_nic($iface_name);
>      }
>  
>      if (my $cidr = Proxmox::Install::Config::get_cidr()) {
> @@ -574,7 +596,7 @@ sub create_ipconf_view {
>  
>      my $fqdn = Proxmox::Install::Config::get_fqdn();
>      my $hostname = $run_env->{network}->{hostname} || $iso_env->{product};
> -    my $domain = $ipconf->{domain} || "example.invalid";
> +    my $domain = $network->{dns}->{domain} || "example.invalid";
>      $fqdn //= "$hostname.$domain";
>  
>      my ($host_label, $hostentry) = create_text_input($fqdn, 'Hostname (FQDN)');
> @@ -585,14 +607,14 @@ sub create_ipconf_view {
>      $grid->attach($cidr_box, 1, 2, 2, 1);
>  
>      my $cfg_gateway = Proxmox::Install::Config::get_gateway();
> -    my $gateway = $cfg_gateway // $ipconf->{gateway} || '192.168.100.1';
> +    my $gateway = $cfg_gateway // $default_gateway->{gateway} || '192.168.100.1';
>  
>      my ($gw_label, $ipconf_entry_gw) = create_text_input($gateway, 'Gateway');
>      $grid->attach($gw_label, 0, 3, 1, 1);
>      $grid->attach($ipconf_entry_gw, 1, 3, 2, 1);
>  
>      my $cfg_dns = Proxmox::Install::Config::get_dns();
> -    my $dnsserver = $cfg_dns // $ipconf->{dnsserver} || $gateway;
> +    my $dnsserver = $cfg_dns // $default_dns || $gateway;
>  
>      my ($dns_label, $ipconf_entry_dns) = create_text_input($dnsserver, 'DNS Server');
>  
> @@ -1915,7 +1937,7 @@ my $initial_error = 0;
>  }
>  
>  my $run_env = Proxmox::Install::RunEnv::get();
> -if (!$initial_error && (scalar keys $run_env->{ipconf}->{ifaces}->%* == 0)) {
> +if (!$initial_error && (scalar keys $run_env->{network}->{interfaces}->%* == 0)) {
>      print STDERR "no network interfaces found\n";
>      $initial_error = 1;
>      Proxmox::UI::display_html("nonics.htm");





  reply	other threads:[~2026-05-11  9:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-08 18:44 [PATCH installer 0/8] add IPv6 SLAAC and v6-only support Christoph Heiss
2026-05-08 18:44 ` [PATCH installer 1/8] install: drop trivial fromjs() wrapper and use JSON::from_json() Christoph Heiss
2026-05-08 18:44 ` [PATCH installer 2/8] install: move network subroutines to Proxmox::Sys::Net Christoph Heiss
2026-05-08 18:44 ` [PATCH installer 3/8] gui: use run_env->{network} instead of old run_env->{ipconf} Christoph Heiss
2026-05-11  9:09   ` Hannes Laimer [this message]
2026-05-11  9:59     ` Christoph Heiss
2026-05-08 18:44 ` [PATCH installer 4/8] sys: net: drop the now-unused `ipconf` runtime environment configuration Christoph Heiss
2026-05-11  8:55   ` Hannes Laimer
2026-05-11 10:00     ` Christoph Heiss
2026-05-08 18:44 ` [PATCH installer 5/8] sys: net: allow up to /128 netmask for IPv6 Christoph Heiss
2026-05-08 18:44 ` [PATCH RFC installer 6/8] sys: net: ignore ipv6 nameservers with zone identifiers Christoph Heiss
2026-05-08 18:44 ` [PATCH installer 7/8] common: options: rework network address setup to handle ipv6-only Christoph Heiss
2026-05-08 18:44 ` [PATCH installer 8/8] unconfigured: try to retrieve IPv6 SLAAC addresses on startup Christoph Heiss
2026-05-11  9:40 ` [PATCH installer 0/8] add IPv6 SLAAC and v6-only support Gabriel Goller
2026-05-11 15:59 ` Christoph Heiss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=97ad2754-0db5-493c-94e3-2367ab5d539d@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=c.heiss@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal