Re: [PATCH 1/2] 8139too: Allow setting MTU larger than 1500

[Ben Hutchings]

On Fri, 2014-11-21 at 14:58 +0100, Alban wrote:
> On Fri, 21 Nov 2014 00:34:34 +0000
> Ben Hutchings <ben@xxxxxxxxxxxxxxx> wrote:
> 
> > On Sat, 2014-11-08 at 12:48 +0100, Alban Bedel wrote:
> > > Replace the default ndo_change_mtu callback with one that allow
> > > setting MTU that the driver can handle.
> > > 
> > > Signed-off-by: Alban Bedel <albeu@xxxxxxx>
> > > ---
> > >  drivers/net/ethernet/realtek/8139too.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/realtek/8139too.c
> > > b/drivers/net/ethernet/realtek/8139too.c index 007b38c..8387de9
> > > 100644 --- a/drivers/net/ethernet/realtek/8139too.c
> > > +++ b/drivers/net/ethernet/realtek/8139too.c
> > > @@ -185,6 +185,9 @@ static int debug = -1;
> > >  /* max supported ethernet frame size -- must be at least
> > > (dev->mtu+14+4).*/ #define MAX_ETH_FRAME_SIZE	1536
> > >  
> > > +/* max supported payload size */
> > > +#define MAX_ETH_DATA_SIZE	(MAX_ETH_FRAME_SIZE - ETH_HLEN -
> > > ETH_FCS_LEN)
> > [...]
> > 
> > Does this maximum still allow for VLAN tags, or should it use
> > VLAN_ETH_HLEN instead of ETH_HLEN?
> 
> That might well be as the VLAN code seems to assume that the physical
> device can handle frames of MTU + VLAN_HLEN bytes. I can fix it, but to
> me it seems like the VLAN code should be fixed to respect the physical
> device MTU.

Drivers that support VLANs have to allow for at least one VLAN tag when
validating the MTU.  This is obviously broken for multiple layers of
VLAN tags, but those are the semantics we're stuck with.

Ben.

-- 
Ben Hutchings
Beware of bugs in the above code;
I have only proved it correct, not tried it. - Donald Knuth

Attachment: signature.asc
Description: This is a digitally signed message part