[PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering

[PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering

[Ilpo Järvinen]

Maybe it's just me but I guess those md5 people made a mess
out of it by having *_md5_hash_* to use daddr, saddr order
instead of the one that is natural (and equal to what csum
functions use). For the segment were sending, the original
addresses are reversed so buff's saddr == skb's daddr and
vice-versa.

Maybe I can finally proceed with unification of some code
after fixing it first... :-)

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_ipv4.c |    4 ++--
 net/ipv6/tcp_ipv6.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index ba46769..5c8fa7f 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -583,8 +583,8 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
 		rep.th.doff = arg.iov[0].iov_len / 4;
 
 		tcp_v4_md5_hash_hdr((__u8 *) &rep.opt[1],
-				     key, ip_hdr(skb)->daddr,
-				     ip_hdr(skb)->saddr, &rep.th);
+				     key, ip_hdr(skb)->saddr,
+				     ip_hdr(skb)->daddr, &rep.th);
 	}
 #endif
 	arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9c9ecf4..2084f5a 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1006,8 +1006,8 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 			       (TCPOPT_MD5SIG << 8) |
 			       TCPOLEN_MD5SIG);
 		tcp_v6_md5_hash_hdr((__u8 *)&opt[1], key,
-				    &ipv6_hdr(skb)->daddr,
-				    &ipv6_hdr(skb)->saddr, t1);
+				    &ipv6_hdr(skb)->saddr,
+				    &ipv6_hdr(skb)->daddr, t1);
 	}
 #endif
 
-- 
1.5.2.2

[PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset)

[Ilpo Järvinen]

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv6/tcp_ipv6.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 2084f5a..3f268a3 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -948,7 +948,7 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 	struct flowi fl;
 	struct net *net = dev_net(skb->dst->dev);
 	struct sock *ctl_sk = net->ipv6.tcp_sk;
-	unsigned int tot_len = sizeof(*th);
+	unsigned int tot_len = sizeof(struct tcphdr);
 #ifdef CONFIG_TCP_MD5SIG
 	struct tcp_md5sig_key *key;
 #endif
@@ -1032,7 +1032,6 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 	 * namespace
 	 */
 	if (!ip6_dst_lookup(ctl_sk, &buff->dst, &fl)) {
-
 		if (xfrm_lookup(&buff->dst, &fl, NULL, 0) >= 0) {
 			ip6_xmit(ctl_sk, buff, &fl, NULL, 0);
 			TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
@@ -1069,13 +1068,13 @@ static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack, u32 win, u32
 
 	skb_reserve(buff, MAX_HEADER + sizeof(struct ipv6hdr) + tot_len);
 
-	t1 = (struct tcphdr *) skb_push(buff,tot_len);
+	t1 = (struct tcphdr *) skb_push(buff, tot_len);
 
 	/* Swap the send and the receive. */
 	memset(t1, 0, sizeof(*t1));
 	t1->dest = th->source;
 	t1->source = th->dest;
-	t1->doff = tot_len/4;
+	t1->doff = tot_len / 4;
 	t1->seq = htonl(seq);
 	t1->ack_seq = htonl(ack);
 	t1->ack = 1;
-- 
1.5.2.2

[PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset

[Ilpo Järvinen]

after this I get:

$ diff-funcs tcp_v6_send_reset tcp_ipv6.c tcp_ipv6.c tcp_v6_send_ack
 --- tcp_ipv6.c:tcp_v6_send_reset()
 +++ tcp_ipv6.c:tcp_v6_send_ack()
@@ -1,4 +1,5 @@
-static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
+static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack, u32 win,
u32 ts,
+                           struct tcp_md5sig_key *key)
 {
        struct tcphdr *th = tcp_hdr(skb), *t1;
        struct sk_buff *buff;
@@ -7,31 +8,14 @@
        struct sock *ctl_sk = net->ipv6.tcp_sk;
        unsigned int tot_len = sizeof(struct tcphdr);
        __be32 *topt;
-#ifdef CONFIG_TCP_MD5SIG
-       struct tcp_md5sig_key *key;
-#endif
-
-       if (th->rst)
-               return;
-
-       if (!ipv6_unicast_destination(skb))
-               return;

+       if (ts)
+               tot_len += TCPOLEN_TSTAMP_ALIGNED;
 #ifdef CONFIG_TCP_MD5SIG
-       if (sk)
-               key = tcp_v6_md5_do_lookup(sk, &ipv6_hdr(skb)->daddr);
-       else
-               key = NULL;
-
        if (key)
                tot_len += TCPOLEN_MD5SIG_ALIGNED;
 #endif

-       /*
-        * We need to grab some memory, and put together an RST,
-        * and then put it into the queue to be sent.
-        */
-
        buff = alloc_skb(MAX_HEADER + sizeof(struct ipv6hdr) + tot_len,
                         GFP_ATOMIC);
        if (buff == NULL)
@@ -46,18 +30,20 @@
        t1->dest = th->source;
        t1->source = th->dest;
        t1->doff = tot_len / 4;
-       t1->rst = 1;
-
-       if(th->ack) {
-               t1->seq = th->ack_seq;
-       } else {
-               t1->ack = 1;
-               t1->ack_seq = htonl(ntohl(th->seq) + th->syn + th->fin
-                                   + skb->len - (th->doff<<2));
-       }
+       t1->seq = htonl(seq);
+       t1->ack_seq = htonl(ack);
+       t1->ack = 1;
+       t1->window = htons(win);

        topt = (__be32 *)(t1 + 1);

+       if (ts) {
+               *topt++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
+                               (TCPOPT_TIMESTAMP << 8) |
TCPOLEN_TIMESTAMP);
+               *topt++ = htonl(tcp_time_stamp);
+               *topt++ = htonl(ts);
+       }
+
 #ifdef CONFIG_TCP_MD5SIG
        if (key) {
                *topt++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
@@ -84,15 +70,10 @@
        fl.fl_ip_sport = t1->source;
        security_skb_classify_flow(skb, &fl);

-       /* Pass a socket to ip6_dst_lookup either it is for RST
-        * Underlying function will use this to retrieve the network
-        * namespace
-        */
        if (!ip6_dst_lookup(ctl_sk, &buff->dst, &fl)) {
                if (xfrm_lookup(&buff->dst, &fl, NULL, 0) >= 0) {
                        ip6_xmit(ctl_sk, buff, &fl, NULL, 0);
                        TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
-                       TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
                        return;
                }
        }


...which starts to be trivial to combine.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv6/tcp_ipv6.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3f268a3..082724b 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -949,6 +949,7 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 	struct net *net = dev_net(skb->dst->dev);
 	struct sock *ctl_sk = net->ipv6.tcp_sk;
 	unsigned int tot_len = sizeof(struct tcphdr);
+	__be32 *topt;
 #ifdef CONFIG_TCP_MD5SIG
 	struct tcp_md5sig_key *key;
 #endif
@@ -998,14 +999,13 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 				    + skb->len - (th->doff<<2));
 	}
 
+	topt = (__be32 *)(t1 + 1);
+
 #ifdef CONFIG_TCP_MD5SIG
 	if (key) {
-		__be32 *opt = (__be32*)(t1 + 1);
-		opt[0] = htonl((TCPOPT_NOP << 24) |
-			       (TCPOPT_NOP << 16) |
-			       (TCPOPT_MD5SIG << 8) |
-			       TCPOLEN_MD5SIG);
-		tcp_v6_md5_hash_hdr((__u8 *)&opt[1], key,
+		*topt++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
+				(TCPOPT_MD5SIG << 8) | TCPOLEN_MD5SIG);
+		tcp_v6_md5_hash_hdr((__u8 *)topt, key,
 				    &ipv6_hdr(skb)->saddr,
 				    &ipv6_hdr(skb)->daddr, t1);
 	}
-- 
1.5.2.2

[PATCH 4/4] tcpv6: combine tcp_v6_send_(reset|ack)

[Ilpo Järvinen]

$ codiff tcp_ipv6.o.old tcp_ipv6.o.new
net/ipv6/tcp_ipv6.c:
  tcp_v6_md5_hash_hdr | -144
  tcp_v6_send_ack     | -585
  tcp_v6_send_reset   | -540
 3 functions changed, 1269 bytes removed, diff: -1269

net/ipv6/tcp_ipv6.c:
  tcp_v6_send_response | +791
 1 function changed, 791 bytes added, diff: +791

tcp_ipv6.o.new:
 4 functions changed, 791 bytes added, 1269 bytes removed, diff: -478

I choose to leave the reset related netns comment in place (not
the one that is killed) as I cannot understand its English so
it's a bit hard for me to evaluate its usefulness :-).

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv6/tcp_ipv6.c |  139 +++++++++++++++------------------------------------
 1 files changed, 40 insertions(+), 99 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 082724b..9204caf 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -941,7 +941,8 @@ static int tcp_v6_gso_send_check(struct sk_buff *skb)
 	return 0;
 }
 
-static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
+static void tcp_v6_send_response(struct sk_buff *skb, u32 seq, u32 ack, u32 win,
+				 u32 ts, struct tcp_md5sig_key *key, int rst)
 {
 	struct tcphdr *th = tcp_hdr(skb), *t1;
 	struct sk_buff *buff;
@@ -950,31 +951,14 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 	struct sock *ctl_sk = net->ipv6.tcp_sk;
 	unsigned int tot_len = sizeof(struct tcphdr);
 	__be32 *topt;
-#ifdef CONFIG_TCP_MD5SIG
-	struct tcp_md5sig_key *key;
-#endif
-
-	if (th->rst)
-		return;
-
-	if (!ipv6_unicast_destination(skb))
-		return;
 
+	if (ts)
+		tot_len += TCPOLEN_TSTAMP_ALIGNED;
 #ifdef CONFIG_TCP_MD5SIG
-	if (sk)
-		key = tcp_v6_md5_do_lookup(sk, &ipv6_hdr(skb)->daddr);
-	else
-		key = NULL;
-
 	if (key)
 		tot_len += TCPOLEN_MD5SIG_ALIGNED;
 #endif
 
-	/*
-	 * We need to grab some memory, and put together an RST,
-	 * and then put it into the queue to be sent.
-	 */
-
 	buff = alloc_skb(MAX_HEADER + sizeof(struct ipv6hdr) + tot_len,
 			 GFP_ATOMIC);
 	if (buff == NULL)
@@ -989,18 +973,21 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 	t1->dest = th->source;
 	t1->source = th->dest;
 	t1->doff = tot_len / 4;
-	t1->rst = 1;
-
-	if(th->ack) {
-		t1->seq = th->ack_seq;
-	} else {
-		t1->ack = 1;
-		t1->ack_seq = htonl(ntohl(th->seq) + th->syn + th->fin
-				    + skb->len - (th->doff<<2));
-	}
+	t1->seq = htonl(seq);
+	t1->ack_seq = htonl(ack);
+	t1->ack = !rst || !th->ack;
+	t1->rst = rst;
+	t1->window = htons(win);
 
 	topt = (__be32 *)(t1 + 1);
 
+	if (ts) {
+		*topt++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
+				(TCPOPT_TIMESTAMP << 8) | TCPOLEN_TIMESTAMP);
+		*topt++ = htonl(tcp_time_stamp);
+		*topt++ = htonl(ts);
+	}
+
 #ifdef CONFIG_TCP_MD5SIG
 	if (key) {
 		*topt++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
@@ -1035,7 +1022,8 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 		if (xfrm_lookup(&buff->dst, &fl, NULL, 0) >= 0) {
 			ip6_xmit(ctl_sk, buff, &fl, NULL, 0);
 			TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
-			TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
+			if (rst)
+				TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
 			return;
 		}
 	}
@@ -1043,87 +1031,40 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 	kfree_skb(buff);
 }
 
-static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack, u32 win, u32 ts,
-			    struct tcp_md5sig_key *key)
+static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
 {
-	struct tcphdr *th = tcp_hdr(skb), *t1;
-	struct sk_buff *buff;
-	struct flowi fl;
-	struct net *net = dev_net(skb->dst->dev);
-	struct sock *ctl_sk = net->ipv6.tcp_sk;
-	unsigned int tot_len = sizeof(struct tcphdr);
-	__be32 *topt;
-
-	if (ts)
-		tot_len += TCPOLEN_TSTAMP_ALIGNED;
+	struct tcphdr *th = tcp_hdr(skb);
+	u32 seq = 0, ack_seq = 0;
 #ifdef CONFIG_TCP_MD5SIG
-	if (key)
-		tot_len += TCPOLEN_MD5SIG_ALIGNED;
+	struct tcp_md5sig_key *key;
 #endif
 
-	buff = alloc_skb(MAX_HEADER + sizeof(struct ipv6hdr) + tot_len,
-			 GFP_ATOMIC);
-	if (buff == NULL)
+	if (th->rst)
 		return;
 
-	skb_reserve(buff, MAX_HEADER + sizeof(struct ipv6hdr) + tot_len);
-
-	t1 = (struct tcphdr *) skb_push(buff, tot_len);
-
-	/* Swap the send and the receive. */
-	memset(t1, 0, sizeof(*t1));
-	t1->dest = th->source;
-	t1->source = th->dest;
-	t1->doff = tot_len / 4;
-	t1->seq = htonl(seq);
-	t1->ack_seq = htonl(ack);
-	t1->ack = 1;
-	t1->window = htons(win);
-
-	topt = (__be32 *)(t1 + 1);
-
-	if (ts) {
-		*topt++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
-				(TCPOPT_TIMESTAMP << 8) | TCPOLEN_TIMESTAMP);
-		*topt++ = htonl(tcp_time_stamp);
-		*topt++ = htonl(ts);
-	}
+	if (!ipv6_unicast_destination(skb))
+		return;
 
 #ifdef CONFIG_TCP_MD5SIG
-	if (key) {
-		*topt++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
-				(TCPOPT_MD5SIG << 8) | TCPOLEN_MD5SIG);
-		tcp_v6_md5_hash_hdr((__u8 *)topt, key,
-				    &ipv6_hdr(skb)->saddr,
-				    &ipv6_hdr(skb)->daddr, t1);
-	}
+	if (sk)
+		key = tcp_v6_md5_do_lookup(sk, &ipv6_hdr(skb)->daddr);
+	else
+		key = NULL;
 #endif
 
-	buff->csum = csum_partial((char *)t1, tot_len, 0);
-
-	memset(&fl, 0, sizeof(fl));
-	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);
-
-	fl.proto = IPPROTO_TCP;
-	fl.oif = inet6_iif(skb);
-	fl.fl_ip_dport = t1->dest;
-	fl.fl_ip_sport = t1->source;
-	security_skb_classify_flow(skb, &fl);
+	if (th->ack)
+		seq = ntohl(th->ack_seq);
+	else
+		ack_seq = ntohl(th->seq) + th->syn + th->fin + skb->len -
+			  (th->doff << 2);
 
-	if (!ip6_dst_lookup(ctl_sk, &buff->dst, &fl)) {
-		if (xfrm_lookup(&buff->dst, &fl, NULL, 0) >= 0) {
-			ip6_xmit(ctl_sk, buff, &fl, NULL, 0);
-			TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
-			return;
-		}
-	}
+	tcp_v6_send_response(skb, seq, ack_seq, 0, 0, key, 1);
+}
 
-	kfree_skb(buff);
+static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack, u32 win, u32 ts,
+			    struct tcp_md5sig_key *key)
+{
+	tcp_v6_send_response(skb, seq, ack, win, ts, key, 0);
 }
 
 static void tcp_v6_timewait_ack(struct sock *sk, struct sk_buff *skb)
-- 
1.5.2.2

Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset)

[YOSHIFUJI Hideaki / 吉藤英明]

In article <1223544937-12595-2-git-send-email-ilpo.jarvinen@helsinki.fi> (at Thu,  9 Oct 2008 12:35:35 +0300), "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> says:

> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> ---
>  net/ipv6/tcp_ipv6.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 2084f5a..3f268a3 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -948,7 +948,7 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
>  	struct flowi fl;
>  	struct net *net = dev_net(skb->dst->dev);
>  	struct sock *ctl_sk = net->ipv6.tcp_sk;
> -	unsigned int tot_len = sizeof(*th);
> +	unsigned int tot_len = sizeof(struct tcphdr);
>  #ifdef CONFIG_TCP_MD5SIG
>  	struct tcp_md5sig_key *key;
>  #endif

I usually prefer sizeof(var) (or sizeof(*ptr)) over sizeof(type).
Could you justify this change?

--yoshfuji

Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset)

[Ilpo Järvinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1247 bytes --]

On Thu, 9 Oct 2008, YOSHIFUJI Hideaki / ???? wrote:

> In article <1223544937-12595-2-git-send-email-ilpo.jarvinen@helsinki.fi> (at Thu,  9 Oct 2008 12:35:35 +0300), "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> says:
> 
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> > ---
> >  net/ipv6/tcp_ipv6.c |    7 +++----
> >  1 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index 2084f5a..3f268a3 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -948,7 +948,7 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
> >  	struct flowi fl;
> >  	struct net *net = dev_net(skb->dst->dev);
> >  	struct sock *ctl_sk = net->ipv6.tcp_sk;
> > -	unsigned int tot_len = sizeof(*th);
> > +	unsigned int tot_len = sizeof(struct tcphdr);
> >  #ifdef CONFIG_TCP_MD5SIG
> >  	struct tcp_md5sig_key *key;
> >  #endif
> 
> I usually prefer sizeof(var) (or sizeof(*ptr)) over sizeof(type).
> Could you justify this change?

My point was to make those two functions as equal as possible before merge 
so that it's more easier to track what happens at the merge, both 
versions pre-existed anyway. Sure I don't have much opinion either way.

-- 
 i.

Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset)

[Arnaldo Carvalho de Melo]

Em Thu, Oct 09, 2008 at 12:58:07PM +0300, Ilpo Järvinen escreveu:
> On Thu, 9 Oct 2008, YOSHIFUJI Hideaki / ???? wrote:
> 
> > In article <1223544937-12595-2-git-send-email-ilpo.jarvinen@helsinki.fi> (at Thu,  9 Oct 2008 12:35:35 +0300), "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> says:
> > 
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> > > ---
> > >  net/ipv6/tcp_ipv6.c |    7 +++----
> > >  1 files changed, 3 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > > index 2084f5a..3f268a3 100644
> > > --- a/net/ipv6/tcp_ipv6.c
> > > +++ b/net/ipv6/tcp_ipv6.c
> > > @@ -948,7 +948,7 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
> > >  	struct flowi fl;
> > >  	struct net *net = dev_net(skb->dst->dev);
> > >  	struct sock *ctl_sk = net->ipv6.tcp_sk;
> > > -	unsigned int tot_len = sizeof(*th);
> > > +	unsigned int tot_len = sizeof(struct tcphdr);
> > >  #ifdef CONFIG_TCP_MD5SIG
> > >  	struct tcp_md5sig_key *key;
> > >  #endif
> > 
> > I usually prefer sizeof(var) (or sizeof(*ptr)) over sizeof(type).
> > Could you justify this change?
> 
> My point was to make those two functions as equal as possible before merge 
> so that it's more easier to track what happens at the merge, both 
> versions pre-existed anyway. Sure I don't have much opinion either way.

I agree with Yoshifuji, albeit it is unlikely that the type of that
variable will change 8)

- Arnaldo

Re: [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering

[David Miller]

From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu,  9 Oct 2008 12:35:34 +0300

> Maybe it's just me but I guess those md5 people made a mess
> out of it by having *_md5_hash_* to use daddr, saddr order
> instead of the one that is natural (and equal to what csum
> functions use). For the segment were sending, the original
> addresses are reversed so buff's saddr == skb's daddr and
> vice-versa.
> 
> Maybe I can finally proceed with unification of some code
> after fixing it first... :-)
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

Applied, thanks.

Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset)

[David Miller]

From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Thu, 9 Oct 2008 11:16:58 -0300

> Em Thu, Oct 09, 2008 at 12:58:07PM +0300, Ilpo Järvinen escreveu:
> > On Thu, 9 Oct 2008, YOSHIFUJI Hideaki / ???? wrote:
> > > I usually prefer sizeof(var) (or sizeof(*ptr)) over sizeof(type).
> > > Could you justify this change?
> > 
> > My point was to make those two functions as equal as possible before merge 
> > so that it's more easier to track what happens at the merge, both 
> > versions pre-existed anyway. Sure I don't have much opinion either way.
> 
> I agree with Yoshifuji, albeit it is unlikely that the type of that
> variable will change 8)

I agree as well and therefore left out the first hunk of this
patch when adding it to net-next-2.6

Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset)

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 09 Oct 2008 14:38:59 -0700 (PDT)

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Thu, 9 Oct 2008 11:16:58 -0300
> 
> > Em Thu, Oct 09, 2008 at 12:58:07PM +0300, Ilpo Järvinen escreveu:
> > > On Thu, 9 Oct 2008, YOSHIFUJI Hideaki / ???? wrote:
> > > > I usually prefer sizeof(var) (or sizeof(*ptr)) over sizeof(type).
> > > > Could you justify this change?
> > > 
> > > My point was to make those two functions as equal as possible before merge 
> > > so that it's more easier to track what happens at the merge, both 
> > > versions pre-existed anyway. Sure I don't have much opinion either way.
> > 
> > I agree with Yoshifuji, albeit it is unlikely that the type of that
> > variable will change 8)
> 
> I agree as well and therefore left out the first hunk of this
> patch when adding it to net-next-2.6

Nevermind, I take this back.

If Ilpo's purpose is to make the two pieces of code as
similar as possible for maintainence purposes and potential
code consolidation, the sizeof() change does make sense.

So I'll apply his patches as they were.

Re: [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset

[David Miller]

From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu,  9 Oct 2008 12:35:36 +0300

> after this I get:
> 
> $ diff-funcs tcp_v6_send_reset tcp_ipv6.c tcp_ipv6.c tcp_v6_send_ack
>  --- tcp_ipv6.c:tcp_v6_send_reset()
>  +++ tcp_ipv6.c:tcp_v6_send_ack()

Applied.

Re: [PATCH 4/4] tcpv6: combine tcp_v6_send_(reset|ack)

[David Miller]

From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu,  9 Oct 2008 12:35:37 +0300

> $ codiff tcp_ipv6.o.old tcp_ipv6.o.new
> net/ipv6/tcp_ipv6.c:
>   tcp_v6_md5_hash_hdr | -144
>   tcp_v6_send_ack     | -585
>   tcp_v6_send_reset   | -540
>  3 functions changed, 1269 bytes removed, diff: -1269
> 
> net/ipv6/tcp_ipv6.c:
>   tcp_v6_send_response | +791
>  1 function changed, 791 bytes added, diff: +791
> 
> tcp_ipv6.o.new:
>  4 functions changed, 791 bytes added, 1269 bytes removed, diff: -478
> 
> I choose to leave the reset related netns comment in place (not
> the one that is killed) as I cannot understand its English so
> it's a bit hard for me to evaluate its usefulness :-).
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

Applied, thanks!

Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset)

[Ilpo Järvinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 979 bytes --]

On Thu, 9 Oct 2008, David Miller wrote:

> From: David Miller <davem@davemloft.net>
> Date: Thu, 09 Oct 2008 14:38:59 -0700 (PDT)
> 
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Date: Thu, 9 Oct 2008 11:16:58 -0300
> > 
> > > Em Thu, Oct 09, 2008 at 12:58:07PM +0300, Ilpo Järvinen escreveu:
> > > > On Thu, 9 Oct 2008, YOSHIFUJI Hideaki / ???? wrote:
> > > > > I usually prefer sizeof(var) (or sizeof(*ptr)) over sizeof(type).
> > > > > Could you justify this change?
> > > > 
> > > > My point was to make those two functions as equal as possible before merge 
> > > > so that it's more easier to track what happens at the merge, both 
> > > > versions pre-existed anyway. Sure I don't have much opinion either way.
> > > 
> > > I agree with Yoshifuji, albeit it is unlikely that the type of that
> > > variable will change 8)
> > 
> > I agree as well

Ok. I keep this in mind in future (both versions pre-existed the
choice I made was just arbitary)... :-)

-- 
 i.