Has anybody addressed these points yet? I don't understand much about qdiscs myself... Add to todo list? johannes -------- Forwarded Message -------- > From: Patrick McHardy <kaber@xxxxxxxxx> > To: David Kimdon <david.kimdon@xxxxxxxxxxxxxxx> > Cc: netdev@xxxxxxxxxxxxxxx, John W. Linville <linville@xxxxxxxxxxxxx>, > Jiri Benc <jbenc@xxxxxxx> > Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather than > d80211-specific qdisc > Date: Thu, 26 Oct 2006 03:21:10 +0200 [...] > BTW, I noticed a few bugs while looking at the qdisc handling in > wireless-dev: > > - wme_qdiscop_enqueue doesn't increment q.qlen for packets queued > to q->requeued[], which might cause upper layer code to stop > dequeueing if q.qlen reaches zero. > > - classify_1d doesn't care about tc_classify return values. > tc_classify may decide to steal packets, drop them, etc. In case > of stolen packets this causes use-after-free, otherwise just > malfunctions. > > - classify_1d returns res.class if it is != -1, which can never happen > (except with an empty classifier list because of the explicit > initialization, but you should check the return code) since ->get() > and ->bind_tcf() both return 0 for invalid classes and the classid > otherwise. There's also an off-by-one, classids start at one, so it > should return res.class - 1 (or better res.classid - 1, which is > meant to be a numerical identifier). > > - wme_discop_destroy leaks classifier module references and memory > when destroying classifiers, it should use tcf_destroy() > > Considering that it is possibly and may be desirable to attach a > different qdisc than the built-in multiband qdisc, it might also > make sense to split the 80211 specific classification in a seperate > classifier module to allow simple classification of management traffic > with other qdiscs.
Attachment:
signature.asc
Description: This is a digitally signed message part
