Re: [PATCH net-next,v2 05/12] cls_flower: add statistics retrieval infrastructure and use it
From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-11-20 00:27:29
Mon, Nov 19, 2018 at 01:15:12AM CET, pablo@netfilter.org wrote:
quoted hunk ↗ jump to hunk
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@netfilter.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?
quoted hunk ↗ jump to hunk
+} + 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