| 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
| ||
|
Message-ID: <20181119135705.GG2223@nanopsycho.orion>
Date: Mon, 19 Nov 2018 14:57:05 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Pablo Neira Ayuso <pablo@...filter.org>
Cc: netdev@...r.kernel.org, davem@...emloft.net,
thomas.lendacky@....com, f.fainelli@...il.com,
ariel.elior@...ium.com, michael.chan@...adcom.com,
santosh@...lsio.com, madalin.bucur@....com,
yisen.zhuang@...wei.com, salil.mehta@...wei.com,
jeffrey.t.kirsher@...el.com, tariqt@...lanox.com,
saeedm@...lanox.com, jiri@...lanox.com, idosch@...lanox.com,
jakub.kicinski@...ronome.com, peppe.cavallaro@...com,
grygorii.strashko@...com, andrew@...n.ch,
vivien.didelot@...oirfairelinux.com, alexandre.torgue@...com,
joabreu@...opsys.com, linux-net-drivers@...arflare.com,
ganeshgr@...lsio.com, ogerlitz@...lanox.com
Subject: Re: [PATCH net-next,v2 05/12] cls_flower: add statistics retrieval
infrastructure and use it
Mon, Nov 19, 2018 at 01:15:12AM CET, pablo@...filter.org wrote:
>This patch provides a tc_cls_flower_stats structure that acts as
>container for tc_cls_flower_offload, then we can use to restore the
>statistics on the existing TC actions. Hence, tcf_exts_stats_update() is
>not used from drivers.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@...filter.org>
>---
>v2: no changes.
>
> drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 4 ++--
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 6 +++---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
> drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 2 +-
> drivers/net/ethernet/netronome/nfp/flower/offload.c | 6 +++---
> include/net/pkt_cls.h | 15 +++++++++++++++
> net/sched/cls_flower.c | 4 ++++
> 7 files changed, 29 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
>index b82143d6cdde..3d71b2530d67 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
>@@ -1366,8 +1366,8 @@ static int bnxt_tc_get_flow_stats(struct bnxt *bp,
> lastused = flow->lastused;
> spin_unlock(&flow->stats_lock);
>
>- tcf_exts_stats_update(tc_flow_cmd->exts, stats.bytes, stats.packets,
>- lastused);
>+ tc_cls_flower_stats_update(tc_flow_cmd, stats.bytes, stats.packets,
>+ lastused);
> return 0;
> }
>
>diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
>index 39c5af5dad3d..2c7d1aebe214 100644
>--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
>+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
>@@ -807,9 +807,9 @@ int cxgb4_tc_flower_stats(struct net_device *dev,
> if (ofld_stats->packet_count != packets) {
> if (ofld_stats->prev_packet_count != packets)
> ofld_stats->last_used = jiffies;
>- tcf_exts_stats_update(cls->exts, bytes - ofld_stats->byte_count,
>- packets - ofld_stats->packet_count,
>- ofld_stats->last_used);
>+ tc_cls_flower_stats_update(cls, bytes - ofld_stats->byte_count,
>+ packets - ofld_stats->packet_count,
>+ ofld_stats->last_used);
>
> ofld_stats->packet_count = packets;
> ofld_stats->byte_count = bytes;
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>index 2645e5d1e790..c5f0b826fa91 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>@@ -3224,7 +3224,7 @@ int mlx5e_stats_flower(struct mlx5e_priv *priv,
>
> mlx5_fc_query_cached(counter, &bytes, &packets, &lastuse);
>
>- tcf_exts_stats_update(f->exts, bytes, packets, lastuse);
>+ tc_cls_flower_stats_update(f, bytes, packets, lastuse);
>
> return 0;
> }
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
>index 193a6f9acf79..3398984ffb2a 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
>@@ -460,7 +460,7 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
> if (err)
> goto err_rule_get_stats;
>
>- tcf_exts_stats_update(f->exts, bytes, packets, lastuse);
>+ tc_cls_flower_stats_update(f, bytes, packets, lastuse);
>
> mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
> return 0;
>diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
>index 708331234908..bec74d84756c 100644
>--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
>+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
>@@ -532,9 +532,9 @@ nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
> ctx_id = be32_to_cpu(nfp_flow->meta.host_ctx_id);
>
> spin_lock_bh(&priv->stats_lock);
>- tcf_exts_stats_update(flow->exts, priv->stats[ctx_id].bytes,
>- priv->stats[ctx_id].pkts,
>- priv->stats[ctx_id].used);
>+ tc_cls_flower_stats_update(flow, priv->stats[ctx_id].bytes,
>+ priv->stats[ctx_id].pkts,
>+ priv->stats[ctx_id].used);
>
> priv->stats[ctx_id].pkts = 0;
> priv->stats[ctx_id].bytes = 0;
>diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
>index 7d7aefa5fcd2..7f9a8d5ca945 100644
>--- a/include/net/pkt_cls.h
>+++ b/include/net/pkt_cls.h
>@@ -758,6 +758,12 @@ enum tc_fl_command {
> TC_CLSFLOWER_TMPLT_DESTROY,
> };
>
>+struct tc_cls_flower_stats {
>+ u64 pkts;
>+ u64 bytes;
>+ u64 lastused;
>+};
>+
> struct tc_cls_flower_offload {
> struct tc_cls_common_offload common;
> enum tc_fl_command command;
>@@ -765,6 +771,7 @@ struct tc_cls_flower_offload {
> struct flow_rule rule;
> struct tcf_exts *exts;
> u32 classid;
>+ struct tc_cls_flower_stats stats;
> };
>
> static inline struct flow_rule *
>@@ -773,6 +780,14 @@ tc_cls_flower_offload_flow_rule(struct tc_cls_flower_offload *tc_flow_cmd)
> return &tc_flow_cmd->rule;
> }
>
>+static inline void tc_cls_flower_stats_update(struct tc_cls_flower_offload *cls_flower,
>+ u64 pkts, u64 bytes, u64 lastused)
>+{
>+ cls_flower->stats.pkts = pkts;
>+ cls_flower->stats.bytes = bytes;
>+ cls_flower->stats.lastused = lastused;
Why do you need to store the values here in struct tc_cls_flower_offload?
Why don't you just call tcf_exts_stats_update()? Basically,
tc_cls_flower_stats_update() should be just wrapper around
tcf_exts_stats_update() so that drivers wouldn't use ->exts directly, as
you will remove them in follow-up patches, no?
>+}
>+
> enum tc_matchall_command {
> TC_CLSMATCHALL_REPLACE,
> TC_CLSMATCHALL_DESTROY,
>diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>index a301fb8e68e7..ee67f1ae8786 100644
>--- a/net/sched/cls_flower.c
>+++ b/net/sched/cls_flower.c
>@@ -430,6 +430,10 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
>
> tc_setup_cb_call(block, &f->exts, TC_SETUP_CLSFLOWER,
> &cls_flower, false);
>+
>+ tcf_exts_stats_update(&f->exts, cls_flower.stats.bytes,
>+ cls_flower.stats.pkts,
>+ cls_flower.stats.lastused);
> }
>
> static bool __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
>--
>2.11.0
>
Powered by blists - more mailing lists