From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 5B43C1FF0A7 for ; Wed, 30 Sep 2026 09:44:39 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id DAB1721602; Wed, 30 Sep 2026 09:44:34 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 30 Sep 2026 09:44:25 +0200 Message-Id: Subject: Re: SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away From: "Thomas Ellmenreich" To: "Arthur Bied-Charreton" , X-Mailer: aerc 0.20.0 References: <20260925094230.844917-1-a.bied-charreton@proxmox.com> In-Reply-To: <20260925094230.844917-1-a.bied-charreton@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790754265979 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.813 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: RFYYVT6COXHQRRQTTBQ45NJGQBQWPTVC X-Message-ID-Hash: RFYYVT6COXHQRRQTTBQ45NJGQBQWPTVC X-MailFrom: t.ellmenreich@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Thanks for sending in this series! I ran some basic tests on the renaming and dropping functionality and found that everything worked well. Looking at the different operations that now have a new option to deal with references, 'disable' is always possible, why is that not the case for rena= me. There might be a technical reason that I'm currently not thinking of, but I can see 'disabling' the referencing rules being a worthwhile option when performing a rename. The tests I performed were: Renaming of aliases and ipsets: For this, I created aliases both at the cluster and vm level. I then referenced these aliases in the firewall rules of the cluster, node and= vm. The aliases and ipsets defined at the vm level were only referenced in = vm level rules. Performing some renames with the different settings acted exactly as expected. I also used an alias in one of the ip sets and the rename worked perfectly in that case as well. Simple benchmark for rename: By creating 500 guests and then defining a firewall rule on each guest referencing a cluster level alias, I could then benchmark the renaming = of the alias. On average it took ~2,5 seconds which I find reasonable. Deletion of vms: When deleting a vm, one has the option to keep/disable/delete the referencing rules. After creating the 500 vms for the benchmark, I adde= d some more rules that reference the IPAM aliases. Using keep/disable/del= ete all worked exactly as expected. So, aside from the question mentioned at the very beginning, consider this: Tested-by: Thomas Ellmenreich On Fri Sep 25, 2026 at 11:42 AM CEST, Arthur Bied-Charreton wrote: > Renaming or deleting a firewall object (an IPSet or an alias) that rules > still reference leaves those references dangling. The firewall fails to > parse the affected rules and drops them from the generated ruleset, so > an edit in one place can disable a whole set of rules somewhere. This > is especially bad in the rename case, where one might reasonably expect > the references to follow the new object name. > > This series makes the affected operations offer to deal with the > references instead of leaving them behind. > > pve-firewall gains a shared helper, update_refs(), whcih finds > references in rules, security groups and IPset members and applies one > of three actions: 'rename' points them at the new name, 'disable' > disables the referencing rules, and 'drop' removes them. An IPSet member > has no disabled state, so 'disable' removes members as well. For a > cluster object, the helper also walks every downstream config in the > cluster (guest, host and vnet), locking and saving each one. > > On top of this, three wrappers cover the SDN-generated IPSets that no > caller can delete directly. > > These options are added to the API as follows: > > POST .../firewall/ipset update-references no|yes|force > PUT .../firewall/aliases/{name} update-references no|yes|force > DELETE .../ipset/{name} dangling-references keep|disable|dr= op > DELETE .../aliases/{name} dangling-references keep|disable|dr= op > PUT /cluster/sdn dangling-ipset-references keep|disa= ble|drop > DELETE /nodes/{node}/qemu/{vmid} dangling-ipset-references keep|disa= ble|drop > DELETE /nodes/{node}/lxc/{vmid} dangling-ipset-references keep|disa= ble|drop > > On a rename, the new object is persisted before its references are > rewritten, so a concurrent compilation does not observe a reference to > an object that does not exist yet. If a cluster-wide rewrite is > interrupted part way, passing 'force' resumes it (without this, the > next attempt would fail due to the target name already existing in the > config). > > While these options allow to trigger changes to the firewall > configurations from other endpoints requiring different permissions, > like guest destroy and SDN apply, they do not require any additional > permissions, see full explanation here [0]. > > Changes since [v2]: > - Handle references to SDN-generated IPSets in SDN apply and guest > destroy > - Support disabling rules referencing a deleted object instead of only > deleting them > > [v2] https://lore.proxmox.com/pve-devel/20260818133413.450776-1-a.bied-ch= arreton@proxmox.com/ > [v1] https://lore.proxmox.com/pve-devel/20260407073658.90818-1-a.bied-cha= rreton@proxmox.com/ > > [0] https://lore.proxmox.com/pve-devel/awaoshjzbr7adisjngsrcts4zs3hxgoxw2= lgcoq66gtiap3al2@mpxrbmtyfcwj/ [snip]