[Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

[Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

[Robert Bradley]

For those of you not following the openwrt-devel mailing list, this is
an updated version of the baby-jumbo patch I posted previously in
response to Alexander's post.  This version is slightly less
conservative about extended frame lengths (up to 1518 octets), but
removes the awkward magic constant.  The maximum MTU is now calculated
by subtracting header lengths from AG71XX_TX_MTU_LEN.  I also use this
to calculate the maximum received packet size.  Defining
AG71XX_MAX_DATA_LEN as 1500 (or ETH_DATA_LEN) restores the original
behaviour.


---------- Forwarded message ----------
From: Robert Bradley <robert.bradley1@gmail.com>
Date: 22 June 2012 13:50
Subject: [PATCH] ag71xx: Added support for baby-jumbo packets.
To: openwrt-devel@lists.openwrt.org


RFC 4638 support means that a full 1500-octet frame can be transmitted
through a PPPoE tunnel.  For this to work, the network card must support
baby-jumbo frames of 1508 octets plus headers.  This patch enables the use
of MTUs up to 1518 octets (derived from the existing AG71XX_TX_MTU_LEN
value by subtracting header sizes).

The default MTU size is unchanged; if large frames are desired, ifconfig
must be used to set the new MTU.

Signed-off-by: Robert Bradley <robert.bradley1@gmail.com>
---
 .../files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h | 17 ++++++++++++++++-
 .../drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c  | 18 +++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
index b9d95ad..7f77a59 100644
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
@@ -51,8 +51,23 @@
 #define AG71XX_INT_INIT        (AG71XX_INT_ERR | AG71XX_INT_POLL)

 #define AG71XX_TX_MTU_LEN      1540
+/*
+ * AG71XX_MAX_DATA_LEN equals the maximum possible MTU that a frame of length
+ * AG71XX_TX_MTU_LEN can have.  This may be larger than ETH_DATA_LEN!  This
+ * define is necessary for baby jumbo packet support (as per RFC 4638).
+ *
+ * Redefining this as ETH_DATA_LEN restores the 1500-octet MTU limit.
+ */
+#define AG71XX_MAX_DATA_LEN    \
+       (AG71XX_TX_MTU_LEN - (ETH_FCS_LEN + VLAN_HLEN + ETH_HLEN))
+/*
+ * Define AG71XX_RX_PKT_SIZE using AG71XX_MAX_DATA_LEN + ETH_HLEN rather than
+ * ETH_FRAME_LEN so that baby jumbo frames can be used safely.  This definition
+ * means that setting AG71XX_MAX_DATA_LEN to ETH_DATA_LEN restores the old
+ * values, which are safe for standard frames.
+ */
 #define AG71XX_RX_PKT_SIZE     \
-       (ETH_FRAME_LEN + ETH_FCS_LEN + VLAN_HLEN)
+       (AG71XX_MAX_DATA_LEN + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN)
 #define AG71XX_RX_BUF_SIZE (AG71XX_RX_PKT_SIZE + NET_SKB_PAD + NET_IP_ALIGN)

 #define AG71XX_TX_RING_SIZE_DEFAULT    64
diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
index fb99d27..d3e850b 100644
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
@@ -1042,13 +1042,29 @@ static void ag71xx_netpoll(struct net_device *dev)
 }
 #endif

+/*
+ * ag71xx_change_mtu: set interface MTU.
+ * A modified eth_change_mtu, changing the upper limit from ETH_DATA_LEN to
+ * AG71XX_MAX_DATA_LEN.  This means that baby jumbo packets may be used by
+ * PPPoE users, assuming that AG71XX_MAX_DATA_LEN > ETH_DATA_LEN.
+ *
+ * Returns 0 on success, -EINVAL otherwise.
+ */
+static int ag71xx_change_mtu(struct net_device *dev, int new_mtu)
+{
+       if (new_mtu < 68 || new_mtu > AG71XX_MAX_DATA_LEN)
+                       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
--
Robert Bradley


-- 
Robert Bradley

Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

[dpreed]

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


Good step.  Why not allow 9K byte jumbos when one's packets traverse a path that is internal to the local area, and all the 1 GigE links support 9K?
 
-----Original Message-----
From: "Robert Bradley" <robert.bradley1@gmail.com>
Sent: Friday, June 22, 2012 3:12pm
To: cerowrt-devel@lists.bufferbloat.net
Subject: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.



For those of you not following the openwrt-devel mailing list, this is
an updated version of the baby-jumbo patch I posted previously in
response to Alexander's post.  This version is slightly less
conservative about extended frame lengths (up to 1518 octets), but
removes the awkward magic constant.  The maximum MTU is now calculated
by subtracting header lengths from AG71XX_TX_MTU_LEN.  I also use this
to calculate the maximum received packet size.  Defining
AG71XX_MAX_DATA_LEN as 1500 (or ETH_DATA_LEN) restores the original
behaviour.


---------- Forwarded message ----------
From: Robert Bradley <robert.bradley1@gmail.com>
Date: 22 June 2012 13:50
Subject: [PATCH] ag71xx: Added support for baby-jumbo packets.
To: openwrt-devel@lists.openwrt.org


RFC 4638 support means that a full 1500-octet frame can be transmitted
through a PPPoE tunnel.  For this to work, the network card must support
baby-jumbo frames of 1508 octets plus headers.  This patch enables the use
of MTUs up to 1518 octets (derived from the existing AG71XX_TX_MTU_LEN
value by subtracting header sizes).

The default MTU size is unchanged; if large frames are desired, ifconfig
must be used to set the new MTU.

Signed-off-by: Robert Bradley <robert.bradley1@gmail.com>
---
 .../files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h | 17 ++++++++++++++++-
 .../drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c  | 18 +++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
index b9d95ad..7f77a59 100644
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
@@ -51,8 +51,23 @@
 #define AG71XX_INT_INIT        (AG71XX_INT_ERR | AG71XX_INT_POLL)

 #define AG71XX_TX_MTU_LEN      1540
+/*
+ * AG71XX_MAX_DATA_LEN equals the maximum possible MTU that a frame of length
+ * AG71XX_TX_MTU_LEN can have.  This may be larger than ETH_DATA_LEN!  This
+ * define is necessary for baby jumbo packet support (as per RFC 4638).
+ *
+ * Redefining this as ETH_DATA_LEN restores the 1500-octet MTU limit.
+ */
+#define AG71XX_MAX_DATA_LEN    \
+       (AG71XX_TX_MTU_LEN - (ETH_FCS_LEN + VLAN_HLEN + ETH_HLEN))
+/*
+ * Define AG71XX_RX_PKT_SIZE using AG71XX_MAX_DATA_LEN + ETH_HLEN rather than
+ * ETH_FRAME_LEN so that baby jumbo frames can be used safely.  This definition
+ * means that setting AG71XX_MAX_DATA_LEN to ETH_DATA_LEN restores the old
+ * values, which are safe for standard frames.
+ */
 #define AG71XX_RX_PKT_SIZE     \
-       (ETH_FRAME_LEN + ETH_FCS_LEN + VLAN_HLEN)
+       (AG71XX_MAX_DATA_LEN + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN)
 #define AG71XX_RX_BUF_SIZE (AG71XX_RX_PKT_SIZE + NET_SKB_PAD + NET_IP_ALIGN)

 #define AG71XX_TX_RING_SIZE_DEFAULT    64
diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
index fb99d27..d3e850b 100644
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
@@ -1042,13 +1042,29 @@ static void ag71xx_netpoll(struct net_device *dev)
 }
 #endif

+/*
+ * ag71xx_change_mtu: set interface MTU.
+ * A modified eth_change_mtu, changing the upper limit from ETH_DATA_LEN to
+ * AG71XX_MAX_DATA_LEN.  This means that baby jumbo packets may be used by
+ * PPPoE users, assuming that AG71XX_MAX_DATA_LEN > ETH_DATA_LEN.
+ *
+ * Returns 0 on success, -EINVAL otherwise.
+ */
+static int ag71xx_change_mtu(struct net_device *dev, int new_mtu)
+{
+       if (new_mtu < 68 || new_mtu > AG71XX_MAX_DATA_LEN)
+                       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
--
Robert Bradley


-- 
Robert Bradley
_______________________________________________
Cerowrt-devel mailing list
Cerowrt-devel@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cerowrt-devel

[-- Attachment #2: Type: text/html, Size: 6300 bytes --]

Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

[Robert Bradley]

On 22/06/12 21:11, dpreed@reed.com wrote:
> Good step.  Why not allow 9K byte jumbos when one's packets traverse a path that is internal to the local area, and all the 1 GigE links support 9K?
>

All the posts I could see claim that the System-on-Chip NIC (Atheros 
AR7161) cannot handle packets greater than 1540 octets, so routing 9k 
packets from wired->wireless or wired->WAN would not be an option.  If I 
understood Dave's previous post correctly, though, the built-in switch 
allows jumbo packets, so wired->wired internal traffic should work fine 
with 9K packets already.

The only issue with that setup is that with mismatched MTUs, it might be 
impossible to communicate with the router and the 9k-friendly nodes at 
the same time.  Thankfully, on Linux at least, you can set per-route 
MTUs (http://lartc.org/howto/lartc.cookbook.mtu-discovery.html), so it 
might be possible to exploit that to do what you want.  Maybe something 
like this would work?

ip route add default via 172.30.42.1 mtu 1500
ip route add 172.30.42.1/32 dev eth0 mtu 1500
ip route add 172.30.42.1/27 dev eth0 mtu 9000

-- 
Robert Bradley

Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

[dpreed]

I find it curious that packet aggregation is done in 802.11n standards, but that the packets are limited to 1540.   I bet that limit may not be there in all Atheros NICs, and maybe not in this one.  Will investigate now that I'm curious.

-----Original Message-----
From: "Robert Bradley" <robert.bradley1@gmail.com>
Sent: Friday, June 22, 2012 4:41pm
To: cerowrt-devel@lists.bufferbloat.net
Subject: Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

On 22/06/12 21:11, dpreed@reed.com wrote:
> Good step.  Why not allow 9K byte jumbos when one's packets traverse a path that is internal to the local area, and all the 1 GigE links support 9K?
>

All the posts I could see claim that the System-on-Chip NIC (Atheros 
AR7161) cannot handle packets greater than 1540 octets, so routing 9k 
packets from wired->wireless or wired->WAN would not be an option.  If I 
understood Dave's previous post correctly, though, the built-in switch 
allows jumbo packets, so wired->wired internal traffic should work fine 
with 9K packets already.

The only issue with that setup is that with mismatched MTUs, it might be 
impossible to communicate with the router and the 9k-friendly nodes at 
the same time.  Thankfully, on Linux at least, you can set per-route 
MTUs (http://lartc.org/howto/lartc.cookbook.mtu-discovery.html), so it 
might be possible to exploit that to do what you want.  Maybe something 
like this would work?

ip route add default via 172.30.42.1 mtu 1500
ip route add 172.30.42.1/32 dev eth0 mtu 1500
ip route add 172.30.42.1/27 dev eth0 mtu 9000

-- 
Robert Bradley
_______________________________________________
Cerowrt-devel mailing list
Cerowrt-devel@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cerowrt-devel

Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

[Dave Taht]

On Sat, Jun 23, 2012 at 3:29 PM,  <dpreed@reed.com> wrote:
> I find it curious that packet aggregation is done in 802.11n standards, but that the packets are limited to 1540.   I bet that limit may not be there in all Atheros NICs, and maybe not in this one.  Will investigate now that I'm curious.

We are talking about two different interfaces here.

The switch in cerowrt is capable of 9K packets (maybe more, actually,
I'd have to look at the data sheet again)
The ag71xx is the ethernet driver, with the 1540-ish limitation
The ath9k is the wireless driver.

The MTU for 802.11 is 2304 bytes. I think there is a successor
standard which can do a little under 8k. Neither of which are used
used a lot. Interestingly in my previous project to this one (wisp6),
what I'd done was build a pure ipv6 wireless backbone and tunneled
ipv4 over it - encrypted - using larger MTUs on the wifi to do so to
ensure 1500MTU for the ipv4 stack.

This was in part due to Nicaragua already being behind at least 2
layers of NAT in the general case, so I fiddled with things like AFTR
to move the NAT closer to the gateway to the internet, and trying
really hard to lift things like otherwise disconnected schools into
the ipv6 world instead.

I ran into zillions of problems with this, while I sort of made it
work with the original hardware I used (which was an embedded arm
(pocobelle project) which has a max mtu of 2048), as I tried other
things like olpc, openrd (now dreamplug or smileplug), and the current
ar71xx/ath9k (nanostation M5s and 2HPs), I kept hitting walls in the
device drivers, in the encapsulation standards for things like
diffserv bits, in crypto in general, in the MTUs and PMTU, and oh,
yea, bufferbloat. Big time.

I abandoned the MTU changes along the way as too hard, and later on
had to shut down that project for other reasons.

And at the time (2009), the entire country had like 4 ipv6 prefixes in
use, and no ipv6 transit to speak of, with the pan-american fiber
internet 2 line basically cut between Honduras and costa rica, due to
lack of funds and attention, so almost all all US ipv4 traffic ended
up being routed through Florida, and I had to use hurricane electric
tunnels to even prototype the last hop.

The nearest F-root server was 80+ms away from where I was. Redwood
city, california, about 136ms.

>
> -----Original Message-----
> From: "Robert Bradley" <robert.bradley1@gmail.com>
> Sent: Friday, June 22, 2012 4:41pm
> To: cerowrt-devel@lists.bufferbloat.net
> Subject: Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.
>
> On 22/06/12 21:11, dpreed@reed.com wrote:
>> Good step.  Why not allow 9K byte jumbos when one's packets traverse a path that is internal to the local area, and all the 1 GigE links support 9K?
>>
>
> All the posts I could see claim that the System-on-Chip NIC (Atheros
> AR7161) cannot handle packets greater than 1540 octets, so routing 9k
> packets from wired->wireless or wired->WAN would not be an option.  If I
> understood Dave's previous post correctly, though, the built-in switch
> allows jumbo packets, so wired->wired internal traffic should work fine
> with 9K packets already.
>
> The only issue with that setup is that with mismatched MTUs, it might be
> impossible to communicate with the router and the 9k-friendly nodes at
> the same time.  Thankfully, on Linux at least, you can set per-route
> MTUs (http://lartc.org/howto/lartc.cookbook.mtu-discovery.html), so it
> might be possible to exploit that to do what you want.  Maybe something
> like this would work?
>
> ip route add default via 172.30.42.1 mtu 1500
> ip route add 172.30.42.1/32 dev eth0 mtu 1500
> ip route add 172.30.42.1/27 dev eth0 mtu 9000
>
> --
> Robert Bradley
> _______________________________________________
> Cerowrt-devel mailing list
> Cerowrt-devel@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cerowrt-devel
>
>
> _______________________________________________
> Cerowrt-devel mailing list
> Cerowrt-devel@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cerowrt-devel



-- 
Dave Täht
http://www.bufferbloat.net/projects/cerowrt/wiki - "3.3.8-6 is out
with fq_codel!"

Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

[Robert Bradley]

On 23/06/12 20:29, dpreed@reed.com wrote:
> I find it curious that packet aggregation is done in 802.11n standards, but that the packets are limited to 1540.   I bet that limit may not be there in all Atheros NICs, and maybe not in this one.  Will investigate now that I'm curious.
>
>

The patch only affects the wired Ethernet driver, so 802.11n packet 
aggregation does not apply here.  TSO and friends could apply in theory, 
but either is unsupported or disabled in this case.  The wireless side 
is handled by the ath9k driver instead, where I know nothing about the 
upper limits on MTU.  Depending on the cards used by the clients, the 
maximum MTU may be limited to 1500 even if the ath9k driver can cope.
-- 
Robert Bradley

Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

[dpreed]

All true, but most TCP's these days support Path MTU discovery, and most apps that can use > 1500 bytes test the path before using, even if using UDP.

Setting lower MTUs on a home network, etc. is useful only if the large MTUs are problematic on the links involved.   A 9k MTU is bad on a 128k ISDN link, for sure.

Preserving legacy limits because there were problems with them 5 years ago is a sure way to lock yourself in to problems.   Drivers shouldn't have the limits compiled in, instead you can use commands to shrink MTU if need to do so exists.

-----Original Message-----
From: "Robert Bradley" <robert.bradley1@gmail.com>
Sent: Saturday, June 23, 2012 5:47pm
To: cerowrt-devel@lists.bufferbloat.net
Subject: Re: [Cerowrt-devel] Fwd: [PATCH] ag71xx: Added support for baby-jumbo packets.

On 23/06/12 20:29, dpreed@reed.com wrote:
> I find it curious that packet aggregation is done in 802.11n standards, but that the packets are limited to 1540.   I bet that limit may not be there in all Atheros NICs, and maybe not in this one.  Will investigate now that I'm curious.
>
>

The patch only affects the wired Ethernet driver, so 802.11n packet 
aggregation does not apply here.  TSO and friends could apply in theory, 
but either is unsupported or disabled in this case.  The wireless side 
is handled by the ath9k driver instead, where I know nothing about the 
upper limits on MTU.  Depending on the cards used by the clients, the 
maximum MTU may be limited to 1500 even if the ath9k driver can cope.
-- 
Robert Bradley
_______________________________________________
Cerowrt-devel mailing list
Cerowrt-devel@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cerowrt-devel