* [PATCH] (3/3) netem: allow random reordering
@ 2005-05-19 22:12 Stephen Hemminger
2005-05-20 20:28 ` [Netem] " Julio Kriger
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Stephen Hemminger @ 2005-05-19 22:12 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, netem
Enhance the reorder feature of netem to allow random percent to be reordered.
Has expected backwards compatibility behaviour.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
Index: netem-2.6.12-rc4/include/linux/pkt_sched.h
===================================================================
--- netem-2.6.12-rc4.orig/include/linux/pkt_sched.h
+++ netem-2.6.12-rc4/include/linux/pkt_sched.h
@@ -427,6 +427,7 @@ enum
TCA_NETEM_UNSPEC,
TCA_NETEM_CORR,
TCA_NETEM_DELAY_DIST,
+ TCA_NETEM_REORDER,
__TCA_NETEM_MAX,
};
@@ -437,7 +438,7 @@ struct tc_netem_qopt
__u32 latency; /* added delay (us) */
__u32 limit; /* fifo limit (packets) */
__u32 loss; /* random packet loss (0=none ~0=100%) */
- __u32 gap; /* re-ordering gap (0 for delay all) */
+ __u32 gap; /* re-ordering gap (0 for none) */
__u32 duplicate; /* random packet dup (0=none ~0=100%) */
__u32 jitter; /* random jitter in latency (us) */
};
@@ -449,6 +450,12 @@ struct tc_netem_corr
__u32 dup_corr; /* duplicate correlation */
};
+struct tc_netem_reorder
+{
+ __u32 probability;
+ __u32 correlation;
+};
+
#define NETEM_DIST_SCALE 8192
#endif
Index: netem-2.6.12-rc4/net/sched/sch_netem.c
===================================================================
--- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
+++ netem-2.6.12-rc4/net/sched/sch_netem.c
@@ -62,11 +62,12 @@ struct netem_sched_data {
u32 gap;
u32 jitter;
u32 duplicate;
+ u32 reorder;
struct crndstate {
unsigned long last;
unsigned long rho;
- } delay_cor, loss_cor, dup_cor;
+ } delay_cor, loss_cor, dup_cor, reorder_cor;
struct disttable {
u32 size;
@@ -185,18 +186,18 @@ static int netem_enqueue(struct sk_buff
* of the queue.
* gap == 0 is special case for no-reordering.
*/
- if (q->gap == 0 || q->counter != q->gap) {
+ if (q->gap == 0 && q->counter < q->gap &&
+ q->reorder < get_crandom(&q->reorder_cor)) {
psched_time_t now;
PSCHED_GET_TIME(now);
- PSCHED_TADD2(now,
- tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
+ PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
+ &q->delay_cor, q->delay_dist),
cb->time_to_send);
-
++q->counter;
ret = q->qdisc->enqueue(skb, q->qdisc);
} else {
- q->counter = 0;
PSCHED_GET_TIME(cb->time_to_send);
+ q->counter = 0;
ret = q->qdisc->ops->requeue(skb, q->qdisc);
}
@@ -351,6 +352,19 @@ static int get_correlation(struct Qdisc
return 0;
}
+static int get_reorder(struct Qdisc *sch, const struct rtattr *attr)
+{
+ struct netem_sched_data *q = qdisc_priv(sch);
+ const struct tc_netem_reorder *r = RTA_DATA(attr);
+
+ if (RTA_PAYLOAD(attr) != sizeof(*r))
+ return -EINVAL;
+
+ q->reorder = r->probability;
+ init_crandom(&q->reorder_cor, r->correlation);
+ return 0;
+}
+
static int netem_change(struct Qdisc *sch, struct rtattr *opt)
{
struct netem_sched_data *q = qdisc_priv(sch);
@@ -371,9 +385,15 @@ static int netem_change(struct Qdisc *sc
q->jitter = qopt->jitter;
q->limit = qopt->limit;
q->gap = qopt->gap;
+ q->counter = 0;
q->loss = qopt->loss;
q->duplicate = qopt->duplicate;
+ /* for compatiablity with earlier versions.
+ * if gap is set, need to assume 100% probablity
+ */
+ q->reorder = ~0;
+
/* Handle nested options after initial queue options.
* Should have put all options in nested format but too late now.
*/
@@ -395,6 +415,11 @@ static int netem_change(struct Qdisc *sc
if (ret)
return ret;
}
+ if (tb[TCA_NETEM_REORDER-1]) {
+ ret = get_reorder(sch, tb[TCA_NETEM_REORDER-1]);
+ if (ret)
+ return ret;
+ }
}
@@ -412,7 +437,6 @@ static int netem_init(struct Qdisc *sch,
init_timer(&q->timer);
q->timer.function = netem_watchdog;
q->timer.data = (unsigned long) sch;
- q->counter = 0;
q->qdisc = qdisc_create_dflt(sch->dev, &pfifo_qdisc_ops);
if (!q->qdisc) {
@@ -444,6 +468,7 @@ static int netem_dump(struct Qdisc *sch,
struct rtattr *rta = (struct rtattr *) b;
struct tc_netem_qopt qopt;
struct tc_netem_corr cor;
+ struct tc_netem_reorder reorder;
qopt.latency = q->latency;
qopt.jitter = q->jitter;
@@ -457,6 +482,11 @@ static int netem_dump(struct Qdisc *sch,
cor.loss_corr = q->loss_cor.rho;
cor.dup_corr = q->dup_cor.rho;
RTA_PUT(skb, TCA_NETEM_CORR, sizeof(cor), &cor);
+
+ reorder.probability = q->reorder;
+ reorder.correlation = q->reorder_cor.rho;
+ RTA_PUT(skb, TCA_NETEM_REORDER, sizeof(reorder), &reorder);
+
rta->rta_len = skb->tail - b;
return skb->len;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Netem] [PATCH] (3/3) netem: allow random reordering
2005-05-19 22:12 [PATCH] (3/3) netem: allow random reordering Stephen Hemminger
@ 2005-05-20 20:28 ` Julio Kriger
2005-05-23 17:25 ` Stephen Hemminger
2005-05-21 15:05 ` Julio Kriger
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Julio Kriger @ 2005-05-20 20:28 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
Hi!
How do I use this feature? Shouldn't tc be modified to accept this new feature?
Regards,
Julio
On 5/19/05, Stephen Hemminger <shemminger@osdl.org> wrote:
> Enhance the reorder feature of netem to allow random percent to be reordered.
> Has expected backwards compatibility behaviour.
>
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
>
> Index: netem-2.6.12-rc4/include/linux/pkt_sched.h
> ===================================================================
> --- netem-2.6.12-rc4.orig/include/linux/pkt_sched.h
> +++ netem-2.6.12-rc4/include/linux/pkt_sched.h
> @@ -427,6 +427,7 @@ enum
> TCA_NETEM_UNSPEC,
> TCA_NETEM_CORR,
> TCA_NETEM_DELAY_DIST,
> + TCA_NETEM_REORDER,
> __TCA_NETEM_MAX,
> };
>
> @@ -437,7 +438,7 @@ struct tc_netem_qopt
> __u32 latency; /* added delay (us) */
> __u32 limit; /* fifo limit (packets) */
> __u32 loss; /* random packet loss (0=none ~0=100%) */
> - __u32 gap; /* re-ordering gap (0 for delay all) */
> + __u32 gap; /* re-ordering gap (0 for none) */
> __u32 duplicate; /* random packet dup (0=none ~0=100%) */
> __u32 jitter; /* random jitter in latency (us) */
> };
> @@ -449,6 +450,12 @@ struct tc_netem_corr
> __u32 dup_corr; /* duplicate correlation */
> };
>
> +struct tc_netem_reorder
> +{
> + __u32 probability;
> + __u32 correlation;
> +};
> +
> #define NETEM_DIST_SCALE 8192
>
> #endif
> Index: netem-2.6.12-rc4/net/sched/sch_netem.c
> ===================================================================
> --- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
> +++ netem-2.6.12-rc4/net/sched/sch_netem.c
> @@ -62,11 +62,12 @@ struct netem_sched_data {
> u32 gap;
> u32 jitter;
> u32 duplicate;
> + u32 reorder;
>
> struct crndstate {
> unsigned long last;
> unsigned long rho;
> - } delay_cor, loss_cor, dup_cor;
> + } delay_cor, loss_cor, dup_cor, reorder_cor;
>
> struct disttable {
> u32 size;
> @@ -185,18 +186,18 @@ static int netem_enqueue(struct sk_buff
> * of the queue.
> * gap == 0 is special case for no-reordering.
> */
> - if (q->gap == 0 || q->counter != q->gap) {
> + if (q->gap == 0 && q->counter < q->gap &&
> + q->reorder < get_crandom(&q->reorder_cor)) {
> psched_time_t now;
> PSCHED_GET_TIME(now);
> - PSCHED_TADD2(now,
> - tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
> + PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
> + &q->delay_cor, q->delay_dist),
> cb->time_to_send);
> -
> ++q->counter;
> ret = q->qdisc->enqueue(skb, q->qdisc);
> } else {
> - q->counter = 0;
> PSCHED_GET_TIME(cb->time_to_send);
> + q->counter = 0;
> ret = q->qdisc->ops->requeue(skb, q->qdisc);
> }
>
> @@ -351,6 +352,19 @@ static int get_correlation(struct Qdisc
> return 0;
> }
>
> +static int get_reorder(struct Qdisc *sch, const struct rtattr *attr)
> +{
> + struct netem_sched_data *q = qdisc_priv(sch);
> + const struct tc_netem_reorder *r = RTA_DATA(attr);
> +
> + if (RTA_PAYLOAD(attr) != sizeof(*r))
> + return -EINVAL;
> +
> + q->reorder = r->probability;
> + init_crandom(&q->reorder_cor, r->correlation);
> + return 0;
> +}
> +
> static int netem_change(struct Qdisc *sch, struct rtattr *opt)
> {
> struct netem_sched_data *q = qdisc_priv(sch);
> @@ -371,9 +385,15 @@ static int netem_change(struct Qdisc *sc
> q->jitter = qopt->jitter;
> q->limit = qopt->limit;
> q->gap = qopt->gap;
> + q->counter = 0;
> q->loss = qopt->loss;
> q->duplicate = qopt->duplicate;
>
> + /* for compatiablity with earlier versions.
> + * if gap is set, need to assume 100% probablity
> + */
> + q->reorder = ~0;
> +
> /* Handle nested options after initial queue options.
> * Should have put all options in nested format but too late now.
> */
> @@ -395,6 +415,11 @@ static int netem_change(struct Qdisc *sc
> if (ret)
> return ret;
> }
> + if (tb[TCA_NETEM_REORDER-1]) {
> + ret = get_reorder(sch, tb[TCA_NETEM_REORDER-1]);
> + if (ret)
> + return ret;
> + }
> }
>
>
> @@ -412,7 +437,6 @@ static int netem_init(struct Qdisc *sch,
> init_timer(&q->timer);
> q->timer.function = netem_watchdog;
> q->timer.data = (unsigned long) sch;
> - q->counter = 0;
>
> q->qdisc = qdisc_create_dflt(sch->dev, &pfifo_qdisc_ops);
> if (!q->qdisc) {
> @@ -444,6 +468,7 @@ static int netem_dump(struct Qdisc *sch,
> struct rtattr *rta = (struct rtattr *) b;
> struct tc_netem_qopt qopt;
> struct tc_netem_corr cor;
> + struct tc_netem_reorder reorder;
>
> qopt.latency = q->latency;
> qopt.jitter = q->jitter;
> @@ -457,6 +482,11 @@ static int netem_dump(struct Qdisc *sch,
> cor.loss_corr = q->loss_cor.rho;
> cor.dup_corr = q->dup_cor.rho;
> RTA_PUT(skb, TCA_NETEM_CORR, sizeof(cor), &cor);
> +
> + reorder.probability = q->reorder;
> + reorder.correlation = q->reorder_cor.rho;
> + RTA_PUT(skb, TCA_NETEM_REORDER, sizeof(reorder), &reorder);
> +
> rta->rta_len = skb->tail - b;
>
> return skb->len;
>
>
> _______________________________________________
> Netem mailing list
> Netem@lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/netem
>
>
>
--
----------------------------
Julio Kriger
mailto:juliokriger@gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Netem] [PATCH] (3/3) netem: allow random reordering
2005-05-19 22:12 [PATCH] (3/3) netem: allow random reordering Stephen Hemminger
2005-05-20 20:28 ` [Netem] " Julio Kriger
@ 2005-05-21 15:05 ` Julio Kriger
2005-05-21 23:00 ` Julio Kriger
2005-05-24 22:26 ` [PATCH] (3/3) netem: allow random reordering (with fix) Stephen Hemminger
3 siblings, 0 replies; 13+ messages in thread
From: Julio Kriger @ 2005-05-21 15:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
[-- Attachment #1: Type: text/plain, Size: 6316 bytes --]
Hi!
Here is a patch to 'tc' that add funcionality need to use the new
feature reorder.
Regards,
Julio
On 5/19/05, Stephen Hemminger <shemminger@osdl.org> wrote:
> Enhance the reorder feature of netem to allow random percent to be reordered.
> Has expected backwards compatibility behaviour.
>
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
>
> Index: netem-2.6.12-rc4/include/linux/pkt_sched.h
> ===================================================================
> --- netem-2.6.12-rc4.orig/include/linux/pkt_sched.h
> +++ netem-2.6.12-rc4/include/linux/pkt_sched.h
> @@ -427,6 +427,7 @@ enum
> TCA_NETEM_UNSPEC,
> TCA_NETEM_CORR,
> TCA_NETEM_DELAY_DIST,
> + TCA_NETEM_REORDER,
> __TCA_NETEM_MAX,
> };
>
> @@ -437,7 +438,7 @@ struct tc_netem_qopt
> __u32 latency; /* added delay (us) */
> __u32 limit; /* fifo limit (packets) */
> __u32 loss; /* random packet loss (0=none ~0=100%) */
> - __u32 gap; /* re-ordering gap (0 for delay all) */
> + __u32 gap; /* re-ordering gap (0 for none) */
> __u32 duplicate; /* random packet dup (0=none ~0=100%) */
> __u32 jitter; /* random jitter in latency (us) */
> };
> @@ -449,6 +450,12 @@ struct tc_netem_corr
> __u32 dup_corr; /* duplicate correlation */
> };
>
> +struct tc_netem_reorder
> +{
> + __u32 probability;
> + __u32 correlation;
> +};
> +
> #define NETEM_DIST_SCALE 8192
>
> #endif
> Index: netem-2.6.12-rc4/net/sched/sch_netem.c
> ===================================================================
> --- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
> +++ netem-2.6.12-rc4/net/sched/sch_netem.c
> @@ -62,11 +62,12 @@ struct netem_sched_data {
> u32 gap;
> u32 jitter;
> u32 duplicate;
> + u32 reorder;
>
> struct crndstate {
> unsigned long last;
> unsigned long rho;
> - } delay_cor, loss_cor, dup_cor;
> + } delay_cor, loss_cor, dup_cor, reorder_cor;
>
> struct disttable {
> u32 size;
> @@ -185,18 +186,18 @@ static int netem_enqueue(struct sk_buff
> * of the queue.
> * gap == 0 is special case for no-reordering.
> */
> - if (q->gap == 0 || q->counter != q->gap) {
> + if (q->gap == 0 && q->counter < q->gap &&
> + q->reorder < get_crandom(&q->reorder_cor)) {
> psched_time_t now;
> PSCHED_GET_TIME(now);
> - PSCHED_TADD2(now,
> - tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
> + PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
> + &q->delay_cor, q->delay_dist),
> cb->time_to_send);
> -
> ++q->counter;
> ret = q->qdisc->enqueue(skb, q->qdisc);
> } else {
> - q->counter = 0;
> PSCHED_GET_TIME(cb->time_to_send);
> + q->counter = 0;
> ret = q->qdisc->ops->requeue(skb, q->qdisc);
> }
>
> @@ -351,6 +352,19 @@ static int get_correlation(struct Qdisc
> return 0;
> }
>
> +static int get_reorder(struct Qdisc *sch, const struct rtattr *attr)
> +{
> + struct netem_sched_data *q = qdisc_priv(sch);
> + const struct tc_netem_reorder *r = RTA_DATA(attr);
> +
> + if (RTA_PAYLOAD(attr) != sizeof(*r))
> + return -EINVAL;
> +
> + q->reorder = r->probability;
> + init_crandom(&q->reorder_cor, r->correlation);
> + return 0;
> +}
> +
> static int netem_change(struct Qdisc *sch, struct rtattr *opt)
> {
> struct netem_sched_data *q = qdisc_priv(sch);
> @@ -371,9 +385,15 @@ static int netem_change(struct Qdisc *sc
> q->jitter = qopt->jitter;
> q->limit = qopt->limit;
> q->gap = qopt->gap;
> + q->counter = 0;
> q->loss = qopt->loss;
> q->duplicate = qopt->duplicate;
>
> + /* for compatiablity with earlier versions.
> + * if gap is set, need to assume 100% probablity
> + */
> + q->reorder = ~0;
> +
> /* Handle nested options after initial queue options.
> * Should have put all options in nested format but too late now.
> */
> @@ -395,6 +415,11 @@ static int netem_change(struct Qdisc *sc
> if (ret)
> return ret;
> }
> + if (tb[TCA_NETEM_REORDER-1]) {
> + ret = get_reorder(sch, tb[TCA_NETEM_REORDER-1]);
> + if (ret)
> + return ret;
> + }
> }
>
>
> @@ -412,7 +437,6 @@ static int netem_init(struct Qdisc *sch,
> init_timer(&q->timer);
> q->timer.function = netem_watchdog;
> q->timer.data = (unsigned long) sch;
> - q->counter = 0;
>
> q->qdisc = qdisc_create_dflt(sch->dev, &pfifo_qdisc_ops);
> if (!q->qdisc) {
> @@ -444,6 +468,7 @@ static int netem_dump(struct Qdisc *sch,
> struct rtattr *rta = (struct rtattr *) b;
> struct tc_netem_qopt qopt;
> struct tc_netem_corr cor;
> + struct tc_netem_reorder reorder;
>
> qopt.latency = q->latency;
> qopt.jitter = q->jitter;
> @@ -457,6 +482,11 @@ static int netem_dump(struct Qdisc *sch,
> cor.loss_corr = q->loss_cor.rho;
> cor.dup_corr = q->dup_cor.rho;
> RTA_PUT(skb, TCA_NETEM_CORR, sizeof(cor), &cor);
> +
> + reorder.probability = q->reorder;
> + reorder.correlation = q->reorder_cor.rho;
> + RTA_PUT(skb, TCA_NETEM_REORDER, sizeof(reorder), &reorder);
> +
> rta->rta_len = skb->tail - b;
>
> return skb->len;
>
>
> _______________________________________________
> Netem mailing list
> Netem@lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/netem
>
>
>
--
----------------------------
Julio Kriger
mailto:juliokriger@gmail.com
[-- Attachment #2: tc_patch --]
[-- Type: application/octet-stream, Size: 3661 bytes --]
diff -Naur orig/iproute2-2.6.11-050330/include/linux/pkt_sched.h new/iproute2-2.6.11-050330/include/linux/pkt_sched.h
--- orig/iproute2-2.6.11-050330/include/linux/pkt_sched.h 2005-04-01 19:58:11.000000000 +0000
+++ new/iproute2-2.6.11-050330/include/linux/pkt_sched.h 2005-05-21 11:59:12.000000000 +0000
@@ -427,6 +427,7 @@
TCA_NETEM_UNSPEC,
TCA_NETEM_CORR,
TCA_NETEM_DELAY_DIST,
+ TCA_NETEM_REORDER,
__TCA_NETEM_MAX,
};
@@ -437,7 +438,7 @@
__u32 latency; /* added delay (us) */
__u32 limit; /* fifo limit (packets) */
__u32 loss; /* random packet loss (0=none ~0=100%) */
- __u32 gap; /* re-ordering gap (0 for delay all) */
+ __u32 gap; /* re-ordering gap (0 for none) */
__u32 duplicate; /* random packet dup (0=none ~0=100%) */
__u32 jitter; /* random jitter in latency (us) */
};
@@ -449,6 +450,12 @@
__u32 dup_corr; /* duplicate correlation */
};
+struct tc_netem_reorder
+{
+ __u32 probability;
+ __u32 correlation;
+};
+
#define NETEM_DIST_SCALE 8192
#endif
diff -Naur orig/iproute2-2.6.11-050330/tc/q_netem.c new/iproute2-2.6.11-050330/tc/q_netem.c
--- orig/iproute2-2.6.11-050330/tc/q_netem.c 2005-04-01 19:58:11.000000000 +0000
+++ new/iproute2-2.6.11-050330/tc/q_netem.c 2005-05-21 10:58:33.000000000 +0000
@@ -33,6 +33,7 @@
" [ drop PERCENT [CORRELATION]] \n" \
" [ duplicate PERCENT [CORRELATION]]\n" \
" [ distribution {uniform|normal|pareto|paretonormal} ]\n" \
+" [ reorder PERCENT [CORRELATION]]\n" \
" [ gap PACKETS ]\n");
}
@@ -127,11 +128,13 @@
struct rtattr *tail;
struct tc_netem_qopt opt;
struct tc_netem_corr cor;
+ struct tc_netem_reorder reorder;
__s16 dist_data[MAXDIST];
memset(&opt, 0, sizeof(opt));
opt.limit = 1000;
memset(&cor, 0, sizeof(cor));
+ memset(&reorder, 0, sizeof(reorder));
while (argc > 0) {
if (matches(*argv, "limit") == 0) {
@@ -197,6 +200,19 @@
return -1;
}
}
+ } else if (matches(*argv, "reorder") == 0) {
+ NEXT_ARG();
+ if (get_percent(&reorder.probability, *argv)) {
+ explain1("reorder");
+ return -1;
+ }
+ if (NEXT_IS_NUMBER()) {
+ NEXT_ARG();
+ if (get_percent(&reorder.correlation, *argv)) {
+ explain1("reorder");
+ return -1;
+ }
+ }
} else if (matches(*argv, "distribution") == 0) {
NEXT_ARG();
dist_size = get_distribution(*argv, dist_data);
@@ -217,6 +233,7 @@
addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
addattr_l(n, 1024, TCA_NETEM_CORR, &cor, sizeof(cor));
+ addattr_l(n, 1024, TCA_NETEM_REORDER, &reorder, sizeof(reorder));
if (dist_size > 0) {
addattr_l(n, 32768, TCA_NETEM_DELAY_DIST,
@@ -229,7 +246,8 @@
static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
{
const struct tc_netem_corr *cor = NULL;
- struct tc_netem_qopt qopt;
+ const struct tc_netem_reorder *reorder = NULL;
+ struct tc_netem_qopt qopt;
int len = RTA_PAYLOAD(opt) - sizeof(qopt);
SPRINT_BUF(b1);
@@ -252,6 +270,12 @@
return -1;
cor = RTA_DATA(tb[TCA_NETEM_CORR]);
}
+
+ if (tb[TCA_NETEM_REORDER]) {
+ if (RTA_PAYLOAD(tb[TCA_NETEM_REORDER]) < sizeof(*reorder))
+ return -1;
+ reorder = RTA_DATA(tb[TCA_NETEM_REORDER]);
+ }
}
fprintf(f, "limit %d", qopt.limit);
@@ -279,6 +303,13 @@
fprintf(f, " %s", sprint_percent(cor->dup_corr, b1));
}
+ if (reorder && reorder->probability) {
+ fprintf(f, " reorder %s",
+ sprint_percent(reorder->probability, b1));
+ if (reorder && reorder->correlation)
+ fprintf(f, " %s", sprint_percent(reorder->correlation, b1));
+ }
+
if (qopt.gap)
fprintf(f, " gap %lu", (unsigned long)qopt.gap);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Netem] [PATCH] (3/3) netem: allow random reordering
2005-05-19 22:12 [PATCH] (3/3) netem: allow random reordering Stephen Hemminger
2005-05-20 20:28 ` [Netem] " Julio Kriger
2005-05-21 15:05 ` Julio Kriger
@ 2005-05-21 23:00 ` Julio Kriger
2005-05-23 17:43 ` [PATCH] netem: fix logic bug in reorder conditional Stephen Hemminger
2005-05-24 22:26 ` [PATCH] (3/3) netem: allow random reordering (with fix) Stephen Hemminger
3 siblings, 1 reply; 13+ messages in thread
From: Julio Kriger @ 2005-05-21 23:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
Hi,
I have a doubt about about this lines (#90)
if (q->gap == 0 && q->counter < q->gap &&
q->reorder < get_crandom(&q->reorder_cor)) {
I could not achieve reordering. I've set gap and reordering values but
did'nt success. Then I've modified those lines to this one
if( 0 < q->reorder && q->reorder < get_crandom(&q->reorder_cor) ) {
and I achieve reordering (I test it pinging localhost).
Is this correct or I'm doing something wrong?
Regards,
Julio
On 5/19/05, Stephen Hemminger <shemminger@osdl.org> wrote:
> Enhance the reorder feature of netem to allow random percent to be reordered.
> Has expected backwards compatibility behaviour.
>
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
>
> Index: netem-2.6.12-rc4/include/linux/pkt_sched.h
> ===================================================================
> --- netem-2.6.12-rc4.orig/include/linux/pkt_sched.h
> +++ netem-2.6.12-rc4/include/linux/pkt_sched.h
> @@ -427,6 +427,7 @@ enum
> TCA_NETEM_UNSPEC,
> TCA_NETEM_CORR,
> TCA_NETEM_DELAY_DIST,
> + TCA_NETEM_REORDER,
> __TCA_NETEM_MAX,
> };
>
> @@ -437,7 +438,7 @@ struct tc_netem_qopt
> __u32 latency; /* added delay (us) */
> __u32 limit; /* fifo limit (packets) */
> __u32 loss; /* random packet loss (0=none ~0=100%) */
> - __u32 gap; /* re-ordering gap (0 for delay all) */
> + __u32 gap; /* re-ordering gap (0 for none) */
> __u32 duplicate; /* random packet dup (0=none ~0=100%) */
> __u32 jitter; /* random jitter in latency (us) */
> };
> @@ -449,6 +450,12 @@ struct tc_netem_corr
> __u32 dup_corr; /* duplicate correlation */
> };
>
> +struct tc_netem_reorder
> +{
> + __u32 probability;
> + __u32 correlation;
> +};
> +
> #define NETEM_DIST_SCALE 8192
>
> #endif
> Index: netem-2.6.12-rc4/net/sched/sch_netem.c
> ===================================================================
> --- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
> +++ netem-2.6.12-rc4/net/sched/sch_netem.c
> @@ -62,11 +62,12 @@ struct netem_sched_data {
> u32 gap;
> u32 jitter;
> u32 duplicate;
> + u32 reorder;
>
> struct crndstate {
> unsigned long last;
> unsigned long rho;
> - } delay_cor, loss_cor, dup_cor;
> + } delay_cor, loss_cor, dup_cor, reorder_cor;
>
> struct disttable {
> u32 size;
> @@ -185,18 +186,18 @@ static int netem_enqueue(struct sk_buff
> * of the queue.
> * gap == 0 is special case for no-reordering.
> */
> - if (q->gap == 0 || q->counter != q->gap) {
> + if (q->gap == 0 && q->counter < q->gap &&
> + q->reorder < get_crandom(&q->reorder_cor)) {
> psched_time_t now;
> PSCHED_GET_TIME(now);
> - PSCHED_TADD2(now,
> - tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
> + PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
> + &q->delay_cor, q->delay_dist),
> cb->time_to_send);
> -
> ++q->counter;
> ret = q->qdisc->enqueue(skb, q->qdisc);
> } else {
> - q->counter = 0;
> PSCHED_GET_TIME(cb->time_to_send);
> + q->counter = 0;
> ret = q->qdisc->ops->requeue(skb, q->qdisc);
> }
>
> @@ -351,6 +352,19 @@ static int get_correlation(struct Qdisc
> return 0;
> }
>
> +static int get_reorder(struct Qdisc *sch, const struct rtattr *attr)
> +{
> + struct netem_sched_data *q = qdisc_priv(sch);
> + const struct tc_netem_reorder *r = RTA_DATA(attr);
> +
> + if (RTA_PAYLOAD(attr) != sizeof(*r))
> + return -EINVAL;
> +
> + q->reorder = r->probability;
> + init_crandom(&q->reorder_cor, r->correlation);
> + return 0;
> +}
> +
> static int netem_change(struct Qdisc *sch, struct rtattr *opt)
> {
> struct netem_sched_data *q = qdisc_priv(sch);
> @@ -371,9 +385,15 @@ static int netem_change(struct Qdisc *sc
> q->jitter = qopt->jitter;
> q->limit = qopt->limit;
> q->gap = qopt->gap;
> + q->counter = 0;
> q->loss = qopt->loss;
> q->duplicate = qopt->duplicate;
>
> + /* for compatiablity with earlier versions.
> + * if gap is set, need to assume 100% probablity
> + */
> + q->reorder = ~0;
> +
> /* Handle nested options after initial queue options.
> * Should have put all options in nested format but too late now.
> */
> @@ -395,6 +415,11 @@ static int netem_change(struct Qdisc *sc
> if (ret)
> return ret;
> }
> + if (tb[TCA_NETEM_REORDER-1]) {
> + ret = get_reorder(sch, tb[TCA_NETEM_REORDER-1]);
> + if (ret)
> + return ret;
> + }
> }
>
>
> @@ -412,7 +437,6 @@ static int netem_init(struct Qdisc *sch,
> init_timer(&q->timer);
> q->timer.function = netem_watchdog;
> q->timer.data = (unsigned long) sch;
> - q->counter = 0;
>
> q->qdisc = qdisc_create_dflt(sch->dev, &pfifo_qdisc_ops);
> if (!q->qdisc) {
> @@ -444,6 +468,7 @@ static int netem_dump(struct Qdisc *sch,
> struct rtattr *rta = (struct rtattr *) b;
> struct tc_netem_qopt qopt;
> struct tc_netem_corr cor;
> + struct tc_netem_reorder reorder;
>
> qopt.latency = q->latency;
> qopt.jitter = q->jitter;
> @@ -457,6 +482,11 @@ static int netem_dump(struct Qdisc *sch,
> cor.loss_corr = q->loss_cor.rho;
> cor.dup_corr = q->dup_cor.rho;
> RTA_PUT(skb, TCA_NETEM_CORR, sizeof(cor), &cor);
> +
> + reorder.probability = q->reorder;
> + reorder.correlation = q->reorder_cor.rho;
> + RTA_PUT(skb, TCA_NETEM_REORDER, sizeof(reorder), &reorder);
> +
> rta->rta_len = skb->tail - b;
>
> return skb->len;
>
>
> _______________________________________________
> Netem mailing list
> Netem@lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/netem
>
>
>
--
----------------------------
Julio Kriger
mailto:juliokriger@gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Netem] [PATCH] (3/3) netem: allow random reordering
2005-05-20 20:28 ` [Netem] " Julio Kriger
@ 2005-05-23 17:25 ` Stephen Hemminger
0 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2005-05-23 17:25 UTC (permalink / raw)
To: Julio Kriger; +Cc: David S. Miller, netdev, netem
On Fri, 20 May 2005 20:28:40 +0000
Julio Kriger <juliokriger@gmail.com> wrote:
> Hi!
> How do I use this feature? Shouldn't tc be modified to accept this new feature?
> Regards,
>
> Julio
Use this (it will be in the next version of iproute2).
Also, you need to specify a delay because that is how netem does the reordering.
diff -X dontdiff -urN iproute2-2.6.11-050330/include/linux/pkt_sched.h iproute2-netem/include/linux/pkt_sched.h
--- iproute2-2.6.11-050330/include/linux/pkt_sched.h 2005-04-01 11:58:11.000000000 -0800
+++ iproute2-netem/include/linux/pkt_sched.h 2005-05-04 11:31:14.000000000 -0700
@@ -427,6 +427,7 @@
TCA_NETEM_UNSPEC,
TCA_NETEM_CORR,
TCA_NETEM_DELAY_DIST,
+ TCA_NETEM_REORDER,
__TCA_NETEM_MAX,
};
@@ -437,7 +438,7 @@
__u32 latency; /* added delay (us) */
__u32 limit; /* fifo limit (packets) */
__u32 loss; /* random packet loss (0=none ~0=100%) */
- __u32 gap; /* re-ordering gap (0 for delay all) */
+ __u32 gap; /* re-ordering gap (0 for none) */
__u32 duplicate; /* random packet dup (0=none ~0=100%) */
__u32 jitter; /* random jitter in latency (us) */
};
@@ -449,6 +450,12 @@
__u32 dup_corr; /* duplicate correlation */
};
+struct tc_netem_reorder
+{
+ __u32 probability;
+ __u32 correlation;
+};
+
#define NETEM_DIST_SCALE 8192
#endif
diff -X dontdiff -urN iproute2-2.6.11-050330/tc/q_netem.c iproute2-netem/tc/q_netem.c
--- iproute2-2.6.11-050330/tc/q_netem.c 2005-04-01 11:58:11.000000000 -0800
+++ iproute2-netem/tc/q_netem.c 2005-05-04 13:31:25.000000000 -0700
@@ -29,11 +29,11 @@
{
fprintf(stderr,
"Usage: ... netem [ limit PACKETS ] \n" \
-" [ delay TIME [ JITTER [CORRELATION]]]\n" \
+" [ delay TIME [ JITTER [CORRELATION]]]\n" \
+" [ distribution {uniform|normal|pareto|paretonormal} ]\n" \
" [ drop PERCENT [CORRELATION]] \n" \
" [ duplicate PERCENT [CORRELATION]]\n" \
-" [ distribution {uniform|normal|pareto|paretonormal} ]\n" \
-" [ gap PACKETS ]\n");
+" [ reorder PRECENT [CORRELATION] [ gap DISTANCE ]]\n");
}
static void explain1(const char *arg)
@@ -127,11 +127,13 @@
struct rtattr *tail;
struct tc_netem_qopt opt;
struct tc_netem_corr cor;
+ struct tc_netem_reorder reorder;
__s16 dist_data[MAXDIST];
memset(&opt, 0, sizeof(opt));
opt.limit = 1000;
memset(&cor, 0, sizeof(cor));
+ memset(&reorder, 0, sizeof(reorder));
while (argc > 0) {
if (matches(*argv, "limit") == 0) {
@@ -178,6 +180,19 @@
return -1;
}
}
+ } else if (matches(*argv, "reorder") == 0) {
+ NEXT_ARG();
+ if (get_percent(&reorder.probability, *argv)) {
+ explain1("reorder");
+ return -1;
+ }
+ if (NEXT_IS_NUMBER()) {
+ NEXT_ARG();
+ if (get_percent(&reorder.correlation, *argv)) {
+ explain1("reorder");
+ return -1;
+ }
+ }
} else if (matches(*argv, "gap") == 0) {
NEXT_ARG();
if (get_u32(&opt.gap, *argv, 0)) {
@@ -215,8 +230,27 @@
tail = NLMSG_TAIL(n);
+ if (reorder.probability) {
+ if (opt.latency == 0) {
+ fprintf(stderr, "reordering not possible without specifying some delay\n");
+ }
+ if (opt.gap == 0)
+ opt.gap = 1;
+ } else if (opt.gap > 0) {
+ fprintf(stderr, "gap specified without reorder probability\n");
+ explain();
+ return -1;
+ }
+
+ if (dist_size > 0 && (opt.latency == 0 || opt.jitter == 0)) {
+ fprintf(stderr, "distribution specified but no latency and jitter values\n");
+ explain();
+ return -1;
+ }
+
addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
addattr_l(n, 1024, TCA_NETEM_CORR, &cor, sizeof(cor));
+ addattr_l(n, 1024, TCA_NETEM_REORDER, &reorder, sizeof(reorder));
if (dist_size > 0) {
addattr_l(n, 32768, TCA_NETEM_DELAY_DIST,
@@ -229,6 +263,7 @@
static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
{
const struct tc_netem_corr *cor = NULL;
+ const struct tc_netem_reorder *reorder = NULL;
struct tc_netem_qopt qopt;
int len = RTA_PAYLOAD(opt) - sizeof(qopt);
SPRINT_BUF(b1);
@@ -252,6 +287,11 @@
return -1;
cor = RTA_DATA(tb[TCA_NETEM_CORR]);
}
+ if (tb[TCA_NETEM_REORDER]) {
+ if (RTA_PAYLOAD(tb[TCA_NETEM_REORDER]) < sizeof(*reorder))
+ return -1;
+ reorder = RTA_DATA(tb[TCA_NETEM_REORDER]);
+ }
}
fprintf(f, "limit %d", qopt.limit);
@@ -278,10 +318,19 @@
if (cor && cor->dup_corr)
fprintf(f, " %s", sprint_percent(cor->dup_corr, b1));
}
+
+ if (reorder && reorder->probability) {
+ fprintf(f, " reorder %s",
+ sprint_percent(reorder->probability, b1));
+ if (reorder->correlation)
+ fprintf(f, " %s",
+ sprint_percent(reorder->correlation, b1));
+ }
if (qopt.gap)
fprintf(f, " gap %lu", (unsigned long)qopt.gap);
+
return 0;
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] netem: fix logic bug in reorder conditional
2005-05-21 23:00 ` Julio Kriger
@ 2005-05-23 17:43 ` Stephen Hemminger
2005-05-23 20:55 ` Julio Kriger
0 siblings, 1 reply; 13+ messages in thread
From: Stephen Hemminger @ 2005-05-23 17:43 UTC (permalink / raw)
To: Julio Kriger; +Cc: David S. Miller, netdev, netem
Thanks, Julio you spotted the problem with the logic. This should fix it:
Index: netem-2.6.12-rc4/net/sched/sch_netem.c
===================================================================
--- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
+++ netem-2.6.12-rc4/net/sched/sch_netem.c
@@ -181,13 +181,9 @@ static int netem_enqueue(struct sk_buff
q->duplicate = dupsave;
}
- /*
- * Do re-ordering by putting one out of N packets at the front
- * of the queue.
- * gap == 0 is special case for no-reordering.
- */
- if (q->gap == 0 && q->counter < q->gap &&
- q->reorder < get_crandom(&q->reorder_cor)) {
+ if (q->gap == 0 /* not doing reordering */
+ || q->counter < q->gap /* inside last reordering gap */
+ || q->reorder < get_crandom(&q->reorder_cor)) {
psched_time_t now;
PSCHED_GET_TIME(now);
PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
@@ -196,6 +192,10 @@ static int netem_enqueue(struct sk_buff
++q->counter;
ret = q->qdisc->enqueue(skb, q->qdisc);
} else {
+ /*
+ * Do re-ordering by putting one out of N packets at the front
+ * of the queue.
+ */
PSCHED_GET_TIME(cb->time_to_send);
q->counter = 0;
ret = q->qdisc->ops->requeue(skb, q->qdisc);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] netem: fix logic bug in reorder conditional
2005-05-23 17:43 ` [PATCH] netem: fix logic bug in reorder conditional Stephen Hemminger
@ 2005-05-23 20:55 ` Julio Kriger
2005-05-23 21:00 ` Stephen Hemminger
0 siblings, 1 reply; 13+ messages in thread
From: Julio Kriger @ 2005-05-23 20:55 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
Hi!
1) Supouse I don't want reordering, so in 'tc' I don't add reorder
option. Will this code work? The previous (on this subject) patch has
a the following:
+ /* for compatiablity with earlier versions.
+ * if gap is set, need to assume 100% probablity
+ */
+ q->reorder = ~0;
+
Should it be set to 100% or 101%?
This "q->reorder = ~0;" mean 100%?
2) If I set latency = 50ms and a jitter = 300ms, tabledist can give me
a negative number. This value is addes to cb->time_to_send, so it
could change it to a negative value. Should we only accept positives
number before add it to cb->time_to_send? or will
q->qdisc->enqueue(skb, q->qdisc) put the package on the queue in a
special way so it will be handled "before" other packages alrealy on
the queue but with gretaer time_to_send?
Regards,
Julio
On 5/23/05, Stephen Hemminger <shemminger@osdl.org> wrote:
> Thanks, Julio you spotted the problem with the logic. This should fix it:
>
> Index: netem-2.6.12-rc4/net/sched/sch_netem.c
> ===================================================================
> --- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
> +++ netem-2.6.12-rc4/net/sched/sch_netem.c
> @@ -181,13 +181,9 @@ static int netem_enqueue(struct sk_buff
> q->duplicate = dupsave;
> }
>
> - /*
> - * Do re-ordering by putting one out of N packets at the front
> - * of the queue.
> - * gap == 0 is special case for no-reordering.
> - */
> - if (q->gap == 0 && q->counter < q->gap &&
> - q->reorder < get_crandom(&q->reorder_cor)) {
> + if (q->gap == 0 /* not doing reordering */
> + || q->counter < q->gap /* inside last reordering gap */
> + || q->reorder < get_crandom(&q->reorder_cor)) {
> psched_time_t now;
> PSCHED_GET_TIME(now);
> PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
> @@ -196,6 +192,10 @@ static int netem_enqueue(struct sk_buff
> ++q->counter;
> ret = q->qdisc->enqueue(skb, q->qdisc);
> } else {
> + /*
> + * Do re-ordering by putting one out of N packets at the front
> + * of the queue.
> + */
> PSCHED_GET_TIME(cb->time_to_send);
> q->counter = 0;
> ret = q->qdisc->ops->requeue(skb, q->qdisc);
>
--
----------------------------
Julio Kriger
mailto:juliokriger@gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] netem: fix logic bug in reorder conditional
2005-05-23 20:55 ` Julio Kriger
@ 2005-05-23 21:00 ` Stephen Hemminger
2005-05-24 15:41 ` Julio Kriger
0 siblings, 1 reply; 13+ messages in thread
From: Stephen Hemminger @ 2005-05-23 21:00 UTC (permalink / raw)
To: Julio Kriger; +Cc: David S. Miller, netdev, netem
On Mon, 23 May 2005 17:55:35 -0300
Julio Kriger <juliokriger@gmail.com> wrote:
> Hi!
>
> 1) Supouse I don't want reordering, so in 'tc' I don't add reorder
> option. Will this code work? The previous (on this subject) patch has
> a the following:
>
> + /* for compatiablity with earlier versions.
> + * if gap is set, need to assume 100% probablity
> + */
> + q->reorder = ~0;
> +
If you don't want reordering, then gap = 0. so reorder is ignored.
>
> Should it be set to 100% or 101%?
> This "q->reorder = ~0;" mean 100%?
yes probability percentages are converted to 32-bit unsigned long scaled
value. so 100% = 0xffffffff and 0% = 0
> 2) If I set latency = 50ms and a jitter = 300ms, tabledist can give me
> a negative number. This value is addes to cb->time_to_send, so it
> could change it to a negative value. Should we only accept positives
> number before add it to cb->time_to_send? or will
> q->qdisc->enqueue(skb, q->qdisc) put the package on the queue in a
> special way so it will be handled "before" other packages alrealy on
> the queue but with gretaer time_to_send?
probably should bound the value to 0 before the addition, to avoid large
wraparound problems, but since enqueue checks for for time it will work
as long as delta less than 2^32/2.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] netem: fix logic bug in reorder conditional
2005-05-23 21:00 ` Stephen Hemminger
@ 2005-05-24 15:41 ` Julio Kriger
2005-05-24 16:57 ` Stephen Hemminger
0 siblings, 1 reply; 13+ messages in thread
From: Julio Kriger @ 2005-05-24 15:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
> > 2) If I set latency = 50ms and a jitter = 300ms, tabledist can give me
> > a negative number. This value is addes to cb->time_to_send, so it
> > could change it to a negative value. Should we only accept positives
> > number before add it to cb->time_to_send? or will
> > q->qdisc->enqueue(skb, q->qdisc) put the package on the queue in a
> > special way so it will be handled "before" other packages alrealy on
> > the queue but with gretaer time_to_send?
>
> probably should bound the value to 0 before the addition, to avoid large
> wraparound problems, but since enqueue checks for for time it will work
> as long as delta less than 2^32/2.
>
I think the value should be restricted to be positive and greater than
zero. Becuase if a negative number is allowed we will be "losing"
packages to be reordered, hence we will not be reordering, say 25%, of
packages instead we will be reordering about 15%.
In other words, packages that should be reordered will not be
reordered because its new time to send will be the same as the old
time to send.
Regards,
Julio
--
----------------------------
Julio Kriger
mailto:juliokriger@gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] netem: fix logic bug in reorder conditional
2005-05-24 15:41 ` Julio Kriger
@ 2005-05-24 16:57 ` Stephen Hemminger
2005-05-24 17:27 ` Julio Kriger
0 siblings, 1 reply; 13+ messages in thread
From: Stephen Hemminger @ 2005-05-24 16:57 UTC (permalink / raw)
To: Julio Kriger; +Cc: David S. Miller, netdev, netem
On Tue, 24 May 2005 12:41:11 -0300
Julio Kriger <juliokriger@gmail.com> wrote:
> > > 2) If I set latency = 50ms and a jitter = 300ms, tabledist can give me
> > > a negative number. This value is addes to cb->time_to_send, so it
> > > could change it to a negative value. Should we only accept positives
> > > number before add it to cb->time_to_send? or will
> > > q->qdisc->enqueue(skb, q->qdisc) put the package on the queue in a
> > > special way so it will be handled "before" other packages alrealy on
> > > the queue but with gretaer time_to_send?
> >
> > probably should bound the value to 0 before the addition, to avoid large
> > wraparound problems, but since enqueue checks for for time it will work
> > as long as delta less than 2^32/2.
> >
>
> I think the value should be restricted to be positive and greater than
> zero. Becuase if a negative number is allowed we will be "losing"
> packages to be reordered, hence we will not be reordering, say 25%, of
> packages instead we will be reordering about 15%.
> In other words, packages that should be reordered will not be
> reordered because its new time to send will be the same as the old
> time to send.
> Regards,
> Julio
The problem is that the user specification (latency 50ms +/- 300ms with reordering)
is problematic. Just like specifying reordering without delay (and a fast connection).
I chose to keep the original method of reordering by putting things at front of the
queue. Another alternative would be to keep a side-stack of packets to wait for
reordering. The problem with that is that if no following packet arrives, we end up
holding onto the last packet forever; and that would be bad.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] netem: fix logic bug in reorder conditional
2005-05-24 16:57 ` Stephen Hemminger
@ 2005-05-24 17:27 ` Julio Kriger
0 siblings, 0 replies; 13+ messages in thread
From: Julio Kriger @ 2005-05-24 17:27 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
On 5/24/05, Stephen Hemminger <shemminger@osdl.org> wrote:
> On Tue, 24 May 2005 12:41:11 -0300
> Julio Kriger <juliokriger@gmail.com> wrote:
>
> > > > 2) If I set latency = 50ms and a jitter = 300ms, tabledist can give me
> > > > a negative number. This value is addes to cb->time_to_send, so it
> > > > could change it to a negative value. Should we only accept positives
> > > > number before add it to cb->time_to_send? or will
> > > > q->qdisc->enqueue(skb, q->qdisc) put the package on the queue in a
> > > > special way so it will be handled "before" other packages alrealy on
> > > > the queue but with gretaer time_to_send?
> > >
> > > probably should bound the value to 0 before the addition, to avoid large
> > > wraparound problems, but since enqueue checks for for time it will work
> > > as long as delta less than 2^32/2.
> > >
> >
> > I think the value should be restricted to be positive and greater than
> > zero. Becuase if a negative number is allowed we will be "losing"
> > packages to be reordered, hence we will not be reordering, say 25%, of
> > packages instead we will be reordering about 15%.
> > In other words, packages that should be reordered will not be
> > reordered because its new time to send will be the same as the old
> > time to send.
> > Regards,
> > Julio
>
> The problem is that the user specification (latency 50ms +/- 300ms with reordering)
> is problematic. Just like specifying reordering without delay (and a fast connection).
I agree with you. Maybe changing the actual parameters to a "range"
could be a better solution. Say "I want delay a package between 50 and
300 ms with correlation of 50% and normal distribution". The code
should not change too much, I think.
Regards,
Julio
--
----------------------------
Julio Kriger
mailto:juliokriger@gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] (3/3) netem: allow random reordering (with fix)
2005-05-19 22:12 [PATCH] (3/3) netem: allow random reordering Stephen Hemminger
` (2 preceding siblings ...)
2005-05-21 23:00 ` Julio Kriger
@ 2005-05-24 22:26 ` Stephen Hemminger
2005-05-25 22:08 ` David S. Miller
3 siblings, 1 reply; 13+ messages in thread
From: Stephen Hemminger @ 2005-05-24 22:26 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
Here is a fixed up version of the reorder feature of netem.
It is the same as the earlier patch plus with the bugfix from Julio merged in.
Has expected backwards compatibility behaviour.
Go ahead and merge this one, the TCP strangeness I was seeing was due
to the reordering bug, and previous version of TSO patch.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
Index: netem/include/linux/pkt_sched.h
===================================================================
--- netem.orig/include/linux/pkt_sched.h
+++ netem/include/linux/pkt_sched.h
@@ -427,6 +427,7 @@ enum
TCA_NETEM_UNSPEC,
TCA_NETEM_CORR,
TCA_NETEM_DELAY_DIST,
+ TCA_NETEM_REORDER,
__TCA_NETEM_MAX,
};
@@ -437,7 +438,7 @@ struct tc_netem_qopt
__u32 latency; /* added delay (us) */
__u32 limit; /* fifo limit (packets) */
__u32 loss; /* random packet loss (0=none ~0=100%) */
- __u32 gap; /* re-ordering gap (0 for delay all) */
+ __u32 gap; /* re-ordering gap (0 for none) */
__u32 duplicate; /* random packet dup (0=none ~0=100%) */
__u32 jitter; /* random jitter in latency (us) */
};
@@ -449,6 +450,12 @@ struct tc_netem_corr
__u32 dup_corr; /* duplicate correlation */
};
+struct tc_netem_reorder
+{
+ __u32 probability;
+ __u32 correlation;
+};
+
#define NETEM_DIST_SCALE 8192
#endif
Index: netem/net/sched/sch_netem.c
===================================================================
--- netem.orig/net/sched/sch_netem.c
+++ netem/net/sched/sch_netem.c
@@ -62,11 +62,12 @@ struct netem_sched_data {
u32 gap;
u32 jitter;
u32 duplicate;
+ u32 reorder;
struct crndstate {
unsigned long last;
unsigned long rho;
- } delay_cor, loss_cor, dup_cor;
+ } delay_cor, loss_cor, dup_cor, reorder_cor;
struct disttable {
u32 size;
@@ -180,23 +181,23 @@ static int netem_enqueue(struct sk_buff
q->duplicate = dupsave;
}
- /*
- * Do re-ordering by putting one out of N packets at the front
- * of the queue.
- * gap == 0 is special case for no-reordering.
- */
- if (q->gap == 0 || q->counter != q->gap) {
+ if (q->gap == 0 /* not doing reordering */
+ || q->counter < q->gap /* inside last reordering gap */
+ || q->reorder < get_crandom(&q->reorder_cor)) {
psched_time_t now;
PSCHED_GET_TIME(now);
- PSCHED_TADD2(now,
- tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
+ PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
+ &q->delay_cor, q->delay_dist),
cb->time_to_send);
-
++q->counter;
ret = q->qdisc->enqueue(skb, q->qdisc);
} else {
- q->counter = 0;
+ /*
+ * Do re-ordering by putting one out of N packets at the front
+ * of the queue.
+ */
PSCHED_GET_TIME(cb->time_to_send);
+ q->counter = 0;
ret = q->qdisc->ops->requeue(skb, q->qdisc);
}
@@ -351,6 +352,19 @@ static int get_correlation(struct Qdisc
return 0;
}
+static int get_reorder(struct Qdisc *sch, const struct rtattr *attr)
+{
+ struct netem_sched_data *q = qdisc_priv(sch);
+ const struct tc_netem_reorder *r = RTA_DATA(attr);
+
+ if (RTA_PAYLOAD(attr) != sizeof(*r))
+ return -EINVAL;
+
+ q->reorder = r->probability;
+ init_crandom(&q->reorder_cor, r->correlation);
+ return 0;
+}
+
static int netem_change(struct Qdisc *sch, struct rtattr *opt)
{
struct netem_sched_data *q = qdisc_priv(sch);
@@ -371,9 +385,15 @@ static int netem_change(struct Qdisc *sc
q->jitter = qopt->jitter;
q->limit = qopt->limit;
q->gap = qopt->gap;
+ q->counter = 0;
q->loss = qopt->loss;
q->duplicate = qopt->duplicate;
+ /* for compatiablity with earlier versions.
+ * if gap is set, need to assume 100% probablity
+ */
+ q->reorder = ~0;
+
/* Handle nested options after initial queue options.
* Should have put all options in nested format but too late now.
*/
@@ -395,6 +415,11 @@ static int netem_change(struct Qdisc *sc
if (ret)
return ret;
}
+ if (tb[TCA_NETEM_REORDER-1]) {
+ ret = get_reorder(sch, tb[TCA_NETEM_REORDER-1]);
+ if (ret)
+ return ret;
+ }
}
@@ -412,7 +437,6 @@ static int netem_init(struct Qdisc *sch,
init_timer(&q->timer);
q->timer.function = netem_watchdog;
q->timer.data = (unsigned long) sch;
- q->counter = 0;
q->qdisc = qdisc_create_dflt(sch->dev, &pfifo_qdisc_ops);
if (!q->qdisc) {
@@ -444,6 +468,7 @@ static int netem_dump(struct Qdisc *sch,
struct rtattr *rta = (struct rtattr *) b;
struct tc_netem_qopt qopt;
struct tc_netem_corr cor;
+ struct tc_netem_reorder reorder;
qopt.latency = q->latency;
qopt.jitter = q->jitter;
@@ -457,6 +482,11 @@ static int netem_dump(struct Qdisc *sch,
cor.loss_corr = q->loss_cor.rho;
cor.dup_corr = q->dup_cor.rho;
RTA_PUT(skb, TCA_NETEM_CORR, sizeof(cor), &cor);
+
+ reorder.probability = q->reorder;
+ reorder.correlation = q->reorder_cor.rho;
+ RTA_PUT(skb, TCA_NETEM_REORDER, sizeof(reorder), &reorder);
+
rta->rta_len = skb->tail - b;
return skb->len;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] (3/3) netem: allow random reordering (with fix)
2005-05-24 22:26 ` [PATCH] (3/3) netem: allow random reordering (with fix) Stephen Hemminger
@ 2005-05-25 22:08 ` David S. Miller
0 siblings, 0 replies; 13+ messages in thread
From: David S. Miller @ 2005-05-25 22:08 UTC (permalink / raw)
To: shemminger; +Cc: netdev, netem
All 3 netem patches applied, thanks Stephen.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2005-05-25 22:08 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-05-19 22:12 [PATCH] (3/3) netem: allow random reordering Stephen Hemminger
2005-05-20 20:28 ` [Netem] " Julio Kriger
2005-05-23 17:25 ` Stephen Hemminger
2005-05-21 15:05 ` Julio Kriger
2005-05-21 23:00 ` Julio Kriger
2005-05-23 17:43 ` [PATCH] netem: fix logic bug in reorder conditional Stephen Hemminger
2005-05-23 20:55 ` Julio Kriger
2005-05-23 21:00 ` Stephen Hemminger
2005-05-24 15:41 ` Julio Kriger
2005-05-24 16:57 ` Stephen Hemminger
2005-05-24 17:27 ` Julio Kriger
2005-05-24 22:26 ` [PATCH] (3/3) netem: allow random reordering (with fix) Stephen Hemminger
2005-05-25 22:08 ` David S. Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).