Message ID | 1369303109-12003-1-git-send-email-timo.teras@iki.fi |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, 2013-05-23 at 12:58 +0300, Timo Teräs wrote: > In certain cases (like the follow up commit to arp.c) will need to > check which flags actually changed to avoid excessive work. > > Ben Hutchings nicely worded why to put these transient flags to > struct net_device for the time being: > > It's inelegant to put transient data associated with an event in a > > persistent data structure. On the other hand, having every user cache > > the old state is pretty awful as well. > > > > Really, netdev notifiers should be changed to accept a structure that > > encapsulates the changes rather than just a pointer to the net_device. > > But making such a change would be an enormous pain and error-prone > > because notifier functions aren't type-safe. > > > > As an interim solution, I think either the general priv_flags_changed or > > old_priv_flags would be preferable to defining extra transient flags. > > Signed-off-by: Timo Teräs <timo.teras@iki.fi> > Cc: Ben Hutchings <bhutchings@solarflare.com> Could you delete the two instances of 'priv_' from the quoted text? I was confused about to which flags field was being changed. With that, you may add 'Acked-by: Ben Hutchings <bhutchings@solarflare.com>' Ben. > --- > include/linux/netdevice.h | 4 +++- > net/core/dev.c | 5 ++++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 7dd535d..86c155a 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -1112,7 +1112,9 @@ struct net_device { > /* Hardware header description */ > const struct header_ops *header_ops; > > - unsigned int flags; /* interface flags (a la BSD) */ > + unsigned int flags; /* interface flags (a la BSD) */ > + unsigned int flags_changed; /* flags that are being changed > + * valid during NETDEV_CHANGE notifier */ > unsigned int priv_flags; /* Like 'flags' but invisible to userspace. > * See if.h for definitions. */ > unsigned short gflags; > diff --git a/net/core/dev.c b/net/core/dev.c > index 7229bc3..221634f 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -4741,8 +4741,11 @@ void __dev_notify_flags(struct net_device *dev, unsigned int old_flags) > } > > if (dev->flags & IFF_UP && > - (changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE))) > + (changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE))) { > + dev->flags_changed = changes; > call_netdevice_notifiers(NETDEV_CHANGE, dev); > + dev->flags_changed = 0; > + } > } > > /**
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 7dd535d..86c155a 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1112,7 +1112,9 @@ struct net_device { /* Hardware header description */ const struct header_ops *header_ops; - unsigned int flags; /* interface flags (a la BSD) */ + unsigned int flags; /* interface flags (a la BSD) */ + unsigned int flags_changed; /* flags that are being changed + * valid during NETDEV_CHANGE notifier */ unsigned int priv_flags; /* Like 'flags' but invisible to userspace. * See if.h for definitions. */ unsigned short gflags; diff --git a/net/core/dev.c b/net/core/dev.c index 7229bc3..221634f 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4741,8 +4741,11 @@ void __dev_notify_flags(struct net_device *dev, unsigned int old_flags) } if (dev->flags & IFF_UP && - (changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE))) + (changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE))) { + dev->flags_changed = changes; call_netdevice_notifiers(NETDEV_CHANGE, dev); + dev->flags_changed = 0; + } } /**