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.
