ipv6:fix the outgoing interface selection order in udpv6_sendmsg()

RFC PATCH Applied Yang Hongyang 3 participants 9 messages Dec 9, 2008 – Dec 16, 2008

Subsystems (3)

Networking Network drivers UDP

lore

Share & tools

lore

Yang Hongyang

author

2008-12-09 07:11 UTC #0 lore raw

RFC PATCH 3/3 ipv6:fix the outgoing interface selection order in udpv6_sendmsg()

1.When no interface is specified in an IPV6_PKTINFO ancillary data
item, the interface specified in an IPV6_PKTINFO sticky optionis is used.

RFC3542:
6.7. Summary of Outgoing Interface Selection

This document and [RFC-3493] specify various methods that affect the selection of the packet's outgoing interface. This subsection summarizes the ordering among those in order to ensure deterministic behavior.

For a given outgoing packet on a given socket, the outgoing interface is determined in the following order:

1. if an interface is specified in an IPV6_PKTINFO ancillary data item, the interface is used.

2. otherwise, if an interface is specified in an IPV6_PKTINFO sticky option, the interface is used.

Signed-off-by Yang Hongyang<[email protected]>

---
 net/ipv6/udp.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

net/ipv6/udp.c +3 -0

index 8b48512..addd856 100644
@@ -761,6 +761,9 @@ do_udp_sendmsg:
 	}
 
 	if (!fl.oif)
+		fl.oif = np->sticky_pktinfo.ipi6_ifindex;
+
+	if (!fl.oif)
 		fl.oif = sk->sk_bound_dev_if;
 
 	if (msg->msg_controllen) {

Signature · 1 line

1.6.0.1

Rémi Denis-Courmont

2008-12-09 07:33 UTC #1 lore raw

Quoted 9 lines · you

1.When no interface is specified in an IPV6_PKTINFO ancillary data
item, the interface specified in an IPV6_PKTINFO sticky optionis is used.
net/ipv6/udp.c +0 -0
index 8b48512..addd856 100644

net/ipv6/udp.c do_udp_sendmsg: · line 769

Quoted 9 lines

net/ipv6/udp.c +3 -0

@@ -761,6 +761,9 @@ do_udp_sendmsg:
 	}
 
 	if (!fl.oif)
+		fl.oif = np->sticky_pktinfo.ipi6_ifindex;
+
+	if (!fl.oif)
 		fl.oif = sk->sk_bound_dev_if;
 
 	if (msg->msg_controllen) {

I believe overriding the outgoing interface is only allowed for link-local destinations, _or_ with privileges (as with SO_BINDTODEVICE).

This patch seems to change this, and I am not convinced it's a good idea.

Signature · 2 lines

Rémi Denis-Courmont
Maemo Software, Nokia Devices R&D

Yang Hongyang

author

2008-12-10 00:40 UTC #2 lore raw

Quoted 18 lines · Rémi Denis-Courmont

Quoted 17 lines · you
1.When no interface is specified in an IPV6_PKTINFO ancillary data
item, the interface specified in an IPV6_PKTINFO sticky optionis is used.
net/ipv6/udp.c +3 -0
index 8b48512..addd856 100644
@@ -761,6 +761,9 @@ do_udp_sendmsg:
 	}
 
 	if (!fl.oif)
+		fl.oif = np->sticky_pktinfo.ipi6_ifindex;
+
+	if (!fl.oif)
 		fl.oif = sk->sk_bound_dev_if;
 

Quoted 7 lines

net/ipv6/udp.c do_udp_sendmsg: · line 769
Quoted 1 line
net/ipv6/udp.c +0 -0
 	if (msg->msg_controllen) {
I believe overriding the outgoing interface is only allowed for link-local destinations, _or_ with privileges (as with SO_BINDTODEVICE).
This patch seems to change this, and I am not convinced it's a good idea.

It does not overriding the outgoing interface.
when no interface is specified in an IPV6_PKTINFO ancillary data item(as with
SO_BINDTODEVICE), the interface specified in an IPV6_PKTINFO sticky
optionins is used.

If outgoing interface is set with SO_BINDTODEVICE and also sticky pktinfo the outgoing interface is exactly what SO_BINDTODEVICE set,because in my first patch,when set sticy pktinfo,outgoing interface set by SO_BINDTODEVICE is checked:

+ if(sk->sk_bound_dev_if && pkt.ipi6_ifindex != sk->sk_bound_dev_if)
+ goto e_inval;

David Miller

maintainer

2008-12-10 06:16 UTC #3 lore raw

From: Yang Hongyang <[email protected]>
Date: Wed, 10 Dec 2008 08:42:20 +0800

Review

Quoted 8 lines

If outgoing interface is set with SO_BINDTODEVICE and also sticky pktinfo the outgoing interface is exactly what SO_BINDTODEVICE set,because in my first patch,when set sticy pktinfo,outgoing interface set by SO_BINDTODEVICE is checked:
(quoted diff) +2 -0
+		if(sk->sk_bound_dev_if && pkt.ipi6_ifindex != sk->sk_bound_dev_if)
+			goto e_inval;

You may be checking this, but these two values can become out of sync if the application next makes a SO_BINDTODEVICE setsockopt() call to change the index setting.

Nothing makes sure the sticky ipv6 socket information is updated if that happens. And the most obvious ways to handle that are very ugly, doing some kind of call into ipv6 from the generic socket SO_BINDTODEVICE setsockopt() handler.

Yang Hongyang

author

2008-12-10 06:32 UTC #4 lore raw

Quoted 16 lines · David Miller

From: Yang Hongyang <[email protected]>
Date: Wed, 10 Dec 2008 08:42:20 +0800
Review
Quoted 8 lines
If outgoing interface is set with SO_BINDTODEVICE and also sticky pktinfo the outgoing interface is exactly what SO_BINDTODEVICE set,because in my first patch,when set sticy pktinfo,outgoing interface set by SO_BINDTODEVICE is checked:
(quoted diff) +2 -0
+		if(sk->sk_bound_dev_if && pkt.ipi6_ifindex != sk->sk_bound_dev_if)
+			goto e_inval;
You may be checking this, but these two values can become out of sync if the application next makes a SO_BINDTODEVICE setsockopt() call to change the index setting.

Quoted 6 lines

Nothing makes sure the sticky ipv6 socket information is updated if that happens. And the most obvious ways to handle that are very ugly, doing some kind of call into ipv6 from the generic socket SO_BINDTODEVICE setsockopt() handler.

Hum..can we just ignore the sync of the sticky option and the SO_BINDTODEVICE, when send a message,first check sk_bound_dev_if,If it is specified,use it,otherwise use sticky option?This was what i ment. and in my patch ,the outgoing interface specified by sticky options will be overrided by the interface specified by SO_BINDTODEVICE.

Yang Hongyang

author

2008-12-10 06:36 UTC #5 lore raw

Quoted 23 lines · Yang Hongyang

Quoted 15 lines · David Miller
From: Yang Hongyang <[email protected]>
Date: Wed, 10 Dec 2008 08:42:20 +0800
Review
Quoted 8 lines
If outgoing interface is set with SO_BINDTODEVICE and also sticky pktinfo the outgoing interface is exactly what SO_BINDTODEVICE set,because in my first patch,when set sticy pktinfo,outgoing interface set by SO_BINDTODEVICE is checked:
(quoted diff) +2 -0
+		if(sk->sk_bound_dev_if && pkt.ipi6_ifindex != sk->sk_bound_dev_if)
+			goto e_inval;
You may be checking this, but these two values can become out of sync if the application next makes a SO_BINDTODEVICE setsockopt() call to change the index setting.
Quoted 6 lines
Nothing makes sure the sticky ipv6 socket information is updated if that happens. And the most obvious ways to handle that are very ugly, doing some kind of call into ipv6 from the generic socket SO_BINDTODEVICE setsockopt() handler.

Quoted 5 lines

Hum..can we just ignore the sync of the sticky option and the SO_BINDTODEVICE, when send a message,first check sk_bound_dev_if,If it is specified,use it,otherwise use sticky option?This was what i ment. and in my patch ,the outgoing interface specified by sticky options will be overrided by the interface specified by SO_BINDTODEVICE.

If this is the solution, i'll post a updated patch.

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

David Miller

maintainer

2008-12-10 09:15 UTC #6 lore raw

From: Yang Hongyang <[email protected]>
Date: Wed, 10 Dec 2008 14:38:25 +0800

Quoted 24 lines

Quoted 23 lines · Yang Hongyang
Quoted 21 lines · David Miller
From: Yang Hongyang <[email protected]>
Date: Wed, 10 Dec 2008 08:42:20 +0800
Review
Quoted 8 lines
If outgoing interface is set with SO_BINDTODEVICE and also sticky pktinfo the outgoing interface is exactly what SO_BINDTODEVICE set,because in my first patch,when set sticy pktinfo,outgoing interface set by SO_BINDTODEVICE is checked:
(quoted diff) +2 -0
+		if(sk->sk_bound_dev_if && pkt.ipi6_ifindex != sk->sk_bound_dev_if)
+			goto e_inval;
You may be checking this, but these two values can become out of sync if the application next makes a SO_BINDTODEVICE setsockopt() call to change the index setting.
Nothing makes sure the sticky ipv6 socket information is updated if that happens. And the most obvious ways to handle that are very ugly, doing some kind of call into ipv6 from the generic socket SO_BINDTODEVICE setsockopt() handler.

Quoted 7 lines

Quoted 5 lines
Hum..can we just ignore the sync of the sticky option and the SO_BINDTODEVICE, when send a message,first check sk_bound_dev_if,If it is specified,use it,otherwise use sticky option?This was what i ment. and in my patch ,the outgoing interface specified by sticky options will be overrided by the interface specified by SO_BINDTODEVICE.
If this is the solution, i'll post a updated patch.

Please do, this sounds like the correct approach.

Yang Hongyang

author

2008-12-10 09:39 UTC #7 lore raw

Quoted 3 lines · David Miller

From: Yang Hongyang <[email protected]>
Date: Wed, 10 Dec 2008 14:38:25 +0800

...snip...

Quoted 10 lines

Quoted 7 lines
Quoted 6 lines
Quoted 1 line
Hum..can we just ignore the sync of the sticky option and the SO_BINDTODEVICE, when send a message,first check sk_bound_dev_if,If it is specified,use it,otherwise use sticky option?This was what i ment. and in my patch ,the outgoing interface specified by sticky options will be overrided by the interface specified by SO_BINDTODEVICE.
If this is the solution, i'll post a updated patch.
Please do, this sounds like the correct approach.

Thanks for your comment,here is the updated patch:

1.When no interface is specified in an IPV6_PKTINFO ancillary data
item, the interface specified in an IPV6_PKTINFO sticky optionis is used.

RFC3542:
6.7. Summary of Outgoing Interface Selection

For a given outgoing packet on a given socket, the outgoing interface is determined in the following order:

1. if an interface is specified in an IPV6_PKTINFO ancillary data item, the interface is used.

2. otherwise, if an interface is specified in an IPV6_PKTINFO sticky option, the interface is used.

Signed-off-by Yang Hongyang<[email protected]>

---
 net/ipv6/udp.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

net/ipv6/udp.c +3 -0

index 8b48512..6b06421 100644
@@ -763,6 +763,9 @@ do_udp_sendmsg:
 	if (!fl.oif)
 		fl.oif = sk->sk_bound_dev_if;
 
+	if (!fl.oif)
+		fl.oif = np->sticky_pktinfo.ipi6_ifindex;
+
 	if (msg->msg_controllen) {
 		opt = &opt_space;
 		memset(opt, 0, sizeof(struct ipv6_txoptions));

Signature · 1 line

1.6.0.1

David Miller

maintainer Applied

2008-12-16 10:09 UTC #8 lore raw

From: Yang Hongyang <[email protected]>
Date: Wed, 10 Dec 2008 17:41:14 +0800

Quoted 1 line

Thanks for your comment,here is the updated patch:

...

Quoted 1 line

Signed-off-by Yang Hongyang<[email protected]>

Applied.