diff mbox series

[v4,08/10] flow_offload: add reoffload process to update hw_count

Message ID 20211118130805.23897-9-simon.horman@corigine.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series allow user to offload tc action to net device | expand

Checks

Context Check Description
bpf/vmtest-bpf-next pending VM_Test
bpf/vmtest-bpf-next-PR pending PR summary
bpf/vmtest-bpf pending VM_Test
bpf/vmtest-bpf-PR pending PR summary
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 73 this patch: 73
netdev/cc_maintainers warning 3 maintainers not CCed: elic@nvidia.com davem@davemloft.net kuba@kernel.org
netdev/build_clang fail Errors and warnings before: 22 this patch: 22
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 77 this patch: 84
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 368 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Simon Horman Nov. 18, 2021, 1:08 p.m. UTC 
From: Baowen Zheng <baowen.zheng@corigine.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@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   |  24 +++++
 net/core/flow_offload.c |   4 +
 net/sched/act_api.c     | 213 ++++++++++++++++++++++++++++++++++++----
 3 files changed, 222 insertions(+), 19 deletions(-)

Comments

Vlad Buslov Nov. 19, 2021, 8:09 p.m. UTC | #1 
On Thu 18 Nov 2021 at 15:08, Simon Horman <simon.horman@corigine.com> wrote:
> From: Baowen Zheng <baowen.zheng@corigine.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@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   |  24 +++++
>  net/core/flow_offload.c |   4 +
>  net/sched/act_api.c     | 213 ++++++++++++++++++++++++++++++++++++----
>  3 files changed, 222 insertions(+), 19 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index 7900598d2dd3..e5e6e58df618 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_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/net/core/flow_offload.c b/net/core/flow_offload.c
> index 6676431733ef..92000164ac37 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);
> @@ -470,6 +473,7 @@ void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
>  	__flow_block_indr_cleanup(release, cb_priv, &cleanup_list);
>  	mutex_unlock(&flow_indr_block_lock);
>  
> +	tcf_action_reoffload_cb(cb, cb_priv, false);
>  	flow_block_indr_notify(&cleanup_list);
>  	kfree(indr_dev);
>  }
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index f5834d47a392..ada51b2df851 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -225,15 +225,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)
> @@ -245,9 +241,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] = {
> @@ -275,9 +303,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;
> @@ -290,6 +319,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 = {};
> @@ -302,7 +337,7 @@ int tcf_action_update_hw_stats(struct tc_action *action)
>  	if (err)
>  		return err;
>  
> -	err = tcf_action_offload_cmd(&fl_act, NULL, NULL);
> +	err = tcf_action_offload_cmd(&fl_act, NULL, NULL, NULL);
>  	if (!err) {
>  		preempt_disable();
>  		tcf_action_stats_update(action, fl_act.stats.bytes,
> @@ -321,7 +356,9 @@ int tcf_action_update_hw_stats(struct tc_action *action)
>  }
>  EXPORT_SYMBOL(tcf_action_update_hw_stats);
>  
> -static 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;
> @@ -337,16 +374,25 @@ static 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;
>  }
>  
> +static int tcf_action_offload_del(struct tc_action *action)
> +{
> +	return tcf_action_offload_del_ex(action, NULL, NULL);
> +}
> +
>  static void tcf_action_cleanup(struct tc_action *p)
>  {
>  	tcf_action_offload_del(p);
> @@ -841,6 +887,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)
> @@ -859,18 +958,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:

Rename to 'err_id' to harmonize label naming.

> +	unregister_pernet_subsys(ops);
> +	return ret;
>  }
>  EXPORT_SYMBOL(tcf_register_action);
>  
> @@ -889,12 +1000,76 @@ 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);
>  
> +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);

Deleting skip_sw action that is no longer offloaded to any hardware
device is reasonable, but note that in this case no netlink notification
is ever generated. Don't know if it is a problem, just highlighting the
fact since the whole behavior of implicitly deleting such actions is
completely omitted from the change log.

> +					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;
> +}
> +
>  /* lookup by name */
>  static struct tc_action_ops *tc_lookup_action_n(char *kind)
>  {
Baowen Zheng Nov. 22, 2021, 10:13 a.m. UTC | #2 
On November 20, 2021 4:09 AM, Vlad Buslov wrote:
>On Thu 18 Nov 2021 at 15:08, Simon Horman <simon.horman@corigine.com>
>wrote:
>> From: Baowen Zheng <baowen.zheng@corigine.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@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   |  24 +++++
>>  net/core/flow_offload.c |   4 +
>>  net/sched/act_api.c     | 213
>++++++++++++++++++++++++++++++++++++----
>>  3 files changed, 222 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/net/act_api.h b/include/net/act_api.h index
>> 7900598d2dd3..e5e6e58df618 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_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/net/core/flow_offload.c b/net/core/flow_offload.c
>> index 6676431733ef..92000164ac37 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);
>> @@ -470,6 +473,7 @@ void
>flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
>>  	__flow_block_indr_cleanup(release, cb_priv, &cleanup_list);
>>  	mutex_unlock(&flow_indr_block_lock);
>>
>> +	tcf_action_reoffload_cb(cb, cb_priv, false);
>>  	flow_block_indr_notify(&cleanup_list);
>>  	kfree(indr_dev);
>>  }
>> diff --git a/net/sched/act_api.c b/net/sched/act_api.c index
>> f5834d47a392..ada51b2df851 100644
>> --- a/net/sched/act_api.c
>> +++ b/net/sched/act_api.c
>> @@ -225,15 +225,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)
>> @@ -245,9 +241,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] = { @@ -275,9 +303,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;
>> @@ -290,6 +319,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 = {}; @@ -302,7 +337,7 @@ int
>> tcf_action_update_hw_stats(struct tc_action *action)
>>  	if (err)
>>  		return err;
>>
>> -	err = tcf_action_offload_cmd(&fl_act, NULL, NULL);
>> +	err = tcf_action_offload_cmd(&fl_act, NULL, NULL, NULL);
>>  	if (!err) {
>>  		preempt_disable();
>>  		tcf_action_stats_update(action, fl_act.stats.bytes, @@ -321,7
>> +356,9 @@ int tcf_action_update_hw_stats(struct tc_action *action)  }
>> EXPORT_SYMBOL(tcf_action_update_hw_stats);
>>
>> -static 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;
>> @@ -337,16 +374,25 @@ static 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;
>>  }
>>
>> +static int tcf_action_offload_del(struct tc_action *action) {
>> +	return tcf_action_offload_del_ex(action, NULL, NULL); }
>> +
>>  static void tcf_action_cleanup(struct tc_action *p)  {
>>  	tcf_action_offload_del(p);
>> @@ -841,6 +887,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)
>> @@ -859,18 +958,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:
>
>Rename to 'err_id' to harmonize label naming.
Ok, will make the change.
>
>> +	unregister_pernet_subsys(ops);
>> +	return ret;
>>  }
>>  EXPORT_SYMBOL(tcf_register_action);
>>
>> @@ -889,12 +1000,76 @@ 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);
>>
>> +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);
>
>Deleting skip_sw action that is no longer offloaded to any hardware device is
>reasonable, but note that in this case no netlink notification is ever generated.
>Don't know if it is a problem, just highlighting the fact since the whole
>behavior of implicitly deleting such actions is completely omitted from the
>change log.
Thanks for bring this to us. We will think about to add the notification implement and also we will
change commit message to mention about this.
>
>> +					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;
>> +}
>> +
>>  /* lookup by name */
>>  static struct tc_action_ops *tc_lookup_action_n(char *kind)  {
diff mbox series

Patch

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 7900598d2dd3..e5e6e58df618 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_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/net/core/flow_offload.c b/net/core/flow_offload.c
index 6676431733ef..92000164ac37 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);
@@ -470,6 +473,7 @@  void flow_indr_dev_unregister(flow_indr_block_bind_cb_t *cb, void *cb_priv,
 	__flow_block_indr_cleanup(release, cb_priv, &cleanup_list);
 	mutex_unlock(&flow_indr_block_lock);
 
+	tcf_action_reoffload_cb(cb, cb_priv, false);
 	flow_block_indr_notify(&cleanup_list);
 	kfree(indr_dev);
 }
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f5834d47a392..ada51b2df851 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -225,15 +225,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)
@@ -245,9 +241,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] = {
@@ -275,9 +303,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;
@@ -290,6 +319,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 = {};
@@ -302,7 +337,7 @@  int tcf_action_update_hw_stats(struct tc_action *action)
 	if (err)
 		return err;
 
-	err = tcf_action_offload_cmd(&fl_act, NULL, NULL);
+	err = tcf_action_offload_cmd(&fl_act, NULL, NULL, NULL);
 	if (!err) {
 		preempt_disable();
 		tcf_action_stats_update(action, fl_act.stats.bytes,
@@ -321,7 +356,9 @@  int tcf_action_update_hw_stats(struct tc_action *action)
 }
 EXPORT_SYMBOL(tcf_action_update_hw_stats);
 
-static 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;
@@ -337,16 +374,25 @@  static 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;
 }
 
+static int tcf_action_offload_del(struct tc_action *action)
+{
+	return tcf_action_offload_del_ex(action, NULL, NULL);
+}
+
 static void tcf_action_cleanup(struct tc_action *p)
 {
 	tcf_action_offload_del(p);
@@ -841,6 +887,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)
@@ -859,18 +958,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);
 
@@ -889,12 +1000,76 @@  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);
 
+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;
+}
+
 /* lookup by name */
 static struct tc_action_ops *tc_lookup_action_n(char *kind)
 {