Re: [PATCH v2 bpf-net] bpf: Change bpf_fib_lookup to return lookup status

[David Ahern <dsahern@gmail.com>]

On 6/26/18 3:50 AM, Daniel Borkmann wrote:

> [...]
> You change all the semantics of return code here, but this breaks bpf_skb_fib_lookup().
> I cannot see how this would work in that case. The code does the following with the
> bpf_ipv{4,6}_fib_lookup() return code:
> 
> [...]
>         switch (params->family) {
> #if IS_ENABLED(CONFIG_INET)
>         case AF_INET:
>                 index = bpf_ipv4_fib_lookup(net, params, flags, false);
>                 break;
> #endif
> #if IS_ENABLED(CONFIG_IPV6)
>         case AF_INET6:
>                 index = bpf_ipv6_fib_lookup(net, params, flags, false);
>                 break;
> #endif
>         }
> 
>         if (index > 0) {
>                 struct net_device *dev;
> 
>                 dev = dev_get_by_index_rcu(net, index);
>                 if (!is_skb_forwardable(dev, skb))
>                         index = 0;
>         }

Yes, I forgot to update the skb path. That should be rc now and then the
dev lookup based on params->ifindex. Will fix.

> [...]
> 
> So the BPF_FIB_LKUP_* results become the dev ifindex here and the !is_skb_forwardable()
> case further suggests that the packet *can* be forwarded based on the new semantics
> whereas MTU check is bypassed on success.
> 
> It probably helps to craft a selftest for XDP *and* tc case in future, so we can be sure
> nothing breaks with new changes.

yes, will do.