* [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups
@ 2016-06-23 13:25 Paolo Abeni
2016-06-23 14:20 ` David Ahern
2016-06-27 14:37 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Paolo Abeni @ 2016-06-23 13:25 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, David Ahern, Beniamino Galvani
with the commit 8c14586fc320 ("net: ipv6: Use passed in table for
nexthop lookups"), net hop lookup is first performed on route creation
in the passed-in table.
However device match is not enforced in table lookup, so the found
route can be later discarded due to egress device mismatch and no
global lookup will be performed.
This cause the following to fail:
ip link add dummy1 type dummy
ip link add dummy2 type dummy
ip link set dummy1 up
ip link set dummy2 up
ip route add 2001:db8:8086::/48 dev dummy1 metric 20
ip route add 2001:db8:d34d::/64 via 2001:db8:8086::2 dev dummy1 metric 20
ip route add 2001:db8:8086::/48 dev dummy2 metric 21
ip route add 2001:db8:d34d::/64 via 2001:db8:8086::2 dev dummy2 metric 21
RTNETLINK answers: No route to host
This change fixes the issue enforcing device lookup in
ip6_nh_lookup_table()
v1->v2: updated commit message title
Fixes: 8c14586fc320 ("net: ipv6: Use passed in table for nexthop lookups")
Reported-and-tested-by: Beniamino Galvani <bgalvani@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 969913d..520b788 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1782,7 +1782,7 @@ static struct rt6_info *ip6_nh_lookup_table(struct net *net,
};
struct fib6_table *table;
struct rt6_info *rt;
- int flags = 0;
+ int flags = RT6_LOOKUP_F_IFACE;
table = fib6_get_table(net, cfg->fc_table);
if (!table)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups
2016-06-23 13:25 [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups Paolo Abeni
@ 2016-06-23 14:20 ` David Ahern
2016-06-23 14:29 ` David Ahern
2016-06-27 14:37 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: David Ahern @ 2016-06-23 14:20 UTC (permalink / raw)
To: Paolo Abeni, netdev; +Cc: David S. Miller, Beniamino Galvani
On 6/23/16 7:25 AM, Paolo Abeni wrote:
> with the commit 8c14586fc320 ("net: ipv6: Use passed in table for
> nexthop lookups"), net hop lookup is first performed on route creation
> in the passed-in table.
> However device match is not enforced in table lookup, so the found
> route can be later discarded due to egress device mismatch and no
> global lookup will be performed.
> This cause the following to fail:
>
> ip link add dummy1 type dummy
> ip link add dummy2 type dummy
> ip link set dummy1 up
> ip link set dummy2 up
> ip route add 2001:db8:8086::/48 dev dummy1 metric 20
> ip route add 2001:db8:d34d::/64 via 2001:db8:8086::2 dev dummy1 metric 20
> ip route add 2001:db8:8086::/48 dev dummy2 metric 21
> ip route add 2001:db8:d34d::/64 via 2001:db8:8086::2 dev dummy2 metric 21
> RTNETLINK answers: No route to host
>
> This change fixes the issue enforcing device lookup in
> ip6_nh_lookup_table()
>
> v1->v2: updated commit message title
>
> Fixes: 8c14586fc320 ("net: ipv6: Use passed in table for nexthop lookups")
> Reported-and-tested-by: Beniamino Galvani <bgalvani@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/ipv6/route.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 969913d..520b788 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1782,7 +1782,7 @@ static struct rt6_info *ip6_nh_lookup_table(struct net *net,
> };
> struct fib6_table *table;
> struct rt6_info *rt;
> - int flags = 0;
> + int flags = RT6_LOOKUP_F_IFACE;
>
> table = fib6_get_table(net, cfg->fc_table);
> if (!table)
>
Acked-by: David Ahern <dsa@cumulusnetworks.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups
2016-06-23 14:20 ` David Ahern
@ 2016-06-23 14:29 ` David Ahern
2016-06-23 14:39 ` Paolo Abeni
0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2016-06-23 14:29 UTC (permalink / raw)
To: Paolo Abeni, netdev; +Cc: David S. Miller, Beniamino Galvani
On 6/23/16 8:20 AM, David Ahern wrote:
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index 969913d..520b788 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -1782,7 +1782,7 @@ static struct rt6_info
>> *ip6_nh_lookup_table(struct net *net,
>> };
>> struct fib6_table *table;
>> struct rt6_info *rt;
>> - int flags = 0;
>> + int flags = RT6_LOOKUP_F_IFACE;
>>
>> table = fib6_get_table(net, cfg->fc_table);
>> if (!table)
>>
>
> Acked-by: David Ahern <dsa@cumulusnetworks.com>
I take that back.
I think RT6_LOOKUP_F_IFACE should only be set if cfg->fc_ifindex is set.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups
2016-06-23 14:29 ` David Ahern
@ 2016-06-23 14:39 ` Paolo Abeni
2016-06-23 20:33 ` David Ahern
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2016-06-23 14:39 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, David S. Miller, Beniamino Galvani
On Thu, 2016-06-23 at 08:29 -0600, David Ahern wrote:
> On 6/23/16 8:20 AM, David Ahern wrote:
> >> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> >> index 969913d..520b788 100644
> >> --- a/net/ipv6/route.c
> >> +++ b/net/ipv6/route.c
> >> @@ -1782,7 +1782,7 @@ static struct rt6_info
> >> *ip6_nh_lookup_table(struct net *net,
> >> };
> >> struct fib6_table *table;
> >> struct rt6_info *rt;
> >> - int flags = 0;
> >> + int flags = RT6_LOOKUP_F_IFACE;
> >>
> >> table = fib6_get_table(net, cfg->fc_table);
> >> if (!table)
> >>
> >
> > Acked-by: David Ahern <dsa@cumulusnetworks.com>
>
> I take that back.
>
> I think RT6_LOOKUP_F_IFACE should only be set if cfg->fc_ifindex is set.
AFAICS the latter condition should not be needed. The related
information is passed all way down to rt6_score_route(), where it's
really used:
m = rt6_check_dev(rt, oif);
if (!m && (strict & RT6_LOOKUP_F_IFACE))
return RT6_NUD_FAIL_HARD;
and 'm' can be 0 only if oif is set: RT6_LOOKUP_F_IFACE has no effect
ifindex is set.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups
2016-06-23 14:39 ` Paolo Abeni
@ 2016-06-23 20:33 ` David Ahern
0 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2016-06-23 20:33 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, Beniamino Galvani
On 6/23/16 8:39 AM, Paolo Abeni wrote:
> On Thu, 2016-06-23 at 08:29 -0600, David Ahern wrote:
>> On 6/23/16 8:20 AM, David Ahern wrote:
>>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>>> index 969913d..520b788 100644
>>>> --- a/net/ipv6/route.c
>>>> +++ b/net/ipv6/route.c
>>>> @@ -1782,7 +1782,7 @@ static struct rt6_info
>>>> *ip6_nh_lookup_table(struct net *net,
>>>> };
>>>> struct fib6_table *table;
>>>> struct rt6_info *rt;
>>>> - int flags = 0;
>>>> + int flags = RT6_LOOKUP_F_IFACE;
>>>>
>>>> table = fib6_get_table(net, cfg->fc_table);
>>>> if (!table)
>>>>
>>>
>>> Acked-by: David Ahern <dsa@cumulusnetworks.com>
>>
>> I take that back.
>>
>> I think RT6_LOOKUP_F_IFACE should only be set if cfg->fc_ifindex is set.
>
> AFAICS the latter condition should not be needed. The related
> information is passed all way down to rt6_score_route(), where it's
> really used:
>
> m = rt6_check_dev(rt, oif);
> if (!m && (strict & RT6_LOOKUP_F_IFACE))
> return RT6_NUD_FAIL_HARD;
>
> and 'm' can be 0 only if oif is set: RT6_LOOKUP_F_IFACE has no effect
> ifindex is set.
>
For the simplified lookup yes that is true. Lookups that go through
ip6_pol_route it is not and for my comment above I was thinking about
this latter case.
Anyways, your change is fine for the ip6_nh_lookup_table case.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups
2016-06-23 13:25 [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups Paolo Abeni
2016-06-23 14:20 ` David Ahern
@ 2016-06-27 14:37 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2016-06-27 14:37 UTC (permalink / raw)
To: pabeni; +Cc: netdev, dsa, bgalvani
From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 23 Jun 2016 15:25:09 +0200
> with the commit 8c14586fc320 ("net: ipv6: Use passed in table for
> nexthop lookups"), net hop lookup is first performed on route creation
> in the passed-in table.
> However device match is not enforced in table lookup, so the found
> route can be later discarded due to egress device mismatch and no
> global lookup will be performed.
> This cause the following to fail:
>
> ip link add dummy1 type dummy
> ip link add dummy2 type dummy
> ip link set dummy1 up
> ip link set dummy2 up
> ip route add 2001:db8:8086::/48 dev dummy1 metric 20
> ip route add 2001:db8:d34d::/64 via 2001:db8:8086::2 dev dummy1 metric 20
> ip route add 2001:db8:8086::/48 dev dummy2 metric 21
> ip route add 2001:db8:d34d::/64 via 2001:db8:8086::2 dev dummy2 metric 21
> RTNETLINK answers: No route to host
>
> This change fixes the issue enforcing device lookup in
> ip6_nh_lookup_table()
>
> v1->v2: updated commit message title
>
> Fixes: 8c14586fc320 ("net: ipv6: Use passed in table for nexthop lookups")
> Reported-and-tested-by: Beniamino Galvani <bgalvani@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Applied, thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-27 14:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-23 13:25 [PATCH net v2] ipv6: enforce egress device match in per table nexthop lookups Paolo Abeni
2016-06-23 14:20 ` David Ahern
2016-06-23 14:29 ` David Ahern
2016-06-23 14:39 ` Paolo Abeni
2016-06-23 20:33 ` David Ahern
2016-06-27 14:37 ` David Miller
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.