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: Pedro Tammela <pctammela@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Vlad Buslov <vladbu@nvidia.com>
Link: https://lore.kernel.org/r/20231211181807.96028-3-pctammela@mojatatu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 4ae0580b63cae6d88165f67ee9adc54cdaa3f1e6..ea13e1e4a7c292bfdc6603650dd665de14fd4002 100644 (file)
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -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);

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 076d5cdde656ab7f3017f5fa86da3ce68471272e..3a7770eff52d232bc4406c79f61e77514c931b9f 100644 (file)
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -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;

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 437daebc1fc47ba1afe8c5add1af4db1d990cedc..dc1c19a25882b9d5c02b1ad778e75b5912baf5be 100644 (file)
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -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;