[patch net-next RFC v2 0/2] ipv6: allow temporary address management for user-created addresses

[patch net-next RFC v2 0/2] ipv6: allow temporary address management for user-created addresses

[Jiri Pirko]

Both kernel patches were modified, see details in each patch description.
No modification to iproute2 patches was done.

Jiri Pirko (2):
  ipv6 addrconf: extend ifa_flags to u32
  ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage
    temporary addresses

 include/net/addrconf.h       |   4 +-
 include/net/if_inet6.h       |   2 +-
 include/uapi/linux/if_addr.h |   5 ++
 net/ipv6/addrconf.c          | 197 +++++++++++++++++++++++++------------------
 4 files changed, 122 insertions(+), 86 deletions(-)

-- 
1.8.3.1

[patch net-next RFC v2 1/2] ipv6 addrconf: extend ifa_flags to u32

[Jiri Pirko]

There is no more space in u8 ifa_flags. So do what davem suffested and
add another netlink attr called IFA_FLAGS for carry more flags.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
v1->v2:
- reordered struct inet6_ifaddr as suggested by David Laight
- changed flags type to u32 on couple of places as suggested by Thomas Haller
- changed output of if6_seq_show as suggested by Thomas Haller

 include/net/addrconf.h       |  4 ++--
 include/net/if_inet6.h       |  2 +-
 include/uapi/linux/if_addr.h |  4 ++++
 net/ipv6/addrconf.c          | 32 ++++++++++++++++++++------------
 4 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 86505bf..e70278e 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -81,9 +81,9 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dev,
 		       const struct in6_addr *daddr, unsigned int srcprefs,
 		       struct in6_addr *saddr);
 int __ipv6_get_lladdr(struct inet6_dev *idev, struct in6_addr *addr,
-		      unsigned char banned_flags);
+		      u32 banned_flags);
 int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr,
-		    unsigned char banned_flags);
+		    u32 banned_flags);
 int ipv6_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2);
 void addrconf_join_solict(struct net_device *dev, const struct in6_addr *addr);
 void addrconf_leave_solict(struct inet6_dev *idev, const struct in6_addr *addr);
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 76d5427..b58c36c 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -50,8 +50,8 @@ struct inet6_ifaddr {
 
 	int			state;
 
+	__u32			flags;
 	__u8			dad_probes;
-	__u8			flags;
 
 	__u16			scope;
 
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index 23357ab..8ab0c2c 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -18,6 +18,9 @@ struct ifaddrmsg {
  * It makes no difference for normally configured broadcast interfaces,
  * but for point-to-point IFA_ADDRESS is DESTINATION address,
  * local address is supplied in IFA_LOCAL attribute.
+ *
+ * IFA_FLAGS is a u32 attribute that extends the u8 field ifa_flags.
+ * If present, the value from struct ifaddrmsg will be ignored.
  */
 enum {
 	IFA_UNSPEC,
@@ -28,6 +31,7 @@ enum {
 	IFA_ANYCAST,
 	IFA_CACHEINFO,
 	IFA_MULTICAST,
+	IFA_FLAGS,
 	__IFA_MAX,
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 542d095..c94da31 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1393,7 +1393,7 @@ try_nextdev:
 EXPORT_SYMBOL(ipv6_dev_get_saddr);
 
 int __ipv6_get_lladdr(struct inet6_dev *idev, struct in6_addr *addr,
-		      unsigned char banned_flags)
+		      u32 banned_flags)
 {
 	struct inet6_ifaddr *ifp;
 	int err = -EADDRNOTAVAIL;
@@ -1410,7 +1410,7 @@ int __ipv6_get_lladdr(struct inet6_dev *idev, struct in6_addr *addr,
 }
 
 int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr,
-		    unsigned char banned_flags)
+		    u32 banned_flags)
 {
 	struct inet6_dev *idev;
 	int err = -EADDRNOTAVAIL;
@@ -2181,7 +2181,7 @@ ok:
 		}
 
 		if (ifp) {
-			int flags;
+			u32 flags;
 			unsigned long now;
 			struct inet6_ifaddr *ift;
 			u32 stored_lft;
@@ -2366,10 +2366,11 @@ err_exit:
 /*
  *	Manual configuration of address on an interface
  */
-static int inet6_addr_add(struct net *net, int ifindex, const struct in6_addr *pfx,
+static int inet6_addr_add(struct net *net, int ifindex,
+			  const struct in6_addr *pfx,
 			  const struct in6_addr *peer_pfx,
-			  unsigned int plen, __u8 ifa_flags, __u32 prefered_lft,
-			  __u32 valid_lft)
+			  unsigned int plen, __u32 ifa_flags,
+			  __u32 prefered_lft, __u32 valid_lft)
 {
 	struct inet6_ifaddr *ifp;
 	struct inet6_dev *idev;
@@ -3357,7 +3358,7 @@ static void if6_seq_stop(struct seq_file *seq, void *v)
 static int if6_seq_show(struct seq_file *seq, void *v)
 {
 	struct inet6_ifaddr *ifp = (struct inet6_ifaddr *)v;
-	seq_printf(seq, "%pi6 %02x %02x %02x %02x %8s\n",
+	seq_printf(seq, "%pi6 %02x %02x %02x %03x %8s\n",
 		   &ifp->addr,
 		   ifp->idev->dev->ifindex,
 		   ifp->prefix_len,
@@ -3578,6 +3579,7 @@ static const struct nla_policy ifa_ipv6_policy[IFA_MAX+1] = {
 	[IFA_ADDRESS]		= { .len = sizeof(struct in6_addr) },
 	[IFA_LOCAL]		= { .len = sizeof(struct in6_addr) },
 	[IFA_CACHEINFO]		= { .len = sizeof(struct ifa_cacheinfo) },
+	[IFA_FLAGS]		= { .len = sizeof(u32) },
 };
 
 static int
@@ -3601,7 +3603,7 @@ inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh)
 	return inet6_addr_del(net, ifm->ifa_index, pfx, ifm->ifa_prefixlen);
 }
 
-static int inet6_addr_modify(struct inet6_ifaddr *ifp, u8 ifa_flags,
+static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 			     u32 prefered_lft, u32 valid_lft)
 {
 	u32 flags;
@@ -3656,7 +3658,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
 	struct inet6_ifaddr *ifa;
 	struct net_device *dev;
 	u32 valid_lft = INFINITY_LIFE_TIME, preferred_lft = INFINITY_LIFE_TIME;
-	u8 ifa_flags;
+	u32 ifa_flags;
 	int err;
 
 	err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, ifa_ipv6_policy);
@@ -3683,8 +3685,10 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
 	if (dev == NULL)
 		return -ENODEV;
 
+	ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
+
 	/* We ignore other flags so far. */
-	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
+	ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS;
 
 	ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
 	if (ifa == NULL) {
@@ -3708,7 +3712,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
 	return err;
 }
 
-static void put_ifaddrmsg(struct nlmsghdr *nlh, u8 prefixlen, u8 flags,
+static void put_ifaddrmsg(struct nlmsghdr *nlh, u8 prefixlen, u32 flags,
 			  u8 scope, int ifindex)
 {
 	struct ifaddrmsg *ifm;
@@ -3751,7 +3755,8 @@ static inline int inet6_ifaddr_msgsize(void)
 	return NLMSG_ALIGN(sizeof(struct ifaddrmsg))
 	       + nla_total_size(16) /* IFA_LOCAL */
 	       + nla_total_size(16) /* IFA_ADDRESS */
-	       + nla_total_size(sizeof(struct ifa_cacheinfo));
+	       + nla_total_size(sizeof(struct ifa_cacheinfo))
+	       + nla_total_size(4)  /* IFA_FLAGS */;
 }
 
 static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
@@ -3799,6 +3804,9 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
 	if (put_cacheinfo(skb, ifa->cstamp, ifa->tstamp, preferred, valid) < 0)
 		goto error;
 
+	if (nla_put_u32(skb, IFA_FLAGS, ifa->flags) < 0)
+		goto error;
+
 	return nlmsg_end(skb, nlh);
 
 error:
-- 
1.8.3.1

Re: [patch net-next RFC v2 1/2] ipv6 addrconf: extend ifa_flags to u32

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 19 Nov 2013 11:43:28 +0100

> There is no more space in u8 ifa_flags. So do what davem suffested and
> add another netlink attr called IFA_FLAGS for carry more flags.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Thomas Haller <thaller@redhat.com>
> ---
> v1->v2:
> - reordered struct inet6_ifaddr as suggested by David Laight
> - changed flags type to u32 on couple of places as suggested by Thomas Haller
> - changed output of if6_seq_show as suggested by Thomas Haller

This looks fine, but I would suggest to extend this to all addressing
types using ifm->ifa_flags.  It is not such a big deal, there are only
3 or 4.

DecNET, ipv4, ipv6, and lastly phonet (which just reports "permanent"
flag on get/dump).

That way tools like iproute2 can just generically provide and
interpret IFA_FLAGS attribute unconditionally.

Re: [patch net-next RFC v2 1/2] ipv6 addrconf: extend ifa_flags to u32

[Jiri Pirko]

Tue, Nov 19, 2013 at 10:16:53PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Tue, 19 Nov 2013 11:43:28 +0100
>
>> There is no more space in u8 ifa_flags. So do what davem suffested and
>> add another netlink attr called IFA_FLAGS for carry more flags.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> Signed-off-by: Thomas Haller <thaller@redhat.com>
>> ---
>> v1->v2:
>> - reordered struct inet6_ifaddr as suggested by David Laight
>> - changed flags type to u32 on couple of places as suggested by Thomas Haller
>> - changed output of if6_seq_show as suggested by Thomas Haller
>
>This looks fine, but I would suggest to extend this to all addressing
>types using ifm->ifa_flags.  It is not such a big deal, there are only
>3 or 4.

Okay - I will do that.

>
>DecNET, ipv4, ipv6, and lastly phonet (which just reports "permanent"
>flag on get/dump).
>
>That way tools like iproute2 can just generically provide and
>interpret IFA_FLAGS attribute unconditionally.

Well the check and ->ifa_flags fallback needs to be in iproute2 anyway
in order to maintain compatibility with older kernels.

Thanks for review!

Jiri

[patch net-next RFC v2 2/2] ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses

[Jiri Pirko]

Creating an address with this flag set will result in kernel taking care
of temporary addresses in the same way as if the address was created by
kernel itself (after RA receive). This allows userspace applications
implementing the autoconfiguration (NetworkManager for example) to
implement ipv6 addresses privacy.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
v1->v2:
- Added possibility to change IFA_F_MANAGETEMPADDR flag as suggested by Thomas Haller

 include/uapi/linux/if_addr.h |   1 +
 net/ipv6/addrconf.c          | 167 ++++++++++++++++++++++++-------------------
 2 files changed, 96 insertions(+), 72 deletions(-)

diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index 8ab0c2c..cfed10b 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -48,6 +48,7 @@ enum {
 #define IFA_F_DEPRECATED	0x20
 #define IFA_F_TENTATIVE		0x40
 #define IFA_F_PERMANENT		0x80
+#define IFA_F_MANAGETEMPADDR	0x100
 
 struct ifa_cacheinfo {
 	__u32	ifa_prefered;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c94da31..a29e782 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1010,7 +1010,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
 	u32 addr_flags;
 	unsigned long now = jiffies;
 
-	write_lock(&idev->lock);
+	write_lock_bh(&idev->lock);
 	if (ift) {
 		spin_lock_bh(&ift->lock);
 		memcpy(&addr.s6_addr[8], &ift->addr.s6_addr[8], 8);
@@ -1022,7 +1022,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
 retry:
 	in6_dev_hold(idev);
 	if (idev->cnf.use_tempaddr <= 0) {
-		write_unlock(&idev->lock);
+		write_unlock_bh(&idev->lock);
 		pr_info("%s: use_tempaddr is disabled\n", __func__);
 		in6_dev_put(idev);
 		ret = -1;
@@ -1032,7 +1032,7 @@ retry:
 	if (ifp->regen_count++ >= idev->cnf.regen_max_retry) {
 		idev->cnf.use_tempaddr = -1;	/*XXX*/
 		spin_unlock_bh(&ifp->lock);
-		write_unlock(&idev->lock);
+		write_unlock_bh(&idev->lock);
 		pr_warn("%s: regeneration time exceeded - disabled temporary address support\n",
 			__func__);
 		in6_dev_put(idev);
@@ -1058,7 +1058,7 @@ retry:
 	regen_advance = idev->cnf.regen_max_retry *
 	                idev->cnf.dad_transmits *
 	                idev->nd_parms->retrans_time / HZ;
-	write_unlock(&idev->lock);
+	write_unlock_bh(&idev->lock);
 
 	/* A temporary address is created only if this calculated Preferred
 	 * Lifetime is greater than REGEN_ADVANCE time units.  In particular,
@@ -1085,7 +1085,7 @@ retry:
 		in6_dev_put(idev);
 		pr_info("%s: retry temporary address regeneration\n", __func__);
 		tmpaddr = &addr;
-		write_lock(&idev->lock);
+		write_lock_bh(&idev->lock);
 		goto retry;
 	}
 
@@ -2019,6 +2019,73 @@ static struct inet6_dev *addrconf_add_dev(struct net_device *dev)
 	return idev;
 }
 
+static void manage_tempaddrs(struct inet6_dev *idev,
+			     struct inet6_ifaddr *ifp,
+			     __u32 valid_lft, __u32 prefered_lft,
+			     bool create, unsigned long now)
+{
+	u32 flags;
+	struct inet6_ifaddr *ift;
+
+	read_lock_bh(&idev->lock);
+	/* update all temporary addresses in the list */
+	list_for_each_entry(ift, &idev->tempaddr_list, tmp_list) {
+		int age, max_valid, max_prefered;
+
+		if (ifp != ift->ifpub)
+			continue;
+
+		/* RFC 4941 section 3.3:
+		 * If a received option will extend the lifetime of a public
+		 * address, the lifetimes of temporary addresses should
+		 * be extended, subject to the overall constraint that no
+		 * temporary addresses should ever remain "valid" or "preferred"
+		 * for a time longer than (TEMP_VALID_LIFETIME) or
+		 * (TEMP_PREFERRED_LIFETIME - DESYNC_FACTOR), respectively.
+		 */
+		age = (now - ift->cstamp) / HZ;
+		max_valid = idev->cnf.temp_valid_lft - age;
+		if (max_valid < 0)
+			max_valid = 0;
+
+		max_prefered = idev->cnf.temp_prefered_lft -
+			       idev->cnf.max_desync_factor - age;
+		if (max_prefered < 0)
+			max_prefered = 0;
+
+		if (valid_lft > max_valid)
+			valid_lft = max_valid;
+
+		if (prefered_lft > max_prefered)
+			prefered_lft = max_prefered;
+
+		spin_lock(&ift->lock);
+		flags = ift->flags;
+		ift->valid_lft = valid_lft;
+		ift->prefered_lft = prefered_lft;
+		ift->tstamp = now;
+		if (prefered_lft > 0)
+			ift->flags &= ~IFA_F_DEPRECATED;
+
+		spin_unlock(&ift->lock);
+		if (!(flags&IFA_F_TENTATIVE))
+			ipv6_ifa_notify(0, ift);
+	}
+
+	if ((create || list_empty(&idev->tempaddr_list)) &&
+	    idev->cnf.use_tempaddr > 0) {
+		/* When a new public address is created as described
+		 * in [ADDRCONF], also create a new temporary address.
+		 * Also create a temporary address if it's enabled but
+		 * no temporary address currently exists.
+		 */
+		read_unlock_bh(&idev->lock);
+		ipv6_create_tempaddr(ifp, NULL);
+	} else {
+		read_unlock_bh(&idev->lock);
+	}
+}
+
 void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 {
 	struct prefix_info *pinfo;
@@ -2173,6 +2240,7 @@ ok:
 				return;
 			}
 
+			ifp->flags |= IFA_F_MANAGETEMPADDR;
 			update_lft = 0;
 			create = 1;
 			ifp->cstamp = jiffies;
@@ -2183,7 +2251,6 @@ ok:
 		if (ifp) {
 			u32 flags;
 			unsigned long now;
-			struct inet6_ifaddr *ift;
 			u32 stored_lft;
 
 			/* update lifetime (RFC2462 5.5.3 e) */
@@ -2224,70 +2291,8 @@ ok:
 			} else
 				spin_unlock(&ifp->lock);
 
-			read_lock_bh(&in6_dev->lock);
-			/* update all temporary addresses in the list */
-			list_for_each_entry(ift, &in6_dev->tempaddr_list,
-					    tmp_list) {
-				int age, max_valid, max_prefered;
-
-				if (ifp != ift->ifpub)
-					continue;
-
-				/*
-				 * RFC 4941 section 3.3:
-				 * If a received option will extend the lifetime
-				 * of a public address, the lifetimes of
-				 * temporary addresses should be extended,
-				 * subject to the overall constraint that no
-				 * temporary addresses should ever remain
-				 * "valid" or "preferred" for a time longer than
-				 * (TEMP_VALID_LIFETIME) or
-				 * (TEMP_PREFERRED_LIFETIME - DESYNC_FACTOR),
-				 * respectively.
-				 */
-				age = (now - ift->cstamp) / HZ;
-				max_valid = in6_dev->cnf.temp_valid_lft - age;
-				if (max_valid < 0)
-					max_valid = 0;
-
-				max_prefered = in6_dev->cnf.temp_prefered_lft -
-					       in6_dev->cnf.max_desync_factor -
-					       age;
-				if (max_prefered < 0)
-					max_prefered = 0;
-
-				if (valid_lft > max_valid)
-					valid_lft = max_valid;
-
-				if (prefered_lft > max_prefered)
-					prefered_lft = max_prefered;
-
-				spin_lock(&ift->lock);
-				flags = ift->flags;
-				ift->valid_lft = valid_lft;
-				ift->prefered_lft = prefered_lft;
-				ift->tstamp = now;
-				if (prefered_lft > 0)
-					ift->flags &= ~IFA_F_DEPRECATED;
-
-				spin_unlock(&ift->lock);
-				if (!(flags&IFA_F_TENTATIVE))
-					ipv6_ifa_notify(0, ift);
-			}
-
-			if ((create || list_empty(&in6_dev->tempaddr_list)) && in6_dev->cnf.use_tempaddr > 0) {
-				/*
-				 * When a new public address is created as
-				 * described in [ADDRCONF], also create a new
-				 * temporary address. Also create a temporary
-				 * address if it's enabled but no temporary
-				 * address currently exists.
-				 */
-				read_unlock_bh(&in6_dev->lock);
-				ipv6_create_tempaddr(ifp, NULL);
-			} else {
-				read_unlock_bh(&in6_dev->lock);
-			}
+			manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
+					 create, now);
 
 			in6_ifa_put(ifp);
 			addrconf_verify(0);
@@ -2429,6 +2434,9 @@ static int inet6_addr_add(struct net *net, int ifindex,
 		 * manually configured addresses
 		 */
 		addrconf_dad_start(ifp);
+		if (ifa_flags & IFA_F_MANAGETEMPADDR)
+			manage_tempaddrs(idev, ifp, valid_lft, prefered_lft,
+					 true, jiffies);
 		in6_ifa_put(ifp);
 		addrconf_verify(0);
 		return 0;
@@ -3609,6 +3617,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	u32 flags;
 	clock_t expires;
 	unsigned long timeout;
+	bool was_managetempaddr;
 
 	if (!valid_lft || (prefered_lft > valid_lft))
 		return -EINVAL;
@@ -3632,7 +3641,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	}
 
 	spin_lock_bh(&ifp->lock);
-	ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS)) | ifa_flags;
+	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
+	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
+			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
+	ifp->flags |= ifa_flags;
 	ifp->tstamp = jiffies;
 	ifp->valid_lft = valid_lft;
 	ifp->prefered_lft = prefered_lft;
@@ -3643,6 +3655,14 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 
 	addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
 			      expires, flags);
+
+	if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
+		if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR))
+			valid_lft = prefered_lft = 0;
+		manage_tempaddrs(ifp->idev, ifp, valid_lft, prefered_lft,
+				 false, jiffies);
+	}
+
 	addrconf_verify(0);
 
 	return 0;
@@ -3687,8 +3707,11 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
 
 	ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
 
+	if (ifa_flags & IFA_F_MANAGETEMPADDR && ifm->ifa_prefixlen != 64)
+		return -EINVAL;
+
 	/* We ignore other flags so far. */
-	ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS;
+	ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR;
 
 	ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
 	if (ifa == NULL) {
-- 
1.8.3.1

Re: [patch net-next RFC v2 2/2] ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses

[Hannes Frederic Sowa]

On Tue, Nov 19, 2013 at 11:43:29AM +0100, Jiri Pirko wrote:
> Creating an address with this flag set will result in kernel taking care
> of temporary addresses in the same way as if the address was created by
> kernel itself (after RA receive). This allows userspace applications
> implementing the autoconfiguration (NetworkManager for example) to
> implement ipv6 addresses privacy.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Thomas Haller <thaller@redhat.com>

The patch looks good.

Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

The _bh conversion seems not to be strictly necessary, no?
ipv6_create_tempaddr is always called with bh disabled.

Thanks,

  Hannes

Re: [patch net-next RFC v2 2/2] ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses

[Jiri Pirko]

Wed, Nov 20, 2013 at 05:05:43PM CET, hannes@stressinduktion.org wrote:
>On Tue, Nov 19, 2013 at 11:43:29AM +0100, Jiri Pirko wrote:
>> Creating an address with this flag set will result in kernel taking care
>> of temporary addresses in the same way as if the address was created by
>> kernel itself (after RA receive). This allows userspace applications
>> implementing the autoconfiguration (NetworkManager for example) to
>> implement ipv6 addresses privacy.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> Signed-off-by: Thomas Haller <thaller@redhat.com>
>
>The patch looks good.
>
>Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
>The _bh conversion seems not to be strictly necessary, no?
>ipv6_create_tempaddr is always called with bh disabled.

It is needed. Found out using lock debug config option.

>
>Thanks,
>
>  Hannes
>

Re: [patch net-next RFC v2 2/2] ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses

[Hannes Frederic Sowa]

On Wed, Nov 20, 2013 at 05:18:38PM +0100, Jiri Pirko wrote:
> Wed, Nov 20, 2013 at 05:05:43PM CET, hannes@stressinduktion.org wrote:
> >On Tue, Nov 19, 2013 at 11:43:29AM +0100, Jiri Pirko wrote:
> >> Creating an address with this flag set will result in kernel taking care
> >> of temporary addresses in the same way as if the address was created by
> >> kernel itself (after RA receive). This allows userspace applications
> >> implementing the autoconfiguration (NetworkManager for example) to
> >> implement ipv6 addresses privacy.
> >> 
> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >> Signed-off-by: Thomas Haller <thaller@redhat.com>
> >
> >The patch looks good.
> >
> >Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> >
> >The _bh conversion seems not to be strictly necessary, no?
> >ipv6_create_tempaddr is always called with bh disabled.
> 
> It is needed. Found out using lock debug config option.

Yes, I can see it now, too. ;)

Re: [patch net-next RFC v2 2/2] ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses

[Thomas Haller]

[-- Attachment #1: Type: text/plain, Size: 1923 bytes --]

Hi Jiri,


just two comments:


>  		addrconf_verify(0);
>  		return 0;
> @@ -3609,6 +3617,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
>  	u32 flags;
>  	clock_t expires;
>  	unsigned long timeout;
> +	bool was_managetempaddr;
>  
>  	if (!valid_lft || (prefered_lft > valid_lft))
>  		return -EINVAL;
> @@ -3632,7 +3641,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
>  	}
>  
>  	spin_lock_bh(&ifp->lock);


We should make sure here, not to allow IFA_F_MANAGETEMPADDR, if ifp has
prefixlen!=64 or is IFA_F_TEMPORARY itself. Note, that inet6_rtm_newaddr
does check for ifm->ifa_prefixlen != 64, but when modifying an existing
address, the ifm->ifa_prefixlen is not relevant.


> -	ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS)) | ifa_flags;
> +	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
> +	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
> +			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
> +	ifp->flags |= ifa_flags;
>  	ifp->tstamp = jiffies;
>  	ifp->valid_lft = valid_lft;
>  	ifp->prefered_lft = prefered_lft;
> @@ -3643,6 +3655,14 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
>  
>  	addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
>  			      expires, flags);
> +
> +	if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
> +		if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR))
> +			valid_lft = prefered_lft = 0;
> +		manage_tempaddrs(ifp->idev, ifp, valid_lft, prefered_lft,
> +				 false, jiffies);

I think, the parameter "create" of manage_tempaddrs should be set to
"!was_managetempaddr", because we want to force creation of a new
temporary address, if ifp just gets the flag IFA_F_MANAGETEMPADDR
(without having it before).





Thomas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[patch iproute2 RFC v2 1/2] add support for extended ifa_flags

[Jiri Pirko]

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/if_addr.h |  1 +
 ip/ipaddress.c          | 44 ++++++++++++++++++++++++++++++--------------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/include/linux/if_addr.h b/include/linux/if_addr.h
index 58b39f4..cced59f 100644
--- a/include/linux/if_addr.h
+++ b/include/linux/if_addr.h
@@ -28,6 +28,7 @@ enum {
 	IFA_ANYCAST,
 	IFA_CACHEINFO,
 	IFA_MULTICAST,
+	IFA_FLAGS,
 	__IFA_MAX,
 };
 
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 1c3e4da..59dbd71 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -541,6 +541,13 @@ static int set_lifetime(unsigned int *lifetime, char *argv)
 	return 0;
 }
 
+static unsigned int get_ifa_flags(struct ifaddrmsg *ifa,
+				  struct rtattr *ifa_flags_attr)
+{
+	return ifa_flags_attr ? rta_getattr_u32(ifa_flags_attr) :
+				ifa->ifa_flags;
+}
+
 int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		   void *arg)
 {
@@ -567,6 +574,8 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 
 	parse_rtattr(rta_tb, IFA_MAX, IFA_RTA(ifa), n->nlmsg_len - NLMSG_LENGTH(sizeof(*ifa)));
 
+	ifa_flags = get_ifa_flags(ifa, rta_tb[IFA_FLAGS]);
+
 	if (!rta_tb[IFA_LOCAL])
 		rta_tb[IFA_LOCAL] = rta_tb[IFA_ADDRESS];
 	if (!rta_tb[IFA_ADDRESS])
@@ -576,7 +585,7 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		return 0;
 	if ((filter.scope^ifa->ifa_scope)&filter.scopemask)
 		return 0;
-	if ((filter.flags^ifa->ifa_flags)&filter.flagmask)
+	if ((filter.flags ^ ifa_flags) & filter.flagmask)
 		return 0;
 	if (filter.label) {
 		SPRINT_BUF(b1);
@@ -670,36 +679,35 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 				    abuf, sizeof(abuf)));
 	}
 	fprintf(fp, "scope %s ", rtnl_rtscope_n2a(ifa->ifa_scope, b1, sizeof(b1)));
-	ifa_flags = ifa->ifa_flags;
-	if (ifa->ifa_flags&IFA_F_SECONDARY) {
+	if (ifa_flags & IFA_F_SECONDARY) {
 		ifa_flags &= ~IFA_F_SECONDARY;
 		if (ifa->ifa_family == AF_INET6)
 			fprintf(fp, "temporary ");
 		else
 			fprintf(fp, "secondary ");
 	}
-	if (ifa->ifa_flags&IFA_F_TENTATIVE) {
+	if (ifa_flags & IFA_F_TENTATIVE) {
 		ifa_flags &= ~IFA_F_TENTATIVE;
 		fprintf(fp, "tentative ");
 	}
-	if (ifa->ifa_flags&IFA_F_DEPRECATED) {
+	if (ifa_flags & IFA_F_DEPRECATED) {
 		ifa_flags &= ~IFA_F_DEPRECATED;
 		deprecated = 1;
 		fprintf(fp, "deprecated ");
 	}
-	if (ifa->ifa_flags&IFA_F_HOMEADDRESS) {
+	if (ifa_flags & IFA_F_HOMEADDRESS) {
 		ifa_flags &= ~IFA_F_HOMEADDRESS;
 		fprintf(fp, "home ");
 	}
-	if (ifa->ifa_flags&IFA_F_NODAD) {
+	if (ifa_flags & IFA_F_NODAD) {
 		ifa_flags &= ~IFA_F_NODAD;
 		fprintf(fp, "nodad ");
 	}
-	if (!(ifa->ifa_flags&IFA_F_PERMANENT)) {
+	if (!(ifa_flags & IFA_F_PERMANENT)) {
 		fprintf(fp, "dynamic ");
 	} else
 		ifa_flags &= ~IFA_F_PERMANENT;
-	if (ifa->ifa_flags&IFA_F_DADFAILED) {
+	if (ifa_flags & IFA_F_DADFAILED) {
 		ifa_flags &= ~IFA_F_DADFAILED;
 		fprintf(fp, "dadfailed ");
 	}
@@ -926,6 +934,8 @@ static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
 		for (a = ainfo->head; a; a = a->next) {
 			struct nlmsghdr *n = &a->h;
 			struct ifaddrmsg *ifa = NLMSG_DATA(n);
+			struct rtattr *tb[IFA_MAX + 1];
+			unsigned int ifa_flags;
 
 			if (ifa->ifa_index != ifi->ifi_index)
 				continue;
@@ -934,11 +944,13 @@ static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
 				continue;
 			if ((filter.scope^ifa->ifa_scope)&filter.scopemask)
 				continue;
-			if ((filter.flags^ifa->ifa_flags)&filter.flagmask)
+
+			parse_rtattr(tb, IFA_MAX, IFA_RTA(ifa), IFA_PAYLOAD(n));
+			ifa_flags = get_ifa_flags(ifa, tb[IFA_FLAGS]);
+
+			if ((filter.flags ^ ifa_flags) & filter.flagmask)
 				continue;
 			if (filter.pfx.family || filter.label) {
-				struct rtattr *tb[IFA_MAX+1];
-				parse_rtattr(tb, IFA_MAX, IFA_RTA(ifa), IFA_PAYLOAD(n));
 				if (!tb[IFA_LOCAL])
 					tb[IFA_LOCAL] = tb[IFA_ADDRESS];
 
@@ -1252,6 +1264,7 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 	__u32 preferred_lft = INFINITY_LIFE_TIME;
 	__u32 valid_lft = INFINITY_LIFE_TIME;
 	struct ifa_cacheinfo cinfo;
+	unsigned int ifa_flags = 0;
 
 	memset(&req, 0, sizeof(req));
 
@@ -1329,9 +1342,9 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 			if (set_lifetime(&preferred_lft, *argv))
 				invarg("preferred_lft value", *argv);
 		} else if (strcmp(*argv, "home") == 0) {
-			req.ifa.ifa_flags |= IFA_F_HOMEADDRESS;
+			ifa_flags |= IFA_F_HOMEADDRESS;
 		} else if (strcmp(*argv, "nodad") == 0) {
-			req.ifa.ifa_flags |= IFA_F_NODAD;
+			ifa_flags |= IFA_F_NODAD;
 		} else {
 			if (strcmp(*argv, "local") == 0) {
 				NEXT_ARG();
@@ -1349,6 +1362,9 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 		}
 		argc--; argv++;
 	}
+	req.ifa.ifa_flags = ifa_flags & 0xff;
+	addattr32(&req.n, sizeof(req), IFA_FLAGS, ifa_flags);
+
 	if (d == NULL) {
 		fprintf(stderr, "Not enough information: \"dev\" argument is required.\n");
 		return -1;
-- 
1.8.3.1

[patch iproute2 RFC v2 2/2] add support for IFA_F_MANAGETEMPADDR

[Jiri Pirko]

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/if_addr.h | 1 +
 ip/ipaddress.c          | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/linux/if_addr.h b/include/linux/if_addr.h
index cced59f..e1e95ce 100644
--- a/include/linux/if_addr.h
+++ b/include/linux/if_addr.h
@@ -45,6 +45,7 @@ enum {
 #define IFA_F_DEPRECATED	0x20
 #define IFA_F_TENTATIVE		0x40
 #define IFA_F_PERMANENT		0x80
+#define IFA_F_MANAGETEMPADDR	0x0100
 
 struct ifa_cacheinfo {
 	__u32	ifa_prefered;
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 59dbd71..fa6417d 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -703,6 +703,10 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		ifa_flags &= ~IFA_F_NODAD;
 		fprintf(fp, "nodad ");
 	}
+	if (ifa_flags & IFA_F_MANAGETEMPADDR) {
+		ifa_flags &= ~IFA_F_MANAGETEMPADDR;
+		fprintf(fp, "mngtmpaddr ");
+	}
 	if (!(ifa_flags & IFA_F_PERMANENT)) {
 		fprintf(fp, "dynamic ");
 	} else
@@ -1345,6 +1349,8 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 			ifa_flags |= IFA_F_HOMEADDRESS;
 		} else if (strcmp(*argv, "nodad") == 0) {
 			ifa_flags |= IFA_F_NODAD;
+		} else if (strcmp(*argv, "mngtmpaddr") == 0) {
+			ifa_flags |= IFA_F_MANAGETEMPADDR;
 		} else {
 			if (strcmp(*argv, "local") == 0) {
 				NEXT_ARG();
-- 
1.8.3.1