public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH pve-manager 1/1] cli: add pveeth
Date: Thu, 10 Jul 2025 18:25:15 +0200	[thread overview]
Message-ID: <zg2albkln54cjeptbtf4t2upqpko3vg4p455hsnwoagxeiwhfp@ovjnen4uzz56> (raw)
In-Reply-To: <a110ebe3-d186-4b16-83c3-eea960976dcc@proxmox.com>

On 10.07.2025 17:08, Thomas Lamprecht wrote:
>Am 10.07.25 um 16:53 schrieb Gabriel Goller:
>> On 09.07.2025 21:45, Stefan Hanreich wrote:
>>>> [snip]
>>> Example invocations of pveeth:
>>>
>>> $ pveeth pin --nic enp1s0 --force 0 --dry_run 0
>>>
>>> Generates a pinning for enp1s0 (if it doesn't exist already) and
>>> updates the configuration file.
>>>
>>> $ pveeth pin --force 1 --dry_run 0
>>
>> s/pin/unpin
>>
>>> Deletes all existing pins and re-generates them.
>>>
>>> For more information on the parameters see the API description.
>>>
>>> I've decided to let dry_run generate the configuration files in the
>>> current working directory, since it is then easy to diff the generated
>>> files with the existing configuration files using the diffviewer of
>>> the users' choice.
>>>
>>> Additionally, when writing the configuration files, they get backed up
>>> by creating a .bak at the location of the configuration file.
>>>
>>> Currently we only support a fixed prefix: 'nic'. This is because we
>>> rely on PHYISCAL_NIC_RE for detecting physical network interfaces
>>> across several places in our codebase. For now, nic has been added as
>>> a valid prefix for NICs in pve-common, so we use that prefix here.
>>
>> This would be nice to have as a comment in the code, I can already see
>> people that want to change the prefix and start editing the constant :)
>>
>>> In order to support custom prefixes, we would have to remove every
>>> place in the code relying on PHYISCAL_NIC_RE (at least), in order to
>>> avoid breakage.
>>
>> When pinning, could we add the previous old name as an altname? So tools
>> that respect altnames could work transparently.
>>
>> This should be as simple as adding, e.g.:
>>
>>      AlternativeName=ens18
>>
>> to the link file.
>
>This can interfere with automatic naming of unpinned/new interfaces and can
>get really confusing if the automatic derived name changes due to replugging
>the NIC to another slot and then the pinned alt name is wrong, as it suggest
>that the NIC is in a different slot. Or if the layout changes otherwise and
>another NIC would get this alt name, so there might be different NICs then
>with very similar names.

Ah true, didn't think of this. Yeah this would create a mess, nevermind.

>>
>> However, unpinning has a problem. Currently, we reset to the
>> `ID_NET_NAME_PATH` name from `udevadm` and remove the link file, but we
>> don't know the current systemd `NamePolicy`.
>>
>> For example, if the original interface was `ens18` using Slot policy,
>> after pinning it becomes `nic0`. When unpinning, the interface becomes
>> `ens18` again (because I'm using slot policy), but the config files
>> reference `enp0s18` (because `pveeth` assumes we use path policy). This
>> breaks the network configuration.
>>
>> I think the solution is to avoid deleting the link file during
>> unpinning. Instead, we should just update the interface name.
>>
>> The problem is that we can't determine which policy was originally used,
>> so pin-unpin cycles *don't* restore the original interface name.
>>
>
>This I'd need to think through, just wanted to comment on above before
>I forget.

If we really want to make pin and unpin involutive, we would need to
store somewhere the interface names or store the interface naming
policy.


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


  reply	other threads:[~2025-07-10 16:24 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-09 19:45 [pve-devel] [RFC common/firewall/manager/network/proxmox{-ve-rs, -firewall} 0/7] NIC renaming mitigations Stefan Hanreich
2025-07-09 19:45 ` [pve-devel] [PATCH pve-common 1/2] network: add ip link and altname helpers Stefan Hanreich
2025-07-09 19:45 ` [pve-devel] [PATCH pve-common 2/2] network: add nic prefix to physical nic regex Stefan Hanreich
2025-07-09 19:45 ` [pve-devel] [PATCH proxmox-ve-rs 1/1] config: ip link struct Stefan Hanreich
2025-07-09 19:45 ` [pve-devel] [PATCH proxmox-firewall 1/1] firewall: add altname support for firewall rules Stefan Hanreich
2025-07-09 19:45 ` [pve-devel] [PATCH pve-firewall 1/1] firewall: add altname support Stefan Hanreich
2025-07-09 19:45 ` [pve-devel] [PATCH pve-network 1/1] controllers: isis: " Stefan Hanreich
2025-07-09 19:45 ` [pve-devel] [PATCH pve-manager 1/1] cli: add pveeth Stefan Hanreich
2025-07-10 14:53   ` Gabriel Goller
2025-07-10 15:08     ` Thomas Lamprecht
2025-07-10 16:25       ` Gabriel Goller [this message]
2025-07-15 12:30         ` Stefan Hanreich
2025-07-15 12:35           ` Stefan Hanreich
2025-07-15 13:51           ` Thomas Lamprecht
2025-07-15 14:06             ` Stefan Hanreich
2025-07-15 15:02             ` Stefan Hanreich
2025-07-16 15:19 ` [pve-devel] superseded: [RFC common/firewall/manager/network/proxmox{-ve-rs, -firewall} 0/7] NIC renaming mitigations Stefan Hanreich

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=zg2albkln54cjeptbtf4t2upqpko3vg4p455hsnwoagxeiwhfp@ovjnen4uzz56 \
    --to=g.goller@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.lamprecht@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