From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Ahern <dsa@cumulusnetworks.com>
Subject: Re: [RFC v1 2/3] vxlan: add support for underlay in non-default VRF
Date: Thu, 15 Nov 2018 23:37:30 -0800
Message-ID: <334599aa-c4c9-d2ef-c498-0b14230f7115@cumulusnetworks.com>
References: <20181114093104.93286-1-abauvin@scaleway.com>
 <20181114093104.93286-3-abauvin@scaleway.com>
 <3777c36e-ccc7-2c4d-889b-46730fafc43d@cumulusnetworks.com>
 <41C7AE9A-5C5B-4DA0-9C58-16716F710A62@scaleway.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: netdev@vger.kernel.org, akherbouche@scaleway.com
To: Alexis Bauvin <abauvin@scaleway.com>, roopa@cumulusnetworks.com,
        nicolas.dichtel@6wind.com
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pf1-f193.google.com ([209.85.210.193]:46114 "EHLO
        mail-pf1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1727380AbeKPRsp (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 16 Nov 2018 12:48:45 -0500
Received: by mail-pf1-f193.google.com with SMTP id s9-v6so10974542pfm.13
        for <netdev@vger.kernel.org>; Thu, 15 Nov 2018 23:37:33 -0800 (PST)
In-Reply-To: <41C7AE9A-5C5B-4DA0-9C58-16716F710A62@scaleway.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 11/15/18 2:05 AM, Alexis Bauvin wrote:
> Le 14 nov. 2018 à 20:58, David Ahern <dsa@cumulusnetworks.com> a écrit :
>>
>> you are making this more specific than it needs to be ....
>>
>> On 11/14/18 1:31 AM, Alexis Bauvin wrote:
>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>> index 27bd586b94b0..7477b5510a04 100644
>>> --- a/drivers/net/vxlan.c
>>> +++ b/drivers/net/vxlan.c
>>> @@ -208,11 +208,23 @@ 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_get_l3mdev(struct net *net, int ifindex)
>>> +{
>>> +	struct net_device *dev;
>>> +
>>> +	dev = __dev_get_by_index(net, ifindex);
>>> +	while (dev && !netif_is_l3_master(dev))
>>> +		dev = netdev_master_upper_dev_get(dev);
>>> +
>>> +	return dev ? dev->ifindex : 0;
>>> +}
>>
>> l3mdev_master_ifindex_by_index should work instead of defining this for
>> vxlan.
>>
>> But I do not believe you need this function.
> 
> l3mdev_master_ifindex_by_index does not recursively climbs up the master chain.
> This means that if the l3mdev is not a direct master of the device, it will not
> be found.
> 
> E.G. Calling l3mdev_master_ifindex_by_index with the index of eth0 will
> return 0:
> 
> +------+     +-----+     +----------+
> |      |     |     |     |          |
> | eth0 +-----+ br0 +-----+ vrf-blue |
> |      |     |     |     |          |
> +------+     +-----+     +----------+
> 

eth0 is not the L3/router interface in this picture; br0 is. There
should not be a need for invoking l3mdev_master_ifindex_by_index on eth0.

What device stacking are you expecting to handle with vxlan devices?
vxlan on eth0 with vxlan devices in a VRF? vxlan devices into a bridge
with the bridge (or SVI) enslaved to a VRF?


> This is because the underlying l3mdev_master_dev_rcu function fetches the master
> (br0 in this case), checks whether it is an l3mdev (which it is not), and
> returns its index if so.
> 
> So if using l3mdev_master_dev_rcu, using eth0 as a lower device will still bind
> to no specific device, thus in the default VRF.
> 
> Maybe I should have patched l3mdev_master_dev_rcu to do a recursive resolution
> (as vxlan_get_l3mdev does), but I don’t know the impact of such a change.

no, that is definitely the wrong the approach.