Subject: Re: [PATCH] net: Revert "net_sched: no need to free qdisc in RCU callback"

From: Jakub Kicinski <[email-masked]>
Date: Wed, 20 Dec 2017 13:59:59 -0800

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 <[email-masked]>

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.)