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.
