Commit 1dd7f18f authored by Pedro Tammela's avatar Pedro Tammela Committed by Jakub Kicinski
Browse files

net/sched: act_api: skip idr replace on bound actions 



tcf_idr_insert_many will replace the allocated -EBUSY pointer in
tcf_idr_check_alloc with the real action pointer, exposing it
to all operations. This operation is only needed when the action pointer
is created (ACT_P_CREATED). For actions which are bound to (returned 0),
the pointer already resides in the idr making such operation a nop.

Even though it's a nop, it's still not a cheap operation as internally
the idr code walks the idr and then does a replace on the appropriate slot.
So if the action was bound, better skip the idr replace entirely.

Signed-off-by: default avatarPedro Tammela <pctammela@mojatatu.com>
Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: default avatarVlad Buslov <vladbu@nvidia.com>
Link: https://lore.kernel.org/r/20231211181807.96028-3-pctammela@mojatatu.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 4b55e867

include/net/act_api.h

+1 −1
Original line number Diff line number Diff line
@@ -191,7 +191,7 @@ int tcf_idr_create_from_flags(struct tc_action_net *tn, u32 index, 
			      struct nlattr *est, struct tc_action **a,

			      const struct tc_action_ops *ops, int bind,

			      u32 flags);

void tcf_idr_insert_many(struct tc_action *actions[]);

void tcf_idr_insert_many(struct tc_action *actions[], int init_res[]);

void tcf_idr_cleanup(struct tc_action_net *tn, u32 index);

int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,

			struct tc_action **a, int bind);

net/sched/act_api.c

+6 −5
Original line number Diff line number Diff line
@@ -1304,7 +1304,7 @@ static const struct nla_policy tcf_action_policy[TCA_ACT_MAX + 1] = { 
	[TCA_ACT_HW_STATS]	= NLA_POLICY_BITFIELD32(TCA_ACT_HW_STATS_ANY),

};



void tcf_idr_insert_many(struct tc_action *actions[])

void tcf_idr_insert_many(struct tc_action *actions[], int init_res[])

{

	struct tc_action *a;

	int i;
@@ -1312,11 +1312,12 @@ void tcf_idr_insert_many(struct tc_action *actions[]) 
	tcf_act_for_each_action(i, a, actions) {

		struct tcf_idrinfo *idrinfo;



		if (init_res[i] == 0) /* Bound */

			continue;



		idrinfo = a->idrinfo;

		mutex_lock(&idrinfo->lock);

		/* Replace ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc if

		 * it is just created, otherwise this is just a nop.

		 */

		/* Replace ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */

		idr_replace(&idrinfo->action_idr, a, a->tcfa_index);

		mutex_unlock(&idrinfo->lock);

	}
@@ -1516,7 +1517,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla, 
	/* We have to commit them all together, because if any error happened in

	 * between, we could not handle the failure gracefully.

	 */

	tcf_idr_insert_many(actions);

	tcf_idr_insert_many(actions, init_res);



	*attr_size = tcf_action_full_attrs_size(sz);

	err = i - 1;

net/sched/cls_api.c

+1 −1
Original line number Diff line number Diff line
@@ -3313,7 +3313,7 @@ int tcf_exts_validate_ex(struct net *net, struct tcf_proto *tp, struct nlattr ** 
			act->type = exts->type = TCA_OLD_COMPAT;

			exts->actions[0] = act;

			exts->nr_actions = 1;

			tcf_idr_insert_many(exts->actions);

			tcf_idr_insert_many(exts->actions, init_res);

		} else if (exts->action && tb[exts->action]) {

			int err;