netdev - Re: [PATCH net-next] net: Use rx_nohandler for unhandled packets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 14 Dec 2019 17:45:51 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Patrick Talbert <ptalbert@...hat.com>,
        Jarod Wilson <jarod@...hat.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: Use rx_nohandler for unhandled packets

On Wed, 11 Dec 2019 17:21:07 +0100, Patrick Talbert wrote:
> Since caf586e5f23c ("net: add a core netdev->rx_dropped counter") incoming
> packets which do not have a handler cause a counter named rx_dropped to be
> incremented. This can lead to confusion as some see a non-zero "drop"
> counter as cause for concern.
> 
> To avoid any confusion, instead use the existing rx_nohandler counter. Its
> name more closely aligns with the activity being tracked here.
> 
> Signed-off-by: Patrick Talbert <ptalbert@...hat.com>

Looks like commit 6e7333d315a7 ("net: add rx_nohandler stat counter")
is far more relevant here, it added rx_nohandler but kept using
rx_dropped for non-exact delivery.

Jarod, could you take a look?

> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 9ef20389622d..3d194cea9859 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1618,8 +1618,9 @@ enum netdev_priv_flags {
>   *			do not use this in drivers
>   *	@tx_dropped:	Dropped packets by core network,
>   *			do not use this in drivers
> - *	@rx_nohandler:	nohandler dropped packets by core network on
> - *			inactive devices, do not use this in drivers
> + *	@rx_nohandler:	Dropped packets by core network when they were not handled
> + *			by any protocol/socket or the device was inactive,
> + *			do not use this in drivers.
>   *	@carrier_up_count:	Number of times the carrier has been up
>   *	@carrier_down_count:	Number of times the carrier has been down
>   *
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2c277b8aba38..11e500f8ffa3 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5123,20 +5123,19 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
>  			goto drop;
>  		*ppt_prev = pt_prev;
>  	} else {
> -drop:
> -		if (!deliver_exact)
> -			atomic_long_inc(&skb->dev->rx_dropped);
> -		else
> -			atomic_long_inc(&skb->dev->rx_nohandler);
> +		/* We have not delivered the skb anywhere */
> +		atomic_long_inc(&skb->dev->rx_nohandler);
>  		kfree_skb(skb);
> -		/* Jamal, now you will not able to escape explaining
> -		 * me how you were going to use this. :-)
> -		 */
>  		ret = NET_RX_DROP;
>  	}
>  
>  out:
>  	return ret;
> +drop:
> +	atomic_long_inc(&skb->dev->rx_dropped);
> +	kfree_skb(skb);
> +	ret = NET_RX_DROP;
> +	goto out;
>  }
>  
>  static int __netif_receive_skb_one_core(struct sk_buff *skb, bool pfmemalloc)