* [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering @ 2008-10-09 9:35 Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) Ilpo Järvinen 2008-10-09 21:37 ` [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering David Miller 0 siblings, 2 replies; 13+ messages in thread From: Ilpo Järvinen @ 2008-10-09 9:35 UTC (permalink / raw) To: David Miller; +Cc: netdev, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) 2008-10-09 9:35 [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering Ilpo Järvinen @ 2008-10-09 9:35 ` Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset Ilpo Järvinen 2008-10-09 9:52 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) YOSHIFUJI Hideaki / 吉藤英明 2008-10-09 21:37 ` [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering David Miller 1 sibling, 2 replies; 13+ messages in thread From: Ilpo Järvinen @ 2008-10-09 9:35 UTC (permalink / raw) To: David Miller; +Cc: netdev, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset 2008-10-09 9:35 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) Ilpo Järvinen @ 2008-10-09 9:35 ` Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 4/4] tcpv6: combine tcp_v6_send_(reset|ack) Ilpo Järvinen 2008-10-09 21:42 ` [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset David Miller 2008-10-09 9:52 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) YOSHIFUJI Hideaki / 吉藤英明 1 sibling, 2 replies; 13+ messages in thread From: Ilpo Järvinen @ 2008-10-09 9:35 UTC (permalink / raw) To: David Miller; +Cc: netdev, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] tcpv6: combine tcp_v6_send_(reset|ack) 2008-10-09 9:35 ` [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset Ilpo Järvinen @ 2008-10-09 9:35 ` Ilpo Järvinen 2008-10-09 21:42 ` David Miller 2008-10-09 21:42 ` [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset David Miller 1 sibling, 1 reply; 13+ messages in thread From: Ilpo Järvinen @ 2008-10-09 9:35 UTC (permalink / raw) To: David Miller; +Cc: netdev, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] tcpv6: combine tcp_v6_send_(reset|ack) 2008-10-09 9:35 ` [PATCH 4/4] tcpv6: combine tcp_v6_send_(reset|ack) Ilpo Järvinen @ 2008-10-09 21:42 ` David Miller 0 siblings, 0 replies; 13+ messages in thread From: David Miller @ 2008-10-09 21:42 UTC (permalink / raw) To: ilpo.jarvinen; +Cc: netdev 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! ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset 2008-10-09 9:35 ` [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 4/4] tcpv6: combine tcp_v6_send_(reset|ack) Ilpo Järvinen @ 2008-10-09 21:42 ` David Miller 1 sibling, 0 replies; 13+ messages in thread From: David Miller @ 2008-10-09 21:42 UTC (permalink / raw) To: ilpo.jarvinen; +Cc: netdev 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) 2008-10-09 9:35 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset Ilpo Järvinen @ 2008-10-09 9:52 ` YOSHIFUJI Hideaki / 吉藤英明 2008-10-09 9:58 ` Ilpo Järvinen 1 sibling, 1 reply; 13+ messages in thread From: YOSHIFUJI Hideaki / 吉藤英明 @ 2008-10-09 9:52 UTC (permalink / raw) To: ilpo.jarvinen; +Cc: davem, netdev, yoshfuji 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) 2008-10-09 9:52 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) YOSHIFUJI Hideaki / 吉藤英明 @ 2008-10-09 9:58 ` Ilpo Järvinen 2008-10-09 14:16 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 13+ messages in thread From: Ilpo Järvinen @ 2008-10-09 9:58 UTC (permalink / raw) To: YOSHIFUJI Hideaki / 吉藤英明; +Cc: David Miller, Netdev [-- 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) 2008-10-09 9:58 ` Ilpo Järvinen @ 2008-10-09 14:16 ` Arnaldo Carvalho de Melo 2008-10-09 21:38 ` David Miller 0 siblings, 1 reply; 13+ messages in thread From: Arnaldo Carvalho de Melo @ 2008-10-09 14:16 UTC (permalink / raw) To: Ilpo Järvinen Cc: YOSHIFUJI Hideaki / 吉藤英明, David Miller, Netdev 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) 2008-10-09 14:16 ` Arnaldo Carvalho de Melo @ 2008-10-09 21:38 ` David Miller 2008-10-09 21:40 ` David Miller 0 siblings, 1 reply; 13+ messages in thread From: David Miller @ 2008-10-09 21:38 UTC (permalink / raw) To: acme; +Cc: ilpo.jarvinen, yoshfuji, netdev 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) 2008-10-09 21:38 ` David Miller @ 2008-10-09 21:40 ` David Miller 2008-10-10 12:00 ` Ilpo Järvinen 0 siblings, 1 reply; 13+ messages in thread From: David Miller @ 2008-10-09 21:40 UTC (permalink / raw) To: acme; +Cc: ilpo.jarvinen, yoshfuji, netdev 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) 2008-10-09 21:40 ` David Miller @ 2008-10-10 12:00 ` Ilpo Järvinen 0 siblings, 0 replies; 13+ messages in thread From: Ilpo Järvinen @ 2008-10-10 12:00 UTC (permalink / raw) To: David Miller; +Cc: Arnaldo Carvalho de Melo, yoshfuji, Netdev [-- 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering 2008-10-09 9:35 [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) Ilpo Järvinen @ 2008-10-09 21:37 ` David Miller 1 sibling, 0 replies; 13+ messages in thread From: David Miller @ 2008-10-09 21:37 UTC (permalink / raw) To: ilpo.jarvinen; +Cc: netdev 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2008-10-10 12:00 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-10-09 9:35 [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset Ilpo Järvinen 2008-10-09 9:35 ` [PATCH 4/4] tcpv6: combine tcp_v6_send_(reset|ack) Ilpo Järvinen 2008-10-09 21:42 ` David Miller 2008-10-09 21:42 ` [PATCH 3/4] tcpv6: convert opt[] -> topt in tcp_v6_send_reset David Miller 2008-10-09 9:52 ` [PATCH 2/4] tcpv6: trivial formatting changes to send_(ack|reset) YOSHIFUJI Hideaki / 吉藤英明 2008-10-09 9:58 ` Ilpo Järvinen 2008-10-09 14:16 ` Arnaldo Carvalho de Melo 2008-10-09 21:38 ` David Miller 2008-10-09 21:40 ` David Miller 2008-10-10 12:00 ` Ilpo Järvinen 2008-10-09 21:37 ` [PATCH 1/4] tcpv[46]: fix md5 pseudoheader address field ordering 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).