Re: [pve-devel] [PATCH installer v2 00/15] support network interface name pinning

["Michael Köppl" <m.koeppl@proxmox.com>]

Gave v2 a spin as well. Checked that:
- Interface name validation now also checks for fully-numeric names,
  names that start with a number, and invalid characters

  Noticed that fully-numeric interface names yield error "name must not
  start with a number" due to the order of the checks. Commented on the
  individual patch.

- Errors in the GUI are now displayed on top of the interface name
  pinning dialog
- Unknown MAC address is correctly detected and warning is displayed
  during autoinstall
- GUI installer generally looks as it should after the suggested changes
  by @Maximiliano

Apart from the problem with the noted above, seems to work as
advertised. Did not notice anything off.

Also had a look at the updated patches and, apart from my comments on
patch 06/15, lgtm!

With my comments on the individual patches addressed, please consider
this:
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>

On Thu Oct 30, 2025 at 12:06 PM CET, Christoph Heiss wrote:
> This series adds support for pinning the names of network interfaces
> directly during the installation, for all of GUI, TUI and auto-installer.
>
> There are also some smaller clean-ups and quality-of-life improvements
> interspersed - to keep the patches for each part of the installer
> together - in the series, which can also be applied
> separately/beforehand if wanted.
>
> Tested all combinations, i.e. for each of GUI, TUI and auto-installer,
> installed with pinning disabled (checking for regressions) and with
> pinning enabled at some custom interface names set.
>
> The auto-installer changes can be tested by specifying e.g.
>
>   [network.interface-name-pinning]
>   enabled = true
>   
>   [network.interface-name-pinning.mapping]
>   "ab:cd:ef:12:34:56" = "mgmt"
>   "12:34:56:ab:cd:ef" = "lan0"
>
> in the answer file.
>
> History
> =======
> v1: https://lore.proxmox.com/pve-devel/20251014132207.1171073-1-c.heiss@proxmox.com/
>
> Changes v1 -> v2:
>   * improved interface name validation according to our `pve-iface` json
>     schema
>   * fixed gui error dialog getting wrongly z-ordered
>   * implemented gtk machinery suggestion by Maximilano
>
> Diffstats
> =========
> Christoph Heiss (14):
>   test: parse-kernel-cmdline: fix module import statement
>   run env: network: add kernel driver name to network interface info
>   common: utils: fix clippy warnings
>   common: setup: simplify network address list serialization
>   common: implement support for `network_interface_pin_map` config
>   auto: add support for pinning network interface names
>   assistant: verify network settings in `validate-answer` subcommand
>   post-hook: avoid redundant Option<bool> for (de-)serialization
>   post-hook: add network interface name and pinning status
>   tui: views: move network options view to own module
>   tui: views: form: allow attaching user-defined data to children
>   tui: add support for pinning network interface names
>   ui: gtk3: allow passing of dialog parent window
>   gui: add support for pinning network interface names
>
> Thomas Lamprecht (1):
>   install: add support for network interface name pinning
>
>  Proxmox/Install.pm                            |  47 +-
>  Proxmox/Install/Config.pm                     |   8 +
>  Proxmox/Install/RunEnv.pm                     |  11 +
>  Proxmox/Sys/Net.pm                            |  63 ++-
>  Proxmox/UI.pm                                 |  12 +-
>  Proxmox/UI/Gtk3.pm                            |  12 +-
>  proxinstall                                   | 231 +++++++++-
>  proxmox-auto-install-assistant/src/main.rs    |   3 +-
>  proxmox-auto-installer/src/answer.rs          |  63 ++-
>  proxmox-auto-installer/src/utils.rs           |  54 ++-
>  proxmox-auto-installer/tests/parse-answer.rs  |   2 +
>  .../network_interface_pinning.json            |  30 ++
>  .../network_interface_pinning.toml            |  22 +
>  ...n_from_dhcp_no_default_domain.run-env.json |  36 +-
>  ...rface_pinning_overlong_interface_name.json |   3 +
>  ...rface_pinning_overlong_interface_name.toml |  18 +
>  .../no_fqdn_from_dhcp.run-env.json            |  36 +-
>  .../tests/resources/run-env-info.json         |  38 +-
>  proxmox-installer-common/src/lib.rs           |   5 +
>  proxmox-installer-common/src/options.rs       | 219 ++++++++--
>  proxmox-installer-common/src/setup.rs         |  78 +++-
>  proxmox-installer-common/src/utils.rs         |   6 +-
>  proxmox-post-hook/src/main.rs                 |  62 +--
>  proxmox-tui-installer/src/main.rs             | 105 +----
>  proxmox-tui-installer/src/setup.rs            |   3 +
>  proxmox-tui-installer/src/views/bootdisk.rs   |   6 +-
>  proxmox-tui-installer/src/views/mod.rs        |  41 +-
>  proxmox-tui-installer/src/views/network.rs    | 406 ++++++++++++++++++
>  test/parse-kernel-cmdline.pl                  |   2 +-
>  29 files changed, 1374 insertions(+), 248 deletions(-)
>  create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
>  create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
>  create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
>  create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
>  create mode 100644 proxmox-tui-installer/src/views/network.rs



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel