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

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

[Alban Bedel]

Replace the default ndo_change_mtu callback with one that allow
setting MTU that the driver can handle.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 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 da5972e..9d5d6d1 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)
+
 /* Size of the Tx bounce buffers -- must be at least (dev->mtu+14+4). */
 #define TX_BUF_SIZE	MAX_ETH_FRAME_SIZE
 #define TX_BUF_TOT_LEN	(TX_BUF_SIZE * NUM_TX_DESC)
@@ -920,11 +923,19 @@ static int rtl8139_set_features(struct net_device *dev, netdev_features_t featur
 	return 0;
 }
 
+static int rtl8139_change_mtu(struct net_device *dev, int new_mtu)
+{
+	if (new_mtu < 68 || new_mtu > MAX_ETH_DATA_SIZE)
+		return -EINVAL;
+	dev->mtu = new_mtu;
+	return 0;
+}
+
 static const struct net_device_ops rtl8139_netdev_ops = {
 	.ndo_open		= rtl8139_open,
 	.ndo_stop		= rtl8139_close,
 	.ndo_get_stats64	= rtl8139_get_stats64,
-	.ndo_change_mtu		= eth_change_mtu,
+	.ndo_change_mtu		= rtl8139_change_mtu,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_mac_address 	= rtl8139_set_mac_address,
 	.ndo_start_xmit		= rtl8139_start_xmit,
-- 
1.8.4.2

[PATCH 2/2] 8139too: Allow using the largest possible MTU

[Alban Bedel]

This driver allows MTU up to 1518 bytes which is not enought to run
batman-adv. Simply raise the maximum packet size up to the maximum
allowed by the transmit descriptor, 1792 bytes, giving a maximum MTU
of 1774 bytes.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 drivers/net/ethernet/realtek/8139too.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 9d5d6d1..7be187a 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -183,7 +183,7 @@ static int debug = -1;
 #define NUM_TX_DESC	4
 
 /* max supported ethernet frame size -- must be at least (dev->mtu+14+4).*/
-#define MAX_ETH_FRAME_SIZE	1536
+#define MAX_ETH_FRAME_SIZE	1792
 
 /* max supported payload size */
 #define MAX_ETH_DATA_SIZE	(MAX_ETH_FRAME_SIZE - ETH_HLEN - ETH_FCS_LEN)
-- 
1.8.4.2

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

[David Miller]

Because we are in the middle of the merge window, we are only
accepting bug fixes into the 'net' GIT tree, the 'net-next' tree is
closed.

Please resubmit this feature patch when the 'net-next' is openned
again.

Thank you.

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

On Fri, 2014-11-21 at 19:57 +0100, Alban wrote:
> On Fri, 21 Nov 2014 18:51:51 +0000
> Ben Hutchings <ben@decadent.org.uk> wrote:
> 
> > On Fri, 2014-11-21 at 14:58 +0100, Alban wrote:
> > > On Fri, 21 Nov 2014 00:34:34 +0000
> > > Ben Hutchings <ben@decadent.org.uk> 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@free.fr>
> > > > > ---
> > > > >  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.
> 
> Ok, I see. Should a I send a fix patch or redo a new version of this
> patch?

As your previous patches have already been applied, you'll need to send
a fix patch (if the answer to my question was no).

Ben.

-- 
Ben Hutchings
Never put off till tomorrow what you can avoid all together.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

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

[Alban]

On Fri, 21 Nov 2014 18:51:51 +0000
Ben Hutchings <ben@decadent.org.uk> wrote:

> On Fri, 2014-11-21 at 14:58 +0100, Alban wrote:
> > On Fri, 21 Nov 2014 00:34:34 +0000
> > Ben Hutchings <ben@decadent.org.uk> 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@free.fr>
> > > > ---
> > > >  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.

Ok, I see. Should a I send a fix patch or redo a new version of this
patch?

Alban

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

On Fri, 2014-11-21 at 14:58 +0100, Alban wrote:
> On Fri, 21 Nov 2014 00:34:34 +0000
> Ben Hutchings <ben@decadent.org.uk> 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@free.fr>
> > > ---
> > >  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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

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

[Alban]

On Fri, 21 Nov 2014 00:34:34 +0000
Ben Hutchings <ben@decadent.org.uk> 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@free.fr>
> > ---
> >  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.

Alban

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1079 bytes --]

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@free.fr>
> ---
>  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?

Ben.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

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

[David Miller]

From: Alban Bedel <albeu@free.fr>
Date: Sat,  8 Nov 2014 12:48:35 +0100

> Replace the default ndo_change_mtu callback with one that allow
> setting MTU that the driver can handle.
> 
> Signed-off-by: Alban Bedel <albeu@free.fr>

Applied to net-next

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

[Alban Bedel]

Replace the default ndo_change_mtu callback with one that allow
setting MTU that the driver can handle.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 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)
+
 /* Size of the Tx bounce buffers -- must be at least (dev->mtu+14+4). */
 #define TX_BUF_SIZE	MAX_ETH_FRAME_SIZE
 #define TX_BUF_TOT_LEN	(TX_BUF_SIZE * NUM_TX_DESC)
@@ -920,11 +923,19 @@ static int rtl8139_set_features(struct net_device *dev, netdev_features_t featur
 	return 0;
 }
 
+static int rtl8139_change_mtu(struct net_device *dev, int new_mtu)
+{
+	if (new_mtu < 68 || new_mtu > MAX_ETH_DATA_SIZE)
+		return -EINVAL;
+	dev->mtu = new_mtu;
+	return 0;
+}
+
 static const struct net_device_ops rtl8139_netdev_ops = {
 	.ndo_open		= rtl8139_open,
 	.ndo_stop		= rtl8139_close,
 	.ndo_get_stats64	= rtl8139_get_stats64,
-	.ndo_change_mtu		= eth_change_mtu,
+	.ndo_change_mtu		= rtl8139_change_mtu,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_mac_address 	= rtl8139_set_mac_address,
 	.ndo_start_xmit		= rtl8139_start_xmit,
-- 
2.0.0