netdev - Re: [PATCH net-next 2/2] net: dsa: Set the master device's MTU to account for DSA overheads

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 linux-hardening linux-cve-announce 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]

Message-ID: <20181206204846.GQ18674@lunn.ch>
Date:   Thu, 6 Dec 2018 21:48:46 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Vivien Didelot <vivien.didelot@...il.com>
Subject: Re: [PATCH net-next 2/2] net: dsa: Set the master device's MTU to
 account for DSA overheads

On Thu, Dec 06, 2018 at 12:21:31PM -0800, Florian Fainelli wrote:
> On 12/6/18 2:36 AM, Andrew Lunn wrote:
> > DSA tagging of frames sent over the master interface to the switch
> > increases the size of the frame. Such frames can then be bigger than
> > the normal MTU of the master interface, and it may drop them. Use the
> > overhead information from the tagger to set the MTU of the master
> > device to include this overhead.
> > 
> > Signed-off-by: Andrew Lunn <andrew@...n.ch>
> > ---
> >  net/dsa/master.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/net/dsa/master.c b/net/dsa/master.c
> > index c90ee3227dea..42f525bc68e2 100644
> > --- a/net/dsa/master.c
> > +++ b/net/dsa/master.c
> > @@ -158,8 +158,24 @@ static void dsa_master_ethtool_teardown(struct net_device *dev)
> >  	cpu_dp->orig_ethtool_ops = NULL;
> >  }
> >  
> > +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");
> > +	}
> 
> Would it make sense to warn the user that there might be
> transmit/receive issues with the DSA tagging protocol if either
> dev_set_mtu() fails or mtu > dev->max_mtu?

I thought about that. But we have setups which work today with the
standard MTU. The master might not implement the set_mtu op, or might
impose the standard MTU, but be quite happy to deal with our DSA
packets. So i wanted to make this a hint to the master device, not a
strong requirement.

> Not that I think it matters too much to people because unbinding the
> switch driver and expecting the CPU port to continue operating is
> wishful thinking, but we should probably unwind that operation in
> dsa_master_teardown(), right?

That would make sense.

David has already accepted the patchset, so i will add a followup
patch.

	Andrew