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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 27 May 2013 13:18:52 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	Timo Teräs <timo.teras@....fi>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/6] net: inform NETDEV_CHANGE callbacks which
 flags were changed

Mon, May 27, 2013 at 01:16:11PM CEST, timo.teras@....fi 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 flags_changed or
>> old_flags would be preferable to defining extra transient flags.
>
>Signed-off-by: Timo Teräs <timo.teras@....fi>
>Acked-by: Ben Hutchings <bhutchings@...arflare.com>
>---
> 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@...r.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html