From: Vlad Buslov <vladbu@nvidia.com>
To: Simon Horman <simon.horman@corigine.com>
Cc: David Miller <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Jamal Hadi Salim <jhs@mojatatu.com>,
Cong Wang <xiyou.wangcong@gmail.com>,
Jiri Pirko <jiri@mellanox.com>, <netdev@vger.kernel.org>,
<oss-drivers@corigine.com>,
Baowen Zheng <baowen.zheng@corigine.com>,
"Louis Peens" <louis.peens@corigine.com>
Subject: Re: [PATCH net-next 3/3] flow_offload: add process to update action stats from hardware
Date: Thu, 22 Jul 2021 17:55:49 +0300 [thread overview]
Message-ID: <ygnhczraqtxm.fsf@nvidia.com> (raw)
In-Reply-To: <20210722091938.12956-4-simon.horman@corigine.com>
On Thu 22 Jul 2021 at 12:19, Simon Horman <simon.horman@corigine.com> wrote:
> From: Baowen Zheng <baowen.zheng@corigine.com>
>
> When collecting stats for actions update them using both
> both hardware and software counters.
>
> Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
> Signed-off-by: Louis Peens <louis.peens@corigine.com>
> Signed-off-by: Simon Horman <simon.horman@corigine.com>
> ---
> include/net/act_api.h | 1 +
> include/net/flow_offload.h | 2 +-
> include/net/pkt_cls.h | 4 ++++
> net/sched/act_api.c | 49 ++++++++++++++++++++++++++++++++++----
> 4 files changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index 086b291e9530..fe8331b5efce 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -233,6 +233,7 @@ static inline void tcf_action_inc_overlimit_qstats(struct tc_action *a)
> void tcf_action_update_stats(struct tc_action *a, u64 bytes, u64 packets,
> u64 drops, bool hw);
> int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
> +int tcf_action_update_hw_stats(struct tc_action *action);
>
> int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
> struct tcf_chain **handle,
> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> index 26644596fd54..467688fff7ce 100644
> --- a/include/net/flow_offload.h
> +++ b/include/net/flow_offload.h
> @@ -560,7 +560,7 @@ enum flow_act_command {
> };
>
> struct flow_offload_action {
> - struct netlink_ext_ack *extack;
> + struct netlink_ext_ack *extack; /* NULL in FLOW_ACT_STATS process*/
> enum flow_act_command command;
> struct flow_stats stats;
> struct flow_action action;
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 03dae225d64f..569c9294b15b 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -282,6 +282,10 @@ tcf_exts_stats_update(const struct tcf_exts *exts,
> for (i = 0; i < exts->nr_actions; i++) {
> struct tc_action *a = exts->actions[i];
>
> + /* if stats from hw, just skip */
> + if (!tcf_action_update_hw_stats(a))
> + continue;
> +
Is it okay to call this inside preempt disable section?
> tcf_action_stats_update(a, bytes, packets, drops,
> lastuse, true);
> a->used_hw_stats = used_hw_stats;
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 23a4538916af..7d5535bc2c13 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1089,15 +1089,18 @@ int tcf_action_offload_cmd_pre(struct tc_action *actions[],
> EXPORT_SYMBOL(tcf_action_offload_cmd_pre);
>
> int tcf_action_offload_cmd_post(struct flow_offload_action *fl_act,
> - struct netlink_ext_ack *extack)
> + struct netlink_ext_ack *extack,
> + bool keep_fl_act)
> {
> if (IS_ERR(fl_act))
> return PTR_ERR(fl_act);
>
> flow_indr_dev_setup_offload(NULL, NULL, TC_SETUP_ACT, fl_act, NULL, NULL);
>
> - tc_cleanup_flow_action(&fl_act->action);
> - kfree(fl_act);
> + if (!keep_fl_act) {
> + tc_cleanup_flow_action(&fl_act->action);
> + kfree(fl_act);
> + }
> return 0;
> }
>
> @@ -1115,10 +1118,45 @@ int tcf_action_offload_cmd(struct tc_action *actions[],
> if (err)
> return err;
>
> - return tcf_action_offload_cmd_post(fl_act, extack);
> + return tcf_action_offload_cmd_post(fl_act, extack, false);
> }
> EXPORT_SYMBOL(tcf_action_offload_cmd);
>
> +int tcf_action_update_hw_stats(struct tc_action *action)
> +{
> + struct tc_action *actions[TCA_ACT_MAX_PRIO] = {
> + [0] = action,
> + };
> + struct flow_offload_action *fl_act;
> + int err = 0;
> +
Having some way to distinguish offloaded actions would also be useful
here to skip this function. I wonder how this affects dump rate when
executed for every single action, even when none of them were offloaded
through action API.
> + err = tcf_action_offload_cmd_pre(actions,
> + FLOW_ACT_STATS,
> + NULL,
> + &fl_act);
> + if (err)
> + goto err_out;
> +
> + err = tcf_action_offload_cmd_post(fl_act, NULL, true);
> +
> + if (fl_act->stats.lastused) {
> + tcf_action_stats_update(action, fl_act->stats.bytes,
> + fl_act->stats.pkts,
> + fl_act->stats.drops,
> + fl_act->stats.lastused,
> + true);
> + err = 0;
> + } else {
> + err = -EOPNOTSUPP;
> + }
> + tc_cleanup_flow_action(&fl_act->action);
> + kfree(fl_act);
> +
> +err_out:
> + return err;
> +}
> +EXPORT_SYMBOL(tcf_action_update_hw_stats);
> +
> /* offload the tc command after deleted */
> int tcf_action_offload_del_post(struct flow_offload_action *fl_act,
> struct tc_action *actions[],
> @@ -1255,6 +1293,9 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *p,
> if (p == NULL)
> goto errout;
>
> + /* update hw stats for this action */
> + tcf_action_update_hw_stats(p);
> +
> /* compat_mode being true specifies a call that is supposed
> * to add additional backward compatibility statistic TLVs.
> */
next prev parent reply other threads:[~2021-07-22 14:59 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-22 9:19 [PATCH net-next 0/3] flow_offload: hardware offload of TC actions Simon Horman
2021-07-22 9:19 ` [PATCH net-next 1/3] flow_offload: allow user to offload tc action to net device Simon Horman
2021-07-22 12:24 ` Roi Dayan
2021-07-22 13:19 ` Simon Horman
2021-07-22 13:29 ` Vlad Buslov
2021-07-22 13:33 ` Jamal Hadi Salim
2021-07-27 13:04 ` Simon Horman
2021-07-27 14:38 ` Vlad Buslov
2021-07-27 16:13 ` Jamal Hadi Salim
2021-07-27 16:47 ` Vlad Buslov
2021-07-28 7:46 ` Simon Horman
2021-07-28 8:05 ` Vlad Buslov
2021-07-28 13:51 ` Jamal Hadi Salim
2021-07-28 14:46 ` Simon Horman
2021-07-30 10:17 ` Jamal Hadi Salim
2021-07-30 11:40 ` Vlad Buslov
2021-08-03 9:57 ` Jamal Hadi Salim
2021-08-03 12:02 ` tc offload debug-ability Jamal Hadi Salim
2021-08-03 12:14 ` Vlad Buslov
2021-08-03 12:50 ` Jamal Hadi Salim
2021-08-03 13:34 ` Ido Schimmel
2021-07-30 13:20 ` [PATCH net-next 1/3] flow_offload: allow user to offload tc action to net device Simon Horman
2021-08-03 10:14 ` Jamal Hadi Salim
2021-08-03 11:36 ` Simon Horman
2021-08-03 11:45 ` Jamal Hadi Salim
2021-08-03 12:31 ` Simon Horman
2021-08-03 13:01 ` Jamal Hadi Salim
2021-08-03 14:46 ` Simon Horman
2021-07-22 13:57 ` kernel test robot
2021-07-22 15:31 ` kernel test robot
2021-08-03 10:50 ` Jamal Hadi Salim
2021-08-03 11:05 ` Jamal Hadi Salim
2021-08-03 11:31 ` Simon Horman
2021-07-22 9:19 ` [PATCH net-next 2/3] flow_offload: add process to delete offloaded actions from " Simon Horman
2021-07-22 14:25 ` Vlad Buslov
2021-07-22 14:50 ` kernel test robot
2021-07-22 17:07 ` kernel test robot
2021-08-03 10:59 ` Jamal Hadi Salim
2021-07-22 9:19 ` [PATCH net-next 3/3] flow_offload: add process to update action stats from hardware Simon Horman
2021-07-22 14:55 ` Vlad Buslov [this message]
2021-08-03 11:24 ` Jamal Hadi Salim
2021-08-03 11:35 ` Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ygnhczraqtxm.fsf@nvidia.com \
--to=vladbu@nvidia.com \
--cc=baowen.zheng@corigine.com \
--cc=davem@davemloft.net \
--cc=jhs@mojatatu.com \
--cc=jiri@mellanox.com \
--cc=kuba@kernel.org \
--cc=louis.peens@corigine.com \
--cc=netdev@vger.kernel.org \
--cc=oss-drivers@corigine.com \
--cc=simon.horman@corigine.com \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).