[pve-devel] [PATCH v3 pve-common 5/5] Inotify : add mtu option

[Alexandre DERUMIER]

> 
> this check is IMHO uneccessary as Debian inherites MTU from parent to child 
> interfaces. That is AFAIK officially documente behavior, so this check makes 
> it impossible to edit an interface that uses mtu definition only on parent 
> devices. 

> > + die "check mtu : missing parent interface\n" if !$parent; 
> > + die "check mtu : missing child interface\n" if !$child; 
> > + 
> > + my $pmtu = $ifaces->{$parent}->{mtu} ? $ifaces->{$parent}->{mtu} : 1500; 
> > + my $cmtu = $ifaces->{$child}->{mtu} ? $ifaces->{$child}->{mtu} : 1500; 

>>Might be enough to just remove the ternary and skip the check if 
>>$ifaces->{$child}->{mtu} is not defined. 

Currently, I think that I'm using check_mtu only on child interfaces (bond,vlan,bridgeport).
@Denis : do you have a sample config where it's not working ?

I'm seeing a case where parentmtu could be 1400, and childmtu not defined.

It should be
-my $cmtu = $ifaces->{$child}->{mtu} ? $ifaces->{$child}->{mtu} : 1500;  
+my $cmtu = $ifaces->{$child}->{mtu} ? $ifaces->{$child}->{mtu} : $pmtu; 

> > + 
> > + die "interface '$parent' - mtu $pmtu is bigger than '$child' - mtu $cmtu\n" 
> > + if $pmtu gt $cmtu; 

>>Also, apparently I missed the 'gt' here, that should be '>', ouch. 

you have already fixed it ;)

https://git.proxmox.com/?p=pve-common.git;a=blobdiff;f=src/PVE/INotify.pm;h=330085066f29f461329494084d7a654c0149e7a3;hp=201c97b1965a29650bc16f86619a2ebf407e49c9;hb=c27ef07ff57a0143ef9200ed8ee53401d46f727a;hpb=cebd1c85f0144628a24c61fa06f3602f78da3c61
----- Mail original -----
De: "Wolfgang Bumiller" <w.bumiller at proxmox.com>
À: "Dennis Busch" <dennis.busch at stacktrace.de>
Cc: "pve-devel" <pve-devel at pve.proxmox.com>
Envoyé: Mardi 28 Août 2018 12:17:37
Objet: Re: [pve-devel] [PATCH v3 pve-common 5/5] Inotify : add mtu option

On Tue, Aug 28, 2018 at 12:02:27PM +0200, Dennis Busch wrote: 
> Hello Alexandre, 
> 
> this check is IMHO uneccessary as Debian inherites MTU from parent to child 
> interfaces. That is AFAIK officially documente behavior, so this check makes 
> it impossible to edit an interface that uses mtu definition only on parent 
> devices. 
> 
> Best regards 
> 
> Dennis 
> 
> stacktrace GmbH 
> Querstraße 3 | 96237 Ebersdorf 
> 
> Amtsgericht Coburg, HRB 5043 
> Geschäftsführer: Dennis Busch | Jürgen Haas 
> 
> Tel: +49 9562 78 48 010 
> Mobil: +49 171 12 62 761 
> E-Mail: dennis.busch at stacktrace.de 
> De-Mail: dennis.busch at gmx.de-mail.de 
> 
> Am 05.07.2018 um 02:56 schrieb Alexandre Derumier: 
> > also check if mtu value is lower than parent interface 
> > 
> > fixme: vxlan interface should be 50bytes lower than outgoing interface 
> > we need to find which interface is used (unicast/multicast/frr) 
> > --- 
> > src/PVE/INotify.pm | 22 ++++++++++++++++++++++ 
> > test/etc_network_interfaces/t.create_network.pl | 10 ++++++++++ 
> > 2 files changed, 32 insertions(+) 
> > 
> > diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm 
> > index f0f3144..5dd08c2 100644 
> > --- a/src/PVE/INotify.pm 
> > +++ b/src/PVE/INotify.pm 
> > @@ -753,6 +753,20 @@ my $extract_ovs_option = sub { 
> > return $v; 
> > }; 
> > +my $check_mtu = sub { 
> > + my ($ifaces, $parent, $child) = @_; 
> > + 
> > + die "check mtu : missing parent interface\n" if !$parent; 
> > + die "check mtu : missing child interface\n" if !$child; 
> > + 
> > + my $pmtu = $ifaces->{$parent}->{mtu} ? $ifaces->{$parent}->{mtu} : 1500; 
> > + my $cmtu = $ifaces->{$child}->{mtu} ? $ifaces->{$child}->{mtu} : 1500; 

Might be enough to just remove the ternary and skip the check if 
$ifaces->{$child}->{mtu} is not defined. 

> > + 
> > + die "interface '$parent' - mtu $pmtu is bigger than '$child' - mtu $cmtu\n" 
> > + if $pmtu gt $cmtu; 

Also, apparently I missed the 'gt' here, that should be '>', ouch. 

> > + 
> > +}; 

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