Re: [PATCH net] net: remove MTU limits on a few ether_setup callers

[Andrew Lunn]

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@xxxxxxxxxxxxxxx
> > Reported-by: Asbjoern Sloth Toennesen <asbjorn@xxxxxxxxxx>
> > CC: Asbjoern Sloth Toennesen <asbjorn@xxxxxxxxxx>
> > CC: R Parameswaran <parameswaran.r7@xxxxxxxxx>
> > Signed-off-by: Jarod Wilson <jarod@xxxxxxxxxx>
> > ---
> 
> > 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