public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Christoph Heiss <c.heiss@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH installer v3 00/15] support network interface name pinning
Date: Tue, 11 Nov 2025 18:12:16 +0100	[thread overview]
Message-ID: <20251111181216.3c1ca6a0@rosa.proxmox.com> (raw)
In-Reply-To: <20251111140014.1443471-1-c.heiss@proxmox.com>

Thanks for the quick iteration and the unifying of the implementations for
the validations! 

I ran a few smoke-tests:
* installed a server in our test-lab which had issues with 9.0 (due to
  duplicate predictable names (pci-path-based) for a 100g broadcom nic and
  a 1g broadcom nic...) - this went fine (I installed with
  filter.ID_NET_NAME set to the unpinned name - and the autoinstaller
  correctly configured the corresponding pinned nic-name in
  /etc/network/interfaces)
* during a few smoke-tests in a VM I noticed that the following nic-names
  were accepted (by both the tui and gui installers):
 '_' , 'a' - from a quick look at:
 https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/JSONSchema.pm;h=c6e0f36898487b9584221693b8846195f1ef87dd;hb=HEAD#l677
 I think both would be rejected - so maybe we could adapt this in a
 follow-up (and still cap the maximal length here to 15)

In any case - the series as-is is a worthy addition!
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>

On Tue, 11 Nov 2025 14:59:50 +0100
Christoph Heiss <c.heiss@proxmox.com> 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.
> 
> If pinning is enabled, interfaces which have no explicit mapping
> specified in the answer file will be assigned a pinned name based on
> their enumeration and the fixed `nic` prefix, i.e. `nic2` for the third
> NIC enumerated during boot.
> 
> The hardcoded `nic` prefix could potentially be made configurable in the
> future, although given that the auto-installer provides information
> about all present network interfaces in the answer POST request anyway,
> assigning all network interfaces a custom name programmatically is no
> big deal.
> 
> History
> =======
> v2: https://lore.proxmox.com/pve-devel/20251030110627.812398-1-c.heiss@proxmox.com/
> v1: https://lore.proxmox.com/pve-devel/20251014132207.1171073-1-c.heiss@proxmox.com/
> 
> Notable changes v2 -> v3:
>   * matched up pinning map validation for Rust & Perl (hopefully I
>     caught all cases this time), also adding unit tests to the latter
>   * improved above checks for both sides, mostly w.r.t. numeric checks
> 
> Notable 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
> 
> Diffstat
> ========
> 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                            | 116 ++++-
>  Proxmox/UI.pm                                 |  12 +-
>  Proxmox/UI/Gtk3.pm                            |  12 +-
>  proxinstall                                   | 189 +++++++-
>  proxmox-auto-install-assistant/src/main.rs    |  39 +-
>  proxmox-auto-installer/src/answer.rs          |  63 ++-
>  .../src/bin/proxmox-auto-installer.rs         |   5 +-
>  proxmox-auto-installer/src/utils.rs           |  62 ++-
>  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 +-
>  .../src/fetch_plugins/http.rs                 |   5 +-
>  .../src/fetch_plugins/partition.rs            |   5 +-
>  proxmox-installer-common/src/lib.rs           |   5 +
>  proxmox-installer-common/src/options.rs       | 274 ++++++++++--
>  proxmox-installer-common/src/setup.rs         |  78 +++-
>  proxmox-installer-common/src/utils.rs         |   6 +-
>  proxmox-post-hook/src/main.rs                 |  73 ++--
>  proxmox-tui-installer/src/main.rs             | 110 +----
>  proxmox-tui-installer/src/setup.rs            |   3 +
>  proxmox-tui-installer/src/views/bootdisk.rs   |   8 +-
>  proxmox-tui-installer/src/views/mod.rs        |  90 ++--
>  proxmox-tui-installer/src/views/network.rs    | 406 ++++++++++++++++++
>  test/Makefile                                 |   7 +-
>  test/parse-kernel-cmdline.pl                  |   2 +-
>  test/validate-link-pin-map.pl                 |  42 ++
>  34 files changed, 1534 insertions(+), 329 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
>  create mode 100755 test/validate-link-pin-map.pl
> 



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


  parent reply	other threads:[~2025-11-11 17:11 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-11 13:59 Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 01/15] test: parse-kernel-cmdline: fix module import statement Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 02/15] install: add support for network interface name pinning Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 03/15] run env: network: add kernel driver name to network interface info Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 04/15] common: utils: fix clippy warnings Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 05/15] common: setup: simplify network address list serialization Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 06/15] common: implement support for `network_interface_pin_map` config Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 07/15] auto: add support for pinning network interface names Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 08/15] assistant: verify network settings in `validate-answer` subcommand Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 09/15] post-hook: avoid redundant Option<bool> for (de-)serialization Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 10/15] post-hook: add network interface name and pinning status Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 11/15] tui: views: move network options view to own module Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 12/15] tui: views: form: allow attaching user-defined data to children Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 13/15] tui: add support for pinning network interface names Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 14/15] ui: gtk3: allow passing of dialog parent window Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 15/15] gui: add support for pinning network interface names Christoph Heiss
2025-11-11 16:49   ` Stoiko Ivanov
2025-11-11 15:04 ` [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Thomas Lamprecht
2025-11-12 12:00   ` Christoph Heiss
2025-11-11 17:12 ` Stoiko Ivanov [this message]
2025-11-12  7:13 ` [pve-devel] applied: " Thomas Lamprecht

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=20251111181216.3c1ca6a0@rosa.proxmox.com \
    --to=s.ivanov@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