Re: [PATCH v3 net-next 3/8] net: dsa: configure the MTU for switch ports

[Andrew Lunn <andrew@lunn.ch>]

> -static void dsa_master_set_mtu(struct net_device *dev, struct dsa_port *cpu_dp)
> -{
> -	unsigned int mtu = ETH_DATA_LEN + cpu_dp->tag_ops->overhead;
> -	int err;
> -
> -	rtnl_lock();
> -	if (mtu <= dev->max_mtu) {
> -		err = dev_set_mtu(dev, mtu);
> -		if (err)
> -			netdev_dbg(dev, "Unable to set MTU to include for DSA overheads\n");
> -	}
> -	rtnl_unlock();
> -}
> -
>  static void dsa_master_reset_mtu(struct net_device *dev)
>  {
>  	int err;
> @@ -344,7 +330,14 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
>  {
>  	int ret;
>  
> -	dsa_master_set_mtu(dev,  cpu_dp);
> +	rtnl_lock();
> +	ret = dev_set_mtu(dev, ETH_DATA_LEN + cpu_dp->tag_ops->overhead);
> +	rtnl_unlock();
> +	if (ret) {
> +		netdev_err(dev, "error %d setting MTU to include DSA overhead\n",
> +			   ret);
> +		return ret;
> +	}

I suspect this will break devices. dsa_master_set_mtu() was not fatal
if it failed. I did this deliberately because i suspect there are some
MAC drivers which are unhappy to have the MTU changed, but will still
send and receive frames which are bigger than the MTU. 

So please keep setting the MTU of ETH_DATA_LEN +
cpu_dp->tag_ops->overhead or less as non-fatal. Jumbo frame sizes you
should however check the return code.

> @@ -1465,7 +1556,10 @@ int dsa_slave_create(struct dsa_port *port)
>  	slave_dev->priv_flags |= IFF_NO_QUEUE;
>  	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
>  	slave_dev->min_mtu = 0;
> -	slave_dev->max_mtu = ETH_MAX_MTU;
> +	if (ds->ops->port_max_mtu)
> +		slave_dev->max_mtu = ds->ops->port_max_mtu(ds, port->index);
> +	else
> +		slave_dev->max_mtu = ETH_MAX_MTU;

Does this bring you anything. You have a lot more checks you perform
when performing the actual change. Seems better to keep them all
together.

	Andrew