[PATCH net-next 2/6] net: sched: Add centralized RED flag checking

[Ido Schimmel <idosch@idosch.org>]

From: Petr Machata <petrm@mellanox.com>

The qdiscs RED, GRED, SFQ and CHOKE use different subsets of the same pool
of global RED flags. Add a common function for all of these to validate
that only supported flags are passed. In later patches this function will
be extended with a check for flag compatibility / meaningfulness.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 include/net/red.h     | 12 ++++++++++++
 net/sched/sch_choke.c |  5 +++++
 net/sched/sch_gred.c  |  7 +++----
 net/sched/sch_red.c   |  5 +++++
 net/sched/sch_sfq.c   | 10 ++++++++--
 5 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/include/net/red.h b/include/net/red.h
index 9665582c4687..bb7bac52c365 100644
--- a/include/net/red.h
+++ b/include/net/red.h
@@ -179,6 +179,18 @@ static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog)
 	return true;
 }
 
+static inline bool red_check_flags(unsigned int flags,
+				   unsigned int supported_flags,
+				   struct netlink_ext_ack *extack)
+{
+	if (flags & ~supported_flags) {
+		NL_SET_ERR_MSG_MOD(extack, "unsupported RED flags specified");
+		return false;
+	}
+
+	return true;
+}
+
 static inline void red_set_parms(struct red_parms *p,
 				 u32 qth_min, u32 qth_max, u8 Wlog, u8 Plog,
 				 u8 Scell_log, u8 *stab, u32 max_P)
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index a36974e9c601..c0e0c9f1ace3 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -78,6 +78,8 @@ static unsigned int choke_len(const struct choke_sched_data *q)
 	return (q->tail - q->head) & q->tab_mask;
 }
 
+#define CHOKE_SUPPORTED_FLAGS (TC_RED_ECN | TC_RED_HARDDROP)
+
 /* Is ECN parameter configured */
 static int use_ecn(const struct choke_sched_data *q)
 {
@@ -370,6 +372,9 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt,
 	if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
 		return -EINVAL;
 
+	if (!red_check_flags(ctl->flags, CHOKE_SUPPORTED_FLAGS, extack))
+		return -EINVAL;
+
 	if (ctl->limit > CHOKE_MAX_QUEUE)
 		return -EINVAL;
 
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index 8599c6f31b05..5e1cb4b243ce 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -428,6 +428,8 @@ static int gred_change_table_def(struct Qdisc *sch, struct nlattr *dps,
 		NL_SET_ERR_MSG_MOD(extack, "can't set per-Qdisc RED flags when per-virtual queue flags are used");
 		return -EINVAL;
 	}
+	if (!red_check_flags(sopt->flags, GRED_VQ_RED_FLAGS, extack))
+		return -EINVAL;
 
 	sch_tree_lock(sch);
 	table->DPs = sopt->DPs;
@@ -590,11 +592,8 @@ static int gred_vq_validate(struct gred_sched *table, u32 cdp,
 			NL_SET_ERR_MSG_MOD(extack, "can't change per-virtual queue RED flags when per-Qdisc flags are used");
 			return -EINVAL;
 		}
-		if (red_flags & ~GRED_VQ_RED_FLAGS) {
-			NL_SET_ERR_MSG_MOD(extack,
-					   "invalid RED flags specified");
+		if (!red_check_flags(red_flags, GRED_VQ_RED_FLAGS, extack))
 			return -EINVAL;
-		}
 	}
 
 	return 0;
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 1695421333e3..f9839d68b811 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -44,6 +44,8 @@ struct red_sched_data {
 	struct Qdisc		*qdisc;
 };
 
+#define RED_SUPPORTED_FLAGS (TC_RED_ECN | TC_RED_HARDDROP | TC_RED_ADAPTATIVE)
+
 static inline int red_use_ecn(struct red_sched_data *q)
 {
 	return q->flags & TC_RED_ECN;
@@ -216,6 +218,9 @@ static int red_change(struct Qdisc *sch, struct nlattr *opt,
 	if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
 		return -EINVAL;
 
+	if (!red_check_flags(ctl->flags, RED_SUPPORTED_FLAGS, extack))
+		return -EINVAL;
+
 	if (ctl->limit > 0) {
 		child = fifo_create_dflt(sch, &bfifo_qdisc_ops, ctl->limit,
 					 extack);
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index c787d4d46017..28949e0ec075 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -325,6 +325,8 @@ static unsigned int sfq_drop(struct Qdisc *sch, struct sk_buff **to_free)
 	return 0;
 }
 
+#define SFQ_SUPPORTED_FLAGS (TC_RED_ECN | TC_RED_HARDDROP)
+
 /* Is ECN parameter configured */
 static int sfq_prob_mark(const struct sfq_sched_data *q)
 {
@@ -620,7 +622,8 @@ static void sfq_perturbation(struct timer_list *t)
 		mod_timer(&q->perturb_timer, jiffies + q->perturb_period);
 }
 
-static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
+static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	struct sfq_sched_data *q = qdisc_priv(sch);
 	struct tc_sfq_qopt *ctl = nla_data(opt);
@@ -640,6 +643,9 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
 	if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
 					ctl_v1->Wlog))
 		return -EINVAL;
+	if (ctl_v1 && !red_check_flags(ctl_v1->flags, SFQ_SUPPORTED_FLAGS,
+				       extack))
+		return -EINVAL;
 	if (ctl_v1 && ctl_v1->qth_min) {
 		p = kmalloc(sizeof(*p), GFP_KERNEL);
 		if (!p)
@@ -750,7 +756,7 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt,
 	get_random_bytes(&q->perturbation, sizeof(q->perturbation));
 
 	if (opt) {
-		int err = sfq_change(sch, opt);
+		int err = sfq_change(sch, opt, extack);
 		if (err)
 			return err;
 	}
-- 
2.24.1