| 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
| ||
|
Date: Fri, 29 Oct 2021 20:31:22 +0300
From: Vlad Buslov <vladbu@...dia.com>
To: Simon Horman <simon.horman@...igine.com>
CC: <netdev@...r.kernel.org>, Jamal Hadi Salim <jhs@...atatu.com>,
Roi Dayan <roid@...dia.com>, Ido Schimmel <idosch@...dia.com>,
Cong Wang <xiyou.wangcong@...il.com>,
Jiri Pirko <jiri@...nulli.us>,
Baowen Zheng <notifications@...hub.com>,
Louis Peens <louis.peens@...igine.com>,
<oss-drivers@...igine.com>,
Baowen Zheng <baowen.zheng@...igine.com>
Subject: Re: [RFC/PATCH net-next v3 7/8] flow_offload: add reoffload process
to update hw_count
On Thu 28 Oct 2021 at 14:06, Simon Horman <simon.horman@...igine.com> wrote:
> From: Baowen Zheng <baowen.zheng@...igine.com>
>
> Add reoffload process to update hw_count when driver
> is inserted or removed.
>
> When reoffloading actions, we still offload the actions
> that are added independent of filters.
>
> Signed-off-by: Baowen Zheng <baowen.zheng@...igine.com>
> Signed-off-by: Louis Peens <louis.peens@...igine.com>
> Signed-off-by: Simon Horman <simon.horman@...igine.com>
> ---
> include/net/act_api.h | 24 +++++
> include/net/pkt_cls.h | 5 +
> net/core/flow_offload.c | 5 +
> net/sched/act_api.c | 213 ++++++++++++++++++++++++++++++++++++----
> 4 files changed, 228 insertions(+), 19 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index 80a9d1e7d805..03ff39e347c3 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/refcount.h>
> +#include <net/flow_offload.h>
> #include <net/sch_generic.h>
> #include <net/pkt_sched.h>
> #include <net/net_namespace.h>
> @@ -243,11 +244,26 @@ static inline void flow_action_hw_count_set(struct tc_action *act,
> act->in_hw_count = hw_count;
> }
>
> +static inline void flow_action_hw_count_inc(struct tc_action *act,
> + u32 hw_count)
> +{
> + act->in_hw_count += hw_count;
> +}
> +
> +static inline void flow_action_hw_count_dec(struct tc_action *act,
> + u32 hw_count)
> +{
> + act->in_hw_count = act->in_hw_count > hw_count ?
> + act->in_hw_count - hw_count : 0;
> +}
> +
> 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_offload_del(struct tc_action *action);
> int tcf_action_update_hw_stats(struct tc_action *action);
> +int tcf_action_reoffload_cb(flow_indr_block_bind_cb_t *cb,
> + void *cb_priv, bool add);
> int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
> struct tcf_chain **handle,
> struct netlink_ext_ack *newchain);
> @@ -259,6 +275,14 @@ DECLARE_STATIC_KEY_FALSE(tcf_frag_xmit_count);
> #endif
>
> int tcf_dev_queue_xmit(struct sk_buff *skb, int (*xmit)(struct sk_buff *skb));
> +
> +#else /* !CONFIG_NET_CLS_ACT */
> +
> +static inline int tcf_action_reoffload_cb(flow_indr_block_bind_cb_t *cb,
> + void *cb_priv, bool add) {
> + return 0;
> +}
> +
> #endif /* CONFIG_NET_CLS_ACT */
>
> static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 88788b821f76..82ac631c50bc 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -284,6 +284,11 @@ static inline bool tc_act_flags_valid(u32 flags)
> return flags ^ (TCA_ACT_FLAGS_SKIP_HW | TCA_ACT_FLAGS_SKIP_SW);
> }
>
> +static inline bool tc_act_bind(u32 flags)
> +{
> + return !!(flags & TCA_ACT_FLAGS_BIND);
> +}
> +
> static inline void
> tcf_exts_stats_update(const struct tcf_exts *exts,
> u64 bytes, u64 packets, u64 drops, u64 lastuse,
> diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
> index 6676431733ef..d591204af6e0 100644
> --- a/net/core/flow_offload.c
> +++ b/net/core/flow_offload.c
> @@ -1,6 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> #include <linux/kernel.h>
> #include <linux/slab.h>
> +#include <net/act_api.h>
> #include <net/flow_offload.h>
> #include <linux/rtnetlink.h>
> #include <linux/mutex.h>
> @@ -418,6 +419,8 @@ int flow_indr_dev_register(flow_indr_block_bind_cb_t *cb, void *cb_priv)
> existing_qdiscs_register(cb, cb_priv);
> mutex_unlock(&flow_indr_block_lock);
>
> + tcf_action_reoffload_cb(cb, cb_priv, true);
> +
> return 0;
> }
> EXPORT_SYMBOL(flow_indr_dev_register);
> @@ -472,6 +475,8 @@ void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
>
> flow_block_indr_notify(&cleanup_list);
> kfree(indr_dev);
> +
> + tcf_action_reoffload_cb(cb, cb_priv, false);
Don't know if it is a problem, but shouldn't tcf_action_reoffload_cb()
be called before flow_block_indr_notify(), which calls
flow_block_indr->cleanup() callbacks?
> }
> EXPORT_SYMBOL(flow_indr_dev_unregister);
>
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 3893ffd91192..dce25d8f147b 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -638,6 +638,59 @@ EXPORT_SYMBOL(tcf_idrinfo_destroy);
>
> static LIST_HEAD(act_base);
> static DEFINE_RWLOCK(act_mod_lock);
> +/* since act ops id is stored in pernet subsystem list,
> + * then there is no way to walk through only all the action
> + * subsystem, so we keep tc action pernet ops id for
> + * reoffload to walk through.
> + */
> +static LIST_HEAD(act_pernet_id_list);
> +static DEFINE_MUTEX(act_id_mutex);
> +struct tc_act_pernet_id {
> + struct list_head list;
> + unsigned int id;
> +};
> +
> +static int tcf_pernet_add_id_list(unsigned int id)
> +{
> + struct tc_act_pernet_id *id_ptr;
> + int ret = 0;
> +
> + mutex_lock(&act_id_mutex);
> + list_for_each_entry(id_ptr, &act_pernet_id_list, list) {
> + if (id_ptr->id == id) {
> + ret = -EEXIST;
> + goto err_out;
> + }
> + }
> +
> + id_ptr = kzalloc(sizeof(*id_ptr), GFP_KERNEL);
> + if (!id_ptr) {
> + ret = -ENOMEM;
> + goto err_out;
> + }
> + id_ptr->id = id;
> +
> + list_add_tail(&id_ptr->list, &act_pernet_id_list);
> +
> +err_out:
> + mutex_unlock(&act_id_mutex);
> + return ret;
> +}
> +
> +static void tcf_pernet_del_id_list(unsigned int id)
> +{
> + struct tc_act_pernet_id *id_ptr;
> +
> + mutex_lock(&act_id_mutex);
> + list_for_each_entry(id_ptr, &act_pernet_id_list, list) {
> + if (id_ptr->id == id) {
> + list_del(&id_ptr->list);
> + kfree(id_ptr);
> + break;
> + }
> + }
> + mutex_unlock(&act_id_mutex);
> +}
>
> int tcf_register_action(struct tc_action_ops *act,
> struct pernet_operations *ops)
> @@ -656,18 +709,30 @@ int tcf_register_action(struct tc_action_ops *act,
> if (ret)
> return ret;
>
> + if (ops->id) {
> + ret = tcf_pernet_add_id_list(*ops->id);
> + if (ret)
> + goto id_err;
> + }
> +
> write_lock(&act_mod_lock);
> list_for_each_entry(a, &act_base, head) {
> if (act->id == a->id || (strcmp(act->kind, a->kind) == 0)) {
> - write_unlock(&act_mod_lock);
> - unregister_pernet_subsys(ops);
> - return -EEXIST;
> + ret = -EEXIST;
> + goto err_out;
> }
> }
> list_add_tail(&act->head, &act_base);
> write_unlock(&act_mod_lock);
>
> return 0;
> +
> +err_out:
> + write_unlock(&act_mod_lock);
> + tcf_pernet_del_id_list(*ops->id);
> +id_err:
> + unregister_pernet_subsys(ops);
> + return ret;
> }
> EXPORT_SYMBOL(tcf_register_action);
>
> @@ -686,8 +751,11 @@ int tcf_unregister_action(struct tc_action_ops *act,
> }
> }
> write_unlock(&act_mod_lock);
> - if (!err)
> + if (!err) {
> unregister_pernet_subsys(ops);
> + if (ops->id)
> + tcf_pernet_del_id_list(*ops->id);
> + }
> return err;
> }
> EXPORT_SYMBOL(tcf_unregister_action);
> @@ -1175,15 +1243,11 @@ static int flow_action_init(struct flow_offload_action *fl_action,
> return 0;
> }
>
> -static int tcf_action_offload_cmd(struct flow_offload_action *fl_act,
> - u32 *hw_count,
> - struct netlink_ext_ack *extack)
> +static int tcf_action_offload_cmd_ex(struct flow_offload_action *fl_act,
> + u32 *hw_count)
> {
> int err;
>
> - if (IS_ERR(fl_act))
> - return PTR_ERR(fl_act);
> -
> err = flow_indr_dev_setup_offload(NULL, NULL, TC_SETUP_ACT,
> fl_act, NULL, NULL);
> if (err < 0)
> @@ -1195,9 +1259,41 @@ static int tcf_action_offload_cmd(struct flow_offload_action *fl_act,
> return 0;
> }
>
> +static int tcf_action_offload_cmd_cb_ex(struct flow_offload_action *fl_act,
> + u32 *hw_count,
> + flow_indr_block_bind_cb_t *cb,
> + void *cb_priv)
> +{
> + int err;
> +
> + err = cb(NULL, NULL, cb_priv, TC_SETUP_ACT, NULL, fl_act, NULL);
> + if (err < 0)
> + return err;
> +
> + if (hw_count)
> + *hw_count = 1;
> +
> + return 0;
> +}
> +
> +static int tcf_action_offload_cmd(struct flow_offload_action *fl_act,
> + u32 *hw_count,
> + flow_indr_block_bind_cb_t *cb,
> + void *cb_priv)
> +{
> + if (IS_ERR(fl_act))
> + return PTR_ERR(fl_act);
> +
> + return cb ? tcf_action_offload_cmd_cb_ex(fl_act, hw_count,
> + cb, cb_priv) :
> + tcf_action_offload_cmd_ex(fl_act, hw_count);
> +}
> +
> /* offload the tc command after inserted */
> -static int tcf_action_offload_add(struct tc_action *action,
> - struct netlink_ext_ack *extack)
> +static int tcf_action_offload_add_ex(struct tc_action *action,
> + struct netlink_ext_ack *extack,
> + flow_indr_block_bind_cb_t *cb,
> + void *cb_priv)
> {
> bool skip_sw = tc_act_skip_sw(action->tcfa_flags);
> struct tc_action *actions[TCA_ACT_MAX_PRIO] = {
> @@ -1225,9 +1321,10 @@ static int tcf_action_offload_add(struct tc_action *action,
> goto fl_err;
> }
>
> - err = tcf_action_offload_cmd(fl_action, &in_hw_count, extack);
> + err = tcf_action_offload_cmd(fl_action, &in_hw_count, cb, cb_priv);
> if (!err)
> - flow_action_hw_count_set(action, in_hw_count);
> + cb ? flow_action_hw_count_inc(action, in_hw_count) :
> + flow_action_hw_count_set(action, in_hw_count);
>
> if (skip_sw && !tc_act_in_hw(action))
> err = -EINVAL;
> @@ -1240,6 +1337,12 @@ static int tcf_action_offload_add(struct tc_action *action,
> return err;
> }
>
> +static int tcf_action_offload_add(struct tc_action *action,
> + struct netlink_ext_ack *extack)
> +{
> + return tcf_action_offload_add_ex(action, extack, NULL, NULL);
> +}
> +
> int tcf_action_update_hw_stats(struct tc_action *action)
> {
> struct flow_offload_action fl_act = {};
> @@ -1252,7 +1355,7 @@ int tcf_action_update_hw_stats(struct tc_action *action)
> if (err)
> goto err_out;
>
> - err = tcf_action_offload_cmd(&fl_act, NULL, NULL);
> + err = tcf_action_offload_cmd(&fl_act, NULL, NULL, NULL);
>
> if (!err && fl_act.stats.lastused) {
> preempt_disable();
> @@ -1274,7 +1377,9 @@ int tcf_action_update_hw_stats(struct tc_action *action)
> }
> EXPORT_SYMBOL(tcf_action_update_hw_stats);
>
> -int tcf_action_offload_del(struct tc_action *action)
> +static int tcf_action_offload_del_ex(struct tc_action *action,
> + flow_indr_block_bind_cb_t *cb,
> + void *cb_priv)
> {
> struct flow_offload_action fl_act;
> u32 in_hw_count = 0;
> @@ -1290,13 +1395,83 @@ int tcf_action_offload_del(struct tc_action *action)
> if (err)
> return err;
>
> - err = tcf_action_offload_cmd(&fl_act, &in_hw_count, NULL);
> - if (err)
> + err = tcf_action_offload_cmd(&fl_act, &in_hw_count, cb, cb_priv);
> + if (err < 0)
> return err;
>
> - if (action->in_hw_count != in_hw_count)
> + if (!cb && action->in_hw_count != in_hw_count)
> return -EINVAL;
>
> + /* do not need to update hw state when deleting action */
> + if (cb && in_hw_count)
> + flow_action_hw_count_dec(action, in_hw_count);
> +
> + return 0;
> +}
> +
> +int tcf_action_offload_del(struct tc_action *action)
> +{
> + return tcf_action_offload_del_ex(action, NULL, NULL);
> +}
> +
> +int tcf_action_reoffload_cb(flow_indr_block_bind_cb_t *cb,
> + void *cb_priv, bool add)
> +{
> + struct tc_act_pernet_id *id_ptr;
> + struct tcf_idrinfo *idrinfo;
> + struct tc_action_net *tn;
> + struct tc_action *p;
> + unsigned int act_id;
> + unsigned long tmp;
> + unsigned long id;
> + struct idr *idr;
> + struct net *net;
> + int ret;
> +
> + if (!cb)
> + return -EINVAL;
> +
> + down_read(&net_rwsem);
> + mutex_lock(&act_id_mutex);
> +
> + for_each_net(net) {
> + list_for_each_entry(id_ptr, &act_pernet_id_list, list) {
> + act_id = id_ptr->id;
> + tn = net_generic(net, act_id);
> + if (!tn)
> + continue;
> + idrinfo = tn->idrinfo;
> + if (!idrinfo)
> + continue;
> +
> + mutex_lock(&idrinfo->lock);
> + idr = &idrinfo->action_idr;
> + idr_for_each_entry_ul(idr, p, tmp, id) {
> + if (IS_ERR(p) || tc_act_bind(p->tcfa_flags))
> + continue;
> + if (add) {
> + tcf_action_offload_add_ex(p, NULL, cb,
> + cb_priv);
> + continue;
> + }
> +
> + /* cb unregister to update hw count */
> + ret = tcf_action_offload_del_ex(p, cb, cb_priv);
> + if (ret < 0)
> + continue;
> + if (tc_act_skip_sw(p->tcfa_flags) &&
> + !tc_act_in_hw(p)) {
> + ret = tcf_idr_release_unsafe(p);
> + if (ret == ACT_P_DELETED)
> + module_put(p->ops->owner);
> + }
> + }
> + mutex_unlock(&idrinfo->lock);
> + }
> + }
> + mutex_unlock(&act_id_mutex);
> + up_read(&net_rwsem);
> +
> return 0;
> }
Powered by blists - more mailing lists