* Question regarding bond delete notifications
@ 2014-04-27 6:16 Roopa Prabhu
2014-04-30 6:31 ` Jiri Pirko
0 siblings, 1 reply; 2+ messages in thread
From: Roopa Prabhu @ 2014-04-27 6:16 UTC (permalink / raw
To: netdev; +Cc: Jiri Pirko, Thomas Graf, David Miller
Hello,
I am seeing a problem with the order of rtnl link notifications during
bond deletes.
If i delete a bond without deleting its slaves, kernel sends a
RTM_DELLINK during unregister_netdevice
(in rollback_registered_many) and then calls bond_uninit (ndo_uninit).
bond_uninit inturn cleans up the slaves and generates a few RTM_NEWLINK
rtnl notifications for NETDEV_CHANGEADDR and NETDEV_FEAT_CHANGE for the
bond device.
This is confusing to userspace listening for rtnl LINK Notifications.
For example, libnl gets confused with RTM_NEWLINK arriving after
RTM_DELLINK.
The RTM_NEWLINK makes libnl believe that this is a new bond link. But
kernel has already deleted the bond device.
It seems like the kernel should generate RTM_DELLINK notification after
ndo_uninit. something like below.
But am not sure if this change is safe for other netdevices.
Is this a known problem ?. Any suggestions ?
Thanks,
Roopa
diff --git a/net/core/dev.c b/net/core/dev.c
index b1b0c8d..85cb73a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5568,33 +5568,33 @@ static void rollback_registered_many(struct
list_head *head)
list_for_each_entry(dev, head, unreg_list) {
/* Shutdown queueing discipline. */
dev_shutdown(dev);
/* Notify protocols, that we are about to destroy
this device. They should clean all the things.
*/
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
- if (!dev->rtnl_link_ops ||
- dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
-
/*
* Flush the unicast and multicast chains
*/
dev_uc_flush(dev);
dev_mc_flush(dev);
if (dev->netdev_ops->ndo_uninit)
dev->netdev_ops->ndo_uninit(dev);
+ if (!dev->rtnl_link_ops ||
+ dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
+ rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
+
/* Notifier chain MUST detach us all upper devices. */
WARN_ON(netdev_has_any_upper_dev(dev));
/* Remove entries from kobject tree */
netdev_unregister_kobject(dev);
#ifdef CONFIG_XPS
/* Remove XPS queueing entries */
netif_reset_xps_queues_gt(dev, 0);
#endif
}
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: Question regarding bond delete notifications
2014-04-27 6:16 Question regarding bond delete notifications Roopa Prabhu
@ 2014-04-30 6:31 ` Jiri Pirko
0 siblings, 0 replies; 2+ messages in thread
From: Jiri Pirko @ 2014-04-30 6:31 UTC (permalink / raw
To: Roopa Prabhu; +Cc: netdev, Thomas Graf, David Miller, j.vosburgh, vfalico, andy
Sun, Apr 27, 2014 at 08:16:27AM CEST, roopa@cumulusnetworks.com wrote:
>Hello,
>
>I am seeing a problem with the order of rtnl link notifications
>during bond deletes.
>If i delete a bond without deleting its slaves, kernel sends a
>RTM_DELLINK during unregister_netdevice
>(in rollback_registered_many) and then calls bond_uninit (ndo_uninit).
>bond_uninit inturn cleans up the slaves and generates a few
>RTM_NEWLINK rtnl notifications for NETDEV_CHANGEADDR and
>NETDEV_FEAT_CHANGE for the bond device.
>
>This is confusing to userspace listening for rtnl LINK Notifications.
>For example, libnl gets confused with RTM_NEWLINK arriving after
>RTM_DELLINK.
>The RTM_NEWLINK makes libnl believe that this is a new bond link. But
>kernel has already deleted the bond device.
>
>It seems like the kernel should generate RTM_DELLINK notification
>after ndo_uninit. something like below.
>But am not sure if this change is safe for other netdevices.
>
>Is this a known problem ?. Any suggestions ?
>
>Thanks,
>Roopa
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index b1b0c8d..85cb73a 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -5568,33 +5568,33 @@ static void rollback_registered_many(struct
>list_head *head)
> list_for_each_entry(dev, head, unreg_list) {
> /* Shutdown queueing discipline. */
> dev_shutdown(dev);
>
>
> /* Notify protocols, that we are about to destroy
> this device. They should clean all the things.
> */
> call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
>
>- if (!dev->rtnl_link_ops ||
>- dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
>- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
>-
> /*
> * Flush the unicast and multicast chains
> */
> dev_uc_flush(dev);
> dev_mc_flush(dev);
>
> if (dev->netdev_ops->ndo_uninit)
> dev->netdev_ops->ndo_uninit(dev);
>
>+ if (!dev->rtnl_link_ops ||
>+ dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
>+ rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
>+
> /* Notifier chain MUST detach us all upper devices. */
> WARN_ON(netdev_has_any_upper_dev(dev));
>
> /* Remove entries from kobject tree */
> netdev_unregister_kobject(dev);
> #ifdef CONFIG_XPS
> /* Remove XPS queueing entries */
> netif_reset_xps_queues_gt(dev, 0);
> #endif
> }
>
Now I cannot think about why this might troubles. But my feeling it this
might be problematic, just a feeling.
Your change seems logical though.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-04-30 6:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-27 6:16 Question regarding bond delete notifications Roopa Prabhu
2014-04-30 6:31 ` Jiri Pirko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).