[net-next] icmp: correct return value of icmp_rcv()

[net-next] icmp: correct return value of icmp_rcv()

[Zhang Shengju]

Currently, icmp_rcv() always return zero on a packet delivery upcall.

To make its behavior more compliant with the way this API should be
used, this patch changes this to let it return NET_RX_SUCCESS when the
packet is proper handled, and NET_RX_DROP otherwise.

Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
---
 net/ipv4/icmp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 691146a..f79d7a8 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1047,12 +1047,12 @@ int icmp_rcv(struct sk_buff *skb)
 
 	if (success)  {
 		consume_skb(skb);
-		return 0;
+		return NET_RX_SUCCESS;
 	}
 
 drop:
 	kfree_skb(skb);
-	return 0;
+	return NET_RX_DROP;
 csum_error:
 	__ICMP_INC_STATS(net, ICMP_MIB_CSUMERRORS);
 error:
-- 
1.8.3.1

Re: [net-next] icmp: correct return value of icmp_rcv()

[Eric Dumazet]

On Wed, 2016-12-07 at 14:52 +0800, Zhang Shengju wrote:
> Currently, icmp_rcv() always return zero on a packet delivery upcall.
> 
> To make its behavior more compliant with the way this API should be
> used, this patch changes this to let it return NET_RX_SUCCESS when the
> packet is proper handled, and NET_RX_DROP otherwise.
> 
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> ---
>  net/ipv4/icmp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 691146a..f79d7a8 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -1047,12 +1047,12 @@ int icmp_rcv(struct sk_buff *skb)
>  
>  	if (success)  {
>  		consume_skb(skb);
> -		return 0;
> +		return NET_RX_SUCCESS;
>  	}
>  
>  drop:
>  	kfree_skb(skb);
> -	return 0;
> +	return NET_RX_DROP;
>  csum_error:
>  	__ICMP_INC_STATS(net, ICMP_MIB_CSUMERRORS);
>  error:


I am curious, what external/visible effects do you expect from such a
change ?

We now have a very precise monitoring of where packets are dropped
(consume_skb()/kfree_skb()) 

RE: [net-next] icmp: correct return value of icmp_rcv()

[张胜举]

> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Wednesday, December 07, 2016 10:18 PM
> To: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> Cc: netdev@vger.kernel.org
> Subject: Re: [net-next] icmp: correct return value of icmp_rcv()
> 
> On Wed, 2016-12-07 at 14:52 +0800, Zhang Shengju wrote:
> > Currently, icmp_rcv() always return zero on a packet delivery upcall.
> >
> > To make its behavior more compliant with the way this API should be
> > used, this patch changes this to let it return NET_RX_SUCCESS when the
> > packet is proper handled, and NET_RX_DROP otherwise.
> >
> > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> > ---
> >  net/ipv4/icmp.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index 691146a..f79d7a8
> > 100644
> > --- a/net/ipv4/icmp.c
> > +++ b/net/ipv4/icmp.c
> > @@ -1047,12 +1047,12 @@ int icmp_rcv(struct sk_buff *skb)
> >
> >  	if (success)  {
> >  		consume_skb(skb);
> > -		return 0;
> > +		return NET_RX_SUCCESS;
> >  	}
> >
> >  drop:
> >  	kfree_skb(skb);
> > -	return 0;
> > +	return NET_RX_DROP;
> >  csum_error:
> >  	__ICMP_INC_STATS(net, ICMP_MIB_CSUMERRORS);
> >  error:
> 
> 
> I am curious, what external/visible effects do you expect from such a change ?
> 
> We now have a very precise monitoring of where packets are dropped
> (consume_skb()/kfree_skb())
> 
> 

I know that the return value is always ignored, I just to want to make it 
compliant with the way this API required like I said in the comment.

Thanks,

Re: [net-next] icmp: correct return value of icmp_rcv()

[David Miller]

From: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Date: Wed,  7 Dec 2016 14:52:53 +0800

> Currently, icmp_rcv() always return zero on a packet delivery upcall.
> 
> To make its behavior more compliant with the way this API should be
> used, this patch changes this to let it return NET_RX_SUCCESS when the
> packet is proper handled, and NET_RX_DROP otherwise.
> 
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>

Applied.