netdev - Re: [RFC v1 2/3] vxlan: add support for underlay in non-default VRF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 16 Nov 2018 08:31:06 -0800
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Alexis Bauvin <abauvin@...leway.com>, roopa@...ulusnetworks.com,
        nicolas.dichtel@...nd.com
Cc:     netdev@...r.kernel.org, akherbouche@...leway.com
Subject: Re: [RFC v1 2/3] vxlan: add support for underlay in non-default VRF

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.