On Mon, Sep 01, 2008 at 03:49:14PM -0700, Alexander Duyck wrote:

On Mon, 2008-09-01 at 23:05 +0200, Jarek Poplawski wrote:

quoted

Mostly looks OK to me, but a few (late) doubts below:

Most of your suggestions I agree with, with the following exceptions.

quoted

...

quoted

+static int multiq_tune(struct Qdisc *sch, struct nlattr *opt)
+{
+	struct multiq_sched_data *q = qdisc_priv(sch);
+	struct tc_multiq_qopt *qopt;
+	struct Qdisc **queues;
+	int i;
+
+	if (sch->parent != TC_H_ROOT)
+		return -EINVAL;

Is it necessary?

I think so.  Basically I want to have this qdisc as the root for all
other qdiscs because the hardware queue decision needs to be made as
soon as possible in order to avoid any head of line blocking issues.
This way you don't end up with multiple qdiscs fighting over hardware
queues.

OK, but I wonder if it's not enough to treat this as a recommendation?
Actually, since dequeuing is under the common lock here, the main
difference seems to be this checking for subqueue_stopped could happen
a bit earlier, but this should be safe (a subqueue can't get another
packets in the meantime). So maybe I miss something but this looks
like blocking safe even when used as prio's leaf.

Thanks,
Jarek P.