On 05/03/2018 05:53 AM, David Ahern wrote:
[...]
> +
> +BPF_CALL_4(bpf_xdp_fib_lookup, struct xdp_buff *, ctx,
> + struct bpf_fib_lookup *, params, int, plen, u32, flags)
> +{
> + if (plen < sizeof(*params))
> + return -EINVAL;
> +
> + switch (params->family) {
> +#if IS_ENABLED(CONFIG_INET)
> + case AF_INET:
> + return bpf_ipv4_fib_lookup(dev_net(ctx->rxq->dev), params,
> + flags);
> +#endif
> +#if IS_ENABLED(CONFIG_IPV6)
> + case AF_INET6:
> + return bpf_ipv6_fib_lookup(dev_net(ctx->rxq->dev), params,
> + flags);
> +#endif
> + }
> + return -ENOTSUPP;
> +}
> +
> +static const struct bpf_func_proto bpf_xdp_fib_lookup_proto = {
> + .func = bpf_xdp_fib_lookup,
> + .gpl_only = true,
> + .pkt_access = true,
Hmm, this one and ...
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_PTR_TO_MEM,
> + .arg3_type = ARG_CONST_SIZE,
> + .arg4_type = ARG_ANYTHING,
> +};
> +
> +BPF_CALL_4(bpf_skb_fib_lookup, struct sk_buff *, skb,
> + struct bpf_fib_lookup *, params, int, plen, u32, flags)
> +{
> + if (plen < sizeof(*params))
> + return -EINVAL;
> +
> + switch (params->family) {
> +#if IS_ENABLED(CONFIG_INET)
> + case AF_INET:
> + return bpf_ipv4_fib_lookup(dev_net(skb->dev), params, flags);
> +#endif
> +#if IS_ENABLED(CONFIG_IPV6)
> + case AF_INET6:
> + return bpf_ipv6_fib_lookup(dev_net(skb->dev), params, flags);
> +#endif
> + }
> + return -ENOTSUPP;
> +}
> +
> +static const struct bpf_func_proto bpf_skb_fib_lookup_proto = {
> + .func = bpf_skb_fib_lookup,
> + .gpl_only = true,
> + .pkt_access = true,
... this should both not be marked as pkt_access = true. What this means is that
arg2, which is the struct bpf_fib_lookup, could come from the raw packet buffer.
Meaning instead of the struct sitting on stack it could in that case be passed
out of the packet directly. We have this enabled in helpers like csum_diff() or
map lookup/update where it would make sense to pass raw buffer there, but here
it seems unintentional and should be removed instead. See also description in
last paragraph in 36bbef52c7eb ("bpf: direct packet write and access for helpers
for clsact progs").
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_PTR_TO_MEM,
> + .arg3_type = ARG_CONST_SIZE,
> + .arg4_type = ARG_ANYTHING,
> +};
> +
> static const struct bpf_func_proto *
> bpf_base_func_proto(enum bpf_func_id func_id)
> {
> @@ -3933,6 +4192,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const
> struct bpf_prog *prog)
> case BPF_FUNC_skb_get_xfrm_state:
> return &bpf_skb_get_xfrm_state_proto;
> #endif
> + case BPF_FUNC_fib_lookup:
> + return &bpf_skb_fib_lookup_proto;
> default:
> return bpf_base_func_proto(func_id);
> }
> @@ -3958,6 +4219,8 @@ xdp_func_proto(enum bpf_func_id func_id, const struct
> bpf_prog *prog)
> return &bpf_xdp_redirect_map_proto;
> case BPF_FUNC_xdp_adjust_tail:
> return &bpf_xdp_adjust_tail_proto;
> + case BPF_FUNC_fib_lookup:
> + return &bpf_xdp_fib_lookup_proto;
> default:
> return bpf_base_func_proto(func_id);
> }
>
