Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

From: David Ahern <dsahern@gmail.com>
To: Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>,
	Hangbin Liu <liuhangbin@gmail.com>,
	network dev <netdev@vger.kernel.org>
Subject: Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
Date: Fri, 28 Jul 2017 11:39:52 -0600	[thread overview]
Message-ID: <ea0c0319-1b74-da8f-e307-2bf02e674119@gmail.com> (raw)
In-Reply-To: <CAJieiUgxQ+okYAffbb6dqA-=U9qn1QA7A3-ZJ8=HC=o1bkF3ow@mail.gmail.com>

On 7/28/17 11:13 AM, Roopa Prabhu wrote:
> for fibmatch, my original intent was to return with an error code.
> This is similar
> to the ipv4 behavior. One option is to keep the check in there and put
> the 'fibmatch'
> condition around it. But, i do want to make sure that for the fibmatch case,
> it does not return an error directly on an existing prohibit route
> entry in the fib.
> This is probably doable by checking for appropriate
> net->ipv6.ip6_prohibit_entry entries.
> 

IPv4 does not have the notion of null_entry or prohibit route entries
which makes IPv4 and IPv6 inconsistent - something we really need to be
avoiding from a user experience.

We have the following cases:

# ip -4 rule  add to 172.16.60.0/24 prohibit
# ip -4 route add prohibit 172.16.50.0/24
# ip -6 rule  add to 6000::/120 prohibit
# ip -6 route add prohibit 5000::/120

Behavior before Roopa's patch set:
  Rule match:
    # ip ro get 172.16.60.1
    RTNETLINK answers: Permission denied

    # ip -6 ro get 6000::1
    prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
4294967295  error -13 pref medium

  Route match:
    # ip ro get 172.16.50.1
    RTNETLINK answers: Permission denied

    # ip -6 ro get 5000::1
    prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric
1024  error -13 pref medium

Behavior after Roopa's patch set:
  Rule match:
    # ip ro get 172.16.60.1
    RTNETLINK answers: Permission denied

    # ip -6 ro get 6000::1
    RTNETLINK answers: Permission denied

  Route match:
    # ip ro get 172.16.50.1
    RTNETLINK answers: Permission denied

    # ip -6 ro get 5000::1
    RTNETLINK answers: Permission denied

So Roopa's fibmatch patches brings consistency between IPv4 and IPv6 at
the cost of breaking backwards compatibility for IPv6 when the prohibit
or blackhole routes are hit.

If that is not acceptable, then let's wrap the change in 'if (fibmatch)'
so that when fibmatch is requested we have consistency between IPv4 and
IPv6 when it is set.