All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Bernat <vincent@bernat.im>
To: David Ahern <dsa@cumulusnetworks.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	James Morris <jmorris@namei.org>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	Patrick McHardy <kaber@trash.net>,
	netdev@vger.kernel.org
Subject: Re: [PATCH] net: ipv6: fallback to full lookup if table lookup is unsuitable
Date: Fri, 16 Sep 2016 21:15:14 +0200	[thread overview]
Message-ID: <m3twdfk665.fsf@neo.luffy.cx> (raw)
In-Reply-To: <adff2e07-75e1-d44c-4799-0920ffcd2622@cumulusnetworks.com> (David Ahern's message of "Fri, 16 Sep 2016 12:36:04 -0600")

 ❦ 16 septembre 2016 20:36 CEST, David Ahern <dsa@cumulusnetworks.com> :

>> contained a non-connected route (like a default gateway) fails while it
>> was previously working:
>> 
>>     $ ip link add eth0 type dummy
>>     $ ip link set up dev eth0
>>     $ ip addr add 2001:db8::1/64 dev eth0
>>     $ ip route add ::/0 via 2001:db8::5 dev eth0 table 20
>>     $ ip route add 2001:db8:cafe::1/128 via 2001:db8::6 dev eth0 table 20
>>     RTNETLINK answers: No route to host
>>     $ ip -6 route show table 20
>>     default via 2001:db8::5 dev eth0  metric 1024  pref medium
>
> so your table 20 is not complete in that it lacks a connected route to
> resolve 2001:db8::6 as a nexthop, so you are relying on a fallback to
> other tables (main in this case).

Yes.

>> @@ -1991,33 +2015,15 @@ static struct rt6_info *ip6_route_info_create(struct fib6_config *cfg)
>>  			if (!(gwa_type & IPV6_ADDR_UNICAST))
>>  				goto out;
>>  
>> +			err = -EHOSTUNREACH;
>>  			if (cfg->fc_table)
>>  				grt = ip6_nh_lookup_table(net, cfg, gw_addr);
>
> -----8<-----
>
>> -			if (!(grt->rt6i_flags & RTF_GATEWAY))
>> -				err = 0;
>
> This is the check that is failing for your use
> case. ip6_nh_lookup_table is returning the default route and nexthops
> can not rely on a gateway. Given that a simpler and more direct change
> is (whitespace mangled on paste):
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index ad4a7ff301fc..48bae2ee2e18 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1991,9 +1991,19 @@ static struct rt6_info *ip6_route_info_create(struct fib6_config *cfg)
>                         if (!(gwa_type & IPV6_ADDR_UNICAST))
>                                 goto out;
>
> -                       if (cfg->fc_table)
> +                       if (cfg->fc_table) {
>                                 grt = ip6_nh_lookup_table(net, cfg, gw_addr);
>
> +                               /* a nexthop lookup can not go through a gw.
> +                                * if this happens on a table based lookup
> +                                * then fallback to a full lookup
> +                                */
> +                               if (grt && grt->rt6i_flags & RTF_GATEWAY) {
> +                                       ip6_rt_put(grt);
> +                                       grt = NULL;
> +                               }
> +                       }
> +
>                         if (!grt)
>                                 grt = rt6_lookup(net, gw_addr, NULL,
>                                                  cfg->fc_ifindex, 1);

OK. Should the dev check be dismissed or do we add "dev && dev !=
grt->dst.dev" just as a safety net (this would be a convulated setup,
but the correct direct route could be in an ip rule with higher priority
while the one in this table is incorrect)?
-- 
"... an experienced, industrious, ambitious, and often quite often
picturesque liar."
		-- Mark Twain

  reply	other threads:[~2016-09-16 19:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-16 12:55 [PATCH] net: ipv6: fallback to full lookup if table lookup is unsuitable Vincent Bernat
2016-09-16 18:36 ` David Ahern
2016-09-16 19:15   ` Vincent Bernat [this message]
2016-09-16 19:38     ` David Ahern
2016-09-16 20:33       ` [v2] " Vincent Bernat
2016-09-18 15:12         ` David Ahern
2016-09-18 15:46           ` [v3] " Vincent Bernat
2016-09-18 15:47             ` David Ahern
2016-09-20  7:29             ` David Miller
2016-09-19  4:58 ` [PATCH] " David Miller
2016-09-19 20:27   ` Vincent Bernat

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=m3twdfk665.fsf@neo.luffy.cx \
    --to=vincent@bernat.im \
    --cc=davem@davemloft.net \
    --cc=dsa@cumulusnetworks.com \
    --cc=jmorris@namei.org \
    --cc=kaber@trash.net \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=netdev@vger.kernel.org \
    --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.