Re: [PATCH net-next 3/3] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses

From: Matthias Schiffer
Date: Wed Apr 05 2017 - 13:00:24 EST


On 03/15/2017 04:22 PM, Jiri Benc wrote:
> On Wed, 15 Mar 2017 15:29:29 +0100, Matthias Schiffer wrote:
>> While ensuring that the destination address is link-local iff the source
>> address is would also be an option, it didn't seem too useful as the
>> destination address will be a multicast address anyways in "normal" VXLAN
>> configurations. If we really want to check this, I guess the valid
>> combinations are:
>>
>> source link-local - destination link-local UC
>> source link-local - destination link-local MC
>> source global/... - destination global/... UC
>> source global/... - destination any MC
>>
>> Does this make sense?
>
> It does.
>
> Thanks!
>
> Jiri
>

While trying to integrate the additional checks, I noticed that the
vxlan_dev_configure() function has some serious issues in the changelink
case. An (probably incomplete) list of things that can go wrong:

- vxlan_dev_configure may return with an error as late the "if (conf->mtu)"
branch, after some settings have already been applied to the vxlan_dev or
net_device, leading to partial application in some error cases

- at the moment, changelink is allowed to change the address family of the
source/destionation addresses, but VXLAN_F_IPV6 is never removed, and
sockets are not recreated; I think changing the AF should just be disallowed

- conf->mtu will be re-applied in changelink even when the lowerdev has not
changed, possibly overwriting other MTU changes that have been made after
device creation (having conf->mtu in addition to dev->mtu might be a bad
idea in general?)


Each of the issues could be fixed separately, but at the moment, I'm rather
considering cleaning up the code by factoring out a vxlan_cfg_validate()
from vxlan_dev_configure(), to clearly separate validation and application
of the configuration. Is this the way to go, or do you have other suggestions?

-- Matthias

Attachment: signature.asc
Description: OpenPGP digital signature