Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[Hajime Tazaki]

Hello David,

sorry for the delay.

At Sun, 11 Oct 2015 12:01:30 -0600,
David Ahern wrote:
> 
> On 10/11/15 8:24 AM, Hajime Tazaki wrote:
> >
> > I've faced this issue since the following patch was applied.
> >
> > commit 741a11d9e4103a8e1c590ef1280143fe654e4e33
> > Author: David Ahern 
> > Date:   Mon Sep 28 10:12:13 2015 -0700
> >
> >  net: ipv6: Add RT6_LOOKUP_F_IFACE flag if oif is set
> >
> > I still couldn't spot which part (other than my posted call
> > graph) is broken and am not sure whether the xfrm change
> > affects or not (which I need to check the mip6 code again).
> 
> Ok, this is a separate problem from what Steffen is hitting.

agree.

> >
> >> Can you apply this patch, and then run:
> >>
> >> perf record -e fib6:* -a -g
> >> perf script
> >
> > I'm using libos environment right now, so the perf trace
> > can't be used as it is.
> 
> ok.
> 
> Some path in raw6_sendmsg is setting fl6.flowi6_oif. Can you instrument it?

yes, this sendmsg uses non-zero flowi6_oif.


the conditions are

- sendmsg () with INET6/RAW socket (with IPPROTO_MH)
- ip6_pktinfo.ipi6_addr (fl6.saddr) and ipi6_oif
  (fl6.flowi6_oif) are non-NULL.
=> ipi6_addr (fl6.saddr) is not the IP address of oif, but
another interfaces (home address of mip6)
- fib6_lookup() (in ip6_pol_route()) gives ip6_null_entry

if RT6_LOOKUP_F_IFACE isn't set at ip6_route_output, it
can look for a proper dst_entry of default route
if not, it gives EINVAL.

I'm sure that this is not the right fix for this issue, but
the following patch solves my situation.

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index df24cff4a0cb..02e86989b3cb 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1079,6 +1079,12 @@ redo_rt6_select:
fn = fib6_backtrack(fn, >saddr);
if (fn) 
goto redo_rt6_select;
+   else if (strict & RT6_LOOKUP_F_IFACE) {
+   /* also consider non-interface route */
+   strict &= ~RT6_LOOKUP_F_IFACE;
+   fn = saved_fn;
+   goto redo_rt6_select;
+   }
else if (strict & RT6_LOOKUP_F_REACHABLE) {
/* also consider unreachable route */
strict &= ~RT6_LOOKUP_F_REACHABLE;

I'm trying to create a minimum reproducible code and spot
the issue but not get it yet. let me know if you find any
good idea.

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

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Ahern]

On 10/20/15 6:31 AM, Hajime Tazaki wrote:

yes, this sendmsg uses non-zero flowi6_oif.


the conditions are

- sendmsg () with INET6/RAW socket (with IPPROTO_MH)
- ip6_pktinfo.ipi6_addr (fl6.saddr) and ipi6_oif
   (fl6.flowi6_oif) are non-NULL.
=> ipi6_addr (fl6.saddr) is not the IP address of oif, but
 another interfaces (home address of mip6)


interesting. so forcing a send out of interface X but using the source 
address of interface Y.


Does the attached patch work for you?
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d0619632723a..2701cb3d88e9 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1171,6 +1171,7 @@ struct dst_entry *ip6_route_output(struct net *net, const 
struct sock *sk,
 {
struct dst_entry *dst;
int flags = 0;
+   bool any_src;
 
dst = l3mdev_rt6_dst_by_oif(net, fl6);
if (dst)
@@ -1178,11 +1179,12 @@ struct dst_entry *ip6_route_output(struct net *net, 
const struct sock *sk,
 
fl6->flowi6_iif = LOOPBACK_IFINDEX;
 
+   any_src = ipv6_addr_any(>saddr);
if ((sk && sk->sk_bound_dev_if) || rt6_need_strict(>daddr) ||
-   fl6->flowi6_oif)
+   (fl6->flowi6_oif && any_src))
flags |= RT6_LOOKUP_F_IFACE;
 
-   if (!ipv6_addr_any(>saddr))
+   if (!any_src)
flags |= RT6_LOOKUP_F_HAS_SADDR;
else if (sk)
flags |= rt6_srcprefs2flags(inet6_sk(sk)->srcprefs);

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[Hajime Tazaki]

At Tue, 20 Oct 2015 11:48:48 -0600,
David Ahern wrote:

> > the conditions are
> >
> > - sendmsg () with INET6/RAW socket (with IPPROTO_MH)
> > - ip6_pktinfo.ipi6_addr (fl6.saddr) and ipi6_oif
> >(fl6.flowi6_oif) are non-NULL.
> > => ipi6_addr (fl6.saddr) is not the IP address of oif, but
> >  another interfaces (home address of mip6)
> 
> interesting. so forcing a send out of interface X but using the source 
> address of interface Y.
> 
> Does the attached patch work for you?

this patch works fine for the above case.
other tests of mine are also fine.

many thanks !

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

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[Steffen Klassert]

On Mon, Oct 12, 2015 at 12:49:29PM -0600, David Ahern wrote:
> On 10/9/15 11:27 AM, David Ahern wrote:
> >On 10/9/15 1:17 AM, Steffen Klassert wrote:
> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
> index 30caa289c5db..5cedfda4b241 100644
> --- a/net/ipv6/xfrm6_policy.c
> +++ b/net/ipv6/xfrm6_policy.c
> @@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct
> net *net, int tos, int oif,
> 
>   memset(, 0, sizeof(fl6));
>   fl6.flowi6_oif = oif;
> +fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
>   memcpy(, daddr, sizeof(fl6.daddr));
>   if (saddr)
>   memcpy(, saddr, sizeof(fl6.saddr));
> >>>
> >>>I found that this fix is still not sufficient with the mip6
> >>>(Mobile IPv6) use case.
> >>
> >>It does not even fix the vti case. The behaviour of the vti devices is
> >>the same, with and without the patch.
> >>
> >
> >The attached patch applied to Linus' tree works for me. Currently the
> >above change is not in his tree, so I added it to this patch. Once you
> >confirm that it works for you I'll create the delta-patch for net and
> >send out.
> 
> Steffen: Have you had a chance to try the patch? Does it solve the
> vti6 problem for you?

The delta of your patch and the current net tree fixes the
vti6 problems.

Current net-next works without any changes.

Thanks David!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[Steffen Klassert]

Hi David.

On Mon, Oct 12, 2015 at 12:49:29PM -0600, David Ahern wrote:
> On 10/9/15 11:27 AM, David Ahern wrote:
> >
> >The attached patch applied to Linus' tree works for me. Currently the
> >above change is not in his tree, so I added it to this patch. Once you
> >confirm that it works for you I'll create the delta-patch for net and
> >send out.
> 
> Steffen: Have you had a chance to try the patch? Does it solve the
> vti6 problem for you?
> 

I'm not at office this week, so not able to do a test
before monday. I let you know as soon as I tried the
patch.

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

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Ahern]

On 10/9/15 11:27 AM, David Ahern wrote:

On 10/9/15 1:17 AM, Steffen Klassert wrote:

diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 30caa289c5db..5cedfda4b241 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct
net *net, int tos, int oif,

  memset(, 0, sizeof(fl6));
  fl6.flowi6_oif = oif;
+fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
  memcpy(, daddr, sizeof(fl6.daddr));
  if (saddr)
  memcpy(, saddr, sizeof(fl6.saddr));


I found that this fix is still not sufficient with the mip6
(Mobile IPv6) use case.


It does not even fix the vti case. The behaviour of the vti devices is
the same, with and without the patch.



The attached patch applied to Linus' tree works for me. Currently the
above change is not in his tree, so I added it to this patch. Once you
confirm that it works for you I'll create the delta-patch for net and
send out.


Steffen: Have you had a chance to try the patch? Does it solve the vti6 
problem for you?


David

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

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[Hajime Tazaki]

At Fri, 9 Oct 2015 11:27:36 -0600,
David Ahern wrote:
> 
> [1  ]
> On 10/9/15 1:17 AM, Steffen Klassert wrote:
> >>> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
> >>> index 30caa289c5db..5cedfda4b241 100644
> >>> --- a/net/ipv6/xfrm6_policy.c
> >>> +++ b/net/ipv6/xfrm6_policy.c
> >>> @@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct net 
> >>> *net, int tos, int oif,
> >>>
> >>>   memset(, 0, sizeof(fl6));
> >>>   fl6.flowi6_oif = oif;
> >>> + fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
> >>>   memcpy(, daddr, sizeof(fl6.daddr));
> >>>   if (saddr)
> >>>   memcpy(, saddr, sizeof(fl6.saddr));
> >>
> >> I found that this fix is still not sufficient with the mip6
> >> (Mobile IPv6) use case.
> >
> > It does not even fix the vti case. The behaviour of the vti devices is
> > the same, with and without the patch.
> >
> 
> The attached patch applied to Linus' tree works for me. Currently the 
> above change is not in his tree, so I added it to this patch. Once you 
> confirm that it works for you I'll create the delta-patch for net and 
> send out.

I gave it a try but without any luck unfortunately.
I may need to look carefully what mip6 does here.

the code path where I'm looking at for MH packet (raw socket
with IPPROTO_MH) is:

#0  ip6_route_output (net=0x73a40a40 , sk=0x730b5c50, 
fl6=0x70ed2fe0) at net/ipv6/route.c:1195
#1  0x735d155f in ip6_dst_lookup_tail (net=0x73a40a40 
, sk=0x730b5c50, dst=0x70ed2f18, fl6=0x70ed2fe0)
at net/ipv6/ip6_output.c:929
#2  0x735d1707 in ip6_dst_lookup_flow (sk=0x730b5c50, 
fl6=0x70ed2fe0, final_dst=0x0) at net/ipv6/ip6_output.c:1024
#3  0x736199f7 in rawv6_sendmsg (sk=0x730b5c50, msg=0x70ed3320, 
len=32) at net/ipv6/raw.c:872
#4  0x73526d21 in inet_sendmsg (sock=0x730b5810, 
msg=0x70ed3320, size=32) at net/ipv4/af_inet.c:737
#5  0x7338dc8a in sock_sendmsg_nosec (msg=0x70ed3320, 
sock=0x730b5810) at net/socket.c:610
#6  sock_sendmsg (sock=0x730b5810, msg=0x70ed3320) at net/socket.c:620


which (*dst)->error of ip6_route_output gives -22 (EINVAL).

I will be back once I got any findings.

-- Hajime

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

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Ahern]

On 10/11/15 7:22 AM, Hajime Tazaki wrote:


At Fri, 9 Oct 2015 11:27:36 -0600,
David Ahern wrote:


[1  ]
On 10/9/15 1:17 AM, Steffen Klassert wrote:

diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 30caa289c5db..5cedfda4b241 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct net *net, 
int tos, int oif,

memset(, 0, sizeof(fl6));
fl6.flowi6_oif = oif;
+   fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
memcpy(, daddr, sizeof(fl6.daddr));
if (saddr)
memcpy(, saddr, sizeof(fl6.saddr));


I found that this fix is still not sufficient with the mip6
(Mobile IPv6) use case.


It does not even fix the vti case. The behaviour of the vti devices is
the same, with and without the patch.



The attached patch applied to Linus' tree works for me. Currently the
above change is not in his tree, so I added it to this patch. Once you
confirm that it works for you I'll create the delta-patch for net and
send out.


I gave it a try but without any luck unfortunately.
I may need to look carefully what mip6 does here.

the code path where I'm looking at for MH packet (raw socket
with IPPROTO_MH) is:

#0  ip6_route_output (net=0x73a40a40 , sk=0x730b5c50, 
fl6=0x70ed2fe0) at net/ipv6/route.c:1195
#1  0x735d155f in ip6_dst_lookup_tail (net=0x73a40a40 
, sk=0x730b5c50, dst=0x70ed2f18, fl6=0x70ed2fe0)
 at net/ipv6/ip6_output.c:929
#2  0x735d1707 in ip6_dst_lookup_flow (sk=0x730b5c50, 
fl6=0x70ed2fe0, final_dst=0x0) at net/ipv6/ip6_output.c:1024
#3  0x736199f7 in rawv6_sendmsg (sk=0x730b5c50, msg=0x70ed3320, 
len=32) at net/ipv6/raw.c:872
#4  0x73526d21 in inet_sendmsg (sock=0x730b5810, 
msg=0x70ed3320, size=32) at net/ipv4/af_inet.c:737
#5  0x7338dc8a in sock_sendmsg_nosec (msg=0x70ed3320, 
sock=0x730b5810) at net/socket.c:610
#6  sock_sendmsg (sock=0x730b5810, msg=0x70ed3320) at net/socket.c:620


which (*dst)->error of ip6_route_output gives -22 (EINVAL).

I will be back once I got any findings.


How does a xfrm change affect this code path?

Can you apply this patch, and then run:

perf record -e fib6:* -a -g
perf script

Thanks,
David


>From 5da83796c110d5f2c995b22b38be8e5268a7f573 Mon Sep 17 00:00:00 2001
From: David Ahern 
Date: Thu, 1 Oct 2015 14:52:58 -0700
Subject: [PATCH net-next] net: IPv6 fib and route tracepoints

Signed-off-by: David Ahern 
---
 include/trace/events/fib6.h | 237 
 net/core/net-traces.c   |   7 ++
 net/ipv6/route.c|  24 -
 3 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 include/trace/events/fib6.h

diff --git a/include/trace/events/fib6.h b/include/trace/events/fib6.h
new file mode 100644
index ..9f34da0d59a3
--- /dev/null
+++ b/include/trace/events/fib6.h
@@ -0,0 +1,237 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM fib6
+
+#if !defined(_TRACE_FIB6_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_FIB6_H
+
+#include 
+#include 
+#include 
+#include 
+
+TRACE_EVENT(fib6_table_lookup,
+
+   TP_PROTO(const struct net *net, const struct rt6_info *rt,
+u32 tb_id, const struct flowi6 *flp),
+
+   TP_ARGS(net, rt, tb_id, flp),
+
+   TP_STRUCT__entry(
+   __field(u32,tb_id   )
+
+   __field(int,oif )
+   __field(int,iif )
+   __field(__u8,   tos )
+   __field(__u8,   scope   )
+   __field(__u8,   flags   )
+   __array(__u8,   src,16  )
+   __array(__u8,   dst,16  )
+
+   __dynamic_array(char,   name,   IFNAMSIZ )
+   __array(__u8,   gw, 16   )
+   ),
+
+   TP_fast_assign(
+   struct in6_addr *in6;
+
+   __entry->tb_id = tb_id;
+   __entry->oif = flp->flowi6_oif;
+   __entry->iif = flp->flowi6_iif;
+   __entry->tos = flp->flowi6_tos;
+   __entry->scope = flp->flowi6_scope;
+   __entry->flags = flp->flowi6_flags;
+
+   in6 = (struct in6_addr *)__entry->src;
+   *in6 = flp->saddr;
+
+   in6 = (struct in6_addr *)__entry->dst;
+   *in6 = flp->daddr;
+
+   if (rt->rt6i_idev) {
+   __assign_str(name, rt->rt6i_idev->dev->name);
+   } else {
+   __assign_str(name, "");
+   }
+   if (rt == net->ipv6.ip6_null_entry) {
+   struct in6_addr in6_zero = {};
+
+   in6 = (struct in6_addr *)__entry->gw;
+ 

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[Hajime Tazaki]

At Sun, 11 Oct 2015 07:31:32 -0600,
David Ahern wrote:

> >> The attached patch applied to Linus' tree works for me. Currently the
> >> above change is not in his tree, so I added it to this patch. Once you
> >> confirm that it works for you I'll create the delta-patch for net and
> >> send out.
> >
> > I gave it a try but without any luck unfortunately.
> > I may need to look carefully what mip6 does here.
> >
> > the code path where I'm looking at for MH packet (raw socket
> > with IPPROTO_MH) is:
> >
> > #0  ip6_route_output (net=0x73a40a40 , 
> > sk=0x730b5c50, fl6=0x70ed2fe0) at net/ipv6/route.c:1195
> > #1  0x735d155f in ip6_dst_lookup_tail (net=0x73a40a40 
> > , sk=0x730b5c50, dst=0x70ed2f18, 
> > fl6=0x70ed2fe0)
> >  at net/ipv6/ip6_output.c:929
> > #2  0x735d1707 in ip6_dst_lookup_flow (sk=0x730b5c50, 
> > fl6=0x70ed2fe0, final_dst=0x0) at net/ipv6/ip6_output.c:1024
> > #3  0x736199f7 in rawv6_sendmsg (sk=0x730b5c50, 
> > msg=0x70ed3320, len=32) at net/ipv6/raw.c:872
> > #4  0x73526d21 in inet_sendmsg (sock=0x730b5810, 
> > msg=0x70ed3320, size=32) at net/ipv4/af_inet.c:737
> > #5  0x7338dc8a in sock_sendmsg_nosec (msg=0x70ed3320, 
> > sock=0x730b5810) at net/socket.c:610
> > #6  sock_sendmsg (sock=0x730b5810, msg=0x70ed3320) at 
> > net/socket.c:620
> >
> >
> > which (*dst)->error of ip6_route_output gives -22 (EINVAL).
> >
> > I will be back once I got any findings.
> 
> How does a xfrm change affect this code path?

I've faced this issue since the following patch was applied.

commit 741a11d9e4103a8e1c590ef1280143fe654e4e33
Author: David Ahern 
Date:   Mon Sep 28 10:12:13 2015 -0700

net: ipv6: Add RT6_LOOKUP_F_IFACE flag if oif is set

I still couldn't spot which part (other than my posted call
graph) is broken and am not sure whether the xfrm change
affects or not (which I need to check the mip6 code again).

> Can you apply this patch, and then run:
> 
> perf record -e fib6:* -a -g
> perf script

I'm using libos environment right now, so the perf trace
can't be used as it is.

I'll keep post here.

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

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Ahern]

On 10/11/15 8:24 AM, Hajime Tazaki wrote:


I've faced this issue since the following patch was applied.

commit 741a11d9e4103a8e1c590ef1280143fe654e4e33
Author: David Ahern 
Date:   Mon Sep 28 10:12:13 2015 -0700

 net: ipv6: Add RT6_LOOKUP_F_IFACE flag if oif is set

I still couldn't spot which part (other than my posted call
graph) is broken and am not sure whether the xfrm change
affects or not (which I need to check the mip6 code again).


Ok, this is a separate problem from what Steffen is hitting.




Can you apply this patch, and then run:

perf record -e fib6:* -a -g
perf script


I'm using libos environment right now, so the perf trace
can't be used as it is.


ok.

Some path in raw6_sendmsg is setting fl6.flowi6_oif. Can you instrument it?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[Hajime Tazaki]

Hello David,

At Mon,  5 Oct 2015 08:32:51 -0600,
David Ahern wrote:

> 
> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
> index 30caa289c5db..5cedfda4b241 100644
> --- a/net/ipv6/xfrm6_policy.c
> +++ b/net/ipv6/xfrm6_policy.c
> @@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct net *net, 
> int tos, int oif,
>  
>   memset(, 0, sizeof(fl6));
>   fl6.flowi6_oif = oif;
> + fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
>   memcpy(, daddr, sizeof(fl6.daddr));
>   if (saddr)
>   memcpy(, saddr, sizeof(fl6.saddr));

I found that this fix is still not sufficient with the mip6
(Mobile IPv6) use case.

FLOWI_FLAG_SKIP_NH_OIF is not checked anywhere else in ipv6
code, in ip6_route_output() etc.

Even if I added the check (like below), MH packets are not
sent at all from mobile node, home agent.

do you have any idea ?

I have a reproducible setup here with mip6. let me know if
you need further information.


diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8c0898796ffb..0aba308b5ea3 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1169,9 +1169,9 @@ struct dst_entry *ip6_route_output(struct net *net, const 
struct sock *sk,
 
fl6->flowi6_iif = LOOPBACK_IFINDEX;
 
-   if ((sk && sk->sk_bound_dev_if) || rt6_need_strict(>daddr))
+   if ((sk && sk->sk_bound_dev_if) || rt6_need_strict(>daddr) ||
+   (!(fl6->flowi6_flags & FLOWI_FLAG_SKIP_NH_OIF) && fl6->flowi6_oif))
flags |= RT6_LOOKUP_F_IFACE;

if (!ipv6_addr_any(>saddr))
flags |= RT6_LOOKUP_F_HAS_SADDR;
else if (sk)

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

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[Steffen Klassert]

On Fri, Oct 09, 2015 at 03:54:22PM +0900, Hajime Tazaki wrote:
> 
> Hello David,
> 
> At Mon,  5 Oct 2015 08:32:51 -0600,
> David Ahern wrote:
> 
> > 
> > diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
> > index 30caa289c5db..5cedfda4b241 100644
> > --- a/net/ipv6/xfrm6_policy.c
> > +++ b/net/ipv6/xfrm6_policy.c
> > @@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct net 
> > *net, int tos, int oif,
> >  
> > memset(, 0, sizeof(fl6));
> > fl6.flowi6_oif = oif;
> > +   fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
> > memcpy(, daddr, sizeof(fl6.daddr));
> > if (saddr)
> > memcpy(, saddr, sizeof(fl6.saddr));
> 
> I found that this fix is still not sufficient with the mip6
> (Mobile IPv6) use case.

It does not even fix the vti case. The behaviour of the vti devices is
the same, with and without the patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Ahern]

On 10/9/15 1:17 AM, Steffen Klassert wrote:

On Fri, Oct 09, 2015 at 03:54:22PM +0900, Hajime Tazaki wrote:


Hello David,

At Mon,  5 Oct 2015 08:32:51 -0600,
David Ahern wrote:



diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 30caa289c5db..5cedfda4b241 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct net *net, 
int tos, int oif,

memset(, 0, sizeof(fl6));
fl6.flowi6_oif = oif;
+   fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
memcpy(, daddr, sizeof(fl6.daddr));
if (saddr)
memcpy(, saddr, sizeof(fl6.saddr));


I found that this fix is still not sufficient with the mip6
(Mobile IPv6) use case.


It does not even fix the vti case. The behaviour of the vti devices is
the same, with and without the patch.



I goofed this patch was on top of my IPv6 VRF patches. You need the 
FLOWI_FLAG_SKIP_NH_OIF bits from:


http://www.spinics.net/lists/netdev/msg346860.html

Let me cook up a patch based on Linus' tree which is where it is needed.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Ahern]

On 10/9/15 1:17 AM, Steffen Klassert wrote:

diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 30caa289c5db..5cedfda4b241 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct net *net, 
int tos, int oif,

memset(, 0, sizeof(fl6));
fl6.flowi6_oif = oif;
+   fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
memcpy(, daddr, sizeof(fl6.daddr));
if (saddr)
memcpy(, saddr, sizeof(fl6.saddr));


I found that this fix is still not sufficient with the mip6
(Mobile IPv6) use case.


It does not even fix the vti case. The behaviour of the vti devices is
the same, with and without the patch.



The attached patch applied to Linus' tree works for me. Currently the 
above change is not in his tree, so I added it to this patch. Once you 
confirm that it works for you I'll create the delta-patch for net and 
send out.


Thanks,
David
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 92b1aa38f121..2dbd73014a1b 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -874,7 +874,8 @@ static struct dst_entry *ip6_sk_dst_check(struct sock *sk,
 #ifdef CONFIG_IPV6_SUBTREES
ip6_rt_check(>rt6i_src, >saddr, np->saddr_cache) ||
 #endif
-   (fl6->flowi6_oif && fl6->flowi6_oif != dst->dev->ifindex)) {
+  (!(fl6->flowi6_flags & FLOWI_FLAG_SKIP_NH_OIF) &&
+ (fl6->flowi6_oif && fl6->flowi6_oif != dst->dev->ifindex))) {
dst_release(dst);
dst = NULL;
}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index cb32ce250db0..df24cff4a0cb 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1068,6 +1068,9 @@ static struct rt6_info *ip6_pol_route(struct net *net, 
struct fib6_table *table,
fn = fib6_lookup(>tb6_root, >daddr, >saddr);
saved_fn = fn;
 
+   if (fl6->flowi6_flags & FLOWI_FLAG_SKIP_NH_OIF)
+   oif = 0;
+
 redo_rt6_select:
rt = rt6_select(fn, oif, strict);
if (rt->rt6i_nsiblings)
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 30caa289c5db..5cedfda4b241 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct net *net, 
int tos, int oif,
 
memset(, 0, sizeof(fl6));
fl6.flowi6_oif = oif;
+   fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
memcpy(, daddr, sizeof(fl6.daddr));
if (saddr)
memcpy(, saddr, sizeof(fl6.saddr));

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Miller]

From: David Ahern 
Date: Mon,  5 Oct 2015 08:32:51 -0600

> It occurred to me yesterday that 741a11d9e4103 ("net: ipv6: Add
> RT6_LOOKUP_F_IFACE flag if oif is set") means that xfrm6_dst_lookup
> needs the FLOWI_FLAG_SKIP_NH_OIF flag set. This latest commit causes
> the oif to be considered in lookups which is known to break vti. This
> explains why 58189ca7b274 did not the IPv6 change at the time it was
> submitted.
> 
> Fixes: 42a7b32b73d6 ("xfrm: Add oif to dst lookups")
> Signed-off-by: David Ahern 

If this is needed in v4.3 why are you targetting net-next?

I applied it to both trees...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Ahern]

On 10/7/15 5:25 AM, David Miller wrote:

From: David Ahern 
Date: Mon,  5 Oct 2015 08:32:51 -0600


It occurred to me yesterday that 741a11d9e4103 ("net: ipv6: Add
RT6_LOOKUP_F_IFACE flag if oif is set") means that xfrm6_dst_lookup
needs the FLOWI_FLAG_SKIP_NH_OIF flag set. This latest commit causes
the oif to be considered in lookups which is known to break vti. This
explains why 58189ca7b274 did not the IPv6 change at the time it was
submitted.

Fixes: 42a7b32b73d6 ("xfrm: Add oif to dst lookups")
Signed-off-by: David Ahern 


If this is needed in v4.3 why are you targetting net-next?


default git setting. Missed removal before send.



I applied it to both trees...



thank you.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next] net: Fix vti use case with oif in dst lookups for IPv6

[David Ahern]

It occurred to me yesterday that 741a11d9e4103 ("net: ipv6: Add
RT6_LOOKUP_F_IFACE flag if oif is set") means that xfrm6_dst_lookup
needs the FLOWI_FLAG_SKIP_NH_OIF flag set. This latest commit causes
the oif to be considered in lookups which is known to break vti. This
explains why 58189ca7b274 did not the IPv6 change at the time it was
submitted.

Fixes: 42a7b32b73d6 ("xfrm: Add oif to dst lookups")
Signed-off-by: David Ahern 
---
This is needed in 4.3.

 net/ipv6/xfrm6_policy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 30caa289c5db..5cedfda4b241 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -37,6 +37,7 @@ static struct dst_entry *xfrm6_dst_lookup(struct net *net, 
int tos, int oif,
 
memset(, 0, sizeof(fl6));
fl6.flowi6_oif = oif;
+   fl6.flowi6_flags = FLOWI_FLAG_SKIP_NH_OIF;
memcpy(, daddr, sizeof(fl6.daddr));
if (saddr)
memcpy(, saddr, sizeof(fl6.saddr));
-- 
1.9.1

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