Thread (24 messages) 24 messages, 4 authors, 2013-06-03

Re: [PATCH net-next 1/6] net: inform NETDEV_CHANGE callbacks which flags were changed

From: Jiri Pirko <jiri@resnulli.us>
Date: 2013-05-27 11:18:58

Mon, May 27, 2013 at 01:16:11PM CEST, timo.teras@iki.fi wrote:
quoted hunk ↗ jump to hunk
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:
quoted
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 flags_changed or
old_flags would be preferable to defining extra transient flags.
Signed-off-by: Timo Teräs <redacted>
Acked-by: Ben Hutchings <redacted>
---
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 ea7b6bc..f336e03 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 50c02de..bbaa3c2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4742,8 +4742,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;
+	}
}
Hold on with this please. I'm working on netdev notifier extension. One
would be able to pass custom structure with an event after that. I plan
to post this today/tomorrow.
/**
-- 
1.8.2.3

--
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help