[PATCH net-next v2 0/3] sch_netem: some improvements and cleanups.

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups.
@ 2014-02-14  2:30 Yang Yingliang
  2014-02-14  2:30 ` [PATCH net-next v2 1/3] sch_netem: return errcode before setting params Yang Yingliang
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Yang Yingliang @ 2014-02-14  2:30 UTC (permalink / raw)
  To: netdev; +Cc: davem, stephen

v1 -> v2:
	remove patch #4.

 patch 1/3: To avoid breaking old settings when we change failed,
	    do return errcode before doing setup.
 pacth 2/3: Replace some functions' parameters, these functions
	    only need struct netem_sched_data *q.
 patch 3/3: Replace magic numbers with enumerate for better readability.

Yang Yingliang (3):
  sch_netem: return errcode before setting params
  sch_netem: change some func's param from "struct Qdisc *" to "struct
    netem_sched_data *"
  sch_netem: replace magic numbers with enumerate in GE model

 net/sched/sch_netem.c | 75 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 28 deletions(-)

-- 
1.8.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 1/3] sch_netem: return errcode before setting params
  2014-02-14  2:30 [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups Yang Yingliang
@ 2014-02-14  2:30 ` Yang Yingliang
  2014-02-14  2:30 ` [PATCH net-next v2 2/3] sch_netem: change some func's param from "struct Qdisc *" to "struct netem_sched_data *" Yang Yingliang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Yang Yingliang @ 2014-02-14  2:30 UTC (permalink / raw)
  To: netdev; +Cc: davem, stephen

get_dist_table() and get_loss_clg() may be failed. These
two functions should be called after setting the members
of qdisc_priv(sch), or it will break the old settings while
either of them is failed.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/sch_netem.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index de1059a..b341943 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -821,6 +821,8 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt)
 	struct netem_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_NETEM_MAX + 1];
 	struct tc_netem_qopt *qopt;
+	struct clgstate old_clg;
+	int old_loss_model = CLG_RANDOM;
 	int ret;
 
 	if (opt == NULL)
@@ -831,6 +833,33 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt)
 	if (ret < 0)
 		return ret;
 
+	/* backup q->clg and q->loss_model */
+	old_clg = q->clg;
+	old_loss_model = q->loss_model;
+
+	if (tb[TCA_NETEM_LOSS]) {
+		ret = get_loss_clg(sch, tb[TCA_NETEM_LOSS]);
+		if (ret) {
+			q->loss_model = old_loss_model;
+			return ret;
+		}
+	} else {
+		q->loss_model = CLG_RANDOM;
+	}
+
+	if (tb[TCA_NETEM_DELAY_DIST]) {
+		ret = get_dist_table(sch, tb[TCA_NETEM_DELAY_DIST]);
+		if (ret) {
+			/* recover clg and loss_model, in case of
+			 * q->clg and q->loss_model were modified
+			 * in get_loss_clg()
+			 */
+			q->clg = old_clg;
+			q->loss_model = old_loss_model;
+			return ret;
+		}
+	}
+
 	sch->limit = qopt->limit;
 
 	q->latency = qopt->latency;
@@ -850,12 +879,6 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt)
 	if (tb[TCA_NETEM_CORR])
 		get_correlation(sch, tb[TCA_NETEM_CORR]);
 
-	if (tb[TCA_NETEM_DELAY_DIST]) {
-		ret = get_dist_table(sch, tb[TCA_NETEM_DELAY_DIST]);
-		if (ret)
-			return ret;
-	}
-
 	if (tb[TCA_NETEM_REORDER])
 		get_reorder(sch, tb[TCA_NETEM_REORDER]);
 
@@ -872,10 +895,6 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt)
 	if (tb[TCA_NETEM_ECN])
 		q->ecn = nla_get_u32(tb[TCA_NETEM_ECN]);
 
-	q->loss_model = CLG_RANDOM;
-	if (tb[TCA_NETEM_LOSS])
-		ret = get_loss_clg(sch, tb[TCA_NETEM_LOSS]);
-
 	return ret;
 }
 
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 2/3] sch_netem: change some func's param from "struct Qdisc *" to "struct netem_sched_data *"
  2014-02-14  2:30 [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups Yang Yingliang
  2014-02-14  2:30 ` [PATCH net-next v2 1/3] sch_netem: return errcode before setting params Yang Yingliang
@ 2014-02-14  2:30 ` Yang Yingliang
  2014-02-14  2:30 ` [PATCH net-next v2 3/3] sch_netem: replace magic numbers with enumerate in GE model Yang Yingliang
  2014-02-14  5:15 ` [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups David Miller
  3 siblings, 0 replies; 7+ messages in thread
From: Yang Yingliang @ 2014-02-14  2:30 UTC (permalink / raw)
  To: netdev; +Cc: davem, stephen

In netem_change(), we have already get "struct netem_sched_data *q".
Replace params of get_correlation() and other similar functions with
"struct netem_sched_data *q".

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/sch_netem.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index b341943..4a5eb28 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -689,9 +689,8 @@ static int get_dist_table(struct Qdisc *sch, const struct nlattr *attr)
 	return 0;
 }
 
-static void get_correlation(struct Qdisc *sch, const struct nlattr *attr)
+static void get_correlation(struct netem_sched_data *q, const struct nlattr *attr)
 {
-	struct netem_sched_data *q = qdisc_priv(sch);
 	const struct tc_netem_corr *c = nla_data(attr);
 
 	init_crandom(&q->delay_cor, c->delay_corr);
@@ -699,27 +698,24 @@ static void get_correlation(struct Qdisc *sch, const struct nlattr *attr)
 	init_crandom(&q->dup_cor, c->dup_corr);
 }
 
-static void get_reorder(struct Qdisc *sch, const struct nlattr *attr)
+static void get_reorder(struct netem_sched_data *q, const struct nlattr *attr)
 {
-	struct netem_sched_data *q = qdisc_priv(sch);
 	const struct tc_netem_reorder *r = nla_data(attr);
 
 	q->reorder = r->probability;
 	init_crandom(&q->reorder_cor, r->correlation);
 }
 
-static void get_corrupt(struct Qdisc *sch, const struct nlattr *attr)
+static void get_corrupt(struct netem_sched_data *q, const struct nlattr *attr)
 {
-	struct netem_sched_data *q = qdisc_priv(sch);
 	const struct tc_netem_corrupt *r = nla_data(attr);
 
 	q->corrupt = r->probability;
 	init_crandom(&q->corrupt_cor, r->correlation);
 }
 
-static void get_rate(struct Qdisc *sch, const struct nlattr *attr)
+static void get_rate(struct netem_sched_data *q, const struct nlattr *attr)
 {
-	struct netem_sched_data *q = qdisc_priv(sch);
 	const struct tc_netem_rate *r = nla_data(attr);
 
 	q->rate = r->rate;
@@ -732,9 +728,8 @@ static void get_rate(struct Qdisc *sch, const struct nlattr *attr)
 		q->cell_size_reciprocal = (struct reciprocal_value) { 0 };
 }
 
-static int get_loss_clg(struct Qdisc *sch, const struct nlattr *attr)
+static int get_loss_clg(struct netem_sched_data *q, const struct nlattr *attr)
 {
-	struct netem_sched_data *q = qdisc_priv(sch);
 	const struct nlattr *la;
 	int rem;
 
@@ -838,7 +833,7 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt)
 	old_loss_model = q->loss_model;
 
 	if (tb[TCA_NETEM_LOSS]) {
-		ret = get_loss_clg(sch, tb[TCA_NETEM_LOSS]);
+		ret = get_loss_clg(q, tb[TCA_NETEM_LOSS]);
 		if (ret) {
 			q->loss_model = old_loss_model;
 			return ret;
@@ -877,16 +872,16 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt)
 		q->reorder = ~0;
 
 	if (tb[TCA_NETEM_CORR])
-		get_correlation(sch, tb[TCA_NETEM_CORR]);
+		get_correlation(q, tb[TCA_NETEM_CORR]);
 
 	if (tb[TCA_NETEM_REORDER])
-		get_reorder(sch, tb[TCA_NETEM_REORDER]);
+		get_reorder(q, tb[TCA_NETEM_REORDER]);
 
 	if (tb[TCA_NETEM_CORRUPT])
-		get_corrupt(sch, tb[TCA_NETEM_CORRUPT]);
+		get_corrupt(q, tb[TCA_NETEM_CORRUPT]);
 
 	if (tb[TCA_NETEM_RATE])
-		get_rate(sch, tb[TCA_NETEM_RATE]);
+		get_rate(q, tb[TCA_NETEM_RATE]);
 
 	if (tb[TCA_NETEM_RATE64])
 		q->rate = max_t(u64, q->rate,
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 3/3] sch_netem: replace magic numbers with enumerate in GE model
  2014-02-14  2:30 [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups Yang Yingliang
  2014-02-14  2:30 ` [PATCH net-next v2 1/3] sch_netem: return errcode before setting params Yang Yingliang
  2014-02-14  2:30 ` [PATCH net-next v2 2/3] sch_netem: change some func's param from "struct Qdisc *" to "struct netem_sched_data *" Yang Yingliang
@ 2014-02-14  2:30 ` Yang Yingliang
  2014-02-16 12:44   ` Ben Hutchings
  2014-02-14  5:15 ` [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups David Miller
  3 siblings, 1 reply; 7+ messages in thread
From: Yang Yingliang @ 2014-02-14  2:30 UTC (permalink / raw)
  To: netdev; +Cc: davem, stephen

Replace some magic numbers which describe states of GE model
loss generator with enumerate.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/sch_netem.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 4a5eb28..4fced67 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -117,6 +117,11 @@ struct netem_sched_data {
 		LOST_IN_BURST_PERIOD,
 	} _4_state_model;
 
+	enum {
+		GOOD_STATE = 1,
+		BAD_STATE,
+	} GE_state_model;
+
 	/* Correlated Loss Generation models */
 	struct clgstate {
 		/* state of the Markov chain */
@@ -272,15 +277,15 @@ static bool loss_gilb_ell(struct netem_sched_data *q)
 	struct clgstate *clg = &q->clg;
 
 	switch (clg->state) {
-	case 1:
+	case GOOD_STATE:
 		if (prandom_u32() < clg->a1)
-			clg->state = 2;
+			clg->state = BAD_STATE;
 		if (prandom_u32() < clg->a4)
 			return true;
 		break;
-	case 2:
+	case BAD_STATE:
 		if (prandom_u32() < clg->a2)
-			clg->state = 1;
+			clg->state = GOOD_STATE;
 		if (prandom_u32() > clg->a3)
 			return true;
 	}
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2 3/3] sch_netem: replace magic numbers with enumerate in GE model
  2014-02-14  2:30 ` [PATCH net-next v2 3/3] sch_netem: replace magic numbers with enumerate in GE model Yang Yingliang
@ 2014-02-16 12:44   ` Ben Hutchings
  2014-02-17  6:38     ` Yang Yingliang
  0 siblings, 1 reply; 7+ messages in thread
From: Ben Hutchings @ 2014-02-16 12:44 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: netdev, davem, stephen

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On Fri, 2014-02-14 at 10:30 +0800, Yang Yingliang wrote:
> Replace some magic numbers which describe states of GE model
> loss generator with enumerate.
[...]

The name GOOD_STATE should also be used in get_loss_clg() where
clgstate::state is initialised.

Ben.

-- 
Ben Hutchings
Any sufficiently advanced bug is indistinguishable from a feature.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2 3/3] sch_netem: replace magic numbers with enumerate in GE model
  2014-02-16 12:44   ` Ben Hutchings
@ 2014-02-17  6:38     ` Yang Yingliang
  0 siblings, 0 replies; 7+ messages in thread
From: Yang Yingliang @ 2014-02-17  6:38 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, davem, stephen

On 2014/2/16 20:44, Ben Hutchings wrote:
> On Fri, 2014-02-14 at 10:30 +0800, Yang Yingliang wrote:
>> Replace some magic numbers which describe states of GE model
>> loss generator with enumerate.
> [...]
> 
> The name GOOD_STATE should also be used in get_loss_clg() where
> clgstate::state is initialised.
> 
> Ben.
> 
OK, thanks!

Regards,
Yang

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups.
  2014-02-14  2:30 [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups Yang Yingliang
                   ` (2 preceding siblings ...)
  2014-02-14  2:30 ` [PATCH net-next v2 3/3] sch_netem: replace magic numbers with enumerate in GE model Yang Yingliang
@ 2014-02-14  5:15 ` David Miller
  3 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2014-02-14  5:15 UTC (permalink / raw)
  To: yangyingliang; +Cc: netdev, stephen

From: Yang Yingliang <yangyingliang@huawei.com>
Date: Fri, 14 Feb 2014 10:30:40 +0800

> v1 -> v2:
> 	remove patch #4.
> 
>  patch 1/3: To avoid breaking old settings when we change failed,
> 	    do return errcode before doing setup.
>  pacth 2/3: Replace some functions' parameters, these functions
> 	    only need struct netem_sched_data *q.
>  patch 3/3: Replace magic numbers with enumerate for better readability.

Series applied.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-02-17  6:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-14  2:30 [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups Yang Yingliang
2014-02-14  2:30 ` [PATCH net-next v2 1/3] sch_netem: return errcode before setting params Yang Yingliang
2014-02-14  2:30 ` [PATCH net-next v2 2/3] sch_netem: change some func's param from "struct Qdisc *" to "struct netem_sched_data *" Yang Yingliang
2014-02-14  2:30 ` [PATCH net-next v2 3/3] sch_netem: replace magic numbers with enumerate in GE model Yang Yingliang
2014-02-16 12:44   ` Ben Hutchings
2014-02-17  6:38     ` Yang Yingliang
2014-02-14  5:15 ` [PATCH net-next v2 0/3] sch_netem: some improvements and cleanups David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.