From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Ahern <dsahern@gmail.com>
Subject: Re: [PATCH bpf-net] bpf: Change bpf_fib_lookup to return lookup
 status
Date: Tue, 19 Jun 2018 09:11:00 -0600
Message-ID: <015a0d5e-0fa6-3550-3a2a-89ef7efc3b0d@gmail.com>
References: <20180617151819.6824-1-dsahern@kernel.org>
 <20180618181123.eczjeb3axd6sao57@kafai-mbp.dhcp.thefacebook.com>
 <780331bd-947a-83fe-6e62-c0efc05cfc04@gmail.com>
 <20180618205537.2j645mfujdsqxf2b@kafai-mbp.dhcp.thefacebook.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: dsahern@kernel.org, netdev@vger.kernel.org, borkmann@iogearbox.net,
        ast@kernel.org, davem@davemloft.net
To: Martin KaFai Lau <kafai@fb.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pg0-f65.google.com ([74.125.83.65]:39556 "EHLO
        mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S966263AbeFSPLD (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 19 Jun 2018 11:11:03 -0400
Received: by mail-pg0-f65.google.com with SMTP id w12-v6so2648pgc.6
        for <netdev@vger.kernel.org>; Tue, 19 Jun 2018 08:11:03 -0700 (PDT)
In-Reply-To: <20180618205537.2j645mfujdsqxf2b@kafai-mbp.dhcp.thefacebook.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 6/18/18 2:55 PM, Martin KaFai Lau wrote:
>>
>> Arguably BPF_FIB_LKUP_RET_NO_NHDEV is not needed. See below.
>>

...

>>>> @@ -4252,16 +4277,19 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
>>>>  	if (check_mtu) {
>>>>  		mtu = ipv6_stub->ip6_mtu_from_fib6(f6i, dst, src);
>>>>  		if (params->tot_len > mtu)
>>>> -			return 0;
>>>> +			return BPF_FIB_LKUP_RET_FRAG_NEEDED;
>>>>  	}
>>>>  
>>>>  	if (f6i->fib6_nh.nh_lwtstate)
>>>> -		return 0;
>>>> +		return BPF_FIB_LKUP_RET_UNSUPP_LWT;
>>>>  
>>>>  	if (f6i->fib6_flags & RTF_GATEWAY)
>>>>  		*dst = f6i->fib6_nh.nh_gw;
>>>>  
>>>>  	dev = f6i->fib6_nh.nh_dev;
>>>> +	if (unlikely(!dev))
>>>> +		return BPF_FIB_LKUP_RET_NO_NHDEV;
>>> Is this a bug fix?
>>>
>>
>> Difference between IPv4 and IPv6. Making them consistent.
>>
>> It is a major BUG in the kernel to reach this point in either protocol
>> to have a unicast route not tied to a device. IPv4 has checks; v6 does
>> not. I figured this being new code, why not make bpf_ipv{4,6}_fib_lookup
>> as close to the same as possible.
> Make sense.  A comment in the commit log will be useful if there is a
> re-spin.
> 

Upon further review, I will remove BPF_FIB_LKUP_RET_NO_NHDEV. The dev
check is not needed in either ipv4 or ipv6. For IPv4 after fib_lookup
calls both __mkroute_input and ip_route_output_key_hash_rcu expect dev
to be set for unicast as it should be.