[PATCH] ipv6: make the net.ipv6.conf.all.use_tempaddr sysctl propagate to interface settings

[PATCH] ipv6: make the net.ipv6.conf.all.use_tempaddr sysctl propagate to interface settings

[Mathieu Trudel-Lapierre]

The description for IPV6_PRIVACY mentions using .../all/use_tempaddr to enable
IPv6 Privacy Extensions, and IP sysctl documentation mentions 'all' as setting
all interface-specific settings. We make sure at least use_tempaddr actually
works as documented.

Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
---
 net/ipv6/addrconf.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cf88df8..a20ab55 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4340,6 +4340,84 @@ int addrconf_sysctl_disable(ctl_table *ctl, int write,
 	return ret;
 }
 
+#ifdef CONFIG_IPV6_PRIVACY
+static void dev_tempaddr_change(struct inet6_dev *idev)
+{
+	if (!idev || !idev->dev)
+		return;
+
+	if (!idev->cnf.disable_ipv6) {
+		/* If ipv6 is enabled, try to bring down and back up the
+		 * interface to get new temporary addresses created
+		 */
+		addrconf_notify(NULL, NETDEV_DOWN, idev->dev);
+		addrconf_notify(NULL, NETDEV_UP, idev->dev);
+	}
+}
+
+static void addrconf_tempaddr_change(struct net *net, __s32 newf)
+{
+	struct net_device *dev;
+	struct inet6_dev *idev;
+
+	rcu_read_lock();
+	for_each_netdev_rcu(net, dev) {
+		idev = __in6_dev_get(dev);
+		if (idev) {
+			int changed = (!idev->cnf.use_tempaddr) ^ (!newf);
+			idev->cnf.use_tempaddr = newf;
+			if (changed)
+				dev_tempaddr_change(idev);
+		}
+	}
+	rcu_read_unlock();
+}
+
+static int addrconf_use_tempaddr(struct ctl_table *table, int *p, int old)
+{
+	struct net *net;
+
+	net = (struct net *)table->extra2;
+
+	if (p == &net->ipv6.devconf_dflt->use_tempaddr)
+		return 0;
+
+	if (!rtnl_trylock()) {
+		/* Restore the original values before restarting */
+		*p = old;
+		return restart_syscall();
+	}
+
+	if (p == &net->ipv6.devconf_all->use_tempaddr) {
+		__s32 newf = net->ipv6.devconf_all->use_tempaddr;
+		net->ipv6.devconf_dflt->use_tempaddr = newf;
+		addrconf_tempaddr_change(net, newf);
+	} else if ((!*p) ^ (!old))
+		dev_tempaddr_change((struct inet6_dev *)table->extra1);
+
+	rtnl_unlock();
+	return 0;
+}
+
+static
+int addrconf_sysctl_tempaddr(ctl_table *ctl, int write,
+			     void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	int *valp = ctl->data;
+	int val = *valp;
+	loff_t pos = *ppos;
+	int ret;
+
+	ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
+
+	if (write)
+		ret = addrconf_use_tempaddr(ctl, valp, val);
+	if (ret)
+		*ppos = pos;
+	return ret;
+}
+#endif
+
 static struct addrconf_sysctl_table
 {
 	struct ctl_table_header *sysctl_header;
@@ -4431,7 +4509,7 @@ static struct addrconf_sysctl_table
 			.data		= &ipv6_devconf.use_tempaddr,
 			.maxlen		= sizeof(int),
 			.mode		= 0644,
-			.proc_handler	= proc_dointvec,
+			.proc_handler	= addrconf_sysctl_tempaddr,
 		},
 		{
 			.procname	= "temp_valid_lft",
-- 
1.7.7.3

Re: [PATCH] ipv6: make the net.ipv6.conf.all.use_tempaddr sysctl propagate to interface settings

[David Miller]

From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
Date: Fri, 16 Dec 2011 16:45:33 -0500

> The description for IPV6_PRIVACY mentions using .../all/use_tempaddr to enable
> IPv6 Privacy Extensions, and IP sysctl documentation mentions 'all' as setting
> all interface-specific settings. We make sure at least use_tempaddr actually
> works as documented.
> 
> Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>

"all" settings must be in place before the device is created and comes up.

You can't just "propagate" to the existing devices when the "all"
setting is changed, because an individual device might have had it's
independent sysctl setting modified by the administrator and you'll be
smashing that.

I'm not applying this patch.

Re: [PATCH] ipv6: make the net.ipv6.conf.all.use_tempaddr sysctl propagate to interface settings

[Mathieu Trudel-Lapierre]

On Mon, Dec 19, 2011 at 4:16 PM, David Miller <davem@davemloft.net> wrote:
> "all" settings must be in place before the device is created and comes up.
>
> You can't just "propagate" to the existing devices when the "all"
> setting is changed, because an individual device might have had it's
> independent sysctl setting modified by the administrator and you'll be
> smashing that.
>
> I'm not applying this patch.

Would it be possible in this case to update the documentation to make
this clear? I'm certainly not the only one who has come up with the
question. What seems to come up often is confusion between the purpose
of "default" and that of "all". My understanding really was that
*default* is meant to be set before the devices are created and come
up.

I believe use_tempaddr is a special case in which if you want to
enable it, you'll likely want to have it set globally; at least that
seems the case for mobile systems where this appears to me as making
the most sense. Distributions may want to set a defaut for
installations being that temporary addresses are enabled by default,
which tends to be very impractical to do any other way (interfaces
don't all come up at the same time; on my system at least eth0 comes
up very early, before sysctls can be applied in userland). Dealing
with this particular issue at the distribution level, you can't know
in advance what interfaces are on the system. It seems as though
setting:

net.ipv6.conf.all.use_tempaddr=2
net.ipv6.conf.default.use_tempaddr=2

Should be sufficient to enable privacy addresses everywhere, at boot.
This wouldn't be particularly different from disabling ipv6 with the
sysctls, I think.

Besides the issue of smashing settings applied by administrators, is
there something else I'm missing?

Kind regards,

Mathieu Trudel-Lapierre <mathieu-tl@ubuntu.com>
Freenode: cyphermox, Jabber: mathieu.tl@gmail.com
4096R/EE018C93 1967 8F7D 03A1 8F38 732E  FF82 C126 33E1 EE01 8C93