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

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

[Paolo Abeni]

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@redhat.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. */
-- 
2.14.3

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

[Jason Wang]

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@redhat.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@redhat.com>

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

[David Miller]

From: Paolo Abeni <pabeni@redhat.com>
Date: Fri, 20 Apr 2018 13:18:16 +0200

> 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@redhat.com>

Applied, thank you.