Development issues regarding the cerowrt test router project
 help / color / mirror / Atom feed
From: Robert Bradley <robert.bradley1@gmail.com>
To: cerowrt-devel@lists.bufferbloat.net
Subject: Re: [Cerowrt-devel] Baby jumbo frames support?
Date: Thu, 21 Jun 2012 14:33:13 +0100	[thread overview]
Message-ID: <CAA=Zby4q8fmNyu3Vg9DGwWK3BBWLz620b191xALEJ2oZ5Ku2eg@mail.gmail.com> (raw)
In-Reply-To: <CAA93jw4RHGg2MqScwA3RgbU+xDfuqOcbCnvYYU85NacrtuC-Fg@mail.gmail.com>

On 21 June 2012 01:58, Dave Taht <dave.taht@gmail.com> wrote:
> As for PPoE with a size 1508... um... one or the other device is going
> to get in your way here. I presume that 1500 works? You would do
> better to contact the author of the driver (juhosg) to get your
> question answered as I'm under the impression he is under the right
> NDAs.
>

I think the point here is that MTU=1500 works, but once you add in the
PPPoE header, you end up with an effective MTU of 1492 for outbound
packets:

http://aa.net.uk/kb-broadband-mtu.html
http://tools.ietf.org/html/rfc4638

The short answer is that without baby-jumbo support, you either end up
fragmenting packets or you need to somehow restrict the MTU manually.
You can do that either through MSS clamping or simply configuring each
internal machine to use MTU=1492.  To get around this, the BT ADSL
modems started to support MTU=1508.  This means that the MTU within
the PPPoE tunnel remains at Ethernet-standard 1500, and avoids the
fragmentation or reconfiguration issues.

As for supporting it in CeroWRT ... the ag71xx driver defines
AG71XX_TX_MTU_LEN=1540, so it looks safe enough to use MTU 1508,
especially if you know that no vlans or other additions to the
standard header will be used.  To enable that, you need to reimplement
the eth_change_mtu function for the driver.  The current code uses the
kernel's implementation, which restricts the MTU to 1500.  An initial,
naive patch would look something like:

----
--- C:/Users/robert/AppData/Local/Temp/ag71x-revBASE.svn000.tmp.c	Mon
May 28 03:55:59 2012
+++ C:/Users/robert/Desktop/ag71xx/ag71xx_main.c	Thu Jun 21 13:58:44 2012
@@ -1042,13 +1042,25 @@
 }
 #endif

+/*
+ * Copied from eth_change_mtu and modified so that baby jumbo packets
+ * may be used.  This has not been tested!
+ */
+int ag71xx_change_mtu(struct net_device *dev, int new_mtu)
+{
+        if (new_mtu < 68 || new_mtu > (ETH_DATA_LEN + 8))
+                return -EINVAL;
+        dev->mtu = new_mtu;
+        return 0;
+}
+
 static const struct net_device_ops ag71xx_netdev_ops = {
 	.ndo_open		= ag71xx_open,
 	.ndo_stop		= ag71xx_stop,
 	.ndo_start_xmit		= ag71xx_hard_start_xmit,
 	.ndo_do_ioctl		= ag71xx_do_ioctl,
 	.ndo_tx_timeout		= ag71xx_tx_timeout,
-	.ndo_change_mtu		= eth_change_mtu,
+	.ndo_change_mtu		= ag71xx_change_mtu,
 	.ndo_set_mac_address	= eth_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
 #ifdef CONFIG_NET_POLL_CONTROLLER

----

where I've copied the original function and changed the upper limit to
ETH_DATA_LEN+8, then set up the netdev_ops structure to call the new
version.  In reality, you probably want to add some better checks
(testing for MTU+all possible headers<1540?) and remove the magic
constant - in the worst case, something closer to the e1000 driver's
implementation.  I wouldn't recommend using the present version on
anything other than an experimental build, but the default MTU would
be 1500 anyway so should avoid causing too much damage.  Those on BT
ADSL lines can change the MTU on ge00 themselves and see what breaks.
-- 
Robert Bradley

  reply	other threads:[~2012-06-21 13:33 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-16  9:53 Alexander Hulse
2012-06-16 12:54 ` dpreed
2012-06-21  0:50   ` Dave Taht
2012-06-21  0:58 ` Dave Taht
2012-06-21 13:33   ` Robert Bradley [this message]
2012-06-21 14:25     ` dpreed
2012-06-21 17:03       ` Robert Bradley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.bufferbloat.net/postorius/lists/cerowrt-devel.lists.bufferbloat.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAA=Zby4q8fmNyu3Vg9DGwWK3BBWLz620b191xALEJ2oZ5Ku2eg@mail.gmail.com' \
    --to=robert.bradley1@gmail.com \
    --cc=cerowrt-devel@lists.bufferbloat.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox