* [PATCH v2] ipv6: Change skb->data before using icmpv6_notify() to propagate redirect @ 2012-12-13 11:21 Duan Jiong 2012-12-13 17:59 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Duan Jiong @ 2012-12-13 11:21 UTC (permalink / raw) To: davem; +Cc: Steffen Klassert, netdev 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(-) 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); } -- 1.7.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ipv6: Change skb->data before using icmpv6_notify() to propagate redirect 2012-12-13 11:21 [PATCH v2] ipv6: Change skb->data before using icmpv6_notify() to propagate redirect Duan Jiong @ 2012-12-13 17:59 ` David Miller 2012-12-13 19:37 ` Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2012-12-13 17:59 UTC (permalink / raw) To: djduanjiong; +Cc: steffen.klassert, netdev 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](... ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ipv6: Change skb->data before using icmpv6_notify() to propagate redirect 2012-12-13 17:59 ` David Miller @ 2012-12-13 19:37 ` Joe Perches 2012-12-13 19:50 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Joe Perches @ 2012-12-13 19:37 UTC (permalink / raw) To: David Miller; +Cc: djduanjiong, steffen.klassert, netdev 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ipv6: Change skb->data before using icmpv6_notify() to propagate redirect 2012-12-13 19:37 ` Joe Perches @ 2012-12-13 19:50 ` David Miller 2012-12-14 4:07 ` [PATCH] ndisc: Use more standard logging styles Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2012-12-13 19:50 UTC (permalink / raw) To: joe; +Cc: djduanjiong, steffen.klassert, netdev 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... ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ndisc: Use more standard logging styles 2012-12-13 19:50 ` David Miller @ 2012-12-14 4:07 ` Joe Perches 2012-12-14 4:11 ` Joe Perches 2012-12-14 4:23 ` [PATCH V2] " Joe Perches 0 siblings, 2 replies; 8+ messages in thread From: Joe Perches @ 2012-12-14 4:07 UTC (permalink / raw) To: David Miller; +Cc: djduanjiong, steffen.klassert, netdev The logging style in this file is "baroque". Convert indirect uses of net_<level>_ratelimited to simply use net_<level>_ratelimited. Add a nd_dbg macro and #define ND_DEBUG for the other generally inactivated logging messages. Make those inactivated messages emit only at KERN_DEBUG instead of other levels. Add "%s: " __func__ to all these nd_dbg macros and remove the embedded function names and prefixes. Signed-off-by: Joe Perches <joe@perches.com> --- net/ipv6/ndisc.c | 139 ++++++++++++++++++++++++------------------------------ 1 files changed, 61 insertions(+), 78 deletions(-) diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index f2a007b..de1c1f2 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -72,14 +72,19 @@ #include <linux/netfilter.h> #include <linux/netfilter_ipv6.h> -/* Set to 3 to get tracing... */ -#define ND_DEBUG 1 +/* #define ND_DEBUG */ -#define ND_PRINTK(val, level, fmt, ...) \ +#ifdef ND_DEBUG +#define nd_dbg(fmt, ...) \ + net_dbg_ratelimited("%s: " fmt, __func__, ##__VA_ARGS__) +#else +#define nd_dbg(fmt, ...) \ do { \ - if (val <= ND_DEBUG) \ - net_##level##_ratelimited(fmt, ##__VA_ARGS__); \ + if (0) \ + net_dbg_ratelimited("%s: " fmt, \ + __func__, ##__VA_ARGS__); \ } while (0) +#endif static u32 ndisc_hash(const void *pkey, const struct net_device *dev, @@ -220,9 +225,8 @@ struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len, case ND_OPT_MTU: case ND_OPT_REDIRECT_HDR: if (ndopts->nd_opt_array[nd_opt->nd_opt_type]) { - ND_PRINTK(2, warn, - "%s: duplicated ND6 option found: type=%d\n", - __func__, nd_opt->nd_opt_type); + nd_dbg("duplicated ND6 option found: type=%d\n", + nd_opt->nd_opt_type); } else { ndopts->nd_opt_array[nd_opt->nd_opt_type] = nd_opt; } @@ -250,11 +254,9 @@ struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len, * to accommodate future extension to the * protocol. */ - ND_PRINTK(2, notice, - "%s: ignored unsupported option; type=%d, len=%d\n", - __func__, - nd_opt->nd_opt_type, - nd_opt->nd_opt_len); + nd_dbg("ignored unsupported option; type=%d, len=%d\n", + nd_opt->nd_opt_type, + nd_opt->nd_opt_len); } } opt_len -= l; @@ -399,8 +401,8 @@ static struct sk_buff *ndisc_build_skb(struct net_device *dev, len + hlen + tlen), 1, &err); if (!skb) { - ND_PRINTK(0, err, "ND: %s failed to allocate an skb, err=%d\n", - __func__, err); + net_err_ratelimited("ND: %s failed to allocate an skb, err=%d\n", + __func__, err); return NULL; } @@ -629,9 +631,8 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb) if ((probes -= neigh->parms->ucast_probes) < 0) { if (!(neigh->nud_state & NUD_VALID)) { - ND_PRINTK(1, dbg, - "%s: trying to ucast probe in NUD_INVALID: %pI6\n", - __func__, target); + net_dbg_ratelimited("%s: trying to ucast probe in NUD_INVALID: %pI6\n", + __func__, target); } ndisc_send_ns(dev, neigh, target, target, saddr); } else if ((probes -= neigh->parms->app_probes) < 0) { @@ -677,7 +678,7 @@ static void ndisc_recv_ns(struct sk_buff *skb) int is_router = -1; if (ipv6_addr_is_multicast(&msg->target)) { - ND_PRINTK(2, warn, "NS: multicast target address\n"); + nd_dbg("multicast target address\n"); return; } @@ -690,20 +691,19 @@ static void ndisc_recv_ns(struct sk_buff *skb) daddr->s6_addr32[1] == htonl(0x00000000) && daddr->s6_addr32[2] == htonl(0x00000001) && daddr->s6_addr [12] == 0xff )) { - ND_PRINTK(2, warn, "NS: bad DAD packet (wrong destination)\n"); + nd_dbg("bad DAD packet (wrong destination)\n"); return; } if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) { - ND_PRINTK(2, warn, "NS: invalid ND options\n"); + nd_dbg("invalid ND options\n"); return; } if (ndopts.nd_opts_src_lladdr) { lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr, dev); if (!lladdr) { - ND_PRINTK(2, warn, - "NS: invalid link-layer address length\n"); + nd_dbg("invalid link-layer address length\n"); return; } @@ -713,8 +713,7 @@ static void ndisc_recv_ns(struct sk_buff *skb) * in the message. */ if (dad) { - ND_PRINTK(2, warn, - "NS: bad DAD packet (link-layer address option)\n"); + nd_dbg("bad DAD packet (link-layer address option)\n"); return; } } @@ -832,30 +831,29 @@ static void ndisc_recv_na(struct sk_buff *skb) struct neighbour *neigh; if (skb->len < sizeof(struct nd_msg)) { - ND_PRINTK(2, warn, "NA: packet too short\n"); + nd_dbg("packet too short\n"); return; } if (ipv6_addr_is_multicast(&msg->target)) { - ND_PRINTK(2, warn, "NA: target address is multicast\n"); + nd_dbg("target address is multicast\n"); return; } if (ipv6_addr_is_multicast(daddr) && msg->icmph.icmp6_solicited) { - ND_PRINTK(2, warn, "NA: solicited NA is multicasted\n"); + nd_dbg("solicited NA is multicasted\n"); return; } if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) { - ND_PRINTK(2, warn, "NS: invalid ND option\n"); + nd_dbg("invalid ND option\n"); return; } if (ndopts.nd_opts_tgt_lladdr) { lladdr = ndisc_opt_addr_data(ndopts.nd_opts_tgt_lladdr, dev); if (!lladdr) { - ND_PRINTK(2, warn, - "NA: invalid link-layer address length\n"); + nd_dbg("invalid link-layer address length\n"); return; } } @@ -876,9 +874,8 @@ static void ndisc_recv_na(struct sk_buff *skb) unsolicited advertisement. */ if (skb->pkt_type != PACKET_LOOPBACK) - ND_PRINTK(1, warn, - "NA: someone advertises our address %pI6 on %s!\n", - &ifp->addr, ifp->idev->dev->name); + net_warn_ratelimited("NA: someone advertises our address %pI6 on %s!\n", + &ifp->addr, ifp->idev->dev->name); in6_ifa_put(ifp); return; } @@ -940,7 +937,7 @@ static void ndisc_recv_rs(struct sk_buff *skb) idev = __in6_dev_get(skb->dev); if (!idev) { - ND_PRINTK(1, err, "RS: can't find in6 device\n"); + net_err_ratelimited("RS: can't find in6 device\n"); return; } @@ -957,7 +954,7 @@ static void ndisc_recv_rs(struct sk_buff *skb) /* Parse ND options */ if (!ndisc_parse_options(rs_msg->opt, ndoptlen, &ndopts)) { - ND_PRINTK(2, notice, "NS: invalid ND option, ignored\n"); + nd_dbg("invalid ND option, ignored\n"); goto out; } @@ -1043,17 +1040,17 @@ static void ndisc_router_discovery(struct sk_buff *skb) optlen = (skb->tail - skb->transport_header) - sizeof(struct ra_msg); if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) { - ND_PRINTK(2, warn, "RA: source address is not link-local\n"); + nd_dbg("source address is not link-local\n"); return; } if (optlen < 0) { - ND_PRINTK(2, warn, "RA: packet too short\n"); + nd_dbg("packet too short\n"); return; } #ifdef CONFIG_IPV6_NDISC_NODETYPE if (skb->ndisc_nodetype == NDISC_NODETYPE_HOST) { - ND_PRINTK(2, warn, "RA: from host or unauthorized router\n"); + nd_dbg("from host or unauthorized router\n"); return; } #endif @@ -1064,13 +1061,13 @@ static void ndisc_router_discovery(struct sk_buff *skb) in6_dev = __in6_dev_get(skb->dev); if (in6_dev == NULL) { - ND_PRINTK(0, err, "RA: can't find inet6 device for %s\n", - skb->dev->name); + net_err_ratelimited("RA: can't find inet6 device for %s\n", + skb->dev->name); return; } if (!ndisc_parse_options(opt, optlen, &ndopts)) { - ND_PRINTK(2, warn, "RA: invalid ND options\n"); + nd_dbg("invalid ND options\n"); return; } @@ -1123,9 +1120,8 @@ static void ndisc_router_discovery(struct sk_buff *skb) if (rt) { neigh = dst_neigh_lookup(&rt->dst, &ipv6_hdr(skb)->saddr); if (!neigh) { - ND_PRINTK(0, err, - "RA: %s got default router without neighbour\n", - __func__); + net_err_ratelimited("RA: %s got default router without neighbour\n", + __func__); ip6_rt_put(rt); return; } @@ -1136,21 +1132,19 @@ static void ndisc_router_discovery(struct sk_buff *skb) } if (rt == NULL && lifetime) { - ND_PRINTK(3, dbg, "RA: adding default router\n"); + nd_dbg("adding default router\n"); rt = rt6_add_dflt_router(&ipv6_hdr(skb)->saddr, skb->dev, pref); if (rt == NULL) { - ND_PRINTK(0, err, - "RA: %s failed to add default route\n", - __func__); + net_err_ratelimited("RA: %s failed to add default route\n", + __func__); return; } neigh = dst_neigh_lookup(&rt->dst, &ipv6_hdr(skb)->saddr); if (neigh == NULL) { - ND_PRINTK(0, err, - "RA: %s got default router without neighbour\n", - __func__); + net_err_ratelimited("RA: %s got default router without neighbour\n", + __func__); ip6_rt_put(rt); return; } @@ -1218,8 +1212,7 @@ skip_linkparms: lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr, skb->dev); if (!lladdr) { - ND_PRINTK(2, warn, - "RA: invalid link-layer address length\n"); + nd_dbg("invalid link-layer address length\n"); goto out; } } @@ -1283,7 +1276,7 @@ skip_routeinfo: mtu = ntohl(n); if (mtu < IPV6_MIN_MTU || mtu > skb->dev->mtu) { - ND_PRINTK(2, warn, "RA: invalid mtu: %d\n", mtu); + nd_dbg("invalid mtu: %d\n", mtu); } else if (in6_dev->cnf.mtu6 != mtu) { in6_dev->cnf.mtu6 = mtu; @@ -1304,7 +1297,7 @@ skip_routeinfo: } if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh) { - ND_PRINTK(2, warn, "RA: invalid RA options\n"); + nd_dbg("invalid RA options\n"); } out: ip6_rt_put(rt); @@ -1318,15 +1311,13 @@ static void ndisc_redirect_rcv(struct sk_buff *skb) switch (skb->ndisc_nodetype) { case NDISC_NODETYPE_HOST: case NDISC_NODETYPE_NODEFAULT: - ND_PRINTK(2, warn, - "Redirect: from host or unauthorized router\n"); + nd_dbg("from host or unauthorized router\n"); return; } #endif if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) { - ND_PRINTK(2, warn, - "Redirect: source address is not link-local\n"); + nd_dbg("source address is not link-local\n"); return; } @@ -1356,15 +1347,13 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) bool ret; if (ipv6_get_lladdr(dev, &saddr_buf, IFA_F_TENTATIVE)) { - ND_PRINTK(2, warn, "Redirect: no link-local address on %s\n", - dev->name); + nd_dbg("no link-local address on %s\n", dev->name); return; } if (!ipv6_addr_equal(&ipv6_hdr(skb)->daddr, target) && ipv6_addr_type(target) != (IPV6_ADDR_UNICAST|IPV6_ADDR_LINKLOCAL)) { - ND_PRINTK(2, warn, - "Redirect: target address is not link-local unicast\n"); + nd_dbg("target address is not link-local unicast\n"); return; } @@ -1383,8 +1372,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) rt = (struct rt6_info *) dst; if (rt->rt6i_flags & RTF_GATEWAY) { - ND_PRINTK(2, warn, - "Redirect: destination is not a neighbour\n"); + nd_dbg("destination is not a neighbour\n"); goto release; } peer = inet_getpeer_v6(net->ipv6.peers, &rt->rt6i_dst.addr, 1); @@ -1397,8 +1385,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) if (dev->addr_len) { struct neighbour *neigh = dst_neigh_lookup(skb_dst(skb), target); if (!neigh) { - ND_PRINTK(2, warn, - "Redirect: no neigh for target address\n"); + nd_dbg("no neigh for target address\n"); goto release; } @@ -1426,9 +1413,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) len + hlen + tlen), 1, &err); if (buff == NULL) { - ND_PRINTK(0, err, - "Redirect: %s failed to allocate an skb, err=%d\n", - __func__, err); + net_err_ratelimited("Redirect: %s failed to allocate an skb, err=%d\n", + __func__, err); goto release; } @@ -1513,14 +1499,12 @@ int ndisc_rcv(struct sk_buff *skb) __skb_push(skb, skb->data - skb_transport_header(skb)); if (ipv6_hdr(skb)->hop_limit != 255) { - ND_PRINTK(2, warn, "NDISC: invalid hop-limit: %d\n", - ipv6_hdr(skb)->hop_limit); + nd_dbg("invalid hop-limit: %d\n", ipv6_hdr(skb)->hop_limit); return 0; } if (msg->icmph.icmp6_code != 0) { - ND_PRINTK(2, warn, "NDISC: invalid ICMPv6 code: %d\n", - msg->icmph.icmp6_code); + nd_dbg("invalid ICMPv6 code: %d\n", msg->icmph.icmp6_code); return 0; } @@ -1648,9 +1632,8 @@ static int __net_init ndisc_net_init(struct net *net) err = inet_ctl_sock_create(&sk, PF_INET6, SOCK_RAW, IPPROTO_ICMPV6, net); if (err < 0) { - ND_PRINTK(0, err, - "NDISC: Failed to initialize the control socket (err %d)\n", - err); + net_err_ratelimited("NDISC: Failed to initialize the control socket (err %d)\n", + err); return err; } ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ndisc: Use more standard logging styles 2012-12-14 4:07 ` [PATCH] ndisc: Use more standard logging styles Joe Perches @ 2012-12-14 4:11 ` Joe Perches 2012-12-14 4:23 ` [PATCH V2] " Joe Perches 1 sibling, 0 replies; 8+ messages in thread From: Joe Perches @ 2012-12-14 4:11 UTC (permalink / raw) To: David Miller; +Cc: djduanjiong, steffen.klassert, netdev On Thu, 2012-12-13 at 20:07 -0800, Joe Perches wrote: > The logging style in this file is "baroque". Sorry, this is whitespace damaged. Please ignore. I'll resubmit via git send-email as V2. (evolution, grumble...) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V2] ndisc: Use more standard logging styles 2012-12-14 4:07 ` [PATCH] ndisc: Use more standard logging styles Joe Perches 2012-12-14 4:11 ` Joe Perches @ 2012-12-14 4:23 ` Joe Perches 2012-12-14 13:58 ` Eric Dumazet 1 sibling, 1 reply; 8+ messages in thread From: Joe Perches @ 2012-12-14 4:23 UTC (permalink / raw) To: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy Cc: Duan Jiong, Steffen Klassert, netdev, linux-kernel The logging style in this file is "baroque". Convert indirect uses of net_<level>_ratelimited to simply use net_<level>_ratelimited. Add a nd_dbg macro and #define ND_DEBUG for the other generally inactivated logging messages. Make those inactivated messages emit only at KERN_DEBUG instead of other levels. Add "%s: " __func__ to all these nd_dbg macros and remove the embedded function names and prefixes. Signed-off-by: Joe Perches <joe@perches.com> --- net/ipv6/ndisc.c | 139 ++++++++++++++++++++++++------------------------------ 1 files changed, 61 insertions(+), 78 deletions(-) diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index f2a007b..de1c1f2 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -72,14 +72,19 @@ #include <linux/netfilter.h> #include <linux/netfilter_ipv6.h> -/* Set to 3 to get tracing... */ -#define ND_DEBUG 1 +/* #define ND_DEBUG */ -#define ND_PRINTK(val, level, fmt, ...) \ +#ifdef ND_DEBUG +#define nd_dbg(fmt, ...) \ + net_dbg_ratelimited("%s: " fmt, __func__, ##__VA_ARGS__) +#else +#define nd_dbg(fmt, ...) \ do { \ - if (val <= ND_DEBUG) \ - net_##level##_ratelimited(fmt, ##__VA_ARGS__); \ + if (0) \ + net_dbg_ratelimited("%s: " fmt, \ + __func__, ##__VA_ARGS__); \ } while (0) +#endif static u32 ndisc_hash(const void *pkey, const struct net_device *dev, @@ -220,9 +225,8 @@ struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len, case ND_OPT_MTU: case ND_OPT_REDIRECT_HDR: if (ndopts->nd_opt_array[nd_opt->nd_opt_type]) { - ND_PRINTK(2, warn, - "%s: duplicated ND6 option found: type=%d\n", - __func__, nd_opt->nd_opt_type); + nd_dbg("duplicated ND6 option found: type=%d\n", + nd_opt->nd_opt_type); } else { ndopts->nd_opt_array[nd_opt->nd_opt_type] = nd_opt; } @@ -250,11 +254,9 @@ struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len, * to accommodate future extension to the * protocol. */ - ND_PRINTK(2, notice, - "%s: ignored unsupported option; type=%d, len=%d\n", - __func__, - nd_opt->nd_opt_type, - nd_opt->nd_opt_len); + nd_dbg("ignored unsupported option; type=%d, len=%d\n", + nd_opt->nd_opt_type, + nd_opt->nd_opt_len); } } opt_len -= l; @@ -399,8 +401,8 @@ static struct sk_buff *ndisc_build_skb(struct net_device *dev, len + hlen + tlen), 1, &err); if (!skb) { - ND_PRINTK(0, err, "ND: %s failed to allocate an skb, err=%d\n", - __func__, err); + net_err_ratelimited("ND: %s failed to allocate an skb, err=%d\n", + __func__, err); return NULL; } @@ -629,9 +631,8 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb) if ((probes -= neigh->parms->ucast_probes) < 0) { if (!(neigh->nud_state & NUD_VALID)) { - ND_PRINTK(1, dbg, - "%s: trying to ucast probe in NUD_INVALID: %pI6\n", - __func__, target); + net_dbg_ratelimited("%s: trying to ucast probe in NUD_INVALID: %pI6\n", + __func__, target); } ndisc_send_ns(dev, neigh, target, target, saddr); } else if ((probes -= neigh->parms->app_probes) < 0) { @@ -677,7 +678,7 @@ static void ndisc_recv_ns(struct sk_buff *skb) int is_router = -1; if (ipv6_addr_is_multicast(&msg->target)) { - ND_PRINTK(2, warn, "NS: multicast target address\n"); + nd_dbg("multicast target address\n"); return; } @@ -690,20 +691,19 @@ static void ndisc_recv_ns(struct sk_buff *skb) daddr->s6_addr32[1] == htonl(0x00000000) && daddr->s6_addr32[2] == htonl(0x00000001) && daddr->s6_addr [12] == 0xff )) { - ND_PRINTK(2, warn, "NS: bad DAD packet (wrong destination)\n"); + nd_dbg("bad DAD packet (wrong destination)\n"); return; } if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) { - ND_PRINTK(2, warn, "NS: invalid ND options\n"); + nd_dbg("invalid ND options\n"); return; } if (ndopts.nd_opts_src_lladdr) { lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr, dev); if (!lladdr) { - ND_PRINTK(2, warn, - "NS: invalid link-layer address length\n"); + nd_dbg("invalid link-layer address length\n"); return; } @@ -713,8 +713,7 @@ static void ndisc_recv_ns(struct sk_buff *skb) * in the message. */ if (dad) { - ND_PRINTK(2, warn, - "NS: bad DAD packet (link-layer address option)\n"); + nd_dbg("bad DAD packet (link-layer address option)\n"); return; } } @@ -832,30 +831,29 @@ static void ndisc_recv_na(struct sk_buff *skb) struct neighbour *neigh; if (skb->len < sizeof(struct nd_msg)) { - ND_PRINTK(2, warn, "NA: packet too short\n"); + nd_dbg("packet too short\n"); return; } if (ipv6_addr_is_multicast(&msg->target)) { - ND_PRINTK(2, warn, "NA: target address is multicast\n"); + nd_dbg("target address is multicast\n"); return; } if (ipv6_addr_is_multicast(daddr) && msg->icmph.icmp6_solicited) { - ND_PRINTK(2, warn, "NA: solicited NA is multicasted\n"); + nd_dbg("solicited NA is multicasted\n"); return; } if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) { - ND_PRINTK(2, warn, "NS: invalid ND option\n"); + nd_dbg("invalid ND option\n"); return; } if (ndopts.nd_opts_tgt_lladdr) { lladdr = ndisc_opt_addr_data(ndopts.nd_opts_tgt_lladdr, dev); if (!lladdr) { - ND_PRINTK(2, warn, - "NA: invalid link-layer address length\n"); + nd_dbg("invalid link-layer address length\n"); return; } } @@ -876,9 +874,8 @@ static void ndisc_recv_na(struct sk_buff *skb) unsolicited advertisement. */ if (skb->pkt_type != PACKET_LOOPBACK) - ND_PRINTK(1, warn, - "NA: someone advertises our address %pI6 on %s!\n", - &ifp->addr, ifp->idev->dev->name); + net_warn_ratelimited("NA: someone advertises our address %pI6 on %s!\n", + &ifp->addr, ifp->idev->dev->name); in6_ifa_put(ifp); return; } @@ -940,7 +937,7 @@ static void ndisc_recv_rs(struct sk_buff *skb) idev = __in6_dev_get(skb->dev); if (!idev) { - ND_PRINTK(1, err, "RS: can't find in6 device\n"); + net_err_ratelimited("RS: can't find in6 device\n"); return; } @@ -957,7 +954,7 @@ static void ndisc_recv_rs(struct sk_buff *skb) /* Parse ND options */ if (!ndisc_parse_options(rs_msg->opt, ndoptlen, &ndopts)) { - ND_PRINTK(2, notice, "NS: invalid ND option, ignored\n"); + nd_dbg("invalid ND option, ignored\n"); goto out; } @@ -1043,17 +1040,17 @@ static void ndisc_router_discovery(struct sk_buff *skb) optlen = (skb->tail - skb->transport_header) - sizeof(struct ra_msg); if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) { - ND_PRINTK(2, warn, "RA: source address is not link-local\n"); + nd_dbg("source address is not link-local\n"); return; } if (optlen < 0) { - ND_PRINTK(2, warn, "RA: packet too short\n"); + nd_dbg("packet too short\n"); return; } #ifdef CONFIG_IPV6_NDISC_NODETYPE if (skb->ndisc_nodetype == NDISC_NODETYPE_HOST) { - ND_PRINTK(2, warn, "RA: from host or unauthorized router\n"); + nd_dbg("from host or unauthorized router\n"); return; } #endif @@ -1064,13 +1061,13 @@ static void ndisc_router_discovery(struct sk_buff *skb) in6_dev = __in6_dev_get(skb->dev); if (in6_dev == NULL) { - ND_PRINTK(0, err, "RA: can't find inet6 device for %s\n", - skb->dev->name); + net_err_ratelimited("RA: can't find inet6 device for %s\n", + skb->dev->name); return; } if (!ndisc_parse_options(opt, optlen, &ndopts)) { - ND_PRINTK(2, warn, "RA: invalid ND options\n"); + nd_dbg("invalid ND options\n"); return; } @@ -1123,9 +1120,8 @@ static void ndisc_router_discovery(struct sk_buff *skb) if (rt) { neigh = dst_neigh_lookup(&rt->dst, &ipv6_hdr(skb)->saddr); if (!neigh) { - ND_PRINTK(0, err, - "RA: %s got default router without neighbour\n", - __func__); + net_err_ratelimited("RA: %s got default router without neighbour\n", + __func__); ip6_rt_put(rt); return; } @@ -1136,21 +1132,19 @@ static void ndisc_router_discovery(struct sk_buff *skb) } if (rt == NULL && lifetime) { - ND_PRINTK(3, dbg, "RA: adding default router\n"); + nd_dbg("adding default router\n"); rt = rt6_add_dflt_router(&ipv6_hdr(skb)->saddr, skb->dev, pref); if (rt == NULL) { - ND_PRINTK(0, err, - "RA: %s failed to add default route\n", - __func__); + net_err_ratelimited("RA: %s failed to add default route\n", + __func__); return; } neigh = dst_neigh_lookup(&rt->dst, &ipv6_hdr(skb)->saddr); if (neigh == NULL) { - ND_PRINTK(0, err, - "RA: %s got default router without neighbour\n", - __func__); + net_err_ratelimited("RA: %s got default router without neighbour\n", + __func__); ip6_rt_put(rt); return; } @@ -1218,8 +1212,7 @@ skip_linkparms: lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr, skb->dev); if (!lladdr) { - ND_PRINTK(2, warn, - "RA: invalid link-layer address length\n"); + nd_dbg("invalid link-layer address length\n"); goto out; } } @@ -1283,7 +1276,7 @@ skip_routeinfo: mtu = ntohl(n); if (mtu < IPV6_MIN_MTU || mtu > skb->dev->mtu) { - ND_PRINTK(2, warn, "RA: invalid mtu: %d\n", mtu); + nd_dbg("invalid mtu: %d\n", mtu); } else if (in6_dev->cnf.mtu6 != mtu) { in6_dev->cnf.mtu6 = mtu; @@ -1304,7 +1297,7 @@ skip_routeinfo: } if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh) { - ND_PRINTK(2, warn, "RA: invalid RA options\n"); + nd_dbg("invalid RA options\n"); } out: ip6_rt_put(rt); @@ -1318,15 +1311,13 @@ static void ndisc_redirect_rcv(struct sk_buff *skb) switch (skb->ndisc_nodetype) { case NDISC_NODETYPE_HOST: case NDISC_NODETYPE_NODEFAULT: - ND_PRINTK(2, warn, - "Redirect: from host or unauthorized router\n"); + nd_dbg("from host or unauthorized router\n"); return; } #endif if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) { - ND_PRINTK(2, warn, - "Redirect: source address is not link-local\n"); + nd_dbg("source address is not link-local\n"); return; } @@ -1356,15 +1347,13 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) bool ret; if (ipv6_get_lladdr(dev, &saddr_buf, IFA_F_TENTATIVE)) { - ND_PRINTK(2, warn, "Redirect: no link-local address on %s\n", - dev->name); + nd_dbg("no link-local address on %s\n", dev->name); return; } if (!ipv6_addr_equal(&ipv6_hdr(skb)->daddr, target) && ipv6_addr_type(target) != (IPV6_ADDR_UNICAST|IPV6_ADDR_LINKLOCAL)) { - ND_PRINTK(2, warn, - "Redirect: target address is not link-local unicast\n"); + nd_dbg("target address is not link-local unicast\n"); return; } @@ -1383,8 +1372,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) rt = (struct rt6_info *) dst; if (rt->rt6i_flags & RTF_GATEWAY) { - ND_PRINTK(2, warn, - "Redirect: destination is not a neighbour\n"); + nd_dbg("destination is not a neighbour\n"); goto release; } peer = inet_getpeer_v6(net->ipv6.peers, &rt->rt6i_dst.addr, 1); @@ -1397,8 +1385,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) if (dev->addr_len) { struct neighbour *neigh = dst_neigh_lookup(skb_dst(skb), target); if (!neigh) { - ND_PRINTK(2, warn, - "Redirect: no neigh for target address\n"); + nd_dbg("no neigh for target address\n"); goto release; } @@ -1426,9 +1413,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) len + hlen + tlen), 1, &err); if (buff == NULL) { - ND_PRINTK(0, err, - "Redirect: %s failed to allocate an skb, err=%d\n", - __func__, err); + net_err_ratelimited("Redirect: %s failed to allocate an skb, err=%d\n", + __func__, err); goto release; } @@ -1513,14 +1499,12 @@ int ndisc_rcv(struct sk_buff *skb) __skb_push(skb, skb->data - skb_transport_header(skb)); if (ipv6_hdr(skb)->hop_limit != 255) { - ND_PRINTK(2, warn, "NDISC: invalid hop-limit: %d\n", - ipv6_hdr(skb)->hop_limit); + nd_dbg("invalid hop-limit: %d\n", ipv6_hdr(skb)->hop_limit); return 0; } if (msg->icmph.icmp6_code != 0) { - ND_PRINTK(2, warn, "NDISC: invalid ICMPv6 code: %d\n", - msg->icmph.icmp6_code); + nd_dbg("invalid ICMPv6 code: %d\n", msg->icmph.icmp6_code); return 0; } @@ -1648,9 +1632,8 @@ static int __net_init ndisc_net_init(struct net *net) err = inet_ctl_sock_create(&sk, PF_INET6, SOCK_RAW, IPPROTO_ICMPV6, net); if (err < 0) { - ND_PRINTK(0, err, - "NDISC: Failed to initialize the control socket (err %d)\n", - err); + net_err_ratelimited("NDISC: Failed to initialize the control socket (err %d)\n", + err); return err; } -- 1.7.8.112.g3fd21 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] ndisc: Use more standard logging styles 2012-12-14 4:23 ` [PATCH V2] " Joe Perches @ 2012-12-14 13:58 ` Eric Dumazet 0 siblings, 0 replies; 8+ messages in thread From: Eric Dumazet @ 2012-12-14 13:58 UTC (permalink / raw) To: Joe Perches Cc: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Duan Jiong, Steffen Klassert, netdev, linux-kernel On Thu, 2012-12-13 at 20:23 -0800, Joe Perches wrote: > The logging style in this file is "baroque". > > Convert indirect uses of net_<level>_ratelimited to > simply use net_<level>_ratelimited. > > Add a nd_dbg macro and #define ND_DEBUG for the other > generally inactivated logging messages. > > Make those inactivated messages emit only at KERN_DEBUG > instead of other levels. Add "%s: " __func__ to all > these nd_dbg macros and remove the embedded function > names and prefixes. > > Signed-off-by: Joe Perches <joe@perches.com> It seems its not a bug fix, so why are you sending this now ? Is it so hard to understand the merge window rule ? net-next is currently closed. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-12-14 13:58 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-13 11:21 [PATCH v2] ipv6: Change skb->data before using icmpv6_notify() to propagate redirect Duan Jiong 2012-12-13 17:59 ` David Miller 2012-12-13 19:37 ` Joe Perches 2012-12-13 19:50 ` David Miller 2012-12-14 4:07 ` [PATCH] ndisc: Use more standard logging styles Joe Perches 2012-12-14 4:11 ` Joe Perches 2012-12-14 4:23 ` [PATCH V2] " Joe Perches 2012-12-14 13:58 ` Eric Dumazet
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).