lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 21 Oct 2016 10:44:41 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Jarod Wilson <jarod@...hat.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org,
        Asbjoern Sloth Toennesen <asbjorn@...jorn.st>,
        R Parameswaran <parameswaran.r7@...il.com>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Subject: Re: [PATCH net] net: remove MTU limits on a few ether_setup callers

On Thu, Oct 20, 2016 at 08:42:46PM -0700, Florian Fainelli wrote:
> Le 20/10/2016 à 20:25, Jarod Wilson a écrit :
> > These few drivers call ether_setup(), but have no ndo_change_mtu, and thus
> > were overlooked for changes to MTU range checking behavior. They
> > previously had no range checks, so for feature-parity, set their min_mtu
> > to 0 and max_mtu to ETH_MAX_MTU (65535), instead of the 68 and 1500
> > inherited from the ether_setup() changes. Fine-tuning can come after we get
> > back to full feature-parity here.
> > 
> > CC: netdev@...r.kernel.org
> > Reported-by: Asbjoern Sloth Toennesen <asbjorn@...jorn.st>
> > CC: Asbjoern Sloth Toennesen <asbjorn@...jorn.st>
> > CC: R Parameswaran <parameswaran.r7@...il.com>
> > Signed-off-by: Jarod Wilson <jarod@...hat.com>
> > ---
> 
> > diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> > index 68714a5..d0c7bce 100644
> > --- a/net/dsa/slave.c
> > +++ b/net/dsa/slave.c
> > @@ -1247,6 +1247,8 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
> >  	slave_dev->priv_flags |= IFF_NO_QUEUE;
> >  	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
> >  	slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
> > +	slave_dev->min_mtu = 0;
> > +	slave_dev->max_mtu = ETH_MAX_MTU;
> >  	SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
> 
> Actually for DSA, a reasonable minimum is probably 68 for the following
> reasons: ETH_ZLEN of 60 bytes + FCS (4 bytes) + 2/4/8 bytes of
> Ethernet switch tag (which is dependent on the tagging protocol
> used).

Humm, isn't the switch tag added by the layer below this? So this
should be - 2/4/8 bytes, so that the assembled frame that actually
hits the conduit interface has an MTU of 64.

> Such an Ethernet interface would submit packets through the conduit
> interface which is connected to an Ethernet switches, and those
> typically discard packets smaller than 64 bytes +/- their custom tag
> length.

I have a purely theoretical observation, i.e. i have not checked the
datasheets. You can also control some of the Marvell switches by
sending it ethernet frames containing commands. Most commands are 4
bytes long. So an Ethernet header + 4 bytes is < 64. I expect the
switch will accept command frames which are padded to stop them being
runts. Also, such frames will be submitted to the conduit interface,
not the slave interface.

    Andrew

Powered by blists - more mailing lists