[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230721191332.1424997-2-pctammela@mojatatu.com>
Date: Fri, 21 Jul 2023 16:13:28 -0300
From: Pedro Tammela <pctammela@...atatu.com>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net,
edumazet@...gle.com,
kuba@...nel.org,
pabeni@...hat.com,
jhs@...atatu.com,
xiyou.wangcong@...il.com,
jiri@...nulli.us,
Pedro Tammela <pctammela@...atatu.com>
Subject: [PATCH net-next 1/5] net/sched: wrap open coded Qdics class filter counter
The 'filter_cnt' counter is used to control a Qdisc class lifetime.
Each filter referecing this class by its id will eventually
increment/decrement this counter in their respective
'add/update/delete' routines.
As these operations are always serialized under rtnl lock, we don't
need an atomic type like 'refcount_t'.
It also means that we lose the overflow/underflow checks already
present in refcount_t, which are valuable to hunt down bugs
where the unsigned counter wraps around as it aids automated tools
like syzkaller to scream in such situations.
Wrap the open coded increment/decrement into helper functions and
add overflow checks to the operations.
Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
---
include/net/sch_generic.h | 1 +
include/net/tc_class.h | 33 +++++++++++++++++++++++++++++++++
net/sched/sch_drr.c | 10 +++++-----
net/sched/sch_hfsc.c | 9 +++++----
net/sched/sch_htb.c | 9 ++++-----
net/sched/sch_qfq.c | 9 ++++-----
6 files changed, 52 insertions(+), 19 deletions(-)
create mode 100644 include/net/tc_class.h
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 15be2d96b06d..891ee8637a92 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -599,6 +599,7 @@ get_default_qdisc_ops(const struct net_device *dev, int ntx)
struct Qdisc_class_common {
u32 classid;
+ unsigned int filter_cnt;
struct hlist_node hnode;
};
diff --git a/include/net/tc_class.h b/include/net/tc_class.h
new file mode 100644
index 000000000000..2ab4aa2dba30
--- /dev/null
+++ b/include/net/tc_class.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __NET_TC_CLASS_H
+#define __NET_TC_CLASS_H
+
+#include <linux/overflow.h>
+#include <net/sch_generic.h>
+
+static inline bool qdisc_class_in_use(const struct Qdisc_class_common *cl)
+{
+ return cl->filter_cnt > 0;
+}
+
+static inline void qdisc_class_get(struct Qdisc_class_common *cl)
+{
+ unsigned int res;
+
+ if (check_add_overflow(cl->filter_cnt, 1, &res))
+ WARN(1, "Qdisc class overflow");
+
+ cl->filter_cnt = res;
+}
+
+static inline void qdisc_class_put(struct Qdisc_class_common *cl)
+{
+ unsigned int res;
+
+ if (check_sub_overflow(cl->filter_cnt, 1, &res))
+ WARN(1, "Qdisc class underflow");
+
+ cl->filter_cnt = res;
+}
+
+#endif
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index e35a4e90f4e6..0bffa01c61bb 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -14,10 +14,10 @@
#include <net/sch_generic.h>
#include <net/pkt_sched.h>
#include <net/pkt_cls.h>
+#include <net/tc_class.h>
struct drr_class {
struct Qdisc_class_common common;
- unsigned int filter_cnt;
struct gnet_stats_basic_sync bstats;
struct gnet_stats_queue qstats;
@@ -150,7 +150,7 @@ static int drr_delete_class(struct Qdisc *sch, unsigned long arg,
struct drr_sched *q = qdisc_priv(sch);
struct drr_class *cl = (struct drr_class *)arg;
- if (cl->filter_cnt > 0)
+ if (qdisc_class_in_use(&cl->common))
return -EBUSY;
sch_tree_lock(sch);
@@ -187,8 +187,8 @@ static unsigned long drr_bind_tcf(struct Qdisc *sch, unsigned long parent,
{
struct drr_class *cl = drr_find_class(sch, classid);
- if (cl != NULL)
- cl->filter_cnt++;
+ if (cl)
+ qdisc_class_get(&cl->common);
return (unsigned long)cl;
}
@@ -197,7 +197,7 @@ static void drr_unbind_tcf(struct Qdisc *sch, unsigned long arg)
{
struct drr_class *cl = (struct drr_class *)arg;
- cl->filter_cnt--;
+ qdisc_class_put(&cl->common);
}
static int drr_graft_class(struct Qdisc *sch, unsigned long arg,
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 70b0c5873d32..122fe775c7ab 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -67,6 +67,7 @@
#include <net/pkt_sched.h>
#include <net/pkt_cls.h>
#include <asm/div64.h>
+#include <net/tc_class.h>
/*
* kernel internal service curve representation:
@@ -116,7 +117,6 @@ struct hfsc_class {
struct net_rate_estimator __rcu *rate_est;
struct tcf_proto __rcu *filter_list; /* filter list */
struct tcf_block *block;
- unsigned int filter_cnt; /* filter count */
unsigned int level; /* class level in hierarchy */
struct hfsc_sched *sched; /* scheduler data */
@@ -1094,7 +1094,8 @@ hfsc_delete_class(struct Qdisc *sch, unsigned long arg,
struct hfsc_sched *q = qdisc_priv(sch);
struct hfsc_class *cl = (struct hfsc_class *)arg;
- if (cl->level > 0 || cl->filter_cnt > 0 || cl == &q->root)
+ if (cl->level > 0 || qdisc_class_in_use(&cl->cl_common) ||
+ cl == &q->root)
return -EBUSY;
sch_tree_lock(sch);
@@ -1223,7 +1224,7 @@ hfsc_bind_tcf(struct Qdisc *sch, unsigned long parent, u32 classid)
if (cl != NULL) {
if (p != NULL && p->level <= cl->level)
return 0;
- cl->filter_cnt++;
+ qdisc_class_get(&cl->cl_common);
}
return (unsigned long)cl;
@@ -1234,7 +1235,7 @@ hfsc_unbind_tcf(struct Qdisc *sch, unsigned long arg)
{
struct hfsc_class *cl = (struct hfsc_class *)arg;
- cl->filter_cnt--;
+ qdisc_class_put(&cl->cl_common);
}
static struct tcf_block *hfsc_tcf_block(struct Qdisc *sch, unsigned long arg,
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 325c29041c7d..5223b63cec00 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -37,6 +37,7 @@
#include <net/sch_generic.h>
#include <net/pkt_sched.h>
#include <net/pkt_cls.h>
+#include <net/tc_class.h>
/* HTB algorithm.
Author: devik@....cz
@@ -102,7 +103,6 @@ struct htb_class {
struct tcf_proto __rcu *filter_list; /* class attached filters */
struct tcf_block *block;
- int filter_cnt;
int level; /* our level (see above) */
unsigned int children;
@@ -1710,7 +1710,7 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg,
* tc subsys guarantee us that in htb_destroy it holds no class
* refs so that we can remove children safely there ?
*/
- if (cl->children || cl->filter_cnt)
+ if (cl->children || qdisc_class_in_use(&cl->common))
return -EBUSY;
if (!cl->level && htb_parent_last_child(cl))
@@ -2108,7 +2108,7 @@ static unsigned long htb_bind_filter(struct Qdisc *sch, unsigned long parent,
* be broken by class during destroy IIUC.
*/
if (cl)
- cl->filter_cnt++;
+ qdisc_class_get(&cl->common);
return (unsigned long)cl;
}
@@ -2116,8 +2116,7 @@ static void htb_unbind_filter(struct Qdisc *sch, unsigned long arg)
{
struct htb_class *cl = (struct htb_class *)arg;
- if (cl)
- cl->filter_cnt--;
+ qdisc_class_put(&cl->common);
}
static void htb_walk(struct Qdisc *sch, struct qdisc_walker *arg)
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index befaf74b33ca..2515828d99a6 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -15,6 +15,7 @@
#include <net/sch_generic.h>
#include <net/pkt_sched.h>
#include <net/pkt_cls.h>
+#include <net/tc_class.h>
/* Quick Fair Queueing Plus
@@ -130,8 +131,6 @@ struct qfq_aggregate;
struct qfq_class {
struct Qdisc_class_common common;
- unsigned int filter_cnt;
-
struct gnet_stats_basic_sync bstats;
struct gnet_stats_queue qstats;
struct net_rate_estimator __rcu *rate_est;
@@ -545,7 +544,7 @@ static int qfq_delete_class(struct Qdisc *sch, unsigned long arg,
struct qfq_sched *q = qdisc_priv(sch);
struct qfq_class *cl = (struct qfq_class *)arg;
- if (cl->filter_cnt > 0)
+ if (qdisc_class_in_use(&cl->common))
return -EBUSY;
sch_tree_lock(sch);
@@ -581,7 +580,7 @@ static unsigned long qfq_bind_tcf(struct Qdisc *sch, unsigned long parent,
struct qfq_class *cl = qfq_find_class(sch, classid);
if (cl != NULL)
- cl->filter_cnt++;
+ qdisc_class_get(&cl->common);
return (unsigned long)cl;
}
@@ -590,7 +589,7 @@ static void qfq_unbind_tcf(struct Qdisc *sch, unsigned long arg)
{
struct qfq_class *cl = (struct qfq_class *)arg;
- cl->filter_cnt--;
+ qdisc_class_put(&cl->common);
}
static int qfq_graft_class(struct Qdisc *sch, unsigned long arg,
--
2.39.2
Powered by blists - more mailing lists