All of lore.kernel.org
 help / color / mirror / Atom feed
From: arno@natisbad.org (Arnaud Ebalard)
To: Scott C Otto <otts@alcatel-lucent.com>
Cc: "Brian Haley" <brian.haley@hp.com>,
	"David Miller" <davem@davemloft.net>,
	"YOSHIFUJI Hideaki / 吉藤英明" <yoshfuji@linux-ipv6.org>,
	"Jiri Olsa" <jolsa@redhat.com>,
	netdev@vger.kernel.org
Subject: Re: [REGRESSION,BISECTED] MIPv6 support broken by f4f914b58019f0
Date: Fri, 28 May 2010 10:51:18 +0200	[thread overview]
Message-ID: <87hblss92x.fsf@small.ssi.corp> (raw)
In-Reply-To: <4BFEE49C.8060301@lucent.com> (Scott C. Otto's message of "Thu, 27 May 2010 16:31:08 -0500")

Hi,

Scott C Otto <otts@alcatel-lucent.com> writes:

> All,
> Thanks for looking into this.
>
> The behavior of SO_BINDTODEVICE, certainly with IPV4, is to identify a specific
> interface to use for sending/receiving AF_INET packets upon.   And that's how it
> has behaved with IPV4.   Tools like ping, traceroute (and ping6, traceroute6)
> make use of it with the -i (interface) options.
>
> We ran into this issue when updating an IPV4 application to IPV6.   The report
> provided one example of the issue.
>
> The original change was based on the semantics of flowi.oif used in IPV4.  There
> flowi.oif (as it is with IPV6 as well) is also set to sk->sk_bound_dev_if when
> using ip_route_connect() and routing simply took a non-zero oif to enforce an
> interface.  Looking at IPV4 tunneling, flowi.oif is set the same way from
> tunnel's parms.link as with IPV6 so would follow those semantics as well.
>
> Obviously, with IPV6, the semantics of flowi.oif are different and perhaps even
> vary with the users of IPV6.
>
> If that variability is desired, the proposed change from Brian (using
> sk->sk_bound_dev_if) would be an equivalent means of supporting SO_BINDTODEVICE
> while limiting the impact.

ok. Thanks for the feedback. Can you comment on what is below?

>> The below might actually be what was actually intended, triggering
>> on what the user forced, rather than assuming all callers require
>> strict behavior.
>> 
>> -Brian
>> 
>> 
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index 294cbe8..252d761 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -814,7 +814,7 @@ struct dst_entry * ip6_route_output(struct net *net, struct sock *sk,
>>  {
>>  	int flags = 0;
>>  
>> -	if (fl->oif || rt6_need_strict(&fl->fl6_dst))
>> +	if ((sk && sk->sk_bound_dev_if) || rt6_need_strict(&fl->fl6_dst))
>>  		flags |= RT6_LOOKUP_F_IFACE;
>>  
>>  	if (!ipv6_addr_any(&fl->fl6_src))

Brian, I tested the patch on my Mobile Node: it fixes the regression. I
also updated the kernel on my Home Agent to a 2.6.34 with that fix and
everything works as expected. *For that aspect* and fwiw, you get my

Tested-by: Arnaud Ebalard <arno@natisbad.org>

For the SO_BINDTODEVICE aspect, I don't have code at hand to test if the
fix works as expected. We should also double check that this will not
break other paths which use the sk->sk_bound_dev_if with a different
semantic:

$ grep -R sk_bound_dev_if net/ | wc -l
125
$ grep -R 'sk_bound_dev_if = ' net/
net/ieee802154/raw.c:   sk->sk_bound_dev_if = dev->ifindex;
net/core/sock.c:        sk->sk_bound_dev_if = index;
net/ipv6/datagram.c:    sk->sk_bound_dev_if = usin->sin6_scope_id;
net/ipv6/datagram.c:    sk->sk_bound_dev_if = np->mcast_oif;
net/ipv6/af_inet6.c:    sk->sk_bound_dev_if = addr->sin6_scope_id;
net/ipv6/tcp_ipv6.c:    sk->sk_bound_dev_if = usin->sin6_scope_id;
net/ipv6/tcp_ipv6.c:    newsk->sk_bound_dev_if = treq->iif;
net/ipv6/raw.c:         sk->sk_bound_dev_if = addr->sin6_scope_id;
net/sctp/socket.c:      newsk->sk_bound_dev_if = sk->sk_bound_dev_if;
net/ipv4/ip_output.c:   sk->sk_bound_dev_if = arg->bound_dev_if;
net/ipv4/udp.c:         sk->sk_bound_dev_if = 0;
net/dccp/ipv6.c:        newsk->sk_bound_dev_if = ireq6->iif;
net/dccp/ipv6.c:        sk->sk_bound_dev_if = usin->sin6_scope_id;

Cheers,

a+

  reply	other threads:[~2010-05-28  8:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-26 17:01 [REGRESSION,BISECTED] MIPv6 support broken by f4f914b58019f0 Arnaud Ebalard
2010-05-27  0:48 ` Brian Haley
2010-05-27 15:14   ` Arnaud Ebalard
2010-05-27 19:39     ` Brian Haley
2010-05-27 21:01       ` Arnaud Ebalard
2010-05-28 18:40         ` YOSHIFUJI Hideaki
2010-05-28 21:15           ` Arnaud Ebalard
2010-05-27 21:31       ` Scott C Otto
2010-05-28  8:51         ` Arnaud Ebalard [this message]
2010-05-28 17:59           ` Brian Haley
2010-05-28 18:17             ` [PATCH] IPv6: fix Mobile IPv6 regression Brian Haley
2010-05-29  6:03               ` David Miller
2010-05-31  8:46               ` Jiri Olsa
2010-05-31 12:49                 ` Jiri Olsa
2010-05-27 17:39   ` [REGRESSION,BISECTED] MIPv6 support broken by f4f914b58019f0 YOSHIFUJI Hideaki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87hblss92x.fsf@small.ssi.corp \
    --to=arno@natisbad.org \
    --cc=brian.haley@hp.com \
    --cc=davem@davemloft.net \
    --cc=jolsa@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=otts@alcatel-lucent.com \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.