On Fri, 10 Feb 2017 18:21:25 +0200, Or Gerlitz wrote:
> On Fri, Feb 10, 2017 at 3:34 AM, Jakub Kicinski wrote:
> > On Thu, 9 Feb 2017 17:38:43 +0200, Or Gerlitz wrote:
> >> Running with CONFIG_PREEMPT set, I get a
> >>
> >> BUG: using smp_processor_id() in preemptible [00000000] code: tc/3793
> >>
> >> asserion from the TC action (mirred) stats_update callback, when the do
> >>
> >> _bstats_cpu_update(this_cpu_ptr(a->cpu_bstats), bytes, packets)
> >>
> >> As done by commit 66860be "nfp: bpf: allow offloaded filters to update
> >> stats",
> >> disabling/enabling preemption around the TC upcall solves that.
> >>
> >> Fixes: aad7e08d39bd ('net/mlx5e: Hardware offloaded flower filter
> >> statistics support')
> >> Signed-off-by: Or Gerlitz <ogerl...@mellanox.com>
> >> ---
> >>
> >> I marked it as RFC, since I wasn't fully sure on the nature of the
> >> problem, nor if this is the direction we should take to the fix.
>
> > I think it's the right fix
>
> Do you under the problem? what's wrong with the call done in the TC
> action code w.r.t preemption?
>
> does it make sense to do this (say) 100K times/sec?
TC actions have pre-cpu stats, referencing them has to be done with
preemption disabled. Let's CC Jamal and Cong - maybe there are some
more clever things we could do here? The situation in a nutshell is
that the offload drivers read the stats from HW and want to write them
back to the TC action stats. The writeback happens in process context
when user requests stats dump (potentially for multiple actions but we
currently would just iterate over all actions in driver code).
