2014-11-08 11:48:57

by Alban

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

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

Signed-off-by: Alban Bedel <[email protected]>
---
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

2014-11-08 11:49:05

by Alban

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

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 <[email protected]>
---
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 8387de9..e157541 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)
--
2.0.0

2014-11-10 20:30:25

by David Miller

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

From: Alban Bedel <[email protected]>
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 <[email protected]>

Applied to net-next

2014-11-10 20:30:33

by David Miller

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

From: Alban Bedel <[email protected]>
Date: Sat, 8 Nov 2014 12:48:36 +0100

> 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 <[email protected]>

Also applied to net-next, thanks.

2014-11-21 00:34:50

by Ben Hutchings

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

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 <[email protected]>
> ---
> 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

Attachments:

2014-11-21 13:59:11

by Alban

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

On Fri, 21 Nov 2014 00:34:34 +0000
Ben Hutchings <[email protected]> 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 <[email protected]>
> > ---
> > 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

2014-11-21 18:52:01

by Ben Hutchings

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

On Fri, 2014-11-21 at 14:58 +0100, Alban wrote:
> On Fri, 21 Nov 2014 00:34:34 +0000
> Ben Hutchings <[email protected]> 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 <[email protected]>
> > > ---
> > > 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

Attachments:

2014-11-21 18:57:52

by Alban

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

On Fri, 21 Nov 2014 18:51:51 +0000
Ben Hutchings <[email protected]> wrote:

> On Fri, 2014-11-21 at 14:58 +0100, Alban wrote:
> > On Fri, 21 Nov 2014 00:34:34 +0000
> > Ben Hutchings <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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

2014-11-23 02:43:14

by Ben Hutchings

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

On Fri, 2014-11-21 at 19:57 +0100, Alban wrote:
> On Fri, 21 Nov 2014 18:51:51 +0000
> Ben Hutchings <[email protected]> wrote:
>
> > On Fri, 2014-11-21 at 14:58 +0100, Alban wrote:
> > > On Fri, 21 Nov 2014 00:34:34 +0000
> > > Ben Hutchings <[email protected]> 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 <[email protected]>
> > > > > ---
> > > > > 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.

Attachments: