On 7/15/25 14:36, Thomas Lamprecht wrote: > Am 15.07.25 um 13:06 schrieb Stefan Hanreich: >> On 7/15/25 12:33, Dominik Csapak wrote: >>> On 7/15/25 11:41, Stefan Hanreich wrote: >>>> On 7/15/25 11:30, Dominik Csapak wrote: >>>>> On 7/15/25 11:21, Stefan Hanreich wrote: >>>>>> Was formulating my response to v1, but you were too fast with sending a >>>>>> v2 for me. I think we need to also consider the case where we pinned >>>>>> and >>>>>> replaced the names in /e/n/i via pveeth already, but the changes to the >>>>>> network interface names haven't yet been applied. The problem here is >>>>>> that we also return interfaces contained in /proc/net/dev in the >>>>>> overview as well [1] - which would lead to duplicate interface names in >>>>>> the view. >>>>>> >>>>>> I considered applying the changes to the network interface names >>>>>> immediately via `udevadm trigger`. Alternatively, I thought of adding >>>>>> the old names as altnames when pinning network interfaces, but not in a >>>>>> persistent manner. I think both would solve this state between >>>>>> transitioning from the old names to the new names. What do you think? >>>>> >>>>> >>>>> ok, so from what i can tell 'pveeth pin' writes directly into the /e/n/i >>>>> file? >>>>> shouldn't it write to /e/n/i.new file (or whatever it's named) so we >>>>> show it as pending change? >>>> >>>> Currently yes, but I am still considering what would be the best way >>>> forward. I think you could make an argument for both. The problem is >>>> that while we have this mechanism of a temporary configuration file for >>>> /etc/network/interfaces and SDN, we do not have one for the firewall >>>> configuration. There is a dry_run functionality in pveeth, but it just >>>> generates the files in a different location. >>>> >>>> It might make more sense to implement it as a two-step process: >>>> >>>> * Pinning generates the new configuration files in the pending config of >>>> /e/n/i and SDN. For the firewall we'd have to create one as well and >>>> probably just handle this manually in the following step. >>>> >>>> * Add another command that applies the temporary changes which would >>>> also include applying the changes via udevadm immediately. >>>> >>>> Then users could inspect the generated changes via our UI (at least for >>>> Network / SDN). This would then also allow us to remove the dry_run >>>> functionality, since it would be implicitly included in this create / >>>> apply process. It would also solve the issue with this weird limbo state. >>>> >>>> I'm just unsure on how we could integrate showing the changes to the FW >>>> configuration in a nice way. > > but what changes are there required if it's _transparent_ altname support?
We would refer to the newly generated, pinned, names then in the configuration files. > While pending changes for FW might be nice in general, it ideally should > not be a require prerequisite for this effort now. Agree, that would too short notice to implement properly imo. >>> mhmm on the spot i'd probably say we could color code the fw rule changes >>> (e.g. yellow for pending changed, red for pending removed, etc.) >>> maybe also a seperate column with that info as text/symbol >> >> Yeah, I'll have to check how complicated that would be to implement - >> particularly when the two firewall rulesets start to diverge. But having >> a temporary firewall configuration similar to how /e/n/i or SDN works, >> was something that we wanted to explore anyway - so that could actually >> be a good starting point for that. >> >>>> >>>>> then we could do the udevadm trigger in the 'apply config' api handler? >>>>> >>>>> also, basically the 'altname' lookup code would also have to lookup the >>>>> .link files ? (seems like it's a lot of work/disk reading to do?) >>>> >>>> Yes, exactly - hence why I considered adding them as an altname since >>>> this would save us this procedure of parsing *all* link files in order >>>> to be able to generate a sensible view of /e/n/i. >>> >>> ah ok, so you'd do 'ip link property add dev ensX altname nic0' ? >>> that way you could immediately add that too for the new name, no? >>> >>> when you'd do that, we would not need to parse the link files >>> in the api code, since ip would already have that info ? >>> >> >> Yes, that was one of the solutions I had in mind, and that would be >> quite convenient for that case if we do not immediately apply the changes. > before I forget it, lets not name the executable "pveeth", that's hard > to read and it includes an outdated legacy named for NICs "eth" that's > basically burned for being used as pinned names. > > For now it would be probably best to name it in a rather specific way, > in the mid term we might get a common tool shared across products to > provide basic network status/config edit support, but until then I'd > not suggest that this evolves in some general network interface handling. > > Could be as verbose as: > proxmox-network-interface-pinning > > Less verbose but shorter: > proxmox-iface-pin Fine with me, pveeth was more of a placeholder anyway. > And then probably have two commands for preparing the pinning for one or > more interface and for applying the prepared changes. > As I'd find it very odd if I–as user–could create the preparation for > pinning here, but then would need to login on the web ui to apply > that on the various panels (in what order?!). > And ideally firewall and SDN handles this transparently, so it doesn't > matter if that configs get changed for the common case. See my post on the other thread for the pinning tool itself where I basically propose something like that. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
