Message ID | 50C9BA41.1010507@gmail.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
From: Duan Jiong <djduanjiong@gmail.com> Date: Thu, 13 Dec 2012 19:21:37 +0800 > + if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) { > + ND_PRINTK(2, warn, > + "Redirect: invalid ND options\n"); Do not add more uses of such baroque kernel logging mechanisms. > + if (!ndopts.nd_opts_rh) { > + return; > + } Single statement basic blocks should never be surrounded by curly braces, they just waste lines. > + hdr = (u8 *)ndopts.nd_opts_rh; (u8 *)[SPACE]ndopts... > + if(!pskb_pull(skb, hdr - skb_transport_header(skb))) { if[SPACE](... -- 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
On Thu, 2012-12-13 at 12:59 -0500, David Miller wrote: > > + hdr = (u8 *)ndopts.nd_opts_rh; > > (u8 *)[SPACE]ndopts... Really? checkpatch bleats a --strict message: CHECK: No space is necessary after a cast #5: FILE: t.c:5: + + bar = *(int *) foo; It's 2/1 no space v space in drivers/net and net. $ git grep -E "\*\) " drivers/net net | wc -l 4293 $ git grep -E "\*\)[^ ;]" drivers/net net | wc -l 8261 There are many false positives in the first case, not too many for the second. I was a bit surprised about how many of the first case existed. -- 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
From: Joe Perches <joe@perches.com> Date: Thu, 13 Dec 2012 11:37:12 -0800 > On Thu, 2012-12-13 at 12:59 -0500, David Miller wrote: >> > + hdr = (u8 *)ndopts.nd_opts_rh; >> >> (u8 *)[SPACE]ndopts... > > Really? > > checkpatch bleats a --strict message: > > CHECK: No space is necessary after a cast > #5: FILE: t.c:5: > + > + bar = *(int *) foo; Ho hum, I'm actually ambivalent about this so... -- 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
diff --git a/include/net/ndisc.h b/include/net/ndisc.h index 980d263..6b305d7 100644 --- a/include/net/ndisc.h +++ b/include/net/ndisc.h @@ -78,6 +78,13 @@ struct ra_msg { __be32 retrans_timer; }; +struct rd_msg { + struct icmp6hdr icmph; + struct in6_addr target; + struct in6_addr dest; + __u8 opt[0]; +}; + struct nd_opt_hdr { __u8 nd_opt_type; __u8 nd_opt_len; diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index 2edce30..03deabc 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -1333,6 +1333,12 @@ out: static void ndisc_redirect_rcv(struct sk_buff *skb) { + u8 *hdr; + struct ndisc_options ndopts; + struct rd_msg *msg = (struct rd_msg *)skb_transport_header(skb); + u32 ndoptlen = skb->tail - (skb->transport_header + + offsetof(struct rd_msg, opt)); + #ifdef CONFIG_IPV6_NDISC_NODETYPE switch (skb->ndisc_nodetype) { case NDISC_NODETYPE_HOST: @@ -1349,6 +1355,22 @@ static void ndisc_redirect_rcv(struct sk_buff *skb) return; } + if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) { + ND_PRINTK(2, warn, + "Redirect: invalid ND options\n"); + return; + } + + if (!ndopts.nd_opts_rh) { + return; + } + + hdr = (u8 *)ndopts.nd_opts_rh; + hdr += 8; + if(!pskb_pull(skb, hdr - skb_transport_header(skb))) { + return; + } + icmpv6_notify(skb, NDISC_REDIRECT, 0, 0); }
In function ndisc_redirect_rcv(), the skb->data points to the transport header, but function icmpv6_notify() need the skb->data points to the inner IP packet. So before using icmpv6_notify() to propagate redirect, change skb->data to point the inner IP packet that triggered the sending of the Redirect, and introduce struct rd_msg to make it easy. Many thanks to Steffen Klassert. Signed-off-by: Duan Jiong <djduanjiong@gmail.com> --- include/net/ndisc.h | 7 +++++++ net/ipv6/ndisc.c | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-)