* [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing
@ 2011-08-31 15:15 Jiri Pirko
2011-08-31 15:20 ` Stephen Hemminger
2011-08-31 15:25 ` Ben Hutchings
0 siblings, 2 replies; 6+ messages in thread
From: Jiri Pirko @ 2011-08-31 15:15 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, bhutchings, shemminger
In some situations, like when the device is used as slave device in
bond/br/etc it is not nice if someone closes the device. This allows
it's masters to forbid this closure.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
include/linux/netdevice.h | 1 +
net/core/dev.c | 34 ++++++++++++++++++++++++++++------
net/core/rtnetlink.c | 1 +
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dad7e4d..b8047d3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1559,6 +1559,7 @@ struct packet_type {
#define NETDEV_RELEASE 0x0012
#define NETDEV_NOTIFY_PEERS 0x0013
#define NETDEV_JOIN 0x0014
+#define NETDEV_PRE_DOWN 0x0015
extern int register_netdevice_notifier(struct notifier_block *nb);
extern int unregister_netdevice_notifier(struct notifier_block *nb);
diff --git a/net/core/dev.c b/net/core/dev.c
index 11b0fc7..d252a7e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1253,11 +1253,24 @@ static int __dev_close_many(struct list_head *head)
return 0;
}
+static int __dev_pre_close(struct net_device *dev)
+{
+ int err;
+
+ err = call_netdevice_notifiers(NETDEV_PRE_DOWN, dev);
+ if (err)
+ return notifier_to_errno(err);
+ return 0;
+}
+
static int __dev_close(struct net_device *dev)
{
int retval;
LIST_HEAD(single);
+ retval = __dev_pre_close(dev);
+ if (retval)
+ return retval;
list_add(&dev->unreg_list, &single);
retval = __dev_close_many(&single);
list_del(&single);
@@ -1269,9 +1282,12 @@ static int dev_close_many(struct list_head *head)
struct net_device *dev, *tmp;
LIST_HEAD(tmp_list);
- list_for_each_entry_safe(dev, tmp, head, unreg_list)
+ list_for_each_entry_safe(dev, tmp, head, unreg_list) {
if (!(dev->flags & IFF_UP))
list_move(&dev->unreg_list, &tmp_list);
+ else
+ __dev_pre_close(dev);
+ }
__dev_close_many(head);
@@ -1289,21 +1305,26 @@ static int dev_close_many(struct list_head *head)
* dev_close - shutdown an interface.
* @dev: device to shutdown
*
- * This function moves an active device into down state. A
- * %NETDEV_GOING_DOWN is sent to the netdev notifier chain. The device
- * is then deactivated and finally a %NETDEV_DOWN is sent to the notifier
- * chain.
+ * This function moves an active device into down state.
+ * A %NETDEV_PRE_DOWN and %NETDEV_GOING_DOWN is sent to the netdev
+ * notifier chain. The device is then deactivated and finally
+ * a %NETDEV_DOWN is sent to the notifier chain.
*/
int dev_close(struct net_device *dev)
{
+ int retval = 0;
+
if (dev->flags & IFF_UP) {
LIST_HEAD(single);
+ retval = __dev_pre_close(dev);
+ if (retval)
+ return retval;
list_add(&dev->unreg_list, &single);
dev_close_many(&single);
list_del(&single);
}
- return 0;
+ return retval;
}
EXPORT_SYMBOL(dev_close);
@@ -1397,6 +1418,7 @@ rollback:
break;
if (dev->flags & IFF_UP) {
+ nb->notifier_call(nb, NETDEV_PRE_DOWN, dev);
nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
nb->notifier_call(nb, NETDEV_DOWN, dev);
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 39f8dd6..34f5b32 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1997,6 +1997,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_UP:
case NETDEV_DOWN:
case NETDEV_PRE_UP:
+ case NETDEV_PRE_DOWN:
case NETDEV_POST_INIT:
case NETDEV_REGISTER:
case NETDEV_CHANGE:
--
1.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing
2011-08-31 15:15 [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing Jiri Pirko
@ 2011-08-31 15:20 ` Stephen Hemminger
2011-08-31 15:25 ` Ben Hutchings
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2011-08-31 15:20 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, eric.dumazet, bhutchings
On Wed, 31 Aug 2011 17:15:31 +0200
Jiri Pirko <jpirko@redhat.com> wrote:
> In some situations, like when the device is used as slave device in
> bond/br/etc it is not nice if someone closes the device. This allows
> it's masters to forbid this closure.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
I don't think this is necessary, for bridging case.
bridging handles it. And bonding should as well.
It is a good way to test STP etc.
Is this a case of "you really shouldn't do this", or
"don't do this it will crash"? In general Linux allows the
former and uses references to prevent the later.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing
2011-08-31 15:15 [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing Jiri Pirko
2011-08-31 15:20 ` Stephen Hemminger
@ 2011-08-31 15:25 ` Ben Hutchings
2011-08-31 15:53 ` Stephen Hemminger
2011-08-31 16:21 ` Jiri Pirko
1 sibling, 2 replies; 6+ messages in thread
From: Ben Hutchings @ 2011-08-31 15:25 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, eric.dumazet, shemminger
On Wed, 2011-08-31 at 17:15 +0200, Jiri Pirko wrote:
> In some situations, like when the device is used as slave device in
> bond/br/etc it is not nice if someone closes the device. This allows
> it's masters to forbid this closure.
No it doesn't.
[...]
> @@ -1269,9 +1282,12 @@ static int dev_close_many(struct list_head *head)
> struct net_device *dev, *tmp;
> LIST_HEAD(tmp_list);
>
> - list_for_each_entry_safe(dev, tmp, head, unreg_list)
> + list_for_each_entry_safe(dev, tmp, head, unreg_list) {
> if (!(dev->flags & IFF_UP))
> list_move(&dev->unreg_list, &tmp_list);
> + else
> + __dev_pre_close(dev);
> + }
>
> __dev_close_many(head);
The return value is ignored here.
And this is called from dev_close(), where you are adding the
notification as well. So the notifier will usually be called twice.
[...]
> @@ -1397,6 +1418,7 @@ rollback:
> break;
>
> if (dev->flags & IFF_UP) {
> + nb->notifier_call(nb, NETDEV_PRE_DOWN, dev);
> nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
> nb->notifier_call(nb, NETDEV_DOWN, dev);
> }
[...]
The return value has to be ignored here. Not sure it makes any sense to
call the notifier at all.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing
2011-08-31 15:25 ` Ben Hutchings
@ 2011-08-31 15:53 ` Stephen Hemminger
2011-08-31 16:22 ` Jiri Pirko
2011-08-31 16:21 ` Jiri Pirko
1 sibling, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2011-08-31 15:53 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Jiri Pirko, netdev, davem, eric.dumazet
On Wed, 31 Aug 2011 16:25:51 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Wed, 2011-08-31 at 17:15 +0200, Jiri Pirko wrote:
> > In some situations, like when the device is used as slave device in
> > bond/br/etc it is not nice if someone closes the device. This allows
> > it's masters to forbid this closure.
>
> No it doesn't.
>
> [...]
> > @@ -1269,9 +1282,12 @@ static int dev_close_many(struct list_head *head)
> > struct net_device *dev, *tmp;
> > LIST_HEAD(tmp_list);
> >
> > - list_for_each_entry_safe(dev, tmp, head, unreg_list)
> > + list_for_each_entry_safe(dev, tmp, head, unreg_list) {
> > if (!(dev->flags & IFF_UP))
> > list_move(&dev->unreg_list, &tmp_list);
> > + else
> > + __dev_pre_close(dev);
> > + }
> >
> > __dev_close_many(head);
>
> The return value is ignored here.
>
> And this is called from dev_close(), where you are adding the
> notification as well. So the notifier will usually be called twice.
>
> [...]
> > @@ -1397,6 +1418,7 @@ rollback:
> > break;
> >
> > if (dev->flags & IFF_UP) {
> > + nb->notifier_call(nb, NETDEV_PRE_DOWN, dev);
> > nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
> > nb->notifier_call(nb, NETDEV_DOWN, dev);
> > }
> [...]
>
> The return value has to be ignored here. Not sure it makes any sense to
> call the notifier at all.
>
> Ben.
>
Also we need to allow rmmod'ing a network device even it is
part of a bridge and that implicitly
calls close.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing
2011-08-31 15:53 ` Stephen Hemminger
@ 2011-08-31 16:22 ` Jiri Pirko
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Pirko @ 2011-08-31 16:22 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ben Hutchings, netdev, davem, eric.dumazet
Wed, Aug 31, 2011 at 05:53:38PM CEST, shemminger@vyatta.com wrote:
>On Wed, 31 Aug 2011 16:25:51 +0100
>Ben Hutchings <bhutchings@solarflare.com> wrote:
>
>> On Wed, 2011-08-31 at 17:15 +0200, Jiri Pirko wrote:
>> > In some situations, like when the device is used as slave device in
>> > bond/br/etc it is not nice if someone closes the device. This allows
>> > it's masters to forbid this closure.
>>
>> No it doesn't.
>>
>> [...]
>> > @@ -1269,9 +1282,12 @@ static int dev_close_many(struct list_head *head)
>> > struct net_device *dev, *tmp;
>> > LIST_HEAD(tmp_list);
>> >
>> > - list_for_each_entry_safe(dev, tmp, head, unreg_list)
>> > + list_for_each_entry_safe(dev, tmp, head, unreg_list) {
>> > if (!(dev->flags & IFF_UP))
>> > list_move(&dev->unreg_list, &tmp_list);
>> > + else
>> > + __dev_pre_close(dev);
>> > + }
>> >
>> > __dev_close_many(head);
>>
>> The return value is ignored here.
>>
>> And this is called from dev_close(), where you are adding the
>> notification as well. So the notifier will usually be called twice.
>>
>> [...]
>> > @@ -1397,6 +1418,7 @@ rollback:
>> > break;
>> >
>> > if (dev->flags & IFF_UP) {
>> > + nb->notifier_call(nb, NETDEV_PRE_DOWN, dev);
>> > nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
>> > nb->notifier_call(nb, NETDEV_DOWN, dev);
>> > }
>> [...]
>>
>> The return value has to be ignored here. Not sure it makes any sense to
>> call the notifier at all.
>>
>> Ben.
>>
>
>Also we need to allow rmmod'ing a network device even it is
>part of a bridge and that implicitly
>calls close.
this is not a problem because when it's called from
rollback_registered_many, return value if pre_down is ignored.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing
2011-08-31 15:25 ` Ben Hutchings
2011-08-31 15:53 ` Stephen Hemminger
@ 2011-08-31 16:21 ` Jiri Pirko
1 sibling, 0 replies; 6+ messages in thread
From: Jiri Pirko @ 2011-08-31 16:21 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, davem, eric.dumazet, shemminger
Wed, Aug 31, 2011 at 05:25:51PM CEST, bhutchings@solarflare.com wrote:
>On Wed, 2011-08-31 at 17:15 +0200, Jiri Pirko wrote:
>> In some situations, like when the device is used as slave device in
>> bond/br/etc it is not nice if someone closes the device. This allows
>> it's masters to forbid this closure.
>
>No it doesn't.
It does
>
>[...]
>> @@ -1269,9 +1282,12 @@ static int dev_close_many(struct list_head *head)
>> struct net_device *dev, *tmp;
>> LIST_HEAD(tmp_list);
>>
>> - list_for_each_entry_safe(dev, tmp, head, unreg_list)
>> + list_for_each_entry_safe(dev, tmp, head, unreg_list) {
>> if (!(dev->flags & IFF_UP))
>> list_move(&dev->unreg_list, &tmp_list);
>> + else
>> + __dev_pre_close(dev);
>> + }
>>
>> __dev_close_many(head);
>
>The return value is ignored here.
That's intended. The reason is this is called from
rollback_registered_many - refuse should be ignored in that case
>
>And this is called from dev_close(), where you are adding the
>notification as well. So the notifier will usually be called twice.
>
Indeed. Anyway I thought about it and we probably do not need this patch
as Stephen said.
>[...]
>> @@ -1397,6 +1418,7 @@ rollback:
>> break;
>>
>> if (dev->flags & IFF_UP) {
>> + nb->notifier_call(nb, NETDEV_PRE_DOWN, dev);
>> nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
>> nb->notifier_call(nb, NETDEV_DOWN, dev);
>> }
>[...]
>
>The return value has to be ignored here. Not sure it makes any sense to
>call the notifier at all.
>
>Ben.
>
>--
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-31 16:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-31 15:15 [patch net-next-2.6] net: allow notifier subscribers to forbid device from closing Jiri Pirko
2011-08-31 15:20 ` Stephen Hemminger
2011-08-31 15:25 ` Ben Hutchings
2011-08-31 15:53 ` Stephen Hemminger
2011-08-31 16:22 ` Jiri Pirko
2011-08-31 16:21 ` Jiri Pirko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox