On 5/28/19, 6:43 AM, "netdev-ow...@vger.kernel.org on behalf of Eric Dumazet"
<netdev-ow...@vger.kernel.org on behalf of eric.duma...@gmail.com> wrote:
On 5/27/19 8:49 PM, brakmo wrote:
> Create new macro BPF_PROG_CGROUP_INET_EGRESS_RUN_ARRAY() to be used by
> __cgroup_bpf_run_filter_skb for EGRESS BPF progs so BPF programs can
> request cwr for TCP packets.
>
...
> +#define BPF_PROG_CGROUP_INET_EGRESS_RUN_ARRAY(array, ctx, func)
\
> + ({ \
> + struct bpf_prog_array_item *_item; \
> + struct bpf_prog *_prog; \
> + struct bpf_prog_array *_array; \
> + u32 ret; \
> + u32 _ret = 1; \
> + u32 _cn = 0; \
> + preempt_disable(); \
> + rcu_read_lock(); \
> + _array = rcu_dereference(array); \
Why _array can not be NULL here ?
I replaced the call chain
BPF_CGROUP_RUN_PROG_INET_EGRESS()
__cgroup_bpf_run_filter_skb()
BPF_PROG_RUN_ARRAY()
__BPF_PROG_RUN_ARRAY( , , false)
With
BPF_CGROUP_RUN_PROG_INET_EGRESS()
__cgroup_bpf_run_filter_skb()
BPF_PROG_CGROUP_INET_EGRESS_RUN_ARRAY()
BPF_PROG_CGROUP_INET_EGRESS_RUN_ARRAY() is a instantiation of
__BPF_PROG_RUN_ARRAY() with the argument "check_non_null" = false
Hence I removed the test "if (unlikely(check_non_null && !_array))"
since it always fails. I have assumed the existing code is correct.
> + _item = &_array->items[0]; \
> + while ((_prog = READ_ONCE(_item->prog))) { \
> + bpf_cgroup_storage_set(_item->cgroup_storage); \
> + ret = func(_prog, ctx); \
> + _ret &= (ret & 1); \
> + _cn |= (ret & 2); \
> + _item++; \
> + } \
> + rcu_read_unlock(); \
> + preempt_enable_no_resched();
Why are you using preempt_enable_no_resched() here ?
Because that is what __BPF_PROG_RUN_ARRAY() calls and the macro
BPF_PROG_CGROUP_INET_EGRESS_RUN_ARRAY() is an instantiation of it
(with minor changes in the return value).
\
> + if (_ret) \
> + _ret = (_cn ? NET_XMIT_CN : NET_XMIT_SUCCESS); \
> + else \
> + _ret = (_cn ? NET_XMIT_DROP : -EPERM); \
> + _ret; \
> + })
> +
> #define BPF_PROG_RUN_ARRAY(array, ctx, func) \
> __BPF_PROG_RUN_ARRAY(array, ctx, func, false)
>
>
