On Wed, May 31, 2017 at 6:06 PM, Jakub Kicinski
<jakub.kicin...@netronome.com> wrote:
> Forgetting to disable preemption around tcf_action_stats_update()
> seems to be a common mistake. Add a helper function for updating
> stats on all actions of a filter.
>
> Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
> ---
> CC: Jiri Pirko <j...@mellanox.com>
> CC: Jamal Hadi Salim <j...@mojatatu.com>
> CC: Cong Wang <xiyou.wangc...@gmail.com>
> CC: Saeed Mahameed <sae...@mellanox.com>
> CC: Or Gerlitz <ogerl...@mellanox.com>
>
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 10 +---------
> drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 10 +---------
> drivers/net/ethernet/netronome/nfp/nfp_net_offload.c | 11 ++---------
> include/net/pkt_cls.h | 19
> +++++++++++++++++++
> 4 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index d2f90ba2dbc4..7914a32a3036 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -1863,9 +1863,7 @@ int mlx5e_stats_flower(struct mlx5e_priv *priv,
> {
> struct mlx5e_tc_table *tc = &priv->fs.tc;
> struct mlx5e_tc_flow *flow;
> - struct tc_action *a;
> struct mlx5_fc *counter;
> - LIST_HEAD(actions);
> u64 bytes;
> u64 packets;
> u64 lastuse;
> @@ -1884,13 +1882,7 @@ int mlx5e_stats_flower(struct mlx5e_priv *priv,
>
> mlx5_fc_query_cached(counter, &bytes, &packets, &lastuse);
>
> - preempt_disable();
> -
> - tcf_exts_to_list(f->exts, &actions);
> - list_for_each_entry(a, &actions, list)
> - tcf_action_stats_update(a, bytes, packets, lastuse);
> -
> - preempt_enable();
> + tcf_exts_stats_update(f->exts, bytes, packets, lastuse);
>
> return 0;
> }
> index f7762295b7b8..537d0a0ad4c4 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -157,6 +157,25 @@ static inline void tcf_exts_to_list(const struct
> tcf_exts *exts,
> #endif
> }
>
> +static inline void
> +tcf_exts_stats_update(const struct tcf_exts *exts,
> + u64 bytes, u64 packets, u64 lastuse)
> +{
> +#ifdef CONFIG_NET_CLS_ACT
> + int i;
> +
> + preempt_disable();
> +
> + for (i = 0; i < exts->nr_actions; i++) {
> + struct tc_action *a = exts->actions[i];
> +
> + tcf_action_stats_update(a, bytes, packets, lastuse);
> + }
> +
> + preempt_enable();
> +#endif
> +}
looks ok, for the mlx5 and pkt_cls.h parts:
Acked-by: Or Gerlitz <ogerl...@mellanox.com>
