Re: [net-next PATCH] net: devmap fix mutex in rcu critical section

[John Fastabend <john.fastabend@gmail.com>]

On 08/04/2017 02:21 PM, John Fastabend wrote:
> Originally we used a mutex to protect concurrent devmap update
> and delete operations from racing with netdev unregister notifier
> callbacks.
> 

[...]

>  }
> @@ -396,22 +385,20 @@ static int dev_map_notification(struct notifier_block *notifier,
>  

Daniel reminds me this is not in a rcu_read_lock/unlock() section as
needed, so v2 on its way. Thanks!

>  	switch (event) {
>  	case NETDEV_UNREGISTER:
> -		mutex_lock(&dev_map_list_mutex);

		rcu_read_lock();

>  		list_for_each_entry(dtab, &dev_map_list, list) {
>  			for (i = 0; i < dtab->map.max_entries; i++) {
> -				struct bpf_dtab_netdev *dev;
> +				struct bpf_dtab_netdev *dev, *odev;
>  
> -				dev = dtab->netdev_map[i];
> +				dev = READ_ONCE(dtab->netdev_map[i]);
>  				if (!dev ||
>  				    dev->dev->ifindex != netdev->ifindex)
>  					continue;
> -				dev = xchg(&dtab->netdev_map[i], NULL);
> -				if (dev)
> +				odev = cmpxchg(&dtab->netdev_map[i], dev, NULL);
> +				if (dev == odev)
>  					call_rcu(&dev->rcu,
>  						 __dev_map_entry_free);
>  			}
>  		}

		rcu_read_unlock();

> -		mutex_unlock(&dev_map_list_mutex);
>  		break;
>  	default:
>  		break;
>