Patchwork [2/2] ipv4: fix mss comparison

login
register
mail settings
Submitter roy.qing.li@gmail.com
Date Feb. 28, 2012, 8:28 a.m.
Message ID <1330417697-2637-2-git-send-email-roy.qing.li@gmail.com>
Download mbox | patch
Permalink /patch/143374/
State Rejected
Delegated to: David Miller
Headers show

Comments

roy.qing.li@gmail.com - Feb. 28, 2012, 8:28 a.m.
From: RongQing.Li <roy.qing.li@gmail.com>

mss should compare with 65535 - sizeof(struct tcphdr),  
the largest mss is 65535 - sizeof(struct tcphdr).  
40 is sum of sizeof(struct tcphdr) and sizeof(struct iphdr)

Signed-off-by: RongQing.Li <roy.qing.li@gmail.com>
---
 net/ipv4/route.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
françois romieu - Feb. 28, 2012, 9:34 a.m.
roy.qing.li@gmail.com <roy.qing.li@gmail.com> :
[...]
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 0489ced..c72f765 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1914,8 +1914,8 @@ static unsigned int ipv4_default_advmss(const struct dst_entry *dst)
>  	if (advmss == 0) {
>  		advmss = max_t(unsigned int, dst->dev->mtu - 40,
>  			       ip_rt_min_advmss);
> -		if (advmss > 65535 - 40)
> -			advmss = 65535 - 40;
> +		if (advmss > 65535 - sizeof(struct tcphdr))
> +			advmss = 65535 - sizeof(struct tcphdr);

What would prevent net/ipv4/tcp_output.c::tcp_advertise_mss returning
a value beyond 65535 - 40 ?
roy.qing.li@gmail.com - Feb. 28, 2012, 9:55 a.m.
2012/2/28 Francois Romieu <romieu@fr.zoreil.com>:
> roy.qing.li@gmail.com <roy.qing.li@gmail.com> :
> [...]
>> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
>> index 0489ced..c72f765 100644
>> --- a/net/ipv4/route.c
>> +++ b/net/ipv4/route.c
>> @@ -1914,8 +1914,8 @@ static unsigned int ipv4_default_advmss(const struct dst_entry *dst)
>>       if (advmss == 0) {
>>               advmss = max_t(unsigned int, dst->dev->mtu - 40,
>>                              ip_rt_min_advmss);
>> -             if (advmss > 65535 - 40)
>> -                     advmss = 65535 - 40;
>> +             if (advmss > 65535 - sizeof(struct tcphdr))
>> +                     advmss = 65535 - sizeof(struct tcphdr);
>
> What would prevent net/ipv4/tcp_output.c::tcp_advertise_mss returning
> a value beyond 65535 - 40 ?
>

You are right, Please ignore this patch.
The biggest MSS should be (Total Length of ipv4 header -
sizeof(iphdr)- sizeof(tcphdr))

I mix up the Total Length of ipv4 header and Payload Length of ipv6 header.

> --
> Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - Feb. 28, 2012, 10:01 a.m.
Le mardi 28 février 2012 à 16:28 +0800, roy.qing.li@gmail.com a écrit :
> From: RongQing.Li <roy.qing.li@gmail.com>
> 
> mss should compare with 65535 - sizeof(struct tcphdr),  
> the largest mss is 65535 - sizeof(struct tcphdr).  
> 40 is sum of sizeof(struct tcphdr) and sizeof(struct iphdr)
> 
> Signed-off-by: RongQing.Li <roy.qing.li@gmail.com>
> ---
>  net/ipv4/route.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 0489ced..c72f765 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1914,8 +1914,8 @@ static unsigned int ipv4_default_advmss(const struct dst_entry *dst)
>  	if (advmss == 0) {
>  		advmss = max_t(unsigned int, dst->dev->mtu - 40,
>  			       ip_rt_min_advmss);
> -		if (advmss > 65535 - 40)
> -			advmss = 65535 - 40;
> +		if (advmss > 65535 - sizeof(struct tcphdr))
> +			advmss = 65535 - sizeof(struct tcphdr);
>  	}
>  	return advmss;
>  }

Hmm... I am puzzled.

1) If you touch this code, please use min_t() macro to be consistent
with prior line ? Please note I dont advocate a general use of these
min/max macros, just some consistency in the same page of code.

2) Prior line still has the magic 40 value.
    (I personally like 40 more than sizeof(struct tcphdr) +
sizeof(struct iphdr)), its not like 40 is going to change one day :)

3) What is the maximum datagram size in ipv4.
      (Say you name this IPV4_MAXPLEN)

4) What would be the meaning of having TCP MSS bigger than
    IPV4_MAXPLEN - 40 ?



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 0489ced..c72f765 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1914,8 +1914,8 @@  static unsigned int ipv4_default_advmss(const struct dst_entry *dst)
 	if (advmss == 0) {
 		advmss = max_t(unsigned int, dst->dev->mtu - 40,
 			       ip_rt_min_advmss);
-		if (advmss > 65535 - 40)
-			advmss = 65535 - 40;
+		if (advmss > 65535 - sizeof(struct tcphdr))
+			advmss = 65535 - sizeof(struct tcphdr);
 	}
 	return advmss;
 }