Re: [Cake] Possible BUG - parent backlog incorrectly updated in case of NET_XMIT_CN

[Eric Dumazet <eric.dumazet@gmail.com>]

On Sat, 2016-06-11 at 19:41 +0300, Jonathan Morton wrote:
> > On 11 Jun, 2016, at 12:11, Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> wrote:
> > 
> > And now there's a version 2 of Eric's fix: http://marc.info/?l=linux-netdev&m=146507012302643&w=2
> > 
> > Does this change apply to cake?
> 
> I suppose this answers the question of which packet’s size should be
> used in the correction factor.  Of course, by this time in the enqueue
> process, the original packet length has been replaced by the
> overhead-corrected one in Cake, though I can save a copy for this
> purpose.
> 
> And there’s also the problem that we might not need to drop packets as
> large as the incoming packet in order to fit the latter into the queue
> - so this corrected correction may be *negative* (the queue is longer
> than before) - but qdisc_tree_reduce_backlog() only takes an unsigned
> parameter here.

That's a very minor detail.

If the code does :

reduce_backlog(unsigned quantity)
{
	q->backlog -= quantity;
}

Then the fact that @quantity is signed or unsigned is irrelevant.

You can submit a patch if you want ;)
> 


> IMHO the NET_XMIT_CN semantics are broken.  It might be better to drop
> support for it, since it should rarely be triggered.

What exact part is broken ?
Semantic looks good to me.

Implementations might be broken after recent Cong Wang patches.