[PATCH 1/2] improve netem reorder flexibility

[PATCH 1/2] improve netem reorder flexibility

[Christian Samsel]

This patch adds a new feature to netem: The newly introduced parameter 
reorderdelay TIME is the time a reordered packet is delayed. It can be used in
a combination with delay TIME to enable netem to produce late packets, which is
not possible in the standard netem / iproute2. In the standard version 
reordered packets are always sent immediately and therefore are always early packets.

Signed-off-by: Christian Samsel <christian.samsel@rwth-aachen.de>
---
 include/linux/pkt_sched.h |   21 ++++++++++++---------
 net/sched/sch_netem.c     |   40 ++++++++++++++++++++++++----------------
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 2cfa4bc..eca6401 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -441,18 +441,21 @@ enum {
 #define TCA_NETEM_MAX (__TCA_NETEM_MAX - 1)
 
 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 none) */
-	__u32   duplicate;	/* random packet dup  (0=none ~0=100%) */
-	__u32	jitter;		/* random jitter in latency (us) */
+	__u32	latency;		/* added delay (us) */
+	__u32	limit;			/* fifo limit (packets) */
+	__u32	loss;			/* random packet loss (0=none ~0=100%) */
+	__u32	gap;			/* minimum re-ordering gap (0 for none) */
+	__u32	duplicate;		/* random packet dup  (0=none ~0=100%) */
+	__u32	jitter;			/* random jitter in latency (us) */
+	__u32	reorderdelay;		/* delay for reordered packets (us) */
+	__u32	reorderdelayjitter;	/* random jitter for reordered packets (us) */
 };
 
 struct tc_netem_corr {
-	__u32	delay_corr;	/* delay correlation */
-	__u32	loss_corr;	/* packet loss correlation */
-	__u32	dup_corr;	/* duplicate correlation  */
+	__u32	delay_corr;		/* delay correlation */
+	__u32	reorderdelay_corr;	/* reorderdelay correlation */
+	__u32	loss_corr;		/* packet loss correlation */
+	__u32	dup_corr;		/* duplicate correlation  */
 };
 
 struct tc_netem_reorder {
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index d8b10e0..84f7438 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -54,6 +54,7 @@ struct netem_sched_data {
 
 	psched_tdiff_t latency;
 	psched_tdiff_t jitter;
+	psched_tdiff_t reorderdelayjitter;
 
 	u32 loss;
 	u32 limit;
@@ -62,11 +63,12 @@ struct netem_sched_data {
 	u32 duplicate;
 	u32 reorder;
 	u32 corrupt;
+	u32 reorderdelay;
 
 	struct crndstate {
 		u32 last;
 		u32 rho;
-	} delay_cor, loss_cor, dup_cor, reorder_cor, corrupt_cor;
+	} delay_cor, reorderdelay_cor, loss_cor, dup_cor, reorder_cor, corrupt_cor;
 
 	struct disttable {
 		u32  size;
@@ -157,8 +159,10 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	/* We don't fill cb now as skb_unshare() may invalidate it */
 	struct netem_skb_cb *cb;
 	struct sk_buff *skb2;
+	struct sk_buff *skb3;
 	int ret;
 	int count = 1;
+	psched_tdiff_t delay;
 
 	pr_debug("netem_enqueue skb=%p\n", skb);
 
@@ -213,30 +217,30 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	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_tdiff_t delay;
 
+		/* no reordering (use standard delay) */
 		delay = tabledist(q->latency, q->jitter,
 				  &q->delay_cor, q->delay_dist);
-
-		now = psched_get_time();
-		cb->time_to_send = now + delay;
 		++q->counter;
-		ret = qdisc_enqueue(skb, q->qdisc);
 	} else {
-		/*
-		 * Do re-ordering by putting one out of N packets at the front
-		 * of the queue.
-		 */
-		cb->time_to_send = psched_get_time();
+		/* Do reordering (use reorderdelay) */
+		delay = tabledist(q->reorderdelay, q->reorderdelayjitter,
+				  &q->reorderdelay_cor, q->delay_dist);
 		q->counter = 0;
+	}
+	cb->time_to_send = psched_get_time()+delay;
 
-		__skb_queue_head(&q->qdisc->q, skb);
-		q->qdisc->qstats.backlog += qdisc_pkt_len(skb);
-		q->qdisc->qstats.requeues++;
-		ret = NET_XMIT_SUCCESS;
+	skb3 = q->qdisc->ops->peek(q->qdisc);
+	if (skb3) {
+		const struct netem_skb_cb *cb2 = netem_skb_cb(skb3);
+
+		/* if new packet is more recent, update watchdog */
+		if (cb->time_to_send < cb2->time_to_send)
+			qdisc_watchdog_schedule(&q->watchdog,cb->time_to_send);
 	}
 
+	ret = qdisc_enqueue(skb, q->qdisc);
+
 	if (likely(ret == NET_XMIT_SUCCESS)) {
 		sch->q.qlen++;
 		sch->bstats.bytes += qdisc_pkt_len(skb);
@@ -347,6 +351,7 @@ static void get_correlation(struct Qdisc *sch, const struct nlattr *attr)
 	const struct tc_netem_corr *c = nla_data(attr);
 
 	init_crandom(&q->delay_cor, c->delay_corr);
+	init_crandom(&q->reorderdelay_cor, c->reorderdelay_corr);
 	init_crandom(&q->loss_cor, c->loss_corr);
 	init_crandom(&q->dup_cor, c->dup_corr);
 }
@@ -418,6 +423,8 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt)
 	q->counter = 0;
 	q->loss = qopt->loss;
 	q->duplicate = qopt->duplicate;
+	q->reorderdelay = qopt->reorderdelay;
+	q->reorderdelayjitter = qopt->reorderdelayjitter;
 
 	/* for compatibility with earlier versions.
 	 * if gap is set, need to assume 100% probability
@@ -578,6 +585,7 @@ static int netem_dump(struct Qdisc *sch, struct sk_buff *skb)
 	qopt.loss = q->loss;
 	qopt.gap = q->gap;
 	qopt.duplicate = q->duplicate;
+	qopt.reorderdelay = q->reorderdelay;
 	NLA_PUT(skb, TCA_OPTIONS, sizeof(qopt), &qopt);
 
 	cor.delay_corr = q->delay_cor.rho;
-- 
1.6.4.1

[PATCH 2/2] improve netem reorder flexibility

[Christian Samsel]

supplementary patch for iproute2 to match kernel behavior.

Signed-off-by: Christian Samsel <christian.samsel@rwth-aachen.de>
---
 include/linux/pkt_sched.h |   21 +++++++++------
 tc/q_netem.c              |   61 ++++++++++++++++++++++++++++++++------------
 2 files changed, 56 insertions(+), 26 deletions(-)

diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index d51a2b3..2081195 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -473,19 +473,22 @@ enum
 
 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 none) */
-	__u32   duplicate;	/* random packet dup  (0=none ~0=100%) */
-	__u32	jitter;		/* random jitter in latency (us) */
+	__u32	latency;		/* added delay (us) */
+	__u32   limit;			/* fifo limit (packets) */
+	__u32	loss;			/* random packet loss (0=none ~0=100%) */
+	__u32	gap;			/* minimum re-ordering gap (0 for none) */
+	__u32   duplicate;		/* random packet dup  (0=none ~0=100%) */
+	__u32	jitter;			/* random jitter in latency (us) */
+	__u32   reorderdelay;   	/* delay for reordered packets (us) */
+	__u32	reorderdelayjitter;	/* random jitter for reordered packets (us) */
 };
 
 struct tc_netem_corr
 {
-	__u32	delay_corr;	/* delay correlation */
-	__u32	loss_corr;	/* packet loss correlation */
-	__u32	dup_corr;	/* duplicate correlation  */
+	__u32	delay_corr;		/* delay correlation */
+	__u32	reorderdelay_corr;	/* reorderdelay correlation */
+	__u32	loss_corr;		/* packet loss correlation */
+	__u32	dup_corr;		/* duplicate correlation  */
 };
 
 struct tc_netem_reorder
diff --git a/tc/q_netem.c b/tc/q_netem.c
index d06932e..eac9dc5 100644
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -28,13 +28,14 @@
 static void explain(void)
 {
 	fprintf(stderr,
-"Usage: ... netem [ limit PACKETS ] \n" \
-"                 [ delay TIME [ JITTER [CORRELATION]]]\n" \
-"                 [ distribution {uniform|normal|pareto|paretonormal} ]\n" \
-"                 [ drop PERCENT [CORRELATION]] \n" \
-"                 [ corrupt PERCENT [CORRELATION]] \n" \
-"                 [ duplicate PERCENT [CORRELATION]]\n" \
-"                 [ reorder PRECENT [CORRELATION] [ gap DISTANCE ]]\n");
+"Usage: ... netem [ limit PACKETS ]\n" \
+"                 [ delay TIME [JITTER [CORRELATION]] ]\n" \
+"                 [ distribution (uniform|normal|pareto|paretonormal) ]\n" \
+"                 [ drop PERCENT [CORRELATION] ]\n" \
+"                 [ corrupt PERCENT [CORRELATION] ]\n" \
+"                 [ duplicate PERCENT [CORRELATION] ]\n" \
+"                 [ ( reorder PRECENT [CORRELATION] | gap MINDISTANCE ) reorderdelay TIME [JITTER [CORRELATION]] ]\n" \
+);
 }
 
 static void explain1(const char *arg)
@@ -204,6 +205,29 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 					return -1;
 				}
 			}
+
+		} else if  (matches(*argv, "reorderdelay") == 0) {
+                        NEXT_ARG();
+                        if (get_ticks(&opt.reorderdelay, *argv)) {
+                                explain1("reorderdelay");
+                                return -1;
+                        }
+                        if (NEXT_IS_NUMBER()) {
+                                NEXT_ARG();
+                                if (get_ticks(&opt.reorderdelayjitter, *argv)) {
+                                        explain1("reorderdelay");
+                                        return -1;
+                                }
+				if (NEXT_IS_NUMBER()) {
+                                        NEXT_ARG();
+                                        ++present[TCA_NETEM_CORR];
+                                        if (get_percent(&cor.reorderdelay_corr, *argv)) {
+                                                explain1("reorderdelay");
+                                                return -1;
+                                        }
+                                }
+			}
+	
 		} else if (matches(*argv, "corrupt") == 0) {
 			NEXT_ARG();
 			present[TCA_NETEM_CORRUPT] = 1;
@@ -260,19 +284,18 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	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;
+			opt.gap = 1; /* set minimum reorder gap to 1 */
+	} else {
+		if ( opt.gap == 0 && opt.reorderdelay > 0) {
+			fprintf(stderr, "reorderdelay specified without reorder probability or gap\n");
+			explain();
+			return -1;
+		}
 	}
 
-	if (dist_data && (opt.latency == 0 || opt.jitter == 0)) {
-		fprintf(stderr, "distribution specified but no latency and jitter values\n");
+	if (dist_data && opt.jitter == 0 && opt.reorderjitter == 0) {
+		fprintf(stderr, "distribution specified but no delay or reorderdelay jitter values\n");
 		explain();
 		return -1;
 	}
@@ -345,6 +368,10 @@ static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 
 	fprintf(f, "limit %d", qopt.limit);
 
+	if (qopt.reorderdelay) {
+		fprintf(f, " reorderdelay %s", sprint_ticks(qopt.reorderdelay, b1) );
+	}
+
 	if (qopt.latency) {
 		fprintf(f, " delay %s", sprint_ticks(qopt.latency, b1));
 
-- 
1.6.4.1

Re: [PATCH 1/2] improve netem reorder flexibility

[Stephen Hemminger]

On Fri, 18 Dec 2009 14:18:37 +0100
Christian Samsel <christian.samsel@rwth-aachen.de> wrote:

> This patch adds a new feature to netem: The newly introduced parameter 
> reorderdelay TIME is the time a reordered packet is delayed. It can be used in
> a combination with delay TIME to enable netem to produce late packets, which is
> not possible in the standard netem / iproute2. In the standard version 
> reordered packets are always sent immediately and therefore are always early packets.

I like the idea but is it binary compatible with older kernels/ older iproute2
utilities?



-- 

Re: [PATCH 1/2] improve netem reorder flexibility

[Christian Samsel]

Am Freitag, 18. Dezember 2009 18:32:40 schrieb Stephen Hemminger:
> On Fri, 18 Dec 2009 14:18:37 +0100
> 
> Christian Samsel <christian.samsel@rwth-aachen.de> wrote:
> > This patch adds a new feature to netem: The newly introduced parameter
> > reorderdelay TIME is the time a reordered packet is delayed. It can be
> > used in a combination with delay TIME to enable netem to produce late
> > packets, which is not possible in the standard netem / iproute2. In the
> > standard version reordered packets are always sent immediately and
> > therefore are always early packets.
> 
> I like the idea but is it binary compatible with older kernels/ older
>  iproute2 utilities?
> 
Thats a good point. I will test it. So far i think it should work.

Re: [PATCH 1/2] improve netem reorder flexibility

[Christian Samsel]

Am Freitag, 18. Dezember 2009 18:32:40 schrieb Stephen Hemminger:
> On Fri, 18 Dec 2009 14:18:37 +0100
> 
> Christian Samsel <christian.samsel@rwth-aachen.de> wrote:
> > This patch adds a new feature to netem: The newly introduced parameter
> > reorderdelay TIME is the time a reordered packet is delayed. It can be
> > used in a combination with delay TIME to enable netem to produce late
> > packets, which is not possible in the standard netem / iproute2. In the
> > standard version reordered packets are always sent immediately and
> > therefore are always early packets.
> 
> I like the idea but is it binary compatible with older kernels/ older
>  iproute2 utilities?
> 
Thats a good point. I will test it. So far i think it should work.

Re: [PATCH 1/2] improve netem reorder flexibility

[David Miller]

From: Christian Samsel <christian.samsel@rwth-aachen.de>
Date: Fri, 18 Dec 2009 19:01:28 +0100

> Am Freitag, 18. Dezember 2009 18:32:40 schrieb Stephen Hemminger:
>> On Fri, 18 Dec 2009 14:18:37 +0100
>> 
>> Christian Samsel <christian.samsel@rwth-aachen.de> wrote:
>> > This patch adds a new feature to netem: The newly introduced parameter
>> > reorderdelay TIME is the time a reordered packet is delayed. It can be
>> > used in a combination with delay TIME to enable netem to produce late
>> > packets, which is not possible in the standard netem / iproute2. In the
>> > standard version reordered packets are always sent immediately and
>> > therefore are always early packets.
>> 
>> I like the idea but is it binary compatible with older kernels/ older
>>  iproute2 utilities?
>> 
> Thats a good point. I will test it. So far i think it should work.

It won't work.

If you change the size of the tc_netem_qopt, older kernels will
reject because the size is different from what the kernel expects.

So newer iproute2 will not work with older kernels.

You can't change these structures, add new netlink attributes
to pass the new data instead.

Re: [PATCH 1/2] improve netem reorder flexibility

[Christian Samsel]

Am Samstag, 19. Dezember 2009 04:59:07 schrieb David Miller:
> From: Christian Samsel <christian.samsel@rwth-aachen.de>
> Date: Fri, 18 Dec 2009 19:01:28 +0100
> 
> > Am Freitag, 18. Dezember 2009 18:32:40 schrieb Stephen Hemminger:
> >> On Fri, 18 Dec 2009 14:18:37 +0100
> >>
> >> Christian Samsel <christian.samsel@rwth-aachen.de> wrote:
> >> > This patch adds a new feature to netem: The newly introduced parameter
> >> > reorderdelay TIME is the time a reordered packet is delayed. It can be
> >> > used in a combination with delay TIME to enable netem to produce late
> >> > packets, which is not possible in the standard netem / iproute2. In
> >> > the standard version reordered packets are always sent immediately and
> >> > therefore are always early packets.
> >>
> >> I like the idea but is it binary compatible with older kernels/ older
> >>  iproute2 utilities?
> >
> > Thats a good point. I will test it. So far i think it should work.
> 
> It won't work.
> 
> If you change the size of the tc_netem_qopt, older kernels will
> reject because the size is different from what the kernel expects.

I just tested it both ways (vanilla kernel + changed tc, changed kernel + 
vanilla tc). The reordering behaved wierd. But i did not see something like a 
"reject". Other netem features like DROP still worked fine.
Are you sure this can't be fixed in some way? From my feeling, it might be 
just a small tweaking in parameter handling necessary.

Obviously, sending the new data to an unchanged kernel won't work, but that 
should be ok, isn't it?

> So newer iproute2 will not work with older kernels.
> 
> You can't change these structures, add new netlink attributes
> to pass the new data instead.

I will look into this, but having similiar netem data in two different places 
seems awkward to me.

Re: [PATCH 1/2] improve netem reorder flexibility

[Stephen Hemminger]

On Fri, 18 Dec 2009 19:59:07 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Christian Samsel <christian.samsel@rwth-aachen.de>
> Date: Fri, 18 Dec 2009 19:01:28 +0100
> 
> > Am Freitag, 18. Dezember 2009 18:32:40 schrieb Stephen Hemminger:  
> >> On Fri, 18 Dec 2009 14:18:37 +0100
> >> 
> >> Christian Samsel <christian.samsel@rwth-aachen.de> wrote:  
> >> > This patch adds a new feature to netem: The newly introduced parameter
> >> > reorderdelay TIME is the time a reordered packet is delayed. It can be
> >> > used in a combination with delay TIME to enable netem to produce late
> >> > packets, which is not possible in the standard netem / iproute2. In the
> >> > standard version reordered packets are always sent immediately and
> >> > therefore are always early packets.  
> >> 
> >> I like the idea but is it binary compatible with older kernels/ older
> >>  iproute2 utilities?
> >>   
> > Thats a good point. I will test it. So far i think it should work.  
> 
> It won't work.
> 
> If you change the size of the tc_netem_qopt, older kernels will
> reject because the size is different from what the kernel expects.
> 
> So newer iproute2 will not work with older kernels.
> 
> You can't change these structures, add new netlink attributes
> to pass the new data instead.

The netem logic for parsing attributes is different from others for
historical reasons.  Looking at parse_attr():

If qopt is larger than expected, then the extra data is interpreted as
the following nested attribute.

If qopt is smaller than expected, then it returns -EINVAL.

Note: in early versions of netem development, the parsing was different,
and it was possible to grow the structure, but the attribute parsing got
changed and is now more restrictive.  BUT that is okay, since the correct
way to add options to netlink API's is to add new attributes, not extend
size of structures.

Re: [PATCH 1/2] improve netem reorder flexibility

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 21 Dec 2009 09:54:07 -0800

> The netem logic for parsing attributes is different from others for
> historical reasons.  Looking at parse_attr():
> 
> If qopt is larger than expected, then the extra data is interpreted as
> the following nested attribute.
> 
> If qopt is smaller than expected, then it returns -EINVAL.

This is a really dangerous way to handle this.

This means if you grow qopt, older kernels think that the
extended area is attributes.

This also means that if you have new tools and send a qopt +
attributes, older kernels won't find the start of the attributes
correctly.  It will miss the attributes entirely.

This is a complete mess, and we need to sort it out before we think
about changing the size of qopt at this point.

Re: [PATCH 1/2] improve netem reorder flexibility

[Christian Samsel]

Am Samstag, 16. Januar 2010 10:46:54 schrieb David Miller:
> 
> This is a really dangerous way to handle this.
> 
> This means if you grow qopt, older kernels think that the
> extended area is attributes.
> 
> This also means that if you have new tools and send a qopt +
> attributes, older kernels won't find the start of the attributes
> correctly.  It will miss the attributes entirely.
True, i know now. 

> This is a complete mess, and we need to sort it out before we think
> about changing the size of qopt at this point.
> 
Just to state this clearly, by cleaning up the mess you mean switching netem 
completely over to netlink attributes? 

I think 4B3A5F84.4050603@yahoo.it would also take advantage of that. The 
approach they choosed looks a bit wierd to me,  cause someday we could end up 
having 
tc qdisc add dev eth0 root netem
tc qdisc add dev eth0 root netem2
tc qdisc add dev eth0 root netem3
..

Christian

Re: [PATCH 1/2] improve netem reorder flexibility

[David Miller]

From: Christian Samsel <christian.samsel@rwth-aachen.de>
Date: Sun, 17 Jan 2010 13:11:18 +0100

> Am Samstag, 16. Januar 2010 10:46:54 schrieb David Miller:
>> This is a complete mess, and we need to sort it out before we think
>> about changing the size of qopt at this point.
>> 
> Just to state this clearly, by cleaning up the mess you mean switching netem 
> completely over to netlink attributes? 

I think the only way we can proceed is to keep the current qopt
structure static, and only add things using new attributes.

I would also suggest that we make sch_netem.c strictly require a
specific size for the structure.  And before anyone says that will
break something, it's much better than what we have now which is
random, silent, dropping of attributes.

Re: [PATCH 1/2] improve netem reorder flexibility

[Stephen Hemminger]

On Sun, 17 Jan 2010 18:56:36 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Christian Samsel <christian.samsel@rwth-aachen.de>
> Date: Sun, 17 Jan 2010 13:11:18 +0100
> 
> > Am Samstag, 16. Januar 2010 10:46:54 schrieb David Miller:
> >> This is a complete mess, and we need to sort it out before we think
> >> about changing the size of qopt at this point.
> >> 
> > Just to state this clearly, by cleaning up the mess you mean switching netem 
> > completely over to netlink attributes? 
> 
> I think the only way we can proceed is to keep the current qopt
> structure static, and only add things using new attributes.
> 
> I would also suggest that we make sch_netem.c strictly require a
> specific size for the structure.  And before anyone says that will
> break something, it's much better than what we have now which is
> random, silent, dropping of attributes.


Just to make it clearer. The normal netlink method of nesting attributes
(done by nla_parse_nested) is:

 +----------------------+------------
 |  len1 | type1| data1 | len2 | ...
 +----------------------+------------
 
But netem uses a sequential format:

 +------+---------------------+------------
 | qopt | len1 | type1| data1 | len2 | ...
 +------+---------------------+------------

So to enhance netlink, the best way is to add new attributes.
Changing the size of qopt will break binary compatiablity because
the code that parses sequential options will get the wrong offset.
Changing the size of attributes will also break compatiablity
as well.

Also, at some point all the functions in netem could be broken
into separate if too complex. For example, the code that was
submitted to replay a recorded trace probably would be better off
as a separate qdisc.