From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [RFC v1 2/3] vxlan: add support for underlay in non-default VRF Date: Fri, 16 Nov 2018 08:31:06 -0800 Message-ID: <9b8ad235-e120-7047-8b6c-9141fe31bd55@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> <334599aa-c4c9-d2ef-c498-0b14230f7115@cumulusnetworks.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 , roopa@cumulusnetworks.com, nicolas.dichtel@6wind.com Return-path: Received: from mail-pf1-f175.google.com ([209.85.210.175]:39678 "EHLO mail-pf1-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728175AbeKQCoN (ORCPT ); Fri, 16 Nov 2018 21:44:13 -0500 Received: by mail-pf1-f175.google.com with SMTP id c72so6959192pfc.6 for ; Fri, 16 Nov 2018 08:31:09 -0800 (PST) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 11/16/18 2:41 AM, Alexis Bauvin wrote: > The case I am trying to cover here is the user creating a VXLAN device with eth0 > as its lower device (ip link add vxlan0 type vxlan ... dev eth0), thus ignoring > the fact that it should be br0 (the actual L3 interface). In this case, the only > information available from the module's point of view is eth0. I may be wrong, > but eth0 is indirectly "part" of vrf-blue (even if it is only L2), as packets > flowing in from it would land in vrf-blue if L3. for routing lookups, yes. > > As for the device stacking, I am only interested in the VXLAN underlay: the > VXLAN device itself could be in a specific VRF or not, it should not influence > its underlay. > > +----------+ +---------+ > | | | | > | vrf-blue | | vrf-red | > | | | | > +----+-----+ +----+----+ > | | > | | > +----+-----+ +----+----+ > | | | | > | br-blue | | br-red | > | | | | > +----+-----+ +---+-+---+ > | | | > | +-----+ +-----+ > | | | > +----+-----+ +------+----+ +----+----+ > | | lower device | | | | > | eth0 | <- - - - - - - | vxlan-red | | tap-red | (... more taps) > | | | | | | > +----------+ +-----------+ +---------+ > > > While I don't see any use case for having a bridged uplink when using VXLAN, > someone may and would find a different behavior depending on the lower device. > In the above example, vxlan-red's lower device should be br-blue, but a user > would expect the underlay VRF (vrf-blue) to still be taken into account if eth0 > was used as the lower device. > > A different approach would be to check if the lower device is a bridge. If not, > fetch a potential master bridge. Then, with this L3/router interface, we fetch > the l3mdev with l3mdev_master_ifindex_by_index (if any). ok. got it. Add the above diagram to the commit message to document the use case. > >> >>> 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. > > Ok! What is the best approach in your opinion? > Add the new function to l3mdev.c. The name should be consistent with the others -- so something like l3mdev_master_upper_by_index (l3mdev for the namespace, you are passing an index and wanting the master device but in this case want to walk upper devices). Also, annotate with expected locking.