[PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Hannes Frederic Sowa]

This patch requires multicast interface-scoped addresses to supply a
sin6_scope_id. Because the sin6_scope_id is now also correctly used
in case of interface-scoped multicast traffic this enables one to use
interface scoped addresses over interfaces which are not targeted by the
default multicast route (the route has to be put there manually, though).

Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/af_inet6.c              |  6 +++---
 net/ipv6/datagram.c              | 18 ++++++++++--------
 net/ipv6/icmp.c                  |  2 +-
 net/ipv6/inet6_connection_sock.c |  6 ++----
 net/ipv6/raw.c                   |  9 ++++-----
 net/ipv6/udp.c                   | 14 ++++++++------
 6 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 6b793bf..f56277f 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 			struct net_device *dev = NULL;
 
 			rcu_read_lock();
-			if (addr_type & IPV6_ADDR_LINKLOCAL) {
+			if (__ipv6_addr_needs_scope_id(addr_type)) {
 				if (addr_len >= sizeof(struct sockaddr_in6) &&
 				    addr->sin6_scope_id) {
 					/* Override any existing binding, if another one
@@ -471,8 +471,8 @@ int inet6_getname(struct socket *sock, struct sockaddr *uaddr,
 
 		sin->sin6_port = inet->inet_sport;
 	}
-	if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-		sin->sin6_scope_id = sk->sk_bound_dev_if;
+	sin->sin6_scope_id = ipv6_iface_scope_id(&sin->sin6_addr,
+						 sk->sk_bound_dev_if);
 	*uaddr_len = sizeof(*sin);
 	return 0;
 }
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index f5a5478..e356022 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -124,7 +124,7 @@ ipv4_connected:
 		goto out;
 	}
 
-	if (addr_type&IPV6_ADDR_LINKLOCAL) {
+	if (__ipv6_addr_needs_scope_id(addr_type)) {
 		if (addr_len >= sizeof(struct sockaddr_in6) &&
 		    usin->sin6_scope_id) {
 			if (sk->sk_bound_dev_if &&
@@ -355,18 +355,19 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len)
 		sin->sin6_family = AF_INET6;
 		sin->sin6_flowinfo = 0;
 		sin->sin6_port = serr->port;
-		sin->sin6_scope_id = 0;
 		if (skb->protocol == htons(ETH_P_IPV6)) {
 			const struct ipv6hdr *ip6h = container_of((struct in6_addr *)(nh + serr->addr_offset),
 								  struct ipv6hdr, daddr);
 			sin->sin6_addr = ip6h->daddr;
 			if (np->sndflow)
 				sin->sin6_flowinfo = ip6_flowinfo(ip6h);
-			if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-				sin->sin6_scope_id = IP6CB(skb)->iif;
+			sin->sin6_scope_id =
+				ipv6_iface_scope_id(&sin->sin6_addr,
+						    IP6CB(skb)->iif);
 		} else {
 			ipv6_addr_set_v4mapped(*(__be32 *)(nh + serr->addr_offset),
 					       &sin->sin6_addr);
+			sin->sin6_scope_id = 0;
 		}
 	}
 
@@ -376,18 +377,19 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len)
 	if (serr->ee.ee_origin != SO_EE_ORIGIN_LOCAL) {
 		sin->sin6_family = AF_INET6;
 		sin->sin6_flowinfo = 0;
-		sin->sin6_scope_id = 0;
 		if (skb->protocol == htons(ETH_P_IPV6)) {
 			sin->sin6_addr = ipv6_hdr(skb)->saddr;
 			if (np->rxopt.all)
 				ip6_datagram_recv_ctl(sk, msg, skb);
-			if (ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-				sin->sin6_scope_id = IP6CB(skb)->iif;
+			sin->sin6_scope_id =
+				ipv6_iface_scope_id(&sin->sin6_addr,
+						    IP6CB(skb)->iif);
 		} else {
 			struct inet_sock *inet = inet_sk(sk);
 
 			ipv6_addr_set_v4mapped(ip_hdr(skb)->saddr,
 					       &sin->sin6_addr);
+			sin->sin6_scope_id = 0;
 			if (inet->cmsg_flags)
 				ip_cmsg_recv(msg, skb);
 		}
@@ -653,7 +655,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 					rcu_read_unlock();
 					return -ENODEV;
 				}
-			} else if (addr_type & IPV6_ADDR_LINKLOCAL) {
+			} else if (__ipv6_addr_needs_scope_id(addr_type)) {
 				rcu_read_unlock();
 				return -EINVAL;
 			}
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index fff5bdd..71b900c 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -434,7 +434,7 @@ void icmpv6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info)
 	 *	Source addr check
 	 */
 
-	if (addr_type & IPV6_ADDR_LINKLOCAL)
+	if (__ipv6_addr_needs_scope_id(addr_type))
 		iif = skb->dev->ifindex;
 
 	/*
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index b386a2c..9f1020b 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -170,10 +170,8 @@ void inet6_csk_addr2sockaddr(struct sock *sk, struct sockaddr * uaddr)
 	sin6->sin6_port	= inet_sk(sk)->inet_dport;
 	/* We do not store received flowlabel for TCP */
 	sin6->sin6_flowinfo = 0;
-	sin6->sin6_scope_id = 0;
-	if (sk->sk_bound_dev_if &&
-	    ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-		sin6->sin6_scope_id = sk->sk_bound_dev_if;
+	sin6->sin6_scope_id = ipv6_iface_scope_id(&sin6->sin6_addr,
+						  sk->sk_bound_dev_if);
 }
 
 EXPORT_SYMBOL_GPL(inet6_csk_addr2sockaddr);
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 70fa814..0a540f2 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -264,7 +264,7 @@ static int rawv6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	if (addr_type != IPV6_ADDR_ANY) {
 		struct net_device *dev = NULL;
 
-		if (addr_type & IPV6_ADDR_LINKLOCAL) {
+		if (__ipv6_addr_needs_scope_id(addr_type)) {
 			if (addr_len >= sizeof(struct sockaddr_in6) &&
 			    addr->sin6_scope_id) {
 				/* Override any existing binding, if another
@@ -499,9 +499,8 @@ static int rawv6_recvmsg(struct kiocb *iocb, struct sock *sk,
 		sin6->sin6_port = 0;
 		sin6->sin6_addr = ipv6_hdr(skb)->saddr;
 		sin6->sin6_flowinfo = 0;
-		sin6->sin6_scope_id = 0;
-		if (ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-			sin6->sin6_scope_id = IP6CB(skb)->iif;
+		sin6->sin6_scope_id = ipv6_iface_scope_id(&sin6->sin6_addr,
+							  IP6CB(skb)->iif);
 	}
 
 	sock_recv_ts_and_drops(msg, sk, skb);
@@ -803,7 +802,7 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
 
 		if (addr_len >= sizeof(struct sockaddr_in6) &&
 		    sin6->sin6_scope_id &&
-		    ipv6_addr_type(daddr)&IPV6_ADDR_LINKLOCAL)
+		    __ipv6_addr_needs_scope_id(ipv6_addr_scope(daddr)))
 			fl6.flowi6_oif = sin6->sin6_scope_id;
 	} else {
 		if (sk->sk_state != TCP_ESTABLISHED)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 599e1ba6..ba739d9 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -450,15 +450,16 @@ try_again:
 		sin6->sin6_family = AF_INET6;
 		sin6->sin6_port = udp_hdr(skb)->source;
 		sin6->sin6_flowinfo = 0;
-		sin6->sin6_scope_id = 0;
 
-		if (is_udp4)
+		if (is_udp4) {
 			ipv6_addr_set_v4mapped(ip_hdr(skb)->saddr,
 					       &sin6->sin6_addr);
-		else {
+			sin6->sin6_scope_id = 0;
+		} else {
 			sin6->sin6_addr = ipv6_hdr(skb)->saddr;
-			if (ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-				sin6->sin6_scope_id = IP6CB(skb)->iif;
+			sin6->sin6_scope_id =
+				ipv6_iface_scope_id(&sin6->sin6_addr,
+						    IP6CB(skb)->iif);
 		}
 
 	}
@@ -1118,7 +1119,8 @@ do_udp_sendmsg:
 
 		if (addr_len >= sizeof(struct sockaddr_in6) &&
 		    sin6->sin6_scope_id &&
-		    ipv6_addr_type(daddr)&IPV6_ADDR_LINKLOCAL)
+		    __ipv6_addr_needs_scope_id(
+			    ipv6_addr_scope(&sin6->sin6_addr)))
 			fl6.flowi6_oif = sin6->sin6_scope_id;
 	} else {
 		if (sk->sk_state != TCP_ESTABLISHED)
-- 
1.8.1.2

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Hannes Frederic Sowa]

On Tue, Feb 12, 2013 at 11:16:34PM +0100, Hannes Frederic Sowa wrote:
> This patch requires multicast interface-scoped addresses to supply a
> sin6_scope_id. Because the sin6_scope_id is now also correctly used
> in case of interface-scoped multicast traffic this enables one to use
> interface scoped addresses over interfaces which are not targeted by the
> default multicast route (the route has to be put there manually, though).
> 
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/ipv6/af_inet6.c              |  6 +++---
>  net/ipv6/datagram.c              | 18 ++++++++++--------
>  net/ipv6/icmp.c                  |  2 +-
>  net/ipv6/inet6_connection_sock.c |  6 ++----
>  net/ipv6/raw.c                   |  9 ++++-----
>  net/ipv6/udp.c                   | 14 ++++++++------
>  6 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index 6b793bf..f56277f 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>  			struct net_device *dev = NULL;
>  
>  			rcu_read_lock();
> -			if (addr_type & IPV6_ADDR_LINKLOCAL) {
> +			if (__ipv6_addr_needs_scope_id(addr_type)) {
>  				if (addr_len >= sizeof(struct sockaddr_in6) &&
>  				    addr->sin6_scope_id) {
>  					/* Override any existing binding, if another one

By trying to setup the multicast interface scoped routes by default I
just found a bug in this patch essentially breaking ipv6 multicast. I
overlooked that ipv6_addr_type strips off the scopes, thus my check if
a multicast address needs a scope_id always returns true. I'll check
if I can convert the ipv6_addr_type calls to __ipv6_addr_type and will
reroll the patch.  Sorry, my tests were too focused on interface/local
multicast. :(

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Brian Haley]

On 02/12/2013 07:13 PM, Hannes Frederic Sowa wrote:
>> --- a/net/ipv6/af_inet6.c
>> +++ b/net/ipv6/af_inet6.c
>> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>>  			struct net_device *dev = NULL;
>>  
>>  			rcu_read_lock();
>> -			if (addr_type & IPV6_ADDR_LINKLOCAL) {
>> +			if (__ipv6_addr_needs_scope_id(addr_type)) {
>>  				if (addr_len >= sizeof(struct sockaddr_in6) &&
>>  				    addr->sin6_scope_id) {
>>  					/* Override any existing binding, if another one
> 
> By trying to setup the multicast interface scoped routes by default I
> just found a bug in this patch essentially breaking ipv6 multicast. I
> overlooked that ipv6_addr_type strips off the scopes, thus my check if
> a multicast address needs a scope_id always returns true. I'll check
> if I can convert the ipv6_addr_type calls to __ipv6_addr_type and will
> reroll the patch.  Sorry, my tests were too focused on interface/local
> multicast. :(

I'd always thought of adding helper inlines like these in net/ipv6.h:

static inline bool ipv6_addr_linklocal(const struct in6_addr *a)
{
        return ((a->s6_addr32[0] & htonl(0xFFC00000)) == htonl(0xFE800000));
}

static inline bool ipv6_addr_mc_linklocal(const struct in6_addr *a)
{
        return (((a->s6_addr32[0] & htonl(0xFF000000)) == htonl(0xFF000000)) &&
                ((a->s6_addr32[1] & 0x0F) == IPV6_ADDR_SCOPE_LINKLOCAL));
}

Maybe something like that would help here?

When I saw this in patch 3/3 it just seemed like the long way to determine if
the address was a link-local multicast:

	!__ipv6_addr_needs_scope_id(__ipv6_addr_type(&hdr->daddr))

The helper isn't as generic as your patch, but more direct.

-Brian

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Hannes Frederic Sowa]

On Tue, Feb 12, 2013 at 09:51:06PM -0500, Brian Haley wrote:
> On 02/12/2013 07:13 PM, Hannes Frederic Sowa wrote:
> >> --- a/net/ipv6/af_inet6.c
> >> +++ b/net/ipv6/af_inet6.c
> >> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> >>  			struct net_device *dev = NULL;
> >>  
> >>  			rcu_read_lock();
> >> -			if (addr_type & IPV6_ADDR_LINKLOCAL) {
> >> +			if (__ipv6_addr_needs_scope_id(addr_type)) {
> >>  				if (addr_len >= sizeof(struct sockaddr_in6) &&
> >>  				    addr->sin6_scope_id) {
> >>  					/* Override any existing binding, if another one
> > 
> > By trying to setup the multicast interface scoped routes by default I
> > just found a bug in this patch essentially breaking ipv6 multicast. I
> > overlooked that ipv6_addr_type strips off the scopes, thus my check if
> > a multicast address needs a scope_id always returns true. I'll check
> > if I can convert the ipv6_addr_type calls to __ipv6_addr_type and will
> > reroll the patch.  Sorry, my tests were too focused on interface/local
> > multicast. :(
> 
> I'd always thought of adding helper inlines like these in net/ipv6.h:
> 
> static inline bool ipv6_addr_linklocal(const struct in6_addr *a)
> {
>         return ((a->s6_addr32[0] & htonl(0xFFC00000)) == htonl(0xFE800000));
> }
> 
> static inline bool ipv6_addr_mc_linklocal(const struct in6_addr *a)
> {
>         return (((a->s6_addr32[0] & htonl(0xFF000000)) == htonl(0xFF000000)) &&
>                 ((a->s6_addr32[1] & 0x0F) == IPV6_ADDR_SCOPE_LINKLOCAL));
> }
> 
> Maybe something like that would help here?
> 
> When I saw this in patch 3/3 it just seemed like the long way to determine if
> the address was a link-local multicast:
> 
> 	!__ipv6_addr_needs_scope_id(__ipv6_addr_type(&hdr->daddr))
> 
> The helper isn't as generic as your patch, but more direct.

Yup, that would have prevented the bug. My idea was to introduce an
opaque type to have compiler warnings on misuse of addr_type. I'll have a look
later today on how to proceed with this patch. Thanks!

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[YOSHIFUJI Hideaki]

Brian Haley wrote:
> On 02/12/2013 07:13 PM, Hannes Frederic Sowa wrote:
>>> --- a/net/ipv6/af_inet6.c
>>> +++ b/net/ipv6/af_inet6.c
>>> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>>>  			struct net_device *dev = NULL;
>>>  
>>>  			rcu_read_lock();
>>> -			if (addr_type & IPV6_ADDR_LINKLOCAL) {
>>> +			if (__ipv6_addr_needs_scope_id(addr_type)) {
>>>  				if (addr_len >= sizeof(struct sockaddr_in6) &&
>>>  				    addr->sin6_scope_id) {
>>>  					/* Override any existing binding, if another one
>>
>> By trying to setup the multicast interface scoped routes by default I
>> just found a bug in this patch essentially breaking ipv6 multicast. I
>> overlooked that ipv6_addr_type strips off the scopes, thus my check if
>> a multicast address needs a scope_id always returns true. I'll check
>> if I can convert the ipv6_addr_type calls to __ipv6_addr_type and will
>> reroll the patch.  Sorry, my tests were too focused on interface/local
>> multicast. :(
> 
> I'd always thought of adding helper inlines like these in net/ipv6.h:
> 
> static inline bool ipv6_addr_linklocal(const struct in6_addr *a)
> {
>         return ((a->s6_addr32[0] & htonl(0xFFC00000)) == htonl(0xFE800000));
> }
> 
> static inline bool ipv6_addr_mc_linklocal(const struct in6_addr *a)
> {
>         return (((a->s6_addr32[0] & htonl(0xFF000000)) == htonl(0xFF000000)) &&
>                 ((a->s6_addr32[1] & 0x0F) == IPV6_ADDR_SCOPE_LINKLOCAL));
> }
> 
> Maybe something like that would help here?

If you have several address checks around, please use ipv6_addr_type()
(or __ipv6_addr_type()).  Above "direct" checks should be used only for
single-shot test.  But well, I have to agree that ipv6_addr_type and
friends is becoming complex.  In mid-term, I would like to take look
at it.  I might think of having addr_type for src/dst in skb->cb
after all.

--yoshfuji

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Hannes Frederic Sowa]

On Thu, Feb 14, 2013 at 01:47:47AM +0900, YOSHIFUJI Hideaki wrote:
> If you have several address checks around, please use ipv6_addr_type()
> (or __ipv6_addr_type()).  Above "direct" checks should be used only for
> single-shot test.  But well, I have to agree that ipv6_addr_type and
> friends is becoming complex.  In mid-term, I would like to take look
> at it.  I might think of having addr_type for src/dst in skb->cb
> after all.

Yes, I had no plan to duplicate addrconf_core tests. I just thought about

a) not using addr_types at all in the new helper functions
b) use something like

union ipv6_addr_type {
	struct {
		__u16 scope;
		__u16 addr_type;
	};
	__u32 type_and_scope;
}

and explicitly use this in __ipv6_addr_type (this function does not seam to be
used as often as ipv6_addr_type).

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Hannes Frederic Sowa]

On Thu, Feb 14, 2013 at 01:47:47AM +0900, YOSHIFUJI Hideaki wrote:
> If you have several address checks around, please use ipv6_addr_type()
> (or __ipv6_addr_type()).  Above "direct" checks should be used only for
> single-shot test.  But well, I have to agree that ipv6_addr_type and
> friends is becoming complex.  In mid-term, I would like to take look
> at it.  I might think of having addr_type for src/dst in skb->cb
> after all.

What do you think about the attached patch?  If you agree with the changes I
would test it tomorrow and rebase my other patches ontop. The changes are only
compile tested.

[PATCH net-next RFC] ipv6: introduce new type ipv6_addr_props to hold type and scope

---
 include/net/ipv6.h       | 16 +++++---
 net/ipv6/addrconf.c      | 27 +++++++-------
 net/ipv6/addrconf_core.c | 97 +++++++++++++++++++++++++++++++-----------------
 net/ipv6/datagram.c      | 12 +++---
 4 files changed, 95 insertions(+), 57 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 851d541..3a3ec1cc 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -298,20 +298,26 @@ static inline int ip6_frag_mem(struct net *net)
 #define IPV6_FRAG_LOW_THRESH	(3 * 1024*1024)	/* 3145728 */
 #define IPV6_FRAG_TIMEOUT	(60 * HZ)	/* 60 seconds */
 
-extern int __ipv6_addr_type(const struct in6_addr *addr);
+struct ipv6_addr_props {
+	u16 type;
+	s16 scope;
+};
+
+extern struct ipv6_addr_props __ipv6_addr_type(const struct in6_addr *addr);
 static inline int ipv6_addr_type(const struct in6_addr *addr)
 {
-	return __ipv6_addr_type(addr) & 0xffff;
+	return __ipv6_addr_type(addr).type;
 }
 
 static inline int ipv6_addr_scope(const struct in6_addr *addr)
 {
-	return __ipv6_addr_type(addr) & IPV6_ADDR_SCOPE_MASK;
+	return __ipv6_addr_type(addr).scope;
 }
 
-static inline int __ipv6_addr_src_scope(int type)
+static inline int __ipv6_addr_src_scope(struct ipv6_addr_props props)
 {
-	return (type == IPV6_ADDR_ANY) ? __IPV6_ADDR_SCOPE_INVALID : (type >> 16);
+	return (props.type == IPV6_ADDR_ANY) ?
+		__IPV6_ADDR_SCOPE_INVALID : props.scope;
 }
 
 static inline int ipv6_addr_src_scope(const struct in6_addr *addr)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 86c235d..f9adec4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1106,7 +1106,7 @@ enum {
 
 struct ipv6_saddr_score {
 	int			rule;
-	int			addr_type;
+	struct ipv6_addr_props	addr_props;
 	struct inet6_ifaddr	*ifa;
 	DECLARE_BITMAP(scorebits, IPV6_SADDR_RULE_MAX);
 	int			scopedist;
@@ -1180,7 +1180,7 @@ static int ipv6_get_saddr_eval(struct net *net,
 		 * C > d + 14 - B >= 15 + 14 - B = 29 - B.
 		 * Assume B = 0 and we get C > 29.
 		 */
-		ret = __ipv6_addr_src_scope(score->addr_type);
+		ret = __ipv6_addr_src_scope(score->addr_props);
 		if (ret >= dst->scope)
 			ret = -ret;
 		else
@@ -1189,8 +1189,9 @@ static int ipv6_get_saddr_eval(struct net *net,
 		break;
 	case IPV6_SADDR_RULE_PREFERRED:
 		/* Rule 3: Avoid deprecated and optimistic addresses */
-		ret = ipv6_saddr_preferred(score->addr_type) ||
-		      !(score->ifa->flags & (IFA_F_DEPRECATED|IFA_F_OPTIMISTIC));
+		ret = ipv6_saddr_preferred(score->addr_props.type) ||
+			!(score->ifa->flags &
+			  (IFA_F_DEPRECATED|IFA_F_OPTIMISTIC));
 		break;
 #ifdef CONFIG_IPV6_MIP6
 	case IPV6_SADDR_RULE_HOA:
@@ -1209,7 +1210,7 @@ static int ipv6_get_saddr_eval(struct net *net,
 	case IPV6_SADDR_RULE_LABEL:
 		/* Rule 6: Prefer matching label */
 		ret = ipv6_addr_label(net,
-				      &score->ifa->addr, score->addr_type,
+				      &score->ifa->addr, score->addr_props.type,
 				      score->ifa->idev->dev->ifindex) == dst->label;
 		break;
 #ifdef CONFIG_IPV6_PRIVACY
@@ -1258,13 +1259,12 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
 				*score = &scores[0], *hiscore = &scores[1];
 	struct ipv6_saddr_dst dst;
 	struct net_device *dev;
-	int dst_type;
+	struct ipv6_addr_props dst_props = __ipv6_addr_type(daddr);
 
-	dst_type = __ipv6_addr_type(daddr);
 	dst.addr = daddr;
 	dst.ifindex = dst_dev ? dst_dev->ifindex : 0;
-	dst.scope = __ipv6_addr_src_scope(dst_type);
-	dst.label = ipv6_addr_label(net, daddr, dst_type, dst.ifindex);
+	dst.scope = __ipv6_addr_src_scope(dst_props);
+	dst.label = ipv6_addr_label(net, daddr, dst_props.type, dst.ifindex);
 	dst.prefs = prefs;
 
 	hiscore->rule = -1;
@@ -1287,7 +1287,7 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
 		 *    belonging to the same site as the outgoing
 		 *    interface.)
 		 */
-		if (((dst_type & IPV6_ADDR_MULTICAST) ||
+		if ((dst_props.type & IPV6_ADDR_MULTICAST ||
 		     dst.scope <= IPV6_ADDR_SCOPE_LINKLOCAL) &&
 		    dst.ifindex && dev->ifindex != dst.ifindex)
 			continue;
@@ -1314,10 +1314,11 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
 			    (!(score->ifa->flags & IFA_F_OPTIMISTIC)))
 				continue;
 
-			score->addr_type = __ipv6_addr_type(&score->ifa->addr);
+			score->addr_props = __ipv6_addr_type(&score->ifa->addr);
 
-			if (unlikely(score->addr_type == IPV6_ADDR_ANY ||
-				     score->addr_type & IPV6_ADDR_MULTICAST)) {
+			if (unlikely(score->addr_props.type == IPV6_ADDR_ANY ||
+				     score->addr_props.type &
+				     IPV6_ADDR_MULTICAST)) {
 				LIMIT_NETDEBUG(KERN_DEBUG
 					       "ADDRCONF: unspecified / multicast address "
 					       "assigned as unicast address on %s",
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index d051e5f..cb3eb1d 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -6,75 +6,104 @@
 #include <linux/export.h>
 #include <net/ipv6.h>
 
-#define IPV6_ADDR_SCOPE_TYPE(scope)	((scope) << 16)
-
-static inline unsigned int ipv6_addr_scope2type(unsigned int scope)
+static inline struct ipv6_addr_props ipv6_addr_scope2type(unsigned int scope)
 {
 	switch (scope) {
 	case IPV6_ADDR_SCOPE_NODELOCAL:
-		return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_NODELOCAL) |
-			IPV6_ADDR_LOOPBACK);
+		return (struct ipv6_addr_props){
+			.type = IPV6_ADDR_MULTICAST|IPV6_ADDR_LOOPBACK,
+			.scope = IPV6_ADDR_SCOPE_NODELOCAL
+		};
 	case IPV6_ADDR_SCOPE_LINKLOCAL:
-		return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_LINKLOCAL) |
-			IPV6_ADDR_LINKLOCAL);
+		return (struct ipv6_addr_props){
+			.type = IPV6_ADDR_MULTICAST|IPV6_ADDR_LINKLOCAL,
+			.scope = IPV6_ADDR_SCOPE_LINKLOCAL
+		};
 	case IPV6_ADDR_SCOPE_SITELOCAL:
-		return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_SITELOCAL) |
-			IPV6_ADDR_SITELOCAL);
+		return (struct ipv6_addr_props){
+			.type = IPV6_ADDR_MULTICAST|IPV6_ADDR_SITELOCAL,
+			.scope = IPV6_ADDR_SCOPE_SITELOCAL
+		};
 	}
-	return IPV6_ADDR_SCOPE_TYPE(scope);
+	return (struct ipv6_addr_props){
+		.type = IPV6_ADDR_MULTICAST,
+		.scope = scope
+	};
 }
 
-int __ipv6_addr_type(const struct in6_addr *addr)
+struct ipv6_addr_props __ipv6_addr_type(const struct in6_addr *addr)
 {
-	__be32 st;
-
-	st = addr->s6_addr32[0];
+	__be32 st = addr->s6_addr32[0];
 
 	/* Consider all addresses with the first three bits different of
 	   000 and 111 as unicasts.
 	 */
 	if ((st & htonl(0xE0000000)) != htonl(0x00000000) &&
 	    (st & htonl(0xE0000000)) != htonl(0xE0000000))
-		return (IPV6_ADDR_UNICAST |
-			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));
+		return (struct ipv6_addr_props){
+			.type = IPV6_ADDR_UNICAST,
+			.scope = IPV6_ADDR_SCOPE_GLOBAL
+		};
 
 	if ((st & htonl(0xFF000000)) == htonl(0xFF000000)) {
 		/* multicast */
 		/* addr-select 3.1 */
-		return (IPV6_ADDR_MULTICAST |
-			ipv6_addr_scope2type(IPV6_ADDR_MC_SCOPE(addr)));
+		return ipv6_addr_scope2type(IPV6_ADDR_MC_SCOPE(addr));
 	}
 
 	if ((st & htonl(0xFFC00000)) == htonl(0xFE800000))
-		return (IPV6_ADDR_LINKLOCAL | IPV6_ADDR_UNICAST |
-			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_LINKLOCAL));		/* addr-select 3.1 */
+		/* addr-select 3.1 */
+		return (struct ipv6_addr_props){
+			.type = IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST,
+			.scope = IPV6_ADDR_SCOPE_LINKLOCAL
+		};
 	if ((st & htonl(0xFFC00000)) == htonl(0xFEC00000))
-		return (IPV6_ADDR_SITELOCAL | IPV6_ADDR_UNICAST |
-			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_SITELOCAL));		/* addr-select 3.1 */
+		/* addr-select 3.1 */
+		return (struct ipv6_addr_props){
+			.type = IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST,
+			.scope = IPV6_ADDR_SCOPE_SITELOCAL,
+		};
 	if ((st & htonl(0xFE000000)) == htonl(0xFC000000))
-		return (IPV6_ADDR_UNICAST |
-			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));			/* RFC 4193 */
+		/* RFC 4193 */
+		return (struct ipv6_addr_props){
+			.type = IPV6_ADDR_UNICAST,
+			.scope = IPV6_ADDR_SCOPE_GLOBAL,
+		};
 
 	if ((addr->s6_addr32[0] | addr->s6_addr32[1]) == 0) {
 		if (addr->s6_addr32[2] == 0) {
 			if (addr->s6_addr32[3] == 0)
-				return IPV6_ADDR_ANY;
+				return (struct ipv6_addr_props){
+					.type = IPV6_ADDR_ANY
+				};
 
 			if (addr->s6_addr32[3] == htonl(0x00000001))
-				return (IPV6_ADDR_LOOPBACK | IPV6_ADDR_UNICAST |
-					IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_LINKLOCAL));	/* addr-select 3.4 */
+				/* addr-select 3.4 */
+				return (struct ipv6_addr_props){
+					.type = IPV6_ADDR_LOOPBACK|
+						IPV6_ADDR_UNICAST,
+					.scope = IPV6_ADDR_SCOPE_LINKLOCAL
+				};
 
-			return (IPV6_ADDR_COMPATv4 | IPV6_ADDR_UNICAST |
-				IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));	/* addr-select 3.3 */
+			/* addr-select 3.3 */
+			return (struct ipv6_addr_props){
+				.type = IPV6_ADDR_COMPATv4|IPV6_ADDR_UNICAST,
+				.scope = IPV6_ADDR_SCOPE_GLOBAL
+			};
 		}
 
 		if (addr->s6_addr32[2] == htonl(0x0000ffff))
-			return (IPV6_ADDR_MAPPED |
-				IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));	/* addr-select 3.3 */
+			/* addr-select 3.3 */
+			return (struct ipv6_addr_props){
+				.type = IPV6_ADDR_MAPPED,
+					.scope = IPV6_ADDR_SCOPE_GLOBAL
+					};
 	}
 
-	return (IPV6_ADDR_UNICAST |
-		IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));	/* addr-select 3.4 */
+	/* addr-select 3.4 */
+	return (struct ipv6_addr_props){
+		.type = IPV6_ADDR_UNICAST,
+		.scope = IPV6_ADDR_SCOPE_GLOBAL
+	};
 }
 EXPORT_SYMBOL(__ipv6_addr_type);
-
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index f5a5478..e859899 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -614,7 +614,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 	int err = 0;
 
 	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
-		int addr_type;
+		struct ipv6_addr_props addr_props;
 
 		if (!CMSG_OK(msg, cmsg)) {
 			err = -EINVAL;
@@ -644,7 +644,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 				fl6->flowi6_oif = src_info->ipi6_ifindex;
 			}
 
-			addr_type = __ipv6_addr_type(&src_info->ipi6_addr);
+			addr_props = __ipv6_addr_type(&src_info->ipi6_addr);
 
 			rcu_read_lock();
 			if (fl6->flowi6_oif) {
@@ -653,13 +653,15 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 					rcu_read_unlock();
 					return -ENODEV;
 				}
-			} else if (addr_type & IPV6_ADDR_LINKLOCAL) {
+			} else if (addr_props.type & IPV6_ADDR_LINKLOCAL) {
 				rcu_read_unlock();
 				return -EINVAL;
 			}
 
-			if (addr_type != IPV6_ADDR_ANY) {
-				int strict = __ipv6_addr_src_scope(addr_type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
+			if (addr_props.type != IPV6_ADDR_ANY) {
+				int strict =
+					__ipv6_addr_src_scope(addr_props) <=
+					IPV6_ADDR_SCOPE_LINKLOCAL;
 				if (!(inet_sk(sk)->freebind || inet_sk(sk)->transparent) &&
 				    !ipv6_chk_addr(net, &src_info->ipi6_addr,
 						   strict ? dev : NULL, 0))
-- 
1.8.1.2

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Brian Haley]

On 02/13/2013 11:25 PM, Hannes Frederic Sowa wrote:
> On Thu, Feb 14, 2013 at 01:47:47AM +0900, YOSHIFUJI Hideaki wrote:
>> If you have several address checks around, please use ipv6_addr_type()
>> (or __ipv6_addr_type()).  Above "direct" checks should be used only for
>> single-shot test.  But well, I have to agree that ipv6_addr_type and
>> friends is becoming complex.  In mid-term, I would like to take look
>> at it.  I might think of having addr_type for src/dst in skb->cb
>> after all.
> 
> What do you think about the attached patch?  If you agree with the changes I
> would test it tomorrow and rebase my other patches ontop. The changes are only
> compile tested.
> 
> [PATCH net-next RFC] ipv6: introduce new type ipv6_addr_props to hold type and scope
> 
> ---
>  include/net/ipv6.h       | 16 +++++---
>  net/ipv6/addrconf.c      | 27 +++++++-------
>  net/ipv6/addrconf_core.c | 97 +++++++++++++++++++++++++++++++-----------------
>  net/ipv6/datagram.c      | 12 +++---
>  4 files changed, 95 insertions(+), 57 deletions(-)
> 
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 851d541..3a3ec1cc 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -298,20 +298,26 @@ static inline int ip6_frag_mem(struct net *net)
>  #define IPV6_FRAG_LOW_THRESH	(3 * 1024*1024)	/* 3145728 */
>  #define IPV6_FRAG_TIMEOUT	(60 * HZ)	/* 60 seconds */
>  
> -extern int __ipv6_addr_type(const struct in6_addr *addr);
> +struct ipv6_addr_props {
> +	u16 type;
> +	s16 scope;
> +};

Seeing this makes me think we should unify the flags and scope members of
inet6_ifaddr to something like this, moving to a single set of values for IPv6
addresses.  Then ipv6_dev_get_saddr() wouldn't have to use __ipv6_adr_type() as
much since the address struct would already have the values.  Possible future
work...

> diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
> index d051e5f..cb3eb1d 100644
> --- a/net/ipv6/addrconf_core.c
> +++ b/net/ipv6/addrconf_core.c
> @@ -6,75 +6,104 @@
>  #include <linux/export.h>
>  #include <net/ipv6.h>
>  
> -#define IPV6_ADDR_SCOPE_TYPE(scope)	((scope) << 16)
> -
> -static inline unsigned int ipv6_addr_scope2type(unsigned int scope)
> +static inline struct ipv6_addr_props ipv6_addr_scope2type(unsigned int scope)
>  {

Rename to ipv6_addr_mc_props() ?

>  	switch (scope) {
>  	case IPV6_ADDR_SCOPE_NODELOCAL:
> -		return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_NODELOCAL) |
> -			IPV6_ADDR_LOOPBACK);
> +		return (struct ipv6_addr_props){
> +			.type = IPV6_ADDR_MULTICAST|IPV6_ADDR_LOOPBACK,
> +			.scope = IPV6_ADDR_SCOPE_NODELOCAL
> +		};
>  	case IPV6_ADDR_SCOPE_LINKLOCAL:
> -		return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_LINKLOCAL) |
> -			IPV6_ADDR_LINKLOCAL);
> +		return (struct ipv6_addr_props){
> +			.type = IPV6_ADDR_MULTICAST|IPV6_ADDR_LINKLOCAL,
> +			.scope = IPV6_ADDR_SCOPE_LINKLOCAL
> +		};
>  	case IPV6_ADDR_SCOPE_SITELOCAL:
> -		return (IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_SITELOCAL) |
> -			IPV6_ADDR_SITELOCAL);
> +		return (struct ipv6_addr_props){
> +			.type = IPV6_ADDR_MULTICAST|IPV6_ADDR_SITELOCAL,
> +			.scope = IPV6_ADDR_SCOPE_SITELOCAL
> +		};
>  	}
> -	return IPV6_ADDR_SCOPE_TYPE(scope);
> +	return (struct ipv6_addr_props){
> +		.type = IPV6_ADDR_MULTICAST,
> +		.scope = scope
> +	};
>  }
>  
> -int __ipv6_addr_type(const struct in6_addr *addr)
> +struct ipv6_addr_props __ipv6_addr_type(const struct in6_addr *addr)

Should this be __ipv6_addr_props() now?  It's always returned type and scope,
but now it's more obvious with the return value.

>  {
> -	__be32 st;
> -
> -	st = addr->s6_addr32[0];
> +	__be32 st = addr->s6_addr32[0];
>  
>  	/* Consider all addresses with the first three bits different of
>  	   000 and 111 as unicasts.
>  	 */
>  	if ((st & htonl(0xE0000000)) != htonl(0x00000000) &&
>  	    (st & htonl(0xE0000000)) != htonl(0xE0000000))
> -		return (IPV6_ADDR_UNICAST |
> -			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_GLOBAL));
> +		return (struct ipv6_addr_props){
> +			.type = IPV6_ADDR_UNICAST,
> +			.scope = IPV6_ADDR_SCOPE_GLOBAL
> +		};
>  
>  	if ((st & htonl(0xFF000000)) == htonl(0xFF000000)) {
>  		/* multicast */
>  		/* addr-select 3.1 */
> -		return (IPV6_ADDR_MULTICAST |
> -			ipv6_addr_scope2type(IPV6_ADDR_MC_SCOPE(addr)));
> +		return ipv6_addr_scope2type(IPV6_ADDR_MC_SCOPE(addr));
>  	}
>  
>  	if ((st & htonl(0xFFC00000)) == htonl(0xFE800000))
> -		return (IPV6_ADDR_LINKLOCAL | IPV6_ADDR_UNICAST |
> -			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_LINKLOCAL));		/* addr-select 3.1 */
> +		/* addr-select 3.1 */
> +		return (struct ipv6_addr_props){
> +			.type = IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST,
> +			.scope = IPV6_ADDR_SCOPE_LINKLOCAL
> +		};
>  	if ((st & htonl(0xFFC00000)) == htonl(0xFEC00000))
> -		return (IPV6_ADDR_SITELOCAL | IPV6_ADDR_UNICAST |
> -			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_SITELOCAL));		/* addr-select 3.1 */
> +		/* addr-select 3.1 */
> +		return (struct ipv6_addr_props){
> +			.type = IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST,
> +			.scope = IPV6_ADDR_SCOPE_SITELOCAL,
> +		};

type here is wrong, should be IPV6_ADDR_SITELOCAL not linklocal.

> diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
> index f5a5478..e859899 100644
> --- a/net/ipv6/datagram.c
> +++ b/net/ipv6/datagram.c
> @@ -614,7 +614,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
>  	int err = 0;
>  
>  	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
> -		int addr_type;
> +		struct ipv6_addr_props addr_props;
>  
>  		if (!CMSG_OK(msg, cmsg)) {
>  			err = -EINVAL;
> @@ -644,7 +644,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
>  				fl6->flowi6_oif = src_info->ipi6_ifindex;
>  			}
>  
> -			addr_type = __ipv6_addr_type(&src_info->ipi6_addr);
> +			addr_props = __ipv6_addr_type(&src_info->ipi6_addr);
>  
>  			rcu_read_lock();
>  			if (fl6->flowi6_oif) {
> @@ -653,13 +653,15 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
>  					rcu_read_unlock();
>  					return -ENODEV;
>  				}
> -			} else if (addr_type & IPV6_ADDR_LINKLOCAL) {
> +			} else if (addr_props.type & IPV6_ADDR_LINKLOCAL) {

Could be (addr_props.scope == IPV6_ADDR_SCOPE_LINKLOCAL), right?

-Brian

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Hannes Frederic Sowa]

On Thu, Feb 14, 2013 at 10:25:46AM -0500, Brian Haley wrote:
> > [PATCH net-next RFC] ipv6: introduce new type ipv6_addr_props to hold type and scope
> > 
> > ---
> >  include/net/ipv6.h       | 16 +++++---
> >  net/ipv6/addrconf.c      | 27 +++++++-------
> >  net/ipv6/addrconf_core.c | 97 +++++++++++++++++++++++++++++++-----------------
> >  net/ipv6/datagram.c      | 12 +++---
> >  4 files changed, 95 insertions(+), 57 deletions(-)
> > 
> > diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> > index 851d541..3a3ec1cc 100644
> > --- a/include/net/ipv6.h
> > +++ b/include/net/ipv6.h
> > @@ -298,20 +298,26 @@ static inline int ip6_frag_mem(struct net *net)
> >  #define IPV6_FRAG_LOW_THRESH	(3 * 1024*1024)	/* 3145728 */
> >  #define IPV6_FRAG_TIMEOUT	(60 * HZ)	/* 60 seconds */
> >  
> > -extern int __ipv6_addr_type(const struct in6_addr *addr);
> > +struct ipv6_addr_props {
> > +	u16 type;
> > +	s16 scope;
> > +};
> 
> Seeing this makes me think we should unify the flags and scope members of
> inet6_ifaddr to something like this, moving to a single set of values for IPv6
> addresses.  Then ipv6_dev_get_saddr() wouldn't have to use __ipv6_adr_type() as
> much since the address struct would already have the values.  Possible future
> work...

Thanks for your feedback. I will incooperate it in my patch.

> >  	if ((st & htonl(0xFFC00000)) == htonl(0xFEC00000))
> > -		return (IPV6_ADDR_SITELOCAL | IPV6_ADDR_UNICAST |
> > -			IPV6_ADDR_SCOPE_TYPE(IPV6_ADDR_SCOPE_SITELOCAL));		/* addr-select 3.1 */
> > +		/* addr-select 3.1 */
> > +		return (struct ipv6_addr_props){
> > +			.type = IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST,
> > +			.scope = IPV6_ADDR_SCOPE_SITELOCAL,
> > +		};
> 
> type here is wrong, should be IPV6_ADDR_SITELOCAL not linklocal.

Oops, of course. :)

> > @@ -653,13 +653,15 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
> >  					rcu_read_unlock();
> >  					return -ENODEV;
> >  				}
> > -			} else if (addr_type & IPV6_ADDR_LINKLOCAL) {
> > +			} else if (addr_props.type & IPV6_ADDR_LINKLOCAL) {
> 
> Could be (addr_props.scope == IPV6_ADDR_SCOPE_LINKLOCAL), right?

This code will be changed by my other patches (see the series I posted
about introducing helper functions for checking for sin6_scope_id, patch
1/2). I think I will let it as is for the time being.

Thanks,

  Hannes

Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id

[Brian Haley]

On 02/14/2013 01:53 PM, Hannes Frederic Sowa wrote:
> On Thu, Feb 14, 2013 at 10:25:46AM -0500, Brian Haley wrote:
>>> [PATCH net-next RFC] ipv6: introduce new type ipv6_addr_props to hold type and scope
>>>
>>> ---
>>>  include/net/ipv6.h       | 16 +++++---
>>>  net/ipv6/addrconf.c      | 27 +++++++-------
>>>  net/ipv6/addrconf_core.c | 97 +++++++++++++++++++++++++++++++-----------------
>>>  net/ipv6/datagram.c      | 12 +++---
>>>  4 files changed, 95 insertions(+), 57 deletions(-)
>>>
>>> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
>>> index 851d541..3a3ec1cc 100644
>>> --- a/include/net/ipv6.h
>>> +++ b/include/net/ipv6.h
>>> @@ -298,20 +298,26 @@ static inline int ip6_frag_mem(struct net *net)
>>>  #define IPV6_FRAG_LOW_THRESH	(3 * 1024*1024)	/* 3145728 */
>>>  #define IPV6_FRAG_TIMEOUT	(60 * HZ)	/* 60 seconds */
>>>  
>>> -extern int __ipv6_addr_type(const struct in6_addr *addr);
>>> +struct ipv6_addr_props {
>>> +	u16 type;
>>> +	s16 scope;
>>> +};
>>
>> Seeing this makes me think we should unify the flags and scope members of
>> inet6_ifaddr to something like this, moving to a single set of values for IPv6
>> addresses.  Then ipv6_dev_get_saddr() wouldn't have to use __ipv6_adr_type() as
>> much since the address struct would already have the values.  Possible future
>> work...
> 
> Thanks for your feedback. I will incooperate it in my patch.

Ok, just don't worry about this above one right now, once you dig into it you
will realize it's not straightforward.  I'd just fix any existing bugs first.
I'll see if I can find the time to send out an RFC patch of what I mean...

-Brian