netdev - [PATCH net-next RFC 01/20] net/sched: act_api: change act

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230124170510.316970-1-jhs@mojatatu.com>
Date:   Tue, 24 Jan 2023 12:04:51 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     netdev@...r.kernel.org
Cc:     kernel@...atatu.com, deb.chatterjee@...el.com,
        anjali.singhai@...el.com, namrata.limaye@...el.com,
        khalidm@...dia.com, tom@...anda.io, pratyush@...anda.io,
        jiri@...nulli.us, xiyou.wangcong@...il.com, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        vladbu@...dia.com, simon.horman@...igine.com
Subject: [PATCH net-next RFC 01/20] net/sched: act_api: change act_base into an IDR

Convert act_base from a list to an IDR.

With the introduction of P4TC action templates, we introduce the concept of
dynamically creating actions on the fly. Dynamic action IDs are not statically
defined (as was the case previously) and are therefore harder to manage within
existing linked list approach. We convert to IDR because it has built in ID
management which we would have to re-invent with linked lists.

Co-developed-by: Victor Nogueira <victor@...atatu.com>
Signed-off-by: Victor Nogueira <victor@...atatu.com>
Co-developed-by: Pedro Tammela <pctammela@...atatu.com>
Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
---
 include/uapi/linux/pkt_cls.h |  1 +
 net/sched/act_api.c          | 39 +++++++++++++++++++++---------------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 648a82f32..4d716841c 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -139,6 +139,7 @@ enum tca_id {
 	TCA_ID_MPLS,
 	TCA_ID_CT,
 	TCA_ID_GATE,
+	TCA_ID_DYN,
 	/* other actions go here */
 	__TCA_ID_MAX = 255
 };
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index cd09ef49d..811dddc3b 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -890,7 +890,7 @@ void tcf_idrinfo_destroy(const struct tc_action_ops *ops,
 }
 EXPORT_SYMBOL(tcf_idrinfo_destroy);
 
-static LIST_HEAD(act_base);
+static DEFINE_IDR(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
@@ -949,7 +949,6 @@ static void tcf_pernet_del_id_list(unsigned int id)
 int tcf_register_action(struct tc_action_ops *act,
 			struct pernet_operations *ops)
 {
-	struct tc_action_ops *a;
 	int ret;
 
 	if (!act->act || !act->dump || !act->init)
@@ -970,13 +969,24 @@ int tcf_register_action(struct tc_action_ops *act,
 	}
 
 	write_lock(&act_mod_lock);
-	list_for_each_entry(a, &act_base, head) {
-		if (act->id == a->id || (strcmp(act->kind, a->kind) == 0)) {
+	if (act->id) {
+		if (idr_find(&act_base, act->id)) {
 			ret = -EEXIST;
 			goto err_out;
 		}
+		ret = idr_alloc_u32(&act_base, act, &act->id, act->id,
+				    GFP_ATOMIC);
+		if (ret < 0)
+			goto err_out;
+	} else {
+		/* Only dynamic actions will require ID generation */
+		act->id = TCA_ID_DYN;
+
+		ret = idr_alloc_u32(&act_base, act, &act->id, TCA_ID_MAX,
+				    GFP_ATOMIC);
+		if (ret < 0)
+			goto err_out;
 	}
-	list_add_tail(&act->head, &act_base);
 	write_unlock(&act_mod_lock);
 
 	return 0;
@@ -994,17 +1004,12 @@ EXPORT_SYMBOL(tcf_register_action);
 int tcf_unregister_action(struct tc_action_ops *act,
 			  struct pernet_operations *ops)
 {
-	struct tc_action_ops *a;
-	int err = -ENOENT;
+	int err = 0;
 
 	write_lock(&act_mod_lock);
-	list_for_each_entry(a, &act_base, head) {
-		if (a == act) {
-			list_del(&act->head);
-			err = 0;
-			break;
-		}
-	}
+	if (!idr_remove(&act_base, act->id))
+		err = -EINVAL;
+
 	write_unlock(&act_mod_lock);
 	if (!err) {
 		unregister_pernet_subsys(ops);
@@ -1019,10 +1024,11 @@ EXPORT_SYMBOL(tcf_unregister_action);
 static struct tc_action_ops *tc_lookup_action_n(char *kind)
 {
 	struct tc_action_ops *a, *res = NULL;
+	unsigned long tmp, id;
 
 	if (kind) {
 		read_lock(&act_mod_lock);
-		list_for_each_entry(a, &act_base, head) {
+		idr_for_each_entry_ul(&act_base, a, tmp, id) {
 			if (strcmp(kind, a->kind) == 0) {
 				if (try_module_get(a->owner))
 					res = a;
@@ -1038,10 +1044,11 @@ static struct tc_action_ops *tc_lookup_action_n(char *kind)
 static struct tc_action_ops *tc_lookup_action(struct nlattr *kind)
 {
 	struct tc_action_ops *a, *res = NULL;
+	unsigned long tmp, id;
 
 	if (kind) {
 		read_lock(&act_mod_lock);
-		list_for_each_entry(a, &act_base, head) {
+		idr_for_each_entry_ul(&act_base, a, tmp, id) {
 			if (nla_strcmp(kind, a->kind) == 0) {
 				if (try_module_get(a->owner))
 					res = a;
-- 
2.34.1