Hi Paolo,
On 07/13/2018 11:55 AM, Paolo Abeni wrote:
> Each lockless action currently does its own RCU locking in ->act().
> This is allows using plain RCU accessor, even if the context
> is really RCU BH.
>
> This change drops the per action RCU lock, replace the accessors
> with _bh variant, cleans up a bit the surronding code and documents
> the RCU status in the relevant header.
> No functional nor performance change is intended.
>
> The goal of this patch is clarifying that the RCU critical section
> used by the tc actions extends up to the classifier's caller.
>
> Signed-off-by: Paolo Abeni <pab...@redhat.com>
[...]
> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> index 06f743d8ed41..ac20266460c0 100644
> --- a/net/sched/act_bpf.c
> +++ b/net/sched/act_bpf.c
> @@ -45,8 +45,7 @@ static int tcf_bpf(struct sk_buff *skb, const struct
> tc_action *act,
> tcf_lastuse_update(&prog->tcf_tm);
> bstats_cpu_update(this_cpu_ptr(prog->common.cpu_bstats), skb);
>
> - rcu_read_lock();
> - filter = rcu_dereference(prog->filter);
> + filter = rcu_dereference_bh(prog->filter);
> if (at_ingress) {
> __skb_push(skb, skb->mac_len);
> bpf_compute_data_pointers(skb);
> @@ -56,7 +55,6 @@ static int tcf_bpf(struct sk_buff *skb, const struct
> tc_action *act,
> bpf_compute_data_pointers(skb);
> filter_res = BPF_PROG_RUN(filter, skb);
> }
> - rcu_read_unlock();
This conversion is not correct, BPF itself relies on RCU but not RCU-bh flavor.
You might probably see a splat if you do e.g. a map lookup with this change in
interpreter mode on tx side.
> /* A BPF program may overwrite the default action opcode.
> * Similarly as in cls_bpf, if filter_res == -1 we use the
Thanks,
Daniel
