public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: Lukas Sichert <l.sichert@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH access-control/cluster/manager/network/proxmox{-ve-rs,-perl-rs} v5 00/46] Add support for route maps / prefix lists to SDN
Date: Thu, 7 May 2026 17:17:51 +0200	[thread overview]
Message-ID: <n26bqgd7djb3szsmairhb4getzzjgiqarusivzhki6t6frn365@2xzq47yz7y4q> (raw)
In-Reply-To: <DICFD0B9UMWU.23B48RXCBI4K2@proxmox.com>

On 07.05.2026 13:57, Lukas Sichert wrote:
> On 2026-05-05 17:36, Stefan Hanreich <s.hanreich@proxmox.com> wrote:
> 
> > ## Introduction
> >
> > This patch adds support for managing route maps and prefix lists to the SDN
> > stack. With this patch series, route maps can be applied to the BGP and EVPN
> > controller for incoming / outgoing route filtering. Additionally, prefix lists
> > can be used to filter routes that should be installed by a fabric into the
> > kernel routing table, overriding the default behavior of only installing routes
> > from the configured IP prefix. There are currently some other features in
> > development that would make use of route maps as well, namely:
> >
> > * VRF route leaking
> > * Route Redistribution for Fabrics
> 
> On my host there are two SimpleZones configured. In the first SimpleZone
> there is a VM, which shall be called tm1 from now on. In the VNet there
> is a cluster consisting of the nodes cl1, cl2 and cl3. All the nodes can
> reach each other. On the cluster I configured an EVPN with an OpenFabric
> underlay, configured cl1 as the exit node and enabled 'Advertise
> Subnets'. In the EVPN Zone I created the 10.0.10.0/24 subnet.
> 
> Then on cl1 I added a bgp controller with EBGP and 'ebgp-multihop' set
> to 10 and the ip of tm1 set as peer. On tm1 I also added a bgp
> controller with the same configuration, but with cl1 as peer and a
> different ASN. In both cases I ran into a problem where 'delete' was not
> defined in the schema, but this could be resolved by applying [1].
> EBGP multihop was enabled here, because the traffic between the
> SimpleZones was routed via my host, which did not have a BGP controller
> configured.
> 
> I could then verify, that the 10.0.10.0/24 subnet was correctly
> adveritsed from cl1 to tm1. After configuring prefix lists and
> route-maps on both cl1 and tm1 I could verify that both incoming and
> outgoing route-maps work, as the 10.0.10.0/24 was not advertised
> anymore.
> 
> Here I ran into a problem though. I configured a prefix list with
> Prefix: 10.0.10.0/22
> Prefix<=:21
> This is of course wrong, but it is not checked, so applying it is
> possible. Here maybe a check in the frontend would be useful.
> After applying I got a warning and even after correcting the prefix list
> the warning did not go away. After deleting and reconfiguring the list
> it worked again, but apparently, when adding Prefix<=:, it is only
> possible to alter the value, not delete it. Deleting will just be
> ignored.

Quick patch that should fix this -- not sure if this still applies on top of
pve-manager after the last api-changes:


diff --git a/www/manager6/sdn/PrefixListPanel.js b/www/manager6/sdn/PrefixListPanel.js
index 8becf83435..bd4ce491dd 100644
--- a/www/manager6/sdn/PrefixListPanel.js
+++ b/www/manager6/sdn/PrefixListPanel.js
@@ -10,7 +10,16 @@
 
 Ext.define('PVE.sdn.PrefixListEntry', {
     extend: 'Ext.data.Model',
-    fields: ['id', 'action', 'prefix', 'le', 'ge', 'pending'],
+    fields: [
+        'id',
+        'action',
+        'prefix',
+        // keep empty optional fields as '' so clearing a value does not mark
+        // untouched empty fields as dirty when the form returns ''.
+        { name: 'le', defaultValue: '' },
+        { name: 'ge', defaultValue: '' },
+        'pending',
+    ],
 });
 
 Ext.define('PVE.sdn.EditPrefixListWindow', {
@@ -52,6 +61,48 @@
 
     isCreate: false,
 
+    validatePrefixRange: function(name) {
+        let me = this;
+        let prefix = me.down('[name=prefix]').getValue();
+        let value = me.down(`[name=${name}]`).getValue();
+
+        if (value === '') {
+            return true;
+        }
+
+        let match = Proxmox.Utils.IP64_cidr_match.exec(prefix);
+        if (!match) {
+            return true;
+        }
+
+        // IP64_cidr_match stores the IPv6 prefix length in match[1],
+        // and the IPv4 prefix length in match[2].
+        let isIPv6 = match[1] !== undefined;
+        let maskLength = Number(isIPv6 ? match[1] : match[2]);
+        let maxLength = isIPv6 ? 128 : 32;
+        let label = name === 'le' ? gettext('Prefix <=') : gettext('Prefix >=');
+        let number = Number(value);
+
+        if (!/^\d+$/.test(value) || number < maskLength || number > maxLength) {
+            return Ext.String.format(
+                gettext('{0} must be between {1} and {2}'),
+                label,
+                maskLength,
+                maxLength,
+            );
+        }
+
+        // only report the cross-field difference on the 'ge' input, so a valid
+        // 'le' value does not get marked invalid because of an invalid 'ge'.
+        let le = me.down('[name=le]').getValue();
+        let ge = me.down('[name=ge]').getValue();
+        if (name === 'ge' && le !== '' && Number(ge) > Number(le)) {
+            return gettext('Prefix >= must be less than or equal to Prefix <=');
+        }
+
+        return true;
+    },
+
     items: [
         {
             xtype: 'proxmoxKVComboBox',
@@ -68,16 +119,27 @@
             fieldLabel: gettext('Prefix'),
             name: 'prefix',
             vtype: 'IP64CIDRAddress',
+            allowBlank: false,
         },
         {
             xtype: 'proxmoxtextfield',
             fieldLabel: gettext('Prefix <='),
             name: 'le',
+            skipEmptyText: false,
+            maskRe: /[0-9]/,
+            validator: function() {
+                return this.up('window').validatePrefixRange('le');
+            },
         },
         {
             xtype: 'proxmoxtextfield',
             fieldLabel: gettext('Prefix >='),
             name: 'ge',
+            skipEmptyText: false,
+            maskRe: /[0-9]/,
+            validator: function() {
+                return this.up('window').validatePrefixRange('ge');
+            },
         },
     ],
 
@@ -294,8 +356,10 @@
                 .getData()
                 .items
                 .map((item) => {
-                    let data = item.data;
+                    // Clone record data before dropping grid-only metadata for serialization.
+                    let data = Ext.apply({}, item.data);
                     delete data.id;
+                    delete data.pending;
 
                     return PVE.Parser.printPropertyString(data);
                 });


One other thing I noticed is that we don't allow prefixes lower than /8 on ipv4
and lower than /64 on ipv6. This is weird, because in frr 0.0.0.0/0 is usually
used as a "allow-all" prefix. Not sure if we can change the existing input
format or if that will break something (its also used in migration and
replication). Will check that tomorrow.




  reply	other threads:[~2026-05-07 15:18 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05 15:36 [PATCH access-control/cluster/manager/network/proxmox{-ve-rs,-perl-rs} v5 00/46] Add support for route maps / prefix lists to SDN Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-cluster v5 01/46] cfs: add 'sdn/route-maps.cfg' to observed files Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-cluster v5 02/46] cfs: add 'sdn/prefix-lists.cfg' " Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-access-control v5 03/46] permissions: add ACL path for prefix-lists and route-maps Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 04/46] frr: add constructor to prefix list name Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 05/46] sdn-types: add common route-map helper types Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 06/46] frr: change order type to u16 Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 07/46] frr: implement routemap match/set statements via adjacent tagging Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 08/46] frr: implement support for call and exit action Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 09/46] frr-templates: change route maps template to adapt to new frr types Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 10/46] ve-config: fabrics: adapt frr config generation Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 11/46] ve-config: add prefix list section config Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 12/46] ve-config: frr: implement frr config generation for prefix lists Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 13/46] ve-config: add route map section config Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 14/46] ve-config: frr: implement frr config generation for route maps Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 15/46] ve-config: add prefix lists integration tests Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 16/46] ve-config: add route maps " Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 17/46] fabrics: ospf: fix deserializing OspfDeletableProperties Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 18/46] fabrics: ospf: openfabric: allow user-defined route filter Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-ve-rs v5 19/46] frr: fabrics: apply route_filter setting Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-perl-rs v5 20/46] pve-rs: sdn: add route maps module Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-perl-rs v5 21/46] pve-rs: sdn: add prefix lists module Stefan Hanreich
2026-05-05 15:36 ` [PATCH proxmox-perl-rs v5 22/46] sdn: add prefix list / route maps to frr config generation helper Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 23/46] controller: bgp: evpn: adapt to new match / set frr config syntax Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 24/46] sdn: add prefix lists module Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 25/46] sdn: add route map module Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 26/46] api2: add prefix list module Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 27/46] api2: add route maps module Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 28/46] api2: add route map module Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 29/46] api2: add route map entry module Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 30/46] evpn controller: add route_map_{in,out} parameter Stefan Hanreich
2026-05-05 15:36 ` [PATCH pve-network v5 31/46] bgp controller: allow configuring custom route maps Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-network v5 32/46] sdn: commit route map / prefix list configuration on sdn apply Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-network v5 33/46] sdn: frr: consider route maps and prefix lists in dry-run Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-network v5 34/46] fabrics: ospf: openfabric: add route_filter property Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-network v5 35/46] tests: add simple route map test case Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-network v5 36/46] tests: add bgp evpn route map/prefix list testcase Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-network v5 37/46] tests: add route map with prefix " Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-network v5 38/46] tests: add exit node with custom route map testcase Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-manager v5 39/46] ui: sdn: add route map selector Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-manager v5 40/46] ui: sdn: add prefix list selector Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-manager v5 41/46] ui: sdn: add panel for managing prefix lists Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-manager v5 42/46] ui: sdn: add panel for managing route map entries Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-manager v5 43/46] ui: sdn: bgp controller: allow configuring route maps Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-manager v5 44/46] ui: sdn: evpn " Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-manager v5 45/46] ui: sdn: openfabric: add route filter Stefan Hanreich
2026-05-05 15:37 ` [PATCH pve-manager v5 46/46] ui: sdn: ospf: add route filter setting Stefan Hanreich
2026-05-05 23:33 ` partially-applied: [PATCH access-control/cluster/manager/network/proxmox{-ve-rs,-perl-rs} v5 00/46] Add support for route maps / prefix lists to SDN Thomas Lamprecht
2026-05-06  0:08 ` applied: " Thomas Lamprecht
2026-05-06  1:25 ` partially-applied: " Thomas Lamprecht
2026-05-06  8:03   ` Stefan Hanreich
2026-05-06 14:25 ` Gabriel Goller
2026-05-07 11:57 ` Lukas Sichert
2026-05-07 15:17   ` Gabriel Goller [this message]
2026-05-08 16:39   ` Stefan Hanreich
2026-05-08 16:38 ` superseded: " 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=n26bqgd7djb3szsmairhb4getzzjgiqarusivzhki6t6frn365@2xzq47yz7y4q \
    --to=g.goller@proxmox.com \
    --cc=l.sichert@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