Roopa Prabhu <ro...@cumulusnetworks.com> writes:
> On Mon, Dec 17, 2018 at 10:10 AM Petr Machata <pe...@mellanox.com> wrote:
>>
>> Default remotes are stored as FDB entries with an Ethernet address of
>> 00:00:00:00:00:00. When a request is made to change a remote address of
>> a VXLAN device, vxlan_changelink() first deletes the existing default
>> remote, and then creates a new FDB entry.
>>
>> This works well as long as the list of default remotes matches exactly
>> the configuration of a VXLAN remote address. Thus when the VXLAN device
>> has a remote of X, there should be exactly one default remote FDB entry
>> X. If the VXLAN device has no remote address, there should be no such
>> entry.
>>
>> Besides using "ip link set", it is possible to manipulate the list of
>> default remotes by using the "bridge fdb". It is therefore easy to break
>> the above condition. Under such circumstances, the __vxlan_fdb_delete()
>> call doesn't delete the FDB entry itself, but just one remote. The
>> following vxlan_fdb_create() then creates a new FDB entry, leading to a
>> situation where two entries exist for the address 00:00:00:00:00:00,
>> each with a different subset of default remotes.
>>
>> An even more obvious breakage rooted in the same cause can be observed
>> when a remote address is configured for a VXLAN device that did not have
>> one before. In that case vxlan_changelink() doesn't remove any remote,
>> and just creates a new FDB entry for the new address:
>>
>> $ ip link add name vx up type vxlan id 2000 dstport 4789
>> $ bridge fdb ap dev vx 00:00:00:00:00:00 dst 192.0.2.20 self permanent
>> $ bridge fdb ap dev vx 00:00:00:00:00:00 dst 192.0.2.30 self permanent
>> $ ip link set dev vx type vxlan remote 192.0.2.30
>> $ bridge fdb sh dev vx | grep 00:00:00:00:00:00
>> 00:00:00:00:00:00 dst 192.0.2.30 self permanent <- new entry, 1 rdst
>> 00:00:00:00:00:00 dst 192.0.2.20 self permanent <- orig. entry, 2 rdsts
>> 00:00:00:00:00:00 dst 192.0.2.30 self permanent
>>
>> To fix this, instead of calling vxlan_fdb_create() directly, defer to
>> vxlan_fdb_update(). That has logic to handle the duplicates properly.
>> Additionally, it also handles notifications, so drop that call from
>> changelink as well.
>>
>> Fixes: 0241b836732f ("vxlan: fix default fdb entry netlink notify ordering
>> during netdev create")
>> Signed-off-by: Petr Machata <pe...@mellanox.com>
>> ---
>
> Acked-by: Roopa Prabhu <ro...@cumulusnetworks.com>
>
> This fix looks fine. But it almost seems like the changelink Fixes tag
> is more appropriate here unless you are
> sure that it worked correctly after the changelink.
I don't follow. What do you mean?
> Though I agree
> this needs to be fixed, deployments either
> configure the default remote dst on the vxlan device or manage default
> remote dsts via fdb api in control plane
> not both. The trigger for this bug seems like the use of both.
Yep, you need to do both to trigger this. Which is why I think this went
undetected--otherwise people would have noticed some addresses are not
flooded.
