On 27/11/15 15:16, Dave Taht wrote: > We decreased the size of the queue, regardless of what flow we dropped > from... so why do we not need to decrease the queue size when using > NET_XMIT_CN? Where is that done by the caller? I have grepped and > googled... > > As best as I recall the semantics of NET_XMIT_CN changed over time, it used to > only work on enqueue on a tail drop queue, where it would push back on > the stack to not, actually, drop the packet, but to halve the window > and requeue. > > Staring at the code now, I don't see how it could be right... here OR > in the existing fq_codel code in the mainline. > > /* Return Congestion Notification only if we dropped a packet > * from this flow. > */ > if (fq_codel_drop(sch) == idx) > return NET_XMIT_CN; > > /* As we dropped a packet, better let upper stack know this */ > qdisc_tree_decrease_qlen(sch, 1); > return NET_XMIT_SUCCESS; > > > (I am not sure NET_XMIT_CN works right when there is a hash collision, > for example, although the updated doc does seem to indicate that, > > ... still, then, where does (an equivalent for) > qdisc_tree_decrease_qlen kick in?) > > So I ripped it out. Otherwise... the code does look correct, now that > I've reviewed it further. > But does putting it back in have any measurable effect? I've no realistic way of testing & finding out. I'd say that if that bit of code ever gets executed then things are in an unfortunate place already, however if there's a way of telling a 'very active talker' (and they must be 'cos they've just queued a packet *and* cake_drop() has chosen them as having the biggest b->backlogs[i] - and that may help answer one of your todo items as well) to back off, then we should take it. Spaghetti monster knows what *actually* happens further up the stack :-) > > ... > > Also I note there was a bunch of commits to sch_fq_codel mainline since july, > notably this one. > > c0afd9ce4d6a646fb6433536f95a418bb348fab1 > > > Dave Täht > Let's go make home routers and wifi faster! With better software! > https://www.gofundme.com/savewifi >