| Message ID | 20100310202935.GE2834@psychotron.redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 10 Mar 2010 21:29:35 +0100 Jiri Pirko <jpirko@redhat.com> wrote: > diff --git a/net/core/dev.c b/net/core/dev.c > index bcc490c..5de4a15 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1084,9 +1084,9 @@ void netdev_state_change(struct net_device *dev) > } > EXPORT_SYMBOL(netdev_state_change); > > -void netdev_bonding_change(struct net_device *dev, unsigned long event) > +int netdev_bonding_change(struct net_device *dev, unsigned long event) > { > - call_netdevice_notifiers(event, dev); > + return call_netdevice_notifiers(event, dev); > } > EXPORT_SYMBOL(netdev_bonding_change); > I don't think all this pre-post stuff needed. Since this is general (not just bonding) how about int netdev_change_type(struct net_device *dev, int newtype) { int ret, oldtype; if (dev->flags & IFF_UP) return -EBUSY; oldtype = dev->type; dev->type = newtype; ret = call_netdevice_notifier(NETDEV_CHANGETYPE, dev); ret = notifier_to_errno(ret); if (ret) dev->type = oldtype; return ret; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Wed, Mar 10, 2010 at 11:47:10PM CET, shemminger@linux-foundation.org wrote: >On Wed, 10 Mar 2010 21:29:35 +0100 >Jiri Pirko <jpirko@redhat.com> wrote: > >> diff --git a/net/core/dev.c b/net/core/dev.c >> index bcc490c..5de4a15 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -1084,9 +1084,9 @@ void netdev_state_change(struct net_device *dev) >> } >> EXPORT_SYMBOL(netdev_state_change); >> >> -void netdev_bonding_change(struct net_device *dev, unsigned long event) >> +int netdev_bonding_change(struct net_device *dev, unsigned long event) >> { >> - call_netdevice_notifiers(event, dev); >> + return call_netdevice_notifiers(event, dev); >> } >> EXPORT_SYMBOL(netdev_bonding_change); >> > >I don't think all this pre-post stuff needed. >Since this is general (not just bonding) how about Well actually I reused NETDEV_BONDING_OLDTYPE. This and NETDEV_BONDING_NEWTYPE is used in inetdev_event() and addrconf_notify(). These functions apparently need to be notified before and after the change. > >int netdev_change_type(struct net_device *dev, int newtype) >{ > int ret, oldtype; > > if (dev->flags & IFF_UP) > return -EBUSY; > > oldtype = dev->type; > dev->type = newtype; > ret = call_netdevice_notifier(NETDEV_CHANGETYPE, dev); > ret = notifier_to_errno(ret); > if (ret) > dev->type = oldtype; > > return ret; >} I see you point here, but that would be another notifier event. Do you suggest to have 3 of them finally? NETDEV_BONDING_OLDTYPE, NETDEV_BONDING_NEWTYPE and NETDEV_CHANGETYPE. Looks a bit redundant to me... Thanks. Jirka -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thu, Mar 11, 2010 at 08:59:09AM CET, jpirko@redhat.com wrote: >Wed, Mar 10, 2010 at 11:47:10PM CET, shemminger@linux-foundation.org wrote: >>On Wed, 10 Mar 2010 21:29:35 +0100 >>Jiri Pirko <jpirko@redhat.com> wrote: >> >>> diff --git a/net/core/dev.c b/net/core/dev.c >>> index bcc490c..5de4a15 100644 >>> --- a/net/core/dev.c >>> +++ b/net/core/dev.c >>> @@ -1084,9 +1084,9 @@ void netdev_state_change(struct net_device *dev) >>> } >>> EXPORT_SYMBOL(netdev_state_change); >>> >>> -void netdev_bonding_change(struct net_device *dev, unsigned long event) >>> +int netdev_bonding_change(struct net_device *dev, unsigned long event) >>> { >>> - call_netdevice_notifiers(event, dev); >>> + return call_netdevice_notifiers(event, dev); >>> } >>> EXPORT_SYMBOL(netdev_bonding_change); >>> >> >>I don't think all this pre-post stuff needed. >>Since this is general (not just bonding) how about > >Well actually I reused NETDEV_BONDING_OLDTYPE. This and NETDEV_BONDING_NEWTYPE >is used in inetdev_event() and addrconf_notify(). These functions apparently >need to be notified before and after the change. > >> >>int netdev_change_type(struct net_device *dev, int newtype) >>{ >> int ret, oldtype; >> >> if (dev->flags & IFF_UP) >> return -EBUSY; >> >> oldtype = dev->type; >> dev->type = newtype; Also, the type change is not only about dev->type. See for example ether_setup(). This function and it's puprose might be misleading... >> ret = call_netdevice_notifier(NETDEV_CHANGETYPE, dev); >> ret = notifier_to_errno(ret); >> if (ret) >> dev->type = oldtype; >> >> return ret; >>} > >I see you point here, but that would be another notifier event. Do you suggest >to have 3 of them finally? NETDEV_BONDING_OLDTYPE, NETDEV_BONDING_NEWTYPE and >NETDEV_CHANGETYPE. Looks a bit redundant to me... > >Thanks. > >Jirka > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 7eeb187..cbe9e35 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1480,8 +1480,15 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) bond_dev->name, bond_dev->type, slave_dev->type); - netdev_bonding_change(bond_dev, - NETDEV_PRE_TYPE_CHANGE); + res = netdev_bonding_change(bond_dev, + NETDEV_PRE_TYPE_CHANGE); + res = notifier_to_errno(res); + if (res) { + pr_err("%s: refused to change device type\n", + bond_dev->name); + res = -EBUSY; + goto err_undo_flags; + } if (slave_dev->type != ARPHRD_ETHER) bond_setup_by_slave(bond_dev, slave_dev); diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index c79a88b..8cf1d50 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1975,7 +1975,7 @@ extern void __dev_addr_unsync(struct dev_addr_list **to, int *to_count, struct extern int dev_set_promiscuity(struct net_device *dev, int inc); extern int dev_set_allmulti(struct net_device *dev, int inc); extern void netdev_state_change(struct net_device *dev); -extern void netdev_bonding_change(struct net_device *dev, +extern int netdev_bonding_change(struct net_device *dev, unsigned long event); extern void netdev_features_change(struct net_device *dev); /* Load a device via the kmod */ diff --git a/net/core/dev.c b/net/core/dev.c index bcc490c..5de4a15 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1084,9 +1084,9 @@ void netdev_state_change(struct net_device *dev) } EXPORT_SYMBOL(netdev_state_change); -void netdev_bonding_change(struct net_device *dev, unsigned long event) +int netdev_bonding_change(struct net_device *dev, unsigned long event) { - call_netdevice_notifiers(event, dev); + return call_netdevice_notifiers(event, dev); } EXPORT_SYMBOL(netdev_bonding_change);
This patch adds the possibility to refuse the bonding type change for other subsystems (such as for example bridge, vlan, etc.) Signed-off-by: Jiri Pirko <jpirko@redhat.com> --- drivers/net/bonding/bond_main.c | 11 +++++++++-- include/linux/netdevice.h | 2 +- net/core/dev.c | 4 ++-- 3 files changed, 12 insertions(+), 5 deletions(-)