[pve-devel] [PATCH common 1/1] Fix #545: Allow descriptive names for network bridges and bonds

Fabian Grünbichler f.gruenbichler at proxmox.com
Fri Jun 7 18:15:43 CEST 2019 

On Thu, Jun 06, 2019 at 12:09:23PM +0200, Alexandre DERUMIER wrote:
> >>- 11 characters left for hypervisor interfaces (vmbr/vnet/bond all have
> >>  4 characters)
> 
> and also it's possible to have 4096 vlans (bondX.4096),
> so it's 6 characters left for tagged interfaces

fair enough - and since that last part counts as well, it makes our existing
format/option even more wrong:

JSONSchema.pm

  71 register_standard_option('pve-iface', {
  72     description => "Network interface name.",
  73     type => 'string', format => 'pve-iface',
  74     minLength => 2, maxLength => 20,
  75 });

 407     if ($id !~ m/^[a-z][a-z0-9_]{1,20}([:\.]\d+)?$/i) {

> >>the interface name is used/displayed/supported across the board, the 
> >>alias likely is not (yet). e.g. stuff like references in config files, 
> >>firewall rules, monitoring scripts, metrics/billing, ... 
> 
> I was more thinking about using this alias for gui only.
> (or even a simple comment in /etc/network/interfaces could work too),
> 
> and keep the interfaceid reference in all config files.
> 
> Like this you can rename the alias, without need to change all references in config files, without network restart/reload.

which is my main argument for making the names themselves also more
descriptive ;)

> ----- Mail original -----
> De: "Fabian Grünbichler" <f.gruenbichler at proxmox.com>
> À: "pve-devel" <pve-devel at pve.proxmox.com>
> Envoyé: Jeudi 6 Juin 2019 11:49:30
> Objet: Re: [pve-devel] [PATCH common 1/1] Fix #545: Allow descriptive names for network bridges and bonds
> 
> On Thu, Jun 06, 2019 at 07:54:21AM +0200, Alexandre DERUMIER wrote: 
> > Hi, 
> > 
> > I think the main problem could be the 16 characters limit. 
> 
> we currently allow up to 20 characters in our API/the interface standard 
> option, and even more in the actual interface name format ('pve-iface'). 
> 
> 'ip link' starts failing with 16 character-long interface names already, 
> so effectively its 15 characters (maybe this is a bug in iproute2?). 
> 
> so effectively that means we have 
> - 11 characters left for hypervisor interfaces (vmbr/vnet/bond all have 
> 4 characters) 
> - already hit the limit for container interfaces (veth/fwbr/fwpr/fwln + 
> 9 digits of VMID + i + 1 digit interface index) 
> - already over the limit for VM interfaces (fwbr/fpr/fwln/tap + 9 digits 
> of VMID +i + 2 digits interface index) 
> 
> so IMHO, we already have at least two bugs here: 
> - API/JSONSchema accept interface names that are too long 
> - our naming scheme generates interface names that are too long for VMs 
> with firewall enabled, maximum-length VMID, maximum-length interface 
> index (e.g., VM 100000000 and net10) 
> 
> > ifupdown2 already support "alias" option on interfaces 
> > 
> > auto eth0 
> > iface eth0 
> > alias "my super long string of description" 
> > 
> > 
> > which is doing: 
> > 
> > #ip link set dev eth0 alias "my super long string of description" 
> > 
> > 
> > and you can see description with 
> > #ip link 
> 
> that would be nice to support in addition to more readable names. IMHO, 
> allowing at least alphanumeric (and some other characters like '-_.+'?) 
> should be completely fine, and allows admins to set short, descriptive 
> names like 
> 
> vmbr_internal 
> vmbr_external 
> vmbr_dmz 
> vmbr_uplink 
> vmbr_vpn 
> vmbr_private 
> vmbr_c_xyz (for client/customer xyz) 
> vmbr_test 
> vmbr_dev 
> 
> the interface name is used/displayed/supported across the board, the 
> alias likely is not (yet). e.g. stuff like references in config files, 
> firewall rules, monitoring scripts, metrics/billing, ... 
> 
> > I was planning to use that for vnet too. (keeping vnet(\d+) as interface id, and add a custom description). 
> > 
> > We could add easily a ifup script for ifupdown1. 
> 
> if we allow more characters for vmbr and bond, the same would probably 
> apply to vnet as well ;) 
> 
> > What do you think about this ? 
> 
> I'd like to see both - more descriptive names, and some way to specify a 
> long form description (whether as alias, comment, label, .. - whatever 
> has the best ease of implementation/range of support ratio ;)) 
> 
> > ----- Mail original ----- 
> > De: "Fabian Grünbichler" <f.gruenbichler at proxmox.com> 
> > À: "pve-devel" <pve-devel at pve.proxmox.com> 
> > Envoyé: Mercredi 5 Juin 2019 11:25:47 
> > Objet: Re: [pve-devel] [PATCH common 1/1] Fix #545: Allow descriptive names for network bridges and bonds 
> > 
> > On Wed, May 15, 2019 at 11:49:30AM +0200, Dominic Jäger wrote: 
> > > Allowing more descriptive names for network bridges and bonds makes 
> > > their identification easier. 
> > 
> > there are probably a few more places where this would need to be 
> > adjusted: 
> > 
> > /usr/share/perl5/PVE/FirewallSimulator.pm:395: } elsif ($route_state =~ m/^vmbr\d+$/) { 
> > /usr/share/perl5/PVE/FirewallSimulator.pm:529: } elsif ($from =~ m|^(vmbr\d+)/(\S+)$|) { 
> > /usr/share/perl5/PVE/FirewallSimulator.pm:567: } elsif ($to =~ m|^(vmbr\d+)/(\S+)$|) { 
> > /usr/share/perl5/PVE/QemuServer.pm:5914: bridge => "vmbr$ind", 
> > /usr/share/perl5/PVE/Service/pve_firewall.pm:287: pattern => '(host|outside|vm\d+|ct\d+|vmbr\d+/\S+)', 
> > /usr/share/perl5/PVE/Service/pve_firewall.pm:294: pattern => '(host|outside|vm\d+|ct\d+|vmbr\d+/\S+)', 
> > /usr/share/perl5/PVE/Network.pm:498: PVE::Tools::dir_glob_foreach($dir, '(((eth|bond)\d+|en[^.]+)(\.\d+)?)', sub { 
> > 
> > as well as the GUI part. 
> > 
> > per our discussion, maybe this + labels/comments on interfaces could 
> > get a v2? ;) 
> > 
> > > Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com> 
> > > --- 
> > > src/PVE/INotify.pm | 4 ++-- 
> > > 1 file changed, 2 insertions(+), 2 deletions(-) 
> > > 
> > > diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm 
> > > index b660041..75e5010 100644 
> > > --- a/src/PVE/INotify.pm 
> > > +++ b/src/PVE/INotify.pm 
> > > @@ -1027,7 +1027,7 @@ sub __read_etc_network_interfaces { 
> > > 
> > > foreach my $iface (keys %$ifaces) { 
> > > my $d = $ifaces->{$iface}; 
> > > - if ($iface =~ m/^bond\d+$/) { 
> > > + if ($iface =~ m/^bond\w+$/) { 
> > > if (!$d->{ovs_type}) { 
> > > $d->{type} = 'bond'; 
> > > } elsif ($d->{ovs_type} eq 'OVSBond') { 
> > > @@ -1049,7 +1049,7 @@ sub __read_etc_network_interfaces { 
> > > } else { 
> > > $d->{type} = 'unknown'; 
> > > } 
> > > - } elsif ($iface =~ m/^vmbr\d+$/) { 
> > > + } elsif ($iface =~ m/^vmbr\w+$/) { 
> > > if (!$d->{ovs_type}) { 
> > > $d->{type} = 'bridge'; 
> > > 
> > > -- 
> > > 2.11.0 
> > > 
> > > _______________________________________________ 
> > > pve-devel mailing list 
> > > pve-devel at pve.proxmox.com 
> > > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
> > 
> > _______________________________________________ 
> > pve-devel mailing list 
> > pve-devel at pve.proxmox.com 
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
> > 
> > _______________________________________________ 
> > pve-devel mailing list 
> > pve-devel at pve.proxmox.com 
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
> 
> _______________________________________________ 
> pve-devel mailing list 
> pve-devel at pve.proxmox.com 
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

More information about the pve-devel mailing list