On 12/20/2017 01:59 PM, Jakub Kicinski wrote:
> On Wed, 20 Dec 2017 12:09:19 -0800, John Fastabend wrote:
>> RCU grace period is needed for lockless qdiscs added in the commit
>> c5ad119fb6c09 ("net: sched: pfifo_fast use skb_array").
>>
>> It is needed now that qdiscs may be lockless otherwise we risk
>> free'ing a qdisc that is still in use from datapath. Additionally,
>> push list cleanup into RCU callback. Otherwise we risk the datapath
>> adding skbs during removal.
>>
>> Fixes: c5ad119fb6c09 ("net: sched: pfifo_fast use skb_array")
>> Signed-off-by: John Fastabend <john.fastab...@gmail.com>
>
> Seems like this revert may be too heavy handed:
>
> # ./tools/testing/selftests/bpf/test_offload.py --log /tmp/log
> Test destruction of generic XDP...
> Test TC non-offloaded...
> Test TC non-offloaded isn't getting bound...
> Test TC offloads are off by default...
> Test TC offload by default...
> Test TC cBPF bytcode tries offload by default...
> Test TC cBPF unbound bytecode doesn't offload...
> Test TC offloads work...
> FAIL: TC filter did not load with TC offloads enabled
>
> And it's triggering:
>
> WARNING: CPU: 15 PID: 1853 at ../drivers/net/netdevsim/bpf.c:372
> nsim_bpf_uninit+0x2e/0x41 [netdevsim]
>
> Which is:
>
> 368 void nsim_bpf_uninit(struct netdevsim *ns)
> 369 {
> 370 WARN_ON(!list_empty(&ns->bpf_bound_progs));
> 371 WARN_ON(ns->xdp_prog);
>>> 372 WARN_ON(ns->bpf_offloaded);
> 373 }
>
> (Meaning the offload was not stopped by the stack before ndo_uninit.)
>
Dang. So offload code depends on destroy being called on a qdisc
to in turn destroy the filters and unbind any offloads.
I was hoping I could get away with tearing down live qdiscs without
too much work. Looks like not.
Note the fixes tag was bogus nothing is actually broken in current
code until a lockless qdisc with classes shows up.
.John
