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
signature.asc
Description: OpenPGP digital signature