Patrick McHardy wrote:

Corey Hickey wrote:

quoted

Re-implement sfq_change() and enable Qdisc_opts.change so "tc qdisc
change" will work.

Signed-off-by: Corey Hickey <redacted>
---
 net/sched/sch_sfq.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index e6a6a21..e042cd0 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c

@@ -485,6 +485,55 @@ static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
 	return 0;
 }
 
+static int sfq_change(struct Qdisc *sch, struct rtattr *opt)
+{
+	struct sfq_sched_data *q = qdisc_priv(sch);
+	struct sfq_sched_data tmp;
+	struct sk_buff *skb;
+	int err;
+	
+	/* set up tmp queue */
+	memset(&tmp, 0, sizeof(struct sfq_sched_data));
+	tmp.quantum = psched_mtu(sch->dev); /* default */

If no value is given it should use the old value instead of
reinitializing to the default.

quoted

+	if ((err = sfq_q_init(&tmp, opt)))
+		return err;

This will also use defaults for all unspecified values. It would
be more consistent with other qdiscs to only change those values
that are actually specified, so something like "tc qdisc change ...
perturb 10" will *only* change the perturbation parameter.

I somehow had it in my head that a qdisc change was supposed to work 
that way. I'll change my patch.

I'm not sure reusing the initialization function and copying the
parameters is the best way to do this.

I'm not sure either, but I do like that it's conceptually simple and it 
keeps the parameter handling in one place. I've thought and stared for a 
while, and I don't see a better way, but that could of course be due to 
my limited understanding and general newbiehood in that regard.

Thanks again for the review. I'll try to get a new batch of patches sent 
off tomorrow.

-Corey