* [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
@ 2014-07-29 8:19 Duan Jiong
2014-07-30 13:45 ` Hannes Frederic Sowa
2014-07-31 9:54 ` [PATCH v2] " Duan Jiong
0 siblings, 2 replies; 10+ messages in thread
From: Duan Jiong @ 2014-07-29 8:19 UTC (permalink / raw)
To: David Miller; +Cc: Hannes Frederic Sowa, eric.dumazet, netdev
When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
and icmpv6_notify() do some valid checks on packet's length, but then some
protocols check packet's length redaudantly. So remove those duplicated
statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
function icmp_socket_deliver() and icmpv6_notify() respectively.
In addition, add missed counter in udp6/udplite6 when socket is NULL.
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
net/ipv4/icmp.c | 4 +++-
net/ipv4/tcp_ipv4.c | 5 -----
net/ipv6/icmp.c | 11 ++++++++---
net/ipv6/udp.c | 8 ++++++--
net/sctp/input.c | 5 -----
5 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 42b7bcf..092400e 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -663,8 +663,10 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
/* Checkin full IP header plus 8 bytes of protocol to
* avoid additional coding at protocol handlers.
*/
- if (!pskb_may_pull(skb, iph->ihl * 4 + 8))
+ if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) {
+ ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
return;
+ }
raw_icmp_error(skb, protocol, info);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 77cccda..715cf6b 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -342,11 +342,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
int err;
struct net *net = dev_net(icmp_skb->dev);
- if (icmp_skb->len < (iph->ihl << 2) + 8) {
- ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
- return;
- }
-
sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest,
iph->saddr, th->source, inet_iif(icmp_skb));
if (!sk) {
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index f6c84a6..8ca3cc0 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -626,9 +626,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
int inner_offset;
__be16 frag_off;
u8 nexthdr;
+ struct net *net = dev_net(skb->dev);
if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
- return;
+ goto out;
nexthdr = ((struct ipv6hdr *)skb->data)->nexthdr;
if (ipv6_ext_hdr(nexthdr)) {
@@ -636,14 +637,14 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
inner_offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr),
&nexthdr, &frag_off);
if (inner_offset<0)
- return;
+ goto out;
} else {
inner_offset = sizeof(struct ipv6hdr);
}
/* Checkin header including 8 bytes of inner protocol header. */
if (!pskb_may_pull(skb, inner_offset+8))
- return;
+ goto out;
/* BUGGG_FUTURE: we should try to parse exthdrs in this packet.
Without this we will not able f.e. to make source routed
@@ -659,6 +660,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
rcu_read_unlock();
raw6_icmp_error(skb, nexthdr, type, code, inner_offset, info);
+ return;
+
+out:
+ ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
}
/*
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 7092ff7..f13d297 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -534,11 +534,15 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
struct udphdr *uh = (struct udphdr*)(skb->data+offset);
struct sock *sk;
int err;
+ struct net *net = dev_net(skb->dev);
- sk = __udp6_lib_lookup(dev_net(skb->dev), daddr, uh->dest,
+ sk = __udp6_lib_lookup(net, daddr, uh->dest,
saddr, uh->source, inet6_iif(skb), udptable);
- if (sk == NULL)
+ if (sk == NULL) {
+ ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev),
+ ICMP6_MIB_INERRORS);
return;
+ }
if (type == ICMPV6_PKT_TOOBIG) {
if (!ip6_sk_accept_pmtu(sk))
diff --git a/net/sctp/input.c b/net/sctp/input.c
index f2e2cbd..c1b9912 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -575,11 +575,6 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info)
int err;
struct net *net = dev_net(skb->dev);
- if (skb->len < ihlen + 8) {
- ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
- return;
- }
-
/* Fix up skb to look at the embedded net header. */
saveip = skb->network_header;
savesctp = skb->transport_header;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-29 8:19 [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS Duan Jiong
@ 2014-07-30 13:45 ` Hannes Frederic Sowa
2014-07-31 2:58 ` Duan Jiong
2014-07-31 9:54 ` [PATCH v2] " Duan Jiong
1 sibling, 1 reply; 10+ messages in thread
From: Hannes Frederic Sowa @ 2014-07-30 13:45 UTC (permalink / raw)
To: Duan Jiong; +Cc: David Miller, eric.dumazet, netdev
On Di, 2014-07-29 at 16:19 +0800, Duan Jiong wrote:
> When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
> and icmpv6_notify() do some valid checks on packet's length, but then some
> protocols check packet's length redaudantly. So remove those duplicated
> statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
> function icmp_socket_deliver() and icmpv6_notify() respectively.
>
> In addition, add missed counter in udp6/udplite6 when socket is NULL.
I am ok with adding the error counters, but...
> ---
> net/ipv4/icmp.c | 4 +++-
> net/ipv4/tcp_ipv4.c | 5 -----
> net/ipv6/icmp.c | 11 ++++++++---
> net/ipv6/udp.c | 8 ++++++--
> net/sctp/input.c | 5 -----
> 5 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 42b7bcf..092400e 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -663,8 +663,10 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
> /* Checkin full IP header plus 8 bytes of protocol to
> * avoid additional coding at protocol handlers.
> */
> - if (!pskb_may_pull(skb, iph->ihl * 4 + 8))
> + if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) {
> + ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
> return;
> + }
>
> raw_icmp_error(skb, protocol, info);
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 77cccda..715cf6b 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -342,11 +342,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
> int err;
> struct net *net = dev_net(icmp_skb->dev);
>
> - if (icmp_skb->len < (iph->ihl << 2) + 8) {
> - ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
> - return;
> - }
> -
> sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest,
> iph->saddr, th->source, inet_iif(icmp_skb));
> if (!sk) {
> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index f6c84a6..8ca3cc0 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -626,9 +626,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> int inner_offset;
> __be16 frag_off;
> u8 nexthdr;
> + struct net *net = dev_net(skb->dev);
>
> if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
> - return;
> + goto out;
>
> nexthdr = ((struct ipv6hdr *)skb->data)->nexthdr;
> if (ipv6_ext_hdr(nexthdr)) {
> @@ -636,14 +637,14 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> inner_offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr),
> &nexthdr, &frag_off);
> if (inner_offset<0)
> - return;
> + goto out;
> } else {
> inner_offset = sizeof(struct ipv6hdr);
> }
>
> /* Checkin header including 8 bytes of inner protocol header. */
> if (!pskb_may_pull(skb, inner_offset+8))
> - return;
> + goto out;
>
> /* BUGGG_FUTURE: we should try to parse exthdrs in this packet.
> Without this we will not able f.e. to make source routed
> @@ -659,6 +660,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> rcu_read_unlock();
>
> raw6_icmp_error(skb, nexthdr, type, code, inner_offset, info);
> + return;
> +
> +out:
> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
... please don't use __in6_dev_get in BH without rcu, but inet6_iif. You
risk a lockdep error otherwise. You don't have rtnl locked and no RCU
read lock.
> }
>
> /*
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 7092ff7..f13d297 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -534,11 +534,15 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> struct udphdr *uh = (struct udphdr*)(skb->data+offset);
> struct sock *sk;
> int err;
> + struct net *net = dev_net(skb->dev);
>
> - sk = __udp6_lib_lookup(dev_net(skb->dev), daddr, uh->dest,
> + sk = __udp6_lib_lookup(net, daddr, uh->dest,
> saddr, uh->source, inet6_iif(skb), udptable);
> - if (sk == NULL)
> + if (sk == NULL) {
> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev),
> + ICMP6_MIB_INERRORS);
It is not really necessary here, as you like.
Bye,
Hannes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-30 13:45 ` Hannes Frederic Sowa
@ 2014-07-31 2:58 ` Duan Jiong
2014-07-31 7:52 ` Hannes Frederic Sowa
0 siblings, 1 reply; 10+ messages in thread
From: Duan Jiong @ 2014-07-31 2:58 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: David Miller, eric.dumazet, netdev
On 07/30/2014 09:45 PM, Hannes Frederic Sowa wrote:
> On Di, 2014-07-29 at 16:19 +0800, Duan Jiong wrote:
>> When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
>> and icmpv6_notify() do some valid checks on packet's length, but then some
>> protocols check packet's length redaudantly. So remove those duplicated
>> statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
>> function icmp_socket_deliver() and icmpv6_notify() respectively.
>>
>> In addition, add missed counter in udp6/udplite6 when socket is NULL.
>
> I am ok with adding the error counters, but...
>
>> ---
>> net/ipv4/icmp.c | 4 +++-
>> net/ipv4/tcp_ipv4.c | 5 -----
>> net/ipv6/icmp.c | 11 ++++++++---
>> net/ipv6/udp.c | 8 ++++++--
>> net/sctp/input.c | 5 -----
>> 5 files changed, 17 insertions(+), 16 deletions(-)
>>
>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>> index 42b7bcf..092400e 100644
>> --- a/net/ipv4/icmp.c
>> +++ b/net/ipv4/icmp.c
>> @@ -663,8 +663,10 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
>> /* Checkin full IP header plus 8 bytes of protocol to
>> * avoid additional coding at protocol handlers.
>> */
>> - if (!pskb_may_pull(skb, iph->ihl * 4 + 8))
>> + if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) {
>> + ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
>> return;
>> + }
>>
>> raw_icmp_error(skb, protocol, info);
>>
>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>> index 77cccda..715cf6b 100644
>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -342,11 +342,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
>> int err;
>> struct net *net = dev_net(icmp_skb->dev);
>>
>> - if (icmp_skb->len < (iph->ihl << 2) + 8) {
>> - ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
>> - return;
>> - }
>> -
>> sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest,
>> iph->saddr, th->source, inet_iif(icmp_skb));
>> if (!sk) {
>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
>> index f6c84a6..8ca3cc0 100644
>> --- a/net/ipv6/icmp.c
>> +++ b/net/ipv6/icmp.c
>> @@ -626,9 +626,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
>> int inner_offset;
>> __be16 frag_off;
>> u8 nexthdr;
>> + struct net *net = dev_net(skb->dev);
>>
>> if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
>> - return;
>> + goto out;
>>
>> nexthdr = ((struct ipv6hdr *)skb->data)->nexthdr;
>> if (ipv6_ext_hdr(nexthdr)) {
>> @@ -636,14 +637,14 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
>> inner_offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr),
>> &nexthdr, &frag_off);
>> if (inner_offset<0)
>> - return;
>> + goto out;
>> } else {
>> inner_offset = sizeof(struct ipv6hdr);
>> }
>>
>> /* Checkin header including 8 bytes of inner protocol header. */
>> if (!pskb_may_pull(skb, inner_offset+8))
>> - return;
>> + goto out;
>>
>> /* BUGGG_FUTURE: we should try to parse exthdrs in this packet.
>> Without this we will not able f.e. to make source routed
>> @@ -659,6 +660,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
>> rcu_read_unlock();
>>
>> raw6_icmp_error(skb, nexthdr, type, code, inner_offset, info);
>> + return;
>> +
>> +out:
>> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
>
> ... please don't use __in6_dev_get in BH without rcu, but inet6_iif. You
> risk a lockdep error otherwise. You don't have rtnl locked and no RCU
> read lock.
>
May be we can use in6_dev_get instead of __in6_dev_get, and
use in6_dev_put to release the reference later.
>> }
>>
>> /*
>> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
>> index 7092ff7..f13d297 100644
>> --- a/net/ipv6/udp.c
>> +++ b/net/ipv6/udp.c
>> @@ -534,11 +534,15 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>> struct udphdr *uh = (struct udphdr*)(skb->data+offset);
>> struct sock *sk;
>> int err;
>> + struct net *net = dev_net(skb->dev);
>>
>> - sk = __udp6_lib_lookup(dev_net(skb->dev), daddr, uh->dest,
>> + sk = __udp6_lib_lookup(net, daddr, uh->dest,
>> saddr, uh->source, inet6_iif(skb), udptable);
>> - if (sk == NULL)
>> + if (sk == NULL) {
>> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev),
>> + ICMP6_MIB_INERRORS);
>
> It is not really necessary here, as you like.
>
I add it just because i saw the same statements in dccp6/sctp6/tcp6 error
handler.
Thanks,
Duan
> Bye,
> Hannes
>
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-31 2:58 ` Duan Jiong
@ 2014-07-31 7:52 ` Hannes Frederic Sowa
2014-07-31 8:34 ` Duan Jiong
0 siblings, 1 reply; 10+ messages in thread
From: Hannes Frederic Sowa @ 2014-07-31 7:52 UTC (permalink / raw)
To: Duan Jiong; +Cc: David Miller, eric.dumazet, netdev
On Do, 2014-07-31 at 10:58 +0800, Duan Jiong wrote:
> On 07/30/2014 09:45 PM, Hannes Frederic Sowa wrote:
> > On Di, 2014-07-29 at 16:19 +0800, Duan Jiong wrote:
> >> When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
> >> and icmpv6_notify() do some valid checks on packet's length, but then some
> >> protocols check packet's length redaudantly. So remove those duplicated
> >> statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
> >> function icmp_socket_deliver() and icmpv6_notify() respectively.
> >>
> >> In addition, add missed counter in udp6/udplite6 when socket is NULL.
> >
> > I am ok with adding the error counters, but...
> >
> >> ---
> >> net/ipv4/icmp.c | 4 +++-
> >> net/ipv4/tcp_ipv4.c | 5 -----
> >> net/ipv6/icmp.c | 11 ++++++++---
> >> net/ipv6/udp.c | 8 ++++++--
> >> net/sctp/input.c | 5 -----
> >> 5 files changed, 17 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> >> index 42b7bcf..092400e 100644
> >> --- a/net/ipv4/icmp.c
> >> +++ b/net/ipv4/icmp.c
> >> @@ -663,8 +663,10 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
> >> /* Checkin full IP header plus 8 bytes of protocol to
> >> * avoid additional coding at protocol handlers.
> >> */
> >> - if (!pskb_may_pull(skb, iph->ihl * 4 + 8))
> >> + if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) {
> >> + ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
> >> return;
> >> + }
> >>
> >> raw_icmp_error(skb, protocol, info);
> >>
> >> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> >> index 77cccda..715cf6b 100644
> >> --- a/net/ipv4/tcp_ipv4.c
> >> +++ b/net/ipv4/tcp_ipv4.c
> >> @@ -342,11 +342,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
> >> int err;
> >> struct net *net = dev_net(icmp_skb->dev);
> >>
> >> - if (icmp_skb->len < (iph->ihl << 2) + 8) {
> >> - ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
> >> - return;
> >> - }
> >> -
> >> sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest,
> >> iph->saddr, th->source, inet_iif(icmp_skb));
> >> if (!sk) {
> >> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> >> index f6c84a6..8ca3cc0 100644
> >> --- a/net/ipv6/icmp.c
> >> +++ b/net/ipv6/icmp.c
> >> @@ -626,9 +626,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> >> int inner_offset;
> >> __be16 frag_off;
> >> u8 nexthdr;
> >> + struct net *net = dev_net(skb->dev);
> >>
> >> if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
> >> - return;
> >> + goto out;
> >>
> >> nexthdr = ((struct ipv6hdr *)skb->data)->nexthdr;
> >> if (ipv6_ext_hdr(nexthdr)) {
> >> @@ -636,14 +637,14 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> >> inner_offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr),
> >> &nexthdr, &frag_off);
> >> if (inner_offset<0)
> >> - return;
> >> + goto out;
> >> } else {
> >> inner_offset = sizeof(struct ipv6hdr);
> >> }
> >>
> >> /* Checkin header including 8 bytes of inner protocol header. */
> >> if (!pskb_may_pull(skb, inner_offset+8))
> >> - return;
> >> + goto out;
> >>
> >> /* BUGGG_FUTURE: we should try to parse exthdrs in this packet.
> >> Without this we will not able f.e. to make source routed
> >> @@ -659,6 +660,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> >> rcu_read_unlock();
> >>
> >> raw6_icmp_error(skb, nexthdr, type, code, inner_offset, info);
> >> + return;
> >> +
> >> +out:
> >> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
> >
> > ... please don't use __in6_dev_get in BH without rcu, but inet6_iif. You
> > risk a lockdep error otherwise. You don't have rtnl locked and no RCU
> > read lock.
> >
>
> May be we can use in6_dev_get instead of __in6_dev_get, and
> use in6_dev_put to release the reference later.
Sure, you can. You can also protect the update of the counter with an
rcu read lock, but you won't find anything easier than inet6_iif. ;)
> >> }
> >>
> >> /*
> >> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> >> index 7092ff7..f13d297 100644
> >> --- a/net/ipv6/udp.c
> >> +++ b/net/ipv6/udp.c
> >> @@ -534,11 +534,15 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> >> struct udphdr *uh = (struct udphdr*)(skb->data+offset);
> >> struct sock *sk;
> >> int err;
> >> + struct net *net = dev_net(skb->dev);
> >>
> >> - sk = __udp6_lib_lookup(dev_net(skb->dev), daddr, uh->dest,
> >> + sk = __udp6_lib_lookup(net, daddr, uh->dest,
> >> saddr, uh->source, inet6_iif(skb), udptable);
> >> - if (sk == NULL)
> >> + if (sk == NULL) {
> >> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev),
> >> + ICMP6_MIB_INERRORS);
> >
> > It is not really necessary here, as you like.
> >
>
> I add it just because i saw the same statements in dccp6/sctp6/tcp6 error
> handler.
Counting errors here is fine. And because you are have rcu read lock the
__in6_dev_get is also fine.
Bye,
Hannes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-31 7:52 ` Hannes Frederic Sowa
@ 2014-07-31 8:34 ` Duan Jiong
2014-07-31 9:10 ` Hannes Frederic Sowa
0 siblings, 1 reply; 10+ messages in thread
From: Duan Jiong @ 2014-07-31 8:34 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: David Miller, eric.dumazet, netdev
On 07/31/2014 03:52 PM, Hannes Frederic Sowa wrote:
> On Do, 2014-07-31 at 10:58 +0800, Duan Jiong wrote:
>> On 07/30/2014 09:45 PM, Hannes Frederic Sowa wrote:
>>> On Di, 2014-07-29 at 16:19 +0800, Duan Jiong wrote:
>>>> When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
>>>> and icmpv6_notify() do some valid checks on packet's length, but then some
>>>> protocols check packet's length redaudantly. So remove those duplicated
>>>> statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
>>>> function icmp_socket_deliver() and icmpv6_notify() respectively.
>>>>
>>>> In addition, add missed counter in udp6/udplite6 when socket is NULL.
>>>
>>> I am ok with adding the error counters, but...
>>>
>>>> ---
>>>> net/ipv4/icmp.c | 4 +++-
>>>> net/ipv4/tcp_ipv4.c | 5 -----
>>>> net/ipv6/icmp.c | 11 ++++++++---
>>>> net/ipv6/udp.c | 8 ++++++--
>>>> net/sctp/input.c | 5 -----
>>>> 5 files changed, 17 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>>>> index 42b7bcf..092400e 100644
>>>> --- a/net/ipv4/icmp.c
>>>> +++ b/net/ipv4/icmp.c
>>>> @@ -663,8 +663,10 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
>>>> /* Checkin full IP header plus 8 bytes of protocol to
>>>> * avoid additional coding at protocol handlers.
>>>> */
>>>> - if (!pskb_may_pull(skb, iph->ihl * 4 + 8))
>>>> + if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) {
>>>> + ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
>>>> return;
>>>> + }
>>>>
>>>> raw_icmp_error(skb, protocol, info);
>>>>
>>>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>>>> index 77cccda..715cf6b 100644
>>>> --- a/net/ipv4/tcp_ipv4.c
>>>> +++ b/net/ipv4/tcp_ipv4.c
>>>> @@ -342,11 +342,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
>>>> int err;
>>>> struct net *net = dev_net(icmp_skb->dev);
>>>>
>>>> - if (icmp_skb->len < (iph->ihl << 2) + 8) {
>>>> - ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
>>>> - return;
>>>> - }
>>>> -
>>>> sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest,
>>>> iph->saddr, th->source, inet_iif(icmp_skb));
>>>> if (!sk) {
>>>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
>>>> index f6c84a6..8ca3cc0 100644
>>>> --- a/net/ipv6/icmp.c
>>>> +++ b/net/ipv6/icmp.c
>>>> @@ -626,9 +626,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
>>>> int inner_offset;
>>>> __be16 frag_off;
>>>> u8 nexthdr;
>>>> + struct net *net = dev_net(skb->dev);
>>>>
>>>> if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
>>>> - return;
>>>> + goto out;
>>>>
>>>> nexthdr = ((struct ipv6hdr *)skb->data)->nexthdr;
>>>> if (ipv6_ext_hdr(nexthdr)) {
>>>> @@ -636,14 +637,14 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
>>>> inner_offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr),
>>>> &nexthdr, &frag_off);
>>>> if (inner_offset<0)
>>>> - return;
>>>> + goto out;
>>>> } else {
>>>> inner_offset = sizeof(struct ipv6hdr);
>>>> }
>>>>
>>>> /* Checkin header including 8 bytes of inner protocol header. */
>>>> if (!pskb_may_pull(skb, inner_offset+8))
>>>> - return;
>>>> + goto out;
>>>>
>>>> /* BUGGG_FUTURE: we should try to parse exthdrs in this packet.
>>>> Without this we will not able f.e. to make source routed
>>>> @@ -659,6 +660,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
>>>> rcu_read_unlock();
>>>>
>>>> raw6_icmp_error(skb, nexthdr, type, code, inner_offset, info);
>>>> + return;
>>>> +
>>>> +out:
>>>> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
>>>
>>> ... please don't use __in6_dev_get in BH without rcu, but inet6_iif. You
>>> risk a lockdep error otherwise. You don't have rtnl locked and no RCU
>>> read lock.
>>>
>>
>> May be we can use in6_dev_get instead of __in6_dev_get, and
>> use in6_dev_put to release the reference later.
>
> Sure, you can. You can also protect the update of the counter with an
> rcu read lock, but you won't find anything easier than inet6_iif. ;)
Are you sure using inet6_iif is fine?
function inet6_iif() return int, but we need struct inet6_dev *.
Thanks,
Duan
>
>>>> }
>>>>
>>>> /*
>>>> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
>>>> index 7092ff7..f13d297 100644
>>>> --- a/net/ipv6/udp.c
>>>> +++ b/net/ipv6/udp.c
>>>> @@ -534,11 +534,15 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>>>> struct udphdr *uh = (struct udphdr*)(skb->data+offset);
>>>> struct sock *sk;
>>>> int err;
>>>> + struct net *net = dev_net(skb->dev);
>>>>
>>>> - sk = __udp6_lib_lookup(dev_net(skb->dev), daddr, uh->dest,
>>>> + sk = __udp6_lib_lookup(net, daddr, uh->dest,
>>>> saddr, uh->source, inet6_iif(skb), udptable);
>>>> - if (sk == NULL)
>>>> + if (sk == NULL) {
>>>> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev),
>>>> + ICMP6_MIB_INERRORS);
>>>
>>> It is not really necessary here, as you like.
>>>
>>
>> I add it just because i saw the same statements in dccp6/sctp6/tcp6 error
>> handler.
>
> Counting errors here is fine. And because you are have rcu read lock the
> __in6_dev_get is also fine.
>
> Bye,
> Hannes
>
>
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-31 8:34 ` Duan Jiong
@ 2014-07-31 9:10 ` Hannes Frederic Sowa
0 siblings, 0 replies; 10+ messages in thread
From: Hannes Frederic Sowa @ 2014-07-31 9:10 UTC (permalink / raw)
To: Duan Jiong; +Cc: David Miller, eric.dumazet, netdev
On Do, 2014-07-31 at 16:34 +0800, Duan Jiong wrote:
> On 07/31/2014 03:52 PM, Hannes Frederic Sowa wrote:
> > On Do, 2014-07-31 at 10:58 +0800, Duan Jiong wrote:
> >> On 07/30/2014 09:45 PM, Hannes Frederic Sowa wrote:
> >>> On Di, 2014-07-29 at 16:19 +0800, Duan Jiong wrote:
> >>>> When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
> >>>> and icmpv6_notify() do some valid checks on packet's length, but then some
> >>>> protocols check packet's length redaudantly. So remove those duplicated
> >>>> statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
> >>>> function icmp_socket_deliver() and icmpv6_notify() respectively.
> >>>>
> >>>> In addition, add missed counter in udp6/udplite6 when socket is NULL.
> >>>
> >>> I am ok with adding the error counters, but...
> >>>
> >>>> ---
> >>>> net/ipv4/icmp.c | 4 +++-
> >>>> net/ipv4/tcp_ipv4.c | 5 -----
> >>>> net/ipv6/icmp.c | 11 ++++++++---
> >>>> net/ipv6/udp.c | 8 ++++++--
> >>>> net/sctp/input.c | 5 -----
> >>>> 5 files changed, 17 insertions(+), 16 deletions(-)
> >>>>
> >>>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> >>>> index 42b7bcf..092400e 100644
> >>>> --- a/net/ipv4/icmp.c
> >>>> +++ b/net/ipv4/icmp.c
> >>>> @@ -663,8 +663,10 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
> >>>> /* Checkin full IP header plus 8 bytes of protocol to
> >>>> * avoid additional coding at protocol handlers.
> >>>> */
> >>>> - if (!pskb_may_pull(skb, iph->ihl * 4 + 8))
> >>>> + if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) {
> >>>> + ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
> >>>> return;
> >>>> + }
> >>>>
> >>>> raw_icmp_error(skb, protocol, info);
> >>>>
> >>>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> >>>> index 77cccda..715cf6b 100644
> >>>> --- a/net/ipv4/tcp_ipv4.c
> >>>> +++ b/net/ipv4/tcp_ipv4.c
> >>>> @@ -342,11 +342,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
> >>>> int err;
> >>>> struct net *net = dev_net(icmp_skb->dev);
> >>>>
> >>>> - if (icmp_skb->len < (iph->ihl << 2) + 8) {
> >>>> - ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
> >>>> - return;
> >>>> - }
> >>>> -
> >>>> sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest,
> >>>> iph->saddr, th->source, inet_iif(icmp_skb));
> >>>> if (!sk) {
> >>>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> >>>> index f6c84a6..8ca3cc0 100644
> >>>> --- a/net/ipv6/icmp.c
> >>>> +++ b/net/ipv6/icmp.c
> >>>> @@ -626,9 +626,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> >>>> int inner_offset;
> >>>> __be16 frag_off;
> >>>> u8 nexthdr;
> >>>> + struct net *net = dev_net(skb->dev);
> >>>>
> >>>> if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
> >>>> - return;
> >>>> + goto out;
> >>>>
> >>>> nexthdr = ((struct ipv6hdr *)skb->data)->nexthdr;
> >>>> if (ipv6_ext_hdr(nexthdr)) {
> >>>> @@ -636,14 +637,14 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> >>>> inner_offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr),
> >>>> &nexthdr, &frag_off);
> >>>> if (inner_offset<0)
> >>>> - return;
> >>>> + goto out;
> >>>> } else {
> >>>> inner_offset = sizeof(struct ipv6hdr);
> >>>> }
> >>>>
> >>>> /* Checkin header including 8 bytes of inner protocol header. */
> >>>> if (!pskb_may_pull(skb, inner_offset+8))
> >>>> - return;
> >>>> + goto out;
> >>>>
> >>>> /* BUGGG_FUTURE: we should try to parse exthdrs in this packet.
> >>>> Without this we will not able f.e. to make source routed
> >>>> @@ -659,6 +660,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
> >>>> rcu_read_unlock();
> >>>>
> >>>> raw6_icmp_error(skb, nexthdr, type, code, inner_offset, info);
> >>>> + return;
> >>>> +
> >>>> +out:
> >>>> + ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
> >>>
> >>> ... please don't use __in6_dev_get in BH without rcu, but inet6_iif. You
> >>> risk a lockdep error otherwise. You don't have rtnl locked and no RCU
> >>> read lock.
> >>>
> >>
> >> May be we can use in6_dev_get instead of __in6_dev_get, and
> >> use in6_dev_put to release the reference later.
> >
> > Sure, you can. You can also protect the update of the counter with an
> > rcu read lock, but you won't find anything easier than inet6_iif. ;)
>
> Are you sure using inet6_iif is fine?
>
> function inet6_iif() return int, but we need struct inet6_dev *.
No, sorry, you are correct, my fault. ;)
Somehow I thought *6_INC_STATS just take an interface index.
I would just propose to rcu lock this section like:
rcu_read_lock();
ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
rcu_read_unlock();
If you look at the invocations of icmpv6_notify, they are all rcu read
lock protected, so your initial patch already was perfectly correct,
just the rcu_read_unlock() in the context of the change and the use of
__in6_dev_get alerted me.
As we have a nested rcu_read_lock/unlock invocation just above in
icmpv6_notify, I would still add the rcu_read_lock as noted above for
documentation/review purposes (or remove the rcu_read_lock from the
err_handler dispatch part of icmpv6_notify).
Thank you,
Hannes
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-29 8:19 [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS Duan Jiong
2014-07-30 13:45 ` Hannes Frederic Sowa
@ 2014-07-31 9:54 ` Duan Jiong
2014-07-31 21:05 ` Hannes Frederic Sowa
2014-08-01 5:04 ` David Miller
1 sibling, 2 replies; 10+ messages in thread
From: Duan Jiong @ 2014-07-31 9:54 UTC (permalink / raw)
To: David Miller; +Cc: Hannes Frederic Sowa, eric.dumazet, netdev
When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
and icmpv6_notify() do some valid checks on packet's length, but then some
protocols check packet's length redaudantly. So remove those duplicated
statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
function icmp_socket_deliver() and icmpv6_notify() respectively.
In addition, add missed counter in udp6/udplite6 when socket is NULL.
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
v2: remove nested rcu_read_lock from icmpv6_notify
net/ipv4/icmp.c | 4 +++-
net/ipv4/tcp_ipv4.c | 5 -----
net/ipv6/icmp.c | 13 ++++++++-----
net/ipv6/udp.c | 8 ++++++--
net/sctp/input.c | 5 -----
5 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 42b7bcf..092400e 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -663,8 +663,10 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
/* Checkin full IP header plus 8 bytes of protocol to
* avoid additional coding at protocol handlers.
*/
- if (!pskb_may_pull(skb, iph->ihl * 4 + 8))
+ if (!pskb_may_pull(skb, iph->ihl * 4 + 8)) {
+ ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
return;
+ }
raw_icmp_error(skb, protocol, info);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 77cccda..715cf6b 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -342,11 +342,6 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
int err;
struct net *net = dev_net(icmp_skb->dev);
- if (icmp_skb->len < (iph->ihl << 2) + 8) {
- ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
- return;
- }
-
sk = inet_lookup(net, &tcp_hashinfo, iph->daddr, th->dest,
iph->saddr, th->source, inet_iif(icmp_skb));
if (!sk) {
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index f6c84a6..06ba3e5 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -626,9 +626,10 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
int inner_offset;
__be16 frag_off;
u8 nexthdr;
+ struct net *net = dev_net(skb->dev);
if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
- return;
+ goto out;
nexthdr = ((struct ipv6hdr *)skb->data)->nexthdr;
if (ipv6_ext_hdr(nexthdr)) {
@@ -636,14 +637,14 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
inner_offset = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr),
&nexthdr, &frag_off);
if (inner_offset<0)
- return;
+ goto out;
} else {
inner_offset = sizeof(struct ipv6hdr);
}
/* Checkin header including 8 bytes of inner protocol header. */
if (!pskb_may_pull(skb, inner_offset+8))
- return;
+ goto out;
/* BUGGG_FUTURE: we should try to parse exthdrs in this packet.
Without this we will not able f.e. to make source routed
@@ -652,13 +653,15 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
--ANK (980726)
*/
- rcu_read_lock();
ipprot = rcu_dereference(inet6_protos[nexthdr]);
if (ipprot && ipprot->err_handler)
ipprot->err_handler(skb, NULL, type, code, inner_offset, info);
- rcu_read_unlock();
raw6_icmp_error(skb, nexthdr, type, code, inner_offset, info);
+ return;
+
+out:
+ ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev), ICMP6_MIB_INERRORS);
}
/*
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 7092ff7..f13d297 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -534,11 +534,15 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
struct udphdr *uh = (struct udphdr*)(skb->data+offset);
struct sock *sk;
int err;
+ struct net *net = dev_net(skb->dev);
- sk = __udp6_lib_lookup(dev_net(skb->dev), daddr, uh->dest,
+ sk = __udp6_lib_lookup(net, daddr, uh->dest,
saddr, uh->source, inet6_iif(skb), udptable);
- if (sk == NULL)
+ if (sk == NULL) {
+ ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev),
+ ICMP6_MIB_INERRORS);
return;
+ }
if (type == ICMPV6_PKT_TOOBIG) {
if (!ip6_sk_accept_pmtu(sk))
diff --git a/net/sctp/input.c b/net/sctp/input.c
index f2e2cbd..c1b9912 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -575,11 +575,6 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info)
int err;
struct net *net = dev_net(skb->dev);
- if (skb->len < ihlen + 8) {
- ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
- return;
- }
-
/* Fix up skb to look at the embedded net header. */
saveip = skb->network_header;
savesctp = skb->transport_header;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-31 9:54 ` [PATCH v2] " Duan Jiong
@ 2014-07-31 21:05 ` Hannes Frederic Sowa
2014-08-01 1:09 ` Duan Jiong
2014-08-01 5:04 ` David Miller
1 sibling, 1 reply; 10+ messages in thread
From: Hannes Frederic Sowa @ 2014-07-31 21:05 UTC (permalink / raw)
To: Duan Jiong; +Cc: David Miller, eric.dumazet, netdev
On Do, 2014-07-31 at 17:54 +0800, Duan Jiong wrote:
> When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
> and icmpv6_notify() do some valid checks on packet's length, but then some
> protocols check packet's length redaudantly. So remove those duplicated
> statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
> function icmp_socket_deliver() and icmpv6_notify() respectively.
>
> In addition, add missed counter in udp6/udplite6 when socket is NULL.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
>
> v2: remove nested rcu_read_lock from icmpv6_notify
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
IPv4 handler can also be cleaned up. ;)
Thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-31 21:05 ` Hannes Frederic Sowa
@ 2014-08-01 1:09 ` Duan Jiong
0 siblings, 0 replies; 10+ messages in thread
From: Duan Jiong @ 2014-08-01 1:09 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: David Miller, eric.dumazet, netdev
On 08/01/2014 05:05 AM, Hannes Frederic Sowa wrote:
> On Do, 2014-07-31 at 17:54 +0800, Duan Jiong wrote:
>> When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
>> and icmpv6_notify() do some valid checks on packet's length, but then some
>> protocols check packet's length redaudantly. So remove those duplicated
>> statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
>> function icmp_socket_deliver() and icmpv6_notify() respectively.
>>
>> In addition, add missed counter in udp6/udplite6 when socket is NULL.
>>
>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>> ---
>>
>> v2: remove nested rcu_read_lock from icmpv6_notify
>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> IPv4 handler can also be cleaned up. ;)
Ok, i will make another patch soon. :)
Thanks,
Duan
>
> Thanks!
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS
2014-07-31 9:54 ` [PATCH v2] " Duan Jiong
2014-07-31 21:05 ` Hannes Frederic Sowa
@ 2014-08-01 5:04 ` David Miller
1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2014-08-01 5:04 UTC (permalink / raw)
To: duanj.fnst; +Cc: hannes, eric.dumazet, netdev
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Thu, 31 Jul 2014 17:54:32 +0800
>
> When dealing with ICMPv[46] Error Message, function icmp_socket_deliver()
> and icmpv6_notify() do some valid checks on packet's length, but then some
> protocols check packet's length redaudantly. So remove those duplicated
> statements, and increase counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS in
> function icmp_socket_deliver() and icmpv6_notify() respectively.
>
> In addition, add missed counter in udp6/udplite6 when socket is NULL.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
>
> v2: remove nested rcu_read_lock from icmpv6_notify
Applied to net-next, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-08-01 5:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-29 8:19 [PATCH] net: fix the counter ICMP_MIB_INERRORS/ICMP6_MIB_INERRORS Duan Jiong
2014-07-30 13:45 ` Hannes Frederic Sowa
2014-07-31 2:58 ` Duan Jiong
2014-07-31 7:52 ` Hannes Frederic Sowa
2014-07-31 8:34 ` Duan Jiong
2014-07-31 9:10 ` Hannes Frederic Sowa
2014-07-31 9:54 ` [PATCH v2] " Duan Jiong
2014-07-31 21:05 ` Hannes Frederic Sowa
2014-08-01 1:09 ` Duan Jiong
2014-08-01 5:04 ` 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).