[PATCH 1/2] ipv6: fix mss when it is bigger than IPV6_MAXPLEN - sizeof(struct tcphdr)

From: RongQing.Li <redacted>

When mss is bigger than IPV6_MAXPLEN - sizeof(struct tcphdr),
set mss to IPV6_MAXPLEN - sizeof(struct tcphdr), not IPV6_MAXPLEN.

Signed-off-by: RongQing.Li <redacted>
---
 net/ipv6/route.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 92be12b..42cc16c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c

@@ -1054,7 +1054,7 @@ static unsigned int ip6_default_advmss(const struct dst_entry *dst)
 	 * rely only on pmtu discovery"
 	 */
 	if (mtu > IPV6_MAXPLEN - sizeof(struct tcphdr))
-		mtu = IPV6_MAXPLEN;
+		mtu = IPV6_MAXPLEN - sizeof(struct tcphdr);
 	return mtu;
 }
 

-- 
1.7.1

From: RongQing.Li <redacted>

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 <redacted>
---
 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;
 }

-- 
1.7.1

roy.qing.li@gmail.com [off-list ref] :
[...]

What would prevent net/ipv4/tcp_output.c::tcp_advertise_mss returning
a value beyond 65535 - 40 ?

-- 
Ueimor

2012/2/28 Francois Romieu [off-list ref]:

roy.qing.li@gmail.com [off-list ref] :
[...]

quoted

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

Le mardi 28 février 2012 à 16:28 +0800, roy.qing.li@gmail.com a écrit :

Hmm... What about the comment above the code mentioning pmtu discovery ?

        /*
         * Maximal non-jumbo IPv6 payload is IPV6_MAXPLEN and
         * corresponding MSS is IPV6_MAXPLEN - tcp_header_size.
         * IPV6_MAXPLEN is also valid and means: "any MSS,
         * rely only on pmtu discovery"
         */


Check out code from net/ipv6/ip6_output.c, ip6_append_data() :

        if (mtu <= sizeof(struct ipv6hdr) + IPV6_MAXPLEN) {
                if (cork->length + length > sizeof(struct ipv6hdr) + IPV6_MAXPLEN - fragheaderlen) {
                        ipv6_local_error(sk, EMSGSIZE, fl6, mtu-exthdrlen);
                        return -EMSGSIZE;
                }
        }

and __ip6_local_out()

	if (len > IPV6_MAXPLEN)
		len = 0;

Le mardi 28 février 2012 à 16:28 +0800, roy.qing.li@gmail.com a écrit :

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 ?