netdev - Re: [PATCH net-next] tun: do not compute the rxhash, if not needed

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]

Message-ID: <025e7317-0213-426a-f7cb-91c1c08e99bc@redhat.com>
Date:   Fri, 20 Apr 2018 20:19:45 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next] tun: do not compute the rxhash, if not needed



On 2018年04月20日 19:18, Paolo Abeni wrote:
> Currently, the tun driver, in absence of an eBPF steering program,
> always compute the rxhash in its rx path, even when such value
> is later unused due to additional checks (
>
> This changeset moves the all the related checks just before the
> __skb_get_hash_symmetric(), so that the latter is no more computed
> when unneeded.
>
> Also replace an unneeded RCU section with rcu_access_pointer().
>
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
>   drivers/net/tun.c | 15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 1e58be152d5c..091ace726763 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -525,11 +525,6 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>   
>   	rcu_read_lock();
>   
> -	/* We may get a very small possibility of OOO during switching, not
> -	 * worth to optimize.*/
> -	if (tun->numqueues == 1 || tfile->detached)
> -		goto unlock;
> -
>   	e = tun_flow_find(head, rxhash);
>   	if (likely(e)) {
>   		/* TODO: keep queueing to old queue until it's empty? */
> @@ -548,7 +543,6 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>   		spin_unlock_bh(&tun->lock);
>   	}
>   
> -unlock:
>   	rcu_read_unlock();
>   }
>   
> @@ -1937,10 +1931,13 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>   		rcu_read_unlock();
>   	}
>   
> -	rcu_read_lock();
> -	if (!rcu_dereference(tun->steering_prog))
> +	/* Compute the costly rx hash only if needed for flow updates.
> +	 * We may get a very small possibility of OOO during switching, not
> +	 * worth to optimize.
> +	 */
> +	if (!rcu_access_pointer(tun->steering_prog) && tun->numqueues > 1 &&
> +	    !tfile->detached)
>   		rxhash = __skb_get_hash_symmetric(skb);
> -	rcu_read_unlock();
>   
>   	if (frags) {
>   		/* Exercise flow dissector code path. */

Acked-by: Jason Wang <jasowang@...hat.com>