Re: [PATCH net-next 13/16] net: Introduce VRF device driver - v2

[David Ahern <dsa@cumulusnetworks.com>]

On 7/27/15 2:01 PM, Nikolay Aleksandrov wrote:
>> +
>> +	if (!vrf_is_master(dev) || vrf_is_master(port_dev) ||
>
> Hmm, this means that bonds won't be able to be VRF slaves.
> They have the IFF_MASTER flag set.

Right, will change to the IFF_VRF_MASTER flag.

>
>> +	    vrf_is_slave(port_dev))
>> +		return -EINVAL;
>> +
>> +	return do_vrf_add_slave(dev, port_dev);
>> +}
>> +
>> +/* inverse of do_vrf_add_slave */
>> +static int do_vrf_del_slave(struct net_device *dev, struct net_device *port_dev)
>> +{
>> +	struct net_vrf *vrf = netdev_priv(dev);
>> +	struct slave_queue *queue = &vrf->queue;
>> +	struct net_vrf_dev *vrf_ptr = NULL;
>> +	struct slave *slave;
>> +
>> +	vrf_ptr = rcu_dereference(dev->vrf_ptr);
>> +	RCU_INIT_POINTER(dev->vrf_ptr, NULL);
>
> I think this isn't safe, you should wait for a grace period before freeing the
> pointer. Actually you can just move the kfree() below the netdev_rx_handler_unregister()
> since it does synchronize_rcu() anyway.

ok

And ack on all other comments..

Thanks for the review,
David