[pve-devel] [PATCH pve-common 1/2] Inotify: add support for 802.1ad stacked vlans

[Alexandre DERUMIER]

>>While 802.1ad seems to be "specified" to be limited to 2 tags, it may 
>>still be nice to just condense this into a single branch: 
>>
>>if ($d->{type} eq 'vlan' && $iface =~ m/^(\S+(?:\.\d+)*)\.\d+$/) { 
>>my $parent_name = $1; 
>>my $parent = $ifaces->{$parent_name}; 
>>
>>And then add a 'vlan' type branch before the `not eth/bridge/bond` 
>>branch in the old code? 
>>(And then die if $parent_name contains 2 dots (=~ /\..*\./)). 

Ok thanks, I'll rework it.


>>Btw. is this a restriction of ifupdown2? Or do we just want to do this 
>>for safety? As from "technical" point of view nothing prevents me from 
>>tripple-tagging. The kernel also happily allows me to add a range of 
>>multiple 802.1Q tags without even using 802.1ad, or mix them. 
>>eg.: 
>># ip link add br0.5 link br0 type vlan id 5 protocol 802.1Q 
>># ip link add br0.5.6 link br0.5 type vlan id 6 protocol 802.1ad 
>># ip link add br0.5.6.7 link br0.5.6 type vlan id 7 protocol 802.1Q 
>># ip link add br0.5.6.7.8 link br0.5.6.7 type vlan id 8 protocol 802.1ad 
>>
>>tcpdump shows the expected data - I have no idea what it would do to 
>>the usual switches out there in real networks though ;-) 

yes, it was more by safety to avoid user misconfiguration of something not working.

It's also possible to stack 802.1Q on 802.1Q for example,
but I'm unaware of any real switch implementation.
AFAIK, only 1 802.1Q on top of 802.1ad seem to be possible on cisco,juniper,arista,....



----- Mail original -----
De: "Wolfgang Bumiller" <w.bumiller at proxmox.com>
À: "aderumier" <aderumier at odiso.com>
Cc: "pve-devel" <pve-devel at pve.proxmox.com>
Envoyé: Mardi 25 Septembre 2018 14:55:17
Objet: Re: [pve-devel] [PATCH pve-common 1/2] Inotify: add support for 802.1ad stacked vlans

On Mon, Sep 24, 2018 at 09:52:46AM +0200, Alexandre Derumier wrote: 
> --- 
> src/PVE/INotify.pm | 26 ++++++++++++++++++++++++- 
> test/etc_network_interfaces/t.create_network.pl | 14 +++++++++++++ 
> 2 files changed, 39 insertions(+), 1 deletion(-) 
> 
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm 
> index f837596..de61d79 100644 
> --- a/src/PVE/INotify.pm 
> +++ b/src/PVE/INotify.pm 
> @@ -1432,7 +1433,25 @@ sub __write_etc_network_interfaces { 
> # check vlan 
> foreach my $iface (keys %$ifaces) { 
> my $d = $ifaces->{$iface}; 
> - if ($d->{type} eq 'vlan' && $iface =~ m/^(\S+)\.\d+$/) { 
> + if ($d->{type} eq 'vlan' && $iface =~ m/^(\S+\.\d+)\.\d+$/) { 

While 802.1ad seems to be "specified" to be limited to 2 tags, it may 
still be nice to just condense this into a single branch: 

if ($d->{type} eq 'vlan' && $iface =~ m/^(\S+(?:\.\d+)*)\.\d+$/) { 
my $parent_name = $1; 
my $parent = $ifaces->{$parent_name}; 

And then add a 'vlan' type branch before the `not eth/bridge/bond` 
branch in the old code? 
(And then die if $parent_name contains 2 dots (=~ /\..*\./)). 

> + my $p = $1; 
> + my $n = $ifaces->{$p}; 
> + 
> + die "vlan '$iface' - unable to find parent '$p'\n" 
> + if !$n; 
> + 
> + die "stacked vlan '$iface' - parent '$p' is not a vlan interface " 
> + if $n->{type} ne 'vlan'; 
> + 
> + die "stacked vlan '$iface' - parent '$p' vlan-protocol is not 802.1ad" 
> + if !$n->{'vlan-protocol'} || $n->{'vlan-protocol'} ne '802.1ad'; 
> + 
> + die "stacked vlan '$iface' - vlan-protocol can't be 802.1ad" 
> + if $d->{'vlan-protocol'} && $d->{'vlan-protocol'} eq '802.1ad'; 

Btw. is this a restriction of ifupdown2? Or do we just want to do this 
for safety? As from "technical" point of view nothing prevents me from 
tripple-tagging. The kernel also happily allows me to add a range of 
multiple 802.1Q tags without even using 802.1ad, or mix them. 
eg.: 
# ip link add br0.5 link br0 type vlan id 5 protocol 802.1Q 
# ip link add br0.5.6 link br0.5 type vlan id 6 protocol 802.1ad 
# ip link add br0.5.6.7 link br0.5.6 type vlan id 7 protocol 802.1Q 
# ip link add br0.5.6.7.8 link br0.5.6.7 type vlan id 8 protocol 802.1ad 

tcpdump shows the expected data - I have no idea what it would do to 
the usual switches out there in real networks though ;-) 

> + 
> + &$check_mtu($ifaces, $p, $iface); 
> + 
> + } elsif ($d->{type} eq 'vlan' && $iface =~ m/^(\S+)\.\d+$/) { 
> my $p = $1; 
> my $n = $ifaces->{$p}; 
>