Re: [pve-devel] [PATCH common] support for predictable network interface device names

2016-05-17 Thread Igor Vlasenko 
Thanks!

On Tue, May 17, 2016 at 5:14 PM, Dietmar Maurer  wrote:
> applied
>
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



-- 
С уважением,
Игорь Власенко.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH common] support for predictable network interface device names

2016-05-17 Thread Dietmar Maurer 
applied

___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH common] support for predictable network interface device names

2016-05-17 Thread Igor Vlasenko 
thanks! It looks good.

On Tue, May 17, 2016 at 10:21 AM, Wolfgang Bumiller
 wrote:
> Based on patch from: Igor Vlasenko 
> ---
> Since it was just a diff pasted in a response, here the patch in a
> git-am applyable form.
>
>  src/PVE/INotify.pm | 8 
>  src/PVE/Network.pm | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
> index 74a0fe1..c34659f 100644
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -800,7 +800,7 @@ sub __read_etc_network_interfaces {
>
>  if ($proc_net_dev) {
> while (defined ($line = <$proc_net_dev>)) {
> -   if ($line =~ m/^\s*(eth\d+):.*/) {
> +   if ($line =~ m/^\s*(eth\d+|en[^:.]+):.*/) {
> $ifaces->{$1}->{exists} = 1;
> }
> }
> @@ -973,7 +973,7 @@ sub __read_etc_network_interfaces {
> $ifaces->{$1}->{exists} = 0;
> $d->{exists} = 0;
> }
> -   } elsif ($iface =~ m/^eth\d+$/) {
> +   } elsif ($iface =~ m/^(?:eth\d+|en[^:.]+)$/) {
> if (!$d->{ovs_type}) {
> $d->{type} = 'eth';
> } elsif ($d->{ovs_type} eq 'OVSPort') {
> @@ -1200,7 +1200,7 @@ sub __write_etc_network_interfaces {
> $d->{type} eq 'OVSBond') {
> my $brname = $used_ports->{$iface};
> if (!$brname || !$ifaces->{$brname}) {
> -   if ($iface =~ /^eth/) {
> +   if ($iface =~ /^(?:eth|en)/) {
> $ifaces->{$iface} = { type => 'eth',
>   exists => 1,
>   method => 'manual',
> @@ -1289,7 +1289,7 @@ NETWORKDOC
> my $pri;
> if ($iface eq 'lo') {
> $pri = $if_type_hash->{loopback};
> -   } elsif ($iface =~ m/^eth\d+$/) {
> +   } elsif ($iface =~ m/^(?:eth\d+|en[^:.]+)$/) {
> $pri = $if_type_hash->{eth} + $child;
> } elsif ($iface =~ m/^bond\d+$/) {
> $pri = $if_type_hash->{bond} + $child;
> diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
> index be26132..0a984ad 100644
> --- a/src/PVE/Network.pm
> +++ b/src/PVE/Network.pm
> @@ -440,7 +440,7 @@ sub activate_bridge_vlan {
>
>  my @ifaces = ();
>  my $dir = "/sys/class/net/$bridge/brif";
> -PVE::Tools::dir_glob_foreach($dir, '((eth|bond)\d+(\.\d+)?)', sub {
> +PVE::Tools::dir_glob_foreach($dir, '(((eth|bond)\d+|en[^.]+)(\.\d+)?)', 
> sub {
>  push @ifaces, $_[0];
>  });
>
> --
> 2.1.4
>
>



-- 
С уважением,
Игорь Власенко.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH common] support for predictable network interface device names

2016-05-17 Thread Wolfgang Bumiller 
Based on patch from: Igor Vlasenko 
---
Since it was just a diff pasted in a response, here the patch in a
git-am applyable form.

 src/PVE/INotify.pm | 8 
 src/PVE/Network.pm | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index 74a0fe1..c34659f 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -800,7 +800,7 @@ sub __read_etc_network_interfaces {
 
 if ($proc_net_dev) {
while (defined ($line = <$proc_net_dev>)) {
-   if ($line =~ m/^\s*(eth\d+):.*/) {
+   if ($line =~ m/^\s*(eth\d+|en[^:.]+):.*/) {
$ifaces->{$1}->{exists} = 1;
}
}
@@ -973,7 +973,7 @@ sub __read_etc_network_interfaces {
$ifaces->{$1}->{exists} = 0;
$d->{exists} = 0;
}
-   } elsif ($iface =~ m/^eth\d+$/) {
+   } elsif ($iface =~ m/^(?:eth\d+|en[^:.]+)$/) {
if (!$d->{ovs_type}) {
$d->{type} = 'eth';
} elsif ($d->{ovs_type} eq 'OVSPort') {
@@ -1200,7 +1200,7 @@ sub __write_etc_network_interfaces {
$d->{type} eq 'OVSBond') {
my $brname = $used_ports->{$iface};
if (!$brname || !$ifaces->{$brname}) { 
-   if ($iface =~ /^eth/) {
+   if ($iface =~ /^(?:eth|en)/) {
$ifaces->{$iface} = { type => 'eth',
  exists => 1,
  method => 'manual',
@@ -1289,7 +1289,7 @@ NETWORKDOC
my $pri;
if ($iface eq 'lo') {
$pri = $if_type_hash->{loopback};
-   } elsif ($iface =~ m/^eth\d+$/) {
+   } elsif ($iface =~ m/^(?:eth\d+|en[^:.]+)$/) {
$pri = $if_type_hash->{eth} + $child;
} elsif ($iface =~ m/^bond\d+$/) {
$pri = $if_type_hash->{bond} + $child;
diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index be26132..0a984ad 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -440,7 +440,7 @@ sub activate_bridge_vlan {
 
 my @ifaces = ();
 my $dir = "/sys/class/net/$bridge/brif";
-PVE::Tools::dir_glob_foreach($dir, '((eth|bond)\d+(\.\d+)?)', sub {
+PVE::Tools::dir_glob_foreach($dir, '(((eth|bond)\d+|en[^.]+)(\.\d+)?)', 
sub {
 push @ifaces, $_[0];
 });
 
-- 
2.1.4


___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH common] support for predictable network interface device names

2016-05-12 Thread Igor Vlasenko 
On Thu, May 12, 2016 at 3:05 PM, Wolfgang Bumiller
 wrote:
> On Thu, May 12, 2016 at 02:30:11PM +0300, Igor Vlasenko wrote:
>> On Thu, May 12, 2016 at 2:08 PM, Wolfgang Bumiller
>>  wrote:
>> > Could you review the following modified version of your old patch?
>>
>> seems good.
>> The only question I have does not concern the patch, but the current code
>> that is being patched.
[...]
> This portion reads the interfaces from /proc/net/dev where the names are
> terminated with a colon.

Then everything is ok, looking forward to see it applied.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH common] support for predictable network interface device names

2016-05-12 Thread Wolfgang Bumiller 
On Thu, May 12, 2016 at 02:30:11PM +0300, Igor Vlasenko wrote:
> On Thu, May 12, 2016 at 2:08 PM, Wolfgang Bumiller
>  wrote:
> > On Thu, May 12, 2016 at 11:42:29AM +0300, Igor Vlasenko wrote:
> >> On Wed, May 11, 2016 at 10:56 PM, Igor Vlasenko  
> >> wrote:
> >> > This is an improved version of my previous patch
> >> > [ support for udev-style physical interface names (like enp3s0),
> >> >  http://pve.proxmox.com/pipermail/pve-devel/2016-May/020958.html ]
> >> > thanks to Wolfgang.
> >>
> >> Yesterday I finished coding just before going to sleep, so I was a bit
> >> muddleheaded and
> >> left a few mistakes :( Here I fixed them and added a test case to verify.
> >
> > Ah now you added all the ones described in those links. But AFAIK wlan
> > interfaces cannot be added to bridges.
> > This seems a little overkill (and there are a few style concerns in the
> > code).
> 
> I was thinking that repeating the same (possibly incomplete) pattern
> again and again is the code clone bug pattern.
> To have one regex in one place, though it looks a bit overkill, will help
> to maintain the code in the future.
> 
> > Maybe it's best to stick to the devices we know users are currently
> > dealing with and just stick to including 'en'-prefixed devices.
> > Iow. a variant of your old patch with just 'en' instead of 'enp' and the
> > veth/tap hunks removed.
> 
> I have no objections.
> 
> > Could you review the following modified version of your old patch?
> 
> seems good.
> The only question I have does not concern the patch, but the current code
> that is being patched.
> 
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -800,7 +800,7 @@ sub __read_etc_network_interfaces {
> 
>  if ($proc_net_dev) {
> while (defined ($line = <$proc_net_dev>)) {
> -   if ($line =~ m/^\s*(eth\d+):.*/) {
> +   if ($line =~ m/^\s*(eth\d+|en[^:.]+):.*/) {
> $ifaces->{$1}->{exists} = 1;
> }
> }
> 
> in the line 803:
> -   if ($line =~ m/^\s*(eth\d+):.*/) {
> Should it be a colon (`:') there? as in eth0:something?
> Should not it be a point (`.') ? as in eth0.1?
> If it is ok, then the patch is ok, else patch also should use a point
> instead of the colon.

This portion reads the interfaces from /proc/net/dev where the names are
terminated with a colon.

___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH common] support for predictable network interface device names

2016-05-12 Thread Igor Vlasenko 
On Thu, May 12, 2016 at 2:08 PM, Wolfgang Bumiller
 wrote:
> On Thu, May 12, 2016 at 11:42:29AM +0300, Igor Vlasenko wrote:
>> On Wed, May 11, 2016 at 10:56 PM, Igor Vlasenko  wrote:
>> > This is an improved version of my previous patch
>> > [ support for udev-style physical interface names (like enp3s0),
>> >  http://pve.proxmox.com/pipermail/pve-devel/2016-May/020958.html ]
>> > thanks to Wolfgang.
>>
>> Yesterday I finished coding just before going to sleep, so I was a bit
>> muddleheaded and
>> left a few mistakes :( Here I fixed them and added a test case to verify.
>
> Ah now you added all the ones described in those links. But AFAIK wlan
> interfaces cannot be added to bridges.
> This seems a little overkill (and there are a few style concerns in the
> code).

I was thinking that repeating the same (possibly incomplete) pattern
again and again is the code clone bug pattern.
To have one regex in one place, though it looks a bit overkill, will help
to maintain the code in the future.

> Maybe it's best to stick to the devices we know users are currently
> dealing with and just stick to including 'en'-prefixed devices.
> Iow. a variant of your old patch with just 'en' instead of 'enp' and the
> veth/tap hunks removed.

I have no objections.

> Could you review the following modified version of your old patch?

seems good.
The only question I have does not concern the patch, but the current code
that is being patched.

--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -800,7 +800,7 @@ sub __read_etc_network_interfaces {

 if ($proc_net_dev) {
while (defined ($line = <$proc_net_dev>)) {
-   if ($line =~ m/^\s*(eth\d+):.*/) {
+   if ($line =~ m/^\s*(eth\d+|en[^:.]+):.*/) {
$ifaces->{$1}->{exists} = 1;
}
}

in the line 803:
-   if ($line =~ m/^\s*(eth\d+):.*/) {
Should it be a colon (`:') there? as in eth0:something?
Should not it be a point (`.') ? as in eth0.1?
If it is ok, then the patch is ok, else patch also should use a point
instead of the colon.


> ---
>  src/PVE/INotify.pm | 8 
>  src/PVE/Network.pm | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
> index 74a0fe1..c34659f 100644
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -800,7 +800,7 @@ sub __read_etc_network_interfaces {
>
>  if ($proc_net_dev) {
> while (defined ($line = <$proc_net_dev>)) {
> -   if ($line =~ m/^\s*(eth\d+):.*/) {
> +   if ($line =~ m/^\s*(eth\d+|en[^:.]+):.*/) {
> $ifaces->{$1}->{exists} = 1;
> }
> }
> @@ -973,7 +973,7 @@ sub __read_etc_network_interfaces {
> $ifaces->{$1}->{exists} = 0;
> $d->{exists} = 0;
> }
> -   } elsif ($iface =~ m/^eth\d+$/) {
> +   } elsif ($iface =~ m/^(?:eth\d+|en[^:.]+)$/) {
> if (!$d->{ovs_type}) {
> $d->{type} = 'eth';
> } elsif ($d->{ovs_type} eq 'OVSPort') {
> @@ -1200,7 +1200,7 @@ sub __write_etc_network_interfaces {
> $d->{type} eq 'OVSBond') {
> my $brname = $used_ports->{$iface};
> if (!$brname || !$ifaces->{$brname}) {
> -   if ($iface =~ /^eth/) {
> +   if ($iface =~ /^(?:eth|en)/) {
> $ifaces->{$iface} = { type => 'eth',
>   exists => 1,
>   method => 'manual',
> @@ -1289,7 +1289,7 @@ NETWORKDOC
> my $pri;
> if ($iface eq 'lo') {
> $pri = $if_type_hash->{loopback};
> -   } elsif ($iface =~ m/^eth\d+$/) {
> +   } elsif ($iface =~ m/^(?:eth\d+|en[^:.]+)$/) {
> $pri = $if_type_hash->{eth} + $child;
> } elsif ($iface =~ m/^bond\d+$/) {
> $pri = $if_type_hash->{bond} + $child;
> diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
> index be26132..0a984ad 100644
> --- a/src/PVE/Network.pm
> +++ b/src/PVE/Network.pm
> @@ -440,7 +440,7 @@ sub activate_bridge_vlan {
>
>  my @ifaces = ();
>  my $dir = "/sys/class/net/$bridge/brif";
> -PVE::Tools::dir_glob_foreach($dir, '((eth|bond)\d+(\.\d+)?)', sub {
> +PVE::Tools::dir_glob_foreach($dir, '(((eth|bond)\d+|en[^.]+)(\.\d+)?)', 
> sub {
>  push @ifaces, $_[0];
>  });
>
> --
> 2.1.4
>
>



-- 
С уважением,
Игорь Власенко.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH common] support for predictable network interface device names

2016-05-12 Thread Wolfgang Bumiller 
On Thu, May 12, 2016 at 11:42:29AM +0300, Igor Vlasenko wrote:
> On Wed, May 11, 2016 at 10:56 PM, Igor Vlasenko  wrote:
> > This is an improved version of my previous patch
> > [ support for udev-style physical interface names (like enp3s0),
> >  http://pve.proxmox.com/pipermail/pve-devel/2016-May/020958.html ]
> > thanks to Wolfgang.
> 
> Yesterday I finished coding just before going to sleep, so I was a bit
> muddleheaded and
> left a few mistakes :( Here I fixed them and added a test case to verify.

Ah now you added all the ones described in those links. But AFAIK wlan
interfaces cannot be added to bridges.
This seems a little overkill (and there are a few style concerns in the
code).

Maybe it's best to stick to the devices we know users are currently
dealing with and just stick to including 'en'-prefixed devices.
Iow. a variant of your old patch with just 'en' instead of 'enp' and the
veth/tap hunks removed.

Could you review the following modified version of your old patch?

---
 src/PVE/INotify.pm | 8 
 src/PVE/Network.pm | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index 74a0fe1..c34659f 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -800,7 +800,7 @@ sub __read_etc_network_interfaces {
 
 if ($proc_net_dev) {
while (defined ($line = <$proc_net_dev>)) {
-   if ($line =~ m/^\s*(eth\d+):.*/) {
+   if ($line =~ m/^\s*(eth\d+|en[^:.]+):.*/) {
$ifaces->{$1}->{exists} = 1;
}
}
@@ -973,7 +973,7 @@ sub __read_etc_network_interfaces {
$ifaces->{$1}->{exists} = 0;
$d->{exists} = 0;
}
-   } elsif ($iface =~ m/^eth\d+$/) {
+   } elsif ($iface =~ m/^(?:eth\d+|en[^:.]+)$/) {
if (!$d->{ovs_type}) {
$d->{type} = 'eth';
} elsif ($d->{ovs_type} eq 'OVSPort') {
@@ -1200,7 +1200,7 @@ sub __write_etc_network_interfaces {
$d->{type} eq 'OVSBond') {
my $brname = $used_ports->{$iface};
if (!$brname || !$ifaces->{$brname}) { 
-   if ($iface =~ /^eth/) {
+   if ($iface =~ /^(?:eth|en)/) {
$ifaces->{$iface} = { type => 'eth',
  exists => 1,
  method => 'manual',
@@ -1289,7 +1289,7 @@ NETWORKDOC
my $pri;
if ($iface eq 'lo') {
$pri = $if_type_hash->{loopback};
-   } elsif ($iface =~ m/^eth\d+$/) {
+   } elsif ($iface =~ m/^(?:eth\d+|en[^:.]+)$/) {
$pri = $if_type_hash->{eth} + $child;
} elsif ($iface =~ m/^bond\d+$/) {
$pri = $if_type_hash->{bond} + $child;
diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index be26132..0a984ad 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -440,7 +440,7 @@ sub activate_bridge_vlan {
 
 my @ifaces = ();
 my $dir = "/sys/class/net/$bridge/brif";
-PVE::Tools::dir_glob_foreach($dir, '((eth|bond)\d+(\.\d+)?)', sub {
+PVE::Tools::dir_glob_foreach($dir, '(((eth|bond)\d+|en[^.]+)(\.\d+)?)', 
sub {
 push @ifaces, $_[0];
 });
 
-- 
2.1.4


___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH common] support for predictable network interface device names

2016-05-12 Thread Igor Vlasenko 
On Wed, May 11, 2016 at 10:56 PM, Igor Vlasenko  wrote:
> This is an improved version of my previous patch
> [ support for udev-style physical interface names (like enp3s0),
>  http://pve.proxmox.com/pipermail/pve-devel/2016-May/020958.html ]
> thanks to Wolfgang.

Yesterday I finished coding just before going to sleep, so I was a bit
muddleheaded and
left a few mistakes :( Here I fixed them and added a test case to verify.


Signed-off-by: Igor Vlasenko 
---
 src/Makefile   |  1 +
 src/PVE/INotify.pm | 10 +++--
 src/PVE/Network.pm |  7 ++-
 src/PVE/NetworkInterfaces.pm   | 50 ++
 src/PVE/Tools.pm   | 11 +
 .../t.is_physical_interface.pl | 50 ++
 6 files changed, 123 insertions(+), 6 deletions(-)
 create mode 100644 src/PVE/NetworkInterfaces.pm
 create mode 100644 test/etc_network_interfaces/t.is_physical_interface.pl

diff --git a/src/Makefile b/src/Makefile
index a07e2e4..02265e0 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -10,6 +10,7 @@ LIB_SOURCES=\
 Daemon.pm\
 SectionConfig.pm\
 Network.pm\
+NetworkInterfaces.pm\
 ProcFSTools.pm\
 CLIHandler.pm\
 RESTHandler.pm\
diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index 74a0fe1..ce83c16 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -14,6 +14,7 @@ use Fcntl qw(:DEFAULT :flock);
 use PVE::SafeSyslog;
 use PVE::Exception qw(raise_param_exc);
 use PVE::Tools;
+use PVE::NetworkInterfaces;
 use Storable qw(dclone);
 use Linux::Inotify2;
 use base 'Exporter';
@@ -800,7 +801,8 @@ sub __read_etc_network_interfaces {

 if ($proc_net_dev) {
 while (defined ($line = <$proc_net_dev>)) {
-if ($line =~ m/^\s*(eth\d+):.*/) {
+if ($line =~ m/^\s*([^:]+):.*/
+and PVE::NetworkInterfaces::is_physical_interface($1)) {
 $ifaces->{$1}->{exists} = 1;
 }
 }
@@ -973,7 +975,7 @@ sub __read_etc_network_interfaces {
 $ifaces->{$1}->{exists} = 0;
 $d->{exists} = 0;
 }
-} elsif ($iface =~ m/^eth\d+$/) {
+} elsif (PVE::NetworkInterfaces::is_physical_interface($iface)) {
 if (!$d->{ovs_type}) {
 $d->{type} = 'eth';
 } elsif ($d->{ovs_type} eq 'OVSPort') {
@@ -1200,7 +1202,7 @@ sub __write_etc_network_interfaces {
 $d->{type} eq 'OVSBond') {
 my $brname = $used_ports->{$iface};
 if (!$brname || !$ifaces->{$brname}) {
-if ($iface =~ /^eth/) {
+if (PVE::NetworkInterfaces::is_physical_interface($iface)) {
 $ifaces->{$iface} = { type => 'eth',
   exists => 1,
   method => 'manual',
@@ -1289,7 +1291,7 @@ NETWORKDOC
 my $pri;
 if ($iface eq 'lo') {
 $pri = $if_type_hash->{loopback};
-} elsif ($iface =~ m/^eth\d+$/) {
+} elsif (PVE::NetworkInterfaces::is_physical_interface($iface)) {
 $pri = $if_type_hash->{eth} + $child;
 } elsif ($iface =~ m/^bond\d+$/) {
 $pri = $if_type_hash->{bond} + $child;
diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index be26132..1d15990 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use PVE::Tools qw(run_command);
 use PVE::ProcFSTools;
+use PVE::NetworkInterfaces;
 use PVE::INotify;
 use File::Basename;
 use IO::Socket::IP;
@@ -440,8 +441,10 @@ sub activate_bridge_vlan {

 my @ifaces = ();
 my $dir = "/sys/class/net/$bridge/brif";
-PVE::Tools::dir_glob_foreach($dir, '((eth|bond)\d+(\.\d+)?)', sub {
-push @ifaces, $_[0];
+PVE::Tools::dir_lambda_foreach($dir, sub {
+push @ifaces, $_[0] if $_[0] =~ /^(?:bond|eth)\d+(\.\d+)?/
+or PVE::NetworkInterfaces::is_physical_interface($_[0]);
+}
 });

 die "no physical interface on bridge '$bridge'\n" if scalar(@ifaces) == 0;
diff --git a/src/PVE/NetworkInterfaces.pm b/src/PVE/NetworkInterfaces.pm
new file mode 100644
index 000..80daf31
--- /dev/null
+++ b/src/PVE/NetworkInterfaces.pm
@@ -0,0 +1,50 @@
+package PVE::NetworkInterfaces;
+
+use strict;
+use warnings;
+
+# alternatively, on the linux kernel we can readlink /sys/class/net/$iface
+# and check whether it points to ../../devices/virtual/...
+
+# physical network interface pattern matching.
+# matching for predictable network interface device names is based on
+# https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c
+# 
http://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames
+sub is_physical_interface {
+my ($iface) = @_;
+return $iface =~
+/^(?:
+# legacy interface names
+eth\d+
+
+| # predictable network interface device names
+
+# two character prefixes:
+# en — Ethernet
+#

[pve-devel] [PATCH common] support for predictable network interface device names

2016-05-11 Thread Igor Vlasenko 
This is an improved version of my previous patch
[ support for udev-style physical interface names (like enp3s0),
 http://pve.proxmox.com/pipermail/pve-devel/2016-May/020958.html ]
thanks to Wolfgang.
The resulting regex is large, so it is wrapped as a common subroutine.

Signed-off-by: Igor Vlasenko 
---
 src/Makefile|  1 +
 src/PVE/INotify.pm  | 10 +
 src/PVE/Network.pm  |  7 +--
 src/PVE/NetworkInterface.pm | 49 +
 src/PVE/Tools.pm| 11 ++
 5 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 src/PVE/NetworkInterface.pm

diff --git a/src/Makefile b/src/Makefile
index a07e2e4..02265e0 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -10,6 +10,7 @@ LIB_SOURCES=\
 Daemon.pm\
 SectionConfig.pm\
 Network.pm\
+NetworkInterfaces.pm\
 ProcFSTools.pm\
 CLIHandler.pm\
 RESTHandler.pm\
diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index 74a0fe1..ce83c16 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -14,6 +14,7 @@ use Fcntl qw(:DEFAULT :flock);
 use PVE::SafeSyslog;
 use PVE::Exception qw(raise_param_exc);
 use PVE::Tools;
+use PVE::NetworkInterfaces;
 use Storable qw(dclone);
 use Linux::Inotify2;
 use base 'Exporter';
@@ -800,7 +801,8 @@ sub __read_etc_network_interfaces {

 if ($proc_net_dev) {
 while (defined ($line = <$proc_net_dev>)) {
-if ($line =~ m/^\s*(eth\d+):.*/) {
+if ($line =~ m/^\s*([^:]+):.*/
+and PVE::NetworkInterfaces::is_physical_interface($1)) {
 $ifaces->{$1}->{exists} = 1;
 }
 }
@@ -973,7 +975,7 @@ sub __read_etc_network_interfaces {
 $ifaces->{$1}->{exists} = 0;
 $d->{exists} = 0;
 }
-} elsif ($iface =~ m/^eth\d+$/) {
+} elsif (PVE::NetworkInterfaces::is_physical_interface($iface)) {
 if (!$d->{ovs_type}) {
 $d->{type} = 'eth';
 } elsif ($d->{ovs_type} eq 'OVSPort') {
@@ -1200,7 +1202,7 @@ sub __write_etc_network_interfaces {
 $d->{type} eq 'OVSBond') {
 my $brname = $used_ports->{$iface};
 if (!$brname || !$ifaces->{$brname}) {
-if ($iface =~ /^eth/) {
+if (PVE::NetworkInterfaces::is_physical_interface($iface)) {
 $ifaces->{$iface} = { type => 'eth',
   exists => 1,
   method => 'manual',
@@ -1289,7 +1291,7 @@ NETWORKDOC
 my $pri;
 if ($iface eq 'lo') {
 $pri = $if_type_hash->{loopback};
-} elsif ($iface =~ m/^eth\d+$/) {
+} elsif (PVE::NetworkInterfaces::is_physical_interface($iface)) {
 $pri = $if_type_hash->{eth} + $child;
 } elsif ($iface =~ m/^bond\d+$/) {
 $pri = $if_type_hash->{bond} + $child;
diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index be26132..1d15990 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use PVE::Tools qw(run_command);
 use PVE::ProcFSTools;
+use PVE::NetworkInterfaces;
 use PVE::INotify;
 use File::Basename;
 use IO::Socket::IP;
@@ -440,8 +441,10 @@ sub activate_bridge_vlan {

 my @ifaces = ();
 my $dir = "/sys/class/net/$bridge/brif";
-PVE::Tools::dir_glob_foreach($dir, '((eth|bond)\d+(\.\d+)?)', sub {
-push @ifaces, $_[0];
+PVE::Tools::dir_lambda_foreach($dir, sub {
+push @ifaces, $_[0] if $_[0] =~ /^(?:bond|eth)\d+(\.\d+)?/
+or PVE::NetworkInterfaces::is_physical_interface($_[0]);
+}
 });

 die "no physical interface on bridge '$bridge'\n" if scalar(@ifaces) == 0;
diff --git a/src/PVE/NetworkInterface.pm b/src/PVE/NetworkInterface.pm
new file mode 100644
index 000..1c7dcc5
--- /dev/null
+++ b/src/PVE/NetworkInterface.pm
@@ -0,0 +1,49 @@
+package PVE::NetworkInterfaces;
+
+use strict;
+use warnings;
+
+# alternatively, on the linux kernel we can readlink /sys/class/net/$iface
+# and check whether it points to ../../devices/virtual/...
+
+# physical network interface pattern matching.
+# matching for predictable network interface device names is based on
+# https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c
+# 
http://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames
+sub is_physical_interface {
+my ($iface) = @_;
+return $iface =~
+/^(?:
+# legacy interface names
+eth\d+
+
+| # predictable network interface device names
+
+# two character prefixes:
+# en — Ethernet
+# sl — serial line IP (slip)
+# wl — wlan
+# ww — wwan
+(?:en|sl|wl|ww)
+
+# type of names:
+(?:
+# b — BCMA bus core number
+  b\d+
+# c — CCW bus group name, without leading zeros [s390]
+| c\d+
+# o[d] — on-board device index number
+| o\d+(?:d\d+)?|
+# s[f][d] — hotplug slot index number
+| s\d+(?:f\d+)?(?:d\d+)?|
+# x — MAC address
+| x[\da-f]{12}|
+