public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges
@ 2024-02-29 10:40 Stefan Hanreich
  2024-02-29 10:40 ` [pve-devel] [PATCH v3 common 1/6] interfaces: allow arbitrary bridge names in network config Stefan Hanreich
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Stefan Hanreich @ 2024-02-29 10:40 UTC (permalink / raw)
  To: pve-devel

Original patch series by Jillian Morgan <jillian.morgan@primordial.ca>

I've refrained from adding arbitrary bond names in this patch series, since
that would require a bigger amount of changes in the firewall simulator. I'll
look into adding that in a future patch series.

Changes from v2 -> v3:
  * limit bridge names to 10 characters in Web UI
  * introduce common variable for bridge names in firewall simulator
  * update docs to reflect changes
  * add warning for interfaces named vmbrX that are not bridges

Changes from v1 -> v2:
  * limit name to 15 characters
  * properly validate bridge names in vlan/qinq zones
  * properly handle OVS bridges
  * handle new bridge names in firewall simulator



common:

Stefan Hanreich (1):
  interfaces: allow arbitrary bridge names in network config

 src/PVE/INotify.pm | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)


docs:

Stefan Hanreich (1):
  network: update specification for bridge names

 pve-network.adoc | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)


widget-toolkit:

Stefan Hanreich (1):
  network: allow bridges to have any valid interface name

 src/Toolkit.js          | 4 ++--
 src/node/NetworkEdit.js | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)


manager:

Stefan Hanreich (2):
  sdn: qinq: vlan: properly validate bridge name
  sdn: vlan: fix indentation in vlan edit dialogue

 www/manager6/sdn/zones/QinQEdit.js |  3 +++
 www/manager6/sdn/zones/VlanEdit.js | 11 +++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)


firewall:

Stefan Hanreich (1):
  simulator: use new bridge naming scheme

 src/PVE/FirewallSimulator.pm    | 29 +++++++++++++++++++----------
 src/PVE/Service/pve_firewall.pm |  5 +++--
 2 files changed, 22 insertions(+), 12 deletions(-)


Summary over all repositories:
  8 files changed, 61 insertions(+), 38 deletions(-)

-- 
murpp v0.4.0




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH v3 common 1/6] interfaces: allow arbitrary bridge names in network config
  2024-02-29 10:40 [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich
@ 2024-02-29 10:40 ` Stefan Hanreich
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 docs 2/6] network: update specification for bridge names Stefan Hanreich
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Stefan Hanreich @ 2024-02-29 10:40 UTC (permalink / raw)
  To: pve-devel

Similar to other interface types, we can detect a bridge by the
presence of its bridge_ports attribute, rather than solely relying on
the "vmbr" ifname prefix heuristic. For OVS bridges we need to examine
the OVS type instead.

The check needs to be moved up since other prefixes could
theoretically be included in a bridge name and then would otherwise
get picked up wrongly.

Also added a warning for interfaces named vmbrX that are not bridges
to catch possible misconfigurations.

Originally-by: Jillian Morgan <jillian.morgan@primordial.ca>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---

Warning in the read function is a bit awkward, since for one
interfaces multiple warnings are generated - due to multiple calls to
the read function during the apply process. I think it's ok though due
to this being a relatively fringe warning which hopefully shouldn't
come up too often...

 src/PVE/INotify.pm | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index bc33a8f..13724f7 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -22,6 +22,7 @@ use PVE::Network;
 use PVE::ProcFSTools;
 use PVE::SafeSyslog;
 use PVE::Tools;
+use PVE::RESTEnvironment qw(log_warn);
 
 use base 'Exporter';
 
@@ -1033,7 +1034,17 @@ sub __read_etc_network_interfaces {
     foreach my $iface (keys %$ifaces) {
 	my $d = $ifaces->{$iface};
 	$d->{type} = 'unknown';
-	if ($iface =~ m/^bond\d+$/) {
+	if (defined $d->{'bridge_ports'}) {
+	    $d->{type} = 'bridge';
+	    if (!defined ($d->{bridge_stp})) {
+		$d->{bridge_stp} = 'off';
+	    }
+	    if (!defined($d->{bridge_fd}) && $d->{bridge_stp} eq 'off') {
+		$d->{bridge_fd} = 0;
+	    }
+	} elsif ($d->{ovs_type} && $d->{ovs_type} eq 'OVSBridge') {
+	    $d->{type} = $d->{ovs_type};
+	} elsif ($iface =~ m/^bond\d+$/) {
 	    if (!$d->{ovs_type}) {
 		$d->{type} = 'bond';
 	    } elsif ($d->{ovs_type} eq 'OVSBond') {
@@ -1053,18 +1064,6 @@ sub __read_etc_network_interfaces {
 		my $tag = &$extract_ovs_option($d, 'tag');
 		$d->{ovs_tag} = $tag if defined($tag);
 	    }
-	} elsif ($iface =~ m/^vmbr\d+$/) {
-	    if (!$d->{ovs_type}) {
-		$d->{type} = 'bridge';
-		if (!defined ($d->{bridge_stp})) {
-		    $d->{bridge_stp} = 'off';
-		}
-		if (!defined($d->{bridge_fd}) && $d->{bridge_stp} eq 'off') {
-		    $d->{bridge_fd} = 0;
-		}
-	    } elsif ($d->{ovs_type} eq 'OVSBridge') {
-		$d->{type} = $d->{ovs_type};
-	    }
 	} elsif ($iface =~ m/^(\S+):\d+$/) {
 	    $d->{type} = 'alias';
 	    if (defined ($ifaces->{$1})) {
@@ -1116,6 +1115,9 @@ sub __read_etc_network_interfaces {
 	    }
 	}
 
+	log_warn("detected a interface $iface that is not a bridge!")
+	    if !($d->{type} eq 'OVSBridge' || $d->{type} eq 'bridge') && $iface =~ m/^vmbr\d+$/;
+
 	# map address and netmask to cidr
 	if (my $addr = $d->{address}) {
 	    if (_address_is_cidr($addr)) {
-- 
2.39.2




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH v3 docs 2/6] network: update specification for bridge names
  2024-02-29 10:40 [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich
  2024-02-29 10:40 ` [pve-devel] [PATCH v3 common 1/6] interfaces: allow arbitrary bridge names in network config Stefan Hanreich
@ 2024-02-29 10:41 ` Stefan Hanreich
  2024-04-11 14:21   ` Fabian Grünbichler
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 widget-toolkit 3/6] network: allow bridges to have any valid interface name Stefan Hanreich
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Stefan Hanreich @ 2024-02-29 10:41 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 pve-network.adoc | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/pve-network.adoc b/pve-network.adoc
index d1ec64b..a5ad9b4 100644
--- a/pve-network.adoc
+++ b/pve-network.adoc
@@ -13,11 +13,12 @@ page contains the complete format description. All {pve} tools try hard to keep
 direct user modifications, but using the GUI is still preferable, because it
 protects you from errors.
 
-A 'vmbr' interface is needed to connect guests to the underlying physical
-network.  They are a Linux bridge which can be thought of as a virtual switch
-to which the guests and physical interfaces are connected to.  This section
-provides some examples on how the network can be set up to accomodate different
-use cases like redundancy with a xref:sysadmin_network_bond['bond'],
+A bridge interface (commonly called 'vmbrX') is needed to connect guests to the
+underlying physical network.  They are a Linux bridge which can be thought of as
+a virtual switch to which the guests and physical interfaces are connected to.
+This section provides some examples on how the network can be set up to
+accomodate different use cases like redundancy with a
+xref:sysadmin_network_bond['bond'],
 xref:sysadmin_network_vlan['vlans'] or
 xref:sysadmin_network_routed['routed'] and
 xref:sysadmin_network_masquerading['NAT'] setups.
@@ -75,7 +76,9 @@ We currently use the following naming conventions for device names:
 scheme is used for {pve} hosts which were installed before the 5.0
 release. When upgrading to 5.0, the names are kept as-is.
 
-* Bridge names: `vmbr[N]`, where 0 ≤ N ≤ 4094 (`vmbr0` - `vmbr4094`)
+* Bridge names: Commonly `vmbr[N]`, where 0 ≤ N ≤ 4094 (`vmbr0` - `vmbr4094`),
+but you can use any alphanumeric string that starts with a character and is at
+most 10 characters long.
 
 * Bonds: `bond[N]`, where 0 ≤ N (`bond0`, `bond1`, ...)
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH v3 widget-toolkit 3/6] network: allow bridges to have any valid interface name
  2024-02-29 10:40 [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich
  2024-02-29 10:40 ` [pve-devel] [PATCH v3 common 1/6] interfaces: allow arbitrary bridge names in network config Stefan Hanreich
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 docs 2/6] network: update specification for bridge names Stefan Hanreich
@ 2024-02-29 10:41 ` Stefan Hanreich
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 manager 4/6] sdn: qinq: vlan: properly validate bridge name Stefan Hanreich
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Stefan Hanreich @ 2024-02-29 10:41 UTC (permalink / raw)
  To: pve-devel

Allow the web UI to accept bridge interfaces with any valid interface
name, rather than being limited to the arbitrary "vmbr" prefix.

Limiting to at most 10 characters, since SDN possibly adds a .XXXX
prefix for Vlans. Since the hard limit for network interface names is
15 characters, limiting it to 10 characters here enables SDN to append
the VLAN prefix in any case.

Originally-by: Jillian Morgan <jillian.morgan@primordial.ca>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/Toolkit.js          | 4 ++--
 src/node/NetworkEdit.js | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/Toolkit.js b/src/Toolkit.js
index 6fd73f5..23d3a36 100644
--- a/src/Toolkit.js
+++ b/src/Toolkit.js
@@ -76,7 +76,7 @@ Ext.apply(Ext.form.field.VTypes, {
     MacPrefixText: gettext('Example') + ': 02:8f - ' + gettext('only unicast addresses are allowed'),
 
     BridgeName: function(v) {
-	return (/^vmbr\d{1,4}$/).test(v);
+	return (/^[a-zA-Z][a-zA-Z0-9_]{0,9}$/).test(v);
     },
     VlanName: function(v) {
        if (Proxmox.Utils.VlanInterface_match.test(v)) {
@@ -86,7 +86,7 @@ Ext.apply(Ext.form.field.VTypes, {
        }
        return true;
     },
-    BridgeNameText: gettext('Format') + ': vmbr<b>N</b>, where 0 <= <b>N</b> <= 9999',
+    BridgeNameText: gettext('Format') + ': alphanumeric string starting with a character',
 
     BondName: function(v) {
 	return (/^bond\d{1,4}$/).test(v);
diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js
index bb9add3..33113d9 100644
--- a/src/node/NetworkEdit.js
+++ b/src/node/NetworkEdit.js
@@ -38,6 +38,8 @@ Ext.define('Proxmox.node.NetworkEdit', {
 	    throw "unknown network device type specified";
 	}
 
+	let name_max_length = iface_vtype === 'BridgeName' ? 10 : 15;
+
 	me.subject = Proxmox.Utils.render_network_iface_type(me.iftype);
 
 	let column1 = [],
@@ -254,7 +256,7 @@ Ext.define('Proxmox.node.NetworkEdit', {
 	    value: me.iface,
 	    vtype: iface_vtype,
 	    allowBlank: false,
-	    maxLength: 15,
+	    maxLength: name_max_length,
 	    autoEl: {
 		tag: 'div',
 		 'data-qtip': gettext('For example, vmbr0.100, vmbr0, vlan0.100, vlan0'),
-- 
2.39.2




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH v3 manager 4/6] sdn: qinq: vlan: properly validate bridge name
  2024-02-29 10:40 [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich
                   ` (2 preceding siblings ...)
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 widget-toolkit 3/6] network: allow bridges to have any valid interface name Stefan Hanreich
@ 2024-02-29 10:41 ` Stefan Hanreich
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 manager 5/6] sdn: vlan: fix indentation in vlan edit dialogue Stefan Hanreich
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Stefan Hanreich @ 2024-02-29 10:41 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/sdn/zones/QinQEdit.js | 3 +++
 www/manager6/sdn/zones/VlanEdit.js | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/www/manager6/sdn/zones/QinQEdit.js b/www/manager6/sdn/zones/QinQEdit.js
index 795ff9dfd..de177d7cb 100644
--- a/www/manager6/sdn/zones/QinQEdit.js
+++ b/www/manager6/sdn/zones/QinQEdit.js
@@ -24,6 +24,9 @@ Ext.define('PVE.sdn.zones.QinQInputPanel', {
 		name: 'bridge',
 		fieldLabel: 'Bridge',
 		allowBlank: false,
+		vtype: 'BridgeName',
+		minLength: 1,
+		maxLength: 10,
 	    },
 	    {
 		xtype: 'proxmoxintegerfield',
diff --git a/www/manager6/sdn/zones/VlanEdit.js b/www/manager6/sdn/zones/VlanEdit.js
index 23530bfcf..7f7ccca41 100644
--- a/www/manager6/sdn/zones/VlanEdit.js
+++ b/www/manager6/sdn/zones/VlanEdit.js
@@ -24,6 +24,9 @@ Ext.define('PVE.sdn.zones.VlanInputPanel', {
             name: 'bridge',
             fieldLabel: 'Bridge',
             allowBlank: false,
+	    vtype: 'BridgeName',
+	    minLength: 1,
+	    maxLength: 10,
           },
 	];
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH v3 manager 5/6] sdn: vlan: fix indentation in vlan edit dialogue
  2024-02-29 10:40 [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich
                   ` (3 preceding siblings ...)
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 manager 4/6] sdn: qinq: vlan: properly validate bridge name Stefan Hanreich
@ 2024-02-29 10:41 ` Stefan Hanreich
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 firewall 6/6] simulator: use new bridge naming scheme Stefan Hanreich
  2024-04-11 14:21 ` [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Fabian Grünbichler
  6 siblings, 0 replies; 10+ messages in thread
From: Stefan Hanreich @ 2024-02-29 10:41 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/sdn/zones/VlanEdit.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/www/manager6/sdn/zones/VlanEdit.js b/www/manager6/sdn/zones/VlanEdit.js
index 7f7ccca41..0bef5c8ec 100644
--- a/www/manager6/sdn/zones/VlanEdit.js
+++ b/www/manager6/sdn/zones/VlanEdit.js
@@ -20,10 +20,10 @@ Ext.define('PVE.sdn.zones.VlanInputPanel', {
 
         me.items = [
           {
-            xtype: 'textfield',
-            name: 'bridge',
-            fieldLabel: 'Bridge',
-            allowBlank: false,
+	    xtype: 'textfield',
+	    name: 'bridge',
+	    fieldLabel: 'Bridge',
+	    allowBlank: false,
 	    vtype: 'BridgeName',
 	    minLength: 1,
 	    maxLength: 10,
-- 
2.39.2




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH v3 firewall 6/6] simulator: use new bridge naming scheme
  2024-02-29 10:40 [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich
                   ` (4 preceding siblings ...)
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 manager 5/6] sdn: vlan: fix indentation in vlan edit dialogue Stefan Hanreich
@ 2024-02-29 10:41 ` Stefan Hanreich
  2024-04-11 14:21 ` [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Fabian Grünbichler
  6 siblings, 0 replies; 10+ messages in thread
From: Stefan Hanreich @ 2024-02-29 10:41 UTC (permalink / raw)
  To: pve-devel

We now allow bridges without the vmbr prefix, so we need to allow them
here in the simulator as well.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/FirewallSimulator.pm    | 29 +++++++++++++++++++----------
 src/PVE/Service/pve_firewall.pm |  5 +++--
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/PVE/FirewallSimulator.pm b/src/PVE/FirewallSimulator.pm
index 140c46e..fa5ed0e 100644
--- a/src/PVE/FirewallSimulator.pm
+++ b/src/PVE/FirewallSimulator.pm
@@ -7,6 +7,12 @@ use PVE::Firewall;
 use File::Basename;
 use Net::IP;
 
+use base 'Exporter';
+our @EXPORT_OK = qw(
+$bridge_name_pattern
+$bridge_interface_pattern
+);
+
 # dynamically include PVE::QemuServer and PVE::LXC
 # to avoid dependency problems
 my $have_qemu_server;
@@ -27,6 +33,9 @@ my $debug = 0;
 
 my $NUMBER_RE = qr/0x[0-9a-fA-F]+|\d+/;
 
+our $bridge_name_pattern = '[a-zA-Z][a-zA-Z0-9]{0,9}';
+our $bridge_interface_pattern = "($bridge_name_pattern)/(\\S+)";
+
 sub debug {
     my $new_value = shift;
     $debug = $new_value if defined($new_value);
@@ -397,7 +406,7 @@ sub route_packet {
 	    $pkg->{physdev_in} = $target->{fwln} || die 'internal error';
 	    $pkg->{physdev_out} = $target->{tapdev} || die 'internal error';
 
-	} elsif ($route_state =~ m/^vmbr\d+$/) {
+	} elsif ($route_state =~ m/^$bridge_name_pattern$/) {
 
 	    die "missing physdev_in - internal error?" if !$physdev_in;
 	    $pkg->{physdev_in} = $physdev_in;
@@ -531,11 +540,6 @@ sub simulate_firewall {
 	$from_info->{type} = 'host';
 	$start_state = 'host';
 	$pkg->{source} = $host_ip if !defined($pkg->{source});
-    } elsif ($from =~ m|^(vmbr\d+)/(\S+)$|) {
-	$from_info->{type} = 'bport';
-	$from_info->{bridge} = $1;
-	$from_info->{iface} = $2;
-	$start_state = 'from-bport';
     } elsif ($from eq 'outside') {
 	$from_info->{type} = 'bport';
 	$from_info->{bridge} = 'vmbr0';
@@ -559,6 +563,11 @@ sub simulate_firewall {
 	$from_info = extract_vm_info($vmdata, $vmid, $netnum);
 	$start_state = 'fwbr-out';
 	$pkg->{mac_source} = $from_info->{macaddr};
+    } elsif ($from =~ m|^$bridge_interface_pattern$|) {
+	$from_info->{type} = 'bport';
+	$from_info->{bridge} = $1;
+	$from_info->{iface} = $2;
+	$start_state = 'from-bport';
     } else {
 	die "unable to parse \"from => '$from'\"\n";
     }
@@ -569,10 +578,6 @@ sub simulate_firewall {
 	$target->{type} = 'host';
 	$target->{iface} = 'host';
 	$pkg->{dest} = $host_ip if !defined($pkg->{dest});
-    } elsif ($to =~ m|^(vmbr\d+)/(\S+)$|) {
-	$target->{type} = 'bport';
-	$target->{bridge} = $1;
-	$target->{iface} = $2;
     } elsif ($to eq 'outside') {
 	$target->{type} = 'bport';
 	$target->{bridge} = 'vmbr0';
@@ -591,6 +596,10 @@ sub simulate_firewall {
 	my $vmid = $1;
 	$target = extract_vm_info($vmdata, $vmid, 0);
 	$target->{iface} = $target->{tapdev};
+    } elsif ($to =~ m|^$bridge_interface_pattern$|) {
+	$target->{type} = 'bport';
+	$target->{bridge} = $1;
+	$target->{iface} = $2;
     } else {
 	die "unable to parse \"to => '$to'\"\n";
     }
diff --git a/src/PVE/Service/pve_firewall.pm b/src/PVE/Service/pve_firewall.pm
index 30d14d9..65cb2b8 100755
--- a/src/PVE/Service/pve_firewall.pm
+++ b/src/PVE/Service/pve_firewall.pm
@@ -18,6 +18,7 @@ use PVE::Tools qw(dir_glob_foreach file_read_firstline);
 
 use PVE::Firewall;
 use PVE::FirewallSimulator;
+use PVE::FirewallSimulator qw($bridge_interface_pattern);
 
 use base qw(PVE::Daemon);
 
@@ -312,14 +313,14 @@ __PACKAGE__->register_method ({
 	    from => {
 		description => "Source zone.",
 		type => 'string',
-		pattern => '(host|outside|vm\d+|ct\d+|vmbr\d+/\S+)',
+		pattern => "(host|outside|vm\\d+|ct\\d+|$bridge_interface_pattern)",
 		optional => 1,
 		default => 'outside',
 	    },
 	    to => {
 		description => "Destination zone.",
 		type => 'string',
-		pattern => '(host|outside|vm\d+|ct\d+|vmbr\d+/\S+)',
+		pattern => "(host|outside|vm\\d+|ct\\d+|$bridge_interface_pattern)",
 		optional => 1,
 		default => 'host',
 	    },
-- 
2.39.2




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [pve-devel] [PATCH v3 docs 2/6] network: update specification for bridge names
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 docs 2/6] network: update specification for bridge names Stefan Hanreich
@ 2024-04-11 14:21   ` Fabian Grünbichler
  0 siblings, 0 replies; 10+ messages in thread
From: Fabian Grünbichler @ 2024-04-11 14:21 UTC (permalink / raw)
  To: Proxmox VE development discussion

On February 29, 2024 11:41 am, Stefan Hanreich wrote:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  pve-network.adoc | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/pve-network.adoc b/pve-network.adoc
> index d1ec64b..a5ad9b4 100644
> --- a/pve-network.adoc
> +++ b/pve-network.adoc
> @@ -13,11 +13,12 @@ page contains the complete format description. All {pve} tools try hard to keep
>  direct user modifications, but using the GUI is still preferable, because it
>  protects you from errors.
>  
> -A 'vmbr' interface is needed to connect guests to the underlying physical
> -network.  They are a Linux bridge which can be thought of as a virtual switch
> -to which the guests and physical interfaces are connected to.  This section
> -provides some examples on how the network can be set up to accomodate different
> -use cases like redundancy with a xref:sysadmin_network_bond['bond'],
> +A bridge interface (commonly called 'vmbrX') is needed to connect guests to the
> +underlying physical network.  They are a Linux bridge which can be thought of as

both pre-existing, but while we are here:

doubled space before the 'They'
"*A* bridge interface .. *They* are.."

I'd just combine the two?

A Linux bridge interface .. . It can be though ..

> +a virtual switch to which the guests and physical interfaces are connected to.

also pre-existing: 

the first 'to' here can go IMHO ;)

a virtual switch which the .. are connected to.

> +This section provides some examples on how the network can be set up to
> +accomodate different use cases like redundancy with a
> +xref:sysadmin_network_bond['bond'],
>  xref:sysadmin_network_vlan['vlans'] or
>  xref:sysadmin_network_routed['routed'] and
>  xref:sysadmin_network_masquerading['NAT'] setups.
> @@ -75,7 +76,9 @@ We currently use the following naming conventions for device names:
>  scheme is used for {pve} hosts which were installed before the 5.0
>  release. When upgrading to 5.0, the names are kept as-is.
>  
> -* Bridge names: `vmbr[N]`, where 0 ≤ N ≤ 4094 (`vmbr0` - `vmbr4094`)
> +* Bridge names: Commonly `vmbr[N]`, where 0 ≤ N ≤ 4094 (`vmbr0` - `vmbr4094`),
> +but you can use any alphanumeric string that starts with a character and is at
> +most 10 characters long.
>  
>  * Bonds: `bond[N]`, where 0 ≤ N (`bond0`, `bond1`, ...)




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges
  2024-02-29 10:40 [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich
                   ` (5 preceding siblings ...)
  2024-02-29 10:41 ` [pve-devel] [PATCH v3 firewall 6/6] simulator: use new bridge naming scheme Stefan Hanreich
@ 2024-04-11 14:21 ` Fabian Grünbichler
  2024-04-12  7:46   ` Stefan Hanreich
  6 siblings, 1 reply; 10+ messages in thread
From: Fabian Grünbichler @ 2024-04-11 14:21 UTC (permalink / raw)
  To: Proxmox VE development discussion

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

with some small nits for the docs patch, see comment there.

the pve-common patch should probably get the bug number (545) in the
subject as well.

some hint about the inter-dependencies of the patches would be nice, AFAIU:
- pve-manager requires proxmox-widget-toolkit
- the UI changes require pve-common, but since that is cluster-wide, a
  dep there only helps the local node as usual (still, it might be a
  good idea so at least that combo is covered for sure ;))
- the firewall change is rather independent, since it's just the
  simulator that is affected at worst it won't handle a new interface
  name.

I think this one would make quite a few users happy (at least those that
haven't yet switched to SDN with its more flexible naming scheme ;))

On February 29, 2024 11:40 am, Stefan Hanreich wrote:
> Original patch series by Jillian Morgan <jillian.morgan@primordial.ca>
> 
> I've refrained from adding arbitrary bond names in this patch series, since
> that would require a bigger amount of changes in the firewall simulator. I'll
> look into adding that in a future patch series.
> 
> Changes from v2 -> v3:
>   * limit bridge names to 10 characters in Web UI
>   * introduce common variable for bridge names in firewall simulator
>   * update docs to reflect changes
>   * add warning for interfaces named vmbrX that are not bridges
> 
> Changes from v1 -> v2:
>   * limit name to 15 characters
>   * properly validate bridge names in vlan/qinq zones
>   * properly handle OVS bridges
>   * handle new bridge names in firewall simulator
> 
> 
> 
> common:
> 
> Stefan Hanreich (1):
>   interfaces: allow arbitrary bridge names in network config
> 
>  src/PVE/INotify.pm | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> 
> docs:
> 
> Stefan Hanreich (1):
>   network: update specification for bridge names
> 
>  pve-network.adoc | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> 
> widget-toolkit:
> 
> Stefan Hanreich (1):
>   network: allow bridges to have any valid interface name
> 
>  src/Toolkit.js          | 4 ++--
>  src/node/NetworkEdit.js | 4 +++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> 
> manager:
> 
> Stefan Hanreich (2):
>   sdn: qinq: vlan: properly validate bridge name
>   sdn: vlan: fix indentation in vlan edit dialogue
> 
>  www/manager6/sdn/zones/QinQEdit.js |  3 +++
>  www/manager6/sdn/zones/VlanEdit.js | 11 +++++++----
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> 
> firewall:
> 
> Stefan Hanreich (1):
>   simulator: use new bridge naming scheme
> 
>  src/PVE/FirewallSimulator.pm    | 29 +++++++++++++++++++----------
>  src/PVE/Service/pve_firewall.pm |  5 +++--
>  2 files changed, 22 insertions(+), 12 deletions(-)
> 
> 
> Summary over all repositories:
>   8 files changed, 61 insertions(+), 38 deletions(-)
> 
> -- 
> murpp v0.4.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges
  2024-04-11 14:21 ` [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Fabian Grünbichler
@ 2024-04-12  7:46   ` Stefan Hanreich
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Hanreich @ 2024-04-12  7:46 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

On 4/11/24 16:21, Fabian Grünbichler wrote:
> Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

Thanks a lot!

> with some small nits for the docs patch, see comment there.
> 
> the pve-common patch should probably get the bug number (545) in the
> subject as well.

I'll get a v4 ready ASAP, shouldn't take long..




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-04-12  7:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-29 10:40 [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Stefan Hanreich
2024-02-29 10:40 ` [pve-devel] [PATCH v3 common 1/6] interfaces: allow arbitrary bridge names in network config Stefan Hanreich
2024-02-29 10:41 ` [pve-devel] [PATCH v3 docs 2/6] network: update specification for bridge names Stefan Hanreich
2024-04-11 14:21   ` Fabian Grünbichler
2024-02-29 10:41 ` [pve-devel] [PATCH v3 widget-toolkit 3/6] network: allow bridges to have any valid interface name Stefan Hanreich
2024-02-29 10:41 ` [pve-devel] [PATCH v3 manager 4/6] sdn: qinq: vlan: properly validate bridge name Stefan Hanreich
2024-02-29 10:41 ` [pve-devel] [PATCH v3 manager 5/6] sdn: vlan: fix indentation in vlan edit dialogue Stefan Hanreich
2024-02-29 10:41 ` [pve-devel] [PATCH v3 firewall 6/6] simulator: use new bridge naming scheme Stefan Hanreich
2024-04-11 14:21 ` [pve-devel] [PATCH v3 common/docs/widget-toolkit/manager/firewall 0/6] drop vmbr prefix for bridges Fabian Grünbichler
2024-04-12  7:46   ` Stefan Hanreich

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