[pve-devel] [PATCH common/docs/firewall/manager/proxmox-widget-toolkit v4 0/6] drop vmbr prefix for bridges

[pve-devel] [PATCH common/docs/firewall/manager/proxmox-widget-toolkit v4 0/6] drop vmbr prefix for bridges

[Stefan Hanreich]

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.

Dependencies:
  * pve-manager requires proxmox-widget-toolkit & pve-common
  * pve-firewall requires pve-common

Changes from v3 -> v4:
  * Improved wording of the documentation (thanks @Fabian!)
  * added information about dependencies

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

pve-common:

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

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


pve-docs:

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

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


pve-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(-)


pve-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(-)


proxmox-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(-)


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

-- 
Generated by git-murpp 0.6.0

[pve-devel] [PATCH pve-common v4 1/6] fix #545: interfaces: allow arbitrary bridge names in network config

[Stefan Hanreich]

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>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 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 6432295..4568593 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 (sort 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})) {
@@ -1127,6 +1126,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

[pve-devel] [PATCH pve-docs v4 2/6] network: update specification for bridge names

[Stefan Hanreich]

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 pve-network.adoc | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/pve-network.adoc b/pve-network.adoc
index d1ec64b..ef586ec 100644
--- a/pve-network.adoc
+++ b/pve-network.adoc
@@ -13,11 +13,11 @@ 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 Linux bridge interface (commonly called 'vmbrX') is needed to connect guests
+to the underlying physical network. It can be thought of as a virtual switch
+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 +75,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

[pve-devel] [PATCH pve-manager v4 3/6] sdn: qinq: vlan: properly validate bridge name

[Stefan Hanreich]

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
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

[pve-devel] [PATCH pve-manager v4 4/6] sdn: vlan: fix indentation in vlan edit dialogue

[Stefan Hanreich]

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
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

[pve-devel] [PATCH pve-firewall v4 5/6] simulator: use new bridge naming scheme

[Stefan Hanreich]

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

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
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

[pve-devel] [PATCH proxmox-widget-toolkit v4 6/6] network: allow bridges to have any valid interface name

[Stefan Hanreich]

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>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
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 09f4b8d..8a0138d 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

[pve-devel] applied-series: [PATCH common/docs/firewall/manager/proxmox-widget-toolkit v4 0/6] drop vmbr prefix for bridges

[Thomas Lamprecht]

Am 12/04/2024 um 10:07 schrieb Stefan Hanreich:
> 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.
> 
> Dependencies:
>   * pve-manager requires proxmox-widget-toolkit & pve-common
>   * pve-firewall requires pve-common
> 
> Changes from v3 -> v4:
>   * Improved wording of the documentation (thanks @Fabian!)
>   * added information about dependencies
> 
> 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
> 
> pve-common:
> 
> Stefan Hanreich (1):
>   fix #545: interfaces: allow arbitrary bridge names in network config
> 
>  src/PVE/INotify.pm | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> 
> pve-docs:
> 
> Stefan Hanreich (1):
>   network: update specification for bridge names
> 
>  pve-network.adoc | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> 
> pve-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(-)
> 
> 
> pve-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(-)
> 
> 
> proxmox-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(-)
> 
> 
> Summary over all repositories:
>   8 files changed, 60 insertions(+), 38 deletions(-)
> 


applied series, thanks!


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