On Sat, 19 Jan 2002, Martin Devera wrote:
> > Ok, is it sch_gred day or what? ;-> Martin you had to get me out hiding,
> > didnt you?
>
> Hehe, I have to keep you in touch with life .. I need you
> to look over my paper ;-)
BTW, your email seems to have a lot of problems. Please send the
paper via email (i have some cycles right now)
> well, look below ..
>
> > if (!table->initd) {
> > DPRINTK("NO GRED Queues setup!\n");
> > - return -1;
> > + goto rtattr_failure;
> > }
> >
>
> I think that you should not fail so hardly here. I'm not sure what
> is table->initd for but it is possible that user configures it in way
> where table->initd is NULL (actualy one user did it and complained
> about HTB error - this way I found bug above).
GRED insists that table->initd is non-zero to be completely configured.
It is best that they get caught (like what happened with your user);
essentially this is a fatal misconfig.
> Dump in sch_api.c calls xxx_dump for all qdiscs on the interface until
> all are exhausted or -1 is returned.
> By returning -1 in !table->initd case you prevent all other qdisc
> from being displayed. IMHO the -1 value is used for hard error
> (eg. unrecoverable one in RTNETLINK comm) not to report error in
> qdisc's setup.
> Do you think that it makes sense ?
>
In this case of GRED it might make _some_ sense but not in the case of any
other qdisc ... GRED is configured in two steps; essentially initd
protects to ensure that the first stage is complete; all other qdiscs
configure in one atomic operation. If you can think of some atomic way to
provision GRED (eg by batching the transaction) without annoying the user,
and without making the whole thing utterly complex, then -1 as return
would be correct 100% semantically; if not, it is the safest thing to do;
so we can leave it as is and meet 99.9% of semantical meaning. Let the
user suffer and learn ;->
cheers,
jamal
|