netdev - Re: [PATCH net-next 1/9] sched: add helper for updating statistics on all actions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJ3xEMiZU69RpXxVNYCyt7ZThicHmzM=r8Oswq6QLW-y1CAhEQ@mail.gmail.com>
Date:   Thu, 1 Jun 2017 23:54:54 +0300
From:   Or Gerlitz <gerlitz.or@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Linux Netdev List <netdev@...r.kernel.org>,
        oss-drivers@...ronome.com, Jiri Pirko <jiri@...lanox.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Or Gerlitz <ogerlitz@...lanox.com>
Subject: Re: [PATCH net-next 1/9] sched: add helper for updating statistics on
 all actions

On Wed, May 31, 2017 at 6:06 PM, Jakub Kicinski
<jakub.kicinski@...ronome.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.kicinski@...ronome.com>
> ---
> CC: Jiri Pirko <jiri@...lanox.com>
> CC: Jamal Hadi Salim <jhs@...atatu.com>
> CC: Cong Wang <xiyou.wangcong@...il.com>
> CC: Saeed Mahameed <saeedm@...lanox.com>
> CC: Or Gerlitz <ogerlitz@...lanox.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 <ogerlitz@...lanox.com>