* ppp_generic: fix multilink fragment MTU calculation (again)
@ 2011-09-18 23:41 Henry Wong
2011-09-20 19:21 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Henry Wong @ 2011-09-18 23:41 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 914 bytes --]
When using MLPPP, the maximum size of a fragment is incorrectly
calculated with an offset of -2.
This patch reverses the changes in the patch found here:
http://marc.info/?l=linux-netdev&m=123541324010539&w=2
The value of hdrlen includes the size of both the 2-byte PPP protocol
field and the 2- or 4-byte multilink header (2+4=6 for long sequence
numbers, 2+2=4 for short sequence numbers). Section 2 of RFC1661 says
that the MRU that is negotiated (i.e., the MTU of the sending system)
includes only the PPP payload but not the protocol field, thus the
correct MTU should be the link's MTU minus the multilink header (mtu -
(hdrlen-2)).
The incorrect calculation causes Linux to fragment packets to a size two
bytes smaller than the allowed MTU. While not technically illegal, this
behaviour confounds MRU-tuning to avoid PPP-layer fragmentation.
Signed-off-by: Henry Wong <henry@stuffedcow.net>
[-- Attachment #2: mlppp-frag-mtu.patch --]
[-- Type: text/x-patch, Size: 646 bytes --]
diff -urp linux-orig/drivers/net/ppp_generic.c linux-new/drivers/net/ppp_generic.c
--- linux-orig/drivers/net/ppp_generic.c 2011-09-18 17:59:18.000000000 -0400
+++ linux-new/drivers/net/ppp_generic.c 2011-09-18 18:05:29.000000000 -0400
@@ -1464,8 +1464,13 @@ static int ppp_mp_explode(struct ppp *pp
spin_unlock_bh(&pch->downl);
continue;
}
-
- mtu = pch->chan->mtu - hdrlen;
+
+ /*
+ * hdrlen includes the 2-byte PPP protocol field, but the
+ * MTU counts only the payload excluding the protocol field.
+ * (RFC1661 Section 2)
+ */
+ mtu = pch->chan->mtu - (hdrlen - 2);
if (mtu < 4)
mtu = 4;
if (flen > mtu)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ppp_generic: fix multilink fragment MTU calculation (again)
2011-09-18 23:41 ppp_generic: fix multilink fragment MTU calculation (again) Henry Wong
@ 2011-09-20 19:21 ` David Miller
2011-09-20 19:54 ` Henry Wong
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2011-09-20 19:21 UTC (permalink / raw)
To: v4l; +Cc: netdev
From: Henry Wong <v4l@stuffedcow.net>
Date: Sun, 18 Sep 2011 19:41:49 -0400
> When using MLPPP, the maximum size of a fragment is incorrectly
> calculated with an offset of -2.
> This patch reverses the changes in the patch found here:
> http://marc.info/?l=linux-netdev&m=123541324010539&w=2
>
> The value of hdrlen includes the size of both the 2-byte PPP protocol
> field and the 2- or 4-byte multilink header (2+4=6 for long sequence
> numbers, 2+2=4 for short sequence numbers). Section 2 of RFC1661 says
> that the MRU that is negotiated (i.e., the MTU of the sending system)
> includes only the PPP payload but not the protocol field, thus the
> correct MTU should be the link's MTU minus the multilink header (mtu -
> (hdrlen-2)).
>
> The incorrect calculation causes Linux to fragment packets to a size
> two bytes smaller than the allowed MTU. While not technically illegal,
> this behaviour confounds MRU-tuning to avoid PPP-layer fragmentation.
>
> Signed-off-by: Henry Wong <henry@stuffedcow.net>
Applied, thanks.
There were several cases of trailing whitespace in your patch which I
needed to fix up. Please don't be so careless in the future.
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ppp_generic: fix multilink fragment MTU calculation (again)
2011-09-20 19:21 ` David Miller
@ 2011-09-20 19:54 ` Henry Wong
2011-09-20 20:01 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Henry Wong @ 2011-09-20 19:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On 9/20/2011 3:21 PM, David Miller wrote:
> From: Henry Wong<v4l@stuffedcow.net>
> Date: Sun, 18 Sep 2011 19:41:49 -0400
>
>> When using MLPPP, the maximum size of a fragment is incorrectly
>> calculated with an offset of -2.
>> This patch reverses the changes in the patch found here:
>> http://marc.info/?l=linux-netdev&m=123541324010539&w=2
>>
>> The value of hdrlen includes the size of both the 2-byte PPP protocol
>> field and the 2- or 4-byte multilink header (2+4=6 for long sequence
>> numbers, 2+2=4 for short sequence numbers). Section 2 of RFC1661 says
>> that the MRU that is negotiated (i.e., the MTU of the sending system)
>> includes only the PPP payload but not the protocol field, thus the
>> correct MTU should be the link's MTU minus the multilink header (mtu -
>> (hdrlen-2)).
>>
>> The incorrect calculation causes Linux to fragment packets to a size
>> two bytes smaller than the allowed MTU. While not technically illegal,
>> this behaviour confounds MRU-tuning to avoid PPP-layer fragmentation.
>>
>> Signed-off-by: Henry Wong<henry@stuffedcow.net>
> Applied, thanks.
>
> There were several cases of trailing whitespace in your patch which I
> needed to fix up. Please don't be so careless in the future.
>
> Thanks.
>
Thanks.
Out of curiosity, were the trailing whitespaces on Lines 10 and 11? Or
was there a problem with file attachments that cause some whitespace I
don't see? I'll be more careful next time :)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ppp_generic: fix multilink fragment MTU calculation (again)
2011-09-20 19:54 ` Henry Wong
@ 2011-09-20 20:01 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2011-09-20 20:01 UTC (permalink / raw)
To: v4l; +Cc: netdev
From: Henry Wong <v4l@stuffedcow.net>
Date: Tue, 20 Sep 2011 15:54:32 -0400
> Out of curiosity, were the trailing whitespaces on Lines 10 and 11? Or
> was there a problem with file attachments that cause some whitespace I
> don't see? I'll be more careful next time :)
Both the line before the comment (2 tabs) and the first line of the
comment (one space).
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-09-20 20:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-18 23:41 ppp_generic: fix multilink fragment MTU calculation (again) Henry Wong
2011-09-20 19:21 ` David Miller
2011-09-20 19:54 ` Henry Wong
2011-09-20 20:01 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).