| Message ID | 20190801082900.27216-1-liuhangbin@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] ipv4/route: do not check saddr dev if iif is LOOPBACK_IFINDEX | expand |
On 8/1/19 2:29 AM, Hangbin Liu wrote: > Jianlin reported a bug that for IPv4, ip route get from src_addr would fail > if src_addr is not an address on local system. > > \# ip route get 1.1.1.1 from 2.2.2.2 > RTNETLINK answers: Invalid argument so this is a forwarding lookup in which case iif should be set. Based on the above 'route get' inet_rtm_getroute is doing a lookup as if it is locally generated traffic.
On Thu, Aug 01, 2019 at 01:51:25PM -0600, David Ahern wrote: > On 8/1/19 2:29 AM, Hangbin Liu wrote: > > Jianlin reported a bug that for IPv4, ip route get from src_addr would fail > > if src_addr is not an address on local system. > > > > \# ip route get 1.1.1.1 from 2.2.2.2 > > RTNETLINK answers: Invalid argument > > so this is a forwarding lookup in which case iif should be set. Based on with out setting iif in userspace, the kernel set iif to lo by default. > the above 'route get' inet_rtm_getroute is doing a lookup as if it is > locally generated traffic. yeah... but what about the IPv6 part. That cause a different behavior in userspace. Thanks Hangbin
On 8/1/19 10:13 PM, Hangbin Liu wrote: > On Thu, Aug 01, 2019 at 01:51:25PM -0600, David Ahern wrote: >> On 8/1/19 2:29 AM, Hangbin Liu wrote: >>> Jianlin reported a bug that for IPv4, ip route get from src_addr would fail >>> if src_addr is not an address on local system. >>> >>> \# ip route get 1.1.1.1 from 2.2.2.2 >>> RTNETLINK answers: Invalid argument >> >> so this is a forwarding lookup in which case iif should be set. Based on > > with out setting iif in userspace, the kernel set iif to lo by default. right, it presumes locally generated traffic. > >> the above 'route get' inet_rtm_getroute is doing a lookup as if it is >> locally generated traffic. > > yeah... but what about the IPv6 part. That cause a different behavior in > userspace. just one of many, many annoying differences between v4 and v6. We could try to catalog it.
David, On Thu, 1 Aug 2019 13:51:25 -0600 David Ahern <dsahern@gmail.com> wrote: > On 8/1/19 2:29 AM, Hangbin Liu wrote: > > Jianlin reported a bug that for IPv4, ip route get from src_addr would fail > > if src_addr is not an address on local system. > > > > \# ip route get 1.1.1.1 from 2.2.2.2 > > RTNETLINK answers: Invalid argument > > so this is a forwarding lookup in which case iif should be set. On actual forwarding, yes, it will be set. But if we are just doing a lookup for a route (iif is LOOPBACK_IFINDEX), I think this should still give us the matching route, which is what IPv6 already does and what this patch fixes for IPv4. Otherwise, we have no way to fetch that route, no matter if source routing is configured. So I think this patch is correct and to some extent necessary.
From: David Ahern <dsahern@gmail.com> Date: Thu, 1 Aug 2019 22:16:00 -0600 > On 8/1/19 10:13 PM, Hangbin Liu wrote: >> On Thu, Aug 01, 2019 at 01:51:25PM -0600, David Ahern wrote: >>> On 8/1/19 2:29 AM, Hangbin Liu wrote: >>>> Jianlin reported a bug that for IPv4, ip route get from src_addr would fail >>>> if src_addr is not an address on local system. >>>> >>>> \# ip route get 1.1.1.1 from 2.2.2.2 >>>> RTNETLINK answers: Invalid argument >>> >>> so this is a forwarding lookup in which case iif should be set. Based on >> >> with out setting iif in userspace, the kernel set iif to lo by default. > > right, it presumes locally generated traffic. >> >>> the above 'route get' inet_rtm_getroute is doing a lookup as if it is >>> locally generated traffic. >> >> yeah... but what about the IPv6 part. That cause a different behavior in >> userspace. > > just one of many, many annoying differences between v4 and v6. We could > try to catalog it. I think we just have to accept this difference because this change would change behavior for all route lookups, not just those done by ip route get.
On Sun, 11 Aug 2019 20:49:18 -0700 (PDT) David Miller <davem@davemloft.net> wrote: > From: David Ahern <dsahern@gmail.com> > Date: Thu, 1 Aug 2019 22:16:00 -0600 > > > On 8/1/19 10:13 PM, Hangbin Liu wrote: > >> On Thu, Aug 01, 2019 at 01:51:25PM -0600, David Ahern wrote: > >>> On 8/1/19 2:29 AM, Hangbin Liu wrote: > >>>> Jianlin reported a bug that for IPv4, ip route get from src_addr would fail > >>>> if src_addr is not an address on local system. > >>>> > >>>> \# ip route get 1.1.1.1 from 2.2.2.2 > >>>> RTNETLINK answers: Invalid argument > >>> > >>> so this is a forwarding lookup in which case iif should be set. Based on > >> > >> with out setting iif in userspace, the kernel set iif to lo by default. > > > > right, it presumes locally generated traffic. > >> > >>> the above 'route get' inet_rtm_getroute is doing a lookup as if it is > >>> locally generated traffic. > >> > >> yeah... but what about the IPv6 part. That cause a different behavior in > >> userspace. > > > > just one of many, many annoying differences between v4 and v6. We could > > try to catalog it. > > I think we just have to accept this difference because this change would > change behavior for all route lookups, not just those done by ip route get. How so, actually? I don't see how that would happen. On the forwarding path, 'iif' is set (not to loopback interface), so that's not affected. Is there any other route lookup possibility I'm missing?
On 8/12/19 4:58 PM, Stefano Brivio wrote: > How so, actually? I don't see how that would happen. On the forwarding > path, 'iif' is set (not to loopback interface), so that's not affected. > > Is there any other route lookup possibility I'm missing? Use case is saddr is set and FLOWI_FLAG_ANYSRC is not set and that seems pretty common to me. From a quick look, icmp_route_lookup, ipv4_update_pmtu, ipv4_redirect, inet_csk_route_req, ... Enable trace_fib_table_lookup and look at the flags for various use cases.
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index 517300d587a7..1be78e8f9e3c 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -2512,7 +2512,8 @@ struct rtable *ip_route_output_key_hash_rcu(struct net *net, struct flowi4 *fl4, goto make_route; } - if (!(fl4->flowi4_flags & FLOWI_FLAG_ANYSRC)) { + if (!(fl4->flowi4_flags & FLOWI_FLAG_ANYSRC) && + fl4->flowi4_iif != LOOPBACK_IFINDEX) { /* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */ if (!__ip_dev_find(net, fl4->saddr, false)) goto out;
Jianlin reported a bug that for IPv4, ip route get from src_addr would fail if src_addr is not an address on local system. \# ip route get 1.1.1.1 from 2.2.2.2 RTNETLINK answers: Invalid argument While IPv6 would get the default route \# ip -6 route get 1111::1 from 2222::2 1111::1 from 2222::2 via fe80:52:0:4982::1fe dev eth0 proto ra src \ 2620::2cf:e2ff:fe40:37 metric 1024 hoplimit 64 pref medium Fix it by disabling the __ip_dev_find() check if iif is LOOPBACK_IFINDEX. Reported-by: Jianlin Shi <jishi@redhat.com> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- net/ipv4/route.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)