On Mon, Dec 17, 2018 at 6:58 AM Petr Machata <[email protected]> wrote:
>
> In __vxlan_dev_create(), one of the steps is to add a default FDB entry.
> The default entry is created and added before the device is even
> registered, and is likewise not removed until destruction of the device.
> (Except when it's removed explicitly.) Consequently, the notification
> about this default FDB entry is only sent belatedly, to prevent sending
> neighbor notifications before notifications for the corresponding link
> itself.
>
> Before the addition notification is sent, the whole procedure can fail
> due to an error return from rtnl_configure_link(). That triggers
> unregister of the freshly registered device, which causes removal of the
> default FDB entry, including deletion notification. However since at
> that point the addition notification itself was not yet distributed, it
> is incorrect to send deletion notifications.
>
> To fix the issue, allocate a bit at VXLAN device to communicate whether
> a notifier should be invoked for the sole FDB entry or not. Set to true
> after the notification is sent.
>
> Fixes: 0241b836732f ("vxlan: fix default fdb entry netlink notify ordering
> during netdev create")
> Signed-off-by: Petr Machata <[email protected]>
The initial fix used a flag argument to vxlan_fdb_destroy to not send
the notification. I understand that you are reworking it...
But a general do_notify flag in vxlan config to work around this seems
odd. pls see my comment on your other patch that
removes the vxlan_fdb_destroy call, can we fix vxlan_uninit or the
error path in vxlan create to not do call destroy twice ?
> ---
> drivers/net/vxlan.c | 3 ++-
> include/net/vxlan.h | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 139741617b90..c60f1b420d71 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -850,7 +850,7 @@ static void vxlan_fdb_destroy(struct vxlan_dev *vxlan,
> struct vxlan_fdb *f,
> "delete %pM\n", f->eth_addr);
>
> --vxlan->addrcnt;
> - if (do_notify)
> + if (do_notify && vxlan->cfg.do_notify)
> list_for_each_entry(rd, &f->remotes, list)
> vxlan_fdb_notify(vxlan, f, rd, RTM_DELNEIGH);
>
> @@ -3295,6 +3295,7 @@ static int __vxlan_dev_create(struct net *net, struct
> net_device *dev,
> /* notify default fdb entry */
> if (f)
> vxlan_fdb_notify(vxlan, f, first_remote_rtnl(f),
> RTM_NEWNEIGH);
> + vxlan->cfg.do_notify = true;
>
> list_add(&vxlan->next, &vn->vxlan_list);
> return 0;
> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
> index 03431c148e16..5932f89bd932 100644
> --- a/include/net/vxlan.h
> +++ b/include/net/vxlan.h
> @@ -216,6 +216,7 @@ struct vxlan_config {
> unsigned long age_interval;
> unsigned int addrmax;
> bool no_share;
> + u8 do_notify:1;
> };
>
> struct vxlan_dev_node {
> --
> 2.4.11
>
