[PATCH v3 2/2] Implement IPV6_UNICAST_IF socket option.

Subsystems: networking [general], networking [ipv4/ipv6], the rest, user datagram protocol (udp)

STALE5327d

5 messages, 2 authors, 2012-02-07 · open the first message on its own page

[PATCH v3 2/2] Implement IPV6_UNICAST_IF socket option.

From: Erich E. Hoover <hidden>
Date: 2012-02-06 17:58:06

The IPV6_UNICAST_IF feature is the IPv6 compliment to IP_UNICAST_IF.

Signed-off-by: Erich E. Hoover <redacted>
---
 include/linux/in6.h      |    1 +
 include/linux/ipv6.h     |    2 +
 include/net/ipv6.h       |    2 +
 net/ipv6/af_inet6.c      |    1 +
 net/ipv6/icmp.c          |    3 ++
 net/ipv6/ipv6_sockglue.c |   49 ++++++++++++++++++++++++++++++++++++++++++++++
 net/ipv6/raw.c           |    2 +-
 net/ipv6/udp.c           |    2 +-
 8 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/include/linux/in6.h b/include/linux/in6.h
index 097a34b..a67d76e 100644
--- a/include/linux/in6.h
+++ b/include/linux/in6.h
@@ -163,6 +163,7 @@ struct in6_flowlabel_req {
 #define IPV6_NEXTHOP		9
 #define IPV6_AUTHHDR		10	/* obsolete */
 #define IPV6_FLOWINFO		11
+#define IPV6_UNICAST_IF		12
 
 #define IPV6_UNICAST_HOPS	16
 #define IPV6_MULTICAST_IF	17
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 6318268..1602fb0 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -299,6 +299,8 @@ struct ipv6_pinfo {
 	struct in6_addr		*saddr_cache;
 #endif
 
+	int			outif_index;
+
 	__be32			flow_label;
 	__u32			frag_size;
 
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index e4170a2..252677c 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -254,6 +254,8 @@ static inline void fl6_sock_release(struct ip6_flowlabel *fl)
 
 extern int 			ip6_ra_control(struct sock *sk, int sel);
 
+extern int 			ipv6_default_ifindex(const struct sock *sk);
+
 extern int			ipv6_parse_hopopts(struct sk_buff *skb);
 
 extern struct ipv6_txoptions *  ipv6_dup_options(struct sock *sk, struct ipv6_txoptions *opt);
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 13b84bc..d9ac287 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -199,6 +199,7 @@ lookup_protocol:
 	sk->sk_backlog_rcv	= answer->prot->backlog_rcv;
 
 	inet_sk(sk)->pinet6 = np = inet6_sk_generic(sk);
+	np->outif_index = 0;
 	np->hop_limit	= -1;
 	np->mcast_hops	= IPV6_DEFAULT_MCASTHOPS;
 	np->mc_loop	= 1;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 01d46bf..18f144e 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -551,6 +551,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
 		return;
 	np = inet6_sk(sk);
 
+	if (!fl6.flowi6_oif)
+		fl6.flowi6_oif = ipv6_default_ifindex(sk);
+
 	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
 		fl6.flowi6_oif = np->mcast_oif;
 
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 18a2719..7248f5a 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -98,6 +98,21 @@ int ip6_ra_control(struct sock *sk, int sel)
 	return 0;
 }
 
+int ipv6_default_ifindex(const struct sock *sk)
+{
+	struct ipv6_pinfo *np = inet6_sk(sk);
+	int ifindex = sk->sk_bound_dev_if;
+
+	/*
+         * If not bound to a specific interface then set the outgoing interface
+	 * to the value from the IPV6_UNICAST_IF socket option.
+         */
+	if (!ifindex)
+		ifindex = np->outif_index;
+
+	return ifindex;
+}
+
 static
 struct ipv6_txoptions *ipv6_update_options(struct sock *sk,
 					   struct ipv6_txoptions *opt)
@@ -516,6 +531,36 @@ done:
 		retv = 0;
 		break;
 
+	case IPV6_UNICAST_IF:
+	{
+		struct net_device *dev = NULL;
+		int ifindex;
+
+		if (optlen != sizeof(int))
+			goto e_inval;
+
+		ifindex = (__force int)ntohl((__force __be32)val);
+		if (ifindex == 0) {
+			np->outif_index = 0;
+			retv = 0;
+			break;
+		}
+
+		dev = dev_get_by_index(net, ifindex);
+		retv = -EADDRNOTAVAIL;
+		if (!dev)
+			break;
+		dev_put(dev);
+
+		retv = -EINVAL;
+		if (sk->sk_bound_dev_if && ifindex != sk->sk_bound_dev_if)
+			break;
+
+		np->outif_index = ifindex;
+		retv = 0;
+		break;
+	}
+
 	case IPV6_MULTICAST_IF:
 		if (sk->sk_type == SOCK_STREAM)
 			break;
@@ -1160,6 +1205,10 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		val = np->mcast_oif;
 		break;
 
+	case IPV6_UNICAST_IF:
+		val = (__force int)htonl((__u32) np->outif_index);
+		break;
+
 	case IPV6_MTU_DISCOVER:
 		val = np->pmtudisc;
 		break;
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index d02f7e4..25539a1 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -813,7 +813,7 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
 	}
 
 	if (fl6.flowi6_oif == 0)
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
+		fl6.flowi6_oif = ipv6_default_ifindex(sk);
 
 	if (msg->msg_controllen) {
 		opt = &opt_space;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 4f96b5c..bb8db62 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1081,7 +1081,7 @@ do_udp_sendmsg:
 	}
 
 	if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
+		fl6.flowi6_oif = ipv6_default_ifindex(sk);
 
 	if (!fl6.flowi6_oif)
 		fl6.flowi6_oif = np->sticky_pktinfo.ipi6_ifindex;
-- 
1.7.5.4

Re: [PATCH v3 2/2] Implement IPV6_UNICAST_IF socket option.

From: Brian Haley <hidden>
Date: 2012-02-06 21:13:49

On 02/06/2012 12:57 PM, Erich E. Hoover wrote:
The IPV6_UNICAST_IF feature is the IPv6 compliment to IP_UNICAST_IF.
quoted hunk
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 6318268..1602fb0 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -299,6 +299,8 @@ struct ipv6_pinfo {
 	struct in6_addr		*saddr_cache;
 #endif
 
+	int			outif_index;
+
 	__be32			flow_label;
 	__u32			frag_size;
I haven't done the math, but to make sure you don't add a padding hole you could
put this next to mcast_oif.  'pahole -C ipv6_pinfo net/built-in.o' showed there
was 5 bytes of padding in this cache line.

Nitpick: is ucast_oif a better name?
quoted hunk
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 01d46bf..18f144e 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -551,6 +551,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
 		return;
 	np = inet6_sk(sk);
 
+	if (!fl6.flowi6_oif)
+		fl6.flowi6_oif = ipv6_default_ifindex(sk);
+
 	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
 		fl6.flowi6_oif = np->mcast_oif;
This snippet shows (and rawv6_sendmsg() has the same problem), that
IPV6_UNICAST_IF can also affect multicast packets.  And I think we always want
SO_BINDTODEVICE to override them all.  Perhaps these checks should be:

	if (!fl6.flowi6_oif)
		fl6.flowi6_oif = sk->sk_bound_dev_if;

	if (!fl6.flowi6_oif)
		if (ipv6_addr_is_multicast(&fl6.daddr))
			fl6.flowi6_oif = np->mcast_oif;
		else
			fl6.flowi6_oif = np->ucast_oif;
+int ipv6_default_ifindex(const struct sock *sk)
+{
+	struct ipv6_pinfo *np = inet6_sk(sk);
+	int ifindex = sk->sk_bound_dev_if;
+
+	/*
+         * If not bound to a specific interface then set the outgoing interface
+	 * to the value from the IPV6_UNICAST_IF socket option.
+         */
+	if (!ifindex)
+		ifindex = np->outif_index;
+
+	return ifindex;
+}
All callers of this already have 'np', you could just pass it along.  Or with
the above change you don't even need it.  IPv4 code as well.
quoted hunk
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index d02f7e4..25539a1 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -813,7 +813,7 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
 	}
 
 	if (fl6.flowi6_oif == 0)
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
+		fl6.flowi6_oif = ipv6_default_ifindex(sk);
I think you should change this like above.
quoted hunk
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 4f96b5c..bb8db62 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1081,7 +1081,7 @@ do_udp_sendmsg:
 	}
 
 	if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
+		fl6.flowi6_oif = ipv6_default_ifindex(sk);
 
 	if (!fl6.flowi6_oif)
 		fl6.flowi6_oif = np->sticky_pktinfo.ipi6_ifindex;
This too, letting np->ucast_oif be third after PKTINFO, there's a multicast
check below in this code.

-Brian

Re: [PATCH v3 2/2] Implement IPV6_UNICAST_IF socket option.

From: Erich E. Hoover <hidden>
Date: 2012-02-06 22:11:47

On Mon, Feb 6, 2012 at 2:13 PM, Brian Haley [off-list ref] wrote:
On 02/06/2012 12:57 PM, Erich E. Hoover wrote:
quoted
...
+     int                     outif_index;
...
Nitpick: is ucast_oif a better name?
I templated off of the IPv4 code (example: mc_index), but I do think
that that is a better name.  Should I call the IPv4 version
"ucast_index" or have the same name for both IPv4 and IPv6?
quoted
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 01d46bf..18f144e 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -551,6 +551,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
              return;
      np = inet6_sk(sk);

+     if (!fl6.flowi6_oif)
+             fl6.flowi6_oif = ipv6_default_ifindex(sk);
+
      if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
              fl6.flowi6_oif = np->mcast_oif;
This snippet shows (and rawv6_sendmsg() has the same problem), that
IPV6_UNICAST_IF can also affect multicast packets.  And I think we always want
SO_BINDTODEVICE to override them all.  Perhaps these checks should be:
...
I'll double check all of these tonight, but a cursory look seems to
indicate that the multicast check is the right place to put this for
all the files.  I'm sorry about that, I clearly didn't consider
interfering with multicast.

Erich Hoover
ehoover@mines.edu

Re: [PATCH v3 2/2] Implement IPV6_UNICAST_IF socket option.

From: Brian Haley <hidden>
Date: 2012-02-07 03:00:07

On 02/06/2012 05:11 PM, Erich E. Hoover wrote:
On Mon, Feb 6, 2012 at 2:13 PM, Brian Haley [off-list ref] wrote:
quoted
On 02/06/2012 12:57 PM, Erich E. Hoover wrote:
quoted
...
+     int                     outif_index;
...
Nitpick: is ucast_oif a better name?
I templated off of the IPv4 code (example: mc_index), but I do think
that that is a better name.  Should I call the IPv4 version
"ucast_index" or have the same name for both IPv4 and IPv6?
If it was me I'd call it uc_index for IPv4, to mimic mc_ifindex.
quoted
quoted
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 01d46bf..18f144e 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -551,6 +551,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
              return;
      np = inet6_sk(sk);

+     if (!fl6.flowi6_oif)
+             fl6.flowi6_oif = ipv6_default_ifindex(sk);
+
      if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
              fl6.flowi6_oif = np->mcast_oif;
This snippet shows (and rawv6_sendmsg() has the same problem), that
IPV6_UNICAST_IF can also affect multicast packets.  And I think we always want
SO_BINDTODEVICE to override them all.  Perhaps these checks should be:
...
I'll double check all of these tonight, but a cursory look seems to
indicate that the multicast check is the right place to put this for
all the files.  I'm sorry about that, I clearly didn't consider
interfering with multicast.
It's probably important to keep the precedence the same as before:

	SO_BINDTODEVICE
	IPV6_PKTINFO
	IPV6_*CAST_IF

It looks like your IPv4 changes have the same problem.

-Brian

Re: [PATCH v3 2/2] Implement IPV6_UNICAST_IF socket option.

From: Erich E. Hoover <hidden>
Date: 2012-02-07 03:23:55

On Mon, Feb 6, 2012 at 8:00 PM, Brian Haley [off-list ref] wrote:
On 02/06/2012 05:11 PM, Erich E. Hoover wrote:
quoted
...
I'll double check all of these tonight, but a cursory look seems to
indicate that the multicast check is the right place to put this for
all the files.  I'm sorry about that, I clearly didn't consider
interfering with multicast.
It's probably important to keep the precedence the same as before:

       SO_BINDTODEVICE
       IPV6_PKTINFO
       IPV6_*CAST_IF
...
I agree, I mean that the IP*_MULTICAST_IF location appears to be the
correct place to put IP*_UNICAST_IF.  I'll post a revision after I've
had a chance to test it.

Erich Hoover
ehoover@mines.edu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help