Re: [RFC v3 3/3] vxlan: handle underlay VRF changes

[David Ahern <dsa@cumulusnetworks.com>]

On 11/20/18 8:48 AM, David Ahern wrote:
> On 11/20/18 8:35 AM, Roopa Prabhu wrote:
>> On Tue, Nov 20, 2018 at 7:04 AM David Ahern <dsa@cumulusnetworks.com> wrote:
>>>
>>> On 11/20/18 7:23 AM, Alexis Bauvin wrote:
>>>> When underlay VRF changes, either because the lower device itself changed,
>>>> or its VRF changed, this patch releases the current socket of the VXLAN
>>>> device and recreates another one in the right VRF. This allows for
>>>> on-the-fly change of the underlay VRF of a VXLAN device.
>>>>
>>>> Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
>>>> Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
>>>> Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
>>>> ---
>>>>  drivers/net/vxlan.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 94 insertions(+)
>>>>
>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>> index a3de08122269..1e6ccad6df6a 100644
>>>> --- a/drivers/net/vxlan.c
>>>> +++ b/drivers/net/vxlan.c
>>>> @@ -208,6 +208,18 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
>>>>       return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
>>>>  }
>>>>
>>>> +static int vxlan_is_in_l3mdev_chain(struct net_device *chain,
>>>> +                                 struct net_device *dev)
>>>> +{
>>>> +     if (!chain)
>>>> +             return 0;
>>>> +
>>>> +     if (chain->ifindex == dev->ifindex)
>>>> +             return 1;
>>>> +     return vxlan_is_in_l3mdev_chain(netdev_master_upper_dev_get(chain),
>>>> +                                     dev);
>>>> +}
>>>
>>> This should return bool and true/false.
>>>
>>> Also, why l3mdev in the name? None of the checks look at whether it is
>>> an l3mdev master.
>>>
>>> And again here, someone more familiar with the vxlan code should review it.
>>>
>>
>>
>> I understand the need for patch 2. But I don't understand the need for
>> the complexity this patch introduces (especially implicit down and up
>> of the vxlan device).
>> Alexis, If your underlay routing changes,  you can down and up the
>> vxlan device from user-space correct ?. This should be true for any
>> tunnel device.
>>
> 
> I believe this patch handles changes to the VRF association of the bridge.
> 
Re-reading the commit message, this handles changes in VRF association
of the lower device.

If the vxlan socket in general (vrf or not) can be bound to the lower
device instead of the VRF then it simplifies things a lot.