ssh server etc doesn't work anymore with net-2.6

ssh server etc doesn't work anymore with net-2.6

[Yinghai Lu]

caused by:

commit 2626419ad5be1a054d350786b684b41d23de1538
Author: David S. Miller <davem@davemloft.net>
Date:   Thu Apr 8 11:32:30 2010 -0700

    tcp: Set CHECKSUM_UNNECESSARY in tcp_init_nondata_skb

it will make sshd server, telnet server and vnc server doesn't respond
connecting request anymore.

YH

Re: ssh server etc doesn't work anymore with net-2.6

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Sun, 11 Apr 2010 02:17:09 -0700

> caused by:
> 
> commit 2626419ad5be1a054d350786b684b41d23de1538
> Author: David S. Miller <davem@davemloft.net>
> Date:   Thu Apr 8 11:32:30 2010 -0700
> 
>     tcp: Set CHECKSUM_UNNECESSARY in tcp_init_nondata_skb
> 
> it will make sshd server, telnet server and vnc server doesn't respond
> connecting request anymore.

I'll either fix or revert this, thanks for reporting.

Re: ssh server etc doesn't work anymore with net-2.6

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Sun, 11 Apr 2010 02:17:09 -0700

> it will make sshd server, telnet server and vnc server doesn't respond
> connecting request anymore.

What kind of network device are you using?

And regardless of the underlying physical network device, are you
going over some virtual device such as a tunnel, a bonding device, or
a virtualization network interface?

Thanks.

Re: ssh server etc doesn't work anymore with net-2.6

[Yinghai Lu]

On Sun, Apr 11, 2010 at 2:24 AM, David Miller <davem@davemloft.net> wrote:
> From: Yinghai Lu <yinghai@kernel.org>
> Date: Sun, 11 Apr 2010 02:17:09 -0700
>
>> it will make sshd server, telnet server and vnc server doesn't respond
>> connecting request anymore.
>
> What kind of network device are you using?
>
> And regardless of the underlying physical network device, are you
> going over some virtual device such as a tunnel, a bonding device, or
> a virtualization network interface?

server side: 82576 with net-2.6
client side: opensuse 11.2 on thinkpad T61 wireless + vpnc

Yinghai

Re: ssh server etc doesn't work anymore with net-2.6

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Sun, 11 Apr 2010 02:32:33 -0700

> On Sun, Apr 11, 2010 at 2:24 AM, David Miller <davem@davemloft.net> wrote:
>> From: Yinghai Lu <yinghai@kernel.org>
>> Date: Sun, 11 Apr 2010 02:17:09 -0700
>>
>>> it will make sshd server, telnet server and vnc server doesn't respond
>>> connecting request anymore.
>>
>> What kind of network device are you using?
>>
>> And regardless of the underlying physical network device, are you
>> going over some virtual device such as a tunnel, a bonding device, or
>> a virtualization network interface?
> 
> server side: 82576 with net-2.6
> client side: opensuse 11.2 on thinkpad T61 wireless + vpnc

Thanks.

Re: ssh server etc doesn't work anymore with net-2.6

[David Miller]

From: Yinghai Lu <yinghai@kernel.org>
Date: Sun, 11 Apr 2010 02:32:33 -0700

> server side: 82576 with net-2.6
> client side: opensuse 11.2 on thinkpad T61 wireless + vpnc

I've pushed out a revert, we'll sort this out in net-next-2.6
and meanwhile I've started a dialog with the Intel driver
folks to see if there IGB chip has a problem with handling
CHECKSUM_PARTIAL packets with a zero length data area.

Thanks for your report.

Re: ssh server etc doesn't work anymore with net-2.6

[Herbert Xu]

David Miller <davem@davemloft.net> wrote:
> From: Yinghai Lu <yinghai@kernel.org>
> Date: Sun, 11 Apr 2010 02:32:33 -0700
> 
>> server side: 82576 with net-2.6
>> client side: opensuse 11.2 on thinkpad T61 wireless + vpnc
> 
> I've pushed out a revert, we'll sort this out in net-next-2.6
> and meanwhile I've started a dialog with the Intel driver
> folks to see if there IGB chip has a problem with handling
> CHECKSUM_PARTIAL packets with a zero length data area.
> 
> Thanks for your report.

I was confused at first as to whether it was CHECKSUM_UNNECESSARY
that broke or CHECKSUM_PARTIAL.

After looking at the actual net-2.6 tree I see that it is actually
CHECKSUM_PARTIAL that caused this breakage.

The fact that when this was first implemented we didn't use hw
checksums on dataless packets might not have been an oversight
after all.

As I don't think such packets have been tested all by our kernel
on any NICs, and given the fact that IGB apparently broke, I think
the risk of this change is too high.

However, all is not lost as we can still adopt the approach of
the actual patch subject :) That is, we can use CHECKSUM_UNNECESSARY
for packets checksummed locally by software.

This does need a lot more testing so it should spend some time
in net-next first.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: ssh server etc doesn't work anymore with net-2.6

[Herbert Xu]

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> After looking at the actual net-2.6 tree I see that it is actually
> CHECKSUM_PARTIAL that caused this breakage.
> 
> The fact that when this was first implemented we didn't use hw
> checksums on dataless packets might not have been an oversight
> after all.

Looks like I was too quick to blame the hardware, the synack
code can't handle CHECKSUM_PARTIAL so this is probably the real
cause.

I will send patches to fix this.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: ssh server etc doesn't work anymore with net-2.6

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sun, 11 Apr 2010 20:04:03 +0800

> Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> 
>> After looking at the actual net-2.6 tree I see that it is actually
>> CHECKSUM_PARTIAL that caused this breakage.
>> 
>> The fact that when this was first implemented we didn't use hw
>> checksums on dataless packets might not have been an oversight
>> after all.
> 
> Looks like I was too quick to blame the hardware, the synack
> code can't handle CHECKSUM_PARTIAL so this is probably the real
> cause.
> 
> I will send patches to fix this.

Thanks a lot for figuring this out.

I'll put my original patch (with fixed commit log message!)
and your fixes into net-next-2.6

[PATCH 1/3] tcp: Handle CHECKSUM_PARTIAL for SYNACK packets for IPv4

[Herbert Xu]

tcp: Handle CHECKSUM_PARTIAL for SYNACK packets for IPv4

This patch moves the common code between tcp_v4_send_check and
tcp_v4_gso_send_check into a new function __tcp_v4_send_check.

It then uses the new function in tcp_v4_send_synack so that it
handles CHECKSUM_PARTIAL properly.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 net/ipv4/tcp_ipv4.c |   31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3c23e70..aebfd28 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -519,26 +519,31 @@ out:
 	sock_put(sk);
 }
 
-/* This routine computes an IPv4 TCP checksum. */
-void tcp_v4_send_check(struct sock *sk, int len, struct sk_buff *skb)
+static void __tcp_v4_send_check(struct sk_buff *skb,
+				__be32 saddr, __be32 daddr)
 {
-	struct inet_sock *inet = inet_sk(sk);
 	struct tcphdr *th = tcp_hdr(skb);
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		th->check = ~tcp_v4_check(len, inet->inet_saddr,
-					  inet->inet_daddr, 0);
+		th->check = ~tcp_v4_check(skb->len, saddr, daddr, 0);
 		skb->csum_start = skb_transport_header(skb) - skb->head;
 		skb->csum_offset = offsetof(struct tcphdr, check);
 	} else {
-		th->check = tcp_v4_check(len, inet->inet_saddr,
-					 inet->inet_daddr,
+		th->check = tcp_v4_check(skb->len, saddr, daddr,
 					 csum_partial(th,
 						      th->doff << 2,
 						      skb->csum));
 	}
 }
 
+/* This routine computes an IPv4 TCP checksum. */
+void tcp_v4_send_check(struct sock *sk, int len, struct sk_buff *skb)
+{
+	struct inet_sock *inet = inet_sk(sk);
+
+	__tcp_v4_send_check(skb, inet->inet_saddr, inet->inet_daddr);
+}
+
 int tcp_v4_gso_send_check(struct sk_buff *skb)
 {
 	const struct iphdr *iph;
@@ -551,10 +556,8 @@ int tcp_v4_gso_send_check(struct sk_buff *skb)
 	th = tcp_hdr(skb);
 
 	th->check = 0;
-	th->check = ~tcp_v4_check(skb->len, iph->saddr, iph->daddr, 0);
-	skb->csum_start = skb_transport_header(skb) - skb->head;
-	skb->csum_offset = offsetof(struct tcphdr, check);
 	skb->ip_summed = CHECKSUM_PARTIAL;
+	__tcp_v4_send_check(skb, iph->saddr, iph->daddr);
 	return 0;
 }
 
@@ -763,13 +766,7 @@ static int tcp_v4_send_synack(struct sock *sk, struct dst_entry *dst,
 	skb = tcp_make_synack(sk, dst, req, rvp);
 
 	if (skb) {
-		struct tcphdr *th = tcp_hdr(skb);
-
-		th->check = tcp_v4_check(skb->len,
-					 ireq->loc_addr,
-					 ireq->rmt_addr,
-					 csum_partial(th, skb->len,
-						      skb->csum));
+		__tcp_v4_send_check(skb, ireq->loc_addr, ireq->rmt_addr);
 
 		err = ip_build_and_send_pkt(skb, sk, ireq->loc_addr,
 					    ireq->rmt_addr,

Re: [PATCH 1/3] tcp: Handle CHECKSUM_PARTIAL for SYNACK packets for IPv4

[Yinghai]

On 04/11/2010 05:15 AM, Herbert Xu wrote:
> tcp: Handle CHECKSUM_PARTIAL for SYNACK packets for IPv4
> 
> This patch moves the common code between tcp_v4_send_check and
> tcp_v4_gso_send_check into a new function __tcp_v4_send_check.
> 
> It then uses the new function in tcp_v4_send_synack so that it
> handles CHECKSUM_PARTIAL properly.
> 
Good, three patches fix the problem.

Thanks

Yinghai

Re: [PATCH 1/3] tcp: Handle CHECKSUM_PARTIAL for SYNACK packets for IPv4

[David Miller]

From: Yinghai <yinghai.lu@oracle.com>
Date: Sun, 11 Apr 2010 14:07:40 -0700

> On 04/11/2010 05:15 AM, Herbert Xu wrote:
>> tcp: Handle CHECKSUM_PARTIAL for SYNACK packets for IPv4
>> 
>> This patch moves the common code between tcp_v4_send_check and
>> tcp_v4_gso_send_check into a new function __tcp_v4_send_check.
>> 
>> It then uses the new function in tcp_v4_send_synack so that it
>> handles CHECKSUM_PARTIAL properly.
>> 
> Good, three patches fix the problem.

Thanks for testing.

[PATCH 2/3] tcp: Handle CHECKSUM_PARTIAL for SYNACK packets for IPv6

[Herbert Xu]

tcp: Handle CHECKSUM_PARTIAL for SYNACK packets for IPv6

This patch moves the common code between tcp_v6_send_check and
tcp_v6_gso_send_check into a new function __tcp_v6_send_check.

It then uses the new function in tcp_v6_send_synack as well as
tcp_v6_send_response so that they handle CHECKSUM_PARTIAL properly.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 net/ipv6/tcp_ipv6.c |   37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index c92ebe8..f84c506 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -75,6 +75,9 @@ static void	tcp_v6_reqsk_send_ack(struct sock *sk, struct sk_buff *skb,
 				      struct request_sock *req);
 
 static int	tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb);
+static void	__tcp_v6_send_check(struct sk_buff *skb,
+				    struct in6_addr *saddr,
+				    struct in6_addr *daddr);
 
 static const struct inet_connection_sock_af_ops ipv6_mapped;
 static const struct inet_connection_sock_af_ops ipv6_specific;
@@ -503,11 +506,7 @@ static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req,
 
 	skb = tcp_make_synack(sk, dst, req, rvp);
 	if (skb) {
-		struct tcphdr *th = tcp_hdr(skb);
-
-		th->check = tcp_v6_check(skb->len,
-					 &treq->loc_addr, &treq->rmt_addr,
-					 csum_partial(th, skb->len, skb->csum));
+		__tcp_v6_send_check(skb, &treq->loc_addr, &treq->rmt_addr);
 
 		ipv6_addr_copy(&fl.fl6_dst, &treq->rmt_addr);
 		err = ip6_xmit(sk, skb, &fl, opt, 0);
@@ -918,22 +917,29 @@ static struct timewait_sock_ops tcp6_timewait_sock_ops = {
 	.twsk_destructor= tcp_twsk_destructor,
 };
 
-static void tcp_v6_send_check(struct sock *sk, int len, struct sk_buff *skb)
+static void __tcp_v6_send_check(struct sk_buff *skb,
+				struct in6_addr *saddr, struct in6_addr *daddr)
 {
-	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct tcphdr *th = tcp_hdr(skb);
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		th->check = ~csum_ipv6_magic(&np->saddr, &np->daddr, len, IPPROTO_TCP,  0);
+		th->check = ~tcp_v6_check(skb->len, saddr, daddr, 0);
 		skb->csum_start = skb_transport_header(skb) - skb->head;
 		skb->csum_offset = offsetof(struct tcphdr, check);
 	} else {
-		th->check = csum_ipv6_magic(&np->saddr, &np->daddr, len, IPPROTO_TCP,
-					    csum_partial(th, th->doff<<2,
-							 skb->csum));
+		th->check = tcp_v6_check(skb->len, saddr, daddr,
+					 csum_partial(th, th->doff << 2,
+						      skb->csum));
 	}
 }
 
+static void tcp_v6_send_check(struct sock *sk, int len, struct sk_buff *skb)
+{
+	struct ipv6_pinfo *np = inet6_sk(sk);
+
+	__tcp_v6_send_check(skb, &np->saddr, &np->daddr);
+}
+
 static int tcp_v6_gso_send_check(struct sk_buff *skb)
 {
 	struct ipv6hdr *ipv6h;
@@ -946,11 +952,8 @@ static int tcp_v6_gso_send_check(struct sk_buff *skb)
 	th = tcp_hdr(skb);
 
 	th->check = 0;
-	th->check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr, skb->len,
-				     IPPROTO_TCP, 0);
-	skb->csum_start = skb_transport_header(skb) - skb->head;
-	skb->csum_offset = offsetof(struct tcphdr, check);
 	skb->ip_summed = CHECKSUM_PARTIAL;
+	__tcp_v6_send_check(skb, &ipv6h->saddr, &ipv6h->daddr);
 	return 0;
 }
 
@@ -1053,9 +1056,7 @@ static void tcp_v6_send_response(struct sk_buff *skb, u32 seq, u32 ack, u32 win,
 	ipv6_addr_copy(&fl.fl6_dst, &ipv6_hdr(skb)->saddr);
 	ipv6_addr_copy(&fl.fl6_src, &ipv6_hdr(skb)->daddr);
 
-	t1->check = csum_ipv6_magic(&fl.fl6_src, &fl.fl6_dst,
-				    tot_len, IPPROTO_TCP,
-				    buff->csum);
+	__tcp_v6_send_check(buff, &fl.fl6_src, &fl.fl6_dst);
 
 	fl.proto = IPPROTO_TCP;
 	fl.oif = inet6_iif(skb);

[PATCH 3/3] inet: Remove unused send_check length argument

[Herbert Xu]

inet: Remove unused send_check length argument

This patch removes the unused length argument from the send_check
function in struct inet_connection_sock_af_ops.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 include/net/inet_connection_sock.h |    3 +--
 include/net/tcp.h                  |    2 +-
 net/dccp/dccp.h                    |    2 +-
 net/dccp/ipv4.c                    |    2 +-
 net/dccp/ipv6.c                    |    3 +--
 net/dccp/output.c                  |    2 +-
 net/ipv4/tcp_ipv4.c                |    2 +-
 net/ipv4/tcp_output.c              |    2 +-
 net/ipv6/tcp_ipv6.c                |    2 +-
 9 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 696d6e4..52c8b8b 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -37,8 +37,7 @@ struct tcp_congestion_ops;
  */
 struct inet_connection_sock_af_ops {
 	int	    (*queue_xmit)(struct sk_buff *skb, int ipfragok);
-	void	    (*send_check)(struct sock *sk, int len,
-				  struct sk_buff *skb);
+	void	    (*send_check)(struct sock *sk, struct sk_buff *skb);
 	int	    (*rebuild_header)(struct sock *sk);
 	int	    (*conn_request)(struct sock *sk, struct sk_buff *skb);
 	struct sock *(*syn_recv_sock)(struct sock *sk, struct sk_buff *skb,
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 75be5a2..70c5159 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -423,7 +423,7 @@ extern u8			*tcp_parse_md5sig_option(struct tcphdr *th);
  *	TCP v4 functions exported for the inet6 API
  */
 
-extern void		       	tcp_v4_send_check(struct sock *sk, int len,
+extern void		       	tcp_v4_send_check(struct sock *sk,
 						  struct sk_buff *skb);
 
 extern int			tcp_v4_conn_request(struct sock *sk,
diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
index 5ef32c2..33035ab 100644
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -223,7 +223,7 @@ static inline void dccp_csum_outgoing(struct sk_buff *skb)
 	skb->csum = skb_checksum(skb, 0, (cov > skb->len)? skb->len : cov, 0);
 }
 
-extern void dccp_v4_send_check(struct sock *sk, int len, struct sk_buff *skb);
+extern void dccp_v4_send_check(struct sock *sk, struct sk_buff *skb);
 
 extern int  dccp_retransmit_skb(struct sock *sk);
 
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 52ffa1c..d9b11ef 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -349,7 +349,7 @@ static inline __sum16 dccp_v4_csum_finish(struct sk_buff *skb,
 	return csum_tcpudp_magic(src, dst, skb->len, IPPROTO_DCCP, skb->csum);
 }
 
-void dccp_v4_send_check(struct sock *sk, int unused, struct sk_buff *skb)
+void dccp_v4_send_check(struct sock *sk, struct sk_buff *skb)
 {
 	const struct inet_sock *inet = inet_sk(sk);
 	struct dccp_hdr *dh = dccp_hdr(skb);
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 3b11e41..ab1ab95 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -60,8 +60,7 @@ static inline __sum16 dccp_v6_csum_finish(struct sk_buff *skb,
 	return csum_ipv6_magic(saddr, daddr, skb->len, IPPROTO_DCCP, skb->csum);
 }
 
-static inline void dccp_v6_send_check(struct sock *sk, int unused_value,
-				      struct sk_buff *skb)
+static inline void dccp_v6_send_check(struct sock *sk, struct sk_buff *skb)
 {
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct dccp_hdr *dh = dccp_hdr(skb);
diff --git a/net/dccp/output.c b/net/dccp/output.c
index fc3f436..b8d98e3 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -129,7 +129,7 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
 			break;
 		}
 
-		icsk->icsk_af_ops->send_check(sk, 0, skb);
+		icsk->icsk_af_ops->send_check(sk, skb);
 
 		if (set_ack)
 			dccp_event_ack_sent(sk);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index aebfd28..a24995c 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -537,7 +537,7 @@ static void __tcp_v4_send_check(struct sk_buff *skb,
 }
 
 /* This routine computes an IPv4 TCP checksum. */
-void tcp_v4_send_check(struct sock *sk, int len, struct sk_buff *skb)
+void tcp_v4_send_check(struct sock *sk, struct sk_buff *skb)
 {
 	struct inet_sock *inet = inet_sk(sk);
 
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 0dda86e..0ae7ce7 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -878,7 +878,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
 	}
 #endif
 
-	icsk->icsk_af_ops->send_check(sk, skb->len, skb);
+	icsk->icsk_af_ops->send_check(sk, skb);
 
 	if (likely(tcb->flags & TCPCB_FLAG_ACK))
 		tcp_event_ack_sent(sk, tcp_skb_pcount(skb));
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index f84c506..b429dfd 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -933,7 +933,7 @@ static void __tcp_v6_send_check(struct sk_buff *skb,
 	}
 }
 
-static void tcp_v6_send_check(struct sock *sk, int len, struct sk_buff *skb)
+static void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
 {
 	struct ipv6_pinfo *np = inet6_sk(sk);