[PATCH v2 net-next] ipv[4|6]: correct dropwatch false positive in local_deliver_finish

[PATCH v2 net-next] ipv[4|6]: correct dropwatch false positive in local_deliver_finish

[Neil Horman]

I had a report recently of a user trying to use dropwatch to localise some frame
loss, and they were getting false positives.  Turned out they were using a user
space SCTP stack that used raw sockets to grab frames.  When we don't have a
registered protocol for a given packet, we record it as a drop, even if a raw
socket receieves the frame.  We should only record the drop in the event a raw
socket doesnt exist to receive the frames

Tested by the reported successfully

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Reported-by: William Reich <reich@ulticom.com>
Tested-by: William Reich <reich@ulticom.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: William Reich <reich@ulticom.com>
CC: eric.dumazet@gmail.com
---
 net/ipv4/ip_input.c  | 6 ++++--
 net/ipv6/ip6_input.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 87abd3e..2bdf802 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -228,9 +228,11 @@ static int ip_local_deliver_finish(struct sk_buff *skb)
 					icmp_send(skb, ICMP_DEST_UNREACH,
 						  ICMP_PROT_UNREACH, 0);
 				}
-			} else
+				kfree_skb(skb);
+			} else {
 				IP_INC_STATS_BH(net, IPSTATS_MIB_INDELIVERS);
-			kfree_skb(skb);
+				consume_skb(skb);
+			}
 		}
 	}
  out:
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 5b10414..b1876e5 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -241,9 +241,11 @@ resubmit:
 				icmpv6_send(skb, ICMPV6_PARAMPROB,
 					    ICMPV6_UNK_NEXTHDR, nhoff);
 			}
-		} else
+			kfree_skb(skb);
+		} else {
 			IP6_INC_STATS_BH(net, idev, IPSTATS_MIB_INDELIVERS);
-		kfree_skb(skb);
+			consume_skb(skb);
+		}
 	}
 	rcu_read_unlock();
 	return 0;
-- 
1.7.11.7

Re: [PATCH v2 net-next] ipv[4|6]: correct dropwatch false positive in local_deliver_finish

[Eric Dumazet]

On Fri, 2013-03-01 at 12:44 -0500, Neil Horman wrote:
> I had a report recently of a user trying to use dropwatch to localise some frame
> loss, and they were getting false positives.  Turned out they were using a user
> space SCTP stack that used raw sockets to grab frames.  When we don't have a
> registered protocol for a given packet, we record it as a drop, even if a raw
> socket receieves the frame.  We should only record the drop in the event a raw
> socket doesnt exist to receive the frames
> 
> Tested by the reported successfully
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> Reported-by: William Reich <reich@ulticom.com>
> Tested-by: William Reich <reich@ulticom.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: William Reich <reich@ulticom.com>
> CC: eric.dumazet@gmail.com
> ---
>  net/ipv4/ip_input.c  | 6 ++++--
>  net/ipv6/ip6_input.c | 6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)

Thanks !

Acked-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH v2 net-next] ipv[4|6]: correct dropwatch false positive in local_deliver_finish

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 01 Mar 2013 09:56:44 -0800

> On Fri, 2013-03-01 at 12:44 -0500, Neil Horman wrote:
>> I had a report recently of a user trying to use dropwatch to localise some frame
>> loss, and they were getting false positives.  Turned out they were using a user
>> space SCTP stack that used raw sockets to grab frames.  When we don't have a
>> registered protocol for a given packet, we record it as a drop, even if a raw
>> socket receieves the frame.  We should only record the drop in the event a raw
>> socket doesnt exist to receive the frames
>> 
>> Tested by the reported successfully
>> 
>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>> Reported-by: William Reich <reich@ulticom.com>
>> Tested-by: William Reich <reich@ulticom.com>
>> CC: "David S. Miller" <davem@davemloft.net>
>> CC: William Reich <reich@ulticom.com>
>> CC: eric.dumazet@gmail.com
>> ---
>>  net/ipv4/ip_input.c  | 6 ++++--
>>  net/ipv6/ip6_input.c | 6 ++++--
>>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> Thanks !
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

This seems like a reasonable bug fix so I'm going to apply this to
'net', thanks everyone.

Re: [PATCH v2 net-next] ipv[4|6]: correct dropwatch false positive in local_deliver_finish

[Neil Horman]

On Fri, Mar 01, 2013 at 03:57:09PM -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 01 Mar 2013 09:56:44 -0800
> 
> > On Fri, 2013-03-01 at 12:44 -0500, Neil Horman wrote:
> >> I had a report recently of a user trying to use dropwatch to localise some frame
> >> loss, and they were getting false positives.  Turned out they were using a user
> >> space SCTP stack that used raw sockets to grab frames.  When we don't have a
> >> registered protocol for a given packet, we record it as a drop, even if a raw
> >> socket receieves the frame.  We should only record the drop in the event a raw
> >> socket doesnt exist to receive the frames
> >> 
> >> Tested by the reported successfully
> >> 
> >> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >> Reported-by: William Reich <reich@ulticom.com>
> >> Tested-by: William Reich <reich@ulticom.com>
> >> CC: "David S. Miller" <davem@davemloft.net>
> >> CC: William Reich <reich@ulticom.com>
> >> CC: eric.dumazet@gmail.com
> >> ---
> >>  net/ipv4/ip_input.c  | 6 ++++--
> >>  net/ipv6/ip6_input.c | 6 ++++--
> >>  2 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > Thanks !
> > 
> > Acked-by: Eric Dumazet <edumazet@google.com>
> 
> This seems like a reasonable bug fix so I'm going to apply this to
> 'net', thanks everyone.
> 
Copy that, thanks Dave!
Neil