Re: [PATCH v2 net] packet: Do not call fanout_release from atomic contexts

[Eric Dumazet <eric.dumazet@gmail.com>]

On Wed, 2017-02-15 at 16:43 +0000, Anoob Soman wrote:

> 
>  net/packet/af_packet.c | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index d56ee46..af29510 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1497,6 +1497,8 @@ static void __fanout_link(struct sock *sk, struct packet_sock *po)
>  	f->arr[f->num_members] = sk;
>  	smp_wmb();
>  	f->num_members++;
> +	if (f->num_members == 1)
> +		dev_add_pack(&f->prot_hook);
>  	spin_unlock(&f->lock);
>  }
>  
> @@ -1513,6 +1515,8 @@ static void __fanout_unlink(struct sock *sk, struct packet_sock *po)
>  	BUG_ON(i >= f->num_members);
>  	f->arr[i] = f->arr[f->num_members - 1];
>  	f->num_members--;
> +	if (f->num_members == 0)
> +		__dev_remove_pack(&f->prot_hook);
>  	spin_unlock(&f->lock);
>  }
>  
> @@ -1687,7 +1691,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>  		match->prot_hook.func = packet_rcv_fanout;
>  		match->prot_hook.af_packet_priv = match;
>  		match->prot_hook.id_match = match_fanout_group;
> -		dev_add_pack(&match->prot_hook);
>  		list_add(&match->list, &fanout_list);
>  	}
>  	err = -EINVAL;
> @@ -1712,10 +1715,16 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>  	return err;
>  }
>  
> -static void fanout_release(struct sock *sk)
> +/* If pkt_sk(sk)->fanout->sk_ref is zero, this functuon removes
> + * pkt_sk(sk)->fanout from fanout_list and returns pkt_sk(sk)->fanout.
> + * It is the responsibility of the caller to call fanout_release_data() and
> + * free the returned packet_fanout (after synchronize_net())
> + */
> +static struct packet_fanout *fanout_release(struct sock *sk)
>  {
>  	struct packet_sock *po = pkt_sk(sk);
>  	struct packet_fanout *f;
> +	bool ret_fanout = false;

No need for this new variable.

>  
>  	f = po->fanout;
>  	if (!f)
> @@ -1726,14 +1735,14 @@ static void fanout_release(struct sock *sk)
>  
>  	if (atomic_dec_and_test(&f->sk_ref)) {
>  		list_del(&f->list);
> -		dev_remove_pack(&f->prot_hook);
> -		fanout_release_data(f);
> -		kfree(f);
> +		ret_fanout = true;
>  	}

} else {
     f = NULL;
}

>  	mutex_unlock(&fanout_mutex);
>  
>  	if (po->rollover)
>  		kfree_rcu(po->rollover, rcu);
> +
> +	return ret_fanout ? f : NULL;

return f;

>  }
>  
>  

Otherwise, this look good.

But make sure to respin your patch based on latest net tree.

af_packet.c got a recent fix.

Thanks